Groups | Search | Server Info | Keyboard shortcuts | Login | Register [http] [https] [nntp] [nntps]


Groups > linux.kernel > #1269500 > unrolled thread

Re: [PATCH 1/3] dell_wmi: Support new hotkeys on the XPS 13 Skylake

Started byAndy Lutomirski <luto@amacapital.net>
First post2015-11-14 16:50 +0100
Last post2015-11-14 17:20 +0100
Articles 2 — 2 participants

Back to article view | Back to linux.kernel

This discussion starts older than the indexed window; earlier articles aren't shown. The article labeled Started by below is the oldest one visible, not the original post.


Contents

  Re: [PATCH 1/3] dell_wmi: Support new hotkeys on the XPS 13 Skylake Andy Lutomirski <luto@amacapital.net> - 2015-11-14 16:50 +0100
    Re: [PATCH 1/3] dell_wmi: Support new hotkeys on the XPS 13 Skylake Pali Rohár <pali.rohar@gmail.com> - 2015-11-14 17:20 +0100

#1269500 — Re: [PATCH 1/3] dell_wmi: Support new hotkeys on the XPS 13 Skylake

FromAndy Lutomirski <luto@amacapital.net>
Date2015-11-14 16:50 +0100
SubjectRe: [PATCH 1/3] dell_wmi: Support new hotkeys on the XPS 13 Skylake
Message-ID<quLfr-5Qi-5@gated-at.bofh.it>
On Nov 14, 2015 1:27 AM, "Pali Rohár" <pali.rohar@gmail.com> wrote:
>
> On Friday 13 November 2015 21:49:30 Andy Lutomirski wrote:
> > The XPS 13 Skylake has an rfkill button and a switchvideomode button
> > that aren't enumerated in the DMI table AFAICT.  Add a table listing
> > extra un-enumerated hotkeys.  To avoid breaking things that worked
> > before, these un-enumerated hotkeys won't be used if the DMI table
> > maps them to something else.
> >
>
> Do you have any (Dell) documentation which specify list of these wmi
> codes send to dell-wmi driver?
>

No.  Do you know where to get that documentation?

> > This also adds the Fn-lock key as a KE_IGNORE entry.
> >
> > Signed-off-by: Andy Lutomirski <luto@kernel.org>
> > ---
> >  drivers/platform/x86/dell-wmi.c | 48 +++++++++++++++++++++++++++++++++++------
> >  1 file changed, 41 insertions(+), 7 deletions(-)
> >
> > diff --git a/drivers/platform/x86/dell-wmi.c b/drivers/platform/x86/dell-wmi.c
> > index f2d77fe696ac..5be1abec4f64 100644
> > --- a/drivers/platform/x86/dell-wmi.c
> > +++ b/drivers/platform/x86/dell-wmi.c
> > @@ -142,6 +142,16 @@ static const u16 bios_to_linux_keycode[256] __initconst = {
> >       0, 0, 0, 0, 0, 0, 0, 0, 0, KEY_PROG3
> >  };
> >
> > +/* These are applied if the hk table is present and doesn't override them. */
> > +static const struct key_entry dell_wmi_extra_keymap[] __initconst = {
> > +     /* Fn-lock -- no action is required by the kernel. */
> > +     { KE_IGNORE, 0x151, { KEY_RESERVED } },
> > +
> > +     /* Keys that need our help (on XPS 13 Skylake and maybe others. */
> > +     { KE_KEY, 0x152, { KEY_SWITCHVIDEOMODE } },
> > +     { KE_KEY, 0x153, { KEY_RFKILL } },
>
> On more Dell laptops rfkill events are handed by ACPI driver
> dell-rbtn.ko. Are you sure that dell-rbtn.ko does not send keypress
> event and you really need it from dell-wmi? We already masked KEY_RFKILL
> in dell-wmi to prevent double events...
>

Hmm, interesting.  I have DELLABC6, not DELLABCE.  I'll play around
with it a bit.  Are there Dell docs for this?

Regardless, we'll need something like this, but maybe with KE_IGNORE,
just to silence the warnings.

> > +
> > +             /*
> > +              * Check if we've already found this scancode.  This takes
> > +              * quadratic time, but it doesn't matter unless the list
> > +              * of extra keys gets very long.
> > +              */
> > +             for (j = 0; j < num_bios_keys; j++)
> > +                     if (keymap[j].code == dell_wmi_extra_keymap[i].code)
> > +                             goto skip;
>
> Rather move this code into separate boolean function and for return
> value here. This will prevent using hacky goto...
>

I'll do that in v2.

--Andy
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

[toc] | [next] | [standalone]


#1269502

FromPali Rohár <pali.rohar@gmail.com>
Date2015-11-14 17:20 +0100
Message-ID<quLIt-6gp-3@gated-at.bofh.it>
In reply to#1269500

[Multipart message — attachments visible in raw view] — view raw

On Saturday 14 November 2015 16:48:25 Andy Lutomirski wrote:
> On Nov 14, 2015 1:27 AM, "Pali Rohár" <pali.rohar@gmail.com> wrote:
> > On Friday 13 November 2015 21:49:30 Andy Lutomirski wrote:
> > > The XPS 13 Skylake has an rfkill button and a switchvideomode
> > > button that aren't enumerated in the DMI table AFAICT.  Add a
> > > table listing extra un-enumerated hotkeys.  To avoid breaking
> > > things that worked before, these un-enumerated hotkeys won't be
> > > used if the DMI table maps them to something else.
> > 
> > Do you have any (Dell) documentation which specify list of these
> > wmi codes send to dell-wmi driver?
> 
> No.  Do you know where to get that documentation?
> 

Time to time Dell release some documentation or example code. You could 
ask Dell people on LKML (e.g. Mario Limonciello is active) or on smbios 
mailing list libsmbios-devel@lists.us.dell.com.

But currently there there are open questions about WMI hotkeys on Dell 
Vostro V131 which we cannot fix yet :-(

> > > This also adds the Fn-lock key as a KE_IGNORE entry.
> > > 
> > > Signed-off-by: Andy Lutomirski <luto@kernel.org>
> > > ---
> > > 
> > >  drivers/platform/x86/dell-wmi.c | 48
> > >  +++++++++++++++++++++++++++++++++++------ 1 file changed, 41
> > >  insertions(+), 7 deletions(-)
> > > 
> > > diff --git a/drivers/platform/x86/dell-wmi.c
> > > b/drivers/platform/x86/dell-wmi.c index
> > > f2d77fe696ac..5be1abec4f64 100644
> > > --- a/drivers/platform/x86/dell-wmi.c
> > > +++ b/drivers/platform/x86/dell-wmi.c
> > > @@ -142,6 +142,16 @@ static const u16 bios_to_linux_keycode[256]
> > > __initconst = {
> > > 
> > >       0, 0, 0, 0, 0, 0, 0, 0, 0, KEY_PROG3
> > >  
> > >  };
> > > 
> > > +/* These are applied if the hk table is present and doesn't
> > > override them. */ +static const struct key_entry
> > > dell_wmi_extra_keymap[] __initconst = { +     /* Fn-lock -- no
> > > action is required by the kernel. */ +     { KE_IGNORE, 0x151, {
> > > KEY_RESERVED } },
> > > +
> > > +     /* Keys that need our help (on XPS 13 Skylake and maybe
> > > others. */ +     { KE_KEY, 0x152, { KEY_SWITCHVIDEOMODE } },
> > > +     { KE_KEY, 0x153, { KEY_RFKILL } },
> > 
> > On more Dell laptops rfkill events are handed by ACPI driver
> > dell-rbtn.ko. Are you sure that dell-rbtn.ko does not send keypress
> > event and you really need it from dell-wmi? We already masked
> > KEY_RFKILL in dell-wmi to prevent double events...
> 
> Hmm, interesting.  I have DELLABC6, not DELLABCE.  I'll play around
> with it a bit.  Are there Dell docs for this?
> 

Decompiling ACPI table is documentation :-) Probably DELLABC6 will have 
similar (or maybe same) ACPI interface. Post relevant ACPI ASL code, 
maybe I could help with it.

Anyway first version of DELLABCE driver was written by Alex Hung (from 
Canonical), so it is possible that also non-Dell could help with 
documentation/behaviour as well.

> Regardless, we'll need something like this, but maybe with KE_IGNORE,
> just to silence the warnings.
> 

Yes, with KE_IGNORE we will have documented behaviour in code.

> > > +
> > > +             /*
> > > +              * Check if we've already found this scancode. 
> > > This takes +              * quadratic time, but it doesn't
> > > matter unless the list +              * of extra keys gets very
> > > long.
> > > +              */
> > > +             for (j = 0; j < num_bios_keys; j++)
> > > +                     if (keymap[j].code ==
> > > dell_wmi_extra_keymap[i].code) +                            
> > > goto skip;
> > 
> > Rather move this code into separate boolean function and for return
> > value here. This will prevent using hacky goto...
> 
> I'll do that in v2.
> 
> --Andy

OK.

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

[toc] | [prev] | [standalone]


Back to top | Article view | linux.kernel


csiph-web