[PATCH 1/3] mmc: dw_mmc: use slot-gpio to handle cd pin

[PATCH 1/3] mmc: dw_mmc: use slot-gpio to handle cd pin

[Zhangfei Gao]

Suggested by Jaehoon: Use slot-gpio to handle cd-gpio
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>
Acked-by: Jaehoon Chung <jh80.chung@samsung.com>
---
 drivers/mmc/host/dw_mmc.c |   48 +++++++++++++++++++++++++++++++++------------
 1 file changed, 35 insertions(+), 13 deletions(-)

diff --git a/drivers/mmc/host/dw_mmc.c b/drivers/mmc/host/dw_mmc.c
index 4bce0deec362..a776f24f4311 100644
--- a/drivers/mmc/host/dw_mmc.c
+++ b/drivers/mmc/host/dw_mmc.c
@@ -36,6 +36,7 @@
 #include <linux/workqueue.h>
 #include <linux/of.h>
 #include <linux/of_gpio.h>
+#include <linux/mmc/slot-gpio.h>
 
 #include "dw_mmc.h"
 
@@ -1032,20 +1033,26 @@ 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;
 
-	if (present)
+	if (present) {
+		set_bit(DW_MMC_CARD_PRESENT, &slot->flags);
 		dev_dbg(&mmc->class_dev, "card is present\n");
-	else
+	} else {
+		clear_bit(DW_MMC_CARD_PRESENT, &slot->flags);
 		dev_dbg(&mmc->class_dev, "card is not present\n");
+	}
 
 	return present;
 }
@@ -1926,10 +1933,6 @@ static void dw_mci_work_routine_card(struct work_struct *work)
 			/* Card change detected */
 			slot->last_detect_state = present;
 
-			/* Mark card as present if applicable */
-			if (present != 0)
-				set_bit(DW_MMC_CARD_PRESENT, &slot->flags);
-
 			/* Clean up queue if present */
 			mrq = slot->mrq;
 			if (mrq) {
@@ -1977,8 +1980,6 @@ static void dw_mci_work_routine_card(struct work_struct *work)
 
 			/* Power down slot */
 			if (present == 0) {
-				clear_bit(DW_MMC_CARD_PRESENT, &slot->flags);
-
 				/* Clear down the FIFO */
 				dw_mci_fifo_reset(host);
 #ifdef CONFIG_MMC_DW_IDMAC
@@ -2079,6 +2080,26 @@ 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 void 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;
+
+	gpio = of_get_named_gpio(np, "cd-gpios", 0);
+
+	/* Having a missing entry is valid; return silently */
+	if (!gpio_is_valid(gpio))
+		return;
+
+	if (mmc_gpio_request_cd(mmc, gpio, 0))
+		dev_warn(dev, "gpio [%d] request failed\n", gpio);
+}
 #else /* CONFIG_OF */
 static int dw_mci_of_get_slot_quirks(struct device *dev, u8 slot)
 {
@@ -2096,6 +2117,11 @@ static int dw_mci_of_get_wp_gpio(struct device *dev, u8 slot)
 {
 	return -EINVAL;
 }
+static void dw_mci_of_get_cd_gpio(struct device *dev, u8 slot,
+					struct mmc_host *mmc)
+{
+	return;
+}
 #endif /* CONFIG_OF */
 
 static int dw_mci_init_slot(struct dw_mci *host, unsigned int id)
@@ -2197,12 +2223,8 @@ static int dw_mci_init_slot(struct dw_mci *host, unsigned int id)
 #endif /* CONFIG_MMC_DW_IDMAC */
 	}
 
-	if (dw_mci_get_cd(mmc))
-		set_bit(DW_MMC_CARD_PRESENT, &slot->flags);
-	else
-		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 1/3] mmc: dw_mmc: use slot-gpio to handle cd pin

[Zhangfei Gao]

Suggested by Jaehoon: Use slot-gpio to handle cd-gpio
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>
Acked-by: Jaehoon Chung <jh80.chung@samsung.com>
---
 drivers/mmc/host/dw_mmc.c |   48 +++++++++++++++++++++++++++++++++------------
 1 file changed, 35 insertions(+), 13 deletions(-)

diff --git a/drivers/mmc/host/dw_mmc.c b/drivers/mmc/host/dw_mmc.c
index 4bce0deec362..a776f24f4311 100644
--- a/drivers/mmc/host/dw_mmc.c
+++ b/drivers/mmc/host/dw_mmc.c
@@ -36,6 +36,7 @@
 #include <linux/workqueue.h>
 #include <linux/of.h>
 #include <linux/of_gpio.h>
+#include <linux/mmc/slot-gpio.h>
 
 #include "dw_mmc.h"
 
@@ -1032,20 +1033,26 @@ 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;
 
-	if (present)
+	if (present) {
+		set_bit(DW_MMC_CARD_PRESENT, &slot->flags);
 		dev_dbg(&mmc->class_dev, "card is present\n");
-	else
+	} else {
+		clear_bit(DW_MMC_CARD_PRESENT, &slot->flags);
 		dev_dbg(&mmc->class_dev, "card is not present\n");
+	}
 
 	return present;
 }
@@ -1926,10 +1933,6 @@ static void dw_mci_work_routine_card(struct work_struct *work)
 			/* Card change detected */
 			slot->last_detect_state = present;
 
-			/* Mark card as present if applicable */
-			if (present != 0)
-				set_bit(DW_MMC_CARD_PRESENT, &slot->flags);
-
 			/* Clean up queue if present */
 			mrq = slot->mrq;
 			if (mrq) {
@@ -1977,8 +1980,6 @@ static void dw_mci_work_routine_card(struct work_struct *work)
 
 			/* Power down slot */
 			if (present == 0) {
-				clear_bit(DW_MMC_CARD_PRESENT, &slot->flags);
-
 				/* Clear down the FIFO */
 				dw_mci_fifo_reset(host);
 #ifdef CONFIG_MMC_DW_IDMAC
@@ -2079,6 +2080,26 @@ 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 void 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;
+
+	gpio = of_get_named_gpio(np, "cd-gpios", 0);
+
+	/* Having a missing entry is valid; return silently */
+	if (!gpio_is_valid(gpio))
+		return;
+
+	if (mmc_gpio_request_cd(mmc, gpio, 0))
+		dev_warn(dev, "gpio [%d] request failed\n", gpio);
+}
 #else /* CONFIG_OF */
 static int dw_mci_of_get_slot_quirks(struct device *dev, u8 slot)
 {
@@ -2096,6 +2117,11 @@ static int dw_mci_of_get_wp_gpio(struct device *dev, u8 slot)
 {
 	return -EINVAL;
 }
+static void dw_mci_of_get_cd_gpio(struct device *dev, u8 slot,
+					struct mmc_host *mmc)
+{
+	return;
+}
 #endif /* CONFIG_OF */
 
 static int dw_mci_init_slot(struct dw_mci *host, unsigned int id)
@@ -2197,12 +2223,8 @@ static int dw_mci_init_slot(struct dw_mci *host, unsigned int id)
 #endif /* CONFIG_MMC_DW_IDMAC */
 	}
 
-	if (dw_mci_get_cd(mmc))
-		set_bit(DW_MMC_CARD_PRESENT, &slot->flags);
-	else
-		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 v6 0/3] mmc: dw_mmc: add dw_mmc-k3

[Zhangfei Gao]

v6:
0002:
Jaehoon pointed HIGHSPEED cap can be omitted if supports-highspeed is defined.
Seungwon mentioned clk operation should be called after suspend.
Remove k3_dwmmc_caps

V5:
0002:
Follow advice from Arnd, 
Update dt descirption and use of_property_for_each_u32 to get table number.

v4:
Follow Arnd's suggestion abstracting specific tuning to clock,
also because new version ip use different method and not use same tuning registers.

0001 acked by Jaehoon

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 (3):
  mmc: dw_mmc: use slot-gpio to handle cd pin
  mmc: dw_mmc: add dw_mmc-k3 for k3 platform
  clk: hisilicon: add hi3620_mmc_clks

 .../bindings/arm/hisilicon/hisilicon.txt           |   14 ++
 .../devicetree/bindings/clock/hi3620-clock.txt     |    1 +
 .../devicetree/bindings/mmc/k3-dw-mshc.txt         |   60 +++++
 drivers/clk/hisilicon/clk-hi3620.c                 |  262 ++++++++++++++++++++
 drivers/mmc/host/Kconfig                           |   10 +
 drivers/mmc/host/Makefile                          |    1 +
 drivers/mmc/host/dw_mmc-k3.c                       |  126 ++++++++++
 drivers/mmc/host/dw_mmc.c                          |   48 +++-
 include/dt-bindings/clock/hi3620-clock.h           |    5 +
 9 files changed, 514 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 1/3] mmc: dw_mmc: use slot-gpio to handle cd pin

[Zhangfei Gao]

Suggested by Jaehoon: Use slot-gpio to handle cd-gpio
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>
Acked-by: Jaehoon Chung <jh80.chung@samsung.com>
---
 drivers/mmc/host/dw_mmc.c |   48 +++++++++++++++++++++++++++++++++------------
 1 file changed, 35 insertions(+), 13 deletions(-)

diff --git a/drivers/mmc/host/dw_mmc.c b/drivers/mmc/host/dw_mmc.c
index 4bce0deec362..a776f24f4311 100644
--- a/drivers/mmc/host/dw_mmc.c
+++ b/drivers/mmc/host/dw_mmc.c
@@ -36,6 +36,7 @@
 #include <linux/workqueue.h>
 #include <linux/of.h>
 #include <linux/of_gpio.h>
+#include <linux/mmc/slot-gpio.h>
 
 #include "dw_mmc.h"
 
@@ -1032,20 +1033,26 @@ 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;
 
-	if (present)
+	if (present) {
+		set_bit(DW_MMC_CARD_PRESENT, &slot->flags);
 		dev_dbg(&mmc->class_dev, "card is present\n");
-	else
+	} else {
+		clear_bit(DW_MMC_CARD_PRESENT, &slot->flags);
 		dev_dbg(&mmc->class_dev, "card is not present\n");
+	}
 
 	return present;
 }
@@ -1926,10 +1933,6 @@ static void dw_mci_work_routine_card(struct work_struct *work)
 			/* Card change detected */
 			slot->last_detect_state = present;
 
-			/* Mark card as present if applicable */
-			if (present != 0)
-				set_bit(DW_MMC_CARD_PRESENT, &slot->flags);
-
 			/* Clean up queue if present */
 			mrq = slot->mrq;
 			if (mrq) {
@@ -1977,8 +1980,6 @@ static void dw_mci_work_routine_card(struct work_struct *work)
 
 			/* Power down slot */
 			if (present == 0) {
-				clear_bit(DW_MMC_CARD_PRESENT, &slot->flags);
-
 				/* Clear down the FIFO */
 				dw_mci_fifo_reset(host);
 #ifdef CONFIG_MMC_DW_IDMAC
@@ -2079,6 +2080,26 @@ 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 void 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;
+
+	gpio = of_get_named_gpio(np, "cd-gpios", 0);
+
+	/* Having a missing entry is valid; return silently */
+	if (!gpio_is_valid(gpio))
+		return;
+
+	if (mmc_gpio_request_cd(mmc, gpio, 0))
+		dev_warn(dev, "gpio [%d] request failed\n", gpio);
+}
 #else /* CONFIG_OF */
 static int dw_mci_of_get_slot_quirks(struct device *dev, u8 slot)
 {
@@ -2096,6 +2117,11 @@ static int dw_mci_of_get_wp_gpio(struct device *dev, u8 slot)
 {
 	return -EINVAL;
 }
+static void dw_mci_of_get_cd_gpio(struct device *dev, u8 slot,
+					struct mmc_host *mmc)
+{
+	return;
+}
 #endif /* CONFIG_OF */
 
 static int dw_mci_init_slot(struct dw_mci *host, unsigned int id)
@@ -2197,12 +2223,8 @@ static int dw_mci_init_slot(struct dw_mci *host, unsigned int id)
 #endif /* CONFIG_MMC_DW_IDMAC */
 	}
 
-	if (dw_mci_get_cd(mmc))
-		set_bit(DW_MMC_CARD_PRESENT, &slot->flags);
-	else
-		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/3] 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>
Signed-off-by: Zhigang Wang <brooke.wangzhigang@huawei.com>
---
 .../devicetree/bindings/mmc/k3-dw-mshc.txt         |   60 ++++++++++
 drivers/mmc/host/Kconfig                           |   10 ++
 drivers/mmc/host/Makefile                          |    1 +
 drivers/mmc/host/dw_mmc-k3.c                       |  126 ++++++++++++++++++++
 4 files changed, 197 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 000000000000..d7e2d7f159bb
--- /dev/null
+++ b/Documentation/devicetree/bindings/mmc/k3-dw-mshc.txt
@@ -0,0 +1,60 @@
+* Hisilicon specific extensions to the Synopsys Designware Mobile
+  Storage Host Controller
+
+Read synopsys-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 synopsys-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 one of the following.
+  - "hisilicon,hi4511-dw-mshc": for controllers with hi4511 specific extentions.
+
+* clock-freq-table: should be the frequency (in Hz) array of the ciu clock
+	in each	supported mode.
+	0. CIU clock rate in Hz for DS mode
+	1. CIU clock rate in Hz for MMC HS mode
+	2. CIU clock rate in Hz for SD HS mode
+	3. CIU clock rate in Hz for SDR12 mode
+	4. CIU clock rate in Hz for SDR25 mode
+	5. CIU clock rate in Hz for SDR50 mode
+	6. CIU clock rate in Hz for SDR104 mode
+	7. CIU clock rate in Hz for DDR50 mode
+	8. CIU clock rate in Hz for HS200 mode
+
+Example:
+
+	/* for Hi3620 */
+
+	/* 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 = <&mmc_clock HI3620_SD_CIUCLK>, <&clock HI3620_DDRC_PER_CLK>;
+		clock-names = "ciu", "biu";
+		clock-freq-table =
+		<25000000 0 50000000 25000000 50000000 100000000 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>;
+		};
+	};
diff --git a/drivers/mmc/host/Kconfig b/drivers/mmc/host/Kconfig
index 7fc5099e44b2..45aaa2de0f58 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 c41d0c364509..64f5f8d35839 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 000000000000..89f69428e3dc
--- /dev/null
+++ b/drivers/mmc/host/dw_mmc-k3.c
@@ -0,0 +1,126 @@
+/*
+ * 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 MAX_NUMS	10
+struct dw_mci_k3_priv_data {
+	u32	clk_table[MAX_NUMS];
+};
+
+static void dw_mci_k3_set_ios(struct dw_mci *host, struct mmc_ios *ios)
+{
+	struct dw_mci_k3_priv_data *priv = host->priv;
+	u32 rate = priv->clk_table[ios->timing];
+	int ret;
+
+	ret = clk_set_rate(host->ciu_clk, rate);
+	if (ret)
+		dev_warn(host->dev, "failed to set clock rate %uHz\n", rate);
+
+	host->bus_hz = clk_get_rate(host->ciu_clk);
+}
+
+static int dw_mci_k3_parse_dt(struct dw_mci *host)
+{
+	struct dw_mci_k3_priv_data *priv;
+	struct device_node *node = host->dev->of_node;
+	struct property *prop;
+	const __be32 *cur;
+	u32 val, num = 0;
+
+	priv = devm_kzalloc(host->dev, sizeof(*priv), GFP_KERNEL);
+	if (!priv) {
+		dev_err(host->dev, "mem alloc failed for private data\n");
+		return -ENOMEM;
+	}
+	host->priv = priv;
+
+	of_property_for_each_u32(node, "clock-freq-table", prop, cur, val) {
+		if (num >= MAX_NUMS)
+			break;
+		priv->clk_table[num++] = val;
+	}
+	return 0;
+}
+
+static const struct dw_mci_drv_data k3_drv_data = {
+	.set_ios		= dw_mci_k3_set_ios,
+	.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);
+	int ret = 0;
+
+	ret = dw_mci_suspend(host);
+	if (!ret)
+		clk_disable_unprepare(host->ciu_clk);
+
+	return ret;
+}
+
+static int dw_mci_k3_resume(struct device *dev)
+{
+	struct dw_mci *host = dev_get_drvdata(dev);
+	int ret = 0;
+
+	ret = clk_prepare_enable(host->ciu_clk);
+	if (ret) {
+		dev_err(host->dev, "failed to enable ciu clock\n");
+		return ret;
+	}
+
+	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

[PATCH 3/3] clk: hisilicon: add hi3620_mmc_clks

[Zhangfei Gao]

Suggest by Arnd: abstract mmc tuning as clock behavior,
also because different soc have different tuning method and registers.
hi3620_mmc_clks is added to handle mmc clock specifically on hi3620.

Signed-off-by: Zhangfei Gao <zhangfei.gao@linaro.org>
---
 .../bindings/arm/hisilicon/hisilicon.txt           |   14 ++
 .../devicetree/bindings/clock/hi3620-clock.txt     |    1 +
 drivers/clk/hisilicon/clk-hi3620.c                 |  262 ++++++++++++++++++++
 include/dt-bindings/clock/hi3620-clock.h           |    5 +
 4 files changed, 282 insertions(+)

diff --git a/Documentation/devicetree/bindings/arm/hisilicon/hisilicon.txt b/Documentation/devicetree/bindings/arm/hisilicon/hisilicon.txt
index 8c7a4653508d..df0a452b8526 100644
--- a/Documentation/devicetree/bindings/arm/hisilicon/hisilicon.txt
+++ b/Documentation/devicetree/bindings/arm/hisilicon/hisilicon.txt
@@ -30,3 +30,17 @@ Example:
 		resume-offset = <0x308>;
 		reboot-offset = <0x4>;
 	};
+
+PCTRL: Peripheral misc control register
+
+Required Properties:
+- compatible: "hisilicon,pctrl"
+- reg: Address and size of pctrl.
+
+Example:
+
+	/* for Hi3620 */
+	pctrl: pctrl@fca09000 {
+		compatible = "hisilicon,pctrl";
+		reg = <0xfca09000 0x1000>;
+	};
diff --git a/Documentation/devicetree/bindings/clock/hi3620-clock.txt b/Documentation/devicetree/bindings/clock/hi3620-clock.txt
index 4b71ab41be53..dad6269f52c5 100644
--- a/Documentation/devicetree/bindings/clock/hi3620-clock.txt
+++ b/Documentation/devicetree/bindings/clock/hi3620-clock.txt
@@ -7,6 +7,7 @@ Required Properties:
 
 - compatible: should be one of the following.
   - "hisilicon,hi3620-clock" - controller compatible with Hi3620 SoC.
+  - "hisilicon,hi3620-mmc-clock" - controller specific for Hi3620 mmc.
 
 - reg: physical base address of the controller and length of memory mapped
   region.
diff --git a/drivers/clk/hisilicon/clk-hi3620.c b/drivers/clk/hisilicon/clk-hi3620.c
index f24ad6a3a797..e47a4a659df7 100644
--- a/drivers/clk/hisilicon/clk-hi3620.c
+++ b/drivers/clk/hisilicon/clk-hi3620.c
@@ -240,3 +240,265 @@ static void __init hi3620_clk_init(struct device_node *np)
 				   base);
 }
 CLK_OF_DECLARE(hi3620_clk, "hisilicon,hi3620-clock", hi3620_clk_init);
+
+struct hisi_mmc_clock {
+	unsigned int		id;
+	const char		*name;
+	const char		*parent_name;
+	unsigned long		flags;
+	u32			clken_reg;
+	u32			clken_bit;
+	u32			div_reg;
+	u32			div_off;
+	u32			div_bits;
+	u32			drv_reg;
+	u32			drv_off;
+	u32			drv_bits;
+	u32			sam_reg;
+	u32			sam_off;
+	u32			sam_bits;
+};
+
+struct clk_mmc {
+	struct clk_hw	hw;
+	u32		id;
+	void __iomem	*clken_reg;
+	u32		clken_bit;
+	void __iomem	*div_reg;
+	u32		div_off;
+	u32		div_bits;
+	void __iomem	*drv_reg;
+	u32		drv_off;
+	u32		drv_bits;
+	void __iomem	*sam_reg;
+	u32		sam_off;
+	u32		sam_bits;
+};
+
+#define to_mmc(_hw) container_of(_hw, struct clk_mmc, hw)
+
+static struct hisi_mmc_clock hi3620_mmc_clks[] __initdata = {
+	{ HI3620_SD_CIUCLK,	"sd_bclk1", "sd_clk", CLK_SET_RATE_PARENT, 0x1f8, 0, 0x1f8, 1, 3, 0x1f8, 4, 4, 0x1f8, 8, 4},
+	{ HI3620_MMC_CIUCLK1,   "mmc_bclk1", "mmc_clk1", CLK_SET_RATE_PARENT, 0x1f8, 12, 0x1f8, 13, 3, 0x1f8, 16, 4, 0x1f8, 20, 4},
+	{ HI3620_MMC_CIUCLK2,   "mmc_bclk2", "mmc_clk2", CLK_SET_RATE_PARENT, 0x1f8, 24, 0x1f8, 25, 3, 0x1f8, 28, 4, 0x1fc, 0, 4},
+	{ HI3620_MMC_CIUCLK3,   "mmc_bclk3", "mmc_clk3", CLK_SET_RATE_PARENT, 0x1fc, 4, 0x1fc, 5, 3, 0x1fc, 8, 4, 0x1fc, 12, 4},
+};
+
+static unsigned long mmc_clk_recalc_rate(struct clk_hw *hw,
+		       unsigned long parent_rate)
+{
+	switch (parent_rate) {
+	case 26000000:
+		return 13000000;
+	case 180000000:
+		return 25000000;
+	case 360000000:
+		return 50000000;
+	case 720000000:
+		return 100000000;
+	default:
+		return parent_rate;
+	}
+}
+
+static long mmc_clk_determine_rate(struct clk_hw *hw, unsigned long rate,
+			      unsigned long *best_parent_rate,
+			      struct clk **best_parent_p)
+{
+	unsigned long best = 0;
+
+	if (rate <= 13000000) {
+		rate = 13000000;
+		best = 26000000;
+	} else if (rate <= 26000000) {
+		rate = 25000000;
+		best = 180000000;
+	} else if (rate <= 52000000) {
+		rate = 50000000;
+		best = 360000000;
+	} else if (rate <= 100000000) {
+		rate = 100000000;
+		best = 720000000;
+	}
+	*best_parent_rate = best;
+	return rate;
+}
+
+static u32 mmc_clk_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 int mmc_clk_set_timing(struct clk_hw *hw, unsigned long rate)
+{
+	struct clk_mmc *mclk = to_mmc(hw);
+	unsigned long flags;
+	u32 sam, drv, div, val;
+	static DEFINE_SPINLOCK(mmc_clk_lock);
+
+	switch (rate) {
+	case 13000000:
+		sam = 3;
+		drv = 1;
+		div = 1;
+		break;
+	case 25000000:
+		sam = 13;
+		drv = 6;
+		div = 6;
+		break;
+	case 50000000:
+		sam = 3;
+		drv = 6;
+		div = 6;
+		break;
+	case 100000000:
+		sam = 6;
+		drv = 4;
+		div = 6;
+		break;
+	default:
+		return -EINVAL;
+	}
+
+	spin_lock_irqsave(&mmc_clk_lock, flags);
+
+	val = readl_relaxed(mclk->clken_reg);
+	val &= ~(1 << mclk->clken_bit);
+	writel_relaxed(val, mclk->clken_reg);
+
+	val = readl_relaxed(mclk->sam_reg);
+	val = mmc_clk_delay(val, sam, mclk->sam_off, mclk->sam_bits);
+	writel_relaxed(val, mclk->sam_reg);
+
+	val = readl_relaxed(mclk->drv_reg);
+	val = mmc_clk_delay(val, drv, mclk->drv_off, mclk->drv_bits);
+	writel_relaxed(val, mclk->drv_reg);
+
+	val = readl_relaxed(mclk->div_reg);
+	val = mmc_clk_delay(val, div, mclk->div_off, mclk->div_bits);
+	writel_relaxed(val, mclk->div_reg);
+
+	val = readl_relaxed(mclk->clken_reg);
+	val |= 1 << mclk->clken_bit;
+	writel_relaxed(val, mclk->clken_reg);
+
+	spin_unlock_irqrestore(&mmc_clk_lock, flags);
+
+	return 0;
+}
+
+static int mmc_clk_prepare(struct clk_hw *hw)
+{
+	struct clk_mmc *mclk = to_mmc(hw);
+	unsigned long rate;
+
+	if (mclk->id == HI3620_SD_CIUCLK)
+		rate = 13000000;
+	else
+		rate = 25000000;
+
+	return mmc_clk_set_timing(hw, rate);
+}
+
+static int mmc_clk_set_rate(struct clk_hw *hw, unsigned long rate,
+			     unsigned long parent_rate)
+{
+	return mmc_clk_set_timing(hw, rate);
+}
+
+static struct clk_ops clk_mmc_ops = {
+	.prepare = mmc_clk_prepare,
+	.determine_rate = mmc_clk_determine_rate,
+	.set_rate = mmc_clk_set_rate,
+	.recalc_rate = mmc_clk_recalc_rate,
+};
+
+static struct clk *hisi_register_clk_mmc(struct hisi_mmc_clock *mmc_clk,
+			void __iomem *base, struct device_node *np)
+{
+	struct clk_mmc *mclk;
+	struct clk *clk;
+	struct clk_init_data init;
+
+	mclk = kzalloc(sizeof(*mclk), GFP_KERNEL);
+	if (!mclk) {
+		pr_err("%s: fail to allocate mmc clk\n", __func__);
+		return ERR_PTR(-ENOMEM);
+	}
+
+	init.name = mmc_clk->name;
+	init.ops = &clk_mmc_ops;
+	init.flags = mmc_clk->flags | CLK_IS_BASIC;
+	init.parent_names = (mmc_clk->parent_name ? &mmc_clk->parent_name : NULL);
+	init.num_parents = (mmc_clk->parent_name ? 1 : 0);
+	mclk->hw.init = &init;
+
+	mclk->id = mmc_clk->id;
+	mclk->clken_reg = base + mmc_clk->clken_reg;
+	mclk->clken_bit = mmc_clk->clken_bit;
+	mclk->div_reg = base + mmc_clk->div_reg;
+	mclk->div_off = mmc_clk->div_off;
+	mclk->div_bits = mmc_clk->div_bits;
+	mclk->drv_reg = base + mmc_clk->drv_reg;
+	mclk->drv_off = mmc_clk->drv_off;
+	mclk->drv_bits = mmc_clk->drv_bits;
+	mclk->sam_reg = base + mmc_clk->sam_reg;
+	mclk->sam_off = mmc_clk->sam_off;
+	mclk->sam_bits = mmc_clk->sam_bits;
+
+	clk = clk_register(NULL, &mclk->hw);
+	if (WARN_ON(IS_ERR(clk)))
+		kfree(mclk);
+	return clk;
+}
+
+static void __init hi3620_mmc_clk_init(struct device_node *node)
+{
+	void __iomem *base;
+	int i, num = ARRAY_SIZE(hi3620_mmc_clks);
+	struct clk_onecell_data *clk_data;
+
+	if (!node) {
+		pr_err("failed to find pctrl node in DTS\n");
+		return;
+	}
+
+	base = of_iomap(node, 0);
+	if (!base) {
+		pr_err("failed to map pctrl\n");
+		return;
+	}
+
+	clk_data = kzalloc(sizeof(*clk_data), GFP_KERNEL);
+	if (WARN_ON(!clk_data))
+		return;
+
+	clk_data->clks = kzalloc(sizeof(struct clk *) * num, GFP_KERNEL);
+	if (!clk_data->clks) {
+		pr_err("%s: fail to allocate mmc clk\n", __func__);
+		return;
+	}
+
+	for (i = 0; i < num; i++) {
+		struct hisi_mmc_clock *mmc_clk = &hi3620_mmc_clks[i];
+		clk_data->clks[mmc_clk->id] =
+			hisi_register_clk_mmc(mmc_clk, base, node);
+	}
+
+	clk_data->clk_num = num;
+	of_clk_add_provider(node, of_clk_src_onecell_get, clk_data);
+}
+
+CLK_OF_DECLARE(hi3620_mmc_clk, "hisilicon,hi3620-mmc-clock", hi3620_mmc_clk_init);
diff --git a/include/dt-bindings/clock/hi3620-clock.h b/include/dt-bindings/clock/hi3620-clock.h
index 6eaa6a45e110..21b9d0e2eb0c 100644
--- a/include/dt-bindings/clock/hi3620-clock.h
+++ b/include/dt-bindings/clock/hi3620-clock.h
@@ -147,6 +147,11 @@
 #define HI3620_MMC_CLK3		217
 #define HI3620_MCU_CLK		218
 
+#define HI3620_SD_CIUCLK	0
+#define HI3620_MMC_CIUCLK1	1
+#define HI3620_MMC_CIUCLK2	2
+#define HI3620_MMC_CIUCLK3	3
+
 #define HI3620_NR_CLKS		219
 
 #endif	/* __DTS_HI3620_CLOCK_H */
-- 
1.7.9.5

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

[Arnd Bergmann]

On Saturday 28 December 2013, Zhangfei Gao wrote:
> Add dw_mmc-k3.c for k3v2, support sd/emmc
> 
> Signed-off-by: Zhangfei Gao <zhangfei.gao@linaro.org>
> Signed-off-by: Zhigang Wang <brooke.wangzhigang@huawei.com>
> ---
>  .../devicetree/bindings/mmc/k3-dw-mshc.txt         |   60 ++++++++++
>  drivers/mmc/host/Kconfig                           |   10 ++
>  drivers/mmc/host/Makefile                          |    1 +
>  drivers/mmc/host/dw_mmc-k3.c                       |  126 ++++++++++++++++++++
>  4 files changed, 197 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 000000000000..d7e2d7f159bb
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/mmc/k3-dw-mshc.txt
> @@ -0,0 +1,60 @@
> +* Hisilicon specific extensions to the Synopsys Designware Mobile
> +  Storage Host Controller
> +
> +Read synopsys-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 synopsys-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 one of the following.
> +  - "hisilicon,hi4511-dw-mshc": for controllers with hi4511 specific extentions.

I wonder if this is actually a different variant of the mshc hardware, or just
wired up in a different way. Do you know details?

Since the only difference in the binding is the presence of the "clock-freq-table"
property, we could also make this property generic for the mshc driver and use
it if present but fall back to the normal behavior when it is absent.

> +* clock-freq-table: should be the frequency (in Hz) array of the ciu clock
> +	in each	supported mode.
> +	0. CIU clock rate in Hz for DS mode
> +	1. CIU clock rate in Hz for MMC HS mode
> +	2. CIU clock rate in Hz for SD HS mode
> +	3. CIU clock rate in Hz for SDR12 mode
> +	4. CIU clock rate in Hz for SDR25 mode
> +	5. CIU clock rate in Hz for SDR50 mode
> +	6. CIU clock rate in Hz for SDR104 mode
> +	7. CIU clock rate in Hz for DDR50 mode
> +	8. CIU clock rate in Hz for HS200 mode

This looks god now.

> +static void dw_mci_k3_set_ios(struct dw_mci *host, struct mmc_ios *ios)
> +{
> +	struct dw_mci_k3_priv_data *priv = host->priv;
> +	u32 rate = priv->clk_table[ios->timing];
> +	int ret;

I think this should have some range checking to see if the mode that is
being set had a clock frequency set in the DT.

> +
> +	ret = clk_set_rate(host->ciu_clk, rate);
> +	if (ret)
> +		dev_warn(host->dev, "failed to set clock rate %uHz\n", rate);
> +
> +	host->bus_hz = clk_get_rate(host->ciu_clk);
> +}

Why do you call clk_get_rate() here, shouldn't it always be the same
rate that you have just set?

> +static int dw_mci_k3_parse_dt(struct dw_mci *host)
> +{
> +	struct dw_mci_k3_priv_data *priv;
> +	struct device_node *node = host->dev->of_node;
> +	struct property *prop;
> +	const __be32 *cur;
> +	u32 val, num = 0;
> +
> +	priv = devm_kzalloc(host->dev, sizeof(*priv), GFP_KERNEL);
> +	if (!priv) {
> +		dev_err(host->dev, "mem alloc failed for private data\n");
> +		return -ENOMEM;
> +	}
> +	host->priv = priv;
> +
> +	of_property_for_each_u32(node, "clock-freq-table", prop, cur, val) {
> +		if (num >= MAX_NUMS)
> +			break;
> +		priv->clk_table[num++] = val;
> +	}
> +	return 0;
> +}

If we make this property part of the generic binding, this function could
also get moved to the main dw_mci driver.

> +static int dw_mci_k3_suspend(struct device *dev)
> +{
> +	struct dw_mci *host = dev_get_drvdata(dev);
> +	int ret = 0;
> +
> +	ret = dw_mci_suspend(host);

You should never initialize local variables when they are set later in the
function (the ret = 0 part above). For more complex functions, this prevents
gcc from warning you about accidentally uninitialized uses.

> +	if (!ret)
> +		clk_disable_unprepare(host->ciu_clk);
> +
> +	return ret;
> +}

The suspend/resume code also looks very generic. Can't we make these the
default for dw-mci? If you do both, you won't even need a k3 specific driver.
I think in general we should try hard to add code like this to the common
driver when there is a chance that it can be shared with other platforms.

	Arnd

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

[Jaehoon Chung]

On 12/30/2013 06:05 AM, Arnd Bergmann wrote:
> On Saturday 28 December 2013, Zhangfei Gao wrote:
>> Add dw_mmc-k3.c for k3v2, support sd/emmc
>>
>> Signed-off-by: Zhangfei Gao <zhangfei.gao@linaro.org>
>> Signed-off-by: Zhigang Wang <brooke.wangzhigang@huawei.com>
>> ---
>>  .../devicetree/bindings/mmc/k3-dw-mshc.txt         |   60 ++++++++++
>>  drivers/mmc/host/Kconfig                           |   10 ++
>>  drivers/mmc/host/Makefile                          |    1 +
>>  drivers/mmc/host/dw_mmc-k3.c                       |  126 ++++++++++++++++++++
>>  4 files changed, 197 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 000000000000..d7e2d7f159bb
>> --- /dev/null
>> +++ b/Documentation/devicetree/bindings/mmc/k3-dw-mshc.txt
>> @@ -0,0 +1,60 @@
>> +* Hisilicon specific extensions to the Synopsys Designware Mobile
>> +  Storage Host Controller
>> +
>> +Read synopsys-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 synopsys-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 one of the following.
>> +  - "hisilicon,hi4511-dw-mshc": for controllers with hi4511 specific extentions.
> 
> I wonder if this is actually a different variant of the mshc hardware, or just
> wired up in a different way. Do you know details?
> 
> Since the only difference in the binding is the presence of the "clock-freq-table"
> property, we could also make this property generic for the mshc driver and use
> it if present but fall back to the normal behavior when it is absent.
> 
>> +* clock-freq-table: should be the frequency (in Hz) array of the ciu clock
>> +	in each	supported mode.
>> +	0. CIU clock rate in Hz for DS mode
>> +	1. CIU clock rate in Hz for MMC HS mode
>> +	2. CIU clock rate in Hz for SD HS mode
>> +	3. CIU clock rate in Hz for SDR12 mode
>> +	4. CIU clock rate in Hz for SDR25 mode
>> +	5. CIU clock rate in Hz for SDR50 mode
>> +	6. CIU clock rate in Hz for SDR104 mode
>> +	7. CIU clock rate in Hz for DDR50 mode
>> +	8. CIU clock rate in Hz for HS200 mode
> 
> This looks god now.
> 
>> +static void dw_mci_k3_set_ios(struct dw_mci *host, struct mmc_ios *ios)
>> +{
>> +	struct dw_mci_k3_priv_data *priv = host->priv;
>> +	u32 rate = priv->clk_table[ios->timing];
>> +	int ret;
> 
> I think this should have some range checking to see if the mode that is
> being set had a clock frequency set in the DT.
> 
>> +
>> +	ret = clk_set_rate(host->ciu_clk, rate);
>> +	if (ret)
>> +		dev_warn(host->dev, "failed to set clock rate %uHz\n", rate);
>> +
>> +	host->bus_hz = clk_get_rate(host->ciu_clk);
>> +}
> 
> Why do you call clk_get_rate() here, shouldn't it always be the same
> rate that you have just set?
> 
>> +static int dw_mci_k3_parse_dt(struct dw_mci *host)
>> +{
>> +	struct dw_mci_k3_priv_data *priv;
>> +	struct device_node *node = host->dev->of_node;
>> +	struct property *prop;
>> +	const __be32 *cur;
>> +	u32 val, num = 0;
>> +
>> +	priv = devm_kzalloc(host->dev, sizeof(*priv), GFP_KERNEL);
>> +	if (!priv) {
>> +		dev_err(host->dev, "mem alloc failed for private data\n");
>> +		return -ENOMEM;
>> +	}
>> +	host->priv = priv;
>> +
>> +	of_property_for_each_u32(node, "clock-freq-table", prop, cur, val) {
>> +		if (num >= MAX_NUMS)
>> +			break;
>> +		priv->clk_table[num++] = val;
>> +	}
>> +	return 0;
>> +}
> 
> If we make this property part of the generic binding, this function could
> also get moved to the main dw_mci driver.
> 
>> +static int dw_mci_k3_suspend(struct device *dev)
>> +{
>> +	struct dw_mci *host = dev_get_drvdata(dev);
>> +	int ret = 0;
>> +
>> +	ret = dw_mci_suspend(host);
> 
> You should never initialize local variables when they are set later in the
> function (the ret = 0 part above). For more complex functions, this prevents
> gcc from warning you about accidentally uninitialized uses.
> 
>> +	if (!ret)
>> +		clk_disable_unprepare(host->ciu_clk);
>> +
>> +	return ret;
>> +}
> 
> The suspend/resume code also looks very generic. Can't we make these the
> default for dw-mci? If you do both, you won't even need a k3 specific driver.
> I think in general we should try hard to add code like this to the common
> driver when there is a chance that it can be shared with other platforms.

Dw-mmc has the LOW_POWER mode feature at CLKENA register,
this feature is running like clock-gating.
So i have known it didn't control clock enable/disable in dw-mmc.c.

Best Regards,
Jaehoon Chung

> 
> 	Arnd
> 

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

[Zhangfei Gao]

On Mon, Dec 30, 2013 at 7:55 AM, Jaehoon Chung <jh80.chung@samsung.com> wrote:
> On 12/30/2013 06:05 AM, Arnd Bergmann wrote:
>> On Saturday 28 December 2013, Zhangfei Gao wrote:
>>> Add dw_mmc-k3.c for k3v2, support sd/emmc
>>>
>>> Signed-off-by: Zhangfei Gao <zhangfei.gao@linaro.org>
>>> Signed-off-by: Zhigang Wang <brooke.wangzhigang@huawei.com>
>>> ---
>>>  .../devicetree/bindings/mmc/k3-dw-mshc.txt         |   60 ++++++++++
>>>  drivers/mmc/host/Kconfig                           |   10 ++
>>>  drivers/mmc/host/Makefile                          |    1 +
>>>  drivers/mmc/host/dw_mmc-k3.c                       |  126 ++++++++++++++++++++
>>>  4 files changed, 197 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 000000000000..d7e2d7f159bb
>>> --- /dev/null
>>> +++ b/Documentation/devicetree/bindings/mmc/k3-dw-mshc.txt
>>> @@ -0,0 +1,60 @@
>>> +* Hisilicon specific extensions to the Synopsys Designware Mobile
>>> +  Storage Host Controller
>>> +
>>> +Read synopsys-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 synopsys-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 one of the following.
>>> +  - "hisilicon,hi4511-dw-mshc": for controllers with hi4511 specific extentions.
>>
>> I wonder if this is actually a different variant of the mshc hardware, or just
>> wired up in a different way. Do you know details?
>>
>> Since the only difference in the binding is the presence of the "clock-freq-table"
>> property, we could also make this property generic for the mshc driver and use
>> it if present but fall back to the normal behavior when it is absent.

There are still other differences and limitations besides "clock-freq-table".
The controller seems less intelligent than other Synopsys mmc controller.
The tuning process like HS200 is not intelligent, as a result, some
local registers are added to help this.

>>
>>> +* clock-freq-table: should be the frequency (in Hz) array of the ciu clock
>>> +    in each supported mode.
>>> +    0. CIU clock rate in Hz for DS mode
>>> +    1. CIU clock rate in Hz for MMC HS mode
>>> +    2. CIU clock rate in Hz for SD HS mode
>>> +    3. CIU clock rate in Hz for SDR12 mode
>>> +    4. CIU clock rate in Hz for SDR25 mode
>>> +    5. CIU clock rate in Hz for SDR50 mode
>>> +    6. CIU clock rate in Hz for SDR104 mode
>>> +    7. CIU clock rate in Hz for DDR50 mode
>>> +    8. CIU clock rate in Hz for HS200 mode
>>
>> This looks god now.
>>
>>> +static void dw_mci_k3_set_ios(struct dw_mci *host, struct mmc_ios *ios)
>>> +{
>>> +    struct dw_mci_k3_priv_data *priv = host->priv;
>>> +    u32 rate = priv->clk_table[ios->timing];
>>> +    int ret;
>>
>> I think this should have some range checking to see if the mode that is
>> being set had a clock frequency set in the DT.

The range safety should be ensured by the clk_table array, MAX_NUMS=10.
>>
>>> +
>>> +    ret = clk_set_rate(host->ciu_clk, rate);
>>> +    if (ret)
>>> +            dev_warn(host->dev, "failed to set clock rate %uHz\n", rate);
>>> +
>>> +    host->bus_hz = clk_get_rate(host->ciu_clk);
>>> +}
>>
>> Why do you call clk_get_rate() here, shouldn't it always be the same
>> rate that you have just set?

It is more accurate to use clk_get_rate here.
For example, if switch to ios->clock as you suggested before, 52M will
be set for mmc, while 50M is supported.
However, it can simply use rate set currently.

>>
>>> +static int dw_mci_k3_parse_dt(struct dw_mci *host)
>>> +{
>>> +    struct dw_mci_k3_priv_data *priv;
>>> +    struct device_node *node = host->dev->of_node;
>>> +    struct property *prop;
>>> +    const __be32 *cur;
>>> +    u32 val, num = 0;
>>> +
>>> +    priv = devm_kzalloc(host->dev, sizeof(*priv), GFP_KERNEL);
>>> +    if (!priv) {
>>> +            dev_err(host->dev, "mem alloc failed for private data\n");
>>> +            return -ENOMEM;
>>> +    }
>>> +    host->priv = priv;
>>> +
>>> +    of_property_for_each_u32(node, "clock-freq-table", prop, cur, val) {
>>> +            if (num >= MAX_NUMS)
>>> +                    break;
>>> +            priv->clk_table[num++] = val;
>>> +    }
>>> +    return 0;
>>> +}
>>
>> If we make this property part of the generic binding, this function could
>> also get moved to the main dw_mci driver.

This may be not general.
The controller does not generate the clock itself, and set rate to the
outside clock source, which may be different according to the working
mode.

>>
>>> +static int dw_mci_k3_suspend(struct device *dev)
>>> +{
>>> +    struct dw_mci *host = dev_get_drvdata(dev);
>>> +    int ret = 0;
>>> +
>>> +    ret = dw_mci_suspend(host);
>>
>> You should never initialize local variables when they are set later in the
>> function (the ret = 0 part above). For more complex functions, this prevents
>> gcc from warning you about accidentally uninitialized uses.

Frankly speaking, I didn't know this rule at all.
Often see the warning before like “Variable to be used not initialized”,
so preferred to init the local variable as much as possible.
Looks like it is wrong.

>>
>>> +    if (!ret)
>>> +            clk_disable_unprepare(host->ciu_clk);
>>> +
>>> +    return ret;
>>> +}
>>
>> The suspend/resume code also looks very generic. Can't we make these the
>> default for dw-mci? If you do both, you won't even need a k3 specific driver.
>> I think in general we should try hard to add code like this to the common
>> driver when there is a chance that it can be shared with other platforms.
>
> Dw-mmc has the LOW_POWER mode feature at CLKENA register,
> this feature is running like clock-gating.
> So i have known it didn't control clock enable/disable in dw-mmc.c.

It is added here since we have to set special register when resume
back, which has been abstracted to ciu_clk prepare operation.

>
> Best Regards,
> Jaehoon Chung
>
>>
>>       Arnd
>>
>
>
> _______________________________________________
> linux-arm-kernel mailing list
> linux-arm-kernel@lists.infradead.org
> http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

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

[zhangfei]

On 12/30/2013 10:32 AM, Zhangfei Gao wrote:
> On Mon, Dec 30, 2013 at 7:55 AM, Jaehoon Chung <jh80.chung@samsung.com> wrote:
>> On 12/30/2013 06:05 AM, Arnd Bergmann wrote:
>>> On Saturday 28 December 2013, Zhangfei Gao wrote:
>>>> Add dw_mmc-k3.c for k3v2, support sd/emmc
>>>>
>>>> Signed-off-by: Zhangfei Gao <zhangfei.gao@linaro.org>
>>>> Signed-off-by: Zhigang Wang <brooke.wangzhigang@huawei.com>
>>>> ---
>>>>   .../devicetree/bindings/mmc/k3-dw-mshc.txt         |   60 ++++++++++
>>>>   drivers/mmc/host/Kconfig                           |   10 ++
>>>>   drivers/mmc/host/Makefile                          |    1 +
>>>>   drivers/mmc/host/dw_mmc-k3.c                       |  126 ++++++++++++++++++++
>>>>   4 files changed, 197 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 000000000000..d7e2d7f159bb
>>>> --- /dev/null
>>>> +++ b/Documentation/devicetree/bindings/mmc/k3-dw-mshc.txt
>>>> @@ -0,0 +1,60 @@
>>>> +* Hisilicon specific extensions to the Synopsys Designware Mobile
>>>> +  Storage Host Controller
>>>> +
>>>> +Read synopsys-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 synopsys-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 one of the following.
>>>> +  - "hisilicon,hi4511-dw-mshc": for controllers with hi4511 specific extentions.
>>>
>>> I wonder if this is actually a different variant of the mshc hardware, or just
>>> wired up in a different way. Do you know details?
>>>
>>> Since the only difference in the binding is the presence of the "clock-freq-table"
>>> property, we could also make this property generic for the mshc driver and use
>>> it if present but fall back to the normal behavior when it is absent.
>
> There are still other differences and limitations besides "clock-freq-table".
> The controller seems less intelligent than other Synopsys mmc controller.
> The tuning process like HS200 is not intelligent, as a result, some
> local registers are added to help this.
>
>>>
>>>> +* clock-freq-table: should be the frequency (in Hz) array of the ciu clock
>>>> +    in each supported mode.
>>>> +    0. CIU clock rate in Hz for DS mode
>>>> +    1. CIU clock rate in Hz for MMC HS mode
>>>> +    2. CIU clock rate in Hz for SD HS mode
>>>> +    3. CIU clock rate in Hz for SDR12 mode
>>>> +    4. CIU clock rate in Hz for SDR25 mode
>>>> +    5. CIU clock rate in Hz for SDR50 mode
>>>> +    6. CIU clock rate in Hz for SDR104 mode
>>>> +    7. CIU clock rate in Hz for DDR50 mode
>>>> +    8. CIU clock rate in Hz for HS200 mode
>>>
>>> This looks god now.
>>>
>>>> +static void dw_mci_k3_set_ios(struct dw_mci *host, struct mmc_ios *ios)
>>>> +{
>>>> +    struct dw_mci_k3_priv_data *priv = host->priv;
>>>> +    u32 rate = priv->clk_table[ios->timing];
>>>> +    int ret;
>>>
>>> I think this should have some range checking to see if the mode that is
>>> being set had a clock frequency set in the DT.
>
> The range safety should be ensured by the clk_table array, MAX_NUMS=10.
>>>
>>>> +
>>>> +    ret = clk_set_rate(host->ciu_clk, rate);
>>>> +    if (ret)
>>>> +            dev_warn(host->dev, "failed to set clock rate %uHz\n", rate);
>>>> +
>>>> +    host->bus_hz = clk_get_rate(host->ciu_clk);
>>>> +}
>>>
>>> Why do you call clk_get_rate() here, shouldn't it always be the same
>>> rate that you have just set?
>
> It is more accurate to use clk_get_rate here.
> For example, if switch to ios->clock as you suggested before, 52M will
> be set for mmc, while 50M is supported.
> However, it can simply use rate set currently.
>
>>>
>>>> +static int dw_mci_k3_parse_dt(struct dw_mci *host)
>>>> +{
>>>> +    struct dw_mci_k3_priv_data *priv;
>>>> +    struct device_node *node = host->dev->of_node;
>>>> +    struct property *prop;
>>>> +    const __be32 *cur;
>>>> +    u32 val, num = 0;
>>>> +
>>>> +    priv = devm_kzalloc(host->dev, sizeof(*priv), GFP_KERNEL);
>>>> +    if (!priv) {
>>>> +            dev_err(host->dev, "mem alloc failed for private data\n");
>>>> +            return -ENOMEM;
>>>> +    }
>>>> +    host->priv = priv;
>>>> +
>>>> +    of_property_for_each_u32(node, "clock-freq-table", prop, cur, val) {
>>>> +            if (num >= MAX_NUMS)
>>>> +                    break;
>>>> +            priv->clk_table[num++] = val;
>>>> +    }
>>>> +    return 0;
>>>> +}
>>>
>>> If we make this property part of the generic binding, this function could
>>> also get moved to the main dw_mci driver.
>
> This may be not general.
> The controller does not generate the clock itself, and set rate to the
> outside clock source, which may be different according to the working
> mode.
>
>>>
>>>> +static int dw_mci_k3_suspend(struct device *dev)
>>>> +{
>>>> +    struct dw_mci *host = dev_get_drvdata(dev);
>>>> +    int ret = 0;
>>>> +
>>>> +    ret = dw_mci_suspend(host);
>>>
>>> You should never initialize local variables when they are set later in the
>>> function (the ret = 0 part above). For more complex functions, this prevents
>>> gcc from warning you about accidentally uninitialized uses.

I am sorry I may fall into the dead end, but still quite not understand 
this rule.
I alwayes thought it would be a good habit to init local variables before.
Do you mean it should NOT init local variable as much as possible and 
only init on demand, like solving the gcc warning.
Why not init the them at start in case random value cause unpredicted error?

>
> Frankly speaking, I didn't know this rule at all.
> Often see the warning before like “Variable to be used not initialized”,
> so preferred to init the local variable as much as possible.
> Looks like it is wrong.
>
>>>
>>>> +    if (!ret)
>>>> +            clk_disable_unprepare(host->ciu_clk);
>>>> +
>>>> +    return ret;
>>>> +}
>>>
>>> The suspend/resume code also looks very generic. Can't we make these the
>>> default for dw-mci? If you do both, you won't even need a k3 specific driver.
>>> I think in general we should try hard to add code like this to the common
>>> driver when there is a chance that it can be shared with other platforms.
>>
>> Dw-mmc has the LOW_POWER mode feature at CLKENA register,
>> this feature is running like clock-gating.
>> So i have known it didn't control clock enable/disable in dw-mmc.c.
>
> It is added here since we have to set special register when resume
> back, which has been abstracted to ciu_clk prepare operation.
>
>>
>> Best Regards,
>> Jaehoon Chung
>>
>>>
>>>        Arnd
>>>
>>
>>
>> _______________________________________________
>> linux-arm-kernel mailing list
>> linux-arm-kernel@lists.infradead.org
>> http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

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

[Arnd Bergmann]

On Monday 30 December 2013, zhangfei wrote:
> >>>> +static int dw_mci_k3_suspend(struct device *dev)
> >>>> +{
> >>>> +    struct dw_mci *host = dev_get_drvdata(dev);
> >>>> +    int ret = 0;
> >>>> +
> >>>> +    ret = dw_mci_suspend(host);
> >>>
> >>> You should never initialize local variables when they are set later in the
> >>> function (the ret = 0 part above). For more complex functions, this prevents
> >>> gcc from warning you about accidentally uninitialized uses.
> 
> I am sorry I may fall into the dead end, but still quite not understand 
> this rule.
> I alwayes thought it would be a good habit to init local variables before.
> Do you mean it should NOT init local variable as much as possible and 
> only init on demand, like solving the gcc warning.
> Why not init the them at start in case random value cause unpredicted error?

The gcc warnings are 100% correct, we can use them as a tool to write better
code. If you write code that has no warnings with a modern compiler, you will
never use random values, but if you always initialize the local variables,
you can end up accidentally using '0' where you shouldn't have.

See http://rusty.ozlabs.org/?p=232 for an excellent article on the topic
by former Linaro assignee Rusty Russell.

	Arnd

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

[zhangfei]

Dear Arnd,

On 12/31/2013 04:27 AM, Arnd Bergmann wrote:
> On Monday 30 December 2013, zhangfei wrote:
>>>>>> +static int dw_mci_k3_suspend(struct device *dev)
>>>>>> +{
>>>>>> +    struct dw_mci *host = dev_get_drvdata(dev);
>>>>>> +    int ret = 0;
>>>>>> +
>>>>>> +    ret = dw_mci_suspend(host);
>>>>>
>>>>> You should never initialize local variables when they are set later in the
>>>>> function (the ret = 0 part above). For more complex functions, this prevents
>>>>> gcc from warning you about accidentally uninitialized uses.
>>
>> I am sorry I may fall into the dead end, but still quite not understand
>> this rule.
>> I alwayes thought it would be a good habit to init local variables before.
>> Do you mean it should NOT init local variable as much as possible and
>> only init on demand, like solving the gcc warning.
>> Why not init the them at start in case random value cause unpredicted error?
>
> The gcc warnings are 100% correct, we can use them as a tool to write better
> code. If you write code that has no warnings with a modern compiler, you will
> never use random values, but if you always initialize the local variables,
> you can end up accidentally using '0' where you shouldn't have.
>
> See http://rusty.ozlabs.org/?p=232 for an excellent article on the topic
> by former Linaro assignee Rusty Russell.
>

Excellent, this is what I am looking for.
Thanks for the patience.
I may need some time and gradually change the habit to diminish the hurt 
memory caused by uninitialized vector.
Will update and take care latter.

By the way, are you fine with the other comments' explanation.

Thanks

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

[Gerhard Sittig]

On Sun, Dec 29, 2013 at 22:05 +0100, Arnd Bergmann wrote:
> 
> On Saturday 28 December 2013, Zhangfei Gao wrote:
> 
> > +static void dw_mci_k3_set_ios(struct dw_mci *host, struct mmc_ios *ios)
> > +{
> > +	struct dw_mci_k3_priv_data *priv = host->priv;
> > +	u32 rate = priv->clk_table[ios->timing];
> > +	int ret;
> 
> [ ... ]
> 
> > +
> > +	ret = clk_set_rate(host->ciu_clk, rate);
> > +	if (ret)
> > +		dev_warn(host->dev, "failed to set clock rate %uHz\n", rate);
> > +
> > +	host->bus_hz = clk_get_rate(host->ciu_clk);
> > +}
> 
> Why do you call clk_get_rate() here, shouldn't it always be the same
> rate that you have just set?

Not necessarily.  What you pass to clk_set_rate() is the
rate/frequency that you _want_, while what you get from
clk_get_rate() is the rate you _got_ taking the specific input
clock rate and the available sets of multipliers/dividers into
consideration.  Both values should be similar (roughly the same),
but they need not be identical.  The order of the difference
depends on the granularity of the hardware dividers or whether
PLLs are used.

So, re-fetching the resulting rate after setting up a desired
rate actually better reflects the status-quo for diagnostics or
for subsequent processing.


virtually yours
Gerhard Sittig
-- 
DENX Software Engineering GmbH,     MD: Wolfgang Denk & Detlev Zundel
HRB 165235 Munich, Office: Kirchenstr. 5, D-82194 Groebenzell, Germany
Phone: +49-8142-66989-0 Fax: +49-8142-66989-80  Email: office@denx.de

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

[zhangfei]

On 12/31/2013 09:20 PM, Gerhard Sittig wrote:
> On Sun, Dec 29, 2013 at 22:05 +0100, Arnd Bergmann wrote:
>>
>> On Saturday 28 December 2013, Zhangfei Gao wrote:
>>
>>> +static void dw_mci_k3_set_ios(struct dw_mci *host, struct mmc_ios *ios)
>>> +{
>>> +	struct dw_mci_k3_priv_data *priv = host->priv;
>>> +	u32 rate = priv->clk_table[ios->timing];
>>> +	int ret;
>>
>> [ ... ]
>>
>>> +
>>> +	ret = clk_set_rate(host->ciu_clk, rate);
>>> +	if (ret)
>>> +		dev_warn(host->dev, "failed to set clock rate %uHz\n", rate);
>>> +
>>> +	host->bus_hz = clk_get_rate(host->ciu_clk);
>>> +}
>>
>> Why do you call clk_get_rate() here, shouldn't it always be the same
>> rate that you have just set?
>
> Not necessarily.  What you pass to clk_set_rate() is the
> rate/frequency that you _want_, while what you get from
> clk_get_rate() is the rate you _got_ taking the specific input
> clock rate and the available sets of multipliers/dividers into
> consideration.  Both values should be similar (roughly the same),
> but they need not be identical.  The order of the difference
> depends on the granularity of the hardware dividers or whether
> PLLs are used.
>
> So, re-fetching the resulting rate after setting up a desired
> rate actually better reflects the status-quo for diagnostics or
> for subsequent processing.

Thanks Gerhard for the professional explanation.

[PATCH 2/3] 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>
Signed-off-by: Zhigang Wang <brooke.wangzhigang@huawei.com>
---
 .../devicetree/bindings/mmc/k3-dw-mshc.txt         |   60 +++++++++
 drivers/mmc/host/Kconfig                           |   10 ++
 drivers/mmc/host/Makefile                          |    1 +
 drivers/mmc/host/dw_mmc-k3.c                       |  132 ++++++++++++++++++++
 4 files changed, 203 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 000000000000..d7e2d7f159bb
--- /dev/null
+++ b/Documentation/devicetree/bindings/mmc/k3-dw-mshc.txt
@@ -0,0 +1,60 @@
+* Hisilicon specific extensions to the Synopsys Designware Mobile
+  Storage Host Controller
+
+Read synopsys-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 synopsys-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 one of the following.
+  - "hisilicon,hi4511-dw-mshc": for controllers with hi4511 specific extentions.
+
+* clock-freq-table: should be the frequency (in Hz) array of the ciu clock
+	in each	supported mode.
+	0. CIU clock rate in Hz for DS mode
+	1. CIU clock rate in Hz for MMC HS mode
+	2. CIU clock rate in Hz for SD HS mode
+	3. CIU clock rate in Hz for SDR12 mode
+	4. CIU clock rate in Hz for SDR25 mode
+	5. CIU clock rate in Hz for SDR50 mode
+	6. CIU clock rate in Hz for SDR104 mode
+	7. CIU clock rate in Hz for DDR50 mode
+	8. CIU clock rate in Hz for HS200 mode
+
+Example:
+
+	/* for Hi3620 */
+
+	/* 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 = <&mmc_clock HI3620_SD_CIUCLK>, <&clock HI3620_DDRC_PER_CLK>;
+		clock-names = "ciu", "biu";
+		clock-freq-table =
+		<25000000 0 50000000 25000000 50000000 100000000 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>;
+		};
+	};
diff --git a/drivers/mmc/host/Kconfig b/drivers/mmc/host/Kconfig
index 7fc5099e44b2..45aaa2de0f58 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 c41d0c364509..64f5f8d35839 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 000000000000..68e5e428e8f6
--- /dev/null
+++ b/drivers/mmc/host/dw_mmc-k3.c
@@ -0,0 +1,132 @@
+/*
+ * 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 MAX_NUMS	10
+struct dw_mci_k3_priv_data {
+	u32	clk_table[MAX_NUMS];
+};
+
+static void dw_mci_k3_set_ios(struct dw_mci *host, struct mmc_ios *ios)
+{
+	struct dw_mci_k3_priv_data *priv = host->priv;
+	u32 rate = priv->clk_table[ios->timing];
+	int ret;
+
+	if (!rate) {
+		dev_warn(host->dev,
+			"no specified rate in timing %u\n", ios->timing);
+		return;
+	}
+
+	ret = clk_set_rate(host->ciu_clk, rate);
+	if (ret)
+		dev_warn(host->dev, "failed to set clock rate %uHz\n", rate);
+
+	host->bus_hz = clk_get_rate(host->ciu_clk);
+}
+
+static int dw_mci_k3_parse_dt(struct dw_mci *host)
+{
+	struct dw_mci_k3_priv_data *priv;
+	struct device_node *node = host->dev->of_node;
+	struct property *prop;
+	const __be32 *cur;
+	u32 val, num = 0;
+
+	priv = devm_kzalloc(host->dev, sizeof(*priv), GFP_KERNEL);
+	if (!priv) {
+		dev_err(host->dev, "mem alloc failed for private data\n");
+		return -ENOMEM;
+	}
+	host->priv = priv;
+
+	of_property_for_each_u32(node, "clock-freq-table", prop, cur, val) {
+		if (num >= MAX_NUMS)
+			break;
+		priv->clk_table[num++] = val;
+	}
+	return 0;
+}
+
+static const struct dw_mci_drv_data k3_drv_data = {
+	.set_ios		= dw_mci_k3_set_ios,
+	.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);
+	int ret;
+
+	ret = dw_mci_suspend(host);
+	if (!ret)
+		clk_disable_unprepare(host->ciu_clk);
+
+	return ret;
+}
+
+static int dw_mci_k3_resume(struct device *dev)
+{
+	struct dw_mci *host = dev_get_drvdata(dev);
+	int ret;
+
+	ret = clk_prepare_enable(host->ciu_clk);
+	if (ret) {
+		dev_err(host->dev, "failed to enable ciu clock\n");
+		return ret;
+	}
+
+	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

[PATCH 1/3] mmc: dw_mmc: use slot-gpio to handle cd pin

[Zhangfei Gao]

Suggested by Jaehoon: Use slot-gpio to handle cd-gpio
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>
Acked-by: Jaehoon Chung <jh80.chung@samsung.com>
---
 drivers/mmc/host/dw_mmc.c |   48 +++++++++++++++++++++++++++++++++------------
 1 file changed, 35 insertions(+), 13 deletions(-)

diff --git a/drivers/mmc/host/dw_mmc.c b/drivers/mmc/host/dw_mmc.c
index 4bce0deec362..a776f24f4311 100644
--- a/drivers/mmc/host/dw_mmc.c
+++ b/drivers/mmc/host/dw_mmc.c
@@ -36,6 +36,7 @@
 #include <linux/workqueue.h>
 #include <linux/of.h>
 #include <linux/of_gpio.h>
+#include <linux/mmc/slot-gpio.h>
 
 #include "dw_mmc.h"
 
@@ -1032,20 +1033,26 @@ 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;
 
-	if (present)
+	if (present) {
+		set_bit(DW_MMC_CARD_PRESENT, &slot->flags);
 		dev_dbg(&mmc->class_dev, "card is present\n");
-	else
+	} else {
+		clear_bit(DW_MMC_CARD_PRESENT, &slot->flags);
 		dev_dbg(&mmc->class_dev, "card is not present\n");
+	}
 
 	return present;
 }
@@ -1926,10 +1933,6 @@ static void dw_mci_work_routine_card(struct work_struct *work)
 			/* Card change detected */
 			slot->last_detect_state = present;
 
-			/* Mark card as present if applicable */
-			if (present != 0)
-				set_bit(DW_MMC_CARD_PRESENT, &slot->flags);
-
 			/* Clean up queue if present */
 			mrq = slot->mrq;
 			if (mrq) {
@@ -1977,8 +1980,6 @@ static void dw_mci_work_routine_card(struct work_struct *work)
 
 			/* Power down slot */
 			if (present == 0) {
-				clear_bit(DW_MMC_CARD_PRESENT, &slot->flags);
-
 				/* Clear down the FIFO */
 				dw_mci_fifo_reset(host);
 #ifdef CONFIG_MMC_DW_IDMAC
@@ -2079,6 +2080,26 @@ 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 void 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;
+
+	gpio = of_get_named_gpio(np, "cd-gpios", 0);
+
+	/* Having a missing entry is valid; return silently */
+	if (!gpio_is_valid(gpio))
+		return;
+
+	if (mmc_gpio_request_cd(mmc, gpio, 0))
+		dev_warn(dev, "gpio [%d] request failed\n", gpio);
+}
 #else /* CONFIG_OF */
 static int dw_mci_of_get_slot_quirks(struct device *dev, u8 slot)
 {
@@ -2096,6 +2117,11 @@ static int dw_mci_of_get_wp_gpio(struct device *dev, u8 slot)
 {
 	return -EINVAL;
 }
+static void dw_mci_of_get_cd_gpio(struct device *dev, u8 slot,
+					struct mmc_host *mmc)
+{
+	return;
+}
 #endif /* CONFIG_OF */
 
 static int dw_mci_init_slot(struct dw_mci *host, unsigned int id)
@@ -2197,12 +2223,8 @@ static int dw_mci_init_slot(struct dw_mci *host, unsigned int id)
 #endif /* CONFIG_MMC_DW_IDMAC */
 	}
 
-	if (dw_mci_get_cd(mmc))
-		set_bit(DW_MMC_CARD_PRESENT, &slot->flags);
-	else
-		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

Re: [PATCH 1/3] mmc: dw_mmc: use slot-gpio to handle cd pin

[Arnd Bergmann]

On Thursday 09 January 2014, Zhangfei Gao wrote:
> Suggested by Jaehoon: Use slot-gpio to handle cd-gpio
> 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>
> Acked-by: Jaehoon Chung <jh80.chung@samsung.com>
> ---
>  drivers/mmc/host/dw_mmc.c |   48 +++++++++++++++++++++++++++++++++------------
>  1 file changed, 35 insertions(+), 13 deletions(-)

Acked-by: Arnd Bergmann <arnd@arndb.de>

Re: [PATCH 1/3] mmc: dw_mmc: use slot-gpio to handle cd pin

[Kevin Hilman]

On Thu, Jan 9, 2014 at 6:35 AM, Zhangfei Gao <zhangfei.gao@linaro.org> wrote:
> Suggested by Jaehoon: Use slot-gpio to handle cd-gpio
> 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>
> Acked-by: Jaehoon Chung <jh80.chung@samsung.com>

The Samsung Arndale board started failing boot from MMC root tests
starting with next-20140113 and I bisected it down to this patch.
Reverting this patch on top of next-20140114 gets Arndale booting
again from MMC.  Is there some supporting DT data that's missing for
Arndale?

Kevin

Re: [PATCH 1/3] mmc: dw_mmc: use slot-gpio to handle cd pin

[Olof Johansson]

On Tue, Jan 14, 2014 at 7:58 AM, Kevin Hilman <khilman@linaro.org> wrote:
> On Thu, Jan 9, 2014 at 6:35 AM, Zhangfei Gao <zhangfei.gao@linaro.org> wrote:
>> Suggested by Jaehoon: Use slot-gpio to handle cd-gpio
>> 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>
>> Acked-by: Jaehoon Chung <jh80.chung@samsung.com>
>
> The Samsung Arndale board started failing boot from MMC root tests
> starting with next-20140113 and I bisected it down to this patch.
> Reverting this patch on top of next-20140114 gets Arndale booting
> again from MMC.  Is there some supporting DT data that's missing for
> Arndale?

Rather, it looks like this patch changes behaviour and no longer uses
dw_mci_get_cd() to find out if there's a card attached -- it switches
to rely only on GPIO (see the last chunk in the patch). That seems
broken?


-Olof

Re: [PATCH 1/3] mmc: dw_mmc: use slot-gpio to handle cd pin

[zhangfei]

On 01/15/2014 01:23 AM, Olof Johansson wrote:
> On Tue, Jan 14, 2014 at 7:58 AM, Kevin Hilman <khilman@linaro.org> wrote:
>> On Thu, Jan 9, 2014 at 6:35 AM, Zhangfei Gao <zhangfei.gao@linaro.org> wrote:
>>> Suggested by Jaehoon: Use slot-gpio to handle cd-gpio
>>> 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>
>>> Acked-by: Jaehoon Chung <jh80.chung@samsung.com>
>>
>> The Samsung Arndale board started failing boot from MMC root tests
>> starting with next-20140113 and I bisected it down to this patch.
>> Reverting this patch on top of next-20140114 gets Arndale booting
>> again from MMC.  Is there some supporting DT data that's missing for
>> Arndale?
>
> Rather, it looks like this patch changes behaviour and no longer uses
> dw_mci_get_cd() to find out if there's a card attached -- it switches
> to rely only on GPIO (see the last chunk in the patch). That seems
> broken?
>
Oops,
Change using dw_mci_get_cd set flag DW_MMC_CARD_PRESENT.
And dw_mci_get_cd is called from mmc_rescan, a litter later than 
dw_mci_probe.
Is that too late?

Thanks

Re: [PATCH 1/3] mmc: dw_mmc: use slot-gpio to handle cd pin

[zhangfei]

Dear Kevin

On 01/15/2014 09:09 AM, zhangfei wrote:
>
>
> On 01/15/2014 01:23 AM, Olof Johansson wrote:
>> On Tue, Jan 14, 2014 at 7:58 AM, Kevin Hilman <khilman@linaro.org> wrote:
>>> On Thu, Jan 9, 2014 at 6:35 AM, Zhangfei Gao
>>> <zhangfei.gao@linaro.org> wrote:
>>>> Suggested by Jaehoon: Use slot-gpio to handle cd-gpio
>>>> 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>
>>>> Acked-by: Jaehoon Chung <jh80.chung@samsung.com>
>>>
>>> The Samsung Arndale board started failing boot from MMC root tests
>>> starting with next-20140113 and I bisected it down to this patch.
>>> Reverting this patch on top of next-20140114 gets Arndale booting
>>> again from MMC.  Is there some supporting DT data that's missing for
>>> Arndale?
>>
>> Rather, it looks like this patch changes behaviour and no longer uses
>> dw_mci_get_cd() to find out if there's a card attached -- it switches
>> to rely only on GPIO (see the last chunk in the patch). That seems
>> broken?
>>
> Oops,
> Change using dw_mci_get_cd set flag DW_MMC_CARD_PRESENT.
> And dw_mci_get_cd is called from mmc_rescan, a litter later than
> dw_mci_probe.
> Is that too late?

Should have found the issue, CDETECT is ignored since IS_ERR_VALUE does 
not workable to !mmc_gpio_get_cd(mmc), which used for adding debounce.
Sorry for that.

However, with this change the debounce seems not stable as before.
In the test of plug-in-out, sometimes sd detect will return timeout 
although present indicates as 1, while next time it can be detected again.
Still want to check more.


diff --git a/drivers/mmc/host/dw_mmc.c b/drivers/mmc/host/dw_mmc.c
index a776f24f4311..f1683ba194ee 100644
--- a/drivers/mmc/host/dw_mmc.c
+++ b/drivers/mmc/host/dw_mmc.c
@@ -1033,7 +1033,7 @@ 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);
+       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)
@@ -1041,7 +1041,7 @@ static int dw_mci_get_cd(struct mmc_host *mmc)
         else if (brd->get_cd)
                 present = !brd->get_cd(slot->id);
         else if (!IS_ERR_VALUE(gpio_cd))
-               present = !!gpio_cd;
+               present = !gpio_cd;
         else
                 present = (mci_readl(slot->host, CDETECT) & (1 << 
slot->id))
                         == 0 ? 1 : 0;