BUG? Duplicate key code 0xe045 in dell-wmi.c

BUG? Duplicate key code 0xe045 in dell-wmi.c

[Pali Rohár]

[-- Attachment #1: Type: Text/Plain, Size: 1377 bytes --]

Hello,

I'm looking at dell-wmi.c driver and its history in git and I found 
problem with handling WMI key code 0xe045. In current dell-wmi.c code is

{KE_KEY, 0xe045, KEY_PROG1},
{KE_IGNORE, 0xe045, KEY_NUMLOCK},

I bet this is some copy-paste error as one code can be translated only 
to one input key event.

In git history I found that above change was added by commit:

http://git.kernel.org/cgit/linux/kernel/git/torvalds/linux.git/commit/?id=5cab0098171712a9fd51399b06181c8dfdebe9c9

===============================================
commit 5cab0098171712a9fd51399b06181c8dfdebe9c9
Author: Mario Limonciello <mario_limonciello@dell.com>
Date:   Wed Jun 10 19:40:47 2009 +0000

    dell-wmi: add additional keyboard events
    
    Upcoming Dell hardware will send more keyboard events via WMI.  Add
    support for them.
    
    Signed-off-by: Mario Limonciello <mario_limonciello@dell.com>
    Signed-off-by: Matthew Garrett <mjg@redhat.com>
    Signed-off-by: Andrew Morton <akpm@linux-foundation.org>
    Signed-off-by: Len Brown <len.brown@intel.com>
===============================================

Sending email to all signers of this commit. Problematic code is still 
in upstream kernel, so it needs to be fixed.

Mario Limonciello: Do you know if code 0xe045 is some PROG1 or NUMLOCK?

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

[-- Attachment #2: This is a digitally signed message part. --]
[-- Type: application/pgp-signature, Size: 198 bytes --]

Re: BUG? Duplicate key code 0xe045 in dell-wmi.c

[Mario Limonciello]

On 07/04/2015 11:34 AM, Pali Rohár wrote:
> Hello,
>
> I'm looking at dell-wmi.c driver and its history in git and I found 
> problem with handling WMI key code 0xe045. In current dell-wmi.c code is
>
> {KE_KEY, 0xe045, KEY_PROG1},
> {KE_IGNORE, 0xe045, KEY_NUMLOCK},
>
> I bet this is some copy-paste error as one code can be translated only 
> to one input key event.
>
> In git history I found that above change was added by commit:
>
> http://git.kernel.org/cgit/linux/kernel/git/torvalds/linux.git/commit/?id=5cab0098171712a9fd51399b06181c8dfdebe9c9
>
> ===============================================
> commit 5cab0098171712a9fd51399b06181c8dfdebe9c9
> Author: Mario Limonciello <mario_limonciello@dell.com>
> Date:   Wed Jun 10 19:40:47 2009 +0000
>
>     dell-wmi: add additional keyboard events
>     
>     Upcoming Dell hardware will send more keyboard events via WMI.  Add
>     support for them.
>     
>     Signed-off-by: Mario Limonciello <mario_limonciello@dell.com>
>     Signed-off-by: Matthew Garrett <mjg@redhat.com>
>     Signed-off-by: Andrew Morton <akpm@linux-foundation.org>
>     Signed-off-by: Len Brown <len.brown@intel.com>
> ===============================================
>
> Sending email to all signers of this commit. Problematic code is still 
> in upstream kernel, so it needs to be fixed.
>
> Mario Limonciello: Do you know if code 0xe045 is some PROG1 or NUMLOCK?
>
Hi Pali,

Yes this looks like a mistake and that the KEY_PROG1 item should have
been removed in that patch.  It should be a notification (KEY_IGNORE)
for numlock.

Thanks,

Re: BUG? Duplicate key code 0xe045 in dell-wmi.c

[Pali Rohár]

On Tuesday 07 July 2015 13:38:41 Mario Limonciello wrote:
> 
> 
> On 07/04/2015 11:34 AM, Pali Rohár wrote:
> > Hello,
> >
> > I'm looking at dell-wmi.c driver and its history in git and I found 
> > problem with handling WMI key code 0xe045. In current dell-wmi.c code is
> >
> > {KE_KEY, 0xe045, KEY_PROG1},
> > {KE_IGNORE, 0xe045, KEY_NUMLOCK},
> >
> > I bet this is some copy-paste error as one code can be translated only 
> > to one input key event.
> >
> > In git history I found that above change was added by commit:
> >
> > http://git.kernel.org/cgit/linux/kernel/git/torvalds/linux.git/commit/?id=5cab0098171712a9fd51399b06181c8dfdebe9c9
> >
> > ===============================================
> > commit 5cab0098171712a9fd51399b06181c8dfdebe9c9
> > Author: Mario Limonciello <mario_limonciello@dell.com>
> > Date:   Wed Jun 10 19:40:47 2009 +0000
> >
> >     dell-wmi: add additional keyboard events
> >     
> >     Upcoming Dell hardware will send more keyboard events via WMI.  Add
> >     support for them.
> >     
> >     Signed-off-by: Mario Limonciello <mario_limonciello@dell.com>
> >     Signed-off-by: Matthew Garrett <mjg@redhat.com>
> >     Signed-off-by: Andrew Morton <akpm@linux-foundation.org>
> >     Signed-off-by: Len Brown <len.brown@intel.com>
> > ===============================================
> >
> > Sending email to all signers of this commit. Problematic code is still 
> > in upstream kernel, so it needs to be fixed.
> >
> > Mario Limonciello: Do you know if code 0xe045 is some PROG1 or NUMLOCK?
> >
> Hi Pali,
> 
> Yes this looks like a mistake and that the KEY_PROG1 item should have
> been removed in that patch.  It should be a notification (KEY_IGNORE)
> for numlock.
> 
> Thanks,

Hi, I looked into git history of dell-wmi more deeply... into first
commit of dell-wmi.c source code. It is this one from year 2009:

https://git.kernel.org/cgit/linux/kernel/git/torvalds/linux.git/commit/?id=0b3f6109f0c9ff926b5ffc80dc1cebb24f192b35

Here Matthew Garrett says:

... send a generic input event when a button with a picture of a battery
on it is pressed ...

and adds code:

+static struct key_entry dell_wmi_keymap[] = {
+	{KE_KEY, 0xe045, KEY_PROG1},
+	{KE_END, 0}
+};

So basically when "picture of a battery" is pressed WMI send event
0xe045 which is translated to KEY_PROG1.

Matthew Garrett is original author of that code and I believe that on
some his machine 0xe045 code from DELL WMI really could be sent when
battery button is pressed.

Mario, if you as Dell authority says that 0xe045 is sent when numlock is
pressed, it means either code 0xe045 is ambiguous :-( and has different
meaning for different machines or there is error in (your) documentation
or bios... or Matthew cannot differ between picture of battery and
picture of numlock on his dell machine :-)

Matthew: If you still reading this email thread, do you know for which
Dell machine did you write this dell-wmi.c driver originally?

Mario: Do you know more about this problem?


>From function sparse_keymap_setup() called by dell-wmi.c this structure:

  {KE_KEY, 0xe045, KEY_PROG1},
  {KE_IGNORE, 0xe045, KEY_NUMLOCK},

is decoded that 0xe045 event is *not* ignored and reports KEY_PROG1 key.

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

Re: BUG? Duplicate key code 0xe045 in dell-wmi.c

[Pali Rohár]

On Tuesday 07 July 2015 23:18:48 Pali Rohár wrote:
> On Tuesday 07 July 2015 13:38:41 Mario Limonciello wrote:
> > 
> > 
> > On 07/04/2015 11:34 AM, Pali Rohár wrote:
> > > Hello,
> > >
> > > I'm looking at dell-wmi.c driver and its history in git and I found 
> > > problem with handling WMI key code 0xe045. In current dell-wmi.c code is
> > >
> > > {KE_KEY, 0xe045, KEY_PROG1},
> > > {KE_IGNORE, 0xe045, KEY_NUMLOCK},
> > >
> > > I bet this is some copy-paste error as one code can be translated only 
> > > to one input key event.
> > >
> > > In git history I found that above change was added by commit:
> > >
> > > http://git.kernel.org/cgit/linux/kernel/git/torvalds/linux.git/commit/?id=5cab0098171712a9fd51399b06181c8dfdebe9c9
> > >
> > > ===============================================
> > > commit 5cab0098171712a9fd51399b06181c8dfdebe9c9
> > > Author: Mario Limonciello <mario_limonciello@dell.com>
> > > Date:   Wed Jun 10 19:40:47 2009 +0000
> > >
> > >     dell-wmi: add additional keyboard events
> > >     
> > >     Upcoming Dell hardware will send more keyboard events via WMI.  Add
> > >     support for them.
> > >     
> > >     Signed-off-by: Mario Limonciello <mario_limonciello@dell.com>
> > >     Signed-off-by: Matthew Garrett <mjg@redhat.com>
> > >     Signed-off-by: Andrew Morton <akpm@linux-foundation.org>
> > >     Signed-off-by: Len Brown <len.brown@intel.com>
> > > ===============================================
> > >
> > > Sending email to all signers of this commit. Problematic code is still 
> > > in upstream kernel, so it needs to be fixed.
> > >
> > > Mario Limonciello: Do you know if code 0xe045 is some PROG1 or NUMLOCK?
> > >
> > Hi Pali,
> > 
> > Yes this looks like a mistake and that the KEY_PROG1 item should have
> > been removed in that patch.  It should be a notification (KEY_IGNORE)
> > for numlock.
> > 
> > Thanks,
> 
> Hi, I looked into git history of dell-wmi more deeply... into first
> commit of dell-wmi.c source code. It is this one from year 2009:
> 
> https://git.kernel.org/cgit/linux/kernel/git/torvalds/linux.git/commit/?id=0b3f6109f0c9ff926b5ffc80dc1cebb24f192b35
> 
> Here Matthew Garrett says:
> 
> ... send a generic input event when a button with a picture of a battery
> on it is pressed ...
> 
> and adds code:
> 
> +static struct key_entry dell_wmi_keymap[] = {
> +	{KE_KEY, 0xe045, KEY_PROG1},
> +	{KE_END, 0}
> +};
> 
> So basically when "picture of a battery" is pressed WMI send event
> 0xe045 which is translated to KEY_PROG1.
> 
> Matthew Garrett is original author of that code and I believe that on
> some his machine 0xe045 code from DELL WMI really could be sent when
> battery button is pressed.
> 
> Mario, if you as Dell authority says that 0xe045 is sent when numlock is
> pressed, it means either code 0xe045 is ambiguous :-( and has different
> meaning for different machines or there is error in (your) documentation
> or bios... or Matthew cannot differ between picture of battery and
> picture of numlock on his dell machine :-)
> 
> Matthew: If you still reading this email thread, do you know for which
> Dell machine did you write this dell-wmi.c driver originally?
> 
> Mario: Do you know more about this problem?
> 
> 
> From function sparse_keymap_setup() called by dell-wmi.c this structure:
> 
>   {KE_KEY, 0xe045, KEY_PROG1},
>   {KE_IGNORE, 0xe045, KEY_NUMLOCK},
> 
> is decoded that 0xe045 event is *not* ignored and reports KEY_PROG1 key.
> 

Mario & Matthew: ping

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

Re: BUG? Duplicate key code 0xe045 in dell-wmi.c

[Pali Rohár]

On Tuesday 07 July 2015 23:18:48 Pali Rohár wrote:
> On Tuesday 07 July 2015 13:38:41 Mario Limonciello wrote:
> > 
> > 
> > On 07/04/2015 11:34 AM, Pali Rohár wrote:
> > > Hello,
> > >
> > > I'm looking at dell-wmi.c driver and its history in git and I found 
> > > problem with handling WMI key code 0xe045. In current dell-wmi.c code is
> > >
> > > {KE_KEY, 0xe045, KEY_PROG1},
> > > {KE_IGNORE, 0xe045, KEY_NUMLOCK},
> > >
> > > I bet this is some copy-paste error as one code can be translated only 
> > > to one input key event.
> > >
> > > In git history I found that above change was added by commit:
> > >
> > > http://git.kernel.org/cgit/linux/kernel/git/torvalds/linux.git/commit/?id=5cab0098171712a9fd51399b06181c8dfdebe9c9
> > >
> > > ===============================================
> > > commit 5cab0098171712a9fd51399b06181c8dfdebe9c9
> > > Author: Mario Limonciello <mario_limonciello@dell.com>
> > > Date:   Wed Jun 10 19:40:47 2009 +0000
> > >
> > >     dell-wmi: add additional keyboard events
> > >     
> > >     Upcoming Dell hardware will send more keyboard events via WMI.  Add
> > >     support for them.
> > >     
> > >     Signed-off-by: Mario Limonciello <mario_limonciello@dell.com>
> > >     Signed-off-by: Matthew Garrett <mjg@redhat.com>
> > >     Signed-off-by: Andrew Morton <akpm@linux-foundation.org>
> > >     Signed-off-by: Len Brown <len.brown@intel.com>
> > > ===============================================
> > >
> > > Sending email to all signers of this commit. Problematic code is still 
> > > in upstream kernel, so it needs to be fixed.
> > >
> > > Mario Limonciello: Do you know if code 0xe045 is some PROG1 or NUMLOCK?
> > >
> > Hi Pali,
> > 
> > Yes this looks like a mistake and that the KEY_PROG1 item should have
> > been removed in that patch.  It should be a notification (KEY_IGNORE)
> > for numlock.
> > 
> > Thanks,
> 
> Hi, I looked into git history of dell-wmi more deeply... into first
> commit of dell-wmi.c source code. It is this one from year 2009:
> 
> https://git.kernel.org/cgit/linux/kernel/git/torvalds/linux.git/commit/?id=0b3f6109f0c9ff926b5ffc80dc1cebb24f192b35
> 
> Here Matthew Garrett says:
> 
> ... send a generic input event when a button with a picture of a battery
> on it is pressed ...
> 
> and adds code:
> 
> +static struct key_entry dell_wmi_keymap[] = {
> +	{KE_KEY, 0xe045, KEY_PROG1},
> +	{KE_END, 0}
> +};
> 
> So basically when "picture of a battery" is pressed WMI send event
> 0xe045 which is translated to KEY_PROG1.
> 
> Matthew Garrett is original author of that code and I believe that on
> some his machine 0xe045 code from DELL WMI really could be sent when
> battery button is pressed.
> 
> Mario, if you as Dell authority says that 0xe045 is sent when numlock is
> pressed, it means either code 0xe045 is ambiguous :-( and has different
> meaning for different machines or there is error in (your) documentation
> or bios... or Matthew cannot differ between picture of battery and
> picture of numlock on his dell machine :-)
> 
> Matthew: If you still reading this email thread, do you know for which
> Dell machine did you write this dell-wmi.c driver originally?
> 
> Mario: Do you know more about this problem?
> 
> 
> From function sparse_keymap_setup() called by dell-wmi.c this structure:
> 
>   {KE_KEY, 0xe045, KEY_PROG1},
>   {KE_IGNORE, 0xe045, KEY_NUMLOCK},
> 
> is decoded that 0xe045 event is *not* ignored and reports KEY_PROG1 key.
> 

Hi Mario! Can you comment something about that KEY_PROG1?

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