On 10/30/24 2:17 PM, Jagan Teki wrote:
> On Wed, Oct 30, 2024 at 4:15 PM Tudor Ambarus <[email protected]>
> wrote:
>>
>>
>>
>> On 10/30/24 10:33 AM, Jagan Teki wrote:
>>> Hi Marek,
>>>
>>> On Sun, Oct 27, 2024 at 1:48 AM Marek Vasut
>>> <[email protected]> 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 <[email protected]>
>>>> ---
>>>> Cc: Andre Przywara <[email protected]>
>>>> Cc: Ashok Reddy Soma <[email protected]>
>>>> Cc: Jagan Teki <[email protected]>
>>>> Cc: Michael Walle <[email protected]>
>>>> Cc: Michal Simek <[email protected]>
>>>> Cc: Patrice Chotard <[email protected]>
>>>> Cc: Patrick Delaunay <[email protected]>
>>>> Cc: Pratyush Yadav <[email protected]>
>>>> Cc: Quentin Schulz <[email protected]>
>>>> Cc: Sean Anderson <[email protected]>
>>>> Cc: Simon Glass <[email protected]>
>>>> Cc: Takahiro Kuwano <[email protected]>
>>>> Cc: Tom Rini <[email protected]>
>>>> Cc: Tudor Ambarus <[email protected]>
>>>> Cc: Venkatesh Yadav Abbarapu <[email protected]>
>>>> Cc: [email protected]
>>>> Cc: [email protected]
>>>> ---
>>>
>>> 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.
All the stacked/parallel code shall be pulled out from SPI NOR
eventually. The design is treating 2 independent flashes as one.
Whatever ties them shall be above SPI NOR. In the meantime we all suffer
until that is achieved. I guess if things become unbearable we can't
remove everything later on, can we? Or maybe this ship has sailed
already, I don't know.
It's really your decision to make because you maintain this code. If
full revert is chosen, I can agree and ACK that. If we keep it and try
to sweeten everything a bit, then be it.
Cheers,
ta
