* Michal Nazarewicz | 2011-06-04 21:18:40 [+0200]:
>I've looked at the patch and found no big problems with it but it would
>probably be best if respective authors took a look.
Once nobody disagrees with parts of the patch of the patch I Cc each one
of them :)
>On Fri, 03 Jun 2011 20:07:56 +0200, Sebastian Andrzej Siewior wrote:
>>diff --git a/drivers/usb/gadget/at91_udc.c @@ -1853,13 +1857,18 @@
>>static int __init at91udc_probe(struct platform_device *pdev)
>> DBG("no VBUS detection, assuming always-on\n");
>> udc->vbus = 1;
>> }
>>- dev_set_drvdata(dev, udc);
>>+ retval = usb_add_gadget_udc(dev, &udc->gadget);
>>+ if (retval)
>>+ goto fail4;
>>+ dev_set_drvdata(dev, &udc->gadget);
>
>Why did you change the drvdata from udc to &udc->gadget? Not that I can
>find a place where the drvdata is used.
It is a mistake, not sure how this happened. It should remain udc, it is
used in at91udc_remove(). Thanks for noticing.
>> device_init_wakeup(dev, 1);
>> create_debug_file(udc);
>> INFO("%s version %s\n", driver_name, DRIVER_VERSION);
>> return 0;
>>-
>>+fail4:
>>+ if (udc->board.vbus_pin > 0 && !udc->board.vbus_polled)
>>+ free_irq(udc->board.vbus_pin, udc);
>> fail3:
>> if (udc->board.vbus_pin > 0)
>> gpio_free(udc->board.vbus_pin);
>
>
>>diff --git a/drivers/usb/gadget/net2280.c b/drivers/usb/gadget/net2280.c
>>@@ -2915,6 +2920,9 @@ static int net2280_probe (struct pci_dev
>>*pdev, const struct pci_device_id *id)
>> retval = device_create_file (&pdev->dev, &dev_attr_registers);
>> if (retval) goto done;
>>+ retval = usb_add_gadget_udc(&pdev->dev, &dev->gadget);
>>+ if (retval)
>>+ goto done;
>
>Since the other two instances use ???if (retval) goto done;??? on a single line
>it would be justifiable here as well.
>
>
>>diff --git a/drivers/usb/gadget/pxa27x_udc.c @@ -1860,8 +1866,6 @@
>>add_fail:
>> udc->gadget.dev.driver = NULL;
>> return retval;
>> }
>>-EXPORT_SYMBOL(usb_gadget_probe_driver);
>>-
>
>You've deleted one line too much.
Isn't one line between between function and comment not enough?
>>/**
>> * stop_activity - Stops udc endpoints
>
>
>>diff --git a/drivers/usb/musb/musb_gadget.c @@ -1836,6 +1842,15 @@
>>int __init musb_gadget_setup(struct musb *musb)
>> put_device(&musb->g.dev);
>> the_gadget = NULL;
>
>???return??? statement needed here.
right.
>> }
>>+ status = usb_add_gadget_udc(musb->controller, &musb->g);
>>+ if (status)
>>+ goto err;
>>+
>>+ return status;
>>+
>>+err:
>>+ device_unregister(&musb->g.dev);
>>+ the_gadget = NULL;
>
>Alternatively, with no goto:
>
> if (status) {
> device_unregister(...);
> the_gadget = NULL;
> }
>
>> return status;
>> }
oki
>>@@ -1962,7 +1978,6 @@ err1:
>> err0:
>> return retval;
>> }
>>-EXPORT_SYMBOL(usb_gadget_probe_driver);
>
>Empty line needed.
Where? There is one between } and the begin og the next function.
>>static void stop_activity(struct musb *musb, struct
>>usb_gadget_driver *driver)
>> {
>
>>@@ -2071,8 +2086,6 @@ int usb_gadget_unregister_driver(struct
>>usb_gadget_driver *driver)
>> return 0;
>> }
>>-EXPORT_SYMBOL(usb_gadget_unregister_driver);
>>-
>
>Empty line needed.
How so?
Sebastian
--
To unsubscribe from this list: send the line "unsubscribe linux-usb" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
