From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1753966AbcDUXJZ (ORCPT ); Thu, 21 Apr 2016 19:09:25 -0400 Received: from mail-oi0-f46.google.com ([209.85.218.46]:32961 "EHLO mail-oi0-f46.google.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1753927AbcDUXJX convert rfc822-to-8bit (ORCPT ); Thu, 21 Apr 2016 19:09:23 -0400 MIME-Version: 1.0 In-Reply-To: <1460766432-5390-1-git-send-email-giedrius.statkevicius@gmail.com> References: <1460766432-5390-1-git-send-email-giedrius.statkevicius@gmail.com> Date: Fri, 22 Apr 2016 02:09:22 +0300 Message-ID: Subject: Re: [PATCH] asus-laptop: correct error handling in asus_read_brightness() From: Andy Shevchenko To: =?UTF-8?Q?Giedrius_Statkevi=C4=8Dius?= Cc: Corentin Chary , "dvhart@infradead.org" , acpi4asus-user@lists.sourceforge.net, platform-driver-x86@vger.kernel.org, "linux-kernel@vger.kernel.org" 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 On Sat, Apr 16, 2016 at 3:27 AM, Giedrius Statkevičius wrote: > It is possible that acpi_evaluate_integer might fail and value would not be > set to any value so correct this defect by returning 0 in case of an > error. This is also the correct thing to return because the backlight > subsystem will print the old value of brightness in this case. > > Signed-off-by: Giedrius Statkevičius > --- > drivers/platform/x86/asus-laptop.c | 4 +++- > 1 file changed, 3 insertions(+), 1 deletion(-) > > diff --git a/drivers/platform/x86/asus-laptop.c b/drivers/platform/x86/asus-laptop.c > index 9a69734..15f1311 100644 > --- a/drivers/platform/x86/asus-laptop.c > +++ b/drivers/platform/x86/asus-laptop.c > @@ -775,8 +775,10 @@ static int asus_read_brightness(struct backlight_device *bd) > > rv = acpi_evaluate_integer(asus->handle, METHOD_BRIGHTNESS_GET, > NULL, &value); > - if (ACPI_FAILURE(rv)) > + if (ACPI_FAILURE(rv)) { > pr_warn("Error reading brightness\n"); > + return 0; > + } This looks like a workaround. I suppose the real fix is to return here an error code and fix all callers, like drivers/video/backlight/backlight.c. > > return value; > } > -- > 2.8.0 > -- With Best Regards, Andy Shevchenko