v3:
- Explicitly reference synopsis-dw-mshc.txt for base bindings
- Remove "dw-mshc-ciu-div" as driver can get clock information dts from "ciu" entry.
- Fixed indentation issue

v2:
- Remove bus-width and extra line in documentation
- Merge bindings example into a single node in documentation

---
Re-send to updated device tree bindings maintainers.

---
 .../devicetree/bindings/mmc/socfpga-dw-mshc.txt    |   36 ++++++++++++++++++++
 arch/arm/boot/dts/socfpga.dtsi                     |   13 ++++++-
 arch/arm/boot/dts/socfpga_cyclone5.dts             |   12 +++++++
 arch/arm/boot/dts/socfpga_vt.dts                   |   11 ++++++
 drivers/mmc/host/dw_mmc-socfpga.c                  |    9 -----
 5 files changed, 71 insertions(+), 10 deletions(-)
 create mode 100644 Documentation/devicetree/bindings/mmc/socfpga-dw-mshc.txt

Documentation/devicetree/bindings/mmc/socfpga-dw-mshc.txt new file +36 -0

new file mode 100644
index 0000000..d588bf9
@@ -0,0 +1,36 @@
+* Altera SOCFPGA specific extensions to the Synopsis Designware Mobile
+  Storage Host Controller
+
+The Synopsis designware mobile storage host controller is used to interface
+a SoC with storage medium such as eMMC or SD/MMC cards. This file documents
+differences between the core Synopsis dw mshc controller properties described
+by synopsis-dw-mshc.txt and the properties used by the SOCFPGA specific
+extensions to the Synopsis Designware Mobile Storage Host Controller.
+
+Required Properties:
+
+* compatible: should be
+	- "altr,socfpga-dw-mshc": for controllers with Altera SOCFPGA
+	  specific extensions.
+
+* altr,dw-mshc-sdr-timing: Specifies the value of CIU clock phase shift value
+  in transmit mode and CIU clock phase shift value in receive mode for single
+  data rate mode operation. Refer to notes below for the order of the cells and the
+  valid values.
+
+  Notes for the sdr-timing values:
+
+    The order of the cells should be
+      - First Cell: CIU clock phase shift value for RX mode, smplsel bits in
+	the system manager SDMMC control group.
+      - Second Cell: CIU clock phase shift value for TX mode, drvsel bits in
+	the system manager SDMMC control group.
+
+    Valid values for SDR CIU clock timing for SOCFPGA:
+      - valid value for tx phase shift and rx phase shift is 0 to 7.
+
+Example:
+	dwmmc0@ff704000 {
+		compatible = "altr,socfpga-dw-mshc", "snps,dw-mshc";
+		altr,dw-mshc-sdr-timing = <0 3>;
+	};

arch/arm/boot/dts/socfpga.dtsi +12 -1

index bee62a2..dbf7f22 100644
@@ -468,6 +468,17 @@
 			cache-level = <2>;
 		};
 
+		mmc: dwmmc0@ff704000 {
+			compatible = "altr,socfpga-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";

@@ -521,7 +532,7 @@
 			};
 
 		sysmgr@ffd08000 {
-				compatible = "altr,sys-mgr";
+				compatible = "altr,sys-mgr", "syscon";
 				reg = <0xffd08000 0x4000>;
 			};
 	};

arch/arm/boot/dts/socfpga_cyclone5.dts +12 -0

index 973999d..526df6f 100644
@@ -54,6 +54,18 @@
 			status = "okay";
 		};
 
+		dwmmc0@ff704000 {
+			num-slots = <1>;
+			supports-highspeed;
+			broken-cd;
+			altr,dw-mshc-sdr-timing = <0 3>;
+
+			slot@0 {
+				reg = <0>;
+				bus-width = <4>;
+			};
+		};
+
 		timer0@ffc08000 {
 			clock-frequency = <100000000>;
 		};

arch/arm/boot/dts/socfpga_vt.dts +11 -0

index d1ec0ca..6f23121 100644
@@ -46,6 +46,17 @@
 			status = "okay";
 		};
 
+		dwmmc0@ff704000 {
+			num-slots = <1>;
+			supports-highspeed;
+			broken-cd;
+
+			slot@0 {
+				reg = <0>;
+				bus-width = <4>;
+			};
+		};
+
 		timer0@ffc08000 {
 			clock-frequency = <7000000>;
 		};

drivers/mmc/host/dw_mmc-socfpga.c +0 -9

index 14b5961..0cff75d 100644
@@ -31,7 +31,6 @@
 
 /* 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 */
 };

@@ -64,8 +63,6 @@ static int dw_mci_socfpga_setup_clock(struct dw_mci *host)
 	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;
 }

@@ -82,14 +79,8 @@ static int dw_mci_socfpga_parse_dt(struct dw_mci *host)
 	struct dw_mci_socfpga_priv_data *priv = host->priv;
 	struct device_node *np = host->dev->of_node;
 	u32 timing[2];
-	u32 div = 0;
 	int ret;
 
-	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)

Signature · 1 line

1.7.9.5

Dinh Nguyen

author

2013-08-13 19:58 UTC #2 lore raw

PATCH v3 1/1 ARM: socfpga: dts: Add support for SD/MMC

Quoted 11 lines · Stephen Warren

Documentation/devicetree/bindings/mmc/socfpga-dw-mshc.txt
Quoted 6 lines · dinguyen at altera.com
From: Dinh Nguyen <[email protected]>
Add bindings for SD/MMC for SOCFPGA.
Add "syscon" to the "altr,sys-mgr" binding.
Documentation/devicetree/bindings/mmc/socfpga-dw-mshc.txt +0 -0
I think it's reasonable to define one binding as being based on another plus some additions, so, the binding,
Acked-by Stephen Warren <[email protected]>

Thanks Stephen!

Quoted 9 lines

Documentation/devicetree/bindings/mmc/socfpga-dw-mshc.txt line 36
Quoted 6 lines
Documentation/devicetree/bindings/mmc/socfpga-dw-mshc.txt +5 -0
+Example:
+	dwmmc0 at ff704000 {
+		compatible = "altr,socfpga-dw-mshc", "snps,dw-mshc";
+		altr,dw-mshc-sdr-timing = <0 3>;
+	};
It'd be nice to provide a complete example though, rather than only including the properties that this binding adds to the base binding.

I'll add a complete binding.

Quoted 8 lines

arch/arm/boot/dts/socfpga.dtsi line 535
Quoted 6 lines
arch/arm/boot/dts/socfpga.dtsi +1 -1
 
 		sysmgr at ffd08000 {
-				compatible = "altr,sys-mgr";
+				compatible = "altr,sys-mgr", "syscon";
That seems like an unrelated change?

This patch is to enable SD/MMC for SOCFPGA. The "syscon" is needed in dw_mmc-socfpga.c as the clock phases for the SD IP is controlled by registers in "altr,sys-mgr".

Thanks,

Dinh

Quoted 1 line

Stephen Warren

2013-08-13 20:10 UTC #3 lore raw

PATCH v3 1/1 ARM: socfpga: dts: Add support for SD/MMC

Quoted 15 lines · Dinh Nguyen

Quoted 6 lines · Stephen Warren
Quoted 4 lines · dinguyen at altera.com
From: Dinh Nguyen <[email protected]>
Add bindings for SD/MMC for SOCFPGA.
Add "syscon" to the "altr,sys-mgr" binding.
Quoted 7 lines
arch/arm/boot/dts/socfpga.dtsi line 535
Quoted 5 lines
arch/arm/boot/dts/socfpga.dtsi +1 -1
 
 		sysmgr at ffd08000 {
-				compatible = "altr,sys-mgr";
+				compatible = "altr,sys-mgr", "syscon";
That seems like an unrelated change?

Quoted 3 lines

This patch is to enable SD/MMC for SOCFPGA. The "syscon" is needed in dw_mmc-socfpga.c as the clock phases for the SD IP is controlled by registers in "altr,sys-mgr".

But nothing in the dwmmc nodes refers to the syscon node, so how can the syscon driver possibly provide services to the MMC driver; the MMC driver won't be able to find it. I hope there's no back-door assumption that some syscon exists, and that there's only one of them...

Dinh Nguyen

author

2013-08-13 20:17 UTC #4 lore raw

PATCH v3 1/1 ARM: socfpga: dts: Add support for SD/MMC

Quoted 16 lines · Stephen Warren

Quoted 15 lines · Dinh Nguyen
Quoted 13 lines · Stephen Warren
arch/arm/boot/dts/socfpga.dtsi line 535
Quoted 10 lines · dinguyen at altera.com
From: Dinh Nguyen <[email protected]>
Add bindings for SD/MMC for SOCFPGA.
Add "syscon" to the "altr,sys-mgr" binding.
arch/arm/boot/dts/socfpga.dtsi +1 -1
 
 		sysmgr at ffd08000 {
-				compatible = "altr,sys-mgr";
+				compatible = "altr,sys-mgr", "syscon";
That seems like an unrelated change?

Quoted 8 lines

Quoted 3 lines
This patch is to enable SD/MMC for SOCFPGA. The "syscon" is needed in dw_mmc-socfpga.c as the clock phases for the SD IP is controlled by registers in "altr,sys-mgr".
But nothing in the dwmmc nodes refers to the syscon node, so how can the syscon driver possibly provide services to the MMC driver; the MMC driver won't be able to find it. I hope there's no back-door assumption that some syscon exists, and that there's only one of them...

In dw_mmc-socfpga.c:

priv->sysreg = syscon_regmap_lookup_by_compatible("altr,sys-mgr");

then, uses regmap_write() to set the correct values from
"altr,dw-mshc-sdr-timing".

This was a suggestion from Arnd. It seems that this part of patch is causing alot of confusion, I should just split the patches out?

Dinh

Quoted 1 line

Stephen Warren

2013-08-13 20:24 UTC #5 lore raw

PATCH v3 1/1 ARM: socfpga: dts: Add support for SD/MMC

Quoted 26 lines · Dinh Nguyen

Quoted 16 lines · Stephen Warren
Quoted 15 lines · Dinh Nguyen
Quoted 13 lines · Stephen Warren
arch/arm/boot/dts/socfpga.dtsi line 535
Quoted 10 lines · dinguyen at altera.com
From: Dinh Nguyen <[email protected]>
Add bindings for SD/MMC for SOCFPGA.
Add "syscon" to the "altr,sys-mgr" binding.
arch/arm/boot/dts/socfpga.dtsi +1 -1
 
 		sysmgr at ffd08000 {
-				compatible = "altr,sys-mgr";
+				compatible = "altr,sys-mgr", "syscon";
That seems like an unrelated change?
Quoted 8 lines
Quoted 3 lines
This patch is to enable SD/MMC for SOCFPGA. The "syscon" is needed in dw_mmc-socfpga.c as the clock phases for the SD IP is controlled by registers in "altr,sys-mgr".
But nothing in the dwmmc nodes refers to the syscon node, so how can the syscon driver possibly provide services to the MMC driver; the MMC driver won't be able to find it. I hope there's no back-door assumption that some syscon exists, and that there's only one of them...

Quoted 6 lines

In dw_mmc-socfpga.c:
priv->sysreg = syscon_regmap_lookup_by_compatible("altr,sys-mgr");
then, uses regmap_write() to set the correct values from
"altr,dw-mshc-sdr-timing".

Uggh. There should really be a phandle from the MMC node to the syscon node to specify the connectivity. What if there ends up being 2 HW modules that are sys-mgr?

At the very least, the binding for any device that's affected by the sys-mgr should specify that some other node must exist with that compatible value, to provide XXX services for it.

But, I wonder if the altr,dw-mshc-sdr-timing property shouldn't be part of the syscon node itself?

(Probably best to ignore my ack for now given this new information)

Quoted 2 lines

This was a suggestion from Arnd. It seems that this part of patch is causing alot of confusion, I should just split the patches out?

A separate patch to change that compatible value probably would make sense; by the sound of it, that change would also be required for a lot of other devices on the SoC; it's not just specific to MMC?