From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1753342AbcFJMRt (ORCPT ); Fri, 10 Jun 2016 08:17:49 -0400 Received: from mailout2.w1.samsung.com ([210.118.77.12]:50477 "EHLO mailout2.w1.samsung.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1751225AbcFJMRr (ORCPT ); Fri, 10 Jun 2016 08:17:47 -0400 X-AuditID: cbfec7f4-f796c6d000001486-5e-575aafe61be0 Subject: Re: [RFC PATCH 06/15] tty: serial: samsung: add byte-order aware bit functions To: Matthew Leach , Ben Dooks References: <20160608183110.13851-1-matthew@mattleach.net> <20160608183110.13851-7-matthew@mattleach.net> Cc: linux-samsung-soc@vger.kernel.org, linux-arm-kernel@lists.infradead.org, linux-kernel@vger.kernel.org, Greg Kroah-Hartman , Jiri Slaby , linux-serial@vger.kernel.org From: Krzysztof Kozlowski X-Enigmail-Draft-Status: N1110 Message-id: <575AAFE5.8020207@samsung.com> Date: Fri, 10 Jun 2016 14:17:41 +0200 User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:38.0) Gecko/20100101 Thunderbird/38.8.0 MIME-version: 1.0 In-reply-to: <20160608183110.13851-7-matthew@mattleach.net> Content-type: text/plain; charset=windows-1252 Content-transfer-encoding: 7bit X-Brightmail-Tracker: H4sIAAAAAAAAA+NgFvrOLMWRmVeSWpSXmKPExsVy+t/xK7rP1keFGzzfam7x4FYrk0Xz4vVs FlM2fGCyeP3C0GLT42usFpd3zWGzmHF+H5PFmcW97BaNS5exOHB6PHs5mclj/9w17B6bl9R7 TP2wjsWjb8sqRo/1W66yeHzeJBfAHsVlk5Kak1mWWqRvl8CVsXTeJdaCNUoVn59YNzD2ynQx cnJICJhIdB1YzAZhi0lcuLceyObiEBJYyijx72ATI4TzjFHi0YJOli5GDg5hgUiJUzsiQRpE BAIkNhy5zQpiCwlkS5xpnccOUs8scItRYufTXUwgCTYBY4nNy5dAbZCT6O2exAJi8wpoSfx9 soMdxGYRUJV4P+MfWL2oQITErO0/mCBqBCV+TL4HVs8pYC2x/eJGVpAbmAX0JO5f1AIJMwvI S2xe85Z5AqPgLCQdsxCqZiGpWsDIvIpRNLU0uaA4KT3XUK84Mbe4NC9dLzk/dxMjJDq+7GBc fMzqEKMAB6MSD2/ErshwIdbEsuLK3EOMEhzMSiK8qauiwoV4UxIrq1KL8uOLSnNSiw8xSnOw KInzzt31PkRIID2xJDU7NbUgtQgmy8TBKdXAaM6qJcjb91/OLFavJO7Fu4+7i5WSv9arrtvG 85lhcVTMMgOZhPIdav3m87Yv/+5zctH+atGM+/EFKVsOdwcumeXrZmDnM8vQUGEx84yYCTr6 SiLzcmNXz9DeE1/RG3PG58PmjLI3iW7bq3du8Fx7Ls7AKdpi2QeGOT/FjHdZx6VfNe5L5+NV YinOSDTUYi4qTgQAh49QqYoCAAA= Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org 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 > --- > CC: Greg Kroah-Hartman > CC: Jiri Slaby > 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 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 >