Re: [U-Boot] [PATCH v2 02/17] fdt: Add functions to access phandles, arrays and bools
On 12/02/2011 07:11 PM, Simon Glass wrote:
Add a function to look up a property which is a phandle in a node, and
another to read a fixed-length integer array from an fdt property.
Also add a function to read boolean properties, although there is no
actual boolean type in U-Boot.
+/**
+ * Look up a boolean property in a node and return it.
+ *
+ * A boolean properly is true if present in the device tree and false if not
+ * present, or present with a 0 value.
+ *
+ * @param blob FDT blob
+ * @param node node to examine
+ * @param prop_name name of property to find
+ * @return 1 if the properly is present; 0 if it isn't present or is 0
+ */
+int fdtdec_get_bool(const void *blob, int node, const char *prop_name)
+{
+ const s32 *cell;
+ int len;
+
+ debug(%s: %s\n, __func__, prop_name);
+ cell = fdt_getprop(blob, node, prop_name, len);
+ if (!cell)
+ return 0;
+ if (len = sizeof(u32) *cell == 0)
+ return 0;
+
+ return 1;
+}
I'm still slightly worried that this implementation may interpret the
FDT differently to the kernel. At least the code I've written for
boolean properties /only/ looks at the presence of the property, and
never at the contents even if there is one.
That said, the ePAPR specification does only say that boolean properties
/might/ have an empty value, thus implying that a non-zero length is
permissible. so, perhaps this is fine.
Actually no, I'm more than worried now I think it through. You'd argued
for being able to write 0/1 to the property so that U-Boot could modify
the value in-place without having to add/remove nodes to/from the FDT
image, but rather just change their content. However, if this modified
FDT is then passed to the kernel (rather than some other fresh FDT),
then it's imperative that U-Boot and the kernel represent boolean
properties in the exact same way. Hence, we either can't have U-Boot use
this edit-in-place optimization, or U-Boot needs some cleanup of the
FDT image before passing it to the kernel. However, the latter is
impossible, since by then since the boot-the-kernel code has no idea
whether a property is a boolean or not, and hence implementing such a
cleanup is impossible.
--
nvpublic
___
U-Boot mailing list
U-Boot@lists.denx.de
http://lists.denx.de/mailman/listinfo/u-boot
Re: [U-Boot] [PATCH v2 02/17] fdt: Add functions to access phandles, arrays and bools
Hi Stephen,
On Mon, Dec 5, 2011 at 1:59 PM, Stephen Warren swar...@nvidia.com wrote:
On 12/02/2011 07:11 PM, Simon Glass wrote:
Add a function to look up a property which is a phandle in a node, and
another to read a fixed-length integer array from an fdt property.
Also add a function to read boolean properties, although there is no
actual boolean type in U-Boot.
+/**
+ * Look up a boolean property in a node and return it.
+ *
+ * A boolean properly is true if present in the device tree and false if not
+ * present, or present with a 0 value.
+ *
+ * @param blob FDT blob
+ * @param node node to examine
+ * @param prop_name name of property to find
+ * @return 1 if the properly is present; 0 if it isn't present or is 0
+ */
+int fdtdec_get_bool(const void *blob, int node, const char *prop_name)
+{
+ const s32 *cell;
+ int len;
+
+ debug(%s: %s\n, __func__, prop_name);
+ cell = fdt_getprop(blob, node, prop_name, len);
+ if (!cell)
+ return 0;
+ if (len = sizeof(u32) *cell == 0)
+ return 0;
+
+ return 1;
+}
I'm still slightly worried that this implementation may interpret the
FDT differently to the kernel. At least the code I've written for
boolean properties /only/ looks at the presence of the property, and
never at the contents even if there is one.
That said, the ePAPR specification does only say that boolean properties
/might/ have an empty value, thus implying that a non-zero length is
permissible. so, perhaps this is fine.
OK
Actually no, I'm more than worried now I think it through. You'd argued
for being able to write 0/1 to the property so that U-Boot could modify
the value in-place without having to add/remove nodes to/from the FDT
image, but rather just change their content. However, if this modified
FDT is then passed to the kernel (rather than some other fresh FDT),
then it's imperative that U-Boot and the kernel represent boolean
properties in the exact same way. Hence, we either can't have U-Boot use
this edit-in-place optimization, or U-Boot needs some cleanup of the
FDT image before passing it to the kernel. However, the latter is
impossible, since by then since the boot-the-kernel code has no idea
whether a property is a boolean or not, and hence implementing such a
cleanup is impossible.
Hang on - this fdt cannot be passed to the kernel! It is for U-Boot's
use, for it's own configuration. There is no mechanism to fix up
U-Boot's internal fdt and pass it to the kernel. U-Boot scripts can't
even find its address!
That side of U-Boot is a complete different use of fdt and I have been
careful (so far) to keep them apart.
Regards,
Simon
--
nvpublic
___
U-Boot mailing list
U-Boot@lists.denx.de
http://lists.denx.de/mailman/listinfo/u-boot
Re: [U-Boot] [PATCH v2 02/17] fdt: Add functions to access phandles, arrays and bools
On 12/05/2011 03:07 PM, Simon Glass wrote:
On Mon, Dec 5, 2011 at 1:59 PM, Stephen Warren swar...@nvidia.com wrote:
On 12/02/2011 07:11 PM, Simon Glass wrote:
+int fdtdec_get_bool(const void *blob, int node, const char *prop_name)
+{
+ const s32 *cell;
+ int len;
+
+ debug(%s: %s\n, __func__, prop_name);
+ cell = fdt_getprop(blob, node, prop_name, len);
+ if (!cell)
+ return 0;
+ if (len = sizeof(u32) *cell == 0)
+ return 0;
+
+ return 1;
+}
I'm still slightly worried that this implementation may interpret the
FDT differently to the kernel. At least the code I've written for
boolean properties /only/ looks at the presence of the property, and
never at the contents even if there is one.
That said, the ePAPR specification does only say that boolean properties
/might/ have an empty value, thus implying that a non-zero length is
permissible. so, perhaps this is fine.
OK
Actually no, I'm more than worried now I think it through. You'd argued
for being able to write 0/1 to the property so that U-Boot could modify
the value in-place without having to add/remove nodes to/from the FDT
image, but rather just change their content. However, if this modified
FDT is then passed to the kernel (rather than some other fresh FDT),
then it's imperative that U-Boot and the kernel represent boolean
properties in the exact same way. Hence, we either can't have U-Boot use
this edit-in-place optimization, or U-Boot needs some cleanup of the
FDT image before passing it to the kernel. However, the latter is
impossible, since by then since the boot-the-kernel code has no idea
whether a property is a boolean or not, and hence implementing such a
cleanup is impossible.
Hang on - this fdt cannot be passed to the kernel! It is for U-Boot's
use, for it's own configuration. There is no mechanism to fix up
U-Boot's internal fdt and pass it to the kernel. U-Boot scripts can't
even find its address!
That side of U-Boot is a complete different use of fdt and I have been
careful (so far) to keep them apart.
Yes, that's certainly the way it is right now.
I'd heard some discussion on changing that, and allowing U-Boot to pass
its internal configuration FDT to the kernel instead of loading it from
disk a second time. However, it looks like that discussion was purely
internal to NVIDIA and might have been motivated by some confusion
rather than actual intent.
As such, my comments about fixing stuff up to pass to the kernel don't
apply.
Still, I think we should be able pass the same FDT to U-Boot and the
kernel, and as such they should both interpret boolean properties in the
same way. This patch doesn't do that.
--
nvpublic
___
U-Boot mailing list
U-Boot@lists.denx.de
http://lists.denx.de/mailman/listinfo/u-boot
Re: [U-Boot] [PATCH v2 02/17] fdt: Add functions to access phandles, arrays and bools
Hi Stephen,
On Mon, Dec 5, 2011 at 2:36 PM, Stephen Warren swar...@nvidia.com wrote:
On 12/05/2011 03:07 PM, Simon Glass wrote:
On Mon, Dec 5, 2011 at 1:59 PM, Stephen Warren swar...@nvidia.com wrote:
On 12/02/2011 07:11 PM, Simon Glass wrote:
+int fdtdec_get_bool(const void *blob, int node, const char *prop_name)
+{
+ const s32 *cell;
+ int len;
+
+ debug(%s: %s\n, __func__, prop_name);
+ cell = fdt_getprop(blob, node, prop_name, len);
+ if (!cell)
+ return 0;
+ if (len = sizeof(u32) *cell == 0)
+ return 0;
+
+ return 1;
+}
I'm still slightly worried that this implementation may interpret the
FDT differently to the kernel. At least the code I've written for
boolean properties /only/ looks at the presence of the property, and
never at the contents even if there is one.
That said, the ePAPR specification does only say that boolean properties
/might/ have an empty value, thus implying that a non-zero length is
permissible. so, perhaps this is fine.
OK
Actually no, I'm more than worried now I think it through. You'd argued
for being able to write 0/1 to the property so that U-Boot could modify
the value in-place without having to add/remove nodes to/from the FDT
image, but rather just change their content. However, if this modified
FDT is then passed to the kernel (rather than some other fresh FDT),
then it's imperative that U-Boot and the kernel represent boolean
properties in the exact same way. Hence, we either can't have U-Boot use
this edit-in-place optimization, or U-Boot needs some cleanup of the
FDT image before passing it to the kernel. However, the latter is
impossible, since by then since the boot-the-kernel code has no idea
whether a property is a boolean or not, and hence implementing such a
cleanup is impossible.
Hang on - this fdt cannot be passed to the kernel! It is for U-Boot's
use, for it's own configuration. There is no mechanism to fix up
U-Boot's internal fdt and pass it to the kernel. U-Boot scripts can't
even find its address!
That side of U-Boot is a complete different use of fdt and I have been
careful (so far) to keep them apart.
Yes, that's certainly the way it is right now.
I'd heard some discussion on changing that, and allowing U-Boot to pass
its internal configuration FDT to the kernel instead of loading it from
disk a second time. However, it looks like that discussion was purely
internal to NVIDIA and might have been motivated by some confusion
rather than actual intent.
As such, my comments about fixing stuff up to pass to the kernel don't
apply.
OK
Still, I think we should be able pass the same FDT to U-Boot and the
kernel, and as such they should both interpret boolean properties in the
same way. This patch doesn't do that.
OK, I will change it, and start a separate thread on
devicetree-discuss about fdtget/put.
Regards,
Simon
--
nvpublic
___
U-Boot mailing list
U-Boot@lists.denx.de
http://lists.denx.de/mailman/listinfo/u-boot
[U-Boot] [PATCH v2 02/17] fdt: Add functions to access phandles, arrays and bools
Add a function to look up a property which is a phandle in a node, and
another to read a fixed-length integer array from an fdt property.
Also add a function to read boolean properties, although there is no
actual boolean type in U-Boot.
Signed-off-by: Simon Glass s...@chromium.org
---
Changes in v2:
- Rename get_prop_len() to get_prop_check_min_len()
include/fdtdec.h | 40 ++
lib/fdtdec.c | 83 ++
2 files changed, 123 insertions(+), 0 deletions(-)
diff --git a/include/fdtdec.h b/include/fdtdec.h
index 49dbac4..974520a 100644
--- a/include/fdtdec.h
+++ b/include/fdtdec.h
@@ -141,3 +141,43 @@ int fdtdec_get_is_enabled(const void *blob, int node);
* if not.
*/
int fdtdec_check_fdt(void);
+
+/**
+ * Look up a phandle and follow it to its node. Then return the offset
+ * of that node.
+ *
+ * @param blob FDT blob
+ * @param node node to examine
+ * @param prop_namename of property to find
+ * @return node offset if found, -ve error code on error
+ */
+int fdtdec_lookup_phandle(const void *blob, int node, const char *prop_name);
+
+/**
+ * Look up a property in a node and return its contents in an integer
+ * array of given length. The property must have at least enough data for
+ * the array (4*count bytes). It may have more, but this will be ignored.
+ *
+ * @param blob FDT blob
+ * @param node node to examine
+ * @param prop_namename of property to find
+ * @param arrayarray to fill with data
+ * @param countnumber of array elements
+ * @return 0 if ok, or -FDT_ERR_NOTFOUND if the property is not found,
+ * or -FDT_ERR_BADLAYOUT if not enough data
+ */
+int fdtdec_get_int_array(const void *blob, int node, const char *prop_name,
+ u32 *array, int count);
+
+/**
+ * Look up a boolean property in a node and return it.
+ *
+ * A boolean properly is true if present in the device tree and false if not
+ * present, or present with a 0 value.
+ *
+ * @param blob FDT blob
+ * @param node node to examine
+ * @param prop_namename of property to find
+ * @return 1 if the properly is present; 0 if it isn't present or is 0
+ */
+int fdtdec_get_bool(const void *blob, int node, const char *prop_name);
diff --git a/lib/fdtdec.c b/lib/fdtdec.c
index a0a34e4..8f972b7 100644
--- a/lib/fdtdec.c
+++ b/lib/fdtdec.c
@@ -148,3 +148,86 @@ int fdtdec_check_fdt(void)
binary or define CONFIG_OF_EMBED\n);
return 0;
}
+
+int fdtdec_lookup_phandle(const void *blob, int node, const char *prop_name)
+{
+ const u32 *phandle;
+ int lookup;
+
+ phandle = fdt_getprop(blob, node, prop_name, NULL);
+ if (!phandle)
+ return -FDT_ERR_NOTFOUND;
+
+ lookup = fdt_node_offset_by_phandle(blob, fdt32_to_cpu(*phandle));
+ return lookup;
+}
+
+/**
+ * Look up a property in a node and check that it has a minimum length.
+ *
+ * @param blob FDT blob
+ * @param node node to examine
+ * @param prop_namename of property to find
+ * @param min_len minimum property length in bytes
+ * @param err 0 if ok, or -FDT_ERR_NOTFOUND if the property is not
+ found, or -FDT_ERR_BADLAYOUT if not enough data
+ * @return pointer to cell, which is only valid if err == 0
+ */
+static const void *get_prop_check_min_len(const void *blob, int node,
+ const char *prop_name, int min_len, int *err)
+{
+ const void *cell;
+ int len;
+
+ debug(%s: %s\n, __func__, prop_name);
+ cell = fdt_getprop(blob, node, prop_name, len);
+ if (!cell)
+ *err = -FDT_ERR_NOTFOUND;
+ else if (len min_len)
+ *err = -FDT_ERR_BADLAYOUT;
+ else
+ *err = 0;
+ return cell;
+}
+
+int fdtdec_get_int_array(const void *blob, int node, const char *prop_name,
+ u32 *array, int count)
+{
+ const u32 *cell;
+ int i, err = 0;
+
+ debug(%s: %s\n, __func__, prop_name);
+ cell = get_prop_check_min_len(blob, node, prop_name,
+ sizeof(u32) * count, err);
+ if (!err) {
+ for (i = 0; i count; i++)
+ array[i] = fdt32_to_cpu(cell[i]);
+ }
+ return err;
+}
+
+/**
+ * Look up a boolean property in a node and return it.
+ *
+ * A boolean properly is true if present in the device tree and false if not
+ * present, or present with a 0 value.
+ *
+ * @param blob FDT blob
+ * @param node node to examine
+ * @param prop_namename of property to find
+ * @return 1 if the properly is present; 0 if it isn't present or is 0
+ */
+int fdtdec_get_bool(const void *blob, int node, const char *prop_name)
+{
+ const s32 *cell;
+ int len;
+
+ debug(%s: %s\n, __func__, prop_name);
+ cell = fdt_getprop(blob, node, prop_name, len);
+ if (!cell)
+
