On Tue, 21 Sep 2010, Jiri Kosina wrote: > > [ 22.705849] HID debug: hiddev_open(): hid: ffff88013a7c0000, hiddev: (null), intf: ffff880137d0ac00 > > hiddev_open() has been called. usb_find_interface(&hid_driver, minor) > returned interface pointer 0xffff880137d0ac00. Which is bogus! Interface > 0xffff880137d0ac00 is not registered with hiddev at all, we should have > received 0xffff880137d0a400. > > We currently register minors only for those usbhid devices (through > usb_register_dev() in hiddev_connect()) which are going to be claimed by > hiddev. It doesn't seem to be problematic to me, and I don't undersntand > why usb_find_interface() returns wrong interface. > > I have just found out that it's actually CONFIG_USB_DYNAMIC_MINORS which > makes the difference. When unset, the problem doesn't trigger, and > usb_find_interface() always returns the proper interface. When > CONFIG_USB_DYNAMIC_MINORS is being used, the oops happen. > > I'll look into that. Could you guys please verify whether the patch below fixes the issues you were seeing and puts everything back into shape again? Thanks. diff --git a/drivers/hid/hid-core.c b/drivers/hid/hid-core.c index 3f72924..616b449 100644 --- a/drivers/hid/hid-core.c +++ b/drivers/hid/hid-core.c @@ -1154,6 +1154,7 @@ int hid_connect(struct hid_device *hdev, unsigned int connect_mask) char buf[64]; unsigned int i; int len; + int hiddev_minor = 0; if (hdev->quirks & HID_QUIRK_HIDDEV_FORCE) connect_mask |= (HID_CONNECT_HIDDEV_FORCE | HID_CONNECT_HIDDEV); @@ -1168,8 +1169,8 @@ int hid_connect(struct hid_device *hdev, unsigned int connect_mask) connect_mask & HID_CONNECT_HIDINPUT_FORCE)) hdev->claimed |= HID_CLAIMED_INPUT; if ((connect_mask & HID_CONNECT_HIDDEV) && hdev->hiddev_connect && - !hdev->hiddev_connect(hdev, - connect_mask & HID_CONNECT_HIDDEV_FORCE)) + ((hiddev_minor = hdev->hiddev_connect(hdev, + connect_mask & HID_CONNECT_HIDDEV_FORCE)) >= 0)) hdev->claimed |= HID_CLAIMED_HIDDEV; if ((connect_mask & HID_CONNECT_HIDRAW) && !hidraw_connect(hdev)) hdev->claimed |= HID_CLAIMED_HIDRAW; @@ -1189,7 +1190,7 @@ int hid_connect(struct hid_device *hdev, unsigned int connect_mask) len += sprintf(buf + len, "input"); if (hdev->claimed & HID_CLAIMED_HIDDEV) len += sprintf(buf + len, "%shiddev%d", len ? "," : "", - hdev->minor); + hiddev_minor); if (hdev->claimed & HID_CLAIMED_HIDRAW) len += sprintf(buf + len, "%shidraw%d", len ? "," : "", ((struct hidraw *)hdev->hidraw)->minor); diff --git a/drivers/hid/usbhid/hid-core.c b/drivers/hid/usbhid/hid-core.c index 599041a..1f0a770 100644 --- a/drivers/hid/usbhid/hid-core.c +++ b/drivers/hid/usbhid/hid-core.c @@ -1201,6 +1201,9 @@ static int usbhid_probe(struct usb_interface *intf, const struct usb_device_id * hid->driver_data = usbhid; usbhid->hid = hid; usbhid->intf = intf; + /* Only hiddev-claimed devices will have corresponding minor number. + * We can't leave it to 0, as that is valid minor as well */ + usbhid->intf->minor = -1; usbhid->ifnum = interface->desc.bInterfaceNumber; init_waitqueue_head(&usbhid->wait); diff --git a/drivers/hid/usbhid/hiddev.c b/drivers/hid/usbhid/hiddev.c index 681e620..7a81c1f 100644 --- a/drivers/hid/usbhid/hiddev.c +++ b/drivers/hid/usbhid/hiddev.c @@ -899,7 +899,7 @@ int hiddev_connect(struct hid_device *hid, unsigned int force) kfree(hiddev); return -1; } - return 0; + return usbhid->intf->minor; } /* -- Jiri Kosina SUSE Labs, Novell Inc. -- To unsubscribe from this list: send the line "unsubscribe linux-input" in the body of a message to majordomo@xxxxxxxxxxxxxxx More majordomo info at http://vger.kernel.org/majordomo-info.html
