[PATCH 00/10] Add clk-rates and DDR support to sdhci-msm

[PATCH 00/10] Add clk-rates and DDR support to sdhci-msm

[Ritesh Harjani]

Hi All, 

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 0001 - 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 0002-0008 bring in required change for this to
sdhci-msm and DT of db410c.

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

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

Ritesh Harjani (6):
  mmc: sdhci-msm: add pltfm_data support to get clk-rates from DT
  arm64: dts: qcom: msm8916: Add clk-rates to sdhc1 & sdhc2
  mmc: sdhci-msm: Add get_min_clock() and get_max_clock() callback
  mmc: sdhci-msm: Enable SDHCI_QUIRK_CAP_CLOCK_BASE_BROKEN
  mmc: sdhci-msm: Add clock changes for DDR mode.
  arm64: dts: qcom: msm8916: Add ddr support to sdhc1

Sahitya Tummala (3):
  mmc: sdhci: Add SDHCI_QUIRK2_ALWAYS_USE_BASE_CLOCK quirk2 support
  mmc: sdhci-msm: Implement set_clock callback for sdhci-msm
  mmc: sdhci-msm: Enable SDHCI_QUIRK2_ALWAYS_USE_BASE_CLOCK to sdhci-msm

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

 .../devicetree/bindings/mmc/sdhci-msm.txt          |   1 +
 arch/arm64/boot/dts/qcom/msm8916.dtsi              |   5 +
 drivers/mmc/host/sdhci-msm.c                       | 209 ++++++++++++++++++++-
 drivers/mmc/host/sdhci.c                           |   4 +
 drivers/mmc/host/sdhci.h                           |   6 +
 5 files changed, 223 insertions(+), 2 deletions(-)

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

[PATCH 01/10] mmc: sdhci-msm: Update DLL reset sequence

[Ritesh Harjani]

From: Venkat Gopalakrishnan <venkatg@codeaurora.org>

SDCC core with minor version >= 0x42 requires a change 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>
---
 drivers/mmc/host/sdhci-msm.c | 45 ++++++++++++++++++++++++++++++++++++++++++++
 1 file changed, 45 insertions(+)

diff --git a/drivers/mmc/host/sdhci-msm.c b/drivers/mmc/host/sdhci-msm.c
index 8ef44a2a..9e08424 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_updated_dll_reset;
 };
 
 /* Platform specific tuning */
@@ -303,6 +310,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;
 
@@ -316,6 +325,15 @@ static int msm_init_cm_dll(struct sdhci_host *host)
 	writel_relaxed((readl_relaxed(host->ioaddr + CORE_VENDOR_SPEC)
 			& ~CORE_CLK_PWRSAVE), host->ioaddr + CORE_VENDOR_SPEC);
 
+	if (msm_host->use_updated_dll_reset) {
+		writel_relaxed((readl_relaxed(host->ioaddr + CORE_DLL_CONFIG)
+				& ~CORE_CK_OUT_EN),
+				host->ioaddr + CORE_DLL_CONFIG);
+		writel_relaxed((readl_relaxed(host->ioaddr + CORE_DLL_CONFIG_2)
+				| CORE_DLL_CLOCK_DISABLE),
+				host->ioaddr + CORE_DLL_CONFIG_2);
+	}
+
 	/* 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);
@@ -325,6 +343,22 @@ static int msm_init_cm_dll(struct sdhci_host *host)
 			| CORE_DLL_PDN), host->ioaddr + CORE_DLL_CONFIG);
 	msm_cm_dll_set_freq(host);
 
+	if (msm_host->use_updated_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);
+
+		writel_relaxed(((readl_relaxed(host->ioaddr + CORE_DLL_CONFIG_2)
+				& ~(0xFF << 10)) | (mclk_freq << 10)),
+				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 */
 	writel_relaxed((readl_relaxed(host->ioaddr + CORE_DLL_CONFIG)
 			& ~CORE_DLL_RST), host->ioaddr + CORE_DLL_CONFIG);
@@ -333,6 +367,14 @@ static int msm_init_cm_dll(struct sdhci_host *host)
 	writel_relaxed((readl_relaxed(host->ioaddr + CORE_DLL_CONFIG)
 			& ~CORE_DLL_PDN), host->ioaddr + CORE_DLL_CONFIG);
 
+	if (msm_host->use_updated_dll_reset) {
+		msm_cm_dll_set_freq(host);
+		/* Enable the DLL clock */
+		writel_relaxed((readl_relaxed(host->ioaddr + CORE_DLL_CONFIG_2)
+				& ~CORE_DLL_CLOCK_DISABLE),
+				host->ioaddr + CORE_DLL_CONFIG_2);
+	}
+
 	/* Set DLL_EN bit to 1. */
 	writel_relaxed((readl_relaxed(host->ioaddr + CORE_DLL_CONFIG)
 			| CORE_DLL_EN), host->ioaddr + CORE_DLL_CONFIG);
@@ -631,6 +673,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_updated_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 02/10] 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                       | 71 ++++++++++++++++++++++
 2 files changed, 72 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 9e08424..2c745c2 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;
+	size_t 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_updated_dll_reset;
+	struct sdhci_msm_pltfm_data *pdata;
 };
 
 /* Platform specific tuning */
@@ -570,6 +576,67 @@ 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, size_t *size)
+{
+	struct device_node *np = dev->of_node;
+	int count = 0;
+	u32 *arr = NULL;
+	int ret = 0;
+
+	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);
+		ret = count;
+		goto out;
+	}
+
+	arr = devm_kzalloc(dev, count * sizeof(*arr), GFP_KERNEL);
+	if (!arr) {
+		ret = -ENOMEM;
+		goto out;
+	}
+
+	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);
+		goto out;
+	}
+	*table = arr;
+	*size = count;
+out:
+	return ret;
+}
+
+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;
+	size_t table_sz = 0;
+	u32 *table = NULL;
+
+	pdata = devm_kzalloc(dev, sizeof(*pdata), GFP_KERNEL);
+	if (!pdata)
+		goto out;
+
+	if (sdhci_msm_dt_get_array(dev, "clk-rates", &table, &table_sz)) {
+		dev_err(dev, "failed in DT parsing for supported clk-rates\n");
+		goto out;
+	}
+	if (!table || !table_sz) {
+		dev_err(dev, "Invalid clock table\n");
+		goto out;
+	}
+	pdata->clk_table = table;
+	pdata->clk_table_sz = table_sz;
+
+	return pdata;
+out:
+	return NULL;
+}
+
 static int sdhci_msm_probe(struct platform_device *pdev)
 {
 	struct sdhci_host *host;
@@ -596,6 +663,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 03/10] arm64: dts: qcom: msm8916: Add clk-rates to sdhc1 & sdhc2

[Ritesh Harjani]

Add supported msm8916 supported clk-rates for sdhc1 & sdhc2
in DT.

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

diff --git a/arch/arm64/boot/dts/qcom/msm8916.dtsi b/arch/arm64/boot/dts/qcom/msm8916.dtsi
index 9681200..5161740 100644
--- a/arch/arm64/boot/dts/qcom/msm8916.dtsi
+++ b/arch/arm64/boot/dts/qcom/msm8916.dtsi
@@ -388,6 +388,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";
@@ -403,6 +405,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";
 		};
-- 
The Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum, 
a Linux Foundation Collaborative Project.

[PATCH 04/10] 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: Ritesh Harjani <riteshh@codeaurora.org>
---
 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 2c745c2..5913ce1 100644
--- a/drivers/mmc/host/sdhci-msm.c
+++ b/drivers/mmc/host/sdhci-msm.c
@@ -553,6 +553,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" },
 	{},
@@ -564,6 +581,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 05/10] mmc: sdhci-msm: Enable SDHCI_QUIRK_CAP_CLOCK_BASE_BROKEN

[Ritesh Harjani]

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

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

diff --git a/drivers/mmc/host/sdhci-msm.c b/drivers/mmc/host/sdhci-msm.c
index 5913ce1..cfd0209 100644
--- a/drivers/mmc/host/sdhci-msm.c
+++ b/drivers/mmc/host/sdhci-msm.c
@@ -591,7 +591,8 @@ 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,
 	.ops = &sdhci_msm_ops,
 };
 
-- 
The Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum, 
a Linux Foundation Collaborative Project.

[PATCH 06/10] mmc: sdhci: Add SDHCI_QUIRK2_ALWAYS_USE_BASE_CLOCK quirk2 support

[Ritesh Harjani]

From: Sahitya Tummala <stummala@codeaurora.org>

MSM controller uses the base clock and does not use any divider.
The driver will use SDHCI_QUIRK2_ALWAYS_USE_BASE_CLOCK
and controls the base clock (MCLK) directly.

Signed-off-by: Sahitya Tummala <stummala@codeaurora.org>
Signed-off-by: Ritesh Harjani <riteshh@codeaurora.org>
---
 drivers/mmc/host/sdhci.c | 4 ++++
 drivers/mmc/host/sdhci.h | 6 ++++++
 2 files changed, 10 insertions(+)

diff --git a/drivers/mmc/host/sdhci.c b/drivers/mmc/host/sdhci.c
index cd65d47..a5c9dcb 100644
--- a/drivers/mmc/host/sdhci.c
+++ b/drivers/mmc/host/sdhci.c
@@ -1318,6 +1318,10 @@ u16 sdhci_calc_clk(struct sdhci_host *host, unsigned int clock,
 clock_set:
 	if (real_div)
 		*actual_clock = (host->max_clk * clk_mul) / real_div;
+
+	if (host->quirks2 & SDHCI_QUIRK2_ALWAYS_USE_BASE_CLOCK)
+		div = 0;
+
 	clk |= (div & SDHCI_DIV_MASK) << SDHCI_DIVIDER_SHIFT;
 	clk |= ((div & SDHCI_DIV_HI_MASK) >> SDHCI_DIV_MASK_LEN)
 		<< SDHCI_DIVIDER_HI_SHIFT;
diff --git a/drivers/mmc/host/sdhci.h b/drivers/mmc/host/sdhci.h
index 0411c9f..566c0fe 100644
--- a/drivers/mmc/host/sdhci.h
+++ b/drivers/mmc/host/sdhci.h
@@ -422,6 +422,12 @@ struct sdhci_host {
 #define SDHCI_QUIRK2_ACMD23_BROKEN			(1<<14)
 /* Broken Clock divider zero in controller */
 #define SDHCI_QUIRK2_CLOCK_DIV_ZERO_BROKEN		(1<<15)
+/*
+ * If the base clock can be scalable, then there should be no further
+ * clock dividing as the input clock itself will be scaled down to
+ * required frequency.
+ */
+#define SDHCI_QUIRK2_ALWAYS_USE_BASE_CLOCK		(1<<16)
 
 	int irq;		/* Device IRQ */
 	void __iomem *ioaddr;	/* Mapped address */
-- 
The Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum, 
a Linux Foundation Collaborative Project.

Re: [PATCH 06/10] mmc: sdhci: Add SDHCI_QUIRK2_ALWAYS_USE_BASE_CLOCK quirk2 support

[Shawn Lin]

On 2016/8/10 23:01, Ritesh Harjani wrote:
> From: Sahitya Tummala <stummala@codeaurora.org>
>
> MSM controller uses the base clock and does not use any divider.
> The driver will use SDHCI_QUIRK2_ALWAYS_USE_BASE_CLOCK
> and controls the base clock (MCLK) directly.

I think the direction for sdhci now is to overwrite
sdhci_calc_clk in your variant driver..

quirk is unacceptable now.

>
> Signed-off-by: Sahitya Tummala <stummala@codeaurora.org>
> Signed-off-by: Ritesh Harjani <riteshh@codeaurora.org>
> ---
>  drivers/mmc/host/sdhci.c | 4 ++++
>  drivers/mmc/host/sdhci.h | 6 ++++++
>  2 files changed, 10 insertions(+)
>
> diff --git a/drivers/mmc/host/sdhci.c b/drivers/mmc/host/sdhci.c
> index cd65d47..a5c9dcb 100644
> --- a/drivers/mmc/host/sdhci.c
> +++ b/drivers/mmc/host/sdhci.c
> @@ -1318,6 +1318,10 @@ u16 sdhci_calc_clk(struct sdhci_host *host, unsigned int clock,
>  clock_set:
>  	if (real_div)
>  		*actual_clock = (host->max_clk * clk_mul) / real_div;
> +
> +	if (host->quirks2 & SDHCI_QUIRK2_ALWAYS_USE_BASE_CLOCK)
> +		div = 0;
> +
>  	clk |= (div & SDHCI_DIV_MASK) << SDHCI_DIVIDER_SHIFT;
>  	clk |= ((div & SDHCI_DIV_HI_MASK) >> SDHCI_DIV_MASK_LEN)
>  		<< SDHCI_DIVIDER_HI_SHIFT;
> diff --git a/drivers/mmc/host/sdhci.h b/drivers/mmc/host/sdhci.h
> index 0411c9f..566c0fe 100644
> --- a/drivers/mmc/host/sdhci.h
> +++ b/drivers/mmc/host/sdhci.h
> @@ -422,6 +422,12 @@ struct sdhci_host {
>  #define SDHCI_QUIRK2_ACMD23_BROKEN			(1<<14)
>  /* Broken Clock divider zero in controller */
>  #define SDHCI_QUIRK2_CLOCK_DIV_ZERO_BROKEN		(1<<15)
> +/*
> + * If the base clock can be scalable, then there should be no further
> + * clock dividing as the input clock itself will be scaled down to
> + * required frequency.
> + */
> +#define SDHCI_QUIRK2_ALWAYS_USE_BASE_CLOCK		(1<<16)
>
>  	int irq;		/* Device IRQ */
>  	void __iomem *ioaddr;	/* Mapped address */
>


-- 
Best Regards
Shawn Lin

Re: [PATCH 06/10] mmc: sdhci: Add SDHCI_QUIRK2_ALWAYS_USE_BASE_CLOCK quirk2 support

[Ritesh Harjani]

Hi Shawn,

Thanks for the feedback.
In case if you have any comments on other patches of this patch series, 
it will be good to address all of them in next spin.


On 8/11/2016 7:37 AM, Shawn Lin wrote:
> On 2016/8/10 23:01, Ritesh Harjani wrote:
>> From: Sahitya Tummala <stummala@codeaurora.org>
>>
>> MSM controller uses the base clock and does not use any divider.
>> The driver will use SDHCI_QUIRK2_ALWAYS_USE_BASE_CLOCK
>> and controls the base clock (MCLK) directly.
>
> I think the direction for sdhci now is to overwrite
> sdhci_calc_clk in your variant driver..
Alright, will try and see if I can have similar to sdhci_set_clock 
itself in sdhci-msm driver.

>
> quirk is unacceptable now.
Alright, I will try to take care of this.

>
>>
>> Signed-off-by: Sahitya Tummala <stummala@codeaurora.org>
>> Signed-off-by: Ritesh Harjani <riteshh@codeaurora.org>
>> ---
>>  drivers/mmc/host/sdhci.c | 4 ++++
>>  drivers/mmc/host/sdhci.h | 6 ++++++
>>  2 files changed, 10 insertions(+)
>>
>> diff --git a/drivers/mmc/host/sdhci.c b/drivers/mmc/host/sdhci.c
>> index cd65d47..a5c9dcb 100644
>> --- a/drivers/mmc/host/sdhci.c
>> +++ b/drivers/mmc/host/sdhci.c
>> @@ -1318,6 +1318,10 @@ u16 sdhci_calc_clk(struct sdhci_host *host,
>> unsigned int clock,
>>  clock_set:
>>      if (real_div)
>>          *actual_clock = (host->max_clk * clk_mul) / real_div;
>> +
>> +    if (host->quirks2 & SDHCI_QUIRK2_ALWAYS_USE_BASE_CLOCK)
>> +        div = 0;
>> +
>>      clk |= (div & SDHCI_DIV_MASK) << SDHCI_DIVIDER_SHIFT;
>>      clk |= ((div & SDHCI_DIV_HI_MASK) >> SDHCI_DIV_MASK_LEN)
>>          << SDHCI_DIVIDER_HI_SHIFT;
>> diff --git a/drivers/mmc/host/sdhci.h b/drivers/mmc/host/sdhci.h
>> index 0411c9f..566c0fe 100644
>> --- a/drivers/mmc/host/sdhci.h
>> +++ b/drivers/mmc/host/sdhci.h
>> @@ -422,6 +422,12 @@ struct sdhci_host {
>>  #define SDHCI_QUIRK2_ACMD23_BROKEN            (1<<14)
>>  /* Broken Clock divider zero in controller */
>>  #define SDHCI_QUIRK2_CLOCK_DIV_ZERO_BROKEN        (1<<15)
>> +/*
>> + * If the base clock can be scalable, then there should be no further
>> + * clock dividing as the input clock itself will be scaled down to
>> + * required frequency.
>> + */
>> +#define SDHCI_QUIRK2_ALWAYS_USE_BASE_CLOCK        (1<<16)
>>
>>      int irq;        /* Device IRQ */
>>      void __iomem *ioaddr;    /* Mapped address */
>>
>
>

Adding callback in sdhci_calc_clk for geting clk divider

[Ritesh Harjani]

Hi Shawn, 

I have added the callback inside sdhci_calc_clk and removed the quirk.
Do you think this will be the right approach? 
Please let me know for more pointers otherwise.

Regards
Ritesh

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

[PATCH 06/10] mmc: sdhci: Add get_clk_div callback support

[Ritesh Harjani]

Few controllers (like MSM) may have to override div
in certain cases. Hence provide a callback to get the
div value for their driver.

Signed-off-by: Sahitya Tummala <stummala@codeaurora.org>
Signed-off-by: Ritesh Harjani <riteshh@codeaurora.org>
---
 drivers/mmc/host/sdhci.c | 8 ++++++++
 drivers/mmc/host/sdhci.h | 1 +
 2 files changed, 9 insertions(+)

diff --git a/drivers/mmc/host/sdhci.c b/drivers/mmc/host/sdhci.c
index cd65d47..cc3d6f2 100644
--- a/drivers/mmc/host/sdhci.c
+++ b/drivers/mmc/host/sdhci.c
@@ -1318,6 +1318,14 @@ u16 sdhci_calc_clk(struct sdhci_host *host, unsigned int clock,
 clock_set:
 	if (real_div)
 		*actual_clock = (host->max_clk * clk_mul) / real_div;
+	/*
+	 * Few controllers may have to override div
+	 * here. Hence provide a callback to get the
+	 * div value for them.
+	 */
+	if (host->ops->get_clk_div)
+		div = host->ops->get_clk_div(host, div);
+
 	clk |= (div & SDHCI_DIV_MASK) << SDHCI_DIVIDER_SHIFT;
 	clk |= ((div & SDHCI_DIV_HI_MASK) >> SDHCI_DIV_MASK_LEN)
 		<< SDHCI_DIVIDER_HI_SHIFT;
diff --git a/drivers/mmc/host/sdhci.h b/drivers/mmc/host/sdhci.h
index 0411c9f..4701001 100644
--- a/drivers/mmc/host/sdhci.h
+++ b/drivers/mmc/host/sdhci.h
@@ -562,6 +562,7 @@ struct sdhci_ops {
 					 struct mmc_card *card,
 					 unsigned int max_dtr, int host_drv,
 					 int card_drv, int *drv_type);
+	int	(*get_clk_div)(struct sdhci_host *host, int div);
 };
 
 #ifdef CONFIG_MMC_SDHCI_IO_ACCESSORS
-- 
1.8.2.1

Re: [PATCH 06/10] mmc: sdhci: Add get_clk_div callback support

[Jaehoon Chung]

Hi,

On 08/11/2016 10:52 PM, Ritesh Harjani wrote:
> Few controllers (like MSM) may have to override div
> in certain cases. Hence provide a callback to get the
> div value for their driver.
> 
> Signed-off-by: Sahitya Tummala <stummala@codeaurora.org>
> Signed-off-by: Ritesh Harjani <riteshh@codeaurora.org>
> ---
>  drivers/mmc/host/sdhci.c | 8 ++++++++
>  drivers/mmc/host/sdhci.h | 1 +
>  2 files changed, 9 insertions(+)
> 
> diff --git a/drivers/mmc/host/sdhci.c b/drivers/mmc/host/sdhci.c
> index cd65d47..cc3d6f2 100644
> --- a/drivers/mmc/host/sdhci.c
> +++ b/drivers/mmc/host/sdhci.c
> @@ -1318,6 +1318,14 @@ u16 sdhci_calc_clk(struct sdhci_host *host, unsigned int clock,
>  clock_set:
>  	if (real_div)
>  		*actual_clock = (host->max_clk * clk_mul) / real_div;
> +	/*
> +	 * Few controllers may have to override div
> +	 * here. Hence provide a callback to get the
> +	 * div value for them.
> +	 */
> +	if (host->ops->get_clk_div)
> +		div = host->ops->get_clk_div(host, div);

This is for only getting your div value. Few controllers?
Rather, use the existent callback function..It's better than adding new callback.

In your controller, add the your set_clock() callback. not get_clk_div.
(Well, Adrian might have other opinion.)

Best Regards,
Jaehoon Chung


> +
>  	clk |= (div & SDHCI_DIV_MASK) << SDHCI_DIVIDER_SHIFT;
>  	clk |= ((div & SDHCI_DIV_HI_MASK) >> SDHCI_DIV_MASK_LEN)
>  		<< SDHCI_DIVIDER_HI_SHIFT;
> diff --git a/drivers/mmc/host/sdhci.h b/drivers/mmc/host/sdhci.h
> index 0411c9f..4701001 100644
> --- a/drivers/mmc/host/sdhci.h
> +++ b/drivers/mmc/host/sdhci.h
> @@ -562,6 +562,7 @@ struct sdhci_ops {
>  					 struct mmc_card *card,
>  					 unsigned int max_dtr, int host_drv,
>  					 int card_drv, int *drv_type);
> +	int	(*get_clk_div)(struct sdhci_host *host, int div);
>  };
>  
>  #ifdef CONFIG_MMC_SDHCI_IO_ACCESSORS
> 

Re: [PATCH 06/10] mmc: sdhci: Add get_clk_div callback support

[Ritesh Harjani]

Hi Jaehoon/Adrian,


On 8/12/2016 7:04 AM, Jaehoon Chung wrote:
> Hi,
>
> On 08/11/2016 10:52 PM, Ritesh Harjani wrote:
>> Few controllers (like MSM) may have to override div
>> in certain cases. Hence provide a callback to get the
>> div value for their driver.
>>
>> Signed-off-by: Sahitya Tummala <stummala@codeaurora.org>
>> Signed-off-by: Ritesh Harjani <riteshh@codeaurora.org>
>> ---
>>  drivers/mmc/host/sdhci.c | 8 ++++++++
>>  drivers/mmc/host/sdhci.h | 1 +
>>  2 files changed, 9 insertions(+)
>>
>> diff --git a/drivers/mmc/host/sdhci.c b/drivers/mmc/host/sdhci.c
>> index cd65d47..cc3d6f2 100644
>> --- a/drivers/mmc/host/sdhci.c
>> +++ b/drivers/mmc/host/sdhci.c
>> @@ -1318,6 +1318,14 @@ u16 sdhci_calc_clk(struct sdhci_host *host, unsigned int clock,
>>  clock_set:
>>  	if (real_div)
>>  		*actual_clock = (host->max_clk * clk_mul) / real_div;
>> +	/*
>> +	 * Few controllers may have to override div
>> +	 * here. Hence provide a callback to get the
>> +	 * div value for them.
>> +	 */
>> +	if (host->ops->get_clk_div)
>> +		div = host->ops->get_clk_div(host, div);
>
> This is for only getting your div value. Few controllers?
> Rather, use the existent callback function..It's better than adding new callback.

As of today sdhci-of-arasan is the only user of this quirk 
-SDHCI_QUIRK2_CLOCK_DIV_ZERO_BROKEN. I was hoping that with this 
callback, we may get away with this quirk if sdhci-of-arasan can have 
get_clk_div callback implemented in it's driver?

Since I was not sure on this, so I did not modify sdhci-of-arasan. Thoughts?

>
> In your controller, add the your set_clock() callback. not get_clk_div.
> (Well, Adrian might have other opinion.)
Alright, please let me know what would be the right approach.

>
> Best Regards,
> Jaehoon Chung
>
>
>> +
>>  	clk |= (div & SDHCI_DIV_MASK) << SDHCI_DIVIDER_SHIFT;
>>  	clk |= ((div & SDHCI_DIV_HI_MASK) >> SDHCI_DIV_MASK_LEN)
>>  		<< SDHCI_DIVIDER_HI_SHIFT;
>> diff --git a/drivers/mmc/host/sdhci.h b/drivers/mmc/host/sdhci.h
>> index 0411c9f..4701001 100644
>> --- a/drivers/mmc/host/sdhci.h
>> +++ b/drivers/mmc/host/sdhci.h
>> @@ -562,6 +562,7 @@ struct sdhci_ops {
>>  					 struct mmc_card *card,
>>  					 unsigned int max_dtr, int host_drv,
>>  					 int card_drv, int *drv_type);
>> +	int	(*get_clk_div)(struct sdhci_host *host, int div);
>>  };
>>
>>  #ifdef CONFIG_MMC_SDHCI_IO_ACCESSORS
>>
>

Re: [PATCH 06/10] mmc: sdhci: Add get_clk_div callback support

[Shawn Lin]

在 2016/8/12 10:19, Ritesh Harjani 写道:
> Hi Jaehoon/Adrian,
>
>
> On 8/12/2016 7:04 AM, Jaehoon Chung wrote:
>> Hi,
>>
>> On 08/11/2016 10:52 PM, Ritesh Harjani wrote:
>>> Few controllers (like MSM) may have to override div
>>> in certain cases. Hence provide a callback to get the
>>> div value for their driver.
>>>
>>> Signed-off-by: Sahitya Tummala <stummala@codeaurora.org>
>>> Signed-off-by: Ritesh Harjani <riteshh@codeaurora.org>
>>> ---
>>>  drivers/mmc/host/sdhci.c | 8 ++++++++
>>>  drivers/mmc/host/sdhci.h | 1 +
>>>  2 files changed, 9 insertions(+)
>>>
>>> diff --git a/drivers/mmc/host/sdhci.c b/drivers/mmc/host/sdhci.c
>>> index cd65d47..cc3d6f2 100644
>>> --- a/drivers/mmc/host/sdhci.c
>>> +++ b/drivers/mmc/host/sdhci.c
>>> @@ -1318,6 +1318,14 @@ u16 sdhci_calc_clk(struct sdhci_host *host,
>>> unsigned int clock,
>>>  clock_set:
>>>      if (real_div)
>>>          *actual_clock = (host->max_clk * clk_mul) / real_div;
>>> +    /*
>>> +     * Few controllers may have to override div
>>> +     * here. Hence provide a callback to get the
>>> +     * div value for them.
>>> +     */
>>> +    if (host->ops->get_clk_div)
>>> +        div = host->ops->get_clk_div(host, div);
>>
>> This is for only getting your div value. Few controllers?
>> Rather, use the existent callback function..It's better than adding
>> new callback.
>
> As of today sdhci-of-arasan is the only user of this quirk
> -SDHCI_QUIRK2_CLOCK_DIV_ZERO_BROKEN. I was hoping that with this
> callback, we may get away with this quirk if sdhci-of-arasan can have
> get_clk_div callback implemented in it's driver?
>
> Since I was not sure on this, so I did not modify sdhci-of-arasan.
> Thoughts?

yup, I'm still using SDHCI_QUIRK2_CLOCK_DIV_ZERO_BROKEN in
sdhci-of-arasan now. If you are addressing this, please go ahead.

Per previous disscussion of sdhci, it is deprecated to add new quirks
or callback(?) into sdhci. It should be better to make it a library.
 From this view, you should overwrite the set_clock in your variant
driver.

I has a question here, (like MSM) may have to override div
in certain cases. What certain cases is? I just see you simply
return 0 there which means you want to bypass the clk?
If that is a trick of clk rate, we could use clk framework API
there to lower the input clk to make the calculation produce
zero div?


>
>>
>> In your controller, add the your set_clock() callback. not get_clk_div.
>> (Well, Adrian might have other opinion.)
> Alright, please let me know what would be the right approach.
>
>>
>> Best Regards,
>> Jaehoon Chung
>>
>>
>>> +
>>>      clk |= (div & SDHCI_DIV_MASK) << SDHCI_DIVIDER_SHIFT;
>>>      clk |= ((div & SDHCI_DIV_HI_MASK) >> SDHCI_DIV_MASK_LEN)
>>>          << SDHCI_DIVIDER_HI_SHIFT;
>>> diff --git a/drivers/mmc/host/sdhci.h b/drivers/mmc/host/sdhci.h
>>> index 0411c9f..4701001 100644
>>> --- a/drivers/mmc/host/sdhci.h
>>> +++ b/drivers/mmc/host/sdhci.h
>>> @@ -562,6 +562,7 @@ struct sdhci_ops {
>>>                       struct mmc_card *card,
>>>                       unsigned int max_dtr, int host_drv,
>>>                       int card_drv, int *drv_type);
>>> +    int    (*get_clk_div)(struct sdhci_host *host, int div);
>>>  };
>>>
>>>  #ifdef CONFIG_MMC_SDHCI_IO_ACCESSORS
>>>
>>
> --
> 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
>


-- 
Best Regards
Shawn Lin

Re: [PATCH 06/10] mmc: sdhci: Add get_clk_div callback support

[Ritesh Harjani]

Hi Shawn,

On 8/12/2016 8:51 AM, Shawn Lin wrote:
> 在 2016/8/12 10:19, Ritesh Harjani 写道:
>> Hi Jaehoon/Adrian,
>>
>>
>> On 8/12/2016 7:04 AM, Jaehoon Chung wrote:
>>> Hi,
>>>
>>> On 08/11/2016 10:52 PM, Ritesh Harjani wrote:
>>>> Few controllers (like MSM) may have to override div
>>>> in certain cases. Hence provide a callback to get the
>>>> div value for their driver.
>>>>
>>>> Signed-off-by: Sahitya Tummala <stummala@codeaurora.org>
>>>> Signed-off-by: Ritesh Harjani <riteshh@codeaurora.org>
>>>> ---
>>>>  drivers/mmc/host/sdhci.c | 8 ++++++++
>>>>  drivers/mmc/host/sdhci.h | 1 +
>>>>  2 files changed, 9 insertions(+)
>>>>
>>>> diff --git a/drivers/mmc/host/sdhci.c b/drivers/mmc/host/sdhci.c
>>>> index cd65d47..cc3d6f2 100644
>>>> --- a/drivers/mmc/host/sdhci.c
>>>> +++ b/drivers/mmc/host/sdhci.c
>>>> @@ -1318,6 +1318,14 @@ u16 sdhci_calc_clk(struct sdhci_host *host,
>>>> unsigned int clock,
>>>>  clock_set:
>>>>      if (real_div)
>>>>          *actual_clock = (host->max_clk * clk_mul) / real_div;
>>>> +    /*
>>>> +     * Few controllers may have to override div
>>>> +     * here. Hence provide a callback to get the
>>>> +     * div value for them.
>>>> +     */
>>>> +    if (host->ops->get_clk_div)
>>>> +        div = host->ops->get_clk_div(host, div);
>>>
>>> This is for only getting your div value. Few controllers?
>>> Rather, use the existent callback function..It's better than adding
>>> new callback.
>>
>> As of today sdhci-of-arasan is the only user of this quirk
>> -SDHCI_QUIRK2_CLOCK_DIV_ZERO_BROKEN. I was hoping that with this
>> callback, we may get away with this quirk if sdhci-of-arasan can have
>> get_clk_div callback implemented in it's driver?
>>
>> Since I was not sure on this, so I did not modify sdhci-of-arasan.
>> Thoughts?
>
> yup, I'm still using SDHCI_QUIRK2_CLOCK_DIV_ZERO_BROKEN in
> sdhci-of-arasan now. If you are addressing this, please go ahead.
>
> Per previous disscussion of sdhci, it is deprecated to add new quirks
> or callback(?) into sdhci. It should be better to make it a library.
On callback, let Adrian answer this. It will be helpful if the link of 
this discussion(to make sdhci a library) can be shared pls :)
It may be helpful if we have some (Do's and Don'ts), it may help others 
as well while making sdhci changes.

> From this view, you should overwrite the set_clock in your variant
> driver.
>
> I has a question here, (like MSM) may have to override div
> in certain cases. What certain cases is? I just see you simply
> return 0 there which means you want to bypass the clk?

Actually the commit was written by keeping in mind that sdhci-of-arasan 
will also use this callback. Since I was not sure, I thought of dropping 
changes of sdhci-of-arasan but did not change the commit msg.
Sorry for the confusion.

> If that is a trick of clk rate, we could use clk framework API
> there to lower the input clk to make the calculation produce
> zero div?
>
>
>>
>>>
>>> In your controller, add the your set_clock() callback. not get_clk_div.
>>> (Well, Adrian might have other opinion.)
>> Alright, please let me know what would be the right approach.
>>
>>>
>>> Best Regards,
>>> Jaehoon Chung
>>>
>>>
>>>> +
>>>>      clk |= (div & SDHCI_DIV_MASK) << SDHCI_DIVIDER_SHIFT;
>>>>      clk |= ((div & SDHCI_DIV_HI_MASK) >> SDHCI_DIV_MASK_LEN)
>>>>          << SDHCI_DIVIDER_HI_SHIFT;
>>>> diff --git a/drivers/mmc/host/sdhci.h b/drivers/mmc/host/sdhci.h
>>>> index 0411c9f..4701001 100644
>>>> --- a/drivers/mmc/host/sdhci.h
>>>> +++ b/drivers/mmc/host/sdhci.h
>>>> @@ -562,6 +562,7 @@ struct sdhci_ops {
>>>>                       struct mmc_card *card,
>>>>                       unsigned int max_dtr, int host_drv,
>>>>                       int card_drv, int *drv_type);
>>>> +    int    (*get_clk_div)(struct sdhci_host *host, int div);
>>>>  };
>>>>
>>>>  #ifdef CONFIG_MMC_SDHCI_IO_ACCESSORS
>>>>
>>>
>> --
>> 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 06/10] mmc: sdhci: Add get_clk_div callback support

[Shawn Lin]

在 2016/8/12 11:46, Ritesh Harjani 写道:
> Hi Shawn,
>
> On 8/12/2016 8:51 AM, Shawn Lin wrote:
>> 在 2016/8/12 10:19, Ritesh Harjani 写道:
>>> Hi Jaehoon/Adrian,
>>>
>>>
>>> On 8/12/2016 7:04 AM, Jaehoon Chung wrote:
>>>> Hi,
>>>>
>>>> On 08/11/2016 10:52 PM, Ritesh Harjani wrote:
>>>>> Few controllers (like MSM) may have to override div
>>>>> in certain cases. Hence provide a callback to get the
>>>>> div value for their driver.
>>>>>
>>>>> Signed-off-by: Sahitya Tummala <stummala@codeaurora.org>
>>>>> Signed-off-by: Ritesh Harjani <riteshh@codeaurora.org>
>>>>> ---
>>>>>  drivers/mmc/host/sdhci.c | 8 ++++++++
>>>>>  drivers/mmc/host/sdhci.h | 1 +
>>>>>  2 files changed, 9 insertions(+)
>>>>>
>>>>> diff --git a/drivers/mmc/host/sdhci.c b/drivers/mmc/host/sdhci.c
>>>>> index cd65d47..cc3d6f2 100644
>>>>> --- a/drivers/mmc/host/sdhci.c
>>>>> +++ b/drivers/mmc/host/sdhci.c
>>>>> @@ -1318,6 +1318,14 @@ u16 sdhci_calc_clk(struct sdhci_host *host,
>>>>> unsigned int clock,
>>>>>  clock_set:
>>>>>      if (real_div)
>>>>>          *actual_clock = (host->max_clk * clk_mul) / real_div;
>>>>> +    /*
>>>>> +     * Few controllers may have to override div
>>>>> +     * here. Hence provide a callback to get the
>>>>> +     * div value for them.
>>>>> +     */
>>>>> +    if (host->ops->get_clk_div)
>>>>> +        div = host->ops->get_clk_div(host, div);
>>>>
>>>> This is for only getting your div value. Few controllers?
>>>> Rather, use the existent callback function..It's better than adding
>>>> new callback.
>>>
>>> As of today sdhci-of-arasan is the only user of this quirk
>>> -SDHCI_QUIRK2_CLOCK_DIV_ZERO_BROKEN. I was hoping that with this
>>> callback, we may get away with this quirk if sdhci-of-arasan can have
>>> get_clk_div callback implemented in it's driver?
>>>
>>> Since I was not sure on this, so I did not modify sdhci-of-arasan.
>>> Thoughts?
>>
>> yup, I'm still using SDHCI_QUIRK2_CLOCK_DIV_ZERO_BROKEN in
>> sdhci-of-arasan now. If you are addressing this, please go ahead.
>>
>> Per previous disscussion of sdhci, it is deprecated to add new quirks
>> or callback(?) into sdhci. It should be better to make it a library.
> On callback, let Adrian answer this. It will be helpful if the link of
> this discussion(to make sdhci a library) can be shared pls :)
> It may be helpful if we have some (Do's and Don'ts), it may help others
> as well while making sdhci changes.

Well, I just find this thread https://lkml.org/lkml/2016/1/27/309,
but there are some before that which I can't find now.

>
>> From this view, you should overwrite the set_clock in your variant
>> driver.
>>
>> I has a question here, (like MSM) may have to override div
>> in certain cases. What certain cases is? I just see you simply
>> return 0 there which means you want to bypass the clk?
>
> Actually the commit was written by keeping in mind that sdhci-of-arasan
> will also use this callback. Since I was not sure, I thought of dropping
> changes of sdhci-of-arasan but did not change the commit msg.
> Sorry for the confusion.
>
>> If that is a trick of clk rate, we could use clk framework API
>> there to lower the input clk to make the calculation produce
>> zero div?
>>
>>
>>>
>>>>
>>>> In your controller, add the your set_clock() callback. not get_clk_div.
>>>> (Well, Adrian might have other opinion.)
>>> Alright, please let me know what would be the right approach.
>>>
>>>>
>>>> Best Regards,
>>>> Jaehoon Chung
>>>>
>>>>
>>>>> +
>>>>>      clk |= (div & SDHCI_DIV_MASK) << SDHCI_DIVIDER_SHIFT;
>>>>>      clk |= ((div & SDHCI_DIV_HI_MASK) >> SDHCI_DIV_MASK_LEN)
>>>>>          << SDHCI_DIVIDER_HI_SHIFT;
>>>>> diff --git a/drivers/mmc/host/sdhci.h b/drivers/mmc/host/sdhci.h
>>>>> index 0411c9f..4701001 100644
>>>>> --- a/drivers/mmc/host/sdhci.h
>>>>> +++ b/drivers/mmc/host/sdhci.h
>>>>> @@ -562,6 +562,7 @@ struct sdhci_ops {
>>>>>                       struct mmc_card *card,
>>>>>                       unsigned int max_dtr, int host_drv,
>>>>>                       int card_drv, int *drv_type);
>>>>> +    int    (*get_clk_div)(struct sdhci_host *host, int div);
>>>>>  };
>>>>>
>>>>>  #ifdef CONFIG_MMC_SDHCI_IO_ACCESSORS
>>>>>
>>>>
>>> --
>>> 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
>>>
>>
>>
> --
> 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
>


-- 
Best Regards
Shawn Lin

[PATCH 08/10] mmc: sdhci-msm: Add get_clk_div callback definition

[Ritesh Harjani]

MSM controller does not use the div, instead needs
the base clock and the driver controls the
base clock (MCLK) at GCC directly.

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

diff --git a/drivers/mmc/host/sdhci-msm.c b/drivers/mmc/host/sdhci-msm.c
index cfd0209..f148c96 100644
--- a/drivers/mmc/host/sdhci-msm.c
+++ b/drivers/mmc/host/sdhci-msm.c
@@ -570,6 +570,16 @@ static unsigned int sdhci_msm_get_min_clock(struct sdhci_host *host)
 	return msm_host->pdata->clk_table[0];
 }
 
+/*
+ * MSM SDHC controller HW recomendation is to not use
+ * clock divisor hence return 0 from here.
+ * Driver directly controls the clk-rate at GCC.
+ */
+static int sdhci_msm_get_clk_div(struct sdhci_host *host, int div)
+{
+		return 0;
+}
+
 static const struct of_device_id sdhci_msm_dt_match[] = {
 	{ .compatible = "qcom,sdhci-msm-v4" },
 	{},
@@ -586,6 +596,7 @@ static const struct sdhci_ops sdhci_msm_ops = {
 	.set_bus_width = sdhci_set_bus_width,
 	.set_uhs_signaling = sdhci_msm_set_uhs_signaling,
 	.voltage_switch = sdhci_msm_voltage_switch,
+	.get_clk_div = sdhci_msm_get_clk_div,
 };
 
 static const struct sdhci_pltfm_data sdhci_msm_pdata = {
-- 
1.8.2.1

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

[Ritesh Harjani]

From: Sahitya Tummala <stummala@codeaurora.org>

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

diff --git a/drivers/mmc/host/sdhci-msm.c b/drivers/mmc/host/sdhci-msm.c
index cfd0209..03deeab 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_updated_dll_reset;
 	struct sdhci_msm_pltfm_data *pdata;
+	u32 clk_rate;
 };
 
 /* Platform specific tuning */
@@ -570,6 +571,59 @@ 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;
+		} else {
+			sel_clk = msm_host->pdata->clk_table[cnt];
+		}
+	}
+	return sel_clk;
+}
+
+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_set_clock(host, clock);
+}
+
 static const struct of_device_id sdhci_msm_dt_match[] = {
 	{ .compatible = "qcom,sdhci-msm-v4" },
 	{},
@@ -580,7 +634,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 08/10] mmc: sdhci-msm: Enable SDHCI_QUIRK2_ALWAYS_USE_BASE_CLOCK to sdhci-msm

[Ritesh Harjani]

From: Sahitya Tummala <stummala@codeaurora.org>

This enable SDHCI_QUIRK2_ALWAYS_USE_BASE_CLOCK to SDHC MSM driver.
sdhci-msm controller needs the base clock and the driver
controls the base clock (MCLK) directly.

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

diff --git a/drivers/mmc/host/sdhci-msm.c b/drivers/mmc/host/sdhci-msm.c
index 03deeab..717d264 100644
--- a/drivers/mmc/host/sdhci-msm.c
+++ b/drivers/mmc/host/sdhci-msm.c
@@ -647,6 +647,7 @@ static const struct sdhci_pltfm_data sdhci_msm_pdata = {
 		  SDHCI_QUIRK_NO_CARD_NO_RESET |
 		  SDHCI_QUIRK_SINGLE_POWER_WRITE |
 		  SDHCI_QUIRK_CAP_CLOCK_BASE_BROKEN,
+	.quirks2 = SDHCI_QUIRK2_ALWAYS_USE_BASE_CLOCK,
 	.ops = &sdhci_msm_ops,
 };
 
-- 
The Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum, 
a Linux Foundation Collaborative Project.

[PATCH 09/10] 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 717d264..e9f829f 100644
--- a/drivers/mmc/host/sdhci-msm.c
+++ b/drivers/mmc/host/sdhci-msm.c
@@ -602,21 +602,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 = clock, ddr_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);
+	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 10/10] 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 5161740..514c61e 100644
--- a/arch/arm64/boot/dts/qcom/msm8916.dtsi
+++ b/arch/arm64/boot/dts/qcom/msm8916.dtsi
@@ -390,6 +390,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.