Re: [PATCH for-4.5 7/8] xen/irq: Handle multiple action per IRQ

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

Hi Stefano,

On 03/18/2014 02:06 PM, Stefano Stabellini wrote:
>> On 03/18/2014 09:33 AM, Ian Campbell wrote:
>>> On Mon, 2014-03-17 at 21:05 +0000, Julien Grall wrote:
>>>> For instance for the SMMU on midway, the device tree bindings is:
>>>>
>>>>                smmu_sata: smmu@9,20180000 {
>>>>                        compatible = "arm,mmu-400";
>>>>                        reg = <0x9 0x20180000 0x10000>;
>>>>                        mmu-masters = <&sata 0 1 2 3 4 5 6 7 8 9>;
>>>>                        #global-interrupts = <1>;
>>>>                        interrupts = <0 114 4 0 114 4>;
>>>>                        calxeda,smmu-secure-config-access;
>>>>                        arm,smmu-isolate-devices;
>>>>               };
>>>>
>>>> As you can see the same interrupts is used twice:
>>>
>>> Is that actually valid in device tree? Or is this a quirk of the midway
>>> DT?
>>
>> Yes it's valid. The interrupts property for the SMMU is described as:
>>
>> "Interrupt list, with the first #global-irqs entries corresponding to
>> the global interrupts and any following entries corresponding to context
>> interrupts, specified in order of their indexing by the SMMU.
>>
>> For SMMUv2 implementations, there must be exactly one interrupt per
>> context bank. In the case of a single, combined interrupt, it must be
>> listed multiple times."
>>
>> On midway there is only one IRQ with is used for both context interrupt
>> and global interrupt. As it's the only platform on Linux with SMMU
>> support in the device tree, we don't know if every platform will have
>> the same behavior.
> 
> I understand that the SMMU might reuse the same IRQ for multiple
> purposes. I would still handle the scenario entirely within the SMMU
> driver. Can't we register a single handler for each of the IRQ listed
> under the SMMU node and then figure out what was the notification for in
> the handler? 
> 

We will have to check in the SMMU drivers, if the IRQ was already
registered or not (because we don't know in advance if the IRQ is
re-used). If not, Xen will register it with a new handler.

The code to register the IRQ handler will looks like:

int num_irqs = dt_number_of_irq(smmu->node);
dt_irqs irqs[*];
dt_irq *irq;

for ( i = 0; i < num_irqs; i++ )
{
   irq = dt_device_get_irq(smmu->node, i);
   foreach ( a in irqs )
       if ( irq == a )
         continue;
   request_dt_irq();
   irqs <= irq;
}

I understand the point that people can badly use multiple action in the
future, but is it a good reason to make the code more difficult to
understand?

Regards,

-- 
Julien Grall