Re: [PATCH v10 3/7] [v10, 3/7]: soc: mediatek: SVS: introduce MTK SVS engine
On Wed, Jan 6, 2021 at 4:41 PM Roger Lu wrote:
>
> Hi Nicolas,
>
> [snip]
> > >
> > > > +
> > > > + /* Svs efuse parsing */
> > > > + ft_pgm = (svsp->efuse[0] >> 4) & GENMASK(3, 0);
> > > > +
> > > > + for (idx = 0; idx < svsp->bank_num; idx++) {
> > > > + svsb = >banks[idx];
> > > > +
> > > > + if (ft_pgm <= 1)
> > > > + svsb->init01_volt_flag =
> > > > SVSB_INIT01_VOLT_IGNORE;
> > > > +
> > > > + switch (svsb->sw_id) {
> > > > + case SVSB_CPU_LITTLE:
> > > > + svsb->bdes = svsp->efuse[16] & GENMASK(7, 0);
> > > > + svsb->mdes = (svsp->efuse[16] >> 8) &
> > > > GENMASK(7, 0);
> > > > + svsb->dcbdet = (svsp->efuse[16] >> 16) &
> > > > GENMASK(7, 0);
> > > > + svsb->dcmdet = (svsp->efuse[16] >> 24) &
> > > > GENMASK(7, 0);
> > > > + svsb->mtdes = (svsp->efuse[17] >> 16) &
> > > > GENMASK(7, 0);
> > >
> > > Again, if all of those values were u8, there'd be no need for these
> > > GENMASK
> >
> > Ok, I'll use u8 instead of GENMASK. Thanks.
>
> After refining the codes, I think it's much explicit to assign the bits
> I want by GENMASK() and will remove other GENMASK() that are repetitive
> like in svs_set_bank_phase() or svs_set_freqs_pct_v2().
I'm not sure what you mean, but, sure, you can go ahead with v11 and
we'll see how it looks.
Thanks,
> [snip]
>
Re: [PATCH v10 3/7] [v10, 3/7]: soc: mediatek: SVS: introduce MTK SVS engine
Hi Nicolas,
[snip]
> >
> > > +
> > > + /* Svs efuse parsing */
> > > + ft_pgm = (svsp->efuse[0] >> 4) & GENMASK(3, 0);
> > > +
> > > + for (idx = 0; idx < svsp->bank_num; idx++) {
> > > + svsb = >banks[idx];
> > > +
> > > + if (ft_pgm <= 1)
> > > + svsb->init01_volt_flag = SVSB_INIT01_VOLT_IGNORE;
> > > +
> > > + switch (svsb->sw_id) {
> > > + case SVSB_CPU_LITTLE:
> > > + svsb->bdes = svsp->efuse[16] & GENMASK(7, 0);
> > > + svsb->mdes = (svsp->efuse[16] >> 8) & GENMASK(7,
> > > 0);
> > > + svsb->dcbdet = (svsp->efuse[16] >> 16) &
> > > GENMASK(7, 0);
> > > + svsb->dcmdet = (svsp->efuse[16] >> 24) &
> > > GENMASK(7, 0);
> > > + svsb->mtdes = (svsp->efuse[17] >> 16) &
> > > GENMASK(7, 0);
> >
> > Again, if all of those values were u8, there'd be no need for these GENMASK
>
> Ok, I'll use u8 instead of GENMASK. Thanks.
After refining the codes, I think it's much explicit to assign the bits
I want by GENMASK() and will remove other GENMASK() that are repetitive
like in svs_set_bank_phase() or svs_set_freqs_pct_v2().
[snip]
Re: [PATCH v10 3/7] [v10, 3/7]: soc: mediatek: SVS: introduce MTK SVS engine
Hi Nicolas,
On Mon, 2021-01-04 at 17:27 +0800, Nicolas Boichat wrote:
> On Mon, Jan 4, 2021 at 4:51 PM Roger Lu wrote:
> >
> >
> > Hi Nicolas,
> >
> > Thanks for all the advices.
> >
> > On Thu, 2020-12-31 at 10:10 +0800, Nicolas Boichat wrote:
> > > On Sun, Dec 27, 2020 at 6:55 PM Roger Lu wrote:
> [snip]
> > > > +static int svs_adjust_pm_opp_volts(struct svs_bank *svsb, bool
> > > > force_update)
> > > > +{
> > > > + int tzone_temp, ret = -EPERM;
> > >
> > > No need to initialize ret.
> >
> > Oh, excuse me, some coding check tool warn that this `ret` might return
> > without being uninitialized. Therefore, I'll keep the initialization.
>
> Oh, you're right, there is a possible path where ret is not set. sgtm then.
>
> >
> > >
> > > > + u32 i, svsb_volt, opp_volt, temp_offset = 0;
> > > > +
> > > > + mutex_lock(>lock);
> > > > +
> > > > + /*
> > > > +* If svs bank is suspended, it means signed-off voltages are
> > > > applied.
> > > > +* Don't need to update opp voltage anymore.
> > > > +*/
> > > > + if (svsb->suspended && !force_update) {
> > > > + dev_notice(svsb->dev, "bank is suspended\n");
> > > > + ret = -EPERM;
> > > > + goto unlock_mutex;
> > > > + }
> > > > +
> > > > + /* Get thermal effect */
> > > > + if (svsb->phase == SVSB_PHASE_MON) {
> > > > + if (svsb->temp > svsb->temp_upper_bound &&
> > > > + svsb->temp < svsb->temp_lower_bound) {
> > > > + dev_warn(svsb->dev, "svsb temp = 0x%x?\n",
> > > > svsb->temp);
> > > > + ret = -EINVAL;
> > > > + goto unlock_mutex;
> > > > + }
> > > > +
> > > > + ret = svs_get_bank_zone_temperature(svsb->tzone_name,
> > > > + _temp);
> > > > + if (ret) {
> > > > + dev_err(svsb->dev, "no \"%s\"?(%d)?\n",
> > > > + svsb->tzone_name, ret);
> > > > + dev_err(svsb->dev, "set signed-off voltage\n");
> > >
> > > Please merge the error message in one line (I'm not sure what "set
> > > signed-off voltage" means here).
> >
> > 1. Ok, I'll merge them. Thanks.
> > 2. signed-off voltages means CPU DVFS default voltages
>
> So just write "default voltages" then? ,-)
Ok, thanks. :)
>
> >
> > >
> [snip]
> > > > +static irqreturn_t svs_isr(int irq, void *data)
> > > > +{
> > > > + struct svs_platform *svsp = (struct svs_platform *)data;
> > >
> > > cast not needed.
> >
> > Ok, I'll remove it. Thanks.
> >
> > >
> > > > + struct svs_bank *svsb = NULL;
> > > > + unsigned long flags;
> > > > + u32 idx, int_sts, svs_en;
> > > > +
> > > > + for (idx = 0; idx < svsp->bank_num; idx++) {
> > > > + svsb = >banks[idx];
> > > > +
> > > > + spin_lock_irqsave(_svs_lock, flags);
> > > > + svsp->pbank = svsb;
> > > > +
> > > > + /* Find out which svs bank fires interrupt */
> > > > + if (svsb->int_st & svs_readl(svsp, INTST)) {
> > > > + spin_unlock_irqrestore(_svs_lock, flags);
> > > > + continue;
> > > > + }
> > > > +
> > > > + if (!svsb->suspended) {
> > > > + svs_switch_bank(svsp);
> > > > + int_sts = svs_readl(svsp, INTSTS);
> > > > + svs_en = svs_readl(svsp, SVSEN);
> > > > +
> > > > + if (int_sts == SVSB_INTSTS_COMPLETE &&
> > > > + ((svs_en & SVSB_EN_MASK) == SVSB_EN_INIT01))
> > > > + svs_init01_isr_handler(svsp);
> > > > + else if ((int_sts == SVSB_INTSTS_COMPLETE) &&
> > > > +((svs_en & SVSB_EN_MASK) ==
> > > > SVSB_EN_INIT02))
> > > > + svs_init02_isr_handler(svsp);
> > > > + else if (!!(int_sts & SVSB_INTSTS_MONVOP))
> > >
> > > !! is not required.
> >
> > Ok, I'll remove it. Thanks.
> >
> > >
> > > > + svs_mon_mode_isr_handler(svsp);
> > > > + else
> > > > + svs_error_isr_handler(svsp);
> > > > + }
> > > > +
> > > > + spin_unlock_irqrestore(_svs_lock, flags);
> > > > + break;
> > > > + }
> > >
> > > This will panic if svsb is NULL, is that ok or do you want to catch that?
> >
> > Oh, it is fine. Thanks for the heads-up.
>
> I should have been stronger in my statement, I think you want to add a
> BUG_ON(!svsb) to crash in a slightly more predictable manner.
Ok, I'll add BUG_ON(!svsb) to give an evident heads-up. Thanks.
>
> [snip]
> > > > +
> > > > + svsp->tefuse = (u32 *)nvmem_cell_read(cell, >tefuse_num);
> > >
> > > Cast not needed.
> >
> > Ok,
Re: [PATCH v10 3/7] [v10, 3/7]: soc: mediatek: SVS: introduce MTK SVS engine
On Mon, Jan 4, 2021 at 4:51 PM Roger Lu wrote:
>
>
> Hi Nicolas,
>
> Thanks for all the advices.
>
> On Thu, 2020-12-31 at 10:10 +0800, Nicolas Boichat wrote:
> > On Sun, Dec 27, 2020 at 6:55 PM Roger Lu wrote:
[snip]
> > > +static int svs_adjust_pm_opp_volts(struct svs_bank *svsb, bool
> > > force_update)
> > > +{
> > > + int tzone_temp, ret = -EPERM;
> >
> > No need to initialize ret.
>
> Oh, excuse me, some coding check tool warn that this `ret` might return
> without being uninitialized. Therefore, I'll keep the initialization.
Oh, you're right, there is a possible path where ret is not set. sgtm then.
>
> >
> > > + u32 i, svsb_volt, opp_volt, temp_offset = 0;
> > > +
> > > + mutex_lock(>lock);
> > > +
> > > + /*
> > > +* If svs bank is suspended, it means signed-off voltages are
> > > applied.
> > > +* Don't need to update opp voltage anymore.
> > > +*/
> > > + if (svsb->suspended && !force_update) {
> > > + dev_notice(svsb->dev, "bank is suspended\n");
> > > + ret = -EPERM;
> > > + goto unlock_mutex;
> > > + }
> > > +
> > > + /* Get thermal effect */
> > > + if (svsb->phase == SVSB_PHASE_MON) {
> > > + if (svsb->temp > svsb->temp_upper_bound &&
> > > + svsb->temp < svsb->temp_lower_bound) {
> > > + dev_warn(svsb->dev, "svsb temp = 0x%x?\n",
> > > svsb->temp);
> > > + ret = -EINVAL;
> > > + goto unlock_mutex;
> > > + }
> > > +
> > > + ret = svs_get_bank_zone_temperature(svsb->tzone_name,
> > > + _temp);
> > > + if (ret) {
> > > + dev_err(svsb->dev, "no \"%s\"?(%d)?\n",
> > > + svsb->tzone_name, ret);
> > > + dev_err(svsb->dev, "set signed-off voltage\n");
> >
> > Please merge the error message in one line (I'm not sure what "set
> > signed-off voltage" means here).
>
> 1. Ok, I'll merge them. Thanks.
> 2. signed-off voltages means CPU DVFS default voltages
So just write "default voltages" then? ,-)
>
> >
[snip]
> > > +static irqreturn_t svs_isr(int irq, void *data)
> > > +{
> > > + struct svs_platform *svsp = (struct svs_platform *)data;
> >
> > cast not needed.
>
> Ok, I'll remove it. Thanks.
>
> >
> > > + struct svs_bank *svsb = NULL;
> > > + unsigned long flags;
> > > + u32 idx, int_sts, svs_en;
> > > +
> > > + for (idx = 0; idx < svsp->bank_num; idx++) {
> > > + svsb = >banks[idx];
> > > +
> > > + spin_lock_irqsave(_svs_lock, flags);
> > > + svsp->pbank = svsb;
> > > +
> > > + /* Find out which svs bank fires interrupt */
> > > + if (svsb->int_st & svs_readl(svsp, INTST)) {
> > > + spin_unlock_irqrestore(_svs_lock, flags);
> > > + continue;
> > > + }
> > > +
> > > + if (!svsb->suspended) {
> > > + svs_switch_bank(svsp);
> > > + int_sts = svs_readl(svsp, INTSTS);
> > > + svs_en = svs_readl(svsp, SVSEN);
> > > +
> > > + if (int_sts == SVSB_INTSTS_COMPLETE &&
> > > + ((svs_en & SVSB_EN_MASK) == SVSB_EN_INIT01))
> > > + svs_init01_isr_handler(svsp);
> > > + else if ((int_sts == SVSB_INTSTS_COMPLETE) &&
> > > +((svs_en & SVSB_EN_MASK) ==
> > > SVSB_EN_INIT02))
> > > + svs_init02_isr_handler(svsp);
> > > + else if (!!(int_sts & SVSB_INTSTS_MONVOP))
> >
> > !! is not required.
>
> Ok, I'll remove it. Thanks.
>
> >
> > > + svs_mon_mode_isr_handler(svsp);
> > > + else
> > > + svs_error_isr_handler(svsp);
> > > + }
> > > +
> > > + spin_unlock_irqrestore(_svs_lock, flags);
> > > + break;
> > > + }
> >
> > This will panic if svsb is NULL, is that ok or do you want to catch that?
>
> Oh, it is fine. Thanks for the heads-up.
I should have been stronger in my statement, I think you want to add a
BUG_ON(!svsb) to crash in a slightly more predictable manner.
[snip]
> > > +
> > > + svsp->tefuse = (u32 *)nvmem_cell_read(cell, >tefuse_num);
> >
> > Cast not needed.
>
> Ok, I'll remove it if build/test ok. Because nvmem_cell_read returns
> (void *).
>
> >
> > Also, this need to be freed somewhere in remove code (kfree(svsp->tefuse)).
> >
> > And it seems like svsp->tefuse is only used in this function, can you
> > just allocate it here?
>
> Oh, svsp->tefuse will be used in SVS debug patch for debug purpose. So,
> I need to save it as struct member.
Oh I missed that, sgtm then. Thanks.
Re: [PATCH v10 3/7] [v10, 3/7]: soc: mediatek: SVS: introduce MTK SVS engine
Hi Nicolas, Thanks for all the advices. On Thu, 2020-12-31 at 10:10 +0800, Nicolas Boichat wrote: > On Sun, Dec 27, 2020 at 6:55 PM Roger Lu wrote: > > > > The Smart Voltage Scaling(SVS) engine is a piece of hardware > > which calculats suitable SVS bank voltages to OPP voltage table. > > calculates Ok, I'll fix it. Thanks. > > > Then, DVFS driver could apply those SVS bank voltages to PMIC/Buck > > when receiving OPP_EVENT_ADJUST_VOLTAGE. > > > > Signed-off-by: Roger Lu > > --- > > drivers/soc/mediatek/Kconfig | 10 + > > drivers/soc/mediatek/Makefile |1 + > > drivers/soc/mediatek/mtk-svs.c | 1748 > > 3 files changed, 1759 insertions(+) > > create mode 100644 drivers/soc/mediatek/mtk-svs.c > > > > diff --git a/drivers/soc/mediatek/Kconfig b/drivers/soc/mediatek/Kconfig > > index 59a56cd790ec..f33da25f8336 100644 > > --- a/drivers/soc/mediatek/Kconfig > > +++ b/drivers/soc/mediatek/Kconfig > > @@ -51,4 +51,14 @@ config MTK_MMSYS > > Say yes here to add support for the MediaTek Multimedia > > Subsystem (MMSYS). > > > > +config MTK_SVS > > + tristate "MediaTek Smart Voltage Scaling(SVS)" > > + depends on MTK_EFUSE && NVMEM > > + help > > + The Smart Voltage Scaling(SVS) engine is a piece of hardware > > + which has several controllers(banks) for calculating suitable > > + voltage to different power domains(CPU/GPU/CCI) according to > > + chip process corner, temperatures and other factors. Then DVFS > > + driver could apply SVS bank voltage to PMIC/Buck. > > + > > endmenu > > diff --git a/drivers/soc/mediatek/Makefile b/drivers/soc/mediatek/Makefile > > index 01f9f873634a..e10f2cd87514 100644 > > --- a/drivers/soc/mediatek/Makefile > > +++ b/drivers/soc/mediatek/Makefile > > @@ -4,3 +4,4 @@ obj-$(CONFIG_MTK_INFRACFG) += mtk-infracfg.o > > o-$(CONFIG_MTK_PMIC_WRAP) += mtk-pmic-wrap.o > > obj-$(CONFIG_MTK_SCPSYS) += mtk-scpsys.o > > obj-$(CONFIG_MTK_MMSYS) += mtk-mmsys.o > > +obj-$(CONFIG_MTK_SVS) += mtk-svs.o > > diff --git a/drivers/soc/mediatek/mtk-svs.c b/drivers/soc/mediatek/mtk-svs.c > > new file mode 100644 > > index ..0efefb48839d > > --- /dev/null > > +++ b/drivers/soc/mediatek/mtk-svs.c > > @@ -0,0 +1,1748 @@ > > +// SPDX-License-Identifier: GPL-2.0-only > > +/* > > + * Copyright (C) 2020 MediaTek Inc. > > + */ > > + > > +#include > > +#include > > +#include > > +#include > > +#include > > +#include > > +#include > > +#include > > +#include > > +#include > > +#include > > +#include > > +#include > > +#include > > +#include > > +#include > > +#include > > +#include > > +#include > > +#include > > +#include > > +#include > > +#include > > +#include > > + > > +/* svs bank 1-line sw id */ > > +#define SVSB_CPU_LITTLEBIT(0) > > +#define SVSB_CPU_BIG BIT(1) > > +#define SVSB_CCI BIT(2) > > +#define SVSB_GPU BIT(3) > > + > > +/* svs bank mode support */ > > +#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 init01 condition */ > > +#define SVSB_INIT01_VOLT_IGNOREBIT(1) > > +#define SVSB_INIT01_VOLT_INC_ONLY BIT(2) > > +#define SVSB_INIT01_CLK_EN BIT(31) > > + > > +/* svs bank common setting */ > > +#define SVSB_TZONE_HIGH_TEMP_MAX U32_MAX > > +#define SVSB_RUNCONFIG_DEFAULT 0x8000 > > +#define SVSB_DC_SIGNED_BIT 0x8000 > > +#define SVSB_INTEN_INIT0x 0x5f01 > > +#define SVSB_INTEN_MONVOPEN0x00ff > > +#define SVSB_EN_OFF0x0 > > +#define SVSB_EN_MASK 0x7 > > +#define SVSB_EN_INIT01 0x1 > > +#define SVSB_EN_INIT02 0x5 > > +#define SVSB_EN_MON0x2 > > +#define SVSB_INTSTS_MONVOP 0x00ff > > +#define SVSB_INTSTS_COMPLETE 0x1 > > +#define SVSB_INTSTS_CLEAN 0x00ff > > + > > +static DEFINE_SPINLOCK(mtk_svs_lock); > > + > > +/* > > + * enum svsb_phase - svs bank phase enumeration > > + * @SVSB_PHASE_INIT01: basic init for svs bank > > + * @SVSB_PHASE_INIT02: svs bank can provide voltages > > + * @SVSB_PHASE_MON: svs bank can provide voltages with thermal effect > > + * @SVSB_PHASE_ERROR: svs bank encouters unexpected condition > > encounters Ok, I'll fix it. Thanks. > > > + * > > + * Each svs bank has its own independent phase. We enable each svs bank by > > + * running their phase orderly. However, When svs bank ecnounters > > unexpected > > encounters Ok, I'll fix it. Thanks. > > > + * condition, it will fire an irq (PHASE_ERROR) to inform svs software. > > + * > > + * svs bank general phase-enabled order: > > + * SVSB_PHASE_INIT01 -> SVSB_PHASE_INIT02 ->
Re: [PATCH v10 3/7] [v10, 3/7]: soc: mediatek: SVS: introduce MTK SVS engine
On Sun, Dec 27, 2020 at 6:55 PM Roger Lu wrote:
>
> The Smart Voltage Scaling(SVS) engine is a piece of hardware
> which calculats suitable SVS bank voltages to OPP voltage table.
calculates
> Then, DVFS driver could apply those SVS bank voltages to PMIC/Buck
> when receiving OPP_EVENT_ADJUST_VOLTAGE.
>
> Signed-off-by: Roger Lu
> ---
> drivers/soc/mediatek/Kconfig | 10 +
> drivers/soc/mediatek/Makefile |1 +
> drivers/soc/mediatek/mtk-svs.c | 1748
> 3 files changed, 1759 insertions(+)
> create mode 100644 drivers/soc/mediatek/mtk-svs.c
>
> diff --git a/drivers/soc/mediatek/Kconfig b/drivers/soc/mediatek/Kconfig
> index 59a56cd790ec..f33da25f8336 100644
> --- a/drivers/soc/mediatek/Kconfig
> +++ b/drivers/soc/mediatek/Kconfig
> @@ -51,4 +51,14 @@ config MTK_MMSYS
> Say yes here to add support for the MediaTek Multimedia
> Subsystem (MMSYS).
>
> +config MTK_SVS
> + tristate "MediaTek Smart Voltage Scaling(SVS)"
> + depends on MTK_EFUSE && NVMEM
> + help
> + The Smart Voltage Scaling(SVS) engine is a piece of hardware
> + which has several controllers(banks) for calculating suitable
> + voltage to different power domains(CPU/GPU/CCI) according to
> + chip process corner, temperatures and other factors. Then DVFS
> + driver could apply SVS bank voltage to PMIC/Buck.
> +
> endmenu
> diff --git a/drivers/soc/mediatek/Makefile b/drivers/soc/mediatek/Makefile
> index 01f9f873634a..e10f2cd87514 100644
> --- a/drivers/soc/mediatek/Makefile
> +++ b/drivers/soc/mediatek/Makefile
> @@ -4,3 +4,4 @@ obj-$(CONFIG_MTK_INFRACFG) += mtk-infracfg.o
> obj-$(CONFIG_MTK_PMIC_WRAP) += mtk-pmic-wrap.o
> obj-$(CONFIG_MTK_SCPSYS) += mtk-scpsys.o
> obj-$(CONFIG_MTK_MMSYS) += mtk-mmsys.o
> +obj-$(CONFIG_MTK_SVS) += mtk-svs.o
> diff --git a/drivers/soc/mediatek/mtk-svs.c b/drivers/soc/mediatek/mtk-svs.c
> new file mode 100644
> index ..0efefb48839d
> --- /dev/null
> +++ b/drivers/soc/mediatek/mtk-svs.c
> @@ -0,0 +1,1748 @@
> +// SPDX-License-Identifier: GPL-2.0-only
> +/*
> + * Copyright (C) 2020 MediaTek Inc.
> + */
> +
> +#include
> +#include
> +#include
> +#include
> +#include
> +#include
> +#include
> +#include
> +#include
> +#include
> +#include
> +#include
> +#include
> +#include
> +#include
> +#include
> +#include
> +#include
> +#include
> +#include
> +#include
> +#include
> +#include
> +#include
> +
> +/* svs bank 1-line sw id */
> +#define SVSB_CPU_LITTLEBIT(0)
> +#define SVSB_CPU_BIG BIT(1)
> +#define SVSB_CCI BIT(2)
> +#define SVSB_GPU BIT(3)
> +
> +/* svs bank mode support */
> +#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 init01 condition */
> +#define SVSB_INIT01_VOLT_IGNOREBIT(1)
> +#define SVSB_INIT01_VOLT_INC_ONLY BIT(2)
> +#define SVSB_INIT01_CLK_EN BIT(31)
> +
> +/* svs bank common setting */
> +#define SVSB_TZONE_HIGH_TEMP_MAX U32_MAX
> +#define SVSB_RUNCONFIG_DEFAULT 0x8000
> +#define SVSB_DC_SIGNED_BIT 0x8000
> +#define SVSB_INTEN_INIT0x 0x5f01
> +#define SVSB_INTEN_MONVOPEN0x00ff
> +#define SVSB_EN_OFF0x0
> +#define SVSB_EN_MASK 0x7
> +#define SVSB_EN_INIT01 0x1
> +#define SVSB_EN_INIT02 0x5
> +#define SVSB_EN_MON0x2
> +#define SVSB_INTSTS_MONVOP 0x00ff
> +#define SVSB_INTSTS_COMPLETE 0x1
> +#define SVSB_INTSTS_CLEAN 0x00ff
> +
> +static DEFINE_SPINLOCK(mtk_svs_lock);
> +
> +/*
> + * enum svsb_phase - svs bank phase enumeration
> + * @SVSB_PHASE_INIT01: basic init for svs bank
> + * @SVSB_PHASE_INIT02: svs bank can provide voltages
> + * @SVSB_PHASE_MON: svs bank can provide voltages with thermal effect
> + * @SVSB_PHASE_ERROR: svs bank encouters unexpected condition
encounters
> + *
> + * Each svs bank has its own independent phase. We enable each svs bank by
> + * running their phase orderly. However, When svs bank ecnounters unexpected
encounters
> + * 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_MON
> + */
> +enum svsb_phase {
> + SVSB_PHASE_INIT01 = 0,
> + SVSB_PHASE_INIT02,
> + SVSB_PHASE_MON,
> + SVSB_PHASE_ERROR,
> +};
> +
> +enum svs_reg_index {
> + DESCHAR = 0,
> + TEMPCHAR,
> + DETCHAR,
> + AGECHAR,
> + DCCONFIG,
> + AGECONFIG,
> + FREQPCT30,
> + FREQPCT74,
> + LIMITVALS,
> + VBOOT,
> + DETWINDOW,
> + CONFIG,
> + TSCALCS,
> +
