From mboxrd@z Thu Jan  1 00:00:00 1970
From: Simon Glass <sjg@chromium.org>
Date: Sun, 10 Mar 2019 15:51:40 -0600
Subject: [U-Boot] [PATCH 3/6] fdtdec: Implement fdtdec_set_phandle()
In-Reply-To: <20190308201140.2383-3-thierry.reding@gmail.com>
References: <20190308201140.2383-1-thierry.reding@gmail.com>
 <20190308201140.2383-3-thierry.reding@gmail.com>
Message-ID: <CAPnjgZ3hzk6uLOgaEZSk=M=quAKSZ6FVOvfuyELSe8-0O_ndKA@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, 8 Mar 2019 at 13:11, 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>
> ---
>  include/fdtdec.h | 11 +++++++++++
>  lib/fdtdec.c     | 16 ++++++++++++++++
>  2 files changed, 27 insertions(+)

This seems OK, although I think it should have a test.

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

>
> 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?

> +
> +       err = fdt_setprop(blob, node, "phandle", &value, sizeof(value));
> +       if (err < 0)
> +               return err;
> +
> +       return 0;
> +}
> +
>  int fdtdec_setup(void)
>  {
>  #if CONFIG_IS_ENABLED(OF_CONTROL)
> --
> 2.20.1
>

Regards,
SImon