Ian Campbell
2013-Nov-22 13:47 UTC
[PATCH] 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 gic nodes in the same location as in 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.
Signed-off-by: Ian Campbell <ian.campbell@citrix.com>
---
xen/arch/arm/domain_build.c | 116 ++++++++++++++++---------------------------
1 file changed, 44 insertions(+), 72 deletions(-)
diff --git a/xen/arch/arm/domain_build.c b/xen/arch/arm/domain_build.c
index aa7e3d2..a6f7be7 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;
@@ -565,7 +550,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 +559,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 +588,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 +649,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 +739,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 +761,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 (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!\n");
+ DPRINT(" Skip it (used by Xen)\n");
return 0;
}
@@ -868,7 +840,7 @@ 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;
}
--
1.7.10.4
Julien Grall
2013-Nov-22 14:18 UTC
Re: [PATCH] xen: arm: remove /xen-core-devices node from dom0 dtb
On 11/22/2013 01:47 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 gic nodes in the same location as in 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.If you insert the GIC and timer nodes at the same location you need to make sure the use the right value. For instance it may happen to have multiple GIC in the device tree. Actually, Xen skips all the GIC nodes and create just one. Here, you replace all the possible GIC nodes by the same value (I''m not sure how Linux will deal with that). For now, I think the best the solution is to check if we are the main interrupt controller. If not, the function just need to return 0.> > Signed-off-by: Ian Campbell <ian.campbell@citrix.com> > --- > xen/arch/arm/domain_build.c | 116 ++++++++++++++++--------------------------- > 1 file changed, 44 insertions(+), 72 deletions(-) > > diff --git a/xen/arch/arm/domain_build.c b/xen/arch/arm/domain_build.c > index aa7e3d2..a6f7be7 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; > @@ -565,7 +550,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 +559,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 +588,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 +649,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 +739,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 +761,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 (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!\n"); > + DPRINT(" Skip it (used by Xen)\n"); > return 0; > } > > @@ -868,7 +840,7 @@ 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; > } >-- Julien Grall
Ian Campbell
2013-Nov-22 14:40 UTC
Re: [PATCH] xen: arm: remove /xen-core-devices node from dom0 dtb
On Fri, 2013-11-22 at 14:18 +0000, Julien Grall wrote:> > On 11/22/2013 01:47 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 gic nodes in the same location as in 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. > > If you insert the GIC and timer nodes at the same location you need to > make sure the use the right value. For instance it may happen to have > multiple GIC in the device tree. > Actually, Xen skips all the GIC nodes and create just one. Here, you > replace all the possible GIC nodes by the same value (I''m not sure how > Linux will deal with that).Damn, I forgot about this case. I''m not sure how relevant it is to us today though -- do we support any such systems and/or would they be likely to just work, I would expect not?> For now, I think the best the solution is to check if we are the main > interrupt controller. If not, the function just need to return 0.Means checking dt_interrupt_controller->phandle against the node in hand, right?
Julien Grall
2013-Nov-22 14:50 UTC
Re: [PATCH] xen: arm: remove /xen-core-devices node from dom0 dtb
On 11/22/2013 02:40 PM, Ian Campbell wrote:> On Fri, 2013-11-22 at 14:18 +0000, Julien Grall wrote: >> >> On 11/22/2013 01:47 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 gic nodes in the same location as in 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. >> >> If you insert the GIC and timer nodes at the same location you need to >> make sure the use the right value. For instance it may happen to have >> multiple GIC in the device tree. >> Actually, Xen skips all the GIC nodes and create just one. Here, you >> replace all the possible GIC nodes by the same value (I''m not sure how >> Linux will deal with that). > > Damn, I forgot about this case. I''m not sure how relevant it is to us > today though -- do we support any such systems and/or would they be > likely to just work, I would expect not?We don''t support this kind of systems. Xen only handle one GIC at time in arch/arm/gic.c. During dom0 DT building we will just remove all GIC nodes and just create one (the main controller).> >> For now, I think the best the solution is to check if we are the main >> interrupt controller. If not, the function just need to return 0. > > Means checking dt_interrupt_controller->phandle against the node in > hand, right?You give in argument the current node pointer (ie the GIC). So you can compare this pointer to dt_interrupt_controller. BTW, the name of the argument "parent" is wrong. It''s not the parent but the current node. I would rename to "np". -- Julien Grall
Ian Campbell
2013-Nov-22 14:58 UTC
Re: [PATCH] xen: arm: remove /xen-core-devices node from dom0 dtb
On Fri, 2013-11-22 at 14:50 +0000, Julien Grall wrote:> > On 11/22/2013 02:40 PM, Ian Campbell wrote: > > On Fri, 2013-11-22 at 14:18 +0000, Julien Grall wrote: > >> > >> On 11/22/2013 01:47 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 gic nodes in the same location as in 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. > >> > >> If you insert the GIC and timer nodes at the same location you need to > >> make sure the use the right value. For instance it may happen to have > >> multiple GIC in the device tree. > >> Actually, Xen skips all the GIC nodes and create just one. Here, you > >> replace all the possible GIC nodes by the same value (I''m not sure how > >> Linux will deal with that). > > > > Damn, I forgot about this case. I''m not sure how relevant it is to us > > today though -- do we support any such systems and/or would they be > > likely to just work, I would expect not? > > We don''t support this kind of systems. Xen only handle one GIC at time > in arch/arm/gic.c.OK, so my point was that this is a bit of a non-issue, at least right now. And running on such a system is going to take actual work beyond some platform quirks etc.> During dom0 DT building we will just remove all GIC nodes and just > create one (the main controller). > > > > >> For now, I think the best the solution is to check if we are the main > >> interrupt controller. If not, the function just need to return 0. > > > > Means checking dt_interrupt_controller->phandle against the node in > > hand, right? > > You give in argument the current node pointer (ie the GIC). So you can > compare this pointer to dt_interrupt_controller.Right. I''ll do this.> BTW, the name of the argument "parent" is wrong. It''s not the parent but > the current node. I would rename to "np".OK.>
Ian Campbell
2013-Nov-22 15:05 UTC
Re: [PATCH] xen: arm: remove /xen-core-devices node from dom0 dtb
On Fri, 2013-11-22 at 14:58 +0000, Ian Campbell wrote:> > BTW, the name of the argument "parent" is wrong. It''s not the parent but > > the current node. I would rename to "np". > > OK.Actually, everywhere else is using parent too. I''ll fix this separatly.
Julien Grall
2013-Nov-22 15:08 UTC
Re: [PATCH] xen: arm: remove /xen-core-devices node from dom0 dtb
On 11/22/2013 03:05 PM, Ian Campbell wrote:> On Fri, 2013-11-22 at 14:58 +0000, Ian Campbell wrote: >>> BTW, the name of the argument "parent" is wrong. It''s not the parent but >>> the current node. I would rename to "np". >> >> OK. > > Actually, everywhere else is using parent too. I''ll fix this separatly. >What do you mean by everywhere else ? It''s logic to use "parent" for the hypervisor node and PSCI node. Because we will create it under the parent node. Here, we know that we are replacing inplace the GIC, so it''s the current node. -- Julien Grall
Ian Campbell
2013-Nov-22 15:14 UTC
Re: [PATCH] xen: arm: remove /xen-core-devices node from dom0 dtb
On Fri, 2013-11-22 at 15:08 +0000, Julien Grall wrote:> > On 11/22/2013 03:05 PM, Ian Campbell wrote: > > On Fri, 2013-11-22 at 14:58 +0000, Ian Campbell wrote: > >>> BTW, the name of the argument "parent" is wrong. It''s not the parent but > >>> the current node. I would rename to "np". > >> > >> OK. > > > > Actually, everywhere else is using parent too. I''ll fix this separatly. > > > > What do you mean by everywhere else ? It''s logic to use "parent" for the > hypervisor node and PSCI node. Because we will create it under the > parent node. > Here, we know that we are replacing inplace the GIC, so it''s the current > node.Oh of course, that makes sense I guess. I sent out the fixup already, where I also replaced all the np with node because it was that name which lead to the confusion ("node parent"). I''ll respin Ian.