[U-Boot] [PATCH 2/2] RFC: Add XON/XOFF support

[Albert ARIBAUD <albert.u.boot@aribaud.net>]

Hi Simon,

Le 25/10/2011 05:52, Simon Glass a ?crit :
> This is just for testing - please try it out and report back with results.
> For me it works on Minicom but not ser2net.

Exactly how does it not work, i.e. what happens that you did not expect, 
or what does not happen that you did expect?

> This needs to be controlled by an environment variable, CONFIG option or
> both. We may need a way of specifying flow control on a per-device basis.

Hmm... We want XON/XOFF on the console only. What other device would 
require this?

> We may need to do something special for X-modem transfers, etc. This might
> turn out to be a can of worms.

I think any U-Boot command that uses the console as a means of data 
transfer should 'resume console' when starting, and can safely do so 
because if the command was invoked, then most certainly the other end of 
the console is not going to fire away; it is going to wait for U-Boot to 
get ready for transfer. Suspending the console after transfer should be 
done if resume was done.

BTW:

> Signed-off-by: Simon Glass<sjg@chromium.org>
> ---
>   common/command.c |    1 +
>   common/console.c |   12 ++++++++++++
>   common/main.c    |    3 +++
>   include/common.h |    2 ++
>   4 files changed, 18 insertions(+), 0 deletions(-)
>
> diff --git a/common/command.c b/common/command.c
> index acc1c15..7b7c24e 100644
> --- a/common/command.c
> +++ b/common/command.c
> @@ -492,6 +492,7 @@ int cmd_call(cmd_tbl_t *cmdtp, int flag, int argc, char * const argv[])
>   {
>   	int result;
>
> +	console_suspend_input();
>   	result = (cmdtp->cmd)(cmdtp, flag, argc, argv);
>   	if (result)
>   		debug("Command failed, result=%d", result);
> diff --git a/common/console.c b/common/console.c
> index f17875e..eeb58e6 100644
> --- a/common/console.c
> +++ b/common/console.c
> @@ -199,6 +199,18 @@ static inline void console_doenv(int file, struct stdio_dev *dev)
>   }
>   #endif /* defined(CONFIG_CONSOLE_MUX) */
>
> +void console_suspend_input(void)
> +{
> +	/* Send XOFF to tell the other end to stop sending */
> +	console_putc(stdout, 'S' - '@');
> +}

Please define XOFF and XON.

> +void console_resume_input(void)
> +{
> +	/* Send XON to tell the other end to start sending */
> +	console_putc(stdout, 'Q' - '@');
> +}
> +
>   /** U-Boot INITIAL CONSOLE-NOT COMPATIBLE FUNCTIONS *************************/
>
>   int serial_printf(const char *fmt, ...)
> diff --git a/common/main.c b/common/main.c
> index 3b60d27..b030176 100644
> --- a/common/main.c
> +++ b/common/main.c
> @@ -948,6 +948,7 @@ int readline_into_buffer (const char *const prompt, char * buffer)
>
>   		if (prompt)
>   			puts (prompt);
> +		console_resume_input();

This makes XON sent on each readline. XOFF should be sent only if XOFF 
was sent before, IMO (and then, XOFF should only be sent if it was not 
sent already.

Plus, why send it on readlines? Why not just surround the 'result = 
(cmdtp->cmd)(cmdtp, flag, argc, argv);' above with a 'suspend' before 
and a 'resume' after?

>   		rc = cread_line(prompt, p,&len);
>   		return rc<  0 ? rc : len;
> @@ -967,6 +968,8 @@ int readline_into_buffer (const char *const prompt, char * buffer)
>   	}
>   	col = plen;
>
> +	console_resume_input();
> +
>   	for (;;) {
>   #ifdef CONFIG_BOOT_RETRY_TIME
>   		while (!tstc()) {	/* while no incoming data */
> diff --git a/include/common.h b/include/common.h
> index a1683a2..970ec0d 100644
> --- a/include/common.h
> +++ b/include/common.h
> @@ -738,6 +738,8 @@ int	ctrlc (void);
>   int	had_ctrlc (void);	/* have we had a Control-C since last clear? */
>   void	clear_ctrlc (void);	/* clear the Control-C condition */
>   int	disable_ctrlc (int);	/* 1 to disable, 0 to enable Control-C detect */
> +void console_suspend_input(void);
> +void console_resume_input(void);
>
>   /*
>    * STDIO based functions (can always be used)

Amicalement,
-- 
Albert.