mail archive of the barebox mailing list
 help / color / mirror / Atom feed
* [PATCH v4 0/6] boards: qemu-virt: support passing in FIT public key
@ 2023-06-12 12:50 Ahmad Fatoum
  2023-06-12 12:50 ` [PATCH v4 1/6] boards: qemu-virt: apply overlay at postcore_initcall level Ahmad Fatoum
                   ` (5 more replies)
  0 siblings, 6 replies; 10+ messages in thread
From: Ahmad Fatoum @ 2023-06-12 12:50 UTC (permalink / raw)
  To: barebox

FIT public key is usually passed in via board DT. Usual way to use
barebox with QEMU Virt however is to use DT supplied by Qemu and apply
overlay to it. mkimage doesn't generate overlay DTB though. To make
barbebox Qemu Virt behave like other boards, let's define a dummy DT
that includes CONFIG_BOOTM_FITIMAGE_PUBKEY, which is merged with the
barebox live device tree.

v3 -> v4:
  - early exit initcall if compatible doesn't match
  - evaluate DTC_CPP_FLAGS for device tree overlays
  - rename overlay to reference board name
  - Rename older QEMU's /soc/flash@X to /flash@X

v2 -> v3:
  - drop "support of_ensure_probed for top-level machine device"
  - switch from board driver back to initcall

v1 -> v2:
  - support of_ensure_probed for top-level machine device
  - ensure qemu board driver is probed at postcore

Ahmad Fatoum (6):
  boards: qemu-virt: apply overlay at postcore_initcall level
  kbuild: support DTC_CPP_FLAGS_file.dtbo
  boards: qemu-virt: compile overlay as such
  boards: qemu-virt: support passing in FIT public key
  of: implement of_move_node helper
  boards: qemu-virt: support older QEMU with /soc/flash

 common/boards/qemu-virt/Makefile              |  6 +-
 common/boards/qemu-virt/board.c               | 62 ++++++++++++-------
 common/boards/qemu-virt/fitimage-pubkey.dts   |  7 +++
 ...rlay-of-flash.dts => qemu-virt-flash.dtso} |  0
 drivers/of/base.c                             | 18 ++++++
 include/of.h                                  |  1 +
 scripts/Makefile.lib                          |  1 +
 7 files changed, 69 insertions(+), 26 deletions(-)
 create mode 100644 common/boards/qemu-virt/fitimage-pubkey.dts
 rename common/boards/qemu-virt/{overlay-of-flash.dts => qemu-virt-flash.dtso} (100%)

-- 
2.39.2




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

* [PATCH v4 1/6] boards: qemu-virt: apply overlay at postcore_initcall level
  2023-06-12 12:50 [PATCH v4 0/6] boards: qemu-virt: support passing in FIT public key Ahmad Fatoum
@ 2023-06-12 12:50 ` Ahmad Fatoum
  2023-06-12 12:51 ` [PATCH v4 2/6] kbuild: support DTC_CPP_FLAGS_file.dtbo Ahmad Fatoum
                   ` (4 subsequent siblings)
  5 siblings, 0 replies; 10+ messages in thread
From: Ahmad Fatoum @ 2023-06-12 12:50 UTC (permalink / raw)
  To: barebox; +Cc: Ahmad Fatoum

Qemu board driver fixups should be applied very early to be able to
influence core components, even if they are controlled directly by
initcalls. For this reason, let's move the early board code into
the earliest possible initcall level.

Signed-off-by: Ahmad Fatoum <a.fatoum@pengutronix.de>
---
 common/boards/qemu-virt/board.c | 51 +++++++++++++++++++--------------
 1 file changed, 29 insertions(+), 22 deletions(-)

diff --git a/common/boards/qemu-virt/board.c b/common/boards/qemu-virt/board.c
index d0f4412cdea5..19d995aa508e 100644
--- a/common/boards/qemu-virt/board.c
+++ b/common/boards/qemu-virt/board.c
@@ -36,22 +36,6 @@ static inline void arm_virt_init(void) {}
 
 extern char __dtb_overlay_of_flash_start[];
 
-static int virt_probe(struct device *dev)
-{
-	struct device_node *overlay;
-	void (*init)(void);
-
-	init = device_get_match_data(dev);
-	if (init)
-		init();
-
-	overlay = of_unflatten_dtb(__dtb_overlay_of_flash_start, INT_MAX);
-	of_overlay_apply_tree(dev->of_node, overlay);
-	/* of_probe() will happen later at of_populate_initcall */
-
-	return 0;
-}
-
 static const struct of_device_id virt_of_match[] = {
 	{ .compatible = "linux,dummy-virt", .data = arm_virt_init },
 	{ .compatible = "riscv-virtio" },
@@ -59,10 +43,33 @@ static const struct of_device_id virt_of_match[] = {
 };
 BAREBOX_DEEP_PROBE_ENABLE(virt_of_match);
 
-static struct driver virt_board_driver = {
-	.name = "board-qemu-virt",
-	.probe = virt_probe,
-	.of_compatible = virt_of_match,
-};
+/*
+ * We don't have a dedicated qemu-virt device tree and instead rely
+ * on what Qemu passes us. To be able to get fundamental changes
+ * in very early, we forego having a board driver here and do this
+ * directly in the initcall.
+ */
+static int virt_board_driver_init(void)
+{
+	struct device_node *root = of_get_root_node();
+	struct device_node *overlay;
+	const struct of_device_id *id;
+	void (*init)(void);
 
-postcore_platform_driver(virt_board_driver);
+	id = of_match_node(virt_of_match, root);
+	if (!id)
+		return 0;
+
+	if (id->data) {
+		init = id->data;
+		init();
+	}
+
+	overlay = of_unflatten_dtb(__dtb_overlay_of_flash_start, INT_MAX);
+	of_overlay_apply_tree(root, overlay);
+
+	/* of_probe() will happen later at of_populate_initcall */
+
+	return 0;
+}
+postcore_initcall(virt_board_driver_init);
-- 
2.39.2




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

* [PATCH v4 2/6] kbuild: support DTC_CPP_FLAGS_file.dtbo
  2023-06-12 12:50 [PATCH v4 0/6] boards: qemu-virt: support passing in FIT public key Ahmad Fatoum
  2023-06-12 12:50 ` [PATCH v4 1/6] boards: qemu-virt: apply overlay at postcore_initcall level Ahmad Fatoum
@ 2023-06-12 12:51 ` Ahmad Fatoum
  2023-06-12 12:51 ` [PATCH v4 3/6] boards: qemu-virt: compile overlay as such Ahmad Fatoum
                   ` (3 subsequent siblings)
  5 siblings, 0 replies; 10+ messages in thread
From: Ahmad Fatoum @ 2023-06-12 12:51 UTC (permalink / raw)
  To: barebox; +Cc: Ahmad Fatoum

With newly added commit 3610ec11f72b ("kbuild: Add target to build dtb
overlay files"), overlays can now be marked as such in the build system
by using the dtbo extension in the Makefile instead of dtb.

Injecting extra flags with DTC_CPP_FLAGS_ was only supported for *.dtb
though, so add support now for *.dtbo for symmetry.

Signed-off-by: Ahmad Fatoum <a.fatoum@pengutronix.de>
---
 scripts/Makefile.lib | 1 +
 1 file changed, 1 insertion(+)

diff --git a/scripts/Makefile.lib b/scripts/Makefile.lib
index 90cfa579e5d5..42ee27499561 100644
--- a/scripts/Makefile.lib
+++ b/scripts/Makefile.lib
@@ -211,6 +211,7 @@ dtc_cpp_flags  = -Wp,-MD,$(depfile).pre -nostdinc                        \
 		 -I$(srctree)/dts/include                                \
 		 -I$(srctree)/dts/src/                                   \
 		 $(DTC_CPP_FLAGS_$(basetarget).dtb)                      \
+		 $(DTC_CPP_FLAGS_$(basetarget).dtbo)                     \
 		 -undef -D__DTS__
 
 ifdef CONFIG_BOOTM_FITIMAGE_PUBKEY
-- 
2.39.2




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

* [PATCH v4 3/6] boards: qemu-virt: compile overlay as such
  2023-06-12 12:50 [PATCH v4 0/6] boards: qemu-virt: support passing in FIT public key Ahmad Fatoum
  2023-06-12 12:50 ` [PATCH v4 1/6] boards: qemu-virt: apply overlay at postcore_initcall level Ahmad Fatoum
  2023-06-12 12:51 ` [PATCH v4 2/6] kbuild: support DTC_CPP_FLAGS_file.dtbo Ahmad Fatoum
@ 2023-06-12 12:51 ` Ahmad Fatoum
  2023-06-12 12:51 ` [PATCH v4 4/6] boards: qemu-virt: support passing in FIT public key Ahmad Fatoum
                   ` (2 subsequent siblings)
  5 siblings, 0 replies; 10+ messages in thread
From: Ahmad Fatoum @ 2023-06-12 12:51 UTC (permalink / raw)
  To: barebox; +Cc: Ahmad Fatoum

Build system now differentiates between building normal device trees and
overlays; as the latter would be broken when CONFIG_EXTERNAL_DTS_FRAGMENTS
is in use. Switch over Qemu board support to build the overlay as such.

Signed-off-by: Ahmad Fatoum <a.fatoum@pengutronix.de>
---
 common/boards/qemu-virt/Makefile                            | 6 +++---
 common/boards/qemu-virt/board.c                             | 4 ++--
 .../{overlay-of-flash.dts => qemu-virt-flash.dtso}          | 0
 3 files changed, 5 insertions(+), 5 deletions(-)
 rename common/boards/qemu-virt/{overlay-of-flash.dts => qemu-virt-flash.dtso} (100%)

diff --git a/common/boards/qemu-virt/Makefile b/common/boards/qemu-virt/Makefile
index c16727751550..d280e5ebdd2b 100644
--- a/common/boards/qemu-virt/Makefile
+++ b/common/boards/qemu-virt/Makefile
@@ -1,12 +1,12 @@
 # SPDX-License-Identifier: GPL-2.0-only
 
 obj-y += board.o
-obj-y += overlay-of-flash.dtb.o
+obj-y += qemu-virt-flash.dtbo.o
 ifeq ($(CONFIG_RISCV),y)
-DTC_CPP_FLAGS_overlay-of-flash.dtb := -DRISCV_VIRT=1
+DTC_CPP_FLAGS_qemu-virt-flash.dtbo := -DRISCV_VIRT=1
 endif
 ifeq ($(CONFIG_ARM),y)
-DTC_CPP_FLAGS_overlay-of-flash.dtb := -DARM_VIRT=1
+DTC_CPP_FLAGS_qemu-virt-flash.dtbo := -DARM_VIRT=1
 endif
 
 clean-files := *.dtb *.dtb.S .*.dtc .*.pre .*.dts *.dtb.z
diff --git a/common/boards/qemu-virt/board.c b/common/boards/qemu-virt/board.c
index 19d995aa508e..ea29783d6056 100644
--- a/common/boards/qemu-virt/board.c
+++ b/common/boards/qemu-virt/board.c
@@ -34,7 +34,7 @@ static inline void arm_virt_init(void)
 static inline void arm_virt_init(void) {}
 #endif
 
-extern char __dtb_overlay_of_flash_start[];
+extern char __dtbo_qemu_virt_flash_start[];
 
 static const struct of_device_id virt_of_match[] = {
 	{ .compatible = "linux,dummy-virt", .data = arm_virt_init },
@@ -65,7 +65,7 @@ static int virt_board_driver_init(void)
 		init();
 	}
 
-	overlay = of_unflatten_dtb(__dtb_overlay_of_flash_start, INT_MAX);
+	overlay = of_unflatten_dtb(__dtbo_qemu_virt_flash_start, INT_MAX);
 	of_overlay_apply_tree(root, overlay);
 
 	/* of_probe() will happen later at of_populate_initcall */
diff --git a/common/boards/qemu-virt/overlay-of-flash.dts b/common/boards/qemu-virt/qemu-virt-flash.dtso
similarity index 100%
rename from common/boards/qemu-virt/overlay-of-flash.dts
rename to common/boards/qemu-virt/qemu-virt-flash.dtso
-- 
2.39.2




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

* [PATCH v4 4/6] boards: qemu-virt: support passing in FIT public key
  2023-06-12 12:50 [PATCH v4 0/6] boards: qemu-virt: support passing in FIT public key Ahmad Fatoum
                   ` (2 preceding siblings ...)
  2023-06-12 12:51 ` [PATCH v4 3/6] boards: qemu-virt: compile overlay as such Ahmad Fatoum
@ 2023-06-12 12:51 ` Ahmad Fatoum
  2023-06-12 12:51 ` [PATCH v4 5/6] of: implement of_move_node helper Ahmad Fatoum
  2023-06-12 12:51 ` [PATCH v4 6/6] boards: qemu-virt: support older QEMU with /soc/flash Ahmad Fatoum
  5 siblings, 0 replies; 10+ messages in thread
From: Ahmad Fatoum @ 2023-06-12 12:51 UTC (permalink / raw)
  To: barebox; +Cc: Ahmad Fatoum

FIT public key is usually passed in via board DT. Usual way to use
barebox with QEMU Virt however is to use DT supplied by Qemu and apply
overlay to it. mkimage doesn't generate overlay DTB though. To make
barbebox Qemu Virt behave like other boards, let's define a dummy DT
that includes CONFIG_BOOTM_FITIMAGE_PUBKEY, which is merged with the
barebox live device tree.

Suggested-by: Jan Lübbe <jlu@pengutronix.de>
Signed-off-by: Ahmad Fatoum <a.fatoum@pengutronix.de>
---
 common/boards/qemu-virt/Makefile            | 2 +-
 common/boards/qemu-virt/board.c             | 6 +++++-
 common/boards/qemu-virt/fitimage-pubkey.dts | 7 +++++++
 3 files changed, 13 insertions(+), 2 deletions(-)
 create mode 100644 common/boards/qemu-virt/fitimage-pubkey.dts

diff --git a/common/boards/qemu-virt/Makefile b/common/boards/qemu-virt/Makefile
index d280e5ebdd2b..b6883ff7672e 100644
--- a/common/boards/qemu-virt/Makefile
+++ b/common/boards/qemu-virt/Makefile
@@ -1,7 +1,7 @@
 # SPDX-License-Identifier: GPL-2.0-only
 
 obj-y += board.o
-obj-y += qemu-virt-flash.dtbo.o
+obj-y += qemu-virt-flash.dtbo.o fitimage-pubkey.dtb.o
 ifeq ($(CONFIG_RISCV),y)
 DTC_CPP_FLAGS_qemu-virt-flash.dtbo := -DRISCV_VIRT=1
 endif
diff --git a/common/boards/qemu-virt/board.c b/common/boards/qemu-virt/board.c
index ea29783d6056..4c6df5e30252 100644
--- a/common/boards/qemu-virt/board.c
+++ b/common/boards/qemu-virt/board.c
@@ -35,6 +35,7 @@ static inline void arm_virt_init(void) {}
 #endif
 
 extern char __dtbo_qemu_virt_flash_start[];
+extern char __dtb_fitimage_pubkey_start[];
 
 static const struct of_device_id virt_of_match[] = {
 	{ .compatible = "linux,dummy-virt", .data = arm_virt_init },
@@ -52,7 +53,7 @@ BAREBOX_DEEP_PROBE_ENABLE(virt_of_match);
 static int virt_board_driver_init(void)
 {
 	struct device_node *root = of_get_root_node();
-	struct device_node *overlay;
+	struct device_node *overlay, *pubkey;
 	const struct of_device_id *id;
 	void (*init)(void);
 
@@ -68,6 +69,9 @@ static int virt_board_driver_init(void)
 	overlay = of_unflatten_dtb(__dtbo_qemu_virt_flash_start, INT_MAX);
 	of_overlay_apply_tree(root, overlay);
 
+	pubkey = of_unflatten_dtb(__dtb_fitimage_pubkey_start, INT_MAX);
+	of_merge_nodes(root, pubkey);
+
 	/* of_probe() will happen later at of_populate_initcall */
 
 	return 0;
diff --git a/common/boards/qemu-virt/fitimage-pubkey.dts b/common/boards/qemu-virt/fitimage-pubkey.dts
new file mode 100644
index 000000000000..497799fa4b60
--- /dev/null
+++ b/common/boards/qemu-virt/fitimage-pubkey.dts
@@ -0,0 +1,7 @@
+/dts-v1/;
+
+#ifdef CONFIG_BOOTM_FITIMAGE_PUBKEY
+#include CONFIG_BOOTM_FITIMAGE_PUBKEY
+#endif
+
+/{ };
-- 
2.39.2




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

* [PATCH v4 5/6] of: implement of_move_node helper
  2023-06-12 12:50 [PATCH v4 0/6] boards: qemu-virt: support passing in FIT public key Ahmad Fatoum
                   ` (3 preceding siblings ...)
  2023-06-12 12:51 ` [PATCH v4 4/6] boards: qemu-virt: support passing in FIT public key Ahmad Fatoum
@ 2023-06-12 12:51 ` Ahmad Fatoum
  2023-06-13  8:27   ` Sascha Hauer
  2023-06-12 12:51 ` [PATCH v4 6/6] boards: qemu-virt: support older QEMU with /soc/flash Ahmad Fatoum
  5 siblings, 1 reply; 10+ messages in thread
From: Ahmad Fatoum @ 2023-06-12 12:51 UTC (permalink / raw)
  To: barebox; +Cc: Ahmad Fatoum

Reparenting nodes can be a useful thing to do in fixups. Add a helper
for that.

Signed-off-by: Ahmad Fatoum <a.fatoum@pengutronix.de>
---
 drivers/of/base.c | 18 ++++++++++++++++++
 include/of.h      |  1 +
 2 files changed, 19 insertions(+)

diff --git a/drivers/of/base.c b/drivers/of/base.c
index e3416b5b75a7..43a4a923d9c5 100644
--- a/drivers/of/base.c
+++ b/drivers/of/base.c
@@ -2695,6 +2695,24 @@ void of_delete_node(struct device_node *node)
 	free(node);
 }
 
+void of_move_node(struct device_node *parent, struct device_node *node)
+{
+	if (!node)
+		return;
+
+	list_del(&node->list);
+	list_del(&node->parent_list);
+
+	free(node->full_name);
+	node->full_name = basprintf("%s/%s", parent->full_name, node->name);
+
+	if (!node->parent)
+		return;
+
+	list_add(&node->list, &parent->list);
+	list_add_tail(&node->parent_list, &parent->children);
+}
+
 /*
  * of_find_node_by_chosen - Find a node given a chosen property pointing at it
  * @propname:   the name of the property containing a path or alias
diff --git a/include/of.h b/include/of.h
index 0037bad6c4d7..686c1632c267 100644
--- a/include/of.h
+++ b/include/of.h
@@ -187,6 +187,7 @@ extern struct device_node *of_create_node(struct device_node *root,
 extern void of_merge_nodes(struct device_node *np, const struct device_node *other);
 extern struct device_node *of_copy_node(struct device_node *parent,
 				const struct device_node *other);
+void of_move_node(struct device_node *parent, struct device_node *node);
 extern struct device_node *of_dup(const struct device_node *root);
 extern void of_delete_node(struct device_node *node);
 
-- 
2.39.2




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

* [PATCH v4 6/6] boards: qemu-virt: support older QEMU with /soc/flash
  2023-06-12 12:50 [PATCH v4 0/6] boards: qemu-virt: support passing in FIT public key Ahmad Fatoum
                   ` (4 preceding siblings ...)
  2023-06-12 12:51 ` [PATCH v4 5/6] of: implement of_move_node helper Ahmad Fatoum
@ 2023-06-12 12:51 ` Ahmad Fatoum
  5 siblings, 0 replies; 10+ messages in thread
From: Ahmad Fatoum @ 2023-06-12 12:51 UTC (permalink / raw)
  To: barebox; +Cc: Ahmad Fatoum

Qemu versions differ in whether the flash is top-level or under the /soc
node. As we apply a state fixup to the Qemu-passed DT, we need to know
where the flash at. Easiest way is just to move it for older Qemus to
the new location.

Signed-off-by: Ahmad Fatoum <a.fatoum@pengutronix.de>
---
 common/boards/qemu-virt/board.c | 7 ++++++-
 1 file changed, 6 insertions(+), 1 deletion(-)

diff --git a/common/boards/qemu-virt/board.c b/common/boards/qemu-virt/board.c
index 4c6df5e30252..06705dce394a 100644
--- a/common/boards/qemu-virt/board.c
+++ b/common/boards/qemu-virt/board.c
@@ -52,7 +52,7 @@ BAREBOX_DEEP_PROBE_ENABLE(virt_of_match);
  */
 static int virt_board_driver_init(void)
 {
-	struct device_node *root = of_get_root_node();
+	struct device_node *soc, *root = of_get_root_node();
 	struct device_node *overlay, *pubkey;
 	const struct of_device_id *id;
 	void (*init)(void);
@@ -66,6 +66,11 @@ static int virt_board_driver_init(void)
 		init();
 	}
 
+	/* Rename older QEMU's /soc/flash@X to /flash@X */
+	soc = of_get_child_by_name(root, "soc");
+	if (soc)
+		of_move_node(root, of_find_node_by_name(soc, "flash"));
+
 	overlay = of_unflatten_dtb(__dtbo_qemu_virt_flash_start, INT_MAX);
 	of_overlay_apply_tree(root, overlay);
 
-- 
2.39.2




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

* Re: [PATCH v4 5/6] of: implement of_move_node helper
  2023-06-12 12:51 ` [PATCH v4 5/6] of: implement of_move_node helper Ahmad Fatoum
@ 2023-06-13  8:27   ` Sascha Hauer
  2023-06-13  8:30     ` Ahmad Fatoum
  0 siblings, 1 reply; 10+ messages in thread
From: Sascha Hauer @ 2023-06-13  8:27 UTC (permalink / raw)
  To: Ahmad Fatoum; +Cc: barebox

On Mon, Jun 12, 2023 at 02:51:03PM +0200, Ahmad Fatoum wrote:
> Reparenting nodes can be a useful thing to do in fixups. Add a helper
> for that.
> 
> Signed-off-by: Ahmad Fatoum <a.fatoum@pengutronix.de>
> ---
>  drivers/of/base.c | 18 ++++++++++++++++++
>  include/of.h      |  1 +
>  2 files changed, 19 insertions(+)
> 
> diff --git a/drivers/of/base.c b/drivers/of/base.c
> index e3416b5b75a7..43a4a923d9c5 100644
> --- a/drivers/of/base.c
> +++ b/drivers/of/base.c
> @@ -2695,6 +2695,24 @@ void of_delete_node(struct device_node *node)
>  	free(node);
>  }
>  
> +void of_move_node(struct device_node *parent, struct device_node *node)
> +{
> +	if (!node)
> +		return;
> +
> +	list_del(&node->list);
> +	list_del(&node->parent_list);
> +
> +	free(node->full_name);
> +	node->full_name = basprintf("%s/%s", parent->full_name, node->name);
> +
> +	if (!node->parent)
> +		return;

Why is the old parent relevant here? Even when node didn't have a parent
before you should still add it to the children list of the new parent.

Also node->parent should be set to the new parent.

You seem to honour the case that node didn't have a parent previously.
Does the list_del(&node->parent_list) work in this case? I can't see any
INIT_LIST_HEAD(&node->parent_list) in the code, so I assume running a
list_del on it results in a NULL pointer dereference.

Sascha

> +
> +	list_add(&node->list, &parent->list);
> +	list_add_tail(&node->parent_list, &parent->children);
> +}
> +
>  /*
>   * of_find_node_by_chosen - Find a node given a chosen property pointing at it
>   * @propname:   the name of the property containing a path or alias
> diff --git a/include/of.h b/include/of.h
> index 0037bad6c4d7..686c1632c267 100644
> --- a/include/of.h
> +++ b/include/of.h
> @@ -187,6 +187,7 @@ extern struct device_node *of_create_node(struct device_node *root,
>  extern void of_merge_nodes(struct device_node *np, const struct device_node *other);
>  extern struct device_node *of_copy_node(struct device_node *parent,
>  				const struct device_node *other);
> +void of_move_node(struct device_node *parent, struct device_node *node);
>  extern struct device_node *of_dup(const struct device_node *root);
>  extern void of_delete_node(struct device_node *node);
>  
> -- 
> 2.39.2
> 
> 
> 

-- 
Pengutronix e.K.                           |                             |
Steuerwalder Str. 21                       | http://www.pengutronix.de/  |
31137 Hildesheim, Germany                  | Phone: +49-5121-206917-0    |
Amtsgericht Hildesheim, HRA 2686           | Fax:   +49-5121-206917-5555 |



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

* Re: [PATCH v4 5/6] of: implement of_move_node helper
  2023-06-13  8:27   ` Sascha Hauer
@ 2023-06-13  8:30     ` Ahmad Fatoum
  2023-06-13  8:32       ` Sascha Hauer
  0 siblings, 1 reply; 10+ messages in thread
From: Ahmad Fatoum @ 2023-06-13  8:30 UTC (permalink / raw)
  To: Sascha Hauer; +Cc: barebox

On 13.06.23 10:27, Sascha Hauer wrote:
> On Mon, Jun 12, 2023 at 02:51:03PM +0200, Ahmad Fatoum wrote:
>> Reparenting nodes can be a useful thing to do in fixups. Add a helper
>> for that.
>>
>> Signed-off-by: Ahmad Fatoum <a.fatoum@pengutronix.de>
>> ---
>>  drivers/of/base.c | 18 ++++++++++++++++++
>>  include/of.h      |  1 +
>>  2 files changed, 19 insertions(+)
>>
>> diff --git a/drivers/of/base.c b/drivers/of/base.c
>> index e3416b5b75a7..43a4a923d9c5 100644
>> --- a/drivers/of/base.c
>> +++ b/drivers/of/base.c
>> @@ -2695,6 +2695,24 @@ void of_delete_node(struct device_node *node)
>>  	free(node);
>>  }
>>  
>> +void of_move_node(struct device_node *parent, struct device_node *node)
>> +{
>> +	if (!node)
>> +		return;
>> +
>> +	list_del(&node->list);
>> +	list_del(&node->parent_list);
>> +
>> +	free(node->full_name);
>> +	node->full_name = basprintf("%s/%s", parent->full_name, node->name);
>> +
>> +	if (!node->parent)
>> +		return;
> 
> Why is the old parent relevant here? Even when node didn't have a parent
> before you should still add it to the children list of the new parent.
> 
> Also node->parent should be set to the new parent.
> 
> You seem to honour the case that node didn't have a parent previously.
> Does the list_del(&node->parent_list) work in this case? I can't see any
> INIT_LIST_HEAD(&node->parent_list) in the code, so I assume running a
> list_del on it results in a NULL pointer dereference.

I will revisit this. Can you pick patches 1-4 though, so we can finally
get this in?

> 
> Sascha
> 
>> +
>> +	list_add(&node->list, &parent->list);
>> +	list_add_tail(&node->parent_list, &parent->children);
>> +}
>> +
>>  /*
>>   * of_find_node_by_chosen - Find a node given a chosen property pointing at it
>>   * @propname:   the name of the property containing a path or alias
>> diff --git a/include/of.h b/include/of.h
>> index 0037bad6c4d7..686c1632c267 100644
>> --- a/include/of.h
>> +++ b/include/of.h
>> @@ -187,6 +187,7 @@ extern struct device_node *of_create_node(struct device_node *root,
>>  extern void of_merge_nodes(struct device_node *np, const struct device_node *other);
>>  extern struct device_node *of_copy_node(struct device_node *parent,
>>  				const struct device_node *other);
>> +void of_move_node(struct device_node *parent, struct device_node *node);
>>  extern struct device_node *of_dup(const struct device_node *root);
>>  extern void of_delete_node(struct device_node *node);
>>  
>> -- 
>> 2.39.2
>>
>>
>>
> 

-- 
Pengutronix e.K.                           |                             |
Steuerwalder Str. 21                       | http://www.pengutronix.de/  |
31137 Hildesheim, Germany                  | Phone: +49-5121-206917-0    |
Amtsgericht Hildesheim, HRA 2686           | Fax:   +49-5121-206917-5555 |




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

* Re: [PATCH v4 5/6] of: implement of_move_node helper
  2023-06-13  8:30     ` Ahmad Fatoum
@ 2023-06-13  8:32       ` Sascha Hauer
  0 siblings, 0 replies; 10+ messages in thread
From: Sascha Hauer @ 2023-06-13  8:32 UTC (permalink / raw)
  To: Ahmad Fatoum; +Cc: barebox

On Tue, Jun 13, 2023 at 10:30:10AM +0200, Ahmad Fatoum wrote:
> On 13.06.23 10:27, Sascha Hauer wrote:
> > On Mon, Jun 12, 2023 at 02:51:03PM +0200, Ahmad Fatoum wrote:
> >> Reparenting nodes can be a useful thing to do in fixups. Add a helper
> >> for that.
> >>
> >> Signed-off-by: Ahmad Fatoum <a.fatoum@pengutronix.de>
> >> ---
> >>  drivers/of/base.c | 18 ++++++++++++++++++
> >>  include/of.h      |  1 +
> >>  2 files changed, 19 insertions(+)
> >>
> >> diff --git a/drivers/of/base.c b/drivers/of/base.c
> >> index e3416b5b75a7..43a4a923d9c5 100644
> >> --- a/drivers/of/base.c
> >> +++ b/drivers/of/base.c
> >> @@ -2695,6 +2695,24 @@ void of_delete_node(struct device_node *node)
> >>  	free(node);
> >>  }
> >>  
> >> +void of_move_node(struct device_node *parent, struct device_node *node)
> >> +{
> >> +	if (!node)
> >> +		return;
> >> +
> >> +	list_del(&node->list);
> >> +	list_del(&node->parent_list);
> >> +
> >> +	free(node->full_name);
> >> +	node->full_name = basprintf("%s/%s", parent->full_name, node->name);
> >> +
> >> +	if (!node->parent)
> >> +		return;
> > 
> > Why is the old parent relevant here? Even when node didn't have a parent
> > before you should still add it to the children list of the new parent.
> > 
> > Also node->parent should be set to the new parent.
> > 
> > You seem to honour the case that node didn't have a parent previously.
> > Does the list_del(&node->parent_list) work in this case? I can't see any
> > INIT_LIST_HEAD(&node->parent_list) in the code, so I assume running a
> > list_del on it results in a NULL pointer dereference.
> 
> I will revisit this. Can you pick patches 1-4 though, so we can finally
> get this in?

Did that.

Sascha


-- 
Pengutronix e.K.                           |                             |
Steuerwalder Str. 21                       | http://www.pengutronix.de/  |
31137 Hildesheim, Germany                  | Phone: +49-5121-206917-0    |
Amtsgericht Hildesheim, HRA 2686           | Fax:   +49-5121-206917-5555 |



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

end of thread, other threads:[~2023-06-13  8:33 UTC | newest]

Thread overview: 10+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2023-06-12 12:50 [PATCH v4 0/6] boards: qemu-virt: support passing in FIT public key Ahmad Fatoum
2023-06-12 12:50 ` [PATCH v4 1/6] boards: qemu-virt: apply overlay at postcore_initcall level Ahmad Fatoum
2023-06-12 12:51 ` [PATCH v4 2/6] kbuild: support DTC_CPP_FLAGS_file.dtbo Ahmad Fatoum
2023-06-12 12:51 ` [PATCH v4 3/6] boards: qemu-virt: compile overlay as such Ahmad Fatoum
2023-06-12 12:51 ` [PATCH v4 4/6] boards: qemu-virt: support passing in FIT public key Ahmad Fatoum
2023-06-12 12:51 ` [PATCH v4 5/6] of: implement of_move_node helper Ahmad Fatoum
2023-06-13  8:27   ` Sascha Hauer
2023-06-13  8:30     ` Ahmad Fatoum
2023-06-13  8:32       ` Sascha Hauer
2023-06-12 12:51 ` [PATCH v4 6/6] boards: qemu-virt: support older QEMU with /soc/flash Ahmad Fatoum

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