Skip to contentSkip to conversation

ARM: ux500: Enable clocks for Device Tree

PATCH In review Lee Jones 9 participants 89 messages Jun 6, 2013 – Sep 13, 2013

Subsystems (7)
lore
Share & tools
lore

Lee Jones

author
#0 lore raw

cover ARM: ux500: Enable clocks for Device Tree

After this patchset has been applied, we can request clocks directly from Device Tree without using any AUXDATA device-name hacks. We also take care to remove all of thos at the end of the set.

author
#1 lore raw

PATCH 1/33 mfd: dbx500-prcmu: Provide PRCMU numerical clock identifiers

These will we used to request DBx500 PRCMU clocks from Device Tree. The numbers used are taken directly from the Hardware Specification document.

---
 include/dt-bindings/mfd/dbx500-prcmu.h |   71 ++++++++++++++++++++++++++++++++
 1 file changed, 71 insertions(+)
 create mode 100644 include/dt-bindings/mfd/dbx500-prcmu.h
include/dt-bindings/mfd/dbx500-prcmu.h new file +71 -0
new file mode 100644
index 0000000..e646664
@@ -0,0 +1,71 @@
+/*
+ * This header provides constants for the PRCMU bindings.
+ *
+ */
+
+#ifndef _DT_BINDINGS_MFD_PRCMU_H
+#define _DT_BINDINGS_MFD_PRCMU_H
+
+/*
+ * Clock identifiers.
+ */
+
+#define ARMCLK			0
+#define PRCMU_ACLK		1
+#define PRCMU_SVAMMCSPCLK 	2
+#define PRCMU_SIACLK 		3
+#define PRCMU_SGACLK 		4
+#define PRCMU_UARTCLK 		5
+#define PRCMU_MSP02CLK 		6
+#define PRCMU_MSP1CLK 		7
+#define PRCMU_I2CCLK 		8
+#define PRCMU_SDMMCCLK 		9
+#define PRCMU_SLIMCLK 		10
+#define PRCMU_PER1CLK 		11
+#define PRCMU_PER2CLK 		12
+#define PRCMU_PER3CLK 		13
+#define PRCMU_PER5CLK 		14
+#define PRCMU_PER6CLK 		15
+#define PRCMU_PER7CLK 		16
+#define PRCMU_LCDCLK 		17
+#define PRCMU_BMLCLK 		18
+#define PRCMU_HSITXCLK 		19
+#define PRCMU_HSIRXCLK 		20
+#define PRCMU_HDMICLK		21
+#define PRCMU_APEATCLK 		22
+#define PRCMU_APETRACECLK 	23
+#define PRCMU_MCDECLK  	 	24
+#define PRCMU_IPI2CCLK  	25
+#define PRCMU_DSIALTCLK  	26
+#define PRCMU_DMACLK  	 	27
+#define PRCMU_B2R2CLK  	 	28
+#define PRCMU_TVCLK  	 	29
+#define SPARE_UNIPROCLK  	30
+#define PRCMU_SSPCLK  	 	31
+#define PRCMU_RNGCLK  	 	32
+#define PRCMU_UICCCLK  	 	33
+#define SPARE1CLK  	 	34
+#define SPARE2CLK  	 	35
+
+#define PRCMU_NUM_REG_CLOCKS  	39
+
+#define PRCMU_RTCCLK  	 	PRCMU_NUM_REG_CLOCKS
+#define PRCMU_SYSCLK  	 	40
+#define PRCMU_CDCLK  	 	41
+#define PRCMU_TIMCLK  	 	42
+#define PRCMU_PLLSOC0  	 	43
+#define PRCMU_PLLSOC1  	 	44
+#define PRCMU_ARMSS  	 	45
+#define PRCMU_PLLDDR  	 	46
+
+/* DSI Clocks */
+#define PRCMU_PLLDSI  	 	47
+#define PRCMU_DSI0CLK 	  	48
+#define PRCMU_DSI1CLK  	 	49
+#define PRCMU_DSI0ESCCLK  	50
+#define PRCMU_DSI1ESCCLK  	51
+#define PRCMU_DSI2ESCCLK  	52
+
+#define PRCMU_NUM_CLKS  	53
+
+#endif
Signature · 1 line
1.7.10.4
author
#2 lore raw

PATCH 2/33 ARM: ux500: Add PRCMU clock node to DBx500 Device Tree

In reply to Lee Jones
---
 arch/arm/boot/dts/dbx5x0.dtsi |    9 +++++++++
 1 file changed, 9 insertions(+)
arch/arm/boot/dts/dbx5x0.dtsi +9 -0
index a082f0b..a635f7f 100644
@@ -10,6 +10,7 @@
  */
 
 #include <dt-bindings/interrupt-controller/irq.h>
+#include <dt-bindings/mfd/dbx500-prcmu.h>
 #include "skeleton.dtsi"
 
 / {
@@ -42,6 +43,14 @@
 			interrupts = <0 7 IRQ_TYPE_LEVEL_HIGH>;
 		};
 
+		clocks {
+			compatible = "stericsson,u8500-clks";
+
+			prcmu_clk: prcmu-clock {
+				#clock-cells = <1>;
+			};
+		};
+
 		timer at a0410600 {
 			compatible = "arm,cortex-a9-twd-timer";
 			reg = <0xa0410600 0x20>;
Signature · 1 line
1.7.10.4
author
#3 lore raw

PATCH 3/33 ARM: ux500: Supply the DMA clock lookup to the DBX500 DT

In reply to Lee Jones
---
 arch/arm/boot/dts/dbx5x0.dtsi |    2 ++
 1 file changed, 2 insertions(+)
arch/arm/boot/dts/dbx5x0.dtsi +2 -0
index a635f7f..c5f187b 100644
@@ -229,6 +229,8 @@
 
 			#dma-cells = <3>;
 			memcpy-channels = <56 57 58 59 60>;
+
+			clocks = <&prcmu_clk PRCMU_DMACLK>;
 		};
 
 		prcmu: prcmu at 80157000 {
Signature · 1 line
1.7.10.4
author
#4 lore raw

PATCH 4/33 ARM: ux500: Add PRCC Peripheral clock node to DBx500 Device Tree

In reply to Lee Jones
---
 arch/arm/boot/dts/dbx5x0.dtsi |    4 ++++
 1 file changed, 4 insertions(+)
arch/arm/boot/dts/dbx5x0.dtsi +4 -0
index c5f187b..18c5c27 100644
@@ -49,6 +49,10 @@
 			prcmu_clk: prcmu-clock {
 				#clock-cells = <1>;
 			};
+
+			prcc_pclk: prcc-periph-clock {
+				#clock-cells = <2>;
+			};
 		};
 
 		timer at a0410600 {
Signature · 1 line
1.7.10.4
author
#5 lore raw

PATCH 5/33 ARM: ux500: Supply the GPIO clocks lookup to the DBX500 DT

In reply to Lee Jones
---
 arch/arm/boot/dts/dbx5x0.dtsi |   18 ++++++++++++++++++
 1 file changed, 18 insertions(+)
arch/arm/boot/dts/dbx5x0.dtsi +18 -0
index 18c5c27..928ef56 100644
@@ -78,6 +78,8 @@
 			gpio-controller;
 			#gpio-cells = <2>;
 			gpio-bank = <0>;
+
+			clocks = <&prcc_pclk 1 9>;
 		};
 
 		gpio1: gpio at 8012e080 {
@@ -91,6 +93,8 @@
 			gpio-controller;
 			#gpio-cells = <2>;
 			gpio-bank = <1>;
+
+			clocks = <&prcc_pclk 1 9>;
 		};
 
 		gpio2: gpio at 8000e000 {
@@ -104,6 +108,8 @@
 			gpio-controller;
 			#gpio-cells = <2>;
 			gpio-bank = <2>;
+
+			clocks = <&prcc_pclk 3 8>;
 		};
 
 		gpio3: gpio at 8000e080 {
@@ -117,6 +123,8 @@
 			gpio-controller;
 			#gpio-cells = <2>;
 			gpio-bank = <3>;
+
+			clocks = <&prcc_pclk 3 8>;
 		};
 
 		gpio4: gpio at 8000e100 {
@@ -130,6 +138,8 @@
 			gpio-controller;
 			#gpio-cells = <2>;
 			gpio-bank = <4>;
+
+			clocks = <&prcc_pclk 3 8>;
 		};
 
 		gpio5: gpio at 8000e180 {
@@ -143,6 +153,8 @@
 			gpio-controller;
 			#gpio-cells = <2>;
 			gpio-bank = <5>;
+
+			clocks = <&prcc_pclk 3 8>;
 		};
 
 		gpio6: gpio at 8011e000 {
@@ -156,6 +168,8 @@
 			gpio-controller;
 			#gpio-cells = <2>;
 			gpio-bank = <6>;
+
+			clocks = <&prcc_pclk 2 1>;
 		};
 
 		gpio7: gpio at 8011e080 {
@@ -169,6 +183,8 @@
 			gpio-controller;
 			#gpio-cells = <2>;
 			gpio-bank = <7>;
+
+			clocks = <&prcc_pclk 2 1>;
 		};
 
 		gpio8: gpio at a03fe000 {
@@ -182,6 +198,8 @@
 			gpio-controller;
 			#gpio-cells = <2>;
 			gpio-bank = <8>;
+
+			clocks = <&prcc_pclk 6 1>;
 		};
 
 		pinctrl {
Signature · 1 line
1.7.10.4
author
#6 lore raw

PATCH 6/33 ARM: ux500: Supply the USB clock lookup to the DBX500 DT

In reply to Lee Jones
---
 arch/arm/boot/dts/dbx5x0.dtsi |    2 ++
 1 file changed, 2 insertions(+)
arch/arm/boot/dts/dbx5x0.dtsi +2 -0
index 928ef56..d3cc575c 100644
@@ -241,6 +241,8 @@
 				    "iep_6_14", "oep_6_14",
 				    "iep_7_15", "oep_7_15",
 				    "iep_8",    "oep_8";
+
+			clocks = <&prcc_pclk 5 0>;
 		};
 
 		dma: dma-controller at 801C0000 {
Signature · 1 line
1.7.10.4
author
#7 lore raw

PATCH 7/33 ARM: ux500: Supply the Ethernet clock lookup to Snowball's DT

In reply to Lee Jones
---
 arch/arm/boot/dts/snowball.dts |    3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)
arch/arm/boot/dts/snowball.dts +2 -1
index fb9dce5..e20e8ae 100644
@@ -110,12 +110,13 @@
 				vdd33a-supply = <&en_3v3_reg>;
 				vddvario-supply = <&db8500_vape_reg>;
 
-
 				reg-shift = <1>;
 				reg-io-width = <2>;
 				smsc,force-internal-phy;
 				smsc,irq-active-high;
 				smsc,irq-push-pull;
+
+				clocks = <&prcc_pclk 3 0>;
 			};
 		};
 
Signature · 1 line
1.7.10.4
author
#8 lore raw

PATCH 8/33 ARM: ux500: Add PRCC Kernel clock node to DBx500 Device Tree

In reply to Lee Jones
---
 arch/arm/boot/dts/dbx5x0.dtsi |    4 ++++
 1 file changed, 4 insertions(+)
arch/arm/boot/dts/dbx5x0.dtsi +4 -0
index d3cc575c..d682ea0 100644
@@ -53,6 +53,10 @@
 			prcc_pclk: prcc-periph-clock {
 				#clock-cells = <2>;
 			};
+
+			prcc_kclk: prcc-kernel-clock {
+				#clock-cells = <2>;
+			};
 		};
 
 		timer at a0410600 {
Signature · 1 line
1.7.10.4
author
#9 lore raw

PATCH 9/33 ARM: ux500: Supply the I2C clocks lookup to the DBX500 DT

In reply to Lee Jones
---
 arch/arm/boot/dts/dbx5x0.dtsi |   14 ++++++++++++++
 1 file changed, 14 insertions(+)
arch/arm/boot/dts/dbx5x0.dtsi +14 -0
index d682ea0..e49c679 100644
@@ -572,6 +572,8 @@
 			v-i2c-supply = <&db8500_vape_reg>;
 
 			clock-frequency = <400000>;
+			clocks = <&prcc_kclk 3 3>, <&prcc_pclk 3 3>;
+			clock-names = "nmk-i2c.0", "apb_pclk";
 		};
 
 		i2c at 80122000 {
@@ -585,6 +587,9 @@
 			v-i2c-supply = <&db8500_vape_reg>;
 
 			clock-frequency = <400000>;
+
+			clocks = <&prcc_kclk 1 2>, <&prcc_pclk 1 2>;
+			clock-names = "nmk-i2c.1", "apb_pclk";
 		};
 
 		i2c at 80128000 {
@@ -598,6 +603,9 @@
 			v-i2c-supply = <&db8500_vape_reg>;
 
 			clock-frequency = <400000>;
+
+			clocks = <&prcc_kclk 1 6>, <&prcc_pclk 1 6>;
+			clock-names = "nmk-i2c.2", "apb_pclk";
 		};
 
 		i2c at 80110000 {
@@ -611,6 +619,9 @@
 			v-i2c-supply = <&db8500_vape_reg>;
 
 			clock-frequency = <400000>;
+
+			clocks = <&prcc_kclk 2 0>, <&prcc_pclk 2 0>;
+			clock-names = "nmk-i2c.3", "apb_pclk";
 		};
 
 		i2c at 8012a000 {
@@ -624,6 +635,9 @@
 			v-i2c-supply = <&db8500_vape_reg>;
 
 			clock-frequency = <400000>;
+
+			clocks = <&prcc_kclk 1 9>, <&prcc_pclk 1 9>;
+			clock-names = "nmk-i2c.4", "apb_pclk";
 		};
 
 		ssp at 80002000 {
Signature · 1 line
1.7.10.4
#10 lore raw

Re: [PATCH 09/33] ARM: ux500: Supply the I2C clocks lookup to the DBX500 DT

arch/arm/boot/dts/dbx5x0.dtsi line 576

Quoted 7 lines · Lee Jones

+++ b/arch/arm/boot/dts/dbx5x0.dtsi

arch/arm/boot/dts/dbx5x0.dtsi +2 -0
@@ -572,6 +572,8 @@
                        v-i2c-supply = <&db8500_vape_reg>;
 
                        clock-frequency = <400000>;
+                       clocks = <&prcc_kclk 3 3>, <&prcc_pclk 3 3>;
+                       clock-names = "nmk-i2c.0", "apb_pclk";

To avoid confusing the clock name "nmk-i2c.0" with the device name in Linux of that device instance, can we use a name such that it is clear that this is not a dev_name match?

"i2c0" works just fine as name I think?

Linus

#11 lore raw

Re: [PATCH 09/33] ARM: ux500: Supply the I2C clocks lookup to the DBX500 DT

Quoted 9 lines · Linus Walleij

arch/arm/boot/dts/dbx5x0.dtsi line 576

Quoted 7 lines · Lee Jones

+++ b/arch/arm/boot/dts/dbx5x0.dtsi

arch/arm/boot/dts/dbx5x0.dtsi +2 -0
@@ -572,6 +572,8 @@
                        v-i2c-supply = <&db8500_vape_reg>;
 
                        clock-frequency = <400000>;
+                       clocks = <&prcc_kclk 3 3>, <&prcc_pclk 3 3>;
+                       clock-names = "nmk-i2c.0", "apb_pclk";

Why do most clocks in this series have the instance number in the clock names? This looks very wrong to me.

Sascha

Signature · 4 lines
Pengutronix e.K.                           |                             |
Industrial Linux Solutions                 | http://www.pengutronix.de/  |
Peiner Str. 6-8, 31137 Hildesheim, Germany | Phone: +49-5121-206917-0    |
Amtsgericht Hildesheim, HRA 2686           | Fax:   +49-5121-206917-5555 |
#12 lore raw

Re: [PATCH 09/33] ARM: ux500: Supply the I2C clocks lookup to the DBX500 DT

Quoted 7 lines · Sascha Hauer
Quoted 6 lines · Linus Walleij
Quoted 4 lines · Lee Jones

+++ b/arch/arm/boot/dts/dbx5x0.dtsi

(quoted diff) +0 -0
@@ -572,6 +572,8 @@
                        v-i2c-supply = <&db8500_vape_reg>;
 
Quoted 6 lines
Quoted 3 lines

arch/arm/boot/dts/dbx5x0.dtsi line 576

Quoted 3 lines
arch/arm/boot/dts/dbx5x0.dtsi +2 -0
                        clock-frequency = <400000>;
+                       clocks = <&prcc_kclk 3 3>, <&prcc_pclk 3 3>;
+                       clock-names = "nmk-i2c.0", "apb_pclk";

Why do most clocks in this series have the instance number in the clock names? This looks very wrong to me.

+1. The clock names should be the input names to the unit, they shouldn't vary per instance.

Mark.

author
#13 lore raw

Re: [PATCH 09/33] ARM: ux500: Supply the I2C clocks lookup to the DBX500 DT

Quoted 15 lines · Mark Rutland
Quoted 7 lines · Sascha Hauer
Quoted 6 lines · Linus Walleij
Quoted 4 lines · Lee Jones

+++ b/arch/arm/boot/dts/dbx5x0.dtsi

(quoted diff) +0 -0
@@ -572,6 +572,8 @@
                        v-i2c-supply = <&db8500_vape_reg>;
 
Quoted 6 lines
Quoted 3 lines

arch/arm/boot/dts/dbx5x0.dtsi line 576

Quoted 3 lines
arch/arm/boot/dts/dbx5x0.dtsi +2 -0
                        clock-frequency = <400000>;
+                       clocks = <&prcc_kclk 3 3>, <&prcc_pclk 3 3>;
+                       clock-names = "nmk-i2c.0", "apb_pclk";

Why do most clocks in this series have the instance number in the clock names? This looks very wrong to me.

Quoted 2 lines

+1. The clock names should be the input names to the unit, they shouldn't vary per instance.

Duly noted.

Signature · 4 lines
Lee Jones
Linaro ST-Ericsson Landing Team Lead
Linaro.org ? Open source software for ARM SoCs
Follow Linaro: Facebook | Twitter | Blog
author
#14 lore raw

Re: [PATCH 09/33] ARM: ux500: Supply the I2C clocks lookup to the DBX500 DT

In reply to Mark Rutland
Quoted 15 lines · Mark Rutland
Quoted 7 lines · Sascha Hauer
Quoted 6 lines · Linus Walleij
Quoted 4 lines · Lee Jones

+++ b/arch/arm/boot/dts/dbx5x0.dtsi

(quoted diff) +0 -0
@@ -572,6 +572,8 @@
                        v-i2c-supply = <&db8500_vape_reg>;
 
Quoted 6 lines
Quoted 3 lines

arch/arm/boot/dts/dbx5x0.dtsi line 576

Quoted 3 lines
arch/arm/boot/dts/dbx5x0.dtsi +2 -0
                        clock-frequency = <400000>;
+                       clocks = <&prcc_kclk 3 3>, <&prcc_pclk 3 3>;
+                       clock-names = "nmk-i2c.0", "apb_pclk";

Why do most clocks in this series have the instance number in the clock names? This looks very wrong to me.

Quoted 2 lines

+1. The clock names should be the input names to the unit, they shouldn't vary per instance.

So I just had a quick look, and it looks like they each have their own clock:

	clk = clk_reg_prcc_kclk("p1_i2c1_kclk", "i2cclk",
			clkrst1_base, BIT(2), CLK_SET_RATE_GATE);
	clk = clk_reg_prcc_kclk("p1_i2c2_kclk", "i2cclk",
			clkrst1_base, BIT(6), CLK_SET_RATE_GATE);
	clk = clk_reg_prcc_kclk("p2_i2c3_kclk", "i2cclk",
			clkrst2_base, BIT(0), CLK_SET_RATE_GATE);
	clk_register_clkdev(clk, NULL, "nmk-i2c.3");
        
        /* etc */

When using the names in Device Tree it doesn't actually matter what you call the first clock. You can call it "fred" if you wanted and it would still work, but in light of the naming conventions above and the fact that each clock can all be controlled independently, do we still want to use the name of the parent clock i.e. i2cclk?

Signature · 4 lines
Lee Jones
Linaro ST-Ericsson Landing Team Lead
Linaro.org ? Open source software for ARM SoCs
Follow Linaro: Facebook | Twitter | Blog
#15 lore raw

Re: [PATCH 09/33] ARM: ux500: Supply the I2C clocks lookup to the DBX500 DT

Quoted 33 lines · Lee Jones
Quoted 15 lines · Mark Rutland
Quoted 7 lines · Sascha Hauer
Quoted 6 lines · Linus Walleij
Quoted 4 lines · Lee Jones

+++ b/arch/arm/boot/dts/dbx5x0.dtsi

(quoted diff) +0 -0
@@ -572,6 +572,8 @@
                        v-i2c-supply = <&db8500_vape_reg>;
 
Quoted 6 lines
Quoted 3 lines

arch/arm/boot/dts/dbx5x0.dtsi line 576

Quoted 3 lines
arch/arm/boot/dts/dbx5x0.dtsi +2 -0
                        clock-frequency = <400000>;
+                       clocks = <&prcc_kclk 3 3>, <&prcc_pclk 3 3>;
+                       clock-names = "nmk-i2c.0", "apb_pclk";

Why do most clocks in this series have the instance number in the clock names? This looks very wrong to me.

Quoted 2 lines

+1. The clock names should be the input names to the unit, they shouldn't vary per instance.

So I just had a quick look, and it looks like they each have their own clock:

	clk = clk_reg_prcc_kclk("p1_i2c1_kclk", "i2cclk",
			clkrst1_base, BIT(2), CLK_SET_RATE_GATE);
	clk = clk_reg_prcc_kclk("p1_i2c2_kclk", "i2cclk",
			clkrst1_base, BIT(6), CLK_SET_RATE_GATE);
	clk = clk_reg_prcc_kclk("p2_i2c3_kclk", "i2cclk",
			clkrst2_base, BIT(0), CLK_SET_RATE_GATE);
	clk_register_clkdev(clk, NULL, "nmk-i2c.3");
        
        /* etc */
Quoted 5 lines

When using the names in Device Tree it doesn't actually matter what you call the first clock. You can call it "fred" if you wanted and it would still work, but in light of the naming conventions above and the fact that each clock can all be controlled independently, do we still want to use the name of the parent clock i.e. i2cclk?

Sorry, I don't follow.

The name should be the name of the clock _input_ on the block described, as should be listed in documentation for the i2c block. The name should not vary with instance, and the name should not (necessarily) match the _output_ of the provider. Surely there's documentation for the i2c block that gives a name for the clock input(s)?

On a related note, I see that this doesn't follow the primecell clock bindings, which seem to rely on "apb_pclk" being first in the list. I see that other primecell device bindings don't follow that in dts or drivers, so I'm not sure how to fix that brokenness.

Thanks,
Mark.

author
#16 lore raw

Re: [PATCH 09/33] ARM: ux500: Supply the I2C clocks lookup to the DBX500 DT

Quoted 40 lines · Mark Rutland
Quoted 33 lines · Lee Jones
Quoted 15 lines · Mark Rutland
Quoted 7 lines · Sascha Hauer
Quoted 6 lines · Linus Walleij
Quoted 4 lines · Lee Jones

+++ b/arch/arm/boot/dts/dbx5x0.dtsi

(quoted diff) +0 -0
@@ -572,6 +572,8 @@
                        v-i2c-supply = <&db8500_vape_reg>;
 
Quoted 6 lines
Quoted 3 lines

arch/arm/boot/dts/dbx5x0.dtsi line 576

Quoted 3 lines
arch/arm/boot/dts/dbx5x0.dtsi +2 -0
                        clock-frequency = <400000>;
+                       clocks = <&prcc_kclk 3 3>, <&prcc_pclk 3 3>;
+                       clock-names = "nmk-i2c.0", "apb_pclk";

Why do most clocks in this series have the instance number in the clock names? This looks very wrong to me.

Quoted 2 lines

+1. The clock names should be the input names to the unit, they shouldn't vary per instance.

So I just had a quick look, and it looks like they each have their own clock:

	clk = clk_reg_prcc_kclk("p1_i2c1_kclk", "i2cclk",
			clkrst1_base, BIT(2), CLK_SET_RATE_GATE);
	clk = clk_reg_prcc_kclk("p1_i2c2_kclk", "i2cclk",
			clkrst1_base, BIT(6), CLK_SET_RATE_GATE);
	clk = clk_reg_prcc_kclk("p2_i2c3_kclk", "i2cclk",
			clkrst2_base, BIT(0), CLK_SET_RATE_GATE);
	clk_register_clkdev(clk, NULL, "nmk-i2c.3");
        
        /* etc */
Quoted 5 lines

When using the names in Device Tree it doesn't actually matter what you call the first clock. You can call it "fred" if you wanted and it would still work, but in light of the naming conventions above and the fact that each clock can all be controlled independently, do we still want to use the name of the parent clock i.e. i2cclk?

Quoted 8 lines

Sorry, I don't follow.

The name should be the name of the clock _input_ on the block described, as should be listed in documentation for the i2c block. The name should not vary with instance, and the name should not (necessarily) match the _output_ of the provider. Surely there's documentation for the i2c block that gives a name for the clock input(s)?

It's okay, I've fixed the patches.

Linus, branch fixed-up and pushed.

Quoted 4 lines

On a related note, I see that this doesn't follow the primecell clock bindings, which seem to rely on "apb_pclk" being first in the list. I see that other primecell device bindings don't follow that in dts or drivers, so I'm not sure how to fix that brokenness.

To which bindings do you refer? After taking a *quick* look, I see it being the other way around. Whenever the "apb_pclk" is requested, it is done so by name:

  drivers/amba/bus.c: struct clk *pclk = clk_get(&pcdev->dev, "apb_pclk");

So when clk_get() calls of_clk_get_by_name(), the clock-name index is returned correctly by of_property_match_string(), which then passes that index through of_clk_get() and does the right thing.

When most of the other clocks that we deal with are being requested, they rely on being index zero:

  drivers/i2c/busses/i2c-nomadik.c: dev->clk = clk_get(&adev->dev, NULL);

At the moment this works for us, so I don't think we need to go around populating the name params, but we might have to if this falls apart in some way (probably likely if you 'fixed' whatever brokenness you're wanting to fix ;-) )

Signature · 4 lines
Lee Jones
Linaro ST-Ericsson Landing Team Lead
Linaro.org ? Open source software for ARM SoCs
Follow Linaro: Facebook | Twitter | Blog
#17 lore raw

Re: [PATCH 09/33] ARM: ux500: Supply the I2C clocks lookup to the DBX500 DT

Quoted 45 lines · Lee Jones
Quoted 43 lines · Mark Rutland
Quoted 33 lines · Lee Jones
Quoted 15 lines · Mark Rutland
Quoted 7 lines · Sascha Hauer
Quoted 6 lines · Linus Walleij
Quoted 4 lines · Lee Jones

+++ b/arch/arm/boot/dts/dbx5x0.dtsi

(quoted diff) +0 -0
@@ -572,6 +572,8 @@
                        v-i2c-supply = <&db8500_vape_reg>;
 
Quoted 6 lines
Quoted 3 lines

Review line 576

Quoted 3 lines
(quoted diff) +2 -0
                        clock-frequency = <400000>;
+                       clocks = <&prcc_kclk 3 3>, <&prcc_pclk 3 3>;
+                       clock-names = "nmk-i2c.0", "apb_pclk";

Why do most clocks in this series have the instance number in the clock names? This looks very wrong to me.

Quoted 2 lines

+1. The clock names should be the input names to the unit, they shouldn't vary per instance.

So I just had a quick look, and it looks like they each have their own clock:

	clk = clk_reg_prcc_kclk("p1_i2c1_kclk", "i2cclk",
			clkrst1_base, BIT(2), CLK_SET_RATE_GATE);
	clk = clk_reg_prcc_kclk("p1_i2c2_kclk", "i2cclk",
			clkrst1_base, BIT(6), CLK_SET_RATE_GATE);
	clk = clk_reg_prcc_kclk("p2_i2c3_kclk", "i2cclk",
			clkrst2_base, BIT(0), CLK_SET_RATE_GATE);
	clk_register_clkdev(clk, NULL, "nmk-i2c.3");
        
        /* etc */
Quoted 5 lines

When using the names in Device Tree it doesn't actually matter what you call the first clock. You can call it "fred" if you wanted and it would still work, but in light of the naming conventions above and the fact that each clock can all be controlled independently, do we still want to use the name of the parent clock i.e. i2cclk?

Sorry, I don't follow.

Quoted 7 lines
Quoted 5 lines

The name should be the name of the clock _input_ on the block described, as should be listed in documentation for the i2c block. The name should not vary with instance, and the name should not (necessarily) match the _output_ of the provider. Surely there's documentation for the i2c block that gives a name for the clock input(s)?

It's okay, I've fixed the patches.

Ok.

Quoted 21 lines

Linus, branch fixed-up and pushed.

Quoted 4 lines

On a related note, I see that this doesn't follow the primecell clock bindings, which seem to rely on "apb_pclk" being first in the list. I see that other primecell device bindings don't follow that in dts or drivers, so I'm not sure how to fix that brokenness.

To which bindings do you refer? After taking a *quick* look, I see it being the other way around. Whenever the "apb_pclk" is requested, it is done so by name:

  drivers/amba/bus.c: struct clk *pclk = clk_get(&pcdev->dev, "apb_pclk");

So when clk_get() calls of_clk_get_by_name(), the clock-name index is returned correctly by of_property_match_string(), which then passes that index through of_clk_get() and does the right thing.

When most of the other clocks that we deal with are being requested, they rely on being index zero:

Quoted 6 lines
  drivers/i2c/busses/i2c-nomadik.c: dev->clk = clk_get(&adev->dev, NULL);

At the moment this works for us, so I don't think we need to go around populating the name params, but we might have to if this falls apart in some way (probably likely if you 'fixed' whatever brokenness you're wanting to fix ;-) )

The brokenness here is that the binding document states one thing, and drivers do precisely the opposite.

Documentation/devicetree/bindings/arm/primecell.txt states:

- clocks : From common clock binding. First clock is phandle to clock for apb pclk. Additional clocks are optional and specific to those peripherals.
- clock-names : From common clock binding. Shall be "apb_pclk" for first clock.

Yet as you've pointed out above we don't care which index apb_pblk is, but do for the other clock, which we get at index 0 (where the binding doc states apb_pclk should be).

If all users are doing that currently, it's possible to fix up the binding document. If not, then it's a mess that can't be corrected...

Thanks,
Mark.

#18 lore raw

Re: [PATCH 09/33] ARM: ux500: Supply the I2C clocks lookup to the DBX500 DT

In reply to Lee Jones
Quoted 5 lines · Lee Jones

When most of the other clocks that we deal with are being requested, they rely on being index zero:

  drivers/i2c/busses/i2c-nomadik.c: dev->clk = clk_get(&adev->dev, NULL);

Once a device has more than a single clock all clocks should have a connection id. clk_get(dev, NULL) should only be used for single clock devices. Look at drivers/clk/clkdev.c, there's some fuzzy matching involved when you pass NULL as connection id.

Sascha

Signature · 4 lines
Pengutronix e.K.                           |                             |
Industrial Linux Solutions                 | http://www.pengutronix.de/  |
Peiner Str. 6-8, 31137 Hildesheim, Germany | Phone: +49-5121-206917-0    |
Amtsgericht Hildesheim, HRA 2686           | Fax:   +49-5121-206917-5555 |
author
#19 lore raw

Re: [PATCH 09/33] ARM: ux500: Supply the I2C clocks lookup to the DBX500 DT

I had a short chat with Rob last night about this. I'm going to loop him in to the conversation, as he wrote the binding.

Quoted 7 lines
Quoted 4 lines

When most of the other clocks that we deal with are being requested, they rely on being index zero:

  drivers/i2c/busses/i2c-nomadik.c: dev->clk = clk_get(&adev->dev, NULL);

Look at drivers/clk/clkdev.c, there's some fuzzy matching involved when you pass NULL as connection id.

Yes, I've been looking at that. This is why it works currently. I think I need to change all of the drivers to specify which clock they want. At the moment that 'fuzzy matching' is what's saving us. If anyone were to change our DTS file to match what the binding says, then it would cease to work. I'm guessing this is the same for all other DTS files too:

arch/arm/boot/dts/imx23.dtsi:                clock-names = "uart", "apb_pclk";
arch/arm/boot/dts/imx28.dtsi:                clock-names = "uart", "apb_pclk";
arch/arm/boot/dts/nspire-cx.dts:             clock-names = "uart_clk", "apb_pclk";
arch/arm/boot/dts/ste-nomadik-stn8815.dtsi:  clock-names = "timclk", "apb_pclk";
arch/arm/boot/dts/ste-nomadik-stn8815.dtsi:  clock-names = "timclk", "apb_pclk";
arch/arm/boot/dts/ste-nomadik-stn8815.dtsi:  clock-names = "uartclk", "apb_pclk";
arch/arm/boot/dts/ste-nomadik-stn8815.dtsi:  clock-names = "uartclk", "apb_pclk";
arch/arm/boot/dts/ste-nomadik-stn8815.dtsi:  clock-names = "uartclk", "apb_pclk";
arch/arm/boot/dts/ste-nomadik-stn8815.dtsi:  clock-names = "rng", "apb_pclk";
arch/arm/boot/dts/ste-nomadik-stn8815.dtsi:  clock-names = "mclk", "apb_pclk";
arch/arm/boot/dts/vexpress-v2m-rs1.dtsi:     clock-names = "refclk", "timclk", "apb_pclk";
arch/arm/boot/dts/vexpress-v2m-rs1.dtsi:     clock-names = "mclk", "apb_pclk";
arch/arm/boot/dts/vexpress-v2m-rs1.dtsi:     clock-names = "KMIREFCLK", "apb_pclk";
arch/arm/boot/dts/vexpress-v2m-rs1.dtsi:     clock-names = "KMIREFCLK", "apb_pclk";
arch/arm/boot/dts/vexpress-v2m-rs1.dtsi:     clock-names = "uartclk", "apb_pclk";
arch/arm/boot/dts/vexpress-v2m-rs1.dtsi:     clock-names = "uartclk", "apb_pclk";
arch/arm/boot/dts/vexpress-v2m-rs1.dtsi:     clock-names = "uartclk", "apb_pclk";
arch/arm/boot/dts/vexpress-v2m-rs1.dtsi:     clock-names = "uartclk", "apb_pclk";
arch/arm/boot/dts/vexpress-v2m-rs1.dtsi:     clock-names = "wdogclk", "apb_pclk";
arch/arm/boot/dts/vexpress-v2m-rs1.dtsi:     clock-names = "timclken1", "timclken2", "apb_pclk";
arch/arm/boot/dts/vexpress-v2m-rs1.dtsi:     clock-names = "timclken1", "timclken2", "apb_pclk";
arch/arm/boot/dts/vexpress-v2m-rs1.dtsi:     clock-names = "clcdclk", "apb_pclk";
arch/arm/boot/dts/vexpress-v2m.dtsi:         clock-names = "refclk", "timclk", "apb_pclk";
arch/arm/boot/dts/vexpress-v2m.dtsi:         clock-names = "mclk", "apb_pclk";
arch/arm/boot/dts/vexpress-v2m.dtsi:         clock-names = "KMIREFCLK", "apb_pclk";
arch/arm/boot/dts/vexpress-v2m.dtsi:         clock-names = "KMIREFCLK", "apb_pclk";
arch/arm/boot/dts/vexpress-v2m.dtsi:         clock-names = "uartclk", "apb_pclk";
arch/arm/boot/dts/vexpress-v2m.dtsi:         clock-names = "uartclk", "apb_pclk";
arch/arm/boot/dts/vexpress-v2m.dtsi:         clock-names = "uartclk", "apb_pclk";
arch/arm/boot/dts/vexpress-v2m.dtsi:         clock-names = "uartclk", "apb_pclk";
arch/arm/boot/dts/vexpress-v2m.dtsi:         clock-names = "wdogclk", "apb_pclk";
arch/arm/boot/dts/vexpress-v2m.dtsi:         clock-names = "timclken1", "timclken2", "apb_pclk";
arch/arm/boot/dts/vexpress-v2m.dtsi:         clock-names = "timclken1", "timclken2", "apb_pclk";
arch/arm/boot/dts/vexpress-v2m.dtsi:         clock-names = "clcdclk", "apb_pclk";
arch/arm/boot/dts/vexpress-v2p-ca15_a7.dts:  clock-names = "wdogclk", "apb_pclk";
arch/arm/boot/dts/vexpress-v2p-ca9.dts:      clock-names = "clcdclk", "apb_pclk";
arch/arm/boot/dts/vexpress-v2p-ca9.dts:      clock-names = "timclk", "apb_pclk";
arch/arm/boot/dts/vexpress-v2p-ca9.dts:      clock-names = "wdogclk", "apb_pclk";
Signature · 4 lines
Lee Jones
Linaro ST-Ericsson Landing Team Lead
Linaro.org ? Open source software for ARM SoCs
Follow Linaro: Facebook | Twitter | Blog
#20 lore raw

Re: [PATCH 09/33] ARM: ux500: Supply the I2C clocks lookup to the DBX500 DT

Quoted 11 lines · Lee Jones

I had a short chat with Rob last night about this. I'm going to loop him in to the conversation, as he wrote the binding.

Quoted 7 lines
Quoted 4 lines

When most of the other clocks that we deal with are being requested, they rely on being index zero:

  drivers/i2c/busses/i2c-nomadik.c: dev->clk = clk_get(&adev->dev, NULL);

Look at drivers/clk/clkdev.c, there's some fuzzy matching involved when you pass NULL as connection id.

Quoted 6 lines

Yes, I've been looking at that. This is why it works currently. I think I need to change all of the drivers to specify which clock they want. At the moment that 'fuzzy matching' is what's saving us. If anyone were to change our DTS file to match what the binding says, then it would cease to work. I'm guessing this is the same for all other DTS files too:

I think if anything, the binding document(s) should be updated to describe that apb_pclk is referred to by name, and the names of the other clocks should be described in the specific device bindings. We can then modify the drivers which grab clock 0 to explicitly grab the first clock by name, and backwards compatibility should not be broken.

I don't believe any other OS has implemented the common clock bindings, and we've never supported the binding as described. Let's correct the de-facto standard into a standard by decree.

Thanks,
Mark.

Quoted 46 lines
arch/arm/boot/dts/imx23.dtsi:                clock-names = "uart", "apb_pclk";
arch/arm/boot/dts/imx28.dtsi:                clock-names = "uart", "apb_pclk";
arch/arm/boot/dts/nspire-cx.dts:             clock-names = "uart_clk", "apb_pclk";
arch/arm/boot/dts/ste-nomadik-stn8815.dtsi:  clock-names = "timclk", "apb_pclk";
arch/arm/boot/dts/ste-nomadik-stn8815.dtsi:  clock-names = "timclk", "apb_pclk";
arch/arm/boot/dts/ste-nomadik-stn8815.dtsi:  clock-names = "uartclk", "apb_pclk";
arch/arm/boot/dts/ste-nomadik-stn8815.dtsi:  clock-names = "uartclk", "apb_pclk";
arch/arm/boot/dts/ste-nomadik-stn8815.dtsi:  clock-names = "uartclk", "apb_pclk";
arch/arm/boot/dts/ste-nomadik-stn8815.dtsi:  clock-names = "rng", "apb_pclk";
arch/arm/boot/dts/ste-nomadik-stn8815.dtsi:  clock-names = "mclk", "apb_pclk";
arch/arm/boot/dts/vexpress-v2m-rs1.dtsi:     clock-names = "refclk", "timclk", "apb_pclk";
arch/arm/boot/dts/vexpress-v2m-rs1.dtsi:     clock-names = "mclk", "apb_pclk";
arch/arm/boot/dts/vexpress-v2m-rs1.dtsi:     clock-names = "KMIREFCLK", "apb_pclk";
arch/arm/boot/dts/vexpress-v2m-rs1.dtsi:     clock-names = "KMIREFCLK", "apb_pclk";
arch/arm/boot/dts/vexpress-v2m-rs1.dtsi:     clock-names = "uartclk", "apb_pclk";
arch/arm/boot/dts/vexpress-v2m-rs1.dtsi:     clock-names = "uartclk", "apb_pclk";
arch/arm/boot/dts/vexpress-v2m-rs1.dtsi:     clock-names = "uartclk", "apb_pclk";
arch/arm/boot/dts/vexpress-v2m-rs1.dtsi:     clock-names = "uartclk", "apb_pclk";
arch/arm/boot/dts/vexpress-v2m-rs1.dtsi:     clock-names = "wdogclk", "apb_pclk";
arch/arm/boot/dts/vexpress-v2m-rs1.dtsi:     clock-names = "timclken1", "timclken2", "apb_pclk";
arch/arm/boot/dts/vexpress-v2m-rs1.dtsi:     clock-names = "timclken1", "timclken2", "apb_pclk";
arch/arm/boot/dts/vexpress-v2m-rs1.dtsi:     clock-names = "clcdclk", "apb_pclk";
arch/arm/boot/dts/vexpress-v2m.dtsi:         clock-names = "refclk", "timclk", "apb_pclk";
arch/arm/boot/dts/vexpress-v2m.dtsi:         clock-names = "mclk", "apb_pclk";
arch/arm/boot/dts/vexpress-v2m.dtsi:         clock-names = "KMIREFCLK", "apb_pclk";
arch/arm/boot/dts/vexpress-v2m.dtsi:         clock-names = "KMIREFCLK", "apb_pclk";
arch/arm/boot/dts/vexpress-v2m.dtsi:         clock-names = "uartclk", "apb_pclk";
arch/arm/boot/dts/vexpress-v2m.dtsi:         clock-names = "uartclk", "apb_pclk";
arch/arm/boot/dts/vexpress-v2m.dtsi:         clock-names = "uartclk", "apb_pclk";
arch/arm/boot/dts/vexpress-v2m.dtsi:         clock-names = "uartclk", "apb_pclk";
arch/arm/boot/dts/vexpress-v2m.dtsi:         clock-names = "wdogclk", "apb_pclk";
arch/arm/boot/dts/vexpress-v2m.dtsi:         clock-names = "timclken1", "timclken2", "apb_pclk";
arch/arm/boot/dts/vexpress-v2m.dtsi:         clock-names = "timclken1", "timclken2", "apb_pclk";
arch/arm/boot/dts/vexpress-v2m.dtsi:         clock-names = "clcdclk", "apb_pclk";
arch/arm/boot/dts/vexpress-v2p-ca15_a7.dts:  clock-names = "wdogclk", "apb_pclk";
arch/arm/boot/dts/vexpress-v2p-ca9.dts:      clock-names = "clcdclk", "apb_pclk";
arch/arm/boot/dts/vexpress-v2p-ca9.dts:      clock-names = "timclk", "apb_pclk";
arch/arm/boot/dts/vexpress-v2p-ca9.dts:      clock-names = "wdogclk", "apb_pclk";
Signature · 4 lines
Lee Jones
Linaro ST-Ericsson Landing Team Lead
Linaro.org ? Open source software for ARM SoCs
Follow Linaro: Facebook | Twitter | Blog
author
#21 lore raw

Re: [PATCH 09/33] ARM: ux500: Supply the I2C clocks lookup to the DBX500 DT

Quoted 25 lines · Mark Rutland
Quoted 11 lines · Lee Jones

I had a short chat with Rob last night about this. I'm going to loop him in to the conversation, as he wrote the binding.

Quoted 7 lines
Quoted 4 lines

When most of the other clocks that we deal with are being requested, they rely on being index zero:

  drivers/i2c/busses/i2c-nomadik.c: dev->clk = clk_get(&adev->dev, NULL);

Look at drivers/clk/clkdev.c, there's some fuzzy matching involved when you pass NULL as connection id.

Quoted 6 lines

Yes, I've been looking at that. This is why it works currently. I think I need to change all of the drivers to specify which clock they want. At the moment that 'fuzzy matching' is what's saving us. If anyone were to change our DTS file to match what the binding says, then it would cease to work. I'm guessing this is the same for all other DTS files too:

I think if anything, the binding document(s) should be updated to describe that apb_pclk is referred to by name, and the names of the other clocks should be described in the specific device bindings. We can then modify the drivers which grab clock 0 to explicitly grab the first clock by name, and backwards compatibility should not be broken.

Quoted 3 lines

I don't believe any other OS has implemented the common clock bindings, and we've never supported the binding as described. Let's correct the de-facto standard into a standard by decree.

I think we need to respect, or at least take into consideration the reason for the original 'de-facto' standard. Other OSes shouldn't be forced to provide a named clock request in order to obtain 'apb_pclk'. If the binding says it should be first, then perhaps we should do just that. It's simply a matter of naming all subsequent clocks related to AMBA devices.

Signature · 4 lines
Lee Jones
Linaro ST-Ericsson Landing Team Lead
Linaro.org ? Open source software for ARM SoCs
Follow Linaro: Facebook | Twitter | Blog
#22 lore raw

Re: [PATCH 09/33] ARM: ux500: Supply the I2C clocks lookup to the DBX500 DT

Quoted 31 lines · Lee Jones
Quoted 25 lines · Mark Rutland
Quoted 11 lines · Lee Jones

I had a short chat with Rob last night about this. I'm going to loop him in to the conversation, as he wrote the binding.

Quoted 7 lines
Quoted 4 lines

When most of the other clocks that we deal with are being requested, they rely on being index zero:

  drivers/i2c/busses/i2c-nomadik.c: dev->clk = clk_get(&adev->dev, NULL);

Look at drivers/clk/clkdev.c, there's some fuzzy matching involved when you pass NULL as connection id.

Quoted 6 lines

Yes, I've been looking at that. This is why it works currently. I think I need to change all of the drivers to specify which clock they want. At the moment that 'fuzzy matching' is what's saving us. If anyone were to change our DTS file to match what the binding says, then it would cease to work. I'm guessing this is the same for all other DTS files too:

I think if anything, the binding document(s) should be updated to describe that apb_pclk is referred to by name, and the names of the other clocks should be described in the specific device bindings. We can then modify the drivers which grab clock 0 to explicitly grab the first clock by name, and backwards compatibility should not be broken.

Quoted 3 lines

I don't believe any other OS has implemented the common clock bindings, and we've never supported the binding as described. Let's correct the de-facto standard into a standard by decree.

Quoted 6 lines

I think we need to respect, or at least take into consideration the reason for the original 'de-facto' standard. Other OSes shouldn't be forced to provide a named clock request in order to obtain 'apb_pclk'. If the binding says it should be first, then perhaps we should do just that. It's simply a matter of naming all subsequent clocks related to AMBA devices.

Ideally, yes. However, we have to be careful to not break compatibility.

I took a look at existing primecell drivers and what they do. The situation isn't so bad (with the exception of the half-dt/half-platform-code mess):

* Some don't deal with clocks at all (no clk* in the driver). pl320 is in the ecx-common dtsi with only apb_pclk but has no binding defined. Some have no clocks defined in the dt and are presumably few clocks by platform data or are non-functional.

I'm not sure how these DTs are going to be supported if and when we remove the platform data they depend upon. If we're really going to do that, then they are clearly not supported as-is long term.

* The pl022 driver grabs the first clock to figure out the rate of the spi bus (assuming it is SSPCLK). The SSPCLK input is not defined in the binding. The ste-u300 dts has two clock-names, "apb_pclk" and "spi_clk" (in that order), but they refer to the same clock.

Given the existing driver simply grabs the first clock and they're both the same, we could re-order the names and make the driver grab the second clock. That wouldn't break backwards compatibility with the sole dts file we have using the binding, though this assumes no-one else has a dt lying around with different clocks.

* pl010 grabs the first clock given to it to figure out the uart rate (assuming it is UARTCLK), but it's only in integratorap.dts, without clocks, and is presumably fed by platform data. There is no binding document.

pl011 grabs the first clock given to figure out the UART rate (assuming it is UARTCLK). The binding explicitly states it's only given apb_pclk, despite UARTCLK and PCLK being separate inputs to the IP block.

These two bindings don't describe the hardware, and should be fixed. The only way I can think to make this work without breaknig backwards compatibility would be to try to grab the second clock and then fall back to the first if there isn't one. The other option is to break backwards compatibility, but I'm not sure that's much of an option.

* pl111 has no driver or binding in mainline, but appears in dts files. Those dts files clcdclk and apb_pclk, in that order. We could fix those before a driver starts using them.

If you think those suggestions are OK, I can put together a series to fix this.

Thanks,
Mark.

author
#23 lore raw

Re: [PATCH 09/33] ARM: ux500: Supply the I2C clocks lookup to the DBX500 DT

Quoted 81 lines · Mark Rutland
Quoted 31 lines · Lee Jones
Quoted 25 lines · Mark Rutland
Quoted 11 lines · Lee Jones

I had a short chat with Rob last night about this. I'm going to loop him in to the conversation, as he wrote the binding.

Quoted 7 lines
Quoted 4 lines

When most of the other clocks that we deal with are being requested, they rely on being index zero:

  drivers/i2c/busses/i2c-nomadik.c: dev->clk = clk_get(&adev->dev, NULL);

Look at drivers/clk/clkdev.c, there's some fuzzy matching involved when you pass NULL as connection id.

Quoted 6 lines

Yes, I've been looking at that. This is why it works currently. I think I need to change all of the drivers to specify which clock they want. At the moment that 'fuzzy matching' is what's saving us. If anyone were to change our DTS file to match what the binding says, then it would cease to work. I'm guessing this is the same for all other DTS files too:

I think if anything, the binding document(s) should be updated to describe that apb_pclk is referred to by name, and the names of the other clocks should be described in the specific device bindings. We can then modify the drivers which grab clock 0 to explicitly grab the first clock by name, and backwards compatibility should not be broken.

Quoted 3 lines

I don't believe any other OS has implemented the common clock bindings, and we've never supported the binding as described. Let's correct the de-facto standard into a standard by decree.

Quoted 6 lines

I think we need to respect, or at least take into consideration the reason for the original 'de-facto' standard. Other OSes shouldn't be forced to provide a named clock request in order to obtain 'apb_pclk'. If the binding says it should be first, then perhaps we should do just that. It's simply a matter of naming all subsequent clocks related to AMBA devices.

Ideally, yes. However, we have to be careful to not break compatibility.

I took a look at existing primecell drivers and what they do. The situation isn't so bad (with the exception of the half-dt/half-platform-code mess):

* Some don't deal with clocks at all (no clk* in the driver). pl320 is in the ecx-common dtsi with only apb_pclk but has no binding defined. Some have no clocks defined in the dt and are presumably few clocks by platform data or are non-functional.

I'm not sure how these DTs are going to be supported if and when we remove the platform data they depend upon. If we're really going to do that, then they are clearly not supported as-is long term.

* The pl022 driver grabs the first clock to figure out the rate of the spi bus (assuming it is SSPCLK). The SSPCLK input is not defined in the binding. The ste-u300 dts has two clock-names, "apb_pclk" and "spi_clk" (in that order), but they refer to the same clock.

Given the existing driver simply grabs the first clock and they're both the same, we could re-order the names and make the driver grab the second clock. That wouldn't break backwards compatibility with the sole dts file we have using the binding, though this assumes no-one else has a dt lying around with different clocks.

* pl010 grabs the first clock given to it to figure out the uart rate (assuming it is UARTCLK), but it's only in integratorap.dts, without clocks, and is presumably fed by platform data. There is no binding document.

pl011 grabs the first clock given to figure out the UART rate (assuming it is UARTCLK). The binding explicitly states it's only given apb_pclk, despite UARTCLK and PCLK being separate inputs to the IP block.

These two bindings don't describe the hardware, and should be fixed. The only way I can think to make this work without breaknig backwards compatibility would be to try to grab the second clock and then fall back to the first if there isn't one. The other option is to break backwards compatibility, but I'm not sure that's much of an option.

Quoted 6 lines

* pl111 has no driver or binding in mainline, but appears in dts files. Those dts files clcdclk and apb_pclk, in that order. We could fix those before a driver starts using them.

If you think those suggestions are OK, I can put together a series to fix this.

I think we need to hear from Rob before we proceed tbh, as he is the original author and should have a chance to voice his opinion.

Signature · 4 lines
Lee Jones
Linaro ST-Ericsson Landing Team Lead
Linaro.org ? Open source software for ARM SoCs
Follow Linaro: Facebook | Twitter | Blog
#24 lore raw

Re: [PATCH 09/33] ARM: ux500: Supply the I2C clocks lookup to the DBX500 DT

Quoted 77 lines · Lee Jones
Quoted 75 lines · Mark Rutland
Quoted 31 lines · Lee Jones
Quoted 25 lines · Mark Rutland
Quoted 11 lines · Lee Jones

I had a short chat with Rob last night about this. I'm going to loop him in to the conversation, as he wrote the binding.

Quoted 7 lines
Quoted 4 lines

When most of the other clocks that we deal with are being requested, they rely on being index zero:

  drivers/i2c/busses/i2c-nomadik.c: dev->clk = clk_get(&adev->dev, NULL);

Look at drivers/clk/clkdev.c, there's some fuzzy matching involved when you pass NULL as connection id.

Quoted 6 lines

Yes, I've been looking at that. This is why it works currently. I think I need to change all of the drivers to specify which clock they want. At the moment that 'fuzzy matching' is what's saving us. If anyone were to change our DTS file to match what the binding says, then it would cease to work. I'm guessing this is the same for all other DTS files too:

I think if anything, the binding document(s) should be updated to describe that apb_pclk is referred to by name, and the names of the other clocks should be described in the specific device bindings. We can then modify the drivers which grab clock 0 to explicitly grab the first clock by name, and backwards compatibility should not be broken.

Quoted 3 lines

I don't believe any other OS has implemented the common clock bindings, and we've never supported the binding as described. Let's correct the de-facto standard into a standard by decree.

Quoted 6 lines

I think we need to respect, or at least take into consideration the reason for the original 'de-facto' standard. Other OSes shouldn't be forced to provide a named clock request in order to obtain 'apb_pclk'. If the binding says it should be first, then perhaps we should do just that. It's simply a matter of naming all subsequent clocks related to AMBA devices.

Ideally, yes. However, we have to be careful to not break compatibility.

I took a look at existing primecell drivers and what they do. The situation isn't so bad (with the exception of the half-dt/half-platform-code mess):

* Some don't deal with clocks at all (no clk* in the driver). pl320 is in the ecx-common dtsi with only apb_pclk but has no binding defined. Some have no clocks defined in the dt and are presumably few clocks by platform data or are non-functional.

I'm not sure how these DTs are going to be supported if and when we remove the platform data they depend upon. If we're really going to do that, then they are clearly not supported as-is long term.

* The pl022 driver grabs the first clock to figure out the rate of the spi bus (assuming it is SSPCLK). The SSPCLK input is not defined in the binding. The ste-u300 dts has two clock-names, "apb_pclk" and "spi_clk" (in that order), but they refer to the same clock.

Given the existing driver simply grabs the first clock and they're both the same, we could re-order the names and make the driver grab the second clock. That wouldn't break backwards compatibility with the sole dts file we have using the binding, though this assumes no-one else has a dt lying around with different clocks.

* pl010 grabs the first clock given to it to figure out the uart rate (assuming it is UARTCLK), but it's only in integratorap.dts, without clocks, and is presumably fed by platform data. There is no binding document.

pl011 grabs the first clock given to figure out the UART rate (assuming it is UARTCLK). The binding explicitly states it's only given apb_pclk, despite UARTCLK and PCLK being separate inputs to the IP block.

Quoted 5 lines
Quoted 5 lines

These two bindings don't describe the hardware, and should be fixed. The only way I can think to make this work without breaknig backwards compatibility would be to try to grab the second clock and then fall back to the first if there isn't one. The other option is to break backwards compatibility, but I'm not sure that's much of an option.

This was an oversight since highbank has a single clock. But yes, this should be 2 clocks. It should be fixed and in a compatible way please.

Quoted 3 lines
Quoted 3 lines

* pl111 has no driver or binding in mainline, but appears in dts files. Those dts files clcdclk and apb_pclk, in that order. We could fix those before a driver starts using them.

I think this was waiting for some generic display bindings? Pawel may know.

Rob

#25 lore raw

Re: [PATCH 09/33] ARM: ux500: Supply the I2C clocks lookup to the DBX500 DT

Quoted 5 lines · Rob Herring
Quoted 3 lines
Quoted 3 lines

* pl111 has no driver or binding in mainline, but appears in dts files. Those dts files clcdclk and apb_pclk, in that order. We could fix those before a driver starts using them.

I think this was waiting for some generic display bindings? Pawel may know.

Common Display Framework it was ;-)

And yes, it was waiting for it, but it is getting bored with waiting so I'll cut off the CDF bits and post the rest this or next week.

As to the APB clock, we've been there before...
http://article.gmane.org/gmane.linux.ports.arm.kernel/188927

Cheers!

Pawel

author
#26 lore raw

Re: [PATCH 09/33] ARM: ux500: Supply the I2C clocks lookup to the DBX500 DT

In reply to Linus Walleij
Quoted 14 lines · Linus Walleij

arch/arm/boot/dts/dbx5x0.dtsi line 576

Quoted 7 lines · Lee Jones

+++ b/arch/arm/boot/dts/dbx5x0.dtsi

arch/arm/boot/dts/dbx5x0.dtsi +2 -0
@@ -572,6 +572,8 @@
                        v-i2c-supply = <&db8500_vape_reg>;
 
                        clock-frequency = <400000>;
+                       clocks = <&prcc_kclk 3 3>, <&prcc_pclk 3 3>;
+                       clock-names = "nmk-i2c.0", "apb_pclk";

To avoid confusing the clock name "nmk-i2c.0" with the device name in Linux of that device instance, can we use a name such that it is clear that this is not a dev_name match?

Quoted 1 line

"i2c0" works just fine as name I think?

If you do that, then I think you need to change all of these too:

git grep -e "\.0" -e "\.1" -e "\.2" -e "\.3" -- drivers/clk/ux500/

Signature · 4 lines
Lee Jones
Linaro ST-Ericsson Landing Team Lead
Linaro.org ? Open source software for ARM SoCs
Follow Linaro: Facebook | Twitter | Blog
#27 lore raw

Re: [PATCH 09/33] ARM: ux500: Supply the I2C clocks lookup to the DBX500 DT

Quoted 15 lines · Lee Jones
Quoted 14 lines · Linus Walleij

arch/arm/boot/dts/dbx5x0.dtsi line 576

Quoted 7 lines · Lee Jones

+++ b/arch/arm/boot/dts/dbx5x0.dtsi

arch/arm/boot/dts/dbx5x0.dtsi +2 -0
@@ -572,6 +572,8 @@
                        v-i2c-supply = <&db8500_vape_reg>;
 
                        clock-frequency = <400000>;
+                       clocks = <&prcc_kclk 3 3>, <&prcc_pclk 3 3>;
+                       clock-names = "nmk-i2c.0", "apb_pclk";

To avoid confusing the clock name "nmk-i2c.0" with the device name in Linux of that device instance, can we use a name such that it is clear that this is not a dev_name match?

Quoted 5 lines
Quoted 1 line

"i2c0" works just fine as name I think?

If you do that, then I think you need to change all of these too:

git grep -e "\.0" -e "\.1" -e "\.2" -e "\.3" -- drivers/clk/ux500/

No, as I said in some other patch, all these clk_register_clkdev()s are unused in device tree boots and should not even be executed on the DT boot path.

(But maybe you've proven me wrong there in this other thread ...)

Yours,
Linus Walleij

author
#28 lore raw

Re: [PATCH 09/33] ARM: ux500: Supply the I2C clocks lookup to the DBX500 DT

In reply to Linus Walleij
Quoted 6 lines · Linus Walleij
Quoted 4 lines · Lee Jones

+++ b/arch/arm/boot/dts/dbx5x0.dtsi

(quoted diff) +0 -0
@@ -572,6 +572,8 @@
                        v-i2c-supply = <&db8500_vape_reg>;
 
Quoted 7 lines

arch/arm/boot/dts/dbx5x0.dtsi line 576

Quoted 3 lines
arch/arm/boot/dts/dbx5x0.dtsi +2 -0
                        clock-frequency = <400000>;
+                       clocks = <&prcc_kclk 3 3>, <&prcc_pclk 3 3>;
+                       clock-names = "nmk-i2c.0", "apb_pclk";

To avoid confusing the clock name "nmk-i2c.0" with the device name in Linux of that device instance, can we use a name such that it is clear that this is not a dev_name match?

Once we've settled the other thread, I'll look into renaming this something less Linuxy.

Quoted 1 line

"i2c0" works just fine as name I think?

Sure.

Signature · 4 lines
Lee Jones
Linaro ST-Ericsson Landing Team Lead
Linaro.org ? Open source software for ARM SoCs
Follow Linaro: Facebook | Twitter | Blog
author
#29 lore raw

PATCH 10/33 ARM: ux500: Supply the UART clocks lookup to the DBX500 DT

In reply to Lee Jones
---
 arch/arm/boot/dts/dbx5x0.dtsi |    9 +++++++++
 1 file changed, 9 insertions(+)
arch/arm/boot/dts/dbx5x0.dtsi +9 -0
index e49c679..8cfe5f1 100644
@@ -658,6 +658,9 @@
 			       <&dma 13 0 0x0>; /* Logical - MemToDev */
 			dma-names = "rx", "tx";
 
+			clocks = <&prcc_kclk 1 0>, <&prcc_pclk 1 0>;
+			clock-names = "uart0", "apb_pclk";
+
 			status = "disabled";
 		};
 
@@ -670,6 +673,9 @@
 			       <&dma 12 0 0x0>; /* Logical - MemToDev */
 			dma-names = "rx", "tx";
 
+			clocks = <&prcc_kclk 1 1>, <&prcc_pclk 1 1>;
+			clock-names = "uart1", "apb_pclk";
+
 			status = "disabled";
 		};
 
@@ -682,6 +688,9 @@
 			       <&dma 11 0 0x0>; /* Logical - MemToDev */
 			dma-names = "rx", "tx";
 
+			clocks = <&prcc_kclk 3 6>, <&prcc_pclk 3 6>;
+			clock-names = "uart2", "apb_pclk";
+
 			status = "disabled";
 		};
 
Signature · 1 line
1.7.10.4
author
#30 lore raw

PATCH 11/33 ARM: ux500: Supply the SDI (MMC) clocks lookup to the DBX500 DT

In reply to Lee Jones
---
 arch/arm/boot/dts/dbx5x0.dtsi |   20 ++++++++++++++++++++
 1 file changed, 20 insertions(+)
arch/arm/boot/dts/dbx5x0.dtsi +20 -0
index 8cfe5f1..6b9c3a4 100644
@@ -703,6 +703,9 @@
 			       <&dma 29 0 0x0>; /* Logical - MemToDev */
 			dma-names = "rx", "tx";
 
+			clocks = <&prcc_kclk 1 5>, <&prcc_pclk 1 5>;
+			clock-names = "sdi0", "apb_pclk";
+
 			status = "disabled";
 		};
 
@@ -715,6 +718,9 @@
 			       <&dma 32 0 0x0>; /* Logical - MemToDev */
 			dma-names = "rx", "tx";
 
+			clocks = <&prcc_kclk 2 4>, <&prcc_pclk 2 6>;
+			clock-names = "sdi1", "apb_pclk";
+
 			status = "disabled";
 		};
 
@@ -727,6 +733,9 @@
 			       <&dma 28 0 0x0>; /* Logical - MemToDev */
 			dma-names = "rx", "tx";
 
+			clocks = <&prcc_kclk 3 4>, <&prcc_pclk 3 4>;
+			clock-names = "sdi2", "apb_pclk";
+
 			status = "disabled";
 		};
 
@@ -734,6 +743,10 @@
 			compatible = "arm,pl18x", "arm,primecell";
 			reg = <0x80119000 0x1000>;
 			interrupts = <0 59 IRQ_TYPE_LEVEL_HIGH>;
+
+			clocks = <&prcc_kclk 2 5>, <&prcc_pclk 2 7>;
+			clock-names = "sdi3", "apb_pclk";
+
 			status = "disabled";
 		};
 
@@ -746,6 +759,9 @@
 			       <&dma 42 0 0x0>; /* Logical - MemToDev */
 			dma-names = "rx", "tx";
 
+			clocks = <&prcc_kclk 2 2>, <&prcc_pclk 2 4>;
+			clock-names = "sdi4", "apb_pclk";
+
 			status = "disabled";
 		};
 
@@ -753,6 +769,10 @@
 			compatible = "arm,pl18x", "arm,primecell";
 			reg = <0x80008000 0x1000>;
 			interrupts = <0 100 IRQ_TYPE_LEVEL_HIGH>;
+
+			clocks = <&prcc_kclk 3 7>, <&prcc_pclk 3 7>;
+			clock-names = "sdi5", "apb_pclk";
+
 			status = "disabled";
 		};
 
Signature · 1 line
1.7.10.4
author
#31 lore raw

PATCH 12/33 ARM: ux500: Supply the MSP (Audio) clocks lookup to the DBX500 DT

In reply to Lee Jones
---
 arch/arm/boot/dts/dbx5x0.dtsi |   16 ++++++++++++++++
 1 file changed, 16 insertions(+)
arch/arm/boot/dts/dbx5x0.dtsi +16 -0
index 6b9c3a4..e4ce16b 100644
@@ -781,6 +781,10 @@
 			reg = <0x80123000 0x1000>;
 			interrupts = <0 31 IRQ_TYPE_LEVEL_HIGH>;
 			v-ape-supply = <&db8500_vape_reg>;
+
+			clocks = <&prcc_kclk 1 3>, <&prcc_pclk 1 3>;
+			clock-names = "msp0", "apb_pclk";
+
 			status = "disabled";
 		};
 
@@ -789,6 +793,10 @@
 			reg = <0x80124000 0x1000>;
 			interrupts = <0 62 IRQ_TYPE_LEVEL_HIGH>;
 			v-ape-supply = <&db8500_vape_reg>;
+
+			clocks = <&prcc_kclk 1 4>, <&prcc_pclk 1 4>;
+			clock-names = "msp1", "apb_pclk";
+
 			status = "disabled";
 		};
 
@@ -798,6 +806,10 @@
 			reg = <0x80117000 0x1000>;
 			interrupts = <0 98 IRQ_TYPE_LEVEL_HIGH>;
 			v-ape-supply = <&db8500_vape_reg>;
+
+			clocks = <&prcc_kclk 2 3>, <&prcc_pclk 2 5>;
+			clock-names = "msp2", "apb_pclk";
+
 			status = "disabled";
 		};
 
@@ -806,6 +818,10 @@
 			reg = <0x80125000 0x1000>;
 			interrupts = <0 62 IRQ_TYPE_LEVEL_HIGH>;
 			v-ape-supply = <&db8500_vape_reg>;
+
+			clocks = <&prcc_kclk 1 10>, <&prcc_pclk 1 11>;
+			clock-names = "msp3", "apb_pclk";
+
 			status = "disabled";
 		};
 
Signature · 1 line
1.7.10.4
author
#32 lore raw

PATCH 13/33 ARM: ux500: Add RTC (fixed-frequency) clock node to DBx500 Device Tree

In reply to Lee Jones
---
 arch/arm/boot/dts/dbx5x0.dtsi |    4 ++++
 1 file changed, 4 insertions(+)
arch/arm/boot/dts/dbx5x0.dtsi +4 -0
index e4ce16b..a677521 100644
@@ -57,6 +57,10 @@
 			prcc_kclk: prcc-kernel-clock {
 				#clock-cells = <2>;
 			};
+
+			rtc_clk: rtc32k-clock {
+				#clock-cells = <0>;
+			};
 		};
 
 		timer at a0410600 {
Signature · 1 line
1.7.10.4
author
#33 lore raw

PATCH 14/33 ARM: ux500: Supply the RTC clock lookup to the DBX500 DT

In reply to Lee Jones
---
 arch/arm/boot/dts/dbx5x0.dtsi |    3 +++
 1 file changed, 3 insertions(+)
arch/arm/boot/dts/dbx5x0.dtsi +3 -0
index a677521..edb53c63 100644
@@ -73,6 +73,9 @@
 			compatible = "arm,rtc-pl031", "arm,primecell";
 			reg = <0x80154000 0x1000>;
 			interrupts = <0 18 IRQ_TYPE_LEVEL_HIGH>;
+
+			clocks = <&rtc_clk>;
+			clock-names = "apb_pclk";
 		};
 
 		gpio0: gpio at 8012e000 {
Signature · 1 line
1.7.10.4
author
#34 lore raw

PATCH 15/33 ARM: ux500: Add TWD (fixed-factor) clock node to DBx500 Device Tree

In reply to Lee Jones
---
 arch/arm/boot/dts/dbx5x0.dtsi |    4 ++++
 1 file changed, 4 insertions(+)
arch/arm/boot/dts/dbx5x0.dtsi +4 -0
index edb53c63..2027929 100644
@@ -61,6 +61,10 @@
 			rtc_clk: rtc32k-clock {
 				#clock-cells = <0>;
 			};
+
+			smp_twd_clk: smp-twd-clock {
+				#clock-cells = <0>;
+			};
 		};
 
 		timer at a0410600 {
Signature · 1 line
1.7.10.4
author
#35 lore raw

PATCH 16/33 ARM: ux500: Supply the TWD Timer clock lookup to the DBX500 DT

In reply to Lee Jones
---
 arch/arm/boot/dts/dbx5x0.dtsi |    2 ++
 1 file changed, 2 insertions(+)
arch/arm/boot/dts/dbx5x0.dtsi +2 -0
index 2027929..ed5655d 100644
@@ -71,6 +71,8 @@
 			compatible = "arm,cortex-a9-twd-timer";
 			reg = <0xa0410600 0x20>;
 			interrupts = <1 13 0x304>; /* IRQ level high per-CPU */
+
+			clocks = <&smp_twd_clk>;
 		};
 
 		rtc at 80154000 {
Signature · 1 line
1.7.10.4
author
#36 lore raw

PATCH 17/33 clk: ux500: Provide u8500_clk with skeleton Device Tree support

In reply to Lee Jones

The functional components will be added on a per-clock basis.

Cc Mike Turquette <[email protected]>
Cc Ulf Hansson <[email protected]>
---
 drivers/clk/ux500/u8500_clk.c |   17 +++++++++++++++++
 1 file changed, 17 insertions(+)
drivers/clk/ux500/u8500_clk.c +17 -0
index 0b4f35a..ef948ff 100644
@@ -7,6 +7,7 @@
  * License terms: GNU General Public License (GPL) version 2
  */
 
+#include <linux/of.h>
 #include <linux/clk.h>
 #include <linux/clkdev.h>
 #include <linux/clk-provider.h>
@@ -14,10 +15,17 @@
 #include <linux/platform_data/clk-ux500.h>
 #include "clk.h"
 
+const static struct of_device_id u8500_clk_of_match[] = {
+       { .compatible = "stericsson,u8500-clks", },
+       { },
+};
+
 void u8500_clk_init(u32 clkrst1_base, u32 clkrst2_base, u32 clkrst3_base,
 		    u32 clkrst5_base, u32 clkrst6_base)
 {
 	struct prcmu_fw_version *fw_version;
+	struct device_node *np = NULL;
+	struct device_node *child = NULL;
 	const char *sgaclk_parent = NULL;
 	struct clk *clk;
 
@@ -522,4 +530,13 @@ void u8500_clk_init(u32 clkrst1_base, u32 clkrst2_base, u32 clkrst3_base,
 	clk = clk_reg_prcc_kclk("p3_rng_kclk", "rngclk",
 			clkrst6_base, BIT(0), CLK_SET_RATE_GATE);
 	clk_register_clkdev(clk, NULL, "rng");
+
+	if (of_have_populated_dt())
+		np = of_find_matching_node(NULL, u8500_clk_of_match);
+	if (!np)
+		return;
+
+	for_each_child_of_node(np, child) {
+		/* Place holder for supported nodes. */
+	}
 }
Signature · 1 line
1.7.10.4
author
#37 lore raw

PATCH 18/33 clk: ux500: Add a 2-cell Device Tree parser for obtaining PRCC clocks

In reply to Lee Jones

PRCC (peripheral and kernel) clocks are specified using a property tuple <&phandle base bit>, where 'base' is the peripheral (1, 2, 3, 5 or 6), and bit is read-in value into that peripheral stipulated by the hardware specification.

Cc Mike Turquette <[email protected]>
Cc Ulf Hansson <[email protected]>
---
 drivers/clk/ux500/u8500_clk.c |   19 +++++++++++++++++++
 1 file changed, 19 insertions(+)
drivers/clk/ux500/u8500_clk.c +19 -0
index ef948ff..3a7040b 100644
@@ -15,6 +15,25 @@
 #include <linux/platform_data/clk-ux500.h>
 #include "clk.h"
 
+struct clk *ux500_twocell_get(struct of_phandle_args *clkspec, void *data)
+{
+	struct clk **clk_data = data;
+	unsigned int base, bit;
+
+	if (clkspec->args_count != 2)
+		return  ERR_PTR(-EINVAL);
+
+	base = clkspec->args[0];
+	bit = clkspec->args[1];
+
+	if (base != 1 && base != 2 && base != 3 && base != 5 && base != 6) {
+		pr_err("%s: invalid PRCC base %d\n", __func__, base);
+		return ERR_PTR(-EINVAL);
+	}
+
+	return PRCC_SHOW(clk_data, base, bit);
+}
+
 const static struct of_device_id u8500_clk_of_match[] = {
        { .compatible = "stericsson,u8500-clks", },
        { },
Signature · 1 line
1.7.10.4
#38 lore raw

Re: [PATCH 18/33] clk: ux500: Add a 2-cell Device Tree parser for obtaining PRCC clocks

Quoted 27 lines · Lee Jones

PRCC (peripheral and kernel) clocks are specified using a property tuple <&phandle base bit>, where 'base' is the peripheral (1, 2, 3, 5 or 6), and bit is read-in value into that peripheral stipulated by the hardware specification.

Cc Mike Turquette <[email protected]>
Cc Ulf Hansson <[email protected]>
---
 drivers/clk/ux500/u8500_clk.c |   19 +++++++++++++++++++
 1 file changed, 19 insertions(+)
drivers/clk/ux500/u8500_clk.c +7 -0
index ef948ff..3a7040b 100644
@@ -15,6 +15,25 @@
 #include <linux/platform_data/clk-ux500.h>
 #include "clk.h"
 
+struct clk *ux500_twocell_get(struct of_phandle_args *clkspec, void *data)
+{
+       struct clk **clk_data = data;
+       unsigned int base, bit;
+
+       if (clkspec->args_count != 2)
+               return  ERR_PTR(-EINVAL);

drivers/clk/ux500/u8500_clk.c line 34

Quoted 10 lines
drivers/clk/ux500/u8500_clk.c +10 -0
+
+       base = clkspec->args[0];
+       bit = clkspec->args[1];
+
+       if (base != 1 && base != 2 && base != 3 && base != 5 && base != 6) {
+               pr_err("%s: invalid PRCC base %d\n", __func__, base);
+               return ERR_PTR(-EINVAL);
+       }
+
+       return PRCC_SHOW(clk_data, base, bit);

Where did this macro come from?

Kind regards
Ulf Hansson

Quoted 8 lines
drivers/clk/ux500/u8500_clk.c +2 -1
+}
+
 const static struct of_device_id u8500_clk_of_match[] = {
        { .compatible = "stericsson,u8500-clks", },
        { },
--

1.7.10.4

author
#39 lore raw

Re: [PATCH 18/33] clk: ux500: Add a 2-cell Device Tree parser for obtaining PRCC clocks

Quoted 32 lines · Ulf Hansson
Quoted 31 lines · Lee Jones

PRCC (peripheral and kernel) clocks are specified using a property tuple <&phandle base bit>, where 'base' is the peripheral (1, 2, 3, 5 or 6), and bit is read-in value into that peripheral stipulated by the hardware specification.

Cc Mike Turquette <[email protected]>
Cc Ulf Hansson <[email protected]>
---
 drivers/clk/ux500/u8500_clk.c |   19 +++++++++++++++++++
 1 file changed, 19 insertions(+)
drivers/clk/ux500/u8500_clk.c +11 -0
index ef948ff..3a7040b 100644
@@ -15,6 +15,25 @@
 #include <linux/platform_data/clk-ux500.h>
 #include "clk.h"
 
+struct clk *ux500_twocell_get(struct of_phandle_args *clkspec, void *data)
+{
+       struct clk **clk_data = data;
+       unsigned int base, bit;
+
+       if (clkspec->args_count != 2)
+               return  ERR_PTR(-EINVAL);
+
+       base = clkspec->args[0];
+       bit = clkspec->args[1];
+
Quoted 8 lines

drivers/clk/ux500/u8500_clk.c line 34

Quoted 6 lines
drivers/clk/ux500/u8500_clk.c +6 -0
+       if (base != 1 && base != 2 && base != 3 && base != 5 && base != 6) {
+               pr_err("%s: invalid PRCC base %d\n", __func__, base);
+               return ERR_PTR(-EINVAL);
+       }
+
+       return PRCC_SHOW(clk_data, base, bit);

Where did this macro come from?

You're right, there is an ordering issue here, I'll fixup, thanks.

Quoted 6 lines
Quoted 6 lines
drivers/clk/ux500/u8500_clk.c +2 -0
+}
+
 const static struct of_device_id u8500_clk_of_match[] = {
        { .compatible = "stericsson,u8500-clks", },
        { },
Signature · 4 lines
Lee Jones
Linaro ST-Ericsson Landing Team Lead
Linaro.org ? Open source software for ARM SoCs
Follow Linaro: Facebook | Twitter | Blog
author
#40 lore raw

PATCH v2 18/33 clk: ux500: Add a 2-cell Device Tree parser for obtaining PRCC clocks

In reply to Lee Jones

PRCC (peripheral and kernel) clocks are specified using a property tuple <&phandle base bit>, where 'base' is the peripheral (1, 2, 3, 5 or 6), and bit is read-in value into that peripheral stipulated by the hardware specification.

Cc Mike Turquette <[email protected]>
Cc Ulf Hansson <[email protected]>
drivers/clk/ux500/u8500_clk.c +22 -0
index ef948ff..c2d9a1c 100644
@@ -15,6 +15,28 @@
 #include <linux/platform_data/clk-ux500.h>
 #include "clk.h"
 
+#define PRCC_SHOW(clk, base, bit) \
+	clk[(base * PRCC_PERIPHS_PER_CLUSTER) + bit]
+
+struct clk *ux500_twocell_get(struct of_phandle_args *clkspec, void *data)
+{
+	struct clk **clk_data = data;
+	unsigned int base, bit;
+
+	if (clkspec->args_count != 2)
+		return  ERR_PTR(-EINVAL);
+
+	base = clkspec->args[0];
+	bit = clkspec->args[1];
+
+	if (base != 1 && base != 2 && base != 3 && base != 5 && base != 6) {
+		pr_err("%s: invalid PRCC base %d\n", __func__, base);
+		return ERR_PTR(-EINVAL);
+	}
+
+	return PRCC_SHOW(clk_data, base, bit);
+}
+
 const static struct of_device_id u8500_clk_of_match[] = {
        { .compatible = "stericsson,u8500-clks", },
        { },
author
#41 lore raw

PATCH 19/33 clk: ux500: Add Device Tree support for the PRCMU clock

In reply to Lee Jones

This patch enables clocks to be specified from Device Tree via phandles to the "prcmu-clock" node.

Cc Mike Turquette <[email protected]>
Cc Ulf Hansson <[email protected]>
---
 drivers/clk/ux500/u8500_clk.c |   50 +++++++++++++++++++++++++++++++++++++++--
 1 file changed, 48 insertions(+), 2 deletions(-)
drivers/clk/ux500/u8500_clk.c +48 -2
index 3a7040b..4f5ad4c 100644
@@ -15,6 +15,8 @@
 #include <linux/platform_data/clk-ux500.h>
 #include "clk.h"
 
+static struct clk *prcmu_clk[PRCMU_NUM_CLKS];
+
 struct clk *ux500_twocell_get(struct of_phandle_args *clkspec, void *data)
 {
 	struct clk **clk_data = data;
@@ -52,14 +54,17 @@ void u8500_clk_init(u32 clkrst1_base, u32 clkrst2_base, u32 clkrst3_base,
 	clk = clk_reg_prcmu_gate("soc0_pll", NULL, PRCMU_PLLSOC0,
 				CLK_IS_ROOT|CLK_IGNORE_UNUSED);
 	clk_register_clkdev(clk, "soc0_pll", NULL);
+	prcmu_clk[PRCMU_PLLSOC0] = clk;
 
 	clk = clk_reg_prcmu_gate("soc1_pll", NULL, PRCMU_PLLSOC1,
 				CLK_IS_ROOT|CLK_IGNORE_UNUSED);
 	clk_register_clkdev(clk, "soc1_pll", NULL);
+	prcmu_clk[PRCMU_PLLSOC1] = clk;
 
 	clk = clk_reg_prcmu_gate("ddr_pll", NULL, PRCMU_PLLDDR,
 				CLK_IS_ROOT|CLK_IGNORE_UNUSED);
 	clk_register_clkdev(clk, "ddr_pll", NULL);
+	prcmu_clk[PRCMU_PLLDDR] = clk;
 
 	/* FIXME: Add sys, ulp and int clocks here. */
 
@@ -90,65 +95,84 @@ void u8500_clk_init(u32 clkrst1_base, u32 clkrst2_base, u32 clkrst3_base,
 		clk = clk_reg_prcmu_gate("sgclk", NULL,
 					PRCMU_SGACLK, CLK_IS_ROOT);
 	clk_register_clkdev(clk, NULL, "mali");
+	prcmu_clk[PRCMU_SGACLK] = clk;
 
 	clk = clk_reg_prcmu_gate("uartclk", NULL, PRCMU_UARTCLK, CLK_IS_ROOT);
 	clk_register_clkdev(clk, NULL, "UART");
+	prcmu_clk[PRCMU_UARTCLK] = clk;
 
 	clk = clk_reg_prcmu_gate("msp02clk", NULL, PRCMU_MSP02CLK, CLK_IS_ROOT);
 	clk_register_clkdev(clk, NULL, "MSP02");
+	prcmu_clk[PRCMU_MSP02CLK] = clk;
 
 	clk = clk_reg_prcmu_gate("msp1clk", NULL, PRCMU_MSP1CLK, CLK_IS_ROOT);
 	clk_register_clkdev(clk, NULL, "MSP1");
+	prcmu_clk[PRCMU_MSP1CLK] = clk;
 
 	clk = clk_reg_prcmu_gate("i2cclk", NULL, PRCMU_I2CCLK, CLK_IS_ROOT);
 	clk_register_clkdev(clk, NULL, "I2C");
+	prcmu_clk[PRCMU_I2CCLK] = clk;
 
 	clk = clk_reg_prcmu_gate("slimclk", NULL, PRCMU_SLIMCLK, CLK_IS_ROOT);
 	clk_register_clkdev(clk, NULL, "slim");
+	prcmu_clk[PRCMU_SLIMCLK] = clk;
 
 	clk = clk_reg_prcmu_gate("per1clk", NULL, PRCMU_PER1CLK, CLK_IS_ROOT);
 	clk_register_clkdev(clk, NULL, "PERIPH1");
+	prcmu_clk[PRCMU_PER1CLK] = clk;
 
 	clk = clk_reg_prcmu_gate("per2clk", NULL, PRCMU_PER2CLK, CLK_IS_ROOT);
 	clk_register_clkdev(clk, NULL, "PERIPH2");
+	prcmu_clk[PRCMU_PER2CLK] = clk;
 
 	clk = clk_reg_prcmu_gate("per3clk", NULL, PRCMU_PER3CLK, CLK_IS_ROOT);
 	clk_register_clkdev(clk, NULL, "PERIPH3");
+	prcmu_clk[PRCMU_PER3CLK] = clk;
 
 	clk = clk_reg_prcmu_gate("per5clk", NULL, PRCMU_PER5CLK, CLK_IS_ROOT);
 	clk_register_clkdev(clk, NULL, "PERIPH5");
+	prcmu_clk[PRCMU_PER5CLK] = clk;
 
 	clk = clk_reg_prcmu_gate("per6clk", NULL, PRCMU_PER6CLK, CLK_IS_ROOT);
 	clk_register_clkdev(clk, NULL, "PERIPH6");
+	prcmu_clk[PRCMU_PER6CLK] = clk;
 
 	clk = clk_reg_prcmu_gate("per7clk", NULL, PRCMU_PER7CLK, CLK_IS_ROOT);
 	clk_register_clkdev(clk, NULL, "PERIPH7");
+	prcmu_clk[PRCMU_PER7CLK] = clk;
 
 	clk = clk_reg_prcmu_scalable("lcdclk", NULL, PRCMU_LCDCLK, 0,
 				CLK_IS_ROOT|CLK_SET_RATE_GATE);
 	clk_register_clkdev(clk, NULL, "lcd");
 	clk_register_clkdev(clk, "lcd", "mcde");
+	prcmu_clk[PRCMU_LCDCLK] = clk;
 
 	clk = clk_reg_prcmu_opp_gate("bmlclk", NULL, PRCMU_BMLCLK, CLK_IS_ROOT);
 	clk_register_clkdev(clk, NULL, "bml");
+	prcmu_clk[PRCMU_BMLCLK] = clk;
 
 	clk = clk_reg_prcmu_scalable("hsitxclk", NULL, PRCMU_HSITXCLK, 0,
 				CLK_IS_ROOT|CLK_SET_RATE_GATE);
+	prcmu_clk[PRCMU_HSITXCLK] = clk;
 
 	clk = clk_reg_prcmu_scalable("hsirxclk", NULL, PRCMU_HSIRXCLK, 0,
 				CLK_IS_ROOT|CLK_SET_RATE_GATE);
+	prcmu_clk[PRCMU_HSIRXCLK] = clk;
 
 	clk = clk_reg_prcmu_scalable("hdmiclk", NULL, PRCMU_HDMICLK, 0,
 				CLK_IS_ROOT|CLK_SET_RATE_GATE);
 	clk_register_clkdev(clk, NULL, "hdmi");
 	clk_register_clkdev(clk, "hdmi", "mcde");
+	prcmu_clk[PRCMU_HDMICLK] = clk;
 
 	clk = clk_reg_prcmu_gate("apeatclk", NULL, PRCMU_APEATCLK, CLK_IS_ROOT);
 	clk_register_clkdev(clk, NULL, "apeat");
+	prcmu_clk[PRCMU_APEATCLK] = clk;
 
 	clk = clk_reg_prcmu_gate("apetraceclk", NULL, PRCMU_APETRACECLK,
 				CLK_IS_ROOT);
 	clk_register_clkdev(clk, NULL, "apetrace");
+	prcmu_clk[PRCMU_APETRACECLK] = clk;
 
 	clk = clk_reg_prcmu_gate("mcdeclk", NULL, PRCMU_MCDECLK, CLK_IS_ROOT);
 	clk_register_clkdev(clk, NULL, "mcde");
@@ -156,76 +180,92 @@ void u8500_clk_init(u32 clkrst1_base, u32 clkrst2_base, u32 clkrst3_base,
 	clk_register_clkdev(clk, "dsisys", "dsilink.0");
 	clk_register_clkdev(clk, "dsisys", "dsilink.1");
 	clk_register_clkdev(clk, "dsisys", "dsilink.2");
+	prcmu_clk[PRCMU_MCDECLK] = clk;
 
 	clk = clk_reg_prcmu_opp_gate("ipi2cclk", NULL, PRCMU_IPI2CCLK,
 				CLK_IS_ROOT);
 	clk_register_clkdev(clk, NULL, "ipi2");
+	prcmu_clk[PRCMU_IPI2CCLK] = clk;
 
 	clk = clk_reg_prcmu_gate("dsialtclk", NULL, PRCMU_DSIALTCLK,
 				CLK_IS_ROOT);
 	clk_register_clkdev(clk, NULL, "dsialt");
+	prcmu_clk[PRCMU_DSIALTCLK] = clk;
 
 	clk = clk_reg_prcmu_gate("dmaclk", NULL, PRCMU_DMACLK, CLK_IS_ROOT);
 	clk_register_clkdev(clk, NULL, "dma40.0");
+	prcmu_clk[PRCMU_DMACLK] = clk;
 
 	clk = clk_reg_prcmu_gate("b2r2clk", NULL, PRCMU_B2R2CLK, CLK_IS_ROOT);
 	clk_register_clkdev(clk, NULL, "b2r2");
 	clk_register_clkdev(clk, NULL, "b2r2_core");
 	clk_register_clkdev(clk, NULL, "U8500-B2R2.0");
+	prcmu_clk[PRCMU_B2R2CLK] = clk;
 
 	clk = clk_reg_prcmu_scalable("tvclk", NULL, PRCMU_TVCLK, 0,
 				CLK_IS_ROOT|CLK_SET_RATE_GATE);
 	clk_register_clkdev(clk, NULL, "tv");
 	clk_register_clkdev(clk, "tv", "mcde");
+	prcmu_clk[PRCMU_TVCLK] = clk;
 
 	clk = clk_reg_prcmu_gate("sspclk", NULL, PRCMU_SSPCLK, CLK_IS_ROOT);
 	clk_register_clkdev(clk, NULL, "SSP");
+	prcmu_clk[PRCMU_SSPCLK] = clk;
 
 	clk = clk_reg_prcmu_gate("rngclk", NULL, PRCMU_RNGCLK, CLK_IS_ROOT);
 	clk_register_clkdev(clk, NULL, "rngclk");
+	prcmu_clk[PRCMU_RNGCLK] = clk;
 
 	clk = clk_reg_prcmu_gate("uiccclk", NULL, PRCMU_UICCCLK, CLK_IS_ROOT);
 	clk_register_clkdev(clk, NULL, "uicc");
+	prcmu_clk[PRCMU_UICCCLK] = clk;
 
 	clk = clk_reg_prcmu_gate("timclk", NULL, PRCMU_TIMCLK, CLK_IS_ROOT);
 	clk_register_clkdev(clk, NULL, "mtu0");
 	clk_register_clkdev(clk, NULL, "mtu1");
+	prcmu_clk[PRCMU_TIMCLK] = clk;
 
 	clk = clk_reg_prcmu_opp_volt_scalable("sdmmcclk", NULL, PRCMU_SDMMCCLK,
 					100000000,
 					CLK_IS_ROOT|CLK_SET_RATE_GATE);
 	clk_register_clkdev(clk, NULL, "sdmmc");
+	prcmu_clk[PRCMU_SDMMCCLK] = clk;
 
 	clk = clk_reg_prcmu_scalable("dsi_pll", "hdmiclk",
 				PRCMU_PLLDSI, 0, CLK_SET_RATE_GATE);
 	clk_register_clkdev(clk, "dsihs2", "mcde");
 	clk_register_clkdev(clk, "dsihs2", "dsilink.2");
-
+	prcmu_clk[PRCMU_PLLDSI] = clk;
 
 	clk = clk_reg_prcmu_scalable("dsi0clk", "dsi_pll",
 				PRCMU_DSI0CLK, 0, CLK_SET_RATE_GATE);
 	clk_register_clkdev(clk, "dsihs0", "mcde");
 	clk_register_clkdev(clk, "dsihs0", "dsilink.0");
+	prcmu_clk[PRCMU_DSI0CLK] = clk;
 
 	clk = clk_reg_prcmu_scalable("dsi1clk", "dsi_pll",
 				PRCMU_DSI1CLK, 0, CLK_SET_RATE_GATE);
 	clk_register_clkdev(clk, "dsihs1", "mcde");
 	clk_register_clkdev(clk, "dsihs1", "dsilink.1");
+	prcmu_clk[PRCMU_DSI1CLK] = clk;
 
 	clk = clk_reg_prcmu_scalable("dsi0escclk", "tvclk",
 				PRCMU_DSI0ESCCLK, 0, CLK_SET_RATE_GATE);
 	clk_register_clkdev(clk, "dsilp0", "dsilink.0");
 	clk_register_clkdev(clk, "dsilp0", "mcde");
+	prcmu_clk[PRCMU_DSI0ESCCLK] = clk;
 
 	clk = clk_reg_prcmu_scalable("dsi1escclk", "tvclk",
 				PRCMU_DSI1ESCCLK, 0, CLK_SET_RATE_GATE);
 	clk_register_clkdev(clk, "dsilp1", "dsilink.1");
 	clk_register_clkdev(clk, "dsilp1", "mcde");
+	prcmu_clk[PRCMU_DSI1ESCCLK] = clk;
 
 	clk = clk_reg_prcmu_scalable("dsi2escclk", "tvclk",
 				PRCMU_DSI2ESCCLK, 0, CLK_SET_RATE_GATE);
 	clk_register_clkdev(clk, "dsilp2", "dsilink.2");
 	clk_register_clkdev(clk, "dsilp2", "mcde");
+	prcmu_clk[PRCMU_DSI2ESCCLK] = clk;
 
 	clk = clk_reg_prcmu_scalable_rate("armss", NULL,
 				PRCMU_ARMSS, 0, CLK_IS_ROOT|CLK_IGNORE_UNUSED);
@@ -556,6 +596,12 @@ void u8500_clk_init(u32 clkrst1_base, u32 clkrst2_base, u32 clkrst3_base,
 		return;
 
 	for_each_child_of_node(np, child) {
-		/* Place holder for supported nodes. */
+		static struct clk_onecell_data clk_data;
+
+		if (!of_node_cmp(child->name, "prcmu-clock")) {
+			clk_data.clks = prcmu_clk;
+			clk_data.clk_num = ARRAY_SIZE(prcmu_clk);
+			of_clk_add_provider(child, of_clk_src_onecell_get, &clk_data);
+		}
 	}
 }
Signature · 1 line
1.7.10.4
#42 lore raw

Re: [PATCH 19/33] clk: ux500: Add Device Tree support for the PRCMU clock

Quoted 15 lines · Lee Jones

This patch enables clocks to be specified from Device Tree via phandles to the "prcmu-clock" node.

Cc Mike Turquette <[email protected]>
Cc Ulf Hansson <[email protected]>
---
 drivers/clk/ux500/u8500_clk.c |   50 +++++++++++++++++++++++++++++++++++++++--
 1 file changed, 48 insertions(+), 2 deletions(-)
drivers/clk/ux500/u8500_clk.c +0 -0
index 3a7040b..4f5ad4c 100644

drivers/clk/ux500/u8500_clk.c line 20

Quoted 6 lines
drivers/clk/ux500/u8500_clk.c +2 -0
@@ -15,6 +15,8 @@
 #include <linux/platform_data/clk-ux500.h>
 #include "clk.h"
 
+static struct clk *prcmu_clk[PRCMU_NUM_CLKS];
+
 struct clk *ux500_twocell_get(struct of_phandle_args *clkspec, void *data)

Actually I thought ux500_twocell_get was going to be used in this patch, since the previous one was adding this helper function, but it isn't?

Kind regards
Ulf Hansson

Quoted 216 lines
drivers/clk/ux500/u8500_clk.c +46 -3
 {
        struct clk **clk_data = data;
@@ -52,14 +54,17 @@ void u8500_clk_init(u32 clkrst1_base, u32 clkrst2_base, u32 clkrst3_base,
        clk = clk_reg_prcmu_gate("soc0_pll", NULL, PRCMU_PLLSOC0,
                                CLK_IS_ROOT|CLK_IGNORE_UNUSED);
        clk_register_clkdev(clk, "soc0_pll", NULL);
+       prcmu_clk[PRCMU_PLLSOC0] = clk;
 
        clk = clk_reg_prcmu_gate("soc1_pll", NULL, PRCMU_PLLSOC1,
                                CLK_IS_ROOT|CLK_IGNORE_UNUSED);
        clk_register_clkdev(clk, "soc1_pll", NULL);
+       prcmu_clk[PRCMU_PLLSOC1] = clk;
 
        clk = clk_reg_prcmu_gate("ddr_pll", NULL, PRCMU_PLLDDR,
                                CLK_IS_ROOT|CLK_IGNORE_UNUSED);
        clk_register_clkdev(clk, "ddr_pll", NULL);
+       prcmu_clk[PRCMU_PLLDDR] = clk;
 
        /* FIXME: Add sys, ulp and int clocks here. */
 
@@ -90,65 +95,84 @@ void u8500_clk_init(u32 clkrst1_base, u32 clkrst2_base, u32 clkrst3_base,
                clk = clk_reg_prcmu_gate("sgclk", NULL,
                                        PRCMU_SGACLK, CLK_IS_ROOT);
        clk_register_clkdev(clk, NULL, "mali");
+       prcmu_clk[PRCMU_SGACLK] = clk;
 
        clk = clk_reg_prcmu_gate("uartclk", NULL, PRCMU_UARTCLK, CLK_IS_ROOT);
        clk_register_clkdev(clk, NULL, "UART");
+       prcmu_clk[PRCMU_UARTCLK] = clk;
 
        clk = clk_reg_prcmu_gate("msp02clk", NULL, PRCMU_MSP02CLK, CLK_IS_ROOT);
        clk_register_clkdev(clk, NULL, "MSP02");
+       prcmu_clk[PRCMU_MSP02CLK] = clk;
 
        clk = clk_reg_prcmu_gate("msp1clk", NULL, PRCMU_MSP1CLK, CLK_IS_ROOT);
        clk_register_clkdev(clk, NULL, "MSP1");
+       prcmu_clk[PRCMU_MSP1CLK] = clk;
 
        clk = clk_reg_prcmu_gate("i2cclk", NULL, PRCMU_I2CCLK, CLK_IS_ROOT);
        clk_register_clkdev(clk, NULL, "I2C");
+       prcmu_clk[PRCMU_I2CCLK] = clk;
 
        clk = clk_reg_prcmu_gate("slimclk", NULL, PRCMU_SLIMCLK, CLK_IS_ROOT);
        clk_register_clkdev(clk, NULL, "slim");
+       prcmu_clk[PRCMU_SLIMCLK] = clk;
 
        clk = clk_reg_prcmu_gate("per1clk", NULL, PRCMU_PER1CLK, CLK_IS_ROOT);
        clk_register_clkdev(clk, NULL, "PERIPH1");
+       prcmu_clk[PRCMU_PER1CLK] = clk;
 
        clk = clk_reg_prcmu_gate("per2clk", NULL, PRCMU_PER2CLK, CLK_IS_ROOT);
        clk_register_clkdev(clk, NULL, "PERIPH2");
+       prcmu_clk[PRCMU_PER2CLK] = clk;
 
        clk = clk_reg_prcmu_gate("per3clk", NULL, PRCMU_PER3CLK, CLK_IS_ROOT);
        clk_register_clkdev(clk, NULL, "PERIPH3");
+       prcmu_clk[PRCMU_PER3CLK] = clk;
 
        clk = clk_reg_prcmu_gate("per5clk", NULL, PRCMU_PER5CLK, CLK_IS_ROOT);
        clk_register_clkdev(clk, NULL, "PERIPH5");
+       prcmu_clk[PRCMU_PER5CLK] = clk;
 
        clk = clk_reg_prcmu_gate("per6clk", NULL, PRCMU_PER6CLK, CLK_IS_ROOT);
        clk_register_clkdev(clk, NULL, "PERIPH6");
+       prcmu_clk[PRCMU_PER6CLK] = clk;
 
        clk = clk_reg_prcmu_gate("per7clk", NULL, PRCMU_PER7CLK, CLK_IS_ROOT);
        clk_register_clkdev(clk, NULL, "PERIPH7");
+       prcmu_clk[PRCMU_PER7CLK] = clk;
 
        clk = clk_reg_prcmu_scalable("lcdclk", NULL, PRCMU_LCDCLK, 0,
                                CLK_IS_ROOT|CLK_SET_RATE_GATE);
        clk_register_clkdev(clk, NULL, "lcd");
        clk_register_clkdev(clk, "lcd", "mcde");
+       prcmu_clk[PRCMU_LCDCLK] = clk;
 
        clk = clk_reg_prcmu_opp_gate("bmlclk", NULL, PRCMU_BMLCLK, CLK_IS_ROOT);
        clk_register_clkdev(clk, NULL, "bml");
+       prcmu_clk[PRCMU_BMLCLK] = clk;
 
        clk = clk_reg_prcmu_scalable("hsitxclk", NULL, PRCMU_HSITXCLK, 0,
                                CLK_IS_ROOT|CLK_SET_RATE_GATE);
+       prcmu_clk[PRCMU_HSITXCLK] = clk;
 
        clk = clk_reg_prcmu_scalable("hsirxclk", NULL, PRCMU_HSIRXCLK, 0,
                                CLK_IS_ROOT|CLK_SET_RATE_GATE);
+       prcmu_clk[PRCMU_HSIRXCLK] = clk;
 
        clk = clk_reg_prcmu_scalable("hdmiclk", NULL, PRCMU_HDMICLK, 0,
                                CLK_IS_ROOT|CLK_SET_RATE_GATE);
        clk_register_clkdev(clk, NULL, "hdmi");
        clk_register_clkdev(clk, "hdmi", "mcde");
+       prcmu_clk[PRCMU_HDMICLK] = clk;
 
        clk = clk_reg_prcmu_gate("apeatclk", NULL, PRCMU_APEATCLK, CLK_IS_ROOT);
        clk_register_clkdev(clk, NULL, "apeat");
+       prcmu_clk[PRCMU_APEATCLK] = clk;
 
        clk = clk_reg_prcmu_gate("apetraceclk", NULL, PRCMU_APETRACECLK,
                                CLK_IS_ROOT);
        clk_register_clkdev(clk, NULL, "apetrace");
+       prcmu_clk[PRCMU_APETRACECLK] = clk;
 
        clk = clk_reg_prcmu_gate("mcdeclk", NULL, PRCMU_MCDECLK, CLK_IS_ROOT);
        clk_register_clkdev(clk, NULL, "mcde");
@@ -156,76 +180,92 @@ void u8500_clk_init(u32 clkrst1_base, u32 clkrst2_base, u32 clkrst3_base,
        clk_register_clkdev(clk, "dsisys", "dsilink.0");
        clk_register_clkdev(clk, "dsisys", "dsilink.1");
        clk_register_clkdev(clk, "dsisys", "dsilink.2");
+       prcmu_clk[PRCMU_MCDECLK] = clk;
 
        clk = clk_reg_prcmu_opp_gate("ipi2cclk", NULL, PRCMU_IPI2CCLK,
                                CLK_IS_ROOT);
        clk_register_clkdev(clk, NULL, "ipi2");
+       prcmu_clk[PRCMU_IPI2CCLK] = clk;
 
        clk = clk_reg_prcmu_gate("dsialtclk", NULL, PRCMU_DSIALTCLK,
                                CLK_IS_ROOT);
        clk_register_clkdev(clk, NULL, "dsialt");
+       prcmu_clk[PRCMU_DSIALTCLK] = clk;
 
        clk = clk_reg_prcmu_gate("dmaclk", NULL, PRCMU_DMACLK, CLK_IS_ROOT);
        clk_register_clkdev(clk, NULL, "dma40.0");
+       prcmu_clk[PRCMU_DMACLK] = clk;
 
        clk = clk_reg_prcmu_gate("b2r2clk", NULL, PRCMU_B2R2CLK, CLK_IS_ROOT);
        clk_register_clkdev(clk, NULL, "b2r2");
        clk_register_clkdev(clk, NULL, "b2r2_core");
        clk_register_clkdev(clk, NULL, "U8500-B2R2.0");
+       prcmu_clk[PRCMU_B2R2CLK] = clk;
 
        clk = clk_reg_prcmu_scalable("tvclk", NULL, PRCMU_TVCLK, 0,
                                CLK_IS_ROOT|CLK_SET_RATE_GATE);
        clk_register_clkdev(clk, NULL, "tv");
        clk_register_clkdev(clk, "tv", "mcde");
+       prcmu_clk[PRCMU_TVCLK] = clk;
 
        clk = clk_reg_prcmu_gate("sspclk", NULL, PRCMU_SSPCLK, CLK_IS_ROOT);
        clk_register_clkdev(clk, NULL, "SSP");
+       prcmu_clk[PRCMU_SSPCLK] = clk;
 
        clk = clk_reg_prcmu_gate("rngclk", NULL, PRCMU_RNGCLK, CLK_IS_ROOT);
        clk_register_clkdev(clk, NULL, "rngclk");
+       prcmu_clk[PRCMU_RNGCLK] = clk;
 
        clk = clk_reg_prcmu_gate("uiccclk", NULL, PRCMU_UICCCLK, CLK_IS_ROOT);
        clk_register_clkdev(clk, NULL, "uicc");
+       prcmu_clk[PRCMU_UICCCLK] = clk;
 
        clk = clk_reg_prcmu_gate("timclk", NULL, PRCMU_TIMCLK, CLK_IS_ROOT);
        clk_register_clkdev(clk, NULL, "mtu0");
        clk_register_clkdev(clk, NULL, "mtu1");
+       prcmu_clk[PRCMU_TIMCLK] = clk;
 
        clk = clk_reg_prcmu_opp_volt_scalable("sdmmcclk", NULL, PRCMU_SDMMCCLK,
                                        100000000,
                                        CLK_IS_ROOT|CLK_SET_RATE_GATE);
        clk_register_clkdev(clk, NULL, "sdmmc");
+       prcmu_clk[PRCMU_SDMMCCLK] = clk;
 
        clk = clk_reg_prcmu_scalable("dsi_pll", "hdmiclk",
                                PRCMU_PLLDSI, 0, CLK_SET_RATE_GATE);
        clk_register_clkdev(clk, "dsihs2", "mcde");
        clk_register_clkdev(clk, "dsihs2", "dsilink.2");
-
+       prcmu_clk[PRCMU_PLLDSI] = clk;
 
        clk = clk_reg_prcmu_scalable("dsi0clk", "dsi_pll",
                                PRCMU_DSI0CLK, 0, CLK_SET_RATE_GATE);
        clk_register_clkdev(clk, "dsihs0", "mcde");
        clk_register_clkdev(clk, "dsihs0", "dsilink.0");
+       prcmu_clk[PRCMU_DSI0CLK] = clk;
 
        clk = clk_reg_prcmu_scalable("dsi1clk", "dsi_pll",
                                PRCMU_DSI1CLK, 0, CLK_SET_RATE_GATE);
        clk_register_clkdev(clk, "dsihs1", "mcde");
        clk_register_clkdev(clk, "dsihs1", "dsilink.1");
+       prcmu_clk[PRCMU_DSI1CLK] = clk;
 
        clk = clk_reg_prcmu_scalable("dsi0escclk", "tvclk",
                                PRCMU_DSI0ESCCLK, 0, CLK_SET_RATE_GATE);
        clk_register_clkdev(clk, "dsilp0", "dsilink.0");
        clk_register_clkdev(clk, "dsilp0", "mcde");
+       prcmu_clk[PRCMU_DSI0ESCCLK] = clk;
 
        clk = clk_reg_prcmu_scalable("dsi1escclk", "tvclk",
                                PRCMU_DSI1ESCCLK, 0, CLK_SET_RATE_GATE);
        clk_register_clkdev(clk, "dsilp1", "dsilink.1");
        clk_register_clkdev(clk, "dsilp1", "mcde");
+       prcmu_clk[PRCMU_DSI1ESCCLK] = clk;
 
        clk = clk_reg_prcmu_scalable("dsi2escclk", "tvclk",
                                PRCMU_DSI2ESCCLK, 0, CLK_SET_RATE_GATE);
        clk_register_clkdev(clk, "dsilp2", "dsilink.2");
        clk_register_clkdev(clk, "dsilp2", "mcde");
+       prcmu_clk[PRCMU_DSI2ESCCLK] = clk;
 
        clk = clk_reg_prcmu_scalable_rate("armss", NULL,
                                PRCMU_ARMSS, 0, CLK_IS_ROOT|CLK_IGNORE_UNUSED);
@@ -556,6 +596,12 @@ void u8500_clk_init(u32 clkrst1_base, u32 clkrst2_base, u32 clkrst3_base,
                return;
 
        for_each_child_of_node(np, child) {
-               /* Place holder for supported nodes. */
+               static struct clk_onecell_data clk_data;
+
+               if (!of_node_cmp(child->name, "prcmu-clock")) {
+                       clk_data.clks = prcmu_clk;
+                       clk_data.clk_num = ARRAY_SIZE(prcmu_clk);
+                       of_clk_add_provider(child, of_clk_src_onecell_get, &clk_data);
+               }
        }
 }
--

1.7.10.4

author
#43 lore raw

Re: [PATCH 19/33] clk: ux500: Add Device Tree support for the PRCMU clock

Quoted 19 lines · Ulf Hansson
Quoted 18 lines · Lee Jones

This patch enables clocks to be specified from Device Tree via phandles to the "prcmu-clock" node.

Cc Mike Turquette <[email protected]>
Cc Ulf Hansson <[email protected]>
---
 drivers/clk/ux500/u8500_clk.c |   50 +++++++++++++++++++++++++++++++++++++++--
 1 file changed, 48 insertions(+), 2 deletions(-)
drivers/clk/ux500/u8500_clk.c +0 -0
index 3a7040b..4f5ad4c 100644
@@ -15,6 +15,8 @@
 #include <linux/platform_data/clk-ux500.h>
 #include "clk.h"
 
Quoted 7 lines

drivers/clk/ux500/u8500_clk.c line 20

Quoted 3 lines
drivers/clk/ux500/u8500_clk.c +2 -0
+static struct clk *prcmu_clk[PRCMU_NUM_CLKS];
+
 struct clk *ux500_twocell_get(struct of_phandle_args *clkspec, void *data)

Actually I thought ux500_twocell_get was going to be used in this patch, since the previous one was adding this helper function, but it isn't?

Yes it is:

Quoted 8 lines

drivers/clk/ux500/u8500_clk.c void u8500_clk_init(u32 clkrst1_base, u32 clkrst2_base, u32 clkrst3_base, · line 604

Quoted 8 lines
drivers/clk/ux500/u8500_clk.c +6 -1
        for_each_child_of_node(np, child) {
-               /* Place holder for supported nodes. */
+               static struct clk_onecell_data clk_data;
+
+               if (!of_node_cmp(child->name, "prcmu-clock")) {
+                       clk_data.clks = prcmu_clk;
+                       clk_data.clk_num = ARRAY_SIZE(prcmu_clk);
+                       of_clk_add_provider(child, of_clk_src_onecell_get, &clk_data);

Here.

Signature · 4 lines
Lee Jones
Linaro ST-Ericsson Landing Team Lead
Linaro.org ? Open source software for ARM SoCs
Follow Linaro: Facebook | Twitter | Blog
author
#44 lore raw

Re: [PATCH 19/33] clk: ux500: Add Device Tree support for the PRCMU clock

Quoted 29 lines · Lee Jones
Quoted 19 lines · Ulf Hansson
Quoted 18 lines · Lee Jones

This patch enables clocks to be specified from Device Tree via phandles to the "prcmu-clock" node.

Cc Mike Turquette <[email protected]>
Cc Ulf Hansson <[email protected]>
---
 drivers/clk/ux500/u8500_clk.c |   50 +++++++++++++++++++++++++++++++++++++++--
 1 file changed, 48 insertions(+), 2 deletions(-)
drivers/clk/ux500/u8500_clk.c +0 -0
index 3a7040b..4f5ad4c 100644
@@ -15,6 +15,8 @@
 #include <linux/platform_data/clk-ux500.h>
 #include "clk.h"
 
Quoted 7 lines

drivers/clk/ux500/u8500_clk.c line 20

Quoted 3 lines
drivers/clk/ux500/u8500_clk.c +2 -0
+static struct clk *prcmu_clk[PRCMU_NUM_CLKS];
+
 struct clk *ux500_twocell_get(struct of_phandle_args *clkspec, void *data)

Actually I thought ux500_twocell_get was going to be used in this patch, since the previous one was adding this helper function, but it isn't?

Quoted 1 line

Yes it is:

Whoops! No, it's using it in the next patch.

There's no requirement for these things to be sequential. I thought it was better to put adding the clocks sequential, rather than have a random addition of a call-back function in the middle of them.

Signature · 4 lines
Lee Jones
Linaro ST-Ericsson Landing Team Lead
Linaro.org ? Open source software for ARM SoCs
Follow Linaro: Facebook | Twitter | Blog
#45 lore raw

Re: [PATCH 19/33] clk: ux500: Add Device Tree support for the PRCMU clock

In reply to Lee Jones
Quoted 2 lines · Lee Jones

This patch enables clocks to be specified from Device Tree via phandles to the "prcmu-clock" node.

(..)

drivers/clk/ux500/u8500_clk.c void u8500_clk_init(u32 clkrst1_base, u32 clkrst2_base, u32 clkrst3_base, · line 57

Quoted 5 lines
drivers/clk/ux500/u8500_clk.c +1 -0
@@ -52,14 +54,17 @@ void u8500_clk_init(u32 clkrst1_base, u32 clkrst2_base, u32 clkrst3_base,
        clk = clk_reg_prcmu_gate("soc0_pll", NULL, PRCMU_PLLSOC0,
                                CLK_IS_ROOT|CLK_IGNORE_UNUSED);
        clk_register_clkdev(clk, "soc0_pll", NULL);
+       prcmu_clk[PRCMU_PLLSOC0] = clk;

Same comment on all these.

No clk_register_clkdev() on the DT boot path please.

Yours,
Linus Walleij

author
#46 lore raw

Re: [PATCH 19/33] clk: ux500: Add Device Tree support for the PRCMU clock

Quoted 13 lines · Linus Walleij
Quoted 2 lines · Lee Jones

This patch enables clocks to be specified from Device Tree via phandles to the "prcmu-clock" node.

(..)

drivers/clk/ux500/u8500_clk.c void u8500_clk_init(u32 clkrst1_base, u32 clkrst2_base, u32 clkrst3_base, · line 57

Quoted 5 lines
drivers/clk/ux500/u8500_clk.c +1 -0
@@ -52,14 +54,17 @@ void u8500_clk_init(u32 clkrst1_base, u32 clkrst2_base, u32 clkrst3_base,
        clk = clk_reg_prcmu_gate("soc0_pll", NULL, PRCMU_PLLSOC0,
                                CLK_IS_ROOT|CLK_IGNORE_UNUSED);
        clk_register_clkdev(clk, "soc0_pll", NULL);
+       prcmu_clk[PRCMU_PLLSOC0] = clk;

Same comment on all these.

Quoted 1 line

No clk_register_clkdev() on the DT boot path please.

I'll fix this during my ATAG removal patch-set.

Signature · 4 lines
Lee Jones
Linaro ST-Ericsson Landing Team Lead
Linaro.org ? Open source software for ARM SoCs
Follow Linaro: Facebook | Twitter | Blog
author
#47 lore raw

PATCH 20/33 clk: ux500: Add Device Tree support for the PRCC Peripheral clock

In reply to Lee Jones

This patch enables clocks to be specified from Device Tree via phandles to the "prcc-periph-clock" node.

Cc Mike Turquette <[email protected]>
Cc Ulf Hansson <[email protected]>
---
 drivers/clk/ux500/u8500_clk.c |   57 +++++++++++++++++++++++++++++++++++++++++
 1 file changed, 57 insertions(+)
drivers/clk/ux500/u8500_clk.c +57 -0
index 4f5ad4c..86de1a7 100644
@@ -15,7 +15,18 @@
 #include <linux/platform_data/clk-ux500.h>
 #include "clk.h"
 
+#define PRCC_NUM_PERIPH_CLUSTERS 6
+#define PRCC_PERIPHS_PER_CLUSTER 32
+
 static struct clk *prcmu_clk[PRCMU_NUM_CLKS];
+static struct clk *prcc_pclk[(PRCC_NUM_PERIPH_CLUSTERS + 1) * PRCC_PERIPHS_PER_CLUSTER];
+
+#define PRCC_SHOW(clk, base, bit) \
+	clk[(base * PRCC_PERIPHS_PER_CLUSTER) + bit]
+#define PRCC_PCLK_STORE(clk, base, bit)	\
+	prcc_pclk[(base * PRCC_PERIPHS_PER_CLUSTER) + bit] = clk
+#define PRCC_KCLK_STORE(clk, base, bit)	\
+	prcc_kclk[(base * PRCC_PERIPHS_PER_CLUSTER) + bit] = clk
 
 struct clk *ux500_twocell_get(struct of_phandle_args *clkspec, void *data)
 {
@@ -285,143 +296,176 @@ void u8500_clk_init(u32 clkrst1_base, u32 clkrst2_base, u32 clkrst3_base,
 	clk = clk_reg_prcc_pclk("p1_pclk0", "per1clk", clkrst1_base,
 				BIT(0), 0);
 	clk_register_clkdev(clk, "apb_pclk", "uart0");
+	PRCC_PCLK_STORE(clk, 1, 0);
 
 	clk = clk_reg_prcc_pclk("p1_pclk1", "per1clk", clkrst1_base,
 				BIT(1), 0);
 	clk_register_clkdev(clk, "apb_pclk", "uart1");
+	PRCC_PCLK_STORE(clk, 1, 1);
 
 	clk = clk_reg_prcc_pclk("p1_pclk2", "per1clk", clkrst1_base,
 				BIT(2), 0);
 	clk_register_clkdev(clk, "apb_pclk", "nmk-i2c.1");
+	PRCC_PCLK_STORE(clk, 1, 2);
 
 	clk = clk_reg_prcc_pclk("p1_pclk3", "per1clk", clkrst1_base,
 				BIT(3), 0);
 	clk_register_clkdev(clk, "apb_pclk", "msp0");
 	clk_register_clkdev(clk, "apb_pclk", "ux500-msp-i2s.0");
+	PRCC_PCLK_STORE(clk, 1, 3);
 
 	clk = clk_reg_prcc_pclk("p1_pclk4", "per1clk", clkrst1_base,
 				BIT(4), 0);
 	clk_register_clkdev(clk, "apb_pclk", "msp1");
 	clk_register_clkdev(clk, "apb_pclk", "ux500-msp-i2s.1");
+	PRCC_PCLK_STORE(clk, 1, 4);
 
 	clk = clk_reg_prcc_pclk("p1_pclk5", "per1clk", clkrst1_base,
 				BIT(5), 0);
 	clk_register_clkdev(clk, "apb_pclk", "sdi0");
+	PRCC_PCLK_STORE(clk, 1, 5);
 
 	clk = clk_reg_prcc_pclk("p1_pclk6", "per1clk", clkrst1_base,
 				BIT(6), 0);
 	clk_register_clkdev(clk, "apb_pclk", "nmk-i2c.2");
+	PRCC_PCLK_STORE(clk, 1, 6);
 
 	clk = clk_reg_prcc_pclk("p1_pclk7", "per1clk", clkrst1_base,
 				BIT(7), 0);
 	clk_register_clkdev(clk, NULL, "spi3");
+	PRCC_PCLK_STORE(clk, 1, 7);
 
 	clk = clk_reg_prcc_pclk("p1_pclk8", "per1clk", clkrst1_base,
 				BIT(8), 0);
 	clk_register_clkdev(clk, "apb_pclk", "slimbus0");
+	PRCC_PCLK_STORE(clk, 1, 8);
 
 	clk = clk_reg_prcc_pclk("p1_pclk9", "per1clk", clkrst1_base,
 				BIT(9), 0);
 	clk_register_clkdev(clk, NULL, "gpio.0");
 	clk_register_clkdev(clk, NULL, "gpio.1");
 	clk_register_clkdev(clk, NULL, "gpioblock0");
+	PRCC_PCLK_STORE(clk, 1, 9);
 
 	clk = clk_reg_prcc_pclk("p1_pclk10", "per1clk", clkrst1_base,
 				BIT(10), 0);
 	clk_register_clkdev(clk, "apb_pclk", "nmk-i2c.4");
+	PRCC_PCLK_STORE(clk, 1, 10);
 
 	clk = clk_reg_prcc_pclk("p1_pclk11", "per1clk", clkrst1_base,
 				BIT(11), 0);
 	clk_register_clkdev(clk, "apb_pclk", "msp3");
 	clk_register_clkdev(clk, "apb_pclk", "ux500-msp-i2s.3");
+	PRCC_PCLK_STORE(clk, 1, 11);
 
 	clk = clk_reg_prcc_pclk("p2_pclk0", "per2clk", clkrst2_base,
 				BIT(0), 0);
 	clk_register_clkdev(clk, "apb_pclk", "nmk-i2c.3");
+	PRCC_PCLK_STORE(clk, 2, 0);
 
 	clk = clk_reg_prcc_pclk("p2_pclk1", "per2clk", clkrst2_base,
 				BIT(1), 0);
 	clk_register_clkdev(clk, NULL, "spi2");
+	PRCC_PCLK_STORE(clk, 2, 1);
 
 	clk = clk_reg_prcc_pclk("p2_pclk2", "per2clk", clkrst2_base,
 				BIT(2), 0);
 	clk_register_clkdev(clk, NULL, "spi1");
+	PRCC_PCLK_STORE(clk, 2, 2);
 
 	clk = clk_reg_prcc_pclk("p2_pclk3", "per2clk", clkrst2_base,
 				BIT(3), 0);
 	clk_register_clkdev(clk, NULL, "pwl");
+	PRCC_PCLK_STORE(clk, 2, 3);
 
 	clk = clk_reg_prcc_pclk("p2_pclk4", "per2clk", clkrst2_base,
 				BIT(4), 0);
 	clk_register_clkdev(clk, "apb_pclk", "sdi4");
+	PRCC_PCLK_STORE(clk, 2, 4);
 
 	clk = clk_reg_prcc_pclk("p2_pclk5", "per2clk", clkrst2_base,
 				BIT(5), 0);
 	clk_register_clkdev(clk, "apb_pclk", "msp2");
 	clk_register_clkdev(clk, "apb_pclk", "ux500-msp-i2s.2");
+	PRCC_PCLK_STORE(clk, 2, 5);
 
 	clk = clk_reg_prcc_pclk("p2_pclk6", "per2clk", clkrst2_base,
 				BIT(6), 0);
 	clk_register_clkdev(clk, "apb_pclk", "sdi1");
+	PRCC_PCLK_STORE(clk, 2, 6);
 
 	clk = clk_reg_prcc_pclk("p2_pclk7", "per2clk", clkrst2_base,
 				BIT(7), 0);
 	clk_register_clkdev(clk, "apb_pclk", "sdi3");
+	PRCC_PCLK_STORE(clk, 2, 7);
 
 	clk = clk_reg_prcc_pclk("p2_pclk8", "per2clk", clkrst2_base,
 				BIT(8), 0);
 	clk_register_clkdev(clk, NULL, "spi0");
+	PRCC_PCLK_STORE(clk, 2, 8);
 
 	clk = clk_reg_prcc_pclk("p2_pclk9", "per2clk", clkrst2_base,
 				BIT(9), 0);
 	clk_register_clkdev(clk, "hsir_hclk", "ste_hsi.0");
+	PRCC_PCLK_STORE(clk, 2, 9);
 
 	clk = clk_reg_prcc_pclk("p2_pclk10", "per2clk", clkrst2_base,
 				BIT(10), 0);
 	clk_register_clkdev(clk, "hsit_hclk", "ste_hsi.0");
+	PRCC_PCLK_STORE(clk, 2, 10);
 
 	clk = clk_reg_prcc_pclk("p2_pclk11", "per2clk", clkrst2_base,
 				BIT(11), 0);
 	clk_register_clkdev(clk, NULL, "gpio.6");
 	clk_register_clkdev(clk, NULL, "gpio.7");
 	clk_register_clkdev(clk, NULL, "gpioblock1");
+	PRCC_PCLK_STORE(clk, 2, 1);
 
 	clk = clk_reg_prcc_pclk("p2_pclk12", "per2clk", clkrst2_base,
 				BIT(12), 0);
+	PRCC_PCLK_STORE(clk, 2, 12);
 
 	clk = clk_reg_prcc_pclk("p3_pclk0", "per3clk", clkrst3_base,
 				BIT(0), 0);
 	clk_register_clkdev(clk, "fsmc", NULL);
 	clk_register_clkdev(clk, NULL, "smsc911x");
+	PRCC_PCLK_STORE(clk, 3, 0);
 
 	clk = clk_reg_prcc_pclk("p3_pclk1", "per3clk", clkrst3_base,
 				BIT(1), 0);
 	clk_register_clkdev(clk, "apb_pclk", "ssp0");
+	PRCC_PCLK_STORE(clk, 3, 1);
 
 	clk = clk_reg_prcc_pclk("p3_pclk2", "per3clk", clkrst3_base,
 				BIT(2), 0);
 	clk_register_clkdev(clk, "apb_pclk", "ssp1");
+	PRCC_PCLK_STORE(clk, 3, 2);
 
 	clk = clk_reg_prcc_pclk("p3_pclk3", "per3clk", clkrst3_base,
 				BIT(3), 0);
 	clk_register_clkdev(clk, "apb_pclk", "nmk-i2c.0");
+	PRCC_PCLK_STORE(clk, 3, 3);
 
 	clk = clk_reg_prcc_pclk("p3_pclk4", "per3clk", clkrst3_base,
 				BIT(4), 0);
 	clk_register_clkdev(clk, "apb_pclk", "sdi2");
+	PRCC_PCLK_STORE(clk, 3, 4);
 
 	clk = clk_reg_prcc_pclk("p3_pclk5", "per3clk", clkrst3_base,
 				BIT(5), 0);
 	clk_register_clkdev(clk, "apb_pclk", "ske");
 	clk_register_clkdev(clk, "apb_pclk", "nmk-ske-keypad");
+	PRCC_PCLK_STORE(clk, 3, 5);
 
 	clk = clk_reg_prcc_pclk("p3_pclk6", "per3clk", clkrst3_base,
 				BIT(6), 0);
 	clk_register_clkdev(clk, "apb_pclk", "uart2");
+	PRCC_PCLK_STORE(clk, 3, 6);
 
 	clk = clk_reg_prcc_pclk("p3_pclk7", "per3clk", clkrst3_base,
 				BIT(7), 0);
 	clk_register_clkdev(clk, "apb_pclk", "sdi5");
+	PRCC_PCLK_STORE(clk, 3, 7);
 
 	clk = clk_reg_prcc_pclk("p3_pclk8", "per3clk", clkrst3_base,
 				BIT(8), 0);
@@ -430,48 +474,59 @@ void u8500_clk_init(u32 clkrst1_base, u32 clkrst2_base, u32 clkrst3_base,
 	clk_register_clkdev(clk, NULL, "gpio.4");
 	clk_register_clkdev(clk, NULL, "gpio.5");
 	clk_register_clkdev(clk, NULL, "gpioblock2");
+	PRCC_PCLK_STORE(clk, 3, 8);
 
 	clk = clk_reg_prcc_pclk("p5_pclk0", "per5clk", clkrst5_base,
 				BIT(0), 0);
 	clk_register_clkdev(clk, "usb", "musb-ux500.0");
+	PRCC_PCLK_STORE(clk, 5, 0);
 
 	clk = clk_reg_prcc_pclk("p5_pclk1", "per5clk", clkrst5_base,
 				BIT(1), 0);
 	clk_register_clkdev(clk, NULL, "gpio.8");
 	clk_register_clkdev(clk, NULL, "gpioblock3");
+	PRCC_PCLK_STORE(clk, 5, 1);
 
 	clk = clk_reg_prcc_pclk("p6_pclk0", "per6clk", clkrst6_base,
 				BIT(0), 0);
 	clk_register_clkdev(clk, "apb_pclk", "rng");
+	PRCC_PCLK_STORE(clk, 6, 0);
 
 	clk = clk_reg_prcc_pclk("p6_pclk1", "per6clk", clkrst6_base,
 				BIT(1), 0);
 	clk_register_clkdev(clk, NULL, "cryp0");
 	clk_register_clkdev(clk, NULL, "cryp1");
+	PRCC_PCLK_STORE(clk, 6, 1);
 
 	clk = clk_reg_prcc_pclk("p6_pclk2", "per6clk", clkrst6_base,
 				BIT(2), 0);
 	clk_register_clkdev(clk, NULL, "hash0");
+	PRCC_PCLK_STORE(clk, 6, 2);
 
 	clk = clk_reg_prcc_pclk("p6_pclk3", "per6clk", clkrst6_base,
 				BIT(3), 0);
 	clk_register_clkdev(clk, NULL, "pka");
+	PRCC_PCLK_STORE(clk, 6, 3);
 
 	clk = clk_reg_prcc_pclk("p6_pclk4", "per6clk", clkrst6_base,
 				BIT(4), 0);
 	clk_register_clkdev(clk, NULL, "hash1");
+	PRCC_PCLK_STORE(clk, 6, 4);
 
 	clk = clk_reg_prcc_pclk("p6_pclk5", "per6clk", clkrst6_base,
 				BIT(5), 0);
 	clk_register_clkdev(clk, NULL, "cfgreg");
+	PRCC_PCLK_STORE(clk, 6, 5);
 
 	clk = clk_reg_prcc_pclk("p6_pclk6", "per6clk", clkrst6_base,
 				BIT(6), 0);
 	clk_register_clkdev(clk, "apb_pclk", "mtu0");
+	PRCC_PCLK_STORE(clk, 6, 6);
 
 	clk = clk_reg_prcc_pclk("p6_pclk7", "per6clk", clkrst6_base,
 				BIT(7), 0);
 	clk_register_clkdev(clk, "apb_pclk", "mtu1");
+	PRCC_PCLK_STORE(clk, 6, 7);
 
 	/* PRCC K-clocks
 	 *
@@ -603,5 +658,7 @@ void u8500_clk_init(u32 clkrst1_base, u32 clkrst2_base, u32 clkrst3_base,
 			clk_data.clk_num = ARRAY_SIZE(prcmu_clk);
 			of_clk_add_provider(child, of_clk_src_onecell_get, &clk_data);
 		}
+		if (!of_node_cmp(child->name, "prcc-periph-clock"))
+			of_clk_add_provider(child, ux500_twocell_get, prcc_pclk);
 	}
 }
Signature · 1 line
1.7.10.4
author
#48 lore raw

PATCH v2 20/33 clk: ux500: Add Device Tree support for the PRCC Peripheral clock

This patch enables clocks to be specified from Device Tree via phandles to the "prcc-periph-clock" node.

Cc Mike Turquette <[email protected]>
Cc Ulf Hansson <[email protected]>
drivers/clk/ux500/u8500_clk.c +52 -0
index c8ebd19..ff02a26 100644
@@ -15,10 +15,16 @@
 #include <linux/platform_data/clk-ux500.h>
 #include "clk.h"
 
+#define PRCC_NUM_PERIPH_CLUSTERS 6
+#define PRCC_PERIPHS_PER_CLUSTER 32
+
 static struct clk *prcmu_clk[PRCMU_NUM_CLKS];
+static struct clk *prcc_pclk[(PRCC_NUM_PERIPH_CLUSTERS + 1) * PRCC_PERIPHS_PER_CLUSTER];
 
 #define PRCC_SHOW(clk, base, bit) \
 	clk[(base * PRCC_PERIPHS_PER_CLUSTER) + bit]
+#define PRCC_PCLK_STORE(clk, base, bit)	\
+	prcc_pclk[(base * PRCC_PERIPHS_PER_CLUSTER) + bit] = clk
 
 struct clk *ux500_twocell_get(struct of_phandle_args *clkspec, void *data)
 {
@@ -288,143 +294,176 @@ void u8500_clk_init(u32 clkrst1_base, u32 clkrst2_base, u32 clkrst3_base,
 	clk = clk_reg_prcc_pclk("p1_pclk0", "per1clk", clkrst1_base,
 				BIT(0), 0);
 	clk_register_clkdev(clk, "apb_pclk", "uart0");
+	PRCC_PCLK_STORE(clk, 1, 0);
 
 	clk = clk_reg_prcc_pclk("p1_pclk1", "per1clk", clkrst1_base,
 				BIT(1), 0);
 	clk_register_clkdev(clk, "apb_pclk", "uart1");
+	PRCC_PCLK_STORE(clk, 1, 1);
 
 	clk = clk_reg_prcc_pclk("p1_pclk2", "per1clk", clkrst1_base,
 				BIT(2), 0);
 	clk_register_clkdev(clk, "apb_pclk", "nmk-i2c.1");
+	PRCC_PCLK_STORE(clk, 1, 2);
 
 	clk = clk_reg_prcc_pclk("p1_pclk3", "per1clk", clkrst1_base,
 				BIT(3), 0);
 	clk_register_clkdev(clk, "apb_pclk", "msp0");
 	clk_register_clkdev(clk, "apb_pclk", "ux500-msp-i2s.0");
+	PRCC_PCLK_STORE(clk, 1, 3);
 
 	clk = clk_reg_prcc_pclk("p1_pclk4", "per1clk", clkrst1_base,
 				BIT(4), 0);
 	clk_register_clkdev(clk, "apb_pclk", "msp1");
 	clk_register_clkdev(clk, "apb_pclk", "ux500-msp-i2s.1");
+	PRCC_PCLK_STORE(clk, 1, 4);
 
 	clk = clk_reg_prcc_pclk("p1_pclk5", "per1clk", clkrst1_base,
 				BIT(5), 0);
 	clk_register_clkdev(clk, "apb_pclk", "sdi0");
+	PRCC_PCLK_STORE(clk, 1, 5);
 
 	clk = clk_reg_prcc_pclk("p1_pclk6", "per1clk", clkrst1_base,
 				BIT(6), 0);
 	clk_register_clkdev(clk, "apb_pclk", "nmk-i2c.2");
+	PRCC_PCLK_STORE(clk, 1, 6);
 
 	clk = clk_reg_prcc_pclk("p1_pclk7", "per1clk", clkrst1_base,
 				BIT(7), 0);
 	clk_register_clkdev(clk, NULL, "spi3");
+	PRCC_PCLK_STORE(clk, 1, 7);
 
 	clk = clk_reg_prcc_pclk("p1_pclk8", "per1clk", clkrst1_base,
 				BIT(8), 0);
 	clk_register_clkdev(clk, "apb_pclk", "slimbus0");
+	PRCC_PCLK_STORE(clk, 1, 8);
 
 	clk = clk_reg_prcc_pclk("p1_pclk9", "per1clk", clkrst1_base,
 				BIT(9), 0);
 	clk_register_clkdev(clk, NULL, "gpio.0");
 	clk_register_clkdev(clk, NULL, "gpio.1");
 	clk_register_clkdev(clk, NULL, "gpioblock0");
+	PRCC_PCLK_STORE(clk, 1, 9);
 
 	clk = clk_reg_prcc_pclk("p1_pclk10", "per1clk", clkrst1_base,
 				BIT(10), 0);
 	clk_register_clkdev(clk, "apb_pclk", "nmk-i2c.4");
+	PRCC_PCLK_STORE(clk, 1, 10);
 
 	clk = clk_reg_prcc_pclk("p1_pclk11", "per1clk", clkrst1_base,
 				BIT(11), 0);
 	clk_register_clkdev(clk, "apb_pclk", "msp3");
 	clk_register_clkdev(clk, "apb_pclk", "ux500-msp-i2s.3");
+	PRCC_PCLK_STORE(clk, 1, 11);
 
 	clk = clk_reg_prcc_pclk("p2_pclk0", "per2clk", clkrst2_base,
 				BIT(0), 0);
 	clk_register_clkdev(clk, "apb_pclk", "nmk-i2c.3");
+	PRCC_PCLK_STORE(clk, 2, 0);
 
 	clk = clk_reg_prcc_pclk("p2_pclk1", "per2clk", clkrst2_base,
 				BIT(1), 0);
 	clk_register_clkdev(clk, NULL, "spi2");
+	PRCC_PCLK_STORE(clk, 2, 1);
 
 	clk = clk_reg_prcc_pclk("p2_pclk2", "per2clk", clkrst2_base,
 				BIT(2), 0);
 	clk_register_clkdev(clk, NULL, "spi1");
+	PRCC_PCLK_STORE(clk, 2, 2);
 
 	clk = clk_reg_prcc_pclk("p2_pclk3", "per2clk", clkrst2_base,
 				BIT(3), 0);
 	clk_register_clkdev(clk, NULL, "pwl");
+	PRCC_PCLK_STORE(clk, 2, 3);
 
 	clk = clk_reg_prcc_pclk("p2_pclk4", "per2clk", clkrst2_base,
 				BIT(4), 0);
 	clk_register_clkdev(clk, "apb_pclk", "sdi4");
+	PRCC_PCLK_STORE(clk, 2, 4);
 
 	clk = clk_reg_prcc_pclk("p2_pclk5", "per2clk", clkrst2_base,
 				BIT(5), 0);
 	clk_register_clkdev(clk, "apb_pclk", "msp2");
 	clk_register_clkdev(clk, "apb_pclk", "ux500-msp-i2s.2");
+	PRCC_PCLK_STORE(clk, 2, 5);
 
 	clk = clk_reg_prcc_pclk("p2_pclk6", "per2clk", clkrst2_base,
 				BIT(6), 0);
 	clk_register_clkdev(clk, "apb_pclk", "sdi1");
+	PRCC_PCLK_STORE(clk, 2, 6);
 
 	clk = clk_reg_prcc_pclk("p2_pclk7", "per2clk", clkrst2_base,
 				BIT(7), 0);
 	clk_register_clkdev(clk, "apb_pclk", "sdi3");
+	PRCC_PCLK_STORE(clk, 2, 7);
 
 	clk = clk_reg_prcc_pclk("p2_pclk8", "per2clk", clkrst2_base,
 				BIT(8), 0);
 	clk_register_clkdev(clk, NULL, "spi0");
+	PRCC_PCLK_STORE(clk, 2, 8);
 
 	clk = clk_reg_prcc_pclk("p2_pclk9", "per2clk", clkrst2_base,
 				BIT(9), 0);
 	clk_register_clkdev(clk, "hsir_hclk", "ste_hsi.0");
+	PRCC_PCLK_STORE(clk, 2, 9);
 
 	clk = clk_reg_prcc_pclk("p2_pclk10", "per2clk", clkrst2_base,
 				BIT(10), 0);
 	clk_register_clkdev(clk, "hsit_hclk", "ste_hsi.0");
+	PRCC_PCLK_STORE(clk, 2, 10);
 
 	clk = clk_reg_prcc_pclk("p2_pclk11", "per2clk", clkrst2_base,
 				BIT(11), 0);
 	clk_register_clkdev(clk, NULL, "gpio.6");
 	clk_register_clkdev(clk, NULL, "gpio.7");
 	clk_register_clkdev(clk, NULL, "gpioblock1");
+	PRCC_PCLK_STORE(clk, 2, 1);
 
 	clk = clk_reg_prcc_pclk("p2_pclk12", "per2clk", clkrst2_base,
 				BIT(12), 0);
+	PRCC_PCLK_STORE(clk, 2, 12);
 
 	clk = clk_reg_prcc_pclk("p3_pclk0", "per3clk", clkrst3_base,
 				BIT(0), 0);
 	clk_register_clkdev(clk, "fsmc", NULL);
 	clk_register_clkdev(clk, NULL, "smsc911x");
+	PRCC_PCLK_STORE(clk, 3, 0);
 
 	clk = clk_reg_prcc_pclk("p3_pclk1", "per3clk", clkrst3_base,
 				BIT(1), 0);
 	clk_register_clkdev(clk, "apb_pclk", "ssp0");
+	PRCC_PCLK_STORE(clk, 3, 1);
 
 	clk = clk_reg_prcc_pclk("p3_pclk2", "per3clk", clkrst3_base,
 				BIT(2), 0);
 	clk_register_clkdev(clk, "apb_pclk", "ssp1");
+	PRCC_PCLK_STORE(clk, 3, 2);
 
 	clk = clk_reg_prcc_pclk("p3_pclk3", "per3clk", clkrst3_base,
 				BIT(3), 0);
 	clk_register_clkdev(clk, "apb_pclk", "nmk-i2c.0");
+	PRCC_PCLK_STORE(clk, 3, 3);
 
 	clk = clk_reg_prcc_pclk("p3_pclk4", "per3clk", clkrst3_base,
 				BIT(4), 0);
 	clk_register_clkdev(clk, "apb_pclk", "sdi2");
+	PRCC_PCLK_STORE(clk, 3, 4);
 
 	clk = clk_reg_prcc_pclk("p3_pclk5", "per3clk", clkrst3_base,
 				BIT(5), 0);
 	clk_register_clkdev(clk, "apb_pclk", "ske");
 	clk_register_clkdev(clk, "apb_pclk", "nmk-ske-keypad");
+	PRCC_PCLK_STORE(clk, 3, 5);
 
 	clk = clk_reg_prcc_pclk("p3_pclk6", "per3clk", clkrst3_base,
 				BIT(6), 0);
 	clk_register_clkdev(clk, "apb_pclk", "uart2");
+	PRCC_PCLK_STORE(clk, 3, 6);
 
 	clk = clk_reg_prcc_pclk("p3_pclk7", "per3clk", clkrst3_base,
 				BIT(7), 0);
 	clk_register_clkdev(clk, "apb_pclk", "sdi5");
+	PRCC_PCLK_STORE(clk, 3, 7);
 
 	clk = clk_reg_prcc_pclk("p3_pclk8", "per3clk", clkrst3_base,
 				BIT(8), 0);
@@ -433,48 +472,59 @@ void u8500_clk_init(u32 clkrst1_base, u32 clkrst2_base, u32 clkrst3_base,
 	clk_register_clkdev(clk, NULL, "gpio.4");
 	clk_register_clkdev(clk, NULL, "gpio.5");
 	clk_register_clkdev(clk, NULL, "gpioblock2");
+	PRCC_PCLK_STORE(clk, 3, 8);
 
 	clk = clk_reg_prcc_pclk("p5_pclk0", "per5clk", clkrst5_base,
 				BIT(0), 0);
 	clk_register_clkdev(clk, "usb", "musb-ux500.0");
+	PRCC_PCLK_STORE(clk, 5, 0);
 
 	clk = clk_reg_prcc_pclk("p5_pclk1", "per5clk", clkrst5_base,
 				BIT(1), 0);
 	clk_register_clkdev(clk, NULL, "gpio.8");
 	clk_register_clkdev(clk, NULL, "gpioblock3");
+	PRCC_PCLK_STORE(clk, 5, 1);
 
 	clk = clk_reg_prcc_pclk("p6_pclk0", "per6clk", clkrst6_base,
 				BIT(0), 0);
 	clk_register_clkdev(clk, "apb_pclk", "rng");
+	PRCC_PCLK_STORE(clk, 6, 0);
 
 	clk = clk_reg_prcc_pclk("p6_pclk1", "per6clk", clkrst6_base,
 				BIT(1), 0);
 	clk_register_clkdev(clk, NULL, "cryp0");
 	clk_register_clkdev(clk, NULL, "cryp1");
+	PRCC_PCLK_STORE(clk, 6, 1);
 
 	clk = clk_reg_prcc_pclk("p6_pclk2", "per6clk", clkrst6_base,
 				BIT(2), 0);
 	clk_register_clkdev(clk, NULL, "hash0");
+	PRCC_PCLK_STORE(clk, 6, 2);
 
 	clk = clk_reg_prcc_pclk("p6_pclk3", "per6clk", clkrst6_base,
 				BIT(3), 0);
 	clk_register_clkdev(clk, NULL, "pka");
+	PRCC_PCLK_STORE(clk, 6, 3);
 
 	clk = clk_reg_prcc_pclk("p6_pclk4", "per6clk", clkrst6_base,
 				BIT(4), 0);
 	clk_register_clkdev(clk, NULL, "hash1");
+	PRCC_PCLK_STORE(clk, 6, 4);
 
 	clk = clk_reg_prcc_pclk("p6_pclk5", "per6clk", clkrst6_base,
 				BIT(5), 0);
 	clk_register_clkdev(clk, NULL, "cfgreg");
+	PRCC_PCLK_STORE(clk, 6, 5);
 
 	clk = clk_reg_prcc_pclk("p6_pclk6", "per6clk", clkrst6_base,
 				BIT(6), 0);
 	clk_register_clkdev(clk, "apb_pclk", "mtu0");
+	PRCC_PCLK_STORE(clk, 6, 6);
 
 	clk = clk_reg_prcc_pclk("p6_pclk7", "per6clk", clkrst6_base,
 				BIT(7), 0);
 	clk_register_clkdev(clk, "apb_pclk", "mtu1");
+	PRCC_PCLK_STORE(clk, 6, 7);
 
 	/* PRCC K-clocks
 	 *
@@ -606,5 +656,7 @@ void u8500_clk_init(u32 clkrst1_base, u32 clkrst2_base, u32 clkrst3_base,
 			clk_data.clk_num = ARRAY_SIZE(prcmu_clk);
 			of_clk_add_provider(child, of_clk_src_onecell_get, &clk_data);
 		}
+		if (!of_node_cmp(child->name, "prcc-periph-clock"))
+			of_clk_add_provider(child, ux500_twocell_get, prcc_pclk);
 	}
 }
author
#49 lore raw

PATCH 21/33 clk: ux500: Add Device Tree support for the PRCC Kernel clock

In reply to Lee Jones

This patch enables clocks to be specified from Device Tree via phandles to the "prcc-kernel-clock" node.

Cc Mike Turquette <[email protected]>
Cc Ulf Hansson <[email protected]>
---
 drivers/clk/ux500/u8500_clk.c |   30 ++++++++++++++++++++++++++++++
 1 file changed, 30 insertions(+)
drivers/clk/ux500/u8500_clk.c +30 -0
index 86de1a7..c3ed09c 100644
@@ -20,6 +20,7 @@
 
 static struct clk *prcmu_clk[PRCMU_NUM_CLKS];
 static struct clk *prcc_pclk[(PRCC_NUM_PERIPH_CLUSTERS + 1) * PRCC_PERIPHS_PER_CLUSTER];
+static struct clk *prcc_kclk[(PRCC_NUM_PERIPH_CLUSTERS + 1) * PRCC_PERIPHS_PER_CLUSTER];
 
 #define PRCC_SHOW(clk, base, bit) \
 	clk[(base * PRCC_PERIPHS_PER_CLUSTER) + bit]
@@ -540,110 +541,136 @@ void u8500_clk_init(u32 clkrst1_base, u32 clkrst2_base, u32 clkrst3_base,
 	clk = clk_reg_prcc_kclk("p1_uart0_kclk", "uartclk",
 			clkrst1_base, BIT(0), CLK_SET_RATE_GATE);
 	clk_register_clkdev(clk, NULL, "uart0");
+	PRCC_KCLK_STORE(clk, 1, 0);
 
 	clk = clk_reg_prcc_kclk("p1_uart1_kclk", "uartclk",
 			clkrst1_base, BIT(1), CLK_SET_RATE_GATE);
 	clk_register_clkdev(clk, NULL, "uart1");
+	PRCC_KCLK_STORE(clk, 1, 1);
 
 	clk = clk_reg_prcc_kclk("p1_i2c1_kclk", "i2cclk",
 			clkrst1_base, BIT(2), CLK_SET_RATE_GATE);
 	clk_register_clkdev(clk, NULL, "nmk-i2c.1");
+	PRCC_KCLK_STORE(clk, 1, 2);
 
 	clk = clk_reg_prcc_kclk("p1_msp0_kclk", "msp02clk",
 			clkrst1_base, BIT(3), CLK_SET_RATE_GATE);
 	clk_register_clkdev(clk, NULL, "msp0");
 	clk_register_clkdev(clk, NULL, "ux500-msp-i2s.0");
+	PRCC_KCLK_STORE(clk, 1, 3);
 
 	clk = clk_reg_prcc_kclk("p1_msp1_kclk", "msp1clk",
 			clkrst1_base, BIT(4), CLK_SET_RATE_GATE);
 	clk_register_clkdev(clk, NULL, "msp1");
 	clk_register_clkdev(clk, NULL, "ux500-msp-i2s.1");
+	PRCC_KCLK_STORE(clk, 1, 4);
 
 	clk = clk_reg_prcc_kclk("p1_sdi0_kclk", "sdmmcclk",
 			clkrst1_base, BIT(5), CLK_SET_RATE_GATE);
 	clk_register_clkdev(clk, NULL, "sdi0");
+	PRCC_KCLK_STORE(clk, 1, 5);
 
 	clk = clk_reg_prcc_kclk("p1_i2c2_kclk", "i2cclk",
 			clkrst1_base, BIT(6), CLK_SET_RATE_GATE);
 	clk_register_clkdev(clk, NULL, "nmk-i2c.2");
+	PRCC_KCLK_STORE(clk, 1, 6);
 
 	clk = clk_reg_prcc_kclk("p1_slimbus0_kclk", "slimclk",
 			clkrst1_base, BIT(8), CLK_SET_RATE_GATE);
 	clk_register_clkdev(clk, NULL, "slimbus0");
+	PRCC_KCLK_STORE(clk, 1, 8);
 
 	clk = clk_reg_prcc_kclk("p1_i2c4_kclk", "i2cclk",
 			clkrst1_base, BIT(9), CLK_SET_RATE_GATE);
 	clk_register_clkdev(clk, NULL, "nmk-i2c.4");
+	PRCC_KCLK_STORE(clk, 1, 9);
 
 	clk = clk_reg_prcc_kclk("p1_msp3_kclk", "msp1clk",
 			clkrst1_base, BIT(10), CLK_SET_RATE_GATE);
 	clk_register_clkdev(clk, NULL, "msp3");
 	clk_register_clkdev(clk, NULL, "ux500-msp-i2s.3");
+	PRCC_KCLK_STORE(clk, 1, 10);
 
 	/* Periph2 */
 	clk = clk_reg_prcc_kclk("p2_i2c3_kclk", "i2cclk",
 			clkrst2_base, BIT(0), CLK_SET_RATE_GATE);
 	clk_register_clkdev(clk, NULL, "nmk-i2c.3");
+	PRCC_KCLK_STORE(clk, 2, 0);
 
 	clk = clk_reg_prcc_kclk("p2_sdi4_kclk", "sdmmcclk",
 			clkrst2_base, BIT(2), CLK_SET_RATE_GATE);
 	clk_register_clkdev(clk, NULL, "sdi4");
+	PRCC_KCLK_STORE(clk, 2, 2);
 
 	clk = clk_reg_prcc_kclk("p2_msp2_kclk", "msp02clk",
 			clkrst2_base, BIT(3), CLK_SET_RATE_GATE);
 	clk_register_clkdev(clk, NULL, "msp2");
 	clk_register_clkdev(clk, NULL, "ux500-msp-i2s.2");
+	PRCC_KCLK_STORE(clk, 2, 3);
 
 	clk = clk_reg_prcc_kclk("p2_sdi1_kclk", "sdmmcclk",
 			clkrst2_base, BIT(4), CLK_SET_RATE_GATE);
 	clk_register_clkdev(clk, NULL, "sdi1");
+	PRCC_KCLK_STORE(clk, 2, 4);
 
 	clk = clk_reg_prcc_kclk("p2_sdi3_kclk", "sdmmcclk",
 			clkrst2_base, BIT(5), CLK_SET_RATE_GATE);
 	clk_register_clkdev(clk, NULL, "sdi3");
+	PRCC_KCLK_STORE(clk, 2, 5);
 
 	/* Note that rate is received from parent. */
 	clk = clk_reg_prcc_kclk("p2_ssirx_kclk", "hsirxclk",
 			clkrst2_base, BIT(6),
 			CLK_SET_RATE_GATE|CLK_SET_RATE_PARENT);
+	PRCC_KCLK_STORE(clk, 2, 6);
+
 	clk = clk_reg_prcc_kclk("p2_ssitx_kclk", "hsitxclk",
 			clkrst2_base, BIT(7),
 			CLK_SET_RATE_GATE|CLK_SET_RATE_PARENT);
+	PRCC_KCLK_STORE(clk, 2, 7);
 
 	/* Periph3 */
 	clk = clk_reg_prcc_kclk("p3_ssp0_kclk", "sspclk",
 			clkrst3_base, BIT(1), CLK_SET_RATE_GATE);
 	clk_register_clkdev(clk, NULL, "ssp0");
+	PRCC_KCLK_STORE(clk, 3, 1);
 
 	clk = clk_reg_prcc_kclk("p3_ssp1_kclk", "sspclk",
 			clkrst3_base, BIT(2), CLK_SET_RATE_GATE);
 	clk_register_clkdev(clk, NULL, "ssp1");
+	PRCC_KCLK_STORE(clk, 3, 2);
 
 	clk = clk_reg_prcc_kclk("p3_i2c0_kclk", "i2cclk",
 			clkrst3_base, BIT(3), CLK_SET_RATE_GATE);
 	clk_register_clkdev(clk, NULL, "nmk-i2c.0");
+	PRCC_KCLK_STORE(clk, 3, 3);
 
 	clk = clk_reg_prcc_kclk("p3_sdi2_kclk", "sdmmcclk",
 			clkrst3_base, BIT(4), CLK_SET_RATE_GATE);
 	clk_register_clkdev(clk, NULL, "sdi2");
+	PRCC_KCLK_STORE(clk, 3, 4);
 
 	clk = clk_reg_prcc_kclk("p3_ske_kclk", "rtc32k",
 			clkrst3_base, BIT(5), CLK_SET_RATE_GATE);
 	clk_register_clkdev(clk, NULL, "ske");
 	clk_register_clkdev(clk, NULL, "nmk-ske-keypad");
+	PRCC_KCLK_STORE(clk, 3, 5);
 
 	clk = clk_reg_prcc_kclk("p3_uart2_kclk", "uartclk",
 			clkrst3_base, BIT(6), CLK_SET_RATE_GATE);
 	clk_register_clkdev(clk, NULL, "uart2");
+	PRCC_KCLK_STORE(clk, 3, 6);
 
 	clk = clk_reg_prcc_kclk("p3_sdi5_kclk", "sdmmcclk",
 			clkrst3_base, BIT(7), CLK_SET_RATE_GATE);
 	clk_register_clkdev(clk, NULL, "sdi5");
+	PRCC_KCLK_STORE(clk, 3, 7);
 
 	/* Periph6 */
 	clk = clk_reg_prcc_kclk("p3_rng_kclk", "rngclk",
 			clkrst6_base, BIT(0), CLK_SET_RATE_GATE);
 	clk_register_clkdev(clk, NULL, "rng");
+	PRCC_KCLK_STORE(clk, 6, 0);
 
 	if (of_have_populated_dt())
 		np = of_find_matching_node(NULL, u8500_clk_of_match);
@@ -660,5 +687,8 @@ void u8500_clk_init(u32 clkrst1_base, u32 clkrst2_base, u32 clkrst3_base,
 		}
 		if (!of_node_cmp(child->name, "prcc-periph-clock"))
 			of_clk_add_provider(child, ux500_twocell_get, prcc_pclk);
+
+		if (!of_node_cmp(child->name, "prcc-kernel-clock"))
+			of_clk_add_provider(child, ux500_twocell_get, prcc_kclk);
 	}
 }
Signature · 1 line
1.7.10.4
#50 lore raw

Re: [PATCH 21/33] clk: ux500: Add Device Tree support for the PRCC Kernel clock

Quoted 6 lines · Lee Jones

This patch enables clocks to be specified from Device Tree via phandles to the "prcc-kernel-clock" node.

Cc Mike Turquette <[email protected]>
Cc Ulf Hansson <[email protected]>

Could you please fold this patch into a "common PRCC device tree support" patch instead. Thus handling both P and K clocks in the same patch.

That would probaby solve the missmatch of the macro definitions as well. Like were did the PRCC_KCLK_STORE come from?

Kind regards
Ulf Hansson

Quoted 165 lines
---
 drivers/clk/ux500/u8500_clk.c |   30 ++++++++++++++++++++++++++++++
 1 file changed, 30 insertions(+)
drivers/clk/ux500/u8500_clk.c +30 -1
index 86de1a7..c3ed09c 100644
@@ -20,6 +20,7 @@
 
 static struct clk *prcmu_clk[PRCMU_NUM_CLKS];
 static struct clk *prcc_pclk[(PRCC_NUM_PERIPH_CLUSTERS + 1) * PRCC_PERIPHS_PER_CLUSTER];
+static struct clk *prcc_kclk[(PRCC_NUM_PERIPH_CLUSTERS + 1) * PRCC_PERIPHS_PER_CLUSTER];
 
 #define PRCC_SHOW(clk, base, bit) \
        clk[(base * PRCC_PERIPHS_PER_CLUSTER) + bit]
@@ -540,110 +541,136 @@ void u8500_clk_init(u32 clkrst1_base, u32 clkrst2_base, u32 clkrst3_base,
        clk = clk_reg_prcc_kclk("p1_uart0_kclk", "uartclk",
                        clkrst1_base, BIT(0), CLK_SET_RATE_GATE);
        clk_register_clkdev(clk, NULL, "uart0");
+       PRCC_KCLK_STORE(clk, 1, 0);
 
        clk = clk_reg_prcc_kclk("p1_uart1_kclk", "uartclk",
                        clkrst1_base, BIT(1), CLK_SET_RATE_GATE);
        clk_register_clkdev(clk, NULL, "uart1");
+       PRCC_KCLK_STORE(clk, 1, 1);
 
        clk = clk_reg_prcc_kclk("p1_i2c1_kclk", "i2cclk",
                        clkrst1_base, BIT(2), CLK_SET_RATE_GATE);
        clk_register_clkdev(clk, NULL, "nmk-i2c.1");
+       PRCC_KCLK_STORE(clk, 1, 2);
 
        clk = clk_reg_prcc_kclk("p1_msp0_kclk", "msp02clk",
                        clkrst1_base, BIT(3), CLK_SET_RATE_GATE);
        clk_register_clkdev(clk, NULL, "msp0");
        clk_register_clkdev(clk, NULL, "ux500-msp-i2s.0");
+       PRCC_KCLK_STORE(clk, 1, 3);
 
        clk = clk_reg_prcc_kclk("p1_msp1_kclk", "msp1clk",
                        clkrst1_base, BIT(4), CLK_SET_RATE_GATE);
        clk_register_clkdev(clk, NULL, "msp1");
        clk_register_clkdev(clk, NULL, "ux500-msp-i2s.1");
+       PRCC_KCLK_STORE(clk, 1, 4);
 
        clk = clk_reg_prcc_kclk("p1_sdi0_kclk", "sdmmcclk",
                        clkrst1_base, BIT(5), CLK_SET_RATE_GATE);
        clk_register_clkdev(clk, NULL, "sdi0");
+       PRCC_KCLK_STORE(clk, 1, 5);
 
        clk = clk_reg_prcc_kclk("p1_i2c2_kclk", "i2cclk",
                        clkrst1_base, BIT(6), CLK_SET_RATE_GATE);
        clk_register_clkdev(clk, NULL, "nmk-i2c.2");
+       PRCC_KCLK_STORE(clk, 1, 6);
 
        clk = clk_reg_prcc_kclk("p1_slimbus0_kclk", "slimclk",
                        clkrst1_base, BIT(8), CLK_SET_RATE_GATE);
        clk_register_clkdev(clk, NULL, "slimbus0");
+       PRCC_KCLK_STORE(clk, 1, 8);
 
        clk = clk_reg_prcc_kclk("p1_i2c4_kclk", "i2cclk",
                        clkrst1_base, BIT(9), CLK_SET_RATE_GATE);
        clk_register_clkdev(clk, NULL, "nmk-i2c.4");
+       PRCC_KCLK_STORE(clk, 1, 9);
 
        clk = clk_reg_prcc_kclk("p1_msp3_kclk", "msp1clk",
                        clkrst1_base, BIT(10), CLK_SET_RATE_GATE);
        clk_register_clkdev(clk, NULL, "msp3");
        clk_register_clkdev(clk, NULL, "ux500-msp-i2s.3");
+       PRCC_KCLK_STORE(clk, 1, 10);
 
        /* Periph2 */
        clk = clk_reg_prcc_kclk("p2_i2c3_kclk", "i2cclk",
                        clkrst2_base, BIT(0), CLK_SET_RATE_GATE);
        clk_register_clkdev(clk, NULL, "nmk-i2c.3");
+       PRCC_KCLK_STORE(clk, 2, 0);
 
        clk = clk_reg_prcc_kclk("p2_sdi4_kclk", "sdmmcclk",
                        clkrst2_base, BIT(2), CLK_SET_RATE_GATE);
        clk_register_clkdev(clk, NULL, "sdi4");
+       PRCC_KCLK_STORE(clk, 2, 2);
 
        clk = clk_reg_prcc_kclk("p2_msp2_kclk", "msp02clk",
                        clkrst2_base, BIT(3), CLK_SET_RATE_GATE);
        clk_register_clkdev(clk, NULL, "msp2");
        clk_register_clkdev(clk, NULL, "ux500-msp-i2s.2");
+       PRCC_KCLK_STORE(clk, 2, 3);
 
        clk = clk_reg_prcc_kclk("p2_sdi1_kclk", "sdmmcclk",
                        clkrst2_base, BIT(4), CLK_SET_RATE_GATE);
        clk_register_clkdev(clk, NULL, "sdi1");
+       PRCC_KCLK_STORE(clk, 2, 4);
 
        clk = clk_reg_prcc_kclk("p2_sdi3_kclk", "sdmmcclk",
                        clkrst2_base, BIT(5), CLK_SET_RATE_GATE);
        clk_register_clkdev(clk, NULL, "sdi3");
+       PRCC_KCLK_STORE(clk, 2, 5);
 
        /* Note that rate is received from parent. */
        clk = clk_reg_prcc_kclk("p2_ssirx_kclk", "hsirxclk",
                        clkrst2_base, BIT(6),
                        CLK_SET_RATE_GATE|CLK_SET_RATE_PARENT);
+       PRCC_KCLK_STORE(clk, 2, 6);
+
        clk = clk_reg_prcc_kclk("p2_ssitx_kclk", "hsitxclk",
                        clkrst2_base, BIT(7),
                        CLK_SET_RATE_GATE|CLK_SET_RATE_PARENT);
+       PRCC_KCLK_STORE(clk, 2, 7);
 
        /* Periph3 */
        clk = clk_reg_prcc_kclk("p3_ssp0_kclk", "sspclk",
                        clkrst3_base, BIT(1), CLK_SET_RATE_GATE);
        clk_register_clkdev(clk, NULL, "ssp0");
+       PRCC_KCLK_STORE(clk, 3, 1);
 
        clk = clk_reg_prcc_kclk("p3_ssp1_kclk", "sspclk",
                        clkrst3_base, BIT(2), CLK_SET_RATE_GATE);
        clk_register_clkdev(clk, NULL, "ssp1");
+       PRCC_KCLK_STORE(clk, 3, 2);
 
        clk = clk_reg_prcc_kclk("p3_i2c0_kclk", "i2cclk",
                        clkrst3_base, BIT(3), CLK_SET_RATE_GATE);
        clk_register_clkdev(clk, NULL, "nmk-i2c.0");
+       PRCC_KCLK_STORE(clk, 3, 3);
 
        clk = clk_reg_prcc_kclk("p3_sdi2_kclk", "sdmmcclk",
                        clkrst3_base, BIT(4), CLK_SET_RATE_GATE);
        clk_register_clkdev(clk, NULL, "sdi2");
+       PRCC_KCLK_STORE(clk, 3, 4);
 
        clk = clk_reg_prcc_kclk("p3_ske_kclk", "rtc32k",
                        clkrst3_base, BIT(5), CLK_SET_RATE_GATE);
        clk_register_clkdev(clk, NULL, "ske");
        clk_register_clkdev(clk, NULL, "nmk-ske-keypad");
+       PRCC_KCLK_STORE(clk, 3, 5);
 
        clk = clk_reg_prcc_kclk("p3_uart2_kclk", "uartclk",
                        clkrst3_base, BIT(6), CLK_SET_RATE_GATE);
        clk_register_clkdev(clk, NULL, "uart2");
+       PRCC_KCLK_STORE(clk, 3, 6);
 
        clk = clk_reg_prcc_kclk("p3_sdi5_kclk", "sdmmcclk",
                        clkrst3_base, BIT(7), CLK_SET_RATE_GATE);
        clk_register_clkdev(clk, NULL, "sdi5");
+       PRCC_KCLK_STORE(clk, 3, 7);
 
        /* Periph6 */
        clk = clk_reg_prcc_kclk("p3_rng_kclk", "rngclk",
                        clkrst6_base, BIT(0), CLK_SET_RATE_GATE);
        clk_register_clkdev(clk, NULL, "rng");
+       PRCC_KCLK_STORE(clk, 6, 0);
 
        if (of_have_populated_dt())
                np = of_find_matching_node(NULL, u8500_clk_of_match);
@@ -660,5 +687,8 @@ void u8500_clk_init(u32 clkrst1_base, u32 clkrst2_base, u32 clkrst3_base,
                }
                if (!of_node_cmp(child->name, "prcc-periph-clock"))
                        of_clk_add_provider(child, ux500_twocell_get, prcc_pclk);
+
+               if (!of_node_cmp(child->name, "prcc-kernel-clock"))
+                       of_clk_add_provider(child, ux500_twocell_get, prcc_kclk);
        }
 }
--

1.7.10.4

author
#51 lore raw

Re: [PATCH 21/33] clk: ux500: Add Device Tree support for the PRCC Kernel clock

Quoted 10 lines
Quoted 6 lines

This patch enables clocks to be specified from Device Tree via phandles to the "prcc-kernel-clock" node.

Cc Mike Turquette <[email protected]>
Cc Ulf Hansson <[email protected]>

Could you please fold this patch into a "common PRCC device tree support" patch instead. Thus handling both P and K clocks in the same patch.

I'm really not a fan of creating massive patches, so I'd really rather not.

Quoted 2 lines

That would probaby solve the missmatch of the macro definitions as well. Like were did the PRCC_KCLK_STORE come from?

I do see your point though, I will move PRCC_KCLK_STORE() into this patch.

Quoted 163 lines
Quoted 163 lines
---
 drivers/clk/ux500/u8500_clk.c |   30 ++++++++++++++++++++++++++++++
 1 file changed, 30 insertions(+)
drivers/clk/ux500/u8500_clk.c +30 -0
index 86de1a7..c3ed09c 100644
@@ -20,6 +20,7 @@
 
 static struct clk *prcmu_clk[PRCMU_NUM_CLKS];
 static struct clk *prcc_pclk[(PRCC_NUM_PERIPH_CLUSTERS + 1) * PRCC_PERIPHS_PER_CLUSTER];
+static struct clk *prcc_kclk[(PRCC_NUM_PERIPH_CLUSTERS + 1) * PRCC_PERIPHS_PER_CLUSTER];
 
 #define PRCC_SHOW(clk, base, bit) \
        clk[(base * PRCC_PERIPHS_PER_CLUSTER) + bit]
@@ -540,110 +541,136 @@ void u8500_clk_init(u32 clkrst1_base, u32 clkrst2_base, u32 clkrst3_base,
        clk = clk_reg_prcc_kclk("p1_uart0_kclk", "uartclk",
                        clkrst1_base, BIT(0), CLK_SET_RATE_GATE);
        clk_register_clkdev(clk, NULL, "uart0");
+       PRCC_KCLK_STORE(clk, 1, 0);
 
        clk = clk_reg_prcc_kclk("p1_uart1_kclk", "uartclk",
                        clkrst1_base, BIT(1), CLK_SET_RATE_GATE);
        clk_register_clkdev(clk, NULL, "uart1");
+       PRCC_KCLK_STORE(clk, 1, 1);
 
        clk = clk_reg_prcc_kclk("p1_i2c1_kclk", "i2cclk",
                        clkrst1_base, BIT(2), CLK_SET_RATE_GATE);
        clk_register_clkdev(clk, NULL, "nmk-i2c.1");
+       PRCC_KCLK_STORE(clk, 1, 2);
 
        clk = clk_reg_prcc_kclk("p1_msp0_kclk", "msp02clk",
                        clkrst1_base, BIT(3), CLK_SET_RATE_GATE);
        clk_register_clkdev(clk, NULL, "msp0");
        clk_register_clkdev(clk, NULL, "ux500-msp-i2s.0");
+       PRCC_KCLK_STORE(clk, 1, 3);
 
        clk = clk_reg_prcc_kclk("p1_msp1_kclk", "msp1clk",
                        clkrst1_base, BIT(4), CLK_SET_RATE_GATE);
        clk_register_clkdev(clk, NULL, "msp1");
        clk_register_clkdev(clk, NULL, "ux500-msp-i2s.1");
+       PRCC_KCLK_STORE(clk, 1, 4);
 
        clk = clk_reg_prcc_kclk("p1_sdi0_kclk", "sdmmcclk",
                        clkrst1_base, BIT(5), CLK_SET_RATE_GATE);
        clk_register_clkdev(clk, NULL, "sdi0");
+       PRCC_KCLK_STORE(clk, 1, 5);
 
        clk = clk_reg_prcc_kclk("p1_i2c2_kclk", "i2cclk",
                        clkrst1_base, BIT(6), CLK_SET_RATE_GATE);
        clk_register_clkdev(clk, NULL, "nmk-i2c.2");
+       PRCC_KCLK_STORE(clk, 1, 6);
 
        clk = clk_reg_prcc_kclk("p1_slimbus0_kclk", "slimclk",
                        clkrst1_base, BIT(8), CLK_SET_RATE_GATE);
        clk_register_clkdev(clk, NULL, "slimbus0");
+       PRCC_KCLK_STORE(clk, 1, 8);
 
        clk = clk_reg_prcc_kclk("p1_i2c4_kclk", "i2cclk",
                        clkrst1_base, BIT(9), CLK_SET_RATE_GATE);
        clk_register_clkdev(clk, NULL, "nmk-i2c.4");
+       PRCC_KCLK_STORE(clk, 1, 9);
 
        clk = clk_reg_prcc_kclk("p1_msp3_kclk", "msp1clk",
                        clkrst1_base, BIT(10), CLK_SET_RATE_GATE);
        clk_register_clkdev(clk, NULL, "msp3");
        clk_register_clkdev(clk, NULL, "ux500-msp-i2s.3");
+       PRCC_KCLK_STORE(clk, 1, 10);
 
        /* Periph2 */
        clk = clk_reg_prcc_kclk("p2_i2c3_kclk", "i2cclk",
                        clkrst2_base, BIT(0), CLK_SET_RATE_GATE);
        clk_register_clkdev(clk, NULL, "nmk-i2c.3");
+       PRCC_KCLK_STORE(clk, 2, 0);
 
        clk = clk_reg_prcc_kclk("p2_sdi4_kclk", "sdmmcclk",
                        clkrst2_base, BIT(2), CLK_SET_RATE_GATE);
        clk_register_clkdev(clk, NULL, "sdi4");
+       PRCC_KCLK_STORE(clk, 2, 2);
 
        clk = clk_reg_prcc_kclk("p2_msp2_kclk", "msp02clk",
                        clkrst2_base, BIT(3), CLK_SET_RATE_GATE);
        clk_register_clkdev(clk, NULL, "msp2");
        clk_register_clkdev(clk, NULL, "ux500-msp-i2s.2");
+       PRCC_KCLK_STORE(clk, 2, 3);
 
        clk = clk_reg_prcc_kclk("p2_sdi1_kclk", "sdmmcclk",
                        clkrst2_base, BIT(4), CLK_SET_RATE_GATE);
        clk_register_clkdev(clk, NULL, "sdi1");
+       PRCC_KCLK_STORE(clk, 2, 4);
 
        clk = clk_reg_prcc_kclk("p2_sdi3_kclk", "sdmmcclk",
                        clkrst2_base, BIT(5), CLK_SET_RATE_GATE);
        clk_register_clkdev(clk, NULL, "sdi3");
+       PRCC_KCLK_STORE(clk, 2, 5);
 
        /* Note that rate is received from parent. */
        clk = clk_reg_prcc_kclk("p2_ssirx_kclk", "hsirxclk",
                        clkrst2_base, BIT(6),
                        CLK_SET_RATE_GATE|CLK_SET_RATE_PARENT);
+       PRCC_KCLK_STORE(clk, 2, 6);
+
        clk = clk_reg_prcc_kclk("p2_ssitx_kclk", "hsitxclk",
                        clkrst2_base, BIT(7),
                        CLK_SET_RATE_GATE|CLK_SET_RATE_PARENT);
+       PRCC_KCLK_STORE(clk, 2, 7);
 
        /* Periph3 */
        clk = clk_reg_prcc_kclk("p3_ssp0_kclk", "sspclk",
                        clkrst3_base, BIT(1), CLK_SET_RATE_GATE);
        clk_register_clkdev(clk, NULL, "ssp0");
+       PRCC_KCLK_STORE(clk, 3, 1);
 
        clk = clk_reg_prcc_kclk("p3_ssp1_kclk", "sspclk",
                        clkrst3_base, BIT(2), CLK_SET_RATE_GATE);
        clk_register_clkdev(clk, NULL, "ssp1");
+       PRCC_KCLK_STORE(clk, 3, 2);
 
        clk = clk_reg_prcc_kclk("p3_i2c0_kclk", "i2cclk",
                        clkrst3_base, BIT(3), CLK_SET_RATE_GATE);
        clk_register_clkdev(clk, NULL, "nmk-i2c.0");
+       PRCC_KCLK_STORE(clk, 3, 3);
 
        clk = clk_reg_prcc_kclk("p3_sdi2_kclk", "sdmmcclk",
                        clkrst3_base, BIT(4), CLK_SET_RATE_GATE);
        clk_register_clkdev(clk, NULL, "sdi2");
+       PRCC_KCLK_STORE(clk, 3, 4);
 
        clk = clk_reg_prcc_kclk("p3_ske_kclk", "rtc32k",
                        clkrst3_base, BIT(5), CLK_SET_RATE_GATE);
        clk_register_clkdev(clk, NULL, "ske");
        clk_register_clkdev(clk, NULL, "nmk-ske-keypad");
+       PRCC_KCLK_STORE(clk, 3, 5);
 
        clk = clk_reg_prcc_kclk("p3_uart2_kclk", "uartclk",
                        clkrst3_base, BIT(6), CLK_SET_RATE_GATE);
        clk_register_clkdev(clk, NULL, "uart2");
+       PRCC_KCLK_STORE(clk, 3, 6);
 
        clk = clk_reg_prcc_kclk("p3_sdi5_kclk", "sdmmcclk",
                        clkrst3_base, BIT(7), CLK_SET_RATE_GATE);
        clk_register_clkdev(clk, NULL, "sdi5");
+       PRCC_KCLK_STORE(clk, 3, 7);
 
        /* Periph6 */
        clk = clk_reg_prcc_kclk("p3_rng_kclk", "rngclk",
                        clkrst6_base, BIT(0), CLK_SET_RATE_GATE);
        clk_register_clkdev(clk, NULL, "rng");
+       PRCC_KCLK_STORE(clk, 6, 0);
 
        if (of_have_populated_dt())
                np = of_find_matching_node(NULL, u8500_clk_of_match);
@@ -660,5 +687,8 @@ void u8500_clk_init(u32 clkrst1_base, u32 clkrst2_base, u32 clkrst3_base,
                }
                if (!of_node_cmp(child->name, "prcc-periph-clock"))
                        of_clk_add_provider(child, ux500_twocell_get, prcc_pclk);
+
+               if (!of_node_cmp(child->name, "prcc-kernel-clock"))
+                       of_clk_add_provider(child, ux500_twocell_get, prcc_kclk);
        }
 }
Signature · 4 lines
Lee Jones
Linaro ST-Ericsson Landing Team Lead
Linaro.org ? Open source software for ARM SoCs
Follow Linaro: Facebook | Twitter | Blog
author
#52 lore raw

PATCH v2 21/33 clk: ux500: Add Device Tree support for the PRCC Kernel clock

In reply to Lee Jones

This patch enables clocks to be specified from Device Tree via phandles to the "prcc-kernel-clock" node.

Cc Mike Turquette <[email protected]>
Cc Ulf Hansson <[email protected]>
drivers/clk/ux500/u8500_clk.c +32 -0
index ff02a26..c9ac350 100644
@@ -20,11 +20,14 @@
 
 static struct clk *prcmu_clk[PRCMU_NUM_CLKS];
 static struct clk *prcc_pclk[(PRCC_NUM_PERIPH_CLUSTERS + 1) * PRCC_PERIPHS_PER_CLUSTER];
+static struct clk *prcc_kclk[(PRCC_NUM_PERIPH_CLUSTERS + 1) * PRCC_PERIPHS_PER_CLUSTER];
 
 #define PRCC_SHOW(clk, base, bit) \
 	clk[(base * PRCC_PERIPHS_PER_CLUSTER) + bit]
 #define PRCC_PCLK_STORE(clk, base, bit)	\
 	prcc_pclk[(base * PRCC_PERIPHS_PER_CLUSTER) + bit] = clk
+#define PRCC_KCLK_STORE(clk, base, bit)        \
+	prcc_kclk[(base * PRCC_PERIPHS_PER_CLUSTER) + bit] = clk
 
 struct clk *ux500_twocell_get(struct of_phandle_args *clkspec, void *data)
 {
@@ -538,110 +541,136 @@ void u8500_clk_init(u32 clkrst1_base, u32 clkrst2_base, u32 clkrst3_base,
 	clk = clk_reg_prcc_kclk("p1_uart0_kclk", "uartclk",
 			clkrst1_base, BIT(0), CLK_SET_RATE_GATE);
 	clk_register_clkdev(clk, NULL, "uart0");
+	PRCC_KCLK_STORE(clk, 1, 0);
 
 	clk = clk_reg_prcc_kclk("p1_uart1_kclk", "uartclk",
 			clkrst1_base, BIT(1), CLK_SET_RATE_GATE);
 	clk_register_clkdev(clk, NULL, "uart1");
+	PRCC_KCLK_STORE(clk, 1, 1);
 
 	clk = clk_reg_prcc_kclk("p1_i2c1_kclk", "i2cclk",
 			clkrst1_base, BIT(2), CLK_SET_RATE_GATE);
 	clk_register_clkdev(clk, NULL, "nmk-i2c.1");
+	PRCC_KCLK_STORE(clk, 1, 2);
 
 	clk = clk_reg_prcc_kclk("p1_msp0_kclk", "msp02clk",
 			clkrst1_base, BIT(3), CLK_SET_RATE_GATE);
 	clk_register_clkdev(clk, NULL, "msp0");
 	clk_register_clkdev(clk, NULL, "ux500-msp-i2s.0");
+	PRCC_KCLK_STORE(clk, 1, 3);
 
 	clk = clk_reg_prcc_kclk("p1_msp1_kclk", "msp1clk",
 			clkrst1_base, BIT(4), CLK_SET_RATE_GATE);
 	clk_register_clkdev(clk, NULL, "msp1");
 	clk_register_clkdev(clk, NULL, "ux500-msp-i2s.1");
+	PRCC_KCLK_STORE(clk, 1, 4);
 
 	clk = clk_reg_prcc_kclk("p1_sdi0_kclk", "sdmmcclk",
 			clkrst1_base, BIT(5), CLK_SET_RATE_GATE);
 	clk_register_clkdev(clk, NULL, "sdi0");
+	PRCC_KCLK_STORE(clk, 1, 5);
 
 	clk = clk_reg_prcc_kclk("p1_i2c2_kclk", "i2cclk",
 			clkrst1_base, BIT(6), CLK_SET_RATE_GATE);
 	clk_register_clkdev(clk, NULL, "nmk-i2c.2");
+	PRCC_KCLK_STORE(clk, 1, 6);
 
 	clk = clk_reg_prcc_kclk("p1_slimbus0_kclk", "slimclk",
 			clkrst1_base, BIT(8), CLK_SET_RATE_GATE);
 	clk_register_clkdev(clk, NULL, "slimbus0");
+	PRCC_KCLK_STORE(clk, 1, 8);
 
 	clk = clk_reg_prcc_kclk("p1_i2c4_kclk", "i2cclk",
 			clkrst1_base, BIT(9), CLK_SET_RATE_GATE);
 	clk_register_clkdev(clk, NULL, "nmk-i2c.4");
+	PRCC_KCLK_STORE(clk, 1, 9);
 
 	clk = clk_reg_prcc_kclk("p1_msp3_kclk", "msp1clk",
 			clkrst1_base, BIT(10), CLK_SET_RATE_GATE);
 	clk_register_clkdev(clk, NULL, "msp3");
 	clk_register_clkdev(clk, NULL, "ux500-msp-i2s.3");
+	PRCC_KCLK_STORE(clk, 1, 10);
 
 	/* Periph2 */
 	clk = clk_reg_prcc_kclk("p2_i2c3_kclk", "i2cclk",
 			clkrst2_base, BIT(0), CLK_SET_RATE_GATE);
 	clk_register_clkdev(clk, NULL, "nmk-i2c.3");
+	PRCC_KCLK_STORE(clk, 2, 0);
 
 	clk = clk_reg_prcc_kclk("p2_sdi4_kclk", "sdmmcclk",
 			clkrst2_base, BIT(2), CLK_SET_RATE_GATE);
 	clk_register_clkdev(clk, NULL, "sdi4");
+	PRCC_KCLK_STORE(clk, 2, 2);
 
 	clk = clk_reg_prcc_kclk("p2_msp2_kclk", "msp02clk",
 			clkrst2_base, BIT(3), CLK_SET_RATE_GATE);
 	clk_register_clkdev(clk, NULL, "msp2");
 	clk_register_clkdev(clk, NULL, "ux500-msp-i2s.2");
+	PRCC_KCLK_STORE(clk, 2, 3);
 
 	clk = clk_reg_prcc_kclk("p2_sdi1_kclk", "sdmmcclk",
 			clkrst2_base, BIT(4), CLK_SET_RATE_GATE);
 	clk_register_clkdev(clk, NULL, "sdi1");
+	PRCC_KCLK_STORE(clk, 2, 4);
 
 	clk = clk_reg_prcc_kclk("p2_sdi3_kclk", "sdmmcclk",
 			clkrst2_base, BIT(5), CLK_SET_RATE_GATE);
 	clk_register_clkdev(clk, NULL, "sdi3");
+	PRCC_KCLK_STORE(clk, 2, 5);
 
 	/* Note that rate is received from parent. */
 	clk = clk_reg_prcc_kclk("p2_ssirx_kclk", "hsirxclk",
 			clkrst2_base, BIT(6),
 			CLK_SET_RATE_GATE|CLK_SET_RATE_PARENT);
+	PRCC_KCLK_STORE(clk, 2, 6);
+
 	clk = clk_reg_prcc_kclk("p2_ssitx_kclk", "hsitxclk",
 			clkrst2_base, BIT(7),
 			CLK_SET_RATE_GATE|CLK_SET_RATE_PARENT);
+	PRCC_KCLK_STORE(clk, 2, 7);
 
 	/* Periph3 */
 	clk = clk_reg_prcc_kclk("p3_ssp0_kclk", "sspclk",
 			clkrst3_base, BIT(1), CLK_SET_RATE_GATE);
 	clk_register_clkdev(clk, NULL, "ssp0");
+	PRCC_KCLK_STORE(clk, 3, 1);
 
 	clk = clk_reg_prcc_kclk("p3_ssp1_kclk", "sspclk",
 			clkrst3_base, BIT(2), CLK_SET_RATE_GATE);
 	clk_register_clkdev(clk, NULL, "ssp1");
+	PRCC_KCLK_STORE(clk, 3, 2);
 
 	clk = clk_reg_prcc_kclk("p3_i2c0_kclk", "i2cclk",
 			clkrst3_base, BIT(3), CLK_SET_RATE_GATE);
 	clk_register_clkdev(clk, NULL, "nmk-i2c.0");
+	PRCC_KCLK_STORE(clk, 3, 3);
 
 	clk = clk_reg_prcc_kclk("p3_sdi2_kclk", "sdmmcclk",
 			clkrst3_base, BIT(4), CLK_SET_RATE_GATE);
 	clk_register_clkdev(clk, NULL, "sdi2");
+	PRCC_KCLK_STORE(clk, 3, 4);
 
 	clk = clk_reg_prcc_kclk("p3_ske_kclk", "rtc32k",
 			clkrst3_base, BIT(5), CLK_SET_RATE_GATE);
 	clk_register_clkdev(clk, NULL, "ske");
 	clk_register_clkdev(clk, NULL, "nmk-ske-keypad");
+	PRCC_KCLK_STORE(clk, 3, 5);
 
 	clk = clk_reg_prcc_kclk("p3_uart2_kclk", "uartclk",
 			clkrst3_base, BIT(6), CLK_SET_RATE_GATE);
 	clk_register_clkdev(clk, NULL, "uart2");
+	PRCC_KCLK_STORE(clk, 3, 6);
 
 	clk = clk_reg_prcc_kclk("p3_sdi5_kclk", "sdmmcclk",
 			clkrst3_base, BIT(7), CLK_SET_RATE_GATE);
 	clk_register_clkdev(clk, NULL, "sdi5");
+	PRCC_KCLK_STORE(clk, 3, 7);
 
 	/* Periph6 */
 	clk = clk_reg_prcc_kclk("p3_rng_kclk", "rngclk",
 			clkrst6_base, BIT(0), CLK_SET_RATE_GATE);
 	clk_register_clkdev(clk, NULL, "rng");
+	PRCC_KCLK_STORE(clk, 6, 0);
 
 	if (of_have_populated_dt())
 		np = of_find_matching_node(NULL, u8500_clk_of_match);
@@ -658,5 +687,8 @@ void u8500_clk_init(u32 clkrst1_base, u32 clkrst2_base, u32 clkrst3_base,
 		}
 		if (!of_node_cmp(child->name, "prcc-periph-clock"))
 			of_clk_add_provider(child, ux500_twocell_get, prcc_pclk);
+
+		if (!of_node_cmp(child->name, "prcc-kernel-clock"))
+			of_clk_add_provider(child, ux500_twocell_get, prcc_kclk);
 	}
 }
#53 lore raw

Re: [PATCH 21/33 v2] clk: ux500: Add Device Tree support for the PRCC Kernel clock

Quoted 7 lines · Lee Jones

This patch enables clocks to be specified from Device Tree via phandles to the "prcc-kernel-clock" node.

Cc Mike Turquette <[email protected]>
Cc Ulf Hansson <[email protected]>

I don't understand the design of the common clock subsystem here, but is it really necessary to hardcode all the clocks using clk_reg_prcc_kclk() here *and* register a clkdev *and* store the pointer in an array, when you can get all that information from the device tree?

Mike?

Arnd

#54 lore raw

Re: [PATCH 21/33 v2] clk: ux500: Add Device Tree support for the PRCC Kernel clock

Quoting Arnd Bergmann (2013-06-12 07:46:30)

Quoted 9 lines · Lee Jones
Quoted 7 lines

This patch enables clocks to be specified from Device Tree via phandles to the "prcc-kernel-clock" node.

Cc Mike Turquette <[email protected]>
Cc Ulf Hansson <[email protected]>
Quoted 6 lines

I don't understand the design of the common clock subsystem here, but is it really necessary to hardcode all the clocks using clk_reg_prcc_kclk() here *and* register a clkdev *and* store the pointer in an array, when you can get all that information from the device tree?

Mike?

I'm a bit confused by what is going on here too. There are several different ways to handle this.

1) Since you have your own clock driver (e.g. not the basic clock types) then you could expand the argument list of clk_reg_prcc_kclk to include the clkdev dev_id string and toss the call to clk_register_clkdev inside of clk_reg_prcc_kclk.

2) Move your clock data into DT and teach the driver to use of_clk_get to fetch the clk phandle directly instead of using the string-matching clkdev mechanisms. Of course both your clock and device bits need to be converted to DT first.

Can you explain what prcc_kclk[] and friends are doing? Is that a legacy clock framework thing that is still hanging around? I'm curious to know why your clock driver needs a list of the clocks it registers.

Regards,
Mike

Quoted 2 lines

Arnd

author
#55 lore raw

Re: [PATCH 21/33 v2] clk: ux500: Add Device Tree support for the PRCC Kernel clock

Quoted 17 lines

Quoting Arnd Bergmann (2013-06-12 07:46:30)

Quoted 9 lines · Lee Jones
Quoted 7 lines

This patch enables clocks to be specified from Device Tree via phandles to the "prcc-kernel-clock" node.

Cc Mike Turquette <[email protected]>
Cc Ulf Hansson <[email protected]>
Quoted 6 lines

I don't understand the design of the common clock subsystem here, but is it really necessary to hardcode all the clocks using clk_reg_prcc_kclk() here *and* register a clkdev *and* store the pointer in an array, when you can get all that information from the device tree?

Mike?

Quoted 7 lines

I'm a bit confused by what is going on here too. There are several different ways to handle this.

1) Since you have your own clock driver (e.g. not the basic clock types) then you could expand the argument list of clk_reg_prcc_kclk to include the clkdev dev_id string and toss the call to clk_register_clkdev inside of clk_reg_prcc_kclk.

Yes, that's actually a pretty good idea. It has nothing to do with this patchset, but I will add it to my TODO.

NB: I am away from tomorrow until after Connect, so I will continue with this after that.

Quoted 4 lines

2) Move your clock data into DT and teach the driver to use of_clk_get to fetch the clk phandle directly instead of using the string-matching clkdev mechanisms. Of course both your clock and device bits need to be converted to DT first.

I'm sure this is the end-goal, but we still have to support the !DT case. At least until all of the ATAGs stuff has been completely removed from platform.

Quoted 3 lines

Can you explain what prcc_kclk[] and friends are doing? Is that a legacy clock framework thing that is still hanging around? I'm curious to know why your clock driver needs a list of the clocks it registers.

Sure.

1. So when we register a clock, we store a pointer to it in a 'struct clk *array[]' using known cell identifying read-ins. For peripheral (pclk) and kernel (kclk) clocks these are peripheral number (the kernel clocks have these too) and their associated 8bit register enable BIT(). The PRCMU clocks are read-in to the array only by their 32bit register enable BIT().

2. We then register with of_clk_add_provider() passing the array using the 'void *data' argument. So:

  clk = clk_reg_prcc_[p|k]clk(blah, blah, blah);
  array[(periph * PRCC_PERIPHS_PER_CLUSTER) + bit] = clk
  of_clk_add_provider(child, ux500_twocell_get, array);

3. In the DTS(I) files we request clocks using their known identifiers by way of tuplets for the kclks and pclks and by a 1 #cell variant for the PRCMU clocks. So:

pclk & kclk:

  /*        pclk/kclk       periph  BIT() */
  clocks = <&prcc_[p|k]clk  1       9>;

PRCMU:
/* prcmu BIT() */
clocks = <&prcmu_clk PRCMU_DMACLK>;

The PRCMU can then use the clk framework supplied of_clk_src_onecell_get() call-back and the pclk and kclks use the 2 #cell variant we provide. They both read into the aforementioned array[]s we populated earlier in the process to fetch the correct 'struct clk*'.

Signature · 4 lines
Lee Jones
Linaro ST-Ericsson Landing Team Lead
Linaro.org ? Open source software for ARM SoCs
Follow Linaro: Facebook | Twitter | Blog
#56 lore raw

PATCH v2 21/33 clk: ux500: Add Device Tree support for the PRCC Kernel clock

Quoting Lee Jones (2013-06-19 00:42:03)

Quoted 72 lines
Quoted 17 lines

Quoting Arnd Bergmann (2013-06-12 07:46:30)

Quoted 9 lines · Lee Jones
Quoted 7 lines

This patch enables clocks to be specified from Device Tree via phandles to the "prcc-kernel-clock" node.

Cc Mike Turquette <[email protected]>
Cc Ulf Hansson <[email protected]>
Quoted 6 lines

I don't understand the design of the common clock subsystem here, but is it really necessary to hardcode all the clocks using clk_reg_prcc_kclk() here *and* register a clkdev *and* store the pointer in an array, when you can get all that information from the device tree?

Mike?

Quoted 7 lines

I'm a bit confused by what is going on here too. There are several different ways to handle this.

1) Since you have your own clock driver (e.g. not the basic clock types) then you could expand the argument list of clk_reg_prcc_kclk to include the clkdev dev_id string and toss the call to clk_register_clkdev inside of clk_reg_prcc_kclk.

Yes, that's actually a pretty good idea. It has nothing to do with this patchset, but I will add it to my TODO.

NB: I am away from tomorrow until after Connect, so I will continue with this after that.

Quoted 4 lines

2) Move your clock data into DT and teach the driver to use of_clk_get to fetch the clk phandle directly instead of using the string-matching clkdev mechanisms. Of course both your clock and device bits need to be converted to DT first.

I'm sure this is the end-goal, but we still have to support the !DT case. At least until all of the ATAGs stuff has been completely removed from platform.

Quoted 3 lines

Can you explain what prcc_kclk[] and friends are doing? Is that a legacy clock framework thing that is still hanging around? I'm curious to know why your clock driver needs a list of the clocks it registers.

Sure.

1. So when we register a clock, we store a pointer to it in a 'struct clk *array[]' using known cell identifying read-ins. For peripheral (pclk) and kernel (kclk) clocks these are peripheral number (the kernel clocks have these too) and their associated 8bit register enable BIT(). The PRCMU clocks are read-in to the array only by their 32bit register enable BIT().

2. We then register with of_clk_add_provider() passing the array using the 'void *data' argument. So:

  clk = clk_reg_prcc_[p|k]clk(blah, blah, blah);
  array[(periph * PRCC_PERIPHS_PER_CLUSTER) + bit] = clk
  of_clk_add_provider(child, ux500_twocell_get, array);

3. In the DTS(I) files we request clocks using their known identifiers by way of tuplets for the kclks and pclks and by a 1 #cell variant for the PRCMU clocks. So:

pclk & kclk:

  /*        pclk/kclk       periph  BIT() */
  clocks = <&prcc_[p|k]clk  1       9>;

PRCMU:
/* prcmu BIT() */
clocks = <&prcmu_clk PRCMU_DMACLK>;

Quoted 5 lines

The PRCMU can then use the clk framework supplied of_clk_src_onecell_get() call-back and the pclk and kclks use the 2 #cell variant we provide. They both read into the aforementioned array[]s we populated earlier in the process to fetch the correct 'struct clk*'.

Ok, so I had to re-read that several times. It's probably not a win for readability but if that is the binding you want then go for it.

For patches #17-23:

Quoted 6 lines
Signature · 5 lines
-- 
Lee Jones
Linaro ST-Ericsson Landing Team Lead
Linaro.org ? Open source software for ARM SoCs
Follow Linaro: Facebook | Twitter | Blog
#57 lore raw

Re: [PATCH 21/33] clk: ux500: Add Device Tree support for the PRCC Kernel clock

In reply to Lee Jones
Quoted 6 lines · Lee Jones

This patch enables clocks to be specified from Device Tree via phandles to the "prcc-kernel-clock" node.

Cc Mike Turquette <[email protected]>
Cc Ulf Hansson <[email protected]>

(...)

Quoted 7 lines

static struct clk *prcmu_clk[PRCMU_NUM_CLKS];
static struct clk *prcc_pclk[(PRCC_NUM_PERIPH_CLUSTERS + 1) * PRCC_PERIPHS_PER_CLUSTER];

drivers/clk/ux500/u8500_clk.c +1 -0
+static struct clk *prcc_kclk[(PRCC_NUM_PERIPH_CLUSTERS + 1) * PRCC_PERIPHS_PER_CLUSTER];
 
 #define PRCC_SHOW(clk, base, bit) \
        clk[(base * PRCC_PERIPHS_PER_CLUSTER) + bit]

drivers/clk/ux500/u8500_clk.c void u8500_clk_init(u32 clkrst1_base, u32 clkrst2_base, u32 clkrst3_base, · line 549

Quoted 9 lines
drivers/clk/ux500/u8500_clk.c +2 -0
@@ -540,110 +541,136 @@ void u8500_clk_init(u32 clkrst1_base, u32 clkrst2_base, u32 clkrst3_base,
        clk = clk_reg_prcc_kclk("p1_uart0_kclk", "uartclk",
                        clkrst1_base, BIT(0), CLK_SET_RATE_GATE);
        clk_register_clkdev(clk, NULL, "uart0");
+       PRCC_KCLK_STORE(clk, 1, 0);
 
        clk = clk_reg_prcc_kclk("p1_uart1_kclk", "uartclk",
                        clkrst1_base, BIT(1), CLK_SET_RATE_GATE);
        clk_register_clkdev(clk, NULL, "uart1");
+       PRCC_KCLK_STORE(clk, 1, 1);

(etc)

On device tree boots, clk_register_clkdev() is obviously pointless, as you're later deleting the AUXDATA connecting that device name to the clock.

This should not be called at all on the DT boot path.

However this looks cluttered as well:

if (dt_probe)
clk_register_clkdev(clk, NULL, "uart0");
else
PRCC_KCLK_STORE(clk, 1, 0);

Isn't it possible to fork a function u8500_clk_init_dt() to add all the clocks in the DT probe path and keep this function u8500_clk_init() as it is?

Yours,
Linus Walleij

author
#58 lore raw

Re: [PATCH 21/33] clk: ux500: Add Device Tree support for the PRCC Kernel clock

Quoted 36 lines · Linus Walleij
Quoted 6 lines · Lee Jones

This patch enables clocks to be specified from Device Tree via phandles to the "prcc-kernel-clock" node.

Cc Mike Turquette <[email protected]>
Cc Ulf Hansson <[email protected]>

(...)

Quoted 7 lines

static struct clk *prcmu_clk[PRCMU_NUM_CLKS];
static struct clk *prcc_pclk[(PRCC_NUM_PERIPH_CLUSTERS + 1) * PRCC_PERIPHS_PER_CLUSTER];

drivers/clk/ux500/u8500_clk.c +1 -0
+static struct clk *prcc_kclk[(PRCC_NUM_PERIPH_CLUSTERS + 1) * PRCC_PERIPHS_PER_CLUSTER];
 
 #define PRCC_SHOW(clk, base, bit) \
        clk[(base * PRCC_PERIPHS_PER_CLUSTER) + bit]

drivers/clk/ux500/u8500_clk.c void u8500_clk_init(u32 clkrst1_base, u32 clkrst2_base, u32 clkrst3_base, · line 549

Quoted 9 lines
drivers/clk/ux500/u8500_clk.c +2 -0
@@ -540,110 +541,136 @@ void u8500_clk_init(u32 clkrst1_base, u32 clkrst2_base, u32 clkrst3_base,
        clk = clk_reg_prcc_kclk("p1_uart0_kclk", "uartclk",
                        clkrst1_base, BIT(0), CLK_SET_RATE_GATE);
        clk_register_clkdev(clk, NULL, "uart0");
+       PRCC_KCLK_STORE(clk, 1, 0);
 
        clk = clk_reg_prcc_kclk("p1_uart1_kclk", "uartclk",
                        clkrst1_base, BIT(1), CLK_SET_RATE_GATE);
        clk_register_clkdev(clk, NULL, "uart1");
+       PRCC_KCLK_STORE(clk, 1, 1);

(etc)

On device tree boots, clk_register_clkdev() is obviously pointless, as you're later deleting the AUXDATA connecting that device name to the clock.

This should not be called at all on the DT boot path.

However this looks cluttered as well:

Quoted 8 lines

if (dt_probe)
clk_register_clkdev(clk, NULL, "uart0");
else
PRCC_KCLK_STORE(clk, 1, 0);

Isn't it possible to fork a function u8500_clk_init_dt() to add all the clocks in the DT probe path and keep this function u8500_clk_init() as it is?

Yes, we probably could do that, but as we're ripping out ATAG booting support from the ux500 platform, I'll just remove the clk_register_clkdev()s during the process.

Signature · 4 lines
Lee Jones
Linaro ST-Ericsson Landing Team Lead
Linaro.org ? Open source software for ARM SoCs
Follow Linaro: Facebook | Twitter | Blog
#59 lore raw

Re: [PATCH 21/33] clk: ux500: Add Device Tree support for the PRCC Kernel clock

Quoted 9 lines · Lee Jones
Quoted 3 lines · Linus Walleij

Isn't it possible to fork a function u8500_clk_init_dt() to add all the clocks in the DT probe path and keep this function u8500_clk_init() as it is?

Yes, we probably could do that, but as we're ripping out ATAG booting support from the ux500 platform, I'll just remove the clk_register_clkdev()s during the process.

I really do not like the approach of uglifying something and then beautifying it later... I prefer each step in isolation to be good looking, or you will be confused when traversing the history.

Yours,
Linus Walleij

author
#60 lore raw

Re: [PATCH 21/33] clk: ux500: Add Device Tree support for the PRCC Kernel clock

Quoted 7 lines · Linus Walleij
Quoted 6 lines · Lee Jones
Quoted 3 lines · Linus Walleij

Isn't it possible to fork a function u8500_clk_init_dt() to add all the clocks in the DT probe path and keep this function u8500_clk_init() as it is?

Quoted 7 lines
Quoted 3 lines

Yes, we probably could do that, but as we're ripping out ATAG booting support from the ux500 platform, I'll just remove the clk_register_clkdev()s during the process.

I really do not like the approach of uglifying something and then beautifying it later... I prefer each step in isolation to be good looking, or you will be confused when traversing the history.

So then we have a few options, some more realistic than others.

1. Duplicate each of the; clk_reg_prcmu_*(), clk_reg_prcc_pclk(), clk_reg_prcc_kclk() calls into your proposed u8500_clk_init_dt(), which, while keeping everything separate would be unrealistic.

2. Move both clk_register_clkdev() and the struct clock arrays into 'drivers/clk/ux500/clk-prcmu.c' and 'drivers/clk/ux500/clk-prcc.c' and make the correct decisions in clk_reg_prcmu() and clk_reg_prcc(). This would be more viable, but would entail splitting the defines and the struct clock arrays (stores), probably creating a little more disparity. It would also mean adding 3 parameters (con_id, dev_fmt and array_index) to each of; clk_reg_prcmu_gate(), clk_reg_prcmu_scalable(), clk_reg_prcmu_opp_gate(), clk_reg_prcmu_scalable_rate(), clk_reg_prcmu_rate(), clk_reg_prcmu_opp_volt_scalable, clk_reg_prcmu() and 4 parameters (con_id, dev_fmt, array_index_x, array_index_y) to each of clk_reg_prcc_pclk(), clk_reg_prcc_kclk() and clk_reg_prcc().

Or, the most viable option:

3. Leave it as it is. All we're doing here is populating an array of pointers. It costs practically nothing, continues legacy ATAG support as well as adding Device Tree support. It's far neater than the other two options, by a long shot. And then, when we're ready to take out ATAG booting support (which I'm working on right now), we just remove the clk_register_clkdev() calls and we're left with the nice and neat macros.

Unless there's a viable 4th option which hasn't popped into my head just yet. I'm all hears. :)

Signature · 4 lines
Lee Jones
Linaro ST-Ericsson Landing Team Lead
Linaro.org ? Open source software for ARM SoCs
Follow Linaro: Facebook | Twitter | Blog
#61 lore raw

Re: [PATCH 21/33] clk: ux500: Add Device Tree support for the PRCC Kernel clock

Quoted 9 lines · Lee Jones
Quoted 3 lines

I really do not like the approach of uglifying something and then beautifying it later... I prefer each step in isolation to be good looking, or you will be confused when traversing the history.

So then we have a few options, some more realistic than others.

1. Duplicate each of the; clk_reg_prcmu_*(), clk_reg_prcc_pclk(), clk_reg_prcc_kclk() calls into your proposed u8500_clk_init_dt(), which, while keeping everything separate would be unrealistic.

I think this is perfectly realistic.

You're not going to duplicate each clk_register_clkdev(), which makes it way smaller than the original function, and since one of the function will be inside a

#ifdef CONFIG_OF
#endif

After we switch the entire platform to DT-only it will be pretty obvious which big chunk of code that needs to go away, it's a clean cut.

(Note: I know the #ifdef CONFIG_OF is not necessary anymore since we switched to multiplatform, but I intend that marker for humans, not machines.)

Yours,
Linus Walleij

#62 lore raw

Re: [PATCH 21/33] clk: ux500: Add Device Tree support for the PRCC Kernel clock

Quoted 9 lines · Linus Walleij

I think this is perfectly realistic.

You're not going to duplicate each clk_register_clkdev(), which makes it way smaller than the original function, and since one of the function will be inside a

#ifdef CONFIG_OF
#endif

Quoted 8 lines

After we switch the entire platform to DT-only it will be pretty obvious which big chunk of code that needs to go away, it's a clean cut.

(Note: I know the #ifdef CONFIG_OF is not necessary anymore since we switched to multiplatform, but I intend that marker for humans, not machines.)

You just reminded me that I'm still sitting on this old patch to add lots of #ifdef CONFIG_ATAGS to ux500 in the places that need to get cut out. I don't think I'm able to update that patch at the moment, but I can send you the old version I have if you are interested.

Arnd

#63 lore raw

Re: [PATCH 21/33] clk: ux500: Add Device Tree support for the PRCC Kernel clock

Quoted 5 lines · Arnd Bergmann

You just reminded me that I'm still sitting on this old patch to add lots of #ifdef CONFIG_ATAGS to ux500 in the places that need to get cut out. I don't think I'm able to update that patch at the moment, but I can send you the old version I have if you are interested.

Well, right now we are getting to the point where DT is fully functional and Lee has just sent a first round of patches to switch clk to DT and then delete all the ATAG code in one swift stroke. (With patches to also clean out platform-data related cruft in the drivers to follow in the next merge window.)

After that it will actually start to look small and nice, so I think we'll take that path for v3.13.

(Famous last words.)

Yours,
Linus Walleij

author
#64 lore raw

Re: [PATCH 21/33] clk: ux500: Add Device Tree support for the PRCC Kernel clock

In reply to Linus Walleij
Quoted 21 lines · Linus Walleij
Quoted 9 lines · Lee Jones
Quoted 3 lines

I really do not like the approach of uglifying something and then beautifying it later... I prefer each step in isolation to be good looking, or you will be confused when traversing the history.

So then we have a few options, some more realistic than others.

1. Duplicate each of the; clk_reg_prcmu_*(), clk_reg_prcc_pclk(), clk_reg_prcc_kclk() calls into your proposed u8500_clk_init_dt(), which, while keeping everything separate would be unrealistic.

I think this is perfectly realistic.

You're not going to duplicate each clk_register_clkdev(), which makes it way smaller than the original function, and since one of the function will be inside a

#ifdef CONFIG_OF
#endif

Quoted 7 lines

After we switch the entire platform to DT-only it will be pretty obvious which big chunk of code that needs to go away, it's a clean cut.

(Note: I know the #ifdef CONFIG_OF is not necessary anymore since we switched to multiplatform, but I intend that marker for humans, not machines.)

This sounds gross. To duplicate; u8500_clk_init(), u8540_clk_init() and u9540_clk_init() just for the sake of loading a few pointers into an array for a small part of the development cycle sounds obscene.

I genuinely think keeping the current patch in this series and then removing the clk_register_clkdev() in the remove ATAG support series is the best way to go.

If you think I'm wrong then I'll so as you ask however. Just pass me the sick bucket.

Signature · 4 lines
Lee Jones
Linaro ST-Ericsson Landing Team Lead
Linaro.org ? Open source software for ARM SoCs
Follow Linaro: Facebook | Twitter | Blog
#65 lore raw

Re: [PATCH 21/33] clk: ux500: Add Device Tree support for the PRCC Kernel clock

Quoted 24 lines · Lee Jones
Quoted 15 lines · Linus Walleij
Quoted 3 lines · Lee Jones

1. Duplicate each of the; clk_reg_prcmu_*(), clk_reg_prcc_pclk(), clk_reg_prcc_kclk() calls into your proposed u8500_clk_init_dt(), which, while keeping everything separate would be unrealistic.

I think this is perfectly realistic.

You're not going to duplicate each clk_register_clkdev(), which makes it way smaller than the original function, and since one of the function will be inside a

#ifdef CONFIG_OF
#endif

Quoted 7 lines

After we switch the entire platform to DT-only it will be pretty obvious which big chunk of code that needs to go away, it's a clean cut.

(Note: I know the #ifdef CONFIG_OF is not necessary anymore since we switched to multiplatform, but I intend that marker for humans, not machines.)

Quoted 7 lines

This sounds gross. To duplicate; u8500_clk_init(), u8540_clk_init() and u9540_clk_init() just for the sake of loading a few pointers into an array for a small part of the development cycle sounds obscene.

I genuinely think keeping the current patch in this series and then removing the clk_register_clkdev() in the remove ATAG support series is the best way to go.

So what I am worrying about is not only the looks of the code and what is beautiful or not may be something of an opinion anway.

What I worry about is leaving all the calls to clk_register_clkdev() in the DT boot path. Because that has the potential to hide a lot of bugs, as clk_get() from drivers that should've got named and probed randomly now will still find their clocks from their old device names, instead of using the <&ampersand> clocks from the device tree.

But if you still don't like this, let me cook a counter-patch so I can realized on my own how terribly wrong I am...

Yours,
Linus Walleij

author
#66 lore raw

Re: [PATCH 21/33] clk: ux500: Add Device Tree support for the PRCC Kernel clock

Quoted 44 lines · Linus Walleij
Quoted 24 lines · Lee Jones
Quoted 15 lines · Linus Walleij
Quoted 3 lines · Lee Jones

1. Duplicate each of the; clk_reg_prcmu_*(), clk_reg_prcc_pclk(), clk_reg_prcc_kclk() calls into your proposed u8500_clk_init_dt(), which, while keeping everything separate would be unrealistic.

I think this is perfectly realistic.

You're not going to duplicate each clk_register_clkdev(), which makes it way smaller than the original function, and since one of the function will be inside a

#ifdef CONFIG_OF
#endif

Quoted 7 lines

After we switch the entire platform to DT-only it will be pretty obvious which big chunk of code that needs to go away, it's a clean cut.

(Note: I know the #ifdef CONFIG_OF is not necessary anymore since we switched to multiplatform, but I intend that marker for humans, not machines.)

Quoted 7 lines

This sounds gross. To duplicate; u8500_clk_init(), u8540_clk_init() and u9540_clk_init() just for the sake of loading a few pointers into an array for a small part of the development cycle sounds obscene.

I genuinely think keeping the current patch in this series and then removing the clk_register_clkdev() in the remove ATAG support series is the best way to go.

So what I am worrying about is not only the looks of the code and what is beautiful or not may be something of an opinion anway.

What I worry about is leaving all the calls to clk_register_clkdev() in the DT boot path. Because that has the potential to hide a lot of bugs, as clk_get() from drivers that should've got named and probed randomly now will still find their clocks from their old device names, instead of using the <&ampersand> clocks from the device tree.

Quoted 2 lines

But if you still don't like this, let me cook a counter-patch so I can realized on my own how terribly wrong I am...

I'm going to yank all of the clk_register_clkdev() calls out imminently anyway, so all those these hiding bugs will soon become apparent in any case. Why don't I place my 'remove ATAGs' patch-set on top of this one and then finally remove the clk_register_clkdev()s and watch the carnage ensue?

Signature · 4 lines
Lee Jones
Linaro STMicroelectronics Landing Team Lead
Linaro.org ? Open source software for ARM SoCs
Follow Linaro: Facebook | Twitter | Blog
#67 lore raw

Re: [PATCH 21/33] clk: ux500: Add Device Tree support for the PRCC Kernel clock

Quoted 10 lines · Lee Jones
Quoted 2 lines · Linus Walleij

But if you still don't like this, let me cook a counter-patch so I can realized on my own how terribly wrong I am...

I'm going to yank all of the clk_register_clkdev() calls out imminently anyway, so all those these hiding bugs will soon become apparent in any case. Why don't I place my 'remove ATAGs' patch-set on top of this one and then finally remove the clk_register_clkdev()s and watch the carnage ensue?

So that creates problem for me as maintainer if I need to pull or revert some of the patches on top of the stack, all of a sudden the file looks horrible. That is why I want every step to be as clean a cut as possible.

Yours,
Linus Walleij

author
#68 lore raw

PATCH 22/33 clk: ux500: Add Device Tree support for the RTC clock

In reply to Lee Jones

This patch enables the RTC fixed frequency clock to be specified from Device Tree via phandles to the "rtc32k-clock" node.

Cc Mike Turquette <[email protected]>
Cc Ulf Hansson <[email protected]>
---
 drivers/clk/ux500/u8500_clk.c |   11 +++++++----
 1 file changed, 7 insertions(+), 4 deletions(-)
drivers/clk/ux500/u8500_clk.c +7 -4
index c3ed09c..ebd0381 100644
@@ -60,7 +60,7 @@ void u8500_clk_init(u32 clkrst1_base, u32 clkrst2_base, u32 clkrst3_base,
 	struct device_node *np = NULL;
 	struct device_node *child = NULL;
 	const char *sgaclk_parent = NULL;
-	struct clk *clk;
+	struct clk *clk, *rtc_clk;
 
 	/* Clock sources */
 	clk = clk_reg_prcmu_gate("soc0_pll", NULL, PRCMU_PLLSOC0,
@@ -80,11 +80,11 @@ void u8500_clk_init(u32 clkrst1_base, u32 clkrst2_base, u32 clkrst3_base,
 
 	/* FIXME: Add sys, ulp and int clocks here. */
 
-	clk = clk_register_fixed_rate(NULL, "rtc32k", "NULL",
+	rtc_clk = clk_register_fixed_rate(NULL, "rtc32k", "NULL",
 				CLK_IS_ROOT|CLK_IGNORE_UNUSED,
 				32768);
-	clk_register_clkdev(clk, "clk32k", NULL);
-	clk_register_clkdev(clk, "apb_pclk", "rtc-pl031");
+	clk_register_clkdev(rtc_clk, "clk32k", NULL);
+	clk_register_clkdev(rtc_clk, "apb_pclk", "rtc-pl031");
 
 	/* PRCMU clocks */
 	fw_version = prcmu_get_fw_version();
@@ -690,5 +690,8 @@ void u8500_clk_init(u32 clkrst1_base, u32 clkrst2_base, u32 clkrst3_base,
 
 		if (!of_node_cmp(child->name, "prcc-kernel-clock"))
 			of_clk_add_provider(child, ux500_twocell_get, prcc_kclk);
+
+		if (!of_node_cmp(child->name, "rtc32k-clock"))
+			of_clk_add_provider(child, of_clk_src_simple_get, rtc_clk);
 	}
 }
Signature · 1 line
1.7.10.4
author
#69 lore raw

PATCH 23/33 clk: ux500: Add Device Tree support for the TWD clock

In reply to Lee Jones

This patch enables the TWD fixed factor clock to be specified from Device Tree via phandles to the "smp-twd-clock" node.

Cc Mike Turquette <[email protected]>
Cc Ulf Hansson <[email protected]>
---
 drivers/clk/ux500/u8500_clk.c |    9 ++++++---
 1 file changed, 6 insertions(+), 3 deletions(-)
drivers/clk/ux500/u8500_clk.c +6 -3
index ebd0381..b890067 100644
@@ -60,7 +60,7 @@ void u8500_clk_init(u32 clkrst1_base, u32 clkrst2_base, u32 clkrst3_base,
 	struct device_node *np = NULL;
 	struct device_node *child = NULL;
 	const char *sgaclk_parent = NULL;
-	struct clk *clk, *rtc_clk;
+	struct clk *clk, *rtc_clk, *twd_clk;
 
 	/* Clock sources */
 	clk = clk_reg_prcmu_gate("soc0_pll", NULL, PRCMU_PLLSOC0,
@@ -283,9 +283,9 @@ void u8500_clk_init(u32 clkrst1_base, u32 clkrst2_base, u32 clkrst3_base,
 				PRCMU_ARMSS, 0, CLK_IS_ROOT|CLK_IGNORE_UNUSED);
 	clk_register_clkdev(clk, "armss", NULL);
 
-	clk = clk_register_fixed_factor(NULL, "smp_twd", "armss",
+	twd_clk = clk_register_fixed_factor(NULL, "smp_twd", "armss",
 				CLK_IGNORE_UNUSED, 1, 2);
-	clk_register_clkdev(clk, NULL, "smp_twd");
+	clk_register_clkdev(twd_clk, NULL, "smp_twd");
 
 	/*
 	 * FIXME: Add special handled PRCMU clocks here:
@@ -693,5 +693,8 @@ void u8500_clk_init(u32 clkrst1_base, u32 clkrst2_base, u32 clkrst3_base,
 
 		if (!of_node_cmp(child->name, "rtc32k-clock"))
 			of_clk_add_provider(child, of_clk_src_simple_get, rtc_clk);
+
+		if (!of_node_cmp(child->name, "smp-twd-clock"))
+			of_clk_add_provider(child, of_clk_src_simple_get, twd_clk);
 	}
 }
Signature · 1 line
1.7.10.4
author
#70 lore raw

PATCH 24/33 ARM: ux500: Remove AUXDATA relating to GPIO clock-name bindings

In reply to Lee Jones
---
 arch/arm/mach-ux500/cpu-db8500.c |    9 ---------
 1 file changed, 9 deletions(-)
arch/arm/mach-ux500/cpu-db8500.c +0 -9
index 68a3918..8d72826 100644
@@ -228,15 +228,6 @@ static struct of_dev_auxdata u8500_auxdata_lookup[] __initdata = {
 	OF_DEV_AUXDATA("arm,pl18x", 0x80005000, "sdi2",  &mop500_sdi2_data),
 	OF_DEV_AUXDATA("arm,pl18x", 0x80114000, "sdi4",  &mop500_sdi4_data),
 	/* Requires clock name bindings. */
-	OF_DEV_AUXDATA("st,nomadik-gpio", 0x8012e000, "gpio.0", NULL),
-	OF_DEV_AUXDATA("st,nomadik-gpio", 0x8012e080, "gpio.1", NULL),
-	OF_DEV_AUXDATA("st,nomadik-gpio", 0x8000e000, "gpio.2", NULL),
-	OF_DEV_AUXDATA("st,nomadik-gpio", 0x8000e080, "gpio.3", NULL),
-	OF_DEV_AUXDATA("st,nomadik-gpio", 0x8000e100, "gpio.4", NULL),
-	OF_DEV_AUXDATA("st,nomadik-gpio", 0x8000e180, "gpio.5", NULL),
-	OF_DEV_AUXDATA("st,nomadik-gpio", 0x8011e000, "gpio.6", NULL),
-	OF_DEV_AUXDATA("st,nomadik-gpio", 0x8011e080, "gpio.7", NULL),
-	OF_DEV_AUXDATA("st,nomadik-gpio", 0xa03fe000, "gpio.8", NULL),
 	OF_DEV_AUXDATA("st,nomadik-i2c", 0x80004000, "nmk-i2c.0", NULL),
 	OF_DEV_AUXDATA("st,nomadik-i2c", 0x80122000, "nmk-i2c.1", NULL),
 	OF_DEV_AUXDATA("st,nomadik-i2c", 0x80128000, "nmk-i2c.2", NULL),
Signature · 1 line
1.7.10.4
author
#71 lore raw

PATCH 25/33 ARM: ux500: Remove AUXDATA relating to UART clock-name bindings

In reply to Lee Jones
---
 arch/arm/mach-ux500/cpu-db8500.c |    3 ---
 1 file changed, 3 deletions(-)
arch/arm/mach-ux500/cpu-db8500.c +0 -3
index 8d72826..3efb326 100644
@@ -219,9 +219,6 @@ static struct of_dev_auxdata u8500_auxdata_lookup[] __initdata = {
 	/* Requires call-back bindings. */
 	OF_DEV_AUXDATA("arm,cortex-a9-pmu", 0, "arm-pmu", &db8500_pmu_platdata),
 	/* Requires DMA bindings. */
-	OF_DEV_AUXDATA("arm,pl011", 0x80120000, "uart0", NULL),
-	OF_DEV_AUXDATA("arm,pl011", 0x80121000, "uart1", NULL),
-	OF_DEV_AUXDATA("arm,pl011", 0x80007000, "uart2", NULL),
 	OF_DEV_AUXDATA("arm,pl022", 0x80002000, "ssp0",  &ssp0_plat),
 	OF_DEV_AUXDATA("arm,pl18x", 0x80126000, "sdi0",  &mop500_sdi0_data),
 	OF_DEV_AUXDATA("arm,pl18x", 0x80118000, "sdi1",  &mop500_sdi1_data),
Signature · 1 line
1.7.10.4
author
#72 lore raw

PATCH 26/33 ARM: ux500: Remove AUXDATA relating to SDI (MMC) clock-name bindings

In reply to Lee Jones
---
 arch/arm/mach-ux500/cpu-db8500.c |    4 ----
 1 file changed, 4 deletions(-)
arch/arm/mach-ux500/cpu-db8500.c +0 -4
index 3efb326..aa325d8 100644
@@ -220,10 +220,6 @@ static struct of_dev_auxdata u8500_auxdata_lookup[] __initdata = {
 	OF_DEV_AUXDATA("arm,cortex-a9-pmu", 0, "arm-pmu", &db8500_pmu_platdata),
 	/* Requires DMA bindings. */
 	OF_DEV_AUXDATA("arm,pl022", 0x80002000, "ssp0",  &ssp0_plat),
-	OF_DEV_AUXDATA("arm,pl18x", 0x80126000, "sdi0",  &mop500_sdi0_data),
-	OF_DEV_AUXDATA("arm,pl18x", 0x80118000, "sdi1",  &mop500_sdi1_data),
-	OF_DEV_AUXDATA("arm,pl18x", 0x80005000, "sdi2",  &mop500_sdi2_data),
-	OF_DEV_AUXDATA("arm,pl18x", 0x80114000, "sdi4",  &mop500_sdi4_data),
 	/* Requires clock name bindings. */
 	OF_DEV_AUXDATA("st,nomadik-i2c", 0x80004000, "nmk-i2c.0", NULL),
 	OF_DEV_AUXDATA("st,nomadik-i2c", 0x80122000, "nmk-i2c.1", NULL),
Signature · 1 line
1.7.10.4
#73 lore raw

Re: [PATCH 26/33] ARM: ux500: Remove AUXDATA relating to SDI (MMC) clock-name bindings

Quoted 1 line · Lee Jones

(...)

arch/arm/mach-ux500/cpu-db8500.c static struct of_dev_auxdata u8500_auxdata_lookup[] __initdata = { · line 226

Quoted 6 lines
        /* Requires DMA bindings. */
        OF_DEV_AUXDATA("arm,pl022", 0x80002000, "ssp0",  &ssp0_plat),
arch/arm/mach-ux500/cpu-db8500.c +0 -4
-       OF_DEV_AUXDATA("arm,pl18x", 0x80126000, "sdi0",  &mop500_sdi0_data),
-       OF_DEV_AUXDATA("arm,pl18x", 0x80118000, "sdi1",  &mop500_sdi1_data),
-       OF_DEV_AUXDATA("arm,pl18x", 0x80005000, "sdi2",  &mop500_sdi2_data),
-       OF_DEV_AUXDATA("arm,pl18x", 0x80114000, "sdi4",  &mop500_sdi4_data),

This cannot be done. Ulf H would beat me up if I applied this.

The platform data there in the end is actually used.

Especially look at mop500_sdi0_data, it contains a lot of stuff that is not yet DT:ed even (a task in its own right).

But as long as we keep this pdata we can still delete ATAGs support (as per the second patch set pending) and get a clearer picture.

Yours,
Linus Walleij

author
#74 lore raw

Re: [PATCH 26/33] ARM: ux500: Remove AUXDATA relating to SDI (MMC) clock-name bindings

Quoted 13 lines · Linus Walleij
Quoted 1 line · Lee Jones

(...)

arch/arm/mach-ux500/cpu-db8500.c static struct of_dev_auxdata u8500_auxdata_lookup[] __initdata = { · line 226

Quoted 6 lines
        /* Requires DMA bindings. */
        OF_DEV_AUXDATA("arm,pl022", 0x80002000, "ssp0",  &ssp0_plat),
arch/arm/mach-ux500/cpu-db8500.c +0 -4
-       OF_DEV_AUXDATA("arm,pl18x", 0x80126000, "sdi0",  &mop500_sdi0_data),
-       OF_DEV_AUXDATA("arm,pl18x", 0x80118000, "sdi1",  &mop500_sdi1_data),
-       OF_DEV_AUXDATA("arm,pl18x", 0x80005000, "sdi2",  &mop500_sdi2_data),
-       OF_DEV_AUXDATA("arm,pl18x", 0x80114000, "sdi4",  &mop500_sdi4_data),

This cannot be done. Ulf H would beat me up if I applied this.

Quoted 7 lines

The platform data there in the end is actually used.

Especially look at mop500_sdi0_data, it contains a lot of stuff that is not yet DT:ed even (a task in its own right).

But as long as we keep this pdata we can still delete ATAGs support (as per the second patch set pending) and get a clearer picture.

Agreed. Scrap the patch.

Signature · 4 lines
Lee Jones
Linaro ST-Ericsson Landing Team Lead
Linaro.org ? Open source software for ARM SoCs
Follow Linaro: Facebook | Twitter | Blog
#75 lore raw

Re: [PATCH 26/33] ARM: ux500: Remove AUXDATA relating to SDI (MMC) clock-name bindings

In reply to Linus Walleij
Quoted 13 lines · Linus Walleij
Quoted 1 line · Lee Jones

(...)

arch/arm/mach-ux500/cpu-db8500.c static struct of_dev_auxdata u8500_auxdata_lookup[] __initdata = { · line 226

Quoted 6 lines
        /* Requires DMA bindings. */
        OF_DEV_AUXDATA("arm,pl022", 0x80002000, "ssp0",  &ssp0_plat),
arch/arm/mach-ux500/cpu-db8500.c +0 -4
-       OF_DEV_AUXDATA("arm,pl18x", 0x80126000, "sdi0",  &mop500_sdi0_data),
-       OF_DEV_AUXDATA("arm,pl18x", 0x80118000, "sdi1",  &mop500_sdi1_data),
-       OF_DEV_AUXDATA("arm,pl18x", 0x80005000, "sdi2",  &mop500_sdi2_data),
-       OF_DEV_AUXDATA("arm,pl18x", 0x80114000, "sdi4",  &mop500_sdi4_data),

This cannot be done. Ulf H would beat me up if I applied this.

Quoted 7 lines

The platform data there in the end is actually used.

Especially look at mop500_sdi0_data, it contains a lot of stuff that is not yet DT:ed even (a task in its own right).

But as long as we keep this pdata we can still delete ATAGs support (as per the second patch set pending) and get a clearer picture.

Which parts of mop500_sdi0_data are not yet converted? I thought it was all covered months ago.

Arnd

author
#76 lore raw

Re: [PATCH 26/33] ARM: ux500: Remove AUXDATA relating to SDI (MMC) clock-name bindings

Quoted 16 lines · Arnd Bergmann
Quoted 15 lines · Linus Walleij
Quoted 1 line · Lee Jones

(...)

arch/arm/mach-ux500/cpu-db8500.c static struct of_dev_auxdata u8500_auxdata_lookup[] __initdata = { · line 226

Quoted 6 lines
        /* Requires DMA bindings. */
        OF_DEV_AUXDATA("arm,pl022", 0x80002000, "ssp0",  &ssp0_plat),
arch/arm/mach-ux500/cpu-db8500.c +0 -4
-       OF_DEV_AUXDATA("arm,pl18x", 0x80126000, "sdi0",  &mop500_sdi0_data),
-       OF_DEV_AUXDATA("arm,pl18x", 0x80118000, "sdi1",  &mop500_sdi1_data),
-       OF_DEV_AUXDATA("arm,pl18x", 0x80005000, "sdi2",  &mop500_sdi2_data),
-       OF_DEV_AUXDATA("arm,pl18x", 0x80114000, "sdi4",  &mop500_sdi4_data),

This cannot be done. Ulf H would beat me up if I applied this.

The platform data there in the end is actually used.

Quoted 8 lines
Quoted 5 lines

Especially look at mop500_sdi0_data, it contains a lot of stuff that is not yet DT:ed even (a task in its own right).

But as long as we keep this pdata we can still delete ATAGs support (as per the second patch set pending) and get a clearer picture.

Which parts of mop500_sdi0_data are not yet converted? I thought it was all covered months ago.

Well it would have been if you hadn't have stopped my CAPs patch. ;)

Signature · 4 lines
Lee Jones
Linaro ST-Ericsson Landing Team Lead
Linaro.org ? Open source software for ARM SoCs
Follow Linaro: Facebook | Twitter | Blog
author
#77 lore raw

PATCH 27/33 ARM: ux500: Remove AUXDATA relating to I2C clock-name bindings

In reply to Lee Jones
---
 arch/arm/mach-ux500/cpu-db8500.c |    5 -----
 1 file changed, 5 deletions(-)
arch/arm/mach-ux500/cpu-db8500.c +0 -5
index aa325d8..b138d80 100644
@@ -221,11 +221,6 @@ static struct of_dev_auxdata u8500_auxdata_lookup[] __initdata = {
 	/* Requires DMA bindings. */
 	OF_DEV_AUXDATA("arm,pl022", 0x80002000, "ssp0",  &ssp0_plat),
 	/* Requires clock name bindings. */
-	OF_DEV_AUXDATA("st,nomadik-i2c", 0x80004000, "nmk-i2c.0", NULL),
-	OF_DEV_AUXDATA("st,nomadik-i2c", 0x80122000, "nmk-i2c.1", NULL),
-	OF_DEV_AUXDATA("st,nomadik-i2c", 0x80128000, "nmk-i2c.2", NULL),
-	OF_DEV_AUXDATA("st,nomadik-i2c", 0x80110000, "nmk-i2c.3", NULL),
-	OF_DEV_AUXDATA("st,nomadik-i2c", 0x8012a000, "nmk-i2c.4", NULL),
 	OF_DEV_AUXDATA("stericsson,db8500-musb", 0xa03e0000, "musb-ux500.0", NULL),
 	OF_DEV_AUXDATA("stericsson,db8500-prcmu", 0x80157000, "db8500-prcmu",
 			&db8500_prcmu_pdata),
Signature · 1 line
1.7.10.4
author
#78 lore raw

PATCH 28/33 ARM: ux500: Remove AUXDATA relating to MSP (Audio) clock-name bindings

In reply to Lee Jones
---
 arch/arm/mach-ux500/cpu-db8500.c |    9 ---------
 1 file changed, 9 deletions(-)
arch/arm/mach-ux500/cpu-db8500.c +0 -9
index b138d80..747b91f 100644
@@ -232,15 +232,6 @@ static struct of_dev_auxdata u8500_auxdata_lookup[] __initdata = {
 	/* Requires device name bindings. */
 	OF_DEV_AUXDATA("stericsson,db8500-pinctrl", U8500_PRCMU_BASE,
 		"pinctrl-db8500", NULL),
-	/* Requires clock name and DMA bindings. */
-	OF_DEV_AUXDATA("stericsson,ux500-msp-i2s", 0x80123000,
-		"ux500-msp-i2s.0", &msp0_platform_data),
-	OF_DEV_AUXDATA("stericsson,ux500-msp-i2s", 0x80124000,
-		"ux500-msp-i2s.1", &msp1_platform_data),
-	OF_DEV_AUXDATA("stericsson,ux500-msp-i2s", 0x80117000,
-		"ux500-msp-i2s.2", &msp2_platform_data),
-	OF_DEV_AUXDATA("stericsson,ux500-msp-i2s", 0x80125000,
-		"ux500-msp-i2s.3", &msp3_platform_data),
 	/* Requires clock name bindings and channel address lookup table. */
 	OF_DEV_AUXDATA("stericsson,db8500-dma40", 0x801C0000, "dma40.0", NULL),
 	{},
Signature · 1 line
1.7.10.4
#79 lore raw

Re: [PATCH 28/33] ARM: ux500: Remove AUXDATA relating to MSP (Audio) clock-name bindings

arch/arm/mach-ux500/cpu-db8500.c static struct of_dev_auxdata u8500_auxdata_lookup[] __initdata = { · line 235

Quoted 5 lines · Lee Jones
arch/arm/mach-ux500/cpu-db8500.c +0 -1
@@ -232,15 +232,6 @@ static struct of_dev_auxdata u8500_auxdata_lookup[] __initdata = {
        /* Requires device name bindings. */
        OF_DEV_AUXDATA("stericsson,db8500-pinctrl", U8500_PRCMU_BASE,
                "pinctrl-db8500", NULL),
-       /* Requires clock name and DMA bindings. */

The comment is just wrong.

arch/arm/mach-ux500/cpu-db8500.c static struct of_dev_auxdata u8500_auxdata_lookup[] __initdata = { · line 237

Quoted 2 lines
arch/arm/mach-ux500/cpu-db8500.c +0 -2
-       OF_DEV_AUXDATA("stericsson,ux500-msp-i2s", 0x80123000,
-               "ux500-msp-i2s.0", &msp0_platform_data),

Look, it also adds &msp0_platform_data, which is vital.

Result after boot:
ALSA device list:
No soundcards found.

Before the patch set we had audio and I kind of liked it :-)

I think you're able to set the device name as NULL though so we only add this platform data, try it out.

Yours,
Linus Walleij

author
#80 lore raw

Re: [PATCH 28/33] ARM: ux500: Remove AUXDATA relating to MSP (Audio) clock-name bindings

Quoted 15 lines · Linus Walleij

arch/arm/mach-ux500/cpu-db8500.c static struct of_dev_auxdata u8500_auxdata_lookup[] __initdata = { · line 235

Quoted 5 lines · Lee Jones
arch/arm/mach-ux500/cpu-db8500.c +0 -1
@@ -232,15 +232,6 @@ static struct of_dev_auxdata u8500_auxdata_lookup[] __initdata = {
        /* Requires device name bindings. */
        OF_DEV_AUXDATA("stericsson,db8500-pinctrl", U8500_PRCMU_BASE,
                "pinctrl-db8500", NULL),
-       /* Requires clock name and DMA bindings. */

The comment is just wrong.

arch/arm/mach-ux500/cpu-db8500.c static struct of_dev_auxdata u8500_auxdata_lookup[] __initdata = { · line 237

Quoted 2 lines
arch/arm/mach-ux500/cpu-db8500.c +0 -2
-       OF_DEV_AUXDATA("stericsson,ux500-msp-i2s", 0x80123000,
-               "ux500-msp-i2s.0", &msp0_platform_data),

Look, it also adds &msp0_platform_data, which is vital.

Quoted 8 lines

Result after boot:
ALSA device list:
No soundcards found.

Before the patch set we had audio and I kind of liked it :-)

I think you're able to set the device name as NULL though so we only add this platform data, try it out.

Makes sense. Although, I don't think there is a need to change the device name. I'll just put it under the a different heading of "Requires DMA bindings".

Signature · 4 lines
Lee Jones
Linaro ST-Ericsson Landing Team Lead
Linaro.org ? Open source software for ARM SoCs
Follow Linaro: Facebook | Twitter | Blog
author
#81 lore raw

PATCH 29/33 ARM: ux500: Remove AUXDATA relating to USB clock-name bindings

In reply to Lee Jones
---
 arch/arm/mach-ux500/cpu-db8500.c |    1 -
 1 file changed, 1 deletion(-)
arch/arm/mach-ux500/cpu-db8500.c +0 -1
index 747b91f..06c45a6 100644
@@ -221,7 +221,6 @@ static struct of_dev_auxdata u8500_auxdata_lookup[] __initdata = {
 	/* Requires DMA bindings. */
 	OF_DEV_AUXDATA("arm,pl022", 0x80002000, "ssp0",  &ssp0_plat),
 	/* Requires clock name bindings. */
-	OF_DEV_AUXDATA("stericsson,db8500-musb", 0xa03e0000, "musb-ux500.0", NULL),
 	OF_DEV_AUXDATA("stericsson,db8500-prcmu", 0x80157000, "db8500-prcmu",
 			&db8500_prcmu_pdata),
 	OF_DEV_AUXDATA("smsc,lan9115", 0x50000000, "smsc911x.0", NULL),
Signature · 1 line
1.7.10.4
author
#82 lore raw

PATCH 30/33 ARM: ux500: Remove AUXDATA relating to Ethernet clock-name bindings

In reply to Lee Jones
---
 arch/arm/mach-ux500/cpu-db8500.c |    1 -
 1 file changed, 1 deletion(-)
arch/arm/mach-ux500/cpu-db8500.c +0 -1
index 06c45a6..0d42cd4 100644
@@ -223,7 +223,6 @@ static struct of_dev_auxdata u8500_auxdata_lookup[] __initdata = {
 	/* Requires clock name bindings. */
 	OF_DEV_AUXDATA("stericsson,db8500-prcmu", 0x80157000, "db8500-prcmu",
 			&db8500_prcmu_pdata),
-	OF_DEV_AUXDATA("smsc,lan9115", 0x50000000, "smsc911x.0", NULL),
 	OF_DEV_AUXDATA("stericsson,ux500-cryp", 0xa03cb000, "cryp1", NULL),
 	OF_DEV_AUXDATA("stericsson,ux500-hash", 0xa03c2000, "hash1", NULL),
 	OF_DEV_AUXDATA("stericsson,snd-soc-mop500", 0, "snd-soc-mop500.0",
Signature · 1 line
1.7.10.4
author
#83 lore raw

PATCH 31/33 ARM: ux500: Remove AUXDATA relating to DMA clock-name bindings

In reply to Lee Jones
---
 arch/arm/mach-ux500/cpu-db8500.c |    2 --
 1 file changed, 2 deletions(-)
arch/arm/mach-ux500/cpu-db8500.c +0 -2
index 0d42cd4..335ca0d 100644
@@ -230,8 +230,6 @@ static struct of_dev_auxdata u8500_auxdata_lookup[] __initdata = {
 	/* Requires device name bindings. */
 	OF_DEV_AUXDATA("stericsson,db8500-pinctrl", U8500_PRCMU_BASE,
 		"pinctrl-db8500", NULL),
-	/* Requires clock name bindings and channel address lookup table. */
-	OF_DEV_AUXDATA("stericsson,db8500-dma40", 0x801C0000, "dma40.0", NULL),
 	{},
 };
 
Signature · 1 line
1.7.10.4
author
#84 lore raw

PATCH 32/33 ARM: ux500: Reclassify PRCMU AUXDATA entry

In reply to Lee Jones

We still need to utilise the AUXDATA system for the PRCMU to pass through platform data which can not be DT:ed i.e. regulator initialisation values. All we're doing in this patch is changing the comment header to be more accurate.

---
 arch/arm/mach-ux500/cpu-db8500.c |    2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)
arch/arm/mach-ux500/cpu-db8500.c +1 -1
index 335ca0d..f68493d 100644
@@ -220,7 +220,7 @@ static struct of_dev_auxdata u8500_auxdata_lookup[] __initdata = {
 	OF_DEV_AUXDATA("arm,cortex-a9-pmu", 0, "arm-pmu", &db8500_pmu_platdata),
 	/* Requires DMA bindings. */
 	OF_DEV_AUXDATA("arm,pl022", 0x80002000, "ssp0",  &ssp0_plat),
-	/* Requires clock name bindings. */
+	/* Requires non-DT:able platform data. */
 	OF_DEV_AUXDATA("stericsson,db8500-prcmu", 0x80157000, "db8500-prcmu",
 			&db8500_prcmu_pdata),
 	OF_DEV_AUXDATA("stericsson,ux500-cryp", 0xa03cb000, "cryp1", NULL),
Signature · 1 line
1.7.10.4
author
#85 lore raw

PATCH 33/33 ARM: ux500: Remove SSP AUXDATA pertaining to DMA bindings

In reply to Lee Jones

These are now cared for from the Device Tree.

---
 arch/arm/mach-ux500/cpu-db8500.c |    2 --
 1 file changed, 2 deletions(-)
arch/arm/mach-ux500/cpu-db8500.c +0 -2
index f68493d..d8ee616 100644
@@ -218,8 +218,6 @@ struct device * __init u8500_init_devices(void)
 static struct of_dev_auxdata u8500_auxdata_lookup[] __initdata = {
 	/* Requires call-back bindings. */
 	OF_DEV_AUXDATA("arm,cortex-a9-pmu", 0, "arm-pmu", &db8500_pmu_platdata),
-	/* Requires DMA bindings. */
-	OF_DEV_AUXDATA("arm,pl022", 0x80002000, "ssp0",  &ssp0_plat),
 	/* Requires non-DT:able platform data. */
 	OF_DEV_AUXDATA("stericsson,db8500-prcmu", 0x80157000, "db8500-prcmu",
 			&db8500_prcmu_pdata),
Signature · 1 line
1.7.10.4
author
#86 lore raw
In reply to Lee Jones
Quoted 3 lines

After this patchset has been applied, we can request clocks directly from Device Tree without using any AUXDATA device-name hacks. We also take care to remove all of thos at the end of the set.

So it looks like Mike and Grant have okayed the actual design of this patch-set, which is great. Is there any chance on some proper Acks from Linus and Mike please?

I guess it would be best if this went in via Linus' tree as a compete patch-set?

Signature · 4 lines
Lee Jones
Linaro ST-Ericsson Landing Team Lead
Linaro.org ? Open source software for ARM SoCs
Follow Linaro: Facebook | Twitter | Blog
#87 lore raw
Quoted 7 lines · Lee Jones
Quoted 3 lines

After this patchset has been applied, we can request clocks directly from Device Tree without using any AUXDATA device-name hacks. We also take care to remove all of thos at the end of the set.

So it looks like Mike and Grant have okayed the actual design of this patch-set, which is great. Is there any chance on some proper Acks from Linus and Mike please?

I will get to it. Basically I don't see there is a hurry as this has no chance to hit the v3.11 merge window anyway so we'll have lots of time to discuss it.

Why 3.12: my five pull request for ux500 stuff to ARM SoC is still outstanding, and this stuff will clash with that unless we get a common merge base. So we have to wait for ARM SoC
to settle at this point.

Yours,
Linus Walleij

author
#88 lore raw
Quoted 9 lines · Linus Walleij
Quoted 7 lines · Lee Jones
Quoted 3 lines

After this patchset has been applied, we can request clocks directly from Device Tree without using any AUXDATA device-name hacks. We also take care to remove all of thos at the end of the set.

So it looks like Mike and Grant have okayed the actual design of this patch-set, which is great. Is there any chance on some proper Acks from Linus and Mike please?

Quoted 8 lines

I will get to it. Basically I don't see there is a hurry as this has no chance to hit the v3.11 merge window anyway so we'll have lots of time to discuss it.

Why 3.12: my five pull request for ux500 stuff to ARM SoC is still outstanding, and this stuff will clash with that unless we get a common merge base. So we have to wait for ARM SoC
to settle at this point.

I understand, thanks for the clarification.

I'm fairly sure the semantics are sound, so I'll duplicate them for the u8540 - working on that now.

Signature · 4 lines
Lee Jones
Linaro ST-Ericsson Landing Team Lead
Linaro.org ? Open source software for ARM SoCs
Follow Linaro: Facebook | Twitter | Blog