[PATCH v3 0/2] mtd: spi: nor: force mtd name to "nor%d"
This serie is a V3 for http://patchwork.ozlabs.org/project/uboot/list/?series=262017&state=* http://patchwork.ozlabs.org/project/uboot/list/?series=262013&state=*
Now the SPI nor are named "norN" with N after the CFI nor device: "nor0" to "norM" => N= M+1.
See also an other proposal from Marek (not working after test) "mtd: spi-nor: Fix SF MTDIDS when registering multiple MTDs with DM enabled"
http://patchwork.ozlabs.org/project/uboot/list/?series=262362
The first patch of the serie fixed the compilation issues around 'cfi_flash_num_flash_banks' found in CI:
https://source.denx.de/u-boot/custodians/u-boot-stm/-/pipelines/9138
Patrick
Changes in v3: - NEW: solve compilation issue when CONFIG_SYS_MAX_FLASH_BANKS is used - start index after the last CFI device, use CONFIG_SYS_MAX_FLASH_BANKS
Changes in v2: - correct commit message
Patrick Delaunay (2): mtd: cfi_flash: use cfi_flash_num_flash_banks only when supported mtd: spi: nor: force mtd name to "nor%d"
drivers/mtd/spi/spi-nor-core.c | 15 ++++++++++++--- include/linux/mtd/spi-nor.h | 1 + include/mtd/cfi_flash.h | 8 +++++++- 3 files changed, 20 insertions(+), 4 deletions(-)
When CONFIG_SYS_MAX_FLASH_BANKS_DETECT is activated, CONFIG_SYS_MAX_FLASH_BANKS is replaced by cfi_flash_num_flash_banks, but this variable is defined in drivers/mtd/cfi_flash.c, which is compiled only when CONFIG_FLASH_CFI_DRIVER is activated, in U-Boot or in SPL when CONFIG_SPL_MTD_SUPPORT is activated.
This patch deactivates this feature CONFIG_SYS_MAX_FLASH_BANKS_DETECT when flash cfi driver is not activated to avoid compilation issue in the next patch, when CONFIG_SYS_MAX_FLASH_BANKS is used in spi_nor_scan().
Signed-off-by: Patrick Delaunay patrick.delaunay@foss.st.com --- see error in https://source.denx.de/u-boot/custodians/u-boot-stm/-/pipelines/9138
drivers/mtd/spi/spi-nor-core.o: in function `spi_nor_scan': drivers/mtd/spi/spi-nor-core.c:3672: undefined reference to `cfi_flash_num_flash_banks'
compilation issue for the boards: - j721e_hs_evm_r5 - j721e_evm_r5j - j721e_hs_evm_a72 - j721e_evm_a72 - sagem_f@st1704_ram
Changes in v3: - NEW: solve compilation issue when CONFIG_SYS_MAX_FLASH_BANKS is used
include/mtd/cfi_flash.h | 8 +++++++- 1 file changed, 7 insertions(+), 1 deletion(-)
diff --git a/include/mtd/cfi_flash.h b/include/mtd/cfi_flash.h index 4963c89642..a1af6fc200 100644 --- a/include/mtd/cfi_flash.h +++ b/include/mtd/cfi_flash.h @@ -157,11 +157,17 @@ struct cfi_pri_hdr { * Use CONFIG_SYS_MAX_FLASH_BANKS_DETECT if defined */ #if defined(CONFIG_SYS_MAX_FLASH_BANKS_DETECT) -#define CONFIG_SYS_MAX_FLASH_BANKS (cfi_flash_num_flash_banks) #define CFI_MAX_FLASH_BANKS CONFIG_SYS_MAX_FLASH_BANKS_DETECT +/* map to cfi_flash_num_flash_banks only when supported */ +#if IS_ENABLED(CONFIG_FLASH_CFI_DRIVER) && \ + (!IS_ENABLED(CONFIG_SPL_BUILD) || IS_ENABLED(CONFIG_SPL_MTD_SUPPORT)) +#define CONFIG_SYS_MAX_FLASH_BANKS (cfi_flash_num_flash_banks) /* board code can update this variable before CFI detection */ extern int cfi_flash_num_flash_banks; #else +#define CONFIG_SYS_MAX_FLASH_BANKS CONFIG_SYS_MAX_FLASH_BANKS_DETECT +#endif +#else #define CFI_MAX_FLASH_BANKS CONFIG_SYS_MAX_FLASH_BANKS #endif
On 9/16/21 4:01 PM, Patrick Delaunay wrote:
When CONFIG_SYS_MAX_FLASH_BANKS_DETECT is activated, CONFIG_SYS_MAX_FLASH_BANKS is replaced by cfi_flash_num_flash_banks, but this variable is defined in drivers/mtd/cfi_flash.c, which is compiled only when CONFIG_FLASH_CFI_DRIVER is activated, in U-Boot or in SPL when CONFIG_SPL_MTD_SUPPORT is activated.
This patch deactivates this feature CONFIG_SYS_MAX_FLASH_BANKS_DETECT when flash cfi driver is not activated to avoid compilation issue in the next patch, when CONFIG_SYS_MAX_FLASH_BANKS is used in spi_nor_scan().
Maybe just migrate this config option to Kconfig and let Kconfig handle the macro magic ?
Hi Marek,
Marek VasutSept. 16, 2021, 5:24 p.m. UTC | #1
On 9/16/21 4:01 PM, Patrick Delaunay wrote:
When CONFIG_SYS_MAX_FLASH_BANKS_DETECT is activated, CONFIG_SYS_MAX_FLASH_BANKS is replaced by cfi_flash_num_flash_banks, but this variable is defined in drivers/mtd/cfi_flash.c, which is compiled only when CONFIG_FLASH_CFI_DRIVER is activated, in U-Boot or in SPL when CONFIG_SPL_MTD_SUPPORT is activated.
This patch deactivates this feature CONFIG_SYS_MAX_FLASH_BANKS_DETECT when flash cfi driver is not activated to avoid compilation issue in the next patch, when CONFIG_SYS_MAX_FLASH_BANKS is used in
spi_nor_scan().
Maybe just migrate this config option to Kconfig and let Kconfig handle the macro magic ?
Sorry for the format of my answer (it is just copy paste from archive)
because I don't received the U-Boot mails on my @foss.st.com mailbo
since yesterday.
I think about migration but is difficult to don't break the existing behaviour in kconfig
CONFIG_SYS_MAX_FLASH_BANKS and CONFIG_SYS_MAX_FLASH_BANKS_DETECT are define as 'int'
but can be absent => 2 new config CONFIG_USE need to be added
CONFIG_USE_SYS_MAX_FLASH_BANKS
CONFIG_USE_SYS_MAX_FLASH_BANKS_DETECT
and I don't fully understood the mix between the 2 options and CFI_MAX_FLASH_BANKS
in some part of code I think CONFIG_SYS_MAX_FLASH_BANKS should be replaced by CFI_MAX_FLASH_BANKS
to avoid to define CONFIG_SYS_MAX_FLASH_BANKS = cfi_flash_num_flash_banks (as it is not possible in Kconfig)
=> too huge task just to solve compilation issues.
and I also think to use CONFIG_IS_ENABLED(MTD_SUPPORT)
but it not possible because today
- CONFIG_SPL_MTD_SUPPORT exist
- CONFIG_MTD_SUPPORT don't exit ( test on $(mtd-y) in Makefile)
=> the creation of this config is a huge task just to solve compilation issue.
Patrick
On 9/17/21 12:55 PM, Patrick DELAUNAY wrote:
Hi Marek,
Marek VasutSept. 16, 2021, 5:24 p.m. UTC | #1
On 9/16/21 4:01 PM, Patrick Delaunay wrote:
When CONFIG_SYS_MAX_FLASH_BANKS_DETECT is activated, CONFIG_SYS_MAX_FLASH_BANKS is replaced by cfi_flash_num_flash_banks, but this variable is defined in drivers/mtd/cfi_flash.c, which is compiled only when CONFIG_FLASH_CFI_DRIVER is activated, in U-Boot or in SPL when CONFIG_SPL_MTD_SUPPORT is activated.
This patch deactivates this feature CONFIG_SYS_MAX_FLASH_BANKS_DETECT when flash cfi driver is not activated to avoid compilation issue in the next patch, when CONFIG_SYS_MAX_FLASH_BANKS is used in
spi_nor_scan().
Maybe just migrate this config option to Kconfig and let Kconfig handle the macro magic ?
Sorry for the format of my answer (it is just copy paste from archive)
because I don't received the U-Boot mails on my @foss.st.com mailbo
since yesterday.
I think about migration but is difficult to don't break the existing behaviour in kconfig
CONFIG_SYS_MAX_FLASH_BANKS and CONFIG_SYS_MAX_FLASH_BANKS_DETECT are define as 'int'
but can be absent => 2 new config CONFIG_USE need to be added
CONFIG_USE_SYS_MAX_FLASH_BANKS
CONFIG_USE_SYS_MAX_FLASH_BANKS_DETECT
and I don't fully understood the mix between the 2 options and CFI_MAX_FLASH_BANKS
in some part of code I think CONFIG_SYS_MAX_FLASH_BANKS should be replaced by CFI_MAX_FLASH_BANKS
to avoid to define CONFIG_SYS_MAX_FLASH_BANKS = cfi_flash_num_flash_banks (as it is not possible in Kconfig)
=> too huge task just to solve compilation issues.
and I also think to use CONFIG_IS_ENABLED(MTD_SUPPORT)
but it not possible because today
CONFIG_SPL_MTD_SUPPORT exist
CONFIG_MTD_SUPPORT don't exit ( test on $(mtd-y) in Makefile)
=> the creation of this config is a huge task just to solve compilation issue.
All right, well, ew.
Can you send a subsequent patchset _after_ this one to fix this flash banks mess ?
Hi,
On 9/17/21 3:36 PM, Marek Vasut wrote:
On 9/17/21 12:55 PM, Patrick DELAUNAY wrote:
Hi Marek,
> Marek VasutSept. 16, 2021, 5:24 p.m. UTC | #1
> On 9/16/21 4:01 PM, Patrick Delaunay wrote:
>> When CONFIG_SYS_MAX_FLASH_BANKS_DETECT is activated, >> CONFIG_SYS_MAX_FLASH_BANKS is replaced by cfi_flash_num_flash_banks, >> but this variable is defined in drivers/mtd/cfi_flash.c, which is >> compiled only when CONFIG_FLASH_CFI_DRIVER is activated, in U-Boot >> or in SPL when CONFIG_SPL_MTD_SUPPORT is activated. >> >> This patch deactivates this feature CONFIG_SYS_MAX_FLASH_BANKS_DETECT >> when flash cfi driver is not activated to avoid compilation issue in >> the next patch, when CONFIG_SYS_MAX_FLASH_BANKS is used in spi_nor_scan().
> Maybe just migrate this config option to Kconfig and let Kconfig handle > the macro magic ?
Sorry for the format of my answer (it is just copy paste from archive)
because I don't received the U-Boot mails on my @foss.st.com mailbo
since yesterday.
I think about migration but is difficult to don't break the existing behaviour in kconfig
CONFIG_SYS_MAX_FLASH_BANKS and CONFIG_SYS_MAX_FLASH_BANKS_DETECT are define as 'int'
but can be absent => 2 new config CONFIG_USE need to be added
CONFIG_USE_SYS_MAX_FLASH_BANKS
CONFIG_USE_SYS_MAX_FLASH_BANKS_DETECT
and I don't fully understood the mix between the 2 options and CFI_MAX_FLASH_BANKS
in some part of code I think CONFIG_SYS_MAX_FLASH_BANKS should be replaced by CFI_MAX_FLASH_BANKS
to avoid to define CONFIG_SYS_MAX_FLASH_BANKS = cfi_flash_num_flash_banks (as it is not possible in Kconfig)
=> too huge task just to solve compilation issues.
and I also think to use CONFIG_IS_ENABLED(MTD_SUPPORT)
but it not possible because today
CONFIG_SPL_MTD_SUPPORT exist
CONFIG_MTD_SUPPORT don't exit ( test on $(mtd-y) in Makefile)
=> the creation of this config is a huge task just to solve compilation issue.
All right, well, ew.
Can you send a subsequent patchset _after_ this one to fix this flash banks mess ?
I can try,
I will add it in my TODO list.
Patrick
Force the mtd name of spi-nor to "nor" + the driver sequence number: "nor0", "nor1"... beginning after the existing nor devices.
This patch is coherent with existing "nand" and "spi-nand" mtd device names.
When CFI MTD NOR device are supported, the spi-nor index is chosen after the last CFI device defined by CONFIG_SYS_MAX_FLASH_BANKS.
When CONFIG_SYS_MAX_FLASH_BANKS_DETECT is activated, this config is replaced by to cfi_flash_num_flash_banks in the include file mtd/cfi_flash.h.
This generic name "nor%d" can be use to identify the mtd spi-nor device without knowing the real device name or the DT path of the device, used with API get_mtd_device_nm() and is used in mtdparts command.
This patch also avoids issue when the same NOR device is present 2 times, for example on STM32MP15F-EV1:
STM32MP> mtd list SF: Detected mx66l51235l with page size 256 Bytes, erase size 64 KiB, \ total 64 MiB
List of MTD devices: * nand0 - type: NAND flash - block size: 0x40000 bytes - min I/O: 0x1000 bytes - OOB size: 224 bytes - OOB available: 118 bytes - ECC strength: 8 bits - ECC step size: 512 bytes - bitflip threshold: 6 bits - 0x000000000000-0x000040000000 : "nand0" * mx66l51235l - device: mx66l51235l@0 - parent: spi@58003000 - driver: jedec_spi_nor - path: /soc/spi@58003000/mx66l51235l@0 - type: NOR flash - block size: 0x10000 bytes - min I/O: 0x1 bytes - 0x000000000000-0x000004000000 : "mx66l51235l" * mx66l51235l - device: mx66l51235l@1 - parent: spi@58003000 - driver: jedec_spi_nor - path: /soc/spi@58003000/mx66l51235l@1 - type: NOR flash - block size: 0x10000 bytes - min I/O: 0x1 bytes - 0x000000000000-0x000004000000 : "mx66l51235l"
The same mtd name "mx66l51235l" identify the 2 instances mx66l51235l@0 and mx66l51235l@1.
This patch fixes a ST32CubeProgrammer / stm32prog command issue with nor0 target on STM32MP157C-EV1 board introduced by commit b7f060565e31 ("mtd: spi-nor: allow registering multiple MTDs when DM is enabled").
Fixes: b7f060565e31 ("mtd: spi-nor: allow registering multiple MTDs when DM is enabled") Signed-off-by: Patrick Delaunay patrick.delaunay@foss.st.com --- For other device, the mtd name are hardcoded in each MTD driver:
drivers/mtd/nand/spi/core.c:1169: sprintf(mtd->name, "spi-nand%d", spi_nand_idx++); drivers/mtd/nand/raw/nand.c:59: sprintf(dev_name[devnum], "nand%d", devnum);
And the nor device name "nor%d" is also used for CFI in ./drivers/mtd/cfi_mtd.c with i=0 to CONFIG_SYS_MAX_FLASH_BANKS - 1
sprintf(cfi_mtd_names[i], "nor%d", i); mtd->name = cfi_mtd_names[i];
Today the number of CFI device is hardcoded by this config.
Changes in v3: - start index after the last CFI device, use CONFIG_SYS_MAX_FLASH_BANKS
Changes in v2: - correct commit message
drivers/mtd/spi/spi-nor-core.c | 15 ++++++++++++--- include/linux/mtd/spi-nor.h | 1 + 2 files changed, 13 insertions(+), 3 deletions(-)
diff --git a/drivers/mtd/spi/spi-nor-core.c b/drivers/mtd/spi/spi-nor-core.c index d5d905fa5a..7da5ca6285 100644 --- a/drivers/mtd/spi/spi-nor-core.c +++ b/drivers/mtd/spi/spi-nor-core.c @@ -10,6 +10,7 @@ */
#include <common.h> +#include <flash.h> #include <log.h> #include <watchdog.h> #include <dm.h> @@ -26,6 +27,7 @@
#include <linux/mtd/mtd.h> #include <linux/mtd/spi-nor.h> +#include <mtd/cfi_flash.h> #include <spi-mem.h> #include <spi.h>
@@ -3664,6 +3666,11 @@ int spi_nor_scan(struct spi_nor *nor) struct mtd_info *mtd = &nor->mtd; struct spi_slave *spi = nor->spi; int ret; + int cfi_mtd_nb = 0; + +#ifdef CONFIG_SYS_MAX_FLASH_BANKS + cfi_mtd_nb = CONFIG_SYS_MAX_FLASH_BANKS; +#endif
/* Reset SPI protocol for all commands. */ nor->reg_proto = SNOR_PROTO_1_1_1; @@ -3715,8 +3722,10 @@ int spi_nor_scan(struct spi_nor *nor) if (ret) return ret;
- if (!mtd->name) - mtd->name = info->name; + if (!mtd->name) { + sprintf(nor->mtd_name, "nor%d", cfi_mtd_nb + dev_seq(nor->dev)); + mtd->name = nor->mtd_name; + } mtd->dev = nor->dev; mtd->priv = nor; mtd->type = MTD_NORFLASH; @@ -3821,7 +3830,7 @@ int spi_nor_scan(struct spi_nor *nor)
nor->rdsr_dummy = params.rdsr_dummy; nor->rdsr_addr_nbytes = params.rdsr_addr_nbytes; - nor->name = mtd->name; + nor->name = info->name; nor->size = mtd->size; nor->erase_size = mtd->erasesize; nor->sector_size = mtd->erasesize; diff --git a/include/linux/mtd/spi-nor.h b/include/linux/mtd/spi-nor.h index 7ddc4ba2bf..8c3d5032e3 100644 --- a/include/linux/mtd/spi-nor.h +++ b/include/linux/mtd/spi-nor.h @@ -561,6 +561,7 @@ struct spi_nor { int (*ready)(struct spi_nor *nor);
void *priv; + char mtd_name[10]; /* Compatibility for spi_flash, remove once sf layer is merged with mtd */ const char *name; u32 size;
On Thu, 16 Sep 2021 16:01:18 +0200 Patrick Delaunay patrick.delaunay@foss.st.com wrote:
Force the mtd name of spi-nor to "nor" + the driver sequence number: "nor0", "nor1"... beginning after the existing nor devices.
This patch is coherent with existing "nand" and "spi-nand" mtd device names.
When CFI MTD NOR device are supported, the spi-nor index is chosen after the last CFI device defined by CONFIG_SYS_MAX_FLASH_BANKS.
When CONFIG_SYS_MAX_FLASH_BANKS_DETECT is activated, this config is replaced by to cfi_flash_num_flash_banks in the include file mtd/cfi_flash.h.
This generic name "nor%d" can be use to identify the mtd spi-nor device without knowing the real device name or the DT path of the device, used with API get_mtd_device_nm() and is used in mtdparts command.
This patch also avoids issue when the same NOR device is present 2 times, for example on STM32MP15F-EV1:
This is an unfortunate hack :( This is another reason why the whole mtd subsystem should be refactored.
Hi Marek, Patrick,
the patches that made this change also include patch https://source.denx.de/u-boot/u-boot/-/commit/dcb9a80359d699cf659c95b9b6e660...
This patch was added so that when there are multiple identical SPI-NOR chips on the board, you can still select between them in the `mtd` command via the OF path. That's why the `mtd list` command lists OF path.
In the discussion with Tom, we also were talking about backwards compatibility with mtdparts, and if I remember correctly, the conclusion was that mtdparts is deprecated: - many configs with MTDPARTS can now be converted to define the partitions via OF - the remaining which can't need their mtd driver updated
Afterwards there will be no need for mtdpart.
I think this is the correct solution here. Not the one where we are returning back to the "norN" names.
Marek
On 9/16/21 4:01 PM, Patrick Delaunay wrote:
[...]
@@ -3664,6 +3666,11 @@ int spi_nor_scan(struct spi_nor *nor) struct mtd_info *mtd = &nor->mtd; struct spi_slave *spi = nor->spi; int ret;
- int cfi_mtd_nb = 0;
+#ifdef CONFIG_SYS_MAX_FLASH_BANKS
- cfi_mtd_nb = CONFIG_SYS_MAX_FLASH_BANKS;
+#endif
Are we covering all the NORs (HF and co.) with this ?
/* Reset SPI protocol for all commands. */ nor->reg_proto = SNOR_PROTO_1_1_1; @@ -3715,8 +3722,10 @@ int spi_nor_scan(struct spi_nor *nor) if (ret) return ret;
- if (!mtd->name)
mtd->name = info->name;
- if (!mtd->name) {
sprintf(nor->mtd_name, "nor%d", cfi_mtd_nb + dev_seq(nor->dev));mtd->name = nor->mtd_name;- } mtd->dev = nor->dev; mtd->priv = nor; mtd->type = MTD_NORFLASH;
@@ -3821,7 +3830,7 @@ int spi_nor_scan(struct spi_nor *nor)
nor->rdsr_dummy = params.rdsr_dummy; nor->rdsr_addr_nbytes = params.rdsr_addr_nbytes;
- nor->name = mtd->name;
- nor->name = info->name; nor->size = mtd->size; nor->erase_size = mtd->erasesize; nor->sector_size = mtd->erasesize;
diff --git a/include/linux/mtd/spi-nor.h b/include/linux/mtd/spi-nor.h index 7ddc4ba2bf..8c3d5032e3 100644 --- a/include/linux/mtd/spi-nor.h +++ b/include/linux/mtd/spi-nor.h @@ -561,6 +561,7 @@ struct spi_nor { int (*ready)(struct spi_nor *nor);
void *priv;
- char mtd_name[10];
should be 14, because nor%d\0 can be up to 14 bytes long.
Hi Marek,
Marek VasutSept. 16, 2021, 5:27 p.m. UTC | #3 On 9/16/21 4:01 PM, Patrick Delaunay wrote:
[...]
@@ -3664,6 +3666,11 @@ int spi_nor_scan(struct spi_nor *nor) struct mtd_info *mtd = &nor->mtd; struct spi_slave *spi = nor->spi; int ret; + int cfi_mtd_nb = 0;
+#ifdef CONFIG_SYS_MAX_FLASH_BANKS + cfi_mtd_nb = CONFIG_SYS_MAX_FLASH_BANKS; +#endif
Are we covering all the NORs (HF and co.) with this ?
Yes, except if I miss something
any NOR (including hyperflash) wich use the CFI interface / CONFIG_FLASH_CFI_MTD
define the the 'nor%d' name with the calling stack:
initr_flash()
=> flash_init()
==> cfi_flash_init_dm()
==> cfi_mtd_init() "nor%d" wich use loop on CONFIG_SYS_MAX_FLASH_BANKS
I have only one concern today...
if one cfi bank is missing (not activated in DT by example)
and CONFIG_SYS_MAX_FLASH_BANKS_DETECT is not activated
some holes can be done in index
example:
with CONFIG_SYS_MAX_FLASH_BANKS = 2 but with only one NOR on the board
=> "nor1" is absent and we have only 2 MTD device named "nor"
- "nor0" => NOR or HF, first CFI bank
- "nor2" => SPI-NOR (first)
but I don't think that it is blocking
/* Reset SPI protocol for all commands. */ nor->reg_proto = SNOR_PROTO_1_1_1; @@ -3715,8 +3722,10 @@ int spi_nor_scan(struct spi_nor *nor) if (ret) return ret;
- if (!mtd->name) - mtd->name = info->name; + if (!mtd->name) { + sprintf(nor->mtd_name, "nor%d", cfi_mtd_nb +
dev_seq(nor->dev));
+ mtd->name = nor->mtd_name; + } mtd->dev = nor->dev; mtd->priv = nor; mtd->type = MTD_NORFLASH; @@ -3821,7 +3830,7 @@ int spi_nor_scan(struct spi_nor *nor)
nor->rdsr_dummy = params.rdsr_dummy; nor->rdsr_addr_nbytes = params.rdsr_addr_nbytes; - nor->name = mtd->name; + nor->name = info->name; nor->size = mtd->size; nor->erase_size = mtd->erasesize; nor->sector_size = mtd->erasesize; diff --git a/include/linux/mtd/spi-nor.h b/include/linux/mtd/spi-nor.h index 7ddc4ba2bf..8c3d5032e3 100644 --- a/include/linux/mtd/spi-nor.h +++ b/include/linux/mtd/spi-nor.h @@ -561,6 +561,7 @@ struct spi_nor { int (*ready)(struct spi_nor *nor);
void *priv; + char mtd_name[10];
should be 14, because nor%d\0 can be up to 14 bytes long.
normally DM_MAX_SEQ = 999 (but never used)
but Ok with you for 14 with "nor" = 3 + "%d" = 10 for max u32 value + "/0" = 1
for cfi_mtd_names => 16 byte used with "nor%d"
static char cfi_mtd_names[CFI_MAX_FLASH_BANKS][16];
for nand => 8 bytes (./drivers/mtd/nand/raw/nand.c:59)
static char dev_name[CONFIG_SYS_MAX_NAND_DEVICE][8];
for spi-nand => 20 bytes (drivers/mtd/nand/spi/core.c:1169)
mtd->name = malloc(20);
Patrick
On 9/17/21 3:06 PM, Patrick DELAUNAY wrote:
Hi Marek,
Marek VasutSept. 16, 2021, 5:27 p.m. UTC | #3 On 9/16/21 4:01 PM, Patrick Delaunay wrote:
[...]
@@ -3664,6 +3666,11 @@ int spi_nor_scan(struct spi_nor *nor) struct mtd_info *mtd = &nor->mtd; struct spi_slave *spi = nor->spi; int ret; + int cfi_mtd_nb = 0;
+#ifdef CONFIG_SYS_MAX_FLASH_BANKS + cfi_mtd_nb = CONFIG_SYS_MAX_FLASH_BANKS; +#endif
Are we covering all the NORs (HF and co.) with this ?
Yes, except if I miss something
any NOR (including hyperflash) wich use the CFI interface / CONFIG_FLASH_CFI_MTD
define the the 'nor%d' name with the calling stack:
Yes
initr_flash()
=> flash_init()
==> cfi_flash_init_dm()
==> cfi_mtd_init() "nor%d" wich use loop on CONFIG_SYS_MAX_FLASH_BANKS
I have only one concern today...
if one cfi bank is missing (not activated in DT by example)
and CONFIG_SYS_MAX_FLASH_BANKS_DETECT is not activated
some holes can be done in index
example:
with CONFIG_SYS_MAX_FLASH_BANKS = 2 but with only one NOR on the board
=> "nor1" is absent and we have only 2 MTD device named "nor"
"nor0" => NOR or HF, first CFI bank
"nor2" => SPI-NOR (first)
but I don't think that it is blocking
Wasn't the old behavior exactly the same anyway ?
/* Reset SPI protocol for all commands. */ nor->reg_proto = SNOR_PROTO_1_1_1; @@ -3715,8 +3722,10 @@ int spi_nor_scan(struct spi_nor *nor) if (ret) return ret;
- if (!mtd->name) - mtd->name = info->name; + if (!mtd->name) { + sprintf(nor->mtd_name, "nor%d", cfi_mtd_nb +
dev_seq(nor->dev));
+ mtd->name = nor->mtd_name; + } mtd->dev = nor->dev; mtd->priv = nor; mtd->type = MTD_NORFLASH; @@ -3821,7 +3830,7 @@ int spi_nor_scan(struct spi_nor *nor)
nor->rdsr_dummy = params.rdsr_dummy; nor->rdsr_addr_nbytes = params.rdsr_addr_nbytes; - nor->name = mtd->name; + nor->name = info->name; nor->size = mtd->size; nor->erase_size = mtd->erasesize; nor->sector_size = mtd->erasesize; diff --git a/include/linux/mtd/spi-nor.h b/include/linux/mtd/spi-nor.h index 7ddc4ba2bf..8c3d5032e3 100644 --- a/include/linux/mtd/spi-nor.h +++ b/include/linux/mtd/spi-nor.h @@ -561,6 +561,7 @@ struct spi_nor { int (*ready)(struct spi_nor *nor);
void *priv; + char mtd_name[10];
should be 14, because nor%d\0 can be up to 14 bytes long.
normally DM_MAX_SEQ = 999 (but never used)
but Ok with you for 14 with "nor" = 3 + "%d" = 10 for max u32 value + "/0" = 1
for cfi_mtd_names => 16 byte used with "nor%d"
static char cfi_mtd_names[CFI_MAX_FLASH_BANKS][16];
for nand => 8 bytes (./drivers/mtd/nand/raw/nand.c:59)
static char dev_name[CONFIG_SYS_MAX_NAND_DEVICE][8];
for spi-nand => 20 bytes (drivers/mtd/nand/spi/core.c:1169)
mtd->name = malloc(20);
Maybe we need a macro for this length of DM seq string which we could use in these embedded array cases.
On 9/16/21 4:01 PM, Patrick Delaunay wrote:
This serie is a V3 for http://patchwork.ozlabs.org/project/uboot/list/?series=262017&state=* http://patchwork.ozlabs.org/project/uboot/list/?series=262013&state=*
Now the SPI nor are named "norN" with N after the CFI nor device: "nor0" to "norM" => N= M+1.
See also an other proposal from Marek (not working after test) "mtd: spi-nor: Fix SF MTDIDS when registering multiple MTDs with DM enabled"
http://patchwork.ozlabs.org/project/uboot/list/?series=262362
The first patch of the serie fixed the compilation issues around 'cfi_flash_num_flash_banks' found in CI:
https://source.denx.de/u-boot/custodians/u-boot-stm/-/pipelines/9138
Patrick
This looks good to me, except for a few nits.
participants (5)
-
Marek Behún -
Marek Behún -
Marek Vasut -
Patrick DELAUNAY -
Patrick Delaunay