[PATCH] atmel_serial: Atmel RS485 support

[Claudio Scordino]

Hi all,

this is a patch to add RS485 support to the atmel_serial driver.

The patch has been successfully tested by Sebastian Heutling (CC:-ed).

Feedback (comments, suggestions, further testing) is welcome.

Many thanks,

Claudio

[Ryan Mallon]

Please post patches inline in the message body as it makes them easier
to reply to. Some comments below:

> atmel_serial: RS485 support Signed-off-by: Claudio Scordino
> <cla...@evidence.eu.com> Signed-off-by: Michael Trimarchi
> <mic...@evidence.eu.com> Signed-off-by: Rick Bronson <ri...@efn.org>
> Signed-off-by: Sebastian Heutling <Sebastian...@who-ing.de> ---
> arch/arm/include/asm/ioctls.h | 2 +
> arch/arm/mach-at91/include/mach/board.h | 4 +
> drivers/serial/atmel_serial.c | 247 ++++++++++++++++++++++++++----- 3
> files changed, 218 insertions(+), 35 deletions(-) diff --git
> a/arch/arm/include/asm/ioctls.h b/arch/arm/include/asm/ioctls.h index
> a91d8a1..82f2177 100644 --- a/arch/arm/include/asm/ioctls.h +++
> b/arch/arm/include/asm/ioctls.h @@ -70,6 +70,8 @@ #define TIOCGICOUNT
> 0x545D /* read serial port inline interrupt counts */ #define FIOQSIZE
> 0x545E +#define TIOCSRS485 0x5461 + /* Used for packet mode */ #define
> TIOCPKT_DATA 0 #define TIOCPKT_FLUSHREAD 1 diff --git
> a/arch/arm/mach-at91/include/mach/board.h
> b/arch/arm/mach-at91/include/mach/board.h index ceaec6c..21f1308
> 100644 --- a/arch/arm/mach-at91/include/mach/board.h +++
> b/arch/arm/mach-at91/include/mach/board.h @@ -39,6 +39,7 @@ #include
> <linux/usb/atmel_usba_udc.h> #include <linux/atmel-mci.h> #include
> <sound/atmel-ac97c.h> +#include <linux/serial.h> /* USB Device */
> struct at91_udc_data { @@ -146,6 +147,9 @@ struct atmel_uart_data {
> short use_dma_tx; /* use transmit DMA? */ short use_dma_rx; /* use
> receive DMA? */ void __iomem *regs; /* virtual base address, if any */
> + struct serial_rs485 rs485; /* this flag specifies + if the uart must
> be used + as RS485 */
This comment is misleading, it is a struct not a flag. The comment should
say that it is the rs485 settings for the uart.

> }; extern void __init at91_add_device_serial(void); diff --git
> a/drivers/serial/atmel_serial.c b/drivers/serial/atmel_serial.c index
> 2c9bf9b..21bf3a8 100644 --- a/drivers/serial/atmel_serial.c +++
> b/drivers/serial/atmel_serial.c @@ -38,6 +38,7 @@ #include
> <linux/dma-mapping.h> #include <linux/atmel_pdc.h> #include
> <linux/atmel_serial.h> +#include <linux/uaccess.h> #include <asm/io.h>
> @@ -59,6 +60,9 @@ #include <linux/serial_core.h> +static void
> atmel_start_rx(struct uart_port *port); +static void
> atmel_stop_rx(struct uart_port *port);
Can you move these functions, so that these declarations are not needed?

> #ifdef CONFIG_SERIAL_ATMEL_TTYAT /* Use device name ttyAT, major 204
> and minor 154-169. This is necessary if we @@ -93,6 +97,7 @@ #define
> UART_GET_BRGR(port) __raw_readl((port)->membase + ATMEL_US_BRGR)
> #define UART_PUT_BRGR(port,v) __raw_writel(v, (port)->membase +
> ATMEL_US_BRGR) #define UART_PUT_RTOR(port,v) __raw_writel(v,
> (port)->membase + ATMEL_US_RTOR) +#define UART_PUT_TTGR(port, v)
> __raw_writel(v, (port)->membase + ATMEL_US_TTGR) /* PDC registers */
> #define UART_PUT_PTCR(port,v) __raw_writel(v, (port)->membase +
> ATMEL_PDC_PTCR) @@ -147,6 +152,16 @@ struct atmel_uart_port { unsigned
> int irq_status_prev; struct circ_buf rx_ring; + + struct serial_rs485
> rs485; /* this flag specifies + if the uart must be used + as RS485 */
Same here.

> + /* Fields related to interrupts: + if (rs485) = ATMEL_US_TXEMPTY +
> else if (use_dma_tx) = ATMEL_US_ENDTX | ATMEL_US_TXBUFE + else =
> ATMEL_US_TXRDY; + */
This comment doesn't really belong here. Also, please use *s as the beginning
of each line in a multi-line comment.

> + unsigned int tx_done_mask; }; static struct atmel_uart_port
> atmel_ports[ATMEL_MAX_UART]; @@ -187,6 +202,85 @@ static bool
> atmel_use_dma_tx(struct uart_port *port) } #endif +/* Enable or
> disable the rs485 support */ +void atmel_enable_disable_rs485(struct
> uart_port *port, struct serial_rs485 *rs485conf)
Please change the name of this function, something like atmel_config_rs485
would be better.

> +{ + struct atmel_uart_port *atmel_port = to_atmel_uart_port(port); +
> unsigned long flags; + unsigned int mode; + +
> spin_lock_irqsave(&port->lock, flags); + + mode = UART_GET_MR(port); +
> + /* Resetting serial mode to RS232 (0x0) */ + mode &=
> ~ATMEL_US_USMODE; + + atmel_port->rs485 = *rs485conf; + + if
> (!(rs485conf->flags & SER_RS485_ENABLED)) {
Reversing the logic of this if statement will make it slightly easier to read.

> + dev_dbg(port->dev, "Setting UART to RS232\n"); + if
> (atmel_use_dma_tx(port)) + atmel_port->tx_done_mask = ATMEL_US_ENDTX |
> ATMEL_US_TXBUFE; + else + atmel_port->tx_done_mask = ATMEL_US_TXRDY; +
> } else { + dev_dbg(port->dev, "Setting UART to RS485\n"); +
> atmel_port->tx_done_mask = ATMEL_US_TXEMPTY; + UART_PUT_TTGR(port,
> rs485conf->delay_rts_before_send); + mode |= ATMEL_US_USMODE_RS485; +
> } + UART_PUT_MR(port, mode); + + spin_unlock_irqrestore(&port->lock,
> flags); +} + + +static ssize_t show_rs485(struct device *dev, struct
> device_attribute *attr, \ + char *buf) +{ + struct platform_device
> *pdev = to_platform_device(dev); + struct uart_port *port =
> platform_get_drvdata(pdev); + unsigned int current_mode; + +
> current_mode = UART_GET_MR(port); + current_mode &= ATMEL_US_USMODE;
current_mode = UART_GET_MR(port) & ATMEL_US_USMODE;

> + return snprintf(buf, PAGE_SIZE, "%u\n", current_mode); +} + +static
> ssize_t set_rs485(struct device *dev, struct device_attribute *attr, \
> + const char *buf, size_t len) +{ + struct platform_device *pdev =
> to_platform_device(dev); + struct uart_port *port =
> platform_get_drvdata(pdev); + struct atmel_uart_port *atmel_port =
> to_atmel_uart_port(port); + struct serial_rs485 rs485conf =
> atmel_port->rs485; + unsigned int value; + + if (buf == NULL) + goto err;
if (!buf)
goto -EINVAL;

> + + value = simple_strtoul(buf, NULL, 10); + + if ((value != 0) &&
> (value != 1)) + goto err;
Is there any reason to be so strict on the values, you could just do this:

value = !!(simple_strtoul(buf, NULL, 0));

which will allow the user to use values in any base, and will convert any
non-zero values to 1.

> + if (!value) { + rs485conf.flags &= ~(SER_RS485_ENABLED);
Again, I would revert the logic of this if statement.

> + } else { + rs485conf.flags |= SER_RS485_ENABLED; + /* 0x4 is the
> normal reset value. */ + rs485conf.delay_rts_before_send = 0x00000004;
> + } + + atmel_enable_disable_rs485(port, &rs485conf); + +err: + return
> strnlen(buf, PAGE_SIZE); +} + +static DEVICE_ATTR(rs485, 0644,
> show_rs485, set_rs485); + /* * Return TIOCSER_TEMT when transmitter
> FIFO and Shift register is empty. */ @@ -202,6 +296,7 @@ static void
> atmel_set_mctrl(struct uart_port *port, u_int mctrl) { unsigned int
> control = 0; unsigned int mode; + struct atmel_uart_port *atmel_port =
> to_atmel_uart_port(port); #ifdef CONFIG_ARCH_AT91RM9200 if
> (cpu_is_at91rm9200()) { @@ -236,6 +331,16 @@ static void
> atmel_set_mctrl(struct uart_port *port, u_int mctrl) mode |=
> ATMEL_US_CHMODE_LOC_LOOP; else mode |= ATMEL_US_CHMODE_NORMAL; + + /*
> Resetting serial mode to RS232 (0x0) */ + mode &= ~ATMEL_US_USMODE; +
> + if (atmel_port->rs485.flags & SER_RS485_ENABLED) { +
> dev_dbg(port->dev, "Keeping UART to RS485\n"); + UART_PUT_TTGR(port,
> atmel_port->rs485.delay_rts_before_send); + mode |=
> ATMEL_US_USMODE_RS485; + } else + dev_dbg(port->dev, "Keeping UART to
> RS232\n"); UART_PUT_MR(port, mode); } @@ -268,12 +373,17 @@ static
> u_int atmel_get_mctrl(struct uart_port *port) */ static void
> atmel_stop_tx(struct uart_port *port) { + struct atmel_uart_port
> *atmel_port = to_atmel_uart_port(port); + if (atmel_use_dma_tx(port))
> { /* disable PDC transmit */ UART_PUT_PTCR(port, ATMEL_PDC_TXTDIS); -
> UART_PUT_IDR(port, ATMEL_US_ENDTX | ATMEL_US_TXBUFE); - } else -
> UART_PUT_IDR(port, ATMEL_US_TXRDY); + } + /* Disable interrupts */ +
> UART_PUT_IDR(port, atmel_port->tx_done_mask); + + if
> (atmel_port->rs485.flags & SER_RS485_ENABLED) + atmel_start_rx(port);
> } /* @@ -281,17 +391,22 @@ static void atmel_stop_tx(struct uart_port
> *port) */ static void atmel_start_tx(struct uart_port *port) { +
> struct atmel_uart_port *atmel_port = to_atmel_uart_port(port); + if
> (atmel_use_dma_tx(port)) { if (UART_GET_PTSR(port) & ATMEL_PDC_TXTEN)
> /* The transmitter is already running. Yes, we really need this.*/
> return; - UART_PUT_IER(port, ATMEL_US_ENDTX | ATMEL_US_TXBUFE); + if
> (atmel_port->rs485.flags & SER_RS485_ENABLED) + atmel_stop_rx(port); +
> /* re-enable PDC transmit */ UART_PUT_PTCR(port, ATMEL_PDC_TXTEN); - }
> else - UART_PUT_IER(port, ATMEL_US_TXRDY); + } + /* Enable interrupts
> */ + UART_PUT_IER(port, atmel_port->tx_done_mask); } /* @@ -302,12
> +417,28 @@ static void atmel_stop_rx(struct uart_port *port) if
> (atmel_use_dma_rx(port)) { /* disable PDC receive */
> UART_PUT_PTCR(port, ATMEL_PDC_RXTDIS); - UART_PUT_IDR(port,
> ATMEL_US_ENDRX | ATMEL_US_TIMEOUT); + UART_PUT_IDR(port,
> ATMEL_US_ENDRX | ATMEL_US_TIMEOUT | port->read_status_mask);
This line, and a couple of others extend over 80 characters, can you please
reformat, ie:

UART_PUT_IDR(port, ATMEL_US_ENDRX | ATMEL_US_TIMEOUT |
port->read_status_mask);

> } else UART_PUT_IDR(port, ATMEL_US_RXRDY);

Please use braces even on single line else statements in the case where the
if statement has braces.

> } /* + * start receiving - port is in process of being opened. + */
> +static void atmel_start_rx(struct uart_port *port) +{ +
> UART_PUT_CR(port, ATMEL_US_RSTSTA); /* reset status and receiver */ +
> + if (atmel_use_dma_rx(port)) { + /* enable PDC controller */ +
> UART_PUT_IER(port, ATMEL_US_ENDRX | ATMEL_US_TIMEOUT |
> port->read_status_mask); + UART_PUT_PTCR(port, ATMEL_PDC_RXTEN); + }
> else + UART_PUT_IER(port, ATMEL_US_RXRDY); +} + + +/* * Enable modem
> status interrupts */ static void atmel_enable_ms(struct uart_port
> *port) @@ -428,8 +559,9 @@ static void atmel_rx_chars(struct uart_port
> *port) static void atmel_tx_chars(struct uart_port *port) { struct
> circ_buf *xmit = &port->state->xmit; + struct atmel_uart_port
> *atmel_port = to_atmel_uart_port(port); - if (port->x_char &&
> UART_GET_CSR(port) & ATMEL_US_TXRDY) { + if (port->x_char &&
> UART_GET_CSR(port) & atmel_port->tx_done_mask) { UART_PUT_CHAR(port,
> port->x_char); port->icount.tx++; port->x_char = 0; @@ -437,7 +569,7
> @@ static void atmel_tx_chars(struct uart_port *port) if
> (uart_circ_empty(xmit) || uart_tx_stopped(port)) return; - while
> (UART_GET_CSR(port) & ATMEL_US_TXRDY) { + while (UART_GET_CSR(port) &
> atmel_port->tx_done_mask) { UART_PUT_CHAR(port,
> xmit->buf[xmit->tail]); xmit->tail = (xmit->tail + 1) &
> (UART_XMIT_SIZE - 1); port->icount.tx++; @@ -449,7 +581,8 @@ static
> void atmel_tx_chars(struct uart_port *port) uart_write_wakeup(port);
> if (!uart_circ_empty(xmit)) - UART_PUT_IER(port, ATMEL_US_TXRDY); + /*
> Enable interrupts */ + UART_PUT_IER(port, atmel_port->tx_done_mask); }
> /* @@ -501,18 +634,10 @@ atmel_handle_transmit(struct uart_port *port,
> unsigned int pending) { struct atmel_uart_port *atmel_port =
> to_atmel_uart_port(port); - if (atmel_use_dma_tx(port)) { - /* PDC
> transmit */ - if (pending & (ATMEL_US_ENDTX | ATMEL_US_TXBUFE)) { -
> UART_PUT_IDR(port, ATMEL_US_ENDTX | ATMEL_US_TXBUFE); -
> tasklet_schedule(&atmel_port->tasklet); - } - } else { - /* Interrupt
> transmit */ - if (pending & ATMEL_US_TXRDY) { - UART_PUT_IDR(port,
> ATMEL_US_TXRDY); - tasklet_schedule(&atmel_port->tasklet); - } + if
> (pending & atmel_port->tx_done_mask) { + /* Either PDC or interrupt
> transmission */ + UART_PUT_IDR(port, atmel_port->tx_done_mask); +
> tasklet_schedule(&atmel_port->tasklet); } } @@ -590,9 +715,15 @@
> static void atmel_tx_dma(struct uart_port *port) UART_PUT_TPR(port,
> pdc->dma_addr + xmit->tail); UART_PUT_TCR(port, count); - /* re-enable
> PDC transmit and interrupts */ + /* re-enable PDC transmit */
> UART_PUT_PTCR(port, ATMEL_PDC_TXTEN); - UART_PUT_IER(port,
> ATMEL_US_ENDTX | ATMEL_US_TXBUFE); + /* Enable interrupts */ +
> UART_PUT_IER(port, atmel_port->tx_done_mask); + } else { + if
> (atmel_port->rs485.flags & SER_RS485_ENABLED) { + /* DMA done, stop
> TX, start RX for RS485 */ + atmel_start_rx(port); + } } if
> (uart_circ_chars_pending(xmit) < WAKEUP_CHARS) @@ -1017,6 +1148,7 @@
> static void atmel_set_termios(struct uart_port *port, struct ktermios
> *termios, { unsigned long flags; unsigned int mode, imr, quot, baud; +
> struct atmel_uart_port *atmel_port = to_atmel_uart_port(port); /* Get
> current mode register */ mode = UART_GET_MR(port) & ~(ATMEL_US_USCLKS
> | ATMEL_US_CHRL @@ -1115,6 +1247,16 @@ static void
> atmel_set_termios(struct uart_port *port, struct ktermios *termios, /*
> disable receiver and transmitter */ UART_PUT_CR(port, ATMEL_US_TXDIS |
> ATMEL_US_RXDIS); + /* Resetting serial mode to RS232 (0x0) */ + mode
> &= ~ATMEL_US_USMODE; + + if (atmel_port->rs485.flags &
> SER_RS485_ENABLED) { + dev_dbg(port->dev, "Keeping UART to RS485\n");
> + UART_PUT_TTGR(port, atmel_port->rs485.delay_rts_before_send); + mode
> |= ATMEL_US_USMODE_RS485; + } else + dev_dbg(port->dev, "Keeping UART
> to RS232\n"); + /* set the parity, stop bits and data size */
> UART_PUT_MR(port, mode); @@ -1231,6 +1373,31 @@ static void
> atmel_poll_put_char(struct uart_port *port, unsigned char ch) } #endif
> +static int atmel_ioctl(struct uart_port *port, unsigned int cmd,
> unsigned long arg) +{ + + switch (cmd) { + case TIOCSRS485: + { +
> struct serial_rs485 rs485conf;
Moving this declaration to the top of the function will remove two
indentation levels.

Is it necessary to have both an ioctl and a sysfs control for putting the
uart into rs485 mode? The ioctl is more standardised, so perhaps drop the
sysfs control?

> + if (copy_from_user(&rs485conf, (struct serial_rs485 *) arg, +
> sizeof(rs485conf))) + return -EFAULT; + + dev_dbg(port->dev, "enable
> e/o disable rs485\n"); + + atmel_enable_disable_rs485(port,
> &rs485conf); + } + break; + + default: + return -ENOIOCTLCMD; + } +
> return 0; +} + + + static struct uart_ops atmel_pops = { .tx_empty =
> atmel_tx_empty, .set_mctrl = atmel_set_mctrl, @@ -1250,6 +1417,7 @@
> static struct uart_ops atmel_pops = { .config_port =
> atmel_config_port, .verify_port = atmel_verify_port, .pm =
> atmel_serial_pm, + .ioctl = atmel_ioctl, #ifdef CONFIG_CONSOLE_POLL
> .poll_get_char = atmel_poll_get_char, .poll_put_char =
> atmel_poll_put_char, @@ -1265,13 +1433,12 @@ static void __devinit
> atmel_init_port(struct atmel_uart_port *atmel_port, struct uart_port
> *port = &atmel_port->uart; struct atmel_uart_data *data =
> pdev->dev.platform_data; - port->iotype = UPIO_MEM; - port->flags =
> UPF_BOOT_AUTOCONF; - port->ops = &atmel_pops; - port->fifosize = 1; -
> port->line = pdev->id; - port->dev = &pdev->dev; - + port->iotype =
> UPIO_MEM; + port->flags = UPF_BOOT_AUTOCONF; + port->ops =
> &atmel_pops; + port->fifosize = 1; + port->line = pdev->id; +
> port->dev = &pdev->dev; port->mapbase = pdev->resource[0].start;
> port->irq = pdev->resource[1].start; @@ -1299,8 +1466,15 @@ static
> void __devinit atmel_init_port(struct atmel_uart_port *atmel_port,
> atmel_port->use_dma_rx = data->use_dma_rx; atmel_port->use_dma_tx =
> data->use_dma_tx; - if (atmel_use_dma_tx(port)) + atmel_port->rs485 =
> data->rs485; + /* Use TXEMPTY for interrupt when rs485 else TXRDY or
> ENDTX|TXBUFE */ + if (atmel_port->rs485.flags & SER_RS485_ENABLED) +
> atmel_port->tx_done_mask = ATMEL_US_TXEMPTY; + else if
> (atmel_use_dma_tx(port)) { port->fifosize = PDC_BUFFER_SIZE; +
> atmel_port->tx_done_mask = ATMEL_US_ENDTX | ATMEL_US_TXBUFE; + } else
> + atmel_port->tx_done_mask = ATMEL_US_TXRDY; } /* @@ -1334,6 +1508,7
> @@ static void atmel_console_putchar(struct uart_port *port, int ch)
> static void atmel_console_write(struct console *co, const char *s,
> u_int count) { struct uart_port *port = &atmel_ports[co->index].uart;
> + struct atmel_uart_port *atmel_port = to_atmel_uart_port(port);
> unsigned int status, imr; unsigned int pdc_tx; @@ -1341,7 +1516,7 @@
> static void atmel_console_write(struct console *co, const char *s,
> u_int count) * First, save IMR and then disable interrupts */ imr =
> UART_GET_IMR(port); - UART_PUT_IDR(port, ATMEL_US_RXRDY |
> ATMEL_US_TXRDY); + UART_PUT_IDR(port, ATMEL_US_RXRDY |
> atmel_port->tx_done_mask); /* Store PDC transmit status and disable it
> */ pdc_tx = UART_GET_PTSR(port) & ATMEL_PDC_TXTEN; @@ -1355,7 +1530,7
> @@ static void atmel_console_write(struct console *co, const char *s,
> u_int count) */ do { status = UART_GET_CSR(port); - } while (!(status
> & ATMEL_US_TXRDY)); + } while (!(status & atmel_port->tx_done_mask));
> /* Restore PDC transmit status */ if (pdc_tx) @@ -1587,7 +1762,7 @@
> static int __devinit atmel_serial_probe(struct platform_device *pdev)
> device_init_wakeup(&pdev->dev, 1); platform_set_drvdata(pdev, port); -
> return 0; + return device_create_file(&(pdev->dev), &dev_attr_rs485);
> err_add_port: kfree(port->rx_ring.buf); @@ -1619,6 +1794,8 @@ static
> int __devexit atmel_serial_remove(struct platform_device *pdev)
> clk_put(atmel_port->clk); + device_remove_file(&(pdev->dev),
> &dev_attr_rs485); + return ret; } -- 1.6.0.4
~Ryan

--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majo...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/

[Ryan Mallon]

Ugh, not sure what happened to my previous email. It looked fine in
Thunderbird before I sent it. Hopefully this works a bit better:

> + /* Fields related to interrupts:
> + if (rs485) = ATMEL_US_TXEMPTY
> + else if (use_dma_tx) = ATMEL_US_ENDTX | ATMEL_US_TXBUFE
> + else = ATMEL_US_TXRDY;
> + */

This comment doesn't really belong here. Also, please use *s at the
beginning of each line in a multiline comment.

> + unsigned int tx_done_mask;
> };
>
> static struct atmel_uart_port atmel_ports[ATMEL_MAX_UART];
> @@ -187,6 +202,85 @@ static bool atmel_use_dma_tx(struct uart_port *port)
> }
> #endif
>
> +/* Enable or disable the rs485 support */
> +void atmel_enable_disable_rs485(struct uart_port *port, struct
> serial_rs485 *rs485conf)

> +{
Please change the name of this function. Something like
atmel_config_rs485 would be better.

> + struct atmel_uart_port *atmel_port = to_atmel_uart_port(port);

return -EINVAL;

> +
> + value = simple_strtoul(buf, NULL, 10);
> +
> + if ((value != 0) && (value != 1))
> + goto err;

Is there any reason to be so strict on the values, you could just do this:

value = !!(simple_strtoul(buf, NULL, 0));

which will allow the user to use values in any base, and will convert any
non-zero values to 1.
> +

> + if (!value) {

Again, I would revert the logic of this if statement.

> + rs485conf.flags &= ~(SER_RS485_ENABLED);

This line, and a couple of others, extend over 80 characters. Could you
please split them up.

> } else
> UART_PUT_IDR(port, ATMEL_US_RXRDY);

Please use braces for single line else statements when the if clause has
braces.

If you move this declaration to the top of the function you will save
two levels of indentation.

[Claudio Scordino]

Hi Ryan,

first of all, thank you for your feedback.

To me, all comments seem to be reasonable, except maybe the following one:

>>
>> +static void atmel_start_rx(struct uart_port *port);
>> +static void atmel_stop_rx(struct uart_port *port);
>
> Can you move these functions, so that these declarations are not needed?

Actually, atmel_stop_rx is already defined, and I prefer to not move it
(since I change it, people may get confused by the diff...).
I also think that atmel_start_rx (which is added by the patch) should be
near to atmel_stop_rx...

>> + UART_PUT_IDR(port, ATMEL_US_ENDRX | ATMEL_US_TIMEOUT |
>> port->read_status_mask);
>
> This line, and a couple of others, extend over 80 characters. Could you
> please split them up.

Of course. I'm going to split them up and use a backslash at the end of
the original lines.

I will create a new patch according to your comments, test it and submit
again to the mailing list.

Many thanks again,

Claudio