On 10/30/24 4:56 PM, Tom Rini wrote:
> On Wed, Oct 30, 2024 at 04:20:32PM +0100, Michal Simek wrote:
>>
>>
>> On 10/30/24 15:49, Tudor Ambarus wrote:
>>>
>>>
>>> On 10/30/24 2:17 PM, Jagan Teki wrote:
>>>> On Wed, Oct 30, 2024 at 4:15 PM Tudor Ambarus <tudor.amba...@linaro.org>
>>>> wrote:
>>>>>
>>>>>
>>>>>
>>>>> On 10/30/24 10:33 AM, Jagan Teki wrote:
>>>>>> Hi Marek,
>>>>>>
>>>>>> On Sun, Oct 27, 2024 at 1:48 AM Marek Vasut
>>>>>> <marek.vasut+rene...@mailbox.org> wrote:
>>>>>>>
>>>>>>> Remove undocumented nor->addr_width == 3 test. This was added in commit
>>>>>>> 5d40b3d384dc ("mtd: spi-nor: Add parallel and stacked memories support")
>>>>>>> without any explanation in the commit message. Remove it.
>>>>>>>
>>>>>>> This also has a bad side-effect which breaks READ operation of every
>>>>>>> SPI NOR
>>>>>>> which does not use addr_width == 3, e.g. s25fs512s does not work at
>>>>>>> all. This
>>>>>>> is because if addr_width != 3, rem_bank_len is always 0, and if
>>>>>>> rem_bank_len
>>>>>>> is 0, then read_len is 0 and if read_len is 0, then the spi_nor_read()
>>>>>>> returns
>>>>>>> -EIO.
>>>>>>>
>>>>>>> Basic reproducer is as follows:
>>>>>>> "
>>>>>>> => sf probe ; sf read 0x50000000 0 0x10000
>>>>>>> SF: Detected s25fs512s with page size 256 Bytes, erase size 256 KiB,
>>>>>>> total 64 MiB
>>>>>>> device 0 offset 0x0, size 0x10000
>>>>>>> SF: 65536 bytes @ 0x0 Read: ERROR -5
>>>>>>> "
>>>>>>>
>>>>>>> Fixes: 5d40b3d384dc ("mtd: spi-nor: Add parallel and stacked memories
>>>>>>> support")
>>>>>>> Signed-off-by: Marek Vasut <marek.vasut+rene...@mailbox.org>
>>>>>>> ---
>>>>>>> Cc: Andre Przywara <andre.przyw...@arm.com>
>>>>>>> Cc: Ashok Reddy Soma <ashok.reddy.s...@amd.com>
>>>>>>> Cc: Jagan Teki <ja...@amarulasolutions.com>
>>>>>>> Cc: Michael Walle <mwa...@kernel.org>
>>>>>>> Cc: Michal Simek <michal.si...@amd.com>
>>>>>>> Cc: Patrice Chotard <patrice.chot...@foss.st.com>
>>>>>>> Cc: Patrick Delaunay <patrick.delau...@foss.st.com>
>>>>>>> Cc: Pratyush Yadav <p.ya...@ti.com>
>>>>>>> Cc: Quentin Schulz <quentin.sch...@cherry.de>
>>>>>>> Cc: Sean Anderson <sean...@gmail.com>
>>>>>>> Cc: Simon Glass <s...@chromium.org>
>>>>>>> Cc: Takahiro Kuwano <takahiro.kuw...@infineon.com>
>>>>>>> Cc: Tom Rini <tr...@konsulko.com>
>>>>>>> Cc: Tudor Ambarus <tudor.amba...@linaro.org>
>>>>>>> Cc: Venkatesh Yadav Abbarapu <venkatesh.abbar...@amd.com>
>>>>>>> Cc: u-boot@lists.denx.de
>>>>>>> Cc: uboot-st...@st-md-mailman.stormreply.com
>>>>>>> ---
>>>>>>
>>>>>> Is this patch-set next version for 'previous' reverted series?
>>>>>>
>>>>>
>>>>> my 2c: I think I lean towards reverting the stacked/parallel support.
>>>>> The only one that benefits of is AMD, while affecting the core code
>>>>> quality. It also slows down further contributions and I assume it
>>>>> hardens maintainer's job.
>>>>
>>>> I did try this before and it is hard to separate from the core. and at
>>>> the same time it is hard to maintain the core too.
>>>>
>>>>>
>>>>> Even if I (we?) haven't made my mind on how to best implement this, it's
>>>>> clear that it shall be above SPI NOR without affecting current devices.
>>>>>
>>>>> Not sure if it's fine to revert everything, haven't followed u-boot
>>>>> lately, your choice to make.
>>>>
>>>> If we find a way (not sure if it's possible) separate like a wrapper
>>>> or fix the things without revert - these are two points I can see as
>>>> of now.
>>>>
>>>
>>> Then this set shall help move in this direction. Some involvement from
>>> the stacked/parallel authors would be nice here, and some commitment
>>> that the current status in just a temporary situation.
>>
>> I hope that by authors you mean SW owners not really HW authors of this
>> wiring.
>> Jagan is aware that we are using this configuration for quite a long time
>> and we are still here and not leaving.
>> As you know Amit is trying to find a solution in Linux but nothing has been
>> agreed yet. If there is agreement to separate it to own driver or so we will
>> be definitely working with u-boot to get it to the same state.
>>
>> This patchset is using the latest approved DT binding created by Miquel and
>> if new binding is accepted we will be working to align to it.
>> Also intention is to bring this functionality to customers and not break
>> others. That's why these patches are pulled into rc1 to get better test
>> coverage.
>>
>> And to be fair to say version which has been merged was v14 and anybody
>> could comment it before and we are definitely here to help to resolve issues
>> on other boards caused by this patchset. But we need to have help from
>> others because obviously we don't have other boards and they are likely also
>> not wired in CI.
>
> To be clear, we need to resolve the problems that have shown up now on
> all of the hardware that was working in v2024.10. My current inclination
> is to merge
> https://patchwork.ozlabs.org/project/uboot/list/?series=429932&state=*
> (aka this series in question) ASAP.
>
We can surely live with this temporary situation. Now that we're assured
to have AMD's help in moving forward, it feels less like a transfer of
responsibility on the poorly designed stacked/parallel software support.
Fine by me to choose the smaller fixes/improvements over the full revert.
Cheers,
ta
