| lists.openwall.net | lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC | |
|
Open Source and information security mailing list archives
| ||
|
Message-ID: <d78efef4-196a-457a-bf97-39d471dd50fd@collabora.com>
Date: Tue, 21 Nov 2023 12:53:11 +0100
From: AngeloGioacchino Del Regno
<angelogioacchino.delregno@...labora.com>
To: Eugen Hristev <eugen.hristev@...labora.com>, matthias.bgg@...il.com
Cc: krzysztof.kozlowski+dt@...aro.org, conor+dt@...nel.org,
robh+dt@...nel.org, p.zabel@...gutronix.de,
devicetree@...r.kernel.org, linux-kernel@...r.kernel.org,
linux-arm-kernel@...ts.infradead.org,
linux-mediatek@...ts.infradead.org, kernel@...labora.com,
wenst@...omium.org
Subject: Re: [PATCH v1 18/20] soc: mediatek: mtk-svs: Constify
runtime-immutable members of svs_bank
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@...labora.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@...labora.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
Powered by blists - more mailing lists