Linux MultiMedia Card development
 help / color / mirror / Atom feed
* Re: [PATCH v5 0/5] Add intial support to DW MMC host on ZTE SoC
From: Jun Nie @ 2016-11-18  6:34 UTC (permalink / raw)
  To: Jaehoon Chung; +Cc: Shawn Guo, xie.baoyou, Ulf Hansson, Jason Liu, linux-mmc
In-Reply-To: <20eca791-015f-1d9f-d24b-213276db08db@samsung.com>

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

^ permalink raw reply

* [PATCH v6 5/5] mmc: dw: Add fifo watermark alignment property
From: Jun Nie @ 2016-11-18  6:29 UTC (permalink / raw)
  To: shawn.guo, xie.baoyou
  Cc: ulf.hansson, jh80.chung, jason.liu, chen.chaokai, lai.binz,
	linux-mmc, Jun Nie
In-Reply-To: <1479450555-19047-1-git-send-email-jun.nie@linaro.org>

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


^ permalink raw reply related

* [PATCH v6 4/5] mmc: dw: Add fifo address property
From: Jun Nie @ 2016-11-18  6:29 UTC (permalink / raw)
  To: shawn.guo, xie.baoyou
  Cc: ulf.hansson, jh80.chung, jason.liu, chen.chaokai, lai.binz,
	linux-mmc, Jun Nie
In-Reply-To: <1479450555-19047-1-git-send-email-jun.nie@linaro.org>

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


^ permalink raw reply related

* [PATCH v6 3/5] Documentation: synopsys-dw-mshc: add binding for fifo quirks
From: Jun Nie @ 2016-11-18  6:29 UTC (permalink / raw)
  To: shawn.guo, xie.baoyou
  Cc: ulf.hansson, jh80.chung, jason.liu, chen.chaokai, lai.binz,
	linux-mmc, Jun Nie
In-Reply-To: <1479450555-19047-1-git-send-email-jun.nie@linaro.org>

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


^ permalink raw reply related

* [PATCH v6 2/5] mmc: zx: Initial support for ZX mmc controller
From: Jun Nie @ 2016-11-18  6:29 UTC (permalink / raw)
  To: shawn.guo, xie.baoyou
  Cc: ulf.hansson, jh80.chung, jason.liu, chen.chaokai, lai.binz,
	linux-mmc, Jun Nie
In-Reply-To: <1479450555-19047-1-git-send-email-jun.nie@linaro.org>

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 | 242 +++++++++++++++++++++++++++++++++++++++++++
 drivers/mmc/host/dw_mmc-zx.h |  31 ++++++
 4 files changed, 283 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..11b9fc3
--- /dev/null
+++ b/drivers/mmc/host/dw_mmc-zx.c
@@ -0,0 +1,242 @@
+/*
+ * 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 ret, len, start = 0, end = 0, delay, best = 0;
+
+	for (delay = 1 ; delay < 128; delay++) {
+		ret = dw_mci_zx_emmc_set_delay(host, delay, DELAY_TYPE_CLK);
+		if (!ret && 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);
+	return dw_mci_zx_emmc_set_delay(host, best, DELAY_TYPE_CLK);
+}
+
+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


^ permalink raw reply related

* [PATCH v6 1/5] mmc: dt-bindings: add ZTE ZX296718 MMC bindings
From: Jun Nie @ 2016-11-18  6:29 UTC (permalink / raw)
  To: shawn.guo, xie.baoyou
  Cc: ulf.hansson, jh80.chung, jason.liu, chen.chaokai, lai.binz,
	linux-mmc, Jun Nie
In-Reply-To: <1479450555-19047-1-git-send-email-jun.nie@linaro.org>

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


^ permalink raw reply related

* [PATCH v6 0/5] Add intial support to DW MMC host on ZTE SoC
From: Jun Nie @ 2016-11-18  6:29 UTC (permalink / raw)
  To: shawn.guo, xie.baoyou
  Cc: ulf.hansson, jh80.chung, jason.liu, chen.chaokai, lai.binz,
	linux-mmc, 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 5:
  - Add clock delay lock status check to save CPU cycle in timing tuning CMD.

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


^ permalink raw reply

* Re: [PATCH v2] mmc: Hynix: add QUIRK_NOTIFY_POWEROFF_ON_SLEEP
From: Shawn Lin @ 2016-11-18  4:27 UTC (permalink / raw)
  To: Thierry Escande, Ulf Hansson; +Cc: shawn.lin, linux-mmc, bhthompson
In-Reply-To: <1479380356-7228-1-git-send-email-thierry.escande@collabora.com>

On 2016/11/17 18:59, Thierry Escande wrote:
> From: zhaojohn <john.zhao@intel.com>
>
> Hynix eMMC devices sometimes take 50% longer to resume from sleep. This
> occurs on Braswell based chromebook with acpi sdhci controller.
>
> Based on a recommendation from Hynix engineers, this patch sends a
> Power-Off Notification using mmc_poweroff_notify() before going to S3 to
> have a resume time consistently within spec. No voltage regulator are
> being cut through this notification but merely tells the eMMC firmware
> that we're about to do so and get it to behave better on resume.
>
> Signed-off-by: zhaojohn <john.zhao@intel.com>
> Signed-off-by: Arindam Roy <arindam.roy@intel.com>
> Tested-by: Freddy Paul <freddy.paul@intel.com>
> Reviewed-by: Icarus W Sparry <icarus.w.sparry@intel.com>
> Reviewed-by: Marc Herbert <marc.herbert@intel.com>
> Reviewed-by: Eric Caruso <ejcaruso@chromium.org>
> Signed-off-by: Brian Norris <briannorris@chromium.org>
> Signed-off-by: Thierry Escande <thierry.escande@collabora.com>

You shouldn't carry on these tags from your local tree.

> ---
>
> Changes in v2:
> - Add a few more details in the commit message
>
>  drivers/mmc/card/block.c | 8 ++++++++
>  drivers/mmc/core/mmc.c   | 8 +++++++-
>  include/linux/mmc/card.h | 2 ++
>  3 files changed, 17 insertions(+), 1 deletion(-)
>
> diff --git a/drivers/mmc/card/block.c b/drivers/mmc/card/block.c
> index 709a872..3db5344 100644
> --- a/drivers/mmc/card/block.c
> +++ b/drivers/mmc/card/block.c
> @@ -2573,6 +2573,14 @@ static const struct mmc_fixup blk_fixups[] =
>  	MMC_FIXUP("V10016", CID_MANFID_KINGSTON, CID_OEMID_ANY, add_quirk_mmc,
>  		  MMC_QUIRK_TRIM_BROKEN),
>
> +	/*
> +	 * Hynix eMMC devices sometimes take 50% longer to resume from sleep.
> +	 * Based on a recommendation from Hynix, send a Power-Off Notification
> +	 * before going to S3 to restore a resume time consistently within spec.
> +	 */

I don't have time to check this although I have handful of Hynix eMMCs
on my test farm.. But it doesn't seem correct to me that *ALL* of the
Hynix eMMCs have the same problems and they won't be fixed?

> +	MMC_FIXUP(CID_NAME_ANY, CID_MANFID_HYNIX, CID_OEMID_ANY, add_quirk_mmc,
> +		  MMC_QUIRK_NOTIFY_POWEROFF_ON_SLEEP),
> +
>  	END_FIXUP
>  };
>
> diff --git a/drivers/mmc/core/mmc.c b/drivers/mmc/core/mmc.c
> index df19777..774d478 100644
> --- a/drivers/mmc/core/mmc.c
> +++ b/drivers/mmc/core/mmc.c
> @@ -1941,8 +1941,14 @@ static int _mmc_suspend(struct mmc_host *host, bool is_suspend)
>  	if (mmc_can_poweroff_notify(host->card) &&
>  		((host->caps2 & MMC_CAP2_FULL_PWR_CYCLE) || !is_suspend))
>  		err = mmc_poweroff_notify(host->card, notify_type);
> -	else if (mmc_can_sleep(host->card))
> +	else if (mmc_can_sleep(host->card)) {
> +		if (host->card->quirks & MMC_QUIRK_NOTIFY_POWEROFF_ON_SLEEP) {
> +			err = mmc_poweroff_notify(host->card, notify_type);
> +			if (err)
> +				goto out;
> +		}
>  		err = mmc_sleep(host);
> +	}
>  	else if (!mmc_host_is_spi(host))
>  		err = mmc_deselect_cards(host);
>
> diff --git a/include/linux/mmc/card.h b/include/linux/mmc/card.h
> index 73fad83..e4940f4 100644
> --- a/include/linux/mmc/card.h
> +++ b/include/linux/mmc/card.h
> @@ -281,6 +281,8 @@ struct mmc_card {
>  #define MMC_QUIRK_BROKEN_IRQ_POLLING	(1<<11)	/* Polling SDIO_CCCR_INTx could create a fake interrupt */
>  #define MMC_QUIRK_TRIM_BROKEN	(1<<12)		/* Skip trim */
>  #define MMC_QUIRK_BROKEN_HPI	(1<<13)		/* Disable broken HPI support */
> +#define MMC_QUIRK_NOTIFY_POWEROFF_ON_SLEEP \
> +				(1<<14)		/* Poweroff notification*/
>
>
>  	unsigned int		erase_size;	/* erase size in sectors */
>


-- 
Best Regards
Shawn Lin


^ permalink raw reply

* Re: [PATCH v8 04/16] ARM: dts: Add xo to sdhc clock node on qcom platforms
From: Andy Gross @ 2016-11-18  3:56 UTC (permalink / raw)
  To: Ritesh Harjani
  Cc: ulf.hansson, linux-mmc, adrian.hunter, sboyd, shawn.lin,
	devicetree, linux-clk, david.brown, linux-arm-msm, georgi.djakov,
	alex.lemberg, mateusz.nowak, Yuliy.Izrailov, asutoshd,
	david.griego, stummala, venkatg, rnayak, pramod.gurav, jeremymc
In-Reply-To: <1479343419-29326-1-git-send-email-riteshh@codeaurora.org>

On Thu, Nov 17, 2016 at 06:13:39AM +0530, Ritesh Harjani wrote:
> Add xo entry to sdhc clock node on all qcom platforms.
> 
> Signed-off-by: Ritesh Harjani <riteshh@codeaurora.org>
> ---
>  arch/arm/boot/dts/qcom-apq8084.dtsi   | 16 ++++++++++------
>  arch/arm/boot/dts/qcom-msm8974.dtsi   | 16 ++++++++++------
>  arch/arm64/boot/dts/qcom/msm8916.dtsi | 10 ++++++----
>  arch/arm64/boot/dts/qcom/msm8996.dtsi |  9 +++++----
>  4 files changed, 31 insertions(+), 20 deletions(-)
> 
> diff --git a/arch/arm/boot/dts/qcom-apq8084.dtsi b/arch/arm/boot/dts/qcom-apq8084.dtsi
> index 39eb7a4..f756cbb 100644
> --- a/arch/arm/boot/dts/qcom-apq8084.dtsi
> +++ b/arch/arm/boot/dts/qcom-apq8084.dtsi
> @@ -182,13 +182,13 @@
>  	};
>  
>  	clocks {
> -		xo_board {
> +		xo_board: xo_board {
>  			compatible = "fixed-clock";
>  			#clock-cells = <0>;
>  			clock-frequency = <19200000>;
>  		};
>  
> -		sleep_clk {
> +		sleep_clk: sleep_clk {
>  			compatible = "fixed-clock";
>  			#clock-cells = <0>;
>  			clock-frequency = <32768>;
> @@ -416,8 +416,10 @@
>  			reg-names = "hc_mem", "core_mem";
>  			interrupts = <0 123 0>, <0 138 0>;
>  			interrupt-names = "hc_irq", "pwr_irq";
> -			clocks = <&gcc GCC_SDCC1_APPS_CLK>, <&gcc GCC_SDCC1_AHB_CLK>;
> -			clock-names = "core", "iface";
> +			clocks = <&gcc GCC_SDCC1_APPS_CLK>,
> +				 <&gcc GCC_SDCC1_AHB_CLK>,
> +				 <&xo_board 0>;

With clock-cells = <0>, this should be <&xo_board>

Somehow this passes the dtc compiler.  But it is still incorrect.  Please fix
all instances of this to use the correct number of cells in the xo_board
references.


Andy

^ permalink raw reply

* Re: [PATCH v5 0/5] Add intial support to DW MMC host on ZTE SoC
From: Jaehoon Chung @ 2016-11-18  2:16 UTC (permalink / raw)
  To: Jun Nie, shawn.guo, xie.baoyou; +Cc: ulf.hansson, jason.liu, linux-mmc
In-Reply-To: <1478568298-18380-1-git-send-email-jun.nie@linaro.org>

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
> 


^ permalink raw reply

* [PATCH] mmc: pwrseq: add support for Marvell SD8787 chip
From: Matt Ranostay @ 2016-11-18  1:55 UTC (permalink / raw)
  To: linux-wireless-u79uwXL29TY76Z2rM5mHXA,
	linux-kernel-u79uwXL29TY76Z2rM5mHXA,
	devicetree-u79uwXL29TY76Z2rM5mHXA,
	linux-mmc-u79uwXL29TY76Z2rM5mHXA
  Cc: Matt Ranostay, Tony Lindgren, Ulf Hansson, Mark Rutland,
	Srinivas Kandagatla

Allow power sequencing for the Marvell SD8787 Wifi/BT chip.
This can be abstracted to other chipsets if needed in the future.

Cc: Tony Lindgren <tony-4v6yS6AI5VpBDgjK7y7TUQ@public.gmane.org>
Cc: Ulf Hansson <ulf.hansson-QSEj5FYQhm4dnm+yROfE0A@public.gmane.org>
Cc: Mark Rutland <mark.rutland-5wv7dgnIgG8@public.gmane.org>
Cc: Srinivas Kandagatla <srinivas.kandagatla-QSEj5FYQhm4dnm+yROfE0A@public.gmane.org>
Signed-off-by: Matt Ranostay <matt-sk+viVC6FLCDq+mSdOJa79kegs52MxvZ@public.gmane.org>
---
 .../devicetree/bindings/mmc/mmc-pwrseq-sd8787.txt  |  14 +++
 .../bindings/net/wireless/marvell-sd8xxx.txt       |   4 +
 drivers/mmc/core/Kconfig                           |  10 ++
 drivers/mmc/core/Makefile                          |   1 +
 drivers/mmc/core/pwrseq_sd8787.c                   | 117 +++++++++++++++++++++
 5 files changed, 146 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/mmc/mmc-pwrseq-sd8787.txt
 create mode 100644 drivers/mmc/core/pwrseq_sd8787.c

diff --git a/Documentation/devicetree/bindings/mmc/mmc-pwrseq-sd8787.txt b/Documentation/devicetree/bindings/mmc/mmc-pwrseq-sd8787.txt
new file mode 100644
index 000000000000..1b658351629b
--- /dev/null
+++ b/Documentation/devicetree/bindings/mmc/mmc-pwrseq-sd8787.txt
@@ -0,0 +1,14 @@
+* Marvell SD8787 power sequence provider
+
+Required properties:
+- compatible: must be "mmc-pwrseq-sd8787".
+- pwndn-gpio: contains a power down GPIO specifier.
+- reset-gpio: contains a reset GPIO specifier.
+
+Example:
+
+	wifi_pwrseq: wifi_pwrseq {
+		compatible = "mmc-pwrseq-sd8787";
+		pwrdn-gpio = <&twl_gpio 0 GPIO_ACTIVE_LOW>;
+		reset-gpio = <&twl_gpio 1 GPIO_ACTIVE_LOW>;
+	}
diff --git a/Documentation/devicetree/bindings/net/wireless/marvell-sd8xxx.txt b/Documentation/devicetree/bindings/net/wireless/marvell-sd8xxx.txt
index c421aba0a5bc..08fd65d35725 100644
--- a/Documentation/devicetree/bindings/net/wireless/marvell-sd8xxx.txt
+++ b/Documentation/devicetree/bindings/net/wireless/marvell-sd8xxx.txt
@@ -32,6 +32,9 @@ Optional properties:
 		 so that the wifi chip can wakeup host platform under certain condition.
 		 during system resume, the irq will be disabled to make sure
 		 unnecessary interrupt is not received.
+  - vmmc-supply: a phandle of a regulator, supplying VCC to the card
+  - mmc-pwrseq:  phandle to the MMC power sequence node. See "mmc-pwrseq-*"
+		 for documentation of MMC power sequence bindings.
 
 Example:
 
@@ -44,6 +47,7 @@ so that firmware can wakeup host using this device side pin.
 &mmc3 {
 	status = "okay";
 	vmmc-supply = <&wlan_en_reg>;
+	mmc-pwrseq = <&wifi_pwrseq>;
 	bus-width = <4>;
 	cap-power-off-card;
 	keep-power-in-suspend;
diff --git a/drivers/mmc/core/Kconfig b/drivers/mmc/core/Kconfig
index 250f223aaa80..cf61d676ac06 100644
--- a/drivers/mmc/core/Kconfig
+++ b/drivers/mmc/core/Kconfig
@@ -12,6 +12,16 @@ config PWRSEQ_EMMC
 	  This driver can also be built as a module. If so, the module
 	  will be called pwrseq_emmc.
 
+config PWRSEQ_SD8787
+	tristate "HW reset support for SD8787 BT + Wifi module"
+	depends on OF && (MWIFIEX || BT_MRVL_SDIO)
+	help
+	  This selects hardware reset support for the SD8787 BT + Wifi
+	  module. By default this option is set to n.
+
+	  This driver can also be built as a module. If so, the module
+	  will be called pwrseq_sd8787.
+
 config PWRSEQ_SIMPLE
 	tristate "Simple HW reset support for MMC"
 	default y
diff --git a/drivers/mmc/core/Makefile b/drivers/mmc/core/Makefile
index f007151dfdc6..f1c060e56d4e 100644
--- a/drivers/mmc/core/Makefile
+++ b/drivers/mmc/core/Makefile
@@ -10,5 +10,6 @@ mmc_core-y			:= core.o bus.o host.o \
 				   quirks.o slot-gpio.o
 mmc_core-$(CONFIG_OF)		+= pwrseq.o
 obj-$(CONFIG_PWRSEQ_SIMPLE)	+= pwrseq_simple.o
+obj-$(CONFIG_PWRSEQ_SD8787)	+= pwrseq_sd8787.o
 obj-$(CONFIG_PWRSEQ_EMMC)	+= pwrseq_emmc.o
 mmc_core-$(CONFIG_DEBUG_FS)	+= debugfs.o
diff --git a/drivers/mmc/core/pwrseq_sd8787.c b/drivers/mmc/core/pwrseq_sd8787.c
new file mode 100644
index 000000000000..f4080fe6439e
--- /dev/null
+++ b/drivers/mmc/core/pwrseq_sd8787.c
@@ -0,0 +1,117 @@
+/*
+ * pwrseq_sd8787.c - power sequence support for Marvell SD8787 BT + Wifi chip
+ *
+ * Copyright (C) 2016 Matt Ranostay <matt-sk+viVC6FLCDq+mSdOJa79kegs52MxvZ@public.gmane.org>
+ *
+ * Based on the original work pwrseq_simple.c
+ *  Copyright (C) 2014 Linaro Ltd
+ *  Author: Ulf Hansson <ulf.hansson-QSEj5FYQhm4dnm+yROfE0A@public.gmane.org>
+ *
+ * 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.
+ *
+ * This program is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
+ * GNU General Public License for more details.
+ *
+ */
+
+#include <linux/delay.h>
+#include <linux/init.h>
+#include <linux/kernel.h>
+#include <linux/platform_device.h>
+#include <linux/module.h>
+#include <linux/slab.h>
+#include <linux/device.h>
+#include <linux/err.h>
+#include <linux/gpio/consumer.h>
+
+#include <linux/mmc/host.h>
+
+#include "pwrseq.h"
+
+struct mmc_pwrseq_sd8787 {
+	struct mmc_pwrseq pwrseq;
+	struct gpio_desc *reset_gpio;
+	struct gpio_desc *pwrdn_gpio;
+};
+
+#define to_pwrseq_sd8787(p) container_of(p, struct mmc_pwrseq_sd8787, pwrseq)
+
+static void mmc_pwrseq_sd8787_pre_power_on(struct mmc_host *host)
+{
+	struct mmc_pwrseq_sd8787 *pwrseq = to_pwrseq_sd8787(host->pwrseq);
+
+	gpiod_set_value_cansleep(pwrseq->reset_gpio, 1);
+
+	msleep(300);
+	gpiod_set_value_cansleep(pwrseq->pwrdn_gpio, 1);
+}
+
+static void mmc_pwrseq_sd8787_power_off(struct mmc_host *host)
+{
+	struct mmc_pwrseq_sd8787 *pwrseq = to_pwrseq_sd8787(host->pwrseq);
+
+	gpiod_set_value_cansleep(pwrseq->pwrdn_gpio, 0);
+	gpiod_set_value_cansleep(pwrseq->reset_gpio, 0);
+}
+
+static const struct mmc_pwrseq_ops mmc_pwrseq_sd8787_ops = {
+	.pre_power_on = mmc_pwrseq_sd8787_pre_power_on,
+	.power_off = mmc_pwrseq_sd8787_power_off,
+};
+
+static const struct of_device_id mmc_pwrseq_sd8787_of_match[] = {
+	{ .compatible = "mmc-pwrseq-sd8787",},
+	{/* sentinel */},
+};
+MODULE_DEVICE_TABLE(of, mmc_pwrseq_sd8787_of_match);
+
+static int mmc_pwrseq_sd8787_probe(struct platform_device *pdev)
+{
+	struct mmc_pwrseq_sd8787 *pwrseq;
+	struct device *dev = &pdev->dev;
+
+	pwrseq = devm_kzalloc(dev, sizeof(*pwrseq), GFP_KERNEL);
+	if (!pwrseq)
+		return -ENOMEM;
+
+	pwrseq->pwrdn_gpio = devm_gpiod_get(dev, "pwrdn", GPIOD_OUT_LOW);
+	if (IS_ERR(pwrseq->pwrdn_gpio))
+		return PTR_ERR(pwrseq->pwrdn_gpio);
+
+	pwrseq->reset_gpio = devm_gpiod_get(dev, "reset", GPIOD_OUT_LOW);
+	if (IS_ERR(pwrseq->reset_gpio))
+		return PTR_ERR(pwrseq->reset_gpio);
+
+	pwrseq->pwrseq.dev = dev;
+	pwrseq->pwrseq.ops = &mmc_pwrseq_sd8787_ops;
+	pwrseq->pwrseq.owner = THIS_MODULE;
+	platform_set_drvdata(pdev, pwrseq);
+
+	return mmc_pwrseq_register(&pwrseq->pwrseq);
+}
+
+static int mmc_pwrseq_sd8787_remove(struct platform_device *pdev)
+{
+	struct mmc_pwrseq_sd8787 *pwrseq = platform_get_drvdata(pdev);
+
+	mmc_pwrseq_unregister(&pwrseq->pwrseq);
+
+	return 0;
+}
+
+static struct platform_driver mmc_pwrseq_sd8787_driver = {
+	.probe = mmc_pwrseq_sd8787_probe,
+	.remove = mmc_pwrseq_sd8787_remove,
+	.driver = {
+		.name = "pwrseq_sd8787",
+		.of_match_table = mmc_pwrseq_sd8787_of_match,
+	},
+};
+
+module_platform_driver(mmc_pwrseq_sd8787_driver);
+MODULE_LICENSE("GPL v2");
-- 
2.7.4

^ permalink raw reply related

* Re: [PATCH v8 05/16] dt-bindings: sdhci-msm: Add xo property
From: Stephen Boyd @ 2016-11-17 23:03 UTC (permalink / raw)
  To: Ritesh Harjani
  Cc: ulf.hansson, linux-mmc, adrian.hunter, andy.gross, shawn.lin,
	devicetree, linux-clk, david.brown, linux-arm-msm, georgi.djakov,
	alex.lemberg, mateusz.nowak, Yuliy.Izrailov, asutoshd,
	david.griego, stummala, venkatg, rnayak, pramod.gurav, jeremymc
In-Reply-To: <1479343623-31163-1-git-send-email-riteshh@codeaurora.org>

On 11/17, Ritesh Harjani wrote:
> Add "xo" property which is tcxo clock.
> 
> Signed-off-by: Ritesh Harjani <riteshh@codeaurora.org>
> ---
>  Documentation/devicetree/bindings/mmc/sdhci-msm.txt | 1 +
>  1 file changed, 1 insertion(+)
> 
> diff --git a/Documentation/devicetree/bindings/mmc/sdhci-msm.txt b/Documentation/devicetree/bindings/mmc/sdhci-msm.txt
> index 485483a..4e61086 100644
> --- a/Documentation/devicetree/bindings/mmc/sdhci-msm.txt
> +++ b/Documentation/devicetree/bindings/mmc/sdhci-msm.txt
> @@ -17,6 +17,7 @@ Required properties:
>  	"iface" - Main peripheral bus clock (PCLK/HCLK - AHB Bus clock) (required)
>  	"core"	- SDC MMC clock (MCLK) (required)
>  	"bus"	- SDCC bus voter clock (optional)
> +	"xo" - TCXO clock (optional)

Seems everything else is tabbed before the dash. Otherwise,

Reviewed-by: Stephen Boyd <sboyd@codeaurora.org>

-- 
Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum,
a Linux Foundation Collaborative Project

^ permalink raw reply

* Re: [PATCH] mmc: sd: Meet alignment requirements for raw_ssr DMA
From: Paul Burton @ 2016-11-17 13:19 UTC (permalink / raw)
  To: Ulf Hansson; +Cc: linux-mmc
In-Reply-To: <CAPDyKFqNNP-M37_ieq2na0agsQvh8oyv=nWdSsW2nQDw+zB9WA@mail.gmail.com>

[-- Attachment #1: Type: text/plain, Size: 6882 bytes --]

Hi Ulf,

On Thursday, 17 November 2016 13:51:02 GMT Ulf Hansson wrote:
> On 11 November 2016 at 15:22, Paul Burton <paul.burton@imgtec.com> wrote:
> > The mmc_read_ssr() function results in DMA to the raw_ssr member of
> > struct mmc_card, which is not guaranteed to be cache line aligned & thus
> > 
> > might not meet the requirements set out in Documentation/DMA-API.txt:
> >   Warnings:  Memory coherency operates at a granularity called the cache
> >   line width.  In order for memory mapped by this API to operate
> >   correctly, the mapped region must begin exactly on a cache line
> >   boundary and end exactly on one (to prevent two separately mapped
> >   regions from sharing a single cache line).  Since the cache line size
> >   may not be known at compile time, the API will not enforce this
> >   requirement.  Therefore, it is recommended that driver writers who
> >   don't take special care to determine the cache line size at run time
> >   only map virtual regions that begin and end on page boundaries (which
> >   are guaranteed also to be cache line boundaries).
> 
> This about virtual regions, not about physical contiguous allocated
> memories, such as those you get via kmalloc(n, GFP_KERNEL). Right?

This is about memory being cache-line aligned. Since the mapping from virtual
to physical operates on pages, which will always be some multiple of a cache
line size, it doesn't matter whether we're talking about virtual or physical
addresses (& it's a good job because semantics get awkward between
architectures - from my MIPS perspective I'd say kmalloc returns a virtual
address).

This has nothing to do with the memory mapped for DMA being physically
contiguous (which as you say, it typically needs to be), it has to do with how
it is kept coherent with caches that the device performing DMA has no
knowledge of or access to.

> 
> > On some systems where DMA is non-coherent this can lead to us losing
> > data that shares cache lines with the raw_ssr array.
> > 
> > Fix this by kmalloc'ing a temporary buffer to perform DMA into. kmalloc
> > will ensure the buffer is suitably aligned, allowing the DMA to be
> > performed without any loss of data.
> > 
> > Signed-off-by: Paul Burton <paul.burton@imgtec.com>
> > Cc: Ulf Hansson <ulf.hansson@linaro.org>
> > Cc: linux-mmc@vger.kernel.org
> > ---
> > 
> >  drivers/mmc/core/sd.c | 12 ++++++++++--
> >  1 file changed, 10 insertions(+), 2 deletions(-)
> > 
> > diff --git a/drivers/mmc/core/sd.c b/drivers/mmc/core/sd.c
> > index 73c762a..f6f40a1 100644
> > --- a/drivers/mmc/core/sd.c
> > +++ b/drivers/mmc/core/sd.c
> > @@ -223,6 +223,7 @@ static int mmc_decode_scr(struct mmc_card *card)
> > 
> >  static int mmc_read_ssr(struct mmc_card *card)
> >  {
> >  
> >         unsigned int au, es, et, eo;
> > 
> > +       u32 *raw_ssr;
> > 
> >         int i;
> >         
> >         if (!(card->csd.cmdclass & CCC_APP_SPEC)) {
> > 
> > @@ -231,14 +232,21 @@ static int mmc_read_ssr(struct mmc_card *card)
> > 
> >                 return 0;
> >         
> >         }
> > 
> > -       if (mmc_app_sd_status(card, card->raw_ssr)) {
> 
> So the struct mmc_card, which contains "u32 raw_ssr[16]", is already
> allocated via using a kzalloc().
> As kzalloc() returns a physically contiguous allocated memory, using
> DMA for reading the data into the memory pointed to by card->raw_ssr
> should be safe.
> 
> Unless I am missing something, of course.

Sadly you are. This is incorrect because the raw_ssr field of struct mmc_card
may not begin or end at a cache-line boundary. On systems where mapping a
buffer for DMA requires cache invalidation we may therefore lose data if it
shares a cache line with some other data. In this case let's say we have
something like this:

0x1000: u32 raw_scr[2];
0x1008: u32 raw_ssr[16];

Now let's say we have 16 byte cache lines and we try to map raw_ssr for DMA.
If the architecture requires that we invalidate the memory being mapped in
caches (in order to avoid any writebacks clobbering the DMA'd data) then we
have a problem because our choice is either to:

- Writeback the line that overlaps with raw_scr, then invalidate it along with
  the rest of the cache lines covering raw_ssr. This won't do because there's
  nothing stopping us from pulling the line back into the cache whilst the DMA
  occurs on some systems - say we access raw_scr whilst the DMA to raw_ssr
  happens or the CPU just speculatively prefetches something. The DMA API
  cannot detect the former & would have no way to deal with it if it could,
  and on systems where the latter is possible we have no choice but to
  invalidate the cache lines again after the DMA is complete. What would we do
  with the first line then? We can't write it back to preserve raw_scr because
  it includes some potentially stale copy of the start of raw_ssr, which we
  would then clobber the DMA'd data with.

- Or, we just invalidate the line which overlaps with raw_scr. We can't do
  this because we might throw away some part of raw_scr.

So we have no choice but to align the buffers used for DMA at a cache line
boundary. This is what the paragraph in Documentation/DMA-API.txt is about &
the MMC code currently fails to meet the requirements it sets out. Allocating
the buffer to be DMA'd to with kmalloc will meet that alignment requirement.
One possible alternative would be to ensure that the raw_ssr field of struct
mmc_card is always aligned at an ARCH_DMA_MINALIGN boundary within the struct,
with nothing after it until another ARCH_DMA_MINALIGN boundary. That seems
like it'd be a bit messy though, and probably easy for someone to break.

Of course when a system has DMA that is coherent with caches there's generally
no need to worry about any of this, but users of the DMA API generally can't
rely on that, again as Documentation/DMA-API.txt says. Any buffers mapped for
DMA should be ensured to be cache-line aligned at minimum giving the
architecture code backing the DMA API a chance to do the right thing for the
given system.

Thanks,
    Paul

> 
> > +       raw_ssr = kmalloc(sizeof(card->raw_ssr), GFP_KERNEL);
> > +       if (!raw_ssr)
> > +               return -ENOMEM;
> > +
> > +       if (mmc_app_sd_status(card, raw_ssr)) {
> > 
> >                 pr_warn("%s: problem reading SD Status register\n",
> >                 
> >                         mmc_hostname(card->host));
> > 
> > +               kfree(raw_ssr);
> > 
> >                 return 0;
> >         
> >         }
> >         
> >         for (i = 0; i < 16; i++)
> > 
> > -               card->raw_ssr[i] = be32_to_cpu(card->raw_ssr[i]);
> > +               card->raw_ssr[i] = be32_to_cpu(raw_ssr[i]);
> > +
> > +       kfree(raw_ssr);
> > 
> >         /*
> >         
> >          * UNSTUFF_BITS only works with four u32s so we have to offset the
> > 
> > --
> > 2.10.2
> 
> Kind regards
> Uffe


[-- Attachment #2: This is a digitally signed message part. --]
[-- Type: application/pgp-signature, Size: 801 bytes --]

^ permalink raw reply

* Re: [PATCH 7/9] mmc: core: Allow CMD13 polling when switch to HS200 mode
From: Ulf Hansson @ 2016-11-17 15:02 UTC (permalink / raw)
  To: Adrian Hunter
  Cc: linux-mmc, Jaehoon Chung, Linus Walleij, Chaotian Jing,
	Stephen Boyd, Michael Walle, Yong Mao, Shawn Lin
In-Reply-To: <e003f72a-6084-eb1a-6b44-c6a11d93b3ae@intel.com>

On 17 November 2016 at 11:23, Adrian Hunter <adrian.hunter@intel.com> wrote:
> On 16/11/16 12:51, Ulf Hansson wrote:
>> In cases when the mmc host doesn't support HW busy detection, polling for
>> busy by using CMD13 is beneficial. The reasons have already been explained
>> in earlier change logs.
>>
>> To allow polling with CMD13, let's provide MMC_TIMING_MMC_HS200 as the
>> timing parameter to __mmc_switch(), which makes sure the mmc host and the
>> mmc card operates at the same bus timing during the polling.
>
> I have reports of cases where CMD13 always gives CRC errors after switch
> to HS200.  Currently we are assuming the low frequency should mean that
> won't happen, but it does in some cases.  That is not entirely surprising
> since HS200 needs tuning at the final operating frequency.

>From a logical point of view and if tuning is needed also for the CMD
line, this somehow make sense.

However, this is *not* how the JEDEC spec describes the HS200 switch
sequence. It is clearly stated that the host should validate the CM6
status via sending a CMD13 command, *before* performing tuning.

Could it be that the observations about the CRC errors, is related to
a controller/driver issue and not a card issue?

>
> What I would like to do for hosts that support busy waiting or DAT0 polling
> (i.e. MMC_CAP_WAIT_WHILE_BUSY or host->ops->card_busy), is to ignore CRC
> errors from the CMD13 that checks the switch status.  The main reason for
> doing that is that we really expect the switch to succeed and, given HS200
> tuning requirement, the CRC error is not a reliable means of determining
> that it hasn't.

Hmm. So what you are saying is that CMD13 polling for HS200 doesn't
work, as tuning is needed.

So, to me that means we need to fall-back to use the generic CMD6
timeout instead (when HW busy detection isn't supported).

>
> With the existing code I would just change the err check:
>
>
> diff --git a/drivers/mmc/core/mmc.c b/drivers/mmc/core/mmc.c
> index 3268fcd3378d..c8862c58b60b 100644
> --- a/drivers/mmc/core/mmc.c
> +++ b/drivers/mmc/core/mmc.c
> @@ -1387,6 +1387,13 @@ static int mmc_select_hs200(struct mmc_card *card)
>
>                 err = mmc_switch_status(card);
>                 /*
> +                * For HS200, CRC errors are not a reliable way to know the
> +                * switch failed. If there really is a problem, we would expect
> +                * tuning will fail and the result ends up the same.
> +                */
> +               if (err == -EILSEQ)
> +                       err = 0;
> +               /*

I don't think ignoring CRC errors is reliable when verifying the CMD6
status. My point is that we must not parse the status, in case of CRC
errors as it can't be trusted.

So, then we might as well just ignore validating the CMD6 status
altogether, but instead always move on to the tuning and hope that it
succeeds.

I think the CMD21 (tuning) should set the ILLEGAL COMMAND if HS200
mode isn't enabled, so we could check that. Anyway, we should fail
with the tuning if the earlier HS200 switch also failed. Don't you
think?

>                  * mmc_select_timing() assumes timing has not changed if
>                  * it is a switch error.
>                  */
>
>
> Then to support polling:
>
>
> diff --git a/drivers/mmc/core/mmc.c b/drivers/mmc/core/mmc.c
> index c8862c58b60b..66d8d57ae2fb 100644
> --- a/drivers/mmc/core/mmc.c
> +++ b/drivers/mmc/core/mmc.c
> @@ -1352,6 +1352,7 @@ static int mmc_select_hs200(struct mmc_card *card)
>  {
>         struct mmc_host *host = card->host;
>         unsigned int old_timing, old_signal_voltage;
> +       bool send_status;
>         int err = -EINVAL;
>         u8 val;
>
> @@ -1373,18 +1374,20 @@ static int mmc_select_hs200(struct mmc_card *card)
>          * switch to HS200 mode if bus width is set successfully.
>          */
>         err = mmc_select_bus_width(card);
> -       if (err > 0) {
> -               val = EXT_CSD_TIMING_HS200 |
> -                     card->drive_strength << EXT_CSD_DRV_STR_SHIFT;
> -               err = __mmc_switch(card, EXT_CSD_CMD_SET_NORMAL,
> -                                  EXT_CSD_HS_TIMING, val,
> -                                  card->ext_csd.generic_cmd6_time, 0,
> -                                  true, false, true);
> -               if (err)
> -                       goto err;
> -               old_timing = host->ios.timing;
> -               mmc_set_timing(host, MMC_TIMING_MMC_HS200);
> +       if (err <= 0)
> +               goto err;
> +
> +       send_status = !(host->caps & MMC_CAP_WAIT_WHILE_BUSY) &&
> +                     !host->ops->card_busy;
> +       old_timing = host->ios.timing;
> +
> +       val = EXT_CSD_TIMING_HS200 |
> +             card->drive_strength << EXT_CSD_DRV_STR_SHIFT;
> +       err = __mmc_switch(card, EXT_CSD_CMD_SET_NORMAL, EXT_CSD_HS_TIMING, val,
> +                          card->ext_csd.generic_cmd6_time,
> +                          MMC_TIMING_MMC_HS200, true, send_status, true);
>
> +       if (!err && !send_status) {
>                 err = mmc_switch_status(card);
>                 /*
>                  * For HS200, CRC errors are not a reliable way to know the
>
>
>
> Thoughts?

Well, I think the main problem is that if we have cards that returns
CRC errors even after the HS200 switch, then we can't use polling, as
we can't trust to parse the CMD6 status.

Kind regards
Uffe

^ permalink raw reply

* [PATCH 2/2] mmc: tegra: Add Tegra186 support
From: Thierry Reding @ 2016-11-17 17:18 UTC (permalink / raw)
  To: Adrian Hunter, Ulf Hansson
  Cc: Stephen Warren, Alexandre Courbot,
	linux-mmc-u79uwXL29TY76Z2rM5mHXA,
	linux-tegra-u79uwXL29TY76Z2rM5mHXA
In-Reply-To: <20161117171855.20843-1-thierry.reding-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>

From: Thierry Reding <treding-DDmLM1+adcrQT0dZR+AlfA@public.gmane.org>

Signed-off-by: Thierry Reding <treding-DDmLM1+adcrQT0dZR+AlfA@public.gmane.org>
---
 drivers/mmc/host/sdhci-tegra.c | 16 ++++++++++++++++
 1 file changed, 16 insertions(+)

diff --git a/drivers/mmc/host/sdhci-tegra.c b/drivers/mmc/host/sdhci-tegra.c
index eac57bb161d3..0e7143557fe4 100644
--- a/drivers/mmc/host/sdhci-tegra.c
+++ b/drivers/mmc/host/sdhci-tegra.c
@@ -434,7 +434,23 @@ static const struct sdhci_tegra_soc_data soc_data_tegra210 = {
 	.pdata = &sdhci_tegra210_pdata,
 };
 
+static const struct sdhci_pltfm_data sdhci_tegra186_pdata = {
+	.quirks = SDHCI_QUIRK_BROKEN_TIMEOUT_VAL |
+		  SDHCI_QUIRK_DATA_TIMEOUT_USES_SDCLK |
+		  SDHCI_QUIRK_SINGLE_POWER_WRITE |
+		  SDHCI_QUIRK_NO_HISPD_BIT |
+		  SDHCI_QUIRK_BROKEN_ADMA_ZEROLEN_DESC |
+		  SDHCI_QUIRK_CAP_CLOCK_BASE_BROKEN,
+	.quirks2 = SDHCI_QUIRK2_PRESET_VALUE_BROKEN,
+	.ops  = &tegra114_sdhci_ops,
+};
+
+static const struct sdhci_tegra_soc_data soc_data_tegra186 = {
+	.pdata = &sdhci_tegra186_pdata,
+};
+
 static const struct of_device_id sdhci_tegra_dt_match[] = {
+	{ .compatible = "nvidia,tegra186-sdhci", .data = &soc_data_tegra186 },
 	{ .compatible = "nvidia,tegra210-sdhci", .data = &soc_data_tegra210 },
 	{ .compatible = "nvidia,tegra124-sdhci", .data = &soc_data_tegra124 },
 	{ .compatible = "nvidia,tegra114-sdhci", .data = &soc_data_tegra114 },
-- 
2.10.2

^ permalink raw reply related

* [PATCH 1/2] mmc: tegra: Support module reset
From: Thierry Reding @ 2016-11-17 17:18 UTC (permalink / raw)
  To: Adrian Hunter, Ulf Hansson
  Cc: Stephen Warren, Alexandre Courbot,
	linux-mmc-u79uwXL29TY76Z2rM5mHXA,
	linux-tegra-u79uwXL29TY76Z2rM5mHXA

From: Thierry Reding <treding-DDmLM1+adcrQT0dZR+AlfA@public.gmane.org>

The device tree binding for the SDHCI controller found on Tegra SoCs
specifies that a reset control can be provided by the device tree. No
code was ever added to support the module reset, which can cause the
driver to try and access registers from a module that's in reset. On
most Tegra SoC generations doing so would cause a hang.

Note that it's unlikely to see this happen because on most platforms
these resets will have been deasserted by the bootloader. However the
portability can be improved by making sure the driver deasserts the
reset before accessing any registers.

Since resets are synchronous on Tegra SoCs, the platform driver needs
to implement a custom ->remove() callback now to make sure the clock
is disabled after the reset is asserted.

Signed-off-by: Thierry Reding <treding-DDmLM1+adcrQT0dZR+AlfA@public.gmane.org>
---
 drivers/mmc/host/sdhci-tegra.c | 38 +++++++++++++++++++++++++++++++++++++-
 1 file changed, 37 insertions(+), 1 deletion(-)

diff --git a/drivers/mmc/host/sdhci-tegra.c b/drivers/mmc/host/sdhci-tegra.c
index 20b6ff5b4af1..eac57bb161d3 100644
--- a/drivers/mmc/host/sdhci-tegra.c
+++ b/drivers/mmc/host/sdhci-tegra.c
@@ -21,6 +21,7 @@
 #include <linux/io.h>
 #include <linux/of.h>
 #include <linux/of_device.h>
+#include <linux/reset.h>
 #include <linux/mmc/card.h>
 #include <linux/mmc/host.h>
 #include <linux/mmc/mmc.h>
@@ -65,6 +66,8 @@ struct sdhci_tegra {
 	struct gpio_desc *power_gpio;
 	bool ddr_signaling;
 	bool pad_calib_required;
+
+	struct reset_control *rst;
 };
 
 static u16 tegra_sdhci_readw(struct sdhci_host *host, int reg)
@@ -489,6 +492,16 @@ static int sdhci_tegra_probe(struct platform_device *pdev)
 	clk_prepare_enable(clk);
 	pltfm_host->clk = clk;
 
+	tegra_host->rst = devm_reset_control_get(&pdev->dev, "sdhci");
+	if (IS_ERR(tegra_host->rst)) {
+		rc = PTR_ERR(tegra_host->rst);
+		dev_err(&pdev->dev, "failed to get reset control: %d\n", rc);
+		goto err_clk_get;
+	}
+
+	reset_control_deassert(tegra_host->rst);
+	usleep_range(2000, 4000);
+
 	rc = sdhci_add_host(host);
 	if (rc)
 		goto err_add_host;
@@ -504,6 +517,29 @@ static int sdhci_tegra_probe(struct platform_device *pdev)
 	return rc;
 }
 
+static int sdhci_tegra_remove(struct platform_device *pdev)
+{
+	struct sdhci_host *host = platform_get_drvdata(pdev);
+	struct sdhci_pltfm_host *platform = sdhci_priv(host);
+	struct sdhci_tegra *tegra = sdhci_pltfm_priv(platform);
+	int dead = 0;
+	u32 value;
+
+	value = readl(host->ioaddr + SDHCI_INT_STATUS);
+	if (value == 0xffffffff)
+		dead = 1;
+
+	sdhci_remove_host(host, dead);
+
+	reset_control_assert(tegra->rst);
+	usleep_range(2000, 4000);
+	clk_disable_unprepare(platform->clk);
+
+	sdhci_pltfm_free(pdev);
+
+	return 0;
+}
+
 static struct platform_driver sdhci_tegra_driver = {
 	.driver		= {
 		.name	= "sdhci-tegra",
@@ -511,7 +547,7 @@ static struct platform_driver sdhci_tegra_driver = {
 		.pm	= &sdhci_pltfm_pmops,
 	},
 	.probe		= sdhci_tegra_probe,
-	.remove		= sdhci_pltfm_unregister,
+	.remove		= sdhci_tegra_remove,
 };
 
 module_platform_driver(sdhci_tegra_driver);
-- 
2.10.2

^ permalink raw reply related

* Re: [PATCH] mmc: sd: Meet alignment requirements for raw_ssr DMA
From: Ulf Hansson @ 2016-11-17 12:51 UTC (permalink / raw)
  To: Paul Burton; +Cc: linux-mmc
In-Reply-To: <20161111142236.8270-1-paul.burton@imgtec.com>

On 11 November 2016 at 15:22, Paul Burton <paul.burton@imgtec.com> wrote:
> The mmc_read_ssr() function results in DMA to the raw_ssr member of
> struct mmc_card, which is not guaranteed to be cache line aligned & thus
> might not meet the requirements set out in Documentation/DMA-API.txt:
>
>   Warnings:  Memory coherency operates at a granularity called the cache
>   line width.  In order for memory mapped by this API to operate
>   correctly, the mapped region must begin exactly on a cache line
>   boundary and end exactly on one (to prevent two separately mapped
>   regions from sharing a single cache line).  Since the cache line size
>   may not be known at compile time, the API will not enforce this
>   requirement.  Therefore, it is recommended that driver writers who
>   don't take special care to determine the cache line size at run time
>   only map virtual regions that begin and end on page boundaries (which
>   are guaranteed also to be cache line boundaries).

This about virtual regions, not about physical contiguous allocated
memories, such as those you get via kmalloc(n, GFP_KERNEL). Right?

>
> On some systems where DMA is non-coherent this can lead to us losing
> data that shares cache lines with the raw_ssr array.
>
> Fix this by kmalloc'ing a temporary buffer to perform DMA into. kmalloc
> will ensure the buffer is suitably aligned, allowing the DMA to be
> performed without any loss of data.
>
> Signed-off-by: Paul Burton <paul.burton@imgtec.com>
> Cc: Ulf Hansson <ulf.hansson@linaro.org>
> Cc: linux-mmc@vger.kernel.org
> ---
>  drivers/mmc/core/sd.c | 12 ++++++++++--
>  1 file changed, 10 insertions(+), 2 deletions(-)
>
> diff --git a/drivers/mmc/core/sd.c b/drivers/mmc/core/sd.c
> index 73c762a..f6f40a1 100644
> --- a/drivers/mmc/core/sd.c
> +++ b/drivers/mmc/core/sd.c
> @@ -223,6 +223,7 @@ static int mmc_decode_scr(struct mmc_card *card)
>  static int mmc_read_ssr(struct mmc_card *card)
>  {
>         unsigned int au, es, et, eo;
> +       u32 *raw_ssr;
>         int i;
>
>         if (!(card->csd.cmdclass & CCC_APP_SPEC)) {
> @@ -231,14 +232,21 @@ static int mmc_read_ssr(struct mmc_card *card)
>                 return 0;
>         }
>
> -       if (mmc_app_sd_status(card, card->raw_ssr)) {

So the struct mmc_card, which contains "u32 raw_ssr[16]", is already
allocated via using a kzalloc().
As kzalloc() returns a physically contiguous allocated memory, using
DMA for reading the data into the memory pointed to by card->raw_ssr
should be safe.

Unless I am missing something, of course.

> +       raw_ssr = kmalloc(sizeof(card->raw_ssr), GFP_KERNEL);
> +       if (!raw_ssr)
> +               return -ENOMEM;
> +
> +       if (mmc_app_sd_status(card, raw_ssr)) {
>                 pr_warn("%s: problem reading SD Status register\n",
>                         mmc_hostname(card->host));
> +               kfree(raw_ssr);
>                 return 0;
>         }
>
>         for (i = 0; i < 16; i++)
> -               card->raw_ssr[i] = be32_to_cpu(card->raw_ssr[i]);
> +               card->raw_ssr[i] = be32_to_cpu(raw_ssr[i]);
> +
> +       kfree(raw_ssr);
>
>         /*
>          * UNSTUFF_BITS only works with four u32s so we have to offset the
> --
> 2.10.2
>

Kind regards
Uffe

^ permalink raw reply

* Re: [PATCH v2 -next] soc: fsl: fix section mismatch build warnings
From: Arnd Bergmann @ 2016-11-17 15:25 UTC (permalink / raw)
  To: Ulf Hansson; +Cc: Sudeep Holla, linux-kernel, linux-mmc, Scott Wood, Yangbo Lu
In-Reply-To: <1479395519-20742-1-git-send-email-sudeep.holla@arm.com>

On Thursday, November 17, 2016 3:11:59 PM CET Sudeep Holla wrote:
> 
> Cc: Scott Wood <oss@buserror.net>
> Cc: Yangbo Lu <yangbo.lu@nxp.com>
> Cc: Arnd Bergmann <arnd@arndb.de>
> Cc: Ulf Hansson <ulf.hansson@linaro.org>
> Signed-off-by: Sudeep Holla <sudeep.holla@arm.com>
> 

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

Ulf, can you pick this up into the mmc tree?

	Arnd

^ permalink raw reply

* [PATCH v2 -next] soc: fsl: fix section mismatch build warnings
From: Sudeep Holla @ 2016-11-17 15:11 UTC (permalink / raw)
  To: linux-kernel
  Cc: Sudeep Holla, linux-mmc, Scott Wood, Yangbo Lu, Arnd Bergmann,
	Ulf Hansson
In-Reply-To: <1479314367-9169-2-git-send-email-sudeep.holla@arm.com>

We get the following warning with the driver is compiled in:

WARNING: modpost: Found 1 section mismatch(es).
To see full details build your kernel with:
'make CONFIG_DEBUG_SECTION_MISMATCH=y'

With CONFIG_DEBUG_SECTION_MISMATCH enabled, the details are reported:

WARNING: vmlinux.o(.text+0x55d014): Section mismatch in reference from the
function fsl_guts_probe() to the function
.init.text:of_flat_dt_get_machine_name()
The function fsl_guts_probe() references
the function __init of_flat_dt_get_machine_name().
This is often because fsl_guts_probe lacks a __init
annotation or the annotation of of_flat_dt_get_machine_name is wrong.

This patch fixes the issue by using the normal DT/OF API rather than
the of_flat_* one.

Cc: Scott Wood <oss@buserror.net>
Cc: Yangbo Lu <yangbo.lu@nxp.com>
Cc: Arnd Bergmann <arnd@arndb.de>
Cc: Ulf Hansson <ulf.hansson@linaro.org>
Signed-off-by: Sudeep Holla <sudeep.holla@arm.com>
---
 drivers/soc/fsl/guts.c | 7 +++++--
 1 file changed, 5 insertions(+), 2 deletions(-)

v1->v2:
	- As suggested by Arnd, used standard DT APIs over of_flat_* API

diff --git a/drivers/soc/fsl/guts.c b/drivers/soc/fsl/guts.c
index 0ac88263c2d7..6af7a11f09a5 100644
--- a/drivers/soc/fsl/guts.c
+++ b/drivers/soc/fsl/guts.c
@@ -132,7 +132,7 @@ EXPORT_SYMBOL(fsl_guts_get_svr);

 static int fsl_guts_probe(struct platform_device *pdev)
 {
-	struct device_node *np = pdev->dev.of_node;
+	struct device_node *root, *np = pdev->dev.of_node;
 	struct device *dev = &pdev->dev;
 	struct resource *res;
 	const struct fsl_soc_die_attr *soc_die;
@@ -152,7 +152,10 @@ static int fsl_guts_probe(struct platform_device *pdev)
 		return PTR_ERR(guts->regs);

 	/* Register soc device */
-	machine = of_flat_dt_get_machine_name();
+	root = of_find_node_by_path("/");
+	if (of_property_read_string(root, "model", &machine))
+		of_property_read_string_index(root, "compatible", 0, &machine);
+	of_node_put(root);
 	if (machine)
 		soc_dev_attr.machine = devm_kstrdup(dev, machine, GFP_KERNEL);

--
2.7.4

^ permalink raw reply related

* Re: [RFC v2 2/2] mmc: sdhci-pci: Use ACPI to get max frequency for Intel byt sdio controller sub-vended by NI
From: Adrian Hunter @ 2016-11-17 12:26 UTC (permalink / raw)
  To: Zach Brown; +Cc: ulf.hansson, linux-mmc, linux-kernel
In-Reply-To: <1479227541-21201-3-git-send-email-zach.brown@ni.com>

On 15/11/16 18:32, Zach Brown wrote:
> On NI 9037 boards the max SDIO frequency is limited by trace lengths
> and other layout choices. The max SDIO frequency is stored in an ACPI
> table.
> 
> The driver reads the ACPI entry MXFQ during sdio_probe_slot and sets the
> f_max field of the host.
> 
> Signed-off-by: Nathan Sullivan <nathan.sullivan@ni.com>
> Reviewed-by: Jaeden Amero <jaeden.amero@ni.com>
> Reviewed-by: Josh Cartwright <joshc@ni.com>
> Signed-off-by: Zach Brown <zach.brown@ni.com>
> ---
>  drivers/mmc/host/sdhci-pci-core.c | 13 +++++++++++++
>  1 file changed, 13 insertions(+)
> 
> diff --git a/drivers/mmc/host/sdhci-pci-core.c b/drivers/mmc/host/sdhci-pci-core.c
> index 9741505..4c31d16 100644
> --- a/drivers/mmc/host/sdhci-pci-core.c
> +++ b/drivers/mmc/host/sdhci-pci-core.c
> @@ -27,6 +27,7 @@
>  #include <linux/pm_runtime.h>
>  #include <linux/mmc/slot-gpio.h>
>  #include <linux/mmc/sdhci-pci-data.h>
> +#include <linux/acpi.h>
>  
>  #include "sdhci.h"
>  #include "sdhci-pci.h"
> @@ -377,6 +378,18 @@ static int byt_emmc_probe_slot(struct sdhci_pci_slot *slot)
>  
>  static int ni_byt_sdio_probe_slot(struct sdhci_pci_slot *slot)
>  {
> +#ifdef CONFIG_ACPI
> +	/* Get max freq from ACPI for NI hardware */

The comment seems a little redundant.

> +	acpi_status status;
> +	unsigned long long max_freq;
> +
> +	status = acpi_evaluate_integer(ACPI_HANDLE(&slot->chip->pdev->dev),
> +				       "MXFQ", NULL, &max_freq);
> +	if (ACPI_FAILURE(status))
> +		return -EINVAL;

I wonder if you should print an error message to let the user know why the
driver failed to probe.

> +
> +	slot->host->mmc->f_max = max_freq * 1000000;
> +#endif
>  	slot->host->mmc->caps |= MMC_CAP_POWER_OFF_CARD | MMC_CAP_NONREMOVABLE |
>  				 MMC_CAP_WAIT_WHILE_BUSY;
>  	return 0;
> 

^ permalink raw reply

* [PATCH v2] mmc: Hynix: add QUIRK_NOTIFY_POWEROFF_ON_SLEEP
From: Thierry Escande @ 2016-11-17 10:59 UTC (permalink / raw)
  To: Ulf Hansson; +Cc: linux-mmc, bhthompson

From: zhaojohn <john.zhao@intel.com>

Hynix eMMC devices sometimes take 50% longer to resume from sleep. This
occurs on Braswell based chromebook with acpi sdhci controller.

Based on a recommendation from Hynix engineers, this patch sends a
Power-Off Notification using mmc_poweroff_notify() before going to S3 to
have a resume time consistently within spec. No voltage regulator are
being cut through this notification but merely tells the eMMC firmware
that we're about to do so and get it to behave better on resume.

Signed-off-by: zhaojohn <john.zhao@intel.com>
Signed-off-by: Arindam Roy <arindam.roy@intel.com>
Tested-by: Freddy Paul <freddy.paul@intel.com>
Reviewed-by: Icarus W Sparry <icarus.w.sparry@intel.com>
Reviewed-by: Marc Herbert <marc.herbert@intel.com>
Reviewed-by: Eric Caruso <ejcaruso@chromium.org>
Signed-off-by: Brian Norris <briannorris@chromium.org>
Signed-off-by: Thierry Escande <thierry.escande@collabora.com>
---

Changes in v2:
- Add a few more details in the commit message

 drivers/mmc/card/block.c | 8 ++++++++
 drivers/mmc/core/mmc.c   | 8 +++++++-
 include/linux/mmc/card.h | 2 ++
 3 files changed, 17 insertions(+), 1 deletion(-)

diff --git a/drivers/mmc/card/block.c b/drivers/mmc/card/block.c
index 709a872..3db5344 100644
--- a/drivers/mmc/card/block.c
+++ b/drivers/mmc/card/block.c
@@ -2573,6 +2573,14 @@ static const struct mmc_fixup blk_fixups[] =
 	MMC_FIXUP("V10016", CID_MANFID_KINGSTON, CID_OEMID_ANY, add_quirk_mmc,
 		  MMC_QUIRK_TRIM_BROKEN),
 
+	/*
+	 * Hynix eMMC devices sometimes take 50% longer to resume from sleep.
+	 * Based on a recommendation from Hynix, send a Power-Off Notification
+	 * before going to S3 to restore a resume time consistently within spec.
+	 */
+	MMC_FIXUP(CID_NAME_ANY, CID_MANFID_HYNIX, CID_OEMID_ANY, add_quirk_mmc,
+		  MMC_QUIRK_NOTIFY_POWEROFF_ON_SLEEP),
+
 	END_FIXUP
 };
 
diff --git a/drivers/mmc/core/mmc.c b/drivers/mmc/core/mmc.c
index df19777..774d478 100644
--- a/drivers/mmc/core/mmc.c
+++ b/drivers/mmc/core/mmc.c
@@ -1941,8 +1941,14 @@ static int _mmc_suspend(struct mmc_host *host, bool is_suspend)
 	if (mmc_can_poweroff_notify(host->card) &&
 		((host->caps2 & MMC_CAP2_FULL_PWR_CYCLE) || !is_suspend))
 		err = mmc_poweroff_notify(host->card, notify_type);
-	else if (mmc_can_sleep(host->card))
+	else if (mmc_can_sleep(host->card)) {
+		if (host->card->quirks & MMC_QUIRK_NOTIFY_POWEROFF_ON_SLEEP) {
+			err = mmc_poweroff_notify(host->card, notify_type);
+			if (err)
+				goto out;
+		}
 		err = mmc_sleep(host);
+	}
 	else if (!mmc_host_is_spi(host))
 		err = mmc_deselect_cards(host);
 
diff --git a/include/linux/mmc/card.h b/include/linux/mmc/card.h
index 73fad83..e4940f4 100644
--- a/include/linux/mmc/card.h
+++ b/include/linux/mmc/card.h
@@ -281,6 +281,8 @@ struct mmc_card {
 #define MMC_QUIRK_BROKEN_IRQ_POLLING	(1<<11)	/* Polling SDIO_CCCR_INTx could create a fake interrupt */
 #define MMC_QUIRK_TRIM_BROKEN	(1<<12)		/* Skip trim */
 #define MMC_QUIRK_BROKEN_HPI	(1<<13)		/* Disable broken HPI support */
+#define MMC_QUIRK_NOTIFY_POWEROFF_ON_SLEEP \
+				(1<<14)		/* Poweroff notification*/
 
 
 	unsigned int		erase_size;	/* erase size in sectors */
-- 
2.7.4


^ permalink raw reply related

* Re: [PATCH 7/9] mmc: core: Allow CMD13 polling when switch to HS200 mode
From: Adrian Hunter @ 2016-11-17 10:23 UTC (permalink / raw)
  To: Ulf Hansson, linux-mmc
  Cc: Jaehoon Chung, Linus Walleij, Chaotian Jing, Stephen Boyd,
	Michael Walle, Yong Mao, Shawn Lin
In-Reply-To: <1479293481-20186-8-git-send-email-ulf.hansson@linaro.org>

On 16/11/16 12:51, Ulf Hansson wrote:
> In cases when the mmc host doesn't support HW busy detection, polling for
> busy by using CMD13 is beneficial. The reasons have already been explained
> in earlier change logs.
> 
> To allow polling with CMD13, let's provide MMC_TIMING_MMC_HS200 as the
> timing parameter to __mmc_switch(), which makes sure the mmc host and the
> mmc card operates at the same bus timing during the polling.

I have reports of cases where CMD13 always gives CRC errors after switch
to HS200.  Currently we are assuming the low frequency should mean that
won't happen, but it does in some cases.  That is not entirely surprising
since HS200 needs tuning at the final operating frequency.

What I would like to do for hosts that support busy waiting or DAT0 polling
(i.e. MMC_CAP_WAIT_WHILE_BUSY or host->ops->card_busy), is to ignore CRC
errors from the CMD13 that checks the switch status.  The main reason for
doing that is that we really expect the switch to succeed and, given HS200
tuning requirement, the CRC error is not a reliable means of determining
that it hasn't.

With the existing code I would just change the err check:


diff --git a/drivers/mmc/core/mmc.c b/drivers/mmc/core/mmc.c
index 3268fcd3378d..c8862c58b60b 100644
--- a/drivers/mmc/core/mmc.c
+++ b/drivers/mmc/core/mmc.c
@@ -1387,6 +1387,13 @@ static int mmc_select_hs200(struct mmc_card *card)
 
 		err = mmc_switch_status(card);
 		/*
+		 * For HS200, CRC errors are not a reliable way to know the
+		 * switch failed. If there really is a problem, we would expect
+		 * tuning will fail and the result ends up the same.
+		 */
+		if (err == -EILSEQ)
+			err = 0;
+		/*
 		 * mmc_select_timing() assumes timing has not changed if
 		 * it is a switch error.
 		 */


Then to support polling:


diff --git a/drivers/mmc/core/mmc.c b/drivers/mmc/core/mmc.c
index c8862c58b60b..66d8d57ae2fb 100644
--- a/drivers/mmc/core/mmc.c
+++ b/drivers/mmc/core/mmc.c
@@ -1352,6 +1352,7 @@ static int mmc_select_hs200(struct mmc_card *card)
 {
 	struct mmc_host *host = card->host;
 	unsigned int old_timing, old_signal_voltage;
+	bool send_status;
 	int err = -EINVAL;
 	u8 val;
 
@@ -1373,18 +1374,20 @@ static int mmc_select_hs200(struct mmc_card *card)
 	 * switch to HS200 mode if bus width is set successfully.
 	 */
 	err = mmc_select_bus_width(card);
-	if (err > 0) {
-		val = EXT_CSD_TIMING_HS200 |
-		      card->drive_strength << EXT_CSD_DRV_STR_SHIFT;
-		err = __mmc_switch(card, EXT_CSD_CMD_SET_NORMAL,
-				   EXT_CSD_HS_TIMING, val,
-				   card->ext_csd.generic_cmd6_time, 0,
-				   true, false, true);
-		if (err)
-			goto err;
-		old_timing = host->ios.timing;
-		mmc_set_timing(host, MMC_TIMING_MMC_HS200);
+	if (err <= 0)
+		goto err;
+
+	send_status = !(host->caps & MMC_CAP_WAIT_WHILE_BUSY) &&
+		      !host->ops->card_busy;
+	old_timing = host->ios.timing;
+
+	val = EXT_CSD_TIMING_HS200 |
+	      card->drive_strength << EXT_CSD_DRV_STR_SHIFT;
+	err = __mmc_switch(card, EXT_CSD_CMD_SET_NORMAL, EXT_CSD_HS_TIMING, val,
+			   card->ext_csd.generic_cmd6_time,
+			   MMC_TIMING_MMC_HS200, true, send_status, true);
 
+	if (!err && !send_status) {
 		err = mmc_switch_status(card);
 		/*
 		 * For HS200, CRC errors are not a reliable way to know the



Thoughts?


^ permalink raw reply related

* Re: [PATCH 0/9] mmc: core: Re-work CMD13 polling method for CMD6 for mmc
From: Linus Walleij @ 2016-11-17  9:06 UTC (permalink / raw)
  To: Ulf Hansson
  Cc: linux-mmc@vger.kernel.org, Jaehoon Chung, Adrian Hunter,
	Chaotian Jing, Stephen Boyd, Michael Walle, Yong Mao, Shawn Lin
In-Reply-To: <1479293481-20186-1-git-send-email-ulf.hansson@linaro.org>

On Wed, Nov 16, 2016 at 11:51 AM, Ulf Hansson <ulf.hansson@linaro.org> wrote:

> To improve this behaviour and shorten the card initialization time, we need to
> allow using CMD13 as polling method to find out when the card stops signaling
> busy. Although, enabling CMD13 polling may also introduce other issues as
> according to the JEDEC spec, it's not the recommended method. Especially it
> mentions that CRC errors may be triggered when sending a CMD13 command during a
> bus timing change.
>
> To deal with these issues, we need to change from ignoring those CRC errors but
> instead continue to treat the card as being busy and continue to poll with
> CMD13. Perhaps this behaviour is one of reasons to why the earlier CMD13 polling
> method didn't work out well.
>
> This series requires extensive testing, please help with that!

I cloned the MMC tree "next" and applied the patches on top.

(I guess if you want wider testing it's good to publish a branch,
but I'd recommend just putting it into linux-next, it seems solid
to me.)

Tested on my previously problematic APQ8060 Dragonboard, it switches
to highspeed and works like a charm! Now the eMMC comes up smacky
right after registering the host:

[    1.955105] mmci-pl18x 12400000.sdcc: mmc0: PL180 manf 51 rev0 at
0x12400000 irq 197,0 (pio)
[    1.955149] mmci-pl18x 12400000.sdcc: DMA channels RX none, TX none
[    1.964202] mmci-pl18x 12400000.sdcc: Voltage switch failed
[    2.031744] mmci-pl18x 12180000.sdcc: Got CD GPIO
[    2.031829] mmci-pl18x 12180000.sdcc: Got WP GPIO
[    2.040263] mmci-pl18x 12180000.sdcc: DMA channels RX none, TX none
[    2.094161] mmc0: new high speed MMC card at address 0001
[    2.095590] mmcblk0: mmc0:0001 SEM04G 3.69 GiB
[    2.120905] mmcblk0boot0: mmc0:0001 SEM04G partition 1 1.00 MiB
[    2.133068] mmcblk0boot1: mmc0:0001 SEM04G partition 2 1.00 MiB
[    2.139657] mmcblk0rpmb: mmc0:0001 SEM04G partition 3 128 KiB
[    2.158004]  mmcblk0: p1 p2 p3 p4 < p5 p6 p7 p8 p9 p10 p11 p12 p13
p14 p15 p16 p17 p18 p19 p20 p21 >

Tested-by: Linus Walleij <linus.walleij@linaro.org>

Yours,
Linus Walleij

^ permalink raw reply

* Re: [PATCH] mmc: dw_mmc: fix the error handling for dma operation
From: Jaehoon Chung @ 2016-11-17  8:25 UTC (permalink / raw)
  To: Shawn Lin, linux-mmc; +Cc: ulf.hansson
In-Reply-To: <75493dd3-f3c2-e7d7-0d21-01b430978cb6@rock-chips.com>

On 11/17/2016 05:08 PM, Shawn Lin wrote:
> Hi Jaehoon,
> 
> 在 2016/11/17 15:53, Jaehoon Chung 写道:
>> When OWN bit of dma descriptor is not cleared, then it returns -EINVAL.
>> Then it has to fall back to PIO mode for current transfer.
>>
>> Host controller was already set to bits relevant to DMA operation.
>> If needs to use the PIO mode, Host controller has to stop the DMA
>> operation. (It's more stable than now.)
>>
> 
> It looks good to me, but
> 
>> When it occurred error, it's not running any request.
>>
>> Fixes: 3b2a067b98b4 ("mmc: dw_mmc: avoid race condition of cpu and IDMAC")
>>
> 
> I think the real fixes tag should indicate the another commit,
> 3fc7eaef44dbcbcd60 ("mmc: dw_mmc: Add external dma interface support")

You're right. 
But my main problem is "checking OWN bit".  Thanks for noticing this commit. :)
Some time..OWN bit is never released.. 

Best Regards,
Jaehoon Chung

> 
> otherwise,
> 
> Reviewed-by: Shawn Lin <shawn.lin@rock-chips.com>
> 
>> Signed-off-by: Jaehoon Chung <jh80.chung@samsung.com>
>> ---
>>  drivers/mmc/host/dw_mmc.c | 1 +
>>  1 file changed, 1 insertion(+)
>>
>> diff --git a/drivers/mmc/host/dw_mmc.c b/drivers/mmc/host/dw_mmc.c
>> index 9341b18..080003b 100644
>> --- a/drivers/mmc/host/dw_mmc.c
>> +++ b/drivers/mmc/host/dw_mmc.c
>> @@ -1058,6 +1058,7 @@ static int dw_mci_submit_data_dma(struct dw_mci *host, struct mmc_data *data)
>>      spin_unlock_irqrestore(&host->irq_lock, irqflags);
>>
>>      if (host->dma_ops->start(host, sg_len)) {
>> +        host->dma_ops->stop(host);
>>          /* We can't do DMA, try PIO for this one */
>>          dev_dbg(host->dev,
>>              "%s: fall back to PIO mode for current transfer\n",
>>
> 
> 


^ permalink raw reply

* Re: [PATCH] mmc: dw_mmc: fix the error handling for dma operation
From: Shawn Lin @ 2016-11-17  8:08 UTC (permalink / raw)
  To: Jaehoon Chung, linux-mmc; +Cc: shawn.lin, ulf.hansson
In-Reply-To: <20161117075347.9061-1-jh80.chung@samsung.com>

Hi Jaehoon,

在 2016/11/17 15:53, Jaehoon Chung 写道:
> When OWN bit of dma descriptor is not cleared, then it returns -EINVAL.
> Then it has to fall back to PIO mode for current transfer.
>
> Host controller was already set to bits relevant to DMA operation.
> If needs to use the PIO mode, Host controller has to stop the DMA
> operation. (It's more stable than now.)
>

It looks good to me, but

> When it occurred error, it's not running any request.
>
> Fixes: 3b2a067b98b4 ("mmc: dw_mmc: avoid race condition of cpu and IDMAC")
>

I think the real fixes tag should indicate the another commit,
3fc7eaef44dbcbcd60 ("mmc: dw_mmc: Add external dma interface support")

otherwise,

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

> Signed-off-by: Jaehoon Chung <jh80.chung@samsung.com>
> ---
>  drivers/mmc/host/dw_mmc.c | 1 +
>  1 file changed, 1 insertion(+)
>
> diff --git a/drivers/mmc/host/dw_mmc.c b/drivers/mmc/host/dw_mmc.c
> index 9341b18..080003b 100644
> --- a/drivers/mmc/host/dw_mmc.c
> +++ b/drivers/mmc/host/dw_mmc.c
> @@ -1058,6 +1058,7 @@ static int dw_mci_submit_data_dma(struct dw_mci *host, struct mmc_data *data)
>  	spin_unlock_irqrestore(&host->irq_lock, irqflags);
>
>  	if (host->dma_ops->start(host, sg_len)) {
> +		host->dma_ops->stop(host);
>  		/* We can't do DMA, try PIO for this one */
>  		dev_dbg(host->dev,
>  			"%s: fall back to PIO mode for current transfer\n",
>


-- 
Best Regards
Shawn Lin


^ permalink raw reply


This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox