Re: [PATCH v2 3/6] dtc: Clarify when delete placeholders are dropped

[Herve Codina <herve.codina@bootlin.com>]

Hi Simon,

On Wed, 28 Oct 2026 03:36:10 -0600
Simon Glass <sjg@chromium.org> wrote:

> The comment on drop_delete_placeholders() does not make clear which
> subtrees it is for. The call in merge_nodes() also relies on sections
> being merged in source order, since it drops the child's placeholders on
> the basis that nothing earlier remains to be merged, but nothing there
> says so.
> 
> Say that drop_delete_placeholders() is for the first definition of a
> node, i.e. the first in the list of definitions which are merged
> together. Explain at the call in merge_nodes() why the child is the
> first definition, and that merging is not associative.
> 
> Suggested-by: David Gibson <david@gibson.dropbear.id.au>
> Assisted-by: Claude:claude-opus-5-5
> Signed-off-by: Simon Glass <sjg@chromium.org>
> ---
> 
> Changes in v2:
> - Add new patch to clarify the comments
> 
>  livetree.c | 14 ++++++++++++--
>  1 file changed, 12 insertions(+), 2 deletions(-)
> 
> diff --git a/livetree.c b/livetree.c
> index 6609e4e..b7a5161 100644
> --- a/livetree.c
> +++ b/livetree.c
> @@ -122,8 +122,10 @@ static void apply_deletes(struct node *node)
>  
>  /*
>   * Drop the /delete-property/ and /delete-node/ placeholders from a subtree
> - * which joins the tree without being merged, since there is nothing earlier
> - * for them to delete
> + * which is the first definition of its node, i.e. the first in the list of
> + * definitions which are merged together. apply_deletes() has already dropped
> + * what they delete within the subtree, and there is nothing earlier for them
> + * to delete
>   */
>  void drop_delete_placeholders(struct node *node)
>  {
> @@ -273,6 +275,14 @@ struct node *merge_nodes(struct node *old_node, struct node *new_node)
>  
>  		/* if no collision occurred, add child to the old node. */
>  		if (new_child) {
> +			/*
> +			 * This is the first definition of the child, since
> +			 * sections are merged in source order, so its
> +			 * placeholders have nothing to delete. This is
> +			 * important since dropping them makes merging not
> +			 * associative: merging later sections together first
> +			 * would lose their deletes
> +			 */
>  			drop_delete_placeholders(new_child);
>  			add_child(old_node, new_child);
>  		}

drop_delete_placeholders() has been added in patch 1.

Why those both modification related to comment here in patch 3
Worth noting that patch 2 is only related to tests.

IHMO, this patch should be squashed in patch 1.

Best regards,
Hervé