> Hi! > > On Saturday 04 February 2017 02:26:05 Darren Hart wrote: > > Apologies, this time with Pali's correct email address (aliases fail). > > > ... > > > > > > Pali, would you care to offer a review or some testing to verify no unexpected > > > conflicts with the other dell drivers? > > I do not have Dell machine which uses intel-hid.ko so I cannot test this > patch. And obviously as it is not loaded it cannot break machines which > do not use intel-hid.ko. > > > > > +/* 5 button array notification value. */ > > > > +static const struct key_entry intel_array_keymap[] = { > > > > + { KE_KEY, 0xC2, { KEY_LEFTMETA} }, /* Press */ > > > > + { KE_IGNORE, 0xC3, { KEY_LEFTMETA} }, /* Release */ > > > > + { KE_KEY, 0xC4, { KEY_VOLUMEUP} }, /* Press */ > > > > + { KE_IGNORE, 0xC5, { KEY_VOLUMEUP} }, /* Release */ > > > > + { KE_KEY, 0xC6, { KEY_VOLUMEDOWN} }, /* Press */ > > > > + { KE_IGNORE, 0xC7, { KEY_VOLUMEDOWN} }, /* Release */ > > > > + { KE_SW, 0xC8, { .sw = {SW_ROTATE_LOCK, 1} } }, /* Press */ > > > > + { KE_SW, 0xC9, { .sw = {SW_ROTATE_LOCK, 0} } }, /* Release */ > > > > + { KE_KEY, 0xCE, { KEY_POWER} }, /* Press */ > > > > + { KE_IGNORE, 0xCF, { KEY_POWER} }, /* Release */ > > > > + { KE_END }, > > > > +}; > > This looks suspicious. Why are all release events ignored? I also do not have a Dell machine that makes use of this driver, but my understanding is that calling sparse_keymap_report_event() with autorelease set to true makes release events irrelevant and simplifies implementation. Though each release event still needs a KE_IGNORE entry in the keymap so that superfluous KEY_UNKNOWN events are suppressed. -- Best regards, Michał Kępień