On Fri, Jan 10, 2025 at 11:18:55PM +0530, Ayush Singh 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 walks the tree. > > 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]. > > [0]: > https://lore.kernel.org/devicetree-compiler/6b2dba90-3c52-4933-88f3-b47f96dc7710@xxxxxxxxxxxxxxx/T/#m8259c8754f680b9da7b91f7b7dd89f10da91d8ed I'm not sold on the concept of export-symbols in the first place. But at the very least this commit message needs to explain what actually needs to be done within dtc to handle them. It's not obvious to me why it needs to do anything here, as opposed to in dtc consumers. > > Signed-off-by: Ayush Singh <ayush@xxxxxxxxxxxxxxx> > --- > As discussed in the export-symbols kernel patch series [0], the > following patch series adds support for export-symbols in the base > devicetree compiler. > > This patch series is mostly a prototype for the functionality. Depending > on the direction, next revision of the patch will add tests. > > Support for export-symbols in overlays will be part of a seperate patch > series. > > [0]: https://lore.kernel.org/all/20241209151830.95723-1-herve.codina@xxxxxxxxxxx/ > --- > checks.c | 42 ++++++++++++++++++++++++++++++++++++++++-- > 1 file changed, 40 insertions(+), 2 deletions(-) > > diff --git a/checks.c b/checks.c > index 123f2eb425f4a2f8ac22bfe10a842bf08e296ba1..e577f30a3ed9b762aee073fa1ff9d866f5de60b6 100644 > --- a/checks.c > +++ b/checks.c > @@ -600,11 +600,37 @@ ERROR(name_properties, check_name_properties, NULL, &name_is_string); > * Reference fixup functions > */ > > +static const char *fixup_export_symbol(struct node *export_symbols, > + struct marker *m) > +{ > + struct property *prop; > + struct marker *em; > + > + for_each_property(export_symbols, prop) { > + if (streq(m->ref, prop->name)) { > + em = prop->val.markers; > + for_each_marker_of_type(em, REF_PHANDLE) { > + return em->ref; This requires the contents of the symbol to be defined as a reference to work. Yes, that's the obvious way to do it, but in existing things if you really want to hand code a phandle value, you can do that. This makes it so that trees that would otherwise have byte-for-byte identical dtb output don't work the same way. > + } > + } > + } > + > + return NULL; > +} > + > static void fixup_phandle_references(struct check *c, struct dt_info *dti, > struct node *node) > { > struct node *dt = dti->dt; > struct property *prop; > + struct node *child, *export_symbols = NULL; > + > + for_each_child(node, child) { > + if (streq(child->name, "export-symbols")) { > + export_symbols = child; > + break; > + } > + } > > for_each_property(node, prop) { > struct marker *m = prop->val.markers; > @@ -612,13 +638,25 @@ static void fixup_phandle_references(struct check *c, struct dt_info *dti, > cell_t phandle; > > for_each_marker_of_type(m, REF_PHANDLE) { > + const char *ref = NULL; > + > assert(m->offset + sizeof(cell_t) <= prop->val.len); > > - refnode = get_node_by_ref(dt, m->ref); > + /* Check export-symbols if present */ > + if (export_symbols) > + ref = fixup_export_symbol(export_symbols, m); > + > + /* If entry not found in export-symbols (or export-symbols not present), > + * search the normal way. > + */ > + if (!ref) > + ref = m->ref; This muddles semantic layers together. In dtc, the *contents* of nodes and properties doesn't affect the output other than that node or property (though it might affect warnings). Only actual dts syntax affects output non-locally. This changes that, which I think is a very bad idea for understandability. > + > + refnode = get_node_by_ref(dt, ref); > if (! refnode) { > if (!(dti->dtsflags & DTSF_PLUGIN)) > FAIL(c, dti, node, "Reference to non-existent node or " > - "label \"%s\"\n", m->ref); > + "label \"%s\"\n", ref); > else /* mark the entry as unresolved */ > *((fdt32_t *)(prop->val.val + m->offset)) = > cpu_to_fdt32(0xffffffff); > > --- > base-commit: 18f4f305fdd7e14c8941658a29c7b85c27d41de4 > change-id: 20250110-export-symbols-e2ea3df01ea9 > > Best regards, -- David Gibson (he or they) | I'll have my music baroque, and my code david AT gibson.dropbear.id.au | minimalist, thank you, not the other way | around. http://www.ozlabs.org/~dgibson
Attachment:
signature.asc
Description: PGP signature
