[PATCH v1 18/20] soc: mediatek: mtk-svs: Constify runtime-immutable members of svs_bank
Eugen Hristev
eugen.hristev at collabora.com
Fri Nov 17 06:32:34 PST 2023
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 at 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 at 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 ?
> #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 ?
> 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.
> 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
> +};
> +
> +/**
> + * 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'
> * 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 ?
> 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
> 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 ?
> 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 ?
> }
>
> 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
> /* Thermal efuse parsing */
> adc_ge_t = (svsp->tefuse[1] >> 22) & GENMASK(9, 0);
> adc_oe_t = (svsp->tefuse[1] >> 12) & GENMASK(9, 0);
[snip]
More information about the Linux-mediatek
mailing list