| Message ID | 1353088920-17458-2-git-send-email-gregory.clement@free-electrons.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
Hi Gregory Nice work On Fri, Nov 16, 2012 at 07:01:59PM +0100, Gregory CLEMENT wrote: > Signed-off-by: Gregory CLEMENT <gregory.clement@free-electrons.com> > --- > .../bindings/clock/mvebu-gated-clock.txt | 43 ++++++++++++++ > arch/arm/mach-mvebu/Kconfig | 1 + > drivers/clk/mvebu/clk-gating-ctrl.c | 61 ++++++++++++++++++++ > 3 files changed, 105 insertions(+) > > diff --git a/Documentation/devicetree/bindings/clock/mvebu-gated-clock.txt b/Documentation/devicetree/bindings/clock/mvebu-gated-clock.txt > index 7497cc0..9dbcdd9 100644 > --- a/Documentation/devicetree/bindings/clock/mvebu-gated-clock.txt > +++ b/Documentation/devicetree/bindings/clock/mvebu-gated-clock.txt > @@ -6,6 +6,49 @@ the clock ID in its "clocks" phandle cell. The clock ID is directly mapped to > the corresponding clock gating control bit in HW to ease manual clock lookup > in datasheet. > > +The following is a list of provided IDs for Armada XP: Should that the 370, not XP? > +ID Clock Peripheral > +----------------------------------- > +0 Audio AC97 Cntrl > +1 pex0_en PCIe 0 Clock out > +2 pex1_en PCIe 1 Clock out > +3 ge1 Gigabit Ethernet 1 > +4 ge0 Gigabit Ethernet 0 > +5 pex0 PCIe Cntrl 0 > +9 pex1 PCIe Cntrl 1 > +15 sata0 SATA Host 0 > +17 sdio SDHCI Host > +25 tdm Time Division Mplx > +28 ddr DDR Cntrl > +30 sata1 SATA Host 0 Not many clocks there. USB? XOR? Crypto? What is the ddr clock for? Does bad things happen if you turn it off? Kirkwood has a similar clock, dunit, which i decided not to export, since when you turn it off, the whole SoC locks up. Thanks Andrew
Hi Andrew On 11/17/2012 09:26 AM, Andrew Lunn wrote: > Hi Gregory > > Nice work Thanks! > > On Fri, Nov 16, 2012 at 07:01:59PM +0100, Gregory CLEMENT wrote: >> Signed-off-by: Gregory CLEMENT <gregory.clement@free-electrons.com> >> --- >> .../bindings/clock/mvebu-gated-clock.txt | 43 ++++++++++++++ >> arch/arm/mach-mvebu/Kconfig | 1 + >> drivers/clk/mvebu/clk-gating-ctrl.c | 61 ++++++++++++++++++++ >> 3 files changed, 105 insertions(+) >> >> diff --git a/Documentation/devicetree/bindings/clock/mvebu-gated-clock.txt b/Documentation/devicetree/bindings/clock/mvebu-gated-clock.txt >> index 7497cc0..9dbcdd9 100644 >> --- a/Documentation/devicetree/bindings/clock/mvebu-gated-clock.txt >> +++ b/Documentation/devicetree/bindings/clock/mvebu-gated-clock.txt >> @@ -6,6 +6,49 @@ the clock ID in its "clocks" phandle cell. The clock ID is directly mapped to >> the corresponding clock gating control bit in HW to ease manual clock lookup >> in datasheet. >> >> +The following is a list of provided IDs for Armada XP: > > Should that the 370, not XP? Yes indeed, it was a wrong copy and paste. > >> +ID Clock Peripheral >> +----------------------------------- >> +0 Audio AC97 Cntrl >> +1 pex0_en PCIe 0 Clock out >> +2 pex1_en PCIe 1 Clock out >> +3 ge1 Gigabit Ethernet 1 >> +4 ge0 Gigabit Ethernet 0 >> +5 pex0 PCIe Cntrl 0 >> +9 pex1 PCIe Cntrl 1 >> +15 sata0 SATA Host 0 >> +17 sdio SDHCI Host >> +25 tdm Time Division Mplx >> +28 ddr DDR Cntrl >> +30 sata1 SATA Host 0 > > Not many clocks there. USB? XOR? Crypto? Yes I was surprised too, but it was I found on the datasheet. For USB, for example you can turn the USB in low power mode but at the PHY level. > > What is the ddr clock for? Does bad things happen if you turn it off? > Kirkwood has a similar clock, dunit, which i decided not to export, > since when you turn it off, the whole SoC locks up. Well of course if you code run in DDR then it could be a problem. But I think it could be useful to turn it off when going to suspend, it the DDR can do self-refresh. In this case it should be possible to run the code from SRAM or L2 Cache. Gregory
> > What is the ddr clock for? Does bad things happen if you turn it off? > > Kirkwood has a similar clock, dunit, which i decided not to export, > > since when you turn it off, the whole SoC locks up. > > Well of course if you code run in DDR then it could be a problem. But > I think it could be useful to turn it off when going to suspend, it > the DDR can do self-refresh. In this case it should be possible to run > the code from SRAM or L2 Cache. O.K. Just watch out for the lateinit call in the clock framework. Andrew
Quoting Andrew Lunn (2012-11-17 05:54:35) > > > What is the ddr clock for? Does bad things happen if you turn it off? > > > Kirkwood has a similar clock, dunit, which i decided not to export, > > > since when you turn it off, the whole SoC locks up. > > > > Well of course if you code run in DDR then it could be a problem. But > > I think it could be useful to turn it off when going to suspend, it > > the DDR can do self-refresh. In this case it should be possible to run > > the code from SRAM or L2 Cache. > > O.K. Just watch out for the lateinit call in the clock framework. > CLK_IGNORE_UNUSED is the flag you want for this situation. Regards, Mike > Andrew > > _______________________________________________ > linux-arm-kernel mailing list > linux-arm-kernel@lists.infradead.org > http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
Dear Andrew Lunn, On Sat, 17 Nov 2012 14:54:35 +0100, Andrew Lunn wrote: > > > What is the ddr clock for? Does bad things happen if you turn it off? > > > Kirkwood has a similar clock, dunit, which i decided not to export, > > > since when you turn it off, the whole SoC locks up. > > > > Well of course if you code run in DDR then it could be a problem. But > > I think it could be useful to turn it off when going to suspend, it > > the DDR can do self-refresh. In this case it should be possible to run > > the code from SRAM or L2 Cache. > > O.K. Just watch out for the lateinit call in the clock framework. I don't think there is a problem with the dramclk and the lateinit call of the clock framework. The dramclk is a fixed factor clock, and the fixed factor clock driver does not implement the ->disable() operation. And therefore, the clk_disable_unused() code executed as the lateinit call will not be able to disable it: if (__clk_is_enabled(clk) && clk->ops->disable) clk->ops->disable(clk->hw); So I think we're quite safe with fixed rate clocks and fixed factor clocks in that no-one can disable them :-) Best regards, Thomas
On Mon, Nov 19, 2012 at 04:46:11PM +0100, Thomas Petazzoni wrote: > Dear Andrew Lunn, > > On Sat, 17 Nov 2012 14:54:35 +0100, Andrew Lunn wrote: > > > > What is the ddr clock for? Does bad things happen if you turn it off? > > > > Kirkwood has a similar clock, dunit, which i decided not to export, > > > > since when you turn it off, the whole SoC locks up. > > > > > > Well of course if you code run in DDR then it could be a problem. But > > > I think it could be useful to turn it off when going to suspend, it > > > the DDR can do self-refresh. In this case it should be possible to run > > > the code from SRAM or L2 Cache. > > > > O.K. Just watch out for the lateinit call in the clock framework. > > I don't think there is a problem with the dramclk and the lateinit call > of the clock framework. The dramclk is a fixed factor clock, and the > fixed factor clock driver does not implement the ->disable() operation. > And therefore, the clk_disable_unused() code executed as the lateinit > call will not be able to disable it: > > if (__clk_is_enabled(clk) && clk->ops->disable) > clk->ops->disable(clk->hw); > > So I think we're quite safe with fixed rate clocks and fixed factor > clocks in that no-one can disable them :-) Hi Thomas I don't think we are taking about the same clock. I mean the gate clock: 28 ddr DDR Cntrl not the core clock. Andrew
On 11/19/2012 04:46 PM, Thomas Petazzoni wrote: > Dear Andrew Lunn, > > On Sat, 17 Nov 2012 14:54:35 +0100, Andrew Lunn wrote: >>>> What is the ddr clock for? Does bad things happen if you turn it off? >>>> Kirkwood has a similar clock, dunit, which i decided not to export, >>>> since when you turn it off, the whole SoC locks up. >>> >>> Well of course if you code run in DDR then it could be a problem. But >>> I think it could be useful to turn it off when going to suspend, it >>> the DDR can do self-refresh. In this case it should be possible to run >>> the code from SRAM or L2 Cache. >> >> O.K. Just watch out for the lateinit call in the clock framework. > > I don't think there is a problem with the dramclk and the lateinit call > of the clock framework. The dramclk is a fixed factor clock, and the > fixed factor clock driver does not implement the ->disable() operation. > And therefore, the clk_disable_unused() code executed as the lateinit > call will not be able to disable it: > > if (__clk_is_enabled(clk)&& clk->ops->disable) > clk->ops->disable(clk->hw); > > So I think we're quite safe with fixed rate clocks and fixed factor > clocks in that no-one can disable them :-) Thomas, I guess Andrew was referring to the clock gating control bit for ddr on Armada 370 not the fixed factor clock. If there is a clk_gate installed, it will be disabled if not taken by any driver or init code. You disable either the ddr controller clock or the external ddr clock or both, but all will lead to a system lock up. If unsure, you should remove bit 28 from clk-gating-ctrl.c and it's devicetree documentation for Armada 370. Well get all the gates straight as soon as we have more support for e.g. PMU, GEPHY, aso. Sebastian
Dear Andrew Lunn, On Mon, 19 Nov 2012 16:58:56 +0100, Andrew Lunn wrote: > I don't think we are taking about the same clock. I mean the gate > clock: > > 28 ddr DDR Cntrl > > not the core clock. Right, after discussion with Gregory, I found out my misunderstanding. I'm looking now as to how this clock affects the system behavior. I indeed see this clock ->disable() hook being called, but it doesn't seem to cause any sort of problem on the system. It still works nicely. So not sure this clock is actually doing something, or at least maybe not what we think it does. Best regards, Thomas
diff --git a/Documentation/devicetree/bindings/clock/mvebu-gated-clock.txt b/Documentation/devicetree/bindings/clock/mvebu-gated-clock.txt index 7497cc0..9dbcdd9 100644 --- a/Documentation/devicetree/bindings/clock/mvebu-gated-clock.txt +++ b/Documentation/devicetree/bindings/clock/mvebu-gated-clock.txt @@ -6,6 +6,49 @@ the clock ID in its "clocks" phandle cell. The clock ID is directly mapped to the corresponding clock gating control bit in HW to ease manual clock lookup in datasheet. +The following is a list of provided IDs for Armada XP: +ID Clock Peripheral +----------------------------------- +0 Audio AC97 Cntrl +1 pex0_en PCIe 0 Clock out +2 pex1_en PCIe 1 Clock out +3 ge1 Gigabit Ethernet 1 +4 ge0 Gigabit Ethernet 0 +5 pex0 PCIe Cntrl 0 +9 pex1 PCIe Cntrl 1 +15 sata0 SATA Host 0 +17 sdio SDHCI Host +25 tdm Time Division Mplx +28 ddr DDR Cntrl +30 sata1 SATA Host 0 + +The following is a list of provided IDs for Armada XP: +ID Clock Peripheral +----------------------------------- +0 audio Audio Cntrl +1 ge3 Gigabit Ethernet 3 +2 ge2 Gigabit Ethernet 2 +3 ge1 Gigabit Ethernet 1 +4 ge0 Gigabit Ethernet 0 +5 pex0 PCIe Cntrl 0 +6 pex1 PCIe Cntrl 1 +7 pex2 PCIe Cntrl 2 +8 pex3 PCIe Cntrl 3 +13 bp +14 sata0lnk +15 sata0 SATA Host 0 +16 lcd LCD Cntrl +17 sdio SDHCI Host +18 usb0 USB Host 0 +19 usb1 USB Host 1 +20 usb2 USB Host 2 +22 xor0 XOR DMA 0 +23 crypto CESA engine +25 tdm Time Division Mplx +28 xor1 XOR DMA 1 +29 sata1lnk +30 sata1 SATA Host 0 + The following is a list of provided IDs for Dove: ID Clock Peripheral ----------------------------------- diff --git a/arch/arm/mach-mvebu/Kconfig b/arch/arm/mach-mvebu/Kconfig index ad38b64..dc009d5 100644 --- a/arch/arm/mach-mvebu/Kconfig +++ b/arch/arm/mach-mvebu/Kconfig @@ -12,6 +12,7 @@ config ARCH_MVEBU select CLKDEV_LOOKUP select MVEBU_CLK_CORE select MVEBU_CLK_CPU + select MVEBU_CLK_GATING if ARCH_MVEBU diff --git a/drivers/clk/mvebu/clk-gating-ctrl.c b/drivers/clk/mvebu/clk-gating-ctrl.c index 3ac7607..7f04e8b 100644 --- a/drivers/clk/mvebu/clk-gating-ctrl.c +++ b/drivers/clk/mvebu/clk-gating-ctrl.c @@ -101,6 +101,53 @@ static void __init mvebu_clk_gating_setup( * SoC specific clock gating control */ +#ifdef CONFIG_MACH_ARMADA_370 +static const struct mvebu_soc_descr __initconst armada_370_gating_descr[] = { + { "audio", NULL, 0 }, + { "pex0_en", NULL, 1 }, + { "pex1_en", NULL, 2 }, + { "ge1", NULL, 3 }, + { "ge0", NULL, 4 }, + { "pex0", NULL, 5 }, + { "pex1", NULL, 9 }, + { "sata0", NULL, 15 }, + { "sdio", NULL, 17 }, + { "tdm", NULL, 25 }, + { "ddr", NULL, 28 }, + { "sata1", NULL, 30 }, + { } +}; +#endif + +#ifdef CONFIG_MACH_ARMADA_XP +static const struct mvebu_soc_descr __initconst armada_xp_gating_descr[] = { + { "audio", NULL, 0 }, + { "ge3", NULL, 1 }, + { "ge2", NULL, 2 }, + { "ge1", NULL, 3 }, + { "ge0", NULL, 4 }, + { "pex0", NULL, 5 }, + { "pex1", NULL, 6 }, + { "pex2", NULL, 7 }, + { "pex3", NULL, 8 }, + { "bp", NULL, 13 }, + { "sata0lnk", NULL, 14 }, + { "sata0", NULL, 15 }, + { "lcd", NULL, 16 }, + { "sdio", NULL, 17 }, + { "usb0", NULL, 18 }, + { "usb1", NULL, 19 }, + { "usb2", NULL, 20 }, + { "xor0", NULL, 22 }, + { "crypto", NULL, 23 }, + { "tdm", NULL, 25 }, + { "xor1", NULL, 28 }, + { "sata1lnk", NULL, 29 }, + { "sata1", NULL, 30 }, + { } +}; +#endif + #ifdef CONFIG_ARCH_DOVE static const struct mvebu_soc_descr __initconst dove_gating_descr[] = { { "usb0", NULL, 0 }, @@ -147,6 +194,20 @@ static const struct mvebu_soc_descr __initconst kirkwood_gating_descr[] = { #endif static const __initdata struct of_device_id clk_gating_match[] = { +#ifdef CONFIG_MACH_ARMADA_370 + { + .compatible = "marvell,armada-370-clock-gating", + .data = armada_370_gating_descr, + }, +#endif + +#ifdef CONFIG_MACH_ARMADA_XP + { + .compatible = "marvell,armada-xp-clock-gating", + .data = armada_xp_gating_descr, + }, +#endif + #ifdef CONFIG_ARCH_DOVE { .compatible = "marvell,dove-clock-gating",
Signed-off-by: Gregory CLEMENT <gregory.clement@free-electrons.com> --- .../bindings/clock/mvebu-gated-clock.txt | 43 ++++++++++++++ arch/arm/mach-mvebu/Kconfig | 1 + drivers/clk/mvebu/clk-gating-ctrl.c | 61 ++++++++++++++++++++ 3 files changed, 105 insertions(+)