[PATCH] ARM/serial: at91: switch atmel serial to use gpiolib
Nicolas Ferre
nicolas.ferre at atmel.com
Tue Nov 5 11:52:50 EST 2013
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 at 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
More information about the linux-arm-kernel
mailing list