Re: [PATCH v3 12/15] libfdt: Introduce fdt_getprop_offset_namelen()

[Herve Codina]

Hi David,

On Sun, 25 Oct 2026 13:32:41 +1100
David Gibson <david@xxxxxxxxxxxxxxxxxxxxx> wrote:

> On Fri, Oct 23, 2026 at 09:25:59AM +0200, Herve Codina wrote:
> > Hi David,
> > 
> > On Tue, 22 Sep 2026 18:25:47 +0200
> > Herve Codina <herve.codina@xxxxxxxxxxx> wrote:
> >   
> > > On Mon, 21 Sep 2026 16:07:21 +1000
> > > David Gibson <david@xxxxxxxxxxxxxxxxxxxxx> wrote:
> > >   
> > > > On Wed, Aug 26, 2026 at 10:31:43AM +0200, Herve Codina wrote:    
> > > > > Future operations, such as handling unknown tags on dtb modifications,
> > > > > need to get an offset to a property based on the node offset and the
> > > > > property name.
> > > > > 
> > > > > Several function exists to get a property based on the node offset
> > > > > and the property name but none of them allows to get the property
> > > > > offset.
> > > > > 
> > > > > Fill the lack and introduce fdt_getprop_offset_namelen() and its
> > > > > simple variant fdt_getprop_offset().
> > > > > 
> > > > > Signed-off-by: Herve Codina <herve.codina@xxxxxxxxxxx>      
> > > > 
> > > > This seems redundant with fdt_ptr_offset_(), which can trivially get
> > > > this information when combined with fdt_get_property().    
> > > 
> > > Indeed, It should be ok using both functions you pointed out.
> > > 
> > > I will remove fdt_getprop_offset_namelen() introduced here and use directly 
> > > functions mentionned in the next iteration without wrapping those calls in
> > > a new function.   
> > 
> > I've started to replace fdt_getprop_offset_namelen() by the combination
> > of fdt_ptr_offset_() and fdt_get_property_namelen().
> > 
> > As a reminder fdt_getprop_offset_namelen() I proposed was the following:
> > --- 8< ---  
> > > +int fdt_getprop_offset_namelen(const void *fdt, int nodeoffset,
> > > +			       const char *name, int namelen)
> > > +{
> > > +	const struct fdt_property *prop;
> > > +	int lenp, offset;
> > > +
> > > +	prop = fdt_get_property_namelen_(fdt, nodeoffset, name, namelen,
> > > +					 &lenp, &offset);
> > > +	if (!prop)
> > > +		return lenp;
> > > +	return offset;
> > > +}  
> > --- 8< ---
> > 
> > fdt_get_property_namelen_() was called.
> > 
> > Switching to fdt_get_property_namelen() introduces a regression.
> > 
> > fdt_get_property_namelen() calls fdt_get_property_namelen() and this function
> > (without the '_' suffix) performs a check on fdt_version() returning an error
> > on version < 16.
> > 
> > Using it to replace my fdt_getprop_offset_namelen() function will break
> > write-in-place setprop() for dtb version < 16. Indeed, it is used in [1]
> > 
> > [1]: https://lore.kernel.org/devicetree-compiler/20260826083146.304291-15-herve.codina@xxxxxxxxxxx/#Z31libfdt:fdt_wip.c
> > 
> > Now, several options:
> > 
> > a) No problem if we break dtb version < 16
> >    Ok, I will use directly fdt_get_property_namelen() + fdt_ptr_offset_() to
> >    get an offset and remove my fdt_getprop_offset_namelen() wrapper.  
> 
> So, practically speaking this is probably true.  Version before 16 are
> truly ancient, and versions 1..3 were all before dtbs really caught
> on.
> 
> > b) Keep my fdt_getprop_offset_namelen() function without any modification
> >    We support version < 16, problem solved.  
> 
> Still don't like it.
> 
> > c) Keep my fdt_getprop_offset_namelen() but static inline in libfdt_internal.h
> >    Export fdt_get_property_namelen_() in libfdt_internal.h and use it in the
> >    static inline fdt_getprop_offset_namelen().
> >    This implies that fdt_get_property_namelen_() will be no more static and
> >    usable internally out of fdt_ro.c but only the fdt_getprop_offset_namelen()
> >    wrapper should be used.  
> 
> Eh.
> 
> > d) Use directly fdt_get_property_namelen_()
> >    Export fdt_get_property_namelen_() in libfdt_internal.h and use it
> >    directly to get the offset.
> >    I don't really like this solution. I would prefer to have a wrapper to
> >    explicitly return the offset instead of exposing and using directly
> >    fdt_get_property_namelen_().  
> 
> Hm, I think this is my preferred option.  libfdt_internal.h exists
> precisely to "expose" things across modules internally, but without
> exposing them to external users.  Maybe I'm missing something, I don't
> really see why that's objectionable.

Well, my first feeling was to avoid exposing this function and use a wrapper.
I've looked again and indeed the wrapper isn't really worth it.

I will expose (internally) and use directly fdt_get_property_namelen_().

Best regards,
Hervé