2013-06-06 12:17:35

by Lee Jones

[permalink] [raw]
Subject: [PATCH 00/32] 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.


2013-06-06 12:17:37

by Lee Jones

[permalink] [raw]
Subject: [PATCH 02/33] ARM: ux500: Add PRCMU clock node to DBx500 Device Tree

Signed-off-by: Lee Jones <[email protected]>
---
arch/arm/boot/dts/dbx5x0.dtsi | 9 +++++++++
1 file changed, 9 insertions(+)

diff --git a/arch/arm/boot/dts/dbx5x0.dtsi b/arch/arm/boot/dts/dbx5x0.dtsi
index a082f0b..a635f7f 100644
--- a/arch/arm/boot/dts/dbx5x0.dtsi
+++ b/arch/arm/boot/dts/dbx5x0.dtsi
@@ -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@a0410600 {
compatible = "arm,cortex-a9-twd-timer";
reg = <0xa0410600 0x20>;
--
1.7.10.4

2013-06-06 12:17:47

by Lee Jones

[permalink] [raw]
Subject: [PATCH 08/33] ARM: ux500: Add PRCC Kernel clock node to DBx500 Device Tree

Signed-off-by: Lee Jones <[email protected]>
---
arch/arm/boot/dts/dbx5x0.dtsi | 4 ++++
1 file changed, 4 insertions(+)

diff --git a/arch/arm/boot/dts/dbx5x0.dtsi b/arch/arm/boot/dts/dbx5x0.dtsi
index d3cc575c..d682ea0 100644
--- a/arch/arm/boot/dts/dbx5x0.dtsi
+++ b/arch/arm/boot/dts/dbx5x0.dtsi
@@ -53,6 +53,10 @@
prcc_pclk: prcc-periph-clock {
#clock-cells = <2>;
};
+
+ prcc_kclk: prcc-kernel-clock {
+ #clock-cells = <2>;
+ };
};

timer@a0410600 {
--
1.7.10.4

2013-06-06 12:17:58

by Lee Jones

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

Signed-off-by: Lee Jones <[email protected]>
---
arch/arm/boot/dts/dbx5x0.dtsi | 4 ++++
1 file changed, 4 insertions(+)

diff --git a/arch/arm/boot/dts/dbx5x0.dtsi b/arch/arm/boot/dts/dbx5x0.dtsi
index e4ce16b..a677521 100644
--- a/arch/arm/boot/dts/dbx5x0.dtsi
+++ b/arch/arm/boot/dts/dbx5x0.dtsi
@@ -57,6 +57,10 @@
prcc_kclk: prcc-kernel-clock {
#clock-cells = <2>;
};
+
+ rtc_clk: rtc32k-clock {
+ #clock-cells = <0>;
+ };
};

timer@a0410600 {
--
1.7.10.4

2013-06-06 12:17:40

by Lee Jones

[permalink] [raw]
Subject: [PATCH 04/33] ARM: ux500: Add PRCC Peripheral clock node to DBx500 Device Tree

Signed-off-by: Lee Jones <[email protected]>
---
arch/arm/boot/dts/dbx5x0.dtsi | 4 ++++
1 file changed, 4 insertions(+)

diff --git a/arch/arm/boot/dts/dbx5x0.dtsi b/arch/arm/boot/dts/dbx5x0.dtsi
index c5f187b..18c5c27 100644
--- a/arch/arm/boot/dts/dbx5x0.dtsi
+++ b/arch/arm/boot/dts/dbx5x0.dtsi
@@ -49,6 +49,10 @@
prcmu_clk: prcmu-clock {
#clock-cells = <1>;
};
+
+ prcc_pclk: prcc-periph-clock {
+ #clock-cells = <2>;
+ };
};

timer@a0410600 {
--
1.7.10.4

2013-06-06 12:17:52

by Lee Jones

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

Signed-off-by: Lee Jones <[email protected]>
---
arch/arm/boot/dts/dbx5x0.dtsi | 14 ++++++++++++++
1 file changed, 14 insertions(+)

diff --git a/arch/arm/boot/dts/dbx5x0.dtsi b/arch/arm/boot/dts/dbx5x0.dtsi
index d682ea0..e49c679 100644
--- a/arch/arm/boot/dts/dbx5x0.dtsi
+++ b/arch/arm/boot/dts/dbx5x0.dtsi
@@ -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@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@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@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@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@80002000 {
--
1.7.10.4

2013-06-06 12:18:24

by Lee Jones

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

Signed-off-by: Lee Jones <[email protected]>
---
arch/arm/mach-ux500/cpu-db8500.c | 9 ---------
1 file changed, 9 deletions(-)

diff --git a/arch/arm/mach-ux500/cpu-db8500.c b/arch/arm/mach-ux500/cpu-db8500.c
index b138d80..747b91f 100644
--- a/arch/arm/mach-ux500/cpu-db8500.c
+++ b/arch/arm/mach-ux500/cpu-db8500.c
@@ -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),
{},
--
1.7.10.4

2013-06-06 12:18:29

by Lee Jones

[permalink] [raw]
Subject: [PATCH 32/33] ARM: ux500: Reclassify PRCMU AUXDATA entry

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.

Signed-off-by: Lee Jones <[email protected]>
---
arch/arm/mach-ux500/cpu-db8500.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/arch/arm/mach-ux500/cpu-db8500.c b/arch/arm/mach-ux500/cpu-db8500.c
index 335ca0d..f68493d 100644
--- a/arch/arm/mach-ux500/cpu-db8500.c
+++ b/arch/arm/mach-ux500/cpu-db8500.c
@@ -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),
--
1.7.10.4

2013-06-06 12:18:43

by Lee Jones

[permalink] [raw]
Subject: [PATCH 33/33] ARM: ux500: Remove SSP AUXDATA pertaining to DMA bindings

These are now cared for from the Device Tree.

Signed-off-by: Lee Jones <[email protected]>
---
arch/arm/mach-ux500/cpu-db8500.c | 2 --
1 file changed, 2 deletions(-)

diff --git a/arch/arm/mach-ux500/cpu-db8500.c b/arch/arm/mach-ux500/cpu-db8500.c
index f68493d..d8ee616 100644
--- a/arch/arm/mach-ux500/cpu-db8500.c
+++ b/arch/arm/mach-ux500/cpu-db8500.c
@@ -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),
--
1.7.10.4

2013-06-06 12:18:58

by Lee Jones

[permalink] [raw]
Subject: [PATCH 31/33] ARM: ux500: Remove AUXDATA relating to DMA clock-name bindings

Signed-off-by: Lee Jones <[email protected]>
---
arch/arm/mach-ux500/cpu-db8500.c | 2 --
1 file changed, 2 deletions(-)

diff --git a/arch/arm/mach-ux500/cpu-db8500.c b/arch/arm/mach-ux500/cpu-db8500.c
index 0d42cd4..335ca0d 100644
--- a/arch/arm/mach-ux500/cpu-db8500.c
+++ b/arch/arm/mach-ux500/cpu-db8500.c
@@ -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),
{},
};

--
1.7.10.4

2013-06-06 12:19:36

by Lee Jones

[permalink] [raw]
Subject: [PATCH 30/33] ARM: ux500: Remove AUXDATA relating to Ethernet clock-name bindings

Signed-off-by: Lee Jones <[email protected]>
---
arch/arm/mach-ux500/cpu-db8500.c | 1 -
1 file changed, 1 deletion(-)

diff --git a/arch/arm/mach-ux500/cpu-db8500.c b/arch/arm/mach-ux500/cpu-db8500.c
index 06c45a6..0d42cd4 100644
--- a/arch/arm/mach-ux500/cpu-db8500.c
+++ b/arch/arm/mach-ux500/cpu-db8500.c
@@ -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",
--
1.7.10.4

2013-06-06 12:18:21

by Lee Jones

[permalink] [raw]
Subject: [PATCH 27/33] ARM: ux500: Remove AUXDATA relating to I2C clock-name bindings

Signed-off-by: Lee Jones <[email protected]>
---
arch/arm/mach-ux500/cpu-db8500.c | 5 -----
1 file changed, 5 deletions(-)

diff --git a/arch/arm/mach-ux500/cpu-db8500.c b/arch/arm/mach-ux500/cpu-db8500.c
index aa325d8..b138d80 100644
--- a/arch/arm/mach-ux500/cpu-db8500.c
+++ b/arch/arm/mach-ux500/cpu-db8500.c
@@ -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),
--
1.7.10.4

2013-06-06 12:19:51

by Lee Jones

[permalink] [raw]
Subject: [PATCH 29/33] ARM: ux500: Remove AUXDATA relating to USB clock-name bindings

Signed-off-by: Lee Jones <[email protected]>
---
arch/arm/mach-ux500/cpu-db8500.c | 1 -
1 file changed, 1 deletion(-)

diff --git a/arch/arm/mach-ux500/cpu-db8500.c b/arch/arm/mach-ux500/cpu-db8500.c
index 747b91f..06c45a6 100644
--- a/arch/arm/mach-ux500/cpu-db8500.c
+++ b/arch/arm/mach-ux500/cpu-db8500.c
@@ -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),
--
1.7.10.4

2013-06-06 12:18:17

by Lee Jones

[permalink] [raw]
Subject: [PATCH 24/33] ARM: ux500: Remove AUXDATA relating to GPIO clock-name bindings

Signed-off-by: Lee Jones <[email protected]>
---
arch/arm/mach-ux500/cpu-db8500.c | 9 ---------
1 file changed, 9 deletions(-)

diff --git a/arch/arm/mach-ux500/cpu-db8500.c b/arch/arm/mach-ux500/cpu-db8500.c
index 68a3918..8d72826 100644
--- a/arch/arm/mach-ux500/cpu-db8500.c
+++ b/arch/arm/mach-ux500/cpu-db8500.c
@@ -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),
--
1.7.10.4

2013-06-06 12:21:15

by Lee Jones

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

Signed-off-by: Lee Jones <[email protected]>
---
arch/arm/mach-ux500/cpu-db8500.c | 4 ----
1 file changed, 4 deletions(-)

diff --git a/arch/arm/mach-ux500/cpu-db8500.c b/arch/arm/mach-ux500/cpu-db8500.c
index 3efb326..aa325d8 100644
--- a/arch/arm/mach-ux500/cpu-db8500.c
+++ b/arch/arm/mach-ux500/cpu-db8500.c
@@ -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),
--
1.7.10.4

2013-06-06 12:18:14

by Lee Jones

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

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]>
Signed-off-by: Lee Jones <[email protected]>
---
drivers/clk/ux500/u8500_clk.c | 30 ++++++++++++++++++++++++++++++
1 file changed, 30 insertions(+)

diff --git a/drivers/clk/ux500/u8500_clk.c b/drivers/clk/ux500/u8500_clk.c
index 86de1a7..c3ed09c 100644
--- a/drivers/clk/ux500/u8500_clk.c
+++ b/drivers/clk/ux500/u8500_clk.c
@@ -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

2013-06-06 12:21:35

by Lee Jones

[permalink] [raw]
Subject: [PATCH 25/33] ARM: ux500: Remove AUXDATA relating to UART clock-name bindings

Signed-off-by: Lee Jones <[email protected]>
---
arch/arm/mach-ux500/cpu-db8500.c | 3 ---
1 file changed, 3 deletions(-)

diff --git a/arch/arm/mach-ux500/cpu-db8500.c b/arch/arm/mach-ux500/cpu-db8500.c
index 8d72826..3efb326 100644
--- a/arch/arm/mach-ux500/cpu-db8500.c
+++ b/arch/arm/mach-ux500/cpu-db8500.c
@@ -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),
--
1.7.10.4

2013-06-06 12:18:10

by Lee Jones

[permalink] [raw]
Subject: [PATCH 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]>
Signed-off-by: Lee Jones <[email protected]>
---
drivers/clk/ux500/u8500_clk.c | 57 +++++++++++++++++++++++++++++++++++++++++
1 file changed, 57 insertions(+)

diff --git a/drivers/clk/ux500/u8500_clk.c b/drivers/clk/ux500/u8500_clk.c
index 4f5ad4c..86de1a7 100644
--- a/drivers/clk/ux500/u8500_clk.c
+++ b/drivers/clk/ux500/u8500_clk.c
@@ -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);
}
}
--
1.7.10.4

2013-06-06 12:21:54

by Lee Jones

[permalink] [raw]
Subject: [PATCH 23/33] clk: ux500: Add Device Tree support for the TWD clock

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]>
Signed-off-by: Lee Jones <[email protected]>
---
drivers/clk/ux500/u8500_clk.c | 9 ++++++---
1 file changed, 6 insertions(+), 3 deletions(-)

diff --git a/drivers/clk/ux500/u8500_clk.c b/drivers/clk/ux500/u8500_clk.c
index ebd0381..b890067 100644
--- a/drivers/clk/ux500/u8500_clk.c
+++ b/drivers/clk/ux500/u8500_clk.c
@@ -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);
}
}
--
1.7.10.4

2013-06-06 12:22:34

by Lee Jones

[permalink] [raw]
Subject: [PATCH 22/33] clk: ux500: Add Device Tree support for the RTC clock

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]>
Signed-off-by: Lee Jones <[email protected]>
---
drivers/clk/ux500/u8500_clk.c | 11 +++++++----
1 file changed, 7 insertions(+), 4 deletions(-)

diff --git a/drivers/clk/ux500/u8500_clk.c b/drivers/clk/ux500/u8500_clk.c
index c3ed09c..ebd0381 100644
--- a/drivers/clk/ux500/u8500_clk.c
+++ b/drivers/clk/ux500/u8500_clk.c
@@ -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);
}
}
--
1.7.10.4

2013-06-06 12:18:05

by Lee Jones

[permalink] [raw]
Subject: [PATCH 16/33] ARM: ux500: Supply the TWD Timer clock lookup to the DBX500 DT

Signed-off-by: Lee Jones <[email protected]>
---
arch/arm/boot/dts/dbx5x0.dtsi | 2 ++
1 file changed, 2 insertions(+)

diff --git a/arch/arm/boot/dts/dbx5x0.dtsi b/arch/arm/boot/dts/dbx5x0.dtsi
index 2027929..ed5655d 100644
--- a/arch/arm/boot/dts/dbx5x0.dtsi
+++ b/arch/arm/boot/dts/dbx5x0.dtsi
@@ -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@80154000 {
--
1.7.10.4

2013-06-06 12:22:50

by Lee Jones

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

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]>
Signed-off-by: Lee Jones <[email protected]>
---
drivers/clk/ux500/u8500_clk.c | 50 +++++++++++++++++++++++++++++++++++++++--
1 file changed, 48 insertions(+), 2 deletions(-)

diff --git a/drivers/clk/ux500/u8500_clk.c b/drivers/clk/ux500/u8500_clk.c
index 3a7040b..4f5ad4c 100644
--- a/drivers/clk/ux500/u8500_clk.c
+++ b/drivers/clk/ux500/u8500_clk.c
@@ -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);
+ }
}
}
--
1.7.10.4

2013-06-06 12:23:12

by Lee Jones

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

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]>
Signed-off-by: Lee Jones <[email protected]>
---
drivers/clk/ux500/u8500_clk.c | 19 +++++++++++++++++++
1 file changed, 19 insertions(+)

diff --git a/drivers/clk/ux500/u8500_clk.c b/drivers/clk/ux500/u8500_clk.c
index ef948ff..3a7040b 100644
--- a/drivers/clk/ux500/u8500_clk.c
+++ b/drivers/clk/ux500/u8500_clk.c
@@ -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", },
{ },
--
1.7.10.4

2013-06-06 12:23:47

by Lee Jones

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

Signed-off-by: Lee Jones <[email protected]>
---
arch/arm/boot/dts/dbx5x0.dtsi | 4 ++++
1 file changed, 4 insertions(+)

diff --git a/arch/arm/boot/dts/dbx5x0.dtsi b/arch/arm/boot/dts/dbx5x0.dtsi
index edb53c63..2027929 100644
--- a/arch/arm/boot/dts/dbx5x0.dtsi
+++ b/arch/arm/boot/dts/dbx5x0.dtsi
@@ -61,6 +61,10 @@
rtc_clk: rtc32k-clock {
#clock-cells = <0>;
};
+
+ smp_twd_clk: smp-twd-clock {
+ #clock-cells = <0>;
+ };
};

timer@a0410600 {
--
1.7.10.4

2013-06-06 12:23:45

by Lee Jones

[permalink] [raw]
Subject: [PATCH 17/33] clk: ux500: Provide u8500_clk with skeleton Device Tree support

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

Cc: Mike Turquette <[email protected]>
Cc: Ulf Hansson <[email protected]>
Signed-off-by: Lee Jones <[email protected]>
---
drivers/clk/ux500/u8500_clk.c | 17 +++++++++++++++++
1 file changed, 17 insertions(+)

diff --git a/drivers/clk/ux500/u8500_clk.c b/drivers/clk/ux500/u8500_clk.c
index 0b4f35a..ef948ff 100644
--- a/drivers/clk/ux500/u8500_clk.c
+++ b/drivers/clk/ux500/u8500_clk.c
@@ -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. */
+ }
}
--
1.7.10.4

2013-06-06 12:24:30

by Lee Jones

[permalink] [raw]
Subject: [PATCH 12/33] ARM: ux500: Supply the MSP (Audio) clocks lookup to the DBX500 DT

Signed-off-by: Lee Jones <[email protected]>
---
arch/arm/boot/dts/dbx5x0.dtsi | 16 ++++++++++++++++
1 file changed, 16 insertions(+)

diff --git a/arch/arm/boot/dts/dbx5x0.dtsi b/arch/arm/boot/dts/dbx5x0.dtsi
index 6b9c3a4..e4ce16b 100644
--- a/arch/arm/boot/dts/dbx5x0.dtsi
+++ b/arch/arm/boot/dts/dbx5x0.dtsi
@@ -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";
};

--
1.7.10.4

2013-06-06 12:24:29

by Lee Jones

[permalink] [raw]
Subject: [PATCH 14/33] ARM: ux500: Supply the RTC clock lookup to the DBX500 DT

Signed-off-by: Lee Jones <[email protected]>
---
arch/arm/boot/dts/dbx5x0.dtsi | 3 +++
1 file changed, 3 insertions(+)

diff --git a/arch/arm/boot/dts/dbx5x0.dtsi b/arch/arm/boot/dts/dbx5x0.dtsi
index a677521..edb53c63 100644
--- a/arch/arm/boot/dts/dbx5x0.dtsi
+++ b/arch/arm/boot/dts/dbx5x0.dtsi
@@ -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@8012e000 {
--
1.7.10.4

2013-06-06 12:25:28

by Lee Jones

[permalink] [raw]
Subject: [PATCH 10/33] ARM: ux500: Supply the UART clocks lookup to the DBX500 DT

Signed-off-by: Lee Jones <[email protected]>
---
arch/arm/boot/dts/dbx5x0.dtsi | 9 +++++++++
1 file changed, 9 insertions(+)

diff --git a/arch/arm/boot/dts/dbx5x0.dtsi b/arch/arm/boot/dts/dbx5x0.dtsi
index e49c679..8cfe5f1 100644
--- a/arch/arm/boot/dts/dbx5x0.dtsi
+++ b/arch/arm/boot/dts/dbx5x0.dtsi
@@ -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";
};

--
1.7.10.4

2013-06-06 12:25:26

by Lee Jones

[permalink] [raw]
Subject: [PATCH 11/33] ARM: ux500: Supply the SDI (MMC) clocks lookup to the DBX500 DT

Signed-off-by: Lee Jones <[email protected]>
---
arch/arm/boot/dts/dbx5x0.dtsi | 20 ++++++++++++++++++++
1 file changed, 20 insertions(+)

diff --git a/arch/arm/boot/dts/dbx5x0.dtsi b/arch/arm/boot/dts/dbx5x0.dtsi
index 8cfe5f1..6b9c3a4 100644
--- a/arch/arm/boot/dts/dbx5x0.dtsi
+++ b/arch/arm/boot/dts/dbx5x0.dtsi
@@ -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";
};

--
1.7.10.4

2013-06-06 12:17:44

by Lee Jones

[permalink] [raw]
Subject: [PATCH 06/33] ARM: ux500: Supply the USB clock lookup to the DBX500 DT

Signed-off-by: Lee Jones <[email protected]>
---
arch/arm/boot/dts/dbx5x0.dtsi | 2 ++
1 file changed, 2 insertions(+)

diff --git a/arch/arm/boot/dts/dbx5x0.dtsi b/arch/arm/boot/dts/dbx5x0.dtsi
index 928ef56..d3cc575c 100644
--- a/arch/arm/boot/dts/dbx5x0.dtsi
+++ b/arch/arm/boot/dts/dbx5x0.dtsi
@@ -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@801C0000 {
--
1.7.10.4

2013-06-06 12:26:10

by Lee Jones

[permalink] [raw]
Subject: [PATCH 07/33] ARM: ux500: Supply the Ethernet clock lookup to Snowball's DT

Signed-off-by: Lee Jones <[email protected]>
---
arch/arm/boot/dts/snowball.dts | 3 ++-
1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/arch/arm/boot/dts/snowball.dts b/arch/arm/boot/dts/snowball.dts
index fb9dce5..e20e8ae 100644
--- a/arch/arm/boot/dts/snowball.dts
+++ b/arch/arm/boot/dts/snowball.dts
@@ -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>;
};
};

--
1.7.10.4

2013-06-06 12:26:34

by Lee Jones

[permalink] [raw]
Subject: [PATCH 05/33] ARM: ux500: Supply the GPIO clocks lookup to the DBX500 DT

Signed-off-by: Lee Jones <[email protected]>
---
arch/arm/boot/dts/dbx5x0.dtsi | 18 ++++++++++++++++++
1 file changed, 18 insertions(+)

diff --git a/arch/arm/boot/dts/dbx5x0.dtsi b/arch/arm/boot/dts/dbx5x0.dtsi
index 18c5c27..928ef56 100644
--- a/arch/arm/boot/dts/dbx5x0.dtsi
+++ b/arch/arm/boot/dts/dbx5x0.dtsi
@@ -78,6 +78,8 @@
gpio-controller;
#gpio-cells = <2>;
gpio-bank = <0>;
+
+ clocks = <&prcc_pclk 1 9>;
};

gpio1: gpio@8012e080 {
@@ -91,6 +93,8 @@
gpio-controller;
#gpio-cells = <2>;
gpio-bank = <1>;
+
+ clocks = <&prcc_pclk 1 9>;
};

gpio2: gpio@8000e000 {
@@ -104,6 +108,8 @@
gpio-controller;
#gpio-cells = <2>;
gpio-bank = <2>;
+
+ clocks = <&prcc_pclk 3 8>;
};

gpio3: gpio@8000e080 {
@@ -117,6 +123,8 @@
gpio-controller;
#gpio-cells = <2>;
gpio-bank = <3>;
+
+ clocks = <&prcc_pclk 3 8>;
};

gpio4: gpio@8000e100 {
@@ -130,6 +138,8 @@
gpio-controller;
#gpio-cells = <2>;
gpio-bank = <4>;
+
+ clocks = <&prcc_pclk 3 8>;
};

gpio5: gpio@8000e180 {
@@ -143,6 +153,8 @@
gpio-controller;
#gpio-cells = <2>;
gpio-bank = <5>;
+
+ clocks = <&prcc_pclk 3 8>;
};

gpio6: gpio@8011e000 {
@@ -156,6 +168,8 @@
gpio-controller;
#gpio-cells = <2>;
gpio-bank = <6>;
+
+ clocks = <&prcc_pclk 2 1>;
};

gpio7: gpio@8011e080 {
@@ -169,6 +183,8 @@
gpio-controller;
#gpio-cells = <2>;
gpio-bank = <7>;
+
+ clocks = <&prcc_pclk 2 1>;
};

gpio8: gpio@a03fe000 {
@@ -182,6 +198,8 @@
gpio-controller;
#gpio-cells = <2>;
gpio-bank = <8>;
+
+ clocks = <&prcc_pclk 6 1>;
};

pinctrl {
--
1.7.10.4

2013-06-06 12:26:54

by Lee Jones

[permalink] [raw]
Subject: [PATCH 03/33] ARM: ux500: Supply the DMA clock lookup to the DBX500 DT

Signed-off-by: Lee Jones <[email protected]>
---
arch/arm/boot/dts/dbx5x0.dtsi | 2 ++
1 file changed, 2 insertions(+)

diff --git a/arch/arm/boot/dts/dbx5x0.dtsi b/arch/arm/boot/dts/dbx5x0.dtsi
index a635f7f..c5f187b 100644
--- a/arch/arm/boot/dts/dbx5x0.dtsi
+++ b/arch/arm/boot/dts/dbx5x0.dtsi
@@ -229,6 +229,8 @@

#dma-cells = <3>;
memcpy-channels = <56 57 58 59 60>;
+
+ clocks = <&prcmu_clk PRCMU_DMACLK>;
};

prcmu: prcmu@80157000 {
--
1.7.10.4

2013-06-06 12:27:34

by Lee Jones

[permalink] [raw]
Subject: [PATCH 01/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.

Signed-off-by: Lee Jones <[email protected]>
---
include/dt-bindings/mfd/dbx500-prcmu.h | 71 ++++++++++++++++++++++++++++++++
1 file changed, 71 insertions(+)
create mode 100644 include/dt-bindings/mfd/dbx500-prcmu.h

diff --git a/include/dt-bindings/mfd/dbx500-prcmu.h b/include/dt-bindings/mfd/dbx500-prcmu.h
new file mode 100644
index 0000000..e646664
--- /dev/null
+++ b/include/dt-bindings/mfd/dbx500-prcmu.h
@@ -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
--
1.7.10.4

2013-06-10 20:54:34

by Ulf Hansson

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

On 6 June 2013 14:17, Lee Jones <[email protected]> wrote:
> 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]>
> Signed-off-by: Lee Jones <[email protected]>
> ---
> drivers/clk/ux500/u8500_clk.c | 19 +++++++++++++++++++
> 1 file changed, 19 insertions(+)
>
> diff --git a/drivers/clk/ux500/u8500_clk.c b/drivers/clk/ux500/u8500_clk.c
> index ef948ff..3a7040b 100644
> --- a/drivers/clk/ux500/u8500_clk.c
> +++ b/drivers/clk/ux500/u8500_clk.c
> @@ -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);

Where did this macro come from?

Kind regards
Ulf Hansson

> +}
> +
> const static struct of_device_id u8500_clk_of_match[] = {
> { .compatible = "stericsson,u8500-clks", },
> { },
> --
> 1.7.10.4
>

2013-06-10 21:20:00

by Ulf Hansson

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

On 6 June 2013 14:17, Lee Jones <[email protected]> wrote:
> 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]>
> Signed-off-by: Lee Jones <[email protected]>
> ---
> drivers/clk/ux500/u8500_clk.c | 50 +++++++++++++++++++++++++++++++++++++++--
> 1 file changed, 48 insertions(+), 2 deletions(-)
>
> diff --git a/drivers/clk/ux500/u8500_clk.c b/drivers/clk/ux500/u8500_clk.c
> index 3a7040b..4f5ad4c 100644
> --- a/drivers/clk/ux500/u8500_clk.c
> +++ b/drivers/clk/ux500/u8500_clk.c
> @@ -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

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

2013-06-10 21:24:24

by Ulf Hansson

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

On 6 June 2013 14:17, Lee Jones <[email protected]> wrote:
> 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]>
> Signed-off-by: Lee Jones <[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

> ---
> drivers/clk/ux500/u8500_clk.c | 30 ++++++++++++++++++++++++++++++
> 1 file changed, 30 insertions(+)
>
> diff --git a/drivers/clk/ux500/u8500_clk.c b/drivers/clk/ux500/u8500_clk.c
> index 86de1a7..c3ed09c 100644
> --- a/drivers/clk/ux500/u8500_clk.c
> +++ b/drivers/clk/ux500/u8500_clk.c
> @@ -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
>

2013-06-11 09:10:18

by Lee Jones

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

> > 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]>
> > Signed-off-by: Lee Jones <[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.

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

> > ---
> > drivers/clk/ux500/u8500_clk.c | 30 ++++++++++++++++++++++++++++++
> > 1 file changed, 30 insertions(+)
> >
> > diff --git a/drivers/clk/ux500/u8500_clk.c b/drivers/clk/ux500/u8500_clk.c
> > index 86de1a7..c3ed09c 100644
> > --- a/drivers/clk/ux500/u8500_clk.c
> > +++ b/drivers/clk/ux500/u8500_clk.c
> > @@ -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);
> > }
> > }
> >

--
Lee Jones
Linaro ST-Ericsson Landing Team Lead
Linaro.org │ Open source software for ARM SoCs
Follow Linaro: Facebook | Twitter | Blog

2013-06-11 09:12:20

by Lee Jones

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

On Mon, 10 Jun 2013, Ulf Hansson wrote:

> On 6 June 2013 14:17, Lee Jones <[email protected]> wrote:
> > 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]>
> > Signed-off-by: Lee Jones <[email protected]>
> > ---
> > drivers/clk/ux500/u8500_clk.c | 19 +++++++++++++++++++
> > 1 file changed, 19 insertions(+)
> >
> > diff --git a/drivers/clk/ux500/u8500_clk.c b/drivers/clk/ux500/u8500_clk.c
> > index ef948ff..3a7040b 100644
> > --- a/drivers/clk/ux500/u8500_clk.c
> > +++ b/drivers/clk/ux500/u8500_clk.c
> > @@ -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);
>
> Where did this macro come from?

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

> > +}
> > +
> > const static struct of_device_id u8500_clk_of_match[] = {
> > { .compatible = "stericsson,u8500-clks", },
> > { },
> >

--
Lee Jones
Linaro ST-Ericsson Landing Team Lead
Linaro.org │ Open source software for ARM SoCs
Follow Linaro: Facebook | Twitter | Blog

2013-06-11 11:07:52

by Lee Jones

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

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]>
Signed-off-by: Lee Jones <[email protected]>

diff --git a/drivers/clk/ux500/u8500_clk.c b/drivers/clk/ux500/u8500_clk.c
index ef948ff..c2d9a1c 100644
--- a/drivers/clk/ux500/u8500_clk.c
+++ b/drivers/clk/ux500/u8500_clk.c
@@ -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", },
{ },

2013-06-11 11:09:29

by Lee Jones

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

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]>
Signed-off-by: Lee Jones <[email protected]>

diff --git a/drivers/clk/ux500/u8500_clk.c b/drivers/clk/ux500/u8500_clk.c
index ff02a26..c9ac350 100644
--- a/drivers/clk/ux500/u8500_clk.c
+++ b/drivers/clk/ux500/u8500_clk.c
@@ -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);
}
}

2013-06-11 11:10:58

by Lee Jones

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

On Mon, 10 Jun 2013, Ulf Hansson wrote:

> On 6 June 2013 14:17, Lee Jones <[email protected]> wrote:
> > 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]>
> > Signed-off-by: Lee Jones <[email protected]>
> > ---
> > drivers/clk/ux500/u8500_clk.c | 50 +++++++++++++++++++++++++++++++++++++++--
> > 1 file changed, 48 insertions(+), 2 deletions(-)
> >
> > diff --git a/drivers/clk/ux500/u8500_clk.c b/drivers/clk/ux500/u8500_clk.c
> > index 3a7040b..4f5ad4c 100644
> > --- a/drivers/clk/ux500/u8500_clk.c
> > +++ b/drivers/clk/ux500/u8500_clk.c
> > @@ -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?

Yes it is:

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

--
Lee Jones
Linaro ST-Ericsson Landing Team Lead
Linaro.org │ Open source software for ARM SoCs
Follow Linaro: Facebook | Twitter | Blog

2013-06-11 11:13:05

by Lee Jones

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

On Tue, 11 Jun 2013, Lee Jones wrote:

> On Mon, 10 Jun 2013, Ulf Hansson wrote:
>
> > On 6 June 2013 14:17, Lee Jones <[email protected]> wrote:
> > > 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]>
> > > Signed-off-by: Lee Jones <[email protected]>
> > > ---
> > > drivers/clk/ux500/u8500_clk.c | 50 +++++++++++++++++++++++++++++++++++++++--
> > > 1 file changed, 48 insertions(+), 2 deletions(-)
> > >
> > > diff --git a/drivers/clk/ux500/u8500_clk.c b/drivers/clk/ux500/u8500_clk.c
> > > index 3a7040b..4f5ad4c 100644
> > > --- a/drivers/clk/ux500/u8500_clk.c
> > > +++ b/drivers/clk/ux500/u8500_clk.c
> > > @@ -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?
>
> 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.

--
Lee Jones
Linaro ST-Ericsson Landing Team Lead
Linaro.org │ Open source software for ARM SoCs
Follow Linaro: Facebook | Twitter | Blog

2013-06-11 11:51:48

by Lee Jones

[permalink] [raw]
Subject: [PATCH 20/33 v2] 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]>
Signed-off-by: Lee Jones <[email protected]>

diff --git a/drivers/clk/ux500/u8500_clk.c b/drivers/clk/ux500/u8500_clk.c
index c8ebd19..ff02a26 100644
--- a/drivers/clk/ux500/u8500_clk.c
+++ b/drivers/clk/ux500/u8500_clk.c
@@ -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);
}
}

2013-06-12 13:27:21

by Lee Jones

[permalink] [raw]
Subject: Re: [PATCH 00/32] 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.

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?

--
Lee Jones
Linaro ST-Ericsson Landing Team Lead
Linaro.org │ Open source software for ARM SoCs
Follow Linaro: Facebook | Twitter | Blog

2013-06-12 14:46:40

by Arnd Bergmann

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

On Tuesday 11 June 2013, Lee Jones wrote:
> 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]>
> Signed-off-by: Lee Jones <[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

2013-06-13 08:41:58

by Linus Walleij

[permalink] [raw]
Subject: Re: [PATCH 00/32] ARM: ux500: Enable clocks for Device Tree

On Wed, Jun 12, 2013 at 3:27 PM, Lee Jones <[email protected]> wrote:
>> 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

2013-06-13 09:34:18

by Lee Jones

[permalink] [raw]
Subject: Re: [PATCH 00/32] ARM: ux500: Enable clocks for Device Tree

On Thu, 13 Jun 2013, Linus Walleij wrote:

> On Wed, Jun 12, 2013 at 3:27 PM, Lee Jones <[email protected]> wrote:
> >> 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.

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.

--
Lee Jones
Linaro ST-Ericsson Landing Team Lead
Linaro.org │ Open source software for ARM SoCs
Follow Linaro: Facebook | Twitter | Blog

2013-06-18 21:17:42

by Mike Turquette

[permalink] [raw]
Subject: 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)
> On Tuesday 11 June 2013, Lee Jones wrote:
> > 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]>
> > Signed-off-by: Lee Jones <[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?

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

>
> Arnd

2013-06-19 07:42:11

by Lee Jones

[permalink] [raw]
Subject: 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)
> > On Tuesday 11 June 2013, Lee Jones wrote:
> > > 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]>
> > > Signed-off-by: Lee Jones <[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?
>
> 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.

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

> 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*'.

--
Lee Jones
Linaro ST-Ericsson Landing Team Lead
Linaro.org │ Open source software for ARM SoCs
Follow Linaro: Facebook | Twitter | Blog

2013-08-20 09:11:21

by Linus Walleij

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

On Thu, Jun 6, 2013 at 2:16 PM, Lee Jones <[email protected]> wrote:

> +++ b/arch/arm/boot/dts/dbx5x0.dtsi
> @@ -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

2013-08-20 09:30:41

by Sascha Hauer

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

On Tue, Aug 20, 2013 at 11:11:19AM +0200, Linus Walleij wrote:
> On Thu, Jun 6, 2013 at 2:16 PM, Lee Jones <[email protected]> wrote:
>
> > +++ b/arch/arm/boot/dts/dbx5x0.dtsi
> > @@ -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

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

2013-08-21 08:08:41

by Linus Walleij

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

On Thu, Jun 6, 2013 at 2:17 PM, Lee Jones <[email protected]> wrote:

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

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

2013-08-21 08:17:33

by Lee Jones

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

On Wed, 21 Aug 2013, Linus Walleij wrote:

> On Thu, Jun 6, 2013 at 2:17 PM, Lee Jones <[email protected]> wrote:
>
> > @@ -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.
>
> > - 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.

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

--
Lee Jones
Linaro ST-Ericsson Landing Team Lead
Linaro.org │ Open source software for ARM SoCs
Follow Linaro: Facebook | Twitter | Blog

2013-08-21 08:18:02

by Linus Walleij

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

On Thu, Jun 6, 2013 at 2:17 PM, Lee Jones <[email protected]> wrote:

> 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]>
> Signed-off-by: Lee Jones <[email protected]>
(...)
> 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);

(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

2013-08-21 08:23:14

by Linus Walleij

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

On Thu, Jun 6, 2013 at 2:17 PM, Lee Jones <[email protected]> wrote:

> This patch enables clocks to be specified from Device Tree via phandles
> to the "prcmu-clock" node.
(..)
> @@ -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

2013-08-21 08:28:24

by Lee Jones

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

On Tue, 20 Aug 2013, Linus Walleij wrote:

> On Thu, Jun 6, 2013 at 2:16 PM, Lee Jones <[email protected]> wrote:
>
> > +++ b/arch/arm/boot/dts/dbx5x0.dtsi
> > @@ -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?

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/

--
Lee Jones
Linaro ST-Ericsson Landing Team Lead
Linaro.org │ Open source software for ARM SoCs
Follow Linaro: Facebook | Twitter | Blog

2013-08-21 10:10:50

by Lee Jones

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

On Wed, 21 Aug 2013, Linus Walleij wrote:

> On Thu, Jun 6, 2013 at 2:17 PM, Lee Jones <[email protected]> wrote:
>
> > This patch enables clocks to be specified from Device Tree via phandles
> > to the "prcmu-clock" node.
> (..)
> > @@ -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.

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

--
Lee Jones
Linaro ST-Ericsson Landing Team Lead
Linaro.org │ Open source software for ARM SoCs
Follow Linaro: Facebook | Twitter | Blog

2013-08-21 10:14:55

by Lee Jones

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

On Wed, 21 Aug 2013, Linus Walleij wrote:

> On Thu, Jun 6, 2013 at 2:17 PM, Lee Jones <[email protected]> wrote:
>
> > 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]>
> > Signed-off-by: Lee Jones <[email protected]>
> (...)
> > 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);
>
> (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?

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.

--
Lee Jones
Linaro ST-Ericsson Landing Team Lead
Linaro.org │ Open source software for ARM SoCs
Follow Linaro: Facebook | Twitter | Blog

2013-08-21 22:44:58

by Linus Walleij

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

On Wed, Aug 21, 2013 at 10:28 AM, Lee Jones <[email protected]> wrote:
> On Tue, 20 Aug 2013, Linus Walleij wrote:
>> On Thu, Jun 6, 2013 at 2:16 PM, Lee Jones <[email protected]> wrote:
>>
>> > +++ b/arch/arm/boot/dts/dbx5x0.dtsi
>> > @@ -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?
>
> 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

2013-08-21 22:46:49

by Linus Walleij

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

On Wed, Aug 21, 2013 at 12:14 PM, Lee Jones <[email protected]> wrote:
> On Wed, 21 Aug 2013, Linus Walleij wrote:

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

2013-08-22 09:21:37

by Lee Jones

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

On Thu, 22 Aug 2013, Linus Walleij wrote:

> On Wed, Aug 21, 2013 at 12:14 PM, Lee Jones <[email protected]> wrote:
> > On Wed, 21 Aug 2013, Linus Walleij wrote:
>
> >> 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.

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. :)

--
Lee Jones
Linaro ST-Ericsson Landing Team Lead
Linaro.org │ Open source software for ARM SoCs
Follow Linaro: Facebook | Twitter | Blog

2013-08-22 09:23:10

by Lee Jones

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

On Tue, 20 Aug 2013, Linus Walleij wrote:

> On Thu, Jun 6, 2013 at 2:16 PM, Lee Jones <[email protected]> wrote:
>
> > +++ b/arch/arm/boot/dts/dbx5x0.dtsi
> > @@ -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?

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

> "i2c0" works just fine as name I think?

Sure.

--
Lee Jones
Linaro ST-Ericsson Landing Team Lead
Linaro.org │ Open source software for ARM SoCs
Follow Linaro: Facebook | Twitter | Blog

2013-08-22 13:38:21

by Mark Rutland

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

On Tue, Aug 20, 2013 at 10:30:34AM +0100, Sascha Hauer wrote:
> On Tue, Aug 20, 2013 at 11:11:19AM +0200, Linus Walleij wrote:
> > On Thu, Jun 6, 2013 at 2:16 PM, Lee Jones <[email protected]> wrote:
> >
> > > +++ b/arch/arm/boot/dts/dbx5x0.dtsi
> > > @@ -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.

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

Mark.

2013-08-22 13:49:39

by Lee Jones

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

On Thu, 22 Aug 2013, Mark Rutland wrote:

> On Tue, Aug 20, 2013 at 10:30:34AM +0100, Sascha Hauer wrote:
> > On Tue, Aug 20, 2013 at 11:11:19AM +0200, Linus Walleij wrote:
> > > On Thu, Jun 6, 2013 at 2:16 PM, Lee Jones <[email protected]> wrote:
> > >
> > > > +++ b/arch/arm/boot/dts/dbx5x0.dtsi
> > > > @@ -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.
>
> +1. The clock names should be the input names to the unit, they
> shouldn't vary per instance.

Duly noted.

--
Lee Jones
Linaro ST-Ericsson Landing Team Lead
Linaro.org │ Open source software for ARM SoCs
Follow Linaro: Facebook | Twitter | Blog

2013-08-22 14:19:09

by Lee Jones

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

On Thu, 22 Aug 2013, Mark Rutland wrote:

> On Tue, Aug 20, 2013 at 10:30:34AM +0100, Sascha Hauer wrote:
> > On Tue, Aug 20, 2013 at 11:11:19AM +0200, Linus Walleij wrote:
> > > On Thu, Jun 6, 2013 at 2:16 PM, Lee Jones <[email protected]> wrote:
> > >
> > > > +++ b/arch/arm/boot/dts/dbx5x0.dtsi
> > > > @@ -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.
>
> +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?

--
Lee Jones
Linaro ST-Ericsson Landing Team Lead
Linaro.org │ Open source software for ARM SoCs
Follow Linaro: Facebook | Twitter | Blog

2013-08-22 15:18:00

by Mark Rutland

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

On Thu, Aug 22, 2013 at 03:19:00PM +0100, Lee Jones wrote:
> On Thu, 22 Aug 2013, Mark Rutland wrote:
>
> > On Tue, Aug 20, 2013 at 10:30:34AM +0100, Sascha Hauer wrote:
> > > On Tue, Aug 20, 2013 at 11:11:19AM +0200, Linus Walleij wrote:
> > > > On Thu, Jun 6, 2013 at 2:16 PM, Lee Jones <[email protected]> wrote:
> > > >
> > > > > +++ b/arch/arm/boot/dts/dbx5x0.dtsi
> > > > > @@ -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.
> >
> > +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?

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.

2013-08-22 15:41:25

by Lee Jones

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

On Thu, 22 Aug 2013, Mark Rutland wrote:

> On Thu, Aug 22, 2013 at 03:19:00PM +0100, Lee Jones wrote:
> > On Thu, 22 Aug 2013, Mark Rutland wrote:
> >
> > > On Tue, Aug 20, 2013 at 10:30:34AM +0100, Sascha Hauer wrote:
> > > > On Tue, Aug 20, 2013 at 11:11:19AM +0200, Linus Walleij wrote:
> > > > > On Thu, Jun 6, 2013 at 2:16 PM, Lee Jones <[email protected]> wrote:
> > > > >
> > > > > > +++ b/arch/arm/boot/dts/dbx5x0.dtsi
> > > > > > @@ -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.
> > >
> > > +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?
>
> 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.

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

--
Lee Jones
Linaro ST-Ericsson Landing Team Lead
Linaro.org │ Open source software for ARM SoCs
Follow Linaro: Facebook | Twitter | Blog

2013-08-22 16:05:18

by Mark Rutland

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

On Thu, Aug 22, 2013 at 04:41:16PM +0100, Lee Jones wrote:
> On Thu, 22 Aug 2013, Mark Rutland wrote:
>
> > On Thu, Aug 22, 2013 at 03:19:00PM +0100, Lee Jones wrote:
> > > On Thu, 22 Aug 2013, Mark Rutland wrote:
> > >
> > > > On Tue, Aug 20, 2013 at 10:30:34AM +0100, Sascha Hauer wrote:
> > > > > On Tue, Aug 20, 2013 at 11:11:19AM +0200, Linus Walleij wrote:
> > > > > > On Thu, Jun 6, 2013 at 2:16 PM, Lee Jones <[email protected]> wrote:
> > > > > >
> > > > > > > +++ b/arch/arm/boot/dts/dbx5x0.dtsi
> > > > > > > @@ -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.
> > > >
> > > > +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?
> >
> > 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.

Ok.

>
> Linus, branch fixed-up and pushed.
>
> > 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 ;-) )

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.

2013-08-22 21:19:22

by Sascha Hauer

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

On Thu, Aug 22, 2013 at 04:41:16PM +0100, Lee Jones wrote:
>
> 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

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

2013-08-23 07:56:18

by Lee Jones

[permalink] [raw]
Subject: 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.

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

--
Lee Jones
Linaro ST-Ericsson Landing Team Lead
Linaro.org │ Open source software for ARM SoCs
Follow Linaro: Facebook | Twitter | Blog

2013-08-23 13:31:42

by Linus Walleij

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

On Thu, Jun 6, 2013 at 2:17 PM, Lee Jones <[email protected]> wrote:

> Signed-off-by: Lee Jones <[email protected]>
(...)
> /* 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),

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

2013-08-23 14:46:03

by Lee Jones

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

On Fri, 23 Aug 2013, Linus Walleij wrote:

> On Thu, Jun 6, 2013 at 2:17 PM, Lee Jones <[email protected]> wrote:
>
> > Signed-off-by: Lee Jones <[email protected]>
> (...)
> > /* 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),
>
> 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.

Agreed. Scrap the patch.

--
Lee Jones
Linaro ST-Ericsson Landing Team Lead
Linaro.org │ Open source software for ARM SoCs
Follow Linaro: Facebook | Twitter | Blog

2013-08-23 16:56:18

by Mark Rutland

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

On Fri, Aug 23, 2013 at 08:56:07AM +0100, Lee Jones wrote:
> 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.
>
> > > 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:

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.

>
> 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";
>
> --
> Lee Jones
> Linaro ST-Ericsson Landing Team Lead
> Linaro.org │ Open source software for ARM SoCs
> Follow Linaro: Facebook | Twitter | Blog
>

2013-08-23 18:02:01

by Linus Walleij

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

On Thu, Aug 22, 2013 at 11:21 AM, Lee Jones <[email protected]> wrote:

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

2013-08-24 07:58:07

by Arnd Bergmann

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

On Friday 23 August 2013, Linus Walleij wrote:
> On Thu, Jun 6, 2013 at 2:17 PM, Lee Jones <[email protected]> wrote:
>
> > Signed-off-by: Lee Jones <[email protected]>
> (...)
> > /* 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),
>
> 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.

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

Arnd

2013-08-24 08:00:33

by Arnd Bergmann

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

On Friday 23 August 2013, Linus Walleij wrote:
> 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.)
>

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

2013-08-24 21:19:44

by Linus Walleij

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

On Sat, Aug 24, 2013 at 10:00 AM, Arnd Bergmann <[email protected]> wrote:

> 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

2013-08-27 08:06:45

by Lee Jones

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

On Fri, 23 Aug 2013, Mark Rutland wrote:

> On Fri, Aug 23, 2013 at 08:56:07AM +0100, Lee Jones wrote:
> > 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.
> >
> > > > 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:
>
> 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.

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.

--
Lee Jones
Linaro ST-Ericsson Landing Team Lead
Linaro.org │ Open source software for ARM SoCs
Follow Linaro: Facebook | Twitter | Blog

2013-08-27 08:11:46

by Lee Jones

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

On Sat, 24 Aug 2013, Arnd Bergmann wrote:

> On Friday 23 August 2013, Linus Walleij wrote:
> > On Thu, Jun 6, 2013 at 2:17 PM, Lee Jones <[email protected]> wrote:
> >
> > > Signed-off-by: Lee Jones <[email protected]>
> > (...)
> > > /* 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),
> >
> > 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.
>
> 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. ;)

--
Lee Jones
Linaro ST-Ericsson Landing Team Lead
Linaro.org │ Open source software for ARM SoCs
Follow Linaro: Facebook | Twitter | Blog

2013-08-27 08:23:37

by Lee Jones

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

On Fri, 23 Aug 2013, Linus Walleij wrote:

> On Thu, Aug 22, 2013 at 11:21 AM, Lee Jones <[email protected]> wrote:
>
> >> 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.)

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.

--
Lee Jones
Linaro ST-Ericsson Landing Team Lead
Linaro.org │ Open source software for ARM SoCs
Follow Linaro: Facebook | Twitter | Blog

2013-08-27 13:47:03

by Mark Rutland

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

On Tue, Aug 27, 2013 at 09:06:35AM +0100, Lee Jones wrote:
> On Fri, 23 Aug 2013, Mark Rutland wrote:
>
> > On Fri, Aug 23, 2013 at 08:56:07AM +0100, Lee Jones wrote:
> > > 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.
> > >
> > > > > 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:
> >
> > 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.
>
> 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.

2013-08-27 14:08:14

by Lee Jones

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

On Tue, 27 Aug 2013, Mark Rutland wrote:

> On Tue, Aug 27, 2013 at 09:06:35AM +0100, Lee Jones wrote:
> > On Fri, 23 Aug 2013, Mark Rutland wrote:
> >
> > > On Fri, Aug 23, 2013 at 08:56:07AM +0100, Lee Jones wrote:
> > > > 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.
> > > >
> > > > > > 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:
> > >
> > > 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.
> >
> > 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.

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.

--
Lee Jones
Linaro ST-Ericsson Landing Team Lead
Linaro.org │ Open source software for ARM SoCs
Follow Linaro: Facebook | Twitter | Blog

2013-08-27 15:51:46

by Rob Herring

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

On Tue, Aug 27, 2013 at 9:08 AM, Lee Jones <[email protected]> wrote:
> On Tue, 27 Aug 2013, Mark Rutland wrote:
>
>> On Tue, Aug 27, 2013 at 09:06:35AM +0100, Lee Jones wrote:
>> > On Fri, 23 Aug 2013, Mark Rutland wrote:
>> >
>> > > On Fri, Aug 23, 2013 at 08:56:07AM +0100, Lee Jones wrote:
>> > > > 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.
>> > > >
>> > > > > > 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:
>> > >
>> > > 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.
>> >
>> > 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.

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.

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

2013-08-27 16:15:53

by Pawel Moll

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

On Tue, 2013-08-27 at 16:51 +0100, Rob Herring wrote:
> >> * 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

2013-09-12 12:50:05

by Linus Walleij

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

On Tue, Aug 27, 2013 at 10:23 AM, Lee Jones <[email protected]> wrote:
> On Fri, 23 Aug 2013, Linus Walleij wrote:
>> On Thu, Aug 22, 2013 at 11:21 AM, Lee Jones <[email protected]> wrote:

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

2013-09-12 14:56:58

by Lee Jones

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

On Thu, 12 Sep 2013, Linus Walleij wrote:

> On Tue, Aug 27, 2013 at 10:23 AM, Lee Jones <[email protected]> wrote:
> > On Fri, 23 Aug 2013, Linus Walleij wrote:
> >> On Thu, Aug 22, 2013 at 11:21 AM, Lee Jones <[email protected]> wrote:
>
> >> > 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.)
> >
> > 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...

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?

--
Lee Jones
Linaro STMicroelectronics Landing Team Lead
Linaro.org │ Open source software for ARM SoCs
Follow Linaro: Facebook | Twitter | Blog

2013-09-13 07:20:07

by Linus Walleij

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

On Thu, Sep 12, 2013 at 4:56 PM, Lee Jones <[email protected]> wrote:
> On Thu, 12 Sep 2013, Linus Walleij wrote:

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