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

[Olof Johansson <olof@lixom.net>]

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.

> +static struct irq_host *master_irqhost;
> +
> +/*
> + * IRQ Chip operations
> + */
> +static void xilinx_intc_mask(unsigned int virq)
> +{
> +	int irq = irq_map[virq].hwirq;
> +	void * regs = get_irq_chip_data(virq);
> +	pr_debug("mask: %d\n", irq);
> +	out_be32(regs + CIE, 1 << irq);
> +}
> +
> +static void xilinx_intc_unmask(unsigned int virq)
> +{
> +	int irq = irq_map[virq].hwirq;
> +	void * regs = get_irq_chip_data(virq);
> +	pr_debug("unmask: %d\n", irq);
> +	out_be32(regs + SIE, 1 << irq);
> +}
> +
> +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.


-Olof