mail archive of the barebox mailing list
 help / color / mirror / Atom feed
* [PATCH 0/4] of: fdt: read blspec/FIT DT compat without unflattening
@ 2024-03-01 13:04 Ahmad Fatoum
  2024-03-01 13:04 ` [PATCH 1/4] of: fdt: factor out FDT header parsing Ahmad Fatoum
                   ` (4 more replies)
  0 siblings, 5 replies; 9+ messages in thread
From: Ahmad Fatoum @ 2024-03-01 13:04 UTC (permalink / raw)
  To: barebox

blspec and FIT systems usually have multiple entries/configurations,
where a suitable one should be selected at runtime by comparing each
against the board's barebox DT compatible.

For blspec, this meant we need to unflatten the whole DT only to access
a single property. For FIT, this meant we expected the configuration to
carry a "compatible" property and didn't even attempt to parse it out of
the fdt as fallback. Both are resolved with this series.

Ahmad Fatoum (4):
  of: fdt: factor out FDT header parsing
  of: fdt: implement fdt_machine_is_compatible
  blspec: don't parse whole device tree to compare compatibles
  FIT: support finding compatible configuration by FDT compatible

 common/blspec.c    |  16 ++---
 common/image-fit.c |  39 ++++++++++-
 drivers/of/fdt.c   | 171 ++++++++++++++++++++++++++++++++++++---------
 include/of.h       |   8 +++
 4 files changed, 191 insertions(+), 43 deletions(-)

-- 
2.39.2




^ permalink raw reply	[flat|nested] 9+ messages in thread

* [PATCH 1/4] of: fdt: factor out FDT header parsing
  2024-03-01 13:04 [PATCH 0/4] of: fdt: read blspec/FIT DT compat without unflattening Ahmad Fatoum
@ 2024-03-01 13:04 ` Ahmad Fatoum
  2024-03-01 13:04 ` [PATCH 2/4] of: fdt: implement fdt_machine_is_compatible Ahmad Fatoum
                   ` (3 subsequent siblings)
  4 siblings, 0 replies; 9+ messages in thread
From: Ahmad Fatoum @ 2024-03-01 13:04 UTC (permalink / raw)
  To: barebox; +Cc: Ahmad Fatoum

Follow-up commit will need to parse the FDT header without unflattening
the whole device tree at the same time. Therefore split off the header
verification into its own separate function.

Signed-off-by: Ahmad Fatoum <a.fatoum@pengutronix.de>
---
 drivers/of/fdt.c | 76 +++++++++++++++++++++++++++---------------------
 1 file changed, 43 insertions(+), 33 deletions(-)

diff --git a/drivers/of/fdt.c b/drivers/of/fdt.c
index cf08fa1cfdfd..676b8a5535bf 100644
--- a/drivers/of/fdt.c
+++ b/drivers/of/fdt.c
@@ -43,9 +43,9 @@ static inline uint32_t dt_struct_advance(struct fdt_header *f, uint32_t dt, int
 	return dt;
 }
 
-static inline char *dt_string(struct fdt_header *f, char *strstart, uint32_t ofs)
+static inline const char *dt_string(struct fdt_header *f, const char *strstart, uint32_t ofs)
 {
-	char *str;
+	const char *str;
 
 	if (ofs > f->size_dt_strings)
 		return NULL;
@@ -115,6 +115,44 @@ static int of_unflatten_reservemap(struct device_node *root,
 	return 0;
 }
 
+static int fdt_parse_header(const struct fdt_header *fdt, size_t fdt_size,
+			  struct fdt_header *out)
+{
+	if (fdt_size < sizeof(struct fdt_header))
+		return -EINVAL;
+
+	if (fdt->magic != cpu_to_fdt32(FDT_MAGIC)) {
+		pr_err("bad magic: 0x%08x\n", fdt32_to_cpu(fdt->magic));
+		return -EINVAL;
+	}
+
+	if (fdt->version != cpu_to_fdt32(17)) {
+		pr_err("bad dt version: 0x%08x\n", fdt32_to_cpu(fdt->version));
+		return -EINVAL;
+	}
+
+	out->totalsize = fdt32_to_cpu(fdt->totalsize);
+	out->off_dt_struct = fdt32_to_cpu(fdt->off_dt_struct);
+	out->size_dt_struct = fdt32_to_cpu(fdt->size_dt_struct);
+	out->off_dt_strings = fdt32_to_cpu(fdt->off_dt_strings);
+	out->size_dt_strings = fdt32_to_cpu(fdt->size_dt_strings);
+
+	if (out->totalsize > fdt_size)
+		return -EINVAL;
+
+	if (size_add(out->off_dt_struct, out->size_dt_struct) > out->totalsize) {
+		pr_err("unflatten: dt size exceeds total size\n");
+		return -ESPIPE;
+	}
+
+	if (size_add(out->off_dt_strings, out->size_dt_strings) > out->totalsize) {
+		pr_err("unflatten: string size exceeds total size\n");
+		return -ESPIPE;
+	}
+
+	return 0;
+}
+
 /**
  * of_unflatten_dtb - unflatten a dtb binary blob
  * @infdt - the fdt blob to unflatten
@@ -140,37 +178,9 @@ static struct device_node *__of_unflatten_dtb(const void *infdt, int size,
 	unsigned int maxlen;
 	const struct fdt_header *fdt = infdt;
 
-	if (size < sizeof(struct fdt_header))
-		return ERR_PTR(-EINVAL);
-
-	if (fdt->magic != cpu_to_fdt32(FDT_MAGIC)) {
-		pr_err("bad magic: 0x%08x\n", fdt32_to_cpu(fdt->magic));
-		return ERR_PTR(-EINVAL);
-	}
-
-	if (fdt->version != cpu_to_fdt32(17)) {
-		pr_err("bad dt version: 0x%08x\n", fdt32_to_cpu(fdt->version));
-		return ERR_PTR(-EINVAL);
-	}
-
-	f.totalsize = fdt32_to_cpu(fdt->totalsize);
-	f.off_dt_struct = fdt32_to_cpu(fdt->off_dt_struct);
-	f.size_dt_struct = fdt32_to_cpu(fdt->size_dt_struct);
-	f.off_dt_strings = fdt32_to_cpu(fdt->off_dt_strings);
-	f.size_dt_strings = fdt32_to_cpu(fdt->size_dt_strings);
-
-	if (f.totalsize > size)
-		return ERR_PTR(-EINVAL);
-
-	if (size_add(f.off_dt_struct, f.size_dt_struct) > f.totalsize) {
-		pr_err("unflatten: dt size exceeds total size\n");
-		return ERR_PTR(-ESPIPE);
-	}
-
-	if (size_add(f.off_dt_strings, f.size_dt_strings) > f.totalsize) {
-		pr_err("unflatten: string size exceeds total size\n");
-		return ERR_PTR(-ESPIPE);
-	}
+	ret = fdt_parse_header(infdt, size, &f);
+	if (ret < 0)
+		return ERR_PTR(ret);
 
 	dt_struct = f.off_dt_struct;
 	dt_strings = (void *)fdt + f.off_dt_strings;
-- 
2.39.2




^ permalink raw reply	[flat|nested] 9+ messages in thread

* [PATCH 2/4] of: fdt: implement fdt_machine_is_compatible
  2024-03-01 13:04 [PATCH 0/4] of: fdt: read blspec/FIT DT compat without unflattening Ahmad Fatoum
  2024-03-01 13:04 ` [PATCH 1/4] of: fdt: factor out FDT header parsing Ahmad Fatoum
@ 2024-03-01 13:04 ` Ahmad Fatoum
  2024-03-04  9:37   ` Sascha Hauer
  2024-03-01 13:04 ` [PATCH 3/4] blspec: don't parse whole device tree to compare compatibles Ahmad Fatoum
                   ` (2 subsequent siblings)
  4 siblings, 1 reply; 9+ messages in thread
From: Ahmad Fatoum @ 2024-03-01 13:04 UTC (permalink / raw)
  To: barebox; +Cc: Ahmad Fatoum

When finding compatible bootloader spec files, barebox will unflatten
each DTB in turn, allocating objects for each property and node, only to
compare a single property and then free all the allocations again.

Given that this operation is repeated for every device tree until a
match is found, it's a good idea to be able to compare machine
(top-level) compatibles without having to unflatten the whole FDT.

Implemnt fdt_machine_is_compatible() that does just that. This
intentionally opencodes the device tree iteration as to minimize
code and runtime size. Using libfdt without LTO would be slower
and bigger.

Signed-off-by: Ahmad Fatoum <a.fatoum@pengutronix.de>
---
 drivers/of/fdt.c | 95 ++++++++++++++++++++++++++++++++++++++++++++++++
 include/of.h     |  3 ++
 2 files changed, 98 insertions(+)

diff --git a/drivers/of/fdt.c b/drivers/of/fdt.c
index 676b8a5535bf..aa3d2e967acd 100644
--- a/drivers/of/fdt.c
+++ b/drivers/of/fdt.c
@@ -668,3 +668,98 @@ void fdt_print_reserve_map(const void *__fdt)
 			return;
 	}
 }
+
+static int fdt_string_is_compatible(const char *haystack, int haystack_len,
+				    const char *needle, int needle_len)
+{
+	const char *p;
+	int index = 0;
+
+	while (haystack_len >= needle_len) {
+		if (memcmp(needle, haystack, needle_len + 1) == 0)
+			return OF_DEVICE_COMPATIBLE_MAX_SCORE - (index << 2);
+
+		p = memchr(haystack, '\0', haystack_len);
+		if (!p)
+			return 0;
+		haystack_len -= (p - haystack) + 1;
+		haystack = p + 1;
+		index++;
+	}
+
+	return 0;
+}
+
+bool fdt_machine_is_compatible(const struct fdt_header *fdt, size_t fdt_size, const char *compat)
+{
+	uint32_t tag;
+	const struct fdt_property *fdt_prop;
+	const char *name;
+	uint32_t dt_struct;
+	const struct fdt_node_header *fnh;
+	const void *dt_strings;
+	struct fdt_header f;
+	int ret, len;
+	int expect = FDT_BEGIN_NODE;
+	int compat_len = strlen(compat);
+
+	ret = fdt_parse_header(fdt, fdt_size, &f);
+	if (ret < 0)
+		return ERR_PTR(ret);
+
+	dt_struct = f.off_dt_struct;
+	dt_strings = (const void *)fdt + f.off_dt_strings;
+
+	while (1) {
+		const __be32 *tagp = (const void *)fdt + dt_struct;
+		if (!dt_ptr_ok(fdt, tagp))
+			return false;
+
+		tag = be32_to_cpu(*tagp);
+		if (tag != FDT_NOP && tag != expect)
+			return false;
+
+		switch (tag) {
+		case FDT_BEGIN_NODE:
+			fnh = (const void *)fdt + dt_struct;
+
+			/* The root node must have an empty name */
+			if (fnh->name[0] != '\0')
+				return false;
+
+			dt_struct = dt_struct_advance(&f, dt_struct,
+					sizeof(struct fdt_node_header) + 1);
+
+			expect = FDT_PROP;
+			break;
+
+		case FDT_PROP:
+			fdt_prop = (const void *)fdt + dt_struct;
+			len = fdt32_to_cpu(fdt_prop->len);
+
+			name = dt_string(&f, dt_strings, fdt32_to_cpu(fdt_prop->nameoff));
+			if (!name)
+				return false;
+
+			if (strcmp(name, "compatible")) {
+				dt_struct = dt_struct_advance(&f, dt_struct,
+							      sizeof(struct fdt_property) + len);
+				break;
+			}
+
+			return fdt_string_is_compatible(fdt_prop->data, len, compat, compat_len);
+
+		case FDT_NOP:
+			dt_struct = dt_struct_advance(&f, dt_struct, FDT_TAGSIZE);
+			break;
+
+		default:
+			return false;
+		}
+
+		if (!dt_struct)
+			return false;
+	}
+
+	return false;
+}
diff --git a/include/of.h b/include/of.h
index 3391403a347f..b00940c8532e 100644
--- a/include/of.h
+++ b/include/of.h
@@ -64,6 +64,9 @@ void of_clean_reserve_map(void);
 void fdt_add_reserve_map(void *fdt);
 void fdt_print_reserve_map(const void *fdt);
 
+bool fdt_machine_is_compatible(const struct fdt_header *fdt, size_t fdt_size, const char *compat);
+
+
 struct device;
 struct driver;
 struct resource;
-- 
2.39.2




^ permalink raw reply	[flat|nested] 9+ messages in thread

* [PATCH 3/4] blspec: don't parse whole device tree to compare compatibles
  2024-03-01 13:04 [PATCH 0/4] of: fdt: read blspec/FIT DT compat without unflattening Ahmad Fatoum
  2024-03-01 13:04 ` [PATCH 1/4] of: fdt: factor out FDT header parsing Ahmad Fatoum
  2024-03-01 13:04 ` [PATCH 2/4] of: fdt: implement fdt_machine_is_compatible Ahmad Fatoum
@ 2024-03-01 13:04 ` Ahmad Fatoum
  2024-03-01 13:04 ` [PATCH 4/4] FIT: support finding compatible configuration by FDT compatible Ahmad Fatoum
  2024-03-04  9:49 ` [PATCH 0/4] of: fdt: read blspec/FIT DT compat without unflattening Sascha Hauer
  4 siblings, 0 replies; 9+ messages in thread
From: Ahmad Fatoum @ 2024-03-01 13:04 UTC (permalink / raw)
  To: barebox; +Cc: Ahmad Fatoum

Now that we have an fdt_machine_is_compatible that can operate on
flattened device trees, use it to speed up the process of finding
compatible bootloader spec entries.

Signed-off-by: Ahmad Fatoum <a.fatoum@pengutronix.de>
---
 common/blspec.c | 16 ++++++++--------
 1 file changed, 8 insertions(+), 8 deletions(-)

diff --git a/common/blspec.c b/common/blspec.c
index b70332a54c28..23a24c63db15 100644
--- a/common/blspec.c
+++ b/common/blspec.c
@@ -427,7 +427,9 @@ static bool entry_is_of_compatible(struct blspec_entry *entry)
 	const char *devicetree;
 	const char *abspath;
 	int ret;
-	struct device_node *root = NULL, *barebox_root;
+	struct device_node *barebox_root;
+	size_t size;
+	void *fdt;
 	const char *compat;
 	char *filename;
 
@@ -455,25 +457,23 @@ static bool entry_is_of_compatible(struct blspec_entry *entry)
 
 	filename = basprintf("%s/%s", abspath, devicetree);
 
-	root = of_read_file(filename);
-	if (IS_ERR(root)) {
+	fdt = read_file(filename, &size);
+	if (!fdt) {
 		ret = false;
-		root = NULL;
 		goto out;
 	}
 
-	if (of_device_is_compatible(root, compat)) {
+	if (fdt_machine_is_compatible(fdt, size, compat)) {
 		ret = true;
 		goto out;
 	}
 
-	pr_info("ignoring entry with incompatible devicetree \"%s\"\n",
-			(char *)of_get_property(root, "compatible", NULL));
+	pr_info("ignoring entry with incompatible devicetree: %s\n", devicetree);
 
 	ret = false;
 
 out:
-	of_delete_node(root);
+	free(fdt);
 	free(filename);
 
 	return ret;
-- 
2.39.2




^ permalink raw reply	[flat|nested] 9+ messages in thread

* [PATCH 4/4] FIT: support finding compatible configuration by FDT compatible
  2024-03-01 13:04 [PATCH 0/4] of: fdt: read blspec/FIT DT compat without unflattening Ahmad Fatoum
                   ` (2 preceding siblings ...)
  2024-03-01 13:04 ` [PATCH 3/4] blspec: don't parse whole device tree to compare compatibles Ahmad Fatoum
@ 2024-03-01 13:04 ` Ahmad Fatoum
  2024-03-04  9:49 ` [PATCH 0/4] of: fdt: read blspec/FIT DT compat without unflattening Sascha Hauer
  4 siblings, 0 replies; 9+ messages in thread
From: Ahmad Fatoum @ 2024-03-01 13:04 UTC (permalink / raw)
  To: barebox; +Cc: Ahmad Fatoum

So far, we only supported finding compatible configurations that have
a compatible property inside the configuration's device tree node.

According to spec, this is optional however, and e.g. Yocto's
kernel-fitimage.bbclass don't generate it.

Instead, the bootloader is expected to lookup the compatible inside the
referenced FDT. With fdt_machine_is_compatible, this is much less of a
performance hit than with of_machine_is_compatible, so let's implement
support for this.

Signed-off-by: Ahmad Fatoum <a.fatoum@pengutronix.de>
---
 common/image-fit.c | 39 +++++++++++++++++++++++++++++++++++++--
 include/of.h       |  5 +++++
 2 files changed, 42 insertions(+), 2 deletions(-)

diff --git a/common/image-fit.c b/common/image-fit.c
index b16752de05bc..251fda97b3fc 100644
--- a/common/image-fit.c
+++ b/common/image-fit.c
@@ -715,7 +715,8 @@ static int fit_config_verify_signature(struct fit_handle *handle, struct device_
 	return ret;
 }
 
-static int fit_find_compatible_unit(struct device_node *conf_node,
+static int fit_find_compatible_unit(struct fit_handle *handle,
+				    struct device_node *conf_node,
 				    const char **unit)
 {
 	struct device_node *child = NULL;
@@ -734,6 +735,40 @@ static int fit_find_compatible_unit(struct device_node *conf_node,
 
 	for_each_child_of_node(conf_node, child) {
 		int score = of_device_is_compatible(child, machine);
+
+		if (!score && !of_property_present(child, "compatible") &&
+		    of_property_present(child, "fdt")) {
+			struct device_node *image;
+			const char *unit = "fdt";
+			int data_len;
+			const void *data;
+			int ret;
+
+			ret = fit_get_image(handle, child, &unit, &image);
+			if (ret)
+				goto next;
+
+			data = of_get_property(image, "data", &data_len);
+			if (!data) {
+				ret = -EINVAL;
+				goto next;
+			}
+
+			ret = fit_handle_decompression(image, "fdt", &data, &data_len);
+			if (ret) {
+				ret = -EILSEQ;
+				goto next;
+			}
+
+			score = fdt_machine_is_compatible(data, data_len, machine);
+
+			of_delete_property_by_name(image, "uncompressed-data");
+next:
+			if (ret)
+				pr_warn("skipping malformed configuration: %pOF (%pe)\n",
+					child, ERR_PTR(ret));
+		}
+
 		if (score > best_score) {
 			best_score = score;
 			*unit = child->name;
@@ -779,7 +814,7 @@ void *fit_open_configuration(struct fit_handle *handle, const char *name)
 	if (name) {
 		unit = name;
 	} else {
-		ret = fit_find_compatible_unit(conf_node, &unit);
+		ret = fit_find_compatible_unit(handle, conf_node, &unit);
 		if (ret) {
 			pr_info("Couldn't get a valid configuration. Aborting.\n");
 			return ERR_PTR(ret);
diff --git a/include/of.h b/include/of.h
index b00940c8532e..f0214fb13c49 100644
--- a/include/of.h
+++ b/include/of.h
@@ -1226,6 +1226,11 @@ static inline int of_property_write_u64(struct device_node *np,
 	return of_property_write_u64_array(np, propname, &value, 1);
 }
 
+static inline void of_delete_property_by_name(struct device_node *np, const char *name)
+{
+	of_delete_property(of_find_property(np, name, NULL));
+}
+
 extern const struct of_device_id of_default_bus_match_table[];
 
 int of_device_enable(struct device_node *node);
-- 
2.39.2




^ permalink raw reply	[flat|nested] 9+ messages in thread

* Re: [PATCH 2/4] of: fdt: implement fdt_machine_is_compatible
  2024-03-01 13:04 ` [PATCH 2/4] of: fdt: implement fdt_machine_is_compatible Ahmad Fatoum
@ 2024-03-04  9:37   ` Sascha Hauer
  2024-03-04  9:49     ` Sascha Hauer
  2024-03-04  9:50     ` Ahmad Fatoum
  0 siblings, 2 replies; 9+ messages in thread
From: Sascha Hauer @ 2024-03-04  9:37 UTC (permalink / raw)
  To: Ahmad Fatoum; +Cc: barebox

On Fri, Mar 01, 2024 at 02:04:43PM +0100, Ahmad Fatoum wrote:
> When finding compatible bootloader spec files, barebox will unflatten
> each DTB in turn, allocating objects for each property and node, only to
> compare a single property and then free all the allocations again.
> 
> Given that this operation is repeated for every device tree until a
> match is found, it's a good idea to be able to compare machine
> (top-level) compatibles without having to unflatten the whole FDT.
> 
> Implemnt fdt_machine_is_compatible() that does just that. This
> intentionally opencodes the device tree iteration as to minimize
> code and runtime size. Using libfdt without LTO would be slower
> and bigger.
> 
> Signed-off-by: Ahmad Fatoum <a.fatoum@pengutronix.de>
> ---
>  drivers/of/fdt.c | 95 ++++++++++++++++++++++++++++++++++++++++++++++++
>  include/of.h     |  3 ++
>  2 files changed, 98 insertions(+)
> 
> diff --git a/drivers/of/fdt.c b/drivers/of/fdt.c
> index 676b8a5535bf..aa3d2e967acd 100644
> --- a/drivers/of/fdt.c
> +++ b/drivers/of/fdt.c
> @@ -668,3 +668,98 @@ void fdt_print_reserve_map(const void *__fdt)
>  			return;
>  	}
>  }
> +
> +static int fdt_string_is_compatible(const char *haystack, int haystack_len,
> +				    const char *needle, int needle_len)
> +{
> +	const char *p;
> +	int index = 0;
> +
> +	while (haystack_len >= needle_len) {
> +		if (memcmp(needle, haystack, needle_len + 1) == 0)
> +			return OF_DEVICE_COMPATIBLE_MAX_SCORE - (index << 2);
> +
> +		p = memchr(haystack, '\0', haystack_len);
> +		if (!p)
> +			return 0;
> +		haystack_len -= (p - haystack) + 1;
> +		haystack = p + 1;
> +		index++;
> +	}
> +
> +	return 0;
> +}
> +
> +bool fdt_machine_is_compatible(const struct fdt_header *fdt, size_t fdt_size, const char *compat)
> +{
> +	uint32_t tag;
> +	const struct fdt_property *fdt_prop;
> +	const char *name;
> +	uint32_t dt_struct;
> +	const struct fdt_node_header *fnh;
> +	const void *dt_strings;
> +	struct fdt_header f;
> +	int ret, len;
> +	int expect = FDT_BEGIN_NODE;
> +	int compat_len = strlen(compat);
> +
> +	ret = fdt_parse_header(fdt, fdt_size, &f);
> +	if (ret < 0)
> +		return ERR_PTR(ret);

return false here

Sascha

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



^ permalink raw reply	[flat|nested] 9+ messages in thread

* Re: [PATCH 0/4] of: fdt: read blspec/FIT DT compat without unflattening
  2024-03-01 13:04 [PATCH 0/4] of: fdt: read blspec/FIT DT compat without unflattening Ahmad Fatoum
                   ` (3 preceding siblings ...)
  2024-03-01 13:04 ` [PATCH 4/4] FIT: support finding compatible configuration by FDT compatible Ahmad Fatoum
@ 2024-03-04  9:49 ` Sascha Hauer
  4 siblings, 0 replies; 9+ messages in thread
From: Sascha Hauer @ 2024-03-04  9:49 UTC (permalink / raw)
  To: barebox, Ahmad Fatoum


On Fri, 01 Mar 2024 14:04:41 +0100, Ahmad Fatoum wrote:
> blspec and FIT systems usually have multiple entries/configurations,
> where a suitable one should be selected at runtime by comparing each
> against the board's barebox DT compatible.
> 
> For blspec, this meant we need to unflatten the whole DT only to access
> a single property. For FIT, this meant we expected the configuration to
> carry a "compatible" property and didn't even attempt to parse it out of
> the fdt as fallback. Both are resolved with this series.
> 
> [...]

Applied, thanks!

[1/4] of: fdt: factor out FDT header parsing
      https://git.pengutronix.de/cgit/barebox/commit/?id=a8b58bf5d7cd (link may not be stable)
[2/4] of: fdt: implement fdt_machine_is_compatible
      https://git.pengutronix.de/cgit/barebox/commit/?id=143f2ae76809 (link may not be stable)
[3/4] blspec: don't parse whole device tree to compare compatibles
      https://git.pengutronix.de/cgit/barebox/commit/?id=9f2b74f1eb58 (link may not be stable)
[4/4] FIT: support finding compatible configuration by FDT compatible
      https://git.pengutronix.de/cgit/barebox/commit/?id=5c9cd335e893 (link may not be stable)

Best regards,
-- 
Sascha Hauer <s.hauer@pengutronix.de>




^ permalink raw reply	[flat|nested] 9+ messages in thread

* Re: [PATCH 2/4] of: fdt: implement fdt_machine_is_compatible
  2024-03-04  9:37   ` Sascha Hauer
@ 2024-03-04  9:49     ` Sascha Hauer
  2024-03-04  9:50     ` Ahmad Fatoum
  1 sibling, 0 replies; 9+ messages in thread
From: Sascha Hauer @ 2024-03-04  9:49 UTC (permalink / raw)
  To: Ahmad Fatoum; +Cc: barebox

On Mon, Mar 04, 2024 at 10:37:06AM +0100, Sascha Hauer wrote:
> On Fri, Mar 01, 2024 at 02:04:43PM +0100, Ahmad Fatoum wrote:
> > When finding compatible bootloader spec files, barebox will unflatten
> > each DTB in turn, allocating objects for each property and node, only to
> > compare a single property and then free all the allocations again.
> > 
> > Given that this operation is repeated for every device tree until a
> > match is found, it's a good idea to be able to compare machine
> > (top-level) compatibles without having to unflatten the whole FDT.
> > 
> > Implemnt fdt_machine_is_compatible() that does just that. This
> > intentionally opencodes the device tree iteration as to minimize
> > code and runtime size. Using libfdt without LTO would be slower
> > and bigger.
> > 
> > Signed-off-by: Ahmad Fatoum <a.fatoum@pengutronix.de>
> > ---
> >  drivers/of/fdt.c | 95 ++++++++++++++++++++++++++++++++++++++++++++++++
> >  include/of.h     |  3 ++
> >  2 files changed, 98 insertions(+)
> > 
> > diff --git a/drivers/of/fdt.c b/drivers/of/fdt.c
> > index 676b8a5535bf..aa3d2e967acd 100644
> > --- a/drivers/of/fdt.c
> > +++ b/drivers/of/fdt.c
> > @@ -668,3 +668,98 @@ void fdt_print_reserve_map(const void *__fdt)
> >  			return;
> >  	}
> >  }
> > +
> > +static int fdt_string_is_compatible(const char *haystack, int haystack_len,
> > +				    const char *needle, int needle_len)
> > +{
> > +	const char *p;
> > +	int index = 0;
> > +
> > +	while (haystack_len >= needle_len) {
> > +		if (memcmp(needle, haystack, needle_len + 1) == 0)
> > +			return OF_DEVICE_COMPATIBLE_MAX_SCORE - (index << 2);
> > +
> > +		p = memchr(haystack, '\0', haystack_len);
> > +		if (!p)
> > +			return 0;
> > +		haystack_len -= (p - haystack) + 1;
> > +		haystack = p + 1;
> > +		index++;
> > +	}
> > +
> > +	return 0;
> > +}
> > +
> > +bool fdt_machine_is_compatible(const struct fdt_header *fdt, size_t fdt_size, const char *compat)
> > +{
> > +	uint32_t tag;
> > +	const struct fdt_property *fdt_prop;
> > +	const char *name;
> > +	uint32_t dt_struct;
> > +	const struct fdt_node_header *fnh;
> > +	const void *dt_strings;
> > +	struct fdt_header f;
> > +	int ret, len;
> > +	int expect = FDT_BEGIN_NODE;
> > +	int compat_len = strlen(compat);
> > +
> > +	ret = fdt_parse_header(fdt, fdt_size, &f);
> > +	if (ret < 0)
> > +		return ERR_PTR(ret);
> 
> return false here

Fixed while applying.

Sascha

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



^ permalink raw reply	[flat|nested] 9+ messages in thread

* Re: [PATCH 2/4] of: fdt: implement fdt_machine_is_compatible
  2024-03-04  9:37   ` Sascha Hauer
  2024-03-04  9:49     ` Sascha Hauer
@ 2024-03-04  9:50     ` Ahmad Fatoum
  1 sibling, 0 replies; 9+ messages in thread
From: Ahmad Fatoum @ 2024-03-04  9:50 UTC (permalink / raw)
  To: Sascha Hauer; +Cc: barebox

On 04.03.24 10:37, Sascha Hauer wrote:
> On Fri, Mar 01, 2024 at 02:04:43PM +0100, Ahmad Fatoum wrote:
>> When finding compatible bootloader spec files, barebox will unflatten
>> each DTB in turn, allocating objects for each property and node, only to
>> compare a single property and then free all the allocations again.
>>
>> Given that this operation is repeated for every device tree until a
>> match is found, it's a good idea to be able to compare machine
>> (top-level) compatibles without having to unflatten the whole FDT.
>>
>> Implemnt fdt_machine_is_compatible() that does just that. This
>> intentionally opencodes the device tree iteration as to minimize
>> code and runtime size. Using libfdt without LTO would be slower
>> and bigger.
>>
>> Signed-off-by: Ahmad Fatoum <a.fatoum@pengutronix.de>
>> ---
>>  drivers/of/fdt.c | 95 ++++++++++++++++++++++++++++++++++++++++++++++++
>>  include/of.h     |  3 ++
>>  2 files changed, 98 insertions(+)
>>
>> diff --git a/drivers/of/fdt.c b/drivers/of/fdt.c
>> index 676b8a5535bf..aa3d2e967acd 100644
>> --- a/drivers/of/fdt.c
>> +++ b/drivers/of/fdt.c
>> @@ -668,3 +668,98 @@ void fdt_print_reserve_map(const void *__fdt)
>>  			return;
>>  	}
>>  }
>> +
>> +static int fdt_string_is_compatible(const char *haystack, int haystack_len,
>> +				    const char *needle, int needle_len)
>> +{
>> +	const char *p;
>> +	int index = 0;
>> +
>> +	while (haystack_len >= needle_len) {
>> +		if (memcmp(needle, haystack, needle_len + 1) == 0)
>> +			return OF_DEVICE_COMPATIBLE_MAX_SCORE - (index << 2);
>> +
>> +		p = memchr(haystack, '\0', haystack_len);
>> +		if (!p)
>> +			return 0;
>> +		haystack_len -= (p - haystack) + 1;
>> +		haystack = p + 1;
>> +		index++;
>> +	}
>> +
>> +	return 0;
>> +}
>> +
>> +bool fdt_machine_is_compatible(const struct fdt_header *fdt, size_t fdt_size, const char *compat)
>> +{
>> +	uint32_t tag;
>> +	const struct fdt_property *fdt_prop;
>> +	const char *name;
>> +	uint32_t dt_struct;
>> +	const struct fdt_node_header *fnh;
>> +	const void *dt_strings;
>> +	struct fdt_header f;
>> +	int ret, len;
>> +	int expect = FDT_BEGIN_NODE;
>> +	int compat_len = strlen(compat);
>> +
>> +	ret = fdt_parse_header(fdt, fdt_size, &f);
>> +	if (ret < 0)
>> +		return ERR_PTR(ret);
> 
> return false here

Ouch. Thanks for fixing!

> 
> Sascha
> 

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




^ permalink raw reply	[flat|nested] 9+ messages in thread

end of thread, other threads:[~2024-03-04 10:30 UTC | newest]

Thread overview: 9+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2024-03-01 13:04 [PATCH 0/4] of: fdt: read blspec/FIT DT compat without unflattening Ahmad Fatoum
2024-03-01 13:04 ` [PATCH 1/4] of: fdt: factor out FDT header parsing Ahmad Fatoum
2024-03-01 13:04 ` [PATCH 2/4] of: fdt: implement fdt_machine_is_compatible Ahmad Fatoum
2024-03-04  9:37   ` Sascha Hauer
2024-03-04  9:49     ` Sascha Hauer
2024-03-04  9:50     ` Ahmad Fatoum
2024-03-01 13:04 ` [PATCH 3/4] blspec: don't parse whole device tree to compare compatibles Ahmad Fatoum
2024-03-01 13:04 ` [PATCH 4/4] FIT: support finding compatible configuration by FDT compatible Ahmad Fatoum
2024-03-04  9:49 ` [PATCH 0/4] of: fdt: read blspec/FIT DT compat without unflattening Sascha Hauer

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox