| Message ID | b7fd494ac6d51c13a66fec20ec336080479b0f5f.1447479930.git.luto@kernel.org (mailing list archive) |
|---|---|
| State | Superseded, archived |
| Headers | show |
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. Anyway, have you already found some missing mapping which comes from bios/firmware (because your patches do not change that bios table)? > if (keycode == KEY_KBDILLUMTOGGLE) > keymap[pos].type = KE_IGNORE; > else
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. > > 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. --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
On Fri, Nov 13, 2015 at 09:49:32PM -0800, 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> Matthew? Pali? Any concerns? > --- > 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); We don't split up strings for line length, but these arguments can certainly go on the next line. I've taken care of this, no need to resend. Queued for testing.
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?
On Fri, Nov 20, 2015 at 4:29 PM, Darren Hart <dvhart@infradead.org> wrote: > 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? FWIW, my current partial understanding of what changed between previous laptops and the 9350 is that there's Yet Another New Hotkey Mechanism (tm). I'm not sure whether I should comment on exactly what it does, but if you dig around in the DSDT, it's not all that hard to get a decent idea. The upshot is that, on Windows, it's probably still the case that all of the hotkeys that are supposed to be handled through WMI on the 9350 are, indeed, in the DMI table. Presumably the Windows driver doesn't visibly warn about unexpected scancodes. Since we *do* warn about unexpected scancodes, we need to silence the warnings. We could do it with a fancy table-driven approach that can actually generate keystrokes as in this patch (which gives us full support for the 9350's rfkill key), or we could just make a little list of scancodes that aren't worth warning about and add a while new platform driver for the new mechanism. Either way works. --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
On Saturday 21 November 2015 01:29:04 Darren Hart wrote: > Pali, was this something you wanted to discuss more before I merge > it? Just one note: > + if (keycode == 0) { What about using define KEY_RESERVED instead hardcoded constant 0? > + pr_info("firmware scancode %d maps to unrecognized keycode %d\n", bios_entry->keycode, bios_entry->scancode); > + continue; > + }
On Fri, Nov 20, 2015 at 4:41 PM, Pali Rohár <pali.rohar@gmail.com> wrote: > On Saturday 21 November 2015 01:29:04 Darren Hart wrote: >> Pali, was this something you wanted to discuss more before I merge >> it? > > Just one note: > >> + if (keycode == 0) { > > What about using define KEY_RESERVED instead hardcoded constant 0? > >> + pr_info("firmware scancode %d maps to unrecognized keycode %d\n", bios_entry->keycode, bios_entry->scancode); >> + continue; >> + } > That would work, but only because KEY_RESERVED == 0. Let me send a better patch. --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
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; + } + if (keycode == KEY_KBDILLUMTOGGLE) keymap[pos].type = KE_IGNORE; else
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(-)