diff mbox series

[v2,7/9] serial: ar933x: Fix check for RS485 support

Message ID 20220703170039.2058202-8-LinoSanfilippo@gmx.de
State New
Headers show
Series [v2,1/9] serial: core: only get RS485 termination GPIO if supported | expand

Commit Message

Lino Sanfilippo July 3, 2022, 5 p.m. UTC 
From: Lino Sanfilippo <l.sanfilippo@kunbus.com>

Without an RTS GPIO RS485 is not possible so disable the support
regardless of whether RS485 is enabled at boottime or not. Also remove the
now superfluous check for the RTS GPIO in ar933x_config_rs485().

Fixes: e849145e1fdd ("serial: ar933x: Fill in rs485_supported")
Signed-off-by: Lino Sanfilippo <l.sanfilippo@kunbus.com>
---
 drivers/tty/serial/ar933x_uart.c | 17 +++++------------
 1 file changed, 5 insertions(+), 12 deletions(-)

Comments

Andy Shevchenko July 3, 2022, 6:39 p.m. UTC | #1 
On Sun, Jul 3, 2022 at 7:02 PM Lino Sanfilippo <LinoSanfilippo@gmx.de> wrote:
>
> From: Lino Sanfilippo <l.sanfilippo@kunbus.com>
>
> Without an RTS GPIO RS485 is not possible so disable the support
> regardless of whether RS485 is enabled at boottime or not. Also remove the

boot time

> now superfluous check for the RTS GPIO in ar933x_config_rs485().
>
> Fixes: e849145e1fdd ("serial: ar933x: Fill in rs485_supported")

Is it an independent fix? If so, it should be the first patch in the
series, otherwise if it's dependent on something from previous patches
you need to mark all of them as a fix.
Lino Sanfilippo July 4, 2022, 9:21 a.m. UTC | #2 
On 03.07.22 20:39, Andy Shevchenko wrote:
> On Sun, Jul 3, 2022 at 7:02 PM Lino Sanfilippo <LinoSanfilippo@gmx.de> wrote:
>>
>> From: Lino Sanfilippo <l.sanfilippo@kunbus.com>
>>
>> Without an RTS GPIO RS485 is not possible so disable the support
>> regardless of whether RS485 is enabled at boottime or not. Also remove the
>
> boot time
>
>> now superfluous check for the RTS GPIO in ar933x_config_rs485().
>>
>> Fixes: e849145e1fdd ("serial: ar933x: Fill in rs485_supported")
>
> Is it an independent fix? If so, it should be the first patch in the
> series, otherwise if it's dependent on something from previous patches
> you need to mark all of them as a fix.
>

The fix is independent, patch 8 depends on the fix however. I was not
aware of this fixes-first rule for series with patches that are independent
from each other. I will change the order accordingly in the next version of the series.

Thanks,
Lino
Ilpo Järvinen July 4, 2022, 9:53 a.m. UTC | #3 
On Mon, 4 Jul 2022, Lino Sanfilippo wrote:
> On 03.07.22 20:39, Andy Shevchenko wrote:
> > On Sun, Jul 3, 2022 at 7:02 PM Lino Sanfilippo <LinoSanfilippo@gmx.de> wrote:
> >>
> >> From: Lino Sanfilippo <l.sanfilippo@kunbus.com>
> >>
> >> Without an RTS GPIO RS485 is not possible so disable the support
> >> regardless of whether RS485 is enabled at boottime or not. Also remove the
> >
> > boot time
> >
> >> now superfluous check for the RTS GPIO in ar933x_config_rs485().
> >>
> >> Fixes: e849145e1fdd ("serial: ar933x: Fill in rs485_supported")
> >
> > Is it an independent fix? If so, it should be the first patch in the
> > series, otherwise if it's dependent on something from previous patches
> > you need to mark all of them as a fix.
> >
> 
> The fix is independent, patch 8 depends on the fix however. I was not
> aware of this fixes-first rule for series with patches that are independent
> from each other. I will change the order accordingly in the next version of the series.

While at it, you could separate just the fix to own patch and the 
->rs485_config() cleanup to own patch (or move it all to patch 8).

Not that this fix is expected to go anywhere else besides tty-next.
diff mbox series

Patch

diff --git a/drivers/tty/serial/ar933x_uart.c b/drivers/tty/serial/ar933x_uart.c
index b73ce13683db..dac48a330db6 100644
--- a/drivers/tty/serial/ar933x_uart.c
+++ b/drivers/tty/serial/ar933x_uart.c
@@ -583,14 +583,6 @@  static const struct uart_ops ar933x_uart_ops = {
 static int ar933x_config_rs485(struct uart_port *port, struct ktermios *termios,
 				struct serial_rs485 *rs485conf)
 {
-	struct ar933x_uart_port *up =
-		container_of(port, struct ar933x_uart_port, port);
-
-	if ((rs485conf->flags & SER_RS485_ENABLED) &&
-	    !up->rts_gpiod) {
-		dev_err(port->dev, "RS485 needs rts-gpio\n");
-		return 1;
-	}
 	port->rs485 = *rs485conf;
 	return 0;
 }
@@ -798,11 +790,12 @@  static int ar933x_uart_probe(struct platform_device *pdev)
 
 	up->rts_gpiod = mctrl_gpio_to_gpiod(up->gpios, UART_GPIO_RTS);
 
-	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;
+	if (!up->rts_gpiod) {
 		port->rs485_supported = &ar933x_no_rs485;
+		if (port->rs485.flags & SER_RS485_ENABLED) {
+			dev_err(&pdev->dev, "lacking rts-gpio, disabling RS485\n");
+			port->rs485.flags &= ~SER_RS485_ENABLED;
+		}
 	}
 
 #ifdef CONFIG_SERIAL_AR933X_CONSOLE