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

Schrempf Frieder frieder.schrempf at kontron.de
Wed Aug 14 12:07:33 UTC 2019


Hi Ashish,

On 14.08.19 14:02, Ashish Kumar wrote:
> 
> 
>> -----Original Message-----
>> From: U-Boot <u-boot-bounces at lists.denx.de> On Behalf Of Schrempf Frieder
>> Sent: Wednesday, August 14, 2019 5:07 PM
>> To: Ye Li <ye.li at nxp.com>; jagan at amarulasolutions.com
>> Cc: Fabio Estevam <fabio.estevam at nxp.com>; u-boot at lists.denx.de; dl-
>> uboot-imx <uboot-imx at 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 at 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 at 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


More information about the U-Boot mailing list