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

[See, Chin Liang]

On Jum, 2016-11-25 at 17:51 +0100, Marek Vasut wrote:
> 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.

THanks Marek

Hmmm... From 11.15.4.1.1, the data slave port should able to accept
only byte, half-word and word access. This should not create any data
abort but probably bad performance. But it should insignificant as
access time for the flash is longer than the data port access itself.

Thanks
Chin Liang

> 
> 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
> 
> ________________________________
> 
> Confidentiality Notice.
> This message may contain information that is confidential or
> otherwise protected from disclosure. If you are not the intended
> recipient, you are hereby notified that any use, disclosure,
> dissemination, distribution, or copying of this message, or any
> attachments, is strictly prohibited. If you have received this
> message in error, please advise the sender by reply e-mail, and
> delete the message and any attachments. Thank you.