mail archive of the barebox mailing list
 help / color / mirror / Atom feed
* [PATCH] usbgadget: autostart: add DFU support
@ 2018-10-25 22:37 Ladislav Michl
  2018-10-26 10:19 ` Sascha Hauer
  0 siblings, 1 reply; 5+ messages in thread
From: Ladislav Michl @ 2018-10-25 22:37 UTC (permalink / raw)
  To: barebox

Use global variable dfu_function to autostart DFU. As similar code
is used to start multifunction gadget using command, move common
code to common/usbgadget.c and consolidate it.

Signed-off-by: Ladislav Michl <ladis@linux-mips.org>
---
 This is a draft, comments welcome. Getting 
 Cannot set parameter otg.mode: No such device
 but that's same as before this patch. Documentation is probably
 also worth updating...

 commands/Kconfig                              |  1 -
 commands/usbgadget.c                          | 66 +++-----------
 common/Kconfig                                |  7 ++
 common/Makefile                               |  1 +
 .../gadget/autostart.c => common/usbgadget.c  | 85 ++++++++++++++-----
 drivers/usb/gadget/Kconfig                    |  8 +-
 drivers/usb/gadget/Makefile                   |  1 -
 7 files changed, 89 insertions(+), 80 deletions(-)
 rename drivers/usb/gadget/autostart.c => common/usbgadget.c (51%)

diff --git a/commands/Kconfig b/commands/Kconfig
index 675bd1ca7..1de4b9d60 100644
--- a/commands/Kconfig
+++ b/commands/Kconfig
@@ -1910,7 +1910,6 @@ config CMD_USB
 config CMD_USBGADGET
 	bool
 	depends on USB_GADGET
-	select FILE_LIST
 	prompt "usbgadget"
 
 config CMD_WD
diff --git a/commands/usbgadget.c b/commands/usbgadget.c
index a1744cbe1..416b2acfc 100644
--- a/commands/usbgadget.c
+++ b/commands/usbgadget.c
@@ -30,14 +30,18 @@
 #include <usb/dfu.h>
 #include <usb/gadget-multi.h>
 
+int usbgadget_register(bool dfu_set, const char *dfu_opts,
+		       bool fastboot_set, const char *fastboot_opts,
+		       bool create_serial, bool is_acm, bool export_bbu);
+
 static int do_usbgadget(int argc, char *argv[])
 {
-	int opt, ret;
-	int acm = 1, create_serial = 0, fastboot_set = 0, fastboot_export_bbu = 0;
+	int opt;
+	bool acm = true, create_serial = false, dfu_set = false;
+	bool fastboot_set = false, fastboot_export_bbu = false;
 	const char *fastboot_opts = NULL, *dfu_opts = NULL;
-	struct f_multi_opts *opts;
 
-	while ((opt = getopt(argc, argv, "asdA::D:b")) > 0) {
+	while ((opt = getopt(argc, argv, "asdA::D::b")) > 0) {
 		switch (opt) {
 		case 'a':
 			acm = 1;
@@ -49,6 +53,7 @@ static int do_usbgadget(int argc, char *argv[])
 			break;
 		case 'D':
 			dfu_opts = optarg;
+			dfu_set = 1;
 			break;
 		case 'A':
 			fastboot_opts = optarg;
@@ -65,54 +70,8 @@ static int do_usbgadget(int argc, char *argv[])
 		}
 	}
 
-	if (fastboot_set && !fastboot_opts)
-		fastboot_opts = getenv("global.usbgadget.fastboot_function");
-
-	if (!dfu_opts && !fastboot_opts && !create_serial)
-		return COMMAND_ERROR_USAGE;
-
-	/*
-	 * Creating a gadget with both DFU and Fastboot doesn't work.
-	 * Both client tools seem to assume that the device only has
-	 * a single configuration
-	 */
-	if (fastboot_opts && dfu_opts) {
-		printf("Only one of Fastboot and DFU allowed\n");
-		return -EINVAL;
-	}
-
-	opts = xzalloc(sizeof(*opts));
-	opts->release = usb_multi_opts_release;
-
-	if (fastboot_opts) {
-		opts->fastboot_opts.files = file_list_parse(fastboot_opts);
-		if (IS_ERR(opts->fastboot_opts.files))
-			goto err_parse;
-		opts->fastboot_opts.export_bbu = fastboot_export_bbu;
-	}
-
-	if (dfu_opts) {
-		opts->dfu_opts.files = file_list_parse(dfu_opts);
-		if (IS_ERR(opts->dfu_opts.files))
-			goto err_parse;
-	}
-
-	if (create_serial) {
-		opts->create_acm = acm;
-	}
-
-	ret = usb_multi_register(opts);
-	if (ret)
-		usb_multi_opts_release(opts);
-
-	return ret;
-
-err_parse:
-	printf("Cannot parse file list \"%s\": %s\n", fastboot_opts, strerrorp(opts->fastboot_opts.files));
-
-	free(opts);
-
-	return 1;
+	return usbgadget_register(dfu_set, dfu_opts, fastboot_set, fastboot_opts,
+				  create_serial, acm, fastboot_export_bbu);
 }
 
 BAREBOX_CMD_HELP_START(usbgadget)
@@ -124,7 +83,8 @@ BAREBOX_CMD_HELP_OPT ("-s\t", "Create Generic Serial function")
 BAREBOX_CMD_HELP_OPT ("-A <desc>", "Create Android Fastboot function. If 'desc' is not provided, "
 				   "try to use 'global.usbgadget.fastboot_function' variable.")
 BAREBOX_CMD_HELP_OPT ("-b\t", "include registered barebox update handlers (fastboot specific)")
-BAREBOX_CMD_HELP_OPT ("-D <desc>", "Create DFU function")
+BAREBOX_CMD_HELP_OPT ("-D <desc>", "Create DFU function. If 'desc' is not provided, "
+				   "try to use 'global.usbgadget.dfu_function' variable.")
 BAREBOX_CMD_HELP_OPT ("-d\t", "Disable the currently running gadget")
 BAREBOX_CMD_HELP_END
 
diff --git a/common/Kconfig b/common/Kconfig
index eddd99ea3..2ad92158c 100644
--- a/common/Kconfig
+++ b/common/Kconfig
@@ -115,6 +115,13 @@ config UBIFORMAT
 	depends on MTD_UBI
 	default y
 
+config USBGADGET_START
+	bool
+	depends on CMD_USBGADGET || USB_GADGET_AUTOSTART
+	select ENVIRONMENT_VARIABLES
+	select FILE_LIST
+	default y
+
 config BOOT
 	bool
 
diff --git a/common/Makefile b/common/Makefile
index 13920cc5a..2b0f4cc98 100644
--- a/common/Makefile
+++ b/common/Makefile
@@ -64,6 +64,7 @@ obj-$(CONFIG_UBIFORMAT)		+= ubiformat.o
 obj-$(CONFIG_BAREBOX_UPDATE_IMX_NAND_FCB) += imx-bbu-nand-fcb.o
 obj-$(CONFIG_BOOT)		+= boot.o
 obj-$(CONFIG_SERIAL_DEV_BUS)	+= serdev.o
+obj-$(CONFIG_USBGADGET_START)	+= usbgadget.o
 
 ifdef CONFIG_PASSWORD
 
diff --git a/drivers/usb/gadget/autostart.c b/common/usbgadget.c
similarity index 51%
rename from drivers/usb/gadget/autostart.c
rename to common/usbgadget.c
index f640a9667..7d72ad489 100644
--- a/drivers/usb/gadget/autostart.c
+++ b/common/usbgadget.c
@@ -11,7 +11,7 @@
  * GNU General Public License for more details.
  *
  */
-#define pr_fmt(fmt) "usbgadget autostart: " fmt
+#define pr_fmt(fmt) "usbgadget: " fmt
 
 #include <common.h>
 #include <command.h>
@@ -29,35 +29,65 @@
 
 static int autostart;
 static int acm;
+static char *dfu_function;
 static char *fastboot_function;
 static int fastboot_bbu;
 
-static int usbgadget_autostart(void)
+static struct file_list *parse(const char *files)
+{
+	struct file_list *list = file_list_parse(files);
+	if (IS_ERR(list)) {
+		pr_err("Parsing file list \"%s\" failed: %s\n", files,
+		       strerrorp(list));
+		return NULL;
+	}
+	return list;
+}
+
+int usbgadget_register(bool dfu_set, const char *dfu_opts,
+		       bool fastboot_set, const char *fastboot_opts,
+		       bool create_serial, bool is_acm, bool export_bbu)
 {
-	struct f_multi_opts *opts;
 	int ret;
+	struct f_multi_opts *opts;
 
-	if (!autostart)
-		return 0;
+	if (dfu_set && !dfu_opts && dfu_function && *dfu_function)
+		dfu_opts = dfu_function;
+
+	if (fastboot_set && !fastboot_opts &&
+	    fastboot_function && *fastboot_function)
+		fastboot_opts = fastboot_function;
+
+	if (!dfu_opts && !fastboot_opts && !create_serial)
+		return COMMAND_ERROR_USAGE;
+
+	/*
+	 * Creating a gadget with both DFU and Fastboot doesn't work.
+	 * Both client tools seem to assume that the device only has
+	 * a single configuration
+	 */
+	if (fastboot_opts && dfu_opts) {
+		pr_err("Only one of Fastboot and DFU allowed\n");
+		return -EINVAL;
+	}
 
 	opts = xzalloc(sizeof(*opts));
 	opts->release = usb_multi_opts_release;
 
-	if (fastboot_function) {
-		opts->fastboot_opts.files = file_list_parse(fastboot_function);
-		if (IS_ERR(opts->fastboot_opts.files)) {
-			pr_err("Parsing file list \"%s\" failed: %s\n", fastboot_function,
-			       strerrorp(opts->fastboot_opts.files));
-			opts->fastboot_opts.files = NULL;
-		}
-
-		opts->fastboot_opts.export_bbu = fastboot_bbu;
+	if (fastboot_opts) {
+		opts->fastboot_opts.files = parse(fastboot_opts);
+		opts->fastboot_opts.export_bbu = export_bbu;
 	}
 
-	opts->create_acm = acm;
+	if (dfu_opts)
+		opts->dfu_opts.files = parse(dfu_opts);
+
+	opts->create_acm = is_acm;
 
-	if (!opts->fastboot_opts.files && !opts->create_acm) {
+	if (!opts->dfu_opts.files && !opts->fastboot_opts.files &&
+	    !opts->create_acm) {
 		pr_warn("No functions to register\n");
+		free(opts);
 		return 0;
 	}
 
@@ -69,16 +99,28 @@ static int usbgadget_autostart(void)
 
 	return ret;
 }
+
+static int usbgadget_autostart(void)
+{
+	if (!IS_ENABLED(CONFIG_USB_GADGET_AUTOSTART) || !autostart)
+		return 0;
+
+	return usbgadget_register(true, NULL, true, NULL, acm, acm,
+				  fastboot_bbu);
+}
 postenvironment_initcall(usbgadget_autostart);
 
 static int usbgadget_globalvars_init(void)
 {
-
-	globalvar_add_simple_bool("usbgadget.autostart", &autostart);
-	globalvar_add_simple_bool("usbgadget.acm", &acm);
+	if (IS_ENABLED(CONFIG_USB_GADGET_AUTOSTART)) {
+		globalvar_add_simple_bool("usbgadget.autostart", &autostart);
+		globalvar_add_simple_bool("usbgadget.acm", &acm);
+		globalvar_add_simple_bool("usbgadget.fastboot_bbu",
+					  &fastboot_bbu);
+	}
+	globalvar_add_simple_string("usbgadget.dfu_function", &dfu_function);
 	globalvar_add_simple_string("usbgadget.fastboot_function",
 				    &fastboot_function);
-	globalvar_add_simple_bool("usbgadget.fastboot_bbu", &fastboot_bbu);
 
 	return 0;
 }
@@ -90,6 +132,9 @@ BAREBOX_MAGICVAR_NAMED(global_usbgadget_autostart,
 BAREBOX_MAGICVAR_NAMED(global_usbgadget_acm,
 		       global.usbgadget.acm,
 		       "usbgadget: Create CDC ACM function");
+BAREBOX_MAGICVAR_NAMED(global_usbgadget_dfu_function,
+		       global.usbgadget.dfu_function,
+		       "usbgadget: Create DFU function");
 BAREBOX_MAGICVAR_NAMED(global_usbgadget_fastboot_function,
 		       global.usbgadget.fastboot_function,
 		       "usbgadget: Create Android Fastboot function");
diff --git a/drivers/usb/gadget/Kconfig b/drivers/usb/gadget/Kconfig
index b0408e3bb..ca1bfc1b4 100644
--- a/drivers/usb/gadget/Kconfig
+++ b/drivers/usb/gadget/Kconfig
@@ -33,13 +33,11 @@ config USB_GADGET_DRIVER_PXA27X
 config USB_GADGET_AUTOSTART
 	bool
 	default y
-	select ENVIRONMENT_VARIABLES
-	select FILE_LIST
 	prompt "Automatically start usbgadget on boot"
 	help
-	  Enabling this option allows to automatically start a fastboot
-	  gadget during boot. This behaviour is controlled with the
-	  global.usbgadget.fastboot_function variable.
+	  Enabling this option allows to automatically start a dfu or
+	  fastboot gadget during boot. This behaviour is controlled with
+	  the global.usbgadget.{dfu,fastboot}_function variable.
 
 comment "USB Gadget drivers"
 
diff --git a/drivers/usb/gadget/Makefile b/drivers/usb/gadget/Makefile
index e74cf0266..9ef594575 100644
--- a/drivers/usb/gadget/Makefile
+++ b/drivers/usb/gadget/Makefile
@@ -1,6 +1,5 @@
 
 obj-$(CONFIG_USB_GADGET) += composite.o config.o usbstring.o epautoconf.o udc-core.o functions.o config.o multi.o
-obj-$(CONFIG_USB_GADGET_AUTOSTART) += autostart.o
 obj-$(CONFIG_USB_GADGET_SERIAL) += u_serial.o serial.o f_serial.o f_acm.o
 obj-$(CONFIG_USB_GADGET_DFU) += dfu.o
 obj-$(CONFIG_USB_GADGET_FASTBOOT) += f_fastboot.o
-- 
2.19.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: [PATCH] usbgadget: autostart: add DFU support
  2018-10-25 22:37 [PATCH] usbgadget: autostart: add DFU support Ladislav Michl
@ 2018-10-26 10:19 ` Sascha Hauer
  2018-10-26 11:10   ` Ladislav Michl
  0 siblings, 1 reply; 5+ messages in thread
From: Sascha Hauer @ 2018-10-26 10:19 UTC (permalink / raw)
  To: Ladislav Michl; +Cc: barebox

Hi Ladis,

On Fri, Oct 26, 2018 at 12:37:19AM +0200, Ladislav Michl wrote:
> Use global variable dfu_function to autostart DFU. As similar code
> is used to start multifunction gadget using command, move common
> code to common/usbgadget.c and consolidate it.
> 
> Signed-off-by: Ladislav Michl <ladis@linux-mips.org>
> ---
>  This is a draft, comments welcome. Getting 
>  Cannot set parameter otg.mode: No such device

I just sent a fix for this.

>  but that's same as before this patch. Documentation is probably
>  also worth updating...

Indeed.

> diff --git a/commands/usbgadget.c b/commands/usbgadget.c
> index a1744cbe1..416b2acfc 100644
> --- a/commands/usbgadget.c
> +++ b/commands/usbgadget.c
> @@ -30,14 +30,18 @@
>  #include <usb/dfu.h>
>  #include <usb/gadget-multi.h>
>  
> +int usbgadget_register(bool dfu_set, const char *dfu_opts,
> +		       bool fastboot_set, const char *fastboot_opts,
> +		       bool create_serial, bool is_acm, bool export_bbu);
> +

This should get a prototype somewhere in include/, but that's probably
due to the draft status of this patch.

Otherwise looks fine to me.

Sascha

-- 
Pengutronix e.K.                           |                             |
Industrial Linux Solutions                 | http://www.pengutronix.de/  |
Peiner Str. 6-8, 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: [PATCH] usbgadget: autostart: add DFU support
  2018-10-26 10:19 ` Sascha Hauer
@ 2018-10-26 11:10   ` Ladislav Michl
  2018-10-26 11:31     ` Sascha Hauer
  0 siblings, 1 reply; 5+ messages in thread
From: Ladislav Michl @ 2018-10-26 11:10 UTC (permalink / raw)
  To: Sascha Hauer; +Cc: barebox

On Fri, Oct 26, 2018 at 12:19:11PM +0200, Sascha Hauer wrote:
> Hi Ladis,
> 
> On Fri, Oct 26, 2018 at 12:37:19AM +0200, Ladislav Michl wrote:
> > Use global variable dfu_function to autostart DFU. As similar code
> > is used to start multifunction gadget using command, move common
> > code to common/usbgadget.c and consolidate it.
> > 
> > Signed-off-by: Ladislav Michl <ladis@linux-mips.org>
> > ---
> >  This is a draft, comments welcome. Getting 
> >  Cannot set parameter otg.mode: No such device
> 
> I just sent a fix for this.

Thanks, tested, added.

> >  but that's same as before this patch. Documentation is probably
> >  also worth updating...
> 
> Indeed.

Will add prepatch to document bbu then.

> > diff --git a/commands/usbgadget.c b/commands/usbgadget.c
> > index a1744cbe1..416b2acfc 100644
> > --- a/commands/usbgadget.c
> > +++ b/commands/usbgadget.c
> > @@ -30,14 +30,18 @@
> >  #include <usb/dfu.h>
> >  #include <usb/gadget-multi.h>
> >  
> > +int usbgadget_register(bool dfu_set, const char *dfu_opts,
> > +		       bool fastboot_set, const char *fastboot_opts,
> > +		       bool create_serial, bool is_acm, bool export_bbu);
> > +
> 
> This should get a prototype somewhere in include/, but that's probably
> due to the draft status of this patch.

Any idea where would it fit? Or just create new file (which seems overkill
to me)?

> Otherwise looks fine to me.

usbgadget takes -s switch to create serial gadget and -a to create acm.
However I fail to see how is creating former done.

	ladis

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

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

* Re: [PATCH] usbgadget: autostart: add DFU support
  2018-10-26 11:10   ` Ladislav Michl
@ 2018-10-26 11:31     ` Sascha Hauer
  2018-10-26 13:54       ` Ladislav Michl
  0 siblings, 1 reply; 5+ messages in thread
From: Sascha Hauer @ 2018-10-26 11:31 UTC (permalink / raw)
  To: Ladislav Michl; +Cc: barebox

On Fri, Oct 26, 2018 at 01:10:47PM +0200, Ladislav Michl wrote:
> On Fri, Oct 26, 2018 at 12:19:11PM +0200, Sascha Hauer wrote:
> > Hi Ladis,
> > 
> > On Fri, Oct 26, 2018 at 12:37:19AM +0200, Ladislav Michl wrote:
> > > Use global variable dfu_function to autostart DFU. As similar code
> > > is used to start multifunction gadget using command, move common
> > > code to common/usbgadget.c and consolidate it.
> > > 
> > > Signed-off-by: Ladislav Michl <ladis@linux-mips.org>
> > > ---
> > >  This is a draft, comments welcome. Getting 
> > >  Cannot set parameter otg.mode: No such device
> > 
> > I just sent a fix for this.
> 
> Thanks, tested, added.
> 
> > >  but that's same as before this patch. Documentation is probably
> > >  also worth updating...
> > 
> > Indeed.
> 
> Will add prepatch to document bbu then.
> 
> > > diff --git a/commands/usbgadget.c b/commands/usbgadget.c
> > > index a1744cbe1..416b2acfc 100644
> > > --- a/commands/usbgadget.c
> > > +++ b/commands/usbgadget.c
> > > @@ -30,14 +30,18 @@
> > >  #include <usb/dfu.h>
> > >  #include <usb/gadget-multi.h>
> > >  
> > > +int usbgadget_register(bool dfu_set, const char *dfu_opts,
> > > +		       bool fastboot_set, const char *fastboot_opts,
> > > +		       bool create_serial, bool is_acm, bool export_bbu);
> > > +
> > 
> > This should get a prototype somewhere in include/, but that's probably
> > due to the draft status of this patch.
> 
> Any idea where would it fit? Or just create new file (which seems overkill
> to me)?

include/usb/gadget-multi.h perhaps? Not a perfect match, but in the end
that's what the function does: create a USB multi device.

> 
> > Otherwise looks fine to me.
> 
> usbgadget takes -s switch to create serial gadget and -a to create acm.
> However I fail to see how is creating former done.

To be honest I always use acm and never found out how the generic serial
works. It may have bitrotted over time, but for me it never worked.

Sascha

-- 
Pengutronix e.K.                           |                             |
Industrial Linux Solutions                 | http://www.pengutronix.de/  |
Peiner Str. 6-8, 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: [PATCH] usbgadget: autostart: add DFU support
  2018-10-26 11:31     ` Sascha Hauer
@ 2018-10-26 13:54       ` Ladislav Michl
  0 siblings, 0 replies; 5+ messages in thread
From: Ladislav Michl @ 2018-10-26 13:54 UTC (permalink / raw)
  To: Sascha Hauer; +Cc: barebox

On Fri, Oct 26, 2018 at 01:31:56PM +0200, Sascha Hauer wrote:
> On Fri, Oct 26, 2018 at 01:10:47PM +0200, Ladislav Michl wrote:
> > usbgadget takes -s switch to create serial gadget and -a to create acm.
> > However I fail to see how is creating former done.
> 
> To be honest I always use acm and never found out how the generic serial
> works. It may have bitrotted over time, but for me it never worked.

Fair enough then. I do not even see support for it in the initial commit,
which introduces '-s' switch. I suggest removing it altogether, perhaps
with an option to silently fall back to create ACM as well. ACM is so
common, that using anything else for this (console) purpose is probably
unwise anyway. Objections?

	ladis

_______________________________________________
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:[~2018-10-26 13:56 UTC | newest]

Thread overview: 5+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2018-10-25 22:37 [PATCH] usbgadget: autostart: add DFU support Ladislav Michl
2018-10-26 10:19 ` Sascha Hauer
2018-10-26 11:10   ` Ladislav Michl
2018-10-26 11:31     ` Sascha Hauer
2018-10-26 13:54       ` Ladislav Michl

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