From: Julien Grall <julien.grall@arm.com> To: Stefano Stabellini <sstabellini@kernel.org>, xen-devel@lists.xenproject.org Cc: Stefano Stabellini <stefanos@xilinx.com> Subject: Re: [PATCH v2 06/10] xen/arm: extend device_tree_for_each_node Date: Tue, 7 May 2019 18:12:32 +0100 [thread overview] Message-ID: <885fc62e-dbe2-bb8f-1476-de5b5d7df2c8@arm.com> (raw) In-Reply-To: <1556658172-8824-6-git-send-email-sstabellini@kernel.org> Hi Stefano, On 4/30/19 10:02 PM, Stefano Stabellini wrote: > Add two new paramters to device_tree_for_each_node: node and depth. NIT: s/paramters/parameters/ > Node is the node to start the search from and depth is the min depth of > the search. > > Passing 0, 0 triggers the old behavior. It would be good to explain in the commit message why we need this. > > Signed-off-by: Stefano Stabellini <stefanos@xilinx.com> > --- > Changes in v2: > - new > --- > xen/arch/arm/acpi/boot.c | 2 +- > xen/arch/arm/bootfdt.c | 12 ++++++------ > xen/include/xen/device_tree.h | 5 +++-- > 3 files changed, 10 insertions(+), 9 deletions(-) > > diff --git a/xen/arch/arm/acpi/boot.c b/xen/arch/arm/acpi/boot.c > index 9b29769..cfc85c2 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, dt_scan_depth1_nodes, NULL))) > goto disable; > > /* > diff --git a/xen/arch/arm/bootfdt.c b/xen/arch/arm/bootfdt.c > index 891b4b6..e7b08ed 100644 > --- a/xen/arch/arm/bootfdt.c > +++ b/xen/arch/arm/bootfdt.c > @@ -77,6 +77,8 @@ 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 > + * @depth: min depth of the search The interface is not clear, which node is it? The parent node or the first child? Similarly, which depth is it? But then, is the depth really necessary? You basically want to browse all the child of the parent node. > * @func: function to call for each node. > * @data: data to pass to @func. > * > @@ -86,17 +88,15 @@ static u32 __init device_tree_get_u32(const void *fdt, int node, > * returns a value different from 0, that value is returned immediately. > */ > int __init device_tree_for_each_node(const void *fdt, > + int node, int depth, > device_tree_node_func func, > void *data) > { > - int node; > - int depth; > u32 address_cells[DEVICE_TREE_MAX_DEPTH]; > u32 size_cells[DEVICE_TREE_MAX_DEPTH]; > - int ret; > + int ret, min_depth = depth; > > - for ( node = 0, depth = 0; > - node >=0 && depth >= 0; > + for ( ; node >=0 && depth >= min_depth; NIT: While you modify the code, can you please add the missing space between > and 0? Also, the code below is looking at {address, size}_cells[depth - 1]. On the first loop, they will not be initialized and will contain garbage. Note that with my suggestion about dropping the parameter depth, the address/size cells would still be wrongly initialized. > node = fdt_next_node(fdt, node, &depth) ) > { > const char *name = fdt_get_name(fdt, node, NULL); > @@ -357,7 +357,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, 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 7408a6c..4ff78ba 100644 > --- a/xen/include/xen/device_tree.h > +++ b/xen/include/xen/device_tree.h > @@ -159,8 +159,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 node, int depth, > + device_tree_node_func func, > + void *data); > > /** > * dt_unflatten_host_device_tree - Unflatten the host device tree > Cheers, -- Julien Grall _______________________________________________ Xen-devel mailing list Xen-devel@lists.xenproject.org https://lists.xenproject.org/mailman/listinfo/xen-devel
WARNING: multiple messages have this Message-ID (diff)
From: Julien Grall <julien.grall@arm.com> To: Stefano Stabellini <sstabellini@kernel.org>, xen-devel@lists.xenproject.org Cc: Stefano Stabellini <stefanos@xilinx.com> Subject: Re: [Xen-devel] [PATCH v2 06/10] xen/arm: extend device_tree_for_each_node Date: Tue, 7 May 2019 18:12:32 +0100 [thread overview] Message-ID: <885fc62e-dbe2-bb8f-1476-de5b5d7df2c8@arm.com> (raw) Message-ID: <20190507171232.n6-NszDI4ZqP5r4CSrK3UNdEeawxp9xpcHyWIzD_idU@z> (raw) In-Reply-To: <1556658172-8824-6-git-send-email-sstabellini@kernel.org> Hi Stefano, On 4/30/19 10:02 PM, Stefano Stabellini wrote: > Add two new paramters to device_tree_for_each_node: node and depth. NIT: s/paramters/parameters/ > Node is the node to start the search from and depth is the min depth of > the search. > > Passing 0, 0 triggers the old behavior. It would be good to explain in the commit message why we need this. > > Signed-off-by: Stefano Stabellini <stefanos@xilinx.com> > --- > Changes in v2: > - new > --- > xen/arch/arm/acpi/boot.c | 2 +- > xen/arch/arm/bootfdt.c | 12 ++++++------ > xen/include/xen/device_tree.h | 5 +++-- > 3 files changed, 10 insertions(+), 9 deletions(-) > > diff --git a/xen/arch/arm/acpi/boot.c b/xen/arch/arm/acpi/boot.c > index 9b29769..cfc85c2 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, dt_scan_depth1_nodes, NULL))) > goto disable; > > /* > diff --git a/xen/arch/arm/bootfdt.c b/xen/arch/arm/bootfdt.c > index 891b4b6..e7b08ed 100644 > --- a/xen/arch/arm/bootfdt.c > +++ b/xen/arch/arm/bootfdt.c > @@ -77,6 +77,8 @@ 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 > + * @depth: min depth of the search The interface is not clear, which node is it? The parent node or the first child? Similarly, which depth is it? But then, is the depth really necessary? You basically want to browse all the child of the parent node. > * @func: function to call for each node. > * @data: data to pass to @func. > * > @@ -86,17 +88,15 @@ static u32 __init device_tree_get_u32(const void *fdt, int node, > * returns a value different from 0, that value is returned immediately. > */ > int __init device_tree_for_each_node(const void *fdt, > + int node, int depth, > device_tree_node_func func, > void *data) > { > - int node; > - int depth; > u32 address_cells[DEVICE_TREE_MAX_DEPTH]; > u32 size_cells[DEVICE_TREE_MAX_DEPTH]; > - int ret; > + int ret, min_depth = depth; > > - for ( node = 0, depth = 0; > - node >=0 && depth >= 0; > + for ( ; node >=0 && depth >= min_depth; NIT: While you modify the code, can you please add the missing space between > and 0? Also, the code below is looking at {address, size}_cells[depth - 1]. On the first loop, they will not be initialized and will contain garbage. Note that with my suggestion about dropping the parameter depth, the address/size cells would still be wrongly initialized. > node = fdt_next_node(fdt, node, &depth) ) > { > const char *name = fdt_get_name(fdt, node, NULL); > @@ -357,7 +357,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, 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 7408a6c..4ff78ba 100644 > --- a/xen/include/xen/device_tree.h > +++ b/xen/include/xen/device_tree.h > @@ -159,8 +159,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 node, int depth, > + device_tree_node_func func, > + void *data); > > /** > * dt_unflatten_host_device_tree - Unflatten the host device tree > Cheers, -- Julien Grall _______________________________________________ Xen-devel mailing list Xen-devel@lists.xenproject.org https://lists.xenproject.org/mailman/listinfo/xen-devel
next prev parent reply other threads:[~2019-05-07 17:12 UTC|newest] Thread overview: 86+ messages / expand[flat|nested] mbox.gz Atom feed top 2019-04-30 21:02 [PATCH v2 0/10] iomem memory policy Stefano Stabellini 2019-04-30 21:02 ` [Xen-devel] " Stefano Stabellini 2019-04-30 21:02 ` [PATCH v2 01/10] xen: add a p2mt parameter to map_mmio_regions Stefano Stabellini 2019-04-30 21:02 ` [Xen-devel] " Stefano Stabellini 2019-05-02 14:59 ` Jan Beulich 2019-05-02 14:59 ` [Xen-devel] " Jan Beulich 2019-05-02 18:49 ` Stefano Stabellini 2019-05-02 18:49 ` [Xen-devel] " Stefano Stabellini 2019-05-15 13:39 ` Oleksandr 2019-05-15 13:39 ` [Xen-devel] " Oleksandr 2019-04-30 21:02 ` [PATCH v2 02/10] xen: rename un/map_mmio_regions to un/map_regions Stefano Stabellini 2019-04-30 21:02 ` [Xen-devel] " Stefano Stabellini 2019-05-01 9:22 ` Julien Grall 2019-05-01 9:22 ` [Xen-devel] " Julien Grall 2019-06-17 21:24 ` Stefano Stabellini 2019-06-18 11:05 ` Julien Grall 2019-06-18 20:19 ` Stefano Stabellini 2019-05-02 15:03 ` Jan Beulich 2019-05-02 15:03 ` [Xen-devel] " Jan Beulich 2019-05-02 18:55 ` Stefano Stabellini 2019-05-02 18:55 ` [Xen-devel] " Stefano Stabellini 2019-04-30 21:02 ` [PATCH v2 03/10] xen: extend XEN_DOMCTL_memory_mapping to handle memory policy Stefano Stabellini 2019-04-30 21:02 ` [Xen-devel] " Stefano Stabellini 2019-05-02 15:12 ` Jan Beulich 2019-05-02 15:12 ` [Xen-devel] " Jan Beulich 2019-06-17 21:28 ` Stefano Stabellini 2019-06-18 8:59 ` Jan Beulich 2019-06-18 20:32 ` Stefano Stabellini 2019-06-18 23:15 ` Stefano Stabellini 2019-06-19 6:53 ` Jan Beulich 2019-05-07 16:41 ` Julien Grall 2019-05-07 16:41 ` [Xen-devel] " Julien Grall 2019-06-17 22:43 ` Stefano Stabellini 2019-06-18 11:13 ` Julien Grall 2019-05-15 14:40 ` Oleksandr 2019-05-15 14:40 ` [Xen-devel] " Oleksandr 2019-04-30 21:02 ` [PATCH v2 04/10] libxc: introduce xc_domain_mem_map_policy Stefano Stabellini 2019-04-30 21:02 ` [Xen-devel] " Stefano Stabellini 2019-04-30 21:02 ` [PATCH v2 05/10] libxl/xl: add memory policy option to iomem Stefano Stabellini 2019-04-30 21:02 ` [Xen-devel] " Stefano Stabellini 2019-05-01 9:42 ` Julien Grall 2019-05-01 9:42 ` [Xen-devel] " Julien Grall 2019-06-17 22:32 ` Stefano Stabellini 2019-06-18 11:09 ` Julien Grall 2019-06-18 11:15 ` Julien Grall 2019-06-18 22:07 ` Stefano Stabellini 2019-06-18 22:20 ` Julien Grall 2019-06-18 22:46 ` Stefano Stabellini 2019-04-30 21:02 ` [PATCH v2 06/10] xen/arm: extend device_tree_for_each_node Stefano Stabellini 2019-04-30 21:02 ` [Xen-devel] " Stefano Stabellini 2019-05-07 17:12 ` Julien Grall [this message] 2019-05-07 17:12 ` Julien Grall 2019-04-30 21:02 ` [PATCH v2 07/10] xen/arm: make process_memory_node a device_tree_node_func Stefano Stabellini 2019-04-30 21:02 ` [Xen-devel] " Stefano Stabellini 2019-05-01 9:47 ` Julien Grall 2019-05-01 9:47 ` [Xen-devel] " Julien Grall 2019-04-30 21:02 ` [PATCH v2 08/10] xen/arm: keep track of reserved-memory regions Stefano Stabellini 2019-04-30 21:02 ` [Xen-devel] " Stefano Stabellini 2019-05-01 10:03 ` Julien Grall 2019-05-01 10:03 ` [Xen-devel] " Julien Grall 2019-06-21 23:47 ` Stefano Stabellini 2019-05-07 17:21 ` Julien Grall 2019-05-07 17:21 ` [Xen-devel] " Julien Grall 2019-04-30 21:02 ` [PATCH v2 09/10] xen/arm: map reserved-memory regions as normal memory in dom0 Stefano Stabellini 2019-04-30 21:02 ` [Xen-devel] " Stefano Stabellini 2019-05-07 19:52 ` Julien Grall 2019-05-07 19:52 ` [Xen-devel] " Julien Grall 2019-04-30 21:02 ` [PATCH v2 10/10] xen/arm: add reserved-memory regions to the dom0 memory node Stefano Stabellini 2019-04-30 21:02 ` [Xen-devel] " Stefano Stabellini 2019-05-07 20:15 ` Julien Grall 2019-05-07 20:15 ` [Xen-devel] " Julien Grall 2019-05-10 20:51 ` Stefano Stabellini 2019-05-10 20:51 ` [Xen-devel] " Stefano Stabellini 2019-05-10 21:43 ` Julien Grall 2019-05-10 21:43 ` [Xen-devel] " Julien Grall 2019-05-11 12:40 ` Julien Grall 2019-05-11 12:40 ` [Xen-devel] " Julien Grall 2019-05-20 21:26 ` Stefano Stabellini 2019-05-20 21:26 ` [Xen-devel] " Stefano Stabellini 2019-05-20 22:38 ` Julien Grall 2019-05-20 22:38 ` [Xen-devel] " Julien Grall 2019-06-05 16:30 ` Julien Grall 2019-06-21 23:47 ` Stefano Stabellini 2019-05-16 16:52 ` [PATCH v2 0/10] iomem memory policy Oleksandr 2019-05-16 16:52 ` [Xen-devel] " Oleksandr 2019-06-21 23:48 ` 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=885fc62e-dbe2-bb8f-1476-de5b5d7df2c8@arm.com \ --to=julien.grall@arm.com \ --cc=sstabellini@kernel.org \ --cc=stefanos@xilinx.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: linkBe 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).