linux-kernel - Re: [PATCH 3/8] OPP: Reuse _opp_compare_key() in _opp_add_static

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <eb9558fd-4f2d-4e83-f9a3-86ff5dd51784@nvidia.com>
Date:   Wed, 22 Jun 2022 14:58:25 +0100
From:   Jon Hunter <jonathanh@...dia.com>
To:     Viresh Kumar <viresh.kumar@...aro.org>,
        Krzysztof Kozlowski <krzysztof.kozlowski@...aro.org>,
        Viresh Kumar <vireshk@...nel.org>, Nishanth Menon <nm@...com>,
        Stephen Boyd <sboyd@...nel.org>
Cc:     linux-pm@...r.kernel.org,
        Vincent Guittot <vincent.guittot@...aro.org>,
        "Rafael J. Wysocki" <rafael@...nel.org>,
        linux-kernel@...r.kernel.org,
        "linux-tegra@...r.kernel.org" <linux-tegra@...r.kernel.org>
Subject: Re: [PATCH 3/8] OPP: Reuse _opp_compare_key() in _opp_add_static_v2()


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@...aro.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

-- 
nvpublic