Hi Andy, Thank you for your work on enhancing DFU. The patch series is generally Ok.
Please find some minor comments/requests below. > When the `dfu` command is called from the U-Boot environment, > it now accepts an optional parameter that specifies a timeout (in > seconds). If a DFU connection is not made within that time the `dfu` > command exits (as it would if Ctrl+C was pressed). If the timeout is > left empty or being zero the `dfu` command behaves as it does now. > > This is useful for allowing U-Boot to check to see if anything wants > to upload new firmware before continuing to boot. > > The patch is based on the commit > https://github.com/01org/edison-u-boot/commit/5e966ccc3c65c18c9783741fa04e0c45e021780c > which has been heavily reworked due to U-Boot changes in the past. > > Signed-off-by: Sebastien Colleur <sebastienx.coll...@intel.com> > Signed-off-by: Brad Campbell <brad...@gmail.com> > Signed-off-by: Andy Shevchenko <andriy.shevche...@linux.intel.com> > --- > cmd/dfu.c | 15 +++++++++++++-- > common/dfu.c | 17 +++++++++++++++++ > drivers/dfu/Kconfig | 6 ++++++ > drivers/dfu/dfu.c | 15 +++++++++++++++ > include/dfu.h | 5 +++++ > 5 files changed, 56 insertions(+), 2 deletions(-) > > diff --git a/cmd/dfu.c b/cmd/dfu.c > index 14a8ec879e..b30f8a5667 100644 > --- a/cmd/dfu.c > +++ b/cmd/dfu.c > @@ -30,7 +30,7 @@ static int do_dfu(cmd_tbl_t *cmdtp, int flag, int > argc, char * const argv[]) #if defined(CONFIG_DFU_OVER_USB) || > defined(CONFIG_DFU_OVER_TFTP) char *interface = NULL; > char *devstring = NULL; > -#if defined(CONFIG_DFU_OVER_TFTP) > +#if defined(CONFIG_DFU_TIMEOUT) || defined(CONFIG_DFU_OVER_TFTP) > unsigned long value = 0; > #endif > > @@ -39,7 +39,7 @@ static int do_dfu(cmd_tbl_t *cmdtp, int flag, int > argc, char * const argv[]) devstring = argv[3]; > } > > -#if defined(CONFIG_DFU_OVER_TFTP) > +#if defined(CONFIG_DFU_TIMEOUT) || defined(CONFIG_DFU_OVER_TFTP) > if (argc == 5 || argc == 3) > value = simple_strtoul(argv[argc - 1], NULL, 0); > #endif > @@ -55,6 +55,10 @@ static int do_dfu(cmd_tbl_t *cmdtp, int flag, int > argc, char * const argv[]) if (ret) > goto done; > > +#ifdef CONFIG_DFU_TIMEOUT > + dfu_set_timeout(value * 1000); > +#endif > + > ret = CMD_RET_SUCCESS; > if (strcmp(argv[argc - 1], "list") == 0) { > dfu_show_entities(); > @@ -75,10 +79,17 @@ U_BOOT_CMD(dfu, CONFIG_SYS_MAXARGS, 1, do_dfu, > "Device Firmware Upgrade", > "" > #ifdef CONFIG_DFU_OVER_USB > +#ifdef CONFIG_DFU_TIMEOUT > + "<USB_controller> [<interface> <dev>] [<timeout>] [list]\n" > +#else > "<USB_controller> [<interface> <dev>] [list]\n" > +#endif > " - device firmware upgrade via <USB_controller>\n" > " on device <dev>, attached to interface\n" > " <interface>\n" > +#ifdef CONFIG_DFU_TIMEOUT > + " [<timeout>] - specify inactivity timeout in seconds\n" > +#endif > " [list] - list available alt settings\n" > #endif > #ifdef CONFIG_DFU_OVER_TFTP > diff --git a/common/dfu.c b/common/dfu.c > index 44d1484d3d..da6289b218 100644 > --- a/common/dfu.c > +++ b/common/dfu.c > @@ -35,6 +35,10 @@ int run_usb_dnl_gadget(int usbctrl_index, char > *usb_dnl_gadget) return CMD_RET_FAILURE; > } > > +#ifdef CONFIG_DFU_TIMEOUT > + unsigned long start_time = get_timer(0); > +#endif > + > while (1) { > if (g_dnl_detach()) { > /* > @@ -79,6 +83,19 @@ int run_usb_dnl_gadget(int usbctrl_index, char > *usb_dnl_gadget) } > } > > +#ifdef CONFIG_DFU_TIMEOUT > + unsigned long wait_time = dfu_get_timeout(); > + > + if (wait_time) { > + unsigned long current_time = > get_timer(start_time); + > + if (current_time > wait_time) { > + debug("Inactivity timeout, abort > DFU\n"); > + goto exit; > + } > + } > +#endif > + > WATCHDOG_RESET(); > usb_gadget_handle_interrupts(usbctrl_index); > } > diff --git a/drivers/dfu/Kconfig b/drivers/dfu/Kconfig > index 9fe5bc0f58..e070130b5a 100644 > --- a/drivers/dfu/Kconfig > +++ b/drivers/dfu/Kconfig > @@ -23,6 +23,12 @@ config DFU_TFTP > > Detailed description of this feature can be found at > ./doc/README.dfutftp > +config DFU_TIMEOUT > + bool "Timeout waiting for DFU" > + help > + This option adds an optional timeout parameter for DFU > which, if set, > + will cause DFU to only wait for that many seconds before > exiting. + > config DFU_MMC > bool "MMC back end for DFU" > help > diff --git a/drivers/dfu/dfu.c b/drivers/dfu/dfu.c > index 38aecd3a05..df50196dfd 100644 > --- a/drivers/dfu/dfu.c > +++ b/drivers/dfu/dfu.c > @@ -21,6 +21,9 @@ static LIST_HEAD(dfu_list); > static int dfu_alt_num; > static int alt_num_cnt; > static struct hash_algo *dfu_hash_algo; > +#ifdef CONFIG_DFU_TIMEOUT > +static unsigned long dfu_timeout = 0; > +#endif > > /* > * The purpose of the dfu_flush_callback() function is to > @@ -58,6 +61,18 @@ __weak bool dfu_usb_get_reset(void) > #endif > } > > +#ifdef CONFIG_DFU_TIMEOUT > +void dfu_set_timeout(unsigned long timeout) > +{ > + dfu_timeout = timeout; > +} I do guess that dfu_set_timeout() is not yet used in this patch series? > + > +unsigned long dfu_get_timeout(void) > +{ > + return dfu_timeout; > +} > +#endif > + > static int dfu_find_alt_num(const char *s) > { > int i = 0; > diff --git a/include/dfu.h b/include/dfu.h > index 2e3e91c8d2..fb5260d903 100644 > --- a/include/dfu.h > +++ b/include/dfu.h > @@ -178,6 +178,11 @@ unsigned char *dfu_free_buf(void); > unsigned long dfu_get_buf_size(void); > bool dfu_usb_get_reset(void); > > +#ifdef CONFIG_DFU_TIMEOUT > +unsigned long dfu_get_timeout(void); > +void dfu_set_timeout(unsigned long); > +#endif > + > int dfu_read(struct dfu_entity *de, void *buf, int size, int > blk_seq_num); int dfu_write(struct dfu_entity *de, void *buf, int > size, int blk_seq_num); int dfu_flush(struct dfu_entity *de, void > *buf, int size, int blk_seq_num); Please add some description and example of this new option / feature to ./doc/README.dfu file. 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: lu...@denx.de
pgpUolDEw3YyS.pgp
Description: OpenPGP digital signature
_______________________________________________ U-Boot mailing list U-Boot@lists.denx.de https://lists.denx.de/listinfo/u-boot