On Tuesday 09 January 2018 11:31 AM, Goldschmidt Simon wrote:

On Mon, 08/01/2018 12:18, Vignesh R wrote:

...

Make flash writes 32 bit aligned by using bounce buffers to deal with non 32 bit aligned buffers. Allocate a 512 byte bounce buffer (max known page size currently) for this case.

Looking at drivers/mtd/spi/sf_dataflash.c, I see at least one chip with 1024 byte page size (at45db642d). This should probably be a constant somewhere, or at least be checked at runtime, see below.

Oh, I missed the dataflash devices. Since, we don't get the page size info from framework it would be difficult to do allocate bounce buffer only once in probe. I think better way would be to allocate and free bounce buffer as and when required in cadence_qspi_apb_indirect_write_execute() just like bounce_buffer_start/stop. Will make changes and post v2 shortly

Regards Vignesh

...

This is required because as per TI K2G TRM[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 fail sometimes.

[1] http://www.ti.com/lit/ug/spruhy8g/spruhy8g.pdf

Signed-off-by: Vignesh R vigneshr@ti.com

drivers/spi/cadence_qspi.c | 13 ++++++++++++- drivers/spi/cadence_qspi.h | 1 + drivers/spi/cadence_qspi_apb.c | 10 +++++----- 3 files changed, 18 insertions(+), 6 deletions(-)

diff --git a/drivers/spi/cadence_qspi.c b/drivers/spi/cadence_qspi.c index 991a7160bdb8..49c9b477e678 100644 --- a/drivers/spi/cadence_qspi.c +++ b/drivers/spi/cadence_qspi.c @@ -158,6 +158,10 @@ static int cadence_spi_probe(struct udevice *bus)

priv->regbase = plat->regbase; priv->ahbbase = plat->ahbbase;

• /* Bounce buf to handle unaligned writes. 512B is max pagesize */
• priv->bounce_buf = malloc(512);

This should probably be a named constant.

...

• if (!priv->bounce_buf)

• return -ENOMEM;
  
  

  if (!priv->qspi_is_init) { cadence_qspi_apb_controller_init(plat);

@@ -259,8 +263,15 @@ static int cadence_spi_xfer(struct udevice *dev, unsigned int bitlen, err = cadence_qspi_apb_indirect_write_setup (plat, priv->cmd_len, cmd_buf); if (!err) {

• 		const u8 *txbuf = dout;
  

• 		if ((uintptr_t)txbuf % 4) {
  

• 			memcpy(priv->bounce_buf, dout,
  

• 			       data_bytes);
  

Without checking the size of the buffer allocated for bounce_buf here, you risk a buffer overflow for chips with larger page size.

...

• 			txbuf = priv->bounce_buf;
  

• 		}
  	err = cadence_qspi_apb_indirect_write_execute
  

• 		(plat, data_bytes, dout);
  

• 		(plat, data_bytes, txbuf);
  }
  

  break; default:

diff --git a/drivers/spi/cadence_qspi.h b/drivers/spi/cadence_qspi.h index 83154210bd95..c97419af3de3 100644 --- a/drivers/spi/cadence_qspi.h +++ b/drivers/spi/cadence_qspi.h @@ -43,6 +43,7 @@ struct cadence_spi_priv { unsigned int qspi_calibrated_hz; unsigned int qspi_calibrated_cs; unsigned int previous_hz;

• void *bounce_buf;

};

/* Functions call declaration */ diff --git a/drivers/spi/cadence_qspi_apb.c b/drivers/spi/cadence_qspi_apb.c index fa8af33ae19b..fd3ece4cb129 100644 --- a/drivers/spi/cadence_qspi_apb.c +++ b/drivers/spi/cadence_qspi_apb.c @@ -727,11 +727,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, txbuf, write_bytes >> 2);
  

• if (write_bytes % 4)
  

• 	writesb(plat->ahbbase,
  

• 		txbuf + rounddown(write_bytes, 4),
  

• 		write_bytes % 4);
  
  

  ret = wait_for_bit("QSPI", plat->regbase +

CQSPI_REG_SDRAMLEVEL, CQSPI_REG_SDRAMLEVEL_WR_MASK << -- 2.15.1

Regards, Simon