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

[Vignesh Raghavendra]

On 30/04/19 1:13 PM, Schrempf Frieder wrote:
> 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.
> 

I am not familiar with ls2081 defconfig, but it seems like
CONFIG_SPI_FLASH is not enabled in ls2081ardb_defconfig due to which
SPI_MEM does not be selected. Jagan had a patch to make DM_SPI_FLASH
enable SPI_FLASH[2] to fix such configs.

> For anyone who wants to check/test the current state, look here: [1]
> 

That's great!

> Regards,
> Frieder
> 
> [1]: https://github.com/fschrempf/u-boot/commits/fsl_qspi_spimem_port
> 
[2] https://patchwork.ozlabs.org/patch/1039176/