| Message ID | 20220703170039.2058202-1-LinoSanfilippo@gmx.de |
|---|---|
| Headers | show |
| Series | Fixes and cleanup for RS485 | expand |
On 03.07.22 20:34, 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> >> >> When setting the RS485 configuration from userspace via TIOCSRS485 the >> delays are clamped to 100ms. Make this consistent with the values passed >> in by means of device tree parameters. > > I'm not sure I got it right. Is the values from DT now clampet as well > as user space does or other way around? In either way the commit > message misses the explanation why it's not a problem if user > previously passed bigger values either via user space or via DT, > because it's an ABI change, right? > Values are now clamped to 100 ms if set by userspace via ioctl and not clamped at all if set by DT. I will improve the commit message to make this more clear. Thanks, Lino
On Sun, 3 Jul 2022, Lino Sanfilippo wrote: > From: Lino Sanfilippo <l.sanfilippo@kunbus.com> > > In serial8250_em485_config() the termination GPIO is set with the uart_port > spinlock held. This is an issue if setting the GPIO line can sleep (e.g. > since the concerning GPIO expander is connected via SPI or I2C). > > Fix this by setting the termination line outside of the uart_port spinlock > in the serial core and using gpiod_set_value_cansleep() which instead of > gpiod_set_value() allows to sleep. > > Beside fixing the termination GPIO line setting for the 8250 driver this > change also makes setting the termination GPIO generic for all UART > drivers. > > Signed-off-by: Lino Sanfilippo <l.sanfilippo@kunbus.com> > --- > drivers/tty/serial/8250/8250_port.c | 3 --- > drivers/tty/serial/serial_core.c | 12 ++++++++++++ > 2 files changed, 12 insertions(+), 3 deletions(-) > > diff --git a/drivers/tty/serial/8250/8250_port.c b/drivers/tty/serial/8250/8250_port.c > index ed2a606f2da7..72252d956f17 100644 > --- a/drivers/tty/serial/8250/8250_port.c > +++ b/drivers/tty/serial/8250/8250_port.c > @@ -676,9 +676,6 @@ int serial8250_em485_config(struct uart_port *port, struct ktermios *termios, > rs485->flags &= ~SER_RS485_RTS_AFTER_SEND; > } > > - gpiod_set_value(port->rs485_term_gpio, > - rs485->flags & SER_RS485_TERMINATE_BUS); > - I sent a series to make .rs485_supported per uart_port and properly set SER_RS485_TERMINATE_BUS according to DT config. With that series added first, SER_RS485_TERMINATE_BUS should also be removed from serial8250_em485_supported so that serial core can properly manage it all.
On Sun, 03 Jul 2022 19:00:35 +0200, Lino Sanfilippo wrote: > From: Lino Sanfilippo <l.sanfilippo@kunbus.com> > > Currently the documentation claims that a maximum of 1000 msecs is allowed > for RTS delays. However nothing actually checks the values read from device > tree/ACPI and so it is possible to set much higher values. > > There is already a maximum of 100 ms enforced for RTS delays that are set > via the uart TIOCSRS485 ioctl. To be consistent with that use the same > limit for DT/ACPI values. > > Although this change is visible to userspace the risk of breaking anything > when reducing the max delays from 1000 to 100 ms should be very low, since > 100 ms is already a very high maximum for delays that are usually rather in > the usecs range. > > Signed-off-by: Lino Sanfilippo <l.sanfilippo@kunbus.com> > --- > Documentation/devicetree/bindings/serial/rs485.yaml | 4 ++-- > 1 file changed, 2 insertions(+), 2 deletions(-) > Acked-by: Rob Herring <robh@kernel.org>
From: Lino Sanfilippo <l.sanfilippo@kunbus.com> The following series includes cleanup and fixes around RS485 in the serial core and uart drivers: Patch 1: Only request the rs485 termination gpio if it is supported. NOTE: This patch follows the design decision that "rs485_supported" is set by the driver at initialization and cannot be modified afterwards. However the better approach would be to let the serial core modify the termination GPIO support setting based on the existence of a termination GPIO. If "rs485_supported" is not a read-only value any more in future the logic implemented in this patch should be adjusted accordingly. Patch 2: Set the rs485 termination GPIO in the serial core. This is needed since if the gpio is only accessible in sleepable context. It also is a further step to make the RS485 handling more generic. Patch 3: Move sanitizing of RS485 delays into an own function. This is in preparation of patch 4. Patch 4: Sanitize RS485 delays read from device tree. Patch 5: Correct RS485 delays in binding documentation. Patch 6: Remove redundant code in 8250_dwlib. Patch 7: Fix check for RS485 support. Patch 8: Remove redundant code in ar933x. Patch 9: Remove redundant code in 8250-lpc18xx. Changes in v2: - print a warning if termination GPIO is specified in DT/ACPI but is not supported by driver - fixed commit message for devtree documentation (as suggested by Andy) - fixed code comment - added patch 7 Lino Sanfilippo (9): serial: core: only get RS485 termination GPIO if supported serial: core, 8250: set RS485 termination gpio in serial core serial: core: move sanitizing of RS485 delays into own function serial: core: sanitize RS485 delays read from device tree dt_bindings: rs485: Correct delay values serial: 8250_dwlib: remove redundant sanity check for RS485 flags serial: ar933x: Fix check for RS485 support serial: ar933x: Remove redundant assignment in rs485_config serial: 8250: lpc18xx: Remove redundant sanity check for RS485 flags .../devicetree/bindings/serial/rs485.yaml | 4 +- drivers/tty/serial/8250/8250_dwlib.c | 10 +-- drivers/tty/serial/8250/8250_lpc18xx.c | 6 +- drivers/tty/serial/8250/8250_port.c | 3 - drivers/tty/serial/ar933x_uart.c | 18 ++--- drivers/tty/serial/serial_core.c | 70 +++++++++++++------ 6 files changed, 60 insertions(+), 51 deletions(-) base-commit: 7349660438603ed19282e75949561406531785a5