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é
