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 <vigne...@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 > _______________________________________________ U-Boot mailing list U-Boot@lists.denx.de https://lists.denx.de/listinfo/u-boot
