[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

Re: [Xen-devel] [PATCH v4 1/7] xen/arm: extend device_tree_for_each_node



Hi Stefano,

On 06/08/2019 22:49, Stefano Stabellini wrote:
Add new parameters to device_tree_for_each_node: node, depth,
address_cells, size_cells.

address_cells (resp. size_cells) are named address_cells_p (resp. size_cells_p) in the code.

But I am not convinced you need them. Per the DT spec (v0.2 section 2.3.5), the parent should either contain the #address-cells and #size-cells or default values (resp. 2 and 1) will be used. It is clearly stated that values are not inherited from the ancestors.

So technically the implementation of device_tree_for_each_node() is incorrect. If you follow the spec here, then the address_cells_p and size_cells_p would become unnecessary.

Node is the parent node to start the search from;
depth is the min depth of the search (the depth of the parent node);
address_cells and size_cells are the respective parameters at the parent
node. Passing 0, 0, 0, 0 triggers the old behavior. >
We need this change because in follow-up patches we want to be able to
use reuse device_tree_for_each_node to call a function for each children
nodes of a provided node.

Signed-off-by: Stefano Stabellini <stefanos@xxxxxxxxxx>
---
Changes in v4:
- add address_cells and size_cells parameters

Changes in v3:
- improve commit message
- improve in-code comments
- improve code style

Changes in v2:
- new
---
  xen/arch/arm/acpi/boot.c      |  2 +-
  xen/arch/arm/bootfdt.c        | 21 +++++++++++++++------
  xen/include/xen/device_tree.h |  6 ++++--
  3 files changed, 20 insertions(+), 9 deletions(-)

diff --git a/xen/arch/arm/acpi/boot.c b/xen/arch/arm/acpi/boot.c
index 9b29769a10..d275f8c535 100644
--- a/xen/arch/arm/acpi/boot.c
+++ b/xen/arch/arm/acpi/boot.c
@@ -248,7 +248,7 @@ int __init acpi_boot_table_init(void)
       */
      if ( param_acpi_off || ( !param_acpi_force
                               && 
device_tree_for_each_node(device_tree_flattened,
-                                                   dt_scan_depth1_nodes, 
NULL)))
+                                 0, 0, 0, 0, dt_scan_depth1_nodes, NULL)))

NIT: Can we split the if?

Cheers,

--
Julien Grall

_______________________________________________
Xen-devel mailing list
Xen-devel@xxxxxxxxxxxxxxxxxxxx
https://lists.xenproject.org/mailman/listinfo/xen-devel

 


Rackspace

Lists.xenproject.org is hosted with RackSpace, monitoring our
servers 24x7x365 and backed by RackSpace's Fanatical Support®.