Re: [Xen-devel] [RFC PATCH v1 06/10] xen/arm: split gic driver into generic and gicv2 driver

[Ian Campbell <Ian.Campbell@xxxxxxxxxx>]

On Thu, 2014-03-20 at 16:02 +0000, Julien Grall wrote:
> Hello Vijay,
> 
> Thanks for the patch.
> 
> On 03/19/2014 02:17 PM, vijay.kilari@xxxxxxxxx wrote:
> > From: Vijaya Kumar K <Vijaya.Kumar@xxxxxxxxxxxxxxxxxx>
> > 
> > Existing GIC driver has both generic code and hw specific
> > code. Segregate GIC low level driver into gic-v2.c and
> > keep generic code in existing gic.c file
> > 
> > GIC v2 driver registers required functions
> > to generic GIC driver. This helps to plug in next version
> > of GIC drivers like GIC v3.
> > 
> > Signed-off-by: Vijaya Kumar K <Vijaya.Kumar@xxxxxxxxxxxxxxxxxx>
> > ---
> 
> [..]
> 
> > +
> > +void __init gicv2_init(void)
> > +{
> 
> Instead of calling gicv2_init and gicv3_init from generic, it would be
> better to the device API (see xen/include/asm-arm/device.h). An example
> of use is my iommu series (see https://patches.linaro.org/26032/
> iommu_hardware_setup).

Is that usable for something needed as soon as interrupt controller
setup?

> > +struct gic_hw_operations {
> 
> Can you explain a bit more your structure? Why do you need so all theses
> callbacks...

In fact each should come a little doc comment about their usage, e.g.
like platform.h.

> [..]
> > +    void (*enable_irq)(int);
> > +    void (*disable_irq)(int);
> > +    void (*eoi_irq)(int);
> > +    void (*deactivate_irq)(int);
> > +    unsigned int (*ack_irq)(void);
> 
> I would prefer to introduce a new hw_controller here rather than adding
> another abstraction.

Agreed.

Ian.



_______________________________________________
Xen-devel mailing list
Xen-devel@xxxxxxxxxxxxx
http://lists.xen.org/xen-devel