[PATCH v2 1/4] of: add helpers to get alias from device node path + property name

[PATCH v2 1/4] of: add helpers to get alias from device node path + property name

[Ahmad Fatoum]

The existing of_find_device and of_find_path helpers require that
barebox has already probed devices. For use in code where this
assumption doesn't always hold true, let's add helpers that return
the alias if it exists.

Signed-off-by: Ahmad Fatoum <a.fatoum@pengutronix.de>
---
v1 -> v2:
  - fix check for wring variable: rnode = ...; if (!node)
    to if (!rnode)
  - look up property in node at np_name, not in root node
---
 drivers/of/base.c | 57 +++++++++++++++++++++++++++++++++++++++++++++++
 include/of.h      | 32 ++++++++++++++++++++++++++
 2 files changed, 89 insertions(+)

diff --git a/drivers/of/base.c b/drivers/of/base.c
index 960a9327aed2..cf850d4bf789 100644
--- a/drivers/of/base.c
+++ b/drivers/of/base.c
@@ -318,6 +318,63 @@ const char *of_alias_get(struct device_node *np)
 }
 EXPORT_SYMBOL_GPL(of_alias_get);
 
+static const char *of_get_partition_device_alias(struct device_node *np)
+{
+	const char *alias;
+
+	alias = of_alias_get(np);
+	if (alias)
+		return alias;
+
+	np = of_get_parent(np);
+	if (np && of_device_is_compatible(np, "fixed-partitions"))
+		np = of_get_parent(np);
+
+	return of_alias_get(np);
+}
+
+const char *of_property_get_alias_from(struct device_node *root,
+				       const char *np_name, const char *propname,
+				       int index)
+{
+	struct device_node *node, *rnode;
+	const char *path;
+	int ret;
+
+	node = of_find_node_by_path_or_alias(root, np_name);
+	if (!node)
+		return NULL;
+
+	ret = of_property_read_string_index(node, propname, index, &path);
+	if (ret < 0)
+		return NULL;
+
+	rnode = of_find_node_by_path(path);
+	if (!rnode)
+		return NULL;
+
+	return of_get_partition_device_alias(rnode);
+}
+EXPORT_SYMBOL_GPL(of_property_get_alias_from);
+
+const char *of_parse_phandle_and_get_alias_from(struct device_node *root,
+						const char *np_name, const char *phandle_name,
+						int index)
+{
+	struct device_node *node, *rnode;
+
+	node = of_find_node_by_path_or_alias(root, np_name);
+	if (!node)
+		return NULL;
+
+	rnode = of_parse_phandle_from(node, root, phandle_name, index);
+	if (!rnode)
+		return NULL;
+
+	return of_get_partition_device_alias(rnode);
+}
+EXPORT_SYMBOL_GPL(of_parse_phandle_and_get_alias_from);
+
 /*
  * of_find_node_by_alias - Find a node given an alias name
  * @root:    the root node of the tree. If NULL, use internal tree
diff --git a/include/of.h b/include/of.h
index 3f5e5f9b04bb..e93b1bbf2f0a 100644
--- a/include/of.h
+++ b/include/of.h
@@ -314,6 +314,14 @@ extern int of_alias_get_id_from(struct device_node *root, struct device_node *np
 extern const char *of_alias_get(struct device_node *np);
 extern int of_modalias_node(struct device_node *node, char *modalias, int len);
 
+extern const char *of_property_get_alias_from(struct device_node *root,
+					      const char *np_name, const char *propname,
+					      int index);
+
+extern const char *of_parse_phandle_and_get_alias_from(struct device_node *root,
+						       const char *np_name, const char *phandle_name,
+						       int index);
+
 extern struct device_node *of_get_root_node(void);
 extern int of_set_root_node(struct device_node *node);
 extern int barebox_register_of(struct device_node *root);
@@ -944,6 +952,20 @@ static inline int of_modalias_node(struct device_node *node, char *modalias,
 	return -ENOSYS;
 }
 
+static inline const char *of_property_get_alias_from(struct device_node *root,
+						     const char *np_name, const char *propname,
+						     int index)
+{
+	return NULL;
+}
+
+static inline const char *of_parse_phandle_and_get_alias_from(struct device_node *root,
+							      const char *np_name, const char *phandle_name,
+							      int index)
+{
+	return NULL;
+}
+
 static inline int of_platform_populate(struct device_node *root,
 				const struct of_device_id *matches,
 				struct device *parent)
@@ -1275,6 +1297,16 @@ static inline void of_delete_property_by_name(struct device_node *np, const char
 	of_delete_property(of_find_property(np, name, NULL));
 }
 
+static inline const char *of_property_get_alias(const char *np_name, const char *propname)
+{
+	return of_property_get_alias_from(NULL, np_name, propname, 0);
+}
+
+static inline const char *of_parse_phandle_and_get_alias(const char *np_name, const char *phandle_name)
+{
+	return of_parse_phandle_and_get_alias_from(NULL, np_name, phandle_name, 0);
+}
+
 extern const struct of_device_id of_default_bus_match_table[];
 
 int of_device_enable(struct device_node *node);
-- 
2.39.5

[PATCH v2 2/4] environment: implement of_env_get_alias_by_path helper

[Ahmad Fatoum]

For use by board-code that wants to get the alias pointing at the
backing device of an environment node add a simple
of_env_get_alias_by_path() helper, so board code doesn't need to worry
about property name and whether the binding uses a phandle or string path.

Signed-off-by: Ahmad Fatoum <a.fatoum@pengutronix.de>
---
v1 -> v2:
  - no change
---
 drivers/of/barebox.c |  5 +++++
 include/envfs.h      | 10 ++++++++++
 2 files changed, 15 insertions(+)

diff --git a/drivers/of/barebox.c b/drivers/of/barebox.c
index ed5d171e43a8..7d433e280ba5 100644
--- a/drivers/of/barebox.c
+++ b/drivers/of/barebox.c
@@ -15,6 +15,11 @@
 
 #define ENV_MNT_DIR "/boot"	/* If env on filesystem, where to mount */
 
+const char *of_env_get_alias_by_path(const char *of_path)
+{
+	return of_property_get_alias(of_path, "device-path");
+}
+
 /* If dev describes a file on a fs, mount the fs and return a pointer
  * to the file's path.  Otherwise return an error code or NULL if the
  * device path should be used.
diff --git a/include/envfs.h b/include/envfs.h
index 767b34c943de..8bfc83b6b008 100644
--- a/include/envfs.h
+++ b/include/envfs.h
@@ -118,6 +118,16 @@ static inline const char *default_environment_path_get(void)
 }
 #endif
 
+#ifdef CONFIG_OF_BAREBOX_DRIVERS
+const char *of_env_get_alias_by_path(const char *of_path);
+#else
+static inline const char *of_env_get_alias_by_path(const char *of_path)
+{
+	return NULL;
+}
+#endif
+
+
 #ifdef CONFIG_DEFAULT_ENVIRONMENT
 void defaultenv_append(void *buf, unsigned int size, const char *name);
 int defaultenv_load(const char *dir, unsigned flags);
-- 
2.39.5

Re: [PATCH v2 2/4] environment: implement of_env_get_alias_by_path helper

[Ahmad Fatoum]

On 15.11.24 13:16, Ahmad Fatoum wrote:
> For use by board-code that wants to get the alias pointing at the
> backing device of an environment node add a simple
> of_env_get_alias_by_path() helper, so board code doesn't need to worry
> about property name and whether the binding uses a phandle or string path.
> 
> Signed-off-by: Ahmad Fatoum <a.fatoum@pengutronix.de>

Thinking about it, of_env_get_device_alias_by_path may be a bit verbose,
but is a better name.

I'll wait for more feedback before sending v3 though.

Cheers,
Ahmad

> ---
> v1 -> v2:
>   - no change
> ---
>  drivers/of/barebox.c |  5 +++++
>  include/envfs.h      | 10 ++++++++++
>  2 files changed, 15 insertions(+)
> 
> diff --git a/drivers/of/barebox.c b/drivers/of/barebox.c
> index ed5d171e43a8..7d433e280ba5 100644
> --- a/drivers/of/barebox.c
> +++ b/drivers/of/barebox.c
> @@ -15,6 +15,11 @@
>  
>  #define ENV_MNT_DIR "/boot"	/* If env on filesystem, where to mount */
>  
> +const char *of_env_get_alias_by_path(const char *of_path)
> +{
> +	return of_property_get_alias(of_path, "device-path");
> +}
> +
>  /* If dev describes a file on a fs, mount the fs and return a pointer
>   * to the file's path.  Otherwise return an error code or NULL if the
>   * device path should be used.
> diff --git a/include/envfs.h b/include/envfs.h
> index 767b34c943de..8bfc83b6b008 100644
> --- a/include/envfs.h
> +++ b/include/envfs.h
> @@ -118,6 +118,16 @@ static inline const char *default_environment_path_get(void)
>  }
>  #endif
>  
> +#ifdef CONFIG_OF_BAREBOX_DRIVERS
> +const char *of_env_get_alias_by_path(const char *of_path);
> +#else
> +static inline const char *of_env_get_alias_by_path(const char *of_path)
> +{
> +	return NULL;
> +}
> +#endif
> +
> +
>  #ifdef CONFIG_DEFAULT_ENVIRONMENT
>  void defaultenv_append(void *buf, unsigned int size, const char *name);
>  int defaultenv_load(const char *dir, unsigned flags);


-- 
Pengutronix e.K.                           |                             |
Steuerwalder Str. 21                       | http://www.pengutronix.de/  |
31137 Hildesheim, Germany                  | Phone: +49-5121-206917-0    |
Amtsgericht Hildesheim, HRA 2686           | Fax:   +49-5121-206917-5555 |

Re: [PATCH v2 2/4] environment: implement of_env_get_alias_by_path helper

[Sascha Hauer]

On Fri, Nov 15, 2024 at 01:38:03PM +0100, Ahmad Fatoum wrote:
> On 15.11.24 13:16, Ahmad Fatoum wrote:
> > For use by board-code that wants to get the alias pointing at the
> > backing device of an environment node add a simple
> > of_env_get_alias_by_path() helper, so board code doesn't need to worry
> > about property name and whether the binding uses a phandle or string path.
> > 
> > Signed-off-by: Ahmad Fatoum <a.fatoum@pengutronix.de>
> 
> Thinking about it, of_env_get_device_alias_by_path may be a bit verbose,
> but is a better name.
> 
> I'll wait for more feedback before sending v3 though.

LGTM otherwise.

Sascha

> 
> Cheers,
> Ahmad
> 
> > ---
> > v1 -> v2:
> >   - no change
> > ---
> >  drivers/of/barebox.c |  5 +++++
> >  include/envfs.h      | 10 ++++++++++
> >  2 files changed, 15 insertions(+)
> > 
> > diff --git a/drivers/of/barebox.c b/drivers/of/barebox.c
> > index ed5d171e43a8..7d433e280ba5 100644
> > --- a/drivers/of/barebox.c
> > +++ b/drivers/of/barebox.c
> > @@ -15,6 +15,11 @@
> >  
> >  #define ENV_MNT_DIR "/boot"	/* If env on filesystem, where to mount */
> >  
> > +const char *of_env_get_alias_by_path(const char *of_path)
> > +{
> > +	return of_property_get_alias(of_path, "device-path");
> > +}
> > +
> >  /* If dev describes a file on a fs, mount the fs and return a pointer
> >   * to the file's path.  Otherwise return an error code or NULL if the
> >   * device path should be used.
> > diff --git a/include/envfs.h b/include/envfs.h
> > index 767b34c943de..8bfc83b6b008 100644
> > --- a/include/envfs.h
> > +++ b/include/envfs.h
> > @@ -118,6 +118,16 @@ static inline const char *default_environment_path_get(void)
> >  }
> >  #endif
> >  
> > +#ifdef CONFIG_OF_BAREBOX_DRIVERS
> > +const char *of_env_get_alias_by_path(const char *of_path);
> > +#else
> > +static inline const char *of_env_get_alias_by_path(const char *of_path)
> > +{
> > +	return NULL;
> > +}
> > +#endif
> > +
> > +
> >  #ifdef CONFIG_DEFAULT_ENVIRONMENT
> >  void defaultenv_append(void *buf, unsigned int size, const char *name);
> >  int defaultenv_load(const char *dir, unsigned flags);
> 
> 
> -- 
> Pengutronix e.K.                           |                             |
> Steuerwalder Str. 21                       | http://www.pengutronix.de/  |
> 31137 Hildesheim, Germany                  | Phone: +49-5121-206917-0    |
> Amtsgericht Hildesheim, HRA 2686           | Fax:   +49-5121-206917-5555 |
> 
> 

-- 
Pengutronix e.K.                           |                             |
Steuerwalder Str. 21                       | http://www.pengutronix.de/  |
31137 Hildesheim, Germany                  | Phone: +49-5121-206917-0    |
Amtsgericht Hildesheim, HRA 2686           | Fax:   +49-5121-206917-5555 |

[PATCH v2 3/4] bootsource: have bootsource_get_alias_name return const char *

[Ahmad Fatoum]

The only user of bootsource_get_alias_name() calls it and frees it
shortly after and users in board code will likely do the same.

Instead of allocating every time, let's just return a static array and
expect users to call strdup() if they want to keep the bootsource valid
for longer.

Signed-off-by: Ahmad Fatoum <a.fatoum@pengutronix.de>
---
v1 -> v2:
  - no change
---
 common/bootsource.c  | 23 +++++++++++------------
 include/bootsource.h |  2 +-
 2 files changed, 12 insertions(+), 13 deletions(-)

diff --git a/common/bootsource.c b/common/bootsource.c
index 6808c9c51d88..3c8f53f3a890 100644
--- a/common/bootsource.c
+++ b/common/bootsource.c
@@ -72,27 +72,24 @@ const char *bootsource_get_alias_stem(enum bootsource src)
 /**
  * bootsource_get_alias_name() - Get the name of the bootsource alias
  *
- * This function will return newly allocated string containing name of
+ * This function will return a pointer to a static string containing name of
  * the alias that is expected to point to DTB node corresponding to
  * detected bootsource
  *
- * NOTE: Caller is expected to free() the string allocated by this
- * function
  */
-char *bootsource_get_alias_name(void)
+const char *bootsource_get_alias_name(void)
 {
+	static char buf[sizeof("i2c-eeprom-2147483647")];
 	const char *stem;
+	int ret;
 
 	/*
 	 * If alias name was overridden via
 	 * bootsource_set_alias_name() return that value without
 	 * asking any questions.
-	 *
-	 * Note that we have to strdup() the result to make it
-	 * free-able.
 	 */
 	if (bootsource_alias_name)
-		return strdup(bootsource_alias_name);
+		return bootsource_alias_name;
 
 	stem = bootsource_get_alias_stem(bootsource);
 	if (!stem)
@@ -105,13 +102,17 @@ char *bootsource_get_alias_name(void)
 	if (bootsource_instance == BOOTSOURCE_INSTANCE_UNKNOWN)
 		return NULL;
 
-	return basprintf("%s%d", stem, bootsource_instance);
+	ret = snprintf(buf, sizeof(buf), "%s%d", stem, bootsource_instance);
+	if (ret < 0 || ret >= sizeof(buf))
+		return NULL;
+
+	return buf;
 }
 
 struct device_node *bootsource_of_node_get(struct device_node *root)
 {
 	struct device_node *np;
-	char *alias_name;
+	const char *alias_name;
 
 	alias_name = bootsource_get_alias_name();
 	if (!alias_name)
@@ -119,8 +120,6 @@ struct device_node *bootsource_of_node_get(struct device_node *root)
 
 	np = of_find_node_by_alias(root, alias_name);
 
-	free(alias_name);
-
 	return np;
 }
 
diff --git a/include/bootsource.h b/include/bootsource.h
index 33ad26946059..9fb77a0bb0a9 100644
--- a/include/bootsource.h
+++ b/include/bootsource.h
@@ -29,7 +29,7 @@ enum bootsource bootsource_get(void);
 enum bootsource bootsource_get_device(void);
 int bootsource_get_instance(void);
 void bootsource_set_alias_name(const char *name);
-char *bootsource_get_alias_name(void);
+const char *bootsource_get_alias_name(void);
 const char *bootsource_to_string(enum bootsource src);
 const char *bootsource_get_alias_stem(enum bootsource bs);
 int bootsource_of_alias_xlate(enum bootsource bs, int instance);
-- 
2.39.5

[PATCH v2 4/4] ARM: i.MX8MP: tqma8mpxl: fix bbu registration for other boards using SoM

[Ahmad Fatoum]

The board code matches against the SoM compatible and there are two
upstream device trees making use of the SoM.

Both device trees reorder the device tree aliases to be different from
the SoC ordering specified in dts/src/arm64/freescale/imx8mp.dtsi.

This is partially addressed by the /chosen/barebox,bootsource-mmcX
properties in arch/arm/dts/imx8mp.dtsi.

What's not addressed is the name of the eMMC device in /dev: This is
currently hardcoded to /dev/mmc0, but any other board that includes the
same SoM and doesn't override the SoCs aliases will have the eMMC
instead at /dev/mmc2, breaking the barebox update handler.

One way to workaround this would be reverting commit 9f3f4af8cf56
("ARM: i.MX8MP: tqma8mpxl: match board code against SoM compatible") and
expect that other boards take care of registering the bbu handler themselves.

What is implemented here is an alternative that could also be useful to
other boards: Given that /chosen/environment-sd/device-path already points
at the SD and /chosen/environment-mmc/device-path already points at the eMMC,
let's just get the alias for the nodes they point at and use these to
dynamically derive the device's name in /dev.

Doing it this way requires only that there are any aliases for the MMCs at all
(which should be the case as the upstream SoC dtsi has them) and doesn't
suffer from probe order issues.

Fixes: 9f3f4af8cf56 ("ARM: i.MX8MP: tqma8mpxl: match board code against SoM compatible")
Reported-by: Holger Assmann <h.assmann@pengutronix.de>
Signed-off-by: Ahmad Fatoum <a.fatoum@pengutronix.de>
---
v1 -> v2:
  - restore of_device_enable_path instead of wrong of_device_enable_by_alias
    on the MMC alias
---
 arch/arm/boards/tqma8mpxl/board.c | 17 +++++++++++++----
 1 file changed, 13 insertions(+), 4 deletions(-)

diff --git a/arch/arm/boards/tqma8mpxl/board.c b/arch/arm/boards/tqma8mpxl/board.c
index 8b67acb91c55..fc144032c7f9 100644
--- a/arch/arm/boards/tqma8mpxl/board.c
+++ b/arch/arm/boards/tqma8mpxl/board.c
@@ -14,22 +14,31 @@
 #include <mach/imx/iomux-mx8mp.h>
 #include <gpio.h>
 #include <envfs.h>
+#include <string.h>
 
 static int tqma8mpxl_probe(struct device *dev)
 {
+	const char *emmc, *sd;
 	int emmc_bbu_flag = 0;
 	int sd_bbu_flag = 0;
 
-	if (bootsource_get() == BOOTSOURCE_MMC && bootsource_get_instance() == 1) {
+	sd = of_env_get_alias_by_path("/chosen/environment-sd");
+	emmc = of_env_get_alias_by_path("/chosen/environment-emmc");
+
+	if (streq_ptr(bootsource_get_alias_name(), sd)) {
 		of_device_enable_path("/chosen/environment-sd");
 		sd_bbu_flag = BBU_HANDLER_FLAG_DEFAULT;
-	} else {
+	} else if (emmc) {
 		of_device_enable_path("/chosen/environment-emmc");
 		emmc_bbu_flag = BBU_HANDLER_FLAG_DEFAULT;
 	}
 
-	imx8m_bbu_internal_mmc_register_handler("SD", "/dev/mmc1.barebox", sd_bbu_flag);
-	imx8m_bbu_internal_mmcboot_register_handler("eMMC", "/dev/mmc0", emmc_bbu_flag);
+	if (sd)
+		imx8m_bbu_internal_mmc_register_handler("SD",
+			basprintf("/dev/%s.barebox", sd), sd_bbu_flag);
+	if (emmc)
+		imx8m_bbu_internal_mmcboot_register_handler("eMMC",
+			basprintf("/dev/%s", emmc), emmc_bbu_flag);
 
 	return 0;
 }
-- 
2.39.5