Groups | Search | Server Info | Keyboard shortcuts | Login | Register [http] [https] [nntp] [nntps]
Groups > linux.kernel > #1418614
| From | Darren Hart <dvhart@infradead.org> |
|---|---|
| Newsgroups | linux.kernel |
| Subject | Re: [PATCH v2 4/4] dell-wmi: Rework code for generating sparse keymap and processing WMI events |
| Date | 2016-06-09 22:00 +0200 |
| Message-ID | <rIehs-2sV-17@gated-at.bofh.it> (permalink) |
| References | <rHyLf-GT-13@gated-at.bofh.it> <rHUC6-6hJ-29@gated-at.bofh.it> <rI6jU-5Fr-17@gated-at.bofh.it> <rI6tz-5IX-1@gated-at.bofh.it> |
| Organization | linux.* mail to news gateway |
On Thu, Jun 09, 2016 at 01:33:48PM +0200, Pali Rohár wrote:
> On Thursday 09 June 2016 13:27:21 Michał Kępień wrote:
> > > > - 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.
>
> Yes, that is truth, to visually distinguish between events and keys.
The proposed comment accomplishes that without the duplication of code.
--
Darren Hart
Intel Open Source Technology Center
Back to linux.kernel | Previous | Next — Previous in thread | Next in thread | Find similar | Unroll thread
[PATCH v2 0/4] dell-wmi: Changes in WMI event code handling Pali Rohár <pali.rohar@gmail.com> - 2016-06-08 01:40 +0200
[PATCH v2 2/4] dell-wmi: Sort WMI event codes and update comments Pali Rohár <pali.rohar@gmail.com> - 2016-06-08 01:40 +0200
[PATCH v2 3/4] dell-wmi: Add information about other WMI event codes Pali Rohár <pali.rohar@gmail.com> - 2016-06-08 01:40 +0200
[PATCH v2 4/4] dell-wmi: Rework code for generating sparse keymap and processing WMI events Pali Rohár <pali.rohar@gmail.com> - 2016-06-08 01:40 +0200
Re: [PATCH v2 4/4] dell-wmi: Rework code for generating sparse keymap and processing WMI events Darren Hart <dvhart@infradead.org> - 2016-06-09 01:00 +0200
Re: [PATCH v2 4/4] dell-wmi: Rework code for generating sparse keymap and processing WMI events Michał Kępień <kernel@kempniu.pl> - 2016-06-09 13:30 +0200
Re: [PATCH v2 4/4] dell-wmi: Rework code for generating sparse keymap and processing WMI events Pali Rohár <pali.rohar@gmail.com> - 2016-06-09 13:40 +0200
Re: [PATCH v2 4/4] dell-wmi: Rework code for generating sparse keymap and processing WMI events Darren Hart <dvhart@infradead.org> - 2016-06-09 22:00 +0200
[PATCH v2 1/4] dell-wmi: Ignore WMI event code 0xe045 Pali Rohár <pali.rohar@gmail.com> - 2016-06-08 01:40 +0200
Re: [PATCH v2 0/4] dell-wmi: Changes in WMI event code handling Michał Kępień <kernel@kempniu.pl> - 2016-06-09 14:00 +0200
csiph-web