public inbox for u-boot@lists.denx.de
 help / color / mirror / Atom feed
From: Jon Humphreys <j-humphreys@ti.com>
To: Marek Vasut <marek.vasut@mailbox.org>,
	Michal Simek <michal.simek@amd.com>, <u-boot@lists.denx.de>,
	Tom Rini <trini@konsulko.com>
Cc: Andre Przywara <andre.przywara@arm.com>,
	Ashok Reddy Soma <ashok.reddy.soma@amd.com>,
	Jagan Teki <jagan@amarulasolutions.com>,
	"Michael Walle" <mwalle@kernel.org>,
	Patrice Chotard <patrice.chotard@foss.st.com>,
	Patrick Delaunay <patrick.delaunay@foss.st.com>,
	Pratyush Yadav <p.yadav@ti.com>,
	Quentin Schulz <quentin.schulz@cherry.de>,
	Sean Anderson <seanga2@gmail.com>, Simon Glass <sjg@chromium.org>,
	Takahiro Kuwano <Takahiro.Kuwano@infineon.com>,
	Tudor Ambarus <tudor.ambarus@linaro.org>,
	Venkatesh Yadav Abbarapu <venkatesh.abbarapu@amd.com>,
	<uboot-stm32@st-md-mailman.stormreply.com>
Subject: Re: [PATCH 1/6] Revert "spi: zynq_qspi: Add parallel memories support in QSPI driver"
Date: Mon, 11 Nov 2024 08:18:20 -0600	[thread overview]
Message-ID: <86ldxp6ehf.fsf@udb0321960.dhcp.ti.com> (raw)
In-Reply-To: <33e99705-77e3-4acf-95f7-fad65f60ae62@mailbox.org>

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

Let me know if this helps.  

Jon

  reply	other threads:[~2024-11-11 14:18 UTC|newest]

Thread overview: 38+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2024-10-22 21:06 [PATCH 1/6] Revert "spi: zynq_qspi: Add parallel memories support in QSPI driver" Marek Vasut
2024-10-22 21:06 ` [PATCH 2/6] Revert "spi: zynqmp_gqspi: Add parallel memories support in GQSPI driver" Marek Vasut
2024-10-22 21:06 ` [PATCH 3/6] Revert "spi: spi-uclass: Read chipselect and restrict capabilities" Marek Vasut
2024-10-22 21:06 ` [PATCH 4/6] Revert "mtd: spi-nor: Add parallel and stacked memories support in read_bar and write_bar" Marek Vasut
2024-10-22 21:06 ` [PATCH 5/6] Revert "mtd: spi-nor: Add parallel memories support for read_sr and read_fsr" Marek Vasut
2024-10-22 21:06 ` [PATCH 6/6] Revert "mtd: spi-nor: Add parallel and stacked memories support" Marek Vasut
2024-11-02 13:26   ` Heinrich Schuchardt
2024-10-23  3:18 ` [PATCH 1/6] Revert "spi: zynq_qspi: Add parallel memories support in QSPI driver" Abbarapu, Venkatesh
2024-10-23  8:41   ` Marek Vasut
2024-10-23  9:07     ` Abbarapu, Venkatesh
2024-10-23 12:45       ` Marek Vasut
2024-10-23 14:14         ` Abbarapu, Venkatesh
2024-10-23 14:19           ` Marek Vasut
2024-10-23 14:25             ` Abbarapu, Venkatesh
2024-10-23 14:37               ` Abbarapu, Venkatesh
2024-10-23 15:18                 ` Marek Vasut
2024-10-23 15:33                   ` Abbarapu, Venkatesh
2024-10-23 15:10           ` Marek Vasut
2024-10-23  9:21     ` Michael Nazzareno Trimarchi
2024-10-23  8:17 ` Michal Simek
2024-10-23  8:38   ` Marek Vasut
2024-11-06 19:18     ` Jon Humphreys
2024-11-06 20:50       ` Marek Vasut
2024-11-06 21:58         ` Jon Humphreys
2024-11-07  0:46           ` Marek Vasut
2024-11-07 15:49             ` Jon Humphreys
2024-11-07 16:04               ` Marek Vasut
2024-11-11 14:18                 ` Jon Humphreys [this message]
2024-12-05  4:16                   ` Jon Humphreys
2024-12-05  4:27                     ` Abbarapu, Venkatesh
2024-12-05  5:17                       ` Marek Vasut
2024-12-06  4:33                       ` Jon Humphreys
2024-12-06  9:46                         ` Abbarapu, Venkatesh
2024-12-06 16:39                           ` Tom Rini
2024-12-10  3:20                             ` Abbarapu, Venkatesh
2024-12-10  8:19                               ` Marek Vasut
2024-12-10  8:32                                 ` Abbarapu, Venkatesh
2024-12-10 11:36                               ` Tudor Ambarus

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=86ldxp6ehf.fsf@udb0321960.dhcp.ti.com \
    --to=j-humphreys@ti.com \
    --cc=Takahiro.Kuwano@infineon.com \
    --cc=andre.przywara@arm.com \
    --cc=ashok.reddy.soma@amd.com \
    --cc=jagan@amarulasolutions.com \
    --cc=marek.vasut@mailbox.org \
    --cc=michal.simek@amd.com \
    --cc=mwalle@kernel.org \
    --cc=p.yadav@ti.com \
    --cc=patrice.chotard@foss.st.com \
    --cc=patrick.delaunay@foss.st.com \
    --cc=quentin.schulz@cherry.de \
    --cc=seanga2@gmail.com \
    --cc=sjg@chromium.org \
    --cc=trini@konsulko.com \
    --cc=tudor.ambarus@linaro.org \
    --cc=u-boot@lists.denx.de \
    --cc=uboot-stm32@st-md-mailman.stormreply.com \
    --cc=venkatesh.abbarapu@amd.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox