[U-Boot,1/6] spi: fsl_qspi: Fix DDR mode setting for latest iMX platforms
diff mbox series

Message ID 1565777311-1752-1-git-send-email-ye.li@nxp.com
State New
Delegated to: Stefano Babic
Headers show
Series
  • [U-Boot,1/6] spi: fsl_qspi: Fix DDR mode setting for latest iMX platforms
Related show

Commit Message

Ye Li Aug. 14, 2019, 10:08 a.m. UTC
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(-)

Comments

Ye Li Aug. 14, 2019, 10:45 a.m. UTC | #1
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, &regs->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, &regs->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, &regs->flshcr);
> +	reg &= ~(BIT(17));
> +	reg |= BIT(16);
> +	qspi_write32(priv->flags, &regs->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, &regs->mcr);
>  	qspi_write32(priv->flags, &regs->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, &regs->rbct, QSPI_RBCT_RXBRD_USEIPS);
>  
>  	qspi_write32(priv->flags, &regs->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, &regs->mcr);
>  	qspi_write32(priv->flags, &regs->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, &regs->rbct, QSPI_RBCT_RXBRD_USEIPS);
>  
>  	qspi_write32(priv->flags, &regs->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, &regs->mcr);
>  	qspi_write32(priv->flags, &regs->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, &regs->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, &regs->mcr);
>  	qspi_write32(priv->flags, &regs->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, &regs->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, &regs->mcr);
>  	qspi_write32(priv->flags, &regs->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, &regs->rbct, QSPI_RBCT_RXBRD_USEIPS);
>  
>  	qspi_write32(priv->flags, &regs->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, &regs->mcr);
>  	qspi_write32(priv->flags, &regs->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, &regs->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);
>
Schrempf Frieder Aug. 14, 2019, 11:37 a.m. UTC | #2
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, &regs->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, &regs->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, &regs->flshcr);
> +	reg &= ~(BIT(17));
> +	reg |= BIT(16);
> +	qspi_write32(priv->flags, &regs->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, &regs->mcr);
>   	qspi_write32(priv->flags, &regs->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, &regs->rbct, QSPI_RBCT_RXBRD_USEIPS);
>   
>   	qspi_write32(priv->flags, &regs->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, &regs->mcr);
>   	qspi_write32(priv->flags, &regs->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, &regs->rbct, QSPI_RBCT_RXBRD_USEIPS);
>   
>   	qspi_write32(priv->flags, &regs->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, &regs->mcr);
>   	qspi_write32(priv->flags, &regs->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, &regs->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, &regs->mcr);
>   	qspi_write32(priv->flags, &regs->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, &regs->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, &regs->mcr);
>   	qspi_write32(priv->flags, &regs->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, &regs->rbct, QSPI_RBCT_RXBRD_USEIPS);
>   
>   	qspi_write32(priv->flags, &regs->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, &regs->mcr);
>   	qspi_write32(priv->flags, &regs->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, &regs->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);
>
Ashish Kumar Aug. 14, 2019, 12:02 p.m. UTC | #3
> -----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&amp;data=02%7C01%7CAshish.Kumar
> %40nxp.com%7C8882d5662295468a45b008d720abd98c%7C686ea1d3bc2b4c
> 6fa92cd99c5c301635%7C0%7C0%7C637013794778063281&amp;sdata=Za7LB
> 6RyXAHszPfiEMLDb%2FvVNSTQJwxHFtiapmNi3Co%3D&amp;reserved=0
> [2]
> https://eur01.safelinks.protection.outlook.com/?url=https%3A%2F%2Fgithub
> .com%2Ffschrempf%2Fu-
> boot%2Fcommits%2Ffsl_qspi_spimem_port&amp;data=02%7C01%7CAshish.
> Kumar%40nxp.com%7C8882d5662295468a45b008d720abd98c%7C686ea1d3
> bc2b4c6fa92cd99c5c301635%7C0%7C0%7C637013794778063281&amp;sdata
> =kYDjrs2a9DdL8QLFPORfHsMSWvgUxhSTgNC3WLziu7Y%3D&amp;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, &regs->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, &regs->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, &regs->flshcr);
> > +     reg &= ~(BIT(17));
> > +     reg |= BIT(16);
> > +     qspi_write32(priv->flags, &regs->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, &regs->mcr);
> >       qspi_write32(priv->flags, &regs->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, &regs->rbct, QSPI_RBCT_RXBRD_USEIPS);
> >
> >       qspi_write32(priv->flags, &regs->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, &regs->mcr);
> >       qspi_write32(priv->flags, &regs->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, &regs->rbct, QSPI_RBCT_RXBRD_USEIPS);
> >
> >       qspi_write32(priv->flags, &regs->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, &regs->mcr);
> >       qspi_write32(priv->flags, &regs->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, &regs->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, &regs->mcr);
> >       qspi_write32(priv->flags, &regs->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, &regs->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, &regs->mcr);
> >       qspi_write32(priv->flags, &regs->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, &regs->rbct, QSPI_RBCT_RXBRD_USEIPS);
> >
> >       qspi_write32(priv->flags, &regs->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, &regs->mcr);
> >       qspi_write32(priv->flags, &regs->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, &regs->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&amp;data=02%7C01%7CAshish.Kumar%40nxp.com%7C8882d56622954
> 68a45b008d720abd98c%7C686ea1d3bc2b4c6fa92cd99c5c301635%7C0%7C0
> %7C637013794778063281&amp;sdata=l1Pb4yEQABBx5ave4Lb7jGSVRJ1TG68
> KMkl7WpSSwRc%3D&amp;reserved=0
Schrempf Frieder Aug. 14, 2019, 12:07 p.m. UTC | #4
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&amp;data=02%7C01%7CAshish.Kumar
>> %40nxp.com%7C8882d5662295468a45b008d720abd98c%7C686ea1d3bc2b4c
>> 6fa92cd99c5c301635%7C0%7C0%7C637013794778063281&amp;sdata=Za7LB
>> 6RyXAHszPfiEMLDb%2FvVNSTQJwxHFtiapmNi3Co%3D&amp;reserved=0
>> [2]
>> https://eur01.safelinks.protection.outlook.com/?url=https%3A%2F%2Fgithub
>> .com%2Ffschrempf%2Fu-
>> boot%2Fcommits%2Ffsl_qspi_spimem_port&amp;data=02%7C01%7CAshish.
>> Kumar%40nxp.com%7C8882d5662295468a45b008d720abd98c%7C686ea1d3
>> bc2b4c6fa92cd99c5c301635%7C0%7C0%7C637013794778063281&amp;sdata
>> =kYDjrs2a9DdL8QLFPORfHsMSWvgUxhSTgNC3WLziu7Y%3D&amp;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, &regs->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, &regs->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, &regs->flshcr);
>>> +     reg &= ~(BIT(17));
>>> +     reg |= BIT(16);
>>> +     qspi_write32(priv->flags, &regs->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, &regs->mcr);
>>>        qspi_write32(priv->flags, &regs->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, &regs->rbct, QSPI_RBCT_RXBRD_USEIPS);
>>>
>>>        qspi_write32(priv->flags, &regs->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, &regs->mcr);
>>>        qspi_write32(priv->flags, &regs->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, &regs->rbct, QSPI_RBCT_RXBRD_USEIPS);
>>>
>>>        qspi_write32(priv->flags, &regs->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, &regs->mcr);
>>>        qspi_write32(priv->flags, &regs->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, &regs->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, &regs->mcr);
>>>        qspi_write32(priv->flags, &regs->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, &regs->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, &regs->mcr);
>>>        qspi_write32(priv->flags, &regs->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, &regs->rbct, QSPI_RBCT_RXBRD_USEIPS);
>>>
>>>        qspi_write32(priv->flags, &regs->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, &regs->mcr);
>>>        qspi_write32(priv->flags, &regs->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, &regs->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&amp;data=02%7C01%7CAshish.Kumar%40nxp.com%7C8882d56622954
>> 68a45b008d720abd98c%7C686ea1d3bc2b4c6fa92cd99c5c301635%7C0%7C0
>> %7C637013794778063281&amp;sdata=l1Pb4yEQABBx5ave4Lb7jGSVRJ1TG68
>> KMkl7WpSSwRc%3D&amp;reserved=0
Schrempf Frieder Aug. 14, 2019, 12:10 p.m. UTC | #5
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&amp;data=02%7C01%7CAshish.Kumar
>>> %40nxp.com%7C8882d5662295468a45b008d720abd98c%7C686ea1d3bc2b4c
>>> 6fa92cd99c5c301635%7C0%7C0%7C637013794778063281&amp;sdata=Za7LB
>>> 6RyXAHszPfiEMLDb%2FvVNSTQJwxHFtiapmNi3Co%3D&amp;reserved=0
>>> [2]
>>> https://eur01.safelinks.protection.outlook.com/?url=https%3A%2F%2Fgithub
>>> .com%2Ffschrempf%2Fu-
>>> boot%2Fcommits%2Ffsl_qspi_spimem_port&amp;data=02%7C01%7CAshish.
>>> Kumar%40nxp.com%7C8882d5662295468a45b008d720abd98c%7C686ea1d3
>>> bc2b4c6fa92cd99c5c301635%7C0%7C0%7C637013794778063281&amp;sdata
>>> =kYDjrs2a9DdL8QLFPORfHsMSWvgUxhSTgNC3WLziu7Y%3D&amp;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, &regs->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, &regs->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, &regs->flshcr);
>>>> +     reg &= ~(BIT(17));
>>>> +     reg |= BIT(16);
>>>> +     qspi_write32(priv->flags, &regs->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, &regs->mcr);
>>>>        qspi_write32(priv->flags, &regs->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, &regs->rbct, QSPI_RBCT_RXBRD_USEIPS);
>>>>
>>>>        qspi_write32(priv->flags, &regs->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, &regs->mcr);
>>>>        qspi_write32(priv->flags, &regs->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, &regs->rbct, QSPI_RBCT_RXBRD_USEIPS);
>>>>
>>>>        qspi_write32(priv->flags, &regs->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, &regs->mcr);
>>>>        qspi_write32(priv->flags, &regs->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, &regs->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, &regs->mcr);
>>>>        qspi_write32(priv->flags, &regs->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, &regs->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, &regs->mcr);
>>>>        qspi_write32(priv->flags, &regs->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, &regs->rbct, QSPI_RBCT_RXBRD_USEIPS);
>>>>
>>>>        qspi_write32(priv->flags, &regs->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, &regs->mcr);
>>>>        qspi_write32(priv->flags, &regs->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, &regs->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&amp;data=02%7C01%7CAshish.Kumar%40nxp.com%7C8882d56622954
>>> 68a45b008d720abd98c%7C686ea1d3bc2b4c6fa92cd99c5c301635%7C0%7C0
>>> %7C637013794778063281&amp;sdata=l1Pb4yEQABBx5ave4Lb7jGSVRJ1TG68
>>> KMkl7WpSSwRc%3D&amp;reserved=0
Ashish Kumar Aug. 14, 2019, 1:33 p.m. UTC | #6
> -----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&amp;data=02%7C01%7CAshish.Kumar
> >>>
> %40nxp.com%7C8882d5662295468a45b008d720abd98c%7C686ea1d3bc2b4c
> >>>
> 6fa92cd99c5c301635%7C0%7C0%7C637013794778063281&amp;sdata=Za7LB
> >>> 6RyXAHszPfiEMLDb%2FvVNSTQJwxHFtiapmNi3Co%3D&amp;reserved=0
> >>> [2]
> >>> https://github
> >>> .com%2Ffschrempf%2Fu-
> >>>
> boot%2Fcommits%2Ffsl_qspi_spimem_port&amp;data=02%7C01%7CAshish.
> >>>
> Kumar%40nxp.com%7C8882d5662295468a45b008d720abd98c%7C686ea1d3
> >>>
> bc2b4c6fa92cd99c5c301635%7C0%7C0%7C637013794778063281&amp;sdata
> >>>
> =kYDjrs2a9DdL8QLFPORfHsMSWvgUxhSTgNC3WLziu7Y%3D&amp;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, &regs->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, &regs->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, &regs->flshcr);
> >>>> +     reg &= ~(BIT(17));
> >>>> +     reg |= BIT(16);
> >>>> +     qspi_write32(priv->flags, &regs->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, &regs->mcr);
> >>>>        qspi_write32(priv->flags, &regs->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, &regs->rbct,
> >>>> QSPI_RBCT_RXBRD_USEIPS);
> >>>>
> >>>>        qspi_write32(priv->flags, &regs->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, &regs->mcr);
> >>>>        qspi_write32(priv->flags, &regs->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, &regs->rbct,
> >>>> QSPI_RBCT_RXBRD_USEIPS);
> >>>>
> >>>>        qspi_write32(priv->flags, &regs->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, &regs->mcr);
> >>>>        qspi_write32(priv->flags, &regs->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, &regs->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, &regs->mcr);
> >>>>        qspi_write32(priv->flags, &regs->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, &regs->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, &regs->mcr);
> >>>>        qspi_write32(priv->flags, &regs->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, &regs->rbct,
> >>>> QSPI_RBCT_RXBRD_USEIPS);
> >>>>
> >>>>        qspi_write32(priv->flags, &regs->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, &regs->mcr);
> >>>>        qspi_write32(priv->flags, &regs->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, &regs->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&amp;data=02%7C01%7CAshish.Kumar%40nxp.com%7C8882d56622954
> >>>
> 68a45b008d720abd98c%7C686ea1d3bc2b4c6fa92cd99c5c301635%7C0%7C0
> >>>
> %7C637013794778063281&amp;sdata=l1Pb4yEQABBx5ave4Lb7jGSVRJ1TG68
> >>> KMkl7WpSSwRc%3D&amp;reserved=0
Ye Li Aug. 14, 2019, 2:03 p.m. UTC | #7
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&amp;data=02%7C01%7Cye.li%40nxp.com%7Cc0caf3432e8e4a136b5208d720abd020%7C686ea1d3bc2b4c6fa92cd99c5c301635%7C0%7C0%7C637013794616656270&amp;sdata=hcyPpSlxU59NVtsHQdnGksMcu%2BQ8DzOJJJ2lnUGd7uQ%3D&amp;reserved=0
> [2] https://eur01.safelinks.protection.outlook.com/?url=https%3A%2F%2Fgithub.com%2Ffschrempf%2Fu-boot%2Fcommits%2Ffsl_qspi_spimem_port&amp;data=02%7C01%7Cye.li%40nxp.com%7Cc0caf3432e8e4a136b5208d720abd020%7C686ea1d3bc2b4c6fa92cd99c5c301635%7C0%7C0%7C637013794616656270&amp;sdata=eqj4L2daAfOMA6oGbmPCPWHuy%2B6NbizQihJ0QeAcIxY%3D&amp;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, &regs->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, &regs->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, &regs->flshcr);
>> +     reg &= ~(BIT(17));
>> +     reg |= BIT(16);
>> +     qspi_write32(priv->flags, &regs->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, &regs->mcr);
>>       qspi_write32(priv->flags, &regs->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, &regs->rbct, QSPI_RBCT_RXBRD_USEIPS);
>>
>>       qspi_write32(priv->flags, &regs->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, &regs->mcr);
>>       qspi_write32(priv->flags, &regs->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, &regs->rbct, QSPI_RBCT_RXBRD_USEIPS);
>>
>>       qspi_write32(priv->flags, &regs->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, &regs->mcr);
>>       qspi_write32(priv->flags, &regs->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, &regs->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, &regs->mcr);
>>       qspi_write32(priv->flags, &regs->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, &regs->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, &regs->mcr);
>>       qspi_write32(priv->flags, &regs->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, &regs->rbct, QSPI_RBCT_RXBRD_USEIPS);
>>
>>       qspi_write32(priv->flags, &regs->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, &regs->mcr);
>>       qspi_write32(priv->flags, &regs->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, &regs->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);
>>
>

Patch
diff mbox series

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, &regs->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, &regs->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, &regs->flshcr);
+	reg &= ~(BIT(17));
+	reg |= BIT(16);
+	qspi_write32(priv->flags, &regs->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, &regs->mcr);
 	qspi_write32(priv->flags, &regs->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, &regs->rbct, QSPI_RBCT_RXBRD_USEIPS);
 
 	qspi_write32(priv->flags, &regs->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, &regs->mcr);
 	qspi_write32(priv->flags, &regs->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, &regs->rbct, QSPI_RBCT_RXBRD_USEIPS);
 
 	qspi_write32(priv->flags, &regs->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, &regs->mcr);
 	qspi_write32(priv->flags, &regs->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, &regs->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, &regs->mcr);
 	qspi_write32(priv->flags, &regs->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, &regs->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, &regs->mcr);
 	qspi_write32(priv->flags, &regs->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, &regs->rbct, QSPI_RBCT_RXBRD_USEIPS);
 
 	qspi_write32(priv->flags, &regs->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, &regs->mcr);
 	qspi_write32(priv->flags, &regs->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, &regs->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);