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 |
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
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 --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; }
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(-)