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

[Jagan Teki]

On Fri, Nov 25, 2016 at 11:16 PM, Marek Vasut <marex at denx.de> wrote:
> On 11/25/2016 06:42 PM, Jagan Teki wrote:
>> On Thu, Nov 24, 2016 at 11:05 AM, Vignesh R <vigneshr at ti.com> 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>
>>> ---
>>>
>>> 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
>>
>> Better define this on Kconfig and add it on defconfig.
>
> Such change is unrelated to this patch and should be fixed in a
> separate/subsequent one.

Agreed it should be a separate patch.

thanks!
-- 
Jagan Teki
Free Software Engineer | www.openedev.com
U-Boot, Linux | Upstream Maintainer
Hyderabad, India.