[PATCH 0/3] Add support for SD/MMC on SOCFPGA

[PATCH 0/3] Add support for SD/MMC on SOCFPGA

[dinguyen]

From: Dinh Nguyen <dinguyen@altera.com>

Hi,

Here is a patch series that enables SD/MMC support for the SOCFPGA platform.
This is the 6th revision of the series. This series is intended for v3.13.

v6:
- Reuse "samsung,dw-mshc-sdr-timing" binding
- Clean up dw_mmc-socfpga.c to have only the necessary functions
	- Remove dw_mci_socfpga_parse_dt(), as we'll let the platform function:
		socfpga_sysmgr_set_dwmmc_drvsel_smpsel() do that work
	- Always use SDMMC_CMD_USE_HOLD_REG
- Add mach-socfpga/system_mgr.c, which contains function that will set the
  SD/MMC SDR values in the System Manager.
- Expose an API function in the mach-socfpga platform code so that the SD/MMC
  driver can use to set the SDR values.

I know that there's a general rule not to have drivers access specific
function(s) in platform code, but for this case I am really unsure of what
would be the best solution. The system manager IP block itself does not warrant
a driver, so a "syscon" driver should suffice. However, there are specific
register offsets that are needed, these can be defined in either in the SD/MMC 
or DTS entry. But in order for the DTS entries to be correct, it would need register 
offset information as well. And I was informed that register offsets were frowned 
upon in the past.
   
So for this go-around, I am exposing an API from the mach-socfpga platform
code that will only be used by a platform-specific implementation of the SD/MMC
driver.

v5:
- Add "altr,ciu-clk-offset" that represents the necessary offset
	and shift values in the sysmgr phandle. This is used to set
	the correct CIU clock values.
    
v4:
- Add a complete binding example in documentations
- Add a phandle entry for "altr,sysmgr" which links the system
	manager to the SD/MMC IP block that controls the SDR timings.
- Split up patches
	1/3 - Add syscon binding to sys-mgr node
	2/3 - DTS bindings and documentation for SD/MMC on SOCFPGA
	3/3 - Driver changes to use the bindings
    
v3:
- Explicitly reference synopsis-dw-mshc.txt for base bindings
- Remove "dw-mshc-ciu-div" as driver can get clock information dts "ciu" entry
- Fixed indentation issue
    
v2:
- Remove bus-width and extra line in documentation
- Merge bindings example into a single node in documentation


Dinh Nguyen (3):
  arm: socfpga: Set the SDMMC clock phase in system manager
  mmc: dw_mmc-socfpga: Clean up SOCFPGA platform specific functionality
  arm: dts: socfpga: Add support for SD/MMC

 .../devicetree/bindings/mmc/socfpga-dw-mshc.txt    |   38 ++++++++++++
 arch/arm/boot/dts/socfpga.dtsi                     |   11 ++++
 arch/arm/boot/dts/socfpga_cyclone5.dts             |   12 ++++
 arch/arm/boot/dts/socfpga_vt.dts                   |   12 ++++
 arch/arm/mach-socfpga/Makefile                     |    2 +-
 arch/arm/mach-socfpga/core.h                       |    6 ++
 arch/arm/mach-socfpga/system_mgr.c                 |   32 ++++++++++
 drivers/mmc/host/dw_mmc-pltfm.h                    |    2 +-
 drivers/mmc/host/dw_mmc-socfpga.c                  |   63 +-------------------
 9 files changed, 115 insertions(+), 63 deletions(-)
 create mode 100644 Documentation/devicetree/bindings/mmc/socfpga-dw-mshc.txt
 create mode 100644 arch/arm/mach-socfpga/system_mgr.c

-- 
1.7.9.5

[PATCH 1/3] arm: socfpga: Set the SDMMC clock phase in system manager

[dinguyen]

From: Dinh Nguyen <dinguyen@altera.com>

Add functionality in the System Manager to set the SDR settings for the
SD/MMC IP.

Signed-off-by: Dinh Nguyen <dinguyen@altera.com>
Cc: Pavel Machek <pavel@denx.de>
CC: Arnd Bergmann <arnd@arndb.de>
CC: Olof Johansson <olof@lixom.net>
Cc: Rob Herring <rob.herring@calxeda.com>
Cc: Pawel Moll <pawel.moll@arm.com>
Cc: Mark Rutland <mark.rutland@arm.com>
Cc: Stephen Warren <swarren@wwwdotorg.org>
Cc: Ian Campbell <ian.campbell@citrix.com>
Cc: Chris Ball <cjb@laptop.org>
Cc: Jaehoon Chung <jh80.chung@samsung.com>
Cc: Seungwon Jeon <tgih.jun@samsung.com>
Cc: devicetree@vger.kernel.org
Cc: linux-mmc@vger.kernel.org
CC: linux-arm-kernel@lists.infradead.org
---
 arch/arm/mach-socfpga/Makefile     |    2 +-
 arch/arm/mach-socfpga/core.h       |    6 ++++++
 arch/arm/mach-socfpga/system_mgr.c |   32 ++++++++++++++++++++++++++++++++
 3 files changed, 39 insertions(+), 1 deletion(-)
 create mode 100644 arch/arm/mach-socfpga/system_mgr.c

diff --git a/arch/arm/mach-socfpga/Makefile b/arch/arm/mach-socfpga/Makefile
index 6dd7a93..e4ff8b9 100644
--- a/arch/arm/mach-socfpga/Makefile
+++ b/arch/arm/mach-socfpga/Makefile
@@ -2,5 +2,5 @@
 # Makefile for the linux kernel.
 #
 
-obj-y					:= socfpga.o
+obj-y					:= socfpga.o system_mgr.o
 obj-$(CONFIG_SMP)	+= headsmp.o platsmp.o
diff --git a/arch/arm/mach-socfpga/core.h b/arch/arm/mach-socfpga/core.h
index 572b8f7..b05fa6a 100644
--- a/arch/arm/mach-socfpga/core.h
+++ b/arch/arm/mach-socfpga/core.h
@@ -44,4 +44,10 @@ extern unsigned long cpu1start_addr;
 
 #define SOCFPGA_SCU_VIRT_BASE   0xfffec000
 
+/* SDMMC Group for System Manager defines */
+#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))
+
 #endif
diff --git a/arch/arm/mach-socfpga/system_mgr.c b/arch/arm/mach-socfpga/system_mgr.c
new file mode 100644
index 0000000..c69a854
--- /dev/null
+++ b/arch/arm/mach-socfpga/system_mgr.c
@@ -0,0 +1,32 @@
+/*
+ * Copyright Altera Corporation (C) 2013. All Rights Reserved.
+ *
+ * This program is free software; you can redistribute it and/or modify it
+ * under the terms and conditions of the GNU General Public License,
+ * version 2, as published by the Free Software Foundation.
+ *
+ * This program is distributed in the hope it will be useful, but WITHOUT
+ * ANY WARRANTY; without even the implied warranty of MERCHANTABILITY or
+ * FITNESS FOR A PARTICULAR PURPOSE.  See the GNU General Public License for
+ * more details.
+ *
+ * You should have received a copy of the GNU General Public License along with
+ * this program.  If not, see <http://www.gnu.org/licenses/>.
+ */
+#include <linux/of_platform.h>
+#include <asm/mach/map.h>
+
+#include "core.h"
+
+void socfpga_sysmgr_set_dwmmc_drvsel_smpsel(void)
+{
+	struct device_node *np;
+	u32 timing[2];
+	u32 hs_timing;
+
+	np = of_find_compatible_node(NULL, NULL, "altr,socfpga-dw-mshc");
+	of_property_read_u32_array(np, "samsung,dw-mshc-sdr-timing", timing, 2);
+	hs_timing = SYSMGR_SDMMC_CTRL_SET(timing[0], timing[1]);
+	writel(hs_timing, sys_manager_base_addr + SYSMGR_SDMMCGRP_CTRL_OFFSET);
+}
+EXPORT_SYMBOL(socfpga_sysmgr_set_dwmmc_drvsel_smpsel);
-- 
1.7.9.5

[PATCH 2/3] mmc: dw_mmc-socfpga: Clean up SOCFPGA platform specific functionality

[dinguyen]

From: Dinh Nguyen <dinguyen@altera.com>

The SDR timing registers for the SD/MMC IP block for SOCFPGA is located
in the system manager. This system manager IP block is located outside of
the SD IP block itself. Therefore, the function to set the SDR timing
register should be in the platform specific code so that the SD driver can
be autonomous of any future System Manager changes.

Also, there is no need for "altr,dw-mshc-ciu-div" as the driver can get
the value of the CIU clock from the common clock API.

Signed-off-by: Dinh Nguyen <dinguyen@altera.com>
Cc: Pavel Machek <pavel@denx.de>
CC: Arnd Bergmann <arnd@arndb.de>
CC: Olof Johansson <olof@lixom.net>
Cc: Rob Herring <rob.herring@calxeda.com>
Cc: Pawel Moll <pawel.moll@arm.com>
Cc: Mark Rutland <mark.rutland@arm.com>
Cc: Stephen Warren <swarren@wwwdotorg.org>
Cc: Ian Campbell <ian.campbell@citrix.com>
Cc: Chris Ball <cjb@laptop.org>
Cc: Jaehoon Chung <jh80.chung@samsung.com>
Cc: Seungwon Jeon <tgih.jun@samsung.com>
Cc: devicetree@vger.kernel.org
Cc: linux-mmc@vger.kernel.org
CC: linux-arm-kernel@lists.infradead.org
---
 drivers/mmc/host/dw_mmc-pltfm.h   |    2 +-
 drivers/mmc/host/dw_mmc-socfpga.c |   63 ++-----------------------------------
 2 files changed, 3 insertions(+), 62 deletions(-)

diff --git a/drivers/mmc/host/dw_mmc-pltfm.h b/drivers/mmc/host/dw_mmc-pltfm.h
index 68e7fd2..682400f 100644
--- a/drivers/mmc/host/dw_mmc-pltfm.h
+++ b/drivers/mmc/host/dw_mmc-pltfm.h
@@ -16,5 +16,5 @@ extern int dw_mci_pltfm_register(struct platform_device *pdev,
 				const struct dw_mci_drv_data *drv_data);
 extern int dw_mci_pltfm_remove(struct platform_device *pdev);
 extern const struct dev_pm_ops dw_mci_pltfm_pmops;
-
+extern void socfpga_sysmgr_set_dwmmc_drvsel_smpsel(void);
 #endif /* _DW_MMC_PLTFM_H_ */
diff --git a/drivers/mmc/host/dw_mmc-socfpga.c b/drivers/mmc/host/dw_mmc-socfpga.c
index 14b5961..924a950 100644
--- a/drivers/mmc/host/dw_mmc-socfpga.c
+++ b/drivers/mmc/host/dw_mmc-socfpga.c
@@ -24,48 +24,14 @@
 #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)
-{
-	struct dw_mci_socfpga_priv_data *priv;
-
-	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);
-	}
-	host->priv = priv;
-
-	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);
+	socfpga_sysmgr_set_dwmmc_drvsel_smpsel();
 	clk_prepare_enable(host->ciu_clk);
 
-	host->bus_hz /= (priv->ciu_div + 1);
 	return 0;
 }
 
@@ -73,37 +39,12 @@ 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 = 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)
-		return ret;
-
-	priv->hs_timing = SYSMGR_SDMMC_CTRL_SET(timing[0], timing[1]);
-	return 0;
+	*cmdr |= SDMMC_CMD_USE_HOLD_REG;
 }
 
 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[] = {
-- 
1.7.9.5

[PATCH 3/3] arm: dts: socfpga: Add support for SD/MMC

[dinguyen]

From: Dinh Nguyen <dinguyen@altera.com>

Add bindings for SD/MMC for SOCFPGA.

Signed-off-by: Dinh Nguyen <dinguyen@altera.com>
Cc: Pavel Machek <pavel@denx.de>
CC: Arnd Bergmann <arnd@arndb.de>
CC: Olof Johansson <olof@lixom.net>
Cc: Rob Herring <rob.herring@calxeda.com>
Cc: Pawel Moll <pawel.moll@arm.com>
Cc: Mark Rutland <mark.rutland@arm.com>
Cc: Stephen Warren <swarren@wwwdotorg.org>
Cc: Ian Campbell <ian.campbell@citrix.com>
Cc: Chris Ball <cjb@laptop.org>
Cc: Jaehoon Chung <jh80.chung@samsung.com>
Cc: Seungwon Jeon <tgih.jun@samsung.com>
Cc: devicetree@vger.kernel.org
Cc: linux-mmc@vger.kernel.org
CC: linux-arm-kernel@lists.infradead.org
---
 .../devicetree/bindings/mmc/socfpga-dw-mshc.txt    |   38 ++++++++++++++++++++
 arch/arm/boot/dts/socfpga.dtsi                     |   11 ++++++
 arch/arm/boot/dts/socfpga_cyclone5.dts             |   12 +++++++
 arch/arm/boot/dts/socfpga_vt.dts                   |   12 +++++++
 4 files changed, 73 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/mmc/socfpga-dw-mshc.txt

diff --git a/Documentation/devicetree/bindings/mmc/socfpga-dw-mshc.txt b/Documentation/devicetree/bindings/mmc/socfpga-dw-mshc.txt
new file mode 100644
index 0000000..f565835
--- /dev/null
+++ b/Documentation/devicetree/bindings/mmc/socfpga-dw-mshc.txt
@@ -0,0 +1,38 @@
+* 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.
+
+* samsung,dw-mshc-sdr-timing: See exynos-dw-mshc.txt for more information about
+	this binding.
+
+Example:
+        dwmmc0@ff704000 {
+                compatible = "altr,socfpga-dw-mshc", "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";
+                num-slots = <1>;
+                supports-highspeed;
+                broken-cd;
+                samsung,dw-mshc-sdr-timing = <0 3>;
+
+                slot@0 {
+                        reg = <0>;
+                        bus-width = <4>;
+                };
+        };
diff --git a/arch/arm/boot/dts/socfpga.dtsi b/arch/arm/boot/dts/socfpga.dtsi
index bee62a2..25ad850 100644
--- a/arch/arm/boot/dts/socfpga.dtsi
+++ b/arch/arm/boot/dts/socfpga.dtsi
@@ -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";
diff --git a/arch/arm/boot/dts/socfpga_cyclone5.dts b/arch/arm/boot/dts/socfpga_cyclone5.dts
index 973999d..b4b68ad 100644
--- a/arch/arm/boot/dts/socfpga_cyclone5.dts
+++ b/arch/arm/boot/dts/socfpga_cyclone5.dts
@@ -48,6 +48,18 @@
 			};
 		};
 
+		dwmmc0@ff704000 {
+			num-slots = <1>;
+			supports-highspeed;
+			broken-cd;
+			samsung,dw-mshc-sdr-timing = <0 3>;
+
+			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..7dc709b 100644
--- a/arch/arm/boot/dts/socfpga_vt.dts
+++ b/arch/arm/boot/dts/socfpga_vt.dts
@@ -41,6 +41,18 @@
 			};
 		};
 
+		dwmmc0@ff704000 {
+			num-slots = <1>;
+			supports-highspeed;
+			broken-cd;
+			samsung,dw-mshc-sdr-timing = <0 3>;
+
+			slot@0 {
+				reg = <0>;
+				bus-width = <4>;
+			};
+		};
+
 		ethernet@ff700000 {
 			phy-mode = "gmii";
 			status = "okay";
-- 
1.7.9.5

Re: [PATCH 3/3] arm: dts: socfpga: Add support for SD/MMC

[Stephen Warren]

On 09/09/2013 09:33 AM, dinguyen@altera.com wrote:
> From: Dinh Nguyen <dinguyen@altera.com>
> 
> Add bindings for SD/MMC for SOCFPGA.

> diff --git a/Documentation/devicetree/bindings/mmc/socfpga-dw-mshc.txt b/Documentation/devicetree/bindings/mmc/socfpga-dw-mshc.txt

> +* 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.
> +
> +* samsung,dw-mshc-sdr-timing: See exynos-dw-mshc.txt for more information about
> +	this binding.

s/binding/property/

It's odd that this isn't "synopsis," rather than "samsung," if the
property is generic across all/some uses of the Synopsis core. But, I
guess this is fine.

Re: [PATCH 1/3] arm: socfpga: Set the SDMMC clock phase in system manager

[Pavel Machek]

Hi!

> From: Dinh Nguyen <dinguyen@altera.com>
> 
> Add functionality in the System Manager to set the SDR settings for the
> SD/MMC IP.
> 
> Signed-off-by: Dinh Nguyen <dinguyen@altera.com>
> Cc: Pavel Machek <pavel@denx.de>

> +void socfpga_sysmgr_set_dwmmc_drvsel_smpsel(void)
> +{
> +	struct device_node *np;
> +	u32 timing[2];
> +	u32 hs_timing;
> +
> +	np = of_find_compatible_node(NULL, NULL, "altr,socfpga-dw-mshc");
> +	of_property_read_u32_array(np, "samsung,dw-mshc-sdr-timing", timing, 2);
> +	hs_timing = SYSMGR_SDMMC_CTRL_SET(timing[0], timing[1]);
> +	writel(hs_timing, sys_manager_base_addr + SYSMGR_SDMMCGRP_CTRL_OFFSET);
> +}
> +EXPORT_SYMBOL(socfpga_sysmgr_set_dwmmc_drvsel_smpsel);

To get the abstraction right, would it make sense to have timing
parameters as arguments to socfpga_sysmgr_set_dwmmc_drvsel_smpsel(),
so that sysmgr code is not walking MMC's device tree directly?

Thanks,
									Pavel

-- 
(english) http://www.livejournal.com/~pavelmachek
(cesky, pictures) http://atrey.karlin.mff.cuni.cz/~pavel/picture/horses/blog.html

Re: [PATCH 3/3] arm: dts: socfpga: Add support for SD/MMC

[Pavel Machek]

On Mon 2013-09-09 10:33:53, dinguyen-EIB2kfCEclfQT0dZR+AlfA@public.gmane.org wrote:
> From: Dinh Nguyen <dinguyen-EIB2kfCEclfQT0dZR+AlfA@public.gmane.org>
> 
> Add bindings for SD/MMC for SOCFPGA.
> 
> Signed-off-by: Dinh Nguyen <dinguyen-EIB2kfCEclfQT0dZR+AlfA@public.gmane.org>
> Cc: Pavel Machek <pavel-ynQEQJNshbs@public.gmane.org>

Reviewed-by: Pavel Machek <pavel-ynQEQJNshbs@public.gmane.org>

-- 
(english) http://www.livejournal.com/~pavelmachek
(cesky, pictures) http://atrey.karlin.mff.cuni.cz/~pavel/picture/horses/blog.html
--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH 3/3] arm: dts: socfpga: Add support for SD/MMC

[Tomasz Figa]

On Monday 09 of September 2013 15:11:43 Stephen Warren wrote:
> On 09/09/2013 09:33 AM, dinguyen-EIB2kfCEclfQT0dZR+AlfA@public.gmane.org wrote:
> > From: Dinh Nguyen <dinguyen-EIB2kfCEclfQT0dZR+AlfA@public.gmane.org>
> > 
> > Add bindings for SD/MMC for SOCFPGA.
> > 
> > diff --git a/Documentation/devicetree/bindings/mmc/socfpga-dw-mshc.txt
> > b/Documentation/devicetree/bindings/mmc/socfpga-dw-mshc.txt
> > 
> > +* 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.
> > +
> > +* samsung,dw-mshc-sdr-timing: See exynos-dw-mshc.txt for more
> > information about +	this binding.
> 
> s/binding/property/
> 
> It's odd that this isn't "synopsis," rather than "samsung," if the
> property is generic across all/some uses of the Synopsis core. But, I
> guess this is fine.

Just as a side note, correct name is Synopsys, not Synopsis. There are
multiple places around Documentation/devicetree where this typo is
present[1]. Should we consider correcting this or the typo will have
to stay?

[1] git grep -i synopsis Documentation/devicetree/
Documentation/devicetree/bindings/mmc/exynos-dw-mshc.txt:* Samsung Exynos specific extensions to the Synopsis Designware Mobile
Documentation/devicetree/bindings/mmc/exynos-dw-mshc.txt:The Synopsis designware mobile storage host controller is used to interface
Documentation/devicetree/bindings/mmc/exynos-dw-mshc.txt:differences between the core Synopsis dw mshc controller properties described
Documentation/devicetree/bindings/mmc/exynos-dw-mshc.txt:by synopsis-dw-mshc.txt and the properties used by the Samsung Exynos specific
Documentation/devicetree/bindings/mmc/exynos-dw-mshc.txt:extensions to the Synopsis Designware Mobile Storage Host Controller.
Documentation/devicetree/bindings/mmc/rockchip-dw-mshc.txt:* Rockchip specific extensions to the Synopsis Designware Mobile
Documentation/devicetree/bindings/mmc/rockchip-dw-mshc.txt:The Synopsis designware mobile storage host controller is used to interface
Documentation/devicetree/bindings/mmc/rockchip-dw-mshc.txt:differences between the core Synopsis dw mshc controller properties described
Documentation/devicetree/bindings/mmc/rockchip-dw-mshc.txt:by synopsis-dw-mshc.txt and the properties used by the Rockchip specific
Documentation/devicetree/bindings/mmc/rockchip-dw-mshc.txt:extensions to the Synopsis Designware Mobile Storage Host Controller.
Documentation/devicetree/bindings/mmc/synopsis-dw-mshc.txt:* Synopsis Designware Mobile Storage Host Controller
Documentation/devicetree/bindings/mmc/synopsis-dw-mshc.txt:The Synopsis designware mobile storage host controller is used to interface
Documentation/devicetree/bindings/mmc/synopsis-dw-mshc.txt:properties used by the Synopsis Designware Mobile Storage Host Controller.
Documentation/devicetree/bindings/mmc/synopsis-dw-mshc.txt:     - snps,dw-mshc: for controllers compliant with synopsis dw-mshc.
Documentation/devicetree/bindings/pci/designware-pcie.txt:* Synopsis Designware PCIe interface

Best regards,
Tomasz

--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH 3/3] arm: dts: socfpga: Add support for SD/MMC

[Stephen Warren]

On 09/14/2013 06:30 AM, Tomasz Figa wrote:
...
> Just as a side note, correct name is Synopsys, not Synopsis. There are
> multiple places around Documentation/devicetree where this typo is
> present[1]. Should we consider correcting this or the typo will have
> to stay?
> 
> [1] git grep -i synopsis Documentation/devicetree/
> Documentation/devicetree/bindings/mmc/exynos-dw-mshc.txt:* Samsung Exynos specific extensions to the Synopsis Designware Mobile
> Documentation/devicetree/bindings/mmc/exynos-dw-mshc.txt:The Synopsis designware mobile storage host controller is used to interface
> Documentation/devicetree/bindings/mmc/exynos-dw-mshc.txt:differences between the core Synopsis dw mshc controller properties described
> Documentation/devicetree/bindings/mmc/exynos-dw-mshc.txt:by synopsis-dw-mshc.txt and the properties used by the Samsung Exynos specific
> Documentation/devicetree/bindings/mmc/exynos-dw-mshc.txt:extensions to the Synopsis Designware Mobile Storage Host Controller.
> Documentation/devicetree/bindings/mmc/rockchip-dw-mshc.txt:* Rockchip specific extensions to the Synopsis Designware Mobile
> Documentation/devicetree/bindings/mmc/rockchip-dw-mshc.txt:The Synopsis designware mobile storage host controller is used to interface
> Documentation/devicetree/bindings/mmc/rockchip-dw-mshc.txt:differences between the core Synopsis dw mshc controller properties described
> Documentation/devicetree/bindings/mmc/rockchip-dw-mshc.txt:by synopsis-dw-mshc.txt and the properties used by the Rockchip specific
> Documentation/devicetree/bindings/mmc/rockchip-dw-mshc.txt:extensions to the Synopsis Designware Mobile Storage Host Controller.
> Documentation/devicetree/bindings/mmc/synopsis-dw-mshc.txt:* Synopsis Designware Mobile Storage Host Controller
> Documentation/devicetree/bindings/mmc/synopsis-dw-mshc.txt:The Synopsis designware mobile storage host controller is used to interface
> Documentation/devicetree/bindings/mmc/synopsis-dw-mshc.txt:properties used by the Synopsis Designware Mobile Storage Host Controller.
> Documentation/devicetree/bindings/mmc/synopsis-dw-mshc.txt:     - snps,dw-mshc: for controllers compliant with synopsis dw-mshc.
> Documentation/devicetree/bindings/pci/designware-pcie.txt:* Synopsis Designware PCIe interface

Those typos only appear to exist in free-form text and not in any
property names/values. As such, they can all easily be fixed without
impacting any DT content or ABI.
--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH 3/3] arm: dts: socfpga: Add support for SD/MMC

[Dinh Nguyen]

On Mon, 2013-09-16 at 10:36 -0600, Stephen Warren wrote:
> On 09/14/2013 06:30 AM, Tomasz Figa wrote:
> ...
> > Just as a side note, correct name is Synopsys, not Synopsis. There are
> > multiple places around Documentation/devicetree where this typo is
> > present[1]. Should we consider correcting this or the typo will have
> > to stay?

I can send a patch to fix the typos.

Dinh
> > 
> > [1] git grep -i synopsis Documentation/devicetree/
> > Documentation/devicetree/bindings/mmc/exynos-dw-mshc.txt:* Samsung Exynos specific extensions to the Synopsis Designware Mobile
> > Documentation/devicetree/bindings/mmc/exynos-dw-mshc.txt:The Synopsis designware mobile storage host controller is used to interface
> > Documentation/devicetree/bindings/mmc/exynos-dw-mshc.txt:differences between the core Synopsis dw mshc controller properties described
> > Documentation/devicetree/bindings/mmc/exynos-dw-mshc.txt:by synopsis-dw-mshc.txt and the properties used by the Samsung Exynos specific
> > Documentation/devicetree/bindings/mmc/exynos-dw-mshc.txt:extensions to the Synopsis Designware Mobile Storage Host Controller.
> > Documentation/devicetree/bindings/mmc/rockchip-dw-mshc.txt:* Rockchip specific extensions to the Synopsis Designware Mobile
> > Documentation/devicetree/bindings/mmc/rockchip-dw-mshc.txt:The Synopsis designware mobile storage host controller is used to interface
> > Documentation/devicetree/bindings/mmc/rockchip-dw-mshc.txt:differences between the core Synopsis dw mshc controller properties described
> > Documentation/devicetree/bindings/mmc/rockchip-dw-mshc.txt:by synopsis-dw-mshc.txt and the properties used by the Rockchip specific
> > Documentation/devicetree/bindings/mmc/rockchip-dw-mshc.txt:extensions to the Synopsis Designware Mobile Storage Host Controller.
> > Documentation/devicetree/bindings/mmc/synopsis-dw-mshc.txt:* Synopsis Designware Mobile Storage Host Controller
> > Documentation/devicetree/bindings/mmc/synopsis-dw-mshc.txt:The Synopsis designware mobile storage host controller is used to interface
> > Documentation/devicetree/bindings/mmc/synopsis-dw-mshc.txt:properties used by the Synopsis Designware Mobile Storage Host Controller.
> > Documentation/devicetree/bindings/mmc/synopsis-dw-mshc.txt:     - snps,dw-mshc: for controllers compliant with synopsis dw-mshc.
> > Documentation/devicetree/bindings/pci/designware-pcie.txt:* Synopsis Designware PCIe interface
> 
> Those typos only appear to exist in free-form text and not in any
> property names/values. As such, they can all easily be fixed without
> impacting any DT content or ABI.
> 



--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH 1/3] arm: socfpga: Set the SDMMC clock phase in system manager

[Chris Ball]

Hi Dinh,

On Sat, Sep 14 2013, Pavel Machek wrote:
>> From: Dinh Nguyen <dinguyen@altera.com>
>> 
>> Add functionality in the System Manager to set the SDR settings for the
>> SD/MMC IP.
>> 
>> Signed-off-by: Dinh Nguyen <dinguyen@altera.com>
>> Cc: Pavel Machek <pavel@denx.de>
>
>> +void socfpga_sysmgr_set_dwmmc_drvsel_smpsel(void)
>> +{
>> +	struct device_node *np;
>> +	u32 timing[2];
>> +	u32 hs_timing;
>> +
>> +	np = of_find_compatible_node(NULL, NULL, "altr,socfpga-dw-mshc");
>> +	of_property_read_u32_array(np, "samsung,dw-mshc-sdr-timing", timing, 2);
>> +	hs_timing = SYSMGR_SDMMC_CTRL_SET(timing[0], timing[1]);
>> +	writel(hs_timing, sys_manager_base_addr + SYSMGR_SDMMCGRP_CTRL_OFFSET);
>> +}
>> +EXPORT_SYMBOL(socfpga_sysmgr_set_dwmmc_drvsel_smpsel);
>
> To get the abstraction right, would it make sense to have timing
> parameters as arguments to socfpga_sysmgr_set_dwmmc_drvsel_smpsel(),
> so that sysmgr code is not walking MMC's device tree directly?

I think this review comment from Pavel is still open, please reply.
Thanks,

- Chris.
-- 
Chris Ball   <cjb@laptop.org>   <http://printf.net/>

Re: [PATCH 1/3] arm: socfpga: Set the SDMMC clock phase in system manager

[Dinh Nguyen]

Hi Chris,

On Sep 25, 2013, at 8:50 PM, Chris Ball wrote:

> Hi Dinh,
> 
> On Sat, Sep 14 2013, Pavel Machek wrote:
>>> From: Dinh Nguyen <dinguyen-EIB2kfCEclfQT0dZR+AlfA@public.gmane.org>
>>> 
>>> Add functionality in the System Manager to set the SDR settings for the
>>> SD/MMC IP.
>>> 
>>> Signed-off-by: Dinh Nguyen <dinguyen-EIB2kfCEclfQT0dZR+AlfA@public.gmane.org>
>>> Cc: Pavel Machek <pavel-ynQEQJNshbs@public.gmane.org>
>> 
>>> +void socfpga_sysmgr_set_dwmmc_drvsel_smpsel(void)
>>> +{
>>> +	struct device_node *np;
>>> +	u32 timing[2];
>>> +	u32 hs_timing;
>>> +
>>> +	np = of_find_compatible_node(NULL, NULL, "altr,socfpga-dw-mshc");
>>> +	of_property_read_u32_array(np, "samsung,dw-mshc-sdr-timing", timing, 2);
>>> +	hs_timing = SYSMGR_SDMMC_CTRL_SET(timing[0], timing[1]);
>>> +	writel(hs_timing, sys_manager_base_addr + SYSMGR_SDMMCGRP_CTRL_OFFSET);
>>> +}
>>> +EXPORT_SYMBOL(socfpga_sysmgr_set_dwmmc_drvsel_smpsel);
>> 
>> To get the abstraction right, would it make sense to have timing
>> parameters as arguments to socfpga_sysmgr_set_dwmmc_drvsel_smpsel(),
>> so that sysmgr code is not walking MMC's device tree directly?
> 
> I think this review comment from Pavel is still open, please reply.

I sent a Rev 2 that addresses this comment on 9/23. Let me know if I need to resend it in case you missed it.

Thanks,
Dinh
> Thanks,
> 
> - Chris.
> -- 
> Chris Ball   <cjb-2X9k7bc8m7Mdnm+yROfE0A@public.gmane.org>   <http://printf.net/>

--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html