On 09/21/2010 10:40 AM, Jiri Kosina wrote:
> On Tue, 21 Sep 2010, Alan Stern wrote:
>
>> On Tue, 21 Sep 2010, Jiri Kosina wrote:
>>
>>> 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.
>>
>> Apparently the problem is that intf->minors doesn't get initialized
>> properly. This patch should fix it. Everybody, please try it out.
>>
>> Alan Stern
>>
>>
>> Index: usb-2.6/drivers/usb/core/file.c
>> ===================================================================
>> --- usb-2.6.orig/drivers/usb/core/file.c
>> +++ usb-2.6/drivers/usb/core/file.c
>> @@ -159,9 +159,9 @@ void usb_major_cleanup(void)
>> int usb_register_dev(struct usb_interface *intf,
>> struct usb_class_driver *class_driver)
>> {
>> - int retval = -EINVAL;
>> + int retval;
>> int minor_base = class_driver->minor_base;
>> - int minor = 0;
>> + int minor;
>> char name[20];
>> char *temp;
>>
>> @@ -173,12 +173,17 @@ int usb_register_dev(struct usb_interfac
>> */
>> minor_base = 0;
>> #endif
>> - intf->minor = -1;
>> -
>> - dbg ("looking for a minor, starting at %d", minor_base);
>>
>> if (class_driver->fops == NULL)
>> - goto exit;
>> + return -EINVAL;
>> + if (intf->minor >= 0)
>> + return -EADDRINUSE;
>> +
>> + retval = init_usb_class();
>> + if (retval)
>> + return retval;
>> +
>> + dev_dbg(&intf->dev, "looking for a minor, starting at %d", minor_base);
>>
>> down_write(&minor_rwsem);
>> for (minor = minor_base; minor < MAX_USB_MINORS; ++minor) {
>> @@ -186,20 +191,12 @@ int usb_register_dev(struct usb_interfac
>> continue;
>>
>> usb_minors[minor] = class_driver->fops;
>> -
>> - retval = 0;
>> + intf->minor = minor;
>> break;
>> }
>> up_write(&minor_rwsem);
>> -
>> - if (retval)
>> - goto exit;
>> -
>> - retval = init_usb_class();
>> - if (retval)
>> - goto exit;
>> -
>> - intf->minor = minor;
>> + if (intf->minor < 0)
>> + return -EXFULL;
>>
>> /* create a usb class device for this usb interface */
>> snprintf(name, sizeof(name), class_driver->name, minor - minor_base);
>> @@ -212,12 +209,12 @@ int usb_register_dev(struct usb_interfac
>> MKDEV(USB_MAJOR, minor), class_driver,
>> "%s", temp);
>> if (IS_ERR(intf->usb_dev)) {
>> + retval = PTR_ERR(intf->usb_dev);
>> down_write(&minor_rwsem);
>> - usb_minors[intf->minor] = NULL;
>> + usb_minors[minor] = NULL;
>> + intf->minor = -1;
>> up_write(&minor_rwsem);
>> - retval = PTR_ERR(intf->usb_dev);
>> }
>> -exit:
>> return retval;
>> }
>> EXPORT_SYMBOL_GPL(usb_register_dev);
>> Index: usb-2.6/drivers/usb/core/message.c
>> ===================================================================
>> --- usb-2.6.orig/drivers/usb/core/message.c
>> +++ usb-2.6/drivers/usb/core/message.c
>> @@ -1803,6 +1803,7 @@ free_interfaces:
>> intf->dev.groups = usb_interface_groups;
>> intf->dev.dma_mask = dev->dev.dma_mask;
>> INIT_WORK(&intf->reset_ws, __usb_queue_reset_device);
>> + intf->minor = -1;
>> device_initialize(&intf->dev);
>> dev_set_name(&intf->dev, "%d-%s:%d.%d",
>> dev->bus->busnum, dev->devpath,
>
> [ adding Heinz to CC ]
>
> If USB core would guarantee the initialization of intf->minor to -1, that
> would be of course nicer than having to do it myself in the driver (which
> is exactly what my previous patch has been doing).
>
> So everyone please test Alan's patch rather than mine, as it is more
> general.
>
For what it's worth, I just finished testing yours, and it works just fine. I'll try Alan's now.
Phil
--
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
