[PATCH u-boot-marvell 06/14] tools: kwboot: Fix handling of repeated xmodem packets

[Stefan Roese]

On 1/25/22 18:13, Marek Behún wrote:
> From: Pali Rohár <pali at kernel.org>
> 
> Unfortunately during some stages of xmodem transfer, A385 BootROM is not
> able to handle repeated xmodem packets. So if an error occurs during that
> stage, stop the transfer and return failure.
> 
> Signed-off-by: Pali Rohár <pali at kernel.org>
> Signed-off-by: Marek Behún <marek.behun at nic.cz>

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

Thanks,
Stefan

> ---
>   tools/kwboot.c | 28 +++++++++++++++++++++++++---
>   1 file changed, 25 insertions(+), 3 deletions(-)
> 
> diff --git a/tools/kwboot.c b/tools/kwboot.c
> index a619a6c3c1..dfac8aed7b 100644
> --- a/tools/kwboot.c
> +++ b/tools/kwboot.c
> @@ -946,7 +946,7 @@ kwboot_xm_recv_reply(int fd, char *c, int stop_on_non_xm,
>   
>   static int
>   kwboot_xm_sendblock(int fd, struct kwboot_block *block, int allow_non_xm,
> -		    int *done_print, int baudrate)
> +		    int *done_print, int baudrate, int allow_retries)
>   {
>   	int non_xm_print, baud_changed;
>   	int rc, err, retries;
> @@ -977,7 +977,7 @@ kwboot_xm_sendblock(int fd, struct kwboot_block *block, int allow_non_xm,
>   
>   		if (!allow_non_xm && c != ACK)
>   			kwboot_progress(-1, '+');
> -	} while (c == NAK && retries++ < 16);
> +	} while (c == NAK && allow_retries && retries++ < 16);
>   
>   	if (non_xm_print)
>   		kwboot_printv("\n");
> @@ -1044,8 +1044,30 @@ kwboot_xmodem_one(int tty, int *pnum, int header, const uint8_t *data,
>   
>   		last_block = (left <= blksz);
>   
> +		/*
> +		 * Handling of repeated xmodem packets is completely broken in
> +		 * Armada 385 BootROM - it completely ignores xmodem packet
> +		 * numbers, they are only used for checksum verification.
> +		 * BootROM can handle a retry of the xmodem packet only during
> +		 * the transmission of kwbimage header and only if BootROM
> +		 * itself sent NAK response to previous attempt (it does it on
> +		 * checksum failure). During the transmission of kwbimage data
> +		 * part, BootROM always expects next xmodem packet, even if it
> +		 * sent NAK to previous attempt - there is absolutely no way to
> +		 * repair incorrectly transmitted xmodem packet during kwbimage
> +		 * data part upload. Also, if kwboot receives non-ACK/NAK
> +		 * response (meaning that original BootROM response was damaged
> +		 * on UART) there is no way to detect if BootROM accepted xmodem
> +		 * packet or not and no way to check if kwboot could repeat the
> +		 * packet or not.
> +		 *
> +		 * Stop transfer and return failure if kwboot receives unknown
> +		 * reply if non-xmodem reply is not allowed (for all xmodem
> +		 * packets except the last header packet) or when non-ACK reply
> +		 * is received during data part transfer.
> +		 */
>   		rc = kwboot_xm_sendblock(tty, &block, header && last_block,
> -					 &done_print, baudrate);
> +					 &done_print, baudrate, header);
>   		if (rc)
>   			goto out;
>   

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 at denx.de