2015-07-05 08:53:42

by Pali Rohár

[permalink] [raw]
Subject: BUG? Duplicate key code 0xe045 in dell-wmi.c

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 <[email protected]>
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 <[email protected]>
Signed-off-by: Matthew Garrett <[email protected]>
Signed-off-by: Andrew Morton <[email protected]>
Signed-off-by: Len Brown <[email protected]>
===============================================

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
[email protected]


Attachments:
signature.asc (198.00 B)
This is a digitally signed message part.

2015-07-07 18:39:27

by Mario Limonciello

[permalink] [raw]
Subject: Re: BUG? Duplicate key code 0xe045 in dell-wmi.c



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 <[email protected]>
> 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 <[email protected]>
> Signed-off-by: Matthew Garrett <[email protected]>
> Signed-off-by: Andrew Morton <[email protected]>
> Signed-off-by: Len Brown <[email protected]>
> ===============================================
>
> 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,

2015-07-07 21:19:26

by Pali Rohár

[permalink] [raw]
Subject: Re: BUG? Duplicate key code 0xe045 in dell-wmi.c

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 <[email protected]>
> > 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 <[email protected]>
> > Signed-off-by: Matthew Garrett <[email protected]>
> > Signed-off-by: Andrew Morton <[email protected]>
> > Signed-off-by: Len Brown <[email protected]>
> > ===============================================
> >
> > 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
[email protected]

2015-07-20 14:51:06

by Pali Rohár

[permalink] [raw]
Subject: Re: BUG? Duplicate key code 0xe045 in dell-wmi.c

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 <[email protected]>
> > > 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 <[email protected]>
> > > Signed-off-by: Matthew Garrett <[email protected]>
> > > Signed-off-by: Andrew Morton <[email protected]>
> > > Signed-off-by: Len Brown <[email protected]>
> > > ===============================================
> > >
> > > 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
[email protected]