[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [Minios-devel] [UNIKRAFT PATCHv4 4/9] plat/common: Introduce fdt_{address, size}_cells_or_parent helpers
|
Hi Sharan
After I dig more on it, I would like to rework this patch 4/9.
Please see the comments from libfdt author [1]
"
* #address-cells and #size-cells describe the format of addresses
for children of this node, not this node itself. So if you're
looking to parse 'reg' for this node, you *always* need to look at
the parent, not just as a fallback.
* #address-cells and #size-cells are *not* inherited. If they're
missing in a node, then the format for its children's addresses is
2 cell addresses and 2 cell sizes, it is *not* correct to look at
the next parent up for these properties.
"
So, seems I should
parent = fdt_parent_offset(fdt, nodeoffset);
naddr = fdt_address_cells(parent, nodeoffset);
nsize = fdt_size_cells(parent, nodeoffset);
[1] https://lists.denx.de/pipermail/u-boot/2016-March/248354.html
On 2019/4/9 1:29, Sharan Santhanam
wrote:
Hello
Jia He,
Please find the comments inline:
Thanks & Regards
Sharan
On 3/27/19 3:34 AM, Jia He wrote:
In some cases, devices need to find the
"#address-cells" "size-cells"
properties recursively(node, parent, grandparent...).
This patch introduces two helpers to simplify the codes
Signed-off-by: Jia He <justin.he@xxxxxxx>
---
plat/common/fdt.c | 57
+++++++++++++++++++++++++++++++++++++++
plat/common/include/fdt.h | 34 +++++++++++++++++++++++
2 files changed, 91 insertions(+)
diff --git a/plat/common/fdt.c b/plat/common/fdt.c
index 916a263..6feec0a 100644
--- a/plat/common/fdt.c
+++ b/plat/common/fdt.c
@@ -99,3 +99,60 @@ int fdt_interrupt_cells(const void *fdt, int
offset)
return val;
}
Would rename this function fdt_address_cell_get or
fdt_address_cell?
+ +int fdt_address_cells_or_parent(const
void *fdt, int nodeoffset)
+{
+ int prop_len;
+ const fdt32_t *prop;
+
+ do {
The specification[1] states "The #address-cells
property defines the number of <u32> cells used to encode
the
address field in a child node's reg property."
In our case shouldn't the operation be fdt_parent_offset and
then fdt_getprop?
+ /* Find whether the property
exists in this node */
+ prop = fdt_getprop(fdt, nodeoffset,
+ "#address-cells", &prop_len);
+ /* If not, try to find in parent node */
+ nodeoffset = fdt_parent_offset(fdt, nodeoffset);
+ } while (nodeoffset >= 0 && prop == NULL);
+
+ /* as per DT spec, set the address-cells to 2 if no such
+ * property anywhere in this node or parent
+ */
+ if (prop == NULL)
+ return 2;
+
When would this condition happen? If prop is not null why would we
have to check this
+ if (nodeoffset < 0)
+ return nodeoffset;
+
+ if (prop_len >= (int)sizeof(fdt32_t))
+ return fdt32_to_cpu(prop[0]);
+
+ return -FDT_ERR_NOTFOUND;
+}
+
Would rename this function fdt_size_cell_get or fdt_size_cell?
+int fdt_size_cells_or_parent(const void
*fdt, int nodeoffset)
+{
+ int prop_len;
+ const fdt32_t *prop;
+
+ do {
The specification[1] states "The #size-cells
property defines the number of <u32> cells used to encode
the
size field in a child node's reg property."
In our case shouldn't the operation be fdt_parent_offset and
then fdt_getprop?
+ /* Find whether the property
exists in this node */
+ prop = fdt_getprop(fdt, nodeoffset,
+ "#size-cells", &prop_len);
+ break;
+ /* If not, try to find in parent node */
+ nodeoffset = fdt_parent_offset(fdt, nodeoffset);
+ } while (nodeoffset >= 0 && prop == NULL);
+
+ /* as per DT spec, set the size-cells to 1 if no such
+ * property anywhere in this node or parent
+ */
+ if (prop == NULL)
+ return 1;
+
When would this condition happen? If prop is not null why would we
have to check this
+ if (nodeoffset < 0)
+ return nodeoffset;
+
+ if (prop_len >= (int)sizeof(fdt32_t))
+ return fdt32_to_cpu(prop[0]);
+
+ return -FDT_ERR_NOTFOUND;
+}
diff --git a/plat/common/include/fdt.h
b/plat/common/include/fdt.h
index afc4753..969cd28 100644
--- a/plat/common/include/fdt.h
+++ b/plat/common/include/fdt.h
@@ -76,4 +76,38 @@ int fdt_getprop_u32_by_offset(const void
*fdt, int nodeoffset,
*/
int fdt_interrupt_cells(const void *fdt, int nodeoffset);
+/**
+ * fdt_address_cells_or_parent - retrieve cell size for a bus
represented
+ * in the tree
+ * @fdt: pointer to the device tree blob
There is no prop parameter
+ * @prop: cell name of the property
containing the string list
+ * @nodeoffset: offset of the node to find the address size for
+ *
+ * When the node or parent has a valid #address-cells property,
returns
+ * its value.
+ *
+ * returns:
+ * 0 <= n < FDT_MAX_NCELLS, on success
Error return values do not match
+ * -FDT_ERR_BADNCELLS, if the node
has a badly formatted or invalid
+ * #address-cells property
+ */
+int fdt_address_cells_or_parent(const void *fdt, int
nodeoffset);
+
+/**
+ * fdt_size_cells_or_parent - retrieve cell size for a bus
represented
+ * in the tree
+ * @fdt: pointer to the device tree blob
There is no prop parameter
+ * @prop: cell name of the property
containing the string list
+ * @nodeoffset: offset of the node to find the address size for
+ *
+ * When the node or parent has a valid #size-cells property,
returns its
+ * value.
+ *
+ * returns:
+ * 0 <= n < FDT_MAX_NCELLS, on success
Error return values do not match
+ * -FDT_ERR_BADNCELLS, if the node
has a badly formatted or invalid
+ * #address-cells property
+ */
+int fdt_size_cells_or_parent(const void *fdt, int nodeoffset);
+
#endif
[1]
https://github.com/devicetree-org/devicetree-specification/blob/master/source/devicetree-basics.rst
_______________________________________________
Minios-devel mailing list
Minios-devel@xxxxxxxxxxxxxxxxxxxx
https://lists.xenproject.org/mailman/listinfo/minios-devel
--
---
Cheers,
Justin (Jia He)
|
_______________________________________________
Minios-devel mailing list
Minios-devel@xxxxxxxxxxxxxxxxxxxx
https://lists.xenproject.org/mailman/listinfo/minios-devel
|
