linux-kernel - Re: [PATCH 21/33] serial: 8250: extract serial8250_set_errors_and

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite for Android: free password hash cracker in your pocket

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <c3917f82-40e9-6aa8-79d3-f6dae57671c4@linux.intel.com>
Date: Wed, 11 Jun 2025 15:43:45 +0300 (EEST)
From: Ilpo Järvinen <ilpo.jarvinen@...ux.intel.com>
To: "Jiri Slaby (SUSE)" <jirislaby@...nel.org>
cc: Greg Kroah-Hartman <gregkh@...uxfoundation.org>, 
    linux-serial <linux-serial@...r.kernel.org>, 
    LKML <linux-kernel@...r.kernel.org>
Subject: Re: [PATCH 21/33] serial: 8250: extract
 serial8250_set_errors_and_ignores

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@...nel.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.