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

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

[Marek Behún]

Hello,

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

Small rebase was needed since last version.

Finally passing CI since LTO is now merged and can optimize away the
code increase. CI at https://github.com/u-boot/u-boot/pull/55

Changes since v3:
- rebased against current master (removed first patch, not needed
  anymore)
- check for CONFIG_OF_CONTROL in addition to CONFIG_DM, since we are
  also using ofnode_* functions
- match mtd's name in a separate function to make code more readable.
  Also add non-DM version of this name matching function, since #if
  macro must be used (otherwise CI will fail for configurations with
  disabled DM)
- addressed Simon's comments about using IS_ENABLED instead of #ifdefs
- added Miquel's Reviewed-by and Patrice's Tested-by to the whole series

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 (10):
  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                      |   9 ++-
 drivers/core/ofnode.c          |  44 ++++++++++-
 drivers/mtd/mtd-uclass.c       |  15 ----
 drivers/mtd/mtd_uboot.c        | 129 ++++++++++++++++++++-------------
 drivers/mtd/mtdcore.c          |  35 +++++++++
 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        |  10 +++
 include/mtd.h                  |   1 -
 test/dm/ofnode.c               |  26 +++++++
 15 files changed, 315 insertions(+), 75 deletions(-)

-- 
2.26.3

[PATCH u-boot-dm + u-boot-spi v4 01/10] 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>
Reviewed-by: Miquel Raynal <miquel.raynal@bootlin.com>
Tested-by: Patrice Chotard <patrice.chotard@foss.st.com>
---
 drivers/core/ofnode.c | 19 ++++++++++++++++---
 include/dm/ofnode.h   | 17 +++++++++++++++++
 test/dm/ofnode.c      |  5 +++++
 3 files changed, 38 insertions(+), 3 deletions(-)

diff --git a/drivers/core/ofnode.c b/drivers/core/ofnode.c
index 6c771e364f..dd34cf8ca3 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;
 
@@ -319,7 +320,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) && ns > 0) {
+		if (translate && IS_ENABLED(CONFIG_OF_TRANSLATE) && ns > 0) {
 			return of_translate_address(ofnode_to_np(node), prop_val);
 		} else {
 			na = of_n_addr_cells(ofnode_to_np(node));
@@ -330,12 +331,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 8a69fd87da..e3fccb506e 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 e0b525eeb1..9a69cf70c1 100644
--- a/test/dm/ofnode.c
+++ b/test/dm/ofnode.c
@@ -289,6 +289,11 @@ static int dm_test_ofnode_get_reg(struct unit_test_state *uts)
 	ut_asserteq(FDT_ADDR_T_NONE, addr);
 	ut_asserteq(FDT_SIZE_T_NONE, size);
 
+	node = ofnode_path("/translation-test@8000/noxlatebus@3,300/dev@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_reg, UT_TESTF_SCAN_PDATA | UT_TESTF_SCAN_FDT);
-- 
2.26.3

[PATCH u-boot-dm + u-boot-spi v4 02/10] 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>
Reviewed-by: Simon Glass <sjg@chromium.org>
Reviewed-by: Miquel Raynal <miquel.raynal@bootlin.com>
Tested-by: Patrice Chotard <patrice.chotard@foss.st.com>
---
 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 dd34cf8ca3..eeeccfb446 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 e3fccb506e..3da05d8b21 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 9a69cf70c1..94a4d2189e 100644
--- a/test/dm/ofnode.c
+++ b/test/dm/ofnode.c
@@ -297,3 +297,24 @@ static int dm_test_ofnode_get_reg(struct unit_test_state *uts)
 	return 0;
 }
 DM_TEST(dm_test_ofnode_get_reg, 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@8000/noxlatebus@3,300/dev@42";
+	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.3

[PATCH u-boot-dm + u-boot-spi v4 03/10] 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>
Reviewed-by: Miquel Raynal <miquel.raynal@bootlin.com>
Tested-by: Patrice Chotard <patrice.chotard@foss.st.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>
---
 drivers/mtd/mtd_uboot.c | 106 +++++++++++++++++++++++-----------------
 drivers/mtd/mtdpart.c   |  63 ++++++++++++++++++++++++
 include/linux/mtd/mtd.h |  10 ++++
 3 files changed, 135 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..aa58f722da 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;
 }
 
+#if CONFIG_IS_ENABLED(DM) && CONFIG_IS_ENABLED(OF_CONTROL)
+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 /* CONFIG_IS_ENABLED(DM) && CONFIG_IS_ENABLED(OF_CONTROL) */
+
 #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..3b302fb8c3 100644
--- a/include/linux/mtd/mtd.h
+++ b/include/linux/mtd/mtd.h
@@ -581,6 +581,16 @@ static inline int del_mtd_partitions(struct mtd_info *mtd)
 }
 #endif
 
+#if defined(CONFIG_MTD_PARTITIONS) && CONFIG_IS_ENABLED(DM) && \
+    CONFIG_IS_ENABLED(OF_CONTROL)
+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.3

[PATCH u-boot-dm + u-boot-spi v4 04/10] 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>
Reviewed-by: Miquel Raynal <miquel.raynal@bootlin.com>
Tested-by: Patrice Chotard <patrice.chotard@foss.st.com>
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>
---
 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 786301ba4a..0b63e1bfc2 100644
--- a/drivers/mtd/spi/sf_internal.h
+++ b/drivers/mtd/spi/sf_internal.h
@@ -81,14 +81,14 @@ int spi_flash_cmd_get_sw_write_prot(struct spi_flash *flash);
 
 #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 3befbe91ca..7edb8759fd 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);
@@ -150,8 +150,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.3

[PATCH u-boot-dm + u-boot-spi v4 05/10] 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>
Reviewed-by: Miquel Raynal <miquel.raynal@bootlin.com>
Tested-by: Patrice Chotard <patrice.chotard@foss.st.com>
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>
---
 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 a6625535a7..6af9c675a4 100644
--- a/drivers/mtd/spi/spi-nor-core.c
+++ b/drivers/mtd/spi/spi-nor-core.c
@@ -2535,6 +2535,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 1d5861d55c..b0aa97d324 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.3

[PATCH u-boot-dm + u-boot-spi v4 06/10] 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>
Reviewed-by: Miquel Raynal <miquel.raynal@bootlin.com>
Tested-by: Patrice Chotard <patrice.chotard@foss.st.com>
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>
---
 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.3

[PATCH u-boot-dm + u-boot-spi v4 07/10] 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>
Reviewed-by: Miquel Raynal <miquel.raynal@bootlin.com>
Tested-by: Patrice Chotard <patrice.chotard@foss.st.com>
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>
---
 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.3

[PATCH u-boot-dm + u-boot-spi v4 08/10] 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>
Reviewed-by: Miquel Raynal <miquel.raynal@bootlin.com>
Tested-by: Patrice Chotard <patrice.chotard@foss.st.com>
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>
---
 cmd/mtd.c | 7 +++++++
 1 file changed, 7 insertions(+)

diff --git a/cmd/mtd.c b/cmd/mtd.c
index 485a963bdd..2aabfd4d29 100644
--- a/cmd/mtd.c
+++ b/cmd/mtd.c
@@ -126,6 +126,13 @@ static void mtd_show_device(struct mtd_info *mtd)
 		printf("  - driver: %s\n", mtd->dev->driver->name);
 	}
 #endif
+	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");
+	}
 
 	/* MTD device information */
 	printf("  - type: ");
-- 
2.26.3

[PATCH u-boot-dm + u-boot-spi v4 09/10] 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 '/', try interpreting 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@1" or "/soc/spi@10600/spi-nor@1".

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

Signed-off-by: Marek Behún <marek.behun@nic.cz>
Reviewed-by: Miquel Raynal <miquel.raynal@bootlin.com>
Tested-by: Patrice Chotard <patrice.chotard@foss.st.com>
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>
---
 drivers/mtd/mtdcore.c | 35 +++++++++++++++++++++++++++++++++++
 1 file changed, 35 insertions(+)

---
Changes since v3:
- check for CONFIG_OF_CONTROL in addition to CONFIG_DM, since we are
  also using ofnode_* functions
- match mtd's name in a separate function to make code more readable.
  Also add non-DM version of this name matching function, since #if
  macro must be used (otherwise CI will fail for configurations with
  disabled DM)
- addressed Simon's comments about using IS_ENABLED instead of #ifdefs

diff --git a/drivers/mtd/mtdcore.c b/drivers/mtd/mtdcore.c
index 0d1f94c6cb..582129d0df 100644
--- a/drivers/mtd/mtdcore.c
+++ b/drivers/mtd/mtdcore.c
@@ -768,6 +768,32 @@ int __get_mtd_device(struct mtd_info *mtd)
 }
 EXPORT_SYMBOL_GPL(__get_mtd_device);
 
+#if CONFIG_IS_ENABLED(DM) && CONFIG_IS_ENABLED(OF_CONTROL)
+static bool mtd_device_matches_name(struct mtd_info *mtd, const char *name)
+{
+	struct udevice *dev = NULL;
+	bool is_part;
+
+	/*
+	 * If the first character of mtd name is '/', try interpreting as OF
+	 * path. Otherwise try comparing by mtd->name and mtd->dev->name.
+	 */
+	if (*name == '/')
+		device_get_global_by_ofnode(ofnode_path(name), &dev);
+
+	is_part = mtd_is_partition(mtd);
+
+	return (!is_part && dev && mtd->dev == dev) ||
+	       !strcmp(name, mtd->name) ||
+	       (is_part && mtd->dev && !strcmp(name, mtd->dev->name));
+}
+#else
+static bool mtd_device_matches_name(struct mtd_info *mtd, const char *name)
+{
+	return !strcmp(name, mtd->name);
+}
+#endif
+
 /**
  *	get_mtd_device_nm - obtain a validated handle for an MTD device by
  *	device name
@@ -784,10 +810,19 @@ struct mtd_info *get_mtd_device_nm(const char *name)
 	mutex_lock(&mtd_table_mutex);
 
 	mtd_for_each_device(other) {
+#ifdef __UBOOT__
+		if (mtd_device_matches_name(other, name)) {
+			if (mtd)
+				printf("\nWarning: MTD name \"%s\" is not unique!\n\n",
+				       name);
+			mtd = other;
+		}
+#else /* !__UBOOT__ */
 		if (!strcmp(name, other->name)) {
 			mtd = other;
 			break;
 		}
+#endif /* !__UBOOT__ */
 	}
 
 	if (!mtd)
-- 
2.26.3

[PATCH u-boot-dm + u-boot-spi v4 10/10] 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>
Reviewed-by: Miquel Raynal <miquel.raynal@bootlin.com>
Tested-by: Patrice Chotard <patrice.chotard@foss.st.com>
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>
---
 cmd/mtd.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/cmd/mtd.c b/cmd/mtd.c
index 2aabfd4d29..c22478c152 100644
--- a/cmd/mtd.c
+++ b/cmd/mtd.c
@@ -542,7 +542,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.3

Re: [PATCH u-boot-dm + u-boot-spi v4 00/10] Support SPI NORs and OF partitions in `mtd list`

[Jagan Teki]

On Wed, May 26, 2021 at 5:39 PM Marek Behún <marek.behun@nic.cz> wrote:
>
> Hello,
>
> this is v4 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.
>
> Small rebase was needed since last version.
>
> Finally passing CI since LTO is now merged and can optimize away the
> code increase. CI at https://github.com/u-boot/u-boot/pull/55
>
> Changes since v3:
> - rebased against current master (removed first patch, not needed
>   anymore)
> - check for CONFIG_OF_CONTROL in addition to CONFIG_DM, since we are
>   also using ofnode_* functions
> - match mtd's name in a separate function to make code more readable.
>   Also add non-DM version of this name matching function, since #if
>   macro must be used (otherwise CI will fail for configurations with
>   disabled DM)
> - addressed Simon's comments about using IS_ENABLED instead of #ifdefs
> - added Miquel's Reviewed-by and Patrice's Tested-by to the whole series
>
> 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 (10):
>   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                      |   9 ++-
>  drivers/core/ofnode.c          |  44 ++++++++++-
>  drivers/mtd/mtd-uclass.c       |  15 ----
>  drivers/mtd/mtd_uboot.c        | 129 ++++++++++++++++++++-------------
>  drivers/mtd/mtdcore.c          |  35 +++++++++
>  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        |  10 +++
>  include/mtd.h                  |   1 -
>  test/dm/ofnode.c               |  26 +++++++
>  15 files changed, 315 insertions(+), 75 deletions(-)

This series have some conflicts wrt my series about MTD UCLASS
migration. Does this bypass that series?

Jagan.

Re: [PATCH u-boot-dm + u-boot-spi v4 00/10] Support SPI NORs and OF partitions in `mtd list`

[Marek Behún]

On Wed, 26 May 2021 22:28:34 +0530
Jagan Teki <jagan@amarulasolutions.com> wrote:

> This series have some conflicts wrt my series about MTD UCLASS
> migration. Does this bypass that series?
> 
> Jagan.

Jagan, I was working on top of Tom's master branch... Are the conflicts
big?

Marek

Re: [PATCH u-boot-dm + u-boot-spi v4 00/10] Support SPI NORs and OF partitions in `mtd list`

[Jagan Teki]

On Wed, May 26, 2021 at 11:25 PM Marek Behún <marek.behun@nic.cz> wrote:
>
> On Wed, 26 May 2021 22:28:34 +0530
> Jagan Teki <jagan@amarulasolutions.com> wrote:
>
> > This series have some conflicts wrt my series about MTD UCLASS
> > migration. Does this bypass that series?
> >
> > Jagan.
>
> Jagan, I was working on top of Tom's master branch... Are the conflicts
> big?

Not on master, but master with my MTD UCLASS changes in mailing list.
Let me figure it out. I will update it.

Jagan.

Re: [PATCH u-boot-dm + u-boot-spi v4 00/10] Support SPI NORs and OF partitions in `mtd list`

[Marek Behún]

On Wed, 26 May 2021 23:34:18 +0530
Jagan Teki <jagan@amarulasolutions.com> wrote:

> On Wed, May 26, 2021 at 11:25 PM Marek Behún <marek.behun@nic.cz>
> wrote:
> >
> > On Wed, 26 May 2021 22:28:34 +0530
> > Jagan Teki <jagan@amarulasolutions.com> wrote:
> >  
> > > This series have some conflicts wrt my series about MTD UCLASS
> > > migration. Does this bypass that series?
> > >
> > > Jagan.  
> >
> > Jagan, I was working on top of Tom's master branch... Are the
> > conflicts big?  
> 
> Not on master, but master with my MTD UCLASS changes in mailing list.
> Let me figure it out. I will update it.
> 
> Jagan.

Jagan, any news about this series?
Can you review this? Who whould be applying this?
Thanks.

Marek

Re: [PATCH u-boot-dm + u-boot-spi v4 00/10] Support SPI NORs and OF partitions in `mtd list`

[Jagan Teki]

On Wed, May 26, 2021 at 5:39 PM Marek Behún <marek.behun@nic.cz> wrote:
>
> Hello,
>
> this is v4 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.
>
> Small rebase was needed since last version.
>
> Finally passing CI since LTO is now merged and can optimize away the
> code increase. CI at https://github.com/u-boot/u-boot/pull/55
>
> Changes since v3:
> - rebased against current master (removed first patch, not needed
>   anymore)
> - check for CONFIG_OF_CONTROL in addition to CONFIG_DM, since we are
>   also using ofnode_* functions
> - match mtd's name in a separate function to make code more readable.
>   Also add non-DM version of this name matching function, since #if
>   macro must be used (otherwise CI will fail for configurations with
>   disabled DM)
> - addressed Simon's comments about using IS_ENABLED instead of #ifdefs
> - added Miquel's Reviewed-by and Patrice's Tested-by to the whole series
>
> 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 (10):
>   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

Applied to u-boot-spi/master

Re: [PATCH u-boot-dm + u-boot-spi v4 04/10] mtd: spi-nor: allow registering multiple MTDs when DM is enabled

[Masami Hiramatsu]

Hi Marek,

I found that this changes the mtd device name and makes 'mtdparts'
doesn't work on my developerbox platform.

Before this change,
-------
=> sf probe
SF: Detected mx66u51235f with page size 256 Bytes, erase size 4 KiB,
total 64 MiB
=> mtd list
List of MTD devices:
* nor1
  - type: NOR flash
  - block size: 0x1000 bytes
  - min I/O: 0x1 bytes
  - 0x000000000000-0x000004000000 : "nor1"
          - 0x000000000000-0x000000070000 : "BootStrap-BL1"
          - 0x000000070000-0x000000100000 : "Flash-Writer"
          - 0x000000100000-0x000000180000 : "SCP-BL2"
          - 0x000000180000-0x0000001f8000 : "FIP-TFA"
          - 0x0000001f8000-0x000000200000 : "Stg2-Tables"
          - 0x000000200000-0x000000400000 : "EDK2"
          - 0x000000400000-0x000000500000 : "UEFI-Vars"
          - 0x000000500000-0x000000700000 : "OPTEE"
          - 0x000000700000-0x000000800000 : "UBoot-Env"
          - 0x000000800000-0x000000900000 : "U-Boot"
          - 0x000000900000-0x000004000000 : "Free"
=>
-------
after this change,
-------
=> sf probe
SF: Detected mx66u51235f with page size 256 Bytes, erase size 4 KiB,
total 64 MiB
=> mtd list
Could not find a valid device for nor1
List of MTD devices:
* mx66u51235f
  - device: spi-flash@0
  - parent: spi@54800000
  - driver: jedec_spi_nor
  - path: /spi@54800000/spi-flash@0
  - type: NOR flash
  - block size: 0x1000 bytes
  - min I/O: 0x1 bytes
  - 0x000000000000-0x000004000000 : "mx66u51235f"
-------

I think I should update CONFIG_MTDIDS_DEFAULT and CONFIG_MTDPARTS_DEFAULT.
But before that, I would like to confirm that this is an intended
change, and what should I do. (replace nor1 with mx66u51235f ?)

Thank you,

2021年5月26日(水) 21:10 Marek Behún <marek.behun@nic.cz>:

>
> 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>
> Reviewed-by: Miquel Raynal <miquel.raynal@bootlin.com>
> Tested-by: Patrice Chotard <patrice.chotard@foss.st.com>
> 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>
> ---
>  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 786301ba4a..0b63e1bfc2 100644
> --- a/drivers/mtd/spi/sf_internal.h
> +++ b/drivers/mtd/spi/sf_internal.h
> @@ -81,14 +81,14 @@ int spi_flash_cmd_get_sw_write_prot(struct spi_flash *flash);
>
>  #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 3befbe91ca..7edb8759fd 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);
> @@ -150,8 +150,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.3
>


--
Masami Hiramatsu

Re: [PATCH u-boot-dm + u-boot-spi v4 04/10] mtd: spi-nor: allow registering multiple MTDs when DM is enabled

[Marek Behún]

On Thu, 8 Jul 2021 08:54:51 +0900
Masami Hiramatsu <masami.hiramatsu@linaro.org> wrote:

> Hi Marek,
> 
> I found that this changes the mtd device name and makes 'mtdparts'
> doesn't work on my developerbox platform.
> 
> Before this change,
> -------
> => sf probe  
> SF: Detected mx66u51235f with page size 256 Bytes, erase size 4 KiB,
> total 64 MiB
> => mtd list  
> List of MTD devices:
> * nor1
>   - type: NOR flash
>   - block size: 0x1000 bytes
>   - min I/O: 0x1 bytes
>   - 0x000000000000-0x000004000000 : "nor1"
>           - 0x000000000000-0x000000070000 : "BootStrap-BL1"
>           - 0x000000070000-0x000000100000 : "Flash-Writer"
>           - 0x000000100000-0x000000180000 : "SCP-BL2"
>           - 0x000000180000-0x0000001f8000 : "FIP-TFA"
>           - 0x0000001f8000-0x000000200000 : "Stg2-Tables"
>           - 0x000000200000-0x000000400000 : "EDK2"
>           - 0x000000400000-0x000000500000 : "UEFI-Vars"
>           - 0x000000500000-0x000000700000 : "OPTEE"
>           - 0x000000700000-0x000000800000 : "UBoot-Env"
>           - 0x000000800000-0x000000900000 : "U-Boot"
>           - 0x000000900000-0x000004000000 : "Free"
> =>  
> -------
> after this change,
> -------
> => sf probe  
> SF: Detected mx66u51235f with page size 256 Bytes, erase size 4 KiB,
> total 64 MiB
> => mtd list  
> Could not find a valid device for nor1
> List of MTD devices:
> * mx66u51235f
>   - device: spi-flash@0
>   - parent: spi@54800000
>   - driver: jedec_spi_nor
>   - path: /spi@54800000/spi-flash@0
>   - type: NOR flash
>   - block size: 0x1000 bytes
>   - min I/O: 0x1 bytes
>   - 0x000000000000-0x000004000000 : "mx66u51235f"
> -------
> 
> I think I should update CONFIG_MTDIDS_DEFAULT and
> CONFIG_MTDPARTS_DEFAULT. But before that, I would like to confirm
> that this is an intended change, and what should I do. (replace nor1
> with mx66u51235f ?)

Hi Masami,

no. The intended solution here for you is to remove MTDIDS / MTDPARTS
completely and instead define the partitions in your device tree:
  https://source.denx.de/u-boot/u-boot/-/blob/master/arch/arm/dts/synquacer-sc2a11-developerbox-u-boot.dtsi

You should add something like this into the spi-flash@0 node:

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

    partition@0 {
      label = "BootStrap-BL1";
      reg = <0x0 0x70000>;
    };

    partition@70000 {
      label = "Flash-Writer";
      reg = <0x70000 0x90000>;
    };

    partition@100000 {
      label = "SCP-BL2";
      reg = <0x100000 0x80000>;
    };

    ...
  };

I wonder though now whether we should force other boards to do this or
whether we should fix the code to be backwards compatible with the old
names.

Tom, Miquel, Jagan, what do you think?

Marek

Re: [PATCH u-boot-dm + u-boot-spi v4 04/10] mtd: spi-nor: allow registering multiple MTDs when DM is enabled

[Tom Rini]

[-- Attachment #1: Type: text/plain, Size: 3431 bytes --]

On Thu, Jul 08, 2021 at 04:15:17PM +0200, Marek Behún wrote:
> On Thu, 8 Jul 2021 08:54:51 +0900
> Masami Hiramatsu <masami.hiramatsu@linaro.org> wrote:
> 
> > Hi Marek,
> > 
> > I found that this changes the mtd device name and makes 'mtdparts'
> > doesn't work on my developerbox platform.
> > 
> > Before this change,
> > -------
> > => sf probe  
> > SF: Detected mx66u51235f with page size 256 Bytes, erase size 4 KiB,
> > total 64 MiB
> > => mtd list  
> > List of MTD devices:
> > * nor1
> >   - type: NOR flash
> >   - block size: 0x1000 bytes
> >   - min I/O: 0x1 bytes
> >   - 0x000000000000-0x000004000000 : "nor1"
> >           - 0x000000000000-0x000000070000 : "BootStrap-BL1"
> >           - 0x000000070000-0x000000100000 : "Flash-Writer"
> >           - 0x000000100000-0x000000180000 : "SCP-BL2"
> >           - 0x000000180000-0x0000001f8000 : "FIP-TFA"
> >           - 0x0000001f8000-0x000000200000 : "Stg2-Tables"
> >           - 0x000000200000-0x000000400000 : "EDK2"
> >           - 0x000000400000-0x000000500000 : "UEFI-Vars"
> >           - 0x000000500000-0x000000700000 : "OPTEE"
> >           - 0x000000700000-0x000000800000 : "UBoot-Env"
> >           - 0x000000800000-0x000000900000 : "U-Boot"
> >           - 0x000000900000-0x000004000000 : "Free"
> > =>  
> > -------
> > after this change,
> > -------
> > => sf probe  
> > SF: Detected mx66u51235f with page size 256 Bytes, erase size 4 KiB,
> > total 64 MiB
> > => mtd list  
> > Could not find a valid device for nor1
> > List of MTD devices:
> > * mx66u51235f
> >   - device: spi-flash@0
> >   - parent: spi@54800000
> >   - driver: jedec_spi_nor
> >   - path: /spi@54800000/spi-flash@0
> >   - type: NOR flash
> >   - block size: 0x1000 bytes
> >   - min I/O: 0x1 bytes
> >   - 0x000000000000-0x000004000000 : "mx66u51235f"
> > -------
> > 
> > I think I should update CONFIG_MTDIDS_DEFAULT and
> > CONFIG_MTDPARTS_DEFAULT. But before that, I would like to confirm
> > that this is an intended change, and what should I do. (replace nor1
> > with mx66u51235f ?)
> 
> Hi Masami,
> 
> no. The intended solution here for you is to remove MTDIDS / MTDPARTS
> completely and instead define the partitions in your device tree:
>   https://source.denx.de/u-boot/u-boot/-/blob/master/arch/arm/dts/synquacer-sc2a11-developerbox-u-boot.dtsi
> 
> You should add something like this into the spi-flash@0 node:
> 
>   partitions {
>     compatible = "fixed-partitions";
>     #address-cells = <1>;
>     #size-cells = <1>;
> 
>     partition@0 {
>       label = "BootStrap-BL1";
>       reg = <0x0 0x70000>;
>     };
> 
>     partition@70000 {
>       label = "Flash-Writer";
>       reg = <0x70000 0x90000>;
>     };
> 
>     partition@100000 {
>       label = "SCP-BL2";
>       reg = <0x100000 0x80000>;
>     };
> 
>     ...
>   };
> 
> I wonder though now whether we should force other boards to do this or
> whether we should fix the code to be backwards compatible with the old
> names.
> 
> Tom, Miquel, Jagan, what do you think?

I think we need for passing mtdparts/mtdids via the kernel command line
to continue to work like they used it.  And some checkpatch magic to
catch people introducing new ones?  It certainly would be best for
people to define them in dts as allowed, but there's a good size
existing base that doesn't.  Thanks!

-- 
Tom

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 659 bytes --]

Re: [PATCH u-boot-dm + u-boot-spi v4 04/10] mtd: spi-nor: allow registering multiple MTDs when DM is enabled

[Masami Hiramatsu]

Hi Marek,

2021年7月8日(木) 23:15 Marek Behún <marek.behun@nic.cz>:
>
> On Thu, 8 Jul 2021 08:54:51 +0900
> Masami Hiramatsu <masami.hiramatsu@linaro.org> wrote:
>
> > Hi Marek,
> >
> > I found that this changes the mtd device name and makes 'mtdparts'
> > doesn't work on my developerbox platform.
> >
> > Before this change,
> > -------
> > => sf probe
> > SF: Detected mx66u51235f with page size 256 Bytes, erase size 4 KiB,
> > total 64 MiB
> > => mtd list
> > List of MTD devices:
> > * nor1
> >   - type: NOR flash
> >   - block size: 0x1000 bytes
> >   - min I/O: 0x1 bytes
> >   - 0x000000000000-0x000004000000 : "nor1"
> >           - 0x000000000000-0x000000070000 : "BootStrap-BL1"
> >           - 0x000000070000-0x000000100000 : "Flash-Writer"
> >           - 0x000000100000-0x000000180000 : "SCP-BL2"
> >           - 0x000000180000-0x0000001f8000 : "FIP-TFA"
> >           - 0x0000001f8000-0x000000200000 : "Stg2-Tables"
> >           - 0x000000200000-0x000000400000 : "EDK2"
> >           - 0x000000400000-0x000000500000 : "UEFI-Vars"
> >           - 0x000000500000-0x000000700000 : "OPTEE"
> >           - 0x000000700000-0x000000800000 : "UBoot-Env"
> >           - 0x000000800000-0x000000900000 : "U-Boot"
> >           - 0x000000900000-0x000004000000 : "Free"
> > =>
> > -------
> > after this change,
> > -------
> > => sf probe
> > SF: Detected mx66u51235f with page size 256 Bytes, erase size 4 KiB,
> > total 64 MiB
> > => mtd list
> > Could not find a valid device for nor1
> > List of MTD devices:
> > * mx66u51235f
> >   - device: spi-flash@0
> >   - parent: spi@54800000
> >   - driver: jedec_spi_nor
> >   - path: /spi@54800000/spi-flash@0
> >   - type: NOR flash
> >   - block size: 0x1000 bytes
> >   - min I/O: 0x1 bytes
> >   - 0x000000000000-0x000004000000 : "mx66u51235f"
> > -------
> >
> > I think I should update CONFIG_MTDIDS_DEFAULT and
> > CONFIG_MTDPARTS_DEFAULT. But before that, I would like to confirm
> > that this is an intended change, and what should I do. (replace nor1
> > with mx66u51235f ?)
>
> Hi Masami,
>
> no. The intended solution here for you is to remove MTDIDS / MTDPARTS
> completely and instead define the partitions in your device tree:
>   https://source.denx.de/u-boot/u-boot/-/blob/master/arch/arm/dts/synquacer-sc2a11-developerbox-u-boot.dtsi
>
> You should add something like this into the spi-flash@0 node:
>
>   partitions {
>     compatible = "fixed-partitions";
>     #address-cells = <1>;
>     #size-cells = <1>;
>
>     partition@0 {
>       label = "BootStrap-BL1";
>       reg = <0x0 0x70000>;
>     };
>
>     partition@70000 {
>       label = "Flash-Writer";
>       reg = <0x70000 0x90000>;
>     };
>
>     partition@100000 {
>       label = "SCP-BL2";
>       reg = <0x100000 0x80000>;
>     };
>
>     ...
>   };

OK, I'll then update the dtb according to your suggestion.

Thank you!

>
> I wonder though now whether we should force other boards to do this or
> whether we should fix the code to be backwards compatible with the old
> names.
>
> Tom, Miquel, Jagan, what do you think?
>
> Marek



-- 
Masami Hiramatsu