[U-Boot] [PATCH] fdt: Fix fdtdec_get_addr_size() for 64-bit
Stephen Warren
swarren at wwwdotorg.org
Thu Jul 23 18:51:30 CEST 2015
From: Thierry Reding <treding at nvidia.com>
Signed-off-by: Thierry Reding <treding at nvidia.com>
Signed-off-by: Tom Warren <twarren at nvidia.com>
Signed-off-by: Stephen Warren <swarren at 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,
--
1.9.1
More information about the U-Boot
mailing list