From mboxrd@z Thu Jan 1 00:00:00 1970 From: Nicolas Ferre Subject: Re: [PATCH v2] ARM/serial: at91: switch atmel serial to use gpiolib Date: Wed, 6 Nov 2013 11:06:03 +0100 Message-ID: <527A148B.8090309@atmel.com> References: <1383729939-10269-1-git-send-email-linus.walleij@linaro.org> Mime-Version: 1.0 Content-Type: text/plain; charset="ISO-8859-1"; format=flowed Content-Transfer-Encoding: 7bit Return-path: In-Reply-To: <1383729939-10269-1-git-send-email-linus.walleij@linaro.org> Sender: linux-serial-owner@vger.kernel.org To: Linus Walleij , linux-gpio@vger.kernel.org, Andrew Victor , Jean-Christophe Plagniol-Villard , linux-serial@vger.kernel.org, Greg Kroah-Hartman Cc: Alexandre Courbot , linux-arm-kernel@lists.infradead.org List-Id: linux-gpio@vger.kernel.org On 06/11/2013 10:25, 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. > > The patch also adds device tree support for getting the > RTS GPIO pin from the device tree on DT boot paths. > > Signed-off-by: Nicolas Ferre > Signed-off-by: Linus Walleij > --- > ChangeLog v1->v2: > - Skip check for UART base address from leftover hack, > if we have an RTS GPIO we just use it. > - Set default error value on GPIO pin to -EINVAL > - Fold in a device tree support patch from Nicolas Ferre Thanks for having done this Linus! Greg, this patch is ready now and it supersedes the one you acknowledged earlier ([PATCH 1/4] ARM/serial: at91: move machine quirk into machine). Best regards, > --- > .../devicetree/bindings/serial/atmel-usart.txt | 3 ++ > arch/arm/mach-at91/at91rm9200_devices.c | 1 + > drivers/tty/serial/atmel_serial.c | 55 +++++++++++++++------- > include/linux/platform_data/atmel.h | 1 + > 4 files changed, 42 insertions(+), 18 deletions(-) > > diff --git a/Documentation/devicetree/bindings/serial/atmel-usart.txt b/Documentation/devicetree/bindings/serial/atmel-usart.txt > index 2191dcb9f1da..3adc61c2e4ca 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/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..e31f3319be64 100644 > --- a/drivers/tty/serial/atmel_serial.c > +++ b/drivers/tty/serial/atmel_serial.c > @@ -35,21 +35,18 @@ > #include > #include > #include > +#include > #include > #include > #include > #include > #include > #include > +#include > > #include > #include > > -#ifdef CONFIG_ARM > -#include > -#include > -#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 +164,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 +298,16 @@ 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)) { > + 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,31 @@ static int atmel_serial_probe(struct platform_device *pdev) > port = &atmel_ports[ret]; > port->backup_imr = 0; > port->uart.line = ret; > + port->rts_gpio = -EINVAL; /* 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 && 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; > + } > + 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 From mboxrd@z Thu Jan 1 00:00:00 1970 From: nicolas.ferre@atmel.com (Nicolas Ferre) Date: Wed, 6 Nov 2013 11:06:03 +0100 Subject: [PATCH v2] ARM/serial: at91: switch atmel serial to use gpiolib In-Reply-To: <1383729939-10269-1-git-send-email-linus.walleij@linaro.org> References: <1383729939-10269-1-git-send-email-linus.walleij@linaro.org> Message-ID: <527A148B.8090309@atmel.com> To: linux-arm-kernel@lists.infradead.org List-Id: linux-arm-kernel.lists.infradead.org On 06/11/2013 10:25, 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. > > The patch also adds device tree support for getting the > RTS GPIO pin from the device tree on DT boot paths. > > Signed-off-by: Nicolas Ferre > Signed-off-by: Linus Walleij > --- > ChangeLog v1->v2: > - Skip check for UART base address from leftover hack, > if we have an RTS GPIO we just use it. > - Set default error value on GPIO pin to -EINVAL > - Fold in a device tree support patch from Nicolas Ferre Thanks for having done this Linus! Greg, this patch is ready now and it supersedes the one you acknowledged earlier ([PATCH 1/4] ARM/serial: at91: move machine quirk into machine). Best regards, > --- > .../devicetree/bindings/serial/atmel-usart.txt | 3 ++ > arch/arm/mach-at91/at91rm9200_devices.c | 1 + > drivers/tty/serial/atmel_serial.c | 55 +++++++++++++++------- > include/linux/platform_data/atmel.h | 1 + > 4 files changed, 42 insertions(+), 18 deletions(-) > > diff --git a/Documentation/devicetree/bindings/serial/atmel-usart.txt b/Documentation/devicetree/bindings/serial/atmel-usart.txt > index 2191dcb9f1da..3adc61c2e4ca 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/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..e31f3319be64 100644 > --- a/drivers/tty/serial/atmel_serial.c > +++ b/drivers/tty/serial/atmel_serial.c > @@ -35,21 +35,18 @@ > #include > #include > #include > +#include > #include > #include > #include > #include > #include > #include > +#include > > #include > #include > > -#ifdef CONFIG_ARM > -#include > -#include > -#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 +164,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 +298,16 @@ 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)) { > + 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,31 @@ static int atmel_serial_probe(struct platform_device *pdev) > port = &atmel_ports[ret]; > port->backup_imr = 0; > port->uart.line = ret; > + port->rts_gpio = -EINVAL; /* 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 && 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; > + } > + 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