On Thursday 31 May 2018 15:05:39 Andy Shevchenko wrote: > +Cc: Mario > > On Thu, May 24, 2018 at 9:07 PM, Timur Kristóf <timur.kristof@xxxxxxxxx> wrote: > > From: Timur Kristóf <venemo@xxxxxxxxxxxxxxxxx> > > > > The 9370 doesn't expose the necessary KBD_LED_AC_TOKEN in the > > BIOS, so the driver thinks it cannot adjust the AC keyboard > > backlight timeout. This patch adds a quirk to fix this until > > Dell adds the missing token to their BIOS. > > > > For further discussion, see: > > https://github.com/dell/libsmbios/issues/48 > > > > The change per se looks good to me, though I would hear back from Pali > and / or Mario on the subject. > Their formal Ack would be enough. I do not know what is happening here. So Mario should comment if this is really a BIOS bug (and we need to workaround it by DMI whitelisting) or kernel has incorrect implementation (and different fix is needed). > > Signed-off-by: Timur Kristóf <venemo@xxxxxxxxxxxxxxxxx> > > --- > > drivers/platform/x86/dell-laptop.c | 16 +++++++++++++++- > > 1 file changed, 15 insertions(+), 1 deletion(-) > > > > diff --git a/drivers/platform/x86/dell-laptop.c b/drivers/platform/x86/dell-laptop.c > > index c52c6723374b..058944258161 100644 > > --- a/drivers/platform/x86/dell-laptop.c > > +++ b/drivers/platform/x86/dell-laptop.c > > @@ -38,6 +38,7 @@ > > struct quirk_entry { > > bool touchpad_led; > > bool kbd_led_levels_off_1; > > + bool support_kbd_timeout_ac_missing_tag; > > I would rather try to keep kbd_ prefix and perhaps somehow make the > name shorter. > > > > > bool needs_kbd_timeouts; > > /* > > @@ -68,6 +69,10 @@ static struct quirk_entry quirk_dell_xps13_9333 = { > > .kbd_timeouts = { 0, 5, 15, 60, 5 * 60, 15 * 60, -1 }, > > }; > > > > +static struct quirk_entry quirk_dell_xps13_9370 = { > > + .support_kbd_timeout_ac_missing_tag = true, > > +}; > > + > > static struct quirk_entry quirk_dell_latitude_e6410 = { > > .kbd_led_levels_off_1 = true, > > }; > > @@ -291,6 +296,15 @@ static const struct dmi_system_id dell_quirks[] __initconst = { > > }, > > .driver_data = &quirk_dell_xps13_9333, > > }, > > + { > > + .callback = dmi_matched, > > + .ident = "Dell XPS 13 9370", > > + .matches = { > > + DMI_MATCH(DMI_SYS_VENDOR, "Dell Inc."), > > + DMI_MATCH(DMI_PRODUCT_NAME, "XPS 13 9370"), > > + }, > > + .driver_data = &quirk_dell_xps13_9370, > > + }, > > { > > .callback = dmi_matched, > > .ident = "Dell Latitude E6410", > > @@ -1401,7 +1415,7 @@ static inline int kbd_init_info(void) > > * timeout value which is shared for both battery and AC power > > * settings. So do not try to set AC values on old models. > > */ > > - if (dell_smbios_find_token(KBD_LED_AC_TOKEN)) > > > + if (dell_smbios_find_token(KBD_LED_AC_TOKEN) || (quirks && quirks->support_kbd_timeout_ac_missing_tag)) > > Two lines, please. And maybe change order of ||. If we know that token is buggy on some machines then we probably should not ask for it. > > kbd_timeout_ac_supported = true; > > > > kbd_get_state(&state); > > -- > > 2.17.0 > > > > > -- Pali Rohár pali.rohar@xxxxxxxxx
