[PATCH v5 00/12] mmc: sdhci-msm: Add clk-rates, DDR, HS400 support

[PATCH v5 00/12] mmc: sdhci-msm: Add clk-rates, DDR, HS400 support

[Ritesh Harjani]

Hi, 

This is v5 version of the patch series with addition of HS400 patches in this.
These are tested on internal msm8996 & db410c HW.

Though there is a discussion going on w.r.t. clk-rates DT entry[6]
but I would like to get below patches reviewed, since these
patches should not be functionally impacted.

Changes from v4 -> v5 :-
1. Added HS400 sdhci-msm controller specific changes:- (Patch 10, 11, 12)
2. Addressed comment from Adrian on Patch 07 @[3].
3. Addressed comment from Arnd on Patch 03, to directly add
   clk_table into sdhci_msm_host. [4]
4. Addressed comment from Bjorn to not enforce having clk-rates property
   in DT for older targets based on discussion at [5]
5. Retained Acks from Adrian on patches (01 & 02 & 06) where there were no
   changes made while addressing above comments.


Older history:- 
This is v4 version of the patch series.
Patches 01, 02, 05 & 06 were Acked-by Adrian.

Changes from v3 -> v4 :-
1. Addressed comments from Adrian on Patch 03, 07, 08.
2. Addressed comments from Bjorn on Patch 03.
3. Added clk-rate support for sdhc DT nodes to all MSM platforms.
   in Pacth 04.
4. Rebased on next branch of Ulf.

Changes from v2 -> v3 :-
1. Addded Patch 01 based on Bjorn comment[2] - 
   This fixes/unrolls the poor coding style of read/writes of
   registers from base sdhci-msm driver.

2. Fixed/unrolled poor style of reads/writes of registers in Patch 02,
   based on Bjorn comment[2]. Also changed name of flag from
   use_updated_dll_reset -> use_14lpp_dll_reset.

Changes from v1->v2 :-
1. Removed patch 06 & 08 from v1 patch series[1]
(which were introducing unnecessary quirks).
   Instead have implemented __sdhci_msm_set_clock version of
   sdhci_set_clock in sdhci_msm driver itself in patch 07 of
   this patch series.
2. Enabled extra quirk (SDHCI_QUIRK2_PRESET_VALUE_BROKEN) in
   patch 05 of this patch series. 


Description of patches :-
This patchset adds clk-rates & other required changes to
upstream sdhci-msm driver from codeaurora tree.
It has been tested on a db410c Dragonboard and msm8996 based
platform.

Patch 0002 - Adds updated dll sequence for newer controllers
which has minor_version >= 0x42. This is required for msm8996.

MSM controller HW recommendation is to use the base MCI clock
and directly control this MCI clock at GCC in order to
change the clk-rate. 
Patches 03-07 bring in required change for this to
sdhci-msm and DT.

MSM controller would require 2x clock rate from source
for DDR bus speed modes. Patch 08 adds this support.

Patch 09 - adds DDR support in DT for sdhc1 of msm8916.


[1]:- http://www.spinics.net/lists/linux-mmc/msg38467.html
[2]:- http://www.spinics.net/lists/linux-mmc/msg38578.html 
[3]:- https://patchwork.kernel.org/patch/9289345/
[4]:- https://www.spinics.net/lists/linux-mmc/msg39107.html
[5]:- http://www.spinics.net/lists/linux-mmc/msg38749.html
[6]:- https://patchwork.kernel.org/patch/9297381/

Ritesh Harjani (9):
  mmc: sdhci-msm: Change poor style writel/readl of registers
  mmc: sdhci-msm: add pltfm_data support to get clk-rates from DT
  ARM: dts: qcom: Add clk-rates to sdhc1 & sdhc2
  mmc: sdhci-msm: Add get_min_clock() and get_max_clock() callback
  mmc: sdhci-msm: Enable few quirks
  mmc: sdhci-msm: Implement set_clock callback for sdhci-msm
  mmc: sdhci-msm: Add clock changes for DDR mode.
  arm64: dts: qcom: msm8916: Add ddr support to sdhc1
  sdhci: sdhci-msm: update dll configuration

Venkat Gopalakrishnan (3):
  mmc: sdhci-msm: Update DLL reset sequence
  mmc: sdhci-msm: Add HS400 platform support
  mmc: sdhci-msm: Add calibration tuning for CDCLP533 circuit

 .../devicetree/bindings/mmc/sdhci-msm.txt          |   1 +
 arch/arm/boot/dts/qcom-apq8084.dtsi                |   4 +
 arch/arm/boot/dts/qcom-msm8974.dtsi                |   4 +
 arch/arm64/boot/dts/qcom/msm8916.dtsi              |   5 +
 arch/arm64/boot/dts/qcom/msm8996.dtsi              |   2 +
 drivers/mmc/host/sdhci-msm.c                       | 721 ++++++++++++++++++++-
 6 files changed, 703 insertions(+), 34 deletions(-)

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

[PATCH v5 01/12] mmc: sdhci-msm: Change poor style writel/readl of registers

[Ritesh Harjani]

This patch changes the poor style of writel/readl registers
into more readable format. Also to avoid mixed style format
of readl/writel in sdhci-msm driver.

Signed-off-by: Ritesh Harjani <riteshh-sgV2jX0FEOL9JmXXK+q4OQ@public.gmane.org>
Acked-by: Adrian Hunter <adrian.hunter-ral2JQCrhuEAvxtiuMwx3w@public.gmane.org>
---
 drivers/mmc/host/sdhci-msm.c | 54 ++++++++++++++++++++++++++------------------
 1 file changed, 32 insertions(+), 22 deletions(-)

diff --git a/drivers/mmc/host/sdhci-msm.c b/drivers/mmc/host/sdhci-msm.c
index 8ef44a2a..42f42aa 100644
--- a/drivers/mmc/host/sdhci-msm.c
+++ b/drivers/mmc/host/sdhci-msm.c
@@ -137,8 +137,9 @@ static int msm_config_cm_dll_phase(struct sdhci_host *host, u8 phase)
 	writel_relaxed(config, host->ioaddr + CORE_DLL_CONFIG);
 
 	/* Set CK_OUT_EN bit of DLL_CONFIG register to 1. */
-	writel_relaxed((readl_relaxed(host->ioaddr + CORE_DLL_CONFIG)
-			| CORE_CK_OUT_EN), host->ioaddr + CORE_DLL_CONFIG);
+	config = readl_relaxed(host->ioaddr + CORE_DLL_CONFIG);
+	config |= CORE_CK_OUT_EN;
+	writel_relaxed(config, host->ioaddr + CORE_DLL_CONFIG);
 
 	/* Wait until CK_OUT_EN bit of DLL_CONFIG register becomes '1' */
 	rc = msm_dll_poll_ck_out_en(host, 1);
@@ -305,6 +306,7 @@ static int msm_init_cm_dll(struct sdhci_host *host)
 	struct mmc_host *mmc = host->mmc;
 	int wait_cnt = 50;
 	unsigned long flags;
+	u32 config = 0;
 
 	spin_lock_irqsave(&host->lock, flags);
 
@@ -313,33 +315,40 @@ static int msm_init_cm_dll(struct sdhci_host *host)
 	 * tuning is in progress. Keeping PWRSAVE ON may
 	 * turn off the clock.
 	 */
-	writel_relaxed((readl_relaxed(host->ioaddr + CORE_VENDOR_SPEC)
-			& ~CORE_CLK_PWRSAVE), host->ioaddr + CORE_VENDOR_SPEC);
+	config = readl_relaxed(host->ioaddr + CORE_VENDOR_SPEC);
+	config &= ~CORE_CLK_PWRSAVE;
+	writel_relaxed(config, host->ioaddr + CORE_VENDOR_SPEC);
 
 	/* Write 1 to DLL_RST bit of DLL_CONFIG register */
-	writel_relaxed((readl_relaxed(host->ioaddr + CORE_DLL_CONFIG)
-			| CORE_DLL_RST), host->ioaddr + CORE_DLL_CONFIG);
+	config = readl_relaxed(host->ioaddr + CORE_DLL_CONFIG);
+	config |= CORE_DLL_RST;
+	writel_relaxed(config, host->ioaddr + CORE_DLL_CONFIG);
 
 	/* Write 1 to DLL_PDN bit of DLL_CONFIG register */
-	writel_relaxed((readl_relaxed(host->ioaddr + CORE_DLL_CONFIG)
-			| CORE_DLL_PDN), host->ioaddr + CORE_DLL_CONFIG);
+	config = readl_relaxed(host->ioaddr + CORE_DLL_CONFIG);
+	config |= CORE_DLL_PDN;
+	writel_relaxed(config, host->ioaddr + CORE_DLL_CONFIG);
 	msm_cm_dll_set_freq(host);
 
 	/* Write 0 to DLL_RST bit of DLL_CONFIG register */
-	writel_relaxed((readl_relaxed(host->ioaddr + CORE_DLL_CONFIG)
-			& ~CORE_DLL_RST), host->ioaddr + CORE_DLL_CONFIG);
+	config = readl_relaxed(host->ioaddr + CORE_DLL_CONFIG);
+	config &= ~CORE_DLL_RST;
+	writel_relaxed(config, host->ioaddr + CORE_DLL_CONFIG);
 
 	/* Write 0 to DLL_PDN bit of DLL_CONFIG register */
-	writel_relaxed((readl_relaxed(host->ioaddr + CORE_DLL_CONFIG)
-			& ~CORE_DLL_PDN), host->ioaddr + CORE_DLL_CONFIG);
+	config = readl_relaxed(host->ioaddr + CORE_DLL_CONFIG);
+	config &= ~CORE_DLL_PDN;
+	writel_relaxed(config, host->ioaddr + CORE_DLL_CONFIG);
 
 	/* Set DLL_EN bit to 1. */
-	writel_relaxed((readl_relaxed(host->ioaddr + CORE_DLL_CONFIG)
-			| CORE_DLL_EN), host->ioaddr + CORE_DLL_CONFIG);
+	config = readl_relaxed(host->ioaddr + CORE_DLL_CONFIG);
+	config |= CORE_DLL_EN;
+	writel_relaxed(config, host->ioaddr + CORE_DLL_CONFIG);
 
 	/* Set CK_OUT_EN bit to 1. */
-	writel_relaxed((readl_relaxed(host->ioaddr + CORE_DLL_CONFIG)
-			| CORE_CK_OUT_EN), host->ioaddr + CORE_DLL_CONFIG);
+	config = readl_relaxed(host->ioaddr + CORE_DLL_CONFIG);
+	config |= CORE_CK_OUT_EN;
+	writel_relaxed(config, host->ioaddr + CORE_DLL_CONFIG);
 
 	/* Wait until DLL_LOCK bit of DLL_STATUS register becomes '1' */
 	while (!(readl_relaxed(host->ioaddr + CORE_DLL_STATUS) &
@@ -536,7 +545,7 @@ static int sdhci_msm_probe(struct platform_device *pdev)
 	struct resource *core_memres;
 	int ret;
 	u16 host_version, core_minor;
-	u32 core_version, caps;
+	u32 core_version, config;
 	u8 core_major;
 
 	host = sdhci_pltfm_init(pdev, &sdhci_msm_pdata, sizeof(*msm_host));
@@ -605,8 +614,9 @@ static int sdhci_msm_probe(struct platform_device *pdev)
 	}
 
 	/* Reset the core and Enable SDHC mode */
-	writel_relaxed(readl_relaxed(msm_host->core_mem + CORE_POWER) |
-		       CORE_SW_RST, msm_host->core_mem + CORE_POWER);
+	config = readl_relaxed(msm_host->core_mem + CORE_POWER);
+	config |= CORE_SW_RST;
+	writel_relaxed(config, msm_host->core_mem + CORE_POWER);
 
 	/* SW reset can take upto 10HCLK + 15MCLK cycles. (min 40us) */
 	usleep_range(1000, 5000);
@@ -636,9 +646,9 @@ static int sdhci_msm_probe(struct platform_device *pdev)
 	 * controller versions and must be explicitly enabled.
 	 */
 	if (core_major >= 1 && core_minor != 0x11 && core_minor != 0x12) {
-		caps = readl_relaxed(host->ioaddr + SDHCI_CAPABILITIES);
-		caps |= SDHCI_CAN_VDD_300 | SDHCI_CAN_DO_8BIT;
-		writel_relaxed(caps, host->ioaddr +
+		config = readl_relaxed(host->ioaddr + SDHCI_CAPABILITIES);
+		config |= SDHCI_CAN_VDD_300 | SDHCI_CAN_DO_8BIT;
+		writel_relaxed(config, host->ioaddr +
 			       CORE_VENDOR_SPEC_CAPABILITIES0);
 	}
 
-- 
The Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum, 
a Linux Foundation Collaborative Project.

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

[PATCH v5 02/12] mmc: sdhci-msm: Update DLL reset sequence

[Ritesh Harjani]

From: Venkat Gopalakrishnan <venkatg-sgV2jX0FEOL9JmXXK+q4OQ@public.gmane.org>

SDCC core with minor version >= 0x42 introduced new 14lpp
DLL. This has additional requirements in the reset sequence
for DLL tuning. Make necessary changes as needed.

Without this patch we see below errors on such SDHC controllers
	sdhci_msm 7464900.sdhci: mmc0: DLL failed to LOCK
	mmc0: tuning execution failed: -110

Signed-off-by: Venkat Gopalakrishnan <venkatg-sgV2jX0FEOL9JmXXK+q4OQ@public.gmane.org>
Signed-off-by: Ritesh Harjani <riteshh-sgV2jX0FEOL9JmXXK+q4OQ@public.gmane.org>
Acked-by: Adrian Hunter <adrian.hunter-ral2JQCrhuEAvxtiuMwx3w@public.gmane.org>
---
 drivers/mmc/host/sdhci-msm.c | 48 ++++++++++++++++++++++++++++++++++++++++++++
 1 file changed, 48 insertions(+)

diff --git a/drivers/mmc/host/sdhci-msm.c b/drivers/mmc/host/sdhci-msm.c
index 42f42aa..85ddaae 100644
--- a/drivers/mmc/host/sdhci-msm.c
+++ b/drivers/mmc/host/sdhci-msm.c
@@ -58,11 +58,17 @@
 #define CORE_DLL_CONFIG		0x100
 #define CORE_DLL_STATUS		0x108
 
+#define CORE_DLL_CONFIG_2	0x1b4
+#define CORE_FLL_CYCLE_CNT	BIT(18)
+#define CORE_DLL_CLOCK_DISABLE	BIT(21)
+
 #define CORE_VENDOR_SPEC	0x10c
 #define CORE_CLK_PWRSAVE	BIT(1)
 
 #define CORE_VENDOR_SPEC_CAPABILITIES0	0x11c
 
+#define TCXO_FREQ		19200000
+
 #define CDR_SELEXT_SHIFT	20
 #define CDR_SELEXT_MASK		(0xf << CDR_SELEXT_SHIFT)
 #define CMUX_SHIFT_PHASE_SHIFT	24
@@ -76,6 +82,7 @@ struct sdhci_msm_host {
 	struct clk *pclk;	/* SDHC peripheral bus clock */
 	struct clk *bus_clk;	/* SDHC bus voter clock */
 	struct mmc_host *mmc;
+	bool use_14lpp_dll_reset;
 };
 
 /* Platform specific tuning */
@@ -304,6 +311,8 @@ static inline void msm_cm_dll_set_freq(struct sdhci_host *host)
 static int msm_init_cm_dll(struct sdhci_host *host)
 {
 	struct mmc_host *mmc = host->mmc;
+	struct sdhci_pltfm_host *pltfm_host = sdhci_priv(host);
+	struct sdhci_msm_host *msm_host = sdhci_pltfm_priv(pltfm_host);
 	int wait_cnt = 50;
 	unsigned long flags;
 	u32 config = 0;
@@ -319,6 +328,16 @@ static int msm_init_cm_dll(struct sdhci_host *host)
 	config &= ~CORE_CLK_PWRSAVE;
 	writel_relaxed(config, host->ioaddr + CORE_VENDOR_SPEC);
 
+	if (msm_host->use_14lpp_dll_reset) {
+		config = readl_relaxed(host->ioaddr + CORE_DLL_CONFIG);
+		config &= ~CORE_CK_OUT_EN;
+		writel_relaxed(config, host->ioaddr + CORE_DLL_CONFIG);
+
+		config = readl_relaxed(host->ioaddr + CORE_DLL_CONFIG_2);
+		config |= CORE_DLL_CLOCK_DISABLE;
+		writel_relaxed(config, host->ioaddr + CORE_DLL_CONFIG_2);
+	}
+
 	/* Write 1 to DLL_RST bit of DLL_CONFIG register */
 	config = readl_relaxed(host->ioaddr + CORE_DLL_CONFIG);
 	config |= CORE_DLL_RST;
@@ -330,6 +349,24 @@ static int msm_init_cm_dll(struct sdhci_host *host)
 	writel_relaxed(config, host->ioaddr + CORE_DLL_CONFIG);
 	msm_cm_dll_set_freq(host);
 
+	if (msm_host->use_14lpp_dll_reset) {
+		u32 mclk_freq = 0;
+
+		if ((readl_relaxed(host->ioaddr + CORE_DLL_CONFIG_2)
+					& CORE_FLL_CYCLE_CNT))
+			mclk_freq = (u32) ((host->clock / TCXO_FREQ) * 8);
+		else
+			mclk_freq = (u32) ((host->clock / TCXO_FREQ) * 4);
+
+		config = readl_relaxed(host->ioaddr + CORE_DLL_CONFIG_2);
+		config &= ~(0xFF << 10);
+		config |= mclk_freq << 10;
+
+		writel_relaxed(config, host->ioaddr + CORE_DLL_CONFIG_2);
+		/* wait for 5us before enabling DLL clock */
+		udelay(5);
+	}
+
 	/* Write 0 to DLL_RST bit of DLL_CONFIG register */
 	config = readl_relaxed(host->ioaddr + CORE_DLL_CONFIG);
 	config &= ~CORE_DLL_RST;
@@ -340,6 +377,14 @@ static int msm_init_cm_dll(struct sdhci_host *host)
 	config &= ~CORE_DLL_PDN;
 	writel_relaxed(config, host->ioaddr + CORE_DLL_CONFIG);
 
+	if (msm_host->use_14lpp_dll_reset) {
+		msm_cm_dll_set_freq(host);
+		/* Enable the DLL clock */
+		config = readl_relaxed(host->ioaddr + CORE_DLL_CONFIG_2);
+		config &= ~CORE_DLL_CLOCK_DISABLE;
+		writel_relaxed(config, host->ioaddr + CORE_DLL_CONFIG_2);
+	}
+
 	/* Set DLL_EN bit to 1. */
 	config = readl_relaxed(host->ioaddr + CORE_DLL_CONFIG);
 	config |= CORE_DLL_EN;
@@ -641,6 +686,9 @@ static int sdhci_msm_probe(struct platform_device *pdev)
 	dev_dbg(&pdev->dev, "MCI Version: 0x%08x, major: 0x%04x, minor: 0x%02x\n",
 		core_version, core_major, core_minor);
 
+	if ((core_major == 1) && (core_minor >= 0x42))
+		msm_host->use_14lpp_dll_reset = true;
+
 	/*
 	 * Support for some capabilities is not advertised by newer
 	 * controller versions and must be explicitly enabled.
-- 
The Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum, 
a Linux Foundation Collaborative Project.

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

[PATCH v5 10/12] mmc: sdhci-msm: Add HS400 platform support

[Ritesh Harjani]

From: Venkat Gopalakrishnan <venkatg-sgV2jX0FEOL9JmXXK+q4OQ@public.gmane.org>

The following msm platform specific changes are added to support HS400.
- Allow tuning for HS400 mode.
- Configure HS400 timing mode using the VENDOR_SPECIFIC_FUNC register.

Signed-off-by: Venkat Gopalakrishnan <venkatg-sgV2jX0FEOL9JmXXK+q4OQ@public.gmane.org>
Signed-off-by: Ritesh Harjani <riteshh-sgV2jX0FEOL9JmXXK+q4OQ@public.gmane.org>
---
 drivers/mmc/host/sdhci-msm.c | 124 +++++++++++++++++++++++++++++++++++++++----
 1 file changed, 113 insertions(+), 11 deletions(-)

diff --git a/drivers/mmc/host/sdhci-msm.c b/drivers/mmc/host/sdhci-msm.c
index eb1a9e3..612fa82 100644
--- a/drivers/mmc/host/sdhci-msm.c
+++ b/drivers/mmc/host/sdhci-msm.c
@@ -31,6 +31,7 @@
 #define HC_MODE_EN		0x1
 #define CORE_POWER		0x0
 #define CORE_SW_RST		BIT(7)
+#define FF_CLK_SW_RST_DIS	BIT(13)
 
 #define CORE_PWRCTL_STATUS	0xdc
 #define CORE_PWRCTL_MASK	0xe0
@@ -64,11 +65,18 @@
 
 #define CORE_VENDOR_SPEC	0x10c
 #define CORE_CLK_PWRSAVE	BIT(1)
+#define CORE_HC_MCLK_SEL_DFLT	(2 << 8)
+#define CORE_HC_MCLK_SEL_HS400	(3 << 8)
+#define CORE_HC_MCLK_SEL_MASK	(3 << 8)
+#define CORE_HC_SELECT_IN_EN	(1 << 18)
+#define CORE_HC_SELECT_IN_HS400	(6 << 19)
+#define CORE_HC_SELECT_IN_MASK	(7 << 19)
 
 #define CORE_VENDOR_SPEC_CAPABILITIES0	0x11c
 
 #define TCXO_FREQ		19200000
 #define SDHCI_MSM_MIN_CLOCK	400000
+#define CORE_FREQ_100MHZ	(100 * 1000 * 1000)
 
 #define CDR_SELEXT_SHIFT	20
 #define CDR_SELEXT_MASK		(0xf << CDR_SELEXT_SHIFT)
@@ -87,6 +95,8 @@ struct sdhci_msm_host {
 	u32 clk_rate;
 	struct mmc_host *mmc;
 	bool use_14lpp_dll_reset;
+	bool tuning_done;
+	bool calibration_done;
 };
 
 /* Platform specific tuning */
@@ -175,8 +185,8 @@ out:
  * Find out the greatest range of consecuitive selected
  * DLL clock output phases that can be used as sampling
  * setting for SD3.0 UHS-I card read operation (in SDR104
- * timing mode) or for eMMC4.5 card read operation (in HS200
- * timing mode).
+ * timing mode) or for eMMC4.5 card read operation (in
+ * HS400/HS200 timing mode).
  * Select the 3/4 of the range and configure the DLL with the
  * selected DLL clock output phase.
  */
@@ -428,9 +438,10 @@ static int sdhci_msm_execute_tuning(struct sdhci_host *host, u32 opcode)
 	 * Tuning is required for SDR104, HS200 and HS400 cards and
 	 * if clock frequency is greater than 100MHz in these modes.
 	 */
-	if (host->clock <= 100 * 1000 * 1000 ||
-	    !((ios.timing == MMC_TIMING_MMC_HS200) ||
-	      (ios.timing == MMC_TIMING_UHS_SDR104)))
+	if (host->clock <= CORE_FREQ_100MHZ ||
+		!((ios.timing == MMC_TIMING_MMC_HS400) ||
+		(ios.timing == MMC_TIMING_MMC_HS200) ||
+		(ios.timing == MMC_TIMING_UHS_SDR104)))
 		return 0;
 
 retry:
@@ -488,7 +499,10 @@ static void sdhci_msm_set_uhs_signaling(struct sdhci_host *host,
 					unsigned int uhs)
 {
 	struct mmc_host *mmc = host->mmc;
+	struct sdhci_pltfm_host *pltfm_host = sdhci_priv(host);
+	struct sdhci_msm_host *msm_host = sdhci_pltfm_priv(pltfm_host);
 	u16 ctrl_2;
+	u32 config;
 
 	ctrl_2 = sdhci_readw(host, SDHCI_HOST_CONTROL2);
 	/* Select Bus Speed Mode for host */
@@ -503,6 +517,7 @@ static void sdhci_msm_set_uhs_signaling(struct sdhci_host *host,
 	case MMC_TIMING_UHS_SDR50:
 		ctrl_2 |= SDHCI_CTRL_UHS_SDR50;
 		break;
+	case MMC_TIMING_MMC_HS400:
 	case MMC_TIMING_MMC_HS200:
 	case MMC_TIMING_UHS_SDR104:
 		ctrl_2 |= SDHCI_CTRL_UHS_SDR104;
@@ -519,11 +534,33 @@ static void sdhci_msm_set_uhs_signaling(struct sdhci_host *host,
 	 * provide feedback clock, the mode selection can be any value less
 	 * than 3'b011 in bits [2:0] of HOST CONTROL2 register.
 	 */
-	if (host->clock <= 100000000 &&
-	    (uhs == MMC_TIMING_MMC_HS400 ||
-	     uhs == MMC_TIMING_MMC_HS200 ||
-	     uhs == MMC_TIMING_UHS_SDR104))
-		ctrl_2 &= ~SDHCI_CTRL_UHS_MASK;
+	if (host->clock <= CORE_FREQ_100MHZ) {
+		if ((uhs == MMC_TIMING_MMC_HS400) ||
+			(uhs == MMC_TIMING_MMC_HS200) ||
+			(uhs == MMC_TIMING_UHS_SDR104))
+			ctrl_2 &= ~SDHCI_CTRL_UHS_MASK;
+		/*
+		 * Make sure DLL is disabled when not required
+		 *
+		 * Write 1 to DLL_RST bit of DLL_CONFIG register
+		 */
+		config = readl_relaxed(host->ioaddr + CORE_DLL_CONFIG);
+		config |= CORE_DLL_RST;
+		writel_relaxed(config, host->ioaddr + CORE_DLL_CONFIG);
+
+		/* Write 1 to DLL_PDN bit of DLL_CONFIG register */
+		config = readl_relaxed(host->ioaddr + CORE_DLL_CONFIG);
+		config |= CORE_DLL_PDN;
+		writel_relaxed(config, host->ioaddr + CORE_DLL_CONFIG);
+
+		wmb(); /* drain writebuffer */
+
+		/*
+		 * The DLL needs to be restored and CDCLP533 recalibrated
+		 * when the clock frequency is set back to 400MHz.
+		 */
+		msm_host->calibration_done = false;
+	}
 
 	dev_dbg(mmc_dev(mmc), "%s: clock=%u uhs=%u ctrl_2=0x%x\n",
 		mmc_hostname(host->mmc), host->clock, uhs, ctrl_2);
@@ -670,7 +707,7 @@ static void sdhci_msm_set_clock(struct sdhci_host *host, unsigned int clock)
 	struct sdhci_pltfm_host *pltfm_host = sdhci_priv(host);
 	struct sdhci_msm_host *msm_host = sdhci_pltfm_priv(pltfm_host);
 	struct mmc_ios curr_ios = host->mmc->ios;
-	u32 msm_clock;
+	u32 msm_clock, config;
 	int rc;
 
 	if (!clock)
@@ -691,6 +728,66 @@ static void sdhci_msm_set_clock(struct sdhci_host *host, unsigned int clock)
 	}
 	msm_clock = sdhci_msm_get_msm_clk_rate(host, clock);
 
+	/*
+	 * In general all timing modes are controlled via UHS mode select in
+	 * Host Control2 register. eMMC specific HS200/HS400 doesn't have
+	 * their respective modes defined here, hence we use these values.
+	 *
+	 * HS200 - SDR104 (Since they both are equivalent in functionality)
+	 * HS400 - This involves multiple configurations
+	 *		Initially SDR104 - when tuning is required as HS200
+	 *		Then when switching to DDR @ 400MHz (HS400) we use
+	 *		the vendor specific HC_SELECT_IN to control the mode.
+	 *
+	 * In addition to controlling the modes we also need to select the
+	 * correct input clock for DLL depending on the mode.
+	 *
+	 * HS400 - divided clock (free running MCLK/2)
+	 * All other modes - default (free running MCLK)
+	 */
+	if (curr_ios.timing == MMC_TIMING_MMC_HS400) {
+		/* Select the divided clock (free running MCLK/2) */
+		config = readl_relaxed(host->ioaddr + CORE_VENDOR_SPEC);
+		config &= ~CORE_HC_MCLK_SEL_MASK;
+		config |= CORE_HC_MCLK_SEL_HS400;
+
+		writel_relaxed(config, host->ioaddr + CORE_VENDOR_SPEC);
+		/*
+		 * Select HS400 mode using the HC_SELECT_IN from VENDOR SPEC
+		 * register
+		 */
+		if (msm_host->tuning_done && !msm_host->calibration_done) {
+			/*
+			 * Write 0x6 to HC_SELECT_IN and 1 to HC_SELECT_IN_EN
+			 * field in VENDOR_SPEC_FUNC
+			 */
+			config = readl_relaxed(host->ioaddr + CORE_VENDOR_SPEC);
+			config |= CORE_HC_SELECT_IN_HS400;
+			config |= CORE_HC_SELECT_IN_EN;
+			writel_relaxed(config, host->ioaddr + CORE_VENDOR_SPEC);
+		}
+	} else {
+		/* Select the default clock (free running MCLK) */
+		config = readl_relaxed(host->ioaddr + CORE_VENDOR_SPEC);
+		config &= ~CORE_HC_MCLK_SEL_MASK;
+		config |= CORE_HC_MCLK_SEL_DFLT;
+		writel_relaxed(config, host->ioaddr + CORE_VENDOR_SPEC);
+
+		/*
+		 * Disable HC_SELECT_IN to be able to use the UHS mode select
+		 * configuration from Host Control2 register for all other
+		 * modes.
+		 *
+		 * Write 0 to HC_SELECT_IN and HC_SELECT_IN_EN field
+		 * in VENDOR_SPEC_FUNC
+		 */
+		config = readl_relaxed(host->ioaddr + CORE_VENDOR_SPEC);
+		config &= ~CORE_HC_SELECT_IN_EN;
+		config &= ~CORE_HC_SELECT_IN_MASK;
+		writel_relaxed(config, host->ioaddr + CORE_VENDOR_SPEC);
+	}
+	wmb(); /* drain writebuffer */
+
 	if ((msm_clock != msm_host->clk_rate) && msm_host->clk_table) {
 		rc = clk_set_rate(msm_host->clk, msm_clock);
 		if (rc) {
@@ -876,6 +973,11 @@ static int sdhci_msm_probe(struct platform_device *pdev)
 	/* Set HC_MODE_EN bit in HC_MODE register */
 	writel_relaxed(HC_MODE_EN, (msm_host->core_mem + CORE_HC_MODE));
 
+	/* Set FF_CLK_SW_RST_DIS bit in HC_MODE register */
+	config = readl_relaxed(msm_host->core_mem + CORE_HC_MODE);
+	config |= FF_CLK_SW_RST_DIS;
+	writel_relaxed(config, msm_host->core_mem + CORE_HC_MODE);
+
 	host_version = readw_relaxed((host->ioaddr + SDHCI_HOST_VERSION));
 	dev_dbg(&pdev->dev, "Host Version: 0x%x Vendor Version 0x%x\n",
 		host_version, ((host_version & SDHCI_VENDOR_VER_MASK) >>
-- 
The Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum, 
a Linux Foundation Collaborative Project.

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

Re: [PATCH v5 10/12] mmc: sdhci-msm: Add HS400 platform support

[Adrian Hunter]

On 05/10/16 17:40, Ritesh Harjani wrote:
> From: Venkat Gopalakrishnan <venkatg@codeaurora.org>
> 
> The following msm platform specific changes are added to support HS400.
> - Allow tuning for HS400 mode.
> - Configure HS400 timing mode using the VENDOR_SPECIFIC_FUNC register.
> 
> Signed-off-by: Venkat Gopalakrishnan <venkatg@codeaurora.org>
> Signed-off-by: Ritesh Harjani <riteshh@codeaurora.org>
> ---
>  drivers/mmc/host/sdhci-msm.c | 124 +++++++++++++++++++++++++++++++++++++++----
>  1 file changed, 113 insertions(+), 11 deletions(-)
> 
> diff --git a/drivers/mmc/host/sdhci-msm.c b/drivers/mmc/host/sdhci-msm.c
> index eb1a9e3..612fa82 100644
> --- a/drivers/mmc/host/sdhci-msm.c
> +++ b/drivers/mmc/host/sdhci-msm.c
> @@ -31,6 +31,7 @@
>  #define HC_MODE_EN		0x1
>  #define CORE_POWER		0x0
>  #define CORE_SW_RST		BIT(7)
> +#define FF_CLK_SW_RST_DIS	BIT(13)
>  
>  #define CORE_PWRCTL_STATUS	0xdc
>  #define CORE_PWRCTL_MASK	0xe0
> @@ -64,11 +65,18 @@
>  
>  #define CORE_VENDOR_SPEC	0x10c
>  #define CORE_CLK_PWRSAVE	BIT(1)
> +#define CORE_HC_MCLK_SEL_DFLT	(2 << 8)
> +#define CORE_HC_MCLK_SEL_HS400	(3 << 8)
> +#define CORE_HC_MCLK_SEL_MASK	(3 << 8)
> +#define CORE_HC_SELECT_IN_EN	(1 << 18)
> +#define CORE_HC_SELECT_IN_HS400	(6 << 19)
> +#define CORE_HC_SELECT_IN_MASK	(7 << 19)
>  
>  #define CORE_VENDOR_SPEC_CAPABILITIES0	0x11c
>  
>  #define TCXO_FREQ		19200000
>  #define SDHCI_MSM_MIN_CLOCK	400000
> +#define CORE_FREQ_100MHZ	(100 * 1000 * 1000)
>  
>  #define CDR_SELEXT_SHIFT	20
>  #define CDR_SELEXT_MASK		(0xf << CDR_SELEXT_SHIFT)
> @@ -87,6 +95,8 @@ struct sdhci_msm_host {
>  	u32 clk_rate;
>  	struct mmc_host *mmc;
>  	bool use_14lpp_dll_reset;
> +	bool tuning_done;
> +	bool calibration_done;
>  };
>  
>  /* Platform specific tuning */
> @@ -175,8 +185,8 @@ out:
>   * Find out the greatest range of consecuitive selected
>   * DLL clock output phases that can be used as sampling
>   * setting for SD3.0 UHS-I card read operation (in SDR104
> - * timing mode) or for eMMC4.5 card read operation (in HS200
> - * timing mode).
> + * timing mode) or for eMMC4.5 card read operation (in
> + * HS400/HS200 timing mode).
>   * Select the 3/4 of the range and configure the DLL with the
>   * selected DLL clock output phase.
>   */
> @@ -428,9 +438,10 @@ static int sdhci_msm_execute_tuning(struct sdhci_host *host, u32 opcode)
>  	 * Tuning is required for SDR104, HS200 and HS400 cards and
>  	 * if clock frequency is greater than 100MHz in these modes.
>  	 */
> -	if (host->clock <= 100 * 1000 * 1000 ||
> -	    !((ios.timing == MMC_TIMING_MMC_HS200) ||
> -	      (ios.timing == MMC_TIMING_UHS_SDR104)))
> +	if (host->clock <= CORE_FREQ_100MHZ ||
> +		!((ios.timing == MMC_TIMING_MMC_HS400) ||
> +		(ios.timing == MMC_TIMING_MMC_HS200) ||
> +		(ios.timing == MMC_TIMING_UHS_SDR104)))

Don't need () around ios.timing == MMC_TIMING_MMC_HS400 etc

>  		return 0;
>  
>  retry:
> @@ -488,7 +499,10 @@ static void sdhci_msm_set_uhs_signaling(struct sdhci_host *host,
>  					unsigned int uhs)
>  {
>  	struct mmc_host *mmc = host->mmc;
> +	struct sdhci_pltfm_host *pltfm_host = sdhci_priv(host);
> +	struct sdhci_msm_host *msm_host = sdhci_pltfm_priv(pltfm_host);
>  	u16 ctrl_2;
> +	u32 config;
>  
>  	ctrl_2 = sdhci_readw(host, SDHCI_HOST_CONTROL2);
>  	/* Select Bus Speed Mode for host */
> @@ -503,6 +517,7 @@ static void sdhci_msm_set_uhs_signaling(struct sdhci_host *host,
>  	case MMC_TIMING_UHS_SDR50:
>  		ctrl_2 |= SDHCI_CTRL_UHS_SDR50;
>  		break;
> +	case MMC_TIMING_MMC_HS400:
>  	case MMC_TIMING_MMC_HS200:
>  	case MMC_TIMING_UHS_SDR104:
>  		ctrl_2 |= SDHCI_CTRL_UHS_SDR104;
> @@ -519,11 +534,33 @@ static void sdhci_msm_set_uhs_signaling(struct sdhci_host *host,
>  	 * provide feedback clock, the mode selection can be any value less
>  	 * than 3'b011 in bits [2:0] of HOST CONTROL2 register.
>  	 */
> -	if (host->clock <= 100000000 &&
> -	    (uhs == MMC_TIMING_MMC_HS400 ||
> -	     uhs == MMC_TIMING_MMC_HS200 ||
> -	     uhs == MMC_TIMING_UHS_SDR104))
> -		ctrl_2 &= ~SDHCI_CTRL_UHS_MASK;
> +	if (host->clock <= CORE_FREQ_100MHZ) {
> +		if ((uhs == MMC_TIMING_MMC_HS400) ||
> +			(uhs == MMC_TIMING_MMC_HS200) ||
> +			(uhs == MMC_TIMING_UHS_SDR104))
> +			ctrl_2 &= ~SDHCI_CTRL_UHS_MASK;
> +		/*
> +		 * Make sure DLL is disabled when not required
> +		 *
> +		 * Write 1 to DLL_RST bit of DLL_CONFIG register
> +		 */
> +		config = readl_relaxed(host->ioaddr + CORE_DLL_CONFIG);
> +		config |= CORE_DLL_RST;
> +		writel_relaxed(config, host->ioaddr + CORE_DLL_CONFIG);
> +
> +		/* Write 1 to DLL_PDN bit of DLL_CONFIG register */
> +		config = readl_relaxed(host->ioaddr + CORE_DLL_CONFIG);
> +		config |= CORE_DLL_PDN;
> +		writel_relaxed(config, host->ioaddr + CORE_DLL_CONFIG);
> +
> +		wmb(); /* drain writebuffer */

Memory barrier could use more explanation.

> +
> +		/*
> +		 * The DLL needs to be restored and CDCLP533 recalibrated
> +		 * when the clock frequency is set back to 400MHz.
> +		 */
> +		msm_host->calibration_done = false;
> +	}
>  
>  	dev_dbg(mmc_dev(mmc), "%s: clock=%u uhs=%u ctrl_2=0x%x\n",
>  		mmc_hostname(host->mmc), host->clock, uhs, ctrl_2);
> @@ -670,7 +707,7 @@ static void sdhci_msm_set_clock(struct sdhci_host *host, unsigned int clock)
>  	struct sdhci_pltfm_host *pltfm_host = sdhci_priv(host);
>  	struct sdhci_msm_host *msm_host = sdhci_pltfm_priv(pltfm_host);
>  	struct mmc_ios curr_ios = host->mmc->ios;
> -	u32 msm_clock;
> +	u32 msm_clock, config;
>  	int rc;
>  
>  	if (!clock)
> @@ -691,6 +728,66 @@ static void sdhci_msm_set_clock(struct sdhci_host *host, unsigned int clock)
>  	}
>  	msm_clock = sdhci_msm_get_msm_clk_rate(host, clock);
>  
> +	/*
> +	 * In general all timing modes are controlled via UHS mode select in
> +	 * Host Control2 register. eMMC specific HS200/HS400 doesn't have
> +	 * their respective modes defined here, hence we use these values.
> +	 *
> +	 * HS200 - SDR104 (Since they both are equivalent in functionality)
> +	 * HS400 - This involves multiple configurations
> +	 *		Initially SDR104 - when tuning is required as HS200
> +	 *		Then when switching to DDR @ 400MHz (HS400) we use
> +	 *		the vendor specific HC_SELECT_IN to control the mode.
> +	 *
> +	 * In addition to controlling the modes we also need to select the
> +	 * correct input clock for DLL depending on the mode.
> +	 *
> +	 * HS400 - divided clock (free running MCLK/2)
> +	 * All other modes - default (free running MCLK)
> +	 */
> +	if (curr_ios.timing == MMC_TIMING_MMC_HS400) {
> +		/* Select the divided clock (free running MCLK/2) */
> +		config = readl_relaxed(host->ioaddr + CORE_VENDOR_SPEC);
> +		config &= ~CORE_HC_MCLK_SEL_MASK;
> +		config |= CORE_HC_MCLK_SEL_HS400;
> +
> +		writel_relaxed(config, host->ioaddr + CORE_VENDOR_SPEC);
> +		/*
> +		 * Select HS400 mode using the HC_SELECT_IN from VENDOR SPEC
> +		 * register
> +		 */
> +		if (msm_host->tuning_done && !msm_host->calibration_done) {


In this patch, tuning_done and calibration_done are never true.  Is that
intended?


> +			/*
> +			 * Write 0x6 to HC_SELECT_IN and 1 to HC_SELECT_IN_EN
> +			 * field in VENDOR_SPEC_FUNC
> +			 */
> +			config = readl_relaxed(host->ioaddr + CORE_VENDOR_SPEC);
> +			config |= CORE_HC_SELECT_IN_HS400;
> +			config |= CORE_HC_SELECT_IN_EN;
> +			writel_relaxed(config, host->ioaddr + CORE_VENDOR_SPEC);
> +		}
> +	} else {
> +		/* Select the default clock (free running MCLK) */
> +		config = readl_relaxed(host->ioaddr + CORE_VENDOR_SPEC);
> +		config &= ~CORE_HC_MCLK_SEL_MASK;
> +		config |= CORE_HC_MCLK_SEL_DFLT;
> +		writel_relaxed(config, host->ioaddr + CORE_VENDOR_SPEC);
> +
> +		/*
> +		 * Disable HC_SELECT_IN to be able to use the UHS mode select
> +		 * configuration from Host Control2 register for all other
> +		 * modes.
> +		 *
> +		 * Write 0 to HC_SELECT_IN and HC_SELECT_IN_EN field
> +		 * in VENDOR_SPEC_FUNC
> +		 */
> +		config = readl_relaxed(host->ioaddr + CORE_VENDOR_SPEC);
> +		config &= ~CORE_HC_SELECT_IN_EN;
> +		config &= ~CORE_HC_SELECT_IN_MASK;
> +		writel_relaxed(config, host->ioaddr + CORE_VENDOR_SPEC);
> +	}
> +	wmb(); /* drain writebuffer */

Memory barrier could use more explanation.

> +
>  	if ((msm_clock != msm_host->clk_rate) && msm_host->clk_table) {
>  		rc = clk_set_rate(msm_host->clk, msm_clock);
>  		if (rc) {
> @@ -876,6 +973,11 @@ static int sdhci_msm_probe(struct platform_device *pdev)
>  	/* Set HC_MODE_EN bit in HC_MODE register */
>  	writel_relaxed(HC_MODE_EN, (msm_host->core_mem + CORE_HC_MODE));
>  
> +	/* Set FF_CLK_SW_RST_DIS bit in HC_MODE register */
> +	config = readl_relaxed(msm_host->core_mem + CORE_HC_MODE);
> +	config |= FF_CLK_SW_RST_DIS;
> +	writel_relaxed(config, msm_host->core_mem + CORE_HC_MODE);
> +
>  	host_version = readw_relaxed((host->ioaddr + SDHCI_HOST_VERSION));
>  	dev_dbg(&pdev->dev, "Host Version: 0x%x Vendor Version 0x%x\n",
>  		host_version, ((host_version & SDHCI_VENDOR_VER_MASK) >>
> 

Re: [PATCH v5 10/12] mmc: sdhci-msm: Add HS400 platform support

[Ritesh Harjani]

Hi Adrian,

Thanks for the complete review.


On 10/10/2016 5:38 PM, Adrian Hunter wrote:
> On 05/10/16 17:40, Ritesh Harjani wrote:
>> From: Venkat Gopalakrishnan <venkatg@codeaurora.org>
>>
>> The following msm platform specific changes are added to support HS400.
>> - Allow tuning for HS400 mode.
>> - Configure HS400 timing mode using the VENDOR_SPECIFIC_FUNC register.
>>
>> Signed-off-by: Venkat Gopalakrishnan <venkatg@codeaurora.org>
>> Signed-off-by: Ritesh Harjani <riteshh@codeaurora.org>
>> ---
>>  drivers/mmc/host/sdhci-msm.c | 124 +++++++++++++++++++++++++++++++++++++++----
>>  1 file changed, 113 insertions(+), 11 deletions(-)
>>
>> diff --git a/drivers/mmc/host/sdhci-msm.c b/drivers/mmc/host/sdhci-msm.c
>> index eb1a9e3..612fa82 100644
>> --- a/drivers/mmc/host/sdhci-msm.c
>> +++ b/drivers/mmc/host/sdhci-msm.c
>> @@ -31,6 +31,7 @@
>>  #define HC_MODE_EN		0x1
>>  #define CORE_POWER		0x0
>>  #define CORE_SW_RST		BIT(7)
>> +#define FF_CLK_SW_RST_DIS	BIT(13)
>>
>>  #define CORE_PWRCTL_STATUS	0xdc
>>  #define CORE_PWRCTL_MASK	0xe0
>> @@ -64,11 +65,18 @@
>>
>>  #define CORE_VENDOR_SPEC	0x10c
>>  #define CORE_CLK_PWRSAVE	BIT(1)
>> +#define CORE_HC_MCLK_SEL_DFLT	(2 << 8)
>> +#define CORE_HC_MCLK_SEL_HS400	(3 << 8)
>> +#define CORE_HC_MCLK_SEL_MASK	(3 << 8)
>> +#define CORE_HC_SELECT_IN_EN	(1 << 18)
>> +#define CORE_HC_SELECT_IN_HS400	(6 << 19)
>> +#define CORE_HC_SELECT_IN_MASK	(7 << 19)
>>
>>  #define CORE_VENDOR_SPEC_CAPABILITIES0	0x11c
>>
>>  #define TCXO_FREQ		19200000
>>  #define SDHCI_MSM_MIN_CLOCK	400000
>> +#define CORE_FREQ_100MHZ	(100 * 1000 * 1000)
>>
>>  #define CDR_SELEXT_SHIFT	20
>>  #define CDR_SELEXT_MASK		(0xf << CDR_SELEXT_SHIFT)
>> @@ -87,6 +95,8 @@ struct sdhci_msm_host {
>>  	u32 clk_rate;
>>  	struct mmc_host *mmc;
>>  	bool use_14lpp_dll_reset;
>> +	bool tuning_done;
>> +	bool calibration_done;
>>  };
>>
>>  /* Platform specific tuning */
>> @@ -175,8 +185,8 @@ out:
>>   * Find out the greatest range of consecuitive selected
>>   * DLL clock output phases that can be used as sampling
>>   * setting for SD3.0 UHS-I card read operation (in SDR104
>> - * timing mode) or for eMMC4.5 card read operation (in HS200
>> - * timing mode).
>> + * timing mode) or for eMMC4.5 card read operation (in
>> + * HS400/HS200 timing mode).
>>   * Select the 3/4 of the range and configure the DLL with the
>>   * selected DLL clock output phase.
>>   */
>> @@ -428,9 +438,10 @@ static int sdhci_msm_execute_tuning(struct sdhci_host *host, u32 opcode)
>>  	 * Tuning is required for SDR104, HS200 and HS400 cards and
>>  	 * if clock frequency is greater than 100MHz in these modes.
>>  	 */
>> -	if (host->clock <= 100 * 1000 * 1000 ||
>> -	    !((ios.timing == MMC_TIMING_MMC_HS200) ||
>> -	      (ios.timing == MMC_TIMING_UHS_SDR104)))
>> +	if (host->clock <= CORE_FREQ_100MHZ ||
>> +		!((ios.timing == MMC_TIMING_MMC_HS400) ||
>> +		(ios.timing == MMC_TIMING_MMC_HS200) ||
>> +		(ios.timing == MMC_TIMING_UHS_SDR104)))
>
> Don't need () around ios.timing == MMC_TIMING_MMC_HS400 etc
Sure. Done.

>
>>  		return 0;
>>
>>  retry:
>> @@ -488,7 +499,10 @@ static void sdhci_msm_set_uhs_signaling(struct sdhci_host *host,
>>  					unsigned int uhs)
>>  {
>>  	struct mmc_host *mmc = host->mmc;
>> +	struct sdhci_pltfm_host *pltfm_host = sdhci_priv(host);
>> +	struct sdhci_msm_host *msm_host = sdhci_pltfm_priv(pltfm_host);
>>  	u16 ctrl_2;
>> +	u32 config;
>>
>>  	ctrl_2 = sdhci_readw(host, SDHCI_HOST_CONTROL2);
>>  	/* Select Bus Speed Mode for host */
>> @@ -503,6 +517,7 @@ static void sdhci_msm_set_uhs_signaling(struct sdhci_host *host,
>>  	case MMC_TIMING_UHS_SDR50:
>>  		ctrl_2 |= SDHCI_CTRL_UHS_SDR50;
>>  		break;
>> +	case MMC_TIMING_MMC_HS400:
>>  	case MMC_TIMING_MMC_HS200:
>>  	case MMC_TIMING_UHS_SDR104:
>>  		ctrl_2 |= SDHCI_CTRL_UHS_SDR104;
>> @@ -519,11 +534,33 @@ static void sdhci_msm_set_uhs_signaling(struct sdhci_host *host,
>>  	 * provide feedback clock, the mode selection can be any value less
>>  	 * than 3'b011 in bits [2:0] of HOST CONTROL2 register.
>>  	 */
>> -	if (host->clock <= 100000000 &&
>> -	    (uhs == MMC_TIMING_MMC_HS400 ||
>> -	     uhs == MMC_TIMING_MMC_HS200 ||
>> -	     uhs == MMC_TIMING_UHS_SDR104))
>> -		ctrl_2 &= ~SDHCI_CTRL_UHS_MASK;
>> +	if (host->clock <= CORE_FREQ_100MHZ) {
>> +		if ((uhs == MMC_TIMING_MMC_HS400) ||
>> +			(uhs == MMC_TIMING_MMC_HS200) ||
>> +			(uhs == MMC_TIMING_UHS_SDR104))
>> +			ctrl_2 &= ~SDHCI_CTRL_UHS_MASK;
>> +		/*
>> +		 * Make sure DLL is disabled when not required
>> +		 *
>> +		 * Write 1 to DLL_RST bit of DLL_CONFIG register
>> +		 */
>> +		config = readl_relaxed(host->ioaddr + CORE_DLL_CONFIG);
>> +		config |= CORE_DLL_RST;
>> +		writel_relaxed(config, host->ioaddr + CORE_DLL_CONFIG);
>> +
>> +		/* Write 1 to DLL_PDN bit of DLL_CONFIG register */
>> +		config = readl_relaxed(host->ioaddr + CORE_DLL_CONFIG);
>> +		config |= CORE_DLL_PDN;
>> +		writel_relaxed(config, host->ioaddr + CORE_DLL_CONFIG);
>> +
>> +		wmb(); /* drain writebuffer */
>
> Memory barrier could use more explanation.
Ok.

>
>> +
>> +		/*
>> +		 * The DLL needs to be restored and CDCLP533 recalibrated
>> +		 * when the clock frequency is set back to 400MHz.
>> +		 */
>> +		msm_host->calibration_done = false;
>> +	}
>>
>>  	dev_dbg(mmc_dev(mmc), "%s: clock=%u uhs=%u ctrl_2=0x%x\n",
>>  		mmc_hostname(host->mmc), host->clock, uhs, ctrl_2);
>> @@ -670,7 +707,7 @@ static void sdhci_msm_set_clock(struct sdhci_host *host, unsigned int clock)
>>  	struct sdhci_pltfm_host *pltfm_host = sdhci_priv(host);
>>  	struct sdhci_msm_host *msm_host = sdhci_pltfm_priv(pltfm_host);
>>  	struct mmc_ios curr_ios = host->mmc->ios;
>> -	u32 msm_clock;
>> +	u32 msm_clock, config;
>>  	int rc;
>>
>>  	if (!clock)
>> @@ -691,6 +728,66 @@ static void sdhci_msm_set_clock(struct sdhci_host *host, unsigned int clock)
>>  	}
>>  	msm_clock = sdhci_msm_get_msm_clk_rate(host, clock);
>>
>> +	/*
>> +	 * In general all timing modes are controlled via UHS mode select in
>> +	 * Host Control2 register. eMMC specific HS200/HS400 doesn't have
>> +	 * their respective modes defined here, hence we use these values.
>> +	 *
>> +	 * HS200 - SDR104 (Since they both are equivalent in functionality)
>> +	 * HS400 - This involves multiple configurations
>> +	 *		Initially SDR104 - when tuning is required as HS200
>> +	 *		Then when switching to DDR @ 400MHz (HS400) we use
>> +	 *		the vendor specific HC_SELECT_IN to control the mode.
>> +	 *
>> +	 * In addition to controlling the modes we also need to select the
>> +	 * correct input clock for DLL depending on the mode.
>> +	 *
>> +	 * HS400 - divided clock (free running MCLK/2)
>> +	 * All other modes - default (free running MCLK)
>> +	 */
>> +	if (curr_ios.timing == MMC_TIMING_MMC_HS400) {
>> +		/* Select the divided clock (free running MCLK/2) */
>> +		config = readl_relaxed(host->ioaddr + CORE_VENDOR_SPEC);
>> +		config &= ~CORE_HC_MCLK_SEL_MASK;
>> +		config |= CORE_HC_MCLK_SEL_HS400;
>> +
>> +		writel_relaxed(config, host->ioaddr + CORE_VENDOR_SPEC);
>> +		/*
>> +		 * Select HS400 mode using the HC_SELECT_IN from VENDOR SPEC
>> +		 * register
>> +		 */
>> +		if (msm_host->tuning_done && !msm_host->calibration_done) {
>
>
> In this patch, tuning_done and calibration_done are never true.  Is that
> intended?
Select HS400 which is done below should happen only after the tuning is 
performed. Hence the check of that variable above.
We set tuning_done = true in the next patch.

Please let me know if any concerns.

Similarly calibration_done is also getting set to true in the next patch.

>
>
>> +			/*
>> +			 * Write 0x6 to HC_SELECT_IN and 1 to HC_SELECT_IN_EN
>> +			 * field in VENDOR_SPEC_FUNC
>> +			 */
>> +			config = readl_relaxed(host->ioaddr + CORE_VENDOR_SPEC);
>> +			config |= CORE_HC_SELECT_IN_HS400;
>> +			config |= CORE_HC_SELECT_IN_EN;
>> +			writel_relaxed(config, host->ioaddr + CORE_VENDOR_SPEC);
>> +		}
>> +	} else {
>> +		/* Select the default clock (free running MCLK) */
>> +		config = readl_relaxed(host->ioaddr + CORE_VENDOR_SPEC);
>> +		config &= ~CORE_HC_MCLK_SEL_MASK;
>> +		config |= CORE_HC_MCLK_SEL_DFLT;
>> +		writel_relaxed(config, host->ioaddr + CORE_VENDOR_SPEC);
>> +
>> +		/*
>> +		 * Disable HC_SELECT_IN to be able to use the UHS mode select
>> +		 * configuration from Host Control2 register for all other
>> +		 * modes.
>> +		 *
>> +		 * Write 0 to HC_SELECT_IN and HC_SELECT_IN_EN field
>> +		 * in VENDOR_SPEC_FUNC
>> +		 */
>> +		config = readl_relaxed(host->ioaddr + CORE_VENDOR_SPEC);
>> +		config &= ~CORE_HC_SELECT_IN_EN;
>> +		config &= ~CORE_HC_SELECT_IN_MASK;
>> +		writel_relaxed(config, host->ioaddr + CORE_VENDOR_SPEC);
>> +	}
>> +	wmb(); /* drain writebuffer */
>
> Memory barrier could use more explanation.
Ok. Sure.

>
>> +
>>  	if ((msm_clock != msm_host->clk_rate) && msm_host->clk_table) {
>>  		rc = clk_set_rate(msm_host->clk, msm_clock);
>>  		if (rc) {
>> @@ -876,6 +973,11 @@ static int sdhci_msm_probe(struct platform_device *pdev)
>>  	/* Set HC_MODE_EN bit in HC_MODE register */
>>  	writel_relaxed(HC_MODE_EN, (msm_host->core_mem + CORE_HC_MODE));
>>
>> +	/* Set FF_CLK_SW_RST_DIS bit in HC_MODE register */
>> +	config = readl_relaxed(msm_host->core_mem + CORE_HC_MODE);
>> +	config |= FF_CLK_SW_RST_DIS;
>> +	writel_relaxed(config, msm_host->core_mem + CORE_HC_MODE);
>> +
>>  	host_version = readw_relaxed((host->ioaddr + SDHCI_HOST_VERSION));
>>  	dev_dbg(&pdev->dev, "Host Version: 0x%x Vendor Version 0x%x\n",
>>  		host_version, ((host_version & SDHCI_VENDOR_VER_MASK) >>
>>
>

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

[PATCH v5 11/12] mmc: sdhci-msm: Add calibration tuning for CDCLP533 circuit

[Ritesh Harjani]

From: Venkat Gopalakrishnan <venkatg-sgV2jX0FEOL9JmXXK+q4OQ@public.gmane.org>

In HS400 mode a new RCLK is introduced on the interface for read data
transfers. The eMMC5.0 device transmits the read data to the host with
respect to rising and falling edges of RCLK. In order to ensure correct
operation of read data transfers in HS400 mode, the incoming RX data
needs to be sampled by delayed version of RCLK.

The CDCLP533 delay circuit shifts the RCLK by T/4. It needs to be
initialized, configured and enabled once during HS400 mode switch and
when operational voltage/clock is changed.

Signed-off-by: Venkat Gopalakrishnan <venkatg-sgV2jX0FEOL9JmXXK+q4OQ@public.gmane.org>
Signed-off-by: Ritesh Harjani <riteshh-sgV2jX0FEOL9JmXXK+q4OQ@public.gmane.org>
---
 drivers/mmc/host/sdhci-msm.c | 178 +++++++++++++++++++++++++++++++++++++++++++
 1 file changed, 178 insertions(+)

diff --git a/drivers/mmc/host/sdhci-msm.c b/drivers/mmc/host/sdhci-msm.c
index 612fa82..dbf80a9c 100644
--- a/drivers/mmc/host/sdhci-msm.c
+++ b/drivers/mmc/host/sdhci-msm.c
@@ -57,6 +57,7 @@
 #define CORE_DLL_PDN		BIT(29)
 #define CORE_DLL_RST		BIT(30)
 #define CORE_DLL_CONFIG		0x100
+#define CORE_CMD_DAT_TRACK_SEL	BIT(0)
 #define CORE_DLL_STATUS		0x108
 
 #define CORE_DLL_CONFIG_2	0x1b4
@@ -72,8 +73,36 @@
 #define CORE_HC_SELECT_IN_HS400	(6 << 19)
 #define CORE_HC_SELECT_IN_MASK	(7 << 19)
 
+#define CORE_CSR_CDC_CTLR_CFG0		0x130
+#define CORE_SW_TRIG_FULL_CALIB		BIT(16)
+#define CORE_HW_AUTOCAL_ENA		BIT(17)
+
+#define CORE_CSR_CDC_CTLR_CFG1		0x134
+#define CORE_CSR_CDC_CAL_TIMER_CFG0	0x138
+#define CORE_TIMER_ENA			BIT(16)
+
+#define CORE_CSR_CDC_CAL_TIMER_CFG1	0x13C
+#define CORE_CSR_CDC_REFCOUNT_CFG	0x140
+#define CORE_CSR_CDC_COARSE_CAL_CFG	0x144
+#define CORE_CDC_OFFSET_CFG		0x14C
+#define CORE_CSR_CDC_DELAY_CFG		0x150
+#define CORE_CDC_SLAVE_DDA_CFG		0x160
+#define CORE_CSR_CDC_STATUS0		0x164
+#define CORE_CALIBRATION_DONE		BIT(0)
+
+#define CORE_CDC_ERROR_CODE_MASK	0x7000000
+
+#define CORE_CSR_CDC_GEN_CFG		0x178
+#define CORE_CDC_SWITCH_BYPASS_OFF	BIT(0)
+#define CORE_CDC_SWITCH_RC_EN		BIT(1)
+
+#define CORE_DDR_200_CFG		0x184
+#define CORE_CDC_T4_DLY_SEL		BIT(0)
+#define CORE_START_CDC_TRAFFIC		BIT(6)
+
 #define CORE_VENDOR_SPEC_CAPABILITIES0	0x11c
 
+#define INVALID_TUNING_PHASE	-1
 #define TCXO_FREQ		19200000
 #define SDHCI_MSM_MIN_CLOCK	400000
 #define CORE_FREQ_100MHZ	(100 * 1000 * 1000)
@@ -97,6 +126,7 @@ struct sdhci_msm_host {
 	bool use_14lpp_dll_reset;
 	bool tuning_done;
 	bool calibration_done;
+	u8 saved_tuning_phase;
 };
 
 /* Platform specific tuning */
@@ -426,6 +456,136 @@ static int msm_init_cm_dll(struct sdhci_host *host)
 	return 0;
 }
 
+static int sdhci_msm_cdclp533_calibration(struct sdhci_host *host)
+{
+	struct sdhci_pltfm_host *pltfm_host = sdhci_priv(host);
+	struct sdhci_msm_host *msm_host = sdhci_pltfm_priv(pltfm_host);
+	u32 wait_cnt, config;
+	int ret;
+
+	pr_debug("%s: Enter %s\n", mmc_hostname(host->mmc), __func__);
+
+	/*
+	 * Retuning in HS400 (DDR mode) will fail, just reset the
+	 * tuning block and restore the saved tuning phase.
+	 */
+	ret = msm_init_cm_dll(host);
+	if (ret)
+		goto out;
+
+	/* Set the selected phase in delay line hw block */
+	ret = msm_config_cm_dll_phase(host, msm_host->saved_tuning_phase);
+	if (ret)
+		goto out;
+
+	/* Write 1 to CMD_DAT_TRACK_SEL field in DLL_CONFIG */
+	config = readl_relaxed(host->ioaddr + CORE_DLL_CONFIG);
+	config |= CORE_CMD_DAT_TRACK_SEL;
+	writel_relaxed(config, host->ioaddr + CORE_DLL_CONFIG);
+
+	/* Write 0 to CDC_T4_DLY_SEL field in VENDOR_SPEC_DDR200_CFG */
+	config = readl_relaxed(host->ioaddr + CORE_DDR_200_CFG);
+	config &= ~CORE_CDC_T4_DLY_SEL;
+	writel_relaxed(config, host->ioaddr + CORE_DDR_200_CFG);
+
+	/* Write 0 to CDC_SWITCH_BYPASS_OFF field in CORE_CSR_CDC_GEN_CFG */
+	config = readl_relaxed(host->ioaddr + CORE_CSR_CDC_GEN_CFG);
+	config &= ~CORE_CDC_SWITCH_BYPASS_OFF;
+	writel_relaxed(config, host->ioaddr + CORE_CSR_CDC_GEN_CFG);
+
+	/* Write 1 to CDC_SWITCH_RC_EN field in CORE_CSR_CDC_GEN_CFG */
+	config = readl_relaxed(host->ioaddr + CORE_CSR_CDC_GEN_CFG);
+	config |= CORE_CDC_SWITCH_RC_EN;
+	writel_relaxed(config, host->ioaddr + CORE_CSR_CDC_GEN_CFG);
+
+	/* Write 0 to START_CDC_TRAFFIC field in CORE_DDR200_CFG */
+	config = readl_relaxed(host->ioaddr + CORE_DDR_200_CFG);
+	config &= ~CORE_START_CDC_TRAFFIC;
+	writel_relaxed(config, host->ioaddr + CORE_DDR_200_CFG);
+
+	/*
+	 * Perform CDC Register Initialization Sequence
+	 *
+	 * CORE_CSR_CDC_CTLR_CFG0	0x11800EC
+	 * CORE_CSR_CDC_CTLR_CFG1	0x3011111
+	 * CORE_CSR_CDC_CAL_TIMER_CFG0	0x1201000
+	 * CORE_CSR_CDC_CAL_TIMER_CFG1	0x4
+	 * CORE_CSR_CDC_REFCOUNT_CFG	0xCB732020
+	 * CORE_CSR_CDC_COARSE_CAL_CFG	0xB19
+	 * CORE_CSR_CDC_DELAY_CFG	0x3AC
+	 * CORE_CDC_OFFSET_CFG		0x0
+	 * CORE_CDC_SLAVE_DDA_CFG	0x16334
+	 */
+
+	writel_relaxed(0x11800EC, host->ioaddr + CORE_CSR_CDC_CTLR_CFG0);
+	writel_relaxed(0x3011111, host->ioaddr + CORE_CSR_CDC_CTLR_CFG1);
+	writel_relaxed(0x1201000, host->ioaddr + CORE_CSR_CDC_CAL_TIMER_CFG0);
+	writel_relaxed(0x4, host->ioaddr + CORE_CSR_CDC_CAL_TIMER_CFG1);
+	writel_relaxed(0xCB732020, host->ioaddr + CORE_CSR_CDC_REFCOUNT_CFG);
+	writel_relaxed(0xB19, host->ioaddr + CORE_CSR_CDC_COARSE_CAL_CFG);
+	writel_relaxed(0x3AC, host->ioaddr + CORE_CSR_CDC_DELAY_CFG);
+	writel_relaxed(0x0, host->ioaddr + CORE_CDC_OFFSET_CFG);
+	writel_relaxed(0x16334, host->ioaddr + CORE_CDC_SLAVE_DDA_CFG);
+
+	/* CDC HW Calibration */
+
+	/* Write 1 to SW_TRIG_FULL_CALIB field in CORE_CSR_CDC_CTLR_CFG0 */
+	config = readl_relaxed(host->ioaddr + CORE_CSR_CDC_CTLR_CFG0);
+	config |= CORE_SW_TRIG_FULL_CALIB;
+	writel_relaxed(config, host->ioaddr + CORE_CSR_CDC_CTLR_CFG0);
+
+	/* Write 0 to SW_TRIG_FULL_CALIB field in CORE_CSR_CDC_CTLR_CFG0 */
+	config = readl_relaxed(host->ioaddr + CORE_CSR_CDC_CTLR_CFG0);
+	config &= ~CORE_SW_TRIG_FULL_CALIB;
+	writel_relaxed(config, host->ioaddr + CORE_CSR_CDC_CTLR_CFG0);
+
+	/* Write 1 to HW_AUTOCAL_ENA field in CORE_CSR_CDC_CTLR_CFG0 */
+	config = readl_relaxed(host->ioaddr + CORE_CSR_CDC_CTLR_CFG0);
+	config |= CORE_HW_AUTOCAL_ENA;
+	writel_relaxed(config, host->ioaddr + CORE_CSR_CDC_CTLR_CFG0);
+
+	/* Write 1 to TIMER_ENA field in CORE_CSR_CDC_CAL_TIMER_CFG0 */
+	config = readl_relaxed(host->ioaddr + CORE_CSR_CDC_CAL_TIMER_CFG0);
+	config |= CORE_TIMER_ENA;
+	writel_relaxed(config, host->ioaddr + CORE_CSR_CDC_CAL_TIMER_CFG0);
+
+	wmb(); /* drain writebuffer */
+
+	/* Poll on CALIBRATION_DONE field in CORE_CSR_CDC_STATUS0 to be 1 */
+	wait_cnt = 50;
+	while (!(readl_relaxed(host->ioaddr + CORE_CSR_CDC_STATUS0)
+			& CORE_CALIBRATION_DONE)) {
+		/* max. wait for 50us sec for CALIBRATION_DONE bit to be set */
+		if (--wait_cnt == 0) {
+			pr_err("%s: %s: CDC Calibration was not completed\n",
+				mmc_hostname(host->mmc), __func__);
+			ret = -ETIMEDOUT;
+			goto out;
+		}
+		/* wait for 1us before polling again */
+		udelay(1);
+	}
+
+	/* Verify CDC_ERROR_CODE field in CORE_CSR_CDC_STATUS0 is 0 */
+	ret = readl_relaxed(host->ioaddr + CORE_CSR_CDC_STATUS0)
+			& CORE_CDC_ERROR_CODE_MASK;
+	if (ret) {
+		pr_err("%s: %s: CDC Error Code %d\n",
+			mmc_hostname(host->mmc), __func__, ret);
+		ret = -EINVAL;
+		goto out;
+	}
+
+	/* Write 1 to START_CDC_TRAFFIC field in CORE_DDR200_CFG */
+	config = readl_relaxed(host->ioaddr + CORE_DDR_200_CFG);
+	config |= CORE_START_CDC_TRAFFIC;
+	writel_relaxed(config, host->ioaddr + CORE_DDR_200_CFG);
+out:
+	pr_debug("%s: Exit %s, ret:%d\n", mmc_hostname(host->mmc),
+			__func__, ret);
+	return ret;
+}
+
 static int sdhci_msm_execute_tuning(struct sdhci_host *host, u32 opcode)
 {
 	int tuning_seq_cnt = 3;
@@ -433,6 +593,8 @@ static int sdhci_msm_execute_tuning(struct sdhci_host *host, u32 opcode)
 	int rc;
 	struct mmc_host *mmc = host->mmc;
 	struct mmc_ios ios = host->mmc->ios;
+	struct sdhci_pltfm_host *pltfm_host = sdhci_priv(host);
+	struct sdhci_msm_host *msm_host = sdhci_pltfm_priv(pltfm_host);
 
 	/*
 	 * Tuning is required for SDR104, HS200 and HS400 cards and
@@ -457,6 +619,7 @@ retry:
 		if (rc)
 			return rc;
 
+		msm_host->saved_tuning_phase = phase;
 		rc = mmc_send_tuning(mmc, opcode, NULL);
 		if (!rc) {
 			/* Tuning is successful at this tuning point */
@@ -492,6 +655,8 @@ retry:
 		rc = -EIO;
 	}
 
+	if (!rc)
+		msm_host->tuning_done = true;
 	return rc;
 }
 
@@ -565,6 +730,17 @@ static void sdhci_msm_set_uhs_signaling(struct sdhci_host *host,
 	dev_dbg(mmc_dev(mmc), "%s: clock=%u uhs=%u ctrl_2=0x%x\n",
 		mmc_hostname(host->mmc), host->clock, uhs, ctrl_2);
 	sdhci_writew(host, ctrl_2, SDHCI_HOST_CONTROL2);
+
+	spin_unlock_irq(&host->lock);
+
+	/* CDCLP533 HW calibration is only required for HS400 mode*/
+	if (host->clock > CORE_FREQ_100MHZ &&
+	   msm_host->tuning_done && !msm_host->calibration_done &&
+	   (mmc->ios.timing == MMC_TIMING_MMC_HS400))
+		if (!sdhci_msm_cdclp533_calibration(host))
+			msm_host->calibration_done = true;
+
+	spin_lock_irq(&host->lock);
 }
 
 static void sdhci_msm_voltage_switch(struct sdhci_host *host)
@@ -907,6 +1083,8 @@ static int sdhci_msm_probe(struct platform_device *pdev)
 
 	sdhci_msm_populate_dt(&pdev->dev, msm_host);
 
+	msm_host->saved_tuning_phase = INVALID_TUNING_PHASE;
+
 	/* Setup SDCC bus voter clock. */
 	msm_host->bus_clk = devm_clk_get(&pdev->dev, "bus");
 	if (!IS_ERR(msm_host->bus_clk)) {
-- 
The Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum, 
a Linux Foundation Collaborative Project.

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

Re: [PATCH v5 11/12] mmc: sdhci-msm: Add calibration tuning for CDCLP533 circuit

[Adrian Hunter]

On 05/10/16 17:40, Ritesh Harjani wrote:
> From: Venkat Gopalakrishnan <venkatg@codeaurora.org>
> 
> In HS400 mode a new RCLK is introduced on the interface for read data
> transfers. The eMMC5.0 device transmits the read data to the host with
> respect to rising and falling edges of RCLK. In order to ensure correct
> operation of read data transfers in HS400 mode, the incoming RX data
> needs to be sampled by delayed version of RCLK.
> 
> The CDCLP533 delay circuit shifts the RCLK by T/4. It needs to be
> initialized, configured and enabled once during HS400 mode switch and
> when operational voltage/clock is changed.
> 
> Signed-off-by: Venkat Gopalakrishnan <venkatg@codeaurora.org>
> Signed-off-by: Ritesh Harjani <riteshh@codeaurora.org>
> ---
>  drivers/mmc/host/sdhci-msm.c | 178 +++++++++++++++++++++++++++++++++++++++++++
>  1 file changed, 178 insertions(+)
> 
> diff --git a/drivers/mmc/host/sdhci-msm.c b/drivers/mmc/host/sdhci-msm.c
> index 612fa82..dbf80a9c 100644
> --- a/drivers/mmc/host/sdhci-msm.c
> +++ b/drivers/mmc/host/sdhci-msm.c
> @@ -57,6 +57,7 @@
>  #define CORE_DLL_PDN		BIT(29)
>  #define CORE_DLL_RST		BIT(30)
>  #define CORE_DLL_CONFIG		0x100
> +#define CORE_CMD_DAT_TRACK_SEL	BIT(0)
>  #define CORE_DLL_STATUS		0x108
>  
>  #define CORE_DLL_CONFIG_2	0x1b4
> @@ -72,8 +73,36 @@
>  #define CORE_HC_SELECT_IN_HS400	(6 << 19)
>  #define CORE_HC_SELECT_IN_MASK	(7 << 19)
>  
> +#define CORE_CSR_CDC_CTLR_CFG0		0x130
> +#define CORE_SW_TRIG_FULL_CALIB		BIT(16)
> +#define CORE_HW_AUTOCAL_ENA		BIT(17)
> +
> +#define CORE_CSR_CDC_CTLR_CFG1		0x134
> +#define CORE_CSR_CDC_CAL_TIMER_CFG0	0x138
> +#define CORE_TIMER_ENA			BIT(16)
> +
> +#define CORE_CSR_CDC_CAL_TIMER_CFG1	0x13C
> +#define CORE_CSR_CDC_REFCOUNT_CFG	0x140
> +#define CORE_CSR_CDC_COARSE_CAL_CFG	0x144
> +#define CORE_CDC_OFFSET_CFG		0x14C
> +#define CORE_CSR_CDC_DELAY_CFG		0x150
> +#define CORE_CDC_SLAVE_DDA_CFG		0x160
> +#define CORE_CSR_CDC_STATUS0		0x164
> +#define CORE_CALIBRATION_DONE		BIT(0)
> +
> +#define CORE_CDC_ERROR_CODE_MASK	0x7000000
> +
> +#define CORE_CSR_CDC_GEN_CFG		0x178
> +#define CORE_CDC_SWITCH_BYPASS_OFF	BIT(0)
> +#define CORE_CDC_SWITCH_RC_EN		BIT(1)
> +
> +#define CORE_DDR_200_CFG		0x184
> +#define CORE_CDC_T4_DLY_SEL		BIT(0)
> +#define CORE_START_CDC_TRAFFIC		BIT(6)
> +
>  #define CORE_VENDOR_SPEC_CAPABILITIES0	0x11c
>  
> +#define INVALID_TUNING_PHASE	-1
>  #define TCXO_FREQ		19200000
>  #define SDHCI_MSM_MIN_CLOCK	400000
>  #define CORE_FREQ_100MHZ	(100 * 1000 * 1000)
> @@ -97,6 +126,7 @@ struct sdhci_msm_host {
>  	bool use_14lpp_dll_reset;
>  	bool tuning_done;
>  	bool calibration_done;
> +	u8 saved_tuning_phase;
>  };
>  
>  /* Platform specific tuning */
> @@ -426,6 +456,136 @@ static int msm_init_cm_dll(struct sdhci_host *host)
>  	return 0;
>  }
>  
> +static int sdhci_msm_cdclp533_calibration(struct sdhci_host *host)
> +{
> +	struct sdhci_pltfm_host *pltfm_host = sdhci_priv(host);
> +	struct sdhci_msm_host *msm_host = sdhci_pltfm_priv(pltfm_host);
> +	u32 wait_cnt, config;
> +	int ret;
> +
> +	pr_debug("%s: Enter %s\n", mmc_hostname(host->mmc), __func__);
> +
> +	/*
> +	 * Retuning in HS400 (DDR mode) will fail, just reset the
> +	 * tuning block and restore the saved tuning phase.
> +	 */
> +	ret = msm_init_cm_dll(host);
> +	if (ret)
> +		goto out;
> +
> +	/* Set the selected phase in delay line hw block */
> +	ret = msm_config_cm_dll_phase(host, msm_host->saved_tuning_phase);
> +	if (ret)
> +		goto out;
> +
> +	/* Write 1 to CMD_DAT_TRACK_SEL field in DLL_CONFIG */
> +	config = readl_relaxed(host->ioaddr + CORE_DLL_CONFIG);
> +	config |= CORE_CMD_DAT_TRACK_SEL;
> +	writel_relaxed(config, host->ioaddr + CORE_DLL_CONFIG);
> +
> +	/* Write 0 to CDC_T4_DLY_SEL field in VENDOR_SPEC_DDR200_CFG */
> +	config = readl_relaxed(host->ioaddr + CORE_DDR_200_CFG);
> +	config &= ~CORE_CDC_T4_DLY_SEL;
> +	writel_relaxed(config, host->ioaddr + CORE_DDR_200_CFG);
> +
> +	/* Write 0 to CDC_SWITCH_BYPASS_OFF field in CORE_CSR_CDC_GEN_CFG */
> +	config = readl_relaxed(host->ioaddr + CORE_CSR_CDC_GEN_CFG);
> +	config &= ~CORE_CDC_SWITCH_BYPASS_OFF;
> +	writel_relaxed(config, host->ioaddr + CORE_CSR_CDC_GEN_CFG);
> +
> +	/* Write 1 to CDC_SWITCH_RC_EN field in CORE_CSR_CDC_GEN_CFG */
> +	config = readl_relaxed(host->ioaddr + CORE_CSR_CDC_GEN_CFG);
> +	config |= CORE_CDC_SWITCH_RC_EN;
> +	writel_relaxed(config, host->ioaddr + CORE_CSR_CDC_GEN_CFG);
> +
> +	/* Write 0 to START_CDC_TRAFFIC field in CORE_DDR200_CFG */
> +	config = readl_relaxed(host->ioaddr + CORE_DDR_200_CFG);
> +	config &= ~CORE_START_CDC_TRAFFIC;
> +	writel_relaxed(config, host->ioaddr + CORE_DDR_200_CFG);
> +
> +	/*
> +	 * Perform CDC Register Initialization Sequence
> +	 *
> +	 * CORE_CSR_CDC_CTLR_CFG0	0x11800EC
> +	 * CORE_CSR_CDC_CTLR_CFG1	0x3011111
> +	 * CORE_CSR_CDC_CAL_TIMER_CFG0	0x1201000
> +	 * CORE_CSR_CDC_CAL_TIMER_CFG1	0x4
> +	 * CORE_CSR_CDC_REFCOUNT_CFG	0xCB732020
> +	 * CORE_CSR_CDC_COARSE_CAL_CFG	0xB19
> +	 * CORE_CSR_CDC_DELAY_CFG	0x3AC
> +	 * CORE_CDC_OFFSET_CFG		0x0
> +	 * CORE_CDC_SLAVE_DDA_CFG	0x16334
> +	 */
> +
> +	writel_relaxed(0x11800EC, host->ioaddr + CORE_CSR_CDC_CTLR_CFG0);
> +	writel_relaxed(0x3011111, host->ioaddr + CORE_CSR_CDC_CTLR_CFG1);
> +	writel_relaxed(0x1201000, host->ioaddr + CORE_CSR_CDC_CAL_TIMER_CFG0);
> +	writel_relaxed(0x4, host->ioaddr + CORE_CSR_CDC_CAL_TIMER_CFG1);
> +	writel_relaxed(0xCB732020, host->ioaddr + CORE_CSR_CDC_REFCOUNT_CFG);
> +	writel_relaxed(0xB19, host->ioaddr + CORE_CSR_CDC_COARSE_CAL_CFG);
> +	writel_relaxed(0x3AC, host->ioaddr + CORE_CSR_CDC_DELAY_CFG);
> +	writel_relaxed(0x0, host->ioaddr + CORE_CDC_OFFSET_CFG);
> +	writel_relaxed(0x16334, host->ioaddr + CORE_CDC_SLAVE_DDA_CFG);
> +
> +	/* CDC HW Calibration */
> +
> +	/* Write 1 to SW_TRIG_FULL_CALIB field in CORE_CSR_CDC_CTLR_CFG0 */
> +	config = readl_relaxed(host->ioaddr + CORE_CSR_CDC_CTLR_CFG0);
> +	config |= CORE_SW_TRIG_FULL_CALIB;
> +	writel_relaxed(config, host->ioaddr + CORE_CSR_CDC_CTLR_CFG0);
> +
> +	/* Write 0 to SW_TRIG_FULL_CALIB field in CORE_CSR_CDC_CTLR_CFG0 */
> +	config = readl_relaxed(host->ioaddr + CORE_CSR_CDC_CTLR_CFG0);
> +	config &= ~CORE_SW_TRIG_FULL_CALIB;
> +	writel_relaxed(config, host->ioaddr + CORE_CSR_CDC_CTLR_CFG0);
> +
> +	/* Write 1 to HW_AUTOCAL_ENA field in CORE_CSR_CDC_CTLR_CFG0 */
> +	config = readl_relaxed(host->ioaddr + CORE_CSR_CDC_CTLR_CFG0);
> +	config |= CORE_HW_AUTOCAL_ENA;
> +	writel_relaxed(config, host->ioaddr + CORE_CSR_CDC_CTLR_CFG0);
> +
> +	/* Write 1 to TIMER_ENA field in CORE_CSR_CDC_CAL_TIMER_CFG0 */
> +	config = readl_relaxed(host->ioaddr + CORE_CSR_CDC_CAL_TIMER_CFG0);
> +	config |= CORE_TIMER_ENA;
> +	writel_relaxed(config, host->ioaddr + CORE_CSR_CDC_CAL_TIMER_CFG0);
> +
> +	wmb(); /* drain writebuffer */
> +
> +	/* Poll on CALIBRATION_DONE field in CORE_CSR_CDC_STATUS0 to be 1 */
> +	wait_cnt = 50;
> +	while (!(readl_relaxed(host->ioaddr + CORE_CSR_CDC_STATUS0)
> +			& CORE_CALIBRATION_DONE)) {
> +		/* max. wait for 50us sec for CALIBRATION_DONE bit to be set */
> +		if (--wait_cnt == 0) {
> +			pr_err("%s: %s: CDC Calibration was not completed\n",
> +				mmc_hostname(host->mmc), __func__);
> +			ret = -ETIMEDOUT;
> +			goto out;
> +		}
> +		/* wait for 1us before polling again */
> +		udelay(1);
> +	}
> +
> +	/* Verify CDC_ERROR_CODE field in CORE_CSR_CDC_STATUS0 is 0 */
> +	ret = readl_relaxed(host->ioaddr + CORE_CSR_CDC_STATUS0)
> +			& CORE_CDC_ERROR_CODE_MASK;
> +	if (ret) {
> +		pr_err("%s: %s: CDC Error Code %d\n",
> +			mmc_hostname(host->mmc), __func__, ret);
> +		ret = -EINVAL;
> +		goto out;
> +	}
> +
> +	/* Write 1 to START_CDC_TRAFFIC field in CORE_DDR200_CFG */
> +	config = readl_relaxed(host->ioaddr + CORE_DDR_200_CFG);
> +	config |= CORE_START_CDC_TRAFFIC;
> +	writel_relaxed(config, host->ioaddr + CORE_DDR_200_CFG);
> +out:
> +	pr_debug("%s: Exit %s, ret:%d\n", mmc_hostname(host->mmc),
> +			__func__, ret);
> +	return ret;
> +}
> +
>  static int sdhci_msm_execute_tuning(struct sdhci_host *host, u32 opcode)
>  {
>  	int tuning_seq_cnt = 3;
> @@ -433,6 +593,8 @@ static int sdhci_msm_execute_tuning(struct sdhci_host *host, u32 opcode)
>  	int rc;
>  	struct mmc_host *mmc = host->mmc;
>  	struct mmc_ios ios = host->mmc->ios;
> +	struct sdhci_pltfm_host *pltfm_host = sdhci_priv(host);
> +	struct sdhci_msm_host *msm_host = sdhci_pltfm_priv(pltfm_host);
>  
>  	/*
>  	 * Tuning is required for SDR104, HS200 and HS400 cards and
> @@ -457,6 +619,7 @@ retry:
>  		if (rc)
>  			return rc;
>  
> +		msm_host->saved_tuning_phase = phase;
>  		rc = mmc_send_tuning(mmc, opcode, NULL);
>  		if (!rc) {
>  			/* Tuning is successful at this tuning point */
> @@ -492,6 +655,8 @@ retry:
>  		rc = -EIO;
>  	}
>  
> +	if (!rc)
> +		msm_host->tuning_done = true;
>  	return rc;
>  }
>  
> @@ -565,6 +730,17 @@ static void sdhci_msm_set_uhs_signaling(struct sdhci_host *host,
>  	dev_dbg(mmc_dev(mmc), "%s: clock=%u uhs=%u ctrl_2=0x%x\n",
>  		mmc_hostname(host->mmc), host->clock, uhs, ctrl_2);
>  	sdhci_writew(host, ctrl_2, SDHCI_HOST_CONTROL2);
> +
> +	spin_unlock_irq(&host->lock);
> +
> +	/* CDCLP533 HW calibration is only required for HS400 mode*/
> +	if (host->clock > CORE_FREQ_100MHZ &&
> +	   msm_host->tuning_done && !msm_host->calibration_done &&
> +	   (mmc->ios.timing == MMC_TIMING_MMC_HS400))
> +		if (!sdhci_msm_cdclp533_calibration(host))
> +			msm_host->calibration_done = true;
> +
> +	spin_lock_irq(&host->lock);
>  }
>  
>  static void sdhci_msm_voltage_switch(struct sdhci_host *host)
> @@ -907,6 +1083,8 @@ static int sdhci_msm_probe(struct platform_device *pdev)
>  
>  	sdhci_msm_populate_dt(&pdev->dev, msm_host);
>  
> +	msm_host->saved_tuning_phase = INVALID_TUNING_PHASE;

There is never a check for INVALID_TUNING_PHASE which begs the question: why
have it?

> +
>  	/* Setup SDCC bus voter clock. */
>  	msm_host->bus_clk = devm_clk_get(&pdev->dev, "bus");
>  	if (!IS_ERR(msm_host->bus_clk)) {
> 

Re: [PATCH v5 11/12] mmc: sdhci-msm: Add calibration tuning for CDCLP533 circuit

[Ritesh Harjani]

Hi Adrian,

On 10/10/2016 6:19 PM, Adrian Hunter wrote:
> On 05/10/16 17:40, Ritesh Harjani wrote:
>> From: Venkat Gopalakrishnan <venkatg-sgV2jX0FEOL9JmXXK+q4OQ@public.gmane.org>
>>
>> In HS400 mode a new RCLK is introduced on the interface for read data
>> transfers. The eMMC5.0 device transmits the read data to the host with
>> respect to rising and falling edges of RCLK. In order to ensure correct
>> operation of read data transfers in HS400 mode, the incoming RX data
>> needs to be sampled by delayed version of RCLK.
>>
>> The CDCLP533 delay circuit shifts the RCLK by T/4. It needs to be
>> initialized, configured and enabled once during HS400 mode switch and
>> when operational voltage/clock is changed.
>>
>> Signed-off-by: Venkat Gopalakrishnan <venkatg-sgV2jX0FEOL9JmXXK+q4OQ@public.gmane.org>
>> Signed-off-by: Ritesh Harjani <riteshh-sgV2jX0FEOL9JmXXK+q4OQ@public.gmane.org>
>> ---
>>  drivers/mmc/host/sdhci-msm.c | 178 +++++++++++++++++++++++++++++++++++++++++++
>>  1 file changed, 178 insertions(+)
>>
>> diff --git a/drivers/mmc/host/sdhci-msm.c b/drivers/mmc/host/sdhci-msm.c
>> index 612fa82..dbf80a9c 100644
>> --- a/drivers/mmc/host/sdhci-msm.c
>> +++ b/drivers/mmc/host/sdhci-msm.c
>> @@ -57,6 +57,7 @@
>>  #define CORE_DLL_PDN		BIT(29)
>>  #define CORE_DLL_RST		BIT(30)
>>  #define CORE_DLL_CONFIG		0x100
>> +#define CORE_CMD_DAT_TRACK_SEL	BIT(0)
>>  #define CORE_DLL_STATUS		0x108
>>
>>  #define CORE_DLL_CONFIG_2	0x1b4
>> @@ -72,8 +73,36 @@
>>  #define CORE_HC_SELECT_IN_HS400	(6 << 19)
>>  #define CORE_HC_SELECT_IN_MASK	(7 << 19)
>>
>> +#define CORE_CSR_CDC_CTLR_CFG0		0x130
>> +#define CORE_SW_TRIG_FULL_CALIB		BIT(16)
>> +#define CORE_HW_AUTOCAL_ENA		BIT(17)
>> +
>> +#define CORE_CSR_CDC_CTLR_CFG1		0x134
>> +#define CORE_CSR_CDC_CAL_TIMER_CFG0	0x138
>> +#define CORE_TIMER_ENA			BIT(16)
>> +
>> +#define CORE_CSR_CDC_CAL_TIMER_CFG1	0x13C
>> +#define CORE_CSR_CDC_REFCOUNT_CFG	0x140
>> +#define CORE_CSR_CDC_COARSE_CAL_CFG	0x144
>> +#define CORE_CDC_OFFSET_CFG		0x14C
>> +#define CORE_CSR_CDC_DELAY_CFG		0x150
>> +#define CORE_CDC_SLAVE_DDA_CFG		0x160
>> +#define CORE_CSR_CDC_STATUS0		0x164
>> +#define CORE_CALIBRATION_DONE		BIT(0)
>> +
>> +#define CORE_CDC_ERROR_CODE_MASK	0x7000000
>> +
>> +#define CORE_CSR_CDC_GEN_CFG		0x178
>> +#define CORE_CDC_SWITCH_BYPASS_OFF	BIT(0)
>> +#define CORE_CDC_SWITCH_RC_EN		BIT(1)
>> +
>> +#define CORE_DDR_200_CFG		0x184
>> +#define CORE_CDC_T4_DLY_SEL		BIT(0)
>> +#define CORE_START_CDC_TRAFFIC		BIT(6)
>> +
>>  #define CORE_VENDOR_SPEC_CAPABILITIES0	0x11c
>>
>> +#define INVALID_TUNING_PHASE	-1
>>  #define TCXO_FREQ		19200000
>>  #define SDHCI_MSM_MIN_CLOCK	400000
>>  #define CORE_FREQ_100MHZ	(100 * 1000 * 1000)
>> @@ -97,6 +126,7 @@ struct sdhci_msm_host {
>>  	bool use_14lpp_dll_reset;
>>  	bool tuning_done;
>>  	bool calibration_done;
>> +	u8 saved_tuning_phase;
>>  };
>>
>>  /* Platform specific tuning */
>> @@ -426,6 +456,136 @@ static int msm_init_cm_dll(struct sdhci_host *host)
>>  	return 0;
>>  }
>>
>> +static int sdhci_msm_cdclp533_calibration(struct sdhci_host *host)
>> +{
>> +	struct sdhci_pltfm_host *pltfm_host = sdhci_priv(host);
>> +	struct sdhci_msm_host *msm_host = sdhci_pltfm_priv(pltfm_host);
>> +	u32 wait_cnt, config;
>> +	int ret;
>> +
>> +	pr_debug("%s: Enter %s\n", mmc_hostname(host->mmc), __func__);
>> +
>> +	/*
>> +	 * Retuning in HS400 (DDR mode) will fail, just reset the
>> +	 * tuning block and restore the saved tuning phase.
>> +	 */
>> +	ret = msm_init_cm_dll(host);
>> +	if (ret)
>> +		goto out;
>> +
>> +	/* Set the selected phase in delay line hw block */
>> +	ret = msm_config_cm_dll_phase(host, msm_host->saved_tuning_phase);
>> +	if (ret)
>> +		goto out;
>> +
>> +	/* Write 1 to CMD_DAT_TRACK_SEL field in DLL_CONFIG */
>> +	config = readl_relaxed(host->ioaddr + CORE_DLL_CONFIG);
>> +	config |= CORE_CMD_DAT_TRACK_SEL;
>> +	writel_relaxed(config, host->ioaddr + CORE_DLL_CONFIG);
>> +
>> +	/* Write 0 to CDC_T4_DLY_SEL field in VENDOR_SPEC_DDR200_CFG */
>> +	config = readl_relaxed(host->ioaddr + CORE_DDR_200_CFG);
>> +	config &= ~CORE_CDC_T4_DLY_SEL;
>> +	writel_relaxed(config, host->ioaddr + CORE_DDR_200_CFG);
>> +
>> +	/* Write 0 to CDC_SWITCH_BYPASS_OFF field in CORE_CSR_CDC_GEN_CFG */
>> +	config = readl_relaxed(host->ioaddr + CORE_CSR_CDC_GEN_CFG);
>> +	config &= ~CORE_CDC_SWITCH_BYPASS_OFF;
>> +	writel_relaxed(config, host->ioaddr + CORE_CSR_CDC_GEN_CFG);
>> +
>> +	/* Write 1 to CDC_SWITCH_RC_EN field in CORE_CSR_CDC_GEN_CFG */
>> +	config = readl_relaxed(host->ioaddr + CORE_CSR_CDC_GEN_CFG);
>> +	config |= CORE_CDC_SWITCH_RC_EN;
>> +	writel_relaxed(config, host->ioaddr + CORE_CSR_CDC_GEN_CFG);
>> +
>> +	/* Write 0 to START_CDC_TRAFFIC field in CORE_DDR200_CFG */
>> +	config = readl_relaxed(host->ioaddr + CORE_DDR_200_CFG);
>> +	config &= ~CORE_START_CDC_TRAFFIC;
>> +	writel_relaxed(config, host->ioaddr + CORE_DDR_200_CFG);
>> +
>> +	/*
>> +	 * Perform CDC Register Initialization Sequence
>> +	 *
>> +	 * CORE_CSR_CDC_CTLR_CFG0	0x11800EC
>> +	 * CORE_CSR_CDC_CTLR_CFG1	0x3011111
>> +	 * CORE_CSR_CDC_CAL_TIMER_CFG0	0x1201000
>> +	 * CORE_CSR_CDC_CAL_TIMER_CFG1	0x4
>> +	 * CORE_CSR_CDC_REFCOUNT_CFG	0xCB732020
>> +	 * CORE_CSR_CDC_COARSE_CAL_CFG	0xB19
>> +	 * CORE_CSR_CDC_DELAY_CFG	0x3AC
>> +	 * CORE_CDC_OFFSET_CFG		0x0
>> +	 * CORE_CDC_SLAVE_DDA_CFG	0x16334
>> +	 */
>> +
>> +	writel_relaxed(0x11800EC, host->ioaddr + CORE_CSR_CDC_CTLR_CFG0);
>> +	writel_relaxed(0x3011111, host->ioaddr + CORE_CSR_CDC_CTLR_CFG1);
>> +	writel_relaxed(0x1201000, host->ioaddr + CORE_CSR_CDC_CAL_TIMER_CFG0);
>> +	writel_relaxed(0x4, host->ioaddr + CORE_CSR_CDC_CAL_TIMER_CFG1);
>> +	writel_relaxed(0xCB732020, host->ioaddr + CORE_CSR_CDC_REFCOUNT_CFG);
>> +	writel_relaxed(0xB19, host->ioaddr + CORE_CSR_CDC_COARSE_CAL_CFG);
>> +	writel_relaxed(0x3AC, host->ioaddr + CORE_CSR_CDC_DELAY_CFG);
>> +	writel_relaxed(0x0, host->ioaddr + CORE_CDC_OFFSET_CFG);
>> +	writel_relaxed(0x16334, host->ioaddr + CORE_CDC_SLAVE_DDA_CFG);
>> +
>> +	/* CDC HW Calibration */
>> +
>> +	/* Write 1 to SW_TRIG_FULL_CALIB field in CORE_CSR_CDC_CTLR_CFG0 */
>> +	config = readl_relaxed(host->ioaddr + CORE_CSR_CDC_CTLR_CFG0);
>> +	config |= CORE_SW_TRIG_FULL_CALIB;
>> +	writel_relaxed(config, host->ioaddr + CORE_CSR_CDC_CTLR_CFG0);
>> +
>> +	/* Write 0 to SW_TRIG_FULL_CALIB field in CORE_CSR_CDC_CTLR_CFG0 */
>> +	config = readl_relaxed(host->ioaddr + CORE_CSR_CDC_CTLR_CFG0);
>> +	config &= ~CORE_SW_TRIG_FULL_CALIB;
>> +	writel_relaxed(config, host->ioaddr + CORE_CSR_CDC_CTLR_CFG0);
>> +
>> +	/* Write 1 to HW_AUTOCAL_ENA field in CORE_CSR_CDC_CTLR_CFG0 */
>> +	config = readl_relaxed(host->ioaddr + CORE_CSR_CDC_CTLR_CFG0);
>> +	config |= CORE_HW_AUTOCAL_ENA;
>> +	writel_relaxed(config, host->ioaddr + CORE_CSR_CDC_CTLR_CFG0);
>> +
>> +	/* Write 1 to TIMER_ENA field in CORE_CSR_CDC_CAL_TIMER_CFG0 */
>> +	config = readl_relaxed(host->ioaddr + CORE_CSR_CDC_CAL_TIMER_CFG0);
>> +	config |= CORE_TIMER_ENA;
>> +	writel_relaxed(config, host->ioaddr + CORE_CSR_CDC_CAL_TIMER_CFG0);
>> +
>> +	wmb(); /* drain writebuffer */
>> +
>> +	/* Poll on CALIBRATION_DONE field in CORE_CSR_CDC_STATUS0 to be 1 */
>> +	wait_cnt = 50;
>> +	while (!(readl_relaxed(host->ioaddr + CORE_CSR_CDC_STATUS0)
>> +			& CORE_CALIBRATION_DONE)) {
>> +		/* max. wait for 50us sec for CALIBRATION_DONE bit to be set */
>> +		if (--wait_cnt == 0) {
>> +			pr_err("%s: %s: CDC Calibration was not completed\n",
>> +				mmc_hostname(host->mmc), __func__);
>> +			ret = -ETIMEDOUT;
>> +			goto out;
>> +		}
>> +		/* wait for 1us before polling again */
>> +		udelay(1);
>> +	}
>> +
>> +	/* Verify CDC_ERROR_CODE field in CORE_CSR_CDC_STATUS0 is 0 */
>> +	ret = readl_relaxed(host->ioaddr + CORE_CSR_CDC_STATUS0)
>> +			& CORE_CDC_ERROR_CODE_MASK;
>> +	if (ret) {
>> +		pr_err("%s: %s: CDC Error Code %d\n",
>> +			mmc_hostname(host->mmc), __func__, ret);
>> +		ret = -EINVAL;
>> +		goto out;
>> +	}
>> +
>> +	/* Write 1 to START_CDC_TRAFFIC field in CORE_DDR200_CFG */
>> +	config = readl_relaxed(host->ioaddr + CORE_DDR_200_CFG);
>> +	config |= CORE_START_CDC_TRAFFIC;
>> +	writel_relaxed(config, host->ioaddr + CORE_DDR_200_CFG);
>> +out:
>> +	pr_debug("%s: Exit %s, ret:%d\n", mmc_hostname(host->mmc),
>> +			__func__, ret);
>> +	return ret;
>> +}
>> +
>>  static int sdhci_msm_execute_tuning(struct sdhci_host *host, u32 opcode)
>>  {
>>  	int tuning_seq_cnt = 3;
>> @@ -433,6 +593,8 @@ static int sdhci_msm_execute_tuning(struct sdhci_host *host, u32 opcode)
>>  	int rc;
>>  	struct mmc_host *mmc = host->mmc;
>>  	struct mmc_ios ios = host->mmc->ios;
>> +	struct sdhci_pltfm_host *pltfm_host = sdhci_priv(host);
>> +	struct sdhci_msm_host *msm_host = sdhci_pltfm_priv(pltfm_host);
>>
>>  	/*
>>  	 * Tuning is required for SDR104, HS200 and HS400 cards and
>> @@ -457,6 +619,7 @@ retry:
>>  		if (rc)
>>  			return rc;
>>
>> +		msm_host->saved_tuning_phase = phase;
>>  		rc = mmc_send_tuning(mmc, opcode, NULL);
>>  		if (!rc) {
>>  			/* Tuning is successful at this tuning point */
>> @@ -492,6 +655,8 @@ retry:
>>  		rc = -EIO;
>>  	}
>>
>> +	if (!rc)
>> +		msm_host->tuning_done = true;
>>  	return rc;
>>  }
>>
>> @@ -565,6 +730,17 @@ static void sdhci_msm_set_uhs_signaling(struct sdhci_host *host,
>>  	dev_dbg(mmc_dev(mmc), "%s: clock=%u uhs=%u ctrl_2=0x%x\n",
>>  		mmc_hostname(host->mmc), host->clock, uhs, ctrl_2);
>>  	sdhci_writew(host, ctrl_2, SDHCI_HOST_CONTROL2);
>> +
>> +	spin_unlock_irq(&host->lock);
>> +
>> +	/* CDCLP533 HW calibration is only required for HS400 mode*/
>> +	if (host->clock > CORE_FREQ_100MHZ &&
>> +	   msm_host->tuning_done && !msm_host->calibration_done &&
>> +	   (mmc->ios.timing == MMC_TIMING_MMC_HS400))
>> +		if (!sdhci_msm_cdclp533_calibration(host))
>> +			msm_host->calibration_done = true;
>> +
>> +	spin_lock_irq(&host->lock);
>>  }
>>
>>  static void sdhci_msm_voltage_switch(struct sdhci_host *host)
>> @@ -907,6 +1083,8 @@ static int sdhci_msm_probe(struct platform_device *pdev)
>>
>>  	sdhci_msm_populate_dt(&pdev->dev, msm_host);
>>
>> +	msm_host->saved_tuning_phase = INVALID_TUNING_PHASE;
>
> There is never a check for INVALID_TUNING_PHASE which begs the question: why
> have it?
phase value can be between 0x0 to 0xf. So during probe 
saved_tuning_phase is getting initialized with -1 value.

Let me know if any concern, we can remove it as well.
But wont it look incorrect if we initialize it with some valid phase 
value before even tuning is completed?

>
>> +
>>  	/* Setup SDCC bus voter clock. */
>>  	msm_host->bus_clk = devm_clk_get(&pdev->dev, "bus");
>>  	if (!IS_ERR(msm_host->bus_clk)) {
>>
>
--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH v5 11/12] mmc: sdhci-msm: Add calibration tuning for CDCLP533 circuit

[Adrian Hunter]

On 10/10/16 18:42, Ritesh Harjani wrote:
> Hi Adrian,
> 
> On 10/10/2016 6:19 PM, Adrian Hunter wrote:
>> On 05/10/16 17:40, Ritesh Harjani wrote:
>>> From: Venkat Gopalakrishnan <venkatg-sgV2jX0FEOL9JmXXK+q4OQ@public.gmane.org>
>>>
>>> In HS400 mode a new RCLK is introduced on the interface for read data
>>> transfers. The eMMC5.0 device transmits the read data to the host with
>>> respect to rising and falling edges of RCLK. In order to ensure correct
>>> operation of read data transfers in HS400 mode, the incoming RX data
>>> needs to be sampled by delayed version of RCLK.
>>>
>>> The CDCLP533 delay circuit shifts the RCLK by T/4. It needs to be
>>> initialized, configured and enabled once during HS400 mode switch and
>>> when operational voltage/clock is changed.
>>>
>>> Signed-off-by: Venkat Gopalakrishnan <venkatg-sgV2jX0FEOL9JmXXK+q4OQ@public.gmane.org>
>>> Signed-off-by: Ritesh Harjani <riteshh-sgV2jX0FEOL9JmXXK+q4OQ@public.gmane.org>
>>> ---
>>>  drivers/mmc/host/sdhci-msm.c | 178
>>> +++++++++++++++++++++++++++++++++++++++++++
>>>  1 file changed, 178 insertions(+)
>>>
>>> diff --git a/drivers/mmc/host/sdhci-msm.c b/drivers/mmc/host/sdhci-msm.c
>>> index 612fa82..dbf80a9c 100644
>>> --- a/drivers/mmc/host/sdhci-msm.c
>>> +++ b/drivers/mmc/host/sdhci-msm.c
>>> @@ -57,6 +57,7 @@
>>>  #define CORE_DLL_PDN        BIT(29)
>>>  #define CORE_DLL_RST        BIT(30)
>>>  #define CORE_DLL_CONFIG        0x100
>>> +#define CORE_CMD_DAT_TRACK_SEL    BIT(0)
>>>  #define CORE_DLL_STATUS        0x108
>>>
>>>  #define CORE_DLL_CONFIG_2    0x1b4
>>> @@ -72,8 +73,36 @@
>>>  #define CORE_HC_SELECT_IN_HS400    (6 << 19)
>>>  #define CORE_HC_SELECT_IN_MASK    (7 << 19)
>>>
>>> +#define CORE_CSR_CDC_CTLR_CFG0        0x130
>>> +#define CORE_SW_TRIG_FULL_CALIB        BIT(16)
>>> +#define CORE_HW_AUTOCAL_ENA        BIT(17)
>>> +
>>> +#define CORE_CSR_CDC_CTLR_CFG1        0x134
>>> +#define CORE_CSR_CDC_CAL_TIMER_CFG0    0x138
>>> +#define CORE_TIMER_ENA            BIT(16)
>>> +
>>> +#define CORE_CSR_CDC_CAL_TIMER_CFG1    0x13C
>>> +#define CORE_CSR_CDC_REFCOUNT_CFG    0x140
>>> +#define CORE_CSR_CDC_COARSE_CAL_CFG    0x144
>>> +#define CORE_CDC_OFFSET_CFG        0x14C
>>> +#define CORE_CSR_CDC_DELAY_CFG        0x150
>>> +#define CORE_CDC_SLAVE_DDA_CFG        0x160
>>> +#define CORE_CSR_CDC_STATUS0        0x164
>>> +#define CORE_CALIBRATION_DONE        BIT(0)
>>> +
>>> +#define CORE_CDC_ERROR_CODE_MASK    0x7000000
>>> +
>>> +#define CORE_CSR_CDC_GEN_CFG        0x178
>>> +#define CORE_CDC_SWITCH_BYPASS_OFF    BIT(0)
>>> +#define CORE_CDC_SWITCH_RC_EN        BIT(1)
>>> +
>>> +#define CORE_DDR_200_CFG        0x184
>>> +#define CORE_CDC_T4_DLY_SEL        BIT(0)
>>> +#define CORE_START_CDC_TRAFFIC        BIT(6)
>>> +
>>>  #define CORE_VENDOR_SPEC_CAPABILITIES0    0x11c
>>>
>>> +#define INVALID_TUNING_PHASE    -1
>>>  #define TCXO_FREQ        19200000
>>>  #define SDHCI_MSM_MIN_CLOCK    400000
>>>  #define CORE_FREQ_100MHZ    (100 * 1000 * 1000)
>>> @@ -97,6 +126,7 @@ struct sdhci_msm_host {
>>>      bool use_14lpp_dll_reset;
>>>      bool tuning_done;
>>>      bool calibration_done;
>>> +    u8 saved_tuning_phase;
>>>  };
>>>
>>>  /* Platform specific tuning */
>>> @@ -426,6 +456,136 @@ static int msm_init_cm_dll(struct sdhci_host *host)
>>>      return 0;
>>>  }
>>>
>>> +static int sdhci_msm_cdclp533_calibration(struct sdhci_host *host)
>>> +{
>>> +    struct sdhci_pltfm_host *pltfm_host = sdhci_priv(host);
>>> +    struct sdhci_msm_host *msm_host = sdhci_pltfm_priv(pltfm_host);
>>> +    u32 wait_cnt, config;
>>> +    int ret;
>>> +
>>> +    pr_debug("%s: Enter %s\n", mmc_hostname(host->mmc), __func__);
>>> +
>>> +    /*
>>> +     * Retuning in HS400 (DDR mode) will fail, just reset the
>>> +     * tuning block and restore the saved tuning phase.
>>> +     */
>>> +    ret = msm_init_cm_dll(host);
>>> +    if (ret)
>>> +        goto out;
>>> +
>>> +    /* Set the selected phase in delay line hw block */
>>> +    ret = msm_config_cm_dll_phase(host, msm_host->saved_tuning_phase);
>>> +    if (ret)
>>> +        goto out;
>>> +
>>> +    /* Write 1 to CMD_DAT_TRACK_SEL field in DLL_CONFIG */
>>> +    config = readl_relaxed(host->ioaddr + CORE_DLL_CONFIG);
>>> +    config |= CORE_CMD_DAT_TRACK_SEL;
>>> +    writel_relaxed(config, host->ioaddr + CORE_DLL_CONFIG);
>>> +
>>> +    /* Write 0 to CDC_T4_DLY_SEL field in VENDOR_SPEC_DDR200_CFG */
>>> +    config = readl_relaxed(host->ioaddr + CORE_DDR_200_CFG);
>>> +    config &= ~CORE_CDC_T4_DLY_SEL;
>>> +    writel_relaxed(config, host->ioaddr + CORE_DDR_200_CFG);
>>> +
>>> +    /* Write 0 to CDC_SWITCH_BYPASS_OFF field in CORE_CSR_CDC_GEN_CFG */
>>> +    config = readl_relaxed(host->ioaddr + CORE_CSR_CDC_GEN_CFG);
>>> +    config &= ~CORE_CDC_SWITCH_BYPASS_OFF;
>>> +    writel_relaxed(config, host->ioaddr + CORE_CSR_CDC_GEN_CFG);
>>> +
>>> +    /* Write 1 to CDC_SWITCH_RC_EN field in CORE_CSR_CDC_GEN_CFG */
>>> +    config = readl_relaxed(host->ioaddr + CORE_CSR_CDC_GEN_CFG);
>>> +    config |= CORE_CDC_SWITCH_RC_EN;
>>> +    writel_relaxed(config, host->ioaddr + CORE_CSR_CDC_GEN_CFG);
>>> +
>>> +    /* Write 0 to START_CDC_TRAFFIC field in CORE_DDR200_CFG */
>>> +    config = readl_relaxed(host->ioaddr + CORE_DDR_200_CFG);
>>> +    config &= ~CORE_START_CDC_TRAFFIC;
>>> +    writel_relaxed(config, host->ioaddr + CORE_DDR_200_CFG);
>>> +
>>> +    /*
>>> +     * Perform CDC Register Initialization Sequence
>>> +     *
>>> +     * CORE_CSR_CDC_CTLR_CFG0    0x11800EC
>>> +     * CORE_CSR_CDC_CTLR_CFG1    0x3011111
>>> +     * CORE_CSR_CDC_CAL_TIMER_CFG0    0x1201000
>>> +     * CORE_CSR_CDC_CAL_TIMER_CFG1    0x4
>>> +     * CORE_CSR_CDC_REFCOUNT_CFG    0xCB732020
>>> +     * CORE_CSR_CDC_COARSE_CAL_CFG    0xB19
>>> +     * CORE_CSR_CDC_DELAY_CFG    0x3AC
>>> +     * CORE_CDC_OFFSET_CFG        0x0
>>> +     * CORE_CDC_SLAVE_DDA_CFG    0x16334
>>> +     */
>>> +
>>> +    writel_relaxed(0x11800EC, host->ioaddr + CORE_CSR_CDC_CTLR_CFG0);
>>> +    writel_relaxed(0x3011111, host->ioaddr + CORE_CSR_CDC_CTLR_CFG1);
>>> +    writel_relaxed(0x1201000, host->ioaddr + CORE_CSR_CDC_CAL_TIMER_CFG0);
>>> +    writel_relaxed(0x4, host->ioaddr + CORE_CSR_CDC_CAL_TIMER_CFG1);
>>> +    writel_relaxed(0xCB732020, host->ioaddr + CORE_CSR_CDC_REFCOUNT_CFG);
>>> +    writel_relaxed(0xB19, host->ioaddr + CORE_CSR_CDC_COARSE_CAL_CFG);
>>> +    writel_relaxed(0x3AC, host->ioaddr + CORE_CSR_CDC_DELAY_CFG);
>>> +    writel_relaxed(0x0, host->ioaddr + CORE_CDC_OFFSET_CFG);
>>> +    writel_relaxed(0x16334, host->ioaddr + CORE_CDC_SLAVE_DDA_CFG);
>>> +
>>> +    /* CDC HW Calibration */
>>> +
>>> +    /* Write 1 to SW_TRIG_FULL_CALIB field in CORE_CSR_CDC_CTLR_CFG0 */
>>> +    config = readl_relaxed(host->ioaddr + CORE_CSR_CDC_CTLR_CFG0);
>>> +    config |= CORE_SW_TRIG_FULL_CALIB;
>>> +    writel_relaxed(config, host->ioaddr + CORE_CSR_CDC_CTLR_CFG0);
>>> +
>>> +    /* Write 0 to SW_TRIG_FULL_CALIB field in CORE_CSR_CDC_CTLR_CFG0 */
>>> +    config = readl_relaxed(host->ioaddr + CORE_CSR_CDC_CTLR_CFG0);
>>> +    config &= ~CORE_SW_TRIG_FULL_CALIB;
>>> +    writel_relaxed(config, host->ioaddr + CORE_CSR_CDC_CTLR_CFG0);
>>> +
>>> +    /* Write 1 to HW_AUTOCAL_ENA field in CORE_CSR_CDC_CTLR_CFG0 */
>>> +    config = readl_relaxed(host->ioaddr + CORE_CSR_CDC_CTLR_CFG0);
>>> +    config |= CORE_HW_AUTOCAL_ENA;
>>> +    writel_relaxed(config, host->ioaddr + CORE_CSR_CDC_CTLR_CFG0);
>>> +
>>> +    /* Write 1 to TIMER_ENA field in CORE_CSR_CDC_CAL_TIMER_CFG0 */
>>> +    config = readl_relaxed(host->ioaddr + CORE_CSR_CDC_CAL_TIMER_CFG0);
>>> +    config |= CORE_TIMER_ENA;
>>> +    writel_relaxed(config, host->ioaddr + CORE_CSR_CDC_CAL_TIMER_CFG0);
>>> +
>>> +    wmb(); /* drain writebuffer */
>>> +
>>> +    /* Poll on CALIBRATION_DONE field in CORE_CSR_CDC_STATUS0 to be 1 */
>>> +    wait_cnt = 50;
>>> +    while (!(readl_relaxed(host->ioaddr + CORE_CSR_CDC_STATUS0)
>>> +            & CORE_CALIBRATION_DONE)) {
>>> +        /* max. wait for 50us sec for CALIBRATION_DONE bit to be set */
>>> +        if (--wait_cnt == 0) {
>>> +            pr_err("%s: %s: CDC Calibration was not completed\n",
>>> +                mmc_hostname(host->mmc), __func__);
>>> +            ret = -ETIMEDOUT;
>>> +            goto out;
>>> +        }
>>> +        /* wait for 1us before polling again */
>>> +        udelay(1);
>>> +    }
>>> +
>>> +    /* Verify CDC_ERROR_CODE field in CORE_CSR_CDC_STATUS0 is 0 */
>>> +    ret = readl_relaxed(host->ioaddr + CORE_CSR_CDC_STATUS0)
>>> +            & CORE_CDC_ERROR_CODE_MASK;
>>> +    if (ret) {
>>> +        pr_err("%s: %s: CDC Error Code %d\n",
>>> +            mmc_hostname(host->mmc), __func__, ret);
>>> +        ret = -EINVAL;
>>> +        goto out;
>>> +    }
>>> +
>>> +    /* Write 1 to START_CDC_TRAFFIC field in CORE_DDR200_CFG */
>>> +    config = readl_relaxed(host->ioaddr + CORE_DDR_200_CFG);
>>> +    config |= CORE_START_CDC_TRAFFIC;
>>> +    writel_relaxed(config, host->ioaddr + CORE_DDR_200_CFG);
>>> +out:
>>> +    pr_debug("%s: Exit %s, ret:%d\n", mmc_hostname(host->mmc),
>>> +            __func__, ret);
>>> +    return ret;
>>> +}
>>> +
>>>  static int sdhci_msm_execute_tuning(struct sdhci_host *host, u32 opcode)
>>>  {
>>>      int tuning_seq_cnt = 3;
>>> @@ -433,6 +593,8 @@ static int sdhci_msm_execute_tuning(struct sdhci_host
>>> *host, u32 opcode)
>>>      int rc;
>>>      struct mmc_host *mmc = host->mmc;
>>>      struct mmc_ios ios = host->mmc->ios;
>>> +    struct sdhci_pltfm_host *pltfm_host = sdhci_priv(host);
>>> +    struct sdhci_msm_host *msm_host = sdhci_pltfm_priv(pltfm_host);
>>>
>>>      /*
>>>       * Tuning is required for SDR104, HS200 and HS400 cards and
>>> @@ -457,6 +619,7 @@ retry:
>>>          if (rc)
>>>              return rc;
>>>
>>> +        msm_host->saved_tuning_phase = phase;
>>>          rc = mmc_send_tuning(mmc, opcode, NULL);
>>>          if (!rc) {
>>>              /* Tuning is successful at this tuning point */
>>> @@ -492,6 +655,8 @@ retry:
>>>          rc = -EIO;
>>>      }
>>>
>>> +    if (!rc)
>>> +        msm_host->tuning_done = true;
>>>      return rc;
>>>  }
>>>
>>> @@ -565,6 +730,17 @@ static void sdhci_msm_set_uhs_signaling(struct
>>> sdhci_host *host,
>>>      dev_dbg(mmc_dev(mmc), "%s: clock=%u uhs=%u ctrl_2=0x%x\n",
>>>          mmc_hostname(host->mmc), host->clock, uhs, ctrl_2);
>>>      sdhci_writew(host, ctrl_2, SDHCI_HOST_CONTROL2);
>>> +
>>> +    spin_unlock_irq(&host->lock);
>>> +
>>> +    /* CDCLP533 HW calibration is only required for HS400 mode*/
>>> +    if (host->clock > CORE_FREQ_100MHZ &&
>>> +       msm_host->tuning_done && !msm_host->calibration_done &&
>>> +       (mmc->ios.timing == MMC_TIMING_MMC_HS400))
>>> +        if (!sdhci_msm_cdclp533_calibration(host))
>>> +            msm_host->calibration_done = true;
>>> +
>>> +    spin_lock_irq(&host->lock);
>>>  }
>>>
>>>  static void sdhci_msm_voltage_switch(struct sdhci_host *host)
>>> @@ -907,6 +1083,8 @@ static int sdhci_msm_probe(struct platform_device
>>> *pdev)
>>>
>>>      sdhci_msm_populate_dt(&pdev->dev, msm_host);
>>>
>>> +    msm_host->saved_tuning_phase = INVALID_TUNING_PHASE;
>>
>> There is never a check for INVALID_TUNING_PHASE which begs the question: why
>> have it?
> phase value can be between 0x0 to 0xf. So during probe saved_tuning_phase is
> getting initialized with -1 value.
> 
> Let me know if any concern, we can remove it as well.
> But wont it look incorrect if we initialize it with some valid phase value
> before even tuning is completed?

INVALID_TUNING_PHASE is fine, I would just expect it be checked e.g. in
msm_config_cm_dll_phase

	if (phase > 0xf)
		return -EINVAL;

> 
>>
>>> +
>>>      /* Setup SDCC bus voter clock. */
>>>      msm_host->bus_clk = devm_clk_get(&pdev->dev, "bus");
>>>      if (!IS_ERR(msm_host->bus_clk)) {
>>>
>>
> 

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

Re: [PATCH v5 11/12] mmc: sdhci-msm: Add calibration tuning for CDCLP533 circuit

[Ritesh Harjani]

Hi Adrian,

On 10/11/2016 12:09 PM, Adrian Hunter wrote:
> On 10/10/16 18:42, Ritesh Harjani wrote:
>> Hi Adrian,
>>
>> On 10/10/2016 6:19 PM, Adrian Hunter wrote:
>>> On 05/10/16 17:40, Ritesh Harjani wrote:
>>>> From: Venkat Gopalakrishnan <venkatg@codeaurora.org>
>>>>
>>>> In HS400 mode a new RCLK is introduced on the interface for read data
>>>> transfers. The eMMC5.0 device transmits the read data to the host with
>>>> respect to rising and falling edges of RCLK. In order to ensure correct
>>>> operation of read data transfers in HS400 mode, the incoming RX data
>>>> needs to be sampled by delayed version of RCLK.
>>>>
>>>> The CDCLP533 delay circuit shifts the RCLK by T/4. It needs to be
>>>> initialized, configured and enabled once during HS400 mode switch and
>>>> when operational voltage/clock is changed.
>>>>
>>>> Signed-off-by: Venkat Gopalakrishnan <venkatg@codeaurora.org>
>>>> Signed-off-by: Ritesh Harjani <riteshh@codeaurora.org>
>>>> ---
>>>>  drivers/mmc/host/sdhci-msm.c | 178
>>>> +++++++++++++++++++++++++++++++++++++++++++
>>>>  1 file changed, 178 insertions(+)
>>>>
>>>> diff --git a/drivers/mmc/host/sdhci-msm.c b/drivers/mmc/host/sdhci-msm.c
>>>> index 612fa82..dbf80a9c 100644
>>>> --- a/drivers/mmc/host/sdhci-msm.c
>>>> +++ b/drivers/mmc/host/sdhci-msm.c
>>>> @@ -57,6 +57,7 @@
>>>>  #define CORE_DLL_PDN        BIT(29)
>>>>  #define CORE_DLL_RST        BIT(30)
>>>>  #define CORE_DLL_CONFIG        0x100
>>>> +#define CORE_CMD_DAT_TRACK_SEL    BIT(0)
>>>>  #define CORE_DLL_STATUS        0x108
>>>>
>>>>  #define CORE_DLL_CONFIG_2    0x1b4
>>>> @@ -72,8 +73,36 @@
>>>>  #define CORE_HC_SELECT_IN_HS400    (6 << 19)
>>>>  #define CORE_HC_SELECT_IN_MASK    (7 << 19)
>>>>
>>>> +#define CORE_CSR_CDC_CTLR_CFG0        0x130
>>>> +#define CORE_SW_TRIG_FULL_CALIB        BIT(16)
>>>> +#define CORE_HW_AUTOCAL_ENA        BIT(17)
>>>> +
>>>> +#define CORE_CSR_CDC_CTLR_CFG1        0x134
>>>> +#define CORE_CSR_CDC_CAL_TIMER_CFG0    0x138
>>>> +#define CORE_TIMER_ENA            BIT(16)
>>>> +
>>>> +#define CORE_CSR_CDC_CAL_TIMER_CFG1    0x13C
>>>> +#define CORE_CSR_CDC_REFCOUNT_CFG    0x140
>>>> +#define CORE_CSR_CDC_COARSE_CAL_CFG    0x144
>>>> +#define CORE_CDC_OFFSET_CFG        0x14C
>>>> +#define CORE_CSR_CDC_DELAY_CFG        0x150
>>>> +#define CORE_CDC_SLAVE_DDA_CFG        0x160
>>>> +#define CORE_CSR_CDC_STATUS0        0x164
>>>> +#define CORE_CALIBRATION_DONE        BIT(0)
>>>> +
>>>> +#define CORE_CDC_ERROR_CODE_MASK    0x7000000
>>>> +
>>>> +#define CORE_CSR_CDC_GEN_CFG        0x178
>>>> +#define CORE_CDC_SWITCH_BYPASS_OFF    BIT(0)
>>>> +#define CORE_CDC_SWITCH_RC_EN        BIT(1)
>>>> +
>>>> +#define CORE_DDR_200_CFG        0x184
>>>> +#define CORE_CDC_T4_DLY_SEL        BIT(0)
>>>> +#define CORE_START_CDC_TRAFFIC        BIT(6)
>>>> +
>>>>  #define CORE_VENDOR_SPEC_CAPABILITIES0    0x11c
>>>>
>>>> +#define INVALID_TUNING_PHASE    -1
>>>>  #define TCXO_FREQ        19200000
>>>>  #define SDHCI_MSM_MIN_CLOCK    400000
>>>>  #define CORE_FREQ_100MHZ    (100 * 1000 * 1000)
>>>> @@ -97,6 +126,7 @@ struct sdhci_msm_host {
>>>>      bool use_14lpp_dll_reset;
>>>>      bool tuning_done;
>>>>      bool calibration_done;
>>>> +    u8 saved_tuning_phase;
>>>>  };
>>>>
>>>>  /* Platform specific tuning */
>>>> @@ -426,6 +456,136 @@ static int msm_init_cm_dll(struct sdhci_host *host)
>>>>      return 0;
>>>>  }
>>>>
>>>> +static int sdhci_msm_cdclp533_calibration(struct sdhci_host *host)
>>>> +{
>>>> +    struct sdhci_pltfm_host *pltfm_host = sdhci_priv(host);
>>>> +    struct sdhci_msm_host *msm_host = sdhci_pltfm_priv(pltfm_host);
>>>> +    u32 wait_cnt, config;
>>>> +    int ret;
>>>> +
>>>> +    pr_debug("%s: Enter %s\n", mmc_hostname(host->mmc), __func__);
>>>> +
>>>> +    /*
>>>> +     * Retuning in HS400 (DDR mode) will fail, just reset the
>>>> +     * tuning block and restore the saved tuning phase.
>>>> +     */
>>>> +    ret = msm_init_cm_dll(host);
>>>> +    if (ret)
>>>> +        goto out;
>>>> +
>>>> +    /* Set the selected phase in delay line hw block */
>>>> +    ret = msm_config_cm_dll_phase(host, msm_host->saved_tuning_phase);
>>>> +    if (ret)
>>>> +        goto out;
>>>> +
>>>> +    /* Write 1 to CMD_DAT_TRACK_SEL field in DLL_CONFIG */
>>>> +    config = readl_relaxed(host->ioaddr + CORE_DLL_CONFIG);
>>>> +    config |= CORE_CMD_DAT_TRACK_SEL;
>>>> +    writel_relaxed(config, host->ioaddr + CORE_DLL_CONFIG);
>>>> +
>>>> +    /* Write 0 to CDC_T4_DLY_SEL field in VENDOR_SPEC_DDR200_CFG */
>>>> +    config = readl_relaxed(host->ioaddr + CORE_DDR_200_CFG);
>>>> +    config &= ~CORE_CDC_T4_DLY_SEL;
>>>> +    writel_relaxed(config, host->ioaddr + CORE_DDR_200_CFG);
>>>> +
>>>> +    /* Write 0 to CDC_SWITCH_BYPASS_OFF field in CORE_CSR_CDC_GEN_CFG */
>>>> +    config = readl_relaxed(host->ioaddr + CORE_CSR_CDC_GEN_CFG);
>>>> +    config &= ~CORE_CDC_SWITCH_BYPASS_OFF;
>>>> +    writel_relaxed(config, host->ioaddr + CORE_CSR_CDC_GEN_CFG);
>>>> +
>>>> +    /* Write 1 to CDC_SWITCH_RC_EN field in CORE_CSR_CDC_GEN_CFG */
>>>> +    config = readl_relaxed(host->ioaddr + CORE_CSR_CDC_GEN_CFG);
>>>> +    config |= CORE_CDC_SWITCH_RC_EN;
>>>> +    writel_relaxed(config, host->ioaddr + CORE_CSR_CDC_GEN_CFG);
>>>> +
>>>> +    /* Write 0 to START_CDC_TRAFFIC field in CORE_DDR200_CFG */
>>>> +    config = readl_relaxed(host->ioaddr + CORE_DDR_200_CFG);
>>>> +    config &= ~CORE_START_CDC_TRAFFIC;
>>>> +    writel_relaxed(config, host->ioaddr + CORE_DDR_200_CFG);
>>>> +
>>>> +    /*
>>>> +     * Perform CDC Register Initialization Sequence
>>>> +     *
>>>> +     * CORE_CSR_CDC_CTLR_CFG0    0x11800EC
>>>> +     * CORE_CSR_CDC_CTLR_CFG1    0x3011111
>>>> +     * CORE_CSR_CDC_CAL_TIMER_CFG0    0x1201000
>>>> +     * CORE_CSR_CDC_CAL_TIMER_CFG1    0x4
>>>> +     * CORE_CSR_CDC_REFCOUNT_CFG    0xCB732020
>>>> +     * CORE_CSR_CDC_COARSE_CAL_CFG    0xB19
>>>> +     * CORE_CSR_CDC_DELAY_CFG    0x3AC
>>>> +     * CORE_CDC_OFFSET_CFG        0x0
>>>> +     * CORE_CDC_SLAVE_DDA_CFG    0x16334
>>>> +     */
>>>> +
>>>> +    writel_relaxed(0x11800EC, host->ioaddr + CORE_CSR_CDC_CTLR_CFG0);
>>>> +    writel_relaxed(0x3011111, host->ioaddr + CORE_CSR_CDC_CTLR_CFG1);
>>>> +    writel_relaxed(0x1201000, host->ioaddr + CORE_CSR_CDC_CAL_TIMER_CFG0);
>>>> +    writel_relaxed(0x4, host->ioaddr + CORE_CSR_CDC_CAL_TIMER_CFG1);
>>>> +    writel_relaxed(0xCB732020, host->ioaddr + CORE_CSR_CDC_REFCOUNT_CFG);
>>>> +    writel_relaxed(0xB19, host->ioaddr + CORE_CSR_CDC_COARSE_CAL_CFG);
>>>> +    writel_relaxed(0x3AC, host->ioaddr + CORE_CSR_CDC_DELAY_CFG);
>>>> +    writel_relaxed(0x0, host->ioaddr + CORE_CDC_OFFSET_CFG);
>>>> +    writel_relaxed(0x16334, host->ioaddr + CORE_CDC_SLAVE_DDA_CFG);
>>>> +
>>>> +    /* CDC HW Calibration */
>>>> +
>>>> +    /* Write 1 to SW_TRIG_FULL_CALIB field in CORE_CSR_CDC_CTLR_CFG0 */
>>>> +    config = readl_relaxed(host->ioaddr + CORE_CSR_CDC_CTLR_CFG0);
>>>> +    config |= CORE_SW_TRIG_FULL_CALIB;
>>>> +    writel_relaxed(config, host->ioaddr + CORE_CSR_CDC_CTLR_CFG0);
>>>> +
>>>> +    /* Write 0 to SW_TRIG_FULL_CALIB field in CORE_CSR_CDC_CTLR_CFG0 */
>>>> +    config = readl_relaxed(host->ioaddr + CORE_CSR_CDC_CTLR_CFG0);
>>>> +    config &= ~CORE_SW_TRIG_FULL_CALIB;
>>>> +    writel_relaxed(config, host->ioaddr + CORE_CSR_CDC_CTLR_CFG0);
>>>> +
>>>> +    /* Write 1 to HW_AUTOCAL_ENA field in CORE_CSR_CDC_CTLR_CFG0 */
>>>> +    config = readl_relaxed(host->ioaddr + CORE_CSR_CDC_CTLR_CFG0);
>>>> +    config |= CORE_HW_AUTOCAL_ENA;
>>>> +    writel_relaxed(config, host->ioaddr + CORE_CSR_CDC_CTLR_CFG0);
>>>> +
>>>> +    /* Write 1 to TIMER_ENA field in CORE_CSR_CDC_CAL_TIMER_CFG0 */
>>>> +    config = readl_relaxed(host->ioaddr + CORE_CSR_CDC_CAL_TIMER_CFG0);
>>>> +    config |= CORE_TIMER_ENA;
>>>> +    writel_relaxed(config, host->ioaddr + CORE_CSR_CDC_CAL_TIMER_CFG0);
>>>> +
>>>> +    wmb(); /* drain writebuffer */
>>>> +
>>>> +    /* Poll on CALIBRATION_DONE field in CORE_CSR_CDC_STATUS0 to be 1 */
>>>> +    wait_cnt = 50;
>>>> +    while (!(readl_relaxed(host->ioaddr + CORE_CSR_CDC_STATUS0)
>>>> +            & CORE_CALIBRATION_DONE)) {
>>>> +        /* max. wait for 50us sec for CALIBRATION_DONE bit to be set */
>>>> +        if (--wait_cnt == 0) {
>>>> +            pr_err("%s: %s: CDC Calibration was not completed\n",
>>>> +                mmc_hostname(host->mmc), __func__);
>>>> +            ret = -ETIMEDOUT;
>>>> +            goto out;
>>>> +        }
>>>> +        /* wait for 1us before polling again */
>>>> +        udelay(1);
>>>> +    }
>>>> +
>>>> +    /* Verify CDC_ERROR_CODE field in CORE_CSR_CDC_STATUS0 is 0 */
>>>> +    ret = readl_relaxed(host->ioaddr + CORE_CSR_CDC_STATUS0)
>>>> +            & CORE_CDC_ERROR_CODE_MASK;
>>>> +    if (ret) {
>>>> +        pr_err("%s: %s: CDC Error Code %d\n",
>>>> +            mmc_hostname(host->mmc), __func__, ret);
>>>> +        ret = -EINVAL;
>>>> +        goto out;
>>>> +    }
>>>> +
>>>> +    /* Write 1 to START_CDC_TRAFFIC field in CORE_DDR200_CFG */
>>>> +    config = readl_relaxed(host->ioaddr + CORE_DDR_200_CFG);
>>>> +    config |= CORE_START_CDC_TRAFFIC;
>>>> +    writel_relaxed(config, host->ioaddr + CORE_DDR_200_CFG);
>>>> +out:
>>>> +    pr_debug("%s: Exit %s, ret:%d\n", mmc_hostname(host->mmc),
>>>> +            __func__, ret);
>>>> +    return ret;
>>>> +}
>>>> +
>>>>  static int sdhci_msm_execute_tuning(struct sdhci_host *host, u32 opcode)
>>>>  {
>>>>      int tuning_seq_cnt = 3;
>>>> @@ -433,6 +593,8 @@ static int sdhci_msm_execute_tuning(struct sdhci_host
>>>> *host, u32 opcode)
>>>>      int rc;
>>>>      struct mmc_host *mmc = host->mmc;
>>>>      struct mmc_ios ios = host->mmc->ios;
>>>> +    struct sdhci_pltfm_host *pltfm_host = sdhci_priv(host);
>>>> +    struct sdhci_msm_host *msm_host = sdhci_pltfm_priv(pltfm_host);
>>>>
>>>>      /*
>>>>       * Tuning is required for SDR104, HS200 and HS400 cards and
>>>> @@ -457,6 +619,7 @@ retry:
>>>>          if (rc)
>>>>              return rc;
>>>>
>>>> +        msm_host->saved_tuning_phase = phase;
>>>>          rc = mmc_send_tuning(mmc, opcode, NULL);
>>>>          if (!rc) {
>>>>              /* Tuning is successful at this tuning point */
>>>> @@ -492,6 +655,8 @@ retry:
>>>>          rc = -EIO;
>>>>      }
>>>>
>>>> +    if (!rc)
>>>> +        msm_host->tuning_done = true;
>>>>      return rc;
>>>>  }
>>>>
>>>> @@ -565,6 +730,17 @@ static void sdhci_msm_set_uhs_signaling(struct
>>>> sdhci_host *host,
>>>>      dev_dbg(mmc_dev(mmc), "%s: clock=%u uhs=%u ctrl_2=0x%x\n",
>>>>          mmc_hostname(host->mmc), host->clock, uhs, ctrl_2);
>>>>      sdhci_writew(host, ctrl_2, SDHCI_HOST_CONTROL2);
>>>> +
>>>> +    spin_unlock_irq(&host->lock);
>>>> +
>>>> +    /* CDCLP533 HW calibration is only required for HS400 mode*/
>>>> +    if (host->clock > CORE_FREQ_100MHZ &&
>>>> +       msm_host->tuning_done && !msm_host->calibration_done &&
>>>> +       (mmc->ios.timing == MMC_TIMING_MMC_HS400))
>>>> +        if (!sdhci_msm_cdclp533_calibration(host))
>>>> +            msm_host->calibration_done = true;
>>>> +
>>>> +    spin_lock_irq(&host->lock);
>>>>  }
>>>>
>>>>  static void sdhci_msm_voltage_switch(struct sdhci_host *host)
>>>> @@ -907,6 +1083,8 @@ static int sdhci_msm_probe(struct platform_device
>>>> *pdev)
>>>>
>>>>      sdhci_msm_populate_dt(&pdev->dev, msm_host);
>>>>
>>>> +    msm_host->saved_tuning_phase = INVALID_TUNING_PHASE;
>>>
>>> There is never a check for INVALID_TUNING_PHASE which begs the question: why
>>> have it?
>> phase value can be between 0x0 to 0xf. So during probe saved_tuning_phase is
>> getting initialized with -1 value.
>>
>> Let me know if any concern, we can remove it as well.
>> But wont it look incorrect if we initialize it with some valid phase value
>> before even tuning is completed?
>
> INVALID_TUNING_PHASE is fine, I would just expect it be checked e.g. in
> msm_config_cm_dll_phase
>
> 	if (phase > 0xf)
> 		return -EINVAL;
Sure will add these checks.

>
>>
>>>
>>>> +
>>>>      /* Setup SDCC bus voter clock. */
>>>>      msm_host->bus_clk = devm_clk_get(&pdev->dev, "bus");
>>>>      if (!IS_ERR(msm_host->bus_clk)) {
>>>>
>>>
>>
>
> --
> To unsubscribe from this list: send the line "unsubscribe linux-mmc" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html
>

[PATCH v5 03/12] mmc: sdhci-msm: add pltfm_data support to get clk-rates from DT

[Ritesh Harjani]

This adds support for sdhc-msm controllers to get supported
clk-rates from DT. sdhci-msm would need it's own set_clock
ops to be implemented. For this, supported clk-rates needs
to be populated in sdhci_msm_pltfm_data.

Signed-off-by: Ritesh Harjani <riteshh@codeaurora.org>
---
 .../devicetree/bindings/mmc/sdhci-msm.txt          |  1 +
 drivers/mmc/host/sdhci-msm.c                       | 48 ++++++++++++++++++++++
 2 files changed, 49 insertions(+)

diff --git a/Documentation/devicetree/bindings/mmc/sdhci-msm.txt b/Documentation/devicetree/bindings/mmc/sdhci-msm.txt
index 485483a..6a83b38 100644
--- a/Documentation/devicetree/bindings/mmc/sdhci-msm.txt
+++ b/Documentation/devicetree/bindings/mmc/sdhci-msm.txt
@@ -17,6 +17,7 @@ Required properties:
 	"iface" - Main peripheral bus clock (PCLK/HCLK - AHB Bus clock) (required)
 	"core"	- SDC MMC clock (MCLK) (required)
 	"bus"	- SDCC bus voter clock (optional)
+- clk-rates: Array of supported GCC clock frequencies for sdhc, Units - Hz.
 
 Example:
 
diff --git a/drivers/mmc/host/sdhci-msm.c b/drivers/mmc/host/sdhci-msm.c
index 85ddaae..042ecb2 100644
--- a/drivers/mmc/host/sdhci-msm.c
+++ b/drivers/mmc/host/sdhci-msm.c
@@ -81,6 +81,8 @@ struct sdhci_msm_host {
 	struct clk *clk;	/* main SD/MMC bus clock */
 	struct clk *pclk;	/* SDHC peripheral bus clock */
 	struct clk *bus_clk;	/* SDHC bus voter clock */
+	u32 *clk_table;
+	int clk_table_sz;
 	struct mmc_host *mmc;
 	bool use_14lpp_dll_reset;
 };
@@ -582,6 +584,50 @@ static const struct sdhci_pltfm_data sdhci_msm_pdata = {
 	.ops = &sdhci_msm_ops,
 };
 
+static int sdhci_msm_dt_get_array(struct device *dev, const char *prop_name,
+				u32 **table, int *size)
+{
+	struct device_node *np = dev->of_node;
+	int count, ret;
+	u32 *arr;
+
+	count = of_property_count_elems_of_size(np, prop_name, sizeof(u32));
+	if (count < 0) {
+		dev_warn(dev, "%s: Invalid dt property, err(%d)\n",
+				prop_name, count);
+		return count;
+	}
+
+	arr = kcalloc(count, sizeof(*arr), GFP_KERNEL);
+	if (!arr)
+		return -ENOMEM;
+
+	ret = of_property_read_u32_array(np, prop_name, arr, count);
+	if (ret) {
+		kfree(arr);
+		dev_warn(dev, "%s Invalid dt array property, err(%d)\n",
+				prop_name, ret);
+		return ret;
+	}
+	*table = arr;
+	*size = count;
+	return 0;
+}
+
+void sdhci_msm_populate_dt(struct device *dev,
+						struct sdhci_msm_host *msm_host)
+{
+	int table_sz = 0;
+	u32 *table = NULL;
+
+	if (sdhci_msm_dt_get_array(dev, "clk-rates", &table, &table_sz)) {
+		dev_warn(dev, "failed in DT parsing for supported clk-rates\n");
+		return;
+	}
+	msm_host->clk_table = table;
+	msm_host->clk_table_sz = table_sz;
+}
+
 static int sdhci_msm_probe(struct platform_device *pdev)
 {
 	struct sdhci_host *host;
@@ -608,6 +654,8 @@ static int sdhci_msm_probe(struct platform_device *pdev)
 
 	sdhci_get_of_property(pdev);
 
+	sdhci_msm_populate_dt(&pdev->dev, msm_host);
+
 	/* Setup SDCC bus voter clock. */
 	msm_host->bus_clk = devm_clk_get(&pdev->dev, "bus");
 	if (!IS_ERR(msm_host->bus_clk)) {
-- 
The Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum, 
a Linux Foundation Collaborative Project.

Re: [PATCH v5 03/12] mmc: sdhci-msm: add pltfm_data support to get clk-rates from DT

[Adrian Hunter]

On 05/10/16 17:40, Ritesh Harjani wrote:
> This adds support for sdhc-msm controllers to get supported
> clk-rates from DT. sdhci-msm would need it's own set_clock
> ops to be implemented. For this, supported clk-rates needs
> to be populated in sdhci_msm_pltfm_data.
> 
> Signed-off-by: Ritesh Harjani <riteshh@codeaurora.org>
> ---
>  .../devicetree/bindings/mmc/sdhci-msm.txt          |  1 +
>  drivers/mmc/host/sdhci-msm.c                       | 48 ++++++++++++++++++++++
>  2 files changed, 49 insertions(+)
> 
> diff --git a/Documentation/devicetree/bindings/mmc/sdhci-msm.txt b/Documentation/devicetree/bindings/mmc/sdhci-msm.txt
> index 485483a..6a83b38 100644
> --- a/Documentation/devicetree/bindings/mmc/sdhci-msm.txt
> +++ b/Documentation/devicetree/bindings/mmc/sdhci-msm.txt
> @@ -17,6 +17,7 @@ Required properties:
>  	"iface" - Main peripheral bus clock (PCLK/HCLK - AHB Bus clock) (required)
>  	"core"	- SDC MMC clock (MCLK) (required)
>  	"bus"	- SDCC bus voter clock (optional)
> +- clk-rates: Array of supported GCC clock frequencies for sdhc, Units - Hz.
>  
>  Example:
>  
> diff --git a/drivers/mmc/host/sdhci-msm.c b/drivers/mmc/host/sdhci-msm.c
> index 85ddaae..042ecb2 100644
> --- a/drivers/mmc/host/sdhci-msm.c
> +++ b/drivers/mmc/host/sdhci-msm.c
> @@ -81,6 +81,8 @@ struct sdhci_msm_host {
>  	struct clk *clk;	/* main SD/MMC bus clock */
>  	struct clk *pclk;	/* SDHC peripheral bus clock */
>  	struct clk *bus_clk;	/* SDHC bus voter clock */
> +	u32 *clk_table;
> +	int clk_table_sz;
>  	struct mmc_host *mmc;
>  	bool use_14lpp_dll_reset;
>  };
> @@ -582,6 +584,50 @@ static const struct sdhci_pltfm_data sdhci_msm_pdata = {
>  	.ops = &sdhci_msm_ops,
>  };
>  
> +static int sdhci_msm_dt_get_array(struct device *dev, const char *prop_name,
> +				u32 **table, int *size)

It is nice to align to the open parenthesis.  Have a look at the checks from
checkpatch --strict

> +{
> +	struct device_node *np = dev->of_node;
> +	int count, ret;
> +	u32 *arr;
> +
> +	count = of_property_count_elems_of_size(np, prop_name, sizeof(u32));

Shouldn't this be of_property_count_u32_elems()

> +	if (count < 0) {
> +		dev_warn(dev, "%s: Invalid dt property, err(%d)\n",
> +				prop_name, count);
> +		return count;
> +	}
> +
> +	arr = kcalloc(count, sizeof(*arr), GFP_KERNEL);
> +	if (!arr)
> +		return -ENOMEM;
> +
> +	ret = of_property_read_u32_array(np, prop_name, arr, count);
> +	if (ret) {
> +		kfree(arr);
> +		dev_warn(dev, "%s Invalid dt array property, err(%d)\n",

'err(%d)' is an unusual style for printing error numbers.  'error %d' looks
better.

Also in some messages you have 'DT' and others 'dt'.  Also here it is '%s'
but above '%s:', and some messages start with a lower case letter and some
upper case.  Please try to make everything consistent.

> +				prop_name, ret);
> +		return ret;
> +	}
> +	*table = arr;
> +	*size = count;
> +	return 0;
> +}
> +
> +void sdhci_msm_populate_dt(struct device *dev,
> +						struct sdhci_msm_host *msm_host)

Align to open parenthesis

> +{
> +	int table_sz = 0;
> +	u32 *table = NULL;
> +
> +	if (sdhci_msm_dt_get_array(dev, "clk-rates", &table, &table_sz)) {
> +		dev_warn(dev, "failed in DT parsing for supported clk-rates\n");
> +		return;
> +	}
> +	msm_host->clk_table = table;
> +	msm_host->clk_table_sz = table_sz;
> +}
> +
>  static int sdhci_msm_probe(struct platform_device *pdev)
>  {
>  	struct sdhci_host *host;
> @@ -608,6 +654,8 @@ static int sdhci_msm_probe(struct platform_device *pdev)
>  
>  	sdhci_get_of_property(pdev);
>  
> +	sdhci_msm_populate_dt(&pdev->dev, msm_host);
> +
>  	/* Setup SDCC bus voter clock. */
>  	msm_host->bus_clk = devm_clk_get(&pdev->dev, "bus");
>  	if (!IS_ERR(msm_host->bus_clk)) {
> 

Re: [PATCH v5 03/12] mmc: sdhci-msm: add pltfm_data support to get clk-rates from DT

[Ritesh Harjani]

Hi Adrian,

Thanks for the review. I will address your comments.
Responses inline.

--
Regards
Ritesh

On 10/10/2016 3:05 PM, Adrian Hunter wrote:
> On 05/10/16 17:40, Ritesh Harjani wrote:
>> This adds support for sdhc-msm controllers to get supported
>> clk-rates from DT. sdhci-msm would need it's own set_clock
>> ops to be implemented. For this, supported clk-rates needs
>> to be populated in sdhci_msm_pltfm_data.
>>
>> Signed-off-by: Ritesh Harjani <riteshh-sgV2jX0FEOL9JmXXK+q4OQ@public.gmane.org>
>> ---
>>  .../devicetree/bindings/mmc/sdhci-msm.txt          |  1 +
>>  drivers/mmc/host/sdhci-msm.c                       | 48 ++++++++++++++++++++++
>>  2 files changed, 49 insertions(+)
>>
>> diff --git a/Documentation/devicetree/bindings/mmc/sdhci-msm.txt b/Documentation/devicetree/bindings/mmc/sdhci-msm.txt
>> index 485483a..6a83b38 100644
>> --- a/Documentation/devicetree/bindings/mmc/sdhci-msm.txt
>> +++ b/Documentation/devicetree/bindings/mmc/sdhci-msm.txt
>> @@ -17,6 +17,7 @@ Required properties:
>>  	"iface" - Main peripheral bus clock (PCLK/HCLK - AHB Bus clock) (required)
>>  	"core"	- SDC MMC clock (MCLK) (required)
>>  	"bus"	- SDCC bus voter clock (optional)
>> +- clk-rates: Array of supported GCC clock frequencies for sdhc, Units - Hz.
>>
>>  Example:
>>
>> diff --git a/drivers/mmc/host/sdhci-msm.c b/drivers/mmc/host/sdhci-msm.c
>> index 85ddaae..042ecb2 100644
>> --- a/drivers/mmc/host/sdhci-msm.c
>> +++ b/drivers/mmc/host/sdhci-msm.c
>> @@ -81,6 +81,8 @@ struct sdhci_msm_host {
>>  	struct clk *clk;	/* main SD/MMC bus clock */
>>  	struct clk *pclk;	/* SDHC peripheral bus clock */
>>  	struct clk *bus_clk;	/* SDHC bus voter clock */
>> +	u32 *clk_table;
>> +	int clk_table_sz;
>>  	struct mmc_host *mmc;
>>  	bool use_14lpp_dll_reset;
>>  };
>> @@ -582,6 +584,50 @@ static const struct sdhci_pltfm_data sdhci_msm_pdata = {
>>  	.ops = &sdhci_msm_ops,
>>  };
>>
>> +static int sdhci_msm_dt_get_array(struct device *dev, const char *prop_name,
>> +				u32 **table, int *size)
>
> It is nice to align to the open parenthesis.  Have a look at the checks from
> checkpatch --strict
Sure, will do that.

>
>> +{
>> +	struct device_node *np = dev->of_node;
>> +	int count, ret;
>> +	u32 *arr;
>> +
>> +	count = of_property_count_elems_of_size(np, prop_name, sizeof(u32));
>
> Shouldn't this be of_property_count_u32_elems()
Yes, I will make the change.

>
>> +	if (count < 0) {
will change this to below to avoid empty property problem in DT which 
you mentioned in the next patch.
	if (count <= 0) {


>> +		dev_warn(dev, "%s: Invalid dt property, err(%d)\n",
>> +				prop_name, count);
>> +		return count;
>> +	}
>> +
>> +	arr = kcalloc(count, sizeof(*arr), GFP_KERNEL);
>> +	if (!arr)
>> +		return -ENOMEM;
>> +
>> +	ret = of_property_read_u32_array(np, prop_name, arr, count);
>> +	if (ret) {
>> +		kfree(arr);
>> +		dev_warn(dev, "%s Invalid dt array property, err(%d)\n",
>
> 'err(%d)' is an unusual style for printing error numbers.  'error %d' looks
> better.
Done.

>
> Also in some messages you have 'DT' and others 'dt'.  Also here it is '%s'
> but above '%s:', and some messages start with a lower case letter and some
> upper case.  Please try to make everything consistent.
>
Sure thanks.

>> +				prop_name, ret);
>> +		return ret;
>> +	}
>> +	*table = arr;
>> +	*size = count;
>> +	return 0;
>> +}
>> +
>> +void sdhci_msm_populate_dt(struct device *dev,
>> +						struct sdhci_msm_host *msm_host)
>
> Align to open parenthesis
Done.

>
>> +{
>> +	int table_sz = 0;
>> +	u32 *table = NULL;
>> +
>> +	if (sdhci_msm_dt_get_array(dev, "clk-rates", &table, &table_sz)) {
>> +		dev_warn(dev, "failed in DT parsing for supported clk-rates\n");
>> +		return;
>> +	}
>> +	msm_host->clk_table = table;
>> +	msm_host->clk_table_sz = table_sz;
>> +}
>> +
>>  static int sdhci_msm_probe(struct platform_device *pdev)
>>  {
>>  	struct sdhci_host *host;
>> @@ -608,6 +654,8 @@ static int sdhci_msm_probe(struct platform_device *pdev)
>>
>>  	sdhci_get_of_property(pdev);
>>
>> +	sdhci_msm_populate_dt(&pdev->dev, msm_host);
>> +
>>  	/* Setup SDCC bus voter clock. */
>>  	msm_host->bus_clk = devm_clk_get(&pdev->dev, "bus");
>>  	if (!IS_ERR(msm_host->bus_clk)) {
>>
>
> --
> To unsubscribe from this list: send the line "unsubscribe linux-mmc" in
> the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html
>
--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH v5 03/12] mmc: sdhci-msm: add pltfm_data support to get clk-rates from DT

[Rob Herring]

On Wed, Oct 05, 2016 at 08:10:31PM +0530, Ritesh Harjani wrote:
> This adds support for sdhc-msm controllers to get supported
> clk-rates from DT. sdhci-msm would need it's own set_clock
> ops to be implemented. For this, supported clk-rates needs
> to be populated in sdhci_msm_pltfm_data.
> 
> Signed-off-by: Ritesh Harjani <riteshh-sgV2jX0FEOL9JmXXK+q4OQ@public.gmane.org>
> ---
>  .../devicetree/bindings/mmc/sdhci-msm.txt          |  1 +
>  drivers/mmc/host/sdhci-msm.c                       | 48 ++++++++++++++++++++++
>  2 files changed, 49 insertions(+)
> 
> diff --git a/Documentation/devicetree/bindings/mmc/sdhci-msm.txt b/Documentation/devicetree/bindings/mmc/sdhci-msm.txt
> index 485483a..6a83b38 100644
> --- a/Documentation/devicetree/bindings/mmc/sdhci-msm.txt
> +++ b/Documentation/devicetree/bindings/mmc/sdhci-msm.txt
> @@ -17,6 +17,7 @@ Required properties:
>  	"iface" - Main peripheral bus clock (PCLK/HCLK - AHB Bus clock) (required)
>  	"core"	- SDC MMC clock (MCLK) (required)
>  	"bus"	- SDCC bus voter clock (optional)
> +- clk-rates: Array of supported GCC clock frequencies for sdhc, Units - Hz.

Why can't some combination of assigned-clock-rates and querying the 
clock provider for rates be used here?

Minimally this would need unit suffix and either be made common or have 
a vendor prefix.

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

Re: [PATCH v5 03/12] mmc: sdhci-msm: add pltfm_data support to get clk-rates from DT

[Ritesh Harjani]

Hi Rob,

Thanks for review.

On 10/10/2016 6:27 PM, Rob Herring wrote:
> On Wed, Oct 05, 2016 at 08:10:31PM +0530, Ritesh Harjani wrote:
>> This adds support for sdhc-msm controllers to get supported
>> clk-rates from DT. sdhci-msm would need it's own set_clock
>> ops to be implemented. For this, supported clk-rates needs
>> to be populated in sdhci_msm_pltfm_data.
>>
>> Signed-off-by: Ritesh Harjani <riteshh@codeaurora.org>
>> ---
>>  .../devicetree/bindings/mmc/sdhci-msm.txt          |  1 +
>>  drivers/mmc/host/sdhci-msm.c                       | 48 ++++++++++++++++++++++
>>  2 files changed, 49 insertions(+)
>>
>> diff --git a/Documentation/devicetree/bindings/mmc/sdhci-msm.txt b/Documentation/devicetree/bindings/mmc/sdhci-msm.txt
>> index 485483a..6a83b38 100644
>> --- a/Documentation/devicetree/bindings/mmc/sdhci-msm.txt
>> +++ b/Documentation/devicetree/bindings/mmc/sdhci-msm.txt
>> @@ -17,6 +17,7 @@ Required properties:
>>  	"iface" - Main peripheral bus clock (PCLK/HCLK - AHB Bus clock) (required)
>>  	"core"	- SDC MMC clock (MCLK) (required)
>>  	"bus"	- SDCC bus voter clock (optional)
>> +- clk-rates: Array of supported GCC clock frequencies for sdhc, Units - Hz.
>
> Why can't some combination of assigned-clock-rates and querying the
> clock provider for rates be used here?
 From what I understood, assigned-clock-rates would only work for 
setting some default clock rates for certain clocks by calling
of_clk_set_defaults.

Whereas the requirement here is -
That since SDHC msm directly controls the clk(core clock) at source, 
it's sdhci-msm driver needs to know the supported clk-rates by the 
underlying platform to configure the nearest floor value supported on 
this platform (when the request arrives from the core layer to switch 
the clock).

Hence the table of clk-rates is provided for sdhci-msm.

>
> Minimally this would need unit suffix and either be made common or have
> a vendor prefix.
Sure will this work in that case - "qcom-clk-rates"

>
> Rob
> --
> To unsubscribe from this list: send the line "unsubscribe linux-mmc" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html
>

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

Re: [PATCH v5 03/12] mmc: sdhci-msm: add pltfm_data support to get clk-rates from DT

[Rob Herring]

On Mon, Oct 10, 2016 at 11:07 AM, Ritesh Harjani <riteshh@codeaurora.org> wrote:
> Hi Rob,
>
> Thanks for review.
>
> On 10/10/2016 6:27 PM, Rob Herring wrote:
>>
>> On Wed, Oct 05, 2016 at 08:10:31PM +0530, Ritesh Harjani wrote:
>>>
>>> This adds support for sdhc-msm controllers to get supported
>>> clk-rates from DT. sdhci-msm would need it's own set_clock
>>> ops to be implemented. For this, supported clk-rates needs
>>> to be populated in sdhci_msm_pltfm_data.
>>>
>>> Signed-off-by: Ritesh Harjani <riteshh@codeaurora.org>
>>> ---
>>>  .../devicetree/bindings/mmc/sdhci-msm.txt          |  1 +
>>>  drivers/mmc/host/sdhci-msm.c                       | 48
>>> ++++++++++++++++++++++
>>>  2 files changed, 49 insertions(+)
>>>
>>> diff --git a/Documentation/devicetree/bindings/mmc/sdhci-msm.txt
>>> b/Documentation/devicetree/bindings/mmc/sdhci-msm.txt
>>> index 485483a..6a83b38 100644
>>> --- a/Documentation/devicetree/bindings/mmc/sdhci-msm.txt
>>> +++ b/Documentation/devicetree/bindings/mmc/sdhci-msm.txt
>>> @@ -17,6 +17,7 @@ Required properties:
>>>         "iface" - Main peripheral bus clock (PCLK/HCLK - AHB Bus clock)
>>> (required)
>>>         "core"  - SDC MMC clock (MCLK) (required)
>>>         "bus"   - SDCC bus voter clock (optional)
>>> +- clk-rates: Array of supported GCC clock frequencies for sdhc, Units -
>>> Hz.
>>
>>
>> Why can't some combination of assigned-clock-rates and querying the
>> clock provider for rates be used here?
>
> From what I understood, assigned-clock-rates would only work for setting
> some default clock rates for certain clocks by calling
> of_clk_set_defaults.
>
> Whereas the requirement here is -
> That since SDHC msm directly controls the clk(core clock) at source, it's
> sdhci-msm driver needs to know the supported clk-rates by the underlying
> platform to configure the nearest floor value supported on this platform
> (when the request arrives from the core layer to switch the clock).

Why does clk_round_rate not work for you? That will round down to the
nearest frequency supported.

> Hence the table of clk-rates is provided for sdhci-msm.
>
>>
>> Minimally this would need unit suffix and either be made common or have
>> a vendor prefix.
>
> Sure will this work in that case - "qcom-clk-rates"

"qcom,clk-rates", but I'm not yet convinced this is right.

Rob

Re: [PATCH v5 03/12] mmc: sdhci-msm: add pltfm_data support to get clk-rates from DT

[Ritesh Harjani]

Hi Rob

On 10/11/2016 12:59 AM, Rob Herring wrote:
> On Mon, Oct 10, 2016 at 11:07 AM, Ritesh Harjani <riteshh@codeaurora.org> wrote:
>> Hi Rob,
>>
>> Thanks for review.
>>
>> On 10/10/2016 6:27 PM, Rob Herring wrote:
>>>
>>> On Wed, Oct 05, 2016 at 08:10:31PM +0530, Ritesh Harjani wrote:
>>>>
>>>> This adds support for sdhc-msm controllers to get supported
>>>> clk-rates from DT. sdhci-msm would need it's own set_clock
>>>> ops to be implemented. For this, supported clk-rates needs
>>>> to be populated in sdhci_msm_pltfm_data.
>>>>
>>>> Signed-off-by: Ritesh Harjani <riteshh@codeaurora.org>
>>>> ---
>>>>  .../devicetree/bindings/mmc/sdhci-msm.txt          |  1 +
>>>>  drivers/mmc/host/sdhci-msm.c                       | 48
>>>> ++++++++++++++++++++++
>>>>  2 files changed, 49 insertions(+)
>>>>
>>>> diff --git a/Documentation/devicetree/bindings/mmc/sdhci-msm.txt
>>>> b/Documentation/devicetree/bindings/mmc/sdhci-msm.txt
>>>> index 485483a..6a83b38 100644
>>>> --- a/Documentation/devicetree/bindings/mmc/sdhci-msm.txt
>>>> +++ b/Documentation/devicetree/bindings/mmc/sdhci-msm.txt
>>>> @@ -17,6 +17,7 @@ Required properties:
>>>>         "iface" - Main peripheral bus clock (PCLK/HCLK - AHB Bus clock)
>>>> (required)
>>>>         "core"  - SDC MMC clock (MCLK) (required)
>>>>         "bus"   - SDCC bus voter clock (optional)
>>>> +- clk-rates: Array of supported GCC clock frequencies for sdhc, Units -
>>>> Hz.
>>>
>>>
>>> Why can't some combination of assigned-clock-rates and querying the
>>> clock provider for rates be used here?
>>
>> From what I understood, assigned-clock-rates would only work for setting
>> some default clock rates for certain clocks by calling
>> of_clk_set_defaults.
>>
>> Whereas the requirement here is -
>> That since SDHC msm directly controls the clk(core clock) at source, it's
>> sdhci-msm driver needs to know the supported clk-rates by the underlying
>> platform to configure the nearest floor value supported on this platform
>> (when the request arrives from the core layer to switch the clock).
>
> Why does clk_round_rate not work for you? That will round down to the
> nearest frequency supported.
clk_round_rate will round off to nearest supported "ceil" frequency.
But we require nearest rounded off "floor" frequency.

>
>> Hence the table of clk-rates is provided for sdhci-msm.
>>
>>>
>>> Minimally this would need unit suffix and either be made common or have
>>> a vendor prefix.
>>
>> Sure will this work in that case - "qcom-clk-rates"
>
> "qcom,clk-rates", but I'm not yet convinced this is right.
>
> Rob
> --
> To unsubscribe from this list: send the line "unsubscribe linux-mmc" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html
>

Re: [PATCH v5 03/12] mmc: sdhci-msm: add pltfm_data support to get clk-rates from DT

[Rob Herring]

On Tue, Oct 11, 2016 at 4:06 AM, Ritesh Harjani <riteshh@codeaurora.org> wrote:
> Hi Rob
>
>
> On 10/11/2016 12:59 AM, Rob Herring wrote:
>>
>> On Mon, Oct 10, 2016 at 11:07 AM, Ritesh Harjani <riteshh@codeaurora.org>
>> wrote:
>>>
>>> Hi Rob,
>>>
>>> Thanks for review.
>>>
>>> On 10/10/2016 6:27 PM, Rob Herring wrote:
>>>>
>>>>
>>>> On Wed, Oct 05, 2016 at 08:10:31PM +0530, Ritesh Harjani wrote:
>>>>>
>>>>>
>>>>> This adds support for sdhc-msm controllers to get supported
>>>>> clk-rates from DT. sdhci-msm would need it's own set_clock
>>>>> ops to be implemented. For this, supported clk-rates needs
>>>>> to be populated in sdhci_msm_pltfm_data.
>>>>>
>>>>> Signed-off-by: Ritesh Harjani <riteshh@codeaurora.org>
>>>>> ---
>>>>>  .../devicetree/bindings/mmc/sdhci-msm.txt          |  1 +
>>>>>  drivers/mmc/host/sdhci-msm.c                       | 48
>>>>> ++++++++++++++++++++++
>>>>>  2 files changed, 49 insertions(+)
>>>>>
>>>>> diff --git a/Documentation/devicetree/bindings/mmc/sdhci-msm.txt
>>>>> b/Documentation/devicetree/bindings/mmc/sdhci-msm.txt
>>>>> index 485483a..6a83b38 100644
>>>>> --- a/Documentation/devicetree/bindings/mmc/sdhci-msm.txt
>>>>> +++ b/Documentation/devicetree/bindings/mmc/sdhci-msm.txt
>>>>> @@ -17,6 +17,7 @@ Required properties:
>>>>>         "iface" - Main peripheral bus clock (PCLK/HCLK - AHB Bus clock)
>>>>> (required)
>>>>>         "core"  - SDC MMC clock (MCLK) (required)
>>>>>         "bus"   - SDCC bus voter clock (optional)
>>>>> +- clk-rates: Array of supported GCC clock frequencies for sdhc, Units
>>>>> -
>>>>> Hz.
>>>>
>>>>
>>>>
>>>> Why can't some combination of assigned-clock-rates and querying the
>>>> clock provider for rates be used here?
>>>
>>>
>>> From what I understood, assigned-clock-rates would only work for setting
>>> some default clock rates for certain clocks by calling
>>> of_clk_set_defaults.
>>>
>>> Whereas the requirement here is -
>>> That since SDHC msm directly controls the clk(core clock) at source, it's
>>> sdhci-msm driver needs to know the supported clk-rates by the underlying
>>> platform to configure the nearest floor value supported on this platform
>>> (when the request arrives from the core layer to switch the clock).
>>
>>
>> Why does clk_round_rate not work for you? That will round down to the
>> nearest frequency supported.
>
> clk_round_rate will round off to nearest supported "ceil" frequency.
> But we require nearest rounded off "floor" frequency.

Then fix the clk framework to do what you want. This doesn't need to be in DT.

Rob

Re: [PATCH v5 03/12] mmc: sdhci-msm: add pltfm_data support to get clk-rates from DT

[Ritesh Harjani]

Hi Rob,

On 10/11/2016 6:01 PM, Rob Herring wrote:
> On Tue, Oct 11, 2016 at 4:06 AM, Ritesh Harjani <riteshh@codeaurora.org> wrote:
>> Hi Rob
>>
>>
>> On 10/11/2016 12:59 AM, Rob Herring wrote:
>>>
>>> On Mon, Oct 10, 2016 at 11:07 AM, Ritesh Harjani <riteshh@codeaurora.org>
>>> wrote:
>>>>
>>>> Hi Rob,
>>>>
>>>> Thanks for review.
>>>>
>>>> On 10/10/2016 6:27 PM, Rob Herring wrote:
>>>>>
>>>>>
>>>>> On Wed, Oct 05, 2016 at 08:10:31PM +0530, Ritesh Harjani wrote:
>>>>>>
>>>>>>
>>>>>> This adds support for sdhc-msm controllers to get supported
>>>>>> clk-rates from DT. sdhci-msm would need it's own set_clock
>>>>>> ops to be implemented. For this, supported clk-rates needs
>>>>>> to be populated in sdhci_msm_pltfm_data.
>>>>>>
>>>>>> Signed-off-by: Ritesh Harjani <riteshh@codeaurora.org>
>>>>>> ---
>>>>>>  .../devicetree/bindings/mmc/sdhci-msm.txt          |  1 +
>>>>>>  drivers/mmc/host/sdhci-msm.c                       | 48
>>>>>> ++++++++++++++++++++++
>>>>>>  2 files changed, 49 insertions(+)
>>>>>>
>>>>>> diff --git a/Documentation/devicetree/bindings/mmc/sdhci-msm.txt
>>>>>> b/Documentation/devicetree/bindings/mmc/sdhci-msm.txt
>>>>>> index 485483a..6a83b38 100644
>>>>>> --- a/Documentation/devicetree/bindings/mmc/sdhci-msm.txt
>>>>>> +++ b/Documentation/devicetree/bindings/mmc/sdhci-msm.txt
>>>>>> @@ -17,6 +17,7 @@ Required properties:
>>>>>>         "iface" - Main peripheral bus clock (PCLK/HCLK - AHB Bus clock)
>>>>>> (required)
>>>>>>         "core"  - SDC MMC clock (MCLK) (required)
>>>>>>         "bus"   - SDCC bus voter clock (optional)
>>>>>> +- clk-rates: Array of supported GCC clock frequencies for sdhc, Units
>>>>>> -
>>>>>> Hz.
>>>>>
>>>>>
>>>>>
>>>>> Why can't some combination of assigned-clock-rates and querying the
>>>>> clock provider for rates be used here?
>>>>
>>>>
>>>> From what I understood, assigned-clock-rates would only work for setting
>>>> some default clock rates for certain clocks by calling
>>>> of_clk_set_defaults.
>>>>
>>>> Whereas the requirement here is -
>>>> That since SDHC msm directly controls the clk(core clock) at source, it's
>>>> sdhci-msm driver needs to know the supported clk-rates by the underlying
>>>> platform to configure the nearest floor value supported on this platform
>>>> (when the request arrives from the core layer to switch the clock).
>>>
>>>
>>> Why does clk_round_rate not work for you? That will round down to the
>>> nearest frequency supported.
>>
>> clk_round_rate will round off to nearest supported "ceil" frequency.
>> But we require nearest rounded off "floor" frequency.
>
> Then fix the clk framework to do what you want. This doesn't need to be in DT.
Sure. Discussed with clk driver. Will make the required changes in qcom 
clk driver to fix this.

Will soon publish the new series.

>
> Rob
> --
> To unsubscribe from this list: send the line "unsubscribe linux-mmc" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html
>

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

[PATCH v5 04/12] ARM: dts: qcom: Add clk-rates to sdhc1 & sdhc2

[Ritesh Harjani]

Add msm supported clk-rates for all sdhc nodes.

Signed-off-by: Ritesh Harjani <riteshh@codeaurora.org>
---
 arch/arm/boot/dts/qcom-apq8084.dtsi   | 4 ++++
 arch/arm/boot/dts/qcom-msm8974.dtsi   | 4 ++++
 arch/arm64/boot/dts/qcom/msm8916.dtsi | 4 ++++
 arch/arm64/boot/dts/qcom/msm8996.dtsi | 2 ++
 4 files changed, 14 insertions(+)

diff --git a/arch/arm/boot/dts/qcom-apq8084.dtsi b/arch/arm/boot/dts/qcom-apq8084.dtsi
index 41e09c8..b3ce5e5 100644
--- a/arch/arm/boot/dts/qcom-apq8084.dtsi
+++ b/arch/arm/boot/dts/qcom-apq8084.dtsi
@@ -434,6 +434,8 @@
 			interrupt-names = "hc_irq", "pwr_irq";
 			clocks = <&gcc GCC_SDCC1_APPS_CLK>, <&gcc GCC_SDCC1_AHB_CLK>;
 			clock-names = "core", "iface";
+			clk-rates = <400000 25000000 50000000 100000000
+					200000000>;
 			status = "disabled";
 		};
 
@@ -445,6 +447,8 @@
 			interrupt-names = "hc_irq", "pwr_irq";
 			clocks = <&gcc GCC_SDCC2_APPS_CLK>, <&gcc GCC_SDCC2_AHB_CLK>;
 			clock-names = "core", "iface";
+			clk-rates = <400000 25000000 50000000 100000000
+					200000000>;
 			status = "disabled";
 		};
 
diff --git a/arch/arm/boot/dts/qcom-msm8974.dtsi b/arch/arm/boot/dts/qcom-msm8974.dtsi
index f9b0d90..6c78f3e 100644
--- a/arch/arm/boot/dts/qcom-msm8974.dtsi
+++ b/arch/arm/boot/dts/qcom-msm8974.dtsi
@@ -566,6 +566,8 @@
 			interrupt-names = "hc_irq", "pwr_irq";
 			clocks = <&gcc GCC_SDCC1_APPS_CLK>, <&gcc GCC_SDCC1_AHB_CLK>;
 			clock-names = "core", "iface";
+			clk-rates = <400000 25000000 50000000 100000000
+					200000000>;
 			status = "disabled";
 		};
 
@@ -577,6 +579,8 @@
 			interrupt-names = "hc_irq", "pwr_irq";
 			clocks = <&gcc GCC_SDCC2_APPS_CLK>, <&gcc GCC_SDCC2_AHB_CLK>;
 			clock-names = "core", "iface";
+			clk-rates = <400000 25000000 50000000 100000000
+					200000000>;
 			status = "disabled";
 		};
 
diff --git a/arch/arm64/boot/dts/qcom/msm8916.dtsi b/arch/arm64/boot/dts/qcom/msm8916.dtsi
index b5b6014..2f02058 100644
--- a/arch/arm64/boot/dts/qcom/msm8916.dtsi
+++ b/arch/arm64/boot/dts/qcom/msm8916.dtsi
@@ -512,6 +512,8 @@
 			clocks = <&gcc GCC_SDCC1_APPS_CLK>,
 				 <&gcc GCC_SDCC1_AHB_CLK>;
 			clock-names = "core", "iface";
+			clk-rates = <400000 25000000 50000000 100000000
+					177770000>;
 			bus-width = <8>;
 			non-removable;
 			status = "disabled";
@@ -527,6 +529,8 @@
 			clocks = <&gcc GCC_SDCC2_APPS_CLK>,
 				 <&gcc GCC_SDCC2_AHB_CLK>;
 			clock-names = "core", "iface";
+			clk-rates = <400000 25000000 50000000 100000000
+					200000000>;
 			bus-width = <4>;
 			status = "disabled";
 		};
diff --git a/arch/arm64/boot/dts/qcom/msm8996.dtsi b/arch/arm64/boot/dts/qcom/msm8996.dtsi
index 8fb4747..cd1ac12 100644
--- a/arch/arm64/boot/dts/qcom/msm8996.dtsi
+++ b/arch/arm64/boot/dts/qcom/msm8996.dtsi
@@ -410,6 +410,8 @@
 			 clock-names = "iface", "core";
 			 clocks = <&gcc GCC_SDCC2_AHB_CLK>,
 			 <&gcc GCC_SDCC2_APPS_CLK>;
+			 clk-rates = <400000 25000000 50000000 100000000
+					200000000>;
 			 bus-width = <4>;
 		 };
 
-- 
The Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum, 
a Linux Foundation Collaborative Project.

[PATCH v5 05/12] mmc: sdhci-msm: Add get_min_clock() and get_max_clock() callback

[Ritesh Harjani]

This add get_min_clock() and get_max_clock() callback
for sdhci-msm. sdhci-msm min/max clocks may be different
hence implement these callbacks.

Signed-off-by: Sahitya Tummala <stummala@codeaurora.org>
Signed-off-by: Ritesh Harjani <riteshh@codeaurora.org>
---
 drivers/mmc/host/sdhci-msm.c | 29 +++++++++++++++++++++++++++++
 1 file changed, 29 insertions(+)

diff --git a/drivers/mmc/host/sdhci-msm.c b/drivers/mmc/host/sdhci-msm.c
index 042ecb2..4e17201 100644
--- a/drivers/mmc/host/sdhci-msm.c
+++ b/drivers/mmc/host/sdhci-msm.c
@@ -68,6 +68,7 @@
 #define CORE_VENDOR_SPEC_CAPABILITIES0	0x11c
 
 #define TCXO_FREQ		19200000
+#define SDHCI_MSM_MIN_CLOCK	400000
 
 #define CDR_SELEXT_SHIFT	20
 #define CDR_SELEXT_MASK		(0xf << CDR_SELEXT_SHIFT)
@@ -561,6 +562,32 @@ static irqreturn_t sdhci_msm_pwr_irq(int irq, void *data)
 	return IRQ_HANDLED;
 }
 
+static unsigned int sdhci_msm_get_max_clock(struct sdhci_host *host)
+{
+	struct sdhci_pltfm_host *pltfm_host = sdhci_priv(host);
+	struct sdhci_msm_host *msm_host = sdhci_pltfm_priv(pltfm_host);
+	int count;
+
+	if (msm_host->clk_table) {
+		count = msm_host->clk_table_sz;
+		return msm_host->clk_table[count - 1];
+	} else {
+		return clk_round_rate(msm_host->clk, ULONG_MAX);
+	}
+}
+
+static unsigned int sdhci_msm_get_min_clock(struct sdhci_host *host)
+{
+	struct sdhci_pltfm_host *pltfm_host = sdhci_priv(host);
+	struct sdhci_msm_host *msm_host = sdhci_pltfm_priv(pltfm_host);
+
+	if (msm_host->clk_table) {
+		return msm_host->clk_table[0];
+	} else {
+		return SDHCI_MSM_MIN_CLOCK;
+	}
+}
+
 static const struct of_device_id sdhci_msm_dt_match[] = {
 	{ .compatible = "qcom,sdhci-msm-v4" },
 	{},
@@ -572,6 +599,8 @@ static const struct sdhci_ops sdhci_msm_ops = {
 	.platform_execute_tuning = sdhci_msm_execute_tuning,
 	.reset = sdhci_reset,
 	.set_clock = sdhci_set_clock,
+	.get_min_clock = sdhci_msm_get_min_clock,
+	.get_max_clock = sdhci_msm_get_max_clock,
 	.set_bus_width = sdhci_set_bus_width,
 	.set_uhs_signaling = sdhci_msm_set_uhs_signaling,
 	.voltage_switch = sdhci_msm_voltage_switch,
-- 
The Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum, 
a Linux Foundation Collaborative Project.

Re: [PATCH v5 05/12] mmc: sdhci-msm: Add get_min_clock() and get_max_clock() callback

[Adrian Hunter]

On 05/10/16 17:40, Ritesh Harjani wrote:
> This add get_min_clock() and get_max_clock() callback
> for sdhci-msm. sdhci-msm min/max clocks may be different
> hence implement these callbacks.
> 
> Signed-off-by: Sahitya Tummala <stummala@codeaurora.org>
> Signed-off-by: Ritesh Harjani <riteshh@codeaurora.org>
> ---
>  drivers/mmc/host/sdhci-msm.c | 29 +++++++++++++++++++++++++++++
>  1 file changed, 29 insertions(+)
> 
> diff --git a/drivers/mmc/host/sdhci-msm.c b/drivers/mmc/host/sdhci-msm.c
> index 042ecb2..4e17201 100644
> --- a/drivers/mmc/host/sdhci-msm.c
> +++ b/drivers/mmc/host/sdhci-msm.c
> @@ -68,6 +68,7 @@
>  #define CORE_VENDOR_SPEC_CAPABILITIES0	0x11c
>  
>  #define TCXO_FREQ		19200000
> +#define SDHCI_MSM_MIN_CLOCK	400000
>  
>  #define CDR_SELEXT_SHIFT	20
>  #define CDR_SELEXT_MASK		(0xf << CDR_SELEXT_SHIFT)
> @@ -561,6 +562,32 @@ static irqreturn_t sdhci_msm_pwr_irq(int irq, void *data)
>  	return IRQ_HANDLED;
>  }
>  
> +static unsigned int sdhci_msm_get_max_clock(struct sdhci_host *host)
> +{
> +	struct sdhci_pltfm_host *pltfm_host = sdhci_priv(host);
> +	struct sdhci_msm_host *msm_host = sdhci_pltfm_priv(pltfm_host);
> +	int count;
> +
> +	if (msm_host->clk_table) {

I don't know if DT allows empty arrays, but if it does, might not
msm_host->clk_table_sz be zero here.

> +		count = msm_host->clk_table_sz;
> +		return msm_host->clk_table[count - 1];
> +	} else {

'else' is redundant here.

> +		return clk_round_rate(msm_host->clk, ULONG_MAX);
> +	}
> +}
> +
> +static unsigned int sdhci_msm_get_min_clock(struct sdhci_host *host)
> +{
> +	struct sdhci_pltfm_host *pltfm_host = sdhci_priv(host);
> +	struct sdhci_msm_host *msm_host = sdhci_pltfm_priv(pltfm_host);
> +
> +	if (msm_host->clk_table) {

Same about possibility of msm_host->clk_table_sz being zero.

> +		return msm_host->clk_table[0];
> +	} else {

'else' is redundant here.

> +		return SDHCI_MSM_MIN_CLOCK;
> +	}
> +}
> +
>  static const struct of_device_id sdhci_msm_dt_match[] = {
>  	{ .compatible = "qcom,sdhci-msm-v4" },
>  	{},
> @@ -572,6 +599,8 @@ static const struct sdhci_ops sdhci_msm_ops = {
>  	.platform_execute_tuning = sdhci_msm_execute_tuning,
>  	.reset = sdhci_reset,
>  	.set_clock = sdhci_set_clock,
> +	.get_min_clock = sdhci_msm_get_min_clock,
> +	.get_max_clock = sdhci_msm_get_max_clock,
>  	.set_bus_width = sdhci_set_bus_width,
>  	.set_uhs_signaling = sdhci_msm_set_uhs_signaling,
>  	.voltage_switch = sdhci_msm_voltage_switch,
> 

Re: [PATCH v5 05/12] mmc: sdhci-msm: Add get_min_clock() and get_max_clock() callback

[Ritesh Harjani]

Hi Adrian,

On 10/10/2016 3:16 PM, Adrian Hunter wrote:
> On 05/10/16 17:40, Ritesh Harjani wrote:
>> This add get_min_clock() and get_max_clock() callback
>> for sdhci-msm. sdhci-msm min/max clocks may be different
>> hence implement these callbacks.
>>
>> Signed-off-by: Sahitya Tummala <stummala@codeaurora.org>
>> Signed-off-by: Ritesh Harjani <riteshh@codeaurora.org>
>> ---
>>  drivers/mmc/host/sdhci-msm.c | 29 +++++++++++++++++++++++++++++
>>  1 file changed, 29 insertions(+)
>>
>> diff --git a/drivers/mmc/host/sdhci-msm.c b/drivers/mmc/host/sdhci-msm.c
>> index 042ecb2..4e17201 100644
>> --- a/drivers/mmc/host/sdhci-msm.c
>> +++ b/drivers/mmc/host/sdhci-msm.c
>> @@ -68,6 +68,7 @@
>>  #define CORE_VENDOR_SPEC_CAPABILITIES0	0x11c
>>
>>  #define TCXO_FREQ		19200000
>> +#define SDHCI_MSM_MIN_CLOCK	400000
>>
>>  #define CDR_SELEXT_SHIFT	20
>>  #define CDR_SELEXT_MASK		(0xf << CDR_SELEXT_SHIFT)
>> @@ -561,6 +562,32 @@ static irqreturn_t sdhci_msm_pwr_irq(int irq, void *data)
>>  	return IRQ_HANDLED;
>>  }
>>
>> +static unsigned int sdhci_msm_get_max_clock(struct sdhci_host *host)
>> +{
>> +	struct sdhci_pltfm_host *pltfm_host = sdhci_priv(host);
>> +	struct sdhci_msm_host *msm_host = sdhci_pltfm_priv(pltfm_host);
>> +	int count;
>> +
>> +	if (msm_host->clk_table) {
>
> I don't know if DT allows empty arrays, but if it does, might not
> msm_host->clk_table_sz be zero here.
Sure, I will take care of this as mentioned in the response of previous 
patch 03.

>
>> +		count = msm_host->clk_table_sz;
>> +		return msm_host->clk_table[count - 1];
>> +	} else {
>
> 'else' is redundant here.
Done. Will remove it.

>
>> +		return clk_round_rate(msm_host->clk, ULONG_MAX);
>> +	}
>> +}
>> +
>> +static unsigned int sdhci_msm_get_min_clock(struct sdhci_host *host)
>> +{
>> +	struct sdhci_pltfm_host *pltfm_host = sdhci_priv(host);
>> +	struct sdhci_msm_host *msm_host = sdhci_pltfm_priv(pltfm_host);
>> +
>> +	if (msm_host->clk_table) {
>
> Same about possibility of msm_host->clk_table_sz being zero.
Yes, as mentioned. Will take care of this.

>
>> +		return msm_host->clk_table[0];
>> +	} else {
>
> 'else' is redundant here.
Done.

>
>> +		return SDHCI_MSM_MIN_CLOCK;
>> +	}
>> +}
>> +
>>  static const struct of_device_id sdhci_msm_dt_match[] = {
>>  	{ .compatible = "qcom,sdhci-msm-v4" },
>>  	{},
>> @@ -572,6 +599,8 @@ static const struct sdhci_ops sdhci_msm_ops = {
>>  	.platform_execute_tuning = sdhci_msm_execute_tuning,
>>  	.reset = sdhci_reset,
>>  	.set_clock = sdhci_set_clock,
>> +	.get_min_clock = sdhci_msm_get_min_clock,
>> +	.get_max_clock = sdhci_msm_get_max_clock,
>>  	.set_bus_width = sdhci_set_bus_width,
>>  	.set_uhs_signaling = sdhci_msm_set_uhs_signaling,
>>  	.voltage_switch = sdhci_msm_voltage_switch,
>>
>
> --
> To unsubscribe from this list: send the line "unsubscribe linux-mmc" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html
>

[PATCH v5 06/12] mmc: sdhci-msm: Enable few quirks

[Ritesh Harjani]

sdhc-msm controller needs this SDHCI_QUIRK_CAP_CLOCK_BASE_BROKEN
& SDHCI_QUIRK2_PRESET_VALUE_BROKEN to be set. Hence setting it.

Signed-off-by: Sahitya Tummala <stummala@codeaurora.org>
Signed-off-by: Ritesh Harjani <riteshh@codeaurora.org>
Acked-by: Adrian Hunter <adrian.hunter@intel.com>
---
 drivers/mmc/host/sdhci-msm.c | 4 +++-
 1 file changed, 3 insertions(+), 1 deletion(-)

diff --git a/drivers/mmc/host/sdhci-msm.c b/drivers/mmc/host/sdhci-msm.c
index 4e17201..542ddad 100644
--- a/drivers/mmc/host/sdhci-msm.c
+++ b/drivers/mmc/host/sdhci-msm.c
@@ -609,7 +609,9 @@ static const struct sdhci_ops sdhci_msm_ops = {
 static const struct sdhci_pltfm_data sdhci_msm_pdata = {
 	.quirks = SDHCI_QUIRK_BROKEN_CARD_DETECTION |
 		  SDHCI_QUIRK_NO_CARD_NO_RESET |
-		  SDHCI_QUIRK_SINGLE_POWER_WRITE,
+		  SDHCI_QUIRK_SINGLE_POWER_WRITE |
+		  SDHCI_QUIRK_CAP_CLOCK_BASE_BROKEN,
+	.quirks2 = SDHCI_QUIRK2_PRESET_VALUE_BROKEN,
 	.ops = &sdhci_msm_ops,
 };
 
-- 
The Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum, 
a Linux Foundation Collaborative Project.

[PATCH v5 07/12] mmc: sdhci-msm: Implement set_clock callback for sdhci-msm

[Ritesh Harjani]

sdhci-msm controller may have different clk-rates for each
bus speed mode. Thus implement set_clock callback for
sdhci-msm driver.

Signed-off-by: Sahitya Tummala <stummala@codeaurora.org>
Signed-off-by: Ritesh Harjani <riteshh@codeaurora.org>
---
 drivers/mmc/host/sdhci-msm.c | 110 ++++++++++++++++++++++++++++++++++++++++++-
 1 file changed, 109 insertions(+), 1 deletion(-)

diff --git a/drivers/mmc/host/sdhci-msm.c b/drivers/mmc/host/sdhci-msm.c
index 542ddad..9d18cf0 100644
--- a/drivers/mmc/host/sdhci-msm.c
+++ b/drivers/mmc/host/sdhci-msm.c
@@ -84,6 +84,7 @@ struct sdhci_msm_host {
 	struct clk *bus_clk;	/* SDHC bus voter clock */
 	u32 *clk_table;
 	int clk_table_sz;
+	u32 clk_rate;
 	struct mmc_host *mmc;
 	bool use_14lpp_dll_reset;
 };
@@ -588,6 +589,113 @@ static unsigned int sdhci_msm_get_min_clock(struct sdhci_host *host)
 	}
 }
 
+static unsigned int sdhci_msm_get_msm_clk_rate(struct sdhci_host *host,
+					u32 req_clk)
+{
+	struct sdhci_pltfm_host *pltfm_host = sdhci_priv(host);
+	struct sdhci_msm_host *msm_host = sdhci_pltfm_priv(pltfm_host);
+	int count;
+	unsigned int sel_clk = -1;
+
+	if (!msm_host->clk_table)
+		return clk_round_rate(msm_host->clk, ULONG_MAX);
+
+	count = msm_host->clk_table_sz;
+
+	while (count--) {
+		sel_clk = msm_host->clk_table[count];
+		if (req_clk >= sel_clk)
+			return sel_clk;
+	}
+
+	return sel_clk;
+}
+
+/**
+ * __sdhci_msm_set_clock - sdhci_msm clock control.
+ *
+ * Description:
+ * Implement MSM version of sdhci_set_clock.
+ * This is required since MSM controller does not
+ * use internal divider and instead directly control
+ * the GCC clock as per HW recommendation.
+ **/
+void __sdhci_msm_set_clock(struct sdhci_host *host, unsigned int clock)
+{
+	u16 clk;
+	unsigned long timeout;
+
+	/*
+	 * Keep actual_clock as zero -
+	 * - since there is no divider used so no need of having actual_clock.
+	 * - MSM controller uses SDCLK for data timeout calculation. If
+	 *   actual_clock is zero, host->clock is taken for calculation.
+	 */
+	host->mmc->actual_clock = 0;
+
+	sdhci_writew(host, 0, SDHCI_CLOCK_CONTROL);
+
+	if (clock == 0)
+		return;
+
+	/*
+	 * MSM controller do not use clock divider.
+	 * Thus read SDHCI_CLOCK_CONTROL and only enable
+	 * clock with no divider value programmed.
+	 */
+	clk = sdhci_readw(host, SDHCI_CLOCK_CONTROL);
+
+	clk |= SDHCI_CLOCK_INT_EN;
+	sdhci_writew(host, clk, SDHCI_CLOCK_CONTROL);
+
+	/* Wait max 20 ms */
+	timeout = 20;
+	while (!((clk = sdhci_readw(host, SDHCI_CLOCK_CONTROL))
+		& SDHCI_CLOCK_INT_STABLE)) {
+		if (timeout == 0) {
+			pr_err("%s: Internal clock never stabilised.\n",
+			       mmc_hostname(host->mmc));
+			return;
+		}
+		timeout--;
+		mdelay(1);
+	}
+
+	clk |= SDHCI_CLOCK_CARD_EN;
+	sdhci_writew(host, clk, SDHCI_CLOCK_CONTROL);
+}
+
+static void sdhci_msm_set_clock(struct sdhci_host *host, unsigned int clock)
+{
+	struct sdhci_pltfm_host *pltfm_host = sdhci_priv(host);
+	struct sdhci_msm_host *msm_host = sdhci_pltfm_priv(pltfm_host);
+	u32 msm_clock;
+	int rc;
+
+	if (!clock)
+		goto out;
+
+	spin_unlock_irq(&host->lock);
+	if ((clock != msm_host->clk_rate) && msm_host->clk_table) {
+		msm_clock = sdhci_msm_get_msm_clk_rate(host, clock);
+		rc = clk_set_rate(msm_host->clk, msm_clock);
+		if (rc) {
+			pr_err("%s: failed to set clock at rate %u, requested clock rate %u\n",
+				mmc_hostname(host->mmc), msm_clock, clock);
+			goto out;
+		}
+		msm_host->clk_rate = clock;
+		pr_debug("%s: setting clock at rate %lu\n",
+			mmc_hostname(host->mmc), clk_get_rate(msm_host->clk));
+	}
+
+	spin_lock_irq(&host->lock);
+out:
+	if (!msm_host->clk_table)
+		return sdhci_set_clock(host, clock);
+	__sdhci_msm_set_clock(host, clock);
+}
+
 static const struct of_device_id sdhci_msm_dt_match[] = {
 	{ .compatible = "qcom,sdhci-msm-v4" },
 	{},
@@ -598,7 +706,7 @@ MODULE_DEVICE_TABLE(of, sdhci_msm_dt_match);
 static const struct sdhci_ops sdhci_msm_ops = {
 	.platform_execute_tuning = sdhci_msm_execute_tuning,
 	.reset = sdhci_reset,
-	.set_clock = sdhci_set_clock,
+	.set_clock = sdhci_msm_set_clock,
 	.get_min_clock = sdhci_msm_get_min_clock,
 	.get_max_clock = sdhci_msm_get_max_clock,
 	.set_bus_width = sdhci_set_bus_width,
-- 
The Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum, 
a Linux Foundation Collaborative Project.

Re: [PATCH v5 07/12] mmc: sdhci-msm: Implement set_clock callback for sdhci-msm

[Adrian Hunter]

On 05/10/16 17:40, Ritesh Harjani wrote:
> sdhci-msm controller may have different clk-rates for each
> bus speed mode. Thus implement set_clock callback for
> sdhci-msm driver.
> 
> Signed-off-by: Sahitya Tummala <stummala-sgV2jX0FEOL9JmXXK+q4OQ@public.gmane.org>
> Signed-off-by: Ritesh Harjani <riteshh-sgV2jX0FEOL9JmXXK+q4OQ@public.gmane.org>
> ---
>  drivers/mmc/host/sdhci-msm.c | 110 ++++++++++++++++++++++++++++++++++++++++++-
>  1 file changed, 109 insertions(+), 1 deletion(-)
> 
> diff --git a/drivers/mmc/host/sdhci-msm.c b/drivers/mmc/host/sdhci-msm.c
> index 542ddad..9d18cf0 100644
> --- a/drivers/mmc/host/sdhci-msm.c
> +++ b/drivers/mmc/host/sdhci-msm.c
> @@ -84,6 +84,7 @@ struct sdhci_msm_host {
>  	struct clk *bus_clk;	/* SDHC bus voter clock */
>  	u32 *clk_table;
>  	int clk_table_sz;
> +	u32 clk_rate;
>  	struct mmc_host *mmc;
>  	bool use_14lpp_dll_reset;
>  };
> @@ -588,6 +589,113 @@ static unsigned int sdhci_msm_get_min_clock(struct sdhci_host *host)
>  	}
>  }
>  
> +static unsigned int sdhci_msm_get_msm_clk_rate(struct sdhci_host *host,
> +					u32 req_clk)
> +{
> +	struct sdhci_pltfm_host *pltfm_host = sdhci_priv(host);
> +	struct sdhci_msm_host *msm_host = sdhci_pltfm_priv(pltfm_host);
> +	int count;
> +	unsigned int sel_clk = -1;
> +
> +	if (!msm_host->clk_table)
> +		return clk_round_rate(msm_host->clk, ULONG_MAX);
> +
> +	count = msm_host->clk_table_sz;
> +
> +	while (count--) {
> +		sel_clk = msm_host->clk_table[count];
> +		if (req_clk >= sel_clk)
> +			return sel_clk;
> +	}
> +
> +	return sel_clk;
> +}
> +
> +/**
> + * __sdhci_msm_set_clock - sdhci_msm clock control.
> + *
> + * Description:
> + * Implement MSM version of sdhci_set_clock.
> + * This is required since MSM controller does not
> + * use internal divider and instead directly control
> + * the GCC clock as per HW recommendation.
> + **/
> +void __sdhci_msm_set_clock(struct sdhci_host *host, unsigned int clock)
> +{
> +	u16 clk;
> +	unsigned long timeout;
> +
> +	/*
> +	 * Keep actual_clock as zero -
> +	 * - since there is no divider used so no need of having actual_clock.
> +	 * - MSM controller uses SDCLK for data timeout calculation. If
> +	 *   actual_clock is zero, host->clock is taken for calculation.
> +	 */
> +	host->mmc->actual_clock = 0;
> +
> +	sdhci_writew(host, 0, SDHCI_CLOCK_CONTROL);
> +
> +	if (clock == 0)
> +		return;
> +
> +	/*
> +	 * MSM controller do not use clock divider.
> +	 * Thus read SDHCI_CLOCK_CONTROL and only enable
> +	 * clock with no divider value programmed.
> +	 */
> +	clk = sdhci_readw(host, SDHCI_CLOCK_CONTROL);
> +
> +	clk |= SDHCI_CLOCK_INT_EN;
> +	sdhci_writew(host, clk, SDHCI_CLOCK_CONTROL);
> +
> +	/* Wait max 20 ms */
> +	timeout = 20;
> +	while (!((clk = sdhci_readw(host, SDHCI_CLOCK_CONTROL))
> +		& SDHCI_CLOCK_INT_STABLE)) {
> +		if (timeout == 0) {
> +			pr_err("%s: Internal clock never stabilised.\n",
> +			       mmc_hostname(host->mmc));
> +			return;
> +		}
> +		timeout--;
> +		mdelay(1);
> +	}
> +
> +	clk |= SDHCI_CLOCK_CARD_EN;
> +	sdhci_writew(host, clk, SDHCI_CLOCK_CONTROL);
> +}
> +
> +static void sdhci_msm_set_clock(struct sdhci_host *host, unsigned int clock)
> +{
> +	struct sdhci_pltfm_host *pltfm_host = sdhci_priv(host);
> +	struct sdhci_msm_host *msm_host = sdhci_pltfm_priv(pltfm_host);
> +	u32 msm_clock;
> +	int rc;
> +
> +	if (!clock)

Wouldn't you still need to set msm_host->clk_rate = clock in this case

> +		goto out;
> +
> +	spin_unlock_irq(&host->lock);
> +	if ((clock != msm_host->clk_rate) && msm_host->clk_table) {
> +		msm_clock = sdhci_msm_get_msm_clk_rate(host, clock);
> +		rc = clk_set_rate(msm_host->clk, msm_clock);
> +		if (rc) {
> +			pr_err("%s: failed to set clock at rate %u, requested clock rate %u\n",
> +				mmc_hostname(host->mmc), msm_clock, clock);
> +			goto out;
> +		}
> +		msm_host->clk_rate = clock;
> +		pr_debug("%s: setting clock at rate %lu\n",
> +			mmc_hostname(host->mmc), clk_get_rate(msm_host->clk));
> +	}
> +
> +	spin_lock_irq(&host->lock);
> +out:
> +	if (!msm_host->clk_table)
> +		return sdhci_set_clock(host, clock);

Could put the above 2 lines at the start and then no need to check
msm_host->clk_table again.

> +	__sdhci_msm_set_clock(host, clock);
> +}
> +
>  static const struct of_device_id sdhci_msm_dt_match[] = {
>  	{ .compatible = "qcom,sdhci-msm-v4" },
>  	{},
> @@ -598,7 +706,7 @@ MODULE_DEVICE_TABLE(of, sdhci_msm_dt_match);
>  static const struct sdhci_ops sdhci_msm_ops = {
>  	.platform_execute_tuning = sdhci_msm_execute_tuning,
>  	.reset = sdhci_reset,
> -	.set_clock = sdhci_set_clock,
> +	.set_clock = sdhci_msm_set_clock,
>  	.get_min_clock = sdhci_msm_get_min_clock,
>  	.get_max_clock = sdhci_msm_get_max_clock,
>  	.set_bus_width = sdhci_set_bus_width,
> 

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

Re: [PATCH v5 07/12] mmc: sdhci-msm: Implement set_clock callback for sdhci-msm

[Adrian Hunter]

On 10/10/16 13:16, Adrian Hunter wrote:
> On 05/10/16 17:40, Ritesh Harjani wrote:
>> sdhci-msm controller may have different clk-rates for each
>> bus speed mode. Thus implement set_clock callback for
>> sdhci-msm driver.
>>
>> Signed-off-by: Sahitya Tummala <stummala@codeaurora.org>
>> Signed-off-by: Ritesh Harjani <riteshh@codeaurora.org>
>> ---
>>  drivers/mmc/host/sdhci-msm.c | 110 ++++++++++++++++++++++++++++++++++++++++++-
>>  1 file changed, 109 insertions(+), 1 deletion(-)
>>
>> diff --git a/drivers/mmc/host/sdhci-msm.c b/drivers/mmc/host/sdhci-msm.c
>> index 542ddad..9d18cf0 100644
>> --- a/drivers/mmc/host/sdhci-msm.c
>> +++ b/drivers/mmc/host/sdhci-msm.c
>> @@ -84,6 +84,7 @@ struct sdhci_msm_host {
>>  	struct clk *bus_clk;	/* SDHC bus voter clock */
>>  	u32 *clk_table;
>>  	int clk_table_sz;
>> +	u32 clk_rate;
>>  	struct mmc_host *mmc;
>>  	bool use_14lpp_dll_reset;
>>  };
>> @@ -588,6 +589,113 @@ static unsigned int sdhci_msm_get_min_clock(struct sdhci_host *host)
>>  	}
>>  }
>>  
>> +static unsigned int sdhci_msm_get_msm_clk_rate(struct sdhci_host *host,
>> +					u32 req_clk)
>> +{
>> +	struct sdhci_pltfm_host *pltfm_host = sdhci_priv(host);
>> +	struct sdhci_msm_host *msm_host = sdhci_pltfm_priv(pltfm_host);
>> +	int count;
>> +	unsigned int sel_clk = -1;
>> +
>> +	if (!msm_host->clk_table)
>> +		return clk_round_rate(msm_host->clk, ULONG_MAX);
>> +
>> +	count = msm_host->clk_table_sz;
>> +
>> +	while (count--) {
>> +		sel_clk = msm_host->clk_table[count];
>> +		if (req_clk >= sel_clk)
>> +			return sel_clk;
>> +	}
>> +
>> +	return sel_clk;
>> +}
>> +
>> +/**
>> + * __sdhci_msm_set_clock - sdhci_msm clock control.
>> + *
>> + * Description:
>> + * Implement MSM version of sdhci_set_clock.
>> + * This is required since MSM controller does not
>> + * use internal divider and instead directly control
>> + * the GCC clock as per HW recommendation.
>> + **/
>> +void __sdhci_msm_set_clock(struct sdhci_host *host, unsigned int clock)
>> +{
>> +	u16 clk;
>> +	unsigned long timeout;
>> +
>> +	/*
>> +	 * Keep actual_clock as zero -
>> +	 * - since there is no divider used so no need of having actual_clock.
>> +	 * - MSM controller uses SDCLK for data timeout calculation. If
>> +	 *   actual_clock is zero, host->clock is taken for calculation.
>> +	 */
>> +	host->mmc->actual_clock = 0;
>> +
>> +	sdhci_writew(host, 0, SDHCI_CLOCK_CONTROL);
>> +
>> +	if (clock == 0)
>> +		return;
>> +
>> +	/*
>> +	 * MSM controller do not use clock divider.
>> +	 * Thus read SDHCI_CLOCK_CONTROL and only enable
>> +	 * clock with no divider value programmed.
>> +	 */
>> +	clk = sdhci_readw(host, SDHCI_CLOCK_CONTROL);
>> +
>> +	clk |= SDHCI_CLOCK_INT_EN;
>> +	sdhci_writew(host, clk, SDHCI_CLOCK_CONTROL);
>> +
>> +	/* Wait max 20 ms */
>> +	timeout = 20;
>> +	while (!((clk = sdhci_readw(host, SDHCI_CLOCK_CONTROL))
>> +		& SDHCI_CLOCK_INT_STABLE)) {
>> +		if (timeout == 0) {
>> +			pr_err("%s: Internal clock never stabilised.\n",
>> +			       mmc_hostname(host->mmc));
>> +			return;
>> +		}
>> +		timeout--;
>> +		mdelay(1);
>> +	}
>> +
>> +	clk |= SDHCI_CLOCK_CARD_EN;
>> +	sdhci_writew(host, clk, SDHCI_CLOCK_CONTROL);
>> +}
>> +
>> +static void sdhci_msm_set_clock(struct sdhci_host *host, unsigned int clock)
>> +{
>> +	struct sdhci_pltfm_host *pltfm_host = sdhci_priv(host);
>> +	struct sdhci_msm_host *msm_host = sdhci_pltfm_priv(pltfm_host);
>> +	u32 msm_clock;
>> +	int rc;
>> +
>> +	if (!clock)
> 
> Wouldn't you still need to set msm_host->clk_rate = clock in this case
> 
>> +		goto out;
>> +
>> +	spin_unlock_irq(&host->lock);
>> +	if ((clock != msm_host->clk_rate) && msm_host->clk_table) {
>> +		msm_clock = sdhci_msm_get_msm_clk_rate(host, clock);
>> +		rc = clk_set_rate(msm_host->clk, msm_clock);
>> +		if (rc) {
>> +			pr_err("%s: failed to set clock at rate %u, requested clock rate %u\n",
>> +				mmc_hostname(host->mmc), msm_clock, clock);
>> +			goto out;

'goto out' leaves spinlock unlocked

>> +		}
>> +		msm_host->clk_rate = clock;
>> +		pr_debug("%s: setting clock at rate %lu\n",
>> +			mmc_hostname(host->mmc), clk_get_rate(msm_host->clk));
>> +	}
>> +
>> +	spin_lock_irq(&host->lock);
>> +out:
>> +	if (!msm_host->clk_table)
>> +		return sdhci_set_clock(host, clock);
> 
> Could put the above 2 lines at the start and then no need to check
> msm_host->clk_table again.
> 
>> +	__sdhci_msm_set_clock(host, clock);
>> +}
>> +
>>  static const struct of_device_id sdhci_msm_dt_match[] = {
>>  	{ .compatible = "qcom,sdhci-msm-v4" },
>>  	{},
>> @@ -598,7 +706,7 @@ MODULE_DEVICE_TABLE(of, sdhci_msm_dt_match);
>>  static const struct sdhci_ops sdhci_msm_ops = {
>>  	.platform_execute_tuning = sdhci_msm_execute_tuning,
>>  	.reset = sdhci_reset,
>> -	.set_clock = sdhci_set_clock,
>> +	.set_clock = sdhci_msm_set_clock,
>>  	.get_min_clock = sdhci_msm_get_min_clock,
>>  	.get_max_clock = sdhci_msm_get_max_clock,
>>  	.set_bus_width = sdhci_set_bus_width,
>>
> 
> 

Re: [PATCH v5 07/12] mmc: sdhci-msm: Implement set_clock callback for sdhci-msm

[Ritesh Harjani]

Hi Adrian,


On 10/10/2016 3:53 PM, Adrian Hunter wrote:
> On 10/10/16 13:16, Adrian Hunter wrote:
>> On 05/10/16 17:40, Ritesh Harjani wrote:
>>> sdhci-msm controller may have different clk-rates for each
>>> bus speed mode. Thus implement set_clock callback for
>>> sdhci-msm driver.
>>>
>>> Signed-off-by: Sahitya Tummala <stummala-sgV2jX0FEOL9JmXXK+q4OQ@public.gmane.org>
>>> Signed-off-by: Ritesh Harjani <riteshh-sgV2jX0FEOL9JmXXK+q4OQ@public.gmane.org>
>>> ---
>>>  drivers/mmc/host/sdhci-msm.c | 110 ++++++++++++++++++++++++++++++++++++++++++-
>>>  1 file changed, 109 insertions(+), 1 deletion(-)
>>>
>>> diff --git a/drivers/mmc/host/sdhci-msm.c b/drivers/mmc/host/sdhci-msm.c
>>> index 542ddad..9d18cf0 100644
>>> --- a/drivers/mmc/host/sdhci-msm.c
>>> +++ b/drivers/mmc/host/sdhci-msm.c
>>> @@ -84,6 +84,7 @@ struct sdhci_msm_host {
>>>  	struct clk *bus_clk;	/* SDHC bus voter clock */
>>>  	u32 *clk_table;
>>>  	int clk_table_sz;
>>> +	u32 clk_rate;
>>>  	struct mmc_host *mmc;
>>>  	bool use_14lpp_dll_reset;
>>>  };
>>> @@ -588,6 +589,113 @@ static unsigned int sdhci_msm_get_min_clock(struct sdhci_host *host)
>>>  	}
>>>  }
>>>
>>> +static unsigned int sdhci_msm_get_msm_clk_rate(struct sdhci_host *host,
>>> +					u32 req_clk)
>>> +{
>>> +	struct sdhci_pltfm_host *pltfm_host = sdhci_priv(host);
>>> +	struct sdhci_msm_host *msm_host = sdhci_pltfm_priv(pltfm_host);
>>> +	int count;
>>> +	unsigned int sel_clk = -1;
>>> +
>>> +	if (!msm_host->clk_table)
>>> +		return clk_round_rate(msm_host->clk, ULONG_MAX);
>>> +
>>> +	count = msm_host->clk_table_sz;
>>> +
>>> +	while (count--) {
>>> +		sel_clk = msm_host->clk_table[count];
>>> +		if (req_clk >= sel_clk)
>>> +			return sel_clk;
>>> +	}
>>> +
>>> +	return sel_clk;
>>> +}
>>> +
>>> +/**
>>> + * __sdhci_msm_set_clock - sdhci_msm clock control.
>>> + *
>>> + * Description:
>>> + * Implement MSM version of sdhci_set_clock.
>>> + * This is required since MSM controller does not
>>> + * use internal divider and instead directly control
>>> + * the GCC clock as per HW recommendation.
>>> + **/
>>> +void __sdhci_msm_set_clock(struct sdhci_host *host, unsigned int clock)
>>> +{
>>> +	u16 clk;
>>> +	unsigned long timeout;
>>> +
>>> +	/*
>>> +	 * Keep actual_clock as zero -
>>> +	 * - since there is no divider used so no need of having actual_clock.
>>> +	 * - MSM controller uses SDCLK for data timeout calculation. If
>>> +	 *   actual_clock is zero, host->clock is taken for calculation.
>>> +	 */
>>> +	host->mmc->actual_clock = 0;
>>> +
>>> +	sdhci_writew(host, 0, SDHCI_CLOCK_CONTROL);
>>> +
>>> +	if (clock == 0)
>>> +		return;
>>> +
>>> +	/*
>>> +	 * MSM controller do not use clock divider.
>>> +	 * Thus read SDHCI_CLOCK_CONTROL and only enable
>>> +	 * clock with no divider value programmed.
>>> +	 */
>>> +	clk = sdhci_readw(host, SDHCI_CLOCK_CONTROL);
>>> +
>>> +	clk |= SDHCI_CLOCK_INT_EN;
>>> +	sdhci_writew(host, clk, SDHCI_CLOCK_CONTROL);
>>> +
>>> +	/* Wait max 20 ms */
>>> +	timeout = 20;
>>> +	while (!((clk = sdhci_readw(host, SDHCI_CLOCK_CONTROL))
>>> +		& SDHCI_CLOCK_INT_STABLE)) {
>>> +		if (timeout == 0) {
>>> +			pr_err("%s: Internal clock never stabilised.\n",
>>> +			       mmc_hostname(host->mmc));
>>> +			return;
>>> +		}
>>> +		timeout--;
>>> +		mdelay(1);
>>> +	}
>>> +
>>> +	clk |= SDHCI_CLOCK_CARD_EN;
>>> +	sdhci_writew(host, clk, SDHCI_CLOCK_CONTROL);
>>> +}
>>> +
>>> +static void sdhci_msm_set_clock(struct sdhci_host *host, unsigned int clock)
>>> +{
>>> +	struct sdhci_pltfm_host *pltfm_host = sdhci_priv(host);
>>> +	struct sdhci_msm_host *msm_host = sdhci_pltfm_priv(pltfm_host);
>>> +	u32 msm_clock;
>>> +	int rc;
>>> +
>>> +	if (!clock)
>>
>> Wouldn't you still need to set msm_host->clk_rate = clock in this case
Yes. Will do that.

>>
>>> +		goto out;
>>> +
>>> +	spin_unlock_irq(&host->lock);
>>> +	if ((clock != msm_host->clk_rate) && msm_host->clk_table) {
>>> +		msm_clock = sdhci_msm_get_msm_clk_rate(host, clock);
>>> +		rc = clk_set_rate(msm_host->clk, msm_clock);
>>> +		if (rc) {
>>> +			pr_err("%s: failed to set clock at rate %u, requested clock rate %u\n",
>>> +				mmc_hostname(host->mmc), msm_clock, clock);
>>> +			goto out;
>
> 'goto out' leaves spinlock unlocked
Thanks for catching it.

>
>>> +		}
>>> +		msm_host->clk_rate = clock;
>>> +		pr_debug("%s: setting clock at rate %lu\n",
>>> +			mmc_hostname(host->mmc), clk_get_rate(msm_host->clk));
>>> +	}
>>> +
>>> +	spin_lock_irq(&host->lock);
>>> +out:
>>> +	if (!msm_host->clk_table)
>>> +		return sdhci_set_clock(host, clock);
>>
>> Could put the above 2 lines at the start and then no need to check
>> msm_host->clk_table again.
I wanted to make this change, but then may be because of later patches 
which modifies sdhci_msm_set_clock for UHS modes, I did not move this to 
top.
I reviewed that part again, I will make the necessary changes.
Thanks.


>>
>>> +	__sdhci_msm_set_clock(host, clock);
>>> +}
>>> +
>>>  static const struct of_device_id sdhci_msm_dt_match[] = {
>>>  	{ .compatible = "qcom,sdhci-msm-v4" },
>>>  	{},
>>> @@ -598,7 +706,7 @@ MODULE_DEVICE_TABLE(of, sdhci_msm_dt_match);
>>>  static const struct sdhci_ops sdhci_msm_ops = {
>>>  	.platform_execute_tuning = sdhci_msm_execute_tuning,
>>>  	.reset = sdhci_reset,
>>> -	.set_clock = sdhci_set_clock,
>>> +	.set_clock = sdhci_msm_set_clock,
>>>  	.get_min_clock = sdhci_msm_get_min_clock,
>>>  	.get_max_clock = sdhci_msm_get_max_clock,
>>>  	.set_bus_width = sdhci_set_bus_width,
>>>
>>
>>
>
> --
> To unsubscribe from this list: send the line "unsubscribe linux-mmc" in
> the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html
>
--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

[PATCH v5 08/12] mmc: sdhci-msm: Add clock changes for DDR mode.

[Ritesh Harjani]

SDHC MSM controller need 2x clock for MCLK at GCC.
Hence make required changes to have 2x clock for
DDR timing modes.

Signed-off-by: Ritesh Harjani <riteshh@codeaurora.org>
---
 drivers/mmc/host/sdhci-msm.c | 25 ++++++++++++++++++++-----
 1 file changed, 20 insertions(+), 5 deletions(-)

diff --git a/drivers/mmc/host/sdhci-msm.c b/drivers/mmc/host/sdhci-msm.c
index 9d18cf0..eb1a9e3 100644
--- a/drivers/mmc/host/sdhci-msm.c
+++ b/drivers/mmc/host/sdhci-msm.c
@@ -669,6 +669,7 @@ static void sdhci_msm_set_clock(struct sdhci_host *host, unsigned int clock)
 {
 	struct sdhci_pltfm_host *pltfm_host = sdhci_priv(host);
 	struct sdhci_msm_host *msm_host = sdhci_pltfm_priv(pltfm_host);
+	struct mmc_ios curr_ios = host->mmc->ios;
 	u32 msm_clock;
 	int rc;
 
@@ -676,15 +677,29 @@ static void sdhci_msm_set_clock(struct sdhci_host *host, unsigned int clock)
 		goto out;
 
 	spin_unlock_irq(&host->lock);
-	if ((clock != msm_host->clk_rate) && msm_host->clk_table) {
-		msm_clock = sdhci_msm_get_msm_clk_rate(host, clock);
+	if (((curr_ios.timing == MMC_TIMING_UHS_DDR50) ||
+		(curr_ios.timing == MMC_TIMING_MMC_DDR52) ||
+		(curr_ios.timing == MMC_TIMING_MMC_HS400)) &&
+		msm_host->clk_table) {
+		/*
+		 * The SDHC requires internal clock frequency to be double the
+		 * actual clock that will be set for DDR mode. The controller
+		 * uses the faster clock(100/400MHz) for some of its parts and
+		 * send the actual required clock (50/200MHz) to the card.
+		 */
+		clock *= 2;
+	}
+	msm_clock = sdhci_msm_get_msm_clk_rate(host, clock);
+
+	if ((msm_clock != msm_host->clk_rate) && msm_host->clk_table) {
 		rc = clk_set_rate(msm_host->clk, msm_clock);
 		if (rc) {
-			pr_err("%s: failed to set clock at rate %u, requested clock rate %u\n",
-				mmc_hostname(host->mmc), msm_clock, clock);
+			pr_err("%s: failed to set clock at rate %u at timing %d\n",
+				mmc_hostname(host->mmc), msm_clock,
+				curr_ios.timing);
 			goto out;
 		}
-		msm_host->clk_rate = clock;
+		msm_host->clk_rate = msm_clock;
 		pr_debug("%s: setting clock at rate %lu\n",
 			mmc_hostname(host->mmc), clk_get_rate(msm_host->clk));
 	}
-- 
The Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum, 
a Linux Foundation Collaborative Project.

Re: [PATCH v5 08/12] mmc: sdhci-msm: Add clock changes for DDR mode.

[Adrian Hunter]

On 05/10/16 17:40, Ritesh Harjani wrote:
> SDHC MSM controller need 2x clock for MCLK at GCC.
> Hence make required changes to have 2x clock for
> DDR timing modes.
> 
> Signed-off-by: Ritesh Harjani <riteshh-sgV2jX0FEOL9JmXXK+q4OQ@public.gmane.org>

Acked-by: Adrian Hunter <adrian.hunter-ral2JQCrhuEAvxtiuMwx3w@public.gmane.org>

> ---
>  drivers/mmc/host/sdhci-msm.c | 25 ++++++++++++++++++++-----
>  1 file changed, 20 insertions(+), 5 deletions(-)
> 
> diff --git a/drivers/mmc/host/sdhci-msm.c b/drivers/mmc/host/sdhci-msm.c
> index 9d18cf0..eb1a9e3 100644
> --- a/drivers/mmc/host/sdhci-msm.c
> +++ b/drivers/mmc/host/sdhci-msm.c
> @@ -669,6 +669,7 @@ static void sdhci_msm_set_clock(struct sdhci_host *host, unsigned int clock)
>  {
>  	struct sdhci_pltfm_host *pltfm_host = sdhci_priv(host);
>  	struct sdhci_msm_host *msm_host = sdhci_pltfm_priv(pltfm_host);
> +	struct mmc_ios curr_ios = host->mmc->ios;
>  	u32 msm_clock;
>  	int rc;
>  
> @@ -676,15 +677,29 @@ static void sdhci_msm_set_clock(struct sdhci_host *host, unsigned int clock)
>  		goto out;
>  
>  	spin_unlock_irq(&host->lock);
> -	if ((clock != msm_host->clk_rate) && msm_host->clk_table) {
> -		msm_clock = sdhci_msm_get_msm_clk_rate(host, clock);
> +	if (((curr_ios.timing == MMC_TIMING_UHS_DDR50) ||
> +		(curr_ios.timing == MMC_TIMING_MMC_DDR52) ||
> +		(curr_ios.timing == MMC_TIMING_MMC_HS400)) &&
> +		msm_host->clk_table) {
> +		/*
> +		 * The SDHC requires internal clock frequency to be double the
> +		 * actual clock that will be set for DDR mode. The controller
> +		 * uses the faster clock(100/400MHz) for some of its parts and
> +		 * send the actual required clock (50/200MHz) to the card.
> +		 */
> +		clock *= 2;
> +	}
> +	msm_clock = sdhci_msm_get_msm_clk_rate(host, clock);
> +
> +	if ((msm_clock != msm_host->clk_rate) && msm_host->clk_table) {
>  		rc = clk_set_rate(msm_host->clk, msm_clock);
>  		if (rc) {
> -			pr_err("%s: failed to set clock at rate %u, requested clock rate %u\n",
> -				mmc_hostname(host->mmc), msm_clock, clock);
> +			pr_err("%s: failed to set clock at rate %u at timing %d\n",
> +				mmc_hostname(host->mmc), msm_clock,
> +				curr_ios.timing);
>  			goto out;
>  		}
> -		msm_host->clk_rate = clock;
> +		msm_host->clk_rate = msm_clock;
>  		pr_debug("%s: setting clock at rate %lu\n",
>  			mmc_hostname(host->mmc), clk_get_rate(msm_host->clk));
>  	}
> 

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

[PATCH v5 09/12] arm64: dts: qcom: msm8916: Add ddr support to sdhc1

[Ritesh Harjani]

This adds mmc-ddr-1_8v support to DT for sdhc1 of msm8916.

Signed-off-by: Ritesh Harjani <riteshh@codeaurora.org>
---
 arch/arm64/boot/dts/qcom/msm8916.dtsi | 1 +
 1 file changed, 1 insertion(+)

diff --git a/arch/arm64/boot/dts/qcom/msm8916.dtsi b/arch/arm64/boot/dts/qcom/msm8916.dtsi
index 2f02058..ad32265 100644
--- a/arch/arm64/boot/dts/qcom/msm8916.dtsi
+++ b/arch/arm64/boot/dts/qcom/msm8916.dtsi
@@ -514,6 +514,7 @@
 			clock-names = "core", "iface";
 			clk-rates = <400000 25000000 50000000 100000000
 					177770000>;
+			mmc-ddr-1_8v;
 			bus-width = <8>;
 			non-removable;
 			status = "disabled";
-- 
The Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum, 
a Linux Foundation Collaborative Project.

[PATCH v5 12/12] sdhci: sdhci-msm: update dll configuration

[Ritesh Harjani]

The newer msm sdhci's cores use a different DLL hardware for HS400.
Update the configuration and calibration of the newer DLL block.

The HS400 DLL block used previously is CDC LP 533 and requires
programming multiple registers and waiting for configuration to
complete and then enable it. It has about 18 register writes and
two register reads.

The newer HS400 DLL block is SDC4 DLL and requires two register
writes for configuration and one register read to confirm that it
is initialized. There is an additional register write to enable
the power save mode for SDC4 DLL block.

Signed-off-by: Ritesh Harjani <riteshh@codeaurora.org>
Signed-off-by: Krishna Konda <kkonda@codeaurora.org>
---
 drivers/mmc/host/sdhci-msm.c | 141 ++++++++++++++++++++++++++++++++++++++-----
 1 file changed, 127 insertions(+), 14 deletions(-)

diff --git a/drivers/mmc/host/sdhci-msm.c b/drivers/mmc/host/sdhci-msm.c
index dbf80a9c..ddc8dc9 100644
--- a/drivers/mmc/host/sdhci-msm.c
+++ b/drivers/mmc/host/sdhci-msm.c
@@ -19,6 +19,7 @@
 #include <linux/delay.h>
 #include <linux/mmc/mmc.h>
 #include <linux/slab.h>
+#include <linux/iopoll.h>
 
 #include "sdhci-pltfm.h"
 
@@ -50,6 +51,7 @@
 #define INT_MASK		0xf
 #define MAX_PHASES		16
 #define CORE_DLL_LOCK		BIT(7)
+#define CORE_DDR_DLL_LOCK	BIT(11)
 #define CORE_DLL_EN		BIT(16)
 #define CORE_CDR_EN		BIT(17)
 #define CORE_CK_OUT_EN		BIT(18)
@@ -61,6 +63,7 @@
 #define CORE_DLL_STATUS		0x108
 
 #define CORE_DLL_CONFIG_2	0x1b4
+#define CORE_DDR_CAL_EN		BIT(0)
 #define CORE_FLL_CYCLE_CNT	BIT(18)
 #define CORE_DLL_CLOCK_DISABLE	BIT(21)
 
@@ -99,6 +102,11 @@
 #define CORE_DDR_200_CFG		0x184
 #define CORE_CDC_T4_DLY_SEL		BIT(0)
 #define CORE_START_CDC_TRAFFIC		BIT(6)
+#define CORE_VENDOR_SPEC3	0x1b0
+#define CORE_PWRSAVE_DLL	BIT(3)
+
+#define CORE_DDR_CONFIG		0x1b8
+#define DDR_CONFIG_POR_VAL	0x80040853
 
 #define CORE_VENDOR_SPEC_CAPABILITIES0	0x11c
 
@@ -127,6 +135,7 @@ struct sdhci_msm_host {
 	bool tuning_done;
 	bool calibration_done;
 	u8 saved_tuning_phase;
+	bool use_cdclp533;
 };
 
 /* Platform specific tuning */
@@ -460,7 +469,7 @@ static int sdhci_msm_cdclp533_calibration(struct sdhci_host *host)
 {
 	struct sdhci_pltfm_host *pltfm_host = sdhci_priv(host);
 	struct sdhci_msm_host *msm_host = sdhci_pltfm_priv(pltfm_host);
-	u32 wait_cnt, config;
+	u32 config, calib_done;
 	int ret;
 
 	pr_debug("%s: Enter %s\n", mmc_hostname(host->mmc), __func__);
@@ -552,18 +561,13 @@ static int sdhci_msm_cdclp533_calibration(struct sdhci_host *host)
 	wmb(); /* drain writebuffer */
 
 	/* Poll on CALIBRATION_DONE field in CORE_CSR_CDC_STATUS0 to be 1 */
-	wait_cnt = 50;
-	while (!(readl_relaxed(host->ioaddr + CORE_CSR_CDC_STATUS0)
-			& CORE_CALIBRATION_DONE)) {
-		/* max. wait for 50us sec for CALIBRATION_DONE bit to be set */
-		if (--wait_cnt == 0) {
-			pr_err("%s: %s: CDC Calibration was not completed\n",
+	ret = readl_poll_timeout(host->ioaddr + CORE_CSR_CDC_STATUS0,
+		 calib_done, (calib_done & CORE_CALIBRATION_DONE), 1, 50);
+
+	if (ret == -ETIMEDOUT) {
+		pr_err("%s: %s: CDC Calibration was not completed\n",
 				mmc_hostname(host->mmc), __func__);
-			ret = -ETIMEDOUT;
-			goto out;
-		}
-		/* wait for 1us before polling again */
-		udelay(1);
+		goto out;
 	}
 
 	/* Verify CDC_ERROR_CODE field in CORE_CSR_CDC_STATUS0 is 0 */
@@ -586,6 +590,86 @@ out:
 	return ret;
 }
 
+static int sdhci_msm_cm_dll_sdc4_calibration(struct sdhci_host *host)
+{
+	u32 dll_status, config;
+	int ret;
+
+	pr_debug("%s: Enter %s\n", mmc_hostname(host->mmc), __func__);
+
+	/*
+	 * Currently the CORE_DDR_CONFIG register defaults to desired
+	 * configuration on reset. Currently reprogramming the power on
+	 * reset (POR) value in case it might have been modified by
+	 * bootloaders. In the future, if this changes, then the desired
+	 * values will need to be programmed appropriately.
+	 */
+	writel_relaxed(DDR_CONFIG_POR_VAL, host->ioaddr + CORE_DDR_CONFIG);
+
+	/* Write 1 to DDR_CAL_EN field in CORE_DLL_CONFIG_2 */
+	config = readl_relaxed(host->ioaddr + CORE_DLL_CONFIG_2);
+	config |= CORE_DDR_CAL_EN;
+	writel_relaxed(config, host->ioaddr + CORE_DLL_CONFIG_2);
+
+	/* Poll on DDR_DLL_LOCK bit in CORE_DLL_STATUS to be set */
+	ret = readl_poll_timeout(host->ioaddr + CORE_DLL_STATUS,
+		 dll_status, (dll_status & CORE_DDR_DLL_LOCK), 10, 1000);
+
+	if (ret == -ETIMEDOUT) {
+		pr_err("%s: %s: CM_DLL_SDC4 Calibration was not completed\n",
+				mmc_hostname(host->mmc), __func__);
+		goto out;
+	}
+
+	/* set CORE_PWRSAVE_DLL bit in CORE_VENDOR_SPEC3 */
+	config = readl_relaxed(host->ioaddr + CORE_VENDOR_SPEC3);
+	config |= CORE_PWRSAVE_DLL;
+	writel_relaxed(config, host->ioaddr + CORE_VENDOR_SPEC3);
+	wmb(); /* drain writebuffer */
+out:
+	pr_debug("%s: Exit %s, ret:%d\n", mmc_hostname(host->mmc),
+			__func__, ret);
+	return ret;
+}
+
+static int sdhci_msm_hs400_dll_calibration(struct sdhci_host *host)
+{
+	struct sdhci_pltfm_host *pltfm_host = sdhci_priv(host);
+	struct sdhci_msm_host *msm_host = sdhci_pltfm_priv(pltfm_host);
+	int ret;
+	u32 config;
+
+	pr_debug("%s: Enter %s\n", mmc_hostname(host->mmc), __func__);
+
+	/*
+	 * Retuning in HS400 (DDR mode) will fail, just reset the
+	 * tuning block and restore the saved tuning phase.
+	 */
+	ret = msm_init_cm_dll(host);
+	if (ret)
+		goto out;
+
+	/* Set the selected phase in delay line hw block */
+	ret = msm_config_cm_dll_phase(host, msm_host->saved_tuning_phase);
+	if (ret)
+		goto out;
+
+	/* Write 1 to CMD_DAT_TRACK_SEL field in DLL_CONFIG */
+	config = readl_relaxed(host->ioaddr + CORE_DLL_CONFIG);
+	config |= CORE_CMD_DAT_TRACK_SEL;
+	writel_relaxed(config, host->ioaddr + CORE_DLL_CONFIG);
+	if (msm_host->use_cdclp533)
+		/* Calibrate CDCLP533 DLL HW */
+		ret = sdhci_msm_cdclp533_calibration(host);
+	else
+		/* Calibrate CM_DLL_SDC4 HW */
+		ret = sdhci_msm_cm_dll_sdc4_calibration(host);
+out:
+	pr_debug("%s: Exit %s, ret:%d\n", mmc_hostname(host->mmc),
+			__func__, ret);
+	return ret;
+}
+
 static int sdhci_msm_execute_tuning(struct sdhci_host *host, u32 opcode)
 {
 	int tuning_seq_cnt = 3;
@@ -737,7 +821,7 @@ static void sdhci_msm_set_uhs_signaling(struct sdhci_host *host,
 	if (host->clock > CORE_FREQ_100MHZ &&
 	   msm_host->tuning_done && !msm_host->calibration_done &&
 	   (mmc->ios.timing == MMC_TIMING_MMC_HS400))
-		if (!sdhci_msm_cdclp533_calibration(host))
+		if (!sdhci_msm_hs400_dll_calibration(host))
 			msm_host->calibration_done = true;
 
 	spin_lock_irq(&host->lock);
@@ -883,7 +967,7 @@ static void sdhci_msm_set_clock(struct sdhci_host *host, unsigned int clock)
 	struct sdhci_pltfm_host *pltfm_host = sdhci_priv(host);
 	struct sdhci_msm_host *msm_host = sdhci_pltfm_priv(pltfm_host);
 	struct mmc_ios curr_ios = host->mmc->ios;
-	u32 msm_clock, config;
+	u32 msm_clock, config, dll_lock;
 	int rc;
 
 	if (!clock)
@@ -942,7 +1026,29 @@ static void sdhci_msm_set_clock(struct sdhci_host *host, unsigned int clock)
 			config |= CORE_HC_SELECT_IN_EN;
 			writel_relaxed(config, host->ioaddr + CORE_VENDOR_SPEC);
 		}
+		if (!msm_host->clk_rate && !msm_host->use_cdclp533) {
+			/*
+			 * Poll on DLL_LOCK and DDR_DLL_LOCK bits in
+			 * CORE_DLL_STATUS to be set.  This should get set
+			 * with in 15 us at 200 MHz.
+			 */
+			rc = readl_poll_timeout(host->ioaddr + CORE_DLL_STATUS,
+					dll_lock, (dll_lock & (CORE_DLL_LOCK |
+					CORE_DDR_DLL_LOCK)), 10, 1000);
+			if (rc == -ETIMEDOUT)
+				pr_err("%s: Unable to get DLL_LOCK/DDR_DLL_LOCK, dll_status: 0x%08x\n",
+					mmc_hostname(host->mmc), dll_lock);
+		}
 	} else {
+		if (!msm_host->use_cdclp533) {
+			/* set CORE_PWRSAVE_DLL bit in CORE_VENDOR_SPEC3 */
+			config = readl_relaxed(host->ioaddr +
+					CORE_VENDOR_SPEC3);
+			config &= ~CORE_PWRSAVE_DLL;
+			writel_relaxed(config, host->ioaddr +
+					CORE_VENDOR_SPEC3);
+		}
+
 		/* Select the default clock (free running MCLK) */
 		config = readl_relaxed(host->ioaddr + CORE_VENDOR_SPEC);
 		config &= ~CORE_HC_MCLK_SEL_MASK;
@@ -1172,6 +1278,13 @@ static int sdhci_msm_probe(struct platform_device *pdev)
 		msm_host->use_14lpp_dll_reset = true;
 
 	/*
+	 * SDCC 5 controller with major version 1, minor version 0x34 and later
+	 * with HS 400 mode support will use CM DLL instead of CDC LP 533 DLL.
+	 */
+	if ((core_major == 1) && (core_minor < 0x34))
+		msm_host->use_cdclp533 = true;
+
+	/*
 	 * Support for some capabilities is not advertised by newer
 	 * controller versions and must be explicitly enabled.
 	 */
-- 
The Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum, 
a Linux Foundation Collaborative Project.

Re: [PATCH v5 12/12] sdhci: sdhci-msm: update dll configuration

[Adrian Hunter]

On 05/10/16 17:40, Ritesh Harjani wrote:
> The newer msm sdhci's cores use a different DLL hardware for HS400.
> Update the configuration and calibration of the newer DLL block.
> 
> The HS400 DLL block used previously is CDC LP 533 and requires
> programming multiple registers and waiting for configuration to
> complete and then enable it. It has about 18 register writes and
> two register reads.
> 
> The newer HS400 DLL block is SDC4 DLL and requires two register
> writes for configuration and one register read to confirm that it
> is initialized. There is an additional register write to enable
> the power save mode for SDC4 DLL block.
> 
> Signed-off-by: Ritesh Harjani <riteshh@codeaurora.org>
> Signed-off-by: Krishna Konda <kkonda@codeaurora.org>
> ---
>  drivers/mmc/host/sdhci-msm.c | 141 ++++++++++++++++++++++++++++++++++++++-----
>  1 file changed, 127 insertions(+), 14 deletions(-)
> 
> diff --git a/drivers/mmc/host/sdhci-msm.c b/drivers/mmc/host/sdhci-msm.c
> index dbf80a9c..ddc8dc9 100644
> --- a/drivers/mmc/host/sdhci-msm.c
> +++ b/drivers/mmc/host/sdhci-msm.c
> @@ -19,6 +19,7 @@
>  #include <linux/delay.h>
>  #include <linux/mmc/mmc.h>
>  #include <linux/slab.h>
> +#include <linux/iopoll.h>
>  
>  #include "sdhci-pltfm.h"
>  
> @@ -50,6 +51,7 @@
>  #define INT_MASK		0xf
>  #define MAX_PHASES		16
>  #define CORE_DLL_LOCK		BIT(7)
> +#define CORE_DDR_DLL_LOCK	BIT(11)
>  #define CORE_DLL_EN		BIT(16)
>  #define CORE_CDR_EN		BIT(17)
>  #define CORE_CK_OUT_EN		BIT(18)
> @@ -61,6 +63,7 @@
>  #define CORE_DLL_STATUS		0x108
>  
>  #define CORE_DLL_CONFIG_2	0x1b4
> +#define CORE_DDR_CAL_EN		BIT(0)
>  #define CORE_FLL_CYCLE_CNT	BIT(18)
>  #define CORE_DLL_CLOCK_DISABLE	BIT(21)
>  
> @@ -99,6 +102,11 @@
>  #define CORE_DDR_200_CFG		0x184
>  #define CORE_CDC_T4_DLY_SEL		BIT(0)
>  #define CORE_START_CDC_TRAFFIC		BIT(6)
> +#define CORE_VENDOR_SPEC3	0x1b0
> +#define CORE_PWRSAVE_DLL	BIT(3)
> +
> +#define CORE_DDR_CONFIG		0x1b8
> +#define DDR_CONFIG_POR_VAL	0x80040853
>  
>  #define CORE_VENDOR_SPEC_CAPABILITIES0	0x11c
>  
> @@ -127,6 +135,7 @@ struct sdhci_msm_host {
>  	bool tuning_done;
>  	bool calibration_done;
>  	u8 saved_tuning_phase;
> +	bool use_cdclp533;
>  };
>  
>  /* Platform specific tuning */
> @@ -460,7 +469,7 @@ static int sdhci_msm_cdclp533_calibration(struct sdhci_host *host)
>  {
>  	struct sdhci_pltfm_host *pltfm_host = sdhci_priv(host);
>  	struct sdhci_msm_host *msm_host = sdhci_pltfm_priv(pltfm_host);
> -	u32 wait_cnt, config;
> +	u32 config, calib_done;
>  	int ret;
>  
>  	pr_debug("%s: Enter %s\n", mmc_hostname(host->mmc), __func__);
> @@ -552,18 +561,13 @@ static int sdhci_msm_cdclp533_calibration(struct sdhci_host *host)
>  	wmb(); /* drain writebuffer */
>  
>  	/* Poll on CALIBRATION_DONE field in CORE_CSR_CDC_STATUS0 to be 1 */
> -	wait_cnt = 50;
> -	while (!(readl_relaxed(host->ioaddr + CORE_CSR_CDC_STATUS0)
> -			& CORE_CALIBRATION_DONE)) {
> -		/* max. wait for 50us sec for CALIBRATION_DONE bit to be set */
> -		if (--wait_cnt == 0) {
> -			pr_err("%s: %s: CDC Calibration was not completed\n",
> +	ret = readl_poll_timeout(host->ioaddr + CORE_CSR_CDC_STATUS0,

This code was added in a previous patch, so it would make more sense to make
it use readl_poll_timeout in the first place.  Was there a reason to use
readl_poll_timeout instead of readl_relaxed_poll_timeout()?

> +		 calib_done, (calib_done & CORE_CALIBRATION_DONE), 1, 50);
> +
> +	if (ret == -ETIMEDOUT) {
> +		pr_err("%s: %s: CDC Calibration was not completed\n",
>  				mmc_hostname(host->mmc), __func__);
> -			ret = -ETIMEDOUT;
> -			goto out;
> -		}
> -		/* wait for 1us before polling again */
> -		udelay(1);
> +		goto out;
>  	}
>  
>  	/* Verify CDC_ERROR_CODE field in CORE_CSR_CDC_STATUS0 is 0 */
> @@ -586,6 +590,86 @@ out:
>  	return ret;
>  }
>  
> +static int sdhci_msm_cm_dll_sdc4_calibration(struct sdhci_host *host)
> +{
> +	u32 dll_status, config;
> +	int ret;
> +
> +	pr_debug("%s: Enter %s\n", mmc_hostname(host->mmc), __func__);
> +
> +	/*
> +	 * Currently the CORE_DDR_CONFIG register defaults to desired
> +	 * configuration on reset. Currently reprogramming the power on
> +	 * reset (POR) value in case it might have been modified by
> +	 * bootloaders. In the future, if this changes, then the desired
> +	 * values will need to be programmed appropriately.
> +	 */
> +	writel_relaxed(DDR_CONFIG_POR_VAL, host->ioaddr + CORE_DDR_CONFIG);
> +
> +	/* Write 1 to DDR_CAL_EN field in CORE_DLL_CONFIG_2 */
> +	config = readl_relaxed(host->ioaddr + CORE_DLL_CONFIG_2);
> +	config |= CORE_DDR_CAL_EN;
> +	writel_relaxed(config, host->ioaddr + CORE_DLL_CONFIG_2);
> +
> +	/* Poll on DDR_DLL_LOCK bit in CORE_DLL_STATUS to be set */
> +	ret = readl_poll_timeout(host->ioaddr + CORE_DLL_STATUS,

Was there a reason to use readl_poll_timeout instead of
readl_relaxed_poll_timeout()?

> +		 dll_status, (dll_status & CORE_DDR_DLL_LOCK), 10, 1000);
> +
> +	if (ret == -ETIMEDOUT) {
> +		pr_err("%s: %s: CM_DLL_SDC4 Calibration was not completed\n",
> +				mmc_hostname(host->mmc), __func__);
> +		goto out;
> +	}
> +
> +	/* set CORE_PWRSAVE_DLL bit in CORE_VENDOR_SPEC3 */
> +	config = readl_relaxed(host->ioaddr + CORE_VENDOR_SPEC3);
> +	config |= CORE_PWRSAVE_DLL;
> +	writel_relaxed(config, host->ioaddr + CORE_VENDOR_SPEC3);
> +	wmb(); /* drain writebuffer */
> +out:
> +	pr_debug("%s: Exit %s, ret:%d\n", mmc_hostname(host->mmc),
> +			__func__, ret);
> +	return ret;
> +}
> +
> +static int sdhci_msm_hs400_dll_calibration(struct sdhci_host *host)
> +{
> +	struct sdhci_pltfm_host *pltfm_host = sdhci_priv(host);
> +	struct sdhci_msm_host *msm_host = sdhci_pltfm_priv(pltfm_host);
> +	int ret;
> +	u32 config;
> +
> +	pr_debug("%s: Enter %s\n", mmc_hostname(host->mmc), __func__);
> +
> +	/*
> +	 * Retuning in HS400 (DDR mode) will fail, just reset the
> +	 * tuning block and restore the saved tuning phase.
> +	 */
> +	ret = msm_init_cm_dll(host);
> +	if (ret)
> +		goto out;
> +
> +	/* Set the selected phase in delay line hw block */
> +	ret = msm_config_cm_dll_phase(host, msm_host->saved_tuning_phase);
> +	if (ret)
> +		goto out;
> +
> +	/* Write 1 to CMD_DAT_TRACK_SEL field in DLL_CONFIG */
> +	config = readl_relaxed(host->ioaddr + CORE_DLL_CONFIG);
> +	config |= CORE_CMD_DAT_TRACK_SEL;
> +	writel_relaxed(config, host->ioaddr + CORE_DLL_CONFIG);
> +	if (msm_host->use_cdclp533)
> +		/* Calibrate CDCLP533 DLL HW */
> +		ret = sdhci_msm_cdclp533_calibration(host);

sdhci_msm_cdclp533_calibration() does some of the steps above all over
again.  Is that intended?


> +	else
> +		/* Calibrate CM_DLL_SDC4 HW */
> +		ret = sdhci_msm_cm_dll_sdc4_calibration(host);
> +out:
> +	pr_debug("%s: Exit %s, ret:%d\n", mmc_hostname(host->mmc),
> +			__func__, ret);
> +	return ret;
> +}
> +
>  static int sdhci_msm_execute_tuning(struct sdhci_host *host, u32 opcode)
>  {
>  	int tuning_seq_cnt = 3;
> @@ -737,7 +821,7 @@ static void sdhci_msm_set_uhs_signaling(struct sdhci_host *host,
>  	if (host->clock > CORE_FREQ_100MHZ &&
>  	   msm_host->tuning_done && !msm_host->calibration_done &&
>  	   (mmc->ios.timing == MMC_TIMING_MMC_HS400))
> -		if (!sdhci_msm_cdclp533_calibration(host))
> +		if (!sdhci_msm_hs400_dll_calibration(host))
>  			msm_host->calibration_done = true;
>  
>  	spin_lock_irq(&host->lock);
> @@ -883,7 +967,7 @@ static void sdhci_msm_set_clock(struct sdhci_host *host, unsigned int clock)
>  	struct sdhci_pltfm_host *pltfm_host = sdhci_priv(host);
>  	struct sdhci_msm_host *msm_host = sdhci_pltfm_priv(pltfm_host);
>  	struct mmc_ios curr_ios = host->mmc->ios;
> -	u32 msm_clock, config;
> +	u32 msm_clock, config, dll_lock;
>  	int rc;
>  
>  	if (!clock)
> @@ -942,7 +1026,29 @@ static void sdhci_msm_set_clock(struct sdhci_host *host, unsigned int clock)
>  			config |= CORE_HC_SELECT_IN_EN;
>  			writel_relaxed(config, host->ioaddr + CORE_VENDOR_SPEC);
>  		}
> +		if (!msm_host->clk_rate && !msm_host->use_cdclp533) {
> +			/*
> +			 * Poll on DLL_LOCK and DDR_DLL_LOCK bits in
> +			 * CORE_DLL_STATUS to be set.  This should get set
> +			 * with in 15 us at 200 MHz.

'with in' -> 'within'

> +			 */
> +			rc = readl_poll_timeout(host->ioaddr + CORE_DLL_STATUS,

Was there a reason to use readl_poll_timeout instead of
readl_relaxed_poll_timeout()?

> +					dll_lock, (dll_lock & (CORE_DLL_LOCK |
> +					CORE_DDR_DLL_LOCK)), 10, 1000);

The comment says 'DLL_LOCK and DDR_DLL_LOCK' but the logic looks 'DLL_LOCK
or DDR_DLL_LOCK'

> +			if (rc == -ETIMEDOUT)
> +				pr_err("%s: Unable to get DLL_LOCK/DDR_DLL_LOCK, dll_status: 0x%08x\n",
> +					mmc_hostname(host->mmc), dll_lock);
> +		}
>  	} else {
> +		if (!msm_host->use_cdclp533) {
> +			/* set CORE_PWRSAVE_DLL bit in CORE_VENDOR_SPEC3 */
> +			config = readl_relaxed(host->ioaddr +
> +					CORE_VENDOR_SPEC3);
> +			config &= ~CORE_PWRSAVE_DLL;
> +			writel_relaxed(config, host->ioaddr +
> +					CORE_VENDOR_SPEC3);
> +		}
> +
>  		/* Select the default clock (free running MCLK) */
>  		config = readl_relaxed(host->ioaddr + CORE_VENDOR_SPEC);
>  		config &= ~CORE_HC_MCLK_SEL_MASK;
> @@ -1172,6 +1278,13 @@ static int sdhci_msm_probe(struct platform_device *pdev)
>  		msm_host->use_14lpp_dll_reset = true;
>  
>  	/*
> +	 * SDCC 5 controller with major version 1, minor version 0x34 and later
> +	 * with HS 400 mode support will use CM DLL instead of CDC LP 533 DLL.
> +	 */
> +	if ((core_major == 1) && (core_minor < 0x34))
> +		msm_host->use_cdclp533 = true;
> +
> +	/*
>  	 * Support for some capabilities is not advertised by newer
>  	 * controller versions and must be explicitly enabled.
>  	 */
> 

Re: [PATCH v5 12/12] sdhci: sdhci-msm: update dll configuration

[Ritesh Harjani]

Hi Adrian,


On 10/10/2016 6:57 PM, Adrian Hunter wrote:
> On 05/10/16 17:40, Ritesh Harjani wrote:
>> The newer msm sdhci's cores use a different DLL hardware for HS400.
>> Update the configuration and calibration of the newer DLL block.
>>
>> The HS400 DLL block used previously is CDC LP 533 and requires
>> programming multiple registers and waiting for configuration to
>> complete and then enable it. It has about 18 register writes and
>> two register reads.
>>
>> The newer HS400 DLL block is SDC4 DLL and requires two register
>> writes for configuration and one register read to confirm that it
>> is initialized. There is an additional register write to enable
>> the power save mode for SDC4 DLL block.
>>
>> Signed-off-by: Ritesh Harjani <riteshh@codeaurora.org>
>> Signed-off-by: Krishna Konda <kkonda@codeaurora.org>
>> ---
>>  drivers/mmc/host/sdhci-msm.c | 141 ++++++++++++++++++++++++++++++++++++++-----
>>  1 file changed, 127 insertions(+), 14 deletions(-)
>>
>> diff --git a/drivers/mmc/host/sdhci-msm.c b/drivers/mmc/host/sdhci-msm.c
>> index dbf80a9c..ddc8dc9 100644
>> --- a/drivers/mmc/host/sdhci-msm.c
>> +++ b/drivers/mmc/host/sdhci-msm.c
>> @@ -19,6 +19,7 @@
>>  #include <linux/delay.h>
>>  #include <linux/mmc/mmc.h>
>>  #include <linux/slab.h>
>> +#include <linux/iopoll.h>
>>
>>  #include "sdhci-pltfm.h"
>>
>> @@ -50,6 +51,7 @@
>>  #define INT_MASK		0xf
>>  #define MAX_PHASES		16
>>  #define CORE_DLL_LOCK		BIT(7)
>> +#define CORE_DDR_DLL_LOCK	BIT(11)
>>  #define CORE_DLL_EN		BIT(16)
>>  #define CORE_CDR_EN		BIT(17)
>>  #define CORE_CK_OUT_EN		BIT(18)
>> @@ -61,6 +63,7 @@
>>  #define CORE_DLL_STATUS		0x108
>>
>>  #define CORE_DLL_CONFIG_2	0x1b4
>> +#define CORE_DDR_CAL_EN		BIT(0)
>>  #define CORE_FLL_CYCLE_CNT	BIT(18)
>>  #define CORE_DLL_CLOCK_DISABLE	BIT(21)
>>
>> @@ -99,6 +102,11 @@
>>  #define CORE_DDR_200_CFG		0x184
>>  #define CORE_CDC_T4_DLY_SEL		BIT(0)
>>  #define CORE_START_CDC_TRAFFIC		BIT(6)
>> +#define CORE_VENDOR_SPEC3	0x1b0
>> +#define CORE_PWRSAVE_DLL	BIT(3)
>> +
>> +#define CORE_DDR_CONFIG		0x1b8
>> +#define DDR_CONFIG_POR_VAL	0x80040853
>>
>>  #define CORE_VENDOR_SPEC_CAPABILITIES0	0x11c
>>
>> @@ -127,6 +135,7 @@ struct sdhci_msm_host {
>>  	bool tuning_done;
>>  	bool calibration_done;
>>  	u8 saved_tuning_phase;
>> +	bool use_cdclp533;
>>  };
>>
>>  /* Platform specific tuning */
>> @@ -460,7 +469,7 @@ static int sdhci_msm_cdclp533_calibration(struct sdhci_host *host)
>>  {
>>  	struct sdhci_pltfm_host *pltfm_host = sdhci_priv(host);
>>  	struct sdhci_msm_host *msm_host = sdhci_pltfm_priv(pltfm_host);
>> -	u32 wait_cnt, config;
>> +	u32 config, calib_done;
>>  	int ret;
>>
>>  	pr_debug("%s: Enter %s\n", mmc_hostname(host->mmc), __func__);
>> @@ -552,18 +561,13 @@ static int sdhci_msm_cdclp533_calibration(struct sdhci_host *host)
>>  	wmb(); /* drain writebuffer */
>>
>>  	/* Poll on CALIBRATION_DONE field in CORE_CSR_CDC_STATUS0 to be 1 */
>> -	wait_cnt = 50;
>> -	while (!(readl_relaxed(host->ioaddr + CORE_CSR_CDC_STATUS0)
>> -			& CORE_CALIBRATION_DONE)) {
>> -		/* max. wait for 50us sec for CALIBRATION_DONE bit to be set */
>> -		if (--wait_cnt == 0) {
>> -			pr_err("%s: %s: CDC Calibration was not completed\n",
>> +	ret = readl_poll_timeout(host->ioaddr + CORE_CSR_CDC_STATUS0,
>
> This code was added in a previous patch, so it would make more sense to make
> it use readl_poll_timeout in the first place.  Was there a reason to use
> readl_poll_timeout instead of readl_relaxed_poll_timeout()?
Sure will make the change in previous patch.
Thanks for pointing out about readl_relaxed_poll_timeout.
I think I only missed it, will check once again.

>
>> +		 calib_done, (calib_done & CORE_CALIBRATION_DONE), 1, 50);
>> +
>> +	if (ret == -ETIMEDOUT) {
>> +		pr_err("%s: %s: CDC Calibration was not completed\n",
>>  				mmc_hostname(host->mmc), __func__);
>> -			ret = -ETIMEDOUT;
>> -			goto out;
>> -		}
>> -		/* wait for 1us before polling again */
>> -		udelay(1);
>> +		goto out;
>>  	}
>>
>>  	/* Verify CDC_ERROR_CODE field in CORE_CSR_CDC_STATUS0 is 0 */
>> @@ -586,6 +590,86 @@ out:
>>  	return ret;
>>  }
>>
>> +static int sdhci_msm_cm_dll_sdc4_calibration(struct sdhci_host *host)
>> +{
>> +	u32 dll_status, config;
>> +	int ret;
>> +
>> +	pr_debug("%s: Enter %s\n", mmc_hostname(host->mmc), __func__);
>> +
>> +	/*
>> +	 * Currently the CORE_DDR_CONFIG register defaults to desired
>> +	 * configuration on reset. Currently reprogramming the power on
>> +	 * reset (POR) value in case it might have been modified by
>> +	 * bootloaders. In the future, if this changes, then the desired
>> +	 * values will need to be programmed appropriately.
>> +	 */
>> +	writel_relaxed(DDR_CONFIG_POR_VAL, host->ioaddr + CORE_DDR_CONFIG);
>> +
>> +	/* Write 1 to DDR_CAL_EN field in CORE_DLL_CONFIG_2 */
>> +	config = readl_relaxed(host->ioaddr + CORE_DLL_CONFIG_2);
>> +	config |= CORE_DDR_CAL_EN;
>> +	writel_relaxed(config, host->ioaddr + CORE_DLL_CONFIG_2);
>> +
>> +	/* Poll on DDR_DLL_LOCK bit in CORE_DLL_STATUS to be set */
>> +	ret = readl_poll_timeout(host->ioaddr + CORE_DLL_STATUS,
>
> Was there a reason to use readl_poll_timeout instead of
> readl_relaxed_poll_timeout()?
Sure will check it.

>
>> +		 dll_status, (dll_status & CORE_DDR_DLL_LOCK), 10, 1000);
>> +
>> +	if (ret == -ETIMEDOUT) {
>> +		pr_err("%s: %s: CM_DLL_SDC4 Calibration was not completed\n",
>> +				mmc_hostname(host->mmc), __func__);
>> +		goto out;
>> +	}
>> +
>> +	/* set CORE_PWRSAVE_DLL bit in CORE_VENDOR_SPEC3 */
>> +	config = readl_relaxed(host->ioaddr + CORE_VENDOR_SPEC3);
>> +	config |= CORE_PWRSAVE_DLL;
>> +	writel_relaxed(config, host->ioaddr + CORE_VENDOR_SPEC3);
>> +	wmb(); /* drain writebuffer */
>> +out:
>> +	pr_debug("%s: Exit %s, ret:%d\n", mmc_hostname(host->mmc),
>> +			__func__, ret);
>> +	return ret;
>> +}
>> +
>> +static int sdhci_msm_hs400_dll_calibration(struct sdhci_host *host)
>> +{
>> +	struct sdhci_pltfm_host *pltfm_host = sdhci_priv(host);
>> +	struct sdhci_msm_host *msm_host = sdhci_pltfm_priv(pltfm_host);
>> +	int ret;
>> +	u32 config;
>> +
>> +	pr_debug("%s: Enter %s\n", mmc_hostname(host->mmc), __func__);
>> +
>> +	/*
>> +	 * Retuning in HS400 (DDR mode) will fail, just reset the
>> +	 * tuning block and restore the saved tuning phase.
>> +	 */
>> +	ret = msm_init_cm_dll(host);
>> +	if (ret)
>> +		goto out;
>> +
>> +	/* Set the selected phase in delay line hw block */
>> +	ret = msm_config_cm_dll_phase(host, msm_host->saved_tuning_phase);
>> +	if (ret)
>> +		goto out;
>> +
>> +	/* Write 1 to CMD_DAT_TRACK_SEL field in DLL_CONFIG */
>> +	config = readl_relaxed(host->ioaddr + CORE_DLL_CONFIG);
>> +	config |= CORE_CMD_DAT_TRACK_SEL;
>> +	writel_relaxed(config, host->ioaddr + CORE_DLL_CONFIG);
>> +	if (msm_host->use_cdclp533)
>> +		/* Calibrate CDCLP533 DLL HW */
>> +		ret = sdhci_msm_cdclp533_calibration(host);
>
> sdhci_msm_cdclp533_calibration() does some of the steps above all over
> again.  Is that intended?
Yes, this was as per the HW sequence.

>
>
>> +	else
>> +		/* Calibrate CM_DLL_SDC4 HW */
>> +		ret = sdhci_msm_cm_dll_sdc4_calibration(host);
>> +out:
>> +	pr_debug("%s: Exit %s, ret:%d\n", mmc_hostname(host->mmc),
>> +			__func__, ret);
>> +	return ret;
>> +}
>> +
>>  static int sdhci_msm_execute_tuning(struct sdhci_host *host, u32 opcode)
>>  {
>>  	int tuning_seq_cnt = 3;
>> @@ -737,7 +821,7 @@ static void sdhci_msm_set_uhs_signaling(struct sdhci_host *host,
>>  	if (host->clock > CORE_FREQ_100MHZ &&
>>  	   msm_host->tuning_done && !msm_host->calibration_done &&
>>  	   (mmc->ios.timing == MMC_TIMING_MMC_HS400))
>> -		if (!sdhci_msm_cdclp533_calibration(host))
>> +		if (!sdhci_msm_hs400_dll_calibration(host))
>>  			msm_host->calibration_done = true;
>>
>>  	spin_lock_irq(&host->lock);
>> @@ -883,7 +967,7 @@ static void sdhci_msm_set_clock(struct sdhci_host *host, unsigned int clock)
>>  	struct sdhci_pltfm_host *pltfm_host = sdhci_priv(host);
>>  	struct sdhci_msm_host *msm_host = sdhci_pltfm_priv(pltfm_host);
>>  	struct mmc_ios curr_ios = host->mmc->ios;
>> -	u32 msm_clock, config;
>> +	u32 msm_clock, config, dll_lock;
>>  	int rc;
>>
>>  	if (!clock)
>> @@ -942,7 +1026,29 @@ static void sdhci_msm_set_clock(struct sdhci_host *host, unsigned int clock)
>>  			config |= CORE_HC_SELECT_IN_EN;
>>  			writel_relaxed(config, host->ioaddr + CORE_VENDOR_SPEC);
>>  		}
>> +		if (!msm_host->clk_rate && !msm_host->use_cdclp533) {
>> +			/*
>> +			 * Poll on DLL_LOCK and DDR_DLL_LOCK bits in
>> +			 * CORE_DLL_STATUS to be set.  This should get set
>> +			 * with in 15 us at 200 MHz.
>
> 'with in' -> 'within'
Done.

>
>> +			 */
>> +			rc = readl_poll_timeout(host->ioaddr + CORE_DLL_STATUS,
>
> Was there a reason to use readl_poll_timeout instead of
> readl_relaxed_poll_timeout()?
Sure will check it.

>
>> +					dll_lock, (dll_lock & (CORE_DLL_LOCK |
>> +					CORE_DDR_DLL_LOCK)), 10, 1000);
>
> The comment says 'DLL_LOCK and DDR_DLL_LOCK' but the logic looks 'DLL_LOCK
> or DDR_DLL_LOCK'
Ok, I will fix the comment.
will double confirm the HW spec as well.


>
>> +			if (rc == -ETIMEDOUT)
>> +				pr_err("%s: Unable to get DLL_LOCK/DDR_DLL_LOCK, dll_status: 0x%08x\n",
>> +					mmc_hostname(host->mmc), dll_lock);
>> +		}
>>  	} else {
>> +		if (!msm_host->use_cdclp533) {
>> +			/* set CORE_PWRSAVE_DLL bit in CORE_VENDOR_SPEC3 */
>> +			config = readl_relaxed(host->ioaddr +
>> +					CORE_VENDOR_SPEC3);
>> +			config &= ~CORE_PWRSAVE_DLL;
>> +			writel_relaxed(config, host->ioaddr +
>> +					CORE_VENDOR_SPEC3);
>> +		}
>> +
>>  		/* Select the default clock (free running MCLK) */
>>  		config = readl_relaxed(host->ioaddr + CORE_VENDOR_SPEC);
>>  		config &= ~CORE_HC_MCLK_SEL_MASK;
>> @@ -1172,6 +1278,13 @@ static int sdhci_msm_probe(struct platform_device *pdev)
>>  		msm_host->use_14lpp_dll_reset = true;
>>
>>  	/*
>> +	 * SDCC 5 controller with major version 1, minor version 0x34 and later
>> +	 * with HS 400 mode support will use CM DLL instead of CDC LP 533 DLL.
>> +	 */
>> +	if ((core_major == 1) && (core_minor < 0x34))
>> +		msm_host->use_cdclp533 = true;
>> +
>> +	/*
>>  	 * Support for some capabilities is not advertised by newer
>>  	 * controller versions and must be explicitly enabled.
>>  	 */
>>
>