[PATCH v2 3/4] dfu: Add optional timeout parameter
Simon Glass
sjg at chromium.org
Sat Dec 28 03:27:00 CET 2019
HI Andy,
On Wed, 27 Nov 2019 at 09:12, Andy Shevchenko
<andriy.shevchenko at linux.intel.com> wrote:
>
> 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
> by Sebastien Colleur, which has been heavily reworked due to U-Boot changes
> in the past.
>
> Signed-off-by: Brad Campbell <bradjc5 at gmail.com>
> Signed-off-by: Andy Shevchenko <andriy.shevchenko at linux.intel.com>
> ---
> v2:
> - add documentation part (Lukasz)
> - drop original author's SoB due to bouncing email, give credit
> in the commit message anyway
>
> cmd/dfu.c | 15 +++++++++++++--
> common/dfu.c | 17 +++++++++++++++++
> doc/README.dfu | 8 ++++++--
> drivers/dfu/Kconfig | 6 ++++++
> drivers/dfu/dfu.c | 15 +++++++++++++++
> include/dfu.h | 5 +++++
> 6 files changed, 62 insertions(+), 4 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
Can this all use
if (IS_ENABLED(CONFIG_DFU_TIMEOUT))
to reduce the number of #ifdefs?
Also I wonder if CONFIG_VAL(...) returns 0 if the CONFIG doesn't
exist? If so, you might be able to remove all the #ifdefs and use 0 as
the value that means infinite?
> + 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/doc/README.dfu b/doc/README.dfu
> index 558d347c26..caf1c9998c 100644
> --- a/doc/README.dfu
> +++ b/doc/README.dfu
> @@ -43,6 +43,7 @@ Configuration Options:
> CONFIG_DFU_RAM
> CONFIG_DFU_SF
> CONFIG_DFU_SF_PART
> + CONFIG_DFU_TIMEOUT
> CONFIG_DFU_VIRTUAL
> CONFIG_CMD_DFU
>
> @@ -70,12 +71,15 @@ Commands:
> dfu <USB_controller> [<interface> <dev>] list
> list the alternate device defined in "dfu_alt_info"
>
> - dfu <USB_controller> [<interface> <dev>]
> + dfu <USB_controller> [<interface> <dev>] [<timeout>]
> start the dfu stack on the USB instance with the selected medium
> backend and use the "dfu_alt_info" variable to configure the
> alternate setting and link each one with the medium
> The dfu command continue until receive a ^C in console or
> - a DFU detach transaction from HOST.
> + a DFU detach transaction from HOST. If CONFIG_DFU_TIMEOUT option
> + is enabled and <timeout> parameter is present in the command line,
> + the DFU operation will be aborted automatically after <timeout>
> + seconds of waiting remote to initiate DFU session.
>
> The possible values of <interface> are :
> (with <USB controller> = 0 in the dfu command example)
> 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;
> +}
> +
> +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);
Please add function comments.
> +#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);
> --
> 2.24.0
>
Regards,
Simon
More information about the U-Boot
mailing list