[U-Boot] [PATCH] spi: fsl_qspi: Add controller busy check before new spi operation

[Suresh Gupta]

> -----Original Message-----
> From: York Sun
> Sent: Tuesday, August 22, 2017 9:56 PM
> To: Suresh Gupta <suresh.gupta at nxp.com>; u-boot at lists.denx.de
> Cc: jagan at openedev.com; Prabhakar Kushwaha
> <prabhakar.kushwaha at nxp.com>
> Subject: Re: [PATCH] spi: fsl_qspi: Add controller busy check before new spi
> operation
> 
> On 08/21/2017 03:25 AM, Suresh Gupta wrote:
> > It is recommended to check either controller is free to take new spi
> > action. The IP_ACC and AHB_ACC bits indicates that the controller is
> > busy in IP or AHB mode respectively.
> > And the BUSY bit indicates that the controller is currently busy
> > handling a transaction to an external flash device
> >
> > Signed-off-by: Suresh Gupta <suresh.gupta at nxp.com>
> > ---
> >   drivers/spi/fsl_qspi.c | 26 ++++++++++++++++++++++++++
> >   drivers/spi/fsl_qspi.h |  4 ++++
> >   2 files changed, 30 insertions(+)
> >
> > diff --git a/drivers/spi/fsl_qspi.c b/drivers/spi/fsl_qspi.c index
> > 1dfa89a..69e9712 100644
> > --- a/drivers/spi/fsl_qspi.c
> > +++ b/drivers/spi/fsl_qspi.c
> > @@ -165,6 +165,27 @@ static inline u32 qspi_endian_xchg(u32 data)
> >   #endif
> >   }
> >
> > +static inline u32 qspi_controller_busy(struct fsl_qspi_priv *priv) {
> > +	u32 sr;
> > +	u32 retry = 5;
> > +
> > +	do {
> > +		sr = qspi_read32(priv->flags, &priv->regs->sr);
> > +		if ((sr & QSPI_SR_BUSY_MASK) ||
> > +		    (sr & QSPI_SR_AHB_ACC_MASK) ||
> > +		    (sr & QSPI_SR_IP_ACC_MASK)) {
> > +			debug("The controller is busy, sr = 0x%x\n", sr);
> > +			udelay(1);
> > +		} else {
> > +			break;
> > +		}
> > +	} while (--retry);
> 
> Does the 5 microsecond-delay make any difference?
> 
> > +
> > +	return (sr & QSPI_SR_BUSY_MASK) ||
> > +		(sr & QSPI_SR_AHB_ACC_MASK) || (sr &
> QSPI_SR_IP_ACC_MASK); }
> > +
> >   static void qspi_set_lut(struct fsl_qspi_priv *priv)
> >   {
> >   	struct fsl_qspi_regs *regs = priv->regs; @@ -765,6 +786,11 @@ int
> > qspi_xfer(struct fsl_qspi_priv *priv, unsigned int bitlen,
> >
> >   	WATCHDOG_RESET();
> >
> > +	if (qspi_controller_busy(priv)) {
> > +		printf("ERROR : The controller is busy\n");
> > +		return -EBUSY;
> > +	}
> 
> If the controller should never be busy for this operation, I wonder if you really
> need a delay above.

As we see in our setup that controller get free after some time. So, I took 5 microseconds as arbitrary number. 
Moreover, below statement [1] of BG points that controller gets free after prefetch completes. 

[1] Snapshot from RM: 
For any AHB access, the sequence pointed to by the QSPI_BFGENCR [SEQID] field is used for
the flash transaction initiated. The data is returned to the master as soon as the requested
amount is read from the serial flash. The controller however, continues to prefetch the
rest of the data in anticipation of a next consecutive request.
 
> 
> York