[PATCH v4 0/9] mmc: sdhci-msm: Add clk-rates and DDR support

[PATCH v4 0/9] mmc: sdhci-msm: Add clk-rates and DDR support

[Ritesh Harjani]

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 


Ritesh Harjani (8):
  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

Venkat Gopalakrishnan (1):
  mmc: sdhci-msm: Update DLL reset sequence

 .../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                       | 308 +++++++++++++++++++--
 6 files changed, 300 insertions(+), 24 deletions(-)

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

[PATCH v4 1/9] 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@codeaurora.org>
Acked-by: Adrian Hunter <adrian.hunter@intel.com>
---
 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.

[PATCH v4 2/9] mmc: sdhci-msm: Update DLL reset sequence

[Ritesh Harjani]

From: Venkat Gopalakrishnan <venkatg@codeaurora.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@codeaurora.org>
Signed-off-by: Ritesh Harjani <riteshh@codeaurora.org>
Acked-by: Adrian Hunter <adrian.hunter@intel.com>
---
 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.

[PATCH v4 3/9] 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                       | 60 ++++++++++++++++++++++
 2 files changed, 61 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..a46dd98 100644
--- a/drivers/mmc/host/sdhci-msm.c
+++ b/drivers/mmc/host/sdhci-msm.c
@@ -74,6 +74,11 @@
 #define CMUX_SHIFT_PHASE_SHIFT	24
 #define CMUX_SHIFT_PHASE_MASK	(7 << CMUX_SHIFT_PHASE_SHIFT)
 
+struct sdhci_msm_pltfm_data {
+	u32 *clk_table;
+	int clk_table_sz;
+};
+
 struct sdhci_msm_host {
 	struct platform_device *pdev;
 	void __iomem *core_mem;	/* MSM SDCC mapped address */
@@ -83,6 +88,7 @@ struct sdhci_msm_host {
 	struct clk *bus_clk;	/* SDHC bus voter clock */
 	struct mmc_host *mmc;
 	bool use_14lpp_dll_reset;
+	struct sdhci_msm_pltfm_data *pdata;
 };
 
 /* Platform specific tuning */
@@ -582,6 +588,56 @@ 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_err(dev, "%s: Invalid dt property, err(%d)\n",
+				prop_name, count);
+		return count;
+	}
+
+	arr = devm_kzalloc(dev, count * sizeof(*arr), GFP_KERNEL);
+	if (!arr)
+		return -ENOMEM;
+
+	ret = of_property_read_u32_array(np, prop_name, arr, count);
+	if (ret) {
+		dev_err(dev, "%s Invalid dt array property, err(%d)\n",
+				prop_name, ret);
+		return ret;
+	}
+	*table = arr;
+	*size = count;
+	return 0;
+}
+
+static struct sdhci_msm_pltfm_data *sdhci_msm_populate_pdata(struct device *dev,
+						struct sdhci_msm_host *msm_host)
+{
+	struct sdhci_msm_pltfm_data *pdata = NULL;
+	int table_sz = 0;
+	u32 *table = NULL;
+
+	pdata = devm_kzalloc(dev, sizeof(*pdata), GFP_KERNEL);
+	if (!pdata)
+		return NULL;
+
+	if (sdhci_msm_dt_get_array(dev, "clk-rates", &table, &table_sz)) {
+		dev_err(dev, "failed in DT parsing for supported clk-rates\n");
+		return NULL;
+	}
+	pdata->clk_table = table;
+	pdata->clk_table_sz = table_sz;
+
+	return pdata;
+}
+
 static int sdhci_msm_probe(struct platform_device *pdev)
 {
 	struct sdhci_host *host;
@@ -608,6 +664,10 @@ static int sdhci_msm_probe(struct platform_device *pdev)
 
 	sdhci_get_of_property(pdev);
 
+	msm_host->pdata = sdhci_msm_populate_pdata(&pdev->dev, msm_host);
+	if (!msm_host->pdata)
+		goto pltfm_free;
+
 	/* 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.

[PATCH v4 4/9] 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 7c2df06..dd7a92d 100644
--- a/arch/arm/boot/dts/qcom-apq8084.dtsi
+++ b/arch/arm/boot/dts/qcom-apq8084.dtsi
@@ -315,6 +315,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";
 		};
 
@@ -326,6 +328,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 561d4d1..c5f7ac4 100644
--- a/arch/arm/boot/dts/qcom-msm8974.dtsi
+++ b/arch/arm/boot/dts/qcom-msm8974.dtsi
@@ -447,6 +447,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";
 		};
 
@@ -458,6 +460,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 11bdc24..6f2c0b8 100644
--- a/arch/arm64/boot/dts/qcom/msm8916.dtsi
+++ b/arch/arm64/boot/dts/qcom/msm8916.dtsi
@@ -460,6 +460,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";
@@ -475,6 +477,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 55ec3e8..f774e4c 100644
--- a/arch/arm64/boot/dts/qcom/msm8996.dtsi
+++ b/arch/arm64/boot/dts/qcom/msm8996.dtsi
@@ -258,6 +258,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 v4 5/9] 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>
Acked-by: Adrian Hunter <adrian.hunter@intel.com>
---
 drivers/mmc/host/sdhci-msm.c | 19 +++++++++++++++++++
 1 file changed, 19 insertions(+)

diff --git a/drivers/mmc/host/sdhci-msm.c b/drivers/mmc/host/sdhci-msm.c
index a46dd98..244e67a 100644
--- a/drivers/mmc/host/sdhci-msm.c
+++ b/drivers/mmc/host/sdhci-msm.c
@@ -565,6 +565,23 @@ 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 = msm_host->pdata->clk_table_sz;
+
+	return msm_host->pdata->clk_table[count - 1];
+}
+
+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);
+
+	return msm_host->pdata->clk_table[0];
+}
+
 static const struct of_device_id sdhci_msm_dt_match[] = {
 	{ .compatible = "qcom,sdhci-msm-v4" },
 	{},
@@ -576,6 +593,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.

[PATCH v4 6/9] 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 244e67a..a6280aa 100644
--- a/drivers/mmc/host/sdhci-msm.c
+++ b/drivers/mmc/host/sdhci-msm.c
@@ -603,7 +603,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 v4 7/9] 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 | 109 ++++++++++++++++++++++++++++++++++++++++++-
 1 file changed, 108 insertions(+), 1 deletion(-)

diff --git a/drivers/mmc/host/sdhci-msm.c b/drivers/mmc/host/sdhci-msm.c
index a6280aa..853639b 100644
--- a/drivers/mmc/host/sdhci-msm.c
+++ b/drivers/mmc/host/sdhci-msm.c
@@ -89,6 +89,7 @@ struct sdhci_msm_host {
 	struct mmc_host *mmc;
 	bool use_14lpp_dll_reset;
 	struct sdhci_msm_pltfm_data *pdata;
+	u32 clk_rate;
 };
 
 /* Platform specific tuning */
@@ -582,6 +583,112 @@ static unsigned int sdhci_msm_get_min_clock(struct sdhci_host *host)
 	return msm_host->pdata->clk_table[0];
 }
 
+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 = msm_host->pdata->clk_table_sz;
+	unsigned int sel_clk = -1;
+	int cnt;
+
+	if (req_clk < sdhci_msm_get_min_clock(host)) {
+		sel_clk = sdhci_msm_get_min_clock(host);
+		return sel_clk;
+	}
+
+	for (cnt = 0; cnt < count; cnt++) {
+		if (msm_host->pdata->clk_table[cnt] > req_clk) {
+			break;
+		} else if (msm_host->pdata->clk_table[cnt] == req_clk) {
+			sel_clk = msm_host->pdata->clk_table[cnt];
+			break;
+		}
+		sel_clk = msm_host->pdata->clk_table[cnt];
+	}
+	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 = 0;
+	int rc = 0;
+
+	if (!clock)
+		goto out;
+
+	if (clock != msm_host->clk_rate) {
+		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));
+	}
+out:
+	__sdhci_msm_set_clock(host, clock);
+}
+
 static const struct of_device_id sdhci_msm_dt_match[] = {
 	{ .compatible = "qcom,sdhci-msm-v4" },
 	{},
@@ -592,7 +699,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.

[PATCH v4 8/9] 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 | 22 ++++++++++++++++++----
 1 file changed, 18 insertions(+), 4 deletions(-)

diff --git a/drivers/mmc/host/sdhci-msm.c b/drivers/mmc/host/sdhci-msm.c
index 853639b..538594c 100644
--- a/drivers/mmc/host/sdhci-msm.c
+++ b/drivers/mmc/host/sdhci-msm.c
@@ -667,21 +667,35 @@ 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 = 0;
+	struct mmc_ios curr_ios = host->mmc->ios;
+	u32 msm_clock, ddr_clock;
 	int rc = 0;
 
 	if (!clock)
 		goto out;
 
-	if (clock != msm_host->clk_rate) {
-		msm_clock = sdhci_msm_get_msm_clk_rate(host, clock);
+	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)) {
+		/*
+		 * 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.
+		 */
+		ddr_clock = clock * 2;
+		msm_clock = sdhci_msm_get_msm_clk_rate(host, ddr_clock);
+	}
+
+	if (msm_clock != msm_host->clk_rate) {
 		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;
+		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.

[PATCH v4 9/9] 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 6f2c0b8..41e4ce4 100644
--- a/arch/arm64/boot/dts/qcom/msm8916.dtsi
+++ b/arch/arm64/boot/dts/qcom/msm8916.dtsi
@@ -462,6 +462,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.

Re: [PATCH v4 3/9] mmc: sdhci-msm: add pltfm_data support to get clk-rates from DT

[Stephen Boyd]

On 08/24, 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>
> ---

Please include the DT binding review list in binding updates.

>  .../devicetree/bindings/mmc/sdhci-msm.txt          |  1 +
>  drivers/mmc/host/sdhci-msm.c                       | 60 ++++++++++++++++++++++
>  2 files changed, 61 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 not use OPPv2 binding for this? We already have a way to
express frequencies for devices with that binding, and we'll need
to attach voltages to those frequencies at some point in the
future if we want to handle DVFS on these devices.

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

Re: [PATCH v4 8/9] mmc: sdhci-msm: Add clock changes for DDR mode.

[Adrian Hunter]

On 24/08/16 13:03, 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@codeaurora.org>
> ---
>  drivers/mmc/host/sdhci-msm.c | 22 ++++++++++++++++++----
>  1 file changed, 18 insertions(+), 4 deletions(-)
> 
> diff --git a/drivers/mmc/host/sdhci-msm.c b/drivers/mmc/host/sdhci-msm.c
> index 853639b..538594c 100644
> --- a/drivers/mmc/host/sdhci-msm.c
> +++ b/drivers/mmc/host/sdhci-msm.c
> @@ -667,21 +667,35 @@ 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 = 0;
> +	struct mmc_ios curr_ios = host->mmc->ios;
> +	u32 msm_clock, ddr_clock;
>  	int rc = 0;
>  
>  	if (!clock)
>  		goto out;
>  
> -	if (clock != msm_host->clk_rate) {
> -		msm_clock = sdhci_msm_get_msm_clk_rate(host, clock);
> +	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)) {
> +		/*
> +		 * 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.
> +		 */
> +		ddr_clock = clock * 2;
> +		msm_clock = sdhci_msm_get_msm_clk_rate(host, ddr_clock);
> +	}
> +

Could be slightly simpler e.g.

	if ((curr_ios.timing == MMC_TIMING_UHS_DDR50) ||
	    (curr_ios.timing == MMC_TIMING_MMC_DDR52) ||
	    (curr_ios.timing == MMC_TIMING_MMC_HS400)) {
		/*
		 * 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) {
>  		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;
> +		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));
>  	}
> 

Re: [PATCH v4 3/9] mmc: sdhci-msm: add pltfm_data support to get clk-rates from DT

[Ritesh Harjani]

Hi Stephen,

On 8/26/2016 3:57 AM, Stephen Boyd wrote:
> On 08/24, 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>
>> ---
>
> Please include the DT binding review list in binding updates.
>
>>  .../devicetree/bindings/mmc/sdhci-msm.txt          |  1 +
>>  drivers/mmc/host/sdhci-msm.c                       | 60 ++++++++++++++++++++++
>>  2 files changed, 61 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 not use OPPv2 binding for this? We already have a way to
> express frequencies for devices with that binding, and we'll need
> to attach voltages to those frequencies at some point in the
> future if we want to handle DVFS on these devices.
>
OPPv2 may not work out in this case. This was also discussed at [1].
These clk-rates are not as per voltage points(or any OPP) but as per bus 
speed mode request from higher layer.

[1] - https://patchwork.kernel.org/patch/9204879/

Thanks
Ritesh

Re: [PATCH v4 8/9] mmc: sdhci-msm: Add clock changes for DDR mode.

[Ritesh Harjani]

Hi Adrian,


On 8/29/2016 6:47 PM, Adrian Hunter wrote:
> On 24/08/16 13:03, 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@codeaurora.org>
>> ---
>>  drivers/mmc/host/sdhci-msm.c | 22 ++++++++++++++++++----
>>  1 file changed, 18 insertions(+), 4 deletions(-)
>>
>> diff --git a/drivers/mmc/host/sdhci-msm.c b/drivers/mmc/host/sdhci-msm.c
>> index 853639b..538594c 100644
>> --- a/drivers/mmc/host/sdhci-msm.c
>> +++ b/drivers/mmc/host/sdhci-msm.c
>> @@ -667,21 +667,35 @@ 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 = 0;
>> +	struct mmc_ios curr_ios = host->mmc->ios;
>> +	u32 msm_clock, ddr_clock;
>>  	int rc = 0;
>>
>>  	if (!clock)
>>  		goto out;
>>
>> -	if (clock != msm_host->clk_rate) {
>> -		msm_clock = sdhci_msm_get_msm_clk_rate(host, clock);
>> +	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)) {
>> +		/*
>> +		 * 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.
>> +		 */
>> +		ddr_clock = clock * 2;
>> +		msm_clock = sdhci_msm_get_msm_clk_rate(host, ddr_clock);
>> +	}
>> +
>
> Could be slightly simpler e.g.
>
> 	if ((curr_ios.timing == MMC_TIMING_UHS_DDR50) ||
> 	    (curr_ios.timing == MMC_TIMING_MMC_DDR52) ||
> 	    (curr_ios.timing == MMC_TIMING_MMC_HS400)) {
> 		/*
> 		 * 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);
Sure. Done.

>
>
>> +	if (msm_clock != msm_host->clk_rate) {
>>  		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;
>> +		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 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 v4 3/9] mmc: sdhci-msm: add pltfm_data support to get clk-rates from DT

[Stephen Boyd]

On 08/30, Ritesh Harjani wrote:
> On 8/26/2016 3:57 AM, Stephen Boyd wrote:
> >On 08/24, 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>
> >>---
> >
> >Please include the DT binding review list in binding updates.
> >
> >> .../devicetree/bindings/mmc/sdhci-msm.txt          |  1 +
> >> drivers/mmc/host/sdhci-msm.c                       | 60 ++++++++++++++++++++++
> >> 2 files changed, 61 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 not use OPPv2 binding for this? We already have a way to
> >express frequencies for devices with that binding, and we'll need
> >to attach voltages to those frequencies at some point in the
> >future if we want to handle DVFS on these devices.
> >
> OPPv2 may not work out in this case. This was also discussed at [1].
> These clk-rates are not as per voltage points(or any OPP) but as per
> bus speed mode request from higher layer.
> 

That post you link to had a response from Andy that echoed the
same sentiment and then no reply.

Are they a set of frequencies the device can operate at? Yes. Do
they have associated voltages that aren't expressed in this
patch? Yes.

Perhaps I've missed something though and the index of the array
of rates corresponds to some bus speed mode? That isn't clearly
expressed in the binding. Do the frequencies come from some spec
for those bus speed modes? Either way, I'd like to understand why
we need to put frequency tables into DT (hint: the commit text
should explain that).

-- 
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 v4 3/9] mmc: sdhci-msm: add pltfm_data support to get clk-rates from DT

[Ritesh Harjani]

Hi Stephen,


On 9/7/2016 9:44 PM, Stephen Boyd wrote:
> On 08/30, Ritesh Harjani wrote:
>> On 8/26/2016 3:57 AM, Stephen Boyd wrote:
>>> On 08/24, 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>
>>>> ---
>>>
>>> Please include the DT binding review list in binding updates.
Sure, will do it.

>>>
>>>> .../devicetree/bindings/mmc/sdhci-msm.txt          |  1 +
>>>> drivers/mmc/host/sdhci-msm.c                       | 60 ++++++++++++++++++++++
>>>> 2 files changed, 61 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 not use OPPv2 binding for this? We already have a way to
>>> express frequencies for devices with that binding, and we'll need
>>> to attach voltages to those frequencies at some point in the
>>> future if we want to handle DVFS on these devices.
>>>
>> OPPv2 may not work out in this case. This was also discussed at [1].
>> These clk-rates are not as per voltage points(or any OPP) but as per
>> bus speed mode request from higher layer.
>>
>
> That post you link to had a response from Andy that echoed the
> same sentiment and then no reply.
>
> Are they a set of frequencies the device can operate at? Yes. Do
> they have associated voltages that aren't expressed in this
> patch? Yes.
>
> Perhaps I've missed something though and the index of the array
> of rates corresponds to some bus speed mode? That isn't clearly
> expressed in the binding. Do the frequencies come from some spec
> for those bus speed modes? Either way, I'd like to understand why
> we need to put frequency tables into DT (hint: the commit text
> should explain that).
>
As discussed offline, I will get back on this.

Regards
Ritesh

Re: [PATCH v4 3/9] mmc: sdhci-msm: add pltfm_data support to get clk-rates from DT

[Arnd Bergmann]

On Wednesday, August 24, 2016 3:33:38 PM CEST Ritesh Harjani wrote:
>  #define CMUX_SHIFT_PHASE_SHIFT 24
>  #define CMUX_SHIFT_PHASE_MASK  (7 << CMUX_SHIFT_PHASE_SHIFT)
>  
> +struct sdhci_msm_pltfm_data {
> +       u32 *clk_table;
> +       int clk_table_sz;
> +};
> +
>  struct sdhci_msm_host {
>         struct platform_device *pdev;
>         void __iomem *core_mem; /* MSM SDCC mapped address */
> @@ -83,6 +88,7 @@ struct sdhci_msm_host {
>         struct clk *bus_clk;    /* SDHC bus voter clock */
>         struct mmc_host *mmc;
>         bool use_14lpp_dll_reset;
> +       struct sdhci_msm_pltfm_data *pdata;
>  };
>  
>  /* Platform specific tuning */
> 

Just noticed this. Please don't add another structure, just put the
new members into struct sdhci_msm_host directly.

We only need platform_data structures for drivers that also work
with old-style board files, which this one doesn't.

	Arnd

Re: [PATCH v4 3/9] mmc: sdhci-msm: add pltfm_data support to get clk-rates from DT

[Ritesh Harjani]

Hi Arnd,

On 9/12/2016 1:03 PM, Arnd Bergmann wrote:
> On Wednesday, August 24, 2016 3:33:38 PM CEST Ritesh Harjani wrote:
>>  #define CMUX_SHIFT_PHASE_SHIFT 24
>>  #define CMUX_SHIFT_PHASE_MASK  (7 << CMUX_SHIFT_PHASE_SHIFT)
>>
>> +struct sdhci_msm_pltfm_data {
>> +       u32 *clk_table;
>> +       int clk_table_sz;
>> +};
>> +
>>  struct sdhci_msm_host {
>>         struct platform_device *pdev;
>>         void __iomem *core_mem; /* MSM SDCC mapped address */
>> @@ -83,6 +88,7 @@ struct sdhci_msm_host {
>>         struct clk *bus_clk;    /* SDHC bus voter clock */
>>         struct mmc_host *mmc;
>>         bool use_14lpp_dll_reset;
>> +       struct sdhci_msm_pltfm_data *pdata;
>>  };
>>
>>  /* Platform specific tuning */
>>
>
> Just noticed this. Please don't add another structure, just put the
> new members into struct sdhci_msm_host directly.
>
> We only need platform_data structures for drivers that also work
> with old-style board files, which this one doesn't.

Ok, I will move this into sdhci_msm_host directly.


Thanks
Ritesh

>
> 	Arnd
>

Re: [PATCH v4 3/9] mmc: sdhci-msm: add pltfm_data support to get clk-rates from DT

[Ritesh Harjani]

Hi Stephen/Adrian,

Let me give a little background on this. We would need more suggestions 
and opinion on this. Sorry for the delay it took.


Background -
Below change adds clk-rates DT entry since sdhci-msm driver uses this 
clk-rates table to find the nearest *floor* value of clock (this clock 
request comes will come from core layer) supported by msm platform.

Currently we don't have an API from clock driver which we can use it 
here to find these values(supported by msm platforms) and call for 
clk_set_rate. Hence the custom DT entry to have a table.

Please let us know if there is an API to find the floor value of clock 
from clock driver. I tried clk_set_max_rate, but it is not giving 
expected output (at least for msm platforms).


Comment-
The request from Stephen was to check if we could utilize OppV2 
framework since it provides an API - dev_pm_opp_find_freq_floor.
Also to check if we could get it done from existing clock driver APIs.
Please correct me I am missing anything.


Concern and opinion?
1. What about if there are more clocks getting added to sdhci DT node?
I see the Oppv2 framework taking device pointer as 1st argument. In 
cases where we have more than 1 clocks per device, how will Oppv2 handle 
this?
2. Here the concern is that shifting to Oppv2 framework only for use of 
this 1 API does not seems very useful. In case if there exist a clock 
driver API to do this, we may not have anything to be populated in DT.


Opinion and suggestions please?
Is it ok to continue to use clk-rates DT entry here for this case?


Regards
Ritesh


On 9/12/2016 11:47 AM, Ritesh Harjani wrote:
> Hi Stephen,
>
>
> On 9/7/2016 9:44 PM, Stephen Boyd wrote:
>> On 08/30, Ritesh Harjani wrote:
>>> On 8/26/2016 3:57 AM, Stephen Boyd wrote:
>>>> On 08/24, 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>
>>>>> ---
>>>>
>>>> Please include the DT binding review list in binding updates.
> Sure, will do it.
>
>>>>
>>>>> .../devicetree/bindings/mmc/sdhci-msm.txt          |  1 +
>>>>> drivers/mmc/host/sdhci-msm.c                       | 60
>>>>> ++++++++++++++++++++++
>>>>> 2 files changed, 61 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 not use OPPv2 binding for this? We already have a way to
>>>> express frequencies for devices with that binding, and we'll need
>>>> to attach voltages to those frequencies at some point in the
>>>> future if we want to handle DVFS on these devices.
>>>>
>>> OPPv2 may not work out in this case. This was also discussed at [1].
>>> These clk-rates are not as per voltage points(or any OPP) but as per
>>> bus speed mode request from higher layer.
>>>
>>
>> That post you link to had a response from Andy that echoed the
>> same sentiment and then no reply.
>>
>> Are they a set of frequencies the device can operate at? Yes. Do
>> they have associated voltages that aren't expressed in this
>> patch? Yes.
>>
>> Perhaps I've missed something though and the index of the array
>> of rates corresponds to some bus speed mode? That isn't clearly
>> expressed in the binding. Do the frequencies come from some spec
>> for those bus speed modes? Either way, I'd like to understand why
>> we need to put frequency tables into DT (hint: the commit text
>> should explain that).
>>
> As discussed offline, I will get back on this.
>
> Regards
> Ritesh