> Signed-off-by: Pali Rohár <[email protected]>
My guess is that Darren won't let you off without at least a short
commit message.
> ---
> drivers/platform/x86/dell-wmi.c | 31 ++++++++++++++++++-------------
> 1 file changed, 18 insertions(+), 13 deletions(-)
>
> diff --git a/drivers/platform/x86/dell-wmi.c b/drivers/platform/x86/dell-wmi.c
> index 4d23c91..363d927 100644
> --- a/drivers/platform/x86/dell-wmi.c
> +++ b/drivers/platform/x86/dell-wmi.c
> @@ -86,31 +86,32 @@ static const struct dmi_system_id dell_wmi_smbios_list[]
> __initconst = {
> */
>
> static const struct key_entry dell_wmi_legacy_keymap[] __initconst = {
> - { KE_IGNORE, 0x003a, { KEY_CAPSLOCK } },
>
> - { KE_KEY, 0xe009, { KEY_EJECTCD } },
> + { KE_IGNORE, 0x003a, { KEY_CAPSLOCK } },
>
> - /* These also contain the brightness level at offset 6 */
> - { KE_KEY, 0xe006, { KEY_BRIGHTNESSUP } },
> - { KE_KEY, 0xe005, { KEY_BRIGHTNESSDOWN } },
> + /* These also contain the brightness level after key code */
> + { KE_KEY, 0xe006, { KEY_BRIGHTNESSUP } },
> + { KE_KEY, 0xe005, { KEY_BRIGHTNESSDOWN } },
These two entries were left unsorted.
>
> /* Battery health status button */
> - { KE_KEY, 0xe007, { KEY_BATTERY } },
> + { KE_KEY, 0xe007, { KEY_BATTERY } },
>
> - /* Radio devices state change */
> + /* Radio devices state change, also contains additional information */
> { KE_IGNORE, 0xe008, { KEY_RFKILL } },
>
> - /* The next device is at offset 6, the active devices are at
> - offset 8 and the attached devices at offset 10 */
> - { KE_KEY, 0xe00b, { KEY_SWITCHVIDEOMODE } },
> + { KE_KEY, 0xe009, { KEY_EJECTCD } },
>
> + /* After key code is: next device, active devices, attached devices */
> + { KE_KEY, 0xe00b, { KEY_SWITCHVIDEOMODE } },
> +
> + /* Also contains keyboard illumination level after key code */
I know I'm being really pedantic here, but as this is a cleanup patch,
maybe it makes sense to unify the comments a bit? After this patch is
applied, the comments are:
/* These also contain the brightness level after key code */
/* Radio devices state change, also contains additional information */
/* After key code is: next device, active devices, attached devices */
/* Also contains keyboard illumination level after key code */
How about changing them to something like:
/* Key code is followed by brightness level */
/* Radio devices state change, key code is followed by additional
information */
/* Key code is followed by: next device, active devices, attached devices */
/* Key code is followed by keyboard illumination level */
And looking at the bigger picture, do you think these comments
(especially the generic one: "also contains additional information") are
actually needed? Anything that follows the key code is ignored by
kernel code anyway.
--
Best regards,
Michał Kępień
