From: "Rafael J. Wysocki" <rjw@rjwysocki.net>
To: Viresh Kumar <viresh.kumar@linaro.org>
Cc: Linux PM <linux-pm@vger.kernel.org>,
Srinivas Pandruvada <srinivas.pandruvada@linux.intel.com>,
LKML <linux-kernel@vger.kernel.org>,
Jonathan Corbet <corbet@lwn.net>,
Linux Documentation <linux-doc@vger.kernel.org>
Subject: Re: [RFC][PATCH] cpufreq: User/admin documentation update and consolidation
Date: Wed, 22 Feb 2017 01:50:45 +0100 [thread overview]
Message-ID: <8913428.RXS4e600Af@aspire.rjw.lan> (raw)
In-Reply-To: <20170221050010.GW21911@vireshk-i7>
On Tuesday, February 21, 2017 10:30:10 AM Viresh Kumar wrote:
> On 20-02-17, 14:58, Rafael J. Wysocki wrote:
> > Yes, it is called for new and inactive policies.
> >
> > For new policies it has to populate policy->cpus (because otherwise the core
> > doesn't know what CPUs should be there), which quite arguably doesn't have
> > to (or even doesn't need to) be done for inactive policies (because they already
> > have policy->real_cpus and policy->related_cpus populated).
>
> > I would even argue that ->init() should not update policy->cpus for inactive
> > (but not new) policies.
>
> I agree, but I am not aware of any platforms that we have today which
> does any similar checks in ->init(). And I wouldn't encourage drivers
> to have such special handling at all. Why make it complex?
Well, IMO we should not invoke the same callback in two different cases and
then do tricks in the core to make one of them work. That's just too [edited]
ugly.
At least it doesn't help to understand the code flow.
> The way you have written it earlier suggests that the drivers should
> check if the policy is active or not (by looking at related_cpus mask)
> and set the cpus mask only for new policies. I was just worried that
> new driver authors will actually try to write special code in init()
> for that and if we really want that to be the case.
That would not be a bug IMO. Some of them could just check related_cpus
and do nothing if it is set I suppose?
Anyway, this isn't a driver API document, so the description is not super-precise.
Thanks,
Rafael
next prev parent reply other threads:[~2017-02-22 0:55 UTC|newest]
Thread overview: 7+ messages / expand[flat|nested] mbox.gz Atom feed top
2017-02-18 1:36 [RFC][PATCH] cpufreq: User/admin documentation update and consolidation Rafael J. Wysocki
2017-02-20 9:56 ` Viresh Kumar
2017-02-20 13:58 ` Rafael J. Wysocki
2017-02-20 23:47 ` Rafael J. Wysocki
2017-02-21 5:00 ` Viresh Kumar
2017-02-22 0:50 ` Rafael J. Wysocki [this message]
2017-02-21 0:59 ` [RFC][PATCH v2] " 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=8913428.RXS4e600Af@aspire.rjw.lan \
--to=rjw@rjwysocki.net \
--cc=corbet@lwn.net \
--cc=linux-doc@vger.kernel.org \
--cc=linux-kernel@vger.kernel.org \
--cc=linux-pm@vger.kernel.org \
--cc=srinivas.pandruvada@linux.intel.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