* irq_create_fwspec_mapping() in 4.8-rc2
@ 2016-09-02 6:51 Andras Szemzo
2016-09-02 14:15 ` Marc Zyngier
0 siblings, 1 reply; 13+ messages in thread
From: Andras Szemzo @ 2016-09-02 6:51 UTC (permalink / raw)
To: linux-arm-kernel
Hi,
I try to use 4.8-rc2 on my armv7m board, and I have a problem with GPIO IRQ.
In my device-tree, I define the ethernet PHY IRQ like this:
pioC: gpio at 0x400e1200 {
compatible = "atmel,at91sam9x5-gpio", "atmel,at91rm9200-gpio";
reg = <0x400e1200 0x200>;
interrupts = <12>;
#gpio-cells = <2>;
gpio-controller;
interrupt-controller;
#interrupt-cells = <2>;
clocks = <&pioC_clk>;
};
...
ethernet-phy at 1 {
reg = <1>;
interrupt-parent = <&pioC>;
interrupts = <11 IRQ_TYPE_EDGE_FALLING>;
reset-gpios = <&pioC 10 GPIO_ACTIVE_HIGH>;
};
with the 4.8-rc2 kernel at boot it fails with error:
irq: type mismatch, failed to map hwirq-11 for /soc/pinctrl at 0x400e0e00/gpio at 0x400e1200!
This error comes from kernel/irq/irq_domain.c, irq_create_fwspec_mapping().
My quick hack was to replace this function from 4.6.0 (where with the same device-tree, same kernel config it works)
and everything works as expected.
Maybe this hack is hiding a problem from somewhere else, but I can?t figure out what I missed.
Thanks,
Andras
^ permalink raw reply [flat|nested] 13+ messages in thread* irq_create_fwspec_mapping() in 4.8-rc2 2016-09-02 6:51 irq_create_fwspec_mapping() in 4.8-rc2 Andras Szemzo @ 2016-09-02 14:15 ` Marc Zyngier 2016-09-02 14:29 ` Andras Szemzo 0 siblings, 1 reply; 13+ messages in thread From: Marc Zyngier @ 2016-09-02 14:15 UTC (permalink / raw) To: linux-arm-kernel [cc-ing the maintainers of the code usually helps] On 02/09/16 07:51, Andras Szemzo wrote: > Hi, > > I try to use 4.8-rc2 on my armv7m board, and I have a problem with GPIO IRQ. > In my device-tree, I define the ethernet PHY IRQ like this: > > pioC: gpio at 0x400e1200 { > compatible = "atmel,at91sam9x5-gpio", "atmel,at91rm9200-gpio"; > reg = <0x400e1200 0x200>; > interrupts = <12>; > #gpio-cells = <2>; > gpio-controller; > interrupt-controller; > #interrupt-cells = <2>; > clocks = <&pioC_clk>; > }; > ... > ethernet-phy at 1 { > reg = <1>; > interrupt-parent = <&pioC>; > interrupts = <11 IRQ_TYPE_EDGE_FALLING>; So this is the interrupt that gets a mismatch. > reset-gpios = <&pioC 10 GPIO_ACTIVE_HIGH>; > }; > > with the 4.8-rc2 kernel at boot it fails with error: > > irq: type mismatch, failed to map hwirq-11 for /soc/pinctrl at 0x400e0e00/gpio at 0x400e1200! > > This error comes from kernel/irq/irq_domain.c, irq_create_fwspec_mapping(). > > My quick hack was to replace this function from 4.6.0 (where with the same device-tree, same kernel config it works) > and everything works as expected. > > Maybe this hack is hiding a problem from somewhere else, but I can?t figure out what I missed. Is there any other device in your system that uses the same interrupt? Where can I see your full DT file? Thanks, M. -- Jazz is not dead. It just smells funny... ^ permalink raw reply [flat|nested] 13+ messages in thread
* irq_create_fwspec_mapping() in 4.8-rc2 2016-09-02 14:15 ` Marc Zyngier @ 2016-09-02 14:29 ` Andras Szemzo 2016-09-02 14:47 ` Marc Zyngier 0 siblings, 1 reply; 13+ messages in thread From: Andras Szemzo @ 2016-09-02 14:29 UTC (permalink / raw) To: linux-arm-kernel Hi, > On 02 Sep 2016, at 16:15, Marc Zyngier <marc.zyngier@arm.com> wrote: > > So this is the interrupt that gets a mismatch. Yes. > Is there any other device in your system that uses the same interrupt? > Where can I see your full DT file? There is no other device using pioC 11. Here is the DT: http://pastebin.com/G7Z0hF5v ^ permalink raw reply [flat|nested] 13+ messages in thread
* irq_create_fwspec_mapping() in 4.8-rc2 2016-09-02 14:29 ` Andras Szemzo @ 2016-09-02 14:47 ` Marc Zyngier 2016-09-02 14:52 ` Andras Szemzo 0 siblings, 1 reply; 13+ messages in thread From: Marc Zyngier @ 2016-09-02 14:47 UTC (permalink / raw) To: linux-arm-kernel On 02/09/16 15:29, Andras Szemzo wrote: > Hi, > >> On 02 Sep 2016, at 16:15, Marc Zyngier <marc.zyngier@arm.com> wrote: >> >> So this is the interrupt that gets a mismatch. > > Yes. > >> Is there any other device in your system that uses the same interrupt? >> Where can I see your full DT file? > > There is no other device using pioC 11. > Here is the DT: > > http://pastebin.com/G7Z0hF5v Can you try this patch and report the values you find? diff --git a/kernel/irq/irqdomain.c b/kernel/irq/irqdomain.c index 4752b43..d79d780 100644 --- a/kernel/irq/irqdomain.c +++ b/kernel/irq/irqdomain.c @@ -623,7 +623,8 @@ unsigned int irq_create_fwspec_mapping(struct irq_fwspec *fwspec) return virq; } - pr_warn("type mismatch, failed to map hwirq-%lu for %s!\n", + pr_warn("type mismatch (%u/%u), failed to map hwirq-%lu for %s!\n", + type, irq_get_trigger_type(virq), hwirq, of_node_full_name(to_of_node(fwspec->fwnode))); return 0; } Thanks, M. -- Jazz is not dead. It just smells funny... ^ permalink raw reply related [flat|nested] 13+ messages in thread
* irq_create_fwspec_mapping() in 4.8-rc2 2016-09-02 14:47 ` Marc Zyngier @ 2016-09-02 14:52 ` Andras Szemzo 2016-09-02 15:09 ` Marc Zyngier 0 siblings, 1 reply; 13+ messages in thread From: Andras Szemzo @ 2016-09-02 14:52 UTC (permalink / raw) To: linux-arm-kernel > > Can you try this patch and report the values you find? > Sure, here is the output: irq: type mismatch (2/3), failed to map hwirq-11 for /soc/pinctrl at 0x400e0e00/gpio at 0x400e1200! ^ permalink raw reply [flat|nested] 13+ messages in thread
* irq_create_fwspec_mapping() in 4.8-rc2 2016-09-02 14:52 ` Andras Szemzo @ 2016-09-02 15:09 ` Marc Zyngier 2016-09-02 15:15 ` Andras Szemzo 0 siblings, 1 reply; 13+ messages in thread From: Marc Zyngier @ 2016-09-02 15:09 UTC (permalink / raw) To: linux-arm-kernel On 02/09/16 15:52, Andras Szemzo wrote: >> >> Can you try this patch and report the values you find? >> > Sure, here is the output: > > irq: type mismatch (2/3), failed to map hwirq-11 for /soc/pinctrl at 0x400e0e00/gpio at 0x400e1200! 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? Thanks, M. -- Jazz is not dead. It just smells funny... ^ permalink raw reply related [flat|nested] 13+ messages in thread
* irq_create_fwspec_mapping() in 4.8-rc2 2016-09-02 15:09 ` Marc Zyngier @ 2016-09-02 15:15 ` Andras Szemzo 2016-09-02 15:31 ` Marc Zyngier 0 siblings, 1 reply; 13+ messages in thread From: Andras Szemzo @ 2016-09-02 15:15 UTC (permalink / raw) To: linux-arm-kernel Hi, > On 02 Sep 2016, at 17:09, Marc Zyngier <marc.zyngier@arm.com> 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! Regards, Andras ^ permalink raw reply [flat|nested] 13+ messages in thread
* irq_create_fwspec_mapping() in 4.8-rc2 2016-09-02 15:15 ` Andras Szemzo @ 2016-09-02 15:31 ` Marc Zyngier 2016-09-05 12:40 ` Linus Walleij 0 siblings, 1 reply; 13+ messages in thread From: Marc Zyngier @ 2016-09-02 15:31 UTC (permalink / raw) To: linux-arm-kernel +Linus, Jean-Christophe, Jon On 02/09/16 16:15, Andras Szemzo wrote: > Hi, > >> On 02 Sep 2016, at 17:09, Marc Zyngier <marc.zyngier@arm.com> 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. 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. What am I missing? Thanks, M. -- Jazz is not dead. It just smells funny... ^ permalink raw reply [flat|nested] 13+ messages in thread
* irq_create_fwspec_mapping() in 4.8-rc2 2016-09-02 15:31 ` Marc Zyngier @ 2016-09-05 12:40 ` Linus Walleij 2016-09-05 12:56 ` Marc Zyngier 2016-09-05 13:10 ` Nicolas Ferre 0 siblings, 2 replies; 13+ messages in thread From: Linus Walleij @ 2016-09-05 12:40 UTC (permalink / raw) To: linux-arm-kernel On Fri, Sep 2, 2016 at 5:31 PM, Marc Zyngier <marc.zyngier@arm.com> wrote: > +Linus, Jean-Christophe, Jon > > On 02/09/16 16:15, Andras Szemzo wrote: >> Hi, >> >>> On 02 Sep 2016, at 17:09, Marc Zyngier <marc.zyngier@arm.com> 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! > 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). Yours, Linus Walleij ^ permalink raw reply [flat|nested] 13+ messages in thread
* irq_create_fwspec_mapping() in 4.8-rc2 2016-09-05 12:40 ` Linus Walleij @ 2016-09-05 12:56 ` Marc Zyngier 2016-09-05 21:41 ` Linus Walleij 2016-09-05 13:10 ` Nicolas Ferre 1 sibling, 1 reply; 13+ messages in thread From: Marc Zyngier @ 2016-09-05 12:56 UTC (permalink / raw) To: linux-arm-kernel On 05/09/16 13:40, Linus Walleij wrote: > On Fri, Sep 2, 2016 at 5:31 PM, Marc Zyngier <marc.zyngier@arm.com> wrote: > >> +Linus, Jean-Christophe, Jon >> >> On 02/09/16 16:15, Andras Szemzo wrote: >>> Hi, >>> >>>> On 02 Sep 2016, at 17:09, Marc Zyngier <marc.zyngier@arm.com> 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... ^ permalink raw reply [flat|nested] 13+ messages in thread
* irq_create_fwspec_mapping() in 4.8-rc2 2016-09-05 12:56 ` Marc Zyngier @ 2016-09-05 21:41 ` Linus Walleij 2016-09-06 5:53 ` Nicolas Ferre 0 siblings, 1 reply; 13+ messages in thread From: Linus Walleij @ 2016-09-05 21:41 UTC (permalink / raw) To: linux-arm-kernel On Mon, Sep 5, 2016 at 2:56 PM, Marc Zyngier <marc.zyngier@arm.com> wrote: > 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? That happened with boardfiles. Which used to be the case with some of these, but AFAICT not any longer. I am uncertain about sx150x though. In the boardfile case, I think actually the driver is responsible to set up the kind of interrupt trigger it wants (well it still can... but it should nowadays trust what it is given) but very often that did not happen, instead the drivers relied on hacks like these to have set up a default trigger type. Yours, Linus Walleij ^ permalink raw reply [flat|nested] 13+ messages in thread
* irq_create_fwspec_mapping() in 4.8-rc2 2016-09-05 21:41 ` Linus Walleij @ 2016-09-06 5:53 ` Nicolas Ferre 0 siblings, 0 replies; 13+ messages in thread From: Nicolas Ferre @ 2016-09-06 5:53 UTC (permalink / raw) To: linux-arm-kernel Le 05/09/2016 ? 23:41, Linus Walleij a ?crit : > On Mon, Sep 5, 2016 at 2:56 PM, Marc Zyngier <marc.zyngier@arm.com> wrote: > >> 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? > > That happened with boardfiles. Which used to be the case with > some of these, but AFAICT not any longer. I am uncertain about > sx150x though. FYI: Must have been this boardfile historical reason for at91 as well. As we only rely on DT now, we're safe. Bye, > In the boardfile case, I think actually the driver is responsible to > set up the kind of interrupt trigger it wants (well it still can... > but it should nowadays trust what it is given) but very often that > did not happen, instead the drivers relied on hacks like these to > have set up a default trigger type. > > Yours, > Linus Walleij > -- Nicolas Ferre ^ permalink raw reply [flat|nested] 13+ messages in thread
* irq_create_fwspec_mapping() in 4.8-rc2 2016-09-05 12:40 ` Linus Walleij 2016-09-05 12:56 ` Marc Zyngier @ 2016-09-05 13:10 ` Nicolas Ferre 1 sibling, 0 replies; 13+ messages in thread From: Nicolas Ferre @ 2016-09-05 13:10 UTC (permalink / raw) To: linux-arm-kernel Le 05/09/2016 ? 14:40, Linus Walleij a ?crit : > On Fri, Sep 2, 2016 at 5:31 PM, Marc Zyngier <marc.zyngier@arm.com> wrote: > >> +Linus, Jean-Christophe, Jon >> >> On 02/09/16 16:15, Andras Szemzo wrote: >>> Hi, >>> >>>> On 02 Sep 2016, at 17:09, Marc Zyngier <marc.zyngier@arm.com> 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! > >> 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). Absolutely no complains, I've only noticed this discussion at the moment. Thanks Marc and Andras! Bye, -- Nicolas Ferre ^ permalink raw reply [flat|nested] 13+ messages in thread
end of thread, other threads:[~2016-09-06 5:53 UTC | newest] Thread overview: 13+ messages (download: mbox.gz / follow: Atom feed) -- links below jump to the message on this page -- 2016-09-02 6:51 irq_create_fwspec_mapping() in 4.8-rc2 Andras Szemzo 2016-09-02 14:15 ` Marc Zyngier 2016-09-02 14:29 ` Andras Szemzo 2016-09-02 14:47 ` Marc Zyngier 2016-09-02 14:52 ` Andras Szemzo 2016-09-02 15:09 ` Marc Zyngier 2016-09-02 15:15 ` Andras Szemzo 2016-09-02 15:31 ` Marc Zyngier 2016-09-05 12:40 ` Linus Walleij 2016-09-05 12:56 ` Marc Zyngier 2016-09-05 21:41 ` Linus Walleij 2016-09-06 5:53 ` Nicolas Ferre 2016-09-05 13:10 ` Nicolas Ferre
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox; as well as URLs for NNTP newsgroup(s).