[U-Boot] [PATCH 3/6] fdtdec: Implement fdtdec_set_phandle()

[Thierry Reding]

On Sun, Mar 10, 2019 at 03:51:40PM -0600, Simon Glass wrote:
> Hi Thierry,
> 
> On Fri, 8 Mar 2019 at 13:11, Thierry Reding <thierry.reding at gmail.com> wrote:
> >
> > From: Thierry Reding <treding at nvidia.com>
> >
> > This function can be used to set a phandle for a given node.
> >
> > Signed-off-by: Thierry Reding <treding at nvidia.com>
> > ---
> >  include/fdtdec.h | 11 +++++++++++
> >  lib/fdtdec.c     | 16 ++++++++++++++++
> >  2 files changed, 27 insertions(+)
> 
> This seems OK, although I think it should have a test.

I'll add something to test this to the test_fdtdec command. I'm not sure
how much sense it makes to test this individually, because the test is
fairly elaborate (needs to create one node to reference and another to
reference it from), so perhaps I can just add a complete test that will
at the same time validate that the reserved-memory and carveout stuff
works?

> But what about livetree? I think it would make more sense to add a
> high-level API which can deal with livetree/flattree.

I'm not sure it really makes sense to add this kind of information to a
livetree. The only use-case for this that I have is to convey
information about carveouts to the kernel, so by this information is
added to a DTB that is loaded from external storage along with the
kernel and then modified right before the DTB is passed to the kernel on
boot.

The only case where I think such functionality would be useful in a live
tree is if we construct the live tree in U-Boot, update it and then pass
it to the kernel upon boot. That's not something that works today, and I
can't test any of that, so I'm not sure it makes much sense to implement
it now.

> > diff --git a/include/fdtdec.h b/include/fdtdec.h
> > index 5eb3c0c237a9..997103a87cdf 100644
> > --- a/include/fdtdec.h
> > +++ b/include/fdtdec.h
> > @@ -968,6 +968,17 @@ int fdtdec_setup_memory_banksize(void);
> >   */
> >  int fdtdec_get_max_phandle(const void *blob, uint32_t *maxp);
> >
> > +/**
> > + * fdtdec_set_phandle() - sets the phandle of a given node
> > + *
> > + * @param blob         FDT blob
> > + * @param node         offset in the FDT blob of the node whose phandle is to
> > + *                     be set
> > + * @param phandle      phandle to set for the given node
> > + * @return 0 on success or a negative error code on failure
> > + */
> > +int fdtdec_set_phandle(void *blob, int node, uint32_t phandle);
> > +
> >  /**
> >   * Set up the device tree ready for use
> >   */
> > diff --git a/lib/fdtdec.c b/lib/fdtdec.c
> > index f2af947c106e..9195a05d1129 100644
> > --- a/lib/fdtdec.c
> > +++ b/lib/fdtdec.c
> > @@ -1271,6 +1271,22 @@ int fdtdec_get_max_phandle(const void *blob, uint32_t *maxp)
> >         return 0;
> >  }
> >
> > +int fdtdec_set_phandle(void *blob, int node, uint32_t phandle)
> > +{
> > +       fdt32_t value = cpu_to_fdt32(phandle);
> > +       int err;
> > +
> > +       err = fdt_setprop(blob, node, "linux,phandle", &value, sizeof(value));
> > +       if (err < 0)
> > +               return err;
> 
> Why set both properties?

I was trying to mimic the output of DTC, but looking again it seems like
it doesn't even produce linux,phandle properties. Doing some research it
was changed to only produce "phandle" properties in v1.4.5 and the
commit message says:

	commit 0016f8c2aa32423f680ec6e94a00f1095b81b5fc
	Author: Rob Herring <robh at kernel.org>
	Date:   Wed Jul 12 17:20:30 2017 -0500

	    dtc: change default phandles to ePAPR style instead of both

	    Currently, both legacy (linux,phandle) and ePAPR (phandle) properties
	    are inserted into dtbs by default. The newer ePAPR style has been
	    supported in dtc and Linux kernel for 7 years. That should be a long
	    enough transition period. We can save a little space by not putting both
	    into the dtb.

	    Signed-off-by: Rob Herring <robh at kernel.org>
	    Signed-off-by: David Gibson <david at gibson.dropbear.id.au>

So perhaps we no longer need to generate this from U-Boot either. I'll
remove the linux,phandle code.

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/ff2d1fc4/attachment.sig>