Patchwork [U-Boot,v1,03/25] spi: kirkwood_spi.c: Some fixes and cleanup

login
register
mail settings
Submitter Stefan Roese
Date June 27, 2014, 9:54 a.m.
Message ID <1403862911-6138-4-git-send-email-sr@denx.de>
Download mbox | patch
Permalink /patch/364835/
State Accepted
Delegated to: Jagannadha Sutradharudu Teki
Headers show

Comments

Stefan Roese - June 27, 2014, 9:54 a.m.
This patch introduces the clrsetbits_le32() accessor functions in the
kirkwood SPI driver. Note that it also includes a fix:

-	 writel(~KWSPI_CSN_ACT | KWSPI_SMEMRDY, &spireg->ctrl);
+	 writel(KWSPI_SMEMRDY, &spireg->ctrl);

Here the bit KWSPI_CSN_ACT (0x1) should have been cleared. Instead
0xfffffffe is written into this control register. This is the main
reason to use the clrsetbits() functions now. As they make clearing
bits much less error prone.

Additionally KWSPI_IRQUNMASK is not used in spi_cs_activate() and
spi_cs_deactivate() any more. Its the wrong macro but has the same
value as the correct one (KWSPI_CSN_ACT).

This is in preparation for use of this driver on the Marvell Armada XP
platform as well.

Signed-off-by: Stefan Roese <sr@denx.de>
Cc: Jagannadha Sutradharudu Teki <jaganna@xilinx.com>
---

 drivers/spi/kirkwood_spi.c | 11 +++++------
 1 file changed, 5 insertions(+), 6 deletions(-)
Jagannadha Sutradharudu Teki - July 3, 2014, 8:16 p.m.
Reviewed-by: Jagannadha Sutradharudu Teki <jaganna@xilinx.com>

On Fri, Jun 27, 2014 at 3:24 PM, Stefan Roese <sr@denx.de> wrote:
> This patch introduces the clrsetbits_le32() accessor functions in the
> kirkwood SPI driver. Note that it also includes a fix:
>
> -        writel(~KWSPI_CSN_ACT | KWSPI_SMEMRDY, &spireg->ctrl);
> +        writel(KWSPI_SMEMRDY, &spireg->ctrl);
>
> Here the bit KWSPI_CSN_ACT (0x1) should have been cleared. Instead
> 0xfffffffe is written into this control register. This is the main
> reason to use the clrsetbits() functions now. As they make clearing
> bits much less error prone.
>
> Additionally KWSPI_IRQUNMASK is not used in spi_cs_activate() and
> spi_cs_deactivate() any more. Its the wrong macro but has the same
> value as the correct one (KWSPI_CSN_ACT).
>
> This is in preparation for use of this driver on the Marvell Armada XP
> platform as well.
>
> Signed-off-by: Stefan Roese <sr@denx.de>
> Cc: Jagannadha Sutradharudu Teki <jaganna@xilinx.com>
> ---
>
>  drivers/spi/kirkwood_spi.c | 11 +++++------
>  1 file changed, 5 insertions(+), 6 deletions(-)
>
> diff --git a/drivers/spi/kirkwood_spi.c b/drivers/spi/kirkwood_spi.c
> index 942a208..449e9f8 100644
> --- a/drivers/spi/kirkwood_spi.c
> +++ b/drivers/spi/kirkwood_spi.c
> @@ -37,7 +37,7 @@ struct spi_slave *spi_setup_slave(unsigned int bus, unsigned int cs,
>         if (!slave)
>                 return NULL;
>
> -       writel(~KWSPI_CSN_ACT | KWSPI_SMEMRDY, &spireg->ctrl);
> +       writel(KWSPI_SMEMRDY, &spireg->ctrl);
>
>         /* calculate spi clock prescaller using max_hz */
>         data = ((CONFIG_SYS_TCLK / 2) / max_hz) + 0x10;
> @@ -137,12 +137,12 @@ void spi_init(void)
>
>  void spi_cs_activate(struct spi_slave *slave)
>  {
> -       writel(readl(&spireg->ctrl) | KWSPI_IRQUNMASK, &spireg->ctrl);
> +       setbits_le32(&spireg->ctrl, KWSPI_CSN_ACT);
>  }
>
>  void spi_cs_deactivate(struct spi_slave *slave)
>  {
> -       writel(readl(&spireg->ctrl) & KWSPI_IRQMASK, &spireg->ctrl);
> +       clrbits_le32(&spireg->ctrl, KWSPI_CSN_ACT);
>  }
>
>  int spi_xfer(struct spi_slave *slave, unsigned int bitlen, const void *dout,
> @@ -161,8 +161,7 @@ int spi_xfer(struct spi_slave *slave, unsigned int bitlen, const void *dout,
>          * handle data in 8-bit chunks
>          * TBD: 2byte xfer mode to be enabled
>          */
> -       writel(((readl(&spireg->cfg) & ~KWSPI_XFERLEN_MASK) |
> -               KWSPI_XFERLEN_1BYTE), &spireg->cfg);
> +       clrsetbits_le32(&spireg->cfg, KWSPI_XFERLEN_MASK, KWSPI_XFERLEN_1BYTE);
>
>         while (bitlen > 4) {
>                 debug("loopstart bitlen %d\n", bitlen);
> @@ -172,7 +171,7 @@ int spi_xfer(struct spi_slave *slave, unsigned int bitlen, const void *dout,
>                 if (dout)
>                         tmpdout = *(u32 *) dout & 0x0ff;
>
> -               writel(~KWSPI_SMEMRDIRQ, &spireg->irq_cause);
> +               clrbits_le32(&spireg->irq_cause, KWSPI_SMEMRDIRQ);
>                 writel(tmpdout, &spireg->dout); /* Write the data out */
>                 debug("*** spi_xfer: ... %08x written, bitlen %d\n",
>                       tmpdout, bitlen);
> --
> 2.0.1
>
> _______________________________________________
> U-Boot mailing list
> U-Boot@lists.denx.de
> http://lists.denx.de/mailman/listinfo/u-boot

Patch

diff --git a/drivers/spi/kirkwood_spi.c b/drivers/spi/kirkwood_spi.c
index 942a208..449e9f8 100644
--- a/drivers/spi/kirkwood_spi.c
+++ b/drivers/spi/kirkwood_spi.c
@@ -37,7 +37,7 @@  struct spi_slave *spi_setup_slave(unsigned int bus, unsigned int cs,
 	if (!slave)
 		return NULL;
 
-	writel(~KWSPI_CSN_ACT | KWSPI_SMEMRDY, &spireg->ctrl);
+	writel(KWSPI_SMEMRDY, &spireg->ctrl);
 
 	/* calculate spi clock prescaller using max_hz */
 	data = ((CONFIG_SYS_TCLK / 2) / max_hz) + 0x10;
@@ -137,12 +137,12 @@  void spi_init(void)
 
 void spi_cs_activate(struct spi_slave *slave)
 {
-	writel(readl(&spireg->ctrl) | KWSPI_IRQUNMASK, &spireg->ctrl);
+	setbits_le32(&spireg->ctrl, KWSPI_CSN_ACT);
 }
 
 void spi_cs_deactivate(struct spi_slave *slave)
 {
-	writel(readl(&spireg->ctrl) & KWSPI_IRQMASK, &spireg->ctrl);
+	clrbits_le32(&spireg->ctrl, KWSPI_CSN_ACT);
 }
 
 int spi_xfer(struct spi_slave *slave, unsigned int bitlen, const void *dout,
@@ -161,8 +161,7 @@  int spi_xfer(struct spi_slave *slave, unsigned int bitlen, const void *dout,
 	 * handle data in 8-bit chunks
 	 * TBD: 2byte xfer mode to be enabled
 	 */
-	writel(((readl(&spireg->cfg) & ~KWSPI_XFERLEN_MASK) |
-		KWSPI_XFERLEN_1BYTE), &spireg->cfg);
+	clrsetbits_le32(&spireg->cfg, KWSPI_XFERLEN_MASK, KWSPI_XFERLEN_1BYTE);
 
 	while (bitlen > 4) {
 		debug("loopstart bitlen %d\n", bitlen);
@@ -172,7 +171,7 @@  int spi_xfer(struct spi_slave *slave, unsigned int bitlen, const void *dout,
 		if (dout)
 			tmpdout = *(u32 *) dout & 0x0ff;
 
-		writel(~KWSPI_SMEMRDIRQ, &spireg->irq_cause);
+		clrbits_le32(&spireg->irq_cause, KWSPI_SMEMRDIRQ);
 		writel(tmpdout, &spireg->dout);	/* Write the data out */
 		debug("*** spi_xfer: ... %08x written, bitlen %d\n",
 		      tmpdout, bitlen);