On 12/5/24 8:35 AM, Tudor Ambarus wrote:

• Marek

use get_maintainers script please

On 11/18/24 9:05 AM, Venkatesh Yadav Abbarapu wrote:

...

Fix the read issue for 4byte address width by passing the entire length to the read function, split the memory of 16MB size banks only when the address width is 3byte. Also update the size when the configuration is stacked.

Fixes: 5d40b3d384 ("mtd: spi-nor: Add parallel and stacked memories support") Signed-off-by: Venkatesh Yadav Abbarapu venkatesh.abbarapu@amd.com

---

drivers/mtd/spi/spi-nor-core.c | 24 +++++++++++++++--------- 1 file changed, 15 insertions(+), 9 deletions(-)

diff --git a/drivers/mtd/spi/spi-nor-core.c b/drivers/mtd/spi/spi-nor-core.c index ec841fb13b..c37a9bce74 100644 --- a/drivers/mtd/spi/spi-nor-core.c +++ b/drivers/mtd/spi/spi-nor-core.c @@ -1580,6 +1580,7 @@ static int spi_nor_read(struct mtd_info *mtd, loff_t from, size_t len, loff_t offset = from; u32 read_len = 0; u32 rem_bank_len = 0;

• u32 stack_shift = 0; u8 bank; bool is_ofst_odd = false;

@@ -1593,18 +1594,20 @@ 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;
  

• rem_bank_len = SZ_16M * (bank + 1);
  

• if (nor->flags & SNOR_F_HAS_PARALLEL)
  

• 	rem_bank_len *= 2;
  

• rem_bank_len -= from;
  

• if (nor->addr_width == 3) {
  

No, this looks bad because you enter the body of this for all the flashes with addr_width 3, not just for the parallel/stacked ones.

Instead you should do whatever you do here just for your parallel/stacked cases.

...

• 	bank = (u32)from / SZ_16M;
  

• 	if (nor->flags & SNOR_F_HAS_PARALLEL)
  

• 		bank /= 2;
  

• 	rem_bank_len = SZ_16M * (bank + 1);
  

• 	if (nor->flags & SNOR_F_HAS_PARALLEL)
  

• 		rem_bank_len *= 2;
  

• 	rem_bank_len -= from;
  

• }
  

  offset = from;

  if (nor->flags & SNOR_F_HAS_STACKED) {

• 	stack_shift = 1;
  if (offset >= (mtd->size / 2)) {
  	offset = offset - (mtd->size / 2);
  	nor->spi->flags |= SPI_XFER_U_PAGE;
  

@@ -1616,6 +1619,9 @@ static int spi_nor_read(struct mtd_info *mtd, loff_t from, size_t len, if (nor->flags & SNOR_F_HAS_PARALLEL) offset /= 2;

• if (nor->addr_width == 4)
  

• 	rem_bank_len = (mtd->size >> stack_shift) - offset;
  

same here.

If we still want to keep this support until everything is implemented above SPI NOR, other option is to duplicate core methods and introduce dedicated methods for the parallel/stacked cases. You could keep the parallel/stacked methods in dedicated file as well.

The advantage is that you won't interfere with current SPI NOR support and that you won't break any flashes anymore. Then we can easily remove everything once proper support is implemented above SPI NOR. The disadvantage is that we duplicate code ... Marek?

I can imagine the duplication can be solved by factoring out the SPI NOR code into a library design, where each callback implementation could be assembled from building blocks, thus avoiding code duplication. However, this seems like a huge change for the current 2025.01 release cycle, we are already in rc4.

For 2025.01 , either this stacked stuff has to be carefully ifdeffed out, so it can be enabled only for platforms which need it and disabled for all others to avoid breaking them ... or reverted outright .