On Wed, 3 Jan 2024, Lino Sanfilippo wrote:
The drivers RS485 support is deactivated if there is no RTS GPIO available. This is done by nullifying the ports rs485_supported struct. After that however the settings in serial_omap_rs485_supported are assigned to the same structure unconditionally, which results in an unintended reactivation of RS485 support.
Fix this by moving the assignment to the beginning of serial_omap_probe_rs485() and thus before uart_get_rs485_mode() gets called.
This doesn't seem to accurately reflect what the problem is (which you correctly described in the paragraph above this). The problem doesn't seem to have anything to do with the placement of uart_get_rs485_mode() call but the if (IS_ERR(up->rts_gpiod)) block that clears rs485_supported?
A future work item that came to my mind while reviewing this: I suppose uart_disable_rs485_support() could be added into core which memsets rs485_supported and rs485 to zero so this driver could just call it.
Also replace the assignment of rs485_config() to have the complete RS485 setup in one function.
Fixes: e2752ae3cfc9 ("serial: omap: Disallow RS-485 if rts-gpio is not specified") Cc: stable@vger.kernel.org Signed-off-by: Lino Sanfilippo l.sanfilippo@kunbus.com
drivers/tty/serial/omap-serial.c | 27 ++++++++++++++------------- 1 file changed, 14 insertions(+), 13 deletions(-)
diff --git a/drivers/tty/serial/omap-serial.c b/drivers/tty/serial/omap-serial.c index ad4c1c5d0a7f..f4c6ff806465 100644 --- a/drivers/tty/serial/omap-serial.c +++ b/drivers/tty/serial/omap-serial.c @@ -1483,6 +1483,13 @@ static struct omap_uart_port_info *of_get_uart_port_info(struct device *dev) return omap_up_info; } +static const struct serial_rs485 serial_omap_rs485_supported = {
- .flags = SER_RS485_ENABLED | SER_RS485_RTS_ON_SEND | SER_RS485_RTS_AFTER_SEND |
SER_RS485_RX_DURING_TX,- .delay_rts_before_send = 1,
- .delay_rts_after_send = 1,
+};
static int serial_omap_probe_rs485(struct uart_omap_port *up, struct device *dev) { @@ -1497,6 +1504,9 @@ static int serial_omap_probe_rs485(struct uart_omap_port *up, if (!np) return 0;
- up->port.rs485_config = serial_omap_config_rs485;
- up->port.rs485_supported = serial_omap_rs485_supported;
- ret = uart_get_rs485_mode(&up->port); if (ret) return ret;
@@ -1531,13 +1541,6 @@ static int serial_omap_probe_rs485(struct uart_omap_port *up, return 0; } -static const struct serial_rs485 serial_omap_rs485_supported = {
- .flags = SER_RS485_ENABLED | SER_RS485_RTS_ON_SEND | SER_RS485_RTS_AFTER_SEND |
SER_RS485_RX_DURING_TX,- .delay_rts_before_send = 1,
- .delay_rts_after_send = 1,
-};
static int serial_omap_probe(struct platform_device *pdev) { struct omap_uart_port_info *omap_up_info = dev_get_platdata(&pdev->dev); @@ -1604,17 +1607,11 @@ static int serial_omap_probe(struct platform_device *pdev) dev_info(up->port.dev, "no wakeirq for uart%d\n", up->port.line);
- ret = serial_omap_probe_rs485(up, &pdev->dev);
- if (ret < 0)
goto err_rs485;- sprintf(up->name, "OMAP UART%d", up->port.line); up->port.mapbase = mem->start; up->port.membase = base; up->port.flags = omap_up_info->flags; up->port.uartclk = omap_up_info->uartclk;
- up->port.rs485_config = serial_omap_config_rs485;
- up->port.rs485_supported = serial_omap_rs485_supported; if (!up->port.uartclk) { up->port.uartclk = DEFAULT_CLK_SPEED; dev_warn(&pdev->dev,
@@ -1622,6 +1619,10 @@ static int serial_omap_probe(struct platform_device *pdev) DEFAULT_CLK_SPEED); }
- ret = serial_omap_probe_rs485(up, &pdev->dev);
- if (ret < 0)
goto err_rs485;- up->latency = PM_QOS_CPU_LATENCY_DEFAULT_VALUE; up->calc_latency = PM_QOS_CPU_LATENCY_DEFAULT_VALUE; cpu_latency_qos_add_request(&up->pm_qos_request, up->latency);
Moving of serial_omap_probe_rs485() later in serial_omap_probe() doesn't seem anymore required but I guess it doesn't hurt either.
Reviewed-by: Ilpo Järvinen ilpo.jarvinen@linux.intel.com