schemas: bootph: Add check for bootph tags in parent nodes #99
Conversation
When a node has boot phase tags, the parent node is implicitly included and a bootph tag is not needed. Add a schema to check for and disallow bootph tags in both parent and child nodes. It's not the greatest logic and resulting error message, but that's what works for json-schema. Signed-off-by: Rob Herring <[email protected]>
|
If I understand it correctly, this will break all existing devicetree files in U-Boot, since the build system does not support this yet. I need to get the fdtgrep tool upstreamed and then enhance it to handle this, then update U-Boot. I believe it if feasible but it is going to take some time. |
| - required: [ bootph-some-ram ] | ||
| - required: [ bootph-all ] | ||
| then: | ||
| description: Parent nodes don't need bootph tags |
There was a problem hiding this comment.
They don't need the same tag, but they might need a different one. I think the rule will be that if a child has a particular phase tag then it is implied in all its parents
There was a problem hiding this comment.
That should be doable. What about a child with bootph-all and parent with something else?
There was a problem hiding this comment.
In that case we are implying bootph-all in the parent so it doesn't matter what tags are present
It only breaks them if you run the checks. Nothing really breaks, just adds to the pile of warnings many platforms have. Does anyone actually run dtschema on u-boot copies of DTs? I'm assuming the order here is enhance fdtgrep and u-boot first, then submit bootph tags to Linux copies of dts files. So this change is primarily to check the Linux copies. And from the look of it, some platforms may not need to wait as they don't have this issue. |
|
We don't run the schema in U-Boot, but would like to one day. But once we have things in Linux, your warning is going to advise people to change the tags, and then U-Boot won't work. It will be very confusing. Are you saying that you won't accept DTs into Linux until the fdtgrep stuff is done? That is really putting pressure on all of this... |
Yes, that's my position. The dts files in Linux are loosely coupled to u-boot compared to dts files in u-boot tree. IOW, it's an ABI. Plus, why would we add some tags and then later remove them. Just wait until things are stable. However, I don't review (typically) nor apply dts patches (have to pick my firehoses). I do look at overall warnings by platform families though. |
|
OK, well perhaps initially I will just upstream what we have and worry about tidying up later. I had a bit of a look at what it would take to implement your scheme. Options I came up with:
In terms of development time, probably 2 is best. If you want to have a look, see fdt_next_region() where it has the 'case FDT_PROP'. The h_include() function is in fdtgrep.c Obviously fdtgrep needs to know that the bootph things must be propagated upwards. That could be done by adding a new flag in addition to --include-node-with-prop (such as --include-tree-with-prop) |
|
did we decide to pull this in? |
Well I implemented option 2 in fdtgrep in U-Boot, so it is working. The commit is: 7a06cc2027c fdtgrep: Allow propagating properties up to supernodes which landed in v2024.04 |
|
Yes, and I would like to catch places where we can clean up in the kernel if we have this as part of a formal release. |
When a node has boot phase tags, the parent node is implicitly included and a bootph tag is not needed. Add a schema to check for and disallow bootph tags in both parent and child nodes. It's not the greatest logic and resulting error message, but that's what works for json-schema.