Re: [PATCH] Add TI-Nspire irqchip support

[Thomas Gleixner]

On Thu, 30 May 2013, Daniel Tang wrote:
 
> Hi,
> 
> This patch adds a driver for the interrupt controller found in the TI-Nspire 
> calculator series.
> 
> Cheers,
> Daniel Tang

This is NOT a proper changelog.
 
> Signed-off-by: Daniel Tang <dt.ta...@gmail.com>

Also please read through this mail thread:

     https://lkml.org/lkml/2013/5/2/406

and rework your patches against:

git://git.kernel.org/pub/scm/linux/kernel/git/tip/tip.git irq/core

> +static void __iomem *irq_io_base;
> +static struct irq_domain *zevio_irq_domain;
> +
> +static void zevio_irq_ack(struct irq_data *irqd)
> +{
> +     void __iomem *base = irq_io_base;
> +
> +     if (irqd->hwirq < FIQ_START)
> +             base += IO_IRQ_BASE;
> +     else
> +             base += IO_FIQ_BASE;

This is horrible. If you redo this against the generic irq chip then
provide a separate base with the proper offsets to each chip. So you
can avoid this clumsy conditionals completely.

> +     readl(base + IO_RESET);
> +}
> +
> +static void zevio_irq_unmask(struct irq_data *irqd)
> +{
> +     void __iomem *base = irq_io_base;
> +     int irqnr = irqd->hwirq;
> +
> +     if (irqnr < FIQ_START) {
> +             base += IO_IRQ_BASE;
> +     } else {
> +             irqnr -= MAX_INTRS;
> +             base += IO_FIQ_BASE;
> +     }
> +
> +     writel((1<<irqnr), base + IO_ENABLE);

Replace with the generic function

> +}
> +
> +static void zevio_irq_mask(struct irq_data *irqd)
> +{
> +     void __iomem *base = irq_io_base;
> +     int irqnr = irqd->hwirq;
> +
> +     if (irqnr < FIQ_START) {
> +             base += IO_IRQ_BASE;
> +     } else {
> +             irqnr -= FIQ_START;
> +             base += IO_FIQ_BASE;
> +     }
> +
> +     writel((1<<irqnr), base + IO_DISABLE);

Replace with the generic function

> +static int process_base(void __iomem *base, struct pt_regs *regs)
> +{
> +     int irqnr;
> +
> +
> +     if (!readl(base + IO_STATUS))
> +             return 0;
> +
> +     irqnr = readl(base + IO_CURRENT);
> +     irqnr = irq_find_mapping(zevio_irq_domain, irqnr);
> +     handle_IRQ(irqnr, regs);
> +
> +     return 1;
> +}
> +
> +asmlinkage void __exception_irq_entry zevio_handle_irq(struct pt_regs *regs)
> +{
> +     while (process_base(irq_io_base + IO_FIQ_BASE, regs))
> +             ;

Wheee. That's ugly as hell. Why don't you move the while loop into 
process_base() ?

Thanks,

        tglx
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/