Re: [PATCH 3/3] dell_wmi: Improve unknown hotkey handling

[Darren Hart <dvhart@infradead.org>]

On Mon, Nov 16, 2015 at 05:35:44PM -0800, Andy Lutomirski wrote:
> On Sat, Nov 14, 2015 at 1:33 AM, Pali Rohár <pali.rohar@gmail.com> wrote:
> > On Friday 13 November 2015 21:49:32 Andy Lutomirski wrote:
> >> If DMI lists a hotkey that we don't recognize, log and ignore it
> >> instead of trying to map it to keycode 0.  I haven't seen this happen,
> >> but it will help maintain the key map in the future and it will help
> >> avoid sending bogus events.
> >>
> >> This also improves the message that we log when we get an unknown key
> >> event.
> >>
> >> Signed-off-by: Andy Lutomirski <luto@kernel.org>
> >> ---
> >>  drivers/platform/x86/dell-wmi.c | 7 ++++++-
> >>  1 file changed, 6 insertions(+), 1 deletion(-)
> >>
> >> diff --git a/drivers/platform/x86/dell-wmi.c b/drivers/platform/x86/dell-wmi.c
> >> index 92b0149fa4a7..e43228a35f6b 100644
> >> --- a/drivers/platform/x86/dell-wmi.c
> >> +++ b/drivers/platform/x86/dell-wmi.c
> >> @@ -180,7 +180,7 @@ static void dell_wmi_process_key(int reported_key)
> >>       key = sparse_keymap_entry_from_scancode(dell_wmi_input_dev,
> >>                                               reported_key);
> >>       if (!key) {
> >> -             pr_info("Unknown key %x pressed\n", reported_key);
> >> +             pr_info("Unknown key with scancode 0x%x pressed\n", reported_key);
> >>               return;
> >>       }
> >>
> >> @@ -343,6 +343,11 @@ static const struct key_entry * __init dell_wmi_prepare_new_keymap(void)
> >>                                   bios_to_linux_keycode[bios_entry->keycode] :
> >>                                   KEY_RESERVED;
> >>
> >> +             if (keycode == 0) {
> >> +                     pr_info("firmware scancode %d maps to unrecognized keycode %d\n", bios_entry->keycode, bios_entry->scancode);
> >> +                     continue;
> >> +             }
> >> +
> >
> > This line is too long and checkpatch.pl will probably blame you.
> 
> I split it.  The first bit (with the quoted string) is still a bit
> above 80, but that's better than splitting the string itself and
> breaking grep.

So I've taken care of this in my branch (and the one above too).

> 
> >
> > Anyway, have you already found some missing mapping which comes from
> > bios/firmware (because your patches do not change that bios table)?
> >
> 
> I don't see a DMI entry that maps to something outside the table,
> though, so this warning doesn't trigger.  My best guess is that Dell
> simply didn't bother to update the DMI table and has it hardcoded in
> their driver.  Admittedly, I've never played with the official driver
> at all.

Pali, was this something you wanted to discuss more before I merge it?

-- 
Darren Hart
Intel Open Source Technology Center