From mboxrd@z Thu Jan  1 00:00:00 1970
From: Simon Glass <sjg@chromium.org>
Date: Sat, 23 Mar 2019 08:52:44 +0800
Subject: [U-Boot] [PATCH v3 04/13] fdtdec: Implement fdtdec_set_phandle()
In-Reply-To: <20190322083446.GB28640@ulmo>
References: <20190321181010.27005-1-thierry.reding@gmail.com>
 <20190321181010.27005-5-thierry.reding@gmail.com>
 <CAPnjgZ2B-zMoAM60W_Vpty6jT_O40QZW1yO73ZoX2_iyHWtW7g@mail.gmail.com>
 <20190322083446.GB28640@ulmo>
Message-ID: <CAPnjgZ1e9gnh7ak6xWnTOUkEivuuNAreke9Zuu9YSqoOzjEJ7Q@mail.gmail.com>
List-Id: <u-boot.lists.denx.de>
MIME-Version: 1.0
Content-Type: text/plain; charset="us-ascii"
Content-Transfer-Encoding: 7bit
To: u-boot@lists.denx.de

Hi Thierry,

On Fri, 22 Mar 2019 at 16:34, Thierry Reding <thierry.reding@gmail.com> wrote:
>
> On Fri, Mar 22, 2019 at 03:53:01PM +0800, Simon Glass wrote:
> > Hi Thierry,
> >
> > On Fri, 22 Mar 2019 at 02:10, Thierry Reding <thierry.reding@gmail.com> wrote:
> > >
> > > From: Thierry Reding <treding@nvidia.com>
> > >
> > > This function can be used to set a phandle for a given node.
> > >
> > > Signed-off-by: Thierry Reding <treding@nvidia.com>
> > > ---
> > > Changes in v2:
> > > - don't emit deprecated linux,phandle property
> > >
> > >  include/fdtdec.h | 11 +++++++++++
> > >  lib/fdtdec.c     |  7 +++++++
> > >  2 files changed, 18 insertions(+)
> > >
> > > diff --git a/include/fdtdec.h b/include/fdtdec.h
> > > index a0ba57c6318b..55600026c488 100644
> > > --- a/include/fdtdec.h
> > > +++ b/include/fdtdec.h
> > > @@ -981,6 +981,17 @@ int fdtdec_setup_mem_size_base(void);
> > >   */
> > >  int fdtdec_setup_memory_banksize(void);
> > >
> > > +/**
> > > + * 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 09a7e133a539..00db90e3cdfd 100644
> > > --- a/lib/fdtdec.c
> > > +++ b/lib/fdtdec.c
> > > @@ -1243,6 +1243,13 @@ __weak void *board_fdt_blob_setup(void)
> > >  }
> > >  #endif
> > >
> > > +int fdtdec_set_phandle(void *blob, int node, uint32_t phandle)
> > > +{
> > > +       fdt32_t value = cpu_to_fdt32(phandle);
> > > +
> > > +       return fdt_setprop(blob, node, "phandle", &value, sizeof(value));
> >
> > Can we use fdt_setprop_u32() instead?
>
> Yeah, we could. I'm not sure if after that optimization it's even worth
> keeping the extra wrapper. There may be some benefit in having a
> separate name for this because it's a somewhat special purpose. Maybe I
> should make it a static inline function instead?

I think it is worth it. But perhaps a static inline makes sense, up to you.

Regards,
Simon