Re: [PATCH v2] xyz-modem: Allow to configure initial timeout for loadx and loady

[Heinrich Schuchardt <xypron.glpk@gmx.de>]

On 8/27/22 13:48, Pali Rohár wrote:
> Now when loadx and loady commands could be aborted / cancelled by CTRL+C,
> allow to configure timeout for initial x/y-modem packet via env variable
> $loadxy_timeout and by default use value from new compile-time config
> option CONFIG_CMD_LOADXY_TIMEOUT. Value is in seconds and zero value means
> infinite timeout. Default value is 90s which is the measured value used
> before this change for loadx and loady commands.
>
> Other load commands loadb and loads already waits infinitely. Same behavior
> for loadx and loady commands can be achieved by setting $loadxy_timeout or
> CONFIG_CMD_LOADXY_TIMEOUT to 0.
>
> Signed-off-by: Pali Rohár <pali@kernel.org>
> ---
> Changes in v2:
> * Allow to set timeout via env instead of permanent infinite timeout
> ---
>   cmd/Kconfig       |  7 +++++++
>   common/xyzModem.c | 39 +++++++++++++++++++++++++++++++++++++--

The environment variable should be described in doc/usage/cmd/loady.rst

Best regards

Heinrich


>   2 files changed, 44 insertions(+), 2 deletions(-)
>
> diff --git a/cmd/Kconfig b/cmd/Kconfig
> index 211ebe9c8783..54af3769a673 100644
> --- a/cmd/Kconfig
> +++ b/cmd/Kconfig
> @@ -1194,6 +1194,13 @@ config CMD_LOADS
>   	help
>   	  Load an S-Record file over serial line
>
> +config CMD_LOADXY_TIMEOUT
> +	int "loadxy_timeout"
> +	range 0 2000
> +	default 90
> +	help
> +	  Initial timeout for loadx and loady commands. Zero means infinity.
> +
>   config CMD_LSBLK
>   	depends on BLK
>   	bool "lsblk - list block drivers and devices"
> diff --git a/common/xyzModem.c b/common/xyzModem.c
> index ece25acb183b..700df8edd14a 100644
> --- a/common/xyzModem.c
> +++ b/common/xyzModem.c
> @@ -26,6 +26,7 @@
>   #include <stdarg.h>
>   #include <u-boot/crc.h>
>   #include <watchdog.h>
> +#include <env.h>
>
>   /* Assumption - run xyzModem protocol over the console port */
>
> @@ -50,6 +51,8 @@ static struct
>     int len, mode, total_retries;
>     int total_SOH, total_STX, total_CAN;
>     bool crc_mode, at_eof, tx_ack;
> +  bool first_xmodem_packet;
> +  ulong initial_time, timeout;
>     unsigned long file_length, read_length;
>   } xyz;
>
> @@ -409,6 +412,18 @@ xyzModem_get_hdr (void)
>     return 0;
>   }
>
> +static
> +ulong
> +xyzModem_get_initial_timeout (void)
> +{
> +  /* timeout is in seconds, non-positive timeout value is infinity */
> +  const char *timeout_str = env_get("loadxy_timeout");
> +  if (timeout_str)
> +    return 1000 * simple_strtol(timeout_str, NULL, 10);
> +  else
> +    return 1000 * CONFIG_CMD_LOADXY_TIMEOUT;
> +}
> +
>   int
>   xyzModem_stream_open (connection_info_t * info, int *err)
>   {
> @@ -439,18 +454,28 @@ xyzModem_stream_open (connection_info_t * info, int *err)
>     xyz.total_CAN = 0;
>     xyz.read_length = 0;
>     xyz.file_length = 0;
> +  xyz.first_xmodem_packet = false;
> +  xyz.initial_time = get_timer(0);
> +  xyz.timeout = xyzModem_get_initial_timeout();
>
>     CYGACC_COMM_IF_PUTC (*xyz.__chan, (xyz.crc_mode ? 'C' : NAK));
>
>     if (xyz.mode == xyzModem_xmodem)
>       {
>         /* X-modem doesn't have an information header - exit here */
> +      xyz.first_xmodem_packet = true;
>         xyz.next_blk = 1;
>         return 0;
>       }
>
> -  while (retries-- > 0)
> +  while (!(xyz.timeout && get_timer(xyz.initial_time) > xyz.timeout))
>       {
> +      if (--retries <= 0)
> +        {
> +          retries = xyzModem_MAX_RETRIES;
> +          crc_retries = xyzModem_MAX_RETRIES_WITH_CRC;
> +          xyz.crc_mode = true;
> +        }
>         stat = xyzModem_get_hdr ();
>         if (stat == 0)
>   	{
> @@ -503,9 +528,19 @@ xyzModem_stream_read (char *buf, int size, int *err)
>   	  retries = xyzModem_MAX_RETRIES;
>   	  while (retries-- > 0)
>   	    {
> +	      if (xyz.first_xmodem_packet && xyz.timeout &&
> +		  get_timer(xyz.initial_time) > xyz.timeout)
> +		{
> +		  *err = xyzModem_timeout;
> +		  xyz.len = -1;
> +		  return total;
> +		}
> +
>   	      stat = xyzModem_get_hdr ();
>   	      if (stat == 0)
>   		{
> +		  if (xyz.mode == xyzModem_xmodem && xyz.first_xmodem_packet)
> +		    xyz.first_xmodem_packet = false;
>   		  if (xyz.blk == xyz.next_blk)
>   		    {
>   		      xyz.tx_ack = true;
> @@ -583,7 +618,7 @@ xyzModem_stream_read (char *buf, int size, int *err)
>   	      xyz.total_retries++;
>   	      ZM_DEBUG (zm_dprintf ("NAK (%d)\n", __LINE__));
>   	    }
> -	  if (stat < 0)
> +	  if (stat < 0 && (!xyz.first_xmodem_packet || stat != xyzModem_timeout))
>   	    {
>   	      *err = stat;
>   	      xyz.len = -1;