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

David Gibson <[email protected]> Sat, 28 Jun 2025 21:02:18 +1000
Newsgroups	org.kernel.vger.devicetree-compiler
Message-ID	<aF_Luosqutkq6CVo@zatzit>
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 <[email protected]>
> > > > > 
> > > > > 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 <[email protected]>
> > > > > Signed-off-by: Wasim Nazir <[email protected]>
> > > > > ---
> > > > >  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
signature.asc (application/pgp-signature, 833 B)