RE: [PATCH] cpufreq: imx-cpufreq-dt: Assign max supported frequency as suspend frequency
> > On 7/8/2019 10:55 AM, anson.hu...@nxp.com 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
> >
> > > 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.
Please ignore this patch, based on the discussion, I have sent out a new patch
series to support this feature. Please review them:
https://patchwork.kernel.org/patch/11036505/
Thanks,
Anson
RE: [PATCH] cpufreq: imx-cpufreq-dt: Assign max supported frequency as suspend frequency
Hi, Leonard
> On 7/8/2019 10:55 AM, anson.hu...@nxp.com 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
>
> > 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
Re: [PATCH] cpufreq: imx-cpufreq-dt: Assign max supported frequency as suspend frequency
On 7/8/2019 10:55 AM, anson.hu...@nxp.com 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
> 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.
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.
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
--
Regards,
Leonard
