public inbox for linux-kernel@vger.kernel.org
 help / color / mirror / Atom feed
From: Matthias Kaehlcke <mka@chromium.org>
To: Quentin Perret <quentin.perret@arm.com>
Cc: Sudeep Holla <sudeep.holla@arm.com>,
	viresh.kumar@linaro.org, rjw@rjwysocki.net, nm@ti.com,
	sboyd@kernel.org, linux-pm@vger.kernel.org,
	linux-kernel@vger.kernel.org,
	linux-arm-kernel@lists.infradead.org, dietmar.eggemann@arm.com
Subject: Re: [PATCH v3 1/5] PM / OPP: Introduce a power estimation helper
Date: Fri, 1 Feb 2019 10:16:24 -0800	[thread overview]
Message-ID: <20190201181624.GQ81583@google.com> (raw)
In-Reply-To: <20190201120951.lqxy4u7kxfzfmmub@queper01-lin>

On Fri, Feb 01, 2019 at 12:09:53PM +0000, Quentin Perret wrote:
> Hi Sudeep,
> 
> On Friday 01 Feb 2019 at 12:04:53 (+0000), Sudeep Holla wrote:
> > On Fri, Feb 01, 2019 at 09:30:57AM +0000, Quentin Perret wrote:
> > > +void dev_pm_opp_of_register_em(struct cpumask *cpus, int nr_opp)
> > > +{
> > > +	struct em_data_callback em_cb = EM_DATA_CB(_get_cpu_power);
> > > +	int ret, cpu = cpumask_first(cpus);
> > > +	struct device *cpu_dev;
> > > +	struct device_node *np;
> > > +	u32 cap;
> > > +
> > > +	cpu_dev = get_cpu_device(cpu);
> > > +	if (!cpu_dev)
> > > +		return;
> > > +
> > > +	np = of_node_get(cpu_dev->of_node);
> > > +	if (!np)
> > > +		return;
> > > +
> > 
> > Does it make sense to add the check for OPP count here. You need not pass
> > that as parameter. Just makes one less thing to check in new drivers adding
> > this support. Thoughts ?
> 
> Yeah Matthias had the same suggestion. I don't mind moving it here TBH.
> It's just that some users already do the opp count before calling this
> function, so I figured I could as well use that data instead of counting
> again.
> 
> But yeah, that's one less thing to worry about on the driver side so
> I'll move the OPP count in there for v4 and we'll see if people ask me
> to move it out to optimize things ;-)

From an API perspective it would be nice to get rid of the nr_opp
parameter, it seems somewhat arbitrary. Moving dev_pm_opp_get_opp_count()
from the drivers into dev_pm_opp_of_register_em() (instead of calling
it twice) also sounds good in general, as long as the error handling
doesn't become too messy. In the current version
dev_pm_opp_of_register_em() doesn't return a value, with the change it
would have to return one to catch an empty OPP table, and it needs
to be distinguished from other cases where the EM registration fails
but the cpufreq driver is still functional (e.g. no
'dynamic-power-coefficient'). Maybe return -ENOTSUPP in those cases?

Well, let's see how it looks like :)

  parent reply	other threads:[~2019-02-01 18:16 UTC|newest]

Thread overview: 13+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2019-02-01  9:30 [PATCH v3 0/5] Register an Energy Model for Arm reference platforms Quentin Perret
2019-02-01  9:30 ` [PATCH v3 1/5] PM / OPP: Introduce a power estimation helper Quentin Perret
2019-02-01 12:04   ` Sudeep Holla
2019-02-01 12:09     ` Quentin Perret
2019-02-01 12:27       ` Sudeep Holla
2019-02-01 12:44         ` Sudeep Holla
2019-02-01 18:16       ` Matthias Kaehlcke [this message]
2019-02-01  9:30 ` [PATCH v3 2/5] cpufreq: dt: Register an Energy Model Quentin Perret
2019-02-01  9:30 ` [PATCH v3 3/5] cpufreq: scpi: " Quentin Perret
2019-02-01  9:31 ` [PATCH v3 4/5] cpufreq: arm_big_little: " Quentin Perret
2019-02-01 11:53   ` Sudeep Holla
2019-02-01 12:11     ` Quentin Perret
2019-02-01  9:31 ` [PATCH v3 5/5] cpufreq: scmi: " Quentin Perret

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=20190201181624.GQ81583@google.com \
    --to=mka@chromium.org \
    --cc=dietmar.eggemann@arm.com \
    --cc=linux-arm-kernel@lists.infradead.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=linux-pm@vger.kernel.org \
    --cc=nm@ti.com \
    --cc=quentin.perret@arm.com \
    --cc=rjw@rjwysocki.net \
    --cc=sboyd@kernel.org \
    --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