Re: [PATCH v2 3/3] platform/x86: dell-wmi: add new dmi keys to bios_to_linux_keycode

Randy Dunlap
2020-06-08T23:42:48Z

Hi--

On 6/8/20 4:05 PM, Y Paritcher wrote:
> Increase length of bios_to_linux_keycode to 2 bytes (the true size of a
> keycode) to allow for a new keycode 0xffff, this silences the following
> messages being logged at startup on a Dell Inspiron 5593:
>
> dell_wmi: firmware scancode 0x48 maps to unrecognized keycode 0xffff
> dell_wmi: firmware scancode 0x50 maps to unrecognized keycode 0xffff
>
> as per this code comment:
>
> Log if we find an entry in the DMI table that we don't
> understand. If this happens, we should figure out what
> the entry means and add it to bios_to_linux_keycode.
>
> These are keycodes included in the 0xB2 DMI table, for which the
> corosponding keys are not known.

corresponding

>
> Now when a user will encounter this key, a proper message wil be printed:
>
> dell_wmi: Unknown key with type 0xXXXX and code 0xXXXX pressed
>
> This will then allow the key to be identified properly.
>
> Signed-off-by: Y Paritcher <y.linux@paritcher.com>
> ---
> drivers/platform/x86/dell-wmi.c | 8 +++-----
> 1 file changed, 3 insertions(+), 5 deletions(-)
>
> diff --git a/drivers/platform/x86/dell-wmi.c b/drivers/platform/x86/dell-wmi.c
> index 6b510f8431a3..dae1db96b5a0 100644
> --- a/drivers/platform/x86/dell-wmi.c
> +++ b/drivers/platform/x86/dell-wmi.c
> @@ -196,7 +196,7 @@ struct dell_dmi_results {
> };
>
> /* Uninitialized entries here are KEY_RESERVED == 0. */
> -static const u16 bios_to_linux_keycode[256] = {
> +static const u16 bios_to_linux_keycode[65536] = {

It surely seems odd to me to expand an array from 512 bytes to 128 Kbytes
just to handle one special case. Can't it be handled in code as a
special case?

> [0] = KEY_MEDIA,
> [1] = KEY_NEXTSONG,
> [2] = KEY_PLAYPAUSE,
> @@ -237,6 +237,7 @@ static const u16 bios_to_linux_keycode[256] = {
> [37] = KEY_UNKNOWN,
> [38] = KEY_MICMUTE,
> [255] = KEY_PROG3,
> + [65535] = KEY_UNKNOWN,
> };
>
> /*
> @@ -503,10 +504,7 @@ static void handle_dmi_entry(const struct dmi_header *dm, void *opaque)
> &table->keymap[i];
>
> /* Uninitialized entries are 0 aka KEY_RESERVED. */
> - u16 keycode = (bios_entry->keycode <
> - ARRAY_SIZE(bios_to_linux_keycode)) ?
> - bios_to_linux_keycode[bios_entry->keycode] :
> - KEY_RESERVED;
> + u16 keycode = bios_to_linux_keycode[bios_entry->keycode];
>
> /*
> * Log if we find an entry in the DMI table that we don't
>

Something like:

u16 keycode;

keycode = bios_entry->keycode == 0xffff ? KEY_UNKNOWN :
(bios_entry->keycode <
ARRAY_SIZE(bios_to_linux_keycode)) ?
bios_to_linux_keycode[bios_entry->keycode] :
KEY_RESERVED;

Also please fix this:
(no To-header on input) <>

--
~Randy