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

[Heinrich Schuchardt]

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?

> +
> +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 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?

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

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.

> +		{ .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?

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()?

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/