> > -           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ń

Reply via email to