From: Thierry Reding treding@nvidia.com
Signed-off-by: Thierry Reding treding@nvidia.com Signed-off-by: Tom Warren twarren@nvidia.com Signed-off-by: Stephen Warren swarren@nvidia.com --- Simon,
When Thierry first posted this patch, you responded:
parent = fdt_parent_offset(blob, node);This function is very slow as it must scan the whole tree. Can we instead pass in the parent node?
I don't think that's possible in general. This function is called from fdtdec_get_addr(), and it's easy to find call sites of that function that don't have the parent node available. IIRC, the first couple of example I found scan the DT for a node with a certain compatible value, or look up nodes via aliases, rather than being called while parsing the DT in a top-down tree-like fashion, where the parent node is easily available. I didn't do an exhaustive search after I found a few problematic cases.
Also, how about (in addition) a version of this function that works for devices? Like:
device_get_addr_size(struct udevice *dev, ...)
so that it can handle this for you.
That sounds like a separate patch?
Equally, I see that struct udevice contains an of_offset field, but no parent_of_offset or similar. There is a struct udevice *parent though; is the struct udevice hierarchy guaranteed to 100% match the DT hierarchy? I know this isn't necessarily guaranteed in Linux's device model for example.
As such, this patch seems OK to me as-is. --- lib/fdtdec.c | 56 ++++++++++++++++++++++++++++++++++++-------------------- 1 file changed, 36 insertions(+), 20 deletions(-)
diff --git a/lib/fdtdec.c b/lib/fdtdec.c index 9c6b3619da24..56e72eafaade 100644 --- a/lib/fdtdec.c +++ b/lib/fdtdec.c @@ -88,29 +88,45 @@ const char *fdtdec_get_compatible(enum fdt_compat_id id) fdt_addr_t fdtdec_get_addr_size(const void *blob, int node, const char *prop_name, fdt_size_t *sizep) { - const fdt_addr_t *cell; - int len; + const fdt32_t *ptr, *end; + int parent, na, ns, len; + fdt_addr_t addr;
debug("%s: %s: ", __func__, prop_name); - cell = fdt_getprop(blob, node, prop_name, &len); - if (cell && ((!sizep && len == sizeof(fdt_addr_t)) || - len == sizeof(fdt_addr_t) * 2)) { - fdt_addr_t addr = fdt_addr_to_cpu(*cell); - if (sizep) { - const fdt_size_t *size; - - size = (fdt_size_t *)((char *)cell + - sizeof(fdt_addr_t)); - *sizep = fdt_size_to_cpu(*size); - debug("addr=%08lx, size=%llx\n", - (ulong)addr, (u64)*sizep); - } else { - debug("%08lx\n", (ulong)addr); - } - return addr; + + parent = fdt_parent_offset(blob, node); + if (parent < 0) { + debug("(no parent found)\n"); + return FDT_ADDR_T_NONE; } - debug("(not found)\n"); - return FDT_ADDR_T_NONE; + + na = fdt_address_cells(blob, parent); + ns = fdt_size_cells(blob, parent); + + ptr = fdt_getprop(blob, node, prop_name, &len); + if (!ptr) { + debug("(not found)\n"); + return FDT_ADDR_T_NONE; + } + + end = ptr + len / sizeof(*ptr); + + if (ptr + na + ns > end) { + debug("(not enough data: expected %d bytes, got %d bytes)\n", + (na + ns) * 4, len); + return FDT_ADDR_T_NONE; + } + + addr = fdtdec_get_number(ptr, na); + + if (sizep) { + *sizep = fdtdec_get_number(ptr + na, ns); + debug("addr=%pa, size=%pa\n", &addr, sizep); + } else { + debug("%pa\n", &addr); + } + + return addr; }
fdt_addr_t fdtdec_get_addr(const void *blob, int node,