linux-kernel - Re: [PATCH 5/6] tty: serial: 8250-core: add rs485 support

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <20140709190143.GF17769@csclub.uwaterloo.ca>
Date:	Wed, 9 Jul 2014 15:01:44 -0400
From:	"Lennart Sorensen" <lsorense@...lub.uwaterloo.ca>
To:	Sebastian Andrzej Siewior <bigeasy@...utronix.de>
Cc:	linux-omap@...r.kernel.org, linux-arm-kernel@...ts.infradead.org,
	Tony Lindgren <tony@...mide.com>, Felipe Balbi <balbi@...com>,
	linux-kernel@...r.kernel.org, linux-serial@...r.kernel.org
Subject: Re: [PATCH 5/6] tty: serial: 8250-core: add rs485 support

On Wed, Jul 09, 2014 at 07:49:36PM +0200, Sebastian Andrzej Siewior wrote:
> So after I stuffed the rs485 support from the omap-serial into
> 8250-omap, I've been looking at it and the only omap specific part was
> the OMAP_UART_SCR_TX_EMPTY part. The driver has always TX_EMPTY set
> because the 8250 core expects an interrupt after the TX fifo + shift
> register is empty. The rs485 parts seems to wait for half fifo and then
> again for the empty fifo. With this change gone it fits fine as generic
> code and here it is.
> 
> It is expected that the potential rs485 user invokes
> serial_omap_probe_rs485() to configure atleast RTS gpio which is a must
> have property. The code has been taken from omap-serial driver and
> received limited tested due to -ENODEV (the compiler said it works).
> 
> Signed-off-by: Sebastian Andrzej Siewior <bigeasy@...utronix.de>
> ---
>  drivers/tty/serial/8250/8250_core.c | 171 ++++++++++++++++++++++++++++++++++++
>  include/linux/serial_8250.h         |   6 ++
>  2 files changed, 177 insertions(+)
> 
> diff --git a/drivers/tty/serial/8250/8250_core.c b/drivers/tty/serial/8250/8250_core.c
> index c7c3bf7..bf06a4c 100644
> --- a/drivers/tty/serial/8250/8250_core.c
> +++ b/drivers/tty/serial/8250/8250_core.c
> @@ -39,6 +39,8 @@
>  #include <linux/mutex.h>
>  #include <linux/slab.h>
>  #include <linux/pm_runtime.h>
> +#include <linux/gpio.h>
> +#include <linux/of_gpio.h>
>  #ifdef CONFIG_SPARC
>  #include <linux/sunserialcore.h>
>  #endif
> @@ -1281,10 +1283,34 @@ static void autoconfig_irq(struct uart_8250_port *up)
>  
>  static inline void __stop_tx(struct uart_8250_port *p)
>  {
> +	if (p->rs485.flags & SER_RS485_ENABLED) {
> +		int ret;
> +
> +		ret = (p->rs485.flags & SER_RS485_RTS_AFTER_SEND) ? 1 : 0;
> +		if (gpio_get_value(p->rts_gpio) != ret) {
> +			if (p->rs485.delay_rts_after_send > 0)
> +				mdelay(p->rs485.delay_rts_after_send);
> +			gpio_set_value(p->rts_gpio, ret);

Usually the delay for RS485 is done in bit times, not msec.  Not sure
how you expect this to work.  Not sure doing it in software is precise
enough either.  It probably should be calculated based on the current
baudrate with a bit time rather than msec in the DT data.  No one wants
to have to change the DT data to change the baud rate.  After all this
is very often used with modbus and the modbus rules specify turn around
times in bit times.

I hope TI puts this into the UART in future designs where it belongs
(similar to what Exar and many others already did).

-- 
Len Sorensen
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/