[RFC PATCH 05/15] tty: serial: samsung: fixup accessors for endian

[RFC PATCH 05/15] tty: serial: samsung: fixup accessors for endian

[Matthew Leach]

Fix the serial access code to deal with kernels built for big endian
operation.

Signed-off-by: Matthew Leach <matthew@mattleach.net>
---
CC: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
CC: Jiri Slaby <jslaby@suse.com>
CC: linux-serial@vger.kernel.org
CC: linux-kernel@vger.kernel.org
---
 drivers/tty/serial/samsung.h | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/drivers/tty/serial/samsung.h b/drivers/tty/serial/samsung.h
index fc5deaa..8818bdd 100644
--- a/drivers/tty/serial/samsung.h
+++ b/drivers/tty/serial/samsung.h
@@ -118,9 +118,9 @@ struct s3c24xx_uart_port {
 	((unsigned long *)(unsigned long)((port)->membase + (reg)))
 
 #define rd_regb(port, reg) (__raw_readb(portaddr(port, reg)))
-#define rd_regl(port, reg) (__raw_readl(portaddr(port, reg)))
+#define rd_regl(port, reg) (readl_relaxed(portaddr(port, reg)))
 
 #define wr_regb(port, reg, val) __raw_writeb(val, portaddr(port, reg))
-#define wr_regl(port, reg, val) __raw_writel(val, portaddr(port, reg))
+#define wr_regl(port, reg, val) writel_relaxed(val, portaddr(port, reg))
 
 #endif
-- 
2.8.3

[RFC PATCH 06/15] tty: serial: samsung: add byte-order aware bit functions

[Matthew Leach]

This driver makes use of the __set_bit() and __clear_bit() functions.
When running under big-endian, these functions don't convert the bit
indexes when working with peripheral registers, leading to the
incorrect bits being set and cleared when running big-endian.

Add two new driver functions for setting and clearing bits that are
byte-order aware.

Signed-off-by: Matthew Leach <matthew@mattleach.net>
---
CC: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
CC: Jiri Slaby <jslaby@suse.com>
CC: linux-serial@vger.kernel.org
CC: linux-kernel@vger.kernel.org
---
 drivers/tty/serial/samsung.c | 16 +++++++---------
 drivers/tty/serial/samsung.h | 29 +++++++++++++++++++++++++++++
 2 files changed, 36 insertions(+), 9 deletions(-)

diff --git a/drivers/tty/serial/samsung.c b/drivers/tty/serial/samsung.c
index 99bb231..e4f53d5 100644
--- a/drivers/tty/serial/samsung.c
+++ b/drivers/tty/serial/samsung.c
@@ -169,8 +169,7 @@ static void s3c24xx_serial_stop_tx(struct uart_port *port)
 		return;
 
 	if (s3c24xx_serial_has_interrupt_mask(port))
-		__set_bit(S3C64XX_UINTM_TXD,
-			portaddrl(port, S3C64XX_UINTM));
+		s3c24xx_set_bit(port, S3C64XX_UINTM_TXD, S3C64XX_UINTM);
 	else
 		disable_irq_nosync(ourport->tx_irq);
 
@@ -235,8 +234,7 @@ static void enable_tx_dma(struct s3c24xx_uart_port *ourport)
 
 	/* Mask Tx interrupt */
 	if (s3c24xx_serial_has_interrupt_mask(port))
-		__set_bit(S3C64XX_UINTM_TXD,
-			  portaddrl(port, S3C64XX_UINTM));
+		s3c24xx_set_bit(port, S3C64XX_UINTM_TXD, S3C64XX_UINTM);
 	else
 		disable_irq_nosync(ourport->tx_irq);
 
@@ -269,8 +267,8 @@ static void enable_tx_pio(struct s3c24xx_uart_port *ourport)
 
 	/* Unmask Tx interrupt */
 	if (s3c24xx_serial_has_interrupt_mask(port))
-		__clear_bit(S3C64XX_UINTM_TXD,
-			    portaddrl(port, S3C64XX_UINTM));
+		s3c24xx_clear_bit(port, S3C64XX_UINTM_TXD,
+				  S3C64XX_UINTM);
 	else
 		enable_irq(ourport->tx_irq);
 
@@ -397,8 +395,8 @@ static void s3c24xx_serial_stop_rx(struct uart_port *port)
 	if (rx_enabled(port)) {
 		dbg("s3c24xx_serial_stop_rx: port=%p\n", port);
 		if (s3c24xx_serial_has_interrupt_mask(port))
-			__set_bit(S3C64XX_UINTM_RXD,
-				portaddrl(port, S3C64XX_UINTM));
+			s3c24xx_set_bit(port, S3C64XX_UINTM_RXD,
+					S3C64XX_UINTM);
 		else
 			disable_irq_nosync(ourport->rx_irq);
 		rx_enabled(port) = 0;
@@ -1069,7 +1067,7 @@ static int s3c64xx_serial_startup(struct uart_port *port)
 	spin_unlock_irqrestore(&port->lock, flags);
 
 	/* Enable Rx Interrupt */
-	__clear_bit(S3C64XX_UINTM_RXD, portaddrl(port, S3C64XX_UINTM));
+	s3c24xx_clear_bit(port, S3C64XX_UINTM_RXD, S3C64XX_UINTM);
 
 	dbg("s3c64xx_serial_startup ok\n");
 	return ret;
diff --git a/drivers/tty/serial/samsung.h b/drivers/tty/serial/samsung.h
index 8818bdd..e45745a 100644
--- a/drivers/tty/serial/samsung.h
+++ b/drivers/tty/serial/samsung.h
@@ -111,6 +111,7 @@ struct s3c24xx_uart_port {
 
 #define s3c24xx_dev_to_port(__dev) dev_get_drvdata(__dev)
 
+
 /* register access controls */
 
 #define portaddr(port, reg) ((port)->membase + (reg))
@@ -123,4 +124,32 @@ struct s3c24xx_uart_port {
 #define wr_regb(port, reg, val) __raw_writeb(val, portaddr(port, reg))
 #define wr_regl(port, reg, val) writel_relaxed(val, portaddr(port, reg))
 
+/* Byte-order aware bit setting/clearing functions. */
+
+static inline void s3c24xx_set_bit(struct uart_port *port, int idx,
+				   unsigned int reg)
+{
+	unsigned long flags;
+	u32 val;
+
+	local_irq_save(flags);
+	val = rd_regl(port, reg);
+	val |= (1 << idx);
+	wr_regl(port, reg, val);
+	local_irq_restore(flags);
+}
+
+static inline void s3c24xx_clear_bit(struct uart_port *port, int idx,
+				     unsigned int reg)
+{
+	unsigned long flags;
+	u32 val;
+
+	local_irq_save(flags);
+	val = rd_regl(port, reg);
+	val &= ~(1 << idx);
+	wr_regl(port, reg, val);
+	local_irq_restore(flags);
+}
+
 #endif
-- 
2.8.3

Re: [RFC PATCH 05/15] tty: serial: samsung: fixup accessors for endian

[Ben Dooks]

On 08/06/16 19:31, Matthew Leach wrote:
> Fix the serial access code to deal with kernels built for big endian
> operation.
> 
> Signed-off-by: Matthew Leach <matthew@mattleach.net>
> ---
> CC: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
> CC: Jiri Slaby <jslaby@suse.com>
> CC: linux-serial@vger.kernel.org
> CC: linux-kernel@vger.kernel.org
> ---
>  drivers/tty/serial/samsung.h | 4 ++--
>  1 file changed, 2 insertions(+), 2 deletions(-)
> 
> diff --git a/drivers/tty/serial/samsung.h b/drivers/tty/serial/samsung.h
> index fc5deaa..8818bdd 100644
> --- a/drivers/tty/serial/samsung.h
> +++ b/drivers/tty/serial/samsung.h
> @@ -118,9 +118,9 @@ struct s3c24xx_uart_port {
>  	((unsigned long *)(unsigned long)((port)->membase + (reg)))
>  
>  #define rd_regb(port, reg) (__raw_readb(portaddr(port, reg)))
> -#define rd_regl(port, reg) (__raw_readl(portaddr(port, reg)))
> +#define rd_regl(port, reg) (readl_relaxed(portaddr(port, reg)))
>  
>  #define wr_regb(port, reg, val) __raw_writeb(val, portaddr(port, reg))
> -#define wr_regl(port, reg, val) __raw_writel(val, portaddr(port, reg))
> +#define wr_regl(port, reg, val) writel_relaxed(val, portaddr(port, reg))

I suppose for complete-ness we should have changed the readbs as well.

-- 
Ben Dooks				http://www.codethink.co.uk/
Senior Engineer				Codethink - Providing Genius

Re: [RFC PATCH 05/15] tty: serial: samsung: fixup accessors for endian

[Ben Dooks]

On 08/06/16 19:31, Matthew Leach wrote:
> Fix the serial access code to deal with kernels built for big endian
> operation.
> 
> Signed-off-by: Matthew Leach <matthew@mattleach.net>
> ---
> CC: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
> CC: Jiri Slaby <jslaby@suse.com>
> CC: linux-serial@vger.kernel.org
> CC: linux-kernel@vger.kernel.org
> ---
>  drivers/tty/serial/samsung.h | 4 ++--
>  1 file changed, 2 insertions(+), 2 deletions(-)
> 
> diff --git a/drivers/tty/serial/samsung.h b/drivers/tty/serial/samsung.h
> index fc5deaa..8818bdd 100644
> --- a/drivers/tty/serial/samsung.h
> +++ b/drivers/tty/serial/samsung.h
> @@ -118,9 +118,9 @@ struct s3c24xx_uart_port {
>  	((unsigned long *)(unsigned long)((port)->membase + (reg)))
>  
>  #define rd_regb(port, reg) (__raw_readb(portaddr(port, reg)))
> -#define rd_regl(port, reg) (__raw_readl(portaddr(port, reg)))
> +#define rd_regl(port, reg) (readl_relaxed(portaddr(port, reg)))

For completeness we should have changed __raw_readb to readb_relaxed

>  #define wr_regb(port, reg, val) __raw_writeb(val, portaddr(port, reg))
> -#define wr_regl(port, reg, val) __raw_writel(val, portaddr(port, reg))
> +#define wr_regl(port, reg, val) writel_relaxed(val, portaddr(port, reg))

Acked-by: Ben Dooks <ben.dooks@codethink.co.uk>

-- 
Ben Dooks				http://www.codethink.co.uk/
Senior Engineer				Codethink - Providing Genius

Re: [RFC PATCH 05/15] tty: serial: samsung: fixup accessors for endian

[Krzysztof Kozlowski]

On 06/10/2016 12:13 PM, Ben Dooks wrote:
> On 08/06/16 19:31, Matthew Leach wrote:
>> Fix the serial access code to deal with kernels built for big endian
>> operation.
>>
>> Signed-off-by: Matthew Leach <matthew@mattleach.net>
>> ---
>> CC: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
>> CC: Jiri Slaby <jslaby@suse.com>
>> CC: linux-serial@vger.kernel.org
>> CC: linux-kernel@vger.kernel.org
>> ---
>>  drivers/tty/serial/samsung.h | 4 ++--
>>  1 file changed, 2 insertions(+), 2 deletions(-)
>>
>> diff --git a/drivers/tty/serial/samsung.h b/drivers/tty/serial/samsung.h
>> index fc5deaa..8818bdd 100644
>> --- a/drivers/tty/serial/samsung.h
>> +++ b/drivers/tty/serial/samsung.h
>> @@ -118,9 +118,9 @@ struct s3c24xx_uart_port {
>>  	((unsigned long *)(unsigned long)((port)->membase + (reg)))
>>  
>>  #define rd_regb(port, reg) (__raw_readb(portaddr(port, reg)))
>> -#define rd_regl(port, reg) (__raw_readl(portaddr(port, reg)))
>> +#define rd_regl(port, reg) (readl_relaxed(portaddr(port, reg)))
> 
> For completeness we should have changed __raw_readb to readb_relaxed
> 
>>  #define wr_regb(port, reg, val) __raw_writeb(val, portaddr(port, reg))
>> -#define wr_regl(port, reg, val) __raw_writel(val, portaddr(port, reg))
>> +#define wr_regl(port, reg, val) writel_relaxed(val, portaddr(port, reg))
> 
> Acked-by: Ben Dooks <ben.dooks@codethink.co.uk>


With the readb_relaced change:
Reviewed-by: Krzysztof Kozlowski <k.kozlowski@samsung.com>

Best regards,
Krzysztof

Re: [RFC PATCH 06/15] tty: serial: samsung: add byte-order aware bit functions

[Krzysztof Kozlowski]

On 06/08/2016 08:31 PM, Matthew Leach wrote:
> This driver makes use of the __set_bit() and __clear_bit() functions.
> When running under big-endian, these functions don't convert the bit
> indexes when working with peripheral registers, leading to the
> incorrect bits being set and cleared when running big-endian.
> 
> Add two new driver functions for setting and clearing bits that are
> byte-order aware.
> 
> Signed-off-by: Matthew Leach <matthew@mattleach.net>
> ---
> CC: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
> CC: Jiri Slaby <jslaby@suse.com>
> CC: linux-serial@vger.kernel.org
> CC: linux-kernel@vger.kernel.org
> ---
>  drivers/tty/serial/samsung.c | 16 +++++++---------
>  drivers/tty/serial/samsung.h | 29 +++++++++++++++++++++++++++++
>  2 files changed, 36 insertions(+), 9 deletions(-)
> 
> diff --git a/drivers/tty/serial/samsung.c b/drivers/tty/serial/samsung.c
> index 99bb231..e4f53d5 100644
> --- a/drivers/tty/serial/samsung.c
> +++ b/drivers/tty/serial/samsung.c
> @@ -169,8 +169,7 @@ static void s3c24xx_serial_stop_tx(struct uart_port *port)
>  		return;
>  
>  	if (s3c24xx_serial_has_interrupt_mask(port))
> -		__set_bit(S3C64XX_UINTM_TXD,
> -			portaddrl(port, S3C64XX_UINTM));
> +		s3c24xx_set_bit(port, S3C64XX_UINTM_TXD, S3C64XX_UINTM);
>  	else
>  		disable_irq_nosync(ourport->tx_irq);
>  
> @@ -235,8 +234,7 @@ static void enable_tx_dma(struct s3c24xx_uart_port *ourport)
>  
>  	/* Mask Tx interrupt */
>  	if (s3c24xx_serial_has_interrupt_mask(port))
> -		__set_bit(S3C64XX_UINTM_TXD,
> -			  portaddrl(port, S3C64XX_UINTM));
> +		s3c24xx_set_bit(port, S3C64XX_UINTM_TXD, S3C64XX_UINTM);
>  	else
>  		disable_irq_nosync(ourport->tx_irq);
>  
> @@ -269,8 +267,8 @@ static void enable_tx_pio(struct s3c24xx_uart_port *ourport)
>  
>  	/* Unmask Tx interrupt */
>  	if (s3c24xx_serial_has_interrupt_mask(port))
> -		__clear_bit(S3C64XX_UINTM_TXD,
> -			    portaddrl(port, S3C64XX_UINTM));
> +		s3c24xx_clear_bit(port, S3C64XX_UINTM_TXD,
> +				  S3C64XX_UINTM);
>  	else
>  		enable_irq(ourport->tx_irq);
>  
> @@ -397,8 +395,8 @@ static void s3c24xx_serial_stop_rx(struct uart_port *port)
>  	if (rx_enabled(port)) {
>  		dbg("s3c24xx_serial_stop_rx: port=%p\n", port);
>  		if (s3c24xx_serial_has_interrupt_mask(port))
> -			__set_bit(S3C64XX_UINTM_RXD,
> -				portaddrl(port, S3C64XX_UINTM));
> +			s3c24xx_set_bit(port, S3C64XX_UINTM_RXD,
> +					S3C64XX_UINTM);
>  		else
>  			disable_irq_nosync(ourport->rx_irq);
>  		rx_enabled(port) = 0;
> @@ -1069,7 +1067,7 @@ static int s3c64xx_serial_startup(struct uart_port *port)
>  	spin_unlock_irqrestore(&port->lock, flags);
>  
>  	/* Enable Rx Interrupt */
> -	__clear_bit(S3C64XX_UINTM_RXD, portaddrl(port, S3C64XX_UINTM));
> +	s3c24xx_clear_bit(port, S3C64XX_UINTM_RXD, S3C64XX_UINTM);
>  
>  	dbg("s3c64xx_serial_startup ok\n");
>  	return ret;
> diff --git a/drivers/tty/serial/samsung.h b/drivers/tty/serial/samsung.h
> index 8818bdd..e45745a 100644
> --- a/drivers/tty/serial/samsung.h
> +++ b/drivers/tty/serial/samsung.h
> @@ -111,6 +111,7 @@ struct s3c24xx_uart_port {
>  
>  #define s3c24xx_dev_to_port(__dev) dev_get_drvdata(__dev)
>  
> +

This new line looks unnecessary. Beside that it looks okay:

Reviewed-by: Krzysztof Kozlowski <k.kozlowski@samsung.com>

Best regards,
Krzysztof

>  /* register access controls */
>  
>  #define portaddr(port, reg) ((port)->membase + (reg))
> @@ -123,4 +124,32 @@ struct s3c24xx_uart_port {
>  #define wr_regb(port, reg, val) __raw_writeb(val, portaddr(port, reg))
>  #define wr_regl(port, reg, val) writel_relaxed(val, portaddr(port, reg))
>  
> +/* Byte-order aware bit setting/clearing functions. */
> +
> +static inline void s3c24xx_set_bit(struct uart_port *port, int idx,
> +				   unsigned int reg)
> +{
> +	unsigned long flags;
> +	u32 val;
> +
> +	local_irq_save(flags);
> +	val = rd_regl(port, reg);
> +	val |= (1 << idx);
> +	wr_regl(port, reg, val);
> +	local_irq_restore(flags);
> +}
> +
> +static inline void s3c24xx_clear_bit(struct uart_port *port, int idx,
> +				     unsigned int reg)
> +{
> +	unsigned long flags;
> +	u32 val;
> +
> +	local_irq_save(flags);
> +	val = rd_regl(port, reg);
> +	val &= ~(1 << idx);
> +	wr_regl(port, reg, val);
> +	local_irq_restore(flags);
> +}
> +
>  #endif
>