All of lore.kernel.org
 help / color / mirror / Atom feed
* [RESEND LIST PATCHv7 0/4] socfpga: Enable SD/MMC support
@ 2013-12-16 17:04 ` dinguyen at altera.com
  0 siblings, 0 replies; 28+ messages in thread
From: dinguyen @ 2013-12-16 17:04 UTC (permalink / raw)
  To: dinh.linux, arnd, cjb, jh80.chung, tgih.jun, heiko, dianders,
	alim.akhtar, bzhao, mturquette
  Cc: zhangfei.gao, linux-mmc, linux-arm-kernel, devicetree, Dinh Nguyen

From: Dinh Nguyen <dinguyen@altera.com>

RESEND: Apologies, re-send with CC to all the appropriate lists.

Hi,

This is v7 of the patch series to enable SD/MMC on the SOCFPGA platform.

V7 differences from V6:

* Add a new clock binding property clk-phase. This property is used to
  represent the 2 clock phase values that is needed for the SD/MMC driver.

Thanks,


Dinh Nguyen (4):
  clk: socfpga: Add a clk-phase property to the "altr,socfpga-gate-clk"
  dts: socfpga: Add support for SD/MMC on the SOCFPGA platform
  mmc: dw_mmc-socfpga: Remove the SOCFPGA specific platform for dw_mmc
  ARM: socfpga_defconfig: enable SD/MMC support

 .../devicetree/bindings/clock/altr_socfpga.txt     |   14 ++
 arch/arm/boot/dts/socfpga.dtsi                     |   14 +-
 arch/arm/boot/dts/socfpga_arria5.dtsi              |   11 ++
 arch/arm/boot/dts/socfpga_cyclone5.dtsi            |   11 ++
 arch/arm/boot/dts/socfpga_vt.dts                   |   11 ++
 arch/arm/configs/socfpga_defconfig                 |    2 +
 drivers/clk/socfpga/clk.c                          |   37 ++++++
 drivers/mmc/host/Kconfig                           |    8 --
 drivers/mmc/host/dw_mmc-socfpga.c                  |  138 --------------------
 9 files changed, 99 insertions(+), 147 deletions(-)
 delete mode 100644 drivers/mmc/host/dw_mmc-socfpga.c

-- 
1.7.9.5



^ permalink raw reply	[flat|nested] 28+ messages in thread

* [RESEND LIST PATCHv7 0/4] socfpga: Enable SD/MMC support
@ 2013-12-16 17:04 ` dinguyen at altera.com
  0 siblings, 0 replies; 28+ messages in thread
From: dinguyen at altera.com @ 2013-12-16 17:04 UTC (permalink / raw)
  To: linux-arm-kernel

From: Dinh Nguyen <dinguyen@altera.com>

RESEND: Apologies, re-send with CC to all the appropriate lists.

Hi,

This is v7 of the patch series to enable SD/MMC on the SOCFPGA platform.

V7 differences from V6:

* Add a new clock binding property clk-phase. This property is used to
  represent the 2 clock phase values that is needed for the SD/MMC driver.

Thanks,


Dinh Nguyen (4):
  clk: socfpga: Add a clk-phase property to the "altr,socfpga-gate-clk"
  dts: socfpga: Add support for SD/MMC on the SOCFPGA platform
  mmc: dw_mmc-socfpga: Remove the SOCFPGA specific platform for dw_mmc
  ARM: socfpga_defconfig: enable SD/MMC support

 .../devicetree/bindings/clock/altr_socfpga.txt     |   14 ++
 arch/arm/boot/dts/socfpga.dtsi                     |   14 +-
 arch/arm/boot/dts/socfpga_arria5.dtsi              |   11 ++
 arch/arm/boot/dts/socfpga_cyclone5.dtsi            |   11 ++
 arch/arm/boot/dts/socfpga_vt.dts                   |   11 ++
 arch/arm/configs/socfpga_defconfig                 |    2 +
 drivers/clk/socfpga/clk.c                          |   37 ++++++
 drivers/mmc/host/Kconfig                           |    8 --
 drivers/mmc/host/dw_mmc-socfpga.c                  |  138 --------------------
 9 files changed, 99 insertions(+), 147 deletions(-)
 delete mode 100644 drivers/mmc/host/dw_mmc-socfpga.c

-- 
1.7.9.5

^ permalink raw reply	[flat|nested] 28+ messages in thread

* [RESEND LIST PATCHv7 1/4] clk: socfpga: Add a clk-phase property to the "altr,socfpga-gate-clk"
  2013-12-16 17:04 ` dinguyen at altera.com
@ 2013-12-16 17:04   ` dinguyen at altera.com
  -1 siblings, 0 replies; 28+ messages in thread
From: dinguyen @ 2013-12-16 17:04 UTC (permalink / raw)
  To: dinh.linux, arnd, cjb, jh80.chung, tgih.jun, heiko, dianders,
	alim.akhtar, bzhao, mturquette
  Cc: zhangfei.gao, linux-mmc, linux-arm-kernel, devicetree, Dinh Nguyen

From: Dinh Nguyen <dinguyen@altera.com>

The clk-phase property is used to represent the 2 clock phase values that is
needed for the SD/MMC driver. Add a prepare function to the clk_ops, that will
use the syscon driver to set sdmmc_clk's phase shift that is located in the
system manager.

Signed-off-by: Dinh Nguyen <dinguyen@altera.com>
---
v7: Add dts property to represent the clk phase of the sdmmc_clk. Add a
    prepare function to the gate clk that will toggle clock phase setting.
    Remove the "altr,socfpga-sdmmc-sdr-clk" clock type.
v6: Add a new clock type "altr,socfpga-sdmmc-sdr-clk" that will be used to
set the phase shift settings.
v5: Use the "snps,dw-mshc" binding
v4: Use the sdmmc_clk prepare function to set the phase shift settings
v3: Not use the syscon driver because as of 3.13-rc1, the syscon driver is
    loaded after the clock driver.
v2: Use the syscon driver
---
 .../devicetree/bindings/clock/altr_socfpga.txt     |   14 ++++++++
 arch/arm/boot/dts/socfpga.dtsi                     |    1 +
 drivers/clk/socfpga/clk.c                          |   36 ++++++++++++++++++++
 3 files changed, 51 insertions(+)

diff --git a/Documentation/devicetree/bindings/clock/altr_socfpga.txt b/Documentation/devicetree/bindings/clock/altr_socfpga.txt
index 0045433..4b66754 100644
--- a/Documentation/devicetree/bindings/clock/altr_socfpga.txt
+++ b/Documentation/devicetree/bindings/clock/altr_socfpga.txt
@@ -23,3 +23,17 @@ Optional properties:
         and the bit index.
 - div-reg : For "socfpga-gate-clk", div-reg contains the divider register, bit shift,
         and width.
+- clk-phase : For the sdmmc_clk, contains the value of the clock phase that controls
+  the SDMMC CIU clock. The first value is the clk_sample(smpsel), and the second
+  value is the cclk_in_drv(drvsel). The clk-phase is used to enable the correct
+  hold/delay times that is needed for the SD/MMC CIU clock. The values of both
+  value can be one of the following:
+
+	<0x0>	:	0 degrees
+	<0x1>	:	45 degrees
+	<0x2>	:	90 degrees
+	<0x3>	:	135 degrees
+	<0x4>	:	180 degrees
+	<0x5>	:	225 degrees
+	<0x6>	:	270 degrees
+	<0x7>	:	315 degrees
diff --git a/arch/arm/boot/dts/socfpga.dtsi b/arch/arm/boot/dts/socfpga.dtsi
index f936476..616d9ee 100644
--- a/arch/arm/boot/dts/socfpga.dtsi
+++ b/arch/arm/boot/dts/socfpga.dtsi
@@ -413,6 +413,7 @@
 						compatible = "altr,socfpga-gate-clk";
 						clocks = <&f2s_periph_ref_clk>, <&main_nand_sdmmc_clk>, <&per_nand_mmc_clk>;
 						clk-gate = <0xa0 8>;
+						clk-phase = <0 3>;
 					};
 
 					nand_x_clk: nand_x_clk {
diff --git a/drivers/clk/socfpga/clk.c b/drivers/clk/socfpga/clk.c
index 280c983..7cfebd6 100644
--- a/drivers/clk/socfpga/clk.c
+++ b/drivers/clk/socfpga/clk.c
@@ -21,8 +21,10 @@
 #include <linux/clkdev.h>
 #include <linux/clk-provider.h>
 #include <linux/io.h>
+#include <linux/mfd/syscon.h>
 #include <linux/of.h>
 #include <linux/of_address.h>
+#include <linux/regmap.h>
 
 /* Clock Manager offsets */
 #define CLKMGR_CTRL	0x0
@@ -56,6 +58,11 @@
 #define div_mask(width)	((1 << (width)) - 1)
 #define streq(a, b) (strcmp((a), (b)) == 0)
 
+/* SDMMC Group for System Manager defines */
+#define SYSMGR_SDMMCGRP_CTRL_OFFSET	0x108
+#define SYSMGR_SDMMC_CTRL_SET(smplsel, drvsel)	\
+	((((smplsel) & 0x7) << 3) | (((drvsel) & 0x7) << 0))
+
 static void __iomem *clk_mgr_base_addr;
 
 struct socfpga_clk {
@@ -66,6 +73,7 @@ struct socfpga_clk {
 	void __iomem *div_reg;
 	u32 width;	/* only valid if div_reg != 0 */
 	u32 shift;	/* only valid if div_reg != 0 */
+	u32 clk_phase[2];
 };
 #define to_socfpga_clk(p) container_of(p, struct socfpga_clk, hw.hw)
 
@@ -243,7 +251,28 @@ static unsigned long socfpga_clk_recalc_rate(struct clk_hw *hwclk,
 	return parent_rate / div;
 }
 
+static int socfpga_clk_prepare(struct clk_hw *hwclk)
+{
+	struct socfpga_clk *socfpgaclk = to_socfpga_clk(hwclk);
+	struct regmap *sys_mgr_base_addr;
+	u32 hs_timing;
+
+	if (socfpgaclk->clk_phase[0] || socfpgaclk->clk_phase[1]) {
+		sys_mgr_base_addr = syscon_regmap_lookup_by_compatible("altr,sys-mgr");
+		if (IS_ERR(sys_mgr_base_addr)) {
+			pr_err("%s: failed to find altr,sys-mgr regmap!\n", __func__);
+			return -EINVAL;
+		}
+		hs_timing = SYSMGR_SDMMC_CTRL_SET(socfpgaclk->clk_phase[0],
+						socfpgaclk->clk_phase[1]);
+		regmap_write(sys_mgr_base_addr, SYSMGR_SDMMCGRP_CTRL_OFFSET,
+						hs_timing);
+	}
+	return 0;
+}
+
 static struct clk_ops gateclk_ops = {
+	.prepare = socfpga_clk_prepare,
 	.recalc_rate = socfpga_clk_recalc_rate,
 	.get_parent = socfpga_clk_get_parent,
 	.set_parent = socfpga_clk_set_parent,
@@ -254,6 +283,7 @@ static void __init socfpga_gate_clk_init(struct device_node *node,
 {
 	u32 clk_gate[2];
 	u32 div_reg[3];
+	u32 clk_phase[2];
 	u32 fixed_div;
 	struct clk *clk;
 	struct socfpga_clk *socfpga_clk;
@@ -294,6 +324,12 @@ static void __init socfpga_gate_clk_init(struct device_node *node,
 		socfpga_clk->div_reg = 0;
 	}
 
+	rc = of_property_read_u32_array(node, "clk-phase", clk_phase, 2);
+	if (!rc) {
+		socfpga_clk->clk_phase[0] = clk_phase[0];
+		socfpga_clk->clk_phase[1] = clk_phase[1];
+	}
+
 	of_property_read_string(node, "clock-output-names", &clk_name);
 
 	init.name = clk_name;
-- 
1.7.9.5



^ permalink raw reply related	[flat|nested] 28+ messages in thread

* [RESEND LIST PATCHv7 1/4] clk: socfpga: Add a clk-phase property to the "altr, socfpga-gate-clk"
@ 2013-12-16 17:04   ` dinguyen at altera.com
  0 siblings, 0 replies; 28+ messages in thread
From: dinguyen at altera.com @ 2013-12-16 17:04 UTC (permalink / raw)
  To: linux-arm-kernel

From: Dinh Nguyen <dinguyen@altera.com>

The clk-phase property is used to represent the 2 clock phase values that is
needed for the SD/MMC driver. Add a prepare function to the clk_ops, that will
use the syscon driver to set sdmmc_clk's phase shift that is located in the
system manager.

Signed-off-by: Dinh Nguyen <dinguyen@altera.com>
---
v7: Add dts property to represent the clk phase of the sdmmc_clk. Add a
    prepare function to the gate clk that will toggle clock phase setting.
    Remove the "altr,socfpga-sdmmc-sdr-clk" clock type.
v6: Add a new clock type "altr,socfpga-sdmmc-sdr-clk" that will be used to
set the phase shift settings.
v5: Use the "snps,dw-mshc" binding
v4: Use the sdmmc_clk prepare function to set the phase shift settings
v3: Not use the syscon driver because as of 3.13-rc1, the syscon driver is
    loaded after the clock driver.
v2: Use the syscon driver
---
 .../devicetree/bindings/clock/altr_socfpga.txt     |   14 ++++++++
 arch/arm/boot/dts/socfpga.dtsi                     |    1 +
 drivers/clk/socfpga/clk.c                          |   36 ++++++++++++++++++++
 3 files changed, 51 insertions(+)

diff --git a/Documentation/devicetree/bindings/clock/altr_socfpga.txt b/Documentation/devicetree/bindings/clock/altr_socfpga.txt
index 0045433..4b66754 100644
--- a/Documentation/devicetree/bindings/clock/altr_socfpga.txt
+++ b/Documentation/devicetree/bindings/clock/altr_socfpga.txt
@@ -23,3 +23,17 @@ Optional properties:
         and the bit index.
 - div-reg : For "socfpga-gate-clk", div-reg contains the divider register, bit shift,
         and width.
+- clk-phase : For the sdmmc_clk, contains the value of the clock phase that controls
+  the SDMMC CIU clock. The first value is the clk_sample(smpsel), and the second
+  value is the cclk_in_drv(drvsel). The clk-phase is used to enable the correct
+  hold/delay times that is needed for the SD/MMC CIU clock. The values of both
+  value can be one of the following:
+
+	<0x0>	:	0 degrees
+	<0x1>	:	45 degrees
+	<0x2>	:	90 degrees
+	<0x3>	:	135 degrees
+	<0x4>	:	180 degrees
+	<0x5>	:	225 degrees
+	<0x6>	:	270 degrees
+	<0x7>	:	315 degrees
diff --git a/arch/arm/boot/dts/socfpga.dtsi b/arch/arm/boot/dts/socfpga.dtsi
index f936476..616d9ee 100644
--- a/arch/arm/boot/dts/socfpga.dtsi
+++ b/arch/arm/boot/dts/socfpga.dtsi
@@ -413,6 +413,7 @@
 						compatible = "altr,socfpga-gate-clk";
 						clocks = <&f2s_periph_ref_clk>, <&main_nand_sdmmc_clk>, <&per_nand_mmc_clk>;
 						clk-gate = <0xa0 8>;
+						clk-phase = <0 3>;
 					};
 
 					nand_x_clk: nand_x_clk {
diff --git a/drivers/clk/socfpga/clk.c b/drivers/clk/socfpga/clk.c
index 280c983..7cfebd6 100644
--- a/drivers/clk/socfpga/clk.c
+++ b/drivers/clk/socfpga/clk.c
@@ -21,8 +21,10 @@
 #include <linux/clkdev.h>
 #include <linux/clk-provider.h>
 #include <linux/io.h>
+#include <linux/mfd/syscon.h>
 #include <linux/of.h>
 #include <linux/of_address.h>
+#include <linux/regmap.h>
 
 /* Clock Manager offsets */
 #define CLKMGR_CTRL	0x0
@@ -56,6 +58,11 @@
 #define div_mask(width)	((1 << (width)) - 1)
 #define streq(a, b) (strcmp((a), (b)) == 0)
 
+/* SDMMC Group for System Manager defines */
+#define SYSMGR_SDMMCGRP_CTRL_OFFSET	0x108
+#define SYSMGR_SDMMC_CTRL_SET(smplsel, drvsel)	\
+	((((smplsel) & 0x7) << 3) | (((drvsel) & 0x7) << 0))
+
 static void __iomem *clk_mgr_base_addr;
 
 struct socfpga_clk {
@@ -66,6 +73,7 @@ struct socfpga_clk {
 	void __iomem *div_reg;
 	u32 width;	/* only valid if div_reg != 0 */
 	u32 shift;	/* only valid if div_reg != 0 */
+	u32 clk_phase[2];
 };
 #define to_socfpga_clk(p) container_of(p, struct socfpga_clk, hw.hw)
 
@@ -243,7 +251,28 @@ static unsigned long socfpga_clk_recalc_rate(struct clk_hw *hwclk,
 	return parent_rate / div;
 }
 
+static int socfpga_clk_prepare(struct clk_hw *hwclk)
+{
+	struct socfpga_clk *socfpgaclk = to_socfpga_clk(hwclk);
+	struct regmap *sys_mgr_base_addr;
+	u32 hs_timing;
+
+	if (socfpgaclk->clk_phase[0] || socfpgaclk->clk_phase[1]) {
+		sys_mgr_base_addr = syscon_regmap_lookup_by_compatible("altr,sys-mgr");
+		if (IS_ERR(sys_mgr_base_addr)) {
+			pr_err("%s: failed to find altr,sys-mgr regmap!\n", __func__);
+			return -EINVAL;
+		}
+		hs_timing = SYSMGR_SDMMC_CTRL_SET(socfpgaclk->clk_phase[0],
+						socfpgaclk->clk_phase[1]);
+		regmap_write(sys_mgr_base_addr, SYSMGR_SDMMCGRP_CTRL_OFFSET,
+						hs_timing);
+	}
+	return 0;
+}
+
 static struct clk_ops gateclk_ops = {
+	.prepare = socfpga_clk_prepare,
 	.recalc_rate = socfpga_clk_recalc_rate,
 	.get_parent = socfpga_clk_get_parent,
 	.set_parent = socfpga_clk_set_parent,
@@ -254,6 +283,7 @@ static void __init socfpga_gate_clk_init(struct device_node *node,
 {
 	u32 clk_gate[2];
 	u32 div_reg[3];
+	u32 clk_phase[2];
 	u32 fixed_div;
 	struct clk *clk;
 	struct socfpga_clk *socfpga_clk;
@@ -294,6 +324,12 @@ static void __init socfpga_gate_clk_init(struct device_node *node,
 		socfpga_clk->div_reg = 0;
 	}
 
+	rc = of_property_read_u32_array(node, "clk-phase", clk_phase, 2);
+	if (!rc) {
+		socfpga_clk->clk_phase[0] = clk_phase[0];
+		socfpga_clk->clk_phase[1] = clk_phase[1];
+	}
+
 	of_property_read_string(node, "clock-output-names", &clk_name);
 
 	init.name = clk_name;
-- 
1.7.9.5

^ permalink raw reply related	[flat|nested] 28+ messages in thread

* [RESEND LIST PATCHv7 2/4] dts: socfpga: Add support for SD/MMC on the SOCFPGA platform
  2013-12-16 17:04 ` dinguyen at altera.com
@ 2013-12-16 17:04   ` dinguyen at altera.com
  -1 siblings, 0 replies; 28+ messages in thread
From: dinguyen @ 2013-12-16 17:04 UTC (permalink / raw)
  To: dinh.linux, arnd, cjb, jh80.chung, tgih.jun, heiko, dianders,
	alim.akhtar, bzhao, mturquette
  Cc: zhangfei.gao, linux-mmc, linux-arm-kernel, devicetree, Dinh Nguyen

From: Dinh Nguyen <dinguyen@altera.com>

Use the "snps,dw-mshc" binding to enable the dw_mmc driver.
Add the "syscon" binding to the "altr,sys-mgr" node. The clock
driver can use the syscon driver to toggle the register for the SD/MMC
clock phase shift settings.

Signed-off-by: Dinh Nguyen <dinguyen@altera.com>
---
v7: Use the standard "snps,dw-mshc" binding. Remove "altr,socfpga-sdmmc-sdr-clk".
v6: Add documentation for "altr,socfpga-sdmmc-sdr-clk". Add "syscon" to the
sysmgr binding.
v5: Use the "snps,dw-mshc" binding
v4: Re-use "rockchip,rk2928-dw-mshc" binding
v3: none
v2: none
---
 arch/arm/boot/dts/socfpga.dtsi          |   13 ++++++++++++-
 arch/arm/boot/dts/socfpga_arria5.dtsi   |   11 +++++++++++
 arch/arm/boot/dts/socfpga_cyclone5.dtsi |   11 +++++++++++
 arch/arm/boot/dts/socfpga_vt.dts        |   11 +++++++++++
 4 files changed, 45 insertions(+), 1 deletion(-)

diff --git a/arch/arm/boot/dts/socfpga.dtsi b/arch/arm/boot/dts/socfpga.dtsi
index 616d9ee..3b1c0fc 100644
--- a/arch/arm/boot/dts/socfpga.dtsi
+++ b/arch/arm/boot/dts/socfpga.dtsi
@@ -470,6 +470,17 @@
 			cache-level = <2>;
 		};
 
+		mmc: dwmmc0@ff704000 {
+			compatible = "snps,dw-mshc";
+			reg = <0xff704000 0x1000>;
+			interrupts = <0 139 4>;
+			fifo-depth = <0x400>;
+			#address-cells = <1>;
+			#size-cells = <0>;
+			clocks = <&l4_mp_clk>, <&sdmmc_clk>;
+			clock-names = "biu", "ciu";
+		};
+
 		/* Local timer */
 		timer@fffec600 {
 			compatible = "arm,cortex-a9-twd-timer";
@@ -524,7 +535,7 @@
 		};
 
 		sysmgr@ffd08000 {
-				compatible = "altr,sys-mgr";
+				compatible = "altr,sys-mgr", "syscon";
 				reg = <0xffd08000 0x4000>;
 			};
 	};
diff --git a/arch/arm/boot/dts/socfpga_arria5.dtsi b/arch/arm/boot/dts/socfpga_arria5.dtsi
index a85b404..6c87b70 100644
--- a/arch/arm/boot/dts/socfpga_arria5.dtsi
+++ b/arch/arm/boot/dts/socfpga_arria5.dtsi
@@ -27,6 +27,17 @@
 			};
 		};
 
+		dwmmc0@ff704000 {
+			num-slots = <1>;
+			supports-highspeed;
+			broken-cd;
+
+			slot@0 {
+				reg = <0>;
+				bus-width = <4>;
+			};
+		};
+
 		serial0@ffc02000 {
 			clock-frequency = <100000000>;
 		};
diff --git a/arch/arm/boot/dts/socfpga_cyclone5.dtsi b/arch/arm/boot/dts/socfpga_cyclone5.dtsi
index a8716f6..ca41b0e 100644
--- a/arch/arm/boot/dts/socfpga_cyclone5.dtsi
+++ b/arch/arm/boot/dts/socfpga_cyclone5.dtsi
@@ -28,6 +28,17 @@
 			};
 		};
 
+		dwmmc0@ff704000 {
+			num-slots = <1>;
+			supports-highspeed;
+			broken-cd;
+
+			slot@0 {
+				reg = <0>;
+				bus-width = <4>;
+			};
+		};
+
 		ethernet@ff702000 {
 			phy-mode = "rgmii";
 			phy-addr = <0xffffffff>; /* probe for phy addr */
diff --git a/arch/arm/boot/dts/socfpga_vt.dts b/arch/arm/boot/dts/socfpga_vt.dts
index d1ec0ca..222313f 100644
--- a/arch/arm/boot/dts/socfpga_vt.dts
+++ b/arch/arm/boot/dts/socfpga_vt.dts
@@ -41,6 +41,17 @@
 			};
 		};
 
+		dwmmc0@ff704000 {
+			num-slots = <1>;
+			supports-highspeed;
+			broken-cd;
+
+			slot@0 {
+				reg = <0>;
+				bus-width = <4>;
+			};
+		};
+
 		ethernet@ff700000 {
 			phy-mode = "gmii";
 			status = "okay";
-- 
1.7.9.5



^ permalink raw reply related	[flat|nested] 28+ messages in thread

* [RESEND LIST PATCHv7 2/4] dts: socfpga: Add support for SD/MMC on the SOCFPGA platform
@ 2013-12-16 17:04   ` dinguyen at altera.com
  0 siblings, 0 replies; 28+ messages in thread
From: dinguyen at altera.com @ 2013-12-16 17:04 UTC (permalink / raw)
  To: linux-arm-kernel

From: Dinh Nguyen <dinguyen@altera.com>

Use the "snps,dw-mshc" binding to enable the dw_mmc driver.
Add the "syscon" binding to the "altr,sys-mgr" node. The clock
driver can use the syscon driver to toggle the register for the SD/MMC
clock phase shift settings.

Signed-off-by: Dinh Nguyen <dinguyen@altera.com>
---
v7: Use the standard "snps,dw-mshc" binding. Remove "altr,socfpga-sdmmc-sdr-clk".
v6: Add documentation for "altr,socfpga-sdmmc-sdr-clk". Add "syscon" to the
sysmgr binding.
v5: Use the "snps,dw-mshc" binding
v4: Re-use "rockchip,rk2928-dw-mshc" binding
v3: none
v2: none
---
 arch/arm/boot/dts/socfpga.dtsi          |   13 ++++++++++++-
 arch/arm/boot/dts/socfpga_arria5.dtsi   |   11 +++++++++++
 arch/arm/boot/dts/socfpga_cyclone5.dtsi |   11 +++++++++++
 arch/arm/boot/dts/socfpga_vt.dts        |   11 +++++++++++
 4 files changed, 45 insertions(+), 1 deletion(-)

diff --git a/arch/arm/boot/dts/socfpga.dtsi b/arch/arm/boot/dts/socfpga.dtsi
index 616d9ee..3b1c0fc 100644
--- a/arch/arm/boot/dts/socfpga.dtsi
+++ b/arch/arm/boot/dts/socfpga.dtsi
@@ -470,6 +470,17 @@
 			cache-level = <2>;
 		};
 
+		mmc: dwmmc0 at ff704000 {
+			compatible = "snps,dw-mshc";
+			reg = <0xff704000 0x1000>;
+			interrupts = <0 139 4>;
+			fifo-depth = <0x400>;
+			#address-cells = <1>;
+			#size-cells = <0>;
+			clocks = <&l4_mp_clk>, <&sdmmc_clk>;
+			clock-names = "biu", "ciu";
+		};
+
 		/* Local timer */
 		timer at fffec600 {
 			compatible = "arm,cortex-a9-twd-timer";
@@ -524,7 +535,7 @@
 		};
 
 		sysmgr at ffd08000 {
-				compatible = "altr,sys-mgr";
+				compatible = "altr,sys-mgr", "syscon";
 				reg = <0xffd08000 0x4000>;
 			};
 	};
diff --git a/arch/arm/boot/dts/socfpga_arria5.dtsi b/arch/arm/boot/dts/socfpga_arria5.dtsi
index a85b404..6c87b70 100644
--- a/arch/arm/boot/dts/socfpga_arria5.dtsi
+++ b/arch/arm/boot/dts/socfpga_arria5.dtsi
@@ -27,6 +27,17 @@
 			};
 		};
 
+		dwmmc0 at ff704000 {
+			num-slots = <1>;
+			supports-highspeed;
+			broken-cd;
+
+			slot at 0 {
+				reg = <0>;
+				bus-width = <4>;
+			};
+		};
+
 		serial0 at ffc02000 {
 			clock-frequency = <100000000>;
 		};
diff --git a/arch/arm/boot/dts/socfpga_cyclone5.dtsi b/arch/arm/boot/dts/socfpga_cyclone5.dtsi
index a8716f6..ca41b0e 100644
--- a/arch/arm/boot/dts/socfpga_cyclone5.dtsi
+++ b/arch/arm/boot/dts/socfpga_cyclone5.dtsi
@@ -28,6 +28,17 @@
 			};
 		};
 
+		dwmmc0 at ff704000 {
+			num-slots = <1>;
+			supports-highspeed;
+			broken-cd;
+
+			slot at 0 {
+				reg = <0>;
+				bus-width = <4>;
+			};
+		};
+
 		ethernet at ff702000 {
 			phy-mode = "rgmii";
 			phy-addr = <0xffffffff>; /* probe for phy addr */
diff --git a/arch/arm/boot/dts/socfpga_vt.dts b/arch/arm/boot/dts/socfpga_vt.dts
index d1ec0ca..222313f 100644
--- a/arch/arm/boot/dts/socfpga_vt.dts
+++ b/arch/arm/boot/dts/socfpga_vt.dts
@@ -41,6 +41,17 @@
 			};
 		};
 
+		dwmmc0 at ff704000 {
+			num-slots = <1>;
+			supports-highspeed;
+			broken-cd;
+
+			slot at 0 {
+				reg = <0>;
+				bus-width = <4>;
+			};
+		};
+
 		ethernet at ff700000 {
 			phy-mode = "gmii";
 			status = "okay";
-- 
1.7.9.5

^ permalink raw reply related	[flat|nested] 28+ messages in thread

* [RESEND LIST PATCHv7 3/4] mmc: dw_mmc-socfpga: Remove the SOCFPGA specific platform for dw_mmc
  2013-12-16 17:04 ` dinguyen at altera.com
@ 2013-12-16 17:04   ` dinguyen at altera.com
  -1 siblings, 0 replies; 28+ messages in thread
From: dinguyen @ 2013-12-16 17:04 UTC (permalink / raw)
  To: dinh.linux, arnd, cjb, jh80.chung, tgih.jun, heiko, dianders,
	alim.akhtar, bzhao, mturquette
  Cc: zhangfei.gao, linux-mmc, linux-arm-kernel, devicetree, Dinh Nguyen

From: Dinh Nguyen <dinguyen@altera.com>

It turns now that the only really platform specific code that is needed for
SOCFPGA is using the SDMMC_CMD_USE_HOLD_REG in the prepare_command function.
Since the Rockchip already has this functionality, re-use the code that is
already in dw_mmc-pltfm.c.

Signed-off-by: Dinh Nguyen <dinguyen@altera.com>
---
v7: none
v6: none
v5: none
v4: Remove dw_mmc-socfpga platform specific code
v3: none
v2: none
---
 drivers/mmc/host/Kconfig          |    8 ---
 drivers/mmc/host/dw_mmc-socfpga.c |  138 -------------------------------------
 2 files changed, 146 deletions(-)
 delete mode 100644 drivers/mmc/host/dw_mmc-socfpga.c

diff --git a/drivers/mmc/host/Kconfig b/drivers/mmc/host/Kconfig
index 7fc5099..6737a4f 100644
--- a/drivers/mmc/host/Kconfig
+++ b/drivers/mmc/host/Kconfig
@@ -567,14 +567,6 @@ config MMC_DW_EXYNOS
 	  Synopsys DesignWare Memory Card Interface driver. Select this option
 	  for platforms based on Exynos4 and Exynos5 SoC's.
 
-config MMC_DW_SOCFPGA
-	tristate "SOCFPGA specific extensions for Synopsys DW Memory Card Interface"
-	depends on MMC_DW && MFD_SYSCON
-	select MMC_DW_PLTFM
-	help
-	  This selects support for Altera SoCFPGA specific extensions to the
-	  Synopsys DesignWare Memory Card Interface driver.
-
 config MMC_DW_PCI
 	tristate "Synopsys Designware MCI support on PCI bus"
 	depends on MMC_DW && PCI
diff --git a/drivers/mmc/host/dw_mmc-socfpga.c b/drivers/mmc/host/dw_mmc-socfpga.c
deleted file mode 100644
index 3e8e53a..0000000
--- a/drivers/mmc/host/dw_mmc-socfpga.c
+++ /dev/null
@@ -1,138 +0,0 @@
-/*
- * Altera SoCFPGA Specific Extensions for Synopsys DW Multimedia Card Interface
- * driver
- *
- *  Copyright (C) 2012, Samsung Electronics Co., Ltd.
- *  Copyright (C) 2013 Altera Corporation
- *
- * This program is free software; you can redistribute it and/or modify
- * it under the terms of the GNU General Public License as published by
- * the Free Software Foundation; either version 2 of the License, or
- * (at your option) any later version.
- *
- * Taken from dw_mmc-exynos.c
- */
-#include <linux/clk.h>
-#include <linux/mfd/syscon.h>
-#include <linux/mmc/host.h>
-#include <linux/mmc/dw_mmc.h>
-#include <linux/module.h>
-#include <linux/of.h>
-#include <linux/platform_device.h>
-#include <linux/regmap.h>
-
-#include "dw_mmc.h"
-#include "dw_mmc-pltfm.h"
-
-#define SYSMGR_SDMMCGRP_CTRL_OFFSET		0x108
-#define DRV_CLK_PHASE_SHIFT_SEL_MASK	0x7
-#define SYSMGR_SDMMC_CTRL_SET(smplsel, drvsel)          \
-	((((smplsel) & 0x7) << 3) | (((drvsel) & 0x7) << 0))
-
-/* SOCFPGA implementation specific driver private data */
-struct dw_mci_socfpga_priv_data {
-	u8	ciu_div; /* card interface unit divisor */
-	u32	hs_timing; /* bitmask for CIU clock phase shift */
-	struct regmap   *sysreg; /* regmap for system manager register */
-};
-
-static int dw_mci_socfpga_priv_init(struct dw_mci *host)
-{
-	return 0;
-}
-
-static int dw_mci_socfpga_setup_clock(struct dw_mci *host)
-{
-	struct dw_mci_socfpga_priv_data *priv = host->priv;
-
-	clk_disable_unprepare(host->ciu_clk);
-	regmap_write(priv->sysreg, SYSMGR_SDMMCGRP_CTRL_OFFSET,
-		priv->hs_timing);
-	clk_prepare_enable(host->ciu_clk);
-
-	host->bus_hz /= (priv->ciu_div + 1);
-	return 0;
-}
-
-static void dw_mci_socfpga_prepare_command(struct dw_mci *host, u32 *cmdr)
-{
-	struct dw_mci_socfpga_priv_data *priv = host->priv;
-
-	if (priv->hs_timing & DRV_CLK_PHASE_SHIFT_SEL_MASK)
-		*cmdr |= SDMMC_CMD_USE_HOLD_REG;
-}
-
-static int dw_mci_socfpga_parse_dt(struct dw_mci *host)
-{
-	struct dw_mci_socfpga_priv_data *priv;
-	struct device_node *np = host->dev->of_node;
-	u32 timing[2];
-	u32 div = 0;
-	int ret;
-
-	priv = devm_kzalloc(host->dev, sizeof(*priv), GFP_KERNEL);
-	if (!priv) {
-		dev_err(host->dev, "mem alloc failed for private data\n");
-		return -ENOMEM;
-	}
-
-	priv->sysreg = syscon_regmap_lookup_by_compatible("altr,sys-mgr");
-	if (IS_ERR(priv->sysreg)) {
-		dev_err(host->dev, "regmap for altr,sys-mgr lookup failed.\n");
-		return PTR_ERR(priv->sysreg);
-	}
-
-	ret = of_property_read_u32(np, "altr,dw-mshc-ciu-div", &div);
-	if (ret)
-		dev_info(host->dev, "No dw-mshc-ciu-div specified, assuming 1");
-	priv->ciu_div = div;
-
-	ret = of_property_read_u32_array(np,
-			"altr,dw-mshc-sdr-timing", timing, 2);
-	if (ret)
-		return ret;
-
-	priv->hs_timing = SYSMGR_SDMMC_CTRL_SET(timing[0], timing[1]);
-	host->priv = priv;
-	return 0;
-}
-
-static const struct dw_mci_drv_data socfpga_drv_data = {
-	.init			= dw_mci_socfpga_priv_init,
-	.setup_clock		= dw_mci_socfpga_setup_clock,
-	.prepare_command	= dw_mci_socfpga_prepare_command,
-	.parse_dt		= dw_mci_socfpga_parse_dt,
-};
-
-static const struct of_device_id dw_mci_socfpga_match[] = {
-	{ .compatible = "altr,socfpga-dw-mshc",
-			.data = &socfpga_drv_data, },
-	{},
-};
-MODULE_DEVICE_TABLE(of, dw_mci_socfpga_match);
-
-static int dw_mci_socfpga_probe(struct platform_device *pdev)
-{
-	const struct dw_mci_drv_data *drv_data;
-	const struct of_device_id *match;
-
-	match = of_match_node(dw_mci_socfpga_match, pdev->dev.of_node);
-	drv_data = match->data;
-	return dw_mci_pltfm_register(pdev, drv_data);
-}
-
-static struct platform_driver dw_mci_socfpga_pltfm_driver = {
-	.probe		= dw_mci_socfpga_probe,
-	.remove		= __exit_p(dw_mci_pltfm_remove),
-	.driver		= {
-		.name		= "dwmmc_socfpga",
-		.of_match_table	= dw_mci_socfpga_match,
-		.pm		= &dw_mci_pltfm_pmops,
-	},
-};
-
-module_platform_driver(dw_mci_socfpga_pltfm_driver);
-
-MODULE_DESCRIPTION("Altera SOCFPGA Specific DW-MSHC Driver Extension");
-MODULE_LICENSE("GPL v2");
-MODULE_ALIAS("platform:dwmmc-socfpga");
-- 
1.7.9.5



^ permalink raw reply related	[flat|nested] 28+ messages in thread

* [RESEND LIST PATCHv7 3/4] mmc: dw_mmc-socfpga: Remove the SOCFPGA specific platform for dw_mmc
@ 2013-12-16 17:04   ` dinguyen at altera.com
  0 siblings, 0 replies; 28+ messages in thread
From: dinguyen at altera.com @ 2013-12-16 17:04 UTC (permalink / raw)
  To: linux-arm-kernel

From: Dinh Nguyen <dinguyen@altera.com>

It turns now that the only really platform specific code that is needed for
SOCFPGA is using the SDMMC_CMD_USE_HOLD_REG in the prepare_command function.
Since the Rockchip already has this functionality, re-use the code that is
already in dw_mmc-pltfm.c.

Signed-off-by: Dinh Nguyen <dinguyen@altera.com>
---
v7: none
v6: none
v5: none
v4: Remove dw_mmc-socfpga platform specific code
v3: none
v2: none
---
 drivers/mmc/host/Kconfig          |    8 ---
 drivers/mmc/host/dw_mmc-socfpga.c |  138 -------------------------------------
 2 files changed, 146 deletions(-)
 delete mode 100644 drivers/mmc/host/dw_mmc-socfpga.c

diff --git a/drivers/mmc/host/Kconfig b/drivers/mmc/host/Kconfig
index 7fc5099..6737a4f 100644
--- a/drivers/mmc/host/Kconfig
+++ b/drivers/mmc/host/Kconfig
@@ -567,14 +567,6 @@ config MMC_DW_EXYNOS
 	  Synopsys DesignWare Memory Card Interface driver. Select this option
 	  for platforms based on Exynos4 and Exynos5 SoC's.
 
-config MMC_DW_SOCFPGA
-	tristate "SOCFPGA specific extensions for Synopsys DW Memory Card Interface"
-	depends on MMC_DW && MFD_SYSCON
-	select MMC_DW_PLTFM
-	help
-	  This selects support for Altera SoCFPGA specific extensions to the
-	  Synopsys DesignWare Memory Card Interface driver.
-
 config MMC_DW_PCI
 	tristate "Synopsys Designware MCI support on PCI bus"
 	depends on MMC_DW && PCI
diff --git a/drivers/mmc/host/dw_mmc-socfpga.c b/drivers/mmc/host/dw_mmc-socfpga.c
deleted file mode 100644
index 3e8e53a..0000000
--- a/drivers/mmc/host/dw_mmc-socfpga.c
+++ /dev/null
@@ -1,138 +0,0 @@
-/*
- * Altera SoCFPGA Specific Extensions for Synopsys DW Multimedia Card Interface
- * driver
- *
- *  Copyright (C) 2012, Samsung Electronics Co., Ltd.
- *  Copyright (C) 2013 Altera Corporation
- *
- * This program is free software; you can redistribute it and/or modify
- * it under the terms of the GNU General Public License as published by
- * the Free Software Foundation; either version 2 of the License, or
- * (at your option) any later version.
- *
- * Taken from dw_mmc-exynos.c
- */
-#include <linux/clk.h>
-#include <linux/mfd/syscon.h>
-#include <linux/mmc/host.h>
-#include <linux/mmc/dw_mmc.h>
-#include <linux/module.h>
-#include <linux/of.h>
-#include <linux/platform_device.h>
-#include <linux/regmap.h>
-
-#include "dw_mmc.h"
-#include "dw_mmc-pltfm.h"
-
-#define SYSMGR_SDMMCGRP_CTRL_OFFSET		0x108
-#define DRV_CLK_PHASE_SHIFT_SEL_MASK	0x7
-#define SYSMGR_SDMMC_CTRL_SET(smplsel, drvsel)          \
-	((((smplsel) & 0x7) << 3) | (((drvsel) & 0x7) << 0))
-
-/* SOCFPGA implementation specific driver private data */
-struct dw_mci_socfpga_priv_data {
-	u8	ciu_div; /* card interface unit divisor */
-	u32	hs_timing; /* bitmask for CIU clock phase shift */
-	struct regmap   *sysreg; /* regmap for system manager register */
-};
-
-static int dw_mci_socfpga_priv_init(struct dw_mci *host)
-{
-	return 0;
-}
-
-static int dw_mci_socfpga_setup_clock(struct dw_mci *host)
-{
-	struct dw_mci_socfpga_priv_data *priv = host->priv;
-
-	clk_disable_unprepare(host->ciu_clk);
-	regmap_write(priv->sysreg, SYSMGR_SDMMCGRP_CTRL_OFFSET,
-		priv->hs_timing);
-	clk_prepare_enable(host->ciu_clk);
-
-	host->bus_hz /= (priv->ciu_div + 1);
-	return 0;
-}
-
-static void dw_mci_socfpga_prepare_command(struct dw_mci *host, u32 *cmdr)
-{
-	struct dw_mci_socfpga_priv_data *priv = host->priv;
-
-	if (priv->hs_timing & DRV_CLK_PHASE_SHIFT_SEL_MASK)
-		*cmdr |= SDMMC_CMD_USE_HOLD_REG;
-}
-
-static int dw_mci_socfpga_parse_dt(struct dw_mci *host)
-{
-	struct dw_mci_socfpga_priv_data *priv;
-	struct device_node *np = host->dev->of_node;
-	u32 timing[2];
-	u32 div = 0;
-	int ret;
-
-	priv = devm_kzalloc(host->dev, sizeof(*priv), GFP_KERNEL);
-	if (!priv) {
-		dev_err(host->dev, "mem alloc failed for private data\n");
-		return -ENOMEM;
-	}
-
-	priv->sysreg = syscon_regmap_lookup_by_compatible("altr,sys-mgr");
-	if (IS_ERR(priv->sysreg)) {
-		dev_err(host->dev, "regmap for altr,sys-mgr lookup failed.\n");
-		return PTR_ERR(priv->sysreg);
-	}
-
-	ret = of_property_read_u32(np, "altr,dw-mshc-ciu-div", &div);
-	if (ret)
-		dev_info(host->dev, "No dw-mshc-ciu-div specified, assuming 1");
-	priv->ciu_div = div;
-
-	ret = of_property_read_u32_array(np,
-			"altr,dw-mshc-sdr-timing", timing, 2);
-	if (ret)
-		return ret;
-
-	priv->hs_timing = SYSMGR_SDMMC_CTRL_SET(timing[0], timing[1]);
-	host->priv = priv;
-	return 0;
-}
-
-static const struct dw_mci_drv_data socfpga_drv_data = {
-	.init			= dw_mci_socfpga_priv_init,
-	.setup_clock		= dw_mci_socfpga_setup_clock,
-	.prepare_command	= dw_mci_socfpga_prepare_command,
-	.parse_dt		= dw_mci_socfpga_parse_dt,
-};
-
-static const struct of_device_id dw_mci_socfpga_match[] = {
-	{ .compatible = "altr,socfpga-dw-mshc",
-			.data = &socfpga_drv_data, },
-	{},
-};
-MODULE_DEVICE_TABLE(of, dw_mci_socfpga_match);
-
-static int dw_mci_socfpga_probe(struct platform_device *pdev)
-{
-	const struct dw_mci_drv_data *drv_data;
-	const struct of_device_id *match;
-
-	match = of_match_node(dw_mci_socfpga_match, pdev->dev.of_node);
-	drv_data = match->data;
-	return dw_mci_pltfm_register(pdev, drv_data);
-}
-
-static struct platform_driver dw_mci_socfpga_pltfm_driver = {
-	.probe		= dw_mci_socfpga_probe,
-	.remove		= __exit_p(dw_mci_pltfm_remove),
-	.driver		= {
-		.name		= "dwmmc_socfpga",
-		.of_match_table	= dw_mci_socfpga_match,
-		.pm		= &dw_mci_pltfm_pmops,
-	},
-};
-
-module_platform_driver(dw_mci_socfpga_pltfm_driver);
-
-MODULE_DESCRIPTION("Altera SOCFPGA Specific DW-MSHC Driver Extension");
-MODULE_LICENSE("GPL v2");
-MODULE_ALIAS("platform:dwmmc-socfpga");
-- 
1.7.9.5

^ permalink raw reply related	[flat|nested] 28+ messages in thread

* [RESEND LIST PATCHv7 4/4] ARM: socfpga_defconfig: enable SD/MMC support
  2013-12-16 17:04 ` dinguyen at altera.com
@ 2013-12-16 17:04   ` dinguyen at altera.com
  -1 siblings, 0 replies; 28+ messages in thread
From: dinguyen @ 2013-12-16 17:04 UTC (permalink / raw)
  To: dinh.linux, arnd, cjb, jh80.chung, tgih.jun, heiko, dianders,
	alim.akhtar, bzhao, mturquette
  Cc: zhangfei.gao, linux-mmc, linux-arm-kernel, devicetree, Dinh Nguyen

From: Dinh Nguyen <dinguyen@altera.com>

Enables the dw_mmc driver for SOCFPGA.

Signed-off-by: Dinh Nguyen <dinguyen@altera.com>
---
v7: none
v6: none
v5: Add dw_mmc driver into socfpga_defconfig
v4: none
v3: none
v2: none
---
 arch/arm/configs/socfpga_defconfig |    2 ++
 1 file changed, 2 insertions(+)

diff --git a/arch/arm/configs/socfpga_defconfig b/arch/arm/configs/socfpga_defconfig
index 4e1ce21..8fff96ba 100644
--- a/arch/arm/configs/socfpga_defconfig
+++ b/arch/arm/configs/socfpga_defconfig
@@ -82,3 +82,5 @@ CONFIG_DEBUG_INFO=y
 CONFIG_ENABLE_DEFAULT_TRACERS=y
 CONFIG_DEBUG_USER=y
 CONFIG_XZ_DEC=y
+CONFIG_MMC=y
+CONFIG_MMC_DW=y
-- 
1.7.9.5



^ permalink raw reply related	[flat|nested] 28+ messages in thread

* [RESEND LIST PATCHv7 4/4] ARM: socfpga_defconfig: enable SD/MMC support
@ 2013-12-16 17:04   ` dinguyen at altera.com
  0 siblings, 0 replies; 28+ messages in thread
From: dinguyen at altera.com @ 2013-12-16 17:04 UTC (permalink / raw)
  To: linux-arm-kernel

From: Dinh Nguyen <dinguyen@altera.com>

Enables the dw_mmc driver for SOCFPGA.

Signed-off-by: Dinh Nguyen <dinguyen@altera.com>
---
v7: none
v6: none
v5: Add dw_mmc driver into socfpga_defconfig
v4: none
v3: none
v2: none
---
 arch/arm/configs/socfpga_defconfig |    2 ++
 1 file changed, 2 insertions(+)

diff --git a/arch/arm/configs/socfpga_defconfig b/arch/arm/configs/socfpga_defconfig
index 4e1ce21..8fff96ba 100644
--- a/arch/arm/configs/socfpga_defconfig
+++ b/arch/arm/configs/socfpga_defconfig
@@ -82,3 +82,5 @@ CONFIG_DEBUG_INFO=y
 CONFIG_ENABLE_DEFAULT_TRACERS=y
 CONFIG_DEBUG_USER=y
 CONFIG_XZ_DEC=y
+CONFIG_MMC=y
+CONFIG_MMC_DW=y
-- 
1.7.9.5

^ permalink raw reply related	[flat|nested] 28+ messages in thread

* Re: [RESEND LIST PATCHv7 1/4] clk: socfpga: Add a clk-phase property to the "altr,socfpga-gate-clk"
  2013-12-16 17:04   ` [RESEND LIST PATCHv7 1/4] clk: socfpga: Add a clk-phase property to the "altr, socfpga-gate-clk" dinguyen at altera.com
@ 2013-12-17  7:46     ` zhangfei
  -1 siblings, 0 replies; 28+ messages in thread
From: zhangfei @ 2013-12-17  7:46 UTC (permalink / raw)
  To: dinguyen, dinh.linux, arnd, cjb, jh80.chung, tgih.jun, heiko,
	dianders, alim.akhtar, bzhao, mturquette
  Cc: linux-mmc, linux-arm-kernel, devicetree



On 12/17/2013 01:04 AM, dinguyen@altera.com wrote:
> From: Dinh Nguyen <dinguyen@altera.com>

> +static int socfpga_clk_prepare(struct clk_hw *hwclk)
> +{
> +	struct socfpga_clk *socfpgaclk = to_socfpga_clk(hwclk);
> +	struct regmap *sys_mgr_base_addr;
> +	u32 hs_timing;
> +
> +	if (socfpgaclk->clk_phase[0] || socfpgaclk->clk_phase[1]) {
> +		sys_mgr_base_addr = syscon_regmap_lookup_by_compatible("altr,sys-mgr");
> +		if (IS_ERR(sys_mgr_base_addr)) {
> +			pr_err("%s: failed to find altr,sys-mgr regmap!\n", __func__);
> +			return -EINVAL;
> +		}
> +		hs_timing = SYSMGR_SDMMC_CTRL_SET(socfpgaclk->clk_phase[0],
> +						socfpgaclk->clk_phase[1]);
> +		regmap_write(sys_mgr_base_addr, SYSMGR_SDMMCGRP_CTRL_OFFSET,
> +						hs_timing);
> +	}
> +	return 0;
> +}

So reusing gate-clk here and check the node of "altr,sys-mgr".
I think it is good and simple.
Also can define new clock combined with node "altr,sys-mgr" with parent 
of sdmmc_clk.

Thanks for the update, it is fine to me.

> +
>   static struct clk_ops gateclk_ops = {
> +	.prepare = socfpga_clk_prepare,
>   	.recalc_rate = socfpga_clk_recalc_rate,
>   	.get_parent = socfpga_clk_get_parent,
>   	.set_parent = socfpga_clk_set_parent,
> @@ -254,6 +283,7 @@ static void __init socfpga_gate_clk_init(struct device_node *node,
>   {
>   	u32 clk_gate[2];
>   	u32 div_reg[3];
> +	u32 clk_phase[2];
>   	u32 fixed_div;
>   	struct clk *clk;
>   	struct socfpga_clk *socfpga_clk;
> @@ -294,6 +324,12 @@ static void __init socfpga_gate_clk_init(struct device_node *node,
>   		socfpga_clk->div_reg = 0;
>   	}
>
> +	rc = of_property_read_u32_array(node, "clk-phase", clk_phase, 2);
> +	if (!rc) {
> +		socfpga_clk->clk_phase[0] = clk_phase[0];
> +		socfpga_clk->clk_phase[1] = clk_phase[1];
> +	}
> +
>   	of_property_read_string(node, "clock-output-names", &clk_name);
>
>   	init.name = clk_name;
>

^ permalink raw reply	[flat|nested] 28+ messages in thread

* [RESEND LIST PATCHv7 1/4] clk: socfpga: Add a clk-phase property to the "altr,socfpga-gate-clk"
@ 2013-12-17  7:46     ` zhangfei
  0 siblings, 0 replies; 28+ messages in thread
From: zhangfei @ 2013-12-17  7:46 UTC (permalink / raw)
  To: linux-arm-kernel



On 12/17/2013 01:04 AM, dinguyen at altera.com wrote:
> From: Dinh Nguyen <dinguyen@altera.com>

> +static int socfpga_clk_prepare(struct clk_hw *hwclk)
> +{
> +	struct socfpga_clk *socfpgaclk = to_socfpga_clk(hwclk);
> +	struct regmap *sys_mgr_base_addr;
> +	u32 hs_timing;
> +
> +	if (socfpgaclk->clk_phase[0] || socfpgaclk->clk_phase[1]) {
> +		sys_mgr_base_addr = syscon_regmap_lookup_by_compatible("altr,sys-mgr");
> +		if (IS_ERR(sys_mgr_base_addr)) {
> +			pr_err("%s: failed to find altr,sys-mgr regmap!\n", __func__);
> +			return -EINVAL;
> +		}
> +		hs_timing = SYSMGR_SDMMC_CTRL_SET(socfpgaclk->clk_phase[0],
> +						socfpgaclk->clk_phase[1]);
> +		regmap_write(sys_mgr_base_addr, SYSMGR_SDMMCGRP_CTRL_OFFSET,
> +						hs_timing);
> +	}
> +	return 0;
> +}

So reusing gate-clk here and check the node of "altr,sys-mgr".
I think it is good and simple.
Also can define new clock combined with node "altr,sys-mgr" with parent 
of sdmmc_clk.

Thanks for the update, it is fine to me.

> +
>   static struct clk_ops gateclk_ops = {
> +	.prepare = socfpga_clk_prepare,
>   	.recalc_rate = socfpga_clk_recalc_rate,
>   	.get_parent = socfpga_clk_get_parent,
>   	.set_parent = socfpga_clk_set_parent,
> @@ -254,6 +283,7 @@ static void __init socfpga_gate_clk_init(struct device_node *node,
>   {
>   	u32 clk_gate[2];
>   	u32 div_reg[3];
> +	u32 clk_phase[2];
>   	u32 fixed_div;
>   	struct clk *clk;
>   	struct socfpga_clk *socfpga_clk;
> @@ -294,6 +324,12 @@ static void __init socfpga_gate_clk_init(struct device_node *node,
>   		socfpga_clk->div_reg = 0;
>   	}
>
> +	rc = of_property_read_u32_array(node, "clk-phase", clk_phase, 2);
> +	if (!rc) {
> +		socfpga_clk->clk_phase[0] = clk_phase[0];
> +		socfpga_clk->clk_phase[1] = clk_phase[1];
> +	}
> +
>   	of_property_read_string(node, "clock-output-names", &clk_name);
>
>   	init.name = clk_name;
>

^ permalink raw reply	[flat|nested] 28+ messages in thread

* Re: [RESEND LIST PATCHv7 1/4] clk: socfpga: Add a clk-phase property to the "altr,socfpga-gate-clk"
  2013-12-17  7:46     ` zhangfei
@ 2013-12-17 13:44       ` Dinh Nguyen
  -1 siblings, 0 replies; 28+ messages in thread
From: Dinh Nguyen @ 2013-12-17 13:44 UTC (permalink / raw)
  To: zhangfei, dinguyen, arnd, cjb, jh80.chung, tgih.jun, heiko,
	dianders, alim.akhtar, bzhao, mturquette
  Cc: linux-mmc, linux-arm-kernel, devicetree

Hi Zhangfei,

On 12/17/13 1:46 AM, zhangfei wrote:
>
>
> On 12/17/2013 01:04 AM, dinguyen@altera.com wrote:
>> From: Dinh Nguyen <dinguyen@altera.com>
>
>> +static int socfpga_clk_prepare(struct clk_hw *hwclk)
>> +{
>> +    struct socfpga_clk *socfpgaclk = to_socfpga_clk(hwclk);
>> +    struct regmap *sys_mgr_base_addr;
>> +    u32 hs_timing;
>> +
>> +    if (socfpgaclk->clk_phase[0] || socfpgaclk->clk_phase[1]) {
>> +        sys_mgr_base_addr =
>> syscon_regmap_lookup_by_compatible("altr,sys-mgr");
>> +        if (IS_ERR(sys_mgr_base_addr)) {
>> +            pr_err("%s: failed to find altr,sys-mgr regmap!\n",
>> __func__);
>> +            return -EINVAL;
>> +        }
>> +        hs_timing = SYSMGR_SDMMC_CTRL_SET(socfpgaclk->clk_phase[0],
>> +                        socfpgaclk->clk_phase[1]);
>> +        regmap_write(sys_mgr_base_addr, SYSMGR_SDMMCGRP_CTRL_OFFSET,
>> +                        hs_timing);
>> +    }
>> +    return 0;
>> +}
>
> So reusing gate-clk here and check the node of "altr,sys-mgr".
> I think it is good and simple.
> Also can define new clock combined with node "altr,sys-mgr" with
> parent of sdmmc_clk.
>
> Thanks for the update, it is fine to me.
Thanks, can I get an Ack from you for this version?

Thanks,
Dinh
>
>> +
>>   static struct clk_ops gateclk_ops = {
>> +    .prepare = socfpga_clk_prepare,
>>       .recalc_rate = socfpga_clk_recalc_rate,
>>       .get_parent = socfpga_clk_get_parent,
>>       .set_parent = socfpga_clk_set_parent,
>> @@ -254,6 +283,7 @@ static void __init socfpga_gate_clk_init(struct
>> device_node *node,
>>   {
>>       u32 clk_gate[2];
>>       u32 div_reg[3];
>> +    u32 clk_phase[2];
>>       u32 fixed_div;
>>       struct clk *clk;
>>       struct socfpga_clk *socfpga_clk;
>> @@ -294,6 +324,12 @@ static void __init socfpga_gate_clk_init(struct
>> device_node *node,
>>           socfpga_clk->div_reg = 0;
>>       }
>>
>> +    rc = of_property_read_u32_array(node, "clk-phase", clk_phase, 2);
>> +    if (!rc) {
>> +        socfpga_clk->clk_phase[0] = clk_phase[0];
>> +        socfpga_clk->clk_phase[1] = clk_phase[1];
>> +    }
>> +
>>       of_property_read_string(node, "clock-output-names", &clk_name);
>>
>>       init.name = clk_name;
>>


^ permalink raw reply	[flat|nested] 28+ messages in thread

* [RESEND LIST PATCHv7 1/4] clk: socfpga: Add a clk-phase property to the "altr,socfpga-gate-clk"
@ 2013-12-17 13:44       ` Dinh Nguyen
  0 siblings, 0 replies; 28+ messages in thread
From: Dinh Nguyen @ 2013-12-17 13:44 UTC (permalink / raw)
  To: linux-arm-kernel

Hi Zhangfei,

On 12/17/13 1:46 AM, zhangfei wrote:
>
>
> On 12/17/2013 01:04 AM, dinguyen at altera.com wrote:
>> From: Dinh Nguyen <dinguyen@altera.com>
>
>> +static int socfpga_clk_prepare(struct clk_hw *hwclk)
>> +{
>> +    struct socfpga_clk *socfpgaclk = to_socfpga_clk(hwclk);
>> +    struct regmap *sys_mgr_base_addr;
>> +    u32 hs_timing;
>> +
>> +    if (socfpgaclk->clk_phase[0] || socfpgaclk->clk_phase[1]) {
>> +        sys_mgr_base_addr =
>> syscon_regmap_lookup_by_compatible("altr,sys-mgr");
>> +        if (IS_ERR(sys_mgr_base_addr)) {
>> +            pr_err("%s: failed to find altr,sys-mgr regmap!\n",
>> __func__);
>> +            return -EINVAL;
>> +        }
>> +        hs_timing = SYSMGR_SDMMC_CTRL_SET(socfpgaclk->clk_phase[0],
>> +                        socfpgaclk->clk_phase[1]);
>> +        regmap_write(sys_mgr_base_addr, SYSMGR_SDMMCGRP_CTRL_OFFSET,
>> +                        hs_timing);
>> +    }
>> +    return 0;
>> +}
>
> So reusing gate-clk here and check the node of "altr,sys-mgr".
> I think it is good and simple.
> Also can define new clock combined with node "altr,sys-mgr" with
> parent of sdmmc_clk.
>
> Thanks for the update, it is fine to me.
Thanks, can I get an Ack from you for this version?

Thanks,
Dinh
>
>> +
>>   static struct clk_ops gateclk_ops = {
>> +    .prepare = socfpga_clk_prepare,
>>       .recalc_rate = socfpga_clk_recalc_rate,
>>       .get_parent = socfpga_clk_get_parent,
>>       .set_parent = socfpga_clk_set_parent,
>> @@ -254,6 +283,7 @@ static void __init socfpga_gate_clk_init(struct
>> device_node *node,
>>   {
>>       u32 clk_gate[2];
>>       u32 div_reg[3];
>> +    u32 clk_phase[2];
>>       u32 fixed_div;
>>       struct clk *clk;
>>       struct socfpga_clk *socfpga_clk;
>> @@ -294,6 +324,12 @@ static void __init socfpga_gate_clk_init(struct
>> device_node *node,
>>           socfpga_clk->div_reg = 0;
>>       }
>>
>> +    rc = of_property_read_u32_array(node, "clk-phase", clk_phase, 2);
>> +    if (!rc) {
>> +        socfpga_clk->clk_phase[0] = clk_phase[0];
>> +        socfpga_clk->clk_phase[1] = clk_phase[1];
>> +    }
>> +
>>       of_property_read_string(node, "clock-output-names", &clk_name);
>>
>>       init.name = clk_name;
>>

^ permalink raw reply	[flat|nested] 28+ messages in thread

* Re: [RESEND LIST PATCHv7 1/4] clk: socfpga: Add a clk-phase property to the "altr,socfpga-gate-clk"
  2013-12-17 13:44       ` Dinh Nguyen
@ 2013-12-17 14:47         ` zhangfei
  -1 siblings, 0 replies; 28+ messages in thread
From: zhangfei @ 2013-12-17 14:47 UTC (permalink / raw)
  To: Dinh Nguyen, dinguyen, arnd, cjb, jh80.chung, tgih.jun, heiko,
	dianders, alim.akhtar, bzhao, mturquette
  Cc: linux-mmc, linux-arm-kernel, devicetree



On 12/17/2013 09:44 PM, Dinh Nguyen wrote:
> Hi Zhangfei,
>
> On 12/17/13 1:46 AM, zhangfei wrote:
>>
>>
>> On 12/17/2013 01:04 AM, dinguyen@altera.com wrote:
>>> From: Dinh Nguyen <dinguyen@altera.com>
>>
>>> +static int socfpga_clk_prepare(struct clk_hw *hwclk)
>>> +{
>>> +    struct socfpga_clk *socfpgaclk = to_socfpga_clk(hwclk);
>>> +    struct regmap *sys_mgr_base_addr;
>>> +    u32 hs_timing;
>>> +
>>> +    if (socfpgaclk->clk_phase[0] || socfpgaclk->clk_phase[1]) {
>>> +        sys_mgr_base_addr =
>>> syscon_regmap_lookup_by_compatible("altr,sys-mgr");
>>> +        if (IS_ERR(sys_mgr_base_addr)) {
>>> +            pr_err("%s: failed to find altr,sys-mgr regmap!\n",
>>> __func__);
>>> +            return -EINVAL;
>>> +        }
>>> +        hs_timing = SYSMGR_SDMMC_CTRL_SET(socfpgaclk->clk_phase[0],
>>> +                        socfpgaclk->clk_phase[1]);
>>> +        regmap_write(sys_mgr_base_addr, SYSMGR_SDMMCGRP_CTRL_OFFSET,
>>> +                        hs_timing);
>>> +    }
>>> +    return 0;
>>> +}
>>
>> So reusing gate-clk here and check the node of "altr,sys-mgr".
>> I think it is good and simple.
>> Also can define new clock combined with node "altr,sys-mgr" with
>> parent of sdmmc_clk.
>>
>> Thanks for the update, it is fine to me.
> Thanks, can I get an Ack from you for this version?
>

Sure, if it is helpful.

Thanks

^ permalink raw reply	[flat|nested] 28+ messages in thread

* [RESEND LIST PATCHv7 1/4] clk: socfpga: Add a clk-phase property to the "altr,socfpga-gate-clk"
@ 2013-12-17 14:47         ` zhangfei
  0 siblings, 0 replies; 28+ messages in thread
From: zhangfei @ 2013-12-17 14:47 UTC (permalink / raw)
  To: linux-arm-kernel



On 12/17/2013 09:44 PM, Dinh Nguyen wrote:
> Hi Zhangfei,
>
> On 12/17/13 1:46 AM, zhangfei wrote:
>>
>>
>> On 12/17/2013 01:04 AM, dinguyen at altera.com wrote:
>>> From: Dinh Nguyen <dinguyen@altera.com>
>>
>>> +static int socfpga_clk_prepare(struct clk_hw *hwclk)
>>> +{
>>> +    struct socfpga_clk *socfpgaclk = to_socfpga_clk(hwclk);
>>> +    struct regmap *sys_mgr_base_addr;
>>> +    u32 hs_timing;
>>> +
>>> +    if (socfpgaclk->clk_phase[0] || socfpgaclk->clk_phase[1]) {
>>> +        sys_mgr_base_addr =
>>> syscon_regmap_lookup_by_compatible("altr,sys-mgr");
>>> +        if (IS_ERR(sys_mgr_base_addr)) {
>>> +            pr_err("%s: failed to find altr,sys-mgr regmap!\n",
>>> __func__);
>>> +            return -EINVAL;
>>> +        }
>>> +        hs_timing = SYSMGR_SDMMC_CTRL_SET(socfpgaclk->clk_phase[0],
>>> +                        socfpgaclk->clk_phase[1]);
>>> +        regmap_write(sys_mgr_base_addr, SYSMGR_SDMMCGRP_CTRL_OFFSET,
>>> +                        hs_timing);
>>> +    }
>>> +    return 0;
>>> +}
>>
>> So reusing gate-clk here and check the node of "altr,sys-mgr".
>> I think it is good and simple.
>> Also can define new clock combined with node "altr,sys-mgr" with
>> parent of sdmmc_clk.
>>
>> Thanks for the update, it is fine to me.
> Thanks, can I get an Ack from you for this version?
>

Sure, if it is helpful.

Thanks

^ permalink raw reply	[flat|nested] 28+ messages in thread

* Re: [RESEND LIST PATCHv7 1/4] clk: socfpga: Add a clk-phase property to the "altr, socfpga-gate-clk"
  2013-12-17 13:44       ` Dinh Nguyen
@ 2013-12-17 23:55         ` Mike Turquette
  -1 siblings, 0 replies; 28+ messages in thread
From: Mike Turquette @ 2013-12-17 23:55 UTC (permalink / raw)
  To: Dinh Nguyen, zhangfei, dinguyen, arnd, cjb, jh80.chung, tgih.jun,
	heiko, dianders, alim.akhtar, bzhao
  Cc: devicetree, linux-mmc, linux-arm-kernel

Quoting Dinh Nguyen (2013-12-17 05:44:47)
> Hi Zhangfei,
> 
> On 12/17/13 1:46 AM, zhangfei wrote:
> >
> >
> > On 12/17/2013 01:04 AM, dinguyen@altera.com wrote:
> >> From: Dinh Nguyen <dinguyen@altera.com>
> >
> >> +static int socfpga_clk_prepare(struct clk_hw *hwclk)
> >> +{
> >> +    struct socfpga_clk *socfpgaclk = to_socfpga_clk(hwclk);
> >> +    struct regmap *sys_mgr_base_addr;
> >> +    u32 hs_timing;
> >> +
> >> +    if (socfpgaclk->clk_phase[0] || socfpgaclk->clk_phase[1]) {
> >> +        sys_mgr_base_addr =
> >> syscon_regmap_lookup_by_compatible("altr,sys-mgr");
> >> +        if (IS_ERR(sys_mgr_base_addr)) {
> >> +            pr_err("%s: failed to find altr,sys-mgr regmap!\n",
> >> __func__);
> >> +            return -EINVAL;
> >> +        }
> >> +        hs_timing = SYSMGR_SDMMC_CTRL_SET(socfpgaclk->clk_phase[0],
> >> +                        socfpgaclk->clk_phase[1]);
> >> +        regmap_write(sys_mgr_base_addr, SYSMGR_SDMMCGRP_CTRL_OFFSET,
> >> +                        hs_timing);
> >> +    }
> >> +    return 0;
> >> +}
> >
> > So reusing gate-clk here and check the node of "altr,sys-mgr".
> > I think it is good and simple.
> > Also can define new clock combined with node "altr,sys-mgr" with
> > parent of sdmmc_clk.
> >
> > Thanks for the update, it is fine to me.
> Thanks, can I get an Ack from you for this version?

Dinh,

Will you be spinning another version of this patch? If not I can go
ahead and take this into clk-next.

Regards,
Mike

> 
> Thanks,
> Dinh
> >
> >> +
> >>   static struct clk_ops gateclk_ops = {
> >> +    .prepare = socfpga_clk_prepare,
> >>       .recalc_rate = socfpga_clk_recalc_rate,
> >>       .get_parent = socfpga_clk_get_parent,
> >>       .set_parent = socfpga_clk_set_parent,
> >> @@ -254,6 +283,7 @@ static void __init socfpga_gate_clk_init(struct
> >> device_node *node,
> >>   {
> >>       u32 clk_gate[2];
> >>       u32 div_reg[3];
> >> +    u32 clk_phase[2];
> >>       u32 fixed_div;
> >>       struct clk *clk;
> >>       struct socfpga_clk *socfpga_clk;
> >> @@ -294,6 +324,12 @@ static void __init socfpga_gate_clk_init(struct
> >> device_node *node,
> >>           socfpga_clk->div_reg = 0;
> >>       }
> >>
> >> +    rc = of_property_read_u32_array(node, "clk-phase", clk_phase, 2);
> >> +    if (!rc) {
> >> +        socfpga_clk->clk_phase[0] = clk_phase[0];
> >> +        socfpga_clk->clk_phase[1] = clk_phase[1];
> >> +    }
> >> +
> >>       of_property_read_string(node, "clock-output-names", &clk_name);
> >>
> >>       init.name = clk_name;
> >>
> 

^ permalink raw reply	[flat|nested] 28+ messages in thread

* [RESEND LIST PATCHv7 1/4] clk: socfpga: Add a clk-phase property to the "altr, socfpga-gate-clk"
@ 2013-12-17 23:55         ` Mike Turquette
  0 siblings, 0 replies; 28+ messages in thread
From: Mike Turquette @ 2013-12-17 23:55 UTC (permalink / raw)
  To: linux-arm-kernel

Quoting Dinh Nguyen (2013-12-17 05:44:47)
> Hi Zhangfei,
> 
> On 12/17/13 1:46 AM, zhangfei wrote:
> >
> >
> > On 12/17/2013 01:04 AM, dinguyen at altera.com wrote:
> >> From: Dinh Nguyen <dinguyen@altera.com>
> >
> >> +static int socfpga_clk_prepare(struct clk_hw *hwclk)
> >> +{
> >> +    struct socfpga_clk *socfpgaclk = to_socfpga_clk(hwclk);
> >> +    struct regmap *sys_mgr_base_addr;
> >> +    u32 hs_timing;
> >> +
> >> +    if (socfpgaclk->clk_phase[0] || socfpgaclk->clk_phase[1]) {
> >> +        sys_mgr_base_addr =
> >> syscon_regmap_lookup_by_compatible("altr,sys-mgr");
> >> +        if (IS_ERR(sys_mgr_base_addr)) {
> >> +            pr_err("%s: failed to find altr,sys-mgr regmap!\n",
> >> __func__);
> >> +            return -EINVAL;
> >> +        }
> >> +        hs_timing = SYSMGR_SDMMC_CTRL_SET(socfpgaclk->clk_phase[0],
> >> +                        socfpgaclk->clk_phase[1]);
> >> +        regmap_write(sys_mgr_base_addr, SYSMGR_SDMMCGRP_CTRL_OFFSET,
> >> +                        hs_timing);
> >> +    }
> >> +    return 0;
> >> +}
> >
> > So reusing gate-clk here and check the node of "altr,sys-mgr".
> > I think it is good and simple.
> > Also can define new clock combined with node "altr,sys-mgr" with
> > parent of sdmmc_clk.
> >
> > Thanks for the update, it is fine to me.
> Thanks, can I get an Ack from you for this version?

Dinh,

Will you be spinning another version of this patch? If not I can go
ahead and take this into clk-next.

Regards,
Mike

> 
> Thanks,
> Dinh
> >
> >> +
> >>   static struct clk_ops gateclk_ops = {
> >> +    .prepare = socfpga_clk_prepare,
> >>       .recalc_rate = socfpga_clk_recalc_rate,
> >>       .get_parent = socfpga_clk_get_parent,
> >>       .set_parent = socfpga_clk_set_parent,
> >> @@ -254,6 +283,7 @@ static void __init socfpga_gate_clk_init(struct
> >> device_node *node,
> >>   {
> >>       u32 clk_gate[2];
> >>       u32 div_reg[3];
> >> +    u32 clk_phase[2];
> >>       u32 fixed_div;
> >>       struct clk *clk;
> >>       struct socfpga_clk *socfpga_clk;
> >> @@ -294,6 +324,12 @@ static void __init socfpga_gate_clk_init(struct
> >> device_node *node,
> >>           socfpga_clk->div_reg = 0;
> >>       }
> >>
> >> +    rc = of_property_read_u32_array(node, "clk-phase", clk_phase, 2);
> >> +    if (!rc) {
> >> +        socfpga_clk->clk_phase[0] = clk_phase[0];
> >> +        socfpga_clk->clk_phase[1] = clk_phase[1];
> >> +    }
> >> +
> >>       of_property_read_string(node, "clock-output-names", &clk_name);
> >>
> >>       init.name = clk_name;
> >>
> 

^ permalink raw reply	[flat|nested] 28+ messages in thread

* Re: [RESEND LIST PATCHv7 1/4] clk: socfpga: Add a clk-phase property to the "altr, socfpga-gate-clk"
  2013-12-17 23:55         ` Mike Turquette
@ 2013-12-18  4:06           ` Dinh Nguyen
  -1 siblings, 0 replies; 28+ messages in thread
From: Dinh Nguyen @ 2013-12-18  4:06 UTC (permalink / raw)
  To: Mike Turquette, zhangfei, dinguyen, arnd, cjb, jh80.chung,
	tgih.jun, heiko, dianders, alim.akhtar, bzhao
  Cc: linux-mmc, linux-arm-kernel, devicetree

Hi Mike,

On 12/17/13 5:55 PM, Mike Turquette wrote:
> Quoting Dinh Nguyen (2013-12-17 05:44:47)
>> Hi Zhangfei,
>>
>> On 12/17/13 1:46 AM, zhangfei wrote:
>>>
>>> On 12/17/2013 01:04 AM, dinguyen@altera.com wrote:
>>>> From: Dinh Nguyen <dinguyen@altera.com>
>>>> +static int socfpga_clk_prepare(struct clk_hw *hwclk)
>>>> +{
>>>> +    struct socfpga_clk *socfpgaclk = to_socfpga_clk(hwclk);
>>>> +    struct regmap *sys_mgr_base_addr;
>>>> +    u32 hs_timing;
>>>> +
>>>> +    if (socfpgaclk->clk_phase[0] || socfpgaclk->clk_phase[1]) {
>>>> +        sys_mgr_base_addr =
>>>> syscon_regmap_lookup_by_compatible("altr,sys-mgr");
>>>> +        if (IS_ERR(sys_mgr_base_addr)) {
>>>> +            pr_err("%s: failed to find altr,sys-mgr regmap!\n",
>>>> __func__);
>>>> +            return -EINVAL;
>>>> +        }
>>>> +        hs_timing = SYSMGR_SDMMC_CTRL_SET(socfpgaclk->clk_phase[0],
>>>> +                        socfpgaclk->clk_phase[1]);
>>>> +        regmap_write(sys_mgr_base_addr, SYSMGR_SDMMCGRP_CTRL_OFFSET,
>>>> +                        hs_timing);
>>>> +    }
>>>> +    return 0;
>>>> +}
>>> So reusing gate-clk here and check the node of "altr,sys-mgr".
>>> I think it is good and simple.
>>> Also can define new clock combined with node "altr,sys-mgr" with
>>> parent of sdmmc_clk.
>>>
>>> Thanks for the update, it is fine to me.
>> Thanks, can I get an Ack from you for this version?
> Dinh,
>
> Will you be spinning another version of this patch? If not I can go
> ahead and take this into clk-next.
If you are OK with the patch, then yes, can you please apply it to clk-next?

Thanks,
Dinh
>
> Regards,
> Mike
>
>> Thanks,
>> Dinh
>>>> +
>>>>   static struct clk_ops gateclk_ops = {
>>>> +    .prepare = socfpga_clk_prepare,
>>>>       .recalc_rate = socfpga_clk_recalc_rate,
>>>>       .get_parent = socfpga_clk_get_parent,
>>>>       .set_parent = socfpga_clk_set_parent,
>>>> @@ -254,6 +283,7 @@ static void __init socfpga_gate_clk_init(struct
>>>> device_node *node,
>>>>   {
>>>>       u32 clk_gate[2];
>>>>       u32 div_reg[3];
>>>> +    u32 clk_phase[2];
>>>>       u32 fixed_div;
>>>>       struct clk *clk;
>>>>       struct socfpga_clk *socfpga_clk;
>>>> @@ -294,6 +324,12 @@ static void __init socfpga_gate_clk_init(struct
>>>> device_node *node,
>>>>           socfpga_clk->div_reg = 0;
>>>>       }
>>>>
>>>> +    rc = of_property_read_u32_array(node, "clk-phase", clk_phase, 2);
>>>> +    if (!rc) {
>>>> +        socfpga_clk->clk_phase[0] = clk_phase[0];
>>>> +        socfpga_clk->clk_phase[1] = clk_phase[1];
>>>> +    }
>>>> +
>>>>       of_property_read_string(node, "clock-output-names", &clk_name);
>>>>
>>>>       init.name = clk_name;
>>>>


^ permalink raw reply	[flat|nested] 28+ messages in thread

* [RESEND LIST PATCHv7 1/4] clk: socfpga: Add a clk-phase property to the "altr, socfpga-gate-clk"
@ 2013-12-18  4:06           ` Dinh Nguyen
  0 siblings, 0 replies; 28+ messages in thread
From: Dinh Nguyen @ 2013-12-18  4:06 UTC (permalink / raw)
  To: linux-arm-kernel

Hi Mike,

On 12/17/13 5:55 PM, Mike Turquette wrote:
> Quoting Dinh Nguyen (2013-12-17 05:44:47)
>> Hi Zhangfei,
>>
>> On 12/17/13 1:46 AM, zhangfei wrote:
>>>
>>> On 12/17/2013 01:04 AM, dinguyen at altera.com wrote:
>>>> From: Dinh Nguyen <dinguyen@altera.com>
>>>> +static int socfpga_clk_prepare(struct clk_hw *hwclk)
>>>> +{
>>>> +    struct socfpga_clk *socfpgaclk = to_socfpga_clk(hwclk);
>>>> +    struct regmap *sys_mgr_base_addr;
>>>> +    u32 hs_timing;
>>>> +
>>>> +    if (socfpgaclk->clk_phase[0] || socfpgaclk->clk_phase[1]) {
>>>> +        sys_mgr_base_addr =
>>>> syscon_regmap_lookup_by_compatible("altr,sys-mgr");
>>>> +        if (IS_ERR(sys_mgr_base_addr)) {
>>>> +            pr_err("%s: failed to find altr,sys-mgr regmap!\n",
>>>> __func__);
>>>> +            return -EINVAL;
>>>> +        }
>>>> +        hs_timing = SYSMGR_SDMMC_CTRL_SET(socfpgaclk->clk_phase[0],
>>>> +                        socfpgaclk->clk_phase[1]);
>>>> +        regmap_write(sys_mgr_base_addr, SYSMGR_SDMMCGRP_CTRL_OFFSET,
>>>> +                        hs_timing);
>>>> +    }
>>>> +    return 0;
>>>> +}
>>> So reusing gate-clk here and check the node of "altr,sys-mgr".
>>> I think it is good and simple.
>>> Also can define new clock combined with node "altr,sys-mgr" with
>>> parent of sdmmc_clk.
>>>
>>> Thanks for the update, it is fine to me.
>> Thanks, can I get an Ack from you for this version?
> Dinh,
>
> Will you be spinning another version of this patch? If not I can go
> ahead and take this into clk-next.
If you are OK with the patch, then yes, can you please apply it to clk-next?

Thanks,
Dinh
>
> Regards,
> Mike
>
>> Thanks,
>> Dinh
>>>> +
>>>>   static struct clk_ops gateclk_ops = {
>>>> +    .prepare = socfpga_clk_prepare,
>>>>       .recalc_rate = socfpga_clk_recalc_rate,
>>>>       .get_parent = socfpga_clk_get_parent,
>>>>       .set_parent = socfpga_clk_set_parent,
>>>> @@ -254,6 +283,7 @@ static void __init socfpga_gate_clk_init(struct
>>>> device_node *node,
>>>>   {
>>>>       u32 clk_gate[2];
>>>>       u32 div_reg[3];
>>>> +    u32 clk_phase[2];
>>>>       u32 fixed_div;
>>>>       struct clk *clk;
>>>>       struct socfpga_clk *socfpga_clk;
>>>> @@ -294,6 +324,12 @@ static void __init socfpga_gate_clk_init(struct
>>>> device_node *node,
>>>>           socfpga_clk->div_reg = 0;
>>>>       }
>>>>
>>>> +    rc = of_property_read_u32_array(node, "clk-phase", clk_phase, 2);
>>>> +    if (!rc) {
>>>> +        socfpga_clk->clk_phase[0] = clk_phase[0];
>>>> +        socfpga_clk->clk_phase[1] = clk_phase[1];
>>>> +    }
>>>> +
>>>>       of_property_read_string(node, "clock-output-names", &clk_name);
>>>>
>>>>       init.name = clk_name;
>>>>

^ permalink raw reply	[flat|nested] 28+ messages in thread

* Re: [RESEND LIST PATCHv7 1/4] clk: socfpga: Add a clk-phase property to the "altr, socfpga-gate-clk"
  2013-12-16 17:04   ` [RESEND LIST PATCHv7 1/4] clk: socfpga: Add a clk-phase property to the "altr, socfpga-gate-clk" dinguyen at altera.com
@ 2013-12-18 20:56     ` Mike Turquette
  -1 siblings, 0 replies; 28+ messages in thread
From: Mike Turquette @ 2013-12-18 20:56 UTC (permalink / raw)
  To: dinh.linux, arnd, cjb, jh80.chung, tgih.jun, heiko, dianders,
	alim.akhtar, bzhao
  Cc: zhangfei.gao, linux-mmc, linux-arm-kernel, devicetree, Dinh Nguyen

Quoting dinguyen@altera.com (2013-12-16 09:04:33)
> From: Dinh Nguyen <dinguyen@altera.com>
> 
> The clk-phase property is used to represent the 2 clock phase values that is
> needed for the SD/MMC driver. Add a prepare function to the clk_ops, that will
> use the syscon driver to set sdmmc_clk's phase shift that is located in the
> system manager.
> 
> Signed-off-by: Dinh Nguyen <dinguyen@altera.com>
> ---
> v7: Add dts property to represent the clk phase of the sdmmc_clk. Add a
>     prepare function to the gate clk that will toggle clock phase setting.
>     Remove the "altr,socfpga-sdmmc-sdr-clk" clock type.
> v6: Add a new clock type "altr,socfpga-sdmmc-sdr-clk" that will be used to
> set the phase shift settings.
> v5: Use the "snps,dw-mshc" binding
> v4: Use the sdmmc_clk prepare function to set the phase shift settings
> v3: Not use the syscon driver because as of 3.13-rc1, the syscon driver is
>     loaded after the clock driver.
> v2: Use the syscon driver
> ---
>  .../devicetree/bindings/clock/altr_socfpga.txt     |   14 ++++++++
>  arch/arm/boot/dts/socfpga.dtsi                     |    1 +
>  drivers/clk/socfpga/clk.c                          |   36 ++++++++++++++++++++
>  3 files changed, 51 insertions(+)
> 
> diff --git a/Documentation/devicetree/bindings/clock/altr_socfpga.txt b/Documentation/devicetree/bindings/clock/altr_socfpga.txt
> index 0045433..4b66754 100644
> --- a/Documentation/devicetree/bindings/clock/altr_socfpga.txt
> +++ b/Documentation/devicetree/bindings/clock/altr_socfpga.txt
> @@ -23,3 +23,17 @@ Optional properties:
>          and the bit index.
>  - div-reg : For "socfpga-gate-clk", div-reg contains the divider register, bit shift,
>          and width.
> +- clk-phase : For the sdmmc_clk, contains the value of the clock phase that controls
> +  the SDMMC CIU clock. The first value is the clk_sample(smpsel), and the second
> +  value is the cclk_in_drv(drvsel). The clk-phase is used to enable the correct
> +  hold/delay times that is needed for the SD/MMC CIU clock. The values of both
> +  value can be one of the following:
> +
> +       <0x0>   :       0 degrees
> +       <0x1>   :       45 degrees
> +       <0x2>   :       90 degrees
> +       <0x3>   :       135 degrees
> +       <0x4>   :       180 degrees
> +       <0x5>   :       225 degrees
> +       <0x6>   :       270 degrees
> +       <0x7>   :       315 degrees
> diff --git a/arch/arm/boot/dts/socfpga.dtsi b/arch/arm/boot/dts/socfpga.dtsi
> index f936476..616d9ee 100644
> --- a/arch/arm/boot/dts/socfpga.dtsi
> +++ b/arch/arm/boot/dts/socfpga.dtsi
> @@ -413,6 +413,7 @@
>                                                 compatible = "altr,socfpga-gate-clk";
>                                                 clocks = <&f2s_periph_ref_clk>, <&main_nand_sdmmc_clk>, <&per_nand_mmc_clk>;
>                                                 clk-gate = <0xa0 8>;
> +                                               clk-phase = <0 3>;

Looking at this again, I have a hard time understanding the values in
the clk-phase property. You reference some functions in the property
definition above, but they are not obvious to me.

Additionally I wonder if the binding would better if the clock-phase
property was simply the value in degrees. E.g:

	clk-phase = <315>;    // 315 degrees

Regards,
Mike

>                                         };
>  
>                                         nand_x_clk: nand_x_clk {
> diff --git a/drivers/clk/socfpga/clk.c b/drivers/clk/socfpga/clk.c
> index 280c983..7cfebd6 100644
> --- a/drivers/clk/socfpga/clk.c
> +++ b/drivers/clk/socfpga/clk.c
> @@ -21,8 +21,10 @@
>  #include <linux/clkdev.h>
>  #include <linux/clk-provider.h>
>  #include <linux/io.h>
> +#include <linux/mfd/syscon.h>
>  #include <linux/of.h>
>  #include <linux/of_address.h>
> +#include <linux/regmap.h>
>  
>  /* Clock Manager offsets */
>  #define CLKMGR_CTRL    0x0
> @@ -56,6 +58,11 @@
>  #define div_mask(width)        ((1 << (width)) - 1)
>  #define streq(a, b) (strcmp((a), (b)) == 0)
>  
> +/* SDMMC Group for System Manager defines */
> +#define SYSMGR_SDMMCGRP_CTRL_OFFSET    0x108
> +#define SYSMGR_SDMMC_CTRL_SET(smplsel, drvsel) \
> +       ((((smplsel) & 0x7) << 3) | (((drvsel) & 0x7) << 0))
> +
>  static void __iomem *clk_mgr_base_addr;
>  
>  struct socfpga_clk {
> @@ -66,6 +73,7 @@ struct socfpga_clk {
>         void __iomem *div_reg;
>         u32 width;      /* only valid if div_reg != 0 */
>         u32 shift;      /* only valid if div_reg != 0 */
> +       u32 clk_phase[2];
>  };
>  #define to_socfpga_clk(p) container_of(p, struct socfpga_clk, hw.hw)
>  
> @@ -243,7 +251,28 @@ static unsigned long socfpga_clk_recalc_rate(struct clk_hw *hwclk,
>         return parent_rate / div;
>  }
>  
> +static int socfpga_clk_prepare(struct clk_hw *hwclk)
> +{
> +       struct socfpga_clk *socfpgaclk = to_socfpga_clk(hwclk);
> +       struct regmap *sys_mgr_base_addr;
> +       u32 hs_timing;
> +
> +       if (socfpgaclk->clk_phase[0] || socfpgaclk->clk_phase[1]) {
> +               sys_mgr_base_addr = syscon_regmap_lookup_by_compatible("altr,sys-mgr");
> +               if (IS_ERR(sys_mgr_base_addr)) {
> +                       pr_err("%s: failed to find altr,sys-mgr regmap!\n", __func__);
> +                       return -EINVAL;
> +               }
> +               hs_timing = SYSMGR_SDMMC_CTRL_SET(socfpgaclk->clk_phase[0],
> +                                               socfpgaclk->clk_phase[1]);
> +               regmap_write(sys_mgr_base_addr, SYSMGR_SDMMCGRP_CTRL_OFFSET,
> +                                               hs_timing);
> +       }
> +       return 0;
> +}
> +
>  static struct clk_ops gateclk_ops = {
> +       .prepare = socfpga_clk_prepare,
>         .recalc_rate = socfpga_clk_recalc_rate,
>         .get_parent = socfpga_clk_get_parent,
>         .set_parent = socfpga_clk_set_parent,
> @@ -254,6 +283,7 @@ static void __init socfpga_gate_clk_init(struct device_node *node,
>  {
>         u32 clk_gate[2];
>         u32 div_reg[3];
> +       u32 clk_phase[2];
>         u32 fixed_div;
>         struct clk *clk;
>         struct socfpga_clk *socfpga_clk;
> @@ -294,6 +324,12 @@ static void __init socfpga_gate_clk_init(struct device_node *node,
>                 socfpga_clk->div_reg = 0;
>         }
>  
> +       rc = of_property_read_u32_array(node, "clk-phase", clk_phase, 2);
> +       if (!rc) {
> +               socfpga_clk->clk_phase[0] = clk_phase[0];
> +               socfpga_clk->clk_phase[1] = clk_phase[1];
> +       }
> +
>         of_property_read_string(node, "clock-output-names", &clk_name);
>  
>         init.name = clk_name;
> -- 
> 1.7.9.5
> 
> 

^ permalink raw reply	[flat|nested] 28+ messages in thread

* [RESEND LIST PATCHv7 1/4] clk: socfpga: Add a clk-phase property to the "altr, socfpga-gate-clk"
@ 2013-12-18 20:56     ` Mike Turquette
  0 siblings, 0 replies; 28+ messages in thread
From: Mike Turquette @ 2013-12-18 20:56 UTC (permalink / raw)
  To: linux-arm-kernel

Quoting dinguyen at altera.com (2013-12-16 09:04:33)
> From: Dinh Nguyen <dinguyen@altera.com>
> 
> The clk-phase property is used to represent the 2 clock phase values that is
> needed for the SD/MMC driver. Add a prepare function to the clk_ops, that will
> use the syscon driver to set sdmmc_clk's phase shift that is located in the
> system manager.
> 
> Signed-off-by: Dinh Nguyen <dinguyen@altera.com>
> ---
> v7: Add dts property to represent the clk phase of the sdmmc_clk. Add a
>     prepare function to the gate clk that will toggle clock phase setting.
>     Remove the "altr,socfpga-sdmmc-sdr-clk" clock type.
> v6: Add a new clock type "altr,socfpga-sdmmc-sdr-clk" that will be used to
> set the phase shift settings.
> v5: Use the "snps,dw-mshc" binding
> v4: Use the sdmmc_clk prepare function to set the phase shift settings
> v3: Not use the syscon driver because as of 3.13-rc1, the syscon driver is
>     loaded after the clock driver.
> v2: Use the syscon driver
> ---
>  .../devicetree/bindings/clock/altr_socfpga.txt     |   14 ++++++++
>  arch/arm/boot/dts/socfpga.dtsi                     |    1 +
>  drivers/clk/socfpga/clk.c                          |   36 ++++++++++++++++++++
>  3 files changed, 51 insertions(+)
> 
> diff --git a/Documentation/devicetree/bindings/clock/altr_socfpga.txt b/Documentation/devicetree/bindings/clock/altr_socfpga.txt
> index 0045433..4b66754 100644
> --- a/Documentation/devicetree/bindings/clock/altr_socfpga.txt
> +++ b/Documentation/devicetree/bindings/clock/altr_socfpga.txt
> @@ -23,3 +23,17 @@ Optional properties:
>          and the bit index.
>  - div-reg : For "socfpga-gate-clk", div-reg contains the divider register, bit shift,
>          and width.
> +- clk-phase : For the sdmmc_clk, contains the value of the clock phase that controls
> +  the SDMMC CIU clock. The first value is the clk_sample(smpsel), and the second
> +  value is the cclk_in_drv(drvsel). The clk-phase is used to enable the correct
> +  hold/delay times that is needed for the SD/MMC CIU clock. The values of both
> +  value can be one of the following:
> +
> +       <0x0>   :       0 degrees
> +       <0x1>   :       45 degrees
> +       <0x2>   :       90 degrees
> +       <0x3>   :       135 degrees
> +       <0x4>   :       180 degrees
> +       <0x5>   :       225 degrees
> +       <0x6>   :       270 degrees
> +       <0x7>   :       315 degrees
> diff --git a/arch/arm/boot/dts/socfpga.dtsi b/arch/arm/boot/dts/socfpga.dtsi
> index f936476..616d9ee 100644
> --- a/arch/arm/boot/dts/socfpga.dtsi
> +++ b/arch/arm/boot/dts/socfpga.dtsi
> @@ -413,6 +413,7 @@
>                                                 compatible = "altr,socfpga-gate-clk";
>                                                 clocks = <&f2s_periph_ref_clk>, <&main_nand_sdmmc_clk>, <&per_nand_mmc_clk>;
>                                                 clk-gate = <0xa0 8>;
> +                                               clk-phase = <0 3>;

Looking at this again, I have a hard time understanding the values in
the clk-phase property. You reference some functions in the property
definition above, but they are not obvious to me.

Additionally I wonder if the binding would better if the clock-phase
property was simply the value in degrees. E.g:

	clk-phase = <315>;    // 315 degrees

Regards,
Mike

>                                         };
>  
>                                         nand_x_clk: nand_x_clk {
> diff --git a/drivers/clk/socfpga/clk.c b/drivers/clk/socfpga/clk.c
> index 280c983..7cfebd6 100644
> --- a/drivers/clk/socfpga/clk.c
> +++ b/drivers/clk/socfpga/clk.c
> @@ -21,8 +21,10 @@
>  #include <linux/clkdev.h>
>  #include <linux/clk-provider.h>
>  #include <linux/io.h>
> +#include <linux/mfd/syscon.h>
>  #include <linux/of.h>
>  #include <linux/of_address.h>
> +#include <linux/regmap.h>
>  
>  /* Clock Manager offsets */
>  #define CLKMGR_CTRL    0x0
> @@ -56,6 +58,11 @@
>  #define div_mask(width)        ((1 << (width)) - 1)
>  #define streq(a, b) (strcmp((a), (b)) == 0)
>  
> +/* SDMMC Group for System Manager defines */
> +#define SYSMGR_SDMMCGRP_CTRL_OFFSET    0x108
> +#define SYSMGR_SDMMC_CTRL_SET(smplsel, drvsel) \
> +       ((((smplsel) & 0x7) << 3) | (((drvsel) & 0x7) << 0))
> +
>  static void __iomem *clk_mgr_base_addr;
>  
>  struct socfpga_clk {
> @@ -66,6 +73,7 @@ struct socfpga_clk {
>         void __iomem *div_reg;
>         u32 width;      /* only valid if div_reg != 0 */
>         u32 shift;      /* only valid if div_reg != 0 */
> +       u32 clk_phase[2];
>  };
>  #define to_socfpga_clk(p) container_of(p, struct socfpga_clk, hw.hw)
>  
> @@ -243,7 +251,28 @@ static unsigned long socfpga_clk_recalc_rate(struct clk_hw *hwclk,
>         return parent_rate / div;
>  }
>  
> +static int socfpga_clk_prepare(struct clk_hw *hwclk)
> +{
> +       struct socfpga_clk *socfpgaclk = to_socfpga_clk(hwclk);
> +       struct regmap *sys_mgr_base_addr;
> +       u32 hs_timing;
> +
> +       if (socfpgaclk->clk_phase[0] || socfpgaclk->clk_phase[1]) {
> +               sys_mgr_base_addr = syscon_regmap_lookup_by_compatible("altr,sys-mgr");
> +               if (IS_ERR(sys_mgr_base_addr)) {
> +                       pr_err("%s: failed to find altr,sys-mgr regmap!\n", __func__);
> +                       return -EINVAL;
> +               }
> +               hs_timing = SYSMGR_SDMMC_CTRL_SET(socfpgaclk->clk_phase[0],
> +                                               socfpgaclk->clk_phase[1]);
> +               regmap_write(sys_mgr_base_addr, SYSMGR_SDMMCGRP_CTRL_OFFSET,
> +                                               hs_timing);
> +       }
> +       return 0;
> +}
> +
>  static struct clk_ops gateclk_ops = {
> +       .prepare = socfpga_clk_prepare,
>         .recalc_rate = socfpga_clk_recalc_rate,
>         .get_parent = socfpga_clk_get_parent,
>         .set_parent = socfpga_clk_set_parent,
> @@ -254,6 +283,7 @@ static void __init socfpga_gate_clk_init(struct device_node *node,
>  {
>         u32 clk_gate[2];
>         u32 div_reg[3];
> +       u32 clk_phase[2];
>         u32 fixed_div;
>         struct clk *clk;
>         struct socfpga_clk *socfpga_clk;
> @@ -294,6 +324,12 @@ static void __init socfpga_gate_clk_init(struct device_node *node,
>                 socfpga_clk->div_reg = 0;
>         }
>  
> +       rc = of_property_read_u32_array(node, "clk-phase", clk_phase, 2);
> +       if (!rc) {
> +               socfpga_clk->clk_phase[0] = clk_phase[0];
> +               socfpga_clk->clk_phase[1] = clk_phase[1];
> +       }
> +
>         of_property_read_string(node, "clock-output-names", &clk_name);
>  
>         init.name = clk_name;
> -- 
> 1.7.9.5
> 
> 

^ permalink raw reply	[flat|nested] 28+ messages in thread

* Re: [RESEND LIST PATCHv7 1/4] clk: socfpga: Add a clk-phase property to the "altr, socfpga-gate-clk"
  2013-12-18 20:56     ` Mike Turquette
@ 2013-12-18 21:21       ` Arnd Bergmann
  -1 siblings, 0 replies; 28+ messages in thread
From: Arnd Bergmann @ 2013-12-18 21:21 UTC (permalink / raw)
  To: Mike Turquette
  Cc: dinguyen, dinh.linux, cjb, jh80.chung, tgih.jun, heiko, dianders,
	alim.akhtar, bzhao, zhangfei.gao, linux-mmc, linux-arm-kernel,
	devicetree

On Wednesday 18 December 2013, Mike Turquette wrote:
> > diff --git a/arch/arm/boot/dts/socfpga.dtsi b/arch/arm/boot/dts/socfpga.dtsi
> > index f936476..616d9ee 100644
> > --- a/arch/arm/boot/dts/socfpga.dtsi
> > +++ b/arch/arm/boot/dts/socfpga.dtsi
> > @@ -413,6 +413,7 @@
> >                                                 compatible = "altr,socfpga-gate-clk";
> >                                                 clocks = <&f2s_periph_ref_clk>, <&main_nand_sdmmc_clk>, <&per_nand_mmc_clk>;
> >                                                 clk-gate = <0xa0 8>;
> > +                                               clk-phase = <0 3>;
> 
> Looking at this again, I have a hard time understanding the values in
> the clk-phase property. You reference some functions in the property
> definition above, but they are not obvious to me.
> 
> Additionally I wonder if the binding would better if the clock-phase
> property was simply the value in degrees. E.g:
> 
>         clk-phase = <315>;    // 315 degrees

I would definitely prefer using degrees over an arbitrary enumeration that
might work on some platforms but not on others.

I'm also a bit skeptical about the idea of putting the phase into the clock
provider rather than the consumer, given the comments about the
clk_set_phase() interface earlier. Generally we try to avoid having
consumer-specific settings in a provider node (for any DT binding,
not just clocks). Can't you have the same numbers in the dw-mshc
node instead and let the mmc driver call clk_set_phase instead?
If every clock has a fixed phase for a given piece of hardware, it
could even be set automatically by making the common clk code read
the clk-phase attribute at the time a driver calls clk_get.

	Arnd

^ permalink raw reply	[flat|nested] 28+ messages in thread

* [RESEND LIST PATCHv7 1/4] clk: socfpga: Add a clk-phase property to the "altr, socfpga-gate-clk"
@ 2013-12-18 21:21       ` Arnd Bergmann
  0 siblings, 0 replies; 28+ messages in thread
From: Arnd Bergmann @ 2013-12-18 21:21 UTC (permalink / raw)
  To: linux-arm-kernel

On Wednesday 18 December 2013, Mike Turquette wrote:
> > diff --git a/arch/arm/boot/dts/socfpga.dtsi b/arch/arm/boot/dts/socfpga.dtsi
> > index f936476..616d9ee 100644
> > --- a/arch/arm/boot/dts/socfpga.dtsi
> > +++ b/arch/arm/boot/dts/socfpga.dtsi
> > @@ -413,6 +413,7 @@
> >                                                 compatible = "altr,socfpga-gate-clk";
> >                                                 clocks = <&f2s_periph_ref_clk>, <&main_nand_sdmmc_clk>, <&per_nand_mmc_clk>;
> >                                                 clk-gate = <0xa0 8>;
> > +                                               clk-phase = <0 3>;
> 
> Looking at this again, I have a hard time understanding the values in
> the clk-phase property. You reference some functions in the property
> definition above, but they are not obvious to me.
> 
> Additionally I wonder if the binding would better if the clock-phase
> property was simply the value in degrees. E.g:
> 
>         clk-phase = <315>;    // 315 degrees

I would definitely prefer using degrees over an arbitrary enumeration that
might work on some platforms but not on others.

I'm also a bit skeptical about the idea of putting the phase into the clock
provider rather than the consumer, given the comments about the
clk_set_phase() interface earlier. Generally we try to avoid having
consumer-specific settings in a provider node (for any DT binding,
not just clocks). Can't you have the same numbers in the dw-mshc
node instead and let the mmc driver call clk_set_phase instead?
If every clock has a fixed phase for a given piece of hardware, it
could even be set automatically by making the common clk code read
the clk-phase attribute at the time a driver calls clk_get.

	Arnd

^ permalink raw reply	[flat|nested] 28+ messages in thread

* Re: [RESEND LIST PATCHv7 1/4] clk: socfpga: Add a clk-phase property to the "altr, socfpga-gate-clk"
  2013-12-18 21:21       ` Arnd Bergmann
@ 2013-12-19  4:04         ` Dinh Nguyen
  -1 siblings, 0 replies; 28+ messages in thread
From: Dinh Nguyen @ 2013-12-19  4:04 UTC (permalink / raw)
  To: Arnd Bergmann, Mike Turquette
  Cc: dinguyen, cjb, jh80.chung, tgih.jun, heiko, dianders,
	alim.akhtar, bzhao, zhangfei.gao, linux-mmc, linux-arm-kernel,
	devicetree


On 12/18/13 3:21 PM, Arnd Bergmann wrote:
> On Wednesday 18 December 2013, Mike Turquette wrote:
>>> diff --git a/arch/arm/boot/dts/socfpga.dtsi b/arch/arm/boot/dts/socfpga.dtsi
>>> index f936476..616d9ee 100644
>>> --- a/arch/arm/boot/dts/socfpga.dtsi
>>> +++ b/arch/arm/boot/dts/socfpga.dtsi
>>> @@ -413,6 +413,7 @@
>>>                                                 compatible = "altr,socfpga-gate-clk";
>>>                                                 clocks = <&f2s_periph_ref_clk>, <&main_nand_sdmmc_clk>, <&per_nand_mmc_clk>;
>>>                                                 clk-gate = <0xa0 8>;
>>> +                                               clk-phase = <0 3>;
>> Looking at this again, I have a hard time understanding the values in
>> the clk-phase property. You reference some functions in the property
>> definition above, but they are not obvious to me.
>>
>> Additionally I wonder if the binding would better if the clock-phase
>> property was simply the value in degrees. E.g:
>>
>>         clk-phase = <315>;    // 315 degrees
> I would definitely prefer using degrees over an arbitrary enumeration that
> might work on some platforms but not on others.
>
> I'm also a bit skeptical about the idea of putting the phase into the clock
> provider rather than the consumer, given the comments about the
> clk_set_phase() interface earlier. Generally we try to avoid having
> consumer-specific settings in a provider node (for any DT binding,
> not just clocks). Can't you have the same numbers in the dw-mshc
> node instead and let the mmc driver call clk_set_phase instead?
> If every clock has a fixed phase for a given piece of hardware, it
> could even be set automatically by making the common clk code read
> the clk-phase attribute at the time a driver calls clk_get.

So I think this is what you're suggesting:
clocks = <&sdmmc_clk 0 135>, this would specify 0 and 135 degrees phase.

The clock-bindings document is stating that the integer in the clocks
property is
specifying the output number of the clock. Would this approach cause a
conflict and would
need an update to that document/approach?

Dinh

>
> 	Arnd


^ permalink raw reply	[flat|nested] 28+ messages in thread

* [RESEND LIST PATCHv7 1/4] clk: socfpga: Add a clk-phase property to the "altr, socfpga-gate-clk"
@ 2013-12-19  4:04         ` Dinh Nguyen
  0 siblings, 0 replies; 28+ messages in thread
From: Dinh Nguyen @ 2013-12-19  4:04 UTC (permalink / raw)
  To: linux-arm-kernel


On 12/18/13 3:21 PM, Arnd Bergmann wrote:
> On Wednesday 18 December 2013, Mike Turquette wrote:
>>> diff --git a/arch/arm/boot/dts/socfpga.dtsi b/arch/arm/boot/dts/socfpga.dtsi
>>> index f936476..616d9ee 100644
>>> --- a/arch/arm/boot/dts/socfpga.dtsi
>>> +++ b/arch/arm/boot/dts/socfpga.dtsi
>>> @@ -413,6 +413,7 @@
>>>                                                 compatible = "altr,socfpga-gate-clk";
>>>                                                 clocks = <&f2s_periph_ref_clk>, <&main_nand_sdmmc_clk>, <&per_nand_mmc_clk>;
>>>                                                 clk-gate = <0xa0 8>;
>>> +                                               clk-phase = <0 3>;
>> Looking at this again, I have a hard time understanding the values in
>> the clk-phase property. You reference some functions in the property
>> definition above, but they are not obvious to me.
>>
>> Additionally I wonder if the binding would better if the clock-phase
>> property was simply the value in degrees. E.g:
>>
>>         clk-phase = <315>;    // 315 degrees
> I would definitely prefer using degrees over an arbitrary enumeration that
> might work on some platforms but not on others.
>
> I'm also a bit skeptical about the idea of putting the phase into the clock
> provider rather than the consumer, given the comments about the
> clk_set_phase() interface earlier. Generally we try to avoid having
> consumer-specific settings in a provider node (for any DT binding,
> not just clocks). Can't you have the same numbers in the dw-mshc
> node instead and let the mmc driver call clk_set_phase instead?
> If every clock has a fixed phase for a given piece of hardware, it
> could even be set automatically by making the common clk code read
> the clk-phase attribute at the time a driver calls clk_get.

So I think this is what you're suggesting:
clocks = <&sdmmc_clk 0 135>, this would specify 0 and 135 degrees phase.

The clock-bindings document is stating that the integer in the clocks
property is
specifying the output number of the clock. Would this approach cause a
conflict and would
need an update to that document/approach?

Dinh

>
> 	Arnd

^ permalink raw reply	[flat|nested] 28+ messages in thread

* Re: [RESEND LIST PATCHv7 1/4] clk: socfpga: Add a clk-phase property to the "altr, socfpga-gate-clk"
  2013-12-19  4:04         ` Dinh Nguyen
@ 2013-12-19  5:50           ` Arnd Bergmann
  -1 siblings, 0 replies; 28+ messages in thread
From: Arnd Bergmann @ 2013-12-19  5:50 UTC (permalink / raw)
  To: Dinh Nguyen
  Cc: Mike Turquette, dinguyen, cjb, jh80.chung, tgih.jun, heiko,
	dianders, alim.akhtar, bzhao, zhangfei.gao, linux-mmc,
	linux-arm-kernel, devicetree

On Thursday 19 December 2013, Dinh Nguyen wrote:
> On 12/18/13 3:21 PM, Arnd Bergmann wrote:
> > On Wednesday 18 December 2013, Mike Turquette wrote:
> > I would definitely prefer using degrees over an arbitrary enumeration that
> > might work on some platforms but not on others.
> >
> > I'm also a bit skeptical about the idea of putting the phase into the clock
> > provider rather than the consumer, given the comments about the
> > clk_set_phase() interface earlier. Generally we try to avoid having
> > consumer-specific settings in a provider node (for any DT binding,
> > not just clocks). Can't you have the same numbers in the dw-mshc
> > node instead and let the mmc driver call clk_set_phase instead?
> > If every clock has a fixed phase for a given piece of hardware, it
> > could even be set automatically by making the common clk code read
> > the clk-phase attribute at the time a driver calls clk_get.
> 
> So I think this is what you're suggesting:
> clocks = <&sdmmc_clk 0 135>, this would specify 0 and 135 degrees phase.

It's not what I meant in the paragraph above, but I have suggested this
method in the past, and I think that would solve the problem very nicely
as well, if Mike agrees. 

> The clock-bindings document is stating that the integer in the clocks
> property is
> specifying the output number of the clock. Would this approach cause a
> conflict and would
> need an update to that document/approach?

I don't think we have to change the common binding. While it lists the
use of the integer for multiple clock outputs, that's not necessarily
the only possible use, and specific bindings can always override the
generic ones in a compatible way.

You would certainly have to update the binding of your clock controller
to do this, in particular because #clock-cells is now <1>. You will
probably need to add a new "compatible" string for a clock that has
a phase setting rather than just a gate, and document what the argument
is for. If possible, try to model the hardware the way it actually
is implemented. If the clock controller supports both the gate and
the phase in the same block, then make it one clock device node, but
if you have one block generating the clock and another one to shift the
phase, it may be best to model that second clock in a separate node
so you can split the driver code according to the registers, like

	mmcclock-shifted {
		compatible = "altr,socfpga-clk-shift";
		reg = <0xabcd>;
		clocks = <&mmcclock>;
		#clock-cells = <1>
	};

	mshc {
		...
		clocks = <&/clocks/mmcclock-shifted 135>;
		clock-names = "ciu";
	};

Coming back to what I actually tried to suggest above was 

	mshc {
		...
		clocks = <&/clocks/mmcclock>;
		clock-names = "ciu";
		clock-phases = <135>;
	};

This would keep the phase setting out of the specific binding and
move it to the generic clock binding. In Linux we could implement
this either by making the mshc driver read the phase value and
call clock_set_phase() on it, or the common clock code could look
up the clock-phases property at the same time it looks up clock-names
and clocks, and set this behind the back of the driver.

	Arnd

^ permalink raw reply	[flat|nested] 28+ messages in thread

* [RESEND LIST PATCHv7 1/4] clk: socfpga: Add a clk-phase property to the "altr, socfpga-gate-clk"
@ 2013-12-19  5:50           ` Arnd Bergmann
  0 siblings, 0 replies; 28+ messages in thread
From: Arnd Bergmann @ 2013-12-19  5:50 UTC (permalink / raw)
  To: linux-arm-kernel

On Thursday 19 December 2013, Dinh Nguyen wrote:
> On 12/18/13 3:21 PM, Arnd Bergmann wrote:
> > On Wednesday 18 December 2013, Mike Turquette wrote:
> > I would definitely prefer using degrees over an arbitrary enumeration that
> > might work on some platforms but not on others.
> >
> > I'm also a bit skeptical about the idea of putting the phase into the clock
> > provider rather than the consumer, given the comments about the
> > clk_set_phase() interface earlier. Generally we try to avoid having
> > consumer-specific settings in a provider node (for any DT binding,
> > not just clocks). Can't you have the same numbers in the dw-mshc
> > node instead and let the mmc driver call clk_set_phase instead?
> > If every clock has a fixed phase for a given piece of hardware, it
> > could even be set automatically by making the common clk code read
> > the clk-phase attribute at the time a driver calls clk_get.
> 
> So I think this is what you're suggesting:
> clocks = <&sdmmc_clk 0 135>, this would specify 0 and 135 degrees phase.

It's not what I meant in the paragraph above, but I have suggested this
method in the past, and I think that would solve the problem very nicely
as well, if Mike agrees. 

> The clock-bindings document is stating that the integer in the clocks
> property is
> specifying the output number of the clock. Would this approach cause a
> conflict and would
> need an update to that document/approach?

I don't think we have to change the common binding. While it lists the
use of the integer for multiple clock outputs, that's not necessarily
the only possible use, and specific bindings can always override the
generic ones in a compatible way.

You would certainly have to update the binding of your clock controller
to do this, in particular because #clock-cells is now <1>. You will
probably need to add a new "compatible" string for a clock that has
a phase setting rather than just a gate, and document what the argument
is for. If possible, try to model the hardware the way it actually
is implemented. If the clock controller supports both the gate and
the phase in the same block, then make it one clock device node, but
if you have one block generating the clock and another one to shift the
phase, it may be best to model that second clock in a separate node
so you can split the driver code according to the registers, like

	mmcclock-shifted {
		compatible = "altr,socfpga-clk-shift";
		reg = <0xabcd>;
		clocks = <&mmcclock>;
		#clock-cells = <1>
	};

	mshc {
		...
		clocks = <&/clocks/mmcclock-shifted 135>;
		clock-names = "ciu";
	};

Coming back to what I actually tried to suggest above was 

	mshc {
		...
		clocks = <&/clocks/mmcclock>;
		clock-names = "ciu";
		clock-phases = <135>;
	};

This would keep the phase setting out of the specific binding and
move it to the generic clock binding. In Linux we could implement
this either by making the mshc driver read the phase value and
call clock_set_phase() on it, or the common clock code could look
up the clock-phases property at the same time it looks up clock-names
and clocks, and set this behind the back of the driver.

	Arnd

^ permalink raw reply	[flat|nested] 28+ messages in thread

end of thread, other threads:[~2013-12-19  5:50 UTC | newest]

Thread overview: 28+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2013-12-16 17:04 [RESEND LIST PATCHv7 0/4] socfpga: Enable SD/MMC support dinguyen
2013-12-16 17:04 ` dinguyen at altera.com
2013-12-16 17:04 ` [RESEND LIST PATCHv7 1/4] clk: socfpga: Add a clk-phase property to the "altr,socfpga-gate-clk" dinguyen
2013-12-16 17:04   ` [RESEND LIST PATCHv7 1/4] clk: socfpga: Add a clk-phase property to the "altr, socfpga-gate-clk" dinguyen at altera.com
2013-12-17  7:46   ` [RESEND LIST PATCHv7 1/4] clk: socfpga: Add a clk-phase property to the "altr,socfpga-gate-clk" zhangfei
2013-12-17  7:46     ` zhangfei
2013-12-17 13:44     ` Dinh Nguyen
2013-12-17 13:44       ` Dinh Nguyen
2013-12-17 14:47       ` zhangfei
2013-12-17 14:47         ` zhangfei
2013-12-17 23:55       ` [RESEND LIST PATCHv7 1/4] clk: socfpga: Add a clk-phase property to the "altr, socfpga-gate-clk" Mike Turquette
2013-12-17 23:55         ` Mike Turquette
2013-12-18  4:06         ` Dinh Nguyen
2013-12-18  4:06           ` Dinh Nguyen
2013-12-18 20:56   ` Mike Turquette
2013-12-18 20:56     ` Mike Turquette
2013-12-18 21:21     ` Arnd Bergmann
2013-12-18 21:21       ` Arnd Bergmann
2013-12-19  4:04       ` Dinh Nguyen
2013-12-19  4:04         ` Dinh Nguyen
2013-12-19  5:50         ` Arnd Bergmann
2013-12-19  5:50           ` Arnd Bergmann
2013-12-16 17:04 ` [RESEND LIST PATCHv7 2/4] dts: socfpga: Add support for SD/MMC on the SOCFPGA platform dinguyen
2013-12-16 17:04   ` dinguyen at altera.com
2013-12-16 17:04 ` [RESEND LIST PATCHv7 3/4] mmc: dw_mmc-socfpga: Remove the SOCFPGA specific platform for dw_mmc dinguyen
2013-12-16 17:04   ` dinguyen at altera.com
2013-12-16 17:04 ` [RESEND LIST PATCHv7 4/4] ARM: socfpga_defconfig: enable SD/MMC support dinguyen
2013-12-16 17:04   ` dinguyen at altera.com

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.