Re: [PATCH u-boot-marvell 03/14] tools: kwboot: Improve retrying logic for incomplete xmodem packets

Stefan Roese
2022-01-26T15:35:02Z

On 1/25/22 18:13, Marek Behún wrote:
> From: Pali Rohár <pali@kernel.org>
>
> Sometimes if the first byte of xmodem packet (SOH) is incorrectly
> transmitted, BootROM sends NAK for every non-SOH received byte, which
> makes BootROM and the host kwboot tool out of sync. BootROM automatically
> re-synchronizes after 2s pause by dropping its input queue. So when
> attempting retransmit for 9th time or later, ignore NAK reply from BootROM
> and either wait for valid ACK or let kwboot timeout, which implies
> re-synchronization.
>
> This fixes retransmission of xmodem packets and allows kwboot to work also
> without "Waiting ... and flushing tty" code which is at the beginning of
> kwboot xmodem transfer.
>
> Signed-off-by: Pali Rohár <pali@kernel.org>
> Signed-off-by: Marek Behún <marek.behun@nic.cz>

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

Thanks,
Stefan

> ---
> tools/kwboot.c | 11 ++++++++++-
> 1 file changed, 10 insertions(+), 1 deletion(-)
>
> diff --git a/tools/kwboot.c b/tools/kwboot.c
> index 1477c0f078..be9a751406 100644
> --- a/tools/kwboot.c
> +++ b/tools/kwboot.c
> @@ -880,6 +880,7 @@ kwboot_baud_magic_handle(int fd, char c, int baudrate)
>
> static int
> kwboot_xm_recv_reply(int fd, char *c, int nak_on_non_xm,
> + int ignore_nak_reply,
> int allow_non_xm, int *non_xm_print,
> int baudrate, int *baud_changed)
> {
> @@ -899,8 +900,14 @@ kwboot_xm_recv_reply(int fd, char *c, int nak_on_non_xm,
> }
>
> /* If received xmodem reply, end. */
> - if (_is_xm_reply(*c))
> + if (_is_xm_reply(*c)) {
> + if (*c == NAK && ignore_nak_reply) {
> + timeout = recv_until - _now();
> + if (timeout >= 0)
> + continue;
> + }
> break;
> + }
>
> /*
> * If receiving/printing non-xmodem text output is allowed and
> @@ -968,6 +975,7 @@ kwboot_xm_sendblock(int fd, struct kwboot_block *block, int allow_non_xm,
> }
>
> rc = kwboot_xm_recv_reply(fd, &c, retries < 3,
> + retries > 8,
> allow_non_xm, &non_xm_print,
> baudrate, &baud_changed);
> if (rc)
> @@ -1011,6 +1019,7 @@ kwboot_xm_finish(int fd)
> return rc;
>
> rc = kwboot_xm_recv_reply(fd, &c, retries < 3,
> + retries > 8,
> 0, NULL, 0, NULL);
> if (rc)
> return rc;

Viele Grüße,
Stefan Roese

--
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