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

[Jon Humphreys]

Marek Vasut <marek.vasut at mailbox.org> writes:

> On 11/7/24 4:49 PM, Jon Humphreys wrote:
>> Marek Vasut <marek.vasut at mailbox.org> writes:
>> 
>>> On 11/6/24 10:58 PM, Jon Humphreys wrote:
>>>> Marek Vasut <marek.vasut at mailbox.org> writes:
>>>>
>>>>> On 11/6/24 8:18 PM, Jon Humphreys wrote:
>>>>>> Marek Vasut <marek.vasut at 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

Let me know if this helps.  

Jon