+ Ye li
Hi Stefan,
> -----Original Message-----
> From: Stefan Roese <s...@denx.de>
> Sent: Monday, December 2, 2019 3:38 PM
> To: Kuldeep Singh <kuldeep.si...@nxp.com>; u-boot@lists.denx.de
> Cc: Priyanka Jain <priyanka.j...@nxp.com>; ja...@amarulasolutions.com;
> frieder.schre...@kontron.de
> Subject: [EXT] Re: [PATCH 0/8] Transition of fsl qspi driver to spi-mem
> framework
>
> Caution: EXT Email
>
> Hi Kuldeep,
>
> On 29.11.19 06:54, Kuldeep Singh wrote:
> > This entire patch series migrate freescale qspi driver to spi-mem
> framework.
> >
> > Patch 1 removes the old fsl qspi driver
> >
> > Patch 2 adds new qspi driver incorporating spi-mem framework which is
> > ported version of linux qspi driver. Initial port was done by Frieder.
> > Now, no more direct access to spi-nor memory is possible. Few changes
> > were introduced to prevent uboot crash such as to add complete flash
> > size to SFA1AD, SFA2AD, SFB1AD, SFB2AD based on chip select number
> > instead of 1k. Immediate effect was observed on pfe while using 1k
> > size as it access spi-nor memory directly. Using complete flash size
> resolves the crash but data read will not be valid.
> >
> > Patch 3 removes unused config options.
> >
> > Patch 4 moves FSL_QSPI config to defconfigs instead of defining in header
> files.
> > SPI_FLASH_SPANSION is enabled while disabling SPI_FLASH_BAR.
> >
> > Patch 5 removes unused num-cs property for imx platforms
> >
> > Patch 6 enables SPI_FLASH_SPANSION for ls1012 boards. SPI_FLASH_BAR
> is
> > no longer required and can be removed.
> >
> > Patch 7 move SPI_FLASH_SPANSION to defconfigs instead of header files.
> > While enabling SPI_FLASH_SPANSION config, also disable SPI_FLASH_BAR
> at the same time.
> >
> > Patch 8 updates the device-tree properties treewide for layerscape
> > boards by aligning it with linux device-tree properties.
>
> Many thanks for working on this. I've tested this patchset on a new i-
> MX6ULL/ULZ based board and noticed, that I still need to add this kind of
> patch to make it work on my platform:
>
> ---------------------------- drivers/spi/fsl_qspi.c
> ---------------------------- index
> 788fa0416f..0946f9d6d5 100644 @@ -44,6 +44,9 @@
> DECLARE_GLOBAL_DATA_PTR;
> #define QUADSPI_IPCR 0x08
> #define QUADSPI_IPCR_SEQID(x) ((x) << 24)
>
> +#define QUADSPI_FLSHCR 0x0c
> +#define QUADSPI_FLSHCR_TDH(x) ((x) << 16)
> +
> #define QUADSPI_BUF3CR 0x1c
> #define QUADSPI_BUF3CR_ALLMST_MASK BIT(31)
> #define QUADSPI_BUF3CR_ADATSZ(x) ((x) << 8)
> @@ -666,6 +669,16 @@ static int fsl_qspi_default_setup(struct fsl_qspi *q)
> QUADSPI_BUF3CR_ADATSZ(q->devtype_data->ahb_buf_size / 8),
> base + QUADSPI_BUF3CR);
>
> + /*
> + * Clear THD bits to configure SDR mode instead of DDR mode. This
> + * might be necessary, as the BootROM in some versions and on some
> + * SoCs sets these bits to 0x1 for DDR mode. But this driver needs
> + * it set to SDR mode instead.
> + */
> + reg = qspi_readl(q, base + QUADSPI_FLSHCR);
> + reg &= ~QUADSPI_FLSHCR_TDH(0x3);
> + qspi_writel(q, reg, base + QUADSPI_FLSHCR);
>
> This was suggested by Frieder and is also integrated in the Linux QSPI driver:
>
> Commit ID f6910679e17a ("spi: spi-fsl-qspi: Clear TDH bits in FLSHCR
> register")
>
> What is the status of SDR vs DDR mode in this driver? Is this driver version
> supposed to handle DDR mode correctly? If not, could you please integrate
> this patch (or Frieder's Linux patch) into your patchset so that the driver
> also
> works on i.MX6ULL/ULZ ?
I will port Frieder's patch for i.mx from Linux to Uboot.
Meanwhile, I am waiting for comments on other patches so that I'll incorporate
all changes in next version.
@Ye li
Please see attached email for reference.
Does Frieder's patch(f6910679e17a) resolves the fix proposed by you.
Also, can you please help in testing other i.mx platforms after including the
change.
Thanks
Kuldeep
>
> Thanks,
> Stefan
>
> > Frieder Schrempf (1):
> > imx: imx6sx: Remove unused 'num-cs' property
> >
> > Kuldeep Singh (7):
> > spi: Remove old freescale qspi driver
> > spi: Transform the FSL QuadSPI driver to use the SPI MEM API
> > treewide: Remove unused FSL QSPI config options
> > configs: ls1043a: Move CONFIG_FSL_QSPI to defconfig
> > configs: ls1012a: Enable SPI_FLASH_SPANSION in defconfig
> > configs: ls1046a: Move SPI_FLASH_SPANSION to defconfig
> > treewide: Update fsl qspi node dt properties as per spi-mem driver
> >
> > arch/arm/dts/fsl-ls1012a-frdm.dtsi | 5 +-
> > arch/arm/dts/fsl-ls1012a-qds.dtsi | 5 +-
> > arch/arm/dts/fsl-ls1012a-rdb.dtsi | 5 +-
> > arch/arm/dts/fsl-ls1012a.dtsi | 4 +-
> > arch/arm/dts/fsl-ls1043a-qds.dtsi | 5 +-
> > arch/arm/dts/fsl-ls1043a.dtsi | 6 +-
> > arch/arm/dts/fsl-ls1046a-frwy.dts | 5 +-
> > arch/arm/dts/fsl-ls1046a-qds.dtsi | 5 +-
> > arch/arm/dts/fsl-ls1046a-rdb.dts | 5 +-
> > arch/arm/dts/fsl-ls1046a.dtsi | 4 +-
> > arch/arm/dts/fsl-ls1088a-qds.dts | 5 +-
> > arch/arm/dts/fsl-ls1088a-rdb.dts | 5 +-
> > arch/arm/dts/fsl-ls1088a.dtsi | 2 +-
> > arch/arm/dts/fsl-ls2080a-qds.dts | 5 +-
> > arch/arm/dts/fsl-ls2080a.dtsi | 4 +-
> > arch/arm/dts/fsl-ls2088a-rdb-qspi.dts | 5 +-
> > arch/arm/dts/imx6sx-sabreauto-u-boot.dtsi | 2 -
> > arch/arm/dts/imx6sx-sdb-u-boot.dtsi | 2 -
> > arch/arm/dts/ls1021a-twr.dtsi | 5 +-
> > arch/arm/dts/ls1021a.dtsi | 6 +-
> > .../include/asm/arch-fsl-layerscape/config.h | 1 -
> > arch/arm/include/asm/arch-ls102xa/config.h | 1 -
> > configs/ls1012aqds_qspi_defconfig | 2 +-
> > configs/ls1012aqds_tfa_defconfig | 2 +-
> > configs/ls1012ardb_qspi_SECURE_BOOT_defconfig | 2 +-
> > configs/ls1012ardb_qspi_defconfig | 2 +-
> > configs/ls1012ardb_tfa_defconfig | 2 +-
> > configs/ls1043aqds_qspi_defconfig | 2 +-
> > configs/ls1043aqds_sdcard_qspi_defconfig | 2 +-
> > configs/ls1043aqds_tfa_SECURE_BOOT_defconfig | 2 +
> > configs/ls1043aqds_tfa_defconfig | 2 +-
> > configs/ls1046aqds_qspi_defconfig | 2 +-
> > configs/ls1046aqds_sdcard_qspi_defconfig | 2 +-
> > configs/ls1046aqds_tfa_SECURE_BOOT_defconfig | 1 +
> > configs/ls1046aqds_tfa_defconfig | 2 +-
> > configs/ls1046ardb_qspi_SECURE_BOOT_defconfig | 2 +-
> > configs/ls1046ardb_qspi_defconfig | 2 +-
> > configs/ls1046ardb_tfa_SECURE_BOOT_defconfig | 2 +-
> > configs/ls1046ardb_tfa_defconfig | 2 +-
> > drivers/spi/fsl_qspi.c | 1549 ++++++-----------
> > drivers/spi/fsl_qspi.h | 145 --
> > include/configs/ls1012a_common.h | 17 +-
> > include/configs/ls1012afrwy.h | 3 -
> > include/configs/ls1012ardb.h | 3 -
> > include/configs/ls1021aiot.h | 6 -
> > include/configs/ls1021aqds.h | 11 -
> > include/configs/ls1021atwr.h | 10 -
> > include/configs/ls1043aqds.h | 10 -
> > include/configs/ls1046afrwy.h | 9 -
> > include/configs/ls1046aqds.h | 19 -
> > include/configs/ls1046ardb.h | 20 -
> > include/configs/ls1088a_common.h | 6 -
> > include/configs/ls1088aqds.h | 8 -
> > include/configs/ls1088ardb.h | 18 -
> > include/configs/ls2080aqds.h | 2 -
> > include/configs/ls2080ardb.h | 8 +-
> > include/configs/mx6sxsabreauto.h | 6 -
> > include/configs/mx6sxsabresd.h | 11 -
> > include/configs/mx6ul_14x14_evk.h | 6 -
> > include/configs/mx6ullevk.h | 6 -
> > include/configs/mx7dsabresd.h | 8 -
> > include/configs/pcm052.h | 7 -
> > include/configs/vf610twr.h | 8 -
> > scripts/config_whitelist.txt | 5 -
> > 64 files changed, 632 insertions(+), 1394 deletions(-)
> > delete mode 100644 drivers/spi/fsl_qspi.h
> >
>
> Viele Grüße,
> Stefan
>
> --
> DENX Software Engineering GmbH, Managing Director: Wolfgang Denk
> HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany
> Phone: (+49)-8142-66989-51 Fax: (+49)-8142-66989-80 Email: s...@denx.de
--- Begin Message ---
Caution: EXT Email
Hi Frieder,
> Caution: EXT Email
>
> Hi Ye,
>
> On 14.08.19 12:08, Ye Li wrote:
>> On latest iMX platforms like iMX7D/iMX6UL/iMX8MQ, the QSPI controller
>> is updated to have TDH field in FLSHCR register. According to reference
>> manual, this TDH must be set to 1 when DDR_EN is set. Otherwise, the TX
>> DDR delay logic won't be enabled.
>
> This is actually an issue I have experienced myself. But in our case
> this behavior only happened on i.MX6ULL not on i.MX6UL. Either the QSPI
> controller hardware or the BootROM code changed when moving from UL to
> ULL. For details see: [1].
iMX6ULL has same QSPI controller revision with 6UL. I just checked their ROM
codes,
the TDH will be set if the DDR mode is enabled in QSPI configuration parameters.
It won't be cleared by ROM.
Have you compared other registers like LUT and MCR DDR_EN? Is the MCR DDR_EN
set
When the TDH is cleared?
>
>>
>> Another issue in DDR mode is the MCR register will be overwritten in
>> every read/write/erase operation. This causes DDR_EN been cleared while
>> TDH=1, then no clk2x output for TX data shift and all operations will
>> fail.
>
> The best way to fix all of these things (also the ones in the other
> patches) would be to fix them in Linux and port the driver from Linux to
> U-Boot. Actually I've already done most of the porting [2], but didn't
> have the time to finish it recently. It probably needs some rebasing and
> testing.
>
> Could you port your fixes to the Linux driver and submit them to linux-mtd?
Our downstream kernel has fixed the issue, it is similar as this u-boot fix.
TDH must be set along with DDR_EN. I'm not this kernel driver owner, I will
check
with him about the upstream of this fix.
Best regards,
Ye Li
>
> Thanks
> Frieder
>
> [1]
> https://eur01.safelinks.protection.outlook.com/?url=https%3A%2F%2Fcommunity.nxp.com%2Fthread%2F507260&data=02%7C01%7CAshish.Kumar%40nxp.com%7C4652d15e5aa24608cb6e08d720c04bc2%7C686ea1d3bc2b4c6fa92cd99c5c301635%7C0%7C0%7C637013882596856795&sdata=S%2FQhjQxMrFMC5FqUgH0GLhxvOJdqPSrwmRKg1%2BX0WhI%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%7C4652d15e5aa24608cb6e08d720c04bc2%7C686ea1d3bc2b4c6fa92cd99c5c301635%7C0%7C0%7C637013882596856795&sdata=p4evguAr5pm5txLtGyAcWDISycCbecJMC6TVnR09Mx4%3D&reserved=0
>
>>
>> Signed-off-by: Ye Li <ye...@nxp.com>
>> ---
>> drivers/spi/fsl_qspi.c | 30 ++++++++++++++++--------------
>> 1 file changed, 16 insertions(+), 14 deletions(-)
>>
>> diff --git a/drivers/spi/fsl_qspi.c b/drivers/spi/fsl_qspi.c
>> index 41abe19..8845986 100644
>> --- a/drivers/spi/fsl_qspi.c
>> +++ b/drivers/spi/fsl_qspi.c
>> @@ -399,7 +399,7 @@ static inline void qspi_ahb_read(struct fsl_qspi_priv
>> *priv, u8 *rxbuf, int len)
>>
>> qspi_write32(priv->flags, ®s->mcr,
>> QSPI_MCR_CLR_RXF_MASK | QSPI_MCR_CLR_TXF_MASK |
>> - QSPI_MCR_RESERVED_MASK | QSPI_MCR_END_CFD_LE);
>> + mcr_reg);
>>
>> rx_addr = (void *)(uintptr_t)(priv->cur_amba_base + priv->sf_addr);
>> /* Read out the data directly from the AHB buffer. */
>> @@ -429,6 +429,14 @@ static void qspi_enable_ddr_mode(struct fsl_qspi_priv
>> *priv)
>> reg |= BIT(29);
>>
>> qspi_write32(priv->flags, ®s->mcr, reg);
>> +
>> + /* Enable the TDH to 1 for some platforms like imx6ul, imx7d, etc
>> + * These two bits are reserved on other platforms
>> + */
>> + reg = qspi_read32(priv->flags, ®s->flshcr);
>> + reg &= ~(BIT(17));
>> + reg |= BIT(16);
>> + qspi_write32(priv->flags, ®s->flshcr, reg);
>> }
>>
>> /*
>> @@ -482,7 +490,7 @@ static void qspi_op_rdbank(struct fsl_qspi_priv *priv,
>> u8 *rxbuf, u32 len)
>> mcr_reg = qspi_read32(priv->flags, ®s->mcr);
>> qspi_write32(priv->flags, ®s->mcr,
>> QSPI_MCR_CLR_RXF_MASK | QSPI_MCR_CLR_TXF_MASK |
>> - QSPI_MCR_RESERVED_MASK | QSPI_MCR_END_CFD_LE);
>> + mcr_reg);
>> qspi_write32(priv->flags, ®s->rbct, QSPI_RBCT_RXBRD_USEIPS);
>>
>> qspi_write32(priv->flags, ®s->sfar, priv->cur_amba_base);
>> @@ -527,7 +535,7 @@ static void qspi_op_rdid(struct fsl_qspi_priv *priv, u32
>> *rxbuf, u32 len)
>> mcr_reg = qspi_read32(priv->flags, ®s->mcr);
>> qspi_write32(priv->flags, ®s->mcr,
>> QSPI_MCR_CLR_RXF_MASK | QSPI_MCR_CLR_TXF_MASK |
>> - QSPI_MCR_RESERVED_MASK | QSPI_MCR_END_CFD_LE);
>> + mcr_reg);
>> qspi_write32(priv->flags, ®s->rbct, QSPI_RBCT_RXBRD_USEIPS);
>>
>> qspi_write32(priv->flags, ®s->sfar, priv->cur_amba_base);
>> @@ -573,7 +581,7 @@ static void qspi_op_read(struct fsl_qspi_priv *priv, u32
>> *rxbuf, u32 len)
>> mcr_reg = qspi_read32(priv->flags, ®s->mcr);
>> qspi_write32(priv->flags, ®s->mcr,
>> QSPI_MCR_CLR_RXF_MASK | QSPI_MCR_CLR_TXF_MASK |
>> - QSPI_MCR_RESERVED_MASK | QSPI_MCR_END_CFD_LE);
>> + mcr_reg);
>> qspi_write32(priv->flags, ®s->rbct, QSPI_RBCT_RXBRD_USEIPS);
>>
>> to_or_from = priv->sf_addr + priv->cur_amba_base;
>> @@ -625,7 +633,7 @@ static void qspi_op_write(struct fsl_qspi_priv *priv, u8
>> *txbuf, u32 len)
>> mcr_reg = qspi_read32(priv->flags, ®s->mcr);
>> qspi_write32(priv->flags, ®s->mcr,
>> QSPI_MCR_CLR_RXF_MASK | QSPI_MCR_CLR_TXF_MASK |
>> - QSPI_MCR_RESERVED_MASK | QSPI_MCR_END_CFD_LE);
>> + mcr_reg);
>> qspi_write32(priv->flags, ®s->rbct, QSPI_RBCT_RXBRD_USEIPS);
>>
>> status_reg = 0;
>> @@ -700,7 +708,7 @@ static void qspi_op_rdsr(struct fsl_qspi_priv *priv,
>> void *rxbuf, u32 len)
>> mcr_reg = qspi_read32(priv->flags, ®s->mcr);
>> qspi_write32(priv->flags, ®s->mcr,
>> QSPI_MCR_CLR_RXF_MASK | QSPI_MCR_CLR_TXF_MASK |
>> - QSPI_MCR_RESERVED_MASK | QSPI_MCR_END_CFD_LE);
>> + mcr_reg);
>> qspi_write32(priv->flags, ®s->rbct, QSPI_RBCT_RXBRD_USEIPS);
>>
>> qspi_write32(priv->flags, ®s->sfar, priv->cur_amba_base);
>> @@ -737,7 +745,7 @@ static void qspi_op_erase(struct fsl_qspi_priv *priv)
>> mcr_reg = qspi_read32(priv->flags, ®s->mcr);
>> qspi_write32(priv->flags, ®s->mcr,
>> QSPI_MCR_CLR_RXF_MASK | QSPI_MCR_CLR_TXF_MASK |
>> - QSPI_MCR_RESERVED_MASK | QSPI_MCR_END_CFD_LE);
>> + mcr_reg);
>> qspi_write32(priv->flags, ®s->rbct, QSPI_RBCT_RXBRD_USEIPS);
>>
>> to_or_from = priv->sf_addr + priv->cur_amba_base;
>> @@ -900,15 +908,9 @@ static int fsl_qspi_probe(struct udevice *bus)
>> return ret;
>> }
>>
>> - mcr_val = qspi_read32(priv->flags, &priv->regs->mcr);
>> -
>> - /* Set endianness to LE for i.mx */
>> - if (IS_ENABLED(CONFIG_MX6) || IS_ENABLED(CONFIG_MX7))
>> - mcr_val = QSPI_MCR_END_CFD_LE;
>> -
>> qspi_write32(priv->flags, &priv->regs->mcr,
>> QSPI_MCR_RESERVED_MASK | QSPI_MCR_MDIS_MASK |
>> - (mcr_val & QSPI_MCR_END_CFD_MASK));
>> + QSPI_MCR_END_CFD_LE);
>>
>> qspi_cfg_smpr(priv, ~(QSPI_SMPR_FSDLY_MASK | QSPI_SMPR_DDRSMP_MASK |
>> QSPI_SMPR_FSPHS_MASK | QSPI_SMPR_HSENA_MASK), 0);
>>
>
_______________________________________________
U-Boot mailing list
U-Boot@lists.denx.de
https://eur01.safelinks.protection.outlook.com/?url=https%3A%2F%2Flists.denx.de%2Flistinfo%2Fu-boot&data=02%7C01%7CAshish.Kumar%40nxp.com%7C4652d15e5aa24608cb6e08d720c04bc2%7C686ea1d3bc2b4c6fa92cd99c5c301635%7C0%7C0%7C637013882596856795&sdata=QNGgcjpcv%2Fa5DtdY3DTaWq%2Fv3BSwPNOSLa%2B3oXPmhRo%3D&reserved=0
--- End Message ---
_______________________________________________
U-Boot mailing list
U-Boot@lists.denx.de
https://lists.denx.de/listinfo/u-boot
