Re: [PATCH 1/1] of/irq: store IRQ trigger/level in struct resource flags

[Rob Herring <robherring2@gmail.com>]

On 04/08/2013 05:56 PM, Javier Martinez Canillas wrote:
> On 04/09/2013 12:16 AM, Stephen Warren wrote:
>> On 04/08/2013 04:05 PM, Rob Herring wrote:
>>> On 04/05/2013 02:48 AM, Javier Martinez Canillas wrote:
>>>> According to Documentation/devicetree/bindings/interrupt-controller/interrupts.txt
>>>> the "#interrupt-cells" property of an "interrupt-controller" is used
>>>> to define the number of cells needed to specify a single interrupt.
>> ...
>>>> But the type is never returned so it can't be saved on the IRQ struct
>>>> resource flags member.
>>>>
>>>> This means that drivers that need the IRQ type/level flags defined in
>>>> the DT won't be able to get it.
>>>
>>> But the interrupt controllers that need the information should be able
>>> to get to it via irqd_get_trigger_type. What problem exactly are you
>>> trying to fix? What driver would use this?
>>
>> FYI, that is indeed what I did in sound/soc/codecs/wm8903.c. Thinking
>> back, I'm not sure if that was the right thing or whether I should have
>> sent this same patch:-)
>>
> 
> Hi Stephen,
> 
> I'm glad you agree :-)
> 
> I could change drivers/net/ethernet/smsc/smsc911x.c to get the type flags for
> the IRQ with irqd_get_trigger_type() but I prefer $subject because:

irqd_get_trigger_type probably is not meant for outside of irqchips.
Creating an irq_get_irq_type function which takes an irq number would be
the right function as that does not expose struct irq_data.

> a) This works in the non-DT case with board files and filling the resources from
> platform data in arch/arm/mach-omap2/gpmc-smsc911x.c. So this is definitely a
> bug on the DT core.

And hackery/abuse like this:

arch/arm/mach-omap2/board-3630sdp.c:32:    .flags          =
GPMC_MUX_ADD_DATA | IORESOURCE_IRQ_LOWLEVEL,

> b) I don't see why of_irq_to_resource() should discard the type/level flags when
> filling the IORESOURCE_IRQ if it was specified on the DT.
> 
> c) We will have to change all drivers that expect to get the IRQ type flags from
> a IORESOURCE_IRQ struct resource.

I'm not convinced that is a high number of drivers. Nearly all the
occurrences of IORESOURCE_IRQ_ in drivers/ are for ISA (acpi/pnp) and
drivers for ISA devices.

Rob