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 <matt...@mattleach.net>
> ---
> CC: Greg Kroah-Hartman <gre...@linuxfoundation.org>
> CC: Jiri Slaby <jsl...@suse.com>
> CC: linux-ser...@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.kozlow...@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
>
