Re: [PATCH u-boot-marvell v3 16/39] tools: kwboot: Prevent waiting indefinitely if no xmodem reply is received

[Stefan Roese <sr@denx.de>]

On 24.09.21 23:06, Marek Behún wrote:
> From: Marek Behún <marek.behun@nic.cz>
> 
> Currently if BootROM fails to respond with ACK/NAK to a xmodem block, we
> will be waiting indefinitely for such response.
> 
> Make sure that we only wait at most 1 second (blk_rsp_timeo) for ACK/NAK
> for each block in case non-xmodem text output is not being expected.
> Interpret this timeout expiration as NAK, to try to send the block
> again.
> 
> On the other hand, if timeout expires without ACK while some non-xmodem
> output was already received (DDR training output, for example), we know
> that the block was received, since the code is being executed, so in
> this case exit with ETIMEDOUT.
> 
> Signed-off-by: Marek Behún <marek.behun@nic.cz>

Reviewed-by: Stefan Roese <sr@denx.de>

Thanks,
Stefan

> ---
>   tools/kwboot.c | 14 +++++++++++---
>   1 file changed, 11 insertions(+), 3 deletions(-)
> 
> diff --git a/tools/kwboot.c b/tools/kwboot.c
> index cf6e32c6fa..8c11dca5ed 100644
> --- a/tools/kwboot.c
> +++ b/tools/kwboot.c
> @@ -407,17 +407,18 @@ static int
>   kwboot_xm_recv_reply(int fd, char *c, int allow_non_xm, int *non_xm_print)
>   {
>   	int timeout = allow_non_xm ? KWBOOT_HDR_RSP_TIMEO : blk_rsp_timeo;
> -	uint64_t recv_until = 0;
> +	uint64_t recv_until = _now() + timeout;
>   	int rc;
>   
> -	*non_xm_print = 0;
> +	if (non_xm_print)
> +		*non_xm_print = 0;
>   
>   	while (1) {
>   		rc = kwboot_tty_recv(fd, c, 1, timeout);
>   		if (rc) {
>   			if (errno != ETIMEDOUT)
>   				return rc;
> -			else if (recv_until && recv_until < _now())
> +			else if (allow_non_xm && *non_xm_print)
>   				return -1;
>   			else
>   				*c = NAK;
> @@ -430,12 +431,19 @@ kwboot_xm_recv_reply(int fd, char *c, int allow_non_xm, int *non_xm_print)
>   		/*
>   		 * If printing non-xmodem text output is allowed and such a byte
>   		 * was received, print it and increase receiving time.
> +		 * Otherwise decrease timeout by time elapsed.
>   		 */
>   		if (allow_non_xm) {
>   			recv_until = _now() + timeout;
>   			putchar(*c);
>   			fflush(stdout);
>   			*non_xm_print = 1;
> +		} else {
> +			timeout = recv_until - _now();
> +			if (timeout < 0) {
> +				errno = ETIMEDOUT;
> +				return -1;
> +			}
>   		}
>   	}
>   
> 


Viele Grüße,
Stefan

-- 
DENX Software Engineering GmbH,      Managing Director: Wolfgang Denk
HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany
Phone: (+49)-8142-66989-51 Fax: (+49)-8142-66989-80 Email: sr@denx.de