* [PATCH v3 0/2] mtd: spi: nor: force mtd name to "nor%d"
@ 2021-09-16 14:01 Patrick Delaunay
2021-09-16 14:01 ` [PATCH v3 1/2] mtd: cfi_flash: use cfi_flash_num_flash_banks only when supported Patrick Delaunay
` (2 more replies)
0 siblings, 3 replies; 13+ messages in thread
From: Patrick Delaunay @ 2021-09-16 14:01 UTC (permalink / raw)
To: u-boot
Cc: Priyanka Jain, Heiko Schocher, Marek Vasut, Jagan Teki,
Miquel Raynal, Christophe KERELLO, Patrice Chotard,
Pali Rohár, Patrick Delaunay, Marek Behún, Vignesh R,
U-Boot STM32
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(-)
--
2.25.1
^ permalink raw reply [flat|nested] 13+ messages in thread
* [PATCH v3 1/2] mtd: cfi_flash: use cfi_flash_num_flash_banks only when supported
2021-09-16 14:01 [PATCH v3 0/2] mtd: spi: nor: force mtd name to "nor%d" Patrick Delaunay
@ 2021-09-16 14:01 ` Patrick Delaunay
2021-09-16 17:24 ` Marek Vasut
2021-09-17 10:55 ` Patrick DELAUNAY
2021-09-16 14:01 ` [PATCH v3 2/2] mtd: spi: nor: force mtd name to "nor%d" Patrick Delaunay
2021-09-16 17:43 ` [PATCH v3 0/2] " Marek Vasut
2 siblings, 2 replies; 13+ messages in thread
From: Patrick Delaunay @ 2021-09-16 14:01 UTC (permalink / raw)
To: u-boot
Cc: Priyanka Jain, Heiko Schocher, Marek Vasut, Jagan Teki,
Miquel Raynal, Christophe KERELLO, Patrice Chotard,
Pali Rohár, Patrick Delaunay, U-Boot STM32
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
--
2.25.1
^ permalink raw reply related [flat|nested] 13+ messages in thread
* [PATCH v3 2/2] mtd: spi: nor: force mtd name to "nor%d"
2021-09-16 14:01 [PATCH v3 0/2] mtd: spi: nor: force mtd name to "nor%d" Patrick Delaunay
2021-09-16 14:01 ` [PATCH v3 1/2] mtd: cfi_flash: use cfi_flash_num_flash_banks only when supported Patrick Delaunay
@ 2021-09-16 14:01 ` Patrick Delaunay
2021-09-16 14:49 ` Marek Behún
` (3 more replies)
2021-09-16 17:43 ` [PATCH v3 0/2] " Marek Vasut
2 siblings, 4 replies; 13+ messages in thread
From: Patrick Delaunay @ 2021-09-16 14:01 UTC (permalink / raw)
To: u-boot
Cc: Priyanka Jain, Heiko Schocher, Marek Vasut, Jagan Teki,
Miquel Raynal, Christophe KERELLO, Patrice Chotard,
Pali Rohár, Patrick Delaunay, Marek Behún, Vignesh R,
U-Boot STM32
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;
--
2.25.1
^ permalink raw reply related [flat|nested] 13+ messages in thread
* Re: [PATCH v3 2/2] mtd: spi: nor: force mtd name to "nor%d"
2021-09-16 14:01 ` [PATCH v3 2/2] mtd: spi: nor: force mtd name to "nor%d" Patrick Delaunay
@ 2021-09-16 14:49 ` Marek Behún
2021-09-16 15:00 ` Marek Behún
` (2 subsequent siblings)
3 siblings, 0 replies; 13+ messages in thread
From: Marek Behún @ 2021-09-16 14:49 UTC (permalink / raw)
To: Patrick Delaunay
Cc: u-boot, Priyanka Jain, Heiko Schocher, Marek Vasut, Jagan Teki,
Miquel Raynal, Christophe KERELLO, Patrice Chotard,
Pali Rohár, Vignesh R, U-Boot STM32
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.
^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: [PATCH v3 2/2] mtd: spi: nor: force mtd name to "nor%d"
2021-09-16 14:01 ` [PATCH v3 2/2] mtd: spi: nor: force mtd name to "nor%d" Patrick Delaunay
2021-09-16 14:49 ` Marek Behún
@ 2021-09-16 15:00 ` Marek Behún
2021-09-16 17:27 ` Marek Vasut
2021-09-17 13:06 ` Patrick DELAUNAY
3 siblings, 0 replies; 13+ messages in thread
From: Marek Behún @ 2021-09-16 15:00 UTC (permalink / raw)
To: Patrick Delaunay, Marek Vasut, Tom Rini
Cc: u-boot, Priyanka Jain, Heiko Schocher, Jagan Teki, Miquel Raynal,
Christophe KERELLO, Patrice Chotard, Pali Rohár, Vignesh R,
U-Boot STM32
Hi Marek, Patrick,
the patches that made this change also include patch
https://source.denx.de/u-boot/u-boot/-/commit/dcb9a80359d699cf659c95b9b6e6604e2d68adb9
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
^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: [PATCH v3 1/2] mtd: cfi_flash: use cfi_flash_num_flash_banks only when supported
2021-09-16 14:01 ` [PATCH v3 1/2] mtd: cfi_flash: use cfi_flash_num_flash_banks only when supported Patrick Delaunay
@ 2021-09-16 17:24 ` Marek Vasut
2021-09-17 10:55 ` Patrick DELAUNAY
1 sibling, 0 replies; 13+ messages in thread
From: Marek Vasut @ 2021-09-16 17:24 UTC (permalink / raw)
To: Patrick Delaunay, u-boot
Cc: Priyanka Jain, Heiko Schocher, Jagan Teki, Miquel Raynal,
Christophe KERELLO, Patrice Chotard, Pali Rohár,
U-Boot STM32
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 ?
^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: [PATCH v3 2/2] mtd: spi: nor: force mtd name to "nor%d"
2021-09-16 14:01 ` [PATCH v3 2/2] mtd: spi: nor: force mtd name to "nor%d" Patrick Delaunay
2021-09-16 14:49 ` Marek Behún
2021-09-16 15:00 ` Marek Behún
@ 2021-09-16 17:27 ` Marek Vasut
2021-09-17 13:06 ` Patrick DELAUNAY
3 siblings, 0 replies; 13+ messages in thread
From: Marek Vasut @ 2021-09-16 17:27 UTC (permalink / raw)
To: Patrick Delaunay, u-boot
Cc: Priyanka Jain, Heiko Schocher, Jagan Teki, Miquel Raynal,
Christophe KERELLO, Patrice Chotard, Pali Rohár,
Marek Behún, Vignesh R, U-Boot STM32
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.
^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: [PATCH v3 0/2] mtd: spi: nor: force mtd name to "nor%d"
2021-09-16 14:01 [PATCH v3 0/2] mtd: spi: nor: force mtd name to "nor%d" Patrick Delaunay
2021-09-16 14:01 ` [PATCH v3 1/2] mtd: cfi_flash: use cfi_flash_num_flash_banks only when supported Patrick Delaunay
2021-09-16 14:01 ` [PATCH v3 2/2] mtd: spi: nor: force mtd name to "nor%d" Patrick Delaunay
@ 2021-09-16 17:43 ` Marek Vasut
2 siblings, 0 replies; 13+ messages in thread
From: Marek Vasut @ 2021-09-16 17:43 UTC (permalink / raw)
To: Patrick Delaunay, u-boot
Cc: Priyanka Jain, Heiko Schocher, Jagan Teki, Miquel Raynal,
Christophe KERELLO, Patrice Chotard, Pali Rohár,
Marek Behún, Vignesh R, U-Boot STM32
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.
^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: [PATCH v3 1/2] mtd: cfi_flash: use cfi_flash_num_flash_banks only when supported
2021-09-16 14:01 ` [PATCH v3 1/2] mtd: cfi_flash: use cfi_flash_num_flash_banks only when supported Patrick Delaunay
2021-09-16 17:24 ` Marek Vasut
@ 2021-09-17 10:55 ` Patrick DELAUNAY
2021-09-17 13:36 ` Marek Vasut
1 sibling, 1 reply; 13+ messages in thread
From: Patrick DELAUNAY @ 2021-09-17 10:55 UTC (permalink / raw)
To: u-boot
Cc: Priyanka Jain, Heiko Schocher, Marek Vasut, Jagan Teki,
Miquel Raynal, Christophe KERELLO, Patrice Chotard,
Pali Rohár, U-Boot STM32
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
^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: [PATCH v3 2/2] mtd: spi: nor: force mtd name to "nor%d"
2021-09-16 14:01 ` [PATCH v3 2/2] mtd: spi: nor: force mtd name to "nor%d" Patrick Delaunay
` (2 preceding siblings ...)
2021-09-16 17:27 ` Marek Vasut
@ 2021-09-17 13:06 ` Patrick DELAUNAY
2021-09-17 13:39 ` Marek Vasut
3 siblings, 1 reply; 13+ messages in thread
From: Patrick DELAUNAY @ 2021-09-17 13:06 UTC (permalink / raw)
To: u-boot
Cc: Priyanka Jain, Heiko Schocher, Marek Vasut, Jagan Teki,
Miquel Raynal, Christophe KERELLO, Patrice Chotard,
Pali Rohár, Marek Behún, Vignesh R, U-Boot STM32
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
^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: [PATCH v3 1/2] mtd: cfi_flash: use cfi_flash_num_flash_banks only when supported
2021-09-17 10:55 ` Patrick DELAUNAY
@ 2021-09-17 13:36 ` Marek Vasut
2021-09-21 12:38 ` Patrick DELAUNAY
0 siblings, 1 reply; 13+ messages in thread
From: Marek Vasut @ 2021-09-17 13:36 UTC (permalink / raw)
To: Patrick DELAUNAY, u-boot
Cc: Priyanka Jain, Heiko Schocher, Jagan Teki, Miquel Raynal,
Christophe KERELLO, Patrice Chotard, Pali Rohár,
U-Boot STM32
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 ?
^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: [PATCH v3 2/2] mtd: spi: nor: force mtd name to "nor%d"
2021-09-17 13:06 ` Patrick DELAUNAY
@ 2021-09-17 13:39 ` Marek Vasut
0 siblings, 0 replies; 13+ messages in thread
From: Marek Vasut @ 2021-09-17 13:39 UTC (permalink / raw)
To: Patrick DELAUNAY, u-boot
Cc: Priyanka Jain, Heiko Schocher, Jagan Teki, Miquel Raynal,
Christophe KERELLO, Patrice Chotard, Pali Rohár,
Marek Behún, Vignesh R, U-Boot STM32
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.
^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: [PATCH v3 1/2] mtd: cfi_flash: use cfi_flash_num_flash_banks only when supported
2021-09-17 13:36 ` Marek Vasut
@ 2021-09-21 12:38 ` Patrick DELAUNAY
0 siblings, 0 replies; 13+ messages in thread
From: Patrick DELAUNAY @ 2021-09-21 12:38 UTC (permalink / raw)
To: Marek Vasut, u-boot
Cc: Priyanka Jain, Heiko Schocher, Jagan Teki, Miquel Raynal,
Christophe KERELLO, Patrice Chotard, Pali Rohár,
U-Boot STM32
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
^ permalink raw reply [flat|nested] 13+ messages in thread
end of thread, other threads:[~2021-09-21 12:38 UTC | newest]
Thread overview: 13+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2021-09-16 14:01 [PATCH v3 0/2] mtd: spi: nor: force mtd name to "nor%d" Patrick Delaunay
2021-09-16 14:01 ` [PATCH v3 1/2] mtd: cfi_flash: use cfi_flash_num_flash_banks only when supported Patrick Delaunay
2021-09-16 17:24 ` Marek Vasut
2021-09-17 10:55 ` Patrick DELAUNAY
2021-09-17 13:36 ` Marek Vasut
2021-09-21 12:38 ` Patrick DELAUNAY
2021-09-16 14:01 ` [PATCH v3 2/2] mtd: spi: nor: force mtd name to "nor%d" Patrick Delaunay
2021-09-16 14:49 ` Marek Behún
2021-09-16 15:00 ` Marek Behún
2021-09-16 17:27 ` Marek Vasut
2021-09-17 13:06 ` Patrick DELAUNAY
2021-09-17 13:39 ` Marek Vasut
2021-09-16 17:43 ` [PATCH v3 0/2] " Marek Vasut
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.