[PATCH u-boot-marvell 00/14] Another set of kwboot improvements

[PATCH u-boot-marvell 00/14] Another set of kwboot improvements

[Marek Behún]

From: Marek Behún <marek.behun@nic.cz>

Hello Stefan,

here comes another set of kwboot improvements from Pali, reviewed and
signed off by myself.

Marek

Pali Rohár (14):
  tools: kwboot: Increase blk_rsp_timeo to 2s
  tools: kwboot: Wait blk_rsp_timeo when flushing
  tools: kwboot: Improve retrying logic for incomplete xmodem packets
  tools: kwboot: Remove code for handling CAN byte
  tools: kwboot: Do not change received character in
    kwboot_xm_recv_reply()
  tools: kwboot: Fix handling of repeated xmodem packets
  tools: kwboot: Show 'E' in progress output when error occurs
  tools: kwboot: Allow to use option -b without image path
  tools: kwboot: Force BootROM to flush input queue after boot pattern
  tools: kwboot: Remove 2s delay before sending first xmodem packet
  tools: kwboot: Handle EINTR in kwboot_write()
  tools: kwboot: Handle EINTR in kwboot_tty_recv()
  tools: kwboot: Fix usage of -D without -t
  tools: kwboot: Set debug flag to 1

 tools/kwboot.c | 154 ++++++++++++++++++++++++++++++++++++-------------
 1 file changed, 115 insertions(+), 39 deletions(-)

-- 
2.34.1

[PATCH u-boot-marvell 01/14] tools: kwboot: Increase blk_rsp_timeo to 2s

[Marek Behún]

From: Pali Rohár <pali@kernel.org>

Fix xmodem retry mechanism if some bytes from xmodem packet were lost and
BootROM is still waiting for completing previous xmodem packet.

It is required to wait at least 1.312s on A385, otherwise BootROM does not
accept next xmodem packet if previous one was not completely transferred.

2s should be enough timeout cause that BootROM will drop incomplete xmodem
packet and expects new packet.

Signed-off-by: Pali Rohár <pali@kernel.org>
Signed-off-by: Marek Behún <marek.behun@nic.cz>
---
 tools/kwboot.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/tools/kwboot.c b/tools/kwboot.c
index c3d8ab6544..82cfd9a827 100644
--- a/tools/kwboot.c
+++ b/tools/kwboot.c
@@ -75,7 +75,7 @@ struct kwboot_block {
 	uint8_t csum;
 } __packed;
 
-#define KWBOOT_BLK_RSP_TIMEO 1000 /* ms */
+#define KWBOOT_BLK_RSP_TIMEO 2000 /* ms */
 #define KWBOOT_HDR_RSP_TIMEO 10000 /* ms */
 
 /* ARM code to change baudrate */
-- 
2.34.1

Re: [PATCH u-boot-marvell 01/14] tools: kwboot: Increase blk_rsp_timeo to 2s

[Stefan Roese]

On 1/25/22 18:13, Marek Behún wrote:
> From: Pali Rohár <pali@kernel.org>
> 
> Fix xmodem retry mechanism if some bytes from xmodem packet were lost and
> BootROM is still waiting for completing previous xmodem packet.
> 
> It is required to wait at least 1.312s on A385, otherwise BootROM does not
> accept next xmodem packet if previous one was not completely transferred.
> 
> 2s should be enough timeout cause that BootROM will drop incomplete xmodem
> packet and expects new packet.
> 
> 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 | 2 +-
>   1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/tools/kwboot.c b/tools/kwboot.c
> index c3d8ab6544..82cfd9a827 100644
> --- a/tools/kwboot.c
> +++ b/tools/kwboot.c
> @@ -75,7 +75,7 @@ struct kwboot_block {
>   	uint8_t csum;
>   } __packed;
>   
> -#define KWBOOT_BLK_RSP_TIMEO 1000 /* ms */
> +#define KWBOOT_BLK_RSP_TIMEO 2000 /* ms */
>   #define KWBOOT_HDR_RSP_TIMEO 10000 /* ms */
>   
>   /* ARM code to change baudrate */

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

[PATCH u-boot-marvell 02/14] tools: kwboot: Wait blk_rsp_timeo when flushing

[Marek Behún]

From: Pali Rohár <pali@kernel.org>

Use the blk_rsp_timeo variable when sleeping before flushing tty.

Signed-off-by: Pali Rohár <pali@kernel.org>
Signed-off-by: Marek Behún <marek.behun@nic.cz>
---
 tools/kwboot.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/tools/kwboot.c b/tools/kwboot.c
index 82cfd9a827..1477c0f078 100644
--- a/tools/kwboot.c
+++ b/tools/kwboot.c
@@ -1081,8 +1081,8 @@ kwboot_xmodem(int tty, const void *_img, size_t size, int baudrate)
 	 */
 	hdrsz += (KWBOOT_XM_BLKSZ - hdrsz % KWBOOT_XM_BLKSZ) % KWBOOT_XM_BLKSZ;
 
-	kwboot_printv("Waiting 2s and flushing tty\n");
-	sleep(2); /* flush isn't effective without it */
+	kwboot_printv("Waiting %d ms and flushing tty\n", blk_rsp_timeo);
+	usleep(blk_rsp_timeo * 1000);
 	tcflush(tty, TCIOFLUSH);
 
 	pnum = 1;
-- 
2.34.1

Re: [PATCH u-boot-marvell 02/14] tools: kwboot: Wait blk_rsp_timeo when flushing

[Stefan Roese]

On 1/25/22 18:13, Marek Behún wrote:
> From: Pali Rohár <pali@kernel.org>
> 
> Use the blk_rsp_timeo variable when sleeping before flushing tty.
> 
> 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 | 4 ++--
>   1 file changed, 2 insertions(+), 2 deletions(-)
> 
> diff --git a/tools/kwboot.c b/tools/kwboot.c
> index 82cfd9a827..1477c0f078 100644
> --- a/tools/kwboot.c
> +++ b/tools/kwboot.c
> @@ -1081,8 +1081,8 @@ kwboot_xmodem(int tty, const void *_img, size_t size, int baudrate)
>   	 */
>   	hdrsz += (KWBOOT_XM_BLKSZ - hdrsz % KWBOOT_XM_BLKSZ) % KWBOOT_XM_BLKSZ;
>   
> -	kwboot_printv("Waiting 2s and flushing tty\n");
> -	sleep(2); /* flush isn't effective without it */
> +	kwboot_printv("Waiting %d ms and flushing tty\n", blk_rsp_timeo);
> +	usleep(blk_rsp_timeo * 1000);
>   	tcflush(tty, TCIOFLUSH);
>   
>   	pnum = 1;

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

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

[Marek Behún]

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

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

[Stefan Roese]

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

[PATCH u-boot-marvell 04/14] tools: kwboot: Remove code for handling CAN byte

[Marek Behún]

From: Pali Rohár <pali@kernel.org>

It is unknown why handling of CAN byte was added into kwboot tool as
Marvell BootROM does not support CAN byte. It never sends CAN byte to host
and if host sends CAN byte BootROM handles it as an unknown byte.

Remove code for handling and sending CAN bytes from the kwboot tool.

Signed-off-by: Pali Rohár <pali@kernel.org>
Signed-off-by: Marek Behún <marek.behun@nic.cz>
---
 tools/kwboot.c | 16 +++++-----------
 1 file changed, 5 insertions(+), 11 deletions(-)

diff --git a/tools/kwboot.c b/tools/kwboot.c
index be9a751406..0b97990d09 100644
--- a/tools/kwboot.c
+++ b/tools/kwboot.c
@@ -63,7 +63,6 @@ static unsigned char kwboot_msg_debug[] = {
 #define EOT	4	/* sender end of block transfer */
 #define ACK	6	/* target block ack */
 #define NAK	21	/* target block negative ack */
-#define CAN	24	/* target/sender transfer cancellation */
 
 #define KWBOOT_XM_BLKSZ	128 /* xmodem block size */
 
@@ -826,7 +825,7 @@ _now(void)
 static int
 _is_xm_reply(char c)
 {
-	return c == ACK || c == NAK || c == CAN;
+	return c == ACK || c == NAK;
 }
 
 static int
@@ -841,9 +840,6 @@ _xm_reply_to_error(int c)
 	case NAK:
 		errno = EBADMSG;
 		break;
-	case CAN:
-		errno = ECANCELED;
-		break;
 	default:
 		errno = EPROTO;
 		break;
@@ -966,7 +962,7 @@ kwboot_xm_sendblock(int fd, struct kwboot_block *block, int allow_non_xm,
 	do {
 		rc = kwboot_tty_send(fd, block, sizeof(*block), 1);
 		if (rc)
-			return rc;
+			goto err;
 
 		if (allow_non_xm && !*done_print) {
 			kwboot_progress(100, '.');
@@ -979,7 +975,7 @@ kwboot_xm_sendblock(int fd, struct kwboot_block *block, int allow_non_xm,
 					  allow_non_xm, &non_xm_print,
 					  baudrate, &baud_changed);
 		if (rc)
-			goto can;
+			goto err;
 
 		if (!allow_non_xm && c != ACK)
 			kwboot_progress(-1, '+');
@@ -990,15 +986,13 @@ kwboot_xm_sendblock(int fd, struct kwboot_block *block, int allow_non_xm,
 
 	if (allow_non_xm && baudrate && !baud_changed) {
 		fprintf(stderr, "Baudrate was not changed\n");
-		rc = -1;
 		errno = EPROTO;
-		goto can;
+		return -1;
 	}
 
 	return _xm_reply_to_error(c);
-can:
+err:
 	err = errno;
-	kwboot_tty_send_char(fd, CAN);
 	kwboot_printv("\n");
 	errno = err;
 	return rc;
-- 
2.34.1

Re: [PATCH u-boot-marvell 04/14] tools: kwboot: Remove code for handling CAN byte

[Stefan Roese]

On 1/25/22 18:13, Marek Behún wrote:
> From: Pali Rohár <pali@kernel.org>
> 
> It is unknown why handling of CAN byte was added into kwboot tool as
> Marvell BootROM does not support CAN byte. It never sends CAN byte to host
> and if host sends CAN byte BootROM handles it as an unknown byte.
> 
> Remove code for handling and sending CAN bytes from the kwboot tool.
> 
> 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 | 16 +++++-----------
>   1 file changed, 5 insertions(+), 11 deletions(-)
> 
> diff --git a/tools/kwboot.c b/tools/kwboot.c
> index be9a751406..0b97990d09 100644
> --- a/tools/kwboot.c
> +++ b/tools/kwboot.c
> @@ -63,7 +63,6 @@ static unsigned char kwboot_msg_debug[] = {
>   #define EOT	4	/* sender end of block transfer */
>   #define ACK	6	/* target block ack */
>   #define NAK	21	/* target block negative ack */
> -#define CAN	24	/* target/sender transfer cancellation */
>   
>   #define KWBOOT_XM_BLKSZ	128 /* xmodem block size */
>   
> @@ -826,7 +825,7 @@ _now(void)
>   static int
>   _is_xm_reply(char c)
>   {
> -	return c == ACK || c == NAK || c == CAN;
> +	return c == ACK || c == NAK;
>   }
>   
>   static int
> @@ -841,9 +840,6 @@ _xm_reply_to_error(int c)
>   	case NAK:
>   		errno = EBADMSG;
>   		break;
> -	case CAN:
> -		errno = ECANCELED;
> -		break;
>   	default:
>   		errno = EPROTO;
>   		break;
> @@ -966,7 +962,7 @@ kwboot_xm_sendblock(int fd, struct kwboot_block *block, int allow_non_xm,
>   	do {
>   		rc = kwboot_tty_send(fd, block, sizeof(*block), 1);
>   		if (rc)
> -			return rc;
> +			goto err;
>   
>   		if (allow_non_xm && !*done_print) {
>   			kwboot_progress(100, '.');
> @@ -979,7 +975,7 @@ kwboot_xm_sendblock(int fd, struct kwboot_block *block, int allow_non_xm,
>   					  allow_non_xm, &non_xm_print,
>   					  baudrate, &baud_changed);
>   		if (rc)
> -			goto can;
> +			goto err;
>   
>   		if (!allow_non_xm && c != ACK)
>   			kwboot_progress(-1, '+');
> @@ -990,15 +986,13 @@ kwboot_xm_sendblock(int fd, struct kwboot_block *block, int allow_non_xm,
>   
>   	if (allow_non_xm && baudrate && !baud_changed) {
>   		fprintf(stderr, "Baudrate was not changed\n");
> -		rc = -1;
>   		errno = EPROTO;
> -		goto can;
> +		return -1;
>   	}
>   
>   	return _xm_reply_to_error(c);
> -can:
> +err:
>   	err = errno;
> -	kwboot_tty_send_char(fd, CAN);
>   	kwboot_printv("\n");
>   	errno = err;
>   	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

[PATCH u-boot-marvell 05/14] tools: kwboot: Do not change received character in kwboot_xm_recv_reply()

[Marek Behún]

From: Pali Rohár <pali@kernel.org>

Marvell BootROM expects retransmission of previous xmodem packet only in
the case when it sends NAK response to the host.

Do not change non-xmodem response (possibly UART transfer error) to NAK
in kwboot_xm_recv_reply() function. Allow caller to receive original
response from device.

Change argument 'nak_on_non_xm' to 'stop_on_non_xm'. Instead of changing
non-xmodem character to NAK, stop processing on invalid character and
return it.

Signed-off-by: Pali Rohár <pali@kernel.org>
Signed-off-by: Marek Behún <marek.behun@nic.cz>
---
 tools/kwboot.c | 6 ++----
 1 file changed, 2 insertions(+), 4 deletions(-)

diff --git a/tools/kwboot.c b/tools/kwboot.c
index 0b97990d09..a619a6c3c1 100644
--- a/tools/kwboot.c
+++ b/tools/kwboot.c
@@ -875,7 +875,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,
+kwboot_xm_recv_reply(int fd, char *c, int stop_on_non_xm,
 		     int ignore_nak_reply,
 		     int allow_non_xm, int *non_xm_print,
 		     int baudrate, int *baud_changed)
@@ -931,10 +931,8 @@ kwboot_xm_recv_reply(int fd, char *c, int nak_on_non_xm,
 				*non_xm_print = 1;
 			}
 		} else {
-			if (nak_on_non_xm) {
-				*c = NAK;
+			if (stop_on_non_xm)
 				break;
-			}
 			timeout = recv_until - _now();
 			if (timeout < 0) {
 				errno = ETIMEDOUT;
-- 
2.34.1

Re: [PATCH u-boot-marvell 05/14] tools: kwboot: Do not change received character in kwboot_xm_recv_reply()

[Stefan Roese]

On 1/25/22 18:13, Marek Behún wrote:
> From: Pali Rohár <pali@kernel.org>
> 
> Marvell BootROM expects retransmission of previous xmodem packet only in
> the case when it sends NAK response to the host.
> 
> Do not change non-xmodem response (possibly UART transfer error) to NAK
> in kwboot_xm_recv_reply() function. Allow caller to receive original
> response from device.
> 
> Change argument 'nak_on_non_xm' to 'stop_on_non_xm'. Instead of changing
> non-xmodem character to NAK, stop processing on invalid character and
> return it.
> 
> 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 | 6 ++----
>   1 file changed, 2 insertions(+), 4 deletions(-)
> 
> diff --git a/tools/kwboot.c b/tools/kwboot.c
> index 0b97990d09..a619a6c3c1 100644
> --- a/tools/kwboot.c
> +++ b/tools/kwboot.c
> @@ -875,7 +875,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,
> +kwboot_xm_recv_reply(int fd, char *c, int stop_on_non_xm,
>   		     int ignore_nak_reply,
>   		     int allow_non_xm, int *non_xm_print,
>   		     int baudrate, int *baud_changed)
> @@ -931,10 +931,8 @@ kwboot_xm_recv_reply(int fd, char *c, int nak_on_non_xm,
>   				*non_xm_print = 1;
>   			}
>   		} else {
> -			if (nak_on_non_xm) {
> -				*c = NAK;
> +			if (stop_on_non_xm)
>   				break;
> -			}
>   			timeout = recv_until - _now();
>   			if (timeout < 0) {
>   				errno = ETIMEDOUT;

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

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

[Marek Behún]

From: Pali Rohár <pali@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@kernel.org>
Signed-off-by: Marek Behún <marek.behun@nic.cz>
---
 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;
 
-- 
2.34.1

Re: [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@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@kernel.org>
> Signed-off-by: Marek Behún <marek.behun@nic.cz>

Reviewed-by: Stefan Roese <sr@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@denx.de

[PATCH u-boot-marvell 07/14] tools: kwboot: Show 'E' in progress output when error occurs

[Marek Behún]

From: Pali Rohár <pali@kernel.org>

When kwboot is unable to resend current xmodem packet, show an 'E' in the
progress output instead of a '+'. This allows to distinguish between the
state when kwboot is retrying sending the packet and when retry is not
possible.

Signed-off-by: Pali Rohár <pali@kernel.org>
Signed-off-by: Marek Behún <marek.behun@nic.cz>
---
 tools/kwboot.c | 8 ++++++--
 1 file changed, 6 insertions(+), 2 deletions(-)

diff --git a/tools/kwboot.c b/tools/kwboot.c
index dfac8aed7b..1dcec1969a 100644
--- a/tools/kwboot.c
+++ b/tools/kwboot.c
@@ -975,8 +975,12 @@ kwboot_xm_sendblock(int fd, struct kwboot_block *block, int allow_non_xm,
 		if (rc)
 			goto err;
 
-		if (!allow_non_xm && c != ACK)
-			kwboot_progress(-1, '+');
+		if (!allow_non_xm && c != ACK) {
+			if (c == NAK && allow_retries && retries + 1 < 16)
+				kwboot_progress(-1, '+');
+			else
+				kwboot_progress(-1, 'E');
+		}
 	} while (c == NAK && allow_retries && retries++ < 16);
 
 	if (non_xm_print)
-- 
2.34.1

Re: [PATCH u-boot-marvell 07/14] tools: kwboot: Show 'E' in progress output when error occurs

[Stefan Roese]

On 1/25/22 18:13, Marek Behún wrote:
> From: Pali Rohár <pali@kernel.org>
> 
> When kwboot is unable to resend current xmodem packet, show an 'E' in the
> progress output instead of a '+'. This allows to distinguish between the
> state when kwboot is retrying sending the packet and when retry is not
> possible.
> 
> 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 | 8 ++++++--
>   1 file changed, 6 insertions(+), 2 deletions(-)
> 
> diff --git a/tools/kwboot.c b/tools/kwboot.c
> index dfac8aed7b..1dcec1969a 100644
> --- a/tools/kwboot.c
> +++ b/tools/kwboot.c
> @@ -975,8 +975,12 @@ kwboot_xm_sendblock(int fd, struct kwboot_block *block, int allow_non_xm,
>   		if (rc)
>   			goto err;
>   
> -		if (!allow_non_xm && c != ACK)
> -			kwboot_progress(-1, '+');
> +		if (!allow_non_xm && c != ACK) {
> +			if (c == NAK && allow_retries && retries + 1 < 16)
> +				kwboot_progress(-1, '+');
> +			else
> +				kwboot_progress(-1, 'E');
> +		}
>   	} while (c == NAK && allow_retries && retries++ < 16);
>   
>   	if (non_xm_print)

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

[PATCH u-boot-marvell 08/14] tools: kwboot: Allow to use option -b without image path

[Marek Behún]

From: Pali Rohár <pali@kernel.org>

Allow option -b without image path parameter, to send boot pattern and
wait for response but not send any image. This allows to use kwboot just
for processing boot pattern and user can use any other xmodem tool for
transferring the image itself (e.g. sx). Useful for debugging purposes.

Signed-off-by: Pali Rohár <pali@kernel.org>
Signed-off-by: Marek Behún <marek.behun@nic.cz>
---
 tools/kwboot.c | 22 +++++++++++++++++-----
 1 file changed, 17 insertions(+), 5 deletions(-)

diff --git a/tools/kwboot.c b/tools/kwboot.c
index 1dcec1969a..c413a8bf51 100644
--- a/tools/kwboot.c
+++ b/tools/kwboot.c
@@ -1699,6 +1699,8 @@ main(int argc, char **argv)
 	size_t size;
 	size_t after_img_rsv;
 	int baudrate;
+	int prev_optind;
+	int c;
 
 	rv = 1;
 	tty = -1;
@@ -1716,22 +1718,32 @@ main(int argc, char **argv)
 	kwboot_verbose = isatty(STDOUT_FILENO);
 
 	do {
-		int c = getopt(argc, argv, "hb:ptaB:dD:q:s:o:");
+		prev_optind = optind;
+		c = getopt(argc, argv, "hbptaB:dD:q:s:o:");
 		if (c < 0)
 			break;
 
 		switch (c) {
 		case 'b':
+			if (imgpath || bootmsg || debugmsg)
+				goto usage;
 			bootmsg = kwboot_msg_boot;
-			imgpath = optarg;
+			if (prev_optind == optind)
+				goto usage;
+			if (argv[optind] && argv[optind][0] != '-')
+				imgpath = argv[optind++];
 			break;
 
 		case 'D':
+			if (imgpath || bootmsg || debugmsg)
+				goto usage;
 			bootmsg = NULL;
 			imgpath = optarg;
 			break;
 
 		case 'd':
+			if (imgpath || bootmsg || debugmsg)
+				goto usage;
 			debugmsg = kwboot_msg_debug;
 			break;
 
@@ -1774,11 +1786,11 @@ main(int argc, char **argv)
 	if (!bootmsg && !term && !debugmsg)
 		goto usage;
 
-	if (argc - optind < 1)
-		goto usage;
-
 	ttypath = argv[optind++];
 
+	if (optind != argc)
+		goto usage;
+
 	tty = kwboot_open_tty(ttypath, imgpath ? 115200 : baudrate);
 	if (tty < 0) {
 		perror(ttypath);
-- 
2.34.1

Re: [PATCH u-boot-marvell 08/14] tools: kwboot: Allow to use option -b without image path

[Stefan Roese]

On 1/25/22 18:13, Marek Behún wrote:
> From: Pali Rohár <pali@kernel.org>
> 
> Allow option -b without image path parameter, to send boot pattern and
> wait for response but not send any image. This allows to use kwboot just
> for processing boot pattern and user can use any other xmodem tool for
> transferring the image itself (e.g. sx). Useful for debugging purposes.
> 
> 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 | 22 +++++++++++++++++-----
>   1 file changed, 17 insertions(+), 5 deletions(-)
> 
> diff --git a/tools/kwboot.c b/tools/kwboot.c
> index 1dcec1969a..c413a8bf51 100644
> --- a/tools/kwboot.c
> +++ b/tools/kwboot.c
> @@ -1699,6 +1699,8 @@ main(int argc, char **argv)
>   	size_t size;
>   	size_t after_img_rsv;
>   	int baudrate;
> +	int prev_optind;
> +	int c;
>   
>   	rv = 1;
>   	tty = -1;
> @@ -1716,22 +1718,32 @@ main(int argc, char **argv)
>   	kwboot_verbose = isatty(STDOUT_FILENO);
>   
>   	do {
> -		int c = getopt(argc, argv, "hb:ptaB:dD:q:s:o:");
> +		prev_optind = optind;
> +		c = getopt(argc, argv, "hbptaB:dD:q:s:o:");
>   		if (c < 0)
>   			break;
>   
>   		switch (c) {
>   		case 'b':
> +			if (imgpath || bootmsg || debugmsg)
> +				goto usage;
>   			bootmsg = kwboot_msg_boot;
> -			imgpath = optarg;
> +			if (prev_optind == optind)
> +				goto usage;
> +			if (argv[optind] && argv[optind][0] != '-')
> +				imgpath = argv[optind++];
>   			break;
>   
>   		case 'D':
> +			if (imgpath || bootmsg || debugmsg)
> +				goto usage;
>   			bootmsg = NULL;
>   			imgpath = optarg;
>   			break;
>   
>   		case 'd':
> +			if (imgpath || bootmsg || debugmsg)
> +				goto usage;
>   			debugmsg = kwboot_msg_debug;
>   			break;
>   
> @@ -1774,11 +1786,11 @@ main(int argc, char **argv)
>   	if (!bootmsg && !term && !debugmsg)
>   		goto usage;
>   
> -	if (argc - optind < 1)
> -		goto usage;
> -
>   	ttypath = argv[optind++];
>   
> +	if (optind != argc)
> +		goto usage;
> +
>   	tty = kwboot_open_tty(ttypath, imgpath ? 115200 : baudrate);
>   	if (tty < 0) {
>   		perror(ttypath);

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

[PATCH u-boot-marvell 09/14] tools: kwboot: Force BootROM to flush input queue after boot pattern

[Marek Behún]

From: Pali Rohár <pali@kernel.org>

Force the BootROM to flush its input queue after sending boot pattern.

This ensures that after function kwboot_bootmsg() finishes, BootROM is
able to start receiving xmodem packets without any specific delay or
setup.

Signed-off-by: Pali Rohár <pali@kernel.org>
Signed-off-by: Marek Behún <marek.behun@nic.cz>
---
 tools/kwboot.c | 36 +++++++++++++++++++++++++++++++++++-
 1 file changed, 35 insertions(+), 1 deletion(-)

diff --git a/tools/kwboot.c b/tools/kwboot.c
index c413a8bf51..824ae005b2 100644
--- a/tools/kwboot.c
+++ b/tools/kwboot.c
@@ -717,6 +717,7 @@ out:
 static int
 kwboot_bootmsg(int tty, void *msg)
 {
+	struct kwboot_block block;
 	int rc;
 	char c;
 	int count;
@@ -747,7 +748,40 @@ kwboot_bootmsg(int tty, void *msg)
 
 	kwboot_printv("\n");
 
-	return rc;
+	if (rc)
+		return rc;
+
+	/*
+	 * At this stage we have sent more boot message patterns and BootROM
+	 * (at least on Armada XP and 385) started interpreting sent bytes as
+	 * part of xmodem packets. If BootROM is expecting SOH byte as start of
+	 * a xmodem packet and it receives byte 0xff, then it throws it away and
+	 * sends a NAK reply to host. If BootROM does not receive any byte for
+	 * 2s when expecting some continuation of the xmodem packet, it throws
+	 * away the partially received xmodem data and sends NAK reply to host.
+	 *
+	 * Therefore for starting xmodem transfer we have two options: Either
+	 * wait 2s or send 132 0xff bytes (which is the size of xmodem packet)
+	 * to ensure that BootROM throws away any partially received data.
+	 */
+
+	/* flush output queue with remaining boot message patterns */
+	tcflush(tty, TCOFLUSH);
+
+	/* send one xmodem packet with 0xff bytes to force BootROM to re-sync */
+	memset(&block, 0xff, sizeof(block));
+	kwboot_tty_send(tty, &block, sizeof(block), 0);
+
+	/*
+	 * Sending 132 bytes via 115200B/8-N-1 takes 11.45 ms, reading 132 bytes
+	 * takes 11.45 ms, so waiting for 30 ms should be enough.
+	 */
+	usleep(30 * 1000);
+
+	/* flush remaining NAK replies from input queue */
+	tcflush(tty, TCIFLUSH);
+
+	return 0;
 }
 
 static int
-- 
2.34.1

Re: [PATCH u-boot-marvell 09/14] tools: kwboot: Force BootROM to flush input queue after boot pattern

[Stefan Roese]

On 1/25/22 18:13, Marek Behún wrote:
> From: Pali Rohár <pali@kernel.org>
> 
> Force the BootROM to flush its input queue after sending boot pattern.
> 
> This ensures that after function kwboot_bootmsg() finishes, BootROM is
> able to start receiving xmodem packets without any specific delay or
> setup.
> 
> 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 | 36 +++++++++++++++++++++++++++++++++++-
>   1 file changed, 35 insertions(+), 1 deletion(-)
> 
> diff --git a/tools/kwboot.c b/tools/kwboot.c
> index c413a8bf51..824ae005b2 100644
> --- a/tools/kwboot.c
> +++ b/tools/kwboot.c
> @@ -717,6 +717,7 @@ out:
>   static int
>   kwboot_bootmsg(int tty, void *msg)
>   {
> +	struct kwboot_block block;
>   	int rc;
>   	char c;
>   	int count;
> @@ -747,7 +748,40 @@ kwboot_bootmsg(int tty, void *msg)
>   
>   	kwboot_printv("\n");
>   
> -	return rc;
> +	if (rc)
> +		return rc;
> +
> +	/*
> +	 * At this stage we have sent more boot message patterns and BootROM
> +	 * (at least on Armada XP and 385) started interpreting sent bytes as
> +	 * part of xmodem packets. If BootROM is expecting SOH byte as start of
> +	 * a xmodem packet and it receives byte 0xff, then it throws it away and
> +	 * sends a NAK reply to host. If BootROM does not receive any byte for
> +	 * 2s when expecting some continuation of the xmodem packet, it throws
> +	 * away the partially received xmodem data and sends NAK reply to host.
> +	 *
> +	 * Therefore for starting xmodem transfer we have two options: Either
> +	 * wait 2s or send 132 0xff bytes (which is the size of xmodem packet)
> +	 * to ensure that BootROM throws away any partially received data.
> +	 */
> +
> +	/* flush output queue with remaining boot message patterns */
> +	tcflush(tty, TCOFLUSH);
> +
> +	/* send one xmodem packet with 0xff bytes to force BootROM to re-sync */
> +	memset(&block, 0xff, sizeof(block));
> +	kwboot_tty_send(tty, &block, sizeof(block), 0);
> +
> +	/*
> +	 * Sending 132 bytes via 115200B/8-N-1 takes 11.45 ms, reading 132 bytes
> +	 * takes 11.45 ms, so waiting for 30 ms should be enough.
> +	 */
> +	usleep(30 * 1000);
> +
> +	/* flush remaining NAK replies from input queue */
> +	tcflush(tty, TCIFLUSH);
> +
> +	return 0;
>   }
>   
>   static int

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

[PATCH u-boot-marvell 10/14] tools: kwboot: Remove 2s delay before sending first xmodem packet

[Marek Behún]

From: Pali Rohár <pali@kernel.org>

This delay is not needed anymore since kwboot already handles retrying
logic for incomplete xmodem packets and also forces BootROM to flush its
input queue. Removing it decreases total transfer time.

Signed-off-by: Pali Rohár <pali@kernel.org>
Signed-off-by: Marek Behún <marek.behun@nic.cz>
---
 tools/kwboot.c | 4 ----
 1 file changed, 4 deletions(-)

diff --git a/tools/kwboot.c b/tools/kwboot.c
index 824ae005b2..de433c1b04 100644
--- a/tools/kwboot.c
+++ b/tools/kwboot.c
@@ -1142,10 +1142,6 @@ kwboot_xmodem(int tty, const void *_img, size_t size, int baudrate)
 	 */
 	hdrsz += (KWBOOT_XM_BLKSZ - hdrsz % KWBOOT_XM_BLKSZ) % KWBOOT_XM_BLKSZ;
 
-	kwboot_printv("Waiting %d ms and flushing tty\n", blk_rsp_timeo);
-	usleep(blk_rsp_timeo * 1000);
-	tcflush(tty, TCIOFLUSH);
-
 	pnum = 1;
 
 	rc = kwboot_xmodem_one(tty, &pnum, 1, img, hdrsz, baudrate);
-- 
2.34.1

Re: [PATCH u-boot-marvell 10/14] tools: kwboot: Remove 2s delay before sending first xmodem packet

[Stefan Roese]

On 1/25/22 18:13, Marek Behún wrote:
> From: Pali Rohár <pali@kernel.org>
> 
> This delay is not needed anymore since kwboot already handles retrying
> logic for incomplete xmodem packets and also forces BootROM to flush its
> input queue. Removing it decreases total transfer time.
> 
> 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 | 4 ----
>   1 file changed, 4 deletions(-)
> 
> diff --git a/tools/kwboot.c b/tools/kwboot.c
> index 824ae005b2..de433c1b04 100644
> --- a/tools/kwboot.c
> +++ b/tools/kwboot.c
> @@ -1142,10 +1142,6 @@ kwboot_xmodem(int tty, const void *_img, size_t size, int baudrate)
>   	 */
>   	hdrsz += (KWBOOT_XM_BLKSZ - hdrsz % KWBOOT_XM_BLKSZ) % KWBOOT_XM_BLKSZ;
>   
> -	kwboot_printv("Waiting %d ms and flushing tty\n", blk_rsp_timeo);
> -	usleep(blk_rsp_timeo * 1000);
> -	tcflush(tty, TCIOFLUSH);
> -
>   	pnum = 1;
>   
>   	rc = kwboot_xmodem_one(tty, &pnum, 1, img, hdrsz, baudrate);

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

[PATCH u-boot-marvell 11/14] tools: kwboot: Handle EINTR in kwboot_write()

[Marek Behún]

From: Pali Rohár <pali@kernel.org>

The write() syscall may be interrupted. Handle EINTR and retry it.

Signed-off-by: Pali Rohár <pali@kernel.org>
Signed-off-by: Marek Behún <marek.behun@nic.cz>
---
 tools/kwboot.c | 8 +++++---
 1 file changed, 5 insertions(+), 3 deletions(-)

diff --git a/tools/kwboot.c b/tools/kwboot.c
index de433c1b04..8b748f0fdd 100644
--- a/tools/kwboot.c
+++ b/tools/kwboot.c
@@ -292,13 +292,15 @@ static int blk_rsp_timeo = KWBOOT_BLK_RSP_TIMEO;
 static ssize_t
 kwboot_write(int fd, const char *buf, size_t len)
 {
-	size_t tot = 0;
+	ssize_t tot = 0;
 
 	while (tot < len) {
 		ssize_t wr = write(fd, buf + tot, len - tot);
 
-		if (wr < 0)
-			return -1;
+		if (wr < 0 && errno == EINTR)
+			continue;
+		else if (wr < 0)
+			return wr;
 
 		tot += wr;
 	}
-- 
2.34.1

Re: [PATCH u-boot-marvell 11/14] tools: kwboot: Handle EINTR in kwboot_write()

[Stefan Roese]

On 1/25/22 18:13, Marek Behún wrote:
> From: Pali Rohár <pali@kernel.org>
> 
> The write() syscall may be interrupted. Handle EINTR and retry it.
> 
> 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 | 8 +++++---
>   1 file changed, 5 insertions(+), 3 deletions(-)
> 
> diff --git a/tools/kwboot.c b/tools/kwboot.c
> index de433c1b04..8b748f0fdd 100644
> --- a/tools/kwboot.c
> +++ b/tools/kwboot.c
> @@ -292,13 +292,15 @@ static int blk_rsp_timeo = KWBOOT_BLK_RSP_TIMEO;
>   static ssize_t
>   kwboot_write(int fd, const char *buf, size_t len)
>   {
> -	size_t tot = 0;
> +	ssize_t tot = 0;
>   
>   	while (tot < len) {
>   		ssize_t wr = write(fd, buf + tot, len - tot);
>   
> -		if (wr < 0)
> -			return -1;
> +		if (wr < 0 && errno == EINTR)
> +			continue;
> +		else if (wr < 0)
> +			return wr;
>   
>   		tot += wr;
>   	}

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

[PATCH u-boot-marvell 12/14] tools: kwboot: Handle EINTR in kwboot_tty_recv()

[Marek Behún]

From: Pali Rohár <pali@kernel.org>

The select() and read() syscalls may be interrupted. Handle EINTR and
retry them.

Signed-off-by: Pali Rohár <pali@kernel.org>
Signed-off-by: Marek Behún <marek.behun@nic.cz>
---
 tools/kwboot.c | 10 +++++++---
 1 file changed, 7 insertions(+), 3 deletions(-)

diff --git a/tools/kwboot.c b/tools/kwboot.c
index 8b748f0fdd..fca1c73c55 100644
--- a/tools/kwboot.c
+++ b/tools/kwboot.c
@@ -409,15 +409,19 @@ kwboot_tty_recv(int fd, void *buf, size_t len, int timeo)
 
 	do {
 		nfds = select(fd + 1, &rfds, NULL, NULL, &tv);
-		if (nfds < 0)
+		if (nfds < 0 && errno == EINTR)
+			continue;
+		else if (nfds < 0)
 			goto out;
-		if (!nfds) {
+		else if (!nfds) {
 			errno = ETIMEDOUT;
 			goto out;
 		}
 
 		n = read(fd, buf, len);
-		if (n <= 0)
+		if (n < 0 && errno == EINTR)
+			continue;
+		else if (n <= 0)
 			goto out;
 
 		buf = (char *)buf + n;
-- 
2.34.1

Re: [PATCH u-boot-marvell 12/14] tools: kwboot: Handle EINTR in kwboot_tty_recv()

[Stefan Roese]

On 1/25/22 18:13, Marek Behún wrote:
> From: Pali Rohár <pali@kernel.org>
> 
> The select() and read() syscalls may be interrupted. Handle EINTR and
> retry them.
> 
> 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 | 10 +++++++---
>   1 file changed, 7 insertions(+), 3 deletions(-)
> 
> diff --git a/tools/kwboot.c b/tools/kwboot.c
> index 8b748f0fdd..fca1c73c55 100644
> --- a/tools/kwboot.c
> +++ b/tools/kwboot.c
> @@ -409,15 +409,19 @@ kwboot_tty_recv(int fd, void *buf, size_t len, int timeo)
>   
>   	do {
>   		nfds = select(fd + 1, &rfds, NULL, NULL, &tv);
> -		if (nfds < 0)
> +		if (nfds < 0 && errno == EINTR)
> +			continue;
> +		else if (nfds < 0)
>   			goto out;
> -		if (!nfds) {
> +		else if (!nfds) {
>   			errno = ETIMEDOUT;
>   			goto out;
>   		}
>   
>   		n = read(fd, buf, len);
> -		if (n <= 0)
> +		if (n < 0 && errno == EINTR)
> +			continue;
> +		else if (n <= 0)
>   			goto out;
>   
>   		buf = (char *)buf + n;

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

[PATCH u-boot-marvell 13/14] tools: kwboot: Fix usage of -D without -t

[Marek Behún]

From: Pali Rohár <pali@kernel.org>

When -D is specified then both bootmsg and debugmsg are not set, but
imgpath is set. Fix this check for valid and required parameters.

Signed-off-by: Pali Rohár <pali@kernel.org>
Signed-off-by: Marek Behún <marek.behun@nic.cz>
---
 tools/kwboot.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/tools/kwboot.c b/tools/kwboot.c
index fca1c73c55..859559fb72 100644
--- a/tools/kwboot.c
+++ b/tools/kwboot.c
@@ -1819,7 +1819,7 @@ main(int argc, char **argv)
 		}
 	} while (1);
 
-	if (!bootmsg && !term && !debugmsg)
+	if (!bootmsg && !term && !debugmsg && !imgpath)
 		goto usage;
 
 	ttypath = argv[optind++];
-- 
2.34.1

Re: [PATCH u-boot-marvell 13/14] tools: kwboot: Fix usage of -D without -t

[Stefan Roese]

On 1/25/22 18:13, Marek Behún wrote:
> From: Pali Rohár <pali@kernel.org>
> 
> When -D is specified then both bootmsg and debugmsg are not set, but
> imgpath is set. Fix this check for valid and required parameters.
> 
> 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 | 2 +-
>   1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/tools/kwboot.c b/tools/kwboot.c
> index fca1c73c55..859559fb72 100644
> --- a/tools/kwboot.c
> +++ b/tools/kwboot.c
> @@ -1819,7 +1819,7 @@ main(int argc, char **argv)
>   		}
>   	} while (1);
>   
> -	if (!bootmsg && !term && !debugmsg)
> +	if (!bootmsg && !term && !debugmsg && !imgpath)
>   		goto usage;
>   
>   	ttypath = argv[optind++];

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

[PATCH u-boot-marvell 14/14] tools: kwboot: Set debug flag to 1

[Marek Behún]

From: Pali Rohár <pali@kernel.org>

This should enable BootROM output on UART.

(At least on A385 BootROM this is broken, BootROM ignores this debug
 flag and does not enable its output on UART if some valid image is
 available in SPI-NOR.)

Signed-off-by: Pali Rohár <pali@kernel.org>
Signed-off-by: Marek Behún <marek.behun@nic.cz>
---
 tools/kwboot.c | 1 +
 1 file changed, 1 insertion(+)

diff --git a/tools/kwboot.c b/tools/kwboot.c
index 859559fb72..2684f0e75a 100644
--- a/tools/kwboot.c
+++ b/tools/kwboot.c
@@ -1631,6 +1631,7 @@ kwboot_img_patch(void *img, size_t *size, int baudrate)
 			 * baudrate (which should be 115200) and do not touch
 			 * UART MPP configuration.
 			 */
+			hdr->flags |= 0x1;
 			hdr->options &= ~0x1F;
 			hdr->options |= MAIN_HDR_V1_OPT_BAUD_DEFAULT;
 			hdr->options |= 0 << 3;
-- 
2.34.1

Re: [PATCH u-boot-marvell 14/14] tools: kwboot: Set debug flag to 1

[Stefan Roese]

On 1/25/22 18:13, Marek Behún wrote:
> From: Pali Rohár <pali@kernel.org>
> 
> This should enable BootROM output on UART.
> 
> (At least on A385 BootROM this is broken, BootROM ignores this debug
>   flag and does not enable its output on UART if some valid image is
>   available in SPI-NOR.)
> 
> 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 | 1 +
>   1 file changed, 1 insertion(+)
> 
> diff --git a/tools/kwboot.c b/tools/kwboot.c
> index 859559fb72..2684f0e75a 100644
> --- a/tools/kwboot.c
> +++ b/tools/kwboot.c
> @@ -1631,6 +1631,7 @@ kwboot_img_patch(void *img, size_t *size, int baudrate)
>   			 * baudrate (which should be 115200) and do not touch
>   			 * UART MPP configuration.
>   			 */
> +			hdr->flags |= 0x1;
>   			hdr->options &= ~0x1F;
>   			hdr->options |= MAIN_HDR_V1_OPT_BAUD_DEFAULT;
>   			hdr->options |= 0 << 3;

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

Re: [PATCH u-boot-marvell 00/14] Another set of kwboot improvements

[Stefan Roese]

On 1/25/22 18:12, Marek Behún wrote:
> From: Marek Behún <marek.behun@nic.cz>
> 
> Hello Stefan,
> 
> here comes another set of kwboot improvements from Pali, reviewed and
> signed off by myself.
> 
> Marek
> 
> Pali Rohár (14):
>    tools: kwboot: Increase blk_rsp_timeo to 2s
>    tools: kwboot: Wait blk_rsp_timeo when flushing
>    tools: kwboot: Improve retrying logic for incomplete xmodem packets
>    tools: kwboot: Remove code for handling CAN byte
>    tools: kwboot: Do not change received character in
>      kwboot_xm_recv_reply()
>    tools: kwboot: Fix handling of repeated xmodem packets
>    tools: kwboot: Show 'E' in progress output when error occurs
>    tools: kwboot: Allow to use option -b without image path
>    tools: kwboot: Force BootROM to flush input queue after boot pattern
>    tools: kwboot: Remove 2s delay before sending first xmodem packet
>    tools: kwboot: Handle EINTR in kwboot_write()
>    tools: kwboot: Handle EINTR in kwboot_tty_recv()
>    tools: kwboot: Fix usage of -D without -t
>    tools: kwboot: Set debug flag to 1
> 
>   tools/kwboot.c | 154 ++++++++++++++++++++++++++++++++++++-------------
>   1 file changed, 115 insertions(+), 39 deletions(-)
> 

Applied to u-boot-marvell/master

Thanks,
Stefan