From: "Rafael J. Wysocki" <rafael@kernel.org>
To: Viresh Kumar <viresh.kumar@linaro.org>,
Amit Kucheria <amit.kucheria@linaro.org>
Cc: Linux Kernel Mailing List <linux-kernel@vger.kernel.org>,
"Rafael J. Wysocki" <rjw@rjwysocki.net>,
Sudeep Holla <sudeep.holla@arm.com>,
Linux PM list <linux-pm@vger.kernel.org>
Subject: Re: [PATCH v5 2/9] cpufreq: Auto-register the driver as a thermal cooling device if asked
Date: Tue, 29 Jan 2019 10:16:15 +0100 [thread overview]
Message-ID: <CAJZ5v0iXv3yLtGqti1OLbh-7LF1hZyvW+HTTG5mVFmEJ8a1SAA@mail.gmail.com> (raw)
In-Reply-To: <20190129070938.lqhejdu4ilra65ow@vireshk-i7>
On Tue, Jan 29, 2019 at 8:09 AM Viresh Kumar <viresh.kumar@linaro.org> wrote:
>
> On 29-01-19, 11:50, Amit Kucheria wrote:
> > On Tue, Jan 29, 2019 at 11:16 AM Amit Kucheria <amit.kucheria@linaro.org> wrote:
> > >
> > > On Tue, Jan 29, 2019 at 11:06 AM Viresh Kumar <viresh.kumar@linaro.org> wrote:
> > > >
> > > > On 29-01-19, 10:25, Amit Kucheria wrote:
> > > > > All cpufreq drivers do similar things to register as a cooling device.
> > > > > Provide a cpufreq driver flag so drivers can just ask the cpufreq core
> > > > > to register the cooling device on their behalf. This allows us to get
> > > > > rid of duplicated code in the drivers.
> > > > >
> > > > > In order to allow this, we add a struct thermal_cooling_device pointer
> > > > > to struct cpufreq_policy so that drivers don't need to store it in a
> > > > > private data structure.
> > > > >
> > > > > Suggested-by: Stephen Boyd <swboyd@chromium.org>
> > > > > Suggested-by: Viresh Kumar <viresh.kumar@linaro.org>
> > > > > Signed-off-by: Amit Kucheria <amit.kucheria@linaro.org>
> > > > > Reviewed-by: Matthias Kaehlcke <mka@chromium.org>
> > > > > Tested-by: Matthias Kaehlcke <mka@chromium.org>
> > > > > Acked-by: Viresh Kumar <viresh.kumar@linaro.org>
> > > > > ---
> > > > > drivers/cpufreq/cpufreq.c | 13 +++++++++++++
> > > > > include/linux/cpufreq.h | 9 +++++++++
> > > > > 2 files changed, 22 insertions(+)
> > > > >
> > > > > diff --git a/drivers/cpufreq/cpufreq.c b/drivers/cpufreq/cpufreq.c
> > > > > index e35a886e00bc..0f9b50d3ee91 100644
> > > > > --- a/drivers/cpufreq/cpufreq.c
> > > > > +++ b/drivers/cpufreq/cpufreq.c
> > > > > @@ -19,6 +19,7 @@
> > > > >
> > > > > #include <linux/cpu.h>
> > > > > #include <linux/cpufreq.h>
> > > > > +#include <linux/cpu_cooling.h>
> > > > > #include <linux/delay.h>
> > > > > #include <linux/device.h>
> > > > > #include <linux/init.h>
> > > > > @@ -1318,6 +1319,11 @@ static int cpufreq_online(unsigned int cpu)
> > > > > if (cpufreq_driver->ready)
> > > > > cpufreq_driver->ready(policy);
> > > > >
> > > > > +#if IS_ENABLED(CONFIG_CPU_THERMAL)
> > > > > + if (cpufreq_driver->flags & CPUFREQ_IS_COOLING_DEV)
> > > > > + policy->cdev = of_cpufreq_cooling_register(policy);
> > > > > +#endif
> > > >
> > > > I am not sure if Rafael wanted it this way but maybe something like this:
> > > >
> > > > if (IS_ENABLED(CONFIG_CPU_THERMAL) &&
> > > > cpufreq_driver->flags & CPUFREQ_IS_COOLING_DEV))
> > > > policy->cdev = of_cpufreq_cooling_register(policy);
> > > >
> > > > We never wanted ifdef hackery to be in there :)
> > >
> > > OK, that makes more sense. Should I just send out a fixup patch or the
> > > entire series?
>
> Single patch should be fine I believe.
>
> > FWIW, I checked drivers/cpufreq and drivers/thermal before converting
> > over and there is a mixed use of #if IS_ENABLED and if(IS_ENABLED).
> >
> > Perhaps we should clean it up?
>
> No objections from me on that.
Generally speaking, though, "if (IS_ENABLED(SYMBOL))" can only be used
if all of the symbols in the conditional branch are defined regardless
of whether or not SYMBOL itself is defined, so careful there.
next prev parent reply other threads:[~2019-01-29 9:16 UTC|newest]
Thread overview: 24+ messages / expand[flat|nested] mbox.gz Atom feed top
2019-01-29 4:55 [PATCH v5 0/9] cpufreq: Add flag to auto-register as cooling device Amit Kucheria
2019-01-29 4:55 ` [PATCH v5 1/9] thermal: cpu_cooling: Require thermal core to be compiled in Amit Kucheria
2019-01-30 23:53 ` Rafael J. Wysocki
2019-01-29 4:55 ` [PATCH v5 2/9] cpufreq: Auto-register the driver as a thermal cooling device if asked Amit Kucheria
2019-01-29 5:36 ` Viresh Kumar
2019-01-29 5:46 ` Amit Kucheria
2019-01-29 6:20 ` Amit Kucheria
2019-01-29 7:09 ` Viresh Kumar
2019-01-29 9:16 ` Rafael J. Wysocki [this message]
2019-01-29 9:13 ` Rafael J. Wysocki
2019-01-29 20:20 ` Amit Kucheria
2019-01-29 22:22 ` Rafael J. Wysocki
2019-01-30 5:22 ` [PATCH] " Amit Kucheria
2019-01-30 6:34 ` Viresh Kumar
2019-01-30 7:28 ` Daniel Lezcano
2019-01-29 4:55 ` [PATCH v5 3/9] cpufreq: qcom-hw: Register as a cpufreq cooling device Amit Kucheria
2019-01-29 4:55 ` [PATCH v5 4/9] cpufreq: imx6q: Use auto-registration of thermal " Amit Kucheria
2019-01-29 4:55 ` [PATCH v5 5/9] cpufreq: cpufreq-dt: " Amit Kucheria
2019-01-29 4:55 ` [PATCH v5 6/9] cpufreq: mediatek: " Amit Kucheria
2019-02-06 13:55 ` Matthias Brugger
2019-01-29 4:55 ` [PATCH v5 7/9] cpufreq: qoriq: " Amit Kucheria
2019-01-29 4:55 ` [PATCH v5 8/9] cpufreq: scmi: " Amit Kucheria
2019-01-29 4:55 ` [PATCH v5 9/9] cpufreq: scpi: " Amit Kucheria
2019-02-06 10:32 ` [PATCH v5 0/9] cpufreq: Add flag to auto-register as " Rafael J. Wysocki
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=CAJZ5v0iXv3yLtGqti1OLbh-7LF1hZyvW+HTTG5mVFmEJ8a1SAA@mail.gmail.com \
--to=rafael@kernel.org \
--cc=amit.kucheria@linaro.org \
--cc=linux-kernel@vger.kernel.org \
--cc=linux-pm@vger.kernel.org \
--cc=rjw@rjwysocki.net \
--cc=sudeep.holla@arm.com \
--cc=viresh.kumar@linaro.org \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).