[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index] Re: [Xen-devel] [PATCH v2 05/15] xen/arm: segregate GIC low level functionality
On Fri, 2014-04-04 at 17:26 +0530, vijay.kilari@xxxxxxxxx wrote: > From: Vijaya Kumar K <Vijaya.Kumar@xxxxxxxxxxxxxxxxxx> > > GIC low level functionality is segregated into > separate functions and are called using registered > callback wherever required. > > This helps to separate generic and hardware functionality > later > > Signed-off-by: Vijaya Kumar K <Vijaya.Kumar@xxxxxxxxxxxxxxxxxx> > --- > xen/arch/arm/gic.c | 362 > ++++++++++++++++++++++++++++--------- > xen/include/asm-arm/gic.h | 50 +++++ > xen/include/asm-arm/gic_v2_defs.h | 16 +- > 3 files changed, 328 insertions(+), 100 deletions(-) > > diff --git a/xen/arch/arm/gic.c b/xen/arch/arm/gic.c > index 64699e4..9f03135 100644 > --- a/xen/arch/arm/gic.c > +++ b/xen/arch/arm/gic.c > @@ -57,8 +57,21 @@ static irq_desc_t irq_desc[NR_IRQS]; > static DEFINE_PER_CPU(irq_desc_t[NR_LOCAL_IRQS], local_irq_desc); > static DEFINE_PER_CPU(uint64_t, lr_mask); > > +static struct gic_hw_operations *gic_hw_ops; > +static struct gic_hw_operations gic_ops; Why two? Is the second one actually gic_v2_ops? > +void register_gic_ops(struct gic_hw_operations *ops) > +{ > + gic_hw_ops = ops; > +} > + > +void update_cpu_lr_mask(void) > +{ > + this_cpu(lr_mask) = 0ULL; > +} This looks more like init_cpu_lr_mask. > + > static uint8_t nr_lrs; Shouldn't everywhere be using gic_hw_ops->nr_lrs() rendering this unused? > -#define lr_all_full() (this_cpu(lr_mask) == ((1 << nr_lrs) - 1)) > +#define lr_all_full() (this_cpu(lr_mask) == ((1 << gic_hw_ops->nr_lrs()) - > 1)) I think nr_lrs can just be an integer field of gic_hw_ops, rather than using a function to return it, or perhaps a global in gic. set by the lower level driver somehow. Unless nr_lrs is somehow for gicv3? > +static void restore_state(struct vcpu *v) I suspect that all of these callbacks shoujld actually be gic_v2_restore_state or some such? > @@ -230,7 +319,7 @@ static hw_irq_controller gic_guest_irq_type = { > * - needs to be called with a valid cpu_mask, ie each cpu in the mask has > * already called gic_cpu_init > */ > -static void gic_set_irq_properties(unsigned int irq, bool_t level, > +static void gic_set_irq_property(unsigned int irq, bool_t level, > const cpumask_t *cpu_mask, > unsigned int priority) Why did this need to become singular? > +static struct dt_irq * gic_maintenance_irq(void) > +{ > + return &gic.maintenance; > +} This isn't using gic_hw_ops, so what is it for? Ian. _______________________________________________ Xen-devel mailing list Xen-devel@xxxxxxxxxxxxx http://lists.xen.org/xen-devel
|
Lists.xenproject.org is hosted with RackSpace, monitoring our |