[PATCH] ARM/serial: at91: switch atmel serial to use gpiolib

[PATCH] ARM/serial: at91: switch atmel serial to use gpiolib

[Linus Walleij]

This passes the errata fix using a GPIO to control the RTS pin
on one of the AT91 chips to use gpiolib instead of the
AT91-specific interfaces. Also remove the reliance on
compile-time #defines and the cpu_* check and rely on the
platform passing down the proper GPIO pin through platform
data.

This is a prerequisite for getting rid of the local GPIO
implementation in the AT91 platform and move toward
multiplatform.

This also makes way for device tree conversion: the RTS
GPIO pin can be passed by standard GPIO bindings.

Signed-off-by: Linus Walleij <linus.walleij@linaro.org>
---
This is an alternative to the patch entitled
"ARM/serial: at91: move machine quirk into machine"
which needs testing to confirm this approach.
Seeking ACKs on this if the approach seems OK to
all parties.
---
 arch/arm/mach-at91/at91rm9200_devices.c |  1 +
 drivers/tty/serial/atmel_serial.c       | 51 +++++++++++++++++++++------------
 include/linux/platform_data/atmel.h     |  1 +
 3 files changed, 35 insertions(+), 18 deletions(-)

diff --git a/arch/arm/mach-at91/at91rm9200_devices.c b/arch/arm/mach-at91/at91rm9200_devices.c
index c721e9b08066..51d4c08962f6 100644
--- a/arch/arm/mach-at91/at91rm9200_devices.c
+++ b/arch/arm/mach-at91/at91rm9200_devices.c
@@ -961,6 +961,7 @@ static struct resource uart0_resources[] = {
 static struct atmel_uart_data uart0_data = {
 	.use_dma_tx	= 1,
 	.use_dma_rx	= 1,
+	.rts_gpio	= AT91_PIN_PA21,
 };
 
 static u64 uart0_dmamask = DMA_BIT_MASK(32);
diff --git a/drivers/tty/serial/atmel_serial.c b/drivers/tty/serial/atmel_serial.c
index d067285a2d20..3d5c848cdfe1 100644
--- a/drivers/tty/serial/atmel_serial.c
+++ b/drivers/tty/serial/atmel_serial.c
@@ -41,15 +41,11 @@
 #include <linux/uaccess.h>
 #include <linux/platform_data/atmel.h>
 #include <linux/timer.h>
+#include <linux/gpio.h>
 
 #include <asm/io.h>
 #include <asm/ioctls.h>
 
-#ifdef CONFIG_ARM
-#include <mach/cpu.h>
-#include <asm/gpio.h>
-#endif
-
 #define PDC_BUFFER_SIZE		512
 /* Revisit: We should calculate this based on the actual port settings */
 #define PDC_RX_TIMEOUT		(3 * 10)		/* 3 bytes */
@@ -167,6 +163,7 @@ struct atmel_uart_port {
 	struct circ_buf		rx_ring;
 
 	struct serial_rs485	rs485;		/* rs485 settings */
+	int			rts_gpio;	/* optional RTS GPIO */
 	unsigned int		tx_done_mask;
 	bool			is_usart;	/* usart or uart */
 	struct timer_list	uart_timer;	/* uart timer */
@@ -300,20 +297,17 @@ static void atmel_set_mctrl(struct uart_port *port, u_int mctrl)
 	unsigned int mode;
 	struct atmel_uart_port *atmel_port = to_atmel_uart_port(port);
 
-#ifdef CONFIG_ARCH_AT91RM9200
-	if (cpu_is_at91rm9200()) {
-		/*
-		 * AT91RM9200 Errata #39: RTS0 is not internally connected
-		 * to PA21. We need to drive the pin manually.
-		 */
-		if (port->mapbase == AT91RM9200_BASE_US0) {
-			if (mctrl & TIOCM_RTS)
-				at91_set_gpio_value(AT91_PIN_PA21, 0);
-			else
-				at91_set_gpio_value(AT91_PIN_PA21, 1);
-		}
+	/*
+	 * AT91RM9200 Errata #39: RTS0 is not internally connected
+	 * to PA21. We need to drive the pin as a GPIO.
+	 */
+	if (gpio_is_valid(atmel_port->rts_gpio) &&
+	    port->mapbase == AT91RM9200_BASE_US0) {
+		if (mctrl & TIOCM_RTS)
+			gpio_set_value(atmel_port->rts_gpio, 0);
+		else
+			gpio_set_value(atmel_port->rts_gpio, 1);
 	}
-#endif
 
 	if (mctrl & TIOCM_RTS)
 		control |= ATMEL_US_RTSEN;
@@ -2365,6 +2359,27 @@ static int atmel_serial_probe(struct platform_device *pdev)
 	port = &atmel_ports[ret];
 	port->backup_imr = 0;
 	port->uart.line = ret;
+	port->rts_gpio = -1; /* Invalid, zero could be valid */
+	/*
+	 * In theory the GPIO pin controlling RTS could be zero and
+	 * this would be an improper check, but we know that the only
+	 * existing case is != 0 and it's nice to use the zero-initialized
+	 * structs to indicate "no RTS GPIO" instead of open-coding some
+	 * invalid value everywhere.
+	 */
+	if (pdata->rts_gpio > 0) {
+		ret = devm_gpio_request(&pdev->dev, pdata->rts_gpio, "RTS");
+		if (ret) {
+			dev_err(&pdev->dev, "error requesting RTS GPIO\n");
+			goto err;
+		}
+		port->rts_gpio = pdata->rts_gpio;
+		ret = gpio_direction_output(port->rts_gpio, 0);
+		if (ret) {
+			dev_err(&pdev->dev, "error setting up RTS GPIO\n");
+			goto err;
+		}
+	}
 
 	ret = atmel_init_port(port, pdev);
 	if (ret)
diff --git a/include/linux/platform_data/atmel.h b/include/linux/platform_data/atmel.h
index cea9f70133c5..e26b0c14edea 100644
--- a/include/linux/platform_data/atmel.h
+++ b/include/linux/platform_data/atmel.h
@@ -84,6 +84,7 @@ struct atmel_uart_data {
 	short			use_dma_rx;	/* use receive DMA? */
 	void __iomem		*regs;		/* virt. base address, if any */
 	struct serial_rs485	rs485;		/* rs485 settings */
+	int			rts_gpio;	/* optional RTS GPIO */
 };
 
  /* Touchscreen Controller */
-- 
1.8.3.1

[PATCH] ARM/serial: at91: switch atmel serial to use gpiolib

[Linus Walleij]

This passes the errata fix using a GPIO to control the RTS pin
on one of the AT91 chips to use gpiolib instead of the
AT91-specific interfaces. Also remove the reliance on
compile-time #defines and the cpu_* check and rely on the
platform passing down the proper GPIO pin through platform
data.

This is a prerequisite for getting rid of the local GPIO
implementation in the AT91 platform and move toward
multiplatform.

This also makes way for device tree conversion: the RTS
GPIO pin can be passed by standard GPIO bindings.

Signed-off-by: Linus Walleij <linus.walleij@linaro.org>
---
This is an alternative to the patch entitled
"ARM/serial: at91: move machine quirk into machine"
which needs testing to confirm this approach.
Seeking ACKs on this if the approach seems OK to
all parties.
---
 arch/arm/mach-at91/at91rm9200_devices.c |  1 +
 drivers/tty/serial/atmel_serial.c       | 51 +++++++++++++++++++++------------
 include/linux/platform_data/atmel.h     |  1 +
 3 files changed, 35 insertions(+), 18 deletions(-)

diff --git a/arch/arm/mach-at91/at91rm9200_devices.c b/arch/arm/mach-at91/at91rm9200_devices.c
index c721e9b08066..51d4c08962f6 100644
--- a/arch/arm/mach-at91/at91rm9200_devices.c
+++ b/arch/arm/mach-at91/at91rm9200_devices.c
@@ -961,6 +961,7 @@ static struct resource uart0_resources[] = {
 static struct atmel_uart_data uart0_data = {
 	.use_dma_tx	= 1,
 	.use_dma_rx	= 1,
+	.rts_gpio	= AT91_PIN_PA21,
 };
 
 static u64 uart0_dmamask = DMA_BIT_MASK(32);
diff --git a/drivers/tty/serial/atmel_serial.c b/drivers/tty/serial/atmel_serial.c
index d067285a2d20..3d5c848cdfe1 100644
--- a/drivers/tty/serial/atmel_serial.c
+++ b/drivers/tty/serial/atmel_serial.c
@@ -41,15 +41,11 @@
 #include <linux/uaccess.h>
 #include <linux/platform_data/atmel.h>
 #include <linux/timer.h>
+#include <linux/gpio.h>
 
 #include <asm/io.h>
 #include <asm/ioctls.h>
 
-#ifdef CONFIG_ARM
-#include <mach/cpu.h>
-#include <asm/gpio.h>
-#endif
-
 #define PDC_BUFFER_SIZE		512
 /* Revisit: We should calculate this based on the actual port settings */
 #define PDC_RX_TIMEOUT		(3 * 10)		/* 3 bytes */
@@ -167,6 +163,7 @@ struct atmel_uart_port {
 	struct circ_buf		rx_ring;
 
 	struct serial_rs485	rs485;		/* rs485 settings */
+	int			rts_gpio;	/* optional RTS GPIO */
 	unsigned int		tx_done_mask;
 	bool			is_usart;	/* usart or uart */
 	struct timer_list	uart_timer;	/* uart timer */
@@ -300,20 +297,17 @@ static void atmel_set_mctrl(struct uart_port *port, u_int mctrl)
 	unsigned int mode;
 	struct atmel_uart_port *atmel_port = to_atmel_uart_port(port);
 
-#ifdef CONFIG_ARCH_AT91RM9200
-	if (cpu_is_at91rm9200()) {
-		/*
-		 * AT91RM9200 Errata #39: RTS0 is not internally connected
-		 * to PA21. We need to drive the pin manually.
-		 */
-		if (port->mapbase == AT91RM9200_BASE_US0) {
-			if (mctrl & TIOCM_RTS)
-				at91_set_gpio_value(AT91_PIN_PA21, 0);
-			else
-				at91_set_gpio_value(AT91_PIN_PA21, 1);
-		}
+	/*
+	 * AT91RM9200 Errata #39: RTS0 is not internally connected
+	 * to PA21. We need to drive the pin as a GPIO.
+	 */
+	if (gpio_is_valid(atmel_port->rts_gpio) &&
+	    port->mapbase == AT91RM9200_BASE_US0) {
+		if (mctrl & TIOCM_RTS)
+			gpio_set_value(atmel_port->rts_gpio, 0);
+		else
+			gpio_set_value(atmel_port->rts_gpio, 1);
 	}
-#endif
 
 	if (mctrl & TIOCM_RTS)
 		control |= ATMEL_US_RTSEN;
@@ -2365,6 +2359,27 @@ static int atmel_serial_probe(struct platform_device *pdev)
 	port = &atmel_ports[ret];
 	port->backup_imr = 0;
 	port->uart.line = ret;
+	port->rts_gpio = -1; /* Invalid, zero could be valid */
+	/*
+	 * In theory the GPIO pin controlling RTS could be zero and
+	 * this would be an improper check, but we know that the only
+	 * existing case is != 0 and it's nice to use the zero-initialized
+	 * structs to indicate "no RTS GPIO" instead of open-coding some
+	 * invalid value everywhere.
+	 */
+	if (pdata->rts_gpio > 0) {
+		ret = devm_gpio_request(&pdev->dev, pdata->rts_gpio, "RTS");
+		if (ret) {
+			dev_err(&pdev->dev, "error requesting RTS GPIO\n");
+			goto err;
+		}
+		port->rts_gpio = pdata->rts_gpio;
+		ret = gpio_direction_output(port->rts_gpio, 0);
+		if (ret) {
+			dev_err(&pdev->dev, "error setting up RTS GPIO\n");
+			goto err;
+		}
+	}
 
 	ret = atmel_init_port(port, pdev);
 	if (ret)
diff --git a/include/linux/platform_data/atmel.h b/include/linux/platform_data/atmel.h
index cea9f70133c5..e26b0c14edea 100644
--- a/include/linux/platform_data/atmel.h
+++ b/include/linux/platform_data/atmel.h
@@ -84,6 +84,7 @@ struct atmel_uart_data {
 	short			use_dma_rx;	/* use receive DMA? */
 	void __iomem		*regs;		/* virt. base address, if any */
 	struct serial_rs485	rs485;		/* rs485 settings */
+	int			rts_gpio;	/* optional RTS GPIO */
 };
 
  /* Touchscreen Controller */
-- 
1.8.3.1

Re: [PATCH] ARM/serial: at91: switch atmel serial to use gpiolib

[Nicolas Ferre]

On 05/11/2013 13:35, Linus Walleij :
> This passes the errata fix using a GPIO to control the RTS pin
> on one of the AT91 chips to use gpiolib instead of the
> AT91-specific interfaces. Also remove the reliance on
> compile-time #defines and the cpu_* check and rely on the
> platform passing down the proper GPIO pin through platform
> data.
>
> This is a prerequisite for getting rid of the local GPIO
> implementation in the AT91 platform and move toward
> multiplatform.
>
> This also makes way for device tree conversion: the RTS
> GPIO pin can be passed by standard GPIO bindings.
>
> Signed-off-by: Linus Walleij <linus.walleij@linaro.org>
> ---
> This is an alternative to the patch entitled
> "ARM/serial: at91: move machine quirk into machine"
> which needs testing to confirm this approach.
> Seeking ACKs on this if the approach seems OK to
> all parties.
> ---
>   arch/arm/mach-at91/at91rm9200_devices.c |  1 +
>   drivers/tty/serial/atmel_serial.c       | 51 +++++++++++++++++++++------------
>   include/linux/platform_data/atmel.h     |  1 +
>   3 files changed, 35 insertions(+), 18 deletions(-)
>
> diff --git a/arch/arm/mach-at91/at91rm9200_devices.c b/arch/arm/mach-at91/at91rm9200_devices.c
> index c721e9b08066..51d4c08962f6 100644
> --- a/arch/arm/mach-at91/at91rm9200_devices.c
> +++ b/arch/arm/mach-at91/at91rm9200_devices.c
> @@ -961,6 +961,7 @@ static struct resource uart0_resources[] = {
>   static struct atmel_uart_data uart0_data = {
>   	.use_dma_tx	= 1,
>   	.use_dma_rx	= 1,
> +	.rts_gpio	= AT91_PIN_PA21,
>   };
>
>   static u64 uart0_dmamask = DMA_BIT_MASK(32);
> diff --git a/drivers/tty/serial/atmel_serial.c b/drivers/tty/serial/atmel_serial.c
> index d067285a2d20..3d5c848cdfe1 100644
> --- a/drivers/tty/serial/atmel_serial.c
> +++ b/drivers/tty/serial/atmel_serial.c
> @@ -41,15 +41,11 @@
>   #include <linux/uaccess.h>
>   #include <linux/platform_data/atmel.h>
>   #include <linux/timer.h>
> +#include <linux/gpio.h>
>
>   #include <asm/io.h>
>   #include <asm/ioctls.h>
>
> -#ifdef CONFIG_ARM
> -#include <mach/cpu.h>
> -#include <asm/gpio.h>
> -#endif
> -
>   #define PDC_BUFFER_SIZE		512
>   /* Revisit: We should calculate this based on the actual port settings */
>   #define PDC_RX_TIMEOUT		(3 * 10)		/* 3 bytes */
> @@ -167,6 +163,7 @@ struct atmel_uart_port {
>   	struct circ_buf		rx_ring;
>
>   	struct serial_rs485	rs485;		/* rs485 settings */
> +	int			rts_gpio;	/* optional RTS GPIO */
>   	unsigned int		tx_done_mask;
>   	bool			is_usart;	/* usart or uart */
>   	struct timer_list	uart_timer;	/* uart timer */
> @@ -300,20 +297,17 @@ static void atmel_set_mctrl(struct uart_port *port, u_int mctrl)
>   	unsigned int mode;
>   	struct atmel_uart_port *atmel_port = to_atmel_uart_port(port);
>
> -#ifdef CONFIG_ARCH_AT91RM9200
> -	if (cpu_is_at91rm9200()) {
> -		/*
> -		 * AT91RM9200 Errata #39: RTS0 is not internally connected
> -		 * to PA21. We need to drive the pin manually.
> -		 */
> -		if (port->mapbase == AT91RM9200_BASE_US0) {
> -			if (mctrl & TIOCM_RTS)
> -				at91_set_gpio_value(AT91_PIN_PA21, 0);
> -			else
> -				at91_set_gpio_value(AT91_PIN_PA21, 1);
> -		}
> +	/*
> +	 * AT91RM9200 Errata #39: RTS0 is not internally connected
> +	 * to PA21. We need to drive the pin as a GPIO.
> +	 */
> +	if (gpio_is_valid(atmel_port->rts_gpio) &&
> +	    port->mapbase == AT91RM9200_BASE_US0) {

Okay. Let's keep it like this for the moment. If I want to generalize 
this "feature" to other SoCs, I will remove the second test.


> +		if (mctrl & TIOCM_RTS)
> +			gpio_set_value(atmel_port->rts_gpio, 0);
> +		else
> +			gpio_set_value(atmel_port->rts_gpio, 1);
>   	}
> -#endif
>
>   	if (mctrl & TIOCM_RTS)
>   		control |= ATMEL_US_RTSEN;
> @@ -2365,6 +2359,27 @@ static int atmel_serial_probe(struct platform_device *pdev)
>   	port = &atmel_ports[ret];
>   	port->backup_imr = 0;
>   	port->uart.line = ret;
> +	port->rts_gpio = -1; /* Invalid, zero could be valid */

What about -EINVAL?

> +	/*
> +	 * In theory the GPIO pin controlling RTS could be zero and
> +	 * this would be an improper check, but we know that the only
> +	 * existing case is != 0 and it's nice to use the zero-initialized
> +	 * structs to indicate "no RTS GPIO" instead of open-coding some
> +	 * invalid value everywhere.
> +	 */
> +	if (pdata->rts_gpio > 0) {

Okay, let's do the gpio validity checking like this. It is preventing us 
from big headache.

But, unfortunately, on DT-enabled machines we do not have a pdata 
structure. You would have to change the test to:

+	if (pdata && pdata->rts_gpio > 0) {

But anyway, I send an DT support for this feature as a follow-up of this 
patch. You can squash both patches if you want.

> +		ret = devm_gpio_request(&pdev->dev, pdata->rts_gpio, "RTS");
> +		if (ret) {
> +			dev_err(&pdev->dev, "error requesting RTS GPIO\n");
> +			goto err;
> +		}
> +		port->rts_gpio = pdata->rts_gpio;
> +		ret = gpio_direction_output(port->rts_gpio, 0);
> +		if (ret) {
> +			dev_err(&pdev->dev, "error setting up RTS GPIO\n");
> +			goto err;
> +		}
> +	}
>
>   	ret = atmel_init_port(port, pdev);
>   	if (ret)
> diff --git a/include/linux/platform_data/atmel.h b/include/linux/platform_data/atmel.h
> index cea9f70133c5..e26b0c14edea 100644
> --- a/include/linux/platform_data/atmel.h
> +++ b/include/linux/platform_data/atmel.h
> @@ -84,6 +84,7 @@ struct atmel_uart_data {
>   	short			use_dma_rx;	/* use receive DMA? */
>   	void __iomem		*regs;		/* virt. base address, if any */
>   	struct serial_rs485	rs485;		/* rs485 settings */
> +	int			rts_gpio;	/* optional RTS GPIO */
>   };
>
>    /* Touchscreen Controller */
>


-- 
Nicolas Ferre

[PATCH] ARM/serial: at91: switch atmel serial to use gpiolib

[Nicolas Ferre]

On 05/11/2013 13:35, Linus Walleij :
> This passes the errata fix using a GPIO to control the RTS pin
> on one of the AT91 chips to use gpiolib instead of the
> AT91-specific interfaces. Also remove the reliance on
> compile-time #defines and the cpu_* check and rely on the
> platform passing down the proper GPIO pin through platform
> data.
>
> This is a prerequisite for getting rid of the local GPIO
> implementation in the AT91 platform and move toward
> multiplatform.
>
> This also makes way for device tree conversion: the RTS
> GPIO pin can be passed by standard GPIO bindings.
>
> Signed-off-by: Linus Walleij <linus.walleij@linaro.org>
> ---
> This is an alternative to the patch entitled
> "ARM/serial: at91: move machine quirk into machine"
> which needs testing to confirm this approach.
> Seeking ACKs on this if the approach seems OK to
> all parties.
> ---
>   arch/arm/mach-at91/at91rm9200_devices.c |  1 +
>   drivers/tty/serial/atmel_serial.c       | 51 +++++++++++++++++++++------------
>   include/linux/platform_data/atmel.h     |  1 +
>   3 files changed, 35 insertions(+), 18 deletions(-)
>
> diff --git a/arch/arm/mach-at91/at91rm9200_devices.c b/arch/arm/mach-at91/at91rm9200_devices.c
> index c721e9b08066..51d4c08962f6 100644
> --- a/arch/arm/mach-at91/at91rm9200_devices.c
> +++ b/arch/arm/mach-at91/at91rm9200_devices.c
> @@ -961,6 +961,7 @@ static struct resource uart0_resources[] = {
>   static struct atmel_uart_data uart0_data = {
>   	.use_dma_tx	= 1,
>   	.use_dma_rx	= 1,
> +	.rts_gpio	= AT91_PIN_PA21,
>   };
>
>   static u64 uart0_dmamask = DMA_BIT_MASK(32);
> diff --git a/drivers/tty/serial/atmel_serial.c b/drivers/tty/serial/atmel_serial.c
> index d067285a2d20..3d5c848cdfe1 100644
> --- a/drivers/tty/serial/atmel_serial.c
> +++ b/drivers/tty/serial/atmel_serial.c
> @@ -41,15 +41,11 @@
>   #include <linux/uaccess.h>
>   #include <linux/platform_data/atmel.h>
>   #include <linux/timer.h>
> +#include <linux/gpio.h>
>
>   #include <asm/io.h>
>   #include <asm/ioctls.h>
>
> -#ifdef CONFIG_ARM
> -#include <mach/cpu.h>
> -#include <asm/gpio.h>
> -#endif
> -
>   #define PDC_BUFFER_SIZE		512
>   /* Revisit: We should calculate this based on the actual port settings */
>   #define PDC_RX_TIMEOUT		(3 * 10)		/* 3 bytes */
> @@ -167,6 +163,7 @@ struct atmel_uart_port {
>   	struct circ_buf		rx_ring;
>
>   	struct serial_rs485	rs485;		/* rs485 settings */
> +	int			rts_gpio;	/* optional RTS GPIO */
>   	unsigned int		tx_done_mask;
>   	bool			is_usart;	/* usart or uart */
>   	struct timer_list	uart_timer;	/* uart timer */
> @@ -300,20 +297,17 @@ static void atmel_set_mctrl(struct uart_port *port, u_int mctrl)
>   	unsigned int mode;
>   	struct atmel_uart_port *atmel_port = to_atmel_uart_port(port);
>
> -#ifdef CONFIG_ARCH_AT91RM9200
> -	if (cpu_is_at91rm9200()) {
> -		/*
> -		 * AT91RM9200 Errata #39: RTS0 is not internally connected
> -		 * to PA21. We need to drive the pin manually.
> -		 */
> -		if (port->mapbase == AT91RM9200_BASE_US0) {
> -			if (mctrl & TIOCM_RTS)
> -				at91_set_gpio_value(AT91_PIN_PA21, 0);
> -			else
> -				at91_set_gpio_value(AT91_PIN_PA21, 1);
> -		}
> +	/*
> +	 * AT91RM9200 Errata #39: RTS0 is not internally connected
> +	 * to PA21. We need to drive the pin as a GPIO.
> +	 */
> +	if (gpio_is_valid(atmel_port->rts_gpio) &&
> +	    port->mapbase == AT91RM9200_BASE_US0) {

Okay. Let's keep it like this for the moment. If I want to generalize 
this "feature" to other SoCs, I will remove the second test.


> +		if (mctrl & TIOCM_RTS)
> +			gpio_set_value(atmel_port->rts_gpio, 0);
> +		else
> +			gpio_set_value(atmel_port->rts_gpio, 1);
>   	}
> -#endif
>
>   	if (mctrl & TIOCM_RTS)
>   		control |= ATMEL_US_RTSEN;
> @@ -2365,6 +2359,27 @@ static int atmel_serial_probe(struct platform_device *pdev)
>   	port = &atmel_ports[ret];
>   	port->backup_imr = 0;
>   	port->uart.line = ret;
> +	port->rts_gpio = -1; /* Invalid, zero could be valid */

What about -EINVAL?

> +	/*
> +	 * In theory the GPIO pin controlling RTS could be zero and
> +	 * this would be an improper check, but we know that the only
> +	 * existing case is != 0 and it's nice to use the zero-initialized
> +	 * structs to indicate "no RTS GPIO" instead of open-coding some
> +	 * invalid value everywhere.
> +	 */
> +	if (pdata->rts_gpio > 0) {

Okay, let's do the gpio validity checking like this. It is preventing us 
from big headache.

But, unfortunately, on DT-enabled machines we do not have a pdata 
structure. You would have to change the test to:

+	if (pdata && pdata->rts_gpio > 0) {

But anyway, I send an DT support for this feature as a follow-up of this 
patch. You can squash both patches if you want.

> +		ret = devm_gpio_request(&pdev->dev, pdata->rts_gpio, "RTS");
> +		if (ret) {
> +			dev_err(&pdev->dev, "error requesting RTS GPIO\n");
> +			goto err;
> +		}
> +		port->rts_gpio = pdata->rts_gpio;
> +		ret = gpio_direction_output(port->rts_gpio, 0);
> +		if (ret) {
> +			dev_err(&pdev->dev, "error setting up RTS GPIO\n");
> +			goto err;
> +		}
> +	}
>
>   	ret = atmel_init_port(port, pdev);
>   	if (ret)
> diff --git a/include/linux/platform_data/atmel.h b/include/linux/platform_data/atmel.h
> index cea9f70133c5..e26b0c14edea 100644
> --- a/include/linux/platform_data/atmel.h
> +++ b/include/linux/platform_data/atmel.h
> @@ -84,6 +84,7 @@ struct atmel_uart_data {
>   	short			use_dma_rx;	/* use receive DMA? */
>   	void __iomem		*regs;		/* virt. base address, if any */
>   	struct serial_rs485	rs485;		/* rs485 settings */
> +	int			rts_gpio;	/* optional RTS GPIO */
>   };
>
>    /* Touchscreen Controller */
>


-- 
Nicolas Ferre

[PATCH] ARM/serial: at91: specify RTS in DT using gpio

[Nicolas Ferre]

Signed-off-by: Nicolas Ferre <nicolas.ferre@atmel.com>
---
 Documentation/devicetree/bindings/serial/atmel-usart.txt |  3 +++
 drivers/tty/serial/atmel_serial.c                        | 13 ++++++++++---
 2 files changed, 13 insertions(+), 3 deletions(-)

diff --git a/Documentation/devicetree/bindings/serial/atmel-usart.txt b/Documentation/devicetree/bindings/serial/atmel-usart.txt
index 2191dcb..3adc61c 100644
--- a/Documentation/devicetree/bindings/serial/atmel-usart.txt
+++ b/Documentation/devicetree/bindings/serial/atmel-usart.txt
@@ -10,6 +10,8 @@ Required properties:
 Optional properties:
 - atmel,use-dma-rx: use of PDC or DMA for receiving data
 - atmel,use-dma-tx: use of PDC or DMA for transmitting data
+- rts-gpios: specify a GPIO for RTS line. It will use specified PIO instead of the peripheral
+  function pin for the USART RTS feature. If unsure, don't specify this property.
 - add dma bindings for dma transfer:
 	- dmas: DMA specifier, consisting of a phandle to DMA controller node,
 		memory peripheral interface and USART DMA channel ID, FIFO configuration.
@@ -28,6 +30,7 @@ Example:
 		interrupts = <7>;
 		atmel,use-dma-rx;
 		atmel,use-dma-tx;
+		rts-gpios = <&pioD 15 0>;
 	};
 
 - use DMA:
diff --git a/drivers/tty/serial/atmel_serial.c b/drivers/tty/serial/atmel_serial.c
index b4e0794..71f8ea9 100644
--- a/drivers/tty/serial/atmel_serial.c
+++ b/drivers/tty/serial/atmel_serial.c
@@ -35,6 +35,7 @@
 #include <linux/platform_device.h>
 #include <linux/of.h>
 #include <linux/of_device.h>
+#include <linux/of_gpio.h>
 #include <linux/dma-mapping.h>
 #include <linux/atmel_pdc.h>
 #include <linux/atmel_serial.h>
@@ -2327,6 +2328,7 @@ static int atmel_serial_probe(struct platform_device *pdev)
 	struct device_node *np = pdev->dev.of_node;
 	struct atmel_uart_data *pdata = dev_get_platdata(&pdev->dev);
 	void *data;
+	int rts_pin = -EINVAL;
 	int ret = -ENODEV;
 
 	BUILD_BUG_ON(ATMEL_SERIAL_RINGSIZE & (ATMEL_SERIAL_RINGSIZE - 1));
@@ -2364,13 +2366,18 @@ static int atmel_serial_probe(struct platform_device *pdev)
 	 * structs to indicate "no RTS GPIO" instead of open-coding some
 	 * invalid value everywhere.
 	 */
-	if (pdata->rts_gpio > 0) {
-		ret = devm_gpio_request(&pdev->dev, pdata->rts_gpio, "RTS");
+	if (pdata && pdata->rts_gpio > 0)
+		rts_pin = pdata->rts_gpio;
+	else if (np)
+		rts_pin = of_get_named_gpio(np, "rts-gpios", 0);
+
+	if (gpio_is_valid(rts_pin)) {
+		ret = devm_gpio_request(&pdev->dev, rts_pin, "RTS");
 		if (ret) {
 			dev_err(&pdev->dev, "error requesting RTS GPIO\n");
 			goto err;
 		}
-		port->rts_gpio = pdata->rts_gpio;
+		port->rts_gpio = rts_pin;
 		ret = gpio_direction_output(port->rts_gpio, 0);
 		if (ret) {
 			dev_err(&pdev->dev, "error setting up RTS GPIO\n");
-- 
1.8.2.2

[PATCH] ARM/serial: at91: specify RTS in DT using gpio

[Nicolas Ferre]

Signed-off-by: Nicolas Ferre <nicolas.ferre@atmel.com>
---
 Documentation/devicetree/bindings/serial/atmel-usart.txt |  3 +++
 drivers/tty/serial/atmel_serial.c                        | 13 ++++++++++---
 2 files changed, 13 insertions(+), 3 deletions(-)

diff --git a/Documentation/devicetree/bindings/serial/atmel-usart.txt b/Documentation/devicetree/bindings/serial/atmel-usart.txt
index 2191dcb..3adc61c 100644
--- a/Documentation/devicetree/bindings/serial/atmel-usart.txt
+++ b/Documentation/devicetree/bindings/serial/atmel-usart.txt
@@ -10,6 +10,8 @@ Required properties:
 Optional properties:
 - atmel,use-dma-rx: use of PDC or DMA for receiving data
 - atmel,use-dma-tx: use of PDC or DMA for transmitting data
+- rts-gpios: specify a GPIO for RTS line. It will use specified PIO instead of the peripheral
+  function pin for the USART RTS feature. If unsure, don't specify this property.
 - add dma bindings for dma transfer:
 	- dmas: DMA specifier, consisting of a phandle to DMA controller node,
 		memory peripheral interface and USART DMA channel ID, FIFO configuration.
@@ -28,6 +30,7 @@ Example:
 		interrupts = <7>;
 		atmel,use-dma-rx;
 		atmel,use-dma-tx;
+		rts-gpios = <&pioD 15 0>;
 	};
 
 - use DMA:
diff --git a/drivers/tty/serial/atmel_serial.c b/drivers/tty/serial/atmel_serial.c
index b4e0794..71f8ea9 100644
--- a/drivers/tty/serial/atmel_serial.c
+++ b/drivers/tty/serial/atmel_serial.c
@@ -35,6 +35,7 @@
 #include <linux/platform_device.h>
 #include <linux/of.h>
 #include <linux/of_device.h>
+#include <linux/of_gpio.h>
 #include <linux/dma-mapping.h>
 #include <linux/atmel_pdc.h>
 #include <linux/atmel_serial.h>
@@ -2327,6 +2328,7 @@ static int atmel_serial_probe(struct platform_device *pdev)
 	struct device_node *np = pdev->dev.of_node;
 	struct atmel_uart_data *pdata = dev_get_platdata(&pdev->dev);
 	void *data;
+	int rts_pin = -EINVAL;
 	int ret = -ENODEV;
 
 	BUILD_BUG_ON(ATMEL_SERIAL_RINGSIZE & (ATMEL_SERIAL_RINGSIZE - 1));
@@ -2364,13 +2366,18 @@ static int atmel_serial_probe(struct platform_device *pdev)
 	 * structs to indicate "no RTS GPIO" instead of open-coding some
 	 * invalid value everywhere.
 	 */
-	if (pdata->rts_gpio > 0) {
-		ret = devm_gpio_request(&pdev->dev, pdata->rts_gpio, "RTS");
+	if (pdata && pdata->rts_gpio > 0)
+		rts_pin = pdata->rts_gpio;
+	else if (np)
+		rts_pin = of_get_named_gpio(np, "rts-gpios", 0);
+
+	if (gpio_is_valid(rts_pin)) {
+		ret = devm_gpio_request(&pdev->dev, rts_pin, "RTS");
 		if (ret) {
 			dev_err(&pdev->dev, "error requesting RTS GPIO\n");
 			goto err;
 		}
-		port->rts_gpio = pdata->rts_gpio;
+		port->rts_gpio = rts_pin;
 		ret = gpio_direction_output(port->rts_gpio, 0);
 		if (ret) {
 			dev_err(&pdev->dev, "error setting up RTS GPIO\n");
-- 
1.8.2.2

Re: [PATCH] ARM/serial: at91: specify RTS in DT using gpio

[Jean-Christophe PLAGNIOL-VILLARD]

On Nov 5, 2013, at 11:29 PM, Nicolas Ferre <nicolas.ferre@atmel.com> wrote:

> Signed-off-by: Nicolas Ferre <nicolas.ferre@atmel.com>
> ---
> Documentation/devicetree/bindings/serial/atmel-usart.txt |  3 +++
> drivers/tty/serial/atmel_serial.c                        | 13 ++++++++++---
> 2 files changed, 13 insertions(+), 3 deletions(-)
> 
> diff --git a/Documentation/devicetree/bindings/serial/atmel-usart.txt b/Documentation/devicetree/bindings/serial/atmel-usart.txt
> index 2191dcb..3adc61c 100644
> --- a/Documentation/devicetree/bindings/serial/atmel-usart.txt
> +++ b/Documentation/devicetree/bindings/serial/atmel-usart.txt
> @@ -10,6 +10,8 @@ Required properties:
> Optional properties:
> - atmel,use-dma-rx: use of PDC or DMA for receiving data
> - atmel,use-dma-tx: use of PDC or DMA for transmitting data
> +- rts-gpios: specify a GPIO for RTS line. It will use specified PIO instead of the peripheral
> +  function pin for the USART RTS feature. If unsure, don't specify this property.
> - add dma bindings for dma transfer:
> 	- dmas: DMA specifier, consisting of a phandle to DMA controller node,
> 		memory peripheral interface and USART DMA channel ID, FIFO configuration.
> @@ -28,6 +30,7 @@ Example:
> 		interrupts = <7>;
> 		atmel,use-dma-rx;
> 		atmel,use-dma-tx;
> +		rts-gpios = <&pioD 15 0>;
> 	};

gpios means 2 gpio 

I’ll rts-gpio as we can only use one anyway



> 
> - use DMA:
> diff --git a/drivers/tty/serial/atmel_serial.c b/drivers/tty/serial/atmel_serial.c
> index b4e0794..71f8ea9 100644
> --- a/drivers/tty/serial/atmel_serial.c
> +++ b/drivers/tty/serial/atmel_serial.c
> @@ -35,6 +35,7 @@
> #include <linux/platform_device.h>
> #include <linux/of.h>
> #include <linux/of_device.h>
> +#include <linux/of_gpio.h>
> #include <linux/dma-mapping.h>
> #include <linux/atmel_pdc.h>
> #include <linux/atmel_serial.h>
> @@ -2327,6 +2328,7 @@ static int atmel_serial_probe(struct platform_device *pdev)
> 	struct device_node *np = pdev->dev.of_node;
> 	struct atmel_uart_data *pdata = dev_get_platdata(&pdev->dev);
> 	void *data;
> +	int rts_pin = -EINVAL;
> 	int ret = -ENODEV;
> 
> 	BUILD_BUG_ON(ATMEL_SERIAL_RINGSIZE & (ATMEL_SERIAL_RINGSIZE - 1));
> @@ -2364,13 +2366,18 @@ static int atmel_serial_probe(struct platform_device *pdev)
> 	 * structs to indicate "no RTS GPIO" instead of open-coding some
> 	 * invalid value everywhere.
> 	 */
> -	if (pdata->rts_gpio > 0) {
> -		ret = devm_gpio_request(&pdev->dev, pdata->rts_gpio, "RTS");
> +	if (pdata && pdata->rts_gpio > 0)
> +		rts_pin = pdata->rts_gpio;
> +	else if (np)
> +		rts_pin = of_get_named_gpio(np, "rts-gpios", 0);
> +

	use directly port->rts_gpio  so we can drop the assign after

	if the gpio is invalid the port->rts_gpio need to be < 0 too

Best Regards,
J.

> +	if (gpio_is_valid(rts_pin)) {
> +		ret = devm_gpio_request(&pdev->dev, rts_pin, "RTS");
> 		if (ret) {
> 			dev_err(&pdev->dev, "error requesting RTS GPIO\n");
> 			goto err;
> 		}
> -		port->rts_gpio = pdata->rts_gpio;
> +		port->rts_gpio = rts_pin;
> 		ret = gpio_direction_output(port->rts_gpio, 0);
> 		if (ret) {
> 			dev_err(&pdev->dev, "error setting up RTS GPIO\n");
> -- 
> 1.8.2.2
> 

[PATCH] ARM/serial: at91: specify RTS in DT using gpio

[Jean-Christophe PLAGNIOL-VILLARD]

On Nov 5, 2013, at 11:29 PM, Nicolas Ferre <nicolas.ferre@atmel.com> wrote:

> Signed-off-by: Nicolas Ferre <nicolas.ferre@atmel.com>
> ---
> Documentation/devicetree/bindings/serial/atmel-usart.txt |  3 +++
> drivers/tty/serial/atmel_serial.c                        | 13 ++++++++++---
> 2 files changed, 13 insertions(+), 3 deletions(-)
> 
> diff --git a/Documentation/devicetree/bindings/serial/atmel-usart.txt b/Documentation/devicetree/bindings/serial/atmel-usart.txt
> index 2191dcb..3adc61c 100644
> --- a/Documentation/devicetree/bindings/serial/atmel-usart.txt
> +++ b/Documentation/devicetree/bindings/serial/atmel-usart.txt
> @@ -10,6 +10,8 @@ Required properties:
> Optional properties:
> - atmel,use-dma-rx: use of PDC or DMA for receiving data
> - atmel,use-dma-tx: use of PDC or DMA for transmitting data
> +- rts-gpios: specify a GPIO for RTS line. It will use specified PIO instead of the peripheral
> +  function pin for the USART RTS feature. If unsure, don't specify this property.
> - add dma bindings for dma transfer:
> 	- dmas: DMA specifier, consisting of a phandle to DMA controller node,
> 		memory peripheral interface and USART DMA channel ID, FIFO configuration.
> @@ -28,6 +30,7 @@ Example:
> 		interrupts = <7>;
> 		atmel,use-dma-rx;
> 		atmel,use-dma-tx;
> +		rts-gpios = <&pioD 15 0>;
> 	};

gpios means 2 gpio 

I?ll rts-gpio as we can only use one anyway



> 
> - use DMA:
> diff --git a/drivers/tty/serial/atmel_serial.c b/drivers/tty/serial/atmel_serial.c
> index b4e0794..71f8ea9 100644
> --- a/drivers/tty/serial/atmel_serial.c
> +++ b/drivers/tty/serial/atmel_serial.c
> @@ -35,6 +35,7 @@
> #include <linux/platform_device.h>
> #include <linux/of.h>
> #include <linux/of_device.h>
> +#include <linux/of_gpio.h>
> #include <linux/dma-mapping.h>
> #include <linux/atmel_pdc.h>
> #include <linux/atmel_serial.h>
> @@ -2327,6 +2328,7 @@ static int atmel_serial_probe(struct platform_device *pdev)
> 	struct device_node *np = pdev->dev.of_node;
> 	struct atmel_uart_data *pdata = dev_get_platdata(&pdev->dev);
> 	void *data;
> +	int rts_pin = -EINVAL;
> 	int ret = -ENODEV;
> 
> 	BUILD_BUG_ON(ATMEL_SERIAL_RINGSIZE & (ATMEL_SERIAL_RINGSIZE - 1));
> @@ -2364,13 +2366,18 @@ static int atmel_serial_probe(struct platform_device *pdev)
> 	 * structs to indicate "no RTS GPIO" instead of open-coding some
> 	 * invalid value everywhere.
> 	 */
> -	if (pdata->rts_gpio > 0) {
> -		ret = devm_gpio_request(&pdev->dev, pdata->rts_gpio, "RTS");
> +	if (pdata && pdata->rts_gpio > 0)
> +		rts_pin = pdata->rts_gpio;
> +	else if (np)
> +		rts_pin = of_get_named_gpio(np, "rts-gpios", 0);
> +

	use directly port->rts_gpio  so we can drop the assign after

	if the gpio is invalid the port->rts_gpio need to be < 0 too

Best Regards,
J.

> +	if (gpio_is_valid(rts_pin)) {
> +		ret = devm_gpio_request(&pdev->dev, rts_pin, "RTS");
> 		if (ret) {
> 			dev_err(&pdev->dev, "error requesting RTS GPIO\n");
> 			goto err;
> 		}
> -		port->rts_gpio = pdata->rts_gpio;
> +		port->rts_gpio = rts_pin;
> 		ret = gpio_direction_output(port->rts_gpio, 0);
> 		if (ret) {
> 			dev_err(&pdev->dev, "error setting up RTS GPIO\n");
> -- 
> 1.8.2.2
> 

Re: [PATCH] ARM/serial: at91: specify RTS in DT using gpio

[Andrew Lunn]

On Tue, Nov 05, 2013 at 11:59:38PM +0800, Jean-Christophe PLAGNIOL-VILLARD wrote:
> 
> On Nov 5, 2013, at 11:29 PM, Nicolas Ferre <nicolas.ferre@atmel.com> wrote:
> 
> > Signed-off-by: Nicolas Ferre <nicolas.ferre@atmel.com>
> > ---
> > Documentation/devicetree/bindings/serial/atmel-usart.txt |  3 +++
> > drivers/tty/serial/atmel_serial.c                        | 13 ++++++++++---
> > 2 files changed, 13 insertions(+), 3 deletions(-)
> > 
> > diff --git a/Documentation/devicetree/bindings/serial/atmel-usart.txt b/Documentation/devicetree/bindings/serial/atmel-usart.txt
> > index 2191dcb..3adc61c 100644
> > --- a/Documentation/devicetree/bindings/serial/atmel-usart.txt
> > +++ b/Documentation/devicetree/bindings/serial/atmel-usart.txt
> > @@ -10,6 +10,8 @@ Required properties:
> > Optional properties:
> > - atmel,use-dma-rx: use of PDC or DMA for receiving data
> > - atmel,use-dma-tx: use of PDC or DMA for transmitting data
> > +- rts-gpios: specify a GPIO for RTS line. It will use specified PIO instead of the peripheral
> > +  function pin for the USART RTS feature. If unsure, don't specify this property.
> > - add dma bindings for dma transfer:
> > 	- dmas: DMA specifier, consisting of a phandle to DMA controller node,
> > 		memory peripheral interface and USART DMA channel ID, FIFO configuration.
> > @@ -28,6 +30,7 @@ Example:
> > 		interrupts = <7>;
> > 		atmel,use-dma-rx;
> > 		atmel,use-dma-tx;
> > +		rts-gpios = <&pioD 15 0>;
> > 	};
> 
> gpios means 2 gpio 
> 
> I’ll rts-gpio as we can only use one anyway

Hi Jean

Have you read Documentation/devicetree/bindings/gpio/gpio.txt ?

It says:

GPIO properties should be named "[<name>-]gpios". Exact meaning of
each gpios property must be documented in the device tree binding for
each device.

There is also an example of a node with a single gpio using the
property name gpios. So plural is correct, even if there is only one
gpio.

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

[PATCH] ARM/serial: at91: specify RTS in DT using gpio

[Andrew Lunn]

On Tue, Nov 05, 2013 at 11:59:38PM +0800, Jean-Christophe PLAGNIOL-VILLARD wrote:
> 
> On Nov 5, 2013, at 11:29 PM, Nicolas Ferre <nicolas.ferre@atmel.com> wrote:
> 
> > Signed-off-by: Nicolas Ferre <nicolas.ferre@atmel.com>
> > ---
> > Documentation/devicetree/bindings/serial/atmel-usart.txt |  3 +++
> > drivers/tty/serial/atmel_serial.c                        | 13 ++++++++++---
> > 2 files changed, 13 insertions(+), 3 deletions(-)
> > 
> > diff --git a/Documentation/devicetree/bindings/serial/atmel-usart.txt b/Documentation/devicetree/bindings/serial/atmel-usart.txt
> > index 2191dcb..3adc61c 100644
> > --- a/Documentation/devicetree/bindings/serial/atmel-usart.txt
> > +++ b/Documentation/devicetree/bindings/serial/atmel-usart.txt
> > @@ -10,6 +10,8 @@ Required properties:
> > Optional properties:
> > - atmel,use-dma-rx: use of PDC or DMA for receiving data
> > - atmel,use-dma-tx: use of PDC or DMA for transmitting data
> > +- rts-gpios: specify a GPIO for RTS line. It will use specified PIO instead of the peripheral
> > +  function pin for the USART RTS feature. If unsure, don't specify this property.
> > - add dma bindings for dma transfer:
> > 	- dmas: DMA specifier, consisting of a phandle to DMA controller node,
> > 		memory peripheral interface and USART DMA channel ID, FIFO configuration.
> > @@ -28,6 +30,7 @@ Example:
> > 		interrupts = <7>;
> > 		atmel,use-dma-rx;
> > 		atmel,use-dma-tx;
> > +		rts-gpios = <&pioD 15 0>;
> > 	};
> 
> gpios means 2 gpio 
> 
> I?ll rts-gpio as we can only use one anyway

Hi Jean

Have you read Documentation/devicetree/bindings/gpio/gpio.txt ?

It says:

GPIO properties should be named "[<name>-]gpios". Exact meaning of
each gpios property must be documented in the device tree binding for
each device.

There is also an example of a node with a single gpio using the
property name gpios. So plural is correct, even if there is only one
gpio.

	Andrew

Re: [PATCH] ARM/serial: at91: switch atmel serial to use gpiolib

[Jean-Christophe PLAGNIOL-VILLARD]

On 13:35 Tue 05 Nov     , Linus Walleij wrote:
> This passes the errata fix using a GPIO to control the RTS pin
> on one of the AT91 chips to use gpiolib instead of the
> AT91-specific interfaces. Also remove the reliance on
> compile-time #defines and the cpu_* check and rely on the
> platform passing down the proper GPIO pin through platform
> data.
> 
> This is a prerequisite for getting rid of the local GPIO
> implementation in the AT91 platform and move toward
> multiplatform.
> 
> This also makes way for device tree conversion: the RTS
> GPIO pin can be passed by standard GPIO bindings.
> 
> Signed-off-by: Linus Walleij <linus.walleij@linaro.org>
> ---
> This is an alternative to the patch entitled
> "ARM/serial: at91: move machine quirk into machine"
> which needs testing to confirm this approach.
> Seeking ACKs on this if the approach seems OK to
> all parties.
> ---
>  arch/arm/mach-at91/at91rm9200_devices.c |  1 +
>  drivers/tty/serial/atmel_serial.c       | 51 +++++++++++++++++++++------------
>  include/linux/platform_data/atmel.h     |  1 +
>  3 files changed, 35 insertions(+), 18 deletions(-)
> 
> diff --git a/arch/arm/mach-at91/at91rm9200_devices.c b/arch/arm/mach-at91/at91rm9200_devices.c
> index c721e9b08066..51d4c08962f6 100644
> --- a/arch/arm/mach-at91/at91rm9200_devices.c
> +++ b/arch/arm/mach-at91/at91rm9200_devices.c
> @@ -961,6 +961,7 @@ static struct resource uart0_resources[] = {
>  static struct atmel_uart_data uart0_data = {
>  	.use_dma_tx	= 1,
>  	.use_dma_rx	= 1,
> +	.rts_gpio	= AT91_PIN_PA21,
>  };
>  
>  static u64 uart0_dmamask = DMA_BIT_MASK(32);
> diff --git a/drivers/tty/serial/atmel_serial.c b/drivers/tty/serial/atmel_serial.c
> index d067285a2d20..3d5c848cdfe1 100644
> --- a/drivers/tty/serial/atmel_serial.c
> +++ b/drivers/tty/serial/atmel_serial.c
> @@ -41,15 +41,11 @@
>  #include <linux/uaccess.h>
>  #include <linux/platform_data/atmel.h>
>  #include <linux/timer.h>
> +#include <linux/gpio.h>
>  
>  #include <asm/io.h>
>  #include <asm/ioctls.h>
>  
> -#ifdef CONFIG_ARM
> -#include <mach/cpu.h>
> -#include <asm/gpio.h>
> -#endif
> -
>  #define PDC_BUFFER_SIZE		512
>  /* Revisit: We should calculate this based on the actual port settings */
>  #define PDC_RX_TIMEOUT		(3 * 10)		/* 3 bytes */
> @@ -167,6 +163,7 @@ struct atmel_uart_port {
>  	struct circ_buf		rx_ring;
>  
>  	struct serial_rs485	rs485;		/* rs485 settings */
> +	int			rts_gpio;	/* optional RTS GPIO */
>  	unsigned int		tx_done_mask;
>  	bool			is_usart;	/* usart or uart */
>  	struct timer_list	uart_timer;	/* uart timer */
> @@ -300,20 +297,17 @@ static void atmel_set_mctrl(struct uart_port *port, u_int mctrl)
>  	unsigned int mode;
>  	struct atmel_uart_port *atmel_port = to_atmel_uart_port(port);
>  
> -#ifdef CONFIG_ARCH_AT91RM9200
> -	if (cpu_is_at91rm9200()) {
> -		/*
> -		 * AT91RM9200 Errata #39: RTS0 is not internally connected
> -		 * to PA21. We need to drive the pin manually.
> -		 */
> -		if (port->mapbase == AT91RM9200_BASE_US0) {
> -			if (mctrl & TIOCM_RTS)
> -				at91_set_gpio_value(AT91_PIN_PA21, 0);
> -			else
> -				at91_set_gpio_value(AT91_PIN_PA21, 1);
> -		}
> +	/*
> +	 * AT91RM9200 Errata #39: RTS0 is not internally connected
> +	 * to PA21. We need to drive the pin as a GPIO.
> +	 */
> +	if (gpio_is_valid(atmel_port->rts_gpio) &&
> +	    port->mapbase == AT91RM9200_BASE_US0) {
not really a fon of this hack

if we use a uart for rs485 we need to use a gpio for rts too

so we may need to find a better way
> +		if (mctrl & TIOCM_RTS)
> +			gpio_set_value(atmel_port->rts_gpio, 0);
> +		else
> +			gpio_set_value(atmel_port->rts_gpio, 1);
>  	}
> -#endif
>  
>  	if (mctrl & TIOCM_RTS)
>  		control |= ATMEL_US_RTSEN;
> @@ -2365,6 +2359,27 @@ static int atmel_serial_probe(struct platform_device *pdev)
>  	port = &atmel_ports[ret];
>  	port->backup_imr = 0;
>  	port->uart.line = ret;
> +	port->rts_gpio = -1; /* Invalid, zero could be valid */
> +	/*
> +	 * In theory the GPIO pin controlling RTS could be zero and
> +	 * this would be an improper check, but we know that the only
> +	 * existing case is != 0 and it's nice to use the zero-initialized
> +	 * structs to indicate "no RTS GPIO" instead of open-coding some
> +	 * invalid value everywhere.
> +	 */
> +	if (pdata->rts_gpio > 0) {
	this can not work 0 is a valid gpio if you do this you need to update
	ALL the bard filee too to set it as -EINVAL;
> +		ret = devm_gpio_request(&pdev->dev, pdata->rts_gpio, "RTS");
> +		if (ret) {
> +			dev_err(&pdev->dev, "error requesting RTS GPIO\n");
> +			goto err;
> +		}
> +		port->rts_gpio = pdata->rts_gpio;
> +		ret = gpio_direction_output(port->rts_gpio, 0);
> +		if (ret) {
> +			dev_err(&pdev->dev, "error setting up RTS GPIO\n");
> +			goto err;
> +		}
> +	}
>  
>  	ret = atmel_init_port(port, pdev);
>  	if (ret)
> diff --git a/include/linux/platform_data/atmel.h b/include/linux/platform_data/atmel.h
> index cea9f70133c5..e26b0c14edea 100644
> --- a/include/linux/platform_data/atmel.h
> +++ b/include/linux/platform_data/atmel.h
> @@ -84,6 +84,7 @@ struct atmel_uart_data {
>  	short			use_dma_rx;	/* use receive DMA? */
>  	void __iomem		*regs;		/* virt. base address, if any */
>  	struct serial_rs485	rs485;		/* rs485 settings */
> +	int			rts_gpio;	/* optional RTS GPIO */
>  };
>  
>   /* Touchscreen Controller */
> -- 
> 1.8.3.1
> 

[PATCH] ARM/serial: at91: switch atmel serial to use gpiolib

[Jean-Christophe PLAGNIOL-VILLARD]

On 13:35 Tue 05 Nov     , Linus Walleij wrote:
> This passes the errata fix using a GPIO to control the RTS pin
> on one of the AT91 chips to use gpiolib instead of the
> AT91-specific interfaces. Also remove the reliance on
> compile-time #defines and the cpu_* check and rely on the
> platform passing down the proper GPIO pin through platform
> data.
> 
> This is a prerequisite for getting rid of the local GPIO
> implementation in the AT91 platform and move toward
> multiplatform.
> 
> This also makes way for device tree conversion: the RTS
> GPIO pin can be passed by standard GPIO bindings.
> 
> Signed-off-by: Linus Walleij <linus.walleij@linaro.org>
> ---
> This is an alternative to the patch entitled
> "ARM/serial: at91: move machine quirk into machine"
> which needs testing to confirm this approach.
> Seeking ACKs on this if the approach seems OK to
> all parties.
> ---
>  arch/arm/mach-at91/at91rm9200_devices.c |  1 +
>  drivers/tty/serial/atmel_serial.c       | 51 +++++++++++++++++++++------------
>  include/linux/platform_data/atmel.h     |  1 +
>  3 files changed, 35 insertions(+), 18 deletions(-)
> 
> diff --git a/arch/arm/mach-at91/at91rm9200_devices.c b/arch/arm/mach-at91/at91rm9200_devices.c
> index c721e9b08066..51d4c08962f6 100644
> --- a/arch/arm/mach-at91/at91rm9200_devices.c
> +++ b/arch/arm/mach-at91/at91rm9200_devices.c
> @@ -961,6 +961,7 @@ static struct resource uart0_resources[] = {
>  static struct atmel_uart_data uart0_data = {
>  	.use_dma_tx	= 1,
>  	.use_dma_rx	= 1,
> +	.rts_gpio	= AT91_PIN_PA21,
>  };
>  
>  static u64 uart0_dmamask = DMA_BIT_MASK(32);
> diff --git a/drivers/tty/serial/atmel_serial.c b/drivers/tty/serial/atmel_serial.c
> index d067285a2d20..3d5c848cdfe1 100644
> --- a/drivers/tty/serial/atmel_serial.c
> +++ b/drivers/tty/serial/atmel_serial.c
> @@ -41,15 +41,11 @@
>  #include <linux/uaccess.h>
>  #include <linux/platform_data/atmel.h>
>  #include <linux/timer.h>
> +#include <linux/gpio.h>
>  
>  #include <asm/io.h>
>  #include <asm/ioctls.h>
>  
> -#ifdef CONFIG_ARM
> -#include <mach/cpu.h>
> -#include <asm/gpio.h>
> -#endif
> -
>  #define PDC_BUFFER_SIZE		512
>  /* Revisit: We should calculate this based on the actual port settings */
>  #define PDC_RX_TIMEOUT		(3 * 10)		/* 3 bytes */
> @@ -167,6 +163,7 @@ struct atmel_uart_port {
>  	struct circ_buf		rx_ring;
>  
>  	struct serial_rs485	rs485;		/* rs485 settings */
> +	int			rts_gpio;	/* optional RTS GPIO */
>  	unsigned int		tx_done_mask;
>  	bool			is_usart;	/* usart or uart */
>  	struct timer_list	uart_timer;	/* uart timer */
> @@ -300,20 +297,17 @@ static void atmel_set_mctrl(struct uart_port *port, u_int mctrl)
>  	unsigned int mode;
>  	struct atmel_uart_port *atmel_port = to_atmel_uart_port(port);
>  
> -#ifdef CONFIG_ARCH_AT91RM9200
> -	if (cpu_is_at91rm9200()) {
> -		/*
> -		 * AT91RM9200 Errata #39: RTS0 is not internally connected
> -		 * to PA21. We need to drive the pin manually.
> -		 */
> -		if (port->mapbase == AT91RM9200_BASE_US0) {
> -			if (mctrl & TIOCM_RTS)
> -				at91_set_gpio_value(AT91_PIN_PA21, 0);
> -			else
> -				at91_set_gpio_value(AT91_PIN_PA21, 1);
> -		}
> +	/*
> +	 * AT91RM9200 Errata #39: RTS0 is not internally connected
> +	 * to PA21. We need to drive the pin as a GPIO.
> +	 */
> +	if (gpio_is_valid(atmel_port->rts_gpio) &&
> +	    port->mapbase == AT91RM9200_BASE_US0) {
not really a fon of this hack

if we use a uart for rs485 we need to use a gpio for rts too

so we may need to find a better way
> +		if (mctrl & TIOCM_RTS)
> +			gpio_set_value(atmel_port->rts_gpio, 0);
> +		else
> +			gpio_set_value(atmel_port->rts_gpio, 1);
>  	}
> -#endif
>  
>  	if (mctrl & TIOCM_RTS)
>  		control |= ATMEL_US_RTSEN;
> @@ -2365,6 +2359,27 @@ static int atmel_serial_probe(struct platform_device *pdev)
>  	port = &atmel_ports[ret];
>  	port->backup_imr = 0;
>  	port->uart.line = ret;
> +	port->rts_gpio = -1; /* Invalid, zero could be valid */
> +	/*
> +	 * In theory the GPIO pin controlling RTS could be zero and
> +	 * this would be an improper check, but we know that the only
> +	 * existing case is != 0 and it's nice to use the zero-initialized
> +	 * structs to indicate "no RTS GPIO" instead of open-coding some
> +	 * invalid value everywhere.
> +	 */
> +	if (pdata->rts_gpio > 0) {
	this can not work 0 is a valid gpio if you do this you need to update
	ALL the bard filee too to set it as -EINVAL;
> +		ret = devm_gpio_request(&pdev->dev, pdata->rts_gpio, "RTS");
> +		if (ret) {
> +			dev_err(&pdev->dev, "error requesting RTS GPIO\n");
> +			goto err;
> +		}
> +		port->rts_gpio = pdata->rts_gpio;
> +		ret = gpio_direction_output(port->rts_gpio, 0);
> +		if (ret) {
> +			dev_err(&pdev->dev, "error setting up RTS GPIO\n");
> +			goto err;
> +		}
> +	}
>  
>  	ret = atmel_init_port(port, pdev);
>  	if (ret)
> diff --git a/include/linux/platform_data/atmel.h b/include/linux/platform_data/atmel.h
> index cea9f70133c5..e26b0c14edea 100644
> --- a/include/linux/platform_data/atmel.h
> +++ b/include/linux/platform_data/atmel.h
> @@ -84,6 +84,7 @@ struct atmel_uart_data {
>  	short			use_dma_rx;	/* use receive DMA? */
>  	void __iomem		*regs;		/* virt. base address, if any */
>  	struct serial_rs485	rs485;		/* rs485 settings */
> +	int			rts_gpio;	/* optional RTS GPIO */
>  };
>  
>   /* Touchscreen Controller */
> -- 
> 1.8.3.1
> 

Re: [PATCH] ARM/serial: at91: switch atmel serial to use gpiolib

[Nicolas Ferre]

On 05/11/2013 17:28, Jean-Christophe PLAGNIOL-VILLARD :
> On 13:35 Tue 05 Nov     , Linus Walleij wrote:
>> This passes the errata fix using a GPIO to control the RTS pin
>> on one of the AT91 chips to use gpiolib instead of the
>> AT91-specific interfaces. Also remove the reliance on
>> compile-time #defines and the cpu_* check and rely on the
>> platform passing down the proper GPIO pin through platform
>> data.
>>
>> This is a prerequisite for getting rid of the local GPIO
>> implementation in the AT91 platform and move toward
>> multiplatform.
>>
>> This also makes way for device tree conversion: the RTS
>> GPIO pin can be passed by standard GPIO bindings.
>>
>> Signed-off-by: Linus Walleij <linus.walleij@linaro.org>
>> ---
>> This is an alternative to the patch entitled
>> "ARM/serial: at91: move machine quirk into machine"
>> which needs testing to confirm this approach.
>> Seeking ACKs on this if the approach seems OK to
>> all parties.
>> ---
>>   arch/arm/mach-at91/at91rm9200_devices.c |  1 +
>>   drivers/tty/serial/atmel_serial.c       | 51 +++++++++++++++++++++------------
>>   include/linux/platform_data/atmel.h     |  1 +
>>   3 files changed, 35 insertions(+), 18 deletions(-)
>>
>> diff --git a/arch/arm/mach-at91/at91rm9200_devices.c b/arch/arm/mach-at91/at91rm9200_devices.c
>> index c721e9b08066..51d4c08962f6 100644
>> --- a/arch/arm/mach-at91/at91rm9200_devices.c
>> +++ b/arch/arm/mach-at91/at91rm9200_devices.c
>> @@ -961,6 +961,7 @@ static struct resource uart0_resources[] = {
>>   static struct atmel_uart_data uart0_data = {
>>   	.use_dma_tx	= 1,
>>   	.use_dma_rx	= 1,
>> +	.rts_gpio	= AT91_PIN_PA21,
>>   };
>>
>>   static u64 uart0_dmamask = DMA_BIT_MASK(32);
>> diff --git a/drivers/tty/serial/atmel_serial.c b/drivers/tty/serial/atmel_serial.c
>> index d067285a2d20..3d5c848cdfe1 100644
>> --- a/drivers/tty/serial/atmel_serial.c
>> +++ b/drivers/tty/serial/atmel_serial.c
>> @@ -41,15 +41,11 @@
>>   #include <linux/uaccess.h>
>>   #include <linux/platform_data/atmel.h>
>>   #include <linux/timer.h>
>> +#include <linux/gpio.h>
>>
>>   #include <asm/io.h>
>>   #include <asm/ioctls.h>
>>
>> -#ifdef CONFIG_ARM
>> -#include <mach/cpu.h>
>> -#include <asm/gpio.h>
>> -#endif
>> -
>>   #define PDC_BUFFER_SIZE		512
>>   /* Revisit: We should calculate this based on the actual port settings */
>>   #define PDC_RX_TIMEOUT		(3 * 10)		/* 3 bytes */
>> @@ -167,6 +163,7 @@ struct atmel_uart_port {
>>   	struct circ_buf		rx_ring;
>>
>>   	struct serial_rs485	rs485;		/* rs485 settings */
>> +	int			rts_gpio;	/* optional RTS GPIO */
>>   	unsigned int		tx_done_mask;
>>   	bool			is_usart;	/* usart or uart */
>>   	struct timer_list	uart_timer;	/* uart timer */
>> @@ -300,20 +297,17 @@ static void atmel_set_mctrl(struct uart_port *port, u_int mctrl)
>>   	unsigned int mode;
>>   	struct atmel_uart_port *atmel_port = to_atmel_uart_port(port);
>>
>> -#ifdef CONFIG_ARCH_AT91RM9200
>> -	if (cpu_is_at91rm9200()) {
>> -		/*
>> -		 * AT91RM9200 Errata #39: RTS0 is not internally connected
>> -		 * to PA21. We need to drive the pin manually.
>> -		 */
>> -		if (port->mapbase == AT91RM9200_BASE_US0) {
>> -			if (mctrl & TIOCM_RTS)
>> -				at91_set_gpio_value(AT91_PIN_PA21, 0);
>> -			else
>> -				at91_set_gpio_value(AT91_PIN_PA21, 1);
>> -		}
>> +	/*
>> +	 * AT91RM9200 Errata #39: RTS0 is not internally connected
>> +	 * to PA21. We need to drive the pin as a GPIO.
>> +	 */
>> +	if (gpio_is_valid(atmel_port->rts_gpio) &&
>> +	    port->mapbase == AT91RM9200_BASE_US0) {
> not really a fon of this hack
>
> if we use a uart for rs485 we need to use a gpio for rts too
>
> so we may need to find a better way

Yes... but the problem from now on is to remove another hack which 
prevents us from moving forward to the single zImage direction (with the 
help of Linus W.).

So, before that we can enhance the code for RS-485, I am in favor of a 
move with this patch which is equivalent to what we already have in the 
driver nowadays.

But is is true that we can add another patch on top of this one to 
remove the restriction on "AT91RM9200_BASE_US0". If you feel it is a 
good idea, go on and send a patch.

Bye,



>> +		if (mctrl & TIOCM_RTS)
>> +			gpio_set_value(atmel_port->rts_gpio, 0);
>> +		else
>> +			gpio_set_value(atmel_port->rts_gpio, 1);
>>   	}
>> -#endif
>>
>>   	if (mctrl & TIOCM_RTS)
>>   		control |= ATMEL_US_RTSEN;
>> @@ -2365,6 +2359,27 @@ static int atmel_serial_probe(struct platform_device *pdev)
>>   	port = &atmel_ports[ret];
>>   	port->backup_imr = 0;
>>   	port->uart.line = ret;
>> +	port->rts_gpio = -1; /* Invalid, zero could be valid */
>> +	/*
>> +	 * In theory the GPIO pin controlling RTS could be zero and
>> +	 * this would be an improper check, but we know that the only
>> +	 * existing case is != 0 and it's nice to use the zero-initialized
>> +	 * structs to indicate "no RTS GPIO" instead of open-coding some
>> +	 * invalid value everywhere.
>> +	 */
>> +	if (pdata->rts_gpio > 0) {
> 	this can not work 0 is a valid gpio if you do this you need to update
> 	ALL the bard filee too to set it as -EINVAL;
>> +		ret = devm_gpio_request(&pdev->dev, pdata->rts_gpio, "RTS");
>> +		if (ret) {
>> +			dev_err(&pdev->dev, "error requesting RTS GPIO\n");
>> +			goto err;
>> +		}
>> +		port->rts_gpio = pdata->rts_gpio;
>> +		ret = gpio_direction_output(port->rts_gpio, 0);
>> +		if (ret) {
>> +			dev_err(&pdev->dev, "error setting up RTS GPIO\n");
>> +			goto err;
>> +		}
>> +	}
>>
>>   	ret = atmel_init_port(port, pdev);
>>   	if (ret)
>> diff --git a/include/linux/platform_data/atmel.h b/include/linux/platform_data/atmel.h
>> index cea9f70133c5..e26b0c14edea 100644
>> --- a/include/linux/platform_data/atmel.h
>> +++ b/include/linux/platform_data/atmel.h
>> @@ -84,6 +84,7 @@ struct atmel_uart_data {
>>   	short			use_dma_rx;	/* use receive DMA? */
>>   	void __iomem		*regs;		/* virt. base address, if any */
>>   	struct serial_rs485	rs485;		/* rs485 settings */
>> +	int			rts_gpio;	/* optional RTS GPIO */
>>   };
>>
>>    /* Touchscreen Controller */
>> --
>> 1.8.3.1
>>
>
>


-- 
Nicolas Ferre

[PATCH] ARM/serial: at91: switch atmel serial to use gpiolib

[Nicolas Ferre]

On 05/11/2013 17:28, Jean-Christophe PLAGNIOL-VILLARD :
> On 13:35 Tue 05 Nov     , Linus Walleij wrote:
>> This passes the errata fix using a GPIO to control the RTS pin
>> on one of the AT91 chips to use gpiolib instead of the
>> AT91-specific interfaces. Also remove the reliance on
>> compile-time #defines and the cpu_* check and rely on the
>> platform passing down the proper GPIO pin through platform
>> data.
>>
>> This is a prerequisite for getting rid of the local GPIO
>> implementation in the AT91 platform and move toward
>> multiplatform.
>>
>> This also makes way for device tree conversion: the RTS
>> GPIO pin can be passed by standard GPIO bindings.
>>
>> Signed-off-by: Linus Walleij <linus.walleij@linaro.org>
>> ---
>> This is an alternative to the patch entitled
>> "ARM/serial: at91: move machine quirk into machine"
>> which needs testing to confirm this approach.
>> Seeking ACKs on this if the approach seems OK to
>> all parties.
>> ---
>>   arch/arm/mach-at91/at91rm9200_devices.c |  1 +
>>   drivers/tty/serial/atmel_serial.c       | 51 +++++++++++++++++++++------------
>>   include/linux/platform_data/atmel.h     |  1 +
>>   3 files changed, 35 insertions(+), 18 deletions(-)
>>
>> diff --git a/arch/arm/mach-at91/at91rm9200_devices.c b/arch/arm/mach-at91/at91rm9200_devices.c
>> index c721e9b08066..51d4c08962f6 100644
>> --- a/arch/arm/mach-at91/at91rm9200_devices.c
>> +++ b/arch/arm/mach-at91/at91rm9200_devices.c
>> @@ -961,6 +961,7 @@ static struct resource uart0_resources[] = {
>>   static struct atmel_uart_data uart0_data = {
>>   	.use_dma_tx	= 1,
>>   	.use_dma_rx	= 1,
>> +	.rts_gpio	= AT91_PIN_PA21,
>>   };
>>
>>   static u64 uart0_dmamask = DMA_BIT_MASK(32);
>> diff --git a/drivers/tty/serial/atmel_serial.c b/drivers/tty/serial/atmel_serial.c
>> index d067285a2d20..3d5c848cdfe1 100644
>> --- a/drivers/tty/serial/atmel_serial.c
>> +++ b/drivers/tty/serial/atmel_serial.c
>> @@ -41,15 +41,11 @@
>>   #include <linux/uaccess.h>
>>   #include <linux/platform_data/atmel.h>
>>   #include <linux/timer.h>
>> +#include <linux/gpio.h>
>>
>>   #include <asm/io.h>
>>   #include <asm/ioctls.h>
>>
>> -#ifdef CONFIG_ARM
>> -#include <mach/cpu.h>
>> -#include <asm/gpio.h>
>> -#endif
>> -
>>   #define PDC_BUFFER_SIZE		512
>>   /* Revisit: We should calculate this based on the actual port settings */
>>   #define PDC_RX_TIMEOUT		(3 * 10)		/* 3 bytes */
>> @@ -167,6 +163,7 @@ struct atmel_uart_port {
>>   	struct circ_buf		rx_ring;
>>
>>   	struct serial_rs485	rs485;		/* rs485 settings */
>> +	int			rts_gpio;	/* optional RTS GPIO */
>>   	unsigned int		tx_done_mask;
>>   	bool			is_usart;	/* usart or uart */
>>   	struct timer_list	uart_timer;	/* uart timer */
>> @@ -300,20 +297,17 @@ static void atmel_set_mctrl(struct uart_port *port, u_int mctrl)
>>   	unsigned int mode;
>>   	struct atmel_uart_port *atmel_port = to_atmel_uart_port(port);
>>
>> -#ifdef CONFIG_ARCH_AT91RM9200
>> -	if (cpu_is_at91rm9200()) {
>> -		/*
>> -		 * AT91RM9200 Errata #39: RTS0 is not internally connected
>> -		 * to PA21. We need to drive the pin manually.
>> -		 */
>> -		if (port->mapbase == AT91RM9200_BASE_US0) {
>> -			if (mctrl & TIOCM_RTS)
>> -				at91_set_gpio_value(AT91_PIN_PA21, 0);
>> -			else
>> -				at91_set_gpio_value(AT91_PIN_PA21, 1);
>> -		}
>> +	/*
>> +	 * AT91RM9200 Errata #39: RTS0 is not internally connected
>> +	 * to PA21. We need to drive the pin as a GPIO.
>> +	 */
>> +	if (gpio_is_valid(atmel_port->rts_gpio) &&
>> +	    port->mapbase == AT91RM9200_BASE_US0) {
> not really a fon of this hack
>
> if we use a uart for rs485 we need to use a gpio for rts too
>
> so we may need to find a better way

Yes... but the problem from now on is to remove another hack which 
prevents us from moving forward to the single zImage direction (with the 
help of Linus W.).

So, before that we can enhance the code for RS-485, I am in favor of a 
move with this patch which is equivalent to what we already have in the 
driver nowadays.

But is is true that we can add another patch on top of this one to 
remove the restriction on "AT91RM9200_BASE_US0". If you feel it is a 
good idea, go on and send a patch.

Bye,



>> +		if (mctrl & TIOCM_RTS)
>> +			gpio_set_value(atmel_port->rts_gpio, 0);
>> +		else
>> +			gpio_set_value(atmel_port->rts_gpio, 1);
>>   	}
>> -#endif
>>
>>   	if (mctrl & TIOCM_RTS)
>>   		control |= ATMEL_US_RTSEN;
>> @@ -2365,6 +2359,27 @@ static int atmel_serial_probe(struct platform_device *pdev)
>>   	port = &atmel_ports[ret];
>>   	port->backup_imr = 0;
>>   	port->uart.line = ret;
>> +	port->rts_gpio = -1; /* Invalid, zero could be valid */
>> +	/*
>> +	 * In theory the GPIO pin controlling RTS could be zero and
>> +	 * this would be an improper check, but we know that the only
>> +	 * existing case is != 0 and it's nice to use the zero-initialized
>> +	 * structs to indicate "no RTS GPIO" instead of open-coding some
>> +	 * invalid value everywhere.
>> +	 */
>> +	if (pdata->rts_gpio > 0) {
> 	this can not work 0 is a valid gpio if you do this you need to update
> 	ALL the bard filee too to set it as -EINVAL;
>> +		ret = devm_gpio_request(&pdev->dev, pdata->rts_gpio, "RTS");
>> +		if (ret) {
>> +			dev_err(&pdev->dev, "error requesting RTS GPIO\n");
>> +			goto err;
>> +		}
>> +		port->rts_gpio = pdata->rts_gpio;
>> +		ret = gpio_direction_output(port->rts_gpio, 0);
>> +		if (ret) {
>> +			dev_err(&pdev->dev, "error setting up RTS GPIO\n");
>> +			goto err;
>> +		}
>> +	}
>>
>>   	ret = atmel_init_port(port, pdev);
>>   	if (ret)
>> diff --git a/include/linux/platform_data/atmel.h b/include/linux/platform_data/atmel.h
>> index cea9f70133c5..e26b0c14edea 100644
>> --- a/include/linux/platform_data/atmel.h
>> +++ b/include/linux/platform_data/atmel.h
>> @@ -84,6 +84,7 @@ struct atmel_uart_data {
>>   	short			use_dma_rx;	/* use receive DMA? */
>>   	void __iomem		*regs;		/* virt. base address, if any */
>>   	struct serial_rs485	rs485;		/* rs485 settings */
>> +	int			rts_gpio;	/* optional RTS GPIO */
>>   };
>>
>>    /* Touchscreen Controller */
>> --
>> 1.8.3.1
>>
>
>


-- 
Nicolas Ferre

Re: [PATCH] ARM/serial: at91: specify RTS in DT using gpio

[Nicolas Ferre]

On 05/11/2013 16:59, Jean-Christophe PLAGNIOL-VILLARD :
>
> On Nov 5, 2013, at 11:29 PM, Nicolas Ferre <nicolas.ferre@atmel.com> wrote:
>
>> Signed-off-by: Nicolas Ferre <nicolas.ferre@atmel.com>
>> ---
>> Documentation/devicetree/bindings/serial/atmel-usart.txt |  3 +++
>> drivers/tty/serial/atmel_serial.c                        | 13 ++++++++++---
>> 2 files changed, 13 insertions(+), 3 deletions(-)
>>
>> diff --git a/Documentation/devicetree/bindings/serial/atmel-usart.txt b/Documentation/devicetree/bindings/serial/atmel-usart.txt
>> index 2191dcb..3adc61c 100644
>> --- a/Documentation/devicetree/bindings/serial/atmel-usart.txt
>> +++ b/Documentation/devicetree/bindings/serial/atmel-usart.txt
>> @@ -10,6 +10,8 @@ Required properties:
>> Optional properties:
>> - atmel,use-dma-rx: use of PDC or DMA for receiving data
>> - atmel,use-dma-tx: use of PDC or DMA for transmitting data
>> +- rts-gpios: specify a GPIO for RTS line. It will use specified PIO instead of the peripheral
>> +  function pin for the USART RTS feature. If unsure, don't specify this property.
>> - add dma bindings for dma transfer:
>> 	- dmas: DMA specifier, consisting of a phandle to DMA controller node,
>> 		memory peripheral interface and USART DMA channel ID, FIFO configuration.
>> @@ -28,6 +30,7 @@ Example:
>> 		interrupts = <7>;
>> 		atmel,use-dma-rx;
>> 		atmel,use-dma-tx;
>> +		rts-gpios = <&pioD 15 0>;
>> 	};
>
> gpios means 2 gpio
>
> I’ll rts-gpio as we can only use one anyway

Cf. Andrew's comment.


>> - use DMA:
>> diff --git a/drivers/tty/serial/atmel_serial.c b/drivers/tty/serial/atmel_serial.c
>> index b4e0794..71f8ea9 100644
>> --- a/drivers/tty/serial/atmel_serial.c
>> +++ b/drivers/tty/serial/atmel_serial.c
>> @@ -35,6 +35,7 @@
>> #include <linux/platform_device.h>
>> #include <linux/of.h>
>> #include <linux/of_device.h>
>> +#include <linux/of_gpio.h>
>> #include <linux/dma-mapping.h>
>> #include <linux/atmel_pdc.h>
>> #include <linux/atmel_serial.h>
>> @@ -2327,6 +2328,7 @@ static int atmel_serial_probe(struct platform_device *pdev)
>> 	struct device_node *np = pdev->dev.of_node;
>> 	struct atmel_uart_data *pdata = dev_get_platdata(&pdev->dev);
>> 	void *data;
>> +	int rts_pin = -EINVAL;
>> 	int ret = -ENODEV;
>>
>> 	BUILD_BUG_ON(ATMEL_SERIAL_RINGSIZE & (ATMEL_SERIAL_RINGSIZE - 1));
>> @@ -2364,13 +2366,18 @@ static int atmel_serial_probe(struct platform_device *pdev)
>> 	 * structs to indicate "no RTS GPIO" instead of open-coding some
>> 	 * invalid value everywhere.
>> 	 */
>> -	if (pdata->rts_gpio > 0) {
>> -		ret = devm_gpio_request(&pdev->dev, pdata->rts_gpio, "RTS");
>> +	if (pdata && pdata->rts_gpio > 0)
>> +		rts_pin = pdata->rts_gpio;
>> +	else if (np)
>> +		rts_pin = of_get_named_gpio(np, "rts-gpios", 0);
>> +
>
> 	use directly port->rts_gpio  so we can drop the assign after
>
> 	if the gpio is invalid the port->rts_gpio need to be < 0 too

Yes this is definitively an option. I cook a v2 right now.

>
> Best Regards,
> J.
>
>> +	if (gpio_is_valid(rts_pin)) {
>> +		ret = devm_gpio_request(&pdev->dev, rts_pin, "RTS");
>> 		if (ret) {
>> 			dev_err(&pdev->dev, "error requesting RTS GPIO\n");
>> 			goto err;
>> 		}
>> -		port->rts_gpio = pdata->rts_gpio;
>> +		port->rts_gpio = rts_pin;
>> 		ret = gpio_direction_output(port->rts_gpio, 0);
>> 		if (ret) {
>> 			dev_err(&pdev->dev, "error setting up RTS GPIO\n");
>> --
>> 1.8.2.2
>>
>
>
>


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

[PATCH] ARM/serial: at91: specify RTS in DT using gpio

[Nicolas Ferre]

On 05/11/2013 16:59, Jean-Christophe PLAGNIOL-VILLARD :
>
> On Nov 5, 2013, at 11:29 PM, Nicolas Ferre <nicolas.ferre@atmel.com> wrote:
>
>> Signed-off-by: Nicolas Ferre <nicolas.ferre@atmel.com>
>> ---
>> Documentation/devicetree/bindings/serial/atmel-usart.txt |  3 +++
>> drivers/tty/serial/atmel_serial.c                        | 13 ++++++++++---
>> 2 files changed, 13 insertions(+), 3 deletions(-)
>>
>> diff --git a/Documentation/devicetree/bindings/serial/atmel-usart.txt b/Documentation/devicetree/bindings/serial/atmel-usart.txt
>> index 2191dcb..3adc61c 100644
>> --- a/Documentation/devicetree/bindings/serial/atmel-usart.txt
>> +++ b/Documentation/devicetree/bindings/serial/atmel-usart.txt
>> @@ -10,6 +10,8 @@ Required properties:
>> Optional properties:
>> - atmel,use-dma-rx: use of PDC or DMA for receiving data
>> - atmel,use-dma-tx: use of PDC or DMA for transmitting data
>> +- rts-gpios: specify a GPIO for RTS line. It will use specified PIO instead of the peripheral
>> +  function pin for the USART RTS feature. If unsure, don't specify this property.
>> - add dma bindings for dma transfer:
>> 	- dmas: DMA specifier, consisting of a phandle to DMA controller node,
>> 		memory peripheral interface and USART DMA channel ID, FIFO configuration.
>> @@ -28,6 +30,7 @@ Example:
>> 		interrupts = <7>;
>> 		atmel,use-dma-rx;
>> 		atmel,use-dma-tx;
>> +		rts-gpios = <&pioD 15 0>;
>> 	};
>
> gpios means 2 gpio
>
> I?ll rts-gpio as we can only use one anyway

Cf. Andrew's comment.


>> - use DMA:
>> diff --git a/drivers/tty/serial/atmel_serial.c b/drivers/tty/serial/atmel_serial.c
>> index b4e0794..71f8ea9 100644
>> --- a/drivers/tty/serial/atmel_serial.c
>> +++ b/drivers/tty/serial/atmel_serial.c
>> @@ -35,6 +35,7 @@
>> #include <linux/platform_device.h>
>> #include <linux/of.h>
>> #include <linux/of_device.h>
>> +#include <linux/of_gpio.h>
>> #include <linux/dma-mapping.h>
>> #include <linux/atmel_pdc.h>
>> #include <linux/atmel_serial.h>
>> @@ -2327,6 +2328,7 @@ static int atmel_serial_probe(struct platform_device *pdev)
>> 	struct device_node *np = pdev->dev.of_node;
>> 	struct atmel_uart_data *pdata = dev_get_platdata(&pdev->dev);
>> 	void *data;
>> +	int rts_pin = -EINVAL;
>> 	int ret = -ENODEV;
>>
>> 	BUILD_BUG_ON(ATMEL_SERIAL_RINGSIZE & (ATMEL_SERIAL_RINGSIZE - 1));
>> @@ -2364,13 +2366,18 @@ static int atmel_serial_probe(struct platform_device *pdev)
>> 	 * structs to indicate "no RTS GPIO" instead of open-coding some
>> 	 * invalid value everywhere.
>> 	 */
>> -	if (pdata->rts_gpio > 0) {
>> -		ret = devm_gpio_request(&pdev->dev, pdata->rts_gpio, "RTS");
>> +	if (pdata && pdata->rts_gpio > 0)
>> +		rts_pin = pdata->rts_gpio;
>> +	else if (np)
>> +		rts_pin = of_get_named_gpio(np, "rts-gpios", 0);
>> +
>
> 	use directly port->rts_gpio  so we can drop the assign after
>
> 	if the gpio is invalid the port->rts_gpio need to be < 0 too

Yes this is definitively an option. I cook a v2 right now.

>
> Best Regards,
> J.
>
>> +	if (gpio_is_valid(rts_pin)) {
>> +		ret = devm_gpio_request(&pdev->dev, rts_pin, "RTS");
>> 		if (ret) {
>> 			dev_err(&pdev->dev, "error requesting RTS GPIO\n");
>> 			goto err;
>> 		}
>> -		port->rts_gpio = pdata->rts_gpio;
>> +		port->rts_gpio = rts_pin;
>> 		ret = gpio_direction_output(port->rts_gpio, 0);
>> 		if (ret) {
>> 			dev_err(&pdev->dev, "error setting up RTS GPIO\n");
>> --
>> 1.8.2.2
>>
>
>
>


-- 
Nicolas Ferre

[PATCH v2] ARM/serial: at91: specify RTS in DT using gpio

[Nicolas Ferre]

Signed-off-by: Nicolas Ferre <nicolas.ferre@atmel.com>
---
 Documentation/devicetree/bindings/serial/atmel-usart.txt |  3 +++
 drivers/tty/serial/atmel_serial.c                        | 11 ++++++++---
 2 files changed, 11 insertions(+), 3 deletions(-)

diff --git a/Documentation/devicetree/bindings/serial/atmel-usart.txt b/Documentation/devicetree/bindings/serial/atmel-usart.txt
index 2191dcb..3adc61c 100644
--- a/Documentation/devicetree/bindings/serial/atmel-usart.txt
+++ b/Documentation/devicetree/bindings/serial/atmel-usart.txt
@@ -10,6 +10,8 @@ Required properties:
 Optional properties:
 - atmel,use-dma-rx: use of PDC or DMA for receiving data
 - atmel,use-dma-tx: use of PDC or DMA for transmitting data
+- rts-gpios: specify a GPIO for RTS line. It will use specified PIO instead of the peripheral
+  function pin for the USART RTS feature. If unsure, don't specify this property.
 - add dma bindings for dma transfer:
 	- dmas: DMA specifier, consisting of a phandle to DMA controller node,
 		memory peripheral interface and USART DMA channel ID, FIFO configuration.
@@ -28,6 +30,7 @@ Example:
 		interrupts = <7>;
 		atmel,use-dma-rx;
 		atmel,use-dma-tx;
+		rts-gpios = <&pioD 15 0>;
 	};
 
 - use DMA:
diff --git a/drivers/tty/serial/atmel_serial.c b/drivers/tty/serial/atmel_serial.c
index b4e0794..582166f 100644
--- a/drivers/tty/serial/atmel_serial.c
+++ b/drivers/tty/serial/atmel_serial.c
@@ -35,6 +35,7 @@
 #include <linux/platform_device.h>
 #include <linux/of.h>
 #include <linux/of_device.h>
+#include <linux/of_gpio.h>
 #include <linux/dma-mapping.h>
 #include <linux/atmel_pdc.h>
 #include <linux/atmel_serial.h>
@@ -2364,13 +2365,17 @@ static int atmel_serial_probe(struct platform_device *pdev)
 	 * structs to indicate "no RTS GPIO" instead of open-coding some
 	 * invalid value everywhere.
 	 */
-	if (pdata->rts_gpio > 0) {
-		ret = devm_gpio_request(&pdev->dev, pdata->rts_gpio, "RTS");
+	if (pdata && pdata->rts_gpio > 0)
+		port->rts_gpio = pdata->rts_gpio;
+	else if (np)
+		port->rts_gpio = of_get_named_gpio(np, "rts-gpios", 0);
+
+	if (gpio_is_valid(port->rts_gpio)) {
+		ret = devm_gpio_request(&pdev->dev, port->rts_gpio, "RTS");
 		if (ret) {
 			dev_err(&pdev->dev, "error requesting RTS GPIO\n");
 			goto err;
 		}
-		port->rts_gpio = pdata->rts_gpio;
 		ret = gpio_direction_output(port->rts_gpio, 0);
 		if (ret) {
 			dev_err(&pdev->dev, "error setting up RTS GPIO\n");
-- 
1.8.2.2

[PATCH v2] ARM/serial: at91: specify RTS in DT using gpio

[Nicolas Ferre]

Signed-off-by: Nicolas Ferre <nicolas.ferre@atmel.com>
---
 Documentation/devicetree/bindings/serial/atmel-usart.txt |  3 +++
 drivers/tty/serial/atmel_serial.c                        | 11 ++++++++---
 2 files changed, 11 insertions(+), 3 deletions(-)

diff --git a/Documentation/devicetree/bindings/serial/atmel-usart.txt b/Documentation/devicetree/bindings/serial/atmel-usart.txt
index 2191dcb..3adc61c 100644
--- a/Documentation/devicetree/bindings/serial/atmel-usart.txt
+++ b/Documentation/devicetree/bindings/serial/atmel-usart.txt
@@ -10,6 +10,8 @@ Required properties:
 Optional properties:
 - atmel,use-dma-rx: use of PDC or DMA for receiving data
 - atmel,use-dma-tx: use of PDC or DMA for transmitting data
+- rts-gpios: specify a GPIO for RTS line. It will use specified PIO instead of the peripheral
+  function pin for the USART RTS feature. If unsure, don't specify this property.
 - add dma bindings for dma transfer:
 	- dmas: DMA specifier, consisting of a phandle to DMA controller node,
 		memory peripheral interface and USART DMA channel ID, FIFO configuration.
@@ -28,6 +30,7 @@ Example:
 		interrupts = <7>;
 		atmel,use-dma-rx;
 		atmel,use-dma-tx;
+		rts-gpios = <&pioD 15 0>;
 	};
 
 - use DMA:
diff --git a/drivers/tty/serial/atmel_serial.c b/drivers/tty/serial/atmel_serial.c
index b4e0794..582166f 100644
--- a/drivers/tty/serial/atmel_serial.c
+++ b/drivers/tty/serial/atmel_serial.c
@@ -35,6 +35,7 @@
 #include <linux/platform_device.h>
 #include <linux/of.h>
 #include <linux/of_device.h>
+#include <linux/of_gpio.h>
 #include <linux/dma-mapping.h>
 #include <linux/atmel_pdc.h>
 #include <linux/atmel_serial.h>
@@ -2364,13 +2365,17 @@ static int atmel_serial_probe(struct platform_device *pdev)
 	 * structs to indicate "no RTS GPIO" instead of open-coding some
 	 * invalid value everywhere.
 	 */
-	if (pdata->rts_gpio > 0) {
-		ret = devm_gpio_request(&pdev->dev, pdata->rts_gpio, "RTS");
+	if (pdata && pdata->rts_gpio > 0)
+		port->rts_gpio = pdata->rts_gpio;
+	else if (np)
+		port->rts_gpio = of_get_named_gpio(np, "rts-gpios", 0);
+
+	if (gpio_is_valid(port->rts_gpio)) {
+		ret = devm_gpio_request(&pdev->dev, port->rts_gpio, "RTS");
 		if (ret) {
 			dev_err(&pdev->dev, "error requesting RTS GPIO\n");
 			goto err;
 		}
-		port->rts_gpio = pdata->rts_gpio;
 		ret = gpio_direction_output(port->rts_gpio, 0);
 		if (ret) {
 			dev_err(&pdev->dev, "error setting up RTS GPIO\n");
-- 
1.8.2.2

Re: [PATCH] ARM/serial: at91: switch atmel serial to use gpiolib

[Linus Walleij]

On Tue, Nov 5, 2013 at 5:28 PM, Jean-Christophe PLAGNIOL-VILLARD
<plagnioj@jcrosoft.com> wrote:
> On 13:35 Tue 05 Nov     , Linus Walleij wrote:

>> +     /*
>> +      * AT91RM9200 Errata #39: RTS0 is not internally connected
>> +      * to PA21. We need to drive the pin as a GPIO.
>> +      */
>> +     if (gpio_is_valid(atmel_port->rts_gpio) &&
>> +         port->mapbase == AT91RM9200_BASE_US0) {

> not really a fon of this hack
>
> if we use a uart for rs485 we need to use a gpio for rts too

Hm it was completely unnecessary anyway, as I now specify
the RTS pin per UART instance, it will only be valid for
those setting it right.

>> +     /*
>> +      * In theory the GPIO pin controlling RTS could be zero and
>> +      * this would be an improper check, but we know that the only
>> +      * existing case is != 0 and it's nice to use the zero-initialized
>> +      * structs to indicate "no RTS GPIO" instead of open-coding some
>> +      * invalid value everywhere.
>> +      */
>> +     if (pdata->rts_gpio > 0) {

>         this can not work 0 is a valid gpio if you do this you need to update
>         ALL the bard filee too to set it as -EINVAL;

Isn't it the other way around? I did this exactly to avoid updating
all the boardfiles, as the rts_gpio == 0 for all those not
defining it. This is why I test for > 0.

Yours,
Linus Walleij

[PATCH] ARM/serial: at91: switch atmel serial to use gpiolib

[Linus Walleij]

On Tue, Nov 5, 2013 at 5:28 PM, Jean-Christophe PLAGNIOL-VILLARD
<plagnioj@jcrosoft.com> wrote:
> On 13:35 Tue 05 Nov     , Linus Walleij wrote:

>> +     /*
>> +      * AT91RM9200 Errata #39: RTS0 is not internally connected
>> +      * to PA21. We need to drive the pin as a GPIO.
>> +      */
>> +     if (gpio_is_valid(atmel_port->rts_gpio) &&
>> +         port->mapbase == AT91RM9200_BASE_US0) {

> not really a fon of this hack
>
> if we use a uart for rs485 we need to use a gpio for rts too

Hm it was completely unnecessary anyway, as I now specify
the RTS pin per UART instance, it will only be valid for
those setting it right.

>> +     /*
>> +      * In theory the GPIO pin controlling RTS could be zero and
>> +      * this would be an improper check, but we know that the only
>> +      * existing case is != 0 and it's nice to use the zero-initialized
>> +      * structs to indicate "no RTS GPIO" instead of open-coding some
>> +      * invalid value everywhere.
>> +      */
>> +     if (pdata->rts_gpio > 0) {

>         this can not work 0 is a valid gpio if you do this you need to update
>         ALL the bard filee too to set it as -EINVAL;

Isn't it the other way around? I did this exactly to avoid updating
all the boardfiles, as the rts_gpio == 0 for all those not
defining it. This is why I test for > 0.

Yours,
Linus Walleij