diff mbox

[v2] spi: nuc900: Fix setting multiple bits settings in register

Message ID 1395412823.9974.2.camel@phoenix (mailing list archive)
State New, archived
Headers show

Commit Message

Axel Lin March 21, 2014, 2:40 p.m. UTC 
The correct way to set multiple bits settings is always clear these
bit fields before set new settings.

Current code does not cause problem because the reset value of these
bit fields are 0, and these settings only set once during probe.

Signed-off-by: Axel Lin <axel.lin@ingics.com>
---
 drivers/spi/spi-nuc900.c | 22 ++++++----------------
 1 file changed, 6 insertions(+), 16 deletions(-)

Comments

Mark Brown March 26, 2014, 4:48 p.m. UTC | #1 
On Fri, Mar 21, 2014 at 10:40:23PM +0800, Axel Lin wrote:
> The correct way to set multiple bits settings is always clear these
> bit fields before set new settings.
> 
> Current code does not cause problem because the reset value of these
> bit fields are 0, and these settings only set once during probe.

This doesn't seem to apply against current code, can you please check
and resend?
Axel Lin March 27, 2014, 12:41 a.m. UTC | #2 
2014-03-27 0:48 GMT+08:00 Mark Brown <broonie@kernel.org>:
> On Fri, Mar 21, 2014 at 10:40:23PM +0800, Axel Lin wrote:
>> The correct way to set multiple bits settings is always clear these
>> bit fields before set new settings.
>>
>> Current code does not cause problem because the reset value of these
>> bit fields are 0, and these settings only set once during probe.
>
> This doesn't seem to apply against current code, can you please check
> and resend?
It's because v1 is applied.
I personally do not have strong preference between v1 and v2.
Is it ok to keep it as is since it's applied.
--
To unsubscribe from this list: send the line "unsubscribe linux-spi" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
diff mbox

Patch

diff --git a/drivers/spi/spi-nuc900.c b/drivers/spi/spi-nuc900.c
index 675c210..a062a89 100644
--- a/drivers/spi/spi-nuc900.c
+++ b/drivers/spi/spi-nuc900.c
@@ -37,7 +37,9 @@ 
 /* usi register bit */
 #define ENINT		(0x01 << 17)
 #define ENFLG		(0x01 << 16)
+#define SLEEP		(0x0f << 12)
 #define TXNUM		(0x03 << 8)
+#define TXBITLEN	(0x1f << 3)
 #define TXNEG		(0x01 << 2)
 #define RXNEG		(0x01 << 1)
 #define LSB		(0x01 << 10)
@@ -115,8 +117,7 @@  static void nuc900_spi_chipsel(struct spi_device *spi, int value)
 	}
 }
 
-static void nuc900_spi_setup_txnum(struct nuc900_spi *hw,
-							unsigned int txnum)
+static void nuc900_spi_setup_txnum(struct nuc900_spi *hw, unsigned int txnum)
 {
 	unsigned int val;
 	unsigned long flags;
@@ -124,12 +125,7 @@  static void nuc900_spi_setup_txnum(struct nuc900_spi *hw,
 	spin_lock_irqsave(&hw->lock, flags);
 
 	val = __raw_readl(hw->regs + USI_CNT);
-
-	if (!txnum)
-		val &= ~TXNUM;
-	else
-		val |= txnum << 0x08;
-
+	val = (val & ~TXNUM) | (txnum << 0x08);
 	__raw_writel(val, hw->regs + USI_CNT);
 
 	spin_unlock_irqrestore(&hw->lock, flags);
@@ -145,9 +141,7 @@  static void nuc900_spi_setup_txbitlen(struct nuc900_spi *hw,
 	spin_lock_irqsave(&hw->lock, flags);
 
 	val = __raw_readl(hw->regs + USI_CNT);
-
-	val |= (txbitlen << 0x03);
-
+	val = (val & ~TXBITLEN) | (txbitlen << 0x03);
 	__raw_writel(val, hw->regs + USI_CNT);
 
 	spin_unlock_irqrestore(&hw->lock, flags);
@@ -284,11 +278,7 @@  static void nuc900_set_sleep(struct nuc900_spi *hw, unsigned int sleep)
 	spin_lock_irqsave(&hw->lock, flags);
 
 	val = __raw_readl(hw->regs + USI_CNT);
-
-	if (sleep)
-		val |= (sleep << 12);
-	else
-		val &= ~(0x0f << 12);
+	val = (val & ~SLEEP) | (sleep << 12);
 	__raw_writel(val, hw->regs + USI_CNT);
 
 	spin_unlock_irqrestore(&hw->lock, flags);