[PATCH 1/2] HID: add hid_type

[PATCH 1/2] HID: add hid_type

[Jiri Slaby]

Add type to the hid structure to distinguish to which device type
(mouse/kbd) we are talking to. Needed for per device type ignore
list support.

Note: this patch leaves the type as unknown for bluetooth devices,
there is not support for this in the hidp code.

Signed-off-by: Jiri Slaby <jirislaby@gmail.com>
---
 drivers/hid/usbhid/hid-core.c |    8 ++++++++
 include/linux/hid.h           |    7 +++++++
 2 files changed, 15 insertions(+), 0 deletions(-)

diff --git a/drivers/hid/usbhid/hid-core.c b/drivers/hid/usbhid/hid-core.c
index 1d3b8a3..20617d8 100644
--- a/drivers/hid/usbhid/hid-core.c
+++ b/drivers/hid/usbhid/hid-core.c
@@ -972,6 +972,14 @@ static int hid_probe(struct usb_interface *intf, const struct usb_device_id *id)
 	hid->vendor = le16_to_cpu(dev->descriptor.idVendor);
 	hid->product = le16_to_cpu(dev->descriptor.idProduct);
 	hid->name[0] = 0;
+	switch (intf->cur_altsetting->desc.bInterfaceProtocol) {
+	case USB_INTERFACE_PROTOCOL_KEYBOARD:
+		hid->type = HID_TYPE_KEYBOARD;
+		break;
+	case USB_INTERFACE_PROTOCOL_MOUSE:
+		hid->type = HID_TYPE_MOUSE;
+		break;
+	}
 
 	if (dev->manufacturer)
 		strlcpy(hid->name, dev->manufacturer, sizeof(hid->name));
diff --git a/include/linux/hid.h b/include/linux/hid.h
index f13bca2..36a3953 100644
--- a/include/linux/hid.h
+++ b/include/linux/hid.h
@@ -417,6 +417,12 @@ struct hid_input {
 	struct input_dev *input;
 };
 
+enum hid_type {
+	HID_TYPE_UNKNOWN = 0,
+	HID_TYPE_MOUSE,
+	HID_TYPE_KEYBOARD
+};
+
 struct hid_driver;
 struct hid_ll_driver;
 
@@ -431,6 +437,7 @@ struct hid_device {							/* device report descriptor */
 	__u32 vendor;							/* Vendor ID */
 	__u32 product;							/* Product ID */
 	__u32 version;							/* HID version */
+	enum hid_type type;						/* device type (mouse, kbd, ...) */
 	unsigned country;						/* HID country */
 	struct hid_report_enum report_enum[HID_REPORT_TYPES];
 
-- 
1.6.0.2

[PATCH 2/2] HID: fix appletouch regression

[Jiri Slaby]

The appletouch mouse devices are grabbed by the hid bus and not
released even if apple driver says ENODEV (as expected).

Move the ignoration one level upper to deny the hid layer to grab
the device and return error to the usb hid which, as a result,
releases the device.

Otherwise input/mouse/appletouch and others needn't be attached.

Reported-by: Justin Mattock <justinmattock@gmail.com>
Reported-by: Steven Noonan <steven@uplinklabs.net>
Signed-off-by: Jiri Slaby <jirislaby@gmail.com>
---
 drivers/hid/hid-apple.c |   63 ++++++++++++++++------------------------------
 drivers/hid/hid-core.c  |   33 ++++++++++++++++++++++++
 2 files changed, 55 insertions(+), 41 deletions(-)

diff --git a/drivers/hid/hid-apple.c b/drivers/hid/hid-apple.c
index fd7f896..c6ab4ba 100644
--- a/drivers/hid/hid-apple.c
+++ b/drivers/hid/hid-apple.c
@@ -312,13 +312,6 @@ static int apple_probe(struct hid_device *hdev,
 	unsigned int connect_mask = HID_CONNECT_DEFAULT;
 	int ret;
 
-	/* return something else or move to hid layer? device will reside
-	   allocated */
-	if (id->bus == BUS_USB && (quirks & APPLE_IGNORE_MOUSE) &&
-			to_usb_interface(hdev->dev.parent)->cur_altsetting->
-			desc.bInterfaceProtocol == USB_INTERFACE_PROTOCOL_MOUSE)
-		return -ENODEV;
-
 	asc = kzalloc(sizeof(*asc), GFP_KERNEL);
 	if (asc == NULL) {
 		dev_err(&hdev->dev, "can't alloc apple descriptor\n");
@@ -367,38 +360,32 @@ static const struct hid_device_id apple_devices[] = {
 		.driver_data = APPLE_MIGHTYMOUSE | APPLE_INVERT_HWHEEL },
 
 	{ HID_USB_DEVICE(USB_VENDOR_ID_APPLE, USB_DEVICE_ID_APPLE_FOUNTAIN_ANSI),
-		.driver_data = APPLE_NUMLOCK_EMULATION | APPLE_HAS_FN |
-			APPLE_IGNORE_MOUSE },
+		.driver_data = APPLE_NUMLOCK_EMULATION | APPLE_HAS_FN },
 	{ HID_USB_DEVICE(USB_VENDOR_ID_APPLE, USB_DEVICE_ID_APPLE_FOUNTAIN_ISO),
-		.driver_data = APPLE_NUMLOCK_EMULATION | APPLE_HAS_FN |
-			APPLE_IGNORE_MOUSE },
+		.driver_data = APPLE_NUMLOCK_EMULATION | APPLE_HAS_FN },
 	{ HID_USB_DEVICE(USB_VENDOR_ID_APPLE, USB_DEVICE_ID_APPLE_GEYSER_ANSI),
-		.driver_data = APPLE_NUMLOCK_EMULATION | APPLE_HAS_FN |
-			APPLE_IGNORE_MOUSE },
+		.driver_data = APPLE_NUMLOCK_EMULATION | APPLE_HAS_FN },
 	{ HID_USB_DEVICE(USB_VENDOR_ID_APPLE, USB_DEVICE_ID_APPLE_GEYSER_ISO),
 		.driver_data = APPLE_NUMLOCK_EMULATION | APPLE_HAS_FN |
-			APPLE_IGNORE_MOUSE | APPLE_ISO_KEYBOARD },
+			APPLE_ISO_KEYBOARD },
 	{ HID_USB_DEVICE(USB_VENDOR_ID_APPLE, USB_DEVICE_ID_APPLE_GEYSER_JIS),
-		.driver_data = APPLE_NUMLOCK_EMULATION | APPLE_HAS_FN |
-			APPLE_IGNORE_MOUSE },
+		.driver_data = APPLE_NUMLOCK_EMULATION | APPLE_HAS_FN },
 	{ HID_USB_DEVICE(USB_VENDOR_ID_APPLE, USB_DEVICE_ID_APPLE_GEYSER3_ANSI),
-		.driver_data = APPLE_NUMLOCK_EMULATION | APPLE_HAS_FN |
-			APPLE_IGNORE_MOUSE },
+		.driver_data = APPLE_NUMLOCK_EMULATION | APPLE_HAS_FN },
 	{ HID_USB_DEVICE(USB_VENDOR_ID_APPLE, USB_DEVICE_ID_APPLE_GEYSER3_ISO),
 		.driver_data = APPLE_NUMLOCK_EMULATION | APPLE_HAS_FN |
-			APPLE_IGNORE_MOUSE | APPLE_ISO_KEYBOARD },
+			APPLE_ISO_KEYBOARD },
 	{ HID_USB_DEVICE(USB_VENDOR_ID_APPLE, USB_DEVICE_ID_APPLE_GEYSER3_JIS),
 		.driver_data = APPLE_NUMLOCK_EMULATION | APPLE_HAS_FN |
-			APPLE_IGNORE_MOUSE | APPLE_RDESC_JIS },
+			APPLE_RDESC_JIS },
 	{ HID_USB_DEVICE(USB_VENDOR_ID_APPLE, USB_DEVICE_ID_APPLE_GEYSER4_ANSI),
-		.driver_data = APPLE_NUMLOCK_EMULATION | APPLE_HAS_FN |
-			APPLE_IGNORE_MOUSE },
+		.driver_data = APPLE_NUMLOCK_EMULATION | APPLE_HAS_FN },
 	{ HID_USB_DEVICE(USB_VENDOR_ID_APPLE, USB_DEVICE_ID_APPLE_GEYSER4_ISO),
 		.driver_data = APPLE_NUMLOCK_EMULATION | APPLE_HAS_FN |
-			APPLE_IGNORE_MOUSE | APPLE_ISO_KEYBOARD },
+			APPLE_ISO_KEYBOARD },
 	{ HID_USB_DEVICE(USB_VENDOR_ID_APPLE, USB_DEVICE_ID_APPLE_GEYSER4_JIS),
 		.driver_data = APPLE_NUMLOCK_EMULATION | APPLE_HAS_FN |
-			APPLE_IGNORE_MOUSE | APPLE_RDESC_JIS},
+			APPLE_RDESC_JIS },
 	{ HID_USB_DEVICE(USB_VENDOR_ID_APPLE, USB_DEVICE_ID_APPLE_ALU_ANSI),
 		.driver_data = APPLE_HAS_FN },
 	{ HID_USB_DEVICE(USB_VENDOR_ID_APPLE, USB_DEVICE_ID_APPLE_ALU_ISO),
@@ -406,14 +393,12 @@ static const struct hid_device_id apple_devices[] = {
 	{ HID_USB_DEVICE(USB_VENDOR_ID_APPLE, USB_DEVICE_ID_APPLE_ALU_JIS),
 		.driver_data = APPLE_HAS_FN },
 	{ HID_USB_DEVICE(USB_VENDOR_ID_APPLE, USB_DEVICE_ID_APPLE_GEYSER4_HF_ANSI),
-		.driver_data = APPLE_NUMLOCK_EMULATION | APPLE_HAS_FN |
-			APPLE_IGNORE_MOUSE },
+		.driver_data = APPLE_NUMLOCK_EMULATION | APPLE_HAS_FN },
 	{ HID_USB_DEVICE(USB_VENDOR_ID_APPLE, USB_DEVICE_ID_APPLE_GEYSER4_HF_ISO),
-		.driver_data = APPLE_NUMLOCK_EMULATION | APPLE_HAS_FN |
-			APPLE_IGNORE_MOUSE },
+		.driver_data = APPLE_NUMLOCK_EMULATION | APPLE_HAS_FN },
 	{ HID_USB_DEVICE(USB_VENDOR_ID_APPLE, USB_DEVICE_ID_APPLE_GEYSER4_HF_JIS),
 		.driver_data = APPLE_NUMLOCK_EMULATION | APPLE_HAS_FN |
-			APPLE_IGNORE_MOUSE | APPLE_RDESC_JIS },
+			APPLE_RDESC_JIS },
 	{ HID_USB_DEVICE(USB_VENDOR_ID_APPLE, USB_DEVICE_ID_APPLE_ALU_WIRELESS_ANSI),
 		.driver_data = APPLE_NUMLOCK_EMULATION | APPLE_HAS_FN },
 	{ HID_USB_DEVICE(USB_VENDOR_ID_APPLE, USB_DEVICE_ID_APPLE_ALU_WIRELESS_ISO),
@@ -422,25 +407,21 @@ static const struct hid_device_id apple_devices[] = {
 	{ HID_USB_DEVICE(USB_VENDOR_ID_APPLE, USB_DEVICE_ID_APPLE_ALU_WIRELESS_JIS),
 		.driver_data = APPLE_NUMLOCK_EMULATION | APPLE_HAS_FN },
 	{ HID_USB_DEVICE(USB_VENDOR_ID_APPLE, USB_DEVICE_ID_APPLE_WELLSPRING_ANSI),
-		.driver_data = APPLE_HAS_FN | APPLE_IGNORE_MOUSE },
+		.driver_data = APPLE_HAS_FN },
 	{ HID_USB_DEVICE(USB_VENDOR_ID_APPLE, USB_DEVICE_ID_APPLE_WELLSPRING_ISO),
-		.driver_data = APPLE_HAS_FN | APPLE_ISO_KEYBOARD |
-			APPLE_IGNORE_MOUSE },
+		.driver_data = APPLE_HAS_FN | APPLE_ISO_KEYBOARD },
 	{ HID_USB_DEVICE(USB_VENDOR_ID_APPLE, USB_DEVICE_ID_APPLE_WELLSPRING_JIS),
-		.driver_data = APPLE_HAS_FN | APPLE_IGNORE_MOUSE | APPLE_RDESC_JIS },
+		.driver_data = APPLE_HAS_FN | APPLE_RDESC_JIS },
 	{ HID_USB_DEVICE(USB_VENDOR_ID_APPLE, USB_DEVICE_ID_APPLE_WELLSPRING2_ANSI),
-		.driver_data = APPLE_HAS_FN | APPLE_IGNORE_MOUSE },
+		.driver_data = APPLE_HAS_FN },
 	{ HID_USB_DEVICE(USB_VENDOR_ID_APPLE, USB_DEVICE_ID_APPLE_WELLSPRING2_ISO),
-		.driver_data = APPLE_HAS_FN | APPLE_ISO_KEYBOARD |
-			APPLE_IGNORE_MOUSE },
+		.driver_data = APPLE_HAS_FN | APPLE_ISO_KEYBOARD },
 	{ HID_USB_DEVICE(USB_VENDOR_ID_APPLE, USB_DEVICE_ID_APPLE_WELLSPRING2_JIS),
-		.driver_data = APPLE_HAS_FN | APPLE_IGNORE_MOUSE | APPLE_RDESC_JIS },
+		.driver_data = APPLE_HAS_FN | APPLE_RDESC_JIS },
 	{ HID_USB_DEVICE(USB_VENDOR_ID_APPLE, USB_DEVICE_ID_APPLE_FOUNTAIN_TP_ONLY),
-		.driver_data = APPLE_NUMLOCK_EMULATION | APPLE_HAS_FN |
-			APPLE_IGNORE_MOUSE },
+		.driver_data = APPLE_NUMLOCK_EMULATION | APPLE_HAS_FN },
 	{ HID_USB_DEVICE(USB_VENDOR_ID_APPLE, USB_DEVICE_ID_APPLE_GEYSER1_TP_ONLY),
-		.driver_data = APPLE_NUMLOCK_EMULATION | APPLE_HAS_FN |
-			APPLE_IGNORE_MOUSE },
+		.driver_data = APPLE_NUMLOCK_EMULATION | APPLE_HAS_FN },
 
 	/* Apple wireless Mighty Mouse */
 	{ HID_BLUETOOTH_DEVICE(USB_VENDOR_ID_APPLE, 0x030c),
diff --git a/drivers/hid/hid-core.c b/drivers/hid/hid-core.c
index 8a7d9db..90bdc6f 100644
--- a/drivers/hid/hid-core.c
+++ b/drivers/hid/hid-core.c
@@ -1539,6 +1539,35 @@ static const struct hid_device_id hid_ignore_list[] = {
 	{ }
 };
 
+/**
+ * hid_mouse_ignore_list - mouse devices which must not be held by the hid layer
+ */
+static const struct hid_device_id hid_mouse_ignore_list[] = {
+	{ HID_USB_DEVICE(USB_VENDOR_ID_APPLE, USB_DEVICE_ID_APPLE_FOUNTAIN_ANSI) },
+	{ HID_USB_DEVICE(USB_VENDOR_ID_APPLE, USB_DEVICE_ID_APPLE_FOUNTAIN_ISO) },
+	{ HID_USB_DEVICE(USB_VENDOR_ID_APPLE, USB_DEVICE_ID_APPLE_GEYSER_ANSI) },
+	{ HID_USB_DEVICE(USB_VENDOR_ID_APPLE, USB_DEVICE_ID_APPLE_GEYSER_ISO) },
+	{ HID_USB_DEVICE(USB_VENDOR_ID_APPLE, USB_DEVICE_ID_APPLE_GEYSER_JIS) },
+	{ HID_USB_DEVICE(USB_VENDOR_ID_APPLE, USB_DEVICE_ID_APPLE_GEYSER3_ANSI) },
+	{ HID_USB_DEVICE(USB_VENDOR_ID_APPLE, USB_DEVICE_ID_APPLE_GEYSER3_ISO) },
+	{ HID_USB_DEVICE(USB_VENDOR_ID_APPLE, USB_DEVICE_ID_APPLE_GEYSER3_JIS) },
+	{ HID_USB_DEVICE(USB_VENDOR_ID_APPLE, USB_DEVICE_ID_APPLE_GEYSER4_ANSI) },
+	{ HID_USB_DEVICE(USB_VENDOR_ID_APPLE, USB_DEVICE_ID_APPLE_GEYSER4_ISO) },
+	{ HID_USB_DEVICE(USB_VENDOR_ID_APPLE, USB_DEVICE_ID_APPLE_GEYSER4_JIS) },
+	{ HID_USB_DEVICE(USB_VENDOR_ID_APPLE, USB_DEVICE_ID_APPLE_GEYSER4_HF_ANSI) },
+	{ HID_USB_DEVICE(USB_VENDOR_ID_APPLE, USB_DEVICE_ID_APPLE_GEYSER4_HF_ISO) },
+	{ HID_USB_DEVICE(USB_VENDOR_ID_APPLE, USB_DEVICE_ID_APPLE_GEYSER4_HF_JIS) },
+	{ HID_USB_DEVICE(USB_VENDOR_ID_APPLE, USB_DEVICE_ID_APPLE_WELLSPRING_ANSI) },
+	{ HID_USB_DEVICE(USB_VENDOR_ID_APPLE, USB_DEVICE_ID_APPLE_WELLSPRING_ISO) },
+	{ HID_USB_DEVICE(USB_VENDOR_ID_APPLE, USB_DEVICE_ID_APPLE_WELLSPRING_JIS) },
+	{ HID_USB_DEVICE(USB_VENDOR_ID_APPLE, USB_DEVICE_ID_APPLE_WELLSPRING2_ANSI) },
+	{ HID_USB_DEVICE(USB_VENDOR_ID_APPLE, USB_DEVICE_ID_APPLE_WELLSPRING2_ISO) },
+	{ HID_USB_DEVICE(USB_VENDOR_ID_APPLE, USB_DEVICE_ID_APPLE_WELLSPRING2_JIS) },
+	{ HID_USB_DEVICE(USB_VENDOR_ID_APPLE, USB_DEVICE_ID_APPLE_FOUNTAIN_TP_ONLY) },
+	{ HID_USB_DEVICE(USB_VENDOR_ID_APPLE, USB_DEVICE_ID_APPLE_GEYSER1_TP_ONLY) },
+	{ }
+};
+
 static bool hid_ignore(struct hid_device *hdev)
 {
 	switch (hdev->vendor) {
@@ -1555,6 +1584,10 @@ static bool hid_ignore(struct hid_device *hdev)
 		break;
 	}
 
+	if (hdev->type == HID_TYPE_MOUSE &&
+			hid_match_id(hdev, hid_mouse_ignore_list))
+		return true;
+
 	return !!hid_match_id(hdev, hid_ignore_list);
 }
 
-- 
1.6.0.2

Re: [PATCH 1/2] HID: add hid_type

[Justin Mattock]

On Sun, Oct 19, 2008 at 7:15 AM, Jiri Slaby <jirislaby@gmail.com> wrote:
> Add type to the hid structure to distinguish to which device type
> (mouse/kbd) we are talking to. Needed for per device type ignore
> list support.
>
> Note: this patch leaves the type as unknown for bluetooth devices,
> there is not support for this in the hidp code.
>
> Signed-off-by: Jiri Slaby <jirislaby@gmail.com>
> ---
>  drivers/hid/usbhid/hid-core.c |    8 ++++++++
>  include/linux/hid.h           |    7 +++++++
>  2 files changed, 15 insertions(+), 0 deletions(-)
>
> diff --git a/drivers/hid/usbhid/hid-core.c b/drivers/hid/usbhid/hid-core.c
> index 1d3b8a3..20617d8 100644
> --- a/drivers/hid/usbhid/hid-core.c
> +++ b/drivers/hid/usbhid/hid-core.c
> @@ -972,6 +972,14 @@ static int hid_probe(struct usb_interface *intf, const struct usb_device_id *id)
>        hid->vendor = le16_to_cpu(dev->descriptor.idVendor);
>        hid->product = le16_to_cpu(dev->descriptor.idProduct);
>        hid->name[0] = 0;
> +       switch (intf->cur_altsetting->desc.bInterfaceProtocol) {
> +       case USB_INTERFACE_PROTOCOL_KEYBOARD:
> +               hid->type = HID_TYPE_KEYBOARD;
> +               break;
> +       case USB_INTERFACE_PROTOCOL_MOUSE:
> +               hid->type = HID_TYPE_MOUSE;
> +               break;
> +       }
>
>        if (dev->manufacturer)
>                strlcpy(hid->name, dev->manufacturer, sizeof(hid->name));
> diff --git a/include/linux/hid.h b/include/linux/hid.h
> index f13bca2..36a3953 100644
> --- a/include/linux/hid.h
> +++ b/include/linux/hid.h
> @@ -417,6 +417,12 @@ struct hid_input {
>        struct input_dev *input;
>  };
>
> +enum hid_type {
> +       HID_TYPE_UNKNOWN = 0,
> +       HID_TYPE_MOUSE,
> +       HID_TYPE_KEYBOARD
> +};
> +
>  struct hid_driver;
>  struct hid_ll_driver;
>
> @@ -431,6 +437,7 @@ struct hid_device {                                                 /* device report descriptor */
>        __u32 vendor;                                                   /* Vendor ID */
>        __u32 product;                                                  /* Product ID */
>        __u32 version;                                                  /* HID version */
> +       enum hid_type type;                                             /* device type (mouse, kbd, ...) */
>        unsigned country;                                               /* HID country */
>        struct hid_report_enum report_enum[HID_REPORT_TYPES];
>
> --
> 1.6.0.2
>
>

O.k. reverted the old patch and
applied the two new ones.
appletouchpad is working;
I didn't use patch -p1,
just manually applied the two.

-- 
Justin P. Mattock

Re: several messages

[Jiri Kosina]

On Sun, 19 Oct 2008, Jiri Slaby wrote:

> +enum hid_type {
> +	HID_TYPE_UNKNOWN = 0,
> +	HID_TYPE_MOUSE,
> +	HID_TYPE_KEYBOARD
> +};
> +

Do we really need the HID_TYPE_KEYBOARD at all? It's not used anywhere in 
the code. I'd propose to add it when it is actually needed. I.e. have the 
enum contain something like HID_TYPE_MOUSE HID_TYPE_OTHER for now, and add 
whatever will become necessary in the future, what do you think?


On Sun, 19 Oct 2008, Jiri Slaby wrote:

> +/**
> + * hid_mouse_ignore_list - mouse devices which must not be held by the hid layer
> + */

I think a more descriptive comment would be appropriate here. It might not 
be obvious on the first sight why this needs to be done separately from 
the generic hid_blacklist. I.e. something like

/** 
 * There are composite devices for which we want to ignore only a certain
 * interface. This is a list of devices for which only the mouse interface 
 * will be ignored.
 */

maybe?

Thanks,

-- 
Jiri Kosina
SUSE Labs

Re: several messages

[Justin Mattock]

On Sun, Oct 19, 2008 at 12:40 PM, Jiri Kosina <jkosina@suse.cz> wrote:
> On Sun, 19 Oct 2008, Jiri Slaby wrote:
>
>> +enum hid_type {
>> +     HID_TYPE_UNKNOWN = 0,
>> +     HID_TYPE_MOUSE,
>> +     HID_TYPE_KEYBOARD
>> +};
>> +
>
> Do we really need the HID_TYPE_KEYBOARD at all? It's not used anywhere in
> the code. I'd propose to add it when it is actually needed. I.e. have the
> enum contain something like HID_TYPE_MOUSE HID_TYPE_OTHER for now, and add
> whatever will become necessary in the future, what do you think?
>
>
> On Sun, 19 Oct 2008, Jiri Slaby wrote:
>
>> +/**
>> + * hid_mouse_ignore_list - mouse devices which must not be held by the hid layer
>> + */
>
> I think a more descriptive comment would be appropriate here. It might not
> be obvious on the first sight why this needs to be done separately from
> the generic hid_blacklist. I.e. something like
>
> /**
>  * There are composite devices for which we want to ignore only a certain
>  * interface. This is a list of devices for which only the mouse interface
>  * will be ignored.
>  */
>
> maybe?
>
> Thanks,
>
> --
> Jiri Kosina
> SUSE Labs
>

I can agree with that, whats the point having something
there if it not being used,(just eating up precious space);

-- 
Justin P. Mattock

Re: several messages

[Jiri Slaby]

Jiri Kosina napsal(a):
> On Sun, 19 Oct 2008, Jiri Slaby wrote:
> 
>> +enum hid_type {
>> +	HID_TYPE_UNKNOWN = 0,
>> +	HID_TYPE_MOUSE,
>> +	HID_TYPE_KEYBOARD
>> +};
>> +
> 
> Do we really need the HID_TYPE_KEYBOARD at all? It's not used anywhere in 
> the code. I'd propose to add it when it is actually needed. I.e. have the 
> enum contain something like HID_TYPE_MOUSE HID_TYPE_OTHER for now, and add 
> whatever will become necessary in the future, what do you think?

I would use unknown rather than other, since on bluetooth mouse is unknown
not other, if you don't mind?

Or did you mean tristate unknown, mouse and other?

Thanks for review.

[PATCH 2/2] HID: fix appletouch regression (was Re: several messages)

[Jiri Kosina]

[ restored previous subject, sorry for the "several messagess" mess, 
alpine didn't handle multi-message reply with different subjects nicely ]

On Mon, 20 Oct 2008, Jiri Slaby wrote:

> >> +enum hid_type {
> >> +	HID_TYPE_UNKNOWN = 0,
> >> +	HID_TYPE_MOUSE,
> >> +	HID_TYPE_KEYBOARD
> > Do we really need the HID_TYPE_KEYBOARD at all? It's not used anywhere 
> > in the code. I'd propose to add it when it is actually needed. I.e. 
> > have the enum contain something like HID_TYPE_MOUSE HID_TYPE_OTHER for 
> > now, and add whatever will become necessary in the future, what do you 
> > think?
> I would use unknown rather than other, since on bluetooth mouse is unknown
> not other, if you don't mind?
> Or did you mean tristate unknown, mouse and other?

Well, basically the points I am trying to make:

1) the code doesn't currently need HID_TYPE_KEYBOARD at all

and

2) by definition, HID are not just mice and keyboards, so adding just 
   these two doesn't look particularly correct, if just one of them is 
   needed

For bluetooth we even don't have the possibility to distinguish these, 
yes. So probably something like HID_TYPE_USBMOUSE/HID_TYPE_OTHER for now, 
and we could extend the enum whenever the need to recognize a different 
type comes up?

Thanks,

-- 
Jiri Kosina
SUSE Labs