[RFC PATCH 1/5] dt: Provide a way to remove non-compliant nodes and properties

[Sughosh Ganu]

On Sat, 26 Aug 2023 at 15:51, Heinrich Schuchardt <xypron.glpk at gmx.de> wrote:
>
>
>
> On 8/26/23 11:06, Sughosh Ganu wrote:
> > Add a function which is registered to spy for a EVT_FT_FIXUP event,
> > and removes the non upstreamed nodes and properties from the
> > devicetree before it gets passed to the OS.
> >
> > This allows removing entire nodes, or specific properties under nodes
> > from the devicetree. The required nodes and properties can be
> > registered for removal through the DT_NON_COMPLIANT_PURGE and
> > DT_NON_COMPLIANT_PURGE_LIST macros.
> >
> > Signed-off-by: Sughosh Ganu <sughosh.ganu at linaro.org>
> > ---
> >   include/dt-structs.h | 11 +++++++
> >   lib/Makefile         |  1 +
> >   lib/dt_purge.c       | 73 ++++++++++++++++++++++++++++++++++++++++++++
> >   3 files changed, 85 insertions(+)
> >   create mode 100644 lib/dt_purge.c
> >
> > diff --git a/include/dt-structs.h b/include/dt-structs.h
> > index fa1622cb1d..f535c60471 100644
> > --- a/include/dt-structs.h
> > +++ b/include/dt-structs.h
> > @@ -57,3 +57,14 @@ struct phandle_2_arg {
> >   #endif
> >
> >   #endif
> > +
> > +struct dt_non_compliant_purge {
> > +     const char *node_path;
> > +     const char *prop;
> > +};
> > +
> > +#define DT_NON_COMPLIANT_PURGE(__name)       \
> > +     ll_entry_declare(struct dt_non_compliant_purge, __name, dt_purge)
> > +
> > +#define DT_NON_COMPLIANT_PURGE_LIST(__name)  \
> > +     ll_entry_declare_list(struct dt_non_compliant_purge, __name, dt_purge)
> > diff --git a/lib/Makefile b/lib/Makefile
> > index 8d8ccc8bbc..82a906daa0 100644
> > --- a/lib/Makefile
> > +++ b/lib/Makefile
> > @@ -37,6 +37,7 @@ endif
> >   obj-y += crc8.o
> >   obj-y += crc16.o
> >   obj-y += crc16-ccitt.o
> > +obj-y += dt_purge.o
> >   obj-$(CONFIG_ERRNO_STR) += errno_str.o
> >   obj-$(CONFIG_FIT) += fdtdec_common.o
> >   obj-$(CONFIG_TEST_FDTDEC) += fdtdec_test.o
> > diff --git a/lib/dt_purge.c b/lib/dt_purge.c
> > new file mode 100644
> > index 0000000000..f893ba9796
> > --- /dev/null
> > +++ b/lib/dt_purge.c
> > @@ -0,0 +1,73 @@
> > +// SPDX-License-Identifier: GPL-2.0-or-later
> > +/*
> > + * Copyright (c) 2023, Linaro Limited
> > + */
> > +
> > +#include <dt-structs.h>
> > +#include <event.h>
> > +#include <linker_lists.h>
> > +
> > +#include <linux/libfdt.h>
> > +
> > +/**
> > + * dt_non_compliant_purge() -        Remove non-upstreamed nodes and properties
> > + *                           from the DT
> > + * @ctx: Context for event
> > + * @event: Event to process
> > + *
> > + * Iterate through an array of DT nodes and properties, and remove them
> > + * from the device-tree before the DT gets handed over to the kernel.
> > + * These are nodes and properties which do not have upstream bindings
> > + * and need to be purged before being handed over to the kernel.
> > + *
> > + * If both the node and property are specified, delete the property. If
> > + * only the node is specified, delete the entire node, including it's
> > + * subnodes, if any.
> > + *
> > + * Return: 0 if OK, -ve on error
> > + */
> > +static int dt_non_compliant_purge(void *ctx, struct event *event)
> > +{
> > +     int nodeoff = 0;
> > +     int err = 0;
> > +     void *fdt;
> > +     const struct event_ft_fixup *fixup = &event->data.ft_fixup;
> > +     struct dt_non_compliant_purge *purge_entry;
> > +     struct dt_non_compliant_purge *purge_start =
> > +             ll_entry_start(struct dt_non_compliant_purge, dt_purge);
> > +     int nentries = ll_entry_count(struct dt_non_compliant_purge, dt_purge);
> > +
> > +     if (fixup->images)
> > +             return 0;
> > +
> > +     fdt = fixup->tree.fdt;
> > +     for (purge_entry = purge_start; purge_entry != purge_start + nentries;
> > +          purge_entry++) {
> > +             nodeoff = fdt_path_offset(fdt, purge_entry->node_path);
> > +             if (nodeoff < 0) {
> > +                     log_debug("Error (%d) getting node offset for %s\n",
> > +                               nodeoff, purge_entry->node_path);
> > +                     continue;
> > +             }
> > +
> > +             if (purge_entry->prop) {
> > +                     err = fdt_delprop(fdt, nodeoff, purge_entry->prop);
> > +                     if (err < 0 && err != -FDT_ERR_NOTFOUND) {
> > +                             log_debug("Error (%d) deleting %s\n",
> > +                                       err, purge_entry->prop);
> > +                             goto out;
> > +                     }
> > +             } else {
> > +                     err = fdt_del_node(fdt, nodeoff);
> > +                     if (err) {
> > +                             log_debug("Error (%d) trying to delete node %s\n",
> > +                                       err, purge_entry->node_path);
> > +                             goto out;
> > +                     }
> > +             }
> > +     }
> > +
> > +out:
> > +     return err;
> > +}
> > +EVENT_SPY(EVT_FT_FIXUP, dt_non_compliant_purge);
>
> This may interfere with other fixup code requiring U-Boot specific
> properties like bootmeth_vbe_ft_fixup().

Okay, I was thinking if we can have a flag to indicate which event
handler is to be run. I will check this, and add a separate event if
this solution turns to be overly complex.

>
> We should ensure that call dt_non_compliant_purge() is the last fixup
> action. Please, consider a separate event.

Will check.

>
> This code does not check if the property or node complies to the device
> tree scheme or not. So why not call the function dt_purge()?

Okay

-sughosh