Re: [Xen-devel] [PATCH v5 1/7] xen/arm: pass node to device_tree_for_each_node

[Volodymyr Babchuk <Volodymyr_Babchuk@epam.com>]

Hi Stefano,

Stefano Stabellini writes:

> Add a new parameter to device_tree_for_each_node: node, the node to
> start the search from. Passing 0 triggers the old behavior.
>
> Set min_depth to depth of the current node + 1 and replace the for
> loop with a do/while loop to avoid scanning siblings of the initial node
> passed as an argument.
>
> 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 and the node itself.
>
> Signed-off-by: Stefano Stabellini <stefanos@xilinx.com>

You can have my

Reviewed-by: Volodymyr Babchuk <volodymyr.babchuk@epam.com>

providing that you'll fix formatting issue below.

> ---
> Changes in v5:
> - go back to v3
> - code style improvement in acpi/boot.c
> - improve comments and commit message
> - increase min_depth to avoid parsing siblings
> - replace for with do/while loop and increase min_depth to avoid
>   scanning siblings of the initial node
> - pass only node, calculate depth
>
> Changes in v3:
> - improve commit message
> - improve in-code comments
> - improve code style
>
> Changes in v2:
> - new
> ---
>  xen/arch/arm/acpi/boot.c      |  8 +++++---
>  xen/arch/arm/bootfdt.c        | 19 ++++++++++---------
>  xen/include/xen/device_tree.h |  6 +++---
>  3 files changed, 18 insertions(+), 15 deletions(-)
>
> diff --git a/xen/arch/arm/acpi/boot.c b/xen/arch/arm/acpi/boot.c
> index 9b29769a10..d4957cca06 100644
> --- a/xen/arch/arm/acpi/boot.c
> +++ b/xen/arch/arm/acpi/boot.c
> @@ -246,9 +246,11 @@ int __init acpi_boot_table_init(void)
>       * - the device tree is not empty (it has more than just a /chosen node)
>       *   and ACPI has not been force enabled (acpi=force)
>       */
> -    if ( param_acpi_off || ( !param_acpi_force
> -                             && device_tree_for_each_node(device_tree_flattened,
> -                                                   dt_scan_depth1_nodes, NULL)))
> +    if ( param_acpi_off)
> +        goto disable;
> +	if ( !param_acpi_force &&
> +		 device_tree_for_each_node(device_tree_flattened, 0,
> +			                       dt_scan_depth1_nodes, NULL) )
There is 3 tabs, followed by spaces.

This file missed emacs magic at the end. I think, this is cause for this
formatting issue.

>          goto disable;
>  
>      /*
> diff --git a/xen/arch/arm/bootfdt.c b/xen/arch/arm/bootfdt.c
> index 891b4b66ff..a872ea57d6 100644
> --- a/xen/arch/arm/bootfdt.c
> +++ b/xen/arch/arm/bootfdt.c
> @@ -77,6 +77,7 @@ static u32 __init device_tree_get_u32(const void *fdt, int node,
>  /**
>   * device_tree_for_each_node - iterate over all device tree nodes
>   * @fdt: flat device tree.
> + * @node: node to start the search from
>   * @func: function to call for each node.
>   * @data: data to pass to @func.
>   *
> @@ -85,20 +86,17 @@ static u32 __init device_tree_get_u32(const void *fdt, int node,
>   * Returns 0 if all nodes were iterated over successfully.  If @func
>   * returns a value different from 0, that value is returned immediately.
>   */
> -int __init device_tree_for_each_node(const void *fdt,
> +int __init device_tree_for_each_node(const void *fdt, int node,
>                                       device_tree_node_func func,
>                                       void *data)
>  {
> -    int node;
> -    int depth;
> +    int depth = fdt_node_depth(fdt, node);
> +    int min_depth = depth + 1;
>      u32 address_cells[DEVICE_TREE_MAX_DEPTH];
>      u32 size_cells[DEVICE_TREE_MAX_DEPTH];
>      int ret;
>  
> -    for ( node = 0, depth = 0;
> -          node >=0 && depth >= 0;
> -          node = fdt_next_node(fdt, node, &depth) )
> -    {
> +    do {
>          const char *name = fdt_get_name(fdt, node, NULL);
>          u32 as, ss;
>  
> @@ -120,7 +118,10 @@ int __init device_tree_for_each_node(const void *fdt,
>          ret = func(fdt, node, name, depth, as, ss, data);
>          if ( ret != 0 )
>              return ret;
> -    }
> +
> +        node = fdt_next_node(fdt, node, &depth);
> +    } while ( node >= 0 && depth >= min_depth );
> +
>      return 0;
>  }
>  
> @@ -357,7 +358,7 @@ size_t __init boot_fdt_info(const void *fdt, paddr_t paddr)
>  
>      add_boot_module(BOOTMOD_FDT, paddr, fdt_totalsize(fdt), false);
>  
> -    device_tree_for_each_node((void *)fdt, early_scan_node, NULL);
> +    device_tree_for_each_node((void *)fdt, 0, early_scan_node, NULL);
>      early_print_info();
>  
>      return fdt_totalsize(fdt);
> diff --git a/xen/include/xen/device_tree.h b/xen/include/xen/device_tree.h
> index 83156297e2..9a7a8f2dab 100644
> --- a/xen/include/xen/device_tree.h
> +++ b/xen/include/xen/device_tree.h
> @@ -158,9 +158,9 @@ typedef int (*device_tree_node_func)(const void *fdt,
>  
>  extern const void *device_tree_flattened;
>  
> -int device_tree_for_each_node(const void *fdt,
> -                                     device_tree_node_func func,
> -                                     void *data);
> +int device_tree_for_each_node(const void *fdt, int node,
> +                              device_tree_node_func func,
> +                              void *data);
>  
>  /**
>   * dt_unflatten_host_device_tree - Unflatten the host device tree


-- 
Volodymyr Babchuk at EPAM
_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel