[U-Boot] [PATCH 2/6] fdtdec: Implement fdtdec_get_max_phandle()

[Thierry Reding <thierry.reding@gmail.com>]

On Sun, Mar 10, 2019 at 03:51:31PM -0600, Simon Glass wrote:
> Hi Thierry,
> 
> On Fri, 8 Mar 2019 at 13:11, Thierry Reding <thierry.reding@gmail.com> wrote:
> >
> > From: Thierry Reding <treding@nvidia.com>
> >
> > This function allows looking up the highest phandle value stored in a
> > device tree, which is useful to determine the next best phandle value
> > for new nodes.
> >
> > Signed-off-by: Thierry Reding <treding@nvidia.com>
> > ---
> >  include/fdtdec.h | 12 ++++++++++++
> >  lib/fdtdec.c     | 28 ++++++++++++++++++++++++++++
> >  2 files changed, 40 insertions(+)
> 
> Can we use fdt_get_max_phandle() instead? If not, could you please add
> a bit more detail to the commit message as we might consider changing
> the upstream function.

fdt_get_max_phandle() has a slightly awkward signature. It returns the
phandle via the return value, which means that it can distinguish
between error conditions and also uses 0 and -1 to signal no phandle
found and error conditions, respectively. Using the function requires
weird looking code like this:

	uint32_t phandle;

	phandle = fdt_get_max_phandle(fdt);
	if (phandle == 0 || phandle == (uint32_t)-1)
		/* process error */;

So the goal here was to add an alternative that would provide a more
standard interface where a regular error could be returned via the
return value and the phandle would be stored in an output parameter on
success.

I specifically didn't want to update the upstream function because it
was introduced about 2.5 years ago and will probably be used by some
number of users. I'm not sure if there's a stable API policy for libfdt,
but I would expect a lot of users to be annoyed if we just changed the
signature of fdt_get_max_phandle().

That said, perhaps another alternative would be to implement this as a
different function. If you look at the subsequent patches, the goal is
to generate a new phandle for newly added nodes, so perhaps something
like this could work:

	int fdtdec_generate_phandle(const void *fdt, uint32_t *phandle);

That would be slightly more in line with the higher level of fdtdec
compared to libfdt.

Or perhaps you'd prefer fdt_generate_phandle() in libfdt?

Thierry
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 833 bytes
Desc: not available
URL: <http://lists.denx.de/pipermail/u-boot/attachments/20190311/412b11fe/attachment.sig>