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

27 Dec 2024


      Hi,
...
-----Original Message-----
From: Tom Rini trini@konsulko.com
Sent: Thursday, December 26, 2024 8:47 PM
To: Abbarapu, Venkatesh venkatesh.abbarapu@amd.com
Cc: Marek Vasut marex@denx.de; u-boot@lists.denx.de;
tudor.ambarus@linaro.org; j-humphreys@ti.com; Simek, Michal
michal.simek@amd.com; jagan@amarulasolutions.com; vigneshr@ti.com; u-
kumar1@ti.com; seanga2@gmail.com; caleb.connolly@linaro.org;
sjg@chromium.org; william.zhang@broadcom.com; stefan_b@posteo.net;
quentin.schulz@cherry.de; Takahiro.Kuwano@infineon.com; p-mantena@ti.com; git
(AMD-Xilinx) git@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@denx.de
Sent: Thursday, December 26, 2024 2:19 AM
To: Abbarapu, Venkatesh venkatesh.abbarapu@amd.com;
u-boot@lists.denx.de; tudor.ambarus@linaro.org; j-humphreys@ti.com
Cc: Simek, Michal michal.simek@amd.com;
jagan@amarulasolutions.com; vigneshr@ti.com; u-kumar1@ti.com;
trini@konsulko.com; seanga2@gmail.com; caleb.connolly@linaro.org;
sjg@chromium.org; william.zhang@broadcom.com; stefan_b@posteo.net;
quentin.schulz@cherry.de; Takahiro.Kuwano@infineon.com;
p-mantena@ti.com; git (AMD-Xilinx) git@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

    