[RFC Patch 0/3] BLSpec device-tree fallback handling

[RFC Patch 0/3] BLSpec device-tree fallback handling

[Rouven Czerwinski]

Barebox contains code to select the correct bootloader specification
entry by looking at the devicetree option, loading the tree and
comparing that trees first compatible with the compatible of the
internal device tree.

This series implements the comparison to not only use the first
compatible defined in the tree, but latter compatibles as well.
This is useful if compatibles are adjusted, but new bootloaders need to
be able to boot bootloader spec entries with the old compatible set.
In this case the barebox board can set the following compatibles:

  compatible = "newcomp", "oldcomp",… ;

And call blspec_set_compatible_depth(2) in the board code to indicate
that not only the first compatible should be compared, but oldcomp
should be compared as well.

Rouven Czerwinski (3):
  blspec: create list of entries, iterate over list
  blspec: take compatible name as argument
  blspec: allow setting compatible depth

 common/blspec.c  | 125 +++++++++++++++++++++++++++++++++++------------
 include/blspec.h |   2 +-
 2 files changed, 94 insertions(+), 33 deletions(-)

-- 
2.35.1


_______________________________________________
barebox mailing list
barebox@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/barebox

[RFC Patch 1/3] blspec: create list of entries, iterate over list

[Rouven Czerwinski]

Instead of parsing all files in one go and checking the compatible,
create a list and iterate over it to check the compatible. This is a
preparation to not only account for the top-level barebox compatible,
but to instead use the compatible score system to find the correct
bootable device tree entry.

Signed-off-by: Rouven Czerwinski <r.czerwinski@pengutronix.de>
---
 common/blspec.c | 41 +++++++++++++++++++++++++++++++++++++++--
 1 file changed, 39 insertions(+), 2 deletions(-)

diff --git a/common/blspec.c b/common/blspec.c
index 158fd1e9a2..0ea857294f 100644
--- a/common/blspec.c
+++ b/common/blspec.c
@@ -520,6 +520,21 @@ static bool entry_is_match_machine_id(struct blspec_entry *entry)
  *
  * returns the number of entries found or a negative error value otherwise.
  */
+
+struct blspec_list_entry {
+	struct list_head list;
+	struct blspec_entry *entry;
+	char *name;
+};
+
+static int compare(struct list_head *a, struct list_head *b)
+{
+	char *na = (char *)list_entry(a, struct blspec_list_entry, list)->name;
+	char *nb = (char *)list_entry(b, struct blspec_list_entry, list)->name;
+
+	return strcmp(na, nb);
+}
+
 int blspec_scan_directory(struct bootentries *bootentries, const char *root)
 {
 	struct blspec_entry *entry;
@@ -529,6 +544,11 @@ int blspec_scan_directory(struct bootentries *bootentries, const char *root)
 	int ret, found = 0;
 	const char *dirname = "loader/entries";
 	char *nfspath = NULL;
+	struct list_head entry_list;
+	struct blspec_list_entry *lentry;
+	struct blspec_list_entry *temp;
+
+	INIT_LIST_HEAD(&entry_list);
 
 	nfspath = parse_nfs_url(root);
 	if (!IS_ERR(nfspath))
@@ -549,7 +569,6 @@ int blspec_scan_directory(struct bootentries *bootentries, const char *root)
 		char *configname;
 		struct stat s;
 		char *dot;
-		char *devname = NULL, *hwdevname = NULL;
 
 		if (*d->d_name == '.')
 			continue;
@@ -593,13 +612,29 @@ int blspec_scan_directory(struct bootentries *bootentries, const char *root)
 		entry->configpath = configname;
 		entry->cdev = get_cdev_by_mountpath(root);
 
+		lentry = calloc(1, sizeof(*lentry));
+		lentry->entry = entry;
+		lentry->name = d->d_name;
+
+		list_add_sort(&entry_list, &lentry->list, compare);
+	}
+
+	list_for_each_entry_safe(lentry, temp, &entry_list, list) {
+		char *devname = NULL, *hwdevname = NULL;
+
+		entry = lentry->entry;
+
 		if (!entry_is_of_compatible(entry)) {
 			blspec_entry_free(&entry->entry);
+			list_del(&lentry->list);
+			free(lentry);
 			continue;
 		}
 
 		if (!entry_is_match_machine_id(entry)) {
 			blspec_entry_free(&entry->entry);
+			list_del(&lentry->list);
+			free(lentry);
 			continue;
 		}
 
@@ -612,7 +647,7 @@ int blspec_scan_directory(struct bootentries *bootentries, const char *root)
 		}
 
 		entry->entry.title = xasprintf("%s (%s)", blspec_entry_var_get(entry, "title"),
-					       configname);
+					       entry->configpath);
 		entry->entry.description = basprintf("blspec entry, device: %s hwdevice: %s",
 						    devname ? devname : "none",
 						    hwdevname ? hwdevname : "none");
@@ -623,6 +658,8 @@ int blspec_scan_directory(struct bootentries *bootentries, const char *root)
 		entry->entry.release = blspec_entry_free;
 
 		bootentries_add_entry(bootentries, &entry->entry);
+		list_del(&lentry->list);
+		free(lentry);
 	}
 
 	ret = found;
-- 
2.35.1


_______________________________________________
barebox mailing list
barebox@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/barebox

[RFC Patch 2/3] blspec: take compatible name as argument

[Rouven Czerwinski]

Instead of retrieving the root compatible itself, let
entry_is_of_compatible take the compatible as an argument. Pass in
of_get_machine_compatible for now, no functional changes.

Signed-off-by: Rouven Czerwinski <r.czerwinski@pengutronix.de>
---
 common/blspec.c | 9 ++-------
 1 file changed, 2 insertions(+), 7 deletions(-)

diff --git a/common/blspec.c b/common/blspec.c
index 0ea857294f..c1b1991338 100644
--- a/common/blspec.c
+++ b/common/blspec.c
@@ -421,7 +421,7 @@ out:
  *
  * returns true if the entry is compatible, false otherwise
  */
-static bool entry_is_of_compatible(struct blspec_entry *entry)
+static bool entry_is_of_compatible(struct blspec_entry *entry, const char *compat)
 {
 	const char *devicetree;
 	const char *abspath;
@@ -429,7 +429,6 @@ static bool entry_is_of_compatible(struct blspec_entry *entry)
 	void *fdt = NULL;
 	int ret;
 	struct device_node *root = NULL, *barebox_root;
-	const char *compat;
 	char *filename;
 
 	/* If the entry doesn't specify a devicetree we are compatible */
@@ -445,10 +444,6 @@ static bool entry_is_of_compatible(struct blspec_entry *entry)
 	if (!barebox_root)
 		return true;
 
-	ret = of_property_read_string(barebox_root, "compatible", &compat);
-	if (ret)
-		return false;
-
 	if (entry->rootpath)
 		abspath = entry->rootpath;
 	else
@@ -624,7 +619,7 @@ int blspec_scan_directory(struct bootentries *bootentries, const char *root)
 
 		entry = lentry->entry;
 
-		if (!entry_is_of_compatible(entry)) {
+		if (!entry_is_of_compatible(entry, of_get_machine_compatible())) {
 			blspec_entry_free(&entry->entry);
 			list_del(&lentry->list);
 			free(lentry);
-- 
2.35.1


_______________________________________________
barebox mailing list
barebox@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/barebox

[RFC Patch 3/3] blspec: allow setting compatible depth

[Rouven Czerwinski]

In some cases its desirable to not only evaluate the first compatible of
the barebox device tree against the bootspec device tree, but following
compatibles as well. This is the case if fallback needs to be
implemented for older device trees which use a different compatible.
Allow setting the compatible depth by board code.

Signed-off-by: Rouven Czerwinski <r.czerwinski@pengutronix.de>
---
 common/blspec.c  | 99 +++++++++++++++++++++++++++++++-----------------
 include/blspec.h |  2 +-
 2 files changed, 65 insertions(+), 36 deletions(-)

diff --git a/common/blspec.c b/common/blspec.c
index c1b1991338..4713a2157f 100644
--- a/common/blspec.c
+++ b/common/blspec.c
@@ -22,6 +22,8 @@
 #include <linux/err.h>
 #include <mtd/ubi-user.h>
 
+static unsigned int compatible_depth = 1;
+
 /*
  * blspec_entry_var_set - set a variable to a value
  */
@@ -32,6 +34,11 @@ int blspec_entry_var_set(struct blspec_entry *entry, const char *name,
 			val ? strlen(val) + 1 : 0, 1);
 }
 
+void blspec_set_compatible_depth(unsigned int depth)
+{
+	compatible_depth = depth;
+}
+
 static int blspec_overlay_fixup(struct device_node *root, void *ctx)
 {
 	struct blspec_entry *entry = ctx;
@@ -542,6 +549,10 @@ int blspec_scan_directory(struct bootentries *bootentries, const char *root)
 	struct list_head entry_list;
 	struct blspec_list_entry *lentry;
 	struct blspec_list_entry *temp;
+	struct device_node *barebox_root;
+	struct property *prop;
+	const char *compat;
+	int depth;
 
 	INIT_LIST_HEAD(&entry_list);
 
@@ -611,51 +622,69 @@ int blspec_scan_directory(struct bootentries *bootentries, const char *root)
 		lentry->entry = entry;
 		lentry->name = d->d_name;
 
-		list_add_sort(&entry_list, &lentry->list, compare);
+		list_add_sort(&lentry->list, &entry_list, compare);
 	}
 
-	list_for_each_entry_safe(lentry, temp, &entry_list, list) {
-		char *devname = NULL, *hwdevname = NULL;
+	barebox_root = of_get_root_node();
 
-		entry = lentry->entry;
+	prop = of_find_property(barebox_root, "compatible", NULL);
 
-		if (!entry_is_of_compatible(entry, of_get_machine_compatible())) {
-			blspec_entry_free(&entry->entry);
-			list_del(&lentry->list);
-			free(lentry);
-			continue;
-		}
+	compat = of_prop_next_string(prop, NULL);
+	depth = compatible_depth;
+	do {
+		list_for_each_entry_safe(lentry, temp, &entry_list, list) {
+			char *devname = NULL, *hwdevname = NULL;
 
-		if (!entry_is_match_machine_id(entry)) {
-			blspec_entry_free(&entry->entry);
-			list_del(&lentry->list);
-			free(lentry);
-			continue;
-		}
+			entry = lentry->entry;
 
-		found++;
+			if (!entry_is_of_compatible(entry, compat)) {
+				if (depth == 1) {
+					blspec_entry_free(&entry->entry);
+					list_del(&lentry->list);
+					free(lentry);
+				}
+				continue;
+			}
 
-		if (entry->cdev && entry->cdev->dev) {
-			devname = xstrdup(dev_name(entry->cdev->dev));
-			if (entry->cdev->dev->parent)
-				hwdevname = xstrdup(dev_name(entry->cdev->dev->parent));
-		}
+			if (!entry_is_match_machine_id(entry)) {
+				if (depth == 1) {
+					blspec_entry_free(&entry->entry);
+					list_del(&lentry->list);
+					free(lentry);
+				}
+				continue;
+			}
 
-		entry->entry.title = xasprintf("%s (%s)", blspec_entry_var_get(entry, "title"),
-					       entry->configpath);
-		entry->entry.description = basprintf("blspec entry, device: %s hwdevice: %s",
-						    devname ? devname : "none",
-						    hwdevname ? hwdevname : "none");
-		free(devname);
-		free(hwdevname);
+			found++;
 
-		entry->entry.me.type = MENU_ENTRY_NORMAL;
-		entry->entry.release = blspec_entry_free;
+			if (entry->cdev && entry->cdev->dev) {
+				devname = xstrdup(dev_name(entry->cdev->dev));
+				if (entry->cdev->dev->parent)
+					hwdevname = xstrdup(dev_name(entry->cdev->dev->parent));
+			}
 
-		bootentries_add_entry(bootentries, &entry->entry);
-		list_del(&lentry->list);
-		free(lentry);
-	}
+			entry->entry.title = xasprintf("%s (%s)", blspec_entry_var_get(entry, "title"),
+						       entry->configpath);
+			entry->entry.description = basprintf("blspec entry, device: %s hwdevice: %s",
+							     devname ? devname : "none",
+							     hwdevname ? hwdevname : "none");
+			free(devname);
+			free(hwdevname);
+
+			entry->entry.me.type = MENU_ENTRY_NORMAL;
+			entry->entry.release = blspec_entry_free;
+
+			bootentries_add_entry(bootentries, &entry->entry);
+			list_del(&lentry->list);
+			free(lentry);
+		}
+
+		compat = of_prop_next_string(prop, compat);
+		if (!compat)
+			break;
+
+		depth--;
+	} while (depth > 0);
 
 	ret = found;
 
diff --git a/include/blspec.h b/include/blspec.h
index 37076cd47c..f499ad8b2d 100644
--- a/include/blspec.h
+++ b/include/blspec.h
@@ -23,5 +23,5 @@ int blspec_scan_devices(struct bootentries *bootentries);
 int blspec_scan_device(struct bootentries *bootentries, struct device_d *dev);
 int blspec_scan_devicename(struct bootentries *bootentries, const char *devname);
 int blspec_scan_directory(struct bootentries *bootentries, const char *root);
-
+void blspec_set_compatible_depth(unsigned int depth);
 #endif /* __LOADER_H__ */
-- 
2.35.1


_______________________________________________
barebox mailing list
barebox@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/barebox

Re: [RFC Patch 3/3] blspec: allow setting compatible depth

[Sascha Hauer]

On Fri, Mar 04, 2022 at 07:24:13AM +0100, Rouven Czerwinski wrote:
> In some cases its desirable to not only evaluate the first compatible of
> the barebox device tree against the bootspec device tree, but following
> compatibles as well. This is the case if fallback needs to be
> implemented for older device trees which use a different compatible.
> Allow setting the compatible depth by board code.

I had a chat with Rouven yesterday. We agreed to allow to specifiy
additional compatible strings rather than specifying a depth. He will
send a v2 soon.

Sascha

> 
> Signed-off-by: Rouven Czerwinski <r.czerwinski@pengutronix.de>
> ---
>  common/blspec.c  | 99 +++++++++++++++++++++++++++++++-----------------
>  include/blspec.h |  2 +-
>  2 files changed, 65 insertions(+), 36 deletions(-)
> 
> diff --git a/common/blspec.c b/common/blspec.c
> index c1b1991338..4713a2157f 100644
> --- a/common/blspec.c
> +++ b/common/blspec.c
> @@ -22,6 +22,8 @@
>  #include <linux/err.h>
>  #include <mtd/ubi-user.h>
>  
> +static unsigned int compatible_depth = 1;
> +
>  /*
>   * blspec_entry_var_set - set a variable to a value
>   */
> @@ -32,6 +34,11 @@ int blspec_entry_var_set(struct blspec_entry *entry, const char *name,
>  			val ? strlen(val) + 1 : 0, 1);
>  }
>  
> +void blspec_set_compatible_depth(unsigned int depth)
> +{
> +	compatible_depth = depth;
> +}
> +
>  static int blspec_overlay_fixup(struct device_node *root, void *ctx)
>  {
>  	struct blspec_entry *entry = ctx;
> @@ -542,6 +549,10 @@ int blspec_scan_directory(struct bootentries *bootentries, const char *root)
>  	struct list_head entry_list;
>  	struct blspec_list_entry *lentry;
>  	struct blspec_list_entry *temp;
> +	struct device_node *barebox_root;
> +	struct property *prop;
> +	const char *compat;
> +	int depth;
>  
>  	INIT_LIST_HEAD(&entry_list);
>  
> @@ -611,51 +622,69 @@ int blspec_scan_directory(struct bootentries *bootentries, const char *root)
>  		lentry->entry = entry;
>  		lentry->name = d->d_name;
>  
> -		list_add_sort(&entry_list, &lentry->list, compare);
> +		list_add_sort(&lentry->list, &entry_list, compare);
>  	}
>  
> -	list_for_each_entry_safe(lentry, temp, &entry_list, list) {
> -		char *devname = NULL, *hwdevname = NULL;
> +	barebox_root = of_get_root_node();
>  
> -		entry = lentry->entry;
> +	prop = of_find_property(barebox_root, "compatible", NULL);
>  
> -		if (!entry_is_of_compatible(entry, of_get_machine_compatible())) {
> -			blspec_entry_free(&entry->entry);
> -			list_del(&lentry->list);
> -			free(lentry);
> -			continue;
> -		}
> +	compat = of_prop_next_string(prop, NULL);
> +	depth = compatible_depth;
> +	do {
> +		list_for_each_entry_safe(lentry, temp, &entry_list, list) {
> +			char *devname = NULL, *hwdevname = NULL;
>  
> -		if (!entry_is_match_machine_id(entry)) {
> -			blspec_entry_free(&entry->entry);
> -			list_del(&lentry->list);
> -			free(lentry);
> -			continue;
> -		}
> +			entry = lentry->entry;
>  
> -		found++;
> +			if (!entry_is_of_compatible(entry, compat)) {
> +				if (depth == 1) {
> +					blspec_entry_free(&entry->entry);
> +					list_del(&lentry->list);
> +					free(lentry);
> +				}
> +				continue;
> +			}
>  
> -		if (entry->cdev && entry->cdev->dev) {
> -			devname = xstrdup(dev_name(entry->cdev->dev));
> -			if (entry->cdev->dev->parent)
> -				hwdevname = xstrdup(dev_name(entry->cdev->dev->parent));
> -		}
> +			if (!entry_is_match_machine_id(entry)) {
> +				if (depth == 1) {
> +					blspec_entry_free(&entry->entry);
> +					list_del(&lentry->list);
> +					free(lentry);
> +				}
> +				continue;
> +			}
>  
> -		entry->entry.title = xasprintf("%s (%s)", blspec_entry_var_get(entry, "title"),
> -					       entry->configpath);
> -		entry->entry.description = basprintf("blspec entry, device: %s hwdevice: %s",
> -						    devname ? devname : "none",
> -						    hwdevname ? hwdevname : "none");
> -		free(devname);
> -		free(hwdevname);
> +			found++;
>  
> -		entry->entry.me.type = MENU_ENTRY_NORMAL;
> -		entry->entry.release = blspec_entry_free;
> +			if (entry->cdev && entry->cdev->dev) {
> +				devname = xstrdup(dev_name(entry->cdev->dev));
> +				if (entry->cdev->dev->parent)
> +					hwdevname = xstrdup(dev_name(entry->cdev->dev->parent));
> +			}
>  
> -		bootentries_add_entry(bootentries, &entry->entry);
> -		list_del(&lentry->list);
> -		free(lentry);
> -	}
> +			entry->entry.title = xasprintf("%s (%s)", blspec_entry_var_get(entry, "title"),
> +						       entry->configpath);
> +			entry->entry.description = basprintf("blspec entry, device: %s hwdevice: %s",
> +							     devname ? devname : "none",
> +							     hwdevname ? hwdevname : "none");
> +			free(devname);
> +			free(hwdevname);
> +
> +			entry->entry.me.type = MENU_ENTRY_NORMAL;
> +			entry->entry.release = blspec_entry_free;
> +
> +			bootentries_add_entry(bootentries, &entry->entry);
> +			list_del(&lentry->list);
> +			free(lentry);
> +		}
> +
> +		compat = of_prop_next_string(prop, compat);
> +		if (!compat)
> +			break;
> +
> +		depth--;
> +	} while (depth > 0);
>  
>  	ret = found;
>  
> diff --git a/include/blspec.h b/include/blspec.h
> index 37076cd47c..f499ad8b2d 100644
> --- a/include/blspec.h
> +++ b/include/blspec.h
> @@ -23,5 +23,5 @@ int blspec_scan_devices(struct bootentries *bootentries);
>  int blspec_scan_device(struct bootentries *bootentries, struct device_d *dev);
>  int blspec_scan_devicename(struct bootentries *bootentries, const char *devname);
>  int blspec_scan_directory(struct bootentries *bootentries, const char *root);
> -
> +void blspec_set_compatible_depth(unsigned int depth);
>  #endif /* __LOADER_H__ */
> -- 
> 2.35.1
> 
> 
> _______________________________________________
> barebox mailing list
> barebox@lists.infradead.org
> http://lists.infradead.org/mailman/listinfo/barebox
> 

-- 
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 |

_______________________________________________
barebox mailing list
barebox@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/barebox