[PATCH v3 0/7] support overlays to the barebox live tree

[PATCH v3 0/7] support overlays to the barebox live tree

[Sascha Hauer]

Support for applying a device tree overlay to the barebox live tree
is already present. This series adds a few missing pieces:

- add support for applying overlays to the live tree from the command
  line
- add support for adding new I2C/SPI devices via overlays
- add Makefile magic to build dtbo files and dtbo.o files to include
  into the barebox binary from dtso files

Changes since v2:
- Don't build compressed dtbo files
- Do not build overlays for PBL
- rename symbols to __dtbo_ to avoid possible name clashes

Changes since v1:
- add a device_rescan() static inline wrapper
- add missing call to of_i2c_register_devices() in i2c_add_numbered_adapter()
- replace device_detect with device_rescan in of_device_create_on_demand()

Sascha Hauer (7):
  kbuild: Add target to build dtb overlay files
  driver: Add rescan hook to struct device
  i2c: implement rescan
  spi: Directly register SPI device
  spi: reduce scope of 'chip'
  spi: implement rescan
  of_overlay: Add option to apply overlay to live tree

 commands/of_overlay.c  | 38 ++++++++++++++++++++++++++++++++++----
 drivers/i2c/i2c.c      | 24 +++++++++---------------
 drivers/of/platform.c  | 12 +++++-------
 drivers/spi/spi.c      | 29 ++++++++++++++++++++++++-----
 include/driver.h       |  6 ++++++
 scripts/Makefile.build |  2 ++
 scripts/Makefile.lib   | 10 +++++++++-
 scripts/gen-dtbo-s     | 14 ++++++++++++++
 8 files changed, 103 insertions(+), 32 deletions(-)
 create mode 100755 scripts/gen-dtbo-s

-- 
2.30.2

[PATCH v3 1/7] kbuild: Add target to build dtb overlay files

[Sascha Hauer]

Device tree overlay files have the suffix dtso in source format
and dtbo in binary format. Add the necessary targets to build dtbo
files from dtso files and also dtbo.o files to include into the
barebox binary.
The overlay files shouldn't include the device tree snippets from
CONFIG_EXTERNAL_DTS_FRAGMENTS which makes it necessary to specify
these fragments as an argument to cmd_dtc.

Signed-off-by: Sascha Hauer <s.hauer@pengutronix.de>
---
 scripts/Makefile.build |  2 ++
 scripts/Makefile.lib   | 10 +++++++++-
 scripts/gen-dtbo-s     | 14 ++++++++++++++
 3 files changed, 25 insertions(+), 1 deletion(-)
 create mode 100755 scripts/gen-dtbo-s

diff --git a/scripts/Makefile.build b/scripts/Makefile.build
index 216f03677b..25347eee01 100644
--- a/scripts/Makefile.build
+++ b/scripts/Makefile.build
@@ -258,11 +258,13 @@ intermediate_targets = $(foreach sfx, $(2), \
 					$(filter %$(strip $(1)), $(targets))))
 # %.asn1.o <- %.asn1.[ch] <- %.asn1
 # %.dtb.o <- %.dtb.S <- %.dtb <- %.dts
+# %.dtbo.o <- %.dtbo.S <- %.dtbo <- %.dtso
 # %.dtb.pbl.o <- %.dtb.S <- %.dtb <- %.dts (Barebox only)
 # %.lex.o <- %.lex.c <- %.l
 # %.tab.o <- %.tab.[ch] <- %.y
 targets += $(call intermediate_targets, .asn1.o, .asn1.c .asn1.h) \
 	   $(call intermediate_targets, .dtb.o, .dtb.S .dtb.z .dtb) \
+	   $(call intermediate_targets, .dtbo.o, .dtbo.S .dtbo.z .dtbo) \
 	   $(call intermediate_targets, .dtb.pbl.o, .dtb.S .dtb.z .dtb) \
 	   $(call intermediate_targets, .lex.o, .lex.c) \
 	   $(call intermediate_targets, .tab.o, .tab.c .tab.h)
diff --git a/scripts/Makefile.lib b/scripts/Makefile.lib
index 6e0d92cf75..42c59a072c 100644
--- a/scripts/Makefile.lib
+++ b/scripts/Makefile.lib
@@ -361,13 +361,18 @@ cmd_dt_S_dtb = $(srctree)/scripts/gen-dtb-s $(subst -,_,$(*F)) $< $(CONFIG_IMD)
 $(obj)/%.dtb.S: $(obj)/%.dtb $(obj)/%.dtb.z $(srctree)/scripts/gen-dtb-s FORCE
 	$(call if_changed,dt_S_dtb)
 
+quiet_cmd_dt_S_dtbo = DTBO    $@
+cmd_dt_S_dtbo = $(srctree)/scripts/gen-dtbo-s $(subst -,_,$(*F)) $< > $@
+$(obj)/%.dtbo.S: $(obj)/%.dtbo $(srctree)/scripts/gen-dtbo-s FORCE
+	$(call if_changed,dt_S_dtbo)
+
 $(obj)/%.dtb.z: $(obj)/%.dtb FORCE
 	$(call if_changed,$(suffix_y))
 
 dts-frags = $(subst $(quote),,$(CONFIG_EXTERNAL_DTS_FRAGMENTS))
 quiet_cmd_dtc = DTC     $@
 # For compatibility between make 4.2 and 4.3
-cmd_dtc = /usr/bin/env echo -e '$(pound)define $(subst -,_,$(*F))_dts 1\n'$(foreach f,$< $(dts-frags),'$(pound)include "$(f)"\n') | \
+cmd_dtc = /usr/bin/env echo -e '$(pound)define $(subst -,_,$(*F))_dts 1\n'$(foreach f,$< $(2),'$(pound)include "$(f)"\n') | \
 	$(CPP) $(dtc_cpp_flags) -x assembler-with-cpp -o $(dtc-tmp) - ; \
 	$(objtree)/scripts/dtc/dtc -O dtb -o $@ -b 0 \
 		-i $(srctree)/arch/$(SRCARCH)/dts $(DTC_FLAGS) \
@@ -376,6 +381,9 @@ cmd_dtc = /usr/bin/env echo -e '$(pound)define $(subst -,_,$(*F))_dts 1\n'$(fore
 	cat $(depfile).pre $(depfile).dtc > $(depfile)
 
 $(obj)/%.dtb: $(src)/%.dts FORCE
+	$(call if_changed_dep,dtc,$(dts-frags))
+
+$(obj)/%.dtbo: $(src)/%.dtso $(DTC) FORCE
 	$(call if_changed_dep,dtc)
 
 dtc-tmp = $(subst $(comma),_,$(dot-target).dts)
diff --git a/scripts/gen-dtbo-s b/scripts/gen-dtbo-s
new file mode 100755
index 0000000000..06f78609ed
--- /dev/null
+++ b/scripts/gen-dtbo-s
@@ -0,0 +1,14 @@
+#!/usr/bin/env bash
+
+name=$1
+dtbo=$2
+
+echo "#include <asm/barebox.lds.h>"
+
+echo ".section .dtb.rodata.${name}_dtbo,\"a\""
+echo ".balign STRUCT_ALIGNMENT"
+echo ".global __dtbo_${name}_start"
+echo "__dtbo_${name}_start:"
+echo ".incbin \"$dtbo\""
+echo "__dtbo_${name}_end:"
+echo ".global __dtbo_${name}_end"
-- 
2.30.2

[PATCH v3 2/7] driver: Add rescan hook to struct device

[Sascha Hauer]

When devices are enabled with a device tree overlay the newly enabled
devices can be probed by doing a of_probe(). This works fine for the
regular platform devices, but doesn't work for devices which are not
probed by the core, but by the subsystem. Prominent examples are I2C or
SPI devices.

This patch adds a struct device::rescan hook that subsystems can
implement to trigger rescanning the device nodes.

Signed-off-by: Sascha Hauer <s.hauer@pengutronix.de>
---
 drivers/of/platform.c | 12 +++++-------
 include/driver.h      |  6 ++++++
 2 files changed, 11 insertions(+), 7 deletions(-)

diff --git a/drivers/of/platform.c b/drivers/of/platform.c
index edb082b106..23b8fa7934 100644
--- a/drivers/of/platform.c
+++ b/drivers/of/platform.c
@@ -153,8 +153,10 @@ struct device *of_platform_device_create(struct device_node *np,
 	 * Linux uses the OF_POPULATED flag to skip already populated/created
 	 * devices.
 	 */
-	if (np->dev)
+	if (np->dev) {
+		device_rescan(np->dev);
 		return np->dev;
+	}
 
 	/* count the io resources */
 	if (of_can_translate_address(np))
@@ -414,17 +416,13 @@ static struct device *of_device_create_on_demand(struct device_node *np)
 {
 	struct device_node *parent;
 	struct device *parent_dev, *dev;
-	int ret;
 
 	parent = of_get_parent(np);
 	if (!parent)
 		return NULL;
 
-	if (!np->dev && parent->dev) {
-		ret = device_detect(parent->dev);
-		if (ret && ret != -ENOSYS)
-			return ERR_PTR(ret);
-	}
+	if (!np->dev && parent->dev)
+		device_rescan(parent->dev);
 
 	if (!np->dev)
 		pr_debug("Creating device for %s\n", np->full_name);
diff --git a/include/driver.h b/include/driver.h
index f53668711b..2cf0190699 100644
--- a/include/driver.h
+++ b/include/driver.h
@@ -91,6 +91,7 @@ struct device {
 	 * when the driver should actually detect client devices
 	 */
 	int     (*detect) (struct device *);
+	void	(*rescan) (struct device *);
 
 	/*
 	 * if a driver probe is deferred, this stores the last error
@@ -158,6 +159,11 @@ static inline void put_device(struct device *dev) {}
 void free_device_res(struct device *dev);
 void free_device(struct device *dev);
 
+static inline void device_rescan(struct device *dev)
+{
+	if (dev->rescan)
+		dev->rescan(dev);
+}
 
 /* Iterate over a devices children
  */
-- 
2.30.2

[PATCH v3 3/7] i2c: implement rescan

[Sascha Hauer]

struct device::rescan is called by the core to let the subsystem rescan
the controllers device node. Implement that for I2C. Ahmad recently
implemented the struct device::detect hook for the very same purpose.
The downside of that approach was that device_detect() had to be called
manually and that either needs knowledge which device actually has
updated device nodes, or all devices were detected, like unrelated USB
or MMC/SD devices. The rescan hook doesn't need that manual detect call,
so just drop the I2C detect implementation in favour for implementing
rescan.

Signed-off-by: Sascha Hauer <s.hauer@pengutronix.de>
---
 drivers/i2c/i2c.c | 24 +++++++++---------------
 1 file changed, 9 insertions(+), 15 deletions(-)

diff --git a/drivers/i2c/i2c.c b/drivers/i2c/i2c.c
index efcad29342..4c7b7008ce 100644
--- a/drivers/i2c/i2c.c
+++ b/drivers/i2c/i2c.c
@@ -420,6 +420,10 @@ static void of_i2c_register_devices(struct i2c_adapter *adap)
 {
 	struct device_node *n;
 
+	/* Only register child devices if the adapter has a node pointer set */
+	if (!IS_ENABLED(CONFIG_OFDEVICE) || !adap->dev.of_node)
+		return;
+
 	for_each_available_child_of_node(adap->dev.of_node, n) {
 		struct i2c_board_info info = {};
 		struct i2c_client *result;
@@ -471,15 +475,7 @@ int of_i2c_register_devices_by_node(struct device_node *node)
 	return 0;
 }
 
-static int i2c_bus_detect(struct device *dev)
-{
-	struct i2c_adapter *adap = container_of(dev, struct i2c_adapter, dev);
-
-	of_i2c_register_devices(adap);
-	return 0;
-}
-
-static int i2c_hw_detect(struct device *dev)
+static void i2c_hw_rescan(struct device *dev)
 {
 	struct i2c_adapter *adap;
 
@@ -489,8 +485,6 @@ static int i2c_hw_detect(struct device *dev)
 		of_i2c_register_devices(adap);
 		break;
 	}
-
-	return 0;
 }
 
 /**
@@ -720,7 +714,6 @@ int i2c_add_numbered_adapter(struct i2c_adapter *adapter)
 	}
 
 	adapter->dev.id = adapter->nr;
-	adapter->dev.detect = i2c_bus_detect;
 	dev_set_name(&adapter->dev, "i2c");
 
 	ret = register_device(&adapter->dev);
@@ -732,11 +725,12 @@ int i2c_add_numbered_adapter(struct i2c_adapter *adapter)
 	/* populate children from any i2c device tables */
 	scan_boardinfo(adapter);
 
+	of_i2c_register_devices(adapter);
+
 	hw_dev = adapter->dev.parent;
 	if (hw_dev && dev_of_node(hw_dev)) {
-		if (!hw_dev->detect)
-			hw_dev->detect = i2c_hw_detect;
-		i2c_hw_detect(hw_dev);
+		if (!hw_dev->rescan)
+			hw_dev->rescan = i2c_hw_rescan;
 	}
 
 	return 0;
-- 
2.30.2

[PATCH v3 4/7] spi: Directly register SPI device

[Sascha Hauer]

SPI devices described in the device tree are not directly registered,
but instead added to the SPI board_list which is normally used to
register SPI devices from board code. This seems rather unnecessary,
drop this detour and register SPI devices directly from
spi_of_register_slaves().

Signed-off-by: Sascha Hauer <s.hauer@pengutronix.de>
---
 drivers/spi/spi.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/spi/spi.c b/drivers/spi/spi.c
index d6f939e51c..1444fe3741 100644
--- a/drivers/spi/spi.c
+++ b/drivers/spi/spi.c
@@ -138,7 +138,7 @@ static void spi_of_register_slaves(struct spi_controller *ctrl)
 			continue;
 		chip.chip_select = of_read_number(reg->value, 1);
 		chip.device_node = n;
-		spi_register_board_info(&chip, 1);
+		spi_new_device(ctrl, &chip);
 	}
 }
 
-- 
2.30.2

[PATCH v3 5/7] spi: reduce scope of 'chip'

[Sascha Hauer]

Reducde scope of struct spi_board_info chip to the loop where it is
actually only used. While at it drop the memset and let the compiler
do the work of initialising it.

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

diff --git a/drivers/spi/spi.c b/drivers/spi/spi.c
index 1444fe3741..3684647f6e 100644
--- a/drivers/spi/spi.c
+++ b/drivers/spi/spi.c
@@ -108,7 +108,6 @@ EXPORT_SYMBOL(spi_new_device);
 static void spi_of_register_slaves(struct spi_controller *ctrl)
 {
 	struct device_node *n;
-	struct spi_board_info chip;
 	struct property *reg;
 	struct device_node *node = ctrl->dev->of_node;
 
@@ -119,7 +118,8 @@ static void spi_of_register_slaves(struct spi_controller *ctrl)
 		return;
 
 	for_each_available_child_of_node(node, n) {
-		memset(&chip, 0, sizeof(chip));
+		struct spi_board_info chip = {};
+
 		chip.name = xstrdup(n->name);
 		chip.bus_num = ctrl->bus_num;
 		/* Mode (clock phase/polarity/etc.) */
-- 
2.30.2

[PATCH v3 6/7] spi: implement rescan

[Sascha Hauer]

Implement rescan to register and probe newly added devices via device
tree overlays.

While this is easy to do at SPI core level this is not the whole truth.
Many SPI controllers do not use their native chipselects for the SPI
devices, but GPIOs instead. These currently won't be rescanned when new
devices are added, so the chipselects for the new devices must be be
present in the base device tree already. You have been warned.

Signed-off-by: Sascha Hauer <s.hauer@pengutronix.de>
---
 drivers/spi/spi.c | 23 +++++++++++++++++++++--
 1 file changed, 21 insertions(+), 2 deletions(-)

diff --git a/drivers/spi/spi.c b/drivers/spi/spi.c
index 3684647f6e..584d4ab777 100644
--- a/drivers/spi/spi.c
+++ b/drivers/spi/spi.c
@@ -29,6 +29,7 @@ struct boardinfo {
 };
 
 static LIST_HEAD(board_list);
+static LIST_HEAD(spi_controller_list);
 
 /**
  * spi_new_device - instantiate one new SPI device
@@ -120,6 +121,12 @@ static void spi_of_register_slaves(struct spi_controller *ctrl)
 	for_each_available_child_of_node(node, n) {
 		struct spi_board_info chip = {};
 
+		if (n->dev) {
+			dev_dbg(ctrl->dev, "skipping already registered %s\n",
+				dev_name(n->dev));
+			continue;
+		}
+
 		chip.name = xstrdup(n->name);
 		chip.bus_num = ctrl->bus_num;
 		/* Mode (clock phase/polarity/etc.) */
@@ -142,6 +149,17 @@ static void spi_of_register_slaves(struct spi_controller *ctrl)
 	}
 }
 
+static void spi_controller_rescan(struct device *dev)
+{
+	struct spi_controller *ctrl;
+
+	list_for_each_entry(ctrl, &spi_controller_list, list) {
+		if (ctrl->dev != dev)
+			continue;
+		spi_of_register_slaves(ctrl);
+	}
+}
+
 /**
  * spi_register_board_info - register SPI devices for a given board
  * @info: array of chip descriptors
@@ -196,8 +214,6 @@ static void scan_boardinfo(struct spi_controller *ctrl)
 	}
 }
 
-static LIST_HEAD(spi_controller_list);
-
 static int spi_controller_check_ops(struct spi_controller *ctlr)
 {
 	/*
@@ -274,6 +290,9 @@ int spi_register_controller(struct spi_controller *ctrl)
 	scan_boardinfo(ctrl);
 	status = 0;
 
+	if (!ctrl->dev->rescan)
+		ctrl->dev->rescan = spi_controller_rescan;
+
 	return status;
 }
 EXPORT_SYMBOL(spi_register_controller);
-- 
2.30.2

[PATCH v3 7/7] of_overlay: Add option to apply overlay to live tree

[Sascha Hauer]

The of_overlay command currently only supports applying overlays to the
Linux device tree. Add an option to apply an overlay to the live tree.
Using this option will apply the overlay and also triggers rescanning
the device tree in case new devices have been added.

Signed-off-by: Sascha Hauer <s.hauer@pengutronix.de>
---
 commands/of_overlay.c | 38 ++++++++++++++++++++++++++++++++++----
 1 file changed, 34 insertions(+), 4 deletions(-)

diff --git a/commands/of_overlay.c b/commands/of_overlay.c
index b3660b4bf1..1d68e31ef2 100644
--- a/commands/of_overlay.c
+++ b/commands/of_overlay.c
@@ -17,10 +17,20 @@ static int do_of_overlay(int argc, char *argv[])
 	struct fdt_header *fdt;
 	struct device_node *overlay;
 	size_t size;
+	bool live_tree = false;
+	int opt;
 
-	if (argc != 2)
+	if (argc < 2)
 		return COMMAND_ERROR_USAGE;
 
+	while ((opt = getopt(argc, argv, "l")) > 0) {
+		switch (opt) {
+		case 'l':
+			live_tree = true;
+			break;
+		}
+	}
+
 	fdt = read_file(argv[optind], &size);
 	if (!fdt) {
 		printf("cannot read %s\n", argv[optind]);
@@ -32,7 +42,14 @@ static int do_of_overlay(int argc, char *argv[])
 	if (IS_ERR(overlay))
 		return PTR_ERR(overlay);
 
-	ret = of_register_overlay(overlay);
+	if (live_tree) {
+		ret = of_overlay_apply_tree(of_get_root_node(), overlay);
+		if (!ret)
+			ret = of_probe();
+	} else {
+		ret = of_register_overlay(overlay);
+	}
+
 	if (ret) {
 		printf("cannot apply oftree overlay: %s\n", strerror(-ret));
 		goto err;
@@ -45,9 +62,22 @@ err:
 	return ret;
 }
 
+BAREBOX_CMD_HELP_START(of_overlay)
+BAREBOX_CMD_HELP_TEXT("Register a device tree overlay file (dtbo) with barebox.")
+BAREBOX_CMD_HELP_TEXT("By default the overlay is registered as a fixup and the")
+BAREBOX_CMD_HELP_TEXT("overlay will then be applied to the Linux device tree.")
+BAREBOX_CMD_HELP_TEXT("With -l given the overlay is applied to the barebox live")
+BAREBOX_CMD_HELP_TEXT("tree instead. This involves probing new devices added in")
+BAREBOX_CMD_HELP_TEXT("the overlay file.")
+BAREBOX_CMD_HELP_TEXT("")
+BAREBOX_CMD_HELP_TEXT("Options:")
+BAREBOX_CMD_HELP_OPT("-l", "apply to barebox live tree")
+BAREBOX_CMD_HELP_END
+
 BAREBOX_CMD_START(of_overlay)
 	.cmd = do_of_overlay,
-	BAREBOX_CMD_DESC("register device tree overlay as fixup")
-	BAREBOX_CMD_OPTS("FILE")
+	BAREBOX_CMD_DESC("register device tree overlay")
+	BAREBOX_CMD_OPTS("[-l] FILE")
 	BAREBOX_CMD_GROUP(CMD_GRP_MISC)
+	BAREBOX_CMD_HELP(cmd_of_overlay_help)
 BAREBOX_CMD_END
-- 
2.30.2

Re: [PATCH v3 7/7] of_overlay: Add option to apply overlay to live tree

[Michael Riesch]

Hi Sascha,

On 3/10/23 10:42, Sascha Hauer wrote:
> The of_overlay command currently only supports applying overlays to the
> Linux device tree. Add an option to apply an overlay to the live tree.
> Using this option will apply the overlay and also triggers rescanning
> the device tree in case new devices have been added.
> 
> Signed-off-by: Sascha Hauer <s.hauer@pengutronix.de>
> ---
>  commands/of_overlay.c | 38 ++++++++++++++++++++++++++++++++++----
>  1 file changed, 34 insertions(+), 4 deletions(-)
> 
> diff --git a/commands/of_overlay.c b/commands/of_overlay.c
> index b3660b4bf1..1d68e31ef2 100644
> --- a/commands/of_overlay.c
> +++ b/commands/of_overlay.c
> @@ -17,10 +17,20 @@ static int do_of_overlay(int argc, char *argv[])
>  	struct fdt_header *fdt;
>  	struct device_node *overlay;
>  	size_t size;
> +	bool live_tree = false;
> +	int opt;
>  
> -	if (argc != 2)
> +	if (argc < 2)
>  		return COMMAND_ERROR_USAGE;
>  
> +	while ((opt = getopt(argc, argv, "l")) > 0) {
> +		switch (opt) {
> +		case 'l':
> +			live_tree = true;
> +			break;
> +		}
> +	}
> +
>  	fdt = read_file(argv[optind], &size);

of course this is not the intended usage, but "of_overlay -l" (i.e.,
skipping the overlay argument) throws an exception.

>  	if (!fdt) {
>  		printf("cannot read %s\n", argv[optind]);
> @@ -32,7 +42,14 @@ static int do_of_overlay(int argc, char *argv[])
>  	if (IS_ERR(overlay))
>  		return PTR_ERR(overlay);
>  
> -	ret = of_register_overlay(overlay);
> +	if (live_tree) {
> +		ret = of_overlay_apply_tree(of_get_root_node(), overlay);

If I pass a meaningful argument ("of_overlay -l path_to_my_overlay"), I get

ERROR: of_resolver: __symbols__ missing from base devicetree
cannot apply oftree overlay: Invalid argument
of_overlay: Invalid argument

in return. The base device tree does not contain any __symbols__
sections indeed. How can I activate the generation of this section?

Thanks and regards,
Michael

> +		if (!ret)
> +			ret = of_probe();
> +	} else {
> +		ret = of_register_overlay(overlay);
> +	}
> +
>  	if (ret) {
>  		printf("cannot apply oftree overlay: %s\n", strerror(-ret));
>  		goto err;
> @@ -45,9 +62,22 @@ err:
>  	return ret;
>  }
>  
> +BAREBOX_CMD_HELP_START(of_overlay)
> +BAREBOX_CMD_HELP_TEXT("Register a device tree overlay file (dtbo) with barebox.")
> +BAREBOX_CMD_HELP_TEXT("By default the overlay is registered as a fixup and the")
> +BAREBOX_CMD_HELP_TEXT("overlay will then be applied to the Linux device tree.")
> +BAREBOX_CMD_HELP_TEXT("With -l given the overlay is applied to the barebox live")
> +BAREBOX_CMD_HELP_TEXT("tree instead. This involves probing new devices added in")
> +BAREBOX_CMD_HELP_TEXT("the overlay file.")
> +BAREBOX_CMD_HELP_TEXT("")
> +BAREBOX_CMD_HELP_TEXT("Options:")
> +BAREBOX_CMD_HELP_OPT("-l", "apply to barebox live tree")
> +BAREBOX_CMD_HELP_END
> +
>  BAREBOX_CMD_START(of_overlay)
>  	.cmd = do_of_overlay,
> -	BAREBOX_CMD_DESC("register device tree overlay as fixup")
> -	BAREBOX_CMD_OPTS("FILE")
> +	BAREBOX_CMD_DESC("register device tree overlay")
> +	BAREBOX_CMD_OPTS("[-l] FILE")
>  	BAREBOX_CMD_GROUP(CMD_GRP_MISC)
> +	BAREBOX_CMD_HELP(cmd_of_overlay_help)
>  BAREBOX_CMD_END

Re: [PATCH v3 7/7] of_overlay: Add option to apply overlay to live tree

[Sascha Hauer]

On Tue, Mar 14, 2023 at 03:59:29PM +0100, Michael Riesch wrote:
> Hi Sascha,
> 
> On 3/10/23 10:42, Sascha Hauer wrote:
> > The of_overlay command currently only supports applying overlays to the
> > Linux device tree. Add an option to apply an overlay to the live tree.
> > Using this option will apply the overlay and also triggers rescanning
> > the device tree in case new devices have been added.
> > 
> > Signed-off-by: Sascha Hauer <s.hauer@pengutronix.de>
> > ---
> >  commands/of_overlay.c | 38 ++++++++++++++++++++++++++++++++++----
> >  1 file changed, 34 insertions(+), 4 deletions(-)
> > 
> > diff --git a/commands/of_overlay.c b/commands/of_overlay.c
> > index b3660b4bf1..1d68e31ef2 100644
> > --- a/commands/of_overlay.c
> > +++ b/commands/of_overlay.c
> > @@ -17,10 +17,20 @@ static int do_of_overlay(int argc, char *argv[])
> >  	struct fdt_header *fdt;
> >  	struct device_node *overlay;
> >  	size_t size;
> > +	bool live_tree = false;
> > +	int opt;
> >  
> > -	if (argc != 2)
> > +	if (argc < 2)
> >  		return COMMAND_ERROR_USAGE;
> >  
> > +	while ((opt = getopt(argc, argv, "l")) > 0) {
> > +		switch (opt) {
> > +		case 'l':
> > +			live_tree = true;
> > +			break;
> > +		}
> > +	}
> > +
> >  	fdt = read_file(argv[optind], &size);
> 
> of course this is not the intended usage, but "of_overlay -l" (i.e.,
> skipping the overlay argument) throws an exception.

Okay, will fix.

> 
> >  	if (!fdt) {
> >  		printf("cannot read %s\n", argv[optind]);
> > @@ -32,7 +42,14 @@ static int do_of_overlay(int argc, char *argv[])
> >  	if (IS_ERR(overlay))
> >  		return PTR_ERR(overlay);
> >  
> > -	ret = of_register_overlay(overlay);
> > +	if (live_tree) {
> > +		ret = of_overlay_apply_tree(of_get_root_node(), overlay);
> 
> If I pass a meaningful argument ("of_overlay -l path_to_my_overlay"), I get
> 
> ERROR: of_resolver: __symbols__ missing from base devicetree
> cannot apply oftree overlay: Invalid argument
> of_overlay: Invalid argument
> 
> in return. The base device tree does not contain any __symbols__
> sections indeed. How can I activate the generation of this section?

Please enable CONFIG_OF_OVERLAY_LIVE. Maybe we can improve this a bit to
make this clearer somehow / somewhere,

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 |

Re: [PATCH v3 7/7] of_overlay: Add option to apply overlay to live tree

[Michael Riesch]

Hello Sascha,

On 3/14/23 16:49, Sascha Hauer wrote:
> On Tue, Mar 14, 2023 at 03:59:29PM +0100, Michael Riesch wrote:
>> Hi Sascha,
>>
>> On 3/10/23 10:42, Sascha Hauer wrote:
>>> The of_overlay command currently only supports applying overlays to the
>>> Linux device tree. Add an option to apply an overlay to the live tree.
>>> Using this option will apply the overlay and also triggers rescanning
>>> the device tree in case new devices have been added.
>>>
>>> Signed-off-by: Sascha Hauer <s.hauer@pengutronix.de>
>>> ---
>>>  commands/of_overlay.c | 38 ++++++++++++++++++++++++++++++++++----
>>>  1 file changed, 34 insertions(+), 4 deletions(-)
>>>
>>> diff --git a/commands/of_overlay.c b/commands/of_overlay.c
>>> index b3660b4bf1..1d68e31ef2 100644
>>> --- a/commands/of_overlay.c
>>> +++ b/commands/of_overlay.c
>>> @@ -17,10 +17,20 @@ static int do_of_overlay(int argc, char *argv[])
>>>  	struct fdt_header *fdt;
>>>  	struct device_node *overlay;
>>>  	size_t size;
>>> +	bool live_tree = false;
>>> +	int opt;
>>>  
>>> -	if (argc != 2)
>>> +	if (argc < 2)
>>>  		return COMMAND_ERROR_USAGE;
>>>  
>>> +	while ((opt = getopt(argc, argv, "l")) > 0) {
>>> +		switch (opt) {
>>> +		case 'l':
>>> +			live_tree = true;
>>> +			break;
>>> +		}
>>> +	}
>>> +
>>>  	fdt = read_file(argv[optind], &size);
>>
>> of course this is not the intended usage, but "of_overlay -l" (i.e.,
>> skipping the overlay argument) throws an exception.
> 
> Okay, will fix.
> 
>>
>>>  	if (!fdt) {
>>>  		printf("cannot read %s\n", argv[optind]);
>>> @@ -32,7 +42,14 @@ static int do_of_overlay(int argc, char *argv[])
>>>  	if (IS_ERR(overlay))
>>>  		return PTR_ERR(overlay);
>>>  
>>> -	ret = of_register_overlay(overlay);
>>> +	if (live_tree) {
>>> +		ret = of_overlay_apply_tree(of_get_root_node(), overlay);
>>
>> If I pass a meaningful argument ("of_overlay -l path_to_my_overlay"), I get
>>
>> ERROR: of_resolver: __symbols__ missing from base devicetree
>> cannot apply oftree overlay: Invalid argument
>> of_overlay: Invalid argument
>>
>> in return. The base device tree does not contain any __symbols__
>> sections indeed. How can I activate the generation of this section?
> 
> Please enable CONFIG_OF_OVERLAY_LIVE. Maybe we can improve this a bit to
> make this clearer somehow / somewhere,

This seems to do the trick, thanks! A bit of documentation would be
great, though.

Thanks and best regards,
Michael

Re: [PATCH v3 0/7] support overlays to the barebox live tree

[Michael Riesch]

Hi Sascha,

On 3/10/23 10:42, Sascha Hauer wrote:
> Support for applying a device tree overlay to the barebox live tree
> is already present. This series adds a few missing pieces:
> 
> - add support for applying overlays to the live tree from the command
>   line
> - add support for adding new I2C/SPI devices via overlays
> - add Makefile magic to build dtbo files and dtbo.o files to include
>   into the barebox binary from dtso files
> 
> Changes since v2:
> - Don't build compressed dtbo files
> - Do not build overlays for PBL
> - rename symbols to __dtbo_ to avoid possible name clashes
> 
> Changes since v1:
> - add a device_rescan() static inline wrapper
> - add missing call to of_i2c_register_devices() in i2c_add_numbered_adapter()
> - replace device_detect with device_rescan in of_device_create_on_demand()
> 
> Sascha Hauer (7):
>   kbuild: Add target to build dtb overlay files
>   driver: Add rescan hook to struct device
>   i2c: implement rescan
>   spi: Directly register SPI device
>   spi: reduce scope of 'chip'
>   spi: implement rescan
>   of_overlay: Add option to apply overlay to live tree
> 
>  commands/of_overlay.c  | 38 ++++++++++++++++++++++++++++++++++----
>  drivers/i2c/i2c.c      | 24 +++++++++---------------
>  drivers/of/platform.c  | 12 +++++-------
>  drivers/spi/spi.c      | 29 ++++++++++++++++++++++++-----
>  include/driver.h       |  6 ++++++
>  scripts/Makefile.build |  2 ++
>  scripts/Makefile.lib   | 10 +++++++++-
>  scripts/gen-dtbo-s     | 14 ++++++++++++++
>  8 files changed, 103 insertions(+), 32 deletions(-)
>  create mode 100755 scripts/gen-dtbo-s
> 

With this series I can apply a device tree overlay, but a node with a
"fixed-clock" does not lead to a corresponding clock showing up in clk_dump.

Any idea what went wrong here?

Thanks and best regards,
Michael

Re: [PATCH v3 0/7] support overlays to the barebox live tree

[Sascha Hauer]

On Wed, Mar 15, 2023 at 05:09:47PM +0100, Michael Riesch wrote:
> Hi Sascha,
> 
> On 3/10/23 10:42, Sascha Hauer wrote:
> > Support for applying a device tree overlay to the barebox live tree
> > is already present. This series adds a few missing pieces:
> > 
> > - add support for applying overlays to the live tree from the command
> >   line
> > - add support for adding new I2C/SPI devices via overlays
> > - add Makefile magic to build dtbo files and dtbo.o files to include
> >   into the barebox binary from dtso files
> > 
> > Changes since v2:
> > - Don't build compressed dtbo files
> > - Do not build overlays for PBL
> > - rename symbols to __dtbo_ to avoid possible name clashes
> > 
> > Changes since v1:
> > - add a device_rescan() static inline wrapper
> > - add missing call to of_i2c_register_devices() in i2c_add_numbered_adapter()
> > - replace device_detect with device_rescan in of_device_create_on_demand()
> > 
> > Sascha Hauer (7):
> >   kbuild: Add target to build dtb overlay files
> >   driver: Add rescan hook to struct device
> >   i2c: implement rescan
> >   spi: Directly register SPI device
> >   spi: reduce scope of 'chip'
> >   spi: implement rescan
> >   of_overlay: Add option to apply overlay to live tree
> > 
> >  commands/of_overlay.c  | 38 ++++++++++++++++++++++++++++++++++----
> >  drivers/i2c/i2c.c      | 24 +++++++++---------------
> >  drivers/of/platform.c  | 12 +++++-------
> >  drivers/spi/spi.c      | 29 ++++++++++++++++++++++++-----
> >  include/driver.h       |  6 ++++++
> >  scripts/Makefile.build |  2 ++
> >  scripts/Makefile.lib   | 10 +++++++++-
> >  scripts/gen-dtbo-s     | 14 ++++++++++++++
> >  8 files changed, 103 insertions(+), 32 deletions(-)
> >  create mode 100755 scripts/gen-dtbo-s
> > 
> 
> With this series I can apply a device tree overlay, but a node with a
> "fixed-clock" does not lead to a corresponding clock showing up in clk_dump.
> 
> Any idea what went wrong here?

The fixed-clock is initialized with CLK_OF_DECLARE which results in a
linker list which is iterated over in of_clk_init() which is called
in an initcall during barebox startup.

The clocks registered via CLK_OF_DECLARE do not have a device associated
with them. The clocks also do not have a link to the device node they
provide the clock for, also the device node doesn't have a link to the
clock. This means we currently have no idea whether there already is
a clock registered for a particular node or if the node has just been
added by an overlay. There seems to be some work necessary to get this
working.

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 |