[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <20150817090548.GA2762@e104805>
Date: Mon, 17 Aug 2015 10:05:48 +0100
From: Javi Merino <javi.merino@....com>
To: Viresh Kumar <viresh.kumar@...aro.org>
Cc: "linux-pm@...r.kernel.org" <linux-pm@...r.kernel.org>,
"linux-kernel@...r.kernel.org" <linux-kernel@...r.kernel.org>,
"cw00.choi@...sung.com" <cw00.choi@...sung.com>,
"rufus.hamade@...tec.com" <rufus.hamade@...tec.com>,
"Rafael J. Wysocki" <rjw@...ysocki.net>
Subject: Re: [PATCH v4 2/5] PM / OPP: add a function to get the voltage for
disabled OPPs
On Sat, Aug 15, 2015 at 06:59:25AM +0100, Viresh Kumar wrote:
> On 14-08-15, 18:56, Javi Merino wrote:
> > The OPP library is now used for power models to calculate the power
> > that a device would consume at a specific OPP. To do that, we use a
> > simple power model which takes frequency and voltage as inputs. We get
> > the voltage and frequency from the OPP library.
> >
> > The devfreq cooling device for the thermal framework controls temperature
> > by disabling OPPs. The power model needs to calculate the power that
> > would be consumed if we reenabled the OPP. dev_pm_opp_get_voltage()
> > doesn't work for disabled OPPs.
> >
> > Add a dev_pm_opp_get_voltage_always() that works both for enabled and
> > disabled OPPs to be used by the power model. The documentation for this
> > function clearly states that you should use dev_pm_opp_get_voltage()
> > instead unless you know what you're doing.
> >
> > Cc: "Rafael J. Wysocki" <rjw@...ysocki.net>
> > Cc: Viresh Kumar <viresh.kumar@...aro.org>
> > Signed-off-by: Javi Merino <javi.merino@....com>
> > ---
> > drivers/base/power/opp.c | 37 +++++++++++++++++++++++++++++++++++++
> > include/linux/pm_opp.h | 7 +++++++
> > 2 files changed, 44 insertions(+)
> >
> > diff --git a/drivers/base/power/opp.c b/drivers/base/power/opp.c
> > index 677fb2843553..b1a4216c7ec3 100644
> > --- a/drivers/base/power/opp.c
> > +++ b/drivers/base/power/opp.c
> > @@ -182,6 +182,43 @@ unsigned long dev_pm_opp_get_voltage(struct dev_pm_opp *opp)
> > EXPORT_SYMBOL_GPL(dev_pm_opp_get_voltage);
> >
> > /**
> > + * dev_pm_opp_get_voltage_always() - Gets the voltage corresponding to an opp
> > + * @opp: opp for which voltage has to be returned for
> > + *
> > + * This function is similar to dev_pm_opp_get_voltage() except that it
> > + * works for disabled opps as well. In most cases, you want to
> > + * operate only on available opps so you should use
> > + * dev_pm_opp_get_voltage() instead.
> > + *
> > + * Return: voltage in micro volt corresponding to the opp, else
> > + * return 0
> > + *
> > + * Locking: This function must be called under rcu_read_lock(). opp is a rcu
> > + * protected pointer. This means that opp which could have been fetched by
> > + * opp_find_freq_{exact,ceil,floor} functions is valid as long as we are
> > + * under RCU lock. The pointer returned by the opp_find_freq family must be
> > + * used in the same section as the usage of this function with the pointer
> > + * prior to unlocking with rcu_read_unlock() to maintain the integrity of the
> > + * pointer.
> > + */
> > +unsigned long dev_pm_opp_get_voltage_always(struct dev_pm_opp *opp)
> > +{
> > + struct dev_pm_opp *tmp_opp;
> > + unsigned long v = 0;
> > +
> > + opp_rcu_lockdep_assert();
> > +
> > + tmp_opp = rcu_dereference(opp);
> > + if (unlikely(IS_ERR_OR_NULL(tmp_opp)))
> > + pr_err("%s: Invalid parameters\n", __func__);
> > + else
> > + v = tmp_opp->u_volt;
> > +
> > + return v;
> > +}
> > +EXPORT_SYMBOL_GPL(dev_pm_opp_get_voltage_always);
>
> I will rather update dev_pm_opp_get_voltage() and remove the
> 'available' check. There is no need for that.
Even better, I'll do that for the next version.
Cheers,
Javi
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Powered by blists - more mailing lists