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

[Heinrich Schuchardt]

On 9/7/23 00:04, Tom Rini wrote:
> On Wed, Sep 06, 2023 at 09:21:39AM -0500, Rob Herring 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...
>>
>>> 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 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 missing piece is validation of the U-Boot internal device-trees
against a schema in the U-Boot CI. This should be possible even if some
of the schema yaml files are maintained inside the U-Boot repo.

Best regards

Heinrich

>
> It's going to continue to be a "fun" tight-rope to walk. No one wants an
> easy way for vendors to cheat the requirements which is why in some
> other parts of this thread (you may or may not have been on, I don't
> recall, sorry) I've been trying to make it clear that the removal
> mechanism should be both a slight pain to add to, and at least wrt
> upstream, clear that effort was made to upstream things.  Cheaters are
> going to cheat and they could just chain a bunch of "fdt rm" together to
> do it today.
>