| Message ID | 1565777311-1752-1-git-send-email-ye.li@nxp.com |
|---|---|
| State | Changes Requested |
| Delegated to: | Stefano Babic |
| Headers | show |
| Series | [U-Boot,1/6] spi: fsl_qspi: Fix DDR mode setting for latest iMX platforms | expand |
Please ignore the patch set. I will send out V2 to remove 2 patches. > On latest iMX platforms like iMX7D/iMX6UL/iMX8MQ, the QSPI controller > is updated to have TDH field in FLSHCR register. According to reference > manual, this TDH must be set to 1 when DDR_EN is set. Otherwise, the TX > DDR delay logic won't be enabled. > > Another issue in DDR mode is the MCR register will be overwritten in > every read/write/erase operation. This causes DDR_EN been cleared while > TDH=1, then no clk2x output for TX data shift and all operations will > fail. > > Signed-off-by: Ye Li <ye.li@nxp.com> > --- > drivers/spi/fsl_qspi.c | 30 ++++++++++++++++-------------- > 1 file changed, 16 insertions(+), 14 deletions(-) > > diff --git a/drivers/spi/fsl_qspi.c b/drivers/spi/fsl_qspi.c > index 41abe19..8845986 100644 > --- a/drivers/spi/fsl_qspi.c > +++ b/drivers/spi/fsl_qspi.c > @@ -399,7 +399,7 @@ static inline void qspi_ahb_read(struct fsl_qspi_priv *priv, u8 *rxbuf, int len) > > qspi_write32(priv->flags, ®s->mcr, > QSPI_MCR_CLR_RXF_MASK | QSPI_MCR_CLR_TXF_MASK | > - QSPI_MCR_RESERVED_MASK | QSPI_MCR_END_CFD_LE); > + mcr_reg); > > rx_addr = (void *)(uintptr_t)(priv->cur_amba_base + priv->sf_addr); > /* Read out the data directly from the AHB buffer. */ > @@ -429,6 +429,14 @@ static void qspi_enable_ddr_mode(struct fsl_qspi_priv *priv) > reg |= BIT(29); > > qspi_write32(priv->flags, ®s->mcr, reg); > + > + /* Enable the TDH to 1 for some platforms like imx6ul, imx7d, etc > + * These two bits are reserved on other platforms > + */ > + reg = qspi_read32(priv->flags, ®s->flshcr); > + reg &= ~(BIT(17)); > + reg |= BIT(16); > + qspi_write32(priv->flags, ®s->flshcr, reg); > } > > /* > @@ -482,7 +490,7 @@ static void qspi_op_rdbank(struct fsl_qspi_priv *priv, u8 *rxbuf, u32 len) > mcr_reg = qspi_read32(priv->flags, ®s->mcr); > qspi_write32(priv->flags, ®s->mcr, > QSPI_MCR_CLR_RXF_MASK | QSPI_MCR_CLR_TXF_MASK | > - QSPI_MCR_RESERVED_MASK | QSPI_MCR_END_CFD_LE); > + mcr_reg); > qspi_write32(priv->flags, ®s->rbct, QSPI_RBCT_RXBRD_USEIPS); > > qspi_write32(priv->flags, ®s->sfar, priv->cur_amba_base); > @@ -527,7 +535,7 @@ static void qspi_op_rdid(struct fsl_qspi_priv *priv, u32 *rxbuf, u32 len) > mcr_reg = qspi_read32(priv->flags, ®s->mcr); > qspi_write32(priv->flags, ®s->mcr, > QSPI_MCR_CLR_RXF_MASK | QSPI_MCR_CLR_TXF_MASK | > - QSPI_MCR_RESERVED_MASK | QSPI_MCR_END_CFD_LE); > + mcr_reg); > qspi_write32(priv->flags, ®s->rbct, QSPI_RBCT_RXBRD_USEIPS); > > qspi_write32(priv->flags, ®s->sfar, priv->cur_amba_base); > @@ -573,7 +581,7 @@ static void qspi_op_read(struct fsl_qspi_priv *priv, u32 *rxbuf, u32 len) > mcr_reg = qspi_read32(priv->flags, ®s->mcr); > qspi_write32(priv->flags, ®s->mcr, > QSPI_MCR_CLR_RXF_MASK | QSPI_MCR_CLR_TXF_MASK | > - QSPI_MCR_RESERVED_MASK | QSPI_MCR_END_CFD_LE); > + mcr_reg); > qspi_write32(priv->flags, ®s->rbct, QSPI_RBCT_RXBRD_USEIPS); > > to_or_from = priv->sf_addr + priv->cur_amba_base; > @@ -625,7 +633,7 @@ static void qspi_op_write(struct fsl_qspi_priv *priv, u8 *txbuf, u32 len) > mcr_reg = qspi_read32(priv->flags, ®s->mcr); > qspi_write32(priv->flags, ®s->mcr, > QSPI_MCR_CLR_RXF_MASK | QSPI_MCR_CLR_TXF_MASK | > - QSPI_MCR_RESERVED_MASK | QSPI_MCR_END_CFD_LE); > + mcr_reg); > qspi_write32(priv->flags, ®s->rbct, QSPI_RBCT_RXBRD_USEIPS); > > status_reg = 0; > @@ -700,7 +708,7 @@ static void qspi_op_rdsr(struct fsl_qspi_priv *priv, void *rxbuf, u32 len) > mcr_reg = qspi_read32(priv->flags, ®s->mcr); > qspi_write32(priv->flags, ®s->mcr, > QSPI_MCR_CLR_RXF_MASK | QSPI_MCR_CLR_TXF_MASK | > - QSPI_MCR_RESERVED_MASK | QSPI_MCR_END_CFD_LE); > + mcr_reg); > qspi_write32(priv->flags, ®s->rbct, QSPI_RBCT_RXBRD_USEIPS); > > qspi_write32(priv->flags, ®s->sfar, priv->cur_amba_base); > @@ -737,7 +745,7 @@ static void qspi_op_erase(struct fsl_qspi_priv *priv) > mcr_reg = qspi_read32(priv->flags, ®s->mcr); > qspi_write32(priv->flags, ®s->mcr, > QSPI_MCR_CLR_RXF_MASK | QSPI_MCR_CLR_TXF_MASK | > - QSPI_MCR_RESERVED_MASK | QSPI_MCR_END_CFD_LE); > + mcr_reg); > qspi_write32(priv->flags, ®s->rbct, QSPI_RBCT_RXBRD_USEIPS); > > to_or_from = priv->sf_addr + priv->cur_amba_base; > @@ -900,15 +908,9 @@ static int fsl_qspi_probe(struct udevice *bus) > return ret; > } > > - mcr_val = qspi_read32(priv->flags, &priv->regs->mcr); > - > - /* Set endianness to LE for i.mx */ > - if (IS_ENABLED(CONFIG_MX6) || IS_ENABLED(CONFIG_MX7)) > - mcr_val = QSPI_MCR_END_CFD_LE; > - > qspi_write32(priv->flags, &priv->regs->mcr, > QSPI_MCR_RESERVED_MASK | QSPI_MCR_MDIS_MASK | > - (mcr_val & QSPI_MCR_END_CFD_MASK)); > + QSPI_MCR_END_CFD_LE); > > qspi_cfg_smpr(priv, ~(QSPI_SMPR_FSDLY_MASK | QSPI_SMPR_DDRSMP_MASK | > QSPI_SMPR_FSPHS_MASK | QSPI_SMPR_HSENA_MASK), 0); >
Hi Ye, On 14.08.19 12:08, Ye Li wrote: > On latest iMX platforms like iMX7D/iMX6UL/iMX8MQ, the QSPI controller > is updated to have TDH field in FLSHCR register. According to reference > manual, this TDH must be set to 1 when DDR_EN is set. Otherwise, the TX > DDR delay logic won't be enabled. This is actually an issue I have experienced myself. But in our case this behavior only happened on i.MX6ULL not on i.MX6UL. Either the QSPI controller hardware or the BootROM code changed when moving from UL to ULL. For details see: [1]. > > Another issue in DDR mode is the MCR register will be overwritten in > every read/write/erase operation. This causes DDR_EN been cleared while > TDH=1, then no clk2x output for TX data shift and all operations will > fail. The best way to fix all of these things (also the ones in the other patches) would be to fix them in Linux and port the driver from Linux to U-Boot. Actually I've already done most of the porting [2], but didn't have the time to finish it recently. It probably needs some rebasing and testing. Could you port your fixes to the Linux driver and submit them to linux-mtd? Thanks Frieder [1] https://community.nxp.com/thread/507260 [2] https://github.com/fschrempf/u-boot/commits/fsl_qspi_spimem_port > > Signed-off-by: Ye Li <ye.li@nxp.com> > --- > drivers/spi/fsl_qspi.c | 30 ++++++++++++++++-------------- > 1 file changed, 16 insertions(+), 14 deletions(-) > > diff --git a/drivers/spi/fsl_qspi.c b/drivers/spi/fsl_qspi.c > index 41abe19..8845986 100644 > --- a/drivers/spi/fsl_qspi.c > +++ b/drivers/spi/fsl_qspi.c > @@ -399,7 +399,7 @@ static inline void qspi_ahb_read(struct fsl_qspi_priv *priv, u8 *rxbuf, int len) > > qspi_write32(priv->flags, ®s->mcr, > QSPI_MCR_CLR_RXF_MASK | QSPI_MCR_CLR_TXF_MASK | > - QSPI_MCR_RESERVED_MASK | QSPI_MCR_END_CFD_LE); > + mcr_reg); > > rx_addr = (void *)(uintptr_t)(priv->cur_amba_base + priv->sf_addr); > /* Read out the data directly from the AHB buffer. */ > @@ -429,6 +429,14 @@ static void qspi_enable_ddr_mode(struct fsl_qspi_priv *priv) > reg |= BIT(29); > > qspi_write32(priv->flags, ®s->mcr, reg); > + > + /* Enable the TDH to 1 for some platforms like imx6ul, imx7d, etc > + * These two bits are reserved on other platforms > + */ > + reg = qspi_read32(priv->flags, ®s->flshcr); > + reg &= ~(BIT(17)); > + reg |= BIT(16); > + qspi_write32(priv->flags, ®s->flshcr, reg); > } > > /* > @@ -482,7 +490,7 @@ static void qspi_op_rdbank(struct fsl_qspi_priv *priv, u8 *rxbuf, u32 len) > mcr_reg = qspi_read32(priv->flags, ®s->mcr); > qspi_write32(priv->flags, ®s->mcr, > QSPI_MCR_CLR_RXF_MASK | QSPI_MCR_CLR_TXF_MASK | > - QSPI_MCR_RESERVED_MASK | QSPI_MCR_END_CFD_LE); > + mcr_reg); > qspi_write32(priv->flags, ®s->rbct, QSPI_RBCT_RXBRD_USEIPS); > > qspi_write32(priv->flags, ®s->sfar, priv->cur_amba_base); > @@ -527,7 +535,7 @@ static void qspi_op_rdid(struct fsl_qspi_priv *priv, u32 *rxbuf, u32 len) > mcr_reg = qspi_read32(priv->flags, ®s->mcr); > qspi_write32(priv->flags, ®s->mcr, > QSPI_MCR_CLR_RXF_MASK | QSPI_MCR_CLR_TXF_MASK | > - QSPI_MCR_RESERVED_MASK | QSPI_MCR_END_CFD_LE); > + mcr_reg); > qspi_write32(priv->flags, ®s->rbct, QSPI_RBCT_RXBRD_USEIPS); > > qspi_write32(priv->flags, ®s->sfar, priv->cur_amba_base); > @@ -573,7 +581,7 @@ static void qspi_op_read(struct fsl_qspi_priv *priv, u32 *rxbuf, u32 len) > mcr_reg = qspi_read32(priv->flags, ®s->mcr); > qspi_write32(priv->flags, ®s->mcr, > QSPI_MCR_CLR_RXF_MASK | QSPI_MCR_CLR_TXF_MASK | > - QSPI_MCR_RESERVED_MASK | QSPI_MCR_END_CFD_LE); > + mcr_reg); > qspi_write32(priv->flags, ®s->rbct, QSPI_RBCT_RXBRD_USEIPS); > > to_or_from = priv->sf_addr + priv->cur_amba_base; > @@ -625,7 +633,7 @@ static void qspi_op_write(struct fsl_qspi_priv *priv, u8 *txbuf, u32 len) > mcr_reg = qspi_read32(priv->flags, ®s->mcr); > qspi_write32(priv->flags, ®s->mcr, > QSPI_MCR_CLR_RXF_MASK | QSPI_MCR_CLR_TXF_MASK | > - QSPI_MCR_RESERVED_MASK | QSPI_MCR_END_CFD_LE); > + mcr_reg); > qspi_write32(priv->flags, ®s->rbct, QSPI_RBCT_RXBRD_USEIPS); > > status_reg = 0; > @@ -700,7 +708,7 @@ static void qspi_op_rdsr(struct fsl_qspi_priv *priv, void *rxbuf, u32 len) > mcr_reg = qspi_read32(priv->flags, ®s->mcr); > qspi_write32(priv->flags, ®s->mcr, > QSPI_MCR_CLR_RXF_MASK | QSPI_MCR_CLR_TXF_MASK | > - QSPI_MCR_RESERVED_MASK | QSPI_MCR_END_CFD_LE); > + mcr_reg); > qspi_write32(priv->flags, ®s->rbct, QSPI_RBCT_RXBRD_USEIPS); > > qspi_write32(priv->flags, ®s->sfar, priv->cur_amba_base); > @@ -737,7 +745,7 @@ static void qspi_op_erase(struct fsl_qspi_priv *priv) > mcr_reg = qspi_read32(priv->flags, ®s->mcr); > qspi_write32(priv->flags, ®s->mcr, > QSPI_MCR_CLR_RXF_MASK | QSPI_MCR_CLR_TXF_MASK | > - QSPI_MCR_RESERVED_MASK | QSPI_MCR_END_CFD_LE); > + mcr_reg); > qspi_write32(priv->flags, ®s->rbct, QSPI_RBCT_RXBRD_USEIPS); > > to_or_from = priv->sf_addr + priv->cur_amba_base; > @@ -900,15 +908,9 @@ static int fsl_qspi_probe(struct udevice *bus) > return ret; > } > > - mcr_val = qspi_read32(priv->flags, &priv->regs->mcr); > - > - /* Set endianness to LE for i.mx */ > - if (IS_ENABLED(CONFIG_MX6) || IS_ENABLED(CONFIG_MX7)) > - mcr_val = QSPI_MCR_END_CFD_LE; > - > qspi_write32(priv->flags, &priv->regs->mcr, > QSPI_MCR_RESERVED_MASK | QSPI_MCR_MDIS_MASK | > - (mcr_val & QSPI_MCR_END_CFD_MASK)); > + QSPI_MCR_END_CFD_LE); > > qspi_cfg_smpr(priv, ~(QSPI_SMPR_FSDLY_MASK | QSPI_SMPR_DDRSMP_MASK | > QSPI_SMPR_FSPHS_MASK | QSPI_SMPR_HSENA_MASK), 0); >
> -----Original Message----- > From: U-Boot <u-boot-bounces@lists.denx.de> On Behalf Of Schrempf Frieder > Sent: Wednesday, August 14, 2019 5:07 PM > To: Ye Li <ye.li@nxp.com>; jagan@amarulasolutions.com > Cc: Fabio Estevam <fabio.estevam@nxp.com>; u-boot@lists.denx.de; dl- > uboot-imx <uboot-imx@nxp.com> > Subject: [EXT] Re: [U-Boot] [PATCH 1/6] spi: fsl_qspi: Fix DDR mode setting for > latest iMX platforms > > Caution: EXT Email > > Hi Ye, > > On 14.08.19 12:08, Ye Li wrote: > > On latest iMX platforms like iMX7D/iMX6UL/iMX8MQ, the QSPI controller > > is updated to have TDH field in FLSHCR register. According to > > reference manual, this TDH must be set to 1 when DDR_EN is set. > > Otherwise, the TX DDR delay logic won't be enabled. > > This is actually an issue I have experienced myself. But in our case this > behavior only happened on i.MX6ULL not on i.MX6UL. Either the QSPI > controller hardware or the BootROM code changed when moving from UL to > ULL. For details see: [1]. > > > > > Another issue in DDR mode is the MCR register will be overwritten in > > every read/write/erase operation. This causes DDR_EN been cleared > > while TDH=1, then no clk2x output for TX data shift and all operations > > will fail. > > The best way to fix all of these things (also the ones in the other > patches) would be to fix them in Linux and port the driver from Linux to U- > Boot. Actually I've already done most of the porting [2], Hello Frieder, I had tested your porting and it was not functional on u-boot. I found that only erase, read up to TX/RX buf size is working or something like that. Also ip and AHB mode cannot be used at time in code. Previously only IP mode was used in u-boot, Since endianness across different QSPI-IP(ls1012, ls1043, ls1021 big endian), (ls1088,ls2088 little endian) is not consistent on various silicon's. I am not sure if Yogesh who worked with you on Linux porting gave you this information about endianness inconsistency. Please suggest way forward. How to correct this issue? Regards Ashish > time to finish it recently. It probably needs some rebasing and testing. > > Could you port your fixes to the Linux driver and submit them to linux-mtd? > > Thanks > Frieder > > [1] > https://eur01.safelinks.protection.outlook.com/?url=https%3A%2F%2Fcomm > unity.nxp.com%2Fthread%2F507260&data=02%7C01%7CAshish.Kumar > %40nxp.com%7C8882d5662295468a45b008d720abd98c%7C686ea1d3bc2b4c > 6fa92cd99c5c301635%7C0%7C0%7C637013794778063281&sdata=Za7LB > 6RyXAHszPfiEMLDb%2FvVNSTQJwxHFtiapmNi3Co%3D&reserved=0 > [2] > https://eur01.safelinks.protection.outlook.com/?url=https%3A%2F%2Fgithub > .com%2Ffschrempf%2Fu- > boot%2Fcommits%2Ffsl_qspi_spimem_port&data=02%7C01%7CAshish. > Kumar%40nxp.com%7C8882d5662295468a45b008d720abd98c%7C686ea1d3 > bc2b4c6fa92cd99c5c301635%7C0%7C0%7C637013794778063281&sdata > =kYDjrs2a9DdL8QLFPORfHsMSWvgUxhSTgNC3WLziu7Y%3D&reserved=0 > > > > > Signed-off-by: Ye Li <ye.li@nxp.com> > > --- > > drivers/spi/fsl_qspi.c | 30 ++++++++++++++++-------------- > > 1 file changed, 16 insertions(+), 14 deletions(-) > > > > diff --git a/drivers/spi/fsl_qspi.c b/drivers/spi/fsl_qspi.c index > > 41abe19..8845986 100644 > > --- a/drivers/spi/fsl_qspi.c > > +++ b/drivers/spi/fsl_qspi.c > > @@ -399,7 +399,7 @@ static inline void qspi_ahb_read(struct > > fsl_qspi_priv *priv, u8 *rxbuf, int len) > > > > qspi_write32(priv->flags, ®s->mcr, > > QSPI_MCR_CLR_RXF_MASK | QSPI_MCR_CLR_TXF_MASK | > > - QSPI_MCR_RESERVED_MASK | QSPI_MCR_END_CFD_LE); > > + mcr_reg); > > > > rx_addr = (void *)(uintptr_t)(priv->cur_amba_base + priv->sf_addr); > > /* Read out the data directly from the AHB buffer. */ @@ -429,6 > > +429,14 @@ static void qspi_enable_ddr_mode(struct fsl_qspi_priv *priv) > > reg |= BIT(29); > > > > qspi_write32(priv->flags, ®s->mcr, reg); > > + > > + /* Enable the TDH to 1 for some platforms like imx6ul, imx7d, etc > > + * These two bits are reserved on other platforms > > + */ > > + reg = qspi_read32(priv->flags, ®s->flshcr); > > + reg &= ~(BIT(17)); > > + reg |= BIT(16); > > + qspi_write32(priv->flags, ®s->flshcr, reg); > > } > > > > /* > > @@ -482,7 +490,7 @@ static void qspi_op_rdbank(struct fsl_qspi_priv > *priv, u8 *rxbuf, u32 len) > > mcr_reg = qspi_read32(priv->flags, ®s->mcr); > > qspi_write32(priv->flags, ®s->mcr, > > QSPI_MCR_CLR_RXF_MASK | QSPI_MCR_CLR_TXF_MASK | > > - QSPI_MCR_RESERVED_MASK | QSPI_MCR_END_CFD_LE); > > + mcr_reg); > > qspi_write32(priv->flags, ®s->rbct, QSPI_RBCT_RXBRD_USEIPS); > > > > qspi_write32(priv->flags, ®s->sfar, priv->cur_amba_base); @@ > > -527,7 +535,7 @@ static void qspi_op_rdid(struct fsl_qspi_priv *priv, u32 > *rxbuf, u32 len) > > mcr_reg = qspi_read32(priv->flags, ®s->mcr); > > qspi_write32(priv->flags, ®s->mcr, > > QSPI_MCR_CLR_RXF_MASK | QSPI_MCR_CLR_TXF_MASK | > > - QSPI_MCR_RESERVED_MASK | QSPI_MCR_END_CFD_LE); > > + mcr_reg); > > qspi_write32(priv->flags, ®s->rbct, QSPI_RBCT_RXBRD_USEIPS); > > > > qspi_write32(priv->flags, ®s->sfar, priv->cur_amba_base); @@ > > -573,7 +581,7 @@ static void qspi_op_read(struct fsl_qspi_priv *priv, u32 > *rxbuf, u32 len) > > mcr_reg = qspi_read32(priv->flags, ®s->mcr); > > qspi_write32(priv->flags, ®s->mcr, > > QSPI_MCR_CLR_RXF_MASK | QSPI_MCR_CLR_TXF_MASK | > > - QSPI_MCR_RESERVED_MASK | QSPI_MCR_END_CFD_LE); > > + mcr_reg); > > qspi_write32(priv->flags, ®s->rbct, QSPI_RBCT_RXBRD_USEIPS); > > > > to_or_from = priv->sf_addr + priv->cur_amba_base; @@ -625,7 > > +633,7 @@ static void qspi_op_write(struct fsl_qspi_priv *priv, u8 *txbuf, > u32 len) > > mcr_reg = qspi_read32(priv->flags, ®s->mcr); > > qspi_write32(priv->flags, ®s->mcr, > > QSPI_MCR_CLR_RXF_MASK | QSPI_MCR_CLR_TXF_MASK | > > - QSPI_MCR_RESERVED_MASK | QSPI_MCR_END_CFD_LE); > > + mcr_reg); > > qspi_write32(priv->flags, ®s->rbct, QSPI_RBCT_RXBRD_USEIPS); > > > > status_reg = 0; > > @@ -700,7 +708,7 @@ static void qspi_op_rdsr(struct fsl_qspi_priv *priv, > void *rxbuf, u32 len) > > mcr_reg = qspi_read32(priv->flags, ®s->mcr); > > qspi_write32(priv->flags, ®s->mcr, > > QSPI_MCR_CLR_RXF_MASK | QSPI_MCR_CLR_TXF_MASK | > > - QSPI_MCR_RESERVED_MASK | QSPI_MCR_END_CFD_LE); > > + mcr_reg); > > qspi_write32(priv->flags, ®s->rbct, QSPI_RBCT_RXBRD_USEIPS); > > > > qspi_write32(priv->flags, ®s->sfar, priv->cur_amba_base); @@ > > -737,7 +745,7 @@ static void qspi_op_erase(struct fsl_qspi_priv *priv) > > mcr_reg = qspi_read32(priv->flags, ®s->mcr); > > qspi_write32(priv->flags, ®s->mcr, > > QSPI_MCR_CLR_RXF_MASK | QSPI_MCR_CLR_TXF_MASK | > > - QSPI_MCR_RESERVED_MASK | QSPI_MCR_END_CFD_LE); > > + mcr_reg); > > qspi_write32(priv->flags, ®s->rbct, QSPI_RBCT_RXBRD_USEIPS); > > > > to_or_from = priv->sf_addr + priv->cur_amba_base; @@ -900,15 > > +908,9 @@ static int fsl_qspi_probe(struct udevice *bus) > > return ret; > > } > > > > - mcr_val = qspi_read32(priv->flags, &priv->regs->mcr); > > - > > - /* Set endianness to LE for i.mx */ > > - if (IS_ENABLED(CONFIG_MX6) || IS_ENABLED(CONFIG_MX7)) > > - mcr_val = QSPI_MCR_END_CFD_LE; > > - > > qspi_write32(priv->flags, &priv->regs->mcr, > > QSPI_MCR_RESERVED_MASK | QSPI_MCR_MDIS_MASK | > > - (mcr_val & QSPI_MCR_END_CFD_MASK)); > > + QSPI_MCR_END_CFD_LE); > > > > qspi_cfg_smpr(priv, ~(QSPI_SMPR_FSDLY_MASK | > QSPI_SMPR_DDRSMP_MASK | > > QSPI_SMPR_FSPHS_MASK | QSPI_SMPR_HSENA_MASK), 0); > > > _______________________________________________ > U-Boot mailing list > U-Boot@lists.denx.de > https://eur01.safelinks.protection.outlook.com/?url=https%3A%2F%2Flists.d > enx.de%2Flistinfo%2Fu- > boot&data=02%7C01%7CAshish.Kumar%40nxp.com%7C8882d56622954 > 68a45b008d720abd98c%7C686ea1d3bc2b4c6fa92cd99c5c301635%7C0%7C0 > %7C637013794778063281&sdata=l1Pb4yEQABBx5ave4Lb7jGSVRJ1TG68 > KMkl7WpSSwRc%3D&reserved=0
Hi Ashish, On 14.08.19 14:02, Ashish Kumar wrote: > > >> -----Original Message----- >> From: U-Boot <u-boot-bounces@lists.denx.de> On Behalf Of Schrempf Frieder >> Sent: Wednesday, August 14, 2019 5:07 PM >> To: Ye Li <ye.li@nxp.com>; jagan@amarulasolutions.com >> Cc: Fabio Estevam <fabio.estevam@nxp.com>; u-boot@lists.denx.de; dl- >> uboot-imx <uboot-imx@nxp.com> >> Subject: [EXT] Re: [U-Boot] [PATCH 1/6] spi: fsl_qspi: Fix DDR mode setting for >> latest iMX platforms >> >> Caution: EXT Email >> >> Hi Ye, >> >> On 14.08.19 12:08, Ye Li wrote: >>> On latest iMX platforms like iMX7D/iMX6UL/iMX8MQ, the QSPI controller >>> is updated to have TDH field in FLSHCR register. According to >>> reference manual, this TDH must be set to 1 when DDR_EN is set. >>> Otherwise, the TX DDR delay logic won't be enabled. >> >> This is actually an issue I have experienced myself. But in our case this >> behavior only happened on i.MX6ULL not on i.MX6UL. Either the QSPI >> controller hardware or the BootROM code changed when moving from UL to >> ULL. For details see: [1]. >> >>> >>> Another issue in DDR mode is the MCR register will be overwritten in >>> every read/write/erase operation. This causes DDR_EN been cleared >>> while TDH=1, then no clk2x output for TX data shift and all operations >>> will fail. >> >> The best way to fix all of these things (also the ones in the other >> patches) would be to fix them in Linux and port the driver from Linux to U- >> Boot. Actually I've already done most of the porting [2], > Hello Frieder, > > I had tested your porting and it was not functional on u-boot. > I found that only erase, read up to TX/RX buf size is working or something like that. > Also ip and AHB mode cannot be used at time in code. Previously only IP mode was used in u-boot, > Since endianness across different QSPI-IP(ls1012, ls1043, ls1021 big endian), (ls1088,ls2088 little endian) is not consistent on various silicon's. I am not sure if Yogesh who worked with you on Linux porting gave you this information about endianness inconsistency. Ok, thanks for your feedback. The endianness for the different SoCs can be handled by the device data. > Please suggest way forward. How to correct this issue? > > Regards > Ashish > >> time to finish it recently. It probably needs some rebasing and testing. >> >> Could you port your fixes to the Linux driver and submit them to linux-mtd? >> >> Thanks >> Frieder >> >> [1] >> https://eur01.safelinks.protection.outlook.com/?url=https%3A%2F%2Fcomm >> unity.nxp.com%2Fthread%2F507260&data=02%7C01%7CAshish.Kumar >> %40nxp.com%7C8882d5662295468a45b008d720abd98c%7C686ea1d3bc2b4c >> 6fa92cd99c5c301635%7C0%7C0%7C637013794778063281&sdata=Za7LB >> 6RyXAHszPfiEMLDb%2FvVNSTQJwxHFtiapmNi3Co%3D&reserved=0 >> [2] >> https://eur01.safelinks.protection.outlook.com/?url=https%3A%2F%2Fgithub >> .com%2Ffschrempf%2Fu- >> boot%2Fcommits%2Ffsl_qspi_spimem_port&data=02%7C01%7CAshish. >> Kumar%40nxp.com%7C8882d5662295468a45b008d720abd98c%7C686ea1d3 >> bc2b4c6fa92cd99c5c301635%7C0%7C0%7C637013794778063281&sdata >> =kYDjrs2a9DdL8QLFPORfHsMSWvgUxhSTgNC3WLziu7Y%3D&reserved=0 >> >>> >>> Signed-off-by: Ye Li <ye.li@nxp.com> >>> --- >>> drivers/spi/fsl_qspi.c | 30 ++++++++++++++++-------------- >>> 1 file changed, 16 insertions(+), 14 deletions(-) >>> >>> diff --git a/drivers/spi/fsl_qspi.c b/drivers/spi/fsl_qspi.c index >>> 41abe19..8845986 100644 >>> --- a/drivers/spi/fsl_qspi.c >>> +++ b/drivers/spi/fsl_qspi.c >>> @@ -399,7 +399,7 @@ static inline void qspi_ahb_read(struct >>> fsl_qspi_priv *priv, u8 *rxbuf, int len) >>> >>> qspi_write32(priv->flags, ®s->mcr, >>> QSPI_MCR_CLR_RXF_MASK | QSPI_MCR_CLR_TXF_MASK | >>> - QSPI_MCR_RESERVED_MASK | QSPI_MCR_END_CFD_LE); >>> + mcr_reg); >>> >>> rx_addr = (void *)(uintptr_t)(priv->cur_amba_base + priv->sf_addr); >>> /* Read out the data directly from the AHB buffer. */ @@ -429,6 >>> +429,14 @@ static void qspi_enable_ddr_mode(struct fsl_qspi_priv *priv) >>> reg |= BIT(29); >>> >>> qspi_write32(priv->flags, ®s->mcr, reg); >>> + >>> + /* Enable the TDH to 1 for some platforms like imx6ul, imx7d, etc >>> + * These two bits are reserved on other platforms >>> + */ >>> + reg = qspi_read32(priv->flags, ®s->flshcr); >>> + reg &= ~(BIT(17)); >>> + reg |= BIT(16); >>> + qspi_write32(priv->flags, ®s->flshcr, reg); >>> } >>> >>> /* >>> @@ -482,7 +490,7 @@ static void qspi_op_rdbank(struct fsl_qspi_priv >> *priv, u8 *rxbuf, u32 len) >>> mcr_reg = qspi_read32(priv->flags, ®s->mcr); >>> qspi_write32(priv->flags, ®s->mcr, >>> QSPI_MCR_CLR_RXF_MASK | QSPI_MCR_CLR_TXF_MASK | >>> - QSPI_MCR_RESERVED_MASK | QSPI_MCR_END_CFD_LE); >>> + mcr_reg); >>> qspi_write32(priv->flags, ®s->rbct, QSPI_RBCT_RXBRD_USEIPS); >>> >>> qspi_write32(priv->flags, ®s->sfar, priv->cur_amba_base); @@ >>> -527,7 +535,7 @@ static void qspi_op_rdid(struct fsl_qspi_priv *priv, u32 >> *rxbuf, u32 len) >>> mcr_reg = qspi_read32(priv->flags, ®s->mcr); >>> qspi_write32(priv->flags, ®s->mcr, >>> QSPI_MCR_CLR_RXF_MASK | QSPI_MCR_CLR_TXF_MASK | >>> - QSPI_MCR_RESERVED_MASK | QSPI_MCR_END_CFD_LE); >>> + mcr_reg); >>> qspi_write32(priv->flags, ®s->rbct, QSPI_RBCT_RXBRD_USEIPS); >>> >>> qspi_write32(priv->flags, ®s->sfar, priv->cur_amba_base); @@ >>> -573,7 +581,7 @@ static void qspi_op_read(struct fsl_qspi_priv *priv, u32 >> *rxbuf, u32 len) >>> mcr_reg = qspi_read32(priv->flags, ®s->mcr); >>> qspi_write32(priv->flags, ®s->mcr, >>> QSPI_MCR_CLR_RXF_MASK | QSPI_MCR_CLR_TXF_MASK | >>> - QSPI_MCR_RESERVED_MASK | QSPI_MCR_END_CFD_LE); >>> + mcr_reg); >>> qspi_write32(priv->flags, ®s->rbct, QSPI_RBCT_RXBRD_USEIPS); >>> >>> to_or_from = priv->sf_addr + priv->cur_amba_base; @@ -625,7 >>> +633,7 @@ static void qspi_op_write(struct fsl_qspi_priv *priv, u8 *txbuf, >> u32 len) >>> mcr_reg = qspi_read32(priv->flags, ®s->mcr); >>> qspi_write32(priv->flags, ®s->mcr, >>> QSPI_MCR_CLR_RXF_MASK | QSPI_MCR_CLR_TXF_MASK | >>> - QSPI_MCR_RESERVED_MASK | QSPI_MCR_END_CFD_LE); >>> + mcr_reg); >>> qspi_write32(priv->flags, ®s->rbct, QSPI_RBCT_RXBRD_USEIPS); >>> >>> status_reg = 0; >>> @@ -700,7 +708,7 @@ static void qspi_op_rdsr(struct fsl_qspi_priv *priv, >> void *rxbuf, u32 len) >>> mcr_reg = qspi_read32(priv->flags, ®s->mcr); >>> qspi_write32(priv->flags, ®s->mcr, >>> QSPI_MCR_CLR_RXF_MASK | QSPI_MCR_CLR_TXF_MASK | >>> - QSPI_MCR_RESERVED_MASK | QSPI_MCR_END_CFD_LE); >>> + mcr_reg); >>> qspi_write32(priv->flags, ®s->rbct, QSPI_RBCT_RXBRD_USEIPS); >>> >>> qspi_write32(priv->flags, ®s->sfar, priv->cur_amba_base); @@ >>> -737,7 +745,7 @@ static void qspi_op_erase(struct fsl_qspi_priv *priv) >>> mcr_reg = qspi_read32(priv->flags, ®s->mcr); >>> qspi_write32(priv->flags, ®s->mcr, >>> QSPI_MCR_CLR_RXF_MASK | QSPI_MCR_CLR_TXF_MASK | >>> - QSPI_MCR_RESERVED_MASK | QSPI_MCR_END_CFD_LE); >>> + mcr_reg); >>> qspi_write32(priv->flags, ®s->rbct, QSPI_RBCT_RXBRD_USEIPS); >>> >>> to_or_from = priv->sf_addr + priv->cur_amba_base; @@ -900,15 >>> +908,9 @@ static int fsl_qspi_probe(struct udevice *bus) >>> return ret; >>> } >>> >>> - mcr_val = qspi_read32(priv->flags, &priv->regs->mcr); >>> - >>> - /* Set endianness to LE for i.mx */ >>> - if (IS_ENABLED(CONFIG_MX6) || IS_ENABLED(CONFIG_MX7)) >>> - mcr_val = QSPI_MCR_END_CFD_LE; >>> - >>> qspi_write32(priv->flags, &priv->regs->mcr, >>> QSPI_MCR_RESERVED_MASK | QSPI_MCR_MDIS_MASK | >>> - (mcr_val & QSPI_MCR_END_CFD_MASK)); >>> + QSPI_MCR_END_CFD_LE); >>> >>> qspi_cfg_smpr(priv, ~(QSPI_SMPR_FSDLY_MASK | >> QSPI_SMPR_DDRSMP_MASK | >>> QSPI_SMPR_FSPHS_MASK | QSPI_SMPR_HSENA_MASK), 0); >>> >> _______________________________________________ >> U-Boot mailing list >> U-Boot@lists.denx.de >> https://eur01.safelinks.protection.outlook.com/?url=https%3A%2F%2Flists.d >> enx.de%2Flistinfo%2Fu- >> boot&data=02%7C01%7CAshish.Kumar%40nxp.com%7C8882d56622954 >> 68a45b008d720abd98c%7C686ea1d3bc2b4c6fa92cd99c5c301635%7C0%7C0 >> %7C637013794778063281&sdata=l1Pb4yEQABBx5ave4Lb7jGSVRJ1TG68 >> KMkl7WpSSwRc%3D&reserved=0
Sorry, I hit the "Send" button too early ;) On 14.08.19 14:07, Frieder Schrempf wrote: > Hi Ashish, > > On 14.08.19 14:02, Ashish Kumar wrote: >> >> >>> -----Original Message----- >>> From: U-Boot <u-boot-bounces@lists.denx.de> On Behalf Of Schrempf >>> Frieder >>> Sent: Wednesday, August 14, 2019 5:07 PM >>> To: Ye Li <ye.li@nxp.com>; jagan@amarulasolutions.com >>> Cc: Fabio Estevam <fabio.estevam@nxp.com>; u-boot@lists.denx.de; dl- >>> uboot-imx <uboot-imx@nxp.com> >>> Subject: [EXT] Re: [U-Boot] [PATCH 1/6] spi: fsl_qspi: Fix DDR mode >>> setting for >>> latest iMX platforms >>> >>> Caution: EXT Email >>> >>> Hi Ye, >>> >>> On 14.08.19 12:08, Ye Li wrote: >>>> On latest iMX platforms like iMX7D/iMX6UL/iMX8MQ, the QSPI controller >>>> is updated to have TDH field in FLSHCR register. According to >>>> reference manual, this TDH must be set to 1 when DDR_EN is set. >>>> Otherwise, the TX DDR delay logic won't be enabled. >>> >>> This is actually an issue I have experienced myself. But in our case >>> this >>> behavior only happened on i.MX6ULL not on i.MX6UL. Either the QSPI >>> controller hardware or the BootROM code changed when moving from UL to >>> ULL. For details see: [1]. >>> >>>> >>>> Another issue in DDR mode is the MCR register will be overwritten in >>>> every read/write/erase operation. This causes DDR_EN been cleared >>>> while TDH=1, then no clk2x output for TX data shift and all operations >>>> will fail. >>> >>> The best way to fix all of these things (also the ones in the other >>> patches) would be to fix them in Linux and port the driver from Linux >>> to U- >>> Boot. Actually I've already done most of the porting [2], >> Hello Frieder, >> >> I had tested your porting and it was not functional on u-boot. >> I found that only erase, read up to TX/RX buf size is working or >> something like that. >> Also ip and AHB mode cannot be used at time in code. Previously only >> IP mode was used in u-boot, >> Since endianness across different QSPI-IP(ls1012, ls1043, ls1021 big >> endian), (ls1088,ls2088 little endian) is not consistent on various >> silicon's. I am not sure if Yogesh who worked with you on Linux >> porting gave you this information about endianness inconsistency. > > Ok, thanks for your feedback. The endianness for the different SoCs can > be handled by the device data. Does this work correctly in Linux, or does the Linux driver need fixes? > >> Please suggest way forward. How to correct this issue? The first thigh would be to make sure the Linux driver works for all platforms and then do the porting to U-Boot. I will be out of office for 10 days. After that I can try to work on this, but I need support and testing for other platforms. I only have i.MX6UL/ULL. Thanks, Frieder >> >> Regards >> Ashish >>> time to finish it recently. It probably needs some rebasing and testing. >>> >>> Could you port your fixes to the Linux driver and submit them to >>> linux-mtd? >>> >>> Thanks >>> Frieder >>> >>> [1] >>> https://eur01.safelinks.protection.outlook.com/?url=https%3A%2F%2Fcomm >>> unity.nxp.com%2Fthread%2F507260&data=02%7C01%7CAshish.Kumar >>> %40nxp.com%7C8882d5662295468a45b008d720abd98c%7C686ea1d3bc2b4c >>> 6fa92cd99c5c301635%7C0%7C0%7C637013794778063281&sdata=Za7LB >>> 6RyXAHszPfiEMLDb%2FvVNSTQJwxHFtiapmNi3Co%3D&reserved=0 >>> [2] >>> https://eur01.safelinks.protection.outlook.com/?url=https%3A%2F%2Fgithub >>> .com%2Ffschrempf%2Fu- >>> boot%2Fcommits%2Ffsl_qspi_spimem_port&data=02%7C01%7CAshish. >>> Kumar%40nxp.com%7C8882d5662295468a45b008d720abd98c%7C686ea1d3 >>> bc2b4c6fa92cd99c5c301635%7C0%7C0%7C637013794778063281&sdata >>> =kYDjrs2a9DdL8QLFPORfHsMSWvgUxhSTgNC3WLziu7Y%3D&reserved=0 >>> >>>> >>>> Signed-off-by: Ye Li <ye.li@nxp.com> >>>> --- >>>> Â Â drivers/spi/fsl_qspi.c | 30 ++++++++++++++++-------------- >>>> Â Â 1 file changed, 16 insertions(+), 14 deletions(-) >>>> >>>> diff --git a/drivers/spi/fsl_qspi.c b/drivers/spi/fsl_qspi.c index >>>> 41abe19..8845986 100644 >>>> --- a/drivers/spi/fsl_qspi.c >>>> +++ b/drivers/spi/fsl_qspi.c >>>> @@ -399,7 +399,7 @@ static inline void qspi_ahb_read(struct >>>> fsl_qspi_priv *priv, u8 *rxbuf, int len) >>>> >>>> Â Â Â Â Â Â qspi_write32(priv->flags, ®s->mcr, >>>> Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â QSPI_MCR_CLR_RXF_MASK | QSPI_MCR_CLR_TXF_MASK | >>>> -Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â QSPI_MCR_RESERVED_MASK | QSPI_MCR_END_CFD_LE); >>>> +Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â mcr_reg); >>>> >>>> Â Â Â Â Â Â rx_addr = (void *)(uintptr_t)(priv->cur_amba_base + >>>> priv->sf_addr); >>>> Â Â Â Â Â Â /* Read out the data directly from the AHB buffer. */ @@ -429,6 >>>> +429,14 @@ static void qspi_enable_ddr_mode(struct fsl_qspi_priv *priv) >>>> Â Â Â Â Â Â reg |= BIT(29); >>>> >>>> Â Â Â Â Â Â qspi_write32(priv->flags, ®s->mcr, reg); >>>> + >>>> +Â Â Â Â /* Enable the TDH to 1 for some platforms like imx6ul, imx7d, etc >>>> +Â Â Â Â Â * These two bits are reserved on other platforms >>>> +Â Â Â Â Â */ >>>> +Â Â Â Â reg = qspi_read32(priv->flags, ®s->flshcr); >>>> +Â Â Â Â reg &= ~(BIT(17)); >>>> +Â Â Â Â reg |= BIT(16); >>>> +Â Â Â Â qspi_write32(priv->flags, ®s->flshcr, reg); >>>> Â Â } >>>> >>>> Â Â /* >>>> @@ -482,7 +490,7 @@ static void qspi_op_rdbank(struct fsl_qspi_priv >>> *priv, u8 *rxbuf, u32 len) >>>> Â Â Â Â Â Â mcr_reg = qspi_read32(priv->flags, ®s->mcr); >>>> Â Â Â Â Â Â qspi_write32(priv->flags, ®s->mcr, >>>> Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â QSPI_MCR_CLR_RXF_MASK | QSPI_MCR_CLR_TXF_MASK | >>>> -Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â QSPI_MCR_RESERVED_MASK | QSPI_MCR_END_CFD_LE); >>>> +Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â mcr_reg); >>>> Â Â Â Â Â Â qspi_write32(priv->flags, ®s->rbct, QSPI_RBCT_RXBRD_USEIPS); >>>> >>>> Â Â Â Â Â Â qspi_write32(priv->flags, ®s->sfar, priv->cur_amba_base); @@ >>>> -527,7 +535,7 @@ static void qspi_op_rdid(struct fsl_qspi_priv >>>> *priv, u32 >>> *rxbuf, u32 len) >>>> Â Â Â Â Â Â mcr_reg = qspi_read32(priv->flags, ®s->mcr); >>>> Â Â Â Â Â Â qspi_write32(priv->flags, ®s->mcr, >>>> Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â QSPI_MCR_CLR_RXF_MASK | QSPI_MCR_CLR_TXF_MASK | >>>> -Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â QSPI_MCR_RESERVED_MASK | QSPI_MCR_END_CFD_LE); >>>> +Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â mcr_reg); >>>> Â Â Â Â Â Â qspi_write32(priv->flags, ®s->rbct, QSPI_RBCT_RXBRD_USEIPS); >>>> >>>> Â Â Â Â Â Â qspi_write32(priv->flags, ®s->sfar, priv->cur_amba_base); @@ >>>> -573,7 +581,7 @@ static void qspi_op_read(struct fsl_qspi_priv >>>> *priv, u32 >>> *rxbuf, u32 len) >>>> Â Â Â Â Â Â mcr_reg = qspi_read32(priv->flags, ®s->mcr); >>>> Â Â Â Â Â Â qspi_write32(priv->flags, ®s->mcr, >>>> Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â QSPI_MCR_CLR_RXF_MASK | QSPI_MCR_CLR_TXF_MASK | >>>> -Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â QSPI_MCR_RESERVED_MASK | QSPI_MCR_END_CFD_LE); >>>> +Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â mcr_reg); >>>> Â Â Â Â Â Â qspi_write32(priv->flags, ®s->rbct, QSPI_RBCT_RXBRD_USEIPS); >>>> >>>> Â Â Â Â Â Â to_or_from = priv->sf_addr + priv->cur_amba_base; @@ -625,7 >>>> +633,7 @@ static void qspi_op_write(struct fsl_qspi_priv *priv, u8 >>>> *txbuf, >>> u32 len) >>>> Â Â Â Â Â Â mcr_reg = qspi_read32(priv->flags, ®s->mcr); >>>> Â Â Â Â Â Â qspi_write32(priv->flags, ®s->mcr, >>>> Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â QSPI_MCR_CLR_RXF_MASK | QSPI_MCR_CLR_TXF_MASK | >>>> -Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â QSPI_MCR_RESERVED_MASK | QSPI_MCR_END_CFD_LE); >>>> +Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â mcr_reg); >>>> Â Â Â Â Â Â qspi_write32(priv->flags, ®s->rbct, QSPI_RBCT_RXBRD_USEIPS); >>>> >>>> Â Â Â Â Â Â status_reg = 0; >>>> @@ -700,7 +708,7 @@ static void qspi_op_rdsr(struct fsl_qspi_priv >>>> *priv, >>> void *rxbuf, u32 len) >>>> Â Â Â Â Â Â mcr_reg = qspi_read32(priv->flags, ®s->mcr); >>>> Â Â Â Â Â Â qspi_write32(priv->flags, ®s->mcr, >>>> Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â QSPI_MCR_CLR_RXF_MASK | QSPI_MCR_CLR_TXF_MASK | >>>> -Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â QSPI_MCR_RESERVED_MASK | QSPI_MCR_END_CFD_LE); >>>> +Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â mcr_reg); >>>> Â Â Â Â Â Â qspi_write32(priv->flags, ®s->rbct, QSPI_RBCT_RXBRD_USEIPS); >>>> >>>> Â Â Â Â Â Â qspi_write32(priv->flags, ®s->sfar, priv->cur_amba_base); @@ >>>> -737,7 +745,7 @@ static void qspi_op_erase(struct fsl_qspi_priv *priv) >>>> Â Â Â Â Â Â mcr_reg = qspi_read32(priv->flags, ®s->mcr); >>>> Â Â Â Â Â Â qspi_write32(priv->flags, ®s->mcr, >>>> Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â QSPI_MCR_CLR_RXF_MASK | QSPI_MCR_CLR_TXF_MASK | >>>> -Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â QSPI_MCR_RESERVED_MASK | QSPI_MCR_END_CFD_LE); >>>> +Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â mcr_reg); >>>> Â Â Â Â Â Â qspi_write32(priv->flags, ®s->rbct, QSPI_RBCT_RXBRD_USEIPS); >>>> >>>> Â Â Â Â Â Â to_or_from = priv->sf_addr + priv->cur_amba_base; @@ -900,15 >>>> +908,9 @@ static int fsl_qspi_probe(struct udevice *bus) >>>> Â Â Â Â Â Â Â Â Â Â Â Â Â Â return ret; >>>> Â Â Â Â Â Â } >>>> >>>> -Â Â Â Â mcr_val = qspi_read32(priv->flags, &priv->regs->mcr); >>>> - >>>> -Â Â Â Â /* Set endianness to LE for i.mx */ >>>> -Â Â Â Â if (IS_ENABLED(CONFIG_MX6) || IS_ENABLED(CONFIG_MX7)) >>>> -Â Â Â Â Â Â Â Â Â Â Â Â mcr_val = QSPI_MCR_END_CFD_LE; >>>> - >>>> Â Â Â Â Â Â qspi_write32(priv->flags, &priv->regs->mcr, >>>> Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â QSPI_MCR_RESERVED_MASK | QSPI_MCR_MDIS_MASK | >>>> -Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â (mcr_val & QSPI_MCR_END_CFD_MASK)); >>>> +Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â QSPI_MCR_END_CFD_LE); >>>> >>>> Â Â Â Â Â Â qspi_cfg_smpr(priv, ~(QSPI_SMPR_FSDLY_MASK | >>> QSPI_SMPR_DDRSMP_MASK | >>>> Â Â Â Â Â Â Â Â Â Â Â Â Â Â QSPI_SMPR_FSPHS_MASK | QSPI_SMPR_HSENA_MASK), 0); >>>> >>> _______________________________________________ >>> U-Boot mailing list >>> U-Boot@lists.denx.de >>> https://eur01.safelinks.protection.outlook.com/?url=https%3A%2F%2Flists.d >>> >>> enx.de%2Flistinfo%2Fu- >>> boot&data=02%7C01%7CAshish.Kumar%40nxp.com%7C8882d56622954 >>> 68a45b008d720abd98c%7C686ea1d3bc2b4c6fa92cd99c5c301635%7C0%7C0 >>> %7C637013794778063281&sdata=l1Pb4yEQABBx5ave4Lb7jGSVRJ1TG68 >>> KMkl7WpSSwRc%3D&reserved=0
> -----Original Message----- > From: Schrempf Frieder <frieder.schrempf@kontron.de> > Sent: Wednesday, August 14, 2019 5:41 PM > To: Ashish Kumar <ashish.kumar@nxp.com>; Ye Li <ye.li@nxp.com>; > jagan@amarulasolutions.com > Cc: Fabio Estevam <fabio.estevam@nxp.com>; u-boot@lists.denx.de; dl- > uboot-imx <uboot-imx@nxp.com> > Subject: Re: [EXT] Re: [U-Boot] [PATCH 1/6] spi: fsl_qspi: Fix DDR mode setting > for latest iMX platforms > > Caution: EXT Email > > Sorry, I hit the "Send" button too early ;) > > On 14.08.19 14:07, Frieder Schrempf wrote: > > Hi Ashish, > > > > On 14.08.19 14:02, Ashish Kumar wrote: > >> > >> > >>> -----Original Message----- > >>> From: U-Boot <u-boot-bounces@lists.denx.de> On Behalf Of Schrempf > >>> Frieder > >>> Sent: Wednesday, August 14, 2019 5:07 PM > >>> To: Ye Li <ye.li@nxp.com>; jagan@amarulasolutions.com > >>> Cc: Fabio Estevam <fabio.estevam@nxp.com>; u-boot@lists.denx.de; dl- > >>> uboot-imx <uboot-imx@nxp.com> > >>> Subject: [EXT] Re: [U-Boot] [PATCH 1/6] spi: fsl_qspi: Fix DDR mode > >>> setting for latest iMX platforms > >>> > >>> Caution: EXT Email > >>> > >>> Hi Ye, > >>> > >>> On 14.08.19 12:08, Ye Li wrote: > >>>> On latest iMX platforms like iMX7D/iMX6UL/iMX8MQ, the QSPI > >>>> controller is updated to have TDH field in FLSHCR register. > >>>> According to reference manual, this TDH must be set to 1 when DDR_EN > is set. > >>>> Otherwise, the TX DDR delay logic won't be enabled. > >>> > >>> This is actually an issue I have experienced myself. But in our case > >>> this behavior only happened on i.MX6ULL not on i.MX6UL. Either the > >>> QSPI controller hardware or the BootROM code changed when moving > >>> from UL to ULL. For details see: [1]. > >>> > >>>> > >>>> Another issue in DDR mode is the MCR register will be overwritten > >>>> in every read/write/erase operation. This causes DDR_EN been > >>>> cleared while TDH=1, then no clk2x output for TX data shift and all > >>>> operations will fail. > >>> > >>> The best way to fix all of these things (also the ones in the other > >>> patches) would be to fix them in Linux and port the driver from > >>> Linux to U- Boot. Actually I've already done most of the porting > >>> [2], > >> Hello Frieder, > >> > >> I had tested your porting and it was not functional on u-boot. > >> I found that only erase, read up to TX/RX buf size is working or > >> something like that. > >> Also ip and AHB mode cannot be used at time in code. Previously only > >> IP mode was used in u-boot, Since endianness across different > >> QSPI-IP(ls1012, ls1043, ls1021 big endian), (ls1088,ls2088 little > >> endian) is not consistent on various silicon's. I am not sure if > >> Yogesh who worked with you on Linux porting gave you this information > >> about endianness inconsistency. > > > > Ok, thanks for your feedback. The endianness for the different SoCs > > can be handled by the device data. > > Does this work correctly in Linux, or does the Linux driver need fixes? > > > > >> Please suggest way forward. How to correct this issue? > > The first thigh would be to make sure the Linux driver works for all platforms > and then do the porting to U-Boot. I will be out of office for > 10 days. After that I can try to work on this, but I need support and testing for > other platforms. I only have i.MX6UL/ULL. Hi Frieder, Surely, I can help with all LS platforms. In the mean while I will try to debug it further on u-boot. But I believe we can focus on u-boot straight away by disabling AHB for now. Since AHB is taken care with help of another file ./arch/arm/cpu/armv8/fsl-layerscape/soc.c function qspi_ahb_init(), Which is facilitated via md command. Regards Ashish > > Thanks, > Frieder > > >> > >> Regards > >> Ashish > >>> time to finish it recently. It probably needs some rebasing and testing. > >>> > >>> Could you port your fixes to the Linux driver and submit them to > >>> linux-mtd? > >>> > >>> Thanks > >>> Frieder > >>> > >>> [1] > >>> https://comm > >>> > unity.nxp.com%2Fthread%2F507260&data=02%7C01%7CAshish.Kumar > >>> > %40nxp.com%7C8882d5662295468a45b008d720abd98c%7C686ea1d3bc2b4c > >>> > 6fa92cd99c5c301635%7C0%7C0%7C637013794778063281&sdata=Za7LB > >>> 6RyXAHszPfiEMLDb%2FvVNSTQJwxHFtiapmNi3Co%3D&reserved=0 > >>> [2] > >>> https://github > >>> .com%2Ffschrempf%2Fu- > >>> > boot%2Fcommits%2Ffsl_qspi_spimem_port&data=02%7C01%7CAshish. > >>> > Kumar%40nxp.com%7C8882d5662295468a45b008d720abd98c%7C686ea1d3 > >>> > bc2b4c6fa92cd99c5c301635%7C0%7C0%7C637013794778063281&sdata > >>> > =kYDjrs2a9DdL8QLFPORfHsMSWvgUxhSTgNC3WLziu7Y%3D&reserved=0 > >>> > >>>> > >>>> Signed-off-by: Ye Li <ye.li@nxp.com> > >>>> --- > >>>> drivers/spi/fsl_qspi.c | 30 ++++++++++++++++-------------- > >>>> 1 file changed, 16 insertions(+), 14 deletions(-) > >>>> > >>>> diff --git a/drivers/spi/fsl_qspi.c b/drivers/spi/fsl_qspi.c index > >>>> 41abe19..8845986 100644 > >>>> --- a/drivers/spi/fsl_qspi.c > >>>> +++ b/drivers/spi/fsl_qspi.c > >>>> @@ -399,7 +399,7 @@ static inline void qspi_ahb_read(struct > >>>> fsl_qspi_priv *priv, u8 *rxbuf, int len) > >>>> > >>>> qspi_write32(priv->flags, ®s->mcr, > >>>> QSPI_MCR_CLR_RXF_MASK | QSPI_MCR_CLR_TXF_MASK | > >>>> - QSPI_MCR_RESERVED_MASK | QSPI_MCR_END_CFD_LE); > >>>> + mcr_reg); > >>>> > >>>> rx_addr = (void *)(uintptr_t)(priv->cur_amba_base + > >>>> priv->sf_addr); > >>>> /* Read out the data directly from the AHB buffer. */ @@ > >>>> -429,6 > >>>> +429,14 @@ static void qspi_enable_ddr_mode(struct fsl_qspi_priv > >>>> +*priv) > >>>> reg |= BIT(29); > >>>> > >>>> qspi_write32(priv->flags, ®s->mcr, reg); > >>>> + > >>>> + /* Enable the TDH to 1 for some platforms like imx6ul, imx7d, etc > >>>> + * These two bits are reserved on other platforms > >>>> + */ > >>>> + reg = qspi_read32(priv->flags, ®s->flshcr); > >>>> + reg &= ~(BIT(17)); > >>>> + reg |= BIT(16); > >>>> + qspi_write32(priv->flags, ®s->flshcr, reg); > >>>> } > >>>> > >>>> /* > >>>> @@ -482,7 +490,7 @@ static void qspi_op_rdbank(struct fsl_qspi_priv > >>> *priv, u8 *rxbuf, u32 len) > >>>> mcr_reg = qspi_read32(priv->flags, ®s->mcr); > >>>> qspi_write32(priv->flags, ®s->mcr, > >>>> QSPI_MCR_CLR_RXF_MASK | QSPI_MCR_CLR_TXF_MASK | > >>>> - QSPI_MCR_RESERVED_MASK | QSPI_MCR_END_CFD_LE); > >>>> + mcr_reg); > >>>> qspi_write32(priv->flags, ®s->rbct, > >>>> QSPI_RBCT_RXBRD_USEIPS); > >>>> > >>>> qspi_write32(priv->flags, ®s->sfar, priv->cur_amba_base); > >>>> @@ > >>>> -527,7 +535,7 @@ static void qspi_op_rdid(struct fsl_qspi_priv > >>>> *priv, u32 > >>> *rxbuf, u32 len) > >>>> mcr_reg = qspi_read32(priv->flags, ®s->mcr); > >>>> qspi_write32(priv->flags, ®s->mcr, > >>>> QSPI_MCR_CLR_RXF_MASK | QSPI_MCR_CLR_TXF_MASK | > >>>> - QSPI_MCR_RESERVED_MASK | QSPI_MCR_END_CFD_LE); > >>>> + mcr_reg); > >>>> qspi_write32(priv->flags, ®s->rbct, > >>>> QSPI_RBCT_RXBRD_USEIPS); > >>>> > >>>> qspi_write32(priv->flags, ®s->sfar, priv->cur_amba_base); > >>>> @@ > >>>> -573,7 +581,7 @@ static void qspi_op_read(struct fsl_qspi_priv > >>>> *priv, u32 > >>> *rxbuf, u32 len) > >>>> mcr_reg = qspi_read32(priv->flags, ®s->mcr); > >>>> qspi_write32(priv->flags, ®s->mcr, > >>>> QSPI_MCR_CLR_RXF_MASK | QSPI_MCR_CLR_TXF_MASK | > >>>> - QSPI_MCR_RESERVED_MASK | QSPI_MCR_END_CFD_LE); > >>>> + mcr_reg); > >>>> qspi_write32(priv->flags, ®s->rbct, > >>>> QSPI_RBCT_RXBRD_USEIPS); > >>>> > >>>> to_or_from = priv->sf_addr + priv->cur_amba_base; @@ -625,7 > >>>> +633,7 @@ static void qspi_op_write(struct fsl_qspi_priv *priv, u8 > >>>> *txbuf, > >>> u32 len) > >>>> mcr_reg = qspi_read32(priv->flags, ®s->mcr); > >>>> qspi_write32(priv->flags, ®s->mcr, > >>>> QSPI_MCR_CLR_RXF_MASK | QSPI_MCR_CLR_TXF_MASK | > >>>> - QSPI_MCR_RESERVED_MASK | QSPI_MCR_END_CFD_LE); > >>>> + mcr_reg); > >>>> qspi_write32(priv->flags, ®s->rbct, > >>>> QSPI_RBCT_RXBRD_USEIPS); > >>>> > >>>> status_reg = 0; > >>>> @@ -700,7 +708,7 @@ static void qspi_op_rdsr(struct fsl_qspi_priv > >>>> *priv, > >>> void *rxbuf, u32 len) > >>>> mcr_reg = qspi_read32(priv->flags, ®s->mcr); > >>>> qspi_write32(priv->flags, ®s->mcr, > >>>> QSPI_MCR_CLR_RXF_MASK | QSPI_MCR_CLR_TXF_MASK | > >>>> - QSPI_MCR_RESERVED_MASK | QSPI_MCR_END_CFD_LE); > >>>> + mcr_reg); > >>>> qspi_write32(priv->flags, ®s->rbct, > >>>> QSPI_RBCT_RXBRD_USEIPS); > >>>> > >>>> qspi_write32(priv->flags, ®s->sfar, priv->cur_amba_base); > >>>> @@ > >>>> -737,7 +745,7 @@ static void qspi_op_erase(struct fsl_qspi_priv *priv) > >>>> mcr_reg = qspi_read32(priv->flags, ®s->mcr); > >>>> qspi_write32(priv->flags, ®s->mcr, > >>>> QSPI_MCR_CLR_RXF_MASK | QSPI_MCR_CLR_TXF_MASK | > >>>> - QSPI_MCR_RESERVED_MASK | QSPI_MCR_END_CFD_LE); > >>>> + mcr_reg); > >>>> qspi_write32(priv->flags, ®s->rbct, > >>>> QSPI_RBCT_RXBRD_USEIPS); > >>>> > >>>> to_or_from = priv->sf_addr + priv->cur_amba_base; @@ -900,15 > >>>> +908,9 @@ static int fsl_qspi_probe(struct udevice *bus) > >>>> return ret; > >>>> } > >>>> > >>>> - mcr_val = qspi_read32(priv->flags, &priv->regs->mcr); > >>>> - > >>>> - /* Set endianness to LE for i.mx */ > >>>> - if (IS_ENABLED(CONFIG_MX6) || IS_ENABLED(CONFIG_MX7)) > >>>> - mcr_val = QSPI_MCR_END_CFD_LE; > >>>> - > >>>> qspi_write32(priv->flags, &priv->regs->mcr, > >>>> QSPI_MCR_RESERVED_MASK | QSPI_MCR_MDIS_MASK | > >>>> - (mcr_val & QSPI_MCR_END_CFD_MASK)); > >>>> + QSPI_MCR_END_CFD_LE); > >>>> > >>>> qspi_cfg_smpr(priv, ~(QSPI_SMPR_FSDLY_MASK | > >>> QSPI_SMPR_DDRSMP_MASK | > >>>> QSPI_SMPR_FSPHS_MASK | QSPI_SMPR_HSENA_MASK), 0); > >>>> > >>> _______________________________________________ > >>> U-Boot mailing list > >>> U-Boot@lists.denx.de > >>> https://lists.d > >>> > >>> enx.de%2Flistinfo%2Fu- > >>> > boot&data=02%7C01%7CAshish.Kumar%40nxp.com%7C8882d56622954 > >>> > 68a45b008d720abd98c%7C686ea1d3bc2b4c6fa92cd99c5c301635%7C0%7C0 > >>> > %7C637013794778063281&sdata=l1Pb4yEQABBx5ave4Lb7jGSVRJ1TG68 > >>> KMkl7WpSSwRc%3D&reserved=0
Hi Frieder, > Caution: EXT Email > > Hi Ye, > > On 14.08.19 12:08, Ye Li wrote: >> On latest iMX platforms like iMX7D/iMX6UL/iMX8MQ, the QSPI controller >> is updated to have TDH field in FLSHCR register. According to reference >> manual, this TDH must be set to 1 when DDR_EN is set. Otherwise, the TX >> DDR delay logic won't be enabled. > > This is actually an issue I have experienced myself. But in our case > this behavior only happened on i.MX6ULL not on i.MX6UL. Either the QSPI > controller hardware or the BootROM code changed when moving from UL to > ULL. For details see: [1]. iMX6ULL has same QSPI controller revision with 6UL. I just checked their ROM codes, the TDH will be set if the DDR mode is enabled in QSPI configuration parameters. It won't be cleared by ROM. Have you compared other registers like LUT and MCR DDR_EN? Is the MCR DDR_EN set When the TDH is cleared? > >> >> Another issue in DDR mode is the MCR register will be overwritten in >> every read/write/erase operation. This causes DDR_EN been cleared while >> TDH=1, then no clk2x output for TX data shift and all operations will >> fail. > > The best way to fix all of these things (also the ones in the other > patches) would be to fix them in Linux and port the driver from Linux to > U-Boot. Actually I've already done most of the porting [2], but didn't > have the time to finish it recently. It probably needs some rebasing and > testing. > > Could you port your fixes to the Linux driver and submit them to linux-mtd? Our downstream kernel has fixed the issue, it is similar as this u-boot fix. TDH must be set along with DDR_EN. I'm not this kernel driver owner, I will check with him about the upstream of this fix. Best regards, Ye Li > > Thanks > Frieder > > [1] https://eur01.safelinks.protection.outlook.com/?url=https%3A%2F%2Fcommunity.nxp.com%2Fthread%2F507260&data=02%7C01%7Cye.li%40nxp.com%7Cc0caf3432e8e4a136b5208d720abd020%7C686ea1d3bc2b4c6fa92cd99c5c301635%7C0%7C0%7C637013794616656270&sdata=hcyPpSlxU59NVtsHQdnGksMcu%2BQ8DzOJJJ2lnUGd7uQ%3D&reserved=0 > [2] https://eur01.safelinks.protection.outlook.com/?url=https%3A%2F%2Fgithub.com%2Ffschrempf%2Fu-boot%2Fcommits%2Ffsl_qspi_spimem_port&data=02%7C01%7Cye.li%40nxp.com%7Cc0caf3432e8e4a136b5208d720abd020%7C686ea1d3bc2b4c6fa92cd99c5c301635%7C0%7C0%7C637013794616656270&sdata=eqj4L2daAfOMA6oGbmPCPWHuy%2B6NbizQihJ0QeAcIxY%3D&reserved=0 > >> >> Signed-off-by: Ye Li <ye.li@nxp.com> >> --- >> drivers/spi/fsl_qspi.c | 30 ++++++++++++++++-------------- >> 1 file changed, 16 insertions(+), 14 deletions(-) >> >> diff --git a/drivers/spi/fsl_qspi.c b/drivers/spi/fsl_qspi.c >> index 41abe19..8845986 100644 >> --- a/drivers/spi/fsl_qspi.c >> +++ b/drivers/spi/fsl_qspi.c >> @@ -399,7 +399,7 @@ static inline void qspi_ahb_read(struct fsl_qspi_priv *priv, u8 *rxbuf, int len) >> >> qspi_write32(priv->flags, ®s->mcr, >> QSPI_MCR_CLR_RXF_MASK | QSPI_MCR_CLR_TXF_MASK | >> - QSPI_MCR_RESERVED_MASK | QSPI_MCR_END_CFD_LE); >> + mcr_reg); >> >> rx_addr = (void *)(uintptr_t)(priv->cur_amba_base + priv->sf_addr); >> /* Read out the data directly from the AHB buffer. */ >> @@ -429,6 +429,14 @@ static void qspi_enable_ddr_mode(struct fsl_qspi_priv *priv) >> reg |= BIT(29); >> >> qspi_write32(priv->flags, ®s->mcr, reg); >> + >> + /* Enable the TDH to 1 for some platforms like imx6ul, imx7d, etc >> + * These two bits are reserved on other platforms >> + */ >> + reg = qspi_read32(priv->flags, ®s->flshcr); >> + reg &= ~(BIT(17)); >> + reg |= BIT(16); >> + qspi_write32(priv->flags, ®s->flshcr, reg); >> } >> >> /* >> @@ -482,7 +490,7 @@ static void qspi_op_rdbank(struct fsl_qspi_priv *priv, u8 *rxbuf, u32 len) >> mcr_reg = qspi_read32(priv->flags, ®s->mcr); >> qspi_write32(priv->flags, ®s->mcr, >> QSPI_MCR_CLR_RXF_MASK | QSPI_MCR_CLR_TXF_MASK | >> - QSPI_MCR_RESERVED_MASK | QSPI_MCR_END_CFD_LE); >> + mcr_reg); >> qspi_write32(priv->flags, ®s->rbct, QSPI_RBCT_RXBRD_USEIPS); >> >> qspi_write32(priv->flags, ®s->sfar, priv->cur_amba_base); >> @@ -527,7 +535,7 @@ static void qspi_op_rdid(struct fsl_qspi_priv *priv, u32 *rxbuf, u32 len) >> mcr_reg = qspi_read32(priv->flags, ®s->mcr); >> qspi_write32(priv->flags, ®s->mcr, >> QSPI_MCR_CLR_RXF_MASK | QSPI_MCR_CLR_TXF_MASK | >> - QSPI_MCR_RESERVED_MASK | QSPI_MCR_END_CFD_LE); >> + mcr_reg); >> qspi_write32(priv->flags, ®s->rbct, QSPI_RBCT_RXBRD_USEIPS); >> >> qspi_write32(priv->flags, ®s->sfar, priv->cur_amba_base); >> @@ -573,7 +581,7 @@ static void qspi_op_read(struct fsl_qspi_priv *priv, u32 *rxbuf, u32 len) >> mcr_reg = qspi_read32(priv->flags, ®s->mcr); >> qspi_write32(priv->flags, ®s->mcr, >> QSPI_MCR_CLR_RXF_MASK | QSPI_MCR_CLR_TXF_MASK | >> - QSPI_MCR_RESERVED_MASK | QSPI_MCR_END_CFD_LE); >> + mcr_reg); >> qspi_write32(priv->flags, ®s->rbct, QSPI_RBCT_RXBRD_USEIPS); >> >> to_or_from = priv->sf_addr + priv->cur_amba_base; >> @@ -625,7 +633,7 @@ static void qspi_op_write(struct fsl_qspi_priv *priv, u8 *txbuf, u32 len) >> mcr_reg = qspi_read32(priv->flags, ®s->mcr); >> qspi_write32(priv->flags, ®s->mcr, >> QSPI_MCR_CLR_RXF_MASK | QSPI_MCR_CLR_TXF_MASK | >> - QSPI_MCR_RESERVED_MASK | QSPI_MCR_END_CFD_LE); >> + mcr_reg); >> qspi_write32(priv->flags, ®s->rbct, QSPI_RBCT_RXBRD_USEIPS); >> >> status_reg = 0; >> @@ -700,7 +708,7 @@ static void qspi_op_rdsr(struct fsl_qspi_priv *priv, void *rxbuf, u32 len) >> mcr_reg = qspi_read32(priv->flags, ®s->mcr); >> qspi_write32(priv->flags, ®s->mcr, >> QSPI_MCR_CLR_RXF_MASK | QSPI_MCR_CLR_TXF_MASK | >> - QSPI_MCR_RESERVED_MASK | QSPI_MCR_END_CFD_LE); >> + mcr_reg); >> qspi_write32(priv->flags, ®s->rbct, QSPI_RBCT_RXBRD_USEIPS); >> >> qspi_write32(priv->flags, ®s->sfar, priv->cur_amba_base); >> @@ -737,7 +745,7 @@ static void qspi_op_erase(struct fsl_qspi_priv *priv) >> mcr_reg = qspi_read32(priv->flags, ®s->mcr); >> qspi_write32(priv->flags, ®s->mcr, >> QSPI_MCR_CLR_RXF_MASK | QSPI_MCR_CLR_TXF_MASK | >> - QSPI_MCR_RESERVED_MASK | QSPI_MCR_END_CFD_LE); >> + mcr_reg); >> qspi_write32(priv->flags, ®s->rbct, QSPI_RBCT_RXBRD_USEIPS); >> >> to_or_from = priv->sf_addr + priv->cur_amba_base; >> @@ -900,15 +908,9 @@ static int fsl_qspi_probe(struct udevice *bus) >> return ret; >> } >> >> - mcr_val = qspi_read32(priv->flags, &priv->regs->mcr); >> - >> - /* Set endianness to LE for i.mx */ >> - if (IS_ENABLED(CONFIG_MX6) || IS_ENABLED(CONFIG_MX7)) >> - mcr_val = QSPI_MCR_END_CFD_LE; >> - >> qspi_write32(priv->flags, &priv->regs->mcr, >> QSPI_MCR_RESERVED_MASK | QSPI_MCR_MDIS_MASK | >> - (mcr_val & QSPI_MCR_END_CFD_MASK)); >> + QSPI_MCR_END_CFD_LE); >> >> qspi_cfg_smpr(priv, ~(QSPI_SMPR_FSDLY_MASK | QSPI_SMPR_DDRSMP_MASK | >> QSPI_SMPR_FSPHS_MASK | QSPI_SMPR_HSENA_MASK), 0); >> >
> -----Original Message----- > From: Schrempf Frieder <frieder.schrempf@kontron.de> > Sent: Wednesday, August 14, 2019 5:41 PM > To: Ashish Kumar <ashish.kumar@nxp.com>; Ye Li <ye.li@nxp.com>; > jagan@amarulasolutions.com > Cc: Fabio Estevam <fabio.estevam@nxp.com>; u-boot@lists.denx.de; dl- > uboot-imx <uboot-imx@nxp.com> > Subject: Re: [EXT] Re: [U-Boot] [PATCH 1/6] spi: fsl_qspi: Fix DDR mode > setting for latest iMX platforms > > Caution: EXT Email > > Sorry, I hit the "Send" button too early ;) > > On 14.08.19 14:07, Frieder Schrempf wrote: > > Hi Ashish, > > > > On 14.08.19 14:02, Ashish Kumar wrote: > >> > >> > >>> -----Original Message----- > >>> From: U-Boot <u-boot-bounces@lists.denx.de> On Behalf Of Schrempf > >>> Frieder > >>> Sent: Wednesday, August 14, 2019 5:07 PM > >>> To: Ye Li <ye.li@nxp.com>; jagan@amarulasolutions.com > >>> Cc: Fabio Estevam <fabio.estevam@nxp.com>; u-boot@lists.denx.de; > dl- > >>> uboot-imx <uboot-imx@nxp.com> > >>> Subject: [EXT] Re: [U-Boot] [PATCH 1/6] spi: fsl_qspi: Fix DDR mode > >>> setting for latest iMX platforms > >>> > >>> Caution: EXT Email > >>> > >>> Hi Ye, > >>> > >>> On 14.08.19 12:08, Ye Li wrote: > >>>> On latest iMX platforms like iMX7D/iMX6UL/iMX8MQ, the QSPI > >>>> controller is updated to have TDH field in FLSHCR register. > >>>> According to reference manual, this TDH must be set to 1 when > DDR_EN is set. > >>>> Otherwise, the TX DDR delay logic won't be enabled. > >>> > >>> This is actually an issue I have experienced myself. But in our case > >>> this behavior only happened on i.MX6ULL not on i.MX6UL. Either the > >>> QSPI controller hardware or the BootROM code changed when moving > >>> from UL to ULL. For details see: [1]. > >>> > >>>> > >>>> Another issue in DDR mode is the MCR register will be overwritten > >>>> in every read/write/erase operation. This causes DDR_EN been > >>>> cleared while TDH=1, then no clk2x output for TX data shift and all > >>>> operations will fail. > >>> > >>> The best way to fix all of these things (also the ones in the other > >>> patches) would be to fix them in Linux and port the driver from > >>> Linux to U- Boot. Actually I've already done most of the porting > >>> [2], > >> Hello Frieder, > >> > >> I had tested your porting and it was not functional on u-boot. > >> I found that only erase, read up to TX/RX buf size is working or > >> something like that. > >> Also ip and AHB mode cannot be used at time in code. Previously only > >> IP mode was used in u-boot, Since endianness across different > >> QSPI-IP(ls1012, ls1043, ls1021 big endian), (ls1088,ls2088 little > >> endian) is not consistent on various silicon's. I am not sure if > >> Yogesh who worked with you on Linux porting gave you this information > >> about endianness inconsistency. > > > > Ok, thanks for your feedback. The endianness for the different SoCs > > can be handled by the device data. > > Does this work correctly in Linux, or does the Linux driver need fixes? > > > > >> Please suggest way forward. How to correct this issue? > > The first thigh would be to make sure the Linux driver works for all platforms > and then do the porting to U-Boot. I will be out of office for > 10 days. After that I can try to work on this, but I need support and testing for > other platforms. I only have i.MX6UL/ULL. Hi Frieder, I have found some break though after porting to 2019.10 and few modification in driver code, I will update in a weeks' time. Please do not invest time on this. If I need some help I will update. Regards Ashish > > Thanks, > Frieder > > >> > >> Regards > >> Ashish > >>> time to finish it recently. It probably needs some rebasing and testing. > >>> > >>> Could you port your fixes to the Linux driver and submit them to > >>> linux-mtd? > >>> > >>> Thanks > >>> Frieder > >>> > >>> [1] > >>> https://comm > >>> > unity.nxp.com%2Fthread%2F507260&data=02%7C01%7CAshish.Kumar > >>> > %40nxp.com%7C8882d5662295468a45b008d720abd98c%7C686ea1d3bc2b4c > >>> > 6fa92cd99c5c301635%7C0%7C0%7C637013794778063281&sdata=Za7LB > >>> 6RyXAHszPfiEMLDb%2FvVNSTQJwxHFtiapmNi3Co%3D&reserved=0 > >>> [2] > >>> https://github > >>> .com%2Ffschrempf%2Fu- > >>> > boot%2Fcommits%2Ffsl_qspi_spimem_port&data=02%7C01%7CAshish. > >>> > Kumar%40nxp.com%7C8882d5662295468a45b008d720abd98c%7C686ea1d3 > >>> > bc2b4c6fa92cd99c5c301635%7C0%7C0%7C637013794778063281&sdata > >>> > =kYDjrs2a9DdL8QLFPORfHsMSWvgUxhSTgNC3WLziu7Y%3D&reserved= > 0 > >>> > >>>> > >>>> Signed-off-by: Ye Li <ye.li@nxp.com> > >>>> --- > >>>> drivers/spi/fsl_qspi.c | 30 ++++++++++++++++-------------- > >>>> 1 file changed, 16 insertions(+), 14 deletions(-) > >>>> > >>>> diff --git a/drivers/spi/fsl_qspi.c b/drivers/spi/fsl_qspi.c index > >>>> 41abe19..8845986 100644 > >>>> --- a/drivers/spi/fsl_qspi.c > >>>> +++ b/drivers/spi/fsl_qspi.c > >>>> @@ -399,7 +399,7 @@ static inline void qspi_ahb_read(struct > >>>> fsl_qspi_priv *priv, u8 *rxbuf, int len) > >>>> > >>>> qspi_write32(priv->flags, ®s->mcr, > >>>> QSPI_MCR_CLR_RXF_MASK | QSPI_MCR_CLR_TXF_MASK | > >>>> - QSPI_MCR_RESERVED_MASK | QSPI_MCR_END_CFD_LE); > >>>> + mcr_reg); > >>>> > >>>> rx_addr = (void *)(uintptr_t)(priv->cur_amba_base + > >>>> priv->sf_addr); > >>>> /* Read out the data directly from the AHB buffer. */ @@ > >>>> -429,6 > >>>> +429,14 @@ static void qspi_enable_ddr_mode(struct fsl_qspi_priv > >>>> +*priv) > >>>> reg |= BIT(29); > >>>> > >>>> qspi_write32(priv->flags, ®s->mcr, reg); > >>>> + > >>>> + /* Enable the TDH to 1 for some platforms like imx6ul, imx7d, etc > >>>> + * These two bits are reserved on other platforms > >>>> + */ > >>>> + reg = qspi_read32(priv->flags, ®s->flshcr); > >>>> + reg &= ~(BIT(17)); > >>>> + reg |= BIT(16); > >>>> + qspi_write32(priv->flags, ®s->flshcr, reg); > >>>> } > >>>> > >>>> /* > >>>> @@ -482,7 +490,7 @@ static void qspi_op_rdbank(struct fsl_qspi_priv > >>> *priv, u8 *rxbuf, u32 len) > >>>> mcr_reg = qspi_read32(priv->flags, ®s->mcr); > >>>> qspi_write32(priv->flags, ®s->mcr, > >>>> QSPI_MCR_CLR_RXF_MASK | QSPI_MCR_CLR_TXF_MASK | > >>>> - QSPI_MCR_RESERVED_MASK | QSPI_MCR_END_CFD_LE); > >>>> + mcr_reg); > >>>> qspi_write32(priv->flags, ®s->rbct, > >>>> QSPI_RBCT_RXBRD_USEIPS); > >>>> > >>>> qspi_write32(priv->flags, ®s->sfar, priv->cur_amba_base); > >>>> @@ > >>>> -527,7 +535,7 @@ static void qspi_op_rdid(struct fsl_qspi_priv > >>>> *priv, u32 > >>> *rxbuf, u32 len) > >>>> mcr_reg = qspi_read32(priv->flags, ®s->mcr); > >>>> qspi_write32(priv->flags, ®s->mcr, > >>>> QSPI_MCR_CLR_RXF_MASK | QSPI_MCR_CLR_TXF_MASK | > >>>> - QSPI_MCR_RESERVED_MASK | QSPI_MCR_END_CFD_LE); > >>>> + mcr_reg); > >>>> qspi_write32(priv->flags, ®s->rbct, > >>>> QSPI_RBCT_RXBRD_USEIPS); > >>>> > >>>> qspi_write32(priv->flags, ®s->sfar, priv->cur_amba_base); > >>>> @@ > >>>> -573,7 +581,7 @@ static void qspi_op_read(struct fsl_qspi_priv > >>>> *priv, u32 > >>> *rxbuf, u32 len) > >>>> mcr_reg = qspi_read32(priv->flags, ®s->mcr); > >>>> qspi_write32(priv->flags, ®s->mcr, > >>>> QSPI_MCR_CLR_RXF_MASK | QSPI_MCR_CLR_TXF_MASK | > >>>> - QSPI_MCR_RESERVED_MASK | QSPI_MCR_END_CFD_LE); > >>>> + mcr_reg); > >>>> qspi_write32(priv->flags, ®s->rbct, > >>>> QSPI_RBCT_RXBRD_USEIPS); > >>>> > >>>> to_or_from = priv->sf_addr + priv->cur_amba_base; @@ -625,7 > >>>> +633,7 @@ static void qspi_op_write(struct fsl_qspi_priv *priv, u8 > >>>> *txbuf, > >>> u32 len) > >>>> mcr_reg = qspi_read32(priv->flags, ®s->mcr); > >>>> qspi_write32(priv->flags, ®s->mcr, > >>>> QSPI_MCR_CLR_RXF_MASK | QSPI_MCR_CLR_TXF_MASK | > >>>> - QSPI_MCR_RESERVED_MASK | QSPI_MCR_END_CFD_LE); > >>>> + mcr_reg); > >>>> qspi_write32(priv->flags, ®s->rbct, > >>>> QSPI_RBCT_RXBRD_USEIPS); > >>>> > >>>> status_reg = 0; > >>>> @@ -700,7 +708,7 @@ static void qspi_op_rdsr(struct fsl_qspi_priv > >>>> *priv, > >>> void *rxbuf, u32 len) > >>>> mcr_reg = qspi_read32(priv->flags, ®s->mcr); > >>>> qspi_write32(priv->flags, ®s->mcr, > >>>> QSPI_MCR_CLR_RXF_MASK | QSPI_MCR_CLR_TXF_MASK | > >>>> - QSPI_MCR_RESERVED_MASK | QSPI_MCR_END_CFD_LE); > >>>> + mcr_reg); > >>>> qspi_write32(priv->flags, ®s->rbct, > >>>> QSPI_RBCT_RXBRD_USEIPS); > >>>> > >>>> qspi_write32(priv->flags, ®s->sfar, priv->cur_amba_base); > >>>> @@ > >>>> -737,7 +745,7 @@ static void qspi_op_erase(struct fsl_qspi_priv *priv) > >>>> mcr_reg = qspi_read32(priv->flags, ®s->mcr); > >>>> qspi_write32(priv->flags, ®s->mcr, > >>>> QSPI_MCR_CLR_RXF_MASK | QSPI_MCR_CLR_TXF_MASK | > >>>> - QSPI_MCR_RESERVED_MASK | QSPI_MCR_END_CFD_LE); > >>>> + mcr_reg); > >>>> qspi_write32(priv->flags, ®s->rbct, > >>>> QSPI_RBCT_RXBRD_USEIPS); > >>>> > >>>> to_or_from = priv->sf_addr + priv->cur_amba_base; @@ -900,15 > >>>> +908,9 @@ static int fsl_qspi_probe(struct udevice *bus) > >>>> return ret; > >>>> } > >>>> > >>>> - mcr_val = qspi_read32(priv->flags, &priv->regs->mcr); > >>>> - > >>>> - /* Set endianness to LE for i.mx */ > >>>> - if (IS_ENABLED(CONFIG_MX6) || IS_ENABLED(CONFIG_MX7)) > >>>> - mcr_val = QSPI_MCR_END_CFD_LE; > >>>> - > >>>> qspi_write32(priv->flags, &priv->regs->mcr, > >>>> QSPI_MCR_RESERVED_MASK | QSPI_MCR_MDIS_MASK | > >>>> - (mcr_val & QSPI_MCR_END_CFD_MASK)); > >>>> + QSPI_MCR_END_CFD_LE); > >>>> > >>>> qspi_cfg_smpr(priv, ~(QSPI_SMPR_FSDLY_MASK | > >>> QSPI_SMPR_DDRSMP_MASK | > >>>> QSPI_SMPR_FSPHS_MASK | QSPI_SMPR_HSENA_MASK), 0); > >>>> > >>> _______________________________________________ > >>> U-Boot mailing list > >>> U-Boot@lists.denx.de > >>> https://lists.d > >>> > >>> enx.de%2Flistinfo%2Fu- > >>> > boot&data=02%7C01%7CAshish.Kumar%40nxp.com%7C8882d56622954 > >>> > 68a45b008d720abd98c%7C686ea1d3bc2b4c6fa92cd99c5c301635%7C0%7C0 > >>> > %7C637013794778063281&sdata=l1Pb4yEQABBx5ave4Lb7jGSVRJ1TG68 > >>> KMkl7WpSSwRc%3D&reserved=0
Hi Ashish, On 27.08.19 11:56, Ashish Kumar wrote: > > >> -----Original Message----- >> From: Schrempf Frieder <frieder.schrempf@kontron.de> >> Sent: Wednesday, August 14, 2019 5:41 PM >> To: Ashish Kumar <ashish.kumar@nxp.com>; Ye Li <ye.li@nxp.com>; >> jagan@amarulasolutions.com >> Cc: Fabio Estevam <fabio.estevam@nxp.com>; u-boot@lists.denx.de; dl- >> uboot-imx <uboot-imx@nxp.com> >> Subject: Re: [EXT] Re: [U-Boot] [PATCH 1/6] spi: fsl_qspi: Fix DDR mode >> setting for latest iMX platforms >> >> Caution: EXT Email >> >> Sorry, I hit the "Send" button too early ;) >> >> On 14.08.19 14:07, Frieder Schrempf wrote: >>> Hi Ashish, >>> >>> On 14.08.19 14:02, Ashish Kumar wrote: >>>> >>>> >>>>> -----Original Message----- >>>>> From: U-Boot <u-boot-bounces@lists.denx.de> On Behalf Of Schrempf >>>>> Frieder >>>>> Sent: Wednesday, August 14, 2019 5:07 PM >>>>> To: Ye Li <ye.li@nxp.com>; jagan@amarulasolutions.com >>>>> Cc: Fabio Estevam <fabio.estevam@nxp.com>; u-boot@lists.denx.de; >> dl- >>>>> uboot-imx <uboot-imx@nxp.com> >>>>> Subject: [EXT] Re: [U-Boot] [PATCH 1/6] spi: fsl_qspi: Fix DDR mode >>>>> setting for latest iMX platforms >>>>> >>>>> Caution: EXT Email >>>>> >>>>> Hi Ye, >>>>> >>>>> On 14.08.19 12:08, Ye Li wrote: >>>>>> On latest iMX platforms like iMX7D/iMX6UL/iMX8MQ, the QSPI >>>>>> controller is updated to have TDH field in FLSHCR register. >>>>>> According to reference manual, this TDH must be set to 1 when >> DDR_EN is set. >>>>>> Otherwise, the TX DDR delay logic won't be enabled. >>>>> >>>>> This is actually an issue I have experienced myself. But in our case >>>>> this behavior only happened on i.MX6ULL not on i.MX6UL. Either the >>>>> QSPI controller hardware or the BootROM code changed when moving >>>>> from UL to ULL. For details see: [1]. >>>>> >>>>>> >>>>>> Another issue in DDR mode is the MCR register will be overwritten >>>>>> in every read/write/erase operation. This causes DDR_EN been >>>>>> cleared while TDH=1, then no clk2x output for TX data shift and all >>>>>> operations will fail. >>>>> >>>>> The best way to fix all of these things (also the ones in the other >>>>> patches) would be to fix them in Linux and port the driver from >>>>> Linux to U- Boot. Actually I've already done most of the porting >>>>> [2], >>>> Hello Frieder, >>>> >>>> I had tested your porting and it was not functional on u-boot. >>>> I found that only erase, read up to TX/RX buf size is working or >>>> something like that. >>>> Also ip and AHB mode cannot be used at time in code. Previously only >>>> IP mode was used in u-boot, Since endianness across different >>>> QSPI-IP(ls1012, ls1043, ls1021 big endian), (ls1088,ls2088 little >>>> endian) is not consistent on various silicon's. I am not sure if >>>> Yogesh who worked with you on Linux porting gave you this information >>>> about endianness inconsistency. >>> >>> Ok, thanks for your feedback. The endianness for the different SoCs >>> can be handled by the device data. >> >> Does this work correctly in Linux, or does the Linux driver need fixes? >> >>> >>>> Please suggest way forward. How to correct this issue? >> >> The first thigh would be to make sure the Linux driver works for all platforms >> and then do the porting to U-Boot. I will be out of office for >> 10 days. After that I can try to work on this, but I need support and testing for >> other platforms. I only have i.MX6UL/ULL. > > Hi Frieder, > > I have found some break though after porting to 2019.10 and few modification in driver code, I will update in a weeks' time. Please do not invest time on this. > If I need some help I will update. Thanks for your work. Do you already have some news? Can you share your results? Thanks, Frieder
Hi Ashish, Hi Frieder, On 09.09.19 10:10, Schrempf Frieder wrote: > Hi Ashish, > > On 27.08.19 11:56, Ashish Kumar wrote: >> >> >>> -----Original Message----- >>> From: Schrempf Frieder <frieder.schrempf@kontron.de> >>> Sent: Wednesday, August 14, 2019 5:41 PM >>> To: Ashish Kumar <ashish.kumar@nxp.com>; Ye Li <ye.li@nxp.com>; >>> jagan@amarulasolutions.com >>> Cc: Fabio Estevam <fabio.estevam@nxp.com>; u-boot@lists.denx.de; dl- >>> uboot-imx <uboot-imx@nxp.com> >>> Subject: Re: [EXT] Re: [U-Boot] [PATCH 1/6] spi: fsl_qspi: Fix DDR mode >>> setting for latest iMX platforms >>> >>> Caution: EXT Email >>> >>> Sorry, I hit the "Send" button too early ;) >>> >>> On 14.08.19 14:07, Frieder Schrempf wrote: >>>> Hi Ashish, >>>> >>>> On 14.08.19 14:02, Ashish Kumar wrote: >>>>> >>>>> >>>>>> -----Original Message----- >>>>>> From: U-Boot <u-boot-bounces@lists.denx.de> On Behalf Of Schrempf >>>>>> Frieder >>>>>> Sent: Wednesday, August 14, 2019 5:07 PM >>>>>> To: Ye Li <ye.li@nxp.com>; jagan@amarulasolutions.com >>>>>> Cc: Fabio Estevam <fabio.estevam@nxp.com>; u-boot@lists.denx.de; >>> dl- >>>>>> uboot-imx <uboot-imx@nxp.com> >>>>>> Subject: [EXT] Re: [U-Boot] [PATCH 1/6] spi: fsl_qspi: Fix DDR mode >>>>>> setting for latest iMX platforms >>>>>> >>>>>> Caution: EXT Email >>>>>> >>>>>> Hi Ye, >>>>>> >>>>>> On 14.08.19 12:08, Ye Li wrote: >>>>>>> On latest iMX platforms like iMX7D/iMX6UL/iMX8MQ, the QSPI >>>>>>> controller is updated to have TDH field in FLSHCR register. >>>>>>> According to reference manual, this TDH must be set to 1 when >>> DDR_EN is set. >>>>>>> Otherwise, the TX DDR delay logic won't be enabled. >>>>>> >>>>>> This is actually an issue I have experienced myself. But in our case >>>>>> this behavior only happened on i.MX6ULL not on i.MX6UL. Either the >>>>>> QSPI controller hardware or the BootROM code changed when moving >>>>>> from UL to ULL. For details see: [1]. >>>>>> >>>>>>> >>>>>>> Another issue in DDR mode is the MCR register will be overwritten >>>>>>> in every read/write/erase operation. This causes DDR_EN been >>>>>>> cleared while TDH=1, then no clk2x output for TX data shift and all >>>>>>> operations will fail. >>>>>> >>>>>> The best way to fix all of these things (also the ones in the other >>>>>> patches) would be to fix them in Linux and port the driver from >>>>>> Linux to U- Boot. Actually I've already done most of the porting >>>>>> [2], >>>>> Hello Frieder, >>>>> >>>>> I had tested your porting and it was not functional on u-boot. >>>>> I found that only erase, read up to TX/RX buf size is working or >>>>> something like that. >>>>> Also ip and AHB mode cannot be used at time in code. Previously only >>>>> IP mode was used in u-boot, Since endianness across different >>>>> QSPI-IP(ls1012, ls1043, ls1021 big endian), (ls1088,ls2088 little >>>>> endian) is not consistent on various silicon's. I am not sure if >>>>> Yogesh who worked with you on Linux porting gave you this information >>>>> about endianness inconsistency. >>>> >>>> Ok, thanks for your feedback. The endianness for the different SoCs >>>> can be handled by the device data. >>> >>> Does this work correctly in Linux, or does the Linux driver need fixes? >>> >>>> >>>>> Please suggest way forward. How to correct this issue? >>> >>> The first thigh would be to make sure the Linux driver works for all platforms >>> and then do the porting to U-Boot. I will be out of office for >>> 10 days. After that I can try to work on this, but I need support and testing for >>> other platforms. I only have i.MX6UL/ULL. >> >> Hi Frieder, >> >> I have found some break though after porting to 2019.10 and few modification in driver code, I will update in a weeks' time. Please do not invest time on this. >> If I need some help I will update. > > Thanks for your work. Do you already have some news? Can you share your > results? I'm most likely currently running in similar issues on tests with the i.MX6ULL EVK. QSPI does not work reliably. So before digging deeper into the QSPI driver, I wanted to check on the status of any updates in the driver. Is there anything available that I could use for testing already? Thanks, Stefan
> -----Original Message----- > From: Stefan Roese <sr@denx.de> > Sent: Wednesday, September 11, 2019 11:17 AM > To: Schrempf Frieder <frieder.schrempf@kontron.de>; Ashish Kumar > <ashish.kumar@nxp.com>; Ye Li <ye.li@nxp.com>; > jagan@amarulasolutions.com > Cc: Fabio Estevam <fabio.estevam@nxp.com>; u-boot@lists.denx.de; dl- > uboot-imx <uboot-imx@nxp.com> > Subject: Re: [U-Boot] [EXT] Re: [PATCH 1/6] spi: fsl_qspi: Fix DDR mode > setting for latest iMX platforms > > Caution: EXT Email > > Hi Ashish, > Hi Frieder, > > On 09.09.19 10:10, Schrempf Frieder wrote: > > Hi Ashish, > > > > On 27.08.19 11:56, Ashish Kumar wrote: > >> > >> > >>> -----Original Message----- > >>> From: Schrempf Frieder <frieder.schrempf@kontron.de> > >>> Sent: Wednesday, August 14, 2019 5:41 PM > >>> To: Ashish Kumar <ashish.kumar@nxp.com>; Ye Li <ye.li@nxp.com>; > >>> jagan@amarulasolutions.com > >>> Cc: Fabio Estevam <fabio.estevam@nxp.com>; u-boot@lists.denx.de; > dl- > >>> uboot-imx <uboot-imx@nxp.com> > >>> Subject: Re: [EXT] Re: [U-Boot] [PATCH 1/6] spi: fsl_qspi: Fix DDR > >>> mode setting for latest iMX platforms > >>> > >>> Caution: EXT Email > >>> > >>> Sorry, I hit the "Send" button too early ;) > >>> > >>> On 14.08.19 14:07, Frieder Schrempf wrote: > >>>> Hi Ashish, > >>>> > >>>> On 14.08.19 14:02, Ashish Kumar wrote: > >>>>> > >>>>> > >>>>>> -----Original Message----- > >>>>>> From: U-Boot <u-boot-bounces@lists.denx.de> On Behalf Of > Schrempf > >>>>>> Frieder > >>>>>> Sent: Wednesday, August 14, 2019 5:07 PM > >>>>>> To: Ye Li <ye.li@nxp.com>; jagan@amarulasolutions.com > >>>>>> Cc: Fabio Estevam <fabio.estevam@nxp.com>; u- > boot@lists.denx.de; > >>> dl- > >>>>>> uboot-imx <uboot-imx@nxp.com> > >>>>>> Subject: [EXT] Re: [U-Boot] [PATCH 1/6] spi: fsl_qspi: Fix DDR > >>>>>> mode setting for latest iMX platforms > >>>>>> > >>>>>> Caution: EXT Email > >>>>>> > >>>>>> Hi Ye, > >>>>>> > >>>>>> On 14.08.19 12:08, Ye Li wrote: > >>>>>>> On latest iMX platforms like iMX7D/iMX6UL/iMX8MQ, the QSPI > >>>>>>> controller is updated to have TDH field in FLSHCR register. > >>>>>>> According to reference manual, this TDH must be set to 1 when > >>> DDR_EN is set. > >>>>>>> Otherwise, the TX DDR delay logic won't be enabled. > >>>>>> > >>>>>> This is actually an issue I have experienced myself. But in our > >>>>>> case this behavior only happened on i.MX6ULL not on i.MX6UL. > >>>>>> Either the QSPI controller hardware or the BootROM code changed > >>>>>> when moving from UL to ULL. For details see: [1]. > >>>>>> > >>>>>>> > >>>>>>> Another issue in DDR mode is the MCR register will be > >>>>>>> overwritten in every read/write/erase operation. This causes > >>>>>>> DDR_EN been cleared while TDH=1, then no clk2x output for TX > >>>>>>> data shift and all operations will fail. > >>>>>> > >>>>>> The best way to fix all of these things (also the ones in the > >>>>>> other > >>>>>> patches) would be to fix them in Linux and port the driver from > >>>>>> Linux to U- Boot. Actually I've already done most of the porting > >>>>>> [2], > >>>>> Hello Frieder, > >>>>> > >>>>> I had tested your porting and it was not functional on u-boot. > >>>>> I found that only erase, read up to TX/RX buf size is working or > >>>>> something like that. > >>>>> Also ip and AHB mode cannot be used at time in code. Previously > >>>>> only IP mode was used in u-boot, Since endianness across different > >>>>> QSPI-IP(ls1012, ls1043, ls1021 big endian), (ls1088,ls2088 little > >>>>> endian) is not consistent on various silicon's. I am not sure if > >>>>> Yogesh who worked with you on Linux porting gave you this > >>>>> information about endianness inconsistency. > >>>> > >>>> Ok, thanks for your feedback. The endianness for the different SoCs > >>>> can be handled by the device data. > >>> > >>> Does this work correctly in Linux, or does the Linux driver need fixes? > >>> > >>>> > >>>>> Please suggest way forward. How to correct this issue? > >>> > >>> The first thigh would be to make sure the Linux driver works for all > >>> platforms and then do the porting to U-Boot. I will be out of office > >>> for > >>> 10 days. After that I can try to work on this, but I need support > >>> and testing for other platforms. I only have i.MX6UL/ULL. > >> > >> Hi Frieder, > >> > >> I have found some break though after porting to 2019.10 and few > modification in driver code, I will update in a weeks' time. Please do not > invest time on this. > >> If I need some help I will update. > > > > Thanks for your work. Do you already have some news? Can you share > > your results? > > I'm most likely currently running in similar issues on tests with the i.MX6ULL > EVK. QSPI does not work reliably. So before digging deeper into the QSPI > driver, I wanted to check on the status of any updates in the driver. Is there > anything available that I could use for testing already? Hi Stefan, Frieder, The spi-mem version is still under debug, I could make it working for ls1088rdb, ls1046rdb, but it is failing for ls1012ardb and ls2088ardb and untested for i.mx and other Layerscape silicon/boards . It is derived from work done by Frieder earlier. This version can be found here: https://github.com/erashish007/u-boot-spi-mem/tree/spi-mem-port There is completely working version of fsl_qspi.c based on old xfer method, which was not accepted in upstream, considering it is recommended to migrate to spi-mem frame. This version is located here: https://github.com/erashish007/u-boot-spi-mem/tree/xfer_wrking Regards Ashish I have push the working version of these on github. > > Thanks, > Stefan
Hi Ashish, On 12.09.19 07:03, Ashish Kumar wrote: <snip> >>>>>>> Please suggest way forward. How to correct this issue? >>>>> >>>>> The first thigh would be to make sure the Linux driver works for all >>>>> platforms and then do the porting to U-Boot. I will be out of office >>>>> for >>>>> 10 days. After that I can try to work on this, but I need support >>>>> and testing for other platforms. I only have i.MX6UL/ULL. >>>> >>>> Hi Frieder, >>>> >>>> I have found some break though after porting to 2019.10 and few >> modification in driver code, I will update in a weeks' time. Please do not >> invest time on this. >>>> If I need some help I will update. >>> >>> Thanks for your work. Do you already have some news? Can you share >>> your results? >> >> I'm most likely currently running in similar issues on tests with the i.MX6ULL >> EVK. QSPI does not work reliably. So before digging deeper into the QSPI >> driver, I wanted to check on the status of any updates in the driver. Is there >> anything available that I could use for testing already? > Hi Stefan, Frieder, > > The spi-mem version is still under debug, I could make it working > for ls1088rdb, ls1046rdb, but it is failing for > ls1012ardb and ls2088ardb and untested for i.mx and other Layerscape > silicon/boards . It is derived from work done by Frieder earlier. > This version can be found here: > https://github.com/erashish007/u-boot-spi-mem/tree/spi-mem-port Many thanks. I did some tests with this version and it seems to work fine in general on the i.MX6ULL EVK. My first tests show that reading and writing has no issues. So this is very promising. The only thing I noticed is, that when using SPI for environment via CONFIG_ENV_IS_IN_SPI_FLASH, the board hangs upon bootup while trying to read the env. Since you already added some debug print's to the env code, I suspect that you also did run into this problem. I'll try to help with this driver version. At least I can debug this env issue and can always do some test on my mx6ull platform for you once you have any updates here. Just let me know. > There is completely working version of fsl_qspi.c based on old xfer > method, which was not accepted in upstream, > considering it is recommended to migrate to spi-mem frame. This > version is located here: > https://github.com/erashish007/u-boot-spi-mem/tree/xfer_wrking This one does not work for me on the i.MX6ULL EVK. "sf read" command returns almost immediately and the data is not read as it seems. I did not dig into this deeper though. Thanks, Stefan
> -----Original Message----- > From: Stefan Roese <sr@denx.de> > Sent: Thursday, September 12, 2019 6:06 PM > To: Ashish Kumar <ashish.kumar@nxp.com>; Schrempf Frieder > <frieder.schrempf@kontron.de>; Ye Li <ye.li@nxp.com>; > jagan@amarulasolutions.com > Cc: Fabio Estevam <fabio.estevam@nxp.com>; u-boot@lists.denx.de; dl- > uboot-imx <uboot-imx@nxp.com> > Subject: Re: [U-Boot] [EXT] Re: [PATCH 1/6] spi: fsl_qspi: Fix DDR mode > setting for latest iMX platforms > > Caution: EXT Email > > Hi Ashish, > > On 12.09.19 07:03, Ashish Kumar wrote: > > <snip> > > >>>>>>> Please suggest way forward. How to correct this issue? > >>>>> > >>>>> The first thigh would be to make sure the Linux driver works for > >>>>> all platforms and then do the porting to U-Boot. I will be out of > >>>>> office for > >>>>> 10 days. After that I can try to work on this, but I need support > >>>>> and testing for other platforms. I only have i.MX6UL/ULL. > >>>> > >>>> Hi Frieder, > >>>> > >>>> I have found some break though after porting to 2019.10 and few > >> modification in driver code, I will update in a weeks' time. Please > >> do not invest time on this. > >>>> If I need some help I will update. > >>> > >>> Thanks for your work. Do you already have some news? Can you share > >>> your results? > >> > >> I'm most likely currently running in similar issues on tests with the > >> i.MX6ULL EVK. QSPI does not work reliably. So before digging deeper > >> into the QSPI driver, I wanted to check on the status of any updates > >> in the driver. Is there anything available that I could use for testing > already? > > Hi Stefan, Frieder, > > > > The spi-mem version is still under debug, I could make it working for > > ls1088rdb, ls1046rdb, but it is failing for ls1012ardb and ls2088ardb > > and untested for i.mx and other Layerscape silicon/boards . It is > > derived from work done by Frieder earlier. > > This version can be found here: > > https://eur01.safelinks.protection.outlook.com/?url=https%3A%2F%2Fgith > > ub.com%2Ferashish007%2Fu-boot-spi-mem%2Ftree%2Fspi-mem- > port&data=0 > > > 2%7C01%7Cashish.kumar%40nxp.com%7C2697e09d52b94dc737ce08d737cd3 > 34c%7C6 > > > 86ea1d3bc2b4c6fa92cd99c5c301635%7C0%7C0%7C637039226779815773& > ;sdata > > > =NqcR9VBELvvXhcCay850Fj%2BEZuOjkJzf15IXdBR4l2A%3D&reserved=0 > > Many thanks. I did some tests with this version and it seems to work fine in > general on the i.MX6ULL EVK. My first tests show that reading and writing > has no issues. So this is very promising. The only thing I noticed is, that when > using SPI for environment via CONFIG_ENV_IS_IN_SPI_FLASH, the board > hangs upon bootup while trying to read the env. Since you already added > some debug print's to the env code, I suspect that you also did run into this > problem. > > I'll try to help with this driver version. At least I can debug this env issue and > can always do some test on my mx6ull platform for you once you have any > updates here. Just let me know. Hi Stefan, Yes, I was also debugging the same, what confuses me is that it works on Ls1046, ls1088, but fails on ls2088, ls1012. Regards Ashish > > > There is completely working version of fsl_qspi.c based on old xfer > > method, which was not accepted in upstream, considering it is > > recommended to migrate to spi-mem frame. This version is located here: > > https://eur01.safelinks.protection.outlook.com/?url=https%3A%2F%2Fgith > > ub.com%2Ferashish007%2Fu-boot-spi- > mem%2Ftree%2Fxfer_wrking&data=02 > > > %7C01%7Cashish.kumar%40nxp.com%7C2697e09d52b94dc737ce08d737cd33 > 4c%7C68 > > > 6ea1d3bc2b4c6fa92cd99c5c301635%7C0%7C0%7C637039226779815773& > sdata= > > qpksEv36Zjb6jXKsnWN5iVtAoG9UEQFMIi4uu1OgiJ8%3D&reserved=0 > > This one does not work for me on the i.MX6ULL EVK. "sf read" command > returns almost immediately and the data is not read as it seems. I did not dig > into this deeper though. > > Thanks, > Stefan
Hi Ashish, On 12.09.19 14:36, Stefan Roese wrote: <snip> >> The spi-mem version is still under debug, I could make it working >> for ls1088rdb, ls1046rdb, but it is failing for >> ls1012ardb and ls2088ardb and untested for i.mx and other Layerscape >> silicon/boards . It is derived from work done by Frieder earlier. >> This version can be found here: >> https://github.com/erashish007/u-boot-spi-mem/tree/spi-mem-port > > Many thanks. I did some tests with this version and it seems to work > fine in general on the i.MX6ULL EVK. My first tests show that reading > and writing has no issues. So this is very promising. The only thing > I noticed is, that when using SPI for environment via > CONFIG_ENV_IS_IN_SPI_FLASH, the board hangs upon bootup while trying > to read the env. Since you already added some debug print's to the > env code, I suspect that you also did run into this problem. > > I'll try to help with this driver version. At least I can debug this > env issue and can always do some test on my mx6ull platform for you > once you have any updates here. Just let me know. Okay, this one with the env in SPI NOR is fixed. Its a problem with your PR debug printf macro. Please change it this way: -#define PR(fmt, ...) \ - fprintf(stderr, "DEBUG: %s:%d:%s(): " fmt" \n", \ +#define PR(fmt, ...) \ + printf("DEBUG: %s:%d:%s(): " fmt" \n", \ With this change, I can successfully boot with SPI NOR environment on my board. I will do some further tests with your driver next week and will get back to you with the results. Please keep me in the loop, if you have updates on the driver. Thanks, Stefan
Hi Ashish, On 13.09.19 15:11, Stefan Roese wrote: > On 12.09.19 14:36, Stefan Roese wrote: > > <snip> > >>> The spi-mem version is still under debug, I could make it working >>> for ls1088rdb, ls1046rdb, but it is failing for >>> ls1012ardb and ls2088ardb and untested for i.mx and other Layerscape >>> silicon/boards . It is derived from work done by Frieder earlier. >>> This version can be found here: >>> https://github.com/erashish007/u-boot-spi-mem/tree/spi-mem-port >> >> Many thanks. I did some tests with this version and it seems to work >> fine in general on the i.MX6ULL EVK. My first tests show that reading >> and writing has no issues. So this is very promising. The only thing >> I noticed is, that when using SPI for environment via >> CONFIG_ENV_IS_IN_SPI_FLASH, the board hangs upon bootup while trying >> to read the env. Since you already added some debug print's to the >> env code, I suspect that you also did run into this problem. >> >> I'll try to help with this driver version. At least I can debug this >> env issue and can always do some test on my mx6ull platform for you >> once you have any updates here. Just let me know. > > Okay, this one with the env in SPI NOR is fixed. Its a problem with > your PR debug printf macro. Please change it this way: > > -#define PR(fmt, ...) \ > - fprintf(stderr, "DEBUG: %s:%d:%s(): " fmt" \n", \ > +#define PR(fmt, ...) \ > + printf("DEBUG: %s:%d:%s(): " fmt" \n", \ > > With this change, I can successfully boot with SPI NOR environment > on my board. I will do some further tests with your driver next > week and will get back to you with the results. Please keep me in > the loop, if you have updates on the driver. One further update on this QSPI driver. This driver only works when loaded via "imx_usb" on the i.MX6ULL EVK. When programmed into QSPI and booted from QSPI this driver does not detect the SPI NOR flash: => sf probe unrecognized JEDEC id bytes: ff, ff, ff Do you have any idea what might explain this difference. I would have expected that when booting via QSPI it would be "easier" for the driver, as the BootROM already initializes the QSPI interface. Which is not the case in the boot via serial download (imx_usb) mode. Here everyhting (pinmux, clocks, etc) need to be configured. My feeling is that something is configured "incorrectly" by the BootROM in this case which is not re-configured as the QSPI driver needs it to be currently. Do you have any ideas on what might be the problem here? Is there something that I can do to help test this? Would it help if I would send the debug logging of the driver? Thanks, Stefan
Hi Stefan, On 18.09.19 06:51, Stefan Roese wrote: > Hi Ashish, > > On 13.09.19 15:11, Stefan Roese wrote: >> On 12.09.19 14:36, Stefan Roese wrote: >> >> <snip> >> >>>> The spi-mem version is still under debug, I could make it working >>>> for ls1088rdb, ls1046rdb, but it is failing for >>>> ls1012ardb and ls2088ardb and untested for i.mx and other Layerscape >>>> silicon/boards . It is derived from work done by Frieder earlier. >>>> This version can be found here: >>>> https://github.com/erashish007/u-boot-spi-mem/tree/spi-mem-port >>> >>> Many thanks. I did some tests with this version and it seems to work >>> fine in general on the i.MX6ULL EVK. My first tests show that reading >>> and writing has no issues. So this is very promising. The only thing >>> I noticed is, that when using SPI for environment via >>> CONFIG_ENV_IS_IN_SPI_FLASH, the board hangs upon bootup while trying >>> to read the env. Since you already added some debug print's to the >>> env code, I suspect that you also did run into this problem. >>> >>> I'll try to help with this driver version. At least I can debug this >>> env issue and can always do some test on my mx6ull platform for you >>> once you have any updates here. Just let me know. >> >> Okay, this one with the env in SPI NOR is fixed. Its a problem with >> your PR debug printf macro. Please change it this way: >> >> -#define PR(fmt, ...) \ >> -Â Â Â Â Â Â Â fprintf(stderr, "DEBUG: %s:%d:%s(): " fmt" \n", \ >> +#define PR(fmt, ...)Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â \ >> +Â Â Â Â Â Â Â printf("DEBUG: %s:%d:%s(): " fmt" \n", \ >> >> With this change, I can successfully boot with SPI NOR environment >> on my board. I will do some further tests with your driver next >> week and will get back to you with the results. Please keep me in >> the loop, if you have updates on the driver. > > One further update on this QSPI driver. This driver only works when > loaded via "imx_usb" on the i.MX6ULL EVK. When programmed into QSPI > and booted from QSPI this driver does not detect the SPI NOR > flash: > > => sf probe > unrecognized JEDEC id bytes: ff, ff, ff > > Do you have any idea what might explain this difference. I would have > expected that when booting via QSPI it would be "easier" for the > driver, as the BootROM already initializes the QSPI interface. Which > is not the case in the boot via serial download (imx_usb) mode. Here > everyhting (pinmux, clocks, etc) need to be configured. > > My feeling is that something is configured "incorrectly" by the > BootROM in this case which is not re-configured as the QSPI driver > needs it to be currently. > > Do you have any ideas on what might be the problem here? Is there > something that I can do to help test this? Would it help if I would > send the debug logging of the driver? I have a strong suspicion of what goes wrong in your case. We experienced exactly the same issue recently on i.MX6ULL. For some reasons (I guess differences in BootROM) this does not happen on i.MX6UL. The problem is, that the BootROM sets the TDH bits in the QuadSPI_FLSHCR register to '01' in case it uses the DDR mode. Afterwards when U-Boot or Linux try to access the flash in SDR mode, they fail as the TDH bits are still set. Resetting them to '00' solves the problem. Unfortunately the TDH bits are not documented in the manual of the i.MX6UL/ULL, but they can be found in the manual of the i.MX7. For the QSPI driver, this means it needs a fix to set/reset the TDH bits according to the mode that is used (DDR/SDR). For more details please also look here: https://community.nxp.com/thread/507260 Thanks, Frieder
Hi Frieder, On 18.09.19 09:08, Schrempf Frieder wrote: <snip> >> One further update on this QSPI driver. This driver only works when >> loaded via "imx_usb" on the i.MX6ULL EVK. When programmed into QSPI >> and booted from QSPI this driver does not detect the SPI NOR >> flash: >> >> => sf probe >> unrecognized JEDEC id bytes: ff, ff, ff >> >> Do you have any idea what might explain this difference. I would have >> expected that when booting via QSPI it would be "easier" for the >> driver, as the BootROM already initializes the QSPI interface. Which >> is not the case in the boot via serial download (imx_usb) mode. Here >> everyhting (pinmux, clocks, etc) need to be configured. >> >> My feeling is that something is configured "incorrectly" by the >> BootROM in this case which is not re-configured as the QSPI driver >> needs it to be currently. >> >> Do you have any ideas on what might be the problem here? Is there >> something that I can do to help test this? Would it help if I would >> send the debug logging of the driver? > > I have a strong suspicion of what goes wrong in your case. We > experienced exactly the same issue recently on i.MX6ULL. For some > reasons (I guess differences in BootROM) this does not happen on i.MX6UL. > > The problem is, that the BootROM sets the TDH bits in the QuadSPI_FLSHCR > register to '01' in case it uses the DDR mode. Afterwards when U-Boot or > Linux try to access the flash in SDR mode, they fail as the TDH bits are > still set. Resetting them to '00' solves the problem. > > Unfortunately the TDH bits are not documented in the manual of the > i.MX6UL/ULL, but they can be found in the manual of the i.MX7. > > For the QSPI driver, this means it needs a fix to set/reset the TDH bits > according to the mode that is used (DDR/SDR). > > For more details please also look here: > https://community.nxp.com/thread/507260 Perfect. With these bits set to 00 again, booting from QSPI now works on the EVK. Many thanks for this hint! :) BTW: When receiving your mail I was just comparing the registers settings and wondering about this difference in bit 16 as well. ;) Thanks, Stefan
Hi Frieder, Hi Ashish, Hi Ye Li, Hi Fabio, On 18.09.19 09:42, Stefan Roese wrote: > Hi Frieder, > > On 18.09.19 09:08, Schrempf Frieder wrote: > > <snip> > >>> One further update on this QSPI driver. This driver only works when >>> loaded via "imx_usb" on the i.MX6ULL EVK. When programmed into QSPI >>> and booted from QSPI this driver does not detect the SPI NOR >>> flash: >>> >>> => sf probe >>> unrecognized JEDEC id bytes: ff, ff, ff >>> >>> Do you have any idea what might explain this difference. I would have >>> expected that when booting via QSPI it would be "easier" for the >>> driver, as the BootROM already initializes the QSPI interface. Which >>> is not the case in the boot via serial download (imx_usb) mode. Here >>> everyhting (pinmux, clocks, etc) need to be configured. >>> >>> My feeling is that something is configured "incorrectly" by the >>> BootROM in this case which is not re-configured as the QSPI driver >>> needs it to be currently. >>> >>> Do you have any ideas on what might be the problem here? Is there >>> something that I can do to help test this? Would it help if I would >>> send the debug logging of the driver? >> >> I have a strong suspicion of what goes wrong in your case. We >> experienced exactly the same issue recently on i.MX6ULL. For some >> reasons (I guess differences in BootROM) this does not happen on i.MX6UL. >> >> The problem is, that the BootROM sets the TDH bits in the QuadSPI_FLSHCR >> register to '01' in case it uses the DDR mode. Afterwards when U-Boot or >> Linux try to access the flash in SDR mode, they fail as the TDH bits are >> still set. Resetting them to '00' solves the problem. >> >> Unfortunately the TDH bits are not documented in the manual of the >> i.MX6UL/ULL, but they can be found in the manual of the i.MX7. >> >> For the QSPI driver, this means it needs a fix to set/reset the TDH bits >> according to the mode that is used (DDR/SDR). >> >> For more details please also look here: >> https://community.nxp.com/thread/507260 > > Perfect. With these bits set to 00 again, booting from QSPI now > works on the EVK. Many thanks for this hint! :) I'm coming back to this issue, as we now have the new NXP patches integrated into mainline. Including this one: 7949576664ac "spi: fsl_qspi: Fix DDR mode setting for latest iMX platforms" I've now re-tested current mainline on the i.MX6ULL Eval Kit and QSPI does not work reliably. This is with CONFIG_SYS_FSL_QSPI_AHB enabled and disabled. How is QSPI supposed to work on i.MX6ULL/ULZ? Is any one of you running this current mainline driver successfully on one any i-MX6ULL/ULZ based board? If yes, what is your configuration here? BTW: Using the "spi-mem" driver version from Ashish with the fix suggested by Frieder to clear the DDR bit in TDH (reset to 00) still works without any problems. Thanks, Stefan [1] https://github.com/erashish007/u-boot-spi-mem/tree/spi-mem-port
Hi Stefan, On 22.10.19 15:18, Stefan Roese wrote: > Hi Frieder, > Hi Ashish, > Hi Ye Li, > Hi Fabio, > > On 18.09.19 09:42, Stefan Roese wrote: >> Hi Frieder, >> >> On 18.09.19 09:08, Schrempf Frieder wrote: >> >> <snip> >> >>>> One further update on this QSPI driver. This driver only works when >>>> loaded via "imx_usb" on the i.MX6ULL EVK. When programmed into QSPI >>>> and booted from QSPI this driver does not detect the SPI NOR >>>> flash: >>>> >>>> => sf probe >>>> unrecognized JEDEC id bytes: ff, ff, ff >>>> >>>> Do you have any idea what might explain this difference. I would have >>>> expected that when booting via QSPI it would be "easier" for the >>>> driver, as the BootROM already initializes the QSPI interface. Which >>>> is not the case in the boot via serial download (imx_usb) mode. Here >>>> everyhting (pinmux, clocks, etc) need to be configured. >>>> >>>> My feeling is that something is configured "incorrectly" by the >>>> BootROM in this case which is not re-configured as the QSPI driver >>>> needs it to be currently. >>>> >>>> Do you have any ideas on what might be the problem here? Is there >>>> something that I can do to help test this? Would it help if I would >>>> send the debug logging of the driver? >>> >>> I have a strong suspicion of what goes wrong in your case. We >>> experienced exactly the same issue recently on i.MX6ULL. For some >>> reasons (I guess differences in BootROM) this does not happen on >>> i.MX6UL. >>> >>> The problem is, that the BootROM sets the TDH bits in the QuadSPI_FLSHCR >>> register to '01' in case it uses the DDR mode. Afterwards when U-Boot or >>> Linux try to access the flash in SDR mode, they fail as the TDH bits are >>> still set. Resetting them to '00' solves the problem. >>> >>> Unfortunately the TDH bits are not documented in the manual of the >>> i.MX6UL/ULL, but they can be found in the manual of the i.MX7. >>> >>> For the QSPI driver, this means it needs a fix to set/reset the TDH bits >>> according to the mode that is used (DDR/SDR). >>> >>> For more details please also look here: >>> https://community.nxp.com/thread/507260 >> >> Perfect. With these bits set to 00 again, booting from QSPI now >> works on the EVK. Many thanks for this hint! :) > > I'm coming back to this issue, as we now have the new NXP patches > integrated into mainline. Including this one: > > 7949576664ac "spi: fsl_qspi: Fix DDR mode setting for latest iMX platforms" > > I've now re-tested current mainline on the i.MX6ULL Eval Kit and QSPI > does not work reliably. This is with CONFIG_SYS_FSL_QSPI_AHB enabled > and disabled. How is QSPI supposed to work on i.MX6ULL/ULZ? Is any > one of you running this current mainline driver successfully on one > any i-MX6ULL/ULZ based board? If yes, what is your configuration here? I don't have any experience with the current mainline SPI NOR driver. > > BTW: Using the "spi-mem" driver version from Ashish with the fix > suggested by Frieder to clear the DDR bit in TDH (reset to 00) still > works without any problems. There is some cleanup work that needs to be done (e.g. [1]). After that I will send an official patch for the spi-mem driver. Then Ashish and you can comment with your test results and change requests. I have also sent a fix for the TDH bit for the Linux driver [2]. This is also applicable to the new U-Boot driver. Regards, Frieder [1]: https://github.com/fschrempf/u-boot/commits/spi_flash_kconfig_cleanup [2]: https://patchwork.kernel.org/patch/11176905/
On 22.10.19 15:55, Frieder Schrempf wrote: > Hi Stefan, > > On 22.10.19 15:18, Stefan Roese wrote: >> Hi Frieder, >> Hi Ashish, >> Hi Ye Li, >> Hi Fabio, >> >> On 18.09.19 09:42, Stefan Roese wrote: >>> Hi Frieder, >>> >>> On 18.09.19 09:08, Schrempf Frieder wrote: >>> >>> <snip> >>> >>>>> One further update on this QSPI driver. This driver only works when >>>>> loaded via "imx_usb" on the i.MX6ULL EVK. When programmed into QSPI >>>>> and booted from QSPI this driver does not detect the SPI NOR >>>>> flash: >>>>> >>>>> => sf probe >>>>> unrecognized JEDEC id bytes: ff, ff, ff >>>>> >>>>> Do you have any idea what might explain this difference. I would have >>>>> expected that when booting via QSPI it would be "easier" for the >>>>> driver, as the BootROM already initializes the QSPI interface. Which >>>>> is not the case in the boot via serial download (imx_usb) mode. Here >>>>> everyhting (pinmux, clocks, etc) need to be configured. >>>>> >>>>> My feeling is that something is configured "incorrectly" by the >>>>> BootROM in this case which is not re-configured as the QSPI driver >>>>> needs it to be currently. >>>>> >>>>> Do you have any ideas on what might be the problem here? Is there >>>>> something that I can do to help test this? Would it help if I would >>>>> send the debug logging of the driver? >>>> >>>> I have a strong suspicion of what goes wrong in your case. We >>>> experienced exactly the same issue recently on i.MX6ULL. For some >>>> reasons (I guess differences in BootROM) this does not happen on >>>> i.MX6UL. >>>> >>>> The problem is, that the BootROM sets the TDH bits in the >>>> QuadSPI_FLSHCR >>>> register to '01' in case it uses the DDR mode. Afterwards when >>>> U-Boot or >>>> Linux try to access the flash in SDR mode, they fail as the TDH bits >>>> are >>>> still set. Resetting them to '00' solves the problem. >>>> >>>> Unfortunately the TDH bits are not documented in the manual of the >>>> i.MX6UL/ULL, but they can be found in the manual of the i.MX7. >>>> >>>> For the QSPI driver, this means it needs a fix to set/reset the TDH >>>> bits >>>> according to the mode that is used (DDR/SDR). >>>> >>>> For more details please also look here: >>>> https://community.nxp.com/thread/507260 >>> >>> Perfect. With these bits set to 00 again, booting from QSPI now >>> works on the EVK. Many thanks for this hint! :) >> >> I'm coming back to this issue, as we now have the new NXP patches >> integrated into mainline. Including this one: >> >> 7949576664ac "spi: fsl_qspi: Fix DDR mode setting for latest iMX >> platforms" > >> I've now re-tested current mainline on the i.MX6ULL Eval Kit and QSPI >> does not work reliably. This is with CONFIG_SYS_FSL_QSPI_AHB enabled >> and disabled. How is QSPI supposed to work on i.MX6ULL/ULZ? Is any >> one of you running this current mainline driver successfully on one >> any i-MX6ULL/ULZ based board? If yes, what is your configuration here? > > I don't have any experience with the current mainline SPI NOR driver. I had a look and the current mainline driver has 7949576664ac "spi: fsl_qspi: Fix DDR mode setting for latest iMX platforms", but it still does not reset the TDH bits during init, in case the BootROM has left them set, which is the case when booting from QSPI on i.MX6ULL. Initially for detection the driver does not use DDR, so TDH must be cleared. I have an older U-Boot 2017.03 that uses a backported version of the current mainline QSPI driver without DM. And it seems to work fine on i.MX6ULL with a 64MB SPI NOR, except for the issue described above. >> >> BTW: Using the "spi-mem" driver version from Ashish with the fix >> suggested by Frieder to clear the DDR bit in TDH (reset to 00) still >> works without any problems. > > There is some cleanup work that needs to be done (e.g. [1]). After that > I will send an official patch for the spi-mem driver. Then Ashish and > you can comment with your test results and change requests. > > I have also sent a fix for the TDH bit for the Linux driver [2]. This is > also applicable to the new U-Boot driver. > > Regards, > Frieder > > [1]: https://github.com/fschrempf/u-boot/commits/spi_flash_kconfig_cleanup > [2]: https://patchwork.kernel.org/patch/11176905/
Hi Frieder, On 22.10.19 15:55, Schrempf Frieder wrote: > Hi Stefan, > > On 22.10.19 15:18, Stefan Roese wrote: >> Hi Frieder, >> Hi Ashish, >> Hi Ye Li, >> Hi Fabio, >> >> On 18.09.19 09:42, Stefan Roese wrote: >>> Hi Frieder, >>> >>> On 18.09.19 09:08, Schrempf Frieder wrote: >>> >>> <snip> >>> >>>>> One further update on this QSPI driver. This driver only works when >>>>> loaded via "imx_usb" on the i.MX6ULL EVK. When programmed into QSPI >>>>> and booted from QSPI this driver does not detect the SPI NOR >>>>> flash: >>>>> >>>>> => sf probe >>>>> unrecognized JEDEC id bytes: ff, ff, ff >>>>> >>>>> Do you have any idea what might explain this difference. I would have >>>>> expected that when booting via QSPI it would be "easier" for the >>>>> driver, as the BootROM already initializes the QSPI interface. Which >>>>> is not the case in the boot via serial download (imx_usb) mode. Here >>>>> everyhting (pinmux, clocks, etc) need to be configured. >>>>> >>>>> My feeling is that something is configured "incorrectly" by the >>>>> BootROM in this case which is not re-configured as the QSPI driver >>>>> needs it to be currently. >>>>> >>>>> Do you have any ideas on what might be the problem here? Is there >>>>> something that I can do to help test this? Would it help if I would >>>>> send the debug logging of the driver? >>>> >>>> I have a strong suspicion of what goes wrong in your case. We >>>> experienced exactly the same issue recently on i.MX6ULL. For some >>>> reasons (I guess differences in BootROM) this does not happen on >>>> i.MX6UL. >>>> >>>> The problem is, that the BootROM sets the TDH bits in the QuadSPI_FLSHCR >>>> register to '01' in case it uses the DDR mode. Afterwards when U-Boot or >>>> Linux try to access the flash in SDR mode, they fail as the TDH bits are >>>> still set. Resetting them to '00' solves the problem. >>>> >>>> Unfortunately the TDH bits are not documented in the manual of the >>>> i.MX6UL/ULL, but they can be found in the manual of the i.MX7. >>>> >>>> For the QSPI driver, this means it needs a fix to set/reset the TDH bits >>>> according to the mode that is used (DDR/SDR). >>>> >>>> For more details please also look here: >>>> https://community.nxp.com/thread/507260 >>> >>> Perfect. With these bits set to 00 again, booting from QSPI now >>> works on the EVK. Many thanks for this hint! :) >> >> I'm coming back to this issue, as we now have the new NXP patches >> integrated into mainline. Including this one: >> >> 7949576664ac "spi: fsl_qspi: Fix DDR mode setting for latest iMX platforms" > >> I've now re-tested current mainline on the i.MX6ULL Eval Kit and QSPI >> does not work reliably. This is with CONFIG_SYS_FSL_QSPI_AHB enabled >> and disabled. How is QSPI supposed to work on i.MX6ULL/ULZ? Is any >> one of you running this current mainline driver successfully on one >> any i-MX6ULL/ULZ based board? If yes, what is your configuration here? > > I don't have any experience with the current mainline SPI NOR driver. > >> >> BTW: Using the "spi-mem" driver version from Ashish with the fix >> suggested by Frieder to clear the DDR bit in TDH (reset to 00) still >> works without any problems. > > There is some cleanup work that needs to be done (e.g. [1]). After that > I will send an official patch for the spi-mem driver. Then Ashish and > you can comment with your test results and change requests. Sounds like a good plan. Please keep me on Cc on any of these patches, so that I don't forget to review and test them. Thanks in advance. > I have also sent a fix for the TDH bit for the Linux driver [2]. This is > also applicable to the new U-Boot driver. Thanks. > Regards, > Frieder > > [1]: https://github.com/fschrempf/u-boot/commits/spi_flash_kconfig_cleanup > [2]: https://patchwork.kernel.org/patch/11176905/ > Thanks, Stefan
Hi Frieder, On 22.10.19 16:24, Schrempf Frieder wrote: > On 22.10.19 15:55, Frieder Schrempf wrote: >> Hi Stefan, >> >> On 22.10.19 15:18, Stefan Roese wrote: >>> Hi Frieder, >>> Hi Ashish, >>> Hi Ye Li, >>> Hi Fabio, >>> >>> On 18.09.19 09:42, Stefan Roese wrote: >>>> Hi Frieder, >>>> >>>> On 18.09.19 09:08, Schrempf Frieder wrote: >>>> >>>> <snip> >>>> >>>>>> One further update on this QSPI driver. This driver only works when >>>>>> loaded via "imx_usb" on the i.MX6ULL EVK. When programmed into QSPI >>>>>> and booted from QSPI this driver does not detect the SPI NOR >>>>>> flash: >>>>>> >>>>>> => sf probe >>>>>> unrecognized JEDEC id bytes: ff, ff, ff >>>>>> >>>>>> Do you have any idea what might explain this difference. I would have >>>>>> expected that when booting via QSPI it would be "easier" for the >>>>>> driver, as the BootROM already initializes the QSPI interface. Which >>>>>> is not the case in the boot via serial download (imx_usb) mode. Here >>>>>> everyhting (pinmux, clocks, etc) need to be configured. >>>>>> >>>>>> My feeling is that something is configured "incorrectly" by the >>>>>> BootROM in this case which is not re-configured as the QSPI driver >>>>>> needs it to be currently. >>>>>> >>>>>> Do you have any ideas on what might be the problem here? Is there >>>>>> something that I can do to help test this? Would it help if I would >>>>>> send the debug logging of the driver? >>>>> >>>>> I have a strong suspicion of what goes wrong in your case. We >>>>> experienced exactly the same issue recently on i.MX6ULL. For some >>>>> reasons (I guess differences in BootROM) this does not happen on >>>>> i.MX6UL. >>>>> >>>>> The problem is, that the BootROM sets the TDH bits in the >>>>> QuadSPI_FLSHCR >>>>> register to '01' in case it uses the DDR mode. Afterwards when >>>>> U-Boot or >>>>> Linux try to access the flash in SDR mode, they fail as the TDH bits >>>>> are >>>>> still set. Resetting them to '00' solves the problem. >>>>> >>>>> Unfortunately the TDH bits are not documented in the manual of the >>>>> i.MX6UL/ULL, but they can be found in the manual of the i.MX7. >>>>> >>>>> For the QSPI driver, this means it needs a fix to set/reset the TDH >>>>> bits >>>>> according to the mode that is used (DDR/SDR). >>>>> >>>>> For more details please also look here: >>>>> https://community.nxp.com/thread/507260 >>>> >>>> Perfect. With these bits set to 00 again, booting from QSPI now >>>> works on the EVK. Many thanks for this hint! :) >>> >>> I'm coming back to this issue, as we now have the new NXP patches >>> integrated into mainline. Including this one: >>> >>> 7949576664ac "spi: fsl_qspi: Fix DDR mode setting for latest iMX >>> platforms" > >>> I've now re-tested current mainline on the i.MX6ULL Eval Kit and QSPI >>> does not work reliably. This is with CONFIG_SYS_FSL_QSPI_AHB enabled >>> and disabled. How is QSPI supposed to work on i.MX6ULL/ULZ? Is any >>> one of you running this current mainline driver successfully on one >>> any i-MX6ULL/ULZ based board? If yes, what is your configuration here? >> >> I don't have any experience with the current mainline SPI NOR driver. > > I had a look and the current mainline driver has 7949576664ac "spi: > fsl_qspi: Fix DDR mode setting for latest iMX platforms", but it still > does not reset the TDH bits during init, in case the BootROM has left > them set, which is the case when booting from QSPI on i.MX6ULL. > Initially for detection the driver does not use DDR, so TDH must be cleared. I've already "played" with those DDR bits (TDH) in this register - without success so far. Let's concentrate on the spi-mem driver instead. Thanks, Stefan
> -----Original Message----- > From: Stefan Roese <sr@denx.de> > Sent: Tuesday, October 22, 2019 9:12 PM > To: Schrempf Frieder <frieder.schrempf@kontron.de>; Ashish Kumar > <ashish.kumar@nxp.com>; Ye Li <ye.li@nxp.com>; > jagan@amarulasolutions.com > Cc: Fabio Estevam <fabio.estevam@nxp.com>; u-boot@lists.denx.de; dl- > uboot-imx <uboot-imx@nxp.com> > Subject: Re: [U-Boot] [EXT] Re: [PATCH 1/6] spi: fsl_qspi: Fix DDR mode > setting for latest iMX platforms > > Caution: EXT Email > > Hi Frieder, > > On 22.10.19 15:55, Schrempf Frieder wrote: > > Hi Stefan, > > > > On 22.10.19 15:18, Stefan Roese wrote: > >> Hi Frieder, > >> Hi Ashish, > >> Hi Ye Li, > >> Hi Fabio, > >> > >> On 18.09.19 09:42, Stefan Roese wrote: > >>> Hi Frieder, > >>> > >>> On 18.09.19 09:08, Schrempf Frieder wrote: > >>> > >>> <snip> > >>> > >>>>> One further update on this QSPI driver. This driver only works > >>>>> when loaded via "imx_usb" on the i.MX6ULL EVK. When programmed > >>>>> into QSPI and booted from QSPI this driver does not detect the SPI > >>>>> NOR > >>>>> flash: > >>>>> > >>>>> => sf probe > >>>>> unrecognized JEDEC id bytes: ff, ff, ff > >>>>> > >>>>> Do you have any idea what might explain this difference. I would > >>>>> have expected that when booting via QSPI it would be "easier" for > >>>>> the driver, as the BootROM already initializes the QSPI interface. > >>>>> Which is not the case in the boot via serial download (imx_usb) > >>>>> mode. Here everyhting (pinmux, clocks, etc) need to be configured. > >>>>> > >>>>> My feeling is that something is configured "incorrectly" by the > >>>>> BootROM in this case which is not re-configured as the QSPI driver > >>>>> needs it to be currently. > >>>>> > >>>>> Do you have any ideas on what might be the problem here? Is there > >>>>> something that I can do to help test this? Would it help if I > >>>>> would send the debug logging of the driver? > >>>> > >>>> I have a strong suspicion of what goes wrong in your case. We > >>>> experienced exactly the same issue recently on i.MX6ULL. For some > >>>> reasons (I guess differences in BootROM) this does not happen on > >>>> i.MX6UL. > >>>> > >>>> The problem is, that the BootROM sets the TDH bits in the > >>>> QuadSPI_FLSHCR register to '01' in case it uses the DDR mode. > >>>> Afterwards when U-Boot or Linux try to access the flash in SDR > >>>> mode, they fail as the TDH bits are still set. Resetting them to '00' solves > the problem. > >>>> > >>>> Unfortunately the TDH bits are not documented in the manual of the > >>>> i.MX6UL/ULL, but they can be found in the manual of the i.MX7. > >>>> > >>>> For the QSPI driver, this means it needs a fix to set/reset the TDH > >>>> bits according to the mode that is used (DDR/SDR). > >>>> > >>>> For more details please also look here: > >>>> > https://eur01.safelinks.protection.outlook.com/?url=https%3A%2F%2Fc > >>>> > ommunity.nxp.com%2Fthread%2F507260&data=02%7C01%7Cashish.ku > mar% > >>>> > 40nxp.com%7Cc03ee11b869c47c9a7d308d757065561%7C686ea1d3bc2b4c6fa > 92c > >>>> > d99c5c301635%7C0%7C0%7C637073557020848801&sdata=ZvRU8vVPq0ll > gIF > >>>> 56nVugHjTQhM0E1GJ8PPl6P46vrg%3D&reserved=0 > >>> > >>> Perfect. With these bits set to 00 again, booting from QSPI now > >>> works on the EVK. Many thanks for this hint! :) > >> > >> I'm coming back to this issue, as we now have the new NXP patches > >> integrated into mainline. Including this one: > >> > >> 7949576664ac "spi: fsl_qspi: Fix DDR mode setting for latest iMX > >> platforms" > I've now re-tested current mainline on the i.MX6ULL Eval > >> Kit and QSPI does not work reliably. This is with > >> CONFIG_SYS_FSL_QSPI_AHB enabled and disabled. How is QSPI > supposed to > >> work on i.MX6ULL/ULZ? Is any one of you running this current mainline > >> driver successfully on one any i-MX6ULL/ULZ based board? If yes, what is > your configuration here? > > > > I don't have any experience with the current mainline SPI NOR driver. > > > >> > >> BTW: Using the "spi-mem" driver version from Ashish with the fix > >> suggested by Frieder to clear the DDR bit in TDH (reset to 00) still > >> works without any problems. > > > > There is some cleanup work that needs to be done (e.g. [1]). After > > that I will send an official patch for the spi-mem driver. Then Ashish > > and you can comment with your test results and change requests. > > Sounds like a good plan. Please keep me on Cc on any of these patches, so > that I don't forget to review and test them. Thanks in advance. Hi Frieder, I see one issue with current SPI-MEM driver getting ported in u-boot, that is: access via md command is not working, in order words direct access to flash memory is no more possible, which was previously possible. Such access is helpful to access memory directly and make user experience similar to parallel NOR. To fix this I had tried these possible changes in driver, change this to q->devtype_data->ahb_buf_size to flash size rather than AHB buffer size. In function fsl_qspi_default_setup() { /* * In HW there can be a maximum of four chips on two buses with * two chip selects on each bus. We use four chip selects in SW * to differentiate between the four chips. * We use ahb_buf_size for each chip and set SFA1AD, SFA2AD, SFB1AD, * SFB2AD accordingly. */ qspi_writel(q, q->devtype_data->ahb_buf_size + addr_offset, base + QUADSPI_SFA1AD); qspi_writel(q, q->devtype_data->ahb_buf_size * 2 + addr_offset, base + QUADSPI_SFA2AD); qspi_writel(q, q->devtype_data->ahb_buf_size * 3 + addr_offset, base + QUADSPI_SFB1AD); qspi_writel(q, q->devtype_data->ahb_buf_size * 4 + addr_offset, base + QUADSPI_SFB2AD); >>>>>>>>>>>>>>> In function fsl_qspi_exec_op() { qspi_writel(q, QUADSPI_RBCT_WMRK_MASK | QUADSPI_RBCT_RXBRD_USEIPS, base + QUADSPI_RBCT); if (op->data.nbytes && op->data.dir == SPI_MEM_DATA_OUT) fsl_qspi_fill_txfifo(q, op); err = fsl_qspi_do_op(q, op); added this line : spi_writel(q, op->data.nbytes | QUADSPI_IPCR_SEQID(0), base + QUADSPI_IPCR); And set seq_id to 0, Which was initially set by BootROM, after this md works correctly but, sf read is only able to read max data size of rxfifo. Could you please suggest what other thing I am missing to get access via md command. I will post the delta changes via mail or github[1] tomorrow morning. [1]: https://github.com/erashish007/u-boot-spi-mem/blob/a516f41f17acb1ff9be6d34f54644605c666128c/drivers/spi/fsl_qspi.c Regards Ashish > > I have also sent a fix for the TDH bit for the Linux driver [2]. This > > is also applicable to the new U-Boot driver. > > Thanks. > > > Regards, > > Frieder > > > > [1]: > > https://eur01.safelinks.protection.outlook.com/?url=https%3A%2F%2Fgith > > ub.com%2Ffschrempf%2Fu- > boot%2Fcommits%2Fspi_flash_kconfig_cleanup& > > > data=02%7C01%7Cashish.kumar%40nxp.com%7Cc03ee11b869c47c9a7d308d7 > 570655 > > > 61%7C686ea1d3bc2b4c6fa92cd99c5c301635%7C0%7C0%7C6370735570208488 > 01& > > > ;sdata=VvxGQquY5n9fysZueDsbj0f3Jc%2Bo6b0TelevDt0M7cI%3D&rese > rved=0 > > [2]: > > > https://eur01.safelinks.protection.outlook.com/?url=https%3A%2F%2Fpatc > > > hwork.kernel.org%2Fpatch%2F11176905%2F&data=02%7C01%7Cashish. > kumar > > > %40nxp.com%7Cc03ee11b869c47c9a7d308d757065561%7C686ea1d3bc2b4c6f > a92cd9 > > > 9c5c301635%7C0%7C0%7C637073557020848801&sdata=KSz2A%2B0ssK% > 2B7OR6M > > zQtyZdNAX7C4yU%2Fhl5xDLYWSenU%3D&reserved=0 > > > > Thanks, > Stefan
Hi Ashish, On 22.10.19 18:11, Ashish Kumar wrote: > > >> -----Original Message----- >> From: Stefan Roese <sr@denx.de> >> Sent: Tuesday, October 22, 2019 9:12 PM >> To: Schrempf Frieder <frieder.schrempf@kontron.de>; Ashish Kumar >> <ashish.kumar@nxp.com>; Ye Li <ye.li@nxp.com>; >> jagan@amarulasolutions.com >> Cc: Fabio Estevam <fabio.estevam@nxp.com>; u-boot@lists.denx.de; dl- >> uboot-imx <uboot-imx@nxp.com> >> Subject: Re: [U-Boot] [EXT] Re: [PATCH 1/6] spi: fsl_qspi: Fix DDR mode >> setting for latest iMX platforms >> >> Caution: EXT Email >> >> Hi Frieder, >> >> On 22.10.19 15:55, Schrempf Frieder wrote: >>> Hi Stefan, >>> >>> On 22.10.19 15:18, Stefan Roese wrote: >>>> Hi Frieder, >>>> Hi Ashish, >>>> Hi Ye Li, >>>> Hi Fabio, >>>> >>>> On 18.09.19 09:42, Stefan Roese wrote: >>>>> Hi Frieder, >>>>> >>>>> On 18.09.19 09:08, Schrempf Frieder wrote: >>>>> >>>>> <snip> >>>>> >>>>>>> One further update on this QSPI driver. This driver only works >>>>>>> when loaded via "imx_usb" on the i.MX6ULL EVK. When programmed >>>>>>> into QSPI and booted from QSPI this driver does not detect the SPI >>>>>>> NOR >>>>>>> flash: >>>>>>> >>>>>>> => sf probe >>>>>>> unrecognized JEDEC id bytes: ff, ff, ff >>>>>>> >>>>>>> Do you have any idea what might explain this difference. I would >>>>>>> have expected that when booting via QSPI it would be "easier" for >>>>>>> the driver, as the BootROM already initializes the QSPI interface. >>>>>>> Which is not the case in the boot via serial download (imx_usb) >>>>>>> mode. Here everyhting (pinmux, clocks, etc) need to be configured. >>>>>>> >>>>>>> My feeling is that something is configured "incorrectly" by the >>>>>>> BootROM in this case which is not re-configured as the QSPI driver >>>>>>> needs it to be currently. >>>>>>> >>>>>>> Do you have any ideas on what might be the problem here? Is there >>>>>>> something that I can do to help test this? Would it help if I >>>>>>> would send the debug logging of the driver? >>>>>> >>>>>> I have a strong suspicion of what goes wrong in your case. We >>>>>> experienced exactly the same issue recently on i.MX6ULL. For some >>>>>> reasons (I guess differences in BootROM) this does not happen on >>>>>> i.MX6UL. >>>>>> >>>>>> The problem is, that the BootROM sets the TDH bits in the >>>>>> QuadSPI_FLSHCR register to '01' in case it uses the DDR mode. >>>>>> Afterwards when U-Boot or Linux try to access the flash in SDR >>>>>> mode, they fail as the TDH bits are still set. Resetting them to '00' solves >> the problem. >>>>>> >>>>>> Unfortunately the TDH bits are not documented in the manual of the >>>>>> i.MX6UL/ULL, but they can be found in the manual of the i.MX7. >>>>>> >>>>>> For the QSPI driver, this means it needs a fix to set/reset the TDH >>>>>> bits according to the mode that is used (DDR/SDR). >>>>>> >>>>>> For more details please also look here: >>>>>> >> https://eur01.safelinks.protection.outlook.com/?url=https%3A%2F%2Fc >>>>>> >> ommunity.nxp.com%2Fthread%2F507260&data=02%7C01%7Cashish.ku >> mar% >>>>>> >> 40nxp.com%7Cc03ee11b869c47c9a7d308d757065561%7C686ea1d3bc2b4c6fa >> 92c >>>>>> >> d99c5c301635%7C0%7C0%7C637073557020848801&sdata=ZvRU8vVPq0ll >> gIF >>>>>> 56nVugHjTQhM0E1GJ8PPl6P46vrg%3D&reserved=0 >>>>> >>>>> Perfect. With these bits set to 00 again, booting from QSPI now >>>>> works on the EVK. Many thanks for this hint! :) >>>> >>>> I'm coming back to this issue, as we now have the new NXP patches >>>> integrated into mainline. Including this one: >>>> >>>> 7949576664ac "spi: fsl_qspi: Fix DDR mode setting for latest iMX >>>> platforms" > I've now re-tested current mainline on the i.MX6ULL Eval >>>> Kit and QSPI does not work reliably. This is with >>>> CONFIG_SYS_FSL_QSPI_AHB enabled and disabled. How is QSPI >> supposed to >>>> work on i.MX6ULL/ULZ? Is any one of you running this current mainline >>>> driver successfully on one any i-MX6ULL/ULZ based board? If yes, what is >> your configuration here? >>> >>> I don't have any experience with the current mainline SPI NOR driver. >>> >>>> >>>> BTW: Using the "spi-mem" driver version from Ashish with the fix >>>> suggested by Frieder to clear the DDR bit in TDH (reset to 00) still >>>> works without any problems. >>> >>> There is some cleanup work that needs to be done (e.g. [1]). After >>> that I will send an official patch for the spi-mem driver. Then Ashish >>> and you can comment with your test results and change requests. >> >> Sounds like a good plan. Please keep me on Cc on any of these patches, so >> that I don't forget to review and test them. Thanks in advance. > > Hi Frieder, > > I see one issue with current SPI-MEM driver getting ported in u-boot, that is: access via md command is not working, in order words direct access to flash memory is no more possible, which was previously possible. Such access is helpful to access memory directly and make user experience similar to parallel NOR. Yes, direct memory-mapped access to the whole flash is not possible with this driver. Though I don't see any good reason why someone wants to do this, when there's a SPI NOR and MTD layer to abstract the flash and handle access to it. You can still use sf and/or mtd commands to read/write data between flash and RAM. > > To fix this I had tried these possible changes in driver, change this to q->devtype_data->ahb_buf_size to flash size rather than AHB buffer size. This is not a good idea. The driver is not designed to do this. In the upstream Linux SPI MEM layer, we have a implementation for a direct mapping API, that allows the SPI MEM layer to create memory mappings for SPI NOR devices. If needed this could be ported to U-Boot, but IMHO it should be avoided. > > In function > fsl_qspi_default_setup() { > /* > * In HW there can be a maximum of four chips on two buses with > * two chip selects on each bus. We use four chip selects in SW > * to differentiate between the four chips. > * We use ahb_buf_size for each chip and set SFA1AD, SFA2AD, SFB1AD, > * SFB2AD accordingly. > */ > qspi_writel(q, q->devtype_data->ahb_buf_size + addr_offset, > base + QUADSPI_SFA1AD); > qspi_writel(q, q->devtype_data->ahb_buf_size * 2 + addr_offset, > base + QUADSPI_SFA2AD); > qspi_writel(q, q->devtype_data->ahb_buf_size * 3 + addr_offset, > base + QUADSPI_SFB1AD); > qspi_writel(q, q->devtype_data->ahb_buf_size * 4 + addr_offset, > base + QUADSPI_SFB2AD); > >>>>>>>>>>>>>>>> > In function fsl_qspi_exec_op() { > qspi_writel(q, QUADSPI_RBCT_WMRK_MASK | > QUADSPI_RBCT_RXBRD_USEIPS, base + QUADSPI_RBCT); > > if (op->data.nbytes && op->data.dir == SPI_MEM_DATA_OUT) > fsl_qspi_fill_txfifo(q, op); > > err = fsl_qspi_do_op(q, op); > > added this line : spi_writel(q, op->data.nbytes | QUADSPI_IPCR_SEQID(0), base + QUADSPI_IPCR); > > And set seq_id to 0, Which was initially set by BootROM, after this md works correctly but, sf read is only able to read max data size of rxfifo. > > Could you please suggest what other thing I am missing to get access via md command. Sorry no, because as already mentioned, this driver and the SPI MEM layer is not supposed to work like this. At least not without the direct mapping API ([1]). Regards, Frieder [1]: https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/commit?id=aa167f3fed0c37e0e4c707d4331d827661f46644
On 23.10.19 12:34, Schrempf Frieder wrote: <snip> >>>>> BTW: Using the "spi-mem" driver version from Ashish with the fix >>>>> suggested by Frieder to clear the DDR bit in TDH (reset to 00) still >>>>> works without any problems. >>>> >>>> There is some cleanup work that needs to be done (e.g. [1]). After >>>> that I will send an official patch for the spi-mem driver. Then Ashish >>>> and you can comment with your test results and change requests. >>> >>> Sounds like a good plan. Please keep me on Cc on any of these patches, so >>> that I don't forget to review and test them. Thanks in advance. >> >> Hi Frieder, >> >> I see one issue with current SPI-MEM driver getting ported in u-boot, that is: access via md command is not working, in order words direct access to flash memory is no more possible, which was previously possible. Such access is helpful to access memory directly and make user experience similar to parallel NOR. > > Yes, direct memory-mapped access to the whole flash is not possible with > this driver. Though I don't see any good reason why someone wants to do > this, when there's a SPI NOR and MTD layer to abstract the flash and > handle access to it. You can still use sf and/or mtd commands to > read/write data between flash and RAM. I agree with Frieder that we don't need this direct memory access via "md/mm/etc" in U-Boot. Access via the "normal" sf and/or mtd commands should be enough for SPI NOR. Thanks, Stefan
diff --git a/drivers/spi/fsl_qspi.c b/drivers/spi/fsl_qspi.c index 41abe19..8845986 100644 --- a/drivers/spi/fsl_qspi.c +++ b/drivers/spi/fsl_qspi.c @@ -399,7 +399,7 @@ static inline void qspi_ahb_read(struct fsl_qspi_priv *priv, u8 *rxbuf, int len) qspi_write32(priv->flags, ®s->mcr, QSPI_MCR_CLR_RXF_MASK | QSPI_MCR_CLR_TXF_MASK | - QSPI_MCR_RESERVED_MASK | QSPI_MCR_END_CFD_LE); + mcr_reg); rx_addr = (void *)(uintptr_t)(priv->cur_amba_base + priv->sf_addr); /* Read out the data directly from the AHB buffer. */ @@ -429,6 +429,14 @@ static void qspi_enable_ddr_mode(struct fsl_qspi_priv *priv) reg |= BIT(29); qspi_write32(priv->flags, ®s->mcr, reg); + + /* Enable the TDH to 1 for some platforms like imx6ul, imx7d, etc + * These two bits are reserved on other platforms + */ + reg = qspi_read32(priv->flags, ®s->flshcr); + reg &= ~(BIT(17)); + reg |= BIT(16); + qspi_write32(priv->flags, ®s->flshcr, reg); } /* @@ -482,7 +490,7 @@ static void qspi_op_rdbank(struct fsl_qspi_priv *priv, u8 *rxbuf, u32 len) mcr_reg = qspi_read32(priv->flags, ®s->mcr); qspi_write32(priv->flags, ®s->mcr, QSPI_MCR_CLR_RXF_MASK | QSPI_MCR_CLR_TXF_MASK | - QSPI_MCR_RESERVED_MASK | QSPI_MCR_END_CFD_LE); + mcr_reg); qspi_write32(priv->flags, ®s->rbct, QSPI_RBCT_RXBRD_USEIPS); qspi_write32(priv->flags, ®s->sfar, priv->cur_amba_base); @@ -527,7 +535,7 @@ static void qspi_op_rdid(struct fsl_qspi_priv *priv, u32 *rxbuf, u32 len) mcr_reg = qspi_read32(priv->flags, ®s->mcr); qspi_write32(priv->flags, ®s->mcr, QSPI_MCR_CLR_RXF_MASK | QSPI_MCR_CLR_TXF_MASK | - QSPI_MCR_RESERVED_MASK | QSPI_MCR_END_CFD_LE); + mcr_reg); qspi_write32(priv->flags, ®s->rbct, QSPI_RBCT_RXBRD_USEIPS); qspi_write32(priv->flags, ®s->sfar, priv->cur_amba_base); @@ -573,7 +581,7 @@ static void qspi_op_read(struct fsl_qspi_priv *priv, u32 *rxbuf, u32 len) mcr_reg = qspi_read32(priv->flags, ®s->mcr); qspi_write32(priv->flags, ®s->mcr, QSPI_MCR_CLR_RXF_MASK | QSPI_MCR_CLR_TXF_MASK | - QSPI_MCR_RESERVED_MASK | QSPI_MCR_END_CFD_LE); + mcr_reg); qspi_write32(priv->flags, ®s->rbct, QSPI_RBCT_RXBRD_USEIPS); to_or_from = priv->sf_addr + priv->cur_amba_base; @@ -625,7 +633,7 @@ static void qspi_op_write(struct fsl_qspi_priv *priv, u8 *txbuf, u32 len) mcr_reg = qspi_read32(priv->flags, ®s->mcr); qspi_write32(priv->flags, ®s->mcr, QSPI_MCR_CLR_RXF_MASK | QSPI_MCR_CLR_TXF_MASK | - QSPI_MCR_RESERVED_MASK | QSPI_MCR_END_CFD_LE); + mcr_reg); qspi_write32(priv->flags, ®s->rbct, QSPI_RBCT_RXBRD_USEIPS); status_reg = 0; @@ -700,7 +708,7 @@ static void qspi_op_rdsr(struct fsl_qspi_priv *priv, void *rxbuf, u32 len) mcr_reg = qspi_read32(priv->flags, ®s->mcr); qspi_write32(priv->flags, ®s->mcr, QSPI_MCR_CLR_RXF_MASK | QSPI_MCR_CLR_TXF_MASK | - QSPI_MCR_RESERVED_MASK | QSPI_MCR_END_CFD_LE); + mcr_reg); qspi_write32(priv->flags, ®s->rbct, QSPI_RBCT_RXBRD_USEIPS); qspi_write32(priv->flags, ®s->sfar, priv->cur_amba_base); @@ -737,7 +745,7 @@ static void qspi_op_erase(struct fsl_qspi_priv *priv) mcr_reg = qspi_read32(priv->flags, ®s->mcr); qspi_write32(priv->flags, ®s->mcr, QSPI_MCR_CLR_RXF_MASK | QSPI_MCR_CLR_TXF_MASK | - QSPI_MCR_RESERVED_MASK | QSPI_MCR_END_CFD_LE); + mcr_reg); qspi_write32(priv->flags, ®s->rbct, QSPI_RBCT_RXBRD_USEIPS); to_or_from = priv->sf_addr + priv->cur_amba_base; @@ -900,15 +908,9 @@ static int fsl_qspi_probe(struct udevice *bus) return ret; } - mcr_val = qspi_read32(priv->flags, &priv->regs->mcr); - - /* Set endianness to LE for i.mx */ - if (IS_ENABLED(CONFIG_MX6) || IS_ENABLED(CONFIG_MX7)) - mcr_val = QSPI_MCR_END_CFD_LE; - qspi_write32(priv->flags, &priv->regs->mcr, QSPI_MCR_RESERVED_MASK | QSPI_MCR_MDIS_MASK | - (mcr_val & QSPI_MCR_END_CFD_MASK)); + QSPI_MCR_END_CFD_LE); qspi_cfg_smpr(priv, ~(QSPI_SMPR_FSDLY_MASK | QSPI_SMPR_DDRSMP_MASK | QSPI_SMPR_FSPHS_MASK | QSPI_SMPR_HSENA_MASK), 0);
On latest iMX platforms like iMX7D/iMX6UL/iMX8MQ, the QSPI controller is updated to have TDH field in FLSHCR register. According to reference manual, this TDH must be set to 1 when DDR_EN is set. Otherwise, the TX DDR delay logic won't be enabled. Another issue in DDR mode is the MCR register will be overwritten in every read/write/erase operation. This causes DDR_EN been cleared while TDH=1, then no clk2x output for TX data shift and all operations will fail. Signed-off-by: Ye Li <ye.li@nxp.com> --- drivers/spi/fsl_qspi.c | 30 ++++++++++++++++-------------- 1 file changed, 16 insertions(+), 14 deletions(-)