ARM: SAMSUNG: Add support for sdhci clock lookup using generic names

Add support for lookup of sdhci-s3c controller clocks using generic names for s3c2416, s3c64xx, s5pc100, s5pv210 and exynos4 SoC's.

Signed-off-by Rajeshwari Shinde <[email protected]>

---
 arch/arm/mach-exynos4/clock.c         |   88 ++++++++++-------
 arch/arm/mach-s3c2416/clock.c         |   68 +++++++------
 arch/arm/mach-s3c64xx/clock.c         |  126 +++++++++++++++----------
 arch/arm/mach-s5pc100/clock.c         |  130 ++++++++++++++++----------
 arch/arm/mach-s5pv210/clock.c         |  167 ++++++++++++++++++++-------------
 arch/arm/plat-s3c24xx/s3c2443-clock.c |   15 ++-
 6 files changed, 359 insertions(+), 235 deletions(-)

arch/arm/mach-exynos4/clock.c +0 -36

index 9f50e33..c6383b9 100644
@@ -1157,42 +1157,6 @@ static struct clksrc_clk clksrcs[] = {
 		.reg_div = { .reg = S5P_CLKDIV_MFC, .shift = 0, .size = 4 },
 	}, {
 		.clk		= {
-			.name		= "sclk_mmc",
-			.devname	= "s3c-sdhci.0",
-			.parent		= &clk_dout_mmc0.clk,
-			.enable		= exynos4_clksrc_mask_fsys_ctrl,
-			.ctrlbit	= (1 << 0),
-		},
-		.reg_div = { .reg = S5P_CLKDIV_FSYS1, .shift = 8, .size = 8 },
-	}, {
-		.clk		= {
-			.name		= "sclk_mmc",
-			.devname	= "s3c-sdhci.1",
-			.parent         = &clk_dout_mmc1.clk,
-			.enable		= exynos4_clksrc_mask_fsys_ctrl,
-			.ctrlbit	= (1 << 4),
-		},
-		.reg_div = { .reg = S5P_CLKDIV_FSYS1, .shift = 24, .size = 8 },
-	}, {
-		.clk		= {
-			.name		= "sclk_mmc",
-			.devname	= "s3c-sdhci.2",
-			.parent         = &clk_dout_mmc2.clk,
-			.enable		= exynos4_clksrc_mask_fsys_ctrl,
-			.ctrlbit	= (1 << 8),
-		},
-		.reg_div = { .reg = S5P_CLKDIV_FSYS2, .shift = 8, .size = 8 },
-	}, {
-		.clk		= {
-			.name		= "sclk_mmc",
-			.devname	= "s3c-sdhci.3",
-			.parent         = &clk_dout_mmc3.clk,
-			.enable		= exynos4_clksrc_mask_fsys_ctrl,
-			.ctrlbit	= (1 << 12),
-		},
-		.reg_div = { .reg = S5P_CLKDIV_FSYS2, .shift = 24, .size = 8 },
-	}, {
-		.clk		= {
 			.name		= "sclk_dwmmc",
 			.parent         = &clk_dout_mmc4.clk,
 			.enable		= exynos4_clksrc_mask_fsys_ctrl,

arch/arm/mach-exynos4/clock.c +40 -0

+		.parent		= &clk_dout_mmc0.clk,
+		.enable		= exynos4_clksrc_mask_fsys_ctrl,
+		.ctrlbit	= (1 << 0),
+	},
+	.reg_div = { .reg = S5P_CLKDIV_FSYS1, .shift = 8, .size = 8 },
+};
+
+static struct clksrc_clk clk_sclk_mmc1 = {
+	.clk		= {
+		.name		= "sclk_mmc",
+		.devname	= "s3c-sdhci.1",
 
+		.parent         = &clk_dout_mmc1.clk,
+		.enable		= exynos4_clksrc_mask_fsys_ctrl,
+		.ctrlbit	= (1 << 4),
+	},
+	.reg_div = { .reg = S5P_CLKDIV_FSYS1, .shift = 24, .size = 8 },
+};
+
+static struct clksrc_clk clk_sclk_mmc2 = {
+	.clk		= {
+		.name		= "sclk_mmc",
+		.devname	= "s3c-sdhci.2",
 
+		.parent         = &clk_dout_mmc2.clk,
+		.enable		= exynos4_clksrc_mask_fsys_ctrl,
+		.ctrlbit	= (1 << 8),
+	},
+	.reg_div = { .reg = S5P_CLKDIV_FSYS2, .shift = 8, .size = 8 },
+};
+
+static struct clksrc_clk clk_sclk_mmc3 = {
+	.clk		= {
+		.name		= "sclk_mmc",
+		.devname	= "s3c-sdhci.3",
 
+		.parent         = &clk_dout_mmc3.clk,
+		.enable		= exynos4_clksrc_mask_fsys_ctrl,
+		.ctrlbit	= (1 << 12),
+	},
+	.reg_div = { .reg = S5P_CLKDIV_FSYS2, .shift = 24, .size = 8 },
+};
+

/* Clock initialization code */
static struct clksrc_clk *sysclks[] = {

arch/arm/mach-exynos4/clock.c +0 -0

 	&clk_mout_apll,

Hi Rajeshwari,

Quoted 64 lines · Rajeshwari Shinde

Add support for lookup of sdhci-s3c controller clocks using generic names for s3c2416, s3c64xx, s5pc100, s5pv210 and exynos4 SoC's.

Signed-off-by Rajeshwari Shinde <[email protected]>

---

?arch/arm/mach-exynos4/clock.c ? ? ? ? | ? 88 ++++++++++-------
?arch/arm/mach-s3c2416/clock.c ? ? ? ? | ? 68 +++++++------
?arch/arm/mach-s3c64xx/clock.c ? ? ? ? | ?126 +++++++++++++++----------
?arch/arm/mach-s5pc100/clock.c ? ? ? ? | ?130 ++++++++++++++++----------
?arch/arm/mach-s5pv210/clock.c ? ? ? ? | ?167 ++++++++++++++++++++-------------
?arch/arm/plat-s3c24xx/s3c2443-clock.c | ? 15 ++-

?6 files changed, 359 insertions(+), 235 deletions(-)

arch/arm/mach-exynos4/clock.c +0 -36

index 9f50e33..c6383b9 100644
@@ -1157,42 +1157,6 @@ static struct clksrc_clk clksrcs[] = {
 ? ? ? ? ? ? ? .reg_div = { .reg = S5P_CLKDIV_MFC, .shift = 0, .size = 4 },
 ? ? ? }, {
 ? ? ? ? ? ? ? .clk ? ? ? ? ? ?= {
- ? ? ? ? ? ? ? ? ? ? .name ? ? ? ? ? = "sclk_mmc",
- ? ? ? ? ? ? ? ? ? ? .devname ? ? ? ?= "s3c-sdhci.0",
- ? ? ? ? ? ? ? ? ? ? .parent ? ? ? ? = &clk_dout_mmc0.clk,
- ? ? ? ? ? ? ? ? ? ? .enable ? ? ? ? = exynos4_clksrc_mask_fsys_ctrl,
- ? ? ? ? ? ? ? ? ? ? .ctrlbit ? ? ? ?= (1 << 0),
- ? ? ? ? ? ? },
- ? ? ? ? ? ? .reg_div = { .reg = S5P_CLKDIV_FSYS1, .shift = 8, .size = 8 },
- ? ? }, {
- ? ? ? ? ? ? .clk ? ? ? ? ? ?= {
- ? ? ? ? ? ? ? ? ? ? .name ? ? ? ? ? = "sclk_mmc",
- ? ? ? ? ? ? ? ? ? ? .devname ? ? ? ?= "s3c-sdhci.1",
- ? ? ? ? ? ? ? ? ? ? .parent ? ? ? ? = &clk_dout_mmc1.clk,
- ? ? ? ? ? ? ? ? ? ? .enable ? ? ? ? = exynos4_clksrc_mask_fsys_ctrl,
- ? ? ? ? ? ? ? ? ? ? .ctrlbit ? ? ? ?= (1 << 4),
- ? ? ? ? ? ? },
- ? ? ? ? ? ? .reg_div = { .reg = S5P_CLKDIV_FSYS1, .shift = 24, .size = 8 },
- ? ? }, {
- ? ? ? ? ? ? .clk ? ? ? ? ? ?= {
- ? ? ? ? ? ? ? ? ? ? .name ? ? ? ? ? = "sclk_mmc",
- ? ? ? ? ? ? ? ? ? ? .devname ? ? ? ?= "s3c-sdhci.2",
- ? ? ? ? ? ? ? ? ? ? .parent ? ? ? ? = &clk_dout_mmc2.clk,
- ? ? ? ? ? ? ? ? ? ? .enable ? ? ? ? = exynos4_clksrc_mask_fsys_ctrl,
- ? ? ? ? ? ? ? ? ? ? .ctrlbit ? ? ? ?= (1 << 8),
- ? ? ? ? ? ? },
- ? ? ? ? ? ? .reg_div = { .reg = S5P_CLKDIV_FSYS2, .shift = 8, .size = 8 },
- ? ? }, {
- ? ? ? ? ? ? .clk ? ? ? ? ? ?= {
- ? ? ? ? ? ? ? ? ? ? .name ? ? ? ? ? = "sclk_mmc",
- ? ? ? ? ? ? ? ? ? ? .devname ? ? ? ?= "s3c-sdhci.3",
- ? ? ? ? ? ? ? ? ? ? .parent ? ? ? ? = &clk_dout_mmc3.clk,
- ? ? ? ? ? ? ? ? ? ? .enable ? ? ? ? = exynos4_clksrc_mask_fsys_ctrl,
- ? ? ? ? ? ? ? ? ? ? .ctrlbit ? ? ? ?= (1 << 12),
- ? ? ? ? ? ? },
- ? ? ? ? ? ? .reg_div = { .reg = S5P_CLKDIV_FSYS2, .shift = 24, .size = 8 },
- ? ? }, {
- ? ? ? ? ? ? .clk ? ? ? ? ? ?= {
 ? ? ? ? ? ? ? ? ? ? ? .name ? ? ? ? ? = "sclk_dwmmc",
 ? ? ? ? ? ? ? ? ? ? ? .parent ? ? ? ? = &clk_dout_mmc4.clk,
 ? ? ? ? ? ? ? ? ? ? ? .enable ? ? ? ? = exynos4_clksrc_mask_fsys_ctrl,

@@ -1250,6 +1214,50 @@ static struct clksrc_clk clk_sclk_uart3 = {
 ? ? ? .reg_div = { .reg = S5P_CLKDIV_PERIL0, .shift = 12, .size = 4 },
 ?};
 

arch/arm/mach-exynos4/clock.c static struct clksrc_clk clk_sclk_uart3 = { · line 1220

Quoted 4 lines

arch/arm/mach-exynos4/clock.c +4 -0

+static struct clksrc_clk clk_sclk_mmc0 = {
+ ? ? .clk ? ? ? ? ? ?= {
+ ? ? ? ? ? ? .name ? ? ? ? ? = "sclk_mmc",
+ ? ? ? ? ? ? .devname ? ? ? ?= "s3c-sdhci.0",

Would it make sense to drop this 'devname' field here and others until sclk_mmc3 ....

Quoted 49 lines

arch/arm/mach-exynos4/clock.c +40 -0

+ ? ? ? ? ? ? .parent ? ? ? ? = &clk_dout_mmc0.clk,
+ ? ? ? ? ? ? .enable ? ? ? ? = exynos4_clksrc_mask_fsys_ctrl,
+ ? ? ? ? ? ? .ctrlbit ? ? ? ?= (1 << 0),
+ ? ? },
+ ? ? .reg_div = { .reg = S5P_CLKDIV_FSYS1, .shift = 8, .size = 8 },
+};
+
+static struct clksrc_clk clk_sclk_mmc1 = {
+ ? ? .clk ? ? ? ? ? ?= {
+ ? ? ? ? ? ? .name ? ? ? ? ? = "sclk_mmc",
+ ? ? ? ? ? ? .devname ? ? ? ?= "s3c-sdhci.1",
 
+ ? ? ? ? ? ? .parent ? ? ? ? = &clk_dout_mmc1.clk,
+ ? ? ? ? ? ? .enable ? ? ? ? = exynos4_clksrc_mask_fsys_ctrl,
+ ? ? ? ? ? ? .ctrlbit ? ? ? ?= (1 << 4),
+ ? ? },
+ ? ? .reg_div = { .reg = S5P_CLKDIV_FSYS1, .shift = 24, .size = 8 },
+};
+
+static struct clksrc_clk clk_sclk_mmc2 = {
+ ? ? .clk ? ? ? ? ? ?= {
+ ? ? ? ? ? ? .name ? ? ? ? ? = "sclk_mmc",
+ ? ? ? ? ? ? .devname ? ? ? ?= "s3c-sdhci.2",
 
+ ? ? ? ? ? ? .parent ? ? ? ? = &clk_dout_mmc2.clk,
+ ? ? ? ? ? ? .enable ? ? ? ? = exynos4_clksrc_mask_fsys_ctrl,
+ ? ? ? ? ? ? .ctrlbit ? ? ? ?= (1 << 8),
+ ? ? },
+ ? ? .reg_div = { .reg = S5P_CLKDIV_FSYS2, .shift = 8, .size = 8 },
+};
+
+static struct clksrc_clk clk_sclk_mmc3 = {
+ ? ? .clk ? ? ? ? ? ?= {
+ ? ? ? ? ? ? .name ? ? ? ? ? = "sclk_mmc",
+ ? ? ? ? ? ? .devname ? ? ? ?= "s3c-sdhci.3",
 
+ ? ? ? ? ? ? .parent ? ? ? ? = &clk_dout_mmc3.clk,
+ ? ? ? ? ? ? .enable ? ? ? ? = exynos4_clksrc_mask_fsys_ctrl,
+ ? ? ? ? ? ? .ctrlbit ? ? ? ?= (1 << 12),
+ ? ? },
+ ? ? .reg_div = { .reg = S5P_CLKDIV_FSYS2, .shift = 24, .size = 8 },
+};
+

?/* Clock initialization code */
?static struct clksrc_clk *sysclks[] = {
? ? ? &clk_mout_apll,

arch/arm/mach-exynos4/clock.c +0 -0

@@ -1289,6 +1297,10 @@ static struct clksrc_clk *clksrc_cdev[] = {
 ? ? ? &clk_sclk_uart1,

arch/arm/mach-exynos4/clock.c static struct clksrc_clk *clksrc_cdev[] = { · line 1303

Quoted 6 lines

arch/arm/mach-exynos4/clock.c +4 -0

 ? ? ? &clk_sclk_uart2,
 ? ? ? &clk_sclk_uart3,
+ ? ? &clk_sclk_mmc0,
+ ? ? &clk_sclk_mmc1,
+ ? ? &clk_sclk_mmc2,
+ ? ? &clk_sclk_mmc3,

..then drop the above 4 lines...

arch/arm/mach-exynos4/clock.c static struct clk_lookup exynos4_clk_lookup[] = { · line 1314

Quoted 12 lines

arch/arm/mach-exynos4/clock.c +4 -0

 ?};
 
 ?static struct clk_lookup exynos4_clk_lookup[] = {

@@ -1296,6 +1308,10 @@ static struct clk_lookup exynos4_clk_lookup[] = {
 ? ? ? CLKDEV_INIT("exynos4210-uart.1", "clk_uart_baud0", &clk_sclk_uart1.clk),
 ? ? ? CLKDEV_INIT("exynos4210-uart.2", "clk_uart_baud0", &clk_sclk_uart2.clk),
 ? ? ? CLKDEV_INIT("exynos4210-uart.3", "clk_uart_baud0", &clk_sclk_uart3.clk),
+ ? ? CLKDEV_INIT("exynos4-sdhci.0", "mmc_busclk.2", &clk_sclk_mmc0.clk),
+ ? ? CLKDEV_INIT("exynos4-sdhci.1", "mmc_busclk.2", &clk_sclk_mmc1.clk),
+ ? ? CLKDEV_INIT("exynos4-sdhci.2", "mmc_busclk.2", &clk_sclk_mmc2.clk),
+ ? ? CLKDEV_INIT("exynos4-sdhci.3", "mmc_busclk.2", &clk_sclk_mmc3.clk),

..and add something like:

?+ ? ? ?CLKDEV_INIT("s3c-sdhci.0", "sclk_mmc", &clk_sclk_mmc0.clk), ?+ ? ? ?CLKDEV_INIT("s3c-sdhci.1", "sclk_mmc", &clk_sclk_mmc1.clk), ?+ ? ? ?CLKDEV_INIT("s3c-sdhci.2", "sclk_mmc", &clk_sclk_mmc2.clk), ?+ ? ? ?CLKDEV_INIT("s3c-sdhci.3", "sclk_mmc", &clk_sclk_mmc3.clk),

?

Also I'm wondering why we're using different device names for clk_sclk_mmc0..3 clocks, i.e. exynos4-sdhci.? and s3c-sdhci.? ?

Does it all work on exynos ? I would expect the device name to be same across all the clock definitions, otherwise clk_get(dev, ..) will fail.

arch/arm/mach-exynos4/clock.c static struct clk_lookup exynos4_clk_lookup[] = { · line 1316

Quoted 3 lines

arch/arm/mach-exynos4/clock.c +0 -0

 ?};
 

Regards
--
Sylwester Nawrocki
Samsung Poland R&D Center
--
To unsubscribe from this list: send the line "unsubscribe linux-mmc" in the body of a message to majordomo at vger.kernel.org
More majordomo info at ?http://vger.kernel.org/majordomo-info.html

Quoted 22 lines · Sylwester Nawrocki

Hi Rajeshwari,

Quoted 19 lines · Rajeshwari Shinde

Add support for lookup of sdhci-s3c controller clocks using generic names for s3c2416, s3c64xx, s5pc100, s5pv210 and exynos4 SoC's.

Signed-off-by Rajeshwari Shinde <[email protected]>

---
 arch/arm/mach-exynos4/clock.c         |   88 ++++++++++-------
 arch/arm/mach-s3c2416/clock.c         |   68 +++++++------
 arch/arm/mach-s3c64xx/clock.c         |  126 +++++++++++++++----------
 arch/arm/mach-s5pc100/clock.c         |  130 ++++++++++++++++----------
 arch/arm/mach-s5pv210/clock.c         |  167 ++++++++++++++++++++-------------
 arch/arm/plat-s3c24xx/s3c2443-clock.c |   15 ++-
 6 files changed, 359 insertions(+), 235 deletions(-)

arch/arm/mach-exynos4/clock.c +0 -0

index 9f50e33..c6383b9 100644
@@ -1157,42 +1157,6 @@ static struct clksrc_clk clksrcs[] = {
              .reg_div = { .reg = S5P_CLKDIV_MFC, .shift = 0, .size = 4 },

Quoted 7 lines

arch/arm/mach-exynos4/clock.c static struct clksrc_clk clk_sclk_uart3 = { · line 1220

Quoted 4 lines

arch/arm/mach-exynos4/clock.c +4 -0

+static struct clksrc_clk clk_sclk_mmc0 = {
+     .clk            = {
+             .name           = "sclk_mmc",
+             .devname        = "s3c-sdhci.0",

Would it make sense to drop this 'devname' field here and others until sclk_mmc3 ....

*** The devname here distinguishes these clocks. So it should be okay to have a devname for these clocks.

Quoted 50 lines

Quoted 50 lines

arch/arm/mach-exynos4/clock.c +40 -0

+             .parent         = &clk_dout_mmc0.clk,
+             .enable         = exynos4_clksrc_mask_fsys_ctrl,
+             .ctrlbit        = (1 << 0),
+     },
+     .reg_div = { .reg = S5P_CLKDIV_FSYS1, .shift = 8, .size = 8 },
+};
+
+static struct clksrc_clk clk_sclk_mmc1 = {
+     .clk            = {
+             .name           = "sclk_mmc",
+             .devname        = "s3c-sdhci.1",
 
+             .parent         = &clk_dout_mmc1.clk,
+             .enable         = exynos4_clksrc_mask_fsys_ctrl,
+             .ctrlbit        = (1 << 4),
+     },
+     .reg_div = { .reg = S5P_CLKDIV_FSYS1, .shift = 24, .size = 8 },
+};
+
+static struct clksrc_clk clk_sclk_mmc2 = {
+     .clk            = {
+             .name           = "sclk_mmc",
+             .devname        = "s3c-sdhci.2",
 
+             .parent         = &clk_dout_mmc2.clk,
+             .enable         = exynos4_clksrc_mask_fsys_ctrl,
+             .ctrlbit        = (1 << 8),
+     },
+     .reg_div = { .reg = S5P_CLKDIV_FSYS2, .shift = 8, .size = 8 },
+};
+
+static struct clksrc_clk clk_sclk_mmc3 = {
+     .clk            = {
+             .name           = "sclk_mmc",
+             .devname        = "s3c-sdhci.3",
 
+             .parent         = &clk_dout_mmc3.clk,
+             .enable         = exynos4_clksrc_mask_fsys_ctrl,
+             .ctrlbit        = (1 << 12),
+     },
+     .reg_div = { .reg = S5P_CLKDIV_FSYS2, .shift = 24, .size = 8 },
+};
+
 /* Clock initialization code */
 static struct clksrc_clk *sysclks[] = {
      &clk_mout_apll,

@@ -1289,6 +1297,10 @@ static struct clksrc_clk *clksrc_cdev[] = {
      &clk_sclk_uart1,

Quoted 8 lines

arch/arm/mach-exynos4/clock.c static struct clksrc_clk *clksrc_cdev[] = { · line 1303

Quoted 6 lines

arch/arm/mach-exynos4/clock.c +4 -0

      &clk_sclk_uart2,
      &clk_sclk_uart3,
+     &clk_sclk_mmc0,
+     &clk_sclk_mmc1,
+     &clk_sclk_mmc2,
+     &clk_sclk_mmc3,

..then drop the above 4 lines...

**** The registration for these clocks are important. The s3c_register_clksrc() function sets the .ops of this clock and also its parent. So the registration cannot be dropped.

Quoted 21 lines

arch/arm/mach-exynos4/clock.c static struct clk_lookup exynos4_clk_lookup[] = { · line 1314

Quoted 13 lines

arch/arm/mach-exynos4/clock.c +4 -0

 };
 
 static struct clk_lookup exynos4_clk_lookup[] = {

@@ -1296,6 +1308,10 @@ static struct clk_lookup exynos4_clk_lookup[] = {
      CLKDEV_INIT("exynos4210-uart.1", "clk_uart_baud0", &clk_sclk_uart1.clk),
      CLKDEV_INIT("exynos4210-uart.2", "clk_uart_baud0", &clk_sclk_uart2.clk),
      CLKDEV_INIT("exynos4210-uart.3", "clk_uart_baud0", &clk_sclk_uart3.clk),
+     CLKDEV_INIT("exynos4-sdhci.0", "mmc_busclk.2", &clk_sclk_mmc0.clk),
+     CLKDEV_INIT("exynos4-sdhci.1", "mmc_busclk.2", &clk_sclk_mmc1.clk),
+     CLKDEV_INIT("exynos4-sdhci.2", "mmc_busclk.2", &clk_sclk_mmc2.clk),
+     CLKDEV_INIT("exynos4-sdhci.3", "mmc_busclk.2", &clk_sclk_mmc3.clk),

..and add something like:

 +      CLKDEV_INIT("s3c-sdhci.0", "sclk_mmc", &clk_sclk_mmc0.clk),
 +      CLKDEV_INIT("s3c-sdhci.1", "sclk_mmc", &clk_sclk_mmc1.clk),
 +      CLKDEV_INIT("s3c-sdhci.2", "sclk_mmc", &clk_sclk_mmc2.clk),
 +      CLKDEV_INIT("s3c-sdhci.3", "sclk_mmc", &clk_sclk_mmc3.clk),

Quoted 1 line

?

**** The driver uses a common name for the possible bus clock sources, that is ?mmc_busclk?. This keeps the clock lookup code in the driver simple. Also, there could be SoC?s which do no use sclk_mmc as the bus clock name as per the user manual

Quoted 6 lines

Also I'm wondering why we're using different device names for clk_sclk_mmc0..3 clocks, i.e. exynos4-sdhci.? and s3c-sdhci.? ?

Quoted 2 lines

Does it all work on exynos ? I would expect the device name to be same across all the clock definitions, otherwise clk_get(dev, ..) will fail.

**** There was a patch submitted to rename the device name of sdhci for Exynos to exynos4-sdhci. I will remove this change from this patch and let that patch handle this change.

Quoted 24 lines · Rajeshwari Birje

Quoted 22 lines · Sylwester Nawrocki

Hi Rajeshwari,

Quoted 19 lines · Rajeshwari Shinde

Add support for lookup of sdhci-s3c controller clocks using generic names for s3c2416, s3c64xx, s5pc100, s5pv210 and exynos4 SoC's.

Signed-off-by Rajeshwari Shinde <[email protected]>

---

?arch/arm/mach-exynos4/clock.c ? ? ? ? | ? 88 ++++++++++-------
?arch/arm/mach-s3c2416/clock.c ? ? ? ? | ? 68 +++++++------
?arch/arm/mach-s3c64xx/clock.c ? ? ? ? | ?126 +++++++++++++++----------
?arch/arm/mach-s5pc100/clock.c ? ? ? ? | ?130 ++++++++++++++++----------
?arch/arm/mach-s5pv210/clock.c ? ? ? ? | ?167 ++++++++++++++++++++-------------
?arch/arm/plat-s3c24xx/s3c2443-clock.c | ? 15 ++-

?6 files changed, 359 insertions(+), 235 deletions(-)

arch/arm/mach-exynos4/clock.c +0 -0

index 9f50e33..c6383b9 100644
@@ -1157,42 +1157,6 @@ static struct clksrc_clk clksrcs[] = {
 ? ? ? ? ? ? ? .reg_div = { .reg = S5P_CLKDIV_MFC, .shift = 0, .size = 4 },

...

Quoted 8 lines

Quoted 7 lines

Quoted 4 lines

+static struct clksrc_clk clk_sclk_mmc0 = {
+ ? ? .clk ? ? ? ? ? ?= {
+ ? ? ? ? ? ? .name ? ? ? ? ? = "sclk_mmc",
+ ? ? ? ? ? ? .devname ? ? ? ?= "s3c-sdhci.0",

Would it make sense to drop this 'devname' field here and others until sclk_mmc3 ....

Quoted 2 lines

*** The devname here distinguishes these clocks. So it should be okay to have a devname for these clocks.

I'm not sure what's Mr Kukjin's opinion on that, but I personally would really like to see all the devname fields disappear from samsung clk data structures. Possibly if all involved people would keep that in mind we could achieve this over time.

Quoted 60 lines

Quoted 51 lines

Quoted 51 lines

+ ? ? ? ? ? ? .parent ? ? ? ? = &clk_dout_mmc0.clk,
+ ? ? ? ? ? ? .enable ? ? ? ? = exynos4_clksrc_mask_fsys_ctrl,
+ ? ? ? ? ? ? .ctrlbit ? ? ? ?= (1 << 0),
+ ? ? },
+ ? ? .reg_div = { .reg = S5P_CLKDIV_FSYS1, .shift = 8, .size = 8 },

arch/arm/mach-exynos4/clock.c +35 -0

+};
+
+static struct clksrc_clk clk_sclk_mmc1 = {
+ ? ? .clk ? ? ? ? ? ?= {
+ ? ? ? ? ? ? .name ? ? ? ? ? = "sclk_mmc",
+ ? ? ? ? ? ? .devname ? ? ? ?= "s3c-sdhci.1",
 
+ ? ? ? ? ? ? .parent ? ? ? ? = &clk_dout_mmc1.clk,
+ ? ? ? ? ? ? .enable ? ? ? ? = exynos4_clksrc_mask_fsys_ctrl,
+ ? ? ? ? ? ? .ctrlbit ? ? ? ?= (1 << 4),
+ ? ? },
+ ? ? .reg_div = { .reg = S5P_CLKDIV_FSYS1, .shift = 24, .size = 8 },
+};
+
+static struct clksrc_clk clk_sclk_mmc2 = {
+ ? ? .clk ? ? ? ? ? ?= {
+ ? ? ? ? ? ? .name ? ? ? ? ? = "sclk_mmc",
+ ? ? ? ? ? ? .devname ? ? ? ?= "s3c-sdhci.2",
 
+ ? ? ? ? ? ? .parent ? ? ? ? = &clk_dout_mmc2.clk,
+ ? ? ? ? ? ? .enable ? ? ? ? = exynos4_clksrc_mask_fsys_ctrl,
+ ? ? ? ? ? ? .ctrlbit ? ? ? ?= (1 << 8),
+ ? ? },
+ ? ? .reg_div = { .reg = S5P_CLKDIV_FSYS2, .shift = 8, .size = 8 },
+};
+
+static struct clksrc_clk clk_sclk_mmc3 = {
+ ? ? .clk ? ? ? ? ? ?= {
+ ? ? ? ? ? ? .name ? ? ? ? ? = "sclk_mmc",
+ ? ? ? ? ? ? .devname ? ? ? ?= "s3c-sdhci.3",
 
+ ? ? ? ? ? ? .parent ? ? ? ? = &clk_dout_mmc3.clk,
+ ? ? ? ? ? ? .enable ? ? ? ? = exynos4_clksrc_mask_fsys_ctrl,
+ ? ? ? ? ? ? .ctrlbit ? ? ? ?= (1 << 12),
+ ? ? },
+ ? ? .reg_div = { .reg = S5P_CLKDIV_FSYS2, .shift = 24, .size = 8 },
+};
+

?/* Clock initialization code */
?static struct clksrc_clk *sysclks[] = {
? ? ? &clk_mout_apll,

arch/arm/mach-exynos4/clock.c +0 -0

@@ -1289,6 +1297,10 @@ static struct clksrc_clk *clksrc_cdev[] = {
 ? ? ? &clk_sclk_uart1,

Quoted 8 lines

arch/arm/mach-exynos4/clock.c static struct clksrc_clk *clksrc_cdev[] = { · line 1303

Quoted 6 lines

arch/arm/mach-exynos4/clock.c +4 -0

 ? ? ? &clk_sclk_uart2,
 ? ? ? &clk_sclk_uart3,
+ ? ? &clk_sclk_mmc0,
+ ? ? &clk_sclk_mmc1,
+ ? ? &clk_sclk_mmc2,
+ ? ? &clk_sclk_mmc3,

..then drop the above 4 lines...

Quoted 3 lines

**** The registration for these clocks are important. The s3c_register_clksrc() function sets the .ops of this clock and also its parent. So the registration cannot be dropped.

OK.

Quoted 21 lines

Quoted 21 lines

arch/arm/mach-exynos4/clock.c static struct clk_lookup exynos4_clk_lookup[] = { · line 1314

Quoted 13 lines

arch/arm/mach-exynos4/clock.c +4 -0

 ?};
 
 ?static struct clk_lookup exynos4_clk_lookup[] = {

@@ -1296,6 +1308,10 @@ static struct clk_lookup exynos4_clk_lookup[] = {
 ? ? ? CLKDEV_INIT("exynos4210-uart.1", "clk_uart_baud0", &clk_sclk_uart1.clk),
 ? ? ? CLKDEV_INIT("exynos4210-uart.2", "clk_uart_baud0", &clk_sclk_uart2.clk),
 ? ? ? CLKDEV_INIT("exynos4210-uart.3", "clk_uart_baud0", &clk_sclk_uart3.clk),
+ ? ? CLKDEV_INIT("exynos4-sdhci.0", "mmc_busclk.2", &clk_sclk_mmc0.clk),
+ ? ? CLKDEV_INIT("exynos4-sdhci.1", "mmc_busclk.2", &clk_sclk_mmc1.clk),
+ ? ? CLKDEV_INIT("exynos4-sdhci.2", "mmc_busclk.2", &clk_sclk_mmc2.clk),
+ ? ? CLKDEV_INIT("exynos4-sdhci.3", "mmc_busclk.2", &clk_sclk_mmc3.clk),

..and add something like:

?+ ? ? ?CLKDEV_INIT("s3c-sdhci.0", "sclk_mmc", &clk_sclk_mmc0.clk), ?+ ? ? ?CLKDEV_INIT("s3c-sdhci.1", "sclk_mmc", &clk_sclk_mmc1.clk), ?+ ? ? ?CLKDEV_INIT("s3c-sdhci.2", "sclk_mmc", &clk_sclk_mmc2.clk), ?+ ? ? ?CLKDEV_INIT("s3c-sdhci.3", "sclk_mmc", &clk_sclk_mmc3.clk),

Quoted 6 lines

Quoted 1 line

?

**** The driver uses a common name for the possible bus clock sources, that is ?mmc_busclk?. This keeps the clock lookup code in the driver simple. Also, there could be SoC?s which do no use sclk_mmc as the bus clock name as per the user manual

OK, I was aware of that. I didn't mean removing the "mmc_busclk.2" entries, just adding another 4, which are created anyway by s3c_register_clksrc() function. But for now I think the patch is OK.

Quoted 9 lines

Quoted 8 lines

Also I'm wondering why we're using different device names for clk_sclk_mmc0..3 clocks, i.e. exynos4-sdhci.? and s3c-sdhci.? ?

Does it all work on exynos ? I would expect the device name to be same across all the clock definitions, otherwise clk_get(dev, ..) will fail.

Quoted 3 lines

**** There was a patch submitted to rename the device name of sdhci for Exynos to exynos4-sdhci. I will remove this change from this patch and let that patch handle this change.

I wouldn't do that. IMHO it's better to keep this patch as is, to make the final diff size lower.

Perhaps it's even worth to consider doing the clock rename altogether in this patch. But it's of course up to you.

Regards,
--
Sylwester Nawrocki
Samsung Poland R&D Center

Quoted 34 lines · Sylwester Nawrocki

Quoted 24 lines · Rajeshwari Birje

Quoted 22 lines · Sylwester Nawrocki

Hi Rajeshwari,

Quoted 19 lines · Rajeshwari Shinde

Add support for lookup of sdhci-s3c controller clocks using generic names for s3c2416, s3c64xx, s5pc100, s5pv210 and exynos4 SoC's.

Signed-off-by Rajeshwari Shinde <[email protected]>

---
 arch/arm/mach-exynos4/clock.c         |   88 ++++++++++-------
 arch/arm/mach-s3c2416/clock.c         |   68 +++++++------
 arch/arm/mach-s3c64xx/clock.c         |  126 +++++++++++++++----------
 arch/arm/mach-s5pc100/clock.c         |  130 ++++++++++++++++----------
 arch/arm/mach-s5pv210/clock.c         |  167 ++++++++++++++++++++-------------
 arch/arm/plat-s3c24xx/s3c2443-clock.c |   15 ++-
 6 files changed, 359 insertions(+), 235 deletions(-)

arch/arm/mach-exynos4/clock.c +0 -0

index 9f50e33..c6383b9 100644
@@ -1157,42 +1157,6 @@ static struct clksrc_clk clksrcs[] = {
              .reg_div = { .reg = S5P_CLKDIV_MFC, .shift = 0, .size = 4 },

...

Quoted 8 lines

Quoted 7 lines

arch/arm/mach-exynos4/clock.c static struct clksrc_clk clk_sclk_uart3 = { · line 1220

Quoted 4 lines

arch/arm/mach-exynos4/clock.c +4 -0

+static struct clksrc_clk clk_sclk_mmc0 = {
+     .clk            = {
+             .name           = "sclk_mmc",
+             .devname        = "s3c-sdhci.0",

Would it make sense to drop this 'devname' field here and others until sclk_mmc3 ....

Quoted 7 lines

Quoted 2 lines

*** The devname here distinguishes these clocks. So it should be okay to have a devname for these clocks.

I'm not sure what's Mr Kukjin's opinion on that, but I personally would really like to see all the devname fields disappear from samsung clk data structures. Possibly if all involved people would keep that in mind we could achieve this over time.

**** Devname field is still required for clocks like hsmmc for driver to work fine. Hence it would be tough to remove the devname completely.

Yes, it allowed to add clkdev support with minimal changes required.

But let's see what happens when the works on common struct clk for all architectures are more or less completed and we convert Samsung platforms to use it;)

Quoted 5 lines · Sylwester Nawrocki

Yes, it allowed to add clkdev support with minimal changes required.

But let's see what happens when the works on common struct clk for all architectures are more or less completed and we convert Samsung platforms to use it;)

The common struct clk shouldn't have much to do with the clkdev part of it - clkdev just provides a method to associate a struct clk with the drivers request. The common struct clk doesn't change that.