devicetree.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [PATCH 00/12] mmc: sdhci-omap: Add UHS/HS200 mode support
@ 2017-12-14 13:09 Kishon Vijay Abraham I
  2017-12-14 13:09 ` [PATCH 01/12] mmc: sdhci-omap: Update 'power_mode' outside sdhci_omap_init_74_clocks Kishon Vijay Abraham I
                   ` (11 more replies)
  0 siblings, 12 replies; 28+ messages in thread
From: Kishon Vijay Abraham I @ 2017-12-14 13:09 UTC (permalink / raw)
  To: Ulf Hansson, Rob Herring, Tony Lindgren, Adrian Hunter
  Cc: Mark Rutland, Russell King, linux-mmc, devicetree, linux-kernel,
	linux-omap, linux-arm-kernel, nsekhar, kishon

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.

While most of this series is specific to sdhci-omap, it also
patches sdhci to use software timer when the requested timeout
is greater than hardware capablility. This re-uses the SW data timer
already implemented in sdhci while disabling the HW timeout (so that
spurious timeout is not observed). The patch for sdhci.c is based on
an earlier patch that was done specific to omap_hsmmc.c
(https://patchwork.kernel.org/patch/9791449/)

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 be follow this patch series.

This series is created on
git://git.kernel.org/pub/scm/linux/kernel/git/ulfh/mmc.git next

Kishon Vijay Abraham I (12):
  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: Use software timer when timeout greater than hardware
    capablility
  dt-bindings: sdhci-omap: Add K2G specific binding
  mmc: sdhci-omap: Add support for MMC/SD controller in k2g SoC
  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                      | 446 ++++++++++++++++++++-
 drivers/mmc/host/sdhci.c                           |  41 +-
 drivers/mmc/host/sdhci.h                           |  11 +
 include/linux/platform_data/sdhci-omap.h           |  35 ++
 6 files changed, 544 insertions(+), 25 deletions(-)
 create mode 100644 include/linux/platform_data/sdhci-omap.h

-- 
2.11.0

^ permalink raw reply	[flat|nested] 28+ messages in thread

* [PATCH 01/12] mmc: sdhci-omap: Update 'power_mode' outside sdhci_omap_init_74_clocks
  2017-12-14 13:09 [PATCH 00/12] mmc: sdhci-omap: Add UHS/HS200 mode support Kishon Vijay Abraham I
@ 2017-12-14 13:09 ` Kishon Vijay Abraham I
  2017-12-21  8:57   ` Adrian Hunter
  2017-12-14 13:09 ` [PATCH 02/12] mmc: sdhci-omap: Add card_busy host ops Kishon Vijay Abraham I
                   ` (10 subsequent siblings)
  11 siblings, 1 reply; 28+ messages in thread
From: Kishon Vijay Abraham I @ 2017-12-14 13:09 UTC (permalink / raw)
  To: Ulf Hansson, Rob Herring, Tony Lindgren, Adrian Hunter
  Cc: Mark Rutland, devicetree, kishon, nsekhar, linux-mmc,
	Russell King, linux-kernel, linux-omap, linux-arm-kernel

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

^ permalink raw reply related	[flat|nested] 28+ messages in thread

* [PATCH 02/12] mmc: sdhci-omap: Add card_busy host ops
  2017-12-14 13:09 [PATCH 00/12] mmc: sdhci-omap: Add UHS/HS200 mode support Kishon Vijay Abraham I
  2017-12-14 13:09 ` [PATCH 01/12] mmc: sdhci-omap: Update 'power_mode' outside sdhci_omap_init_74_clocks Kishon Vijay Abraham I
@ 2017-12-14 13:09 ` Kishon Vijay Abraham I
  2017-12-21  8:59   ` Adrian Hunter
  2017-12-14 13:09 ` [PATCH 03/12] mmc: sdhci-omap: Add custom set_uhs_signaling sdhci_host ops Kishon Vijay Abraham I
                   ` (9 subsequent siblings)
  11 siblings, 1 reply; 28+ messages in thread
From: Kishon Vijay Abraham I @ 2017-12-14 13:09 UTC (permalink / raw)
  To: Ulf Hansson, Rob Herring, Tony Lindgren, Adrian Hunter
  Cc: Mark Rutland, Russell King, linux-mmc, devicetree, linux-kernel,
	linux-omap, linux-arm-kernel, nsekhar, kishon

card_busy ops is used by mmc core in
  1) mmc_set_uhs_voltage to verify voltage switch
  2) __mmc_start_request/mmc_poll_for_busy to check the card busy status

While only DAT0 can be used to check the card busy status (in '2' above),
CMD and DAT[0..3] is used to verify voltage switch (in '1' above).

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]).

Add card_busy host ops in sdhci_omap that checks for both CMD and
DAT[0..3]. card_busy here returns true if one of CMD and DAT[0..3] is
low though during voltage switch sequence all of CMD and DAT[0..3] has
to be low (however haven't observed a case where some DAT lines are low
and some are high).

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 | 62 +++++++++++++++++++++++++++++++++++++++++++
 1 file changed, 62 insertions(+)

diff --git a/drivers/mmc/host/sdhci-omap.c b/drivers/mmc/host/sdhci-omap.c
index 96985786cadf..defe4eac020d 100644
--- a/drivers/mmc/host/sdhci-omap.c
+++ b/drivers/mmc/host/sdhci-omap.c
@@ -31,11 +31,20 @@
 #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_CLEV             BIT(24)
+#define PSTATE_DLEV_SHIFT	20
+#define PSTATE_DLEV_DAT(x)	(1 << (PSTATE_DLEV_SHIFT + (x)))
+#define PSTATE_DLEV		(PSTATE_DLEV_DAT(0) | PSTATE_DLEV_DAT(1) | \
+					PSTATE_DLEV_DAT(2) | PSTATE_DLEV_DAT(3))
+
 #define SDHCI_OMAP_HCTL		0x228
 #define HCTL_SDBP		BIT(8)
 #define HCTL_SDVS_SHIFT		9
@@ -191,6 +200,58 @@ static void sdhci_omap_conf_bus_power(struct sdhci_omap_host *omap_host,
 	}
 }
 
+static int sdhci_omap_card_busy(struct mmc_host *mmc)
+{
+	int i;
+	u32 reg, ac12;
+	int ret = true;
+	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);
+
+	for (i = 0; i < 5; i++) {
+		/*
+		 * Delay is required for PSTATE to correctly reflect
+		 * DLEV/CLEV values after PADEM is set.
+		 */
+		usleep_range(100, 200);
+		reg = sdhci_omap_readl(omap_host, SDHCI_OMAP_PSTATE);
+		if ((reg & PSTATE_CLEV) &&
+		    ((reg & PSTATE_DLEV) == PSTATE_DLEV)) {
+			ret = false;
+			goto ret;
+		}
+	}
+
+ret:
+	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 +623,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

^ permalink raw reply related	[flat|nested] 28+ messages in thread

* [PATCH 03/12] mmc: sdhci-omap: Add custom set_uhs_signaling sdhci_host ops
  2017-12-14 13:09 [PATCH 00/12] mmc: sdhci-omap: Add UHS/HS200 mode support Kishon Vijay Abraham I
  2017-12-14 13:09 ` [PATCH 01/12] mmc: sdhci-omap: Update 'power_mode' outside sdhci_omap_init_74_clocks Kishon Vijay Abraham I
  2017-12-14 13:09 ` [PATCH 02/12] mmc: sdhci-omap: Add card_busy host ops Kishon Vijay Abraham I
@ 2017-12-14 13:09 ` Kishon Vijay Abraham I
  2017-12-21  9:01   ` Adrian Hunter
  2017-12-14 13:09 ` [PATCH 04/12] mmc: sdhci-omap: Add tuning support Kishon Vijay Abraham I
                   ` (8 subsequent siblings)
  11 siblings, 1 reply; 28+ messages in thread
From: Kishon Vijay Abraham I @ 2017-12-14 13:09 UTC (permalink / raw)
  To: Ulf Hansson, Rob Herring, Tony Lindgren, Adrian Hunter
  Cc: Mark Rutland, Russell King, linux-mmc, devicetree, linux-kernel,
	linux-omap, linux-arm-kernel, nsekhar, kishon

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>
---
 drivers/mmc/host/sdhci-omap.c | 26 +++++++++++++++++++++++++-
 1 file changed, 25 insertions(+), 1 deletion(-)

diff --git a/drivers/mmc/host/sdhci-omap.c b/drivers/mmc/host/sdhci-omap.c
index defe4eac020d..8f7239e2edc2 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)
@@ -93,6 +94,9 @@ struct sdhci_omap_host {
 	u8			power_mode;
 };
 
+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)
 {
@@ -471,6 +475,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,
@@ -480,7 +504,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


^ permalink raw reply related	[flat|nested] 28+ messages in thread

* [PATCH 04/12] mmc: sdhci-omap: Add tuning support
  2017-12-14 13:09 [PATCH 00/12] mmc: sdhci-omap: Add UHS/HS200 mode support Kishon Vijay Abraham I
                   ` (2 preceding siblings ...)
  2017-12-14 13:09 ` [PATCH 03/12] mmc: sdhci-omap: Add custom set_uhs_signaling sdhci_host ops Kishon Vijay Abraham I
@ 2017-12-14 13:09 ` Kishon Vijay Abraham I
  2017-12-21  9:09   ` Adrian Hunter
       [not found] ` <20171214130941.26666-1-kishon-l0cyMroinI0@public.gmane.org>
                   ` (7 subsequent siblings)
  11 siblings, 1 reply; 28+ messages in thread
From: Kishon Vijay Abraham I @ 2017-12-14 13:09 UTC (permalink / raw)
  To: Ulf Hansson, Rob Herring, Tony Lindgren, Adrian Hunter
  Cc: Mark Rutland, Russell King, linux-mmc, devicetree, linux-kernel,
	linux-omap, linux-arm-kernel, nsekhar, kishon

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>
---
 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 8f7239e2edc2..df8a0a472996 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
@@ -66,12 +73,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
@@ -80,6 +91,8 @@
 #define IOV_3V0			3000000		/* 300000 uV */
 #define IOV_3V3			3300000		/* 330000 uV */
 
+#define MAX_PHASE_DELAY		0x7C
+
 struct sdhci_omap_data {
 	u32 offset;
 };
@@ -204,6 +217,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)
+{
+	u32 reg;
+	int ret = 0;
+	u8 cur_match, prev_match = 0;
+	u32 phase_delay = 0;
+	u32 start_window = 0, max_window = 0;
+	u32 length = 0, max_len = 0;
+	struct mmc_ios *ios = &mmc->ios;
+	struct sdhci_host *host = mmc_priv(mmc);
+	struct sdhci_pltfm_host *pltfm_host;
+	struct sdhci_omap_host *omap_host;
+	struct device *dev;
+
+	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)
 {
 	int i;
@@ -312,6 +439,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;
 }
 
@@ -648,6 +777,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


^ permalink raw reply related	[flat|nested] 28+ messages in thread

* [PATCH 05/12] mmc: sdhci-omap: Workaround for Errata i802
       [not found] ` <20171214130941.26666-1-kishon-l0cyMroinI0@public.gmane.org>
@ 2017-12-14 13:09   ` Kishon Vijay Abraham I
  2017-12-21  9:09     ` Adrian Hunter
  0 siblings, 1 reply; 28+ messages in thread
From: Kishon Vijay Abraham I @ 2017-12-14 13:09 UTC (permalink / raw)
  To: Ulf Hansson, Rob Herring, Tony Lindgren, Adrian Hunter
  Cc: Mark Rutland, Russell King, linux-mmc-u79uwXL29TY76Z2rM5mHXA,
	devicetree-u79uwXL29TY76Z2rM5mHXA,
	linux-kernel-u79uwXL29TY76Z2rM5mHXA,
	linux-omap-u79uwXL29TY76Z2rM5mHXA,
	linux-arm-kernel-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r,
	nsekhar-l0cyMroinI0, kishon-l0cyMroinI0

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-l0cyMroinI0@public.gmane.org>
---
 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 df8a0a472996..b20f4c79ccc6 100644
--- a/drivers/mmc/host/sdhci-omap.c
+++ b/drivers/mmc/host/sdhci-omap.c
@@ -266,6 +266,7 @@ static int sdhci_omap_execute_tuning(struct mmc_host *mmc, u32 opcode)
 	struct sdhci_pltfm_host *pltfm_host;
 	struct sdhci_omap_host *omap_host;
 	struct device *dev;
+	u32 ier = host->ier;
 
 	pltfm_host = sdhci_priv(host);
 	omap_host = sdhci_pltfm_priv(pltfm_host);
@@ -283,6 +284,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);
 
@@ -328,6 +339,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

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

^ permalink raw reply related	[flat|nested] 28+ messages in thread

* [PATCH 06/12] mmc: sdhci_omap: Add support to set IODELAY values
  2017-12-14 13:09 [PATCH 00/12] mmc: sdhci-omap: Add UHS/HS200 mode support Kishon Vijay Abraham I
                   ` (4 preceding siblings ...)
       [not found] ` <20171214130941.26666-1-kishon-l0cyMroinI0@public.gmane.org>
@ 2017-12-14 13:09 ` Kishon Vijay Abraham I
       [not found]   ` <20171214130941.26666-7-kishon-l0cyMroinI0@public.gmane.org>
  2017-12-14 13:09 ` [PATCH 07/12] mmc: sdhci_omap: Fix sdhci-omap quirks Kishon Vijay Abraham I
                   ` (5 subsequent siblings)
  11 siblings, 1 reply; 28+ messages in thread
From: Kishon Vijay Abraham I @ 2017-12-14 13:09 UTC (permalink / raw)
  To: Ulf Hansson, Rob Herring, Tony Lindgren, Adrian Hunter
  Cc: Mark Rutland, Russell King, linux-mmc, devicetree, linux-kernel,
	linux-omap, linux-arm-kernel, nsekhar, kishon

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 | 174 ++++++++++++++++++++++++++++++++++++++++++
 1 file changed, 174 insertions(+)

diff --git a/drivers/mmc/host/sdhci-omap.c b/drivers/mmc/host/sdhci-omap.c
index b20f4c79ccc6..594e41200d8a 100644
--- a/drivers/mmc/host/sdhci-omap.c
+++ b/drivers/mmc/host/sdhci-omap.c
@@ -93,8 +93,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 {
@@ -105,6 +109,20 @@ 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;
+	struct pinctrl_state	*default_pinctrl_state;
+	struct pinctrl_state	*sdr104_pinctrl_state;
+	struct pinctrl_state	*hs200_1_8v_pinctrl_state;
+	struct pinctrl_state	*ddr50_pinctrl_state;
+	struct pinctrl_state	*sdr50_pinctrl_state;
+	struct pinctrl_state	*sdr25_pinctrl_state;
+	struct pinctrl_state	*sdr12_pinctrl_state;
+	struct pinctrl_state	*hs_pinctrl_state;
+	struct pinctrl_state	*ddr_1_8v_pinctrl_state;
 };
 
 static void sdhci_omap_start_clock(struct sdhci_omap_host *omap_host);
@@ -449,6 +467,62 @@ 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->timing == timing)
+		return;
+
+	sdhci_omap_stop_clock(omap_host);
+
+	switch (timing) {
+	case MMC_TIMING_UHS_SDR104:
+		pinctrl_state = omap_host->sdr104_pinctrl_state;
+		break;
+	case MMC_TIMING_MMC_HS200:
+		pinctrl_state = omap_host->hs200_1_8v_pinctrl_state;
+		break;
+	case MMC_TIMING_UHS_DDR50:
+		pinctrl_state = omap_host->ddr50_pinctrl_state;
+		break;
+	case MMC_TIMING_UHS_SDR50:
+		pinctrl_state = omap_host->sdr50_pinctrl_state;
+		break;
+	case MMC_TIMING_UHS_SDR25:
+		pinctrl_state = omap_host->sdr25_pinctrl_state;
+		break;
+	case MMC_TIMING_UHS_SDR12:
+		pinctrl_state = omap_host->sdr12_pinctrl_state;
+		break;
+	case MMC_TIMING_SD_HS:
+	case MMC_TIMING_MMC_HS:
+		pinctrl_state = omap_host->hs_pinctrl_state;
+		break;
+	case MMC_TIMING_MMC_DDR52:
+		pinctrl_state = omap_host->ddr_1_8v_pinctrl_state;
+		break;
+	default:
+		pinctrl_state = omap_host->default_pinctrl_state;
+		break;
+	}
+
+	if (omap_host->flags & SDHCI_OMAP_REQUIRE_IODELAY) {
+		ret = pinctrl_select_state(omap_host->pinctrl, pinctrl_state);
+		if (ret) {
+			dev_err(dev, "failed to select pinctrl state\n");
+			goto ret;
+		}
+		omap_host->pinctrl_state = pinctrl_state;
+	}
+
+ret:
+	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)
 {
@@ -485,6 +559,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);
 }
@@ -693,6 +768,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[] = {
@@ -701,6 +777,98 @@ 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;
+
+	if (!(omap_host->flags & SDHCI_OMAP_REQUIRE_IODELAY))
+		return 0;
+
+	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);
+	}
+	omap_host->default_pinctrl_state = state;
+
+	state = sdhci_omap_iodelay_pinctrl_state(omap_host, "sdr104", caps,
+						 MMC_CAP_UHS_SDR104);
+	if (!IS_ERR(state))
+		omap_host->sdr104_pinctrl_state = state;
+
+	state = sdhci_omap_iodelay_pinctrl_state(omap_host, "ddr50", caps,
+						 MMC_CAP_UHS_DDR50);
+	if (!IS_ERR(state))
+		omap_host->ddr50_pinctrl_state = state;
+
+	state = sdhci_omap_iodelay_pinctrl_state(omap_host, "sdr50", caps,
+						 MMC_CAP_UHS_SDR50);
+	if (!IS_ERR(state))
+		omap_host->sdr50_pinctrl_state = state;
+
+	state = sdhci_omap_iodelay_pinctrl_state(omap_host, "sdr25", caps,
+						 MMC_CAP_UHS_SDR25);
+	if (!IS_ERR(state))
+		omap_host->sdr25_pinctrl_state = state;
+
+	state = sdhci_omap_iodelay_pinctrl_state(omap_host, "sdr12", caps,
+						 MMC_CAP_UHS_SDR12);
+	if (!IS_ERR(state))
+		omap_host->sdr12_pinctrl_state = state;
+
+	state = sdhci_omap_iodelay_pinctrl_state(omap_host, "ddr_1_8v", caps,
+						 MMC_CAP_1_8V_DDR);
+	if (!IS_ERR(state))
+		omap_host->ddr_1_8v_pinctrl_state = state;
+
+	state = sdhci_omap_iodelay_pinctrl_state(omap_host, "hs", caps,
+						 MMC_CAP_MMC_HIGHSPEED |
+						 MMC_CAP_SD_HIGHSPEED);
+	if (!IS_ERR(state))
+		omap_host->hs_pinctrl_state = state;
+
+	state = sdhci_omap_iodelay_pinctrl_state(omap_host, "hs200_1_8v", caps2,
+						 MMC_CAP2_HS200_1_8V_SDR);
+	if (!IS_ERR(state))
+		omap_host->hs200_1_8v_pinctrl_state = state;
+
+	omap_host->pinctrl_state = omap_host->default_pinctrl_state;
+
+	return 0;
+}
+
 static int sdhci_omap_probe(struct platform_device *pdev)
 {
 	int ret;
@@ -737,6 +905,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;
@@ -785,6 +955,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

^ permalink raw reply related	[flat|nested] 28+ messages in thread

* [PATCH 07/12] mmc: sdhci_omap: Fix sdhci-omap quirks
  2017-12-14 13:09 [PATCH 00/12] mmc: sdhci-omap: Add UHS/HS200 mode support Kishon Vijay Abraham I
                   ` (5 preceding siblings ...)
  2017-12-14 13:09 ` [PATCH 06/12] mmc: sdhci_omap: Add support to set IODELAY values Kishon Vijay Abraham I
@ 2017-12-14 13:09 ` Kishon Vijay Abraham I
       [not found]   ` <20171214130941.26666-8-kishon-l0cyMroinI0@public.gmane.org>
  2017-12-14 13:09 ` [PATCH 08/12] mmc: sdhci-omap: Add support to override f_max and iodelay from pdata Kishon Vijay Abraham I
                   ` (4 subsequent siblings)
  11 siblings, 1 reply; 28+ messages in thread
From: Kishon Vijay Abraham I @ 2017-12-14 13:09 UTC (permalink / raw)
  To: Ulf Hansson, Rob Herring, Tony Lindgren, Adrian Hunter
  Cc: Mark Rutland, Russell King, linux-mmc, devicetree, linux-kernel,
	linux-omap, linux-arm-kernel, nsekhar, kishon

Remove SDHCI_QUIRK_BROKEN_CARD_DETECTION quirk as gpio card detection
is supported in sdhci-omap.

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 | 7 +++----
 1 file changed, 3 insertions(+), 4 deletions(-)

diff --git a/drivers/mmc/host/sdhci-omap.c b/drivers/mmc/host/sdhci-omap.c
index 594e41200d8a..6dee275b2e57 100644
--- a/drivers/mmc/host/sdhci-omap.c
+++ b/drivers/mmc/host/sdhci-omap.c
@@ -755,13 +755,12 @@ static int sdhci_omap_set_capabilities(struct sdhci_omap_host *omap_host)
 }
 
 static const struct sdhci_pltfm_data sdhci_omap_pdata = {
-	.quirks = SDHCI_QUIRK_BROKEN_CARD_DETECTION |
-		  SDHCI_QUIRK_DATA_TIMEOUT_USES_SDCLK |
+	.quirks = SDHCI_QUIRK_DATA_TIMEOUT_USES_SDCLK |
 		  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

^ permalink raw reply related	[flat|nested] 28+ messages in thread

* [PATCH 08/12] mmc: sdhci-omap: Add support to override f_max and iodelay from pdata
  2017-12-14 13:09 [PATCH 00/12] mmc: sdhci-omap: Add UHS/HS200 mode support Kishon Vijay Abraham I
                   ` (6 preceding siblings ...)
  2017-12-14 13:09 ` [PATCH 07/12] mmc: sdhci_omap: Fix sdhci-omap quirks Kishon Vijay Abraham I
@ 2017-12-14 13:09 ` Kishon Vijay Abraham I
  2017-12-14 14:04   ` Philippe Ombredanne
       [not found]   ` <20171214130941.26666-9-kishon-l0cyMroinI0@public.gmane.org>
  2017-12-14 13:09 ` [RFC PATCH 09/12] mmc: sdhci: Use software timer when timeout greater than hardware capablility Kishon Vijay Abraham I
                   ` (3 subsequent siblings)
  11 siblings, 2 replies; 28+ messages in thread
From: Kishon Vijay Abraham I @ 2017-12-14 13:09 UTC (permalink / raw)
  To: Ulf Hansson, Rob Herring, Tony Lindgren, Adrian Hunter
  Cc: Mark Rutland, devicetree, kishon, nsekhar, linux-mmc,
	Russell King, linux-kernel, linux-omap, linux-arm-kernel

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            | 17 ++++++++++++++++
 include/linux/platform_data/sdhci-omap.h | 35 ++++++++++++++++++++++++++++++++
 2 files changed, 52 insertions(+)
 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 6dee275b2e57..cddc3ad1331f 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>
@@ -102,6 +103,7 @@ struct sdhci_omap_data {
 };
 
 struct sdhci_omap_host {
+	char			*version;
 	void __iomem		*base;
 	struct device		*dev;
 	struct	regulator	*pbias;
@@ -781,11 +783,18 @@ 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;
 
+	if (version) {
+		sprintf(str, "%s-%s", mode, version);
+		pinctrl_state = pinctrl_lookup_state(omap_host->pinctrl, str);
+	}
+
 	pinctrl_state = pinctrl_lookup_state(omap_host->pinctrl, mode);
 	if (IS_ERR(pinctrl_state)) {
 		dev_err(dev, "no pinctrl state for %s mode", mode);
@@ -879,6 +888,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)
@@ -913,6 +923,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..a46e1240956a
--- /dev/null
+++ b/include/linux/platform_data/sdhci-omap.h
@@ -0,0 +1,35 @@
+/**
+ * SDHCI Controller Platform Data 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/>.
+ */
+#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

^ permalink raw reply related	[flat|nested] 28+ messages in thread

* [RFC PATCH 09/12] mmc: sdhci: Use software timer when timeout greater than hardware capablility
  2017-12-14 13:09 [PATCH 00/12] mmc: sdhci-omap: Add UHS/HS200 mode support Kishon Vijay Abraham I
                   ` (7 preceding siblings ...)
  2017-12-14 13:09 ` [PATCH 08/12] mmc: sdhci-omap: Add support to override f_max and iodelay from pdata Kishon Vijay Abraham I
@ 2017-12-14 13:09 ` Kishon Vijay Abraham I
  2017-12-20 14:11   ` Adrian Hunter
  2017-12-14 13:09 ` [PATCH 10/12] dt-bindings: sdhci-omap: Add K2G specific binding Kishon Vijay Abraham I
                   ` (2 subsequent siblings)
  11 siblings, 1 reply; 28+ messages in thread
From: Kishon Vijay Abraham I @ 2017-12-14 13:09 UTC (permalink / raw)
  To: Ulf Hansson, Rob Herring, Tony Lindgren, Adrian Hunter
  Cc: Mark Rutland, Russell King, linux-mmc, devicetree, linux-kernel,
	linux-omap, linux-arm-kernel, nsekhar, kishon

Errata i834 in AM572x Sitara Processors Silicon Revision 2.0, 1.1
(SPRZ429K July 2014–Revised March 2017 [1]) mentions
Under high speed HS200 and SDR104 modes, the functional clock for MMC
modules will reach up to 192 MHz. At this frequency, the maximum obtainable
timeout (DTO = 0xE) through MMC host controller is (1/192MHz)*2^27 = 700ms.
Commands taking longer than 700ms may be affected by this small window
frame. Workaround for this errata is use a software timer instead of
hardware timer to provide the delay requested by the upper layer.

While this errata is specific to AM572x, it is applicable to all sdhci
based controllers when a particular request require timeout greater
than hardware capability.

Re-use the software timer already implemented in sdhci to program the
correct timeout value and also disable the hardware timeout when
the required timeout is greater than hardware capabiltiy in order to
avoid spurious timeout interrupts.

This patch is based on the earlier patch implemented for omap_hsmmc [2]

[1] -> http://www.ti.com/lit/er/sprz429k/sprz429k.pdf
[2] -> https://patchwork.kernel.org/patch/9791449/

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

diff --git a/drivers/mmc/host/sdhci.c b/drivers/mmc/host/sdhci.c
index e9290a3439d5..d0655e1d2cc7 100644
--- a/drivers/mmc/host/sdhci.c
+++ b/drivers/mmc/host/sdhci.c
@@ -673,6 +673,27 @@ 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_host *mmc = host->mmc;
+	struct mmc_ios *ios = &mmc->ios;
+	struct mmc_data *data = cmd->data;
+	unsigned long long transfer_time;
+
+	if (data) {
+		transfer_time = MMC_BLOCK_TRANSFER_TIME_MS(data->blksz,
+							   ios->bus_width,
+							   ios->clock);
+		/* calculate timeout for the entire data */
+		host->data_timeout = (data->blocks * (target_timeout +
+						      transfer_time));
+	} else if (cmd->flags & MMC_RSP_BUSY) {
+		host->data_timeout = cmd->busy_timeout * MSEC_PER_SEC;
+	}
+}
+
 static u8 sdhci_calc_timeout(struct sdhci_host *host, struct mmc_command *cmd)
 {
 	u8 count;
@@ -732,8 +753,12 @@ static u8 sdhci_calc_timeout(struct sdhci_host *host, struct mmc_command *cmd)
 	}
 
 	if (count >= 0xF) {
-		DBG("Too large timeout 0x%x requested for CMD%d!\n",
-		    count, cmd->opcode);
+		DBG("Too large timeout.. using SW timeout for CMD%d!\n",
+		    cmd->opcode);
+		sdhci_calc_sw_timeout(host, cmd, target_timeout);
+		host->ier &= ~SDHCI_INT_DATA_TIMEOUT;
+		sdhci_writel(host, host->ier, SDHCI_INT_ENABLE);
+		sdhci_writel(host, host->ier, SDHCI_SIGNAL_ENABLE);
 		count = 0xE;
 	}
 
@@ -1198,6 +1223,14 @@ static void sdhci_finish_command(struct sdhci_host *host)
 {
 	struct mmc_command *cmd = host->cmd;
 
+	if (host->data_timeout) {
+		unsigned long timeout;
+
+		timeout = jiffies +
+			  msecs_to_jiffies(host->data_timeout);
+		sdhci_mod_timer(host, host->cmd->mrq, timeout);
+	}
+
 	host->cmd = NULL;
 
 	if (cmd->flags & MMC_RSP_PRESENT) {
@@ -2341,6 +2374,10 @@ static bool sdhci_request_done(struct sdhci_host *host)
 		return true;
 	}
 
+	host->data_timeout = 0;
+	host->ier |= SDHCI_INT_DATA_TIMEOUT;
+	sdhci_writel(host, host->ier, SDHCI_INT_ENABLE);
+	sdhci_writel(host, host->ier, SDHCI_SIGNAL_ENABLE);
 	sdhci_del_timer(host, mrq);
 
 	/*
diff --git a/drivers/mmc/host/sdhci.h b/drivers/mmc/host/sdhci.h
index 54bc444c317f..e6e0278bea1a 100644
--- a/drivers/mmc/host/sdhci.h
+++ b/drivers/mmc/host/sdhci.h
@@ -332,6 +332,15 @@ struct sdhci_adma2_64_desc {
 /* Allow for a a command request and a data request at the same time */
 #define SDHCI_MAX_MRQS		2
 
+/*
+ * Time taken for transferring one block. It is multiplied by a constant
+ * factor '2' to account for any errors
+ */
+#define MMC_BLOCK_TRANSFER_TIME_MS(blksz, bus_width, freq)		\
+				   ((unsigned long long)		\
+				   (2 * (((blksz) * MSEC_PER_SEC *	\
+				   (8 / (bus_width))) / (freq))))
+
 enum sdhci_cookie {
 	COOKIE_UNMAPPED,
 	COOKIE_PRE_MAPPED,	/* mapped by sdhci_pre_req() */
@@ -546,6 +555,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

^ permalink raw reply related	[flat|nested] 28+ messages in thread

* [PATCH 10/12] dt-bindings: sdhci-omap: Add K2G specific binding
  2017-12-14 13:09 [PATCH 00/12] mmc: sdhci-omap: Add UHS/HS200 mode support Kishon Vijay Abraham I
                   ` (8 preceding siblings ...)
  2017-12-14 13:09 ` [RFC PATCH 09/12] mmc: sdhci: Use software timer when timeout greater than hardware capablility Kishon Vijay Abraham I
@ 2017-12-14 13:09 ` Kishon Vijay Abraham I
       [not found]   ` <20171214130941.26666-11-kishon-l0cyMroinI0@public.gmane.org>
  2017-12-14 13:09 ` [PATCH 11/12] mmc: sdhci-omap: Add support for MMC/SD controller in k2g SoC Kishon Vijay Abraham I
  2017-12-14 13:09 ` [PATCH 12/12] ARM: OMAP2+: Use sdhci-omap specific pdata-quirks for MMC/SD on DRA74x EVM Kishon Vijay Abraham I
  11 siblings, 1 reply; 28+ messages in thread
From: Kishon Vijay Abraham I @ 2017-12-14 13:09 UTC (permalink / raw)
  To: Ulf Hansson, Rob Herring, Tony Lindgren, Adrian Hunter
  Cc: Mark Rutland, Russell King, linux-mmc, devicetree, linux-kernel,
	linux-omap, linux-arm-kernel, nsekhar, kishon

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

Signed-off-by: Kishon Vijay Abraham I <kishon@ti.com>
---
 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

^ permalink raw reply related	[flat|nested] 28+ messages in thread

* [PATCH 11/12] mmc: sdhci-omap: Add support for MMC/SD controller in k2g SoC
  2017-12-14 13:09 [PATCH 00/12] mmc: sdhci-omap: Add UHS/HS200 mode support Kishon Vijay Abraham I
                   ` (9 preceding siblings ...)
  2017-12-14 13:09 ` [PATCH 10/12] dt-bindings: sdhci-omap: Add K2G specific binding Kishon Vijay Abraham I
@ 2017-12-14 13:09 ` Kishon Vijay Abraham I
  2017-12-21  9:15   ` Adrian Hunter
  2017-12-14 13:09 ` [PATCH 12/12] ARM: OMAP2+: Use sdhci-omap specific pdata-quirks for MMC/SD on DRA74x EVM Kishon Vijay Abraham I
  11 siblings, 1 reply; 28+ messages in thread
From: Kishon Vijay Abraham I @ 2017-12-14 13:09 UTC (permalink / raw)
  To: Ulf Hansson, Rob Herring, Tony Lindgren, Adrian Hunter
  Cc: Mark Rutland, Russell King, linux-mmc, devicetree, linux-kernel,
	linux-omap, linux-arm-kernel, nsekhar, kishon

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>
---
 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 cddc3ad1331f..5e81e29383d9 100644
--- a/drivers/mmc/host/sdhci-omap.c
+++ b/drivers/mmc/host/sdhci-omap.c
@@ -767,6 +767,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,
@@ -774,6 +778,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);
@@ -882,6 +887,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;
@@ -908,6 +914,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

^ permalink raw reply related	[flat|nested] 28+ messages in thread

* [PATCH 12/12] ARM: OMAP2+: Use sdhci-omap specific pdata-quirks for MMC/SD on DRA74x EVM
  2017-12-14 13:09 [PATCH 00/12] mmc: sdhci-omap: Add UHS/HS200 mode support Kishon Vijay Abraham I
                   ` (10 preceding siblings ...)
  2017-12-14 13:09 ` [PATCH 11/12] mmc: sdhci-omap: Add support for MMC/SD controller in k2g SoC Kishon Vijay Abraham I
@ 2017-12-14 13:09 ` Kishon Vijay Abraham I
  11 siblings, 0 replies; 28+ messages in thread
From: Kishon Vijay Abraham I @ 2017-12-14 13:09 UTC (permalink / raw)
  To: Ulf Hansson, Rob Herring, Tony Lindgren, Adrian Hunter
  Cc: Mark Rutland, Russell King, linux-mmc, devicetree, linux-kernel,
	linux-omap, linux-arm-kernel, nsekhar, kishon

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

^ permalink raw reply related	[flat|nested] 28+ messages in thread

* Re: [PATCH 08/12] mmc: sdhci-omap: Add support to override f_max and iodelay from pdata
  2017-12-14 13:09 ` [PATCH 08/12] mmc: sdhci-omap: Add support to override f_max and iodelay from pdata Kishon Vijay Abraham I
@ 2017-12-14 14:04   ` Philippe Ombredanne
       [not found]   ` <20171214130941.26666-9-kishon-l0cyMroinI0@public.gmane.org>
  1 sibling, 0 replies; 28+ messages in thread
From: Philippe Ombredanne @ 2017-12-14 14:04 UTC (permalink / raw)
  To: Kishon Vijay Abraham I
  Cc: Ulf Hansson, Rob Herring, Tony Lindgren, Adrian Hunter,
	Mark Rutland, Russell King, linux-mmc,
	open list:OPEN FIRMWARE AND FLATTENED DEVICE TREE BINDINGS, LKML,
	linux-omap,
	moderated list:ARM/FREESCALE IMX / MXC ARM ARCHITECTURE, nsekhar

Kishon,

On Thu, Dec 14, 2017 at 2:09 PM, 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.
>
> 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>

<snip>

> --- /dev/null
> +++ b/include/linux/platform_data/sdhci-omap.h
> @@ -0,0 +1,35 @@
> +/**
> + * SDHCI Controller Platform Data 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/>.
> + */

Could you use the new SPDX tags instead of this fine and long boilerplate? See
Thomas doc for details [1]

[1] https://lkml.org/lkml/2017/12/4/934

Thanks!
PS: if you could spread the word out in your team too, this would be
much welcomed!

-- 
Cordially
Philippe Ombredanne

^ permalink raw reply	[flat|nested] 28+ messages in thread

* Re: [PATCH 06/12] mmc: sdhci_omap: Add support to set IODELAY values
       [not found]   ` <20171214130941.26666-7-kishon-l0cyMroinI0@public.gmane.org>
@ 2017-12-14 15:04     ` Tony Lindgren
  0 siblings, 0 replies; 28+ messages in thread
From: Tony Lindgren @ 2017-12-14 15:04 UTC (permalink / raw)
  To: Kishon Vijay Abraham I
  Cc: Ulf Hansson, Rob Herring, Adrian Hunter, Mark Rutland,
	Russell King, linux-mmc-u79uwXL29TY76Z2rM5mHXA,
	devicetree-u79uwXL29TY76Z2rM5mHXA,
	linux-kernel-u79uwXL29TY76Z2rM5mHXA,
	linux-omap-u79uwXL29TY76Z2rM5mHXA,
	linux-arm-kernel-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r,
	nsekhar-l0cyMroinI0

Hi,

* Kishon Vijay Abraham I <kishon-l0cyMroinI0@public.gmane.org> [171214 13:13]:
> 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.
...

> --- a/drivers/mmc/host/sdhci-omap.c
> +++ b/drivers/mmc/host/sdhci-omap.c
> @@ -105,6 +109,20 @@ 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;
> +	struct pinctrl_state	*default_pinctrl_state;
> +	struct pinctrl_state	*sdr104_pinctrl_state;
> +	struct pinctrl_state	*hs200_1_8v_pinctrl_state;
> +	struct pinctrl_state	*ddr50_pinctrl_state;
> +	struct pinctrl_state	*sdr50_pinctrl_state;
> +	struct pinctrl_state	*sdr25_pinctrl_state;
> +	struct pinctrl_state	*sdr12_pinctrl_state;
> +	struct pinctrl_state	*hs_pinctrl_state;
> +	struct pinctrl_state	*ddr_1_8v_pinctrl_state;
>  };


You can make the pinctrl code more generic by allocating an array
of states and have just:

	struct pinctrl_state **pinctrl_state;

Then access it with omap_host->pinctrl_state[MMC_TIMING_MMC_HS200]
and so on.

This way the code gets simplified and you can do a generic function
to initialize things and call it from a for loop etc.

Just remember that pinctrl use can be optional as the pins can be
set up in the bootloader alone. Then you can just continue with the
default iodelay state like we are currently doing.

Regards,

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

^ permalink raw reply	[flat|nested] 28+ messages in thread

* Re: [PATCH 10/12] dt-bindings: sdhci-omap: Add K2G specific binding
       [not found]   ` <20171214130941.26666-11-kishon-l0cyMroinI0@public.gmane.org>
@ 2017-12-16 16:49     ` Rob Herring
  0 siblings, 0 replies; 28+ messages in thread
From: Rob Herring @ 2017-12-16 16:49 UTC (permalink / raw)
  To: Kishon Vijay Abraham I
  Cc: Ulf Hansson, Tony Lindgren, Adrian Hunter, Mark Rutland,
	Russell King, linux-mmc-u79uwXL29TY76Z2rM5mHXA,
	devicetree-u79uwXL29TY76Z2rM5mHXA,
	linux-kernel-u79uwXL29TY76Z2rM5mHXA,
	linux-omap-u79uwXL29TY76Z2rM5mHXA,
	linux-arm-kernel-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r,
	nsekhar-l0cyMroinI0

On Thu, Dec 14, 2017 at 06:39:39PM +0530, Kishon Vijay Abraham I wrote:
> Add binding for the TI's sdhci-omap controller present in K2G.
> 
> Signed-off-by: Kishon Vijay Abraham I <kishon-l0cyMroinI0@public.gmane.org>
> ---
>  Documentation/devicetree/bindings/mmc/sdhci-omap.txt | 2 ++
>  1 file changed, 2 insertions(+)

Reviewed-by: Rob Herring <robh-DgEjT+Ai2ygdnm+yROfE0A@public.gmane.org>
--
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

^ permalink raw reply	[flat|nested] 28+ messages in thread

* Re: [RFC PATCH 09/12] mmc: sdhci: Use software timer when timeout greater than hardware capablility
  2017-12-14 13:09 ` [RFC PATCH 09/12] mmc: sdhci: Use software timer when timeout greater than hardware capablility Kishon Vijay Abraham I
@ 2017-12-20 14:11   ` Adrian Hunter
  2018-01-04 12:59     ` Kishon Vijay Abraham I
  0 siblings, 1 reply; 28+ messages in thread
From: Adrian Hunter @ 2017-12-20 14:11 UTC (permalink / raw)
  To: Kishon Vijay Abraham I, Ulf Hansson, Rob Herring, Tony Lindgren
  Cc: Mark Rutland, Russell King, linux-mmc, devicetree, linux-kernel,
	linux-omap, linux-arm-kernel, nsekhar

On 14/12/17 15:09, Kishon Vijay Abraham I wrote:
> Errata i834 in AM572x Sitara Processors Silicon Revision 2.0, 1.1
> (SPRZ429K July 2014–Revised March 2017 [1]) mentions
> Under high speed HS200 and SDR104 modes, the functional clock for MMC
> modules will reach up to 192 MHz. At this frequency, the maximum obtainable
> timeout (DTO = 0xE) through MMC host controller is (1/192MHz)*2^27 = 700ms.
> Commands taking longer than 700ms may be affected by this small window
> frame. Workaround for this errata is use a software timer instead of
> hardware timer to provide the delay requested by the upper layer.
> 
> While this errata is specific to AM572x, it is applicable to all sdhci
> based controllers when a particular request require timeout greater
> than hardware capability.

It doesn't work for our controllers.  Even if the data timeout interrupt is
disabled, it seems like the timeout still "happens" in some fashion - after
which the host controller starts misbehaving.

So you will need to add a quirk.

> 
> Re-use the software timer already implemented in sdhci to program the
> correct timeout value and also disable the hardware timeout when
> the required timeout is greater than hardware capabiltiy in order to
> avoid spurious timeout interrupts.
> 
> This patch is based on the earlier patch implemented for omap_hsmmc [2]
> 
> [1] -> http://www.ti.com/lit/er/sprz429k/sprz429k.pdf
> [2] -> https://patchwork.kernel.org/patch/9791449/
> 
> Signed-off-by: Kishon Vijay Abraham I <kishon@ti.com>
> ---
>  drivers/mmc/host/sdhci.c | 41 +++++++++++++++++++++++++++++++++++++++--
>  drivers/mmc/host/sdhci.h | 11 +++++++++++
>  2 files changed, 50 insertions(+), 2 deletions(-)
> 
> diff --git a/drivers/mmc/host/sdhci.c b/drivers/mmc/host/sdhci.c
> index e9290a3439d5..d0655e1d2cc7 100644
> --- a/drivers/mmc/host/sdhci.c
> +++ b/drivers/mmc/host/sdhci.c
> @@ -673,6 +673,27 @@ 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_host *mmc = host->mmc;
> +	struct mmc_ios *ios = &mmc->ios;
> +	struct mmc_data *data = cmd->data;
> +	unsigned long long transfer_time;
> +
> +	if (data) {
> +		transfer_time = MMC_BLOCK_TRANSFER_TIME_MS(data->blksz,
> +							   ios->bus_width,
> +							   ios->clock);

If it has a value, actual_clock is better than ios->clock.

> +		/* calculate timeout for the entire data */
> +		host->data_timeout = (data->blocks * (target_timeout +
> +						      transfer_time));
> +	} else if (cmd->flags & MMC_RSP_BUSY) {
> +		host->data_timeout = cmd->busy_timeout * MSEC_PER_SEC;

Doesn't need MSEC_PER_SEC multiplier.

> +	}
> +}
> +
>  static u8 sdhci_calc_timeout(struct sdhci_host *host, struct mmc_command *cmd)
>  {
>  	u8 count;
> @@ -732,8 +753,12 @@ static u8 sdhci_calc_timeout(struct sdhci_host *host, struct mmc_command *cmd)
>  	}
>  
>  	if (count >= 0xF) {
> -		DBG("Too large timeout 0x%x requested for CMD%d!\n",
> -		    count, cmd->opcode);
> +		DBG("Too large timeout.. using SW timeout for CMD%d!\n",
> +		    cmd->opcode);
> +		sdhci_calc_sw_timeout(host, cmd, target_timeout);
> +		host->ier &= ~SDHCI_INT_DATA_TIMEOUT;
> +		sdhci_writel(host, host->ier, SDHCI_INT_ENABLE);
> +		sdhci_writel(host, host->ier, SDHCI_SIGNAL_ENABLE);
>  		count = 0xE;
>  	}
>  
> @@ -1198,6 +1223,14 @@ static void sdhci_finish_command(struct sdhci_host *host)
>  {
>  	struct mmc_command *cmd = host->cmd;
>  
> +	if (host->data_timeout) {
> +		unsigned long timeout;
> +
> +		timeout = jiffies +
> +			  msecs_to_jiffies(host->data_timeout);
> +		sdhci_mod_timer(host, host->cmd->mrq, timeout);

cmd could be the sbc or a stop cmd or a command during transfer, so this
needs more logic.

> +	}
> +
>  	host->cmd = NULL;
>  
>  	if (cmd->flags & MMC_RSP_PRESENT) {
> @@ -2341,6 +2374,10 @@ static bool sdhci_request_done(struct sdhci_host *host)
>  		return true;
>  	}
>  
> +	host->data_timeout = 0;
> +	host->ier |= SDHCI_INT_DATA_TIMEOUT;
> +	sdhci_writel(host, host->ier, SDHCI_INT_ENABLE);
> +	sdhci_writel(host, host->ier, SDHCI_SIGNAL_ENABLE);

sdhci can have 2 requests in progress to allow for commands to be sent while
a data transfer is in progress, so this is not necessarily the data transfer
request that is done.  Also we want to avoid unnecessary register writes.

>  	sdhci_del_timer(host, mrq);
>  
>  	/*
> diff --git a/drivers/mmc/host/sdhci.h b/drivers/mmc/host/sdhci.h
> index 54bc444c317f..e6e0278bea1a 100644
> --- a/drivers/mmc/host/sdhci.h
> +++ b/drivers/mmc/host/sdhci.h
> @@ -332,6 +332,15 @@ struct sdhci_adma2_64_desc {
>  /* Allow for a a command request and a data request at the same time */
>  #define SDHCI_MAX_MRQS		2
>  
> +/*
> + * Time taken for transferring one block. It is multiplied by a constant
> + * factor '2' to account for any errors
> + */
> +#define MMC_BLOCK_TRANSFER_TIME_MS(blksz, bus_width, freq)		\
> +				   ((unsigned long long)		\
> +				   (2 * (((blksz) * MSEC_PER_SEC *	\
> +				   (8 / (bus_width))) / (freq))))

I don't think the macro helps make the code more readable.  Might just as
well write a nice function to calculate the entire data request timeout.

> +
>  enum sdhci_cookie {
>  	COOKIE_UNMAPPED,
>  	COOKIE_PRE_MAPPED,	/* mapped by sdhci_pre_req() */
> @@ -546,6 +555,8 @@ struct sdhci_host {
>  	/* Host SDMA buffer boundary. */
>  	u32			sdma_boundary;
>  
> +	unsigned long long	data_timeout;

msecs_to_jiffies() will truncate to 'unsigned int' anyway, so this might as
well be 'unsigned int'.

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


^ permalink raw reply	[flat|nested] 28+ messages in thread

* Re: [PATCH 01/12] mmc: sdhci-omap: Update 'power_mode' outside sdhci_omap_init_74_clocks
  2017-12-14 13:09 ` [PATCH 01/12] mmc: sdhci-omap: Update 'power_mode' outside sdhci_omap_init_74_clocks Kishon Vijay Abraham I
@ 2017-12-21  8:57   ` Adrian Hunter
  0 siblings, 0 replies; 28+ messages in thread
From: Adrian Hunter @ 2017-12-21  8:57 UTC (permalink / raw)
  To: Kishon Vijay Abraham I, Ulf Hansson, Rob Herring, Tony Lindgren
  Cc: Mark Rutland, Russell King, linux-mmc, devicetree, linux-kernel,
	linux-omap, linux-arm-kernel, nsekhar

On 14/12/17 15:09, Kishon Vijay Abraham I wrote:
> 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;
> 


^ permalink raw reply	[flat|nested] 28+ messages in thread

* Re: [PATCH 02/12] mmc: sdhci-omap: Add card_busy host ops
  2017-12-14 13:09 ` [PATCH 02/12] mmc: sdhci-omap: Add card_busy host ops Kishon Vijay Abraham I
@ 2017-12-21  8:59   ` Adrian Hunter
  0 siblings, 0 replies; 28+ messages in thread
From: Adrian Hunter @ 2017-12-21  8:59 UTC (permalink / raw)
  To: Kishon Vijay Abraham I, Ulf Hansson, Rob Herring, Tony Lindgren
  Cc: Mark Rutland, Russell King, linux-mmc, devicetree, linux-kernel,
	linux-omap, linux-arm-kernel, nsekhar

On 14/12/17 15:09, Kishon Vijay Abraham I wrote:
> card_busy ops is used by mmc core in
>   1) mmc_set_uhs_voltage to verify voltage switch
>   2) __mmc_start_request/mmc_poll_for_busy to check the card busy status
> 
> While only DAT0 can be used to check the card busy status (in '2' above),
> CMD and DAT[0..3] is used to verify voltage switch (in '1' above).
> 
> 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]).
> 
> Add card_busy host ops in sdhci_omap that checks for both CMD and
> DAT[0..3]. card_busy here returns true if one of CMD and DAT[0..3] is
> low though during voltage switch sequence all of CMD and DAT[0..3] has
> to be low (however haven't observed a case where some DAT lines are low
> and some are high).

Isn't it better to check DAT0 only since that is all that is defined for 'busy'.

> 
> 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 | 62 +++++++++++++++++++++++++++++++++++++++++++
>  1 file changed, 62 insertions(+)
> 
> diff --git a/drivers/mmc/host/sdhci-omap.c b/drivers/mmc/host/sdhci-omap.c
> index 96985786cadf..defe4eac020d 100644
> --- a/drivers/mmc/host/sdhci-omap.c
> +++ b/drivers/mmc/host/sdhci-omap.c
> @@ -31,11 +31,20 @@
>  #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_CLEV             BIT(24)
> +#define PSTATE_DLEV_SHIFT	20
> +#define PSTATE_DLEV_DAT(x)	(1 << (PSTATE_DLEV_SHIFT + (x)))
> +#define PSTATE_DLEV		(PSTATE_DLEV_DAT(0) | PSTATE_DLEV_DAT(1) | \
> +					PSTATE_DLEV_DAT(2) | PSTATE_DLEV_DAT(3))
> +
>  #define SDHCI_OMAP_HCTL		0x228
>  #define HCTL_SDBP		BIT(8)
>  #define HCTL_SDVS_SHIFT		9
> @@ -191,6 +200,58 @@ static void sdhci_omap_conf_bus_power(struct sdhci_omap_host *omap_host,
>  	}
>  }
>  
> +static int sdhci_omap_card_busy(struct mmc_host *mmc)
> +{
> +	int i;
> +	u32 reg, ac12;
> +	int ret = true;
> +	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);
> +
> +	for (i = 0; i < 5; i++) {
> +		/*
> +		 * Delay is required for PSTATE to correctly reflect
> +		 * DLEV/CLEV values after PADEM is set.
> +		 */
> +		usleep_range(100, 200);
> +		reg = sdhci_omap_readl(omap_host, SDHCI_OMAP_PSTATE);
> +		if ((reg & PSTATE_CLEV) &&
> +		    ((reg & PSTATE_DLEV) == PSTATE_DLEV)) {
> +			ret = false;
> +			goto ret;

'break' is better than 'goto'

> +		}
> +	}
> +
> +ret:
> +	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 +623,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;
> 


^ permalink raw reply	[flat|nested] 28+ messages in thread

* Re: [PATCH 03/12] mmc: sdhci-omap: Add custom set_uhs_signaling sdhci_host ops
  2017-12-14 13:09 ` [PATCH 03/12] mmc: sdhci-omap: Add custom set_uhs_signaling sdhci_host ops Kishon Vijay Abraham I
@ 2017-12-21  9:01   ` Adrian Hunter
  0 siblings, 0 replies; 28+ messages in thread
From: Adrian Hunter @ 2017-12-21  9:01 UTC (permalink / raw)
  To: Kishon Vijay Abraham I, Ulf Hansson, Rob Herring, Tony Lindgren
  Cc: Mark Rutland, Russell King, linux-mmc, devicetree, linux-kernel,
	linux-omap, linux-arm-kernel, nsekhar

On 14/12/17 15:09, Kishon Vijay Abraham I wrote:
> 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>

Apart from 1 minor comment below:

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

> ---
>  drivers/mmc/host/sdhci-omap.c | 26 +++++++++++++++++++++++++-
>  1 file changed, 25 insertions(+), 1 deletion(-)
> 
> diff --git a/drivers/mmc/host/sdhci-omap.c b/drivers/mmc/host/sdhci-omap.c
> index defe4eac020d..8f7239e2edc2 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)
> @@ -93,6 +94,9 @@ struct sdhci_omap_host {
>  	u8			power_mode;
>  };
>  
> +static void sdhci_omap_start_clock(struct sdhci_omap_host *omap_host);
> +static void sdhci_omap_stop_clock(struct sdhci_omap_host *omap_host);

These forward declarations aren't needed are they.

> +
>  static inline u32 sdhci_omap_readl(struct sdhci_omap_host *host,
>  				   unsigned int offset)
>  {
> @@ -471,6 +475,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,
> @@ -480,7 +504,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)
> 


^ permalink raw reply	[flat|nested] 28+ messages in thread

* Re: [PATCH 04/12] mmc: sdhci-omap: Add tuning support
  2017-12-14 13:09 ` [PATCH 04/12] mmc: sdhci-omap: Add tuning support Kishon Vijay Abraham I
@ 2017-12-21  9:09   ` Adrian Hunter
  0 siblings, 0 replies; 28+ messages in thread
From: Adrian Hunter @ 2017-12-21  9:09 UTC (permalink / raw)
  To: Kishon Vijay Abraham I, Ulf Hansson, Rob Herring, Tony Lindgren
  Cc: Mark Rutland, Russell King, linux-mmc, devicetree, linux-kernel,
	linux-omap, linux-arm-kernel, nsekhar

On 14/12/17 15:09, Kishon Vijay Abraham I wrote:
> 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>

Apart from 1 minor comment below:

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 8f7239e2edc2..df8a0a472996 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
> @@ -66,12 +73,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
> @@ -80,6 +91,8 @@
>  #define IOV_3V0			3000000		/* 300000 uV */
>  #define IOV_3V3			3300000		/* 330000 uV */
>  
> +#define MAX_PHASE_DELAY		0x7C
> +
>  struct sdhci_omap_data {
>  	u32 offset;
>  };
> @@ -204,6 +217,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)
> +{
> +	u32 reg;
> +	int ret = 0;
> +	u8 cur_match, prev_match = 0;
> +	u32 phase_delay = 0;
> +	u32 start_window = 0, max_window = 0;
> +	u32 length = 0, max_len = 0;
> +	struct mmc_ios *ios = &mmc->ios;
> +	struct sdhci_host *host = mmc_priv(mmc);
> +	struct sdhci_pltfm_host *pltfm_host;
> +	struct sdhci_omap_host *omap_host;
> +	struct device *dev;
> +
> +	pltfm_host = sdhci_priv(host);
> +	omap_host = sdhci_pltfm_priv(pltfm_host);
> +	dev = omap_host->dev;

These initializations can be combined with the declarations.  Also try to
arrange local variable declaration lines in descending order of length e.g.

	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;

> +
> +	/* 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)
>  {
>  	int i;
> @@ -312,6 +439,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;
>  }
>  
> @@ -648,6 +777,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;
> 

^ permalink raw reply	[flat|nested] 28+ messages in thread

* Re: [PATCH 05/12] mmc: sdhci-omap: Workaround for Errata i802
  2017-12-14 13:09   ` [PATCH 05/12] mmc: sdhci-omap: Workaround for Errata i802 Kishon Vijay Abraham I
@ 2017-12-21  9:09     ` Adrian Hunter
  0 siblings, 0 replies; 28+ messages in thread
From: Adrian Hunter @ 2017-12-21  9:09 UTC (permalink / raw)
  To: Kishon Vijay Abraham I, Ulf Hansson, Rob Herring, Tony Lindgren
  Cc: Mark Rutland, Russell King, linux-mmc, devicetree, linux-kernel,
	linux-omap, linux-arm-kernel, nsekhar

On 14/12/17 15:09, Kishon Vijay Abraham I wrote:
> 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 df8a0a472996..b20f4c79ccc6 100644
> --- a/drivers/mmc/host/sdhci-omap.c
> +++ b/drivers/mmc/host/sdhci-omap.c
> @@ -266,6 +266,7 @@ static int sdhci_omap_execute_tuning(struct mmc_host *mmc, u32 opcode)
>  	struct sdhci_pltfm_host *pltfm_host;
>  	struct sdhci_omap_host *omap_host;
>  	struct device *dev;
> +	u32 ier = host->ier;
>  
>  	pltfm_host = sdhci_priv(host);
>  	omap_host = sdhci_pltfm_priv(pltfm_host);
> @@ -283,6 +284,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);
>  
> @@ -328,6 +339,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;
>  }
>  
> 

^ permalink raw reply	[flat|nested] 28+ messages in thread

* Re: [PATCH 07/12] mmc: sdhci_omap: Fix sdhci-omap quirks
       [not found]   ` <20171214130941.26666-8-kishon-l0cyMroinI0@public.gmane.org>
@ 2017-12-21  9:12     ` Adrian Hunter
  0 siblings, 0 replies; 28+ messages in thread
From: Adrian Hunter @ 2017-12-21  9:12 UTC (permalink / raw)
  To: Kishon Vijay Abraham I, Ulf Hansson, Rob Herring, Tony Lindgren
  Cc: Mark Rutland, Russell King, linux-mmc-u79uwXL29TY76Z2rM5mHXA,
	devicetree-u79uwXL29TY76Z2rM5mHXA,
	linux-kernel-u79uwXL29TY76Z2rM5mHXA,
	linux-omap-u79uwXL29TY76Z2rM5mHXA,
	linux-arm-kernel-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r,
	nsekhar-l0cyMroinI0

On 14/12/17 15:09, Kishon Vijay Abraham I wrote:
> Remove SDHCI_QUIRK_BROKEN_CARD_DETECTION quirk as gpio card detection
> is supported in sdhci-omap.

SDHCI_QUIRK_BROKEN_CARD_DETECTION is for native card detection not gpio card
detection.

> 
> 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-l0cyMroinI0@public.gmane.org>
> ---
>  drivers/mmc/host/sdhci-omap.c | 7 +++----
>  1 file changed, 3 insertions(+), 4 deletions(-)
> 
> diff --git a/drivers/mmc/host/sdhci-omap.c b/drivers/mmc/host/sdhci-omap.c
> index 594e41200d8a..6dee275b2e57 100644
> --- a/drivers/mmc/host/sdhci-omap.c
> +++ b/drivers/mmc/host/sdhci-omap.c
> @@ -755,13 +755,12 @@ static int sdhci_omap_set_capabilities(struct sdhci_omap_host *omap_host)
>  }
>  
>  static const struct sdhci_pltfm_data sdhci_omap_pdata = {
> -	.quirks = SDHCI_QUIRK_BROKEN_CARD_DETECTION |
> -		  SDHCI_QUIRK_DATA_TIMEOUT_USES_SDCLK |
> +	.quirks = SDHCI_QUIRK_DATA_TIMEOUT_USES_SDCLK |
>  		  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,
>  };
> 

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

^ permalink raw reply	[flat|nested] 28+ messages in thread

* Re: [PATCH 08/12] mmc: sdhci-omap: Add support to override f_max and iodelay from pdata
       [not found]   ` <20171214130941.26666-9-kishon-l0cyMroinI0@public.gmane.org>
@ 2017-12-21  9:13     ` Adrian Hunter
  0 siblings, 0 replies; 28+ messages in thread
From: Adrian Hunter @ 2017-12-21  9:13 UTC (permalink / raw)
  To: Kishon Vijay Abraham I, Ulf Hansson, Rob Herring, Tony Lindgren
  Cc: Mark Rutland, Russell King, linux-mmc-u79uwXL29TY76Z2rM5mHXA,
	devicetree-u79uwXL29TY76Z2rM5mHXA,
	linux-kernel-u79uwXL29TY76Z2rM5mHXA,
	linux-omap-u79uwXL29TY76Z2rM5mHXA,
	linux-arm-kernel-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r,
	nsekhar-l0cyMroinI0

On 14/12/17 15:09, Kishon Vijay Abraham I 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.
> 
> 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-l0cyMroinI0@public.gmane.org>
> ---
>  drivers/mmc/host/sdhci-omap.c            | 17 ++++++++++++++++
>  include/linux/platform_data/sdhci-omap.h | 35 ++++++++++++++++++++++++++++++++
>  2 files changed, 52 insertions(+)
>  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 6dee275b2e57..cddc3ad1331f 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>
> @@ -102,6 +103,7 @@ struct sdhci_omap_data {
>  };
>  
>  struct sdhci_omap_host {
> +	char			*version;
>  	void __iomem		*base;
>  	struct device		*dev;
>  	struct	regulator	*pbias;
> @@ -781,11 +783,18 @@ 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;
>  
> +	if (version) {
> +		sprintf(str, "%s-%s", mode, version);

snprintf please

> +		pinctrl_state = pinctrl_lookup_state(omap_host->pinctrl, str);

Doesn't look like this 'pinctrl_state' is used?

> +	}
> +
>  	pinctrl_state = pinctrl_lookup_state(omap_host->pinctrl, mode);
>  	if (IS_ERR(pinctrl_state)) {
>  		dev_err(dev, "no pinctrl state for %s mode", mode);
> @@ -879,6 +888,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)
> @@ -913,6 +923,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..a46e1240956a
> --- /dev/null
> +++ b/include/linux/platform_data/sdhci-omap.h
> @@ -0,0 +1,35 @@
> +/**
> + * SDHCI Controller Platform Data 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/>.
> + */
> +#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
> 

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

^ permalink raw reply	[flat|nested] 28+ messages in thread

* Re: [PATCH 11/12] mmc: sdhci-omap: Add support for MMC/SD controller in k2g SoC
  2017-12-14 13:09 ` [PATCH 11/12] mmc: sdhci-omap: Add support for MMC/SD controller in k2g SoC Kishon Vijay Abraham I
@ 2017-12-21  9:15   ` Adrian Hunter
  0 siblings, 0 replies; 28+ messages in thread
From: Adrian Hunter @ 2017-12-21  9:15 UTC (permalink / raw)
  To: Kishon Vijay Abraham I, Ulf Hansson, Rob Herring, Tony Lindgren
  Cc: Mark Rutland, Russell King, linux-mmc, devicetree, linux-kernel,
	linux-omap, linux-arm-kernel, nsekhar

On 14/12/17 15:09, Kishon Vijay Abraham I wrote:
> 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 cddc3ad1331f..5e81e29383d9 100644
> --- a/drivers/mmc/host/sdhci-omap.c
> +++ b/drivers/mmc/host/sdhci-omap.c
> @@ -767,6 +767,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,
> @@ -774,6 +778,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);
> @@ -882,6 +887,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;
> @@ -908,6 +914,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;
> 


^ permalink raw reply	[flat|nested] 28+ messages in thread

* Re: [RFC PATCH 09/12] mmc: sdhci: Use software timer when timeout greater than hardware capablility
  2017-12-20 14:11   ` Adrian Hunter
@ 2018-01-04 12:59     ` Kishon Vijay Abraham I
  2018-01-11  8:46       ` Adrian Hunter
  0 siblings, 1 reply; 28+ messages in thread
From: Kishon Vijay Abraham I @ 2018-01-04 12:59 UTC (permalink / raw)
  To: Adrian Hunter, Ulf Hansson, Rob Herring, Tony Lindgren
  Cc: Mark Rutland, Russell King, linux-mmc, devicetree, linux-kernel,
	linux-omap, linux-arm-kernel, nsekhar

Hi Adrian,

On Wednesday 20 December 2017 07:41 PM, Adrian Hunter wrote:
> On 14/12/17 15:09, Kishon Vijay Abraham I wrote:
>> Errata i834 in AM572x Sitara Processors Silicon Revision 2.0, 1.1
>> (SPRZ429K July 2014–Revised March 2017 [1]) mentions
>> Under high speed HS200 and SDR104 modes, the functional clock for MMC
>> modules will reach up to 192 MHz. At this frequency, the maximum obtainable
>> timeout (DTO = 0xE) through MMC host controller is (1/192MHz)*2^27 = 700ms.
>> Commands taking longer than 700ms may be affected by this small window
>> frame. Workaround for this errata is use a software timer instead of
>> hardware timer to provide the delay requested by the upper layer.
>>
>> While this errata is specific to AM572x, it is applicable to all sdhci
>> based controllers when a particular request require timeout greater
>> than hardware capability.
> 
> It doesn't work for our controllers.  Even if the data timeout interrupt is
> disabled, it seems like the timeout still "happens" in some fashion - after
> which the host controller starts misbehaving.

even if the data timeout doesn't get disabled, count = 0xE is still present. So
ideally this shouldn't break any existing platforms no?
> 
> So you will need to add a quirk.
> 
>>
>> Re-use the software timer already implemented in sdhci to program the
>> correct timeout value and also disable the hardware timeout when
>> the required timeout is greater than hardware capabiltiy in order to
>> avoid spurious timeout interrupts.
>>
>> This patch is based on the earlier patch implemented for omap_hsmmc [2]
>>
>> [1] -> http://www.ti.com/lit/er/sprz429k/sprz429k.pdf
>> [2] -> https://patchwork.kernel.org/patch/9791449/
>>
>> Signed-off-by: Kishon Vijay Abraham I <kishon@ti.com>
>> ---
>>  drivers/mmc/host/sdhci.c | 41 +++++++++++++++++++++++++++++++++++++++--
>>  drivers/mmc/host/sdhci.h | 11 +++++++++++
>>  2 files changed, 50 insertions(+), 2 deletions(-)
>>
>> diff --git a/drivers/mmc/host/sdhci.c b/drivers/mmc/host/sdhci.c
>> index e9290a3439d5..d0655e1d2cc7 100644
>> --- a/drivers/mmc/host/sdhci.c
>> +++ b/drivers/mmc/host/sdhci.c
>> @@ -673,6 +673,27 @@ 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_host *mmc = host->mmc;
>> +	struct mmc_ios *ios = &mmc->ios;
>> +	struct mmc_data *data = cmd->data;
>> +	unsigned long long transfer_time;
>> +
>> +	if (data) {
>> +		transfer_time = MMC_BLOCK_TRANSFER_TIME_MS(data->blksz,
>> +							   ios->bus_width,
>> +							   ios->clock);
> 
> If it has a value, actual_clock is better than ios->clock.

okay.
> 
>> +		/* calculate timeout for the entire data */
>> +		host->data_timeout = (data->blocks * (target_timeout +
>> +						      transfer_time));
>> +	} else if (cmd->flags & MMC_RSP_BUSY) {
>> +		host->data_timeout = cmd->busy_timeout * MSEC_PER_SEC;
> 
> Doesn't need MSEC_PER_SEC multiplier.

right.
> 
>> +	}
>> +}
>> +
>>  static u8 sdhci_calc_timeout(struct sdhci_host *host, struct mmc_command *cmd)
>>  {
>>  	u8 count;
>> @@ -732,8 +753,12 @@ static u8 sdhci_calc_timeout(struct sdhci_host *host, struct mmc_command *cmd)
>>  	}
>>  
>>  	if (count >= 0xF) {
>> -		DBG("Too large timeout 0x%x requested for CMD%d!\n",
>> -		    count, cmd->opcode);
>> +		DBG("Too large timeout.. using SW timeout for CMD%d!\n",
>> +		    cmd->opcode);
>> +		sdhci_calc_sw_timeout(host, cmd, target_timeout);
>> +		host->ier &= ~SDHCI_INT_DATA_TIMEOUT;
>> +		sdhci_writel(host, host->ier, SDHCI_INT_ENABLE);
>> +		sdhci_writel(host, host->ier, SDHCI_SIGNAL_ENABLE);
>>  		count = 0xE;
>>  	}
>>  
>> @@ -1198,6 +1223,14 @@ static void sdhci_finish_command(struct sdhci_host *host)
>>  {
>>  	struct mmc_command *cmd = host->cmd;
>>  
>> +	if (host->data_timeout) {
>> +		unsigned long timeout;
>> +
>> +		timeout = jiffies +
>> +			  msecs_to_jiffies(host->data_timeout);
>> +		sdhci_mod_timer(host, host->cmd->mrq, timeout);
> 
> cmd could be the sbc or a stop cmd or a command during transfer, so this
> needs more logic.

host->data_timeout gets set only for data commands or commands with busy
timeout. But I guess for commands during data transfer, host->data_timeout
might still be set?

Checking sdhci_data_line_cmd(mrq->cmd) in addition to host->data_timeout should
take care of all cases right?
> 
>> +	}
>> +
>>  	host->cmd = NULL;
>>  
>>  	if (cmd->flags & MMC_RSP_PRESENT) {
>> @@ -2341,6 +2374,10 @@ static bool sdhci_request_done(struct sdhci_host *host)
>>  		return true;
>>  	}
>>  
>> +	host->data_timeout = 0;
>> +	host->ier |= SDHCI_INT_DATA_TIMEOUT;
>> +	sdhci_writel(host, host->ier, SDHCI_INT_ENABLE);
>> +	sdhci_writel(host, host->ier, SDHCI_SIGNAL_ENABLE);
> 
> sdhci can have 2 requests in progress to allow for commands to be sent while
> a data transfer is in progress, so this is not necessarily the data transfer
> request that is done.  Also we want to avoid unnecessary register writes.
> 

okay.. got it.
>>  	sdhci_del_timer(host, mrq);
>>  
>>  	/*
>> diff --git a/drivers/mmc/host/sdhci.h b/drivers/mmc/host/sdhci.h
>> index 54bc444c317f..e6e0278bea1a 100644
>> --- a/drivers/mmc/host/sdhci.h
>> +++ b/drivers/mmc/host/sdhci.h
>> @@ -332,6 +332,15 @@ struct sdhci_adma2_64_desc {
>>  /* Allow for a a command request and a data request at the same time */
>>  #define SDHCI_MAX_MRQS		2
>>  
>> +/*
>> + * Time taken for transferring one block. It is multiplied by a constant
>> + * factor '2' to account for any errors
>> + */
>> +#define MMC_BLOCK_TRANSFER_TIME_MS(blksz, bus_width, freq)		\
>> +				   ((unsigned long long)		\
>> +				   (2 * (((blksz) * MSEC_PER_SEC *	\
>> +				   (8 / (bus_width))) / (freq))))
> 
> I don't think the macro helps make the code more readable.  Might just as
> well write a nice function to calculate the entire data request timeout.

okay.
> 
>> +
>>  enum sdhci_cookie {
>>  	COOKIE_UNMAPPED,
>>  	COOKIE_PRE_MAPPED,	/* mapped by sdhci_pre_req() */
>> @@ -546,6 +555,8 @@ struct sdhci_host {
>>  	/* Host SDMA buffer boundary. */
>>  	u32			sdma_boundary;
>>  
>> +	unsigned long long	data_timeout;
> 
> msecs_to_jiffies() will truncate to 'unsigned int' anyway, so this might as
> well be 'unsigned int'.
> 

okay.

Thanks
Kishon

^ permalink raw reply	[flat|nested] 28+ messages in thread

* Re: [RFC PATCH 09/12] mmc: sdhci: Use software timer when timeout greater than hardware capablility
  2018-01-04 12:59     ` Kishon Vijay Abraham I
@ 2018-01-11  8:46       ` Adrian Hunter
  2018-02-02 13:25         ` Kishon Vijay Abraham I
  0 siblings, 1 reply; 28+ messages in thread
From: Adrian Hunter @ 2018-01-11  8:46 UTC (permalink / raw)
  To: Kishon Vijay Abraham I, Ulf Hansson, Rob Herring, Tony Lindgren
  Cc: Mark Rutland, Russell King, linux-mmc, devicetree, linux-kernel,
	linux-omap, linux-arm-kernel, nsekhar

On 04/01/18 14:59, Kishon Vijay Abraham I wrote:
> Hi Adrian,
> 
> On Wednesday 20 December 2017 07:41 PM, Adrian Hunter wrote:
>> On 14/12/17 15:09, Kishon Vijay Abraham I wrote:
>>> Errata i834 in AM572x Sitara Processors Silicon Revision 2.0, 1.1
>>> (SPRZ429K July 2014–Revised March 2017 [1]) mentions
>>> Under high speed HS200 and SDR104 modes, the functional clock for MMC
>>> modules will reach up to 192 MHz. At this frequency, the maximum obtainable
>>> timeout (DTO = 0xE) through MMC host controller is (1/192MHz)*2^27 = 700ms.
>>> Commands taking longer than 700ms may be affected by this small window
>>> frame. Workaround for this errata is use a software timer instead of
>>> hardware timer to provide the delay requested by the upper layer.
>>>
>>> While this errata is specific to AM572x, it is applicable to all sdhci
>>> based controllers when a particular request require timeout greater
>>> than hardware capability.
>>
>> It doesn't work for our controllers.  Even if the data timeout interrupt is
>> disabled, it seems like the timeout still "happens" in some fashion - after
>> which the host controller starts misbehaving.
> 
> even if the data timeout doesn't get disabled, count = 0xE is still present. So
> ideally this shouldn't break any existing platforms no?

I don't want to hide this kind of variation in the hardware behaviour.

>>
>> So you will need to add a quirk.
>>
>>>
>>> Re-use the software timer already implemented in sdhci to program the
>>> correct timeout value and also disable the hardware timeout when
>>> the required timeout is greater than hardware capabiltiy in order to
>>> avoid spurious timeout interrupts.
>>>
>>> This patch is based on the earlier patch implemented for omap_hsmmc [2]
>>>
>>> [1] -> http://www.ti.com/lit/er/sprz429k/sprz429k.pdf
>>> [2] -> https://patchwork.kernel.org/patch/9791449/
>>>
>>> Signed-off-by: Kishon Vijay Abraham I <kishon@ti.com>
>>> ---
>>>  drivers/mmc/host/sdhci.c | 41 +++++++++++++++++++++++++++++++++++++++--
>>>  drivers/mmc/host/sdhci.h | 11 +++++++++++
>>>  2 files changed, 50 insertions(+), 2 deletions(-)
>>>
>>> diff --git a/drivers/mmc/host/sdhci.c b/drivers/mmc/host/sdhci.c
>>> index e9290a3439d5..d0655e1d2cc7 100644
>>> --- a/drivers/mmc/host/sdhci.c
>>> +++ b/drivers/mmc/host/sdhci.c
>>> @@ -673,6 +673,27 @@ 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_host *mmc = host->mmc;
>>> +	struct mmc_ios *ios = &mmc->ios;
>>> +	struct mmc_data *data = cmd->data;
>>> +	unsigned long long transfer_time;
>>> +
>>> +	if (data) {
>>> +		transfer_time = MMC_BLOCK_TRANSFER_TIME_MS(data->blksz,
>>> +							   ios->bus_width,
>>> +							   ios->clock);
>>
>> If it has a value, actual_clock is better than ios->clock.
> 
> okay.
>>
>>> +		/* calculate timeout for the entire data */
>>> +		host->data_timeout = (data->blocks * (target_timeout +
>>> +						      transfer_time));
>>> +	} else if (cmd->flags & MMC_RSP_BUSY) {
>>> +		host->data_timeout = cmd->busy_timeout * MSEC_PER_SEC;
>>
>> Doesn't need MSEC_PER_SEC multiplier.
> 
> right.
>>
>>> +	}
>>> +}
>>> +
>>>  static u8 sdhci_calc_timeout(struct sdhci_host *host, struct mmc_command *cmd)
>>>  {
>>>  	u8 count;
>>> @@ -732,8 +753,12 @@ static u8 sdhci_calc_timeout(struct sdhci_host *host, struct mmc_command *cmd)
>>>  	}
>>>  
>>>  	if (count >= 0xF) {
>>> -		DBG("Too large timeout 0x%x requested for CMD%d!\n",
>>> -		    count, cmd->opcode);
>>> +		DBG("Too large timeout.. using SW timeout for CMD%d!\n",
>>> +		    cmd->opcode);
>>> +		sdhci_calc_sw_timeout(host, cmd, target_timeout);
>>> +		host->ier &= ~SDHCI_INT_DATA_TIMEOUT;
>>> +		sdhci_writel(host, host->ier, SDHCI_INT_ENABLE);
>>> +		sdhci_writel(host, host->ier, SDHCI_SIGNAL_ENABLE);
>>>  		count = 0xE;
>>>  	}
>>>  
>>> @@ -1198,6 +1223,14 @@ static void sdhci_finish_command(struct sdhci_host *host)
>>>  {
>>>  	struct mmc_command *cmd = host->cmd;
>>>  
>>> +	if (host->data_timeout) {
>>> +		unsigned long timeout;
>>> +
>>> +		timeout = jiffies +
>>> +			  msecs_to_jiffies(host->data_timeout);
>>> +		sdhci_mod_timer(host, host->cmd->mrq, timeout);
>>
>> cmd could be the sbc or a stop cmd or a command during transfer, so this
>> needs more logic.
> 
> host->data_timeout gets set only for data commands or commands with busy
> timeout. But I guess for commands during data transfer, host->data_timeout
> might still be set?
> 
> Checking sdhci_data_line_cmd(mrq->cmd) in addition to host->data_timeout should
> take care of all cases right?

I suggest you make the timeout calculation allow for the commands as well
and then reorder sdhci_mod_timer() to be called after sdhci_prepare_data()
and make sdhci_mod_timer() do the right thing.

>>
>>> +	}
>>> +
>>>  	host->cmd = NULL;
>>>  
>>>  	if (cmd->flags & MMC_RSP_PRESENT) {
>>> @@ -2341,6 +2374,10 @@ static bool sdhci_request_done(struct sdhci_host *host)
>>>  		return true;
>>>  	}
>>>  
>>> +	host->data_timeout = 0;
>>> +	host->ier |= SDHCI_INT_DATA_TIMEOUT;
>>> +	sdhci_writel(host, host->ier, SDHCI_INT_ENABLE);
>>> +	sdhci_writel(host, host->ier, SDHCI_SIGNAL_ENABLE);
>>
>> sdhci can have 2 requests in progress to allow for commands to be sent while
>> a data transfer is in progress, so this is not necessarily the data transfer
>> request that is done.  Also we want to avoid unnecessary register writes.
>>
> 
> okay.. got it.
>>>  	sdhci_del_timer(host, mrq);
>>>  
>>>  	/*
>>> diff --git a/drivers/mmc/host/sdhci.h b/drivers/mmc/host/sdhci.h
>>> index 54bc444c317f..e6e0278bea1a 100644
>>> --- a/drivers/mmc/host/sdhci.h
>>> +++ b/drivers/mmc/host/sdhci.h
>>> @@ -332,6 +332,15 @@ struct sdhci_adma2_64_desc {
>>>  /* Allow for a a command request and a data request at the same time */
>>>  #define SDHCI_MAX_MRQS		2
>>>  
>>> +/*
>>> + * Time taken for transferring one block. It is multiplied by a constant
>>> + * factor '2' to account for any errors
>>> + */
>>> +#define MMC_BLOCK_TRANSFER_TIME_MS(blksz, bus_width, freq)		\
>>> +				   ((unsigned long long)		\
>>> +				   (2 * (((blksz) * MSEC_PER_SEC *	\
>>> +				   (8 / (bus_width))) / (freq))))
>>
>> I don't think the macro helps make the code more readable.  Might just as
>> well write a nice function to calculate the entire data request timeout.
> 
> okay.
>>
>>> +
>>>  enum sdhci_cookie {
>>>  	COOKIE_UNMAPPED,
>>>  	COOKIE_PRE_MAPPED,	/* mapped by sdhci_pre_req() */
>>> @@ -546,6 +555,8 @@ struct sdhci_host {
>>>  	/* Host SDMA buffer boundary. */
>>>  	u32			sdma_boundary;
>>>  
>>> +	unsigned long long	data_timeout;
>>
>> msecs_to_jiffies() will truncate to 'unsigned int' anyway, so this might as
>> well be 'unsigned int'.
>>
> 
> okay.
> 
> Thanks
> Kishon
> 

^ permalink raw reply	[flat|nested] 28+ messages in thread

* Re: [RFC PATCH 09/12] mmc: sdhci: Use software timer when timeout greater than hardware capablility
  2018-01-11  8:46       ` Adrian Hunter
@ 2018-02-02 13:25         ` Kishon Vijay Abraham I
  0 siblings, 0 replies; 28+ messages in thread
From: Kishon Vijay Abraham I @ 2018-02-02 13:25 UTC (permalink / raw)
  To: Adrian Hunter, Ulf Hansson, Rob Herring, Tony Lindgren
  Cc: Mark Rutland, Russell King, linux-mmc, devicetree, linux-kernel,
	linux-omap, linux-arm-kernel, nsekhar

Hi Adrian,

On Thursday 11 January 2018 02:16 PM, Adrian Hunter wrote:
> On 04/01/18 14:59, Kishon Vijay Abraham I wrote:
>> Hi Adrian,
>>
>> On Wednesday 20 December 2017 07:41 PM, Adrian Hunter wrote:
>>> On 14/12/17 15:09, Kishon Vijay Abraham I wrote:
>>>> Errata i834 in AM572x Sitara Processors Silicon Revision 2.0, 1.1
>>>> (SPRZ429K July 2014–Revised March 2017 [1]) mentions
>>>> Under high speed HS200 and SDR104 modes, the functional clock for MMC
>>>> modules will reach up to 192 MHz. At this frequency, the maximum obtainable
>>>> timeout (DTO = 0xE) through MMC host controller is (1/192MHz)*2^27 = 700ms.
>>>> Commands taking longer than 700ms may be affected by this small window
>>>> frame. Workaround for this errata is use a software timer instead of
>>>> hardware timer to provide the delay requested by the upper layer.
>>>>
>>>> While this errata is specific to AM572x, it is applicable to all sdhci
>>>> based controllers when a particular request require timeout greater
>>>> than hardware capability.
>>>
>>> It doesn't work for our controllers.  Even if the data timeout interrupt is
>>> disabled, it seems like the timeout still "happens" in some fashion - after
>>> which the host controller starts misbehaving.
>>
>> even if the data timeout doesn't get disabled, count = 0xE is still present. So
>> ideally this shouldn't break any existing platforms no?
> 
> I don't want to hide this kind of variation in the hardware behaviour.
> 
>>>
>>> So you will need to add a quirk.
>>>
>>>>
>>>> Re-use the software timer already implemented in sdhci to program the
>>>> correct timeout value and also disable the hardware timeout when
>>>> the required timeout is greater than hardware capabiltiy in order to
>>>> avoid spurious timeout interrupts.
>>>>
>>>> This patch is based on the earlier patch implemented for omap_hsmmc [2]
>>>>
>>>> [1] -> http://www.ti.com/lit/er/sprz429k/sprz429k.pdf
>>>> [2] -> https://patchwork.kernel.org/patch/9791449/
>>>>
>>>> Signed-off-by: Kishon Vijay Abraham I <kishon@ti.com>
>>>> ---
>>>>  drivers/mmc/host/sdhci.c | 41 +++++++++++++++++++++++++++++++++++++++--
>>>>  drivers/mmc/host/sdhci.h | 11 +++++++++++
>>>>  2 files changed, 50 insertions(+), 2 deletions(-)
>>>>
>>>> diff --git a/drivers/mmc/host/sdhci.c b/drivers/mmc/host/sdhci.c
>>>> index e9290a3439d5..d0655e1d2cc7 100644
>>>> --- a/drivers/mmc/host/sdhci.c
>>>> +++ b/drivers/mmc/host/sdhci.c
>>>> @@ -673,6 +673,27 @@ 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_host *mmc = host->mmc;
>>>> +	struct mmc_ios *ios = &mmc->ios;
>>>> +	struct mmc_data *data = cmd->data;
>>>> +	unsigned long long transfer_time;
>>>> +
>>>> +	if (data) {
>>>> +		transfer_time = MMC_BLOCK_TRANSFER_TIME_MS(data->blksz,
>>>> +							   ios->bus_width,
>>>> +							   ios->clock);
>>>
>>> If it has a value, actual_clock is better than ios->clock.
>>
>> okay.
>>>
>>>> +		/* calculate timeout for the entire data */
>>>> +		host->data_timeout = (data->blocks * (target_timeout +
>>>> +						      transfer_time));
>>>> +	} else if (cmd->flags & MMC_RSP_BUSY) {
>>>> +		host->data_timeout = cmd->busy_timeout * MSEC_PER_SEC;
>>>
>>> Doesn't need MSEC_PER_SEC multiplier.
>>
>> right.
>>>
>>>> +	}
>>>> +}
>>>> +
>>>>  static u8 sdhci_calc_timeout(struct sdhci_host *host, struct mmc_command *cmd)
>>>>  {
>>>>  	u8 count;
>>>> @@ -732,8 +753,12 @@ static u8 sdhci_calc_timeout(struct sdhci_host *host, struct mmc_command *cmd)
>>>>  	}
>>>>  
>>>>  	if (count >= 0xF) {
>>>> -		DBG("Too large timeout 0x%x requested for CMD%d!\n",
>>>> -		    count, cmd->opcode);
>>>> +		DBG("Too large timeout.. using SW timeout for CMD%d!\n",
>>>> +		    cmd->opcode);
>>>> +		sdhci_calc_sw_timeout(host, cmd, target_timeout);
>>>> +		host->ier &= ~SDHCI_INT_DATA_TIMEOUT;
>>>> +		sdhci_writel(host, host->ier, SDHCI_INT_ENABLE);
>>>> +		sdhci_writel(host, host->ier, SDHCI_SIGNAL_ENABLE);
>>>>  		count = 0xE;
>>>>  	}
>>>>  
>>>> @@ -1198,6 +1223,14 @@ static void sdhci_finish_command(struct sdhci_host *host)
>>>>  {
>>>>  	struct mmc_command *cmd = host->cmd;
>>>>  
>>>> +	if (host->data_timeout) {
>>>> +		unsigned long timeout;
>>>> +
>>>> +		timeout = jiffies +
>>>> +			  msecs_to_jiffies(host->data_timeout);
>>>> +		sdhci_mod_timer(host, host->cmd->mrq, timeout);
>>>
>>> cmd could be the sbc or a stop cmd or a command during transfer, so this
>>> needs more logic.
>>
>> host->data_timeout gets set only for data commands or commands with busy
>> timeout. But I guess for commands during data transfer, host->data_timeout
>> might still be set?
>>
>> Checking sdhci_data_line_cmd(mrq->cmd) in addition to host->data_timeout should
>> take care of all cases right?
> 
> I suggest you make the timeout calculation allow for the commands as well
> and then reorder sdhci_mod_timer() to be called after sdhci_prepare_data()
> and make sdhci_mod_timer() do the right thing.

Since we don't know when exactly the command will be sent, the timeout
calculation might not be very accurate. Programming the timer in command
complete will be bit more accurate IMO. What do you think?

Thanks
Kishon

^ permalink raw reply	[flat|nested] 28+ messages in thread

end of thread, other threads:[~2018-02-02 13:25 UTC | newest]

Thread overview: 28+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2017-12-14 13:09 [PATCH 00/12] mmc: sdhci-omap: Add UHS/HS200 mode support Kishon Vijay Abraham I
2017-12-14 13:09 ` [PATCH 01/12] mmc: sdhci-omap: Update 'power_mode' outside sdhci_omap_init_74_clocks Kishon Vijay Abraham I
2017-12-21  8:57   ` Adrian Hunter
2017-12-14 13:09 ` [PATCH 02/12] mmc: sdhci-omap: Add card_busy host ops Kishon Vijay Abraham I
2017-12-21  8:59   ` Adrian Hunter
2017-12-14 13:09 ` [PATCH 03/12] mmc: sdhci-omap: Add custom set_uhs_signaling sdhci_host ops Kishon Vijay Abraham I
2017-12-21  9:01   ` Adrian Hunter
2017-12-14 13:09 ` [PATCH 04/12] mmc: sdhci-omap: Add tuning support Kishon Vijay Abraham I
2017-12-21  9:09   ` Adrian Hunter
     [not found] ` <20171214130941.26666-1-kishon-l0cyMroinI0@public.gmane.org>
2017-12-14 13:09   ` [PATCH 05/12] mmc: sdhci-omap: Workaround for Errata i802 Kishon Vijay Abraham I
2017-12-21  9:09     ` Adrian Hunter
2017-12-14 13:09 ` [PATCH 06/12] mmc: sdhci_omap: Add support to set IODELAY values Kishon Vijay Abraham I
     [not found]   ` <20171214130941.26666-7-kishon-l0cyMroinI0@public.gmane.org>
2017-12-14 15:04     ` Tony Lindgren
2017-12-14 13:09 ` [PATCH 07/12] mmc: sdhci_omap: Fix sdhci-omap quirks Kishon Vijay Abraham I
     [not found]   ` <20171214130941.26666-8-kishon-l0cyMroinI0@public.gmane.org>
2017-12-21  9:12     ` Adrian Hunter
2017-12-14 13:09 ` [PATCH 08/12] mmc: sdhci-omap: Add support to override f_max and iodelay from pdata Kishon Vijay Abraham I
2017-12-14 14:04   ` Philippe Ombredanne
     [not found]   ` <20171214130941.26666-9-kishon-l0cyMroinI0@public.gmane.org>
2017-12-21  9:13     ` Adrian Hunter
2017-12-14 13:09 ` [RFC PATCH 09/12] mmc: sdhci: Use software timer when timeout greater than hardware capablility Kishon Vijay Abraham I
2017-12-20 14:11   ` Adrian Hunter
2018-01-04 12:59     ` Kishon Vijay Abraham I
2018-01-11  8:46       ` Adrian Hunter
2018-02-02 13:25         ` Kishon Vijay Abraham I
2017-12-14 13:09 ` [PATCH 10/12] dt-bindings: sdhci-omap: Add K2G specific binding Kishon Vijay Abraham I
     [not found]   ` <20171214130941.26666-11-kishon-l0cyMroinI0@public.gmane.org>
2017-12-16 16:49     ` Rob Herring
2017-12-14 13:09 ` [PATCH 11/12] mmc: sdhci-omap: Add support for MMC/SD controller in k2g SoC Kishon Vijay Abraham I
2017-12-21  9:15   ` Adrian Hunter
2017-12-14 13:09 ` [PATCH 12/12] ARM: OMAP2+: Use sdhci-omap specific pdata-quirks for MMC/SD on DRA74x EVM Kishon Vijay Abraham I

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).