Re: [PATCH 21/33] serial: 8250: extract serial8250_set_errors_and_ignores

["Ilpo Järvinen" <ilpo.jarvinen@linux.intel.com>]

On Wed, 11 Jun 2025, Jiri Slaby (SUSE) wrote:

> serial8250_do_set_termios() consists of many registers and up flags
> settings. Extract all these into separate functions. This time, setting
> of ignore_status_mask and read_status_mask.
> 
> Signed-off-by: Jiri Slaby (SUSE) <jirislaby@kernel.org>
> ---
>  drivers/tty/serial/8250/8250_port.c | 67 ++++++++++++++---------------
>  1 file changed, 33 insertions(+), 34 deletions(-)
> 
> diff --git a/drivers/tty/serial/8250/8250_port.c b/drivers/tty/serial/8250/8250_port.c
> index b15371838366..6bb7f004d607 100644
> --- a/drivers/tty/serial/8250/8250_port.c
> +++ b/drivers/tty/serial/8250/8250_port.c
> @@ -2720,6 +2720,38 @@ static void serial8250_set_afe(struct uart_port *port, struct ktermios *termios)
>  		up->mcr |= UART_MCR_AFE;
>  }
>  
> +static void serial8250_set_errors_and_ignores(struct uart_port *port, struct ktermios *termios)
> +{
> +	/*
> +	 * Specify which conditions may be considered for error handling and the ignoring of
> +	 * characters. The actual ignoring of characters only occurs if the bit is set in
> +	 * @ignore_status_mask as well.
> +	 */
> +	port->read_status_mask = UART_LSR_OE | UART_LSR_DR;
> +	if (termios->c_iflag & INPCK)
> +		port->read_status_mask |= UART_LSR_FE | UART_LSR_PE;
> +	if (termios->c_iflag & (IGNBRK | BRKINT | PARMRK))
> +		port->read_status_mask |= UART_LSR_BI;
> +
> +	/* Characters to ignore */
> +	port->ignore_status_mask = 0;
> +	if (termios->c_iflag & IGNPAR)
> +		port->ignore_status_mask |= UART_LSR_PE | UART_LSR_FE;
> +	if (termios->c_iflag & IGNBRK) {
> +		port->ignore_status_mask |= UART_LSR_BI;
> +		/*
> +		 * If we're ignoring parity and break indicators, ignore overruns too (for real raw
> +		 * support).
> +		 */
> +		if (termios->c_iflag & IGNPAR)
> +			port->ignore_status_mask |= UART_LSR_OE;
> +	}
> +
> +	/* ignore all characters if CREAD is not set */
> +	if ((termios->c_cflag & CREAD) == 0)
> +		port->ignore_status_mask |= UART_LSR_DR;
> +}

Can this function be used also by 8250_omap.c or made reusable if not 
directly possible?

Inside non-8250 drivers there might even more duplication related to 
these. There might also be some duplication on the read character side
as I've feeling I've seen plenty of copy paste related these flags (or 
what looked copy paste).

>  void
>  serial8250_do_set_termios(struct uart_port *port, struct ktermios *termios,
>  		          const struct ktermios *old)
> @@ -2747,40 +2779,7 @@ serial8250_do_set_termios(struct uart_port *port, struct ktermios *termios,
>  	serial8250_set_trigger_for_slow_speed(port, termios, baud);
>  	serial8250_set_afe(port, termios);
>  	uart_update_timeout(port, termios->c_cflag, baud);
> -
> -	/*
> -	 * Specify which conditions may be considered for error
> -	 * handling and the ignoring of characters. The actual
> -	 * ignoring of characters only occurs if the bit is set
> -	 * in @ignore_status_mask as well.
> -	 */
> -	port->read_status_mask = UART_LSR_OE | UART_LSR_DR;
> -	if (termios->c_iflag & INPCK)
> -		port->read_status_mask |= UART_LSR_FE | UART_LSR_PE;
> -	if (termios->c_iflag & (IGNBRK | BRKINT | PARMRK))
> -		port->read_status_mask |= UART_LSR_BI;
> -
> -	/*
> -	 * Characters to ignore
> -	 */
> -	port->ignore_status_mask = 0;
> -	if (termios->c_iflag & IGNPAR)
> -		port->ignore_status_mask |= UART_LSR_PE | UART_LSR_FE;
> -	if (termios->c_iflag & IGNBRK) {
> -		port->ignore_status_mask |= UART_LSR_BI;
> -		/*
> -		 * If we're ignoring parity and break indicators,
> -		 * ignore overruns too (for real raw support).
> -		 */
> -		if (termios->c_iflag & IGNPAR)
> -			port->ignore_status_mask |= UART_LSR_OE;
> -	}
> -
> -	/*
> -	 * ignore all characters if CREAD is not set
> -	 */
> -	if ((termios->c_cflag & CREAD) == 0)
> -		port->ignore_status_mask |= UART_LSR_DR;
> +	serial8250_set_errors_and_ignores(port, termios);
>  
>  	/*
>  	 * CTS flow control flag and modem status interrupts
> 

-- 
 i.