[PATCH v2 00/16] mmc: sdhci-omap: Add UHS/HS200 mode support

[PATCH v2 00/16] mmc: sdhci-omap: Add UHS/HS200 mode support

[Kishon Vijay Abraham I]

Add UHS/HS200 mode support in sdhci-omap. The programming sequence
for voltage switching, tuning is followed from AM572x TRM
http://www.ti.com/lit/ug/spruhz6i/spruhz6i.pdf
(Similar to all AM57x/DRA7x SoCs). The patch series also implements
workaround for errata published in
http://www.ti.com/lit/er/sprz429k/sprz429k.pdf.

It also includes a pdata-quirk patch since both pdata-quirks and
sdhci-omap uses struct sdhci_omap_platform_data.

The dt patches enabling UHS/HS200 will follow this series.

Changes from v1:
*) Only poll on DAT0 and DATI for card_busy status
*) Cleanup iodelay patch as suggested by Tony.
*) Added quirk to disable HW timeout
*) Use the existing data timer but program a relatively accurate
   SW timeout value (Impacts all platforms)
*) Fix a bug in sdhci which was using data_timer for non data line
   commands

I've tested on dra72-evm, dra7xx-evm, dra71-evm, am572x-idk, am571x-idk
dra76-evm, am57xx-evm and k2g-evm.

I've also pushed the series (along with dt) to
https://github.com/kishon/linux-wip.git sdhci_uhs_v2

Kishon Vijay Abraham I (16):
  mmc: sdhci-omap: Update 'power_mode' outside sdhci_omap_init_74_clocks
  mmc: sdhci-omap: Add card_busy host ops
  mmc: sdhci-omap: Add custom set_uhs_signaling sdhci_host ops
  mmc: sdhci-omap: Add tuning support
  mmc: sdhci-omap: Workaround for Errata i802
  mmc: sdhci_omap: Add support to set IODELAY values
  mmc: sdhci_omap: Fix sdhci-omap quirks
  mmc: sdhci-omap: Add support to override f_max and iodelay from pdata
  mmc: sdhci: Add quirk to disable HW timeout
  mmc: sdhci: Fix to use data_timer only for data line commands
  mmc: sdhci: Program a relatively accurate SW timeout value
  mmc: sdhci-omap: Workaround for Errata i834
  dt-bindings: sdhci-omap: Add K2G specific binding
  mmc: sdhci-omap: Add support for MMC/SD controller in k2g SoC
  mmc: sdhci-omap: Add SPDX identifier
  ARM: OMAP2+: Use sdhci-omap specific pdata-quirks for MMC/SD on DRA74x
    EVM

 .../devicetree/bindings/mmc/sdhci-omap.txt         |   2 +
 arch/arm/mach-omap2/pdata-quirks.c                 |  34 +-
 drivers/mmc/host/sdhci-omap.c                      | 430 +++++++++++++++++++--
 drivers/mmc/host/sdhci.c                           |  62 ++-
 drivers/mmc/host/sdhci.h                           |  17 +
 include/linux/platform_data/sdhci-omap.h           |  22 ++
 6 files changed, 518 insertions(+), 49 deletions(-)
 create mode 100644 include/linux/platform_data/sdhci-omap.h

-- 
2.11.0

[PATCH v2 01/16] mmc: sdhci-omap: Update 'power_mode' outside sdhci_omap_init_74_clocks

[Kishon Vijay Abraham I]

Updating 'power_mode' in sdhci_omap_init_74_clocks results in
'power_mode' never updated to MMC_POWER_OFF during card
removal. This results in initialization sequence not sent to the
card during re-insertion.
Fix it here by adding sdhci_omap_set_power_mode to update power_mode.
This function can also be used later to perform operations that
are specific to a power mode (e.g, disable tuning during power off).

Signed-off-by: Kishon Vijay Abraham I <kishon@ti.com>
Acked-by: Adrian Hunter <adrian.hunter@intel.com>
---
 drivers/mmc/host/sdhci-omap.c | 10 ++++++++--
 1 file changed, 8 insertions(+), 2 deletions(-)

diff --git a/drivers/mmc/host/sdhci-omap.c b/drivers/mmc/host/sdhci-omap.c
index 628bfe9a3d17..96985786cadf 100644
--- a/drivers/mmc/host/sdhci-omap.c
+++ b/drivers/mmc/host/sdhci-omap.c
@@ -244,6 +244,12 @@ static int sdhci_omap_start_signal_voltage_switch(struct mmc_host *mmc,
 	return 0;
 }
 
+static void sdhci_omap_set_power_mode(struct sdhci_omap_host *omap_host,
+				      u8 power_mode)
+{
+	omap_host->power_mode = power_mode;
+}
+
 static void sdhci_omap_set_bus_mode(struct sdhci_omap_host *omap_host,
 				    unsigned int mode)
 {
@@ -273,6 +279,7 @@ static void sdhci_omap_set_ios(struct mmc_host *mmc, struct mmc_ios *ios)
 
 	sdhci_omap_set_bus_mode(omap_host, ios->bus_mode);
 	sdhci_set_ios(mmc, ios);
+	sdhci_omap_set_power_mode(omap_host, ios->power_mode);
 }
 
 static u16 sdhci_omap_calc_divisor(struct sdhci_pltfm_host *host,
@@ -401,8 +408,6 @@ static void sdhci_omap_init_74_clocks(struct sdhci_host *host, u8 power_mode)
 	sdhci_omap_writel(omap_host, SDHCI_OMAP_STAT, INT_CC_EN);
 
 	enable_irq(host->irq);
-
-	omap_host->power_mode = power_mode;
 }
 
 static struct sdhci_ops sdhci_omap_ops = {
@@ -504,6 +509,7 @@ static int sdhci_omap_probe(struct platform_device *pdev)
 	omap_host->host = host;
 	omap_host->base = host->ioaddr;
 	omap_host->dev = dev;
+	omap_host->power_mode = MMC_POWER_UNDEFINED;
 	host->ioaddr += offset;
 
 	mmc = host->mmc;
-- 
2.11.0

[PATCH v2 02/16] mmc: sdhci-omap: Add card_busy host ops

[Kishon Vijay Abraham I]

Add card_busy host ops in sdhci_omap to check card busy status.

The voltage switching sequence for AM572x platform is mentioned
in Figure 25-48. eMMC/SD/SDIO Power Switching Procedure of
AM572x Sitara Processors Silicon Revision 2.0, 1.1 TRM
(SPRUHZ6I - October 2014–Revised April 2017 [1]).

In the voltage switching sequence, CLKEXTFREE bit in MMCHS_CON
should also be set after switching to 1.8v which is also taken
care in the card_busy ops.

[1] -> http://www.ti.com/lit/ug/spruhz6i/spruhz6i.pdf

Signed-off-by: Kishon Vijay Abraham I <kishon@ti.com>
---
 drivers/mmc/host/sdhci-omap.c | 52 +++++++++++++++++++++++++++++++++++++++++++
 1 file changed, 52 insertions(+)

diff --git a/drivers/mmc/host/sdhci-omap.c b/drivers/mmc/host/sdhci-omap.c
index 96985786cadf..df927f3faaf6 100644
--- a/drivers/mmc/host/sdhci-omap.c
+++ b/drivers/mmc/host/sdhci-omap.c
@@ -31,11 +31,17 @@
 #define SDHCI_OMAP_CON		0x12c
 #define CON_DW8			BIT(5)
 #define CON_DMA_MASTER		BIT(20)
+#define CON_CLKEXTFREE		BIT(16)
+#define CON_PADEN		BIT(15)
 #define CON_INIT		BIT(1)
 #define CON_OD			BIT(0)
 
 #define SDHCI_OMAP_CMD		0x20c
 
+#define SDHCI_OMAP_PSTATE	0x0224
+#define PSTATE_DLEV_DAT0	BIT(20)
+#define PSTATE_DATI		BIT(1)
+
 #define SDHCI_OMAP_HCTL		0x228
 #define HCTL_SDBP		BIT(8)
 #define HCTL_SDVS_SHIFT		9
@@ -191,6 +197,51 @@ static void sdhci_omap_conf_bus_power(struct sdhci_omap_host *omap_host,
 	}
 }
 
+static int sdhci_omap_card_busy(struct mmc_host *mmc)
+{
+	u32 reg, ac12;
+	int ret = false;
+	struct sdhci_host *host = mmc_priv(mmc);
+	struct sdhci_pltfm_host *pltfm_host;
+	struct sdhci_omap_host *omap_host;
+	u32 ier = host->ier;
+
+	pltfm_host = sdhci_priv(host);
+	omap_host = sdhci_pltfm_priv(pltfm_host);
+
+	reg = sdhci_omap_readl(omap_host, SDHCI_OMAP_CON);
+	ac12 = sdhci_omap_readl(omap_host, SDHCI_OMAP_AC12);
+	reg &= ~CON_CLKEXTFREE;
+	if (ac12 & AC12_V1V8_SIGEN)
+		reg |= CON_CLKEXTFREE;
+	reg |= CON_PADEN;
+	sdhci_omap_writel(omap_host, SDHCI_OMAP_CON, reg);
+
+	disable_irq(host->irq);
+	ier |= SDHCI_INT_CARD_INT;
+	sdhci_writel(host, ier, SDHCI_INT_ENABLE);
+	sdhci_writel(host, ier, SDHCI_SIGNAL_ENABLE);
+
+	/*
+	 * Delay is required for PSTATE to correctly reflect
+	 * DLEV/CLEV values after PADEN is set.
+	 */
+	usleep_range(50, 100);
+	reg = sdhci_omap_readl(omap_host, SDHCI_OMAP_PSTATE);
+	if ((reg & PSTATE_DATI) || !(reg & PSTATE_DLEV_DAT0))
+		ret = true;
+
+	reg = sdhci_omap_readl(omap_host, SDHCI_OMAP_CON);
+	reg &= ~(CON_CLKEXTFREE | CON_PADEN);
+	sdhci_omap_writel(omap_host, SDHCI_OMAP_CON, reg);
+
+	sdhci_writel(host, host->ier, SDHCI_INT_ENABLE);
+	sdhci_writel(host, host->ier, SDHCI_SIGNAL_ENABLE);
+	enable_irq(host->irq);
+
+	return ret;
+}
+
 static int sdhci_omap_start_signal_voltage_switch(struct mmc_host *mmc,
 						  struct mmc_ios *ios)
 {
@@ -562,6 +613,7 @@ static int sdhci_omap_probe(struct platform_device *pdev)
 	host->mmc_host_ops.start_signal_voltage_switch =
 					sdhci_omap_start_signal_voltage_switch;
 	host->mmc_host_ops.set_ios = sdhci_omap_set_ios;
+	host->mmc_host_ops.card_busy = sdhci_omap_card_busy;
 
 	sdhci_read_caps(host);
 	host->caps |= SDHCI_CAN_DO_ADMA2;
-- 
2.11.0

[PATCH v2 03/16] mmc: sdhci-omap: Add custom set_uhs_signaling sdhci_host ops

[Kishon Vijay Abraham I]

UHS-1 DDR50 and MMC DDR52 mode require DDR bit to be
set in the configuration register (MMCHS_CON). Add
sdhci-omap specific set_uhs_signaling ops to set
this bit. Also while setting the UHSMS bit, clock should be
disabled.

Signed-off-by: Kishon Vijay Abraham I <kishon@ti.com>
Acked-by: Adrian Hunter <adrian.hunter@intel.com>
---
 drivers/mmc/host/sdhci-omap.c | 23 ++++++++++++++++++++++-
 1 file changed, 22 insertions(+), 1 deletion(-)

diff --git a/drivers/mmc/host/sdhci-omap.c b/drivers/mmc/host/sdhci-omap.c
index df927f3faaf6..86b6cc0a5380 100644
--- a/drivers/mmc/host/sdhci-omap.c
+++ b/drivers/mmc/host/sdhci-omap.c
@@ -31,6 +31,7 @@
 #define SDHCI_OMAP_CON		0x12c
 #define CON_DW8			BIT(5)
 #define CON_DMA_MASTER		BIT(20)
+#define CON_DDR			BIT(19)
 #define CON_CLKEXTFREE		BIT(16)
 #define CON_PADEN		BIT(15)
 #define CON_INIT		BIT(1)
@@ -461,6 +462,26 @@ static void sdhci_omap_init_74_clocks(struct sdhci_host *host, u8 power_mode)
 	enable_irq(host->irq);
 }
 
+static void sdhci_omap_set_uhs_signaling(struct sdhci_host *host,
+					 unsigned int timing)
+{
+	u32 reg;
+	struct sdhci_pltfm_host *pltfm_host = sdhci_priv(host);
+	struct sdhci_omap_host *omap_host = sdhci_pltfm_priv(pltfm_host);
+
+	sdhci_omap_stop_clock(omap_host);
+
+	reg = sdhci_omap_readl(omap_host, SDHCI_OMAP_CON);
+	if (timing == MMC_TIMING_UHS_DDR50 || timing == MMC_TIMING_MMC_DDR52)
+		reg |= CON_DDR;
+	else
+		reg &= ~CON_DDR;
+	sdhci_omap_writel(omap_host, SDHCI_OMAP_CON, reg);
+
+	sdhci_set_uhs_signaling(host, timing);
+	sdhci_omap_start_clock(omap_host);
+}
+
 static struct sdhci_ops sdhci_omap_ops = {
 	.set_clock = sdhci_omap_set_clock,
 	.set_power = sdhci_omap_set_power,
@@ -470,7 +491,7 @@ static struct sdhci_ops sdhci_omap_ops = {
 	.set_bus_width = sdhci_omap_set_bus_width,
 	.platform_send_init_74_clocks = sdhci_omap_init_74_clocks,
 	.reset = sdhci_reset,
-	.set_uhs_signaling = sdhci_set_uhs_signaling,
+	.set_uhs_signaling = sdhci_omap_set_uhs_signaling,
 };
 
 static int sdhci_omap_set_capabilities(struct sdhci_omap_host *omap_host)
-- 
2.11.0

[PATCH v2 04/16] mmc: sdhci-omap: Add tuning support

[Kishon Vijay Abraham I]

MMC tuning procedure is required to support SD card
UHS1-SDR104 mode and EMMC HS200 mode.

SDR104/HS200 DLL Tuning Procedure for AM572x platform is mentioned
in Figure 25-51. SDR104/HS200 DLL Tuning Procedure of
AM572x Sitara Processors Silicon Revision 2.0, 1.1 TRM
(SPRUHZ6I - October 2014–Revised April 2017 [1]).

The tuning function sdhci_omap_execute_tuning() will only be
called by the MMC/SD core if the corresponding speed modes
are supported by the OMAP silicon which is set in the mmc
host "caps" field.

[1] -> http://www.ti.com/lit/ug/spruhz6i/spruhz6i.pdf

Signed-off-by: Kishon Vijay Abraham I <kishon@ti.com>
Acked-by: Adrian Hunter <adrian.hunter@intel.com>
---
 drivers/mmc/host/sdhci-omap.c | 130 ++++++++++++++++++++++++++++++++++++++++++
 1 file changed, 130 insertions(+)

diff --git a/drivers/mmc/host/sdhci-omap.c b/drivers/mmc/host/sdhci-omap.c
index 86b6cc0a5380..36e0626d3de2 100644
--- a/drivers/mmc/host/sdhci-omap.c
+++ b/drivers/mmc/host/sdhci-omap.c
@@ -37,6 +37,13 @@
 #define CON_INIT		BIT(1)
 #define CON_OD			BIT(0)
 
+#define SDHCI_OMAP_DLL		0x0134
+#define DLL_SWT			BIT(20)
+#define DLL_FORCE_SR_C_SHIFT	13
+#define DLL_FORCE_SR_C_MASK	(0x7f << DLL_FORCE_SR_C_SHIFT)
+#define DLL_FORCE_VALUE		BIT(12)
+#define DLL_CALIB		BIT(1)
+
 #define SDHCI_OMAP_CMD		0x20c
 
 #define SDHCI_OMAP_PSTATE	0x0224
@@ -63,12 +70,16 @@
 
 #define SDHCI_OMAP_AC12		0x23c
 #define AC12_V1V8_SIGEN		BIT(19)
+#define AC12_SCLK_SEL		BIT(23)
 
 #define SDHCI_OMAP_CAPA		0x240
 #define CAPA_VS33		BIT(24)
 #define CAPA_VS30		BIT(25)
 #define CAPA_VS18		BIT(26)
 
+#define SDHCI_OMAP_CAPA2	0x0244
+#define CAPA2_TSDR50		BIT(13)
+
 #define SDHCI_OMAP_TIMEOUT	1		/* 1 msec */
 
 #define SYSCTL_CLKD_MAX		0x3FF
@@ -77,6 +88,8 @@
 #define IOV_3V0			3000000		/* 300000 uV */
 #define IOV_3V3			3300000		/* 330000 uV */
 
+#define MAX_PHASE_DELAY		0x7C
+
 struct sdhci_omap_data {
 	u32 offset;
 };
@@ -198,6 +211,120 @@ static void sdhci_omap_conf_bus_power(struct sdhci_omap_host *omap_host,
 	}
 }
 
+static inline void sdhci_omap_set_dll(struct sdhci_omap_host *omap_host,
+				      int count)
+{
+	int i;
+	u32 reg;
+
+	reg = sdhci_omap_readl(omap_host, SDHCI_OMAP_DLL);
+	reg |= DLL_FORCE_VALUE;
+	reg &= ~DLL_FORCE_SR_C_MASK;
+	reg |= (count << DLL_FORCE_SR_C_SHIFT);
+	sdhci_omap_writel(omap_host, SDHCI_OMAP_DLL, reg);
+
+	reg |= DLL_CALIB;
+	sdhci_omap_writel(omap_host, SDHCI_OMAP_DLL, reg);
+	for (i = 0; i < 1000; i++) {
+		reg = sdhci_omap_readl(omap_host, SDHCI_OMAP_DLL);
+		if (reg & DLL_CALIB)
+			break;
+	}
+	reg &= ~DLL_CALIB;
+	sdhci_omap_writel(omap_host, SDHCI_OMAP_DLL, reg);
+}
+
+static void sdhci_omap_disable_tuning(struct sdhci_omap_host *omap_host)
+{
+	u32 reg;
+
+	reg = sdhci_omap_readl(omap_host, SDHCI_OMAP_AC12);
+	reg &= ~AC12_SCLK_SEL;
+	sdhci_omap_writel(omap_host, SDHCI_OMAP_AC12, reg);
+
+	reg = sdhci_omap_readl(omap_host, SDHCI_OMAP_DLL);
+	reg &= ~(DLL_FORCE_VALUE | DLL_SWT);
+	sdhci_omap_writel(omap_host, SDHCI_OMAP_DLL, reg);
+}
+
+static int sdhci_omap_execute_tuning(struct mmc_host *mmc, u32 opcode)
+{
+	struct sdhci_host *host = mmc_priv(mmc);
+	struct sdhci_pltfm_host *pltfm_host = sdhci_priv(host);
+	struct sdhci_omap_host *omap_host = sdhci_pltfm_priv(pltfm_host);
+	struct device *dev = omap_host->dev;
+	struct mmc_ios *ios = &mmc->ios;
+	u32 start_window = 0, max_window = 0;
+	u8 cur_match, prev_match = 0;
+	u32 length = 0, max_len = 0;
+	u32 phase_delay = 0;
+	int ret = 0;
+	u32 reg;
+
+	pltfm_host = sdhci_priv(host);
+	omap_host = sdhci_pltfm_priv(pltfm_host);
+	dev = omap_host->dev;
+
+	/* clock tuning is not needed for upto 52MHz */
+	if (ios->clock <= 52000000)
+		return 0;
+
+	reg = sdhci_omap_readl(omap_host, SDHCI_OMAP_CAPA2);
+	if (ios->timing == MMC_TIMING_UHS_SDR50 && !(reg & CAPA2_TSDR50))
+		return 0;
+
+	reg = sdhci_omap_readl(omap_host, SDHCI_OMAP_DLL);
+	reg |= DLL_SWT;
+	sdhci_omap_writel(omap_host, SDHCI_OMAP_DLL, reg);
+
+	while (phase_delay <= MAX_PHASE_DELAY) {
+		sdhci_omap_set_dll(omap_host, phase_delay);
+
+		cur_match = !mmc_send_tuning(mmc, opcode, NULL);
+		if (cur_match) {
+			if (prev_match) {
+				length++;
+			} else {
+				start_window = phase_delay;
+				length = 1;
+			}
+		}
+
+		if (length > max_len) {
+			max_window = start_window;
+			max_len = length;
+		}
+
+		prev_match = cur_match;
+		phase_delay += 4;
+	}
+
+	if (!max_len) {
+		dev_err(dev, "Unable to find match\n");
+		ret = -EIO;
+		goto tuning_error;
+	}
+
+	reg = sdhci_omap_readl(omap_host, SDHCI_OMAP_AC12);
+	if (!(reg & AC12_SCLK_SEL)) {
+		ret = -EIO;
+		goto tuning_error;
+	}
+
+	phase_delay = max_window + 4 * (max_len >> 1);
+	sdhci_omap_set_dll(omap_host, phase_delay);
+
+	goto ret;
+
+tuning_error:
+	dev_err(dev, "Tuning failed\n");
+	sdhci_omap_disable_tuning(omap_host);
+
+ret:
+	sdhci_reset(host, SDHCI_RESET_CMD | SDHCI_RESET_DATA);
+	return ret;
+}
+
 static int sdhci_omap_card_busy(struct mmc_host *mmc)
 {
 	u32 reg, ac12;
@@ -299,6 +426,8 @@ static int sdhci_omap_start_signal_voltage_switch(struct mmc_host *mmc,
 static void sdhci_omap_set_power_mode(struct sdhci_omap_host *omap_host,
 				      u8 power_mode)
 {
+	if (omap_host->bus_mode == MMC_POWER_OFF)
+		sdhci_omap_disable_tuning(omap_host);
 	omap_host->power_mode = power_mode;
 }
 
@@ -635,6 +764,7 @@ static int sdhci_omap_probe(struct platform_device *pdev)
 					sdhci_omap_start_signal_voltage_switch;
 	host->mmc_host_ops.set_ios = sdhci_omap_set_ios;
 	host->mmc_host_ops.card_busy = sdhci_omap_card_busy;
+	host->mmc_host_ops.execute_tuning = sdhci_omap_execute_tuning;
 
 	sdhci_read_caps(host);
 	host->caps |= SDHCI_CAN_DO_ADMA2;
-- 
2.11.0


_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

[PATCH v2 05/16] mmc: sdhci-omap: Workaround for Errata i802

[Kishon Vijay Abraham I]

Errata i802 in AM572x Sitara Processors Silicon Revision 2.0, 1.1
(SPRZ429K July 2014–Revised March 2017 [1]) mentions
DCRC error interrupts (MMCHS_STAT[21] DCRC=0x1) can occur
during the tuning procedure and it has to be disabled during the
tuning procedure Implement workaround for Errata i802 here..

[1] -> http://www.ti.com/lit/er/sprz429k/sprz429k.pdf

Signed-off-by: Kishon Vijay Abraham I <kishon@ti.com>
Acked-by: Adrian Hunter <adrian.hunter@intel.com>
---
 drivers/mmc/host/sdhci-omap.c | 13 +++++++++++++
 1 file changed, 13 insertions(+)

diff --git a/drivers/mmc/host/sdhci-omap.c b/drivers/mmc/host/sdhci-omap.c
index 36e0626d3de2..e24ae903f7ba 100644
--- a/drivers/mmc/host/sdhci-omap.c
+++ b/drivers/mmc/host/sdhci-omap.c
@@ -257,6 +257,7 @@ static int sdhci_omap_execute_tuning(struct mmc_host *mmc, u32 opcode)
 	u32 start_window = 0, max_window = 0;
 	u8 cur_match, prev_match = 0;
 	u32 length = 0, max_len = 0;
+	u32 ier = host->ier;
 	u32 phase_delay = 0;
 	int ret = 0;
 	u32 reg;
@@ -277,6 +278,16 @@ static int sdhci_omap_execute_tuning(struct mmc_host *mmc, u32 opcode)
 	reg |= DLL_SWT;
 	sdhci_omap_writel(omap_host, SDHCI_OMAP_DLL, reg);
 
+	/*
+	 * OMAP5/DRA74X/DRA72x Errata i802:
+	 * DCRC error interrupts (MMCHS_STAT[21] DCRC=0x1) can occur
+	 * during the tuning procedure. So disable it during the
+	 * tuning procedure.
+	 */
+	ier &= ~SDHCI_INT_DATA_CRC;
+	sdhci_writel(host, ier, SDHCI_INT_ENABLE);
+	sdhci_writel(host, ier, SDHCI_SIGNAL_ENABLE);
+
 	while (phase_delay <= MAX_PHASE_DELAY) {
 		sdhci_omap_set_dll(omap_host, phase_delay);
 
@@ -322,6 +333,8 @@ static int sdhci_omap_execute_tuning(struct mmc_host *mmc, u32 opcode)
 
 ret:
 	sdhci_reset(host, SDHCI_RESET_CMD | SDHCI_RESET_DATA);
+	sdhci_writel(host, host->ier, SDHCI_INT_ENABLE);
+	sdhci_writel(host, host->ier, SDHCI_SIGNAL_ENABLE);
 	return ret;
 }
 
-- 
2.11.0

[PATCH v2 06/16] mmc: sdhci_omap: Add support to set IODELAY values

[Kishon Vijay Abraham I]

The data manual of J6/J6 Eco recommends to set different IODELAY values
depending on the mode in which the MMC/SD is enumerated in order to
ensure IO timings are met.

Add support to set the IODELAY values depending on the various MMC
modes using the pinctrl APIs.

Signed-off-by: Kishon Vijay Abraham I <kishon@ti.com>
---
 drivers/mmc/host/sdhci-omap.c | 147 ++++++++++++++++++++++++++++++++++++++++++
 1 file changed, 147 insertions(+)

diff --git a/drivers/mmc/host/sdhci-omap.c b/drivers/mmc/host/sdhci-omap.c
index e24ae903f7ba..8b6170cd689b 100644
--- a/drivers/mmc/host/sdhci-omap.c
+++ b/drivers/mmc/host/sdhci-omap.c
@@ -90,8 +90,12 @@
 
 #define MAX_PHASE_DELAY		0x7C
 
+/* sdhci-omap controller flags */
+#define SDHCI_OMAP_REQUIRE_IODELAY	BIT(0)
+
 struct sdhci_omap_data {
 	u32 offset;
+	u8 flags;
 };
 
 struct sdhci_omap_host {
@@ -102,8 +106,16 @@ struct sdhci_omap_host {
 	struct sdhci_host	*host;
 	u8			bus_mode;
 	u8			power_mode;
+	u8			timing;
+	u8			flags;
+
+	struct pinctrl		*pinctrl;
+	struct pinctrl_state	**pinctrl_state;
 };
 
+static void sdhci_omap_start_clock(struct sdhci_omap_host *omap_host);
+static void sdhci_omap_stop_clock(struct sdhci_omap_host *omap_host);
+
 static inline u32 sdhci_omap_readl(struct sdhci_omap_host *host,
 				   unsigned int offset)
 {
@@ -436,6 +448,31 @@ static int sdhci_omap_start_signal_voltage_switch(struct mmc_host *mmc,
 	return 0;
 }
 
+static void sdhci_omap_set_timing(struct sdhci_omap_host *omap_host, u8 timing)
+{
+	int ret;
+	struct pinctrl_state *pinctrl_state;
+	struct device *dev = omap_host->dev;
+
+	if (!(omap_host->flags & SDHCI_OMAP_REQUIRE_IODELAY))
+		return;
+
+	if (omap_host->timing == timing)
+		return;
+
+	sdhci_omap_stop_clock(omap_host);
+
+	pinctrl_state = omap_host->pinctrl_state[timing];
+	ret = pinctrl_select_state(omap_host->pinctrl, pinctrl_state);
+	if (ret) {
+		dev_err(dev, "failed to select pinctrl state\n");
+		return;
+	}
+
+	sdhci_omap_start_clock(omap_host);
+	omap_host->timing = timing;
+}
+
 static void sdhci_omap_set_power_mode(struct sdhci_omap_host *omap_host,
 				      u8 power_mode)
 {
@@ -472,6 +509,7 @@ static void sdhci_omap_set_ios(struct mmc_host *mmc, struct mmc_ios *ios)
 	omap_host = sdhci_pltfm_priv(pltfm_host);
 
 	sdhci_omap_set_bus_mode(omap_host, ios->bus_mode);
+	sdhci_omap_set_timing(omap_host, ios->timing);
 	sdhci_set_ios(mmc, ios);
 	sdhci_omap_set_power_mode(omap_host, ios->power_mode);
 }
@@ -680,6 +718,7 @@ static const struct sdhci_pltfm_data sdhci_omap_pdata = {
 
 static const struct sdhci_omap_data dra7_data = {
 	.offset = 0x200,
+	.flags	= SDHCI_OMAP_REQUIRE_IODELAY,
 };
 
 static const struct of_device_id omap_sdhci_match[] = {
@@ -688,6 +727,108 @@ static const struct of_device_id omap_sdhci_match[] = {
 };
 MODULE_DEVICE_TABLE(of, omap_sdhci_match);
 
+static struct pinctrl_state
+*sdhci_omap_iodelay_pinctrl_state(struct sdhci_omap_host *omap_host, char *mode,
+				  u32 *caps, u32 capmask)
+{
+	struct device *dev = omap_host->dev;
+	struct pinctrl_state *pinctrl_state = ERR_PTR(-ENODEV);
+
+	if (!(*caps & capmask))
+		goto ret;
+
+	pinctrl_state = pinctrl_lookup_state(omap_host->pinctrl, mode);
+	if (IS_ERR(pinctrl_state)) {
+		dev_err(dev, "no pinctrl state for %s mode", mode);
+		*caps &= ~capmask;
+	}
+
+ret:
+	return pinctrl_state;
+}
+
+static int sdhci_omap_config_iodelay_pinctrl_state(struct sdhci_omap_host
+						   *omap_host)
+{
+	struct device *dev = omap_host->dev;
+	struct sdhci_host *host = omap_host->host;
+	struct mmc_host *mmc = host->mmc;
+	u32 *caps = &mmc->caps;
+	u32 *caps2 = &mmc->caps2;
+	struct pinctrl_state *state;
+	struct pinctrl_state **pinctrl_state;
+
+	if (!(omap_host->flags & SDHCI_OMAP_REQUIRE_IODELAY))
+		return 0;
+
+	pinctrl_state = devm_kzalloc(dev, sizeof(*pinctrl_state) *
+				     (MMC_TIMING_MMC_HS200 + 1), GFP_KERNEL);
+	if (!pinctrl_state)
+		return -ENOMEM;
+
+	omap_host->pinctrl = devm_pinctrl_get(omap_host->dev);
+	if (IS_ERR(omap_host->pinctrl)) {
+		dev_err(dev, "Cannot get pinctrl\n");
+		return PTR_ERR(omap_host->pinctrl);
+	}
+
+	state = pinctrl_lookup_state(omap_host->pinctrl, "default");
+	if (IS_ERR(state)) {
+		dev_err(dev, "no pinctrl state for default mode\n");
+		return PTR_ERR(state);
+	}
+	pinctrl_state[MMC_TIMING_LEGACY] = state;
+
+	state = sdhci_omap_iodelay_pinctrl_state(omap_host, "sdr104", caps,
+						 MMC_CAP_UHS_SDR104);
+	if (!IS_ERR(state))
+		pinctrl_state[MMC_TIMING_UHS_SDR104] = state;
+
+	state = sdhci_omap_iodelay_pinctrl_state(omap_host, "ddr50", caps,
+						 MMC_CAP_UHS_DDR50);
+	if (!IS_ERR(state))
+		pinctrl_state[MMC_TIMING_UHS_DDR50] = state;
+
+	state = sdhci_omap_iodelay_pinctrl_state(omap_host, "sdr50", caps,
+						 MMC_CAP_UHS_SDR50);
+	if (!IS_ERR(state))
+		pinctrl_state[MMC_TIMING_UHS_SDR50] = state;
+
+	state = sdhci_omap_iodelay_pinctrl_state(omap_host, "sdr25", caps,
+						 MMC_CAP_UHS_SDR25);
+	if (!IS_ERR(state))
+		pinctrl_state[MMC_TIMING_UHS_SDR25] = state;
+
+	state = sdhci_omap_iodelay_pinctrl_state(omap_host, "sdr12", caps,
+						 MMC_CAP_UHS_SDR12);
+	if (!IS_ERR(state))
+		pinctrl_state[MMC_TIMING_UHS_SDR12] = state;
+
+	state = sdhci_omap_iodelay_pinctrl_state(omap_host, "ddr_1_8v", caps,
+						 MMC_CAP_1_8V_DDR);
+	if (!IS_ERR(state))
+		pinctrl_state[MMC_TIMING_MMC_DDR52] = state;
+
+	state = sdhci_omap_iodelay_pinctrl_state(omap_host, "hs", caps,
+						 MMC_CAP_SD_HIGHSPEED);
+	if (!IS_ERR(state))
+		pinctrl_state[MMC_TIMING_SD_HS] = state;
+
+	state = sdhci_omap_iodelay_pinctrl_state(omap_host, "hs", caps,
+						 MMC_CAP_MMC_HIGHSPEED);
+	if (!IS_ERR(state))
+		pinctrl_state[MMC_TIMING_MMC_HS] = state;
+
+	state = sdhci_omap_iodelay_pinctrl_state(omap_host, "hs200_1_8v", caps2,
+						 MMC_CAP2_HS200_1_8V_SDR);
+	if (!IS_ERR(state))
+		pinctrl_state[MMC_TIMING_MMC_HS200] = state;
+
+	omap_host->pinctrl_state = pinctrl_state;
+
+	return 0;
+}
+
 static int sdhci_omap_probe(struct platform_device *pdev)
 {
 	int ret;
@@ -724,6 +865,8 @@ static int sdhci_omap_probe(struct platform_device *pdev)
 	omap_host->base = host->ioaddr;
 	omap_host->dev = dev;
 	omap_host->power_mode = MMC_POWER_UNDEFINED;
+	omap_host->timing = MMC_TIMING_LEGACY;
+	omap_host->flags = data->flags;
 	host->ioaddr += offset;
 
 	mmc = host->mmc;
@@ -772,6 +915,10 @@ static int sdhci_omap_probe(struct platform_device *pdev)
 		goto err_put_sync;
 	}
 
+	ret = sdhci_omap_config_iodelay_pinctrl_state(omap_host);
+	if (ret)
+		goto err_put_sync;
+
 	host->mmc_host_ops.get_ro = mmc_gpio_get_ro;
 	host->mmc_host_ops.start_signal_voltage_switch =
 					sdhci_omap_start_signal_voltage_switch;
-- 
2.11.0

[PATCH v2 07/16] mmc: sdhci_omap: Fix sdhci-omap quirks

[Kishon Vijay Abraham I]

Add SDHCI_QUIRK2_PRESET_VALUE_BROKEN quirk as setting preset values loads
incorrect CLKD values (for UHS modes).

Remove SDHCI_QUIRK2_NO_1_8_V quirk as sdhci-omap now supports UHS modes.

Signed-off-by: Kishon Vijay Abraham I <kishon@ti.com>
---
 drivers/mmc/host/sdhci-omap.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/drivers/mmc/host/sdhci-omap.c b/drivers/mmc/host/sdhci-omap.c
index 8b6170cd689b..d5a9e688dc1b 100644
--- a/drivers/mmc/host/sdhci-omap.c
+++ b/drivers/mmc/host/sdhci-omap.c
@@ -710,8 +710,8 @@ static const struct sdhci_pltfm_data sdhci_omap_pdata = {
 		  SDHCI_QUIRK_CAP_CLOCK_BASE_BROKEN |
 		  SDHCI_QUIRK_NO_HISPD_BIT |
 		  SDHCI_QUIRK_BROKEN_ADMA_ZEROLEN_DESC,
-	.quirks2 = SDHCI_QUIRK2_NO_1_8_V |
-		   SDHCI_QUIRK2_ACMD23_BROKEN |
+	.quirks2 = SDHCI_QUIRK2_ACMD23_BROKEN |
+		   SDHCI_QUIRK2_PRESET_VALUE_BROKEN |
 		   SDHCI_QUIRK2_RSP_136_HAS_CRC,
 	.ops = &sdhci_omap_ops,
 };
-- 
2.11.0

[PATCH v2 08/16] mmc: sdhci-omap: Add support to override f_max and iodelay from pdata

[Kishon Vijay Abraham I]

DRA74x EVM Rev H EVM comes with revision 2.0 silicon. However, earlier
versions of EVM can come with either revision 1.1 or revision 1.0 of
silicon.

The device-tree file is written to support rev 2.0 of silicon.
pdata-quirks are used to then override the settings needed for
PG 1.1 silicon.

PG 1.1 silicon has limitations w.r.t frequencies at which MMC1/2/3
can operate as well as different IOdelay numbers.

Add support in sdhci-omap driver to get platform data if available
(added using pdata quirks) and override the data (max-frequency and
iodelay data) obtained from device tree.

Signed-off-by: Kishon Vijay Abraham I <kishon@ti.com>
---
 drivers/mmc/host/sdhci-omap.c            | 21 ++++++++++++++++++++-
 include/linux/platform_data/sdhci-omap.h | 22 ++++++++++++++++++++++
 2 files changed, 42 insertions(+), 1 deletion(-)
 create mode 100644 include/linux/platform_data/sdhci-omap.h

diff --git a/drivers/mmc/host/sdhci-omap.c b/drivers/mmc/host/sdhci-omap.c
index d5a9e688dc1b..c2570db3f7a2 100644
--- a/drivers/mmc/host/sdhci-omap.c
+++ b/drivers/mmc/host/sdhci-omap.c
@@ -22,6 +22,7 @@
 #include <linux/module.h>
 #include <linux/of.h>
 #include <linux/of_device.h>
+#include <linux/platform_data/sdhci-omap.h>
 #include <linux/platform_device.h>
 #include <linux/pm_runtime.h>
 #include <linux/regulator/consumer.h>
@@ -99,6 +100,7 @@ struct sdhci_omap_data {
 };
 
 struct sdhci_omap_host {
+	char			*version;
 	void __iomem		*base;
 	struct device		*dev;
 	struct	regulator	*pbias;
@@ -732,12 +734,21 @@ static struct pinctrl_state
 				  u32 *caps, u32 capmask)
 {
 	struct device *dev = omap_host->dev;
+	char *version = omap_host->version;
 	struct pinctrl_state *pinctrl_state = ERR_PTR(-ENODEV);
+	char str[20];
 
 	if (!(*caps & capmask))
 		goto ret;
 
-	pinctrl_state = pinctrl_lookup_state(omap_host->pinctrl, mode);
+	if (version) {
+		snprintf(str, 20, "%s-%s", mode, version);
+		pinctrl_state = pinctrl_lookup_state(omap_host->pinctrl, str);
+	}
+
+	if (IS_ERR(pinctrl_state))
+		pinctrl_state = pinctrl_lookup_state(omap_host->pinctrl, mode);
+
 	if (IS_ERR(pinctrl_state)) {
 		dev_err(dev, "no pinctrl state for %s mode", mode);
 		*caps &= ~capmask;
@@ -840,6 +851,7 @@ static int sdhci_omap_probe(struct platform_device *pdev)
 	struct mmc_host *mmc;
 	const struct of_device_id *match;
 	struct sdhci_omap_data *data;
+	struct sdhci_omap_platform_data *platform_data;
 
 	match = of_match_device(omap_sdhci_match, dev);
 	if (!match)
@@ -874,6 +886,13 @@ static int sdhci_omap_probe(struct platform_device *pdev)
 	if (ret)
 		goto err_pltfm_free;
 
+	platform_data = dev_get_platdata(dev);
+	if (platform_data) {
+		omap_host->version = platform_data->version;
+		if (platform_data->max_freq)
+			mmc->f_max = platform_data->max_freq;
+	}
+
 	pltfm_host->clk = devm_clk_get(dev, "fck");
 	if (IS_ERR(pltfm_host->clk)) {
 		ret = PTR_ERR(pltfm_host->clk);
diff --git a/include/linux/platform_data/sdhci-omap.h b/include/linux/platform_data/sdhci-omap.h
new file mode 100644
index 000000000000..92abd53dcd00
--- /dev/null
+++ b/include/linux/platform_data/sdhci-omap.h
@@ -0,0 +1,22 @@
+// SPDX-License-Identifier: GPL-2.0
+// Copyright (c) 2018 Texas Instruments
+// SDHCI Controller Platform Data for TI's OMAP SoCs
+// Author: Kishon Vijay Abraham I <kishon@ti.com>
+
+#ifndef __SDHCI_OMAP_PDATA_H__
+#define __SDHCI_OMAP_PDATA_H__
+
+struct sdhci_omap_platform_data {
+	const char *name;
+
+	/*
+	 * set if your board has components or wiring that limits the
+	 * maximum frequency on the MMC bus
+	 */
+	unsigned int max_freq;
+
+	/* string specifying a particular variant of hardware */
+	char *version;
+};
+
+#endif
-- 
2.11.0

[PATCH v2 09/16] mmc: sdhci: Add quirk to disable HW timeout

[Kishon Vijay Abraham I]

Add quirk to disable HW timeout if the requested timeout is more than
the maximum obtainable timeout.

Signed-off-by: Kishon Vijay Abraham I <kishon@ti.com>
---
 drivers/mmc/host/sdhci.c | 12 ++++++++++++
 drivers/mmc/host/sdhci.h |  7 +++++++
 2 files changed, 19 insertions(+)

diff --git a/drivers/mmc/host/sdhci.c b/drivers/mmc/host/sdhci.c
index 070aff9c108f..1aa74b4682f3 100644
--- a/drivers/mmc/host/sdhci.c
+++ b/drivers/mmc/host/sdhci.c
@@ -735,6 +735,12 @@ static u8 sdhci_calc_timeout(struct sdhci_host *host, struct mmc_command *cmd)
 		DBG("Too large timeout 0x%x requested for CMD%d!\n",
 		    count, cmd->opcode);
 		count = 0xE;
+		if (host->quirks2 & SDHCI_QUIRK2_DISABLE_HW_TIMEOUT) {
+			host->ier &= ~SDHCI_INT_DATA_TIMEOUT;
+			sdhci_writel(host, host->ier, SDHCI_INT_ENABLE);
+			sdhci_writel(host, host->ier, SDHCI_SIGNAL_ENABLE);
+			host->hw_timeout_disabled = true;
+		}
 	}
 
 	return count;
@@ -2349,6 +2355,12 @@ static bool sdhci_request_done(struct sdhci_host *host)
 	}
 
 	sdhci_del_timer(host, mrq);
+	if (sdhci_data_line_cmd(mrq->cmd) && host->hw_timeout_disabled) {
+		host->ier |= SDHCI_INT_DATA_TIMEOUT;
+		sdhci_writel(host, host->ier, SDHCI_INT_ENABLE);
+		sdhci_writel(host, host->ier, SDHCI_SIGNAL_ENABLE);
+		host->hw_timeout_disabled = false;
+	}
 
 	/*
 	 * Always unmap the data buffers if they were mapped by
diff --git a/drivers/mmc/host/sdhci.h b/drivers/mmc/host/sdhci.h
index afab26fd70e6..3a967a56fcc3 100644
--- a/drivers/mmc/host/sdhci.h
+++ b/drivers/mmc/host/sdhci.h
@@ -437,6 +437,11 @@ struct sdhci_host {
 #define SDHCI_QUIRK2_CLOCK_DIV_ZERO_BROKEN		(1<<15)
 /* Controller has CRC in 136 bit Command Response */
 #define SDHCI_QUIRK2_RSP_136_HAS_CRC			(1<<16)
+/*
+ * Disable HW timeout if the requested timeout is more than the maximum
+ * obtainable timeout
+ */
+#define SDHCI_QUIRK2_DISABLE_HW_TIMEOUT			(1<<17)
 
 	int irq;		/* Device IRQ */
 	void __iomem *ioaddr;	/* Mapped address */
@@ -522,6 +527,8 @@ struct sdhci_host {
 	unsigned int            ocr_avail_mmc;
 	u32 ocr_mask;		/* available voltages */
 
+	bool			hw_timeout_disabled;
+
 	unsigned		timing;		/* Current timing */
 
 	u32			thread_isr;
-- 
2.11.0

[PATCH v2 10/16] mmc: sdhci: Fix to use data_timer only for data line commands

[Kishon Vijay Abraham I]

commit d7422fb489eee5587d3eec ("mmc: sdhci: Separate timer timeout for
command and data requests") while separating timer timeout for
command and data requests, passed cmd->mrq to sdhci_mod_timer (cmd is an
argument to sdhci_send_command) and in sdhci_mod_timer used mrq->cmd
to check if it is a data line command. This results in using
data timer for commands like MMC_SET_BLOCK_COUNT (CMD23) though it is
not a data line command. Fix it here.

Fixes: commit d7422fb489eee5587d3eec ("mmc: sdhci: Separate timer timeout for
command and data requests")

Signed-off-by: Kishon Vijay Abraham I <kishon@ti.com>
---
 drivers/mmc/host/sdhci.c | 6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)

diff --git a/drivers/mmc/host/sdhci.c b/drivers/mmc/host/sdhci.c
index 1aa74b4682f3..0489572d1892 100644
--- a/drivers/mmc/host/sdhci.c
+++ b/drivers/mmc/host/sdhci.c
@@ -1073,10 +1073,10 @@ static void sdhci_finish_data(struct sdhci_host *host)
 	}
 }
 
-static void sdhci_mod_timer(struct sdhci_host *host, struct mmc_request *mrq,
+static void sdhci_mod_timer(struct sdhci_host *host, struct mmc_command *cmd,
 			    unsigned long timeout)
 {
-	if (sdhci_data_line_cmd(mrq->cmd))
+	if (sdhci_data_line_cmd(cmd))
 		mod_timer(&host->data_timer, timeout);
 	else
 		mod_timer(&host->timer, timeout);
@@ -1135,7 +1135,7 @@ void sdhci_send_command(struct sdhci_host *host, struct mmc_command *cmd)
 		timeout += DIV_ROUND_UP(cmd->busy_timeout, 1000) * HZ + HZ;
 	else
 		timeout += 10 * HZ;
-	sdhci_mod_timer(host, cmd->mrq, timeout);
+	sdhci_mod_timer(host, cmd, timeout);
 
 	host->cmd = cmd;
 	if (sdhci_data_line_cmd(cmd)) {
-- 
2.11.0

[PATCH v2 11/16] mmc: sdhci: Program a relatively accurate SW timeout value

[Kishon Vijay Abraham I]

sdhci has a 10 second timeout to catch devices that stop responding.
Instead of programming 10 second arbitrary value, calculate the total time
it would take for the entire transfer to happen and program the timeout
value accordingly.

Signed-off-by: Kishon Vijay Abraham I <kishon@ti.com>
---
 drivers/mmc/host/sdhci.c | 46 +++++++++++++++++++++++++++++++++++++++-------
 drivers/mmc/host/sdhci.h | 10 ++++++++++
 2 files changed, 49 insertions(+), 7 deletions(-)

diff --git a/drivers/mmc/host/sdhci.c b/drivers/mmc/host/sdhci.c
index 0489572d1892..d52f9e7eabe2 100644
--- a/drivers/mmc/host/sdhci.c
+++ b/drivers/mmc/host/sdhci.c
@@ -673,6 +673,37 @@ static void sdhci_adma_table_post(struct sdhci_host *host,
 	}
 }
 
+static void sdhci_calc_sw_timeout(struct sdhci_host *host,
+				  struct mmc_command *cmd,
+				  unsigned int target_timeout)
+{
+	struct mmc_data *data = cmd->data;
+	struct mmc_host *mmc = host->mmc;
+	unsigned long long transfer_time;
+	struct mmc_ios *ios = &mmc->ios;
+	unsigned char bus_width = ios->bus_width;
+	unsigned int blksz;
+	unsigned int freq;
+
+	if (data) {
+		blksz = data->blksz;
+		freq = host->mmc->actual_clock ? host->mmc->actual_clock :
+						host->clock;
+		transfer_time = (unsigned long long)(blksz * NSEC_PER_SEC *
+						     (8 / bus_width)) / freq;
+		/* multiply by '2' to account for any unknowns */
+		transfer_time = transfer_time * 2;
+		/* calculate timeout for the entire data */
+		host->data_timeout = (data->blocks * ((target_timeout *
+						       NSEC_PER_USEC) +
+						       transfer_time));
+	} else {
+		host->data_timeout = target_timeout * NSEC_PER_USEC;
+	}
+
+	host->data_timeout += MMC_CMD_TRANSFER_TIME;
+}
+
 static u8 sdhci_calc_timeout(struct sdhci_host *host, struct mmc_command *cmd)
 {
 	u8 count;
@@ -742,6 +773,7 @@ static u8 sdhci_calc_timeout(struct sdhci_host *host, struct mmc_command *cmd)
 			host->hw_timeout_disabled = true;
 		}
 	}
+	sdhci_calc_sw_timeout(host, cmd, target_timeout);
 
 	return count;
 }
@@ -1130,13 +1162,6 @@ void sdhci_send_command(struct sdhci_host *host, struct mmc_command *cmd)
 		mdelay(1);
 	}
 
-	timeout = jiffies;
-	if (!cmd->data && cmd->busy_timeout > 9000)
-		timeout += DIV_ROUND_UP(cmd->busy_timeout, 1000) * HZ + HZ;
-	else
-		timeout += 10 * HZ;
-	sdhci_mod_timer(host, cmd, timeout);
-
 	host->cmd = cmd;
 	if (sdhci_data_line_cmd(cmd)) {
 		WARN_ON(host->data_cmd);
@@ -1176,6 +1201,13 @@ void sdhci_send_command(struct sdhci_host *host, struct mmc_command *cmd)
 	    cmd->opcode == MMC_SEND_TUNING_BLOCK_HS200)
 		flags |= SDHCI_CMD_DATA;
 
+	timeout = jiffies;
+	if (sdhci_data_line_cmd(cmd))
+		timeout += nsecs_to_jiffies(host->data_timeout);
+	else
+		timeout += 10 * HZ;
+	sdhci_mod_timer(host, cmd, timeout);
+
 	sdhci_writew(host, SDHCI_MAKE_CMD(cmd->opcode, flags), SDHCI_COMMAND);
 }
 EXPORT_SYMBOL_GPL(sdhci_send_command);
diff --git a/drivers/mmc/host/sdhci.h b/drivers/mmc/host/sdhci.h
index 3a967a56fcc3..b73577d77856 100644
--- a/drivers/mmc/host/sdhci.h
+++ b/drivers/mmc/host/sdhci.h
@@ -332,6 +332,14 @@ struct sdhci_adma2_64_desc {
 /* Allow for a a command request and a data request at the same time */
 #define SDHCI_MAX_MRQS		2
 
+/*
+ * 48bit command and 136 bit response in 400KHz clock should take 0.46ms.
+ * However since the start time of the command, the time between
+ * command and response, and the time between response and start of data is
+ * not known, set the command transfer time to 2ms.
+ */
+#define MMC_CMD_TRANSFER_TIME	(2 * NSEC_PER_MSEC) /* max 2 ms */
+
 enum sdhci_cookie {
 	COOKIE_UNMAPPED,
 	COOKIE_PRE_MAPPED,	/* mapped by sdhci_pre_req() */
@@ -554,6 +562,8 @@ struct sdhci_host {
 	/* Host SDMA buffer boundary. */
 	u32			sdma_boundary;
 
+	unsigned long long	data_timeout;
+
 	unsigned long private[0] ____cacheline_aligned;
 };
 
-- 
2.11.0

[PATCH v2 12/16] mmc: sdhci-omap: Workaround for Errata i834

[Kishon Vijay Abraham I]

Errata i834 in AM572x Sitara Processors Silicon Revision 2.0, 1.1
(SPRZ429K July 2014–Revised March 2017 [1]) mentions the maximum
obtainable timeout through MMC host controller is 700ms. And for
commands taking longer than 700ms, hardware timeout should be
disabled and software timeout should be used.

The workaround for Errata i834 can be achieved by adding
SDHCI_QUIRK2_DISABLE_HW_TIMEOUT quirk in sdhci-omap.

[1] -> http://www.ti.com/lit/er/sprz429k/sprz429k.pdf

Signed-off-by: Kishon Vijay Abraham I <kishon@ti.com>
---
 drivers/mmc/host/sdhci-omap.c | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/drivers/mmc/host/sdhci-omap.c b/drivers/mmc/host/sdhci-omap.c
index c2570db3f7a2..db487b77fbcc 100644
--- a/drivers/mmc/host/sdhci-omap.c
+++ b/drivers/mmc/host/sdhci-omap.c
@@ -714,7 +714,8 @@ static const struct sdhci_pltfm_data sdhci_omap_pdata = {
 		  SDHCI_QUIRK_BROKEN_ADMA_ZEROLEN_DESC,
 	.quirks2 = SDHCI_QUIRK2_ACMD23_BROKEN |
 		   SDHCI_QUIRK2_PRESET_VALUE_BROKEN |
-		   SDHCI_QUIRK2_RSP_136_HAS_CRC,
+		   SDHCI_QUIRK2_RSP_136_HAS_CRC |
+		   SDHCI_QUIRK2_DISABLE_HW_TIMEOUT,
 	.ops = &sdhci_omap_ops,
 };
 
-- 
2.11.0

[PATCH v2 13/16] dt-bindings: sdhci-omap: Add K2G specific binding

[Kishon Vijay Abraham I]

Add binding for the TI's sdhci-omap controller present in K2G.

Signed-off-by: Kishon Vijay Abraham I <kishon@ti.com>
Reviewed-by: Rob Herring <robh@kernel.org>
---
 Documentation/devicetree/bindings/mmc/sdhci-omap.txt | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/Documentation/devicetree/bindings/mmc/sdhci-omap.txt b/Documentation/devicetree/bindings/mmc/sdhci-omap.txt
index 51775a372c06..8d09b837e350 100644
--- a/Documentation/devicetree/bindings/mmc/sdhci-omap.txt
+++ b/Documentation/devicetree/bindings/mmc/sdhci-omap.txt
@@ -4,7 +4,9 @@ Refer to mmc.txt for standard MMC bindings.
 
 Required properties:
 - compatible: Should be "ti,dra7-sdhci" for DRA7 and DRA72 controllers
+	      Should be "ti,k2g-sdhci" for K2G
 - ti,hwmods: Must be "mmc<n>", <n> is controller instance starting 1
+	     (Not required for K2G).
 
 Example:
 	mmc1: mmc@4809c000 {
-- 
2.11.0

[PATCH v2 14/16] mmc: sdhci-omap: Add support for MMC/SD controller in k2g SoC

[Kishon Vijay Abraham I]

Add support for the new compatible added specifically to support
k2g's MMC/SD controller.

Signed-off-by: Kishon Vijay Abraham I <kishon@ti.com>
Acked-by: Adrian Hunter <adrian.hunter@intel.com>
---
 drivers/mmc/host/sdhci-omap.c | 9 +++++++++
 1 file changed, 9 insertions(+)

diff --git a/drivers/mmc/host/sdhci-omap.c b/drivers/mmc/host/sdhci-omap.c
index db487b77fbcc..863f6736c2fe 100644
--- a/drivers/mmc/host/sdhci-omap.c
+++ b/drivers/mmc/host/sdhci-omap.c
@@ -719,6 +719,10 @@ static const struct sdhci_pltfm_data sdhci_omap_pdata = {
 	.ops = &sdhci_omap_ops,
 };
 
+static const struct sdhci_omap_data k2g_data = {
+	.offset = 0x200,
+};
+
 static const struct sdhci_omap_data dra7_data = {
 	.offset = 0x200,
 	.flags	= SDHCI_OMAP_REQUIRE_IODELAY,
@@ -726,6 +730,7 @@ static const struct sdhci_omap_data dra7_data = {
 
 static const struct of_device_id omap_sdhci_match[] = {
 	{ .compatible = "ti,dra7-sdhci", .data = &dra7_data },
+	{ .compatible = "ti,k2g-sdhci", .data = &k2g_data },
 	{},
 };
 MODULE_DEVICE_TABLE(of, omap_sdhci_match);
@@ -846,6 +851,7 @@ static int sdhci_omap_probe(struct platform_device *pdev)
 	int ret;
 	u32 offset;
 	struct device *dev = &pdev->dev;
+	struct device_node *node = dev->of_node;
 	struct sdhci_host *host;
 	struct sdhci_pltfm_host *pltfm_host;
 	struct sdhci_omap_host *omap_host;
@@ -872,6 +878,9 @@ static int sdhci_omap_probe(struct platform_device *pdev)
 		return PTR_ERR(host);
 	}
 
+	if (of_device_is_compatible(node, "ti,k2g-sdhci"))
+		host->quirks2 |= SDHCI_QUIRK2_NO_1_8_V;
+
 	pltfm_host = sdhci_priv(host);
 	omap_host = sdhci_pltfm_priv(pltfm_host);
 	omap_host->host = host;
-- 
2.11.0

[PATCH v2 15/16] mmc: sdhci-omap: Add SPDX identifier

[Kishon Vijay Abraham I]

The SPDX identifier is a legally binding shorthand, which can be used
instead of the full boiler plate text. Update sdhci-omap driver with the
correct SPDX license identifier.

Signed-off-by: Kishon Vijay Abraham I <kishon-l0cyMroinI0@public.gmane.org>
---
 drivers/mmc/host/sdhci-omap.c | 22 ++++------------------
 1 file changed, 4 insertions(+), 18 deletions(-)

diff --git a/drivers/mmc/host/sdhci-omap.c b/drivers/mmc/host/sdhci-omap.c
index 863f6736c2fe..d44918172197 100644
--- a/drivers/mmc/host/sdhci-omap.c
+++ b/drivers/mmc/host/sdhci-omap.c
@@ -1,21 +1,7 @@
-/**
- * SDHCI Controller driver for TI's OMAP SoCs
- *
- * Copyright (C) 2017 Texas Instruments
- * Author: Kishon Vijay Abraham I <kishon-l0cyMroinI0@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 version 2 of
- * the License as published by the Free Software Foundation.
- *
- * 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.
- *
- * You should have received a copy of the GNU General Public License
- * along with this program.  If not, see <http://www.gnu.org/licenses/>.
- */
+// SPDX-License-Identifier: GPL-2.0
+// Copyright (c) 2018 Texas Instruments
+// SDHCI Controller driver for TI's OMAP SoCs
+// Author: Kishon Vijay Abraham I <kishon-l0cyMroinI0@public.gmane.org>
 
 #include <linux/delay.h>
 #include <linux/mmc/slot-gpio.h>
-- 
2.11.0

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

[PATCH v2 16/16] ARM: OMAP2+: Use sdhci-omap specific pdata-quirks for MMC/SD on DRA74x EVM

[Kishon Vijay Abraham I]

Use sdhci-omap specific pdata-quirks for MMC/SD on DRA74x EVM since
UHS modes are supported only in sdhci-omap (and not in omap-hsmmc).

Signed-off-by: Kishon Vijay Abraham I <kishon@ti.com>
---
 arch/arm/mach-omap2/pdata-quirks.c | 34 +++++++++++++++++-----------------
 1 file changed, 17 insertions(+), 17 deletions(-)

diff --git a/arch/arm/mach-omap2/pdata-quirks.c b/arch/arm/mach-omap2/pdata-quirks.c
index 6b433fce65a5..92fb8828d57f 100644
--- a/arch/arm/mach-omap2/pdata-quirks.c
+++ b/arch/arm/mach-omap2/pdata-quirks.c
@@ -21,7 +21,7 @@
 #include <linux/regulator/fixed.h>
 
 #include <linux/platform_data/pinctrl-single.h>
-#include <linux/platform_data/hsmmc-omap.h>
+#include <linux/platform_data/sdhci-omap.h>
 #include <linux/platform_data/iommu-omap.h>
 #include <linux/platform_data/wkup_m3.h>
 #include <linux/platform_data/pwm_omap_dmtimer.h>
@@ -38,7 +38,7 @@
 #include "soc.h"
 #include "hsmmc.h"
 
-static struct omap_hsmmc_platform_data __maybe_unused mmc_pdata[2];
+static struct sdhci_omap_platform_data __maybe_unused mmc_pdata[2];
 
 struct pdata_init {
 	const char *compatible;
@@ -435,21 +435,21 @@ static void __init omap5_uevm_legacy_init(void)
 #endif
 
 #ifdef CONFIG_SOC_DRA7XX
-static struct omap_hsmmc_platform_data dra7_hsmmc_data_mmc1;
-static struct omap_hsmmc_platform_data dra7_hsmmc_data_mmc2;
-static struct omap_hsmmc_platform_data dra7_hsmmc_data_mmc3;
+static struct sdhci_omap_platform_data dra7_sdhci_data_mmc1;
+static struct sdhci_omap_platform_data dra7_sdhci_data_mmc2;
+static struct sdhci_omap_platform_data dra7_sdhci_data_mmc3;
 
 static void __init dra7x_evm_mmc_quirk(void)
 {
 	if (omap_rev() == DRA752_REV_ES1_1 || omap_rev() == DRA752_REV_ES1_0) {
-		dra7_hsmmc_data_mmc1.version = "rev11";
-		dra7_hsmmc_data_mmc1.max_freq = 96000000;
+		dra7_sdhci_data_mmc1.version = "rev11";
+		dra7_sdhci_data_mmc1.max_freq = 96000000;
 
-		dra7_hsmmc_data_mmc2.version = "rev11";
-		dra7_hsmmc_data_mmc2.max_freq = 48000000;
+		dra7_sdhci_data_mmc2.version = "rev11";
+		dra7_sdhci_data_mmc2.max_freq = 48000000;
 
-		dra7_hsmmc_data_mmc3.version = "rev11";
-		dra7_hsmmc_data_mmc3.max_freq = 48000000;
+		dra7_sdhci_data_mmc3.version = "rev11";
+		dra7_sdhci_data_mmc3.max_freq = 48000000;
 	}
 }
 #endif
@@ -582,12 +582,12 @@ static struct of_dev_auxdata omap_auxdata_lookup[] __initdata = {
 		       &omap4_iommu_pdata),
 #endif
 #ifdef CONFIG_SOC_DRA7XX
-	OF_DEV_AUXDATA("ti,dra7-hsmmc", 0x4809c000, "4809c000.mmc",
-		       &dra7_hsmmc_data_mmc1),
-	OF_DEV_AUXDATA("ti,dra7-hsmmc", 0x480b4000, "480b4000.mmc",
-		       &dra7_hsmmc_data_mmc2),
-	OF_DEV_AUXDATA("ti,dra7-hsmmc", 0x480ad000, "480ad000.mmc",
-		       &dra7_hsmmc_data_mmc3),
+	OF_DEV_AUXDATA("ti,dra7-sdhci", 0x4809c000, "4809c000.mmc",
+		       &dra7_sdhci_data_mmc1),
+	OF_DEV_AUXDATA("ti,dra7-sdhci", 0x480b4000, "480b4000.mmc",
+		       &dra7_sdhci_data_mmc2),
+	OF_DEV_AUXDATA("ti,dra7-sdhci", 0x480ad000, "480ad000.mmc",
+		       &dra7_sdhci_data_mmc3),
 #endif
 	/* Common auxdata */
 	OF_DEV_AUXDATA("pinctrl-single", 0, NULL, &pcs_pdata),
-- 
2.11.0

Re: [PATCH v2 15/16] mmc: sdhci-omap: Add SPDX identifier

[Joe Perches]

(Adding Kate Stewart)

On Mon, 2018-02-05 at 18:20 +0530, Kishon Vijay Abraham I wrote:
> The SPDX identifier is a legally binding shorthand,

As far as I know that "legally binding" bit is unproven.
So maybe, maybe not.

I don't doubt that it should be added, but I would be
hesitant to state in a commit message that it is
legally binding.

> which can be used
> instead of the full boiler plate text.

I would also be hesitant to remove existing license text.
--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH v2 08/16] mmc: sdhci-omap: Add support to override f_max and iodelay from pdata

[Ulf Hansson]

On 5 February 2018 at 13:50, Kishon Vijay Abraham I <kishon@ti.com> wrote:
> DRA74x EVM Rev H EVM comes with revision 2.0 silicon. However, earlier
> versions of EVM can come with either revision 1.1 or revision 1.0 of
> silicon.
>
> The device-tree file is written to support rev 2.0 of silicon.
> pdata-quirks are used to then override the settings needed for
> PG 1.1 silicon.
>
> PG 1.1 silicon has limitations w.r.t frequencies at which MMC1/2/3
> can operate as well as different IOdelay numbers.

I fail to understand the need for platform data. Why can't DT be used
for the older revisions as well?

>
> Add support in sdhci-omap driver to get platform data if available
> (added using pdata quirks) and override the data (max-frequency and
> iodelay data) obtained from device tree.

[...]

Kind regards
Uffe

Re: [PATCH v2 00/16] mmc: sdhci-omap: Add UHS/HS200 mode support

[Ulf Hansson]

On 5 February 2018 at 13:50, Kishon Vijay Abraham I <kishon-l0cyMroinI0@public.gmane.org> wrote:
> Add UHS/HS200 mode support in sdhci-omap. The programming sequence
> for voltage switching, tuning is followed from AM572x TRM
> http://www.ti.com/lit/ug/spruhz6i/spruhz6i.pdf
> (Similar to all AM57x/DRA7x SoCs). The patch series also implements
> workaround for errata published in
> http://www.ti.com/lit/er/sprz429k/sprz429k.pdf.
>
> It also includes a pdata-quirk patch since both pdata-quirks and
> sdhci-omap uses struct sdhci_omap_platform_data.
>
> The dt patches enabling UHS/HS200 will follow this series.
>
> Changes from v1:
> *) Only poll on DAT0 and DATI for card_busy status
> *) Cleanup iodelay patch as suggested by Tony.
> *) Added quirk to disable HW timeout
> *) Use the existing data timer but program a relatively accurate
>    SW timeout value (Impacts all platforms)
> *) Fix a bug in sdhci which was using data_timer for non data line
>    commands
>
> I've tested on dra72-evm, dra7xx-evm, dra71-evm, am572x-idk, am571x-idk
> dra76-evm, am57xx-evm and k2g-evm.
>
> I've also pushed the series (along with dt) to
> https://github.com/kishon/linux-wip.git sdhci_uhs_v2
>
> Kishon Vijay Abraham I (16):
>   mmc: sdhci-omap: Update 'power_mode' outside sdhci_omap_init_74_clocks
>   mmc: sdhci-omap: Add card_busy host ops
>   mmc: sdhci-omap: Add custom set_uhs_signaling sdhci_host ops
>   mmc: sdhci-omap: Add tuning support
>   mmc: sdhci-omap: Workaround for Errata i802
>   mmc: sdhci_omap: Add support to set IODELAY values
>   mmc: sdhci_omap: Fix sdhci-omap quirks
>   mmc: sdhci-omap: Add support to override f_max and iodelay from pdata
>   mmc: sdhci: Add quirk to disable HW timeout
>   mmc: sdhci: Fix to use data_timer only for data line commands
>   mmc: sdhci: Program a relatively accurate SW timeout value
>   mmc: sdhci-omap: Workaround for Errata i834
>   dt-bindings: sdhci-omap: Add K2G specific binding
>   mmc: sdhci-omap: Add support for MMC/SD controller in k2g SoC
>   mmc: sdhci-omap: Add SPDX identifier
>   ARM: OMAP2+: Use sdhci-omap specific pdata-quirks for MMC/SD on DRA74x
>     EVM
>
>  .../devicetree/bindings/mmc/sdhci-omap.txt         |   2 +
>  arch/arm/mach-omap2/pdata-quirks.c                 |  34 +-
>  drivers/mmc/host/sdhci-omap.c                      | 430 +++++++++++++++++++--
>  drivers/mmc/host/sdhci.c                           |  62 ++-
>  drivers/mmc/host/sdhci.h                           |  17 +
>  include/linux/platform_data/sdhci-omap.h           |  22 ++
>  6 files changed, 518 insertions(+), 49 deletions(-)
>  create mode 100644 include/linux/platform_data/sdhci-omap.h
>
> --
> 2.11.0
>

To move forward and while waiting for Adrian to come back, I have
applied patch 1 -> 7 for next.

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

Re: [PATCH v2 08/16] mmc: sdhci-omap: Add support to override f_max and iodelay from pdata

[Tony Lindgren]

* Ulf Hansson <ulf.hansson@linaro.org> [180214 09:53]:
> On 5 February 2018 at 13:50, Kishon Vijay Abraham I <kishon@ti.com> wrote:
> > DRA74x EVM Rev H EVM comes with revision 2.0 silicon. However, earlier
> > versions of EVM can come with either revision 1.1 or revision 1.0 of
> > silicon.
> >
> > The device-tree file is written to support rev 2.0 of silicon.
> > pdata-quirks are used to then override the settings needed for
> > PG 1.1 silicon.
> >
> > PG 1.1 silicon has limitations w.r.t frequencies at which MMC1/2/3
> > can operate as well as different IOdelay numbers.
> 
> I fail to understand the need for platform data. Why can't DT be used
> for the older revisions as well?

Maybe the silicon revision is not available via soc_device_match()?

Regards,

Tony

Re: [PATCH v2 11/16] mmc: sdhci: Program a relatively accurate SW timeout value

[Kishon Vijay Abraham I]

On Monday 05 February 2018 06:20 PM, Kishon Vijay Abraham I wrote:
> sdhci has a 10 second timeout to catch devices that stop responding.
> Instead of programming 10 second arbitrary value, calculate the total time
> it would take for the entire transfer to happen and program the timeout
> value accordingly.
> 
> Signed-off-by: Kishon Vijay Abraham I <kishon-l0cyMroinI0@public.gmane.org>
> ---
>  drivers/mmc/host/sdhci.c | 46 +++++++++++++++++++++++++++++++++++++++-------
>  drivers/mmc/host/sdhci.h | 10 ++++++++++
>  2 files changed, 49 insertions(+), 7 deletions(-)
> 
> diff --git a/drivers/mmc/host/sdhci.c b/drivers/mmc/host/sdhci.c
> index 0489572d1892..d52f9e7eabe2 100644
> --- a/drivers/mmc/host/sdhci.c
> +++ b/drivers/mmc/host/sdhci.c
> @@ -673,6 +673,37 @@ static void sdhci_adma_table_post(struct sdhci_host *host,
>  	}
>  }
>  
> +static void sdhci_calc_sw_timeout(struct sdhci_host *host,
> +				  struct mmc_command *cmd,
> +				  unsigned int target_timeout)
> +{
> +	struct mmc_data *data = cmd->data;
> +	struct mmc_host *mmc = host->mmc;
> +	unsigned long long transfer_time;
> +	struct mmc_ios *ios = &mmc->ios;
> +	unsigned char bus_width = ios->bus_width;

This should have been 1 << ios->bus_width.

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

Re: [PATCH v2 08/16] mmc: sdhci-omap: Add support to override f_max and iodelay from pdata

[Kishon Vijay Abraham I]

Hi,

On Wednesday 14 February 2018 10:54 PM, Tony Lindgren wrote:
> * Ulf Hansson <ulf.hansson@linaro.org> [180214 09:53]:
>> On 5 February 2018 at 13:50, Kishon Vijay Abraham I <kishon@ti.com> wrote:
>>> DRA74x EVM Rev H EVM comes with revision 2.0 silicon. However, earlier
>>> versions of EVM can come with either revision 1.1 or revision 1.0 of
>>> silicon.
>>>
>>> The device-tree file is written to support rev 2.0 of silicon.
>>> pdata-quirks are used to then override the settings needed for
>>> PG 1.1 silicon.
>>>
>>> PG 1.1 silicon has limitations w.r.t frequencies at which MMC1/2/3
>>> can operate as well as different IOdelay numbers.
>>
>> I fail to understand the need for platform data. Why can't DT be used
>> for the older revisions as well?

It was decided to have dt file for a single soc revision. So the default dts
corresponds to rev 2.0.
> 
> Maybe the silicon revision is not available via soc_device_match()?

hmm.. looks like we can get rid of pdata if we use soc_device_match. Thanks for
the hint.

Regards
Kishon

Re: [PATCH v2 10/16] mmc: sdhci: Fix to use data_timer only for data line commands

[Adrian Hunter]

On 05/02/18 14:50, Kishon Vijay Abraham I wrote:
> commit d7422fb489eee5587d3eec ("mmc: sdhci: Separate timer timeout for
> command and data requests") while separating timer timeout for
> command and data requests, passed cmd->mrq to sdhci_mod_timer (cmd is an
> argument to sdhci_send_command) and in sdhci_mod_timer used mrq->cmd
> to check if it is a data line command. This results in using
> data timer for commands like MMC_SET_BLOCK_COUNT (CMD23) though it is
> not a data line command. Fix it here.

I am not sure why you need this change, but it is not right actually.  There
are 2 timers because there can be 2 mrqs and we need to delete the correct
timer in sdhci_request_done() so it is better to make the selection based on
the mrq not the last command.

> 
> Fixes: commit d7422fb489eee5587d3eec ("mmc: sdhci: Separate timer timeout for
> command and data requests")
> 
> Signed-off-by: Kishon Vijay Abraham I <kishon@ti.com>
> ---
>  drivers/mmc/host/sdhci.c | 6 +++---
>  1 file changed, 3 insertions(+), 3 deletions(-)
> 
> diff --git a/drivers/mmc/host/sdhci.c b/drivers/mmc/host/sdhci.c
> index 1aa74b4682f3..0489572d1892 100644
> --- a/drivers/mmc/host/sdhci.c
> +++ b/drivers/mmc/host/sdhci.c
> @@ -1073,10 +1073,10 @@ static void sdhci_finish_data(struct sdhci_host *host)
>  	}
>  }
>  
> -static void sdhci_mod_timer(struct sdhci_host *host, struct mmc_request *mrq,
> +static void sdhci_mod_timer(struct sdhci_host *host, struct mmc_command *cmd,
>  			    unsigned long timeout)
>  {
> -	if (sdhci_data_line_cmd(mrq->cmd))
> +	if (sdhci_data_line_cmd(cmd))
>  		mod_timer(&host->data_timer, timeout);
>  	else
>  		mod_timer(&host->timer, timeout);
> @@ -1135,7 +1135,7 @@ void sdhci_send_command(struct sdhci_host *host, struct mmc_command *cmd)
>  		timeout += DIV_ROUND_UP(cmd->busy_timeout, 1000) * HZ + HZ;
>  	else
>  		timeout += 10 * HZ;
> -	sdhci_mod_timer(host, cmd->mrq, timeout);
> +	sdhci_mod_timer(host, cmd, timeout);
>  
>  	host->cmd = cmd;
>  	if (sdhci_data_line_cmd(cmd)) {
> 

Re: [PATCH v2 09/16] mmc: sdhci: Add quirk to disable HW timeout

[Adrian Hunter]

On 05/02/18 14:50, Kishon Vijay Abraham I wrote:
> Add quirk to disable HW timeout if the requested timeout is more than
> the maximum obtainable timeout.
> 
> Signed-off-by: Kishon Vijay Abraham I <kishon@ti.com>
> ---
>  drivers/mmc/host/sdhci.c | 12 ++++++++++++
>  drivers/mmc/host/sdhci.h |  7 +++++++
>  2 files changed, 19 insertions(+)
> 
> diff --git a/drivers/mmc/host/sdhci.c b/drivers/mmc/host/sdhci.c
> index 070aff9c108f..1aa74b4682f3 100644
> --- a/drivers/mmc/host/sdhci.c
> +++ b/drivers/mmc/host/sdhci.c
> @@ -735,6 +735,12 @@ static u8 sdhci_calc_timeout(struct sdhci_host *host, struct mmc_command *cmd)
>  		DBG("Too large timeout 0x%x requested for CMD%d!\n",
>  		    count, cmd->opcode);
>  		count = 0xE;
> +		if (host->quirks2 & SDHCI_QUIRK2_DISABLE_HW_TIMEOUT) {
> +			host->ier &= ~SDHCI_INT_DATA_TIMEOUT;
> +			sdhci_writel(host, host->ier, SDHCI_INT_ENABLE);
> +			sdhci_writel(host, host->ier, SDHCI_SIGNAL_ENABLE);
> +			host->hw_timeout_disabled = true;
> +		}

sdhci_calc_timeout() is really for calculations so please do this in
sdhci_set_timeout() instead (i.e. change sdhci_calc_timeout() to return
whether the timeout is too big).  Note that you need to cater for the "/*
Unspecified timeout, assume max */" case and the
SDHCI_QUIRK_BROKEN_TIMEOUT_VAL case.

Also if SDHCI_INT_DATA_TIMEOUT is not being disabled, check here to
re-enable it and leave sdhci_request_done() alone. i.e.

	else if (!(host->ier & SDHCI_INT_DATA_TIMEOUT)) {
		host->ier |= SDHCI_INT_DATA_TIMEOUT;
		sdhci_writel(host, host->ier, SDHCI_INT_ENABLE);
		sdhci_writel(host, host->ier, SDHCI_SIGNAL_ENABLE);
	}

Maybe make a separate subroutine for the update:

void sdhci_set_data_timeout_irq(struct sdhci_host *host, bool enable)
{
	if (enable)
		host->ier |= SDHCI_INT_DATA_TIMEOUT;
	else
		host->ier &= ~SDHCI_INT_DATA_TIMEOUT;
	sdhci_writel(host, host->ier, SDHCI_INT_ENABLE);
	sdhci_writel(host, host->ier, SDHCI_SIGNAL_ENABLE);
}


>  	}
>  
>  	return count;
> @@ -2349,6 +2355,12 @@ static bool sdhci_request_done(struct sdhci_host *host)
>  	}
>  
>  	sdhci_del_timer(host, mrq);
> +	if (sdhci_data_line_cmd(mrq->cmd) && host->hw_timeout_disabled) {
> +		host->ier |= SDHCI_INT_DATA_TIMEOUT;
> +		sdhci_writel(host, host->ier, SDHCI_INT_ENABLE);
> +		sdhci_writel(host, host->ier, SDHCI_SIGNAL_ENABLE);
> +		host->hw_timeout_disabled = false;
> +	}
>  
>  	/*
>  	 * Always unmap the data buffers if they were mapped by
> diff --git a/drivers/mmc/host/sdhci.h b/drivers/mmc/host/sdhci.h
> index afab26fd70e6..3a967a56fcc3 100644
> --- a/drivers/mmc/host/sdhci.h
> +++ b/drivers/mmc/host/sdhci.h
> @@ -437,6 +437,11 @@ struct sdhci_host {
>  #define SDHCI_QUIRK2_CLOCK_DIV_ZERO_BROKEN		(1<<15)
>  /* Controller has CRC in 136 bit Command Response */
>  #define SDHCI_QUIRK2_RSP_136_HAS_CRC			(1<<16)
> +/*
> + * Disable HW timeout if the requested timeout is more than the maximum
> + * obtainable timeout
> + */
> +#define SDHCI_QUIRK2_DISABLE_HW_TIMEOUT			(1<<17)
>  
>  	int irq;		/* Device IRQ */
>  	void __iomem *ioaddr;	/* Mapped address */
> @@ -522,6 +527,8 @@ struct sdhci_host {
>  	unsigned int            ocr_avail_mmc;
>  	u32 ocr_mask;		/* available voltages */
>  
> +	bool			hw_timeout_disabled;
> +
>  	unsigned		timing;		/* Current timing */
>  
>  	u32			thread_isr;
> 

Re: [PATCH v2 11/16] mmc: sdhci: Program a relatively accurate SW timeout value

[Adrian Hunter]

On 05/02/18 14:50, Kishon Vijay Abraham I wrote:
> sdhci has a 10 second timeout to catch devices that stop responding.
> Instead of programming 10 second arbitrary value, calculate the total time
> it would take for the entire transfer to happen and program the timeout
> value accordingly.
> 
> Signed-off-by: Kishon Vijay Abraham I <kishon@ti.com>
> ---
>  drivers/mmc/host/sdhci.c | 46 +++++++++++++++++++++++++++++++++++++++-------
>  drivers/mmc/host/sdhci.h | 10 ++++++++++
>  2 files changed, 49 insertions(+), 7 deletions(-)
> 
> diff --git a/drivers/mmc/host/sdhci.c b/drivers/mmc/host/sdhci.c
> index 0489572d1892..d52f9e7eabe2 100644
> --- a/drivers/mmc/host/sdhci.c
> +++ b/drivers/mmc/host/sdhci.c
> @@ -673,6 +673,37 @@ static void sdhci_adma_table_post(struct sdhci_host *host,
>  	}
>  }
>  
> +static void sdhci_calc_sw_timeout(struct sdhci_host *host,
> +				  struct mmc_command *cmd,
> +				  unsigned int target_timeout)
> +{
> +	struct mmc_data *data = cmd->data;
> +	struct mmc_host *mmc = host->mmc;
> +	unsigned long long transfer_time;
> +	struct mmc_ios *ios = &mmc->ios;
> +	unsigned char bus_width = ios->bus_width;
> +	unsigned int blksz;
> +	unsigned int freq;
> +
> +	if (data) {
> +		blksz = data->blksz;
> +		freq = host->mmc->actual_clock ? host->mmc->actual_clock :
> +						host->clock;

I think this can be:

		freq = host->mmc->actual_clock ? : host->clock;

> +		transfer_time = (unsigned long long)(blksz * NSEC_PER_SEC *
> +						     (8 / bus_width)) / freq;

You have got a 32-bit overflow here and a 64-bit division that can't always
be done with '/'

> +		/* multiply by '2' to account for any unknowns */
> +		transfer_time = transfer_time * 2;

		transfer_time *= 2;

> +		/* calculate timeout for the entire data */
> +		host->data_timeout = (data->blocks * ((target_timeout *
> +						       NSEC_PER_USEC) +
> +						       transfer_time));
> +	} else {
> +		host->data_timeout = target_timeout * NSEC_PER_USEC;

And another 32-bit overflow here

> +	}
> +
> +	host->data_timeout += MMC_CMD_TRANSFER_TIME;
> +}
> +
>  static u8 sdhci_calc_timeout(struct sdhci_host *host, struct mmc_command *cmd)
>  {
>  	u8 count;
> @@ -742,6 +773,7 @@ static u8 sdhci_calc_timeout(struct sdhci_host *host, struct mmc_command *cmd)
>  			host->hw_timeout_disabled = true;
>  		}
>  	}
> +	sdhci_calc_sw_timeout(host, cmd, target_timeout);
>  
>  	return count;
>  }
> @@ -1130,13 +1162,6 @@ void sdhci_send_command(struct sdhci_host *host, struct mmc_command *cmd)
>  		mdelay(1);
>  	}
>  
> -	timeout = jiffies;
> -	if (!cmd->data && cmd->busy_timeout > 9000)
> -		timeout += DIV_ROUND_UP(cmd->busy_timeout, 1000) * HZ + HZ;
> -	else
> -		timeout += 10 * HZ;
> -	sdhci_mod_timer(host, cmd, timeout);
> -
>  	host->cmd = cmd;
>  	if (sdhci_data_line_cmd(cmd)) {
>  		WARN_ON(host->data_cmd);
> @@ -1176,6 +1201,13 @@ void sdhci_send_command(struct sdhci_host *host, struct mmc_command *cmd)
>  	    cmd->opcode == MMC_SEND_TUNING_BLOCK_HS200)
>  		flags |= SDHCI_CMD_DATA;
>  
> +	timeout = jiffies;
> +	if (sdhci_data_line_cmd(cmd))
> +		timeout += nsecs_to_jiffies(host->data_timeout);
> +	else
> +		timeout += 10 * HZ;
> +	sdhci_mod_timer(host, cmd, timeout);

Here you probably want to avoid updating the timer if the mrq has already
started using host->data_timeout.

> +
>  	sdhci_writew(host, SDHCI_MAKE_CMD(cmd->opcode, flags), SDHCI_COMMAND);
>  }
>  EXPORT_SYMBOL_GPL(sdhci_send_command);
> diff --git a/drivers/mmc/host/sdhci.h b/drivers/mmc/host/sdhci.h
> index 3a967a56fcc3..b73577d77856 100644
> --- a/drivers/mmc/host/sdhci.h
> +++ b/drivers/mmc/host/sdhci.h
> @@ -332,6 +332,14 @@ struct sdhci_adma2_64_desc {
>  /* Allow for a a command request and a data request at the same time */
>  #define SDHCI_MAX_MRQS		2
>  
> +/*
> + * 48bit command and 136 bit response in 400KHz clock should take 0.46ms.

I am not sure the math is correct here.  I would add the 64 clocks before
the response also, and allow for 100 kHz clock.

	(48 + 64 + 136) * 100 us = 24.8 ms

Given that you are looking at data block timeouts in excess of 700 ms, and
anything up to 10% of that is relatively negligible, anything less that 70
ms seems fine, so I would set 30 ms here as a round number.

> + * However since the start time of the command, the time between
> + * command and response, and the time between response and start of data is
> + * not known, set the command transfer time to 2ms.
> + */
> +#define MMC_CMD_TRANSFER_TIME	(2 * NSEC_PER_MSEC) /* max 2 ms */
> +
>  enum sdhci_cookie {
>  	COOKIE_UNMAPPED,
>  	COOKIE_PRE_MAPPED,	/* mapped by sdhci_pre_req() */
> @@ -554,6 +562,8 @@ struct sdhci_host {
>  	/* Host SDMA buffer boundary. */
>  	u32			sdma_boundary;
>  
> +	unsigned long long	data_timeout;

nsecs_to_jiffies() uses u64 which is nicer I think

> +
>  	unsigned long private[0] ____cacheline_aligned;
>  };
>  
> 

Re: [PATCH v2 15/16] mmc: sdhci-omap: Add SPDX identifier

[Adrian Hunter]

On 05/02/18 14:50, Kishon Vijay Abraham I wrote:
> The SPDX identifier is a legally binding shorthand, which can be used
> instead of the full boiler plate text. Update sdhci-omap driver with the
> correct SPDX license identifier.
> 
> Signed-off-by: Kishon Vijay Abraham I <kishon@ti.com>

Acked-by: Adrian Hunter <adrian.hunter@intel.com>

> ---
>  drivers/mmc/host/sdhci-omap.c | 22 ++++------------------
>  1 file changed, 4 insertions(+), 18 deletions(-)
> 
> diff --git a/drivers/mmc/host/sdhci-omap.c b/drivers/mmc/host/sdhci-omap.c
> index 863f6736c2fe..d44918172197 100644
> --- a/drivers/mmc/host/sdhci-omap.c
> +++ b/drivers/mmc/host/sdhci-omap.c
> @@ -1,21 +1,7 @@
> -/**
> - * SDHCI Controller driver for TI's OMAP SoCs
> - *
> - * Copyright (C) 2017 Texas Instruments
> - * Author: Kishon Vijay Abraham I <kishon@ti.com>
> - *
> - * This program is free software: you can redistribute it and/or modify
> - * it under the terms of the GNU General Public License version 2 of
> - * the License as published by the Free Software Foundation.
> - *
> - * 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.
> - *
> - * You should have received a copy of the GNU General Public License
> - * along with this program.  If not, see <http://www.gnu.org/licenses/>.
> - */
> +// SPDX-License-Identifier: GPL-2.0
> +// Copyright (c) 2018 Texas Instruments
> +// SDHCI Controller driver for TI's OMAP SoCs
> +// Author: Kishon Vijay Abraham I <kishon@ti.com>
>  
>  #include <linux/delay.h>
>  #include <linux/mmc/slot-gpio.h>
> 

Re: [PATCH v2 10/16] mmc: sdhci: Fix to use data_timer only for data line commands

[Kishon Vijay Abraham I]

Hi Adrian,

On Monday 19 February 2018 01:33 PM, Adrian Hunter wrote:
> On 05/02/18 14:50, Kishon Vijay Abraham I wrote:
>> commit d7422fb489eee5587d3eec ("mmc: sdhci: Separate timer timeout for
>> command and data requests") while separating timer timeout for
>> command and data requests, passed cmd->mrq to sdhci_mod_timer (cmd is an
>> argument to sdhci_send_command) and in sdhci_mod_timer used mrq->cmd
>> to check if it is a data line command. This results in using
>> data timer for commands like MMC_SET_BLOCK_COUNT (CMD23) though it is
>> not a data line command. Fix it here.
> 
> I am not sure why you need this change, but it is not right actually.  There

After adding below (similar to next patch)
+	timeout = jiffies;
+	if (sdhci_data_line_cmd(mrq->cmd))
+		timeout += nsecs_to_jiffies(host->data_timeout);
+	else
+		timeout += 10 * HZ;
+	sdhci_mod_timer(host, mrq->cmd, timeout);
+

I saw that host->data_timeout was having a value of '0'. And that's because for
set block count command, sdhci_data_line_cmd(mrq->cmd) evaluates to 'true'
though sdhci_prepare_data() doesn't set timeout.

> are 2 timers because there can be 2 mrqs and we need to delete the correct
> timer in sdhci_request_done() so it is better to make the selection based on
> the mrq not the last command.

okay, I have to change the mod_timer logic in sdhci_send_command().

Thanks
Kishon
> 
>>
>> Fixes: commit d7422fb489eee5587d3eec ("mmc: sdhci: Separate timer timeout for
>> command and data requests")
>>
>> Signed-off-by: Kishon Vijay Abraham I <kishon@ti.com>
>> ---
>>  drivers/mmc/host/sdhci.c | 6 +++---
>>  1 file changed, 3 insertions(+), 3 deletions(-)
>>
>> diff --git a/drivers/mmc/host/sdhci.c b/drivers/mmc/host/sdhci.c
>> index 1aa74b4682f3..0489572d1892 100644
>> --- a/drivers/mmc/host/sdhci.c
>> +++ b/drivers/mmc/host/sdhci.c
>> @@ -1073,10 +1073,10 @@ static void sdhci_finish_data(struct sdhci_host *host)
>>  	}
>>  }
>>  
>> -static void sdhci_mod_timer(struct sdhci_host *host, struct mmc_request *mrq,
>> +static void sdhci_mod_timer(struct sdhci_host *host, struct mmc_command *cmd,
>>  			    unsigned long timeout)
>>  {
>> -	if (sdhci_data_line_cmd(mrq->cmd))
>> +	if (sdhci_data_line_cmd(cmd))
>>  		mod_timer(&host->data_timer, timeout);
>>  	else
>>  		mod_timer(&host->timer, timeout);
>> @@ -1135,7 +1135,7 @@ void sdhci_send_command(struct sdhci_host *host, struct mmc_command *cmd)
>>  		timeout += DIV_ROUND_UP(cmd->busy_timeout, 1000) * HZ + HZ;
>>  	else
>>  		timeout += 10 * HZ;
>> -	sdhci_mod_timer(host, cmd->mrq, timeout);
>> +	sdhci_mod_timer(host, cmd, timeout);
>>  
>>  	host->cmd = cmd;
>>  	if (sdhci_data_line_cmd(cmd)) {
>>
> 

Re: [PATCH v2 11/16] mmc: sdhci: Program a relatively accurate SW timeout value

[Adrian Hunter]

On 19/02/18 11:24, Adrian Hunter wrote:
> On 05/02/18 14:50, Kishon Vijay Abraham I wrote:
>> sdhci has a 10 second timeout to catch devices that stop responding.
>> Instead of programming 10 second arbitrary value, calculate the total time
>> it would take for the entire transfer to happen and program the timeout
>> value accordingly.
>>
>> Signed-off-by: Kishon Vijay Abraham I <kishon@ti.com>
>> ---
>>  drivers/mmc/host/sdhci.c | 46 +++++++++++++++++++++++++++++++++++++++-------
>>  drivers/mmc/host/sdhci.h | 10 ++++++++++
>>  2 files changed, 49 insertions(+), 7 deletions(-)
>>
>> diff --git a/drivers/mmc/host/sdhci.c b/drivers/mmc/host/sdhci.c
>> index 0489572d1892..d52f9e7eabe2 100644
>> --- a/drivers/mmc/host/sdhci.c
>> +++ b/drivers/mmc/host/sdhci.c
>> @@ -673,6 +673,37 @@ static void sdhci_adma_table_post(struct sdhci_host *host,
>>  	}
>>  }
>>  
>> +static void sdhci_calc_sw_timeout(struct sdhci_host *host,
>> +				  struct mmc_command *cmd,
>> +				  unsigned int target_timeout)
>> +{
>> +	struct mmc_data *data = cmd->data;
>> +	struct mmc_host *mmc = host->mmc;
>> +	unsigned long long transfer_time;
>> +	struct mmc_ios *ios = &mmc->ios;
>> +	unsigned char bus_width = ios->bus_width;
>> +	unsigned int blksz;
>> +	unsigned int freq;
>> +
>> +	if (data) {
>> +		blksz = data->blksz;
>> +		freq = host->mmc->actual_clock ? host->mmc->actual_clock :
>> +						host->clock;
> 
> I think this can be:
> 
> 		freq = host->mmc->actual_clock ? : host->clock;
> 
>> +		transfer_time = (unsigned long long)(blksz * NSEC_PER_SEC *
>> +						     (8 / bus_width)) / freq;
> 
> You have got a 32-bit overflow here and a 64-bit division that can't always
> be done with '/'
> 
>> +		/* multiply by '2' to account for any unknowns */
>> +		transfer_time = transfer_time * 2;
> 
> 		transfer_time *= 2;
> 
>> +		/* calculate timeout for the entire data */
>> +		host->data_timeout = (data->blocks * ((target_timeout *
>> +						       NSEC_PER_USEC) +
>> +						       transfer_time));
>> +	} else {
>> +		host->data_timeout = target_timeout * NSEC_PER_USEC;
> 
> And another 32-bit overflow here
> 
>> +	}
>> +
>> +	host->data_timeout += MMC_CMD_TRANSFER_TIME;
>> +}
>> +
>>  static u8 sdhci_calc_timeout(struct sdhci_host *host, struct mmc_command *cmd)
>>  {
>>  	u8 count;
>> @@ -742,6 +773,7 @@ static u8 sdhci_calc_timeout(struct sdhci_host *host, struct mmc_command *cmd)
>>  			host->hw_timeout_disabled = true;
>>  		}
>>  	}
>> +	sdhci_calc_sw_timeout(host, cmd, target_timeout);
>>  
>>  	return count;
>>  }
>> @@ -1130,13 +1162,6 @@ void sdhci_send_command(struct sdhci_host *host, struct mmc_command *cmd)
>>  		mdelay(1);
>>  	}
>>  
>> -	timeout = jiffies;
>> -	if (!cmd->data && cmd->busy_timeout > 9000)
>> -		timeout += DIV_ROUND_UP(cmd->busy_timeout, 1000) * HZ + HZ;
>> -	else
>> -		timeout += 10 * HZ;
>> -	sdhci_mod_timer(host, cmd, timeout);
>> -
>>  	host->cmd = cmd;
>>  	if (sdhci_data_line_cmd(cmd)) {
>>  		WARN_ON(host->data_cmd);
>> @@ -1176,6 +1201,13 @@ void sdhci_send_command(struct sdhci_host *host, struct mmc_command *cmd)
>>  	    cmd->opcode == MMC_SEND_TUNING_BLOCK_HS200)
>>  		flags |= SDHCI_CMD_DATA;
>>  
>> +	timeout = jiffies;
>> +	if (sdhci_data_line_cmd(cmd))
>> +		timeout += nsecs_to_jiffies(host->data_timeout);
>> +	else
>> +		timeout += 10 * HZ;
>> +	sdhci_mod_timer(host, cmd, timeout);
> 
> Here you probably want to avoid updating the timer if the mrq has already
> started using host->data_timeout.
> 
>> +
>>  	sdhci_writew(host, SDHCI_MAKE_CMD(cmd->opcode, flags), SDHCI_COMMAND);
>>  }
>>  EXPORT_SYMBOL_GPL(sdhci_send_command);
>> diff --git a/drivers/mmc/host/sdhci.h b/drivers/mmc/host/sdhci.h
>> index 3a967a56fcc3..b73577d77856 100644
>> --- a/drivers/mmc/host/sdhci.h
>> +++ b/drivers/mmc/host/sdhci.h
>> @@ -332,6 +332,14 @@ struct sdhci_adma2_64_desc {
>>  /* Allow for a a command request and a data request at the same time */
>>  #define SDHCI_MAX_MRQS		2
>>  
>> +/*
>> + * 48bit command and 136 bit response in 400KHz clock should take 0.46ms.
> 
> I am not sure the math is correct here.  I would add the 64 clocks before
> the response also, and allow for 100 kHz clock.
> 
> 	(48 + 64 + 136) * 100 us = 24.8 ms

No I am off by x10, sorry!  I would still go for 10 ms.

> 
> Given that you are looking at data block timeouts in excess of 700 ms, and
> anything up to 10% of that is relatively negligible, anything less that 70
> ms seems fine, so I would set 30 ms here as a round number.
> 
>> + * However since the start time of the command, the time between
>> + * command and response, and the time between response and start of data is
>> + * not known, set the command transfer time to 2ms.
>> + */
>> +#define MMC_CMD_TRANSFER_TIME	(2 * NSEC_PER_MSEC) /* max 2 ms */
>> +
>>  enum sdhci_cookie {
>>  	COOKIE_UNMAPPED,
>>  	COOKIE_PRE_MAPPED,	/* mapped by sdhci_pre_req() */
>> @@ -554,6 +562,8 @@ struct sdhci_host {
>>  	/* Host SDMA buffer boundary. */
>>  	u32			sdma_boundary;
>>  
>> +	unsigned long long	data_timeout;
> 
> nsecs_to_jiffies() uses u64 which is nicer I think
> 
>> +
>>  	unsigned long private[0] ____cacheline_aligned;
>>  };
>>  
>>
> 
> 

Re: [PATCH v2 09/16] mmc: sdhci: Add quirk to disable HW timeout

[Kishon Vijay Abraham I]

Hi Adrian,

On Monday 19 February 2018 02:21 PM, Adrian Hunter wrote:
> On 05/02/18 14:50, Kishon Vijay Abraham I wrote:
>> Add quirk to disable HW timeout if the requested timeout is more than
>> the maximum obtainable timeout.
>>
>> Signed-off-by: Kishon Vijay Abraham I <kishon@ti.com>
>> ---
>>  drivers/mmc/host/sdhci.c | 12 ++++++++++++
>>  drivers/mmc/host/sdhci.h |  7 +++++++
>>  2 files changed, 19 insertions(+)
>>
>> diff --git a/drivers/mmc/host/sdhci.c b/drivers/mmc/host/sdhci.c
>> index 070aff9c108f..1aa74b4682f3 100644
>> --- a/drivers/mmc/host/sdhci.c
>> +++ b/drivers/mmc/host/sdhci.c
>> @@ -735,6 +735,12 @@ static u8 sdhci_calc_timeout(struct sdhci_host *host, struct mmc_command *cmd)
>>  		DBG("Too large timeout 0x%x requested for CMD%d!\n",
>>  		    count, cmd->opcode);
>>  		count = 0xE;
>> +		if (host->quirks2 & SDHCI_QUIRK2_DISABLE_HW_TIMEOUT) {
>> +			host->ier &= ~SDHCI_INT_DATA_TIMEOUT;
>> +			sdhci_writel(host, host->ier, SDHCI_INT_ENABLE);
>> +			sdhci_writel(host, host->ier, SDHCI_SIGNAL_ENABLE);
>> +			host->hw_timeout_disabled = true;
>> +		}
> 
> sdhci_calc_timeout() is really for calculations so please do this in
> sdhci_set_timeout() instead (i.e. change sdhci_calc_timeout() to return
> whether the timeout is too big).  Note that you need to cater for the "/*
> Unspecified timeout, assume max */" case and the
> SDHCI_QUIRK_BROKEN_TIMEOUT_VAL case.
> 
> Also if SDHCI_INT_DATA_TIMEOUT is not being disabled, check here to
> re-enable it and leave sdhci_request_done() alone. i.e.
> 
> 	else if (!(host->ier & SDHCI_INT_DATA_TIMEOUT)) {
> 		host->ier |= SDHCI_INT_DATA_TIMEOUT;
> 		sdhci_writel(host, host->ier, SDHCI_INT_ENABLE);
> 		sdhci_writel(host, host->ier, SDHCI_SIGNAL_ENABLE);
> 	}
> 
> Maybe make a separate subroutine for the update:
> 
> void sdhci_set_data_timeout_irq(struct sdhci_host *host, bool enable)
> {
> 	if (enable)
> 		host->ier |= SDHCI_INT_DATA_TIMEOUT;
> 	else
> 		host->ier &= ~SDHCI_INT_DATA_TIMEOUT;
> 	sdhci_writel(host, host->ier, SDHCI_INT_ENABLE);
> 	sdhci_writel(host, host->ier, SDHCI_SIGNAL_ENABLE);
> }
> 
> 
>>  	}
>>  
>>  	return count;
>> @@ -2349,6 +2355,12 @@ static bool sdhci_request_done(struct sdhci_host *host)
>>  	}
>>  
>>  	sdhci_del_timer(host, mrq);
>> +	if (sdhci_data_line_cmd(mrq->cmd) && host->hw_timeout_disabled) {
>> +		host->ier |= SDHCI_INT_DATA_TIMEOUT;
>> +		sdhci_writel(host, host->ier, SDHCI_INT_ENABLE);
>> +		sdhci_writel(host, host->ier, SDHCI_SIGNAL_ENABLE);
>> +		host->hw_timeout_disabled = false;
>> +	}
>>  
>>  	/*
>>  	 * Always unmap the data buffers if they were mapped by
>> diff --git a/drivers/mmc/host/sdhci.h b/drivers/mmc/host/sdhci.h
>> index afab26fd70e6..3a967a56fcc3 100644
>> --- a/drivers/mmc/host/sdhci.h
>> +++ b/drivers/mmc/host/sdhci.h
>> @@ -437,6 +437,11 @@ struct sdhci_host {
>>  #define SDHCI_QUIRK2_CLOCK_DIV_ZERO_BROKEN		(1<<15)
>>  /* Controller has CRC in 136 bit Command Response */
>>  #define SDHCI_QUIRK2_RSP_136_HAS_CRC			(1<<16)
>> +/*
>> + * Disable HW timeout if the requested timeout is more than the maximum
>> + * obtainable timeout
>> + */
>> +#define SDHCI_QUIRK2_DISABLE_HW_TIMEOUT			(1<<17)

Are you okay with the definition of this quirk? i.e this quirk is applied only
when the requested timeout is "more" than the maximum obtainable timeout.

By this way platforms can continue to use hardware timeout for smaller timeout
value.

Thanks
Kishon

Re: [PATCH v2 09/16] mmc: sdhci: Add quirk to disable HW timeout

[Adrian Hunter]

On 05/03/18 11:30, Kishon Vijay Abraham I wrote:
> Hi Adrian,
> 
> On Monday 19 February 2018 02:21 PM, Adrian Hunter wrote:
>> On 05/02/18 14:50, Kishon Vijay Abraham I wrote:
>>> Add quirk to disable HW timeout if the requested timeout is more than
>>> the maximum obtainable timeout.
>>>
>>> Signed-off-by: Kishon Vijay Abraham I <kishon@ti.com>
>>> ---
>>>  drivers/mmc/host/sdhci.c | 12 ++++++++++++
>>>  drivers/mmc/host/sdhci.h |  7 +++++++
>>>  2 files changed, 19 insertions(+)
>>>
>>> diff --git a/drivers/mmc/host/sdhci.c b/drivers/mmc/host/sdhci.c
>>> index 070aff9c108f..1aa74b4682f3 100644
>>> --- a/drivers/mmc/host/sdhci.c
>>> +++ b/drivers/mmc/host/sdhci.c
>>> @@ -735,6 +735,12 @@ static u8 sdhci_calc_timeout(struct sdhci_host *host, struct mmc_command *cmd)
>>>  		DBG("Too large timeout 0x%x requested for CMD%d!\n",
>>>  		    count, cmd->opcode);
>>>  		count = 0xE;
>>> +		if (host->quirks2 & SDHCI_QUIRK2_DISABLE_HW_TIMEOUT) {
>>> +			host->ier &= ~SDHCI_INT_DATA_TIMEOUT;
>>> +			sdhci_writel(host, host->ier, SDHCI_INT_ENABLE);
>>> +			sdhci_writel(host, host->ier, SDHCI_SIGNAL_ENABLE);
>>> +			host->hw_timeout_disabled = true;
>>> +		}
>>
>> sdhci_calc_timeout() is really for calculations so please do this in
>> sdhci_set_timeout() instead (i.e. change sdhci_calc_timeout() to return
>> whether the timeout is too big).  Note that you need to cater for the "/*
>> Unspecified timeout, assume max */" case and the
>> SDHCI_QUIRK_BROKEN_TIMEOUT_VAL case.
>>
>> Also if SDHCI_INT_DATA_TIMEOUT is not being disabled, check here to
>> re-enable it and leave sdhci_request_done() alone. i.e.
>>
>> 	else if (!(host->ier & SDHCI_INT_DATA_TIMEOUT)) {
>> 		host->ier |= SDHCI_INT_DATA_TIMEOUT;
>> 		sdhci_writel(host, host->ier, SDHCI_INT_ENABLE);
>> 		sdhci_writel(host, host->ier, SDHCI_SIGNAL_ENABLE);
>> 	}
>>
>> Maybe make a separate subroutine for the update:
>>
>> void sdhci_set_data_timeout_irq(struct sdhci_host *host, bool enable)
>> {
>> 	if (enable)
>> 		host->ier |= SDHCI_INT_DATA_TIMEOUT;
>> 	else
>> 		host->ier &= ~SDHCI_INT_DATA_TIMEOUT;
>> 	sdhci_writel(host, host->ier, SDHCI_INT_ENABLE);
>> 	sdhci_writel(host, host->ier, SDHCI_SIGNAL_ENABLE);
>> }
>>
>>
>>>  	}
>>>  
>>>  	return count;
>>> @@ -2349,6 +2355,12 @@ static bool sdhci_request_done(struct sdhci_host *host)
>>>  	}
>>>  
>>>  	sdhci_del_timer(host, mrq);
>>> +	if (sdhci_data_line_cmd(mrq->cmd) && host->hw_timeout_disabled) {
>>> +		host->ier |= SDHCI_INT_DATA_TIMEOUT;
>>> +		sdhci_writel(host, host->ier, SDHCI_INT_ENABLE);
>>> +		sdhci_writel(host, host->ier, SDHCI_SIGNAL_ENABLE);
>>> +		host->hw_timeout_disabled = false;
>>> +	}
>>>  
>>>  	/*
>>>  	 * Always unmap the data buffers if they were mapped by
>>> diff --git a/drivers/mmc/host/sdhci.h b/drivers/mmc/host/sdhci.h
>>> index afab26fd70e6..3a967a56fcc3 100644
>>> --- a/drivers/mmc/host/sdhci.h
>>> +++ b/drivers/mmc/host/sdhci.h
>>> @@ -437,6 +437,11 @@ struct sdhci_host {
>>>  #define SDHCI_QUIRK2_CLOCK_DIV_ZERO_BROKEN		(1<<15)
>>>  /* Controller has CRC in 136 bit Command Response */
>>>  #define SDHCI_QUIRK2_RSP_136_HAS_CRC			(1<<16)
>>> +/*
>>> + * Disable HW timeout if the requested timeout is more than the maximum
>>> + * obtainable timeout
>>> + */
>>> +#define SDHCI_QUIRK2_DISABLE_HW_TIMEOUT			(1<<17)
> 
> Are you okay with the definition of this quirk? i.e this quirk is applied only
> when the requested timeout is "more" than the maximum obtainable timeout.
> 
> By this way platforms can continue to use hardware timeout for smaller timeout
> value.

It is fine to me.

Re: [PATCH v2 09/16] mmc: sdhci: Add quirk to disable HW timeout

[Kishon Vijay Abraham I]

On Monday 05 March 2018 03:08 PM, Adrian Hunter wrote:
> On 05/03/18 11:30, Kishon Vijay Abraham I wrote:
>> Hi Adrian,
>>
>> On Monday 19 February 2018 02:21 PM, Adrian Hunter wrote:
>>> On 05/02/18 14:50, Kishon Vijay Abraham I wrote:
>>>> Add quirk to disable HW timeout if the requested timeout is more than
>>>> the maximum obtainable timeout.
>>>>
>>>> Signed-off-by: Kishon Vijay Abraham I <kishon@ti.com>
>>>> ---
>>>>  drivers/mmc/host/sdhci.c | 12 ++++++++++++
>>>>  drivers/mmc/host/sdhci.h |  7 +++++++
>>>>  2 files changed, 19 insertions(+)
>>>>
>>>> diff --git a/drivers/mmc/host/sdhci.c b/drivers/mmc/host/sdhci.c
>>>> index 070aff9c108f..1aa74b4682f3 100644
>>>> --- a/drivers/mmc/host/sdhci.c
>>>> +++ b/drivers/mmc/host/sdhci.c
>>>> @@ -735,6 +735,12 @@ static u8 sdhci_calc_timeout(struct sdhci_host *host, struct mmc_command *cmd)
>>>>  		DBG("Too large timeout 0x%x requested for CMD%d!\n",
>>>>  		    count, cmd->opcode);
>>>>  		count = 0xE;
>>>> +		if (host->quirks2 & SDHCI_QUIRK2_DISABLE_HW_TIMEOUT) {
>>>> +			host->ier &= ~SDHCI_INT_DATA_TIMEOUT;
>>>> +			sdhci_writel(host, host->ier, SDHCI_INT_ENABLE);
>>>> +			sdhci_writel(host, host->ier, SDHCI_SIGNAL_ENABLE);
>>>> +			host->hw_timeout_disabled = true;
>>>> +		}
>>>
>>> sdhci_calc_timeout() is really for calculations so please do this in
>>> sdhci_set_timeout() instead (i.e. change sdhci_calc_timeout() to return
>>> whether the timeout is too big).  Note that you need to cater for the "/*
>>> Unspecified timeout, assume max */" case and the
>>> SDHCI_QUIRK_BROKEN_TIMEOUT_VAL case.
>>>
>>> Also if SDHCI_INT_DATA_TIMEOUT is not being disabled, check here to
>>> re-enable it and leave sdhci_request_done() alone. i.e.
>>>
>>> 	else if (!(host->ier & SDHCI_INT_DATA_TIMEOUT)) {
>>> 		host->ier |= SDHCI_INT_DATA_TIMEOUT;
>>> 		sdhci_writel(host, host->ier, SDHCI_INT_ENABLE);
>>> 		sdhci_writel(host, host->ier, SDHCI_SIGNAL_ENABLE);
>>> 	}
>>>
>>> Maybe make a separate subroutine for the update:
>>>
>>> void sdhci_set_data_timeout_irq(struct sdhci_host *host, bool enable)
>>> {
>>> 	if (enable)
>>> 		host->ier |= SDHCI_INT_DATA_TIMEOUT;
>>> 	else
>>> 		host->ier &= ~SDHCI_INT_DATA_TIMEOUT;
>>> 	sdhci_writel(host, host->ier, SDHCI_INT_ENABLE);
>>> 	sdhci_writel(host, host->ier, SDHCI_SIGNAL_ENABLE);
>>> }
>>>
>>>
>>>>  	}
>>>>  
>>>>  	return count;
>>>> @@ -2349,6 +2355,12 @@ static bool sdhci_request_done(struct sdhci_host *host)
>>>>  	}
>>>>  
>>>>  	sdhci_del_timer(host, mrq);
>>>> +	if (sdhci_data_line_cmd(mrq->cmd) && host->hw_timeout_disabled) {
>>>> +		host->ier |= SDHCI_INT_DATA_TIMEOUT;
>>>> +		sdhci_writel(host, host->ier, SDHCI_INT_ENABLE);
>>>> +		sdhci_writel(host, host->ier, SDHCI_SIGNAL_ENABLE);
>>>> +		host->hw_timeout_disabled = false;
>>>> +	}
>>>>  
>>>>  	/*
>>>>  	 * Always unmap the data buffers if they were mapped by
>>>> diff --git a/drivers/mmc/host/sdhci.h b/drivers/mmc/host/sdhci.h
>>>> index afab26fd70e6..3a967a56fcc3 100644
>>>> --- a/drivers/mmc/host/sdhci.h
>>>> +++ b/drivers/mmc/host/sdhci.h
>>>> @@ -437,6 +437,11 @@ struct sdhci_host {
>>>>  #define SDHCI_QUIRK2_CLOCK_DIV_ZERO_BROKEN		(1<<15)
>>>>  /* Controller has CRC in 136 bit Command Response */
>>>>  #define SDHCI_QUIRK2_RSP_136_HAS_CRC			(1<<16)
>>>> +/*
>>>> + * Disable HW timeout if the requested timeout is more than the maximum
>>>> + * obtainable timeout
>>>> + */
>>>> +#define SDHCI_QUIRK2_DISABLE_HW_TIMEOUT			(1<<17)
>>
>> Are you okay with the definition of this quirk? i.e this quirk is applied only
>> when the requested timeout is "more" than the maximum obtainable timeout.
>>
>> By this way platforms can continue to use hardware timeout for smaller timeout
>> value.
> 
> It is fine to me.
> 
Okay thanks. I've posted v3 sometime last week. I'm trying to get this in for
4.17? Can you review the new revision please?

Thanks
Kishon