On Thu, Jan 12, 2006 at 10:41:40AM +1100, Benjamin Herrenschmidt wrote:
> On Thu, 2006-01-12 at 00:26 +0100, Michael Hanselmann wrote:
>
> > * This is the global environment of the parser. This information is
> > @@ -431,6 +433,14 @@ struct hid_device { /* device repo
> > void (*ff_exit)(struct hid_device*); /* Called by hid_exit_ff(hid) */
> > int (*ff_event)(struct hid_device *hid, struct input_dev *input,
> > unsigned int type, unsigned int code, int value);
> > +
> > +#ifdef CONFIG_USB_HIDINPUT_POWERBOOK
> > + /* We do this here because it's only relevant for the
> > + * USB devices, not for all input_dev's.
> > + */
> > + unsigned long pb_fn[NBITS(KEY_MAX)];
> > + unsigned long pb_numlock[NBITS(KEY_MAX)];
> > +#endif
> > };
>
> I don't understand the comment above ? You are adding this to all struct
> hid_device ? There can be only one of those keyboards plugged at one
> point in time, I don't think there is any problem having the above
> static in the driver rather than in the hid_device structure.
I think having it in struct hid_device is safer. We might want to
dynamically allocate only for PowerBook keyboards, though, to save
memory.
>
> .../...
>
> >
> > + if ((hid->quirks & HID_QUIRK_POWERBOOK_HAS_FN) &&
> > + hidinput_pb_event(hid, input, usage, value)) {
> > + return;
> > + }
> > +
>
> Dimitry might disagree but it's generally considered bad taste to have
> { and } for a single statement :)
I do agree, though.
--
Vojtech Pavlik
SuSE Labs, SuSE CR
-
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to [email protected]
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
[Index of Archives]
[Kernel Newbies]
[Netfilter]
[Bugtraq]
[Photo]
[Stuff]
[Gimp]
[Yosemite News]
[MIPS Linux]
[ARM Linux]
[Linux Security]
[Linux RAID]
[Video 4 Linux]
[Linux for the blind]
[Linux Resources]
