Subject: irq_create_fwspec_mapping() in 4.8-rc2

From: [email-masked] (Marc Zyngier)
Date: Mon, 5 Sep 2016 13:56:39 +0100

On 05/09/16 13:40, Linus Walleij wrote:
> On Fri, Sep 2, 2016 at 5:31 PM, Marc Zyngier <[email-masked]> wrote:
> 
>> +Linus, Jean-Christophe, Jon
>>
>> On 02/09/16 16:15, Andras Szemzo wrote:
>>> Hi,
>>>
>>>> On 02 Sep 2016, at 17:09, Marc Zyngier <[email-masked]> wrote:
>>>>
>>>> So something has already configured the interrupt to be
>>>> IRQ_TYPE_EDGE_BOTH, and this clashes with your
>>>> IRQ_TYPE_EDGE_FALLING.
>>>>
>>>> My bet is on this one:
>>>>
>>>> diff --git a/drivers/pinctrl/pinctrl-at91.c b/drivers/pinctrl/pinctrl-at91.c
>>>> index 80daead..9f09041 100644
>>>> --- a/drivers/pinctrl/pinctrl-at91.c
>>>> +++ b/drivers/pinctrl/pinctrl-at91.c
>>>> @@ -1614,7 +1614,7 @@ static int at91_gpio_of_irq_setup(struct platform_device *pdev,
>>>>                                 &gpio_irqchip,
>>>>                                 0,
>>>>                                 handle_edge_irq,
>>>> -                               IRQ_TYPE_EDGE_BOTH);
>>>> +                               IRQ_TYPE_NONE);
>>>>      if (ret) {
>>>>              dev_err(&pdev->dev, "at91_gpio.%d: Couldn't add irqchip to gpiochip.\n",
>>>>                      at91_gpio->pioc_idx);
>>>>
>>>> Can you give it a go and let me know what happens?
>>>
>>> yes, this fixed the problem. Thank you, it was fast!
>>
>> Right. So the at91 pinctlr seems to enforce a default configuration. The
>> question is *why*? All interrupts connected to it should provide their
>> own trigger coming from DT.
> 
> I guess for legacy boardfile usecases or just how it happened to end
> up during development.
> 
>> As we now actually check that we have some consistency between what is
>> configured and what is requested, it is bound to fail, unless you happen
>> to match the default.
> 
> Which is good!

Oh, I'm glad we now have these checks in. We found quite a few oddities
thanks to those.

>> What am I missing?
> 
> Nothing, I am missing your patch on the mailing list with a Signed-off-by
> so I can apply it (unless the Atmel people have complaints).

OK, I'll whip up a patch shortly. Should I also address the handful of
other non-TYPE_NONE that we have in the tree:

	drivers/gpio/gpio-sx150x.c
	drivers/pinctrl/nomadik/pinctrl-nomadik.c
	drivers/pinctrl/pinctrl-coh901.c
	drivers/pinctrl/pinctrl-st.c

But the bigger question is whether or not there is any value in the
pinctrl driver providing a default value at all. Do we have any case
where the trigger information is not provided by the firmware (DT or
ACPI), and where we rely on the default being set?

Thanks,

	M.
-- 
Jazz is not dead. It just smells funny...