[PATCH v4] mtd: spi-nor: Fix the spi_nor_read() when config SPI_STACKED_PARALLEL is enabled

[Tom Rini]

On Fri, Dec 27, 2024 at 04:48:47AM +0000, Abbarapu, Venkatesh wrote:
> Hi,
> 
> > -----Original Message-----
> > From: Tom Rini <trini at konsulko.com>
> > Sent: Thursday, December 26, 2024 8:47 PM
> > To: Abbarapu, Venkatesh <venkatesh.abbarapu at amd.com>
> > Cc: Marek Vasut <marex at denx.de>; u-boot at lists.denx.de;
> > tudor.ambarus at linaro.org; j-humphreys at ti.com; Simek, Michal
> > <michal.simek at amd.com>; jagan at amarulasolutions.com; vigneshr at ti.com; u-
> > kumar1 at ti.com; seanga2 at gmail.com; caleb.connolly at linaro.org;
> > sjg at chromium.org; william.zhang at broadcom.com; stefan_b at posteo.net;
> > quentin.schulz at cherry.de; Takahiro.Kuwano at infineon.com; p-mantena at ti.com; git
> > (AMD-Xilinx) <git at amd.com>
> > Subject: Re: [PATCH v4] mtd: spi-nor: Fix the spi_nor_read() when config
> > SPI_STACKED_PARALLEL is enabled
> > 
> > On Thu, Dec 26, 2024 at 03:34:33AM +0000, Abbarapu, Venkatesh wrote:
> > > Hi,
> > >
> > > > -----Original Message-----
> > > > From: Marek Vasut <marex at denx.de>
> > > > Sent: Thursday, December 26, 2024 2:19 AM
> > > > To: Abbarapu, Venkatesh <venkatesh.abbarapu at amd.com>;
> > > > u-boot at lists.denx.de; tudor.ambarus at linaro.org; j-humphreys at ti.com
> > > > Cc: Simek, Michal <michal.simek at amd.com>;
> > > > jagan at amarulasolutions.com; vigneshr at ti.com; u-kumar1 at ti.com;
> > > > trini at konsulko.com; seanga2 at gmail.com; caleb.connolly at linaro.org;
> > > > sjg at chromium.org; william.zhang at broadcom.com; stefan_b at posteo.net;
> > > > quentin.schulz at cherry.de; Takahiro.Kuwano at infineon.com;
> > > > p-mantena at ti.com; git (AMD-Xilinx) <git at amd.com>
> > > > Subject: Re: [PATCH v4] mtd: spi-nor: Fix the spi_nor_read() when
> > > > config SPI_STACKED_PARALLEL is enabled
> > > >
> > > > On 12/24/24 4:37 PM, Venkatesh Yadav Abbarapu wrote:
> > > >
> > > > [...]
> > > >
> > > > > @@ -1593,18 +1596,22 @@ static int spi_nor_read(struct mtd_info
> > > > > *mtd, loff_t
> > > > from, size_t len,
> > > > >   	}
> > > > >
> > > > >   	while (len) {
> > > > > -		bank = (u32)from / SZ_16M;
> > > > > -		if (nor->flags & SNOR_F_HAS_PARALLEL)
> > > > > -			bank /= 2;
> > > > > +		read_len = len;
> > > > > +		offset = from;
> > > > >
> > > > > -		rem_bank_len = SZ_16M * (bank + 1);
> > > > > -		if (nor->flags & SNOR_F_HAS_PARALLEL)
> > > > > -			rem_bank_len *= 2;
> > > > > -		rem_bank_len -= from;
> > > > > +		if (CONFIG_IS_ENABLED(SPI_FLASH_BAR)) {
> > > > > +			bank = (u32)from / SZ_16M;
> > > > > +			if (nor->flags & SNOR_F_HAS_PARALLEL)
> > > > > +				bank /= 2;
> > > > Is this code which operates on (nor->flags & SNOR_F_HAS_PARALLEL)
> > > > really supposed to be enabled if (CONFIG_IS_ENABLED(SPI_FLASH_BAR))
> > > > is SET or instead if STACKED_PARALLEL symbol is SET ?
> > >
> > > The FLASH_BAR and STACKED_PARALLEL configs are independent. They
> > won't depend on each other.
> > 
> > At this point in the release cycle, I'd really rather see things as:
> > if (CONFIG_SPI_STACKED_PARALLEL) {
> >   ... anything needed for the new support mode ..
> > } else {
> >   ... the way it used to be ...
> > }
> > 
> > And for v2025.04 we can move the codebase forward. But I'm worried at this point
> > I'm going to have to revert all of the stacked stuff, which could get messy, in order to
> > avoid regressions elsewhere.
> 
> I am updating the parallel/stacked code based on the config
> SPI_STACKED_PARALLEL and not touching the default code.

Right, but to be clear, are you bringing back the default code to the
state  / algorithms it was in for v2024.10?

-- 
Tom
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 659 bytes
Desc: not available
URL: <https://lists.denx.de/pipermail/u-boot/attachments/20241227/99301a36/attachment.sig>