diff mbox

[U-Boot] spi: oc_tiny_spi: Refactor to simplify spi_xfer implementation

Message ID 1389341288.25856.2.camel@phoenix
State Superseded
Delegated to: Jagannadha Sutradharudu Teki
Headers show

Commit Message

Axel Lin Jan. 10, 2014, 8:08 a.m. UTC 
Currently we have similar code for (txp && rxp), (txp && !rxp), (!rxp & txp),
and (!txp && !rxp) cases. This patch refactors the code a bit to avoid
duplicate similar code.

Signed-off-by: Axel Lin <axel.lin@ingics.com>
---
Hi Thomas,
This path is similar to the patch I sent for spi-oc-tiny.c linux driver.
I'd appreciate if you can review and test this patch.
Regards,
Axel
 drivers/spi/oc_tiny_spi.c | 85 ++++++-----------------------------------------
 1 file changed, 11 insertions(+), 74 deletions(-)

Comments

Jagan Teki March 2, 2014, 6:43 p.m. UTC | #1 
Hi Axel,

Any testing on this patch?

On Fri, Jan 10, 2014 at 1:38 PM, Axel Lin <axel.lin@ingics.com> wrote:
> Currently we have similar code for (txp && rxp), (txp && !rxp), (!rxp & txp),
> and (!txp && !rxp) cases. This patch refactors the code a bit to avoid
> duplicate similar code.
>
> Signed-off-by: Axel Lin <axel.lin@ingics.com>
> ---
> Hi Thomas,
> This path is similar to the patch I sent for spi-oc-tiny.c linux driver.
> I'd appreciate if you can review and test this patch.
> Regards,
> Axel
>  drivers/spi/oc_tiny_spi.c | 85 ++++++-----------------------------------------
>  1 file changed, 11 insertions(+), 74 deletions(-)
>
> diff --git a/drivers/spi/oc_tiny_spi.c b/drivers/spi/oc_tiny_spi.c
> index 4de5d00..a8c1dfd 100644
> --- a/drivers/spi/oc_tiny_spi.c
> +++ b/drivers/spi/oc_tiny_spi.c
> @@ -158,85 +158,22 @@ int spi_xfer(struct spi_slave *slave, unsigned int bitlen, const void *dout,
>                 spi_cs_activate(slave);
>
>         /* we need to tighten the transfer loop */
> -       if (txp && rxp) {
> -               writeb(*txp++, &regs->txdata);
> -               if (bytes > 1) {
> -                       writeb(*txp++, &regs->txdata);
> -                       for (i = 2; i < bytes; i++) {
> -                               u8 rx, tx = *txp++;
> -                               while (!(readb(&regs->status) &
> -                                        TINY_SPI_STATUS_TXR))
> +       writeb(txp ? *txp++ : CONFIG_TINY_SPI_IDLE_VAL, &regs->txdata);
> +       for (i = 1; i < bytes; i++) {
> +               writeb(txp ? *txp++ : CONFIG_TINY_SPI_IDLE_VAL, &regs->txdata);
> +
> +               if (rxp || (i != bytes - 1)) {
> +                       while (!(readb(&regs->status) & TINY_SPI_STATUS_TXR))
>                                         ;
> -                               rx = readb(&regs->txdata);
> -                               writeb(tx, &regs->txdata);
> -                               *rxp++ = rx;
> -                       }
> -                       while (!(readb(&regs->status) &
> -                                TINY_SPI_STATUS_TXR))
> -                               ;
> -                       *rxp++ = readb(&regs->txdata);
>                 }
> -               while (!(readb(&regs->status) &
> -                        TINY_SPI_STATUS_TXE))
> -                       ;
> -               *rxp++ = readb(&regs->rxdata);
> -       } else if (rxp) {
> -               writeb(CONFIG_TINY_SPI_IDLE_VAL, &regs->txdata);
> -               if (bytes > 1) {
> -                       writeb(CONFIG_TINY_SPI_IDLE_VAL,
> -                              &regs->txdata);
> -                       for (i = 2; i < bytes; i++) {
> -                               u8 rx;
> -                               while (!(readb(&regs->status) &
> -                                        TINY_SPI_STATUS_TXR))
> -                                       ;
> -                               rx = readb(&regs->txdata);
> -                               writeb(CONFIG_TINY_SPI_IDLE_VAL,
> -                                      &regs->txdata);
> -                               *rxp++ = rx;
> -                       }
> -                       while (!(readb(&regs->status) &
> -                                TINY_SPI_STATUS_TXR))
> -                               ;
> +
> +               if (rxp)
>                         *rxp++ = readb(&regs->txdata);
> -               }
> -               while (!(readb(&regs->status) &
> -                        TINY_SPI_STATUS_TXE))
> +       }
> +       while (!(readb(&regs->status) & TINY_SPI_STATUS_TXE))
>                         ;
> +       if (rxp)
>                 *rxp++ = readb(&regs->rxdata);
> -       } else if (txp) {
> -               writeb(*txp++, &regs->txdata);
> -               if (bytes > 1) {
> -                       writeb(*txp++, &regs->txdata);
> -                       for (i = 2; i < bytes; i++) {
> -                               u8 tx = *txp++;
> -                               while (!(readb(&regs->status) &
> -                                        TINY_SPI_STATUS_TXR))
> -                                       ;
> -                               writeb(tx, &regs->txdata);
> -                       }
> -               }
> -               while (!(readb(&regs->status) &
> -                        TINY_SPI_STATUS_TXE))
> -                       ;
> -       } else {
> -               writeb(CONFIG_TINY_SPI_IDLE_VAL, &regs->txdata);
> -               if (bytes > 1) {
> -                       writeb(CONFIG_TINY_SPI_IDLE_VAL,
> -                              &regs->txdata);
> -                       for (i = 2; i < bytes; i++) {
> -                               while (!(readb(&regs->status) &
> -                                        TINY_SPI_STATUS_TXR))
> -                                       ;
> -                               writeb(CONFIG_TINY_SPI_IDLE_VAL,
> -                                      &regs->txdata);
> -                       }
> -               }
> -               while (!(readb(&regs->status) &
> -                        TINY_SPI_STATUS_TXE))
> -                       ;
> -       }
> -
>   done:
>         if (flags & SPI_XFER_END)
>                 spi_cs_deactivate(slave);
> --
> 1.8.1.2
>
>
>
> _______________________________________________
> U-Boot mailing list
> U-Boot@lists.denx.de
> http://lists.denx.de/mailman/listinfo/u-boot

thanks!
diff mbox

Patch

diff --git a/drivers/spi/oc_tiny_spi.c b/drivers/spi/oc_tiny_spi.c
index 4de5d00..a8c1dfd 100644
--- a/drivers/spi/oc_tiny_spi.c
+++ b/drivers/spi/oc_tiny_spi.c
@@ -158,85 +158,22 @@  int spi_xfer(struct spi_slave *slave, unsigned int bitlen, const void *dout,
 		spi_cs_activate(slave);
 
 	/* we need to tighten the transfer loop */
-	if (txp && rxp) {
-		writeb(*txp++, &regs->txdata);
-		if (bytes > 1) {
-			writeb(*txp++, &regs->txdata);
-			for (i = 2; i < bytes; i++) {
-				u8 rx, tx = *txp++;
-				while (!(readb(&regs->status) &
-					 TINY_SPI_STATUS_TXR))
+	writeb(txp ? *txp++ : CONFIG_TINY_SPI_IDLE_VAL, &regs->txdata);
+	for (i = 1; i < bytes; i++) {
+		writeb(txp ? *txp++ : CONFIG_TINY_SPI_IDLE_VAL, &regs->txdata);
+
+		if (rxp || (i != bytes - 1)) {
+			while (!(readb(&regs->status) & TINY_SPI_STATUS_TXR))
 					;
-				rx = readb(&regs->txdata);
-				writeb(tx, &regs->txdata);
-				*rxp++ = rx;
-			}
-			while (!(readb(&regs->status) &
-				 TINY_SPI_STATUS_TXR))
-				;
-			*rxp++ = readb(&regs->txdata);
 		}
-		while (!(readb(&regs->status) &
-			 TINY_SPI_STATUS_TXE))
-			;
-		*rxp++ = readb(&regs->rxdata);
-	} else if (rxp) {
-		writeb(CONFIG_TINY_SPI_IDLE_VAL, &regs->txdata);
-		if (bytes > 1) {
-			writeb(CONFIG_TINY_SPI_IDLE_VAL,
-			       &regs->txdata);
-			for (i = 2; i < bytes; i++) {
-				u8 rx;
-				while (!(readb(&regs->status) &
-					 TINY_SPI_STATUS_TXR))
-					;
-				rx = readb(&regs->txdata);
-				writeb(CONFIG_TINY_SPI_IDLE_VAL,
-				       &regs->txdata);
-				*rxp++ = rx;
-			}
-			while (!(readb(&regs->status) &
-				 TINY_SPI_STATUS_TXR))
-				;
+
+		if (rxp)
 			*rxp++ = readb(&regs->txdata);
-		}
-		while (!(readb(&regs->status) &
-			 TINY_SPI_STATUS_TXE))
+	}
+	while (!(readb(&regs->status) & TINY_SPI_STATUS_TXE))
 			;
+	if (rxp)
 		*rxp++ = readb(&regs->rxdata);
-	} else if (txp) {
-		writeb(*txp++, &regs->txdata);
-		if (bytes > 1) {
-			writeb(*txp++, &regs->txdata);
-			for (i = 2; i < bytes; i++) {
-				u8 tx = *txp++;
-				while (!(readb(&regs->status) &
-					 TINY_SPI_STATUS_TXR))
-					;
-				writeb(tx, &regs->txdata);
-			}
-		}
-		while (!(readb(&regs->status) &
-			 TINY_SPI_STATUS_TXE))
-			;
-	} else {
-		writeb(CONFIG_TINY_SPI_IDLE_VAL, &regs->txdata);
-		if (bytes > 1) {
-			writeb(CONFIG_TINY_SPI_IDLE_VAL,
-			       &regs->txdata);
-			for (i = 2; i < bytes; i++) {
-				while (!(readb(&regs->status) &
-					 TINY_SPI_STATUS_TXR))
-					;
-				writeb(CONFIG_TINY_SPI_IDLE_VAL,
-				       &regs->txdata);
-			}
-		}
-		while (!(readb(&regs->status) &
-			 TINY_SPI_STATUS_TXE))
-			;
-	}
-
  done:
 	if (flags & SPI_XFER_END)
 		spi_cs_deactivate(slave);