Sebastian Andrzej Siewior <bigeasy@xxxxxxxxxxxxx> 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 <bigeasy@xxxxxxxxxxxxx>
In general:
Acked-by: Michal Nazarewicz <mina86@xxxxxxxxxx>
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: mpn@xxxxxxxxxx>--------------ooO--(_)--Ooo--Attachment:
pgpNrP5P_I6Gl.pgp
Description: PGP signature
