[PATCH u-boot-dm + u-boot-spi v2 0/7] Support SPI NORs and OF partitions in `mtd list`

[PATCH u-boot-dm + u-boot-spi v2 0/7] Support SPI NORs and OF partitions in `mtd list`

[Marek Behún]

Hello,

this is v2 of patchset that adds support for U-Boot to parse MTD
partitions from device-tree, and also improves support for SPI NOR
access via the `mtd` command.

Since mtd subsystem is unmaintained, who shall apply the mtd patches?
I put an u-boot-spi tag into the subject prefix, but am not sure if
this is correct.

Changes since v1:
- added tests of ofnode_get_addr_size_index() and
  ofnode_get_addr_size_index_notrans() as requested by Simon
- the last patch now probes SPI NORs in both versions of
  mtd_probe_devices(), that is when MTDPARTS is enabled or disabled

Marek

Marek Beh?n (7):
  dm: core: add test for ofnode_get_addr_size_index()
  dm: core: add non-translating version of ofnode_get_addr_size_index()
  mtd: add support for parsing partitions defined in OF
  mtd: spi-nor: allow registering multiple MTDs when DM is enabled
  mtd: spi-nor: fill-in mtd->dev member
  mtd: remove mtd_probe function
  mtd: probe SPI NOR devices in mtd_probe_devices()

 drivers/core/ofnode.c          |  19 ++++-
 drivers/mtd/mtd-uclass.c       |  15 ----
 drivers/mtd/mtd_uboot.c        | 129 ++++++++++++++++++++-------------
 drivers/mtd/mtdpart.c          |  60 +++++++++++++++
 drivers/mtd/spi/sf_internal.h  |   4 +-
 drivers/mtd/spi/sf_mtd.c       |  19 ++++-
 drivers/mtd/spi/sf_probe.c     |   6 +-
 drivers/mtd/spi/spi-nor-core.c |   1 +
 drivers/mtd/spi/spi-nor-tiny.c |   1 +
 include/dm/ofnode.h            |  17 +++++
 include/linux/mtd/mtd.h        |   9 +++
 include/mtd.h                  |   1 -
 test/dm/ofnode.c               |  29 ++++++++
 13 files changed, 236 insertions(+), 74 deletions(-)

-- 
2.26.2

[PATCH u-boot-dm + u-boot-spi v2 1/7] dm: core: add test for ofnode_get_addr_size_index()

[Marek Behún]

Add test for ofnode_get_addr_size_index(), which will test OF address
translation.

Signed-off-by: Marek Beh?n <marek.behun@nic.cz>
Cc: Simon Glass <sjg@chromium.org>
---
 test/dm/ofnode.c | 23 +++++++++++++++++++++++
 1 file changed, 23 insertions(+)

diff --git a/test/dm/ofnode.c b/test/dm/ofnode.c
index c539134296..0e1eb0d7ea 100644
--- a/test/dm/ofnode.c
+++ b/test/dm/ofnode.c
@@ -261,3 +261,26 @@ static int dm_test_ofnode_is_enabled(struct unit_test_state *uts)
 	return 0;
 }
 DM_TEST(dm_test_ofnode_is_enabled, UT_TESTF_SCAN_PDATA | UT_TESTF_SCAN_FDT);
+
+static int dm_test_ofnode_get_addr_size(struct unit_test_state *uts)
+{
+	fdt_addr_t addr, size;
+	ofnode node;
+
+	node = ofnode_path("/translation-test at 8000/dev at 0,0");
+	ut_assert(ofnode_valid(node));
+
+	addr = ofnode_get_addr_size_index(node, 0, &size);
+	ut_asserteq_64(0x8000, addr);
+	ut_asserteq_64(0x1000, size);
+
+	node = ofnode_path("/translation-test at 8000/dev at 1,100");
+	ut_assert(ofnode_valid(node));
+
+	addr = ofnode_get_addr_size_index(node, 0, &size);
+	ut_asserteq_64(0x9000, addr);
+	ut_asserteq_64(0x1000, size);
+
+	return 0;
+}
+DM_TEST(dm_test_ofnode_get_addr_size, UT_TESTF_SCAN_PDATA | UT_TESTF_SCAN_FDT);
-- 
2.26.2

[PATCH u-boot-dm + u-boot-spi v2 1/7] dm: core: add test for ofnode_get_addr_size_index()

[Simon Glass]

On Tue, 9 Feb 2021 at 07:45, Marek Beh?n <marek.behun@nic.cz> wrote:
>
> Add test for ofnode_get_addr_size_index(), which will test OF address
> translation.
>
> Signed-off-by: Marek Beh?n <marek.behun@nic.cz>
> Cc: Simon Glass <sjg@chromium.org>
> ---
>  test/dm/ofnode.c | 23 +++++++++++++++++++++++
>  1 file changed, 23 insertions(+)

Reviewed-by: Simon Glass <sjg@chromium.org>

[PATCH u-boot-dm + u-boot-spi v2 1/7] dm: core: add test for ofnode_get_addr_size_index()

[Marek Behún]

On Tue, 9 Feb 2021 22:10:27 -0700
Simon Glass <sjg@chromium.org> wrote:

> On Tue, 9 Feb 2021 at 07:45, Marek Beh?n <marek.behun@nic.cz> wrote:
> >
> > Add test for ofnode_get_addr_size_index(), which will test OF
> > address translation.
> >
> > Signed-off-by: Marek Beh?n <marek.behun@nic.cz>
> > Cc: Simon Glass <sjg@chromium.org>
> > ---
> >  test/dm/ofnode.c | 23 +++++++++++++++++++++++
> >  1 file changed, 23 insertions(+)  
> 
> Reviewed-by: Simon Glass <sjg@chromium.org>

Simon, I just sent v3 of this series, which adds one more DM patch
(adding ofnode_get_path() function). Could you please review this patch?

Thank you.

Marek

[PATCH u-boot-dm + u-boot-spi v2 2/7] dm: core: add non-translating version of ofnode_get_addr_size_index()

[Marek Behún]

Add functions ofnode_get_addr_size_index_notrans(), which is a
non-translating version of ofnode_get_addr_size_index().

Some addresses are not meant to be translated, for example those of MTD
fixed-partitions.

Signed-off-by: Marek Beh?n <marek.behun@nic.cz>
Cc: Dario Binacchi <dariobin@libero.it>
Cc: Simon Glass <sjg@chromium.org>
---
 drivers/core/ofnode.c | 19 ++++++++++++++++---
 include/dm/ofnode.h   | 17 +++++++++++++++++
 test/dm/ofnode.c      |  6 ++++++
 3 files changed, 39 insertions(+), 3 deletions(-)

diff --git a/drivers/core/ofnode.c b/drivers/core/ofnode.c
index 7a5f4c0a73..88266e2641 100644
--- a/drivers/core/ofnode.c
+++ b/drivers/core/ofnode.c
@@ -298,7 +298,8 @@ ofnode ofnode_get_by_phandle(uint phandle)
 	return node;
 }
 
-fdt_addr_t ofnode_get_addr_size_index(ofnode node, int index, fdt_size_t *size)
+static fdt_addr_t __ofnode_get_addr_size_index(ofnode node, int index,
+					       fdt_size_t *size, bool translate)
 {
 	int na, ns;
 
@@ -316,7 +317,7 @@ fdt_addr_t ofnode_get_addr_size_index(ofnode node, int index, fdt_size_t *size)
 
 		ns = of_n_size_cells(ofnode_to_np(node));
 
-		if (IS_ENABLED(CONFIG_OF_TRANSLATE) &&
+		if (translate && IS_ENABLED(CONFIG_OF_TRANSLATE) &&
 		    (ns > 0 || gd_size_cells_0())) {
 			return of_translate_address(ofnode_to_np(node), prop_val);
 		} else {
@@ -328,12 +329,24 @@ fdt_addr_t ofnode_get_addr_size_index(ofnode node, int index, fdt_size_t *size)
 		ns = ofnode_read_simple_size_cells(ofnode_get_parent(node));
 		return fdtdec_get_addr_size_fixed(gd->fdt_blob,
 						  ofnode_to_offset(node), "reg",
-						  index, na, ns, size, true);
+						  index, na, ns, size,
+						  translate);
 	}
 
 	return FDT_ADDR_T_NONE;
 }
 
+fdt_addr_t ofnode_get_addr_size_index(ofnode node, int index, fdt_size_t *size)
+{
+	return __ofnode_get_addr_size_index(node, index, size, true);
+}
+
+fdt_addr_t ofnode_get_addr_size_index_notrans(ofnode node, int index,
+					      fdt_size_t *size)
+{
+	return __ofnode_get_addr_size_index(node, index, size, false);
+}
+
 fdt_addr_t ofnode_get_addr_index(ofnode node, int index)
 {
 	fdt_size_t size;
diff --git a/include/dm/ofnode.h b/include/dm/ofnode.h
index 5b088650d3..6ab6fc5586 100644
--- a/include/dm/ofnode.h
+++ b/include/dm/ofnode.h
@@ -487,6 +487,23 @@ int ofnode_read_size(ofnode node, const char *propname);
 phys_addr_t ofnode_get_addr_size_index(ofnode node, int index,
 				       fdt_size_t *size);
 
+/**
+ * ofnode_get_addr_size_index_notrans() - get an address/size from a node
+ *					  based on index, without address
+ *					  translation
+ *
+ * This reads the register address/size from a node based on index.
+ * The resulting address is not translated. Useful for example for on-disk
+ * addresses.
+ *
+ * @node: node to read from
+ * @index: Index of address to read (0 for first)
+ * @size: Pointer to size of the address
+ * @return address, or FDT_ADDR_T_NONE if not present or invalid
+ */
+phys_addr_t ofnode_get_addr_size_index_notrans(ofnode node, int index,
+					       fdt_size_t *size);
+
 /**
  * ofnode_get_addr_index() - get an address from a node
  *
diff --git a/test/dm/ofnode.c b/test/dm/ofnode.c
index 0e1eb0d7ea..48c121df25 100644
--- a/test/dm/ofnode.c
+++ b/test/dm/ofnode.c
@@ -281,6 +281,12 @@ static int dm_test_ofnode_get_addr_size(struct unit_test_state *uts)
 	ut_asserteq_64(0x9000, addr);
 	ut_asserteq_64(0x1000, size);
 
+	node = ofnode_path("/translation-test at 8000/noxlatebus at 3,300/dev at 42");
+	ut_assert(ofnode_valid(node));
+
+	addr = ofnode_get_addr_size_index_notrans(node, 0, &size);
+	ut_asserteq_64(0x42, addr);
+
 	return 0;
 }
 DM_TEST(dm_test_ofnode_get_addr_size, UT_TESTF_SCAN_PDATA | UT_TESTF_SCAN_FDT);
-- 
2.26.2

[PATCH u-boot-dm + u-boot-spi v2 2/7] dm: core: add non-translating version of ofnode_get_addr_size_index()

[Simon Glass]

On Tue, 9 Feb 2021 at 07:45, Marek Beh?n <marek.behun@nic.cz> wrote:
>
> Add functions ofnode_get_addr_size_index_notrans(), which is a
> non-translating version of ofnode_get_addr_size_index().
>
> Some addresses are not meant to be translated, for example those of MTD
> fixed-partitions.
>
> Signed-off-by: Marek Beh?n <marek.behun@nic.cz>
> Cc: Dario Binacchi <dariobin@libero.it>
> Cc: Simon Glass <sjg@chromium.org>
> ---
>  drivers/core/ofnode.c | 19 ++++++++++++++++---
>  include/dm/ofnode.h   | 17 +++++++++++++++++
>  test/dm/ofnode.c      |  6 ++++++
>  3 files changed, 39 insertions(+), 3 deletions(-)

Reviewed-by: Simon Glass <sjg@chromium.org>

[PATCH u-boot-dm + u-boot-spi v2 3/7] mtd: add support for parsing partitions defined in OF

[Marek Behún]

Add support for parsing partitions defined in device-trees via the
`partitions` node with `fixed-partitions` compatible.

The `mtdparts`/`mtdids` mechanism takes precedence. If some partitions
are defined for a MTD device via this mechanism, the code won't register
partitions for that MTD device from OF, even if they are defined.

Signed-off-by: Marek Beh?n <marek.behun@nic.cz>
Cc: Simon Glass <sjg@chromium.org>
Cc: Heiko Schocher <hs@denx.de>
Cc: Jagan Teki <jagan@amarulasolutions.com>
---
 drivers/mtd/mtd_uboot.c | 106 +++++++++++++++++++++++-----------------
 drivers/mtd/mtdpart.c   |  60 +++++++++++++++++++++++
 include/linux/mtd/mtd.h |   9 ++++
 3 files changed, 131 insertions(+), 44 deletions(-)

diff --git a/drivers/mtd/mtd_uboot.c b/drivers/mtd/mtd_uboot.c
index 9360d4ed17..7fb72eb1f4 100644
--- a/drivers/mtd/mtd_uboot.c
+++ b/drivers/mtd/mtd_uboot.c
@@ -197,53 +197,11 @@ static void mtd_del_all_parts(void)
 	} while (ret > 0);
 }
 
-int mtd_probe_devices(void)
+static int parse_mtdparts(const char *mtdparts, const char *mtdids)
 {
-	static char *old_mtdparts;
-	static char *old_mtdids;
-	const char *mtdparts = get_mtdparts();
-	const char *mtdids = get_mtdids();
-	const char *mtdparts_next = mtdparts;
+	const char *mtdparts_next;
 	struct mtd_info *mtd;
 
-	mtd_probe_uclass_mtd_devs();
-
-	/*
-	 * Check if mtdparts/mtdids changed, if the MTD dev list was updated
-	 * or if our previous attempt to delete existing partititions failed.
-	 * In any of these cases we want to update the partitions, otherwise,
-	 * everything is up-to-date and we can return 0 directly.
-	 */
-	if ((!mtdparts && !old_mtdparts && !mtdids && !old_mtdids) ||
-	    (mtdparts && old_mtdparts && mtdids && old_mtdids &&
-	     !mtd_dev_list_updated() && !mtd_del_all_parts_failed &&
-	     !strcmp(mtdparts, old_mtdparts) &&
-	     !strcmp(mtdids, old_mtdids)))
-		return 0;
-
-	/* Update the local copy of mtdparts */
-	free(old_mtdparts);
-	free(old_mtdids);
-	old_mtdparts = strdup(mtdparts);
-	old_mtdids = strdup(mtdids);
-
-	/*
-	 * Remove all old parts. Note that partition removal can fail in case
-	 * one of the partition is still being used by an MTD user, so this
-	 * does not guarantee that all old partitions are gone.
-	 */
-	mtd_del_all_parts();
-
-	/*
-	 * Call mtd_dev_list_updated() to clear updates generated by our own
-	 * parts removal loop.
-	 */
-	mtd_dev_list_updated();
-
-	/* If either mtdparts or mtdids is empty, then exit */
-	if (!mtdparts || !mtdids)
-		return 0;
-
 	/* Start the parsing by ignoring the extra 'mtdparts=' prefix, if any */
 	if (!strncmp(mtdparts, "mtdparts=", sizeof("mtdparts=") - 1))
 		mtdparts += 9;
@@ -342,6 +300,66 @@ int mtd_probe_devices(void)
 		put_mtd_device(mtd);
 	}
 
+	return 0;
+}
+
+int mtd_probe_devices(void)
+{
+	static char *old_mtdparts;
+	static char *old_mtdids;
+	const char *mtdparts = get_mtdparts();
+	const char *mtdids = get_mtdids();
+	struct mtd_info *mtd;
+
+	mtd_probe_uclass_mtd_devs();
+
+	/*
+	 * Check if mtdparts/mtdids changed, if the MTD dev list was updated
+	 * or if our previous attempt to delete existing partititions failed.
+	 * In any of these cases we want to update the partitions, otherwise,
+	 * everything is up-to-date and we can return 0 directly.
+	 */
+	if ((!mtdparts && !old_mtdparts && !mtdids && !old_mtdids) ||
+	    (mtdparts && old_mtdparts && mtdids && old_mtdids &&
+	     !mtd_dev_list_updated() && !mtd_del_all_parts_failed &&
+	     !strcmp(mtdparts, old_mtdparts) &&
+	     !strcmp(mtdids, old_mtdids)))
+		return 0;
+
+	/* Update the local copy of mtdparts */
+	free(old_mtdparts);
+	free(old_mtdids);
+	old_mtdparts = strdup(mtdparts);
+	old_mtdids = strdup(mtdids);
+
+	/*
+	 * Remove all old parts. Note that partition removal can fail in case
+	 * one of the partition is still being used by an MTD user, so this
+	 * does not guarantee that all old partitions are gone.
+	 */
+	mtd_del_all_parts();
+
+	/*
+	 * Call mtd_dev_list_updated() to clear updates generated by our own
+	 * parts removal loop.
+	 */
+	mtd_dev_list_updated();
+
+	/* If both mtdparts and mtdids are non-empty, parse */
+	if (mtdparts && mtdids) {
+		if (parse_mtdparts(mtdparts, mtdids) < 0)
+			printf("Failed parsing MTD partitions from mtdparts!\n");
+	}
+
+	/* Fallback to OF partitions */
+	mtd_for_each_device(mtd) {
+		if (list_empty(&mtd->partitions)) {
+			if (add_mtd_partitions_of(mtd) < 0)
+				printf("Failed parsing MTD %s OF partitions!\n",
+					mtd->name);
+		}
+	}
+
 	/*
 	 * Call mtd_dev_list_updated() to clear updates generated by our own
 	 * parts registration loop.
diff --git a/drivers/mtd/mtdpart.c b/drivers/mtd/mtdpart.c
index d064ac3048..2ebbf74d92 100644
--- a/drivers/mtd/mtdpart.c
+++ b/drivers/mtd/mtdpart.c
@@ -892,6 +892,66 @@ int add_mtd_partitions(struct mtd_info *master,
 	return 0;
 }
 
+#ifdef CONFIG_DM
+int add_mtd_partitions_of(struct mtd_info *master)
+{
+	ofnode parts, child;
+	int i = 0;
+
+	if (!master->dev)
+		return 0;
+
+	parts = ofnode_find_subnode(mtd_get_ofnode(master), "partitions");
+	if (!ofnode_valid(parts) || !ofnode_is_available(parts) ||
+	    !ofnode_device_is_compatible(parts, "fixed-partitions"))
+		return 0;
+
+	ofnode_for_each_subnode(child, parts) {
+		struct mtd_partition part = { 0 };
+		struct mtd_info *slave;
+		fdt_addr_t offset, size;
+
+		if (!ofnode_is_available(child))
+			continue;
+
+		if (ofnode_get_property(child, "compatible", NULL))
+			continue;
+
+		offset = ofnode_get_addr_size_index_notrans(child, 0, &size);
+		if (offset == FDT_ADDR_T_NONE || !size) {
+			debug("Missing partition offset/size on \"%s\" partition\n",
+			      master->name);
+			continue;
+		}
+
+		part.name = ofnode_read_string(child, "label");
+		if (!part.name)
+			part.name = ofnode_read_string(child, "name");
+
+		if (ofnode_read_bool(child, "read-only"))
+			part.mask_flags |= MTD_WRITEABLE;
+		if (ofnode_read_bool(child, "lock"))
+			part.mask_flags |= MTD_POWERUP_LOCK;
+
+		part.offset = offset;
+		part.size = size;
+		part.ecclayout = master->ecclayout;
+
+		slave = allocate_partition(master, &part, i++, 0);
+		if (IS_ERR(slave))
+			return PTR_ERR(slave);
+
+		mutex_lock(&mtd_partitions_mutex);
+		list_add_tail(&slave->node, &master->partitions);
+		mutex_unlock(&mtd_partitions_mutex);
+
+		add_mtd_device(slave);
+	}
+
+	return 0;
+}
+#endif
+
 #ifndef __UBOOT__
 static DEFINE_SPINLOCK(part_parser_lock);
 static LIST_HEAD(part_parsers);
diff --git a/include/linux/mtd/mtd.h b/include/linux/mtd/mtd.h
index 927854950a..ec9331b804 100644
--- a/include/linux/mtd/mtd.h
+++ b/include/linux/mtd/mtd.h
@@ -581,6 +581,15 @@ static inline int del_mtd_partitions(struct mtd_info *mtd)
 }
 #endif
 
+#if defined(CONFIG_MTD_PARTITIONS) && defined(CONFIG_DM)
+int add_mtd_partitions_of(struct mtd_info *master);
+#else
+static inline int add_mtd_partitions_of(struct mtd_info *master)
+{
+	return 0;
+}
+#endif
+
 struct mtd_info *__mtd_next_device(int i);
 #define mtd_for_each_device(mtd)			\
 	for ((mtd) = __mtd_next_device(0);		\
-- 
2.26.2

[PATCH u-boot-dm + u-boot-spi v2 3/7] mtd: add support for parsing partitions defined in OF

[Pali Rohár]

On Tuesday 09 February 2021 15:44:48 Marek Beh?n wrote:
> Add support for parsing partitions defined in device-trees via the
> `partitions` node with `fixed-partitions` compatible.
> 
> The `mtdparts`/`mtdids` mechanism takes precedence. If some partitions
> are defined for a MTD device via this mechanism, the code won't register
> partitions for that MTD device from OF, even if they are defined.
> 
> Signed-off-by: Marek Beh?n <marek.behun@nic.cz>
> Cc: Simon Glass <sjg@chromium.org>
> Cc: Heiko Schocher <hs@denx.de>
> Cc: Jagan Teki <jagan@amarulasolutions.com>
> ---
>  drivers/mtd/mtd_uboot.c | 106 +++++++++++++++++++++++-----------------
>  drivers/mtd/mtdpart.c   |  60 +++++++++++++++++++++++
>  include/linux/mtd/mtd.h |   9 ++++
>  3 files changed, 131 insertions(+), 44 deletions(-)
> 
> diff --git a/drivers/mtd/mtd_uboot.c b/drivers/mtd/mtd_uboot.c
> index 9360d4ed17..7fb72eb1f4 100644
> --- a/drivers/mtd/mtd_uboot.c
> +++ b/drivers/mtd/mtd_uboot.c
> @@ -197,53 +197,11 @@ static void mtd_del_all_parts(void)
>  	} while (ret > 0);
>  }
>  
> -int mtd_probe_devices(void)
> +static int parse_mtdparts(const char *mtdparts, const char *mtdids)
>  {
> -	static char *old_mtdparts;
> -	static char *old_mtdids;
> -	const char *mtdparts = get_mtdparts();
> -	const char *mtdids = get_mtdids();
> -	const char *mtdparts_next = mtdparts;
> +	const char *mtdparts_next;
>  	struct mtd_info *mtd;
>  
> -	mtd_probe_uclass_mtd_devs();
> -
> -	/*
> -	 * Check if mtdparts/mtdids changed, if the MTD dev list was updated
> -	 * or if our previous attempt to delete existing partititions failed.
> -	 * In any of these cases we want to update the partitions, otherwise,
> -	 * everything is up-to-date and we can return 0 directly.
> -	 */
> -	if ((!mtdparts && !old_mtdparts && !mtdids && !old_mtdids) ||
> -	    (mtdparts && old_mtdparts && mtdids && old_mtdids &&
> -	     !mtd_dev_list_updated() && !mtd_del_all_parts_failed &&
> -	     !strcmp(mtdparts, old_mtdparts) &&
> -	     !strcmp(mtdids, old_mtdids)))
> -		return 0;
> -
> -	/* Update the local copy of mtdparts */
> -	free(old_mtdparts);
> -	free(old_mtdids);
> -	old_mtdparts = strdup(mtdparts);
> -	old_mtdids = strdup(mtdids);
> -
> -	/*
> -	 * Remove all old parts. Note that partition removal can fail in case
> -	 * one of the partition is still being used by an MTD user, so this
> -	 * does not guarantee that all old partitions are gone.
> -	 */
> -	mtd_del_all_parts();
> -
> -	/*
> -	 * Call mtd_dev_list_updated() to clear updates generated by our own
> -	 * parts removal loop.
> -	 */
> -	mtd_dev_list_updated();
> -
> -	/* If either mtdparts or mtdids is empty, then exit */
> -	if (!mtdparts || !mtdids)
> -		return 0;
> -
>  	/* Start the parsing by ignoring the extra 'mtdparts=' prefix, if any */
>  	if (!strncmp(mtdparts, "mtdparts=", sizeof("mtdparts=") - 1))
>  		mtdparts += 9;
> @@ -342,6 +300,66 @@ int mtd_probe_devices(void)
>  		put_mtd_device(mtd);
>  	}
>  
> +	return 0;
> +}
> +
> +int mtd_probe_devices(void)
> +{
> +	static char *old_mtdparts;
> +	static char *old_mtdids;
> +	const char *mtdparts = get_mtdparts();
> +	const char *mtdids = get_mtdids();
> +	struct mtd_info *mtd;
> +
> +	mtd_probe_uclass_mtd_devs();
> +
> +	/*
> +	 * Check if mtdparts/mtdids changed, if the MTD dev list was updated
> +	 * or if our previous attempt to delete existing partititions failed.
> +	 * In any of these cases we want to update the partitions, otherwise,
> +	 * everything is up-to-date and we can return 0 directly.
> +	 */
> +	if ((!mtdparts && !old_mtdparts && !mtdids && !old_mtdids) ||
> +	    (mtdparts && old_mtdparts && mtdids && old_mtdids &&
> +	     !mtd_dev_list_updated() && !mtd_del_all_parts_failed &&
> +	     !strcmp(mtdparts, old_mtdparts) &&
> +	     !strcmp(mtdids, old_mtdids)))
> +		return 0;
> +
> +	/* Update the local copy of mtdparts */
> +	free(old_mtdparts);
> +	free(old_mtdids);
> +	old_mtdparts = strdup(mtdparts);
> +	old_mtdids = strdup(mtdids);
> +
> +	/*
> +	 * Remove all old parts. Note that partition removal can fail in case
> +	 * one of the partition is still being used by an MTD user, so this
> +	 * does not guarantee that all old partitions are gone.
> +	 */
> +	mtd_del_all_parts();
> +
> +	/*
> +	 * Call mtd_dev_list_updated() to clear updates generated by our own
> +	 * parts removal loop.
> +	 */
> +	mtd_dev_list_updated();
> +
> +	/* If both mtdparts and mtdids are non-empty, parse */
> +	if (mtdparts && mtdids) {
> +		if (parse_mtdparts(mtdparts, mtdids) < 0)
> +			printf("Failed parsing MTD partitions from mtdparts!\n");
> +	}
> +
> +	/* Fallback to OF partitions */
> +	mtd_for_each_device(mtd) {
> +		if (list_empty(&mtd->partitions)) {
> +			if (add_mtd_partitions_of(mtd) < 0)
> +				printf("Failed parsing MTD %s OF partitions!\n",
> +					mtd->name);
> +		}
> +	}
> +
>  	/*
>  	 * Call mtd_dev_list_updated() to clear updates generated by our own
>  	 * parts registration loop.
> diff --git a/drivers/mtd/mtdpart.c b/drivers/mtd/mtdpart.c
> index d064ac3048..2ebbf74d92 100644
> --- a/drivers/mtd/mtdpart.c
> +++ b/drivers/mtd/mtdpart.c
> @@ -892,6 +892,66 @@ int add_mtd_partitions(struct mtd_info *master,
>  	return 0;
>  }
>  
> +#ifdef CONFIG_DM
> +int add_mtd_partitions_of(struct mtd_info *master)
> +{
> +	ofnode parts, child;
> +	int i = 0;
> +
> +	if (!master->dev)
> +		return 0;
> +
> +	parts = ofnode_find_subnode(mtd_get_ofnode(master), "partitions");
> +	if (!ofnode_valid(parts) || !ofnode_is_available(parts) ||
> +	    !ofnode_device_is_compatible(parts, "fixed-partitions"))
> +		return 0;
> +
> +	ofnode_for_each_subnode(child, parts) {
> +		struct mtd_partition part = { 0 };
> +		struct mtd_info *slave;
> +		fdt_addr_t offset, size;
> +
> +		if (!ofnode_is_available(child))
> +			continue;
> +
> +		if (ofnode_get_property(child, "compatible", NULL))
> +			continue;

I think this check is incorrect. Partition node may have another
"compatible" property with another node. Linux kernel supports defining
partitions on partitions. E.g.

    partitions {
        compatible = "fixed-partitions";
        #address-cells = <1>;
        #size-cells = <1>;

        partition at 0 {
            label = "bootloader";
            reg = <0x000000 0x100000>;
            read-only;
        };

        firmware at 100000 {
            compatible = "brcm,trx";
            label = "firmware";
            reg = <0x100000 0xe00000>;
        };

        calibration at f00000 {
            compatible = "fixed-partitions";
            label = "calibration";
            reg = <0xf00000 0x100000>;
            ranges = <0 0xf00000 0x100000>;
            #address-cells = <1>;
            #size-cells = <1>;

            partition at 0 {
                label = "wifi0";
                reg = <0x000000 0x080000>;
            };

            partition at 80000 {
                label = "wifi1";
                reg = <0x080000 0x080000>;
            };
        };
    };

See Documentation/devicetree/bindings/mtd/partitions/fixed-partitions.yaml

> +
> +		offset = ofnode_get_addr_size_index_notrans(child, 0, &size);
> +		if (offset == FDT_ADDR_T_NONE || !size) {
> +			debug("Missing partition offset/size on \"%s\" partition\n",
> +			      master->name);
> +			continue;
> +		}
> +
> +		part.name = ofnode_read_string(child, "label");
> +		if (!part.name)
> +			part.name = ofnode_read_string(child, "name");
> +
> +		if (ofnode_read_bool(child, "read-only"))
> +			part.mask_flags |= MTD_WRITEABLE;
> +		if (ofnode_read_bool(child, "lock"))
> +			part.mask_flags |= MTD_POWERUP_LOCK;

I would suggest to add some explanation why setting these two mask flags
according to those DT properties is correct as it is not obvious.

mask_flags contains flags which have to be removed from partition.
DT "read-only" means that partition is read-only, so removing writable
is correct. DR "lock" means to *not* unlock the partition at
initialization time and MTD_POWERUP_LOCK means that partition is always
locked after reset and needs to be (always) unlocked. So masking this
flag means that it is never unlocked.

> +
> +		part.offset = offset;
> +		part.size = size;
> +		part.ecclayout = master->ecclayout;
> +
> +		slave = allocate_partition(master, &part, i++, 0);
> +		if (IS_ERR(slave))
> +			return PTR_ERR(slave);
> +
> +		mutex_lock(&mtd_partitions_mutex);
> +		list_add_tail(&slave->node, &master->partitions);
> +		mutex_unlock(&mtd_partitions_mutex);
> +
> +		add_mtd_device(slave);
> +	}
> +
> +	return 0;
> +}
> +#endif
> +
>  #ifndef __UBOOT__
>  static DEFINE_SPINLOCK(part_parser_lock);
>  static LIST_HEAD(part_parsers);
> diff --git a/include/linux/mtd/mtd.h b/include/linux/mtd/mtd.h
> index 927854950a..ec9331b804 100644
> --- a/include/linux/mtd/mtd.h
> +++ b/include/linux/mtd/mtd.h
> @@ -581,6 +581,15 @@ static inline int del_mtd_partitions(struct mtd_info *mtd)
>  }
>  #endif
>  
> +#if defined(CONFIG_MTD_PARTITIONS) && defined(CONFIG_DM)
> +int add_mtd_partitions_of(struct mtd_info *master);
> +#else
> +static inline int add_mtd_partitions_of(struct mtd_info *master)
> +{
> +	return 0;
> +}
> +#endif
> +
>  struct mtd_info *__mtd_next_device(int i);
>  #define mtd_for_each_device(mtd)			\
>  	for ((mtd) = __mtd_next_device(0);		\
> -- 
> 2.26.2
> 

[PATCH u-boot-dm + u-boot-spi v2 4/7] mtd: spi-nor: allow registering multiple MTDs when DM is enabled

[Marek Behún]

Currently when the SPI_FLASH_MTD config option is enabled, only one SPI
can be registered as MTD at any time - it is the last one probed (since
with old non-DM model only one SPI NOR could be probed at any time).

When DM is enabled, allow for registering multiple SPI NORs as MTDs by
utilizing the nor->mtd structure, which is filled in by spi_nor_scan
anyway, instead of filling a separate struct mtd_info.

Signed-off-by: Marek Beh?n <marek.behun@nic.cz>
Cc: Jagan Teki <jagan@amarulasolutions.com>
Cc: Priyanka Jain <priyanka.jain@nxp.com>
Cc: Simon Glass <sjg@chromium.org>
Cc: Heiko Schocher <hs@denx.de>
Cc: Jagan Teki <jagan@amarulasolutions.com>
---
 drivers/mtd/spi/sf_internal.h |  4 ++--
 drivers/mtd/spi/sf_mtd.c      | 18 +++++++++++++++++-
 drivers/mtd/spi/sf_probe.c    |  6 ++++--
 3 files changed, 23 insertions(+), 5 deletions(-)

diff --git a/drivers/mtd/spi/sf_internal.h b/drivers/mtd/spi/sf_internal.h
index 9ceff0e7c1..865955124c 100644
--- a/drivers/mtd/spi/sf_internal.h
+++ b/drivers/mtd/spi/sf_internal.h
@@ -77,14 +77,14 @@ extern const struct flash_info spi_nor_ids[];
 
 #if CONFIG_IS_ENABLED(SPI_FLASH_MTD)
 int spi_flash_mtd_register(struct spi_flash *flash);
-void spi_flash_mtd_unregister(void);
+void spi_flash_mtd_unregister(struct spi_flash *flash);
 #else
 static inline int spi_flash_mtd_register(struct spi_flash *flash)
 {
 	return 0;
 }
 
-static inline void spi_flash_mtd_unregister(void)
+static inline void spi_flash_mtd_unregister(struct spi_flash *flash)
 {
 }
 #endif
diff --git a/drivers/mtd/spi/sf_mtd.c b/drivers/mtd/spi/sf_mtd.c
index 987fac2501..94854fbfc4 100644
--- a/drivers/mtd/spi/sf_mtd.c
+++ b/drivers/mtd/spi/sf_mtd.c
@@ -10,6 +10,20 @@
 #include <linux/mtd/mtd.h>
 #include <spi_flash.h>
 
+#if CONFIG_IS_ENABLED(DM_SPI_FLASH)
+
+int spi_flash_mtd_register(struct spi_flash *flash)
+{
+	return add_mtd_device(&flash->mtd);
+}
+
+void spi_flash_mtd_unregister(struct spi_flash *flash)
+{
+	del_mtd_device(&flash->mtd);
+}
+
+#else /* !CONFIG_IS_ENABLED(DM_SPI_FLASH) */
+
 static struct mtd_info sf_mtd_info;
 static bool sf_mtd_registered;
 static char sf_mtd_name[8];
@@ -123,7 +137,7 @@ int spi_flash_mtd_register(struct spi_flash *flash)
 	return ret;
 }
 
-void spi_flash_mtd_unregister(void)
+void spi_flash_mtd_unregister(struct spi_flash *flash)
 {
 	int ret;
 
@@ -146,3 +160,5 @@ void spi_flash_mtd_unregister(void)
 	printf("Failed to unregister MTD %s and the spi_flash object is going away: you're in deep trouble!",
 	       sf_mtd_info.name);
 }
+
+#endif /* !CONFIG_IS_ENABLED(DM_SPI_FLASH) */
diff --git a/drivers/mtd/spi/sf_probe.c b/drivers/mtd/spi/sf_probe.c
index 6c87434867..3bf2ecd51a 100644
--- a/drivers/mtd/spi/sf_probe.c
+++ b/drivers/mtd/spi/sf_probe.c
@@ -84,7 +84,7 @@ struct spi_flash *spi_flash_probe(unsigned int busnum, unsigned int cs,
 void spi_flash_free(struct spi_flash *flash)
 {
 	if (CONFIG_IS_ENABLED(SPI_FLASH_MTD))
-		spi_flash_mtd_unregister();
+		spi_flash_mtd_unregister(flash);
 
 	spi_free_slave(flash->spi);
 	free(flash);
@@ -143,8 +143,10 @@ int spi_flash_std_probe(struct udevice *dev)
 
 static int spi_flash_std_remove(struct udevice *dev)
 {
+	struct spi_flash *flash = dev_get_uclass_priv(dev);
+
 	if (CONFIG_IS_ENABLED(SPI_FLASH_MTD))
-		spi_flash_mtd_unregister();
+		spi_flash_mtd_unregister(flash);
 
 	return 0;
 }
-- 
2.26.2

[PATCH u-boot-dm + u-boot-spi v2 4/7] mtd: spi-nor: allow registering multiple MTDs when DM is enabled

[Pali Rohár]

On Tuesday 09 February 2021 15:44:49 Marek Beh?n wrote:
> Currently when the SPI_FLASH_MTD config option is enabled, only one SPI
> can be registered as MTD at any time - it is the last one probed (since
> with old non-DM model only one SPI NOR could be probed at any time).
> 
> When DM is enabled, allow for registering multiple SPI NORs as MTDs by
> utilizing the nor->mtd structure, which is filled in by spi_nor_scan
> anyway, instead of filling a separate struct mtd_info.
> 
> Signed-off-by: Marek Beh?n <marek.behun@nic.cz>
> Cc: Jagan Teki <jagan@amarulasolutions.com>
> Cc: Priyanka Jain <priyanka.jain@nxp.com>
> Cc: Simon Glass <sjg@chromium.org>
> Cc: Heiko Schocher <hs@denx.de>
> Cc: Jagan Teki <jagan@amarulasolutions.com>

Reviewed-by: Pali Roh?r <pali@kernel.org>

> ---
>  drivers/mtd/spi/sf_internal.h |  4 ++--
>  drivers/mtd/spi/sf_mtd.c      | 18 +++++++++++++++++-
>  drivers/mtd/spi/sf_probe.c    |  6 ++++--
>  3 files changed, 23 insertions(+), 5 deletions(-)
> 
> diff --git a/drivers/mtd/spi/sf_internal.h b/drivers/mtd/spi/sf_internal.h
> index 9ceff0e7c1..865955124c 100644
> --- a/drivers/mtd/spi/sf_internal.h
> +++ b/drivers/mtd/spi/sf_internal.h
> @@ -77,14 +77,14 @@ extern const struct flash_info spi_nor_ids[];
>  
>  #if CONFIG_IS_ENABLED(SPI_FLASH_MTD)
>  int spi_flash_mtd_register(struct spi_flash *flash);
> -void spi_flash_mtd_unregister(void);
> +void spi_flash_mtd_unregister(struct spi_flash *flash);
>  #else
>  static inline int spi_flash_mtd_register(struct spi_flash *flash)
>  {
>  	return 0;
>  }
>  
> -static inline void spi_flash_mtd_unregister(void)
> +static inline void spi_flash_mtd_unregister(struct spi_flash *flash)
>  {
>  }
>  #endif
> diff --git a/drivers/mtd/spi/sf_mtd.c b/drivers/mtd/spi/sf_mtd.c
> index 987fac2501..94854fbfc4 100644
> --- a/drivers/mtd/spi/sf_mtd.c
> +++ b/drivers/mtd/spi/sf_mtd.c
> @@ -10,6 +10,20 @@
>  #include <linux/mtd/mtd.h>
>  #include <spi_flash.h>
>  
> +#if CONFIG_IS_ENABLED(DM_SPI_FLASH)
> +
> +int spi_flash_mtd_register(struct spi_flash *flash)
> +{
> +	return add_mtd_device(&flash->mtd);
> +}
> +
> +void spi_flash_mtd_unregister(struct spi_flash *flash)
> +{
> +	del_mtd_device(&flash->mtd);
> +}
> +
> +#else /* !CONFIG_IS_ENABLED(DM_SPI_FLASH) */
> +
>  static struct mtd_info sf_mtd_info;
>  static bool sf_mtd_registered;
>  static char sf_mtd_name[8];
> @@ -123,7 +137,7 @@ int spi_flash_mtd_register(struct spi_flash *flash)
>  	return ret;
>  }
>  
> -void spi_flash_mtd_unregister(void)
> +void spi_flash_mtd_unregister(struct spi_flash *flash)
>  {
>  	int ret;
>  
> @@ -146,3 +160,5 @@ void spi_flash_mtd_unregister(void)
>  	printf("Failed to unregister MTD %s and the spi_flash object is going away: you're in deep trouble!",
>  	       sf_mtd_info.name);
>  }
> +
> +#endif /* !CONFIG_IS_ENABLED(DM_SPI_FLASH) */
> diff --git a/drivers/mtd/spi/sf_probe.c b/drivers/mtd/spi/sf_probe.c
> index 6c87434867..3bf2ecd51a 100644
> --- a/drivers/mtd/spi/sf_probe.c
> +++ b/drivers/mtd/spi/sf_probe.c
> @@ -84,7 +84,7 @@ struct spi_flash *spi_flash_probe(unsigned int busnum, unsigned int cs,
>  void spi_flash_free(struct spi_flash *flash)
>  {
>  	if (CONFIG_IS_ENABLED(SPI_FLASH_MTD))
> -		spi_flash_mtd_unregister();
> +		spi_flash_mtd_unregister(flash);
>  
>  	spi_free_slave(flash->spi);
>  	free(flash);
> @@ -143,8 +143,10 @@ int spi_flash_std_probe(struct udevice *dev)
>  
>  static int spi_flash_std_remove(struct udevice *dev)
>  {
> +	struct spi_flash *flash = dev_get_uclass_priv(dev);
> +
>  	if (CONFIG_IS_ENABLED(SPI_FLASH_MTD))
> -		spi_flash_mtd_unregister();
> +		spi_flash_mtd_unregister(flash);
>  
>  	return 0;
>  }
> -- 
> 2.26.2
> 

[PATCH u-boot-dm + u-boot-spi v2 5/7] mtd: spi-nor: fill-in mtd->dev member

[Marek Behún]

Fill in mtd->dev member with nor->dev.

This can be used by MTD OF partition parser.

Signed-off-by: Marek Beh?n <marek.behun@nic.cz>
Cc: Jagan Teki <jagan@amarulasolutions.com>
Cc: Priyanka Jain <priyanka.jain@nxp.com>
Cc: Simon Glass <sjg@chromium.org>
Cc: Heiko Schocher <hs@denx.de>
Cc: Jagan Teki <jagan@amarulasolutions.com>
---
 drivers/mtd/spi/sf_mtd.c       | 1 +
 drivers/mtd/spi/spi-nor-core.c | 1 +
 drivers/mtd/spi/spi-nor-tiny.c | 1 +
 3 files changed, 3 insertions(+)

diff --git a/drivers/mtd/spi/sf_mtd.c b/drivers/mtd/spi/sf_mtd.c
index 94854fbfc4..04de868080 100644
--- a/drivers/mtd/spi/sf_mtd.c
+++ b/drivers/mtd/spi/sf_mtd.c
@@ -125,6 +125,7 @@ int spi_flash_mtd_register(struct spi_flash *flash)
 
 	sf_mtd_info.size = flash->size;
 	sf_mtd_info.priv = flash;
+	sf_mtd_info.dev = flash->dev;
 
 	/* Only uniform flash devices for now */
 	sf_mtd_info.numeraseregions = 0;
diff --git a/drivers/mtd/spi/spi-nor-core.c b/drivers/mtd/spi/spi-nor-core.c
index ef426dac02..57b7fa3b37 100644
--- a/drivers/mtd/spi/spi-nor-core.c
+++ b/drivers/mtd/spi/spi-nor-core.c
@@ -2532,6 +2532,7 @@ int spi_nor_scan(struct spi_nor *nor)
 
 	if (!mtd->name)
 		mtd->name = info->name;
+	mtd->dev = nor->dev;
 	mtd->priv = nor;
 	mtd->type = MTD_NORFLASH;
 	mtd->writesize = 1;
diff --git a/drivers/mtd/spi/spi-nor-tiny.c b/drivers/mtd/spi/spi-nor-tiny.c
index 07c8c7b82b..80cc091469 100644
--- a/drivers/mtd/spi/spi-nor-tiny.c
+++ b/drivers/mtd/spi/spi-nor-tiny.c
@@ -751,6 +751,7 @@ int spi_nor_scan(struct spi_nor *nor)
 		return ret;
 
 	mtd->name = "spi-flash";
+	mtd->dev = nor->dev;
 	mtd->priv = nor;
 	mtd->type = MTD_NORFLASH;
 	mtd->writesize = 1;
-- 
2.26.2

[PATCH u-boot-dm + u-boot-spi v2 5/7] mtd: spi-nor: fill-in mtd->dev member

[Pali Rohár]

On Tuesday 09 February 2021 15:44:50 Marek Beh?n wrote:
> Fill in mtd->dev member with nor->dev.
> 
> This can be used by MTD OF partition parser.
> 
> Signed-off-by: Marek Beh?n <marek.behun@nic.cz>
> Cc: Jagan Teki <jagan@amarulasolutions.com>
> Cc: Priyanka Jain <priyanka.jain@nxp.com>
> Cc: Simon Glass <sjg@chromium.org>
> Cc: Heiko Schocher <hs@denx.de>
> Cc: Jagan Teki <jagan@amarulasolutions.com>

Reviewed-by: Pali Roh?r <pali@kernel.org>

> ---
>  drivers/mtd/spi/sf_mtd.c       | 1 +
>  drivers/mtd/spi/spi-nor-core.c | 1 +
>  drivers/mtd/spi/spi-nor-tiny.c | 1 +
>  3 files changed, 3 insertions(+)
> 
> diff --git a/drivers/mtd/spi/sf_mtd.c b/drivers/mtd/spi/sf_mtd.c
> index 94854fbfc4..04de868080 100644
> --- a/drivers/mtd/spi/sf_mtd.c
> +++ b/drivers/mtd/spi/sf_mtd.c
> @@ -125,6 +125,7 @@ int spi_flash_mtd_register(struct spi_flash *flash)
>  
>  	sf_mtd_info.size = flash->size;
>  	sf_mtd_info.priv = flash;
> +	sf_mtd_info.dev = flash->dev;
>  
>  	/* Only uniform flash devices for now */
>  	sf_mtd_info.numeraseregions = 0;
> diff --git a/drivers/mtd/spi/spi-nor-core.c b/drivers/mtd/spi/spi-nor-core.c
> index ef426dac02..57b7fa3b37 100644
> --- a/drivers/mtd/spi/spi-nor-core.c
> +++ b/drivers/mtd/spi/spi-nor-core.c
> @@ -2532,6 +2532,7 @@ int spi_nor_scan(struct spi_nor *nor)
>  
>  	if (!mtd->name)
>  		mtd->name = info->name;
> +	mtd->dev = nor->dev;
>  	mtd->priv = nor;
>  	mtd->type = MTD_NORFLASH;
>  	mtd->writesize = 1;
> diff --git a/drivers/mtd/spi/spi-nor-tiny.c b/drivers/mtd/spi/spi-nor-tiny.c
> index 07c8c7b82b..80cc091469 100644
> --- a/drivers/mtd/spi/spi-nor-tiny.c
> +++ b/drivers/mtd/spi/spi-nor-tiny.c
> @@ -751,6 +751,7 @@ int spi_nor_scan(struct spi_nor *nor)
>  		return ret;
>  
>  	mtd->name = "spi-flash";
> +	mtd->dev = nor->dev;
>  	mtd->priv = nor;
>  	mtd->type = MTD_NORFLASH;
>  	mtd->writesize = 1;
> -- 
> 2.26.2
> 

[PATCH u-boot-dm + u-boot-spi v2 6/7] mtd: remove mtd_probe function

[Marek Behún]

The device_probe() function does the same thing as mtd_probe() and
mtd_probe() is only used in mtd_probe_uclass_mtd_devs(), where the
probing can be made simpler by using uclass_foreach_dev_probe macro.

Signed-off-by: Marek Beh?n <marek.behun@nic.cz>
Cc: Jagan Teki <jagan@amarulasolutions.com>
Cc: Priyanka Jain <priyanka.jain@nxp.com>
Cc: Simon Glass <sjg@chromium.org>
Cc: Heiko Schocher <hs@denx.de>
Cc: Jagan Teki <jagan@amarulasolutions.com>
---
 drivers/mtd/mtd-uclass.c | 15 ---------------
 drivers/mtd/mtd_uboot.c  |  9 +++------
 include/mtd.h            |  1 -
 3 files changed, 3 insertions(+), 22 deletions(-)

diff --git a/drivers/mtd/mtd-uclass.c b/drivers/mtd/mtd-uclass.c
index 9f5f672ba3..4ab84de553 100644
--- a/drivers/mtd/mtd-uclass.c
+++ b/drivers/mtd/mtd-uclass.c
@@ -9,21 +9,6 @@
 #include <errno.h>
 #include <mtd.h>
 
-/**
- * mtd_probe - Probe the device @dev if not already done
- *
- * @dev: U-Boot device to probe
- *
- * @return 0 on success, an error otherwise.
- */
-int mtd_probe(struct udevice *dev)
-{
-	if (device_active(dev))
-		return 0;
-
-	return device_probe(dev);
-}
-
 /*
  * Implement a MTD uclass which should include most flash drivers.
  * The uclass private is pointed to mtd_info.
diff --git a/drivers/mtd/mtd_uboot.c b/drivers/mtd/mtd_uboot.c
index 7fb72eb1f4..865e9ae4cb 100644
--- a/drivers/mtd/mtd_uboot.c
+++ b/drivers/mtd/mtd_uboot.c
@@ -9,6 +9,7 @@
 #include <malloc.h>
 #include <dm/device.h>
 #include <dm/uclass-internal.h>
+#include <dm/uclass.h>
 #include <linux/err.h>
 #include <linux/mtd/mtd.h>
 #include <linux/mtd/partitions.h>
@@ -105,13 +106,9 @@ int mtd_search_alternate_name(const char *mtdname, char *altname,
 static void mtd_probe_uclass_mtd_devs(void)
 {
 	struct udevice *dev;
-	int idx = 0;
 
-	/* Probe devices with DM compliant drivers */
-	while (!uclass_find_device(UCLASS_MTD, idx, &dev) && dev) {
-		mtd_probe(dev);
-		idx++;
-	}
+	uclass_foreach_dev_probe(UCLASS_MTD, dev)
+		;
 }
 #else
 static void mtd_probe_uclass_mtd_devs(void) { }
diff --git a/include/mtd.h b/include/mtd.h
index b0f8693386..b569331edb 100644
--- a/include/mtd.h
+++ b/include/mtd.h
@@ -8,7 +8,6 @@
 
 #include <linux/mtd/mtd.h>
 
-int mtd_probe(struct udevice *dev);
 int mtd_probe_devices(void);
 
 void board_mtdparts_default(const char **mtdids, const char **mtdparts);
-- 
2.26.2

[PATCH u-boot-dm + u-boot-spi v2 6/7] mtd: remove mtd_probe function

[Pali Rohár]

On Tuesday 09 February 2021 15:44:51 Marek Beh?n wrote:
> The device_probe() function does the same thing as mtd_probe() and
> mtd_probe() is only used in mtd_probe_uclass_mtd_devs(), where the
> probing can be made simpler by using uclass_foreach_dev_probe macro.
> 
> Signed-off-by: Marek Beh?n <marek.behun@nic.cz>
> Cc: Jagan Teki <jagan@amarulasolutions.com>
> Cc: Priyanka Jain <priyanka.jain@nxp.com>
> Cc: Simon Glass <sjg@chromium.org>
> Cc: Heiko Schocher <hs@denx.de>
> Cc: Jagan Teki <jagan@amarulasolutions.com>

Reviewed-by: Pali Roh?r <pali@kernel.org>

> ---
>  drivers/mtd/mtd-uclass.c | 15 ---------------
>  drivers/mtd/mtd_uboot.c  |  9 +++------
>  include/mtd.h            |  1 -
>  3 files changed, 3 insertions(+), 22 deletions(-)
> 
> diff --git a/drivers/mtd/mtd-uclass.c b/drivers/mtd/mtd-uclass.c
> index 9f5f672ba3..4ab84de553 100644
> --- a/drivers/mtd/mtd-uclass.c
> +++ b/drivers/mtd/mtd-uclass.c
> @@ -9,21 +9,6 @@
>  #include <errno.h>
>  #include <mtd.h>
>  
> -/**
> - * mtd_probe - Probe the device @dev if not already done
> - *
> - * @dev: U-Boot device to probe
> - *
> - * @return 0 on success, an error otherwise.
> - */
> -int mtd_probe(struct udevice *dev)
> -{
> -	if (device_active(dev))
> -		return 0;
> -
> -	return device_probe(dev);
> -}
> -
>  /*
>   * Implement a MTD uclass which should include most flash drivers.
>   * The uclass private is pointed to mtd_info.
> diff --git a/drivers/mtd/mtd_uboot.c b/drivers/mtd/mtd_uboot.c
> index 7fb72eb1f4..865e9ae4cb 100644
> --- a/drivers/mtd/mtd_uboot.c
> +++ b/drivers/mtd/mtd_uboot.c
> @@ -9,6 +9,7 @@
>  #include <malloc.h>
>  #include <dm/device.h>
>  #include <dm/uclass-internal.h>
> +#include <dm/uclass.h>
>  #include <linux/err.h>
>  #include <linux/mtd/mtd.h>
>  #include <linux/mtd/partitions.h>
> @@ -105,13 +106,9 @@ int mtd_search_alternate_name(const char *mtdname, char *altname,
>  static void mtd_probe_uclass_mtd_devs(void)
>  {
>  	struct udevice *dev;
> -	int idx = 0;
>  
> -	/* Probe devices with DM compliant drivers */
> -	while (!uclass_find_device(UCLASS_MTD, idx, &dev) && dev) {
> -		mtd_probe(dev);
> -		idx++;
> -	}
> +	uclass_foreach_dev_probe(UCLASS_MTD, dev)
> +		;
>  }
>  #else
>  static void mtd_probe_uclass_mtd_devs(void) { }
> diff --git a/include/mtd.h b/include/mtd.h
> index b0f8693386..b569331edb 100644
> --- a/include/mtd.h
> +++ b/include/mtd.h
> @@ -8,7 +8,6 @@
>  
>  #include <linux/mtd/mtd.h>
>  
> -int mtd_probe(struct udevice *dev);
>  int mtd_probe_devices(void);
>  
>  void board_mtdparts_default(const char **mtdids, const char **mtdparts);
> -- 
> 2.26.2
> 

[PATCH u-boot-dm + u-boot-spi v2 7/7] mtd: probe SPI NOR devices in mtd_probe_devices()

[Marek Behún]

In order for `mtd list` U-Boot command to list SPI NOR devices without
the need to run `sf probe` before, we have to probe SPI NOR devices in
mtd_probe_devices().

Signed-off-by: Marek Beh?n <marek.behun@nic.cz>
Cc: Jagan Teki <jagan@amarulasolutions.com>
Cc: Priyanka Jain <priyanka.jain@nxp.com>
Cc: Simon Glass <sjg@chromium.org>
Cc: Heiko Schocher <hs@denx.de>
Cc: Jagan Teki <jagan@amarulasolutions.com>
---
 drivers/mtd/mtd_uboot.c | 14 ++++++++++++++
 1 file changed, 14 insertions(+)

diff --git a/drivers/mtd/mtd_uboot.c b/drivers/mtd/mtd_uboot.c
index 865e9ae4cb..4f5b2fd91d 100644
--- a/drivers/mtd/mtd_uboot.c
+++ b/drivers/mtd/mtd_uboot.c
@@ -114,6 +114,18 @@ static void mtd_probe_uclass_mtd_devs(void)
 static void mtd_probe_uclass_mtd_devs(void) { }
 #endif
 
+#if IS_ENABLED(CONFIG_DM_SPI_FLASH) && IS_ENABLED(CONFIG_SPI_FLASH_MTD)
+static void mtd_probe_uclass_spi_nor_devs(void)
+{
+	struct udevice *dev;
+
+	uclass_foreach_dev_probe(UCLASS_SPI_FLASH, dev)
+		;
+}
+#else
+static void mtd_probe_uclass_spi_nor_devs(void) { }
+#endif
+
 #if defined(CONFIG_MTD_PARTITIONS)
 
 #define MTDPARTS_MAXLEN         512
@@ -309,6 +321,7 @@ int mtd_probe_devices(void)
 	struct mtd_info *mtd;
 
 	mtd_probe_uclass_mtd_devs();
+	mtd_probe_uclass_spi_nor_devs();
 
 	/*
 	 * Check if mtdparts/mtdids changed, if the MTD dev list was updated
@@ -369,6 +382,7 @@ int mtd_probe_devices(void)
 int mtd_probe_devices(void)
 {
 	mtd_probe_uclass_mtd_devs();
+	mtd_probe_uclass_spi_nor_devs();
 
 	return 0;
 }
-- 
2.26.2

[PATCH u-boot-dm + u-boot-spi v2 7/7] mtd: probe SPI NOR devices in mtd_probe_devices()

[Pali Rohár]

On Tuesday 09 February 2021 15:44:52 Marek Beh?n wrote:
> In order for `mtd list` U-Boot command to list SPI NOR devices without
> the need to run `sf probe` before, we have to probe SPI NOR devices in
> mtd_probe_devices().
> 
> Signed-off-by: Marek Beh?n <marek.behun@nic.cz>
> Cc: Jagan Teki <jagan@amarulasolutions.com>
> Cc: Priyanka Jain <priyanka.jain@nxp.com>
> Cc: Simon Glass <sjg@chromium.org>
> Cc: Heiko Schocher <hs@denx.de>
> Cc: Jagan Teki <jagan@amarulasolutions.com>

Reviewed-by: Pali Roh?r <pali@kernel.org>

> ---
>  drivers/mtd/mtd_uboot.c | 14 ++++++++++++++
>  1 file changed, 14 insertions(+)
> 
> diff --git a/drivers/mtd/mtd_uboot.c b/drivers/mtd/mtd_uboot.c
> index 865e9ae4cb..4f5b2fd91d 100644
> --- a/drivers/mtd/mtd_uboot.c
> +++ b/drivers/mtd/mtd_uboot.c
> @@ -114,6 +114,18 @@ static void mtd_probe_uclass_mtd_devs(void)
>  static void mtd_probe_uclass_mtd_devs(void) { }
>  #endif
>  
> +#if IS_ENABLED(CONFIG_DM_SPI_FLASH) && IS_ENABLED(CONFIG_SPI_FLASH_MTD)
> +static void mtd_probe_uclass_spi_nor_devs(void)
> +{
> +	struct udevice *dev;
> +
> +	uclass_foreach_dev_probe(UCLASS_SPI_FLASH, dev)
> +		;
> +}
> +#else
> +static void mtd_probe_uclass_spi_nor_devs(void) { }
> +#endif
> +
>  #if defined(CONFIG_MTD_PARTITIONS)
>  
>  #define MTDPARTS_MAXLEN         512
> @@ -309,6 +321,7 @@ int mtd_probe_devices(void)
>  	struct mtd_info *mtd;
>  
>  	mtd_probe_uclass_mtd_devs();
> +	mtd_probe_uclass_spi_nor_devs();
>  
>  	/*
>  	 * Check if mtdparts/mtdids changed, if the MTD dev list was updated
> @@ -369,6 +382,7 @@ int mtd_probe_devices(void)
>  int mtd_probe_devices(void)
>  {
>  	mtd_probe_uclass_mtd_devs();
> +	mtd_probe_uclass_spi_nor_devs();
>  
>  	return 0;
>  }
> -- 
> 2.26.2
> 

[PATCH u-boot-dm + u-boot-spi v2 0/7] Support SPI NORs and OF partitions in `mtd list`

[Patrice CHOTARD]

+Patrick

Hi All

I have tested this series on stm32mp1-ev1 board which embeds 1 nand and 2 identical spi-nor. 
spi-nor DT node is the following:

&qspi {
	pinctrl-names = "default", "sleep";
	pinctrl-0 = <&qspi_clk_pins_a &qspi_bk1_pins_a &qspi_bk2_pins_a>;
	pinctrl-1 = <&qspi_clk_sleep_pins_a &qspi_bk1_sleep_pins_a &qspi_bk2_sleep_pins_a>;
	reg = <0x58003000 0x1000>, <0x70000000 0x4000000>;
	#address-cells = <1>;
	#size-cells = <0>;
	status = "okay";

	flash0: mx66l51235l at 0 {
		compatible = "jedec,spi-nor";
		reg = <0>;
		spi-rx-bus-width = <4>;
		spi-max-frequency = <108000000>;
		#address-cells = <1>;
		#size-cells = <1>;
	};

	flash1: mx66l51235l at 1 {
		compatible = "jedec,spi-nor";
		reg = <1>;
		spi-rx-bus-width = <4>;
		spi-max-frequency = <108000000>;
		#address-cells = <1>;
		#size-cells = <1>;
	};
};

As is, this series is not able to differentiate the both identical spi-nor using the MTD framework. 
Both spi-nor have the same name "mx66l51235l" as shown below, and usage of mtd command is not possible to reach
both spi-nor:

STM32MP> mtd list
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 at 0
  - parent: spi at 58003000
  - driver: jedec_spi_nor
  - type: NOR flash
  - block size: 0x10000 bytes
  - min I/O: 0x1 bytes
  - 0x000000000000-0x000004000000 : "mx66l51235l"
* mx66l51235l
  - device: mx66l51235l at 1
  - parent: spi at 58003000
  - driver: jedec_spi_nor
  - type: NOR flash
  - block size: 0x10000 bytes
  - min I/O: 0x1 bytes
  - 0x000000000000-0x000004000000 : "mx66l51235l"

The following patch fix it :

@@ -2528,11 +2528,11 @@ int spi_nor_scan(struct spi_nor *nor)
 	ret = spi_nor_init_params(nor, info, &params);
 	if (ret)
 		return ret;
 
 	if (!mtd->name)
-		mtd->name = info->name;
+		mtd->name = (char *)nor->dev->name;
 	mtd->dev = nor->dev;
 	mtd->priv = nor;
 	mtd->type = MTD_NORFLASH;
 	mtd->writesize = 1;
 	mtd->flags = MTD_CAP_NORFLASH;



Now it looks like :

STM32MP> mtd list
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 at 0
  - device: mx66l51235l at 0
  - parent: spi at 58003000
  - driver: jedec_spi_nor
  - type: NOR flash
  - block size: 0x10000 bytes
  - min I/O: 0x1 bytes
  - 0x000000000000-0x000004000000 : "mx66l51235l at 0"
* mx66l51235l at 1
  - device: mx66l51235l at 1
  - parent: spi at 58003000
  - driver: jedec_spi_nor
  - type: NOR flash
  - block size: 0x10000 bytes
  - min I/O: 0x1 bytes
  - 0x000000000000-0x000004000000 : "mx66l51235l at 1"
STM32MP> 

Everything goes fine, i observed one side effect regarding "sf probe" command:

STM32MP> sf probe
SF: Detected mx66l51235l at 0 with page size 256 Bytes, erase size 64 KiB, total 64 MiB
STM32MP> sf probe 1  
SF: Detected mx66l51235l at 1 with page size 256 Bytes, erase size 64 KiB, total 64 MiB

Here the flash name is mx66l51235l at 0 or mx66l51235l at 1 and no more mx66l51235l.

Thanks

Patrice



On 2/9/21 3:44 PM, Marek Beh?n wrote:
> Hello,
> 
> this is v2 of patchset that adds support for U-Boot to parse MTD
> partitions from device-tree, and also improves support for SPI NOR
> access via the `mtd` command.
> 
> Since mtd subsystem is unmaintained, who shall apply the mtd patches?
> I put an u-boot-spi tag into the subject prefix, but am not sure if
> this is correct.
> 
> Changes since v1:
> - added tests of ofnode_get_addr_size_index() and
>   ofnode_get_addr_size_index_notrans() as requested by Simon
> - the last patch now probes SPI NORs in both versions of
>   mtd_probe_devices(), that is when MTDPARTS is enabled or disabled
> 
> Marek
> 
> Marek Beh?n (7):
>   dm: core: add test for ofnode_get_addr_size_index()
>   dm: core: add non-translating version of ofnode_get_addr_size_index()
>   mtd: add support for parsing partitions defined in OF
>   mtd: spi-nor: allow registering multiple MTDs when DM is enabled
>   mtd: spi-nor: fill-in mtd->dev member
>   mtd: remove mtd_probe function
>   mtd: probe SPI NOR devices in mtd_probe_devices()
> 
>  drivers/core/ofnode.c          |  19 ++++-
>  drivers/mtd/mtd-uclass.c       |  15 ----
>  drivers/mtd/mtd_uboot.c        | 129 ++++++++++++++++++++-------------
>  drivers/mtd/mtdpart.c          |  60 +++++++++++++++
>  drivers/mtd/spi/sf_internal.h  |   4 +-
>  drivers/mtd/spi/sf_mtd.c       |  19 ++++-
>  drivers/mtd/spi/sf_probe.c     |   6 +-
>  drivers/mtd/spi/spi-nor-core.c |   1 +
>  drivers/mtd/spi/spi-nor-tiny.c |   1 +
>  include/dm/ofnode.h            |  17 +++++
>  include/linux/mtd/mtd.h        |   9 +++
>  include/mtd.h                  |   1 -
>  test/dm/ofnode.c               |  29 ++++++++
>  13 files changed, 236 insertions(+), 74 deletions(-)
> 

[PATCH u-boot-dm + u-boot-spi v2 0/7] Support SPI NORs and OF partitions in `mtd list`

[Marek Behun]

On Wed, 10 Feb 2021 14:48:23 +0100
Patrice CHOTARD <patrice.chotard@foss.st.com> wrote:

> +Patrick
> 
> Hi All
> 
> I have tested this series on stm32mp1-ev1 board which embeds 1 nand and 2 identical spi-nor. 
> spi-nor DT node is the following:
> 
> &qspi {
> 	pinctrl-names = "default", "sleep";
> 	pinctrl-0 = <&qspi_clk_pins_a &qspi_bk1_pins_a &qspi_bk2_pins_a>;
> 	pinctrl-1 = <&qspi_clk_sleep_pins_a &qspi_bk1_sleep_pins_a &qspi_bk2_sleep_pins_a>;
> 	reg = <0x58003000 0x1000>, <0x70000000 0x4000000>;
> 	#address-cells = <1>;
> 	#size-cells = <0>;
> 	status = "okay";
> 
> 	flash0: mx66l51235l at 0 {
> 		compatible = "jedec,spi-nor";
> 		reg = <0>;
> 		spi-rx-bus-width = <4>;
> 		spi-max-frequency = <108000000>;
> 		#address-cells = <1>;
> 		#size-cells = <1>;
> 	};
> 
> 	flash1: mx66l51235l at 1 {
> 		compatible = "jedec,spi-nor";
> 		reg = <1>;
> 		spi-rx-bus-width = <4>;
> 		spi-max-frequency = <108000000>;
> 		#address-cells = <1>;
> 		#size-cells = <1>;
> 	};
> };
> 
> As is, this series is not able to differentiate the both identical spi-nor using the MTD framework. 
> Both spi-nor have the same name "mx66l51235l" as shown below, and usage of mtd command is not possible to reach
> both spi-nor:
> 
> STM32MP> mtd list  
> 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 at 0
>   - parent: spi at 58003000
>   - driver: jedec_spi_nor
>   - type: NOR flash
>   - block size: 0x10000 bytes
>   - min I/O: 0x1 bytes
>   - 0x000000000000-0x000004000000 : "mx66l51235l"
> * mx66l51235l
>   - device: mx66l51235l at 1
>   - parent: spi at 58003000
>   - driver: jedec_spi_nor
>   - type: NOR flash
>   - block size: 0x10000 bytes
>   - min I/O: 0x1 bytes
>   - 0x000000000000-0x000004000000 : "mx66l51235l"
> 
> The following patch fix it :
> 
> @@ -2528,11 +2528,11 @@ int spi_nor_scan(struct spi_nor *nor)
>  	ret = spi_nor_init_params(nor, info, &params);
>  	if (ret)
>  		return ret;
>  
>  	if (!mtd->name)
> -		mtd->name = info->name;
> +		mtd->name = (char *)nor->dev->name;
>  	mtd->dev = nor->dev;
>  	mtd->priv = nor;
>  	mtd->type = MTD_NORFLASH;
>  	mtd->writesize = 1;
>  	mtd->flags = MTD_CAP_NORFLASH;
> 
> 
> 
> Now it looks like :
> 
> STM32MP> mtd list  
> 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 at 0
>   - device: mx66l51235l at 0
>   - parent: spi at 58003000
>   - driver: jedec_spi_nor
>   - type: NOR flash
>   - block size: 0x10000 bytes
>   - min I/O: 0x1 bytes
>   - 0x000000000000-0x000004000000 : "mx66l51235l at 0"
> * mx66l51235l at 1
>   - device: mx66l51235l at 1
>   - parent: spi at 58003000
>   - driver: jedec_spi_nor
>   - type: NOR flash
>   - block size: 0x10000 bytes
>   - min I/O: 0x1 bytes
>   - 0x000000000000-0x000004000000 : "mx66l51235l at 1"
> STM32MP>   
> 
> Everything goes fine, i observed one side effect regarding "sf probe" command:
> 
> STM32MP> sf probe  
> SF: Detected mx66l51235l at 0 with page size 256 Bytes, erase size 64 KiB, total 64 MiB
> STM32MP> sf probe 1    
> SF: Detected mx66l51235l at 1 with page size 256 Bytes, erase size 64 KiB, total 64 MiB
> 
> Here the flash name is mx66l51235l at 0 or mx66l51235l at 1 and no more mx66l51235l.

I too tried that change (using (char *)nor->dev->name as mtd name).

The thing is that the name "mx66l51235l" is determined from the SPI NOR
ID (not the device-tree), so if you solder a different NOR, the name
will be different. Meanwhile dts name is always the same.

I think the mtd command should at least inform the user what kind of SPI
NOR is soldered on the board.

So one solution is to use device-tree name to select the NOR, but still
print the name determined from SPI ID somewhere.

Second solution is to allow selecting the SPI by number, i.e. (the n-th
enumerated SPI).

(BTW regarding your dts node names, the node names should be spi-nor at 0
 and spi-nor at 1. Yes, I know that spi-nor is not documented in
 device-tree bindings yet, but this would be consistent with other
 device-tree bindings in Linux.)

Marek

[PATCH u-boot-dm + u-boot-spi v2 0/7] Support SPI NORs and OF partitions in `mtd list`

[Patrice CHOTARD]

Hi Marek

On 2/10/21 3:14 PM, Marek Behun wrote:
> On Wed, 10 Feb 2021 14:48:23 +0100
> Patrice CHOTARD <patrice.chotard@foss.st.com> wrote:
> 
>> +Patrick
>>
>> Hi All
>>
>> I have tested this series on stm32mp1-ev1 board which embeds 1 nand and 2 identical spi-nor. 
>> spi-nor DT node is the following:
>>
>> &qspi {
>> 	pinctrl-names = "default", "sleep";
>> 	pinctrl-0 = <&qspi_clk_pins_a &qspi_bk1_pins_a &qspi_bk2_pins_a>;
>> 	pinctrl-1 = <&qspi_clk_sleep_pins_a &qspi_bk1_sleep_pins_a &qspi_bk2_sleep_pins_a>;
>> 	reg = <0x58003000 0x1000>, <0x70000000 0x4000000>;
>> 	#address-cells = <1>;
>> 	#size-cells = <0>;
>> 	status = "okay";
>>
>> 	flash0: mx66l51235l at 0 {
>> 		compatible = "jedec,spi-nor";
>> 		reg = <0>;
>> 		spi-rx-bus-width = <4>;
>> 		spi-max-frequency = <108000000>;
>> 		#address-cells = <1>;
>> 		#size-cells = <1>;
>> 	};
>>
>> 	flash1: mx66l51235l at 1 {
>> 		compatible = "jedec,spi-nor";
>> 		reg = <1>;
>> 		spi-rx-bus-width = <4>;
>> 		spi-max-frequency = <108000000>;
>> 		#address-cells = <1>;
>> 		#size-cells = <1>;
>> 	};
>> };
>>
>> As is, this series is not able to differentiate the both identical spi-nor using the MTD framework. 
>> Both spi-nor have the same name "mx66l51235l" as shown below, and usage of mtd command is not possible to reach
>> both spi-nor:
>>
>> STM32MP> mtd list  
>> 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 at 0
>>   - parent: spi at 58003000
>>   - driver: jedec_spi_nor
>>   - type: NOR flash
>>   - block size: 0x10000 bytes
>>   - min I/O: 0x1 bytes
>>   - 0x000000000000-0x000004000000 : "mx66l51235l"
>> * mx66l51235l
>>   - device: mx66l51235l at 1
>>   - parent: spi at 58003000
>>   - driver: jedec_spi_nor
>>   - type: NOR flash
>>   - block size: 0x10000 bytes
>>   - min I/O: 0x1 bytes
>>   - 0x000000000000-0x000004000000 : "mx66l51235l"
>>
>> The following patch fix it :
>>
>> @@ -2528,11 +2528,11 @@ int spi_nor_scan(struct spi_nor *nor)
>>  	ret = spi_nor_init_params(nor, info, &params);
>>  	if (ret)
>>  		return ret;
>>  
>>  	if (!mtd->name)
>> -		mtd->name = info->name;
>> +		mtd->name = (char *)nor->dev->name;
>>  	mtd->dev = nor->dev;
>>  	mtd->priv = nor;
>>  	mtd->type = MTD_NORFLASH;
>>  	mtd->writesize = 1;
>>  	mtd->flags = MTD_CAP_NORFLASH;
>>
>>
>>
>> Now it looks like :
>>
>> STM32MP> mtd list  
>> 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 at 0
>>   - device: mx66l51235l at 0
>>   - parent: spi at 58003000
>>   - driver: jedec_spi_nor
>>   - type: NOR flash
>>   - block size: 0x10000 bytes
>>   - min I/O: 0x1 bytes
>>   - 0x000000000000-0x000004000000 : "mx66l51235l at 0"
>> * mx66l51235l at 1
>>   - device: mx66l51235l at 1
>>   - parent: spi at 58003000
>>   - driver: jedec_spi_nor
>>   - type: NOR flash
>>   - block size: 0x10000 bytes
>>   - min I/O: 0x1 bytes
>>   - 0x000000000000-0x000004000000 : "mx66l51235l at 1"
>> STM32MP>   
>>
>> Everything goes fine, i observed one side effect regarding "sf probe" command:
>>
>> STM32MP> sf probe  
>> SF: Detected mx66l51235l at 0 with page size 256 Bytes, erase size 64 KiB, total 64 MiB
>> STM32MP> sf probe 1    
>> SF: Detected mx66l51235l at 1 with page size 256 Bytes, erase size 64 KiB, total 64 MiB
>>
>> Here the flash name is mx66l51235l at 0 or mx66l51235l at 1 and no more mx66l51235l.
> 
> I too tried that change (using (char *)nor->dev->name as mtd name).
> 
> The thing is that the name "mx66l51235l" is determined from the SPI NOR
> ID (not the device-tree), so if you solder a different NOR, the name
> will be different. Meanwhile dts name is always the same.

Ok got it, i didn't pay attention to the fact nor->dev->name doesn't come from spi-nor-ids but from DT .... :-(

> 
> I think the mtd command should at least inform the user what kind of SPI
> NOR is soldered on the board.

Fully agree

> 
> So one solution is to use device-tree name to select the NOR, but still
> print the name determined from SPI ID somewhere.
> 
> Second solution is to allow selecting the SPI by number, i.e. (the n-th
> enumerated SPI).

and what about <name from SPI id>@<chip select> as shown in my previous email ?

> 
> (BTW regarding your dts node names, the node names should be spi-nor at 0
>  and spi-nor at 1. Yes, I know that spi-nor is not documented in
>  device-tree bindings yet, but this would be consistent with other
>  device-tree bindings in Linux.)

You are right

Patrice

> 
> Marek
> 

[PATCH u-boot-dm + u-boot-spi v2 0/7] Support SPI NORs and OF partitions in `mtd list`

[Marek Behun]

On Wed, 10 Feb 2021 15:31:56 +0100
Patrice CHOTARD <patrice.chotard@foss.st.com> wrote:

> > So one solution is to use device-tree name to select the NOR, but still
> > print the name determined from SPI ID somewhere.
> > 
> > Second solution is to allow selecting the SPI by number, i.e. (the n-th
> > enumerated SPI).  
> 
> and what about <name from SPI id>@<chip select> as shown in my previous email ?

That could be one solution, but there would still be a collision if
there were multiple SPI controllers. It is possible to have two
identical SPI-NORs connected to 2 different SPI controllers on the same
chip select. But maybe this situation is rare enough that we can ignore
it.

[PATCH u-boot-dm + u-boot-spi v2 0/7] Support SPI NORs and OF partitions in `mtd list`

[Marek Behun]

On Wed, 10 Feb 2021 15:31:56 +0100
Patrice CHOTARD <patrice.chotard@foss.st.com> wrote:

> and what about <name from SPI id>@<chip select> as shown in my previous email ?

And there is a second problem with this: this can be bad for automatic
U-Boot scripts.

For example on Turris Omnia there are boards which were manufactured
with a Macronix SPI-NOR, and boards which were manufactured with
Winbond SPI-NOR (or Spansion or something different from Macronix).

So I think that there should be some mechanism available via the mtd
command to select the SPI-NOR by its chip select somehow, without
knowing the SPI ID.