Hi Ayush,
Cc Thomas and Lucas (interested in this topic).
On Sun, 12 Jan 2025 00:58:52 +0530
Ayush Singh <ayush@xxxxxxxxxxxxxxx> wrote:
The export symbols node adds some additional symbols that can be used
in the symbols resolution. The resolver tries to match unresolved
symbols first using the export symbols node and, if a match is not
found, it tries the normal route and searches the __symbols__ node.
Contrary to symbols available in the global __symbols__ node, symbols
listed in the export symbols node can be considered as local symbols.
Indeed, they can be changed depending on the node the overlay is going
to be applied to and are only visible from the current node properties.
export-symbols are phandle based as opposed to global __symbols__. This
allows for much simpler use with overlays as opposed to __symbols__
where paths require resizing of overlays as show in [0].
The phandle resolution in overlay is now broken into 2 steps:
- fragment target phandles
- All other phandles
The reason this is required is because to find the export-symbols node
in the target node, we first need to resolve the target phandles. Here
is an example of overlay that will generate the target __fixup__ after
the property fixup, and thus fail without this 2 step resolution.
```
/dts-v1/;
/plugin/;
&connectora {
prop = <&gpio_connector>;
};
&connectorb {
prop = <&gpio_connector>;
};
```
Using fdtdump, we get the following:
```
**** fdtdump is a low-level debugging tool, not meant for general use.
**** If you want to decompile a dtb, you probably want
**** dtc -I dtb -O dts <filename>
/dts-v1/;
// magic: 0xd00dfeed
// totalsize: 0x1b1 (433)
// off_dt_struct: 0x38
// off_dt_strings: 0x180
// off_mem_rsvmap: 0x28
// version: 17
// last_comp_version: 16
// boot_cpuid_phys: 0x0
// size_dt_strings: 0x31
// size_dt_struct: 0x148
/ {
fragment@0 {
target = <0xffffffff>;
__overlay__ {
prop = <0xffffffff>;
};
};
fragment@1 {
target = <0xffffffff>;
__overlay__ {
prop = <0xffffffff>;
};
};
__fixups__ {
connectora = "/fragment@0:target:0";
gpio_connector = "/fragment@0/__overlay__:prop:0", "/fragment@1/__overlay__:prop:0";
connectorb = "/fragment@1:target:0";
};
};
```
[0]: https://lore.kernel.org/devicetree-compiler/6b2dba90-3c52-4933-88f3-b47f96dc7710@xxxxxxxxxxxxxxx/T/#m8259c8754f680b9da7b91f7b7dd89f10da91d8ed
Signed-off-by: Ayush Singh <ayush@xxxxxxxxxxxxxxx>
---
As discussed in the export-symbols kernel patch series [0] and [1], the
following patch series adds support for export-symbols in fdtoverlay.
This patch series is mostly a prototype for the functionality. Depending
on the direction, next revision of the patch will add tests.
[0]: https://lore.kernel.org/all/20241209151830.95723-1-herve.codina@xxxxxxxxxxx/
[1]: https://lore.kernel.org/r/20250110-export-symbols-v1-1-b6213fcd6c82@xxxxxxxxxxxxxxx
---
libfdt/fdt_overlay.c | 245 ++++++++++++++++++++++++++++++++++++++++++++++-----
1 file changed, 222 insertions(+), 23 deletions(-)
Thanks a lot for this patch!
I did a little modification related to unitialized variables leading to
compilation errors.
Here is my modification.
--- 8< ---
diff --git a/libfdt/fdt_overlay.c b/libfdt/fdt_overlay.c
index 7cb2230..12745b5 100644
--- a/libfdt/fdt_overlay.c
+++ b/libfdt/fdt_overlay.c
@@ -411,7 +411,7 @@ static int overlay_fixup_target_phandle(void *fdt, void *fdto, int symbols_off,
const char *symbol_path;
int prop_len;
int symbol_off;
- uint32_t phandle;
+ uint32_t phandle = 0;
value = fdt_getprop_by_offset(fdto, property,
&label, &len);
@@ -544,7 +544,7 @@ static int overlay_fixup_non_target_phandle(void *fdt, void *fdto,
const char *symbol_path;
int prop_len;
int symbol_off;
- uint32_t symbol_phandle;
+ uint32_t symbol_phandle = 0;
value = fdt_getprop_by_offset(fdto, property, &label, &len);
if (!value) {
--- 8< ---
I tested your code and fdtoverlay failed when the __symbols__ node is not in
the base devicetree.
With the export-symbols node, the resolution can be successful without the
need of the global __symbols__ node.
The error was just related to a check in overlay_fixup_one_phandle().
All verification are already done in previous operations. And so, I removed
the check and the symbols_off parameter (not needed anymore). Here is the
exact modification I did:
--- 8< ---
diff --git a/libfdt/fdt_overlay.c b/libfdt/fdt_overlay.c
index 12745b5..a9f0465 100644
--- a/libfdt/fdt_overlay.c
+++ b/libfdt/fdt_overlay.c
@@ -308,7 +308,6 @@ static int overlay_update_local_references(void *fdto, uint32_t delta)
/**
* overlay_fixup_one_phandle - Set an overlay phandle to the base one
* @fdto: Device tree overlay blob
- * @symbols_off: Node offset of the symbols node in the base device tree
* @path: Path to a node holding a phandle in the overlay
* @path_len: number of path characters to consider
* @name: Name of the property holding the phandle reference in the overlay
@@ -327,7 +326,7 @@ static int overlay_update_local_references(void *fdto, uint32_t delta)
* 0 on success
* Negative error code on failure
*/
-static int overlay_fixup_one_phandle(void *fdto, int symbols_off,
+static int overlay_fixup_one_phandle(void *fdto,
const char *path, uint32_t path_len,
const char *name, uint32_t name_len,
int poffset, uint32_t phandle)
@@ -335,9 +334,6 @@ static int overlay_fixup_one_phandle(void *fdto, int symbols_off,
fdt32_t phandle_prop;
int fixup_off;
- if (symbols_off < 0)
- return symbols_off;
-
fixup_off = fdt_path_offset_namelen(fdto, path, path_len);
if (fixup_off == -FDT_ERR_NOTFOUND)
return -FDT_ERR_BADOVERLAY;
@@ -468,7 +464,7 @@ static int overlay_fixup_target_phandle(void *fdt, void *fdto, int symbols_off,
if (path_len == (fixup_len - 1))
return -FDT_ERR_BADOVERLAY;
- ret = overlay_fixup_one_phandle(fdto, symbols_off, path,
+ ret = overlay_fixup_one_phandle(fdto, path,
path_len, "target",
sizeof("target") - 1, 0,
phandle);
@@ -628,7 +624,7 @@ static int overlay_fixup_non_target_phandle(void *fdt, void *fdto,
target_phandle = symbol_phandle;
}
- ret = overlay_fixup_one_phandle(fdto, symbols_off, path,
+ ret = overlay_fixup_one_phandle(fdto, path,
path_len, name, name_len,
poffset, target_phandle);
if (ret)
--- 8< ---
With this modification applied, fdtoverlay worked without any issue using
the following commands:
dtc -I dts -O dtb base.dts > base.dtb
dtc -I dts -O dtb overlay.dtso > overlay.dtbo
fdtoverlay -i base.dtb -o full.dtb overlay.dtbo
The resulting full.dtb was correct.
The file base.dts used for the test was:
--- 8< ---
/dts-v1/;
/{
somewhere {
base_node1: base-node1 {
prop = "base,node1";
};
base-node2 {
prop = "base,node2";
ref-base-node1 = <&base_node1>;
};
base_node3: base-node3 {
prop = "base,node3";
};
export-symbols {
base_node = <&base_node3>;
};
};
};
--- 8< ---
And the overlay.dso:
--- 8< ---
/dts-v1/;
/plugin/;
/ {
fragment@0 {
target-path = "/somewhere";
__overlay__ {
ovl_node1: ovl_node1 {
prop = "ovl,node1";
};
ovl_node2 {
prop = "ovl,node2";
ref-ovl-node1 = <&ovl_node1>;
};
ovl_node3 {
prop = "ovl,node3";
ref-base-node = <&base_node>;
};
};
};
};
--- 8< ---
Hope this will help moving forward on this topic.
Best regards,
Hervé
