| Message ID | 20230111074528.29354-14-roger.lu@mediatek.com |
|---|---|
| State | Superseded |
| Headers | show |
| Series | Enahance SVS's robustness | expand |
On 11/01/2023 08:45, Roger Lu wrote: > The timing of disabling SVS bank and restore default voltage is more > than one place. Therefore, add a common function to use for removing > the superfluous codes. > > Signed-off-by: Roger Lu <roger.lu@mediatek.com> > Reviewed-by: AngeloGioacchino Del Regno <angelogioacchino.delregno@collabora.com> Same here, change looks good. Could you please rebase and resend: Reviewed-by: Matthias Brugger <matthias.bgg@gmail.com> > --- > drivers/soc/mediatek/mtk-svs.c | 54 ++++++++++++++-------------------- > 1 file changed, 22 insertions(+), 32 deletions(-) > > diff --git a/drivers/soc/mediatek/mtk-svs.c b/drivers/soc/mediatek/mtk-svs.c > index a7f0a6f02d52..89117807e85d 100644 > --- a/drivers/soc/mediatek/mtk-svs.c > +++ b/drivers/soc/mediatek/mtk-svs.c > @@ -648,6 +648,25 @@ static int svs_adjust_pm_opp_volts(struct svs_bank *svsb) > return ret; > } > > +static void svs_bank_disable_and_restore_default_volts(struct svs_platform *svsp, > + struct svs_bank *svsb) > +{ > + unsigned long flags; > + > + if (svsb->mode_support == SVSB_MODE_ALL_DISABLE) > + return; > + > + spin_lock_irqsave(&svs_lock, flags); > + svsp->pbank = svsb; > + svs_switch_bank(svsp); > + svs_writel_relaxed(svsp, SVSB_PTPEN_OFF, SVSEN); > + svs_writel_relaxed(svsp, SVSB_INTSTS_VAL_CLEAN, INTSTS); > + spin_unlock_irqrestore(&svs_lock, flags); > + > + svsb->phase = SVSB_PHASE_ERROR; > + svs_adjust_pm_opp_volts(svsb); > +} > + > #ifdef CONFIG_DEBUG_FS > static int svs_dump_debug_show(struct seq_file *m, void *p) > { > @@ -724,7 +743,6 @@ static ssize_t svs_enable_debug_write(struct file *filp, > { > struct svs_bank *svsb = file_inode(filp)->i_private; > struct svs_platform *svsp = dev_get_drvdata(svsb->dev); > - unsigned long flags; > int enabled, ret; > char *buf = NULL; > > @@ -740,16 +758,8 @@ static ssize_t svs_enable_debug_write(struct file *filp, > return ret; > > if (!enabled) { > - spin_lock_irqsave(&svs_lock, flags); > - svsp->pbank = svsb; > + svs_bank_disable_and_restore_default_volts(svsp, svsb); > svsb->mode_support = SVSB_MODE_ALL_DISABLE; > - svs_switch_bank(svsp); > - svs_writel_relaxed(svsp, SVSB_PTPEN_OFF, SVSEN); > - svs_writel_relaxed(svsp, SVSB_INTSTS_VAL_CLEAN, INTSTS); > - spin_unlock_irqrestore(&svs_lock, flags); > - > - svsb->phase = SVSB_PHASE_ERROR; > - svs_adjust_pm_opp_volts(svsb); > } > > kfree(buf); > @@ -1532,16 +1542,7 @@ static int svs_init02(struct svs_platform *svsp) > out_of_init02: > for (idx = 0; idx < svsp->bank_max; idx++) { > svsb = &svsp->banks[idx]; > - > - spin_lock_irqsave(&svs_lock, flags); > - svsp->pbank = svsb; > - svs_switch_bank(svsp); > - svs_writel_relaxed(svsp, SVSB_PTPEN_OFF, SVSEN); > - svs_writel_relaxed(svsp, SVSB_INTSTS_VAL_CLEAN, INTSTS); > - spin_unlock_irqrestore(&svs_lock, flags); > - > - svsb->phase = SVSB_PHASE_ERROR; > - svs_adjust_pm_opp_volts(svsb); > + svs_bank_disable_and_restore_default_volts(svsp, svsb); > } > > return ret; > @@ -1587,7 +1588,6 @@ static int svs_suspend(struct device *dev) > { > struct svs_platform *svsp = dev_get_drvdata(dev); > struct svs_bank *svsb; > - unsigned long flags; > int ret; > u32 idx; > > @@ -1599,17 +1599,7 @@ static int svs_suspend(struct device *dev) > > for (idx = 0; idx < svsp->bank_max; idx++) { > svsb = &svsp->banks[idx]; > - > - /* This might wait for svs_isr() process */ > - spin_lock_irqsave(&svs_lock, flags); > - svsp->pbank = svsb; > - svs_switch_bank(svsp); > - svs_writel_relaxed(svsp, SVSB_PTPEN_OFF, SVSEN); > - svs_writel_relaxed(svsp, SVSB_INTSTS_VAL_CLEAN, INTSTS); > - spin_unlock_irqrestore(&svs_lock, flags); > - > - svsb->phase = SVSB_PHASE_ERROR; > - svs_adjust_pm_opp_volts(svsb); > + svs_bank_disable_and_restore_default_volts(svsp, svsb); > } > > ret = reset_control_assert(svsp->rst);
Hi Matthias Sir, On Tue, 2023-01-31 at 14:40 +0100, Matthias Brugger wrote: > > On 11/01/2023 08:45, Roger Lu wrote: > > The timing of disabling SVS bank and restore default voltage is more > > than one place. Therefore, add a common function to use for removing > > the superfluous codes. > > > > Signed-off-by: Roger Lu <roger.lu@mediatek.com> > > Reviewed-by: AngeloGioacchino Del Regno < > > angelogioacchino.delregno@collabora.com> > > Same here, change looks good. Could you please rebase and resend: No Problem, I'll rebase and resend it. Thanks. > > Reviewed-by: Matthias Brugger <matthias.bgg@gmail.com> ... [snip] ...
diff --git a/drivers/soc/mediatek/mtk-svs.c b/drivers/soc/mediatek/mtk-svs.c index a7f0a6f02d52..89117807e85d 100644 --- a/drivers/soc/mediatek/mtk-svs.c +++ b/drivers/soc/mediatek/mtk-svs.c @@ -648,6 +648,25 @@ static int svs_adjust_pm_opp_volts(struct svs_bank *svsb) return ret; } +static void svs_bank_disable_and_restore_default_volts(struct svs_platform *svsp, + struct svs_bank *svsb) +{ + unsigned long flags; + + if (svsb->mode_support == SVSB_MODE_ALL_DISABLE) + return; + + spin_lock_irqsave(&svs_lock, flags); + svsp->pbank = svsb; + svs_switch_bank(svsp); + svs_writel_relaxed(svsp, SVSB_PTPEN_OFF, SVSEN); + svs_writel_relaxed(svsp, SVSB_INTSTS_VAL_CLEAN, INTSTS); + spin_unlock_irqrestore(&svs_lock, flags); + + svsb->phase = SVSB_PHASE_ERROR; + svs_adjust_pm_opp_volts(svsb); +} + #ifdef CONFIG_DEBUG_FS static int svs_dump_debug_show(struct seq_file *m, void *p) { @@ -724,7 +743,6 @@ static ssize_t svs_enable_debug_write(struct file *filp, { struct svs_bank *svsb = file_inode(filp)->i_private; struct svs_platform *svsp = dev_get_drvdata(svsb->dev); - unsigned long flags; int enabled, ret; char *buf = NULL; @@ -740,16 +758,8 @@ static ssize_t svs_enable_debug_write(struct file *filp, return ret; if (!enabled) { - spin_lock_irqsave(&svs_lock, flags); - svsp->pbank = svsb; + svs_bank_disable_and_restore_default_volts(svsp, svsb); svsb->mode_support = SVSB_MODE_ALL_DISABLE; - svs_switch_bank(svsp); - svs_writel_relaxed(svsp, SVSB_PTPEN_OFF, SVSEN); - svs_writel_relaxed(svsp, SVSB_INTSTS_VAL_CLEAN, INTSTS); - spin_unlock_irqrestore(&svs_lock, flags); - - svsb->phase = SVSB_PHASE_ERROR; - svs_adjust_pm_opp_volts(svsb); } kfree(buf); @@ -1532,16 +1542,7 @@ static int svs_init02(struct svs_platform *svsp) out_of_init02: for (idx = 0; idx < svsp->bank_max; idx++) { svsb = &svsp->banks[idx]; - - spin_lock_irqsave(&svs_lock, flags); - svsp->pbank = svsb; - svs_switch_bank(svsp); - svs_writel_relaxed(svsp, SVSB_PTPEN_OFF, SVSEN); - svs_writel_relaxed(svsp, SVSB_INTSTS_VAL_CLEAN, INTSTS); - spin_unlock_irqrestore(&svs_lock, flags); - - svsb->phase = SVSB_PHASE_ERROR; - svs_adjust_pm_opp_volts(svsb); + svs_bank_disable_and_restore_default_volts(svsp, svsb); } return ret; @@ -1587,7 +1588,6 @@ static int svs_suspend(struct device *dev) { struct svs_platform *svsp = dev_get_drvdata(dev); struct svs_bank *svsb; - unsigned long flags; int ret; u32 idx; @@ -1599,17 +1599,7 @@ static int svs_suspend(struct device *dev) for (idx = 0; idx < svsp->bank_max; idx++) { svsb = &svsp->banks[idx]; - - /* This might wait for svs_isr() process */ - spin_lock_irqsave(&svs_lock, flags); - svsp->pbank = svsb; - svs_switch_bank(svsp); - svs_writel_relaxed(svsp, SVSB_PTPEN_OFF, SVSEN); - svs_writel_relaxed(svsp, SVSB_INTSTS_VAL_CLEAN, INTSTS); - spin_unlock_irqrestore(&svs_lock, flags); - - svsb->phase = SVSB_PHASE_ERROR; - svs_adjust_pm_opp_volts(svsb); + svs_bank_disable_and_restore_default_volts(svsp, svsb); } ret = reset_control_assert(svsp->rst);