[PATCH v21 4/8] soc: mediatek: SVS: add monitor mode
AngeloGioacchino Del Regno
angelogioacchino.delregno at collabora.com
Fri Jan 7 06:34:03 PST 2022
Il 07/01/22 10:51, Roger Lu ha scritto:
> SVS monitor mode is based on different thermal temperature
> to provide suitable SVS bank voltages.
>
> Signed-off-by: Roger Lu <roger.lu at mediatek.com>
> ---
> drivers/soc/mediatek/mtk-svs.c | 253 ++++++++++++++++++++++++++++++++-
> 1 file changed, 247 insertions(+), 6 deletions(-)
>
> diff --git a/drivers/soc/mediatek/mtk-svs.c b/drivers/soc/mediatek/mtk-svs.c
> index fc7e2ee44a92..042c6e8e9069 100644
> --- a/drivers/soc/mediatek/mtk-svs.c
> +++ b/drivers/soc/mediatek/mtk-svs.c
> @@ -25,6 +25,7 @@
> #include <linux/regulator/consumer.h>
> #include <linux/slab.h>
> #include <linux/spinlock.h>
> +#include <linux/thermal.h>
>
> /* svs bank 1-line sw id */
> #define SVSB_CPU_LITTLE BIT(0)
> @@ -36,6 +37,7 @@
> #define SVSB_MODE_ALL_DISABLE 0
> #define SVSB_MODE_INIT01 BIT(1)
> #define SVSB_MODE_INIT02 BIT(2)
> +#define SVSB_MODE_MON BIT(3)
>
> /* svs bank volt flags */
> #define SVSB_INIT01_PD_REQ BIT(0)
> @@ -53,11 +55,16 @@
> #define SVSB_EN_INIT01 0x1
> #define SVSB_EN_INIT02 0x5
> #define SVSB_EN_MASK 0x7
> +#define SVSB_EN_MON 0x2
> #define SVSB_EN_OFF 0x0
> #define SVSB_INTEN_INIT0x 0x00005f01
> +#define SVSB_INTEN_MONVOPEN 0x00ff0000
> #define SVSB_INTSTS_CLEAN 0x00ffffff
> #define SVSB_INTSTS_COMPLETE 0x1
> +#define SVSB_INTSTS_MONVOP 0x00ff0000
> #define SVSB_RUNCONFIG_DEFAULT 0x80000000
> +#define SVSB_TEMP_LOWER_BOUND 0xb2
> +#define SVSB_TEMP_UPPER_BOUND 0x64
>
> static DEFINE_SPINLOCK(svs_lock);
>
> @@ -66,6 +73,7 @@ static DEFINE_SPINLOCK(svs_lock);
> * @SVSB_PHASE_ERROR: svs bank encounters unexpected condition
> * @SVSB_PHASE_INIT01: svs bank basic init for data calibration
> * @SVSB_PHASE_INIT02: svs bank can provide voltages to opp table
> + * @SVSB_PHASE_MON: svs bank can provide voltages with thermal effect
> * @SVSB_PHASE_MAX: total number of svs bank phase (debug purpose)
> *
> * Each svs bank has its own independent phase and we enable each svs bank by
> @@ -73,12 +81,13 @@ static DEFINE_SPINLOCK(svs_lock);
> * condition, it will fire an irq (PHASE_ERROR) to inform svs software.
> *
> * svs bank general phase-enabled order:
> - * SVSB_PHASE_INIT01 -> SVSB_PHASE_INIT02
> + * SVSB_PHASE_INIT01 -> SVSB_PHASE_INIT02 -> SVSB_PHASE_MON
> */
> enum svsb_phase {
> SVSB_PHASE_ERROR = 0,
> SVSB_PHASE_INIT01,
> SVSB_PHASE_INIT02,
> + SVSB_PHASE_MON,
> SVSB_PHASE_MAX,
> };
>
> @@ -209,9 +218,11 @@ static const u32 svs_regs_v2[] = {
> * @probe: svs platform probe function pointer
> * @irqflags: svs platform irq settings flags
> * @efuse_max: total number of svs efuse
> + * @tefuse_max: total number of thermal efuse
> * @regs: svs platform registers map
> * @bank_max: total number of svs banks
> * @efuse: svs efuse data received from NVMEM framework
> + * @tefuse: thermal efuse data received from NVMEM framework
> */
> struct svs_platform {
> char *name;
> @@ -224,9 +235,11 @@ struct svs_platform {
> int (*probe)(struct svs_platform *svsp);
> unsigned long irqflags;
> size_t efuse_max;
> + size_t tefuse_max;
> const u32 *regs;
> u32 bank_max;
> u32 *efuse;
> + u32 *tefuse;
> };
>
> /**
> @@ -241,6 +254,7 @@ struct svs_platform {
> * @get_volts: function pointer to get bank voltages
> * @name: bank name
> * @buck_name: regulator name
> + * @tzone_name: thermal zone name
> * @suspended: suspend flag of this bank
> * @phase: bank current phase
> * @volt_od: bank voltage overdrive
> @@ -270,6 +284,13 @@ struct svs_platform {
> * @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
> * @mdes: svs efuse data
> * @mtdes: svs efuse data
> @@ -292,6 +313,7 @@ struct svs_bank {
> void (*get_volts)(struct svs_platform *svsp);
> char *name;
> char *buck_name;
> + char *tzone_name;
> bool suspended;
> enum svsb_phase phase;
> s32 volt_od;
> @@ -321,6 +343,13 @@ struct svs_bank {
> u32 sw_id;
> u32 cpu_id;
> u32 ctl0;
> + u32 temp;
> + u32 tzone_htemp;
> + u32 tzone_htemp_voffset;
> + u32 tzone_ltemp;
> + u32 tzone_ltemp_voffset;
> + u32 bts;
> + u32 mts;
> u32 bdes;
> u32 mdes;
> u32 mtdes;
> @@ -361,10 +390,21 @@ static u32 svs_bank_volt_to_opp_volt(u32 svsb_volt, u32 svsb_volt_step,
> return (svsb_volt * svsb_volt_step) + svsb_volt_base;
> }
>
I'm sorry for the double review, but this went unnoticed in the previous one.
> +static int svs_get_zone_temperature(const char *tzone_name, int *tzone_temp)
> +{
> + struct thermal_zone_device *tzd;
> +
> + tzd = thermal_zone_get_zone_by_name(tzone_name);
This call is expensive, as it's iterating through the (possibly) entire
thermal_tz_list (drivers/thermal/thermal_core.c) so, for performance purposes,
noting that you're using this in svs_adjust_pm_opp_volts(), it's not a good idea
to call it at every ISR.
I would instead propose to get a pointer to the thermal_zone at driver probe
time and cache that in struct svs_bank: this function would also be removed
as the only thing that you'd need to do then would be just one call...
[read forward...]
> + if (IS_ERR(tzd))
> + return PTR_ERR(tzd);
> +
> + return thermal_zone_get_temp(tzd, tzone_temp);
> +}
> +
> static int svs_adjust_pm_opp_volts(struct svs_bank *svsb, bool force_update)
> {
> - int ret = -EPERM;
> - u32 i, svsb_volt, opp_volt;
> + int ret = -EPERM, tzone_temp = 0;
> + u32 i, svsb_volt, opp_volt, temp_voffset = 0;
>
> mutex_lock(&svsb->lock);
>
> @@ -378,6 +418,22 @@ static int svs_adjust_pm_opp_volts(struct svs_bank *svsb, bool force_update)
> goto unlock_mutex;
> }
>
> + /* Get thermal effect */
> + if (svsb->phase == SVSB_PHASE_MON) {
> + ret = svs_get_zone_temperature(svsb->tzone_name, &tzone_temp);
... so you can simply call ...
ret = thermal_zone_get_temp(svsb->tzd, tzone_temp);
...without any need for any helper.
> + 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);
> + 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;
> + }
> +
> /* vmin <= svsb_volt (opp_volt) <= default opp voltage */
> for (i = 0; i < svsb->opp_count; i++) {
> switch (svsb->phase) {
Apart from that, the commit looks good. Looking forward to review the new version!
Regards,
- Angelo
More information about the Linux-mediatek
mailing list