spi: nuc900: Fix setting multiple bits settings in register
PATCH 1/1 spi: nuc900: Fix setting multiple bits settings in register
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.
--- drivers/spi/spi-nuc900.c | 18 ++++++++---------- 1 file changed, 8 insertions(+), 10 deletions(-)
drivers/spi/spi-nuc900.c +8 -10
@@ -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,19 +117,16 @@ 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;spin_lock_irqsave (&hw->lock, flags); - val =__raw_readl (hw->regs +USI_CNT ); + val =__raw_readl (hw->regs +USI_CNT ) & ~TXNUM ; -if (!txnum) - val &= ~TXNUM ; -else +if (txnum) val |= txnum <<0x08 ;__raw_writel (val, hw->regs +USI_CNT );
@@ -144,7 +143,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 =__raw_readl (hw->regs +USI_CNT ) & ~TXBITLEN ; val |= (txbitlen <<0x03 );
@@ -283,12 +282,11 @@ 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 ); + val =__raw_readl (hw->regs +USI_CNT ) & ~SLEEP ;if (sleep) val |= (sleep <<12 ); -else - val &= ~(0x0f <<12 ); +__raw_writel (val, hw->regs +USI_CNT );spin_unlock_irqrestore (&hw->lock, flags);
Signature · 8 lines
1.8.3.2 -- To unsubscribe from this list: send the line "unsubscribe linux-spi" in the body of a message to [email protected] More majordomo info at http://vger.kernel.org/majordomo-info.html
Hi Axel,
drivers/spi/spi-nuc900.c static void nuc900_spi_chipsel(struct spi_device *spi, int value) · line 130
Quoted 8 lines · Axel Lin
drivers/spi/spi-nuc900.c +2
-4- val =__raw_readl (hw->regs +USI_CNT ); + val =__raw_readl (hw->regs +USI_CNT ) & ~TXNUM ; -if (!txnum) - val &= ~TXNUM ; -else +if (txnum) val |= txnum <<0x08 ;
I think it's easier to read if you keep the masking and setting together, and remove the conditional, e.g.
val = __raw_readl(hw->regs + USI_CNT);
val = (val & ~TXNUM) | (txnum << 0x08);
__raw_writel(val, hw->regs + USI_CNT);Just my personal thoughts, too.
Gr{oetje,eeting}s,
Geert
--
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- [email protected]
In personal conversations with technical people, I call myself a hacker. But when I'm talking to journalists I just say "programmer" or something like that.
-- Linus Torvalds
--
To unsubscribe from this list: send the line "unsubscribe linux-spi" in the body of a message to [email protected] More majordomo info at http://vger.kernel.org/majordomo-info.html
A message that is not in the archive (sent off-list or not yet received): CAMuHMdVYHnv2PGaerL50PhK_a1E=LaEwdkhHNgyaqX=WO-FOXA-JsoAwUIsXosN+BqQ9rBEUg@public.gmane.org
Quoted 8 lines · Geert Uytterhoeven
I think it's easier to read if you keep the masking and setting together, and remove the conditional, e.g.
val = __raw_readl(hw->regs + USI_CNT); val = (val & ~TXNUM) | (txnum << 0x08); __raw_writel(val, hw->regs + USI_CNT);Just my personal thoughts, too.
I agree, that seems clearer.
Mark Brown
AppliedQuoted 2 lines · Axel Lin
The correct way to set multiple bits settings is always clear these bit fields before set new settings.
Applied, thanks.