[U-Boot] [PATCH 00/22] dm: Add support for a 'live' device tree

[U-Boot] [PATCH 00/22] dm: Add support for a 'live' device tree

[Simon Glass]

So far U-Boot uses a 'flat' device tree, which means that it is decoded
on the fly as needed. This uses the libfdt library and avoids needing
extra memory for additional tables.

For some time there has been discussion about moving U-Boot to use a
'live' tree, where the device tree is decoded at start-up into a set of
hierarchical structures with pointers.

The advantages are:

- It is somewhat faster to access (in particular scanning and looking for a
    parent)
- It permits the device tree to be changed at run-time (this is not
    recommended at present since devices store the offset of their device
    tree node and updating the tree may invalidate that). This could be
    useful for overlays, for example.
- It allows nodes to be referenced by a single pointer, instead of the
    current device tree pointer plus offset

The disadvantages are:

- It requires more memory
- It takes (a little) time to build the live tree
- It adds more complexity under the hood, including an additional
    abstraction layer

This series is an attempt to introduce a useful live tree feature into
U-Boot. There are many options and trade-offs. This series is the
culmination of quite a bit of thought and experimentation.

The approach used in this series is:

- Before relocation the flat tree is used, to avoid extra memory usage
and time. In general, there is not much access before relocation since
most drivers are not started up. So there is little benefit in having a
live tree

- After relocation the live tree is built. At this point the CPU should be
running quickly and there is plenty of memory. All available devices will
be bound so the overhead of building the live tree may be outweighed by
its greater efficiency.

As a simplification, this series supports only one tree or the other. When
the live tree is active, the flat tree cannot be used. That makes it easy
to know the current state and avoids confusion over mixing offset and node
pointers.

Some drivers will need to be used both before and after relocation. This
means that they must support both the flat and the live tree. To support
this, the concept of a node 'reference' is defined. A reference can hold
either a node offset (for the flat tree) or a node pointer (for the live
tree). This allows drivers to access values from the tree regardless of
which tree is in use.

In addition, since most device tree access happens in the context of a
device (struct udevice), a new 'dev_read' layer is provided to read device
tree configuration associated with a device. This encapsulates the details
of exactly how this information is read.

I have taken the view that code compatibility with Linux is desirable. So
the of_access.c file brings in code from Linux with very little
modification. As new access methods are needed we should be able to bring
in more code and avoid writing it ourselves in U-Boot.

Conversion of drivers and subsystems to support the live tree (as well as
flat tree) is fairly easy. A patch is included to add support to the GPIO
uclass, and the cros_ec device is converted over to provide a driver
example.

Once I have comments on this series I will update it to build for all
boards, to pass current tests and to tidy up the code. I will also add
tests for the live tree and finish conversion for sandbox. So for now I am
just looking for high-level comments, rather than a detailed code review.


Simon Glass (22):
  dm: core: Set return value first in lists_bind_fdt()
  dm: core: atmel: Dont export dm_scan_fdt_node()
  Update WARN_ON() to return a value
  dm: Add livetree definitions
  dm: Add livetree access functions
  dm: Add a function to create a 'live' device tree
  dm: Build a live tree after relocation
  dm: Add support for device tree references
  dm: Add a place to put extra device-tree reading functions
  dm: core: Add device-based functions to access DT properties
  dm: core: Allow binding a device from a live tree
  dm: core: Add a method to find a driver for a livetree node
  dm: core: Scan the live tree when setting up driver model
  dm: core: Add a way to find a device by its live-tree node
  dm: core: Add a way to find a device by node reference
  dm: gpio: Refactor to prepare for live tree support
  dm: gpio: Add live tree support
  dm: gpio: Drop blank line in gpio_xlate_offs_flags() comment
  dm: gpio: sandbox: Use dm_read...() functions to access DT
  cros_ec: Fix debug() statement in ec_command_inptr()
  cros_ec: Convert to support live tree
  sandbox: Move to use live tree

 common/board_r.c                  |  12 ++
 configs/sandbox_defconfig         |   1 +
 drivers/core/Makefile             |   1 +
 drivers/core/device.c             |  27 +++-
 drivers/core/lists.c              |  80 +++++++++-
 drivers/core/of_dev.c             |  55 +++++++
 drivers/core/of_extra.c           |  37 +++++
 drivers/core/of_ref.c             | 218 +++++++++++++++++++++++++
 drivers/core/root.c               |  61 ++++++-
 drivers/core/uclass.c             |  78 +++++++++
 drivers/gpio/atmel_pio4.c         |   3 +-
 drivers/gpio/gpio-uclass.c        | 100 ++++++++----
 drivers/gpio/sandbox.c            |  10 +-
 drivers/misc/cros_ec.c            |  36 ++---
 drivers/misc/cros_ec_sandbox.c    |   2 +-
 dts/Kconfig                       |  11 ++
 include/asm-generic/global_data.h |   3 +
 include/asm-generic/gpio.h        |   7 +-
 include/cros_ec.h                 |   8 +-
 include/dm/device-internal.h      |   6 +
 include/dm/device.h               |  19 ++-
 include/dm/lists.h                |   5 +
 include/dm/of.h                   |  53 ++++++
 include/dm/of_access.h            | 164 +++++++++++++++++++
 include/dm/of_dev.h               |  68 ++++++++
 include/dm/of_extra.h             |  46 ++++++
 include/dm/of_ref.h               | 306 +++++++++++++++++++++++++++++++++++
 include/dm/root.h                 |  16 --
 include/dm/uclass-internal.h      |  22 +++
 include/dm/uclass.h               |  23 +++
 include/fdtdec.h                  |  34 ----
 include/linux/compat.h            |   8 +-
 include/livetree.h                |  24 +++
 lib/Makefile                      |   1 +
 lib/fdtdec.c                      |  32 ----
 lib/livetree.c                    | 320 +++++++++++++++++++++++++++++++++++++
 lib/of_access.c                   | 328 ++++++++++++++++++++++++++++++++++++++
 37 files changed, 2066 insertions(+), 159 deletions(-)
 create mode 100644 drivers/core/of_dev.c
 create mode 100644 drivers/core/of_extra.c
 create mode 100644 drivers/core/of_ref.c
 create mode 100644 include/dm/of.h
 create mode 100644 include/dm/of_access.h
 create mode 100644 include/dm/of_dev.h
 create mode 100644 include/dm/of_extra.h
 create mode 100644 include/dm/of_ref.h
 create mode 100644 include/livetree.h
 create mode 100644 lib/livetree.c
 create mode 100644 lib/of_access.c

-- 
2.11.0.483.g087da7b7c-goog

[U-Boot] [PATCH 01/22] dm: core: Set return value first in lists_bind_fdt()

[Simon Glass]

Adjust the order to make it clear that *devp is set to NULL by default.

Signed-off-by: Simon Glass <sjg@chromium.org>
---

 drivers/core/lists.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/drivers/core/lists.c b/drivers/core/lists.c
index 23b6ba78d3b..72c55e205f9 100644
--- a/drivers/core/lists.c
+++ b/drivers/core/lists.c
@@ -140,10 +140,10 @@ int lists_bind_fdt(struct udevice *parent, const void *blob, int offset,
 	int result = 0;
 	int ret = 0;
 
-	name = fdt_get_name(blob, offset, NULL);
-	dm_dbg("bind node %s\n", name);
 	if (devp)
 		*devp = NULL;
+	name = fdt_get_name(blob, offset, NULL);
+	dm_dbg("bind node %s\n", name);
 
 	compat_list = fdt_getprop(blob, offset, "compatible", &compat_length);
 	if (!compat_list) {
-- 
2.11.0.483.g087da7b7c-goog

[U-Boot] [PATCH 02/22] dm: core: atmel: Dont export dm_scan_fdt_node()

[Simon Glass]

This function is only used in one place. It is better to jsut use it
internally since there is a simpler replacement for use outside the
driver-model core code.

Signed-off-by: Simon Glass <sjg@chromium.org>
---

 drivers/core/root.c       | 17 +++++++++++++++--
 drivers/gpio/atmel_pio4.c |  3 +--
 include/dm/root.h         | 16 ----------------
 3 files changed, 16 insertions(+), 20 deletions(-)

diff --git a/drivers/core/root.c b/drivers/core/root.c
index 175fd3fb252..cd09c55d9d2 100644
--- a/drivers/core/root.c
+++ b/drivers/core/root.c
@@ -196,8 +196,21 @@ int dm_scan_platdata(bool pre_reloc_only)
 }
 
 #if CONFIG_IS_ENABLED(OF_CONTROL) && !CONFIG_IS_ENABLED(OF_PLATDATA)
-int dm_scan_fdt_node(struct udevice *parent, const void *blob, int offset,
-		     bool pre_reloc_only)
+/**
+ * dm_scan_fdt_node() - Scan the device tree and bind drivers for a node
+ *
+ * This scans the subnodes of a device tree node and and creates a driver
+ * for each one.
+ *
+ * @parent: Parent device for the devices that will be created
+ * @blob: Pointer to device tree blob
+ * @offset: Offset of node to scan
+ * @pre_reloc_only: If true, bind only drivers with the DM_FLAG_PRE_RELOC
+ * flag. If false bind all drivers.
+ * @return 0 if OK, -ve on error
+ */
+static int dm_scan_fdt_node(struct udevice *parent, const void *blob,
+			    int offset, bool pre_reloc_only)
 {
 	int ret = 0, err;
 
diff --git a/drivers/gpio/atmel_pio4.c b/drivers/gpio/atmel_pio4.c
index 81c30475514..1409b575064 100644
--- a/drivers/gpio/atmel_pio4.c
+++ b/drivers/gpio/atmel_pio4.c
@@ -10,7 +10,6 @@
 #include <clk.h>
 #include <dm.h>
 #include <fdtdec.h>
-#include <dm/root.h>
 #include <asm/arch/hardware.h>
 #include <asm/gpio.h>
 #include <mach/gpio.h>
@@ -276,7 +275,7 @@ static const struct dm_gpio_ops atmel_pio4_ops = {
 
 static int atmel_pio4_bind(struct udevice *dev)
 {
-	return dm_scan_fdt_node(dev, gd->fdt_blob, dev_of_offset(dev), false);
+	return dm_scan_fdt_dev(dev);
 }
 
 static int atmel_pio4_probe(struct udevice *dev)
diff --git a/include/dm/root.h b/include/dm/root.h
index 3cf730dcee1..9f8904ff68c 100644
--- a/include/dm/root.h
+++ b/include/dm/root.h
@@ -56,22 +56,6 @@ int dm_scan_platdata(bool pre_reloc_only);
 int dm_scan_fdt(const void *blob, bool pre_reloc_only);
 
 /**
- * dm_scan_fdt_node() - Scan the device tree and bind drivers for a node
- *
- * This scans the subnodes of a device tree node and and creates a driver
- * for each one.
- *
- * @parent: Parent device for the devices that will be created
- * @blob: Pointer to device tree blob
- * @offset: Offset of node to scan
- * @pre_reloc_only: If true, bind only drivers with the DM_FLAG_PRE_RELOC
- * flag. If false bind all drivers.
- * @return 0 if OK, -ve on error
- */
-int dm_scan_fdt_node(struct udevice *parent, const void *blob, int offset,
-		     bool pre_reloc_only);
-
-/**
  * dm_scan_other() - Scan for other devices
  *
  * Some devices may not be visible to Driver Model. This weak function can
-- 
2.11.0.483.g087da7b7c-goog

[U-Boot] [PATCH 03/22] Update WARN_ON() to return a value

[Simon Glass]

In linux v4.9 this returns a value. This saves checking the warning
condition twice in some code.

Update the U-Boot version to do this also.

Signed-off-by: Simon Glass <sjg@chromium.org>
---

 include/linux/compat.h | 8 ++++++--
 1 file changed, 6 insertions(+), 2 deletions(-)

diff --git a/include/linux/compat.h b/include/linux/compat.h
index a43e4d66983..03f9bef0dae 100644
--- a/include/linux/compat.h
+++ b/include/linux/compat.h
@@ -106,8 +106,12 @@ static inline void kmem_cache_destroy(struct kmem_cache *cachep)
 #define BUG_ON(condition) do { if (condition) BUG(); } while(0)
 #endif /* BUG */
 
-#define WARN_ON(x) if (x) {printf("WARNING in %s line %d\n" \
-				  , __FILE__, __LINE__); }
+#define WARN_ON(condition) ({						\
+	int __ret_warn_on = !!(condition);				\
+	if (unlikely(__ret_warn_on))					\
+		printf("WARNING in %s line %d\n", __FILE__, __LINE__);;	\
+	unlikely(__ret_warn_on);					\
+})
 
 #define PAGE_SIZE	4096
 
-- 
2.11.0.483.g087da7b7c-goog

[U-Boot] [PATCH 04/22] dm: Add livetree definitions

[Simon Glass]

Add a Kconfig option to enable a live device tree, built at run time from
the flat tree. Also all structure definitions and a root node.

Signed-off-by: Simon Glass <sjg@chromium.org>
---

 dts/Kconfig                       | 11 ++++++++
 include/asm-generic/global_data.h |  3 +++
 include/dm/of.h                   | 53 +++++++++++++++++++++++++++++++++++++++
 3 files changed, 67 insertions(+)
 create mode 100644 include/dm/of.h

diff --git a/dts/Kconfig b/dts/Kconfig
index 4b7d8b15cc0..0fa1fb5c656 100644
--- a/dts/Kconfig
+++ b/dts/Kconfig
@@ -22,6 +22,17 @@ config SPL_OF_CONTROL
 	  which is not enough to support device tree. Enable this option to
 	  allow such boards to be supported by U-Boot SPL.
 
+config OF_LIVE
+	bool "Enable use of a live tree"
+	depends on OF_CONTROL
+	help
+	  Normally U-Boot uses a flat device tree which saves space and
+	  avoids the need to unpack the tree before use. However a flat
+	  tree does not support modifcation from within U-Boot since it
+	  can invalidate driver-model device tree offsets. This option
+	  enables a live tree which is available during relocation,
+	  and can be adjusted as needed.
+
 choice
 	prompt "Provider of DTB for DT control"
 	depends on OF_CONTROL
diff --git a/include/asm-generic/global_data.h b/include/asm-generic/global_data.h
index e02863dc031..c35f5d0c330 100644
--- a/include/asm-generic/global_data.h
+++ b/include/asm-generic/global_data.h
@@ -72,6 +72,9 @@ typedef struct global_data {
 	const void *fdt_blob;		/* Our device tree, NULL if none */
 	void *new_fdt;			/* Relocated FDT */
 	unsigned long fdt_size;		/* Space reserved for relocated FDT */
+#ifdef CONFIG_OF_LIVE
+	struct device_node *of_root;
+#endif
 	struct jt_funcs *jt;		/* jump table */
 	char env_buf[32];		/* buffer for getenv() before reloc. */
 #ifdef CONFIG_TRACE
diff --git a/include/dm/of.h b/include/dm/of.h
new file mode 100644
index 00000000000..716fd73ab38
--- /dev/null
+++ b/include/dm/of.h
@@ -0,0 +1,53 @@
+/*
+ * Copyright (c) 2017 Google, Inc
+ * Written by Simon Glass <sjg@chromium.org>
+ *
+ * SPDX-License-Identifier:	GPL-2.0+
+ */
+
+#ifndef _DM_OF_H
+#define _DM_OF_H
+
+typedef u32 phandle;
+typedef u32 ihandle;
+
+struct property {
+	char	*name;
+	int	length;
+	void	*value;
+	struct property *next;
+};
+
+struct device_node {
+	const char *name;
+	phandle phandle;
+	const char *full_name;
+
+	struct	property *properties;
+	struct	device_node *parent;
+	struct	device_node *child;
+	struct	device_node *sibling;
+};
+
+#define OF_MAX_PHANDLE_ARGS 16
+struct of_phandle_args {
+	struct device_node *np;
+	int args_count;
+	uint32_t args[OF_MAX_PHANDLE_ARGS];
+};
+
+DECLARE_GLOBAL_DATA_PTR;
+
+#ifdef CONFIG_OF_LIVE
+static inline bool of_use_livetree(void)
+{
+	return gd->of_root != NULL;
+}
+#else
+static inline bool of_use_livetree(void)
+{
+	return false;
+}
+#endif
+
+#endif
-- 
2.11.0.483.g087da7b7c-goog

[U-Boot] [PATCH 00/22] dm: Add support for a 'live' device tree

[Simon Glass]

Hi,

On 17 January 2017 at 21:50, Simon Glass <sjg@chromium.org> wrote:
> So far U-Boot uses a 'flat' device tree, which means that it is decoded
> on the fly as needed. This uses the libfdt library and avoids needing
> extra memory for additional tables.
>
> For some time there has been discussion about moving U-Boot to use a
> 'live' tree, where the device tree is decoded at start-up into a set of
> hierarchical structures with pointers.
>
> The advantages are:
>
> - It is somewhat faster to access (in particular scanning and looking for a
>     parent)
> - It permits the device tree to be changed at run-time (this is not
>     recommended at present since devices store the offset of their device
>     tree node and updating the tree may invalidate that). This could be
>     useful for overlays, for example.
> - It allows nodes to be referenced by a single pointer, instead of the
>     current device tree pointer plus offset
>
> The disadvantages are:
>
> - It requires more memory
> - It takes (a little) time to build the live tree
> - It adds more complexity under the hood, including an additional
>     abstraction layer
>
> This series is an attempt to introduce a useful live tree feature into
> U-Boot. There are many options and trade-offs. This series is the
> culmination of quite a bit of thought and experimentation.
>
> The approach used in this series is:
>
> - Before relocation the flat tree is used, to avoid extra memory usage
> and time. In general, there is not much access before relocation since
> most drivers are not started up. So there is little benefit in having a
> live tree
>
> - After relocation the live tree is built. At this point the CPU should be
> running quickly and there is plenty of memory. All available devices will
> be bound so the overhead of building the live tree may be outweighed by
> its greater efficiency.
>
> As a simplification, this series supports only one tree or the other. When
> the live tree is active, the flat tree cannot be used. That makes it easy
> to know the current state and avoids confusion over mixing offset and node
> pointers.
>
> Some drivers will need to be used both before and after relocation. This
> means that they must support both the flat and the live tree. To support
> this, the concept of a node 'reference' is defined. A reference can hold
> either a node offset (for the flat tree) or a node pointer (for the live
> tree). This allows drivers to access values from the tree regardless of
> which tree is in use.
>
> In addition, since most device tree access happens in the context of a
> device (struct udevice), a new 'dev_read' layer is provided to read device
> tree configuration associated with a device. This encapsulates the details
> of exactly how this information is read.
>
> I have taken the view that code compatibility with Linux is desirable. So
> the of_access.c file brings in code from Linux with very little
> modification. As new access methods are needed we should be able to bring
> in more code and avoid writing it ourselves in U-Boot.
>
> Conversion of drivers and subsystems to support the live tree (as well as
> flat tree) is fairly easy. A patch is included to add support to the GPIO
> uclass, and the cros_ec device is converted over to provide a driver
> example.
>
> Once I have comments on this series I will update it to build for all
> boards, to pass current tests and to tidy up the code. I will also add
> tests for the live tree and finish conversion for sandbox. So for now I am
> just looking for high-level comments, rather than a detailed code review.
>
>
> Simon Glass (22):
>   dm: core: Set return value first in lists_bind_fdt()
>   dm: core: atmel: Dont export dm_scan_fdt_node()
>   Update WARN_ON() to return a value
>   dm: Add livetree definitions
>   dm: Add livetree access functions
>   dm: Add a function to create a 'live' device tree
>   dm: Build a live tree after relocation
>   dm: Add support for device tree references
>   dm: Add a place to put extra device-tree reading functions
>   dm: core: Add device-based functions to access DT properties
>   dm: core: Allow binding a device from a live tree
>   dm: core: Add a method to find a driver for a livetree node
>   dm: core: Scan the live tree when setting up driver model
>   dm: core: Add a way to find a device by its live-tree node
>   dm: core: Add a way to find a device by node reference
>   dm: gpio: Refactor to prepare for live tree support
>   dm: gpio: Add live tree support
>   dm: gpio: Drop blank line in gpio_xlate_offs_flags() comment
>   dm: gpio: sandbox: Use dm_read...() functions to access DT
>   cros_ec: Fix debug() statement in ec_command_inptr()
>   cros_ec: Convert to support live tree
>   sandbox: Move to use live tree
>

Any comments on the general concept?

If you want to see what code that uses this might look like, see the
patch 'cros_ec: Convert to support live tree'.

Regards,
Simon

[U-Boot] [PATCH 00/22] dm: Add support for a 'live' device tree

[Tom Rini]

On Sat, Feb 04, 2017 at 07:34:59PM -0800, Simon Glass wrote:
> Hi,
> 
> On 17 January 2017 at 21:50, Simon Glass <sjg@chromium.org> wrote:
> > So far U-Boot uses a 'flat' device tree, which means that it is decoded
> > on the fly as needed. This uses the libfdt library and avoids needing
> > extra memory for additional tables.
> >
> > For some time there has been discussion about moving U-Boot to use a
> > 'live' tree, where the device tree is decoded at start-up into a set of
> > hierarchical structures with pointers.
> >
> > The advantages are:
> >
> > - It is somewhat faster to access (in particular scanning and looking for a
> >     parent)
> > - It permits the device tree to be changed at run-time (this is not
> >     recommended at present since devices store the offset of their device
> >     tree node and updating the tree may invalidate that). This could be
> >     useful for overlays, for example.
> > - It allows nodes to be referenced by a single pointer, instead of the
> >     current device tree pointer plus offset
> >
> > The disadvantages are:
> >
> > - It requires more memory
> > - It takes (a little) time to build the live tree
> > - It adds more complexity under the hood, including an additional
> >     abstraction layer
> >
> > This series is an attempt to introduce a useful live tree feature into
> > U-Boot. There are many options and trade-offs. This series is the
> > culmination of quite a bit of thought and experimentation.
> >
> > The approach used in this series is:
> >
> > - Before relocation the flat tree is used, to avoid extra memory usage
> > and time. In general, there is not much access before relocation since
> > most drivers are not started up. So there is little benefit in having a
> > live tree
> >
> > - After relocation the live tree is built. At this point the CPU should be
> > running quickly and there is plenty of memory. All available devices will
> > be bound so the overhead of building the live tree may be outweighed by
> > its greater efficiency.
> >
> > As a simplification, this series supports only one tree or the other. When
> > the live tree is active, the flat tree cannot be used. That makes it easy
> > to know the current state and avoids confusion over mixing offset and node
> > pointers.
> >
> > Some drivers will need to be used both before and after relocation. This
> > means that they must support both the flat and the live tree. To support
> > this, the concept of a node 'reference' is defined. A reference can hold
> > either a node offset (for the flat tree) or a node pointer (for the live
> > tree). This allows drivers to access values from the tree regardless of
> > which tree is in use.
> >
> > In addition, since most device tree access happens in the context of a
> > device (struct udevice), a new 'dev_read' layer is provided to read device
> > tree configuration associated with a device. This encapsulates the details
> > of exactly how this information is read.
> >
> > I have taken the view that code compatibility with Linux is desirable. So
> > the of_access.c file brings in code from Linux with very little
> > modification. As new access methods are needed we should be able to bring
> > in more code and avoid writing it ourselves in U-Boot.
> >
> > Conversion of drivers and subsystems to support the live tree (as well as
> > flat tree) is fairly easy. A patch is included to add support to the GPIO
> > uclass, and the cros_ec device is converted over to provide a driver
> > example.
> >
> > Once I have comments on this series I will update it to build for all
> > boards, to pass current tests and to tidy up the code. I will also add
> > tests for the live tree and finish conversion for sandbox. So for now I am
> > just looking for high-level comments, rather than a detailed code review.
> >
> >
> > Simon Glass (22):
> >   dm: core: Set return value first in lists_bind_fdt()
> >   dm: core: atmel: Dont export dm_scan_fdt_node()
> >   Update WARN_ON() to return a value
> >   dm: Add livetree definitions
> >   dm: Add livetree access functions
> >   dm: Add a function to create a 'live' device tree
> >   dm: Build a live tree after relocation
> >   dm: Add support for device tree references
> >   dm: Add a place to put extra device-tree reading functions
> >   dm: core: Add device-based functions to access DT properties
> >   dm: core: Allow binding a device from a live tree
> >   dm: core: Add a method to find a driver for a livetree node
> >   dm: core: Scan the live tree when setting up driver model
> >   dm: core: Add a way to find a device by its live-tree node
> >   dm: core: Add a way to find a device by node reference
> >   dm: gpio: Refactor to prepare for live tree support
> >   dm: gpio: Add live tree support
> >   dm: gpio: Drop blank line in gpio_xlate_offs_flags() comment
> >   dm: gpio: sandbox: Use dm_read...() functions to access DT
> >   cros_ec: Fix debug() statement in ec_command_inptr()
> >   cros_ec: Convert to support live tree
> >   sandbox: Move to use live tree
> 
> Any comments on the general concept?

Sorry, I do need to review this and the concept a bit more, just been
busy of late.

-- 
Tom
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 819 bytes
Desc: Digital signature
URL: <http://lists.denx.de/pipermail/u-boot/attachments/20170205/dc25f26c/attachment.sig>

[U-Boot] [PATCH 00/22] dm: Add support for a 'live' device tree

[Tom Rini]

On Sat, Feb 04, 2017 at 07:34:59PM -0800, Simon Glass wrote:
> Hi,
> 
> On 17 January 2017 at 21:50, Simon Glass <sjg@chromium.org> wrote:
> > So far U-Boot uses a 'flat' device tree, which means that it is decoded
> > on the fly as needed. This uses the libfdt library and avoids needing
> > extra memory for additional tables.
> >
> > For some time there has been discussion about moving U-Boot to use a
> > 'live' tree, where the device tree is decoded at start-up into a set of
> > hierarchical structures with pointers.
> >
> > The advantages are:
> >
> > - It is somewhat faster to access (in particular scanning and looking for a
> >     parent)
> > - It permits the device tree to be changed at run-time (this is not
> >     recommended at present since devices store the offset of their device
> >     tree node and updating the tree may invalidate that). This could be
> >     useful for overlays, for example.
> > - It allows nodes to be referenced by a single pointer, instead of the
> >     current device tree pointer plus offset
> >
> > The disadvantages are:
> >
> > - It requires more memory
> > - It takes (a little) time to build the live tree
> > - It adds more complexity under the hood, including an additional
> >     abstraction layer
> >
> > This series is an attempt to introduce a useful live tree feature into
> > U-Boot. There are many options and trade-offs. This series is the
> > culmination of quite a bit of thought and experimentation.
> >
> > The approach used in this series is:
> >
> > - Before relocation the flat tree is used, to avoid extra memory usage
> > and time. In general, there is not much access before relocation since
> > most drivers are not started up. So there is little benefit in having a
> > live tree
> >
> > - After relocation the live tree is built. At this point the CPU should be
> > running quickly and there is plenty of memory. All available devices will
> > be bound so the overhead of building the live tree may be outweighed by
> > its greater efficiency.
> >
> > As a simplification, this series supports only one tree or the other. When
> > the live tree is active, the flat tree cannot be used. That makes it easy
> > to know the current state and avoids confusion over mixing offset and node
> > pointers.
> >
> > Some drivers will need to be used both before and after relocation. This
> > means that they must support both the flat and the live tree. To support
> > this, the concept of a node 'reference' is defined. A reference can hold
> > either a node offset (for the flat tree) or a node pointer (for the live
> > tree). This allows drivers to access values from the tree regardless of
> > which tree is in use.
> >
> > In addition, since most device tree access happens in the context of a
> > device (struct udevice), a new 'dev_read' layer is provided to read device
> > tree configuration associated with a device. This encapsulates the details
> > of exactly how this information is read.
> >
> > I have taken the view that code compatibility with Linux is desirable. So
> > the of_access.c file brings in code from Linux with very little
> > modification. As new access methods are needed we should be able to bring
> > in more code and avoid writing it ourselves in U-Boot.
> >
> > Conversion of drivers and subsystems to support the live tree (as well as
> > flat tree) is fairly easy. A patch is included to add support to the GPIO
> > uclass, and the cros_ec device is converted over to provide a driver
> > example.
> >
> > Once I have comments on this series I will update it to build for all
> > boards, to pass current tests and to tidy up the code. I will also add
> > tests for the live tree and finish conversion for sandbox. So for now I am
> > just looking for high-level comments, rather than a detailed code review.
> >
> >
> > Simon Glass (22):
> >   dm: core: Set return value first in lists_bind_fdt()
> >   dm: core: atmel: Dont export dm_scan_fdt_node()
> >   Update WARN_ON() to return a value
> >   dm: Add livetree definitions
> >   dm: Add livetree access functions
> >   dm: Add a function to create a 'live' device tree
> >   dm: Build a live tree after relocation
> >   dm: Add support for device tree references
> >   dm: Add a place to put extra device-tree reading functions
> >   dm: core: Add device-based functions to access DT properties
> >   dm: core: Allow binding a device from a live tree
> >   dm: core: Add a method to find a driver for a livetree node
> >   dm: core: Scan the live tree when setting up driver model
> >   dm: core: Add a way to find a device by its live-tree node
> >   dm: core: Add a way to find a device by node reference
> >   dm: gpio: Refactor to prepare for live tree support
> >   dm: gpio: Add live tree support
> >   dm: gpio: Drop blank line in gpio_xlate_offs_flags() comment
> >   dm: gpio: sandbox: Use dm_read...() functions to access DT
> >   cros_ec: Fix debug() statement in ec_command_inptr()
> >   cros_ec: Convert to support live tree
> >   sandbox: Move to use live tree
> >
> 
> Any comments on the general concept?
> 
> If you want to see what code that uses this might look like, see the
> patch 'cros_ec: Convert to support live tree'.

So, in the end, are the interfaces drivers use easier?  Do we reduce or
further increase the overall amount of code?  How does this look on the
binary size side of things?  Those kind of things are my biggest
questions after giving the series a quick look.  I'm not sure either way
at this moment, especially given what Franklin Cooper is doing on
keystone 2 for example (we need to switch from a "generic" SoC tree to
the real board tree, so he adds a hook to change the tree very early
on).  Thanks!

-- 
Tom
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 819 bytes
Desc: Digital signature
URL: <http://lists.denx.de/pipermail/u-boot/attachments/20170321/ee5170a7/attachment.sig>

[U-Boot] [PATCH 00/22] dm: Add support for a 'live' device tree

[Simon Glass]

Hi Tom,

On 21 March 2017 at 08:07, Tom Rini <trini@konsulko.com> wrote:
> On Sat, Feb 04, 2017 at 07:34:59PM -0800, Simon Glass wrote:
>> Hi,
>>
>> On 17 January 2017 at 21:50, Simon Glass <sjg@chromium.org> wrote:
>> > So far U-Boot uses a 'flat' device tree, which means that it is decoded
>> > on the fly as needed. This uses the libfdt library and avoids needing
>> > extra memory for additional tables.
>> >
>> > For some time there has been discussion about moving U-Boot to use a
>> > 'live' tree, where the device tree is decoded at start-up into a set of
>> > hierarchical structures with pointers.
>> >
>> > The advantages are:
>> >
>> > - It is somewhat faster to access (in particular scanning and looking for a
>> >     parent)
>> > - It permits the device tree to be changed at run-time (this is not
>> >     recommended at present since devices store the offset of their device
>> >     tree node and updating the tree may invalidate that). This could be
>> >     useful for overlays, for example.
>> > - It allows nodes to be referenced by a single pointer, instead of the
>> >     current device tree pointer plus offset
>> >
>> > The disadvantages are:
>> >
>> > - It requires more memory
>> > - It takes (a little) time to build the live tree
>> > - It adds more complexity under the hood, including an additional
>> >     abstraction layer
>> >
>> > This series is an attempt to introduce a useful live tree feature into
>> > U-Boot. There are many options and trade-offs. This series is the
>> > culmination of quite a bit of thought and experimentation.
>> >
>> > The approach used in this series is:
>> >
>> > - Before relocation the flat tree is used, to avoid extra memory usage
>> > and time. In general, there is not much access before relocation since
>> > most drivers are not started up. So there is little benefit in having a
>> > live tree
>> >
>> > - After relocation the live tree is built. At this point the CPU should be
>> > running quickly and there is plenty of memory. All available devices will
>> > be bound so the overhead of building the live tree may be outweighed by
>> > its greater efficiency.
>> >
>> > As a simplification, this series supports only one tree or the other. When
>> > the live tree is active, the flat tree cannot be used. That makes it easy
>> > to know the current state and avoids confusion over mixing offset and node
>> > pointers.
>> >
>> > Some drivers will need to be used both before and after relocation. This
>> > means that they must support both the flat and the live tree. To support
>> > this, the concept of a node 'reference' is defined. A reference can hold
>> > either a node offset (for the flat tree) or a node pointer (for the live
>> > tree). This allows drivers to access values from the tree regardless of
>> > which tree is in use.
>> >
>> > In addition, since most device tree access happens in the context of a
>> > device (struct udevice), a new 'dev_read' layer is provided to read device
>> > tree configuration associated with a device. This encapsulates the details
>> > of exactly how this information is read.
>> >
>> > I have taken the view that code compatibility with Linux is desirable. So
>> > the of_access.c file brings in code from Linux with very little
>> > modification. As new access methods are needed we should be able to bring
>> > in more code and avoid writing it ourselves in U-Boot.
>> >
>> > Conversion of drivers and subsystems to support the live tree (as well as
>> > flat tree) is fairly easy. A patch is included to add support to the GPIO
>> > uclass, and the cros_ec device is converted over to provide a driver
>> > example.
>> >
>> > Once I have comments on this series I will update it to build for all
>> > boards, to pass current tests and to tidy up the code. I will also add
>> > tests for the live tree and finish conversion for sandbox. So for now I am
>> > just looking for high-level comments, rather than a detailed code review.
>> >
>> >
>> > Simon Glass (22):
>> >   dm: core: Set return value first in lists_bind_fdt()
>> >   dm: core: atmel: Dont export dm_scan_fdt_node()
>> >   Update WARN_ON() to return a value
>> >   dm: Add livetree definitions
>> >   dm: Add livetree access functions
>> >   dm: Add a function to create a 'live' device tree
>> >   dm: Build a live tree after relocation
>> >   dm: Add support for device tree references
>> >   dm: Add a place to put extra device-tree reading functions
>> >   dm: core: Add device-based functions to access DT properties
>> >   dm: core: Allow binding a device from a live tree
>> >   dm: core: Add a method to find a driver for a livetree node
>> >   dm: core: Scan the live tree when setting up driver model
>> >   dm: core: Add a way to find a device by its live-tree node
>> >   dm: core: Add a way to find a device by node reference
>> >   dm: gpio: Refactor to prepare for live tree support
>> >   dm: gpio: Add live tree support
>> >   dm: gpio: Drop blank line in gpio_xlate_offs_flags() comment
>> >   dm: gpio: sandbox: Use dm_read...() functions to access DT
>> >   cros_ec: Fix debug() statement in ec_command_inptr()
>> >   cros_ec: Convert to support live tree
>> >   sandbox: Move to use live tree
>> >
>>
>> Any comments on the general concept?
>>
>> If you want to see what code that uses this might look like, see the
>> patch 'cros_ec: Convert to support live tree'.
>
> So, in the end, are the interfaces drivers use easier?  Do we reduce or
> further increase the overall amount of code?  How does this look on the
> binary size side of things?  Those kind of things are my biggest
> questions after giving the series a quick look.  I'm not sure either way
> at this moment, especially given what Franklin Cooper is doing on
> keystone 2 for example (we need to switch from a "generic" SoC tree to
> the real board tree, so he adds a hook to change the tree very early
> on).  Thanks!

1. Driver ease of writing

I believe the interface makes drivers a bit easier, e.g.:

dev_read_u32_defaut(dev, "propname", default_val)

instead of:

fdtget_get_int(gd->of_blob, dev_of_offset(dev), "propname", default_val)

2. Amount of driver code

It reduces the amount of code in drivers slightly due to the above,
and the fact that you don't need to deal with the device tree pointer
and offsets directory.

It adds another layer so there is more code overall, but (a) it is
optional so can be skipped for SPL where we don't want a livetree and
(b) it is a one-time increase so as more code is added the percentage
of increase falls

3. Binary size

Obviously there is an increase to support livetree, but it is not
large. I cannot measure it easily until I convert (say) an ARM board.
But basically it is the livetree generation code (which is small) and
the translation library (which converts node reference to either
fdtdec_... or of_... calls). You don't get anything for free, but this
is not a big change.

One benefit of doing this change soon is that it reduces the migration
cost. I am not quite happen with fdtdec_...(blob, offset...)
everywhere.

Perhaps I should try to convert an ARM board over and see how it looks.

Regards,
Simon