[PATCH v2 0/3] Update aliases when added or removed

[PATCH v2 0/3] Update aliases when added or removed

[Ayush Singh]

Currently the list of aliases is not updated when a DT overlay that adds
an alias is loaded or unloaded. This break drivers (e.g. serial) that
rely on of_alias_get_id().

This picks up the original patch series from Geert Uytterhoeven.

I have not added unittests in this version since I am not sure if kunit
tests should be added, or if the runtime unittests (CONFIG_OF_UNITTEST)
need to be added. Additionally, it would be great if someone can inform
me how to run the runtime unittests since the unittests seem to fail in
my current setup (tried running on qemu x86_64 and beagleplay).

Changes in v2:
- Use `of_reconfig_notifier_register`
- Leak memory when freeing alias entry
- Link to v1: https://lore.kernel.org/all/1435675876-2159-1-git-send-email-geert+renesas@glider.be/

---
Ayush Singh (3):
      of: Extract of_alias_create()
      of: Add of_alias_destroy()
      of: Add notifier in of_reconfig_notifier_register()

 drivers/of/base.c | 132 +++++++++++++++++++++++++++++++++++++++---------------
 1 file changed, 95 insertions(+), 37 deletions(-)
---
base-commit: 929beafbe7acce3267c06115e13e03ff6e50548a
change-id: 20241016-of-alias-bc4f333dc773

Best regards,
-- 
Ayush Singh <ayush@beagleboard.org>

[PATCH v2 1/3] of: Extract of_alias_create()

[Ayush Singh]

Extract the code to create and add an alias from of_alias_scan() into
its own function of_alias_create().

Co-developed-by: Geert Uytterhoeven <geert@linux-m68k.org>
Signed-off-by: Geert Uytterhoeven <geert@linux-m68k.org>
Signed-off-by: Ayush Singh <ayush@beagleboard.org>
---
 drivers/of/base.c | 79 +++++++++++++++++++++++++++++--------------------------
 1 file changed, 42 insertions(+), 37 deletions(-)

diff --git a/drivers/of/base.c b/drivers/of/base.c
index a8b0c42bdc8e9c352b0af9f9a8540aa6a3afd771..e5cd75fca95132060334aa8547c58951d2132cfb 100644
--- a/drivers/of/base.c
+++ b/drivers/of/base.c
@@ -180,6 +180,47 @@ void __of_phandle_cache_inv_entry(phandle handle)
 		phandle_cache[handle_hash] = NULL;
 }
 
+static void of_alias_add(struct alias_prop *ap, struct device_node *np,
+			 int id, const char *stem, int stem_len)
+{
+	ap->np = np;
+	ap->id = id;
+	strscpy(ap->stem, stem, stem_len + 1);
+	list_add_tail(&ap->link, &aliases_lookup);
+	pr_debug("adding DT alias:%s: stem=%s id=%i node=%pOF\n",
+		 ap->alias, ap->stem, ap->id, np);
+}
+
+static void of_alias_create(const struct property *pp,
+			    void *(*dt_alloc)(u64 size, u64 align))
+{
+	const char *start = pp->name;
+	const char *end = start + strlen(start);
+	struct device_node *np;
+	struct alias_prop *ap;
+	int id, len;
+
+	np = of_find_node_by_path(pp->value);
+	if (!np)
+		return;
+
+	/* walk the alias backwards to extract the id and work out the 'stem' string */
+	while (isdigit(*(end - 1)) && end > start)
+		end--;
+	len = end - start;
+
+	if (kstrtoint(end, 10, &id) < 0)
+		return;
+
+	/* Allocate an alias_prop with enough space for the stem */
+	ap = dt_alloc(sizeof(*ap) + len + 1, 4);
+	if (!ap)
+		return;
+	memset(ap, 0, sizeof(*ap) + len + 1);
+	ap->alias = start;
+	of_alias_add(ap, np, id, start, len);
+}
+
 void __init of_core_init(void)
 {
 	struct device_node *np;
@@ -1763,17 +1804,6 @@ int of_update_property(struct device_node *np, struct property *newprop)
 	return rc;
 }
 
-static void of_alias_add(struct alias_prop *ap, struct device_node *np,
-			 int id, const char *stem, int stem_len)
-{
-	ap->np = np;
-	ap->id = id;
-	strscpy(ap->stem, stem, stem_len + 1);
-	list_add_tail(&ap->link, &aliases_lookup);
-	pr_debug("adding DT alias:%s: stem=%s id=%i node=%pOF\n",
-		 ap->alias, ap->stem, ap->id, np);
-}
-
 /**
  * of_alias_scan - Scan all properties of the 'aliases' node
  * @dt_alloc:	An allocator that provides a virtual address to memory
@@ -1811,38 +1841,13 @@ void of_alias_scan(void * (*dt_alloc)(u64 size, u64 align))
 		return;
 
 	for_each_property_of_node(of_aliases, pp) {
-		const char *start = pp->name;
-		const char *end = start + strlen(start);
-		struct device_node *np;
-		struct alias_prop *ap;
-		int id, len;
-
 		/* Skip those we do not want to proceed */
 		if (!strcmp(pp->name, "name") ||
 		    !strcmp(pp->name, "phandle") ||
 		    !strcmp(pp->name, "linux,phandle"))
 			continue;
 
-		np = of_find_node_by_path(pp->value);
-		if (!np)
-			continue;
-
-		/* walk the alias backwards to extract the id and work out
-		 * the 'stem' string */
-		while (isdigit(*(end-1)) && end > start)
-			end--;
-		len = end - start;
-
-		if (kstrtoint(end, 10, &id) < 0)
-			continue;
-
-		/* Allocate an alias_prop with enough space for the stem */
-		ap = dt_alloc(sizeof(*ap) + len + 1, __alignof__(*ap));
-		if (!ap)
-			continue;
-		memset(ap, 0, sizeof(*ap) + len + 1);
-		ap->alias = start;
-		of_alias_add(ap, np, id, start, len);
+		of_alias_create(pp, dt_alloc);
 	}
 }
 

-- 
2.47.0

[PATCH v2 2/3] of: Add of_alias_destroy()

[Ayush Singh]

Add a helper function to find an alias, remove it from the list of
aliases, and destroy it.

Co-developed-by: Geert Uytterhoeven <geert@linux-m68k.org>
Signed-off-by: Geert Uytterhoeven <geert@linux-m68k.org>
Signed-off-by: Ayush Singh <ayush@beagleboard.org>
---
 drivers/of/base.c | 14 ++++++++++++++
 1 file changed, 14 insertions(+)

diff --git a/drivers/of/base.c b/drivers/of/base.c
index e5cd75fca95132060334aa8547c58951d2132cfb..1cfb3cd4493e17d16868981288115798c6a6a151 100644
--- a/drivers/of/base.c
+++ b/drivers/of/base.c
@@ -221,6 +221,20 @@ static void of_alias_create(const struct property *pp,
 	of_alias_add(ap, np, id, start, len);
 }
 
+static void of_alias_destroy(const char *name, void (*dt_free)(void *))
+{
+	struct alias_prop *ap;
+
+	list_for_each_entry(ap, &aliases_lookup, link) {
+		if (strcmp(ap->alias, name))
+			continue;
+
+		list_del(&ap->link);
+		dt_free(ap);
+		return;
+	}
+}
+
 void __init of_core_init(void)
 {
 	struct device_node *np;

-- 
2.47.0

[PATCH v2 3/3] of: Add notifier in of_reconfig_notifier_register()

[Ayush Singh]

Currently the list of aliases is not updated when an overlay that
modifies /aliases is added or removed. This breaks drivers (e.g. serial)
that rely on of_alias_get_id().

Update the list of aliases when a property of the /aliases node is
added, removed, or updated by registering a notifier.

Co-developed-by: Geert Uytterhoeven <geert@linux-m68k.org>
Signed-off-by: Geert Uytterhoeven <geert@linux-m68k.org>
Signed-off-by: Ayush Singh <ayush@beagleboard.org>
---
 drivers/of/base.c | 39 +++++++++++++++++++++++++++++++++++++++
 1 file changed, 39 insertions(+)

diff --git a/drivers/of/base.c b/drivers/of/base.c
index 1cfb3cd4493e17d16868981288115798c6a6a151..5c061fdca885973d4389c0f6afe459a751d850b8 100644
--- a/drivers/of/base.c
+++ b/drivers/of/base.c
@@ -235,11 +235,50 @@ static void of_alias_destroy(const char *name, void (*dt_free)(void *))
 	}
 }
 
+static void *alias_alloc(u64 size, u64 align)
+{
+	return kzalloc(size, GFP_KERNEL);
+}
+
+static void alias_free(void *p)
+{
+	/* Leak memory */
+}
+
+static int alias_OF_notifier(struct notifier_block *np, unsigned long action,
+			     void *data)
+{
+	struct of_reconfig_data *reconf_data = data;
+
+	if (reconf_data->dn != of_aliases)
+		return NOTIFY_DONE;
+
+	switch (action) {
+	case OF_RECONFIG_ADD_PROPERTY:
+		of_alias_create(reconf_data->prop, alias_alloc);
+		break;
+	case OF_RECONFIG_REMOVE_PROPERTY:
+		of_alias_destroy(reconf_data->prop->name, alias_free);
+		break;
+	case OF_RECONFIG_UPDATE_PROPERTY:
+		of_alias_destroy(reconf_data->old_prop->name, alias_free);
+		of_alias_create(reconf_data->prop, alias_alloc);
+		break;
+	}
+
+	return NOTIFY_OK;
+}
+
+static struct notifier_block alias_of_nb = {
+	.notifier_call = alias_OF_notifier,
+};
+
 void __init of_core_init(void)
 {
 	struct device_node *np;
 
 	of_platform_register_reconfig_notifier();
+	of_reconfig_notifier_register(&alias_of_nb);
 
 	/* Create the kset, and register existing nodes */
 	mutex_lock(&of_mutex);

-- 
2.47.0

Re: [PATCH v2 0/3] Update aliases when added or removed

[Rob Herring]

On Sat, Nov 9, 2024 at 1:52 PM Ayush Singh <ayush@beagleboard.org> wrote:
>
> Currently the list of aliases is not updated when a DT overlay that adds
> an alias is loaded or unloaded. This break drivers (e.g. serial) that
> rely on of_alias_get_id().

Drivers use the non-existent alias numbers for instances without an
alias. So what happens if an index is already in use and then an
overlay uses the same index.

I don't see how this can work reliably unless the alias name doesn't
exist in the base DT.

> This picks up the original patch series from Geert Uytterhoeven.
>
> I have not added unittests in this version since I am not sure if kunit
> tests should be added, or if the runtime unittests (CONFIG_OF_UNITTEST)
> need to be added. Additionally, it would be great if someone can inform
> me how to run the runtime unittests since the unittests seem to fail in
> my current setup (tried running on qemu x86_64 and beagleplay).

You enable the config and boot.

Rob