[U-Boot] [EXT] Re: [PATCH 0/8] Transition of fsl qspi driver to spi-mem framework

[Schrempf Frieder]

Hi Ashish,

On 03.12.19 10:44, Frieder Schrempf wrote:
> On 03.12.19 10:33, Ashish Kumar wrote:
>> Hi Frieder,
>>
>>> -----Original Message-----
>>> From: Schrempf Frieder <frieder.schrempf at kontron.de>
>>> Sent: Tuesday, December 3, 2019 2:27 PM
>>> To: Kuldeep Singh <kuldeep.singh at nxp.com>; u-boot at lists.denx.de
>>> Cc: Priyanka Jain <priyanka.jain at nxp.com>; jagan at amarulasolutions.com;
>>> sr at denx.de; Ashish Kumar <ashish.kumar at nxp.com>; Ye Li
>>> <ye.li at nxp.com>
>>> Subject: Re: [EXT] Re: [PATCH 0/8] Transition of fsl qspi driver to 
>>> spi-mem
>>> framework
>>>
>>> Caution: EXT Email
>>>
>>> On 03.12.19 07:30, Kuldeep Singh wrote:
>>>> Hi Frieder,
>>>>
>>>>> -----Original Message-----
>>>>> From: Schrempf Frieder <frieder.schrempf at kontron.de>
>>>>> Sent: Monday, December 2, 2019 5:35 PM
>>>>> To: Kuldeep Singh <kuldeep.singh at nxp.com>; u-boot at lists.denx.de
>>>>> Cc: Priyanka Jain <priyanka.jain at nxp.com>;
>>>>> jagan at amarulasolutions.com; sr at denx.de; Ashish Kumar
>>>>> <ashish.kumar at nxp.com>
>>>>> Subject: [EXT] Re: [PATCH 0/8] Transition of fsl qspi driver to
>>>>> spi-mem framework
>>>>>
>>>>> Caution: EXT Email
>>>>>
>>>>> + Ashish
>>>>>
>>>>> Hi Kuldeep,
>>>>>
>>>>> On 29.11.19 06:54, Kuldeep Singh wrote:
>>>>>> This entire patch series migrate freescale qspi driver to spi-mem
>>> framework.
>>>>>
>>>>> First, thanks for working on this. I have this on my list for quite a
>>>>> long time, but struggled to find enough time to actually get it done.
>>>>>
>>>>>>
>>>>>> Patch 1 removes the old fsl qspi driver
>>>>>
>>>>> You shouldn't remove the old driver before the new one is in place,
>>>>> as this breaks the build in between the two patches.
>>>>
>>>> I first merged the patch1 and patch2 and found that the diff output was
>>> very much less readable.
>>>> That's why I split them into 2 patches so as to make new driver changes
>>> legible.
>>>> Please let me know how shall I proceed. Shall I merge the two patches?
>>>
>>> Yes, the merged patch will not be very good to read, but as far as I 
>>> know
>>> there is no other option. We must not break the build to keep 
>>> bisectability.
>>>
>>>>
>>>>>
>>>>>>
>>>>>> 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.
>>>>>
>>>>> Can you provide more information about the problem/crash you
>>>>> experience and the platform you are working on?
>>>>
>>>> I observed crash on LS1012. Also, any access to flash direct memory 
>>>> above
>>> 1k will crash without this change.
>>>
>>> As I already told Ashish in the conversation referenced in my last mail:
>>> I can't see any good reason why the direct memory access is something 
>>> that
>>> we need or should support. We should always use the APIs provided by U-
>>> Boot to access the flash and that is mtd.
>>>
>>>> By adding this, crash will be resolved but data is invalid as 
>>>> mentioned in
>>> patch-set.
>>>
>>> So what's the purpose of your changes at all, if they do not solve 
>>> the problem
>>> you're trying to solve?
>>> Why don't you just use sf/mtd to access the flash?
>>>
>>>>
>>>>> Are you referring to the same issue as Ashish in this discussion 
>>>>> here [1]?
>>>>
>>>> Yes, I had a discussion with him.
>>>>
>>>>> There are two reasons why I'd like to avoid using the whole memory
>>>>> mapped area for AHB access.
>>>>> First, I'd like to keep the U-Boot driver as close as possible to 
>>>>> the Linux
>>> driver.
>>>>> Second, the intention of the spi-mem layer is to abstract the flash
>>>>> layer and therefore this driver should work independently of flash 
>>>>> type or
>>> size.
>>>>
>>>> Boot from QSPI-NAND will still not be possible. Code in bootROM is 
>>>> only to
>>> access QSPI-NOR.
>>>
>>> It will not be possible to use SPI NAND directly from the BootROM, 
>>> but you
>>> can just load the bootloader from a different device like SPI NOR and 
>>> then
>>> fetch the rest of the system (Kernel, rootfs, etc.) from a SPI NAND 
>>> device.
>>> Actually that's exactly the use case, that led to the development of 
>>> the SPI
>>> MEM layer and the migration of the QSPI driver.
>> Is setting SFA1AD, SFA2AD, SFB1AD, SFB2AD to flash size, QSPI-NAND 
>> access is
>> broken?
> 
> I haven't checked yet, but that's not my main point here.
> 
> 
> I cannot check this since our board does not have such configuration.
>> If yes, then it should be reverted.
>>
>> otherwise I believe there is no harm in this version of driver, it 
>> does everything intended.
> 
> You still haven't given any good reason why you need this. And you said 
> that your use case is still broken, even with your changes. So what's 
> your point? I don't get it. And what about answering my questions above?

Sorry I've just mistaken you for Kuldeep. I don't expect you to answer 
my questions to Kuldeep, of course.

Frieder