[U-Boot] [PATCH v2 8/9] dm: core: Add ofnode to represent device tree nodes

[Masahiro Yamada <yamada.masahiro@socionext.com>]

Hi Simon,


2017-05-02 0:18 GMT+09:00 Simon Glass <sjg@chromium.org>:
> With live tree we need a struct device_node * to reference a node. With
> the existing flat tree, we need an int offset. We need to unify these into
> a single value which can represent both.
>
> Add an ofnode union for this and adjust existing code to move to this.
>
> Signed-off-by: Simon Glass <sjg@chromium.org>
> ---
>
> Changes in v2: None
>
>  drivers/core/device.c |   2 +-
>  drivers/core/root.c   |   2 +-
>  include/dm.h          |   1 +
>  include/dm/device.h   |  14 +++++--
>  include/dm/ofnode.h   | 100 ++++++++++++++++++++++++++++++++++++++++++++++++++
>  5 files changed, 113 insertions(+), 6 deletions(-)
>  create mode 100644 include/dm/ofnode.h
>
> diff --git a/drivers/core/device.c b/drivers/core/device.c
> index 483f8368f7..2738685092 100644
> --- a/drivers/core/device.c
> +++ b/drivers/core/device.c
> @@ -60,7 +60,7 @@ static int device_bind_common(struct udevice *parent, const struct driver *drv,
>         dev->platdata = platdata;
>         dev->driver_data = driver_data;
>         dev->name = name;
> -       dev->of_offset = of_offset;
> +       dev->node = offset_to_ofnode(of_offset);
>         dev->parent = parent;
>         dev->driver = drv;
>         dev->uclass = uc;
> diff --git a/drivers/core/root.c b/drivers/core/root.c
> index 4bb71f3cac..570b4d855f 100644
> --- a/drivers/core/root.c
> +++ b/drivers/core/root.c
> @@ -167,7 +167,7 @@ int dm_init(void)
>         if (ret)
>                 return ret;
>  #if CONFIG_IS_ENABLED(OF_CONTROL)
> -       DM_ROOT_NON_CONST->of_offset = 0;
> +       DM_ROOT_NON_CONST->node = offset_to_ofnode(0);
>  #endif
>         ret = device_probe(DM_ROOT_NON_CONST);
>         if (ret)
> diff --git a/include/dm.h b/include/dm.h
> index 84f789d807..e634814d74 100644
> --- a/include/dm.h
> +++ b/include/dm.h
> @@ -7,6 +7,7 @@
>  #ifndef _DM_H_
>  #define _DM_H_
>
> +#include <dm/ofnode.h>
>  #include <dm/device.h>
>  #include <dm/fdtaddr.h>
>  #include <dm/platdata.h>
> diff --git a/include/dm/device.h b/include/dm/device.h
> index 6c4aab6c96..89f9f53c43 100644
> --- a/include/dm/device.h
> +++ b/include/dm/device.h
> @@ -11,6 +11,7 @@
>  #ifndef _DM_DEVICE_H
>  #define _DM_DEVICE_H
>
> +#include <dm/ofnode.h>
>  #include <dm/uclass-id.h>
>  #include <fdtdec.h>
>  #include <linker_lists.h>
> @@ -94,7 +95,7 @@ enum {
>   * @platdata: Configuration data for this device
>   * @parent_platdata: The parent bus's configuration data for this device
>   * @uclass_platdata: The uclass's configuration data for this device
> - * @of_offset: Device tree node offset for this device (- for none)
> + * @node: Reference to device tree node for this device
>   * @driver_data: Driver data word for the entry that matched this device with
>   *             its driver
>   * @parent: Parent of this device, or NULL for the top level device
> @@ -120,7 +121,7 @@ struct udevice {
>         void *platdata;
>         void *parent_platdata;
>         void *uclass_platdata;
> -       int of_offset;
> +       ofnode node;
>         ulong driver_data;
>         struct udevice *parent;
>         void *priv;
> @@ -149,12 +150,17 @@ struct udevice {
>
>  static inline int dev_of_offset(const struct udevice *dev)
>  {
> -       return dev->of_offset;
> +       return ofnode_to_offset(dev->node);
>  }
>
>  static inline void dev_set_of_offset(struct udevice *dev, int of_offset)
>  {
> -       dev->of_offset = of_offset;
> +       dev->node = offset_to_ofnode(of_offset);
> +}
> +
> +static inline bool dev_has_of_node(struct udevice *dev)
> +{
> +       return ofnode_valid(dev->node);
>  }
>
>  /**
> diff --git a/include/dm/ofnode.h b/include/dm/ofnode.h
> new file mode 100644
> index 0000000000..f952c989d2
> --- /dev/null
> +++ b/include/dm/ofnode.h
> @@ -0,0 +1,100 @@
> +/*
> + * Copyright (c) 2017 Google, Inc
> + * Written by Simon Glass <sjg@chromium.org>
> + *
> + * SPDX-License-Identifier:    GPL-2.0+
> + */
> +
> +#ifndef _DM_OFNODE_H
> +#define _DM_OFNODE_H
> +
> +DECLARE_GLOBAL_DATA_PTR;
> +
> +struct fdtdec_phandle_args;
> +
> +/**
> + * ofnode - reference to a device tree node
> + *
> + * This union can hold either a straightforward pointer to a struct device_node
> + * in the live device tree, or an offset within the flat device tree. In the
> + * latter case, the pointer value is just the integer offset within the flat DT.
> + *
> + * Thus we can reference nodes in both the live tree (once available) and the
> + * flat tree (until then). Functions are available to translate between an
> + * ofnode and either an offset or a struct device_node *.
> + *
> + * The reference can also hold a null offset, in which case the pointer value
> + * here is (void *)-1. This corresponds to a struct device_node * value of
> + * NULL, or an offset of -1.
> + *
> + * There is no ambiguity as to whether ofnode holds an offset or a node
> + * pointer: when the live tree is active it holds a node pointer, otherwise it
> + * holds an offset. The value itself does not need to be unique and in theory
> + * the same value could point to a valid device node or a valid offset. We
> + * could arrange for a unique value to be used (e.g. by making the pointer
> + * point to an offset within the flat device tree in the case of an offset) but
> + * this increases code size slightly due to the subtraction. Since it offers no
> + * real benefit, the approach described here seems best.
> + *
> + * For now these points use constant types, since we don't allow writing
> + * the DT.
> + *
> + * @np: Pointer to device node, used for live tree
> + * @flat_ptr: Pointer into flat device tree, used for flat tree. Note that this
> + *     is not a really a pointer to a node: it is an offset value. See above.
> + */
> +typedef union ofnode_union {
> +       const struct device_node *np;   /* will be used for future live tree */
> +       long of_offset;
> +} ofnode;
> +
> +/**
> + * ofnode_to_offset() - convert an ofnode to a flat DT offset
> + *
> + * This cannot be called if the reference contains a node pointer.
> + *
> + * @node: Reference containing offset (possibly invalid)
> + * @return DT offset (can be -1)
> + */
> +static inline int ofnode_to_offset(ofnode node)
> +{
> +       return node.of_offset;
> +}
> +
> +/**
> + * ofnode_valid() - check if an ofnode is valid
> + *
> + * @return true if the reference contains a valid ofnode, false if it is NULL
> + */
> +static inline bool ofnode_valid(ofnode node)
> +{
> +       return node.of_offset != -1;
> +}
> +
> +/**
> + * offset_to_ofnode() - convert a DT offset to an ofnode
> + *
> + * @of_offset: DT offset (either valid, or -1)
> + * @return reference to the associated DT offset
> + */
> +static inline ofnode offset_to_ofnode(int of_offset)
> +{
> +       ofnode node;
> +
> +       node.of_offset = of_offset;
> +
> +       return node;
> +}
> +
> +/**
> + * ofnode_equal() - check if two references are equal
> + *
> + * @return true if equal, else false
> + */
> +static inline bool ofnode_equal(ofnode ref1, ofnode ref2)
> +{
> +       /* We only need to compare the contents */
> +       return ref1.of_offset == ref2.of_offset;
> +}
> +
> +#endif

When you add a new header, please make sure it is self-contained.

You use bool for offset_toofnode() and ofnode_equal().
So you need to include <stdbool.h> from this header.

(or, use "int" for the return type.)



-- 
Best Regards
Masahiro Yamada