[PATCH linux-next 1/1] tty/serial: at91: fix I/O accesses on RHR and THR for AVR32

[PATCH linux-next 1/1] tty/serial: at91: fix I/O accesses on RHR and THR for AVR32

[Cyrille Pitchen]

This patch fixes I/O accesses on the Receiver Holding Register and on the
Transmitter Holding Register. Indeed AVR32 can only perform 32bit I/O
accesses on registers: using 8bit I/O accesses would read or write garbage
data.

Fixes: commit b5199d468177 ("tty/serial: at91: add support to FIFOs")
Signed-off-by: Cyrille Pitchen <cyrille.pitchen@atmel.com>
---
Hi all,

this patch fixes a bug on AVR32 introduced by the support of FIFOs for
AT91. Indeed when FIFOs are enabled they work in Multiple Data mode.
The Multiple Data mode allows to read up to 4 data from the Receiver
Holding Register (RHR) or write up to 4 data into the Transmitter Holding
Register (THR) in a single I/O access. Hence when only one data is to be
read from the RHR or written into the THR, the driver cannot use 32bit I/O
access anymore but must use 8bit I/O access instead.

For ARM-base AT91 SoCs, 8bit I/O accesses can always be used even when
FIFOs are not available. However AV32 can only use 32bit I/O accesses.

Since atmel_uart_readb(), resp. atmel_uart_writeb(), was created by the
FIFO support patch to access the RHR, resp. the THR, this patch replaces
it by atmel_uart_read_char(), resp. atmel_uart_write_char(). Hence the
actual width of the I/O access now depends only on the architecture.

All registers but RHR and THR use 32bit I/O accesses.

This patch was tested on a at91sam9260ek board.

Best Regards,

Cyrille

ChangeLog:

v1: initial version

 drivers/tty/serial/atmel_serial.c | 37 +++++++++++++++++++++++++++----------
 1 file changed, 27 insertions(+), 10 deletions(-)

diff --git a/drivers/tty/serial/atmel_serial.c b/drivers/tty/serial/atmel_serial.c
index e91b3b2f0590..5ca5cf3e9359 100644
--- a/drivers/tty/serial/atmel_serial.c
+++ b/drivers/tty/serial/atmel_serial.c
@@ -204,16 +204,33 @@ static inline void atmel_uart_writel(struct uart_port *port, u32 reg, u32 value)
 	__raw_writel(value, port->membase + reg);
 }
 
-static inline u8 atmel_uart_readb(struct uart_port *port, u32 reg)
+#ifdef CONFIG_AVR32
+
+/* AVR32 cannot handle 8 or 16bit I/O accesses but only 32bit I/O accesses */
+static inline u8 atmel_uart_read_char(struct uart_port *port)
+{
+	return __raw_readl(port->membase + ATMEL_US_RHR);
+}
+
+static inline void atmel_uart_write_char(struct uart_port *port, u8 value)
 {
-	return __raw_readb(port->membase + reg);
+	__raw_writel(value, port->membase + ATMEL_US_THR);
 }
 
-static inline void atmel_uart_writeb(struct uart_port *port, u32 reg, u8 value)
+#else
+
+static inline u8 atmel_uart_read_char(struct uart_port *port)
 {
-	__raw_writeb(value, port->membase + reg);
+	return __raw_readb(port->membase + ATMEL_US_RHR);
 }
 
+static inline void atmel_uart_write_char(struct uart_port *port, u8 value)
+{
+	__raw_writeb(value, port->membase + ATMEL_US_THR);
+}
+
+#endif
+
 #ifdef CONFIG_SERIAL_ATMEL_PDC
 static bool atmel_use_pdc_rx(struct uart_port *port)
 {
@@ -658,7 +675,7 @@ static void atmel_rx_chars(struct uart_port *port)
 
 	status = atmel_uart_readl(port, ATMEL_US_CSR);
 	while (status & ATMEL_US_RXRDY) {
-		ch = atmel_uart_readb(port, ATMEL_US_RHR);
+		ch = atmel_uart_read_char(port);
 
 		/*
 		 * note that the error handling code is
@@ -709,7 +726,7 @@ static void atmel_tx_chars(struct uart_port *port)
 
 	if (port->x_char &&
 	    (atmel_uart_readl(port, ATMEL_US_CSR) & atmel_port->tx_done_mask)) {
-		atmel_uart_writeb(port, ATMEL_US_THR, port->x_char);
+		atmel_uart_write_char(port, port->x_char);
 		port->icount.tx++;
 		port->x_char = 0;
 	}
@@ -718,7 +735,7 @@ static void atmel_tx_chars(struct uart_port *port)
 
 	while (atmel_uart_readl(port, ATMEL_US_CSR) &
 	       atmel_port->tx_done_mask) {
-		atmel_uart_writeb(port, ATMEL_US_THR, xmit->buf[xmit->tail]);
+		atmel_uart_write_char(port, xmit->buf[xmit->tail]);
 		xmit->tail = (xmit->tail + 1) & (UART_XMIT_SIZE - 1);
 		port->icount.tx++;
 		if (uart_circ_empty(xmit))
@@ -2294,7 +2311,7 @@ static int atmel_poll_get_char(struct uart_port *port)
 	while (!(atmel_uart_readl(port, ATMEL_US_CSR) & ATMEL_US_RXRDY))
 		cpu_relax();
 
-	return atmel_uart_readb(port, ATMEL_US_RHR);
+	return atmel_uart_read_char(port);
 }
 
 static void atmel_poll_put_char(struct uart_port *port, unsigned char ch)
@@ -2302,7 +2319,7 @@ static void atmel_poll_put_char(struct uart_port *port, unsigned char ch)
 	while (!(atmel_uart_readl(port, ATMEL_US_CSR) & ATMEL_US_TXRDY))
 		cpu_relax();
 
-	atmel_uart_writeb(port, ATMEL_US_THR, ch);
+	atmel_uart_write_char(port, ch);
 }
 #endif
 
@@ -2409,7 +2426,7 @@ static void atmel_console_putchar(struct uart_port *port, int ch)
 {
 	while (!(atmel_uart_readl(port, ATMEL_US_CSR) & ATMEL_US_TXRDY))
 		cpu_relax();
-	atmel_uart_writeb(port, ATMEL_US_THR, ch);
+	atmel_uart_write_char(port, ch);
 }
 
 /*
-- 
1.8.2.2

Re: [PATCH linux-next 1/1] tty/serial: at91: fix I/O accesses on RHR and THR for AVR32

[Andy Shevchenko]

On Thu, 2015-07-30 at 16:33 +0200, Cyrille Pitchen wrote:
> This patch fixes I/O accesses on the Receiver Holding Register and on 
> the
> Transmitter Holding Register. Indeed AVR32 can only perform 32bit I/O
> accesses on registers: using 8bit I/O accesses would read or write 
> garbage
> data.
> 

Thanks!

Tested-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com>

> Fixes: commit b5199d468177 ("tty/serial: at91: add support to FIFOs")
> Signed-off-by: Cyrille Pitchen <cyrille.pitchen@atmel.com>
> ---
> Hi all,
> 
> this patch fixes a bug on AVR32 introduced by the support of FIFOs 
> for
> AT91. Indeed when FIFOs are enabled they work in Multiple Data mode.
> The Multiple Data mode allows to read up to 4 data from the Receiver
> Holding Register (RHR) or write up to 4 data into the Transmitter 
> Holding
> Register (THR) in a single I/O access. Hence when only one data is to 
> be
> read from the RHR or written into the THR, the driver cannot use 
> 32bit I/O
> access anymore but must use 8bit I/O access instead.
> 
> For ARM-base AT91 SoCs, 8bit I/O accesses can always be used even 
> when
> FIFOs are not available. However AV32 can only use 32bit I/O 
> accesses.
> 
> Since atmel_uart_readb(), resp. atmel_uart_writeb(), was created by 
> the
> FIFO support patch to access the RHR, resp. the THR, this patch 
> replaces
> it by atmel_uart_read_char(), resp. atmel_uart_write_char(). Hence 
> the
> actual width of the I/O access now depends only on the architecture.
> 
> All registers but RHR and THR use 32bit I/O accesses.
> 
> This patch was tested on a at91sam9260ek board.
> 
> Best Regards,
> 
> Cyrille
> 
> ChangeLog:
> 
> v1: initial version
> 
>  drivers/tty/serial/atmel_serial.c | 37 +++++++++++++++++++++++++++--
> --------
>  1 file changed, 27 insertions(+), 10 deletions(-)
> 
> diff --git a/drivers/tty/serial/atmel_serial.c 
> b/drivers/tty/serial/atmel_serial.c
> index e91b3b2f0590..5ca5cf3e9359 100644
> --- a/drivers/tty/serial/atmel_serial.c
> +++ b/drivers/tty/serial/atmel_serial.c
> @@ -204,16 +204,33 @@ static inline void atmel_uart_writel(struct 
> uart_port *port, u32 reg, u32 value)
>  	__raw_writel(value, port->membase + reg);
>  }
>  
> -static inline u8 atmel_uart_readb(struct uart_port *port, u32 reg)
> +#ifdef CONFIG_AVR32
> +
> +/* AVR32 cannot handle 8 or 16bit I/O accesses but only 32bit I/O 
> accesses */
> +static inline u8 atmel_uart_read_char(struct uart_port *port)
> +{
> +	return __raw_readl(port->membase + ATMEL_US_RHR);
> +}
> +
> +static inline void atmel_uart_write_char(struct uart_port *port, u8 
> value)
>  {
> -	return __raw_readb(port->membase + reg);
> +	__raw_writel(value, port->membase + ATMEL_US_THR);
>  }
>  
> -static inline void atmel_uart_writeb(struct uart_port *port, u32 
> reg, u8 value)
> +#else
> +
> +static inline u8 atmel_uart_read_char(struct uart_port *port)
>  {
> -	__raw_writeb(value, port->membase + reg);
> +	return __raw_readb(port->membase + ATMEL_US_RHR);
>  }
>  
> +static inline void atmel_uart_write_char(struct uart_port *port, u8 
> value)
> +{
> +	__raw_writeb(value, port->membase + ATMEL_US_THR);
> +}
> +
> +#endif
> +
>  #ifdef CONFIG_SERIAL_ATMEL_PDC
>  static bool atmel_use_pdc_rx(struct uart_port *port)
>  {
> @@ -658,7 +675,7 @@ static void atmel_rx_chars(struct uart_port 
> *port)
>  
>  	status = atmel_uart_readl(port, ATMEL_US_CSR);
>  	while (status & ATMEL_US_RXRDY) {
> -		ch = atmel_uart_readb(port, ATMEL_US_RHR);
> +		ch = atmel_uart_read_char(port);
>  
>  		/*
>  		 * note that the error handling code is
> @@ -709,7 +726,7 @@ static void atmel_tx_chars(struct uart_port 
> *port)
>  
>  	if (port->x_char &&
>  	    (atmel_uart_readl(port, ATMEL_US_CSR) & atmel_port
> ->tx_done_mask)) {
> -		atmel_uart_writeb(port, ATMEL_US_THR, port->x_char);
> +		atmel_uart_write_char(port, port->x_char);
>  		port->icount.tx++;
>  		port->x_char = 0;
>  	}
> @@ -718,7 +735,7 @@ static void atmel_tx_chars(struct uart_port 
> *port)
>  
>  	while (atmel_uart_readl(port, ATMEL_US_CSR) &
>  	       atmel_port->tx_done_mask) {
> -		atmel_uart_writeb(port, ATMEL_US_THR, xmit->buf[xmit
> ->tail]);
> +		atmel_uart_write_char(port, xmit->buf[xmit->tail]);
>  		xmit->tail = (xmit->tail + 1) & (UART_XMIT_SIZE - 
> 1);
>  		port->icount.tx++;
>  		if (uart_circ_empty(xmit))
> @@ -2294,7 +2311,7 @@ static int atmel_poll_get_char(struct uart_port 
> *port)
>  	while (!(atmel_uart_readl(port, ATMEL_US_CSR) & 
> ATMEL_US_RXRDY))
>  		cpu_relax();
>  
> -	return atmel_uart_readb(port, ATMEL_US_RHR);
> +	return atmel_uart_read_char(port);
>  }
>  
>  static void atmel_poll_put_char(struct uart_port *port, unsigned 
> char ch)
> @@ -2302,7 +2319,7 @@ static void atmel_poll_put_char(struct 
> uart_port *port, unsigned char ch)
>  	while (!(atmel_uart_readl(port, ATMEL_US_CSR) & 
> ATMEL_US_TXRDY))
>  		cpu_relax();
>  
> -	atmel_uart_writeb(port, ATMEL_US_THR, ch);
> +	atmel_uart_write_char(port, ch);
>  }
>  #endif
>  
> @@ -2409,7 +2426,7 @@ static void atmel_console_putchar(struct 
> uart_port *port, int ch)
>  {
>  	while (!(atmel_uart_readl(port, ATMEL_US_CSR) & 
> ATMEL_US_TXRDY))
>  		cpu_relax();
> -	atmel_uart_writeb(port, ATMEL_US_THR, ch);
> +	atmel_uart_write_char(port, ch);
>  }
>  
>  /*

-- 
Andy Shevchenko <andriy.shevchenko@linux.intel.com>
Intel Finland Oy

Re: [PATCH linux-next 1/1] tty/serial: at91: fix I/O accesses on RHR and THR for AVR32

[Nicolas Ferre]

Le 30/07/2015 16:33, Cyrille Pitchen a écrit :
> This patch fixes I/O accesses on the Receiver Holding Register and on the
> Transmitter Holding Register. Indeed AVR32 can only perform 32bit I/O
> accesses on registers: using 8bit I/O accesses would read or write garbage
> data.
> 
> Fixes: commit b5199d468177 ("tty/serial: at91: add support to FIFOs")

In fact, this commit ID is in linux-next, so It might not apply to
Linus' tree...

> Signed-off-by: Cyrille Pitchen <cyrille.pitchen@atmel.com>

Now that Andy tested the patch:
Acked-by: Nicolas Ferre <nicolas.ferre@atmel.com>

Bye,


> ---
> Hi all,
> 
> this patch fixes a bug on AVR32 introduced by the support of FIFOs for
> AT91. Indeed when FIFOs are enabled they work in Multiple Data mode.
> The Multiple Data mode allows to read up to 4 data from the Receiver
> Holding Register (RHR) or write up to 4 data into the Transmitter Holding
> Register (THR) in a single I/O access. Hence when only one data is to be
> read from the RHR or written into the THR, the driver cannot use 32bit I/O
> access anymore but must use 8bit I/O access instead.
> 
> For ARM-base AT91 SoCs, 8bit I/O accesses can always be used even when
> FIFOs are not available. However AV32 can only use 32bit I/O accesses.
> 
> Since atmel_uart_readb(), resp. atmel_uart_writeb(), was created by the
> FIFO support patch to access the RHR, resp. the THR, this patch replaces
> it by atmel_uart_read_char(), resp. atmel_uart_write_char(). Hence the
> actual width of the I/O access now depends only on the architecture.
> 
> All registers but RHR and THR use 32bit I/O accesses.
> 
> This patch was tested on a at91sam9260ek board.
> 
> Best Regards,
> 
> Cyrille
> 
> ChangeLog:
> 
> v1: initial version
> 
>  drivers/tty/serial/atmel_serial.c | 37 +++++++++++++++++++++++++++----------
>  1 file changed, 27 insertions(+), 10 deletions(-)
> 
> diff --git a/drivers/tty/serial/atmel_serial.c b/drivers/tty/serial/atmel_serial.c
> index e91b3b2f0590..5ca5cf3e9359 100644
> --- a/drivers/tty/serial/atmel_serial.c
> +++ b/drivers/tty/serial/atmel_serial.c
> @@ -204,16 +204,33 @@ static inline void atmel_uart_writel(struct uart_port *port, u32 reg, u32 value)
>  	__raw_writel(value, port->membase + reg);
>  }
>  
> -static inline u8 atmel_uart_readb(struct uart_port *port, u32 reg)
> +#ifdef CONFIG_AVR32
> +
> +/* AVR32 cannot handle 8 or 16bit I/O accesses but only 32bit I/O accesses */
> +static inline u8 atmel_uart_read_char(struct uart_port *port)
> +{
> +	return __raw_readl(port->membase + ATMEL_US_RHR);
> +}
> +
> +static inline void atmel_uart_write_char(struct uart_port *port, u8 value)
>  {
> -	return __raw_readb(port->membase + reg);
> +	__raw_writel(value, port->membase + ATMEL_US_THR);
>  }
>  
> -static inline void atmel_uart_writeb(struct uart_port *port, u32 reg, u8 value)
> +#else
> +
> +static inline u8 atmel_uart_read_char(struct uart_port *port)
>  {
> -	__raw_writeb(value, port->membase + reg);
> +	return __raw_readb(port->membase + ATMEL_US_RHR);
>  }
>  
> +static inline void atmel_uart_write_char(struct uart_port *port, u8 value)
> +{
> +	__raw_writeb(value, port->membase + ATMEL_US_THR);
> +}
> +
> +#endif
> +
>  #ifdef CONFIG_SERIAL_ATMEL_PDC
>  static bool atmel_use_pdc_rx(struct uart_port *port)
>  {
> @@ -658,7 +675,7 @@ static void atmel_rx_chars(struct uart_port *port)
>  
>  	status = atmel_uart_readl(port, ATMEL_US_CSR);
>  	while (status & ATMEL_US_RXRDY) {
> -		ch = atmel_uart_readb(port, ATMEL_US_RHR);
> +		ch = atmel_uart_read_char(port);
>  
>  		/*
>  		 * note that the error handling code is
> @@ -709,7 +726,7 @@ static void atmel_tx_chars(struct uart_port *port)
>  
>  	if (port->x_char &&
>  	    (atmel_uart_readl(port, ATMEL_US_CSR) & atmel_port->tx_done_mask)) {
> -		atmel_uart_writeb(port, ATMEL_US_THR, port->x_char);
> +		atmel_uart_write_char(port, port->x_char);
>  		port->icount.tx++;
>  		port->x_char = 0;
>  	}
> @@ -718,7 +735,7 @@ static void atmel_tx_chars(struct uart_port *port)
>  
>  	while (atmel_uart_readl(port, ATMEL_US_CSR) &
>  	       atmel_port->tx_done_mask) {
> -		atmel_uart_writeb(port, ATMEL_US_THR, xmit->buf[xmit->tail]);
> +		atmel_uart_write_char(port, xmit->buf[xmit->tail]);
>  		xmit->tail = (xmit->tail + 1) & (UART_XMIT_SIZE - 1);
>  		port->icount.tx++;
>  		if (uart_circ_empty(xmit))
> @@ -2294,7 +2311,7 @@ static int atmel_poll_get_char(struct uart_port *port)
>  	while (!(atmel_uart_readl(port, ATMEL_US_CSR) & ATMEL_US_RXRDY))
>  		cpu_relax();
>  
> -	return atmel_uart_readb(port, ATMEL_US_RHR);
> +	return atmel_uart_read_char(port);
>  }
>  
>  static void atmel_poll_put_char(struct uart_port *port, unsigned char ch)
> @@ -2302,7 +2319,7 @@ static void atmel_poll_put_char(struct uart_port *port, unsigned char ch)
>  	while (!(atmel_uart_readl(port, ATMEL_US_CSR) & ATMEL_US_TXRDY))
>  		cpu_relax();
>  
> -	atmel_uart_writeb(port, ATMEL_US_THR, ch);
> +	atmel_uart_write_char(port, ch);
>  }
>  #endif
>  
> @@ -2409,7 +2426,7 @@ static void atmel_console_putchar(struct uart_port *port, int ch)
>  {
>  	while (!(atmel_uart_readl(port, ATMEL_US_CSR) & ATMEL_US_TXRDY))
>  		cpu_relax();
> -	atmel_uart_writeb(port, ATMEL_US_THR, ch);
> +	atmel_uart_write_char(port, ch);
>  }
>  
>  /*
> 


-- 
Nicolas Ferre

Re: [PATCH linux-next 1/1] tty/serial: at91: fix I/O accesses on RHR and THR for AVR32

[Alexandre Belloni]

On 30/07/2015 at 16:33:38 +0200, Cyrille Pitchen wrote :
> This patch fixes I/O accesses on the Receiver Holding Register and on the
> Transmitter Holding Register. Indeed AVR32 can only perform 32bit I/O
> accesses on registers: using 8bit I/O accesses would read or write garbage
> data.
> 
> Fixes: commit b5199d468177 ("tty/serial: at91: add support to FIFOs")
> Signed-off-by: Cyrille Pitchen <cyrille.pitchen@atmel.com>
Acked-by: Alexandre Belloni <alexandre.belloni@free-electrons.com>

Also, tested on at91rm9200ek.

> ---
> Hi all,
> 
> this patch fixes a bug on AVR32 introduced by the support of FIFOs for
> AT91. Indeed when FIFOs are enabled they work in Multiple Data mode.
> The Multiple Data mode allows to read up to 4 data from the Receiver
> Holding Register (RHR) or write up to 4 data into the Transmitter Holding
> Register (THR) in a single I/O access. Hence when only one data is to be
> read from the RHR or written into the THR, the driver cannot use 32bit I/O
> access anymore but must use 8bit I/O access instead.
> 
> For ARM-base AT91 SoCs, 8bit I/O accesses can always be used even when
> FIFOs are not available. However AV32 can only use 32bit I/O accesses.
> 
> Since atmel_uart_readb(), resp. atmel_uart_writeb(), was created by the
> FIFO support patch to access the RHR, resp. the THR, this patch replaces
> it by atmel_uart_read_char(), resp. atmel_uart_write_char(). Hence the
> actual width of the I/O access now depends only on the architecture.
> 
> All registers but RHR and THR use 32bit I/O accesses.
> 
> This patch was tested on a at91sam9260ek board.
> 
> Best Regards,
> 
> Cyrille
> 
> ChangeLog:
> 
> v1: initial version
> 
>  drivers/tty/serial/atmel_serial.c | 37 +++++++++++++++++++++++++++----------
>  1 file changed, 27 insertions(+), 10 deletions(-)
> 
> diff --git a/drivers/tty/serial/atmel_serial.c b/drivers/tty/serial/atmel_serial.c
> index e91b3b2f0590..5ca5cf3e9359 100644
> --- a/drivers/tty/serial/atmel_serial.c
> +++ b/drivers/tty/serial/atmel_serial.c
> @@ -204,16 +204,33 @@ static inline void atmel_uart_writel(struct uart_port *port, u32 reg, u32 value)
>  	__raw_writel(value, port->membase + reg);
>  }
>  
> -static inline u8 atmel_uart_readb(struct uart_port *port, u32 reg)
> +#ifdef CONFIG_AVR32
> +
> +/* AVR32 cannot handle 8 or 16bit I/O accesses but only 32bit I/O accesses */
> +static inline u8 atmel_uart_read_char(struct uart_port *port)
> +{
> +	return __raw_readl(port->membase + ATMEL_US_RHR);
> +}
> +
> +static inline void atmel_uart_write_char(struct uart_port *port, u8 value)
>  {
> -	return __raw_readb(port->membase + reg);
> +	__raw_writel(value, port->membase + ATMEL_US_THR);
>  }
>  
> -static inline void atmel_uart_writeb(struct uart_port *port, u32 reg, u8 value)
> +#else
> +
> +static inline u8 atmel_uart_read_char(struct uart_port *port)
>  {
> -	__raw_writeb(value, port->membase + reg);
> +	return __raw_readb(port->membase + ATMEL_US_RHR);
>  }
>  
> +static inline void atmel_uart_write_char(struct uart_port *port, u8 value)
> +{
> +	__raw_writeb(value, port->membase + ATMEL_US_THR);
> +}
> +
> +#endif
> +
>  #ifdef CONFIG_SERIAL_ATMEL_PDC
>  static bool atmel_use_pdc_rx(struct uart_port *port)
>  {
> @@ -658,7 +675,7 @@ static void atmel_rx_chars(struct uart_port *port)
>  
>  	status = atmel_uart_readl(port, ATMEL_US_CSR);
>  	while (status & ATMEL_US_RXRDY) {
> -		ch = atmel_uart_readb(port, ATMEL_US_RHR);
> +		ch = atmel_uart_read_char(port);
>  
>  		/*
>  		 * note that the error handling code is
> @@ -709,7 +726,7 @@ static void atmel_tx_chars(struct uart_port *port)
>  
>  	if (port->x_char &&
>  	    (atmel_uart_readl(port, ATMEL_US_CSR) & atmel_port->tx_done_mask)) {
> -		atmel_uart_writeb(port, ATMEL_US_THR, port->x_char);
> +		atmel_uart_write_char(port, port->x_char);
>  		port->icount.tx++;
>  		port->x_char = 0;
>  	}
> @@ -718,7 +735,7 @@ static void atmel_tx_chars(struct uart_port *port)
>  
>  	while (atmel_uart_readl(port, ATMEL_US_CSR) &
>  	       atmel_port->tx_done_mask) {
> -		atmel_uart_writeb(port, ATMEL_US_THR, xmit->buf[xmit->tail]);
> +		atmel_uart_write_char(port, xmit->buf[xmit->tail]);
>  		xmit->tail = (xmit->tail + 1) & (UART_XMIT_SIZE - 1);
>  		port->icount.tx++;
>  		if (uart_circ_empty(xmit))
> @@ -2294,7 +2311,7 @@ static int atmel_poll_get_char(struct uart_port *port)
>  	while (!(atmel_uart_readl(port, ATMEL_US_CSR) & ATMEL_US_RXRDY))
>  		cpu_relax();
>  
> -	return atmel_uart_readb(port, ATMEL_US_RHR);
> +	return atmel_uart_read_char(port);
>  }
>  
>  static void atmel_poll_put_char(struct uart_port *port, unsigned char ch)
> @@ -2302,7 +2319,7 @@ static void atmel_poll_put_char(struct uart_port *port, unsigned char ch)
>  	while (!(atmel_uart_readl(port, ATMEL_US_CSR) & ATMEL_US_TXRDY))
>  		cpu_relax();
>  
> -	atmel_uart_writeb(port, ATMEL_US_THR, ch);
> +	atmel_uart_write_char(port, ch);
>  }
>  #endif
>  
> @@ -2409,7 +2426,7 @@ static void atmel_console_putchar(struct uart_port *port, int ch)
>  {
>  	while (!(atmel_uart_readl(port, ATMEL_US_CSR) & ATMEL_US_TXRDY))
>  		cpu_relax();
> -	atmel_uart_writeb(port, ATMEL_US_THR, ch);
> +	atmel_uart_write_char(port, ch);
>  }
>  
>  /*
> -- 
> 1.8.2.2
> 

-- 
Alexandre Belloni, Free Electrons
Embedded Linux, Kernel and Android engineering
http://free-electrons.com

Re: [PATCH linux-next 1/1] tty/serial: at91: fix I/O accesses on RHR and THR for AVR32

[Nicolas Ferre]

Le 30/07/2015 19:46, Alexandre Belloni a écrit :
> On 30/07/2015 at 16:33:38 +0200, Cyrille Pitchen wrote :
>> This patch fixes I/O accesses on the Receiver Holding Register and on the
>> Transmitter Holding Register. Indeed AVR32 can only perform 32bit I/O
>> accesses on registers: using 8bit I/O accesses would read or write garbage
>> data.
>>
>> Fixes: commit b5199d468177 ("tty/serial: at91: add support to FIFOs")
>> Signed-off-by: Cyrille Pitchen <cyrille.pitchen@atmel.com>
> Acked-by: Alexandre Belloni <alexandre.belloni@free-electrons.com>
> 
> Also, tested on at91rm9200ek.

Great! thanks Alexandre.


>> ---
>> Hi all,
>>
>> this patch fixes a bug on AVR32 introduced by the support of FIFOs for
>> AT91. Indeed when FIFOs are enabled they work in Multiple Data mode.
>> The Multiple Data mode allows to read up to 4 data from the Receiver
>> Holding Register (RHR) or write up to 4 data into the Transmitter Holding
>> Register (THR) in a single I/O access. Hence when only one data is to be
>> read from the RHR or written into the THR, the driver cannot use 32bit I/O
>> access anymore but must use 8bit I/O access instead.
>>
>> For ARM-base AT91 SoCs, 8bit I/O accesses can always be used even when
>> FIFOs are not available. However AV32 can only use 32bit I/O accesses.
>>
>> Since atmel_uart_readb(), resp. atmel_uart_writeb(), was created by the
>> FIFO support patch to access the RHR, resp. the THR, this patch replaces
>> it by atmel_uart_read_char(), resp. atmel_uart_write_char(). Hence the
>> actual width of the I/O access now depends only on the architecture.
>>
>> All registers but RHR and THR use 32bit I/O accesses.
>>
>> This patch was tested on a at91sam9260ek board.
>>
>> Best Regards,
>>
>> Cyrille
>>
>> ChangeLog:
>>
>> v1: initial version
>>
>>  drivers/tty/serial/atmel_serial.c | 37 +++++++++++++++++++++++++++----------
>>  1 file changed, 27 insertions(+), 10 deletions(-)
>>
>> diff --git a/drivers/tty/serial/atmel_serial.c b/drivers/tty/serial/atmel_serial.c
>> index e91b3b2f0590..5ca5cf3e9359 100644
>> --- a/drivers/tty/serial/atmel_serial.c
>> +++ b/drivers/tty/serial/atmel_serial.c
>> @@ -204,16 +204,33 @@ static inline void atmel_uart_writel(struct uart_port *port, u32 reg, u32 value)
>>  	__raw_writel(value, port->membase + reg);
>>  }
>>  
>> -static inline u8 atmel_uart_readb(struct uart_port *port, u32 reg)
>> +#ifdef CONFIG_AVR32
>> +
>> +/* AVR32 cannot handle 8 or 16bit I/O accesses but only 32bit I/O accesses */
>> +static inline u8 atmel_uart_read_char(struct uart_port *port)
>> +{
>> +	return __raw_readl(port->membase + ATMEL_US_RHR);
>> +}
>> +
>> +static inline void atmel_uart_write_char(struct uart_port *port, u8 value)
>>  {
>> -	return __raw_readb(port->membase + reg);
>> +	__raw_writel(value, port->membase + ATMEL_US_THR);
>>  }
>>  
>> -static inline void atmel_uart_writeb(struct uart_port *port, u32 reg, u8 value)
>> +#else
>> +
>> +static inline u8 atmel_uart_read_char(struct uart_port *port)
>>  {
>> -	__raw_writeb(value, port->membase + reg);
>> +	return __raw_readb(port->membase + ATMEL_US_RHR);
>>  }
>>  
>> +static inline void atmel_uart_write_char(struct uart_port *port, u8 value)
>> +{
>> +	__raw_writeb(value, port->membase + ATMEL_US_THR);
>> +}
>> +
>> +#endif
>> +
>>  #ifdef CONFIG_SERIAL_ATMEL_PDC
>>  static bool atmel_use_pdc_rx(struct uart_port *port)
>>  {
>> @@ -658,7 +675,7 @@ static void atmel_rx_chars(struct uart_port *port)
>>  
>>  	status = atmel_uart_readl(port, ATMEL_US_CSR);
>>  	while (status & ATMEL_US_RXRDY) {
>> -		ch = atmel_uart_readb(port, ATMEL_US_RHR);
>> +		ch = atmel_uart_read_char(port);
>>  
>>  		/*
>>  		 * note that the error handling code is
>> @@ -709,7 +726,7 @@ static void atmel_tx_chars(struct uart_port *port)
>>  
>>  	if (port->x_char &&
>>  	    (atmel_uart_readl(port, ATMEL_US_CSR) & atmel_port->tx_done_mask)) {
>> -		atmel_uart_writeb(port, ATMEL_US_THR, port->x_char);
>> +		atmel_uart_write_char(port, port->x_char);
>>  		port->icount.tx++;
>>  		port->x_char = 0;
>>  	}
>> @@ -718,7 +735,7 @@ static void atmel_tx_chars(struct uart_port *port)
>>  
>>  	while (atmel_uart_readl(port, ATMEL_US_CSR) &
>>  	       atmel_port->tx_done_mask) {
>> -		atmel_uart_writeb(port, ATMEL_US_THR, xmit->buf[xmit->tail]);
>> +		atmel_uart_write_char(port, xmit->buf[xmit->tail]);
>>  		xmit->tail = (xmit->tail + 1) & (UART_XMIT_SIZE - 1);
>>  		port->icount.tx++;
>>  		if (uart_circ_empty(xmit))
>> @@ -2294,7 +2311,7 @@ static int atmel_poll_get_char(struct uart_port *port)
>>  	while (!(atmel_uart_readl(port, ATMEL_US_CSR) & ATMEL_US_RXRDY))
>>  		cpu_relax();
>>  
>> -	return atmel_uart_readb(port, ATMEL_US_RHR);
>> +	return atmel_uart_read_char(port);
>>  }
>>  
>>  static void atmel_poll_put_char(struct uart_port *port, unsigned char ch)
>> @@ -2302,7 +2319,7 @@ static void atmel_poll_put_char(struct uart_port *port, unsigned char ch)
>>  	while (!(atmel_uart_readl(port, ATMEL_US_CSR) & ATMEL_US_TXRDY))
>>  		cpu_relax();
>>  
>> -	atmel_uart_writeb(port, ATMEL_US_THR, ch);
>> +	atmel_uart_write_char(port, ch);
>>  }
>>  #endif
>>  
>> @@ -2409,7 +2426,7 @@ static void atmel_console_putchar(struct uart_port *port, int ch)
>>  {
>>  	while (!(atmel_uart_readl(port, ATMEL_US_CSR) & ATMEL_US_TXRDY))
>>  		cpu_relax();
>> -	atmel_uart_writeb(port, ATMEL_US_THR, ch);
>> +	atmel_uart_write_char(port, ch);
>>  }
>>  
>>  /*
>> -- 
>> 1.8.2.2
>>
> 


-- 
Nicolas Ferre