Re: [PATCH v3 1/4] libfdt: overlay_merge: Introduce fdt_overlay_merge()

[David Gibson <david@gibson.dropbear.id.au>]

[-- Attachment #1: Type: text/plain, Size: 3126 bytes --]

On Fri, Jun 27, 2025 at 04:01:02PM +0530, Wasim Nazir wrote:
> On Tue, Jun 03, 2025 at 09:09:12PM +1000, David Gibson wrote:
> > On Fri, May 30, 2025 at 05:58:58PM +0530, Wasim Nazir wrote:
> > > On Wed, May 21, 2025 at 02:23:29PM +1000, David Gibson wrote:
> > > > On Mon, May 19, 2025 at 02:40:40PM +0530, Wasim Nazir wrote:
> > > > > From: Srivatsa Vaddagiri <vatsa@codeaurora.org>
> > > > > 
> > > > > fdt_overlay_merge() merges two overlay blobs. It is largely expected to be
> > > > > used offline on a build machine to combine two or more overlay blobs into one.
> > > > > 
> > > > > It is intended to help maintain device-tree overlay code in
> > > > > multiple source repositories, but merge their binary forms (overlay blobs)
> > > > > into one so that bootloader's task of searching for all relevant overlay blobs
> > > > > is simplified.
> > > > > 
> > > > > Introduce fdt_overlay_merge() which is identical to fdt_overlay_apply().
> > > > > Subsequent patches will introduce required changes to merge overlay blobs.
> > > > > 
> > > > > Signed-off-by: Srivatsa Vaddagiri <vatsa@codeaurora.org>
> > > > > Signed-off-by: Wasim Nazir <quic_wasimn@quicinc.com>
> > > > > ---
> > > > >  libfdt/fdt_overlay.c | 59 ++++++++++++++++++++++++++++++++++++++++++++
> > > > >  libfdt/libfdt.h      | 18 ++++++++++++++
> > > > >  2 files changed, 77 insertions(+)
> > > > > 
> > > > > diff --git a/libfdt/fdt_overlay.c b/libfdt/fdt_overlay.c
> > > > > index e6b9eb643958..8690ed55c8f6 100644
> > > > > --- a/libfdt/fdt_overlay.c
> > > > > +++ b/libfdt/fdt_overlay.c
> > > > > @@ -1098,3 +1098,62 @@ err:
> > > > > 
> > > > >  	return ret;
> > > > >  }
> > > > > +
> > > > > +int fdt_overlay_merge(void *fdt, void *fdto, int *fdto_nospace)
> > > > 
> > > > The parameters should be renamed to reflect the new semantics.
> > > 
> > > Sure will change to proper name.
> > > 
> > > > 
> > > > The 'fdto_nospace' parameter seems weird.  Why not just return
> > > > -FDT_ERR_NOSPACE from the function?
> > > 
> > > fdto_nospace variable is used to know cases when 2nd fdto (overlaying fdto)
> > > needs more space while -FDT_ERR_NOSPACE is used for cases when
> > > 1st fdto (base fdto) needs more space.
> > 
> > Ah, I see.  I can see the reason now, but it's still a deeply ugly
> > interface.
> > 
> > Hmm... why do you need to ever expand the applied size?  It's obvious
> > you'd need to expand the based dtbo, but not why you'd ever need to
> > expand the one you're applying on top.
> 
> Since, we are updating phandle references of fragments & __local_fixups__ of
> 2nd dtbo so we need to check that.

Updating phandle references doesn't usually require expanding the
tree, since phandles have a fixed size.  Have you actually hit running
out of space for the dtb in practice, or are you just assuming you
need this because you're making modifications to the dtbo.

-- 
David Gibson (he or they)	| I'll have my music baroque, and my code
david AT gibson.dropbear.id.au	| minimalist, thank you, not the other way
				| around.
http://www.ozlabs.org/~dgibson

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]