[PATCH 0/2] Add clock gating support for Armada 370/XP

[PATCH 0/2] Add clock gating support for Armada 370/XP

[Gregory CLEMENT]

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

[PATCH 1/2] clk: mvebu: armada 370/XP add clock gating control provider for DT

[Gregory CLEMENT]

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:
+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

Re: [PATCH 1/2] clk: mvebu: armada 370/XP add clock gating control provider for DT

[Andrew Lunn]

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

Re: [PATCH 1/2] clk: mvebu: armada 370/XP add clock gating control provider for DT

[Gregory CLEMENT]

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

Re: [PATCH 1/2] clk: mvebu: armada 370/XP add clock gating control provider for DT

[Andrew Lunn]

> > 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

Re: [PATCH 1/2] clk: mvebu: armada 370/XP add clock gating control provider for DT

[Mike Turquette]

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

Re: [PATCH 1/2] clk: mvebu: armada 370/XP add clock gating control provider for DT

[Thomas Petazzoni]

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

Re: [PATCH 1/2] clk: mvebu: armada 370/XP add clock gating control provider for DT

[Andrew Lunn]

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

Re: [PATCH 1/2] clk: mvebu: armada 370/XP add clock gating control provider for DT

[Thomas Petazzoni]

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

Re: [PATCH 1/2] clk: mvebu: armada 370/XP add clock gating control provider for DT

[Sebastian Hesselbarth]

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

[PATCH 2/2] arm: mvebu: armada 370/XP adding clock gating support: dt binding

[Gregory CLEMENT]

Signed-off-by: Gregory CLEMENT <gregory.clement@free-electrons.com>
---
 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