linux-kernel - Re: [PATCH 2/4] PM / OPP: fix warning in of_free_opp

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]

Date:	Wed, 24 Dec 2014 10:42:19 -0600
From:	Nishanth Menon <nm@...com>
To:	Viresh Kumar <viresh.kumar@...aro.org>,
	Dmitry Torokhov <dtor@...omium.org>
CC:	"Rafael J. Wysocki" <rjw@...ysocki.net>,
	Thomas Petazzoni <thomas.petazzoni@...e-electrons.com>,
	Geert Uytterhoeven <geert+renesas@...der.be>,
	Stefan Wahren <stefan.wahren@...e.com>,
	Paul Gortmaker <paul.gortmaker@...driver.com>,
	"linux-pm@...r.kernel.org" <linux-pm@...r.kernel.org>,
	Linux Kernel Mailing List <linux-kernel@...r.kernel.org>
Subject: Re: [PATCH 2/4] PM / OPP: fix warning in of_free_opp_table

$subject:
PM / OPP: Do not warn when no OPP was ever registered in of_free_opp_table

might be more appropriate?

On 12/16/2014 10:28 PM, Viresh Kumar wrote:
> On 17 December 2014 at 04:39, Dmitry Torokhov <dtor@...omium.org> wrote:
>> Not having OPP defined for a device is not a crime, we should not splat
>> warning in this case. Also, it seems that we are ready to accept invalid
>> dev (find_device_opp will return ERR_PTR(-EINVAL) then) so let's not
>> crash in dev_name() in such case.
>>
>> Signed-off-by: Dmitry Torokhov <dtor@...omium.org>
>> ---
>>  drivers/base/power/opp.c | 10 ++++++++--
>>  1 file changed, 8 insertions(+), 2 deletions(-)
>>
>> diff --git a/drivers/base/power/opp.c b/drivers/base/power/opp.c
>> index b78c14d..413c7fe 100644
>> --- a/drivers/base/power/opp.c
>> +++ b/drivers/base/power/opp.c
>> @@ -799,9 +799,15 @@ void of_free_opp_table(struct device *dev)
>>
>>         /* Check for existing list for 'dev' */
>>         dev_opp = find_device_opp(dev);
>> -       if (WARN(IS_ERR(dev_opp), "%s: dev_opp: %ld\n", dev_name(dev),
>> -                PTR_ERR(dev_opp)))
>> +       if (IS_ERR(dev_opp)) {
>> +               int error = PTR_ERR(dev_opp);
>> +               if (error != -ENODEV)
>> +                       WARN(1, "%s: dev_opp: %ld\n",
>> +                            IS_ERR_OR_NULL(dev) ?
>> +                                       "Invalid device" : dev_name(dev),
>> +                            error);
>>                 return;
> 
> What about this:
> 
>         if (IS_ERR(dev_opp)) {
>                 int error = PTR_ERR(dev_opp);
>                 WARN(error != -ENODEV, "%s: dev_opp: %ld\n",
>                      IS_ERR_OR_NULL(dev) ?  "Invalid device" : dev_name(dev),
>                      error);
>                 return;
>         }
Adds the following build warning: I suggest changing the %ld to %d

"warning: format ‘%ld’ expects argument of type ‘long int’, but
argument 5 has type ‘int’ [-Wformat]"


This also fixes the warning I got on AM437x based platforms. test results:
1: am437x-sk: BOOT: PASS: crit=2 err=13 warn=25, CPUFreq: N/A,
CPUIdle: N/A: http://slexy.org/raw/s20MAtNEbt
 2: am437x-sk-before: BOOT: PASS: crit=2 err=13 warn=57, CPUFreq: N/A,
CPUIdle: N/A: http://slexy.org/raw/s21rZCo4cD

With the suggested changes,

Acked-by: Nishanth Menon <nm@...com>

> 
> We can get rid of the extra indentation level and an extra comparison check.
> 
> Otherwise:
> 
> Acked-by: Viresh Kumar <viresh.kumar@...aro.org>


PS: I am sure you have already done some level of checks, but I
suggest using something like aiaiai or
https://github.com/nmenon/kernel_patch_verify or other solutions like
those discussed in
https://plus.google.com/112464029509057661457/posts/5LTuHHMyXLT to
help do some initial clean up of the patches..


-- 
Regards,
Nishanth Menon
--
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/