> > - case 0x10: > > - /* Keys pressed */ > > + case 0x0010: > > + /* Sequence of keys pressed */ > > for (i = 2; i < len; ++i) > > - dell_wmi_process_key(buffer_entry[i]); > > + dell_wmi_process_key(0x0010, buffer_entry[i]); > > break; > > - case 0x11: > > - for (i = 2; i < len; ++i) { > > - switch (buffer_entry[i]) { > > - case 0xfff0: > > - /* Battery unplugged */ > > - pr_debug("Battery unplugged\n"); > > - break; > > - case 0xfff1: > > - /* Battery inserted */ > > - pr_debug("Battery inserted\n"); > > - break; > > - case 0x01e1: > > - case 0x02ea: > > - case 0x02eb: > > - case 0x02ec: > > - case 0x02f6: > > - /* Keyboard backlight level changed */ > > - pr_debug("Keyboard backlight level " > > - "changed\n"); > > - break; > > - default: > > - /* Unknown event */ > > - pr_info("Unknown WMI event type 0x11: " > > - "0x%x\n", (int)buffer_entry[i]); > > - break; > > - } > > - } > > + case 0x0011: > > + /* Sequence of events occurred */ > > + for (i = 2; i < len; ++i) > > + dell_wmi_process_key(0x0011, buffer_entry[i]); > > Since this is identical to case 0x010, let's avoid the duplication of code and > handle this with a fall-through, like: > > case 0x0010: > case 0x0011: > /* Sequence of events occurred */ > for (i = 2; i < len; ++i) > dell_wmi_process_key(buffer_entry[1], > buffer_entry[i]);
I believe it was Pali's intention to make a distinction between keys being pressed (0x0010) and other events occuring (0x0011), so perhaps a comment after the first case label could be useful? case 0x0010: /* Sequence of keys pressed; fall through */ case 0x0011: /* Sequence of events occurred */ for (i = 2; i < len; ++i) dell_wmi_process_key(buffer_entry[1], buffer_entry[i]); I'll leave it to Pali to decide, I'm just throwing in my two cents. -- Best regards, Michał Kępień