Ian Campbell
2013-Nov-22 15:07 UTC
[PATCH v2] xen: arm: remove /xen-core-devices node from dom0 dtb
The intention of adding this node to contain the GIC, timer and memory nodes
(in 1c08d6004ea7) was to allow us to control the #address-cells and
However in the case of the memory node the #*-cells are always taken from the
root node (see ePAPR 3.1, "the following nodes shall be present at the
root...
memory node"). This caused breakage on the arndale platform.
In addition it is not valid to just create sub-nodes like this. Unless they
declare themselves as a bus then they will not necessarily be enumerated
(although Linux currently does so in practice).
Therefore:
- Move the memory node back to the top level.
- Insert the timer and primary gic nodes in the same location as the host
DTB, replacing the originals. Note that the nodes here may be marked as in
use by Xen and therefore the check must be before we discard nodes used by
Xen.
- Drop any secondary gics.
Signed-off-by: Ian Campbell <ian.campbell@citrix.com>
---
v2:
Discard secondary gics.
Left parameter as "parent" for now since that is used everywhere.
---
xen/arch/arm/domain_build.c | 127 +++++++++++++++++++------------------------
1 file changed, 55 insertions(+), 72 deletions(-)
diff --git a/xen/arch/arm/domain_build.c b/xen/arch/arm/domain_build.c
index aa7e3d2..d79e038 100644
--- a/xen/arch/arm/domain_build.c
+++ b/xen/arch/arm/domain_build.c
@@ -48,24 +48,6 @@ custom_param("dom0_mem", parse_dom0_mem);
*/
#define DOM0_FDT_EXTRA_SIZE (128 + sizeof(struct fdt_reserve_entry))
-/*
- * Number of cells used for addresses and sizes under the /xen/
- * node.
- *
- * We don''t have a struct dt_device_node we can reference as a parent
- * to get these values, so use these constants instead.
- */
-#define XEN_FDT_NODE_ADDRESS_CELLS 2
-#define XEN_FDT_NODE_SIZE_CELLS 2
-#define XEN_FDT_NODE_REG_SIZE \
- dt_cells_to_size(XEN_FDT_NODE_ADDRESS_CELLS + XEN_FDT_NODE_SIZE_CELLS)
-
-static void set_xen_range(__be32 **cellp, u64 address, u64 size)
-{
- dt_set_cell(cellp, XEN_FDT_NODE_ADDRESS_CELLS, address);
- dt_set_cell(cellp, XEN_FDT_NODE_SIZE_CELLS, size);
-}
-
struct vcpu *__init alloc_dom0_vcpu0(void)
{
if ( opt_dom0_max_vcpus == 0 )
@@ -294,14 +276,16 @@ static int fdt_property_interrupts(void *fdt,
gic_interrupt_t *intr,
static int make_memory_node(const struct domain *d,
void *fdt,
+ const struct dt_device_node *parent,
const struct kernel_info *kinfo)
{
int res, i;
- int nr_cells = XEN_FDT_NODE_REG_SIZE*kinfo->mem.nr_banks;
+ int reg_size = dt_n_addr_cells(parent) + dt_n_size_cells(parent);
+ int nr_cells = reg_size*kinfo->mem.nr_banks;
__be32 reg[nr_cells];
__be32 *cells;
- DPRINT("Create memory node\n");
+ DPRINT("Create memory node (reg size %d, nr cells %d)\n",
reg_size, nr_cells);
/* ePAPR 3.4 */
res = fdt_begin_node(fdt, "memory");
@@ -321,10 +305,10 @@ static int make_memory_node(const struct domain *d,
DPRINT(" Bank %d:
%#"PRIx64"->%#"PRIx64"\n",
i, start, start + size);
- set_xen_range(&cells, start, size);
+ dt_set_range(&cells, parent, start, size);
}
- res = fdt_property(fdt, "reg", reg, nr_cells);
+ res = fdt_property(fdt, "reg", reg, sizeof(reg));
if ( res )
return res;
@@ -531,7 +515,8 @@ static int make_cpus_node(const struct domain *d, void *fdt,
return res;
}
-static int make_gic_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 *gic = dt_interrupt_controller;
const void *compatible = NULL;
@@ -539,6 +524,16 @@ static int make_gic_node(const struct domain *d, void *fdt)
__be32 *new_cells, *tmp;
int res = 0;
+ /*
+ * Xen currently supports only a single GIC. Discard any secondary
+ * GIC entries.
+ */
+ if ( parent != dt_interrupt_controller )
+ {
+ DPRINT(" Skipping (secondary GIC)\n");
+ return 0;
+ }
+
DPRINT("Create gic node\n");
compatible = dt_get_property(gic, "compatible", &len);
@@ -565,7 +560,8 @@ static int make_gic_node(const struct domain *d, void *fdt)
if ( res )
return res;
- len = XEN_FDT_NODE_REG_SIZE * 2;
+ len = dt_cells_to_size(dt_n_addr_cells(parent) + dt_n_size_cells(parent));
+ len *= 2; /* GIC has two memory regions: Distributor + CPU interface */
new_cells = xzalloc_bytes(len);
if ( new_cells == NULL )
return -FDT_ERR_XEN(ENOMEM);
@@ -573,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);
- set_xen_range(&tmp, d->arch.vgic.dbase, PAGE_SIZE);
+ dt_set_range(&tmp, parent, 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);
- set_xen_range(&tmp, d->arch.vgic.cbase, PAGE_SIZE * 2);
+ dt_set_range(&tmp, parent, d->arch.vgic.cbase, PAGE_SIZE * 2);
res = fdt_property(fdt, "reg", new_cells, len);
xfree(new_cells);
@@ -602,7 +598,8 @@ static int make_gic_node(const struct domain *d, void *fdt)
return res;
}
-static int make_timer_node(const struct domain *d, void *fdt)
+static int make_timer_node(const struct domain *d, void *fdt,
+ const struct dt_device_node *parent)
{
static const struct dt_device_match timer_ids[] __initconst {
@@ -662,46 +659,6 @@ static int make_timer_node(const struct domain *d, void
*fdt)
return res;
}
-static int make_xen_node(const struct domain *d, void *fdt,
- const struct dt_device_node *parent,
- const struct kernel_info *kinfo)
-{
- int res;
-
- res = fdt_begin_node(fdt, "xen-core-devices");
- if ( res )
- return res;
-
- res = fdt_property_cell(fdt, "#address-cells",
- XEN_FDT_NODE_ADDRESS_CELLS);
- if ( res )
- return res;
-
- res = fdt_property_cell(fdt, "#size-cells",
- XEN_FDT_NODE_SIZE_CELLS);
- if ( res )
- return res;
-
- res = fdt_property(fdt, "ranges", NULL, 0);
- if ( res )
- return res;
-
- res = make_memory_node(d, fdt, kinfo);
- if ( res )
- return res;
-
- res = make_gic_node(d, fdt);
- if ( res )
- return res;
-
- res = make_timer_node(d, fdt);
- if ( res )
- return res;
-
- res = fdt_end_node(fdt);
- return res;
-}
-
/* Map the device in the domain */
static int map_device(struct domain *d, const struct dt_device_node *dev)
{
@@ -792,7 +749,15 @@ static int handle_node(struct domain *d, struct kernel_info
*kinfo,
DT_MATCH_COMPATIBLE("arm,psci"),
DT_MATCH_PATH("/cpus"),
DT_MATCH_TYPE("memory"),
+ { /* sentinel */ },
+ };
+ static const struct dt_device_match gic_matches[] __initconst + {
DT_MATCH_GIC,
+ { /* sentinel */ },
+ };
+ static const struct dt_device_match timer_matches[] __initconst + {
DT_MATCH_TIMER,
{ /* sentinel */ },
};
@@ -806,11 +771,28 @@ static int handle_node(struct domain *d, struct
kernel_info *kinfo,
DPRINT("handle %s\n", path);
/* Skip theses nodes and the sub-nodes */
- if ( dt_match_node(skip_matches, np ) ||
- platform_device_is_blacklisted(np) ||
- dt_device_used_by(np) == DOMID_XEN )
+ if ( dt_match_node(skip_matches, np ) )
+ {
+ DPRINT(" Skip it (matched)\n");
+ return 0;
+ }
+ if ( platform_device_is_blacklisted(np) )
{
- DPRINT(" Skip it!\n");
+ DPRINT(" Skip it (blacklisted)\n");
+ return 0;
+ }
+
+ /* 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);
+
+ /* Skip nodes used by Xen */
+ if ( dt_device_used_by(np) == DOMID_XEN )
+ {
+ DPRINT(" Skip it (used by Xen)\n");
return 0;
}
@@ -868,9 +850,10 @@ static int handle_node(struct domain *d, struct kernel_info
*kinfo,
if ( res )
return res;
- res = make_xen_node(d, kinfo->fdt, np, kinfo);
+ res = make_memory_node(d, kinfo->fdt, np, kinfo);
if ( res )
return res;
+
}
res = fdt_end_node(kinfo->fdt);
--
1.7.10.4
Ian Campbell
2013-Nov-22 15:12 UTC
[PATCH] xen: arm: correct name of the dt node passed around when building dom0 DT
The argument is the node, not the parent. 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 either "parent" or "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 <ian.campbell@citrix.com>
---
xen/arch/arm/domain_build.c | 98 +++++++++++++++++++++----------------------
1 file changed, 49 insertions(+), 49 deletions(-)
diff --git a/xen/arch/arm/domain_build.c b/xen/arch/arm/domain_build.c
index d79e038..7626c96 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);
@@ -276,11 +276,11 @@ static int fdt_property_interrupts(void *fdt,
gic_interrupt_t *intr,
static int make_memory_node(const struct domain *d,
void *fdt,
- const struct dt_device_node *parent,
+ const struct dt_device_node *node,
const struct kernel_info *kinfo)
{
int res, i;
- int reg_size = dt_n_addr_cells(parent) + dt_n_size_cells(parent);
+ int reg_size = dt_n_addr_cells(node) + dt_n_size_cells(node);
int nr_cells = reg_size*kinfo->mem.nr_banks;
__be32 reg[nr_cells];
__be32 *cells;
@@ -305,7 +305,7 @@ static int make_memory_node(const struct domain *d,
DPRINT(" Bank %d:
%#"PRIx64"->%#"PRIx64"\n",
i, start, start + size);
- dt_set_range(&cells, parent, start, size);
+ dt_set_range(&cells, node, start, size);
}
res = fdt_property(fdt, "reg", reg, sizeof(reg));
@@ -318,7 +318,7 @@ static int make_memory_node(const struct domain *d,
}
static int make_hypervisor_node(struct domain *d,
- void *fdt, const struct dt_device_node *parent)
+ void *fdt, const struct dt_device_node *node)
{
const char compat[]
"xen,xen-"__stringify(XEN_VERSION)"."__stringify(XEN_SUBVERSION)"\0"
@@ -327,8 +327,8 @@ static int make_hypervisor_node(struct domain *d,
gic_interrupt_t intr;
__be32 *cells;
int res;
- int addrcells = dt_n_addr_cells(parent);
- int sizecells = dt_n_size_cells(parent);
+ int addrcells = dt_n_addr_cells(node);
+ int sizecells = dt_n_size_cells(node);
DPRINT("Create hypervisor node\n");
@@ -353,7 +353,7 @@ static int make_hypervisor_node(struct domain *d,
DPRINT(" Grant table range: 0xb0000000-0x20000\n");
/* reg 0 is grant table space */
cells = ®[0];
- dt_set_range(&cells, parent, 0xb0000000, 0x20000);
+ dt_set_range(&cells, node, 0xb0000000, 0x20000);
res = fdt_property(fdt, "reg", reg,
dt_cells_to_size(addrcells + sizecells));
if ( res )
@@ -379,7 +379,7 @@ static int make_hypervisor_node(struct domain *d,
return res;
}
-static int make_psci_node(void *fdt, const struct dt_device_node *parent)
+static int make_psci_node(void *fdt, const struct dt_device_node *node)
{
int res;
@@ -412,7 +412,7 @@ static int make_psci_node(void *fdt, const struct
dt_device_node *parent)
}
static int make_cpus_node(const struct domain *d, void *fdt,
- const struct dt_device_node *parent)
+ const struct dt_device_node *node)
{
int res;
const struct dt_device_node *cpus =
dt_find_node_by_path("/cpus");
@@ -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;
--
1.7.10.4
Ian Campbell
2013-Nov-22 15:15 UTC
Re: [PATCH] xen: arm: correct name of the dt node passed around when building dom0 DT
On Fri, 2013-11-22 at 15:12 +0000, Ian Campbell wrote:> The argument is the node, not the parent. 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 either "parent" or "np" with "node".As Julien pointed out separately this is bogus. In some cases the argument is the parent and in others it is the node to replace. I''ll respin this.> > 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 <ian.campbell@citrix.com> > --- > xen/arch/arm/domain_build.c | 98 +++++++++++++++++++++---------------------- > 1 file changed, 49 insertions(+), 49 deletions(-) > > diff --git a/xen/arch/arm/domain_build.c b/xen/arch/arm/domain_build.c > index d79e038..7626c96 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); > @@ -276,11 +276,11 @@ static int fdt_property_interrupts(void *fdt, gic_interrupt_t *intr, > > static int make_memory_node(const struct domain *d, > void *fdt, > - const struct dt_device_node *parent, > + const struct dt_device_node *node, > const struct kernel_info *kinfo) > { > int res, i; > - int reg_size = dt_n_addr_cells(parent) + dt_n_size_cells(parent); > + int reg_size = dt_n_addr_cells(node) + dt_n_size_cells(node); > int nr_cells = reg_size*kinfo->mem.nr_banks; > __be32 reg[nr_cells]; > __be32 *cells; > @@ -305,7 +305,7 @@ static int make_memory_node(const struct domain *d, > DPRINT(" Bank %d: %#"PRIx64"->%#"PRIx64"\n", > i, start, start + size); > > - dt_set_range(&cells, parent, start, size); > + dt_set_range(&cells, node, start, size); > } > > res = fdt_property(fdt, "reg", reg, sizeof(reg)); > @@ -318,7 +318,7 @@ static int make_memory_node(const struct domain *d, > } > > static int make_hypervisor_node(struct domain *d, > - void *fdt, const struct dt_device_node *parent) > + void *fdt, const struct dt_device_node *node) > { > const char compat[] > "xen,xen-"__stringify(XEN_VERSION)"."__stringify(XEN_SUBVERSION)"\0" > @@ -327,8 +327,8 @@ static int make_hypervisor_node(struct domain *d, > gic_interrupt_t intr; > __be32 *cells; > int res; > - int addrcells = dt_n_addr_cells(parent); > - int sizecells = dt_n_size_cells(parent); > + int addrcells = dt_n_addr_cells(node); > + int sizecells = dt_n_size_cells(node); > > DPRINT("Create hypervisor node\n"); > > @@ -353,7 +353,7 @@ static int make_hypervisor_node(struct domain *d, > DPRINT(" Grant table range: 0xb0000000-0x20000\n"); > /* reg 0 is grant table space */ > cells = ®[0]; > - dt_set_range(&cells, parent, 0xb0000000, 0x20000); > + dt_set_range(&cells, node, 0xb0000000, 0x20000); > res = fdt_property(fdt, "reg", reg, > dt_cells_to_size(addrcells + sizecells)); > if ( res ) > @@ -379,7 +379,7 @@ static int make_hypervisor_node(struct domain *d, > return res; > } > > -static int make_psci_node(void *fdt, const struct dt_device_node *parent) > +static int make_psci_node(void *fdt, const struct dt_device_node *node) > { > int res; > > @@ -412,7 +412,7 @@ static int make_psci_node(void *fdt, const struct dt_device_node *parent) > } > > static int make_cpus_node(const struct domain *d, void *fdt, > - const struct dt_device_node *parent) > + const struct dt_device_node *node) > { > int res; > const struct dt_device_node *cpus = dt_find_node_by_path("/cpus"); > @@ -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; >
Ian Campbell
2013-Nov-22 15:25 UTC
[PATCH v2] xen: arm: correct name of the dt node passed around when building dom0 DT
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 <ian.campbell@citrix.com>
---
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;
--
1.7.10.4
Julien Grall
2013-Nov-22 16:14 UTC
Re: [PATCH v2] xen: arm: correct name of the dt node passed around when building dom0 DT
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 <ian.campbell@citrix.com>Acked-by: Julien Grall <julien.grall@linaro.org>> --- > 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
Julien Grall
2013-Nov-22 16:22 UTC
Re: [PATCH v2] xen: arm: remove /xen-core-devices node from dom0 dtb
On 11/22/2013 03:07 PM, Ian Campbell wrote:> The intention of adding this node to contain the GIC, timer and memory nodes > (in 1c08d6004ea7) was to allow us to control the #address-cells and > > However in the case of the memory node the #*-cells are always taken from the > root node (see ePAPR 3.1, "the following nodes shall be present at the root... > memory node"). This caused breakage on the arndale platform. > > In addition it is not valid to just create sub-nodes like this. Unless they > declare themselves as a bus then they will not necessarily be enumerated > (although Linux currently does so in practice). > > Therefore: > - Move the memory node back to the top level. > - Insert the timer and primary gic nodes in the same location as the host > DTB, replacing the originals. Note that the nodes here may be marked as in > use by Xen and therefore the check must be before we discard nodes used by > Xen. > - Drop any secondary gics. > > Signed-off-by: Ian Campbell <ian.campbell@citrix.com> > --- > v2: > Discard secondary gics. > Left parameter as "parent" for now since that is used everywhere. > ---[..]> @@ -792,7 +749,15 @@ static int handle_node(struct domain *d, struct kernel_info *kinfo, > DT_MATCH_COMPATIBLE("arm,psci"), > DT_MATCH_PATH("/cpus"), > DT_MATCH_TYPE("memory"), > + { /* sentinel */ }, > + }; > + static const struct dt_device_match gic_matches[] __initconst > + { > DT_MATCH_GIC, > + { /* sentinel */ }, > + }; > + static const struct dt_device_match timer_matches[] __initconst > + { > DT_MATCH_TIMER, > { /* sentinel */ }, > }; > @@ -806,11 +771,28 @@ static int handle_node(struct domain *d, struct kernel_info *kinfo, > DPRINT("handle %s\n", path); > > /* Skip theses nodes and the sub-nodes */ > - if ( dt_match_node(skip_matches, np ) || > - platform_device_is_blacklisted(np) || > - dt_device_used_by(np) == DOMID_XEN ) > + if ( dt_match_node(skip_matches, np ) )spurious space before '')''> + { > + DPRINT(" Skip it (matched)\n"); > + return 0; > + } > + if ( platform_device_is_blacklisted(np) ) > { > - DPRINT(" Skip it!\n"); > + DPRINT(" Skip it (blacklisted)\n"); > + return 0; > + } > + > + /* 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 ) )same here> + return make_gic_node(d, kinfo->fdt, np); > + if ( dt_match_node(timer_matches, np ) )same here> + return make_timer_node(d, kinfo->fdt, np); > + > + /* Skip nodes used by Xen */ > + if ( dt_device_used_by(np) == DOMID_XEN ) > + { > + DPRINT(" Skip it (used by Xen)\n"); > return 0; > } >[..] Except the minor coding style issue: Acked-by: Julien Grall <julien.grall@linaro.org> -- Julien Grall
Ian Campbell
2013-Nov-22 17:05 UTC
Re: [PATCH v2] xen: arm: remove /xen-core-devices node from dom0 dtb
On Fri, 2013-11-22 at 16:22 +0000, Julien Grall wrote:> Except the minor coding style issue: > > Acked-by: Julien Grall <julien.grall@linaro.org>Thanks. I fixed those up as I went and applied. I also added a Tested-by since you told me on IRC that you had tried this on arndale. I also applied the subsequent renaming patch. Ian.