On Thu, 2018-05-31 at 14:22 +0200, Pali Rohár wrote: > 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@gmail > > .com> 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). Discussed with Mario here: https://github.com/dell/libsmbios/issues/48 The conclusion is that this is a BIOS bug. The machine is capable of changing the AC keyboard backlight timeout setting, it is just missing the token which indicates the capability. > > > > 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. Ok. How about kbd_missing_ac_tag? > > > > > > 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. Ok. > And maybe change order of ||. If we know that token is buggy on some > machines then we probably should not ask for it. Ok. > > > > kbd_timeout_ac_supported = true; > > > > > > kbd_get_state(&state); > > > -- > > > 2.17.0 > > > > > > > > > > >
