On Mon, 2018-06-04 at 21:52 +0000, Mario.Limonciello@xxxxxxxx wrote: > > -----Original Message----- > > From: Timur Kristóf [mailto:timur.kristof@xxxxxxxxx] > > Sent: Thursday, May 31, 2018 12:14 PM > > To: Pali Rohár > > Cc: Andy Shevchenko; Limonciello, Mario; Platform Driver; Timur > > Kristóf; Matthew > > Garrett; Darren Hart; Andy Shevchenko > > Subject: Re: [PATCH] Fix AC keyboard backlight timeout on Dell XPS > > 13 9370. > > > > On Thu, 2018-05-31 at 14:50 +0200, Pali Rohár wrote: > > > On Thursday 31 May 2018 14:35:45 Timur Kristóf wrote: > > > > 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.krist > > > > > > of@g > > > > > > mail > > > > > > .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. > > > > > > Can you try to report this problem to Dell support (if possible)? > > > Really > > > if Dell is interested in good Linux support (which looks like > > > yes), > > > then > > > Dell should know that has a bug in BIOS/firmware which is causing > > > problems on Linux. > > > > > > For me it sounds stupid to adding hacks into kernel which just > > > due to > > > firmware bugs about which vendor does not know. > > > > > > As a short term fix for one laptop it is OK, but not if Dell > > > starts > > > producing e.g. new generation of all laptops with same bug. Long > > > term > > > fix is for sure in BIOS/firmware. > > > > > > I already wrote this to above github issue. > > > > > > > Thank you Pali. I agree with your point. Let's hope they will fix > > it in > > the 9380. In the meantime I think it doesn't hurt to add the patch > > for > > people who use the 9370 now. > > I of course can't make any guarantees in what bugs are introduced in > new generations of hardware, but I would say that it's at least > understood > why this behavior occurs on the 9370. I hope it should be fixed in a > future > firmware version but I also can not guarantee that. > > I think that v2 of this patch looks fine, but I would like to ask > Timur that if > a firmware is released that fixes this behavior please amend with a > follow up patch > that restricts the quirk only to the applicable firmware (or drop the > quirk). Thank you Mario! Sure, it makes sense to restrict the quirk to firmware which is known to be buggy. I will be happy to do that when the issue is fixed by new firmware update! In the meantime I've been running 4.17-rc7 with this patch added, and it works well for me. Though not sure if this could still make it into 4.17, but would be nice! Best regards, Timur
