Re: [PATCH 01/23] fdt: Add functions to query a node's #address- and #size-cells

On Fri, Aug 22, 2014 at 09:03:49PM -0600, Simon Glass wrote:

Hi Thierry,

On 19 August 2014 07:06, Thierry Reding [off-list ref] wrote:

quoted

On Tue, Aug 19, 2014 at 06:52:22AM -0600, Simon Glass wrote:

quoted

Hi Theirry,


On 19 August 2014 04:59, Thierry Reding [off-list ref] wrote:

quoted

On Mon, Aug 18, 2014 at 11:52:36AM -0600, Simon Glass wrote:

quoted

Hi Thierry,

On 18 August 2014 01:16, Thierry Reding [off-list ref]

wrote:

quoted

quoted

From: Thierry Reding <redacted>

Given a device tree node, the fdt_n_addr_cells() function will walk up
the device tree and search for an #address-cells property. It returns
the number of cells required by the device tree node to represent an
address.

Similarly the fdt_n_size_cells() function returns the number of cells
required by the given device tree node to represent a size. It achieves
that by walking up the device tree in seach for a #size-cells property.

If no #address-cells or #size-cells property can be found, both of the
functions return 1.

Signed-off-by: Thierry Reding <redacted>
---
 include/libfdt.h    | 28 ++++++++++++++++++++++++++++
 lib/libfdt/fdt_ro.c | 36 ++++++++++++++++++++++++++++++++++++
 2 files changed, 64 insertions(+)

diff --git a/include/libfdt.h b/include/libfdt.h
index a1ef1e15df3d..e7f991b388cf 100644
--- a/include/libfdt.h
+++ b/include/libfdt.h

@@ -1638,4 +1638,32 @@ int fdt_find_regions(const void *fdt, char *

const inc[], int inc_count,

quoted

quoted

                     struct fdt_region region[], int max_regions,
                     char *path, int path_len, int add_string_tab);

+/**
+ * fdt_n_addr_cells() - find the number of address cells required by

a node

quoted

quoted

+ *
+ * Looks up the #address-cells property of the node to examine. If

that has

quoted

quoted

+ * no such property, walks up the device tree until it finds one in

one of

quoted

quoted

+ * the device's parents. If no #address-cells property is found, it is
+ * assumed to be 1.
+ *
+ * @param fdt          FDT blob
+ * @param node         node to examine
+ * @return number of address cells
+ */
+int fdt_n_addr_cells(const void *fdt, int node);

There is a new fdt_address_cells() recently that looks suitable.

quoted

+
+/**
+ * fdt_n_size_cells() - find the number of size cells required by a

node

quoted

Also fdt_size_cells().

Neither of those seem to walk up the tree, so inheriting #address-cells
or #size-cells from a parent will not work.

According to the comments in the code they're also supposed to return
the number of cells represented by a bus, not a device (which might
explain why it doesn't walk up the tree).

I also can't reuse them to implement these fdt_n_addr_cells() and
fdt_n_size_cells() functions because they don't return an accurate error
if the #address-cells and #size-cells are not found, therefore it's
impossible to decide when to climb further up the tree.

OK but I have one more question. I thought that dtc gave a warning when you
don't explicitly specify these values in the parent node?

It doesn't explicitly warn about the properties being absent. Rather it
will fallback to the default (#address-cells = <2>, #size-cells = <1> in
the version that I have) and warn if those don't match what's expected.

Interestingly as opposed to what the Linux kernel does, DTC doesn't seem
to climb further up the tree if it can't find the properties in a device
tree node. Instead, it seems to always use the default values instead.

Adding the devicetree-u79uwXL29TY76Z2rM5mHXA@public.gmane.org mailing list. Maybe somebody there
can help iron out this inconsistency. ePAPR explicitly says that both
"... #address-cells and #size-cells properties are not inherited from
ancestors in the device tree. They shall be explicitly defined." (see
2.3.5 "#address-cells and #size-cells").

That's my understanding too. So should we drop this patch?

Yes, in this particular case it works fine using the existing
fdt_address_cells() and fdt_size_cells() functions so I've dropped this
patch.

Thierry