On 27.06.22 10:14, Ilpo Järvinen wrote: > On Sun, 26 Jun 2022, Lino Sanfilippo wrote: > >> On 25.06.22 at 12:14, Ilpo Järvinen wrote: >>> On Wed, 22 Jun 2022, Lino Sanfilippo wrote: >>> >>>> From: Lino Sanfilippo <l.sanfilippo@xxxxxxxxxx> >>>> >>>> In uart_set_rs485_config() the serial core already assigns the passed >>>> serial_rs485 struct to the uart port. >>>> >>>> So remove the assignment in the drivers rs485_config() function to avoid >>>> redundancy. >>>> >>>> Signed-off-by: Lino Sanfilippo <l.sanfilippo@xxxxxxxxxx> >>>> --- >>>> drivers/tty/serial/ar933x_uart.c | 1 - >>>> 1 file changed, 1 deletion(-) >>>> >>>> diff --git a/drivers/tty/serial/ar933x_uart.c b/drivers/tty/serial/ar933x_uart.c >>>> index ab2c5b2a1ce8..857e010d01dc 100644 >>>> --- a/drivers/tty/serial/ar933x_uart.c >>>> +++ b/drivers/tty/serial/ar933x_uart.c >>>> @@ -591,7 +591,6 @@ static int ar933x_config_rs485(struct uart_port *port, >>>> dev_err(port->dev, "RS485 needs rts-gpio\n"); >>>> return 1; >>>> } >>>> - port->rs485 = *rs485conf; >>>> return 0; >>>> } >>> >>> Hmm, I realize that for some reason I missed cleaning up this particular >>> driver after introducing the serial_rs485 sanitization. It shouldn't need >>> that preceeding if block either because ar933x_no_rs485 gets applied if >>> there's no rts_gpiod so the core clears SER_RS485_ENABLED. >> >> I think we still need that "if" in case that RS485 was not enabled at driver >> startup (no rs485-enabled-at-boot-time) and no RTS GPIO was defined but then >> RS485 is enabled via TIOCSRS485. >> >> Maybe in ar933x_uart_probe() >> >> if ((port->rs485.flags & SER_RS485_ENABLED) && >> !up->rts_gpiod) { >> dev_err(&pdev->dev, "lacking rts-gpio, disabling RS485\n"); >> port->rs485.flags &= ~SER_RS485_ENABLED; >> port->rs485_supported = &ar933x_no_rs485; >> } >> >> should rather be > > I think it would be better (and what I should have done while moving the > check there in the first place but I missed it). In addition, however, it > would be useful to not print unnecessarily: > >> if (!up->rts_gpiod) { > > if (port->rs485.flags & SER_RS485_ENABLED) { > >> dev_err(&pdev->dev, "lacking rts-gpio, disabling RS485\n"); >> port->rs485.flags &= ~SER_RS485_ENABLED; > > } Right. I will send a fix for this with the new version of my series. Regards, Lino