[U-Boot] [EXT] Re: [PATCH 1/6] spi: fsl_qspi: Fix DDR mode setting for latest iMX platforms
Ashish Kumar
ashish.kumar at nxp.com
Wed Aug 14 13:33:56 UTC 2019
> -----Original Message-----
> From: Schrempf Frieder <frieder.schrempf at kontron.de>
> Sent: Wednesday, August 14, 2019 5:41 PM
> To: Ashish Kumar <ashish.kumar at nxp.com>; 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: 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 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.
>
> 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 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, ®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 at 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
More information about the U-Boot
mailing list