[RFC,3/8] cpufreq: imx6q: Set max suspend_freq to avoid changes during suspend
diff mbox

Message ID f2126423e233757eb113f115e0be562ea0902a7a.1490199005.git.leonard.crestez@nxp.com
State RFC, archived
Headers show

Commit Message

Leonard Crestez March 22, 2017, 4:53 p.m. UTC
If the cpufreq driver tries to modify voltage/freq during suspend/resume
it might need to control an external PMIC via I2C or SPI but those
devices might be already suspended.

To avoid this scenario we just increase cpufreq to highest setpoint
before suspend. This issue can easily be triggered by ldo-bypass but in
theory any regulator set_voltage call can end up having to modify
external supply voltages.

Signed-off-by: Leonard Crestez <leonard.crestez@nxp.com>
---
 drivers/cpufreq/imx6q-cpufreq.c | 2 ++
 1 file changed, 2 insertions(+)

Comments

Viresh Kumar March 23, 2017, 4:34 a.m. UTC | #1
On 22-03-17, 18:53, Leonard Crestez wrote:
> If the cpufreq driver tries to modify voltage/freq during suspend/resume
> it might need to control an external PMIC via I2C or SPI but those
> devices might be already suspended.
> 
> To avoid this scenario we just increase cpufreq to highest setpoint
> before suspend. This issue can easily be triggered by ldo-bypass but in
> theory any regulator set_voltage call can end up having to modify
> external supply voltages.
> 
> Signed-off-by: Leonard Crestez <leonard.crestez@nxp.com>
> ---
>  drivers/cpufreq/imx6q-cpufreq.c | 2 ++
>  1 file changed, 2 insertions(+)

Acked-by: Viresh Kumar <viresh.kumar@linaro.org>
Leonard Crestez March 28, 2017, 8:03 p.m. UTC | #2
On Thu, 2017-03-23 at 10:04 +0530, Viresh Kumar wrote:
> On 22-03-17, 18:53, Leonard Crestez wrote:
> > If the cpufreq driver tries to modify voltage/freq during suspend/resume
> > it might need to control an external PMIC via I2C or SPI but those
> > devices might be already suspended.
> > 
> > To avoid this scenario we just increase cpufreq to highest setpoint
> > before suspend. This issue can easily be triggered by ldo-bypass but in
> > theory any regulator set_voltage call can end up having to modify
> > external supply voltages.
> > 
> > Signed-off-by: Leonard Crestez <leonard.crestez@nxp.com>
> > ---
> >  drivers/cpufreq/imx6q-cpufreq.c | 2 ++
> >  1 file changed, 2 insertions(+)
> 
> Acked-by: Viresh Kumar <viresh.kumar@linaro.org>

The first couple of patches are obvious fixes despite being marked as
RFC. It would be great if you could apply them to your tree separately
from the rest of the series but I'm not sure what the process is here.
Rafael J. Wysocki March 28, 2017, 8:50 p.m. UTC | #3
On Tuesday, March 28, 2017 11:03:51 PM Leonard Crestez wrote:
> On Thu, 2017-03-23 at 10:04 +0530, Viresh Kumar wrote:
> > On 22-03-17, 18:53, Leonard Crestez wrote:
> > > If the cpufreq driver tries to modify voltage/freq during suspend/resume
> > > it might need to control an external PMIC via I2C or SPI but those
> > > devices might be already suspended.
> > > 
> > > To avoid this scenario we just increase cpufreq to highest setpoint
> > > before suspend. This issue can easily be triggered by ldo-bypass but in
> > > theory any regulator set_voltage call can end up having to modify
> > > external supply voltages.
> > > 
> > > Signed-off-by: Leonard Crestez <leonard.crestez@nxp.com>
> > > ---
> > >  drivers/cpufreq/imx6q-cpufreq.c | 2 ++
> > >  1 file changed, 2 insertions(+)
> > 
> > Acked-by: Viresh Kumar <viresh.kumar@linaro.org>
> 
> The first couple of patches are obvious fixes despite being marked as
> RFC. It would be great if you could apply them to your tree separately

Why?

> from the rest of the series but I'm not sure what the process is here.

Well, you have to talk to me.

Thanks,
Rafael
Rafael J. Wysocki March 28, 2017, 10:23 p.m. UTC | #4
On Tuesday, March 28, 2017 10:50:50 PM Rafael J. Wysocki wrote:
> On Tuesday, March 28, 2017 11:03:51 PM Leonard Crestez wrote:
> > On Thu, 2017-03-23 at 10:04 +0530, Viresh Kumar wrote:
> > > On 22-03-17, 18:53, Leonard Crestez wrote:
> > > > If the cpufreq driver tries to modify voltage/freq during suspend/resume
> > > > it might need to control an external PMIC via I2C or SPI but those
> > > > devices might be already suspended.
> > > > 
> > > > To avoid this scenario we just increase cpufreq to highest setpoint
> > > > before suspend. This issue can easily be triggered by ldo-bypass but in
> > > > theory any regulator set_voltage call can end up having to modify
> > > > external supply voltages.
> > > > 
> > > > Signed-off-by: Leonard Crestez <leonard.crestez@nxp.com>
> > > > ---
> > > >  drivers/cpufreq/imx6q-cpufreq.c | 2 ++
> > > >  1 file changed, 2 insertions(+)
> > > 
> > > Acked-by: Viresh Kumar <viresh.kumar@linaro.org>
> > 
> > The first couple of patches are obvious fixes despite being marked as
> > RFC. It would be great if you could apply them to your tree separately
> 
> Why?
> 
> > from the rest of the series but I'm not sure what the process is here.
> 
> Well, you have to talk to me.

OK, so if I understand this correctly, you would like the patches ACKed by
Viresh to be applied regardless of what happens to the rest of the series,
right?

In that case please resend them separately without the [RFC] tag and with
the ACKs from Viresh.

Thanks,
Rafael

Patch
diff mbox

diff --git a/drivers/cpufreq/imx6q-cpufreq.c b/drivers/cpufreq/imx6q-cpufreq.c
index be90ee3..e2c1fbf 100644
--- a/drivers/cpufreq/imx6q-cpufreq.c
+++ b/drivers/cpufreq/imx6q-cpufreq.c
@@ -162,6 +162,7 @@  static int imx6q_set_target(struct cpufreq_policy *policy, unsigned int index)
 static int imx6q_cpufreq_init(struct cpufreq_policy *policy)
 {
 	policy->clk = arm_clk;
+	policy->suspend_freq = freq_table[soc_opp_count - 1].frequency;
 	return cpufreq_generic_init(policy, freq_table, transition_latency);
 }
 
@@ -173,6 +174,7 @@  static struct cpufreq_driver imx6q_cpufreq_driver = {
 	.init = imx6q_cpufreq_init,
 	.name = "imx6q-cpufreq",
 	.attr = cpufreq_generic_attr,
+	.suspend = cpufreq_generic_suspend,
 };
 
 static int imx6q_cpufreq_probe(struct platform_device *pdev)