diff mbox

[U-Boot,1/3] MX28: SPI: Refactor spi_xfer a bit

Message ID 1341797627-27782-1-git-send-email-marex@denx.de
State Superseded
Delegated to: Stefano Babic
Headers show

Commit Message

Marek Vasut July 9, 2012, 1:33 a.m. UTC 
This makes it easier to adapt for addition of DMA support.

Signed-off-by: Marek Vasut <marex@denx.de>
Cc: Fabio Estevam <festevam@freescale.com>
Cc: Otavio Salvador <otavio@ossystems.com.br>
Cc: Stefano Babic <sbabic@denx.de>
Cc: Wolfgang Denk <wd@denx.de>
---
 drivers/spi/mxs_spi.c |   31 +++++++++++++++++++++----------
 1 file changed, 21 insertions(+), 10 deletions(-)

Comments

Stefano Babic July 9, 2012, 9:37 a.m. UTC | #1 
On 09/07/2012 03:33, Marek Vasut wrote:
> This makes it easier to adapt for addition of DMA support.
> 
> Signed-off-by: Marek Vasut <marex@denx.de>
> Cc: Fabio Estevam <festevam@freescale.com>
> Cc: Otavio Salvador <otavio@ossystems.com.br>
> Cc: Stefano Babic <sbabic@denx.de>
> Cc: Wolfgang Denk <wd@denx.de>
> ---
>  drivers/spi/mxs_spi.c |   31 +++++++++++++++++++++----------
>  1 file changed, 21 insertions(+), 10 deletions(-)
> 
> diff --git a/drivers/spi/mxs_spi.c b/drivers/spi/mxs_spi.c
> index 7859536..a47b25b 100644
> --- a/drivers/spi/mxs_spi.c
> +++ b/drivers/spi/mxs_spi.c
> @@ -146,20 +146,31 @@ int spi_xfer(struct spi_slave *slave, unsigned int bitlen,
>  	struct mxs_spi_slave *mxs_slave = to_mxs_slave(slave);
>  	struct mx28_ssp_regs *ssp_regs = mxs_slave->regs;
>  	int len = bitlen / 8;
> -	const char *tx = dout;
> -	char *rx = din;
>  	char dummy;
> +	int write = 0;
> +	char *data = NULL;
>  
>  	if (bitlen == 0) {
>  		if (flags & SPI_XFER_END) {
> -			rx = &dummy;
> +			din = (void *)&dummy;
>  			len = 1;
>  		} else
>  			return 0;
>  	}
>  
> -	if (!rx && !tx)
> +	if (din && dout) {
> +		/* Half-duplex only */
> +		return -EINVAL;
> +	} else if (!din && !dout) {
> +		/* No data */
>  		return 0;
> +	} else if (dout) {
> +		data = (char *)dout;
> +		write = 1;
> +	} else if (din) {
> +		data = (char *)din;
> +		write = 0;
> +	}

You do not need this if-then-else chain. I think is more readable with :

	if (din && dout) {
		/* Half-duplex only */
		return -EINVAL;
	}
	if (!din && !dout) {
		/* No data */
  		return 0;
	}


Best regards,
Stefano Babic
Marek Vasut July 9, 2012, 10:13 a.m. UTC | #2 
Dear Stefano Babic,

> On 09/07/2012 03:33, Marek Vasut wrote:
> > This makes it easier to adapt for addition of DMA support.
> > 
> > Signed-off-by: Marek Vasut <marex@denx.de>
> > Cc: Fabio Estevam <festevam@freescale.com>
> > Cc: Otavio Salvador <otavio@ossystems.com.br>
> > Cc: Stefano Babic <sbabic@denx.de>
> > Cc: Wolfgang Denk <wd@denx.de>
> > ---

[...]

> You do not need this if-then-else chain. I think is more readable with :

I think you're right ;-)

> 	if (din && dout) {
> 		/* Half-duplex only */
> 		return -EINVAL;
> 	}
> 	if (!din && !dout) {
> 		/* No data */
>   		return 0;
> 	}
> 
> 
> Best regards,
> Stefano Babic

Best regards,
Marek Vasut
diff mbox

Patch

diff --git a/drivers/spi/mxs_spi.c b/drivers/spi/mxs_spi.c
index 7859536..a47b25b 100644
--- a/drivers/spi/mxs_spi.c
+++ b/drivers/spi/mxs_spi.c
@@ -146,20 +146,31 @@  int spi_xfer(struct spi_slave *slave, unsigned int bitlen,
 	struct mxs_spi_slave *mxs_slave = to_mxs_slave(slave);
 	struct mx28_ssp_regs *ssp_regs = mxs_slave->regs;
 	int len = bitlen / 8;
-	const char *tx = dout;
-	char *rx = din;
 	char dummy;
+	int write = 0;
+	char *data = NULL;
 
 	if (bitlen == 0) {
 		if (flags & SPI_XFER_END) {
-			rx = &dummy;
+			din = (void *)&dummy;
 			len = 1;
 		} else
 			return 0;
 	}
 
-	if (!rx && !tx)
+	if (din && dout) {
+		/* Half-duplex only */
+		return -EINVAL;
+	} else if (!din && !dout) {
+		/* No data */
 		return 0;
+	} else if (dout) {
+		data = (char *)dout;
+		write = 1;
+	} else if (din) {
+		data = (char *)din;
+		write = 0;
+	}
 
 	if (flags & SPI_XFER_BEGIN)
 		mxs_spi_start_xfer(ssp_regs);
@@ -171,7 +182,7 @@  int spi_xfer(struct spi_slave *slave, unsigned int bitlen,
 		if ((flags & SPI_XFER_END) && !len)
 			mxs_spi_end_xfer(ssp_regs);
 
-		if (tx)
+		if (write)
 			writel(SSP_CTRL0_READ, &ssp_regs->hw_ssp_ctrl0_clr);
 		else
 			writel(SSP_CTRL0_READ, &ssp_regs->hw_ssp_ctrl0_set);
@@ -184,20 +195,20 @@  int spi_xfer(struct spi_slave *slave, unsigned int bitlen,
 			return -ETIMEDOUT;
 		}
 
-		if (tx)
-			writel(*tx++, &ssp_regs->hw_ssp_data);
+		if (write)
+			writel(*data++, &ssp_regs->hw_ssp_data);
 
 		writel(SSP_CTRL0_DATA_XFER, &ssp_regs->hw_ssp_ctrl0_set);
 
-		if (rx) {
+		if (!write) {
 			if (mx28_wait_mask_clr(&ssp_regs->hw_ssp_status_reg,
 				SSP_STATUS_FIFO_EMPTY, MXS_SPI_MAX_TIMEOUT)) {
 				printf("MXS SPI: Timeout waiting for data\n");
 				return -ETIMEDOUT;
 			}
 
-			*rx = readl(&ssp_regs->hw_ssp_data);
-			rx++;
+			*data = readl(&ssp_regs->hw_ssp_data);
+			data++;
 		}
 
 		if (mx28_wait_mask_clr(&ssp_regs->hw_ssp_ctrl0_reg,