[PATCH v5 0/5] Add intial support to DW MMC host on ZTE SoC

[PATCH v5 0/5] Add intial support to DW MMC host on ZTE SoC

[Jun Nie]

Add intial support to DW MMC host on ZTE SoC. It include platform
specific wrapper driver and workarounds for fifo quirk.

Patches are prepared based on latest dw mmc runtime change:
   https://github.com/jh80chung/dw-mmc.git for-ulf

Changes vs version 4:
  - Fix missing empty dts compatible element in the end of compatible array.

Changes vs version 3:
  - Fix brace error in document.

Changes vs version 2:
  - Change dt property fifo-addr to data-addr and fifo-watermark-quirk to
    fifo-watermark-aligned.
  - Polish ZX MMC driver on minor coding style issues.

Changes vs version 1:
  - Change fifo-addr-override to fifo-addr and remove its workaround tag in comments.
  - Remove ZX DW MMC driver reset cap in driver, which can be added in dt nodes.

Jun Nie (5):
  mmc: dt-bindings: add ZTE ZX296718 MMC bindings
  mmc: zx: Initial support for ZX mmc controller
  Documentation: synopsys-dw-mshc: add binding for fifo quirks
  mmc: dw: Add fifo address property
  mmc: dw: Add fifo watermark alignment property

 .../devicetree/bindings/mmc/synopsys-dw-mshc.txt   |  13 ++
 .../devicetree/bindings/mmc/zx-dw-mshc.txt         |  34 +++
 drivers/mmc/host/Kconfig                           |   9 +
 drivers/mmc/host/Makefile                          |   1 +
 drivers/mmc/host/dw_mmc-zx.c                       | 242 +++++++++++++++++++++
 drivers/mmc/host/dw_mmc-zx.h                       |  31 +++
 drivers/mmc/host/dw_mmc.c                          |  17 +-
 include/linux/mmc/dw_mmc.h                         |   5 +
 8 files changed, 349 insertions(+), 3 deletions(-)
 create mode 100644 Documentation/devicetree/bindings/mmc/zx-dw-mshc.txt
 create mode 100644 drivers/mmc/host/dw_mmc-zx.c
 create mode 100644 drivers/mmc/host/dw_mmc-zx.h

-- 
1.9.1

[PATCH v5 1/5] mmc: dt-bindings: add ZTE ZX296718 MMC bindings

[Jun Nie]

Document the device-tree binding of ZTE MMC host on
ZX296718 SoC.

Signed-off-by: Jun Nie <jun.nie@linaro.org>
---
 .../devicetree/bindings/mmc/zx-dw-mshc.txt         | 35 ++++++++++++++++++++++
 1 file changed, 35 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/mmc/zx-dw-mshc.txt

diff --git a/Documentation/devicetree/bindings/mmc/zx-dw-mshc.txt b/Documentation/devicetree/bindings/mmc/zx-dw-mshc.txt
new file mode 100644
index 0000000..c175c4b
--- /dev/null
+++ b/Documentation/devicetree/bindings/mmc/zx-dw-mshc.txt
@@ -0,0 +1,35 @@
+* ZTE specific extensions to the Synopsys Designware Mobile Storage
+  Host Controller
+
+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 ZTE specific
+extensions to the Synopsys Designware Mobile Storage Host Controller.
+
+Required Properties:
+
+* compatible: should be
+	- "zte,zx296718-dw-mshc": for ZX SoCs
+
+Example:
+
+	mmc1: mmc@1110000 {
+		compatible = "zte,zx296718-dw-mshc";
+		#address-cells = <1>;
+		#size-cells = <0>;
+		reg = <0x01110000 0x1000>;
+		interrupts = <GIC_SPI 15 IRQ_TYPE_LEVEL_HIGH>;
+		fifo-depth = <32>;
+		data-addr = <0x200>;
+		fifo-watermark-aligned;
+		bus-width = <4>;
+		clock-frequency = <50000000>;
+		clocks = <&topcrm SD0_AHB>, <&topcrm SD0_WCLK>;
+		clock-names = "biu", "ciu";
+		num-slots = <1>;
+		max-frequency = <50000000>;
+		cap-sdio-irq;
+		cap-sd-highspeed;
+		status = "disabled";
+	};
-- 
1.9.1

[PATCH v5 2/5] mmc: zx: Initial support for ZX mmc controller

[Jun Nie]

This platform driver adds initial support for the DW host controller
found on ZTE SoCs.

It has been tested on ZX296718 EVB board currently. More support on
timing tuning will be added when hardware is available.

Signed-off-by: Jun Nie <jun.nie@linaro.org>
---
 drivers/mmc/host/Kconfig     |   9 ++
 drivers/mmc/host/Makefile    |   1 +
 drivers/mmc/host/dw_mmc-zx.c | 243 +++++++++++++++++++++++++++++++++++++++++++
 drivers/mmc/host/dw_mmc-zx.h |  31 ++++++
 4 files changed, 284 insertions(+)
 create mode 100644 drivers/mmc/host/dw_mmc-zx.c
 create mode 100644 drivers/mmc/host/dw_mmc-zx.h

diff --git a/drivers/mmc/host/Kconfig b/drivers/mmc/host/Kconfig
index 5274f50..4dafbc2 100644
--- a/drivers/mmc/host/Kconfig
+++ b/drivers/mmc/host/Kconfig
@@ -662,6 +662,15 @@ config MMC_DW_ROCKCHIP
 	  Synopsys DesignWare Memory Card Interface driver. Select this option
 	  for platforms based on RK3066, RK3188 and RK3288 SoC's.
 
+config MMC_DW_ZX
+	tristate "ZTE specific extensions for Synopsys DW Memory Card Interface"
+	depends on MMC_DW && ARCH_ZX
+	select MMC_DW_PLTFM
+	help
+	  This selects support for ZTE SoC specific extensions to the
+	  Synopsys DesignWare Memory Card Interface driver. Select this option
+	  for platforms based on ZX296718 SoC's.
+
 config MMC_SH_MMCIF
 	tristate "SuperH Internal MMCIF support"
 	depends on HAS_DMA
diff --git a/drivers/mmc/host/Makefile b/drivers/mmc/host/Makefile
index e2bdaaf..9766143 100644
--- a/drivers/mmc/host/Makefile
+++ b/drivers/mmc/host/Makefile
@@ -48,6 +48,7 @@ obj-$(CONFIG_MMC_DW_EXYNOS)	+= dw_mmc-exynos.o
 obj-$(CONFIG_MMC_DW_K3)		+= dw_mmc-k3.o
 obj-$(CONFIG_MMC_DW_PCI)	+= dw_mmc-pci.o
 obj-$(CONFIG_MMC_DW_ROCKCHIP)	+= dw_mmc-rockchip.o
+obj-$(CONFIG_MMC_DW_ZX)		+= dw_mmc-zx.o
 obj-$(CONFIG_MMC_SH_MMCIF)	+= sh_mmcif.o
 obj-$(CONFIG_MMC_JZ4740)	+= jz4740_mmc.o
 obj-$(CONFIG_MMC_VUB300)	+= vub300.o
diff --git a/drivers/mmc/host/dw_mmc-zx.c b/drivers/mmc/host/dw_mmc-zx.c
new file mode 100644
index 0000000..c48d851
--- /dev/null
+++ b/drivers/mmc/host/dw_mmc-zx.c
@@ -0,0 +1,243 @@
+/*
+ * ZX Specific Extensions for Synopsys DW Multimedia Card Interface driver
+ *
+ * Copyright (C) 2016, Linaro Ltd.
+ * Copyright (C) 2016, ZTE Corp.
+ *
+ * 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/clk.h>
+#include <linux/mfd/syscon.h>
+#include <linux/mmc/dw_mmc.h>
+#include <linux/mmc/host.h>
+#include <linux/mmc/mmc.h>
+#include <linux/module.h>
+#include <linux/of.h>
+#include <linux/platform_device.h>
+#include <linux/pm_runtime.h>
+#include <linux/regmap.h>
+#include <linux/slab.h>
+
+#include "dw_mmc.h"
+#include "dw_mmc-pltfm.h"
+#include "dw_mmc-zx.h"
+
+struct dw_mci_zx_priv_data {
+	struct regmap	*sysc_base;
+};
+
+enum delay_type {
+	DELAY_TYPE_READ,	/* read dqs delay */
+	DELAY_TYPE_CLK,		/* clk sample delay */
+};
+
+static int dw_mci_zx_emmc_set_delay(struct dw_mci *host, unsigned int delay,
+				    enum delay_type dflag)
+{
+	struct dw_mci_zx_priv_data *priv = host->priv;
+	struct regmap *sysc_base = priv->sysc_base;
+	unsigned int clksel;
+	unsigned int loop = 1000;
+	int ret;
+
+	if (!sysc_base)
+		return -EINVAL;
+
+	ret = regmap_update_bits(sysc_base, LB_AON_EMMC_CFG_REG0,
+				 PARA_HALF_CLK_MODE | PARA_DLL_BYPASS_MODE |
+				 PARA_PHASE_DET_SEL_MASK |
+				 PARA_DLL_LOCK_NUM_MASK |
+				 DLL_REG_SET | PARA_DLL_START_MASK,
+				 PARA_DLL_START(4) | PARA_DLL_LOCK_NUM(4));
+	if (ret)
+		return ret;
+
+	ret = regmap_read(sysc_base, LB_AON_EMMC_CFG_REG1, &clksel);
+	if (ret)
+		return ret;
+
+	if (dflag == DELAY_TYPE_CLK) {
+		clksel &= ~CLK_SAMP_DELAY_MASK;
+		clksel |= CLK_SAMP_DELAY(delay);
+	} else {
+		clksel &= ~READ_DQS_DELAY_MASK;
+		clksel |= READ_DQS_DELAY(delay);
+	}
+
+	regmap_write(sysc_base, LB_AON_EMMC_CFG_REG1, clksel);
+	regmap_update_bits(sysc_base, LB_AON_EMMC_CFG_REG0,
+			   PARA_DLL_START_MASK | PARA_DLL_LOCK_NUM_MASK |
+			   DLL_REG_SET,
+			   PARA_DLL_START(4) | PARA_DLL_LOCK_NUM(4) |
+			   DLL_REG_SET);
+
+	do {
+		ret = regmap_read(sysc_base, LB_AON_EMMC_CFG_REG2, &clksel);
+		if (ret)
+			return ret;
+
+	} while (--loop && !(clksel & ZX_DLL_LOCKED));
+
+	if (!loop) {
+		dev_err(host->dev, "Error: %s dll lock fail\n", __func__);
+		return -EIO;
+	}
+
+	return 0;
+}
+
+static int dw_mci_zx_emmc_execute_tuning(struct dw_mci_slot *slot, u32 opcode)
+{
+	struct dw_mci *host = slot->host;
+	struct mmc_host *mmc = slot->mmc;
+	int len, start = 0, end = 0, delay, best = 0;
+
+	for (delay = 1 ; delay < 128; delay++) {
+		dw_mci_zx_emmc_set_delay(host, delay, DELAY_TYPE_CLK);
+		if (mmc_send_tuning(mmc, opcode, NULL)) {
+			if (start >= 0) {
+				end = delay - 1;
+				/* check and update longest good range */
+				if ((end - start) > len) {
+					best = (start + end) >> 1;
+					len = end - start;
+				}
+			}
+			start = -1;
+			end = 0;
+			continue;
+		}
+		if (start < 0)
+			start = delay;
+	}
+
+	if (start >= 0) {
+		end = delay - 1;
+		if ((end - start) > len) {
+			best = (start + end) >> 1;
+			len = end - start;
+		}
+	}
+	if (best < 0)
+		return -EIO;
+
+	dev_info(host->dev, "%s best range: start %d end %d\n", __func__,
+		 start, end);
+	dw_mci_zx_emmc_set_delay(host, best, DELAY_TYPE_CLK);
+	return 0;
+}
+
+static int dw_mci_zx_prepare_hs400_tuning(struct dw_mci *host,
+					  struct mmc_ios *ios)
+{
+	int ret;
+
+	/* config phase shift as 90 degree */
+	ret = dw_mci_zx_emmc_set_delay(host, 32, DELAY_TYPE_READ);
+	if (ret < 0)
+		return -EIO;
+
+	return 0;
+}
+
+static int dw_mci_zx_execute_tuning(struct dw_mci_slot *slot, u32 opcode)
+{
+	struct dw_mci *host = slot->host;
+
+	if (host->verid == 0x290a) /* only for emmc */
+		return dw_mci_zx_emmc_execute_tuning(slot, opcode);
+	/* TODO: Add 0x210a dedicated tuning for sd/sdio */
+
+	return 0;
+}
+
+static int dw_mci_zx_parse_dt(struct dw_mci *host)
+{
+	struct device_node *np = host->dev->of_node;
+	struct device_node *node;
+	struct dw_mci_zx_priv_data *priv;
+	struct regmap *sysc_base;
+	int ret;
+
+	/* syscon is needed only by emmc */
+	node = of_parse_phandle(np, "zte,aon-syscon", 0);
+	if (node) {
+		sysc_base = syscon_node_to_regmap(node);
+		of_node_put(node);
+
+		if (IS_ERR(sysc_base)) {
+			ret = PTR_ERR(sysc_base);
+			if (ret != -EPROBE_DEFER)
+				dev_err(host->dev, "Can't get syscon: %d\n",
+					ret);
+			return ret;
+		}
+	} else {
+		return 0;
+	}
+
+	priv = devm_kzalloc(host->dev, sizeof(*priv), GFP_KERNEL);
+	if (!priv)
+		return -ENOMEM;
+	priv->sysc_base = sysc_base;
+	host->priv = priv;
+
+	return 0;
+}
+
+static unsigned long zx_dwmmc_caps[3] = {
+	MMC_CAP_CMD23,
+	MMC_CAP_CMD23,
+	MMC_CAP_CMD23,
+};
+
+static const struct dw_mci_drv_data zx_drv_data = {
+	.caps			= zx_dwmmc_caps,
+	.execute_tuning		= dw_mci_zx_execute_tuning,
+	.prepare_hs400_tuning	= dw_mci_zx_prepare_hs400_tuning,
+	.parse_dt               = dw_mci_zx_parse_dt,
+};
+
+static const struct of_device_id dw_mci_zx_match[] = {
+	{ .compatible = "zte,zx296718-dw-mshc", .data = &zx_drv_data},
+	{},
+};
+MODULE_DEVICE_TABLE(of, dw_mci_zx_match);
+
+static int dw_mci_zx_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_zx_match, pdev->dev.of_node);
+	drv_data = match->data;
+
+	return dw_mci_pltfm_register(pdev, drv_data);
+}
+
+static const struct dev_pm_ops dw_mci_zx_dev_pm_ops = {
+	SET_SYSTEM_SLEEP_PM_OPS(pm_runtime_force_suspend,
+				pm_runtime_force_resume)
+	SET_RUNTIME_PM_OPS(dw_mci_runtime_suspend,
+			   dw_mci_runtime_resume,
+			   NULL)
+};
+
+static struct platform_driver dw_mci_zx_pltfm_driver = {
+	.probe		= dw_mci_zx_probe,
+	.remove		= dw_mci_pltfm_remove,
+	.driver		= {
+		.name		= "dwmmc_zx",
+		.of_match_table	= dw_mci_zx_match,
+		.pm		= &dw_mci_zx_dev_pm_ops,
+	},
+};
+
+module_platform_driver(dw_mci_zx_pltfm_driver);
+
+MODULE_DESCRIPTION("ZTE emmc/sd driver");
+MODULE_LICENSE("GPL v2");
diff --git a/drivers/mmc/host/dw_mmc-zx.h b/drivers/mmc/host/dw_mmc-zx.h
new file mode 100644
index 0000000..f369997
--- /dev/null
+++ b/drivers/mmc/host/dw_mmc-zx.h
@@ -0,0 +1,31 @@
+#ifndef _DW_MMC_ZX_H_
+#define _DW_MMC_ZX_H_
+
+/* ZX296718 SoC specific DLL register offset. */
+#define LB_AON_EMMC_CFG_REG0  0x1B0
+#define LB_AON_EMMC_CFG_REG1  0x1B4
+#define LB_AON_EMMC_CFG_REG2  0x1B8
+
+/* LB_AON_EMMC_CFG_REG0 register defines */
+#define PARA_DLL_START(x)	((x) & 0xFF)
+#define PARA_DLL_START_MASK	0xFF
+#define DLL_REG_SET		BIT(8)
+#define PARA_DLL_LOCK_NUM(x)	(((x) & 7) << 16)
+#define PARA_DLL_LOCK_NUM_MASK  (7 << 16)
+#define PARA_PHASE_DET_SEL(x)	(((x) & 7) << 20)
+#define PARA_PHASE_DET_SEL_MASK	(7 << 20)
+#define PARA_DLL_BYPASS_MODE	BIT(23)
+#define PARA_HALF_CLK_MODE	BIT(24)
+
+/* LB_AON_EMMC_CFG_REG1 register defines */
+#define READ_DQS_DELAY(x)	((x) & 0x7F)
+#define READ_DQS_DELAY_MASK	(0x7F)
+#define READ_DQS_BYPASS_MODE	BIT(7)
+#define CLK_SAMP_DELAY(x)	(((x) & 0x7F) << 8)
+#define CLK_SAMP_DELAY_MASK	(0x7F << 8)
+#define CLK_SAMP_BYPASS_MODE	BIT(15)
+
+/* LB_AON_EMMC_CFG_REG2 register defines */
+#define ZX_DLL_LOCKED		BIT(2)
+
+#endif /* _DW_MMC_ZX_H_ */
-- 
1.9.1

[PATCH v5 3/5] Documentation: synopsys-dw-mshc: add binding for fifo quirks

[Jun Nie]

Add fifo-addr property and fifo-watermark-quirk property to
synopsys-dw-mshc bindings. It is intended to provide more
dt interface to support SoCs specific configuration.

Signed-off-by: Jun Nie <jun.nie@linaro.org>
---
 Documentation/devicetree/bindings/mmc/synopsys-dw-mshc.txt | 13 +++++++++++++
 1 file changed, 13 insertions(+)

diff --git a/Documentation/devicetree/bindings/mmc/synopsys-dw-mshc.txt b/Documentation/devicetree/bindings/mmc/synopsys-dw-mshc.txt
index 4e00e85..8bf2e41 100644
--- a/Documentation/devicetree/bindings/mmc/synopsys-dw-mshc.txt
+++ b/Documentation/devicetree/bindings/mmc/synopsys-dw-mshc.txt
@@ -76,6 +76,17 @@ Optional properties:
 
 * broken-cd: as documented in mmc core bindings.
 
+* data-addr: Override fifo address with value provided by DT. The default FIFO reg
+  offset is assumed as 0x100 (version < 0x240A) and 0x200(version >= 0x240A) by
+  driver. If the controller does not follow this rule, please use this property
+  to set fifo address in device tree.
+
+* fifo-watermark-aligned: Data done irq is expected if data length is less than
+  watermark in PIO mode. But fifo watermark is requested to be aligned with data
+  length in some SoC so that TX/RX irq can be generated with data done irq. Add this
+  watermark quirk to mark this requirement and force fifo watermark setting
+  accordingly.
+
 * vmmc-supply: The phandle to the regulator to use for vmmc.  If this is
   specified we'll defer probe until we can find this regulator.
 
@@ -103,6 +114,8 @@ board specific portions as listed below.
 		interrupts = <0 75 0>;
 		#address-cells = <1>;
 		#size-cells = <0>;
+		data-addr = <0x200>;
+		fifo-watermark-aligned;
 	};
 
 [board specific internal DMA resources]
-- 
1.9.1

[PATCH v5 4/5] mmc: dw: Add fifo address property

[Jun Nie]

The FIFO address may break default address assumption of 0x100
(version < 0x240A) and 0x200(version >= 0x240A) in current driver.
The new property is introduced to override fifo address via DT
node information.

Signed-off-by: Jun Nie <jun.nie@linaro.org>
---
 drivers/mmc/host/dw_mmc.c  | 6 +++++-
 include/linux/mmc/dw_mmc.h | 2 ++
 2 files changed, 7 insertions(+), 1 deletion(-)

diff --git a/drivers/mmc/host/dw_mmc.c b/drivers/mmc/host/dw_mmc.c
index 1c9ee36..696b5e6 100644
--- a/drivers/mmc/host/dw_mmc.c
+++ b/drivers/mmc/host/dw_mmc.c
@@ -2955,6 +2955,8 @@ static struct dw_mci_board *dw_mci_parse_dt(struct dw_mci *host)
 
 	of_property_read_u32(np, "card-detect-delay", &pdata->detect_delay_ms);
 
+	of_property_read_u32(np, "data-addr", &host->data_addr_override);
+
 	if (!of_property_read_u32(np, "clock-frequency", &clock_frequency))
 		pdata->bus_hz = clock_frequency;
 
@@ -3158,7 +3160,9 @@ int dw_mci_probe(struct dw_mci *host)
 	host->verid = SDMMC_GET_VERID(mci_readl(host, VERID));
 	dev_info(host->dev, "Version ID is %04x\n", host->verid);
 
-	if (host->verid < DW_MMC_240A)
+	if (host->data_addr_override)
+		host->fifo_reg = host->regs + host->data_addr_override;
+	else if (host->verid < DW_MMC_240A)
 		host->fifo_reg = host->regs + DATA_OFFSET;
 	else
 		host->fifo_reg = host->regs + DATA_240A_OFFSET;
diff --git a/include/linux/mmc/dw_mmc.h b/include/linux/mmc/dw_mmc.h
index f5af2bd..17cb95a 100644
--- a/include/linux/mmc/dw_mmc.h
+++ b/include/linux/mmc/dw_mmc.h
@@ -107,6 +107,7 @@ struct dw_mci_dma_slave {
  * @ciu_clk: Pointer to card interface unit clock instance.
  * @slot: Slots sharing this MMC controller.
  * @fifo_depth: depth of FIFO.
+ * @data_addr_override: override fifo reg offset with this value.
  * @data_shift: log2 of FIFO item size.
  * @part_buf_start: Start index in part_buf.
  * @part_buf_count: Bytes of partial data in part_buf.
@@ -154,6 +155,7 @@ struct dw_mci {
 	spinlock_t		irq_lock;
 	void __iomem		*regs;
 	void __iomem		*fifo_reg;
+	u32			data_addr_override;
 
 	struct scatterlist	*sg;
 	struct sg_mapping_iter	sg_miter;
-- 
1.9.1

[PATCH v5 5/5] mmc: dw: Add fifo watermark alignment property

[Jun Nie]

Data done irq is expected if data length is less than
watermark in PIO mode. But fifo watermark is requested
to be aligned with data length in some SoC so that TX/RX
irq can be generated with data done irq. Add the
watermark alignment to mark this requirement and force
fifo watermark setting accordingly.

Signed-off-by: Jun Nie <jun.nie@linaro.org>
---
 drivers/mmc/host/dw_mmc.c  | 11 +++++++++--
 include/linux/mmc/dw_mmc.h |  3 +++
 2 files changed, 12 insertions(+), 2 deletions(-)

diff --git a/drivers/mmc/host/dw_mmc.c b/drivers/mmc/host/dw_mmc.c
index 696b5e6..6d85ca6 100644
--- a/drivers/mmc/host/dw_mmc.c
+++ b/drivers/mmc/host/dw_mmc.c
@@ -1111,11 +1111,15 @@ static void dw_mci_submit_data(struct dw_mci *host, struct mmc_data *data)
 		mci_writel(host, CTRL, temp);
 
 		/*
-		 * Use the initial fifoth_val for PIO mode.
+		 * Use the initial fifoth_val for PIO mode. If wm_algined
+		 * is set, we set watermark same as data size.
 		 * If next issued data may be transfered by DMA mode,
 		 * prev_blksz should be invalidated.
 		 */
-		mci_writel(host, FIFOTH, host->fifoth_val);
+		if (host->wm_aligned)
+			dw_mci_adjust_fifoth(host, data);
+		else
+			mci_writel(host, FIFOTH, host->fifoth_val);
 		host->prev_blksz = 0;
 	} else {
 		/*
@@ -2957,6 +2961,9 @@ static struct dw_mci_board *dw_mci_parse_dt(struct dw_mci *host)
 
 	of_property_read_u32(np, "data-addr", &host->data_addr_override);
 
+	if (of_get_property(np, "fifo-watermark-aligned", NULL))
+		host->wm_aligned = true;
+
 	if (!of_property_read_u32(np, "clock-frequency", &clock_frequency))
 		pdata->bus_hz = clock_frequency;
 
diff --git a/include/linux/mmc/dw_mmc.h b/include/linux/mmc/dw_mmc.h
index 17cb95a..ee4bb30 100644
--- a/include/linux/mmc/dw_mmc.h
+++ b/include/linux/mmc/dw_mmc.h
@@ -108,6 +108,8 @@ struct dw_mci_dma_slave {
  * @slot: Slots sharing this MMC controller.
  * @fifo_depth: depth of FIFO.
  * @data_addr_override: override fifo reg offset with this value.
+ * @wm_aligned: force fifo watermark equal with data length in PIO mode.
+ *	Set as true if alignment is needed.
  * @data_shift: log2 of FIFO item size.
  * @part_buf_start: Start index in part_buf.
  * @part_buf_count: Bytes of partial data in part_buf.
@@ -156,6 +158,7 @@ struct dw_mci {
 	void __iomem		*regs;
 	void __iomem		*fifo_reg;
 	u32			data_addr_override;
+	bool			wm_aligned;
 
 	struct scatterlist	*sg;
 	struct sg_mapping_iter	sg_miter;
-- 
1.9.1

Re: [PATCH v5 0/5] Add intial support to DW MMC host on ZTE SoC

[Jun Nie]

2016-11-08 9:24 GMT+08:00 Jun Nie <jun.nie@linaro.org>:
> Add intial support to DW MMC host on ZTE SoC. It include platform
> specific wrapper driver and workarounds for fifo quirk.
>
> Patches are prepared based on latest dw mmc runtime change:
>    https://github.com/jh80chung/dw-mmc.git for-ulf
>
> Changes vs version 4:
>   - Fix missing empty dts compatible element in the end of compatible array.
>
> Changes vs version 3:
>   - Fix brace error in document.
>
> Changes vs version 2:
>   - Change dt property fifo-addr to data-addr and fifo-watermark-quirk to
>     fifo-watermark-aligned.
>   - Polish ZX MMC driver on minor coding style issues.
>
> Changes vs version 1:
>   - Change fifo-addr-override to fifo-addr and remove its workaround tag in comments.
>   - Remove ZX DW MMC driver reset cap in driver, which can be added in dt nodes.
>
> Jun Nie (5):
>   mmc: dt-bindings: add ZTE ZX296718 MMC bindings
>   mmc: zx: Initial support for ZX mmc controller
>   Documentation: synopsys-dw-mshc: add binding for fifo quirks
>   mmc: dw: Add fifo address property
>   mmc: dw: Add fifo watermark alignment property
>
>  .../devicetree/bindings/mmc/synopsys-dw-mshc.txt   |  13 ++
>  .../devicetree/bindings/mmc/zx-dw-mshc.txt         |  34 +++
>  drivers/mmc/host/Kconfig                           |   9 +
>  drivers/mmc/host/Makefile                          |   1 +
>  drivers/mmc/host/dw_mmc-zx.c                       | 242 +++++++++++++++++++++
>  drivers/mmc/host/dw_mmc-zx.h                       |  31 +++
>  drivers/mmc/host/dw_mmc.c                          |  17 +-
>  include/linux/mmc/dw_mmc.h                         |   5 +
>  8 files changed, 349 insertions(+), 3 deletions(-)
>  create mode 100644 Documentation/devicetree/bindings/mmc/zx-dw-mshc.txt
>  create mode 100644 drivers/mmc/host/dw_mmc-zx.c
>  create mode 100644 drivers/mmc/host/dw_mmc-zx.h
>
> --
> 1.9.1
>

Jaehoon Chung,

I assume you do not have more comments on this serial patches, neither
from community. If so, could you help merge them to your trees so that
it can go to mainline later?

Thank you!
Jun

Re: [PATCH v5 0/5] Add intial support to DW MMC host on ZTE SoC

[Jaehoon Chung]

Hi Jun,

On 11/14/2016 12:21 PM, Jun Nie wrote:
> 2016-11-08 9:24 GMT+08:00 Jun Nie <jun.nie@linaro.org>:
>> Add intial support to DW MMC host on ZTE SoC. It include platform
>> specific wrapper driver and workarounds for fifo quirk.
>>
>> Patches are prepared based on latest dw mmc runtime change:
>>    https://github.com/jh80chung/dw-mmc.git for-ulf
>>
>> Changes vs version 4:
>>   - Fix missing empty dts compatible element in the end of compatible array.
>>
>> Changes vs version 3:
>>   - Fix brace error in document.
>>
>> Changes vs version 2:
>>   - Change dt property fifo-addr to data-addr and fifo-watermark-quirk to
>>     fifo-watermark-aligned.
>>   - Polish ZX MMC driver on minor coding style issues.
>>
>> Changes vs version 1:
>>   - Change fifo-addr-override to fifo-addr and remove its workaround tag in comments.
>>   - Remove ZX DW MMC driver reset cap in driver, which can be added in dt nodes.
>>
>> Jun Nie (5):
>>   mmc: dt-bindings: add ZTE ZX296718 MMC bindings
>>   mmc: zx: Initial support for ZX mmc controller
>>   Documentation: synopsys-dw-mshc: add binding for fifo quirks
>>   mmc: dw: Add fifo address property
>>   mmc: dw: Add fifo watermark alignment property
>>
>>  .../devicetree/bindings/mmc/synopsys-dw-mshc.txt   |  13 ++
>>  .../devicetree/bindings/mmc/zx-dw-mshc.txt         |  34 +++
>>  drivers/mmc/host/Kconfig                           |   9 +
>>  drivers/mmc/host/Makefile                          |   1 +
>>  drivers/mmc/host/dw_mmc-zx.c                       | 242 +++++++++++++++++++++
>>  drivers/mmc/host/dw_mmc-zx.h                       |  31 +++
>>  drivers/mmc/host/dw_mmc.c                          |  17 +-
>>  include/linux/mmc/dw_mmc.h                         |   5 +
>>  8 files changed, 349 insertions(+), 3 deletions(-)
>>  create mode 100644 Documentation/devicetree/bindings/mmc/zx-dw-mshc.txt
>>  create mode 100644 drivers/mmc/host/dw_mmc-zx.c
>>  create mode 100644 drivers/mmc/host/dw_mmc-zx.h
>>
>> --
>> 1.9.1
>>
> 
> Jaehoon Chung,
> 
> I assume you do not have more comments on this serial patches, neither
> from community. If so, could you help merge them to your trees so that
> it can go to mainline later?

Sorry for late..because I had holidays until yesterday. :)
Now, I'm checking sequentially. ASAP, i will reply. 
Thanks! 

Best Regards,
Jaehoon Chung

> 
> Thank you!
> Jun
> --
> 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 v5 1/5] mmc: dt-bindings: add ZTE ZX296718 MMC bindings

[Shawn Lin]

On 2016/11/8 9:24, Jun Nie wrote:
> Document the device-tree binding of ZTE MMC host on
> ZX296718 SoC.
>
> Signed-off-by: Jun Nie <jun.nie@linaro.org>
> ---
>  .../devicetree/bindings/mmc/zx-dw-mshc.txt         | 35 ++++++++++++++++++++++
>  1 file changed, 35 insertions(+)
>  create mode 100644 Documentation/devicetree/bindings/mmc/zx-dw-mshc.txt
>
> diff --git a/Documentation/devicetree/bindings/mmc/zx-dw-mshc.txt b/Documentation/devicetree/bindings/mmc/zx-dw-mshc.txt
> new file mode 100644
> index 0000000..c175c4b
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/mmc/zx-dw-mshc.txt
> @@ -0,0 +1,35 @@
> +* ZTE specific extensions to the Synopsys Designware Mobile Storage
> +  Host Controller
> +
> +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 ZTE specific
> +extensions to the Synopsys Designware Mobile Storage Host Controller.
> +
> +Required Properties:
> +
> +* compatible: should be
> +	- "zte,zx296718-dw-mshc": for ZX SoCs
> +
> +Example:
> +
> +	mmc1: mmc@1110000 {
> +		compatible = "zte,zx296718-dw-mshc";
> +		#address-cells = <1>;
> +		#size-cells = <0>;
> +		reg = <0x01110000 0x1000>;
> +		interrupts = <GIC_SPI 15 IRQ_TYPE_LEVEL_HIGH>;
> +		fifo-depth = <32>;
> +		data-addr = <0x200>;
> +		fifo-watermark-aligned;
> +		bus-width = <4>;
> +		clock-frequency = <50000000>;

do you need both clock-frequency and max-frequency here?

> +		clocks = <&topcrm SD0_AHB>, <&topcrm SD0_WCLK>;
> +		clock-names = "biu", "ciu";
> +		num-slots = <1>;
> +		max-frequency = <50000000>;
> +		cap-sdio-irq;
> +		cap-sd-highspeed;
> +		status = "disabled";
> +	};
>


-- 
Best Regards
Shawn Lin

Re: [PATCH v5 2/5] mmc: zx: Initial support for ZX mmc controller

[Shawn Lin]

Hi, Jun,

Mostly it looks fine but some minor nits.

On 2016/11/8 9:24, Jun Nie wrote:
> This platform driver adds initial support for the DW host controller
> found on ZTE SoCs.
>
> It has been tested on ZX296718 EVB board currently. More support on
> timing tuning will be added when hardware is available.
>
> Signed-off-by: Jun Nie <jun.nie@linaro.org>
> ---
>  drivers/mmc/host/Kconfig     |   9 ++
>  drivers/mmc/host/Makefile    |   1 +
>  drivers/mmc/host/dw_mmc-zx.c | 243 +++++++++++++++++++++++++++++++++++++++++++
>  drivers/mmc/host/dw_mmc-zx.h |  31 ++++++
>  4 files changed, 284 insertions(+)
>  create mode 100644 drivers/mmc/host/dw_mmc-zx.c
>  create mode 100644 drivers/mmc/host/dw_mmc-zx.h
>
> diff --git a/drivers/mmc/host/Kconfig b/drivers/mmc/host/Kconfig
> index 5274f50..4dafbc2 100644
> --- a/drivers/mmc/host/Kconfig
> +++ b/drivers/mmc/host/Kconfig
> @@ -662,6 +662,15 @@ config MMC_DW_ROCKCHIP
>  	  Synopsys DesignWare Memory Card Interface driver. Select this option
>  	  for platforms based on RK3066, RK3188 and RK3288 SoC's.
>
> +config MMC_DW_ZX
> +	tristate "ZTE specific extensions for Synopsys DW Memory Card Interface"
> +	depends on MMC_DW && ARCH_ZX
> +	select MMC_DW_PLTFM
> +	help
> +	  This selects support for ZTE SoC specific extensions to the
> +	  Synopsys DesignWare Memory Card Interface driver. Select this option
> +	  for platforms based on ZX296718 SoC's.
> +
>  config MMC_SH_MMCIF
>  	tristate "SuperH Internal MMCIF support"
>  	depends on HAS_DMA
> diff --git a/drivers/mmc/host/Makefile b/drivers/mmc/host/Makefile
> index e2bdaaf..9766143 100644
> --- a/drivers/mmc/host/Makefile
> +++ b/drivers/mmc/host/Makefile
> @@ -48,6 +48,7 @@ obj-$(CONFIG_MMC_DW_EXYNOS)	+= dw_mmc-exynos.o
>  obj-$(CONFIG_MMC_DW_K3)		+= dw_mmc-k3.o
>  obj-$(CONFIG_MMC_DW_PCI)	+= dw_mmc-pci.o
>  obj-$(CONFIG_MMC_DW_ROCKCHIP)	+= dw_mmc-rockchip.o
> +obj-$(CONFIG_MMC_DW_ZX)		+= dw_mmc-zx.o
>  obj-$(CONFIG_MMC_SH_MMCIF)	+= sh_mmcif.o
>  obj-$(CONFIG_MMC_JZ4740)	+= jz4740_mmc.o
>  obj-$(CONFIG_MMC_VUB300)	+= vub300.o
> diff --git a/drivers/mmc/host/dw_mmc-zx.c b/drivers/mmc/host/dw_mmc-zx.c
> new file mode 100644
> index 0000000..c48d851
> --- /dev/null
> +++ b/drivers/mmc/host/dw_mmc-zx.c
> @@ -0,0 +1,243 @@
> +/*
> + * ZX Specific Extensions for Synopsys DW Multimedia Card Interface driver
> + *
> + * Copyright (C) 2016, Linaro Ltd.
> + * Copyright (C) 2016, ZTE Corp.
> + *
> + * 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/clk.h>
> +#include <linux/mfd/syscon.h>
> +#include <linux/mmc/dw_mmc.h>
> +#include <linux/mmc/host.h>
> +#include <linux/mmc/mmc.h>
> +#include <linux/module.h>
> +#include <linux/of.h>
> +#include <linux/platform_device.h>
> +#include <linux/pm_runtime.h>
> +#include <linux/regmap.h>
> +#include <linux/slab.h>
> +
> +#include "dw_mmc.h"
> +#include "dw_mmc-pltfm.h"
> +#include "dw_mmc-zx.h"
> +
> +struct dw_mci_zx_priv_data {
> +	struct regmap	*sysc_base;
> +};
> +
> +enum delay_type {
> +	DELAY_TYPE_READ,	/* read dqs delay */
> +	DELAY_TYPE_CLK,		/* clk sample delay */
> +};
> +
> +static int dw_mci_zx_emmc_set_delay(struct dw_mci *host, unsigned int delay,
> +				    enum delay_type dflag)
> +{
> +	struct dw_mci_zx_priv_data *priv = host->priv;
> +	struct regmap *sysc_base = priv->sysc_base;
> +	unsigned int clksel;
> +	unsigned int loop = 1000;
> +	int ret;
> +
> +	if (!sysc_base)
> +		return -EINVAL;
> +
> +	ret = regmap_update_bits(sysc_base, LB_AON_EMMC_CFG_REG0,
> +				 PARA_HALF_CLK_MODE | PARA_DLL_BYPASS_MODE |
> +				 PARA_PHASE_DET_SEL_MASK |
> +				 PARA_DLL_LOCK_NUM_MASK |
> +				 DLL_REG_SET | PARA_DLL_START_MASK,
> +				 PARA_DLL_START(4) | PARA_DLL_LOCK_NUM(4));
> +	if (ret)
> +		return ret;
> +
> +	ret = regmap_read(sysc_base, LB_AON_EMMC_CFG_REG1, &clksel);
> +	if (ret)
> +		return ret;
> +
> +	if (dflag == DELAY_TYPE_CLK) {
> +		clksel &= ~CLK_SAMP_DELAY_MASK;
> +		clksel |= CLK_SAMP_DELAY(delay);
> +	} else {
> +		clksel &= ~READ_DQS_DELAY_MASK;
> +		clksel |= READ_DQS_DELAY(delay);
> +	}
> +
> +	regmap_write(sysc_base, LB_AON_EMMC_CFG_REG1, clksel);
> +	regmap_update_bits(sysc_base, LB_AON_EMMC_CFG_REG0,
> +			   PARA_DLL_START_MASK | PARA_DLL_LOCK_NUM_MASK |
> +			   DLL_REG_SET,
> +			   PARA_DLL_START(4) | PARA_DLL_LOCK_NUM(4) |
> +			   DLL_REG_SET);
> +
> +	do {
> +		ret = regmap_read(sysc_base, LB_AON_EMMC_CFG_REG2, &clksel);
> +		if (ret)
> +			return ret;
> +
> +	} while (--loop && !(clksel & ZX_DLL_LOCKED));
> +
> +	if (!loop) {
> +		dev_err(host->dev, "Error: %s dll lock fail\n", __func__);
> +		return -EIO;
> +	}
> +
> +	return 0;
> +}
> +
> +static int dw_mci_zx_emmc_execute_tuning(struct dw_mci_slot *slot, u32 opcode)
> +{
> +	struct dw_mci *host = slot->host;
> +	struct mmc_host *mmc = slot->mmc;
> +	int len, start = 0, end = 0, delay, best = 0;
> +
> +	for (delay = 1 ; delay < 128; delay++) {
> +		dw_mci_zx_emmc_set_delay(host, delay, DELAY_TYPE_CLK);

you never check if doing dw_mci_zx_emmc_set_delay successfully?

> +		if (mmc_send_tuning(mmc, opcode, NULL)) {
> +			if (start >= 0) {
> +				end = delay - 1;
> +				/* check and update longest good range */
> +				if ((end - start) > len) {
> +					best = (start + end) >> 1;
> +					len = end - start;
> +				}
> +			}
> +			start = -1;
> +			end = 0;
> +			continue;
> +		}
> +		if (start < 0)
> +			start = delay;
> +	}
> +
> +	if (start >= 0) {
> +		end = delay - 1;
> +		if ((end - start) > len) {
> +			best = (start + end) >> 1;
> +			len = end - start;
> +		}
> +	}
> +	if (best < 0)
> +		return -EIO;
> +
> +	dev_info(host->dev, "%s best range: start %d end %d\n", __func__,
> +		 start, end);
> +	dw_mci_zx_emmc_set_delay(host, best, DELAY_TYPE_CLK);

ditto.

> +	return 0;
> +}
> +
> +static int dw_mci_zx_prepare_hs400_tuning(struct dw_mci *host,
> +					  struct mmc_ios *ios)
> +{
> +	int ret;
> +
> +	/* config phase shift as 90 degree */
> +	ret = dw_mci_zx_emmc_set_delay(host, 32, DELAY_TYPE_READ);
> +	if (ret < 0)
> +		return -EIO;
> +
> +	return 0;
> +}
> +
> +static int dw_mci_zx_execute_tuning(struct dw_mci_slot *slot, u32 opcode)
> +{
> +	struct dw_mci *host = slot->host;
> +
> +	if (host->verid == 0x290a) /* only for emmc */
> +		return dw_mci_zx_emmc_execute_tuning(slot, opcode);
> +	/* TODO: Add 0x210a dedicated tuning for sd/sdio */
> +
> +	return 0;
> +}
> +
> +static int dw_mci_zx_parse_dt(struct dw_mci *host)
> +{
> +	struct device_node *np = host->dev->of_node;
> +	struct device_node *node;
> +	struct dw_mci_zx_priv_data *priv;
> +	struct regmap *sysc_base;
> +	int ret;
> +
> +	/* syscon is needed only by emmc */
> +	node = of_parse_phandle(np, "zte,aon-syscon", 0);
> +	if (node) {
> +		sysc_base = syscon_node_to_regmap(node);
> +		of_node_put(node);
> +
> +		if (IS_ERR(sysc_base)) {
> +			ret = PTR_ERR(sysc_base);
> +			if (ret != -EPROBE_DEFER)
> +				dev_err(host->dev, "Can't get syscon: %d\n",
> +					ret);
> +			return ret;
> +		}
> +	} else {
> +		return 0;
> +	}
> +
> +	priv = devm_kzalloc(host->dev, sizeof(*priv), GFP_KERNEL);
> +	if (!priv)
> +		return -ENOMEM;
> +	priv->sysc_base = sysc_base;
> +	host->priv = priv;
> +
> +	return 0;
> +}
> +
> +static unsigned long zx_dwmmc_caps[3] = {
> +	MMC_CAP_CMD23,
> +	MMC_CAP_CMD23,
> +	MMC_CAP_CMD23,
> +};
> +
> +static const struct dw_mci_drv_data zx_drv_data = {
> +	.caps			= zx_dwmmc_caps,
> +	.execute_tuning		= dw_mci_zx_execute_tuning,
> +	.prepare_hs400_tuning	= dw_mci_zx_prepare_hs400_tuning,
> +	.parse_dt               = dw_mci_zx_parse_dt,
> +};
> +
> +static const struct of_device_id dw_mci_zx_match[] = {
> +	{ .compatible = "zte,zx296718-dw-mshc", .data = &zx_drv_data},
> +	{},
> +};
> +MODULE_DEVICE_TABLE(of, dw_mci_zx_match);
> +
> +static int dw_mci_zx_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_zx_match, pdev->dev.of_node);
> +	drv_data = match->data;
> +
> +	return dw_mci_pltfm_register(pdev, drv_data);
> +}
> +
> +static const struct dev_pm_ops dw_mci_zx_dev_pm_ops = {
> +	SET_SYSTEM_SLEEP_PM_OPS(pm_runtime_force_suspend,
> +				pm_runtime_force_resume)
> +	SET_RUNTIME_PM_OPS(dw_mci_runtime_suspend,
> +			   dw_mci_runtime_resume,
> +			   NULL)
> +};
> +
> +static struct platform_driver dw_mci_zx_pltfm_driver = {
> +	.probe		= dw_mci_zx_probe,
> +	.remove		= dw_mci_pltfm_remove,
> +	.driver		= {
> +		.name		= "dwmmc_zx",
> +		.of_match_table	= dw_mci_zx_match,
> +		.pm		= &dw_mci_zx_dev_pm_ops,
> +	},
> +};
> +
> +module_platform_driver(dw_mci_zx_pltfm_driver);
> +
> +MODULE_DESCRIPTION("ZTE emmc/sd driver");
> +MODULE_LICENSE("GPL v2");
> diff --git a/drivers/mmc/host/dw_mmc-zx.h b/drivers/mmc/host/dw_mmc-zx.h
> new file mode 100644
> index 0000000..f369997
> --- /dev/null
> +++ b/drivers/mmc/host/dw_mmc-zx.h
> @@ -0,0 +1,31 @@
> +#ifndef _DW_MMC_ZX_H_
> +#define _DW_MMC_ZX_H_
> +
> +/* ZX296718 SoC specific DLL register offset. */
> +#define LB_AON_EMMC_CFG_REG0  0x1B0
> +#define LB_AON_EMMC_CFG_REG1  0x1B4
> +#define LB_AON_EMMC_CFG_REG2  0x1B8
> +
> +/* LB_AON_EMMC_CFG_REG0 register defines */
> +#define PARA_DLL_START(x)	((x) & 0xFF)
> +#define PARA_DLL_START_MASK	0xFF
> +#define DLL_REG_SET		BIT(8)
> +#define PARA_DLL_LOCK_NUM(x)	(((x) & 7) << 16)
> +#define PARA_DLL_LOCK_NUM_MASK  (7 << 16)
> +#define PARA_PHASE_DET_SEL(x)	(((x) & 7) << 20)
> +#define PARA_PHASE_DET_SEL_MASK	(7 << 20)
> +#define PARA_DLL_BYPASS_MODE	BIT(23)
> +#define PARA_HALF_CLK_MODE	BIT(24)
> +
> +/* LB_AON_EMMC_CFG_REG1 register defines */
> +#define READ_DQS_DELAY(x)	((x) & 0x7F)
> +#define READ_DQS_DELAY_MASK	(0x7F)
> +#define READ_DQS_BYPASS_MODE	BIT(7)
> +#define CLK_SAMP_DELAY(x)	(((x) & 0x7F) << 8)
> +#define CLK_SAMP_DELAY_MASK	(0x7F << 8)
> +#define CLK_SAMP_BYPASS_MODE	BIT(15)
> +
> +/* LB_AON_EMMC_CFG_REG2 register defines */
> +#define ZX_DLL_LOCKED		BIT(2)
> +
> +#endif /* _DW_MMC_ZX_H_ */
>


-- 
Best Regards
Shawn Lin

Re: [PATCH v5 5/5] mmc: dw: Add fifo watermark alignment property

[Shawn Lin]

On 2016/11/8 9:24, Jun Nie wrote:
> Data done irq is expected if data length is less than
> watermark in PIO mode. But fifo watermark is requested
> to be aligned with data length in some SoC so that TX/RX
> irq can be generated with data done irq. Add the
> watermark alignment to mark this requirement and force
> fifo watermark setting accordingly.
>
> Signed-off-by: Jun Nie <jun.nie@linaro.org>
> ---
>  drivers/mmc/host/dw_mmc.c  | 11 +++++++++--
>  include/linux/mmc/dw_mmc.h |  3 +++
>  2 files changed, 12 insertions(+), 2 deletions(-)
>

Reviewed-by: Shawn Lin <shawn.lin@rock-chips.com>

> diff --git a/drivers/mmc/host/dw_mmc.c b/drivers/mmc/host/dw_mmc.c
> index 696b5e6..6d85ca6 100644
> --- a/drivers/mmc/host/dw_mmc.c
> +++ b/drivers/mmc/host/dw_mmc.c
> @@ -1111,11 +1111,15 @@ static void dw_mci_submit_data(struct dw_mci *host, struct mmc_data *data)
>  		mci_writel(host, CTRL, temp);
>
>  		/*
> -		 * Use the initial fifoth_val for PIO mode.
> +		 * Use the initial fifoth_val for PIO mode. If wm_algined
> +		 * is set, we set watermark same as data size.
>  		 * If next issued data may be transfered by DMA mode,
>  		 * prev_blksz should be invalidated.
>  		 */
> -		mci_writel(host, FIFOTH, host->fifoth_val);
> +		if (host->wm_aligned)
> +			dw_mci_adjust_fifoth(host, data);
> +		else
> +			mci_writel(host, FIFOTH, host->fifoth_val);
>  		host->prev_blksz = 0;
>  	} else {
>  		/*
> @@ -2957,6 +2961,9 @@ static struct dw_mci_board *dw_mci_parse_dt(struct dw_mci *host)
>
>  	of_property_read_u32(np, "data-addr", &host->data_addr_override);
>
> +	if (of_get_property(np, "fifo-watermark-aligned", NULL))
> +		host->wm_aligned = true;
> +
>  	if (!of_property_read_u32(np, "clock-frequency", &clock_frequency))
>  		pdata->bus_hz = clock_frequency;
>
> diff --git a/include/linux/mmc/dw_mmc.h b/include/linux/mmc/dw_mmc.h
> index 17cb95a..ee4bb30 100644
> --- a/include/linux/mmc/dw_mmc.h
> +++ b/include/linux/mmc/dw_mmc.h
> @@ -108,6 +108,8 @@ struct dw_mci_dma_slave {
>   * @slot: Slots sharing this MMC controller.
>   * @fifo_depth: depth of FIFO.
>   * @data_addr_override: override fifo reg offset with this value.
> + * @wm_aligned: force fifo watermark equal with data length in PIO mode.
> + *	Set as true if alignment is needed.
>   * @data_shift: log2 of FIFO item size.
>   * @part_buf_start: Start index in part_buf.
>   * @part_buf_count: Bytes of partial data in part_buf.
> @@ -156,6 +158,7 @@ struct dw_mci {
>  	void __iomem		*regs;
>  	void __iomem		*fifo_reg;
>  	u32			data_addr_override;
> +	bool			wm_aligned;
>
>  	struct scatterlist	*sg;
>  	struct sg_mapping_iter	sg_miter;
>


-- 
Best Regards
Shawn Lin

Re: [PATCH v5 4/5] mmc: dw: Add fifo address property

[Shawn Lin]

On 2016/11/8 9:24, Jun Nie wrote:
> The FIFO address may break default address assumption of 0x100
> (version < 0x240A) and 0x200(version >= 0x240A) in current driver.
> The new property is introduced to override fifo address via DT
> node information.
>
> Signed-off-by: Jun Nie <jun.nie@linaro.org>
> ---
>  drivers/mmc/host/dw_mmc.c  | 6 +++++-
>  include/linux/mmc/dw_mmc.h | 2 ++
>  2 files changed, 7 insertions(+), 1 deletion(-)
>

Looks good to me,

Reviewed-by: Shawn Lin <shawn.lin@rock-chips.com>

> diff --git a/drivers/mmc/host/dw_mmc.c b/drivers/mmc/host/dw_mmc.c
> index 1c9ee36..696b5e6 100644
> --- a/drivers/mmc/host/dw_mmc.c
> +++ b/drivers/mmc/host/dw_mmc.c
> @@ -2955,6 +2955,8 @@ static struct dw_mci_board *dw_mci_parse_dt(struct dw_mci *host)
>
>  	of_property_read_u32(np, "card-detect-delay", &pdata->detect_delay_ms);
>
> +	of_property_read_u32(np, "data-addr", &host->data_addr_override);
> +
>  	if (!of_property_read_u32(np, "clock-frequency", &clock_frequency))
>  		pdata->bus_hz = clock_frequency;
>
> @@ -3158,7 +3160,9 @@ int dw_mci_probe(struct dw_mci *host)
>  	host->verid = SDMMC_GET_VERID(mci_readl(host, VERID));
>  	dev_info(host->dev, "Version ID is %04x\n", host->verid);
>
> -	if (host->verid < DW_MMC_240A)
> +	if (host->data_addr_override)
> +		host->fifo_reg = host->regs + host->data_addr_override;
> +	else if (host->verid < DW_MMC_240A)
>  		host->fifo_reg = host->regs + DATA_OFFSET;
>  	else
>  		host->fifo_reg = host->regs + DATA_240A_OFFSET;
> diff --git a/include/linux/mmc/dw_mmc.h b/include/linux/mmc/dw_mmc.h
> index f5af2bd..17cb95a 100644
> --- a/include/linux/mmc/dw_mmc.h
> +++ b/include/linux/mmc/dw_mmc.h
> @@ -107,6 +107,7 @@ struct dw_mci_dma_slave {
>   * @ciu_clk: Pointer to card interface unit clock instance.
>   * @slot: Slots sharing this MMC controller.
>   * @fifo_depth: depth of FIFO.
> + * @data_addr_override: override fifo reg offset with this value.
>   * @data_shift: log2 of FIFO item size.
>   * @part_buf_start: Start index in part_buf.
>   * @part_buf_count: Bytes of partial data in part_buf.
> @@ -154,6 +155,7 @@ struct dw_mci {
>  	spinlock_t		irq_lock;
>  	void __iomem		*regs;
>  	void __iomem		*fifo_reg;
> +	u32			data_addr_override;
>
>  	struct scatterlist	*sg;
>  	struct sg_mapping_iter	sg_miter;
>


-- 
Best Regards
Shawn Lin

Re: [PATCH v5 2/5] mmc: zx: Initial support for ZX mmc controller

[Jun Nie]

2016-11-14 16:07 GMT+08:00 Shawn Lin <shawn.lin@rock-chips.com>:
> Hi, Jun,
>
> Mostly it looks fine but some minor nits.
>
>
> On 2016/11/8 9:24, Jun Nie wrote:
>>
>> This platform driver adds initial support for the DW host controller
>> found on ZTE SoCs.
>>
>> It has been tested on ZX296718 EVB board currently. More support on
>> timing tuning will be added when hardware is available.
>>
>> Signed-off-by: Jun Nie <jun.nie@linaro.org>
>> ---
>>  drivers/mmc/host/Kconfig     |   9 ++
>>  drivers/mmc/host/Makefile    |   1 +
>>  drivers/mmc/host/dw_mmc-zx.c | 243
>> +++++++++++++++++++++++++++++++++++++++++++
>>  drivers/mmc/host/dw_mmc-zx.h |  31 ++++++
>>  4 files changed, 284 insertions(+)
>>  create mode 100644 drivers/mmc/host/dw_mmc-zx.c
>>  create mode 100644 drivers/mmc/host/dw_mmc-zx.h
>>
>> diff --git a/drivers/mmc/host/Kconfig b/drivers/mmc/host/Kconfig
>> index 5274f50..4dafbc2 100644
>> --- a/drivers/mmc/host/Kconfig
>> +++ b/drivers/mmc/host/Kconfig
>> @@ -662,6 +662,15 @@ config MMC_DW_ROCKCHIP
>>           Synopsys DesignWare Memory Card Interface driver. Select this
>> option
>>           for platforms based on RK3066, RK3188 and RK3288 SoC's.
>>
>> +config MMC_DW_ZX
>> +       tristate "ZTE specific extensions for Synopsys DW Memory Card
>> Interface"
>> +       depends on MMC_DW && ARCH_ZX
>> +       select MMC_DW_PLTFM
>> +       help
>> +         This selects support for ZTE SoC specific extensions to the
>> +         Synopsys DesignWare Memory Card Interface driver. Select this
>> option
>> +         for platforms based on ZX296718 SoC's.
>> +
>>  config MMC_SH_MMCIF
>>         tristate "SuperH Internal MMCIF support"
>>         depends on HAS_DMA
>> diff --git a/drivers/mmc/host/Makefile b/drivers/mmc/host/Makefile
>> index e2bdaaf..9766143 100644
>> --- a/drivers/mmc/host/Makefile
>> +++ b/drivers/mmc/host/Makefile
>> @@ -48,6 +48,7 @@ obj-$(CONFIG_MMC_DW_EXYNOS)   += dw_mmc-exynos.o
>>  obj-$(CONFIG_MMC_DW_K3)                += dw_mmc-k3.o
>>  obj-$(CONFIG_MMC_DW_PCI)       += dw_mmc-pci.o
>>  obj-$(CONFIG_MMC_DW_ROCKCHIP)  += dw_mmc-rockchip.o
>> +obj-$(CONFIG_MMC_DW_ZX)                += dw_mmc-zx.o
>>  obj-$(CONFIG_MMC_SH_MMCIF)     += sh_mmcif.o
>>  obj-$(CONFIG_MMC_JZ4740)       += jz4740_mmc.o
>>  obj-$(CONFIG_MMC_VUB300)       += vub300.o
>> diff --git a/drivers/mmc/host/dw_mmc-zx.c b/drivers/mmc/host/dw_mmc-zx.c
>> new file mode 100644
>> index 0000000..c48d851
>> --- /dev/null
>> +++ b/drivers/mmc/host/dw_mmc-zx.c
>> @@ -0,0 +1,243 @@
>> +/*
>> + * ZX Specific Extensions for Synopsys DW Multimedia Card Interface
>> driver
>> + *
>> + * Copyright (C) 2016, Linaro Ltd.
>> + * Copyright (C) 2016, ZTE Corp.
>> + *
>> + * 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/clk.h>
>> +#include <linux/mfd/syscon.h>
>> +#include <linux/mmc/dw_mmc.h>
>> +#include <linux/mmc/host.h>
>> +#include <linux/mmc/mmc.h>
>> +#include <linux/module.h>
>> +#include <linux/of.h>
>> +#include <linux/platform_device.h>
>> +#include <linux/pm_runtime.h>
>> +#include <linux/regmap.h>
>> +#include <linux/slab.h>
>> +
>> +#include "dw_mmc.h"
>> +#include "dw_mmc-pltfm.h"
>> +#include "dw_mmc-zx.h"
>> +
>> +struct dw_mci_zx_priv_data {
>> +       struct regmap   *sysc_base;
>> +};
>> +
>> +enum delay_type {
>> +       DELAY_TYPE_READ,        /* read dqs delay */
>> +       DELAY_TYPE_CLK,         /* clk sample delay */
>> +};
>> +
>> +static int dw_mci_zx_emmc_set_delay(struct dw_mci *host, unsigned int
>> delay,
>> +                                   enum delay_type dflag)
>> +{
>> +       struct dw_mci_zx_priv_data *priv = host->priv;
>> +       struct regmap *sysc_base = priv->sysc_base;
>> +       unsigned int clksel;
>> +       unsigned int loop = 1000;
>> +       int ret;
>> +
>> +       if (!sysc_base)
>> +               return -EINVAL;
>> +
>> +       ret = regmap_update_bits(sysc_base, LB_AON_EMMC_CFG_REG0,
>> +                                PARA_HALF_CLK_MODE | PARA_DLL_BYPASS_MODE
>> |
>> +                                PARA_PHASE_DET_SEL_MASK |
>> +                                PARA_DLL_LOCK_NUM_MASK |
>> +                                DLL_REG_SET | PARA_DLL_START_MASK,
>> +                                PARA_DLL_START(4) |
>> PARA_DLL_LOCK_NUM(4));
>> +       if (ret)
>> +               return ret;
>> +
>> +       ret = regmap_read(sysc_base, LB_AON_EMMC_CFG_REG1, &clksel);
>> +       if (ret)
>> +               return ret;
>> +
>> +       if (dflag == DELAY_TYPE_CLK) {
>> +               clksel &= ~CLK_SAMP_DELAY_MASK;
>> +               clksel |= CLK_SAMP_DELAY(delay);
>> +       } else {
>> +               clksel &= ~READ_DQS_DELAY_MASK;
>> +               clksel |= READ_DQS_DELAY(delay);
>> +       }
>> +
>> +       regmap_write(sysc_base, LB_AON_EMMC_CFG_REG1, clksel);
>> +       regmap_update_bits(sysc_base, LB_AON_EMMC_CFG_REG0,
>> +                          PARA_DLL_START_MASK | PARA_DLL_LOCK_NUM_MASK |
>> +                          DLL_REG_SET,
>> +                          PARA_DLL_START(4) | PARA_DLL_LOCK_NUM(4) |
>> +                          DLL_REG_SET);
>> +
>> +       do {
>> +               ret = regmap_read(sysc_base, LB_AON_EMMC_CFG_REG2,
>> &clksel);
>> +               if (ret)
>> +                       return ret;
>> +
>> +       } while (--loop && !(clksel & ZX_DLL_LOCKED));
>> +
>> +       if (!loop) {
>> +               dev_err(host->dev, "Error: %s dll lock fail\n", __func__);
>> +               return -EIO;
>> +       }
>> +
>> +       return 0;
>> +}
>> +
>> +static int dw_mci_zx_emmc_execute_tuning(struct dw_mci_slot *slot, u32
>> opcode)
>> +{
>> +       struct dw_mci *host = slot->host;
>> +       struct mmc_host *mmc = slot->mmc;
>> +       int len, start = 0, end = 0, delay, best = 0;
>> +
>> +       for (delay = 1 ; delay < 128; delay++) {
>> +               dw_mci_zx_emmc_set_delay(host, delay, DELAY_TYPE_CLK);
>
>
> you never check if doing dw_mci_zx_emmc_set_delay successfully?
>
Set delay failure leads to tuning command failure. So failure case
will be reflected in tuning result.

>
>> +               if (mmc_send_tuning(mmc, opcode, NULL)) {
>> +                       if (start >= 0) {
>> +                               end = delay - 1;
>> +                               /* check and update longest good range */
>> +                               if ((end - start) > len) {
>> +                                       best = (start + end) >> 1;
>> +                                       len = end - start;
>> +                               }
>> +                       }
>> +                       start = -1;
>> +                       end = 0;
>> +                       continue;
>> +               }
>> +               if (start < 0)
>> +                       start = delay;
>> +       }
>> +
>> +       if (start >= 0) {
>> +               end = delay - 1;
>> +               if ((end - start) > len) {
>> +                       best = (start + end) >> 1;
>> +                       len = end - start;
>> +               }
>> +       }
>> +       if (best < 0)
>> +               return -EIO;
>> +
>> +       dev_info(host->dev, "%s best range: start %d end %d\n", __func__,
>> +                start, end);
>> +       dw_mci_zx_emmc_set_delay(host, best, DELAY_TYPE_CLK);
>
>
> ditto.
>
>
>> +       return 0;
>> +}
>> +
>> +static int dw_mci_zx_prepare_hs400_tuning(struct dw_mci *host,
>> +                                         struct mmc_ios *ios)
>> +{
>> +       int ret;
>> +
>> +       /* config phase shift as 90 degree */
>> +       ret = dw_mci_zx_emmc_set_delay(host, 32, DELAY_TYPE_READ);
>> +       if (ret < 0)
>> +               return -EIO;
>> +
>> +       return 0;
>> +}
>> +
>> +static int dw_mci_zx_execute_tuning(struct dw_mci_slot *slot, u32 opcode)
>> +{
>> +       struct dw_mci *host = slot->host;
>> +
>> +       if (host->verid == 0x290a) /* only for emmc */
>> +               return dw_mci_zx_emmc_execute_tuning(slot, opcode);
>> +       /* TODO: Add 0x210a dedicated tuning for sd/sdio */
>> +
>> +       return 0;
>> +}
>> +
>> +static int dw_mci_zx_parse_dt(struct dw_mci *host)
>> +{
>> +       struct device_node *np = host->dev->of_node;
>> +       struct device_node *node;
>> +       struct dw_mci_zx_priv_data *priv;
>> +       struct regmap *sysc_base;
>> +       int ret;
>> +
>> +       /* syscon is needed only by emmc */
>> +       node = of_parse_phandle(np, "zte,aon-syscon", 0);
>> +       if (node) {
>> +               sysc_base = syscon_node_to_regmap(node);
>> +               of_node_put(node);
>> +
>> +               if (IS_ERR(sysc_base)) {
>> +                       ret = PTR_ERR(sysc_base);
>> +                       if (ret != -EPROBE_DEFER)
>> +                               dev_err(host->dev, "Can't get syscon:
>> %d\n",
>> +                                       ret);
>> +                       return ret;
>> +               }
>> +       } else {
>> +               return 0;
>> +       }
>> +
>> +       priv = devm_kzalloc(host->dev, sizeof(*priv), GFP_KERNEL);
>> +       if (!priv)
>> +               return -ENOMEM;
>> +       priv->sysc_base = sysc_base;
>> +       host->priv = priv;
>> +
>> +       return 0;
>> +}
>> +
>> +static unsigned long zx_dwmmc_caps[3] = {
>> +       MMC_CAP_CMD23,
>> +       MMC_CAP_CMD23,
>> +       MMC_CAP_CMD23,
>> +};
>> +
>> +static const struct dw_mci_drv_data zx_drv_data = {
>> +       .caps                   = zx_dwmmc_caps,
>> +       .execute_tuning         = dw_mci_zx_execute_tuning,
>> +       .prepare_hs400_tuning   = dw_mci_zx_prepare_hs400_tuning,
>> +       .parse_dt               = dw_mci_zx_parse_dt,
>> +};
>> +
>> +static const struct of_device_id dw_mci_zx_match[] = {
>> +       { .compatible = "zte,zx296718-dw-mshc", .data = &zx_drv_data},
>> +       {},
>> +};
>> +MODULE_DEVICE_TABLE(of, dw_mci_zx_match);
>> +
>> +static int dw_mci_zx_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_zx_match, pdev->dev.of_node);
>> +       drv_data = match->data;
>> +
>> +       return dw_mci_pltfm_register(pdev, drv_data);
>> +}
>> +
>> +static const struct dev_pm_ops dw_mci_zx_dev_pm_ops = {
>> +       SET_SYSTEM_SLEEP_PM_OPS(pm_runtime_force_suspend,
>> +                               pm_runtime_force_resume)
>> +       SET_RUNTIME_PM_OPS(dw_mci_runtime_suspend,
>> +                          dw_mci_runtime_resume,
>> +                          NULL)
>> +};
>> +
>> +static struct platform_driver dw_mci_zx_pltfm_driver = {
>> +       .probe          = dw_mci_zx_probe,
>> +       .remove         = dw_mci_pltfm_remove,
>> +       .driver         = {
>> +               .name           = "dwmmc_zx",
>> +               .of_match_table = dw_mci_zx_match,
>> +               .pm             = &dw_mci_zx_dev_pm_ops,
>> +       },
>> +};
>> +
>> +module_platform_driver(dw_mci_zx_pltfm_driver);
>> +
>> +MODULE_DESCRIPTION("ZTE emmc/sd driver");
>> +MODULE_LICENSE("GPL v2");
>> diff --git a/drivers/mmc/host/dw_mmc-zx.h b/drivers/mmc/host/dw_mmc-zx.h
>> new file mode 100644
>> index 0000000..f369997
>> --- /dev/null
>> +++ b/drivers/mmc/host/dw_mmc-zx.h
>> @@ -0,0 +1,31 @@
>> +#ifndef _DW_MMC_ZX_H_
>> +#define _DW_MMC_ZX_H_
>> +
>> +/* ZX296718 SoC specific DLL register offset. */
>> +#define LB_AON_EMMC_CFG_REG0  0x1B0
>> +#define LB_AON_EMMC_CFG_REG1  0x1B4
>> +#define LB_AON_EMMC_CFG_REG2  0x1B8
>> +
>> +/* LB_AON_EMMC_CFG_REG0 register defines */
>> +#define PARA_DLL_START(x)      ((x) & 0xFF)
>> +#define PARA_DLL_START_MASK    0xFF
>> +#define DLL_REG_SET            BIT(8)
>> +#define PARA_DLL_LOCK_NUM(x)   (((x) & 7) << 16)
>> +#define PARA_DLL_LOCK_NUM_MASK  (7 << 16)
>> +#define PARA_PHASE_DET_SEL(x)  (((x) & 7) << 20)
>> +#define PARA_PHASE_DET_SEL_MASK        (7 << 20)
>> +#define PARA_DLL_BYPASS_MODE   BIT(23)
>> +#define PARA_HALF_CLK_MODE     BIT(24)
>> +
>> +/* LB_AON_EMMC_CFG_REG1 register defines */
>> +#define READ_DQS_DELAY(x)      ((x) & 0x7F)
>> +#define READ_DQS_DELAY_MASK    (0x7F)
>> +#define READ_DQS_BYPASS_MODE   BIT(7)
>> +#define CLK_SAMP_DELAY(x)      (((x) & 0x7F) << 8)
>> +#define CLK_SAMP_DELAY_MASK    (0x7F << 8)
>> +#define CLK_SAMP_BYPASS_MODE   BIT(15)
>> +
>> +/* LB_AON_EMMC_CFG_REG2 register defines */
>> +#define ZX_DLL_LOCKED          BIT(2)
>> +
>> +#endif /* _DW_MMC_ZX_H_ */
>>
>
>
> --
> Best Regards
> Shawn Lin
>

Re: [PATCH v5 2/5] mmc: zx: Initial support for ZX mmc controller

[Shawn Lin]

在 2016/11/14 16:50, Jun Nie 写道:
> 2016-11-14 16:07 GMT+08:00 Shawn Lin <shawn.lin@rock-chips.com>:
>> Hi, Jun,
>>
>> Mostly it looks fine but some minor nits.
>>
>>
>> On 2016/11/8 9:24, Jun Nie wrote:
>>>
>>> This platform driver adds initial support for the DW host controller
>>> found on ZTE SoCs.
>>>
>>> It has been tested on ZX296718 EVB board currently. More support on
>>> timing tuning will be added when hardware is available.
>>>
>>> Signed-off-by: Jun Nie <jun.nie@linaro.org>
>>> ---
>>>  drivers/mmc/host/Kconfig     |   9 ++
>>>  drivers/mmc/host/Makefile    |   1 +
>>>  drivers/mmc/host/dw_mmc-zx.c | 243
>>> +++++++++++++++++++++++++++++++++++++++++++
>>>  drivers/mmc/host/dw_mmc-zx.h |  31 ++++++
>>>  4 files changed, 284 insertions(+)
>>>  create mode 100644 drivers/mmc/host/dw_mmc-zx.c
>>>  create mode 100644 drivers/mmc/host/dw_mmc-zx.h
>>>
>>> diff --git a/drivers/mmc/host/Kconfig b/drivers/mmc/host/Kconfig
>>> index 5274f50..4dafbc2 100644
>>> --- a/drivers/mmc/host/Kconfig
>>> +++ b/drivers/mmc/host/Kconfig
>>> @@ -662,6 +662,15 @@ config MMC_DW_ROCKCHIP
>>>           Synopsys DesignWare Memory Card Interface driver. Select this
>>> option
>>>           for platforms based on RK3066, RK3188 and RK3288 SoC's.
>>>
>>> +config MMC_DW_ZX
>>> +       tristate "ZTE specific extensions for Synopsys DW Memory Card
>>> Interface"
>>> +       depends on MMC_DW && ARCH_ZX
>>> +       select MMC_DW_PLTFM
>>> +       help
>>> +         This selects support for ZTE SoC specific extensions to the
>>> +         Synopsys DesignWare Memory Card Interface driver. Select this
>>> option
>>> +         for platforms based on ZX296718 SoC's.
>>> +
>>>  config MMC_SH_MMCIF
>>>         tristate "SuperH Internal MMCIF support"
>>>         depends on HAS_DMA
>>> diff --git a/drivers/mmc/host/Makefile b/drivers/mmc/host/Makefile
>>> index e2bdaaf..9766143 100644
>>> --- a/drivers/mmc/host/Makefile
>>> +++ b/drivers/mmc/host/Makefile
>>> @@ -48,6 +48,7 @@ obj-$(CONFIG_MMC_DW_EXYNOS)   += dw_mmc-exynos.o
>>>  obj-$(CONFIG_MMC_DW_K3)                += dw_mmc-k3.o
>>>  obj-$(CONFIG_MMC_DW_PCI)       += dw_mmc-pci.o
>>>  obj-$(CONFIG_MMC_DW_ROCKCHIP)  += dw_mmc-rockchip.o
>>> +obj-$(CONFIG_MMC_DW_ZX)                += dw_mmc-zx.o
>>>  obj-$(CONFIG_MMC_SH_MMCIF)     += sh_mmcif.o
>>>  obj-$(CONFIG_MMC_JZ4740)       += jz4740_mmc.o
>>>  obj-$(CONFIG_MMC_VUB300)       += vub300.o
>>> diff --git a/drivers/mmc/host/dw_mmc-zx.c b/drivers/mmc/host/dw_mmc-zx.c
>>> new file mode 100644
>>> index 0000000..c48d851
>>> --- /dev/null
>>> +++ b/drivers/mmc/host/dw_mmc-zx.c
>>> @@ -0,0 +1,243 @@
>>> +/*
>>> + * ZX Specific Extensions for Synopsys DW Multimedia Card Interface
>>> driver
>>> + *
>>> + * Copyright (C) 2016, Linaro Ltd.
>>> + * Copyright (C) 2016, ZTE Corp.
>>> + *
>>> + * 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/clk.h>
>>> +#include <linux/mfd/syscon.h>
>>> +#include <linux/mmc/dw_mmc.h>
>>> +#include <linux/mmc/host.h>
>>> +#include <linux/mmc/mmc.h>
>>> +#include <linux/module.h>
>>> +#include <linux/of.h>
>>> +#include <linux/platform_device.h>
>>> +#include <linux/pm_runtime.h>
>>> +#include <linux/regmap.h>
>>> +#include <linux/slab.h>
>>> +
>>> +#include "dw_mmc.h"
>>> +#include "dw_mmc-pltfm.h"
>>> +#include "dw_mmc-zx.h"
>>> +
>>> +struct dw_mci_zx_priv_data {
>>> +       struct regmap   *sysc_base;
>>> +};
>>> +
>>> +enum delay_type {
>>> +       DELAY_TYPE_READ,        /* read dqs delay */
>>> +       DELAY_TYPE_CLK,         /* clk sample delay */
>>> +};
>>> +
>>> +static int dw_mci_zx_emmc_set_delay(struct dw_mci *host, unsigned int
>>> delay,
>>> +                                   enum delay_type dflag)
>>> +{
>>> +       struct dw_mci_zx_priv_data *priv = host->priv;
>>> +       struct regmap *sysc_base = priv->sysc_base;
>>> +       unsigned int clksel;
>>> +       unsigned int loop = 1000;
>>> +       int ret;
>>> +
>>> +       if (!sysc_base)
>>> +               return -EINVAL;
>>> +
>>> +       ret = regmap_update_bits(sysc_base, LB_AON_EMMC_CFG_REG0,
>>> +                                PARA_HALF_CLK_MODE | PARA_DLL_BYPASS_MODE
>>> |
>>> +                                PARA_PHASE_DET_SEL_MASK |
>>> +                                PARA_DLL_LOCK_NUM_MASK |
>>> +                                DLL_REG_SET | PARA_DLL_START_MASK,
>>> +                                PARA_DLL_START(4) |
>>> PARA_DLL_LOCK_NUM(4));
>>> +       if (ret)
>>> +               return ret;
>>> +
>>> +       ret = regmap_read(sysc_base, LB_AON_EMMC_CFG_REG1, &clksel);
>>> +       if (ret)
>>> +               return ret;
>>> +
>>> +       if (dflag == DELAY_TYPE_CLK) {
>>> +               clksel &= ~CLK_SAMP_DELAY_MASK;
>>> +               clksel |= CLK_SAMP_DELAY(delay);
>>> +       } else {
>>> +               clksel &= ~READ_DQS_DELAY_MASK;
>>> +               clksel |= READ_DQS_DELAY(delay);
>>> +       }
>>> +
>>> +       regmap_write(sysc_base, LB_AON_EMMC_CFG_REG1, clksel);
>>> +       regmap_update_bits(sysc_base, LB_AON_EMMC_CFG_REG0,
>>> +                          PARA_DLL_START_MASK | PARA_DLL_LOCK_NUM_MASK |
>>> +                          DLL_REG_SET,
>>> +                          PARA_DLL_START(4) | PARA_DLL_LOCK_NUM(4) |
>>> +                          DLL_REG_SET);
>>> +
>>> +       do {
>>> +               ret = regmap_read(sysc_base, LB_AON_EMMC_CFG_REG2,
>>> &clksel);
>>> +               if (ret)
>>> +                       return ret;
>>> +
>>> +       } while (--loop && !(clksel & ZX_DLL_LOCKED));
>>> +
>>> +       if (!loop) {
>>> +               dev_err(host->dev, "Error: %s dll lock fail\n", __func__);
>>> +               return -EIO;
>>> +       }
>>> +
>>> +       return 0;
>>> +}
>>> +
>>> +static int dw_mci_zx_emmc_execute_tuning(struct dw_mci_slot *slot, u32
>>> opcode)
>>> +{
>>> +       struct dw_mci *host = slot->host;
>>> +       struct mmc_host *mmc = slot->mmc;
>>> +       int len, start = 0, end = 0, delay, best = 0;
>>> +
>>> +       for (delay = 1 ; delay < 128; delay++) {
>>> +               dw_mci_zx_emmc_set_delay(host, delay, DELAY_TYPE_CLK);
>>
>>
>> you never check if doing dw_mci_zx_emmc_set_delay successfully?
>>
> Set delay failure leads to tuning command failure. So failure case
> will be reflected in tuning result.

okay, I see. But doesn't that imply you could save some cpu cycle?
Why you still try to do tuning when failing to set delay.

>
>>
>>> +               if (mmc_send_tuning(mmc, opcode, NULL)) {
>>> +                       if (start >= 0) {
>>> +                               end = delay - 1;
>>> +                               /* check and update longest good range */
>>> +                               if ((end - start) > len) {
>>> +                                       best = (start + end) >> 1;
>>> +                                       len = end - start;
>>> +                               }
>>> +                       }
>>> +                       start = -1;
>>> +                       end = 0;
>>> +                       continue;
>>> +               }
>>> +               if (start < 0)
>>> +                       start = delay;
>>> +       }
>>> +
>>> +       if (start >= 0) {
>>> +               end = delay - 1;
>>> +               if ((end - start) > len) {
>>> +                       best = (start + end) >> 1;
>>> +                       len = end - start;
>>> +               }
>>> +       }
>>> +       if (best < 0)
>>> +               return -EIO;
>>> +
>>> +       dev_info(host->dev, "%s best range: start %d end %d\n", __func__,
>>> +                start, end);
>>> +       dw_mci_zx_emmc_set_delay(host, best, DELAY_TYPE_CLK);
>>
>>
>> ditto.
>>
>>
>>> +       return 0;
>>> +}
>>> +
>>> +static int dw_mci_zx_prepare_hs400_tuning(struct dw_mci *host,
>>> +                                         struct mmc_ios *ios)
>>> +{
>>> +       int ret;
>>> +
>>> +       /* config phase shift as 90 degree */
>>> +       ret = dw_mci_zx_emmc_set_delay(host, 32, DELAY_TYPE_READ);
>>> +       if (ret < 0)
>>> +               return -EIO;
>>> +
>>> +       return 0;
>>> +}
>>> +
>>> +static int dw_mci_zx_execute_tuning(struct dw_mci_slot *slot, u32 opcode)
>>> +{
>>> +       struct dw_mci *host = slot->host;
>>> +
>>> +       if (host->verid == 0x290a) /* only for emmc */
>>> +               return dw_mci_zx_emmc_execute_tuning(slot, opcode);
>>> +       /* TODO: Add 0x210a dedicated tuning for sd/sdio */
>>> +
>>> +       return 0;
>>> +}
>>> +
>>> +static int dw_mci_zx_parse_dt(struct dw_mci *host)
>>> +{
>>> +       struct device_node *np = host->dev->of_node;
>>> +       struct device_node *node;
>>> +       struct dw_mci_zx_priv_data *priv;
>>> +       struct regmap *sysc_base;
>>> +       int ret;
>>> +
>>> +       /* syscon is needed only by emmc */
>>> +       node = of_parse_phandle(np, "zte,aon-syscon", 0);
>>> +       if (node) {
>>> +               sysc_base = syscon_node_to_regmap(node);
>>> +               of_node_put(node);
>>> +
>>> +               if (IS_ERR(sysc_base)) {
>>> +                       ret = PTR_ERR(sysc_base);
>>> +                       if (ret != -EPROBE_DEFER)
>>> +                               dev_err(host->dev, "Can't get syscon:
>>> %d\n",
>>> +                                       ret);
>>> +                       return ret;
>>> +               }
>>> +       } else {
>>> +               return 0;
>>> +       }
>>> +
>>> +       priv = devm_kzalloc(host->dev, sizeof(*priv), GFP_KERNEL);
>>> +       if (!priv)
>>> +               return -ENOMEM;
>>> +       priv->sysc_base = sysc_base;
>>> +       host->priv = priv;
>>> +
>>> +       return 0;
>>> +}
>>> +
>>> +static unsigned long zx_dwmmc_caps[3] = {
>>> +       MMC_CAP_CMD23,
>>> +       MMC_CAP_CMD23,
>>> +       MMC_CAP_CMD23,
>>> +};
>>> +
>>> +static const struct dw_mci_drv_data zx_drv_data = {
>>> +       .caps                   = zx_dwmmc_caps,
>>> +       .execute_tuning         = dw_mci_zx_execute_tuning,
>>> +       .prepare_hs400_tuning   = dw_mci_zx_prepare_hs400_tuning,
>>> +       .parse_dt               = dw_mci_zx_parse_dt,
>>> +};
>>> +
>>> +static const struct of_device_id dw_mci_zx_match[] = {
>>> +       { .compatible = "zte,zx296718-dw-mshc", .data = &zx_drv_data},
>>> +       {},
>>> +};
>>> +MODULE_DEVICE_TABLE(of, dw_mci_zx_match);
>>> +
>>> +static int dw_mci_zx_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_zx_match, pdev->dev.of_node);
>>> +       drv_data = match->data;
>>> +
>>> +       return dw_mci_pltfm_register(pdev, drv_data);
>>> +}
>>> +
>>> +static const struct dev_pm_ops dw_mci_zx_dev_pm_ops = {
>>> +       SET_SYSTEM_SLEEP_PM_OPS(pm_runtime_force_suspend,
>>> +                               pm_runtime_force_resume)
>>> +       SET_RUNTIME_PM_OPS(dw_mci_runtime_suspend,
>>> +                          dw_mci_runtime_resume,
>>> +                          NULL)
>>> +};
>>> +
>>> +static struct platform_driver dw_mci_zx_pltfm_driver = {
>>> +       .probe          = dw_mci_zx_probe,
>>> +       .remove         = dw_mci_pltfm_remove,
>>> +       .driver         = {
>>> +               .name           = "dwmmc_zx",
>>> +               .of_match_table = dw_mci_zx_match,
>>> +               .pm             = &dw_mci_zx_dev_pm_ops,
>>> +       },
>>> +};
>>> +
>>> +module_platform_driver(dw_mci_zx_pltfm_driver);
>>> +
>>> +MODULE_DESCRIPTION("ZTE emmc/sd driver");
>>> +MODULE_LICENSE("GPL v2");
>>> diff --git a/drivers/mmc/host/dw_mmc-zx.h b/drivers/mmc/host/dw_mmc-zx.h
>>> new file mode 100644
>>> index 0000000..f369997
>>> --- /dev/null
>>> +++ b/drivers/mmc/host/dw_mmc-zx.h
>>> @@ -0,0 +1,31 @@
>>> +#ifndef _DW_MMC_ZX_H_
>>> +#define _DW_MMC_ZX_H_
>>> +
>>> +/* ZX296718 SoC specific DLL register offset. */
>>> +#define LB_AON_EMMC_CFG_REG0  0x1B0
>>> +#define LB_AON_EMMC_CFG_REG1  0x1B4
>>> +#define LB_AON_EMMC_CFG_REG2  0x1B8
>>> +
>>> +/* LB_AON_EMMC_CFG_REG0 register defines */
>>> +#define PARA_DLL_START(x)      ((x) & 0xFF)
>>> +#define PARA_DLL_START_MASK    0xFF
>>> +#define DLL_REG_SET            BIT(8)
>>> +#define PARA_DLL_LOCK_NUM(x)   (((x) & 7) << 16)
>>> +#define PARA_DLL_LOCK_NUM_MASK  (7 << 16)
>>> +#define PARA_PHASE_DET_SEL(x)  (((x) & 7) << 20)
>>> +#define PARA_PHASE_DET_SEL_MASK        (7 << 20)
>>> +#define PARA_DLL_BYPASS_MODE   BIT(23)
>>> +#define PARA_HALF_CLK_MODE     BIT(24)
>>> +
>>> +/* LB_AON_EMMC_CFG_REG1 register defines */
>>> +#define READ_DQS_DELAY(x)      ((x) & 0x7F)
>>> +#define READ_DQS_DELAY_MASK    (0x7F)
>>> +#define READ_DQS_BYPASS_MODE   BIT(7)
>>> +#define CLK_SAMP_DELAY(x)      (((x) & 0x7F) << 8)
>>> +#define CLK_SAMP_DELAY_MASK    (0x7F << 8)
>>> +#define CLK_SAMP_BYPASS_MODE   BIT(15)
>>> +
>>> +/* LB_AON_EMMC_CFG_REG2 register defines */
>>> +#define ZX_DLL_LOCKED          BIT(2)
>>> +
>>> +#endif /* _DW_MMC_ZX_H_ */
>>>
>>
>>
>> --
>> Best Regards
>> Shawn Lin
>>
> --
> 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
>


-- 
Best Regards
Shawn Lin

Re: [PATCH v5 1/5] mmc: dt-bindings: add ZTE ZX296718 MMC bindings

[Jun Nie]

2016-11-14 15:58 GMT+08:00 Shawn Lin <shawn.lin@rock-chips.com>:
> On 2016/11/8 9:24, Jun Nie wrote:
>>
>> Document the device-tree binding of ZTE MMC host on
>> ZX296718 SoC.
>>
>> Signed-off-by: Jun Nie <jun.nie@linaro.org>
>> ---
>>  .../devicetree/bindings/mmc/zx-dw-mshc.txt         | 35
>> ++++++++++++++++++++++
>>  1 file changed, 35 insertions(+)
>>  create mode 100644 Documentation/devicetree/bindings/mmc/zx-dw-mshc.txt
>>
>> diff --git a/Documentation/devicetree/bindings/mmc/zx-dw-mshc.txt
>> b/Documentation/devicetree/bindings/mmc/zx-dw-mshc.txt
>> new file mode 100644
>> index 0000000..c175c4b
>> --- /dev/null
>> +++ b/Documentation/devicetree/bindings/mmc/zx-dw-mshc.txt
>> @@ -0,0 +1,35 @@
>> +* ZTE specific extensions to the Synopsys Designware Mobile Storage
>> +  Host Controller
>> +
>> +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 ZTE specific
>> +extensions to the Synopsys Designware Mobile Storage Host Controller.
>> +
>> +Required Properties:
>> +
>> +* compatible: should be
>> +       - "zte,zx296718-dw-mshc": for ZX SoCs
>> +
>> +Example:
>> +
>> +       mmc1: mmc@1110000 {
>> +               compatible = "zte,zx296718-dw-mshc";
>> +               #address-cells = <1>;
>> +               #size-cells = <0>;
>> +               reg = <0x01110000 0x1000>;
>> +               interrupts = <GIC_SPI 15 IRQ_TYPE_LEVEL_HIGH>;
>> +               fifo-depth = <32>;
>> +               data-addr = <0x200>;
>> +               fifo-watermark-aligned;
>> +               bus-width = <4>;
>> +               clock-frequency = <50000000>;
>
>
> do you need both clock-frequency and max-frequency here?

According to dts document, clock-frequency is for clock configuration
when dw core probe. max-frequency is for mmc core to limit max
frequency for any cards at any time. Do you have any suggestion? Thank
you for your time!

>
>> +               clocks = <&topcrm SD0_AHB>, <&topcrm SD0_WCLK>;
>> +               clock-names = "biu", "ciu";
>> +               num-slots = <1>;
>> +               max-frequency = <50000000>;
>> +               cap-sdio-irq;
>> +               cap-sd-highspeed;
>> +               status = "disabled";
>> +       };
>>
>
>
> --
> Best Regards
> Shawn Lin
>

Re: [PATCH v5 2/5] mmc: zx: Initial support for ZX mmc controller

[Jun Nie]

2016-11-14 17:03 GMT+08:00 Shawn Lin <shawn.lin@rock-chips.com>:
> 在 2016/11/14 16:50, Jun Nie 写道:
>>
>> 2016-11-14 16:07 GMT+08:00 Shawn Lin <shawn.lin@rock-chips.com>:
>>>
>>> Hi, Jun,
>>>
>>> Mostly it looks fine but some minor nits.
>>>
>>>
>>> On 2016/11/8 9:24, Jun Nie wrote:
>>>>
>>>>
>>>> This platform driver adds initial support for the DW host controller
>>>> found on ZTE SoCs.
>>>>
>>>> It has been tested on ZX296718 EVB board currently. More support on
>>>> timing tuning will be added when hardware is available.
>>>>
>>>> Signed-off-by: Jun Nie <jun.nie@linaro.org>
>>>> ---
>>>>  drivers/mmc/host/Kconfig     |   9 ++
>>>>  drivers/mmc/host/Makefile    |   1 +
>>>>  drivers/mmc/host/dw_mmc-zx.c | 243
>>>> +++++++++++++++++++++++++++++++++++++++++++
>>>>  drivers/mmc/host/dw_mmc-zx.h |  31 ++++++
>>>>  4 files changed, 284 insertions(+)
>>>>  create mode 100644 drivers/mmc/host/dw_mmc-zx.c
>>>>  create mode 100644 drivers/mmc/host/dw_mmc-zx.h
>>>>
>>>> diff --git a/drivers/mmc/host/Kconfig b/drivers/mmc/host/Kconfig
>>>> index 5274f50..4dafbc2 100644
>>>> --- a/drivers/mmc/host/Kconfig
>>>> +++ b/drivers/mmc/host/Kconfig
>>>> @@ -662,6 +662,15 @@ config MMC_DW_ROCKCHIP
>>>>           Synopsys DesignWare Memory Card Interface driver. Select this
>>>> option
>>>>           for platforms based on RK3066, RK3188 and RK3288 SoC's.
>>>>
>>>> +config MMC_DW_ZX
>>>> +       tristate "ZTE specific extensions for Synopsys DW Memory Card
>>>> Interface"
>>>> +       depends on MMC_DW && ARCH_ZX
>>>> +       select MMC_DW_PLTFM
>>>> +       help
>>>> +         This selects support for ZTE SoC specific extensions to the
>>>> +         Synopsys DesignWare Memory Card Interface driver. Select this
>>>> option
>>>> +         for platforms based on ZX296718 SoC's.
>>>> +
>>>>  config MMC_SH_MMCIF
>>>>         tristate "SuperH Internal MMCIF support"
>>>>         depends on HAS_DMA
>>>> diff --git a/drivers/mmc/host/Makefile b/drivers/mmc/host/Makefile
>>>> index e2bdaaf..9766143 100644
>>>> --- a/drivers/mmc/host/Makefile
>>>> +++ b/drivers/mmc/host/Makefile
>>>> @@ -48,6 +48,7 @@ obj-$(CONFIG_MMC_DW_EXYNOS)   += dw_mmc-exynos.o
>>>>  obj-$(CONFIG_MMC_DW_K3)                += dw_mmc-k3.o
>>>>  obj-$(CONFIG_MMC_DW_PCI)       += dw_mmc-pci.o
>>>>  obj-$(CONFIG_MMC_DW_ROCKCHIP)  += dw_mmc-rockchip.o
>>>> +obj-$(CONFIG_MMC_DW_ZX)                += dw_mmc-zx.o
>>>>  obj-$(CONFIG_MMC_SH_MMCIF)     += sh_mmcif.o
>>>>  obj-$(CONFIG_MMC_JZ4740)       += jz4740_mmc.o
>>>>  obj-$(CONFIG_MMC_VUB300)       += vub300.o
>>>> diff --git a/drivers/mmc/host/dw_mmc-zx.c b/drivers/mmc/host/dw_mmc-zx.c
>>>> new file mode 100644
>>>> index 0000000..c48d851
>>>> --- /dev/null
>>>> +++ b/drivers/mmc/host/dw_mmc-zx.c
>>>> @@ -0,0 +1,243 @@
>>>> +/*
>>>> + * ZX Specific Extensions for Synopsys DW Multimedia Card Interface
>>>> driver
>>>> + *
>>>> + * Copyright (C) 2016, Linaro Ltd.
>>>> + * Copyright (C) 2016, ZTE Corp.
>>>> + *
>>>> + * 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/clk.h>
>>>> +#include <linux/mfd/syscon.h>
>>>> +#include <linux/mmc/dw_mmc.h>
>>>> +#include <linux/mmc/host.h>
>>>> +#include <linux/mmc/mmc.h>
>>>> +#include <linux/module.h>
>>>> +#include <linux/of.h>
>>>> +#include <linux/platform_device.h>
>>>> +#include <linux/pm_runtime.h>
>>>> +#include <linux/regmap.h>
>>>> +#include <linux/slab.h>
>>>> +
>>>> +#include "dw_mmc.h"
>>>> +#include "dw_mmc-pltfm.h"
>>>> +#include "dw_mmc-zx.h"
>>>> +
>>>> +struct dw_mci_zx_priv_data {
>>>> +       struct regmap   *sysc_base;
>>>> +};
>>>> +
>>>> +enum delay_type {
>>>> +       DELAY_TYPE_READ,        /* read dqs delay */
>>>> +       DELAY_TYPE_CLK,         /* clk sample delay */
>>>> +};
>>>> +
>>>> +static int dw_mci_zx_emmc_set_delay(struct dw_mci *host, unsigned int
>>>> delay,
>>>> +                                   enum delay_type dflag)
>>>> +{
>>>> +       struct dw_mci_zx_priv_data *priv = host->priv;
>>>> +       struct regmap *sysc_base = priv->sysc_base;
>>>> +       unsigned int clksel;
>>>> +       unsigned int loop = 1000;
>>>> +       int ret;
>>>> +
>>>> +       if (!sysc_base)
>>>> +               return -EINVAL;
>>>> +
>>>> +       ret = regmap_update_bits(sysc_base, LB_AON_EMMC_CFG_REG0,
>>>> +                                PARA_HALF_CLK_MODE |
>>>> PARA_DLL_BYPASS_MODE
>>>> |
>>>> +                                PARA_PHASE_DET_SEL_MASK |
>>>> +                                PARA_DLL_LOCK_NUM_MASK |
>>>> +                                DLL_REG_SET | PARA_DLL_START_MASK,
>>>> +                                PARA_DLL_START(4) |
>>>> PARA_DLL_LOCK_NUM(4));
>>>> +       if (ret)
>>>> +               return ret;
>>>> +
>>>> +       ret = regmap_read(sysc_base, LB_AON_EMMC_CFG_REG1, &clksel);
>>>> +       if (ret)
>>>> +               return ret;
>>>> +
>>>> +       if (dflag == DELAY_TYPE_CLK) {
>>>> +               clksel &= ~CLK_SAMP_DELAY_MASK;
>>>> +               clksel |= CLK_SAMP_DELAY(delay);
>>>> +       } else {
>>>> +               clksel &= ~READ_DQS_DELAY_MASK;
>>>> +               clksel |= READ_DQS_DELAY(delay);
>>>> +       }
>>>> +
>>>> +       regmap_write(sysc_base, LB_AON_EMMC_CFG_REG1, clksel);
>>>> +       regmap_update_bits(sysc_base, LB_AON_EMMC_CFG_REG0,
>>>> +                          PARA_DLL_START_MASK | PARA_DLL_LOCK_NUM_MASK
>>>> |
>>>> +                          DLL_REG_SET,
>>>> +                          PARA_DLL_START(4) | PARA_DLL_LOCK_NUM(4) |
>>>> +                          DLL_REG_SET);
>>>> +
>>>> +       do {
>>>> +               ret = regmap_read(sysc_base, LB_AON_EMMC_CFG_REG2,
>>>> &clksel);
>>>> +               if (ret)
>>>> +                       return ret;
>>>> +
>>>> +       } while (--loop && !(clksel & ZX_DLL_LOCKED));
>>>> +
>>>> +       if (!loop) {
>>>> +               dev_err(host->dev, "Error: %s dll lock fail\n",
>>>> __func__);
>>>> +               return -EIO;
>>>> +       }
>>>> +
>>>> +       return 0;
>>>> +}
>>>> +
>>>> +static int dw_mci_zx_emmc_execute_tuning(struct dw_mci_slot *slot, u32
>>>> opcode)
>>>> +{
>>>> +       struct dw_mci *host = slot->host;
>>>> +       struct mmc_host *mmc = slot->mmc;
>>>> +       int len, start = 0, end = 0, delay, best = 0;
>>>> +
>>>> +       for (delay = 1 ; delay < 128; delay++) {
>>>> +               dw_mci_zx_emmc_set_delay(host, delay, DELAY_TYPE_CLK);
>>>
>>>
>>>
>>> you never check if doing dw_mci_zx_emmc_set_delay successfully?
>>>
>> Set delay failure leads to tuning command failure. So failure case
>> will be reflected in tuning result.
>
>
> okay, I see. But doesn't that imply you could save some cpu cycle?
> Why you still try to do tuning when failing to set delay.

You are right. Skipping tuning can save CPU cycle in theory. Will
change in next version.

>
>>
>>>
>>>> +               if (mmc_send_tuning(mmc, opcode, NULL)) {
>>>> +                       if (start >= 0) {
>>>> +                               end = delay - 1;
>>>> +                               /* check and update longest good range
>>>> */
>>>> +                               if ((end - start) > len) {
>>>> +                                       best = (start + end) >> 1;
>>>> +                                       len = end - start;
>>>> +                               }
>>>> +                       }
>>>> +                       start = -1;
>>>> +                       end = 0;
>>>> +                       continue;
>>>> +               }
>>>> +               if (start < 0)
>>>> +                       start = delay;
>>>> +       }
>>>> +
>>>> +       if (start >= 0) {
>>>> +               end = delay - 1;
>>>> +               if ((end - start) > len) {
>>>> +                       best = (start + end) >> 1;
>>>> +                       len = end - start;
>>>> +               }
>>>> +       }
>>>> +       if (best < 0)
>>>> +               return -EIO;
>>>> +
>>>> +       dev_info(host->dev, "%s best range: start %d end %d\n",
>>>> __func__,
>>>> +                start, end);
>>>> +       dw_mci_zx_emmc_set_delay(host, best, DELAY_TYPE_CLK);
>>>
>>>
>>>
>>> ditto.
>>>
>>>
>>>> +       return 0;
>>>> +}
>>>> +
>>>> +static int dw_mci_zx_prepare_hs400_tuning(struct dw_mci *host,
>>>> +                                         struct mmc_ios *ios)
>>>> +{
>>>> +       int ret;
>>>> +
>>>> +       /* config phase shift as 90 degree */
>>>> +       ret = dw_mci_zx_emmc_set_delay(host, 32, DELAY_TYPE_READ);
>>>> +       if (ret < 0)
>>>> +               return -EIO;
>>>> +
>>>> +       return 0;
>>>> +}
>>>> +
>>>> +static int dw_mci_zx_execute_tuning(struct dw_mci_slot *slot, u32
>>>> opcode)
>>>> +{
>>>> +       struct dw_mci *host = slot->host;
>>>> +
>>>> +       if (host->verid == 0x290a) /* only for emmc */
>>>> +               return dw_mci_zx_emmc_execute_tuning(slot, opcode);
>>>> +       /* TODO: Add 0x210a dedicated tuning for sd/sdio */
>>>> +
>>>> +       return 0;
>>>> +}
>>>> +
>>>> +static int dw_mci_zx_parse_dt(struct dw_mci *host)
>>>> +{
>>>> +       struct device_node *np = host->dev->of_node;
>>>> +       struct device_node *node;
>>>> +       struct dw_mci_zx_priv_data *priv;
>>>> +       struct regmap *sysc_base;
>>>> +       int ret;
>>>> +
>>>> +       /* syscon is needed only by emmc */
>>>> +       node = of_parse_phandle(np, "zte,aon-syscon", 0);
>>>> +       if (node) {
>>>> +               sysc_base = syscon_node_to_regmap(node);
>>>> +               of_node_put(node);
>>>> +
>>>> +               if (IS_ERR(sysc_base)) {
>>>> +                       ret = PTR_ERR(sysc_base);
>>>> +                       if (ret != -EPROBE_DEFER)
>>>> +                               dev_err(host->dev, "Can't get syscon:
>>>> %d\n",
>>>> +                                       ret);
>>>> +                       return ret;
>>>> +               }
>>>> +       } else {
>>>> +               return 0;
>>>> +       }
>>>> +
>>>> +       priv = devm_kzalloc(host->dev, sizeof(*priv), GFP_KERNEL);
>>>> +       if (!priv)
>>>> +               return -ENOMEM;
>>>> +       priv->sysc_base = sysc_base;
>>>> +       host->priv = priv;
>>>> +
>>>> +       return 0;
>>>> +}
>>>> +
>>>> +static unsigned long zx_dwmmc_caps[3] = {
>>>> +       MMC_CAP_CMD23,
>>>> +       MMC_CAP_CMD23,
>>>> +       MMC_CAP_CMD23,
>>>> +};
>>>> +
>>>> +static const struct dw_mci_drv_data zx_drv_data = {
>>>> +       .caps                   = zx_dwmmc_caps,
>>>> +       .execute_tuning         = dw_mci_zx_execute_tuning,
>>>> +       .prepare_hs400_tuning   = dw_mci_zx_prepare_hs400_tuning,
>>>> +       .parse_dt               = dw_mci_zx_parse_dt,
>>>> +};
>>>> +
>>>> +static const struct of_device_id dw_mci_zx_match[] = {
>>>> +       { .compatible = "zte,zx296718-dw-mshc", .data = &zx_drv_data},
>>>> +       {},
>>>> +};
>>>> +MODULE_DEVICE_TABLE(of, dw_mci_zx_match);
>>>> +
>>>> +static int dw_mci_zx_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_zx_match, pdev->dev.of_node);
>>>> +       drv_data = match->data;
>>>> +
>>>> +       return dw_mci_pltfm_register(pdev, drv_data);
>>>> +}
>>>> +
>>>> +static const struct dev_pm_ops dw_mci_zx_dev_pm_ops = {
>>>> +       SET_SYSTEM_SLEEP_PM_OPS(pm_runtime_force_suspend,
>>>> +                               pm_runtime_force_resume)
>>>> +       SET_RUNTIME_PM_OPS(dw_mci_runtime_suspend,
>>>> +                          dw_mci_runtime_resume,
>>>> +                          NULL)
>>>> +};
>>>> +
>>>> +static struct platform_driver dw_mci_zx_pltfm_driver = {
>>>> +       .probe          = dw_mci_zx_probe,
>>>> +       .remove         = dw_mci_pltfm_remove,
>>>> +       .driver         = {
>>>> +               .name           = "dwmmc_zx",
>>>> +               .of_match_table = dw_mci_zx_match,
>>>> +               .pm             = &dw_mci_zx_dev_pm_ops,
>>>> +       },
>>>> +};
>>>> +
>>>> +module_platform_driver(dw_mci_zx_pltfm_driver);
>>>> +
>>>> +MODULE_DESCRIPTION("ZTE emmc/sd driver");
>>>> +MODULE_LICENSE("GPL v2");
>>>> diff --git a/drivers/mmc/host/dw_mmc-zx.h b/drivers/mmc/host/dw_mmc-zx.h
>>>> new file mode 100644
>>>> index 0000000..f369997
>>>> --- /dev/null
>>>> +++ b/drivers/mmc/host/dw_mmc-zx.h
>>>> @@ -0,0 +1,31 @@
>>>> +#ifndef _DW_MMC_ZX_H_
>>>> +#define _DW_MMC_ZX_H_
>>>> +
>>>> +/* ZX296718 SoC specific DLL register offset. */
>>>> +#define LB_AON_EMMC_CFG_REG0  0x1B0
>>>> +#define LB_AON_EMMC_CFG_REG1  0x1B4
>>>> +#define LB_AON_EMMC_CFG_REG2  0x1B8
>>>> +
>>>> +/* LB_AON_EMMC_CFG_REG0 register defines */
>>>> +#define PARA_DLL_START(x)      ((x) & 0xFF)
>>>> +#define PARA_DLL_START_MASK    0xFF
>>>> +#define DLL_REG_SET            BIT(8)
>>>> +#define PARA_DLL_LOCK_NUM(x)   (((x) & 7) << 16)
>>>> +#define PARA_DLL_LOCK_NUM_MASK  (7 << 16)
>>>> +#define PARA_PHASE_DET_SEL(x)  (((x) & 7) << 20)
>>>> +#define PARA_PHASE_DET_SEL_MASK        (7 << 20)
>>>> +#define PARA_DLL_BYPASS_MODE   BIT(23)
>>>> +#define PARA_HALF_CLK_MODE     BIT(24)
>>>> +
>>>> +/* LB_AON_EMMC_CFG_REG1 register defines */
>>>> +#define READ_DQS_DELAY(x)      ((x) & 0x7F)
>>>> +#define READ_DQS_DELAY_MASK    (0x7F)
>>>> +#define READ_DQS_BYPASS_MODE   BIT(7)
>>>> +#define CLK_SAMP_DELAY(x)      (((x) & 0x7F) << 8)
>>>> +#define CLK_SAMP_DELAY_MASK    (0x7F << 8)
>>>> +#define CLK_SAMP_BYPASS_MODE   BIT(15)
>>>> +
>>>> +/* LB_AON_EMMC_CFG_REG2 register defines */
>>>> +#define ZX_DLL_LOCKED          BIT(2)
>>>> +
>>>> +#endif /* _DW_MMC_ZX_H_ */
>>>>
>>>
>>>
>>> --
>>> Best Regards
>>> Shawn Lin
>>>
>> --
>> 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
>>
>
>
> --
> Best Regards
> Shawn Lin
>

Re: [PATCH v5 1/5] mmc: dt-bindings: add ZTE ZX296718 MMC bindings

[Jaehoon Chung]

On 11/14/2016 07:00 PM, Jun Nie wrote:
> 2016-11-14 15:58 GMT+08:00 Shawn Lin <shawn.lin@rock-chips.com>:
>> On 2016/11/8 9:24, Jun Nie wrote:
>>>
>>> Document the device-tree binding of ZTE MMC host on
>>> ZX296718 SoC.
>>>
>>> Signed-off-by: Jun Nie <jun.nie@linaro.org>
>>> ---
>>>  .../devicetree/bindings/mmc/zx-dw-mshc.txt         | 35
>>> ++++++++++++++++++++++
>>>  1 file changed, 35 insertions(+)
>>>  create mode 100644 Documentation/devicetree/bindings/mmc/zx-dw-mshc.txt
>>>
>>> diff --git a/Documentation/devicetree/bindings/mmc/zx-dw-mshc.txt
>>> b/Documentation/devicetree/bindings/mmc/zx-dw-mshc.txt
>>> new file mode 100644
>>> index 0000000..c175c4b
>>> --- /dev/null
>>> +++ b/Documentation/devicetree/bindings/mmc/zx-dw-mshc.txt
>>> @@ -0,0 +1,35 @@
>>> +* ZTE specific extensions to the Synopsys Designware Mobile Storage
>>> +  Host Controller
>>> +
>>> +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 ZTE specific
>>> +extensions to the Synopsys Designware Mobile Storage Host Controller.
>>> +
>>> +Required Properties:
>>> +
>>> +* compatible: should be
>>> +       - "zte,zx296718-dw-mshc": for ZX SoCs
>>> +
>>> +Example:
>>> +
>>> +       mmc1: mmc@1110000 {
>>> +               compatible = "zte,zx296718-dw-mshc";
>>> +               #address-cells = <1>;
>>> +               #size-cells = <0>;
>>> +               reg = <0x01110000 0x1000>;
>>> +               interrupts = <GIC_SPI 15 IRQ_TYPE_LEVEL_HIGH>;
>>> +               fifo-depth = <32>;
>>> +               data-addr = <0x200>;
>>> +               fifo-watermark-aligned;
>>> +               bus-width = <4>;
>>> +               clock-frequency = <50000000>;
>>
>>
>> do you need both clock-frequency and max-frequency here?
> 
> According to dts document, clock-frequency is for clock configuration
> when dw core probe. max-frequency is for mmc core to limit max
> frequency for any cards at any time. Do you have any suggestion? Thank
> you for your time!

As i know, Jun's comment is right. :)
clock-frequency should be used with clk_set_rate().

> 
>>
>>> +               clocks = <&topcrm SD0_AHB>, <&topcrm SD0_WCLK>;
>>> +               clock-names = "biu", "ciu";
>>> +               num-slots = <1>;
>>> +               max-frequency = <50000000>;
>>> +               cap-sdio-irq;
>>> +               cap-sd-highspeed;
>>> +               status = "disabled";
>>> +       };
>>>
>>
>>
>> --
>> Best Regards
>> Shawn Lin
>>
> 
> 
> 

Re: [PATCH v5 1/5] mmc: dt-bindings: add ZTE ZX296718 MMC bindings

[Shawn Lin]

On 2016/11/14 18:04, Jaehoon Chung wrote:
> On 11/14/2016 07:00 PM, Jun Nie wrote:
>> 2016-11-14 15:58 GMT+08:00 Shawn Lin <shawn.lin@rock-chips.com>:
>>> On 2016/11/8 9:24, Jun Nie wrote:
>>>>
>>>> Document the device-tree binding of ZTE MMC host on
>>>> ZX296718 SoC.
>>>>
>>>> Signed-off-by: Jun Nie <jun.nie@linaro.org>
>>>> ---
>>>>  .../devicetree/bindings/mmc/zx-dw-mshc.txt         | 35
>>>> ++++++++++++++++++++++
>>>>  1 file changed, 35 insertions(+)
>>>>  create mode 100644 Documentation/devicetree/bindings/mmc/zx-dw-mshc.txt
>>>>
>>>> diff --git a/Documentation/devicetree/bindings/mmc/zx-dw-mshc.txt
>>>> b/Documentation/devicetree/bindings/mmc/zx-dw-mshc.txt
>>>> new file mode 100644
>>>> index 0000000..c175c4b
>>>> --- /dev/null
>>>> +++ b/Documentation/devicetree/bindings/mmc/zx-dw-mshc.txt
>>>> @@ -0,0 +1,35 @@
>>>> +* ZTE specific extensions to the Synopsys Designware Mobile Storage
>>>> +  Host Controller
>>>> +
>>>> +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 ZTE specific
>>>> +extensions to the Synopsys Designware Mobile Storage Host Controller.
>>>> +
>>>> +Required Properties:
>>>> +
>>>> +* compatible: should be
>>>> +       - "zte,zx296718-dw-mshc": for ZX SoCs
>>>> +
>>>> +Example:
>>>> +
>>>> +       mmc1: mmc@1110000 {
>>>> +               compatible = "zte,zx296718-dw-mshc";
>>>> +               #address-cells = <1>;
>>>> +               #size-cells = <0>;
>>>> +               reg = <0x01110000 0x1000>;
>>>> +               interrupts = <GIC_SPI 15 IRQ_TYPE_LEVEL_HIGH>;
>>>> +               fifo-depth = <32>;
>>>> +               data-addr = <0x200>;
>>>> +               fifo-watermark-aligned;
>>>> +               bus-width = <4>;
>>>> +               clock-frequency = <50000000>;
>>>
>>>
>>> do you need both clock-frequency and max-frequency here?
>>
>> According to dts document, clock-frequency is for clock configuration
>> when dw core probe. max-frequency is for mmc core to limit max
>> frequency for any cards at any time. Do you have any suggestion? Thank
>> you for your time!
>
> As i know, Jun's comment is right. :)
> clock-frequency should be used with clk_set_rate().

yup, I was thinking that should we reuse max-frequency instead of
clock-frequency in the future?  I saw most of the DT(I didn't check all
of them) assign clock-frequency to the same value as max of clock-
freq-min-max. I think it's pointless if the clock-frequency is different
from max-frequency as both of them will be setted via dw_mmc and finally
we only take min(clock-frequency, max-frquency). Was I missing
something?

What is your opinion, Jaehoon and Jun? :)

>
>>
>>>
>>>> +               clocks = <&topcrm SD0_AHB>, <&topcrm SD0_WCLK>;
>>>> +               clock-names = "biu", "ciu";
>>>> +               num-slots = <1>;
>>>> +               max-frequency = <50000000>;
>>>> +               cap-sdio-irq;
>>>> +               cap-sd-highspeed;
>>>> +               status = "disabled";
>>>> +       };
>>>>
>>>
>>>
>>> --
>>> Best Regards
>>> Shawn Lin
>>>
>>
>>
>>
>
> --
> 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
>


-- 
Best Regards
Shawn Lin

Re: [PATCH v5 1/5] mmc: dt-bindings: add ZTE ZX296718 MMC bindings

[Jaehoon Chung]

On 11/15/2016 09:48 AM, Shawn Lin wrote:
> On 2016/11/14 18:04, Jaehoon Chung wrote:
>> On 11/14/2016 07:00 PM, Jun Nie wrote:
>>> 2016-11-14 15:58 GMT+08:00 Shawn Lin <shawn.lin@rock-chips.com>:
>>>> On 2016/11/8 9:24, Jun Nie wrote:
>>>>>
>>>>> Document the device-tree binding of ZTE MMC host on
>>>>> ZX296718 SoC.
>>>>>
>>>>> Signed-off-by: Jun Nie <jun.nie@linaro.org>
>>>>> ---
>>>>>  .../devicetree/bindings/mmc/zx-dw-mshc.txt         | 35
>>>>> ++++++++++++++++++++++
>>>>>  1 file changed, 35 insertions(+)
>>>>>  create mode 100644 Documentation/devicetree/bindings/mmc/zx-dw-mshc.txt
>>>>>
>>>>> diff --git a/Documentation/devicetree/bindings/mmc/zx-dw-mshc.txt
>>>>> b/Documentation/devicetree/bindings/mmc/zx-dw-mshc.txt
>>>>> new file mode 100644
>>>>> index 0000000..c175c4b
>>>>> --- /dev/null
>>>>> +++ b/Documentation/devicetree/bindings/mmc/zx-dw-mshc.txt
>>>>> @@ -0,0 +1,35 @@
>>>>> +* ZTE specific extensions to the Synopsys Designware Mobile Storage
>>>>> +  Host Controller
>>>>> +
>>>>> +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 ZTE specific
>>>>> +extensions to the Synopsys Designware Mobile Storage Host Controller.
>>>>> +
>>>>> +Required Properties:
>>>>> +
>>>>> +* compatible: should be
>>>>> +       - "zte,zx296718-dw-mshc": for ZX SoCs
>>>>> +
>>>>> +Example:
>>>>> +
>>>>> +       mmc1: mmc@1110000 {
>>>>> +               compatible = "zte,zx296718-dw-mshc";
>>>>> +               #address-cells = <1>;
>>>>> +               #size-cells = <0>;
>>>>> +               reg = <0x01110000 0x1000>;
>>>>> +               interrupts = <GIC_SPI 15 IRQ_TYPE_LEVEL_HIGH>;
>>>>> +               fifo-depth = <32>;
>>>>> +               data-addr = <0x200>;
>>>>> +               fifo-watermark-aligned;
>>>>> +               bus-width = <4>;
>>>>> +               clock-frequency = <50000000>;
>>>>
>>>>
>>>> do you need both clock-frequency and max-frequency here?
>>>
>>> According to dts document, clock-frequency is for clock configuration
>>> when dw core probe. max-frequency is for mmc core to limit max
>>> frequency for any cards at any time. Do you have any suggestion? Thank
>>> you for your time!
>>
>> As i know, Jun's comment is right. :)
>> clock-frequency should be used with clk_set_rate().
> 
> yup, I was thinking that should we reuse max-frequency instead of
> clock-frequency in the future?  I saw most of the DT(I didn't check all
> of them) assign clock-frequency to the same value as max of clock-
> freq-min-max. I think it's pointless if the clock-frequency is different
> from max-frequency as both of them will be setted via dw_mmc and finally
> we only take min(clock-frequency, max-frquency). Was I missing
> something?

Well, clock-frequency is for setting CMU. but max-frequency is not really used for CMU.
For example, clock-frequency is 100MHz. Max-frequency is 50MHz.
It's possible to use. then dwmmc controller should divide to 2 for max-frequency.

There are too many cases. Source clock can be 400MHz or 200MHz..etc..
but maximum clock is decided according to busmode.

I'm not sure because i didn't have HW knowledge..
but in my experience, 
1) 400MHz/2 = 200MHz,
2) 800MHz/4 = 200MHz.

1) and 2) are same value as 200MHz. but those clock phase might be a little difference.

So i want to keep the clock-frequency for setting the initial CMU value.
What your opinion? :)

Best Regards,
Jaehoon Chung

> 
> What is your opinion, Jaehoon and Jun? :)
> 
>>
>>>
>>>>
>>>>> +               clocks = <&topcrm SD0_AHB>, <&topcrm SD0_WCLK>;
>>>>> +               clock-names = "biu", "ciu";
>>>>> +               num-slots = <1>;
>>>>> +               max-frequency = <50000000>;
>>>>> +               cap-sdio-irq;
>>>>> +               cap-sd-highspeed;
>>>>> +               status = "disabled";
>>>>> +       };
>>>>>
>>>>
>>>>
>>>> -- 
>>>> Best Regards
>>>> Shawn Lin
>>>>
>>>
>>>
>>>
>>
>> -- 
>> 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 v5 1/5] mmc: dt-bindings: add ZTE ZX296718 MMC bindings

[Shawn Lin]

On 2016/11/15 13:21, Jaehoon Chung wrote:
> On 11/15/2016 09:48 AM, Shawn Lin wrote:
>> On 2016/11/14 18:04, Jaehoon Chung wrote:
>>> On 11/14/2016 07:00 PM, Jun Nie wrote:
>>>> 2016-11-14 15:58 GMT+08:00 Shawn Lin <shawn.lin@rock-chips.com>:
>>>>> On 2016/11/8 9:24, Jun Nie wrote:
>>>>>>
>>>>>> Document the device-tree binding of ZTE MMC host on
>>>>>> ZX296718 SoC.
>>>>>>
>>>>>> Signed-off-by: Jun Nie <jun.nie@linaro.org>
>>>>>> ---
>>>>>>  .../devicetree/bindings/mmc/zx-dw-mshc.txt         | 35
>>>>>> ++++++++++++++++++++++
>>>>>>  1 file changed, 35 insertions(+)
>>>>>>  create mode 100644 Documentation/devicetree/bindings/mmc/zx-dw-mshc.txt
>>>>>>
>>>>>> diff --git a/Documentation/devicetree/bindings/mmc/zx-dw-mshc.txt
>>>>>> b/Documentation/devicetree/bindings/mmc/zx-dw-mshc.txt
>>>>>> new file mode 100644
>>>>>> index 0000000..c175c4b
>>>>>> --- /dev/null
>>>>>> +++ b/Documentation/devicetree/bindings/mmc/zx-dw-mshc.txt
>>>>>> @@ -0,0 +1,35 @@
>>>>>> +* ZTE specific extensions to the Synopsys Designware Mobile Storage
>>>>>> +  Host Controller
>>>>>> +
>>>>>> +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 ZTE specific
>>>>>> +extensions to the Synopsys Designware Mobile Storage Host Controller.
>>>>>> +
>>>>>> +Required Properties:
>>>>>> +
>>>>>> +* compatible: should be
>>>>>> +       - "zte,zx296718-dw-mshc": for ZX SoCs
>>>>>> +
>>>>>> +Example:
>>>>>> +
>>>>>> +       mmc1: mmc@1110000 {
>>>>>> +               compatible = "zte,zx296718-dw-mshc";
>>>>>> +               #address-cells = <1>;
>>>>>> +               #size-cells = <0>;
>>>>>> +               reg = <0x01110000 0x1000>;
>>>>>> +               interrupts = <GIC_SPI 15 IRQ_TYPE_LEVEL_HIGH>;
>>>>>> +               fifo-depth = <32>;
>>>>>> +               data-addr = <0x200>;
>>>>>> +               fifo-watermark-aligned;
>>>>>> +               bus-width = <4>;
>>>>>> +               clock-frequency = <50000000>;
>>>>>
>>>>>
>>>>> do you need both clock-frequency and max-frequency here?
>>>>
>>>> According to dts document, clock-frequency is for clock configuration
>>>> when dw core probe. max-frequency is for mmc core to limit max
>>>> frequency for any cards at any time. Do you have any suggestion? Thank
>>>> you for your time!
>>>
>>> As i know, Jun's comment is right. :)
>>> clock-frequency should be used with clk_set_rate().
>>
>> yup, I was thinking that should we reuse max-frequency instead of
>> clock-frequency in the future?  I saw most of the DT(I didn't check all
>> of them) assign clock-frequency to the same value as max of clock-
>> freq-min-max. I think it's pointless if the clock-frequency is different
>> from max-frequency as both of them will be setted via dw_mmc and finally
>> we only take min(clock-frequency, max-frquency). Was I missing
>> something?
>
> Well, clock-frequency is for setting CMU. but max-frequency is not really used for CMU.
> For example, clock-frequency is 100MHz. Max-frequency is 50MHz.
> It's possible to use. then dwmmc controller should divide to 2 for max-frequency.

yes, but why shouldn't we ask clock unit to generate max-frequency, so
we don't need to divide 2 here?

>
> There are too many cases. Source clock can be 400MHz or 200MHz..etc..
> but maximum clock is decided according to busmode.
>
> I'm not sure because i didn't have HW knowledge..
> but in my experience,
> 1) 400MHz/2 = 200MHz,
> 2) 800MHz/4 = 200MHz.
>
> 1) and 2) are same value as 200MHz. but those clock phase might be a little difference.

clock rate shouldn't be problem but the clock phase maybe according to
the different clock design.

>
> So i want to keep the clock-frequency for setting the initial CMU value.
> What your opinion? :)

it' okay and I was just think that
(1) most of the host drivers direct use max-frequenct(f_max) for
setting clock source rate and use internal divider to generate other 
different rate. Also you can look at mmc_set_clock function.
That impiles they doesn't care the phase or any differences at all.
(2) If clock-frequency may be a little different with max-frequency,
but I now check all the DT using clock-frequency with clock-freq-min-max
before your cleanup and comfirm that all of the clock-frequency(s) are
the same as the max of clock-freq-min-max, namely max-frequency after
your cleanup. That implies that there are no difference between
setting source rate with clock-frequency and with max-frequency.


Anyway, that is just some random thoughts and shouldn't block this
patchset. We could disscuss this topic later. :)

>
> Best Regards,
> Jaehoon Chung
>
>>
>> What is your opinion, Jaehoon and Jun? :)
>>
>>>
>>>>
>>>>>
>>>>>> +               clocks = <&topcrm SD0_AHB>, <&topcrm SD0_WCLK>;
>>>>>> +               clock-names = "biu", "ciu";
>>>>>> +               num-slots = <1>;
>>>>>> +               max-frequency = <50000000>;
>>>>>> +               cap-sdio-irq;
>>>>>> +               cap-sd-highspeed;
>>>>>> +               status = "disabled";
>>>>>> +       };
>>>>>>
>>>>>
>>>>>
>>>>> --
>>>>> Best Regards
>>>>> Shawn Lin
>>>>>
>>>>
>>>>
>>>>
>>>
>>> --
>>> 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
>>>
>>
>>
>
> --
> 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
>


-- 
Best Regards
Shawn Lin

Re: [PATCH v5 1/5] mmc: dt-bindings: add ZTE ZX296718 MMC bindings

[Jaehoon Chung]

On 11/15/2016 04:49 PM, Shawn Lin wrote:
> On 2016/11/15 13:21, Jaehoon Chung wrote:
>> On 11/15/2016 09:48 AM, Shawn Lin wrote:
>>> On 2016/11/14 18:04, Jaehoon Chung wrote:
>>>> On 11/14/2016 07:00 PM, Jun Nie wrote:
>>>>> 2016-11-14 15:58 GMT+08:00 Shawn Lin <shawn.lin@rock-chips.com>:
>>>>>> On 2016/11/8 9:24, Jun Nie wrote:
>>>>>>>
>>>>>>> Document the device-tree binding of ZTE MMC host on
>>>>>>> ZX296718 SoC.
>>>>>>>
>>>>>>> Signed-off-by: Jun Nie <jun.nie@linaro.org>
>>>>>>> ---
>>>>>>>  .../devicetree/bindings/mmc/zx-dw-mshc.txt         | 35
>>>>>>> ++++++++++++++++++++++
>>>>>>>  1 file changed, 35 insertions(+)
>>>>>>>  create mode 100644 Documentation/devicetree/bindings/mmc/zx-dw-mshc.txt
>>>>>>>
>>>>>>> diff --git a/Documentation/devicetree/bindings/mmc/zx-dw-mshc.txt
>>>>>>> b/Documentation/devicetree/bindings/mmc/zx-dw-mshc.txt
>>>>>>> new file mode 100644
>>>>>>> index 0000000..c175c4b
>>>>>>> --- /dev/null
>>>>>>> +++ b/Documentation/devicetree/bindings/mmc/zx-dw-mshc.txt
>>>>>>> @@ -0,0 +1,35 @@
>>>>>>> +* ZTE specific extensions to the Synopsys Designware Mobile Storage
>>>>>>> +  Host Controller
>>>>>>> +
>>>>>>> +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 ZTE specific
>>>>>>> +extensions to the Synopsys Designware Mobile Storage Host Controller.
>>>>>>> +
>>>>>>> +Required Properties:
>>>>>>> +
>>>>>>> +* compatible: should be
>>>>>>> +       - "zte,zx296718-dw-mshc": for ZX SoCs
>>>>>>> +
>>>>>>> +Example:
>>>>>>> +
>>>>>>> +       mmc1: mmc@1110000 {
>>>>>>> +               compatible = "zte,zx296718-dw-mshc";
>>>>>>> +               #address-cells = <1>;
>>>>>>> +               #size-cells = <0>;
>>>>>>> +               reg = <0x01110000 0x1000>;
>>>>>>> +               interrupts = <GIC_SPI 15 IRQ_TYPE_LEVEL_HIGH>;
>>>>>>> +               fifo-depth = <32>;
>>>>>>> +               data-addr = <0x200>;
>>>>>>> +               fifo-watermark-aligned;
>>>>>>> +               bus-width = <4>;
>>>>>>> +               clock-frequency = <50000000>;
>>>>>>
>>>>>>
>>>>>> do you need both clock-frequency and max-frequency here?
>>>>>
>>>>> According to dts document, clock-frequency is for clock configuration
>>>>> when dw core probe. max-frequency is for mmc core to limit max
>>>>> frequency for any cards at any time. Do you have any suggestion? Thank
>>>>> you for your time!
>>>>
>>>> As i know, Jun's comment is right. :)
>>>> clock-frequency should be used with clk_set_rate().
>>>
>>> yup, I was thinking that should we reuse max-frequency instead of
>>> clock-frequency in the future?  I saw most of the DT(I didn't check all
>>> of them) assign clock-frequency to the same value as max of clock-
>>> freq-min-max. I think it's pointless if the clock-frequency is different
>>> from max-frequency as both of them will be setted via dw_mmc and finally
>>> we only take min(clock-frequency, max-frquency). Was I missing
>>> something?
>>
>> Well, clock-frequency is for setting CMU. but max-frequency is not really used for CMU.
>> For example, clock-frequency is 100MHz. Max-frequency is 50MHz.
>> It's possible to use. then dwmmc controller should divide to 2 for max-frequency.
> 
> yes, but why shouldn't we ask clock unit to generate max-frequency, so
> we don't need to divide 2 here?
> 
>>
>> There are too many cases. Source clock can be 400MHz or 200MHz..etc..
>> but maximum clock is decided according to busmode.
>>
>> I'm not sure because i didn't have HW knowledge..
>> but in my experience,
>> 1) 400MHz/2 = 200MHz,
>> 2) 800MHz/4 = 200MHz.
>>
>> 1) and 2) are same value as 200MHz. but those clock phase might be a little difference.
> 
> clock rate shouldn't be problem but the clock phase maybe according to
> the different clock design.
> 
>>
>> So i want to keep the clock-frequency for setting the initial CMU value.
>> What your opinion? :)
> 
> it' okay and I was just think that
> (1) most of the host drivers direct use max-frequenct(f_max) for
> setting clock source rate and use internal divider to generate other different rate. Also you can look at mmc_set_clock function.
> That impiles they doesn't care the phase or any differences at all.
> (2) If clock-frequency may be a little different with max-frequency,
> but I now check all the DT using clock-frequency with clock-freq-min-max
> before your cleanup and comfirm that all of the clock-frequency(s) are
> the same as the max of clock-freq-min-max, namely max-frequency after
> your cleanup. That implies that there are no difference between
> setting source rate with clock-frequency and with max-frequency.
> 
> 
> Anyway, that is just some random thoughts and shouldn't block this
> patchset. We could disscuss this topic later. :)

Yes, we can discuss about this in future. :)

Best Regards,
Jaehoon Chung

> 
>>
>> Best Regards,
>> Jaehoon Chung
>>
>>>
>>> What is your opinion, Jaehoon and Jun? :)
>>>
>>>>
>>>>>
>>>>>>
>>>>>>> +               clocks = <&topcrm SD0_AHB>, <&topcrm SD0_WCLK>;
>>>>>>> +               clock-names = "biu", "ciu";
>>>>>>> +               num-slots = <1>;
>>>>>>> +               max-frequency = <50000000>;
>>>>>>> +               cap-sdio-irq;
>>>>>>> +               cap-sd-highspeed;
>>>>>>> +               status = "disabled";
>>>>>>> +       };
>>>>>>>
>>>>>>
>>>>>>
>>>>>> -- 
>>>>>> Best Regards
>>>>>> Shawn Lin
>>>>>>
>>>>>
>>>>>
>>>>>
>>>>
>>>> -- 
>>>> 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
>>>>
>>>
>>>
>>
>> -- 
>> 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 v5 0/5] Add intial support to DW MMC host on ZTE SoC

[Jaehoon Chung]

Hi Jun,

On 11/08/2016 10:24 AM, Jun Nie wrote:
> Add intial support to DW MMC host on ZTE SoC. It include platform
> specific wrapper driver and workarounds for fifo quirk.

If you send the patch after modifying Shawn's comment, I will apply these patchset on my repository.
I don't know but i guess you want to apply these patches on v4.10.

Best Regards,
Jaehoon Chung

> 
> Patches are prepared based on latest dw mmc runtime change:
>    https://github.com/jh80chung/dw-mmc.git for-ulf
> 
> Changes vs version 4:
>   - Fix missing empty dts compatible element in the end of compatible array.
> 
> Changes vs version 3:
>   - Fix brace error in document.
> 
> Changes vs version 2:
>   - Change dt property fifo-addr to data-addr and fifo-watermark-quirk to
>     fifo-watermark-aligned.
>   - Polish ZX MMC driver on minor coding style issues.
> 
> Changes vs version 1:
>   - Change fifo-addr-override to fifo-addr and remove its workaround tag in comments.
>   - Remove ZX DW MMC driver reset cap in driver, which can be added in dt nodes.
> 
> Jun Nie (5):
>   mmc: dt-bindings: add ZTE ZX296718 MMC bindings
>   mmc: zx: Initial support for ZX mmc controller
>   Documentation: synopsys-dw-mshc: add binding for fifo quirks
>   mmc: dw: Add fifo address property
>   mmc: dw: Add fifo watermark alignment property
> 
>  .../devicetree/bindings/mmc/synopsys-dw-mshc.txt   |  13 ++
>  .../devicetree/bindings/mmc/zx-dw-mshc.txt         |  34 +++
>  drivers/mmc/host/Kconfig                           |   9 +
>  drivers/mmc/host/Makefile                          |   1 +
>  drivers/mmc/host/dw_mmc-zx.c                       | 242 +++++++++++++++++++++
>  drivers/mmc/host/dw_mmc-zx.h                       |  31 +++
>  drivers/mmc/host/dw_mmc.c                          |  17 +-
>  include/linux/mmc/dw_mmc.h                         |   5 +
>  8 files changed, 349 insertions(+), 3 deletions(-)
>  create mode 100644 Documentation/devicetree/bindings/mmc/zx-dw-mshc.txt
>  create mode 100644 drivers/mmc/host/dw_mmc-zx.c
>  create mode 100644 drivers/mmc/host/dw_mmc-zx.h
> 

Re: [PATCH v5 0/5] Add intial support to DW MMC host on ZTE SoC

[Jun Nie]

2016-11-18 10:16 GMT+08:00 Jaehoon Chung <jh80.chung@samsung.com>:
> Hi Jun,
>
> On 11/08/2016 10:24 AM, Jun Nie wrote:
>> Add intial support to DW MMC host on ZTE SoC. It include platform
>> specific wrapper driver and workarounds for fifo quirk.
>
> If you send the patch after modifying Shawn's comment, I will apply these patchset on my repository.
> I don't know but i guess you want to apply these patches on v4.10.
>
> Best Regards,
> Jaehoon Chung
>
Thanks for you guys' review. Could you please help add your review tag
to V6 patches as I already send out and do not want to send twice to
confuse everyone. Thank you!

Reviewed-by: Shawn Lin <shawn.lin@rock-chips.com>

Jun

>>
>> Patches are prepared based on latest dw mmc runtime change:
>>    https://github.com/jh80chung/dw-mmc.git for-ulf
>>
>> Changes vs version 4:
>>   - Fix missing empty dts compatible element in the end of compatible array.
>>
>> Changes vs version 3:
>>   - Fix brace error in document.
>>
>> Changes vs version 2:
>>   - Change dt property fifo-addr to data-addr and fifo-watermark-quirk to
>>     fifo-watermark-aligned.
>>   - Polish ZX MMC driver on minor coding style issues.
>>
>> Changes vs version 1:
>>   - Change fifo-addr-override to fifo-addr and remove its workaround tag in comments.
>>   - Remove ZX DW MMC driver reset cap in driver, which can be added in dt nodes.
>>
>> Jun Nie (5):
>>   mmc: dt-bindings: add ZTE ZX296718 MMC bindings
>>   mmc: zx: Initial support for ZX mmc controller
>>   Documentation: synopsys-dw-mshc: add binding for fifo quirks
>>   mmc: dw: Add fifo address property
>>   mmc: dw: Add fifo watermark alignment property
>>
>>  .../devicetree/bindings/mmc/synopsys-dw-mshc.txt   |  13 ++
>>  .../devicetree/bindings/mmc/zx-dw-mshc.txt         |  34 +++
>>  drivers/mmc/host/Kconfig                           |   9 +
>>  drivers/mmc/host/Makefile                          |   1 +
>>  drivers/mmc/host/dw_mmc-zx.c                       | 242 +++++++++++++++++++++
>>  drivers/mmc/host/dw_mmc-zx.h                       |  31 +++
>>  drivers/mmc/host/dw_mmc.c                          |  17 +-
>>  include/linux/mmc/dw_mmc.h                         |   5 +
>>  8 files changed, 349 insertions(+), 3 deletions(-)
>>  create mode 100644 Documentation/devicetree/bindings/mmc/zx-dw-mshc.txt
>>  create mode 100644 drivers/mmc/host/dw_mmc-zx.c
>>  create mode 100644 drivers/mmc/host/dw_mmc-zx.h
>>
>