On Thu, Apr 18, 2013 at 11:35:22AM +0200, Thomas Gleixner wrote:
> On Wed, 17 Apr 2013, Uwe Kleine-König wrote:
> > +struct nvic_bank_data {
> > + /*
> > + * For irq i base holds nvic_base + 4 * i / 32. So you can access the
> > + * right ISER register (i.e ISER[i / 32]) by just taking base + ISER.
> > + * Ditto for ICER.
> > + */
> > + void __iomem *base;
> > +};
>
> What's the point of a struct with a single member? Why not having an
> array of base pointers ?
It gives a name to that single member and maybe makes future changes
easier. Obviously you could argue ...
When switching to generic irq chip this struct probably goes away, so I
suggest to postpone this discussion.
> > +static struct nvic_chip_data {
> > + struct irq_domain *domain;
> > + struct nvic_bank_data bdata[NVIC_MAX_BANKS];
> > +} nvic_chip_data;
> > +
> > +asmlinkage void __exception_irq_entry
> > +nvic_do_IRQ(irq_hw_number_t hwirq, struct pt_regs *regs)
> > +{
> > + unsigned int irq = irq_linear_revmap(nvic_chip_data.domain, hwirq);
> > +
> > + handle_IRQ(irq, regs);
> > +}
> > +
> > +static inline void __iomem *nvic_bank_base(struct irq_data *d)
> > +{
> > + struct nvic_bank_data *bank_data = irq_data_get_irq_chip_data(d);
> > + return bank_data->base;
> > +}
> > +
> > +static void nvic_mask_irq(struct irq_data *d)
> > +{
> > + u32 mask = 1 << (d->hwirq % 32);
> > +
> > + writel_relaxed(mask, nvic_bank_base(d) + NVIC_ICER);
> > +}
> > +
> > +static void nvic_unmask_irq(struct irq_data *d)
> > +{
> > + u32 mask = 1 << (d->hwirq % 32);
> > +
> > + writel_relaxed(mask, nvic_bank_base(d) + NVIC_ISER);
> > +}
>
> How is that different from what the generic irq chip implementation
> does? The only difference is that mask is generated by d->hwirq and
> not by d->irq. And due to the fact, that you use a full linear mapping
> between hwirq and virq the generic code simply works.
I'm not sure what you mean when you say "full linear mapping". AFAICT
using irq_domain_add_linear doesn't imply that two consecutive hardware
irq numbers get consecutive Linux irq numbers, so using d->irq won't work.
> Even if it would not work, it would be trivial to extend the generic
> chip with that functionality instead of hacking another slightly
> different copy of the same thing.
I will try that and report back.
Best regards
Uwe
--
Pengutronix e.K. | Uwe Kleine-König |
Industrial Linux Solutions | http://www.pengutronix.de/ |
--
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/
