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

在 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 <[email protected]>
> 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