[PATCH] genirq: move mask_cache into struct irq_chip_type

[saeed bishara]

On Tue, Jul 26, 2011 at 6:39 PM, Simon Guinot <simon at sequanux.org> wrote:
> Hi Saeed,
>
> On Tue, Jul 26, 2011 at 05:35:50PM +0300, saeed bishara wrote:
>> On Fri, Jul 22, 2011 at 3:49 AM, Simon Guinot <simon at sequanux.org> wrote:
>> > From: Simon Guinot <sguinot at lacie.com>
>> >
>> > This fixes a regression introduced by e59347a
>> > "arm: orion: Use generic irq chip".
>> >
>> > The same interrupt mask cache (stored within struct irq_chip_generic)
>> > is shared between all the irq_chip_type instances. As each irq_chip_type
>> > can use a distinct mask register, share a single mask cache is not
>> > correct. This bug affects Orion SoCs, which have separate mask registers
>> > for edge and level interrupts.
>> >
>> > This patch move mask_cache from struct irq_chip_generic into struct
>> > irq_chip_type. Note that the interrupt support for Samsung SoCs is also
>> > slightly affected.
>> The patch looks to fix the issue with orion, but it seems that it
>> won't work for SoC with multiple irq_chip_type that use one mask
>> register.
>
> Yes indeed, but does such SoCs exists ? I mean that the only supported
> SoC using multiple irq_chip_type (for now) is Orion.
thats right, but as you code is generic, we should find some way to
fix it or to prevent such devices to use this generic code.
 What is the most
> generic case for edge/level interrupts ? shared or separated mask
> registers ?
orion gpios use separate mask.
>
> If Orion is the specific case, maybe we could provide a dedicated
> irq_mask() handler instead of using the generic one. It would be a
> little sad.
I personally prefer this method, along with getting rid off the
multiple irq_chip_types, my main consideration in this case is
performance.
the irq_gc_mask_clr_bit/irq_gc_ack_set_bit/irq_gc_unmask_enable_reg
are critical since it get called for every (level) interrupt, and it
would be great if you optimize those functions,
I think you better do the following:
1. use pre-calculated offsets instead using  gc->reg_base + cur_regs(d)->ack.
2. put all hot variables (lock, mask/ack/eoi register offset) in the
same cache line if possible.

regards
saeed