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

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

Hi David,

On Tue, 22 Sep 2026 18:25:47 +0200
Herve Codina <herve.codina@bootlin.com> wrote:

> On Mon, 21 Sep 2026 16:07:21 +1000
> David Gibson <david@gibson.dropbear.id.au> 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@bootlin.com>    
> > 
> > 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@bootlin.com/#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.

b) Keep my fdt_getprop_offset_namelen() function without any modification
   We support version < 16, problem solved.

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.

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_().

On my side, I would prefer b) as it support version < 16 without the need
to expose fdt_get_property_namelen_() out of fdt_ro.c.

I would like to have your opinion on this point.
Is also option b) ok on your side ?

Best regards,
Hervé