diff mbox

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

Message ID db83b63cc163ecc640d5d1c5554f8aa87beaadfc.1447479930.git.luto@kernel.org (mailing list archive)
State Changes Requested, archived
Headers show

Commit Message

Andy Lutomirski Nov. 14, 2015, 5:49 a.m. UTC
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.

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(-)

Comments

Pali Rohár Nov. 14, 2015, 9:27 a.m. UTC | #1
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?

> 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...

> +};
> +
>  static struct input_dev *dell_wmi_input_dev;
>  
>  static void dell_wmi_process_key(int reported_key)
> @@ -300,9 +310,10 @@ static const struct key_entry * __init dell_wmi_prepare_new_keymap(void)
>  	int hotkey_num = (dell_bios_hotkey_table->header.length - 4) /
>  				sizeof(struct dell_bios_keymap_entry);
>  	struct key_entry *keymap;
> -	int i;
> +	int i, pos = 0, num_bios_keys;
>  
> -	keymap = kcalloc(hotkey_num + 1, sizeof(struct key_entry), GFP_KERNEL);
> +	keymap = kcalloc(hotkey_num + ARRAY_SIZE(dell_wmi_extra_keymap),
> +			 sizeof(struct key_entry), GFP_KERNEL);
>  	if (!keymap)
>  		return NULL;
>  
> @@ -314,14 +325,37 @@ static const struct key_entry * __init dell_wmi_prepare_new_keymap(void)
>  				    KEY_RESERVED;
>  
>  		if (keycode == KEY_KBDILLUMTOGGLE)
> -			keymap[i].type = KE_IGNORE;
> +			keymap[pos].type = KE_IGNORE;
>  		else
> -			keymap[i].type = KE_KEY;
> -		keymap[i].code = bios_entry->scancode;
> -		keymap[i].keycode = keycode;
> +			keymap[pos].type = KE_KEY;
> +		keymap[pos].code = bios_entry->scancode;
> +		keymap[pos].keycode = keycode;
> +
> +		pos++;
> +	}
> +
> +	num_bios_keys = pos;
> +
> +	for (i = 0; i < ARRAY_SIZE(dell_wmi_extra_keymap); i++) {
> +		int j;
> +
> +		/*
> +		 * 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...

> +
> +		keymap[pos] = dell_wmi_extra_keymap[i];
> +		pos++;
> +
> +skip:
> +		;
>  	}
>  
> -	keymap[hotkey_num].type = KE_END;
> +	keymap[pos].type = KE_END;
>  
>  	return keymap;
>  }
Andy Lutomirski Nov. 14, 2015, 3:48 p.m. UTC | #2
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 platform-driver-x86" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Pali Rohár Nov. 14, 2015, 4:13 p.m. UTC | #3
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 Nov. 17, 2015, 8:36 a.m. UTC | #4
On Saturday 14 November 2015 18:07:57 Andy Lutomirski wrote:
> [lots of people added]
> 
> On Sat, Nov 14, 2015 at 8:13 AM, Pali Rohár <pali.rohar@gmail.com>
> wrote:
> > 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 :-(
> 
> On the Dell XPS 13 Skylake (XPS 13 9350), upstream Linux doesn't
> support the rfkill button.
> 
> There seem to be three WMI events that aren't reflected in the OEM
> type 178 DMI table:
> 
> 0x151: Fn-lock (no action needed by OS)
> 0x152: Switch video mode (should send KEY_SWITCHVIDEOMODE, but
> currently just warns)
> 0x153: rfkill -- currently warns, and see below.
> 
> On several Dell models, there's the dell_rbtn (DELRBTN / DELLABCE)
> device.  It's here in the DSDT, too, but it seems to be disabled if
> _OSI reports "Windows 2012" or "Windows 2013", so _STA returns zero.
> (It also shows up as DELLRBC6, but I haven't tried all the _OSI
> hackery that seems to be needed in order to test the driver.)
> 

Hi!

In your ASL code is:

            Method (_STA, 0, NotSerialized)  // _STA: Status
            {
                If ((OIDE () < One))
                {
                    Return (0x0F)
                }

                Return (Zero)
            }

OIDE() returns 1 for Windows 8.

This is quite interesting, on my Latitude E6440 is this ASL code:

            Method (_STA, 0, NotSerialized)
            {
                If (LLess (OIDE (), One))
                {
                    Return (Zero)
                }

                Return (0x0F)
            }

And again OIDE() returns 1 for Windows 8. So behaviour is negated.

Can you check if you have latest version of BIOS? Maybe Dell written 
that condition incorrectly?

Can you try to add "DELLRBC6" into dell-rbtn.c acpi table and boot 
kernel with acpi_osi="!Windows 2012" acpi_osi="!Windows 2013" what 
happens?

> My proposal is to modify dell_wmi to handle 0x151 (ignore), 0x152
> (send KEY_SWITCHVIDEOMODE), and 0x153 (send KEY_RFKILL), but only if
> there isn't something else mapped to them in the DMI table.
> 
> I've attached dmidecode output and the DSDT.
> 

Can you please provide debug output from dell-wmi module when you press 
those hotkeys? Specially I want to see wmi buffer for each pressed 
hotkey. In debug dmesg it starts with "Received WMI event" and "Process 
buffer". Look into dell-wmi.c source code.
Andy Lutomirski Nov. 17, 2015, 7:03 p.m. UTC | #5
On Nov 17, 2015 12:36 AM, "Pali Rohár" <pali.rohar@gmail.com> wrote:
>
> On Saturday 14 November 2015 18:07:57 Andy Lutomirski wrote:
> > [lots of people added]
> >
> > On Sat, Nov 14, 2015 at 8:13 AM, Pali Rohár <pali.rohar@gmail.com>
> > wrote:
> > > 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 :-(
> >
> > On the Dell XPS 13 Skylake (XPS 13 9350), upstream Linux doesn't
> > support the rfkill button.
> >
> > There seem to be three WMI events that aren't reflected in the OEM
> > type 178 DMI table:
> >
> > 0x151: Fn-lock (no action needed by OS)
> > 0x152: Switch video mode (should send KEY_SWITCHVIDEOMODE, but
> > currently just warns)
> > 0x153: rfkill -- currently warns, and see below.
> >
> > On several Dell models, there's the dell_rbtn (DELRBTN / DELLABCE)
> > device.  It's here in the DSDT, too, but it seems to be disabled if
> > _OSI reports "Windows 2012" or "Windows 2013", so _STA returns zero.
> > (It also shows up as DELLRBC6, but I haven't tried all the _OSI
> > hackery that seems to be needed in order to test the driver.)
> >
>
> Hi!
>
> In your ASL code is:
>
>             Method (_STA, 0, NotSerialized)  // _STA: Status
>             {
>                 If ((OIDE () < One))
>                 {
>                     Return (0x0F)
>                 }
>
>                 Return (Zero)
>             }
>
> OIDE() returns 1 for Windows 8.
>
> This is quite interesting, on my Latitude E6440 is this ASL code:
>
>             Method (_STA, 0, NotSerialized)
>             {
>                 If (LLess (OIDE (), One))
>                 {
>                     Return (Zero)
>                 }
>
>                 Return (0x0F)
>             }
>
> And again OIDE() returns 1 for Windows 8. So behaviour is negated.
>
> Can you check if you have latest version of BIOS? Maybe Dell written
> that condition incorrectly?
>

I'll check later on.  There's one newer version.

> Can you try to add "DELLRBC6" into dell-rbtn.c acpi table and boot
> kernel with acpi_osi="!Windows 2012" acpi_osi="!Windows 2013" what
> happens?
>

I did exactly that.  The dell_rbtn driver partially worked, but, when
I pushed the rfkill button to turn off wireless and then turned it
back on using NetworkManager's menu, NM thought it was back on but it
didn't seem to work until I pushed the rfkill button again.  dell-rbtn
does very strange things with device creation and deletion, and maybe
that's related.

> > My proposal is to modify dell_wmi to handle 0x151 (ignore), 0x152
> > (send KEY_SWITCHVIDEOMODE), and 0x153 (send KEY_RFKILL), but only if
> > there isn't something else mapped to them in the DMI table.
> >
> > I've attached dmidecode output and the DSDT.
> >
>
> Can you please provide debug output from dell-wmi module when you press
> those hotkeys? Specially I want to see wmi buffer for each pressed
> hotkey. In debug dmesg it starts with "Received WMI event" and "Process
> buffer". Look into dell-wmi.c source code.

It looks like this:

[ 7822.601910] dell_wmi: Received WMI event (02 00 10 00 53 01 00 00
00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
00 00 00 00 00 00 00 00 00 00)
[ 7822.601913] dell_wmi: Process buffer (02 00 10 00 53 01)
[ 7822.601915] dell_wmi: Key 153 pressed

--Andy
--
To unsubscribe from this list: send the line "unsubscribe platform-driver-x86" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Andy Lutomirski Nov. 18, 2015, 3:44 a.m. UTC | #6
On Tue, Nov 17, 2015 at 11:03 AM, Andy Lutomirski <luto@amacapital.net> wrote:
> On Nov 17, 2015 12:36 AM, "Pali Rohár" <pali.rohar@gmail.com> wrote:
>>
>> On Saturday 14 November 2015 18:07:57 Andy Lutomirski wrote:
>> > [lots of people added]
>> >
>> > On Sat, Nov 14, 2015 at 8:13 AM, Pali Rohár <pali.rohar@gmail.com>
>> > wrote:
>> > > 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 :-(
>> >
>> > On the Dell XPS 13 Skylake (XPS 13 9350), upstream Linux doesn't
>> > support the rfkill button.
>> >
>> > There seem to be three WMI events that aren't reflected in the OEM
>> > type 178 DMI table:
>> >
>> > 0x151: Fn-lock (no action needed by OS)
>> > 0x152: Switch video mode (should send KEY_SWITCHVIDEOMODE, but
>> > currently just warns)
>> > 0x153: rfkill -- currently warns, and see below.
>> >
>> > On several Dell models, there's the dell_rbtn (DELRBTN / DELLABCE)
>> > device.  It's here in the DSDT, too, but it seems to be disabled if
>> > _OSI reports "Windows 2012" or "Windows 2013", so _STA returns zero.
>> > (It also shows up as DELLRBC6, but I haven't tried all the _OSI
>> > hackery that seems to be needed in order to test the driver.)
>> >
>>
>> Hi!
>>
>> In your ASL code is:
>>
>>             Method (_STA, 0, NotSerialized)  // _STA: Status
>>             {
>>                 If ((OIDE () < One))
>>                 {
>>                     Return (0x0F)
>>                 }
>>
>>                 Return (Zero)
>>             }
>>
>> OIDE() returns 1 for Windows 8.
>>
>> This is quite interesting, on my Latitude E6440 is this ASL code:
>>
>>             Method (_STA, 0, NotSerialized)
>>             {
>>                 If (LLess (OIDE (), One))
>>                 {
>>                     Return (Zero)
>>                 }
>>
>>                 Return (0x0F)
>>             }
>>
>> And again OIDE() returns 1 for Windows 8. So behaviour is negated.
>>
>> Can you check if you have latest version of BIOS? Maybe Dell written
>> that condition incorrectly?
>>
>
> I'll check later on.  There's one newer version.

No relevant changes in 1.0.4.

--Andy
--
To unsubscribe from this list: send the line "unsubscribe platform-driver-x86" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Andy Lutomirski Nov. 18, 2015, 9:24 p.m. UTC | #7
On Fri, Nov 13, 2015 at 9:49 PM, Andy Lutomirski <luto@kernel.org> 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.

Consider this withdrawn entirely for now.  Apparently there's an
entirely new mechanism for this in the era of Windows 10.

--Andy

>
> 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 } },
> +};
> +
>  static struct input_dev *dell_wmi_input_dev;
>
>  static void dell_wmi_process_key(int reported_key)
> @@ -300,9 +310,10 @@ static const struct key_entry * __init dell_wmi_prepare_new_keymap(void)
>         int hotkey_num = (dell_bios_hotkey_table->header.length - 4) /
>                                 sizeof(struct dell_bios_keymap_entry);
>         struct key_entry *keymap;
> -       int i;
> +       int i, pos = 0, num_bios_keys;
>
> -       keymap = kcalloc(hotkey_num + 1, sizeof(struct key_entry), GFP_KERNEL);
> +       keymap = kcalloc(hotkey_num + ARRAY_SIZE(dell_wmi_extra_keymap),
> +                        sizeof(struct key_entry), GFP_KERNEL);
>         if (!keymap)
>                 return NULL;
>
> @@ -314,14 +325,37 @@ static const struct key_entry * __init dell_wmi_prepare_new_keymap(void)
>                                     KEY_RESERVED;
>
>                 if (keycode == KEY_KBDILLUMTOGGLE)
> -                       keymap[i].type = KE_IGNORE;
> +                       keymap[pos].type = KE_IGNORE;
>                 else
> -                       keymap[i].type = KE_KEY;
> -               keymap[i].code = bios_entry->scancode;
> -               keymap[i].keycode = keycode;
> +                       keymap[pos].type = KE_KEY;
> +               keymap[pos].code = bios_entry->scancode;
> +               keymap[pos].keycode = keycode;
> +
> +               pos++;
> +       }
> +
> +       num_bios_keys = pos;
> +
> +       for (i = 0; i < ARRAY_SIZE(dell_wmi_extra_keymap); i++) {
> +               int j;
> +
> +               /*
> +                * 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;
> +
> +               keymap[pos] = dell_wmi_extra_keymap[i];
> +               pos++;
> +
> +skip:
> +               ;
>         }
>
> -       keymap[hotkey_num].type = KE_END;
> +       keymap[pos].type = KE_END;
>
>         return keymap;
>  }
> --
> 2.5.0
>
Pali Rohár Nov. 19, 2015, 8:19 a.m. UTC | #8
On Wednesday 18 November 2015 13:24:33 Andy Lutomirski wrote:
> On Fri, Nov 13, 2015 at 9:49 PM, Andy Lutomirski <luto@kernel.org> 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.
> 
> Consider this withdrawn entirely for now.  Apparently there's an
> entirely new mechanism for this in the era of Windows 10.
> 

Can you provide more information? I'm planning to cleanup dell-wmi.c
driver and if there are needed changes for Windows 10, please let me
know what... Thanks!
Darren Hart Nov. 21, 2015, 12:06 a.m. UTC | #9
On Fri, Nov 13, 2015 at 09:49:30PM -0800, 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.
> 
> 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 } },
> +};
> +
>  static struct input_dev *dell_wmi_input_dev;
>  
>  static void dell_wmi_process_key(int reported_key)
> @@ -300,9 +310,10 @@ static const struct key_entry * __init dell_wmi_prepare_new_keymap(void)
>  	int hotkey_num = (dell_bios_hotkey_table->header.length - 4) /
>  				sizeof(struct dell_bios_keymap_entry);
>  	struct key_entry *keymap;
> -	int i;
> +	int i, pos = 0, num_bios_keys;

Just a nit, "reverse christmas tree" order (longest line length first) please.
(only if you resend after this review for other reasons)

>  
> -	keymap = kcalloc(hotkey_num + 1, sizeof(struct key_entry), GFP_KERNEL);
> +	keymap = kcalloc(hotkey_num + ARRAY_SIZE(dell_wmi_extra_keymap),

This previously allocated kotkey_num + 1, but you dropeed the +1, making it
eactly the size of hotkey_num + the new entries you added.

Why don't we need the +1 anymore? (it isn't clear to me why we needed to before
actually, but I want to confirm you considered it).

Thanks,

> +			 sizeof(struct key_entry), GFP_KERNEL);
>  	if (!keymap)
>  		return NULL;
>  
> @@ -314,14 +325,37 @@ static const struct key_entry * __init dell_wmi_prepare_new_keymap(void)
>  				    KEY_RESERVED;
>  
>  		if (keycode == KEY_KBDILLUMTOGGLE)
> -			keymap[i].type = KE_IGNORE;
> +			keymap[pos].type = KE_IGNORE;
>  		else
> -			keymap[i].type = KE_KEY;
> -		keymap[i].code = bios_entry->scancode;
> -		keymap[i].keycode = keycode;
> +			keymap[pos].type = KE_KEY;
> +		keymap[pos].code = bios_entry->scancode;
> +		keymap[pos].keycode = keycode;
> +
> +		pos++;
> +	}
> +
> +	num_bios_keys = pos;
> +
> +	for (i = 0; i < ARRAY_SIZE(dell_wmi_extra_keymap); i++) {
> +		int j;
> +
> +		/*
> +		 * 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;
> +
> +		keymap[pos] = dell_wmi_extra_keymap[i];
> +		pos++;
> +
> +skip:
> +		;
>  	}
>  
> -	keymap[hotkey_num].type = KE_END;
> +	keymap[pos].type = KE_END;
>  
>  	return keymap;
>  }
> -- 
> 2.5.0
> 
> --
> To unsubscribe from this list: send the line "unsubscribe platform-driver-x86" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html
>
Pali Rohár Nov. 21, 2015, 12:11 a.m. UTC | #10
On Saturday 21 November 2015 01:06:36 Darren Hart wrote:
> > -	keymap = kcalloc(hotkey_num + 1, sizeof(struct key_entry),
> > GFP_KERNEL); +	keymap = kcalloc(hotkey_num +
> > ARRAY_SIZE(dell_wmi_extra_keymap),
> 
> This previously allocated kotkey_num + 1, but you dropeed the +1,
> making it eactly the size of hotkey_num + the new entries you added.
> 
> Why don't we need the +1 anymore? (it isn't clear to me why we needed
> to before actually, but I want to confirm you considered it).
> 

Last entry in keymap array must be KE_END, right? So this is that +1.
Andy Lutomirski Nov. 21, 2015, 12:12 a.m. UTC | #11
On Fri, Nov 20, 2015 at 4:06 PM, Darren Hart <dvhart@infradead.org> wrote:
> On Fri, Nov 13, 2015 at 09:49:30PM -0800, 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.
>>
>> 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 } },
>> +};
>> +
>>  static struct input_dev *dell_wmi_input_dev;
>>
>>  static void dell_wmi_process_key(int reported_key)
>> @@ -300,9 +310,10 @@ static const struct key_entry * __init dell_wmi_prepare_new_keymap(void)
>>       int hotkey_num = (dell_bios_hotkey_table->header.length - 4) /
>>                               sizeof(struct dell_bios_keymap_entry);
>>       struct key_entry *keymap;
>> -     int i;
>> +     int i, pos = 0, num_bios_keys;
>
> Just a nit, "reverse christmas tree" order (longest line length first) please.
> (only if you resend after this review for other reasons)

Will do if I resubmit.

>
>>
>> -     keymap = kcalloc(hotkey_num + 1, sizeof(struct key_entry), GFP_KERNEL);
>> +     keymap = kcalloc(hotkey_num + ARRAY_SIZE(dell_wmi_extra_keymap),
>
> This previously allocated kotkey_num + 1, but you dropeed the +1, making it
> eactly the size of hotkey_num + the new entries you added.
>
> Why don't we need the +1 anymore? (it isn't clear to me why we needed to before
> actually, but I want to confirm you considered it).

Whoops!  It's for the KE_END entry.

Jared, want to give us some guidance as to whether this code is
correct at all and, if so, whether we should actually send a
KEY_RFKILL event from dell-wmi when the key is pressed?

IOW, should we allow dell-wmi to handle rfkill or should we wait for
the other mechanism?

--Andy
--
To unsubscribe from this list: send the line "unsubscribe platform-driver-x86" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
D. Jared Dominguez Nov. 22, 2015, 2:04 a.m. UTC | #12
On Fri, Nov 20, 2015 at 06:12:58PM -0600, Andy Lutomirski wrote:
>On Fri, Nov 20, 2015 at 4:06 PM, Darren Hart <dvhart@infradead.org> wrote:
>> On Fri, Nov 13, 2015 at 09:49:30PM -0800, 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.
>>>
>>> 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 } },
>>> +};
>>> +
>>>  static struct input_dev *dell_wmi_input_dev;
>>>
>>>  static void dell_wmi_process_key(int reported_key)
>>> @@ -300,9 +310,10 @@ static const struct key_entry * __init dell_wmi_prepare_new_keymap(void)
>>>       int hotkey_num = (dell_bios_hotkey_table->header.length - 4) /
>>>                               sizeof(struct dell_bios_keymap_entry);
>>>       struct key_entry *keymap;
>>> -     int i;
>>> +     int i, pos = 0, num_bios_keys;
>>
>> Just a nit, "reverse christmas tree" order (longest line length first) please.
>> (only if you resend after this review for other reasons)
>
>Will do if I resubmit.
>
>>
>>>
>>> -     keymap = kcalloc(hotkey_num + 1, sizeof(struct key_entry), GFP_KERNEL);
>>> +     keymap = kcalloc(hotkey_num + ARRAY_SIZE(dell_wmi_extra_keymap),
>>
>> This previously allocated kotkey_num + 1, but you dropeed the +1, making it
>> eactly the size of hotkey_num + the new entries you added.
>>
>> Why don't we need the +1 anymore? (it isn't clear to me why we needed to before
>> actually, but I want to confirm you considered it).
>
>Whoops!  It's for the KE_END entry.
>
>Jared, want to give us some guidance as to whether this code is
>correct at all and, if so, whether we should actually send a
>KEY_RFKILL event from dell-wmi when the key is pressed?
>
>IOW, should we allow dell-wmi to handle rfkill or should we wait for
>the other mechanism?
>
>--Andy

I'm adding Mario since he's worked on our client systems much longer and 
knows more about the correct behavior for our hotkeys. I've not yet read 
through any of our documentation about our hotkey behavior so wouldn't 
be as qualified to answer that. Note that both Mario and I are on 
vacation until the 30th, so he may not reply until then.

--Jared
Pali Rohár Nov. 23, 2015, 2:53 p.m. UTC | #13
On Saturday 21 November 2015 20:04:15 D. Jared Dominguez wrote:
> On Fri, Nov 20, 2015 at 06:12:58PM -0600, Andy Lutomirski wrote:
> >Jared, want to give us some guidance as to whether this code is
> >correct at all and, if so, whether we should actually send a
> >KEY_RFKILL event from dell-wmi when the key is pressed?
> >
> >IOW, should we allow dell-wmi to handle rfkill or should we wait for
> >the other mechanism?
> >
> >--Andy
> 
> I'm adding Mario since he's worked on our client systems much longer and
> knows more about the correct behavior for our hotkeys. I've not yet read
> through any of our documentation about our hotkey behavior so wouldn't be as
> qualified to answer that. Note that both Mario and I are on vacation until
> the 30th, so he may not reply until then.
> 
> --Jared
> 

Jared, thanks for your input! When you or Mario are back from vacation,
can you look at it? Or if you do not have much time, can you provide
documentation (or some snippets), so somebody else can update driver to
work correctly?
Pali Rohár Jan. 21, 2016, 10:17 a.m. UTC | #14
On Monday 23 November 2015 15:53:51 Pali Rohár wrote:
> On Saturday 21 November 2015 20:04:15 D. Jared Dominguez wrote:
> > On Fri, Nov 20, 2015 at 06:12:58PM -0600, Andy Lutomirski wrote:
> > >Jared, want to give us some guidance as to whether this code is
> > >correct at all and, if so, whether we should actually send a
> > >KEY_RFKILL event from dell-wmi when the key is pressed?
> > >
> > >IOW, should we allow dell-wmi to handle rfkill or should we wait for
> > >the other mechanism?
> > >
> > >--Andy
> > 
> > I'm adding Mario since he's worked on our client systems much longer and
> > knows more about the correct behavior for our hotkeys. I've not yet read
> > through any of our documentation about our hotkey behavior so wouldn't be as
> > qualified to answer that. Note that both Mario and I are on vacation until
> > the 30th, so he may not reply until then.
> > 
> > --Jared
> > 
> 
> Jared, thanks for your input! When you or Mario are back from vacation,
> can you look at it? Or if you do not have much time, can you provide
> documentation (or some snippets), so somebody else can update driver to
> work correctly?

Hi Jared & Mario, I believe you are back from vacation, can you look at it?
Mario Limonciello Jan. 22, 2016, 12:08 a.m. UTC | #15
On 01/21/2016 04:17 AM, Pali Rohár wrote:
> On Monday 23 November 2015 15:53:51 Pali Rohár wrote:
>> On Saturday 21 November 2015 20:04:15 D. Jared Dominguez wrote:
>>> On Fri, Nov 20, 2015 at 06:12:58PM -0600, Andy Lutomirski wrote:
>>>> Jared, want to give us some guidance as to whether this code is
>>>> correct at all and, if so, whether we should actually send a
>>>> KEY_RFKILL event from dell-wmi when the key is pressed?
>>>>
>>>> IOW, should we allow dell-wmi to handle rfkill or should we wait for
>>>> the other mechanism?
>>>>
>>>> --Andy
>>> I'm adding Mario since he's worked on our client systems much longer and
>>> knows more about the correct behavior for our hotkeys. I've not yet read
>>> through any of our documentation about our hotkey behavior so wouldn't be as
>>> qualified to answer that. Note that both Mario and I are on vacation until
>>> the 30th, so he may not reply until then.
>>>
>>> --Jared
>>>
>> Jared, thanks for your input! When you or Mario are back from vacation,
>> can you look at it? Or if you do not have much time, can you provide
>> documentation (or some snippets), so somebody else can update driver to
>> work correctly?
> Hi Jared & Mario, I believe you are back from vacation, can you look at it?
>
Pali,

I understand that intel-hid will be making it in at this point.  If I'm
not mistaken, it should be handling this portion.

Andy,

Is that right now?  Or is this on a different topic?
Andy Lutomirski Jan. 22, 2016, 12:13 a.m. UTC | #16
On Thu, Jan 21, 2016 at 4:08 PM, Mario Limonciello
<mario_limonciello@dell.com> wrote:
>
>
> On 01/21/2016 04:17 AM, Pali Rohár wrote:
>> On Monday 23 November 2015 15:53:51 Pali Rohár wrote:
>>> On Saturday 21 November 2015 20:04:15 D. Jared Dominguez wrote:
>>>> On Fri, Nov 20, 2015 at 06:12:58PM -0600, Andy Lutomirski wrote:
>>>>> Jared, want to give us some guidance as to whether this code is
>>>>> correct at all and, if so, whether we should actually send a
>>>>> KEY_RFKILL event from dell-wmi when the key is pressed?
>>>>>
>>>>> IOW, should we allow dell-wmi to handle rfkill or should we wait for
>>>>> the other mechanism?
>>>>>
>>>>> --Andy
>>>> I'm adding Mario since he's worked on our client systems much longer and
>>>> knows more about the correct behavior for our hotkeys. I've not yet read
>>>> through any of our documentation about our hotkey behavior so wouldn't be as
>>>> qualified to answer that. Note that both Mario and I are on vacation until
>>>> the 30th, so he may not reply until then.
>>>>
>>>> --Jared
>>>>
>>> Jared, thanks for your input! When you or Mario are back from vacation,
>>> can you look at it? Or if you do not have much time, can you provide
>>> documentation (or some snippets), so somebody else can update driver to
>>> work correctly?
>> Hi Jared & Mario, I believe you are back from vacation, can you look at it?
>>
> Pali,
>
> I understand that intel-hid will be making it in at this point.  If I'm
> not mistaken, it should be handling this portion.
>
> Andy,
>
> Is that right now?  Or is this on a different topic?
>

I think that's right.  And intel-hid is in Linus' tree now.

--Andy
--
To unsubscribe from this list: send the line "unsubscribe platform-driver-x86" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Pali Rohár Jan. 22, 2016, 8:26 a.m. UTC | #17
On Thursday 21 January 2016 18:08:38 Mario Limonciello wrote:
> 
> 
> On 01/21/2016 04:17 AM, Pali Rohár wrote:
> > On Monday 23 November 2015 15:53:51 Pali Rohár wrote:
> >> On Saturday 21 November 2015 20:04:15 D. Jared Dominguez wrote:
> >>> On Fri, Nov 20, 2015 at 06:12:58PM -0600, Andy Lutomirski wrote:
> >>>> Jared, want to give us some guidance as to whether this code is
> >>>> correct at all and, if so, whether we should actually send a
> >>>> KEY_RFKILL event from dell-wmi when the key is pressed?
> >>>>
> >>>> IOW, should we allow dell-wmi to handle rfkill or should we wait for
> >>>> the other mechanism?
> >>>>
> >>>> --Andy
> >>> I'm adding Mario since he's worked on our client systems much longer and
> >>> knows more about the correct behavior for our hotkeys. I've not yet read
> >>> through any of our documentation about our hotkey behavior so wouldn't be as
> >>> qualified to answer that. Note that both Mario and I are on vacation until
> >>> the 30th, so he may not reply until then.
> >>>
> >>> --Jared
> >>>
> >> Jared, thanks for your input! When you or Mario are back from vacation,
> >> can you look at it? Or if you do not have much time, can you provide
> >> documentation (or some snippets), so somebody else can update driver to
> >> work correctly?
> > Hi Jared & Mario, I believe you are back from vacation, can you look at it?
> >
> Pali,
> 
> I understand that intel-hid will be making it in at this point.  If I'm
> not mistaken, it should be handling this portion.

Hi Mario! Thats truth, but we still needs to know all codes which
dell-wmi should ignore (because they will be handled by intel-hid).

Currently Andy's patch adds three codes which are not specified in
vendor-specific DMI table.

Do you such table of key events which can ACPI/BIOS/WMI send to OS?
Mario Limonciello Jan. 22, 2016, 3:58 p.m. UTC | #18
On 01/22/2016 02:26 AM, Pali Rohár wrote:
> On Thursday 21 January 2016 18:08:38 Mario Limonciello wrote:
> Hi Mario! Thats truth, but we still needs to know all codes which
> dell-wmi should ignore (because they will be handled by intel-hid).
>
> Currently Andy's patch adds three codes which are not specified in
> vendor-specific DMI table.
>
> Do you such table of key events which can ACPI/BIOS/WMI send to OS?
Hi Pali,

Andy conversed with us privately on this, and we double checked with our
BIOS team on the EC source code for these items that aren't mentioned in
the DMI table.
This should now be complete as of current EC feature support.

Thanks,
Pali Rohár Jan. 22, 2016, 5:11 p.m. UTC | #19
On Friday 22 January 2016 16:58:02 Mario Limonciello wrote:
> On 01/22/2016 02:26 AM, Pali Rohár wrote:
> > On Thursday 21 January 2016 18:08:38 Mario Limonciello wrote:
> > Hi Mario! Thats truth, but we still needs to know all codes which
> > dell-wmi should ignore (because they will be handled by intel-hid).
> > 
> > Currently Andy's patch adds three codes which are not specified in
> > vendor-specific DMI table.
> > 
> > Do you such table of key events which can ACPI/BIOS/WMI send to OS?
> 
> Hi Pali,
> 
> Andy conversed with us privately on this, and we double checked with
> our BIOS team on the EC source code for these items that aren't
> mentioned in the DMI table.
> This should now be complete as of current EC feature support.
> 
> Thanks,

Great! Thank you for info.
diff mbox

Patch

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 } },
+};
+
 static struct input_dev *dell_wmi_input_dev;
 
 static void dell_wmi_process_key(int reported_key)
@@ -300,9 +310,10 @@  static const struct key_entry * __init dell_wmi_prepare_new_keymap(void)
 	int hotkey_num = (dell_bios_hotkey_table->header.length - 4) /
 				sizeof(struct dell_bios_keymap_entry);
 	struct key_entry *keymap;
-	int i;
+	int i, pos = 0, num_bios_keys;
 
-	keymap = kcalloc(hotkey_num + 1, sizeof(struct key_entry), GFP_KERNEL);
+	keymap = kcalloc(hotkey_num + ARRAY_SIZE(dell_wmi_extra_keymap),
+			 sizeof(struct key_entry), GFP_KERNEL);
 	if (!keymap)
 		return NULL;
 
@@ -314,14 +325,37 @@  static const struct key_entry * __init dell_wmi_prepare_new_keymap(void)
 				    KEY_RESERVED;
 
 		if (keycode == KEY_KBDILLUMTOGGLE)
-			keymap[i].type = KE_IGNORE;
+			keymap[pos].type = KE_IGNORE;
 		else
-			keymap[i].type = KE_KEY;
-		keymap[i].code = bios_entry->scancode;
-		keymap[i].keycode = keycode;
+			keymap[pos].type = KE_KEY;
+		keymap[pos].code = bios_entry->scancode;
+		keymap[pos].keycode = keycode;
+
+		pos++;
+	}
+
+	num_bios_keys = pos;
+
+	for (i = 0; i < ARRAY_SIZE(dell_wmi_extra_keymap); i++) {
+		int j;
+
+		/*
+		 * 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;
+
+		keymap[pos] = dell_wmi_extra_keymap[i];
+		pos++;
+
+skip:
+		;
 	}
 
-	keymap[hotkey_num].type = KE_END;
+	keymap[pos].type = KE_END;
 
 	return keymap;
 }