On Monday 24 October 2016 15:43:50 Hans de Goede wrote: > Hi, > > On 24-10-16 15:34, Pali Rohár wrote: > >On Sunday 23 October 2016 21:46:51 Hans de Goede wrote: > >>diff --git a/drivers/platform/x86/dell-wmi.c b/drivers/platform/x86/dell-wmi.c > >>index da2fe18..f86e774 100644 > >>--- a/drivers/platform/x86/dell-wmi.c > >>+++ b/drivers/platform/x86/dell-wmi.c > >>@@ -319,6 +319,11 @@ static void dell_wmi_process_key(int type, int code) > >> if (type == 0x0000 && code == 0xe025 && !wmi_requires_smbios_request) > >> return; > >> > >>+ if (type == 0x0011 && (code == 0x01e1 || code == 0x02ea || > >>+ code == 0x02eb || code == 0x02ec || code == 0x02f6)) > >>+ dell_smbios_call_notifier( > >>+ dell_smbios_kbd_backlight_brightness_changed, NULL); > >>+ > >> sparse_keymap_report_entry(dell_wmi_input_dev, key, 1, true); > >> } > >> > > > >This part of patch is ugly. Some random numbers are checked and then > >notifier is called. We already have big table with explanation of those > >events... It is not possible to extend it with some flag or somehow > >other that value should be called via notifier? > > Nope, sparse_keymaps are a well defined API for, well, keymaps! The problem > really is this commit: > > https://git.kernel.org/cgit/linux/kernel/git/torvalds/linux.git/commit/drivers/platform/x86?id=e075b3c898e4055ec62a1f0ed7f3b8e62814bfb6 > > Which mixes status-events and key-press events in one sparse-keymap, > which happens to work because so far all the status events are > using { KE_IGNORE, 0x...., { KEY_RESERVED } }, but now we want to > actually do something and that shows that the above commit really > is a bad idea (at least for the 0x0011 type events, if we (partially) > revert that, then the ugly if goes away and I can simply insert > the dell_smbios_call_notifier() above the break in the original > switch-case handling for 0x0011 type events. > > So shall I revert the 0011 part of the mentioned commit? Does not help us, because keyboard backlight change event is also in dell_wmi_keymap_type_0000 table. Another idea: instead of struct key_entry create new structure which reflect information which comes from dell's WMI: u16 type (key, event or ignore) u16 code (wmi code) union { key, event } (linux keycode or enum notifier event) -- Pali Rohár pali.rohar@xxxxxxxxx -- To unsubscribe from this list: send the line "unsubscribe platform-driver-x86" in the body of a message to majordomo@xxxxxxxxxxxxxxx More majordomo info at http://vger.kernel.org/majordomo-info.html
