[PATCH] dell-laptop: Fix backlight detection

[PATCH] dell-laptop: Fix backlight detection

[Damien Thébault]

Fix return code check for "max brightness" ACPI call.

The dell laptop ACPI video brightness control is not present on dell
laptops anymore, but was present in older kernel versions.

The code that checks the return value is incorrect since the SMM
refactoring.

The old code was:
  if (buffer->output[0] == 0)

Which was changed to:
  ret = dell_send_request(...)
  if (ret)

However, dell_send_request() will return 0 if buffer->output[0] == 0,
so we must change the check to:
  if (ret == 0)

This issue was found on a Dell M4800 laptop, and the fix tested on it
as well.

Fixes: 549b4930f057 ("dell-smbios: Introduce dispatcher for SMM calls")
Signed-off-by: Damien Thébault <damien@dtbo.net>
Tested-by: Damien Thébault <damien@dtbo.net>
---
 drivers/platform/x86/dell-laptop.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/platform/x86/dell-laptop.c
b/drivers/platform/x86/dell-laptop.c index f1fa8612db40..06978c14c83b
100644 --- a/drivers/platform/x86/dell-laptop.c
+++ b/drivers/platform/x86/dell-laptop.c
@@ -2185,7 +2185,7 @@ static int __init dell_init(void)
 		dell_fill_request(&buffer, token->location, 0, 0, 0);
 		ret = dell_send_request(&buffer,
 					CLASS_TOKEN_READ,
SELECT_TOKEN_AC);
-		if (ret)
+		if (ret == 0)
 			max_intensity = buffer.output[3];
 	}
 
-- 
2.17.1

Re: [PATCH] dell-laptop: Fix backlight detection

[Pali Rohár]

[-- Attachment #1: Type: text/plain, Size: 1657 bytes --]

On Sunday 08 July 2018 17:02:20 Damien Thébault wrote:
> Fix return code check for "max brightness" ACPI call.
> 
> The dell laptop ACPI video brightness control is not present on dell
> laptops anymore, but was present in older kernel versions.
> 
> The code that checks the return value is incorrect since the SMM
> refactoring.
> 
> The old code was:
>   if (buffer->output[0] == 0)
> 
> Which was changed to:
>   ret = dell_send_request(...)
>   if (ret)
> 
> However, dell_send_request() will return 0 if buffer->output[0] == 0,
> so we must change the check to:
>   if (ret == 0)

You are right, dell_send_request() return non-zero value on error.

> This issue was found on a Dell M4800 laptop, and the fix tested on it
> as well.
> 
> Fixes: 549b4930f057 ("dell-smbios: Introduce dispatcher for SMM calls")
> Signed-off-by: Damien Thébault <damien@dtbo.net>
> Tested-by: Damien Thébault <damien@dtbo.net>
> ---
>  drivers/platform/x86/dell-laptop.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/drivers/platform/x86/dell-laptop.c
> b/drivers/platform/x86/dell-laptop.c index f1fa8612db40..06978c14c83b
> 100644 --- a/drivers/platform/x86/dell-laptop.c
> +++ b/drivers/platform/x86/dell-laptop.c
> @@ -2185,7 +2185,7 @@ static int __init dell_init(void)
>  		dell_fill_request(&buffer, token->location, 0, 0, 0);
>  		ret = dell_send_request(&buffer,
>  					CLASS_TOKEN_READ,
> SELECT_TOKEN_AC);
> -		if (ret)
> +		if (ret == 0)
>  			max_intensity = buffer.output[3];
>  	}
>  

Reviewed-by: Pali Rohár <pali.rohar@gmail.com>

-- 
Pali Rohár
pali.rohar@gmail.com

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 195 bytes --]

RE: [PATCH] dell-laptop: Fix backlight detection

[Mario.Limonciello]

> -----Original Message-----
> From: Pali Rohár [mailto:pali.rohar@gmail.com]
> Sent: Sunday, July 8, 2018 8:09 AM
> To: Damien Thébault
> Cc: Matthew Garrett; Darren Hart; Andy Shevchenko; Limonciello, Mario; platform-
> driver-x86@vger.kernel.org; linux-kernel@vger.kernel.org
> Subject: Re: [PATCH] dell-laptop: Fix backlight detection
> 
> On Sunday 08 July 2018 17:02:20 Damien Thébault wrote:
> > Fix return code check for "max brightness" ACPI call.
> >
> > The dell laptop ACPI video brightness control is not present on dell
> > laptops anymore, but was present in older kernel versions.
> >
> > The code that checks the return value is incorrect since the SMM
> > refactoring.
> >
> > The old code was:
> >   if (buffer->output[0] == 0)
> >
> > Which was changed to:
> >   ret = dell_send_request(...)
> >   if (ret)
> >
> > However, dell_send_request() will return 0 if buffer->output[0] == 0,
> > so we must change the check to:
> >   if (ret == 0)
> 
> You are right, dell_send_request() return non-zero value on error.
> 
> > This issue was found on a Dell M4800 laptop, and the fix tested on it
> > as well.
> >
> > Fixes: 549b4930f057 ("dell-smbios: Introduce dispatcher for SMM calls")
> > Signed-off-by: Damien Thébault <damien@dtbo.net>
> > Tested-by: Damien Thébault <damien@dtbo.net>
> > ---
> >  drivers/platform/x86/dell-laptop.c | 2 +-
> >  1 file changed, 1 insertion(+), 1 deletion(-)
> >
> > diff --git a/drivers/platform/x86/dell-laptop.c
> > b/drivers/platform/x86/dell-laptop.c index f1fa8612db40..06978c14c83b
> > 100644 --- a/drivers/platform/x86/dell-laptop.c
> > +++ b/drivers/platform/x86/dell-laptop.c
> > @@ -2185,7 +2185,7 @@ static int __init dell_init(void)
> >  		dell_fill_request(&buffer, token->location, 0, 0, 0);
> >  		ret = dell_send_request(&buffer,
> >  					CLASS_TOKEN_READ,
> > SELECT_TOKEN_AC);
> > -		if (ret)
> > +		if (ret == 0)
> >  			max_intensity = buffer.output[3];
> >  	}
> >
> 
> Reviewed-by: Pali Rohár <pali.rohar@gmail.com>
> 

Thanks for this catch.

Reviewed-by: Mario Limonciello <mario.limonciello@dell.com>

This should probably go to stable as well.

Re: [PATCH] dell-laptop: Fix backlight detection

[Darren Hart]

On Sun, Jul 08, 2018 at 05:02:20PM +0200, Damien Thébault wrote:
> Fix return code check for "max brightness" ACPI call.
> 
> The dell laptop ACPI video brightness control is not present on dell
> laptops anymore, but was present in older kernel versions.
> 
> The code that checks the return value is incorrect since the SMM
> refactoring.
> 
> The old code was:
>   if (buffer->output[0] == 0)
> 
> Which was changed to:
>   ret = dell_send_request(...)
>   if (ret)
> 
> However, dell_send_request() will return 0 if buffer->output[0] == 0,
> so we must change the check to:
>   if (ret == 0)
> 
> This issue was found on a Dell M4800 laptop, and the fix tested on it
> as well.
> 
> Fixes: 549b4930f057 ("dell-smbios: Introduce dispatcher for SMM calls")
> Signed-off-by: Damien Thébault <damien@dtbo.net>
> Tested-by: Damien Thébault <damien@dtbo.net>

Thanks for the analysis and fix Damien.

I was unable to apply the patch. Apparently due to your Content-Tranfer-Encoding
being set to "quoted-printable". This converts "=" characters to =3D.
Please work with your mail client and try sending the patch
to yourself and applying it. Once that works, please resend.

I suspect you may need to do one or both of the following:

- Disable preferences->compose->"wrap pasted text" (or whichever applies to how
  you get the patch into the compose window)

- Set Preferences->Mail Handling->Sending->Transfer Encoding to 8bit to avoid
  "quoted-printable".

Other claws users who send patches have Content-Transfer-Encoding set to 7bit

Thanks,

> ---
>  drivers/platform/x86/dell-laptop.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/drivers/platform/x86/dell-laptop.c
> b/drivers/platform/x86/dell-laptop.c index f1fa8612db40..06978c14c83b
> 100644 --- a/drivers/platform/x86/dell-laptop.c
> +++ b/drivers/platform/x86/dell-laptop.c
> @@ -2185,7 +2185,7 @@ static int __init dell_init(void)
>  		dell_fill_request(&buffer, token->location, 0, 0, 0);
>  		ret = dell_send_request(&buffer,
>  					CLASS_TOKEN_READ,
> SELECT_TOKEN_AC);
> -		if (ret)
> +		if (ret == 0)
>  			max_intensity = buffer.output[3];
>  	}
>  
> -- 
> 2.17.1
> 

-- 
Darren Hart
VMware Open Source Technology Center