| Message ID | 56EBD4E5.1060002@nvidia.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
Hi Jon, On Fri, Mar 18, 2016 at 11:13 AM, Jon Hunter <jonathanh@nvidia.com> wrote: > On 18/03/16 09:13, Geert Uytterhoeven wrote: >> On Thu, Mar 17, 2016 at 3:19 PM, Jon Hunter <jonathanh@nvidia.com> wrote: >>> Commit afbbd2338176 ("irqchip/gic: Document optional Clock and Power >>> Domain properties") documented optional clock and power-dmoain properties >>> for the ARM GIC. Currently, there are no users of these and for the >>> Tegra210 Audio GIC (based upon the GIC-400) there are two clocks, a >>> functional clock and interface clock, that need to be enabled. >> >> The reason that there are no users for this is twofold: >> 1. The GIC driver doesn't have Runtime PM support yet, >> 2. There was no clean way to prevent the GIC's clock from being disabled. >> Due to this, adding the clocks to the DTSes would mean that they will be >> disabled during boot up as unused clocks, leading to a system lock-up. >> >> I had hoped your series would fix part 1. I gave it a try on r8a7791/koelsch, >> but unfortunately it seems the platform driver only supports non-root >> controllers, while the r8a7791 GIC is the primary one... > > Can you try making the following change ... Thanks! I gave it a try, but no difference. 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-omap" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On 18/03/16 10:52, Geert Uytterhoeven wrote: > Hi Jon, > > On Fri, Mar 18, 2016 at 11:13 AM, Jon Hunter <jonathanh@nvidia.com> wrote: >> On 18/03/16 09:13, Geert Uytterhoeven wrote: >>> On Thu, Mar 17, 2016 at 3:19 PM, Jon Hunter <jonathanh@nvidia.com> wrote: >>>> Commit afbbd2338176 ("irqchip/gic: Document optional Clock and Power >>>> Domain properties") documented optional clock and power-dmoain properties >>>> for the ARM GIC. Currently, there are no users of these and for the >>>> Tegra210 Audio GIC (based upon the GIC-400) there are two clocks, a >>>> functional clock and interface clock, that need to be enabled. >>> >>> The reason that there are no users for this is twofold: >>> 1. The GIC driver doesn't have Runtime PM support yet, >>> 2. There was no clean way to prevent the GIC's clock from being disabled. >>> Due to this, adding the clocks to the DTSes would mean that they will be >>> disabled during boot up as unused clocks, leading to a system lock-up. >>> >>> I had hoped your series would fix part 1. I gave it a try on r8a7791/koelsch, >>> but unfortunately it seems the platform driver only supports non-root >>> controllers, while the r8a7791 GIC is the primary one... >> >> Can you try making the following change ... > > Thanks! I gave it a try, but no difference. I assume you added the appropriate compatible flag? Any more details you can share about why it is not working? Is it not registered early enough? Jon -- To unsubscribe from this list: send the line "unsubscribe linux-omap" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Hi Jon, On Fri, Mar 18, 2016 at 11:56 AM, Jon Hunter <jonathanh@nvidia.com> wrote: > On 18/03/16 10:52, Geert Uytterhoeven wrote: >> On Fri, Mar 18, 2016 at 11:13 AM, Jon Hunter <jonathanh@nvidia.com> wrote: >>> On 18/03/16 09:13, Geert Uytterhoeven wrote: >>>> On Thu, Mar 17, 2016 at 3:19 PM, Jon Hunter <jonathanh@nvidia.com> wrote: >>>>> Commit afbbd2338176 ("irqchip/gic: Document optional Clock and Power >>>>> Domain properties") documented optional clock and power-dmoain properties >>>>> for the ARM GIC. Currently, there are no users of these and for the >>>>> Tegra210 Audio GIC (based upon the GIC-400) there are two clocks, a >>>>> functional clock and interface clock, that need to be enabled. >>>> >>>> The reason that there are no users for this is twofold: >>>> 1. The GIC driver doesn't have Runtime PM support yet, >>>> 2. There was no clean way to prevent the GIC's clock from being disabled. >>>> Due to this, adding the clocks to the DTSes would mean that they will be >>>> disabled during boot up as unused clocks, leading to a system lock-up. >>>> >>>> I had hoped your series would fix part 1. I gave it a try on r8a7791/koelsch, >>>> but unfortunately it seems the platform driver only supports non-root >>>> controllers, while the r8a7791 GIC is the primary one... >>> >>> Can you try making the following change ... >> >> Thanks! I gave it a try, but no difference. > > I assume you added the appropriate compatible flag? Any more details you Doh... bad assumption... Silly me. > can share about why it is not working? Is it not registered early enough? With + { .compatible = "arm,gic-400", }, the kernel no longer crashes due to accessing the GIC registers while the GIC module clock is disabled. However, the system doesn't boot completely, and time outs on SPI transfers make me believe interrupts are not working. Both with and without "the following change". 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-omap" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On 18/03/16 12:05, Geert Uytterhoeven wrote: > Hi Jon, > > On Fri, Mar 18, 2016 at 11:56 AM, Jon Hunter <jonathanh@nvidia.com> wrote: >> On 18/03/16 10:52, Geert Uytterhoeven wrote: >>> On Fri, Mar 18, 2016 at 11:13 AM, Jon Hunter <jonathanh@nvidia.com> wrote: >>>> On 18/03/16 09:13, Geert Uytterhoeven wrote: >>>>> On Thu, Mar 17, 2016 at 3:19 PM, Jon Hunter <jonathanh@nvidia.com> wrote: >>>>>> Commit afbbd2338176 ("irqchip/gic: Document optional Clock and Power >>>>>> Domain properties") documented optional clock and power-dmoain properties >>>>>> for the ARM GIC. Currently, there are no users of these and for the >>>>>> Tegra210 Audio GIC (based upon the GIC-400) there are two clocks, a >>>>>> functional clock and interface clock, that need to be enabled. >>>>> >>>>> The reason that there are no users for this is twofold: >>>>> 1. The GIC driver doesn't have Runtime PM support yet, >>>>> 2. There was no clean way to prevent the GIC's clock from being disabled. >>>>> Due to this, adding the clocks to the DTSes would mean that they will be >>>>> disabled during boot up as unused clocks, leading to a system lock-up. >>>>> >>>>> I had hoped your series would fix part 1. I gave it a try on r8a7791/koelsch, >>>>> but unfortunately it seems the platform driver only supports non-root >>>>> controllers, while the r8a7791 GIC is the primary one... >>>> >>>> Can you try making the following change ... >>> >>> Thanks! I gave it a try, but no difference. >> >> I assume you added the appropriate compatible flag? Any more details you > > Doh... bad assumption... Silly me. > >> can share about why it is not working? Is it not registered early enough? > > With > > + { .compatible = "arm,gic-400", }, > > the kernel no longer crashes due to accessing the GIC registers while the > GIC module clock is disabled. > > However, the system doesn't boot completely, and time outs on SPI transfers > make me believe interrupts are not working. > Both with and without "the following change". Yes, I recall now why I did not support primary controllers and it is because you need to call set_smp_cross_call() (for SMP) and set_handle_irq(). Both of which are located in the __init section and need to be called early during boot. So to make this work for primary controllers, more work would need to be done. Jon -- To unsubscribe from this list: send the line "unsubscribe linux-omap" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On 03/18/2016 02:05 PM, Geert Uytterhoeven wrote: > Hi Jon, > > On Fri, Mar 18, 2016 at 11:56 AM, Jon Hunter <jonathanh@nvidia.com> wrote: >> On 18/03/16 10:52, Geert Uytterhoeven wrote: >>> On Fri, Mar 18, 2016 at 11:13 AM, Jon Hunter <jonathanh@nvidia.com> wrote: >>>> On 18/03/16 09:13, Geert Uytterhoeven wrote: >>>>> On Thu, Mar 17, 2016 at 3:19 PM, Jon Hunter <jonathanh@nvidia.com> wrote: >>>>>> Commit afbbd2338176 ("irqchip/gic: Document optional Clock and Power >>>>>> Domain properties") documented optional clock and power-dmoain properties >>>>>> for the ARM GIC. Currently, there are no users of these and for the >>>>>> Tegra210 Audio GIC (based upon the GIC-400) there are two clocks, a >>>>>> functional clock and interface clock, that need to be enabled. >>>>> >>>>> The reason that there are no users for this is twofold: >>>>> 1. The GIC driver doesn't have Runtime PM support yet, >>>>> 2. There was no clean way to prevent the GIC's clock from being disabled. >>>>> Due to this, adding the clocks to the DTSes would mean that they will be >>>>> disabled during boot up as unused clocks, leading to a system lock-up. >>>>> >>>>> I had hoped your series would fix part 1. I gave it a try on r8a7791/koelsch, >>>>> but unfortunately it seems the platform driver only supports non-root >>>>> controllers, while the r8a7791 GIC is the primary one... >>>> >>>> Can you try making the following change ... >>> >>> Thanks! I gave it a try, but no difference. >> >> I assume you added the appropriate compatible flag? Any more details you > > Doh... bad assumption... Silly me. > >> can share about why it is not working? Is it not registered early enough? > > With > > + { .compatible = "arm,gic-400", }, > > the kernel no longer crashes due to accessing the GIC registers while the > GIC module clock is disabled. > > However, the system doesn't boot completely, and time outs on SPI transfers > make me believe interrupts are not working. > Both with and without "the following change". > Is my assumption correct that you are trying to enable RPM for primary GIC controller? If yes it may help to take a look on clocksource drivers which use early_platform_device/driver sh_cmt.c sh_mtu2.c sh_tmu.c The primary interrupt controller is initialized very early init_IRQ->irqchip_init->of_irq_init() (IRQCHIP_DECLARE) and, at least as i can see from st_xxx code, the same case is valid for clocksource devices and it was solved using early_platform_device/drive staff.
Hi Grygorii, On Fri, Mar 18, 2016 at 1:47 PM, Grygorii Strashko <grygorii.strashko@ti.com> wrote: > On 03/18/2016 02:05 PM, Geert Uytterhoeven wrote: >> On Fri, Mar 18, 2016 at 11:56 AM, Jon Hunter <jonathanh@nvidia.com> wrote: >>> On 18/03/16 10:52, Geert Uytterhoeven wrote: >>>> On Fri, Mar 18, 2016 at 11:13 AM, Jon Hunter <jonathanh@nvidia.com> wrote: >>>>> On 18/03/16 09:13, Geert Uytterhoeven wrote: >>>>>> On Thu, Mar 17, 2016 at 3:19 PM, Jon Hunter <jonathanh@nvidia.com> wrote: >>>>>>> Commit afbbd2338176 ("irqchip/gic: Document optional Clock and Power >>>>>>> Domain properties") documented optional clock and power-dmoain properties >>>>>>> for the ARM GIC. Currently, there are no users of these and for the >>>>>>> Tegra210 Audio GIC (based upon the GIC-400) there are two clocks, a >>>>>>> functional clock and interface clock, that need to be enabled. >>>>>> >>>>>> The reason that there are no users for this is twofold: >>>>>> 1. The GIC driver doesn't have Runtime PM support yet, >>>>>> 2. There was no clean way to prevent the GIC's clock from being disabled. >>>>>> Due to this, adding the clocks to the DTSes would mean that they will be >>>>>> disabled during boot up as unused clocks, leading to a system lock-up. >>>>>> >>>>>> I had hoped your series would fix part 1. I gave it a try on r8a7791/koelsch, >>>>>> but unfortunately it seems the platform driver only supports non-root >>>>>> controllers, while the r8a7791 GIC is the primary one... >>>>> >>>>> Can you try making the following change ... >>>> >>>> Thanks! I gave it a try, but no difference. >>> >>> I assume you added the appropriate compatible flag? Any more details you >> >> Doh... bad assumption... Silly me. >> >>> can share about why it is not working? Is it not registered early enough? >> >> With >> >> + { .compatible = "arm,gic-400", }, >> >> the kernel no longer crashes due to accessing the GIC registers while the >> GIC module clock is disabled. >> >> However, the system doesn't boot completely, and time outs on SPI transfers >> make me believe interrupts are not working. >> Both with and without "the following change". >> > > Is my assumption correct that you are trying to enable RPM for primary GIC controller? That's correct. > If yes it may help to take a look on clocksource drivers which use early_platform_device/driver > sh_cmt.c sh_mtu2.c sh_tmu.c > > The primary interrupt controller is initialized very early init_IRQ->irqchip_init->of_irq_init() > (IRQCHIP_DECLARE) and, at least as i can see from st_xxx code, the same case is valid for > clocksource devices and it was solved using early_platform_device/drive staff. The GIC now depends on the clock driver, which may be a real platform driver, not initialized from CLK_OF_DECLARE(). Or do you mean to make the clock driver an early platform driver? 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-omap" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On 03/18/2016 03:02 PM, Geert Uytterhoeven wrote: > Hi Grygorii, > > On Fri, Mar 18, 2016 at 1:47 PM, Grygorii Strashko > <grygorii.strashko@ti.com> wrote: >> On 03/18/2016 02:05 PM, Geert Uytterhoeven wrote: >>> On Fri, Mar 18, 2016 at 11:56 AM, Jon Hunter <jonathanh@nvidia.com> wrote: >>>> On 18/03/16 10:52, Geert Uytterhoeven wrote: >>>>> On Fri, Mar 18, 2016 at 11:13 AM, Jon Hunter <jonathanh@nvidia.com> wrote: >>>>>> On 18/03/16 09:13, Geert Uytterhoeven wrote: >>>>>>> On Thu, Mar 17, 2016 at 3:19 PM, Jon Hunter <jonathanh@nvidia.com> wrote: >>>>>>>> Commit afbbd2338176 ("irqchip/gic: Document optional Clock and Power >>>>>>>> Domain properties") documented optional clock and power-dmoain properties >>>>>>>> for the ARM GIC. Currently, there are no users of these and for the >>>>>>>> Tegra210 Audio GIC (based upon the GIC-400) there are two clocks, a >>>>>>>> functional clock and interface clock, that need to be enabled. >>>>>>> >>>>>>> The reason that there are no users for this is twofold: >>>>>>> 1. The GIC driver doesn't have Runtime PM support yet, >>>>>>> 2. There was no clean way to prevent the GIC's clock from being disabled. >>>>>>> Due to this, adding the clocks to the DTSes would mean that they will be >>>>>>> disabled during boot up as unused clocks, leading to a system lock-up. >>>>>>> >>>>>>> I had hoped your series would fix part 1. I gave it a try on r8a7791/koelsch, >>>>>>> but unfortunately it seems the platform driver only supports non-root >>>>>>> controllers, while the r8a7791 GIC is the primary one... >>>>>> >>>>>> Can you try making the following change ... >>>>> >>>>> Thanks! I gave it a try, but no difference. >>>> >>>> I assume you added the appropriate compatible flag? Any more details you >>> >>> Doh... bad assumption... Silly me. >>> >>>> can share about why it is not working? Is it not registered early enough? >>> >>> With >>> >>> + { .compatible = "arm,gic-400", }, >>> >>> the kernel no longer crashes due to accessing the GIC registers while the >>> GIC module clock is disabled. >>> >>> However, the system doesn't boot completely, and time outs on SPI transfers >>> make me believe interrupts are not working. >>> Both with and without "the following change". >>> >> >> Is my assumption correct that you are trying to enable RPM for primary GIC controller? > > That's correct. > >> If yes it may help to take a look on clocksource drivers which use early_platform_device/driver >> sh_cmt.c sh_mtu2.c sh_tmu.c >> >> The primary interrupt controller is initialized very early init_IRQ->irqchip_init->of_irq_init() >> (IRQCHIP_DECLARE) and, at least as i can see from st_xxx code, the same case is valid for >> clocksource devices and it was solved using early_platform_device/drive staff. > > The GIC now depends on the clock driver, which may be a real platform driver, > not initialized from CLK_OF_DECLARE(). Clock need to be accessible, but, seems, there is another issue - if you will try to use gic_driver by just adding compatible string then, most probably, gic_init_bases() will be called twice: 1: init_IRQ->irqchip_init->of_irq_init()->__gic_init_bases()->gic_init_bases() 2: gic_probe->gic_init_bases() And GIC data will be replaced on the fly ;P > > Or do you mean to make the clock driver an early platform driver? I can't say definitely - I've just studied it some time ago, but did not try it by myself.
diff --git a/drivers/irqchip/irq-gic.c b/drivers/irqchip/irq-gic.c index 9e7cf7abf757..2e971e600036 100644 --- a/drivers/irqchip/irq-gic.c +++ b/drivers/irqchip/irq-gic.c @@ -1372,7 +1372,7 @@ static int gic_probe(struct platform_device *pdev) void __iomem *dist_base; void __iomem *cpu_base; u32 percpu_offset; - int ret, irq; + int ret, irq = 0; if (dev->of_node == NULL) return -EINVAL; @@ -1393,11 +1393,8 @@ static int gic_probe(struct platform_device *pdev) if (ret < 0) goto rpm_disable; - irq = irq_of_parse_and_map(dev->of_node, 0); - if (!irq) { - ret = -EINVAL; - goto rpm_put; - } + if (of_irq_count(dev->of_node) > 0) + irq = irq_of_parse_and_map(dev->of_node, 0); ret = gic_of_setup(dev->of_node, &dist_base, &cpu_base, &percpu_offset); if (ret) @@ -1411,7 +1408,8 @@ static int gic_probe(struct platform_device *pdev) gic->chip.parent = dev; - irq_set_chained_handler_and_data(irq, gic_handle_cascade_irq, gic); + if (irq) + irq_set_chained_handler_and_data(irq, gic_handle_cascade_irq, gic); pm_runtime_put(dev); @@ -1424,7 +1422,6 @@ gic_unmap: iounmap(cpu_base); irq_dispose: irq_dispose_mapping(irq); -rpm_put: pm_runtime_put_sync(dev); rpm_disable: pm_runtime_disable(dev);