> -----Original Message-----
> From: Jagan Teki [mailto:[email protected]]
> Sent: Tuesday, August 29, 2017 11:08 PM
> To: Suresh Gupta <[email protected]>
> Cc: [email protected]; York Sun <[email protected]>
> Subject: Re: [PATCH v2] spi: fsl_qspi: Add controller busy check before new
> spi
> operation
>
> On Tue, Aug 29, 2017 at 6:55 PM, Suresh Gupta <[email protected]>
> 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 controller is currently busy handling
> > a transaction to an external flash device
> >
> > Signed-off-by: Suresh Gupta <[email protected]>
> > ---
> > Changes in v2:
> >
> > - Add wait_for_bit instead of while
> > - move the busy check code to fsl_qspi_claim_bus form qspi_xfer
> >
> > drivers/spi/fsl_qspi.c | 28 +++++++++++++++++++++++++++-
> > drivers/spi/fsl_qspi.h | 4 ++++
> > 2 files changed, 31 insertions(+), 1 deletion(-)
> >
> > diff --git a/drivers/spi/fsl_qspi.c b/drivers/spi/fsl_qspi.c index
> > 1dfa89a..ed23aac 100644
> > --- a/drivers/spi/fsl_qspi.c
> > +++ b/drivers/spi/fsl_qspi.c
> > @@ -14,6 +14,7 @@
> > #include <dm.h>
> > #include <errno.h>
> > #include <watchdog.h>
> > +#include <wait_bit.h>
> > #include "fsl_qspi.h"
> >
> > DECLARE_GLOBAL_DATA_PTR;
> > @@ -991,7 +992,7 @@ static int fsl_qspi_probe(struct udevice *bus)
> > struct fsl_qspi_platdata *plat = dev_get_platdata(bus);
> > struct fsl_qspi_priv *priv = dev_get_priv(bus);
> > struct dm_spi_bus *dm_spi_bus;
> > - int i;
> > + int i, ret;
> >
> > dm_spi_bus = bus->uclass_priv;
> >
> > @@ -1011,6 +1012,18 @@ static int fsl_qspi_probe(struct udevice *bus)
> > priv->flash_num = plat->flash_num;
> > priv->num_chipselect = plat->num_chipselect;
> >
>
> I think in previous version, this code not added in probe is it? is this
> because
> probe doing mcr and other reg operations?
The probe function changes the LUTs, change AHB configuration and change the
endianness.
So, changing above setting when the controller is busy in some AHB access will
affect the running access.
>
> > + /* make sure controller is not busy anywhere */
> > + ret = wait_for_bit(__func__, &priv->regs->sr,
> > + QSPI_SR_BUSY_MASK |
> > + QSPI_SR_AHB_ACC_MASK |
> > + QSPI_SR_IP_ACC_MASK,
> > + false, 1000, false);
> > +
> > + if (ret) {
> > + printf("ERROR : The controller is busy\n");
> > + return -EBUSY;
> > + }
>
> Better to drop printf or use debug and
The error above is trivial and after this error, the sf commands do not work.
And if we hide the message under debug, normal user will not understand the
issue.
As per me this should be printf, what you say?
> wait_for_bit usually return -ETIMEDOUT
> or -EINTR on failure so just return ret.
Ok, I will make changes in next patch
>
> thanks!
> --
> Jagan Teki
> Free Software Engineer | www.openedev.com U-Boot, Linux | Upstream
> Maintainer Hyderabad, India.
_______________________________________________
U-Boot mailing list
[email protected]
https://lists.denx.de/listinfo/u-boot
