dell-laptop: Fix backlight detection

PATCH In review Damien Thébault 4 participants 4 messages Jul 8, 2018 – Jul 9, 2018 x86 platform drivers

lore

Share & tools

lore

Damien Thébault

author

2018-07-08 15:02 UTC #0 lore raw

PATCH 1/1 dell-laptop: Fix backlight detection

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 <[email protected]>

Tested-by Damien Thébault <[email protected]>

---
 drivers/platform/x86/dell-laptop.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

(quoted diff) +0 -0

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];
}

Signature · 1 line

2.17.1

Pali Rohár

Reviewed-by on v1 1/1

2018-07-08 15:09 UTC #1 lore raw

Quoted 11 lines · 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)

Quoted 7 lines

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.

Quoted 16 lines

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 <[email protected]>
Tested-by Damien Thébault <[email protected]>
---
 drivers/platform/x86/dell-laptop.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)
(quoted diff) +0 -0
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

Review static int __init dell_init(void) · line 2191

Quoted 8 lines

(quoted diff) +1 -1

@@ -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 <[email protected]>

Signature · 2 lines

Pali Rohár
[email protected]

[email protected]

Reviewed-by

2018-07-09 04:14 UTC #2 lore raw

Quoted 40 lines

-----Original Message-----
From: Pali Rohár [mailto:[email protected]]
Sent: Sunday, July 8, 2018 8:09 AM
To: Damien Thébault
Cc: Matthew Garrett; Darren Hart; Andy Shevchenko; Limonciello, Mario; platform- [email protected]; [email protected]
Subject: Re: [PATCH] dell-laptop: Fix backlight detection
Quoted 11 lines · 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)
Quoted 7 lines
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.
Quoted 10 lines
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 <[email protected]>
Tested-by Damien Thébault <[email protected]>
---
 drivers/platform/x86/dell-laptop.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

Quoted 17 lines

Quoted 6 lines
(quoted diff) +0 -0
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
Review static int __init dell_init(void) · line 2191
Quoted 8 lines
(quoted diff) +1 -1
@@ -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 <[email protected]>

Thanks for this catch.

Reviewed-by Mario Limonciello <[email protected]>

This should probably go to stable as well.

Darren Hart

2018-07-09 21:57 UTC #3 lore raw

In reply to Damien Thébault

Quoted 19 lines · 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)

Quoted 6 lines

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 <[email protected]>
Tested-by Damien Thébault <[email protected]>

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,

Quoted 21 lines

---
 drivers/platform/x86/dell-laptop.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)
(quoted diff) +0 -0
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
(quoted diff) +1 -1
@@ -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];
 	}
Signature · 2 lines
-- 
2.17.1

Signature · 2 lines

Darren Hart
VMware Open Source Technology Center