Re: [U-Boot] [PATCH 1/6] spi: fsl_qspi: Fix DDR mode setting for latest iMX platforms

2019-08-14 Thread Schrempf Frieder
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 
> ---
>   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, >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, >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, >flshcr);
> + reg &= ~(BIT(17));
> + reg |= BIT(16);
> + qspi_write32(priv->flags, >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, >mcr);
>   qspi_write32(priv->flags, >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, >rbct, QSPI_RBCT_RXBRD_USEIPS);
>   
>   qspi_write32(priv->flags, >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, >mcr);
>   qspi_write32(priv->flags, >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, >rbct, QSPI_RBCT_RXBRD_USEIPS);
>   
>   qspi_write32(priv->flags, >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, >mcr);
>   qspi_write32(priv->flags, >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, >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, >mcr);
>   qspi_write32(priv->flags, >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, >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, >mcr);
>   qspi_write32(priv->flags, >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, >rbct, QSPI_RBCT_RXBRD_USEIPS);
>   
>   qspi_write32(priv->flags, >sfar, priv->cur_amba_base);
> @@ -737,7 +745,7 @@ static void qspi_op_erase(struct 

Re: [U-Boot] [PATCH 1/6] spi: fsl_qspi: Fix DDR mode setting for latest iMX platforms

2019-08-14 Thread Ye Li
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 
> ---
>  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, >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, >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, >flshcr);
> + reg &= ~(BIT(17));
> + reg |= BIT(16);
> + qspi_write32(priv->flags, >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, >mcr);
>   qspi_write32(priv->flags, >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, >rbct, QSPI_RBCT_RXBRD_USEIPS);
>  
>   qspi_write32(priv->flags, >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, >mcr);
>   qspi_write32(priv->flags, >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, >rbct, QSPI_RBCT_RXBRD_USEIPS);
>  
>   qspi_write32(priv->flags, >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, >mcr);
>   qspi_write32(priv->flags, >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, >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, >mcr);
>   qspi_write32(priv->flags, >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, >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, >mcr);
>   qspi_write32(priv->flags, >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, >rbct, QSPI_RBCT_RXBRD_USEIPS);
>  
>   qspi_write32(priv->flags, >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, >mcr);
>   qspi_write32(priv->flags, >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, >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, >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;
> -
>

[U-Boot] [PATCH 1/6] spi: fsl_qspi: Fix DDR mode setting for latest iMX platforms

2019-08-14 Thread Ye Li
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 
---
 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, >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, >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, >flshcr);
+   reg &= ~(BIT(17));
+   reg |= BIT(16);
+   qspi_write32(priv->flags, >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, >mcr);
qspi_write32(priv->flags, >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, >rbct, QSPI_RBCT_RXBRD_USEIPS);
 
qspi_write32(priv->flags, >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, >mcr);
qspi_write32(priv->flags, >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, >rbct, QSPI_RBCT_RXBRD_USEIPS);
 
qspi_write32(priv->flags, >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, >mcr);
qspi_write32(priv->flags, >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, >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, >mcr);
qspi_write32(priv->flags, >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, >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, >mcr);
qspi_write32(priv->flags, >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, >rbct, QSPI_RBCT_RXBRD_USEIPS);
 
qspi_write32(priv->flags, >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, >mcr);
qspi_write32(priv->flags, >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, >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, >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, >regs->mcr,
 QSPI_MCR_RESERVED_MASK | QSPI_MCR_MDIS_MASK |
-(mcr_val & QSPI_MCR_END_CFD_MASK));
+