diff mbox series

[U-Boot,04/14] dfu: allow to manage DFU on several devices

Message ID 20190913141930.15784-5-patrick.delaunay@st.com
State Changes Requested
Delegated to: Lukasz Majewski
Headers show
Series dfu: update dfu stack and add MTD backend | expand

Commit Message

Patrick DELAUNAY Sept. 13, 2019, 2:19 p.m. UTC 
Add support of DFU for several interface/device
with one command.

The format for "dfu_alt_info" in this case is :
  interface with devstring'='alternate list (';' separated)
  and each interface is separated by '&'

The previous behavior is always supported.

One example for NOR (bootloaders) + NAND (rootfs in UBI):

U-Boot> env set dfu_alt_info \
"sf 0:0:10000000:0=spl part 0 1;u-boot part 0 2; \
u-boot-env part 0 3&nand 0=UBI partubi 0,3"

U-Boot> dfu 0 list

DFU alt settings list:
dev: SF alt: 0 name: spl layout: RAW_ADDR
dev: SF alt: 1 name: ssbl layout: RAW_ADDR
dev: SF alt: 2 name: u-boot-env layout: RAW_ADDR
dev: NAND alt: 3 name: UBI layout: RAW_ADDR

U-Boot> dfu 0

$> dfu-util -l

Found DFU: [0483:5720] ver=9999, devnum=96, cfg=1,\
 intf=0, alt=3, name="UBI", serial="002700333338511934383330"
Found DFU: [0483:5720] ver=9999, devnum=96, cfg=1,\
 intf=0, alt=2, name="u-boot-env", serial="002700333338511934383330"
Found DFU: [0483:5720] ver=9999, devnum=96, cfg=1,\
 intf=0, alt=1, name="u-boot", serial="002700333338511934383330"
Found DFU: [0483:5720] ver=9999, devnum=96, cfg=1,\
 intf=0, alt=0, name="spl", serial="002700333338511934383330"

Signed-off-by: Patrick Delaunay <patrick.delaunay@st.com>
---

 cmd/dfu.c         | 21 ++++++++++-------
 drivers/dfu/dfu.c | 60 ++++++++++++++++++++++++++++++++++++++++++++++-
 2 files changed, 72 insertions(+), 9 deletions(-)

Comments

Lukasz Majewski Sept. 17, 2019, 10:25 a.m. UTC | #1 
Hi Patrick,

> Add support of DFU for several interface/device
> with one command.
> 
> The format for "dfu_alt_info" in this case is :
>   interface with devstring'='alternate list (';' separated)
>   and each interface is separated by '&'
> 
> The previous behavior is always supported.
> 
> One example for NOR (bootloaders) + NAND (rootfs in UBI):
> 
> U-Boot> env set dfu_alt_info \
> "sf 0:0:10000000:0=spl part 0 1;u-boot part 0 2; \
> u-boot-env part 0 3&nand 0=UBI partubi 0,3"
> 
> U-Boot> dfu 0 list
> 
> DFU alt settings list:
> dev: SF alt: 0 name: spl layout: RAW_ADDR
> dev: SF alt: 1 name: ssbl layout: RAW_ADDR
> dev: SF alt: 2 name: u-boot-env layout: RAW_ADDR
> dev: NAND alt: 3 name: UBI layout: RAW_ADDR
> 
> U-Boot> dfu 0
> 
> $> dfu-util -l  
> 
> Found DFU: [0483:5720] ver=9999, devnum=96, cfg=1,\
>  intf=0, alt=3, name="UBI", serial="002700333338511934383330"
> Found DFU: [0483:5720] ver=9999, devnum=96, cfg=1,\
>  intf=0, alt=2, name="u-boot-env", serial="002700333338511934383330"
> Found DFU: [0483:5720] ver=9999, devnum=96, cfg=1,\
>  intf=0, alt=1, name="u-boot", serial="002700333338511934383330"
> Found DFU: [0483:5720] ver=9999, devnum=96, cfg=1,\
>  intf=0, alt=0, name="spl", serial="002700333338511934383330"
> 

My two remarks:

1. As you mentioned above - the current behavior must be preserved
(this is my main concern).

2. You added the example of usage to the commit message. Could you also
add it to the ./doc/README.dfu (not yet present) file ?

Anyway, thanks for your work :-)

> Signed-off-by: Patrick Delaunay <patrick.delaunay@st.com>

Acked-by: Lukasz Majewski <lukma@denx.de>

> ---
> 
>  cmd/dfu.c         | 21 ++++++++++-------
>  drivers/dfu/dfu.c | 60
> ++++++++++++++++++++++++++++++++++++++++++++++- 2 files changed, 72
> insertions(+), 9 deletions(-)
> 
> diff --git a/cmd/dfu.c b/cmd/dfu.c
> index 91a750a4fc..33491d0bc9 100644
> --- a/cmd/dfu.c
> +++ b/cmd/dfu.c
> @@ -21,23 +21,28 @@
>  static int do_dfu(cmd_tbl_t *cmdtp, int flag, int argc, char * const
> argv[]) {
>  
> -	if (argc < 4)
> +	if (argc < 2)
>  		return CMD_RET_USAGE;
>  
>  #ifdef CONFIG_DFU_OVER_USB
>  	char *usb_controller = argv[1];
>  #endif
>  #if defined(CONFIG_DFU_OVER_USB) || defined(CONFIG_DFU_OVER_TFTP)
> -	char *interface = argv[2];
> -	char *devstring = argv[3];
> +	char *interface = NULL;
> +	char *devstring = NULL;
> +
> +	if (argc >= 4) {
> +		interface = argv[2];
> +		devstring = argv[3];
> +	}
>  #endif
>  
>  	int ret = 0;
>  #ifdef CONFIG_DFU_OVER_TFTP
>  	unsigned long addr = 0;
>  	if (!strcmp(argv[1], "tftp")) {
> -		if (argc == 5)
> -			addr = simple_strtoul(argv[4], NULL, 0);
> +		if (argc == 5 || argc == 3)
> +			addr = simple_strtoul(argv[argc - 1], NULL,
> 0); 
>  		return update_tftp(addr, interface, devstring);
>  	}
> @@ -48,7 +53,7 @@ static int do_dfu(cmd_tbl_t *cmdtp, int flag, int
> argc, char * const argv[]) goto done;
>  
>  	ret = CMD_RET_SUCCESS;
> -	if (argc > 4 && strcmp(argv[4], "list") == 0) {
> +	if (strcmp(argv[argc - 1], "list") == 0) {
>  		dfu_show_entities();
>  		goto done;
>  	}
> @@ -67,7 +72,7 @@ U_BOOT_CMD(dfu, CONFIG_SYS_MAXARGS, 1, do_dfu,
>  	"Device Firmware Upgrade",
>  	""
>  #ifdef CONFIG_DFU_OVER_USB
> -	"<USB_controller> <interface> <dev> [list]\n"
> +	"<USB_controller> [<interface> <dev>] [list]\n"
>  	"  - device firmware upgrade via <USB_controller>\n"
>  	"    on device <dev>, attached to interface\n"
>  	"    <interface>\n"
> @@ -77,7 +82,7 @@ U_BOOT_CMD(dfu, CONFIG_SYS_MAXARGS, 1, do_dfu,
>  #ifdef CONFIG_DFU_OVER_USB
>  	"dfu "
>  #endif
> -	"tftp <interface> <dev> [<addr>]\n"
> +	"tftp [<interface> <dev>] [<addr>]\n"
>  	"  - device firmware upgrade via TFTP\n"
>  	"    on device <dev>, attached to interface\n"
>  	"    <interface>\n"
> diff --git a/drivers/dfu/dfu.c b/drivers/dfu/dfu.c
> index 900a844d15..8bd5216017 100644
> --- a/drivers/dfu/dfu.c
> +++ b/drivers/dfu/dfu.c
> @@ -53,6 +53,54 @@ static int dfu_find_alt_num(const char *s)
>  	return ++i;
>  }
>  
> +/*
> + * treat dfu_alt_info with several interface information
> + * to allow DFU on several device with one command,
> + * the string format is
> + * interface devstring'='alternate list (';' separated)
> + * and each interface separated by '&'
> + */
> +int dfu_config_interfaces(char *env)
> +{
> +	struct dfu_entity *dfu;
> +	char *s, *i, *d, *a, *part;
> +	int ret = -EINVAL;
> +	int n = 1;
> +
> +	s = env;
> +	for (; *s; s++) {
> +		if (*s == ';')
> +			n++;
> +		if (*s == '&')
> +			n++;
> +	}
> +	ret = dfu_alt_init(n, &dfu);
> +	if (ret)
> +		return ret;
> +
> +	s = env;
> +	while (s) {
> +		ret = -EINVAL;
> +		i = strsep(&s, " ");
> +		if (!i)
> +			break;
> +		d = strsep(&s, "=");
> +		if (!d)
> +			break;
> +		a = strsep(&s, "&");
> +		if (!a)
> +			a = s;
> +		do {
> +			part = strsep(&a, ";");
> +			ret = dfu_alt_add(dfu, i, d, part);
> +			if (ret)
> +				return ret;
> +		} while (a);
> +	}
> +
> +	return ret;
> +}
> +
>  int dfu_init_env_entities(char *interface, char *devstr)
>  {
>  	const char *str_env;
> @@ -69,7 +117,11 @@ int dfu_init_env_entities(char *interface, char
> *devstr) }
>  
>  	env_bkp = strdup(str_env);
> -	ret = dfu_config_entities(env_bkp, interface, devstr);
> +	if (!interface && !devstr)
> +		ret = dfu_config_interfaces(env_bkp);
> +	else
> +		ret = dfu_config_entities(env_bkp, interface,
> devstr); +
>  	if (ret) {
>  		pr_err("DFU entities configuration failed!\n");
>  		pr_err("(partition table does not match
> dfu_alt_info?)\n"); @@ -83,6 +135,7 @@ done:
>  
>  static unsigned char *dfu_buf;
>  static unsigned long dfu_buf_size;
> +static enum dfu_device_type dfu_buf_device_type;
>  
>  unsigned char *dfu_free_buf(void)
>  {
> @@ -100,6 +153,10 @@ unsigned char *dfu_get_buf(struct dfu_entity
> *dfu) {
>  	char *s;
>  
> +	/* manage several entity with several contraint */
> +	if (dfu_buf && dfu->dev_type != dfu_buf_device_type)
> +		dfu_free_buf();
> +
>  	if (dfu_buf != NULL)
>  		return dfu_buf;
>  
> @@ -118,6 +175,7 @@ unsigned char *dfu_get_buf(struct dfu_entity *dfu)
>  		printf("%s: Could not memalign 0x%lx bytes\n",
>  		       __func__, dfu_buf_size);
>  
> +	dfu_buf_device_type = dfu->dev_type;
>  	return dfu_buf;
>  }
>  



Best regards,

Lukasz Majewski

--

DENX Software Engineering GmbH,      Managing Director: Wolfgang Denk
HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany
Phone: (+49)-8142-66989-59 Fax: (+49)-8142-66989-80 Email: lukma@denx.de
Patrick DELAUNAY Sept. 17, 2019, 11:28 a.m. UTC | #2 
Hi Lukasz,

> From: Lukasz Majewski <lukma@denx.de>
> Sent: mardi 17 septembre 2019 12:26
> To: Patrick DELAUNAY <patrick.delaunay@st.com>
> Cc: u-boot@lists.denx.de; U-Boot STM32 <uboot-stm32@st-md-
> mailman.stormreply.com>
> Subject: Re: [PATCH 04/14] dfu: allow to manage DFU on several devices
> Importance: High
> 
> Hi Patrick,
> 
> > Add support of DFU for several interface/device with one command.
> >
> > The format for "dfu_alt_info" in this case is :
> >   interface with devstring'='alternate list (';' separated)
> >   and each interface is separated by '&'
> >
> > The previous behavior is always supported.
> >
> > One example for NOR (bootloaders) + NAND (rootfs in UBI):
> >
> > U-Boot> env set dfu_alt_info \
> > "sf 0:0:10000000:0=spl part 0 1;u-boot part 0 2; \ u-boot-env part 0
> > 3&nand 0=UBI partubi 0,3"
> >
> > U-Boot> dfu 0 list
> >
> > DFU alt settings list:
> > dev: SF alt: 0 name: spl layout: RAW_ADDR
> > dev: SF alt: 1 name: ssbl layout: RAW_ADDR
> > dev: SF alt: 2 name: u-boot-env layout: RAW_ADDR
> > dev: NAND alt: 3 name: UBI layout: RAW_ADDR
> >
> > U-Boot> dfu 0
> >
> > $> dfu-util -l
> >
> > Found DFU: [0483:5720] ver=9999, devnum=96, cfg=1,\  intf=0, alt=3,
> > name="UBI", serial="002700333338511934383330"
> > Found DFU: [0483:5720] ver=9999, devnum=96, cfg=1,\  intf=0, alt=2,
> > name="u-boot-env", serial="002700333338511934383330"
> > Found DFU: [0483:5720] ver=9999, devnum=96, cfg=1,\  intf=0, alt=1,
> > name="u-boot", serial="002700333338511934383330"
> > Found DFU: [0483:5720] ver=9999, devnum=96, cfg=1,\  intf=0, alt=0,
> > name="spl", serial="002700333338511934383330"
> >
> 
> My two remarks:
> 
> 1. As you mentioned above - the current behavior must be preserved (this is my
> main concern).

I agree, it was also my concern.

I don't indicated it clearly by I test it on my board and it but it is preserved.

For example, on my stm32mp1 board :

STM32MP> env set dfu_alt_info "sdcard_fsbl1 part 0 1;sdcard_fsbl2 part 0 2;sdcard_ssbl part 0 3;sdcard_bootfs part 0 4;sdcard_vendorfs part 0 5;sdcard_rootfs part 0 6"
STM32MP> dfu 0 mmc 0

On the host side 

dfu-util -l
dfu-util 0.9

Copyright 2005-2009 Weston Schmidt, Harald Welte and OpenMoko Inc.
Copyright 2010-2016 Tormod Volden and Stefan Schmidt
This program is Free Software and has ABSOLUTELY NO WARRANTY
Please report bugs to http://sourceforge.net/p/dfu-util/tickets/

Found DFU: [0483:df11] ver=0200, devnum=17, cfg=1, intf=0, path="3-1.3.1", alt=5, name="sdcard_rootfs", serial="002700333338511934383330"
Found DFU: [0483:df11] ver=0200, devnum=17, cfg=1, intf=0, path="3-1.3.1", alt=4, name="sdcard_vendorfs", serial="002700333338511934383330"
Found DFU: [0483:df11] ver=0200, devnum=17, cfg=1, intf=0, path="3-1.3.1", alt=3, name="sdcard_bootfs", serial="002700333338511934383330"
Found DFU: [0483:df11] ver=0200, devnum=17, cfg=1, intf=0, path="3-1.3.1", alt=2, name="sdcard_ssbl", serial="002700333338511934383330"
Found DFU: [0483:df11] ver=0200, devnum=17, cfg=1, intf=0, path="3-1.3.1", alt=1, name="sdcard_fsbl2", serial="002700333338511934383330"
Found DFU: [0483:df11] ver=0200, devnum=17, cfg=1, intf=0, path="3-1.3.1", alt=0, name="sdcard_fsbl1", serial="002700333338511934383330"


> 2. You added the example of usage to the commit message. Could you also add it
> to the ./doc/README.dfu (not yet present) file ?

Yes I willl create the file in V2

> Anyway, thanks for your work :-)
> 
> > Signed-off-by: Patrick Delaunay <patrick.delaunay@st.com>
> 
> Acked-by: Lukasz Majewski <lukma@denx.de>
> 

Regards

Patrick
diff mbox series

Patch

diff --git a/cmd/dfu.c b/cmd/dfu.c
index 91a750a4fc..33491d0bc9 100644
--- a/cmd/dfu.c
+++ b/cmd/dfu.c
@@ -21,23 +21,28 @@ 
 static int do_dfu(cmd_tbl_t *cmdtp, int flag, int argc, char * const argv[])
 {
 
-	if (argc < 4)
+	if (argc < 2)
 		return CMD_RET_USAGE;
 
 #ifdef CONFIG_DFU_OVER_USB
 	char *usb_controller = argv[1];
 #endif
 #if defined(CONFIG_DFU_OVER_USB) || defined(CONFIG_DFU_OVER_TFTP)
-	char *interface = argv[2];
-	char *devstring = argv[3];
+	char *interface = NULL;
+	char *devstring = NULL;
+
+	if (argc >= 4) {
+		interface = argv[2];
+		devstring = argv[3];
+	}
 #endif
 
 	int ret = 0;
 #ifdef CONFIG_DFU_OVER_TFTP
 	unsigned long addr = 0;
 	if (!strcmp(argv[1], "tftp")) {
-		if (argc == 5)
-			addr = simple_strtoul(argv[4], NULL, 0);
+		if (argc == 5 || argc == 3)
+			addr = simple_strtoul(argv[argc - 1], NULL, 0);
 
 		return update_tftp(addr, interface, devstring);
 	}
@@ -48,7 +53,7 @@  static int do_dfu(cmd_tbl_t *cmdtp, int flag, int argc, char * const argv[])
 		goto done;
 
 	ret = CMD_RET_SUCCESS;
-	if (argc > 4 && strcmp(argv[4], "list") == 0) {
+	if (strcmp(argv[argc - 1], "list") == 0) {
 		dfu_show_entities();
 		goto done;
 	}
@@ -67,7 +72,7 @@  U_BOOT_CMD(dfu, CONFIG_SYS_MAXARGS, 1, do_dfu,
 	"Device Firmware Upgrade",
 	""
 #ifdef CONFIG_DFU_OVER_USB
-	"<USB_controller> <interface> <dev> [list]\n"
+	"<USB_controller> [<interface> <dev>] [list]\n"
 	"  - device firmware upgrade via <USB_controller>\n"
 	"    on device <dev>, attached to interface\n"
 	"    <interface>\n"
@@ -77,7 +82,7 @@  U_BOOT_CMD(dfu, CONFIG_SYS_MAXARGS, 1, do_dfu,
 #ifdef CONFIG_DFU_OVER_USB
 	"dfu "
 #endif
-	"tftp <interface> <dev> [<addr>]\n"
+	"tftp [<interface> <dev>] [<addr>]\n"
 	"  - device firmware upgrade via TFTP\n"
 	"    on device <dev>, attached to interface\n"
 	"    <interface>\n"
diff --git a/drivers/dfu/dfu.c b/drivers/dfu/dfu.c
index 900a844d15..8bd5216017 100644
--- a/drivers/dfu/dfu.c
+++ b/drivers/dfu/dfu.c
@@ -53,6 +53,54 @@  static int dfu_find_alt_num(const char *s)
 	return ++i;
 }
 
+/*
+ * treat dfu_alt_info with several interface information
+ * to allow DFU on several device with one command,
+ * the string format is
+ * interface devstring'='alternate list (';' separated)
+ * and each interface separated by '&'
+ */
+int dfu_config_interfaces(char *env)
+{
+	struct dfu_entity *dfu;
+	char *s, *i, *d, *a, *part;
+	int ret = -EINVAL;
+	int n = 1;
+
+	s = env;
+	for (; *s; s++) {
+		if (*s == ';')
+			n++;
+		if (*s == '&')
+			n++;
+	}
+	ret = dfu_alt_init(n, &dfu);
+	if (ret)
+		return ret;
+
+	s = env;
+	while (s) {
+		ret = -EINVAL;
+		i = strsep(&s, " ");
+		if (!i)
+			break;
+		d = strsep(&s, "=");
+		if (!d)
+			break;
+		a = strsep(&s, "&");
+		if (!a)
+			a = s;
+		do {
+			part = strsep(&a, ";");
+			ret = dfu_alt_add(dfu, i, d, part);
+			if (ret)
+				return ret;
+		} while (a);
+	}
+
+	return ret;
+}
+
 int dfu_init_env_entities(char *interface, char *devstr)
 {
 	const char *str_env;
@@ -69,7 +117,11 @@  int dfu_init_env_entities(char *interface, char *devstr)
 	}
 
 	env_bkp = strdup(str_env);
-	ret = dfu_config_entities(env_bkp, interface, devstr);
+	if (!interface && !devstr)
+		ret = dfu_config_interfaces(env_bkp);
+	else
+		ret = dfu_config_entities(env_bkp, interface, devstr);
+
 	if (ret) {
 		pr_err("DFU entities configuration failed!\n");
 		pr_err("(partition table does not match dfu_alt_info?)\n");
@@ -83,6 +135,7 @@  done:
 
 static unsigned char *dfu_buf;
 static unsigned long dfu_buf_size;
+static enum dfu_device_type dfu_buf_device_type;
 
 unsigned char *dfu_free_buf(void)
 {
@@ -100,6 +153,10 @@  unsigned char *dfu_get_buf(struct dfu_entity *dfu)
 {
 	char *s;
 
+	/* manage several entity with several contraint */
+	if (dfu_buf && dfu->dev_type != dfu_buf_device_type)
+		dfu_free_buf();
+
 	if (dfu_buf != NULL)
 		return dfu_buf;
 
@@ -118,6 +175,7 @@  unsigned char *dfu_get_buf(struct dfu_entity *dfu)
 		printf("%s: Could not memalign 0x%lx bytes\n",
 		       __func__, dfu_buf_size);
 
+	dfu_buf_device_type = dfu->dev_type;
 	return dfu_buf;
 }