[RFC PATCH 0/5] Allow for removal of DT nodes and properties

[Simon Glass]

Hi Rob,

On Wed, 6 Sept 2023 at 08:21, Rob Herring <robh at kernel.org> wrote:
>
> On Mon, Aug 28, 2023 at 04:09:29PM -0600, Simon Glass wrote:
> > Hi Peter,
> >
> > On Mon, 28 Aug 2023 at 14:29, Peter Robinson <pbrobinson at gmail.com> wrote:
> > >
> > > On Mon, Aug 28, 2023 at 6:54 PM Simon Glass <sjg at chromium.org> wrote:
> > > >
> > > > Hi Peter,
> > > >
> > > > On Mon, 28 Aug 2023 at 10:37, Peter Robinson <pbrobinson at gmail.com> wrote:
> > > > >
> > > > > On Mon, Aug 28, 2023 at 5:20 PM Simon Glass <sjg at chromium.org> wrote:
> > > > > >
> > > > > > Hi,
> > > > > >
> > > > > > On Sat, 26 Aug 2023 at 03:07, Sughosh Ganu <sughosh.ganu at linaro.org> wrote:
> > > > > > >
> > > > > > >
> > > > > > > Provide a way for removing certain devicetree nodes and/or properties
> > > > > > > from the devicetree. This is needed to purge certain nodes and
> > > > > > > properties which may be relevant only in U-Boot. Such nodes and
> > > > > > > properties are then removed from the devicetree before it is passed to
> > > > > > > the kernel. This ensures that the devicetree passed to the OS does not
> > > > > > > contain any non-compliant nodes and properties.
> > > > > > >
> > > > > > > The removal of the nodes and properties is being done through an
> > > > > > > EVT_FT_FIXUP handler. I am not sure if the removal code needs to be
> > > > > > > behind any Kconfig symbol.
> > > > > > >
> > > > > > > I have only build tested this on sandbox, and tested on qemu arm64
> > > > > > > virt platform. This being a RFC, I have not put this through a CI run.
> > > > > > >
> > > > > > > Sughosh Ganu (5):
> > > > > > >   dt: Provide a way to remove non-compliant nodes and properties
> > > > > > >   fwu: Add the fwu-mdata node for removal from devicetree
> > > > > > >   capsule: Add the capsule-key property for removal from devicetree
> > > > > > >   bootefi: Call the EVT_FT_FIXUP event handler
> > > > > > >   doc: Add a document for non-compliant DT node/property removal
> > > > > > >
> > > > > > >  cmd/bootefi.c                                 | 18 +++++
> > > > > > >  .../devicetree/dt_non_compliant_purge.rst     | 64 ++++++++++++++++
> > > > > > >  drivers/fwu-mdata/fwu-mdata-uclass.c          |  5 ++
> > > > > > >  include/dt-structs.h                          | 11 +++
> > > > > > >  lib/Makefile                                  |  1 +
> > > > > > >  lib/dt_purge.c                                | 73 +++++++++++++++++++
> > > > > > >  lib/efi_loader/efi_capsule.c                  |  7 ++
> > > > > > >  7 files changed, 179 insertions(+)
> > > > > > >  create mode 100644 doc/develop/devicetree/dt_non_compliant_purge.rst
> > > > > > >  create mode 100644 lib/dt_purge.c
> > > > > >
> > > > > > What is the point of removing them? Instead, we should make sure that
> > > > > > we upstream the bindings and encourage SoC vendors to sync them. If we
> > > > > > remove them, no one will bother and U-Boot just becomes a dumping
> > > > > > ground.
> > > > >
> > > > > Well things like the binman entries in DT are U-Boot specific and not
> > > > > useful for HW related descriptions or for Linux or another OS being
> > > > > able to deal with HW so arguably we're already a dumping ground to
> > > > > some degree for not defining hardware.
> > > >
> > > > I have started the process to upstream the binman bindings.
> > >
> > > I don't think they should be in DT at all, they don't describe
> > > anything to do with hardware, or generally even the runtime of a
> > > device, they don't even describe the boot/runtime state of the
> > > firmware, they describe build time, so I don't see what that has to do
> > > with device tree? Can you explain that? To me those sorts of things
> > > should live in a build time style config file.
>
> For the record, I tend to agree.
>
> > I beg to differ. Devicetree is more than just hardware and always has
> > been. See, for example the /chosen and /options nodes.
>
> There are exceptions...

I wish we could stop having this discussion entirely...to me the
devicetree is all that firmware has to handle its configuration, both
within projects and across project boundaries. There is no user space,
no filesystem, etc.

>
> > We need run-time configuration here, since we cannot know at build
> > time what we will be asked to do by a previous firmware phase.
>
> Really, I don't want to have to care about the binman binding. If it is
> u-boot specific, then it should stay in u-boot.

I don't believe it is U-Boot-specific since it describes the system
firmware as a whole. It collects things from multiple projects and we
want to know where they ended up, so we can do firmware update, etc.

> I took /options/u-boot/,
> but now I'm starting to have second thoughts on that being in dtschema
> if it is going to be continually and frequently extended. Validating it
> in SR does little. If a vendor is abusing /options/u-boot/ in some way
> they could just as easily remove the node in their u-boot fork to pass.
> SR is certainly not going to require the node be there.

The challenge here is that we are trying to validate that the U-Boot
DT matches the schema. We don't want to have people adding things
willy nilly, even in /options/u-boot.

>
> On A/B updates, that really doesn't seem like a u-boot specific problem
> to me. No one wants A/B updates in EDK2 or anything else?

I agree we should specify it correctly and in a general manner.

Regards,
Simon