Re: [PATCH v7 1/9] serial: max310x: don't clobber the TX break bit in set_termios

[Hugo Villeneuve]

Hi Tapio,

You commit message indicate only part of what your patch changed,
but not why.

In this case, IIUC, your patch prevent ending a preconfigured
TX break when calling set_termios()?

Check this great resource for tips:
https://cbea.ms/git-commit/#why-not-how


On Mon, 05 Oct 2026 13:19:32 +0000
Tapio Reijonen <tapio.reijonen@xxxxxxxxxxx> wrote:

> max310x_set_termios() overwrites the whole LCR register, but LCR
> also carries the TX break bit that max310x_break_ctl() drives. A break
> is a state, not an event: TIOCSBRK sets the bit and it must stay set
> until TIOCCBRK. Any termios change in between - no concurrency
> required - rewrites LCR from the termios bits alone and silently ends
> the break early.
> 
> Update only the LCR bits that are derived from termios and leave the
> TX break and RTS pin control bits untouched. Since nothing clears a
> break when a port is closed with the break still asserted - the tty
> core sends no break-off on release, and the unconditional write here
> was the accidental recovery - clear TXBREAK in startup(), the same way
> 8250 does.
> 
> Fixes: f65444187a66 ("serial: New serial driver MAX310X")
> Assisted-by: Claude:claude-fable-5
> Signed-off-by: Tapio Reijonen <tapio.reijonen@xxxxxxxxxxx>
> ---
>  drivers/tty/serial/max310x.c | 17 +++++++++++++++--
>  1 file changed, 15 insertions(+), 2 deletions(-)
> 
> diff --git a/drivers/tty/serial/max310x.c b/drivers/tty/serial/max310x.c
> index 022502986c5fcf1ff4de9328746ddc71677be730..fead9c51163d8372d1b609ee9cd5b87faa917fc1 100644
> --- a/drivers/tty/serial/max310x.c
> +++ b/drivers/tty/serial/max310x.c
> @@ -158,6 +158,8 @@
>  #define MAX310X_LCR_FORCEPARITY_BIT	(1 << 5) /* 9-bit multidrop parity */
>  #define MAX310X_LCR_TXBREAK_BIT		(1 << 6) /* TX break enable */
>  #define MAX310X_LCR_RTS_BIT		(1 << 7) /* RTS pin control */
> +/* LCR bits owned by termios; TX break and RTS are driven elsewhere */
> +#define MAX310X_LCR_TERMIOS_MASK	GENMASK(5, 0)
>  
>  /* IRDA register bits */
>  #define MAX310X_IRDA_IRDAEN_BIT		(1 << 0) /* IRDA mode enable */
> @@ -969,8 +971,12 @@ static void max310x_set_termios(struct uart_port *port,
>  	if (termios->c_cflag & CSTOPB)
>  		lcr |= MAX310X_LCR_STOPLEN_BIT; /* 2 stops */
>  
> -	/* Update LCR register */
> -	max310x_port_write(port, MAX310X_LCR_REG, lcr);
> +	/*
> +	 * Update LCR register. Leave the TX break bit alone: it is driven by
> +	 * break_ctl(), and a whole-register write here would end a break in
> +	 * progress.
> +	 */
> +	max310x_port_update(port, MAX310X_LCR_REG, MAX310X_LCR_TERMIOS_MASK, lcr);
>  
>  	/* Set read status mask */
>  	port->read_status_mask = MAX310X_LSR_RXOVR_BIT;
> @@ -1088,6 +1094,13 @@ static int max310x_startup(struct uart_port *port)
>  
>  	max310x_power(port, 1);
>  
> +	/*
> +	 * Clear a latched break: nothing clears TXBREAK when a port is
> +	 * closed with a break still asserted, and set_termios() no longer
> +	 * rewrites it.
> +	 */
> +	max310x_port_update(port, MAX310X_LCR_REG, MAX310X_LCR_TXBREAK_BIT, 0);
> +
>  	/* Configure MODE1 register */
>  	max310x_port_update(port, MAX310X_MODE1_REG,
>  			    MAX310X_MODE1_TRNSCVCTRL_BIT, 0);
> 
> -- 
> 2.47.3
> 


-- 
Hugo Villeneuve