Get the address of the GIC distributor, cpu, virtual and virtual cpu
interfaces registers from device tree.
Note: I couldn''t completely get rid of GIC_BASE_ADDRESS, GIC_DR_OFFSET
and friends because we are using them from mode_switch.S, that is
executed before device tree has been parsed. But at least mode_switch.S
is known to contain vexpress specific code anyway.
Changes in v2:
- remove 2 superflous lines from process_gic_node;
- introduce device_tree_get_reg_ranges;
- add a check for uninitialized GIC interface addresses;
- add a check for non-page aligned GIC interface addresses;
- remove the code to deal with non-page aligned addresses from GICC and
GICH.
Signed-off-by: Stefano Stabellini <stefano.stabellini@eu.citrix.com>
diff --git a/xen/arch/arm/gic.c b/xen/arch/arm/gic.c
index 0c6fab9..2b29e7e 100644
--- a/xen/arch/arm/gic.c
+++ b/xen/arch/arm/gic.c
@@ -26,6 +26,7 @@
#include <xen/errno.h>
#include <xen/softirq.h>
#include <xen/list.h>
+#include <xen/device_tree.h>
#include <asm/p2m.h>
#include <asm/domain.h>
@@ -33,10 +34,8 @@
/* Access to the GIC Distributor registers through the fixmap */
#define GICD ((volatile uint32_t *) FIXMAP_ADDR(FIXMAP_GICD))
-#define GICC ((volatile uint32_t *) (FIXMAP_ADDR(FIXMAP_GICC1) \
- + (GIC_CR_OFFSET & 0xfff)))
-#define GICH ((volatile uint32_t *) (FIXMAP_ADDR(FIXMAP_GICH) \
- + (GIC_HR_OFFSET & 0xfff)))
+#define GICC ((volatile uint32_t *) FIXMAP_ADDR(FIXMAP_GICC1))
+#define GICH ((volatile uint32_t *) FIXMAP_ADDR(FIXMAP_GICH))
static void gic_restore_pending_irqs(struct vcpu *v);
/* Global state */
@@ -44,6 +43,7 @@ static struct {
paddr_t dbase; /* Address of distributor registers */
paddr_t cbase; /* Address of CPU interface registers */
paddr_t hbase; /* Address of virtual interface registers */
+ paddr_t vbase; /* Address of virtual cpu interface registers */
unsigned int lines;
unsigned int cpus;
spinlock_t lock;
@@ -306,10 +306,27 @@ static void __cpuinit gic_hyp_disable(void)
/* Set up the GIC */
void __init gic_init(void)
{
- /* XXX FIXME get this from devicetree */
- gic.dbase = GIC_BASE_ADDRESS + GIC_DR_OFFSET;
- gic.cbase = GIC_BASE_ADDRESS + GIC_CR_OFFSET;
- gic.hbase = GIC_BASE_ADDRESS + GIC_HR_OFFSET;
+ if ( !early_info.gic.gic_dist_addr ||
+ !early_info.gic.gic_cpu_addr ||
+ !early_info.gic.gic_hyp_addr ||
+ !early_info.gic.gic_vcpu_addr )
+ panic("the physical address of one of the GIC interfaces is
missing:\n"
+ " gic_dist_addr=%"PRIpaddr"\n"
+ " gic_cpu_addr=%"PRIpaddr"\n"
+ " gic_hyp_addr=%"PRIpaddr"\n"
+ " gic_vcpu_addr=%"PRIpaddr"\n",
+ early_info.gic.gic_dist_addr, early_info.gic.gic_cpu_addr,
+ early_info.gic.gic_hyp_addr, early_info.gic.gic_vcpu_addr);
+ if ( (early_info.gic.gic_dist_addr & ~PAGE_MASK) ||
+ (early_info.gic.gic_cpu_addr & ~PAGE_MASK) ||
+ (early_info.gic.gic_hyp_addr & ~PAGE_MASK) ||
+ (early_info.gic.gic_vcpu_addr & ~PAGE_MASK) )
+ panic("error: GIC interfaces not page aligned.\n");
+
+ gic.dbase = early_info.gic.gic_dist_addr;
+ gic.cbase = early_info.gic.gic_cpu_addr;
+ gic.hbase = early_info.gic.gic_hyp_addr;
+ gic.vbase = early_info.gic.gic_vcpu_addr;
set_fixmap(FIXMAP_GICD, gic.dbase >> PAGE_SHIFT, DEV_SHARED);
BUILD_BUG_ON(FIXMAP_ADDR(FIXMAP_GICC1) !
FIXMAP_ADDR(FIXMAP_GICC2)-PAGE_SIZE);
@@ -569,9 +586,9 @@ int gicv_setup(struct domain *d)
{
/* map the gic virtual cpu interface in the gic cpu interface region of
* the guest */
- return map_mmio_regions(d, GIC_BASE_ADDRESS + GIC_CR_OFFSET,
- GIC_BASE_ADDRESS + GIC_CR_OFFSET + (2 * PAGE_SIZE) - 1,
- GIC_BASE_ADDRESS + GIC_VR_OFFSET);
+ return map_mmio_regions(d, gic.cbase,
+ gic.cbase + (2 * PAGE_SIZE) - 1,
+ gic.vbase);
}
static void maintenance_interrupt(int irq, void *dev_id, struct cpu_user_regs
*regs)
diff --git a/xen/common/device_tree.c b/xen/common/device_tree.c
index da0af77..8d5b6b0 100644
--- a/xen/common/device_tree.c
+++ b/xen/common/device_tree.c
@@ -54,6 +54,33 @@ bool_t device_tree_type_matches(const void *fdt, int node,
const char *match)
return !strncmp(prop, match, len);
}
+bool_t device_tree_node_compatible(const void *fdt, int node, const char
*match)
+{
+ int len, l;
+ const void *prop;
+
+ prop = fdt_getprop(fdt, node, "compatible", &len);
+ if ( prop == NULL )
+ return 0;
+
+ while ( len > 0 ) {
+ if ( !strncmp(prop, match, strlen(match)) )
+ return 1;
+ l = strlen(prop) + 1;
+ prop += l;
+ len -= l;
+ }
+
+ return 0;
+}
+
+static void device_tree_get_reg_ranges(const struct fdt_property *prop,
+ u32 address_cells, u32 size_cells, int *ranges)
+{
+ u32 reg_cells = address_cells + size_cells;
+ *ranges = fdt32_to_cpu(prop->len) / (reg_cells * sizeof(u32));
+}
+
static void __init get_val(const u32 **cell, u32 cells, u64 *val)
{
*val = 0;
@@ -209,7 +236,6 @@ static void __init process_memory_node(const void *fdt, int
node,
u32 address_cells, u32 size_cells)
{
const struct fdt_property *prop;
- size_t reg_cells;
int i;
int banks;
const u32 *cell;
@@ -230,8 +256,7 @@ static void __init process_memory_node(const void *fdt, int
node,
}
cell = (const u32 *)prop->data;
- reg_cells = address_cells + size_cells;
- banks = fdt32_to_cpu(prop->len) / (reg_cells * sizeof(u32));
+ device_tree_get_reg_ranges(prop, address_cells, size_cells, &banks);
for ( i = 0; i < banks && early_info.mem.nr_banks <
NR_MEM_BANKS; i++ )
{
@@ -270,6 +295,46 @@ static void __init process_cpu_node(const void *fdt, int
node,
cpumask_set_cpu(start, &cpu_possible_map);
}
+static void __init process_gic_node(const void *fdt, int node,
+ const char *name,
+ u32 address_cells, u32 size_cells)
+{
+ const struct fdt_property *prop;
+ const u32 *cell;
+ paddr_t start, size;
+ int interfaces;
+
+ if ( address_cells < 1 || size_cells < 1 )
+ {
+ early_printk("fdt: node `%s'': invalid #address-cells or
#size-cells",
+ name);
+ return;
+ }
+
+ prop = fdt_get_property(fdt, node, "reg", NULL);
+ if ( !prop )
+ {
+ early_printk("fdt: node `%s'': missing `reg''
property\n", name);
+ return;
+ }
+
+ cell = (const u32 *)prop->data;
+ device_tree_get_reg_ranges(prop, address_cells, size_cells,
&interfaces);
+ if ( interfaces < 4 )
+ {
+ early_printk("fdt: node `%s'': invalid `reg''
property\n", name);
+ return;
+ }
+ device_tree_get_reg(&cell, address_cells, size_cells, &start,
&size);
+ early_info.gic.gic_dist_addr = start;
+ device_tree_get_reg(&cell, address_cells, size_cells, &start,
&size);
+ early_info.gic.gic_cpu_addr = start;
+ device_tree_get_reg(&cell, address_cells, size_cells, &start,
&size);
+ early_info.gic.gic_hyp_addr = start;
+ device_tree_get_reg(&cell, address_cells, size_cells, &start,
&size);
+ early_info.gic.gic_vcpu_addr = start;
+}
+
static int __init early_scan_node(const void *fdt,
int node, const char *name, int depth,
u32 address_cells, u32 size_cells,
@@ -279,6 +344,8 @@ static int __init early_scan_node(const void *fdt,
process_memory_node(fdt, node, name, address_cells, size_cells);
else if ( device_tree_type_matches(fdt, node, "cpu") )
process_cpu_node(fdt, node, name, address_cells, size_cells);
+ else if ( device_tree_node_compatible(fdt, node,
"arm,cortex-a15-gic") )
+ process_gic_node(fdt, node, name, address_cells, size_cells);
return 0;
}
diff --git a/xen/include/xen/device_tree.h b/xen/include/xen/device_tree.h
index 4d010c0..a0e3a97 100644
--- a/xen/include/xen/device_tree.h
+++ b/xen/include/xen/device_tree.h
@@ -26,8 +26,16 @@ struct dt_mem_info {
struct membank bank[NR_MEM_BANKS];
};
+struct dt_gic_info {
+ paddr_t gic_dist_addr;
+ paddr_t gic_cpu_addr;
+ paddr_t gic_hyp_addr;
+ paddr_t gic_vcpu_addr;
+};
+
struct dt_early_info {
struct dt_mem_info mem;
+ struct dt_gic_info gic;
};
typedef int (*device_tree_node_func)(const void *fdt,
On Fri, 2012-11-30 at 14:28 +0000, Stefano Stabellini wrote:> Get the address of the GIC distributor, cpu, virtual and virtual cpu > interfaces registers from device tree. > > Note: I couldn''t completely get rid of GIC_BASE_ADDRESS, GIC_DR_OFFSET > and friends because we are using them from mode_switch.S, that is > executed before device tree has been parsed. But at least mode_switch.S > is known to contain vexpress specific code anyway. > > > Changes in v2: > - remove 2 superflous lines from process_gic_node; > - introduce device_tree_get_reg_ranges; > - add a check for uninitialized GIC interface addresses; > - add a check for non-page aligned GIC interface addresses; > - remove the code to deal with non-page aligned addresses from GICC and > GICH. > > > Signed-off-by: Stefano Stabellini <stefano.stabellini@eu.citrix.com> > > > diff --git a/xen/arch/arm/gic.c b/xen/arch/arm/gic.c > index 0c6fab9..2b29e7e 100644 > --- a/xen/arch/arm/gic.c > +++ b/xen/arch/arm/gic.c > @@ -26,6 +26,7 @@ > #include <xen/errno.h> > #include <xen/softirq.h> > #include <xen/list.h> > +#include <xen/device_tree.h> > #include <asm/p2m.h> > #include <asm/domain.h> > > @@ -33,10 +34,8 @@ > > /* Access to the GIC Distributor registers through the fixmap */ > #define GICD ((volatile uint32_t *) FIXMAP_ADDR(FIXMAP_GICD)) > -#define GICC ((volatile uint32_t *) (FIXMAP_ADDR(FIXMAP_GICC1) \ > - + (GIC_CR_OFFSET & 0xfff))) > -#define GICH ((volatile uint32_t *) (FIXMAP_ADDR(FIXMAP_GICH) \ > - + (GIC_HR_OFFSET & 0xfff))) > +#define GICC ((volatile uint32_t *) FIXMAP_ADDR(FIXMAP_GICC1)) > +#define GICH ((volatile uint32_t *) FIXMAP_ADDR(FIXMAP_GICH)) > static void gic_restore_pending_irqs(struct vcpu *v); > > /* Global state */ > @@ -44,6 +43,7 @@ static struct { > paddr_t dbase; /* Address of distributor registers */ > paddr_t cbase; /* Address of CPU interface registers */ > paddr_t hbase; /* Address of virtual interface registers */ > + paddr_t vbase; /* Address of virtual cpu interface registers */ > unsigned int lines; > unsigned int cpus; > spinlock_t lock; > @@ -306,10 +306,27 @@ static void __cpuinit gic_hyp_disable(void) > /* Set up the GIC */ > void __init gic_init(void) > { > - /* XXX FIXME get this from devicetree */ > - gic.dbase = GIC_BASE_ADDRESS + GIC_DR_OFFSET; > - gic.cbase = GIC_BASE_ADDRESS + GIC_CR_OFFSET; > - gic.hbase = GIC_BASE_ADDRESS + GIC_HR_OFFSET; > + if ( !early_info.gic.gic_dist_addr || > + !early_info.gic.gic_cpu_addr || > + !early_info.gic.gic_hyp_addr || > + !early_info.gic.gic_vcpu_addr ) > + panic("the physical address of one of the GIC interfaces is missing:\n" > + " gic_dist_addr=%"PRIpaddr"\n" > + " gic_cpu_addr=%"PRIpaddr"\n" > + " gic_hyp_addr=%"PRIpaddr"\n" > + " gic_vcpu_addr=%"PRIpaddr"\n", > + early_info.gic.gic_dist_addr, early_info.gic.gic_cpu_addr, > + early_info.gic.gic_hyp_addr, early_info.gic.gic_vcpu_addr); > + if ( (early_info.gic.gic_dist_addr & ~PAGE_MASK) || > + (early_info.gic.gic_cpu_addr & ~PAGE_MASK) || > + (early_info.gic.gic_hyp_addr & ~PAGE_MASK) || > + (early_info.gic.gic_vcpu_addr & ~PAGE_MASK) ) > + panic("error: GIC interfaces not page aligned.\n");Oh, maybe I should have skipped "arm: add few checks to gic_init" and come straight here instead?> static void maintenance_interrupt(int irq, void *dev_id, struct cpu_user_regs *regs) > diff --git a/xen/common/device_tree.c b/xen/common/device_tree.c > index da0af77..8d5b6b0 100644 > --- a/xen/common/device_tree.c > +++ b/xen/common/device_tree.c > @@ -54,6 +54,33 @@ bool_t device_tree_type_matches(const void *fdt, int node, const char *match) > return !strncmp(prop, match, len); > } > > +bool_t device_tree_node_compatible(const void *fdt, int node, const char *match) > +{ > + int len, l; > + const void *prop; > + > + prop = fdt_getprop(fdt, node, "compatible", &len); > + if ( prop == NULL ) > + return 0; > + > + while ( len > 0 ) { > + if ( !strncmp(prop, match, strlen(match)) ) > + return 1;This will decide that match="foo-bar" is compatible with a node which contains compatible = "foo-bar-baz". Is that deliberate? I thought the DT way would be to have compatible = "foo-bar-baz", "foo-bar" ? Perhaps this is just due to cut-n-paste from device_tree_node_matches (where it makes sense, I think)? I now wonder the same thing about device_tree_type_matches too...> + l = strlen(prop) + 1; > + prop += l; > + len -= l; > + } > + > + return 0; > +} > + > +static void device_tree_get_reg_ranges(const struct fdt_property *prop,"nr" in the name somewhere? I thought at first this was somehow returning the ranges themselves.> + u32 address_cells, u32 size_cells, int *ranges) > +{ > + u32 reg_cells = address_cells + size_cells; > + *ranges = fdt32_to_cpu(prop->len) / (reg_cells * sizeof(u32)); > +} > + > static void __init get_val(const u32 **cell, u32 cells, u64 *val) > { > *val = 0; > @@ -209,7 +236,6 @@ static void __init process_memory_node(const void *fdt, int node, > u32 address_cells, u32 size_cells) > { > const struct fdt_property *prop; > - size_t reg_cells; > int i; > int banks; > const u32 *cell; > @@ -230,8 +256,7 @@ static void __init process_memory_node(const void *fdt, int node, > } > > cell = (const u32 *)prop->data; > - reg_cells = address_cells + size_cells; > - banks = fdt32_to_cpu(prop->len) / (reg_cells * sizeof(u32)); > + device_tree_get_reg_ranges(prop, address_cells, size_cells, &banks); > > for ( i = 0; i < banks && early_info.mem.nr_banks < NR_MEM_BANKS; i++ ) > { > @@ -270,6 +295,46 @@ static void __init process_cpu_node(const void *fdt, int node, > cpumask_set_cpu(start, &cpu_possible_map); > } > > +static void __init process_gic_node(const void *fdt, int node, > + const char *name, > + u32 address_cells, u32 size_cells) > +{ > + const struct fdt_property *prop; > + const u32 *cell; > + paddr_t start, size; > + int interfaces; > + > + if ( address_cells < 1 || size_cells < 1 ) > + { > + early_printk("fdt: node `%s'': invalid #address-cells or #size-cells", > + name); > + return; > + }Is this the sort of check which the generic DT walker helper thing ought to include?> + > + prop = fdt_get_property(fdt, node, "reg", NULL); > + if ( !prop ) > + { > + early_printk("fdt: node `%s'': missing `reg'' property\n", name); > + return; > + } > + > + cell = (const u32 *)prop->data; > + device_tree_get_reg_ranges(prop, address_cells, size_cells, &interfaces); > + if ( interfaces < 4 ) > + { > + early_printk("fdt: node `%s'': invalid `reg'' property\n", name);A more specific message would be "not enough ranges in ..."> + return; > + } > + device_tree_get_reg(&cell, address_cells, size_cells, &start, &size); > + early_info.gic.gic_dist_addr = start; > + device_tree_get_reg(&cell, address_cells, size_cells, &start, &size); > + early_info.gic.gic_cpu_addr = start; > + device_tree_get_reg(&cell, address_cells, size_cells, &start, &size); > + early_info.gic.gic_hyp_addr = start; > + device_tree_get_reg(&cell, address_cells, size_cells, &start, &size); > + early_info.gic.gic_vcpu_addr = start; > +} > + > static int __init early_scan_node(const void *fdt, > int node, const char *name, int depth, > u32 address_cells, u32 size_cells, > @@ -279,6 +344,8 @@ static int __init early_scan_node(const void *fdt, > process_memory_node(fdt, node, name, address_cells, size_cells); > else if ( device_tree_type_matches(fdt, node, "cpu") ) > process_cpu_node(fdt, node, name, address_cells, size_cells); > + else if ( device_tree_node_compatible(fdt, node, "arm,cortex-a15-gic") ) > + process_gic_node(fdt, node, name, address_cells, size_cells); > > return 0; > } > diff --git a/xen/include/xen/device_tree.h b/xen/include/xen/device_tree.h > index 4d010c0..a0e3a97 100644 > --- a/xen/include/xen/device_tree.h > +++ b/xen/include/xen/device_tree.h > @@ -26,8 +26,16 @@ struct dt_mem_info { > struct membank bank[NR_MEM_BANKS]; > }; > > +struct dt_gic_info { > + paddr_t gic_dist_addr; > + paddr_t gic_cpu_addr; > + paddr_t gic_hyp_addr; > + paddr_t gic_vcpu_addr; > +}; > + > struct dt_early_info { > struct dt_mem_info mem; > + struct dt_gic_info gic; > }; > > typedef int (*device_tree_node_func)(const void *fdt,
On Tue, 4 Dec 2012, Ian Campbell wrote:> On Fri, 2012-11-30 at 14:28 +0000, Stefano Stabellini wrote: > > Get the address of the GIC distributor, cpu, virtual and virtual cpu > > interfaces registers from device tree. > > > > Note: I couldn''t completely get rid of GIC_BASE_ADDRESS, GIC_DR_OFFSET > > and friends because we are using them from mode_switch.S, that is > > executed before device tree has been parsed. But at least mode_switch.S > > is known to contain vexpress specific code anyway. > > > > > > Changes in v2: > > - remove 2 superflous lines from process_gic_node; > > - introduce device_tree_get_reg_ranges; > > - add a check for uninitialized GIC interface addresses; > > - add a check for non-page aligned GIC interface addresses; > > - remove the code to deal with non-page aligned addresses from GICC and > > GICH. > > > > > > Signed-off-by: Stefano Stabellini <stefano.stabellini@eu.citrix.com> > > > > > > diff --git a/xen/arch/arm/gic.c b/xen/arch/arm/gic.c > > index 0c6fab9..2b29e7e 100644 > > --- a/xen/arch/arm/gic.c > > +++ b/xen/arch/arm/gic.c > > @@ -26,6 +26,7 @@ > > #include <xen/errno.h> > > #include <xen/softirq.h> > > #include <xen/list.h> > > +#include <xen/device_tree.h> > > #include <asm/p2m.h> > > #include <asm/domain.h> > > > > @@ -33,10 +34,8 @@ > > > > /* Access to the GIC Distributor registers through the fixmap */ > > #define GICD ((volatile uint32_t *) FIXMAP_ADDR(FIXMAP_GICD)) > > -#define GICC ((volatile uint32_t *) (FIXMAP_ADDR(FIXMAP_GICC1) \ > > - + (GIC_CR_OFFSET & 0xfff))) > > -#define GICH ((volatile uint32_t *) (FIXMAP_ADDR(FIXMAP_GICH) \ > > - + (GIC_HR_OFFSET & 0xfff))) > > +#define GICC ((volatile uint32_t *) FIXMAP_ADDR(FIXMAP_GICC1)) > > +#define GICH ((volatile uint32_t *) FIXMAP_ADDR(FIXMAP_GICH)) > > static void gic_restore_pending_irqs(struct vcpu *v); > > > > /* Global state */ > > @@ -44,6 +43,7 @@ static struct { > > paddr_t dbase; /* Address of distributor registers */ > > paddr_t cbase; /* Address of CPU interface registers */ > > paddr_t hbase; /* Address of virtual interface registers */ > > + paddr_t vbase; /* Address of virtual cpu interface registers */ > > unsigned int lines; > > unsigned int cpus; > > spinlock_t lock; > > @@ -306,10 +306,27 @@ static void __cpuinit gic_hyp_disable(void) > > /* Set up the GIC */ > > void __init gic_init(void) > > { > > - /* XXX FIXME get this from devicetree */ > > - gic.dbase = GIC_BASE_ADDRESS + GIC_DR_OFFSET; > > - gic.cbase = GIC_BASE_ADDRESS + GIC_CR_OFFSET; > > - gic.hbase = GIC_BASE_ADDRESS + GIC_HR_OFFSET; > > + if ( !early_info.gic.gic_dist_addr || > > + !early_info.gic.gic_cpu_addr || > > + !early_info.gic.gic_hyp_addr || > > + !early_info.gic.gic_vcpu_addr ) > > + panic("the physical address of one of the GIC interfaces is missing:\n" > > + " gic_dist_addr=%"PRIpaddr"\n" > > + " gic_cpu_addr=%"PRIpaddr"\n" > > + " gic_hyp_addr=%"PRIpaddr"\n" > > + " gic_vcpu_addr=%"PRIpaddr"\n", > > + early_info.gic.gic_dist_addr, early_info.gic.gic_cpu_addr, > > + early_info.gic.gic_hyp_addr, early_info.gic.gic_vcpu_addr); > > + if ( (early_info.gic.gic_dist_addr & ~PAGE_MASK) || > > + (early_info.gic.gic_cpu_addr & ~PAGE_MASK) || > > + (early_info.gic.gic_hyp_addr & ~PAGE_MASK) || > > + (early_info.gic.gic_vcpu_addr & ~PAGE_MASK) ) > > + panic("error: GIC interfaces not page aligned.\n"); > > Oh, maybe I should have skipped "arm: add few checks to gic_init" and > come straight here instead?Yep. I am going to address the other comments too.> > static void maintenance_interrupt(int irq, void *dev_id, struct cpu_user_regs *regs) > > diff --git a/xen/common/device_tree.c b/xen/common/device_tree.c > > index da0af77..8d5b6b0 100644 > > --- a/xen/common/device_tree.c > > +++ b/xen/common/device_tree.c > > @@ -54,6 +54,33 @@ bool_t device_tree_type_matches(const void *fdt, int node, const char *match) > > return !strncmp(prop, match, len); > > } > > > > +bool_t device_tree_node_compatible(const void *fdt, int node, const char *match) > > +{ > > + int len, l; > > + const void *prop; > > + > > + prop = fdt_getprop(fdt, node, "compatible", &len); > > + if ( prop == NULL ) > > + return 0; > > + > > + while ( len > 0 ) { > > + if ( !strncmp(prop, match, strlen(match)) ) > > + return 1; > > This will decide that match="foo-bar" is compatible with a node which > contains compatible = "foo-bar-baz". Is that deliberate? I thought the > DT way would be to have compatible = "foo-bar-baz", "foo-bar" ? > > Perhaps this is just due to cut-n-paste from device_tree_node_matches > (where it makes sense, I think)?You are right, I think that we should have a perfect match for the compatible string.> I now wonder the same thing about device_tree_type_matches too...I think we should use strcmp in both cases. I''ll send another patch for that.> > + l = strlen(prop) + 1; > > + prop += l; > > + len -= l; > > + } > > + > > + return 0; > > +} > > + > > +static void device_tree_get_reg_ranges(const struct fdt_property *prop, > > "nr" in the name somewhere? I thought at first this was somehow > returning the ranges themselves.OK> > + u32 address_cells, u32 size_cells, int *ranges) > > +{ > > + u32 reg_cells = address_cells + size_cells; > > + *ranges = fdt32_to_cpu(prop->len) / (reg_cells * sizeof(u32)); > > +} > > + > > static void __init get_val(const u32 **cell, u32 cells, u64 *val) > > { > > *val = 0; > > @@ -209,7 +236,6 @@ static void __init process_memory_node(const void *fdt, int node, > > u32 address_cells, u32 size_cells) > > { > > const struct fdt_property *prop; > > - size_t reg_cells; > > int i; > > int banks; > > const u32 *cell; > > @@ -230,8 +256,7 @@ static void __init process_memory_node(const void *fdt, int node, > > } > > > > cell = (const u32 *)prop->data; > > - reg_cells = address_cells + size_cells; > > - banks = fdt32_to_cpu(prop->len) / (reg_cells * sizeof(u32)); > > + device_tree_get_reg_ranges(prop, address_cells, size_cells, &banks); > > > > for ( i = 0; i < banks && early_info.mem.nr_banks < NR_MEM_BANKS; i++ ) > > { > > @@ -270,6 +295,46 @@ static void __init process_cpu_node(const void *fdt, int node, > > cpumask_set_cpu(start, &cpu_possible_map); > > } > > > > +static void __init process_gic_node(const void *fdt, int node, > > + const char *name, > > + u32 address_cells, u32 size_cells) > > +{ > > + const struct fdt_property *prop; > > + const u32 *cell; > > + paddr_t start, size; > > + int interfaces; > > + > > + if ( address_cells < 1 || size_cells < 1 ) > > + { > > + early_printk("fdt: node `%s'': invalid #address-cells or #size-cells", > > + name); > > + return; > > + } > > Is this the sort of check which the generic DT walker helper thing ought > to include?No, sometimes address_cells < 1 or size_cells < 1 are completely acceptable, see process_cpu_node for example.> > + > > + prop = fdt_get_property(fdt, node, "reg", NULL); > > + if ( !prop ) > > + { > > + early_printk("fdt: node `%s'': missing `reg'' property\n", name); > > + return; > > + } > > + > > + cell = (const u32 *)prop->data; > > + device_tree_get_reg_ranges(prop, address_cells, size_cells, &interfaces); > > + if ( interfaces < 4 ) > > + { > > + early_printk("fdt: node `%s'': invalid `reg'' property\n", name); > > A more specific message would be "not enough ranges in ..."OK