Re: [PATCH/RFC?] usb/input: Add support for fn key on Apple PowerBooks

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:

quoted

  * 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.

  .../...

quoted

 
+	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