Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1752155AbaLXQmj (ORCPT ); Wed, 24 Dec 2014 11:42:39 -0500 Received: from comal.ext.ti.com ([198.47.26.152]:50709 "EHLO comal.ext.ti.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1752013AbaLXQmd (ORCPT ); Wed, 24 Dec 2014 11:42:33 -0500 Message-ID: <549AECEB.7020505@ti.com> Date: Wed, 24 Dec 2014 10:42:19 -0600 From: Nishanth Menon User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:31.0) Gecko/20100101 Thunderbird/31.3.0 MIME-Version: 1.0 To: Viresh Kumar , Dmitry Torokhov CC: "Rafael J. Wysocki" , Thomas Petazzoni , Geert Uytterhoeven , Stefan Wahren , Paul Gortmaker , "linux-pm@vger.kernel.org" , Linux Kernel Mailing List Subject: Re: [PATCH 2/4] PM / OPP: fix warning in of_free_opp_table References: <1418771379-24369-1-git-send-email-dtor@chromium.org> <1418771379-24369-3-git-send-email-dtor@chromium.org> In-Reply-To: Content-Type: text/plain; charset="utf-8" Content-Transfer-Encoding: 8bit Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org $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 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 >> --- >> 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 > > We can get rid of the extra indentation level and an extra comparison check. > > Otherwise: > > Acked-by: Viresh Kumar 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@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/