[PATCH 0/3] Add RS-485 support and fix RTS/CTS implementation

[PATCH 0/3] Add RS-485 support and fix RTS/CTS implementation

[Bhuvanchandra DV]

- Add pinmux for UART_0 RTS/CTS pins.

- Since hardware can handle RTS/CTS flow control automatically,
  remove manual control of RTS/CTS from set/get mctrl.

- Add RS-485 support for Vybird platform.


Bhuvanchandra DV (3):
  ARM: dts: colibri-vf: Add pinmux for UART_0 aka UART_A RTS/CTS pins
  tty: serial: fsl_lpuart: remove RTS/CTS control from set/get_mctrl
  tty: serial: fsl_lpuart: Add support for RS-485

 arch/arm/boot/dts/vf-colibri.dtsi |   2 +
 drivers/tty/serial/fsl_lpuart.c   | 112 +++++++++++++++++++++++---------------
 2 files changed, 70 insertions(+), 44 deletions(-)

-- 
2.1.0

[PATCH 1/3] ARM: dts: colibri-vf: Add pinmux for UART_0 aka UART_A RTS/CTS pins

[Bhuvanchandra DV]

Signed-off-by: Bhuvanchandra DV <bhuvanchandra.dv@toradex.com>
---
 arch/arm/boot/dts/vf-colibri.dtsi | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/arch/arm/boot/dts/vf-colibri.dtsi b/arch/arm/boot/dts/vf-colibri.dtsi
index 68ca125..ad6c5ca 100644
--- a/arch/arm/boot/dts/vf-colibri.dtsi
+++ b/arch/arm/boot/dts/vf-colibri.dtsi
@@ -174,6 +174,8 @@
 			fsl,pins = <
 				VF610_PAD_PTB10__UART0_TX		0x21a2
 				VF610_PAD_PTB11__UART0_RX		0x21a1
+				VF610_PAD_PTB12__UART0_RTS		0x21a2
+				VF610_PAD_PTB13__UART0_CTS		0x21a1
 			>;
 		};
 
-- 
2.1.0

[PATCH 2/3] tty: serial: fsl_lpuart: remove RTS/CTS control from set/get_mctrl

[Bhuvanchandra DV]

The LPUART does not provide manual control of RTS/CTS signals,
those can only be controlled by the hardware directly. Therefore
manual control of those signals through mctrl can not be provided.
The current implementation enables/disables the automatic control,
which is not what mctrl should do, hence remove the incorrect
implementation.

Signed-off-by: Bhuvanchandra DV <bhuvanchandra.dv@toradex.com>
---
 drivers/tty/serial/fsl_lpuart.c | 56 ++++++++---------------------------------
 1 file changed, 10 insertions(+), 46 deletions(-)

diff --git a/drivers/tty/serial/fsl_lpuart.c b/drivers/tty/serial/fsl_lpuart.c
index 08ce76f..357f623 100644
--- a/drivers/tty/serial/fsl_lpuart.c
+++ b/drivers/tty/serial/fsl_lpuart.c
@@ -822,64 +822,28 @@ static unsigned int lpuart32_tx_empty(struct uart_port *port)
 
 static unsigned int lpuart_get_mctrl(struct uart_port *port)
 {
-	unsigned int temp = 0;
-	unsigned char reg;
-
-	reg = readb(port->membase + UARTMODEM);
-	if (reg & UARTMODEM_TXCTSE)
-		temp |= TIOCM_CTS;
-
-	if (reg & UARTMODEM_RXRTSE)
-		temp |= TIOCM_RTS;
-
-	return temp;
+	return TIOCM_CAR | TIOCM_CTS | TIOCM_DSR;
 }
 
 static unsigned int lpuart32_get_mctrl(struct uart_port *port)
 {
-	unsigned int temp = 0;
-	unsigned long reg;
-
-	reg = lpuart32_read(port->membase + UARTMODIR);
-	if (reg & UARTMODIR_TXCTSE)
-		temp |= TIOCM_CTS;
-
-	if (reg & UARTMODIR_RXRTSE)
-		temp |= TIOCM_RTS;
-
-	return temp;
+	return TIOCM_CAR | TIOCM_CTS | TIOCM_DSR;
 }
 
 static void lpuart_set_mctrl(struct uart_port *port, unsigned int mctrl)
 {
-	unsigned char temp;
-
-	temp = readb(port->membase + UARTMODEM) &
-			~(UARTMODEM_RXRTSE | UARTMODEM_TXCTSE);
-
-	if (mctrl & TIOCM_RTS)
-		temp |= UARTMODEM_RXRTSE;
-
-	if (mctrl & TIOCM_CTS)
-		temp |= UARTMODEM_TXCTSE;
-
-	writeb(temp, port->membase + UARTMODEM);
+    /*
+     * CTS/RTS can _only_ be handled
+     * automatically by the hardware.
+     */
 }
 
 static void lpuart32_set_mctrl(struct uart_port *port, unsigned int mctrl)
 {
-	unsigned long temp;
-
-	temp = lpuart32_read(port->membase + UARTMODIR) &
-			~(UARTMODIR_RXRTSE | UARTMODIR_TXCTSE);
-
-	if (mctrl & TIOCM_RTS)
-		temp |= UARTMODIR_RXRTSE;
-
-	if (mctrl & TIOCM_CTS)
-		temp |= UARTMODIR_TXCTSE;
-
-	lpuart32_write(temp, port->membase + UARTMODIR);
+    /*
+     * CTS/RTS can _only_ be handled
+     * automatically by the hardware.
+     */
 }
 
 static void lpuart_break_ctl(struct uart_port *port, int break_state)
-- 
2.1.0

[PATCH 2/3] tty: serial: fsl_lpuart: remove RTS/CTS control from set/get_mctrl

[Jakub Kiciński]

On Fri, 29 May 2015 13:35:53 +0530, Bhuvanchandra DV wrote:
> The LPUART does not provide manual control of RTS/CTS signals,
> those can only be controlled by the hardware directly. Therefore
> manual control of those signals through mctrl can not be provided.
> The current implementation enables/disables the automatic control,
> which is not what mctrl should do, hence remove the incorrect
> implementation.
> 
> Signed-off-by: Bhuvanchandra DV <bhuvanchandra.dv@toradex.com>

Now that the functions do nothing there is probably no point in keeping
separate lpuart/lpuart32 variants.

[PATCH 2/3] tty: serial: fsl_lpuart: remove RTS/CTS control from set/get_mctrl

[Bhuvanchandra DV]

Hello Jakub,

On 05/29/2015 03:34 PM, Jakub Kici?ski wrote:
> On Fri, 29 May 2015 13:35:53 +0530, Bhuvanchandra DV wrote:
>> The LPUART does not provide manual control of RTS/CTS signals,
>> those can only be controlled by the hardware directly. Therefore
>> manual control of those signals through mctrl can not be provided.
>> The current implementation enables/disables the automatic control,
>> which is not what mctrl should do, hence remove the incorrect
>> implementation.
>>
>> Signed-off-by: Bhuvanchandra DV <bhuvanchandra.dv@toradex.com>
>
> Now that the functions do nothing there is probably no point in keeping
> separate lpuart/lpuart32 variants.
Agreed, will update it accordingly.

Thanks! for pointing this.
>

Best regards,
Bhuvan

[PATCH 3/3] tty: serial: fsl_lpuart: Add support for RS-485

[Bhuvanchandra DV]

Enable Vybrid's build-in support for RS-485 auto RTS for
controlling line direction of RS-485 transceiver driver.

Signed-off-by: Bhuvanchandra DV <bhuvanchandra.dv@toradex.com>
---
 drivers/tty/serial/fsl_lpuart.c | 60 +++++++++++++++++++++++++++++++++++++++++
 1 file changed, 60 insertions(+)

diff --git a/drivers/tty/serial/fsl_lpuart.c b/drivers/tty/serial/fsl_lpuart.c
index 357f623..c553b14 100644
--- a/drivers/tty/serial/fsl_lpuart.c
+++ b/drivers/tty/serial/fsl_lpuart.c
@@ -820,6 +820,50 @@ static unsigned int lpuart32_tx_empty(struct uart_port *port)
 		TIOCSER_TEMT : 0;
 }
 
+static int lpuart_config_rs485(struct uart_port *port,
+			struct serial_rs485 *rs485)
+{
+	struct lpuart_port *sport = container_of(port,
+			struct lpuart_port, port);
+	u8 modem = readb(sport->port.membase + UARTMODEM) &
+		~(UARTMODEM_TXRTSPOL | UARTMODEM_TXRTSE);
+	writeb(modem, sport->port.membase + UARTMODEM);
+
+	if (rs485->flags & SER_RS485_ENABLED) {
+		/* Enable auto RS-485 RTS mode */
+		modem |= UARTMODEM_TXRTSE;
+
+		/*
+		* RTS needs to be logic HIGH either during transer _or_ after
+		* transfer, other variants are not supported by the hardware.
+		*/
+		if (!(rs485->flags & (SER_RS485_RTS_ON_SEND |
+				SER_RS485_RTS_AFTER_SEND)))
+			rs485->flags |= SER_RS485_RTS_ON_SEND;
+
+		if (rs485->flags & SER_RS485_RTS_ON_SEND &&
+				rs485->flags & SER_RS485_RTS_AFTER_SEND)
+			rs485->flags &= ~SER_RS485_RTS_AFTER_SEND;
+
+		/*
+		* The hardware defaults to RTS logic HIGH while transfer.
+		* Switch polarity in case RTS shall be logic HIGH
+		* after transfer.
+		* Note: UART is assumed to be active high.
+		*/
+		if (rs485->flags & SER_RS485_RTS_ON_SEND)
+			modem &= ~UARTMODEM_TXRTSPOL;
+		else if (rs485->flags & SER_RS485_RTS_AFTER_SEND)
+			modem |= UARTMODEM_TXRTSPOL;
+	}
+
+	/* Store the new configuration */
+	sport->port.rs485 = *rs485;
+
+	writeb(modem, sport->port.membase + UARTMODEM);
+	return 0;
+}
+
 static unsigned int lpuart_get_mctrl(struct uart_port *port)
 {
 	return TIOCM_CAR | TIOCM_CTS | TIOCM_DSR;
@@ -1204,6 +1248,12 @@ lpuart_set_termios(struct uart_port *port, struct ktermios *termios,
 		cr1 |= UARTCR1_M;
 	}
 
+	/* When auto RS-485 RTS mode is enabled,
+	 * hardware flow control need to be disabled.
+	 */
+	if (sport->port.rs485.flags & SER_RS485_ENABLED)
+		termios->c_cflag &= ~CRTSCTS;
+
 	if (termios->c_cflag & CRTSCTS) {
 		modem |= (UARTMODEM_RXRTSE | UARTMODEM_TXCTSE);
 	} else {
@@ -1762,6 +1812,8 @@ static int lpuart_probe(struct platform_device *pdev)
 		sport->port.ops = &lpuart_pops;
 	sport->port.flags = UPF_BOOT_AUTOCONF;
 
+	sport->port.rs485_config = lpuart_config_rs485;
+
 	sport->clk = devm_clk_get(&pdev->dev, "ipg");
 	if (IS_ERR(sport->clk)) {
 		ret = PTR_ERR(sport->clk);
@@ -1802,6 +1854,14 @@ static int lpuart_probe(struct platform_device *pdev)
 		dev_info(sport->port.dev, "DMA rx channel request failed, "
 				"operating without rx DMA\n");
 
+	if (of_property_read_bool(np, "linux,rs485-enabled-at-boot-time")) {
+		sport->port.rs485.flags |= SER_RS485_ENABLED;
+		sport->port.rs485.flags |= SER_RS485_RTS_ON_SEND;
+		writeb(UARTMODEM_TXRTSE, sport->port.membase + UARTMODEM);
+	} else {
+		sport->port.rs485.flags &= ~SER_RS485_ENABLED;
+	}
+
 	return 0;
 }
 
-- 
2.1.0

[PATCH 3/3] tty: serial: fsl_lpuart: Add support for RS-485

[Jakub Kiciński]

On Fri, 29 May 2015 13:35:54 +0530, Bhuvanchandra DV wrote:
> Enable Vybrid's build-in support for RS-485 auto RTS for
> controlling line direction of RS-485 transceiver driver.
> 
> Signed-off-by: Bhuvanchandra DV <bhuvanchandra.dv@toradex.com>
> ---
>  drivers/tty/serial/fsl_lpuart.c | 60 +++++++++++++++++++++++++++++++++++++++++
>  1 file changed, 60 insertions(+)
> 
> diff --git a/drivers/tty/serial/fsl_lpuart.c b/drivers/tty/serial/fsl_lpuart.c
> index 357f623..c553b14 100644
> --- a/drivers/tty/serial/fsl_lpuart.c
> +++ b/drivers/tty/serial/fsl_lpuart.c
> @@ -820,6 +820,50 @@ static unsigned int lpuart32_tx_empty(struct uart_port *port)
>  		TIOCSER_TEMT : 0;
>  }
>  
> +static int lpuart_config_rs485(struct uart_port *port,
> +			struct serial_rs485 *rs485)
> +{
> +	struct lpuart_port *sport = container_of(port,
> +			struct lpuart_port, port);
> +	u8 modem = readb(sport->port.membase + UARTMODEM) &
> +		~(UARTMODEM_TXRTSPOL | UARTMODEM_TXRTSE);

Please put empty line between variables and code.

> +	writeb(modem, sport->port.membase + UARTMODEM);
> +
> +	if (rs485->flags & SER_RS485_ENABLED) {
> +		/* Enable auto RS-485 RTS mode */
> +		modem |= UARTMODEM_TXRTSE;
> +
> +		/*
> +		* RTS needs to be logic HIGH either during transer _or_ after
> +		* transfer, other variants are not supported by the hardware.
> +		*/

Indentation is off here.  '*' should be aligned.

> +		if (!(rs485->flags & (SER_RS485_RTS_ON_SEND |
> +				SER_RS485_RTS_AFTER_SEND)))

and here - things should be aligned on the containing bracket (SER_
under SER_ here).

> +			rs485->flags |= SER_RS485_RTS_ON_SEND;
> +
> +		if (rs485->flags & SER_RS485_RTS_ON_SEND &&
> +				rs485->flags & SER_RS485_RTS_AFTER_SEND)

and here - same bracket rule (rs485-> under rs485->)

> +			rs485->flags &= ~SER_RS485_RTS_AFTER_SEND;
> +
> +		/*
> +		* The hardware defaults to RTS logic HIGH while transfer.
> +		* Switch polarity in case RTS shall be logic HIGH
> +		* after transfer.
> +		* Note: UART is assumed to be active high.
> +		*/
> +		if (rs485->flags & SER_RS485_RTS_ON_SEND)
> +			modem &= ~UARTMODEM_TXRTSPOL;

UARTMODEM_TXRTSPOL was already clear when you read modem.

> +		else if (rs485->flags & SER_RS485_RTS_AFTER_SEND)
> +			modem |= UARTMODEM_TXRTSPOL;
> +	}
> +
> +	/* Store the new configuration */
> +	sport->port.rs485 = *rs485;
> +
> +	writeb(modem, sport->port.membase + UARTMODEM);
> +	return 0;
> +}
> +
>  static unsigned int lpuart_get_mctrl(struct uart_port *port)
>  {
>  	return TIOCM_CAR | TIOCM_CTS | TIOCM_DSR;
[...]
> @@ -1802,6 +1854,14 @@ static int lpuart_probe(struct platform_device *pdev)
>  		dev_info(sport->port.dev, "DMA rx channel request failed, "
>  				"operating without rx DMA\n");
>  
> +	if (of_property_read_bool(np, "linux,rs485-enabled-at-boot-time")) {
> +		sport->port.rs485.flags |= SER_RS485_ENABLED;
> +		sport->port.rs485.flags |= SER_RS485_RTS_ON_SEND;
> +		writeb(UARTMODEM_TXRTSE, sport->port.membase + UARTMODEM);
> +	} else {
> +		sport->port.rs485.flags &= ~SER_RS485_ENABLED;

Why the need to clear the flag?  sport was kzalloc'ed.

[PATCH 3/3] tty: serial: fsl_lpuart: Add support for RS-485

[Bhuvanchandra DV]

On 05/29/2015 03:48 PM, Jakub Kici?ski wrote:
> On Fri, 29 May 2015 13:35:54 +0530, Bhuvanchandra DV wrote:
>> Enable Vybrid's build-in support for RS-485 auto RTS for
>> controlling line direction of RS-485 transceiver driver.
>>
>> Signed-off-by: Bhuvanchandra DV <bhuvanchandra.dv@toradex.com>
>> ---
>>   drivers/tty/serial/fsl_lpuart.c | 60 +++++++++++++++++++++++++++++++++++++++++
>>   1 file changed, 60 insertions(+)
>>
>> diff --git a/drivers/tty/serial/fsl_lpuart.c b/drivers/tty/serial/fsl_lpuart.c
>> index 357f623..c553b14 100644
>> --- a/drivers/tty/serial/fsl_lpuart.c
>> +++ b/drivers/tty/serial/fsl_lpuart.c
>> @@ -820,6 +820,50 @@ static unsigned int lpuart32_tx_empty(struct uart_port *port)
>>   		TIOCSER_TEMT : 0;
>>   }
>>
>> +static int lpuart_config_rs485(struct uart_port *port,
>> +			struct serial_rs485 *rs485)
>> +{
>> +	struct lpuart_port *sport = container_of(port,
>> +			struct lpuart_port, port);
>> +	u8 modem = readb(sport->port.membase + UARTMODEM) &
>> +		~(UARTMODEM_TXRTSPOL | UARTMODEM_TXRTSE);
>
> Please put empty line between variables and code.
Ok
>
>> +	writeb(modem, sport->port.membase + UARTMODEM);
>> +
>> +	if (rs485->flags & SER_RS485_ENABLED) {
>> +		/* Enable auto RS-485 RTS mode */
>> +		modem |= UARTMODEM_TXRTSE;
>> +
>> +		/*
>> +		* RTS needs to be logic HIGH either during transer _or_ after
>> +		* transfer, other variants are not supported by the hardware.
>> +		*/
>
> Indentation is off here.  '*' should be aligned.
Ok
>
>> +		if (!(rs485->flags & (SER_RS485_RTS_ON_SEND |
>> +				SER_RS485_RTS_AFTER_SEND)))
>
> and here - things should be aligned on the containing bracket (SER_
> under SER_ here).
Ok
>
>> +			rs485->flags |= SER_RS485_RTS_ON_SEND;
>> +
>> +		if (rs485->flags & SER_RS485_RTS_ON_SEND &&
>> +				rs485->flags & SER_RS485_RTS_AFTER_SEND)
>
> and here - same bracket rule (rs485-> under rs485->)
Ok
>
>> +			rs485->flags &= ~SER_RS485_RTS_AFTER_SEND;
>> +
>> +		/*
>> +		* The hardware defaults to RTS logic HIGH while transfer.
>> +		* Switch polarity in case RTS shall be logic HIGH
>> +		* after transfer.
>> +		* Note: UART is assumed to be active high.
>> +		*/
>> +		if (rs485->flags & SER_RS485_RTS_ON_SEND)
>> +			modem &= ~UARTMODEM_TXRTSPOL;
>
> UARTMODEM_TXRTSPOL was already clear when you read modem.
Ok
>
>> +		else if (rs485->flags & SER_RS485_RTS_AFTER_SEND)
>> +			modem |= UARTMODEM_TXRTSPOL;
>> +	}
>> +
>> +	/* Store the new configuration */
>> +	sport->port.rs485 = *rs485;
>> +
>> +	writeb(modem, sport->port.membase + UARTMODEM);
>> +	return 0;
>> +}
>> +
>>   static unsigned int lpuart_get_mctrl(struct uart_port *port)
>>   {
>>   	return TIOCM_CAR | TIOCM_CTS | TIOCM_DSR;
> [...]
>> @@ -1802,6 +1854,14 @@ static int lpuart_probe(struct platform_device *pdev)
>>   		dev_info(sport->port.dev, "DMA rx channel request failed, "
>>   				"operating without rx DMA\n");
>>
>> +	if (of_property_read_bool(np, "linux,rs485-enabled-at-boot-time")) {
>> +		sport->port.rs485.flags |= SER_RS485_ENABLED;
>> +		sport->port.rs485.flags |= SER_RS485_RTS_ON_SEND;
>> +		writeb(UARTMODEM_TXRTSE, sport->port.membase + UARTMODEM);
>> +	} else {
>> +		sport->port.rs485.flags &= ~SER_RS485_ENABLED;
>
> Why the need to clear the flag?  sport was kzalloc'ed.
Will remove it.
>

Best Regards,
Bhuvan