From: Stefano Stabellini <sstabellini@kernel.org>
To: Volodymyr Babchuk <Volodymyr_Babchuk@epam.com>
Cc: "xen-devel@lists.xenproject.org" <xen-devel@lists.xenproject.org>,
"julien.grall@arm.com" <julien.grall@arm.com>,
Stefano Stabellini <sstabellini@kernel.org>
Subject: Re: [Xen-devel] [PATCH v5 1/7] xen/arm: pass node to device_tree_for_each_node
Date: Wed, 14 Aug 2019 15:12:37 -0700 (PDT) [thread overview]
Message-ID: <alpine.DEB.2.21.1908141511510.8737@sstabellini-ThinkPad-T480s> (raw)
In-Reply-To: <87h86lgovx.fsf@epam.com>
On Tue, 13 Aug 2019, Volodymyr Babchuk wrote:
> 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.
Thank you! I'll fix the formatting issue, but won't add reviewed-by for
now as I'll change the function a bit to skip calling func() on the
first node.
> > ---
> > 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
next prev parent reply other threads:[~2019-08-14 22:12 UTC|newest]
Thread overview: 38+ messages / expand[flat|nested] mbox.gz Atom feed top
2019-08-12 22:28 [Xen-devel] [PATCH v5 0/7] reserved-memory in dom0 Stefano Stabellini
2019-08-12 22:28 ` [Xen-devel] [PATCH v5 1/7] xen/arm: pass node to device_tree_for_each_node Stefano Stabellini
2019-08-13 13:45 ` Volodymyr Babchuk
2019-08-14 22:12 ` Stefano Stabellini [this message]
2019-08-13 17:25 ` Julien Grall
2019-08-14 22:11 ` Stefano Stabellini
2019-08-12 22:28 ` [Xen-devel] [PATCH v5 2/7] xen/arm: make process_memory_node a device_tree_node_func Stefano Stabellini
2019-08-13 14:14 ` Volodymyr Babchuk
2019-08-14 22:35 ` Stefano Stabellini
2019-08-15 9:12 ` Julien Grall
2019-08-15 11:20 ` Volodymyr Babchuk
2019-08-15 11:24 ` Julien Grall
2019-08-15 11:29 ` Julien Grall
2019-08-15 12:14 ` Volodymyr Babchuk
2019-08-15 12:33 ` Julien Grall
2019-08-15 13:51 ` Volodymyr Babchuk
2019-08-15 14:15 ` Julien Grall
2019-08-13 17:37 ` Julien Grall
2019-08-14 22:54 ` Stefano Stabellini
2019-08-12 22:28 ` [Xen-devel] [PATCH v5 3/7] xen/arm: keep track of reserved-memory regions Stefano Stabellini
2019-08-13 14:23 ` Volodymyr Babchuk
2019-08-13 14:46 ` Julien Grall
2019-08-13 15:14 ` Volodymyr Babchuk
2019-08-13 15:15 ` Julien Grall
2019-08-13 15:39 ` Volodymyr Babchuk
2019-08-14 12:48 ` Julien Grall
2019-08-12 22:28 ` [Xen-devel] [PATCH v5 4/7] xen/arm: early_print_info print reserved_mem Stefano Stabellini
2019-08-13 14:28 ` Volodymyr Babchuk
2019-08-13 14:47 ` Julien Grall
2019-08-14 22:21 ` Stefano Stabellini
2019-08-14 12:52 ` Julien Grall
2019-08-12 22:28 ` [Xen-devel] [PATCH v5 5/7] xen/arm: handle reserved-memory in consider_modules and dt_unreserved_regions Stefano Stabellini
2019-08-12 22:28 ` [Xen-devel] [PATCH v5 6/7] xen/arm: don't iomem_permit_access for reserved-memory regions Stefano Stabellini
2019-08-13 14:34 ` Volodymyr Babchuk
2019-08-13 14:55 ` Julien Grall
2019-08-14 22:40 ` Stefano Stabellini
2019-08-15 9:15 ` Julien Grall
2019-08-12 22:28 ` [Xen-devel] [PATCH v5 7/7] xen/arm: add reserved-memory regions to the dom0 memory node Stefano Stabellini
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=alpine.DEB.2.21.1908141511510.8737@sstabellini-ThinkPad-T480s \
--to=sstabellini@kernel.org \
--cc=Volodymyr_Babchuk@epam.com \
--cc=julien.grall@arm.com \
--cc=xen-devel@lists.xenproject.org \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).