Re: [U-Boot] [PATCH v3 10/12] libfdt: Add overlay application function

2016-06-29 Thread David Gibson
On Wed, Jun 29, 2016 at 07:34:54PM -0700, Frank Rowand wrote:
> On 06/27/16 20:12, David Gibson wrote:
> > On Mon, Jun 27, 2016 at 01:40:00PM +0200, Maxime Ripard wrote:
> >> Hi David,
> >>
> >> On Mon, Jun 27, 2016 at 03:26:07PM +1000, David Gibson wrote:
>  +static uint32_t overlay_get_target_phandle(const void *fdto, int 
>  fragment)
>  +{
>  +const uint32_t *val;
>  +int len;
>  +
>  +val = fdt_getprop(fdto, fragment, "target", &len);
>  +if (!val || (*val == 0x) || (len != sizeof(*val)))
>  +return 0;
> >>>
> >>> This doesn't distinguish between a missing property (which may
> >>> indicate a valid overlay using a target-path or some other method)
> >>> and a badly formatted 'target' property, which is definitely an error
> >>> in the overlay.
> >>>
> >>> I think those should be treated differently.
> >>
> >> AFAIK, phandles can have any 32 bits values but 0x. In order
> >> to cover the two cases, we would need to have some error code, but
> >> that doesn't really work with returning a uint32_t.
> > 
> > Actually phandles can have any value except 0x *or* 0.  So you
> > can use 0 for "couldn't find" and -1 for "badly formatted".
> 
> < snip >
> 
> Hi David,
> 
> I would like to capture this for the specification.
> 
> It seems like I could say that a value of 0 in the FDT is not allowed.
> 
> Then thinking of what Pantelis is doing with overlays, it seems like a
> value of 0x is allowed in the FDT, but it means not a valid
> phandle, so do not try to de-reference it.
> 
> Does that sound good?

That should be ok.  Basically both 0 and -1 are invalid phandle
values, so it's up to us if we want to assign them specific "error"
meanings.

-- 
David Gibson| I'll have my music baroque, and my code
david AT gibson.dropbear.id.au  | minimalist, thank you.  NOT _the_ _other_
| _way_ _around_!
http://www.ozlabs.org/~dgibson


signature.asc
Description: PGP signature
___
U-Boot mailing list
U-Boot@lists.denx.de
http://lists.denx.de/mailman/listinfo/u-boot


Re: [U-Boot] [PATCH v3 10/12] libfdt: Add overlay application function

2016-06-29 Thread Frank Rowand
On 06/27/16 20:12, David Gibson wrote:
> On Mon, Jun 27, 2016 at 01:40:00PM +0200, Maxime Ripard wrote:
>> Hi David,
>>
>> On Mon, Jun 27, 2016 at 03:26:07PM +1000, David Gibson wrote:
 +static uint32_t overlay_get_target_phandle(const void *fdto, int fragment)
 +{
 +  const uint32_t *val;
 +  int len;
 +
 +  val = fdt_getprop(fdto, fragment, "target", &len);
 +  if (!val || (*val == 0x) || (len != sizeof(*val)))
 +  return 0;
>>>
>>> This doesn't distinguish between a missing property (which may
>>> indicate a valid overlay using a target-path or some other method)
>>> and a badly formatted 'target' property, which is definitely an error
>>> in the overlay.
>>>
>>> I think those should be treated differently.
>>
>> AFAIK, phandles can have any 32 bits values but 0x. In order
>> to cover the two cases, we would need to have some error code, but
>> that doesn't really work with returning a uint32_t.
> 
> Actually phandles can have any value except 0x *or* 0.  So you
> can use 0 for "couldn't find" and -1 for "badly formatted".

< snip >

Hi David,

I would like to capture this for the specification.

It seems like I could say that a value of 0 in the FDT is not allowed.

Then thinking of what Pantelis is doing with overlays, it seems like a
value of 0x is allowed in the FDT, but it means not a valid
phandle, so do not try to de-reference it.

Does that sound good?

-Frank

___
U-Boot mailing list
U-Boot@lists.denx.de
http://lists.denx.de/mailman/listinfo/u-boot


Re: [U-Boot] [PATCH v3 10/12] libfdt: Add overlay application function

2016-06-27 Thread David Gibson
On Mon, Jun 27, 2016 at 01:40:00PM +0200, Maxime Ripard wrote:
> Hi David,
> 
> On Mon, Jun 27, 2016 at 03:26:07PM +1000, David Gibson wrote:
> > > +static uint32_t overlay_get_target_phandle(const void *fdto, int 
> > > fragment)
> > > +{
> > > + const uint32_t *val;
> > > + int len;
> > > +
> > > + val = fdt_getprop(fdto, fragment, "target", &len);
> > > + if (!val || (*val == 0x) || (len != sizeof(*val)))
> > > + return 0;
> > 
> > This doesn't distinguish between a missing property (which may
> > indicate a valid overlay using a target-path or some other method)
> > and a badly formatted 'target' property, which is definitely an error
> > in the overlay.
> > 
> > I think those should be treated differently.
> 
> AFAIK, phandles can have any 32 bits values but 0x. In order
> to cover the two cases, we would need to have some error code, but
> that doesn't really work with returning a uint32_t.

Actually phandles can have any value except 0x *or* 0.  So you
can use 0 for "couldn't find" and -1 for "badly formatted".

> Or maybe we can simply remove all the checks but the missing property,
> and let fdt_node_offset_by_phandle deal with the improper values?
> 
> > 
> > > + return fdt32_to_cpu(*val);
> > > +}
> > > +
> > > +static int overlay_get_target(const void *fdt, const void *fdto,
> > > +   int fragment)
> > > +{
> > > + uint32_t phandle;
> > > + const char *path;
> > > +
> > > + /* Try first to do a phandle based lookup */
> > > + phandle = overlay_get_target_phandle(fdto, fragment);
> > > + if (phandle)
> > > + return fdt_node_offset_by_phandle(fdt, phandle);
> > > +
> > > + /* And then a path based lookup */
> > > + path = fdt_getprop(fdto, fragment, "target-path", NULL);
> > > + if (!path)
> > > + return -FDT_ERR_NOTFOUND;
> > > +
> > > + return fdt_path_offset(fdt, path);
> > > +}
> > > +
> > > +static int overlay_phandle_add_offset(void *fdt, int node,
> > > +   const char *name, uint32_t delta)
> > > +{
> > > + const uint32_t *val;
> > > + uint32_t adj_val;
> > > + int len;
> > > +
> > > + val = fdt_getprop(fdt, node, name, &len);
> > > + if (!val)
> > > + return len;
> > > +
> > > + if (len != sizeof(*val))
> > > + return -FDT_ERR_BADSTRUCTURE;
> > > +
> > > + adj_val = fdt32_to_cpu(*val);
> > > + adj_val += delta;
> > 
> > You should probably check for overflow here.
> > 
> > > +
> > > + return fdt_setprop_inplace_u32(fdt, node, name, adj_val);
> > > +}
> > > +
> > > +static int overlay_adjust_node_phandles(void *fdto, int node,
> > > + uint32_t delta)
> > > +{
> > > + bool found = false;
> > > + int child;
> > > + int ret;
> > > +
> > > + ret = overlay_phandle_add_offset(fdto, node, "phandle", delta);
> > > + if (ret && ret != -FDT_ERR_NOTFOUND)
> > > + return ret;
> > > +
> > > + if (!ret)
> > > + found = true;
> > > +
> > > + ret = overlay_phandle_add_offset(fdto, node, "linux,phandle", delta);
> > > + if (ret && ret != -FDT_ERR_NOTFOUND)
> > > + return ret;
> > 
> > I think the check for phandle vs. linux,phandle should be folded into
> > overlay_phandle_add_offset().
> 
> I created overlay_phandle_add_offset to avoid duplicating the getprop,
> offset, setprop, pattern which I don't think is a good idea.
> 
> And we'll have to have that kind of errors construct anyway to know if
> we modified any of the two, which is a success, or none, which is a
> failure.

Hm.. ok, you convinced me.

> > > + /*
> > > +  * If neither phandle nor linux,phandle have been found return
> > > +  * an error.
> > > +  */
> > > + if (!found && !ret)
> > > + return ret;
> > > +
> > > + fdt_for_each_subnode(fdto, child, node)
> > > + overlay_adjust_node_phandles(fdto, child, delta);
> > > +
> > > + return 0;
> > > +}
> > > +
> > > +static int overlay_adjust_local_phandles(void *fdto, uint32_t delta)
> > > +{
> > > + /*
> > > +  * Start adjusting the phandles from the overlay root
> > > +  */
> > > + return overlay_adjust_node_phandles(fdto, 0, delta);
> > > +}
> > > +
> > > +static int overlay_update_local_node_references(void *fdto,
> > > + int tree_node,
> > > + int fixup_node,
> > > + uint32_t delta)
> > > +{
> > > + int fixup_prop;
> > > + int fixup_child;
> > > + int ret;
> > > +
> > > + fdt_for_each_property_offset(fixup_prop, fdto, fixup_node) {
> > > + const uint32_t *val = NULL;
> > > + uint32_t adj_val, index;
> > > + const char *name;
> > > + int fixup_len;
> > > + int tree_len;
> > > +
> > > + val = fdt_getprop_by_offset(fdto, fixup_prop,
> > > + &name, &fixup_len);
> > > + if (!val)
> > > + return fixup_len;
> > > + index = fdt32_to_cpu(*val);
> > 
> > This still doesn't handle the case 

Re: [U-Boot] [PATCH v3 10/12] libfdt: Add overlay application function

2016-06-27 Thread Maxime Ripard
Hi David,

On Mon, Jun 27, 2016 at 03:26:07PM +1000, David Gibson wrote:
> > +static uint32_t overlay_get_target_phandle(const void *fdto, int fragment)
> > +{
> > +   const uint32_t *val;
> > +   int len;
> > +
> > +   val = fdt_getprop(fdto, fragment, "target", &len);
> > +   if (!val || (*val == 0x) || (len != sizeof(*val)))
> > +   return 0;
> 
> This doesn't distinguish between a missing property (which may
> indicate a valid overlay using a target-path or some other method)
> and a badly formatted 'target' property, which is definitely an error
> in the overlay.
> 
> I think those should be treated differently.

AFAIK, phandles can have any 32 bits values but 0x. In order
to cover the two cases, we would need to have some error code, but
that doesn't really work with returning a uint32_t.

Or maybe we can simply remove all the checks but the missing property,
and let fdt_node_offset_by_phandle deal with the improper values?

> 
> > +   return fdt32_to_cpu(*val);
> > +}
> > +
> > +static int overlay_get_target(const void *fdt, const void *fdto,
> > + int fragment)
> > +{
> > +   uint32_t phandle;
> > +   const char *path;
> > +
> > +   /* Try first to do a phandle based lookup */
> > +   phandle = overlay_get_target_phandle(fdto, fragment);
> > +   if (phandle)
> > +   return fdt_node_offset_by_phandle(fdt, phandle);
> > +
> > +   /* And then a path based lookup */
> > +   path = fdt_getprop(fdto, fragment, "target-path", NULL);
> > +   if (!path)
> > +   return -FDT_ERR_NOTFOUND;
> > +
> > +   return fdt_path_offset(fdt, path);
> > +}
> > +
> > +static int overlay_phandle_add_offset(void *fdt, int node,
> > + const char *name, uint32_t delta)
> > +{
> > +   const uint32_t *val;
> > +   uint32_t adj_val;
> > +   int len;
> > +
> > +   val = fdt_getprop(fdt, node, name, &len);
> > +   if (!val)
> > +   return len;
> > +
> > +   if (len != sizeof(*val))
> > +   return -FDT_ERR_BADSTRUCTURE;
> > +
> > +   adj_val = fdt32_to_cpu(*val);
> > +   adj_val += delta;
> 
> You should probably check for overflow here.
> 
> > +
> > +   return fdt_setprop_inplace_u32(fdt, node, name, adj_val);
> > +}
> > +
> > +static int overlay_adjust_node_phandles(void *fdto, int node,
> > +   uint32_t delta)
> > +{
> > +   bool found = false;
> > +   int child;
> > +   int ret;
> > +
> > +   ret = overlay_phandle_add_offset(fdto, node, "phandle", delta);
> > +   if (ret && ret != -FDT_ERR_NOTFOUND)
> > +   return ret;
> > +
> > +   if (!ret)
> > +   found = true;
> > +
> > +   ret = overlay_phandle_add_offset(fdto, node, "linux,phandle", delta);
> > +   if (ret && ret != -FDT_ERR_NOTFOUND)
> > +   return ret;
> 
> I think the check for phandle vs. linux,phandle should be folded into
> overlay_phandle_add_offset().

I created overlay_phandle_add_offset to avoid duplicating the getprop,
offset, setprop, pattern which I don't think is a good idea.

And we'll have to have that kind of errors construct anyway to know if
we modified any of the two, which is a success, or none, which is a
failure.

> > +   /*
> > +* If neither phandle nor linux,phandle have been found return
> > +* an error.
> > +*/
> > +   if (!found && !ret)
> > +   return ret;
> > +
> > +   fdt_for_each_subnode(fdto, child, node)
> > +   overlay_adjust_node_phandles(fdto, child, delta);
> > +
> > +   return 0;
> > +}
> > +
> > +static int overlay_adjust_local_phandles(void *fdto, uint32_t delta)
> > +{
> > +   /*
> > +* Start adjusting the phandles from the overlay root
> > +*/
> > +   return overlay_adjust_node_phandles(fdto, 0, delta);
> > +}
> > +
> > +static int overlay_update_local_node_references(void *fdto,
> > +   int tree_node,
> > +   int fixup_node,
> > +   uint32_t delta)
> > +{
> > +   int fixup_prop;
> > +   int fixup_child;
> > +   int ret;
> > +
> > +   fdt_for_each_property_offset(fixup_prop, fdto, fixup_node) {
> > +   const uint32_t *val = NULL;
> > +   uint32_t adj_val, index;
> > +   const char *name;
> > +   int fixup_len;
> > +   int tree_len;
> > +
> > +   val = fdt_getprop_by_offset(fdto, fixup_prop,
> > +   &name, &fixup_len);
> > +   if (!val)
> > +   return fixup_len;
> > +   index = fdt32_to_cpu(*val);
> 
> This still doesn't handle the case of multiple fixups within the same
> property.  That would be entirely plausible for something like an
> interrupt-map property.

Good point, I'll fix that and add a test case for it.

> > +   val = fdt_getprop(fdto, tree_node, name, &tree_len);
> > +   if (!val)
> > +   return tree_len;
> > +
> > +   adj_val = fdt32_to_cpu(*(va

Re: [U-Boot] [PATCH v3 10/12] libfdt: Add overlay application function

2016-06-27 Thread Maxime Ripard
Hi Simon,

On Sat, Jun 25, 2016 at 08:53:58PM -0600, Simon Glass wrote:
> Hi Maxime,
> 
> On 24 June 2016 at 08:27, Maxime Ripard
>  wrote:
> > The device tree overlays are a good way to deal with user-modifyable
> > boards or boards with some kind of an expansion mechanism where we can
> > easily plug new board in (like the BBB, the Raspberry Pi or the CHIP).
> >
> > Add a new function to merge overlays with a base device tree.
> >
> > Signed-off-by: Maxime Ripard 
> > ---
> >  include/libfdt.h |  30 
> >  lib/Kconfig  |   5 +
> >  lib/libfdt/Makefile  |   2 +
> >  lib/libfdt/fdt_overlay.c | 381 
> > +++
> >  4 files changed, 418 insertions(+)
> >  create mode 100644 lib/libfdt/fdt_overlay.c
> >
> 
> Is there a change log?

Yes, in the cover letter.

> > +static int overlay_update_local_node_references(void *fdto,
> > +   int tree_node,
> > +   int fixup_node,
> > +   uint32_t delta)
> > +{
> 
> Please add function comments for all of these.

Ack

> > +static int overlay_update_local_references(void *dto, uint32_t delta)
> > +{
> > +   int fixups;
> > +
> > +   fixups = fdt_path_offset(dto, "/__local_fixups__");
> > +   if (fixups < 0) {
> > +   /*
> > +* There's no local phandles to adjust, bail out
> 
> On on line:
> 
> /* There's not local ... */

Ok.

Thanks!
Maxime

-- 
Maxime Ripard, Free Electrons
Embedded Linux, Kernel and Android engineering
http://free-electrons.com


signature.asc
Description: PGP signature
___
U-Boot mailing list
U-Boot@lists.denx.de
http://lists.denx.de/mailman/listinfo/u-boot


Re: [U-Boot] [PATCH v3 10/12] libfdt: Add overlay application function

2016-06-26 Thread David Gibson
On Fri, Jun 24, 2016 at 04:27:55PM +0200, Maxime Ripard wrote:
> The device tree overlays are a good way to deal with user-modifyable
> boards or boards with some kind of an expansion mechanism where we can
> easily plug new board in (like the BBB, the Raspberry Pi or the CHIP).
> 
> Add a new function to merge overlays with a base device tree.
> 
> Signed-off-by: Maxime Ripard 
> ---
>  include/libfdt.h |  30 
>  lib/Kconfig  |   5 +
>  lib/libfdt/Makefile  |   2 +
>  lib/libfdt/fdt_overlay.c | 381 
> +++
>  4 files changed, 418 insertions(+)
>  create mode 100644 lib/libfdt/fdt_overlay.c
> 
> diff --git a/include/libfdt.h b/include/libfdt.h
> index b8758de3ae54..039ecb0c1ba7 100644
> --- a/include/libfdt.h
> +++ b/include/libfdt.h
> @@ -1747,6 +1747,36 @@ int fdt_add_subnode(void *fdt, int parentoffset, const 
> char *name);
>   */
>  int fdt_del_node(void *fdt, int nodeoffset);
>  
> +/**
> + * fdt_overlay_apply - Applies a DT overlay on a base DT
> + * @fdt: pointer to the base device tree blob
> + * @fdto: pointer to the device tree overlay blob
> + *
> + * fdt_overlay_apply() will apply the given device tree overlay on the
> + * given base device tree.
> + *
> + * Expect the base device tree to be modified, even if the function
> + * returns an error.
> + *
> + * returns:
> + *   0, on success
> + *   -FDT_ERR_NOSPACE, there's not enough space in the base device tree
> + *   -FDT_ERR_NOTFOUND, the overlay points to some inexistant nodes or
> + *   properties in the base DT
> + *   -FDT_ERR_BADPHANDLE, the phandles in the overlay do not have the right
> + *   magic
> + *   -FDT_ERR_INTERNAL,
> + *   -FDT_ERR_BADLAYOUT,
> + *   -FDT_ERR_BADMAGIC,
> + *   -FDT_ERR_BADOFFSET,
> + *   -FDT_ERR_BADPATH,
> + *   -FDT_ERR_BADVERSION,
> + *   -FDT_ERR_BADSTRUCTURE,
> + *   -FDT_ERR_BADSTATE,
> + *   -FDT_ERR_TRUNCATED, standard meanings
> + */
> +int fdt_overlay_apply(void *fdt, void *fdto);
> +
>  /**/
>  /* Debugging / informational functions*/
>  /**/
> diff --git a/lib/Kconfig b/lib/Kconfig
> index 02ca4058d37e..16ff01a2cd37 100644
> --- a/lib/Kconfig
> +++ b/lib/Kconfig
> @@ -139,6 +139,11 @@ config OF_LIBFDT
> particular compatible nodes. The library operates on a flattened
> version of the device tree.
>  
> +config OF_LIBFDT_OVERLAY
> + bool "Enable the FDT library overlay support"
> + help
> +   This enables the FDT library (libfdt) overlay support.
> +
>  config SPL_OF_LIBFDT
>   bool "Enable the FDT library for SPL"
>   default y if SPL_OF_CONTROL
> diff --git a/lib/libfdt/Makefile b/lib/libfdt/Makefile
> index 8b86c15904cb..6ef8290f4e09 100644
> --- a/lib/libfdt/Makefile
> +++ b/lib/libfdt/Makefile
> @@ -15,3 +15,5 @@ obj-y += \
>   fdt_empty_tree.o \
>   fdt_addresses.o \
>   fdt_region.o
> +
> +obj-$(CONFIG_OF_LIBFDT_OVERLAY) += fdt_overlay.o
> diff --git a/lib/libfdt/fdt_overlay.c b/lib/libfdt/fdt_overlay.c
> new file mode 100644
> index ..5835a90afafe
> --- /dev/null
> +++ b/lib/libfdt/fdt_overlay.c
> @@ -0,0 +1,381 @@
> +#include "libfdt_env.h"
> +
> +#include 
> +#include 
> +
> +#include "libfdt_internal.h"
> +
> +static uint32_t overlay_get_target_phandle(const void *fdto, int fragment)
> +{
> + const uint32_t *val;
> + int len;
> +
> + val = fdt_getprop(fdto, fragment, "target", &len);
> + if (!val || (*val == 0x) || (len != sizeof(*val)))
> + return 0;

This doesn't distinguish between a missing property (which may
indicate a valid overlay using a target-path or some other method)
and a badly formatted 'target' property, which is definitely an error
in the overlay.

I think those should be treated differently.

> + return fdt32_to_cpu(*val);
> +}
> +
> +static int overlay_get_target(const void *fdt, const void *fdto,
> +   int fragment)
> +{
> + uint32_t phandle;
> + const char *path;
> +
> + /* Try first to do a phandle based lookup */
> + phandle = overlay_get_target_phandle(fdto, fragment);
> + if (phandle)
> + return fdt_node_offset_by_phandle(fdt, phandle);
> +
> + /* And then a path based lookup */
> + path = fdt_getprop(fdto, fragment, "target-path", NULL);
> + if (!path)
> + return -FDT_ERR_NOTFOUND;
> +
> + return fdt_path_offset(fdt, path);
> +}
> +
> +static int overlay_phandle_add_offset(void *fdt, int node,
> +   const char *name, uint32_t delta)
> +{
> + const uint32_t *val;
> + uint32_t adj_val;
> + int len;
> +
> + val = fdt_getprop(fdt, node, name, &len);
> + if (!val)
> + return len;
> +
> + if (len != sizeof(*val))
> + return -FDT_ERR_BADSTRUCTURE;
> +
> +   

Re: [U-Boot] [PATCH v3 10/12] libfdt: Add overlay application function

2016-06-25 Thread Simon Glass
Hi Maxime,

On 24 June 2016 at 08:27, Maxime Ripard
 wrote:
> The device tree overlays are a good way to deal with user-modifyable
> boards or boards with some kind of an expansion mechanism where we can
> easily plug new board in (like the BBB, the Raspberry Pi or the CHIP).
>
> Add a new function to merge overlays with a base device tree.
>
> Signed-off-by: Maxime Ripard 
> ---
>  include/libfdt.h |  30 
>  lib/Kconfig  |   5 +
>  lib/libfdt/Makefile  |   2 +
>  lib/libfdt/fdt_overlay.c | 381 
> +++
>  4 files changed, 418 insertions(+)
>  create mode 100644 lib/libfdt/fdt_overlay.c
>

Is there a change log?

> diff --git a/include/libfdt.h b/include/libfdt.h
> index b8758de3ae54..039ecb0c1ba7 100644
> --- a/include/libfdt.h
> +++ b/include/libfdt.h
> @@ -1747,6 +1747,36 @@ int fdt_add_subnode(void *fdt, int parentoffset, const 
> char *name);
>   */
>  int fdt_del_node(void *fdt, int nodeoffset);
>
> +/**
> + * fdt_overlay_apply - Applies a DT overlay on a base DT
> + * @fdt: pointer to the base device tree blob
> + * @fdto: pointer to the device tree overlay blob
> + *
> + * fdt_overlay_apply() will apply the given device tree overlay on the
> + * given base device tree.
> + *
> + * Expect the base device tree to be modified, even if the function
> + * returns an error.
> + *
> + * returns:
> + * 0, on success
> + * -FDT_ERR_NOSPACE, there's not enough space in the base device tree
> + * -FDT_ERR_NOTFOUND, the overlay points to some inexistant nodes or
> + * properties in the base DT
> + * -FDT_ERR_BADPHANDLE, the phandles in the overlay do not have the right
> + * magic
> + * -FDT_ERR_INTERNAL,
> + * -FDT_ERR_BADLAYOUT,
> + * -FDT_ERR_BADMAGIC,
> + * -FDT_ERR_BADOFFSET,
> + * -FDT_ERR_BADPATH,
> + * -FDT_ERR_BADVERSION,
> + * -FDT_ERR_BADSTRUCTURE,
> + * -FDT_ERR_BADSTATE,
> + * -FDT_ERR_TRUNCATED, standard meanings
> + */
> +int fdt_overlay_apply(void *fdt, void *fdto);
> +
>  /**/
>  /* Debugging / informational functions*/
>  /**/
> diff --git a/lib/Kconfig b/lib/Kconfig
> index 02ca4058d37e..16ff01a2cd37 100644
> --- a/lib/Kconfig
> +++ b/lib/Kconfig
> @@ -139,6 +139,11 @@ config OF_LIBFDT
>   particular compatible nodes. The library operates on a flattened
>   version of the device tree.
>
> +config OF_LIBFDT_OVERLAY
> +   bool "Enable the FDT library overlay support"
> +   help
> + This enables the FDT library (libfdt) overlay support.
> +
>  config SPL_OF_LIBFDT
> bool "Enable the FDT library for SPL"
> default y if SPL_OF_CONTROL
> diff --git a/lib/libfdt/Makefile b/lib/libfdt/Makefile
> index 8b86c15904cb..6ef8290f4e09 100644
> --- a/lib/libfdt/Makefile
> +++ b/lib/libfdt/Makefile
> @@ -15,3 +15,5 @@ obj-y += \
> fdt_empty_tree.o \
> fdt_addresses.o \
> fdt_region.o
> +
> +obj-$(CONFIG_OF_LIBFDT_OVERLAY) += fdt_overlay.o
> diff --git a/lib/libfdt/fdt_overlay.c b/lib/libfdt/fdt_overlay.c
> new file mode 100644
> index ..5835a90afafe
> --- /dev/null
> +++ b/lib/libfdt/fdt_overlay.c
> @@ -0,0 +1,381 @@
> +#include "libfdt_env.h"
> +
> +#include 
> +#include 
> +
> +#include "libfdt_internal.h"
> +
> +static uint32_t overlay_get_target_phandle(const void *fdto, int fragment)
> +{
> +   const uint32_t *val;
> +   int len;
> +
> +   val = fdt_getprop(fdto, fragment, "target", &len);
> +   if (!val || (*val == 0x) || (len != sizeof(*val)))
> +   return 0;
> +
> +   return fdt32_to_cpu(*val);
> +}
> +
> +static int overlay_get_target(const void *fdt, const void *fdto,
> + int fragment)

function comment please.

> +{
> +   uint32_t phandle;
> +   const char *path;
> +
> +   /* Try first to do a phandle based lookup */
> +   phandle = overlay_get_target_phandle(fdto, fragment);
> +   if (phandle)
> +   return fdt_node_offset_by_phandle(fdt, phandle);
> +
> +   /* And then a path based lookup */
> +   path = fdt_getprop(fdto, fragment, "target-path", NULL);
> +   if (!path)
> +   return -FDT_ERR_NOTFOUND;
> +
> +   return fdt_path_offset(fdt, path);
> +}
> +
> +static int overlay_phandle_add_offset(void *fdt, int node,
> + const char *name, uint32_t delta)

function comment please.

> +{
> +   const uint32_t *val;
> +   uint32_t adj_val;
> +   int len;
> +
> +   val = fdt_getprop(fdt, node, name, &len);
> +   if (!val)
> +   return len;
> +
> +   if (len != sizeof(*val))
> +   return -FDT_ERR_BADSTRUCTURE;
> +
> +   adj_val = fdt32_to_cpu(*val);
> +   adj_val += delta;
> +
> +   return fdt_

[U-Boot] [PATCH v3 10/12] libfdt: Add overlay application function

2016-06-24 Thread Maxime Ripard
The device tree overlays are a good way to deal with user-modifyable
boards or boards with some kind of an expansion mechanism where we can
easily plug new board in (like the BBB, the Raspberry Pi or the CHIP).

Add a new function to merge overlays with a base device tree.

Signed-off-by: Maxime Ripard 
---
 include/libfdt.h |  30 
 lib/Kconfig  |   5 +
 lib/libfdt/Makefile  |   2 +
 lib/libfdt/fdt_overlay.c | 381 +++
 4 files changed, 418 insertions(+)
 create mode 100644 lib/libfdt/fdt_overlay.c

diff --git a/include/libfdt.h b/include/libfdt.h
index b8758de3ae54..039ecb0c1ba7 100644
--- a/include/libfdt.h
+++ b/include/libfdt.h
@@ -1747,6 +1747,36 @@ int fdt_add_subnode(void *fdt, int parentoffset, const 
char *name);
  */
 int fdt_del_node(void *fdt, int nodeoffset);
 
+/**
+ * fdt_overlay_apply - Applies a DT overlay on a base DT
+ * @fdt: pointer to the base device tree blob
+ * @fdto: pointer to the device tree overlay blob
+ *
+ * fdt_overlay_apply() will apply the given device tree overlay on the
+ * given base device tree.
+ *
+ * Expect the base device tree to be modified, even if the function
+ * returns an error.
+ *
+ * returns:
+ * 0, on success
+ * -FDT_ERR_NOSPACE, there's not enough space in the base device tree
+ * -FDT_ERR_NOTFOUND, the overlay points to some inexistant nodes or
+ * properties in the base DT
+ * -FDT_ERR_BADPHANDLE, the phandles in the overlay do not have the right
+ * magic
+ * -FDT_ERR_INTERNAL,
+ * -FDT_ERR_BADLAYOUT,
+ * -FDT_ERR_BADMAGIC,
+ * -FDT_ERR_BADOFFSET,
+ * -FDT_ERR_BADPATH,
+ * -FDT_ERR_BADVERSION,
+ * -FDT_ERR_BADSTRUCTURE,
+ * -FDT_ERR_BADSTATE,
+ * -FDT_ERR_TRUNCATED, standard meanings
+ */
+int fdt_overlay_apply(void *fdt, void *fdto);
+
 /**/
 /* Debugging / informational functions*/
 /**/
diff --git a/lib/Kconfig b/lib/Kconfig
index 02ca4058d37e..16ff01a2cd37 100644
--- a/lib/Kconfig
+++ b/lib/Kconfig
@@ -139,6 +139,11 @@ config OF_LIBFDT
  particular compatible nodes. The library operates on a flattened
  version of the device tree.
 
+config OF_LIBFDT_OVERLAY
+   bool "Enable the FDT library overlay support"
+   help
+ This enables the FDT library (libfdt) overlay support.
+
 config SPL_OF_LIBFDT
bool "Enable the FDT library for SPL"
default y if SPL_OF_CONTROL
diff --git a/lib/libfdt/Makefile b/lib/libfdt/Makefile
index 8b86c15904cb..6ef8290f4e09 100644
--- a/lib/libfdt/Makefile
+++ b/lib/libfdt/Makefile
@@ -15,3 +15,5 @@ obj-y += \
fdt_empty_tree.o \
fdt_addresses.o \
fdt_region.o
+
+obj-$(CONFIG_OF_LIBFDT_OVERLAY) += fdt_overlay.o
diff --git a/lib/libfdt/fdt_overlay.c b/lib/libfdt/fdt_overlay.c
new file mode 100644
index ..5835a90afafe
--- /dev/null
+++ b/lib/libfdt/fdt_overlay.c
@@ -0,0 +1,381 @@
+#include "libfdt_env.h"
+
+#include 
+#include 
+
+#include "libfdt_internal.h"
+
+static uint32_t overlay_get_target_phandle(const void *fdto, int fragment)
+{
+   const uint32_t *val;
+   int len;
+
+   val = fdt_getprop(fdto, fragment, "target", &len);
+   if (!val || (*val == 0x) || (len != sizeof(*val)))
+   return 0;
+
+   return fdt32_to_cpu(*val);
+}
+
+static int overlay_get_target(const void *fdt, const void *fdto,
+ int fragment)
+{
+   uint32_t phandle;
+   const char *path;
+
+   /* Try first to do a phandle based lookup */
+   phandle = overlay_get_target_phandle(fdto, fragment);
+   if (phandle)
+   return fdt_node_offset_by_phandle(fdt, phandle);
+
+   /* And then a path based lookup */
+   path = fdt_getprop(fdto, fragment, "target-path", NULL);
+   if (!path)
+   return -FDT_ERR_NOTFOUND;
+
+   return fdt_path_offset(fdt, path);
+}
+
+static int overlay_phandle_add_offset(void *fdt, int node,
+ const char *name, uint32_t delta)
+{
+   const uint32_t *val;
+   uint32_t adj_val;
+   int len;
+
+   val = fdt_getprop(fdt, node, name, &len);
+   if (!val)
+   return len;
+
+   if (len != sizeof(*val))
+   return -FDT_ERR_BADSTRUCTURE;
+
+   adj_val = fdt32_to_cpu(*val);
+   adj_val += delta;
+
+   return fdt_setprop_inplace_u32(fdt, node, name, adj_val);
+}
+
+static int overlay_adjust_node_phandles(void *fdto, int node,
+   uint32_t delta)
+{
+   bool found = false;
+   int child;
+   int ret;
+
+   ret = overlay_phandle_add_offset(fdto, node, "phandle", delta);
+   if (ret && ret != -FDT_ERR_NOTFOUND)
+   return ret;
+
+   if (!ret)
+   foun