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

[Sughosh Ganu]

hi Simon,

On Tue, 29 Aug 2023 at 22:55, Simon Glass <sjg at chromium.org> wrote:
>
> Hi Sughosh,
>
> On Mon, 28 Aug 2023 at 12:35, Sughosh Ganu <sughosh.ganu at linaro.org> wrote:
> >
> > hi Simon,
> >
> > On Mon, 28 Aug 2023 at 23:25, Simon Glass <sjg at chromium.org> wrote:
> > >
> > > Hi Sughosh,
> > >
> > > On Sat, 26 Aug 2023 at 03:07, Sughosh Ganu <sughosh.ganu at linaro.org> 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>
> > > > ---
> > > >  .../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.
> > >
> > > It has nothing to do with describing hardware. The DT can describe
> > > other things too. See the /options node, for example.
> > >
> > > Please don't bring this highly misleading language into U-Boot.
> >
> > Please point out what is misleading in the above paragraph. What is
> > being emphasised in the above paragraph is that certain nodes and
> > properties in the devicetree are relevant only in u-boot, and not the
> > kernel. And this is precisely what the devicetree maintainers are
> > saying [1].
>
> That is not relevant though...we need to make sure all the nodes are
> in the dt schema.
>
> It is misleading because you imply that DT should only describe
> hardware. That is not true.
>
> >
> > >
> > > > +
> > > > +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
> > > > +fit for upstreaming.
> > >
> > > Then either they should not be in U-Boot, or there is a problem with
> > > the process.
> > >
> > > > 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
> > > > +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
> > > > +devicetree before it gets passed on to the OS [3].
> > >
> > > Hard NAK. If we go this way, then no one will ever have an incentive
> > > to do the right thing.
> > >
> > > Please send bindings for Linaro's work, instead. If something is
> > > entirely U-Boot-specific, then it can go in /options/u-boot but it
> > > still must be in the dt-schema.
> >
> > Please re-read the document including the last link [1]. If you go
> > through that entire thread, you will notice that this is precisely
> > what Linaro was trying to do here -- upstream the binding for the
> > fwu-mdata node. It is only based on the feedback of the devicetree
> > maintainers that this patchset was required.
>
> It looks like it should go in /options/u-boot ? Can you resubmit it there?

Okay. I will try this approach. If I face any hurdles, or the
devicetree maintainers have a different take on this, I will let you
know.

-sughosh

>
> The stripping is a no-go for me, sorry. It is absolutely going to
> destroy any chance of tidying up DT in U-Boot.
>
> Please also figure out how to add DT validation to U-Boot, so we can
> see what the gap is. Once we have that in, I will be happier to do
> workarounds.
>
> Regards,
> Simon
>
> >
> > -sughosh
> >
> > [1] - https://lore.kernel.org/u-boot/CAL_JsqJN4FeHomL7z3yj0WJ9bpx1oSE7zf26L_GV2oS6cg-5qg@mail.gmail.com/#t
> >
> > >
> > > > +
> > > > +Removing nodes/properties
> > > > +-------------------------
> > > > +
> > > > +In U-Boot, this is been done through adding information on such nodes
> > > > +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",
> > > > +       };
> > > > +
> > > > +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",
> > > > +       };
> > > > +
> > > > +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) = {
> > > > +               { .node_path = "/some_node", .prop = "some_bar" },
> > > > +               { .node_path = "/some_node" },
> > > > +       };
> > > > +
> > > > +[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/
> > > > --
> > > > 2.34.1
> > > >
> > >
> > > Regards,
> > > Simon