[U-Boot] [PATCH] QSPI flash driver compatibility fixes for mx6ull

Stefano Babic sbabic at denx.de
Mon Jan 28 11:45:12 UTC 2019


Hi Neil,

this patch landed in my queue and I am taking a look at it:

On 09/10/18 12:43, Neil Stainton wrote:
> Avoid "SF: Timeout!" messages and generally speed up QSPI flash
> operations on mx6ull EVK by adding a QSPI sequence for reading
> "flag status register" as required for some (ST/Micron) NOR
> flash devices by the DM spi-flash driver.
> 
> Enable QSPI clock in EVK specific board_init function, as per
> other Freescale EVKs.
> 
> Tested on mcimx6ull EVK, U303 populated with a Micron MT25QL256ABA
> (ID reads as 'n25q256').
> 
> Cc: Peng Fan <peng.fan at nxp.com>
> Cc: Jagan Teki <jagan at openedev.com>
> Signed-off-by: Neil Stainton <nstainton at asl-control.co.uk>
> ---
>  arch/arm/dts/imx6ull-14x14-evk.dts    |  2 +-
>  board/freescale/mx6ullevk/mx6ullevk.c | 15 +++++++++++++++
>  drivers/mtd/spi/spi_flash.c           |  4 ++--
>  drivers/spi/fsl_qspi.c                | 24 +++++++++++++++++++-----
>  4 files changed, 37 insertions(+), 8 deletions(-)
> 
> diff --git a/arch/arm/dts/imx6ull-14x14-evk.dts
> b/arch/arm/dts/imx6ull-14x14-evk.dts
> index 8a1b67d..7cb6d16 100644
> --- a/arch/arm/dts/imx6ull-14x14-evk.dts
> +++ b/arch/arm/dts/imx6ull-14x14-evk.dts
> @@ -454,10 +454,10 @@
>      flash0: n25q256a at 0 {
>          #address-cells = <1>;
>          #size-cells = <1>;
> -        /* compatible = "micron,n25q256a"; */

It is not bad to remember this is a micron, why do you want to drop it ?

>          compatible = "spi-flash";
>          spi-max-frequency = <29000000>;
>          spi-nor,ddr-quad-read-dummy = <6>;
> +        memory-map = <0x60000000 0x02000000>;

What is the meaning here for a a SPI flash ? I do not understand it.

>          reg = <0>;
>      };
>  };

You are touching several topics and they must be addressed in separate
patches. You have a fix for the SPI flash driver and changes for
imx6ull-14x14-evk board. They should belong to separate patches.

> diff --git a/board/freescale/mx6ullevk/mx6ullevk.c
> b/board/freescale/mx6ullevk/mx6ullevk.c
> index ad83f36..46d61bc 100644
> --- a/board/freescale/mx6ullevk/mx6ullevk.c
> +++ b/board/freescale/mx6ullevk/mx6ullevk.c
> @@ -58,11 +58,26 @@ int board_early_init_f(void)
>      return 0;
>  }
> 
> +#ifdef CONFIG_FSL_QSPI
> +static int board_qspi_init(void)
> +{
> +    /* Set the clock */
> +    enable_qspi_clk(0);
> +
> +    return 0;
> +}
> +#endif /* !defined(CONFIG_FSL_QSPI) */
> +
>  int board_init(void)
>  {
>      /* Address of boot parameters */
>      gd->bd->bi_boot_params = PHYS_SDRAM + 0x100;
> 
> +#ifdef CONFIG_FSL_QSPI
> +    /* Set the clock */
> +    board_qspi_init();
> +#endif
> +
>      return 0;

These two are related to mx6ull-evk, and they should belong to an own patch.

>  }
> 
> diff --git a/drivers/mtd/spi/spi_flash.c b/drivers/mtd/spi/spi_flash.c
> index c159124..c817772 100644
> --- a/drivers/mtd/spi/spi_flash.c
> +++ b/drivers/mtd/spi/spi_flash.c
> @@ -360,7 +360,7 @@ int spi_flash_cmd_erase_ops(struct spi_flash *flash,
> u32 offset, size_t len)
>      }
> 
>  #ifdef CONFIG_SPI_FLASH_BAR
> -    ret = clean_bar(flash);
> +    clean_bar(flash);

I supposed the return value is checked, not that it will simply ignored.

>  #endif
> 
>      return ret;
> @@ -422,7 +422,7 @@ int spi_flash_cmd_write_ops(struct spi_flash *flash,
> u32 offset,
>      }
> 
>  #ifdef CONFIG_SPI_FLASH_BAR
> -    ret = clean_bar(flash);
> +    clean_bar(flash);
>  #endif
> 
>      return ret;
> diff --git a/drivers/spi/fsl_qspi.c b/drivers/spi/fsl_qspi.c
> index 1598c4f..07c3ade 100644
> --- a/drivers/spi/fsl_qspi.c
> +++ b/drivers/spi/fsl_qspi.c
> @@ -47,10 +47,12 @@ DECLARE_GLOBAL_DATA_PTR;
>  #endif
>  #define SEQID_WRAR        13
>  #define SEQID_RDAR        14
> +#define SEQID_RDFSR        15
> 
>  /* QSPI CMD */
>  #define QSPI_CMD_PP        0x02    /* Page program (up to 256 bytes) */
>  #define QSPI_CMD_RDSR        0x05    /* Read status register */
> +#define QSPI_CMD_RDFSR        0x70    /* Read flag status register */
>  #define QSPI_CMD_WREN        0x06    /* Write enable */
>  #define QSPI_CMD_FAST_READ    0x0b    /* Read data bytes (high
> frequency) */
>  #define QSPI_CMD_BE_4K        0x20    /* 4K erase */
> @@ -230,6 +232,15 @@ static void qspi_set_lut(struct fsl_qspi_priv *priv)
>      qspi_write32(priv->flags, &regs->lut[lut_base + 2], 0);
>      qspi_write32(priv->flags, &regs->lut[lut_base + 3], 0);
> 
> +    /* Read Flag Status */
> +    lut_base = SEQID_RDFSR * 4;
> +    qspi_write32(priv->flags, &regs->lut[lut_base],
> OPRND0(QSPI_CMD_RDFSR) |
> +        PAD0(LUT_PAD1) | INSTR0(LUT_CMD) | OPRND1(1) |
> +        PAD1(LUT_PAD1) | INSTR1(LUT_READ));
> +    qspi_write32(priv->flags, &regs->lut[lut_base + 1], 0);
> +    qspi_write32(priv->flags, &regs->lut[lut_base + 2], 0);
> +    qspi_write32(priv->flags, &regs->lut[lut_base + 3], 0);
> +
>      /* Erase a sector */
>      lut_base = SEQID_SE * 4;
>  #ifdef CONFIG_SPI_FLASH_BAR
> @@ -698,7 +709,7 @@ static void qspi_op_write(struct fsl_qspi_priv
> *priv, u8 *txbuf, u32 len)
>      qspi_write32(priv->flags, &regs->mcr, mcr_reg);
>  }
> 
> -static void qspi_op_rdsr(struct fsl_qspi_priv *priv, void *rxbuf, u32 len)
> +static void qspi_op_rdsr(struct fsl_qspi_priv *priv, void *rxbuf, u32
> len, u8 seqid)
>  {
>      struct fsl_qspi_regs *regs = priv->regs;
>      u32 mcr_reg, reg, data;
> @@ -712,7 +723,7 @@ static void qspi_op_rdsr(struct fsl_qspi_priv *priv,
> void *rxbuf, u32 len)
>      qspi_write32(priv->flags, &regs->sfar, priv->cur_amba_base);
> 
>      qspi_write32(priv->flags, &regs->ipcr,
> -             (SEQID_RDSR << QSPI_IPCR_SEQID_SHIFT) | 0);
> +             (seqid << QSPI_IPCR_SEQID_SHIFT) | 0);
>      while (qspi_read32(priv->flags, &regs->sr) & QSPI_SR_BUSY_MASK)
>          ;
> 
> @@ -815,10 +826,13 @@ int qspi_xfer(struct fsl_qspi_priv *priv, unsigned
> int bitlen,
>  #endif
>          } else if (priv->cur_seqid == QSPI_CMD_RDAR) {
>              qspi_op_read(priv, din, bytes);
> -        } else if (priv->cur_seqid == QSPI_CMD_RDID)
> +        } else if (priv->cur_seqid == QSPI_CMD_RDID) {
>              qspi_op_rdid(priv, din, bytes);
> -        else if (priv->cur_seqid == QSPI_CMD_RDSR)
> -            qspi_op_rdsr(priv, din, bytes);
> +        } else if (priv->cur_seqid == QSPI_CMD_RDSR) {
> +            qspi_op_rdsr(priv, din, bytes, SEQID_RDSR);
> +        } else if (priv->cur_seqid == QSPI_CMD_RDFSR) {
> +            qspi_op_rdsr(priv, din, bytes, SEQID_RDFSR);
> +        }

I am not so deep in SF to comment this - maybe Jagan can say something
more. I have never seen this issue up now - Heiko, you have also a M6ULL
board, have you seen this ?

Best regards,
Stefano

-- 
=====================================================================
DENX Software Engineering GmbH,      Managing Director: Wolfgang Denk
HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany
Phone: +49-8142-66989-53 Fax: +49-8142-66989-80 Email: sbabic at denx.de
=====================================================================


More information about the U-Boot mailing list