Date: Tue, 7 Jul 2015 23:18:48 +0200
From: Pali =?utf-8?B?Um9ow6Fy?= <pali.rohar@gmail.com>
To: Mario Limonciello <mario_limonciello@dell.com>
Cc: Matthew Garrett <mjg@redhat.com>,
        Andrew Morton <akpm@linux-foundation.org>,
        Len Brown <len.brown@intel.com>,
        "platform-driver-x86@vger.kernel.org" 
	<platform-driver-x86@vger.kernel.org>,
        "linux-kernel@vger.kernel.org" <linux-kernel@vger.kernel.org>,
        Gabriele Mazzotta <gabriele.mzt@gmail.com>,
        =?utf-8?B?TWljaGHFgiBLxJlwaWXFhA==?= <kernel@kempniu.pl>
Subject: Re: BUG? Duplicate key code 0xe045 in dell-wmi.c
Message-ID: <20150707211848.GB20901@pali>
References: <201507041834.03944@pali>
 <559C1CB1.4040509@dell.com>
MIME-Version: 1.0
Content-Type: text/plain; charset=utf-8
Content-Disposition: inline
Content-Transfer-Encoding: 8bit
In-Reply-To: <559C1CB1.4040509@dell.com>
User-Agent: Mutt/1.5.21 (2010-09-15)
Sender: linux-kernel-owner@vger.kernel.org
Content-Length: 3502
Lines: 99

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