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

[Jagan Teki]

On Tue, Aug 29, 2017 at 6:55 PM, Suresh Gupta <suresh.gupta at 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 at nxp.com>
> ---
> 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?

> +       /* 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 wait_for_bit usually return
-ETIMEDOUT or -EINTR on failure so just return ret.

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