Re: [PATCH 1/1] thermal: cpu_cooling: check for the readiness of cpufreq layer
On 27 November 2014 at 19:38, Eduardo Valentin wrote: >> Acked-by: Viresh Kumar > > Ok. > > though.. "normal practice" says ack's are oftern used by the maintainer > of the affected code (quoting Documentation/SubmittingPatches) :-) Hehe :) Another paragraph from the same file: Acked-by: does not necessarily indicate acknowledgement of the entire patch. For example, if a patch affects multiple subsystems and has an Acked-by: from one subsystem maintainer then this usually indicates acknowledgement of just the part which affects that maintainer's code. Judgement should be used here. When in doubt people should refer to the original discussion in the mailing list archives. So my Acked-by was as cpufreq subsystem Maintainer :) -- To unsubscribe from this list: send the line "unsubscribe linux-samsung-soc" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 1/1] thermal: cpu_cooling: check for the readiness of cpufreq layer
Hello Viresh,
On Thu, Nov 27, 2014 at 09:38:39AM +0530, Viresh Kumar wrote:
> Few nits..
>
> On 26 November 2014 at 23:20, Eduardo Valentin wrote:
>
> > Signed-off-by: Eduardo Valentin
> > ---
>
> The normal practice is to write the non-commitable part here ...
>
err... normal practice by whom? hehe...
My "normal" practice is to allow people to read the diff stat before my
extra comments :-)
> > drivers/thermal/cpu_cooling.c | 5 +
> > drivers/thermal/db8500_cpufreq_cooling.c | 5 -
> > drivers/thermal/imx_thermal.c | 5 -
> > drivers/thermal/samsung/exynos_thermal_common.c| 2 +-
> > drivers/thermal/ti-soc-thermal/ti-thermal-common.c | 6 --
> > 5 files changed, 6 insertions(+), 17 deletions(-)
> > ---
>
> But this works as well :)
>
hehe ok.
> > This is attempt to organize the cpu cooling vs. cpufreq boot sequencing.
> > The main change in this patch, as in the commit log, is to have the check
> > for the cpufreq layer in the cpu cooling device registration, instead of
> > in thermal drivers. This way, drivers don't need to bother about it, they
> > just need to propagate the error value.
> >
> > This change was tested on top of:
> > (0) - Viresh's change in cpufreq layer and cpufreq-dt (up to patch 4):
> > https://patchwork.kernel.org/patch/5384141/
> > https://patchwork.kernel.org/patch/5384151/
> > https://patchwork.kernel.org/patch/5384161/
> > https://patchwork.kernel.org/patch/5384171/
> > (1) - fix of thermal core:
> > https://patchwork.kernel.org/patch/5326991/
> >
> > After Viresh's changes, cpufreq-dt is properly sequenced with cpu cooling
> > registration. Non-of based drivers also should take advantage if these
> > changes, as now they do not need to check for cpufreq layer. The check is
> > where it belongs, in cpu cooling device registration.
> >
> > BR, Eduardo Valentin
> >
> >
> > diff --git a/drivers/thermal/cpu_cooling.c b/drivers/thermal/cpu_cooling.c
> > index 1ab0018..9e6945b 100644
> > --- a/drivers/thermal/cpu_cooling.c
> > +++ b/drivers/thermal/cpu_cooling.c
> > @@ -440,6 +440,11 @@ __cpufreq_cooling_register(struct device_node *np,
> > int ret = 0, i;
> > struct cpufreq_policy policy;
> >
> > + if (!cpufreq_get_current_driver() ||
> > !cpufreq_frequency_get_table(0)) {
>
> Only !cpufreq_frequency_get_table(0) is enough here.
>
Yeah, I thought of it too. Just combined what people had in their
drivers here. But I agree that latter is enough.
> For rest:
>
> Acked-by: Viresh Kumar
Ok.
though.. "normal practice" says ack's are oftern used by the maintainer
of the affected code (quoting Documentation/SubmittingPatches) :-)
BR, Eduardo Valenti
signature.asc
Description: Digital signature
Re: [PATCH 1/1] thermal: cpu_cooling: check for the readiness of cpufreq layer
Few nits..
On 26 November 2014 at 23:20, Eduardo Valentin wrote:
> Signed-off-by: Eduardo Valentin
> ---
The normal practice is to write the non-commitable part here ...
> drivers/thermal/cpu_cooling.c | 5 +
> drivers/thermal/db8500_cpufreq_cooling.c | 5 -
> drivers/thermal/imx_thermal.c | 5 -
> drivers/thermal/samsung/exynos_thermal_common.c| 2 +-
> drivers/thermal/ti-soc-thermal/ti-thermal-common.c | 6 --
> 5 files changed, 6 insertions(+), 17 deletions(-)
> ---
But this works as well :)
> This is attempt to organize the cpu cooling vs. cpufreq boot sequencing.
> The main change in this patch, as in the commit log, is to have the check
> for the cpufreq layer in the cpu cooling device registration, instead of
> in thermal drivers. This way, drivers don't need to bother about it, they
> just need to propagate the error value.
>
> This change was tested on top of:
> (0) - Viresh's change in cpufreq layer and cpufreq-dt (up to patch 4):
> https://patchwork.kernel.org/patch/5384141/
> https://patchwork.kernel.org/patch/5384151/
> https://patchwork.kernel.org/patch/5384161/
> https://patchwork.kernel.org/patch/5384171/
> (1) - fix of thermal core:
> https://patchwork.kernel.org/patch/5326991/
>
> After Viresh's changes, cpufreq-dt is properly sequenced with cpu cooling
> registration. Non-of based drivers also should take advantage if these
> changes, as now they do not need to check for cpufreq layer. The check is
> where it belongs, in cpu cooling device registration.
>
> BR, Eduardo Valentin
>
>
> diff --git a/drivers/thermal/cpu_cooling.c b/drivers/thermal/cpu_cooling.c
> index 1ab0018..9e6945b 100644
> --- a/drivers/thermal/cpu_cooling.c
> +++ b/drivers/thermal/cpu_cooling.c
> @@ -440,6 +440,11 @@ __cpufreq_cooling_register(struct device_node *np,
> int ret = 0, i;
> struct cpufreq_policy policy;
>
> + if (!cpufreq_get_current_driver() || !cpufreq_frequency_get_table(0))
> {
Only !cpufreq_frequency_get_table(0) is enough here.
For rest:
Acked-by: Viresh Kumar
--
To unsubscribe from this list: send the line "unsubscribe linux-samsung-soc" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
