RE: [PATCH v3] dell-laptop: Fix keyboard led max_brightness property for Dell Latitude E6410

RE: [PATCH v3] dell-laptop: Fix keyboard led max_brightness property for Dell Latitude E6410

[Gabriel M. Elder]

-------- Original Message --------
Subject: Re: [PATCH v3] dell-laptop: Fix keyboard led max_brightness
property for Dell Latitude E6410
From: Pali Rohár <pali.rohar@gmail.com>
Date: Sat, November 11, 2017 4:12 pm
To: Darren Hart <dvhart@infradead.org>
Cc: Matthew Garrett <mjg59@srcf.ucam.org>, Andy Shevchenko
<andy@infradead.org>, "Gabriel M. Elder" <gabriel@tekgnowsys.com>,
Gabriele Mazzotta <gabriele.mzt@gmail.com>, Mario.Limonciello@dell.com,
platform-driver-x86@vger.kernel.org, linux-kernel@vger.kernel.org

On Thursday 02 November 2017 18:18:43 Darren Hart wrote:
> On Thu, Nov 02, 2017 at 09:25:24PM +0100, Pali Rohár wrote:
> > This machine reports number of keyboard backlight led levels, instead of
> > value of the last led level index. Therefore max_brightness properly needs
> > to be subtracted by 1 to match led max_brightness API.
> > 
> > Signed-off-by: Pali Rohár <pali.rohar@gmail.com>
> > Reported-by: Gabriel M. Elder <gabriel@tekgnowsys.com>
> > Link: https://bugzilla.kernel.org/show_bug.cgi?id=196913
> > ---
> > Changes since v2:
> > * Rename quirk entry to kbd_led_levels_off_1
> > Changes since v1:
> > * Update kbd_info.levels at initialization time based on quirk
> > ---
> > drivers/platform/x86/dell-laptop.c | 17 +++++++++++++++++
> > 1 file changed, 17 insertions(+)
> > 
> > diff --git a/drivers/platform/x86/dell-laptop.c b/drivers/platform/x86/dell-laptop.c
> > index f42159f..7424e53 100644
> > --- a/drivers/platform/x86/dell-laptop.c
> > +++ b/drivers/platform/x86/dell-laptop.c
> > @@ -49,6 +49,7 @@
> > 
> > struct quirk_entry {
> > u8 touchpad_led;
> > + u8 kbd_led_levels_off_1;
> 
> I believe you and Andy agreed to use a boolean type here?

I'm going to fix this and other entries to boolean type in another
patch.

-- 
Pali Rohár
pali.rohar@gmail.com
-------- End Original Message --------

If necessary, I am willing to test whatever final proposed patch the
community recommends, settles on, and accepts. Would you all like me to
test the v3 patch as well? Or are we calling it good at this point, and
can expect this to be merged into a final stable kernel release sometime
soon? I must admit, I'm looking forward to officially making this bug go
bye-bye!

- Gabriel

Re: [PATCH v3] dell-laptop: Fix keyboard led max_brightness property for Dell Latitude E6410

[Pali Rohár]

On Sunday 12 November 2017 10:01:10 Gabriel M. Elder wrote:
> If necessary, I am willing to test whatever final proposed patch the
> community recommends, settles on, and accepts. Would you all like me to
> test the v3 patch as well?

v3 has just renamed variable, so I do not think that it is needed to
test it again, if you have already tested v2. But of course you can test
v3...

> Or are we calling it good at this point, and
> can expect this to be merged into a final stable kernel release sometime
> soon? I must admit, I'm looking forward to officially making this bug go
> bye-bye!

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

Re: [PATCH v3] dell-laptop: Fix keyboard led max_brightness property for Dell Latitude E6410

[Darren Hart]

On Thu, Nov 02, 2017 at 09:25:24PM +0100, Pali Rohár wrote:
> This machine reports number of keyboard backlight led levels, instead of
> value of the last led level index. Therefore max_brightness properly needs
> to be subtracted by 1 to match led max_brightness API.
> 
> Signed-off-by: Pali Rohár <pali.rohar@gmail.com>
> Reported-by: Gabriel M. Elder <gabriel@tekgnowsys.com>
> Link: https://bugzilla.kernel.org/show_bug.cgi?id=196913


Queued, thanks Pali (sorry for the delay).

-- 
Darren Hart
VMware Open Source Technology Center

Re: [PATCH v3] dell-laptop: Fix keyboard led max_brightness property for Dell Latitude E6410

[Pali Rohár]

On Thursday 02 November 2017 18:18:43 Darren Hart wrote:
> On Thu, Nov 02, 2017 at 09:25:24PM +0100, Pali Rohár wrote:
> > This machine reports number of keyboard backlight led levels, instead of
> > value of the last led level index. Therefore max_brightness properly needs
> > to be subtracted by 1 to match led max_brightness API.
> > 
> > Signed-off-by: Pali Rohár <pali.rohar@gmail.com>
> > Reported-by: Gabriel M. Elder <gabriel@tekgnowsys.com>
> > Link: https://bugzilla.kernel.org/show_bug.cgi?id=196913
> > ---
> > Changes since v2:
> > * Rename quirk entry to kbd_led_levels_off_1
> > Changes since v1:
> > * Update kbd_info.levels at initialization time based on quirk
> > ---
> >  drivers/platform/x86/dell-laptop.c |   17 +++++++++++++++++
> >  1 file changed, 17 insertions(+)
> > 
> > diff --git a/drivers/platform/x86/dell-laptop.c b/drivers/platform/x86/dell-laptop.c
> > index f42159f..7424e53 100644
> > --- a/drivers/platform/x86/dell-laptop.c
> > +++ b/drivers/platform/x86/dell-laptop.c
> > @@ -49,6 +49,7 @@
> >  
> >  struct quirk_entry {
> >  	u8 touchpad_led;
> > +	u8 kbd_led_levels_off_1;
> 
> I believe you and Andy agreed to use a boolean type here?

I'm going to fix this and other entries to boolean type in another
patch.

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

Re: [PATCH v3] dell-laptop: Fix keyboard led max_brightness property for Dell Latitude E6410

[Darren Hart]

On Thu, Nov 02, 2017 at 09:25:24PM +0100, Pali Rohár wrote:
> This machine reports number of keyboard backlight led levels, instead of
> value of the last led level index. Therefore max_brightness properly needs
> to be subtracted by 1 to match led max_brightness API.
> 
> Signed-off-by: Pali Rohár <pali.rohar@gmail.com>
> Reported-by: Gabriel M. Elder <gabriel@tekgnowsys.com>
> Link: https://bugzilla.kernel.org/show_bug.cgi?id=196913
> ---
> Changes since v2:
> * Rename quirk entry to kbd_led_levels_off_1
> Changes since v1:
> * Update kbd_info.levels at initialization time based on quirk
> ---
>  drivers/platform/x86/dell-laptop.c |   17 +++++++++++++++++
>  1 file changed, 17 insertions(+)
> 
> diff --git a/drivers/platform/x86/dell-laptop.c b/drivers/platform/x86/dell-laptop.c
> index f42159f..7424e53 100644
> --- a/drivers/platform/x86/dell-laptop.c
> +++ b/drivers/platform/x86/dell-laptop.c
> @@ -49,6 +49,7 @@
>  
>  struct quirk_entry {
>  	u8 touchpad_led;
> +	u8 kbd_led_levels_off_1;

I believe you and Andy agreed to use a boolean type here?

-- 
Darren Hart
VMware Open Source Technology Center

[PATCH v3] dell-laptop: Fix keyboard led max_brightness property for Dell Latitude E6410

[Pali Rohár]

This machine reports number of keyboard backlight led levels, instead of
value of the last led level index. Therefore max_brightness properly needs
to be subtracted by 1 to match led max_brightness API.

Signed-off-by: Pali Rohár <pali.rohar@gmail.com>
Reported-by: Gabriel M. Elder <gabriel@tekgnowsys.com>
Link: https://bugzilla.kernel.org/show_bug.cgi?id=196913
---
Changes since v2:
* Rename quirk entry to kbd_led_levels_off_1
Changes since v1:
* Update kbd_info.levels at initialization time based on quirk
---
 drivers/platform/x86/dell-laptop.c |   17 +++++++++++++++++
 1 file changed, 17 insertions(+)

diff --git a/drivers/platform/x86/dell-laptop.c b/drivers/platform/x86/dell-laptop.c
index f42159f..7424e53 100644
--- a/drivers/platform/x86/dell-laptop.c
+++ b/drivers/platform/x86/dell-laptop.c
@@ -49,6 +49,7 @@
 
 struct quirk_entry {
 	u8 touchpad_led;
+	u8 kbd_led_levels_off_1;
 
 	int needs_kbd_timeouts;
 	/*
@@ -79,6 +80,10 @@ static int __init dmi_matched(const struct dmi_system_id *dmi)
 	.kbd_timeouts = { 0, 5, 15, 60, 5 * 60, 15 * 60, -1 },
 };
 
+static struct quirk_entry quirk_dell_latitude_e6410 = {
+	.kbd_led_levels_off_1 = 1,
+};
+
 static struct platform_driver platform_driver = {
 	.driver = {
 		.name = "dell-laptop",
@@ -280,6 +285,15 @@ static int __init dmi_matched(const struct dmi_system_id *dmi)
 		},
 		.driver_data = &quirk_dell_xps13_9333,
 	},
+	{
+		.callback = dmi_matched,
+		.ident = "Dell Latitude E6410",
+		.matches = {
+			DMI_MATCH(DMI_SYS_VENDOR, "Dell Inc."),
+			DMI_MATCH(DMI_PRODUCT_NAME, "Latitude E6410"),
+		},
+		.driver_data = &quirk_dell_latitude_e6410,
+	},
 	{ }
 };
 
@@ -1200,6 +1214,9 @@ static int kbd_get_info(struct kbd_info *info)
 	units = (buffer->output[2] >> 8) & 0xFF;
 	info->levels = (buffer->output[2] >> 16) & 0xFF;
 
+	if (quirks && quirks->kbd_led_levels_off_1 && info->levels)
+		info->levels--;
+
 	if (units & BIT(0))
 		info->seconds = (buffer->output[3] >> 0) & 0xFF;
 	if (units & BIT(1))
-- 
1.7.9.5