From mboxrd@z Thu Jan 1 00:00:00 1970 From: Julien Grall Subject: Re: [PATCH v2] xen: arm: correct name of the dt node passed around when building dom0 DT Date: Fri, 22 Nov 2013 16:14:08 +0000 Message-ID: <528F82D0.3080506@linaro.org> References: <1385132821-23807-1-git-send-email-ian.campbell@citrix.com> <1385133918-30110-1-git-send-email-ian.campbell@citrix.com> Mime-Version: 1.0 Content-Type: text/plain; charset="us-ascii"; Format="flowed" Content-Transfer-Encoding: 7bit Return-path: In-Reply-To: <1385133918-30110-1-git-send-email-ian.campbell@citrix.com> List-Unsubscribe: , List-Post: List-Help: List-Subscribe: , Sender: xen-devel-bounces@lists.xen.org Errors-To: xen-devel-bounces@lists.xen.org To: Ian Campbell , xen-devel@lists.xen.org Cc: tim@xen.org, stefano.stabellini@eu.citrix.com List-Id: xen-devel@lists.xenproject.org On 11/22/2013 03:25 PM, Ian Campbell wrote: > In the case of the GIC, timer and write_properties the argument is the node, > not the parent. Rename the argument to "node" in this case. > > I think this stems from the use of the name "np" in other places (confusing > "node parent" for "node pointer"). Therefore replace all uses of "np" with > "node". > > In addition in write_properties now that np=>node the name pp makes no sense. > Rename it to "prop". > > No semantic change. > > Signed-off-by: Ian Campbell Acked-by: Julien Grall > --- > v2: Some of the "parents" were correct. > --- > xen/arch/arm/domain_build.c | 80 +++++++++++++++++++++---------------------- > 1 file changed, 40 insertions(+), 40 deletions(-) > > diff --git a/xen/arch/arm/domain_build.c b/xen/arch/arm/domain_build.c > index d79e038..e57cd10 100644 > --- a/xen/arch/arm/domain_build.c > +++ b/xen/arch/arm/domain_build.c > @@ -153,10 +153,10 @@ static void allocate_memory(struct domain *d, struct kernel_info *kinfo) > } > > static int write_properties(struct domain *d, struct kernel_info *kinfo, > - const struct dt_device_node *np) > + const struct dt_device_node *node) > { > const char *bootargs = NULL; > - const struct dt_property *pp; > + const struct dt_property *prop; > int res = 0; > int had_dom0_bootargs = 0; > > @@ -164,11 +164,11 @@ static int write_properties(struct domain *d, struct kernel_info *kinfo, > early_info.modules.module[MOD_KERNEL].cmdline[0] ) > bootargs = &early_info.modules.module[MOD_KERNEL].cmdline[0]; > > - dt_for_each_property_node (np, pp) > + dt_for_each_property_node (node, prop) > { > - const void *prop_data = pp->value; > + const void *prop_data = prop->value; > void *new_data = NULL; > - u32 prop_len = pp->length; > + u32 prop_len = prop->length; > > /* > * In chosen node: > @@ -178,28 +178,28 @@ static int write_properties(struct domain *d, struct kernel_info *kinfo, > * * remove bootargs, xen,dom0-bootargs, xen,xen-bootargs, > * linux,initrd-start and linux,initrd-end. > */ > - if ( dt_node_path_is_equal(np, "/chosen") ) > + if ( dt_node_path_is_equal(node, "/chosen") ) > { > - if ( dt_property_name_is_equal(pp, "xen,xen-bootargs") || > - dt_property_name_is_equal(pp, "linux,initrd-start") || > - dt_property_name_is_equal(pp, "linux,initrd-end") ) > + if ( dt_property_name_is_equal(prop, "xen,xen-bootargs") || > + dt_property_name_is_equal(prop, "linux,initrd-start") || > + dt_property_name_is_equal(prop, "linux,initrd-end") ) > continue; > > - if ( dt_property_name_is_equal(pp, "xen,dom0-bootargs") ) > + if ( dt_property_name_is_equal(prop, "xen,dom0-bootargs") ) > { > had_dom0_bootargs = 1; > - bootargs = pp->value; > + bootargs = prop->value; > continue; > } > - if ( dt_property_name_is_equal(pp, "bootargs") ) > + if ( dt_property_name_is_equal(prop, "bootargs") ) > { > if ( !bootargs && !had_dom0_bootargs ) > - bootargs = pp->value; > + bootargs = prop->value; > continue; > } > } > > - res = fdt_property(kinfo->fdt, pp->name, prop_data, prop_len); > + res = fdt_property(kinfo->fdt, prop->name, prop_data, prop_len); > > xfree(new_data); > > @@ -207,7 +207,7 @@ static int write_properties(struct domain *d, struct kernel_info *kinfo, > return res; > } > > - if ( dt_node_path_is_equal(np, "/chosen") && bootargs ) > + if ( dt_node_path_is_equal(node, "/chosen") && bootargs ) > { > res = fdt_property(kinfo->fdt, "bootargs", bootargs, > strlen(bootargs) + 1); > @@ -516,7 +516,7 @@ static int make_cpus_node(const struct domain *d, void *fdt, > } > > static int make_gic_node(const struct domain *d, void *fdt, > - const struct dt_device_node *parent) > + const struct dt_device_node *node) > { > const struct dt_device_node *gic = dt_interrupt_controller; > const void *compatible = NULL; > @@ -528,7 +528,7 @@ static int make_gic_node(const struct domain *d, void *fdt, > * Xen currently supports only a single GIC. Discard any secondary > * GIC entries. > */ > - if ( parent != dt_interrupt_controller ) > + if ( node != dt_interrupt_controller ) > { > DPRINT(" Skipping (secondary GIC)\n"); > return 0; > @@ -560,7 +560,7 @@ static int make_gic_node(const struct domain *d, void *fdt, > if ( res ) > return res; > > - len = dt_cells_to_size(dt_n_addr_cells(parent) + dt_n_size_cells(parent)); > + len = dt_cells_to_size(dt_n_addr_cells(node) + dt_n_size_cells(node)); > len *= 2; /* GIC has two memory regions: Distributor + CPU interface */ > new_cells = xzalloc_bytes(len); > if ( new_cells == NULL ) > @@ -569,11 +569,11 @@ static int make_gic_node(const struct domain *d, void *fdt, > tmp = new_cells; > DPRINT(" Set Distributor Base 0x%"PRIpaddr"-0x%"PRIpaddr"\n", > d->arch.vgic.dbase, d->arch.vgic.dbase + PAGE_SIZE - 1); > - dt_set_range(&tmp, parent, d->arch.vgic.dbase, PAGE_SIZE); > + dt_set_range(&tmp, node, d->arch.vgic.dbase, PAGE_SIZE); > > DPRINT(" Set Cpu Base 0x%"PRIpaddr"-0x%"PRIpaddr"\n", > d->arch.vgic.cbase, d->arch.vgic.cbase + (PAGE_SIZE * 2) - 1); > - dt_set_range(&tmp, parent, d->arch.vgic.cbase, PAGE_SIZE * 2); > + dt_set_range(&tmp, node, d->arch.vgic.cbase, PAGE_SIZE * 2); > > res = fdt_property(fdt, "reg", new_cells, len); > xfree(new_cells); > @@ -599,7 +599,7 @@ static int make_gic_node(const struct domain *d, void *fdt, > } > > static int make_timer_node(const struct domain *d, void *fdt, > - const struct dt_device_node *parent) > + const struct dt_device_node *node) > { > static const struct dt_device_match timer_ids[] __initconst = > { > @@ -740,7 +740,7 @@ static int map_device(struct domain *d, const struct dt_device_node *dev) > } > > static int handle_node(struct domain *d, struct kernel_info *kinfo, > - const struct dt_device_node *np) > + const struct dt_device_node *node) > { > static const struct dt_device_match skip_matches[] __initconst = > { > @@ -766,17 +766,17 @@ static int handle_node(struct domain *d, struct kernel_info *kinfo, > const char *name; > const char *path; > > - path = dt_node_full_name(np); > + path = dt_node_full_name(node); > > DPRINT("handle %s\n", path); > > /* Skip theses nodes and the sub-nodes */ > - if ( dt_match_node(skip_matches, np ) ) > + if ( dt_match_node(skip_matches, node ) ) > { > DPRINT(" Skip it (matched)\n"); > return 0; > } > - if ( platform_device_is_blacklisted(np) ) > + if ( platform_device_is_blacklisted(node) ) > { > DPRINT(" Skip it (blacklisted)\n"); > return 0; > @@ -784,13 +784,13 @@ static int handle_node(struct domain *d, struct kernel_info *kinfo, > > /* Replace these nodes with our own. Note that the original may be > * used_by DOMID_XEN so this check comes first. */ > - if ( dt_match_node(gic_matches, np ) ) > - return make_gic_node(d, kinfo->fdt, np); > - if ( dt_match_node(timer_matches, np ) ) > - return make_timer_node(d, kinfo->fdt, np); > + if ( dt_match_node(gic_matches, node ) ) > + return make_gic_node(d, kinfo->fdt, node); > + if ( dt_match_node(timer_matches, node ) ) > + return make_timer_node(d, kinfo->fdt, node); > > /* Skip nodes used by Xen */ > - if ( dt_device_used_by(np) == DOMID_XEN ) > + if ( dt_device_used_by(node) == DOMID_XEN ) > { > DPRINT(" Skip it (used by Xen)\n"); > return 0; > @@ -804,10 +804,10 @@ static int handle_node(struct domain *d, struct kernel_info *kinfo, > * property. Therefore these device doesn't need to be mapped. This > * solution can be use later for pass through. > */ > - if ( !dt_device_type_is_equal(np, "memory") && > - dt_device_is_available(np) ) > + if ( !dt_device_type_is_equal(node, "memory") && > + dt_device_is_available(node) ) > { > - res = map_device(d, np); > + res = map_device(d, node); > > if ( res ) > return res; > @@ -825,32 +825,32 @@ static int handle_node(struct domain *d, struct kernel_info *kinfo, > if ( res ) > return res; > > - res = write_properties(d, kinfo, np); > + res = write_properties(d, kinfo, node); > if ( res ) > return res; > > - for ( child = np->child; child != NULL; child = child->sibling ) > + for ( child = node->child; child != NULL; child = child->sibling ) > { > res = handle_node(d, kinfo, child); > if ( res ) > return res; > } > > - if ( np == dt_host ) > + if ( node == dt_host ) > { > - res = make_hypervisor_node(d, kinfo->fdt, np); > + res = make_hypervisor_node(d, kinfo->fdt, node); > if ( res ) > return res; > > - res = make_psci_node(kinfo->fdt, np); > + res = make_psci_node(kinfo->fdt, node); > if ( res ) > return res; > > - res = make_cpus_node(d, kinfo->fdt, np); > + res = make_cpus_node(d, kinfo->fdt, node); > if ( res ) > return res; > > - res = make_memory_node(d, kinfo->fdt, np, kinfo); > + res = make_memory_node(d, kinfo->fdt, node, kinfo); > if ( res ) > return res; > > -- Julien Grall