Re: [PATCH v3 05/15] serial: 8250_mxpcie: offload XON/XOFF flow control to MUEx50 hardware

[Jiri Slaby <jirislaby@kernel.org>]

Hi,

On 09. 07. 26, 7:33, Crescent Hsieh wrote:
> The MUEx50 UART can handle in-band software flow control (XON/XOFF)
> directly in hardware.
> 
> Program the on-chip XON/XOFF characters from termios settings and enable
> the corresponding MUEx50 flow control modes when IXON or IXOFF is
> requested. Provide throttle and unthrottle callbacks so RX can be
> stopped and resumed cleanly.
> 
> Signed-off-by: Crescent Hsieh <crescentcy.hsieh@moxa.com>
> ---
>   drivers/tty/serial/8250/8250_mxpcie.c | 55 ++++++++++++++++++++++++++-
>   1 file changed, 54 insertions(+), 1 deletion(-)
> 
> diff --git a/drivers/tty/serial/8250/8250_mxpcie.c b/drivers/tty/serial/8250/8250_mxpcie.c
> index 0f5323581bba..0cdb7984cbcc 100644
> --- a/drivers/tty/serial/8250/8250_mxpcie.c
> +++ b/drivers/tty/serial/8250/8250_mxpcie.c
> @@ -53,13 +53,31 @@
>   #define MOXA_PUART_SFR_950	BIT(5)
>   
>   /* Enhanced Function Register (EFR) */
> +/*
> + * EFR[1:0] - In-Band Receive Flow Control Mode (Compare XON/XOFF):
> + *	00b (0x00) = Disabled
> + *	01b (0x01) = Recognize XON2 & XOFF2 as XOFF character
> + *	10b (0x02) = Recognize XON1 & XOFF1 as XOFF character
> + *	11b (0x03) = Depends on EFR[3:2]
> + * EFR[3:2] - In-Band Transmit Flow Control Mode (Insert XON/XOFF):
> + *	00b (0x00) = Disabled
> + *	01b (0x04) = Use XON2 & XOFF2 as XOFF character
> + *	10b (0x08) = Use XON1 & XOFF1 as XOFF character
> + *	11b (0x0C) = Reserved
> + */
>   #define MOXA_PUART_EFR			0x0A
> +#define MOXA_PUART_EFR_RX_FLOW		0x02	/* Recognize XON1 & XOFF1 as XOFF character */
> +#define MOXA_PUART_EFR_TX_FLOW		0x08	/* Use XON1 & XOFF1 as XOFF character */

For the above, see below:

>   #define MOXA_PUART_EFR_ENHANCED		BIT(4)
>   #define MOXA_PUART_EFR_AUTO_RTS		BIT(6)
>   #define MOXA_PUART_EFR_AUTO_CTS		BIT(7)
>   #define MOXA_PUART_EFR_RX_FLOW_MASK	GENMASK(1, 0)
>   #define MOXA_PUART_EFR_TX_FLOW_MASK	GENMASK(3, 2)

So instead of the above new comment and defines, what about:
#define MOXA_PUART_EFR_RX_FLOW_DISABLED		0x0
#define MOXA_PUART_EFR_RX_FLOW_XON2_XOFF2	0x1
#define MOXA_PUART_EFR_RX_FLOW_XON1_XON2	0x2
#define MOXA_PUART_EFR_RX_FLOW_COPY_TX		0x3

Similarly for TX. And the use below:

> @@ -169,6 +187,21 @@ static void mxpcie8250_set_termios(struct uart_port *port,
>   		efr |= (MOXA_PUART_EFR_AUTO_RTS | MOXA_PUART_EFR_AUTO_CTS);
>   		up->port.status |= (UPSTAT_AUTORTS | UPSTAT_AUTOCTS);
>   	}
> +	/* Set on-chip software flow control character */
> +	serial_out(up, MOXA_PUART_XON1, START_CHAR(tty));
> +	serial_out(up, MOXA_PUART_XON2, START_CHAR(tty));
> +	serial_out(up, MOXA_PUART_XOFF1, STOP_CHAR(tty));
> +	serial_out(up, MOXA_PUART_XOFF2, STOP_CHAR(tty));
> +
> +	efr &= ~(MOXA_PUART_EFR_RX_FLOW_MASK | MOXA_PUART_EFR_TX_FLOW_MASK);
> +
> +	if (I_IXON(tty))
> +		efr |= MOXA_PUART_EFR_RX_FLOW;

This would become:
unsigned val = I_IXON(tty) ? MOXA_PUART_EFR_RX_FLOW_XON1_XON2 : 
MOXA_PUART_EFR_RX_FLOW_DISABLED;

efr = FIELD_SET(MOXA_PUART_EFR_RX_FLOW_MASK, val, efr);

> +
> +	if (I_IXOFF(tty)) {
> +		efr |= MOXA_PUART_EFR_TX_FLOW;
> +		up->port.status |= UPSTAT_AUTOXOFF;
> +	}

Similarly.

>   	serial_out(up, MOXA_PUART_EFR, efr);
>   }
>   

thanks,
-- 
js
suse labs