From 19e189b1de0c12cbd5a93aa37da675ee4b076d24 Mon Sep 17 00:00:00 2001 From: Marek Vasut Date: Sat, 26 Oct 2024 22:16:18 +0200 Subject: [PATCH 1/9] mtd: spi-nor: Remove recently added nor->addr_width == 3 test 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 --- drivers/mtd/spi/spi-nor-core.c | 33 ++++++++++++++------------------- 1 file changed, 14 insertions(+), 19 deletions(-) diff --git a/drivers/mtd/spi/spi-nor-core.c b/drivers/mtd/spi/spi-nor-core.c index f5c9868bbca..8dfdcda47fb 100644 --- a/drivers/mtd/spi/spi-nor-core.c +++ b/drivers/mtd/spi/spi-nor-core.c @@ -1595,16 +1595,14 @@ static int spi_nor_read(struct mtd_info *mtd, loff_t from, size_t len, } while (len) { - if (nor->addr_width == 3) { - if (nor->flags & SNOR_F_HAS_PARALLEL) { - bank = (u32)from / (SZ_16M << 0x01); - rem_bank_len = ((SZ_16M << 0x01) * - (bank + 1)) - from; - } else { - bank = (u32)from / SZ_16M; - rem_bank_len = (SZ_16M * (bank + 1)) - from; - } + if (nor->flags & SNOR_F_HAS_PARALLEL) { + bank = (u32)from / (SZ_16M << 0x01); + rem_bank_len = ((SZ_16M << 0x01) * (bank + 1)) - from; + } else { + bank = (u32)from / SZ_16M; + rem_bank_len = (SZ_16M * (bank + 1)) - from; } + offset = from; if (nor->flags & SNOR_F_HAS_STACKED) { @@ -1619,13 +1617,11 @@ 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 == 3) { #ifdef CONFIG_SPI_FLASH_BAR - ret = write_bar(nor, offset); - if (ret < 0) - return log_ret(ret); + ret = write_bar(nor, offset); + if (ret < 0) + return log_ret(ret); #endif - } if (len < rem_bank_len) read_len = len; @@ -2012,13 +2008,12 @@ static int spi_nor_write(struct mtd_info *mtd, loff_t to, size_t len, } } - if (nor->addr_width == 3) { #ifdef CONFIG_SPI_FLASH_BAR - ret = write_bar(nor, offset); - if (ret < 0) - return ret; + ret = write_bar(nor, offset); + if (ret < 0) + return ret; #endif - } + /* the size of data remaining on the first page */ page_remain = min_t(size_t, nor->page_size - page_offset, len - i); From b8807c8c4065fd9bc686a5c03042e9c5d7af3ec9 Mon Sep 17 00:00:00 2001 From: Marek Vasut Date: Sat, 26 Oct 2024 22:16:19 +0200 Subject: [PATCH 2/9] mtd: spi-nor: Remove recently added spi_nor_wait_till_ready() call Remove undocumented spi_nor_wait_till_ready() call. This was added in commit 5d40b3d384dc ("mtd: spi-nor: Add parallel and stacked memories support") without any explanation in the commit message. Remove it. Fixes: 5d40b3d384dc ("mtd: spi-nor: Add parallel and stacked memories support") Signed-off-by: Marek Vasut --- drivers/mtd/spi/spi-nor-core.c | 8 -------- 1 file changed, 8 deletions(-) diff --git a/drivers/mtd/spi/spi-nor-core.c b/drivers/mtd/spi/spi-nor-core.c index 8dfdcda47fb..0b131ad9aba 100644 --- a/drivers/mtd/spi/spi-nor-core.c +++ b/drivers/mtd/spi/spi-nor-core.c @@ -1631,10 +1631,6 @@ static int spi_nor_read(struct mtd_info *mtd, loff_t from, size_t len, if (read_len == 0) return -EIO; - ret = spi_nor_wait_till_ready(nor); - if (ret) - goto read_err; - ret = nor->read(nor, offset, read_len, buf); if (ret == 0) { /* We shouldn't see 0-length reads */ @@ -2018,10 +2014,6 @@ static int spi_nor_write(struct mtd_info *mtd, loff_t to, size_t len, page_remain = min_t(size_t, nor->page_size - page_offset, len - i); - ret = spi_nor_wait_till_ready(nor); - if (ret) - goto write_err; - write_enable(nor); /* * On DTR capable flashes like Micron Xcella the writes cannot From 8ef342f7cbb5b3a0c67448536bf4e0f9dbcf2ef0 Mon Sep 17 00:00:00 2001 From: Marek Vasut Date: Sat, 26 Oct 2024 22:16:20 +0200 Subject: [PATCH 3/9] mtd: spi-nor: Remove recently added set_4byte() call Remove undocumented set_4byte() call. This was added in commit 5d40b3d384dc ("mtd: spi-nor: Add parallel and stacked memories support") without any explanation in the commit message. Remove it. Fixes: 5d40b3d384dc ("mtd: spi-nor: Add parallel and stacked memories support") Signed-off-by: Marek Vasut --- drivers/mtd/spi/spi-nor-core.c | 1 - 1 file changed, 1 deletion(-) diff --git a/drivers/mtd/spi/spi-nor-core.c b/drivers/mtd/spi/spi-nor-core.c index 0b131ad9aba..3114281832b 100644 --- a/drivers/mtd/spi/spi-nor-core.c +++ b/drivers/mtd/spi/spi-nor-core.c @@ -4593,7 +4593,6 @@ int spi_nor_scan(struct spi_nor *nor) #else /* Configure the BAR - discover bank cmds and read current bank */ nor->addr_width = 3; - set_4byte(nor, info, 0); ret = read_bar(nor, info); if (ret < 0) return ret; From 787692c8d75224bb80bb9b16e10d53316b12f697 Mon Sep 17 00:00:00 2001 From: Marek Vasut Date: Sat, 26 Oct 2024 22:16:21 +0200 Subject: [PATCH 4/9] mtd: spi-nor: Remove recently added write_disable() call Remove undocumented write_disable() call. This was added in commit 5d40b3d384dc ("mtd: spi-nor: Add parallel and stacked memories support") without any explanation in the commit message. Remove it. Fixes: 5d40b3d384dc ("mtd: spi-nor: Add parallel and stacked memories support") Signed-off-by: Marek Vasut --- drivers/mtd/spi/spi-nor-core.c | 10 +--------- 1 file changed, 1 insertion(+), 9 deletions(-) diff --git a/drivers/mtd/spi/spi-nor-core.c b/drivers/mtd/spi/spi-nor-core.c index 3114281832b..d22363e8267 100644 --- a/drivers/mtd/spi/spi-nor-core.c +++ b/drivers/mtd/spi/spi-nor-core.c @@ -2073,10 +2073,6 @@ static int spi_nor_write(struct mtd_info *mtd, loff_t to, size_t len, if (ret) goto write_err; - ret = write_disable(nor); - if (ret) - goto write_err; - *retlen += written; i += written; } @@ -2117,10 +2113,6 @@ static int macronix_quad_enable(struct spi_nor *nor) if (ret) return ret; - ret = write_disable(nor); - if (ret) - return ret; - ret = read_sr(nor); if (!(ret > 0 && (ret & SR_QUAD_EN_MX))) { dev_err(nor->dev, "Macronix Quad bit not set\n"); @@ -2182,7 +2174,7 @@ static int spansion_quad_enable_volatile(struct spi_nor *nor, u32 addr_base, return -EINVAL; } - return write_disable(nor); + return 0; } #endif From 34cd4a72fb2d113e2754c0d643618a8e3fa549ab Mon Sep 17 00:00:00 2001 From: Marek Vasut Date: Sat, 26 Oct 2024 22:16:22 +0200 Subject: [PATCH 5/9] mtd: spi-nor: Remove recently added SST special case Remove undocumented SST special case. This was added in commit 5d40b3d384dc ("mtd: spi-nor: Add parallel and stacked memories support") without any explanation in the commit message. Remove it. Fixes: 5d40b3d384dc ("mtd: spi-nor: Add parallel and stacked memories support") Signed-off-by: Marek Vasut --- drivers/mtd/spi/spi-nor-core.c | 13 ------------- 1 file changed, 13 deletions(-) diff --git a/drivers/mtd/spi/spi-nor-core.c b/drivers/mtd/spi/spi-nor-core.c index d22363e8267..3c47751348e 100644 --- a/drivers/mtd/spi/spi-nor-core.c +++ b/drivers/mtd/spi/spi-nor-core.c @@ -3578,19 +3578,6 @@ static int spi_nor_select_erase(struct spi_nor *nor, mtd->erasesize = info->sector_size; } - if ((JEDEC_MFR(info) == SNOR_MFR_SST) && info->flags & SECT_4K) { - nor->erase_opcode = SPINOR_OP_BE_4K; - /* - * In parallel-memories the erase operation is - * performed on both the flashes simultaneously - * so, double the erasesize. - */ - if (nor->flags & SNOR_F_HAS_PARALLEL) - mtd->erasesize = 4096 * 2; - else - mtd->erasesize = 4096; - } - return 0; } From a21cfc4e7c5380fb674989d2a00a9b9b7c42dd8e Mon Sep 17 00:00:00 2001 From: Marek Vasut Date: Sat, 26 Oct 2024 22:16:23 +0200 Subject: [PATCH 6/9] mtd: spi-nor: Fix multiple coding style issues The offset variable is set, but never used afterward. Fix indent. Fix predecrement without justification. Remove use of parenthesis where unnecessary. Fixes: 5d40b3d384dc ("mtd: spi-nor: Add parallel and stacked memories support") Signed-off-by: Marek Vasut --- drivers/mtd/spi/spi-nor-core.c | 16 +++++++--------- 1 file changed, 7 insertions(+), 9 deletions(-) diff --git a/drivers/mtd/spi/spi-nor-core.c b/drivers/mtd/spi/spi-nor-core.c index 3c47751348e..e49b7cad023 100644 --- a/drivers/mtd/spi/spi-nor-core.c +++ b/drivers/mtd/spi/spi-nor-core.c @@ -1134,12 +1134,10 @@ static int spi_nor_erase(struct mtd_info *mtd, struct erase_info *instr) offset /= 2; if (nor->flags & SNOR_F_HAS_STACKED) { - if (offset >= (mtd->size / 2)) { - offset = offset - (mtd->size / 2); + if (offset >= (mtd->size / 2)) nor->spi->flags |= SPI_XFER_U_PAGE; - } else { + else nor->spi->flags &= ~SPI_XFER_U_PAGE; - } } #ifdef CONFIG_SPI_FLASH_BAR ret = write_bar(nor, addr); @@ -1588,7 +1586,7 @@ static int spi_nor_read(struct mtd_info *mtd, loff_t from, size_t len, dev_dbg(nor->dev, "from 0x%08x, len %zd\n", (u32)from, len); if ((nor->flags & SNOR_F_HAS_PARALLEL) && (offset & 1)) { - /* We can hit this case when we use file system like ubifs */ + /* We can hit this case when we use file system like ubifs */ from--; len++; is_ofst_odd = true; @@ -1969,9 +1967,9 @@ static int spi_nor_write(struct mtd_info *mtd, loff_t to, size_t len, return ret; *retlen += 1; /* We've written only one actual byte */ - ++buf; - --len; - ++to; + buf++; + len--; + to++; } for (i = 0; i < len; ) { @@ -1991,7 +1989,7 @@ static int spi_nor_write(struct mtd_info *mtd, loff_t to, size_t len, page_offset = do_div(aux, nor->page_size); } - offset = (to + i); + offset = to + i; if (nor->flags & SNOR_F_HAS_PARALLEL) offset /= 2; From 003157bd1c2c029abaefb9457965551f305effda Mon Sep 17 00:00:00 2001 From: Marek Vasut Date: Sat, 26 Oct 2024 22:16:24 +0200 Subject: [PATCH 7/9] mtd: spi-nor: Rewrite rem_bank_len calculation Rewrite the code to make it clear exactly where the SNOR_F_HAS_PARALLEL flag leads to *2 and /2 operation compared to regular code path. No functional change. Fixes: 5d40b3d384dc ("mtd: spi-nor: Add parallel and stacked memories support") Signed-off-by: Marek Vasut --- drivers/mtd/spi/spi-nor-core.c | 15 ++++++++------- 1 file changed, 8 insertions(+), 7 deletions(-) diff --git a/drivers/mtd/spi/spi-nor-core.c b/drivers/mtd/spi/spi-nor-core.c index e49b7cad023..c1ee466bcc0 100644 --- a/drivers/mtd/spi/spi-nor-core.c +++ b/drivers/mtd/spi/spi-nor-core.c @@ -1593,13 +1593,14 @@ static int spi_nor_read(struct mtd_info *mtd, loff_t from, size_t len, } while (len) { - if (nor->flags & SNOR_F_HAS_PARALLEL) { - bank = (u32)from / (SZ_16M << 0x01); - rem_bank_len = ((SZ_16M << 0x01) * (bank + 1)) - from; - } else { - bank = (u32)from / SZ_16M; - rem_bank_len = (SZ_16M * (bank + 1)) - from; - } + 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; From f896aa656774821fee44f0da3fdd7e52068130da Mon Sep 17 00:00:00 2001 From: Marek Vasut Date: Sat, 26 Oct 2024 22:16:25 +0200 Subject: [PATCH 8/9] mtd: spi-nor: Rename SPI_ADVANCE to SPI_STACKED_PARALLEL The SPI_ADVANCE description does not explain what the switch does. It does not have anything to do with any advanced functionality, it only gates off support for stacked and parallel SPI NORs. Rename the Kconfig symbol, update description, and move it right next to Xilinx hardware as it seems to be specific to this hardware. Make sure the symbol is also protected by if DM_SPI in Kconfig. Fixes: 5d40b3d384dc ("mtd: spi-nor: Add parallel and stacked memories support") Signed-off-by: Marek Vasut --- drivers/mtd/spi/spi-nor-core.c | 4 ++-- drivers/spi/Kconfig | 12 ++++++------ include/linux/mtd/spi-nor.h | 2 +- 3 files changed, 9 insertions(+), 9 deletions(-) diff --git a/drivers/mtd/spi/spi-nor-core.c b/drivers/mtd/spi/spi-nor-core.c index c1ee466bcc0..74aec6b60c4 100644 --- a/drivers/mtd/spi/spi-nor-core.c +++ b/drivers/mtd/spi/spi-nor-core.c @@ -3047,7 +3047,7 @@ static int spi_nor_init_params(struct spi_nor *nor, const struct flash_info *info, struct spi_nor_flash_parameter *params) { -#if CONFIG_IS_ENABLED(DM_SPI) && CONFIG_IS_ENABLED(SPI_ADVANCE) +#if CONFIG_IS_ENABLED(DM_SPI) && CONFIG_IS_ENABLED(SPI_STACKED_PARALLEL) struct udevice *dev = nor->spi->dev; u64 flash_size[SNOR_FLASH_CNT_MAX] = {0}; u32 idx = 0, i = 0; @@ -3172,7 +3172,7 @@ static int spi_nor_init_params(struct spi_nor *nor, spi_nor_post_sfdp_fixups(nor, params); } -#if CONFIG_IS_ENABLED(DM_SPI) && CONFIG_IS_ENABLED(SPI_ADVANCE) +#if CONFIG_IS_ENABLED(DM_SPI) && CONFIG_IS_ENABLED(SPI_STACKED_PARALLEL) /* * The flashes that are connected in stacked mode should be of same make. * Except the flash size all other properties are identical for all the diff --git a/drivers/spi/Kconfig b/drivers/spi/Kconfig index fa817ec4883..fd5cb3694f6 100644 --- a/drivers/spi/Kconfig +++ b/drivers/spi/Kconfig @@ -20,12 +20,6 @@ menuconfig SPI if SPI -config SPI_ADVANCE - bool "Enable the advance feature" - help - Enable the SPI advance feature support. By default this is disabled. - If you intend to use the advance feature support you should enable. - config DM_SPI bool "Enable Driver Model for SPI drivers" depends on DM @@ -615,6 +609,12 @@ config ZYNQMP_GQSPI This option is used to enable ZynqMP QSPI controller driver which is used to communicate with qspi flash devices. +config SPI_STACKED_PARALLEL + bool "Enable support for stacked or parallel memories" + help + Enable support for stacked/or parallel memories. This functionality + may appear on Xilinx hardware. By default this is disabled. + endif # if DM_SPI config FSL_ESPI diff --git a/include/linux/mtd/spi-nor.h b/include/linux/mtd/spi-nor.h index 655a6d197ea..b8b207f7b5c 100644 --- a/include/linux/mtd/spi-nor.h +++ b/include/linux/mtd/spi-nor.h @@ -308,7 +308,7 @@ enum spi_nor_option_flags { SNOR_F_BROKEN_RESET = BIT(6), SNOR_F_SOFT_RESET = BIT(7), SNOR_F_IO_MODE_EN_VOLATILE = BIT(8), -#if defined(CONFIG_SPI_ADVANCE) +#if defined(CONFIG_SPI_STACKED_PARALLEL) SNOR_F_HAS_STACKED = BIT(9), SNOR_F_HAS_PARALLEL = BIT(10), #else From 43423cdc5dc174468546c7c48f771fe26fe6d101 Mon Sep 17 00:00:00 2001 From: Marek Vasut Date: Sat, 26 Oct 2024 22:16:26 +0200 Subject: [PATCH 9/9] mtd: spi-nor: Always build SPI_STACKED_PARALLEL code Prevent the code gated by SPI_STACKED_PARALLEL from bitrot by using if (CONFIG_IS_ENABLED(SPI_STACKED_PARALLEL)) around it. That way, it is always at least compiled. Fixes: 5d40b3d384dc ("mtd: spi-nor: Add parallel and stacked memories support") Signed-off-by: Marek Vasut --- drivers/mtd/spi/spi-nor-core.c | 120 ++++++++++++++++----------------- 1 file changed, 60 insertions(+), 60 deletions(-) diff --git a/drivers/mtd/spi/spi-nor-core.c b/drivers/mtd/spi/spi-nor-core.c index 74aec6b60c4..a3a62fff213 100644 --- a/drivers/mtd/spi/spi-nor-core.c +++ b/drivers/mtd/spi/spi-nor-core.c @@ -3047,13 +3047,6 @@ static int spi_nor_init_params(struct spi_nor *nor, const struct flash_info *info, struct spi_nor_flash_parameter *params) { -#if CONFIG_IS_ENABLED(DM_SPI) && CONFIG_IS_ENABLED(SPI_STACKED_PARALLEL) - struct udevice *dev = nor->spi->dev; - u64 flash_size[SNOR_FLASH_CNT_MAX] = {0}; - u32 idx = 0, i = 0; - int rc; -#endif - /* Set legacy flash parameters as default. */ memset(params, 0, sizeof(*params)); @@ -3172,62 +3165,69 @@ static int spi_nor_init_params(struct spi_nor *nor, spi_nor_post_sfdp_fixups(nor, params); } -#if CONFIG_IS_ENABLED(DM_SPI) && CONFIG_IS_ENABLED(SPI_STACKED_PARALLEL) - /* - * The flashes that are connected in stacked mode should be of same make. - * Except the flash size all other properties are identical for all the - * flashes connected in stacked mode. - * The flashes that are connected in parallel mode should be identical. - */ - while (i < SNOR_FLASH_CNT_MAX) { - rc = ofnode_read_u64_index(dev_ofnode(dev), "stacked-memories", - idx, &flash_size[i]); - if (rc == -EINVAL) { - break; - } else if (rc == -EOVERFLOW) { - idx++; - } else { - idx++; - i++; - if (!(nor->flags & SNOR_F_HAS_STACKED)) - nor->flags |= SNOR_F_HAS_STACKED; - if (!(nor->spi->flags & SPI_XFER_STACKED)) - nor->spi->flags |= SPI_XFER_STACKED; + + if (CONFIG_IS_ENABLED(SPI_STACKED_PARALLEL)) { + u64 flash_size[SNOR_FLASH_CNT_MAX] = { 0 }; + struct udevice *dev = nor->spi->dev; + u32 idx = 0, i = 0; + int rc; + + /* + * The flashes that are connected in stacked mode should be of same make. + * Except the flash size all other properties are identical for all the + * flashes connected in stacked mode. + * The flashes that are connected in parallel mode should be identical. + */ + while (i < SNOR_FLASH_CNT_MAX) { + rc = ofnode_read_u64_index(dev_ofnode(dev), "stacked-memories", + idx, &flash_size[i]); + if (rc == -EINVAL) { + break; + } else if (rc == -EOVERFLOW) { + idx++; + } else { + idx++; + i++; + if (!(nor->flags & SNOR_F_HAS_STACKED)) + nor->flags |= SNOR_F_HAS_STACKED; + if (!(nor->spi->flags & SPI_XFER_STACKED)) + nor->spi->flags |= SPI_XFER_STACKED; + } + } + + i = 0; + idx = 0; + while (i < SNOR_FLASH_CNT_MAX) { + rc = ofnode_read_u64_index(dev_ofnode(dev), "parallel-memories", + idx, &flash_size[i]); + if (rc == -EINVAL) { + break; + } else if (rc == -EOVERFLOW) { + idx++; + } else { + idx++; + i++; + if (!(nor->flags & SNOR_F_HAS_PARALLEL)) + nor->flags |= SNOR_F_HAS_PARALLEL; + } + } + + if (nor->flags & (SNOR_F_HAS_STACKED | SNOR_F_HAS_PARALLEL)) { + params->size = 0; + for (idx = 0; idx < SNOR_FLASH_CNT_MAX; idx++) + params->size += flash_size[idx]; + } + /* + * In parallel-memories the erase operation is + * performed on both the flashes simultaneously + * so, double the erasesize. + */ + if (nor->flags & SNOR_F_HAS_PARALLEL) { + nor->mtd.erasesize <<= 1; + params->page_size <<= 1; } } - i = 0; - idx = 0; - while (i < SNOR_FLASH_CNT_MAX) { - rc = ofnode_read_u64_index(dev_ofnode(dev), "parallel-memories", - idx, &flash_size[i]); - if (rc == -EINVAL) { - break; - } else if (rc == -EOVERFLOW) { - idx++; - } else { - idx++; - i++; - if (!(nor->flags & SNOR_F_HAS_PARALLEL)) - nor->flags |= SNOR_F_HAS_PARALLEL; - } - } - - if (nor->flags & (SNOR_F_HAS_STACKED | SNOR_F_HAS_PARALLEL)) { - params->size = 0; - for (idx = 0; idx < SNOR_FLASH_CNT_MAX; idx++) - params->size += flash_size[idx]; - } - /* - * In parallel-memories the erase operation is - * performed on both the flashes simultaneously - * so, double the erasesize. - */ - if (nor->flags & SNOR_F_HAS_PARALLEL) { - nor->mtd.erasesize <<= 1; - params->page_size <<= 1; - } -#endif spi_nor_late_init_fixups(nor, params); return 0;