Re: [PATCH 3/3] serial: 8250_mxpcie: take the line settings from the new termios

[Linmao Li]

在 2026/8/19 18:00, Crescent Hsieh 写道:
> On Tue, Aug 18, 2026 at 05:39:18PM +0800, Linmao Li wrote:
> > mxpcie8250_set_termios() reads the line settings out of
> > port->state->port.tty, which is only set once the port has been opened.
> >
> > uart_set_options() builds a termios of its own and calls ->set_termios()
> > with no tty behind it, so using such a board as the console
> > (console=ttyS<n>) dereferences a NULL tty during console setup, as does
> > attaching kgdboc to it and resuming a suspended console from
> > uart_resume_port().
> >
> > Read the settings from the termios the serial core passes in instead.
> > It holds the same values on the normal path - uart_change_line_settings()
> > passes &tty->termios - and it is what serial8250_do_set_termios() right
> > above already uses.
> >
> > Fixes: 55edf8511f47 ("serial: 8250_mxpcie: enable automatic RTS/CTS flow control")
> > Signed-off-by: Linmao Li <lilinmao@xxxxxxxxxx>
> Hi,
>
> I tested this by configuring kgdboc on a CP-168EL-A. Thanks for the fix.
Thank you for testing it.  May I add your Tested-by to v2?
> > ---
> >   drivers/tty/serial/8250/8250_mxpcie.c | 19 ++++++++++---------
> >   1 file changed, 10 insertions(+), 9 deletions(-)
> >
> > diff --git a/drivers/tty/serial/8250/8250_mxpcie.c b/drivers/tty/serial/8250/8250_mxpcie.c
> > index ddd02d5622ec4..eba59f1724b78 100644
> > --- a/drivers/tty/serial/8250/8250_mxpcie.c
> > +++ b/drivers/tty/serial/8250/8250_mxpcie.c
> > @@ -217,8 +217,7 @@ static void mxpcie8250_set_termios(struct uart_port *port,
> >   				   const struct ktermios *old)
> >   {
> >   	struct uart_8250_port *up = up_to_u8250p(port);
> > -	struct tty_struct *tty = port->state->port.tty;
> > -	unsigned int cflag = tty->termios.c_cflag;
> > +	unsigned int cflag = new->c_cflag;
> Nit: Since cflag is used only once, while c_iflag and c_cc are
> accessed directly through new below, could we drop the local variable
> and use new->c_cflag directly for consistency?

Yes, that reads better.  v2 drops the local and uses new->c_cflag.


Thanks,
Linmao

> >   	u8 efr, val;
> >   
> >   	serial8250_do_set_termios(port, new, old);
> > @@ -233,18 +232,20 @@ static void mxpcie8250_set_termios(struct uart_port *port,
> >   		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));
> > +	serial_out(up, MOXA_PUART_XON1, new->c_cc[VSTART]);
> > +	serial_out(up, MOXA_PUART_XON2, new->c_cc[VSTART]);
> > +	serial_out(up, MOXA_PUART_XOFF1, new->c_cc[VSTOP]);
> > +	serial_out(up, MOXA_PUART_XOFF2, new->c_cc[VSTOP]);
> >   
> > -	val = I_IXON(tty) ? MOXA_PUART_EFR_RX_FLOW_XON1_XOFF1 : MOXA_PUART_EFR_RX_FLOW_DISABLED;
> > +	val = (new->c_iflag & IXON) ? MOXA_PUART_EFR_RX_FLOW_XON1_XOFF1 :
> > +				      MOXA_PUART_EFR_RX_FLOW_DISABLED;
> >   	FIELD_MODIFY(MOXA_PUART_EFR_RX_FLOW_MASK, &efr, val);
> >   
> > -	val = I_IXOFF(tty) ? MOXA_PUART_EFR_TX_FLOW_XON1_XOFF1 : MOXA_PUART_EFR_TX_FLOW_DISABLED;
> > +	val = (new->c_iflag & IXOFF) ? MOXA_PUART_EFR_TX_FLOW_XON1_XOFF1 :
> > +				       MOXA_PUART_EFR_TX_FLOW_DISABLED;
> >   	FIELD_MODIFY(MOXA_PUART_EFR_TX_FLOW_MASK, &efr, val);
> >   
> > -	if (I_IXOFF(tty))
> > +	if (new->c_iflag & IXOFF)
> >   		up->port.status |= UPSTAT_AUTOXOFF;
> >   
> >   	serial_out(up, MOXA_PUART_EFR, efr);
> > --
> > 2.25.1
> ---
> Thanks,
> Crescent Hsieh