| Message ID | 1508083394-22823-1-git-send-email-pali.rohar@gmail.com (mailing list archive) |
|---|---|
| State | Superseded, archived |
| Delegated to: | Darren Hart |
| Headers | show |
On Sun, Oct 15, 2017 at 06:03:14PM +0200, 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. Which field is behaving differently? If I'm understanding this correctly: Since the max level is something we test for once at runtime, it seems to me a cleaner fix would be to check for this quirk in kbd_get_info() and set kbd_info.levels accordingly. The rest of the driver logic would then remain unchanged. The idea being to translate what the platform firmware reports into what the driver already understands, instead of giving the levels fields of the structure multiple meanings.
On Monday 16 October 2017 14:48:37 Darren Hart wrote: > On Sun, Oct 15, 2017 at 06:03:14PM +0200, 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. > > Which field is behaving differently? > > If I'm understanding this correctly: > > Since the max level is something we test for once at runtime, it seems > to me a cleaner fix would be to check for this quirk in kbd_get_info() > and set kbd_info.levels accordingly. The rest of the driver logic would > then remain unchanged. Great, this is a good idea. I would send a new patch in a few minutes. > The idea being to translate what the platform firmware reports into what > the driver already understands, instead of giving the levels fields of > the structure multiple meanings.
diff --git a/drivers/platform/x86/dell-laptop.c b/drivers/platform/x86/dell-laptop.c index f42159f..81f14ea 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_num_of_levels_instead_of_last_index; 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_num_of_levels_instead_of_last_index = 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, + }, { } }; @@ -1216,8 +1230,15 @@ static int kbd_get_info(struct kbd_info *info) static unsigned int kbd_get_max_level(void) { - if (kbd_info.levels != 0) - return kbd_info.levels; + u8 levels = kbd_info.levels; + + if (quirks && quirks->kbd_led_num_of_levels_instead_of_last_index) { + if (levels > 1) + return levels - 1; + } else { + if (levels > 0) + return levels; + } if (kbd_mode_levels_count > 0) return kbd_mode_levels_count - 1; return 0;
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 --- Hi! I have not tested this patch yet, but I'm sending it, so other people including Gabriel can test or review it. If you have better idea how to call that quirk variable, then fell free to rename it. I'm not happy with "kbd_led_num_of_levels_instead_of_last_index" but I have nothing better in my mind. --- drivers/platform/x86/dell-laptop.c | 25 +++++++++++++++++++++++-- 1 file changed, 23 insertions(+), 2 deletions(-)