Re: [PATCH v5 2/6] xen/arm: IRQ: Store IRQ type in arch_irq_desc

[Julien Grall <julien.grall@linaro.org>]

Hi Ian,

On 05/08/2014 11:03 AM, Ian Campbell wrote:
> On Fri, 2014-05-02 at 16:52 +0100, Julien Grall wrote:
>> For now, ARM uses different IRQ functions to setup an interrupt handler. This
>> is a bit annoying for common driver because we have to add idefery when
>> an IRQ is setup (see ns16550_init_postirq for an example).
>>
>> To avoid to completely fork the IRQ management code, we can introduce a field
>> to store the IRQ type (e.g level/edge ...).
>>
>> This patch also adds platform_get_irq which will retrieve the IRQ from the
>> device tree and setup correctly the IRQ type.
>>
>> In order to use this solution, we have to move init_IRQ earlier for the boot
>> CPU. It's fine because the code only depends on percpu.
>>
>> Signed-off-by: Julien Grall <julien.grall@linaro.org>
> 
> Looks good. But based on the comments raised previously I am wondering
> if 
> 	static int ppi_types[NR_PPIS]
> which is set on configuration of such an interrupt and propagated to the
> cpu local copy when a CPU comes up?
> 
> Apart from avoiding the slightly odd per_cpu(0) (even if hidden behind a
> macro) it also means that there is less need to be concerned about which
> processor ends up initialising an interrupt, which would simplify some
> stuff around irq_set_type and platform_get_irq (essentially you could
> then sanity check the change against ppi_types and then propagate to all
> online cpus without further checks).
> 
> Thoughts?

It looks good to me. I will give a try with this solution.

-- 
Julien Grall