On Mon, Aug 18, 2014 at 12:06:26PM -0600, Simon Glass wrote:
Hi Thierry,
On 18 August 2014 01:16, Thierry Reding thierry.reding@gmail.com wrote:
From: Thierry Reding treding@nvidia.com
Add the fdt_get_resource() and fdt_get_named_resource() functions which can be used to parse resources (memory regions) from an FDT. A helper to compute the size of a region is also provided.
Signed-off-by: Thierry Reding treding@nvidia.com
include/fdtdec.h | 48 ++++++++++++++++++++++++++++++++++++++++++++++++ lib/fdtdec.c | 43 +++++++++++++++++++++++++++++++++++++++++++ 2 files changed, 91 insertions(+)
diff --git a/include/fdtdec.h b/include/fdtdec.h index 856e6cf766de..e9091eee6bae 100644 --- a/include/fdtdec.h +++ b/include/fdtdec.h @@ -40,6 +40,23 @@ struct fdt_memory { fdt_addr_t end; };
+/* information about a resource */
Please add comments, e.g. that end is inclusive.
I've added this comment:
/* * Information about a resource. start is the first address of the resource * and end is the last address (inclusive). The length of the resource will * be equal to: end - start + 1. */
Is that enough or do you want me to be more verbose?
+/**
- Obtain a named resource from a device property.
- Look up the index of the name in a list of strings and return the resource
- at that index.
- @param fdt FDT blob
- @param node node to examine
- @param property name of the property to parse
- @param names name of the property to obtain the match the name to
- @param name the name of the entry to look up
These two parameters are confusing. Perhaps rename names to something better?
How about "prop_names" so that it indicates it is the name of a property? I've also adjusted the comment a bit:
* @param prop_names name of the property containing the list of names
Hopefully that will make it more obvious.
+int fdt_get_resource(const void *fdt, int node, const char *property,
unsigned int index, struct fdt_resource *res)s/index/find_index/
In my opinion the find_ prefix is redundant. After all the function name already indicates that we're looking for a resource inside a property. And index would be the offset in that property.
if (!ptr)return len;end = ptr + len / 4;sizeof(*ptr) might be better than 4.
Device tree explicitly specifies that cells are 32-bit, so this can't ever be anything other than 4. But I'll change it anyway if you prefer.
while (ptr + na + ns <= end) {if (i == index) {res->start = fdt_addr_to_cpu(*ptr);This doesn't deal with 64-bit addresses. There is a half-hearted attempt with fdt_addr_t, but I wonder if we need a helper function like fdt_get_addr(ptr, num_cells)?
The Linux kernel handles this by wrapping it in an of_read_number() helper and always returning u64, like this:
static inline u64 of_read_number(const __be32 *cell, int size) { u64 r = 0;
while (size--) r = (r << 32) | be32_to_cpu(*(cell++));
return r; }
It obviously only works for size in { 0, 1, 2 }, but I can add a helper like that if you want.
Thierry