diff mbox

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

Message ID 1404928177-26554-6-git-send-email-bigeasy@linutronix.de (mailing list archive)
State New, archived
Headers show

Commit Message

Sebastian Andrzej Siewior July 9, 2014, 5:49 p.m. UTC
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@linutronix.de>
---
 drivers/tty/serial/8250/8250_core.c | 171 ++++++++++++++++++++++++++++++++++++
 include/linux/serial_8250.h         |   6 ++
 2 files changed, 177 insertions(+)

Comments

Lennart Sorensen July 9, 2014, 7:01 p.m. UTC | #1
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@linutronix.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).
Alan Cox July 10, 2014, 2:52 p.m. UTC | #2
>  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;
> +	bool			rts_gpio_valid;

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

Alan
Sebastian Andrzej Siewior July 10, 2014, 3:55 p.m. UTC | #3
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
diff mbox

Patch

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);
+		}
+	}
+
 	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_valid = true;
+
+	if (of_property_read_bool(np, "rs485-rts-active-high"))
+		rs485conf->flags |= SER_RS485_RTS_ON_SEND;
+	else
+		rs485conf->flags |= SER_RS485_RTS_AFTER_SEND;
+
+	if (of_property_read_u32_array(np, "rs485-rts-delay",
+				rs485_delay, 2) == 0) {
+		rs485conf->delay_rts_before_send = rs485_delay[0];
+		rs485conf->delay_rts_after_send = rs485_delay[1];
+	}
+
+	if (of_property_read_bool(np, "rs485-rx-during-tx"))
+		rs485conf->flags |= SER_RS485_RX_DURING_TX;
+
+	if (of_property_read_bool(np, "linux,rs485-enabled-at-boot-time"))
+		rs485conf->flags |= SER_RS485_ENABLED;
+
+	return 0;
+}
+EXPORT_SYMBOL_GPL(serial8250_probe_rs485);
+
+static void serial8250_config_rs485(struct uart_port *port,
+		struct serial_rs485 *rs485conf)
+{
+	struct uart_8250_port *up =
+		container_of(port, struct uart_8250_port, port);
+	unsigned long flags;
+	unsigned int mode;
+	int val;
+
+	pm_runtime_get_sync(port->dev);
+	spin_lock_irqsave(&up->port.lock, flags);
+
+	/* Disable interrupts from this port */
+	mode = up->ier;
+	up->ier = 0;
+	serial_out(up, UART_IER, 0);
+
+	/* store new config */
+	up->rs485 = *rs485conf;
+
+	/* enable / disable rts */
+	val = (up->rs485.flags & SER_RS485_ENABLED) ?
+		SER_RS485_RTS_AFTER_SEND : SER_RS485_RTS_ON_SEND;
+	val = (up->rs485.flags & val) ? 1 : 0;
+	gpio_set_value(up->rts_gpio, val);
+
+	/* Enable interrupts */
+	up->ier = mode;
+	serial_out(up, UART_IER, up->ier);
+
+	spin_unlock_irqrestore(&up->port.lock, flags);
+	pm_runtime_mark_last_busy(port->dev);
+	pm_runtime_put_autosuspend(port->dev);
+}
+
+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;
+		if (copy_from_user(&rs485conf, (void __user *) arg,
+					sizeof(rs485conf)))
+			return -EFAULT;
+
+		serial8250_config_rs485(port, &rs485conf);
+		break;
+
+	case TIOCGRS485:
+		if (!gpio_is_valid(up->rts_gpio))
+			return -ENODEV;
+		if (copy_to_user((void __user *) arg, &up->rs485,
+					sizeof(rs485conf)))
+			return -EFAULT;
+		break;
+
+	default:
+		return -ENOIOCTLCMD;
+	}
+	return 0;
+}
+
 static const char *
 serial8250_type(struct uart_port *port)
 {
@@ -2840,6 +2999,7 @@  static struct uart_ops serial8250_pops = {
 	.request_port	= serial8250_request_port,
 	.config_port	= serial8250_config_port,
 	.verify_port	= serial8250_verify_port,
+	.ioctl		= serial8250_ioctl,
 #ifdef CONFIG_CONSOLE_POLL
 	.poll_get_char = serial8250_get_poll_char,
 	.poll_put_char = serial8250_put_poll_char,
@@ -3375,6 +3535,17 @@  int serial8250_register_8250_port(struct uart_8250_port *up)
 		uart->tx_loadsz		= up->tx_loadsz;
 		uart->capabilities	= up->capabilities;
 
+		/*
+		 * gpio_is_valid() is nice but if this struct wasn't initialized
+		 * then it is 0 which is considered as valid. With the ioctl()
+		 * which can enable the rs485 routine we could abuse a gpio.
+		 */
+		if (up->rts_gpio_valid) {
+			uart->rs485		= up->rs485;
+			uart->rts_gpio		= up->rts_gpio;
+		} else
+			uart->rts_gpio = -EINVAL;
+
 		/* Take tx_loadsz from fifosize if it wasn't set separately */
 		if (uart->port.fifosize && !uart->tx_loadsz)
 			uart->tx_loadsz = uart->port.fifosize;
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;
+	bool			rts_gpio_valid;
 
 	/* 8250 specific callbacks */
 	int			(*dl_read)(struct uart_8250_port *);
@@ -107,6 +111,8 @@  void serial8250_resume_port(int line);
 
 extern int early_serial_setup(struct uart_port *port);
 
+extern int serial8250_probe_rs485(struct uart_8250_port *up,
+		struct device *dev);
 extern int serial8250_find_port(struct uart_port *p);
 extern int serial8250_find_port_for_earlycon(void);
 extern unsigned int serial8250_early_in(struct uart_port *port, int offset);