Hi,

> -----Original Message-----
> From: Tom Rini <tr...@konsulko.com>
> Sent: Thursday, December 26, 2024 8:47 PM
> To: Abbarapu, Venkatesh <venkatesh.abbar...@amd.com>
> Cc: Marek Vasut <ma...@denx.de>; u-boot@lists.denx.de;
> tudor.amba...@linaro.org; j-humphr...@ti.com; Simek, Michal
> <michal.si...@amd.com>; ja...@amarulasolutions.com; vigne...@ti.com; u-
> kum...@ti.com; sean...@gmail.com; caleb.conno...@linaro.org;
> s...@chromium.org; william.zh...@broadcom.com; stefa...@posteo.net;
> quentin.sch...@cherry.de; takahiro.kuw...@infineon.com; p-mant...@ti.com; git
> (AMD-Xilinx) <g...@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 <ma...@denx.de>
> > > Sent: Thursday, December 26, 2024 2:19 AM
> > > To: Abbarapu, Venkatesh <venkatesh.abbar...@amd.com>;
> > > u-boot@lists.denx.de; tudor.amba...@linaro.org; j-humphr...@ti.com
> > > Cc: Simek, Michal <michal.si...@amd.com>;
> > > ja...@amarulasolutions.com; vigne...@ti.com; u-kum...@ti.com;
> > > tr...@konsulko.com; sean...@gmail.com; caleb.conno...@linaro.org;
> > > s...@chromium.org; william.zh...@broadcom.com; stefa...@posteo.net;
> > > quentin.sch...@cherry.de; takahiro.kuw...@infineon.com;
> > > p-mant...@ti.com; git (AMD-Xilinx) <g...@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.

Thanks
Venkatesh
 
> --
> Tom

Reply via email to