Re: [PATCH v2] tty: serial: Add RS422 flag to struct serial_rs485

[Lino Sanfilippo]

Hi,

On 01.11.23 07:49, Greg Kroah-Hartman wrote:
> On Wed, Nov 01, 2023 at 02:44:04PM +0800, Crescent CY Hsieh wrote:
>> Add "SER_RS422_ENABLED" flag within struct serial_rs485, so that serial
>> port can switching interface into RS422 if supported by using ioctl
>> command "TIOCSRS485".
>>
>> In case of interfaces confliction, add checks within
>> uart_sanitize_serial_rs485() such that only one of RS422/RS485 is set.
>>
>> Signed-off-by: Crescent CY Hsieh <crescentcy.hsieh@xxxxxxxx>
>>
>> ---
>> Changes in v2:
>> - Revise the logic that checks whether RS422/RS485 are enabled
>>   simultaneously.
>>
>> v1: https://lore.kernel.org/all/20231030053632.5109-1-crescentcy.hsieh@xxxxxxxx/
>>
>> ---
>>  drivers/tty/serial/serial_core.c | 19 +++++++++++++++++--
>>  include/uapi/linux/serial.h      |  4 ++++
>>  2 files changed, 21 insertions(+), 2 deletions(-)
>>
>> diff --git a/drivers/tty/serial/serial_core.c b/drivers/tty/serial/serial_core.c
>> index 831d03361..54a104c52 100644
>> --- a/drivers/tty/serial/serial_core.c
>> +++ b/drivers/tty/serial/serial_core.c
>> @@ -1305,7 +1305,7 @@ static int uart_get_icount(struct tty_struct *tty,
>>
>>  #define SER_RS485_LEGACY_FLAGS	(SER_RS485_ENABLED | SER_RS485_RTS_ON_SEND | \
>>  				 SER_RS485_RTS_AFTER_SEND | SER_RS485_RX_DURING_TX | \
>> -				 SER_RS485_TERMINATE_BUS)
>> +				 SER_RS485_TERMINATE_BUS | SER_RS422_ENABLED)
>
> A new flag is "legacy"?
>
>>
>>  static int uart_check_rs485_flags(struct uart_port *port, struct serial_rs485 *rs485)
>>  {
>> @@ -1371,11 +1371,26 @@ static void uart_sanitize_serial_rs485(struct uart_port *port, struct serial_rs4
>>  {
>>  	u32 supported_flags = port->rs485_supported.flags;
>>
>> -	if (!(rs485->flags & SER_RS485_ENABLED)) {
>> +	if (!(rs485->flags & (SER_RS485_ENABLED | SER_RS422_ENABLED))) {
>>  		memset(rs485, 0, sizeof(*rs485));
>>  		return;
>>  	}
>>
>> +	/* Pick sane setting if the user enables both interfaces */
>> +	if (rs485->flags & SER_RS485_ENABLED && rs485->flags & SER_RS422_ENABLED) {
>> +		dev_warn_ratelimited(port->dev,
>> +			"%s (%d): Invalid serial interface setting, using RS485 instead\n",
>> +			port->name, port->line);
>
> Why is this ratelimited?  What would cause lots of repeats of this?
>
>

uart_sanitize_serial_rs485() is called when userspace sets the RS485 configuration via TIOCSRS485. So
warnings in this function are ratelimited to prevent userspace from spamming the kernel log.

Regards,
Lino