[PATCH 00/17] Apply device tree overlays to kernel tree

[PATCH 00/17] Apply device tree overlays to kernel tree

[Sascha Hauer]

With this series barebox gains the capability to apply device tree
overlays to the kernel device tree before starting the kernel with it.
Which overlays are applied can be controlled with a set of globavars,
for details see the documentation patch in this series.

The particular usecase we have in mind with this series is that the
rootfs has a bunch of device tree overlays in it which shall be applied
based on bootloader knowledge. For example information of the exact
board type might be stored in an EEPROM. Based on that information barebox
can now pick the right overlays for that board from the rootfs.
Interpreting the EEPROM content is board specific and not part of this
series, for now the the globalvars must be set manually.

Sascha

Sascha Hauer (17):
  fdt: Check blob size during unflattening
  firmware: make device_node argument non const
  libbb: Add find_path function
  firmware: consolidate ifdefs
  firmware: Add search path
  firmware: Fix device_node matching
  firmware: recognize by reproducible name
  blspec: Set firmware searchpath
  overlay: only apply compatible trees
  overlay: Add of_overlay_apply_file()
  firmware: Load from global search path
  blspec: Rework firmware load
  of_overlay: apply overlays during booting
  blspec: Apply overlays from rootfs
  doc: devicetree: Refer to internal device tree also as live tree
  Documentation: Add documentation for device tree overlays
  of_firmware: Fix handling of firmware-name property

 Documentation/user/devicetree.rst         |  39 +++++-
 arch/arm/boards/highbank/init.c           |   2 +-
 arch/arm/boards/qemu-virt/board.c         |   4 +-
 arch/arm/boards/raspberry-pi/rpi-common.c |   2 +-
 arch/arm/boards/webasto-ccbv2/board.c     |   2 +-
 arch/arm/lib32/bootm.c                    |   2 +-
 commands/of_diff.c                        |   2 +-
 commands/of_display_timings.c             |   2 +-
 commands/of_dump.c                        |   4 +-
 commands/of_overlay.c                     |  16 +--
 commands/oftree.c                         |   2 +-
 common/blspec.c                           |  99 ++++++---------
 common/bootm.c                            |   4 +-
 common/efi/efi.c                          |   2 +-
 common/firmware.c                         |  59 ++++++++-
 common/image-fit.c                        |   2 +-
 common/state/backend_format_dtb.c         |   2 +-
 drivers/firmware/altera_serial.c          |   1 +
 drivers/firmware/socfpga.c                |   2 +
 drivers/firmware/zynqmp-fpga.c            |   2 +
 drivers/of/base.c                         |   2 +-
 drivers/of/fdt.c                          |  17 ++-
 drivers/of/of_firmware.c                  |  56 ++-------
 drivers/of/overlay.c                      | 146 ++++++++++++++++++++++
 include/firmware.h                        |  26 ++--
 include/libbb.h                           |   4 +-
 include/of.h                              |  33 ++++-
 lib/libbb.c                               |  58 ++++++---
 28 files changed, 413 insertions(+), 179 deletions(-)

-- 
2.29.2


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

[PATCH 01/17] fdt: Check blob size during unflattening

[Sascha Hauer]

of_unflatten_dtb() doesn't check the size of the device tree blob
passed to it. Add a size argument end add checks for the size. Some
callers have no idea of the buffer size themselves, INT_MAX is passed
in these cases.

Signed-off-by: Sascha Hauer <s.hauer@pengutronix.de>
---
 arch/arm/boards/highbank/init.c           |  2 +-
 arch/arm/boards/qemu-virt/board.c         |  4 ++--
 arch/arm/boards/raspberry-pi/rpi-common.c |  2 +-
 arch/arm/boards/webasto-ccbv2/board.c     |  2 +-
 arch/arm/lib32/bootm.c                    |  2 +-
 commands/of_diff.c                        |  2 +-
 commands/of_display_timings.c             |  2 +-
 commands/of_dump.c                        |  4 ++--
 commands/of_overlay.c                     |  5 +++--
 commands/oftree.c                         |  2 +-
 common/blspec.c                           |  7 ++++---
 common/bootm.c                            |  4 ++--
 common/efi/efi.c                          |  2 +-
 common/image-fit.c                        |  2 +-
 common/state/backend_format_dtb.c         |  2 +-
 drivers/of/base.c                         |  2 +-
 drivers/of/fdt.c                          | 17 ++++++++++++-----
 include/of.h                              |  4 ++--
 18 files changed, 38 insertions(+), 29 deletions(-)

diff --git a/arch/arm/boards/highbank/init.c b/arch/arm/boards/highbank/init.c
index 32e217321a..d013294ff3 100644
--- a/arch/arm/boards/highbank/init.c
+++ b/arch/arm/boards/highbank/init.c
@@ -75,7 +75,7 @@ static int highbank_mem_init(void)
 	/* load by the firmware at 0x1000 */
 	fdt = IOMEM(FIRMWARE_DTB_BASE);
 
-	root = of_unflatten_dtb(fdt);
+	root = of_unflatten_dtb(fdt, INT_MAX);
 	if (IS_ERR(root)) {
 		pr_warn("no dtb found at 0x1000 use default configuration\n");
 		fdt = NULL;
diff --git a/arch/arm/boards/qemu-virt/board.c b/arch/arm/boards/qemu-virt/board.c
index 9d8c90eb5b..0e8f052186 100644
--- a/arch/arm/boards/qemu-virt/board.c
+++ b/arch/arm/boards/qemu-virt/board.c
@@ -25,14 +25,14 @@ static int replace_dtb(void) {
 		return 0;
 	}
 
-	root = of_unflatten_dtb(fdt);
+	root = of_unflatten_dtb(fdt, INT_MAX);
 
 	if (!of_device_is_compatible(root, "linux,dummy-virt")) {
 		of_delete_node(root);
 		return 0;
 	}
 
-	overlay = of_unflatten_dtb(__dtb_overlay_of_flash_start);
+	overlay = of_unflatten_dtb(__dtb_overlay_of_flash_start, INT_MAX);
 	of_overlay_apply_tree(root, overlay);
 	barebox_register_of(root);
 
diff --git a/arch/arm/boards/raspberry-pi/rpi-common.c b/arch/arm/boards/raspberry-pi/rpi-common.c
index e326732b3a..6c5df6fd69 100644
--- a/arch/arm/boards/raspberry-pi/rpi-common.c
+++ b/arch/arm/boards/raspberry-pi/rpi-common.c
@@ -430,7 +430,7 @@ static int rpi_vc_fdt_bootargs(void *fdt)
 	struct device_node *root = NULL, *node;
 	const char *cmdline;
 
-	root = of_unflatten_dtb(fdt);
+	root = of_unflatten_dtb(fdt, INT_MAX);
 	if (IS_ERR(root)) {
 		ret = PTR_ERR(root);
 		root = NULL;
diff --git a/arch/arm/boards/webasto-ccbv2/board.c b/arch/arm/boards/webasto-ccbv2/board.c
index a78258ea6a..477771309e 100644
--- a/arch/arm/boards/webasto-ccbv2/board.c
+++ b/arch/arm/boards/webasto-ccbv2/board.c
@@ -28,7 +28,7 @@ static int ccbv2_probe(struct device_d *dev)
 		return 0;
 
 	fdt = (void*)OPTEE_OVERLAY_LOCATION;
-	overlay = of_unflatten_dtb(fdt);
+	overlay = of_unflatten_dtb(fdt, INT_MAX);
 
 	if (IS_ERR(overlay))
 		return PTR_ERR(overlay);
diff --git a/arch/arm/lib32/bootm.c b/arch/arm/lib32/bootm.c
index 28a645a9d0..0ffb374cf1 100644
--- a/arch/arm/lib32/bootm.c
+++ b/arch/arm/lib32/bootm.c
@@ -421,7 +421,7 @@ static int do_bootz_linux_fdt(int fd, struct image_data *data, void **outfdt)
 	if (IS_BUILTIN(CONFIG_OFTREE)) {
 		struct device_node *root;
 
-		root = of_unflatten_dtb(oftree);
+		root = of_unflatten_dtb(oftree, header->totalsize);
 		if (IS_ERR(root)) {
 			pr_err("unable to unflatten devicetree\n");
 			goto err_free;
diff --git a/commands/of_diff.c b/commands/of_diff.c
index ec039776cf..e1ac98c26e 100644
--- a/commands/of_diff.c
+++ b/commands/of_diff.c
@@ -44,7 +44,7 @@ static struct device_node *get_tree(const char *filename, struct device_node *ro
 	if (ret)
 		return ERR_PTR(ret);
 
-	node = of_unflatten_dtb(fdt);
+	node = of_unflatten_dtb(fdt, size);
 
 	free(fdt);
 
diff --git a/commands/of_display_timings.c b/commands/of_display_timings.c
index 27b91f311a..4e5ec223b7 100644
--- a/commands/of_display_timings.c
+++ b/commands/of_display_timings.c
@@ -83,7 +83,7 @@ static int do_of_display_timings(int argc, char *argv[])
 			return -EINVAL;
 		}
 
-		root = of_unflatten_dtb(fdt);
+		root = of_unflatten_dtb(fdt, size);
 
 		free(fdt);
 
diff --git a/commands/of_dump.c b/commands/of_dump.c
index 6792af3afc..569a6287c6 100644
--- a/commands/of_dump.c
+++ b/commands/of_dump.c
@@ -68,7 +68,7 @@ static int do_of_dump(int argc, char *argv[])
 			return -errno;
 		}
 
-		root = of_unflatten_dtb(fdt);
+		root = of_unflatten_dtb(fdt, size);
 
 		free(fdt);
 
@@ -85,7 +85,7 @@ static int do_of_dump(int argc, char *argv[])
 			/* create a copy of internal devicetree */
 			void *fdt;
 			fdt = of_flatten_dtb(root);
-			root = of_unflatten_dtb(fdt);
+			root = of_unflatten_dtb(fdt, fdt_totalsize(fdt));
 
 			free(fdt);
 
diff --git a/commands/of_overlay.c b/commands/of_overlay.c
index aa4b6484ca..9a4c008efc 100644
--- a/commands/of_overlay.c
+++ b/commands/of_overlay.c
@@ -17,6 +17,7 @@ static int do_of_overlay(int argc, char *argv[])
 	struct fdt_header *fdt;
 	struct device_node *overlay;
 	const char *search_path = NULL;
+	size_t size;
 
 	while ((opt = getopt(argc, argv, "S:")) > 0) {
 		switch (opt) {
@@ -31,13 +32,13 @@ static int do_of_overlay(int argc, char *argv[])
 	if (argc != optind + 1)
 		return COMMAND_ERROR_USAGE;
 
-	fdt = read_file(argv[optind], NULL);
+	fdt = read_file(argv[optind], &size);
 	if (!fdt) {
 		printf("cannot read %s\n", argv[optind]);
 		return 1;
 	}
 
-	overlay = of_unflatten_dtb(fdt);
+	overlay = of_unflatten_dtb(fdt, size);
 	free(fdt);
 	if (IS_ERR(overlay))
 		return PTR_ERR(overlay);
diff --git a/commands/oftree.c b/commands/oftree.c
index 1d902f2830..7d4b08c9d3 100644
--- a/commands/oftree.c
+++ b/commands/oftree.c
@@ -82,7 +82,7 @@ static int do_oftree(int argc, char *argv[])
 			return 1;
 		}
 
-		root = of_unflatten_dtb(fdt);
+		root = of_unflatten_dtb(fdt, size);
 
 		free(fdt);
 
diff --git a/common/blspec.c b/common/blspec.c
index c05d8a8297..b6f91e3114 100644
--- a/common/blspec.c
+++ b/common/blspec.c
@@ -40,17 +40,18 @@ static int blspec_apply_oftree_overlay(char *file, const char *abspath,
 	struct device_node *overlay;
 	char *path;
 	char *firmware_path;
+	size_t size;
 
 	path = basprintf("%s/%s", abspath, file);
 
-	fdt = read_file(path, NULL);
+	fdt = read_file(path, &size);
 	if (!fdt) {
 		pr_warn("unable to read \"%s\"\n", path);
 		ret = -EINVAL;
 		goto out;
 	}
 
-	overlay = of_unflatten_dtb(fdt);
+	overlay = of_unflatten_dtb(fdt, size);
 	free(fdt);
 	if (IS_ERR(overlay)) {
 		ret = PTR_ERR(overlay);
@@ -490,7 +491,7 @@ static bool entry_is_of_compatible(struct blspec_entry *entry)
 		goto out;
 	}
 
-	root = of_unflatten_dtb(fdt);
+	root = of_unflatten_dtb(fdt, size);
 	if (IS_ERR(root)) {
 		ret = false;
 		root = NULL;
diff --git a/common/bootm.c b/common/bootm.c
index 092116beb9..c6839c2ca9 100644
--- a/common/bootm.c
+++ b/common/bootm.c
@@ -360,7 +360,7 @@ void *bootm_get_devicetree(struct image_data *data)
 		if (ret)
 			return ERR_PTR(ret);
 
-		data->of_root_node = of_unflatten_dtb(of_tree);
+		data->of_root_node = of_unflatten_dtb(of_tree, of_size);
 	} else if (data->oftree_file) {
 		size_t size;
 
@@ -388,7 +388,7 @@ void *bootm_get_devicetree(struct image_data *data)
 		if (ret)
 			return ERR_PTR(ret);
 
-		data->of_root_node = of_unflatten_dtb(oftree);
+		data->of_root_node = of_unflatten_dtb(oftree, size);
 
 		free(oftree);
 
diff --git a/common/efi/efi.c b/common/efi/efi.c
index 01003dc00f..7f12342cf9 100644
--- a/common/efi/efi.c
+++ b/common/efi/efi.c
@@ -437,7 +437,7 @@ static int efi_late_init(void)
 			return -EINVAL;
 		}
 
-		root = of_unflatten_dtb(fdt);
+		root = of_unflatten_dtb(fdt, size);
 
 		free(fdt);
 
diff --git a/common/image-fit.c b/common/image-fit.c
index 538e61f51b..266c7d5425 100644
--- a/common/image-fit.c
+++ b/common/image-fit.c
@@ -756,7 +756,7 @@ static int fit_do_open(struct fit_handle *handle)
 	const char *desc = "(no description)";
 	struct device_node *root;
 
-	root = of_unflatten_dtb_const(handle->fit);
+	root = of_unflatten_dtb_const(handle->fit, handle->size);
 	if (IS_ERR(root))
 		return PTR_ERR(root);
 
diff --git a/common/state/backend_format_dtb.c b/common/state/backend_format_dtb.c
index 48f30db1f5..d0fc948859 100644
--- a/common/state/backend_format_dtb.c
+++ b/common/state/backend_format_dtb.c
@@ -59,7 +59,7 @@ static int state_backend_format_dtb_verify(struct state_backend_format *format,
 		fdtb->root = NULL;
 	}
 
-	root = of_unflatten_dtb(buf);
+	root = of_unflatten_dtb(buf, dtb_len);
 	if (IS_ERR(root)) {
 		dev_err(fdtb->dev, "Failed to unflatten dtb from buffer with length %zd, %ld\n",
 			len, PTR_ERR(root));
diff --git a/drivers/of/base.c b/drivers/of/base.c
index 8759099d74..3cc58380f0 100644
--- a/drivers/of/base.c
+++ b/drivers/of/base.c
@@ -1718,7 +1718,7 @@ void barebox_register_fdt(const void *dtb)
 	if (root_node)
 		return;
 
-	root = of_unflatten_dtb(dtb);
+	root = of_unflatten_dtb(dtb, INT_MAX);
 	if (IS_ERR(root)) {
 		pr_err("Cannot unflatten dtb: %pe\n", root);
 		return;
diff --git a/drivers/of/fdt.c b/drivers/of/fdt.c
index d98913e54a..f72f5e3a30 100644
--- a/drivers/of/fdt.c
+++ b/drivers/of/fdt.c
@@ -114,7 +114,8 @@ static int of_unflatten_reservemap(struct device_node *root,
  * Parse a flat device tree binary blob and return a pointer to the
  * unflattened tree.
  */
-static struct device_node *__of_unflatten_dtb(const void *infdt, bool constprops)
+static struct device_node *__of_unflatten_dtb(const void *infdt, int size,
+					      bool constprops)
 {
 	const void *nodep;	/* property node pointer */
 	uint32_t tag;		/* tag */
@@ -131,6 +132,9 @@ static struct device_node *__of_unflatten_dtb(const void *infdt, bool constprops
 	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);
@@ -147,6 +151,9 @@ static struct device_node *__of_unflatten_dtb(const void *infdt, bool constprops
 	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 (f.off_dt_struct + f.size_dt_struct > f.totalsize) {
 		pr_err("unflatten: dt size exceeds total size\n");
 		return ERR_PTR(-ESPIPE);
@@ -274,9 +281,9 @@ err:
  * Parse a flat device tree binary blob and return a pointer to the unflattened
  * tree. The tree must be freed after use with of_delete_node().
  */
-struct device_node *of_unflatten_dtb(const void *infdt)
+struct device_node *of_unflatten_dtb(const void *infdt, int size)
 {
-	return __of_unflatten_dtb(infdt, false);
+	return __of_unflatten_dtb(infdt, size, false);
 }
 
 /**
@@ -290,9 +297,9 @@ struct device_node *of_unflatten_dtb(const void *infdt)
  * whole lifetime of the returned tree. This is normally not what you want, so
  * use of_unflatten_dtb() instead.
  */
-struct device_node *of_unflatten_dtb_const(const void *infdt)
+struct device_node *of_unflatten_dtb_const(const void *infdt, int size)
 {
-	return __of_unflatten_dtb(infdt, true);
+	return __of_unflatten_dtb(infdt, size, true);
 }
 
 struct fdt {
diff --git a/include/of.h b/include/of.h
index 645f429bde..74b82ae003 100644
--- a/include/of.h
+++ b/include/of.h
@@ -108,8 +108,8 @@ void of_print_nodes(struct device_node *node, int indent);
 void of_diff(struct device_node *a, struct device_node *b, int indent);
 int of_probe(void);
 int of_parse_dtb(struct fdt_header *fdt);
-struct device_node *of_unflatten_dtb(const void *fdt);
-struct device_node *of_unflatten_dtb_const(const void *infdt);
+struct device_node *of_unflatten_dtb(const void *fdt, int size);
+struct device_node *of_unflatten_dtb_const(const void *infdt, int size);
 
 struct cdev;
 
-- 
2.29.2


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

[PATCH 02/17] firmware: make device_node argument non const

[Sascha Hauer]

firmwaremgr_find_by_node() takes a const pointer to a device tree node.
Most functions that take a device tree node take a non const pointer
though, so we can't call them from there. It might be worth looking
into making the pointers const for other functions, but we are not
there yet. Make the pointer non const for now.

Signed-off-by: Sascha Hauer <s.hauer@pengutronix.de>
---
 common/firmware.c  | 2 +-
 include/firmware.h | 4 ++--
 2 files changed, 3 insertions(+), 3 deletions(-)

diff --git a/common/firmware.c b/common/firmware.c
index 58509d5da6..d2d6d6f97c 100644
--- a/common/firmware.c
+++ b/common/firmware.c
@@ -61,7 +61,7 @@ struct firmware_mgr *firmwaremgr_find(const char *id)
  * handler. This allows to retrieve the firmware handler with a phandle from
  * the device tree.
  */
-struct firmware_mgr *firmwaremgr_find_by_node(const struct device_node *np)
+struct firmware_mgr *firmwaremgr_find_by_node(struct device_node *np)
 {
 	struct firmware_mgr *mgr;
 
diff --git a/include/firmware.h b/include/firmware.h
index 19777d9bf7..4e69412832 100644
--- a/include/firmware.h
+++ b/include/firmware.h
@@ -27,9 +27,9 @@ int firmwaremgr_register(struct firmware_handler *);
 
 struct firmware_mgr *firmwaremgr_find(const char *);
 #ifdef CONFIG_FIRMWARE
-struct firmware_mgr *firmwaremgr_find_by_node(const struct device_node *np);
+struct firmware_mgr *firmwaremgr_find_by_node(struct device_node *np);
 #else
-static inline struct firmware_mgr *firmwaremgr_find_by_node(const struct device_node *np)
+static inline struct firmware_mgr *firmwaremgr_find_by_node(struct device_node *np)
 {
 	return NULL;
 }
-- 
2.29.2


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

[PATCH 03/17] libbb: Add find_path function

[Sascha Hauer]

libbb contains a find_execable() function to find an executable in a
colon separated path. The code can be reused by making the environment
variable name and the is-executable test parameters. Do this and add
a find_path()

Signed-off-by: Sascha Hauer <s.hauer@pengutronix.de>
---
 include/libbb.h |  4 +++-
 lib/libbb.c     | 58 ++++++++++++++++++++++++++++++++-----------------
 2 files changed, 41 insertions(+), 21 deletions(-)

diff --git a/include/libbb.h b/include/libbb.h
index a3a13b41ce..e191874052 100644
--- a/include/libbb.h
+++ b/include/libbb.h
@@ -8,8 +8,10 @@
 
 char *concat_path_file(const char *path, const char *filename);
 char *concat_subpath_file(const char *path, const char *f);
-int execable_file(const char *name);
+bool execable_file(const char *name);
 char *find_execable(const char *filename);
+char *find_path(const char *path, const char *filename,
+				bool (*filter)(const char *));
 char* last_char_is(const char *s, int c);
 
 enum {
diff --git a/lib/libbb.c b/lib/libbb.c
index d0c9bf4d80..642e54d78f 100644
--- a/lib/libbb.c
+++ b/lib/libbb.c
@@ -49,11 +49,47 @@ char *concat_subpath_file(const char *path, const char *f)
 }
 EXPORT_SYMBOL(concat_subpath_file);
 
+/**
+ * find_path - find a file in a colon separated path
+ * @path: The search path, colon separated
+ * @filename: The filename to search for
+ * @filter: filter function
+ *
+ * searches for @filename in a colon separated list of directories given in
+ * @path. @filter should return true when the current file matches the expectations,
+ * false otherwise. @filter should check for existence of the file, but could also
+ * check for additional flags.
+ */
+char *find_path(const char *path, const char *filename,
+		bool (*filter)(const char *))
+{
+	char *p, *n, *freep;
+
+	freep = p = strdup(path);
+	while (p) {
+		n = strchr(p, ':');
+		if (n)
+			*n++ = '\0';
+		if (*p != '\0') { /* it's not a PATH="foo::bar" situation */
+			p = concat_path_file(p, filename);
+			if (filter(p)) {
+				free(freep);
+				return p;
+			}
+			free(p);
+		}
+		p = n;
+	}
+	free(freep);
+	return NULL;
+}
+EXPORT_SYMBOL(find_path);
+
 /* check if path points to an executable file;
  * return 1 if found;
  * return 0 otherwise;
  */
-int execable_file(const char *name)
+bool execable_file(const char *name)
 {
 	struct stat s;
 	return (!stat(name, &s) && S_ISREG(s.st_mode));
@@ -67,25 +103,7 @@ EXPORT_SYMBOL(execable_file);
  */
 char *find_execable(const char *filename)
 {
-	char *path, *p, *n;
-
-	p = path = strdup(getenv("PATH"));
-	while (p) {
-		n = strchr(p, ':');
-		if (n)
-			*n++ = '\0';
-		if (*p != '\0') { /* it's not a PATH="foo::bar" situation */
-			p = concat_path_file(p, filename);
-			if (execable_file(p)) {
-				free(path);
-				return p;
-			}
-			free(p);
-		}
-		p = n;
-	}
-	free(path);
-	return NULL;
+	return find_path(getenv("PATH"), filename, execable_file);
 }
 EXPORT_SYMBOL(find_execable);
 
-- 
2.29.2


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

[PATCH 04/17] firmware: consolidate ifdefs

[Sascha Hauer]

Instead of having multiple #ifdef CONFIG_FIRMWARE in a single file move
the code together inside a single #ifdef.

Signed-off-by: Sascha Hauer <s.hauer@pengutronix.de>
---
 include/firmware.h | 9 +++------
 1 file changed, 3 insertions(+), 6 deletions(-)

diff --git a/include/firmware.h b/include/firmware.h
index 4e69412832..f09a64e7ed 100644
--- a/include/firmware.h
+++ b/include/firmware.h
@@ -28,24 +28,21 @@ int firmwaremgr_register(struct firmware_handler *);
 struct firmware_mgr *firmwaremgr_find(const char *);
 #ifdef CONFIG_FIRMWARE
 struct firmware_mgr *firmwaremgr_find_by_node(struct device_node *np);
+int firmwaremgr_load_file(struct firmware_mgr *, const char *path);
 #else
 static inline struct firmware_mgr *firmwaremgr_find_by_node(struct device_node *np)
 {
 	return NULL;
 }
-#endif
 
-void firmwaremgr_list_handlers(void);
-
-#ifdef CONFIG_FIRMWARE
-int firmwaremgr_load_file(struct firmware_mgr *, const char *path);
-#else
 static inline int firmwaremgr_load_file(struct firmware_mgr *mgr, const char *path)
 {
 	return -ENOSYS;
 }
 #endif
 
+void firmwaremgr_list_handlers(void);
+
 #define get_builtin_firmware(name, start, size) \
 	{							\
 		extern char _fw_##name##_start[];		\
-- 
2.29.2


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

[PATCH 05/17] firmware: Add search path

[Sascha Hauer]

This adds a colon separated search path for firmware files. When the
firmware we are searching is not an absolute path then look in the
search path first. This will be useful later when the bootloader spec
implementation shall look for firmware files relative to the provided
root.

Signed-off-by: Sascha Hauer <s.hauer@pengutronix.de>
---
 common/firmware.c  | 41 +++++++++++++++++++++++++++++++++++++++++
 include/firmware.h | 12 ++++++++++++
 2 files changed, 53 insertions(+)

diff --git a/common/firmware.c b/common/firmware.c
index d2d6d6f97c..008fb7720e 100644
--- a/common/firmware.c
+++ b/common/firmware.c
@@ -11,6 +11,8 @@
 #include <libbb.h>
 #include <libfile.h>
 #include <fs.h>
+#include <globalvar.h>
+#include <magicvar.h>
 #include <linux/list.h>
 #include <linux/stat.h>
 #include <linux/err.h>
@@ -206,17 +208,56 @@ out:
 	return ret;
 }
 
+static char *firmware_path;
+
+const char *firmware_get_searchpath(void)
+{
+	return firmware_path;
+}
+
+void firmware_set_searchpath(const char *path)
+{
+	free(firmware_path);
+	firmware_path = strdup(path);
+}
+
+static bool file_exists(const char *filename)
+{
+	struct stat s;
+
+	return !stat(filename, &s);
+}
+
 /*
  * firmware_load_file - load a firmware to a device
  */
 int firmwaremgr_load_file(struct firmware_mgr *mgr, const char *firmware)
 {
 	int ret;
+	char *fw = NULL;
 	char *name = basprintf("/dev/%s", mgr->handler->id);
 
+	if (*firmware != '/') {
+		fw = find_path(firmware_path, firmware, file_exists);
+		if (fw)
+			firmware = fw;
+	}
+
 	ret = copy_file(firmware, name, 0);
 
 	free(name);
+	free(fw);
 
 	return ret;
 }
+
+static int firmware_init(void)
+{
+	firmware_path = strdup("/env/firmware");
+	globalvar_add_simple_string("firmware.path", &firmware_path);
+
+	return 0;
+}
+device_initcall(firmware_init);
+
+BAREBOX_MAGICVAR(global.firmware.path, "Firmware search path");
diff --git a/include/firmware.h b/include/firmware.h
index f09a64e7ed..348fa4ee2d 100644
--- a/include/firmware.h
+++ b/include/firmware.h
@@ -29,6 +29,8 @@ struct firmware_mgr *firmwaremgr_find(const char *);
 #ifdef CONFIG_FIRMWARE
 struct firmware_mgr *firmwaremgr_find_by_node(struct device_node *np);
 int firmwaremgr_load_file(struct firmware_mgr *, const char *path);
+const char *firmware_get_searchpath(void);
+void firmware_set_searchpath(const char *path);
 #else
 static inline struct firmware_mgr *firmwaremgr_find_by_node(struct device_node *np)
 {
@@ -39,6 +41,16 @@ static inline int firmwaremgr_load_file(struct firmware_mgr *mgr, const char *pa
 {
 	return -ENOSYS;
 }
+
+static inline const char *firmware_get_searchpath(void)
+{
+	return NULL;
+}
+
+static inline void firmware_set_searchpath(const char *path)
+{
+}
+
 #endif
 
 void firmwaremgr_list_handlers(void);
-- 
2.29.2


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

[PATCH 06/17] firmware: Fix device_node matching

[Sascha Hauer]

firmwaremgr_find_by_node() matches against the device node of the parent
of the device associated to the handler. This is correct for the socfpga
and zyncmp driver, but not for the altera_serial driver. Add a
device_node argument to the handler which is set by the drivers to the
correct device node and match against that.

Signed-off-by: Sascha Hauer <s.hauer@pengutronix.de>
---
 common/firmware.c                | 2 +-
 drivers/firmware/altera_serial.c | 1 +
 drivers/firmware/socfpga.c       | 2 ++
 drivers/firmware/zynqmp-fpga.c   | 2 ++
 include/firmware.h               | 1 +
 5 files changed, 7 insertions(+), 1 deletion(-)

diff --git a/common/firmware.c b/common/firmware.c
index 008fb7720e..fce1f4b596 100644
--- a/common/firmware.c
+++ b/common/firmware.c
@@ -68,7 +68,7 @@ struct firmware_mgr *firmwaremgr_find_by_node(struct device_node *np)
 	struct firmware_mgr *mgr;
 
 	list_for_each_entry(mgr, &firmwaremgr_list, list)
-		if (mgr->handler->dev->parent->device_node == np)
+		if (mgr->handler->device_node == np)
 			return mgr;
 
 	return NULL;
diff --git a/drivers/firmware/altera_serial.c b/drivers/firmware/altera_serial.c
index 3a0175dd07..49460c6a70 100644
--- a/drivers/firmware/altera_serial.c
+++ b/drivers/firmware/altera_serial.c
@@ -371,6 +371,7 @@ static int altera_spi_probe(struct device_d *dev)
 	if (model)
 		fh->model = xstrdup(model);
 	fh->dev = dev;
+	fh->device_node = dev->device_node;
 
 	this->spi = (struct spi_device *)dev->type_data;
 	this->data = data;
diff --git a/drivers/firmware/socfpga.c b/drivers/firmware/socfpga.c
index 234fb2d094..4bcf079ab0 100644
--- a/drivers/firmware/socfpga.c
+++ b/drivers/firmware/socfpga.c
@@ -451,6 +451,8 @@ static int fpgamgr_probe(struct device_d *dev)
 	}
 
 	fh->dev = &mgr->dev;
+	fh->device_node = dev->device_node;
+
 	ret = firmwaremgr_register(fh);
 	if (ret != 0) {
 		free(mgr);
diff --git a/drivers/firmware/zynqmp-fpga.c b/drivers/firmware/zynqmp-fpga.c
index ab70d99933..0fc229bfd3 100644
--- a/drivers/firmware/zynqmp-fpga.c
+++ b/drivers/firmware/zynqmp-fpga.c
@@ -383,6 +383,8 @@ static int zynqmp_fpga_probe(struct device_d *dev)
 	}
 
 	fh->dev = &mgr->dev;
+	fh->device_node = dev->device_node;
+
 	ret = firmwaremgr_register(fh);
 	if (ret != 0) {
 		free(mgr);
diff --git a/include/firmware.h b/include/firmware.h
index 348fa4ee2d..a44208b634 100644
--- a/include/firmware.h
+++ b/include/firmware.h
@@ -13,6 +13,7 @@ struct firmware_handler {
 	char *id; /* unique identifier for this firmware device */
 	char *model; /* description for this device */
 	struct device_d *dev;
+	struct device_node *device_node;
 	/* called once to prepare the firmware's programming cycle */
 	int (*open)(struct firmware_handler*);
 	/* called multiple times to program the firmware with the given data */
-- 
2.29.2


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

[PATCH 07/17] firmware: recognize by reproducible name

[Sascha Hauer]

firmwaremgr_find_by_node() matches the device node pointers to find the
firmware manager associated to a node. This function is called by the
of_overlay code when it finds a firmware-name property to find a firmware
manager for this node. This works when the overlay is applied to the
live tree, but not when it's applied to the tree we are going to load
the kernel with. To overcome this limitation match by the nodes
reproducible name instead of pointers.

Signed-off-by: Sascha Hauer <s.hauer@pengutronix.de>
---
 common/firmware.c | 16 ++++++++++++++--
 1 file changed, 14 insertions(+), 2 deletions(-)

diff --git a/common/firmware.c b/common/firmware.c
index fce1f4b596..fc8ccd768c 100644
--- a/common/firmware.c
+++ b/common/firmware.c
@@ -66,10 +66,22 @@ struct firmware_mgr *firmwaremgr_find(const char *id)
 struct firmware_mgr *firmwaremgr_find_by_node(struct device_node *np)
 {
 	struct firmware_mgr *mgr;
+	char *na, *nb;
 
-	list_for_each_entry(mgr, &firmwaremgr_list, list)
-		if (mgr->handler->device_node == np)
+	na = of_get_reproducible_name(np);
+
+	list_for_each_entry(mgr, &firmwaremgr_list, list) {
+		nb = of_get_reproducible_name(mgr->handler->device_node);
+		if (!strcmp(na, nb)) {
+			free(na);
+			free(nb);
 			return mgr;
+		}
+
+		free(nb);
+	}
+
+	free(na);
 
 	return NULL;
 }
-- 
2.29.2


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

[PATCH 08/17] blspec: Set firmware searchpath

[Sascha Hauer]

When bootloader spec triggers dt overlay loading then this might
also trigger loading firmware. This firmware should be looked for
relative to the filesystem providing the bootloader spec files,
so add that to the firmware search path.

Signed-off-by: Sascha Hauer <s.hauer@pengutronix.de>
---
 common/blspec.c | 10 ++++++++++
 1 file changed, 10 insertions(+)

diff --git a/common/blspec.c b/common/blspec.c
index b6f91e3114..14e3238c0f 100644
--- a/common/blspec.c
+++ b/common/blspec.c
@@ -122,6 +122,8 @@ static int blspec_boot(struct bootentry *be, int verbose, int dryrun)
 	const char *abspath, *devicetree, *options, *initrd, *linuximage;
 	const char *overlays;
 	const char *appendroot;
+	const char *old_fws;
+	char *fws;
 	struct bootm_data data = {
 		.dryrun = dryrun,
 	};
@@ -184,9 +186,17 @@ static int blspec_boot(struct bootentry *be, int verbose, int dryrun)
 			(entry->cdev && entry->cdev->dev) ?
 			dev_name(entry->cdev->dev) : "none");
 
+	old_fws = firmware_get_searchpath();
+	fws = basprintf("%s/lib/firmware:%s", abspath, old_fws);
+	firmware_set_searchpath(fws);
+	free(fws);
+
 	ret = bootm_boot(&data);
 	if (ret)
 		pr_err("Booting failed\n");
+
+	firmware_set_searchpath(old_fws);
+
 err_out:
 	free((char *)data.oftree_file);
 	free((char *)data.initrd_file);
-- 
2.29.2


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

[PATCH 09/17] overlay: only apply compatible trees

[Sascha Hauer]

This adds support for a global.of_overlay_compatible variable which is a
space separated list of compatible strings. Device tree overlays not
matching any won't be applied (unless the overlay doesn't have a
compatible property at all).

Signed-off-by: Sascha Hauer <s.hauer@pengutronix.de>
---
 drivers/of/overlay.c | 43 +++++++++++++++++++++++++++++++++++++++++++
 1 file changed, 43 insertions(+)

diff --git a/drivers/of/overlay.c b/drivers/of/overlay.c
index 8f4ee3f0a2..7a89cbe948 100644
--- a/drivers/of/overlay.c
+++ b/drivers/of/overlay.c
@@ -11,6 +11,9 @@
 #include <common.h>
 #include <of.h>
 #include <errno.h>
+#include <globalvar.h>
+#include <magicvar.h>
+#include <string.h>
 
 static struct device_node *find_target(struct device_node *root,
 				       struct device_node *fragment)
@@ -160,6 +163,33 @@ static int of_overlay_apply_fragment(struct device_node *root,
 	return of_overlay_apply(target, overlay);
 }
 
+static char *of_overlay_compatible;
+
+static bool of_overlay_is_compatible(struct device_node *ovl)
+{
+	char *p, *n, *compatibles;
+	bool res = false;
+
+	if (!of_overlay_compatible || !*of_overlay_compatible)
+		return true;
+	if (!of_find_property(ovl, "compatible", NULL))
+		return true;
+
+	p = compatibles = xstrdup(of_overlay_compatible);
+
+	while ((n = strsep_unescaped(&p, " "))) {
+		if (of_device_is_compatible(ovl, n)) {
+			res = true;
+			break;
+		}
+	}
+
+	free(compatibles);
+
+	return res;
+}
+
+
 /**
  * Apply the overlay on the passed devicetree root
  * @root: the devicetree onto which the overlay will be applied
@@ -172,6 +202,9 @@ int of_overlay_apply_tree(struct device_node *root,
 	struct device_node *fragment;
 	int err = 0;
 
+	if (!of_overlay_is_compatible(overlay))
+		return -ENODEV;
+
 	resolved = of_resolve_phandles(root, overlay);
 	if (!resolved)
 		return -EINVAL;
@@ -250,3 +283,13 @@ int of_register_overlay(struct device_node *overlay)
 {
 	return of_register_fixup(of_overlay_fixup, overlay);
 }
+
+static int of_overlay_init(void)
+{
+	globalvar_add_simple_string("of_overlay_compatible", &of_overlay_compatible);
+
+	return 0;
+}
+device_initcall(of_overlay_init);
+
+BAREBOX_MAGICVAR(global.of_overlay_compatible, "space separated list of compatibles an overlay must match");
-- 
2.29.2


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

[PATCH 10/17] overlay: Add of_overlay_apply_file()

[Sascha Hauer]

A shortcut to of_overlay_apply_tree() which takes a filename rather than
an unflattened device tree.

Signed-off-by: Sascha Hauer <s.hauer@pengutronix.de>
---
 drivers/of/overlay.c | 35 +++++++++++++++++++++++++++++++++++
 include/of.h         |  7 +++++++
 2 files changed, 42 insertions(+)

diff --git a/drivers/of/overlay.c b/drivers/of/overlay.c
index 7a89cbe948..c9ede614a6 100644
--- a/drivers/of/overlay.c
+++ b/drivers/of/overlay.c
@@ -14,6 +14,8 @@
 #include <globalvar.h>
 #include <magicvar.h>
 #include <string.h>
+#include <libfile.h>
+#include <fs.h>
 
 static struct device_node *find_target(struct device_node *root,
 				       struct device_node *fragment)
@@ -224,6 +226,39 @@ int of_overlay_apply_tree(struct device_node *root,
 	return err;
 }
 
+int of_overlay_apply_file(struct device_node *root, const char *filename)
+{
+	void *fdt;
+	struct device_node *ovl;
+	size_t size;
+	int ret;
+
+	ret = read_file_2(filename, &size, &fdt, FILESIZE_MAX);
+	if (ret)
+		return ret;
+
+	ovl = of_unflatten_dtb(fdt, size);
+
+	free(fdt);
+
+	if (IS_ERR(ovl)) {
+		pr_err("Failed to unflatten %s: %pe\n", filename, ovl);
+		return PTR_ERR(ovl);
+	}
+
+	ret = of_overlay_apply_tree(root, ovl);
+	if (ret == -ENODEV)
+		pr_debug("Not applied %s (not compatible)\n", filename);
+	else if (ret)
+		pr_err("Cannot apply %s: %s\n", filename, strerror(-ret));
+	else
+		pr_info("Applied %s\n", filename);
+
+	of_delete_node(ovl);
+
+	return ret;
+}
+
 static int of_overlay_fixup(struct device_node *root, void *data)
 {
 	struct device_node *overlay = data;
diff --git a/include/of.h b/include/of.h
index 74b82ae003..e43e225338 100644
--- a/include/of.h
+++ b/include/of.h
@@ -1025,6 +1025,7 @@ struct device_node *of_resolve_phandles(struct device_node *root,
 					const struct device_node *overlay);
 int of_overlay_apply_tree(struct device_node *root,
 			  struct device_node *overlay);
+int of_overlay_apply_file(struct device_node *root, const char *filename);
 int of_register_overlay(struct device_node *overlay);
 int of_process_overlay(struct device_node *root,
 		    struct device_node *overlay,
@@ -1046,6 +1047,12 @@ static inline int of_overlay_apply_tree(struct device_node *root,
 	return -ENOSYS;
 }
 
+static inline int of_overlay_apply_file(struct device_node *root,
+					const char *filename)
+{
+	return -ENOSYS;
+}
+
 static inline int of_register_overlay(struct device_node *overlay)
 {
 	return -ENOSYS;
-- 
2.29.2


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

[PATCH 11/17] firmware: Load from global search path

[Sascha Hauer]

We have a global firmware search path, so use it. This removes the
path argument from of_firmware_load_overlay(). blspec already extends
the global firmware search path, so the path is not needed there.
The of_overlay command has an option for specifying the search path,
this is simply ignored from now on in favour for the global search path.

Signed-off-by: Sascha Hauer <s.hauer@pengutronix.de>
---
 commands/of_overlay.c    | 13 +++++--------
 common/blspec.c          | 11 +----------
 drivers/of/of_firmware.c | 25 ++++---------------------
 include/of.h             |  4 ++--
 4 files changed, 12 insertions(+), 41 deletions(-)

diff --git a/commands/of_overlay.c b/commands/of_overlay.c
index 9a4c008efc..9b1cc6e8c7 100644
--- a/commands/of_overlay.c
+++ b/commands/of_overlay.c
@@ -16,13 +16,12 @@ static int do_of_overlay(int argc, char *argv[])
 	int opt, ret;
 	struct fdt_header *fdt;
 	struct device_node *overlay;
-	const char *search_path = NULL;
 	size_t size;
 
 	while ((opt = getopt(argc, argv, "S:")) > 0) {
 		switch (opt) {
 		case 'S':
-			search_path = optarg;
+			/* Ignore for compatibility */
 			break;
 		default:
 			return COMMAND_ERROR_USAGE;
@@ -43,11 +42,9 @@ static int do_of_overlay(int argc, char *argv[])
 	if (IS_ERR(overlay))
 		return PTR_ERR(overlay);
 
-	if (search_path) {
-		ret = of_firmware_load_overlay(overlay, search_path);
-		if (ret)
-			goto err;
-	}
+	ret = of_firmware_load_overlay(overlay);
+	if (ret)
+		goto err;
 
 	ret = of_register_overlay(overlay);
 	if (ret) {
@@ -64,7 +61,7 @@ err:
 
 BAREBOX_CMD_HELP_START(of_overlay)
 BAREBOX_CMD_HELP_TEXT("Options:")
-BAREBOX_CMD_HELP_OPT("-S path", "load firmware using this search path")
+BAREBOX_CMD_HELP_OPT("-S path", "ignored")
 BAREBOX_CMD_HELP_END
 
 BAREBOX_CMD_START(of_overlay)
diff --git a/common/blspec.c b/common/blspec.c
index 14e3238c0f..d771176a45 100644
--- a/common/blspec.c
+++ b/common/blspec.c
@@ -39,7 +39,6 @@ static int blspec_apply_oftree_overlay(char *file, const char *abspath,
 	struct fdt_header *fdt;
 	struct device_node *overlay;
 	char *path;
-	char *firmware_path;
 	size_t size;
 
 	path = basprintf("%s/%s", abspath, file);
@@ -64,15 +63,7 @@ static int blspec_apply_oftree_overlay(char *file, const char *abspath,
 		goto out;
 	}
 
-	/*
-	 * Unfortunately the device tree overlay contains only the filename of
-	 * the firmware and relies on the firmware search paths to find the
-	 * actual file. Use /lib/firmware in the Linux root directory and hope
-	 * for the best.
-	 */
-	firmware_path = basprintf("%s/%s", abspath, "/lib/firmware");
-	ret = of_firmware_load_overlay(overlay, firmware_path);
-	free(firmware_path);
+	ret = of_firmware_load_overlay(overlay);
 	if (ret) {
 		pr_warn("failed to load firmware: skip overlay \"%s\"\n", path);
 		of_delete_node(overlay);
diff --git a/drivers/of/of_firmware.c b/drivers/of/of_firmware.c
index 096f84572e..12ce1d95d0 100644
--- a/drivers/of/of_firmware.c
+++ b/drivers/of/of_firmware.c
@@ -6,10 +6,6 @@
 #include <firmware.h>
 #include <of.h>
 
-struct overlay_info {
-	const char *firmware_path;
-};
-
 static struct firmware_mgr *of_node_get_mgr(struct device_node *np)
 {
 	struct device_node *mgr_node;
@@ -26,12 +22,9 @@ static struct firmware_mgr *of_node_get_mgr(struct device_node *np)
 }
 
 static int load_firmware(struct device_node *target,
-			 struct device_node *fragment, void *data)
+			 struct device_node *fragment, void *unused)
 {
-	struct overlay_info *info = data;
 	const char *firmware_name;
-	const char *firmware_path = info->firmware_path;
-	char *firmware;
 	int err;
 	struct firmware_mgr *mgr;
 
@@ -50,22 +43,13 @@ static int load_firmware(struct device_node *target,
 	if (!mgr)
 		return -EINVAL;
 
-	firmware = basprintf("%s/%s", firmware_path, firmware_name);
-	if (!firmware)
-		return -ENOMEM;
-
-	err = firmwaremgr_load_file(mgr, firmware);
-
-	free(firmware);
+	err = firmwaremgr_load_file(mgr, firmware_name);
 
 	return err;
 }
 
-int of_firmware_load_overlay(struct device_node *overlay, const char *path)
+int of_firmware_load_overlay(struct device_node *overlay)
 {
-	struct overlay_info info = {
-		.firmware_path = path,
-	};
 	int err;
 	struct device_node *root;
 	struct device_node *resolved;
@@ -81,8 +65,7 @@ int of_firmware_load_overlay(struct device_node *overlay, const char *path)
 	 */
 	ovl = resolved ? resolved : overlay;
 
-	err = of_process_overlay(root, ovl,
-				 load_firmware, &info);
+	err = of_process_overlay(root, ovl, load_firmware, NULL);
 
 	if (resolved)
 		of_delete_node(resolved);
diff --git a/include/of.h b/include/of.h
index e43e225338..56291b9801 100644
--- a/include/of.h
+++ b/include/of.h
@@ -1033,7 +1033,7 @@ int of_process_overlay(struct device_node *root,
 				   struct device_node *overlay, void *data),
 		    void *data);
 
-int of_firmware_load_overlay(struct device_node *overlay, const char *path);
+int of_firmware_load_overlay(struct device_node *overlay);
 #else
 static inline struct device_node *of_resolve_phandles(struct device_node *root,
 					const struct device_node *overlay)
@@ -1067,7 +1067,7 @@ static inline int of_process_overlay(struct device_node *root,
 	return -ENOSYS;
 }
 
-static inline int of_firmware_load_overlay(struct device_node *overlay, const char *path)
+static inline int of_firmware_load_overlay(struct device_node *overlay)
 {
 	return -ENOSYS;
 }
-- 
2.29.2


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

[PATCH 12/17] blspec: Rework firmware load

[Sascha Hauer]

Applying overlays in blspec currently works in two steps. First
of_firmware_load_overlay() is called which doesn't load an overlay,
but instead loads firmware when one is needed by the overlay. This
is done on the live tree, because that was needed to find the firmware
manager. The second step is to call of_register_overlay() to apply
the overlay to the kernel device tree when the fixups are executed.

Instead of using a separate step to load the firmware, load the firmware
as part of the of_fixups.

Signed-off-by: Sascha Hauer <s.hauer@pengutronix.de>
---
 commands/of_overlay.c    |  4 ---
 common/blspec.c          | 73 +++++++++++-----------------------------
 drivers/of/of_firmware.c | 24 ++-----------
 drivers/of/overlay.c     |  2 ++
 include/of.h             |  4 +--
 5 files changed, 25 insertions(+), 82 deletions(-)

diff --git a/commands/of_overlay.c b/commands/of_overlay.c
index 9b1cc6e8c7..17b0c9f4cb 100644
--- a/commands/of_overlay.c
+++ b/commands/of_overlay.c
@@ -42,10 +42,6 @@ static int do_of_overlay(int argc, char *argv[])
 	if (IS_ERR(overlay))
 		return PTR_ERR(overlay);
 
-	ret = of_firmware_load_overlay(overlay);
-	if (ret)
-		goto err;
-
 	ret = of_register_overlay(overlay);
 	if (ret) {
 		printf("cannot apply oftree overlay: %s\n", strerror(-ret));
diff --git a/common/blspec.c b/common/blspec.c
index d771176a45..fd3e088920 100644
--- a/common/blspec.c
+++ b/common/blspec.c
@@ -32,71 +32,33 @@ int blspec_entry_var_set(struct blspec_entry *entry, const char *name,
 			val ? strlen(val) + 1 : 0, 1);
 }
 
-static int blspec_apply_oftree_overlay(char *file, const char *abspath,
-				       int dryrun)
-{
-	int ret = 0;
-	struct fdt_header *fdt;
-	struct device_node *overlay;
-	char *path;
-	size_t size;
-
-	path = basprintf("%s/%s", abspath, file);
-
-	fdt = read_file(path, &size);
-	if (!fdt) {
-		pr_warn("unable to read \"%s\"\n", path);
-		ret = -EINVAL;
-		goto out;
-	}
-
-	overlay = of_unflatten_dtb(fdt, size);
-	free(fdt);
-	if (IS_ERR(overlay)) {
-		ret = PTR_ERR(overlay);
-		goto out;
-	}
-
-	if (dryrun) {
-		pr_info("dry run: skip overlay %s\n", path);
-		of_delete_node(overlay);
-		goto out;
-	}
-
-	ret = of_firmware_load_overlay(overlay);
-	if (ret) {
-		pr_warn("failed to load firmware: skip overlay \"%s\"\n", path);
-		of_delete_node(overlay);
-		goto out;
-	}
-
-	ret = of_register_overlay(overlay);
-	if (ret) {
-		pr_warn("cannot register devicetree overlay \"%s\"\n", path);
-		of_delete_node(overlay);
-	}
-
-out:
-	free(path);
-
-	return ret;
-}
-
-static void blspec_apply_oftree_overlays(const char *overlays,
-					 const char *abspath, int dryrun)
+static int blspec_overlay_fixup(struct device_node *root, void *ctx)
 {
+	struct blspec_entry *entry = ctx;
+	const char *overlays;
 	char *overlay;
 	char *sep, *freep;
 
+	overlays = blspec_entry_var_get(entry, "devicetree-overlay");
+
 	sep = freep = xstrdup(overlays);
 
 	while ((overlay = strsep(&sep, " "))) {
+		char *path;
+
 		if (!*overlay)
 			continue;
-		blspec_apply_oftree_overlay(overlay, abspath, dryrun);
+
+		path = basprintf("%s/%s", entry->rootpath, overlay);
+
+		of_overlay_apply_file(root, path);
+
+		free(path);
 	}
 
 	free(freep);
+
+	return 0;
 }
 
 /*
@@ -153,7 +115,7 @@ static int blspec_boot(struct bootentry *be, int verbose, int dryrun)
 	}
 
 	if (overlays)
-		blspec_apply_oftree_overlays(overlays, abspath, dryrun);
+		of_register_fixup(blspec_overlay_fixup, entry);
 
 	if (initrd)
 		data.initrd_file = basprintf("%s/%s", abspath, initrd);
@@ -186,6 +148,9 @@ static int blspec_boot(struct bootentry *be, int verbose, int dryrun)
 	if (ret)
 		pr_err("Booting failed\n");
 
+	if (overlays)
+		of_unregister_fixup(blspec_overlay_fixup, entry);
+
 	firmware_set_searchpath(old_fws);
 
 err_out:
diff --git a/drivers/of/of_firmware.c b/drivers/of/of_firmware.c
index 12ce1d95d0..d66f0ae7ce 100644
--- a/drivers/of/of_firmware.c
+++ b/drivers/of/of_firmware.c
@@ -48,27 +48,7 @@ static int load_firmware(struct device_node *target,
 	return err;
 }
 
-int of_firmware_load_overlay(struct device_node *overlay)
+int of_overlay_load_firmware(struct device_node *root, struct device_node *overlay)
 {
-	int err;
-	struct device_node *root;
-	struct device_node *resolved;
-	struct device_node *ovl;
-
-	root = of_get_root_node();
-	resolved = of_resolve_phandles(root, overlay);
-	/*
-	 * If the overlay cannot be resolved, use the load_firmware callback
-	 * with the unresolved overlay to verify that the overlay does not
-	 * depend on a firmware to be loaded. If a required firmware cannot be
-	 * loaded, the overlay must not be applied.
-	 */
-	ovl = resolved ? resolved : overlay;
-
-	err = of_process_overlay(root, ovl, load_firmware, NULL);
-
-	if (resolved)
-		of_delete_node(resolved);
-
-	return err;
+	return of_process_overlay(root, overlay, load_firmware, NULL);
 }
diff --git a/drivers/of/overlay.c b/drivers/of/overlay.c
index c9ede614a6..d42e15ed1c 100644
--- a/drivers/of/overlay.c
+++ b/drivers/of/overlay.c
@@ -221,6 +221,8 @@ int of_overlay_apply_tree(struct device_node *root,
 			pr_warn("failed to apply %s\n", fragment->name);
 	}
 
+	err = of_overlay_load_firmware(root, resolved);
+
 	of_delete_node(resolved);
 
 	return err;
diff --git a/include/of.h b/include/of.h
index 56291b9801..ecc5f5101a 100644
--- a/include/of.h
+++ b/include/of.h
@@ -1033,7 +1033,7 @@ int of_process_overlay(struct device_node *root,
 				   struct device_node *overlay, void *data),
 		    void *data);
 
-int of_firmware_load_overlay(struct device_node *overlay);
+int of_overlay_load_firmware(struct device_node *root, struct device_node *overlay);
 #else
 static inline struct device_node *of_resolve_phandles(struct device_node *root,
 					const struct device_node *overlay)
@@ -1067,7 +1067,7 @@ static inline int of_process_overlay(struct device_node *root,
 	return -ENOSYS;
 }
 
-static inline int of_firmware_load_overlay(struct device_node *overlay)
+static inline int of_overlay_load_firmware(struct device_node *root, struct device_node *overlay)
 {
 	return -ENOSYS;
 }
-- 
2.29.2


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

[PATCH 13/17] of_overlay: apply overlays during booting

[Sascha Hauer]

This offers the possibility to load dt overlays on the kernel device
tree during booting. Several globalvars control which device trees
are used. global.of_overlay_dir specifies the directory where overlay
files are expected. global.of_overlay_filepattern specifies file
patterns which the overlay files must match. It is a colon separated
list for multiple file patterns. The patterns may contain '*' and '?'
wildcards. When this variable is empty then no overlays are applied.
global.of_overlay_compatible is used to apply only compatible overlays.
This is also a colon separated list which contains compatible strings.
An overlay is only applied when it's compatible to one of the strings.

Signed-off-by: Sascha Hauer <s.hauer@pengutronix.de>
---
 drivers/of/overlay.c | 66 ++++++++++++++++++++++++++++++++++++++++++++
 include/of.h         | 18 ++++++++++++
 2 files changed, 84 insertions(+)

diff --git a/drivers/of/overlay.c b/drivers/of/overlay.c
index d42e15ed1c..b0916028fe 100644
--- a/drivers/of/overlay.c
+++ b/drivers/of/overlay.c
@@ -16,6 +16,7 @@
 #include <string.h>
 #include <libfile.h>
 #include <fs.h>
+#include <glob.h>
 
 static struct device_node *find_target(struct device_node *root,
 				       struct device_node *fragment)
@@ -321,12 +322,77 @@ int of_register_overlay(struct device_node *overlay)
 	return of_register_fixup(of_overlay_fixup, overlay);
 }
 
+static char *of_overlay_filepattern;
+static char *of_overlay_dir;
+
+const char *of_overlay_get_dir(void)
+{
+	return of_overlay_dir;
+}
+
+void of_overlay_set_dir(const char *path)
+{
+	free(of_overlay_dir);
+	of_overlay_dir = strdup(path);
+}
+
+int of_overlay_apply_dir(struct device_node *root, const char *dir)
+{
+	char *p, *path, *n, *str;
+	int ret = 0, i;
+	glob_t g = {};
+	int globflags = GLOB_NOCHECK;
+
+	if (!dir || !*dir)
+		return 0;
+
+	p = path = strdup(of_overlay_filepattern);
+
+	while ((n = strsep_unescaped(&p, " "))) {
+		str = basprintf("%s/%s", dir, n);
+		if (!str) {
+			ret = -ENOMEM;
+			goto out;
+		}
+
+		ret = glob(str, globflags, NULL, &g);
+
+		free(str);
+
+		if (ret)
+			goto out;
+
+		globflags |= GLOB_APPEND;
+	}
+
+	for (i = 0; i < g.gl_pathc; i++)
+		of_overlay_apply_file(root, g.gl_pathv[i]);
+
+out:
+	globfree(&g);
+
+	free(path);
+
+	return ret;
+}
+
+static int of_overlay_global_fixup(struct device_node *root, void *data)
+{
+	return of_overlay_apply_dir(root, of_overlay_dir);
+}
+
 static int of_overlay_init(void)
 {
 	globalvar_add_simple_string("of_overlay_compatible", &of_overlay_compatible);
+	globalvar_add_simple_string("of_overlay_filepattern", &of_overlay_filepattern);
+	globalvar_add_simple_string("of_overlay_dir", &of_overlay_dir);
+
+	of_register_fixup(of_overlay_global_fixup, NULL);
 
 	return 0;
 }
 device_initcall(of_overlay_init);
 
 BAREBOX_MAGICVAR(global.of_overlay_compatible, "space separated list of compatibles an overlay must match");
+BAREBOX_MAGICVAR(global.of_overlay_filepattern, "space separated list of filepatterns an overlay must match");
+BAREBOX_MAGICVAR(global.of_overlay_dir, "Directory to look for dt overlays");
diff --git a/include/of.h b/include/of.h
index ecc5f5101a..54185076a8 100644
--- a/include/of.h
+++ b/include/of.h
@@ -1034,6 +1034,9 @@ int of_process_overlay(struct device_node *root,
 		    void *data);
 
 int of_overlay_load_firmware(struct device_node *root, struct device_node *overlay);
+int of_overlay_apply_dir(struct device_node *root, const char *dir);
+const char *of_overlay_get_dir(void);
+void of_overlay_set_dir(const char *path);
 #else
 static inline struct device_node *of_resolve_phandles(struct device_node *root,
 					const struct device_node *overlay)
@@ -1071,6 +1074,21 @@ static inline int of_overlay_load_firmware(struct device_node *root, struct devi
 {
 	return -ENOSYS;
 }
+
+static inline int of_overlay_apply_dir(struct device_node *root, const char *dir)
+{
+	return 0;
+}
+
+static inline const char *of_overlay_get_dir(void)
+{
+	return NULL;
+}
+
+static inline void of_overlay_set_dir(const char *path)
+{
+}
+
 #endif
 
 #endif /* __OF_H */
-- 
2.29.2


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

[PATCH 14/17] blspec: Apply overlays from rootfs

[Sascha Hauer]

This sets the overlay search path to $BOOT/overlays during starting
an bootloader spec entry with the effect that overlays from there
can be applied.

Signed-off-by: Sascha Hauer <s.hauer@pengutronix.de>
---
 common/blspec.c | 10 ++++++++--
 1 file changed, 8 insertions(+), 2 deletions(-)

diff --git a/common/blspec.c b/common/blspec.c
index fd3e088920..ded149d715 100644
--- a/common/blspec.c
+++ b/common/blspec.c
@@ -75,8 +75,8 @@ static int blspec_boot(struct bootentry *be, int verbose, int dryrun)
 	const char *abspath, *devicetree, *options, *initrd, *linuximage;
 	const char *overlays;
 	const char *appendroot;
-	const char *old_fws;
-	char *fws;
+	const char *old_fws, *old_ovld;
+	char *fws, *ovld;
 	struct bootm_data data = {
 		.dryrun = dryrun,
 	};
@@ -139,6 +139,11 @@ static int blspec_boot(struct bootentry *be, int verbose, int dryrun)
 			(entry->cdev && entry->cdev->dev) ?
 			dev_name(entry->cdev->dev) : "none");
 
+	old_ovld = of_overlay_get_dir();
+	ovld = basprintf("%s/overlays", abspath);
+	of_overlay_set_dir(ovld);
+	free(ovld);
+
 	old_fws = firmware_get_searchpath();
 	fws = basprintf("%s/lib/firmware:%s", abspath, old_fws);
 	firmware_set_searchpath(fws);
@@ -151,6 +156,7 @@ static int blspec_boot(struct bootentry *be, int verbose, int dryrun)
 	if (overlays)
 		of_unregister_fixup(blspec_overlay_fixup, entry);
 
+	of_overlay_set_dir(old_ovld);
 	firmware_set_searchpath(old_fws);
 
 err_out:
-- 
2.29.2


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

[PATCH 15/17] doc: devicetree: Refer to internal device tree also as live tree

[Sascha Hauer]

Signed-off-by: Sascha Hauer <s.hauer@pengutronix.de>
---
 Documentation/user/devicetree.rst | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/Documentation/user/devicetree.rst b/Documentation/user/devicetree.rst
index 679cae7f00..4be6a1e703 100644
--- a/Documentation/user/devicetree.rst
+++ b/Documentation/user/devicetree.rst
@@ -21,7 +21,7 @@ The internal devicetree
 -----------------------
 
 The devicetree consulted by barebox plays a special role. It is referred to
-as the "internal devicetree." The barebox devicetree commands work on this
+as the "internal devicetree" or "live tree". The barebox devicetree commands work on this
 devicetree. The devicetree source (DTS) files are kept in sync with the kernel DTS
 files. As the FDT files are meant to be backward compatible, it should always be possible
 to start a kernel with the barebox internal devicetree. However, since the barebox
-- 
2.29.2


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

[PATCH 16/17] Documentation: Add documentation for device tree overlays

[Sascha Hauer]

Document the recently gained device tree overlay features.

Signed-off-by: Sascha Hauer <s.hauer@pengutronix.de>
---
 Documentation/user/devicetree.rst | 37 +++++++++++++++++++++++++++++++
 1 file changed, 37 insertions(+)

diff --git a/Documentation/user/devicetree.rst b/Documentation/user/devicetree.rst
index 4be6a1e703..d78394b7aa 100644
--- a/Documentation/user/devicetree.rst
+++ b/Documentation/user/devicetree.rst
@@ -75,3 +75,40 @@ It is important to know that these commands normally work on the internal
 devicetree. If you want to modify the devicetree the kernel is started with
 see the -f options to of_property and of_node. This option will register the
 operation for later execution on the Kernel devicetree.
+
+Device tree overlays
+--------------------
+
+barebox has support for device tree overlays. barebox knows two different trees,
+the live tree and the device tree the kernel is started with. Both can be applied
+overlays to.
+
+Device tree overlays on the live tree
+.....................................
+
+While the live tree can be patched by board code, barebox does not
+detect any changes to the live tree. To let the overlays have any effect, board
+code must make sure the live tree is patched before the devices are instanciated
+from it.
+
+Device tree overlays on the kernel device tree
+..............................................
+
+Overlays can be applied to the kernel device tree before it is handed over to
+the kernel. The behaviour is controlled by different variables:
+
+``global.of_overlay_dir``
+  Overlays are read from this directory. barebox will try to apply all overlays
+  found here if not limited by one of the other variables below. When using
+  bootloader spec this variable will be temporarily overwritten with
+  ``$mountpoint/overlays`` where ``$mountpoint`` is the path where the filesystem
+  containing the bootloader spec entries are located.
+``global.of_overlay_compatible``
+  This is a space separated list of compatibles. Only overlays matching one of
+  these compatibles will be applied. When this list is empty then all overlays
+  will be applied. Overlays that don't have a compatible are considered being
+  always compatible.
+``global.of_overlay_filepattern``
+  This is a space separated list of file patterns. An overlay is only applied
+  when its filename matches one of the patterns. The patterns can contain
+  ``*`` and ``?`` as wildcards. When empty, all files are applied.
-- 
2.29.2


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

[PATCH 17/17] of_firmware: Fix handling of firmware-name property

[Sascha Hauer]

The fpga-region binding states that the firmware-name property must be
in the same node as the fpga-region compatible, so checking for this
compatible when walking up the parents is wrong. Instead, only test for
it in the target node. Also, when the current node is not a fpga-region
we must return successfully.

Signed-off-by: Sascha Hauer <s.hauer@pengutronix.de>
---
 drivers/of/of_firmware.c | 11 ++++++-----
 1 file changed, 6 insertions(+), 5 deletions(-)

diff --git a/drivers/of/of_firmware.c b/drivers/of/of_firmware.c
index d66f0ae7ce..cf288a0a88 100644
--- a/drivers/of/of_firmware.c
+++ b/drivers/of/of_firmware.c
@@ -11,11 +11,9 @@ static struct firmware_mgr *of_node_get_mgr(struct device_node *np)
 	struct device_node *mgr_node;
 
 	do {
-		if (of_device_is_compatible(np, "fpga-region")) {
-			mgr_node = of_parse_phandle(np, "fpga-mgr", 0);
-			if (mgr_node)
-				return firmwaremgr_find_by_node(mgr_node);
-		}
+		mgr_node = of_parse_phandle(np, "fpga-mgr", 0);
+		if (mgr_node)
+			return firmwaremgr_find_by_node(mgr_node);
 	} while ((np = of_get_parent(np)) != NULL);
 
 	return NULL;
@@ -39,6 +37,9 @@ static int load_firmware(struct device_node *target,
 	if (!target)
 		return -EINVAL;
 
+	if (!of_device_is_compatible(target, "fpga-region"))
+		return 0;
+
 	mgr = of_node_get_mgr(target);
 	if (!mgr)
 		return -EINVAL;
-- 
2.29.2


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

Re: [PATCH 09/17] overlay: only apply compatible trees

[Ahmad Fatoum]

On 10.03.21 14:28, Sascha Hauer wrote:
> This adds support for a global.of_overlay_compatible variable which is a
> space separated list of compatible strings. Device tree overlays not
> matching any won't be applied (unless the overlay doesn't have a
> compatible property at all).
> 
> Signed-off-by: Sascha Hauer <s.hauer@pengutronix.de>
> ---
>  drivers/of/overlay.c | 43 +++++++++++++++++++++++++++++++++++++++++++
>  1 file changed, 43 insertions(+)
> 
> diff --git a/drivers/of/overlay.c b/drivers/of/overlay.c
> index 8f4ee3f0a2..7a89cbe948 100644
> --- a/drivers/of/overlay.c
> +++ b/drivers/of/overlay.c
> @@ -11,6 +11,9 @@
>  #include <common.h>
>  #include <of.h>
>  #include <errno.h>
> +#include <globalvar.h>
> +#include <magicvar.h>
> +#include <string.h>
>  
>  static struct device_node *find_target(struct device_node *root,
>  				       struct device_node *fragment)
> @@ -160,6 +163,33 @@ static int of_overlay_apply_fragment(struct device_node *root,
>  	return of_overlay_apply(target, overlay);
>  }
>  
> +static char *of_overlay_compatible;
> +
> +static bool of_overlay_is_compatible(struct device_node *ovl)
> +{
> +	char *p, *n, *compatibles;
> +	bool res = false;
> +
> +	if (!of_overlay_compatible || !*of_overlay_compatible)
> +		return true;
> +	if (!of_find_property(ovl, "compatible", NULL))
> +		return true;
> +
> +	p = compatibles = xstrdup(of_overlay_compatible);
> +
> +	while ((n = strsep_unescaped(&p, " "))) {

This returns an empty string when two delimiters are in a row and it seems
of_device_is_compatible will return true when comparing with the empty
string. Probably not what you want here.

> +		if (of_device_is_compatible(ovl, n)) {
> +			res = true;
> +			break;
> +		}
> +	}
> +
> +	free(compatibles);
> +
> +	return res;
> +}
> +
> +
>  /**
>   * Apply the overlay on the passed devicetree root
>   * @root: the devicetree onto which the overlay will be applied
> @@ -172,6 +202,9 @@ int of_overlay_apply_tree(struct device_node *root,
>  	struct device_node *fragment;
>  	int err = 0;
>  
> +	if (!of_overlay_is_compatible(overlay))
> +		return -ENODEV;
> +
>  	resolved = of_resolve_phandles(root, overlay);
>  	if (!resolved)
>  		return -EINVAL;
> @@ -250,3 +283,13 @@ int of_register_overlay(struct device_node *overlay)
>  {
>  	return of_register_fixup(of_overlay_fixup, overlay);
>  }
> +
> +static int of_overlay_init(void)
> +{
> +	globalvar_add_simple_string("of_overlay_compatible", &of_overlay_compatible);
> +
> +	return 0;
> +}
> +device_initcall(of_overlay_init);
> +
> +BAREBOX_MAGICVAR(global.of_overlay_compatible, "space separated list of compatibles an overlay must match");
> 

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

Re: [PATCH 11/17] firmware: Load from global search path

[Ahmad Fatoum]

Hi,

On 10.03.21 14:28, Sascha Hauer wrote:
> We have a global firmware search path, so use it. This removes the
> path argument from of_firmware_load_overlay(). blspec already extends
> the global firmware search path, so the path is not needed there.
> The of_overlay command has an option for specifying the search path,
> this is simply ignored from now on in favour for the global search path.
> 
> Signed-off-by: Sascha Hauer <s.hauer@pengutronix.de>
> ---
>  commands/of_overlay.c    | 13 +++++--------
>  common/blspec.c          | 11 +----------
>  drivers/of/of_firmware.c | 25 ++++---------------------
>  include/of.h             |  4 ++--
>  4 files changed, 12 insertions(+), 41 deletions(-)
> 
> diff --git a/commands/of_overlay.c b/commands/of_overlay.c
> index 9a4c008efc..9b1cc6e8c7 100644
> --- a/commands/of_overlay.c
> +++ b/commands/of_overlay.c
> @@ -16,13 +16,12 @@ static int do_of_overlay(int argc, char *argv[])
>  	int opt, ret;
>  	struct fdt_header *fdt;
>  	struct device_node *overlay;
> -	const char *search_path = NULL;
>  	size_t size;
>  
>  	while ((opt = getopt(argc, argv, "S:")) > 0) {
>  		switch (opt) {
>  		case 'S':
> -			search_path = optarg;
> +			/* Ignore for compatibility */

If you really want to maintain compatibility, why don't do a setenv?
Keeping the flag, but ignoring the argument sounds very surprising.

>  			break;
>  		default:
>  			return COMMAND_ERROR_USAGE;
> @@ -43,11 +42,9 @@ static int do_of_overlay(int argc, char *argv[])
>  	if (IS_ERR(overlay))
>  		return PTR_ERR(overlay);
>  
> -	if (search_path) {
> -		ret = of_firmware_load_overlay(overlay, search_path);
> -		if (ret)
> -			goto err;
> -	}
> +	ret = of_firmware_load_overlay(overlay);
> +	if (ret)
> +		goto err;
>  
>  	ret = of_register_overlay(overlay);
>  	if (ret) {
> @@ -64,7 +61,7 @@ err:
>  
>  BAREBOX_CMD_HELP_START(of_overlay)
>  BAREBOX_CMD_HELP_TEXT("Options:")
> -BAREBOX_CMD_HELP_OPT("-S path", "load firmware using this search path")
> +BAREBOX_CMD_HELP_OPT("-S path", "ignored")
>  BAREBOX_CMD_HELP_END
>  
>  BAREBOX_CMD_START(of_overlay)
> diff --git a/common/blspec.c b/common/blspec.c
> index 14e3238c0f..d771176a45 100644
> --- a/common/blspec.c
> +++ b/common/blspec.c
> @@ -39,7 +39,6 @@ static int blspec_apply_oftree_overlay(char *file, const char *abspath,
>  	struct fdt_header *fdt;
>  	struct device_node *overlay;
>  	char *path;
> -	char *firmware_path;
>  	size_t size;
>  
>  	path = basprintf("%s/%s", abspath, file);
> @@ -64,15 +63,7 @@ static int blspec_apply_oftree_overlay(char *file, const char *abspath,
>  		goto out;
>  	}
>  
> -	/*
> -	 * Unfortunately the device tree overlay contains only the filename of
> -	 * the firmware and relies on the firmware search paths to find the
> -	 * actual file. Use /lib/firmware in the Linux root directory and hope
> -	 * for the best.
> -	 */
> -	firmware_path = basprintf("%s/%s", abspath, "/lib/firmware");
> -	ret = of_firmware_load_overlay(overlay, firmware_path);
> -	free(firmware_path);
> +	ret = of_firmware_load_overlay(overlay);
>  	if (ret) {
>  		pr_warn("failed to load firmware: skip overlay \"%s\"\n", path);
>  		of_delete_node(overlay);
> diff --git a/drivers/of/of_firmware.c b/drivers/of/of_firmware.c
> index 096f84572e..12ce1d95d0 100644
> --- a/drivers/of/of_firmware.c
> +++ b/drivers/of/of_firmware.c
> @@ -6,10 +6,6 @@
>  #include <firmware.h>
>  #include <of.h>
>  
> -struct overlay_info {
> -	const char *firmware_path;
> -};
> -
>  static struct firmware_mgr *of_node_get_mgr(struct device_node *np)
>  {
>  	struct device_node *mgr_node;
> @@ -26,12 +22,9 @@ static struct firmware_mgr *of_node_get_mgr(struct device_node *np)
>  }
>  
>  static int load_firmware(struct device_node *target,
> -			 struct device_node *fragment, void *data)
> +			 struct device_node *fragment, void *unused)
>  {
> -	struct overlay_info *info = data;
>  	const char *firmware_name;
> -	const char *firmware_path = info->firmware_path;
> -	char *firmware;
>  	int err;
>  	struct firmware_mgr *mgr;
>  
> @@ -50,22 +43,13 @@ static int load_firmware(struct device_node *target,
>  	if (!mgr)
>  		return -EINVAL;
>  
> -	firmware = basprintf("%s/%s", firmware_path, firmware_name);
> -	if (!firmware)
> -		return -ENOMEM;
> -
> -	err = firmwaremgr_load_file(mgr, firmware);
> -
> -	free(firmware);
> +	err = firmwaremgr_load_file(mgr, firmware_name);
>  
>  	return err;
>  }
>  
> -int of_firmware_load_overlay(struct device_node *overlay, const char *path)
> +int of_firmware_load_overlay(struct device_node *overlay)
>  {
> -	struct overlay_info info = {
> -		.firmware_path = path,
> -	};
>  	int err;
>  	struct device_node *root;
>  	struct device_node *resolved;
> @@ -81,8 +65,7 @@ int of_firmware_load_overlay(struct device_node *overlay, const char *path)
>  	 */
>  	ovl = resolved ? resolved : overlay;
>  
> -	err = of_process_overlay(root, ovl,
> -				 load_firmware, &info);
> +	err = of_process_overlay(root, ovl, load_firmware, NULL);
>  
>  	if (resolved)
>  		of_delete_node(resolved);
> diff --git a/include/of.h b/include/of.h
> index e43e225338..56291b9801 100644
> --- a/include/of.h
> +++ b/include/of.h
> @@ -1033,7 +1033,7 @@ int of_process_overlay(struct device_node *root,
>  				   struct device_node *overlay, void *data),
>  		    void *data);
>  
> -int of_firmware_load_overlay(struct device_node *overlay, const char *path);
> +int of_firmware_load_overlay(struct device_node *overlay);
>  #else
>  static inline struct device_node *of_resolve_phandles(struct device_node *root,
>  					const struct device_node *overlay)
> @@ -1067,7 +1067,7 @@ static inline int of_process_overlay(struct device_node *root,
>  	return -ENOSYS;
>  }
>  
> -static inline int of_firmware_load_overlay(struct device_node *overlay, const char *path)
> +static inline int of_firmware_load_overlay(struct device_node *overlay)
>  {
>  	return -ENOSYS;
>  }
> 

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

Re: [PATCH 12/17] blspec: Rework firmware load

[Michael Tretter]

On Wed, 10 Mar 2021 14:28:24 +0100, Sascha Hauer wrote:
> Applying overlays in blspec currently works in two steps. First
> of_firmware_load_overlay() is called which doesn't load an overlay,
> but instead loads firmware when one is needed by the overlay. This
> is done on the live tree, because that was needed to find the firmware
> manager. The second step is to call of_register_overlay() to apply
> the overlay to the kernel device tree when the fixups are executed.
> 
> Instead of using a separate step to load the firmware, load the firmware
> as part of the of_fixups.

Wouldn't this result in the firmware being loaded whenever of_fix_tree is
called? Then, every use of the of_dump or of_diff commands would result in a
reload of the firmware.

> 
> Signed-off-by: Sascha Hauer <s.hauer@pengutronix.de>
> ---
>  commands/of_overlay.c    |  4 ---
>  common/blspec.c          | 73 +++++++++++-----------------------------
>  drivers/of/of_firmware.c | 24 ++-----------
>  drivers/of/overlay.c     |  2 ++
>  include/of.h             |  4 +--
>  5 files changed, 25 insertions(+), 82 deletions(-)
> 
> diff --git a/commands/of_overlay.c b/commands/of_overlay.c
> index 9b1cc6e8c7..17b0c9f4cb 100644
> --- a/commands/of_overlay.c
> +++ b/commands/of_overlay.c
> @@ -42,10 +42,6 @@ static int do_of_overlay(int argc, char *argv[])
>  	if (IS_ERR(overlay))
>  		return PTR_ERR(overlay);
>  
> -	ret = of_firmware_load_overlay(overlay);
> -	if (ret)
> -		goto err;
> -
>  	ret = of_register_overlay(overlay);
>  	if (ret) {
>  		printf("cannot apply oftree overlay: %s\n", strerror(-ret));
> diff --git a/common/blspec.c b/common/blspec.c
> index d771176a45..fd3e088920 100644
> --- a/common/blspec.c
> +++ b/common/blspec.c
> @@ -32,71 +32,33 @@ int blspec_entry_var_set(struct blspec_entry *entry, const char *name,
>  			val ? strlen(val) + 1 : 0, 1);
>  }
>  
> -static int blspec_apply_oftree_overlay(char *file, const char *abspath,
> -				       int dryrun)
> -{
> -	int ret = 0;
> -	struct fdt_header *fdt;
> -	struct device_node *overlay;
> -	char *path;
> -	size_t size;
> -
> -	path = basprintf("%s/%s", abspath, file);
> -
> -	fdt = read_file(path, &size);
> -	if (!fdt) {
> -		pr_warn("unable to read \"%s\"\n", path);
> -		ret = -EINVAL;
> -		goto out;
> -	}
> -
> -	overlay = of_unflatten_dtb(fdt, size);
> -	free(fdt);
> -	if (IS_ERR(overlay)) {
> -		ret = PTR_ERR(overlay);
> -		goto out;
> -	}
> -
> -	if (dryrun) {
> -		pr_info("dry run: skip overlay %s\n", path);
> -		of_delete_node(overlay);
> -		goto out;
> -	}
> -
> -	ret = of_firmware_load_overlay(overlay);
> -	if (ret) {
> -		pr_warn("failed to load firmware: skip overlay \"%s\"\n", path);
> -		of_delete_node(overlay);
> -		goto out;
> -	}
> -
> -	ret = of_register_overlay(overlay);
> -	if (ret) {
> -		pr_warn("cannot register devicetree overlay \"%s\"\n", path);
> -		of_delete_node(overlay);
> -	}
> -
> -out:
> -	free(path);
> -
> -	return ret;
> -}
> -
> -static void blspec_apply_oftree_overlays(const char *overlays,
> -					 const char *abspath, int dryrun)
> +static int blspec_overlay_fixup(struct device_node *root, void *ctx)
>  {
> +	struct blspec_entry *entry = ctx;
> +	const char *overlays;
>  	char *overlay;
>  	char *sep, *freep;
>  
> +	overlays = blspec_entry_var_get(entry, "devicetree-overlay");
> +
>  	sep = freep = xstrdup(overlays);
>  
>  	while ((overlay = strsep(&sep, " "))) {
> +		char *path;
> +
>  		if (!*overlay)
>  			continue;
> -		blspec_apply_oftree_overlay(overlay, abspath, dryrun);
> +
> +		path = basprintf("%s/%s", entry->rootpath, overlay);
> +
> +		of_overlay_apply_file(root, path);
> +
> +		free(path);
>  	}
>  
>  	free(freep);
> +
> +	return 0;
>  }
>  
>  /*
> @@ -153,7 +115,7 @@ static int blspec_boot(struct bootentry *be, int verbose, int dryrun)
>  	}
>  
>  	if (overlays)
> -		blspec_apply_oftree_overlays(overlays, abspath, dryrun);
> +		of_register_fixup(blspec_overlay_fixup, entry);
>  
>  	if (initrd)
>  		data.initrd_file = basprintf("%s/%s", abspath, initrd);
> @@ -186,6 +148,9 @@ static int blspec_boot(struct bootentry *be, int verbose, int dryrun)
>  	if (ret)
>  		pr_err("Booting failed\n");
>  
> +	if (overlays)
> +		of_unregister_fixup(blspec_overlay_fixup, entry);
> +
>  	firmware_set_searchpath(old_fws);
>  
>  err_out:
> diff --git a/drivers/of/of_firmware.c b/drivers/of/of_firmware.c
> index 12ce1d95d0..d66f0ae7ce 100644
> --- a/drivers/of/of_firmware.c
> +++ b/drivers/of/of_firmware.c
> @@ -48,27 +48,7 @@ static int load_firmware(struct device_node *target,
>  	return err;
>  }
>  
> -int of_firmware_load_overlay(struct device_node *overlay)
> +int of_overlay_load_firmware(struct device_node *root, struct device_node *overlay)
>  {
> -	int err;
> -	struct device_node *root;
> -	struct device_node *resolved;
> -	struct device_node *ovl;
> -
> -	root = of_get_root_node();
> -	resolved = of_resolve_phandles(root, overlay);
> -	/*
> -	 * If the overlay cannot be resolved, use the load_firmware callback
> -	 * with the unresolved overlay to verify that the overlay does not
> -	 * depend on a firmware to be loaded. If a required firmware cannot be
> -	 * loaded, the overlay must not be applied.
> -	 */
> -	ovl = resolved ? resolved : overlay;
> -
> -	err = of_process_overlay(root, ovl, load_firmware, NULL);
> -
> -	if (resolved)
> -		of_delete_node(resolved);
> -
> -	return err;
> +	return of_process_overlay(root, overlay, load_firmware, NULL);

This drops the check, if the overlay depends on firmware, which cannot be
loaded, because Barebox could not resolve the overlay. This might be OK,
because in this case, we also don't know if the target is an fpga-region.
Looks fragile to me, anyway.

>  }
> diff --git a/drivers/of/overlay.c b/drivers/of/overlay.c
> index c9ede614a6..d42e15ed1c 100644
> --- a/drivers/of/overlay.c
> +++ b/drivers/of/overlay.c
> @@ -221,6 +221,8 @@ int of_overlay_apply_tree(struct device_node *root,
>  			pr_warn("failed to apply %s\n", fragment->name);
>  	}
>  
> +	err = of_overlay_load_firmware(root, resolved);

The firmware must be loaded before the overlay is applied. If the firmware
cannot be loaded, the device tree must not be modified.

Michael

> +
>  	of_delete_node(resolved);
>  
>  	return err;
> diff --git a/include/of.h b/include/of.h
> index 56291b9801..ecc5f5101a 100644
> --- a/include/of.h
> +++ b/include/of.h
> @@ -1033,7 +1033,7 @@ int of_process_overlay(struct device_node *root,
>  				   struct device_node *overlay, void *data),
>  		    void *data);
>  
> -int of_firmware_load_overlay(struct device_node *overlay);
> +int of_overlay_load_firmware(struct device_node *root, struct device_node *overlay);
>  #else
>  static inline struct device_node *of_resolve_phandles(struct device_node *root,
>  					const struct device_node *overlay)
> @@ -1067,7 +1067,7 @@ static inline int of_process_overlay(struct device_node *root,
>  	return -ENOSYS;
>  }
>  
> -static inline int of_firmware_load_overlay(struct device_node *overlay)
> +static inline int of_overlay_load_firmware(struct device_node *root, struct device_node *overlay)
>  {
>  	return -ENOSYS;
>  }
> -- 
> 2.29.2

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

Re: [PATCH 12/17] blspec: Rework firmware load

[Sascha Hauer]

On Fri, Mar 12, 2021 at 12:15:05PM +0100, Michael Tretter wrote:
> On Wed, 10 Mar 2021 14:28:24 +0100, Sascha Hauer wrote:
> > Applying overlays in blspec currently works in two steps. First
> > of_firmware_load_overlay() is called which doesn't load an overlay,
> > but instead loads firmware when one is needed by the overlay. This
> > is done on the live tree, because that was needed to find the firmware
> > manager. The second step is to call of_register_overlay() to apply
> > the overlay to the kernel device tree when the fixups are executed.
> > 
> > Instead of using a separate step to load the firmware, load the firmware
> > as part of the of_fixups.
> 
> Wouldn't this result in the firmware being loaded whenever of_fix_tree is
> called? Then, every use of the of_dump or of_diff commands would result in a
> reload of the firmware.

You are right, that is undesired. Ahmad just suggested a dryrun
parameter to the of_fixup callbacks. That could be useful elsewhere. A
dryrun would mean "Do the device tree fixups, but don't change anything
else".

> > -int of_firmware_load_overlay(struct device_node *overlay)
> > +int of_overlay_load_firmware(struct device_node *root, struct device_node *overlay)
> >  {
> > -	int err;
> > -	struct device_node *root;
> > -	struct device_node *resolved;
> > -	struct device_node *ovl;
> > -
> > -	root = of_get_root_node();
> > -	resolved = of_resolve_phandles(root, overlay);
> > -	/*
> > -	 * If the overlay cannot be resolved, use the load_firmware callback
> > -	 * with the unresolved overlay to verify that the overlay does not
> > -	 * depend on a firmware to be loaded. If a required firmware cannot be
> > -	 * loaded, the overlay must not be applied.
> > -	 */
> > -	ovl = resolved ? resolved : overlay;
> > -
> > -	err = of_process_overlay(root, ovl, load_firmware, NULL);
> > -
> > -	if (resolved)
> > -		of_delete_node(resolved);
> > -
> > -	return err;
> > +	return of_process_overlay(root, overlay, load_firmware, NULL);
> 
> This drops the check, if the overlay depends on firmware, which cannot be
> loaded, because Barebox could not resolve the overlay. This might be OK,
> because in this case, we also don't know if the target is an fpga-region.
> Looks fragile to me, anyway.
> 
> >  }
> > diff --git a/drivers/of/overlay.c b/drivers/of/overlay.c
> > index c9ede614a6..d42e15ed1c 100644
> > --- a/drivers/of/overlay.c
> > +++ b/drivers/of/overlay.c
> > @@ -221,6 +221,8 @@ int of_overlay_apply_tree(struct device_node *root,
> >  			pr_warn("failed to apply %s\n", fragment->name);
> >  	}
> >  
> > +	err = of_overlay_load_firmware(root, resolved);
> 
> The firmware must be loaded before the overlay is applied. If the firmware
> cannot be loaded, the device tree must not be modified.

OK, I will load the firmware before applying the overlay.

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 |

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

[PATCH 17/17] of_firmware: Fix handling of firmware-name property

[Sascha Hauer]

The fpga-region binding states that the firmware-name property must be
in the same node as the fpga-region compatible, so checking for this
compatible when walking up the parents is wrong. Instead, only test for
it in the target node. Also, when the current node is not a fpga-region
we must return successfully.

Signed-off-by: Sascha Hauer <s.hauer@pengutronix.de>
---
 drivers/of/of_firmware.c | 11 ++++++-----
 1 file changed, 6 insertions(+), 5 deletions(-)

diff --git a/drivers/of/of_firmware.c b/drivers/of/of_firmware.c
index 2d8245a02e..687e675302 100644
--- a/drivers/of/of_firmware.c
+++ b/drivers/of/of_firmware.c
@@ -11,11 +11,9 @@ static struct firmware_mgr *of_node_get_mgr(struct device_node *np)
 	struct device_node *mgr_node;
 
 	do {
-		if (of_device_is_compatible(np, "fpga-region")) {
-			mgr_node = of_parse_phandle(np, "fpga-mgr", 0);
-			if (mgr_node)
-				return firmwaremgr_find_by_node(mgr_node);
-		}
+		mgr_node = of_parse_phandle(np, "fpga-mgr", 0);
+		if (mgr_node)
+			return firmwaremgr_find_by_node(mgr_node);
 	} while ((np = of_get_parent(np)) != NULL);
 
 	return NULL;
@@ -48,6 +46,9 @@ static int load_firmware(struct device_node *target,
 	if (!target)
 		return -EINVAL;
 
+	if (!of_device_is_compatible(target, "fpga-region"))
+		return 0;
+
 	mgr = of_node_get_mgr(target);
 	if (!mgr)
 		return -EINVAL;
-- 
2.29.2


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