Message ID | 20240409-b4-qcom-mmc-fixes-v1-1-5039164ecbf7@linaro.org |
---|---|
State | New |
Headers | show |
Series | qcom: mmc fixes and sdm845 support | expand |
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 > >> >
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 },
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(-)