[PATCH u-boot-dm + u-boot-spi v4 00/10] Support SPI NORs and OF partitions in `mtd list`
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(-)
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);
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);
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); \
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; }
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
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-sc...
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
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-sc...
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!
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-sc...
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
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;
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);
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; }
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: ");
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)
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"
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:
- adding ofnode_get_path() function
- printing OF path in `mtd list`
- in `mtd read <name> ...`, <name> can now also be DM's device name or OF path
- 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.
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
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.
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
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:
- adding ofnode_get_path() function
- printing OF path in `mtd list`
- in `mtd read <name> ...`, <name> can now also be DM's device name or OF path
- 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
participants (4)
-
Jagan Teki -
Marek Behún -
Masami Hiramatsu -
Tom Rini