[PATCH u-boot-marvell 04/10] tools: kwboot: Use separate thread for sending boot message pattern

[Stefan Roese]

On 3/2/22 11:49, Pali Rohár wrote:
> After BootROM successfully detects boot message pattern on UART it waits
> until host stop sending data on UART. For example Armada 385 BootROM
> requires that host does not send anything on UART at least 24 ms. If host
> is still sending something then BootROM waits (possibly infinitely).
> 
> BootROM successfully detects boot message pattern if it receives it in
> small period of time after power on.
> 
> So to ensure that host put BootROM into UART boot mode, host must send
> continuous stream of boot message pattern with a small gap (for A385 at
> least 24 ms) after series of pattern. But this gap cannot be too often or
> too long to ensure that it does not cover whole BootROM time window when it
> is detecting for boot message pattern.
> 
> Therefore it is needed to do following steps in cycle without any delay:
> 1. send series of boot message pattern over UART
> 2. wait until kernel transmit all data
> 3. sleep small period of time
> 
> At the same time, host needs to monitor input queue, data received on the
> UART and checking if it contains NAK byte by which BootROM informs that
> xmodem transfer is ready.
> 
> But it is not possible to wait until kernel transmit all data on UART and
> at the same time in the one process to also wait for input data. This is
> limitation of POSIX tty API and also by linux kernel that it does not
> provide asynchronous function for waiting until all data are transmitted.
> There is only synchronous variant tcdrain().
> 
> So to correctly implement this handshake on systems with linux kernel, it
> is needed to use tcdrain() in separate thread.
> 
> Implement sending of boot message pattern in one thread and reading of
> reply in the main thread. Use pthread library for threads.
> 
> This change makes UART booting on Armada 385 more reliable. It is possible
> to start kwboot and power on board after minute and kwboot correctly put
> board into UART boot mode.
> 
> Old implementation without separate thread has an issue that it read just
> one byte from UART input queue and then it send 128 message pattern to the
> output queue. If some noise was on UART then kwboot was not able to read
> BootROM response as its input queue was just overflowed and kwboot was
> sending more data than receiving.
> 
> This change basically fixed above issue too.
> 
> Signed-off-by: Pali Rohár <pali at kernel.org>

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

Thanks,
Stefan

> ---
>   tools/Makefile |   3 ++
>   tools/kwboot.c | 120 +++++++++++++++++++++++++++++++++++++++++--------
>   2 files changed, 104 insertions(+), 19 deletions(-)
> 
> diff --git a/tools/Makefile b/tools/Makefile
> index df941e0dca8d..c4a06dd9ba36 100644
> --- a/tools/Makefile
> +++ b/tools/Makefile
> @@ -199,6 +199,9 @@ hostprogs-$(CONFIG_EXYNOS5250) += mkexynosspl
>   hostprogs-$(CONFIG_EXYNOS5420) += mkexynosspl
>   HOSTCFLAGS_mkexynosspl.o := -pedantic
>   
> +HOSTCFLAGS_kwboot.o += -pthread
> +HOSTLDLIBS_kwboot += -pthread
> +
>   ifdtool-objs := $(LIBFDT_OBJS) ifdtool.o
>   hostprogs-$(CONFIG_X86) += ifdtool
>   
> diff --git a/tools/kwboot.c b/tools/kwboot.c
> index 4e2acb52458a..9fd90b9bec71 100644
> --- a/tools/kwboot.c
> +++ b/tools/kwboot.c
> @@ -28,6 +28,7 @@
>   #include <stdint.h>
>   #include <time.h>
>   #include <sys/stat.h>
> +#include <pthread.h>
>   
>   #ifdef __linux__
>   #include "termios_linux.h"
> @@ -717,37 +718,120 @@ out:
>   	return rc;
>   }
>   
> +static void *
> +kwboot_msg_write_handler(void *arg)
> +{
> +	int tty = *(int *)((void **)arg)[0];
> +	const void *msg = ((void **)arg)[1];
> +	int rsp_timeo = msg_rsp_timeo;
> +	int i, dummy_oldtype;
> +
> +	/* allow to cancel this thread at any time */
> +	pthread_setcanceltype(PTHREAD_CANCEL_ASYNCHRONOUS, &dummy_oldtype);
> +
> +	while (1) {
> +		/* write 128 samples of message pattern into the output queue without waiting */
> +		for (i = 0; i < 128; i++) {
> +			if (kwboot_tty_send(tty, msg, 8, 1) < 0) {
> +				perror("\nFailed to send message pattern");
> +				exit(1);
> +			}
> +		}
> +		/* wait until output queue is transmitted and then make pause */
> +		if (tcdrain(tty) < 0) {
> +			perror("\nFailed to send message pattern");
> +			exit(1);
> +		}
> +		/* BootROM requires pause on UART after it detects message pattern */
> +		usleep(rsp_timeo * 1000);
> +	}
> +}
> +
> +static int
> +kwboot_msg_start_thread(pthread_t *thread, int *tty, void *msg)
> +{
> +	void *arg[2];
> +	int rc;
> +
> +	arg[0] = tty;
> +	arg[1] = msg;
> +	rc = pthread_create(thread, NULL, kwboot_msg_write_handler, arg);
> +	if (rc) {
> +		errno = rc;
> +		return -1;
> +	}
> +
> +	return 0;
> +}
> +
> +static int
> +kwboot_msg_stop_thread(pthread_t thread)
> +{
> +	int rc;
> +
> +	rc = pthread_cancel(thread);
> +	if (rc) {
> +		errno = rc;
> +		return -1;
> +	}
> +
> +	rc = pthread_join(thread, NULL);
> +	if (rc) {
> +		errno = rc;
> +		return -1;
> +	}
> +
> +	return 0;
> +}
> +
>   static int
>   kwboot_bootmsg(int tty)
>   {
>   	struct kwboot_block block;
> -	int rc;
> +	pthread_t write_thread;
> +	int rc, err;
>   	char c;
> -	int count;
> -
> -	kwboot_printv("Sending boot message. Please reboot the target...");
>   
> -	do {
> -		rc = tcflush(tty, TCIOFLUSH);
> -		if (rc)
> -			break;
> +	/* flush input and output queue */
> +	tcflush(tty, TCIOFLUSH);
>   
> -		for (count = 0; count < 128; count++) {
> -			rc = kwboot_tty_send(tty, kwboot_msg_boot, sizeof(kwboot_msg_boot), 0);
> -			if (rc)
> -				break;
> -		}
> +	rc = kwboot_msg_start_thread(&write_thread, &tty, kwboot_msg_boot);
> +	if (rc) {
> +		perror("Failed to start write thread");
> +		return rc;
> +	}
>   
> -		rc = kwboot_tty_recv(tty, &c, 1, msg_rsp_timeo);
> +	kwboot_printv("Sending boot message. Please reboot the target...");
>   
> +	err = 0;
> +	while (1) {
>   		kwboot_spinner();
>   
> -	} while (rc || c != NAK);
> +		rc = kwboot_tty_recv(tty, &c, 1, msg_rsp_timeo);
> +		if (rc && errno == ETIMEDOUT) {
> +			continue;
> +		} else if (rc) {
> +			err = errno;
> +			break;
> +		}
> +
> +		if (c == NAK)
> +			break;
> +	}
>   
>   	kwboot_printv("\n");
>   
> -	if (rc)
> +	rc = kwboot_msg_stop_thread(write_thread);
> +	if (rc) {
> +		perror("Failed to stop write thread");
>   		return rc;
> +	}
> +
> +	if (err) {
> +		errno = err;
> +		perror("Failed to read response for boot message pattern");
> +		return -1;
> +	}
>   
>   	/*
>   	 * At this stage we have sent more boot message patterns and BootROM
> @@ -1873,10 +1957,8 @@ main(int argc, char **argv)
>   		}
>   	} else if (bootmsg) {
>   		rc = kwboot_bootmsg(tty);
> -		if (rc) {
> -			perror("bootmsg");
> +		if (rc)
>   			goto out;
> -		}
>   	}
>   
>   	if (img) {

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