Hi Andrew,
With this 2 patches I added clock gating support for Armada 370 and
Armada XP. I compiled and tested on the boards, and managed to see the
new clock using debugfs.
Feel free to squash them in your series if you want.
Regards,
Gregory CLEMENT (2):
clk: mvebu: armada 370/XP add clock gating control provider for DT
arm: mvebu: armada 370/XP adding clock gating support: dt binding
.../bindings/clock/mvebu-gated-clock.txt | 43 ++++++++++++++
arch/arm/boot/dts/armada-370.dtsi | 8 +++
arch/arm/boot/dts/armada-xp.dtsi | 7 +++
arch/arm/mach-mvebu/Kconfig | 1 +
drivers/clk/mvebu/clk-gating-ctrl.c | 61 ++++++++++++++++++++
5 files changed, 120 insertions(+)
--
1.7.9.5
Signed-off-by: Gregory CLEMENT <[email protected]>
---
.../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:
+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",
--
1.7.9.5
Signed-off-by: Gregory CLEMENT <[email protected]>
---
arch/arm/boot/dts/armada-370.dtsi | 8 ++++++++
arch/arm/boot/dts/armada-xp.dtsi | 7 +++++++
2 files changed, 15 insertions(+)
diff --git a/arch/arm/boot/dts/armada-370.dtsi b/arch/arm/boot/dts/armada-370.dtsi
index dae966c..25524eb 100644
--- a/arch/arm/boot/dts/armada-370.dtsi
+++ b/arch/arm/boot/dts/armada-370.dtsi
@@ -82,5 +82,13 @@
#clock-cells = <1>;
};
+ clk_gate: clock-gating-control@d0018220 {
+ compatible = "marvell,armada-370-clock-gating";
+ reg = <0xd0018220 0x4>;
+ clocks = <&coreclk 0>;
+ #clock-cells = <1>;
+ };
+
+
};
};
diff --git a/arch/arm/boot/dts/armada-xp.dtsi b/arch/arm/boot/dts/armada-xp.dtsi
index b3c9fbc..82ea5e4 100644
--- a/arch/arm/boot/dts/armada-xp.dtsi
+++ b/arch/arm/boot/dts/armada-xp.dtsi
@@ -99,6 +99,13 @@
clocks = <&coreclk 1>;
};
+ clk_gate: clock-gating-control@d001821c {
+ compatible = "marvell,armada-xp-clock-gating";
+ reg = <0xd001821c 0x4>;
+ clocks = <&coreclk 0>;
+ #clock-cells = <1>;
+ };
+
system-controller@d0018200 {
compatible = "marvell,armada-370-xp-system-controller";
reg = <0xd0018200 0x500>;
--
1.7.9.5
Hi Gregory
Nice work
On Fri, Nov 16, 2012 at 07:01:59PM +0100, Gregory CLEMENT wrote:
> Signed-off-by: Gregory CLEMENT <[email protected]>
> ---
> .../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 <[email protected]>
>> ---
>> .../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
> [email protected]
> 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
--
Thomas Petazzoni, Free Electrons
Kernel, drivers, real-time and embedded Linux
development, consulting, training and support.
http://free-electrons.com
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
--
Thomas Petazzoni, Free Electrons
Kernel, drivers, real-time and embedded Linux
development, consulting, training and support.
http://free-electrons.com