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

[PATCH v2 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 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      | 20 +++++---------------
 drivers/of/platform.c  | 11 +++++------
 drivers/spi/spi.c      | 29 ++++++++++++++++++++++++-----
 include/driver.h       |  6 ++++++
 scripts/Makefile.build |  4 ++++
 scripts/Makefile.lib   | 11 ++++++++++-
 7 files changed, 88 insertions(+), 31 deletions(-)

-- 
2.30.2

[PATCH v2 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 |  4 ++++
 scripts/Makefile.lib   | 11 ++++++++++-
 2 files changed, 14 insertions(+), 1 deletion(-)

diff --git a/scripts/Makefile.build b/scripts/Makefile.build
index 216f03677b..33480a81b4 100644
--- a/scripts/Makefile.build
+++ b/scripts/Makefile.build
@@ -258,12 +258,16 @@ 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)
+# %.dtbo.pbl.o <- %.dtbo.S <- %.dtbo <- %.dtso (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, .dtbo.pbl.o, .dtbo.S .dtbo.z .dtbo) \
 	   $(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..6af2fe07b2 100644
--- a/scripts/Makefile.lib
+++ b/scripts/Makefile.lib
@@ -361,13 +361,19 @@ 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)
 
+$(obj)/%.dtbo.S: $(obj)/%.dtbo $(obj)/%.dtbo.z $(srctree)/scripts/gen-dtb-s FORCE
+	$(call if_changed,dt_S_dtb)
+
 $(obj)/%.dtb.z: $(obj)/%.dtb FORCE
 	$(call if_changed,$(suffix_y))
 
+$(obj)/%.dtbo.z: $(obj)/%.dtbo 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 +382,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)
-- 
2.30.2

[PATCH v2 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 | 11 +++++------
 include/driver.h      |  6 ++++++
 2 files changed, 11 insertions(+), 6 deletions(-)

diff --git a/drivers/of/platform.c b/drivers/of/platform.c
index edb082b106..2e9abe32bc 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))
@@ -420,11 +422,8 @@ static struct device *of_device_create_on_demand(struct device_node *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..91e1f01974 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,7 +159,12 @@ 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
  */
 #define device_for_each_child(dev, child) \
-- 
2.30.2

[PATCH v2 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 | 20 +++++---------------
 1 file changed, 5 insertions(+), 15 deletions(-)

diff --git a/drivers/i2c/i2c.c b/drivers/i2c/i2c.c
index efcad29342..df89b8fb65 100644
--- a/drivers/i2c/i2c.c
+++ b/drivers/i2c/i2c.c
@@ -471,15 +471,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 +481,6 @@ static int i2c_hw_detect(struct device *dev)
 		of_i2c_register_devices(adap);
 		break;
 	}
-
-	return 0;
 }
 
 /**
@@ -720,7 +710,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 +721,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 v2 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 v2 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 v2 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..ee467d055b 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, "of_i2c: 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 v2 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 v2 2/7] driver: Add rescan hook to struct device

[Ahmad Fatoum]

On 09.03.23 12:52, Sascha Hauer wrote:
> 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>

Reviewed-by: Ahmad Fatoum <a.fatoum@pengutronix.de>

> ---
>  drivers/of/platform.c | 11 +++++------
>  include/driver.h      |  6 ++++++
>  2 files changed, 11 insertions(+), 6 deletions(-)
> 
> diff --git a/drivers/of/platform.c b/drivers/of/platform.c
> index edb082b106..2e9abe32bc 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))
> @@ -420,11 +422,8 @@ static struct device *of_device_create_on_demand(struct device_node *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..91e1f01974 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,7 +159,12 @@ 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
>   */
>  #define device_for_each_child(dev, child) \

-- 
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 v2 3/7] i2c: implement rescan

[Ahmad Fatoum]

On 09.03.23 12:52, Sascha Hauer wrote:
> 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 | 20 +++++---------------
>  1 file changed, 5 insertions(+), 15 deletions(-)
> 
> diff --git a/drivers/i2c/i2c.c b/drivers/i2c/i2c.c
> index efcad29342..df89b8fb65 100644
> --- a/drivers/i2c/i2c.c
> +++ b/drivers/i2c/i2c.c
> @@ -471,15 +471,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 +481,6 @@ static int i2c_hw_detect(struct device *dev)
>  		of_i2c_register_devices(adap);
>  		break;
>  	}
> -
> -	return 0;
>  }
>  
>  /**
> @@ -720,7 +710,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 +721,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);

I think this would fail for a non-OF I2C contoller on a CONFIG_OFTREE=y system,
because the function doesn't guard against !CONFIG_OFTREE. Just move it into
the if clause below.

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

-- 
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 v2 6/7] spi: implement rescan

[Ahmad Fatoum]

On 09.03.23 12:52, Sascha Hauer wrote:
> 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..ee467d055b 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, "of_i2c: skipping already registered %s\n",
> +				dev_name(n->dev));

s/of_i2c/of_spi/

> +			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);

-- 
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 v2 2/7] driver: Add rescan hook to struct device

[Ahmad Fatoum]

On 09.03.23 12:52, Sascha Hauer wrote:
> 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 | 11 +++++------
>  include/driver.h      |  6 ++++++
>  2 files changed, 11 insertions(+), 6 deletions(-)
> 
> diff --git a/drivers/of/platform.c b/drivers/of/platform.c
> index edb082b106..2e9abe32bc 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))
> @@ -420,11 +422,8 @@ static struct device *of_device_create_on_demand(struct device_node *np)
>  	if (!parent)
>  		return NULL;
>  
> -	if (!np->dev && parent->dev) {
> -		ret = device_detect(parent->dev);
> -		if (ret && ret != -ENOSYS)
> -			return ERR_PTR(ret);
> -	}

The int ret definition at the top of the function should be ok to be removed now.

> +	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..91e1f01974 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,7 +159,12 @@ 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
>   */
>  #define device_for_each_child(dev, child) \

-- 
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 v2 1/7] kbuild: Add target to build dtb overlay files

[Ahmad Fatoum]

On 09.03.23 12:52, Sascha Hauer wrote:
> 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 |  4 ++++
>  scripts/Makefile.lib   | 11 ++++++++++-
>  2 files changed, 14 insertions(+), 1 deletion(-)
> 
> diff --git a/scripts/Makefile.build b/scripts/Makefile.build
> index 216f03677b..33480a81b4 100644
> --- a/scripts/Makefile.build
> +++ b/scripts/Makefile.build
> @@ -258,12 +258,16 @@ 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)
> +# %.dtbo.pbl.o <- %.dtbo.S <- %.dtbo <- %.dtso (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, .dtbo.pbl.o, .dtbo.S .dtbo.z .dtbo) \

I see now reason to allow building an overlay for PBL. Should we drop this?

>  	   $(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..6af2fe07b2 100644
> --- a/scripts/Makefile.lib
> +++ b/scripts/Makefile.lib
> @@ -361,13 +361,19 @@ 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)
>  
> +$(obj)/%.dtbo.S: $(obj)/%.dtbo $(obj)/%.dtbo.z $(srctree)/scripts/gen-dtb-s FORCE
> +	$(call if_changed,dt_S_dtb)
> +
>  $(obj)/%.dtb.z: $(obj)/%.dtb FORCE
>  	$(call if_changed,$(suffix_y))
>  
> +$(obj)/%.dtbo.z: $(obj)/%.dtbo FORCE
> +	$(call if_changed,$(suffix_y))

Can we drop this and change scripts/gen-dtb-s to skip creating the compressed DTB
if we have a *.dtbo? While at, we should rename the symbol to __dtbo_. Otherwise,
we can have name clashes.

>  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 +382,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)

-- 
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 v2 2/7] driver: Add rescan hook to struct device

[Ahmad Fatoum]

On 09.03.23 13:54, Ahmad Fatoum wrote:
> On 09.03.23 12:52, Sascha Hauer wrote:
>> 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>
> 
> Reviewed-by: Ahmad Fatoum <a.fatoum@pengutronix.de>
> 
>> ---
>>  drivers/of/platform.c | 11 +++++------
>>  include/driver.h      |  6 ++++++
>>  2 files changed, 11 insertions(+), 6 deletions(-)
>>
>> diff --git a/drivers/of/platform.c b/drivers/of/platform.c
>> index edb082b106..2e9abe32bc 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))
>> @@ -420,11 +422,8 @@ static struct device *of_device_create_on_demand(struct device_node *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..91e1f01974 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,7 +159,12 @@ 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);
>>  
>> +}

Nitpick: add brace a liner higher, so the blank line is after it

>>  /* Iterate over a devices children
>>   */
>>  #define device_for_each_child(dev, child) \
> 

-- 
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 v2 1/7] kbuild: Add target to build dtb overlay files

[Sascha Hauer]

On Thu, Mar 09, 2023 at 02:24:36PM +0100, Ahmad Fatoum wrote:
> On 09.03.23 12:52, Sascha Hauer wrote:
> > 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 |  4 ++++
> >  scripts/Makefile.lib   | 11 ++++++++++-
> >  2 files changed, 14 insertions(+), 1 deletion(-)
> > 
> > diff --git a/scripts/Makefile.build b/scripts/Makefile.build
> > index 216f03677b..33480a81b4 100644
> > --- a/scripts/Makefile.build
> > +++ b/scripts/Makefile.build
> > @@ -258,12 +258,16 @@ 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)
> > +# %.dtbo.pbl.o <- %.dtbo.S <- %.dtbo <- %.dtso (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, .dtbo.pbl.o, .dtbo.S .dtbo.z .dtbo) \
> 
> I see now reason to allow building an overlay for PBL. Should we drop this?

I thought some board might want to apply an overlay in PBL, but the code
to apply an overlay is not there, not even the code to unflatten a dtb.
So right, this can be removed.

> 
> >  	   $(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..6af2fe07b2 100644
> > --- a/scripts/Makefile.lib
> > +++ b/scripts/Makefile.lib
> > @@ -361,13 +361,19 @@ 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)
> >  
> > +$(obj)/%.dtbo.S: $(obj)/%.dtbo $(obj)/%.dtbo.z $(srctree)/scripts/gen-dtb-s FORCE
> > +	$(call if_changed,dt_S_dtb)
> > +
> >  $(obj)/%.dtb.z: $(obj)/%.dtb FORCE
> >  	$(call if_changed,$(suffix_y))
> >  
> > +$(obj)/%.dtbo.z: $(obj)/%.dtbo FORCE
> > +	$(call if_changed,$(suffix_y))
> 
> Can we drop this and change scripts/gen-dtb-s to skip creating the compressed DTB
> if we have a *.dtbo?

What's wrong with building compressed overlays? That might come in
handy later.

> While at, we should rename the symbol to __dtbo_. Otherwise,
> we can have name clashes.

Right.

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 v2 1/7] kbuild: Add target to build dtb overlay files

[Ahmad Fatoum]

On 09.03.23 16:28, Sascha Hauer wrote:
> On Thu, Mar 09, 2023 at 02:24:36PM +0100, Ahmad Fatoum wrote:
>> On 09.03.23 12:52, Sascha Hauer wrote:
>>> 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 |  4 ++++
>>>  scripts/Makefile.lib   | 11 ++++++++++-
>>>  2 files changed, 14 insertions(+), 1 deletion(-)
>>>
>>> diff --git a/scripts/Makefile.build b/scripts/Makefile.build
>>> index 216f03677b..33480a81b4 100644
>>> --- a/scripts/Makefile.build
>>> +++ b/scripts/Makefile.build
>>> @@ -258,12 +258,16 @@ 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)
>>> +# %.dtbo.pbl.o <- %.dtbo.S <- %.dtbo <- %.dtso (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, .dtbo.pbl.o, .dtbo.S .dtbo.z .dtbo) \
>>
>> I see now reason to allow building an overlay for PBL. Should we drop this?
> 
> I thought some board might want to apply an overlay in PBL, but the code
> to apply an overlay is not there, not even the code to unflatten a dtb.
> So right, this can be removed.
> 
>>
>>>  	   $(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..6af2fe07b2 100644
>>> --- a/scripts/Makefile.lib
>>> +++ b/scripts/Makefile.lib
>>> @@ -361,13 +361,19 @@ 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)
>>>  
>>> +$(obj)/%.dtbo.S: $(obj)/%.dtbo $(obj)/%.dtbo.z $(srctree)/scripts/gen-dtb-s FORCE
>>> +	$(call if_changed,dt_S_dtb)
>>> +
>>>  $(obj)/%.dtb.z: $(obj)/%.dtb FORCE
>>>  	$(call if_changed,$(suffix_y))
>>>  
>>> +$(obj)/%.dtbo.z: $(obj)/%.dtbo FORCE
>>> +	$(call if_changed,$(suffix_y))
>>
>> Can we drop this and change scripts/gen-dtb-s to skip creating the compressed DTB
>> if we have a *.dtbo?
> 
> What's wrong with building compressed overlays? That might come in
> handy later.
Either you link it into barebox proper and then you should rather compress
barebox proper itself.

Or you pick out the *.dtbo.z from the barebox build directory and in that
case you will need decompress by hand anyway, because compressed DT
is currently only handled for barebox live DT.

Cheers,
Ahmad 

> 
>> While at, we should rename the symbol to __dtbo_. Otherwise,
>> we can have name clashes.
> 
> Right.
> 
> 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 v2 1/7] kbuild: Add target to build dtb overlay files

[Sascha Hauer]

On Thu, Mar 09, 2023 at 04:33:22PM +0100, Ahmad Fatoum wrote:
> On 09.03.23 16:28, Sascha Hauer wrote:
> > On Thu, Mar 09, 2023 at 02:24:36PM +0100, Ahmad Fatoum wrote:
> >> On 09.03.23 12:52, Sascha Hauer wrote:
> >>> 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 |  4 ++++
> >>>  scripts/Makefile.lib   | 11 ++++++++++-
> >>>  2 files changed, 14 insertions(+), 1 deletion(-)
> >>>
> >>> diff --git a/scripts/Makefile.build b/scripts/Makefile.build
> >>> index 216f03677b..33480a81b4 100644
> >>> --- a/scripts/Makefile.build
> >>> +++ b/scripts/Makefile.build
> >>> @@ -258,12 +258,16 @@ 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)
> >>> +# %.dtbo.pbl.o <- %.dtbo.S <- %.dtbo <- %.dtso (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, .dtbo.pbl.o, .dtbo.S .dtbo.z .dtbo) \
> >>
> >> I see now reason to allow building an overlay for PBL. Should we drop this?
> > 
> > I thought some board might want to apply an overlay in PBL, but the code
> > to apply an overlay is not there, not even the code to unflatten a dtb.
> > So right, this can be removed.
> > 
> >>
> >>>  	   $(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..6af2fe07b2 100644
> >>> --- a/scripts/Makefile.lib
> >>> +++ b/scripts/Makefile.lib
> >>> @@ -361,13 +361,19 @@ 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)
> >>>  
> >>> +$(obj)/%.dtbo.S: $(obj)/%.dtbo $(obj)/%.dtbo.z $(srctree)/scripts/gen-dtb-s FORCE
> >>> +	$(call if_changed,dt_S_dtb)
> >>> +
> >>>  $(obj)/%.dtb.z: $(obj)/%.dtb FORCE
> >>>  	$(call if_changed,$(suffix_y))
> >>>  
> >>> +$(obj)/%.dtbo.z: $(obj)/%.dtbo FORCE
> >>> +	$(call if_changed,$(suffix_y))
> >>
> >> Can we drop this and change scripts/gen-dtb-s to skip creating the compressed DTB
> >> if we have a *.dtbo?
> > 
> > What's wrong with building compressed overlays? That might come in
> > handy later.
> Either you link it into barebox proper and then you should rather compress
> barebox proper itself.
> 
> Or you pick out the *.dtbo.z from the barebox build directory and in that
> case you will need decompress by hand anyway, because compressed DT
> is currently only handled for barebox live DT.

Ok, right.

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 v2 2/7] driver: Add rescan hook to struct device

[Sascha Hauer]

On Thu, Mar 09, 2023 at 02:08:50PM +0100, Ahmad Fatoum wrote:
> On 09.03.23 12:52, Sascha Hauer wrote:
> > 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 | 11 +++++------
> >  include/driver.h      |  6 ++++++
> >  2 files changed, 11 insertions(+), 6 deletions(-)
> > 
> > diff --git a/drivers/of/platform.c b/drivers/of/platform.c
> > index edb082b106..2e9abe32bc 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))
> > @@ -420,11 +422,8 @@ static struct device *of_device_create_on_demand(struct device_node *np)
> >  	if (!parent)
> >  		return NULL;
> >  
> > -	if (!np->dev && parent->dev) {
> > -		ret = device_detect(parent->dev);
> > -		if (ret && ret != -ENOSYS)
> > -			return ERR_PTR(ret);
> > -	}
> 
> The int ret definition at the top of the function should be ok to be removed now.

No, it's still used.

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 v2 3/7] i2c: implement rescan

[Sascha Hauer]

On Thu, Mar 09, 2023 at 02:02:09PM +0100, Ahmad Fatoum wrote:
> On 09.03.23 12:52, Sascha Hauer wrote:
> > 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 | 20 +++++---------------
> >  1 file changed, 5 insertions(+), 15 deletions(-)
> > 
> > diff --git a/drivers/i2c/i2c.c b/drivers/i2c/i2c.c
> > index efcad29342..df89b8fb65 100644
> > --- a/drivers/i2c/i2c.c
> > +++ b/drivers/i2c/i2c.c
> > @@ -471,15 +471,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 +481,6 @@ static int i2c_hw_detect(struct device *dev)
> >  		of_i2c_register_devices(adap);
> >  		break;
> >  	}
> > -
> > -	return 0;
> >  }
> >  
> >  /**
> > @@ -720,7 +710,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 +721,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);
> 
> I think this would fail for a non-OF I2C contoller on a CONFIG_OFTREE=y system,
> because the function doesn't guard against !CONFIG_OFTREE. Just move it into
> the if clause below.

This is not equivalent as of_i2c_register_devices() uses
adap->dev.of_node whereas the if clause below uses adapter->dev.parent->of_node.

Ideally this should be the same, but for now I think I'll just
revert to what has been done before the i2c_hw_detect patches
which adding a check in of_i2c_register_devices().

Sascha

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

-- 
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 v2 2/7] driver: Add rescan hook to struct device

[Sascha Hauer]

On Thu, Mar 09, 2023 at 04:41:05PM +0100, Sascha Hauer wrote:
> On Thu, Mar 09, 2023 at 02:08:50PM +0100, Ahmad Fatoum wrote:
> > On 09.03.23 12:52, Sascha Hauer wrote:
> > > 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 | 11 +++++------
> > >  include/driver.h      |  6 ++++++
> > >  2 files changed, 11 insertions(+), 6 deletions(-)
> > > 
> > > diff --git a/drivers/of/platform.c b/drivers/of/platform.c
> > > index edb082b106..2e9abe32bc 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))
> > > @@ -420,11 +422,8 @@ static struct device *of_device_create_on_demand(struct device_node *np)
> > >  	if (!parent)
> > >  		return NULL;
> > >  
> > > -	if (!np->dev && parent->dev) {
> > > -		ret = device_detect(parent->dev);
> > > -		if (ret && ret != -ENOSYS)
> > > -			return ERR_PTR(ret);
> > > -	}
> > 
> > The int ret definition at the top of the function should be ok to be removed now.
> 
> No, it's still used.

Hm, looked at the wrong function it seems. Compiler says it's unused.

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 |