[U-Boot] [PATCH v2] spi: cadence_qspi_apb: Use 32 bit indirect write transaction when possible

[Marek Vasut]

On 11/24/2016 06:35 AM, Vignesh R wrote:
> According to Section 11.15.4.9.2 Indirect Write Controller of K2G SoC
> TRM SPRUHY8D[1], the external master is only permitted to issue 32-bit
> data interface writes until the last word of an indirect transfer
> otherwise indirect writes is known to fails sometimes. So, make sure
> that QSPI indirect writes are 32 bit sized except for the last write. If
> the txbuf is unaligned then use bounce buffer to avoid data aborts.
> 
> So, now that the driver uses bounce_buffer, enable CONFIG_BOUNCE_BUFFER
> for all boards that use Cadence QSPI driver.
> 
> [1]www.ti.com/lit/ug/spruhy8d/spruhy8d.pdf
> 
> Signed-off-by: Vignesh R <vigneshr at ti.com>
> ---

Reviewed-by: Marek Vasut <marex at denx.de>

I'd like to have at least Dinh's/Chin's ack on this.

btw don't you need BB for READ as well ?

> v2:
>  - Use bounce buffer
>  - Link to v1: https://patchwork.ozlabs.org/patch/693069/
> 
>  drivers/spi/cadence_qspi_apb.c   | 26 ++++++++++++++++++++------
>  include/configs/k2g_evm.h        |  1 +
>  include/configs/socfpga_common.h |  1 +
>  include/configs/stv0991.h        |  1 +
>  4 files changed, 23 insertions(+), 6 deletions(-)
> 
> diff --git a/drivers/spi/cadence_qspi_apb.c b/drivers/spi/cadence_qspi_apb.c
> index e285d3c1e761..6ce98acf747d 100644
> --- a/drivers/spi/cadence_qspi_apb.c
> +++ b/drivers/spi/cadence_qspi_apb.c
> @@ -30,6 +30,7 @@
>  #include <linux/errno.h>
>  #include <wait_bit.h>
>  #include <spi.h>
> +#include <bouncebuf.h>
>  #include "cadence_qspi.h"
>  
>  #define CQSPI_REG_POLL_US			(1) /* 1us */
> @@ -741,6 +742,17 @@ int cadence_qspi_apb_indirect_write_execute(struct cadence_spi_platdata *plat,
>  	unsigned int remaining = n_tx;
>  	unsigned int write_bytes;
>  	int ret;
> +	struct bounce_buffer bb;
> +	u8 *bb_txbuf;
> +
> +	/*
> +	 * Handle non-4-byte aligned accesses via bounce buffer to
> +	 * avoid data abort.
> +	 */
> +	ret = bounce_buffer_start(&bb, (void *)txbuf, n_tx, GEN_BB_READ);
> +	if (ret)
> +		return ret;
> +	bb_txbuf = bb.bounce_buffer;
>  
>  	/* Configure the indirect read transfer bytes */
>  	writel(n_tx, plat->regbase + CQSPI_REG_INDIRECTWRBYTES);
> @@ -751,11 +763,11 @@ int cadence_qspi_apb_indirect_write_execute(struct cadence_spi_platdata *plat,
>  
>  	while (remaining > 0) {
>  		write_bytes = remaining > page_size ? page_size : remaining;
> -		/* Handle non-4-byte aligned access to avoid data abort. */
> -		if (((uintptr_t)txbuf % 4) || (write_bytes % 4))
> -			writesb(plat->ahbbase, txbuf, write_bytes);
> -		else
> -			writesl(plat->ahbbase, txbuf, write_bytes >> 2);
> +		writesl(plat->ahbbase, bb_txbuf, write_bytes >> 2);
> +		if (write_bytes % 4)
> +			writesb(plat->ahbbase,
> +				bb_txbuf + rounddown(write_bytes, 4),
> +				write_bytes % 4);
>  
>  		ret = wait_for_bit("QSPI", plat->regbase + CQSPI_REG_SDRAMLEVEL,
>  				   CQSPI_REG_SDRAMLEVEL_WR_MASK <<
> @@ -765,7 +777,7 @@ int cadence_qspi_apb_indirect_write_execute(struct cadence_spi_platdata *plat,
>  			goto failwr;
>  		}
>  
> -		txbuf += write_bytes;
> +		bb_txbuf += write_bytes;
>  		remaining -= write_bytes;
>  	}
>  
> @@ -776,6 +788,7 @@ int cadence_qspi_apb_indirect_write_execute(struct cadence_spi_platdata *plat,
>  		printf("Indirect write completion error (%i)\n", ret);
>  		goto failwr;
>  	}
> +	bounce_buffer_stop(&bb);
>  
>  	/* Clear indirect completion status */
>  	writel(CQSPI_REG_INDIRECTWR_DONE_MASK,
> @@ -786,6 +799,7 @@ failwr:
>  	/* Cancel the indirect write */
>  	writel(CQSPI_REG_INDIRECTWR_CANCEL_MASK,
>  	       plat->regbase + CQSPI_REG_INDIRECTWR);
> +	bounce_buffer_stop(&bb);
>  	return ret;
>  }
>  
> diff --git a/include/configs/k2g_evm.h b/include/configs/k2g_evm.h
> index a14544526c71..1d603e0c002f 100644
> --- a/include/configs/k2g_evm.h
> +++ b/include/configs/k2g_evm.h
> @@ -79,6 +79,7 @@
>  #define CONFIG_CADENCE_QSPI
>  #define CONFIG_CQSPI_REF_CLK 384000000
>  #define CONFIG_CQSPI_DECODER 0x0
> +#define CONFIG_BOUNCE_BUFFER
>  #endif
>  
>  #endif /* __CONFIG_K2G_EVM_H */
> diff --git a/include/configs/socfpga_common.h b/include/configs/socfpga_common.h
> index d37e5958b586..2de57b063334 100644
> --- a/include/configs/socfpga_common.h
> +++ b/include/configs/socfpga_common.h
> @@ -208,6 +208,7 @@ unsigned int cm_get_qspi_controller_clk_hz(void);
>  #define CONFIG_CQSPI_REF_CLK		cm_get_qspi_controller_clk_hz()
>  #endif
>  #define CONFIG_CQSPI_DECODER		0
> +#define CONFIG_BOUNCE_BUFFER
>  
>  /*
>   * Designware SPI support
> diff --git a/include/configs/stv0991.h b/include/configs/stv0991.h
> index bfd1bd719285..09a3064bd6d6 100644
> --- a/include/configs/stv0991.h
> +++ b/include/configs/stv0991.h
> @@ -74,6 +74,7 @@
>  #ifdef CONFIG_OF_CONTROL		/* QSPI is controlled via DT */
>  #define CONFIG_CQSPI_DECODER		0
>  #define CONFIG_CQSPI_REF_CLK		((30/4)/2)*1000*1000
> +#define CONFIG_BOUNCE_BUFFER
>  
>  #endif
>  
> 


-- 
Best regards,
Marek Vasut