| Message ID | 20250122094707.24859-1-quic_sachgupt@quicinc.com |
|---|---|
| Headers | show |
| Series | mmc: sdhci-msm: Rectify DLL programming sequence for SDCC | expand |
On Wed, Jan 22, 2025 at 03:17:06PM +0530, Sachin Gupta wrote: > 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> > --- > drivers/mmc/host/sdhci-msm.c | 86 ++++++++++++++++++++++++++++++++++++ > 1 file changed, 86 insertions(+) > > diff --git a/drivers/mmc/host/sdhci-msm.c b/drivers/mmc/host/sdhci-msm.c > index 2a5e588779fc..cc7756a59c55 100644 > --- a/drivers/mmc/host/sdhci-msm.c > +++ b/drivers/mmc/host/sdhci-msm.c > @@ -256,6 +256,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 */ > @@ -264,6 +277,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 > @@ -292,6 +306,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) > @@ -2400,6 +2415,74 @@ 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 **bw_vecs, int *len) It just reads an array from the DT, please rename the bw_vecs param which is inaccurate in this case. > +{ > + struct device_node *np = dev->of_node; > + u32 *arr = NULL; > + int ret = 0; > + int sz; > + > + if (!np) > + return -ENODEV; > + > + if (!of_get_property(np, prop_name, &sz)) > + return -EINVAL; > + > + sz = sz / sizeof(*arr); > + if (sz <= 0) > + return -EINVAL; > + > + arr = devm_kzalloc(dev, 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; > + } > + > + *bw_vecs = arr; > + *len = sz; > + ret = 0; > + > + 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; > + > + 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; i < 2; i++) { > + msm_host->dll.dll_config[i] = dll_table[i]; > + msm_host->dll.dll_config_2[i] = dll_table[i + 1]; > + msm_host->dll.dll_config_3[i] = dll_table[i + 2]; > + msm_host->dll.dll_usr_ctl[i] = dll_table[i + 3]; > + msm_host->dll.ddr_config[i] = dll_table[i + 4]; > + } > + > + msm_host->artanis_dll = true; And the pointer to dll_table is lost, lingering for the driver lifetime. Please drop the devm_ part and kfree() it once it is not used anymore. > + > + return 0; > +} > + > static int sdhci_msm_probe(struct platform_device *pdev) > { > struct sdhci_host *host; > @@ -2446,6 +2529,9 @@ static int sdhci_msm_probe(struct platform_device *pdev) > > msm_host->saved_tuning_phase = INVALID_TUNING_PHASE; > > + if (sdhci_msm_dt_parse_dll_info(&pdev->dev, msm_host)) > + goto pltfm_free; > + > ret = sdhci_msm_gcc_reset(&pdev->dev, host); > if (ret) > goto pltfm_free; > -- > 2.17.1 >
On 22/01/2025 10:47, Sachin Gupta wrote: > Document the 'dll-hsr-list' property for MMC device tree bindings. > The 'dll-hsr-list' property defines the DLL configurations for HS400 > and HS200 modes. > > Signed-off-by: Sachin Gupta <quic_sachgupt@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 8b393e26e025..65dc3053df75 100644 > --- a/Documentation/devicetree/bindings/mmc/sdhci-msm.yaml > +++ b/Documentation/devicetree/bindings/mmc/sdhci-msm.yaml > @@ -133,6 +133,11 @@ properties: > $ref: /schemas/types.yaml#/definitions/uint32 > description: platform specific settings for DLL_CONFIG reg. > > + qcom,dll-hsr-list: > + maxItems: 10 > + $ref: /schemas/types.yaml#/definitions/uint32 uint32 has only one item. Anyway, there is already DLL there, so don't duplicate or explain why this is different. Explain also why this is not deducible from the compatible. Please provide here link to DTS user so we can validate how you use it. Best regards, Krzysztof
From: sachgupt <quic_sachgupt@quicinc.com> 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 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-hsr-list 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 | 362 +++++++++++++++++- 2 files changed, 349 insertions(+), 18 deletions(-)