[PATCH 1/2] serial: imx: Add rs485 support to imx thanks to rs485 functions in serial_core

[PATCH 1/2] serial: imx: Add rs485 support to imx thanks to rs485 functions in serial_core

[Aurelien BOUIN]

This is a patch to add rs485 support with imx freescale processor
It allows to set the transmit pin used in the structure padding (rs485.padding[0])

Signed-off-by: Aurelien BOUIN <a.bouin@gmail.com>

diff --git a/drivers/tty/serial/imx.c b/drivers/tty/serial/imx.c
index 4c5e909..a086eef 100644
--- a/drivers/tty/serial/imx.c
+++ b/drivers/tty/serial/imx.c
@@ -53,6 +53,8 @@
 #include <asm/irq.h>
 #include <linux/platform_data/serial-imx.h>
 #include <linux/platform_data/dma-imx.h>
+#include <linux/uaccess.h>
+#include <linux/gpio.h>
 
 /* Register definitions */
 #define URXD0 0x0  /* Receiver Register */
@@ -280,6 +282,56 @@ static struct of_device_id imx_uart_dt_ids[] = {
 };
 MODULE_DEVICE_TABLE(of, imx_uart_dt_ids);
 
+static inline struct imx_port *to_imx_port(struct uart_port *uart)
+{
+	return container_of(uart, struct imx_port, port);
+}
+
+static void imx_rs485_stop_tx(struct imx_port *imx_uart_port)
+{
+	/*
+	 * Deactivate transmit pin configured in the structure padding
+	 */
+	gpio_set_value(imx_uart_port->port.rs485.padding[0], 0);
+}
+
+static void imx_rs485_start_tx(struct imx_port *imx_uart_port)
+{
+	/*
+	 * Activate transmit pin configured in the structure padding
+	 */
+	gpio_set_value(imx_uart_port->port.rs485.padding[0], 1);
+}
+
+/* Enable or disable the rs485 support */
+static int imx_config_rs485(struct uart_port *port,
+		struct serial_rs485 *rs485conf)
+{
+	port->rs485 = *rs485conf;
+	if (rs485conf->flags & SER_RS485_ENABLED) {
+		int ret;
+
+		dev_dbg(port->dev, "Setting UART /dev/ttymxc%d with the pin 0x%x to RS485\n",
+				port->line, port->rs485.padding[0]);
+		/*
+		 * Set the transmit pin configured in the structure padding
+		 * in GPIO output
+		 */
+		ret = gpio_request(port->rs485.padding[0], "RS485 transmit");
+		if (ret)
+			dev_err(port->dev, "Unable to request the RS485 transmit %d\n",
+					port->rs485.padding[0]);
+		gpio_direction_output(port->rs485.padding[0], 0);
+
+	} else {
+		dev_dbg(port->dev, "Setting UART /dev/ttymxc%d to RS232\n",
+				port->line);
+		if (port->rs485.padding[0])
+			gpio_free(port->rs485.padding[0]);
+	}
+	return 0;
+}
+
 static inline unsigned uts_reg(struct imx_port *sport)
 {
 	return sport->devdata->uts_reg;
@@ -580,6 +632,15 @@ static void imx_start_tx(struct uart_port *port)
 	struct imx_port *sport = (struct imx_port *)port;
 	unsigned long temp;
 
+	if (port->rs485.flags & SER_RS485_ENABLED) {
+		imx_rs485_start_tx(sport);
+		/*
+		 * Transmit complete interrupt change to receiver mode
+		 */
+		temp = readl(sport->port.membase + UCR4);
+		writel(temp | UCR4_TCEN, sport->port.membase + UCR4);
+	}
+
 	if (USE_IRDA(sport)) {
 		/* half duplex in IrDA mode; have to disable receive mode */
 		temp = readl(sport->port.membase + UCR4);
@@ -693,8 +754,10 @@ static irqreturn_t imx_rxint(int irq, void *dev_id)
 					goto out;
 				continue;
 			}
-
-			rx &= sport->port.read_status_mask;
+			/*
+			 * Preserve characters with parity or framing errors
+			 */
+			rx &= sport->port.read_status_mask | 0xFF;
 
 			if (rx & URXD_BRK)
 				flg = TTY_BREAK;
@@ -747,8 +810,30 @@ static irqreturn_t imx_int(int irq, void *dev_id)
 	struct imx_port *sport = dev_id;
 	unsigned int sts;
 	unsigned int sts2;
+	unsigned int cr1, cr2, cr3, cr4;
 
 	sts = readl(sport->port.membase + USR1);
+	sts2 = readl(sport->port.membase + USR2);
+	cr1 = readl(sport->port.membase + UCR1);
+	cr2 = readl(sport->port.membase + UCR2);
+	cr3 = readl(sport->port.membase + UCR3);
+	cr4 = readl(sport->port.membase + UCR4);
+
+	if (sport->port.rs485.flags & SER_RS485_ENABLED)	{
+		/*
+		 * Check if the transmit is complete
+		 */
+		if ((cr4 & UCR4_TCEN) && (sts2 & USR2_TXDC)) {
+			unsigned long temp;
+
+			imx_rs485_stop_tx(sport);
+			/*
+			 * Transmit complete interrupt disabled
+			 */
+			temp = readl(sport->port.membase + UCR4);
+			writel(temp & ~UCR4_TCEN, sport->port.membase + UCR4);
+		}
+	}
 
 	if (sts & USR1_RRDY) {
 		if (sport->dma_is_enabled)
@@ -1072,6 +1157,17 @@ static int imx_startup(struct uart_port *port)
 	struct imx_port *sport = (struct imx_port *)port;
 	int retval, i;
 	unsigned long flags, temp;
+	int reset_time = 100;
+
+	/* reset fifo's and state machines to be sure
+	 * to start the UART in correct conditions */
+	temp = readl(sport->port.membase + UCR2);
+	temp &= ~UCR2_SRST;
+	writel(temp, sport->port.membase + UCR2);
+	while (!(readl(sport->port.membase + UCR2) & UCR2_SRST) &&
+	    (--reset_time > 0)) {
+		udelay(1);
+	}
 
 	retval = clk_prepare_enable(sport->clk_per);
 	if (retval)
@@ -1225,6 +1321,14 @@ static void imx_shutdown(struct uart_port *port)
 
 	writel(temp, sport->port.membase + UCR1);
 	spin_unlock_irqrestore(&sport->port.lock, flags);
+	/*
+	 * Deactivate RS485 mode
+	 */
+	if (port->rs485.flags & SER_RS485_ENABLED) {
+		imx_rs485_stop_tx(sport);
+		port->rs485.flags &= ~SER_RS485_ENABLED;
+		imx_config_rs485(port, &port->rs485);
+	}
 
 	clk_disable_unprepare(sport->clk_per);
 	clk_disable_unprepare(sport->clk_ipg);
@@ -1318,8 +1422,10 @@ imx_set_termios(struct uart_port *port, struct ktermios *termios,
 	 * Characters to ignore
 	 */
 	sport->port.ignore_status_mask = 0;
-	if (termios->c_iflag & IGNPAR)
-		sport->port.ignore_status_mask |= URXD_PRERR;
+	if (termios->c_iflag & IGNPAR) {
+		/* Ignore Framing errors when IGNPAR is set */
+		sport->port.ignore_status_mask |= URXD_PRERR | URXD_FRMERR;
+	}
 	if (termios->c_iflag & IGNBRK) {
 		sport->port.ignore_status_mask |= URXD_BRK;
 		/*
@@ -1560,8 +1666,8 @@ static void
 imx_console_write(struct console *co, const char *s, unsigned int count)
 {
 	struct imx_port *sport = imx_ports[co->index];
-	struct imx_port_ucrs old_ucr;
 	unsigned int ucr1;
+	unsigned int old_ucr1, old_ucr2;
 	unsigned long flags = 0;
 	int locked = 1;
 	int retval;
@@ -1583,10 +1689,10 @@ imx_console_write(struct console *co, const char *s, unsigned int count)
 		spin_lock_irqsave(&sport->port.lock, flags);
 
 	/*
-	 *	First, save UCR1/2/3 and then disable interrupts
+	 *	First, save UCR1/2 and then disable interrupts
 	 */
-	imx_port_ucrs_save(&sport->port, &old_ucr);
-	ucr1 = old_ucr.ucr1;
+	ucr1 = old_ucr1 = readl(sport->port.membase + UCR1);
+	old_ucr2 = readl(sport->port.membase + UCR2);
 
 	if (is_imx1_uart(sport))
 		ucr1 |= IMX1_UCR1_UARTCLKEN;
@@ -1595,17 +1701,18 @@ imx_console_write(struct console *co, const char *s, unsigned int count)
 
 	writel(ucr1, sport->port.membase + UCR1);
 
-	writel(old_ucr.ucr2 | UCR2_TXEN, sport->port.membase + UCR2);
+	writel(old_ucr2 | UCR2_TXEN, sport->port.membase + UCR2);
 
 	uart_console_write(&sport->port, s, count, imx_console_putchar);
 
 	/*
 	 *	Finally, wait for transmitter to become empty
-	 *	and restore UCR1/2/3
+	 *	and restore UCR1/2
 	 */
 	while (!(readl(sport->port.membase + USR2) & USR2_TXDC));
 
-	imx_port_ucrs_restore(&sport->port, &old_ucr);
+	writel(old_ucr1, sport->port.membase + UCR1);
+	writel(old_ucr2, sport->port.membase + UCR2);
 
 	if (locked)
 		spin_unlock_irqrestore(&sport->port.lock, flags);
@@ -1873,6 +1980,7 @@ static int serial_imx_probe(struct platform_device *pdev)
 	sport->port.type = PORT_IMX,
 	sport->port.iotype = UPIO_MEM;
 	sport->port.irq = platform_get_irq(pdev, 0);
+	sport->port.rs485_config = imx_config_rs485;
 	sport->rxirq = platform_get_irq(pdev, 0);
 	sport->txirq = platform_get_irq(pdev, 1);
 	sport->rtsirq = platform_get_irq(pdev, 2);
-- 
1.9.1

[PATCH 2/2] serial: imx: checkpatch cleanup

[Aurelien BOUIN]

This patch solve various issue detected by checkpatch.pl

Signed-off-by: Aurelien BOUIN <a.bouin@gmail.com>

diff --git a/drivers/tty/serial/imx.c b/drivers/tty/serial/imx.c
index a086eef..63eb457 100644
--- a/drivers/tty/serial/imx.c
+++ b/drivers/tty/serial/imx.c
@@ -275,9 +275,12 @@ static struct platform_device_id imx_uart_devtype[] = {
 MODULE_DEVICE_TABLE(platform, imx_uart_devtype);
 
 static struct of_device_id imx_uart_dt_ids[] = {
-	{ .compatible = "fsl,imx6q-uart", .data = &imx_uart_devdata[IMX6Q_UART], },
-	{ .compatible = "fsl,imx1-uart", .data = &imx_uart_devdata[IMX1_UART], },
-	{ .compatible = "fsl,imx21-uart", .data = &imx_uart_devdata[IMX21_UART], },
+	{ .compatible = "fsl,imx6q-uart",
+			.data = &imx_uart_devdata[IMX6Q_UART], },
+	{ .compatible = "fsl,imx1-uart",
+			.data = &imx_uart_devdata[IMX1_UART], },
+	{ .compatible = "fsl,imx21-uart",
+			.data = &imx_uart_devdata[IMX21_UART], },
 	{ /* sentinel */ }
 };
 MODULE_DEVICE_TABLE(of, imx_uart_dt_ids);
@@ -430,6 +433,7 @@ static void imx_stop_tx(struct uart_port *port)
 	if (USE_IRDA(sport)) {
 		/* half duplex - wait for end of transmission */
 		int n = 256;
+
 		while ((--n > 0) &&
 		      !(readl(sport->port.membase + USR2) & USR2_TXDC)) {
 			udelay(5);
@@ -621,7 +625,6 @@ static void imx_dma_tx(struct imx_port *sport)
 	sport->dma_is_txing = 1;
 	dmaengine_submit(desc);
 	dma_async_issue_pending(chan);
-	return;
 }
 
 /*
@@ -870,7 +873,8 @@ static unsigned int imx_tx_empty(struct uart_port *port)
 	struct imx_port *sport = (struct imx_port *)port;
 	unsigned int ret;
 
-	ret = (readl(sport->port.membase + USR2) & USR2_TXDC) ?  TIOCSER_TEMT : 0;
+	ret = (readl(sport->port.membase + USR2) & USR2_TXDC) ?
+			TIOCSER_TEMT : 0;
 
 	/* If the TX DMA is working, return 0. */
 	if (sport->dma_is_enabled && sport->dma_is_txing)
@@ -1256,6 +1260,7 @@ static int imx_startup(struct uart_port *port)
 
 	if (USE_IRDA(sport)) {
 		struct imxuart_platform_data *pdata;
+
 		pdata = dev_get_platdata(sport->port.dev);
 		sport->irda_inv_rx = pdata->irda_inv_rx;
 		sport->irda_inv_tx = pdata->irda_inv_tx;
@@ -1299,6 +1304,7 @@ static void imx_shutdown(struct uart_port *port)
 
 	if (USE_IRDA(sport)) {
 		struct imxuart_platform_data *pdata;
+
 		pdata = dev_get_platdata(sport->port.dev);
 		if (pdata->irda_enable)
 			pdata->irda_enable(0);
@@ -1754,7 +1760,8 @@ imx_console_get_options(struct imx_port *sport, int *baud,
 		ubir = readl(sport->port.membase + UBIR) & 0xffff;
 		ubmr = readl(sport->port.membase + UBMR) & 0xffff;
 
-		ucfr_rfdiv = (readl(sport->port.membase + UFCR) & UFCR_RFDIV) >> 7;
+		ucfr_rfdiv = (readl(sport->port.membase + UFCR) & UFCR_RFDIV)
+				>> 7;
 		if (ucfr_rfdiv == 6)
 			ucfr_rfdiv = 7;
 		else
@@ -1763,12 +1770,13 @@ imx_console_get_options(struct imx_port *sport, int *baud,
 		uartclk = clk_get_rate(sport->clk_per);
 		uartclk /= ucfr_rfdiv;
 
-		{	/*
-			 * The next code provides exact computation of
-			 *   baud_raw = round(((uartclk/16) * (ubir + 1)) / (ubmr + 1))
-			 * without need of float support or long long division,
-			 * which would be required to prevent 32bit arithmetic overflow
-			 */
+		{
+		/*
+		 * The next code provides exact computation of
+		 *   baud_raw = round(((uartclk/16) * (ubir + 1)) / (ubmr + 1))
+		 * without need of float support or long long division,
+		 * which would be required to prevent 32bit arithmetic overflow
+		 */
 			unsigned int mul = ubir + 1;
 			unsigned int div = 16 * (ubmr + 1);
 			unsigned int rem = uartclk % div;
@@ -1844,7 +1852,7 @@ static struct console imx_console = {
 	.data		= &imx_reg,
 };
 
-#define IMX_CONSOLE	&imx_console
+#define IMX_CONSOLE	(&imx_console)
 #else
 #define IMX_CONSOLE	NULL
 #endif
-- 
1.9.1

Re: [PATCH 1/2] serial: imx: Add rs485 support to imx thanks to rs485 functions in serial_core

[Alexander Stein]

Hello,

On Wednesday 11 February 2015 16:30:34, Aurelien BOUIN wrote:
> This is a patch to add rs485 support with imx freescale processor
> It allows to set the transmit pin used in the structure padding (rs485.padding[0])

Nice idea to use the padding for GPIO configuration. I've done an implementation using board-specific call-backs which doesn't work with DT.

> Signed-off-by: Aurelien BOUIN <a.bouin@gmail.com>
> 
> diff --git a/drivers/tty/serial/imx.c b/drivers/tty/serial/imx.c
> index 4c5e909..a086eef 100644
> --- a/drivers/tty/serial/imx.c
> +++ b/drivers/tty/serial/imx.c
> @@ -53,6 +53,8 @@
>  #include <asm/irq.h>
>  #include <linux/platform_data/serial-imx.h>
>  #include <linux/platform_data/dma-imx.h>
> +#include <linux/uaccess.h>
> +#include <linux/gpio.h>
>  
>  /* Register definitions */
>  #define URXD0 0x0  /* Receiver Register */
> @@ -280,6 +282,56 @@ static struct of_device_id imx_uart_dt_ids[] = {
>  };
>  MODULE_DEVICE_TABLE(of, imx_uart_dt_ids);
>  
> +static inline struct imx_port *to_imx_port(struct uart_port *uart)
> +{
> +	return container_of(uart, struct imx_port, port);
> +}
> +
> +static void imx_rs485_stop_tx(struct imx_port *imx_uart_port)
> +{
> +	/*
> +	 * Deactivate transmit pin configured in the structure padding
> +	 */
> +	gpio_set_value(imx_uart_port->port.rs485.padding[0], 0);
> +}
> +
> +static void imx_rs485_start_tx(struct imx_port *imx_uart_port)
> +{
> +	/*
> +	 * Activate transmit pin configured in the structure padding
> +	 */
> +	gpio_set_value(imx_uart_port->port.rs485.padding[0], 1);
> +}

Please add the feature to support active-low GPIOs, e.g. using rs485.padding[1].

> +/* Enable or disable the rs485 support */
> +static int imx_config_rs485(struct uart_port *port,
> +		struct serial_rs485 *rs485conf)
> +{
> +	port->rs485 = *rs485conf;
> +	if (rs485conf->flags & SER_RS485_ENABLED) {
> +		int ret;
> +
> +		dev_dbg(port->dev, "Setting UART /dev/ttymxc%d with the pin 0x%x to RS485\n",
> +				port->line, port->rs485.padding[0]);
> +		/*
> +		 * Set the transmit pin configured in the structure padding
> +		 * in GPIO output
> +		 */
> +		ret = gpio_request(port->rs485.padding[0], "RS485 transmit");
> +		if (ret)
> +			dev_err(port->dev, "Unable to request the RS485 transmit %d\n",
> +					port->rs485.padding[0]);
> +		gpio_direction_output(port->rs485.padding[0], 0);
> +
> +	} else {
> +		dev_dbg(port->dev, "Setting UART /dev/ttymxc%d to RS232\n",
> +				port->line);
> +		if (port->rs485.padding[0])
> +			gpio_free(port->rs485.padding[0]);
> +	}
> +	return 0;
> +}
> +
>  static inline unsigned uts_reg(struct imx_port *sport)
>  {
>  	return sport->devdata->uts_reg;
> @@ -580,6 +632,15 @@ static void imx_start_tx(struct uart_port *port)
>  	struct imx_port *sport = (struct imx_port *)port;
>  	unsigned long temp;
>  
> +	if (port->rs485.flags & SER_RS485_ENABLED) {
> +		imx_rs485_start_tx(sport);
> +		/*
> +		 * Transmit complete interrupt change to receiver mode
> +		 */
> +		temp = readl(sport->port.membase + UCR4);
> +		writel(temp | UCR4_TCEN, sport->port.membase + UCR4);
> +	}
> +

If you run a RT-PREEMPT kernel, interrupts aren't actually disabled here. So when you enable the TXDC interrupt here, it will fire up immediately.
Move that after calling imx_transmit_buffer so you are sure there are actually characters inserted.

>  	if (USE_IRDA(sport)) {
>  		/* half duplex in IrDA mode; have to disable receive mode */
>  		temp = readl(sport->port.membase + UCR4);
> @@ -693,8 +754,10 @@ static irqreturn_t imx_rxint(int irq, void *dev_id)
>  					goto out;
>  				continue;
>  			}
> -
> -			rx &= sport->port.read_status_mask;
> +			/*
> +			 * Preserve characters with parity or framing errors
> +			 */
> +			rx &= sport->port.read_status_mask | 0xFF;
>  
>  			if (rx & URXD_BRK)
>  				flg = TTY_BREAK;
> @@ -747,8 +810,30 @@ static irqreturn_t imx_int(int irq, void *dev_id)
>  	struct imx_port *sport = dev_id;
>  	unsigned int sts;
>  	unsigned int sts2;
> +	unsigned int cr1, cr2, cr3, cr4;
>  
>  	sts = readl(sport->port.membase + USR1);
> +	sts2 = readl(sport->port.membase + USR2);
> +	cr1 = readl(sport->port.membase + UCR1);
> +	cr2 = readl(sport->port.membase + UCR2);
> +	cr3 = readl(sport->port.membase + UCR3);
> +	cr4 = readl(sport->port.membase + UCR4);
> +
> +	if (sport->port.rs485.flags & SER_RS485_ENABLED)	{
> +		/*
> +		 * Check if the transmit is complete
> +		 */
> +		if ((cr4 & UCR4_TCEN) && (sts2 & USR2_TXDC)) {

Can this actually happen that UCR4_TCEN is unset when in S485 mode?

Best regards,
Alexander
-- 
Dipl.-Inf. Alexander Stein

SYS TEC electronic GmbH
Am Windrad 2
08468 Heinsdorfergrund
Tel.: 03765 38600-1156
Fax: 03765 38600-4100
Email: alexander.stein@systec-electronic.com
Website: www.systec-electronic.com
 
Managing Director: Dipl.-Phys. Siegmar Schmidt
Commercial registry: Amtsgericht Chemnitz, HRB 28082