Messages in this thread | | | Date | Tue, 21 Nov 2023 12:53:11 +0100 | Subject | Re: [PATCH v1 18/20] soc: mediatek: mtk-svs: Constify runtime-immutable members of svs_bank | From | AngeloGioacchino Del Regno <> |
| |
Il 17/11/23 15:32, Eugen Hristev ha scritto: > > Hi Angelo, > > > On 11/17/23 11:42, AngeloGioacchino Del Regno wrote: >> Some members of struct svs_bank are not changed during runtime, so those >> are not variables but constants: move all of those to a new structure >> called svs_bank_pdata and refactor the code to make use of that. >> This effectively moves at least 50 bytes to the text segment. >> While at it, also uniform the thermal zone names across the banks. >> >> Signed-off-by: AngeloGioacchino Del Regno <angelogioacchino.delregno@collabora.com> >> --- >> drivers/soc/mediatek/mtk-svs.c | 1201 +++++++++++++++++--------------- >> 1 file changed, 631 insertions(+), 570 deletions(-) >> >> diff --git a/drivers/soc/mediatek/mtk-svs.c b/drivers/soc/mediatek/mtk-svs.c >> index df39e7430ba9..aa50ae0cc1d1 100644 >> --- a/drivers/soc/mediatek/mtk-svs.c >> +++ b/drivers/soc/mediatek/mtk-svs.c >> @@ -1,6 +1,8 @@ >> // SPDX-License-Identifier: GPL-2.0-only >> /* >> * Copyright (C) 2022 MediaTek Inc. >> + * Copyright (C) 2022 Collabora Ltd. >> + * AngeloGioacchino Del Regno >> <angelogioacchino.delregno@collabora.com> >> */ >> #include <linux/bitfield.h> >> @@ -118,7 +120,7 @@ >> #define SVSB_VOPS_FLD_VOP2_6 GENMASK(23, 16) >> #define SVSB_VOPS_FLD_VOP3_7 GENMASK(31, 24) >> -/* SVS Thermal Coefficients */ >> +/* SVS Thermal */ > > 'Coefficients' made sense for me. Without the word , does the comment many any > sense anymore ? >
I didn't mean to change that, thanks for pointing that out.
>> #define SVSB_TS_COEFF_MT8195 250460 >> #define SVSB_TS_COEFF_MT8186 204650 >> @@ -391,7 +393,7 @@ struct svs_platform { >> size_t efuse_max; >> size_t tefuse_max; >> const u32 *regs; >> - u32 bank_max; >> + u16 bank_max; > > does it make sense to order them by size ? >
Yes, it does. Reordered in v2.
>> u32 *efuse; >> u32 *tefuse; >> u32 ts_coeff; >> @@ -404,59 +406,92 @@ struct svs_platform_data { >> int (*probe)(struct svs_platform *svsp); >> const struct svs_fusemap *glb_fuse_map; >> const u32 *regs; >> - u32 bank_max; >> + u16 bank_max; > here as well.
yep.
>> u32 ts_coeff; >> }; >> /** >> - * struct svs_bank - svs bank representation >> + * struct svs_bank_pdata - SVS Bank immutable config parameters >> * @dev_fuse_map: Bank fuse map data >> + * @buck_name: Regulator name >> + * @tzone_name: Thermal zone name >> + * @age_config: Bank age configuration >> + * @ctl0: TS-x selection >> + * @dc_config: Bank dc configuration >> + * @int_st: Bank interrupt identification >> + * @turn_freq_base: Reference frequency for 2-line turn point >> + * @tzone_htemp: Thermal zone high temperature threshold >> + * @tzone_ltemp: Thermal zone low temperature threshold >> + * @volt_step: Bank voltage step >> + * @volt_base: Bank voltage base >> + * @tzone_htemp_voffset: Thermal zone high temperature voltage offset >> + * @tzone_ltemp_voffset: Thermal zone low temperature voltage offset >> + * @chk_shift: Bank chicken shift >> + * @cpu_id: CPU core ID for SVS CPU bank use only >> + * @opp_count: Bank opp count >> + * @vboot: Voltage request for bank init01 only >> + * @vco: Bank VCO value >> + * @sw_id: Bank software identification >> + * @type: SVS Bank Type (1 or 2-line) and Role (high/low) >> + * @set_freq_pct: function pointer to set bank frequency percent table >> + * @get_volts: function pointer to get bank voltages >> + */ >> +struct svs_bank_pdata { >> + const struct svs_fusemap *dev_fuse_map; >> + char *buck_name; >> + char *tzone_name; >> + u32 age_config; >> + u32 ctl0; >> + u32 dc_config; >> + u32 int_st; >> + u32 turn_freq_base; >> + u32 tzone_htemp; >> + u32 tzone_ltemp; >> + u32 volt_step; >> + u32 volt_base; >> + u16 tzone_htemp_voffset; >> + u16 tzone_ltemp_voffset; >> + u8 chk_shift; >> + u8 cpu_id; >> + u8 opp_count; >> + u8 vboot; >> + u8 vco; >> + u8 sw_id; >> + u8 type; >> + >> + /* Callbacks */ >> + void (*set_freq_pct)(struct svs_platform *, struct svs_bank *svsb); >> + void (*get_volts)(struct svs_platform *, struct svs_bank *svsb); > > WARNING: function definition argument 'struct svs_platform *' should also have an > identifier name >
Indeed. Fixed.
>> +}; >> + >> +/** >> + * struct svs_bank - svs bank representation >> + * @pdata: SVS Bank immutable config parameters >> * @dev: bank device >> * @opp_dev: device for opp table/buck control >> * @init_completion: the timeout completion for bank init >> * @buck: regulator used by opp_dev >> * @tzd: thermal zone device for getting temperature >> * @lock: mutex lock to protect voltage update process >> - * @set_freq_pct: function pointer to set bank frequency percent table >> - * @get_volts: function pointer to get bank voltages >> * @name: bank name >> - * @buck_name: regulator name >> - * @tzone_name: thermal zone name >> * @phase: bank current phase >> * @volt_od: bank voltage overdrive >> * @reg_data: bank register data in different phase for debug purpose >> * @pm_runtime_enabled_count: bank pm runtime enabled count >> - * @mode_support: bank mode support. >> + * @mode_support: bank mode support >> * @freq_base: reference frequency for bank init >> - * @turn_freq_base: refenrece frequency for 2-line turn point >> - * @vboot: voltage request for bank init01 only >> * @opp_dfreq: default opp frequency table >> * @opp_dvolt: default opp voltage table >> * @freq_pct: frequency percent table for bank init >> * @volt: bank voltage table >> - * @volt_step: bank voltage step >> - * @volt_base: bank voltage base >> * @volt_flags: bank voltage flags >> * @vmax: bank voltage maximum >> * @vmin: bank voltage minimum >> - * @age_config: bank age configuration >> * @age_voffset_in: bank age voltage offset >> - * @dc_config: bank dc configuration >> * @dc_voffset_in: bank dc voltage offset >> * @dvt_fixed: bank dvt fixed value >> - * @vco: bank VCO value >> - * @chk_shift: bank chicken shift >> * @core_sel: bank selection >> - * @opp_count: bank opp count >> - * @int_st: bank interrupt identification >> - * @sw_id: bank software identification >> - * @cpu_id: cpu core id for SVS CPU bank use only >> - * @ctl0: TS-x selection >> * @temp: bank temperature >> - * @tzone_htemp: thermal zone high temperature threshold >> - * @tzone_htemp_voffset: thermal zone high temperature voltage offset >> - * @tzone_ltemp: thermal zone low temperature threshold >> - * @tzone_ltemp_voffset: thermal zone low temperature voltage offset >> * @bts: svs efuse data >> * @mts: svs efuse data >> * @bdes: svs efuse data >> @@ -466,7 +501,6 @@ struct svs_platform_data { >> * @dcmdet: svs efuse data >> * @turn_pt: 2-line turn point tells which opp_volt calculated by high/low bank >> * @vbin_turn_pt: voltage bin turn point helps know which svsb_volt should be >> overridden >> - * @type: bank type to represent it is 2-line (high/low) bank or 1-line bank >> * >> * Svs bank will generate suitalbe voltages by below general math equation > > can you also fix this typo 'suitalbe' >
Fixed.
>> * and provide these voltages to opp voltage table. >> @@ -474,53 +508,34 @@ struct svs_platform_data { >> * opp_volt[i] = (volt[i] * volt_step) + volt_base; >> */ >> struct svs_bank { >> - const struct svs_fusemap *dev_fuse_map; >> + const struct svs_bank_pdata pdata; >> struct device *dev; >> struct device *opp_dev; >> struct completion init_completion; >> struct regulator *buck; >> struct thermal_zone_device *tzd; >> - struct mutex lock; /* lock to protect voltage update process */ >> - void (*set_freq_pct)(struct svs_platform *svsp, struct svs_bank *svsb); >> - void (*get_volts)(struct svs_platform *svsp, struct svs_bank *svsb); >> + struct mutex lock; >> + int pm_runtime_enabled_count; >> + short int volt_od; >> char *name; >> - char *buck_name; >> - char *tzone_name; >> enum svsb_phase phase; >> - short int volt_od; >> u32 reg_data[SVSB_PHASE_MAX][SVS_REG_MAX]; >> - u32 pm_runtime_enabled_count; >> u8 mode_support; >> - u32 freq_base; >> - u32 turn_freq_base; >> - u8 vboot; >> u32 opp_dfreq[MAX_OPP_ENTRIES]; >> u32 opp_dvolt[MAX_OPP_ENTRIES]; >> u32 freq_pct[MAX_OPP_ENTRIES]; >> u32 volt[MAX_OPP_ENTRIES]; >> - u32 volt_step; >> - u32 volt_base; >> u32 volt_flags; >> - u8 vmax; >> - u8 vmin; >> - u32 age_config; >> + u32 freq_base; >> + u32 turn_pt; >> + u32 vbin_turn_pt; >> + u32 core_sel; >> + u32 temp; >> u16 age_voffset_in; >> - u32 dc_config; >> u16 dc_voffset_in; >> u8 dvt_fixed; >> - u8 vco; >> - u8 chk_shift; >> - u32 core_sel; >> - u8 opp_count; >> - u32 int_st; >> - u8 sw_id; >> - u8 cpu_id; >> - u32 ctl0; >> - u32 temp; >> - u32 tzone_htemp; >> - u16 tzone_htemp_voffset; >> - u32 tzone_ltemp; >> - u16 tzone_ltemp_voffset; >> + u8 vmax; >> + u8 vmin; >> u16 bts; >> u16 mts; >> u16 bdes; >> @@ -528,9 +543,6 @@ struct svs_bank { >> u8 mtdes; >> u8 dcbdet; >> u8 dcmdet; >> - u32 turn_pt; >> - u32 vbin_turn_pt; >> - u8 type; >> }; >> static u32 percent(u32 numerator, u32 denominator) >> @@ -572,10 +584,11 @@ static u32 svs_opp_volt_to_bank_volt(u32 opp_u_volt, u32 >> svsb_volt_step, >> static int svs_sync_bank_volts_from_opp(struct svs_bank *svsb) >> { >> + const struct svs_bank_pdata *bdata = &svsb->pdata; >> struct dev_pm_opp *opp; >> u32 i, opp_u_volt; >> - for (i = 0; i < svsb->opp_count; i++) { >> + for (i = 0; i < bdata->opp_count; i++) { >> opp = dev_pm_opp_find_freq_exact(svsb->opp_dev, >> svsb->opp_dfreq[i], >> true); >> @@ -587,8 +600,8 @@ static int svs_sync_bank_volts_from_opp(struct svs_bank *svsb) >> opp_u_volt = dev_pm_opp_get_voltage(opp); >> svsb->volt[i] = svs_opp_volt_to_bank_volt(opp_u_volt, >> - svsb->volt_step, >> - svsb->volt_base); >> + bdata->volt_step, >> + bdata->volt_base); >> dev_pm_opp_put(opp); >> } >> @@ -598,6 +611,7 @@ static int svs_sync_bank_volts_from_opp(struct svs_bank *svsb) >> static int svs_adjust_pm_opp_volts(struct svs_bank *svsb) >> { >> int ret = -EPERM, tzone_temp = 0; >> + const struct svs_bank_pdata *bdata = &svsb->pdata; >> u32 i, svsb_volt, opp_volt, temp_voffset = 0, opp_start, opp_stop; >> mutex_lock(&svsb->lock); >> @@ -606,15 +620,15 @@ static int svs_adjust_pm_opp_volts(struct svs_bank *svsb) >> * 2-line bank updates its corresponding opp volts. >> * 1-line bank updates all opp volts. >> */ >> - if (svsb->type == SVSB_TYPE_HIGH) { >> + if (bdata->type == SVSB_TYPE_HIGH) { >> opp_start = 0; >> opp_stop = svsb->turn_pt; >> - } else if (svsb->type == SVSB_TYPE_LOW) { >> + } else if (bdata->type == SVSB_TYPE_LOW) { >> opp_start = svsb->turn_pt; >> - opp_stop = svsb->opp_count; >> + opp_stop = bdata->opp_count; >> } else { >> opp_start = 0; >> - opp_stop = svsb->opp_count; >> + opp_stop = bdata->opp_count; >> } >> /* Get thermal effect */ >> @@ -623,20 +637,20 @@ static int svs_adjust_pm_opp_volts(struct svs_bank *svsb) >> if (ret || (svsb->temp > SVSB_TEMP_UPPER_BOUND && >> svsb->temp < SVSB_TEMP_LOWER_BOUND)) { >> dev_err(svsb->dev, "%s: %d (0x%x), run default volts\n", >> - svsb->tzone_name, ret, svsb->temp); >> + bdata->tzone_name, ret, svsb->temp); >> svsb->phase = SVSB_PHASE_ERROR; >> } >> - if (tzone_temp >= svsb->tzone_htemp) >> - temp_voffset += svsb->tzone_htemp_voffset; >> - else if (tzone_temp <= svsb->tzone_ltemp) >> - temp_voffset += svsb->tzone_ltemp_voffset; >> + if (tzone_temp >= bdata->tzone_htemp) >> + temp_voffset += bdata->tzone_htemp_voffset; >> + else if (tzone_temp <= bdata->tzone_ltemp) >> + temp_voffset += bdata->tzone_ltemp_voffset; >> /* 2-line bank update all opp volts when running mon mode */ >> - if (svsb->phase == SVSB_PHASE_MON && (svsb->type == SVSB_TYPE_HIGH || >> - svsb->type == SVSB_TYPE_LOW)) { >> + if (svsb->phase == SVSB_PHASE_MON && (bdata->type == SVSB_TYPE_HIGH || >> + bdata->type == SVSB_TYPE_LOW)) { >> opp_start = 0; >> - opp_stop = svsb->opp_count; >> + opp_stop = bdata->opp_count; >> } >> } >> @@ -653,8 +667,8 @@ static int svs_adjust_pm_opp_volts(struct svs_bank *svsb) >> case SVSB_PHASE_MON: >> svsb_volt = max(svsb->volt[i] + temp_voffset, svsb->vmin); >> opp_volt = svs_bank_volt_to_opp_volt(svsb_volt, >> - svsb->volt_step, >> - svsb->volt_base); >> + bdata->volt_step, >> + bdata->volt_base); >> break; >> default: >> dev_err(svsb->dev, "unknown phase: %u\n", svsb->phase); >> @@ -816,7 +830,7 @@ static int svs_status_debug_show(struct seq_file *m, void *v) >> svsb->name, tzone_temp, svsb->vbin_turn_pt, >> svsb->turn_pt); >> - for (i = 0; i < svsb->opp_count; i++) { >> + for (i = 0; i < svsb->pdata.opp_count; i++) { >> opp = dev_pm_opp_find_freq_exact(svsb->opp_dev, >> svsb->opp_dfreq[i], true); >> if (IS_ERR(opp)) { >> @@ -923,9 +937,10 @@ static u32 interpolate(u32 f0, u32 f1, u32 v0, u32 v1, u32 fx) >> static void svs_get_bank_volts_v3(struct svs_platform *svsp, struct svs_bank >> *svsb) >> { >> + const struct svs_bank_pdata *bdata = &svsb->pdata; >> u32 i, j, *vop, vop74, vop30, turn_pt = svsb->turn_pt; >> u32 b_sft, shift_byte = 0, opp_start = 0, opp_stop = 0; >> - u32 middle_index = (svsb->opp_count / 2); >> + u32 middle_index = (bdata->opp_count / 2); >> if (svsb->phase == SVSB_PHASE_MON && >> svsb->volt_flags & SVSB_MON_VOLT_IGNORE) >> @@ -936,7 +951,7 @@ static void svs_get_bank_volts_v3(struct svs_platform *svsp, >> struct svs_bank *sv >> /* Target is to set svsb->volt[] by algorithm */ >> if (turn_pt < middle_index) { >> - if (svsb->type == SVSB_TYPE_HIGH) { >> + if (bdata->type == SVSB_TYPE_HIGH) { >> /* volt[0] ~ volt[turn_pt - 1] */ >> for (i = 0; i < turn_pt; i++) { >> b_sft = BITS8 * (shift_byte % REG_BYTES); >> @@ -945,12 +960,12 @@ static void svs_get_bank_volts_v3(struct svs_platform >> *svsp, struct svs_bank *sv >> svsb->volt[i] = (*vop >> b_sft) & GENMASK(7, 0); >> shift_byte++; >> } >> - } else if (svsb->type == SVSB_TYPE_LOW) { >> + } else if (bdata->type == SVSB_TYPE_LOW) { >> /* volt[turn_pt] + volt[j] ~ volt[opp_count - 1] */ >> - j = svsb->opp_count - 7; >> + j = bdata->opp_count - 7; >> svsb->volt[turn_pt] = FIELD_GET(SVSB_VOPS_FLD_VOP0_4, vop30); >> shift_byte++; >> - for (i = j; i < svsb->opp_count; i++) { >> + for (i = j; i < bdata->opp_count; i++) { >> b_sft = BITS8 * (shift_byte % REG_BYTES); >> vop = (shift_byte < REG_BYTES) ? &vop30 : >> &vop74; >> @@ -967,7 +982,7 @@ static void svs_get_bank_volts_v3(struct svs_platform *svsp, >> struct svs_bank *sv >> svsb->freq_pct[i]); >> } >> } else { >> - if (svsb->type == SVSB_TYPE_HIGH) { >> + if (bdata->type == SVSB_TYPE_HIGH) { >> /* volt[0] + volt[j] ~ volt[turn_pt - 1] */ >> j = turn_pt - 7; >> svsb->volt[0] = FIELD_GET(SVSB_VOPS_FLD_VOP0_4, vop30); >> @@ -987,9 +1002,9 @@ static void svs_get_bank_volts_v3(struct svs_platform *svsp, >> struct svs_bank *sv >> svsb->volt[0], >> svsb->volt[j], >> svsb->freq_pct[i]); >> - } else if (svsb->type == SVSB_TYPE_LOW) { >> + } else if (bdata->type == SVSB_TYPE_LOW) { >> /* volt[turn_pt] ~ volt[opp_count - 1] */ >> - for (i = turn_pt; i < svsb->opp_count; i++) { >> + for (i = turn_pt; i < bdata->opp_count; i++) { >> b_sft = BITS8 * (shift_byte % REG_BYTES); >> vop = (shift_byte < REG_BYTES) ? &vop30 : >> &vop74; >> @@ -999,12 +1014,12 @@ static void svs_get_bank_volts_v3(struct svs_platform >> *svsp, struct svs_bank *sv >> } >> } >> - if (svsb->type == SVSB_TYPE_HIGH) { >> + if (bdata->type == SVSB_TYPE_HIGH) { >> opp_start = 0; >> opp_stop = svsb->turn_pt; >> - } else if (svsb->type == SVSB_TYPE_LOW) { >> + } else if (bdata->type == SVSB_TYPE_LOW) { >> opp_start = svsb->turn_pt; >> - opp_stop = svsb->opp_count; >> + opp_stop = bdata->opp_count; >> } >> for (i = opp_start; i < opp_stop; i++) >> @@ -1014,11 +1029,11 @@ static void svs_get_bank_volts_v3(struct svs_platform >> *svsp, struct svs_bank *sv >> /* For voltage bin support */ >> if (svsb->opp_dfreq[0] > svsb->freq_base) { >> svsb->volt[0] = svs_opp_volt_to_bank_volt(svsb->opp_dvolt[0], >> - svsb->volt_step, >> - svsb->volt_base); >> + bdata->volt_step, >> + bdata->volt_base); >> /* Find voltage bin turn point */ >> - for (i = 0; i < svsb->opp_count; i++) { >> + for (i = 0; i < bdata->opp_count; i++) { >> if (svsb->opp_dfreq[i] <= svsb->freq_base) { >> svsb->vbin_turn_pt = i; >> break; >> @@ -1037,12 +1052,13 @@ static void svs_get_bank_volts_v3(struct svs_platform >> *svsp, struct svs_bank *sv >> static void svs_set_bank_freq_pct_v3(struct svs_platform *svsp, struct svs_bank >> *svsb) >> { >> + const struct svs_bank_pdata *bdata = &svsb->pdata; >> u32 i, j, *freq_pct, freq_pct74 = 0, freq_pct30 = 0; >> u32 b_sft, shift_byte = 0, turn_pt; >> - u32 middle_index = (svsb->opp_count / 2); >> + u32 middle_index = (bdata->opp_count / 2); >> - for (i = 0; i < svsb->opp_count; i++) { >> - if (svsb->opp_dfreq[i] <= svsb->turn_freq_base) { >> + for (i = 0; i < bdata->opp_count; i++) { >> + if (svsb->opp_dfreq[i] <= bdata->turn_freq_base) { >> svsb->turn_pt = i; >> break; >> } >> @@ -1052,7 +1068,7 @@ static void svs_set_bank_freq_pct_v3(struct svs_platform >> *svsp, struct svs_bank >> /* Target is to fill out freq_pct74 / freq_pct30 by algorithm */ >> if (turn_pt < middle_index) { >> - if (svsb->type == SVSB_TYPE_HIGH) { >> + if (bdata->type == SVSB_TYPE_HIGH) { >> /* >> * If we don't handle this situation, >> * SVSB_TYPE_HIGH's FREQPCT74 / FREQPCT30 would keep "0" >> @@ -1069,15 +1085,15 @@ static void svs_set_bank_freq_pct_v3(struct svs_platform >> *svsp, struct svs_bank >> *freq_pct |= (svsb->freq_pct[i] << b_sft); >> shift_byte++; >> } >> - } else if (svsb->type == SVSB_TYPE_LOW) { >> + } else if (bdata->type == SVSB_TYPE_LOW) { >> /* >> * freq_pct[turn_pt] + >> * freq_pct[opp_count - 7] ~ freq_pct[opp_count -1] >> */ >> freq_pct30 = svsb->freq_pct[turn_pt]; >> shift_byte++; >> - j = svsb->opp_count - 7; >> - for (i = j; i < svsb->opp_count; i++) { >> + j = bdata->opp_count - 7; >> + for (i = j; i < bdata->opp_count; i++) { >> b_sft = BITS8 * (shift_byte % REG_BYTES); >> freq_pct = (shift_byte < REG_BYTES) ? >> &freq_pct30 : &freq_pct74; >> @@ -1086,7 +1102,7 @@ static void svs_set_bank_freq_pct_v3(struct svs_platform >> *svsp, struct svs_bank >> } >> } >> } else { >> - if (svsb->type == SVSB_TYPE_HIGH) { >> + if (bdata->type == SVSB_TYPE_HIGH) { >> /* >> * freq_pct[0] + >> * freq_pct[turn_pt - 7] ~ freq_pct[turn_pt - 1] >> @@ -1101,9 +1117,9 @@ static void svs_set_bank_freq_pct_v3(struct svs_platform >> *svsp, struct svs_bank >> *freq_pct |= (svsb->freq_pct[i] << b_sft); >> shift_byte++; >> } >> - } else if (svsb->type == SVSB_TYPE_LOW) { >> + } else if (bdata->type == SVSB_TYPE_LOW) { >> /* freq_pct[turn_pt] ~ freq_pct[opp_count - 1] */ >> - for (i = turn_pt; i < svsb->opp_count; i++) { >> + for (i = turn_pt; i < bdata->opp_count; i++) { >> b_sft = BITS8 * (shift_byte % REG_BYTES); >> freq_pct = (shift_byte < REG_BYTES) ? >> &freq_pct30 : &freq_pct74; >> @@ -1119,6 +1135,7 @@ static void svs_set_bank_freq_pct_v3(struct svs_platform >> *svsp, struct svs_bank >> static void svs_get_bank_volts_v2(struct svs_platform *svsp, struct svs_bank >> *svsb) >> { >> + const struct svs_bank_pdata *bdata = &svsb->pdata; >> u32 temp, i; >> temp = svs_readl_relaxed(svsp, VOP74); >> @@ -1146,17 +1163,17 @@ static void svs_get_bank_volts_v2(struct svs_platform >> *svsp, struct svs_bank *sv >> svsb->volt[14], >> svsb->freq_pct[15]); >> - for (i = 0; i < svsb->opp_count; i++) >> + for (i = 0; i < bdata->opp_count; i++) >> svsb->volt[i] += svsb->volt_od; >> /* For voltage bin support */ >> if (svsb->opp_dfreq[0] > svsb->freq_base) { >> svsb->volt[0] = svs_opp_volt_to_bank_volt(svsb->opp_dvolt[0], >> - svsb->volt_step, >> - svsb->volt_base); >> + bdata->volt_step, >> + bdata->volt_base); >> /* Find voltage bin turn point */ >> - for (i = 0; i < svsb->opp_count; i++) { >> + for (i = 0; i < bdata->opp_count; i++) { >> if (svsb->opp_dfreq[i] <= svsb->freq_base) { >> svsb->vbin_turn_pt = i; >> break; >> @@ -1196,6 +1213,7 @@ static void svs_set_bank_phase(struct svs_platform *svsp, >> enum svsb_phase target_phase) >> { >> struct svs_bank *svsb = &svsp->banks[bank_idx]; >> + const struct svs_bank_pdata *bdata = &svsb->pdata; >> u32 des_char, temp_char, det_char, limit_vals, init2vals, ts_calcs; >> svs_switch_bank(svsp, svsb); >> @@ -1204,7 +1222,7 @@ static void svs_set_bank_phase(struct svs_platform *svsp, >> FIELD_PREP(SVSB_DESCHAR_FLD_MDES, svsb->mdes); >> svs_writel_relaxed(svsp, des_char, DESCHAR); >> - temp_char = FIELD_PREP(SVSB_TEMPCHAR_FLD_VCO, svsb->vco) | >> + temp_char = FIELD_PREP(SVSB_TEMPCHAR_FLD_VCO, bdata->vco) | >> FIELD_PREP(SVSB_TEMPCHAR_FLD_MTDES, svsb->mtdes) | >> FIELD_PREP(SVSB_TEMPCHAR_FLD_DVT_FIXED, svsb->dvt_fixed); >> svs_writel_relaxed(svsp, temp_char, TEMPCHAR); >> @@ -1213,11 +1231,11 @@ static void svs_set_bank_phase(struct svs_platform *svsp, >> FIELD_PREP(SVSB_DETCHAR_FLD_DCMDET, svsb->dcmdet); >> svs_writel_relaxed(svsp, det_char, DETCHAR); >> - svs_writel_relaxed(svsp, svsb->dc_config, DCCONFIG); >> - svs_writel_relaxed(svsp, svsb->age_config, AGECONFIG); >> + svs_writel_relaxed(svsp, bdata->dc_config, DCCONFIG); >> + svs_writel_relaxed(svsp, bdata->age_config, AGECONFIG); >> svs_writel_relaxed(svsp, SVSB_RUNCONFIG_DEFAULT, RUNCONFIG); >> - svsb->set_freq_pct(svsp, svsb); >> + bdata->set_freq_pct(svsp, svsb); >> limit_vals = FIELD_PREP(SVSB_LIMITVALS_FLD_DTLO, SVSB_VAL_DTLO) | >> FIELD_PREP(SVSB_LIMITVALS_FLD_DTHI, SVSB_VAL_DTHI) | >> @@ -1227,13 +1245,13 @@ static void svs_set_bank_phase(struct svs_platform *svsp, >> svs_writel_relaxed(svsp, SVSB_DET_WINDOW, DETWINDOW); >> svs_writel_relaxed(svsp, SVSB_DET_MAX, CONFIG); >> - svs_writel_relaxed(svsp, svsb->chk_shift, CHKSHIFT); >> - svs_writel_relaxed(svsp, svsb->ctl0, CTL0); >> + svs_writel_relaxed(svsp, bdata->chk_shift, CHKSHIFT); >> + svs_writel_relaxed(svsp, bdata->ctl0, CTL0); >> svs_writel_relaxed(svsp, SVSB_INTSTS_VAL_CLEAN, INTSTS); >> switch (target_phase) { >> case SVSB_PHASE_INIT01: >> - svs_writel_relaxed(svsp, svsb->vboot, VBOOT); >> + svs_writel_relaxed(svsp, bdata->vboot, VBOOT); >> svs_writel_relaxed(svsp, SVSB_INTEN_INIT0x, INTEN); >> svs_writel_relaxed(svsp, SVSB_PTPEN_INIT01, SVSEN); >> break; >> @@ -1305,8 +1323,10 @@ static inline void svs_init01_isr_handler(struct >> svs_platform *svsp, >> svs_save_bank_register_data(svsp, bank_idx, SVSB_PHASE_INIT01); >> svsb->phase = SVSB_PHASE_INIT01; >> + >> val = ~(svs_readl_relaxed(svsp, DCVALUES) & GENMASK(15, 0)) + 1; >> svsb->dc_voffset_in = val & GENMASK(15, 0); >> + > Do these new blank lines have any added value ? >
Removed.
>> if (svsb->volt_flags & SVSB_INIT01_VOLT_IGNORE || >> (svsb->dc_voffset_in & SVSB_DC_SIGNED_BIT && >> svsb->volt_flags & SVSB_INIT01_VOLT_INC_ONLY)) >> @@ -1324,6 +1344,8 @@ static inline void svs_init02_isr_handler(struct >> svs_platform *svsp, >> unsigned short bank_idx) >> { >> struct svs_bank *svsb = &svsp->banks[bank_idx]; >> + const struct svs_bank_pdata *bdata = &svsb->pdata; >> + > I guess here is a bogus new line >
Fixed.
>> dev_info(svsb->dev, "%s: VOP74~30:0x%08x~0x%08x, DC:0x%08x\n", >> __func__, svs_readl_relaxed(svsp, VOP74), >> @@ -1333,7 +1355,7 @@ static inline void svs_init02_isr_handler(struct >> svs_platform *svsp, >> svs_save_bank_register_data(svsp, bank_idx, SVSB_PHASE_INIT02); >> svsb->phase = SVSB_PHASE_INIT02; >> - svsb->get_volts(svsp, svsb); >> + bdata->get_volts(svsp, svsb); >> svs_writel_relaxed(svsp, SVSB_PTPEN_OFF, SVSEN); >> svs_writel_relaxed(svsp, SVSB_INTSTS_F0_COMPLETE, INTSTS); >> @@ -1343,11 +1365,12 @@ static inline void svs_mon_mode_isr_handler(struct >> svs_platform *svsp, >> unsigned short bank_idx) >> { >> struct svs_bank *svsb = &svsp->banks[bank_idx]; >> + const struct svs_bank_pdata *bdata = &svsb->pdata; >> svs_save_bank_register_data(svsp, bank_idx, SVSB_PHASE_MON); >> svsb->phase = SVSB_PHASE_MON; >> - svsb->get_volts(svsp, svsb); >> + bdata->get_volts(svsp, svsb); >> svsb->temp = svs_readl_relaxed(svsp, TEMP) & GENMASK(7, 0); >> svs_writel_relaxed(svsp, SVSB_INTSTS_FLD_MONVOP, INTSTS); >> @@ -1356,18 +1379,20 @@ static inline void svs_mon_mode_isr_handler(struct >> svs_platform *svsp, >> static irqreturn_t svs_isr(int irq, void *data) >> { >> struct svs_platform *svsp = data; >> + const struct svs_bank_pdata *bdata = NULL; > > Is this init to NULL redundant ? >
Fixed
>> struct svs_bank *svsb = NULL; >> unsigned long flags; >> u32 idx, int_sts, svs_en; >> for (idx = 0; idx < svsp->bank_max; idx++) { >> svsb = &svsp->banks[idx]; >> + bdata = &svsb->pdata; >> WARN(!svsb, "%s: svsb(%s) is null", __func__, svsb->name); >> spin_lock_irqsave(&svs_lock, flags); >> /* Find out which svs bank fires interrupt */ >> - if (svsb->int_st & svs_readl_relaxed(svsp, INTST)) { >> + if (bdata->int_st & svs_readl_relaxed(svsp, INTST)) { >> spin_unlock_irqrestore(&svs_lock, flags); >> continue; >> } >> @@ -1412,6 +1437,7 @@ static bool svs_mode_available(struct svs_platform *svsp, >> u8 mode) >> static int svs_init01(struct svs_platform *svsp) >> { >> + const struct svs_bank_pdata *bdata; >> struct svs_bank *svsb; >> unsigned long flags, time_left; >> bool search_done; >> @@ -1427,6 +1453,7 @@ static int svs_init01(struct svs_platform *svsp) >> /* Svs bank init01 preparation - power enable */ >> for (idx = 0; idx < svsp->bank_max; idx++) { >> svsb = &svsp->banks[idx]; >> + bdata = &svsb->pdata; >> if (!(svsb->mode_support & SVSB_MODE_INIT01)) >> continue; >> @@ -1434,7 +1461,7 @@ static int svs_init01(struct svs_platform *svsp) >> ret = regulator_enable(svsb->buck); >> if (ret) { >> dev_err(svsb->dev, "%s enable fail: %d\n", >> - svsb->buck_name, ret); >> + bdata->buck_name, ret); >> goto svs_init01_resume_cpuidle; >> } >> @@ -1464,6 +1491,7 @@ static int svs_init01(struct svs_platform *svsp) >> */ >> for (idx = 0; idx < svsp->bank_max; idx++) { >> svsb = &svsp->banks[idx]; >> + bdata = &svsb->pdata; >> if (!(svsb->mode_support & SVSB_MODE_INIT01)) >> continue; >> @@ -1473,11 +1501,11 @@ static int svs_init01(struct svs_platform *svsp) >> * fix to that freq until svs_init01 is done. >> */ >> search_done = false; >> - opp_vboot = svs_bank_volt_to_opp_volt(svsb->vboot, >> - svsb->volt_step, >> - svsb->volt_base); >> + opp_vboot = svs_bank_volt_to_opp_volt(bdata->vboot, >> + bdata->volt_step, >> + bdata->volt_base); >> - for (i = 0; i < svsb->opp_count; i++) { >> + for (i = 0; i < bdata->opp_count; i++) { >> opp_freq = svsb->opp_dfreq[i]; >> if (!search_done && svsb->opp_dvolt[i] <= opp_vboot) { >> ret = dev_pm_opp_adjust_voltage(svsb->opp_dev, >> @@ -1509,13 +1537,14 @@ static int svs_init01(struct svs_platform *svsp) >> /* Svs bank init01 begins */ >> for (idx = 0; idx < svsp->bank_max; idx++) { >> svsb = &svsp->banks[idx]; >> + bdata = &svsb->pdata; >> if (!(svsb->mode_support & SVSB_MODE_INIT01)) >> continue; >> - opp_vboot = svs_bank_volt_to_opp_volt(svsb->vboot, >> - svsb->volt_step, >> - svsb->volt_base); >> + opp_vboot = svs_bank_volt_to_opp_volt(bdata->vboot, >> + bdata->volt_step, >> + bdata->volt_base); >> buck_volt = regulator_get_voltage(svsb->buck); >> if (buck_volt != opp_vboot) { >> @@ -1542,11 +1571,12 @@ static int svs_init01(struct svs_platform *svsp) >> svs_init01_finish: >> for (idx = 0; idx < svsp->bank_max; idx++) { >> svsb = &svsp->banks[idx]; >> + bdata = &svsb->pdata; >> if (!(svsb->mode_support & SVSB_MODE_INIT01)) >> continue; >> - for (i = 0; i < svsb->opp_count; i++) { >> + for (i = 0; i < bdata->opp_count; i++) { >> r = dev_pm_opp_enable(svsb->opp_dev, >> svsb->opp_dfreq[i]); >> if (r) >> @@ -1572,7 +1602,7 @@ static int svs_init01(struct svs_platform *svsp) >> r = regulator_disable(svsb->buck); >> if (r) >> dev_err(svsb->dev, "%s disable fail: %d\n", >> - svsb->buck_name, r); >> + bdata->buck_name, r); >> } >> svs_init01_resume_cpuidle: >> @@ -1583,6 +1613,7 @@ static int svs_init01(struct svs_platform *svsp) >> static int svs_init02(struct svs_platform *svsp) >> { >> + const struct svs_bank_pdata *bdata; >> struct svs_bank *svsb; >> unsigned long flags, time_left; >> int ret; >> @@ -1618,11 +1649,12 @@ static int svs_init02(struct svs_platform *svsp) >> */ >> for (idx = 0; idx < svsp->bank_max; idx++) { >> svsb = &svsp->banks[idx]; >> + bdata = &svsb->pdata; >> if (!(svsb->mode_support & SVSB_MODE_INIT02)) >> continue; >> - if (svsb->type == SVSB_TYPE_HIGH || svsb->type == SVSB_TYPE_LOW) { >> + if (bdata->type == SVSB_TYPE_HIGH || bdata->type == SVSB_TYPE_LOW) { >> if (svs_sync_bank_volts_from_opp(svsb)) { >> dev_err(svsb->dev, "sync volt fail\n"); >> ret = -EPERM; >> @@ -1680,12 +1712,12 @@ static int svs_start(struct svs_platform *svsp) >> static int svs_suspend(struct device *dev) >> { >> struct svs_platform *svsp = dev_get_drvdata(dev); >> - struct svs_bank *svsb; >> int ret; >> u32 idx; >> for (idx = 0; idx < svsp->bank_max; idx++) { >> - svsb = &svsp->banks[idx]; >> + struct svs_bank *svsb = &svsp->banks[idx]; >> + >> svs_bank_disable_and_restore_default_volts(svsp, svsb); >> } >> @@ -1736,6 +1768,7 @@ static int svs_resume(struct device *dev) >> static int svs_bank_resource_setup(struct svs_platform *svsp) >> { >> + const struct svs_bank_pdata *bdata; >> struct svs_bank *svsb; >> struct dev_pm_opp *opp; >> unsigned long freq; >> @@ -1746,8 +1779,9 @@ static int svs_bank_resource_setup(struct svs_platform *svsp) >> for (idx = 0; idx < svsp->bank_max; idx++) { >> svsb = &svsp->banks[idx]; >> + bdata = &svsb->pdata; >> - if (svsb->sw_id >= SVSB_SWID_MAX || svsb->type >= SVSB_TYPE_MAX) { >> + if (bdata->sw_id >= SVSB_SWID_MAX || bdata->type >= SVSB_TYPE_MAX) { >> dev_err(svsb->dev, "unknown bank sw_id or type\n"); >> return -EINVAL; >> } >> @@ -1757,8 +1791,8 @@ static int svs_bank_resource_setup(struct svs_platform *svsp) >> return -ENOMEM; >> svsb->name = devm_kasprintf(svsp->dev, GFP_KERNEL, "%s%s", >> - svs_swid_names[svsb->sw_id], >> - svs_type_names[svsb->type]); >> + svs_swid_names[bdata->sw_id], >> + svs_type_names[bdata->type]); >> if (!svsb->name) >> return -ENOMEM; >> @@ -1779,10 +1813,10 @@ static int svs_bank_resource_setup(struct svs_platform >> *svsp) >> if (svsb->mode_support & SVSB_MODE_INIT01) { >> svsb->buck = devm_regulator_get_optional(svsb->opp_dev, >> - svsb->buck_name); >> + bdata->buck_name); >> if (IS_ERR(svsb->buck)) { >> dev_err(svsb->dev, "cannot get \"%s-supply\"\n", >> - svsb->buck_name); >> + bdata->buck_name); >> return PTR_ERR(svsb->buck); >> } >> } >> @@ -1793,18 +1827,17 @@ static int svs_bank_resource_setup(struct svs_platform >> *svsp) >> dev_err(svsb->dev, "cannot get \"%s\" thermal zone\n", >> svsb->tzone_name); >> return PTR_ERR(svsb->tzd); >> - } > > This deleted line above is strange. Have you removed an open bracket and I missed it ? >
I have no idea how this got deleted in this patch. Fixed.
>> } >> count = dev_pm_opp_get_opp_count(svsb->opp_dev); >> - if (svsb->opp_count != count) { >> + if (bdata->opp_count != count) { >> dev_err(svsb->dev, >> "opp_count not \"%u\" but get \"%d\"?\n", >> - svsb->opp_count, count); >> + bdata->opp_count, count); >> return count; >> } >> - for (i = 0, freq = ULONG_MAX; i < svsb->opp_count; i++, freq--) { >> + for (i = 0, freq = ULONG_MAX; i < bdata->opp_count; i++, freq--) { >> opp = dev_pm_opp_find_freq_floor(svsb->opp_dev, &freq); >> if (IS_ERR(opp)) { >> dev_err(svsb->dev, "cannot find freq = %ld\n", >> @@ -1901,7 +1934,8 @@ static bool svs_common_parse_efuse(struct svs_platform *svsp, >> for (i = 0; i < svsp->bank_max; i++) { >> struct svs_bank *svsb = &svsp->banks[i]; >> - const struct svs_fusemap *dfmap = svsb->dev_fuse_map; >> + const struct svs_bank_pdata *bdata = &svsb->pdata; >> + const struct svs_fusemap *dfmap = bdata->dev_fuse_map; >> if (vmin == 1) >> svsb->vmin = 0x1e; >> @@ -1927,11 +1961,11 @@ static bool svs_mt8183_efuse_parsing(struct svs_platform >> *svsp, >> const struct svs_platform_data *pdata) >> { >> struct svs_bank *svsb; >> + const struct svs_bank_pdata *bdata; >> int format[6], x_roomt[6], o_vtsmcu[5], o_vtsabb, tb_roomt = 0; >> int adc_ge_t, adc_oe_t, ge, oe, gain, degc_cali, adc_cali_en_t; >> int o_slope, o_slope_sign, ts_id; >> u32 idx, i, ft_pgm, mts, temp0, temp1, temp2; >> - int ret; >> for (i = 0; i < svsp->efuse_max; i++) >> if (svsp->efuse[i]) >> @@ -1948,7 +1982,8 @@ static bool svs_mt8183_efuse_parsing(struct svs_platform >> *svsp, >> for (idx = 0; idx < svsp->bank_max; idx++) { >> svsb = &svsp->banks[idx]; >> - const struct svs_fusemap *dfmap = svsb->dev_fuse_map; >> + bdata = &svsb->pdata; >> + const struct svs_fusemap *dfmap = bdata->dev_fuse_map; >> if (ft_pgm <= 1) >> svsb->volt_flags |= SVSB_INIT01_VOLT_IGNORE; >> @@ -1959,7 +1994,7 @@ static bool svs_mt8183_efuse_parsing(struct svs_platform >> *svsp, >> svsb->dcbdet = svs_get_fuse_val(svsp->efuse, &dfmap[BDEV_DCBDET], 8); >> svsb->dcmdet = svs_get_fuse_val(svsp->efuse, &dfmap[BDEV_DCMDET], 8); >> - switch (svsb->sw_id) { >> + switch (bdata->sw_id) { >> case SVSB_SWID_CPU_LITTLE: >> case SVSB_SWID_CCI: >> if (ft_pgm <= 3) >> @@ -1985,6 +2020,7 @@ static bool svs_mt8183_efuse_parsing(struct svs_platform >> *svsp, >> } >> } >> + > > This line appears to be bogus >
Fixed.
>> /* Thermal efuse parsing */ >> adc_ge_t = (svsp->tefuse[1] >> 22) & GENMASK(9, 0); >> adc_oe_t = (svsp->tefuse[1] >> 12) & GENMASK(9, 0); > > > [snip]
Thanks, Angelo
| |