On Thu, Sep 06 2012, Sebastian Andrzej Siewior wrote: > This patch removes the global variable composite in composite.c. > The private data which was saved there is now passed via an additional > argument to the bind() function in struct usb_gadget_driver. > > Only the "old-style" UDC drivers have to be touched here, new style are > doing it right because this change is made in udc-core. > > Signed-off-by: Sebastian Andrzej Siewior <bige...@linutronix.de>
Acked-by: Michal Nazarewicz <min...@mina86.com>
Two comments inline:
> diff --git a/drivers/usb/gadget/composite.c b/drivers/usb/gadget/composite.c
> index 2a345f2..5615675 100644
> --- a/drivers/usb/gadget/composite.c
> +++ b/drivers/usb/gadget/composite.c
> @@ -31,8 +31,6 @@
> /* big enough to hold our biggest descriptor */
> #define USB_BUFSIZ 1024
>
> -static struct usb_composite_driver *composite;
> -
> /* Some systems will need runtime overrides for the product identifiers
> * published in the device descriptor, either numbers or strings or both.
> * String parameters are in UTF-8 (superset of ASCII's 7 bit characters).
> @@ -889,6 +887,7 @@ static int lookup_string(
> static int get_string(struct usb_composite_dev *cdev,
> void *buf, u16 language, int id)
> {
> + struct usb_composite_driver *cdriver = cdev->driver;
I still think you should use “composite” for the variable name as this
would get rid of a lot of the following hunks:
> struct usb_configuration *c;
> struct usb_function *f;
> int len;
> @@ -907,7 +906,7 @@ static int get_string(struct usb_composite_dev *cdev,
> memset(s, 0, 256);
> s->bDescriptorType = USB_DT_STRING;
>
> - sp = composite->strings;
> + sp = cdriver->strings;
> if (sp)
> collect_langs(sp, s->wData);
>
> @@ -936,12 +935,12 @@ static int get_string(struct usb_composite_dev *cdev,
> * check if the string has not been overridden.
> */
> if (cdev->manufacturer_override == id)
> - str = iManufacturer ?: composite->iManufacturer ?:
> + str = iManufacturer ?: cdriver->iManufacturer ?:
> composite_manufacturer;
> else if (cdev->product_override == id)
> - str = iProduct ?: composite->iProduct;
> + str = iProduct ?: cdriver->iProduct;
> else if (cdev->serial_override == id)
> - str = iSerialNumber ?: composite->iSerialNumber;
> + str = iSerialNumber ?: cdriver->iSerialNumber;
> else
> str = NULL;
> if (str) {
> @@ -956,8 +955,8 @@ static int get_string(struct usb_composite_dev *cdev,
> * table we're told about. These lookups are infrequent;
> * simpler-is-better here.
> */
> - if (composite->strings) {
> - len = lookup_string(composite->strings, buf, language, id);
> + if (cdriver->strings) {
> + len = lookup_string(cdriver->strings, buf, language, id);
> if (len > 0)
> return len;
> }
> @@ -1359,8 +1358,8 @@ static void composite_disconnect(struct usb_gadget
> *gadget)
> spin_lock_irqsave(&cdev->lock, flags);
> if (cdev->config)
> reset_config(cdev);
> - if (composite->disconnect)
> - composite->disconnect(cdev);
> + if (cdev->driver->disconnect)
> + cdev->driver->disconnect(cdev);
> spin_unlock_irqrestore(&cdev->lock, flags);
> }
>
> @@ -1396,8 +1395,8 @@ composite_unbind(struct usb_gadget *gadget)
> struct usb_configuration, list);
> remove_config(cdev, c);
> }
> - if (composite->unbind)
> - composite->unbind(cdev);
> + if (cdev->driver->unbind)
> + cdev->driver->unbind(cdev);
>
> if (cdev->req) {
> kfree(cdev->req->buf);
> @@ -1406,7 +1405,6 @@ composite_unbind(struct usb_gadget *gadget)
> device_remove_file(&gadget->dev, &dev_attr_suspended);
> kfree(cdev);
> set_gadget_data(gadget, NULL);
> - composite = NULL;
> }
>
> static u8 override_id(struct usb_composite_dev *cdev, u8 *desc)
> @@ -1422,9 +1420,16 @@ static u8 override_id(struct usb_composite_dev *cdev,
> u8 *desc)
> return *desc;
> }
>
> -static int composite_bind(struct usb_gadget *gadget)
> +static struct usb_composite_driver *to_cdriver(struct usb_gadget_driver
> *gdrv)
> +{
> + return container_of(gdrv, struct usb_composite_driver, gadget_driver);
> +}
> +
> +static int composite_bind(struct usb_gadget *gadget,
> + struct usb_gadget_driver *gdriver)
> {
> struct usb_composite_dev *cdev;
> + struct usb_composite_driver *cdriver = to_cdriver(gdriver);
Same here.
> int status = -ENOMEM;
>
> cdev = kzalloc(sizeof *cdev, GFP_KERNEL);
> @@ -1447,7 +1452,7 @@ static int composite_bind(struct usb_gadget *gadget)
> gadget->ep0->driver_data = cdev;
>
> cdev->bufsiz = USB_BUFSIZ;
> - cdev->driver = composite;
> + cdev->driver = cdriver;
>
> /*
> * As per USB compliance update, a device that is actively drawing
> @@ -1467,11 +1472,11 @@ static int composite_bind(struct usb_gadget *gadget)
> * serial number), register function drivers, potentially update
> * power state and consumption, etc
> */
> - status = composite->bind(cdev);
> + status = cdriver->bind(cdev);
> if (status < 0)
> goto fail;
>
> - cdev->desc = *composite->dev;
> + cdev->desc = *cdriver->dev;
>
> /* standardized runtime overrides for device ID data */
> if (idVendor)
> @@ -1489,7 +1494,7 @@ static int composite_bind(struct usb_gadget *gadget)
>
> /* string overrides */
> if (iManufacturer || !cdev->desc.iManufacturer) {
> - if (!iManufacturer && !composite->iManufacturer &&
> + if (!iManufacturer && !cdriver->iManufacturer &&
> !*composite_manufacturer)
> snprintf(composite_manufacturer,
> sizeof composite_manufacturer,
> @@ -1502,17 +1507,17 @@ static int composite_bind(struct usb_gadget *gadget)
> override_id(cdev, &cdev->desc.iManufacturer);
> }
>
> - if (iProduct || (!cdev->desc.iProduct && composite->iProduct))
> + if (iProduct || (!cdev->desc.iProduct && cdriver->iProduct))
> cdev->product_override =
> override_id(cdev, &cdev->desc.iProduct);
>
> if (iSerialNumber ||
> - (!cdev->desc.iSerialNumber && composite->iSerialNumber))
> + (!cdev->desc.iSerialNumber && cdriver->iSerialNumber))
> cdev->serial_override =
> override_id(cdev, &cdev->desc.iSerialNumber);
>
> /* has userspace failed to provide a serial number? */
> - if (composite->needs_serial && !cdev->desc.iSerialNumber)
> + if (cdriver->needs_serial && !cdev->desc.iSerialNumber)
> WARNING(cdev, "userspace failed to provide iSerialNumber\n");
>
> /* finish up */
> @@ -1520,7 +1525,7 @@ static int composite_bind(struct usb_gadget *gadget)
> if (status)
> goto fail;
>
> - INFO(cdev, "%s ready\n", composite->name);
> + INFO(cdev, "%s ready\n", cdriver->name);
> return 0;
>
> fail:
> @@ -1546,8 +1551,8 @@ composite_suspend(struct usb_gadget *gadget)
> f->suspend(f);
> }
> }
> - if (composite->suspend)
> - composite->suspend(cdev);
> + if (cdev->driver->suspend)
> + cdev->driver->suspend(cdev);
>
> cdev->suspended = 1;
>
> @@ -1565,8 +1570,8 @@ composite_resume(struct usb_gadget *gadget)
> * suspend/resume callbacks?
> */
> DBG(cdev, "resume\n");
> - if (composite->resume)
> - composite->resume(cdev);
> + if (cdev->driver->resume)
> + cdev->driver->resume(cdev);
> if (cdev->config) {
> list_for_each_entry(f, &cdev->config->functions, list) {
> if (f->resume)
--
Best regards, _ _
.o. | Liege of Serenely Enlightened Majesty of o' \,=./ `o
..o | Computer Science, Michał “mina86” Nazarewicz (o o)
ooo +----<email/xmpp: m...@google.com>--------------ooO--(_)--Ooo--
pgphYkaPmHnD5.pgp
Description: PGP signature
