| lists.openwall.net | lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC | |
|
Open Source and information security mailing list archives
| ||
|
Message-ID: <e1b61851-85e6-3897-196c-1f432e7e8432@quicinc.com>
Date: Tue, 7 Oct 2025 16:34:21 +0530
From: Ram Prakash Gupta <quic_rampraka@...cinc.com>
To: Adrian Hunter <adrian.hunter@...el.com>,
Ulf Hansson
<ulf.hansson@...aro.org>, Rob Herring <robh@...nel.org>,
Krzysztof Kozlowski
<krzk+dt@...nel.org>,
Conor Dooley <conor+dt@...nel.org>,
Bjorn Andersson
<andersson@...nel.org>,
Konrad Dybcio <konradybcio@...nel.org>
CC: <linux-mmc@...r.kernel.org>, <devicetree@...r.kernel.org>,
<linux-kernel@...r.kernel.org>, <linux-arm-msm@...r.kernel.org>,
<dmitry.baryshkov@....qualcomm.com>, <quic_pragalla@...cinc.com>,
<quic_sayalil@...cinc.com>, <quic_nitirawa@...cinc.com>,
<quic_bhaskarv@...cinc.com>, <kernel@....qualcomm.com>,
Sachin Gupta
<quic_sachgupt@...cinc.com>
Subject: Re: [PATCH v4 3/4] mmc: sdhci-msm: Add Device tree parsing logic for
DLL settings
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@...cinc.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@...cinc.com>
>> Signed-off-by: Ram Prakash Gupta <quic_rampraka@...cinc.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;
Powered by blists - more mailing lists