usbhid: Changes since 2.6.28 in quirk handling?

usbhid: Changes since 2.6.28 in quirk handling?

[Bill Gribble]

[-- Attachment #1: Type: text/plain, Size: 1205 bytes --]

Hi, I have been maintaining a patch for a quirky USB device built-in to
a UMPC, the Raon Digital Everun.  Without the patch, the builtin
keyboard, mouse, and touchscreen are useless.  I have never submitted it
for inclusion in the kernel because there are less than 5 people who
have ever even attempted to install Linux on this device, and it's
discontinued.  However, *I* use the thing daily, and its wifi device
(libertas) has continued to get better support in the recent kernels so
I port the patch forward. 

Anyway the patch adds a quirk handler that does a mandatory
initialization step to turn the USB device on at powerup or resume.  In
recent kernels (I moved from 2.6.28, which was reliable, at 2.6.29-rc5,
I believe) this has become unreliable; about half the time the device is
not enabled at boot, and about 1 time in 10 it is disabled when resuming
from suspend-to-RAM. 

I noticed that the structure of the quirk handling code has changed
somewhat; I left my patch using the "old" way of adding an entry to
hid_blacklist in hid-quirks.c.  Is there a document or email trail that
describes the new way of structuring this quirk code?  Any pitfalls to
look out for? 

Thanks,
Bill Gribble


[-- Attachment #2: everun.patch --]
[-- Type: text/x-patch, Size: 3077 bytes --]

diff -u -r linux-2.6-stock/drivers/hid/usbhid/hid-core.c linux-2.6/drivers/hid/usbhid/hid-core.c
--- linux-2.6-stock/drivers/hid/usbhid/hid-core.c	2009-03-13 08:28:42.000000000 -0400
+++ linux-2.6/drivers/hid/usbhid/hid-core.c	2009-03-13 08:05:48.000000000 -0400
@@ -697,6 +697,35 @@
 	usb_buffer_free(dev, usbhid->bufsize, usbhid->ctrlbuf, usbhid->ctrlbuf_dma);
 }
 
+/*
+ * Sending HID_REQ_SET_REPORT changes the operation mode of the
+ * Microchip controller in the Raon Everun to "operational".  Without
+ * this, the keyboard/mouse/touchpad will not send any events
+ */
+static void hid_fixup_microchip_exphid(struct usb_device *dev, int ifnum)
+{
+       int result;
+       char *buf = kmalloc(1, GFP_KERNEL);
+
+       if (!buf)
+               return;
+
+       buf[0] = 0;
+       dbg_hid("exphid quirk, iface=%d", ifnum);
+       /* only do this for interface 0, the keyboard */
+       if (ifnum == 0) {
+               result = usb_control_msg(dev, usb_sndctrlpipe(dev, 0),
+                                        HID_REQ_SET_REPORT,
+                                        USB_TYPE_CLASS | USB_RECIP_INTERFACE,
+                                        0x0200, 0x00, buf, 1, USB_CTRL_GET_TIMEOUT);
+
+               if (result < 0)
+                       err_hid("%s failed: %d\n", __func__, result);
+       }
+
+       kfree(buf);
+}
+
 static int usbhid_parse(struct hid_device *hid)
 {
 	struct usb_interface *intf = to_usb_interface(hid->dev.parent);
@@ -772,6 +801,7 @@
 	return ret;
 }
 
+
 static int usbhid_start(struct hid_device *hid)
 {
 	struct usb_interface *intf = to_usb_interface(hid->dev.parent);
@@ -801,6 +831,10 @@
 		goto fail;
 	}
 
+        if (hid->quirks & HID_QUIRK_MICROCHIP_EXPHID)
+                hid_fixup_microchip_exphid(interface_to_usbdev(intf),
+                        intf->cur_altsetting->desc.bInterfaceNumber);
+
 	for (n = 0; n < interface->desc.bNumEndpoints; n++) {
 		struct usb_endpoint_descriptor *endpoint;
 		int pipe;
@@ -1075,10 +1109,16 @@
 	if (!test_bit(HID_STARTED, &usbhid->iofl))
 		return 0;
 
+        if (hid->quirks & HID_QUIRK_MICROCHIP_EXPHID)
+                hid_fixup_microchip_exphid(interface_to_usbdev(intf),
+                        intf->cur_altsetting->desc.bInterfaceNumber);
+
 	clear_bit(HID_SUSPENDED, &usbhid->iofl);
 	usbhid->retry_delay = 0;
 	status = hid_start_in(hid);
 	dev_dbg(&intf->dev, "resume status %d\n", status);
+
+
 	return status;
 }
 
diff -u -r linux-2.6-stock/drivers/hid/usbhid/hid-quirks.c linux-2.6/drivers/hid/usbhid/hid-quirks.c
--- linux-2.6-stock/drivers/hid/usbhid/hid-quirks.c	2009-03-13 08:28:42.000000000 -0400
+++ linux-2.6/drivers/hid/usbhid/hid-quirks.c	2009-03-13 08:05:48.000000000 -0400
@@ -61,6 +61,8 @@
 	{ USB_VENDOR_ID_WISEGROUP_LTD, USB_DEVICE_ID_SMARTJOY_DUAL_PLUS, HID_QUIRK_NOGET | HID_QUIRK_MULTI_INPUT },
 	{ USB_VENDOR_ID_WISEGROUP_LTD2, USB_DEVICE_ID_SMARTJOY_DUAL_PLUS, HID_QUIRK_NOGET | HID_QUIRK_MULTI_INPUT },
 
+        { USB_VENDOR_ID_MICROCHIP, USB_DEVICE_ID_MICROCHIP_EXPHID, HID_QUIRK_MICROCHIP_EXPHID },
+
 	{ 0, 0 }
 };
 

Re: usbhid: Changes since 2.6.28 in quirk handling?

[Jiri Kosina]

On Fri, 13 Mar 2009, Bill Gribble wrote:

> Hi, I have been maintaining a patch for a quirky USB device built-in to 
> a UMPC, the Raon Digital Everun.  Without the patch, the builtin 
> keyboard, mouse, and touchscreen are useless.  I have never submitted it 
> for inclusion in the kernel because there are less than 5 people who 
> have ever even attempted to install Linux on this device, and it's 
> discontinued.  However, *I* use the thing daily, and its wifi device 
> (libertas) has continued to get better support in the recent kernels so 
> I port the patch forward.

Hi Bill,

we have drivers (and even whole architectures) supported by the kernel, 
which have less than 5 users in the world, that's not a big deal.

It makes 5 people happy, and saves you the pain from having to keep the 
out-of-tree code working with the Linux kernel API changing all the time.

> I noticed that the structure of the quirk handling code has changed 
> somewhat; I left my patch using the "old" way of adding an entry to 
> hid_blacklist in hid-quirks.c.  Is there a document or email trail that 
> describes the new way of structuring this quirk code?  Any pitfalls to 
> look out for?

Looking at your patch, the only thing you need is sending a special URB to 
the device in order to make it operational, right?

Please look at hid-sony driver, which does exactly that for some of the 
PS3 devices.

You then only need to add your device to hid_blacklist[], so that generic 
driver doesn't get bound to it and it'd be driven by your special driver.

-- 
Jiri Kosina
SUSE Labs

Re: usbhid: Changes since 2.6.28 in quirk handling?

[Bill Gribble]

On Wed, 2009-03-18 at 15:14 +0100, Jiri Kosina wrote:
> we have drivers (and even whole architectures) supported by the kernel, 
> which have less than 5 users in the world, that's not a big deal.

Great to hear that!  I will try to clean up my patches according to the
guidelines and send them in.  

> Looking at your patch, the only thing you need is sending a special URB to 
> the device in order to make it operational, right?

That's right. 

> Please look at hid-sony driver, which does exactly that for some of the 
> PS3 devices.
> 
> You then only need to add your device to hid_blacklist[], so that generic 
> driver doesn't get bound to it and it'd be driven by your special driver.

OK, thanks for the advice.  I will do that. 

Bill Gribble