| Message ID | 1504081806-16877-1-git-send-email-suresh.gupta@nxp.com |
|---|---|
| State | Superseded |
| Headers | show |
| Series | [U-Boot,v3] spi: fsl_qspi: Add controller busy check before new spi operation | expand |
On Wed, Aug 30, 2017 at 2:00 PM, Suresh Gupta <suresh.gupta@nxp.com> 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 <suresh.gupta@nxp.com> > --- > Chnages in v3: > - replace printf to debug > - return whatever return from wait_for_bit, before it was -EBUSY > > 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..8f0f29e 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; > > + /* 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); 100ms not enough? thanks!
> -----Original Message----- > From: Jagan Teki [mailto:jagannadh.teki@gmail.com] > Sent: Wednesday, August 30, 2017 7:54 PM > To: Suresh Gupta <suresh.gupta@nxp.com> > Cc: u-boot@lists.denx.de; York Sun <york.sun@nxp.com> > Subject: Re: [PATCH v3] spi: fsl_qspi: Add controller busy check before new spi > operation > > On Wed, Aug 30, 2017 at 2:00 PM, Suresh Gupta <suresh.gupta@nxp.com> > 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 <suresh.gupta@nxp.com> > > --- > > Chnages in v3: > > - replace printf to debug > > - return whatever return from wait_for_bit, before it was -EBUSY > > > > 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..8f0f29e 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; > > > > + /* 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); > > 100ms not enough? It should be, Will send new patch > > thanks! > -- > Jagan Teki > Free Software Engineer | www.openedev.com U-Boot, Linux | Upstream > Maintainer Hyderabad, India.
On Wed, Aug 30, 2017 at 8:01 PM, Suresh Gupta <suresh.gupta@nxp.com> wrote: > > >> -----Original Message----- >> From: Jagan Teki [mailto:jagannadh.teki@gmail.com] >> Sent: Wednesday, August 30, 2017 7:54 PM >> To: Suresh Gupta <suresh.gupta@nxp.com> >> Cc: u-boot@lists.denx.de; York Sun <york.sun@nxp.com> >> Subject: Re: [PATCH v3] spi: fsl_qspi: Add controller busy check before new spi >> operation >> >> On Wed, Aug 30, 2017 at 2:00 PM, Suresh Gupta <suresh.gupta@nxp.com> >> 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 <suresh.gupta@nxp.com> >> > --- >> > Chnages in v3: >> > - replace printf to debug >> > - return whatever return from wait_for_bit, before it was -EBUSY >> > >> > 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..8f0f29e 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; >> > >> > + /* 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); >> >> 100ms not enough? > It should be, Will send new patch It's fine, if you're OK I will change it while applying. thanks!
diff --git a/drivers/spi/fsl_qspi.c b/drivers/spi/fsl_qspi.c index 1dfa89a..8f0f29e 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; + /* 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) { + debug("ERROR : The controller is busy\n"); + return ret; + } + mcr_val = qspi_read32(priv->flags, &priv->regs->mcr); qspi_write32(priv->flags, &priv->regs->mcr, QSPI_MCR_RESERVED_MASK | QSPI_MCR_MDIS_MASK | @@ -1156,10 +1169,23 @@ static int fsl_qspi_claim_bus(struct udevice *dev) struct fsl_qspi_priv *priv; struct udevice *bus; struct dm_spi_slave_platdata *slave_plat = dev_get_parent_platdata(dev); + int ret; bus = dev->parent; priv = dev_get_priv(bus); + /* 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) { + debug("ERROR : The controller is busy\n"); + return ret; + } + priv->cur_amba_base = priv->amba_base[slave_plat->cs]; qspi_module_disable(priv, 0); diff --git a/drivers/spi/fsl_qspi.h b/drivers/spi/fsl_qspi.h index 6cb3610..e468eb2 100644 --- a/drivers/spi/fsl_qspi.h +++ b/drivers/spi/fsl_qspi.h @@ -105,6 +105,10 @@ struct fsl_qspi_regs { #define QSPI_RBCT_RXBRD_SHIFT 8 #define QSPI_RBCT_RXBRD_USEIPS (1 << QSPI_RBCT_RXBRD_SHIFT) +#define QSPI_SR_AHB_ACC_SHIFT 2 +#define QSPI_SR_AHB_ACC_MASK (1 << QSPI_SR_AHB_ACC_SHIFT) +#define QSPI_SR_IP_ACC_SHIFT 1 +#define QSPI_SR_IP_ACC_MASK (1 << QSPI_SR_IP_ACC_SHIFT) #define QSPI_SR_BUSY_SHIFT 0 #define QSPI_SR_BUSY_MASK (1 << QSPI_SR_BUSY_SHIFT)
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 <suresh.gupta@nxp.com> --- Chnages in v3: - replace printf to debug - return whatever return from wait_for_bit, before it was -EBUSY 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(-)