[U-Boot] [EXT] Re: [PATCH 2/2] fsl_qspi: Improve QSPI driver to incorporate 4 byte commands

[Ashish Kumar]

> -----Original Message-----
> From: U-Boot <u-boot-bounces at lists.denx.de> On Behalf Of Schrempf Frieder
> Sent: Tuesday, April 30, 2019 1:14 PM
> To: Vignesh Raghavendra <vigneshr at ti.com>; Rajat Srivastava
> <rajat.srivastava at nxp.com>; u-boot at lists.denx.de; jagan at openedev.com
> Subject: [EXT] Re: [U-Boot] [PATCH 2/2] fsl_qspi: Improve QSPI driver to
> incorporate 4 byte commands
> 
> Caution: EXT Email
> 
> Hi,
> 
> On 26.04.19 06:58, Vignesh Raghavendra wrote:
> >
> >
> > On 25/04/19 5:20 PM, Rajat Srivastava wrote:
> >>
> >>
> >>> -----Original Message-----
> >>> From: Vignesh Raghavendra <vigneshr at ti.com>
> >>> Sent: Wednesday, April 24, 2019 10:17 PM
> >>> To: Rajat Srivastava <rajat.srivastava at nxp.com>;
> >>> u-boot at lists.denx.de; jagan at openedev.com
> >>> Cc: Ashish Kumar <ashish.kumar at nxp.com>
> >>> Subject: [EXT] Re: [PATCH 2/2] fsl_qspi: Improve QSPI driver to
> >>> incorporate 4 byte commands
> >>>
> >>> Caution: EXT Email
> >>>
> >>> On 24-Apr-19 6:10 PM, Rajat Srivastava wrote:
> >>>> Signed-off-by: Ashish Kumar <ashish.kumar at nxp.com>
> >>>> Signed-off-by: Rajat Srivastava <rajat.srivastava at nxp.com>
> >>>
> >>> Commit message is missing.
> >>
> >> I'll add proper commit message in the next patch version.
> >>
> >>> But from $patch subject, I infer that $patch is adding new feature
> >>> and not actually fixing something broken?
> >>
> >> Earlier the framework was designed to work for only 3-byte opcodes
> >> but our controller supports flashes of size greater than 16 MB. As a
> >> workaround, FSL QSPI driver used to explicitly send 4-byte opcodes
> >> for 3-byte opcodes sent by framework to the flash. Also there used to
> >> exist a temporary patch for framework which never got accepted In
> upstream.
> >> Now the new framework supports 4-byte opcodes and FSL QSPI driver
> >> needs correction. I am not introducing any new feature. I am just
> >> fixing the driver to suit the current framework.
> >>
> >
> > With SPL_FLASH_BAR set fsl_qspi driver should never see 4 byte opcodes
> > and should work like it did before. I guess you are disabling SPI_FLASH_BAR?
> >
> > Please add all details to the commit message and I recommend you to
> > move the driver over to spi-mem as soon as possible. Else you would
> > have to keep handling new opcodes in your driver as and when spi-nor-core
> changes.
> >
> > Regards
> > Vignesh
> >
> >> Please let me know your feedback.
> >>
> >> Regards
> >> Rajat
> >>
> >>> If so, you should move the driver over to use spi-mem APIs instead
> >>> of adding more features and hard coding more flash specific commands in
> the driver.
> >>> This makes driver duplicate more of spi-nor core code. I discourage
> >>> adding new features w/o moving driver over to spi-mem. IMHO,
> >>> converting the driver would not be a huge effort. And I believe its already
> done in kernel.
> 
> I started moving the driver to spi-mem, by porting the kernel driver over to U-
> Boot. I still have some Kconfig dependency problem for the
> LS2081 platform (CONFIG_SPI_MEM is not enabled implicitly), that needs to
> be solved, otherwise it should be more or less ready.
Hi Frieder,

What is the change, it seems it is moving the driver from Linux as it is to uboot ?
I am not sure how stable is the current Linux driver, since it is not yet tested by FSL/NXP system test team. Last time I tested the current Linux driver(QSPI) it was functional in only 1-1-1 mode. Moving to u-boot may not be good idea at this point of time.

How do you plan to cater  CONFIG_SPI_BAR change in uboot now. Some old board still uses flash that supports CONFIG_SPI_BAR.

Me and Rajat have recently posted patches to fix current u-boot driver to be functional with new frame work pushed by Vignesh, and it serves all the current supported board with and without CONFIG_SPI_BAR.

May be spi-nxp-fspi.c can be a good starting point, but then again I not sure how booting from serial-nand will be taken care in that case.

Regards
Ashish 
 
> 
> For anyone who wants to check/test the current state, look here: [1]
> 
> Regards,
> Frieder
> 
> [1]:
> https://eur01.safelinks.protection.outlook.com/?url=https%3A%2F%2Fgithub.
> com%2Ffschrempf%2Fu-
> boot%2Fcommits%2Ffsl_qspi_spimem_port&data=02%7C01%7CAshish.K
> umar%40nxp.com%7C459c6d89b8b748c928ca08d6cd3fa6cd%7C686ea1d3bc
> 2b4c6fa92cd99c5c301635%7C0%7C0%7C636922070665346047&sdata
> =8HplTAZYeEaBrXZd38h4hgT7hLRixjobx%2ByCjL%2FjJjc%3D&reserved=0
> _______________________________________________
> U-Boot mailing list
> U-Boot at lists.denx.de
> https://eur01.safelinks.protection.outlook.com/?url=https%3A%2F%2Flists.de
> nx.de%2Flistinfo%2Fu-
> boot&data=02%7C01%7CAshish.Kumar%40nxp.com%7C459c6d89b8b74
> 8c928ca08d6cd3fa6cd%7C686ea1d3bc2b4c6fa92cd99c5c301635%7C0%7C0
> %7C636922070665346047&sdata=sk6GB%2BMeEKjqR5BR5K9PX6yYayC
> nyDUG69LTWl05xlM%3D&reserved=0