[RFC PATCH 5/5] doc: Add a document for non-compliant DT node/property removal

[Sughosh Ganu]

On Sat, 26 Aug 2023 at 15:35, Heinrich Schuchardt <xypron.glpk at gmx.de> wrote:
>
> On 8/26/23 11:06, Sughosh Ganu wrote:
> > Add a document explaining the need for removal of non-compliant
> > devicetree nodes and properties. Also describe in brief, the macros
> > that can be used for this removal.
> >
> > Signed-off-by: Sughosh Ganu <sughosh.ganu at linaro.org>
>
> Thanks for properly documenting the change.
>
> Warning, treated as error:
> doc/develop/devicetree/dt_non_compliant_purge.rst:
> document isn't included in any toctree
>
> Please, add the document to doc/develop/devicetree/index.rst
>
> Please, run make htmldocs before resubmitting.
>
> > ---
> >   .../devicetree/dt_non_compliant_purge.rst     | 64 +++++++++++++++++++
> >   1 file changed, 64 insertions(+)
> >   create mode 100644 doc/develop/devicetree/dt_non_compliant_purge.rst
> >
> > diff --git a/doc/develop/devicetree/dt_non_compliant_purge.rst b/doc/develop/devicetree/dt_non_compliant_purge.rst
> > new file mode 100644
> > index 0000000000..c3a8feab5b
> > --- /dev/null
> > +++ b/doc/develop/devicetree/dt_non_compliant_purge.rst
> > @@ -0,0 +1,64 @@
> > +.. SPDX-License-Identifier: GPL-2.0+
> > +
> > +Removal of non-compliant nodes and properties
> > +=============================================
> > +
> > +The devicetree used in U-Boot might contain nodes and properties which
> > +are specific only to U-Boot, and are not necessarily being used to
> > +describe hardware but to pass information to U-Boot. An example of
> > +such a property would be the public key being passed to U-Boot for
> > +verification.
> > +
> > +This devicetree can then be passed to the OS. Since certain nodes and
> > +properties are not really describing hardware, and more importantly,
> > +these are only relevant to U-Boot, bindings for these cannot be
> > +upstreamed into the devicetree repository. There have been instances
> > +of attempts being made to upstream such bindings, and these deemed not
>
> but these were not deemed fit
>
> > +fit for upstreaming. Not having a binding for these nodes and
> > +properties means that the devicetree fails the schema compliance tests
> > +[1]. This also means that the platform cannot get certifications like
> > +SystemReady [2] which, among other things require a devicetree which
>
> %s/require/requires/
>
> > +passes the schema compliance tests.
> > +
> > +For such nodes and properties, it has been suggested by the devicetree
> > +maintainers that the right thing to do is to remove them from the
>
> %s/that the right thing to do is//
>
> > +devicetree before it gets passed on to the OS [3].
>
> %s/on to/to/
>
> > +
> > +Removing nodes/properties
> > +-------------------------
> > +
> > +In U-Boot, this is been done through adding information on such nodes
>
> %s/is been done through/is done by/
>
> > +and properties in a list. The entire node can be deleted, or a
> > +specific property under a node can be deleted. The list of such nodes
> > +and properties is generated at compile time, and the function to purge
> > +these can be invoked through a EVT_FT_FIXUP event notify call.
> > +
> > +For deleting a node, this can be done by declaring a macro::
> > +
> > +     DT_NON_COMPLIANT_PURGE(fwu_mdata) = {
> > +             .node_path      = "/fwu-mdata",
> > +     };
>
> Where should such a macro be placed?

It should be placed in any file that will get compiled.

>
> > +
> > +Similarly, for deleting a property under a node, that can be done by
> > +specifying the property name::
> > +
> > +     DT_NON_COMPLIANT_PURGE(capsule_key) = {
> > +             .node_path      = "/signature",
> > +             .prop           = "capsule-key",
> > +     };
>
> Why is capsule_key needed twice here? What would be the effect of:
>
> DT_NON_COMPLIANT_PURGE(voodoo) = {
>         .node_path      = "/signature",
>         .prop           = "capsule-key",
> };

In your above example, voodoo just happens to be the name of the entry
that will be created. But the property that will be searched in the
devicetree for removal is "capsule-key".

>
> > +
> > +In the first example, the entire node with path /fwu-mdata will be
> > +removed. In the second example, the property capsule-key
> > +under /signature node will be removed.
> > +
> > +Similarly, a list of nodes and properties can be specified using the
> > +following macro::
> > +
> > +     DT_NON_COMPLIANT_PURGE_LIST(foo) = {
>
> What does foo refer to here? Can this parameter be eliminated from the
> syntax?

foo is the name of the entry being generated through the
ll_entry_declare macro. We do need this parameter for ll_entry_declare
to generate a unique entry. Using a common name results in a linker
error.

>
> Will the application of the removals be sorted alphabetically by this
> argument?

I am not sure how the linker arranges the entries in the list. Why
does it matter?

>
> What will happen if a property or node does not exist at time of
> removal? Hopefully that does not stop the boot process but is silently
> ignored.

Yes, it will not cause the boot to fail.

>
> > +             { .node_path = "/some_node", .prop = "some_bar" },
> > +             { .node_path = "/some_node" },
> > +     };
>
> Why do you need the first element in your example?
> Wouldn't deleting /some/node imply that all its children are deleted?

Ah, this is a bad example. I should be using a different name for the
second entry here. Will fix.

>
> Why do we need separate properties .node_path and .prop. It would be
> less effort to write:
>
> DT_NON_COMPLIANT_PURGE_LIST() =
> {
>         "/some_node_1/some_property_1",
>         "/some_node_1/some_property_2",
>         "/some_node_2",
> }
>
> I assume the macro does its job irrespective of non-compliance.
> Why not call it DT_PURGE()?

Yes, it can be called DT_PURGE. I was using the term non_compliant
simply to highlight the fact that we would need to use this for
removal of non-compliant nodes or properties.

-sughosh

>
> Best regards
>
> Heinrich
>
> > +
> > +[1] - https://github.com/devicetree-org/dt-schema
> > +[2] - https://www.arm.com/architecture/system-architectures/systemready-certification-program
> > +[3] - https://lore.kernel.org/u-boot/CAL_JsqJN4FeHomL7z3yj0WJ9bpx1oSE7zf26L_GV2oS6cg-5qg@mail.gmail.com/