[v2 PATCH 0/2] mmc: dw_mmc: add dw_mmc-k3

[v2 PATCH 0/2] mmc: dw_mmc: add dw_mmc-k3

[Zhangfei Gao]

v2:
Follow Jaehoon's suggestion
Use slot-gpio.c handle cd pin
Move table out to dts
other suggestion

Zhangfei Gao (2):
  mmc: dw_mmc: add dw_mci_of_get_cd_gpio to handle cd pin
  mmc: dw_mmc: add dw_mmc-k3 for k3 platform

 .../devicetree/bindings/mmc/k3-dw-mshc.txt         |   77 +++++
 drivers/mmc/host/Kconfig                           |   10 +
 drivers/mmc/host/Makefile                          |    1 +
 drivers/mmc/host/dw_mmc-k3.c                       |  298 ++++++++++++++++++++
 drivers/mmc/host/dw_mmc.c                          |   34 +++
 5 files changed, 420 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/mmc/k3-dw-mshc.txt
 create mode 100644 drivers/mmc/host/dw_mmc-k3.c

-- 
1.7.9.5

[PATCH 1/2] mmc: dw_mmc: add dw_mci_of_get_cd_gpio to handle cd pin

[Zhangfei Gao]

Add function dw_mci_of_get_cd_gpio to check "cd-gpios" from dts.
mmc_gpio_request_cd and mmc_gpio_get_cd are used to handle cd pin

Signed-off-by: Zhangfei Gao <zhangfei.gao@linaro.org>
CC: Jaehoon Chung <jh80.chung@samsung.com>
---
 drivers/mmc/host/dw_mmc.c |   34 ++++++++++++++++++++++++++++++++++
 1 file changed, 34 insertions(+)

diff --git a/drivers/mmc/host/dw_mmc.c b/drivers/mmc/host/dw_mmc.c
index 018f365..b8bb9a5 100644
--- a/drivers/mmc/host/dw_mmc.c
+++ b/drivers/mmc/host/dw_mmc.c
@@ -35,6 +35,7 @@
 #include <linux/workqueue.h>
 #include <linux/of.h>
 #include <linux/of_gpio.h>
+#include <linux/mmc/slot-gpio.h>
 
 #include "dw_mmc.h"
 
@@ -872,12 +873,15 @@ static int dw_mci_get_cd(struct mmc_host *mmc)
 	int present;
 	struct dw_mci_slot *slot = mmc_priv(mmc);
 	struct dw_mci_board *brd = slot->host->pdata;
+	int gpio_cd = !mmc_gpio_get_cd(mmc);
 
 	/* Use platform get_cd function, else try onboard card detect */
 	if (brd->quirks & DW_MCI_QUIRK_BROKEN_CARD_DETECTION)
 		present = 1;
 	else if (brd->get_cd)
 		present = !brd->get_cd(slot->id);
+	else if (!IS_ERR_VALUE(gpio_cd))
+		present = !!gpio_cd;
 	else
 		present = (mci_readl(slot->host, CDETECT) & (1 << slot->id))
 			== 0 ? 1 : 0;
@@ -1876,6 +1880,30 @@ static int dw_mci_of_get_wp_gpio(struct device *dev, u8 slot)
 
 	return gpio;
 }
+
+/* find the cd gpio for a given slot; or -1 if none specified */
+static int dw_mci_of_get_cd_gpio(struct device *dev, u8 slot,
+					struct mmc_host *mmc)
+{
+	struct device_node *np = dw_mci_of_find_slot_node(dev, slot);
+	int gpio;
+
+	if (!np)
+		return -EINVAL;
+
+	gpio = of_get_named_gpio(np, "cd-gpios", 0);
+
+	/* Having a missing entry is valid; return silently */
+	if (!gpio_is_valid(gpio))
+		return -EINVAL;
+
+	if (mmc_gpio_request_cd(mmc, gpio, 0)) {
+		dev_warn(dev, "gpio [%d] request failed\n", gpio);
+		return -EINVAL;
+	}
+
+	return gpio;
+}
 #else /* CONFIG_OF */
 static int dw_mci_of_get_slot_quirks(struct device *dev, u8 slot)
 {
@@ -1893,6 +1921,11 @@ static int dw_mci_of_get_wp_gpio(struct device *dev, u8 slot)
 {
 	return -EINVAL;
 }
+static int dw_mci_of_get_cd_gpio(struct device *dev, u8 slot,
+					struct mmc_host *mmc)
+{
+	return -EINVAL;
+}
 #endif /* CONFIG_OF */
 
 static int dw_mci_init_slot(struct dw_mci *host, unsigned int id)
@@ -1996,6 +2029,7 @@ static int dw_mci_init_slot(struct dw_mci *host, unsigned int id)
 		clear_bit(DW_MMC_CARD_PRESENT, &slot->flags);
 
 	slot->wp_gpio = dw_mci_of_get_wp_gpio(host->dev, slot->id);
+	dw_mci_of_get_cd_gpio(host->dev, slot->id, mmc);
 
 	ret = mmc_add_host(mmc);
 	if (ret)
-- 
1.7.9.5

[PATCH 2/2] mmc: dw_mmc: add dw_mmc-k3 for k3 platform

[Zhangfei Gao]

Add dw_mmc-k3.c for k3v2, support sd/emmc

Signed-off-by: Zhangfei Gao <zhangfei.gao-QSEj5FYQhm4dnm+yROfE0A@public.gmane.org>
Tested-by: Zhigang Wang <brooke.wangzhigang-hv44wF8Li93QT0dZR+AlfA@public.gmane.org>
---
 .../devicetree/bindings/mmc/k3-dw-mshc.txt         |   77 +++++
 drivers/mmc/host/Kconfig                           |   10 +
 drivers/mmc/host/Makefile                          |    1 +
 drivers/mmc/host/dw_mmc-k3.c                       |  298 ++++++++++++++++++++
 4 files changed, 386 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/mmc/k3-dw-mshc.txt
 create mode 100644 drivers/mmc/host/dw_mmc-k3.c

diff --git a/Documentation/devicetree/bindings/mmc/k3-dw-mshc.txt b/Documentation/devicetree/bindings/mmc/k3-dw-mshc.txt
new file mode 100644
index 0000000..0de8c27
--- /dev/null
+++ b/Documentation/devicetree/bindings/mmc/k3-dw-mshc.txt
@@ -0,0 +1,77 @@
+* Hisilicon specific extensions to the Synopsys Designware Mobile
+  Storage Host Controller
+
+Read synopsis-dw-mshc.txt for more details
+The Synopsys 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 Synopsys dw mshc controller properties described
+by synopsis-dw-mshc.txt and the properties used by the Hisilicon specific
+extensions to the Synopsys Designware Mobile Storage Host Controller.
+
+Required Properties:
+
+* compatible: should be
+	- "hisilicon,hi4511-dw-mshc": for controllers with hi4511
+	  specific extentions.
+* vmmc-supply: should be vmmc used in dwmmc
+* fifo-depth: should be provided if register can not provide correct value
+* clken-reg: should be clock enable register and offset
+* drv-sel-reg: should be driver delay select register, offset and bits
+* sam-sel-reg: should be sample delay select register, offset and bits
+* div-reg: should be divider register, offset and bits
+* tune-table: should be array of clock tune mmc controller
+
+Example:
+
+  The MSHC controller node can be split into two portions, SoC specific and
+  board specific portions as listed below.
+
+	dwmmc_0: dwmmc0@fcd03000 {
+		compatible = "hisilicon,hi4511-dw-mshc";
+		reg = <0xfcd03000 0x1000>;
+		interrupts = <0 16 4>;
+		#address-cells = <1>;
+		#size-cells = <0>;
+		clocks = <&clk_sd>, <&clk_ddrc_per>;
+		clock-names = "ciu", "biu";
+		clken-reg = <0x1f8 0>;
+		drv-sel-reg = <0x1f8 4 4>;
+		sam-sel-reg = <0x1f8 8 4>;
+		div-reg = <0x1f8 1 3>;
+		tune-table =
+		<180000000 6 6 13 13 25000000>,
+		<0 0 0 0 0 0>,
+		<360000000 6 4 2 0 50000000>,
+		<180000000 6 4 13 13 25000000>,
+		<360000000 6 4 2 0 50000000>,
+		<720000000 6 1 9 4 100000000>,
+		<0 0 0 0 0 0>,
+		<360000000 7 1 3 0 50000000>;
+	};
+	dwmmc0@fcd03000 {
+		num-slots = <1>;
+		vmmc-supply = <&ldo12>;
+		fifo-depth = <0x100>;
+		supports-highspeed;
+		pinctrl-names = "default";
+		pinctrl-0 = <&sd_pmx_pins &sd_cfg_func1 &sd_cfg_func2>;
+		slot@0 {
+			reg = <0>;
+			bus-width = <4>;
+			disable-wp;
+			cd-gpios = <&gpio10 3 0>;
+		};
+	};
+
+PCTRL:
+
+Required Properties:
+* compatible: should be
+	- "hisilicon,pctrl": Peripheral control
+
+Example:
+
+	pctrl: pctrl@fca09000 {
+		compatible = "hisilicon,pctrl";
+		reg = <0xfca09000 0x1000>;
+	};
diff --git a/drivers/mmc/host/Kconfig b/drivers/mmc/host/Kconfig
index 7fc5099..45aaa2d 100644
--- a/drivers/mmc/host/Kconfig
+++ b/drivers/mmc/host/Kconfig
@@ -575,6 +575,16 @@ config MMC_DW_SOCFPGA
 	  This selects support for Altera SoCFPGA specific extensions to the
 	  Synopsys DesignWare Memory Card Interface driver.
 
+config MMC_DW_K3
+	tristate "K3 specific extensions for Synopsys DW Memory Card Interface"
+	depends on MMC_DW
+	select MMC_DW_PLTFM
+	select MMC_DW_IDMAC
+	help
+	  This selects support for Hisilicon K3 SoC specific extensions to the
+	  Synopsys DesignWare Memory Card Interface driver. Select this option
+	  for platforms based on Hisilicon K3 SoC's.
+
 config MMC_DW_PCI
 	tristate "Synopsys Designware MCI support on PCI bus"
 	depends on MMC_DW && PCI
diff --git a/drivers/mmc/host/Makefile b/drivers/mmc/host/Makefile
index c41d0c3..64f5f8d 100644
--- a/drivers/mmc/host/Makefile
+++ b/drivers/mmc/host/Makefile
@@ -43,6 +43,7 @@ obj-$(CONFIG_MMC_DW)		+= dw_mmc.o
 obj-$(CONFIG_MMC_DW_PLTFM)	+= dw_mmc-pltfm.o
 obj-$(CONFIG_MMC_DW_EXYNOS)	+= dw_mmc-exynos.o
 obj-$(CONFIG_MMC_DW_SOCFPGA)	+= dw_mmc-socfpga.o
+obj-$(CONFIG_MMC_DW_K3)		+= dw_mmc-k3.o
 obj-$(CONFIG_MMC_DW_PCI)	+= dw_mmc-pci.o
 obj-$(CONFIG_MMC_SH_MMCIF)	+= sh_mmcif.o
 obj-$(CONFIG_MMC_JZ4740)	+= jz4740_mmc.o
diff --git a/drivers/mmc/host/dw_mmc-k3.c b/drivers/mmc/host/dw_mmc-k3.c
new file mode 100644
index 0000000..f186b84
--- /dev/null
+++ b/drivers/mmc/host/dw_mmc-k3.c
@@ -0,0 +1,298 @@
+/*
+ * Copyright (c) 2013 Linaro Ltd.
+ * Copyright (c) 2013 Hisilicon Limited.
+ *
+ * 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.
+ */
+
+#include <linux/module.h>
+#include <linux/platform_device.h>
+#include <linux/clk.h>
+#include <linux/mmc/host.h>
+#include <linux/mmc/dw_mmc.h>
+#include <linux/of.h>
+#include <linux/of_gpio.h>
+#include <linux/of_address.h>
+
+#include "dw_mmc.h"
+#include "dw_mmc-pltfm.h"
+
+enum dw_mci_k3_type {
+	DW_MCI_TYPE_HI4511,
+};
+
+static struct dw_mci_k3_compatible {
+	char				*compatible;
+	enum dw_mci_k3_type		type;
+} k3_compat[] = {
+	{
+		.compatible	= "hisilicon,hi4511-dw-mshc",
+		.type		= DW_MCI_TYPE_HI4511,
+	},
+};
+
+#define TABLE_WIDTH 6
+#define TABLE_HEIGHT 8
+struct dw_mci_k3_priv_data {
+	enum dw_mci_k3_type	type;
+	int			old_timing;
+	u32			table[TABLE_WIDTH * TABLE_HEIGHT];
+	u32			clken_reg;
+	u32			clken_bit;
+	u32			sam_sel_reg;
+	u32			sam_sel_off;
+	u32			sam_sel_bits;
+	u32			drv_sel_reg;
+	u32			drv_sel_off;
+	u32			drv_sel_bits;
+	u32			div_reg;
+	u32			div_off;
+	u32			div_bits;
+};
+
+static void __iomem *pctrl;
+static DEFINE_SPINLOCK(mmc_tuning_lock);
+
+static u32 dw_mci_k3_delay(u32 val, u32 para, u32 off, u32 len)
+{
+	u32 i;
+
+	if (para >= 0) {
+		for (i = 0; i < len; i++) {
+			if (para % 2)
+				val |= 1<<(off + i);
+			else
+				val &= ~(1<<(off + i));
+			para = para >> 1;
+		}
+	}
+	return val;
+}
+
+static void dw_mci_k3_set_timing(struct dw_mci_k3_priv_data *priv,
+					u32 sam, u32 drv, u32 div)
+{
+	u32 val;
+	unsigned long flags;
+
+	if (!pctrl)
+		return;
+
+	spin_lock_irqsave(&mmc_tuning_lock, flags);
+
+	/* disable clock */
+	val = readl(pctrl + priv->clken_reg);
+	val &= ~(1 << priv->clken_bit);
+	writel(val, pctrl + priv->clken_reg);
+
+	val = readl(pctrl + priv->sam_sel_reg);
+	val = dw_mci_k3_delay(val, sam, priv->sam_sel_off, priv->sam_sel_bits);
+	writel(val, pctrl + priv->sam_sel_reg);
+
+	val = readl(pctrl + priv->drv_sel_reg);
+	val = dw_mci_k3_delay(val, drv, priv->drv_sel_off, priv->drv_sel_bits);
+	writel(val, pctrl + priv->drv_sel_reg);
+
+	val = readl(pctrl + priv->div_reg);
+	val = dw_mci_k3_delay(val, drv, priv->div_off, priv->div_bits);
+	writel(val, pctrl + priv->div_reg);
+
+	/* enable clock */
+	val = readl(pctrl + priv->clken_reg);
+	val |= 1 << priv->clken_bit;
+	writel(val, pctrl + priv->clken_reg);
+
+	spin_unlock_irqrestore(&mmc_tuning_lock, flags);
+}
+
+static void dw_mci_k3_tun(struct dw_mci *host, int timing)
+{
+	struct dw_mci_k3_priv_data *priv = host->priv;
+	struct device_node *np = host->dev->of_node;
+	int ret;
+	u32 *table = &priv->table[TABLE_WIDTH * timing];
+
+	if (priv->old_timing == timing)
+		return;
+
+	ret = clk_set_rate(host->ciu_clk, table[0]);
+	if (ret) {
+		dev_err(host->dev, "clk_set_rate failed\n");
+		return;
+	}
+	dw_mci_k3_set_timing(priv, (table[3] + table[4]) / 2,
+			table[2], table[1]);
+	host->bus_hz = table[5];
+	priv->old_timing = timing;
+}
+
+static void dw_mci_k3_set_ios(struct dw_mci *host, struct mmc_ios *ios)
+{
+	struct dw_mci_k3_priv_data *priv = host->priv;
+
+	if (priv->type == DW_MCI_TYPE_HI4511)
+		dw_mci_k3_tun(host, ios->timing);
+}
+
+static int dw_mci_k3_priv_init(struct dw_mci *host)
+{
+	struct dw_mci_k3_priv_data *priv;
+	int i, 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->old_timing = -1;
+	host->priv = priv;
+
+	for (i = 0; i < ARRAY_SIZE(k3_compat); i++) {
+		if (of_device_is_compatible(host->dev->of_node,
+					k3_compat[i].compatible))
+			priv->type = k3_compat[i].type;
+	}
+
+	if (priv->type == DW_MCI_TYPE_HI4511) {
+		ret = of_property_read_u32_array(host->dev->of_node,
+			"tune-table", priv->table, TABLE_WIDTH * TABLE_HEIGHT);
+		if (ret)
+			return -EINVAL;
+
+		if (!pctrl) {
+			struct device_node *node;
+
+			node = of_find_compatible_node(NULL, NULL,
+						"hisilicon,pctrl");
+			pctrl = of_iomap(node, 0);
+		}
+	}
+
+	return 0;
+}
+
+static int dw_mci_k3_setup_clock(struct dw_mci *host)
+{
+	struct dw_mci_k3_priv_data *priv = host->priv;
+
+	if (priv->type == DW_MCI_TYPE_HI4511)
+		dw_mci_k3_tun(host, MMC_TIMING_LEGACY);
+
+	return 0;
+}
+
+static int dw_mci_k3_parse_dt(struct dw_mci *host)
+{
+	struct dw_mci_k3_priv_data *priv = host->priv;
+	struct device_node *np = host->dev->of_node;
+	u32 data[3];
+	int ret;
+
+	if (priv->type == DW_MCI_TYPE_HI4511) {
+		ret = of_property_read_u32_array(np,
+				"clken-reg", data, 2);
+		if (!ret) {
+			priv->clken_reg = data[0];
+			priv->clken_bit = data[1];
+		}
+
+		ret = of_property_read_u32_array(np,
+				"drv-sel-reg", data, 3);
+		if (!ret) {
+			priv->drv_sel_reg = data[0];
+			priv->drv_sel_off = data[1];
+			priv->drv_sel_bits = data[2];
+		}
+
+		ret = of_property_read_u32_array(np,
+				"sam-sel-reg", data, 3);
+		if (!ret) {
+			priv->sam_sel_reg = data[0];
+			priv->sam_sel_off = data[1];
+			priv->sam_sel_bits = data[2];
+		}
+
+		ret = of_property_read_u32_array(np,
+				"div-reg", data, 3);
+		if (!ret) {
+			priv->div_reg = data[0];
+			priv->div_off = data[1];
+			priv->div_bits = data[2];
+		}
+	}
+
+	return 0;
+}
+
+static unsigned long k3_dwmmc_caps[4] = {
+	MMC_CAP_4_BIT_DATA | MMC_CAP_SD_HIGHSPEED,
+	MMC_CAP_8_BIT_DATA | MMC_CAP_MMC_HIGHSPEED,
+	0,
+	0,
+};
+
+static const struct dw_mci_drv_data k3_drv_data = {
+	.caps			= k3_dwmmc_caps,
+	.init			= dw_mci_k3_priv_init,
+	.set_ios		= dw_mci_k3_set_ios,
+	.setup_clock		= dw_mci_k3_setup_clock,
+	.parse_dt		= dw_mci_k3_parse_dt,
+};
+
+static const struct of_device_id dw_mci_k3_match[] = {
+	{ .compatible = "hisilicon,hi4511-dw-mshc",
+			.data = &k3_drv_data, },
+	{},
+};
+MODULE_DEVICE_TABLE(of, dw_mci_k3_match);
+
+static int dw_mci_k3_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_k3_match, pdev->dev.of_node);
+	drv_data = match->data;
+
+	return dw_mci_pltfm_register(pdev, drv_data);
+}
+
+static int dw_mci_k3_suspend(struct device *dev)
+{
+	struct dw_mci *host = dev_get_drvdata(dev);
+
+	return dw_mci_suspend(host);
+}
+
+static int dw_mci_k3_resume(struct device *dev)
+{
+	struct dw_mci *host = dev_get_drvdata(dev);
+	struct dw_mci_k3_priv_data *priv = host->priv;
+
+	if (priv->type == DW_MCI_TYPE_HI4511) {
+		priv->old_timing = -1;
+		dw_mci_k3_tun(host, MMC_TIMING_LEGACY);
+	}
+
+	return dw_mci_resume(host);
+}
+
+SIMPLE_DEV_PM_OPS(dw_mci_k3_pmops, dw_mci_k3_suspend, dw_mci_k3_resume);
+
+static struct platform_driver dw_mci_k3_pltfm_driver = {
+	.probe		= dw_mci_k3_probe,
+	.remove		= dw_mci_pltfm_remove,
+	.driver		= {
+		.name		= "dwmmc_k3",
+		.of_match_table	= dw_mci_k3_match,
+		.pm		= &dw_mci_k3_pmops,
+	},
+};
+
+module_platform_driver(dw_mci_k3_pltfm_driver);
+
+MODULE_DESCRIPTION("K3 Specific DW-MSHC Driver Extension");
+MODULE_LICENSE("GPL v2");
-- 
1.7.9.5

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

[PATCH 2/2] mmc: dw_mmc: add dw_mmc-k3 for k3 platform

[Zhangfei Gao]

update: fix typo

Add dw_mmc-k3.c for k3v2, support sd/emmc

Signed-off-by: Zhangfei Gao <zhangfei.gao@linaro.org>
Tested-by: Zhigang Wang <brooke.wangzhigang@huawei.com>
---
 .../devicetree/bindings/mmc/k3-dw-mshc.txt         |   77 +++++
 drivers/mmc/host/Kconfig                           |   10 +
 drivers/mmc/host/Makefile                          |    1 +
 drivers/mmc/host/dw_mmc-k3.c                       |  297 ++++++++++++++++++++
 4 files changed, 385 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/mmc/k3-dw-mshc.txt
 create mode 100644 drivers/mmc/host/dw_mmc-k3.c

diff --git a/Documentation/devicetree/bindings/mmc/k3-dw-mshc.txt b/Documentation/devicetree/bindings/mmc/k3-dw-mshc.txt
new file mode 100644
index 0000000..0de8c27
--- /dev/null
+++ b/Documentation/devicetree/bindings/mmc/k3-dw-mshc.txt
@@ -0,0 +1,77 @@
+* Hisilicon specific extensions to the Synopsys Designware Mobile
+  Storage Host Controller
+
+Read synopsis-dw-mshc.txt for more details
+The Synopsys 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 Synopsys dw mshc controller properties described
+by synopsis-dw-mshc.txt and the properties used by the Hisilicon specific
+extensions to the Synopsys Designware Mobile Storage Host Controller.
+
+Required Properties:
+
+* compatible: should be
+	- "hisilicon,hi4511-dw-mshc": for controllers with hi4511
+	  specific extentions.
+* vmmc-supply: should be vmmc used in dwmmc
+* fifo-depth: should be provided if register can not provide correct value
+* clken-reg: should be clock enable register and offset
+* drv-sel-reg: should be driver delay select register, offset and bits
+* sam-sel-reg: should be sample delay select register, offset and bits
+* div-reg: should be divider register, offset and bits
+* tune-table: should be array of clock tune mmc controller
+
+Example:
+
+  The MSHC controller node can be split into two portions, SoC specific and
+  board specific portions as listed below.
+
+	dwmmc_0: dwmmc0@fcd03000 {
+		compatible = "hisilicon,hi4511-dw-mshc";
+		reg = <0xfcd03000 0x1000>;
+		interrupts = <0 16 4>;
+		#address-cells = <1>;
+		#size-cells = <0>;
+		clocks = <&clk_sd>, <&clk_ddrc_per>;
+		clock-names = "ciu", "biu";
+		clken-reg = <0x1f8 0>;
+		drv-sel-reg = <0x1f8 4 4>;
+		sam-sel-reg = <0x1f8 8 4>;
+		div-reg = <0x1f8 1 3>;
+		tune-table =
+		<180000000 6 6 13 13 25000000>,
+		<0 0 0 0 0 0>,
+		<360000000 6 4 2 0 50000000>,
+		<180000000 6 4 13 13 25000000>,
+		<360000000 6 4 2 0 50000000>,
+		<720000000 6 1 9 4 100000000>,
+		<0 0 0 0 0 0>,
+		<360000000 7 1 3 0 50000000>;
+	};
+	dwmmc0@fcd03000 {
+		num-slots = <1>;
+		vmmc-supply = <&ldo12>;
+		fifo-depth = <0x100>;
+		supports-highspeed;
+		pinctrl-names = "default";
+		pinctrl-0 = <&sd_pmx_pins &sd_cfg_func1 &sd_cfg_func2>;
+		slot@0 {
+			reg = <0>;
+			bus-width = <4>;
+			disable-wp;
+			cd-gpios = <&gpio10 3 0>;
+		};
+	};
+
+PCTRL:
+
+Required Properties:
+* compatible: should be
+	- "hisilicon,pctrl": Peripheral control
+
+Example:
+
+	pctrl: pctrl@fca09000 {
+		compatible = "hisilicon,pctrl";
+		reg = <0xfca09000 0x1000>;
+	};
diff --git a/drivers/mmc/host/Kconfig b/drivers/mmc/host/Kconfig
index 7fc5099..45aaa2d 100644
--- a/drivers/mmc/host/Kconfig
+++ b/drivers/mmc/host/Kconfig
@@ -575,6 +575,16 @@ config MMC_DW_SOCFPGA
 	  This selects support for Altera SoCFPGA specific extensions to the
 	  Synopsys DesignWare Memory Card Interface driver.
 
+config MMC_DW_K3
+	tristate "K3 specific extensions for Synopsys DW Memory Card Interface"
+	depends on MMC_DW
+	select MMC_DW_PLTFM
+	select MMC_DW_IDMAC
+	help
+	  This selects support for Hisilicon K3 SoC specific extensions to the
+	  Synopsys DesignWare Memory Card Interface driver. Select this option
+	  for platforms based on Hisilicon K3 SoC's.
+
 config MMC_DW_PCI
 	tristate "Synopsys Designware MCI support on PCI bus"
 	depends on MMC_DW && PCI
diff --git a/drivers/mmc/host/Makefile b/drivers/mmc/host/Makefile
index c41d0c3..64f5f8d 100644
--- a/drivers/mmc/host/Makefile
+++ b/drivers/mmc/host/Makefile
@@ -43,6 +43,7 @@ obj-$(CONFIG_MMC_DW)		+= dw_mmc.o
 obj-$(CONFIG_MMC_DW_PLTFM)	+= dw_mmc-pltfm.o
 obj-$(CONFIG_MMC_DW_EXYNOS)	+= dw_mmc-exynos.o
 obj-$(CONFIG_MMC_DW_SOCFPGA)	+= dw_mmc-socfpga.o
+obj-$(CONFIG_MMC_DW_K3)		+= dw_mmc-k3.o
 obj-$(CONFIG_MMC_DW_PCI)	+= dw_mmc-pci.o
 obj-$(CONFIG_MMC_SH_MMCIF)	+= sh_mmcif.o
 obj-$(CONFIG_MMC_JZ4740)	+= jz4740_mmc.o
diff --git a/drivers/mmc/host/dw_mmc-k3.c b/drivers/mmc/host/dw_mmc-k3.c
new file mode 100644
index 0000000..0ff75328
--- /dev/null
+++ b/drivers/mmc/host/dw_mmc-k3.c
@@ -0,0 +1,297 @@
+/*
+ * Copyright (c) 2013 Linaro Ltd.
+ * Copyright (c) 2013 Hisilicon Limited.
+ *
+ * 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.
+ */
+
+#include <linux/module.h>
+#include <linux/platform_device.h>
+#include <linux/clk.h>
+#include <linux/mmc/host.h>
+#include <linux/mmc/dw_mmc.h>
+#include <linux/of_address.h>
+
+#include "dw_mmc.h"
+#include "dw_mmc-pltfm.h"
+
+enum dw_mci_k3_type {
+	DW_MCI_TYPE_HI4511,
+};
+
+static struct dw_mci_k3_compatible {
+	char				*compatible;
+	enum dw_mci_k3_type		type;
+} k3_compat[] = {
+	{
+		.compatible	= "hisilicon,hi4511-dw-mshc",
+		.type		= DW_MCI_TYPE_HI4511,
+	},
+};
+
+#define TABLE_WIDTH 6
+#define TABLE_HEIGHT 8
+struct dw_mci_k3_priv_data {
+	enum dw_mci_k3_type	type;
+	int			old_timing;
+	u32			table[TABLE_WIDTH * TABLE_HEIGHT];
+	u32			clken_reg;
+	u32			clken_bit;
+	u32			sam_reg;
+	u32			sam_off;
+	u32			sam_bits;
+	u32			drv_reg;
+	u32			drv_off;
+	u32			drv_bits;
+	u32			div_reg;
+	u32			div_off;
+	u32			div_bits;
+};
+
+static void __iomem *pctrl;
+static DEFINE_SPINLOCK(mmc_tuning_lock);
+
+static u32 dw_mci_k3_delay(u32 val, u32 para, u32 off, u32 len)
+{
+	u32 i;
+
+	if (para >= 0) {
+		for (i = 0; i < len; i++) {
+			if (para % 2)
+				val |= 1<<(off + i);
+			else
+				val &= ~(1<<(off + i));
+			para = para >> 1;
+		}
+	}
+	return val;
+}
+
+static void dw_mci_k3_set_timing(struct dw_mci_k3_priv_data *priv,
+					u32 sam, u32 drv, u32 div)
+{
+	u32 val;
+	unsigned long flags;
+
+	if (!pctrl || !priv->clken_reg || !priv->sam_reg
+		|| !priv->drv_reg || !priv->div_reg)
+		return;
+
+	spin_lock_irqsave(&mmc_tuning_lock, flags);
+
+	val = readl(pctrl + priv->clken_reg);
+	val &= ~(1 << priv->clken_bit);
+	writel(val, pctrl + priv->clken_reg);
+
+	val = readl(pctrl + priv->sam_reg);
+	val = dw_mci_k3_delay(val, sam, priv->sam_off, priv->sam_bits);
+	writel(val, pctrl + priv->sam_reg);
+
+	val = readl(pctrl + priv->drv_reg);
+	val = dw_mci_k3_delay(val, drv, priv->drv_off, priv->drv_bits);
+	writel(val, pctrl + priv->drv_reg);
+
+	val = readl(pctrl + priv->div_reg);
+	val = dw_mci_k3_delay(val, div, priv->div_off, priv->div_bits);
+	writel(val, pctrl + priv->div_reg);
+
+	val = readl(pctrl + priv->clken_reg);
+	val |= 1 << priv->clken_bit;
+	writel(val, pctrl + priv->clken_reg);
+
+	spin_unlock_irqrestore(&mmc_tuning_lock, flags);
+}
+
+static void dw_mci_k3_tun(struct dw_mci *host, int timing)
+{
+	struct dw_mci_k3_priv_data *priv = host->priv;
+	int ret;
+	u32 *table = &priv->table[TABLE_WIDTH * timing];
+
+	if (priv->old_timing == timing)
+		return;
+
+	ret = clk_set_rate(host->ciu_clk, table[0]);
+	if (ret) {
+		dev_err(host->dev, "clk_set_rate failed\n");
+		return;
+	}
+	dw_mci_k3_set_timing(priv, (table[3] + table[4]) / 2,
+			table[2], table[1]);
+	host->bus_hz = table[5];
+	priv->old_timing = timing;
+}
+
+static void dw_mci_k3_set_ios(struct dw_mci *host, struct mmc_ios *ios)
+{
+	struct dw_mci_k3_priv_data *priv = host->priv;
+
+	if (priv->type == DW_MCI_TYPE_HI4511)
+		dw_mci_k3_tun(host, ios->timing);
+}
+
+static int dw_mci_k3_priv_init(struct dw_mci *host)
+{
+	struct dw_mci_k3_priv_data *priv;
+	int i, 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->old_timing = -1;
+	host->priv = priv;
+
+	for (i = 0; i < ARRAY_SIZE(k3_compat); i++) {
+		if (of_device_is_compatible(host->dev->of_node,
+					k3_compat[i].compatible))
+			priv->type = k3_compat[i].type;
+	}
+
+	if (priv->type == DW_MCI_TYPE_HI4511) {
+		ret = of_property_read_u32_array(host->dev->of_node,
+			"tune-table", priv->table, TABLE_WIDTH * TABLE_HEIGHT);
+		if (ret) {
+			dev_err(host->dev, "not found tune-table\n");
+			return -EINVAL;
+		}
+
+		if (!pctrl) {
+			struct device_node *node;
+
+			node = of_find_compatible_node(NULL, NULL,
+						"hisilicon,pctrl");
+			pctrl = of_iomap(node, 0);
+		}
+	}
+
+	return 0;
+}
+
+static int dw_mci_k3_setup_clock(struct dw_mci *host)
+{
+	struct dw_mci_k3_priv_data *priv = host->priv;
+
+	if (priv->type == DW_MCI_TYPE_HI4511)
+		dw_mci_k3_tun(host, MMC_TIMING_LEGACY);
+
+	return 0;
+}
+
+static int dw_mci_k3_parse_dt(struct dw_mci *host)
+{
+	struct dw_mci_k3_priv_data *priv = host->priv;
+	struct device_node *np = host->dev->of_node;
+	u32 data[3];
+	int ret;
+
+	if (priv->type == DW_MCI_TYPE_HI4511) {
+		ret = of_property_read_u32_array(np,
+				"clken-reg", data, 2);
+		if (!ret) {
+			priv->clken_reg = data[0];
+			priv->clken_bit = data[1];
+		}
+
+		ret = of_property_read_u32_array(np,
+				"drv-sel-reg", data, 3);
+		if (!ret) {
+			priv->drv_reg = data[0];
+			priv->drv_off = data[1];
+			priv->drv_bits = data[2];
+		}
+
+		ret = of_property_read_u32_array(np,
+				"sam-sel-reg", data, 3);
+		if (!ret) {
+			priv->sam_reg = data[0];
+			priv->sam_off = data[1];
+			priv->sam_bits = data[2];
+		}
+
+		ret = of_property_read_u32_array(np,
+				"div-reg", data, 3);
+		if (!ret) {
+			priv->div_reg = data[0];
+			priv->div_off = data[1];
+			priv->div_bits = data[2];
+		}
+	}
+
+	return 0;
+}
+
+static unsigned long k3_dwmmc_caps[4] = {
+	MMC_CAP_4_BIT_DATA | MMC_CAP_SD_HIGHSPEED,
+	MMC_CAP_8_BIT_DATA | MMC_CAP_MMC_HIGHSPEED,
+	0,
+	0,
+};
+
+static const struct dw_mci_drv_data k3_drv_data = {
+	.caps			= k3_dwmmc_caps,
+	.init			= dw_mci_k3_priv_init,
+	.set_ios		= dw_mci_k3_set_ios,
+	.setup_clock		= dw_mci_k3_setup_clock,
+	.parse_dt		= dw_mci_k3_parse_dt,
+};
+
+static const struct of_device_id dw_mci_k3_match[] = {
+	{ .compatible = "hisilicon,hi4511-dw-mshc",
+			.data = &k3_drv_data, },
+	{},
+};
+MODULE_DEVICE_TABLE(of, dw_mci_k3_match);
+
+static int dw_mci_k3_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_k3_match, pdev->dev.of_node);
+	drv_data = match->data;
+
+	return dw_mci_pltfm_register(pdev, drv_data);
+}
+
+static int dw_mci_k3_suspend(struct device *dev)
+{
+	struct dw_mci *host = dev_get_drvdata(dev);
+
+	return dw_mci_suspend(host);
+}
+
+static int dw_mci_k3_resume(struct device *dev)
+{
+	struct dw_mci *host = dev_get_drvdata(dev);
+	struct dw_mci_k3_priv_data *priv = host->priv;
+
+	if (priv->type == DW_MCI_TYPE_HI4511) {
+		priv->old_timing = -1;
+		dw_mci_k3_tun(host, MMC_TIMING_LEGACY);
+	}
+
+	return dw_mci_resume(host);
+}
+
+SIMPLE_DEV_PM_OPS(dw_mci_k3_pmops, dw_mci_k3_suspend, dw_mci_k3_resume);
+
+static struct platform_driver dw_mci_k3_pltfm_driver = {
+	.probe		= dw_mci_k3_probe,
+	.remove		= dw_mci_pltfm_remove,
+	.driver		= {
+		.name		= "dwmmc_k3",
+		.of_match_table	= dw_mci_k3_match,
+		.pm		= &dw_mci_k3_pmops,
+	},
+};
+
+module_platform_driver(dw_mci_k3_pltfm_driver);
+
+MODULE_DESCRIPTION("K3 Specific DW-MSHC Driver Extension");
+MODULE_LICENSE("GPL v2");
+MODULE_ALIAS("platform:dwmmc-k3");
-- 
1.7.9.5

Re: [PATCH 2/2] mmc: dw_mmc: add dw_mmc-k3 for k3 platform

[Chris Ball]

Hi Zhangfei, just small changes,

On Wed, Oct 23 2013, Zhangfei Gao wrote:
> +* tune-table: should be array of clock tune mmc controller

This sounds like it should be "tuning-table", and shouldn't it also
have a prefix?

> +			node = of_find_compatible_node(NULL, NULL,
> +						"hisilicon,pctrl");

No need to use two lines here.

> +		ret = of_property_read_u32_array(np,
> +				"clken-reg", data, 2);

Or here, same with the other of_property_read_u32_array() calls.

> +MODULE_DESCRIPTION("K3 Specific DW-MSHC Driver Extension");
> +MODULE_LICENSE("GPL v2");
> +MODULE_ALIAS("platform:dwmmc-k3");

Can you add a MODULE_AUTHOR too?

Thanks,

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

Re: [PATCH 2/2] mmc: dw_mmc: add dw_mmc-k3 for k3 platform

[Kumar Gala]

On Oct 23, 2013, at 8:03 AM, Zhangfei Gao wrote:

> update: fix typo
> 
> Add dw_mmc-k3.c for k3v2, support sd/emmc
> 
> Signed-off-by: Zhangfei Gao <zhangfei.gao@linaro.org>
> Tested-by: Zhigang Wang <brooke.wangzhigang@huawei.com>
> ---
> .../devicetree/bindings/mmc/k3-dw-mshc.txt         |   77 +++++
> drivers/mmc/host/Kconfig                           |   10 +
> drivers/mmc/host/Makefile                          |    1 +
> drivers/mmc/host/dw_mmc-k3.c                       |  297 ++++++++++++++++++++
> 4 files changed, 385 insertions(+)
> create mode 100644 Documentation/devicetree/bindings/mmc/k3-dw-mshc.txt
> create mode 100644 drivers/mmc/host/dw_mmc-k3.c
> 
> diff --git a/Documentation/devicetree/bindings/mmc/k3-dw-mshc.txt b/Documentation/devicetree/bindings/mmc/k3-dw-mshc.txt
> new file mode 100644
> index 0000000..0de8c27
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/mmc/k3-dw-mshc.txt
> @@ -0,0 +1,77 @@
> +* Hisilicon specific extensions to the Synopsys Designware Mobile
> +  Storage Host Controller
> +
> +Read synopsis-dw-mshc.txt for more details
> +The Synopsys 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 Synopsys dw mshc controller properties described
> +by synopsis-dw-mshc.txt and the properties used by the Hisilicon specific
> +extensions to the Synopsys Designware Mobile Storage Host Controller.
> +
> +Required Properties:
> +
> +* compatible: should be
> +	- "hisilicon,hi4511-dw-mshc": for controllers with hi4511
> +	  specific extentions.
> +* vmmc-supply: should be vmmc used in dwmmc
> +* fifo-depth: should be provided if register can not provide correct value
> +* clken-reg: should be clock enable register and offset
> +* drv-sel-reg: should be driver delay select register, offset and bits
> +* sam-sel-reg: should be sample delay select register, offset and bits
> +* div-reg: should be divider register, offset and bits
> +* tune-table: should be array of clock tune mmc controller
> +

1. These should have vendor prefixes
2. why do we need the *-reg properties
3. for the *-reg properties its not clear what bits means?
4. tune-table is not well described, what does 'clock tune' mean, why an array?


> +Example:
> +
> +  The MSHC controller node can be split into two portions, SoC specific and
> +  board specific portions as listed below.
> +

Does dtc merge this all into a single node?  Would probably be good to have comments
in the example stating both that the merge into one node, and which is board and which
is soc.

> +	dwmmc_0: dwmmc0@fcd03000 {
> +		compatible = "hisilicon,hi4511-dw-mshc";
> +		reg = <0xfcd03000 0x1000>;
> +		interrupts = <0 16 4>;
> +		#address-cells = <1>;
> +		#size-cells = <0>;
> +		clocks = <&clk_sd>, <&clk_ddrc_per>;
> +		clock-names = "ciu", "biu";
> +		clken-reg = <0x1f8 0>;
> +		drv-sel-reg = <0x1f8 4 4>;
> +		sam-sel-reg = <0x1f8 8 4>;
> +		div-reg = <0x1f8 1 3>;
> +		tune-table =
> +		<180000000 6 6 13 13 25000000>,
> +		<0 0 0 0 0 0>,
> +		<360000000 6 4 2 0 50000000>,
> +		<180000000 6 4 13 13 25000000>,
> +		<360000000 6 4 2 0 50000000>,
> +		<720000000 6 1 9 4 100000000>,
> +		<0 0 0 0 0 0>,
> +		<360000000 7 1 3 0 50000000>;
> +	};
> +	dwmmc0@fcd03000 {
> +		num-slots = <1>;
> +		vmmc-supply = <&ldo12>;
> +		fifo-depth = <0x100>;
> +		supports-highspeed;
> +		pinctrl-names = "default";
> +		pinctrl-0 = <&sd_pmx_pins &sd_cfg_func1 &sd_cfg_func2>;
> +		slot@0 {
> +			reg = <0>;
> +			bus-width = <4>;
> +			disable-wp;
> +			cd-gpios = <&gpio10 3 0>;
> +		};
> +	};
> +
> +PCTRL:
> +

should have a description

> +Required Properties:
> +* compatible: should be
> +	- "hisilicon,pctrl": Peripheral control
> +

Reg is not described as a required property.

> +Example:
> +
> +	pctrl: pctrl@fca09000 {
> +		compatible = "hisilicon,pctrl";
> +		reg = <0xfca09000 0x1000>;
> +	};

This should be in its own binding document, and not part of the mmc binding

- k
> 

-- 
Employee of Qualcomm Innovation Center, Inc.
Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum, hosted by The Linux Foundation

Re: [PATCH 2/2] mmc: dw_mmc: add dw_mmc-k3 for k3 platform

[zhangfei gao]

Dear Kumar

Thanks for the careful review.

On Sun, Oct 27, 2013 at 11:29 PM, Kumar Gala <galak@codeaurora.org> wrote:
>
> On Oct 23, 2013, at 8:03 AM, Zhangfei Gao wrote:
>

>> +Required Properties:
>> +
>> +* compatible: should be
>> +     - "hisilicon,hi4511-dw-mshc": for controllers with hi4511
>> +       specific extentions.
>> +* vmmc-supply: should be vmmc used in dwmmc
>> +* fifo-depth: should be provided if register can not provide correct value
>> +* clken-reg: should be clock enable register and offset
>> +* drv-sel-reg: should be driver delay select register, offset and bits
>> +* sam-sel-reg: should be sample delay select register, offset and bits
>> +* div-reg: should be divider register, offset and bits
>> +* tune-table: should be array of clock tune mmc controller
>> +
>
> 1. These should have vendor prefixes
However Olof and Mark once suggested not using prefix if vector is
internally used.

Example before:
> + - #hisilicon,clkdiv-table-cells : the number of parameters after phandle in
> +   hisilicon,clkdiv-table property.
> + - hisilicon,clkdiv-table : list of value that are used to configure clock

Olof:
For a binding that is your own, you don't need to prefix the properties.
Prefixes are mostly used when adding new vendor-specific attributes to a common
binding.

Mark:
People seem to be *very* keen on adding them for all bindings for some
reason.

> 2. why do we need the *-reg properties
> 3. for the *-reg properties its not clear what bits means?

The silicon has some limitaiton to tune the clock timing according to
working mode,
including driver delay, sample delay, and divider.
If not set even the host register may not read out.
The register in PCTRL described below,
And one register may contains sereral usage,
for example
mmc0: 0x1f8: bit 0 is clock en/dis, bit 1 ~ bit 3 controls div, bit
4~7 controls drv-sel, bit 8 ~11 controls sam-sel
mmc1: 0x1f8: bit 12 ~~
register, offset and bits stands for 0x1f8, start bit and how many bits.
Is this fine?


> 4. tune-table is not well described, what does 'clock tune' mean, why an array?
Will change to  "tuning-table"
It is values used for computing these div, drv, sample register,
Also contains system clock need to be set, and output clock output
after PCTRL control.

>
>
>> +Example:
>> +
>> +  The MSHC controller node can be split into two portions, SoC specific and
>> +  board specific portions as listed below.
>> +
>
> Does dtc merge this all into a single node?  Would probably be good to have comments
> in the example stating both that the merge into one node, and which is board and which
> is soc.
OK, will add comments to distinguish dtsi for soc and dts for board.

>
>> +     dwmmc_0: dwmmc0@fcd03000 {
>> +             compatible = "hisilicon,hi4511-dw-mshc";
>> +             reg = <0xfcd03000 0x1000>;
>> +             interrupts = <0 16 4>;
>> +             #address-cells = <1>;
>> +             #size-cells = <0>;
>> +             clocks = <&clk_sd>, <&clk_ddrc_per>;
>> +             clock-names = "ciu", "biu";
>> +             clken-reg = <0x1f8 0>;
>> +             drv-sel-reg = <0x1f8 4 4>;
>> +             sam-sel-reg = <0x1f8 8 4>;
>> +             div-reg = <0x1f8 1 3>;
>> +             tune-table =
>> +             <180000000 6 6 13 13 25000000>,
>> +             <0 0 0 0 0 0>,
>> +             <360000000 6 4 2 0 50000000>,
>> +             <180000000 6 4 13 13 25000000>,
>> +             <360000000 6 4 2 0 50000000>,
>> +             <720000000 6 1 9 4 100000000>,
>> +             <0 0 0 0 0 0>,
>> +             <360000000 7 1 3 0 50000000>;
>> +     };
>> +     dwmmc0@fcd03000 {
>> +             num-slots = <1>;
>> +             vmmc-supply = <&ldo12>;
>> +             fifo-depth = <0x100>;
>> +             supports-highspeed;
>> +             pinctrl-names = "default";
>> +             pinctrl-0 = <&sd_pmx_pins &sd_cfg_func1 &sd_cfg_func2>;
>> +             slot@0 {
>> +                     reg = <0>;
>> +                     bus-width = <4>;
>> +                     disable-wp;
>> +                     cd-gpios = <&gpio10 3 0>;
>> +             };
>> +     };
>> +
>> +PCTRL:
>> +
>
> should have a description
OK.

>
>> +Required Properties:
>> +* compatible: should be
>> +     - "hisilicon,pctrl": Peripheral control
>> +
>
> Reg is not described as a required property.
OK.

>
>> +Example:
>> +
>> +     pctrl: pctrl@fca09000 {
>> +             compatible = "hisilicon,pctrl";
>> +             reg = <0xfca09000 0x1000>;
>> +     };
>
> This should be in its own binding document, and not part of the mmc binding
bindings/arm/hisilicon/hisilicon.txt still not been merged in another
patch, where
we want to push such desc as well as other system binding.
Is it OK adding this node desc twice in two files, since mmc need such node.
or move this desc after hisilicon.txt get merged for saving conflict?

Thanks

RE: [PATCH 2/2] mmc: dw_mmc: add dw_mmc-k3 for k3 platform

[Seungwon Jeon]

Hi Zhangfei,

On Wed, Oct 23, 2013, Zhangfei Gao wrote:
> update: fix typo
> 
> Add dw_mmc-k3.c for k3v2, support sd/emmc
> 
> Signed-off-by: Zhangfei Gao <zhangfei.gao@linaro.org>
> Tested-by: Zhigang Wang <brooke.wangzhigang@huawei.com>
> ---
>  .../devicetree/bindings/mmc/k3-dw-mshc.txt         |   77 +++++
>  drivers/mmc/host/Kconfig                           |   10 +
>  drivers/mmc/host/Makefile                          |    1 +
>  drivers/mmc/host/dw_mmc-k3.c                       |  297 ++++++++++++++++++++
>  4 files changed, 385 insertions(+)
>  create mode 100644 Documentation/devicetree/bindings/mmc/k3-dw-mshc.txt
>  create mode 100644 drivers/mmc/host/dw_mmc-k3.c
> 
> diff --git a/Documentation/devicetree/bindings/mmc/k3-dw-mshc.txt
> b/Documentation/devicetree/bindings/mmc/k3-dw-mshc.txt
> new file mode 100644
> index 0000000..0de8c27
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/mmc/k3-dw-mshc.txt
> @@ -0,0 +1,77 @@
> +* Hisilicon specific extensions to the Synopsys Designware Mobile
> +  Storage Host Controller
> +
> +Read synopsis-dw-mshc.txt for more details
> +The Synopsys 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 Synopsys dw mshc controller properties described
> +by synopsis-dw-mshc.txt and the properties used by the Hisilicon specific
> +extensions to the Synopsys Designware Mobile Storage Host Controller.
> +
> +Required Properties:
> +
> +* compatible: should be
> +	- "hisilicon,hi4511-dw-mshc": for controllers with hi4511
> +	  specific extentions.
> +* vmmc-supply: should be vmmc used in dwmmc
> +* fifo-depth: should be provided if register can not provide correct value
> +* clken-reg: should be clock enable register and offset
> +* drv-sel-reg: should be driver delay select register, offset and bits
> +* sam-sel-reg: should be sample delay select register, offset and bits
> +* div-reg: should be divider register, offset and bits
> +* tune-table: should be array of clock tune mmc controller
> +
> +Example:
> +
> +  The MSHC controller node can be split into two portions, SoC specific and
> +  board specific portions as listed below.
> +
> +	dwmmc_0: dwmmc0@fcd03000 {
> +		compatible = "hisilicon,hi4511-dw-mshc";
> +		reg = <0xfcd03000 0x1000>;
> +		interrupts = <0 16 4>;
> +		#address-cells = <1>;
> +		#size-cells = <0>;
> +		clocks = <&clk_sd>, <&clk_ddrc_per>;
> +		clock-names = "ciu", "biu";
> +		clken-reg = <0x1f8 0>;
> +		drv-sel-reg = <0x1f8 4 4>;
> +		sam-sel-reg = <0x1f8 8 4>;
> +		div-reg = <0x1f8 1 3>;
> +		tune-table =
> +		<180000000 6 6 13 13 25000000>,
> +		<0 0 0 0 0 0>,
> +		<360000000 6 4 2 0 50000000>,
> +		<180000000 6 4 13 13 25000000>,
> +		<360000000 6 4 2 0 50000000>,
> +		<720000000 6 1 9 4 100000000>,
> +		<0 0 0 0 0 0>,
> +		<360000000 7 1 3 0 50000000>;
> +	};
> +	dwmmc0@fcd03000 {
> +		num-slots = <1>;
> +		vmmc-supply = <&ldo12>;
> +		fifo-depth = <0x100>;
> +		supports-highspeed;
> +		pinctrl-names = "default";
> +		pinctrl-0 = <&sd_pmx_pins &sd_cfg_func1 &sd_cfg_func2>;
> +		slot@0 {
> +			reg = <0>;
> +			bus-width = <4>;
> +			disable-wp;
> +			cd-gpios = <&gpio10 3 0>;
> +		};
> +	};
> +
> +PCTRL:
> +
> +Required Properties:
> +* compatible: should be
> +	- "hisilicon,pctrl": Peripheral control
> +
> +Example:
> +
> +	pctrl: pctrl@fca09000 {
> +		compatible = "hisilicon,pctrl";
> +		reg = <0xfca09000 0x1000>;
> +	};
> diff --git a/drivers/mmc/host/Kconfig b/drivers/mmc/host/Kconfig
> index 7fc5099..45aaa2d 100644
> --- a/drivers/mmc/host/Kconfig
> +++ b/drivers/mmc/host/Kconfig
> @@ -575,6 +575,16 @@ config MMC_DW_SOCFPGA
>  	  This selects support for Altera SoCFPGA specific extensions to the
>  	  Synopsys DesignWare Memory Card Interface driver.
> 
> +config MMC_DW_K3
> +	tristate "K3 specific extensions for Synopsys DW Memory Card Interface"
> +	depends on MMC_DW
> +	select MMC_DW_PLTFM
> +	select MMC_DW_IDMAC
> +	help
> +	  This selects support for Hisilicon K3 SoC specific extensions to the
> +	  Synopsys DesignWare Memory Card Interface driver. Select this option
> +	  for platforms based on Hisilicon K3 SoC's.
> +
>  config MMC_DW_PCI
>  	tristate "Synopsys Designware MCI support on PCI bus"
>  	depends on MMC_DW && PCI
> diff --git a/drivers/mmc/host/Makefile b/drivers/mmc/host/Makefile
> index c41d0c3..64f5f8d 100644
> --- a/drivers/mmc/host/Makefile
> +++ b/drivers/mmc/host/Makefile
> @@ -43,6 +43,7 @@ obj-$(CONFIG_MMC_DW)		+= dw_mmc.o
>  obj-$(CONFIG_MMC_DW_PLTFM)	+= dw_mmc-pltfm.o
>  obj-$(CONFIG_MMC_DW_EXYNOS)	+= dw_mmc-exynos.o
>  obj-$(CONFIG_MMC_DW_SOCFPGA)	+= dw_mmc-socfpga.o
> +obj-$(CONFIG_MMC_DW_K3)		+= dw_mmc-k3.o
>  obj-$(CONFIG_MMC_DW_PCI)	+= dw_mmc-pci.o
>  obj-$(CONFIG_MMC_SH_MMCIF)	+= sh_mmcif.o
>  obj-$(CONFIG_MMC_JZ4740)	+= jz4740_mmc.o
> diff --git a/drivers/mmc/host/dw_mmc-k3.c b/drivers/mmc/host/dw_mmc-k3.c
> new file mode 100644
> index 0000000..0ff75328
> --- /dev/null
> +++ b/drivers/mmc/host/dw_mmc-k3.c
> @@ -0,0 +1,297 @@
> +/*
> + * Copyright (c) 2013 Linaro Ltd.
> + * Copyright (c) 2013 Hisilicon Limited.
> + *
> + * 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.
> + */
> +
> +#include <linux/module.h>
> +#include <linux/platform_device.h>
> +#include <linux/clk.h>
> +#include <linux/mmc/host.h>
> +#include <linux/mmc/dw_mmc.h>
> +#include <linux/of_address.h>
> +
> +#include "dw_mmc.h"
> +#include "dw_mmc-pltfm.h"
> +
> +enum dw_mci_k3_type {
> +	DW_MCI_TYPE_HI4511,
> +};
> +
> +static struct dw_mci_k3_compatible {
> +	char				*compatible;
> +	enum dw_mci_k3_type		type;
> +} k3_compat[] = {
> +	{
> +		.compatible	= "hisilicon,hi4511-dw-mshc",
> +		.type		= DW_MCI_TYPE_HI4511,
> +	},
> +};
> +
> +#define TABLE_WIDTH 6
> +#define TABLE_HEIGHT 8
> +struct dw_mci_k3_priv_data {
> +	enum dw_mci_k3_type	type;
> +	int			old_timing;
> +	u32			table[TABLE_WIDTH * TABLE_HEIGHT];
> +	u32			clken_reg;
> +	u32			clken_bit;
> +	u32			sam_reg;
> +	u32			sam_off;
> +	u32			sam_bits;
> +	u32			drv_reg;
> +	u32			drv_off;
> +	u32			drv_bits;
> +	u32			div_reg;
> +	u32			div_off;
> +	u32			div_bits;
> +};
> +
> +static void __iomem *pctrl;
> +static DEFINE_SPINLOCK(mmc_tuning_lock);
> +
> +static u32 dw_mci_k3_delay(u32 val, u32 para, u32 off, u32 len)
> +{
> +	u32 i;
> +
> +	if (para >= 0) {
> +		for (i = 0; i < len; i++) {
> +			if (para % 2)
> +				val |= 1<<(off + i);
> +			else
> +				val &= ~(1<<(off + i));
> +			para = para >> 1;
> +		}
> +	}
> +	return val;
> +}
> +
> +static void dw_mci_k3_set_timing(struct dw_mci_k3_priv_data *priv,
> +					u32 sam, u32 drv, u32 div)
> +{
> +	u32 val;
> +	unsigned long flags;
> +
> +	if (!pctrl || !priv->clken_reg || !priv->sam_reg
> +		|| !priv->drv_reg || !priv->div_reg)
> +		return;
> +
> +	spin_lock_irqsave(&mmc_tuning_lock, flags);
> +
> +	val = readl(pctrl + priv->clken_reg);
> +	val &= ~(1 << priv->clken_bit);
> +	writel(val, pctrl + priv->clken_reg);
> +
> +	val = readl(pctrl + priv->sam_reg);
> +	val = dw_mci_k3_delay(val, sam, priv->sam_off, priv->sam_bits);
> +	writel(val, pctrl + priv->sam_reg);
> +
> +	val = readl(pctrl + priv->drv_reg);
> +	val = dw_mci_k3_delay(val, drv, priv->drv_off, priv->drv_bits);
> +	writel(val, pctrl + priv->drv_reg);
> +
> +	val = readl(pctrl + priv->div_reg);
> +	val = dw_mci_k3_delay(val, div, priv->div_off, priv->div_bits);
> +	writel(val, pctrl + priv->div_reg);
> +
> +	val = readl(pctrl + priv->clken_reg);
> +	val |= 1 << priv->clken_bit;
> +	writel(val, pctrl + priv->clken_reg);
> +
> +	spin_unlock_irqrestore(&mmc_tuning_lock, flags);
> +}
> +
> +static void dw_mci_k3_tun(struct dw_mci *host, int timing)
> +{
> +	struct dw_mci_k3_priv_data *priv = host->priv;
> +	int ret;
> +	u32 *table = &priv->table[TABLE_WIDTH * timing];
> +
> +	if (priv->old_timing == timing)
> +		return;
> +
> +	ret = clk_set_rate(host->ciu_clk, table[0]);
> +	if (ret) {
> +		dev_err(host->dev, "clk_set_rate failed\n");
> +		return;
> +	}
> +	dw_mci_k3_set_timing(priv, (table[3] + table[4]) / 2,
> +			table[2], table[1]);
> +	host->bus_hz = table[5];
> +	priv->old_timing = timing;
> +}
> +
> +static void dw_mci_k3_set_ios(struct dw_mci *host, struct mmc_ios *ios)
> +{
> +	struct dw_mci_k3_priv_data *priv = host->priv;
> +
> +	if (priv->type == DW_MCI_TYPE_HI4511)
There are several checking controller type.
But now DW_MCI_TYPE_HI4511 is just introduced alone.
It seems like unnecessary.

> +		dw_mci_k3_tun(host, ios->timing);
> +}
> +
> +static int dw_mci_k3_priv_init(struct dw_mci *host)
> +{
> +	struct dw_mci_k3_priv_data *priv;
> +	int i, 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->old_timing = -1;
> +	host->priv = priv;
> +
> +	for (i = 0; i < ARRAY_SIZE(k3_compat); i++) {
> +		if (of_device_is_compatible(host->dev->of_node,
> +					k3_compat[i].compatible))
> +			priv->type = k3_compat[i].type;
> +	}
> +
> +	if (priv->type == DW_MCI_TYPE_HI4511) {
> +		ret = of_property_read_u32_array(host->dev->of_node,
> +			"tune-table", priv->table, TABLE_WIDTH * TABLE_HEIGHT);
> +		if (ret) {
> +			dev_err(host->dev, "not found tune-table\n");
> +			return -EINVAL;
> +		}
> +
> +		if (!pctrl) {
> +			struct device_node *node;
> +
> +			node = of_find_compatible_node(NULL, NULL,
> +						"hisilicon,pctrl");
> +			pctrl = of_iomap(node, 0);
> +		}
> +	}
> +
> +	return 0;
> +}
> +
> +static int dw_mci_k3_setup_clock(struct dw_mci *host)
> +{
> +	struct dw_mci_k3_priv_data *priv = host->priv;
> +
> +	if (priv->type == DW_MCI_TYPE_HI4511)
> +		dw_mci_k3_tun(host, MMC_TIMING_LEGACY);
> +
> +	return 0;
> +}
> +
> +static int dw_mci_k3_parse_dt(struct dw_mci *host)
> +{
> +	struct dw_mci_k3_priv_data *priv = host->priv;
> +	struct device_node *np = host->dev->of_node;
> +	u32 data[3];
> +	int ret;
> +
> +	if (priv->type == DW_MCI_TYPE_HI4511) {
Didn't you see Kernel panic here?
host->priv is not allocated yet, it's a invalid pointer dereference.
dw_mci_k3_parse_dt() is called prior to dw_mci_k3_priv_init().
You can check dw_mci_probe() sequence.

> +		ret = of_property_read_u32_array(np,
> +				"clken-reg", data, 2);
> +		if (!ret) {
> +			priv->clken_reg = data[0];
> +			priv->clken_bit = data[1];
> +		}
> +
> +		ret = of_property_read_u32_array(np,
> +				"drv-sel-reg", data, 3);
> +		if (!ret) {
> +			priv->drv_reg = data[0];
> +			priv->drv_off = data[1];
> +			priv->drv_bits = data[2];
> +		}
> +
> +		ret = of_property_read_u32_array(np,
> +				"sam-sel-reg", data, 3);
> +		if (!ret) {
> +			priv->sam_reg = data[0];
> +			priv->sam_off = data[1];
> +			priv->sam_bits = data[2];
> +		}
> +
> +		ret = of_property_read_u32_array(np,
> +				"div-reg", data, 3);
> +		if (!ret) {
> +			priv->div_reg = data[0];
> +			priv->div_off = data[1];
> +			priv->div_bits = data[2];
> +		}
Should these register information be got from dt?
It could be define in source code instead.

Thanks,
Seungwon Jeon
> +	}
> +
> +	return 0;
> +}
> +
> +static unsigned long k3_dwmmc_caps[4] = {
> +	MMC_CAP_4_BIT_DATA | MMC_CAP_SD_HIGHSPEED,
> +	MMC_CAP_8_BIT_DATA | MMC_CAP_MMC_HIGHSPEED,
> +	0,
> +	0,
> +};
> +
> +static const struct dw_mci_drv_data k3_drv_data = {
> +	.caps			= k3_dwmmc_caps,
> +	.init			= dw_mci_k3_priv_init,
> +	.set_ios		= dw_mci_k3_set_ios,
> +	.setup_clock		= dw_mci_k3_setup_clock,
> +	.parse_dt		= dw_mci_k3_parse_dt,
> +};
> +
> +static const struct of_device_id dw_mci_k3_match[] = {
> +	{ .compatible = "hisilicon,hi4511-dw-mshc",
> +			.data = &k3_drv_data, },
> +	{},
> +};
> +MODULE_DEVICE_TABLE(of, dw_mci_k3_match);
> +
> +static int dw_mci_k3_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_k3_match, pdev->dev.of_node);
> +	drv_data = match->data;
> +
> +	return dw_mci_pltfm_register(pdev, drv_data);
> +}
> +
> +static int dw_mci_k3_suspend(struct device *dev)
> +{
> +	struct dw_mci *host = dev_get_drvdata(dev);
> +
> +	return dw_mci_suspend(host);
> +}
> +
> +static int dw_mci_k3_resume(struct device *dev)
> +{
> +	struct dw_mci *host = dev_get_drvdata(dev);
> +	struct dw_mci_k3_priv_data *priv = host->priv;
> +
> +	if (priv->type == DW_MCI_TYPE_HI4511) {
> +		priv->old_timing = -1;
> +		dw_mci_k3_tun(host, MMC_TIMING_LEGACY);
> +	}
> +
> +	return dw_mci_resume(host);
> +}
> +
> +SIMPLE_DEV_PM_OPS(dw_mci_k3_pmops, dw_mci_k3_suspend, dw_mci_k3_resume);
> +
> +static struct platform_driver dw_mci_k3_pltfm_driver = {
> +	.probe		= dw_mci_k3_probe,
> +	.remove		= dw_mci_pltfm_remove,
> +	.driver		= {
> +		.name		= "dwmmc_k3",
> +		.of_match_table	= dw_mci_k3_match,
> +		.pm		= &dw_mci_k3_pmops,
> +	},
> +};
> +
> +module_platform_driver(dw_mci_k3_pltfm_driver);
> +
> +MODULE_DESCRIPTION("K3 Specific DW-MSHC Driver Extension");
> +MODULE_LICENSE("GPL v2");
> +MODULE_ALIAS("platform:dwmmc-k3");
> --
> 1.7.9.5
> 
> --
> To unsubscribe from this list: send the line "unsubscribe linux-mmc" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH 2/2] mmc: dw_mmc: add dw_mmc-k3 for k3 platform

[zhangfei gao]

Dear Seungwon

Thanks for giving suggestion.

On Thu, Oct 31, 2013 at 11:24 PM, Seungwon Jeon <tgih.jun@samsung.com> wrote:
> Hi Zhangfei,

>> +static void dw_mci_k3_set_ios(struct dw_mci *host, struct mmc_ios *ios)
>> +{
>> +     struct dw_mci_k3_priv_data *priv = host->priv;
>> +
>> +     if (priv->type == DW_MCI_TYPE_HI4511)
> There are several checking controller type.
> But now DW_MCI_TYPE_HI4511 is just introduced alone.
> It seems like unnecessary.

Yes, currently only K3V2 is added, and k3v3 code will be added soon.
Do you think type is more suitable to add later?

>> +static int dw_mci_k3_parse_dt(struct dw_mci *host)
>> +{
>> +     struct dw_mci_k3_priv_data *priv = host->priv;
>> +     struct device_node *np = host->dev->of_node;
>> +     u32 data[3];
>> +     int ret;
>> +
>> +     if (priv->type == DW_MCI_TYPE_HI4511) {
> Didn't you see Kernel panic here?
> host->priv is not allocated yet, it's a invalid pointer dereference.
> dw_mci_k3_parse_dt() is called prior to dw_mci_k3_priv_init().
> You can check dw_mci_probe() sequence.

It is no problem here
dw_mci_pltfm_register -> drv_data->init(host) -> dw_mci_probe -> dw_mci_parse_dt

>
>> +             ret = of_property_read_u32_array(np,
>> +                             "clken-reg", data, 2);
>> +             if (!ret) {
>> +                     priv->clken_reg = data[0];
>> +                     priv->clken_bit = data[1];
>> +             }
>> +
>> +             ret = of_property_read_u32_array(np,
>> +                             "drv-sel-reg", data, 3);
>> +             if (!ret) {
>> +                     priv->drv_reg = data[0];
>> +                     priv->drv_off = data[1];
>> +                     priv->drv_bits = data[2];
>> +             }
>> +
>> +             ret = of_property_read_u32_array(np,
>> +                             "sam-sel-reg", data, 3);
>> +             if (!ret) {
>> +                     priv->sam_reg = data[0];
>> +                     priv->sam_off = data[1];
>> +                     priv->sam_bits = data[2];
>> +             }
>> +
>> +             ret = of_property_read_u32_array(np,
>> +                             "div-reg", data, 3);
>> +             if (!ret) {
>> +                     priv->div_reg = data[0];
>> +                     priv->div_off = data[1];
>> +                     priv->div_bits = data[2];
>> +             }
> Should these register information be got from dt?
> It could be define in source code instead.

There are many register from pctrl node instead of mmc controller.
If set in code, we may read id and then switch id, set register, start
bit, bits number,
which is no rules for different controller, using defiine is not helpful.
for example:
 switch (idx) {
        case 0:
                clken_reg = 0x1F8;
                clken_bit = 0;
                drv_sel_reg = 0x1F8;
                drv_sel_bit = 4;
                sam_sel_reg = 0x1F8;
                sam_sel_bit = 8;
                div_reg = 0x1F8;
                div_bit = 1;
                break;
        case 1:
                clken_reg = 0x1F8;
                clken_bit = 12;
                drv_sel_reg = 0x1F8;
                drv_sel_bit = 16;
                sam_sel_reg = 0x1F8;
                sam_sel_bit = 20;
                div_reg = 0x1F8;
                div_bit = 13;
                break;
        case 2:
                clken_reg = 0x1F8;
                clken_bit = 24;
                drv_sel_reg = 0x1F8;
                drv_sel_bit = 28;
                sam_sel_reg = 0x1FC;
                sam_sel_bit = 0;
                div_reg = 0x1F8;
                div_bit = 25;
                break;

So define register offset, start bit and bits number in dts make it simplier.
Is this acceptable?

Thanks.

RE: [PATCH 2/2] mmc: dw_mmc: add dw_mmc-k3 for k3 platform

[Seungwon Jeon]

On Fri, November 01, 2013, zhangfei gao wrote:
> Dear Seungwon
> 
> Thanks for giving suggestion.
> 
> On Thu, Oct 31, 2013 at 11:24 PM, Seungwon Jeon <tgih.jun@samsung.com> wrote:
> > Hi Zhangfei,
> 
> >> +static void dw_mci_k3_set_ios(struct dw_mci *host, struct mmc_ios *ios)
> >> +{
> >> +     struct dw_mci_k3_priv_data *priv = host->priv;
> >> +
> >> +     if (priv->type == DW_MCI_TYPE_HI4511)
> > There are several checking controller type.
> > But now DW_MCI_TYPE_HI4511 is just introduced alone.
> > It seems like unnecessary.
> 
> Yes, currently only K3V2 is added, and k3v3 code will be added soon.
> Do you think type is more suitable to add later?
Yes, it would be better.

> 
> >> +static int dw_mci_k3_parse_dt(struct dw_mci *host)
> >> +{
> >> +     struct dw_mci_k3_priv_data *priv = host->priv;
> >> +     struct device_node *np = host->dev->of_node;
> >> +     u32 data[3];
> >> +     int ret;
> >> +
> >> +     if (priv->type == DW_MCI_TYPE_HI4511) {
> > Didn't you see Kernel panic here?
> > host->priv is not allocated yet, it's a invalid pointer dereference.
> > dw_mci_k3_parse_dt() is called prior to dw_mci_k3_priv_init().
> > You can check dw_mci_probe() sequence.
> 
> It is no problem here
> dw_mci_pltfm_register -> drv_data->init(host) -> dw_mci_probe -> dw_mci_parse_dt
I guess your base is not latest.
Can you check cjb's tree?

> 
> >
> >> +             ret = of_property_read_u32_array(np,
> >> +                             "clken-reg", data, 2);
> >> +             if (!ret) {
> >> +                     priv->clken_reg = data[0];
> >> +                     priv->clken_bit = data[1];
> >> +             }
> >> +
> >> +             ret = of_property_read_u32_array(np,
> >> +                             "drv-sel-reg", data, 3);
> >> +             if (!ret) {
> >> +                     priv->drv_reg = data[0];
> >> +                     priv->drv_off = data[1];
> >> +                     priv->drv_bits = data[2];
> >> +             }
> >> +
> >> +             ret = of_property_read_u32_array(np,
> >> +                             "sam-sel-reg", data, 3);
> >> +             if (!ret) {
> >> +                     priv->sam_reg = data[0];
> >> +                     priv->sam_off = data[1];
> >> +                     priv->sam_bits = data[2];
> >> +             }
> >> +
> >> +             ret = of_property_read_u32_array(np,
> >> +                             "div-reg", data, 3);
> >> +             if (!ret) {
> >> +                     priv->div_reg = data[0];
> >> +                     priv->div_off = data[1];
> >> +                     priv->div_bits = data[2];
> >> +             }
> > Should these register information be got from dt?
> > It could be define in source code instead.
> 
> There are many register from pctrl node instead of mmc controller.
> If set in code, we may read id and then switch id, set register, start
> bit, bits number,
> which is no rules for different controller, using defiine is not helpful.
> for example:
>  switch (idx) {
>         case 0:
>                 clken_reg = 0x1F8;
>                 clken_bit = 0;
>                 drv_sel_reg = 0x1F8;
>                 drv_sel_bit = 4;
>                 sam_sel_reg = 0x1F8;
>                 sam_sel_bit = 8;
>                 div_reg = 0x1F8;
>                 div_bit = 1;
>                 break;
>         case 1:
>                 clken_reg = 0x1F8;
>                 clken_bit = 12;
>                 drv_sel_reg = 0x1F8;
>                 drv_sel_bit = 16;
>                 sam_sel_reg = 0x1F8;
>                 sam_sel_bit = 20;
>                 div_reg = 0x1F8;
>                 div_bit = 13;
>                 break;
>         case 2:
>                 clken_reg = 0x1F8;
>                 clken_bit = 24;
>                 drv_sel_reg = 0x1F8;
>                 drv_sel_bit = 28;
>                 sam_sel_reg = 0x1FC;
>                 sam_sel_bit = 0;
>                 div_reg = 0x1F8;
>                 div_bit = 25;
>                 break;
> 
> So define register offset, start bit and bits number in dts make it simplier.
> Is this acceptable?
It's up to you. I think there will be better ways.
For example, some table for variable register can be used for each controller type.
Additionally, if you intend to use dt, error handling is needed.
dw_mci_k3_parse_dt always returns success although of_property_read_u32_array is failed.

Thanks,
Seungwon Jeon

Re: [PATCH 2/2] mmc: dw_mmc: add dw_mmc-k3 for k3 platform

[zhangfei gao]

On Fri, Nov 1, 2013 at 1:21 AM, Seungwon Jeon <tgih.jun@samsung.com> wrote:
> On Fri, November 01, 2013, zhangfei gao wrote:
>> Dear Seungwon
>>
>> Thanks for giving suggestion.
>>
>> On Thu, Oct 31, 2013 at 11:24 PM, Seungwon Jeon <tgih.jun@samsung.com> wrote:
>> > Hi Zhangfei,
>>
>> >> +static void dw_mci_k3_set_ios(struct dw_mci *host, struct mmc_ios *ios)
>> >> +{
>> >> +     struct dw_mci_k3_priv_data *priv = host->priv;
>> >> +
>> >> +     if (priv->type == DW_MCI_TYPE_HI4511)
>> > There are several checking controller type.
>> > But now DW_MCI_TYPE_HI4511 is just introduced alone.
>> > It seems like unnecessary.
>>
>> Yes, currently only K3V2 is added, and k3v3 code will be added soon.
>> Do you think type is more suitable to add later?
> Yes, it would be better.
OK, got it.

>
>>
>> >> +static int dw_mci_k3_parse_dt(struct dw_mci *host)
>> >> +{
>> >> +     struct dw_mci_k3_priv_data *priv = host->priv;
>> >> +     struct device_node *np = host->dev->of_node;
>> >> +     u32 data[3];
>> >> +     int ret;
>> >> +
>> >> +     if (priv->type == DW_MCI_TYPE_HI4511) {
>> > Didn't you see Kernel panic here?
>> > host->priv is not allocated yet, it's a invalid pointer dereference.
>> > dw_mci_k3_parse_dt() is called prior to dw_mci_k3_priv_init().
>> > You can check dw_mci_probe() sequence.
>>
>> It is no problem here
>> dw_mci_pltfm_register -> drv_data->init(host) -> dw_mci_probe -> dw_mci_parse_dt
> I guess your base is not latest.
> Can you check cjb's tree?

Yes, thanks for the info.
It is changed in 3.12-rc2
While what we use is 3.12-rc1, will rebase.

>
>>
>> >
>> >> +             ret = of_property_read_u32_array(np,
>> >> +                             "clken-reg", data, 2);
>> >> +             if (!ret) {
>> >> +                     priv->clken_reg = data[0];
>> >> +                     priv->clken_bit = data[1];
>> >> +             }
>> >> +
>> >> +             ret = of_property_read_u32_array(np,
>> >> +                             "drv-sel-reg", data, 3);
>> >> +             if (!ret) {
>> >> +                     priv->drv_reg = data[0];
>> >> +                     priv->drv_off = data[1];
>> >> +                     priv->drv_bits = data[2];
>> >> +             }
>> >> +
>> >> +             ret = of_property_read_u32_array(np,
>> >> +                             "sam-sel-reg", data, 3);
>> >> +             if (!ret) {
>> >> +                     priv->sam_reg = data[0];
>> >> +                     priv->sam_off = data[1];
>> >> +                     priv->sam_bits = data[2];
>> >> +             }
>> >> +
>> >> +             ret = of_property_read_u32_array(np,
>> >> +                             "div-reg", data, 3);
>> >> +             if (!ret) {
>> >> +                     priv->div_reg = data[0];
>> >> +                     priv->div_off = data[1];
>> >> +                     priv->div_bits = data[2];
>> >> +             }
>> > Should these register information be got from dt?
>> > It could be define in source code instead.
>>
>> There are many register from pctrl node instead of mmc controller.
>> If set in code, we may read id and then switch id, set register, start
>> bit, bits number,
>> which is no rules for different controller, using defiine is not helpful.
>> for example:
>>  switch (idx) {
>>         case 0:
>>                 clken_reg = 0x1F8;
>>                 clken_bit = 0;
>>                 drv_sel_reg = 0x1F8;
>>                 drv_sel_bit = 4;
>>                 sam_sel_reg = 0x1F8;
>>                 sam_sel_bit = 8;
>>                 div_reg = 0x1F8;
>>                 div_bit = 1;
>>                 break;
>>         case 1:
>>                 clken_reg = 0x1F8;
>>                 clken_bit = 12;
>>                 drv_sel_reg = 0x1F8;
>>                 drv_sel_bit = 16;
>>                 sam_sel_reg = 0x1F8;
>>                 sam_sel_bit = 20;
>>                 div_reg = 0x1F8;
>>                 div_bit = 13;
>>                 break;
>>         case 2:
>>                 clken_reg = 0x1F8;
>>                 clken_bit = 24;
>>                 drv_sel_reg = 0x1F8;
>>                 drv_sel_bit = 28;
>>                 sam_sel_reg = 0x1FC;
>>                 sam_sel_bit = 0;
>>                 div_reg = 0x1F8;
>>                 div_bit = 25;
>>                 break;
>>
>> So define register offset, start bit and bits number in dts make it simplier.
>> Is this acceptable?
> It's up to you. I think there will be better ways.
> For example, some table for variable register can be used for each controller type.
> Additionally, if you intend to use dt, error handling is needed.
> dw_mci_k3_parse_dt always returns success although of_property_read_u32_array is failed.

The reason is they are optional property.
SD would fail if not setting the property, while emmc does not matter.
So set for optional, dw_mci_k3_set_timing will check whether they exists.

Thanks

[PATCH v3 0/2] mmc: dw_mmc: add dw_mmc-k3

[Zhangfei Gao]

v3:
0001: 
Put set/clear_bit DW_MMC_CARD_PRESENT in dw_mci_get_cd, 
Since dw_mci_request will check DW_MMC_CARD_PRESENT before sending cmd

0002:
Follow suggestion from Chris, Kumar and Seungwon
Sync to latest mmc-next, which is 3.12-rc2
Remove enum dw_mci_k3_type etc

v2:
Follow Jaehoon's suggestion
Use slot-gpio.c handle cd pin
Move table out to dts
other suggestion

Zhangfei Gao (2):
  mmc: dw_mmc: add dw_mci_of_get_cd_gpio to handle cd pin
  mmc: dw_mmc: add dw_mmc-k3 for k3 platform

 .../devicetree/bindings/mmc/k3-dw-mshc.txt         |   83 +++++++
 drivers/mmc/host/Kconfig                           |   10 +
 drivers/mmc/host/Makefile                          |    1 +
 drivers/mmc/host/dw_mmc-k3.c                       |  247 ++++++++++++++++++++
 drivers/mmc/host/dw_mmc.c                          |   52 +++--
 5 files changed, 380 insertions(+), 13 deletions(-)
 create mode 100644 Documentation/devicetree/bindings/mmc/k3-dw-mshc.txt
 create mode 100644 drivers/mmc/host/dw_mmc-k3.c

-- 
1.7.9.5

[PATCH 2/2] mmc: dw_mmc: add dw_mmc-k3 for k3 platform

[Zhangfei Gao]

Add dw_mmc-k3.c for k3v2, support sd/emmc

Signed-off-by: Zhangfei Gao <zhangfei.gao@linaro.org>
Tested-by: Zhigang Wang <brooke.wangzhigang@huawei.com>
---
 .../devicetree/bindings/mmc/k3-dw-mshc.txt         |   83 +++++++
 drivers/mmc/host/Kconfig                           |   10 +
 drivers/mmc/host/Makefile                          |    1 +
 drivers/mmc/host/dw_mmc-k3.c                       |  247 ++++++++++++++++++++
 4 files changed, 341 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/mmc/k3-dw-mshc.txt
 create mode 100644 drivers/mmc/host/dw_mmc-k3.c

diff --git a/Documentation/devicetree/bindings/mmc/k3-dw-mshc.txt b/Documentation/devicetree/bindings/mmc/k3-dw-mshc.txt
new file mode 100644
index 0000000..ea858f4
--- /dev/null
+++ b/Documentation/devicetree/bindings/mmc/k3-dw-mshc.txt
@@ -0,0 +1,83 @@
+* Hisilicon specific extensions to the Synopsys Designware Mobile
+  Storage Host Controller
+
+Read synopsis-dw-mshc.txt for more details
+The Synopsys 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 Synopsys dw mshc controller properties described
+by synopsis-dw-mshc.txt and the properties used by the Hisilicon specific
+extensions to the Synopsys Designware Mobile Storage Host Controller.
+
+Required Properties:
+
+* compatible: should be
+	- "hisilicon,hi4511-dw-mshc": for controllers with hi4511
+	  specific extentions.
+* reg: should be address and size of mmc controller
+* vmmc-supply: should be vmmc used in dwmmc
+* fifo-depth: should be provided if register can not provide correct value
+* tuning-table: should be array to tune mmc controller, including clock rate
+	to be set and values for setting optional register.
+
+Optional properties:
+
+/* These registers from pctrl node used for tuning mmc controller if required */
+* clken-reg: should be clock enable register and offset bit
+* drv-sel-reg: should be driver delay select register, start bit and bits numbers
+* sam-sel-reg: should be sample delay select register, start bit and bits numbers
+* div-reg: should be divider register, start bit and bits numbers
+
+Example:
+
+	/* SoC portion */
+	dwmmc_0: dwmmc0@fcd03000 {
+		compatible = "hisilicon,hi4511-dw-mshc";
+		reg = <0xfcd03000 0x1000>;
+		interrupts = <0 16 4>;
+		#address-cells = <1>;
+		#size-cells = <0>;
+		clocks = <&clk_sd>, <&clk_ddrc_per>;
+		clock-names = "ciu", "biu";
+		clken-reg = <0x1f8 0>;
+		drv-sel-reg = <0x1f8 4 4>;
+		sam-sel-reg = <0x1f8 8 4>;
+		div-reg = <0x1f8 1 3>;
+		tuning-table =
+		<180000000 6 6 13 13 25000000>,
+		<0 0 0 0 0 0>,
+		<360000000 6 4 2 0 50000000>,
+		<180000000 6 4 13 13 25000000>,
+		<360000000 6 4 2 0 50000000>,
+		<720000000 6 1 9 4 100000000>,
+		<0 0 0 0 0 0>,
+		<360000000 7 1 3 0 50000000>;
+	};
+
+	/* Board portion */
+	dwmmc0@fcd03000 {
+		num-slots = <1>;
+		vmmc-supply = <&ldo12>;
+		fifo-depth = <0x100>;
+		supports-highspeed;
+		pinctrl-names = "default";
+		pinctrl-0 = <&sd_pmx_pins &sd_cfg_func1 &sd_cfg_func2>;
+		slot@0 {
+			reg = <0>;
+			bus-width = <4>;
+			disable-wp;
+			cd-gpios = <&gpio10 3 0>;
+		};
+	};
+
+PCTRL: Peripheral misc control register
+
+Required Properties:
+- compatible: "hisilicon,pctrl"
+- reg: Address and size of pctrl.
+
+Example:
+
+	pctrl: pctrl@fca09000 {
+		compatible = "hisilicon,pctrl";
+		reg = <0xfca09000 0x1000>;
+	};
diff --git a/drivers/mmc/host/Kconfig b/drivers/mmc/host/Kconfig
index 7fc5099..45aaa2d 100644
--- a/drivers/mmc/host/Kconfig
+++ b/drivers/mmc/host/Kconfig
@@ -575,6 +575,16 @@ config MMC_DW_SOCFPGA
 	  This selects support for Altera SoCFPGA specific extensions to the
 	  Synopsys DesignWare Memory Card Interface driver.
 
+config MMC_DW_K3
+	tristate "K3 specific extensions for Synopsys DW Memory Card Interface"
+	depends on MMC_DW
+	select MMC_DW_PLTFM
+	select MMC_DW_IDMAC
+	help
+	  This selects support for Hisilicon K3 SoC specific extensions to the
+	  Synopsys DesignWare Memory Card Interface driver. Select this option
+	  for platforms based on Hisilicon K3 SoC's.
+
 config MMC_DW_PCI
 	tristate "Synopsys Designware MCI support on PCI bus"
 	depends on MMC_DW && PCI
diff --git a/drivers/mmc/host/Makefile b/drivers/mmc/host/Makefile
index c41d0c3..64f5f8d 100644
--- a/drivers/mmc/host/Makefile
+++ b/drivers/mmc/host/Makefile
@@ -43,6 +43,7 @@ obj-$(CONFIG_MMC_DW)		+= dw_mmc.o
 obj-$(CONFIG_MMC_DW_PLTFM)	+= dw_mmc-pltfm.o
 obj-$(CONFIG_MMC_DW_EXYNOS)	+= dw_mmc-exynos.o
 obj-$(CONFIG_MMC_DW_SOCFPGA)	+= dw_mmc-socfpga.o
+obj-$(CONFIG_MMC_DW_K3)		+= dw_mmc-k3.o
 obj-$(CONFIG_MMC_DW_PCI)	+= dw_mmc-pci.o
 obj-$(CONFIG_MMC_SH_MMCIF)	+= sh_mmcif.o
 obj-$(CONFIG_MMC_JZ4740)	+= jz4740_mmc.o
diff --git a/drivers/mmc/host/dw_mmc-k3.c b/drivers/mmc/host/dw_mmc-k3.c
new file mode 100644
index 0000000..881d2f4
--- /dev/null
+++ b/drivers/mmc/host/dw_mmc-k3.c
@@ -0,0 +1,247 @@
+/*
+ * Copyright (c) 2013 Linaro Ltd.
+ * Copyright (c) 2013 Hisilicon Limited.
+ *
+ * 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.
+ */
+
+#include <linux/module.h>
+#include <linux/platform_device.h>
+#include <linux/clk.h>
+#include <linux/mmc/host.h>
+#include <linux/mmc/dw_mmc.h>
+#include <linux/of_address.h>
+
+#include "dw_mmc.h"
+#include "dw_mmc-pltfm.h"
+
+#define TABLE_WIDTH 6
+#define TABLE_HEIGHT 8
+struct dw_mci_k3_priv_data {
+	int			old_timing;
+	u32			table[TABLE_WIDTH * TABLE_HEIGHT];
+	u32			clken_reg;
+	u32			clken_bit;
+	u32			sam_reg;
+	u32			sam_off;
+	u32			sam_bits;
+	u32			drv_reg;
+	u32			drv_off;
+	u32			drv_bits;
+	u32			div_reg;
+	u32			div_off;
+	u32			div_bits;
+};
+
+static void __iomem *pctrl;
+static DEFINE_SPINLOCK(mmc_tuning_lock);
+
+static u32 dw_mci_k3_delay(u32 val, u32 para, u32 off, u32 len)
+{
+	u32 i;
+
+	if (para >= 0) {
+		for (i = 0; i < len; i++) {
+			if (para % 2)
+				val |= 1 << (off + i);
+			else
+				val &= ~(1 << (off + i));
+			para = para >> 1;
+		}
+	}
+	return val;
+}
+
+static void dw_mci_k3_set_timing(struct dw_mci_k3_priv_data *priv,
+					u32 sam, u32 drv, u32 div)
+{
+	u32 val;
+	unsigned long flags;
+
+	if (!pctrl || !priv->clken_reg || !priv->sam_reg
+		|| !priv->drv_reg || !priv->div_reg)
+		return;
+
+	spin_lock_irqsave(&mmc_tuning_lock, flags);
+
+	val = readl(pctrl + priv->clken_reg);
+	val &= ~(1 << priv->clken_bit);
+	writel(val, pctrl + priv->clken_reg);
+
+	val = readl(pctrl + priv->sam_reg);
+	val = dw_mci_k3_delay(val, sam, priv->sam_off, priv->sam_bits);
+	writel(val, pctrl + priv->sam_reg);
+
+	val = readl(pctrl + priv->drv_reg);
+	val = dw_mci_k3_delay(val, drv, priv->drv_off, priv->drv_bits);
+	writel(val, pctrl + priv->drv_reg);
+
+	val = readl(pctrl + priv->div_reg);
+	val = dw_mci_k3_delay(val, div, priv->div_off, priv->div_bits);
+	writel(val, pctrl + priv->div_reg);
+
+	val = readl(pctrl + priv->clken_reg);
+	val |= 1 << priv->clken_bit;
+	writel(val, pctrl + priv->clken_reg);
+
+	spin_unlock_irqrestore(&mmc_tuning_lock, flags);
+}
+
+static void dw_mci_k3_tun(struct dw_mci *host, int timing)
+{
+	struct dw_mci_k3_priv_data *priv = host->priv;
+	u32 *table = &priv->table[TABLE_WIDTH * timing];
+	int ret;
+
+	if (priv->old_timing == timing)
+		return;
+
+	ret = clk_set_rate(host->ciu_clk, table[0]);
+	if (ret) {
+		dev_err(host->dev, "clk_set_rate failed\n");
+		return;
+	}
+	dw_mci_k3_set_timing(priv, (table[3] + table[4]) / 2,
+			table[2], table[1]);
+	host->bus_hz = table[5];
+	priv->old_timing = timing;
+}
+
+static void dw_mci_k3_set_ios(struct dw_mci *host, struct mmc_ios *ios)
+{
+	dw_mci_k3_tun(host, ios->timing);
+}
+
+static int dw_mci_k3_setup_clock(struct dw_mci *host)
+{
+	dw_mci_k3_tun(host, MMC_TIMING_LEGACY);
+	return 0;
+}
+
+static int dw_mci_k3_parse_dt(struct dw_mci *host)
+{
+	struct dw_mci_k3_priv_data *priv;
+	struct device_node *np = host->dev->of_node, *node;
+	u32 data[3];
+	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->old_timing = -1;
+	host->priv = priv;
+
+	ret = of_property_read_u32_array(host->dev->of_node, "tuning-table",
+			priv->table, TABLE_WIDTH * TABLE_HEIGHT);
+	if (ret) {
+		dev_err(host->dev, "not found tuning-table\n");
+		return -EINVAL;
+	}
+
+	if (!pctrl) {
+		node = of_find_compatible_node(NULL, NULL, "hisilicon,pctrl");
+		pctrl = of_iomap(node, 0);
+	}
+
+	ret = of_property_read_u32_array(np, "clken-reg", data, 2);
+	if (!ret) {
+		priv->clken_reg = data[0];
+		priv->clken_bit = data[1];
+	}
+
+	ret = of_property_read_u32_array(np, "drv-sel-reg", data, 3);
+	if (!ret) {
+		priv->drv_reg = data[0];
+		priv->drv_off = data[1];
+		priv->drv_bits = data[2];
+	}
+
+	ret = of_property_read_u32_array(np, "sam-sel-reg", data, 3);
+	if (!ret) {
+		priv->sam_reg = data[0];
+		priv->sam_off = data[1];
+		priv->sam_bits = data[2];
+	}
+
+	ret = of_property_read_u32_array(np, "div-reg", data, 3);
+	if (!ret) {
+		priv->div_reg = data[0];
+		priv->div_off = data[1];
+		priv->div_bits = data[2];
+	}
+
+	return 0;
+}
+
+static unsigned long k3_dwmmc_caps[4] = {
+	MMC_CAP_4_BIT_DATA | MMC_CAP_SD_HIGHSPEED,
+	MMC_CAP_8_BIT_DATA | MMC_CAP_MMC_HIGHSPEED,
+	0,
+	0,
+};
+
+static const struct dw_mci_drv_data k3_drv_data = {
+	.caps			= k3_dwmmc_caps,
+	.set_ios		= dw_mci_k3_set_ios,
+	.setup_clock		= dw_mci_k3_setup_clock,
+	.parse_dt		= dw_mci_k3_parse_dt,
+};
+
+static const struct of_device_id dw_mci_k3_match[] = {
+	{ .compatible = "hisilicon,hi4511-dw-mshc",
+			.data = &k3_drv_data, },
+	{},
+};
+MODULE_DEVICE_TABLE(of, dw_mci_k3_match);
+
+static int dw_mci_k3_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_k3_match, pdev->dev.of_node);
+	drv_data = match->data;
+
+	return dw_mci_pltfm_register(pdev, drv_data);
+}
+
+static int dw_mci_k3_suspend(struct device *dev)
+{
+	struct dw_mci *host = dev_get_drvdata(dev);
+
+	return dw_mci_suspend(host);
+}
+
+static int dw_mci_k3_resume(struct device *dev)
+{
+	struct dw_mci *host = dev_get_drvdata(dev);
+	struct dw_mci_k3_priv_data *priv = host->priv;
+
+	priv->old_timing = -1;
+	dw_mci_k3_tun(host, MMC_TIMING_LEGACY);
+
+	return dw_mci_resume(host);
+}
+
+SIMPLE_DEV_PM_OPS(dw_mci_k3_pmops, dw_mci_k3_suspend, dw_mci_k3_resume);
+
+static struct platform_driver dw_mci_k3_pltfm_driver = {
+	.probe		= dw_mci_k3_probe,
+	.remove		= dw_mci_pltfm_remove,
+	.driver		= {
+		.name		= "dwmmc_k3",
+		.of_match_table	= dw_mci_k3_match,
+		.pm		= &dw_mci_k3_pmops,
+	},
+};
+
+module_platform_driver(dw_mci_k3_pltfm_driver);
+
+MODULE_DESCRIPTION("K3 Specific DW-MSHC Driver Extension");
+MODULE_LICENSE("GPL v2");
+MODULE_ALIAS("platform:dwmmc-k3");
-- 
1.7.9.5

RE: [PATCH 2/2] mmc: dw_mmc: add dw_mmc-k3 for k3 platform

[Seungwon Jeon]

On Fri, November 08, 2013, Zhangfei Gao wrote:
> Add dw_mmc-k3.c for k3v2, support sd/emmc
> 
> Signed-off-by: Zhangfei Gao <zhangfei.gao@linaro.org>
> Tested-by: Zhigang Wang <brooke.wangzhigang@huawei.com>
> ---
>  .../devicetree/bindings/mmc/k3-dw-mshc.txt         |   83 +++++++
>  drivers/mmc/host/Kconfig                           |   10 +
>  drivers/mmc/host/Makefile                          |    1 +
>  drivers/mmc/host/dw_mmc-k3.c                       |  247 ++++++++++++++++++++
>  4 files changed, 341 insertions(+)
>  create mode 100644 Documentation/devicetree/bindings/mmc/k3-dw-mshc.txt
>  create mode 100644 drivers/mmc/host/dw_mmc-k3.c
> 
> diff --git a/Documentation/devicetree/bindings/mmc/k3-dw-mshc.txt
> b/Documentation/devicetree/bindings/mmc/k3-dw-mshc.txt
> new file mode 100644
> index 0000000..ea858f4
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/mmc/k3-dw-mshc.txt
> @@ -0,0 +1,83 @@
> +* Hisilicon specific extensions to the Synopsys Designware Mobile
> +  Storage Host Controller
> +
> +Read synopsis-dw-mshc.txt for more details
> +The Synopsys 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 Synopsys dw mshc controller properties described
> +by synopsis-dw-mshc.txt and the properties used by the Hisilicon specific
> +extensions to the Synopsys Designware Mobile Storage Host Controller.
> +
> +Required Properties:
> +
> +* compatible: should be
> +	- "hisilicon,hi4511-dw-mshc": for controllers with hi4511
> +	  specific extentions.
> +* reg: should be address and size of mmc controller
> +* vmmc-supply: should be vmmc used in dwmmc
> +* fifo-depth: should be provided if register can not provide correct value
> +* tuning-table: should be array to tune mmc controller, including clock rate
> +	to be set and values for setting optional register.
Descriptions for each cell would be required here.
And isn't tuning-table closer to board portion rather than soc portion?

> +
> +Optional properties:
> +
> +/* These registers from pctrl node used for tuning mmc controller if required */
> +* clken-reg: should be clock enable register and offset bit
> +* drv-sel-reg: should be driver delay select register, start bit and bits numbers
> +* sam-sel-reg: should be sample delay select register, start bit and bits numbers
> +* div-reg: should be divider register, start bit and bits numbers
> +
> +Example:
> +
> +	/* SoC portion */
> +	dwmmc_0: dwmmc0@fcd03000 {
> +		compatible = "hisilicon,hi4511-dw-mshc";
> +		reg = <0xfcd03000 0x1000>;
> +		interrupts = <0 16 4>;
> +		#address-cells = <1>;
> +		#size-cells = <0>;
> +		clocks = <&clk_sd>, <&clk_ddrc_per>;
> +		clock-names = "ciu", "biu";
> +		clken-reg = <0x1f8 0>;
> +		drv-sel-reg = <0x1f8 4 4>;
> +		sam-sel-reg = <0x1f8 8 4>;
> +		div-reg = <0x1f8 1 3>;
> +		tuning-table =
> +		<180000000 6 6 13 13 25000000>,
> +		<0 0 0 0 0 0>,
> +		<360000000 6 4 2 0 50000000>,
> +		<180000000 6 4 13 13 25000000>,
> +		<360000000 6 4 2 0 50000000>,
> +		<720000000 6 1 9 4 100000000>,
> +		<0 0 0 0 0 0>,
> +		<360000000 7 1 3 0 50000000>;
> +	};
> +
> +	/* Board portion */
> +	dwmmc0@fcd03000 {
> +		num-slots = <1>;
> +		vmmc-supply = <&ldo12>;
> +		fifo-depth = <0x100>;
> +		supports-highspeed;
> +		pinctrl-names = "default";
> +		pinctrl-0 = <&sd_pmx_pins &sd_cfg_func1 &sd_cfg_func2>;
> +		slot@0 {
> +			reg = <0>;
> +			bus-width = <4>;
> +			disable-wp;
> +			cd-gpios = <&gpio10 3 0>;
> +		};
> +	};
> +
> +PCTRL: Peripheral misc control register
> +
> +Required Properties:
> +- compatible: "hisilicon,pctrl"
> +- reg: Address and size of pctrl.
> +
> +Example:
> +
> +	pctrl: pctrl@fca09000 {
> +		compatible = "hisilicon,pctrl";
> +		reg = <0xfca09000 0x1000>;
> +	};
> diff --git a/drivers/mmc/host/Kconfig b/drivers/mmc/host/Kconfig
> index 7fc5099..45aaa2d 100644
> --- a/drivers/mmc/host/Kconfig
> +++ b/drivers/mmc/host/Kconfig
> @@ -575,6 +575,16 @@ config MMC_DW_SOCFPGA
>  	  This selects support for Altera SoCFPGA specific extensions to the
>  	  Synopsys DesignWare Memory Card Interface driver.
> 
> +config MMC_DW_K3
> +	tristate "K3 specific extensions for Synopsys DW Memory Card Interface"
> +	depends on MMC_DW
> +	select MMC_DW_PLTFM
> +	select MMC_DW_IDMAC
> +	help
> +	  This selects support for Hisilicon K3 SoC specific extensions to the
> +	  Synopsys DesignWare Memory Card Interface driver. Select this option
> +	  for platforms based on Hisilicon K3 SoC's.
> +
>  config MMC_DW_PCI
>  	tristate "Synopsys Designware MCI support on PCI bus"
>  	depends on MMC_DW && PCI
> diff --git a/drivers/mmc/host/Makefile b/drivers/mmc/host/Makefile
> index c41d0c3..64f5f8d 100644
> --- a/drivers/mmc/host/Makefile
> +++ b/drivers/mmc/host/Makefile
> @@ -43,6 +43,7 @@ obj-$(CONFIG_MMC_DW)		+= dw_mmc.o
>  obj-$(CONFIG_MMC_DW_PLTFM)	+= dw_mmc-pltfm.o
>  obj-$(CONFIG_MMC_DW_EXYNOS)	+= dw_mmc-exynos.o
>  obj-$(CONFIG_MMC_DW_SOCFPGA)	+= dw_mmc-socfpga.o
> +obj-$(CONFIG_MMC_DW_K3)		+= dw_mmc-k3.o
>  obj-$(CONFIG_MMC_DW_PCI)	+= dw_mmc-pci.o
>  obj-$(CONFIG_MMC_SH_MMCIF)	+= sh_mmcif.o
>  obj-$(CONFIG_MMC_JZ4740)	+= jz4740_mmc.o
> diff --git a/drivers/mmc/host/dw_mmc-k3.c b/drivers/mmc/host/dw_mmc-k3.c
> new file mode 100644
> index 0000000..881d2f4
> --- /dev/null
> +++ b/drivers/mmc/host/dw_mmc-k3.c
> @@ -0,0 +1,247 @@
> +/*
> + * Copyright (c) 2013 Linaro Ltd.
> + * Copyright (c) 2013 Hisilicon Limited.
> + *
> + * 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.
> + */
> +
> +#include <linux/module.h>
> +#include <linux/platform_device.h>
> +#include <linux/clk.h>
> +#include <linux/mmc/host.h>
> +#include <linux/mmc/dw_mmc.h>
> +#include <linux/of_address.h>
> +
> +#include "dw_mmc.h"
> +#include "dw_mmc-pltfm.h"
> +
> +#define TABLE_WIDTH 6
> +#define TABLE_HEIGHT 8
> +struct dw_mci_k3_priv_data {
> +	int			old_timing;
> +	u32			table[TABLE_WIDTH * TABLE_HEIGHT];
> +	u32			clken_reg;
> +	u32			clken_bit;
> +	u32			sam_reg;
> +	u32			sam_off;
> +	u32			sam_bits;
> +	u32			drv_reg;
> +	u32			drv_off;
> +	u32			drv_bits;
> +	u32			div_reg;
> +	u32			div_off;
> +	u32			div_bits;
> +};
> +
> +static void __iomem *pctrl;
> +static DEFINE_SPINLOCK(mmc_tuning_lock);
Can the above variables be involved in dw_mci_k3_priv_data instead of global declaration?

> +
> +static u32 dw_mci_k3_delay(u32 val, u32 para, u32 off, u32 len)
> +{
> +	u32 i;
> +
> +	if (para >= 0) {
> +		for (i = 0; i < len; i++) {
> +			if (para % 2)
> +				val |= 1 << (off + i);
> +			else
> +				val &= ~(1 << (off + i));
> +			para = para >> 1;
> +		}
> +	}
> +	return val;
> +}
> +
> +static void dw_mci_k3_set_timing(struct dw_mci_k3_priv_data *priv,
> +					u32 sam, u32 drv, u32 div)
> +{
> +	u32 val;
> +	unsigned long flags;
> +
> +	if (!pctrl || !priv->clken_reg || !priv->sam_reg
> +		|| !priv->drv_reg || !priv->div_reg)
> +		return;
> +
> +	spin_lock_irqsave(&mmc_tuning_lock, flags);
> +
> +	val = readl(pctrl + priv->clken_reg);
> +	val &= ~(1 << priv->clken_bit);
> +	writel(val, pctrl + priv->clken_reg);
> +
> +	val = readl(pctrl + priv->sam_reg);
> +	val = dw_mci_k3_delay(val, sam, priv->sam_off, priv->sam_bits);
> +	writel(val, pctrl + priv->sam_reg);
> +
> +	val = readl(pctrl + priv->drv_reg);
> +	val = dw_mci_k3_delay(val, drv, priv->drv_off, priv->drv_bits);
> +	writel(val, pctrl + priv->drv_reg);
> +
> +	val = readl(pctrl + priv->div_reg);
> +	val = dw_mci_k3_delay(val, div, priv->div_off, priv->div_bits);
> +	writel(val, pctrl + priv->div_reg);
> +
> +	val = readl(pctrl + priv->clken_reg);
> +	val |= 1 << priv->clken_bit;
> +	writel(val, pctrl + priv->clken_reg);
> +
> +	spin_unlock_irqrestore(&mmc_tuning_lock, flags);
> +}
> +
> +static void dw_mci_k3_tun(struct dw_mci *host, int timing)
> +{
> +	struct dw_mci_k3_priv_data *priv = host->priv;
> +	u32 *table = &priv->table[TABLE_WIDTH * timing];
Timing value should be less than TABLE_HEIGHT.
It needs to check the range of value.

> +	int ret;
> +
> +	if (priv->old_timing == timing)
> +		return;
> +
> +	ret = clk_set_rate(host->ciu_clk, table[0]);
> +	if (ret) {
> +		dev_err(host->dev, "clk_set_rate failed\n");
> +		return;
> +	}
> +	dw_mci_k3_set_timing(priv, (table[3] + table[4]) / 2,
> +			table[2], table[1]);
> +	host->bus_hz = table[5];
> +	priv->old_timing = timing;
> +}
> +
> +static void dw_mci_k3_set_ios(struct dw_mci *host, struct mmc_ios *ios)
> +{
> +	dw_mci_k3_tun(host, ios->timing);
> +}
> +
> +static int dw_mci_k3_setup_clock(struct dw_mci *host)
> +{
> +	dw_mci_k3_tun(host, MMC_TIMING_LEGACY);
Is it necessary here?
When card-init, MMC_TIMING_LEGACY will be passed via dw_mci_k3_set_ios().

> +	return 0;
> +}
> +
> +static int dw_mci_k3_parse_dt(struct dw_mci *host)
> +{
> +	struct dw_mci_k3_priv_data *priv;
> +	struct device_node *np = host->dev->of_node, *node;
> +	u32 data[3];
> +	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->old_timing = -1;
> +	host->priv = priv;
> +
> +	ret = of_property_read_u32_array(host->dev->of_node, "tuning-table",
> +			priv->table, TABLE_WIDTH * TABLE_HEIGHT);
> +	if (ret) {
> +		dev_err(host->dev, "not found tuning-table\n");
> +		return -EINVAL;
> +	}
> +
> +	if (!pctrl) {
> +		node = of_find_compatible_node(NULL, NULL, "hisilicon,pctrl");
> +		pctrl = of_iomap(node, 0);
> +	}
> +
> +	ret = of_property_read_u32_array(np, "clken-reg", data, 2);
> +	if (!ret) {
> +		priv->clken_reg = data[0];
> +		priv->clken_bit = data[1];
> +	}
> +
> +	ret = of_property_read_u32_array(np, "drv-sel-reg", data, 3);
> +	if (!ret) {
> +		priv->drv_reg = data[0];
> +		priv->drv_off = data[1];
> +		priv->drv_bits = data[2];
> +	}
> +
> +	ret = of_property_read_u32_array(np, "sam-sel-reg", data, 3);
> +	if (!ret) {
> +		priv->sam_reg = data[0];
> +		priv->sam_off = data[1];
> +		priv->sam_bits = data[2];
> +	}
> +
> +	ret = of_property_read_u32_array(np, "div-reg", data, 3);
> +	if (!ret) {
> +		priv->div_reg = data[0];
> +		priv->div_off = data[1];
> +		priv->div_bits = data[2];
> +	}
All properties seem mandatory.
If right, it should return error in case of failure?

> +
> +	return 0;
> +}
> +
> +static unsigned long k3_dwmmc_caps[4] = {
> +	MMC_CAP_4_BIT_DATA | MMC_CAP_SD_HIGHSPEED,
> +	MMC_CAP_8_BIT_DATA | MMC_CAP_MMC_HIGHSPEED,
> +	0,
> +	0,
> +};
> +
> +static const struct dw_mci_drv_data k3_drv_data = {
> +	.caps			= k3_dwmmc_caps,
> +	.set_ios		= dw_mci_k3_set_ios,
> +	.setup_clock		= dw_mci_k3_setup_clock,
> +	.parse_dt		= dw_mci_k3_parse_dt,
> +};
> +
> +static const struct of_device_id dw_mci_k3_match[] = {
> +	{ .compatible = "hisilicon,hi4511-dw-mshc",
> +			.data = &k3_drv_data, },
> +	{},
> +};
> +MODULE_DEVICE_TABLE(of, dw_mci_k3_match);
> +
> +static int dw_mci_k3_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_k3_match, pdev->dev.of_node);
> +	drv_data = match->data;
> +
> +	return dw_mci_pltfm_register(pdev, drv_data);
> +}
> +
> +static int dw_mci_k3_suspend(struct device *dev)
> +{
> +	struct dw_mci *host = dev_get_drvdata(dev);
> +
> +	return dw_mci_suspend(host);
> +}
> +
> +static int dw_mci_k3_resume(struct device *dev)
> +{
> +	struct dw_mci *host = dev_get_drvdata(dev);
> +	struct dw_mci_k3_priv_data *priv = host->priv;
> +
> +	priv->old_timing = -1;
> +	dw_mci_k3_tun(host, MMC_TIMING_LEGACY);
Same above.
When resuming, MMC_TIMING_LEGACY will be passed via dw_mci_k3_set_ios().

Thanks,
Seungwon Jeon

> +
> +	return dw_mci_resume(host);
> +}
> +
> +SIMPLE_DEV_PM_OPS(dw_mci_k3_pmops, dw_mci_k3_suspend, dw_mci_k3_resume);
> +
> +static struct platform_driver dw_mci_k3_pltfm_driver = {
> +	.probe		= dw_mci_k3_probe,
> +	.remove		= dw_mci_pltfm_remove,
> +	.driver		= {
> +		.name		= "dwmmc_k3",
> +		.of_match_table	= dw_mci_k3_match,
> +		.pm		= &dw_mci_k3_pmops,
> +	},
> +};
> +
> +module_platform_driver(dw_mci_k3_pltfm_driver);
> +
> +MODULE_DESCRIPTION("K3 Specific DW-MSHC Driver Extension");
> +MODULE_LICENSE("GPL v2");
> +MODULE_ALIAS("platform:dwmmc-k3");
> --
> 1.7.9.5
> 
> --
> To unsubscribe from this list: send the line "unsubscribe linux-mmc" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH 2/2] mmc: dw_mmc: add dw_mmc-k3 for k3 platform

[Zhangfei Gao]

Dear Seungwon

On Thu, Dec 5, 2013 at 10:00 PM, Seungwon Jeon <tgih.jun@samsung.com> wrote:

>> +static DEFINE_SPINLOCK(mmc_tuning_lock);
> Can the above variables be involved in dw_mci_k3_priv_data instead of global declaration?
>
Unfortunately, this can be be put in priv_data, which is different
instance for different controller.
Here need protect register shared by different controllers


>> +static int dw_mci_k3_setup_clock(struct dw_mci *host)
>> +{
>> +     dw_mci_k3_tun(host, MMC_TIMING_LEGACY);
> Is it necessary here?
> When card-init, MMC_TIMING_LEGACY will be passed via dw_mci_k3_set_ios().

Read register will fail if not set these register earlier, and set_ios
is too late.
Same reason for resume.

Thanks

Re: [PATCH 2/2] mmc: dw_mmc: add dw_mmc-k3 for k3 platform

[Rob Herring]

On Thu, Nov 7, 2013 at 11:38 PM, Zhangfei Gao <zhangfei.gao@linaro.org> wrote:
> Add dw_mmc-k3.c for k3v2, support sd/emmc
>
> Signed-off-by: Zhangfei Gao <zhangfei.gao@linaro.org>
> Tested-by: Zhigang Wang <brooke.wangzhigang@huawei.com>
> ---
>  .../devicetree/bindings/mmc/k3-dw-mshc.txt         |   83 +++++++
>  drivers/mmc/host/Kconfig                           |   10 +
>  drivers/mmc/host/Makefile                          |    1 +
>  drivers/mmc/host/dw_mmc-k3.c                       |  247 ++++++++++++++++++++
>  4 files changed, 341 insertions(+)
>  create mode 100644 Documentation/devicetree/bindings/mmc/k3-dw-mshc.txt
>  create mode 100644 drivers/mmc/host/dw_mmc-k3.c
>
> diff --git a/Documentation/devicetree/bindings/mmc/k3-dw-mshc.txt b/Documentation/devicetree/bindings/mmc/k3-dw-mshc.txt
> new file mode 100644
> index 0000000..ea858f4
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/mmc/k3-dw-mshc.txt
> @@ -0,0 +1,83 @@
> +* Hisilicon specific extensions to the Synopsys Designware Mobile
> +  Storage Host Controller
> +
> +Read synopsis-dw-mshc.txt for more details
> +The Synopsys 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 Synopsys dw mshc controller properties described
> +by synopsis-dw-mshc.txt and the properties used by the Hisilicon specific
> +extensions to the Synopsys Designware Mobile Storage Host Controller.
> +
> +Required Properties:
> +
> +* compatible: should be
> +       - "hisilicon,hi4511-dw-mshc": for controllers with hi4511
> +         specific extentions.
> +* reg: should be address and size of mmc controller

This should be covered by the base binding.

> +* vmmc-supply: should be vmmc used in dwmmc
> +* fifo-depth: should be provided if register can not provide correct value
> +* tuning-table: should be array to tune mmc controller, including clock rate
> +       to be set and values for setting optional register.

Please define the size and what the values mean.

> +
> +Optional properties:
> +
> +/* These registers from pctrl node used for tuning mmc controller if required */
> +* clken-reg: should be clock enable register and offset bit
> +* drv-sel-reg: should be driver delay select register, start bit and bits numbers
> +* sam-sel-reg: should be sample delay select register, start bit and bits numbers
> +* div-reg: should be divider register, start bit and bits numbers

Do these really vary on different boards or versions of the IP? If
not, then put this information in the kernel.

Otherwise, these need better description such as number of words and
order of values.

> +
> +Example:
> +
> +       /* SoC portion */
> +       dwmmc_0: dwmmc0@fcd03000 {
> +               compatible = "hisilicon,hi4511-dw-mshc";
> +               reg = <0xfcd03000 0x1000>;
> +               interrupts = <0 16 4>;
> +               #address-cells = <1>;
> +               #size-cells = <0>;
> +               clocks = <&clk_sd>, <&clk_ddrc_per>;
> +               clock-names = "ciu", "biu";
> +               clken-reg = <0x1f8 0>;
> +               drv-sel-reg = <0x1f8 4 4>;
> +               sam-sel-reg = <0x1f8 8 4>;
> +               div-reg = <0x1f8 1 3>;
> +               tuning-table =
> +               <180000000 6 6 13 13 25000000>,
> +               <0 0 0 0 0 0>,
> +               <360000000 6 4 2 0 50000000>,
> +               <180000000 6 4 13 13 25000000>,
> +               <360000000 6 4 2 0 50000000>,
> +               <720000000 6 1 9 4 100000000>,
> +               <0 0 0 0 0 0>,
> +               <360000000 7 1 3 0 50000000>;
> +       };
> +
> +       /* Board portion */
> +       dwmmc0@fcd03000 {
> +               num-slots = <1>;
> +               vmmc-supply = <&ldo12>;
> +               fifo-depth = <0x100>;
> +               supports-highspeed;

Please reference that the binding uses standard SD properties (state
which binding document).

> +               pinctrl-names = "default";
> +               pinctrl-0 = <&sd_pmx_pins &sd_cfg_func1 &sd_cfg_func2>;
> +               slot@0 {
> +                       reg = <0>;
> +                       bus-width = <4>;
> +                       disable-wp;
> +                       cd-gpios = <&gpio10 3 0>;
> +               };
> +       };
> +
> +PCTRL: Peripheral misc control register

Is this only MMC control bits? Seems like this belongs in its own doc.

> +
> +Required Properties:
> +- compatible: "hisilicon,pctrl"
> +- reg: Address and size of pctrl.
> +
> +Example:
> +
> +       pctrl: pctrl@fca09000 {
> +               compatible = "hisilicon,pctrl";
> +               reg = <0xfca09000 0x1000>;
> +       };
> diff --git a/drivers/mmc/host/Kconfig b/drivers/mmc/host/Kconfig
> index 7fc5099..45aaa2d 100644
> --- a/drivers/mmc/host/Kconfig
> +++ b/drivers/mmc/host/Kconfig
> @@ -575,6 +575,16 @@ config MMC_DW_SOCFPGA
>           This selects support for Altera SoCFPGA specific extensions to the
>           Synopsys DesignWare Memory Card Interface driver.
>
> +config MMC_DW_K3
> +       tristate "K3 specific extensions for Synopsys DW Memory Card Interface"
> +       depends on MMC_DW
> +       select MMC_DW_PLTFM
> +       select MMC_DW_IDMAC
> +       help
> +         This selects support for Hisilicon K3 SoC specific extensions to the
> +         Synopsys DesignWare Memory Card Interface driver. Select this option
> +         for platforms based on Hisilicon K3 SoC's.
> +
>  config MMC_DW_PCI
>         tristate "Synopsys Designware MCI support on PCI bus"
>         depends on MMC_DW && PCI
> diff --git a/drivers/mmc/host/Makefile b/drivers/mmc/host/Makefile
> index c41d0c3..64f5f8d 100644
> --- a/drivers/mmc/host/Makefile
> +++ b/drivers/mmc/host/Makefile
> @@ -43,6 +43,7 @@ obj-$(CONFIG_MMC_DW)          += dw_mmc.o
>  obj-$(CONFIG_MMC_DW_PLTFM)     += dw_mmc-pltfm.o
>  obj-$(CONFIG_MMC_DW_EXYNOS)    += dw_mmc-exynos.o
>  obj-$(CONFIG_MMC_DW_SOCFPGA)   += dw_mmc-socfpga.o
> +obj-$(CONFIG_MMC_DW_K3)                += dw_mmc-k3.o
>  obj-$(CONFIG_MMC_DW_PCI)       += dw_mmc-pci.o
>  obj-$(CONFIG_MMC_SH_MMCIF)     += sh_mmcif.o
>  obj-$(CONFIG_MMC_JZ4740)       += jz4740_mmc.o
> diff --git a/drivers/mmc/host/dw_mmc-k3.c b/drivers/mmc/host/dw_mmc-k3.c
> new file mode 100644
> index 0000000..881d2f4
> --- /dev/null
> +++ b/drivers/mmc/host/dw_mmc-k3.c
> @@ -0,0 +1,247 @@
> +/*
> + * Copyright (c) 2013 Linaro Ltd.
> + * Copyright (c) 2013 Hisilicon Limited.
> + *
> + * 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.
> + */
> +
> +#include <linux/module.h>
> +#include <linux/platform_device.h>
> +#include <linux/clk.h>
> +#include <linux/mmc/host.h>
> +#include <linux/mmc/dw_mmc.h>
> +#include <linux/of_address.h>
> +
> +#include "dw_mmc.h"
> +#include "dw_mmc-pltfm.h"
> +
> +#define TABLE_WIDTH 6
> +#define TABLE_HEIGHT 8
> +struct dw_mci_k3_priv_data {
> +       int                     old_timing;
> +       u32                     table[TABLE_WIDTH * TABLE_HEIGHT];
> +       u32                     clken_reg;
> +       u32                     clken_bit;
> +       u32                     sam_reg;
> +       u32                     sam_off;
> +       u32                     sam_bits;
> +       u32                     drv_reg;
> +       u32                     drv_off;
> +       u32                     drv_bits;
> +       u32                     div_reg;
> +       u32                     div_off;
> +       u32                     div_bits;
> +};
> +
> +static void __iomem *pctrl;
> +static DEFINE_SPINLOCK(mmc_tuning_lock);
> +
> +static u32 dw_mci_k3_delay(u32 val, u32 para, u32 off, u32 len)
> +{
> +       u32 i;
> +
> +       if (para >= 0) {
> +               for (i = 0; i < len; i++) {
> +                       if (para % 2)
> +                               val |= 1 << (off + i);
> +                       else
> +                               val &= ~(1 << (off + i));
> +                       para = para >> 1;
> +               }
> +       }
> +       return val;
> +}
> +
> +static void dw_mci_k3_set_timing(struct dw_mci_k3_priv_data *priv,
> +                                       u32 sam, u32 drv, u32 div)
> +{
> +       u32 val;
> +       unsigned long flags;
> +
> +       if (!pctrl || !priv->clken_reg || !priv->sam_reg
> +               || !priv->drv_reg || !priv->div_reg)
> +               return;
> +
> +       spin_lock_irqsave(&mmc_tuning_lock, flags);
> +
> +       val = readl(pctrl + priv->clken_reg);
> +       val &= ~(1 << priv->clken_bit);
> +       writel(val, pctrl + priv->clken_reg);
> +
> +       val = readl(pctrl + priv->sam_reg);
> +       val = dw_mci_k3_delay(val, sam, priv->sam_off, priv->sam_bits);
> +       writel(val, pctrl + priv->sam_reg);
> +
> +       val = readl(pctrl + priv->drv_reg);
> +       val = dw_mci_k3_delay(val, drv, priv->drv_off, priv->drv_bits);
> +       writel(val, pctrl + priv->drv_reg);
> +
> +       val = readl(pctrl + priv->div_reg);
> +       val = dw_mci_k3_delay(val, div, priv->div_off, priv->div_bits);
> +       writel(val, pctrl + priv->div_reg);
> +
> +       val = readl(pctrl + priv->clken_reg);
> +       val |= 1 << priv->clken_bit;
> +       writel(val, pctrl + priv->clken_reg);
> +
> +       spin_unlock_irqrestore(&mmc_tuning_lock, flags);
> +}
> +
> +static void dw_mci_k3_tun(struct dw_mci *host, int timing)
> +{
> +       struct dw_mci_k3_priv_data *priv = host->priv;
> +       u32 *table = &priv->table[TABLE_WIDTH * timing];
> +       int ret;
> +
> +       if (priv->old_timing == timing)
> +               return;
> +
> +       ret = clk_set_rate(host->ciu_clk, table[0]);
> +       if (ret) {
> +               dev_err(host->dev, "clk_set_rate failed\n");
> +               return;
> +       }
> +       dw_mci_k3_set_timing(priv, (table[3] + table[4]) / 2,
> +                       table[2], table[1]);
> +       host->bus_hz = table[5];
> +       priv->old_timing = timing;
> +}
> +
> +static void dw_mci_k3_set_ios(struct dw_mci *host, struct mmc_ios *ios)
> +{
> +       dw_mci_k3_tun(host, ios->timing);
> +}
> +
> +static int dw_mci_k3_setup_clock(struct dw_mci *host)
> +{
> +       dw_mci_k3_tun(host, MMC_TIMING_LEGACY);
> +       return 0;
> +}
> +
> +static int dw_mci_k3_parse_dt(struct dw_mci *host)
> +{
> +       struct dw_mci_k3_priv_data *priv;
> +       struct device_node *np = host->dev->of_node, *node;
> +       u32 data[3];
> +       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->old_timing = -1;
> +       host->priv = priv;
> +
> +       ret = of_property_read_u32_array(host->dev->of_node, "tuning-table",
> +                       priv->table, TABLE_WIDTH * TABLE_HEIGHT);
> +       if (ret) {
> +               dev_err(host->dev, "not found tuning-table\n");
> +               return -EINVAL;
> +       }
> +
> +       if (!pctrl) {
> +               node = of_find_compatible_node(NULL, NULL, "hisilicon,pctrl");
> +               pctrl = of_iomap(node, 0);
> +       }
> +
> +       ret = of_property_read_u32_array(np, "clken-reg", data, 2);
> +       if (!ret) {
> +               priv->clken_reg = data[0];
> +               priv->clken_bit = data[1];
> +       }
> +
> +       ret = of_property_read_u32_array(np, "drv-sel-reg", data, 3);
> +       if (!ret) {
> +               priv->drv_reg = data[0];
> +               priv->drv_off = data[1];
> +               priv->drv_bits = data[2];
> +       }
> +
> +       ret = of_property_read_u32_array(np, "sam-sel-reg", data, 3);
> +       if (!ret) {
> +               priv->sam_reg = data[0];
> +               priv->sam_off = data[1];
> +               priv->sam_bits = data[2];
> +       }
> +
> +       ret = of_property_read_u32_array(np, "div-reg", data, 3);
> +       if (!ret) {
> +               priv->div_reg = data[0];
> +               priv->div_off = data[1];
> +               priv->div_bits = data[2];
> +       }
> +
> +       return 0;
> +}
> +
> +static unsigned long k3_dwmmc_caps[4] = {
> +       MMC_CAP_4_BIT_DATA | MMC_CAP_SD_HIGHSPEED,
> +       MMC_CAP_8_BIT_DATA | MMC_CAP_MMC_HIGHSPEED,
> +       0,
> +       0,
> +};
> +
> +static const struct dw_mci_drv_data k3_drv_data = {
> +       .caps                   = k3_dwmmc_caps,
> +       .set_ios                = dw_mci_k3_set_ios,
> +       .setup_clock            = dw_mci_k3_setup_clock,
> +       .parse_dt               = dw_mci_k3_parse_dt,
> +};
> +
> +static const struct of_device_id dw_mci_k3_match[] = {
> +       { .compatible = "hisilicon,hi4511-dw-mshc",
> +                       .data = &k3_drv_data, },
> +       {},
> +};
> +MODULE_DEVICE_TABLE(of, dw_mci_k3_match);
> +
> +static int dw_mci_k3_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_k3_match, pdev->dev.of_node);
> +       drv_data = match->data;
> +
> +       return dw_mci_pltfm_register(pdev, drv_data);
> +}
> +
> +static int dw_mci_k3_suspend(struct device *dev)
> +{
> +       struct dw_mci *host = dev_get_drvdata(dev);
> +
> +       return dw_mci_suspend(host);
> +}
> +
> +static int dw_mci_k3_resume(struct device *dev)
> +{
> +       struct dw_mci *host = dev_get_drvdata(dev);
> +       struct dw_mci_k3_priv_data *priv = host->priv;
> +
> +       priv->old_timing = -1;
> +       dw_mci_k3_tun(host, MMC_TIMING_LEGACY);
> +
> +       return dw_mci_resume(host);
> +}
> +
> +SIMPLE_DEV_PM_OPS(dw_mci_k3_pmops, dw_mci_k3_suspend, dw_mci_k3_resume);
> +
> +static struct platform_driver dw_mci_k3_pltfm_driver = {
> +       .probe          = dw_mci_k3_probe,
> +       .remove         = dw_mci_pltfm_remove,
> +       .driver         = {
> +               .name           = "dwmmc_k3",
> +               .of_match_table = dw_mci_k3_match,
> +               .pm             = &dw_mci_k3_pmops,
> +       },
> +};
> +
> +module_platform_driver(dw_mci_k3_pltfm_driver);
> +
> +MODULE_DESCRIPTION("K3 Specific DW-MSHC Driver Extension");
> +MODULE_LICENSE("GPL v2");
> +MODULE_ALIAS("platform:dwmmc-k3");
> --
> 1.7.9.5
>
>
> _______________________________________________
> linux-arm-kernel mailing list
> linux-arm-kernel@lists.infradead.org
> http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

Re: [PATCH 2/2] mmc: dw_mmc: add dw_mmc-k3 for k3 platform

[Zhangfei Gao]

Dear Rob

On Thu, Dec 5, 2013 at 10:29 PM, Rob Herring <robherring2@gmail.com> wrote:
> On Thu, Nov 7, 2013 at 11:38 PM, Zhangfei Gao <zhangfei.gao@linaro.org> wrote:

>> +++ b/Documentation/devicetree/bindings/mmc/k3-dw-mshc.txt
>> @@ -0,0 +1,83 @@
>> +* Hisilicon specific extensions to the Synopsys Designware Mobile
>> +  Storage Host Controller
>> +
>> +Read synopsis-dw-mshc.txt for more details
>> +The Synopsys 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 Synopsys dw mshc controller properties described
>> +by synopsis-dw-mshc.txt and the properties used by the Hisilicon specific
>> +extensions to the Synopsys Designware Mobile Storage Host Controller.
>> +
>> +Required Properties:
>> +
>> +* compatible: should be
>> +       - "hisilicon,hi4511-dw-mshc": for controllers with hi4511
>> +         specific extentions.
>> +* reg: should be address and size of mmc controller
>
> This should be covered by the base binding.
Yes, got it.

>
>> +* vmmc-supply: should be vmmc used in dwmmc
>> +* fifo-depth: should be provided if register can not provide correct value
>> +* tuning-table: should be array to tune mmc controller, including clock rate
>> +       to be set and values for setting optional register.
>
> Please define the size and what the values mean.
>
>> +
>> +Optional properties:
>> +
>> +/* These registers from pctrl node used for tuning mmc controller if required */
>> +* clken-reg: should be clock enable register and offset bit
>> +* drv-sel-reg: should be driver delay select register, start bit and bits numbers
>> +* sam-sel-reg: should be sample delay select register, start bit and bits numbers
>> +* div-reg: should be divider register, start bit and bits numbers
>
> Do these really vary on different boards or versions of the IP? If
> not, then put this information in the kernel.
>
> Otherwise, these need better description such as number of words and
> order of values.

Double checked these registers are not used in newer version.
Will follow Arnd's suggestion to abstract in clock.

>> +
>> +       /* Board portion */
>> +       dwmmc0@fcd03000 {
>> +               num-slots = <1>;
>> +               vmmc-supply = <&ldo12>;
>> +               fifo-depth = <0x100>;
>> +               supports-highspeed;
>
> Please reference that the binding uses standard SD properties (state
> which binding document).
Documentation/devicetree/bindings/mmc/synopsys-dw-mshc.txt is used for
base doc describing supports-highspeed.

>
>> +               pinctrl-names = "default";
>> +               pinctrl-0 = <&sd_pmx_pins &sd_cfg_func1 &sd_cfg_func2>;
>> +               slot@0 {
>> +                       reg = <0>;
>> +                       bus-width = <4>;
>> +                       disable-wp;
>> +                       cd-gpios = <&gpio10 3 0>;
>> +               };
>> +       };
>> +
>> +PCTRL: Peripheral misc control register
>
> Is this only MMC control bits? Seems like this belongs in its own doc.
OK, will move out.

Thanks

Re: [PATCH 2/2] mmc: dw_mmc: add dw_mmc-k3 for k3 platform

[Arnd Bergmann]

On Friday 08 November 2013, Zhangfei Gao wrote:
> Add dw_mmc-k3.c for k3v2, support sd/emmc
> 
> Signed-off-by: Zhangfei Gao <zhangfei.gao@linaro.org>
> Tested-by: Zhigang Wang <brooke.wangzhigang@huawei.com>

We are currently having the exact same discussion problem on the altera
version of dw_mmc: It seems that all this driver does in addition to the
common code is to have a custom way to set up the card clocks.

In both cases, there is a custom piece of IP logic providing this clock
(here it is in the "hisilicon,pctrl" node). IMHO that should be a proper
clock driver so you can use the existing call to clk_set_rate to set
it up and not need a special platform driver at all.

	Arnd

Re: [PATCH 2/2] mmc: dw_mmc: add dw_mmc-k3 for k3 platform

[Zhangfei Gao]

Dear Arnd

On Fri, Dec 6, 2013 at 9:39 AM, Arnd Bergmann <arnd@arndb.de> wrote:
> On Friday 08 November 2013, Zhangfei Gao wrote:
>> Add dw_mmc-k3.c for k3v2, support sd/emmc
>>
>> Signed-off-by: Zhangfei Gao <zhangfei.gao@linaro.org>
>> Tested-by: Zhigang Wang <brooke.wangzhigang@huawei.com>
>
> We are currently having the exact same discussion problem on the altera
> version of dw_mmc: It seems that all this driver does in addition to the
> common code is to have a custom way to set up the card clocks.
>
> In both cases, there is a custom piece of IP logic providing this clock
> (here it is in the "hisilicon,pctrl" node). IMHO that should be a proper
> clock driver so you can use the existing call to clk_set_rate to set
> it up and not need a special platform driver at all.
>

Thanks for the good suggestion, it can be abstracted to clock.

The issue for this version's ip is there are some registers need to be
set, including fixed factor and phase, which may be required to be
tuned, however it can be hide in clock.

Double checked next version's ip, it is more intelligent and only
fixed factor is required, more like a clock, and all these register
accessing are NOT required.

Will update new version to abstract these chip depended registers to clock.

Thanks

Re: [PATCH 2/2] mmc: dw_mmc: add dw_mmc-k3 for k3 platform

[Arnd Bergmann]

On Wednesday 11 December 2013, Zhangfei Gao wrote:
> Thanks for the good suggestion, it can be abstracted to clock.
> 
> The issue for this version's ip is there are some registers need to be
> set, including fixed factor and phase, which may be required to be
> tuned, however it can be hide in clock.
> 
> Double checked next version's ip, it is more intelligent and only
> fixed factor is required, more like a clock, and all these register
> accessing are NOT required.
> 
> Will update new version to abstract these chip depended registers to clock.

Ok, please stay in contact with Dinh Nguyen over this, since his side
is work-in-progress and we are still evaluating how to best encode
the phase setting in DT in a generic form.

	Arnd

Re: [PATCH 2/2] mmc: dw_mmc: add dw_mmc-k3 for k3 platform

[Dinh Nguyen]

On Wed, 2013-12-11 at 04:45 +0100, Arnd Bergmann wrote:
> On Wednesday 11 December 2013, Zhangfei Gao wrote:
> > Thanks for the good suggestion, it can be abstracted to clock.
> > 
> > The issue for this version's ip is there are some registers need to be
> > set, including fixed factor and phase, which may be required to be
> > tuned, however it can be hide in clock.
> > 
> > Double checked next version's ip, it is more intelligent and only
> > fixed factor is required, more like a clock, and all these register
> > accessing are NOT required.
> > 
> > Will update new version to abstract these chip depended registers to clock.
> 
> Ok, please stay in contact with Dinh Nguyen over this, since his side
> is work-in-progress and we are still evaluating how to best encode
> the phase setting in DT in a generic form.

Will the bindings "samsung,dw-mshc-sdr-timing" and
"samsung,dw-mshc-ddr-timing" also work for the k3 platform as well? I
think they should work, but just wanted to verify.

Heiko, on the rockchip platform, can you check to see if you have these
cclk_in_drv and cclk_sample settings for the phase shift of the clocks?
The SD/MMC IP version(2.40a) on Rockchip is the same version that is on
SOCFPGA, so I think these settings are there. They can be located in
some other "system" register block like SOCFPGA. The possible reason
that you have not need to set them in the Rockchip code could be that
the bootloader/firmware has already set them if you are booting from the
SD/MMC source. You may need to set the clock phase settings if you boot
from a non-SD source.

I think the reading of the "samsung,dw-mshc-ddr-timing" and
"samsung,dw-msh-sdr-timing" bindings can also be moved into the generic
dw_mmc driver if the above 2 conditions are correct.

Thanks,
Din
> 
> 	Arnd
> 

Re: [PATCH 2/2] mmc: dw_mmc: add dw_mmc-k3 for k3 platform

[Heiko Stübner]

Am Mittwoch, 11. Dezember 2013, 19:48:10 schrieb Dinh Nguyen:
> On Wed, 2013-12-11 at 04:45 +0100, Arnd Bergmann wrote:
> > On Wednesday 11 December 2013, Zhangfei Gao wrote:
> > > Thanks for the good suggestion, it can be abstracted to clock.
> > > 
> > > The issue for this version's ip is there are some registers need to be
> > > set, including fixed factor and phase, which may be required to be
> > > tuned, however it can be hide in clock.
> > > 
> > > Double checked next version's ip, it is more intelligent and only
> > > fixed factor is required, more like a clock, and all these register
> > > accessing are NOT required.
> > > 
> > > Will update new version to abstract these chip depended registers to
> > > clock.
> > 
> > Ok, please stay in contact with Dinh Nguyen over this, since his side
> > is work-in-progress and we are still evaluating how to best encode
> > the phase setting in DT in a generic form.
> 
> Will the bindings "samsung,dw-mshc-sdr-timing" and
> "samsung,dw-mshc-ddr-timing" also work for the k3 platform as well? I
> think they should work, but just wanted to verify.
> 
> Heiko, on the rockchip platform, can you check to see if you have these
> cclk_in_drv and cclk_sample settings for the phase shift of the clocks?
> The SD/MMC IP version(2.40a) on Rockchip is the same version that is on
> SOCFPGA, so I think these settings are there. They can be located in
> some other "system" register block like SOCFPGA. The possible reason
> that you have not need to set them in the Rockchip code could be that
> the bootloader/firmware has already set them if you are booting from the
> SD/MMC source. You may need to set the clock phase settings if you boot
> from a non-SD source.
> 
> I think the reading of the "samsung,dw-mshc-ddr-timing" and
> "samsung,dw-msh-sdr-timing" bindings can also be moved into the generic
> dw_mmc driver if the above 2 conditions are correct.

The mmc-controller part of the rk-manual that is flying around the net, 
mentions these cclk_in_drv and cclk_in_sample as coming from somewhere, but 
sadly never where to get/set/access them.

The doc of the USE_HOLD_REG bit also mention the required settings of these 
cclk_in_* but nowhere is a word about the where to be found.

And the manual is notable for lacking all information related to the clock 
controller. And I also wasn't able to find any other indication where this 
might be set.


Heiko