Skip to content

Some phandle related improvements and some bug fixes#93

Closed
ukleinek wants to merge 6 commits intodgibson:mainfrom
ukleinek:fdt-phandle
Closed

Some phandle related improvements and some bug fixes#93
ukleinek wants to merge 6 commits intodgibson:mainfrom
ukleinek:fdt-phandle

Conversation

@ukleinek
Copy link
Contributor

This series start with a few minor improvments.

The fourth patch fixes overlays overwriting phandles in the base dtb and thus breaking references there.

The last patch interprets the nodes /__symbols and /__local_fixups to restore labels and references when compiling to dts.

All patches were also sent to the devicetree-compiler mailing list.

Uwe Kleine-König and others added 6 commits April 24, 2023 17:31
Device trees with a /plugin/ tag ("overlays") generate a __fixups__ node
when needed and independent of -q being given or not. The same is true for
__local__fixups__. So don't mention these two nodes in the paragraph about
-@.

To not shorten the description too much, describe the semantic of the
properties contained in the generated __symbols__ node.

Signed-off-by: Uwe Kleine-König <u.kleine-koenig@pengutronix.de>
Signed-off-by: Uwe Kleine-König <uwe@kleine-koenig.org>
A string that contains '\0' can be written as a list of strings e.g.

	clock-names = "di0_pll\0di1_pll\0di0_sel\0di1_sel\0di2_sel\0di3_sel\0di0\0di1";

is equivalent to

	clock-names = "di0_pll", "di1_pll", "di0_sel", "di1_sel", "di2_sel", "di3_sel", "di0", "di1";

The latter is easier to read, to use this format instead.

Signed-off-by: Uwe Kleine-König <u.kleine-koenig@pengutronix.de>
A phandle in an overlay is not supposed to overwrite a phandle that
already exists in the base dtb as this breaks references to the respective
node in the base.

Traditionally all phandle values in the overlay were increased by a fixed
offset such that the base and overlay handle values don't overlap. With
this change that still happens to phandles that are new. Already existing
phandles are kept and an array "phandlemap" is used to track how the
overlay's phandles change.

overlay_adjust_local_phandles() which creates the mapping between old and
new phandles in the overlay needs to check the nodes in the base. To be
able to find the base's node, overlay_fixup_phandles() must be run first,
so the order of functions fdt_overlay_apply() had to change.

A test is added that checks that newly added phandles and existing phandles
work as expected.

Signed-off-by: Uwe Kleine-König <u.kleine-koenig@pengutronix.de>
This happens implicitly for dts files with a /plugin/ tag.

Signed-off-by: Uwe Kleine-König <u.kleine-koenig@pengutronix.de>
The data contained in these nodes can be used to restore labels for nodes
(from __symbols__ if -@ is given) and to identify which data values are
references (from __local_fixups__ if -L is given).

The resulting dts doesn't necessarily compiles back into the bitwise same
dtb, but the result is equivalent and easier to modify without breaking
references.

Signed-off-by: Uwe Kleine-König <u.kleine-koenig@pengutronix.de>
@ukleinek
Copy link
Contributor Author

The ci failures are unrelated to my changes I think. make and make check work fine for me and I didn't touch the yaml stuff.

Copy link
Owner

@dgibson dgibson left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I've applied a couple of the simpler patches here, and made comments on some of the others. For the bigger patches, I have some more substantial comments on the mailing list.

@@ -122,12 +122,10 @@ Options:
Relevant for dtb and asm output only.
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Not shortening the description isn't a virtue of itself. However, the information you've added is potentially useful, so the patch is good.

Generates a __local_fixups__ node at the root node. For each property
that contains a phandle, the __local_fixups__ node contains a property
(at path __local_fixups__/$a if $a is the path of the node). Its value
is a list of offsets that are phandle values.
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This doesn't seem quite accurate. It will generate __fixups__ as well as __local_fixups__, although that's likely not to exist in the cases you use it with. I think it's worth explicitly pointing out how this interacts with /plugin/.

@dgibson
Copy link
Owner

dgibson commented Apr 28, 2023

Oh, and, yes, the github CI has been broken for some time (since Travis CI went away at least, I think). I just haven't had the time to figure out how github CI stuff works and fix it up.

@ukleinek
Copy link
Contributor Author

Oh, and, yes, the github CI has been broken for some time (since Travis CI went away at least, I think). I just haven't had the time to figure out how github CI stuff works and fix it up.

I created a GitHub Actions setup, see #94

@ukleinek
Copy link
Contributor Author

Some of the changes are in main already (slightly different) and the remainder is in #151, so I'm closing this one.

@ukleinek ukleinek closed this Oct 21, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants