Re: [PATCH 1/6] Revert "spi: zynq_qspi: Add parallel memories support in QSPI driver"

[Jon Humphreys <j-humphreys@ti.com>]

Jon Humphreys <j-humphreys@ti.com> writes:

> Marek Vasut <marek.vasut@mailbox.org> writes:
>
>> On 11/7/24 4:49 PM, Jon Humphreys wrote:
>>> Marek Vasut <marek.vasut@mailbox.org> writes:
>>> 
>>>> On 11/6/24 10:58 PM, Jon Humphreys wrote:
>>>>> Marek Vasut <marek.vasut@mailbox.org> writes:
>>>>>
>>>>>> On 11/6/24 8:18 PM, Jon Humphreys wrote:
>>>>>>> Marek Vasut <marek.vasut@mailbox.org> writes:
>>>>>>>
>>>>>>>> On 10/23/24 10:17 AM, Michal Simek wrote:
>>>>>>>>>
>>>>>>>>>
>>>>>>>>> On 10/22/24 23:06, Marek Vasut wrote:
>>>>>>>>>> This reverts commit 1e36d34b52e7a1ebe5a2a5339d6905540f4253aa.
>>>>>>>>>>
>>>>>>>>>> This parallel/stacked support breaks basic SPI NOR support,
>>>>>>>>>> e.g. this no longer works:
>>>>>>>>>>
>>>>>>>>>> => sf probe && sf update 0x50000000 0 0x160000
>>>>>>>>>> SF: Detected s25fs512s with page size 256 Bytes, erase size 256 KiB,
>>>>>>>>>> total 64 MiB
>>>>>>>>>> device 0 offset 0x0, size 0x160000
>>>>>>>>>> SPI flash failed in read step
>>>>>>>>>
>>>>>>>>> Reverting everything seems to me too much. Tom has tested it on his HW
>>>>>>>>> and didn't see any issue. That's why better to look at code which is
>>>>>>>>> causing this.
>>>>>>>>> You are reverting everything but likely there is specific patch which is
>>>>>>>>> causing this. Which one is it?
>>>>>>>>> Which board was used for your testing? Likely we don't have access to it.
>>>>>>>>> Is there any QEMU available which can be used for debugging?
>>>>>>>>
>>>>>>>> The testcase including the exact SPI NOR model is above.
>>>>>>>>
>>>>>>>> iMX6 with w25q16dw seems to be broken too.
>>>>>>>>
>>>>>>>> Basically every board I have access no longer has a working "sf probe ;
>>>>>>>> sf update" combination ... so yeah, this means this patchset is
>>>>>>>> fundamentally broken.
>>>>>>>>
>>>>>>>
>>>>>>> I can also confirm that the patch series:
>>>>>>>
>>>>>>> f8efc68b30e Merge patch series "spi-nor: Add parallel and stacked memories
>>>>>>> support"
>>>>>>>
>>>>>>> breaks SPI NOR on TI platforms, particularly SK-AM62 and SK-AM62P:
>>>>>>>
>>>>>>> U-Boot 2024.10-00752-gf8efc68b30e2 (Nov 06 2024 - 12:25:13 -0600)
>>>>>>>
>>>>>>> SoC:   AM62X SR1.0 HS-FS
>>>>>>> Model: Texas Instruments AM625 SK
>>>>>>> ...
>>>>>>> Hit any key to stop autoboot:  0
>>>>>>> => sf probe && sf update ${loadaddr} 0x400000 0x10
>>>>>>> SF: Detected s28hs512t with page size 256 Bytes, erase size 256 KiB, total 64 MiB
>>>>>>> device 0 offset 0x400000, size 0x10
>>>>>>> SPI flash failed in read step
>>>>>>> =>
>>>>>> Sigh ... can you please test current u-boot/master and see if the error
>>>>>> is fixed there ?
>>>>>>
>>>>>
>>>>> Yes I had verified it also fails against master, although the behavior was
>>>>> a bit different.  The .'s below are our DMA engine waiting indefinitely.
>>>>>
>>>>> => sf probe && sf update ${loadaddr} 0x400000 0x10
>>>>> SF: Detected s28hs512t with page size 256 Bytes, erase size 256 KiB, total 64 MiB
>>>>> device 0 offset 0x400000, size 0x10
>>>>> .....................................................
>>>>>
>>>>> I have not investigated further.
>>>>
>>>> Can you try and run some 'git bisect' to find out exactly which commit
>>>> broke your use case ? There is a bunch of fixes for the worst breakage
>>>> that landed recently, but clearly there is more.
>>>>
>>>> Full revert seems increasingly appealing ...
>>> 
>>> commit 5d40b3d384d
>> So there is still something broken in that specific commit that I missed 
>> when removing the defects ? Sigh ... can you try to narrow it down ?
>
> Hi Marek, I tried to narrow the changes in commit
> 5d40b3d384dc536ec26ce9b76b20b0b84749d2d1 a bit by first applying all of the
> changes in the .h files, and then for spi-nor-core.c, only applying changes
> per function. The only function change that causes errors was
> spi_nor_read(). With the changes in spi_nor_read() in commit
> 5d40b3d384dc536ec26ce9b76b20b0b84749d2d1 applied to the prior commit
> fbe16bc2801, did I get:
>
> => sf probe && sf update ${loadaddr} 0x400000 0x10
> SF: Detected s28hs512t with page size 256 Bytes, erase size 256 KiB, total 64 MiB
> device 0 offset 0x400000, size 0x10
> SPI flash failed in read step
>

Hi all.  What is the status of addressing the failures introduced by this
patchset?  I tried building from u-boot next and still see the failure.

Marek, was the isolation of changes I performed above helpful in
understanding the failure?

Thanks
Jon

> Let me know if this helps.  
>
> Jon