Re: [PATCH 03/18] Virtex: add xilinx interrupt controller driver

["Grant Likely" <grant.likely@secretlab.ca>]

On 9/28/07, Olof Johansson <olof@lixom.net> wrote:
> On Fri, Sep 28, 2007 at 12:16:07PM -0600, Grant Likely wrote:
>
> > +/*
> > + * INTC Registers
> > + */
> > +#define ISR  0       /* Interrupt Status */
> > +#define IPR  4       /* Interrupt Pending */
> > +#define IER  8       /* Interrupt Enable */
> > +#define IAR  12      /* Interrupt Acknowledge */
> > +#define SIE  16      /* Set Interrupt Enable bits */
> > +#define CIE  20      /* Clear Interrupt Enable bits */
> > +#define IVR  24      /* Interrupt Vector */
> > +#define MER  28      /* Master Enable */
>
> The defines are fairly generic, I guess you haven't ran across cases
> where there's naming conflicts, but you might want to prefix them with
> something more unique just in case.

Will do

>
> > +static void xilinx_intc_ack(unsigned int virq)
> > +{
> > +     int irq = irq_map[virq].hwirq;
> > +     void * regs = get_irq_chip_data(virq);
> > +     pr_debug("ack: %d\n", irq);
> > +     out_be32(regs + IAR, 1 << irq);
> > +}
>
> I guess some of the above are open-coded instead of using virq_to_hw()
> for performance reasons, it could be useful to have comments regarding
> this so they aren't changed by some janitor down the road. Or, in case
> they're not performance-critical, change them to use virq_to_hw.

Or it was just that my example code from another driver wasn't using
virq_to_hw() either.  I'll fix this.

g.

-- 
Grant Likely, B.Sc., P.Eng.
Secret Lab Technologies Ltd.
grant.likely@secretlab.ca
(403) 399-0195