From: Sachin Gupta <quic_sachgupt@quicinc.com>

This update introduces the capability to configure HS200
and HS400 DLL settings via the device tree and parsing it.

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

diff --git a/drivers/mmc/host/sdhci-msm.c b/drivers/mmc/host/sdhci-msm.c
index 36700735aa3e..d07f0105b733 100644
--- a/drivers/mmc/host/sdhci-msm.c
+++ b/drivers/mmc/host/sdhci-msm.c
@@ -265,6 +265,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[2];
+	u32 dll_config_2[2];
+	u32 dll_config_3[2];
+	u32 dll_usr_ctl[2];
+	u32 ddr_config[2];
+};
+
 struct sdhci_msm_host {
 	struct platform_device *pdev;
 	void __iomem *core_mem;	/* MSM SDCC mapped address */
@@ -273,6 +286,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;
 #ifdef CONFIG_MMC_CRYPTO
 	struct qcom_ice *ice;
 #endif
@@ -301,6 +315,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)
@@ -2516,6 +2531,73 @@ static int sdhci_msm_gcc_reset(struct device *dev, struct sdhci_host *host)
 	return ret;
 }
 
+static int sdhci_msm_dt_get_array(struct device *dev, const char *prop_name,
+				  u32 **dll_table, int *len)
+{
+	struct device_node *np = dev->of_node;
+	u32 *arr = NULL;
+	int ret = 0, sz = 0;
+
+	if (!np)
+		return -ENODEV;
+	if (!of_get_property(np, prop_name, &sz))
+		return -EINVAL;
+
+	sz = sz / sizeof(*arr);
+	if (sz <= 0)
+		return -EINVAL;
+
+	arr = kcalloc(sz,  sizeof(*arr), GFP_KERNEL);
+	if (!arr)
+		return -ENOMEM;
+
+	ret = of_property_read_u32_array(np, prop_name, arr, sz);
+	if (ret) {
+		dev_err(dev, "%s failed reading array %d\n", prop_name, ret);
+		*len = 0;
+		return ret;
+	}
+
+	*dll_table = arr;
+	*len = sz;
+
+	return ret;
+}
+
+static int sdhci_msm_dt_parse_dll_info(struct device *dev, struct sdhci_msm_host *msm_host)
+{
+	int dll_table_len, dll_reg_count;
+	u32 *dll_table = NULL;
+	int i, j;
+
+	msm_host->artanis_dll = false;
+
+	if (sdhci_msm_dt_get_array(dev, "qcom,dll-hsr-list",
+				   &dll_table, &dll_table_len))
+		return -EINVAL;
+
+	dll_reg_count = sizeof(struct sdhci_msm_dll) / sizeof(u32);
+
+	if (dll_table_len != dll_reg_count) {
+		dev_err(dev, "Number of HSR entries are not matching\n");
+		return -EINVAL;
+	}
+
+	for (i = 0, j = 0; j < 2; i = i + 5, j++) {
+		msm_host->dll.dll_config[j] = dll_table[i];
+		msm_host->dll.dll_config_2[j] = dll_table[i + 1];
+		msm_host->dll.dll_config_3[j] = dll_table[i + 2];
+		msm_host->dll.dll_usr_ctl[j] = dll_table[i + 3];
+		msm_host->dll.ddr_config[j] = dll_table[i + 4];
+	}
+
+	msm_host->artanis_dll = true;
+
+	kfree(dll_table);
+
+	return 0;
+}
+
 static int sdhci_msm_probe(struct platform_device *pdev)
 {
 	struct sdhci_host *host;
@@ -2562,6 +2644,15 @@ static int sdhci_msm_probe(struct platform_device *pdev)
 
 	msm_host->saved_tuning_phase = INVALID_TUNING_PHASE;
 
+	/*
+	 * Parse HSR dll only when property is present in DT.
+	 */
+	if (of_find_property(node, "qcom,dll-hsr-list", NULL)) {
+		ret = sdhci_msm_dt_parse_dll_info(&pdev->dev, msm_host);
+		if (ret)
+			return ret;
+	}
+
 	ret = sdhci_msm_gcc_reset(&pdev->dev, host);
 	if (ret)
 		return ret;
-- 
2.34.1

On Mon, Sep 29, 2025 at 05:05:14PM +0530, Ram Prakash Gupta wrote:
> From: Sachin Gupta <quic_sachgupt@quicinc.com>
> 
> This update introduces the capability to configure HS200
> and HS400 DLL settings via the device tree and parsing it.
> 
> 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 | 91 ++++++++++++++++++++++++++++++++++++
>  1 file changed, 91 insertions(+)
> 
> diff --git a/drivers/mmc/host/sdhci-msm.c b/drivers/mmc/host/sdhci-msm.c
> index 36700735aa3e..d07f0105b733 100644
> --- a/drivers/mmc/host/sdhci-msm.c
> +++ b/drivers/mmc/host/sdhci-msm.c
> @@ -265,6 +265,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[2];
> +	u32 dll_config_2[2];
> +	u32 dll_config_3[2];
> +	u32 dll_usr_ctl[2];
> +	u32 ddr_config[2];
> +};
> +
>  struct sdhci_msm_host {
>  	struct platform_device *pdev;
>  	void __iomem *core_mem;	/* MSM SDCC mapped address */
> @@ -273,6 +286,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;
>  #ifdef CONFIG_MMC_CRYPTO
>  	struct qcom_ice *ice;
>  #endif
> @@ -301,6 +315,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)
> @@ -2516,6 +2531,73 @@ static int sdhci_msm_gcc_reset(struct device *dev, struct sdhci_host *host)
>  	return ret;
>  }
>  
> +static int sdhci_msm_dt_get_array(struct device *dev, const char *prop_name,
> +				  u32 **dll_table, int *len)
> +{
> +	struct device_node *np = dev->of_node;
> +	u32 *arr = NULL;
> +	int ret = 0, sz = 0;
> +
> +	if (!np)
> +		return -ENODEV;
> +	if (!of_get_property(np, prop_name, &sz))

Don't add new users of of_get_property which adds untracked pointers to 
raw DT data.

> +		return -EINVAL;
> +
> +	sz = sz / sizeof(*arr);
> +	if (sz <= 0)
> +		return -EINVAL;
> +
> +	arr = kcalloc(sz,  sizeof(*arr), GFP_KERNEL);

Why do you need to count the length first when only 5 entries is valid?

> +	if (!arr)
> +		return -ENOMEM;
> +
> +	ret = of_property_read_u32_array(np, prop_name, arr, sz);
> +	if (ret) {
> +		dev_err(dev, "%s failed reading array %d\n", prop_name, ret);
> +		*len = 0;
> +		return ret;
> +	}
> +
> +	*dll_table = arr;
> +	*len = sz;
> +
> +	return ret;
> +}
> +
> +static int sdhci_msm_dt_parse_dll_info(struct device *dev, struct sdhci_msm_host *msm_host)
> +{
> +	int dll_table_len, dll_reg_count;
> +	u32 *dll_table = NULL;
> +	int i, j;
> +
> +	msm_host->artanis_dll = false;
> +
> +	if (sdhci_msm_dt_get_array(dev, "qcom,dll-hsr-list",
> +				   &dll_table, &dll_table_len))
> +		return -EINVAL;
> +
> +	dll_reg_count = sizeof(struct sdhci_msm_dll) / sizeof(u32);
> +
> +	if (dll_table_len != dll_reg_count) {
> +		dev_err(dev, "Number of HSR entries are not matching\n");
> +		return -EINVAL;

You just leaked memory. devm_* functions are your friend.

> +	}
> +
> +	for (i = 0, j = 0; j < 2; i = i + 5, j++) {
> +		msm_host->dll.dll_config[j] = dll_table[i];
> +		msm_host->dll.dll_config_2[j] = dll_table[i + 1];
> +		msm_host->dll.dll_config_3[j] = dll_table[i + 2];
> +		msm_host->dll.dll_usr_ctl[j] = dll_table[i + 3];
> +		msm_host->dll.ddr_config[j] = dll_table[i + 4];
> +	}
> +
> +	msm_host->artanis_dll = true;
> +
> +	kfree(dll_table);
> +
> +	return 0;
> +}

On 10/7/2025 3:14 AM, Rob Herring wrote:
> On Mon, Sep 29, 2025 at 05:05:14PM +0530, Ram Prakash Gupta wrote:
>> From: Sachin Gupta <quic_sachgupt@quicinc.com>
>>
>> This update introduces the capability to configure HS200
>> and HS400 DLL settings via the device tree and parsing it.
>>
>> 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 | 91 ++++++++++++++++++++++++++++++++++++
>>  1 file changed, 91 insertions(+)
>>
>> diff --git a/drivers/mmc/host/sdhci-msm.c b/drivers/mmc/host/sdhci-msm.c
>> index 36700735aa3e..d07f0105b733 100644
>> --- a/drivers/mmc/host/sdhci-msm.c
>> +++ b/drivers/mmc/host/sdhci-msm.c
>> @@ -265,6 +265,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[2];
>> +	u32 dll_config_2[2];
>> +	u32 dll_config_3[2];
>> +	u32 dll_usr_ctl[2];
>> +	u32 ddr_config[2];
>> +};
>> +
>>  struct sdhci_msm_host {
>>  	struct platform_device *pdev;
>>  	void __iomem *core_mem;	/* MSM SDCC mapped address */
>> @@ -273,6 +286,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;
>>  #ifdef CONFIG_MMC_CRYPTO
>>  	struct qcom_ice *ice;
>>  #endif
>> @@ -301,6 +315,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)
>> @@ -2516,6 +2531,73 @@ static int sdhci_msm_gcc_reset(struct device *dev, struct sdhci_host *host)
>>  	return ret;
>>  }
>>  
>> +static int sdhci_msm_dt_get_array(struct device *dev, const char *prop_name,
>> +				  u32 **dll_table, int *len)
>> +{
>> +	struct device_node *np = dev->of_node;
>> +	u32 *arr = NULL;
>> +	int ret = 0, sz = 0;
>> +
>> +	if (!np)
>> +		return -ENODEV;
>> +	if (!of_get_property(np, prop_name, &sz))
> Don't add new users of of_get_property which adds untracked pointers to 
> raw DT data.

Adrian also checked and provided some suggestion, and with those I could
get rid of this function, so it is no longer needed.

>
>> +		return -EINVAL;
>> +
>> +	sz = sz / sizeof(*arr);
>> +	if (sz <= 0)
>> +		return -EINVAL;
>> +
>> +	arr = kcalloc(sz,  sizeof(*arr), GFP_KERNEL);
> Why do you need to count the length first when only 5 entries is valid?

This line also would not be needed in needed in next patchset.

>> +	if (!arr)
>> +		return -ENOMEM;
>> +
>> +	ret = of_property_read_u32_array(np, prop_name, arr, sz);
>> +	if (ret) {
>> +		dev_err(dev, "%s failed reading array %d\n", prop_name, ret);
>> +		*len = 0;
>> +		return ret;
>> +	}
>> +
>> +	*dll_table = arr;
>> +	*len = sz;
>> +
>> +	return ret;
>> +}
>> +
>> +static int sdhci_msm_dt_parse_dll_info(struct device *dev, struct sdhci_msm_host *msm_host)
>> +{
>> +	int dll_table_len, dll_reg_count;
>> +	u32 *dll_table = NULL;
>> +	int i, j;
>> +
>> +	msm_host->artanis_dll = false;
>> +
>> +	if (sdhci_msm_dt_get_array(dev, "qcom,dll-hsr-list",
>> +				   &dll_table, &dll_table_len))
>> +		return -EINVAL;
>> +
>> +	dll_reg_count = sizeof(struct sdhci_msm_dll) / sizeof(u32);
>> +
>> +	if (dll_table_len != dll_reg_count) {
>> +		dev_err(dev, "Number of HSR entries are not matching\n");
>> +		return -EINVAL;
> You just leaked memory. devm_* functions are your friend.

These changes would also be removed in next patchset, after addressing
Adrian comment.

>
>> +	}
>> +
>> +	for (i = 0, j = 0; j < 2; i = i + 5, j++) {
>> +		msm_host->dll.dll_config[j] = dll_table[i];
>> +		msm_host->dll.dll_config_2[j] = dll_table[i + 1];
>> +		msm_host->dll.dll_config_3[j] = dll_table[i + 2];
>> +		msm_host->dll.dll_usr_ctl[j] = dll_table[i + 3];
>> +		msm_host->dll.ddr_config[j] = dll_table[i + 4];
>> +	}
>> +
>> +	msm_host->artanis_dll = true;
>> +
>> +	kfree(dll_table);
>> +
>> +	return 0;
>> +}

On 29/09/2025 14:35, Ram Prakash Gupta wrote:
> From: Sachin Gupta <quic_sachgupt@quicinc.com>
> 
> This update introduces the capability to configure HS200
> and HS400 DLL settings via the device tree and parsing it.
> 
> 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 | 91 ++++++++++++++++++++++++++++++++++++
>  1 file changed, 91 insertions(+)
> 
> diff --git a/drivers/mmc/host/sdhci-msm.c b/drivers/mmc/host/sdhci-msm.c
> index 36700735aa3e..d07f0105b733 100644
> --- a/drivers/mmc/host/sdhci-msm.c
> +++ b/drivers/mmc/host/sdhci-msm.c
> @@ -265,6 +265,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[2];
> +	u32 dll_config_2[2];
> +	u32 dll_config_3[2];
> +	u32 dll_usr_ctl[2];
> +	u32 ddr_config[2];
> +};
> +
>  struct sdhci_msm_host {
>  	struct platform_device *pdev;
>  	void __iomem *core_mem;	/* MSM SDCC mapped address */
> @@ -273,6 +286,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;
>  #ifdef CONFIG_MMC_CRYPTO
>  	struct qcom_ice *ice;
>  #endif
> @@ -301,6 +315,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)
> @@ -2516,6 +2531,73 @@ static int sdhci_msm_gcc_reset(struct device *dev, struct sdhci_host *host)
>  	return ret;
>  }
>  
> +static int sdhci_msm_dt_get_array(struct device *dev, const char *prop_name,
> +				  u32 **dll_table, int *len)
> +{
> +	struct device_node *np = dev->of_node;
> +	u32 *arr = NULL;
> +	int ret = 0, sz = 0;
> +
> +	if (!np)
> +		return -ENODEV;
> +	if (!of_get_property(np, prop_name, &sz))
> +		return -EINVAL;
> +
> +	sz = sz / sizeof(*arr);
> +	if (sz <= 0)
> +		return -EINVAL;
> +
> +	arr = kcalloc(sz,  sizeof(*arr), GFP_KERNEL);
> +	if (!arr)
> +		return -ENOMEM;
> +
> +	ret = of_property_read_u32_array(np, prop_name, arr, sz);
> +	if (ret) {
> +		dev_err(dev, "%s failed reading array %d\n", prop_name, ret);
> +		*len = 0;
> +		return ret;
> +	}
> +
> +	*dll_table = arr;
> +	*len = sz;
> +
> +	return ret;
> +}
> +
> +static int sdhci_msm_dt_parse_dll_info(struct device *dev, struct sdhci_msm_host *msm_host)
> +{
> +	int dll_table_len, dll_reg_count;
> +	u32 *dll_table = NULL;
> +	int i, j;
> +
> +	msm_host->artanis_dll = false;
> +
> +	if (sdhci_msm_dt_get_array(dev, "qcom,dll-hsr-list",
> +				   &dll_table, &dll_table_len))
> +		return -EINVAL;
> +
> +	dll_reg_count = sizeof(struct sdhci_msm_dll) / sizeof(u32);
> +
> +	if (dll_table_len != dll_reg_count) {
> +		dev_err(dev, "Number of HSR entries are not matching\n");
> +		return -EINVAL;
> +	}
> +
> +	for (i = 0, j = 0; j < 2; i = i + 5, j++) {
> +		msm_host->dll.dll_config[j] = dll_table[i];
> +		msm_host->dll.dll_config_2[j] = dll_table[i + 1];
> +		msm_host->dll.dll_config_3[j] = dll_table[i + 2];
> +		msm_host->dll.dll_usr_ctl[j] = dll_table[i + 3];
> +		msm_host->dll.ddr_config[j] = dll_table[i + 4];
> +	}

Kind of begs the question, why the driver and the DT have to be in
a different order.

It might be simpler to have:

	struct sdhci_msm_dll {
		u32 dll_config;
		u32 dll_config_2;
		u32 dll_config_3;
		u32 dll_usr_ctl;
		u32 ddr_config;
	};

And:
	struct sdhci_msm_dll dll[2];

And then dereference like:

	msm_host->dll[index].dll_config_3

Also then you could perhaps use something like:

	of_property_read_variable_u32_array(np, "qcom,dll-hsr-list", msm_host->dll, 10, 10)

instead of most of sdhci_msm_dt_get_array()

> +
> +	msm_host->artanis_dll = true;
> +
> +	kfree(dll_table);
> +
> +	return 0;
> +}
> +
>  static int sdhci_msm_probe(struct platform_device *pdev)
>  {
>  	struct sdhci_host *host;
> @@ -2562,6 +2644,15 @@ static int sdhci_msm_probe(struct platform_device *pdev)
>  
>  	msm_host->saved_tuning_phase = INVALID_TUNING_PHASE;
>  
> +	/*
> +	 * Parse HSR dll only when property is present in DT.
> +	 */
> +	if (of_find_property(node, "qcom,dll-hsr-list", NULL)) {
> +		ret = sdhci_msm_dt_parse_dll_info(&pdev->dev, msm_host);
> +		if (ret)
> +			return ret;
> +	}
> +
>  	ret = sdhci_msm_gcc_reset(&pdev->dev, host);
>  	if (ret)
>  		return ret;

On 9/29/2025 9:42 PM, Adrian Hunter wrote:
> On 29/09/2025 14:35, Ram Prakash Gupta wrote:
>> From: Sachin Gupta <quic_sachgupt@quicinc.com>
>>
>> This update introduces the capability to configure HS200
>> and HS400 DLL settings via the device tree and parsing it.
>>
>> 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 | 91 ++++++++++++++++++++++++++++++++++++
>>  1 file changed, 91 insertions(+)
>>
>> diff --git a/drivers/mmc/host/sdhci-msm.c b/drivers/mmc/host/sdhci-msm.c
>> index 36700735aa3e..d07f0105b733 100644
>> --- a/drivers/mmc/host/sdhci-msm.c
>> +++ b/drivers/mmc/host/sdhci-msm.c
>> @@ -265,6 +265,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[2];
>> +	u32 dll_config_2[2];
>> +	u32 dll_config_3[2];
>> +	u32 dll_usr_ctl[2];
>> +	u32 ddr_config[2];
>> +};
>> +
>>  struct sdhci_msm_host {
>>  	struct platform_device *pdev;
>>  	void __iomem *core_mem;	/* MSM SDCC mapped address */
>> @@ -273,6 +286,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;
>>  #ifdef CONFIG_MMC_CRYPTO
>>  	struct qcom_ice *ice;
>>  #endif
>> @@ -301,6 +315,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)
>> @@ -2516,6 +2531,73 @@ static int sdhci_msm_gcc_reset(struct device *dev, struct sdhci_host *host)
>>  	return ret;
>>  }
>>  
>> +static int sdhci_msm_dt_get_array(struct device *dev, const char *prop_name,
>> +				  u32 **dll_table, int *len)
>> +{
>> +	struct device_node *np = dev->of_node;
>> +	u32 *arr = NULL;
>> +	int ret = 0, sz = 0;
>> +
>> +	if (!np)
>> +		return -ENODEV;
>> +	if (!of_get_property(np, prop_name, &sz))
>> +		return -EINVAL;
>> +
>> +	sz = sz / sizeof(*arr);
>> +	if (sz <= 0)
>> +		return -EINVAL;
>> +
>> +	arr = kcalloc(sz,  sizeof(*arr), GFP_KERNEL);
>> +	if (!arr)
>> +		return -ENOMEM;
>> +
>> +	ret = of_property_read_u32_array(np, prop_name, arr, sz);
>> +	if (ret) {
>> +		dev_err(dev, "%s failed reading array %d\n", prop_name, ret);
>> +		*len = 0;
>> +		return ret;
>> +	}
>> +
>> +	*dll_table = arr;
>> +	*len = sz;
>> +
>> +	return ret;
>> +}
>> +
>> +static int sdhci_msm_dt_parse_dll_info(struct device *dev, struct sdhci_msm_host *msm_host)
>> +{
>> +	int dll_table_len, dll_reg_count;
>> +	u32 *dll_table = NULL;
>> +	int i, j;
>> +
>> +	msm_host->artanis_dll = false;
>> +
>> +	if (sdhci_msm_dt_get_array(dev, "qcom,dll-hsr-list",
>> +				   &dll_table, &dll_table_len))
>> +		return -EINVAL;
>> +
>> +	dll_reg_count = sizeof(struct sdhci_msm_dll) / sizeof(u32);
>> +
>> +	if (dll_table_len != dll_reg_count) {
>> +		dev_err(dev, "Number of HSR entries are not matching\n");
>> +		return -EINVAL;
>> +	}
>> +
>> +	for (i = 0, j = 0; j < 2; i = i + 5, j++) {
>> +		msm_host->dll.dll_config[j] = dll_table[i];
>> +		msm_host->dll.dll_config_2[j] = dll_table[i + 1];
>> +		msm_host->dll.dll_config_3[j] = dll_table[i + 2];
>> +		msm_host->dll.dll_usr_ctl[j] = dll_table[i + 3];
>> +		msm_host->dll.ddr_config[j] = dll_table[i + 4];
>> +	}
> Kind of begs the question, why the driver and the DT have to be in
> a different order.
>
> It might be simpler to have:
>
> 	struct sdhci_msm_dll {
> 		u32 dll_config;
> 		u32 dll_config_2;
> 		u32 dll_config_3;
> 		u32 dll_usr_ctl;
> 		u32 ddr_config;
> 	};
>
> And:
> 	struct sdhci_msm_dll dll[2];
>
> And then dereference like:
>
> 	msm_host->dll[index].dll_config_3
>
> Also then you could perhaps use something like:
>
> 	of_property_read_variable_u32_array(np, "qcom,dll-hsr-list", msm_host->dll, 10, 10)
>
> instead of most of sdhci_msm_dt_get_array()

Thanks Adrian for your suggestion, I could get rid of most of the lines of code
of this patch with this approach. Accordingly I had to make some changes in
patch #4 of this version.

please let me know if I can push next patchset version.

>> +
>> +	msm_host->artanis_dll = true;
>> +
>> +	kfree(dll_table);
>> +
>> +	return 0;
>> +}
>> +
>>  static int sdhci_msm_probe(struct platform_device *pdev)
>>  {
>>  	struct sdhci_host *host;
>> @@ -2562,6 +2644,15 @@ static int sdhci_msm_probe(struct platform_device *pdev)
>>  
>>  	msm_host->saved_tuning_phase = INVALID_TUNING_PHASE;
>>  
>> +	/*
>> +	 * Parse HSR dll only when property is present in DT.
>> +	 */
>> +	if (of_find_property(node, "qcom,dll-hsr-list", NULL)) {
>> +		ret = sdhci_msm_dt_parse_dll_info(&pdev->dev, msm_host);
>> +		if (ret)
>> +			return ret;
>> +	}
>> +
>>  	ret = sdhci_msm_gcc_reset(&pdev->dev, host);
>>  	if (ret)
>>  		return ret;

On 07/10/2025 14:04, Ram Prakash Gupta wrote:
> please let me know if I can push next patchset version.

That is up to you.  Whenever you are ready, review comments addressed etc