Sebastian Andrzej Siewior <bige...@linutronix.de> writes:
> This patch moves the module options idVendor, idProduct and bcdDevice
> from composite.c into each gadgets. This ensures compatibility with
> current gadgets and removes the global variable which brings me step
> closer towards composite.c in libcomposite
>
> Signed-off-by: Sebastian Andrzej Siewior <bige...@linutronix.de>

In general:

Acked-by: Michal Nazarewicz <min...@mina86.com>

but two comments below:

> diff --git a/drivers/usb/gadget/audio.c b/drivers/usb/gadget/audio.c
> index 689d142..a9c687a 100644
> --- a/drivers/usb/gadget/audio.c
> +++ b/drivers/usb/gadget/audio.c
> @@ -13,6 +13,7 @@
>  
>  #include <linux/kernel.h>
>  #include <linux/utsname.h>
> +#include <linux/usb/composite.h>

Why is this needed in this patch?  I think it belongs to one of the
later patches.

>  
>  #include "gadget_chips.h"
>  #define DRIVER_DESC          "Linux USB Audio Gadget"
> @@ -28,6 +29,7 @@
>   * a "gcc --combine ... part1.c part2.c part3.c ... " build would.
>   */
>  #include "composite.c"
> +USB_GADGET_COMPOSITE_OPTIONS();
>  
>  /* string IDs are assigned dynamically */
>  
> @@ -180,6 +182,7 @@ static int __init audio_bind(struct usb_composite_dev 
> *cdev)
>       status = usb_add_config(cdev, &audio_config_driver, audio_do_config);
>       if (status < 0)
>               goto fail;
> +     USB_GADGET_COMPOSITE_OVERWRITE_OPTIONS(device_desc);
>  
>       INFO(cdev, "%s, version: %s\n", DRIVER_DESC, DRIVER_VERSION);
>       return 0;

> diff --git a/drivers/usb/gadget/printer.c b/drivers/usb/gadget/printer.c
> index 3d3d20d..39279e2 100644
> --- a/drivers/usb/gadget/printer.c
> +++ b/drivers/usb/gadget/printer.c
> @@ -54,6 +54,7 @@
>  #include "composite.c"
>  
>  /*-------------------------------------------------------------------------*/
> +USB_GADGET_COMPOSITE_OPTIONS();
>  
>  #define DRIVER_DESC          "Printer Gadget"
>  #define DRIVER_VERSION               "2007 OCT 06"
> @@ -1263,7 +1264,13 @@ static int printer_unbind(struct usb_composite_dev 
> *cdev)
>  
>  static int __init printer_bind(struct usb_composite_dev *cdev)
>  {
> -     return usb_add_config(cdev, &printer_cfg_driver, printer_bind_config);
> +     int ret;
> +
> +     ret = usb_add_config(cdev, &printer_cfg_driver, printer_bind_config);
> +     if (ret)
> +             return ret;
> +     USB_GADGET_COMPOSITE_OVERWRITE_OPTIONS(device_desc);
> +     return 0;

Alternatively you could just add invocation of the
USB_GADGET_COMPOSITE_OVERWRITE_OPTIONS(device_desc) macro before the
usb_add_config() is added, which would add only one additional line,
like so:

 {
+       USB_GADGET_COMPOSITE_OVERWRITE_OPTIONS(device_desc);
        return usb_add_config(cdev, &printer_cfg_driver, printer_bind_config);
 }

-- 
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--

Attachment: pgpO6ZXz0Lc3A.pgp
Description: PGP signature

Reply via email to