mail archive of the barebox mailing list
 help / color / mirror / Atom feed
* [RFC PATCH] usb: gadget: dfu: Rework dfu command to use usbgadget
@ 2021-08-30 14:48 Jules Maselbas
  2021-09-08 12:45 ` Anže Lešnik
  2021-09-10  8:46 ` Ahmad Fatoum
  0 siblings, 2 replies; 5+ messages in thread
From: Jules Maselbas @ 2021-08-30 14:48 UTC (permalink / raw)
  To: barebox; +Cc: Ahmad Fatoum, Jules Maselbas

The dfu command now uses the composite multi gadget to register the usb
functionality. This allows the removal of the usb composite driver from
dfu.c

As the dfu command is blocking the command slice must be released while
the dfu gadget is running in order to do operations on the file system.

The usb_dfu_register() function is replaced with usb_dfu_detached() for
the dfu command to return a different value depending on if it has been
interrupted with CTRL-C or if the gadget has been detached.

Signed-off-by: Jules Maselbas <jmaselbas@kalray.eu>
---
 commands/dfu.c           |  24 +++---
 drivers/usb/gadget/dfu.c | 163 +--------------------------------------
 include/usb/dfu.h        |   2 +-
 3 files changed, 15 insertions(+), 174 deletions(-)

diff --git a/commands/dfu.c b/commands/dfu.c
index 3132a7479d..0d8a703300 100644
--- a/commands/dfu.c
+++ b/commands/dfu.c
@@ -11,6 +11,7 @@
 #include <fs.h>
 #include <xfuncs.h>
 #include <usb/dfu.h>
+#include <usb/gadget-multi.h>
 #include <linux/err.h>
 
 /* dfu /dev/self0(bootloader)sr,/dev/nand0.root.bb(root)
@@ -20,28 +21,27 @@
  */
 static int do_dfu(int argc, char *argv[])
 {
-	struct f_dfu_opts opts;
 	char *argstr;
-	struct usb_dfu_dev *dfu_alts = NULL;
 	int ret;
 
 	if (argc != optind + 1)
 		return COMMAND_ERROR_USAGE;
 
 	argstr = argv[optind];
+	ret = usbgadget_register(true, argstr, false, NULL, false, false);
+	if (ret)
+		return ret;
 
-	opts.files = file_list_parse(argstr);
-	if (IS_ERR(opts.files)) {
-		ret = PTR_ERR(opts.files);
-		goto out;
+	command_slice_release();
+	while (!usb_dfu_detached()) {
+		if (ctrlc()) {
+			ret = -EINTR;
+			break;
+		}
 	}
+	command_slice_acquire();
 
-	ret = usb_dfu_register(&opts);
-
-	file_list_free(opts.files);
-out:
-
-	free(dfu_alts);
+	usb_multi_unregister();
 
 	return ret;
 }
diff --git a/drivers/usb/gadget/dfu.c b/drivers/usb/gadget/dfu.c
index fd0ec505dc..ba5fdd5b74 100644
--- a/drivers/usb/gadget/dfu.c
+++ b/drivers/usb/gadget/dfu.c
@@ -817,168 +817,9 @@ static void dfu_disable(struct usb_function *f)
 	dfu_abort(dfu);
 }
 
-#define STRING_MANUFACTURER_IDX		0
-#define STRING_PRODUCT_IDX		1
-#define STRING_DESCRIPTION_IDX		2
-
-static struct usb_string strings_dev[] = {
-	[STRING_MANUFACTURER_IDX].s = NULL,
-	[STRING_PRODUCT_IDX].s = NULL,
-	[STRING_DESCRIPTION_IDX].s = "USB Device Firmware Upgrade",
-	{  } /* end of list */
-};
-
-static struct usb_gadget_strings stringtab_dev = {
-	.language	= 0x0409,	/* en-us */
-	.strings	= strings_dev,
-};
-
-static struct usb_gadget_strings *dev_strings[] = {
-	&stringtab_dev,
-	NULL,
-};
-
-static void dfu_unbind_config(struct usb_configuration *c)
-{
-	free(dfu_string_defs);
-}
-
-static struct usb_configuration dfu_config_driver = {
-	.label			= "USB DFU",
-	.unbind			= dfu_unbind_config,
-	.bConfigurationValue	= 1,
-	.bmAttributes		= USB_CONFIG_ATT_SELFPOWER,
-};
-
-static struct usb_device_descriptor dfu_dev_descriptor = {
-	.bLength		= USB_DT_DEVICE_SIZE,
-	.bDescriptorType	= USB_DT_DEVICE,
-	.bcdUSB			= 0x0100,
-	.bDeviceClass		= 0x00,
-	.bDeviceSubClass	= 0x00,
-	.bDeviceProtocol	= 0x00,
-/*	.idVendor		= dynamic */
-/*	.idProduct		= dynamic */
-	.bcdDevice		= 0x0000,
-	.bNumConfigurations	= 0x01,
-};
-
-static struct usb_function_instance *fi_dfu;
-static struct usb_function *f_dfu;
-
-static int dfu_driver_bind(struct usb_composite_dev *cdev)
+int usb_dfu_detached(void)
 {
-	struct usb_gadget *gadget = cdev->gadget;
-	int status;
-
-	if (gadget->vendor_id && gadget->product_id) {
-		dfu_dev_descriptor.idVendor = cpu_to_le16(gadget->vendor_id);
-		dfu_dev_descriptor.idProduct = cpu_to_le16(gadget->product_id);
-	} else {
-		dfu_dev_descriptor.idVendor = cpu_to_le16(0x1d50); /* Openmoko, Inc */
-		dfu_dev_descriptor.idProduct = cpu_to_le16(0x60a2); /* barebox bootloader USB DFU Mode */
-	}
-
-	strings_dev[STRING_MANUFACTURER_IDX].s = gadget->manufacturer;
-	strings_dev[STRING_PRODUCT_IDX].s = gadget->productname;
-
-	status = usb_string_id(cdev);
-	if (status < 0)
-		goto fail;
-	strings_dev[STRING_MANUFACTURER_IDX].id = status;
-	dfu_dev_descriptor.iManufacturer = status;
-
-	status = usb_string_id(cdev);
-	if (status < 0)
-		goto fail;
-	strings_dev[STRING_PRODUCT_IDX].id = status;
-	dfu_dev_descriptor.iProduct = status;
-
-	/* config description */
-	status = usb_string_id(cdev);
-	if (status < 0)
-		goto fail;
-	strings_dev[STRING_DESCRIPTION_IDX].id = status;
-	dfu_config_driver.iConfiguration = status;
-
-	status = usb_add_config_only(cdev, &dfu_config_driver);
-	if (status < 0)
-		goto fail;
-
-	fi_dfu = usb_get_function_instance("dfu");
-	if (IS_ERR(fi_dfu)) {
-		status = PTR_ERR(fi_dfu);
-		goto fail;
-	}
-
-	f_dfu = usb_get_function(fi_dfu);
-	if (IS_ERR(f_dfu)) {
-		status = PTR_ERR(f_dfu);
-		goto fail;
-	}
-
-	status = usb_add_function(&dfu_config_driver, f_dfu);
-	if (status)
-		goto fail;
-
-	return 0;
-fail:
-	return status;
-}
-
-static int dfu_driver_unbind(struct usb_composite_dev *cdev)
-{
-	usb_put_function(f_dfu);
-	usb_put_function_instance(fi_dfu);
-
-	return 0;
-}
-
-static struct usb_composite_driver dfu_driver = {
-	.name		= "g_dfu",
-	.dev		= &dfu_dev_descriptor,
-	.strings	= dev_strings,
-	.max_speed	= USB_SPEED_HIGH,
-	.bind		= dfu_driver_bind,
-	.unbind		= dfu_driver_unbind,
-};
-
-int usb_dfu_register(struct f_dfu_opts *opts)
-{
-	int ret;
-
-	if (dfu_files)
-		return -EBUSY;
-
-	dfu_files = opts->files;
-
-	ret = usb_composite_probe(&dfu_driver);
-	if (ret)
-		goto out;
-
-	while (1) {
-		ret = usb_gadget_poll();
-		if (ret < 0)
-			goto out1;
-
-		if (dfudetach) {
-			ret = 0;
-			goto out1;
-		}
-
-		if (ctrlc()) {
-			ret = -EINTR;
-			goto out1;
-		}
-	}
-
-out1:
-	dfudetach = 0;
-	usb_composite_unregister(&dfu_driver);
-out:
-	dfu_files = NULL;
-
-	return ret;
+	return dfudetach;
 }
 
 static void dfu_free_func(struct usb_function *f)
diff --git a/include/usb/dfu.h b/include/usb/dfu.h
index 560a0318fe..81425f7c62 100644
--- a/include/usb/dfu.h
+++ b/include/usb/dfu.h
@@ -29,6 +29,6 @@ struct f_dfu_opts {
 	struct file_list *files;
 };
 
-int usb_dfu_register(struct f_dfu_opts *);
+int usb_dfu_detached(void);
 
 #endif /* _USB_DFU_H */
-- 
2.17.1


_______________________________________________
barebox mailing list
barebox@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/barebox


^ permalink raw reply	[flat|nested] 5+ messages in thread

* Re: [RFC PATCH] usb: gadget: dfu: Rework dfu command to use usbgadget
  2021-08-30 14:48 [RFC PATCH] usb: gadget: dfu: Rework dfu command to use usbgadget Jules Maselbas
@ 2021-09-08 12:45 ` Anže Lešnik
  2021-09-10  8:44   ` Ahmad Fatoum
  2021-09-10  8:46 ` Ahmad Fatoum
  1 sibling, 1 reply; 5+ messages in thread
From: Anže Lešnik @ 2021-09-08 12:45 UTC (permalink / raw)
  To: barebox

We have tested this patch on a PHYTEC phyCORE-i.MX6 ULL SOM running 
barebox 2021.04 (using the imx-usb driver) and it seems to resolve the 
issue. DFU transfer now works as intended.

If one receives timeouts on the host computer, try setting 
usbcore.autosuspend=-1.


Anže


_______________________________________________
barebox mailing list
barebox@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/barebox

^ permalink raw reply	[flat|nested] 5+ messages in thread

* Re: [RFC PATCH] usb: gadget: dfu: Rework dfu command to use usbgadget
  2021-09-08 12:45 ` Anže Lešnik
@ 2021-09-10  8:44   ` Ahmad Fatoum
  2021-09-10 18:27     ` Anže Lešnik
  0 siblings, 1 reply; 5+ messages in thread
From: Ahmad Fatoum @ 2021-09-10  8:44 UTC (permalink / raw)
  To: 20210830144835.27458-1-jmaselbas, barebox

Hi,

On 08.09.21 14:45, Anže Lešnik wrote:
> We have tested this patch on a PHYTEC phyCORE-i.MX6 ULL SOM running barebox 2021.04 (using the imx-usb driver) and it seems to resolve the issue. DFU transfer now works as intended.

Great. So it's ok for Sascha to add

Tested-by: Anže Lešnik <anze.lesnik@norik.com>

to the commit log?

> If one receives timeouts on the host computer, try setting usbcore.autosuspend=-1.

Did you need this kernel parameter with the old dfu command as well?
If only new dfu command makes this parameter necessary, we might need to look
why the timing behavior changed.

Cheers,
Ahmad

> 
> 
> Anže
> 
> 
> _______________________________________________
> barebox mailing list
> barebox@lists.infradead.org
> http://lists.infradead.org/mailman/listinfo/barebox


-- 
Pengutronix e.K.                           |                             |
Steuerwalder Str. 21                       | http://www.pengutronix.de/  |
31137 Hildesheim, Germany                  | Phone: +49-5121-206917-0    |
Amtsgericht Hildesheim, HRA 2686           | Fax:   +49-5121-206917-5555 |

_______________________________________________
barebox mailing list
barebox@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/barebox

^ permalink raw reply	[flat|nested] 5+ messages in thread

* Re: [RFC PATCH] usb: gadget: dfu: Rework dfu command to use usbgadget
  2021-08-30 14:48 [RFC PATCH] usb: gadget: dfu: Rework dfu command to use usbgadget Jules Maselbas
  2021-09-08 12:45 ` Anže Lešnik
@ 2021-09-10  8:46 ` Ahmad Fatoum
  1 sibling, 0 replies; 5+ messages in thread
From: Ahmad Fatoum @ 2021-09-10  8:46 UTC (permalink / raw)
  To: Jules Maselbas, barebox

Hi Jules,

On 30.08.21 16:48, Jules Maselbas wrote:
> The dfu command now uses the composite multi gadget to register the usb
> functionality. This allows the removal of the usb composite driver from
> dfu.c
> 
> As the dfu command is blocking the command slice must be released while
> the dfu gadget is running in order to do operations on the file system.
> 
> The usb_dfu_register() function is replaced with usb_dfu_detached() for
> the dfu command to return a different value depending on if it has been
> interrupted with CTRL-C or if the gadget has been detached.
> 
> Signed-off-by: Jules Maselbas <jmaselbas@kalray.eu>
> ---
>  commands/dfu.c           |  24 +++---
>  drivers/usb/gadget/dfu.c | 163 +--------------------------------------
>  include/usb/dfu.h        |   2 +-
>  3 files changed, 15 insertions(+), 174 deletions(-)
> 
> diff --git a/commands/dfu.c b/commands/dfu.c
> index 3132a7479d..0d8a703300 100644
> --- a/commands/dfu.c
> +++ b/commands/dfu.c
> @@ -11,6 +11,7 @@
>  #include <fs.h>
>  #include <xfuncs.h>
>  #include <usb/dfu.h>
> +#include <usb/gadget-multi.h>
>  #include <linux/err.h>
>  
>  /* dfu /dev/self0(bootloader)sr,/dev/nand0.root.bb(root)
> @@ -20,28 +21,27 @@
>   */
>  static int do_dfu(int argc, char *argv[])
>  {
> -	struct f_dfu_opts opts;
>  	char *argstr;
> -	struct usb_dfu_dev *dfu_alts = NULL;
>  	int ret;
>  
>  	if (argc != optind + 1)
>  		return COMMAND_ERROR_USAGE;
>  
>  	argstr = argv[optind];
> +	ret = usbgadget_register(true, argstr, false, NULL, false, false);
> +	if (ret)
> +		return ret;

next branch has a different prototype for usbgadget_register now.
Could you adapt it for v2?


Cheers,
Ahmad


-- 
Pengutronix e.K.                           |                             |
Steuerwalder Str. 21                       | http://www.pengutronix.de/  |
31137 Hildesheim, Germany                  | Phone: +49-5121-206917-0    |
Amtsgericht Hildesheim, HRA 2686           | Fax:   +49-5121-206917-5555 |

_______________________________________________
barebox mailing list
barebox@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/barebox


^ permalink raw reply	[flat|nested] 5+ messages in thread

* Re: [RFC PATCH] usb: gadget: dfu: Rework dfu command to use usbgadget
  2021-09-10  8:44   ` Ahmad Fatoum
@ 2021-09-10 18:27     ` Anže Lešnik
  0 siblings, 0 replies; 5+ messages in thread
From: Anže Lešnik @ 2021-09-10 18:27 UTC (permalink / raw)
  To: barebox

On 10. 09. 21 10:44, Ahmad Fatoum wrote:

> Hi,
>
> On 08.09.21 14:45, Anže Lešnik wrote:
>> We have tested this patch on a PHYTEC phyCORE-i.MX6 ULL SOM running barebox 2021.04 (using the imx-usb driver) and it seems to resolve the issue. DFU transfer now works as intended.
> Great. So it's ok for Sascha to add
>
> Tested-by: Anže Lešnik <anze.lesnik@norik.com>
>
> to the commit log?

Yes. We have also tested the V2 of this patch, submitted earlier today, 
and it functions correctly.

>> If one receives timeouts on the host computer, try setting usbcore.autosuspend=-1.
> Did you need this kernel parameter with the old dfu command as well?
> If only new dfu command makes this parameter necessary, we might need to look
> why the timing behavior changed.

Yes, it was also required before this patch. We have tested it with a 
couple of hosts and this issue was only present on one machine.

> Cheers,
> Ahmad
>
>>
>> Anže
>>
>>
>> _______________________________________________
>> barebox mailing list
>> barebox@lists.infradead.org
>> http://lists.infradead.org/mailman/listinfo/barebox
>

_______________________________________________
barebox mailing list
barebox@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/barebox

^ permalink raw reply	[flat|nested] 5+ messages in thread

end of thread, other threads:[~2021-09-10 18:29 UTC | newest]

Thread overview: 5+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2021-08-30 14:48 [RFC PATCH] usb: gadget: dfu: Rework dfu command to use usbgadget Jules Maselbas
2021-09-08 12:45 ` Anže Lešnik
2021-09-10  8:44   ` Ahmad Fatoum
2021-09-10 18:27     ` Anže Lešnik
2021-09-10  8:46 ` Ahmad Fatoum

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox