| Message ID | 1423254173-31852-1-git-send-email-geert+renesas@glider.be (mailing list archive) |
|---|---|
| State | RFC |
| Delegated to: | Simon Horman |
| Headers | show |
* Geert Uytterhoeven <geert+renesas@glider.be> [150206 12:26]: > Currently the pin function controller (which is also a GPIO controller) > is instantiated before the interrupt controllers due to the order in the > DTS. At that time, the irq domains for the interrupt controllers > referenced by its interrupts-extended property cannot be found yet: > > irq: no irq domain found for /interrupt-controller@e61c0000 ! > > Nevertheless, the core OF probing code ignores this failure, besides a > debug message that's not normally printed: > > not all legacy IRQ resources mapped for pfc > > and continues initialization of the device. Then, the sh-pfc driver > cannot find any IRQ resources, and thinks no interrupts are available, > causing gpio-keys to fail later: > > gpio-keys keyboard: Unable to claim irq 0; error -22 > gpio-keys: probe of keyboard failed with error -22 > > Move the pin function controller node after the interrupt controller > nodes it references to work around the bug in the core OF probing code. > > Signed-off-by: Geert Uytterhoeven <geert+renesas@glider.be> > --- > Notes: > - It seems several people tried to solve this in the core OF probing > code before, but the final solution never went in? > - This can be reproduced on other SoCs (e.g. sh73a0 and r8a7740) by > moving their pfc nodes before their interrupt controller nodes. > - This patch is against my working tree, so it doesn't apply to > Simon's repository, but you get the idea.... No issues with the patch, but here are few comments on the core reasons (without looking at the code in this case) that might help fix similar issues. In all the cases I've seen these errors are caused by non-standard custom initcall levels for drivers like i2c bus. The real solution is to initialize drivers later with standard module_init, and stop the race to the bottom with custom initcall levels. If there is legacy board specific platofrm init code that needs i2c gpios early, that code can probably be moved to initialize later on. Also, there should not be any need for custom driver initcall levels from Linux generic framework point of view as for example irqchip implementing drivers work just fine as a loadable module. Regards, Tony -- To unsubscribe from this list: send the line "unsubscribe linux-sh" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Hi Tony, On Mon, Feb 9, 2015 at 5:24 PM, Tony Lindgren <tony@atomide.com> wrote: > * Geert Uytterhoeven <geert+renesas@glider.be> [150206 12:26]: >> Currently the pin function controller (which is also a GPIO controller) >> is instantiated before the interrupt controllers due to the order in the >> DTS. At that time, the irq domains for the interrupt controllers >> referenced by its interrupts-extended property cannot be found yet: >> >> irq: no irq domain found for /interrupt-controller@e61c0000 ! >> >> Nevertheless, the core OF probing code ignores this failure, besides a >> debug message that's not normally printed: >> >> not all legacy IRQ resources mapped for pfc >> >> and continues initialization of the device. Then, the sh-pfc driver >> cannot find any IRQ resources, and thinks no interrupts are available, >> causing gpio-keys to fail later: >> >> gpio-keys keyboard: Unable to claim irq 0; error -22 >> gpio-keys: probe of keyboard failed with error -22 >> >> Move the pin function controller node after the interrupt controller >> nodes it references to work around the bug in the core OF probing code. >> >> Signed-off-by: Geert Uytterhoeven <geert+renesas@glider.be> >> --- >> Notes: >> - It seems several people tried to solve this in the core OF probing >> code before, but the final solution never went in? >> - This can be reproduced on other SoCs (e.g. sh73a0 and r8a7740) by >> moving their pfc nodes before their interrupt controller nodes. >> - This patch is against my working tree, so it doesn't apply to >> Simon's repository, but you get the idea.... > > No issues with the patch, but here are few comments on the core > reasons (without looking at the code in this case) that might help > fix similar issues. > > In all the cases I've seen these errors are caused by non-standard > custom initcall levels for drivers like i2c bus. The real solution > is to initialize drivers later with standard module_init, and stop > the race to the bottom with custom initcall levels. > > If there is legacy board specific platofrm init code that needs > i2c gpios early, that code can probably be moved to initialize > later on. In this case no i2c is involved. The drivers for both pinctrl (renesas,pfc-r8a73a4) and irqchip (renesas,irqc) are registered at the same level: - postcore_initcall(sh_pfc_init); - postcore_initcall(irqc_init); Hence the system uses the "natural" order from within the DTS, and decided to instantiate the pfc before the irqchip. > Also, there should not be any need for custom driver initcall > levels from Linux generic framework point of view as for example > irqchip implementing drivers work just fine as a loadable module. As long as no other device that's instantiated earlier references that irqchip? Gr{oetje,eeting}s, Geert -- Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@linux-m68k.org In personal conversations with technical people, I call myself a hacker. But when I'm talking to journalists I just say "programmer" or something like that. -- Linus Torvalds -- To unsubscribe from this list: send the line "unsubscribe linux-sh" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
* Geert Uytterhoeven <geert@linux-m68k.org> [150209 09:17]: > On Mon, Feb 9, 2015 at 5:24 PM, Tony Lindgren <tony@atomide.com> wrote: > > * Geert Uytterhoeven <geert+renesas@glider.be> [150206 12:26]: > >> Notes: > >> - It seems several people tried to solve this in the core OF probing > >> code before, but the final solution never went in? > >> - This can be reproduced on other SoCs (e.g. sh73a0 and r8a7740) by > >> moving their pfc nodes before their interrupt controller nodes. > >> - This patch is against my working tree, so it doesn't apply to > >> Simon's repository, but you get the idea.... > > > > No issues with the patch, but here are few comments on the core > > reasons (without looking at the code in this case) that might help > > fix similar issues. > > > > In all the cases I've seen these errors are caused by non-standard > > custom initcall levels for drivers like i2c bus. The real solution > > is to initialize drivers later with standard module_init, and stop > > the race to the bottom with custom initcall levels. > > > > If there is legacy board specific platofrm init code that needs > > i2c gpios early, that code can probably be moved to initialize > > later on. > > In this case no i2c is involved. The drivers for both pinctrl > (renesas,pfc-r8a73a4) and irqchip (renesas,irqc) are registered > at the same level: > - postcore_initcall(sh_pfc_init); > - postcore_initcall(irqc_init); > Hence the system uses the "natural" order from within the DTS, > and decided to instantiate the pfc before the irqchip. OK > > Also, there should not be any need for custom driver initcall > > levels from Linux generic framework point of view as for example > > irqchip implementing drivers work just fine as a loadable module. > > As long as no other device that's instantiated earlier references that > irqchip? Right :) And deferred probe won't still remove the warning in that case. From what I remember, that's a valid warning from irq framework point of view. Regards, Tony -- To unsubscribe from this list: send the line "unsubscribe linux-sh" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On Mon, Feb 9, 2015 at 7:29 PM, Tony Lindgren <tony@atomide.com> wrote: > * Geert Uytterhoeven <geert@linux-m68k.org> [150209 09:17]: >> On Mon, Feb 9, 2015 at 5:24 PM, Tony Lindgren <tony@atomide.com> wrote: >> > * Geert Uytterhoeven <geert+renesas@glider.be> [150206 12:26]: >> >> Notes: >> >> - It seems several people tried to solve this in the core OF probing >> >> code before, but the final solution never went in? >> >> - This can be reproduced on other SoCs (e.g. sh73a0 and r8a7740) by >> >> moving their pfc nodes before their interrupt controller nodes. >> >> - This patch is against my working tree, so it doesn't apply to >> >> Simon's repository, but you get the idea.... >> > >> > No issues with the patch, but here are few comments on the core >> > reasons (without looking at the code in this case) that might help >> > fix similar issues. >> > >> > In all the cases I've seen these errors are caused by non-standard >> > custom initcall levels for drivers like i2c bus. The real solution >> > is to initialize drivers later with standard module_init, and stop >> > the race to the bottom with custom initcall levels. >> > >> > If there is legacy board specific platofrm init code that needs >> > i2c gpios early, that code can probably be moved to initialize >> > later on. >> >> In this case no i2c is involved. The drivers for both pinctrl >> (renesas,pfc-r8a73a4) and irqchip (renesas,irqc) are registered >> at the same level: >> - postcore_initcall(sh_pfc_init); >> - postcore_initcall(irqc_init); >> Hence the system uses the "natural" order from within the DTS, >> and decided to instantiate the pfc before the irqchip. > > OK I tried converting the renesas,irqc driver to IRQCHIP_DECLARE(). This solves the probe ordering problem, but it no longer allows the use of a platform device. As I was already adding clock and runtime PM support (which use platform devices), IRQCHIP_DECLARE doesn't look like the right road to follow... Gr{oetje,eeting}s, Geert -- Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@linux-m68k.org In personal conversations with technical people, I call myself a hacker. But when I'm talking to journalists I just say "programmer" or something like that. -- Linus Torvalds -- To unsubscribe from this list: send the line "unsubscribe linux-sh" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Hi Geert, On Tuesday 10 February 2015 11:19:39 Geert Uytterhoeven wrote: > On Mon, Feb 9, 2015 at 7:29 PM, Tony Lindgren <tony@atomide.com> wrote: > > * Geert Uytterhoeven <geert@linux-m68k.org> [150209 09:17]: > >> On Mon, Feb 9, 2015 at 5:24 PM, Tony Lindgren <tony@atomide.com> wrote: > >>> * Geert Uytterhoeven <geert+renesas@glider.be> [150206 12:26]: > >>>> Notes: > >>>> - It seems several people tried to solve this in the core OF probing > >>>> code before, but the final solution never went in? > >>>> > >>>> - This can be reproduced on other SoCs (e.g. sh73a0 and r8a7740) by > >>>> moving their pfc nodes before their interrupt controller nodes. > >>>> > >>>> - This patch is against my working tree, so it doesn't apply to > >>>> Simon's repository, but you get the idea.... > >>> > >>> No issues with the patch, but here are few comments on the core > >>> reasons (without looking at the code in this case) that might help > >>> fix similar issues. > >>> > >>> In all the cases I've seen these errors are caused by non-standard > >>> custom initcall levels for drivers like i2c bus. The real solution > >>> is to initialize drivers later with standard module_init, and stop > >>> the race to the bottom with custom initcall levels. > >>> > >>> If there is legacy board specific platofrm init code that needs > >>> i2c gpios early, that code can probably be moved to initialize > >>> later on. > >> > >> In this case no i2c is involved. The drivers for both pinctrl > >> (renesas,pfc-r8a73a4) and irqchip (renesas,irqc) are registered > >> > >> at the same level: > >> - postcore_initcall(sh_pfc_init); > >> - postcore_initcall(irqc_init); > >> > >> Hence the system uses the "natural" order from within the DTS, > >> and decided to instantiate the pfc before the irqchip. > > > > OK > > I tried converting the renesas,irqc driver to IRQCHIP_DECLARE(). > This solves the probe ordering problem, but it no longer allows the use of > a platform device. One (quite hackish) possible way around this is to register the platform driver (using a static variable to ensure the driver isn't registered multiple times) in the IRQCHIP_DECLARE'd init function, and then call of_platform_device_create() to create the platform device. This should get the device probed immediately. > As I was already adding clock and runtime PM support > (which use platform devices), IRQCHIP_DECLARE doesn't look like the > right road to follow...
diff --git a/arch/arm/boot/dts/r8a73a4.dtsi b/arch/arm/boot/dts/r8a73a4.dtsi index 8a23442f0c70359d..47b657de4f68f56c 100644 --- a/arch/arm/boot/dts/r8a73a4.dtsi +++ b/arch/arm/boot/dts/r8a73a4.dtsi @@ -259,30 +259,6 @@ }; }; - pfc: pfc@e6050000 { - compatible = "renesas,pfc-r8a73a4"; - reg = <0 0xe6050000 0 0x9000>; - gpio-controller; - #gpio-cells = <2>; - interrupts-extended = - <&irqc0 0 0>, <&irqc0 1 0>, <&irqc0 2 0>, <&irqc0 3 0>, - <&irqc0 4 0>, <&irqc0 5 0>, <&irqc0 6 0>, <&irqc0 7 0>, - <&irqc0 8 0>, <&irqc0 9 0>, <&irqc0 10 0>, <&irqc0 11 0>, - <&irqc0 12 0>, <&irqc0 13 0>, <&irqc0 14 0>, <&irqc0 15 0>, - <&irqc0 16 0>, <&irqc0 17 0>, <&irqc0 18 0>, <&irqc0 19 0>, - <&irqc0 20 0>, <&irqc0 21 0>, <&irqc0 22 0>, <&irqc0 23 0>, - <&irqc0 24 0>, <&irqc0 25 0>, <&irqc0 26 0>, <&irqc0 27 0>, - <&irqc0 28 0>, <&irqc0 29 0>, <&irqc0 30 0>, <&irqc0 31 0>, - <&irqc1 0 0>, <&irqc1 1 0>, <&irqc1 2 0>, <&irqc1 3 0>, - <&irqc1 4 0>, <&irqc1 5 0>, <&irqc1 6 0>, <&irqc1 7 0>, - <&irqc1 8 0>, <&irqc1 9 0>, <&irqc1 10 0>, <&irqc1 11 0>, - <&irqc1 12 0>, <&irqc1 13 0>, <&irqc1 14 0>, <&irqc1 15 0>, - <&irqc1 16 0>, <&irqc1 17 0>, <&irqc1 18 0>, <&irqc1 19 0>, - <&irqc1 20 0>, <&irqc1 21 0>, <&irqc1 22 0>, <&irqc1 23 0>, - <&irqc1 24 0>, <&irqc1 25 0>; - power-domains = <&pd_c5>; - }; - i2c5: i2c@e60b0000 { #address-cells = <1>; #size-cells = <0>; @@ -382,6 +358,30 @@ power-domains = <&pd_c4>; }; + pfc: pfc@e6050000 { + compatible = "renesas,pfc-r8a73a4"; + reg = <0 0xe6050000 0 0x9000>; + gpio-controller; + #gpio-cells = <2>; + interrupts-extended = + <&irqc0 0 0>, <&irqc0 1 0>, <&irqc0 2 0>, <&irqc0 3 0>, + <&irqc0 4 0>, <&irqc0 5 0>, <&irqc0 6 0>, <&irqc0 7 0>, + <&irqc0 8 0>, <&irqc0 9 0>, <&irqc0 10 0>, <&irqc0 11 0>, + <&irqc0 12 0>, <&irqc0 13 0>, <&irqc0 14 0>, <&irqc0 15 0>, + <&irqc0 16 0>, <&irqc0 17 0>, <&irqc0 18 0>, <&irqc0 19 0>, + <&irqc0 20 0>, <&irqc0 21 0>, <&irqc0 22 0>, <&irqc0 23 0>, + <&irqc0 24 0>, <&irqc0 25 0>, <&irqc0 26 0>, <&irqc0 27 0>, + <&irqc0 28 0>, <&irqc0 29 0>, <&irqc0 30 0>, <&irqc0 31 0>, + <&irqc1 0 0>, <&irqc1 1 0>, <&irqc1 2 0>, <&irqc1 3 0>, + <&irqc1 4 0>, <&irqc1 5 0>, <&irqc1 6 0>, <&irqc1 7 0>, + <&irqc1 8 0>, <&irqc1 9 0>, <&irqc1 10 0>, <&irqc1 11 0>, + <&irqc1 12 0>, <&irqc1 13 0>, <&irqc1 14 0>, <&irqc1 15 0>, + <&irqc1 16 0>, <&irqc1 17 0>, <&irqc1 18 0>, <&irqc1 19 0>, + <&irqc1 20 0>, <&irqc1 21 0>, <&irqc1 22 0>, <&irqc1 23 0>, + <&irqc1 24 0>, <&irqc1 25 0>; + power-domains = <&pd_c5>; + }; + thermal@e61f0000 { compatible = "renesas,thermal-r8a73a4", "renesas,rcar-thermal"; reg = <0 0xe61f0000 0 0x14>, <0 0xe61f0100 0 0x38>,
Currently the pin function controller (which is also a GPIO controller) is instantiated before the interrupt controllers due to the order in the DTS. At that time, the irq domains for the interrupt controllers referenced by its interrupts-extended property cannot be found yet: irq: no irq domain found for /interrupt-controller@e61c0000 ! Nevertheless, the core OF probing code ignores this failure, besides a debug message that's not normally printed: not all legacy IRQ resources mapped for pfc and continues initialization of the device. Then, the sh-pfc driver cannot find any IRQ resources, and thinks no interrupts are available, causing gpio-keys to fail later: gpio-keys keyboard: Unable to claim irq 0; error -22 gpio-keys: probe of keyboard failed with error -22 Move the pin function controller node after the interrupt controller nodes it references to work around the bug in the core OF probing code. Signed-off-by: Geert Uytterhoeven <geert+renesas@glider.be> --- Notes: - It seems several people tried to solve this in the core OF probing code before, but the final solution never went in? - This can be reproduced on other SoCs (e.g. sh73a0 and r8a7740) by moving their pfc nodes before their interrupt controller nodes. - This patch is against my working tree, so it doesn't apply to Simon's repository, but you get the idea.... --- arch/arm/boot/dts/r8a73a4.dtsi | 48 +++++++++++++++++++++--------------------- 1 file changed, 24 insertions(+), 24 deletions(-)