Re: [PATCH V1 2/3] mmc: sdhci-msm: Enable tuning for SDR50 mode for SD card

[Adrian Hunter]

On 7/11/24 10:05, Sarthak Garg wrote:
> For Qualcomm SoCs which needs level shifter for SD card, extra delay is
> seen on receiver data path.
> 
> To compensate this delay enable tuning for SDR50 mode for targets which
> has level shifter.
> 
> Signed-off-by: Sarthak Garg <quic_sartgarg@xxxxxxxxxxx>
> ---
>  drivers/mmc/host/sdhci-msm.c | 16 ++++++++++++++++
>  1 file changed, 16 insertions(+)
> 
> diff --git a/drivers/mmc/host/sdhci-msm.c b/drivers/mmc/host/sdhci-msm.c
> index e00208535bd1..16325c21de52 100644
> --- a/drivers/mmc/host/sdhci-msm.c
> +++ b/drivers/mmc/host/sdhci-msm.c
> @@ -81,6 +81,7 @@
>  #define CORE_IO_PAD_PWR_SWITCH_EN	BIT(15)
>  #define CORE_IO_PAD_PWR_SWITCH	BIT(16)
>  #define CORE_HC_SELECT_IN_EN	BIT(18)
> +#define CORE_HC_SELECT_IN_SDR50	(4 << 19)
>  #define CORE_HC_SELECT_IN_HS400	(6 << 19)
>  #define CORE_HC_SELECT_IN_MASK	(7 << 19)
>  
> @@ -1124,6 +1125,10 @@ static bool sdhci_msm_is_tuning_needed(struct sdhci_host *host)
>  {
>  	struct mmc_ios *ios = &host->mmc->ios;
>  
> +	if (ios->timing == MMC_TIMING_UHS_SDR50 &&
> +			host->flags & SDHCI_SDR50_NEEDS_TUNING)

Please do line up code as suggested by checkpatch:

CHECK: Alignment should match open parenthesis
#35: FILE: drivers/mmc/host/sdhci-msm.c:1129:
+       if (ios->timing == MMC_TIMING_UHS_SDR50 &&
+                       host->flags & SDHCI_SDR50_NEEDS_TUNING)

CHECK: Alignment should match open parenthesis
#55: FILE: drivers/mmc/host/sdhci-msm.c:1219:
+       if (ios.timing == MMC_TIMING_UHS_SDR50 &&
+                       host->flags & SDHCI_SDR50_NEEDS_TUNING) {

total: 0 errors, 0 warnings, 2 checks, 40 lines checked


> +		return true;
> +
>  	/*
>  	 * Tuning is required for SDR104, HS200 and HS400 cards and
>  	 * if clock frequency is greater than 100MHz in these modes.
> @@ -1192,6 +1197,8 @@ static int sdhci_msm_execute_tuning(struct mmc_host *mmc, u32 opcode)
>  	struct mmc_ios ios = host->mmc->ios;
>  	struct sdhci_pltfm_host *pltfm_host = sdhci_priv(host);
>  	struct sdhci_msm_host *msm_host = sdhci_pltfm_priv(pltfm_host);
> +	const struct sdhci_msm_offset *msm_offset = msm_host->offset;
> +	u32 config;
>  
>  	if (!sdhci_msm_is_tuning_needed(host)) {
>  		msm_host->use_cdr = false;
> @@ -1208,6 +1215,15 @@ static int sdhci_msm_execute_tuning(struct mmc_host *mmc, u32 opcode)
>  	 */
>  	msm_host->tuning_done = 0;
>  
> +	if (ios.timing == MMC_TIMING_UHS_SDR50 &&
> +			host->flags & SDHCI_SDR50_NEEDS_TUNING) {

Ditto alignment

> +		config = readl_relaxed(host->ioaddr + msm_offset->core_vendor_spec);
> +		config |= CORE_HC_SELECT_IN_EN;
> +		config &= ~CORE_HC_SELECT_IN_MASK;
> +		config |= CORE_HC_SELECT_IN_SDR50;

Perhaps clear bits first, then set bits e.g.

		config &= ~CORE_HC_SELECT_IN_MASK;
		config |= CORE_HC_SELECT_IN_EN | CORE_HC_SELECT_IN_SDR50;

> +		writel_relaxed(config, host->ioaddr + msm_offset->core_vendor_spec);
> +	}
> +
>  	/*
>  	 * For HS400 tuning in HS200 timing requires:
>  	 * - select MCLK/2 in VENDOR_SPEC