| Message ID | 2e335a6c263704a8d465bd02896fc5fff0533fdc.1654849214.git.viresh.kumar@linaro.org (mailing list archive) |
|---|---|
| State | New, archived |
| Delegated to: | viresh kumar |
| Headers | show |
| Series | OPP: Add support for multiple clocks | expand |
On 10/06/2022 09:20, Viresh Kumar wrote: > Reuse _opp_compare_key() in _opp_add_static_v2() instead of just > comparing frequency while finding suspend frequency. Also add a comment > over _opp_compare_key() explaining its return values. > > Signed-off-by: Viresh Kumar <viresh.kumar@linaro.org> > --- > drivers/opp/core.c | 6 ++++++ > drivers/opp/of.c | 4 ++-- > 2 files changed, 8 insertions(+), 2 deletions(-) > > diff --git a/drivers/opp/core.c b/drivers/opp/core.c > index fe447f41c99e..9f284dc0d9d7 100644 > --- a/drivers/opp/core.c > +++ b/drivers/opp/core.c > @@ -1618,6 +1618,12 @@ static bool _opp_supported_by_regulators(struct dev_pm_opp *opp, > return true; > } > > +/* > + * Returns > + * 0: opp1 == opp2 > + * 1: opp1 > opp2 > + * -1: opp1 < opp2 > + */ > int _opp_compare_key(struct dev_pm_opp *opp1, struct dev_pm_opp *opp2) > { > if (opp1->rate != opp2->rate) > diff --git a/drivers/opp/of.c b/drivers/opp/of.c > index bec9644a7260..843923ab9d66 100644 > --- a/drivers/opp/of.c > +++ b/drivers/opp/of.c > @@ -929,8 +929,8 @@ static struct dev_pm_opp *_opp_add_static_v2(struct opp_table *opp_table, > /* OPP to select on device suspend */ > if (of_property_read_bool(np, "opp-suspend")) { > if (opp_table->suspend_opp) { > - /* Pick the OPP with higher rate as suspend OPP */ > - if (new_opp->rate > opp_table->suspend_opp->rate) { > + /* Pick the OPP with higher rate/bw/level as suspend OPP */ > + if (_opp_compare_key(opp_table, new_opp, opp_table->suspend_opp) == 1) { > opp_table->suspend_opp->suspend = false; > new_opp->suspend = true; > opp_table->suspend_opp = new_opp; FYI ... if I checkout commit 00d776d33da9 ("OPP: Reuse _opp_compare_key() in _opp_add_static_v2()") from next-20220622 it does not compile ... drivers/opp/of.c: In function ‘_opp_add_static_v2’: drivers/opp/of.c:933:25: error: passing argument 1 of ‘_opp_compare_key’ from incompatible pointer type [-Werror=incompatible-pointer-types] if (_opp_compare_key(opp_table, new_opp, opp_table->suspend_opp) == 1) { ^~~~~~~~~ In file included from drivers/opp/of.c:22:0: drivers/opp/opp.h:228:5: note: expected ‘struct dev_pm_opp *’ but argument is of type ‘struct opp_table *’ int _opp_compare_key(struct dev_pm_opp *opp1, struct dev_pm_opp *opp2); ^~~~~~~~~~~~~~~~ drivers/opp/of.c:933:8: error: too many arguments to function ‘_opp_compare_key’ if (_opp_compare_key(opp_table, new_opp, opp_table->suspend_opp) == 1) { ^~~~~~~~~~~~~~~~ In file included from drivers/opp/of.c:22:0: drivers/opp/opp.h:228:5: note: declared here int _opp_compare_key(struct dev_pm_opp *opp1, struct dev_pm_opp *opp2); ^~~~~~~~~~~~~~~~ This breaks bisecting -next and so would be good to fix this. Cheers Jon
On 22-06-22, 14:58, Jon Hunter wrote: > FYI ... if I checkout commit 00d776d33da9 ("OPP: Reuse > _opp_compare_key() in _opp_add_static_v2()") from next-20220622 > it does not compile ... > > drivers/opp/of.c: In function ‘_opp_add_static_v2’: > drivers/opp/of.c:933:25: error: passing argument 1 of ‘_opp_compare_key’ from incompatible pointer type [-Werror=incompatible-pointer-types] > if (_opp_compare_key(opp_table, new_opp, opp_table->suspend_opp) == 1) { > ^~~~~~~~~ > In file included from drivers/opp/of.c:22:0: > drivers/opp/opp.h:228:5: note: expected ‘struct dev_pm_opp *’ but argument is of type ‘struct opp_table *’ > int _opp_compare_key(struct dev_pm_opp *opp1, struct dev_pm_opp *opp2); > ^~~~~~~~~~~~~~~~ > drivers/opp/of.c:933:8: error: too many arguments to function ‘_opp_compare_key’ > if (_opp_compare_key(opp_table, new_opp, opp_table->suspend_opp) == 1) { > ^~~~~~~~~~~~~~~~ > In file included from drivers/opp/of.c:22:0: > drivers/opp/opp.h:228:5: note: declared here > int _opp_compare_key(struct dev_pm_opp *opp1, struct dev_pm_opp *opp2); > ^~~~~~~~~~~~~~~~ > > This breaks bisecting -next and so would be good to fix this. Yeah, this was reported yesterday and is already fixed in my branch, along with few more fixes. git://git.kernel.org/pub/scm/linux/kernel/git/vireshk/pm.git opp/linux-next It hasn't landed into linux-next/master yet though.
diff --git a/drivers/opp/core.c b/drivers/opp/core.c index fe447f41c99e..9f284dc0d9d7 100644 --- a/drivers/opp/core.c +++ b/drivers/opp/core.c @@ -1618,6 +1618,12 @@ static bool _opp_supported_by_regulators(struct dev_pm_opp *opp, return true; } +/* + * Returns + * 0: opp1 == opp2 + * 1: opp1 > opp2 + * -1: opp1 < opp2 + */ int _opp_compare_key(struct dev_pm_opp *opp1, struct dev_pm_opp *opp2) { if (opp1->rate != opp2->rate) diff --git a/drivers/opp/of.c b/drivers/opp/of.c index bec9644a7260..843923ab9d66 100644 --- a/drivers/opp/of.c +++ b/drivers/opp/of.c @@ -929,8 +929,8 @@ static struct dev_pm_opp *_opp_add_static_v2(struct opp_table *opp_table, /* OPP to select on device suspend */ if (of_property_read_bool(np, "opp-suspend")) { if (opp_table->suspend_opp) { - /* Pick the OPP with higher rate as suspend OPP */ - if (new_opp->rate > opp_table->suspend_opp->rate) { + /* Pick the OPP with higher rate/bw/level as suspend OPP */ + if (_opp_compare_key(opp_table, new_opp, opp_table->suspend_opp) == 1) { opp_table->suspend_opp->suspend = false; new_opp->suspend = true; opp_table->suspend_opp = new_opp;
Reuse _opp_compare_key() in _opp_add_static_v2() instead of just comparing frequency while finding suspend frequency. Also add a comment over _opp_compare_key() explaining its return values. Signed-off-by: Viresh Kumar <viresh.kumar@linaro.org> --- drivers/opp/core.c | 6 ++++++ drivers/opp/of.c | 4 ++-- 2 files changed, 8 insertions(+), 2 deletions(-)