| Message ID | 1458224359-32665-15-git-send-email-jonathanh@nvidia.com |
|---|---|
| State | Superseded, archived |
| Delegated to: | Jon Hunter |
| Headers | show |
On Thu, Mar 17, 2016 at 9:19 AM, 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. > > To allow flexibility, drop the 'clock-names' from the GIC binding and > just provide a list of clocks which the driver can parse. It is assumed > that any clocks that are listed, need to be enabled in order to access > the GIC. > > Signed-off-by: Jon Hunter <jonathanh@nvidia.com> > > --- > > Please note that I am not sure if this will be popular, but I am trying > to come up with a generic way to handle multiple clocks that may be > required for accessing a GIC. It's not. :) We need to specify the number and order of clocks by compatible string at a minimum. Sadly, ARM's GICs are well documented and include clock names, so you can't just make up genericish names either which is probably often the case. Rob -- To unsubscribe from this list: send the line "unsubscribe linux-tegra" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On 17/03/16 20:14, Rob Herring wrote: > On Thu, Mar 17, 2016 at 9:19 AM, 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. >> >> To allow flexibility, drop the 'clock-names' from the GIC binding and >> just provide a list of clocks which the driver can parse. It is assumed >> that any clocks that are listed, need to be enabled in order to access >> the GIC. >> >> Signed-off-by: Jon Hunter <jonathanh@nvidia.com> >> >> --- >> >> Please note that I am not sure if this will be popular, but I am trying >> to come up with a generic way to handle multiple clocks that may be >> required for accessing a GIC. > > It's not. :) > > We need to specify the number and order of clocks by compatible string > at a minimum. Sadly, ARM's GICs are well documented and include clock > names, so you can't just make up genericish names either which is > probably often the case. Do you have any suggestions then? I have had a look at the ARM TRMs and although I see that they do show the functional clock, there is no mention of whether there are any other clocks need in order to interface to the GIC (ie. bus clock). I know that for other SoCs such as OMAP it is common to have both a functional clock and interface clock. So I believe this is fairly common. Cheers Jon -- To unsubscribe from this list: send the line "unsubscribe linux-tegra" 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 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... Alternatively, part 2 can to be fixed by "clk: introduce CLK_ENABLE_HAND_OFF flag", combined with the clock driver setting the flag when needed. Unfortunately that patch is not yet upstream, and not even in -next. Note that drivers/clk/renesas/renesas-cpg-mssr.c already handles CLK_ENABLE_HAND_OFF if present, and else just ignores the clock. So I could already add the clock to r8a7795.dtsi, which uses that driver. For older SoCs, the module clocks are described in the dtsi, and I would need a crude hack to enable CLK_ENABLE_HAND_OFF in the clock driver. > To allow flexibility, drop the 'clock-names' from the GIC binding and > just provide a list of clocks which the driver can parse. It is assumed > that any clocks that are listed, need to be enabled in order to access > the GIC. Originally I just wanted to have "clocks", and let the details be handled by SoC-specific code. However, Mark Rutland insisted on using the clock naming from the GIC TRMs, as the number of clocks and their names depend on the GIC variant. Apparently they also depend on the SoC... 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-tegra" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Rob, Mark, Marc, On 18/03/16 08:37, Jon Hunter wrote: > > On 17/03/16 20:14, Rob Herring wrote: >> On Thu, Mar 17, 2016 at 9:19 AM, 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. >>> >>> To allow flexibility, drop the 'clock-names' from the GIC binding and >>> just provide a list of clocks which the driver can parse. It is assumed >>> that any clocks that are listed, need to be enabled in order to access >>> the GIC. >>> >>> Signed-off-by: Jon Hunter <jonathanh@nvidia.com> >>> >>> --- >>> >>> Please note that I am not sure if this will be popular, but I am trying >>> to come up with a generic way to handle multiple clocks that may be >>> required for accessing a GIC. >> >> It's not. :) >> >> We need to specify the number and order of clocks by compatible string >> at a minimum. Sadly, ARM's GICs are well documented and include clock >> names, so you can't just make up genericish names either which is >> probably often the case. > > Do you have any suggestions then? > > I have had a look at the ARM TRMs and although I see that they do show > the functional clock, there is no mention of whether there are any other > clocks need in order to interface to the GIC (ie. bus clock). I know > that for other SoCs such as OMAP it is common to have both a functional > clock and interface clock. So I believe this is fairly common. Any thoughts on how I should handle this for the Tegra AGIC? Cheers Jon -- To unsubscribe from this list: send the line "unsubscribe linux-tegra" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
diff --git a/Documentation/devicetree/bindings/interrupt-controller/arm,gic.txt b/Documentation/devicetree/bindings/interrupt-controller/arm,gic.txt index 793c20ff8fcc..c471d1a7a8ea 100644 --- a/Documentation/devicetree/bindings/interrupt-controller/arm,gic.txt +++ b/Documentation/devicetree/bindings/interrupt-controller/arm,gic.txt @@ -61,15 +61,8 @@ Optional regions, used when the GIC doesn't have banked registers. The offset is cpu-offset * cpu-nr. -- clocks : List of phandle and clock-specific pairs, one for each entry - in clock-names. -- clock-names : List of names for the GIC clock input(s). Valid clock names - depend on the GIC variant: - "ic_clk" (for "arm,arm11mp-gic") - "PERIPHCLKEN" (for "arm,cortex-a15-gic") - "PERIPHCLK", "PERIPHCLKEN" (for "arm,cortex-a9-gic") - "clk" (for "arm,gic-400") - "gclk" (for "arm,pl390") +- clocks : List of phandle and clock-specific pairs required for + accessing the GIC. - power-domains : A phandle and PM domain specifier as defined by bindings of the power controller specified by phandle, used when the GIC
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. To allow flexibility, drop the 'clock-names' from the GIC binding and just provide a list of clocks which the driver can parse. It is assumed that any clocks that are listed, need to be enabled in order to access the GIC. Signed-off-by: Jon Hunter <jonathanh@nvidia.com> --- Please note that I am not sure if this will be popular, but I am trying to come up with a generic way to handle multiple clocks that may be required for accessing a GIC. .../devicetree/bindings/interrupt-controller/arm,gic.txt | 11 ++--------- 1 file changed, 2 insertions(+), 9 deletions(-)