[PATCH u-boot-dm + u-boot-spi v3 00/11] Support SPI NORs and OF partitions in `mtd list`

[PATCH u-boot-dm + u-boot-spi v3 00/11] Support SPI NORs and OF partitions in `mtd list`

[Marek Behún]

Hello,

this is v3 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.

Changes since v2:
- addressed Pali's comments in patch that adds partition parsing (4/7 in
  this version): no check for whether the `compatible` property is
  present in a partition node and added comment explaining mask flags)
- added 4 more patches:
  1) adding ofnode_get_path() function
  2) printing OF path in `mtd list`
  3) in `mtd read <name> ...`, <name> can now also be DM's device name
     or OF path
  4) the fact from 3) is added to `mtd help`

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

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>
Cc: Patrick Delaunay <patrick.delaunay@st.com>
Cc: Patrice CHOTARD <patrice.chotard@foss.st.com>
Cc: Miquel Raynal <miquel.raynal@bootlin.com>

Marek Beh?n (11):
  dm: core: add test for ofnode_get_addr_size_index()
  dm: core: add non-translating version of ofnode_get_addr_size_index()
  dm: core: add ofnode_get_path()
  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()
  cmd: mtd: print device OF path in listing
  mtd: compare also with OF path and device name in get_mtd_device_nm()
  cmd: mtd: expand <name> argument definition in command help

 cmd/mtd.c                      |  11 ++-
 drivers/core/ofnode.c          |  44 ++++++++++-
 drivers/mtd/mtd-uclass.c       |  15 ----
 drivers/mtd/mtd_uboot.c        | 129 ++++++++++++++++++++-------------
 drivers/mtd/mtdcore.c          |  29 ++++++++
 drivers/mtd/mtdpart.c          |  63 ++++++++++++++++
 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            |  27 +++++++
 include/linux/mtd/mtd.h        |   9 +++
 include/mtd.h                  |   1 -
 test/dm/ofnode.c               |  50 +++++++++++++
 15 files changed, 334 insertions(+), 75 deletions(-)

-- 
2.26.2

[PATCH u-boot-dm + u-boot-spi v3 01/11] 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>
Reviewed-by: 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 v3 02/11] 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>
Reviewed-by: 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 fa0bd2a9c4..702cd7482c 100644
--- a/drivers/core/ofnode.c
+++ b/drivers/core/ofnode.c
@@ -299,7 +299,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;
 
@@ -317,7 +318,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 {
@@ -329,12 +330,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 2c0597c407..8e641418cb 100644
--- a/include/dm/ofnode.h
+++ b/include/dm/ofnode.h
@@ -489,6 +489,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 v3 03/11] dm: core: add ofnode_get_path()

[Marek Behún]

Add function for retrieving full node path of a given ofnode.
This uses np->full_name if OF is live, otherwise a call to
fdt_get_path() is made.

Signed-off-by: Marek Beh?n <marek.behun@nic.cz>
Cc: Simon Glass <sjg@chromium.org>
---
 drivers/core/ofnode.c | 25 +++++++++++++++++++++++++
 include/dm/ofnode.h   | 10 ++++++++++
 test/dm/ofnode.c      | 21 +++++++++++++++++++++
 3 files changed, 56 insertions(+)

diff --git a/drivers/core/ofnode.c b/drivers/core/ofnode.c
index 702cd7482c..5bc40618d7 100644
--- a/drivers/core/ofnode.c
+++ b/drivers/core/ofnode.c
@@ -286,6 +286,31 @@ const char *ofnode_get_name(ofnode node)
 	return fdt_get_name(gd->fdt_blob, ofnode_to_offset(node), NULL);
 }
 
+int ofnode_get_path(ofnode node, char *buf, int buflen)
+{
+	assert(ofnode_valid(node));
+
+	if (ofnode_is_np(node)) {
+		if (strlen(node.np->full_name) >= buflen)
+			return -ENOSPC;
+
+		strcpy(buf, node.np->full_name);
+
+		return 0;
+	} else {
+		int res;
+
+		res = fdt_get_path(gd->fdt_blob, ofnode_to_offset(node), buf,
+				   buflen);
+		if (!res)
+			return res;
+		else if (res == -FDT_ERR_NOSPACE)
+			return -ENOSPC;
+		else
+			return -EINVAL;
+	}
+}
+
 ofnode ofnode_get_by_phandle(uint phandle)
 {
 	ofnode node;
diff --git a/include/dm/ofnode.h b/include/dm/ofnode.h
index 8e641418cb..8518e3cabb 100644
--- a/include/dm/ofnode.h
+++ b/include/dm/ofnode.h
@@ -458,6 +458,16 @@ ofnode ofnode_get_parent(ofnode node);
  */
 const char *ofnode_get_name(ofnode node);
 
+/**
+ * ofnode_get_path() - get the full path of a node
+ *
+ * @node: valid node to look up
+ * @buf: buffer to write the node path into
+ * @buflen: buffer size
+ * @return 0 if OK, -ve on error
+ */
+int ofnode_get_path(ofnode node, char *buf, int buflen);
+
 /**
  * ofnode_get_by_phandle() - get ofnode from phandle
  *
diff --git a/test/dm/ofnode.c b/test/dm/ofnode.c
index 48c121df25..1fe635c94b 100644
--- a/test/dm/ofnode.c
+++ b/test/dm/ofnode.c
@@ -290,3 +290,24 @@ static int dm_test_ofnode_get_addr_size(struct unit_test_state *uts)
 	return 0;
 }
 DM_TEST(dm_test_ofnode_get_addr_size, UT_TESTF_SCAN_PDATA | UT_TESTF_SCAN_FDT);
+
+static int dm_test_ofnode_get_path(struct unit_test_state *uts)
+{
+	const char *path = "/translation-test at 8000/xlatebus at 4,400/devs/dev at 19";
+	char buf[64];
+	ofnode node;
+	int res;
+
+	node = ofnode_path(path);
+	ut_assert(ofnode_valid(node));
+
+	res = ofnode_get_path(node, buf, 64);
+	ut_asserteq(0, res);
+	ut_asserteq_str(path, buf);
+
+	res = ofnode_get_path(node, buf, 32);
+	ut_asserteq(-ENOSPC, res);
+
+	return 0;
+}
+DM_TEST(dm_test_ofnode_get_path, UT_TESTF_SCAN_PDATA | UT_TESTF_SCAN_FDT);
-- 
2.26.2

[PATCH u-boot-dm + u-boot-spi v3 03/11] dm: core: add ofnode_get_path()

[Simon Glass]

On Thu, 25 Feb 2021 at 09:14, Marek Beh?n <marek.behun@nic.cz> wrote:
>
> Add function for retrieving full node path of a given ofnode.
> This uses np->full_name if OF is live, otherwise a call to
> fdt_get_path() is made.
>
> Signed-off-by: Marek Beh?n <marek.behun@nic.cz>
> Cc: Simon Glass <sjg@chromium.org>
> ---
>  drivers/core/ofnode.c | 25 +++++++++++++++++++++++++
>  include/dm/ofnode.h   | 10 ++++++++++
>  test/dm/ofnode.c      | 21 +++++++++++++++++++++
>  3 files changed, 56 insertions(+)

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

[PATCH u-boot-dm + u-boot-spi v3 04/11] 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>
Cc: Patrick Delaunay <patrick.delaunay@st.com>
Cc: Patrice CHOTARD <patrice.chotard@foss.st.com>
Cc: Miquel Raynal <miquel.raynal@bootlin.com>
---
 drivers/mtd/mtd_uboot.c | 106 +++++++++++++++++++++++-----------------
 drivers/mtd/mtdpart.c   |  63 ++++++++++++++++++++++++
 include/linux/mtd/mtd.h |   9 ++++
 3 files changed, 134 insertions(+), 44 deletions(-)

diff --git a/drivers/mtd/mtd_uboot.c b/drivers/mtd/mtd_uboot.c
index c53ec657a3..4843cf1b84 100644
--- a/drivers/mtd/mtd_uboot.c
+++ b/drivers/mtd/mtd_uboot.c
@@ -198,53 +198,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;
@@ -343,6 +301,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..ed5fb5a21f 100644
--- a/drivers/mtd/mtdpart.c
+++ b/drivers/mtd/mtdpart.c
@@ -892,6 +892,69 @@ 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;
+
+		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");
+
+		/*
+		 * .mask_flags is used to remove flags in allocate_partition(),
+		 * so when "read-only" is present, we add MTD_WRITABLE to the
+		 * mask, and so MTD_WRITABLE will be removed on partition
+		 * allocation
+		 */
+		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 v3 05/11] 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>
Reviewed-by: Pali Roh?r <pali@kernel.org>
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>
Cc: Patrick Delaunay <patrick.delaunay@st.com>
Cc: Patrice CHOTARD <patrice.chotard@foss.st.com>
Cc: Miquel Raynal <miquel.raynal@bootlin.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 v3 05/11] mtd: spi-nor: allow registering multiple MTDs when DM is enabled

[Miquel Raynal]

Hi Marek,

Marek Beh?n <marek.behun@nic.cz> wrote on Thu, 25 Feb 2021 15:13:30
+0100:

> 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>
> Reviewed-by: Pali Roh?r <pali@kernel.org>
> 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>
> Cc: Patrick Delaunay <patrick.delaunay@st.com>
> Cc: Patrice CHOTARD <patrice.chotard@foss.st.com>
> Cc: Miquel Raynal <miquel.raynal@bootlin.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) */
> +

I actually started something to overcome the same issue but left it
aside waiting for more inspiration. Your approach is neat, thanks for
pushing this!

Thanks,
Miqu?l

[PATCH u-boot-dm + u-boot-spi v3 06/11] 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>
Reviewed-by: Pali Roh?r <pali@kernel.org>
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>
Cc: Patrick Delaunay <patrick.delaunay@st.com>
Cc: Patrice CHOTARD <patrice.chotard@foss.st.com>
Cc: Miquel Raynal <miquel.raynal@bootlin.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 v3 07/11] 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>
Reviewed-by: Pali Roh?r <pali@kernel.org>
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>
Cc: Patrick Delaunay <patrick.delaunay@st.com>
Cc: Patrice CHOTARD <patrice.chotard@foss.st.com>
Cc: Miquel Raynal <miquel.raynal@bootlin.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 4843cf1b84..a652d431ba 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>
@@ -106,13 +107,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 v3 08/11] 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>
Reviewed-by: Pali Roh?r <pali@kernel.org>
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>
Cc: Patrick Delaunay <patrick.delaunay@st.com>
Cc: Patrice CHOTARD <patrice.chotard@foss.st.com>
Cc: Miquel Raynal <miquel.raynal@bootlin.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 a652d431ba..90767ec417 100644
--- a/drivers/mtd/mtd_uboot.c
+++ b/drivers/mtd/mtd_uboot.c
@@ -115,6 +115,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
@@ -310,6 +322,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
@@ -370,6 +383,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 v3 09/11] cmd: mtd: print device OF path in listing

[Marek Behún]

Print MTD's device OF path in the output of `mtd list` command.

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>
Cc: Patrick Delaunay <patrick.delaunay@st.com>
Cc: Patrice CHOTARD <patrice.chotard@foss.st.com>
Cc: Miquel Raynal <miquel.raynal@bootlin.com>
---
 cmd/mtd.c | 9 +++++++++
 1 file changed, 9 insertions(+)

diff --git a/cmd/mtd.c b/cmd/mtd.c
index 485a963bdd..446cbc1038 100644
--- a/cmd/mtd.c
+++ b/cmd/mtd.c
@@ -126,6 +126,15 @@ static void mtd_show_device(struct mtd_info *mtd)
 		printf("  - driver: %s\n", mtd->dev->driver->name);
 	}
 #endif
+#if defined(CONFIG_OF_CONTROL)
+	if (mtd->dev) {
+		char buf[256];
+		int res;
+
+		res = ofnode_get_path(mtd_get_ofnode(mtd), buf, 256);
+		printf("  - path: %s\n", res == 0 ? buf : "unavailable");
+	}
+#endif
 
 	/* MTD device information */
 	printf("  - type: ");
-- 
2.26.2

[PATCH u-boot-dm + u-boot-spi v3 09/11] cmd: mtd: print device OF path in listing

[Simon Glass]

Hi Marek,

On Thu, 25 Feb 2021 at 09:14, Marek Beh?n <marek.behun@nic.cz> wrote:
>
> Print MTD's device OF path in the output of `mtd list` command.
>
> 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>
> Cc: Patrick Delaunay <patrick.delaunay@st.com>
> Cc: Patrice CHOTARD <patrice.chotard@foss.st.com>
> Cc: Miquel Raynal <miquel.raynal@bootlin.com>
> ---
>  cmd/mtd.c | 9 +++++++++
>  1 file changed, 9 insertions(+)
>
> diff --git a/cmd/mtd.c b/cmd/mtd.c
> index 485a963bdd..446cbc1038 100644
> --- a/cmd/mtd.c
> +++ b/cmd/mtd.c
> @@ -126,6 +126,15 @@ static void mtd_show_device(struct mtd_info *mtd)
>                 printf("  - driver: %s\n", mtd->dev->driver->name);
>         }
>  #endif
> +#if defined(CONFIG_OF_CONTROL)
> +       if (mtd->dev) {

Can you do:

if (IS_ENABLED(CONFIG_OF_CONTROL) && mtd->dev)

?

> +               char buf[256];
> +               int res;
> +
> +               res = ofnode_get_path(mtd_get_ofnode(mtd), buf, 256);
> +               printf("  - path: %s\n", res == 0 ? buf : "unavailable");
> +       }
> +#endif
>
>         /* MTD device information */
>         printf("  - type: ");
> --
> 2.26.2
>

Regards,
Simon

[PATCH u-boot-dm + u-boot-spi v3 10/11] mtd: compare also with OF path and device name in get_mtd_device_nm()

[Marek Behún]

The get_mtd_device_nm() function (code imported from Linux) simply
iterates all registered MTD devices and compares the given name with
all MTDs' names.

With SPI_FLASH_MTD enabled U-Boot registers a SPI-NOR as a MTD device
with name identical to the SPI flash chip name (from SPI ID table). Thus
for a board with multiple same SPI-NORs it registers multiple MTDs, but
all with the same name (such as "s25fl164k"). We do not want to change
this behaviour, since such a change could break existing boot scripts,
which can rely on a hardcoded name.

In order to allow somehow to uniqely select a MTD device, change
get_mtd_device_nm() function as such:
- if first character of name is '/', interpret it as OF path
- otherwise compare the name with MTDs name and MTDs device name.

In the following example a board has two "s25fl164k" SPI-NORs. They both
have name "s25fl164k", thus cannot be uniquely selected via this name.
With this change, the user can select the second SPI-NOR either with
"spi-nor at 1" or "/soc/spi at 10600/spi-nor at 1".

Example:
  => mtd list
  List of MTD devices:
  * s25fl164k
    - device: spi-nor at 0
    - parent: spi at 10600
    - driver: jedec_spi_nor
    - path: /soc/spi at 10600/spi-nor at 0
    - type: NOR flash
    - block size: 0x1000 bytes
    - min I/O: 0x1 bytes
    - 0x000000000000-0x000000800000 : "s25fl164k"
  * s25fl164k
    - device: spi-nor at 1
    - parent: spi at 10600
    - driver: jedec_spi_nor
    - path: /soc/spi at 10600/spi-nor at 1
    - type: NOR flash
    - block size: 0x1000 bytes
    - min I/O: 0x1 bytes
    - 0x000000000000-0x000000800000 : "s25fl164k"

This change adds code that depends on CONFIG_DM. Although CONFIG_DM
is compulsory since v2020.01, there are still some boards (for example
tqma6s_wru4_mmc_defconfig) that don't enable it. Therefore the code
guards this parts by #ifdefs.

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>
Cc: Patrick Delaunay <patrick.delaunay@st.com>
Cc: Patrice CHOTARD <patrice.chotard@foss.st.com>
Cc: Miquel Raynal <miquel.raynal@bootlin.com>
---
 drivers/mtd/mtdcore.c | 29 +++++++++++++++++++++++++++++
 1 file changed, 29 insertions(+)

diff --git a/drivers/mtd/mtdcore.c b/drivers/mtd/mtdcore.c
index 0d1f94c6cb..7c894b2e41 100644
--- a/drivers/mtd/mtdcore.c
+++ b/drivers/mtd/mtdcore.c
@@ -780,13 +780,42 @@ struct mtd_info *get_mtd_device_nm(const char *name)
 {
 	int err = -ENODEV;
 	struct mtd_info *mtd = NULL, *other;
+#ifdef CONFIG_DM
+	struct udevice *dev = NULL;
+#endif
+
+#ifdef CONFIG_DM
+	/*
+	 * If the first character of mtd name is '/', interpret it as OF path.
+	 * Otherwise try comparing by mtd->name and mtd->dev->name.
+	 */
+	if (*name == '/') {
+		err = device_get_global_by_ofnode(ofnode_path(name), &dev);
+		if (err)
+			return ERR_PTR(err);
+	}
+#endif
 
 	mutex_lock(&mtd_table_mutex);
 
 	mtd_for_each_device(other) {
+#ifdef CONFIG_DM
+		if ((dev && !mtd_is_partition(other) && other->dev == dev) ||
+		    !strcmp(name, other->name) ||
+		    (!mtd_is_partition(other) && other->dev &&
+		     !strcmp(name, other->dev->name))) {
+#else
 		if (!strcmp(name, other->name)) {
+#endif
+#ifdef __UBOOT__
+			if (mtd)
+				printf("\nWarning: MTD name \"%s\" is not unique!\n\n",
+				       name);
+			mtd = other;
+#else /* !__UBOOT__ */
 			mtd = other;
 			break;
+#endif /* !__UBOOT__ */
 		}
 	}
 
-- 
2.26.2

[PATCH u-boot-dm + u-boot-spi v3 10/11] mtd: compare also with OF path and device name in get_mtd_device_nm()

[Simon Glass]

Hi Marek,

On Thu, 25 Feb 2021 at 09:14, Marek Beh?n <marek.behun@nic.cz> wrote:
>
> The get_mtd_device_nm() function (code imported from Linux) simply
> iterates all registered MTD devices and compares the given name with
> all MTDs' names.
>
> With SPI_FLASH_MTD enabled U-Boot registers a SPI-NOR as a MTD device
> with name identical to the SPI flash chip name (from SPI ID table). Thus
> for a board with multiple same SPI-NORs it registers multiple MTDs, but
> all with the same name (such as "s25fl164k"). We do not want to change
> this behaviour, since such a change could break existing boot scripts,
> which can rely on a hardcoded name.
>
> In order to allow somehow to uniqely select a MTD device, change
> get_mtd_device_nm() function as such:
> - if first character of name is '/', interpret it as OF path
> - otherwise compare the name with MTDs name and MTDs device name.
>
> In the following example a board has two "s25fl164k" SPI-NORs. They both
> have name "s25fl164k", thus cannot be uniquely selected via this name.
> With this change, the user can select the second SPI-NOR either with
> "spi-nor at 1" or "/soc/spi at 10600/spi-nor at 1".
>
> Example:
>   => mtd list
>   List of MTD devices:
>   * s25fl164k
>     - device: spi-nor at 0
>     - parent: spi at 10600
>     - driver: jedec_spi_nor
>     - path: /soc/spi at 10600/spi-nor at 0
>     - type: NOR flash
>     - block size: 0x1000 bytes
>     - min I/O: 0x1 bytes
>     - 0x000000000000-0x000000800000 : "s25fl164k"
>   * s25fl164k
>     - device: spi-nor at 1
>     - parent: spi at 10600
>     - driver: jedec_spi_nor
>     - path: /soc/spi at 10600/spi-nor at 1
>     - type: NOR flash
>     - block size: 0x1000 bytes
>     - min I/O: 0x1 bytes
>     - 0x000000000000-0x000000800000 : "s25fl164k"
>
> This change adds code that depends on CONFIG_DM. Although CONFIG_DM
> is compulsory since v2020.01, there are still some boards (for example
> tqma6s_wru4_mmc_defconfig) that don't enable it. Therefore the code
> guards this parts by #ifdefs.
>
> 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>
> Cc: Patrick Delaunay <patrick.delaunay@st.com>
> Cc: Patrice CHOTARD <patrice.chotard@foss.st.com>
> Cc: Miquel Raynal <miquel.raynal@bootlin.com>
> ---
>  drivers/mtd/mtdcore.c | 29 +++++++++++++++++++++++++++++
>  1 file changed, 29 insertions(+)
>
> diff --git a/drivers/mtd/mtdcore.c b/drivers/mtd/mtdcore.c
> index 0d1f94c6cb..7c894b2e41 100644
> --- a/drivers/mtd/mtdcore.c
> +++ b/drivers/mtd/mtdcore.c
> @@ -780,13 +780,42 @@ struct mtd_info *get_mtd_device_nm(const char *name)
>  {
>         int err = -ENODEV;
>         struct mtd_info *mtd = NULL, *other;
> +#ifdef CONFIG_DM
> +       struct udevice *dev = NULL;
> +#endif
> +
> +#ifdef CONFIG_DM

We should not need CONFIG_DM here...it should be enabled for all
boards. You can always disable MTD for a board if not, or send a
removable patch.

If for some reason you do, please use if (IS_ENABLED() so that 'dev'
can always be declared.

> +       /*
> +        * If the first character of mtd name is '/', interpret it as OF path.
> +        * Otherwise try comparing by mtd->name and mtd->dev->name.
> +        */
> +       if (*name == '/') {
> +               err = device_get_global_by_ofnode(ofnode_path(name), &dev);
> +               if (err)
> +                       return ERR_PTR(err);
> +       }
> +#endif
>
>         mutex_lock(&mtd_table_mutex);
>
>         mtd_for_each_device(other) {
> +#ifdef CONFIG_DM
> +               if ((dev && !mtd_is_partition(other) && other->dev == dev) ||
> +                   !strcmp(name, other->name) ||
> +                   (!mtd_is_partition(other) && other->dev &&
> +                    !strcmp(name, other->dev->name))) {
> +#else
>                 if (!strcmp(name, other->name)) {
> +#endif
> +#ifdef __UBOOT__
> +                       if (mtd)
> +                               printf("\nWarning: MTD name \"%s\" is not unique!\n\n",
> +                                      name);
> +                       mtd = other;
> +#else /* !__UBOOT__ */
>                         mtd = other;
>                         break;
> +#endif /* !__UBOOT__ */
>                 }
>         }
>
> --
> 2.26.2
>

Regards,
Simon

[PATCH u-boot-dm + u-boot-spi v3 10/11] mtd: compare also with OF path and device name in get_mtd_device_nm()

[Marek Behun]

On Thu, 25 Feb 2021 14:31:42 -0500
Simon Glass <sjg@chromium.org> wrote:

> We should not need CONFIG_DM here...it should be enabled for all
> boards. You can always disable MTD for a board if not, or send a
> removable patch.
> 
> If for some reason you do, please use if (IS_ENABLED() so that 'dev'
> can always be declared.

Simon, it still isn't enabled for all boards. For example
tqma6s_wru4_mmc_defconfig does not compile with this. I actually wrote
this into commit message:

  Although CONFIG_DM is compulsory since v2020.01, there are still some
  boards (for example tqma6s_wru4_mmc_defconfig) that don't enable it. 

But if breaking such boards is not a problem anymore, I will gladly
just remove the ifdefs :) Should I?

Marek

[PATCH u-boot-dm + u-boot-spi v3 10/11] mtd: compare also with OF path and device name in get_mtd_device_nm()

[Tom Rini]

On Thu, Feb 25, 2021 at 09:07:40PM +0100, Marek Behun wrote:
> On Thu, 25 Feb 2021 14:31:42 -0500
> Simon Glass <sjg@chromium.org> wrote:
> 
> > We should not need CONFIG_DM here...it should be enabled for all
> > boards. You can always disable MTD for a board if not, or send a
> > removable patch.
> > 
> > If for some reason you do, please use if (IS_ENABLED() so that 'dev'
> > can always be declared.
> 
> Simon, it still isn't enabled for all boards. For example
> tqma6s_wru4_mmc_defconfig does not compile with this. I actually wrote
> this into commit message:
> 
>   Although CONFIG_DM is compulsory since v2020.01, there are still some
>   boards (for example tqma6s_wru4_mmc_defconfig) that don't enable it. 
> 
> But if breaking such boards is not a problem anymore, I will gladly
> just remove the ifdefs :) Should I?

So, I'm working hard at dropping boards that are well past migration
deadlines.  That specific one fails DM_MMC more importantly, and I will
be dropping it if it's not migrated, after v2021.04 is out.  What else
fails?  If you rebase your series on my
WIP/remove-non-AHCI_LIBATA-drivers (as it has the most board removals in
it), what fails to build if your series is just DM only?

-- 
Tom
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 659 bytes
Desc: not available
URL: <https://lists.denx.de/pipermail/u-boot/attachments/20210225/98628cb9/attachment.sig>

[PATCH u-boot-dm + u-boot-spi v3 10/11] mtd: compare also with OF path and device name in get_mtd_device_nm()

[Marek Behun]

On Thu, 25 Feb 2021 15:28:56 -0500
Tom Rini <trini@konsulko.com> wrote:

> On Thu, Feb 25, 2021 at 09:07:40PM +0100, Marek Behun wrote:
> > On Thu, 25 Feb 2021 14:31:42 -0500
> > Simon Glass <sjg@chromium.org> wrote:
> >   
> > > We should not need CONFIG_DM here...it should be enabled for all
> > > boards. You can always disable MTD for a board if not, or send a
> > > removable patch.
> > > 
> > > If for some reason you do, please use if (IS_ENABLED() so that 'dev'
> > > can always be declared.  
> > 
> > Simon, it still isn't enabled for all boards. For example
> > tqma6s_wru4_mmc_defconfig does not compile with this. I actually wrote
> > this into commit message:
> > 
> >   Although CONFIG_DM is compulsory since v2020.01, there are still some
> >   boards (for example tqma6s_wru4_mmc_defconfig) that don't enable it. 
> > 
> > But if breaking such boards is not a problem anymore, I will gladly
> > just remove the ifdefs :) Should I?  
> 
> So, I'm working hard at dropping boards that are well past migration
> deadlines.  That specific one fails DM_MMC more importantly, and I will
> be dropping it if it's not migrated, after v2021.04 is out.  What else
> fails?  If you rebase your series on my
> WIP/remove-non-AHCI_LIBATA-drivers (as it has the most board removals in
> it), what fails to build if your series is just DM only?
> 

I haven't tried building all boards, just quickly found a defconfig
with disabled CONFIG_DM :)
With removing the CONFIG_DM ifdefs this series shouldn't break anything
that is already past migration deadline.

Marek

[PATCH u-boot-dm + u-boot-spi v3 10/11] mtd: compare also with OF path and device name in get_mtd_device_nm()

[Tom Rini]

On Thu, Feb 25, 2021 at 09:35:13PM +0100, Marek Behun wrote:
> On Thu, 25 Feb 2021 15:28:56 -0500
> Tom Rini <trini@konsulko.com> wrote:
> 
> > On Thu, Feb 25, 2021 at 09:07:40PM +0100, Marek Behun wrote:
> > > On Thu, 25 Feb 2021 14:31:42 -0500
> > > Simon Glass <sjg@chromium.org> wrote:
> > >   
> > > > We should not need CONFIG_DM here...it should be enabled for all
> > > > boards. You can always disable MTD for a board if not, or send a
> > > > removable patch.
> > > > 
> > > > If for some reason you do, please use if (IS_ENABLED() so that 'dev'
> > > > can always be declared.  
> > > 
> > > Simon, it still isn't enabled for all boards. For example
> > > tqma6s_wru4_mmc_defconfig does not compile with this. I actually wrote
> > > this into commit message:
> > > 
> > >   Although CONFIG_DM is compulsory since v2020.01, there are still some
> > >   boards (for example tqma6s_wru4_mmc_defconfig) that don't enable it. 
> > > 
> > > But if breaking such boards is not a problem anymore, I will gladly
> > > just remove the ifdefs :) Should I?  
> > 
> > So, I'm working hard at dropping boards that are well past migration
> > deadlines.  That specific one fails DM_MMC more importantly, and I will
> > be dropping it if it's not migrated, after v2021.04 is out.  What else
> > fails?  If you rebase your series on my
> > WIP/remove-non-AHCI_LIBATA-drivers (as it has the most board removals in
> > it), what fails to build if your series is just DM only?
> > 
> 
> I haven't tried building all boards, just quickly found a defconfig
> with disabled CONFIG_DM :)
> With removing the CONFIG_DM ifdefs this series shouldn't break anything
> that is already past migration deadline.

Well, past migration deadline doesn't mean it can cause CI to fail.  I'm
just now throwing things out that are 2 years past the deadline, and
we'll probably be doing that for a few releases.

-- 
Tom
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 659 bytes
Desc: not available
URL: <https://lists.denx.de/pipermail/u-boot/attachments/20210225/00ba0c03/attachment.sig>

[PATCH u-boot-dm + u-boot-spi v3 10/11] mtd: compare also with OF path and device name in get_mtd_device_nm()

[Marek Behun]

On Thu, 25 Feb 2021 15:39:09 -0500
Tom Rini <trini@konsulko.com> wrote:

> Well, past migration deadline doesn't mean it can cause CI to fail.  I'm
> just now throwing things out that are 2 years past the deadline, and
> we'll probably be doing that for a few releases.

Tom, I would like to send v4 without the CONFIG_DM #ifdefs.

tqma6s_wru4_mmc_defconfig is the only defconfig that defines CONFIG_DM=n

Since I do not have this board, I cannot work on converting it to DM.

I can either
- add patch removing this board from U-Boot to this patch series
- send patch removing this board as separate from this series and send
  this series only after the board is removed
- stick if #ifdefs
- try to use IS_ENABLED(CONFIG_DM), but this would need some extra
  patches into header files because of undefined references (we really
  don't want this)
- something different

How should I proceed?

Marek

[PATCH u-boot-dm + u-boot-spi v3 10/11] mtd: compare also with OF path and device name in get_mtd_device_nm()

[Tom Rini]

On Mon, Mar 01, 2021 at 03:03:11PM +0100, Marek Behun wrote:

> On Thu, 25 Feb 2021 15:39:09 -0500
> Tom Rini <trini@konsulko.com> wrote:
> 
> > Well, past migration deadline doesn't mean it can cause CI to fail.  I'm
> > just now throwing things out that are 2 years past the deadline, and
> > we'll probably be doing that for a few releases.
> 
> Tom, I would like to send v4 without the CONFIG_DM #ifdefs.
> 
> tqma6s_wru4_mmc_defconfig is the only defconfig that defines CONFIG_DM=n
> 
> Since I do not have this board, I cannot work on converting it to DM.
> 
> I can either
> - add patch removing this board from U-Boot to this patch series
> - send patch removing this board as separate from this series and send
>   this series only after the board is removed
> - stick if #ifdefs
> - try to use IS_ENABLED(CONFIG_DM), but this would need some extra
>   patches into header files because of undefined references (we really
>   don't want this)
> - something different
> 
> How should I proceed?

You can say your series depends on:
https://patchwork.ozlabs.org/project/uboot/patch/20210221010634.21310-42-trini at konsulko.com/
which drops tqma6s_wru4_mmc_defconfig and you should submit a PR against
https://github.com/u-boot/u-boot/ which will trigger a CI run in Azure
so you can see what else fails to build, or if that really is the only
board that would fail to build, in this case.  Thanks!

-- 
Tom
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 659 bytes
Desc: not available
URL: <https://lists.denx.de/pipermail/u-boot/attachments/20210301/fc499183/attachment.sig>

[PATCH u-boot-dm + u-boot-spi v3 11/11] cmd: mtd: expand <name> argument definition in command help

[Marek Behún]

The <name> argument can now also be MTD's DM device name or OF path.
Mention this is command help.

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>
Cc: Patrick Delaunay <patrick.delaunay@st.com>
Cc: Patrice CHOTARD <patrice.chotard@foss.st.com>
Cc: Miquel Raynal <miquel.raynal@bootlin.com>
---
 cmd/mtd.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/cmd/mtd.c b/cmd/mtd.c
index 446cbc1038..728c790fbb 100644
--- a/cmd/mtd.c
+++ b/cmd/mtd.c
@@ -544,7 +544,7 @@ static char mtd_help_text[] =
 	"mtd bad                               <name>\n"
 	"\n"
 	"With:\n"
-	"\t<name>: NAND partition/chip name\n"
+	"\t<name>: NAND partition/chip name (or corresponding DM device name or OF path)\n"
 	"\t<addr>: user address from/to which data will be retrieved/stored\n"
 	"\t<off>: offset in <name> in bytes (default: start of the part)\n"
 	"\t\t* must be block-aligned for erase\n"
-- 
2.26.2

[PATCH u-boot-dm + u-boot-spi v3 00/11] Support SPI NORs and OF partitions in `mtd list`

[Miquel Raynal]

Hi Marek,

Marek Beh?n <marek.behun@nic.cz> wrote on Thu, 25 Feb 2021 15:13:25
+0100:

> Hello,
> 
> this is v3 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.

Very nice contribution, I didn't reviewed in detail every change but
for what I saw:

Reviewed-by: Miquel Raynal <miquel.raynal@bootlin.com>

for the whole series.

Thanks,
Miqu?l

[PATCH u-boot-dm + u-boot-spi v3 00/11] Support SPI NORs and OF partitions in `mtd list`

[Patrice CHOTARD]

Hi All

On 2/25/21 3:13 PM, Marek Beh?n wrote:
> Hello,
> 
> this is v3 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.
> 
> Changes since v2:
> - addressed Pali's comments in patch that adds partition parsing (4/7 in
>   this version): no check for whether the `compatible` property is
>   present in a partition node and added comment explaining mask flags)
> - added 4 more patches:
>   1) adding ofnode_get_path() function
>   2) printing OF path in `mtd list`
>   3) in `mtd read <name> ...`, <name> can now also be DM's device name
>      or OF path
>   4) the fact from 3) is added to `mtd help`
> 
> 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
> 
> 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>
> Cc: Patrick Delaunay <patrick.delaunay@st.com>
> Cc: Patrice CHOTARD <patrice.chotard@foss.st.com>
> Cc: Miquel Raynal <miquel.raynal@bootlin.com>
> 
> Marek Beh?n (11):
>   dm: core: add test for ofnode_get_addr_size_index()
>   dm: core: add non-translating version of ofnode_get_addr_size_index()
>   dm: core: add ofnode_get_path()
>   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()
>   cmd: mtd: print device OF path in listing
>   mtd: compare also with OF path and device name in get_mtd_device_nm()
>   cmd: mtd: expand <name> argument definition in command help
> 
>  cmd/mtd.c                      |  11 ++-
>  drivers/core/ofnode.c          |  44 ++++++++++-
>  drivers/mtd/mtd-uclass.c       |  15 ----
>  drivers/mtd/mtd_uboot.c        | 129 ++++++++++++++++++++-------------
>  drivers/mtd/mtdcore.c          |  29 ++++++++
>  drivers/mtd/mtdpart.c          |  63 ++++++++++++++++
>  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            |  27 +++++++
>  include/linux/mtd/mtd.h        |   9 +++
>  include/mtd.h                  |   1 -
>  test/dm/ofnode.c               |  50 +++++++++++++
>  15 files changed, 334 insertions(+), 75 deletions(-)
> 

Tested-by: Patrice Chotard <patrice.chotard@foss.st.com>
on stm32mp1-ev1 

Thanks
Patrice