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

2014-07-09 Thread Sebastian Andrzej Siewior
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 
---
 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 
 #include 
 #include 
+#include 
+#include 
 #ifdef CONFIG_SPARC
 #include 
 #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);
+   }
+   }
+
if (p->ier & UART_IER_THRI) {
p->ier &= ~UART_IER_THRI;
serial_out(p, UART_IER, p->ier);
}
+
+   if ((p->rs485.flags & SER_RS485_ENABLED) &&
+   !(p->rs485.flags & SER_RS485_RX_DURING_TX)) {
+   /*
+* Empty the RX FIFO, we are not interested in anything
+* received during the half-duplex transmission.
+*/
+   serial_out(p, UART_FCR, p->fcr | UART_FCR_CLEAR_RCVR);
+   /* Re-enable RX interrupts */
+   p->ier |= UART_IER_RLSI | UART_IER_RDI;
+   p->port.read_status_mask |= UART_LSR_DR;
+   serial_out(p, UART_IER, p->ier);
+   }
 }
 
 static void serial8250_stop_tx(struct uart_port *port)
@@ -1330,6 +1356,20 @@ static void serial8250_start_tx(struct uart_port *port)
if (up->dma && !serial8250_tx_dma(up)) {
goto out;
} else if (!(up->ier & UART_IER_THRI)) {
+
+   if (up->rs485.flags & SER_RS485_ENABLED) {
+   int ret;
+
+   ret = (up->rs485.flags & SER_RS485_RTS_ON_SEND) ? 1 : 0;
+   if (gpio_get_value(up->rts_gpio) != ret) {
+   gpio_set_value(up->rts_gpio, ret);
+   if (up->rs485.delay_rts_before_send > 0)
+   mdelay(up->rs485.delay_rts_before_send);
+   }
+   if (!(up->rs485.flags & SER_RS485_RX_DURING_TX))
+   serial8250_stop_rx(port);
+   }
+
up->ier |= UART_IER_THRI;
serial_port_out(port, UART_IER, up->ier);
 
@@ -2556,6 +2596,7 @@ serial8250_do_set_termios(struct uart_port *port, struct 
ktermios *termios,
serial_port_out(port, UART_FCR, UART_FCR_ENABLE_FIFO);
serial_port_out(port, UART_FCR, fcr);   /* set fcr */
}
+   up->fcr = fcr;
serial8250_set_mctrl(port, port->mctrl);
spin_unlock_irqrestore(&port->lock, flags);
pm_runtime_mark_last_busy(port->dev);
@@ -2811,6 +2852,124 @@ serial8250_verify_port(struct uart_port *port, struct 
serial_struct *ser)
return 0;
 }
 
+int serial8250_probe_rs485(struct uart_8250_port *up,
+   struct device *dev)
+{
+   struct serial_rs485 *rs485conf = &up->rs485;
+   struct device_node *np = dev->of_node;
+   u32 rs485_delay[2];
+   enum of_gpio_flags flags;
+   int ret;
+
+   rs485conf->flags = 0;
+   if (!np)
+   return 0;
+
+   /* check for tx enable gpio */
+   up->rts_gpio = of_get_named_gpio_flags(np, "rts-gpio", 0, &flags);
+   if (up->rts_gpio == -EPROBE_DEFER)
+   return -EPROBE_DEFER;
+   if (!gpio_is_valid(up->rts_gpio))
+   return 0;
+
+   ret = devm_gpio_request(dev, up->rts_gpio, "serial_rts");
+   if (ret < 0)
+   return ret;
+   ret = gpio_direction_output(up->rts_gpio,
+   flags & SER_RS485_RTS_AFTER_SEND);
+   if (ret < 0)
+   return ret;
+
+   up->rts_gpio_

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

2014-07-09 Thread Lennart Sorensen
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 
> ---
>  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 
>  #include 
>  #include 
> +#include 
> +#include 
>  #ifdef CONFIG_SPARC
>  #include 
>  #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-omap" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


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

2014-07-10 Thread One Thousand Gnomes
>  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);
> + }

RTS is not normally a GPIO. We should be controlling the UART RTS here,
and if the UART has a magic special case RTS wired to a GPIO then really
the hardware specific part should handle that gunge. I don't care whether
the drive does it via serial_out magic or a more explicit hook but it
doesn't belong here in core code.

Likewise the mdelay probably should be in the device specific bits or
controlled by a flag as not all hardware is so braindead.

> @@ -1330,6 +1356,20 @@ static void serial8250_start_tx(struct uart_port *port)
>   if (up->dma && !serial8250_tx_dma(up)) {

Ditto

> +int serial8250_probe_rs485(struct uart_8250_port *up,
> + struct device *dev)
> +{
> + struct serial_rs485 *rs485conf = &up->rs485;
> + struct device_node *np = dev->of_node;
> + u32 rs485_delay[2];
> + enum of_gpio_flags flags;
> + int ret;
> +
> + rs485conf->flags = 0;
> + if (!np)
> + return 0;
> +
> + /* check for tx enable gpio */
> + up->rts_gpio = of_get_named_gpio_flags(np, "rts-gpio", 0, &flags);

No of_ dependencies in core 8250.c either please. That looks a perfectly
good implementation of serial8250_of_probe_rs485 however, just belongs in
the right place.

> +static int serial8250_ioctl(struct uart_port *port, unsigned int cmd,
> + unsigned long arg)
> +{
> + struct serial_rs485 rs485conf;
> + struct uart_8250_port *up;
> +
> + up = container_of(port, struct uart_8250_port, port);
> + switch (cmd) {
> + case TIOCSRS485:
> + if (!gpio_is_valid(up->rts_gpio))
> + return -ENODEV;

GPIO assumption again needs to go


> diff --git a/include/linux/serial_8250.h b/include/linux/serial_8250.h
> index 0ec21ec..056a73f 100644
> --- a/include/linux/serial_8250.h
> +++ b/include/linux/serial_8250.h
> @@ -78,6 +78,7 @@ struct uart_8250_port {
>   unsigned char   acr;
>   unsigned char   ier;
>   unsigned char   lcr;
> + unsigned char   fcr;
>   unsigned char   mcr;
>   unsigned char   mcr_mask;   /* mask of user bits */
>   unsigned char   mcr_force;  /* mask of forced bits */
> @@ -94,6 +95,9 @@ struct uart_8250_port {
>   unsigned char   msr_saved_flags;
>  
>   struct uart_8250_dma*dma;
> + struct serial_rs485 rs485;
> + int rts_gpio;
> + boolrts_gpio_valid;

Keeping the gpio here doesn't look unreasonable if one is in use.

Alan
--
To unsubscribe from this list: send the line "unsubscribe linux-omap" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


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

2014-07-10 Thread Sebastian Andrzej Siewior
On 07/09/2014 09:01 PM, Lennart Sorensen wrote:
>> 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
>> @@ -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.

My understanding is that this is not baudrate related. This is the
delay that the hardware (the transceiver) to perform the change and
work.
There is the ioctl() interface which can change the delay, too. The
only required thing is the gpio.

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

Sebastian
--
To unsubscribe from this list: send the line "unsubscribe linux-omap" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html