[PATCH 0/7] qcom: mmc fixes and sdm845 support

This series does some long overdue cleanup to the msm_sdhci driver, fixes v5 support, and adds the necessary clock configuration to get the sdcard up and running on the RB3.
--- Caleb Connolly (7): mmc: msm_sdhci: correct vendor_spec_cap0 register for v5 mmc: msm_sdhci: use modern DT handling mmc: msm_sdhci: print core version mmc: msm_sdhci: use a more sensible default clock rate clk/qcom: sdm845: enable SDCC2 core clock pinctrl: qcom: sdm845: add special pin names dts: sdm845-db845c-u-boot: adjust MMC clocks
arch/arm/dts/sdm845-db845c-u-boot.dtsi | 7 ++++++ drivers/clk/qcom/clock-qcom.h | 1 + drivers/clk/qcom/clock-sdm845.c | 17 ++++++++++++++ drivers/mmc/msm_sdhci.c | 43 +++++++++++++++++++++++----------- drivers/pinctrl/qcom/pinctrl-sdm845.c | 13 +++++++++- 5 files changed, 66 insertions(+), 15 deletions(-) --- change-id: 20240409-b4-qcom-mmc-fixes-d30782746f4a base-commit: b40edaf6383b1494222ea9c043d7c1716d4d118f
// Caleb (they/them)

The V4 and V5 controllers have quite varied register layouts. Inherit the register offsets and naming from the Linux driver. More version specific offsets can be inherited from Linux as needed.
Fixes: 364c22a ("mmc: msm_sdhci: Add SDCC version 5.0.0 support") Signed-off-by: Caleb Connolly caleb.connolly@linaro.org --- drivers/mmc/msm_sdhci.c | 11 +++++++---- 1 file changed, 7 insertions(+), 4 deletions(-)
diff --git a/drivers/mmc/msm_sdhci.c b/drivers/mmc/msm_sdhci.c index 059cb3da77c5..f23d425144ef 100644 --- a/drivers/mmc/msm_sdhci.c +++ b/drivers/mmc/msm_sdhci.c @@ -32,11 +32,8 @@ #define SDCC_MCI_STATUS2 0x6C #define SDCC_MCI_STATUS2_MCI_ACT 0x1 #define SDCC_MCI_HC_MODE 0x78
-/* Non standard (?) SDHCI register */ -#define SDHCI_VENDOR_SPEC_CAPABILITIES0 0x11c - struct msm_sdhc_plat { struct mmc_config cfg; struct mmc mmc; }; @@ -48,8 +45,10 @@ struct msm_sdhc { };
struct msm_sdhc_variant_info { bool mci_removed; + + u32 core_vendor_spec_capabilities0; };
DECLARE_GLOBAL_DATA_PTR;
@@ -180,9 +179,9 @@ static int msm_sdc_probe(struct udevice *dev) */ if (core_major >= 1 && core_minor != 0x11 && core_minor != 0x12) { caps = readl(host->ioaddr + SDHCI_CAPABILITIES); caps |= SDHCI_CAN_VDD_300 | SDHCI_CAN_DO_8BIT; - writel(caps, host->ioaddr + SDHCI_VENDOR_SPEC_CAPABILITIES0); + writel(caps, host->ioaddr + var_info->core_vendor_spec_capabilities0); }
ret = mmc_of_parse(dev, &plat->cfg); if (ret) @@ -243,12 +242,16 @@ static int msm_sdc_bind(struct udevice *dev) }
static const struct msm_sdhc_variant_info msm_sdhc_mci_var = { .mci_removed = false, + + .core_vendor_spec_capabilities0 = 0x21c, };
static const struct msm_sdhc_variant_info msm_sdhc_v5_var = { .mci_removed = true, + + .core_vendor_spec_capabilities0 = 0x11c, };
static const struct udevice_id msm_mmc_ids[] = { { .compatible = "qcom,sdhci-msm-v4", .data = (ulong)&msm_sdhc_mci_var },

On 09/04/2024 20:03, Caleb Connolly wrote:
The V4 and V5 controllers have quite varied register layouts. Inherit the register offsets and naming from the Linux driver. More version specific offsets can be inherited from Linux as needed.
Fixes: 364c22a ("mmc: msm_sdhci: Add SDCC version 5.0.0 support") Signed-off-by: Caleb Connolly caleb.connolly@linaro.org
drivers/mmc/msm_sdhci.c | 11 +++++++---- 1 file changed, 7 insertions(+), 4 deletions(-)
diff --git a/drivers/mmc/msm_sdhci.c b/drivers/mmc/msm_sdhci.c index 059cb3da77c5..f23d425144ef 100644 --- a/drivers/mmc/msm_sdhci.c +++ b/drivers/mmc/msm_sdhci.c @@ -32,11 +32,8 @@ #define SDCC_MCI_STATUS2 0x6C #define SDCC_MCI_STATUS2_MCI_ACT 0x1 #define SDCC_MCI_HC_MODE 0x78
-/* Non standard (?) SDHCI register */ -#define SDHCI_VENDOR_SPEC_CAPABILITIES0 0x11c
- struct msm_sdhc_plat { struct mmc_config cfg; struct mmc mmc; };
@@ -48,8 +45,10 @@ struct msm_sdhc { };
struct msm_sdhc_variant_info { bool mci_removed;
u32 core_vendor_spec_capabilities0; };
DECLARE_GLOBAL_DATA_PTR;
@@ -180,9 +179,9 @@ static int msm_sdc_probe(struct udevice *dev) */ if (core_major >= 1 && core_minor != 0x11 && core_minor != 0x12) { caps = readl(host->ioaddr + SDHCI_CAPABILITIES); caps |= SDHCI_CAN_VDD_300 | SDHCI_CAN_DO_8BIT;
writel(caps, host->ioaddr + SDHCI_VENDOR_SPEC_CAPABILITIES0);
writel(caps, host->ioaddr + var_info->core_vendor_spec_capabilities0);
}
ret = mmc_of_parse(dev, &plat->cfg); if (ret)
@@ -243,12 +242,16 @@ static int msm_sdc_bind(struct udevice *dev) }
static const struct msm_sdhc_variant_info msm_sdhc_mci_var = { .mci_removed = false,
.core_vendor_spec_capabilities0 = 0x21c, };
static const struct msm_sdhc_variant_info msm_sdhc_v5_var = { .mci_removed = true,
.core_vendor_spec_capabilities0 = 0x11c, };
static const struct udevice_id msm_mmc_ids[] = { { .compatible = "qcom,sdhci-msm-v4", .data = (ulong)&msm_sdhc_mci_var },
Reviewed-by: Neil Armstrong neil.armstrong@linaro.org

On Tue, 9 Apr 2024 at 23:33, Caleb Connolly caleb.connolly@linaro.org wrote:
The V4 and V5 controllers have quite varied register layouts. Inherit the register offsets and naming from the Linux driver. More version specific offsets can be inherited from Linux as needed.
Fixes: 364c22a ("mmc: msm_sdhci: Add SDCC version 5.0.0 support") Signed-off-by: Caleb Connolly caleb.connolly@linaro.org
drivers/mmc/msm_sdhci.c | 11 +++++++---- 1 file changed, 7 insertions(+), 4 deletions(-)
This patch broke booting on the HMIBSC board, have you tested it on db410c? It's very likely that this has caused regression there too.
Error observed:
sdhci_send_command: Timeout for status update: 00000000 00000001
-Sumit
diff --git a/drivers/mmc/msm_sdhci.c b/drivers/mmc/msm_sdhci.c index 059cb3da77c5..f23d425144ef 100644 --- a/drivers/mmc/msm_sdhci.c +++ b/drivers/mmc/msm_sdhci.c @@ -32,11 +32,8 @@ #define SDCC_MCI_STATUS2 0x6C #define SDCC_MCI_STATUS2_MCI_ACT 0x1 #define SDCC_MCI_HC_MODE 0x78
-/* Non standard (?) SDHCI register */ -#define SDHCI_VENDOR_SPEC_CAPABILITIES0 0x11c
struct msm_sdhc_plat { struct mmc_config cfg; struct mmc mmc; }; @@ -48,8 +45,10 @@ struct msm_sdhc { };
struct msm_sdhc_variant_info { bool mci_removed;
u32 core_vendor_spec_capabilities0;
};
DECLARE_GLOBAL_DATA_PTR;
@@ -180,9 +179,9 @@ static int msm_sdc_probe(struct udevice *dev) */ if (core_major >= 1 && core_minor != 0x11 && core_minor != 0x12) { caps = readl(host->ioaddr + SDHCI_CAPABILITIES); caps |= SDHCI_CAN_VDD_300 | SDHCI_CAN_DO_8BIT;
writel(caps, host->ioaddr + SDHCI_VENDOR_SPEC_CAPABILITIES0);
writel(caps, host->ioaddr + var_info->core_vendor_spec_capabilities0); } ret = mmc_of_parse(dev, &plat->cfg); if (ret)
@@ -243,12 +242,16 @@ static int msm_sdc_bind(struct udevice *dev) }
static const struct msm_sdhc_variant_info msm_sdhc_mci_var = { .mci_removed = false,
.core_vendor_spec_capabilities0 = 0x21c,
};
static const struct msm_sdhc_variant_info msm_sdhc_v5_var = { .mci_removed = true,
.core_vendor_spec_capabilities0 = 0x11c,
};
static const struct udevice_id msm_mmc_ids[] = { { .compatible = "qcom,sdhci-msm-v4", .data = (ulong)&msm_sdhc_mci_var },
-- 2.44.0

On 11/04/2024 15:59, Sumit Garg wrote:
On Tue, 9 Apr 2024 at 23:33, Caleb Connolly caleb.connolly@linaro.org wrote:
The V4 and V5 controllers have quite varied register layouts. Inherit the register offsets and naming from the Linux driver. More version specific offsets can be inherited from Linux as needed.
Fixes: 364c22a ("mmc: msm_sdhci: Add SDCC version 5.0.0 support") Signed-off-by: Caleb Connolly caleb.connolly@linaro.org
drivers/mmc/msm_sdhci.c | 11 +++++++---- 1 file changed, 7 insertions(+), 4 deletions(-)
This patch broke booting on the HMIBSC board, have you tested it on db410c? It's very likely that this has caused regression there too.
Error observed:
sdhci_send_command: Timeout for status update: 00000000 00000001
Indeed swapping the core_vendor_spec_capabilities0 between msm_sdhc_v5_var & msm_sdhc_mci_var fixes this and I'm now able to enable SDCard on the SM8550-HDK
Neil
-Sumit
diff --git a/drivers/mmc/msm_sdhci.c b/drivers/mmc/msm_sdhci.c index 059cb3da77c5..f23d425144ef 100644 --- a/drivers/mmc/msm_sdhci.c +++ b/drivers/mmc/msm_sdhci.c @@ -32,11 +32,8 @@ #define SDCC_MCI_STATUS2 0x6C #define SDCC_MCI_STATUS2_MCI_ACT 0x1 #define SDCC_MCI_HC_MODE 0x78
-/* Non standard (?) SDHCI register */ -#define SDHCI_VENDOR_SPEC_CAPABILITIES0 0x11c
- struct msm_sdhc_plat { struct mmc_config cfg; struct mmc mmc; };
@@ -48,8 +45,10 @@ struct msm_sdhc { };
struct msm_sdhc_variant_info { bool mci_removed;
u32 core_vendor_spec_capabilities0;
};
DECLARE_GLOBAL_DATA_PTR;
@@ -180,9 +179,9 @@ static int msm_sdc_probe(struct udevice *dev) */ if (core_major >= 1 && core_minor != 0x11 && core_minor != 0x12) { caps = readl(host->ioaddr + SDHCI_CAPABILITIES); caps |= SDHCI_CAN_VDD_300 | SDHCI_CAN_DO_8BIT;
writel(caps, host->ioaddr + SDHCI_VENDOR_SPEC_CAPABILITIES0);
writel(caps, host->ioaddr + var_info->core_vendor_spec_capabilities0); } ret = mmc_of_parse(dev, &plat->cfg); if (ret)
@@ -243,12 +242,16 @@ static int msm_sdc_bind(struct udevice *dev) }
static const struct msm_sdhc_variant_info msm_sdhc_mci_var = { .mci_removed = false,
.core_vendor_spec_capabilities0 = 0x21c,
};
static const struct msm_sdhc_variant_info msm_sdhc_v5_var = { .mci_removed = true,
.core_vendor_spec_capabilities0 = 0x11c,
};
static const struct udevice_id msm_mmc_ids[] = { { .compatible = "qcom,sdhci-msm-v4", .data = (ulong)&msm_sdhc_mci_var },
-- 2.44.0

On Fri, 12 Apr 2024 at 14:06, Neil Armstrong neil.armstrong@linaro.org wrote:
On 11/04/2024 15:59, Sumit Garg wrote:
On Tue, 9 Apr 2024 at 23:33, Caleb Connolly caleb.connolly@linaro.org wrote:
The V4 and V5 controllers have quite varied register layouts. Inherit the register offsets and naming from the Linux driver. More version specific offsets can be inherited from Linux as needed.
Fixes: 364c22a ("mmc: msm_sdhci: Add SDCC version 5.0.0 support") Signed-off-by: Caleb Connolly caleb.connolly@linaro.org
drivers/mmc/msm_sdhci.c | 11 +++++++---- 1 file changed, 7 insertions(+), 4 deletions(-)
This patch broke booting on the HMIBSC board, have you tested it on db410c? It's very likely that this has caused regression there too.
Error observed:
sdhci_send_command: Timeout for status update: 00000000 00000001
Indeed swapping the core_vendor_spec_capabilities0 between msm_sdhc_v5_var & msm_sdhc_mci_var fixes this and I'm now able to enable SDCard on the SM8550-HDK
Yeah this fixed the problem for me too. I am unsure how it worked for Caleb on db845c.
Caleb,
Can you fix up this patch which is already in your tree already?
-Sumit
Neil
-Sumit
diff --git a/drivers/mmc/msm_sdhci.c b/drivers/mmc/msm_sdhci.c index 059cb3da77c5..f23d425144ef 100644 --- a/drivers/mmc/msm_sdhci.c +++ b/drivers/mmc/msm_sdhci.c @@ -32,11 +32,8 @@ #define SDCC_MCI_STATUS2 0x6C #define SDCC_MCI_STATUS2_MCI_ACT 0x1 #define SDCC_MCI_HC_MODE 0x78
-/* Non standard (?) SDHCI register */ -#define SDHCI_VENDOR_SPEC_CAPABILITIES0 0x11c
- struct msm_sdhc_plat { struct mmc_config cfg; struct mmc mmc; };
@@ -48,8 +45,10 @@ struct msm_sdhc { };
struct msm_sdhc_variant_info { bool mci_removed;
u32 core_vendor_spec_capabilities0;
};
DECLARE_GLOBAL_DATA_PTR;
@@ -180,9 +179,9 @@ static int msm_sdc_probe(struct udevice *dev) */ if (core_major >= 1 && core_minor != 0x11 && core_minor != 0x12) { caps = readl(host->ioaddr + SDHCI_CAPABILITIES); caps |= SDHCI_CAN_VDD_300 | SDHCI_CAN_DO_8BIT;
writel(caps, host->ioaddr + SDHCI_VENDOR_SPEC_CAPABILITIES0);
writel(caps, host->ioaddr + var_info->core_vendor_spec_capabilities0); } ret = mmc_of_parse(dev, &plat->cfg); if (ret)
@@ -243,12 +242,16 @@ static int msm_sdc_bind(struct udevice *dev) }
static const struct msm_sdhc_variant_info msm_sdhc_mci_var = { .mci_removed = false,
.core_vendor_spec_capabilities0 = 0x21c,
};
static const struct msm_sdhc_variant_info msm_sdhc_v5_var = { .mci_removed = true,
.core_vendor_spec_capabilities0 = 0x11c,
};
static const struct udevice_id msm_mmc_ids[] = { { .compatible = "qcom,sdhci-msm-v4", .data = (ulong)&msm_sdhc_mci_var },
-- 2.44.0

using fdtdec_* functions is incompatible with OF_LIVE and generally offers a less friendly interface. Update to use dev_read_* functions instead.
Signed-off-by: Caleb Connolly caleb.connolly@linaro.org --- drivers/mmc/msm_sdhci.c | 28 +++++++++++++++++++--------- 1 file changed, 19 insertions(+), 9 deletions(-)
diff --git a/drivers/mmc/msm_sdhci.c b/drivers/mmc/msm_sdhci.c index f23d425144ef..5689b4765122 100644 --- a/drivers/mmc/msm_sdhci.c +++ b/drivers/mmc/msm_sdhci.c @@ -205,9 +205,9 @@ static int msm_sdc_remove(struct udevice *dev)
var_info = (void *)dev_get_driver_data(dev);
/* Disable host-controller mode */ - if (!var_info->mci_removed) + if (!var_info->mci_removed && priv->base) writel(0, priv->base + SDCC_MCI_HC_MODE);
clk_release_bulk(&priv->clks);
@@ -215,23 +215,33 @@ static int msm_sdc_remove(struct udevice *dev) }
static int msm_of_to_plat(struct udevice *dev) { - struct udevice *parent = dev->parent; struct msm_sdhc *priv = dev_get_priv(dev); + const struct msm_sdhc_variant_info *var_info; struct sdhci_host *host = &priv->host; - int node = dev_of_offset(dev); + int ret; + + var_info = (void*)dev_get_driver_data(dev);
host->name = strdup(dev->name); host->ioaddr = dev_read_addr_ptr(dev); - host->bus_width = fdtdec_get_int(gd->fdt_blob, node, "bus-width", 4); - host->index = fdtdec_get_uint(gd->fdt_blob, node, "index", 0); - priv->base = (void *)fdtdec_get_addr_size_auto_parent(gd->fdt_blob, - dev_of_offset(parent), node, "reg", 1, NULL, false); - if (priv->base == (void *)FDT_ADDR_T_NONE || - host->ioaddr == (void *)FDT_ADDR_T_NONE) + ret = dev_read_u32(dev, "bus-width", &host->bus_width); + if (ret) + host->bus_width = 4; + ret = dev_read_u32(dev, "index", &host->index); + if (ret) + host->index = 0; + priv->base = dev_read_addr_index_ptr(dev, 1); + + if (!host->ioaddr) return -EINVAL;
+ if (!var_info->mci_removed && !priv->base) { + printf("msm_sdhci: MCI base address not found\n"); + return -EINVAL; + } + return 0; }
static int msm_sdc_bind(struct udevice *dev)

On 09/04/2024 20:03, Caleb Connolly wrote:
using fdtdec_* functions is incompatible with OF_LIVE and generally offers a less friendly interface. Update to use dev_read_* functions instead.
Signed-off-by: Caleb Connolly caleb.connolly@linaro.org
drivers/mmc/msm_sdhci.c | 28 +++++++++++++++++++--------- 1 file changed, 19 insertions(+), 9 deletions(-)
diff --git a/drivers/mmc/msm_sdhci.c b/drivers/mmc/msm_sdhci.c index f23d425144ef..5689b4765122 100644 --- a/drivers/mmc/msm_sdhci.c +++ b/drivers/mmc/msm_sdhci.c @@ -205,9 +205,9 @@ static int msm_sdc_remove(struct udevice *dev)
var_info = (void *)dev_get_driver_data(dev);
/* Disable host-controller mode */
- if (!var_info->mci_removed)
if (!var_info->mci_removed && priv->base) writel(0, priv->base + SDCC_MCI_HC_MODE);
clk_release_bulk(&priv->clks);
@@ -215,23 +215,33 @@ static int msm_sdc_remove(struct udevice *dev) }
static int msm_of_to_plat(struct udevice *dev) {
- struct udevice *parent = dev->parent; struct msm_sdhc *priv = dev_get_priv(dev);
- const struct msm_sdhc_variant_info *var_info; struct sdhci_host *host = &priv->host;
- int node = dev_of_offset(dev);
int ret;
var_info = (void*)dev_get_driver_data(dev);
host->name = strdup(dev->name); host->ioaddr = dev_read_addr_ptr(dev);
- host->bus_width = fdtdec_get_int(gd->fdt_blob, node, "bus-width", 4);
- host->index = fdtdec_get_uint(gd->fdt_blob, node, "index", 0);
- priv->base = (void *)fdtdec_get_addr_size_auto_parent(gd->fdt_blob,
dev_of_offset(parent), node, "reg", 1, NULL, false);
- if (priv->base == (void *)FDT_ADDR_T_NONE ||
host->ioaddr == (void *)FDT_ADDR_T_NONE)
ret = dev_read_u32(dev, "bus-width", &host->bus_width);
if (ret)
host->bus_width = 4;
ret = dev_read_u32(dev, "index", &host->index);
if (ret)
host->index = 0;
priv->base = dev_read_addr_index_ptr(dev, 1);
if (!host->ioaddr) return -EINVAL;
if (!var_info->mci_removed && !priv->base) {
printf("msm_sdhci: MCI base address not found\n");
return -EINVAL;
}
return 0; }
static int msm_sdc_bind(struct udevice *dev)
Reviewed-by: Neil Armstrong neil.armstrong@linaro.org

This is useful for debugging.
Signed-off-by: Caleb Connolly caleb.connolly@linaro.org --- drivers/mmc/msm_sdhci.c | 2 ++ 1 file changed, 2 insertions(+)
diff --git a/drivers/mmc/msm_sdhci.c b/drivers/mmc/msm_sdhci.c index 5689b4765122..ea5d6b4cbbee 100644 --- a/drivers/mmc/msm_sdhci.c +++ b/drivers/mmc/msm_sdhci.c @@ -172,8 +172,10 @@ static int msm_sdc_probe(struct udevice *dev) core_major >>= SDCC_VERSION_MAJOR_SHIFT;
core_minor = core_version & SDCC_VERSION_MINOR_MASK;
+ log_debug("SDCC version %d.%d\n", core_major, core_minor); + /* * Support for some capabilities is not advertised by newer * controller versions and must be explicitly enabled. */

On 09/04/2024 20:03, Caleb Connolly wrote:
This is useful for debugging.
Signed-off-by: Caleb Connolly caleb.connolly@linaro.org
drivers/mmc/msm_sdhci.c | 2 ++ 1 file changed, 2 insertions(+)
diff --git a/drivers/mmc/msm_sdhci.c b/drivers/mmc/msm_sdhci.c index 5689b4765122..ea5d6b4cbbee 100644 --- a/drivers/mmc/msm_sdhci.c +++ b/drivers/mmc/msm_sdhci.c @@ -172,8 +172,10 @@ static int msm_sdc_probe(struct udevice *dev) core_major >>= SDCC_VERSION_MAJOR_SHIFT;
core_minor = core_version & SDCC_VERSION_MINOR_MASK;
- log_debug("SDCC version %d.%d\n", core_major, core_minor);
- /*
*/
- Support for some capabilities is not advertised by newer
- controller versions and must be explicitly enabled.
Reviewed-by: Neil Armstrong neil.armstrong@linaro.org

We currently default to the lowest rate but this actually doesn't work on most platforms. Default to the HS400 speed instead which is most common on Qualcomm platforms.
Signed-off-by: Caleb Connolly caleb.connolly@linaro.org --- drivers/mmc/msm_sdhci.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/drivers/mmc/msm_sdhci.c b/drivers/mmc/msm_sdhci.c index ea5d6b4cbbee..2144772ac325 100644 --- a/drivers/mmc/msm_sdhci.c +++ b/drivers/mmc/msm_sdhci.c @@ -61,9 +61,9 @@ static int msm_sdc_clk_init(struct udevice *dev) const char *clk_name;
ret = ofnode_read_u32(node, "clock-frequency", (uint *)(&clk_rate)); if (ret) - clk_rate = 400000; + clk_rate = 201500000;
ret = clk_get_bulk(dev, &prv->clks); if (ret) { log_warning("Couldn't get mmc clocks: %d\n", ret);

Hi,
On 09/04/2024 20:03, Caleb Connolly wrote:
We currently default to the lowest rate but this actually doesn't work on most platforms. Default to the HS400 speed instead which is most common on Qualcomm platforms.
Signed-off-by: Caleb Connolly caleb.connolly@linaro.org
drivers/mmc/msm_sdhci.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/drivers/mmc/msm_sdhci.c b/drivers/mmc/msm_sdhci.c index ea5d6b4cbbee..2144772ac325 100644 --- a/drivers/mmc/msm_sdhci.c +++ b/drivers/mmc/msm_sdhci.c @@ -61,9 +61,9 @@ static int msm_sdc_clk_init(struct udevice *dev) const char *clk_name;
ret = ofnode_read_u32(node, "clock-frequency", (uint *)(&clk_rate)); if (ret)
clk_rate = 400000;
clk_rate = 201500000;
I think we may either use INT_MAX so the clock driver uses the max freq in the table, or we could also parse the DT OPPs and take the max frequency ?
ret = clk_get_bulk(dev, &prv->clks); if (ret) { log_warning("Couldn't get mmc clocks: %d\n", ret);

Allow setting the clock rate for the SD card core clock. This is required for SD card support on SDM845 devices.
Signed-off-by: Caleb Connolly caleb.connolly@linaro.org --- drivers/clk/qcom/clock-qcom.h | 1 + drivers/clk/qcom/clock-sdm845.c | 17 +++++++++++++++++ 2 files changed, 18 insertions(+)
diff --git a/drivers/clk/qcom/clock-qcom.h b/drivers/clk/qcom/clock-qcom.h index cc170d8e3f9e..f6445c8f566f 100644 --- a/drivers/clk/qcom/clock-qcom.h +++ b/drivers/clk/qcom/clock-qcom.h @@ -12,8 +12,9 @@ #define CFG_CLK_SRC_GPLL0_AUX2 (2 << 8) #define CFG_CLK_SRC_GPLL9 (2 << 8) #define CFG_CLK_SRC_GPLL6 (4 << 8) #define CFG_CLK_SRC_GPLL7 (3 << 8) +#define CFG_CLK_SRC_GPLL4 (5 << 8) #define CFG_CLK_SRC_GPLL0_EVEN (6 << 8) #define CFG_CLK_SRC_MASK (7 << 8)
#define RCG_CFG_REG 0x4 diff --git a/drivers/clk/qcom/clock-sdm845.c b/drivers/clk/qcom/clock-sdm845.c index e9c61eb480de..782df7da8444 100644 --- a/drivers/clk/qcom/clock-sdm845.c +++ b/drivers/clk/qcom/clock-sdm845.c @@ -23,8 +23,9 @@
#define USB30_PRIM_MASTER_CLK_CMD_RCGR 0xf018 #define USB30_PRIM_MOCK_UTMI_CLK_CMD_RCGR 0xf030 #define USB3_PRIM_PHY_AUX_CMD_RCGR 0xf05c +#define SDCC2_APPS_CLK_CMD_RCGR 0x1400c
static const struct freq_tbl ftbl_gcc_qupv3_wrap0_s0_clk_src[] = { F(7372800, CFG_CLK_SRC_GPLL0_EVEN, 1, 384, 15625), F(14745600, CFG_CLK_SRC_GPLL0_EVEN, 1, 768, 15625), @@ -43,8 +44,19 @@ static const struct freq_tbl ftbl_gcc_qupv3_wrap0_s0_clk_src[] = { F(128000000, CFG_CLK_SRC_GPLL0, 1, 16, 75), { } };
+static const struct freq_tbl ftbl_gcc_sdcc2_apps_clk_src[] = { + F(400000, CFG_CLK_SRC_CXO, 12, 1, 4), + F(9600000, CFG_CLK_SRC_CXO, 2, 0, 0), + F(19200000, CFG_CLK_SRC_CXO, 1, 0, 0), + F(25000000, CFG_CLK_SRC_GPLL0_EVEN, 12, 0, 0), + F(50000000, CFG_CLK_SRC_GPLL0_EVEN, 6, 0, 0), + F(100000000, CFG_CLK_SRC_GPLL0, 6, 0, 0), + F(201500000, CFG_CLK_SRC_GPLL4, 4, 0, 0), + { } +}; + static ulong sdm845_clk_set_rate(struct clk *clk, ulong rate) { struct msm_clk_priv *priv = dev_get_priv(clk->dev); const struct freq_tbl *freq; @@ -54,8 +66,13 @@ static ulong sdm845_clk_set_rate(struct clk *clk, ulong rate) freq = qcom_find_freq(ftbl_gcc_qupv3_wrap0_s0_clk_src, rate); clk_rcg_set_rate_mnd(priv->base, SE9_UART_APPS_CMD_RCGR, freq->pre_div, freq->m, freq->n, freq->src, 16); return freq->freq; + case GCC_SDCC2_APPS_CLK: + freq = qcom_find_freq(ftbl_gcc_sdcc2_apps_clk_src, rate); + clk_rcg_set_rate_mnd(priv->base, SDCC2_APPS_CLK_CMD_RCGR, + freq->pre_div, freq->m, freq->n, freq->src, 8); + return freq->freq; default: return 0; } }

Adjust sdm845_get_pin_name() to return the correct names for the special pins. This fixes a non-fatal -ENOSYS error when probing MMC.
Signed-off-by: Caleb Connolly caleb.connolly@linaro.org --- drivers/pinctrl/qcom/pinctrl-sdm845.c | 13 ++++++++++++- 1 file changed, 12 insertions(+), 1 deletion(-)
diff --git a/drivers/pinctrl/qcom/pinctrl-sdm845.c b/drivers/pinctrl/qcom/pinctrl-sdm845.c index 459a4329ec80..c1e5cc01fded 100644 --- a/drivers/pinctrl/qcom/pinctrl-sdm845.c +++ b/drivers/pinctrl/qcom/pinctrl-sdm845.c @@ -65,9 +65,20 @@ static const char *sdm845_get_function_name(struct udevice *dev,
static const char *sdm845_get_pin_name(struct udevice *dev, unsigned int selector) { - snprintf(pin_name, MAX_PIN_NAME_LEN, "gpio%u", selector); + static const char *special_pins_names[] = { + "ufs_reset", + "sdc2_clk", + "sdc2_cmd", + "sdc2_data", + }; + + if (selector >= 150 && selector <= 154) + snprintf(pin_name, MAX_PIN_NAME_LEN, special_pins_names[selector - 150]); + else + snprintf(pin_name, MAX_PIN_NAME_LEN, "gpio%u", selector); + return pin_name; }
static unsigned int sdm845_get_function_mux(__maybe_unused unsigned int pin,

On 09/04/2024 20:03, Caleb Connolly wrote:
Adjust sdm845_get_pin_name() to return the correct names for the special pins. This fixes a non-fatal -ENOSYS error when probing MMC.
Signed-off-by: Caleb Connolly caleb.connolly@linaro.org
drivers/pinctrl/qcom/pinctrl-sdm845.c | 13 ++++++++++++- 1 file changed, 12 insertions(+), 1 deletion(-)
diff --git a/drivers/pinctrl/qcom/pinctrl-sdm845.c b/drivers/pinctrl/qcom/pinctrl-sdm845.c index 459a4329ec80..c1e5cc01fded 100644 --- a/drivers/pinctrl/qcom/pinctrl-sdm845.c +++ b/drivers/pinctrl/qcom/pinctrl-sdm845.c @@ -65,9 +65,20 @@ static const char *sdm845_get_function_name(struct udevice *dev,
static const char *sdm845_get_pin_name(struct udevice *dev, unsigned int selector) {
- snprintf(pin_name, MAX_PIN_NAME_LEN, "gpio%u", selector);
static const char *special_pins_names[] = {
"ufs_reset",
"sdc2_clk",
"sdc2_cmd",
"sdc2_data",
};
if (selector >= 150 && selector <= 154)
snprintf(pin_name, MAX_PIN_NAME_LEN, special_pins_names[selector - 150]);
else
snprintf(pin_name, MAX_PIN_NAME_LEN, "gpio%u", selector);
return pin_name; }
static unsigned int sdm845_get_function_mux(__maybe_unused unsigned int pin,
At some point we should add the pinconf settings for SDC and UFS, but for now it's ok!
Reviewed-by: Neil Armstrong neil.armstrong@linaro.org

Remove the reference to the xo clock which is on the unsupported rpmhcc clock controller. It isn't needed for MMC functionality.
Signed-off-by: Caleb Connolly caleb.connolly@linaro.org --- arch/arm/dts/sdm845-db845c-u-boot.dtsi | 7 +++++++ 1 file changed, 7 insertions(+)
diff --git a/arch/arm/dts/sdm845-db845c-u-boot.dtsi b/arch/arm/dts/sdm845-db845c-u-boot.dtsi index 906f9faa5451..9e4533e603c5 100644 --- a/arch/arm/dts/sdm845-db845c-u-boot.dtsi +++ b/arch/arm/dts/sdm845-db845c-u-boot.dtsi @@ -6,4 +6,11 @@ */ &pcie0_3p3v_dual { regulator-always-on; }; + +&sdhc_2 { + /* Remove the unsupported rpmhcc xo clock reference */ + clocks = <&gcc GCC_SDCC2_AHB_CLK>, + <&gcc GCC_SDCC2_APPS_CLK>; + clock-names = "iface", "core"; +};

Hi Caleb,
On Tue, 9 Apr 2024 at 23:33, Caleb Connolly caleb.connolly@linaro.org wrote:
Remove the reference to the xo clock which is on the unsupported rpmhcc clock controller. It isn't needed for MMC functionality.
Can it rather be handled via a NOP clock driver for rpmhcc? I suppose this kind of DT modifications would push us away from upstream DT compatibility.
-Sumit
Signed-off-by: Caleb Connolly caleb.connolly@linaro.org
arch/arm/dts/sdm845-db845c-u-boot.dtsi | 7 +++++++ 1 file changed, 7 insertions(+)
diff --git a/arch/arm/dts/sdm845-db845c-u-boot.dtsi b/arch/arm/dts/sdm845-db845c-u-boot.dtsi index 906f9faa5451..9e4533e603c5 100644 --- a/arch/arm/dts/sdm845-db845c-u-boot.dtsi +++ b/arch/arm/dts/sdm845-db845c-u-boot.dtsi @@ -6,4 +6,11 @@ */ &pcie0_3p3v_dual { regulator-always-on; };
+&sdhc_2 {
/* Remove the unsupported rpmhcc xo clock reference */
clocks = <&gcc GCC_SDCC2_AHB_CLK>,
<&gcc GCC_SDCC2_APPS_CLK>;
clock-names = "iface", "core";
+};
-- 2.44.0

On 10/04/2024 12:56, Sumit Garg wrote:
Hi Caleb,
On Tue, 9 Apr 2024 at 23:33, Caleb Connolly caleb.connolly@linaro.org wrote:
Remove the reference to the xo clock which is on the unsupported rpmhcc clock controller. It isn't needed for MMC functionality.
Can it rather be handled via a NOP clock driver for rpmhcc? I suppose this kind of DT modifications would push us away from upstream DT compatibility.
Yes, I plan to switch to that eventually with these patches (which I'll try and send soon). I just didn't want to leave this broken in the meantime.
https://git.codelinaro.org/linaro/qcomlt/u-boot/-/commit/85f5fc736777c9a0807...
https://git.codelinaro.org/linaro/qcomlt/u-boot/-/commit/973279605dd19e82356...
-Sumit
Signed-off-by: Caleb Connolly caleb.connolly@linaro.org
arch/arm/dts/sdm845-db845c-u-boot.dtsi | 7 +++++++ 1 file changed, 7 insertions(+)
diff --git a/arch/arm/dts/sdm845-db845c-u-boot.dtsi b/arch/arm/dts/sdm845-db845c-u-boot.dtsi index 906f9faa5451..9e4533e603c5 100644 --- a/arch/arm/dts/sdm845-db845c-u-boot.dtsi +++ b/arch/arm/dts/sdm845-db845c-u-boot.dtsi @@ -6,4 +6,11 @@ */ &pcie0_3p3v_dual { regulator-always-on; };
+&sdhc_2 {
/* Remove the unsupported rpmhcc xo clock reference */
clocks = <&gcc GCC_SDCC2_AHB_CLK>,
<&gcc GCC_SDCC2_APPS_CLK>;
clock-names = "iface", "core";
+};
-- 2.44.0

On Tue, 09 Apr 2024 20:02:59 +0200, Caleb Connolly wrote:
This series does some long overdue cleanup to the msm_sdhci driver, fixes v5 support, and adds the necessary clock configuration to get the sdcard up and running on the RB3.
Applied, thanks!
[1/7] mmc: msm_sdhci: correct vendor_spec_cap0 register for v5 commit: a737d8962cae2a37634bf0fe9f2b6907f7a047a7 [2/7] mmc: msm_sdhci: use modern DT handling commit: 5fc028897fb16abd0823b545b6f7b8a1df54d4dd [3/7] mmc: msm_sdhci: print core version commit: 0dc3cd3e5e501a575d0579b78b238bdd1b2a8274 [4/7] mmc: msm_sdhci: use a more sensible default clock rate commit: 6d327bca5ac7112f168fef8f2284e67211637e6c [5/7] clk/qcom: sdm845: enable SDCC2 core clock commit: da74cd018493b5a1f6bb8785bf6cc7754e42dfd8 [6/7] pinctrl: qcom: sdm845: add special pin names commit: 691e3cfc645504397a5f0da7afd2db96e4445f26 [7/7] dts: sdm845-db845c-u-boot: adjust MMC clocks commit: 17cfde60a2fcaf3365fdef7755a825aeddc6179a
Best regards,
participants (3)
-
Caleb Connolly
-
Neil Armstrong
-
Sumit Garg