RE: [PATCH] cpufreq: imx-cpufreq-dt: Assign max supported frequency as suspend frequency

[Anson Huang]

Hi, Leonard

> On 7/8/2019 10:55 AM, Anson.Huang@xxxxxxx wrote:
> > To reduce the suspend/resume latency, CPU's max supported frequency
> > should be used during low level suspend/resume phase, "opp-suspend"
> > property is NOT feasible since OPP defined in DT could be NOT
> > supported according to speed garding and market segment fuse settings.
> > So we can assign the cpufreq policy's suspend_freq with max available
> > frequency provided by cpufreq driver.
> >
> > Signed-off-by: Anson Huang <Anson.Huang@xxxxxxx>
> 
> > diff --git a/drivers/cpufreq/imx-cpufreq-dt.c
> > b/drivers/cpufreq/imx-cpufreq-dt.c
> 
> > +static int __init imx_cpufreq_dt_setup_suspend_opp(void)
> > +{
> > +	struct cpufreq_policy *policy = cpufreq_cpu_get(0);
> > +
> > +	policy->suspend_freq = cpufreq_quick_get_max(0);
> > +
> > +	return 0;
> > +}
> > +late_initcall(imx_cpufreq_dt_setup_suspend_opp);
> 
> The imx-cpufreq-dt driver is built as a module by default and this patch
> produces an error:
> 
> In file included from ../drivers/cpufreq/imx-cpufreq-dt.c:11:
> ../include/linux/module.h:131:42: error: redefinition of â__inittestâ
>    static inline initcall_t __maybe_unused __inittest(void)  \
>                                            ^~~~~~~~~~
> ../include/linux/device.h:1656:1: note: in expansion of macro âmodule_initâ
>   module_init(__driver##_init); \
>   ^~~~~~~~~~~
> 
> As far as I can tell late_initcall is not supported for modules.

Ah, yes, I have NOT test the module build, I ONLY tested the built-in case, thanks for
reminder.

> 
> Viresh: "max freq as suspend freq" is something that could be useful for
> other SOC families. The hardware can suspend at any freq; it's just that the
> highest one makes sense because it makes suspend/resume slightly faster.

Agree.

> 
> Could this behavior be pushed to cpufreq-dt as a bool flag inside struct
> cpufreq_dt_platform_data?
> 
> Only a few other platforms use this, most others pass NULL like imx. But
> passing custom SOC-specific flags to cpufreq-dt makes a lot of sense

Although we have other methods to make it work for i.MX platforms, like setting
the suspend freq as this patch did but ONLY be called before suspend, but if common
cpufreq-dt can handle this case, that would benefit us and other platforms a lot, waiting
for your opinion.

Thanks,
Anson