arm: mvebu: Various mvebu clock fixes/improvements

PATCH New Thomas Petazzoni 2 participants 19 messages Nov 19, 2012 – Nov 20, 2012

Subsystems (6)

Device tree bindings Arm (32-bit) Arm sub-architectures Common clk +2 more subsystems

lore

Share & tools

lore

Thomas Petazzoni

author

2012-11-19 16:52 UTC #0 lore raw

Andrew,

You'll find enclosed seven patches brigging fixes and improvements to the CLK support on mvebu platforms. You can either squash them into your current clk series if you intend to send a new version of it to Jason, or forward the patches as follow-up patches to Jason after reviewing.

In summary:

* Patch 1 fixes a real problem: the gating clocks are not registered on Armada 370/XP.

* Patch 2, 3, 5 and 6 are more cosmetic patches, but I think they are good to have.

* Patch 4 implements a fix that we've been discussing: moving the Armada XP CPU clocks down to the individual armada-xp-mv78xxx.dtsi files.

* Patch 7 fixes the references to the SATA clocks on Armada 370/XP. Without this patch, Armada 370 currently hangs at boot when trying to initialize SATA.

Thanks,

Thomas

Thomas Petazzoni

author

2012-11-19 16:52 UTC #1 lore raw

PATCH 1/7 clk: mvebu: init all mvebu clock types from mvebu_clocks_init()

The mvebu_clocks_init() function should register all the mvebu clock types, so that all mvebu platforms only have to call mvebu_clocks_init().

This fixes mach-mvebu because mvebu_clk_gating_init() was never called and therefore none of the gating clocks were registered.

mach-kirkwood and mach-dove are updated to not call mvebu_clk_gating_init() a second time. Thanks to this, it is no longer necessary to expose mvebu_clk_gating_init() beyond the clock framework, so we get rid of its prototype in <linux/clk/mvebu.h> and we add it in a private drivers/clk/mvebu/clk-gating-ctrl.h header file.

Signed-off-by Thomas Petazzoni <[email protected]>

---
 arch/arm/mach-dove/common.c         |    1 -
 arch/arm/mach-kirkwood/board-dt.c   |    1 -
 drivers/clk/mvebu/clk-gating-ctrl.h |   18 ++++++++++++++++++
 drivers/clk/mvebu/clk.c             |    2 ++
 include/linux/clk/mvebu.h           |    1 -
 5 files changed, 20 insertions(+), 3 deletions(-)
 create mode 100644 drivers/clk/mvebu/clk-gating-ctrl.h

arch/arm/mach-dove/common.c +0 -1

index 4b09d12..6a2c4dc 100644
@@ -421,7 +421,6 @@ static void __init dove_legacy_clk_init(void)
 static void __init dove_of_clk_init(void)
 {
 	mvebu_clocks_init();
-	mvebu_clk_gating_init();
 	dove_legacy_clk_init();
 }

arch/arm/mach-kirkwood/board-dt.c +0 -1

index f55393b..40db4a8 100644
@@ -80,7 +80,6 @@ static void __init kirkwood_legacy_clk_init(void)
 static void __init kirkwood_of_clk_init(void)
 {
 	mvebu_clocks_init();
-	mvebu_clk_gating_init();
 	kirkwood_legacy_clk_init();
 }

drivers/clk/mvebu/clk-gating-ctrl.h new file +18 -0

new file mode 100644
index 0000000..20cc4ad
@@ -0,0 +1,18 @@
+/*
+ * Marvell EBU gating clock handling
+ *
+ * Copyright (C) 2012 Marvell
+ *
+ * Thomas Petazzoni <[email protected]>
+ *
+ * This file is licensed under the terms of the GNU General Public
+ * License version 2.  This program is licensed "as is" without any
+ * warranty of any kind, whether express or implied.
+ */
+
+#ifndef __MVEBU_CLK_GATING_H
+#define __MVEBU_CLK_GATING_H
+
+void __init mvebu_clk_gating_init(void);
+
+#endif

drivers/clk/mvebu/clk.c +2 -0

index dc310d6..3f0e43e 100644
@@ -17,6 +17,7 @@
 #include <linux/of.h>
 #include "clk-core.h"
 #include "clk-cpu.h"
+#include "clk-gating-ctrl.h"
 
 static const __initconst struct of_device_id clk_match[] = {
 #ifdef CONFIG_MVEBU_CLK_CPU

@@ -33,5 +34,6 @@ static const __initconst struct of_device_id clk_match[] = {
 void __init mvebu_clocks_init(void)
 {
 	mvebu_core_clk_init();
+	mvebu_clk_gating_init();
 	of_clk_init(clk_match);
 }

include/linux/clk/mvebu.h +0 -1

index 70c366f..8c4ae71 100644
@@ -18,6 +18,5 @@
 #define __CLK_MVEBU_H_
 
 void __init mvebu_clocks_init(void);
-void mvebu_clk_gating_init(void);
 
 #endif

Signature · 1 line

1.7.9.5

Andrew Lunn

maintainer

2012-11-19 18:25 UTC #2 lore raw

PATCH 1/7 clk: mvebu: init all mvebu clock types from mvebu_clocks_init()

Quoted 85 lines · Thomas Petazzoni

The mvebu_clocks_init() function should register all the mvebu clock types, so that all mvebu platforms only have to call mvebu_clocks_init().
This fixes mach-mvebu because mvebu_clk_gating_init() was never called and therefore none of the gating clocks were registered.
mach-kirkwood and mach-dove are updated to not call mvebu_clk_gating_init() a second time. Thanks to this, it is no longer necessary to expose mvebu_clk_gating_init() beyond the clock framework, so we get rid of its prototype in <linux/clk/mvebu.h> and we add it in a private drivers/clk/mvebu/clk-gating-ctrl.h header file.
Signed-off-by Thomas Petazzoni <[email protected]>
---
 arch/arm/mach-dove/common.c         |    1 -
 arch/arm/mach-kirkwood/board-dt.c   |    1 -
 drivers/clk/mvebu/clk-gating-ctrl.h |   18 ++++++++++++++++++
 drivers/clk/mvebu/clk.c             |    2 ++
 include/linux/clk/mvebu.h           |    1 -
 5 files changed, 20 insertions(+), 3 deletions(-)
 create mode 100644 drivers/clk/mvebu/clk-gating-ctrl.h
arch/arm/mach-dove/common.c +0 -1
index 4b09d12..6a2c4dc 100644
@@ -421,7 +421,6 @@ static void __init dove_legacy_clk_init(void)
 static void __init dove_of_clk_init(void)
 {
 	mvebu_clocks_init();
-	mvebu_clk_gating_init();
 	dove_legacy_clk_init();
 }
 
arch/arm/mach-kirkwood/board-dt.c +0 -1
index f55393b..40db4a8 100644
@@ -80,7 +80,6 @@ static void __init kirkwood_legacy_clk_init(void)
 static void __init kirkwood_of_clk_init(void)
 {
 	mvebu_clocks_init();
-	mvebu_clk_gating_init();
 	kirkwood_legacy_clk_init();
 }
 
drivers/clk/mvebu/clk-gating-ctrl.h new file +18 -0
new file mode 100644
index 0000000..20cc4ad
@@ -0,0 +1,18 @@
+/*
+ * Marvell EBU gating clock handling
+ *
+ * Copyright (C) 2012 Marvell
+ *
+ * Thomas Petazzoni <[email protected]>
+ *
+ * This file is licensed under the terms of the GNU General Public
+ * License version 2.  This program is licensed "as is" without any
+ * warranty of any kind, whether express or implied.
+ */
+
+#ifndef __MVEBU_CLK_GATING_H
+#define __MVEBU_CLK_GATING_H
+
+void __init mvebu_clk_gating_init(void);
+
+#endif
drivers/clk/mvebu/clk.c +1 -0
index dc310d6..3f0e43e 100644
@@ -17,6 +17,7 @@
 #include <linux/of.h>
 #include "clk-core.h"
 #include "clk-cpu.h"
+#include "clk-gating-ctrl.h"
 
 static const __initconst struct of_device_id clk_match[] = {
 #ifdef CONFIG_MVEBU_CLK_CPU

drivers/clk/mvebu/clk.c static const __initconst struct of_device_id clk_match[] = { · line 39

Quoted 6 lines

drivers/clk/mvebu/clk.c +1 -0

@@ -33,5 +34,6 @@ static const __initconst struct of_device_id clk_match[] = {
 void __init mvebu_clocks_init(void)
 {
 	mvebu_core_clk_init();
+	mvebu_clk_gating_init();
 	of_clk_init(clk_match);
 }

Hi Thomas

You have a minor problem here.

obj-$(CONFIG_MVEBU_CLK_GATING) += clk-gating-ctrl.o

There is no guarantee mvebu_clk_gating_init() exists. In fact, orion5x does not have gated clocks, so won't be selecting this option.

Andrew

Thomas Petazzoni

author

2012-11-19 18:44 UTC #3 lore raw

PATCH 1/7 clk: mvebu: init all mvebu clock types from mvebu_clocks_init()

Dear Andrew Lunn,

Quoted 6 lines · Andrew Lunn

You have a minor problem here.
obj-$(CONFIG_MVEBU_CLK_GATING) += clk-gating-ctrl.o
There is no guarantee mvebu_clk_gating_init() exists. In fact, orion5x does not have gated clocks, so won't be selecting this option.

Correct, I will do the same as I did in PATCH 2/7 for the CPU clocks.

Thanks for the review,

Thomas

Signature · 4 lines

Thomas Petazzoni, Free Electrons
Kernel, drivers, real-time and embedded Linux
development, consulting, training and support.
http://free-electrons.com

Thomas Petazzoni

author

2012-11-19 16:52 UTC #4 lore raw

PATCH 2/7 clk: mvebu: move clk-cpu compatible strings into the clk-cpu driver

In reply to Thomas Petazzoni

Both the clk-core and clk-gating drivers define their compatible string in their own .c file and provide an initialization function to register the clock types. clk-cpu was not doing the same: it was defining its compatible string directly in clk.c, and calling of_clk_init() from here.

For consistency reasons, this patch moves the clk-cpu compatible string in clk-cpu.c, and implements a proper mvebu_cpu_clk_init() function like the other two clock drivers.

Signed-off-by Thomas Petazzoni <[email protected]>

---
 drivers/clk/mvebu/clk-cpu.c |   15 +++++++++++++++
 drivers/clk/mvebu/clk-cpu.h |    6 +++++-
 drivers/clk/mvebu/clk.c     |   14 +-------------
 3 files changed, 21 insertions(+), 14 deletions(-)

drivers/clk/mvebu/clk-cpu.c +15 -0

index 1df027a..3d7c9f1 100644
@@ -153,3 +153,18 @@ bail_out:
 	kfree(clks);
 	kfree(cpuclk);
 }
+
+static const __initconst struct of_device_id clk_cpu_match[] = {
+	{
+		.compatible = "marvell,armada-xp-cpu-clockctrl",
+		.data = of_cpu_clk_setup,
+	},
+	{
+		/* sentinel */
+	},
+};
+
+void __init mvebu_cpu_clk_init(void)
+{
+	of_clk_init(clk_cpu_match);
+}

drivers/clk/mvebu/clk-cpu.h +5 -1

index e208336..08e2aff 100644
@@ -13,6 +13,10 @@
 #ifndef __MVEBU_CLK_CPU_H
 #define __MVEBU_CLK_CPU_H
 
-void __init of_cpu_clk_setup(struct device_node *node);
+#ifdef CONFIG_MVEBU_CLK_CPU
+void __init mvebu_cpu_clk_init(void);
+#else
+static inline void mvebu_cpu_clk_init(void) {}
+#endif
 
 #endif

drivers/clk/mvebu/clk.c +1 ~~-13~~

index 3f0e43e..e3bc036 100644
@@ -19,21 +19,9 @@
 #include "clk-cpu.h"
 #include "clk-gating-ctrl.h"
 
-static const __initconst struct of_device_id clk_match[] = {
-#ifdef CONFIG_MVEBU_CLK_CPU
-	{
-		.compatible = "marvell,armada-xp-cpu-clockctrl",
-		.data = of_cpu_clk_setup,
-	},
-#endif
-	{
-		/* sentinel */
-	}
-};
-
 void __init mvebu_clocks_init(void)
 {
 	mvebu_core_clk_init();
 	mvebu_clk_gating_init();
-	of_clk_init(clk_match);
+	mvebu_cpu_clk_init();
 }

Signature · 1 line

1.7.9.5

Thomas Petazzoni

author

2012-11-19 16:52 UTC #5 lore raw

PATCH 3/7 clk: mvebu: rename mvebu_clk_gating_init() to mvebu_gating_clk_init()

In reply to Thomas Petazzoni

For consistency with mvebu_core_clk_init() and mvebu_cpu_clk_init(), rename mvebu_clk_gating_init() to mvebu_gating_clk_init(), so we have the nice sequence:

	mvebu_core_clk_init();
	mvebu_gating_clk_init();
	mvebu_cpu_clk_init();

in mvebu_clocks_init().

Signed-off-by Thomas Petazzoni <[email protected]>

---
 drivers/clk/mvebu/clk-gating-ctrl.c |    2 +-
 drivers/clk/mvebu/clk-gating-ctrl.h |    2 +-
 drivers/clk/mvebu/clk.c             |    2 +-
 3 files changed, 3 insertions(+), 3 deletions(-)

drivers/clk/mvebu/clk-gating-ctrl.c +1 -1

index e640d5c..745a908 100644
@@ -225,7 +225,7 @@ static const __initdata struct of_device_id clk_gating_match[] = {
 	{ }
 };
 
-void __init mvebu_clk_gating_init(void)
+void __init mvebu_gating_clk_init(void)
 {
 	struct device_node *np;

drivers/clk/mvebu/clk-gating-ctrl.h +1 -1

index 20cc4ad..f9cde9b 100644
@@ -13,6 +13,6 @@
 #ifndef __MVEBU_CLK_GATING_H
 #define __MVEBU_CLK_GATING_H
 
-void __init mvebu_clk_gating_init(void);
+void __init mvebu_gating_clk_init(void);
 
 #endif

drivers/clk/mvebu/clk.c +1 -1

index e3bc036..855681b 100644
@@ -22,6 +22,6 @@
 void __init mvebu_clocks_init(void)
 {
 	mvebu_core_clk_init();
-	mvebu_clk_gating_init();
+	mvebu_gating_clk_init();
 	mvebu_cpu_clk_init();
 }

Signature · 1 line

1.7.9.5

Thomas Petazzoni

author

2012-11-19 16:52 UTC #6 lore raw

PATCH 4/7 arm: mvebu: move DT cpu clock nodes to the Armada XP specific .dtsi

In reply to Thomas Petazzoni

The cpu clock nodes have originally been added in armada-xp.dtsi. However, this .dtsi file is common to all SoCs in the Armada XP family: the MV78230, the MV78260 and the MV78460. Those SoCs differ in a number of ways, but one of the main difference is the number of CPUs: one CPU in the MV78230, two CPUs in the MV78260 and four CPUs in the MV78460.

Therefore, this patch moves the cpu clock DT nodes into the armada-xp-mv78230.dtsi, armada-xp-mv78260.dtsi and armada-xp-mv78460.dtsi files, instantiating the right number of CPUs and CPU clocks as needed.

Signed-off-by Thomas Petazzoni <[email protected]>

---
 arch/arm/boot/dts/armada-xp-mv78230.dtsi |   11 +++++++++++
 arch/arm/boot/dts/armada-xp-mv78260.dtsi |   17 +++++++++++++++++
 arch/arm/boot/dts/armada-xp-mv78460.dtsi |   30 ++++++++++++++++++++++++++++++
 arch/arm/boot/dts/armada-xp.dtsi         |   29 -----------------------------
 4 files changed, 58 insertions(+), 29 deletions(-)

arch/arm/boot/dts/armada-xp-mv78230.dtsi +11 -0

index ea35519..0be699b 100644
@@ -24,6 +24,17 @@
 		gpio1 = &gpio1;
 	};
 
+	cpus {
+	    #address-cells = <1>;
+	    #size-cells = <0>;
+
+	    cpu at 0 {
+		compatible = "marvell,sheeva-v7";
+		reg = <0>;
+		clocks = <&cpuclk 0>;
+	    };
+	}
+
 	soc {
 		pinctrl {
 			compatible = "marvell,mv78230-pinctrl";

arch/arm/boot/dts/armada-xp-mv78260.dtsi +17 -0

index 2057863..4417b87 100644
@@ -25,6 +25,23 @@
 		gpio2 = &gpio2;
 	};
 
+	cpus {
+	    #address-cells = <1>;
+	    #size-cells = <0>;
+
+	    cpu at 0 {
+		compatible = "marvell,sheeva-v7";
+		reg = <0>;
+		clocks = <&cpuclk 0>;
+	    };
+
+	    cpu at 1 {
+		compatible = "marvell,sheeva-v7";
+		reg = <1>;
+		clocks = <&cpuclk 1>;
+	    };
+	};
+
 	soc {
 		pinctrl {
 			compatible = "marvell,mv78260-pinctrl";

arch/arm/boot/dts/armada-xp-mv78460.dtsi +30 -0

index ffac983..1313c3e 100644
@@ -25,6 +25,36 @@
 		gpio2 = &gpio2;
 	};
 
+
+	cpus {
+	    #address-cells = <1>;
+	    #size-cells = <0>;
+
+	    cpu at 0 {
+		compatible = "marvell,sheeva-v7";
+		reg = <0>;
+		clocks = <&cpuclk 0>;
+	    };
+
+	    cpu at 1 {
+		compatible = "marvell,sheeva-v7";
+		reg = <1>;
+		clocks = <&cpuclk 1>;
+	    };
+
+	    cpu at 2 {
+		compatible = "marvell,sheeva-v7";
+		reg = <2>;
+		clocks = <&cpuclk 2>;
+	    };
+
+	    cpu at 3 {
+		compatible = "marvell,sheeva-v7";
+		reg = <3>;
+		clocks = <&cpuclk 3>;
+	    };
+	};
+
 	soc {
 		pinctrl {
 			compatible = "marvell,mv78460-pinctrl";

arch/arm/boot/dts/armada-xp.dtsi +0 ~~-29~~

index 84cb87d..a3824ac 100644
@@ -34,35 +34,6 @@
 		    <0xd0021870 0x58>;
 	};
 
-	cpus {
-	    #address-cells = <1>;
-	    #size-cells = <0>;
-
-	    cpu at 0 {
-		compatible = "marvell,sheeva-v7";
-		reg = <0>;
-		clocks = <&cpuclk 0>;
-	    };
-
-	    cpu at 1 {
-		compatible = "marvell,sheeva-v7";
-		reg = <1>;
-		clocks = <&cpuclk 1>;
-	    };
-
-	    cpu at 2 {
-		compatible = "marvell,sheeva-v7";
-		reg = <2>;
-		clocks = <&cpuclk 2>;
-	    };
-
-	    cpu at 3 {
-		compatible = "marvell,sheeva-v7";
-		reg = <3>;
-		clocks = <&cpuclk 3>;
-	    };
-	};
-
 	soc {
 		serial at d0012200 {
 				compatible = "ns16550";

Signature · 1 line

1.7.9.5

Andrew Lunn

maintainer

2012-11-19 18:38 UTC #7 lore raw

PATCH 4/7 arm: mvebu: move DT cpu clock nodes to the Armada XP specific .dtsi

Quoted 11 lines · Thomas Petazzoni

The cpu clock nodes have originally been added in armada-xp.dtsi. However, this .dtsi file is common to all SoCs in the Armada XP family: the MV78230, the MV78260 and the MV78460. Those SoCs differ in a number of ways, but one of the main difference is the number of CPUs: one CPU in the MV78230, two CPUs in the MV78260 and four CPUs in the MV78460.
Therefore, this patch moves the cpu clock DT nodes into the armada-xp-mv78230.dtsi, armada-xp-mv78260.dtsi and armada-xp-mv78460.dtsi files, instantiating the right number of CPUs and CPU clocks as needed.

Hi Thomas

This is only part of the story:

void __init of_cpu_clk_setup(struct device_node *node)

{
        struct cpu_clk *cpuclk;
        void __iomem *clock_complex_base = of_iomap(node, 0);
        int cpu;
        if (clock_complex_base == NULL) {
                pr_err("%s: clock-complex base register not set\n",
                        __func__);
                return;
        }

        cpuclk = kzalloc(MAX_CPU * sizeof(*cpuclk), GFP_KERNEL);
        clks = kzalloc(MAX_CPU * sizeof(*clks), GFP_KERNEL);

        if (WARN_ON(!cpuclk))
                return;
        for (cpu = 0; cpu < MAX_CPU; cpu++) {
                struct clk_init_data init;
                struct clk *clk;
                struct clk *parent_clk;
                char *clk_name = kzalloc(5, GFP_KERNEL);

                sprintf(clk_name, "cpu%d", cpu);
                parent_clk = of_clk_get(node, 0);

...

                clk = clk_register(NULL, &cpuclk[cpu].hw);
                if (WARN_ON(IS_ERR(clk)))
                        goto bail_out;
                clks[cpu] = clk;
        }

You are still registering 4 clocks as far as i can see.

Andrew

Thomas Petazzoni

author

2012-11-19 18:50 UTC #8 lore raw

PATCH 4/7 arm: mvebu: move DT cpu clock nodes to the Armada XP specific .dtsi

Dear Andrew Lunn,

Quoted 3 lines · Andrew Lunn

This is only part of the story:
void __init of_cpu_clk_setup(struct device_node *node)

[...]

Quoted 1 line

        for (cpu = 0; cpu < MAX_CPU; cpu++) {

[...]

Quoted 1 line

You are still registering 4 clocks as far as i can see.

Aah, correct. I'll fix this and resend!

Thanks for the review,

Thomas

Signature · 4 lines

Thomas Petazzoni, Free Electrons
Kernel, drivers, real-time and embedded Linux
development, consulting, training and support.
http://free-electrons.com

Thomas Petazzoni

author

2012-11-19 16:52 UTC #9 lore raw

PATCH 5/7 clk: mvebu: rename *-core-clocks to *-core-clock

In reply to Thomas Petazzoni

Most of the existing clock drivers have a compatible string that ends with "-clock", so change the compatible strings of the mvebu clk-core.c driver to not have the final 's' in '*-core-clocks'.

Signed-off-by Thomas Petazzoni <[email protected]>

---
 .../devicetree/bindings/clock/mvebu-core-clock.txt |   12 ++++++------
 arch/arm/boot/dts/armada-370.dtsi                  |    2 +-
 arch/arm/boot/dts/armada-xp.dtsi                   |    2 +-
 arch/arm/boot/dts/dove.dtsi                        |    2 +-
 arch/arm/boot/dts/kirkwood.dtsi                    |    2 +-
 drivers/clk/mvebu/clk-core.c                       |   10 +++++-----
 6 files changed, 15 insertions(+), 15 deletions(-)

Documentation/devicetree/bindings/clock/mvebu-core-clock.txt +6 -6

index ffb3d69..1e66294 100644
@@ -19,11 +19,11 @@ The following is a list of provided IDs and clock names on Kirkwood and Dove:
 
 Required properties:
 - compatible : shall be one of the following:
-	"marvell,armada-370-core-clocks" - For Armada 370 SoC core clocks
-	"marvell,armada-xp-core-clocks" - For Armada XP SoC core clocks
-	"marvell,dove-core-clocks" - for Dove SoC core clocks
-	"marvell,kirkwood-core-clocks" - for Kirkwood SoC (except mv88f6180)
-	"marvell,mv88f6180-core-clocks" - for Kirkwood MV88f6180 SoC
+	"marvell,armada-370-core-clock" - For Armada 370 SoC core clocks
+	"marvell,armada-xp-core-clock" - For Armada XP SoC core clocks
+	"marvell,dove-core-clock" - for Dove SoC core clocks
+	"marvell,kirkwood-core-clock" - for Kirkwood SoC (except mv88f6180)
+	"marvell,mv88f6180-core-clock" - for Kirkwood MV88f6180 SoC
 - reg : shall be the register address of the Sample-At-Reset (SAR) register
 - #clock-cells : from common clock binding; shall be set to 1

@@ -34,7 +34,7 @@ Optional properties:
 Example:
 
 core_clk: core-clocks at d0214 {
-	compatible = "marvell,dove-core-clocks";
+	compatible = "marvell,dove-core-clock";
 	reg = <0xd0214 0x4>;
 	#clock-cells = <1>;
 };

arch/arm/boot/dts/armada-370.dtsi +1 -1

index d4fac86..1ed9e5c 100644
@@ -83,7 +83,7 @@
 		};
 
 		coreclk: mvebu-sar at d0018230 {
-			compatible = "marvell,armada-370-core-clocks";
+			compatible = "marvell,armada-370-core-clock";
 			reg = <0xd0018230 0x08>;
 			#clock-cells = <1>;
 		};

arch/arm/boot/dts/armada-xp.dtsi +1 -1

index a3824ac..d05515c 100644
@@ -55,7 +55,7 @@
 		};
 
 		coreclk: mvebu-sar at d0018230 {
-			compatible = "marvell,armada-xp-core-clocks";
+			compatible = "marvell,armada-xp-core-clock";
 			reg = <0xd0018230 0x08>;
 			#clock-cells = <1>;
 		};

arch/arm/boot/dts/dove.dtsi +1 -1

index 8c8ab621..b524ee3 100644
@@ -32,7 +32,7 @@
 		};
 
 		core_clk: core-clocks at d0214 {
-			compatible = "marvell,dove-core-clocks";
+			compatible = "marvell,dove-core-clock";
 			reg = <0xd0214 0x4>;
 			#clock-cells = <1>;
 		};

arch/arm/boot/dts/kirkwood.dtsi +1 -1

index 46bfa3c..0e94750 100644
@@ -24,7 +24,7 @@
 		#size-cells = <1>;
 
 		core_clk: core-clocks at 10030 {
-                	compatible = "marvell,kirkwood-core-clocks";
+			compatible = "marvell,kirkwood-core-clock";
 			reg = <0x10030 0x4>;
 	        	#clock-cells = <1>;
 		};

drivers/clk/mvebu/clk-core.c +5 -5

index 7e715ae..69056a7 100644
@@ -634,28 +634,28 @@ static const struct core_clocks mv88f6180_core_clocks = {
 static const __initdata struct of_device_id clk_core_match[] = {
 #ifdef CONFIG_MACH_ARMADA_370_XP
 	{
-		.compatible = "marvell,armada-370-core-clocks",
+		.compatible = "marvell,armada-370-core-clock",
 		.data = &armada_370_core_clocks,
 	},
 	{
-		.compatible = "marvell,armada-xp-core-clocks",
+		.compatible = "marvell,armada-xp-core-clock",
 		.data = &armada_xp_core_clocks,
 	},
 #endif
 #ifdef CONFIG_ARCH_DOVE
 	{
-		.compatible = "marvell,dove-core-clocks",
+		.compatible = "marvell,dove-core-clock",
 		.data = &dove_core_clocks,
 	},
 #endif
 
 #ifdef CONFIG_ARCH_KIRKWOOD
 	{
-		.compatible = "marvell,kirkwood-core-clocks",
+		.compatible = "marvell,kirkwood-core-clock",
 		.data = &kirkwood_core_clocks,
 	},
 	{
-		.compatible = "marvell,mv88f6180-core-clocks",
+		.compatible = "marvell,mv88f6180-core-clock",
 		.data = &mv88f6180_core_clocks,
 	},
 #endif

Signature · 1 line

1.7.9.5

Thomas Petazzoni

author

2012-11-19 16:52 UTC #10 lore raw

PATCH 6/7 clk: mvebu: rename armada-xp-cpu-clockctrl to armada-xp-cpu-clock

In reply to Thomas Petazzoni

Most of the existing clock drivers have a compatible string that ends with "-clock", so change the "armada-xp-cpu-clockctrl" compatible strings of the mvebu clk-cpu.c driver to "armada-xp-cpu-clock".

Signed-off-by Thomas Petazzoni <[email protected]>

---
 .../devicetree/bindings/clock/mvebu-cpu-clock.txt  |    4 ++--
 arch/arm/boot/dts/armada-xp.dtsi                   |    2 +-
 drivers/clk/mvebu/clk-cpu.c                        |    2 +-
 3 files changed, 4 insertions(+), 4 deletions(-)

Documentation/devicetree/bindings/clock/mvebu-cpu-clock.txt +2 -2

index 1fb5a64..feb8301 100644
@@ -2,14 +2,14 @@ Device Tree Clock bindings for cpu clock of Marvell EBU platforms
 
 Required properties:
 - compatible : shall be one of the following:
-	"marvell,armada-xp-cpu-clockctrl" - cpu clocks for Armada XP
+	"marvell,armada-xp-cpu-clock" - cpu clocks for Armada XP
 - reg : Address and length of the clock complex register set
 - #clock-cells : should be set to 1.
 - clocks : shall be the input parent clock phandle for the clock.
 
 cpuclk: clock-complex at d0018700 {
 	#clock-cells = <1>;
-	compatible = "marvell,armada-xp-cpu-clockctrl";
+	compatible = "marvell,armada-xp-cpu-clock";
 	reg = <0xd0018700 0xA0>;
 	clocks = <&coreclk 1>;
 }

arch/arm/boot/dts/armada-xp.dtsi +1 -1

index d05515c..781127e 100644
@@ -62,7 +62,7 @@
 
 		cpuclk: clock-complex at d0018700 {
 			#clock-cells = <1>;
-			compatible = "marvell,armada-xp-cpu-clockctrl";
+			compatible = "marvell,armada-xp-cpu-clock";
 			reg = <0xd0018700 0xA0>;
 			clocks = <&coreclk 1>;
 		};

drivers/clk/mvebu/clk-cpu.c +1 -1

index 3d7c9f1..44f541a 100644
@@ -156,7 +156,7 @@ bail_out:
 
 static const __initconst struct of_device_id clk_cpu_match[] = {
 	{
-		.compatible = "marvell,armada-xp-cpu-clockctrl",
+		.compatible = "marvell,armada-xp-cpu-clock",
 		.data = of_cpu_clk_setup,
 	},
 	{

Signature · 1 line

1.7.9.5

Thomas Petazzoni

author

2012-11-19 16:52 UTC #11 lore raw

PATCH 7/7 arm: mvebu: fix SATA clocks on Armada 370/XP

In reply to Thomas Petazzoni

The Armada 370/XP have gatable clocks for the SATA0 and SATA1 ports, and are not connected directly to the core clocks. Therefore, we adjust the clock references to reflect this.

Note that the clock references are moved down from the common armada-370-xp.dtsi to the more specific armada-370.dtsi and armada-xp.dtsi because the gatable clock driver is different on both SoCs. Also, the Armada XP has two more "SATA Link" clocks that we may have to use in the future.

Signed-off-by Thomas Petazzoni <[email protected]>

---
 arch/arm/boot/dts/armada-370-xp.dtsi |    1 -
 arch/arm/boot/dts/armada-370.dtsi    |    5 +++++
 arch/arm/boot/dts/armada-xp.dtsi     |    5 +++++
 3 files changed, 10 insertions(+), 1 deletion(-)

arch/arm/boot/dts/armada-370-xp.dtsi +0 -1

index 08ec354..c0414ae 100644
@@ -94,7 +94,6 @@
 			compatible = "marvell,orion-sata";
 			reg = <0xd00a0000 0x2400>;
 			interrupts = <55>;
-			clocks = <&coreclk 0>;
 			status = "disabled";
 		};

arch/arm/boot/dts/armada-370.dtsi +5 -0

index 1ed9e5c..5901995 100644
@@ -82,6 +82,11 @@
 			interrupts = <91>;
 		};
 
+		sata at d00a0000 {
+			clocks = <&gateclk 15>, <&gateclk 30>;
+			clock-names = "0", "1";
+		};
+
 		coreclk: mvebu-sar at d0018230 {
 			compatible = "marvell,armada-370-core-clock";
 			reg = <0xd0018230 0x08>;

arch/arm/boot/dts/armada-xp.dtsi +5 -0

index 781127e..be4c7cf 100644
@@ -92,5 +92,10 @@
 				interrupts = <14>;
 				status = "disabled";
 		};
+
+		sata at d00a0000 {
+			clocks = <&gateclk 15>, <&gateclk 30>;
+			clock-names = "0", "1";
+		};
 	};
 };

Signature · 1 line

1.7.9.5

Andrew Lunn

maintainer

2012-11-19 18:27 UTC #12 lore raw

In reply to Thomas Petazzoni

Quoted 16 lines · Thomas Petazzoni

Andrew,
You'll find enclosed seven patches brigging fixes and improvements to the CLK support on mvebu platforms. You can either squash them into your current clk series if you intend to send a new version of it to Jason, or forward the patches as follow-up patches to Jason after reviewing.
In summary:
* Patch 1 fixes a real problem: the gating clocks are not registered on Armada 370/XP.
* Patch 2, 3, 5 and 6 are more cosmetic patches, but I think they are good to have.

Quoted 7 lines

* Patch 4 implements a fix that we've been discussing: moving the Armada XP CPU clocks down to the individual armada-xp-mv78xxx.dtsi files.
* Patch 7 fixes the references to the SATA clocks on Armada 370/XP. Without this patch, Armada 370 currently hangs at boot when trying to initialize SATA.

Hi Thomas

You probably want patch 1 last. Anybody bisecting this changeset if going to have a hung 370/XP.

Andrew

Thomas Petazzoni

author

2012-11-19 18:46 UTC #13 lore raw

Dear Andrew Lunn,

Quoted 21 lines · Andrew Lunn

Quoted 20 lines · Thomas Petazzoni
Andrew,
You'll find enclosed seven patches brigging fixes and improvements to the CLK support on mvebu platforms. You can either squash them into your current clk series if you intend to send a new version of it to Jason, or forward the patches as follow-up patches to Jason after reviewing.
In summary:
* Patch 1 fixes a real problem: the gating clocks are not registered on Armada 370/XP.
* Patch 2, 3, 5 and 6 are more cosmetic patches, but I think they are good to have.
* Patch 4 implements a fix that we've been discussing: moving the Armada XP CPU clocks down to the individual armada-xp-mv78xxx.dtsi files.

Quoted 8 lines

Quoted 3 lines
* Patch 7 fixes the references to the SATA clocks on Armada 370/XP. Without this patch, Armada 370 currently hangs at boot when trying to initialize SATA.
Hi Thomas
You probably want patch 1 last. Anybody bisecting this changeset if going to have a hung 370/XP.

Took me a while to figure out why: it's the fact that gating clocks are getting registered that I was seeing the SATA hang on Armada 370/XP. Sure, I'll put PATCH 1 last in the series then.

Thanks!

Thomas

Signature · 4 lines

Thomas Petazzoni, Free Electrons
Kernel, drivers, real-time and embedded Linux
development, consulting, training and support.
http://free-electrons.com

Thomas Petazzoni

author

2012-11-20 09:40 UTC #14 lore raw

In reply to Andrew Lunn

Andrew,

Quoted 12 lines · Andrew Lunn

Quoted 12 lines
In summary:
* Patch 1 fixes a real problem: the gating clocks are not registered on Armada 370/XP.
* Patch 2, 3, 5 and 6 are more cosmetic patches, but I think they are good to have.
* Patch 4 implements a fix that we've been discussing: moving the Armada XP CPU clocks down to the individual armada-xp-mv78xxx.dtsi files.

Quoted 8 lines

Quoted 3 lines
* Patch 7 fixes the references to the SATA clocks on Armada 370/XP. Without this patch, Armada 370 currently hangs at boot when trying to initialize SATA.
Hi Thomas
You probably want patch 1 last. Anybody bisecting this changeset if going to have a hung 370/XP.

Unfortunately, I don't think putting patch 1 last will work either: in patch 7, I change the SATA clocks to point to the gating clocks. So if patch 1 is not there, those clocks do not exist and the driver fails to clk_get() those clocks.

Thomas

Signature · 4 lines

Thomas Petazzoni, Free Electrons
Kernel, drivers, real-time and embedded Linux
development, consulting, training and support.
http://free-electrons.com

Andrew Lunn

maintainer

2012-11-20 09:43 UTC #15 lore raw

Quoted 22 lines · Thomas Petazzoni

Andrew,
Quoted 18 lines · Andrew Lunn
Quoted 8 lines
In summary:
* Patch 1 fixes a real problem: the gating clocks are not registered on Armada 370/XP.
* Patch 2, 3, 5 and 6 are more cosmetic patches, but I think they are good to have.
Quoted 7 lines
* Patch 4 implements a fix that we've been discussing: moving the Armada XP CPU clocks down to the individual armada-xp-mv78xxx.dtsi files.
* Patch 7 fixes the references to the SATA clocks on Armada 370/XP. Without this patch, Armada 370 currently hangs at boot when trying to initialize SATA.
Hi Thomas

Quoted 7 lines

Quoted 2 lines
You probably want patch 1 last. Anybody bisecting this changeset if going to have a hung 370/XP.
Unfortunately, I don't think putting patch 1 last will work either: in patch 7, I change the SATA clocks to point to the gating clocks. So if patch 1 is not there, those clocks do not exist and the driver fails to clk_get() those clocks.

I don't think that is fatal. Orion5x does not have those clocks. The PCI version of the SATA devices also does not have those clocks.

Andrew

Thomas Petazzoni

author

2012-11-20 10:06 UTC #16 lore raw

Dear Andrew Lunn,

Quoted 7 lines · Andrew Lunn

Quoted 4 lines
Unfortunately, I don't think putting patch 1 last will work either: in patch 7, I change the SATA clocks to point to the gating clocks. So if patch 1 is not there, those clocks do not exist and the driver fails to clk_get() those clocks.
I don't think that is fatal. Orion5x does not have those clocks. The PCI version of the SATA devices also does not have those clocks.

Hum, I will check this. But I think that the point that lead me to look into the clocks is that when I added:

	clocks = <&gate_clk xx>;

to my Ethernet DT nodes, then clk_get() was returning an error, until I enabled the gating clocks. But I'll double check that.

Thanks,

Thomas

Signature · 4 lines

Thomas Petazzoni, Free Electrons
Kernel, drivers, real-time and embedded Linux
development, consulting, training and support.
http://free-electrons.com

Thomas Petazzoni

author

2012-11-20 10:50 UTC #17 lore raw

Quoted 7 lines · Thomas Petazzoni

Hum, I will check this. But I think that the point that lead me to look into the clocks is that when I added:
	clocks = <&gate_clk xx>;
to my Ethernet DT nodes, then clk_get() was returning an error, until I enabled the gating clocks. But I'll double check that.

It is indeed not fatal with SATA because the driver tries to get the clock, and if it can't, the driver just gives up.

However, with the mvneta driver, it really wants the clock (the driver needs at some point to do a clk_get_rate(), so we have to have a clock), and in this case, not having the gating clocks is an issue.

That said, since the mvneta driver is new, I don't mind if it is non-functional up to the point where the gating clocks are enabled.

Thomas

Signature · 4 lines

Thomas Petazzoni, Free Electrons
Kernel, drivers, real-time and embedded Linux
development, consulting, training and support.
http://free-electrons.com

Thomas Petazzoni

author

2012-11-20 16:22 UTC #18 lore raw

In reply to Andrew Lunn

Andrew,

Quoted 10 lines · Andrew Lunn

Quoted 10 lines
* Patch 1 fixes a real problem: the gating clocks are not
registered on Armada 370/XP.
* Patch 2, 3, 5 and 6 are more cosmetic patches, but I think they
are good to have.
* Patch 4 implements a fix that we've been discussing: moving the Armada XP CPU clocks down to the individual
armada-xp-mv78xxx.dtsi files.

Quoted 8 lines

Quoted 3 lines
* Patch 7 fixes the references to the SATA clocks on Armada 370/XP. Without this patch, Armada 370 currently hangs at boot
when trying to initialize SATA.
Hi Thomas
You probably want patch 1 last. Anybody bisecting this changeset if going to have a hung 370/XP.

In the end, here is how I fixed this:

* The clk patches are going to be pulled first by Jason. It enables gating clocks on all platforms, and it boots fine on Armada 370, Armada XP and Kirkwood.

* The entire Armada 370/XP SATA patches are going to be pulled by Jason *after* the clk patches. So the SATA DT entries will from the beginning have the correct 'clocks' property, so that when you boot clk + sata, it works ok on Armada 370 and Armada XP.

I hope that's ok for you.

Thanks!

Thomas

Signature · 4 lines

Thomas Petazzoni, Free Electrons
Kernel, drivers, real-time and embedded Linux
development, consulting, training and support.
http://free-electrons.com