| Message ID | 20180926220739.620-1-lukma@denx.de (mailing list archive) |
|---|---|
| Headers | show |
| Series | mtd: fsl: quadspi: Fixes for fsl-quadspi.c driver (vybrid HW) | expand |
Hi Lukasz, On Thu, 27 Sep 2018 00:07:30 +0200 Lukasz Majewski <lukma@denx.de> wrote: > Please find following set of patches, which provide improved behaviour > of the fsl-quadspi.c driver on Vybrid vf610 HW. > > Below code is based on previous work done by Albert ARIBAUD: > https://patchwork.ozlabs.org/patch/675401/ > > I've cleaned up the code a bit, make separate patches, exclude not > needed parts and port it to v4.19-rc3. > > After applying those patches, the quadspi controller on vf610 works > with UBI/UBIFS. > The problem is with some corner case writes to raw MTD device > (like writing 1023B with single byte writes). Those fail sometimes. > > Regression tests can be found in the following repository: > https://github.com/lmajewski/tests-spi/tree/master/tests > > (Please read README.txt). > > The NXP's community thread regarding HW issues in this block on > the vf610: > https://community.nxp.com/thread/485139 > > Maybe those patches will be helpful with the new, work in progress > driver for spi-fsl-qspi.c Talking about that, can you try to port your fixes on top of Frieder's patchset? I'm pretty sure some bug fixes are irrelevant after the migration to spi-mem (patch 1, 3, 4, 5, 6, 7 and 9 should be dropped I think). > > Lukasz Majewski (9): > Revert "mtd: fsl-quadspi: Rename SEQID_QUAD_READ to SEQID_READ" > mtd: qspi: Provide quirk to read only half of RX buffer (NXP's vybrid) > mtd: spi: Do not setup the default seqid as we got it set for DUAL and > QUAD > mtd: spi: Modify the HW capability mask according to supported RX > lanes > mtd: spi: Provide LUT entry to perform DUAL read > mtd: spi: Enhance the fsl_qspi_read() method to support DUAL and QUAD > mtd: spi: Add SPI_NOR_DUAL_READ property for the 'n25q128a13' Micron > memory > mtd: spi: Allocate memory corresponding to maximal fsl-quadspi.c > controller area > mtd: spi: Skip reading the Serial Flash Discoverable Parameters
On Sat, 29 Sep 2018 00:03:59 +0200 Boris Brezillon <boris.brezillon@bootlin.com> wrote: > Hi Lukasz, > > On Thu, 27 Sep 2018 00:07:30 +0200 > Lukasz Majewski <lukma@denx.de> wrote: > > > Please find following set of patches, which provide improved > > behaviour of the fsl-quadspi.c driver on Vybrid vf610 HW. > > > > Below code is based on previous work done by Albert ARIBAUD: > > https://patchwork.ozlabs.org/patch/675401/ > > > > I've cleaned up the code a bit, make separate patches, exclude not > > needed parts and port it to v4.19-rc3. > > > > After applying those patches, the quadspi controller on vf610 works > > with UBI/UBIFS. > > The problem is with some corner case writes to raw MTD device > > (like writing 1023B with single byte writes). Those fail sometimes. > > > > Regression tests can be found in the following repository: > > https://github.com/lmajewski/tests-spi/tree/master/tests > > > > (Please read README.txt). > > > > The NXP's community thread regarding HW issues in this block on > > the vf610: > > https://community.nxp.com/thread/485139 > > > > Maybe those patches will be helpful with the new, work in progress > > driver for spi-fsl-qspi.c > > Talking about that, can you try to port your fixes on top of Frieder's > patchset? I'm pretty sure some bug fixes are irrelevant after the > migration to spi-mem (patch 1, 3, 4, 5, 6, 7 and 9 should be dropped I > think). The problem is that Frieder's patch is using IP command mode for transfers smaller than AHB RX fifo size. This according to the comment in the current driver is broken in HW for Vybrid (so this is a regression). I'm also wondering if other users of vf610 based boards experience issues with QSPI? In my case, after running the UBI/UBIFS tests (on the original and new driver without those "fixes") I cannot mount the volume after creation as the header is wrongly read. > > > > > Lukasz Majewski (9): > > Revert "mtd: fsl-quadspi: Rename SEQID_QUAD_READ to SEQID_READ" > > mtd: qspi: Provide quirk to read only half of RX buffer (NXP's > > vybrid) mtd: spi: Do not setup the default seqid as we got it set > > for DUAL and QUAD > > mtd: spi: Modify the HW capability mask according to supported RX > > lanes > > mtd: spi: Provide LUT entry to perform DUAL read > > mtd: spi: Enhance the fsl_qspi_read() method to support DUAL and > > QUAD mtd: spi: Add SPI_NOR_DUAL_READ property for the 'n25q128a13' > > Micron memory > > mtd: spi: Allocate memory corresponding to maximal fsl-quadspi.c > > controller area > > mtd: spi: Skip reading the Serial Flash Discoverable Parameters Best regards, Lukasz Majewski -- DENX Software Engineering GmbH, Managing Director: Wolfgang Denk HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany Phone: (+49)-8142-66989-10 Fax: (+49)-8142-66989-80 Email: wd@denx.de
Hi Lukasz, On Sat, 29 Sep 2018 23:02:40 +0200 Lukasz Majewski <lukma@denx.de> wrote: > > Talking about that, can you try to port your fixes on top of Frieder's > > patchset? I'm pretty sure some bug fixes are irrelevant after the > > migration to spi-mem (patch 1, 3, 4, 5, 6, 7 and 9 should be dropped I > > think). > > The problem is that Frieder's patch is using IP command mode for > transfers smaller than AHB RX fifo size. > This according to the comment in the current driver is broken in HW for > Vybrid (so this is a regression). Why not fixing that in the new driver? > > I'm also wondering if other users of vf610 based boards experience > issues with QSPI? Are you sure the 4 I/O lines are wired on your design? Anyway, if it's a bug that only hurts vf610, you should mask quad modes in ->hwcaps (or patch fsl_qspi_supports_op() in the new driver), not change the SPI NOR definition. > > In my case, after running the UBI/UBIFS tests (on the original and > new driver without those "fixes") I cannot mount the volume after > creation as the header is wrongly read. I'm not denying this fact, I'm just saying, now that you've found where the issue comes from, you can also port the fix to the new driver. Regards, Boris
Hi Boris, > Hi Lukasz, > > On Sat, 29 Sep 2018 23:02:40 +0200 > Lukasz Majewski <lukma@denx.de> wrote: > > > > Talking about that, can you try to port your fixes on top of > > > Frieder's patchset? I'm pretty sure some bug fixes are irrelevant > > > after the migration to spi-mem (patch 1, 3, 4, 5, 6, 7 and 9 > > > should be dropped I think). > > > > The problem is that Frieder's patch is using IP command mode for > > transfers smaller than AHB RX fifo size. > > This according to the comment in the current driver is broken in HW > > for Vybrid (so this is a regression). > > Why not fixing that in the new driver? I did not say that I will not fix it in the new driver :-) The problem is that I'm waiting for NXP's community reply - to get some more info regarding the bug. > > > > > I'm also wondering if other users of vf610 based boards experience > > issues with QSPI? > > Are you sure the 4 I/O lines are wired on your design? This is a somewhat special case. There are two identical SPI-NOR memories: one with QUAD lines connected and second only for DUAL due to HW design decision. > Anyway, if it's > a bug that only hurts vf610, you should mask quad modes in ->hwcaps > (or patch fsl_qspi_supports_op() in the new driver), not change the > SPI NOR definition. Yes, this was also pointed out by Cyrille - and yes, I do agree that I shouldn't mask it. > > > > > In my case, after running the UBI/UBIFS tests (on the original and > > new driver without those "fixes") I cannot mount the volume after > > creation as the header is wrongly read. > > I'm not denying this fact, I'm just saying, now that you've found > where the issue comes from, you can also port the fix to the new > driver. Yes, as the old driver is now in a "good enough" shape (though I don't know the exact HW bug reason) - the code can be ported to the new driver. > > Regards, > > Boris > > ______________________________________________________ > Linux MTD discussion mailing list > http://lists.infradead.org/mailman/listinfo/linux-mtd/ Best regards, Lukasz Majewski -- DENX Software Engineering GmbH, Managing Director: Wolfgang Denk HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany Phone: (+49)-8142-66989-10 Fax: (+49)-8142-66989-80 Email: wd@denx.de
Please find following set of patches, which provide improved behaviour of the fsl-quadspi.c driver on Vybrid vf610 HW. Below code is based on previous work done by Albert ARIBAUD: https://patchwork.ozlabs.org/patch/675401/ I've cleaned up the code a bit, make separate patches, exclude not needed parts and port it to v4.19-rc3. After applying those patches, the quadspi controller on vf610 works with UBI/UBIFS. The problem is with some corner case writes to raw MTD device (like writing 1023B with single byte writes). Those fail sometimes. Regression tests can be found in the following repository: https://github.com/lmajewski/tests-spi/tree/master/tests (Please read README.txt). The NXP's community thread regarding HW issues in this block on the vf610: https://community.nxp.com/thread/485139 Maybe those patches will be helpful with the new, work in progress driver for spi-fsl-qspi.c Lukasz Majewski (9): Revert "mtd: fsl-quadspi: Rename SEQID_QUAD_READ to SEQID_READ" mtd: qspi: Provide quirk to read only half of RX buffer (NXP's vybrid) mtd: spi: Do not setup the default seqid as we got it set for DUAL and QUAD mtd: spi: Modify the HW capability mask according to supported RX lanes mtd: spi: Provide LUT entry to perform DUAL read mtd: spi: Enhance the fsl_qspi_read() method to support DUAL and QUAD mtd: spi: Add SPI_NOR_DUAL_READ property for the 'n25q128a13' Micron memory mtd: spi: Allocate memory corresponding to maximal fsl-quadspi.c controller area mtd: spi: Skip reading the Serial Flash Discoverable Parameters drivers/mtd/spi-nor/fsl-quadspi.c | 103 +++++++++++++++++++++++++++++--------- drivers/mtd/spi-nor/spi-nor.c | 2 +- 2 files changed, 79 insertions(+), 26 deletions(-)