[RFC PATCH 0/7] Device Tree Overlay Support

[RFC PATCH 0/7] Device Tree Overlay Support

[Michael Tretter]

Hello,

This series adds devicetree overlay support loosely based on the Linux drivers
and a fpga-region driver to program an FPGA based on a devicetree overlay.

The overlay is registered as a fixup, which is necessary to apply the overlay
either on the build-in device tree or external device trees that are passed to
Linux.

The overlay driver uses notifiers to notify other drivers before and after an
overlay is applied. This allows other drivers to react on overlays, e.g., to
program an FPGA. If other drivers report an error from the notifier, the error
will be ignored and the overlay will be applied anyway.

The fpga-region driver understands the firmware-name property and loads a
firmware if the firmware-name property is added to a device node. The binding
defines that the firmware is searched in the firmware search path. The
global.firmware.path variable stores this path and allows other drivers or the
user to specify the search path. Not sure if this is a good variable name for
that.

I extended the blspec and allow to specify a device tree overlay with the
"devicetree-overlay" key. The overlay is applied before booting the OS from
that blspec entry.

Michael

Michael Tretter (7):
  commands: unify newlines for options
  dtc: add -@ option to enable __symbols__
  of: add support for devicetree overlays
  commands: add oftree -o option for overlays
  firmware: allow to find manager by device node
  firmware: add support for fpga-regions
  blspec: add support for devicetree overlays

 Documentation/user/booting-linux.rst |   4 +
 commands/crc.c                       |   4 +-
 commands/firmwareload.c              |   4 +-
 commands/of_display_timings.c        |   8 +-
 commands/of_dump.c                   |   6 +-
 commands/of_fixup_status.c           |   2 +-
 commands/oftree.c                    |  41 +++-
 common/blspec.c                      |  10 +
 common/bootm.c                       |  35 ++++
 common/firmware.c                    |  18 ++
 drivers/firmware/Kconfig             |   7 +
 drivers/firmware/Makefile            |   1 +
 drivers/firmware/of-fpga-region.c    | 153 +++++++++++++++
 drivers/of/Kconfig                   |   9 +
 drivers/of/Makefile                  |   1 +
 drivers/of/overlay.c                 | 251 ++++++++++++++++++++++++
 drivers/of/resolver.c                | 278 +++++++++++++++++++++++++++
 include/bootm.h                      |   1 +
 include/firmware.h                   |   1 +
 include/of.h                         |  42 ++++
 scripts/Makefile.lib                 |   2 +-
 21 files changed, 860 insertions(+), 18 deletions(-)
 create mode 100644 drivers/firmware/of-fpga-region.c
 create mode 100644 drivers/of/overlay.c
 create mode 100644 drivers/of/resolver.c

-- 
2.20.1


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

[RFC PATCH 1/7] commands: unify newlines for options

[Michael Tretter]

The BAREBOX_CMD_HELP_OPT macro adds a newline to the string, but users
of the macro inconsistently add another newline resulting in empty
newlines for some commands, but not for others. Remove any newline in
the description and rely on BAREBOX_CMD_HELP_OPT for the formatting.

Signed-off-by: Michael Tretter <m.tretter@pengutronix.de>
---
 commands/crc.c                | 4 ++--
 commands/firmwareload.c       | 4 ++--
 commands/of_display_timings.c | 8 ++++----
 commands/of_dump.c            | 6 +++---
 commands/of_fixup_status.c    | 2 +-
 commands/oftree.c             | 4 ++--
 6 files changed, 14 insertions(+), 14 deletions(-)

diff --git a/commands/crc.c b/commands/crc.c
index edb1fb125c..580521d677 100644
--- a/commands/crc.c
+++ b/commands/crc.c
@@ -138,8 +138,8 @@ BAREBOX_CMD_HELP_OPT ("-F FILE", "Use file to compare.")
 #endif
 BAREBOX_CMD_HELP_OPT ("-v CRC",  "Verify")
 BAREBOX_CMD_HELP_OPT ("-V FILE", "Verify with CRC read from FILE")
-BAREBOX_CMD_HELP_OPT  ("-r <var>",  "Set <var> to the checksum result\n")
-BAREBOX_CMD_HELP_OPT  ("-s <var>",  "Set <var> to the data size\n")
+BAREBOX_CMD_HELP_OPT  ("-r <var>",  "Set <var> to the checksum result")
+BAREBOX_CMD_HELP_OPT  ("-s <var>",  "Set <var> to the data size")
 BAREBOX_CMD_HELP_END
 
 BAREBOX_CMD_START(crc32)
diff --git a/commands/firmwareload.c b/commands/firmwareload.c
index 071f25be99..dbd43e046d 100644
--- a/commands/firmwareload.c
+++ b/commands/firmwareload.c
@@ -55,8 +55,8 @@ static int do_firmwareload(int argc, char *argv[])
 
 BAREBOX_CMD_HELP_START(firmwareload)
 BAREBOX_CMD_HELP_TEXT("Options:")
-BAREBOX_CMD_HELP_OPT("-t <target>", "define the firmware handler by name\n")
-BAREBOX_CMD_HELP_OPT("-l\t", "list devices capable of firmware loading\n")
+BAREBOX_CMD_HELP_OPT("-t <target>", "define the firmware handler by name")
+BAREBOX_CMD_HELP_OPT("-l\t", "list devices capable of firmware loading")
 BAREBOX_CMD_HELP_END
 
 BAREBOX_CMD_START(firmwareload)
diff --git a/commands/of_display_timings.c b/commands/of_display_timings.c
index ccf2db0da5..365ff80c36 100644
--- a/commands/of_display_timings.c
+++ b/commands/of_display_timings.c
@@ -148,10 +148,10 @@ static int do_of_display_timings(int argc, char *argv[])
 
 BAREBOX_CMD_HELP_START(of_display_timings)
 BAREBOX_CMD_HELP_TEXT("Options:")
-BAREBOX_CMD_HELP_OPT("-l",  "list path of all available display-timings\n")
-BAREBOX_CMD_HELP_OPT("-s",  "list path of all selected display-timings\n")
-BAREBOX_CMD_HELP_OPT("-S path",  "select display-timings and register oftree fixup\n")
-BAREBOX_CMD_HELP_OPT("-f dtb",  "work on dtb. Has no effect on -s option\n")
+BAREBOX_CMD_HELP_OPT("-l",  "list path of all available display-timings")
+BAREBOX_CMD_HELP_OPT("-s",  "list path of all selected display-timings")
+BAREBOX_CMD_HELP_OPT("-S path",  "select display-timings and register oftree fixup")
+BAREBOX_CMD_HELP_OPT("-f dtb",  "work on dtb. Has no effect on -s option")
 BAREBOX_CMD_HELP_END
 
 BAREBOX_CMD_START(of_display_timings)
diff --git a/commands/of_dump.c b/commands/of_dump.c
index 7bec0b925e..06b8e9fcda 100644
--- a/commands/of_dump.c
+++ b/commands/of_dump.c
@@ -139,9 +139,9 @@ out:
 
 BAREBOX_CMD_HELP_START(of_dump)
 BAREBOX_CMD_HELP_TEXT("Options:")
-BAREBOX_CMD_HELP_OPT  ("-f dtb",  "work on dtb instead of internal devicetree\n")
-BAREBOX_CMD_HELP_OPT  ("-F",  "return fixed devicetree\n")
-BAREBOX_CMD_HELP_OPT  ("-n",  "Print node names only, no properties\n")
+BAREBOX_CMD_HELP_OPT  ("-f dtb",  "work on dtb instead of internal devicetree")
+BAREBOX_CMD_HELP_OPT  ("-F",  "return fixed devicetree")
+BAREBOX_CMD_HELP_OPT  ("-n",  "Print node names only, no properties")
 BAREBOX_CMD_HELP_END
 
 BAREBOX_CMD_START(of_dump)
diff --git a/commands/of_fixup_status.c b/commands/of_fixup_status.c
index 9a4a619195..e0da429ac3 100644
--- a/commands/of_fixup_status.c
+++ b/commands/of_fixup_status.c
@@ -59,7 +59,7 @@ static int do_of_fixup_status(int argc, char *argv[])
 BAREBOX_CMD_HELP_START(of_fixup_status)
 BAREBOX_CMD_HELP_TEXT("Options:")
 BAREBOX_CMD_HELP_OPT("-d",  "disable node")
-BAREBOX_CMD_HELP_OPT("path",  "Node path\n")
+BAREBOX_CMD_HELP_OPT("path",  "Node path")
 BAREBOX_CMD_HELP_TEXT("Register a fixup to enable or disable a device tree node.")
 BAREBOX_CMD_HELP_TEXT("Nodes are enabled on default. Disabled with -d.")
 BAREBOX_CMD_HELP_END
diff --git a/commands/oftree.c b/commands/oftree.c
index 26a47bb581..299c2edfcd 100644
--- a/commands/oftree.c
+++ b/commands/oftree.c
@@ -123,8 +123,8 @@ out:
 
 BAREBOX_CMD_HELP_START(oftree)
 BAREBOX_CMD_HELP_TEXT("Options:")
-BAREBOX_CMD_HELP_OPT ("-l <DTB>",  "Load <DTB> to internal devicetree\n")
-BAREBOX_CMD_HELP_OPT ("-s <DTB>",  "save internal devicetree to <DTB>\n")
+BAREBOX_CMD_HELP_OPT ("-l <DTB>",  "Load <DTB> to internal devicetree")
+BAREBOX_CMD_HELP_OPT ("-s <DTB>",  "save internal devicetree to <DTB>")
 BAREBOX_CMD_HELP_OPT ("-p",  "probe devices from stored device tree")
 BAREBOX_CMD_HELP_END
 
-- 
2.20.1


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

[RFC PATCH 2/7] dtc: add -@ option to enable __symbols__

[Michael Tretter]

The devicetree overlay driver requires the __symbols__ node to resolve
phandles to the base devicetree. As Barebox applies the overlay to the
live devicetree, the build-in devicetree must be built with the
__symbols__ node.

Signed-off-by: Michael Tretter <m.tretter@pengutronix.de>
---
 scripts/Makefile.lib | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/scripts/Makefile.lib b/scripts/Makefile.lib
index 8c07a54d05..6152bc6211 100644
--- a/scripts/Makefile.lib
+++ b/scripts/Makefile.lib
@@ -292,7 +292,7 @@ $(obj)/%.dtb.S: $(obj)/%.dtb $(srctree)/scripts/gen-dtb-s FORCE
 
 quiet_cmd_dtc = DTC     $@
 cmd_dtc = $(CPP) $(dtc_cpp_flags) -x assembler-with-cpp -o $(dtc-tmp) $< ; \
-	$(objtree)/scripts/dtc/dtc -O dtb -o $@ -b 0 \
+	$(objtree)/scripts/dtc/dtc -@ -O dtb -o $@ -b 0 \
 		-i $(srctree)/arch/$(SRCARCH)/dts $(DTC_FLAGS) \
 		-i $(srctree)/dts/src/$(SRCARCH) \
 		-d $(depfile).dtc $(dtc-tmp) ; \
-- 
2.20.1


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

[RFC PATCH 3/7] of: add support for devicetree overlays

[Michael Tretter]

The devicetree overlay support is based on the Linux driver for device
tree overlays, but many features that are not required in Barebox are
left out.

Unlike Linux, which applies the overlay to the live devicetree, Barebox
registers a fixup for the overlay which is applied with other fixups to
whatever tree is fixed. This is necessary to apply the overlay to
devicetrees that are passed to Linux, which might differ from the
devicetree that is currently live in Barebox.

Therefore, it is not possible to remove overlays that have been
registered anymore.

Drivers can register to be notified when an overlay is registered and
will be notified before and after the overlay has been registered. The
target of the overlay in the notification is the live devicetree to
allow drivers to act on the current state of the system.

Signed-off-by: Michael Tretter <m.tretter@pengutronix.de>
---
 drivers/of/Kconfig    |   9 ++
 drivers/of/Makefile   |   1 +
 drivers/of/overlay.c  | 251 ++++++++++++++++++++++++++++++++++++++
 drivers/of/resolver.c | 278 ++++++++++++++++++++++++++++++++++++++++++
 include/of.h          |  42 +++++++
 5 files changed, 581 insertions(+)
 create mode 100644 drivers/of/overlay.c
 create mode 100644 drivers/of/resolver.c

diff --git a/drivers/of/Kconfig b/drivers/of/Kconfig
index 24cf4465a8..1bb6639c5a 100644
--- a/drivers/of/Kconfig
+++ b/drivers/of/Kconfig
@@ -50,3 +50,12 @@ config OF_BAREBOX_ENV_IN_FS
 	help
 	  Allow the devie tree configuration of the barebox environment path
 	  to specify a file in filesystem, which will be mounted.
+
+config OF_OVERLAY
+	select OFTREE
+	bool "Devicetree overlays"
+	help
+	  Overlays allow to patch the devicetree. Unlike Linux, Barebox does
+	  not patch the live devicetree, but applies the overlays as fixup to
+	  the devicetree. Furthermore, overlays cannot be removed after they
+	  have been applied.
diff --git a/drivers/of/Makefile b/drivers/of/Makefile
index ec43870061..9c6f8de814 100644
--- a/drivers/of/Makefile
+++ b/drivers/of/Makefile
@@ -6,3 +6,4 @@ obj-y += partition.o
 obj-y += of_net.o
 obj-$(CONFIG_MTD) += of_mtd.o
 obj-$(CONFIG_OF_BAREBOX_DRIVERS) += barebox.o
+obj-$(CONFIG_OF_OVERLAY) += overlay.o resolver.o
diff --git a/drivers/of/overlay.c b/drivers/of/overlay.c
new file mode 100644
index 0000000000..e3fd7ccd45
--- /dev/null
+++ b/drivers/of/overlay.c
@@ -0,0 +1,251 @@
+// SPDX-License-Identifier: GPL-2.0
+/*
+ * Functions for working with device tree overlays
+ *
+ * Copyright (C) 2012 Pantelis Antoniou <panto@antoniou-consulting.com>
+ * Copyright (C) 2012 Texas Instruments Inc.
+ * Copyright (C) 2019 Pengutronix, Michael Tretter <m.tretter@pengutronix.de>
+ */
+
+#include <common.h>
+#include <notifier.h>
+#include <of.h>
+#include <errno.h>
+
+static struct device_node *find_target(struct device_node *root,
+				       struct device_node *fragment)
+{
+	struct device_node *node;
+	const char *path;
+	u32 phandle;
+	int ret;
+
+	ret = of_property_read_u32(fragment, "target", &phandle);
+	if (!ret) {
+		node = of_find_node_by_phandle_from(phandle, root);
+		if (!node)
+			pr_err("fragment %pOF: phandle 0x%x not found\n",
+			       fragment, phandle);
+		return node;
+	}
+
+	ret = of_property_read_string(fragment, "target-path", &path);
+	if (!ret) {
+		node = of_find_node_by_path_from(root, path);
+		if (!node)
+			pr_err("fragment %pOF: path '%s' not found\n",
+			       fragment, path);
+		return node;
+	}
+
+	pr_err("fragment %pOF: no target property\n", fragment);
+
+	return NULL;
+}
+
+static NOTIFIER_HEAD(overlay_notify_chain);
+
+int of_overlay_notifier_register(struct notifier_block *nb)
+{
+	return notifier_chain_register(&overlay_notify_chain, nb);
+}
+
+int of_overlay_notifier_unregister(struct notifier_block *nb)
+{
+	return notifier_chain_unregister(&overlay_notify_chain, nb);
+}
+
+static int of_overlay_notify(struct device_node *overlay,
+			     enum of_overlay_notify_action action)
+{
+	struct of_overlay_notify_data nd;
+	struct device_node *root;
+	struct device_node *resolved;
+	struct device_node *fragment;
+
+	root = of_get_root_node();
+	if (!root)
+		return -ENODEV;
+
+	resolved = of_resolve_phandles(root, overlay);
+	if (!resolved)
+		return -EINVAL;
+
+	for_each_child_of_node(resolved, fragment) {
+		nd.overlay = of_get_child_by_name(fragment, "__overlay__");
+		if (!nd.overlay)
+			continue;
+
+		nd.target = find_target(root, fragment);
+		if (!nd.target)
+			continue;
+
+		notifier_call_chain(&overlay_notify_chain, action, &nd);
+	}
+
+	of_delete_node(resolved);
+
+	return 0;
+}
+
+static int of_overlay_apply(struct device_node *target,
+			    const struct device_node *overlay)
+{
+	struct device_node *child;
+	struct device_node *target_child;
+	struct property *prop;
+	int err;
+
+	if (target == NULL || overlay == NULL)
+		return -EINVAL;
+
+	list_for_each_entry(prop, &overlay->properties, list) {
+		if (of_prop_cmp(prop->name, "name") == 0)
+			continue;
+
+		err = of_set_property(target, prop->name, prop->value,
+				      prop->length, true);
+		if (err)
+			return err;
+	}
+
+	for_each_child_of_node(overlay, child) {
+		target_child = of_get_child_by_name(target, child->name);
+		if (!target_child)
+			target_child = of_new_node(target, child->name);
+		if (!target_child)
+			return -ENOMEM;
+
+		err = of_overlay_apply(target_child, child);
+		if (err)
+			return err;
+	}
+
+	return 0;
+}
+
+static char *of_overlay_fix_path(struct device_node *root,
+				 struct device_node *overlay, const char *path)
+{
+	struct device_node *fragment;
+	struct device_node *target;
+	const char *path_tail;
+
+	fragment = of_find_node_by_path_from(overlay, path);
+	while ((fragment = of_get_parent(fragment)) != NULL) {
+		if (of_get_child_by_name(fragment, "__overlay__"))
+			break;
+	}
+	if (!fragment)
+		return NULL;
+
+	target = find_target(root, fragment);
+	if (!target)
+		return NULL;
+
+	path_tail = path + strlen(of_get_child_by_name(fragment, "__overlay__")->full_name);
+
+	return basprintf("%s%s", target->full_name, path_tail);
+}
+
+static int of_overlay_apply_symbols(struct device_node *root,
+				    struct device_node *overlay)
+{
+	const char *old_path;
+	char *new_path;
+	struct property *prop;
+	struct device_node *root_symbols;
+	struct device_node *overlay_symbols;
+
+	root_symbols = of_get_child_by_name(root, "__symbols__");
+	if (!root_symbols)
+		return -EINVAL;
+
+	overlay_symbols = of_get_child_by_name(overlay, "__symbols__");
+	if (!overlay_symbols)
+		return -EINVAL;
+
+	list_for_each_entry(prop, &overlay_symbols->properties, list) {
+		if (of_prop_cmp(prop->name, "name") == 0)
+			continue;
+
+		old_path = of_property_get_value(prop);
+		new_path = of_overlay_fix_path(root, overlay, old_path);
+
+		pr_debug("add symbol %s with new path %s\n",
+			 prop->name, new_path);
+		of_property_write_string(root_symbols, prop->name, new_path);
+	}
+
+	return 0;
+}
+
+static int of_overlay_apply_fragment(struct device_node *root,
+				     struct device_node *fragment)
+{
+	struct device_node *target;
+	struct device_node *overlay;
+
+	overlay = of_get_child_by_name(fragment, "__overlay__");
+	if (!overlay)
+		return 0;
+
+	target = find_target(root, fragment);
+	if (!target)
+		return -EINVAL;
+
+	return of_overlay_apply(target, overlay);
+}
+
+/**
+ * Fix the passed root node using the device tree overlay in data
+ */
+static int of_overlay_fixup(struct device_node *root, void *data)
+{
+	struct device_node *overlay = data;
+	struct device_node *resolved;
+	struct device_node *fragment;
+	int err;
+
+	resolved = of_resolve_phandles(root, overlay);
+	if (!resolved)
+		return -EINVAL;
+
+	/* Copy symbols from resolved overlay to base device tree */
+	err = of_overlay_apply_symbols(root, resolved);
+	if (err)
+		pr_warn("failed to copy symbols from overlay");
+
+	/* Copy nodes and properties from resolved overlay to root */
+	for_each_child_of_node(resolved, fragment) {
+		err = of_overlay_apply_fragment(root, fragment);
+		if (err)
+			pr_warn("failed to apply %s", fragment->name);
+	}
+
+	of_delete_node(resolved);
+
+	return 0;
+}
+
+/**
+ * Register a devicetree overlay
+ * @param: The overlay that is registered
+ *
+ * When an overlay is registered, all drivers that registered for the overlay
+ * notifier will be notified about that overlay.
+ *
+ * The overlay is not applied to the live device tree, but registered as fixup
+ * for the fixed up device tree. Therefore, drivers relying on the overlay
+ * must use the fixed device tree.
+ */
+int of_register_overlay(struct device_node *overlay)
+{
+	of_overlay_notify(overlay, OF_OVERLAY_PRE_APPLY);
+
+	of_register_fixup(of_overlay_fixup, overlay);
+
+	of_overlay_notify(overlay, OF_OVERLAY_POST_APPLY);
+
+	return 0;
+}
diff --git a/drivers/of/resolver.c b/drivers/of/resolver.c
new file mode 100644
index 0000000000..fa238241d1
--- /dev/null
+++ b/drivers/of/resolver.c
@@ -0,0 +1,278 @@
+// SPDX-License-Identifier: GPL-2.0
+/*
+ * Functions for dealing with DT resolution
+ *
+ * Copyright (C) 2012 Pantelis Antoniou <panto@antoniou-consulting.com>
+ * Copyright (C) 2012 Texas Instruments Inc.
+ * Copyright (C) 2019 Pengutronix, Michael Tretter <m.tretter@pengutronix.de>
+ */
+
+#include <common.h>
+#include <of.h>
+#include <errno.h>
+
+/**
+ * Recursively update phandles in overlay by adding delta
+ */
+static void adjust_overlay_phandles(struct device_node *overlay, int delta)
+{
+	struct device_node *child;
+	struct property *prop;
+
+	if (overlay->phandle != 0)
+		overlay->phandle += delta;
+
+	list_for_each_entry(prop, &overlay->properties, list) {
+		if (of_prop_cmp(prop->name, "phandle") != 0 &&
+		    of_prop_cmp(prop->name, "linux,phandle") != 0)
+			continue;
+		if (prop->length < 4)
+			continue;
+
+		be32_add_cpu(prop->value, delta);
+	}
+
+	for_each_child_of_node(overlay, child)
+		adjust_overlay_phandles(child, delta);
+}
+
+/**
+ * Update all unresolved phandles in the overlay using prop_fixup
+ *
+ * prop_fixup contains a list of tuples of path:property_name:offset, each of
+ * which refers to a property that is phandle to a node in the base
+ * devicetree.
+ */
+static int update_usages_of_a_phandle_reference(struct device_node *overlay,
+						struct property *prop_fixup,
+						phandle phandle)
+{
+	struct device_node *refnode;
+	struct property *prop;
+	char *value, *cur, *end, *node_path, *prop_name, *s;
+	int offset, len;
+	int err = 0;
+
+	pr_debug("resolve references to %s to phandle 0x%x\n",
+		 prop_fixup->name, phandle);
+
+	value = kmemdup(prop_fixup->value, prop_fixup->length, GFP_KERNEL);
+	if (!value)
+		return -ENOMEM;
+
+	end = value + prop_fixup->length;
+	for (cur = value; cur < end; cur += len + 1) {
+		len = strlen(cur);
+
+		node_path = cur;
+		s = strchr(cur, ':');
+		if (!s) {
+			err = -EINVAL;
+			goto err_fail;
+		}
+		*s++ = '\0';
+
+		prop_name = s;
+		s = strchr(s, ':');
+		if (!s) {
+			err = -EINVAL;
+			goto err_fail;
+		}
+		*s++ = '\0';
+
+		err = kstrtoint(s, 10, &offset);
+		if (err)
+			goto err_fail;
+
+		refnode = of_find_node_by_path_from(overlay, node_path);
+		if (!refnode)
+			continue;
+
+		prop = of_find_property(refnode, prop_name, NULL);
+		if (!prop) {
+			err = -ENOENT;
+			goto err_fail;
+		}
+
+		if (offset < 0 || offset + sizeof(__be32) > prop->length) {
+			err = -EINVAL;
+			goto err_fail;
+		}
+
+		*(__be32 *)(prop->value + offset) = cpu_to_be32(phandle);
+	}
+
+err_fail:
+	kfree(value);
+
+	if (err)
+		pr_debug("failed to resolve references to %s\n",
+			 prop_fixup->name);
+
+	return err;
+}
+
+/*
+ * Adjust the local phandle references by the given phandle delta.
+ *
+ * Subtree @local_fixups, which is overlay node __local_fixups__,
+ * mirrors the fragment node structure at the root of the overlay.
+ *
+ * For each property in the fragments that contains a phandle reference,
+ * @local_fixups has a property of the same name that contains a list
+ * of offsets of the phandle reference(s) within the respective property
+ * value(s).  The values at these offsets will be fixed up.
+ */
+static int adjust_local_phandle_references(struct device_node *local_fixups,
+		struct device_node *overlay, int phandle_delta)
+{
+	struct device_node *child, *overlay_child;
+	struct property *prop_fix, *prop;
+	int err, i, count;
+	unsigned int off;
+
+	if (!local_fixups)
+		return 0;
+
+	list_for_each_entry(prop_fix, &local_fixups->properties, list) {
+		if (!of_prop_cmp(prop_fix->name, "name") ||
+		    !of_prop_cmp(prop_fix->name, "phandle") ||
+		    !of_prop_cmp(prop_fix->name, "linux,phandle"))
+			continue;
+
+		if ((prop_fix->length % sizeof(__be32)) != 0 ||
+		    prop_fix->length == 0)
+			return -EINVAL;
+		count = prop_fix->length / sizeof(__be32);
+
+		prop = of_find_property(overlay, prop_fix->name, NULL);
+		if (!prop)
+			return -EINVAL;
+
+		for (i = 0; i < count; i++) {
+			off = be32_to_cpu(((__be32 *)prop_fix->value)[i]);
+			if ((off + sizeof(__be32)) > prop->length)
+				return -EINVAL;
+
+			be32_add_cpu(prop->value + off, phandle_delta);
+		}
+	}
+
+	for_each_child_of_node(local_fixups, child) {
+		for_each_child_of_node(overlay, overlay_child)
+			if (!of_node_cmp(child->name, overlay_child->name))
+				break;
+		if (!overlay_child)
+			return -EINVAL;
+
+		err = adjust_local_phandle_references(child, overlay_child,
+				phandle_delta);
+		if (err)
+			return err;
+	}
+
+	return 0;
+}
+
+/**
+ * of_resolve_phandles - Resolve phandles in overlay based on root
+ *
+ * Rename phandles in overlay to avoid conflicts with the base devicetree and
+ * replace all phandles in the overlay with their renamed versions. Resolve
+ * phandles referring to nodes in the base devicetree with the phandle from
+ * the base devicetree.
+ *
+ * Returns a new device_node with resolved phandles which must be deleted by
+ * the caller of this function.
+ */
+struct device_node *of_resolve_phandles(struct device_node *root,
+					struct device_node *overlay)
+{
+	struct device_node *result;
+	struct device_node *local_fixups;
+	struct device_node *refnode;
+	struct device_node *symbols;
+	struct device_node *overlay_fixups;
+	struct property *prop;
+	const char *refpath;
+	phandle delta;
+	int err;
+
+	result = of_copy_node(NULL, overlay);
+	if (!result)
+		return NULL;
+
+	delta = of_get_tree_max_phandle(root) + 1;
+
+	/*
+	 * Rename the phandles in the devicetree overlay to prevent conflicts
+	 * with the phandles in the base devicetree.
+	 */
+	adjust_overlay_phandles(result, delta);
+
+	/*
+	 * __local_fixups__ contains all locations in the overlay that refer
+	 * to a phandle defined in the overlay. We must update the references,
+	 * because we just adjusted the definitions.
+	 */
+	local_fixups = of_find_node_by_name(result, "__local_fixups__");
+	err = adjust_local_phandle_references(local_fixups, result, delta);
+	if (err) {
+		pr_err("failed to fix phandles in overlay\n");
+		goto err;
+	}
+
+	/*
+	 * __fixups__ contains all locations in the overlay that refer to a
+	 * phandle that is not defined in the overlay and should be defined in
+	 * the base device tree. We must update the references, because they
+	 * are otherwise undefined.
+	 */
+	overlay_fixups = of_find_node_by_name(overlay, "__fixups__");
+	if (!overlay_fixups) {
+		pr_debug("overlay does not contain phandles to base devicetree\n");
+		goto out;
+	}
+
+	symbols = of_find_node_by_path_from(root, "/__symbols__");
+	if (!symbols) {
+		pr_err("__symbols__ missing from base devicetree\n");
+		goto err;
+	}
+
+	list_for_each_entry(prop, &overlay_fixups->properties, list) {
+		if (!of_prop_cmp(prop->name, "name"))
+			continue;
+
+		err = of_property_read_string(symbols, prop->name, &refpath);
+		if (err) {
+			pr_err("cannot find node %s in base devicetree\n",
+			       prop->name);
+			goto err;
+		}
+
+		refnode = of_find_node_by_path_from(root, refpath);
+		if (!refnode) {
+			pr_err("cannot find path %s in base devicetree\n",
+			       refpath);
+			err = -EINVAL;
+			goto err;
+		}
+
+		err = update_usages_of_a_phandle_reference(overlay, prop,
+							   refnode->phandle);
+		if (err) {
+			pr_err("failed to update phandles for %s in overlay",
+			       prop->name);
+			goto err;
+		}
+	}
+
+out:
+	return result;
+err:
+	of_delete_node(result);
+
+	return NULL;
+
+}
diff --git a/include/of.h b/include/of.h
index b5f54dd4e5..13ba46a669 100644
--- a/include/of.h
+++ b/include/of.h
@@ -3,6 +3,7 @@
 
 #include <fdt.h>
 #include <errno.h>
+#include <notifier.h>
 #include <linux/types.h>
 #include <linux/list.h>
 #include <asm/byteorder.h>
@@ -870,4 +871,45 @@ static inline struct device_node *of_find_root_node(struct device_node *node)
 
 	return node;
 }
+
+enum of_overlay_notify_action {
+	OF_OVERLAY_PRE_APPLY = 0,
+	OF_OVERLAY_POST_APPLY,
+};
+
+struct of_overlay_notify_data {
+	struct device_node *overlay;
+	struct device_node *target;
+};
+
+#ifdef CONFIG_OF_OVERLAY
+struct device_node *of_resolve_phandles(struct device_node *root,
+				        struct device_node *overlay);
+int of_register_overlay(struct device_node *overlay);
+int of_overlay_notifier_register(struct notifier_block *nb);
+int of_overlay_notifier_unregister(struct notifier_block *nb);
+#else
+static inline struct device_node *of_resolve_phandles(struct device_node *root,
+						struct device_node *overlay)
+{
+	return NULL;
+}
+
+static inline int of_overlay_apply(const void *fdt, struct device_node *tree)
+{
+	return -ENOSYS;
+}
+
+static inline int of_overlay_notifier_register(struct notifier_block *nb)
+{
+	return -ENOSYS;
+}
+
+static inline int of_overlay_notifier_unregister(struct notifier_block *nb)
+{
+	return -ENOSYS;
+}
+
+#endif
+
 #endif /* __OF_H */
-- 
2.20.1


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

[RFC PATCH 4/7] commands: add oftree -o option for overlays

[Michael Tretter]

Extend the oftree command to allow to register devicetree overlays.

Signed-off-by: Michael Tretter <m.tretter@pengutronix.de>
---
 commands/oftree.c | 37 ++++++++++++++++++++++++++++++++++---
 include/of.h      |  2 +-
 2 files changed, 35 insertions(+), 4 deletions(-)

diff --git a/commands/oftree.c b/commands/oftree.c
index 299c2edfcd..e61393ce6a 100644
--- a/commands/oftree.c
+++ b/commands/oftree.c
@@ -48,10 +48,11 @@ static int do_oftree(int argc, char *argv[])
 	int probe = 0;
 	char *load = NULL;
 	char *save = NULL;
+	char *overlay = NULL;
 	int ret;
 	struct device_node *root;
 
-	while ((opt = getopt(argc, argv, "pfl:s:")) > 0) {
+	while ((opt = getopt(argc, argv, "pfl:o:s:")) > 0) {
 		switch (opt) {
 		case 'l':
 			load = optarg;
@@ -64,13 +65,21 @@ static int do_oftree(int argc, char *argv[])
 				return COMMAND_ERROR_USAGE;
 			}
 			break;
+		case 'o':
+			if (IS_ENABLED(CONFIG_OF_OVERLAY)) {
+				overlay = optarg;
+			} else {
+				printf("oftree overlay support disabled\n");
+				return COMMAND_ERROR_USAGE;
+			}
+			break;
 		case 's':
 			save = optarg;
 			break;
 		}
 	}
 
-	if (!probe && !load && !save)
+	if (!probe && !load && !save && !overlay)
 		return COMMAND_ERROR_USAGE;
 
 	if (save) {
@@ -109,6 +118,27 @@ static int do_oftree(int argc, char *argv[])
 		}
 	}
 
+	if (overlay) {
+		fdt = read_file(overlay, &size);
+		if (!fdt) {
+			printf("unable to read %s\n", overlay);
+			return 1;
+		}
+
+		root = of_unflatten_dtb(fdt);
+		if (IS_ERR(root))
+			return PTR_ERR(root);
+
+		free(fdt);
+
+		ret = of_register_overlay(root);
+		if (ret) {
+			printf("cannot apply oftree overlay: %s\n", strerror(-ret));
+			of_delete_node(root);
+			goto out;
+		}
+	}
+
 	if (probe) {
 		ret = of_probe();
 		if (ret)
@@ -125,13 +155,14 @@ BAREBOX_CMD_HELP_START(oftree)
 BAREBOX_CMD_HELP_TEXT("Options:")
 BAREBOX_CMD_HELP_OPT ("-l <DTB>",  "Load <DTB> to internal devicetree")
 BAREBOX_CMD_HELP_OPT ("-s <DTB>",  "save internal devicetree to <DTB>")
+BAREBOX_CMD_HELP_OPT ("-o <DTBO>",  "register devicetree overlay <DTBO>")
 BAREBOX_CMD_HELP_OPT ("-p",  "probe devices from stored device tree")
 BAREBOX_CMD_HELP_END
 
 BAREBOX_CMD_START(oftree)
 	.cmd		= do_oftree,
 	BAREBOX_CMD_DESC("handle device trees")
-	BAREBOX_CMD_OPTS("[-lsp]")
+	BAREBOX_CMD_OPTS("[-lspo]")
 	BAREBOX_CMD_GROUP(CMD_GRP_MISC)
 	BAREBOX_CMD_HELP(cmd_oftree_help)
 BAREBOX_CMD_END
diff --git a/include/of.h b/include/of.h
index 13ba46a669..ec7872e613 100644
--- a/include/of.h
+++ b/include/of.h
@@ -884,7 +884,7 @@ struct of_overlay_notify_data {
 
 #ifdef CONFIG_OF_OVERLAY
 struct device_node *of_resolve_phandles(struct device_node *root,
-				        struct device_node *overlay);
+					struct device_node *overlay);
 int of_register_overlay(struct device_node *overlay);
 int of_overlay_notifier_register(struct notifier_block *nb);
 int of_overlay_notifier_unregister(struct notifier_block *nb);
-- 
2.20.1


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

[RFC PATCH 5/7] firmware: allow to find manager by device node

[Michael Tretter]

The firmware manager can be found by using the id-String. The id cannot
be used, if the user of the firmware handler is a driver that only has a
handle to the firmware managers devicetree node, because there is no way
to map from the devicetree to the id.

Add a function that allows to get the firmware handler using its device
tree node.

Signed-off-by: Michael Tretter <m.tretter@pengutronix.de>
---
 common/firmware.c  | 18 ++++++++++++++++++
 include/firmware.h |  1 +
 2 files changed, 19 insertions(+)

diff --git a/common/firmware.c b/common/firmware.c
index 9d55d73e7a..609cf11822 100644
--- a/common/firmware.c
+++ b/common/firmware.c
@@ -62,6 +62,24 @@ struct firmware_mgr *firmwaremgr_find(const char *id)
 	return NULL;
 }
 
+/*
+ * firmwaremgr_find_by_node - find a firmware device handler
+ *
+ * Find a firmware device handler using the device node of the firmware
+ * 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 *mgr;
+
+	list_for_each_entry(mgr, &firmwaremgr_list, list)
+		if (mgr->handler->dev->parent->device_node == np)
+			return mgr;
+
+	return NULL;
+}
+
 /*
  * firmwaremgr_list_handlers - list registered firmware device handlers
  *                             in pretty format
diff --git a/include/firmware.h b/include/firmware.h
index 284e0f9705..ccd3b0669b 100644
--- a/include/firmware.h
+++ b/include/firmware.h
@@ -34,6 +34,7 @@ struct firmware_mgr;
 int firmwaremgr_register(struct firmware_handler *);
 
 struct firmware_mgr *firmwaremgr_find(const char *);
+struct firmware_mgr *firmwaremgr_find_by_node(const struct device_node *);
 
 void firmwaremgr_list_handlers(void);
 
-- 
2.20.1


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

[RFC PATCH 6/7] firmware: add support for fpga-regions

[Michael Tretter]

The Barebox fpga-region driver is merely glue code between the
devicetree overlay support and the various firmware handlers.

The driver registers notifiers for each fpga-region in the devicetree.
If an overlay is registered for a fpga-region, it uses the referenced
firmware handler to load the specified firmware image.

Signed-off-by: Michael Tretter <m.tretter@pengutronix.de>
---
 drivers/firmware/Kconfig          |   7 ++
 drivers/firmware/Makefile         |   1 +
 drivers/firmware/of-fpga-region.c | 153 ++++++++++++++++++++++++++++++
 3 files changed, 161 insertions(+)
 create mode 100644 drivers/firmware/of-fpga-region.c

diff --git a/drivers/firmware/Kconfig b/drivers/firmware/Kconfig
index 710b500ab0..95167efae8 100644
--- a/drivers/firmware/Kconfig
+++ b/drivers/firmware/Kconfig
@@ -14,4 +14,11 @@ config FIRMWARE_ALTERA_SOCFPGA
 	bool "Altera SoCFPGA fpga loader"
 	depends on ARCH_SOCFPGA
 	select FIRMWARE
+
+config OF_FPGA_REGION
+        tristate "FPGA Region Device Tree Overlay Support"
+        help
+          Support for loading FPGA images by applying a Device Tree
+          overlay.
+
 endmenu
diff --git a/drivers/firmware/Makefile b/drivers/firmware/Makefile
index c3a3c34004..f648ad6259 100644
--- a/drivers/firmware/Makefile
+++ b/drivers/firmware/Makefile
@@ -1,2 +1,3 @@
 obj-$(CONFIG_FIRMWARE_ALTERA_SERIAL) += altera_serial.o
 obj-$(CONFIG_FIRMWARE_ALTERA_SOCFPGA) += socfpga.o
+obj-$(CONFIG_OF_FPGA_REGION) += of-fpga-region.o
diff --git a/drivers/firmware/of-fpga-region.c b/drivers/firmware/of-fpga-region.c
new file mode 100644
index 0000000000..dcc6b66d81
--- /dev/null
+++ b/drivers/firmware/of-fpga-region.c
@@ -0,0 +1,153 @@
+// SPDX-License-Identifier: GPL-2.0
+/*
+ * FPGA Region - Device Tree support for FPGA programming under Linux
+ *
+ *  Copyright (C) 2013-2016 Altera Corporation
+ *  Copyright (C) 2017 Intel Corporation
+ *  Copyright (C) 2019 Pengutronix, Michael Tretter <m.tretter@pengutronix.de>
+ */
+#include <common.h>
+#include <globalvar.h>
+#include <firmware.h>
+#include <init.h>
+#include <magicvar.h>
+#include <of.h>
+
+char *firmware_path;
+
+struct fpga_region {
+	struct device_d *dev;
+	struct notifier_block nb;
+	struct firmware_mgr *mgr;
+};
+
+static struct firmware_mgr *of_fpga_region_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);
+		}
+	} while ((np = of_get_parent(np)) != NULL);
+
+	return NULL;
+}
+
+static int of_fpga_region_notify(struct notifier_block *nb,
+				 unsigned long action, void *arg)
+{
+	struct of_overlay_notify_data *nd = arg;
+	struct fpga_region *region = container_of(nb, struct fpga_region, nb);
+	const char *firmware_name;
+	char *firmware;
+	int err;
+
+	if (action != OF_OVERLAY_PRE_APPLY) {
+		dev_dbg(region->dev, "only interested in pre-apply hook\n");
+		return 0;
+	}
+
+	if (nd->target != region->dev->device_node) {
+		dev_dbg(region->dev, "%s is not overlay target %s\n",
+			nd->target->name, region->dev->device_node->name);
+		return 0;
+	}
+
+	err = of_property_read_string(nd->target,
+				      "firmware-name", &firmware_name);
+	if (err != -EINVAL) {
+		dev_dbg(region->dev,
+			"%s has already been programmed with firmware %s\n",
+			nd->target->name, firmware_name);
+		return 0;
+	}
+
+	err = of_property_read_string(nd->overlay,
+				      "firmware-name", &firmware_name);
+	if (err)
+		return err;
+
+	firmware = basprintf("%s/%s", firmware_path, firmware_name);
+	if (!firmware) {
+		err = -ENOMEM;
+		goto out;
+	}
+
+	dev_dbg(region->dev, "programming %s with firmware %s\n",
+		nd->target->name, firmware);
+
+	err = firmwaremgr_load_file(region->mgr, firmware);
+	if (err) {
+		dev_err("programming %s failed: %s\n", strerror(-err));
+		goto out;
+	}
+
+	/*
+	 * Add the firmware-name to the live device tree to indicate that the
+	 * FPGA has been programmed with this image.
+	 */
+	of_property_write_string(nd->target, "firmware-name", firmware_name);
+
+out:
+	free(firmware);
+
+	return err;
+}
+
+static int of_fpga_region_probe(struct device_d *dev)
+{
+	struct device_node *np = dev->device_node;
+	struct fpga_region *region;
+	struct firmware_mgr *mgr;
+	int err;
+
+	mgr = of_fpga_region_get_mgr(np);
+	if (!mgr) {
+		pr_err("cannot find firmware loader for %s\n", np->name);
+		return -EINVAL;
+	}
+
+	region = kzalloc(sizeof(*region), GFP_KERNEL);
+	if (!region)
+		return -ENOMEM;
+
+	region->dev = dev;
+	region->mgr = mgr;
+	region->nb.notifier_call = of_fpga_region_notify;
+
+	err = of_overlay_notifier_register(&region->nb);
+	if (err)
+		goto out;
+
+	return 0;
+
+out:
+	kfree(region);
+	return err;
+}
+
+static const struct of_device_id fpga_region_of_match[] = {
+	{ .compatible = "fpga-region", },
+	{},
+};
+
+static struct driver_d of_fpga_region_driver = {
+	.name = "of-region",
+	.probe = of_fpga_region_probe,
+	.of_compatible = DRV_OF_COMPAT(fpga_region_of_match),
+};
+device_platform_driver(of_fpga_region_driver);
+
+static int fpga_region_init(void)
+{
+	globalvar_add_simple_string("firmware.path", &firmware_path);
+
+	return 0;
+}
+postcore_initcall(fpga_region_init);
+
+BAREBOX_MAGICVAR_NAMED(global_firmware_path, global.firmware.path,
+		       "Firmware search path");
-- 
2.20.1


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

[RFC PATCH 7/7] blspec: add support for devicetree overlays

[Michael Tretter]

Read the devicetree-overlay property from the blspec entry, set the
firmware search-path based on the blspec root and register a devicetree
overlay when booting the blspec entry.

Signed-off-by: Michael Tretter <m.tretter@pengutronix.de>
---
 Documentation/user/booting-linux.rst |  4 ++++
 common/blspec.c                      | 10 ++++++++
 common/bootm.c                       | 35 ++++++++++++++++++++++++++++
 include/bootm.h                      |  1 +
 4 files changed, 50 insertions(+)

diff --git a/Documentation/user/booting-linux.rst b/Documentation/user/booting-linux.rst
index 437f4e80ca..12cd505e71 100644
--- a/Documentation/user/booting-linux.rst
+++ b/Documentation/user/booting-linux.rst
@@ -232,6 +232,10 @@ device where the entry is found on. This makes it possible to use the same rootf
 image on different devices without having to specify a different root= option each
 time.
 
+Additionally to the options defined in the original spec, Barebox has the
+``devicetree-overlay`` option. This is a string value that refer to overlays
+that will be applied to the device tree before passing it to Linux.
+
 Network boot
 ------------
 
diff --git a/common/blspec.c b/common/blspec.c
index 41f2a4c534..d0f2f3b228 100644
--- a/common/blspec.c
+++ b/common/blspec.c
@@ -54,6 +54,7 @@ static int blspec_boot(struct bootentry *be, int verbose, int dryrun)
 	struct blspec_entry *entry = container_of(be, struct blspec_entry, entry);
 	int ret;
 	const char *abspath, *devicetree, *options, *initrd, *linuximage;
+	const char *devicetree_overlay;
 	const char *appendroot;
 	struct bootm_data data = {
 		.initrd_address = UIMAGE_INVALID_ADDRESS,
@@ -69,6 +70,7 @@ static int blspec_boot(struct bootentry *be, int verbose, int dryrun)
 	initrd = blspec_entry_var_get(entry, "initrd");
 	options = blspec_entry_var_get(entry, "options");
 	linuximage = blspec_entry_var_get(entry, "linux");
+	devicetree_overlay = blspec_entry_var_get(entry, "devicetree-overlay");
 
 	if (entry->rootpath)
 		abspath = entry->rootpath;
@@ -88,6 +90,13 @@ static int blspec_boot(struct bootentry *be, int verbose, int dryrun)
 		}
 	}
 
+	if (devicetree_overlay) {
+		data.oftree_overlay_file = basprintf("%s/%s", abspath,
+						     devicetree_overlay);
+		setenv("global.firmware.path",
+		       basprintf("%s/%s", abspath, "lib/firmware/"));
+	}
+
 	if (initrd)
 		data.initrd_file = basprintf("%s/%s", abspath, initrd);
 
@@ -114,6 +123,7 @@ static int blspec_boot(struct bootentry *be, int verbose, int dryrun)
 	if (ret)
 		pr_err("Booting failed\n");
 err_out:
+	free((char *)data.oftree_overlay_file);
 	free((char *)data.oftree_file);
 	free((char *)data.initrd_file);
 	free((char *)data.os_file);
diff --git a/common/bootm.c b/common/bootm.c
index 36f6c41bbd..a796e89857 100644
--- a/common/bootm.c
+++ b/common/bootm.c
@@ -499,6 +499,32 @@ static int bootm_open_os_uimage(struct image_data *data)
 	return 0;
 }
 
+static int bootm_apply_oftree_overlay(const char *path)
+{
+	int ret;
+	struct fdt_header *fdt;
+	struct device_node *root;
+
+	fdt = read_file(path, NULL);
+	if (!fdt) {
+		printf("Unable to read \"%s\"\n", path);
+		return -EINVAL;
+	}
+
+	root = of_unflatten_dtb(fdt);
+	if (IS_ERR(root)) {
+		printf("\"%s\" is not a valid devicetree\n", path);
+		return -EINVAL;
+	}
+
+	ret = of_register_overlay(root);
+	if (ret)
+		pr_warn("Failed to register devicetree overlay \"%s\"\n",
+			path);
+
+	return 0;
+}
+
 static void bootm_print_info(struct image_data *data)
 {
 	if (data->os_res)
@@ -631,6 +657,15 @@ int bootm_boot(struct bootm_data *bootm_data)
 		}
 	}
 
+	if (bootm_data->oftree_overlay_file) {
+		ret = bootm_apply_oftree_overlay(bootm_data->oftree_overlay_file);
+		if (ret) {
+			printf("Applying device tree overlay failed with: %s\n",
+					strerror(-ret));
+			ret = 0;
+		}
+	}
+
 	if (bootm_data->appendroot) {
 		char *rootarg;
 
diff --git a/include/bootm.h b/include/bootm.h
index fdc73f711a..d5afd3f018 100644
--- a/include/bootm.h
+++ b/include/bootm.h
@@ -16,6 +16,7 @@ struct bootm_data {
 	const char *os_file;
 	const char *initrd_file;
 	const char *oftree_file;
+	const char *oftree_overlay_file;
 	int verbose;
 	enum bootm_verify verify;
 	bool force;
-- 
2.20.1


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

Re: [RFC PATCH 6/7] firmware: add support for fpga-regions

[Sascha Hauer]

On Thu, Apr 04, 2019 at 04:53:19PM +0200, Michael Tretter wrote:
> The Barebox fpga-region driver is merely glue code between the
> devicetree overlay support and the various firmware handlers.
> 
> The driver registers notifiers for each fpga-region in the devicetree.
> If an overlay is registered for a fpga-region, it uses the referenced
> firmware handler to load the specified firmware image.
> 
> +#include <common.h>
> +#include <globalvar.h>
> +#include <firmware.h>
> +#include <init.h>
> +#include <magicvar.h>
> +#include <of.h>
> +
> +char *firmware_path;

static?

> +
> +struct fpga_region {
> +	struct device_d *dev;
> +	struct notifier_block nb;
> +	struct firmware_mgr *mgr;
> +};
> +
> +	if (action != OF_OVERLAY_PRE_APPLY) {
> +		dev_dbg(region->dev, "only interested in pre-apply hook\n");
> +		return 0;
> +	}
> +
> +	if (nd->target != region->dev->device_node) {
> +		dev_dbg(region->dev, "%s is not overlay target %s\n",
> +			nd->target->name, region->dev->device_node->name);
> +		return 0;
> +	}
> +
> +	err = of_property_read_string(nd->target,
> +				      "firmware-name", &firmware_name);
> +	if (err != -EINVAL) {

of_property_read_string could also return other error codes. I think you
should test for err == 0 here, maybe bail out with an error on other,
unexpected error codes.

> +		dev_dbg(region->dev,
> +			"%s has already been programmed with firmware %s\n",
> +			nd->target->name, firmware_name);
> +		return 0;
> +	}
> +
> +static int of_fpga_region_probe(struct device_d *dev)
> +{
> +	struct device_node *np = dev->device_node;
> +	struct fpga_region *region;
> +	struct firmware_mgr *mgr;
> +	int err;
> +
> +	mgr = of_fpga_region_get_mgr(np);
> +	if (!mgr) {
> +		pr_err("cannot find firmware loader for %s\n", np->name);

dev_err

> +		return -EINVAL;
> +	}
> +
> +	region = kzalloc(sizeof(*region), GFP_KERNEL);
> +	if (!region)
> +		return -ENOMEM;

You can use xzalloc here.

Sascha

-- 
Pengutronix e.K.                           |                             |
Industrial Linux Solutions                 | http://www.pengutronix.de/  |
Peiner Str. 6-8, 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: [RFC PATCH 7/7] blspec: add support for devicetree overlays

[Sascha Hauer]

On Thu, Apr 04, 2019 at 04:53:20PM +0200, Michael Tretter wrote:
> Read the devicetree-overlay property from the blspec entry, set the
> firmware search-path based on the blspec root and register a devicetree
> overlay when booting the blspec entry.
> 
> Signed-off-by: Michael Tretter <m.tretter@pengutronix.de>
> ---
>  Documentation/user/booting-linux.rst |  4 ++++
>  common/blspec.c                      | 10 ++++++++
>  common/bootm.c                       | 35 ++++++++++++++++++++++++++++
>  include/bootm.h                      |  1 +
>  4 files changed, 50 insertions(+)
> 
> diff --git a/Documentation/user/booting-linux.rst b/Documentation/user/booting-linux.rst
> index 437f4e80ca..12cd505e71 100644
> --- a/Documentation/user/booting-linux.rst
> +++ b/Documentation/user/booting-linux.rst
> @@ -232,6 +232,10 @@ device where the entry is found on. This makes it possible to use the same rootf
>  image on different devices without having to specify a different root= option each
>  time.
>  
> +Additionally to the options defined in the original spec, Barebox has the
> +``devicetree-overlay`` option. This is a string value that refer to overlays

s/refer/refers/

You say "overlays" in plural, but the code seems to handle only a single
overlay.

> +that will be applied to the device tree before passing it to Linux.
> +
>  Network boot
>  ------------
>  
> diff --git a/common/blspec.c b/common/blspec.c
> index 41f2a4c534..d0f2f3b228 100644
> --- a/common/blspec.c
> +++ b/common/blspec.c
> @@ -54,6 +54,7 @@ static int blspec_boot(struct bootentry *be, int verbose, int dryrun)
>  	struct blspec_entry *entry = container_of(be, struct blspec_entry, entry);
>  	int ret;
>  	const char *abspath, *devicetree, *options, *initrd, *linuximage;
> +	const char *devicetree_overlay;
>  	const char *appendroot;
>  	struct bootm_data data = {
>  		.initrd_address = UIMAGE_INVALID_ADDRESS,
> @@ -69,6 +70,7 @@ static int blspec_boot(struct bootentry *be, int verbose, int dryrun)
>  	initrd = blspec_entry_var_get(entry, "initrd");
>  	options = blspec_entry_var_get(entry, "options");
>  	linuximage = blspec_entry_var_get(entry, "linux");
> +	devicetree_overlay = blspec_entry_var_get(entry, "devicetree-overlay");
>  
>  	if (entry->rootpath)
>  		abspath = entry->rootpath;
> @@ -88,6 +90,13 @@ static int blspec_boot(struct bootentry *be, int verbose, int dryrun)
>  		}
>  	}
>  
> +	if (devicetree_overlay) {
> +		data.oftree_overlay_file = basprintf("%s/%s", abspath,
> +						     devicetree_overlay);
> +		setenv("global.firmware.path",
> +		       basprintf("%s/%s", abspath, "lib/firmware/"));

The memory allocated here can never be freed.

I know from the previous patches what you are trying to archieve here,
but really this is non obvious and deserves an explanation somewhere.

So the overlay specifies a (FPGA-) Firmware filename which is then
searched for in the /lib/firmware directory of the rootfs where the
blspec entry is found. To me this looks quite special, I'm not sure
we want to make this a generic case.

>  static void bootm_print_info(struct image_data *data)
>  {
>  	if (data->os_res)
> @@ -631,6 +657,15 @@ int bootm_boot(struct bootm_data *bootm_data)
>  		}
>  	}
>  
> +	if (bootm_data->oftree_overlay_file) {
> +		ret = bootm_apply_oftree_overlay(bootm_data->oftree_overlay_file);
> +		if (ret) {
> +			printf("Applying device tree overlay failed with: %s\n",
> +					strerror(-ret));
> +			ret = 0;
> +		}
> +	}

bootm has a dryrun mode. You probably don't want to do steps you can't
revert in this mode.

Sascha

-- 
Pengutronix e.K.                           |                             |
Industrial Linux Solutions                 | http://www.pengutronix.de/  |
Peiner Str. 6-8, 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: [RFC PATCH 1/7] commands: unify newlines for options

[Sascha Hauer]

On Thu, Apr 04, 2019 at 04:53:14PM +0200, Michael Tretter wrote:
> The BAREBOX_CMD_HELP_OPT macro adds a newline to the string, but users
> of the macro inconsistently add another newline resulting in empty
> newlines for some commands, but not for others. Remove any newline in
> the description and rely on BAREBOX_CMD_HELP_OPT for the formatting.
> 
> Signed-off-by: Michael Tretter <m.tretter@pengutronix.de>
> ---

This one is easy. Applied, no need to resend when you have to update
this series.

Sascha

-- 
Pengutronix e.K.                           |                             |
Industrial Linux Solutions                 | http://www.pengutronix.de/  |
Peiner Str. 6-8, 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