| Message ID | 20220827114835.22584-1-pali@kernel.org |
|---|---|
| State | Superseded |
| Delegated to: | Tom Rini |
| Headers | show |
| Series | [v2] xyz-modem: Allow to configure initial timeout for loadx and loady | expand |
On Sat, Aug 27, 2022 at 01:48:35PM +0200, 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 This breaks platforms like "porter" where we have SPL_YMODEM support but do not have environment support.
On Saturday 27 August 2022 08:53:08 Tom Rini wrote: > On Sat, Aug 27, 2022 at 01:48:35PM +0200, 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 > > This breaks platforms like "porter" where we have SPL_YMODEM support but > do not have environment support. By "breaks" do you mean compile error? Because if env is not set then CONFIG_CMD_LOADXY_TIMEOUT is used. Just I'm not sure if env_get() function returns NULL or what when there is no environment support.
On Sat, Aug 27, 2022 at 02:56:15PM +0200, Pali Rohár wrote: > On Saturday 27 August 2022 08:53:08 Tom Rini wrote: > > On Sat, Aug 27, 2022 at 01:48:35PM +0200, 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 > > > > This breaks platforms like "porter" where we have SPL_YMODEM support but > > do not have environment support. > > By "breaks" do you mean compile error? Because if env is not set then > CONFIG_CMD_LOADXY_TIMEOUT is used. Just I'm not sure if env_get() > function returns NULL or what when there is no environment support. Yes, it fails to link.
On Saturday 27 August 2022 10:30:05 Tom Rini wrote: > On Sat, Aug 27, 2022 at 02:56:15PM +0200, Pali Rohár wrote: > > On Saturday 27 August 2022 08:53:08 Tom Rini wrote: > > > On Sat, Aug 27, 2022 at 01:48:35PM +0200, 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 > > > > > > This breaks platforms like "porter" where we have SPL_YMODEM support but > > > do not have environment support. > > > > By "breaks" do you mean compile error? Because if env is not set then > > CONFIG_CMD_LOADXY_TIMEOUT is used. Just I'm not sure if env_get() > > function returns NULL or what when there is no environment support. > > Yes, it fails to link. Then just #ifdef for env is needed. I will prepare v3. Which defconfig it is?
On Sat, Aug 27, 2022 at 04:32:37PM +0200, Pali Rohár wrote: > On Saturday 27 August 2022 10:30:05 Tom Rini wrote: > > On Sat, Aug 27, 2022 at 02:56:15PM +0200, Pali Rohár wrote: > > > On Saturday 27 August 2022 08:53:08 Tom Rini wrote: > > > > On Sat, Aug 27, 2022 at 01:48:35PM +0200, 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 > > > > > > > > This breaks platforms like "porter" where we have SPL_YMODEM support but > > > > do not have environment support. > > > > > > By "breaks" do you mean compile error? Because if env is not set then > > > CONFIG_CMD_LOADXY_TIMEOUT is used. Just I'm not sure if env_get() > > > function returns NULL or what when there is no environment support. > > > > Yes, it fails to link. > > Then just #ifdef for env is needed. I will prepare v3. Which defconfig it is? It's the "porter" defconfig, sorry I thought it would be clearer. There's probably others too in the case of SPL_YMODEM without SPL_ENV, it was just the first one I guessed.
On Saturday 27 August 2022 10:41:00 Tom Rini wrote: > On Sat, Aug 27, 2022 at 04:32:37PM +0200, Pali Rohár wrote: > > On Saturday 27 August 2022 10:30:05 Tom Rini wrote: > > > On Sat, Aug 27, 2022 at 02:56:15PM +0200, Pali Rohár wrote: > > > > On Saturday 27 August 2022 08:53:08 Tom Rini wrote: > > > > > On Sat, Aug 27, 2022 at 01:48:35PM +0200, 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 > > > > > > > > > > This breaks platforms like "porter" where we have SPL_YMODEM support but > > > > > do not have environment support. > > > > > > > > By "breaks" do you mean compile error? Because if env is not set then > > > > CONFIG_CMD_LOADXY_TIMEOUT is used. Just I'm not sure if env_get() > > > > function returns NULL or what when there is no environment support. > > > > > > Yes, it fails to link. > > > > Then just #ifdef for env is needed. I will prepare v3. Which defconfig it is? > > It's the "porter" defconfig, sorry I thought it would be clearer. > There's probably others too in the case of SPL_YMODEM without SPL_ENV, > it was just the first one I guessed. Ou, I did not think that porter is defconfig. It sounds like code porting. Anyway I have already sent v3 and now I tested it with porter_defconfig that it compiles fine: https://patchwork.ozlabs.org/project/uboot/patch/20220827143755.21412-1-pali@kernel.org/
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;
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;
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 +++++++++++++++++++++++++++++++++++++-- 2 files changed, 44 insertions(+), 2 deletions(-)