[PATCH 08/10] mmc: msm_sdhci: Handle different vendor cap register offsets

[Caleb Connolly]

On 26/02/2024 10:08, Varadarajan Narayanan wrote:
> In the IPQ9574, the vendor capabilities register is at a
> different offset than in other SoCs. Hence add a vendor_cap
> field to msm_sdhc_variant_info structure to get the correct
> register offset.
> 
> Signed-off-by: Varadarajan Narayanan <quic_varada at quicinc.com>
With below feedback addressed, please add

Reviewed-by: Caleb Connolly <caleb.connolly at linaro.org>
> ---
> 
>  drivers/mmc/msm_sdhci.c | 11 ++++++++++-
>  1 file changed, 10 insertions(+), 1 deletion(-)
> 
> diff --git a/drivers/mmc/msm_sdhci.c b/drivers/mmc/msm_sdhci.c
> index fe1e754bfd..9285d412fe 100644
> --- a/drivers/mmc/msm_sdhci.c
> +++ b/drivers/mmc/msm_sdhci.c
> @@ -48,6 +48,7 @@ struct msm_sdhc {
>  
>  struct msm_sdhc_variant_info {
>  	bool mci_removed;
> +	u32  vendor_cap;	/* Non standard (?) SDHCI register */
>  };
>  
>  DECLARE_GLOBAL_DATA_PTR;
> @@ -158,7 +159,7 @@ 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->vendor_cap);
>  	}
>  
>  	ret = mmc_of_parse(dev, &plat->cfg);
> @@ -219,15 +220,23 @@ static int msm_sdc_bind(struct udevice *dev)
>  
>  static const struct msm_sdhc_variant_info msm_sdhc_mci_var = {
>  	.mci_removed = false,
> +	.vendor_cap = SDHCI_VENDOR_SPEC_CAPABILITIES0,
>  };
>  
>  static const struct msm_sdhc_variant_info msm_sdhc_v5_var = {
>  	.mci_removed = true,
> +	.vendor_cap = SDHCI_VENDOR_SPEC_CAPABILITIES0,
This is wrong, it should be 0x21c for all v5 controllers (see
drivers/mmc/host/sdhci-msm.c in Linux).
> +};
> +
> +static const struct msm_sdhc_variant_info msm_sdhc_v5_ipq_var = {
So this can be dropped
> +	.mci_removed = true,
> +	.vendor_cap = 0x21c,
>  };
>  
>  static const struct udevice_id msm_mmc_ids[] = {
>  	{ .compatible = "qcom,sdhci-msm-v4", .data = (ulong)&msm_sdhc_mci_var },
>  	{ .compatible = "qcom,sdhci-msm-v5", .data = (ulong)&msm_sdhc_v5_var },
> +	{ .compatible = "qti,sdhci-msm-v5", .data = (ulong)&msm_sdhc_v5_ipq_var },
This can be dropped too.

For future reference as I really don't want to deal with these, please
never submit a patch using or otherwise referencing a compatible string
starting with "qti,". There is never a reason such a compatible should
appear in upstream code or DT.
>  	{ }
>  };
>  

-- 
// Caleb (they/them)