diff mbox series

[1/7] mmc: msm_sdhci: correct vendor_spec_cap0 register for v5

Message ID 20240409-b4-qcom-mmc-fixes-v1-1-5039164ecbf7@linaro.org
State New
Headers show
Series qcom: mmc fixes and sdm845 support | expand

Commit Message

Caleb Connolly April 9, 2024, 6:03 p.m. UTC 
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(-)

Comments

Neil Armstrong April 10, 2024, 8:42 a.m. UTC | #1 
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>
Sumit Garg April 11, 2024, 1:59 p.m. UTC | #2 
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
>
Neil Armstrong April 12, 2024, 8:35 a.m. UTC | #3 
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
>>
Sumit Garg April 12, 2024, 9:40 a.m. UTC | #4 
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 mbox series

Patch

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 },