| Message ID | 20231209125836.16294-7-l.sanfilippo@kunbus.com |
|---|---|
| State | New |
| Headers | show |
| Series | Fixes and improvements for RS485 | expand |
On Sat, 9 Dec 2023, Lino Sanfilippo wrote: > In serial_omap_rs485() RS485 support may be deactivated due to a missing There's no serial_omap_rs485() function. I assume/know you meant serial_omap_probe_rs485() but please correct. > RTS GPIO. This is done by nullifying the ports rs485_supported struct. > After that however the serial_omap_rs485_supported struct is assigned to > the same structure unconditionally, which results in an unintended > reactivation of RS485 support. > > Fix this by callling serial_omap_rs485() after the assignment of callling -> calling. Again, the function name is incorrect. > rs485_supported. Wouldn't it be better if all rs485 init/setups would occur in the same place rather than being spread around? That is, move the rs485_config and rs485_supported setup into serial_omap_probe_rs485()?
On 13.12.23 11:26, Ilpo Järvinen wrote: > On Sat, 9 Dec 2023, Lino Sanfilippo wrote: > >> In serial_omap_rs485() RS485 support may be deactivated due to a missing > > There's no serial_omap_rs485() function. I assume/know you meant > serial_omap_probe_rs485() but please correct. > Right, I meant serial_omap_probe_rs485(). Will fix the misnaming as well as the typos below. >> RTS GPIO. This is done by nullifying the ports rs485_supported struct. >> After that however the serial_omap_rs485_supported struct is assigned to >> the same structure unconditionally, which results in an unintended >> reactivation of RS485 support. >> >> Fix this by callling serial_omap_rs485() after the assignment of > > callling -> calling. > > Again, the function name is incorrect. > >> rs485_supported. > > Wouldn't it be better if all rs485 init/setups would occur in the same > place rather than being spread around? That is, move the rs485_config and > rs485_supported setup into serial_omap_probe_rs485()? > No problem, I can do that. Thanks for the review(s)! Regards, Lino
diff --git a/drivers/tty/serial/omap-serial.c b/drivers/tty/serial/omap-serial.c index ad4c1c5d0a7f..d9b2936308c4 100644 --- a/drivers/tty/serial/omap-serial.c +++ b/drivers/tty/serial/omap-serial.c @@ -1604,10 +1604,6 @@ 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; @@ -1622,6 +1618,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);
In serial_omap_rs485() RS485 support may be deactivated due to a missing RTS GPIO. This is done by nullifying the ports rs485_supported struct. After that however the serial_omap_rs485_supported struct is assigned to the same structure unconditionally, which results in an unintended reactivation of RS485 support. Fix this by callling serial_omap_rs485() after the assignment of rs485_supported. 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 | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-)