[PATCH v5 0/4] mmc: sdhci-msm: Rectify DLL programming sequence for SDCC

[PATCH v5 0/4] mmc: sdhci-msm: Rectify DLL programming sequence for SDCC

[Ram Prakash Gupta]

With the current DLL sequence stability issues are seen in
HS400 and HS200 mode for data transfers.

Rectify the DLL programming sequence as per latest hardware
programming guide and also incorporate support for HS200 and
HS400 DLL settings using the device tree.

Changes from v4:
1. Addressed Rob Herrirng & Konrad Dybcio comments:
   a. Regarding naming of dt entry.
2. Addressed Adrian Hunter comments:
   a. Regarding parsing of dt and storing variable in driver.
3. Additional change:
   a. Changes in patch 4/4 according to parsing change.

Changes from v3:
1. Addressed Dmitry Baryshkov comments:
   a. Regarding clk division by in V2 patchset
2. Addressed Konrad Dybcio comments:
   a. Renaming of parameters
   b. Memory allocation
   c. couldn't address __free, as didn't fit here
3. Addressed Krzysztof Kozlowsk comment:
   a. Regarding the dt binding
   b. commit message to reflect the need of dt
4. Additional change:
   a. DT parsing logic
   b. Maintain backward compatibility

Changes from v2:
1. Addressed Dmitry Baryshkov comments:
   a. Regarding TCXO frequency.
   b. Regarding clock rate.
   c. regarding checkpatch.

Changes from v1:
1. Addressed Tengfei Fan comment, added missing semicolocon
   in sdhci_msm_host structure.

Sachin Gupta (4):
  dt-bindings: mmc: Add dll-presets values for HS400 and HS200 modes
  mmc: sdhci-msm: Add core_major, minor to msm_host structure
  mmc: sdhci-msm: Add device tree parsing logic for DLL settings
  mmc: sdhci-msm: Rectify DLL programming sequence for SDCC

 .../devicetree/bindings/mmc/sdhci-msm.yaml    |   5 +
 drivers/mmc/host/sdhci-msm.c                  | 316 ++++++++++++++++--
 2 files changed, 302 insertions(+), 19 deletions(-)

-- 
2.34.1

[PATCH v5 1/4] dt-bindings: mmc: Add dll-presets values for HS400 and HS200 modes

[Ram Prakash Gupta]

From: Sachin Gupta <quic_sachgupt@quicinc.com>

Document the 'dll-presets' property for MMC device tree bindings.
The 'dll-presets' property defines the DLL configurations for HS400
and HS200 modes.

QC SoCs can have 0 to 4 SDHCI instances, and each one may need
different tuning.

Signed-off-by: Sachin Gupta <quic_sachgupt@quicinc.com>
Signed-off-by: Ram Prakash Gupta <quic_rampraka@quicinc.com>
---
 Documentation/devicetree/bindings/mmc/sdhci-msm.yaml | 5 +++++
 1 file changed, 5 insertions(+)

diff --git a/Documentation/devicetree/bindings/mmc/sdhci-msm.yaml b/Documentation/devicetree/bindings/mmc/sdhci-msm.yaml
index 594bd174ff21..f7b3b1ced3ce 100644
--- a/Documentation/devicetree/bindings/mmc/sdhci-msm.yaml
+++ b/Documentation/devicetree/bindings/mmc/sdhci-msm.yaml
@@ -138,6 +138,11 @@ properties:
     $ref: /schemas/types.yaml#/definitions/uint32
     description: platform specific settings for DLL_CONFIG reg.
 
+  qcom,dll-presets:
+    maxItems: 10
+    $ref: /schemas/types.yaml#/definitions/uint32-array
+    description: platform specific settings for DLL registers.
+
   iommus:
     minItems: 1
     maxItems: 8
-- 
2.34.1

[PATCH v5 2/4] mmc: sdhci-msm: Add core_major, minor to msm_host structure

[Ram Prakash Gupta]

From: Sachin Gupta <quic_sachgupt@quicinc.com>

Add the core_major and core_minor variables to
the msm_host structure, allowing these variables to be
accessed more easily throughout the msm_host context.
This update is necessary for an upcoming follow-up patch.

Signed-off-by: Sachin Gupta <quic_sachgupt@quicinc.com>
Signed-off-by: Ram Prakash Gupta <quic_rampraka@quicinc.com>
---
 drivers/mmc/host/sdhci-msm.c | 6 ++++++
 1 file changed, 6 insertions(+)

diff --git a/drivers/mmc/host/sdhci-msm.c b/drivers/mmc/host/sdhci-msm.c
index 4e5edbf2fc9b..423e7cccab7d 100644
--- a/drivers/mmc/host/sdhci-msm.c
+++ b/drivers/mmc/host/sdhci-msm.c
@@ -283,6 +283,8 @@ struct sdhci_msm_host {
 	bool tuning_done;
 	bool calibration_done;
 	u8 saved_tuning_phase;
+	u8 core_major;
+	u16 core_minor;
 	bool use_cdclp533;
 	u32 curr_pwr_state;
 	u32 curr_io_level;
@@ -2688,6 +2690,10 @@ static int sdhci_msm_probe(struct platform_device *pdev)
 	core_major = (core_version & CORE_VERSION_MAJOR_MASK) >>
 		      CORE_VERSION_MAJOR_SHIFT;
 	core_minor = core_version & CORE_VERSION_MINOR_MASK;
+
+	msm_host->core_major = core_major;
+	msm_host->core_minor = core_minor;
+
 	dev_dbg(&pdev->dev, "MCI Version: 0x%08x, major: 0x%04x, minor: 0x%02x\n",
 		core_version, core_major, core_minor);
 
-- 
2.34.1

[PATCH v5 3/4] mmc: sdhci-msm: Add device tree parsing logic for DLL settings

[Ram Prakash Gupta]

From: Sachin Gupta <quic_sachgupt@quicinc.com>

Parse dll-presets from dt and introduces the capability to
configure HS200 and HS400 DLL settings from device tree.

Signed-off-by: Sachin Gupta <quic_sachgupt@quicinc.com>
Signed-off-by: Ram Prakash Gupta <quic_rampraka@quicinc.com>
---
 drivers/mmc/host/sdhci-msm.c | 38 ++++++++++++++++++++++++++++++++++++
 1 file changed, 38 insertions(+)

diff --git a/drivers/mmc/host/sdhci-msm.c b/drivers/mmc/host/sdhci-msm.c
index 423e7cccab7d..8038b8def355 100644
--- a/drivers/mmc/host/sdhci-msm.c
+++ b/drivers/mmc/host/sdhci-msm.c
@@ -266,6 +266,19 @@ struct sdhci_msm_variant_info {
 	const struct sdhci_msm_offset *offset;
 };
 
+/*
+ * DLL registers which needs be programmed with HSR settings.
+ * Add any new register only at the end and don't change the
+ * sequence.
+ */
+struct sdhci_msm_dll {
+	u32 dll_config;
+	u32 dll_config_2;
+	u32 dll_config_3;
+	u32 dll_usr_ctl;
+	u32 ddr_config;
+};
+
 struct sdhci_msm_host {
 	struct platform_device *pdev;
 	void __iomem *core_mem;	/* MSM SDCC mapped address */
@@ -274,6 +287,7 @@ struct sdhci_msm_host {
 	struct clk *xo_clk;	/* TCXO clk needed for FLL feature of cm_dll*/
 	/* core, iface, cal and sleep clocks */
 	struct clk_bulk_data bulk_clks[4];
+	struct sdhci_msm_dll dll[2];
 #ifdef CONFIG_MMC_CRYPTO
 	struct qcom_ice *ice;
 #endif
@@ -302,6 +316,7 @@ struct sdhci_msm_host {
 	u32 dll_config;
 	u32 ddr_config;
 	bool vqmmc_enabled;
+	bool artanis_dll;
 };
 
 static const struct sdhci_msm_offset *sdhci_priv_msm_offset(struct sdhci_host *host)
@@ -2531,6 +2546,23 @@ static int sdhci_msm_gcc_reset(struct device *dev, struct sdhci_host *host)
 	return ret;
 }
 
+#define DLL_SIZE 10
+static int sdhci_msm_dt_parse_dll(struct device *dev, struct sdhci_msm_host *msm_host)
+{
+	int ret;
+	u32 *dll_table = &msm_host->dll[0].dll_config;
+
+	msm_host->artanis_dll = false;
+
+	ret = of_property_read_variable_u32_array(dev->of_node,
+						  "qcom,dll-presets",
+						  dll_table, DLL_SIZE, DLL_SIZE);
+	if (ret != -EINVAL)
+		msm_host->artanis_dll = true;
+
+	return ret;
+}
+
 static int sdhci_msm_probe(struct platform_device *pdev)
 {
 	struct sdhci_host *host;
@@ -2577,6 +2609,12 @@ static int sdhci_msm_probe(struct platform_device *pdev)
 
 	msm_host->saved_tuning_phase = INVALID_TUNING_PHASE;
 
+	ret = sdhci_msm_dt_parse_dll(&pdev->dev, msm_host);
+	if (ret == -ENODATA || ret == -EOVERFLOW) {
+		dev_err(&pdev->dev, "Bad DLL in dt (%d)\n", ret);
+		return ret;
+	}
+
 	ret = sdhci_msm_gcc_reset(&pdev->dev, host);
 	if (ret)
 		return ret;
-- 
2.34.1

[PATCH v5 4/4] mmc: sdhci-msm: Rectify DLL programming sequence for SDCC

[Ram Prakash Gupta]

From: Sachin Gupta <quic_sachgupt@quicinc.com>

With the current DLL sequence stability issues for data
transfer seen in HS400 and HS200 modes.

"mmc0: cqhci: error IRQ status: 0x00000000 cmd error -84
data error 0"

Rectify the DLL programming sequence as per latest hardware
programming guide

Signed-off-by: Sachin Gupta <quic_sachgupt@quicinc.com>
Signed-off-by: Ram Prakash Gupta <quic_rampraka@quicinc.com>
---
 drivers/mmc/host/sdhci-msm.c | 272 ++++++++++++++++++++++++++++++++---
 1 file changed, 253 insertions(+), 19 deletions(-)

diff --git a/drivers/mmc/host/sdhci-msm.c b/drivers/mmc/host/sdhci-msm.c
index 8038b8def355..a875e92f8663 100644
--- a/drivers/mmc/host/sdhci-msm.c
+++ b/drivers/mmc/host/sdhci-msm.c
@@ -28,6 +28,7 @@
 #define CORE_VERSION_MAJOR_SHIFT	28
 #define CORE_VERSION_MAJOR_MASK		(0xf << CORE_VERSION_MAJOR_SHIFT)
 #define CORE_VERSION_MINOR_MASK		0xff
+#define SDHCI_MSM_MIN_V_7FF		0x6e
 
 #define CORE_MCI_GENERICS		0x70
 #define SWITCHABLE_SIGNALING_VOLTAGE	BIT(29)
@@ -119,7 +120,8 @@
 #define CORE_PWRSAVE_DLL	BIT(3)
 
 #define DDR_CONFIG_POR_VAL	0x80040873
-
+#define DLL_CONFIG_3_POR_VAL	0x10
+#define TCXO_FREQ               19200000
 
 #define INVALID_TUNING_PHASE	-1
 #define SDHCI_MSM_MIN_CLOCK	400000
@@ -319,6 +321,16 @@ struct sdhci_msm_host {
 	bool artanis_dll;
 };
 
+enum dll_init_context {
+	DLL_INIT_NORMAL,
+	DLL_INIT_FROM_CX_COLLAPSE_EXIT,
+};
+
+enum mode {
+	HS400, // equivalent to SDR104 mode for DLL.
+	HS200, // equivalent to SDR50 mode for DLL.
+};
+
 static const struct sdhci_msm_offset *sdhci_priv_msm_offset(struct sdhci_host *host)
 {
 	struct sdhci_pltfm_host *pltfm_host = sdhci_priv(host);
@@ -803,6 +815,209 @@ static int msm_init_cm_dll(struct sdhci_host *host)
 	return 0;
 }
 
+static unsigned int sdhci_msm_get_min_clock(struct sdhci_host *host)
+{
+	return SDHCI_MSM_MIN_CLOCK;
+}
+
+static unsigned int sdhci_msm_get_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);
+	struct clk *core_clk = msm_host->bulk_clks[0].clk;
+	struct mmc_ios ios = host->mmc->ios;
+	unsigned int sup_clk;
+
+	if (req_clk < sdhci_msm_get_min_clock(host))
+		return sdhci_msm_get_min_clock(host);
+
+	sup_clk = clk_get_rate(core_clk);
+
+	if (ios.timing == MMC_TIMING_MMC_HS400 ||
+	    host->flags & SDHCI_HS400_TUNING)
+		sup_clk = sup_clk / 2;
+
+	return sup_clk;
+}
+
+/* Initialize the DLL (Programmable Delay Line) */
+static int sdhci_msm_configure_dll(struct sdhci_host *host, enum dll_init_context
+				 init_context, enum mode index)
+{
+	struct sdhci_pltfm_host *pltfm_host = sdhci_priv(host);
+	struct sdhci_msm_host *msm_host = sdhci_pltfm_priv(pltfm_host);
+	const struct sdhci_msm_offset *msm_offset = msm_host->offset;
+	struct mmc_host *mmc = host->mmc;
+	u32 ddr_cfg_offset, core_vendor_spec, config;
+	void __iomem *ioaddr = host->ioaddr;
+	unsigned long flags, dll_clock;
+	int rc = 0, wait_cnt = 50;
+
+	dll_clock = sdhci_msm_get_clk_rate(host, host->clock);
+	spin_lock_irqsave(&host->lock, flags);
+
+	core_vendor_spec = readl_relaxed(ioaddr + msm_offset->core_vendor_spec);
+
+	/*
+	 * Always disable PWRSAVE during the DLL power
+	 * up regardless of its current setting.
+	 */
+	core_vendor_spec &= ~CORE_CLK_PWRSAVE;
+	writel_relaxed(core_vendor_spec, ioaddr + msm_offset->core_vendor_spec);
+
+	if (msm_host->use_14lpp_dll_reset) {
+		/* Disable CK_OUT */
+		config = readl_relaxed(ioaddr + msm_offset->core_dll_config);
+		config &= ~CORE_CK_OUT_EN;
+		writel_relaxed(config, ioaddr + msm_offset->core_dll_config);
+
+		/* Disable the DLL clock */
+		config = readl_relaxed(ioaddr + msm_offset->core_dll_config_2);
+		config |= CORE_DLL_CLOCK_DISABLE;
+		writel_relaxed(config, ioaddr + msm_offset->core_dll_config_2);
+	}
+
+	/*
+	 * Write 1 to DLL_RST bit of DLL_CONFIG register
+	 * and Write 1 to DLL_PDN bit of DLL_CONFIG register.
+	 */
+	config = readl_relaxed(ioaddr + msm_offset->core_dll_config);
+	config |= (CORE_DLL_RST | CORE_DLL_PDN);
+	writel_relaxed(config, ioaddr + msm_offset->core_dll_config);
+
+	/*
+	 * Configure DLL_CONFIG_3 and USER_CTRL
+	 * (Only applicable for 7FF projects).
+	 */
+	if (msm_host->core_minor >= SDHCI_MSM_MIN_V_7FF) {
+		writel_relaxed(msm_host->dll[index].dll_config_3,
+			       ioaddr + msm_offset->core_dll_config_3);
+		writel_relaxed(msm_host->dll[index].dll_usr_ctl,
+			       ioaddr + msm_offset->core_dll_usr_ctl);
+	}
+
+	/*
+	 * Set DDR_CONFIG since step 7 is setting TEST_CTRL that can be skipped.
+	 */
+	ddr_cfg_offset = msm_host->updated_ddr_cfg ? msm_offset->core_ddr_config
+					: msm_offset->core_ddr_config_old;
+
+	config = msm_host->dll[index].ddr_config;
+	writel_relaxed(config, ioaddr + ddr_cfg_offset);
+
+	/* Set DLL_CONFIG_2 */
+	if (msm_host->use_14lpp_dll_reset) {
+		u32 mclk_freq;
+		int cycle_cnt;
+
+		/*
+		 * Only configure the mclk_freq in normal DLL init
+		 * context. If the DLL init is coming from
+		 * CX Collapse Exit context, the host->clock may be zero.
+		 * The DLL_CONFIG_2 register has already been restored to
+		 * proper value prior to getting here.
+		 */
+		if (init_context == DLL_INIT_NORMAL) {
+			cycle_cnt = readl_relaxed(ioaddr +
+					msm_offset->core_dll_config_2)
+					& CORE_FLL_CYCLE_CNT ? 8 : 4;
+
+			mclk_freq = DIV_ROUND_CLOSEST_ULL(dll_clock * cycle_cnt, TCXO_FREQ);
+
+			if (dll_clock < 100000000) {
+				pr_err("%s: %s: Non standard clk freq =%u\n",
+				       mmc_hostname(mmc), __func__, dll_clock);
+				rc = -EINVAL;
+				goto out;
+			}
+
+			config = readl_relaxed(ioaddr + msm_offset->core_dll_config_2);
+			config = (config & ~GENMASK(17, 10)) |
+					FIELD_PREP(GENMASK(17, 10), mclk_freq);
+			writel_relaxed(config, ioaddr + msm_offset->core_dll_config_2);
+		}
+		/* wait for 5us before enabling DLL clock */
+		udelay(5);
+	}
+
+	config = msm_host->dll[index].dll_config;
+	writel_relaxed(config, ioaddr + msm_offset->core_dll_config);
+
+	/* Wait for 52us */
+	spin_unlock_irqrestore(&host->lock, flags);
+	usleep_range(60, 70);
+	spin_lock_irqsave(&host->lock, flags);
+
+	/*
+	 * Write 0 to DLL_RST bit of DLL_CONFIG register
+	 * and Write 0 to DLL_PDN bit of DLL_CONFIG register.
+	 */
+	config &= ~CORE_DLL_RST;
+	writel_relaxed(config, ioaddr + msm_offset->core_dll_config);
+
+	config &= ~CORE_DLL_PDN;
+	writel_relaxed(config, ioaddr + msm_offset->core_dll_config);
+	/* Write 1 to DLL_RST bit of DLL_CONFIG register */
+	config |= CORE_DLL_RST;
+	writel_relaxed(config, ioaddr + msm_offset->core_dll_config);
+
+	/* Write 0 to DLL_RST bit of DLL_CONFIG register */
+	config &= ~CORE_DLL_RST;
+	writel_relaxed(config, ioaddr + msm_offset->core_dll_config);
+
+	/* Set CORE_DLL_CLOCK_DISABLE to 0 */
+	if (msm_host->use_14lpp_dll_reset) {
+		config = readl_relaxed(ioaddr + msm_offset->core_dll_config_2);
+		config &= ~CORE_DLL_CLOCK_DISABLE;
+		writel_relaxed(config, ioaddr + msm_offset->core_dll_config_2);
+	}
+
+	/* Set DLL_EN bit to 1. */
+	config = readl_relaxed(ioaddr + msm_offset->core_dll_config);
+	config |= CORE_DLL_EN;
+	writel_relaxed(config, ioaddr + msm_offset->core_dll_config);
+
+	/*
+	 * Wait for 8000 input clock. Here we calculate the
+	 * delay from fixed clock freq 192MHz, which turns out 42us.
+	 */
+	spin_unlock_irqrestore(&host->lock, flags);
+	usleep_range(50, 60);
+	spin_lock_irqsave(&host->lock, flags);
+
+	/* Set CK_OUT_EN bit to 1. */
+	config |= CORE_CK_OUT_EN;
+	writel_relaxed(config, ioaddr + msm_offset->core_dll_config);
+
+	/*
+	 * Wait until DLL_LOCK bit of DLL_STATUS register
+	 * becomes '1'.
+	 */
+	while (!(readl_relaxed(ioaddr + msm_offset->core_dll_status) &
+		 CORE_DLL_LOCK)) {
+		/* max. wait for 50us sec for LOCK bit to be set */
+		if (--wait_cnt == 0) {
+			dev_err(mmc_dev(mmc), "%s: DLL failed to LOCK\n",
+				mmc_hostname(mmc));
+			rc = -ETIMEDOUT;
+			goto out;
+		}
+		/* wait for 1us before polling again */
+		udelay(1);
+	}
+
+out:
+	if (core_vendor_spec & CORE_CLK_PWRSAVE) {
+		/* Reenable PWRSAVE as needed */
+		config = readl_relaxed(ioaddr + msm_offset->core_vendor_spec);
+		config |= CORE_CLK_PWRSAVE;
+		writel_relaxed(config, ioaddr + msm_offset->core_vendor_spec);
+	}
+
+	spin_unlock_irqrestore(&host->lock, flags);
+	return rc;
+}
+
 static void msm_hc_select_default(struct sdhci_host *host)
 {
 	struct sdhci_pltfm_host *pltfm_host = sdhci_priv(host);
@@ -925,14 +1140,31 @@ static void sdhci_msm_hc_select_mode(struct sdhci_host *host)
 		msm_hc_select_default(host);
 }
 
+static int sdhci_msm_init_dll(struct sdhci_host *host, enum dll_init_context init_context)
+{
+	if (host->mmc->ios.timing == MMC_TIMING_UHS_SDR104 ||
+	    host->mmc->ios.timing == MMC_TIMING_MMC_HS400)
+		return sdhci_msm_configure_dll(host, init_context, HS400);
+
+	return sdhci_msm_configure_dll(host, init_context, HS200);
+}
+
+static int sdhci_msm_dll_config(struct sdhci_host *host, enum dll_init_context init_context)
+{
+	struct sdhci_pltfm_host *pltfm_host = sdhci_priv(host);
+	struct sdhci_msm_host *msm_host = sdhci_pltfm_priv(pltfm_host);
+
+	return msm_host->artanis_dll ? sdhci_msm_init_dll(host, init_context) :
+		msm_init_cm_dll(host);
+}
+
 static int sdhci_msm_cdclp533_calibration(struct sdhci_host *host)
 {
 	struct sdhci_pltfm_host *pltfm_host = sdhci_priv(host);
 	struct sdhci_msm_host *msm_host = sdhci_pltfm_priv(pltfm_host);
+	const struct sdhci_msm_offset *msm_offset = msm_host->offset;
 	u32 config, calib_done;
 	int ret;
-	const struct sdhci_msm_offset *msm_offset =
-					msm_host->offset;
 
 	pr_debug("%s: %s: Enter\n", mmc_hostname(host->mmc), __func__);
 
@@ -940,7 +1172,7 @@ static int sdhci_msm_cdclp533_calibration(struct sdhci_host *host)
 	 * Retuning in HS400 (DDR mode) will fail, just reset the
 	 * tuning block and restore the saved tuning phase.
 	 */
-	ret = msm_init_cm_dll(host);
+	ret = sdhci_msm_dll_config(host, DLL_INIT_NORMAL);
 	if (ret)
 		goto out;
 
@@ -1028,7 +1260,7 @@ static int sdhci_msm_cdclp533_calibration(struct sdhci_host *host)
 	return ret;
 }
 
-static int sdhci_msm_cm_dll_sdc4_calibration(struct sdhci_host *host)
+static int sdhci_msm_cm_dll_sdc4_calibration(struct sdhci_host *host, enum mode index)
 {
 	struct mmc_host *mmc = host->mmc;
 	u32 dll_status, config, ddr_cfg_offset;
@@ -1051,7 +1283,11 @@ static int sdhci_msm_cm_dll_sdc4_calibration(struct sdhci_host *host)
 		ddr_cfg_offset = msm_offset->core_ddr_config;
 	else
 		ddr_cfg_offset = msm_offset->core_ddr_config_old;
-	writel_relaxed(msm_host->ddr_config, host->ioaddr + ddr_cfg_offset);
+
+	if (msm_host->artanis_dll)
+		writel_relaxed(msm_host->dll[index].ddr_config, host->ioaddr + ddr_cfg_offset);
+	else
+		writel_relaxed(msm_host->ddr_config, host->ioaddr + ddr_cfg_offset);
 
 	if (mmc->ios.enhanced_strobe) {
 		config = readl_relaxed(host->ioaddr +
@@ -1108,11 +1344,10 @@ static int sdhci_msm_hs400_dll_calibration(struct sdhci_host *host)
 {
 	struct sdhci_pltfm_host *pltfm_host = sdhci_priv(host);
 	struct sdhci_msm_host *msm_host = sdhci_pltfm_priv(pltfm_host);
+	const struct sdhci_msm_offset *msm_offset = msm_host->offset;
 	struct mmc_host *mmc = host->mmc;
-	int ret;
 	u32 config;
-	const struct sdhci_msm_offset *msm_offset =
-					msm_host->offset;
+	int ret;
 
 	pr_debug("%s: %s: Enter\n", mmc_hostname(host->mmc), __func__);
 
@@ -1120,7 +1355,8 @@ static int sdhci_msm_hs400_dll_calibration(struct sdhci_host *host)
 	 * Retuning in HS400 (DDR mode) will fail, just reset the
 	 * tuning block and restore the saved tuning phase.
 	 */
-	ret = msm_init_cm_dll(host);
+	ret = sdhci_msm_dll_config(host, DLL_INIT_NORMAL);
+
 	if (ret)
 		goto out;
 
@@ -1140,7 +1376,7 @@ static int sdhci_msm_hs400_dll_calibration(struct sdhci_host *host)
 	if (msm_host->use_cdclp533)
 		ret = sdhci_msm_cdclp533_calibration(host);
 	else
-		ret = sdhci_msm_cm_dll_sdc4_calibration(host);
+		ret = sdhci_msm_cm_dll_sdc4_calibration(host, HS400);
 out:
 	pr_debug("%s: %s: Exit, ret %d\n", mmc_hostname(host->mmc),
 		 __func__, ret);
@@ -1183,7 +1419,8 @@ static int sdhci_msm_restore_sdr_dll_config(struct sdhci_host *host)
 		return 0;
 
 	/* Reset the tuning block */
-	ret = msm_init_cm_dll(host);
+	ret = sdhci_msm_dll_config(host, DLL_INIT_NORMAL);
+
 	if (ret)
 		return ret;
 
@@ -1257,12 +1494,11 @@ static int sdhci_msm_execute_tuning(struct mmc_host *mmc, u32 opcode)
 	if (host->flags & SDHCI_HS400_TUNING) {
 		sdhci_msm_hc_select_mode(host);
 		msm_set_clock_rate_for_bus_mode(host, ios.clock);
-		host->flags &= ~SDHCI_HS400_TUNING;
 	}
 
 retry:
 	/* First of all reset the tuning block */
-	rc = msm_init_cm_dll(host);
+	rc = sdhci_msm_dll_config(host, DLL_INIT_NORMAL);
 	if (rc)
 		return rc;
 
@@ -1325,6 +1561,9 @@ static int sdhci_msm_execute_tuning(struct mmc_host *mmc, u32 opcode)
 		rc = -EIO;
 	}
 
+	if (host->flags & SDHCI_HS400_TUNING)
+		host->flags &= ~SDHCI_HS400_TUNING;
+
 	if (!rc)
 		msm_host->tuning_done = true;
 	return rc;
@@ -1845,11 +2084,6 @@ static unsigned int sdhci_msm_get_max_clock(struct sdhci_host *host)
 	return clk_round_rate(core_clk, ULONG_MAX);
 }
 
-static unsigned int sdhci_msm_get_min_clock(struct sdhci_host *host)
-{
-	return SDHCI_MSM_MIN_CLOCK;
-}
-
 /*
  * __sdhci_msm_set_clock - sdhci_msm clock control.
  *
-- 
2.34.1

Re: [PATCH v5 1/4] dt-bindings: mmc: Add dll-presets values for HS400 and HS200 modes

[Krzysztof Kozlowski]

On 13/10/2025 16:53, Ram Prakash Gupta wrote:
> From: Sachin Gupta <quic_sachgupt@quicinc.com>
> 
> Document the 'dll-presets' property for MMC device tree bindings.
> The 'dll-presets' property defines the DLL configurations for HS400
> and HS200 modes.
> 
> QC SoCs can have 0 to 4 SDHCI instances, and each one may need
> different tuning.
> 
> Signed-off-by: Sachin Gupta <quic_sachgupt@quicinc.com>
> Signed-off-by: Ram Prakash Gupta <quic_rampraka@quicinc.com>
> ---
>  Documentation/devicetree/bindings/mmc/sdhci-msm.yaml | 5 +++++
>  1 file changed, 5 insertions(+)
> 
> diff --git a/Documentation/devicetree/bindings/mmc/sdhci-msm.yaml b/Documentation/devicetree/bindings/mmc/sdhci-msm.yaml
> index 594bd174ff21..f7b3b1ced3ce 100644
> --- a/Documentation/devicetree/bindings/mmc/sdhci-msm.yaml
> +++ b/Documentation/devicetree/bindings/mmc/sdhci-msm.yaml
> @@ -138,6 +138,11 @@ properties:
>      $ref: /schemas/types.yaml#/definitions/uint32
>      description: platform specific settings for DLL_CONFIG reg.
>  
> +  qcom,dll-presets:
> +    maxItems: 10
> +    $ref: /schemas/types.yaml#/definitions/uint32-array
> +    description: platform specific settings for DLL registers.

One of my questions, never answered in original submission and in your
versions, was to see the DTS user of it. I still do not see the DTS user.

Best regards,
Krzysztof

Re: [PATCH v5 3/4] mmc: sdhci-msm: Add device tree parsing logic for DLL settings

[Adrian Hunter]

On 13/10/2025 17:53, Ram Prakash Gupta wrote:
> From: Sachin Gupta <quic_sachgupt@quicinc.com>
> 
> Parse dll-presets from dt and introduces the capability to
> configure HS200 and HS400 DLL settings from device tree.
> 
> Signed-off-by: Sachin Gupta <quic_sachgupt@quicinc.com>
> Signed-off-by: Ram Prakash Gupta <quic_rampraka@quicinc.com>
> ---
>  drivers/mmc/host/sdhci-msm.c | 38 ++++++++++++++++++++++++++++++++++++
>  1 file changed, 38 insertions(+)
> 
> diff --git a/drivers/mmc/host/sdhci-msm.c b/drivers/mmc/host/sdhci-msm.c
> index 423e7cccab7d..8038b8def355 100644
> --- a/drivers/mmc/host/sdhci-msm.c
> +++ b/drivers/mmc/host/sdhci-msm.c
> @@ -266,6 +266,19 @@ struct sdhci_msm_variant_info {
>  	const struct sdhci_msm_offset *offset;
>  };
>  
> +/*
> + * DLL registers which needs be programmed with HSR settings.
> + * Add any new register only at the end and don't change the
> + * sequence.
> + */
> +struct sdhci_msm_dll {
> +	u32 dll_config;
> +	u32 dll_config_2;
> +	u32 dll_config_3;
> +	u32 dll_usr_ctl;
> +	u32 ddr_config;
> +};
> +
>  struct sdhci_msm_host {
>  	struct platform_device *pdev;
>  	void __iomem *core_mem;	/* MSM SDCC mapped address */
> @@ -274,6 +287,7 @@ struct sdhci_msm_host {
>  	struct clk *xo_clk;	/* TCXO clk needed for FLL feature of cm_dll*/
>  	/* core, iface, cal and sleep clocks */
>  	struct clk_bulk_data bulk_clks[4];
> +	struct sdhci_msm_dll dll[2];
>  #ifdef CONFIG_MMC_CRYPTO
>  	struct qcom_ice *ice;
>  #endif
> @@ -302,6 +316,7 @@ struct sdhci_msm_host {
>  	u32 dll_config;
>  	u32 ddr_config;
>  	bool vqmmc_enabled;
> +	bool artanis_dll;
>  };
>  
>  static const struct sdhci_msm_offset *sdhci_priv_msm_offset(struct sdhci_host *host)
> @@ -2531,6 +2546,23 @@ static int sdhci_msm_gcc_reset(struct device *dev, struct sdhci_host *host)
>  	return ret;
>  }
>  
> +#define DLL_SIZE 10
> +static int sdhci_msm_dt_parse_dll(struct device *dev, struct sdhci_msm_host *msm_host)
> +{
> +	int ret;
> +	u32 *dll_table = &msm_host->dll[0].dll_config;

Local declarations in order of descending line length tends to
look neater, more readable e.g.

	u32 *dll_table = &msm_host->dll[0].dll_config;
	int ret;

> +
> +	msm_host->artanis_dll = false;
> +
> +	ret = of_property_read_variable_u32_array(dev->of_node,
> +						  "qcom,dll-presets",
> +						  dll_table, DLL_SIZE, DLL_SIZE);
> +	if (ret != -EINVAL)

Would rather have expected:

	if (ret == DLL_SIZE)
or
	if (ret > 0)

> +		msm_host->artanis_dll = true;
> +
> +	return ret;
> +}
> +
>  static int sdhci_msm_probe(struct platform_device *pdev)
>  {
>  	struct sdhci_host *host;
> @@ -2577,6 +2609,12 @@ static int sdhci_msm_probe(struct platform_device *pdev)
>  
>  	msm_host->saved_tuning_phase = INVALID_TUNING_PHASE;
>  
> +	ret = sdhci_msm_dt_parse_dll(&pdev->dev, msm_host);
> +	if (ret == -ENODATA || ret == -EOVERFLOW) {
> +		dev_err(&pdev->dev, "Bad DLL in dt (%d)\n", ret);
> +		return ret;
> +	}
> +
>  	ret = sdhci_msm_gcc_reset(&pdev->dev, host);
>  	if (ret)
>  		return ret;

Re: [PATCH v5 4/4] mmc: sdhci-msm: Rectify DLL programming sequence for SDCC

[Adrian Hunter]

On 13/10/2025 17:53, Ram Prakash Gupta wrote:
> From: Sachin Gupta <quic_sachgupt@quicinc.com>
> 
> With the current DLL sequence stability issues for data
> transfer seen in HS400 and HS200 modes.
> 
> "mmc0: cqhci: error IRQ status: 0x00000000 cmd error -84
> data error 0"
> 
> Rectify the DLL programming sequence as per latest hardware
> programming guide
> 
> Signed-off-by: Sachin Gupta <quic_sachgupt@quicinc.com>
> Signed-off-by: Ram Prakash Gupta <quic_rampraka@quicinc.com>
> ---
>  drivers/mmc/host/sdhci-msm.c | 272 ++++++++++++++++++++++++++++++++---
>  1 file changed, 253 insertions(+), 19 deletions(-)
> 
> diff --git a/drivers/mmc/host/sdhci-msm.c b/drivers/mmc/host/sdhci-msm.c
> index 8038b8def355..a875e92f8663 100644
> --- a/drivers/mmc/host/sdhci-msm.c
> +++ b/drivers/mmc/host/sdhci-msm.c
> @@ -28,6 +28,7 @@
>  #define CORE_VERSION_MAJOR_SHIFT	28
>  #define CORE_VERSION_MAJOR_MASK		(0xf << CORE_VERSION_MAJOR_SHIFT)
>  #define CORE_VERSION_MINOR_MASK		0xff
> +#define SDHCI_MSM_MIN_V_7FF		0x6e
>  
>  #define CORE_MCI_GENERICS		0x70
>  #define SWITCHABLE_SIGNALING_VOLTAGE	BIT(29)
> @@ -119,7 +120,8 @@
>  #define CORE_PWRSAVE_DLL	BIT(3)
>  
>  #define DDR_CONFIG_POR_VAL	0x80040873
> -
> +#define DLL_CONFIG_3_POR_VAL	0x10
> +#define TCXO_FREQ               19200000
>  
>  #define INVALID_TUNING_PHASE	-1
>  #define SDHCI_MSM_MIN_CLOCK	400000
> @@ -319,6 +321,16 @@ struct sdhci_msm_host {
>  	bool artanis_dll;
>  };
>  
> +enum dll_init_context {
> +	DLL_INIT_NORMAL,
> +	DLL_INIT_FROM_CX_COLLAPSE_EXIT,

DLL_INIT_FROM_CX_COLLAPSE_EXIT is never used?

> +};
> +
> +enum mode {
> +	HS400, // equivalent to SDR104 mode for DLL.
> +	HS200, // equivalent to SDR50 mode for DLL.
> +};
> +
>  static const struct sdhci_msm_offset *sdhci_priv_msm_offset(struct sdhci_host *host)
>  {
>  	struct sdhci_pltfm_host *pltfm_host = sdhci_priv(host);
> @@ -803,6 +815,209 @@ static int msm_init_cm_dll(struct sdhci_host *host)
>  	return 0;
>  }
>  
> +static unsigned int sdhci_msm_get_min_clock(struct sdhci_host *host)
> +{
> +	return SDHCI_MSM_MIN_CLOCK;
> +}
> +
> +static unsigned int sdhci_msm_get_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);
> +	struct clk *core_clk = msm_host->bulk_clks[0].clk;
> +	struct mmc_ios ios = host->mmc->ios;
> +	unsigned int sup_clk;
> +
> +	if (req_clk < sdhci_msm_get_min_clock(host))
> +		return sdhci_msm_get_min_clock(host);
> +
> +	sup_clk = clk_get_rate(core_clk);
> +
> +	if (ios.timing == MMC_TIMING_MMC_HS400 ||
> +	    host->flags & SDHCI_HS400_TUNING)
> +		sup_clk = sup_clk / 2;
> +
> +	return sup_clk;
> +}
> +
> +/* Initialize the DLL (Programmable Delay Line) */
> +static int sdhci_msm_configure_dll(struct sdhci_host *host, enum dll_init_context
> +				 init_context, enum mode index)
> +{
> +	struct sdhci_pltfm_host *pltfm_host = sdhci_priv(host);
> +	struct sdhci_msm_host *msm_host = sdhci_pltfm_priv(pltfm_host);
> +	const struct sdhci_msm_offset *msm_offset = msm_host->offset;
> +	struct mmc_host *mmc = host->mmc;
> +	u32 ddr_cfg_offset, core_vendor_spec, config;
> +	void __iomem *ioaddr = host->ioaddr;
> +	unsigned long flags, dll_clock;
> +	int rc = 0, wait_cnt = 50;
> +
> +	dll_clock = sdhci_msm_get_clk_rate(host, host->clock);
> +	spin_lock_irqsave(&host->lock, flags);
> +
> +	core_vendor_spec = readl_relaxed(ioaddr + msm_offset->core_vendor_spec);
> +
> +	/*
> +	 * Always disable PWRSAVE during the DLL power
> +	 * up regardless of its current setting.
> +	 */
> +	core_vendor_spec &= ~CORE_CLK_PWRSAVE;
> +	writel_relaxed(core_vendor_spec, ioaddr + msm_offset->core_vendor_spec);
> +
> +	if (msm_host->use_14lpp_dll_reset) {
> +		/* Disable CK_OUT */
> +		config = readl_relaxed(ioaddr + msm_offset->core_dll_config);
> +		config &= ~CORE_CK_OUT_EN;
> +		writel_relaxed(config, ioaddr + msm_offset->core_dll_config);
> +
> +		/* Disable the DLL clock */
> +		config = readl_relaxed(ioaddr + msm_offset->core_dll_config_2);
> +		config |= CORE_DLL_CLOCK_DISABLE;
> +		writel_relaxed(config, ioaddr + msm_offset->core_dll_config_2);
> +	}
> +
> +	/*
> +	 * Write 1 to DLL_RST bit of DLL_CONFIG register
> +	 * and Write 1 to DLL_PDN bit of DLL_CONFIG register.
> +	 */
> +	config = readl_relaxed(ioaddr + msm_offset->core_dll_config);
> +	config |= (CORE_DLL_RST | CORE_DLL_PDN);
> +	writel_relaxed(config, ioaddr + msm_offset->core_dll_config);
> +
> +	/*
> +	 * Configure DLL_CONFIG_3 and USER_CTRL
> +	 * (Only applicable for 7FF projects).
> +	 */
> +	if (msm_host->core_minor >= SDHCI_MSM_MIN_V_7FF) {
> +		writel_relaxed(msm_host->dll[index].dll_config_3,
> +			       ioaddr + msm_offset->core_dll_config_3);
> +		writel_relaxed(msm_host->dll[index].dll_usr_ctl,
> +			       ioaddr + msm_offset->core_dll_usr_ctl);
> +	}
> +
> +	/*
> +	 * Set DDR_CONFIG since step 7 is setting TEST_CTRL that can be skipped.
> +	 */
> +	ddr_cfg_offset = msm_host->updated_ddr_cfg ? msm_offset->core_ddr_config
> +					: msm_offset->core_ddr_config_old;
> +
> +	config = msm_host->dll[index].ddr_config;
> +	writel_relaxed(config, ioaddr + ddr_cfg_offset);
> +
> +	/* Set DLL_CONFIG_2 */
> +	if (msm_host->use_14lpp_dll_reset) {
> +		u32 mclk_freq;
> +		int cycle_cnt;
> +
> +		/*
> +		 * Only configure the mclk_freq in normal DLL init
> +		 * context. If the DLL init is coming from
> +		 * CX Collapse Exit context, the host->clock may be zero.
> +		 * The DLL_CONFIG_2 register has already been restored to
> +		 * proper value prior to getting here.
> +		 */
> +		if (init_context == DLL_INIT_NORMAL) {
> +			cycle_cnt = readl_relaxed(ioaddr +
> +					msm_offset->core_dll_config_2)
> +					& CORE_FLL_CYCLE_CNT ? 8 : 4;
> +
> +			mclk_freq = DIV_ROUND_CLOSEST_ULL(dll_clock * cycle_cnt, TCXO_FREQ);
> +
> +			if (dll_clock < 100000000) {
> +				pr_err("%s: %s: Non standard clk freq =%u\n",
> +				       mmc_hostname(mmc), __func__, dll_clock);
> +				rc = -EINVAL;
> +				goto out;
> +			}
> +
> +			config = readl_relaxed(ioaddr + msm_offset->core_dll_config_2);
> +			config = (config & ~GENMASK(17, 10)) |
> +					FIELD_PREP(GENMASK(17, 10), mclk_freq);
> +			writel_relaxed(config, ioaddr + msm_offset->core_dll_config_2);
> +		}
> +		/* wait for 5us before enabling DLL clock */
> +		udelay(5);
> +	}
> +
> +	config = msm_host->dll[index].dll_config;
> +	writel_relaxed(config, ioaddr + msm_offset->core_dll_config);
> +
> +	/* Wait for 52us */
> +	spin_unlock_irqrestore(&host->lock, flags);
> +	usleep_range(60, 70);
> +	spin_lock_irqsave(&host->lock, flags);
> +
> +	/*
> +	 * Write 0 to DLL_RST bit of DLL_CONFIG register
> +	 * and Write 0 to DLL_PDN bit of DLL_CONFIG register.
> +	 */
> +	config &= ~CORE_DLL_RST;
> +	writel_relaxed(config, ioaddr + msm_offset->core_dll_config);
> +
> +	config &= ~CORE_DLL_PDN;
> +	writel_relaxed(config, ioaddr + msm_offset->core_dll_config);
> +	/* Write 1 to DLL_RST bit of DLL_CONFIG register */
> +	config |= CORE_DLL_RST;
> +	writel_relaxed(config, ioaddr + msm_offset->core_dll_config);
> +
> +	/* Write 0 to DLL_RST bit of DLL_CONFIG register */
> +	config &= ~CORE_DLL_RST;
> +	writel_relaxed(config, ioaddr + msm_offset->core_dll_config);
> +
> +	/* Set CORE_DLL_CLOCK_DISABLE to 0 */
> +	if (msm_host->use_14lpp_dll_reset) {
> +		config = readl_relaxed(ioaddr + msm_offset->core_dll_config_2);
> +		config &= ~CORE_DLL_CLOCK_DISABLE;
> +		writel_relaxed(config, ioaddr + msm_offset->core_dll_config_2);
> +	}
> +
> +	/* Set DLL_EN bit to 1. */
> +	config = readl_relaxed(ioaddr + msm_offset->core_dll_config);
> +	config |= CORE_DLL_EN;
> +	writel_relaxed(config, ioaddr + msm_offset->core_dll_config);
> +
> +	/*
> +	 * Wait for 8000 input clock. Here we calculate the
> +	 * delay from fixed clock freq 192MHz, which turns out 42us.
> +	 */
> +	spin_unlock_irqrestore(&host->lock, flags);
> +	usleep_range(50, 60);
> +	spin_lock_irqsave(&host->lock, flags);
> +
> +	/* Set CK_OUT_EN bit to 1. */
> +	config |= CORE_CK_OUT_EN;
> +	writel_relaxed(config, ioaddr + msm_offset->core_dll_config);
> +
> +	/*
> +	 * Wait until DLL_LOCK bit of DLL_STATUS register
> +	 * becomes '1'.
> +	 */
> +	while (!(readl_relaxed(ioaddr + msm_offset->core_dll_status) &
> +		 CORE_DLL_LOCK)) {
> +		/* max. wait for 50us sec for LOCK bit to be set */
> +		if (--wait_cnt == 0) {
> +			dev_err(mmc_dev(mmc), "%s: DLL failed to LOCK\n",
> +				mmc_hostname(mmc));
> +			rc = -ETIMEDOUT;
> +			goto out;
> +		}
> +		/* wait for 1us before polling again */
> +		udelay(1);
> +	}

Please use an iopoll macro like readl_relaxed_poll_timeout_atomic().

> +
> +out:
> +	if (core_vendor_spec & CORE_CLK_PWRSAVE) {
> +		/* Reenable PWRSAVE as needed */
> +		config = readl_relaxed(ioaddr + msm_offset->core_vendor_spec);
> +		config |= CORE_CLK_PWRSAVE;
> +		writel_relaxed(config, ioaddr + msm_offset->core_vendor_spec);
> +	}
> +
> +	spin_unlock_irqrestore(&host->lock, flags);
> +	return rc;
> +}
> +
>  static void msm_hc_select_default(struct sdhci_host *host)
>  {
>  	struct sdhci_pltfm_host *pltfm_host = sdhci_priv(host);
> @@ -925,14 +1140,31 @@ static void sdhci_msm_hc_select_mode(struct sdhci_host *host)
>  		msm_hc_select_default(host);
>  }
>  
> +static int sdhci_msm_init_dll(struct sdhci_host *host, enum dll_init_context init_context)
> +{
> +	if (host->mmc->ios.timing == MMC_TIMING_UHS_SDR104 ||
> +	    host->mmc->ios.timing == MMC_TIMING_MMC_HS400)
> +		return sdhci_msm_configure_dll(host, init_context, HS400);
> +
> +	return sdhci_msm_configure_dll(host, init_context, HS200);
> +}
> +
> +static int sdhci_msm_dll_config(struct sdhci_host *host, enum dll_init_context init_context)
> +{
> +	struct sdhci_pltfm_host *pltfm_host = sdhci_priv(host);
> +	struct sdhci_msm_host *msm_host = sdhci_pltfm_priv(pltfm_host);
> +
> +	return msm_host->artanis_dll ? sdhci_msm_init_dll(host, init_context) :
> +		msm_init_cm_dll(host);
> +}
> +
>  static int sdhci_msm_cdclp533_calibration(struct sdhci_host *host)
>  {
>  	struct sdhci_pltfm_host *pltfm_host = sdhci_priv(host);
>  	struct sdhci_msm_host *msm_host = sdhci_pltfm_priv(pltfm_host);
> +	const struct sdhci_msm_offset *msm_offset = msm_host->offset;
>  	u32 config, calib_done;
>  	int ret;
> -	const struct sdhci_msm_offset *msm_offset =
> -					msm_host->offset;
>  
>  	pr_debug("%s: %s: Enter\n", mmc_hostname(host->mmc), __func__);
>  
> @@ -940,7 +1172,7 @@ static int sdhci_msm_cdclp533_calibration(struct sdhci_host *host)
>  	 * Retuning in HS400 (DDR mode) will fail, just reset the
>  	 * tuning block and restore the saved tuning phase.
>  	 */
> -	ret = msm_init_cm_dll(host);
> +	ret = sdhci_msm_dll_config(host, DLL_INIT_NORMAL);
>  	if (ret)
>  		goto out;
>  
> @@ -1028,7 +1260,7 @@ static int sdhci_msm_cdclp533_calibration(struct sdhci_host *host)
>  	return ret;
>  }
>  
> -static int sdhci_msm_cm_dll_sdc4_calibration(struct sdhci_host *host)
> +static int sdhci_msm_cm_dll_sdc4_calibration(struct sdhci_host *host, enum mode index)
>  {
>  	struct mmc_host *mmc = host->mmc;
>  	u32 dll_status, config, ddr_cfg_offset;
> @@ -1051,7 +1283,11 @@ static int sdhci_msm_cm_dll_sdc4_calibration(struct sdhci_host *host)
>  		ddr_cfg_offset = msm_offset->core_ddr_config;
>  	else
>  		ddr_cfg_offset = msm_offset->core_ddr_config_old;
> -	writel_relaxed(msm_host->ddr_config, host->ioaddr + ddr_cfg_offset);
> +
> +	if (msm_host->artanis_dll)
> +		writel_relaxed(msm_host->dll[index].ddr_config, host->ioaddr + ddr_cfg_offset);
> +	else
> +		writel_relaxed(msm_host->ddr_config, host->ioaddr + ddr_cfg_offset);
>  
>  	if (mmc->ios.enhanced_strobe) {
>  		config = readl_relaxed(host->ioaddr +
> @@ -1108,11 +1344,10 @@ static int sdhci_msm_hs400_dll_calibration(struct sdhci_host *host)
>  {
>  	struct sdhci_pltfm_host *pltfm_host = sdhci_priv(host);
>  	struct sdhci_msm_host *msm_host = sdhci_pltfm_priv(pltfm_host);
> +	const struct sdhci_msm_offset *msm_offset = msm_host->offset;
>  	struct mmc_host *mmc = host->mmc;
> -	int ret;
>  	u32 config;
> -	const struct sdhci_msm_offset *msm_offset =
> -					msm_host->offset;
> +	int ret;

Here and elsewhere, this re-ordering of local definitions is a nice
improvement but it would be better as a separate patch

>  
>  	pr_debug("%s: %s: Enter\n", mmc_hostname(host->mmc), __func__);
>  
> @@ -1120,7 +1355,8 @@ static int sdhci_msm_hs400_dll_calibration(struct sdhci_host *host)
>  	 * Retuning in HS400 (DDR mode) will fail, just reset the
>  	 * tuning block and restore the saved tuning phase.
>  	 */
> -	ret = msm_init_cm_dll(host);
> +	ret = sdhci_msm_dll_config(host, DLL_INIT_NORMAL);
> +
>  	if (ret)
>  		goto out;
>  
> @@ -1140,7 +1376,7 @@ static int sdhci_msm_hs400_dll_calibration(struct sdhci_host *host)
>  	if (msm_host->use_cdclp533)
>  		ret = sdhci_msm_cdclp533_calibration(host);
>  	else
> -		ret = sdhci_msm_cm_dll_sdc4_calibration(host);
> +		ret = sdhci_msm_cm_dll_sdc4_calibration(host, HS400);
>  out:
>  	pr_debug("%s: %s: Exit, ret %d\n", mmc_hostname(host->mmc),
>  		 __func__, ret);
> @@ -1183,7 +1419,8 @@ static int sdhci_msm_restore_sdr_dll_config(struct sdhci_host *host)
>  		return 0;
>  
>  	/* Reset the tuning block */
> -	ret = msm_init_cm_dll(host);
> +	ret = sdhci_msm_dll_config(host, DLL_INIT_NORMAL);
> +
>  	if (ret)
>  		return ret;
>  
> @@ -1257,12 +1494,11 @@ static int sdhci_msm_execute_tuning(struct mmc_host *mmc, u32 opcode)
>  	if (host->flags & SDHCI_HS400_TUNING) {
>  		sdhci_msm_hc_select_mode(host);
>  		msm_set_clock_rate_for_bus_mode(host, ios.clock);
> -		host->flags &= ~SDHCI_HS400_TUNING;
>  	}
>  
>  retry:
>  	/* First of all reset the tuning block */
> -	rc = msm_init_cm_dll(host);
> +	rc = sdhci_msm_dll_config(host, DLL_INIT_NORMAL);
>  	if (rc)
>  		return rc;
>  
> @@ -1325,6 +1561,9 @@ static int sdhci_msm_execute_tuning(struct mmc_host *mmc, u32 opcode)
>  		rc = -EIO;
>  	}
>  
> +	if (host->flags & SDHCI_HS400_TUNING)
> +		host->flags &= ~SDHCI_HS400_TUNING;

Really the flag should be cleared on all return paths

> +
>  	if (!rc)
>  		msm_host->tuning_done = true;
>  	return rc;
> @@ -1845,11 +2084,6 @@ static unsigned int sdhci_msm_get_max_clock(struct sdhci_host *host)
>  	return clk_round_rate(core_clk, ULONG_MAX);
>  }
>  
> -static unsigned int sdhci_msm_get_min_clock(struct sdhci_host *host)
> -{
> -	return SDHCI_MSM_MIN_CLOCK;
> -}
> -
>  /*
>   * __sdhci_msm_set_clock - sdhci_msm clock control.
>   *

Re: [PATCH v5 1/4] dt-bindings: mmc: Add dll-presets values for HS400 and HS200 modes

[Krzysztof Kozlowski]

On Tue, Oct 14, 2025 at 02:06:46AM +0200, Krzysztof Kozlowski wrote:
> On 13/10/2025 16:53, Ram Prakash Gupta wrote:
> > From: Sachin Gupta <quic_sachgupt@quicinc.com>
> > 
> > Document the 'dll-presets' property for MMC device tree bindings.
> > The 'dll-presets' property defines the DLL configurations for HS400
> > and HS200 modes.
> > 
> > QC SoCs can have 0 to 4 SDHCI instances, and each one may need
> > different tuning.
> > 
> > Signed-off-by: Sachin Gupta <quic_sachgupt@quicinc.com>
> > Signed-off-by: Ram Prakash Gupta <quic_rampraka@quicinc.com>
> > ---
> >  Documentation/devicetree/bindings/mmc/sdhci-msm.yaml | 5 +++++
> >  1 file changed, 5 insertions(+)
> > 
> > diff --git a/Documentation/devicetree/bindings/mmc/sdhci-msm.yaml b/Documentation/devicetree/bindings/mmc/sdhci-msm.yaml
> > index 594bd174ff21..f7b3b1ced3ce 100644
> > --- a/Documentation/devicetree/bindings/mmc/sdhci-msm.yaml
> > +++ b/Documentation/devicetree/bindings/mmc/sdhci-msm.yaml
> > @@ -138,6 +138,11 @@ properties:
> >      $ref: /schemas/types.yaml#/definitions/uint32
> >      description: platform specific settings for DLL_CONFIG reg.
> >  
> > +  qcom,dll-presets:
> > +    maxItems: 10
> > +    $ref: /schemas/types.yaml#/definitions/uint32-array
> > +    description: platform specific settings for DLL registers.
> 
> One of my questions, never answered in original submission and in your
> versions, was to see the DTS user of it. I still do not see the DTS user.

There is no answer, so I mark the patch as changes requested.

Best regards,
Krzysztof

Re: [PATCH v5 4/4] mmc: sdhci-msm: Rectify DLL programming sequence for SDCC

[Ram Prakash Gupta]

On 10/16/2025 11:54 AM, Adrian Hunter wrote:
> On 13/10/2025 17:53, Ram Prakash Gupta wrote:
>> From: Sachin Gupta <quic_sachgupt@quicinc.com>
>>
>> With the current DLL sequence stability issues for data
>> transfer seen in HS400 and HS200 modes.
>>
>> "mmc0: cqhci: error IRQ status: 0x00000000 cmd error -84
>> data error 0"
>>
>> Rectify the DLL programming sequence as per latest hardware
>> programming guide
>>
>> Signed-off-by: Sachin Gupta <quic_sachgupt@quicinc.com>
>> Signed-off-by: Ram Prakash Gupta <quic_rampraka@quicinc.com>
>> ---
>>  drivers/mmc/host/sdhci-msm.c | 272 ++++++++++++++++++++++++++++++++---
>>  1 file changed, 253 insertions(+), 19 deletions(-)
>>
>> diff --git a/drivers/mmc/host/sdhci-msm.c b/drivers/mmc/host/sdhci-msm.c
>> index 8038b8def355..a875e92f8663 100644
>> --- a/drivers/mmc/host/sdhci-msm.c
>> +++ b/drivers/mmc/host/sdhci-msm.c
>> @@ -28,6 +28,7 @@
>>  #define CORE_VERSION_MAJOR_SHIFT	28
>>  #define CORE_VERSION_MAJOR_MASK		(0xf << CORE_VERSION_MAJOR_SHIFT)
>>  #define CORE_VERSION_MINOR_MASK		0xff
>> +#define SDHCI_MSM_MIN_V_7FF		0x6e
>>  
>>  #define CORE_MCI_GENERICS		0x70
>>  #define SWITCHABLE_SIGNALING_VOLTAGE	BIT(29)
>> @@ -119,7 +120,8 @@
>>  #define CORE_PWRSAVE_DLL	BIT(3)
>>  
>>  #define DDR_CONFIG_POR_VAL	0x80040873
>> -
>> +#define DLL_CONFIG_3_POR_VAL	0x10
>> +#define TCXO_FREQ               19200000
>>  
>>  #define INVALID_TUNING_PHASE	-1
>>  #define SDHCI_MSM_MIN_CLOCK	400000
>> @@ -319,6 +321,16 @@ struct sdhci_msm_host {
>>  	bool artanis_dll;
>>  };
>>  
>> +enum dll_init_context {
>> +	DLL_INIT_NORMAL,
>> +	DLL_INIT_FROM_CX_COLLAPSE_EXIT,
> DLL_INIT_FROM_CX_COLLAPSE_EXIT is never used?

This is ball parked for a collapse scenario, but that is yet to be
added, will remove this for time being in next patchset.

>
>> +};
>> +
>> +enum mode {
>> +	HS400, // equivalent to SDR104 mode for DLL.
>> +	HS200, // equivalent to SDR50 mode for DLL.
>> +};
>> +
>>  static const struct sdhci_msm_offset *sdhci_priv_msm_offset(struct sdhci_host *host)
>>  {
>>  	struct sdhci_pltfm_host *pltfm_host = sdhci_priv(host);
>> @@ -803,6 +815,209 @@ static int msm_init_cm_dll(struct sdhci_host *host)
>>  	return 0;
>>  }
>>  
>> +static unsigned int sdhci_msm_get_min_clock(struct sdhci_host *host)
>> +{
>> +	return SDHCI_MSM_MIN_CLOCK;
>> +}
>> +
>> +static unsigned int sdhci_msm_get_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);
>> +	struct clk *core_clk = msm_host->bulk_clks[0].clk;
>> +	struct mmc_ios ios = host->mmc->ios;
>> +	unsigned int sup_clk;
>> +
>> +	if (req_clk < sdhci_msm_get_min_clock(host))
>> +		return sdhci_msm_get_min_clock(host);
>> +
>> +	sup_clk = clk_get_rate(core_clk);
>> +
>> +	if (ios.timing == MMC_TIMING_MMC_HS400 ||
>> +	    host->flags & SDHCI_HS400_TUNING)
>> +		sup_clk = sup_clk / 2;
>> +
>> +	return sup_clk;
>> +}
>> +
>> +/* Initialize the DLL (Programmable Delay Line) */
>> +static int sdhci_msm_configure_dll(struct sdhci_host *host, enum dll_init_context
>> +				 init_context, enum mode index)
>> +{
>> +	struct sdhci_pltfm_host *pltfm_host = sdhci_priv(host);
>> +	struct sdhci_msm_host *msm_host = sdhci_pltfm_priv(pltfm_host);
>> +	const struct sdhci_msm_offset *msm_offset = msm_host->offset;
>> +	struct mmc_host *mmc = host->mmc;
>> +	u32 ddr_cfg_offset, core_vendor_spec, config;
>> +	void __iomem *ioaddr = host->ioaddr;
>> +	unsigned long flags, dll_clock;
>> +	int rc = 0, wait_cnt = 50;
>> +
>> +	dll_clock = sdhci_msm_get_clk_rate(host, host->clock);
>> +	spin_lock_irqsave(&host->lock, flags);
>> +
>> +	core_vendor_spec = readl_relaxed(ioaddr + msm_offset->core_vendor_spec);
>> +
>> +	/*
>> +	 * Always disable PWRSAVE during the DLL power
>> +	 * up regardless of its current setting.
>> +	 */
>> +	core_vendor_spec &= ~CORE_CLK_PWRSAVE;
>> +	writel_relaxed(core_vendor_spec, ioaddr + msm_offset->core_vendor_spec);
>> +
>> +	if (msm_host->use_14lpp_dll_reset) {
>> +		/* Disable CK_OUT */
>> +		config = readl_relaxed(ioaddr + msm_offset->core_dll_config);
>> +		config &= ~CORE_CK_OUT_EN;
>> +		writel_relaxed(config, ioaddr + msm_offset->core_dll_config);
>> +
>> +		/* Disable the DLL clock */
>> +		config = readl_relaxed(ioaddr + msm_offset->core_dll_config_2);
>> +		config |= CORE_DLL_CLOCK_DISABLE;
>> +		writel_relaxed(config, ioaddr + msm_offset->core_dll_config_2);
>> +	}
>> +
>> +	/*
>> +	 * Write 1 to DLL_RST bit of DLL_CONFIG register
>> +	 * and Write 1 to DLL_PDN bit of DLL_CONFIG register.
>> +	 */
>> +	config = readl_relaxed(ioaddr + msm_offset->core_dll_config);
>> +	config |= (CORE_DLL_RST | CORE_DLL_PDN);
>> +	writel_relaxed(config, ioaddr + msm_offset->core_dll_config);
>> +
>> +	/*
>> +	 * Configure DLL_CONFIG_3 and USER_CTRL
>> +	 * (Only applicable for 7FF projects).
>> +	 */
>> +	if (msm_host->core_minor >= SDHCI_MSM_MIN_V_7FF) {
>> +		writel_relaxed(msm_host->dll[index].dll_config_3,
>> +			       ioaddr + msm_offset->core_dll_config_3);
>> +		writel_relaxed(msm_host->dll[index].dll_usr_ctl,
>> +			       ioaddr + msm_offset->core_dll_usr_ctl);
>> +	}
>> +
>> +	/*
>> +	 * Set DDR_CONFIG since step 7 is setting TEST_CTRL that can be skipped.
>> +	 */
>> +	ddr_cfg_offset = msm_host->updated_ddr_cfg ? msm_offset->core_ddr_config
>> +					: msm_offset->core_ddr_config_old;
>> +
>> +	config = msm_host->dll[index].ddr_config;
>> +	writel_relaxed(config, ioaddr + ddr_cfg_offset);
>> +
>> +	/* Set DLL_CONFIG_2 */
>> +	if (msm_host->use_14lpp_dll_reset) {
>> +		u32 mclk_freq;
>> +		int cycle_cnt;
>> +
>> +		/*
>> +		 * Only configure the mclk_freq in normal DLL init
>> +		 * context. If the DLL init is coming from
>> +		 * CX Collapse Exit context, the host->clock may be zero.
>> +		 * The DLL_CONFIG_2 register has already been restored to
>> +		 * proper value prior to getting here.
>> +		 */
>> +		if (init_context == DLL_INIT_NORMAL) {
>> +			cycle_cnt = readl_relaxed(ioaddr +
>> +					msm_offset->core_dll_config_2)
>> +					& CORE_FLL_CYCLE_CNT ? 8 : 4;
>> +
>> +			mclk_freq = DIV_ROUND_CLOSEST_ULL(dll_clock * cycle_cnt, TCXO_FREQ);
>> +
>> +			if (dll_clock < 100000000) {
>> +				pr_err("%s: %s: Non standard clk freq =%u\n",
>> +				       mmc_hostname(mmc), __func__, dll_clock);
>> +				rc = -EINVAL;
>> +				goto out;
>> +			}
>> +
>> +			config = readl_relaxed(ioaddr + msm_offset->core_dll_config_2);
>> +			config = (config & ~GENMASK(17, 10)) |
>> +					FIELD_PREP(GENMASK(17, 10), mclk_freq);
>> +			writel_relaxed(config, ioaddr + msm_offset->core_dll_config_2);
>> +		}
>> +		/* wait for 5us before enabling DLL clock */
>> +		udelay(5);
>> +	}
>> +
>> +	config = msm_host->dll[index].dll_config;
>> +	writel_relaxed(config, ioaddr + msm_offset->core_dll_config);
>> +
>> +	/* Wait for 52us */
>> +	spin_unlock_irqrestore(&host->lock, flags);
>> +	usleep_range(60, 70);
>> +	spin_lock_irqsave(&host->lock, flags);
>> +
>> +	/*
>> +	 * Write 0 to DLL_RST bit of DLL_CONFIG register
>> +	 * and Write 0 to DLL_PDN bit of DLL_CONFIG register.
>> +	 */
>> +	config &= ~CORE_DLL_RST;
>> +	writel_relaxed(config, ioaddr + msm_offset->core_dll_config);
>> +
>> +	config &= ~CORE_DLL_PDN;
>> +	writel_relaxed(config, ioaddr + msm_offset->core_dll_config);
>> +	/* Write 1 to DLL_RST bit of DLL_CONFIG register */
>> +	config |= CORE_DLL_RST;
>> +	writel_relaxed(config, ioaddr + msm_offset->core_dll_config);
>> +
>> +	/* Write 0 to DLL_RST bit of DLL_CONFIG register */
>> +	config &= ~CORE_DLL_RST;
>> +	writel_relaxed(config, ioaddr + msm_offset->core_dll_config);
>> +
>> +	/* Set CORE_DLL_CLOCK_DISABLE to 0 */
>> +	if (msm_host->use_14lpp_dll_reset) {
>> +		config = readl_relaxed(ioaddr + msm_offset->core_dll_config_2);
>> +		config &= ~CORE_DLL_CLOCK_DISABLE;
>> +		writel_relaxed(config, ioaddr + msm_offset->core_dll_config_2);
>> +	}
>> +
>> +	/* Set DLL_EN bit to 1. */
>> +	config = readl_relaxed(ioaddr + msm_offset->core_dll_config);
>> +	config |= CORE_DLL_EN;
>> +	writel_relaxed(config, ioaddr + msm_offset->core_dll_config);
>> +
>> +	/*
>> +	 * Wait for 8000 input clock. Here we calculate the
>> +	 * delay from fixed clock freq 192MHz, which turns out 42us.
>> +	 */
>> +	spin_unlock_irqrestore(&host->lock, flags);
>> +	usleep_range(50, 60);
>> +	spin_lock_irqsave(&host->lock, flags);
>> +
>> +	/* Set CK_OUT_EN bit to 1. */
>> +	config |= CORE_CK_OUT_EN;
>> +	writel_relaxed(config, ioaddr + msm_offset->core_dll_config);
>> +
>> +	/*
>> +	 * Wait until DLL_LOCK bit of DLL_STATUS register
>> +	 * becomes '1'.
>> +	 */
>> +	while (!(readl_relaxed(ioaddr + msm_offset->core_dll_status) &
>> +		 CORE_DLL_LOCK)) {
>> +		/* max. wait for 50us sec for LOCK bit to be set */
>> +		if (--wait_cnt == 0) {
>> +			dev_err(mmc_dev(mmc), "%s: DLL failed to LOCK\n",
>> +				mmc_hostname(mmc));
>> +			rc = -ETIMEDOUT;
>> +			goto out;
>> +		}
>> +		/* wait for 1us before polling again */
>> +		udelay(1);
>> +	}
> Please use an iopoll macro like readl_relaxed_poll_timeout_atomic().

sure, thanks, will explore and update accordingly.

>
>> +
>> +out:
>> +	if (core_vendor_spec & CORE_CLK_PWRSAVE) {
>> +		/* Reenable PWRSAVE as needed */
>> +		config = readl_relaxed(ioaddr + msm_offset->core_vendor_spec);
>> +		config |= CORE_CLK_PWRSAVE;
>> +		writel_relaxed(config, ioaddr + msm_offset->core_vendor_spec);
>> +	}
>> +
>> +	spin_unlock_irqrestore(&host->lock, flags);
>> +	return rc;
>> +}
>> +
>>  static void msm_hc_select_default(struct sdhci_host *host)
>>  {
>>  	struct sdhci_pltfm_host *pltfm_host = sdhci_priv(host);
>> @@ -925,14 +1140,31 @@ static void sdhci_msm_hc_select_mode(struct sdhci_host *host)
>>  		msm_hc_select_default(host);
>>  }
>>  
>> +static int sdhci_msm_init_dll(struct sdhci_host *host, enum dll_init_context init_context)
>> +{
>> +	if (host->mmc->ios.timing == MMC_TIMING_UHS_SDR104 ||
>> +	    host->mmc->ios.timing == MMC_TIMING_MMC_HS400)
>> +		return sdhci_msm_configure_dll(host, init_context, HS400);
>> +
>> +	return sdhci_msm_configure_dll(host, init_context, HS200);
>> +}
>> +
>> +static int sdhci_msm_dll_config(struct sdhci_host *host, enum dll_init_context init_context)
>> +{
>> +	struct sdhci_pltfm_host *pltfm_host = sdhci_priv(host);
>> +	struct sdhci_msm_host *msm_host = sdhci_pltfm_priv(pltfm_host);
>> +
>> +	return msm_host->artanis_dll ? sdhci_msm_init_dll(host, init_context) :
>> +		msm_init_cm_dll(host);
>> +}
>> +
>>  static int sdhci_msm_cdclp533_calibration(struct sdhci_host *host)
>>  {
>>  	struct sdhci_pltfm_host *pltfm_host = sdhci_priv(host);
>>  	struct sdhci_msm_host *msm_host = sdhci_pltfm_priv(pltfm_host);
>> +	const struct sdhci_msm_offset *msm_offset = msm_host->offset;
>>  	u32 config, calib_done;
>>  	int ret;
>> -	const struct sdhci_msm_offset *msm_offset =
>> -					msm_host->offset;
>>  
>>  	pr_debug("%s: %s: Enter\n", mmc_hostname(host->mmc), __func__);
>>  
>> @@ -940,7 +1172,7 @@ static int sdhci_msm_cdclp533_calibration(struct sdhci_host *host)
>>  	 * Retuning in HS400 (DDR mode) will fail, just reset the
>>  	 * tuning block and restore the saved tuning phase.
>>  	 */
>> -	ret = msm_init_cm_dll(host);
>> +	ret = sdhci_msm_dll_config(host, DLL_INIT_NORMAL);
>>  	if (ret)
>>  		goto out;
>>  
>> @@ -1028,7 +1260,7 @@ static int sdhci_msm_cdclp533_calibration(struct sdhci_host *host)
>>  	return ret;
>>  }
>>  
>> -static int sdhci_msm_cm_dll_sdc4_calibration(struct sdhci_host *host)
>> +static int sdhci_msm_cm_dll_sdc4_calibration(struct sdhci_host *host, enum mode index)
>>  {
>>  	struct mmc_host *mmc = host->mmc;
>>  	u32 dll_status, config, ddr_cfg_offset;
>> @@ -1051,7 +1283,11 @@ static int sdhci_msm_cm_dll_sdc4_calibration(struct sdhci_host *host)
>>  		ddr_cfg_offset = msm_offset->core_ddr_config;
>>  	else
>>  		ddr_cfg_offset = msm_offset->core_ddr_config_old;
>> -	writel_relaxed(msm_host->ddr_config, host->ioaddr + ddr_cfg_offset);
>> +
>> +	if (msm_host->artanis_dll)
>> +		writel_relaxed(msm_host->dll[index].ddr_config, host->ioaddr + ddr_cfg_offset);
>> +	else
>> +		writel_relaxed(msm_host->ddr_config, host->ioaddr + ddr_cfg_offset);
>>  
>>  	if (mmc->ios.enhanced_strobe) {
>>  		config = readl_relaxed(host->ioaddr +
>> @@ -1108,11 +1344,10 @@ static int sdhci_msm_hs400_dll_calibration(struct sdhci_host *host)
>>  {
>>  	struct sdhci_pltfm_host *pltfm_host = sdhci_priv(host);
>>  	struct sdhci_msm_host *msm_host = sdhci_pltfm_priv(pltfm_host);
>> +	const struct sdhci_msm_offset *msm_offset = msm_host->offset;
>>  	struct mmc_host *mmc = host->mmc;
>> -	int ret;
>>  	u32 config;
>> -	const struct sdhci_msm_offset *msm_offset =
>> -					msm_host->offset;
>> +	int ret;
> Here and elsewhere, this re-ordering of local definitions is a nice
> improvement but it would be better as a separate patch

sure will update separately, will remove this change in next patchset.

>
>>  
>>  	pr_debug("%s: %s: Enter\n", mmc_hostname(host->mmc), __func__);
>>  
>> @@ -1120,7 +1355,8 @@ static int sdhci_msm_hs400_dll_calibration(struct sdhci_host *host)
>>  	 * Retuning in HS400 (DDR mode) will fail, just reset the
>>  	 * tuning block and restore the saved tuning phase.
>>  	 */
>> -	ret = msm_init_cm_dll(host);
>> +	ret = sdhci_msm_dll_config(host, DLL_INIT_NORMAL);
>> +
>>  	if (ret)
>>  		goto out;
>>  
>> @@ -1140,7 +1376,7 @@ static int sdhci_msm_hs400_dll_calibration(struct sdhci_host *host)
>>  	if (msm_host->use_cdclp533)
>>  		ret = sdhci_msm_cdclp533_calibration(host);
>>  	else
>> -		ret = sdhci_msm_cm_dll_sdc4_calibration(host);
>> +		ret = sdhci_msm_cm_dll_sdc4_calibration(host, HS400);
>>  out:
>>  	pr_debug("%s: %s: Exit, ret %d\n", mmc_hostname(host->mmc),
>>  		 __func__, ret);
>> @@ -1183,7 +1419,8 @@ static int sdhci_msm_restore_sdr_dll_config(struct sdhci_host *host)
>>  		return 0;
>>  
>>  	/* Reset the tuning block */
>> -	ret = msm_init_cm_dll(host);
>> +	ret = sdhci_msm_dll_config(host, DLL_INIT_NORMAL);
>> +
>>  	if (ret)
>>  		return ret;
>>  
>> @@ -1257,12 +1494,11 @@ static int sdhci_msm_execute_tuning(struct mmc_host *mmc, u32 opcode)
>>  	if (host->flags & SDHCI_HS400_TUNING) {
>>  		sdhci_msm_hc_select_mode(host);
>>  		msm_set_clock_rate_for_bus_mode(host, ios.clock);
>> -		host->flags &= ~SDHCI_HS400_TUNING;
>>  	}
>>  
>>  retry:
>>  	/* First of all reset the tuning block */
>> -	rc = msm_init_cm_dll(host);
>> +	rc = sdhci_msm_dll_config(host, DLL_INIT_NORMAL);
>>  	if (rc)
>>  		return rc;
>>  
>> @@ -1325,6 +1561,9 @@ static int sdhci_msm_execute_tuning(struct mmc_host *mmc, u32 opcode)
>>  		rc = -EIO;
>>  	}
>>  
>> +	if (host->flags & SDHCI_HS400_TUNING)
>> +		host->flags &= ~SDHCI_HS400_TUNING;
> Really the flag should be cleared on all return paths

Primary reason for moving this here is to have the flag set during 
sdhci_msm_dll_config(), for right clk division as per requirement,
so to keep it clear, I ll move just after sdhci_msm_dll_config()
then no other path would come into picture.

>
>> +
>>  	if (!rc)
>>  		msm_host->tuning_done = true;
>>  	return rc;
>> @@ -1845,11 +2084,6 @@ static unsigned int sdhci_msm_get_max_clock(struct sdhci_host *host)
>>  	return clk_round_rate(core_clk, ULONG_MAX);
>>  }
>>  
>> -static unsigned int sdhci_msm_get_min_clock(struct sdhci_host *host)
>> -{
>> -	return SDHCI_MSM_MIN_CLOCK;
>> -}
>> -
>>  /*
>>   * __sdhci_msm_set_clock - sdhci_msm clock control.
>>   *

Re: [PATCH v5 1/4] dt-bindings: mmc: Add dll-presets values for HS400 and HS200 modes

[Ram Prakash Gupta]

On 10/20/2025 3:39 PM, Krzysztof Kozlowski wrote:
> On Tue, Oct 14, 2025 at 02:06:46AM +0200, Krzysztof Kozlowski wrote:
>> On 13/10/2025 16:53, Ram Prakash Gupta wrote:
>>> From: Sachin Gupta <quic_sachgupt@quicinc.com>
>>>
>>> Document the 'dll-presets' property for MMC device tree bindings.
>>> The 'dll-presets' property defines the DLL configurations for HS400
>>> and HS200 modes.
>>>
>>> QC SoCs can have 0 to 4 SDHCI instances, and each one may need
>>> different tuning.
>>>
>>> Signed-off-by: Sachin Gupta <quic_sachgupt@quicinc.com>
>>> Signed-off-by: Ram Prakash Gupta <quic_rampraka@quicinc.com>
>>> ---
>>>  Documentation/devicetree/bindings/mmc/sdhci-msm.yaml | 5 +++++
>>>  1 file changed, 5 insertions(+)
>>>
>>> diff --git a/Documentation/devicetree/bindings/mmc/sdhci-msm.yaml b/Documentation/devicetree/bindings/mmc/sdhci-msm.yaml
>>> index 594bd174ff21..f7b3b1ced3ce 100644
>>> --- a/Documentation/devicetree/bindings/mmc/sdhci-msm.yaml
>>> +++ b/Documentation/devicetree/bindings/mmc/sdhci-msm.yaml
>>> @@ -138,6 +138,11 @@ properties:
>>>      $ref: /schemas/types.yaml#/definitions/uint32
>>>      description: platform specific settings for DLL_CONFIG reg.
>>>  
>>> +  qcom,dll-presets:
>>> +    maxItems: 10
>>> +    $ref: /schemas/types.yaml#/definitions/uint32-array
>>> +    description: platform specific settings for DLL registers.
>> One of my questions, never answered in original submission and in your
>> versions, was to see the DTS user of it. I still do not see the DTS user.
> There is no answer, so I mark the patch as changes requested.
>
> Best regards,
> Krzysztof

ok, I will provide this in next patchset.

Thanks,
Ram

>