[PATCH 1/9] mtd: spi-nor: Remove recently added nor->addr_width == 3 test

[Jagan Teki]

On Wed, Oct 30, 2024 at 4:15 PM Tudor Ambarus <tudor.ambarus at 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+renesas at 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+renesas at mailbox.org>
> >> ---
> >> Cc: Andre Przywara <andre.przywara at arm.com>
> >> Cc: Ashok Reddy Soma <ashok.reddy.soma at amd.com>
> >> Cc: Jagan Teki <jagan at amarulasolutions.com>
> >> Cc: Michael Walle <mwalle at kernel.org>
> >> Cc: Michal Simek <michal.simek at amd.com>
> >> Cc: Patrice Chotard <patrice.chotard at foss.st.com>
> >> Cc: Patrick Delaunay <patrick.delaunay at foss.st.com>
> >> Cc: Pratyush Yadav <p.yadav at ti.com>
> >> Cc: Quentin Schulz <quentin.schulz at cherry.de>
> >> Cc: Sean Anderson <seanga2 at gmail.com>
> >> Cc: Simon Glass <sjg at chromium.org>
> >> Cc: Takahiro Kuwano <Takahiro.Kuwano at infineon.com>
> >> Cc: Tom Rini <trini at konsulko.com>
> >> Cc: Tudor Ambarus <tudor.ambarus at linaro.org>
> >> Cc: Venkatesh Yadav Abbarapu <venkatesh.abbarapu at amd.com>
> >> Cc: u-boot at lists.denx.de
> >> Cc: uboot-stm32 at 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.

Jagan.