Re: [PATCH V1 3/3] mmc: sdhci-msm: Limit HS mode frequency to 37.5MHz

[Sarthak Garg <quic_sartgarg@quicinc.com>]

On 11/11/2024 2:28 PM, Adrian Hunter wrote:
> On 7/11/24 10:05, Sarthak Garg wrote:
>> For Qualcomm SoCs with level shifter delays are seen on receivers data
>> path due to latency added by level shifter.
>>
>> To bring these delays in normal range and avoid CMD CRC errors
>> reduce frequency for HS mode SD cards to 37.5MHz for targets which has
>> level shifter.
>>
>> Signed-off-by: Sarthak Garg <quic_sartgarg@quicinc.com>
>> ---
>>   drivers/mmc/host/sdhci-msm.c | 10 ++++++++++
>>   1 file changed, 10 insertions(+)
>>
>> diff --git a/drivers/mmc/host/sdhci-msm.c b/drivers/mmc/host/sdhci-msm.c
>> index 16325c21de52..5e1dc06c4707 100644
>> --- a/drivers/mmc/host/sdhci-msm.c
>> +++ b/drivers/mmc/host/sdhci-msm.c
>> @@ -138,6 +138,8 @@
>>   /* Max load for eMMC Vdd-io supply */
>>   #define MMC_VQMMC_MAX_LOAD_UA	325000
>>   
>> +#define LEVEL_SHIFTER_HIGH_SPEED_FREQ	37500000
>> +
>>   #define msm_host_readl(msm_host, host, offset) \
>>   	msm_host->var_ops->msm_readl_relaxed(host, offset)
>>   
>> @@ -287,6 +289,7 @@ struct sdhci_msm_host {
>>   	bool use_cdr;
>>   	u32 transfer_mode;
>>   	bool updated_ddr_cfg;
>> +	bool uses_level_shifter;
>>   	bool uses_tassadar_dll;
>>   	u32 dll_config;
>>   	u32 ddr_config;
>> @@ -366,6 +369,11 @@ static void msm_set_clock_rate_for_bus_mode(struct sdhci_host *host,
>>   
>>   	mult = msm_get_clock_mult_for_bus_mode(host);
>>   	desired_rate = clock * mult;
>> +
>> +	if (curr_ios.timing == MMC_TIMING_SD_HS && desired_rate == 50000000
> 
> Wouldn't desired_rate > LEVEL_SHIFTER_HIGH_SPEED_FREQ make more sense?
> 

Sure will update in V2.

>> +		&& msm_host->uses_level_shifter)
>> +		desired_rate = LEVEL_SHIFTER_HIGH_SPEED_FREQ;
> 
> As checkpatch says:
> 
> CHECK: Logical continuations should be on the previous line
> #46: FILE: drivers/mmc/host/sdhci-msm.c:374:
> +       if (curr_ios.timing == MMC_TIMING_SD_HS && desired_rate == 50000000
> +               && msm_host->uses_level_shifter)
> 
> total: 0 errors, 0 warnings, 1 checks, 34 lines checked
> 
> 

Sure will fix this in V2.

>> +
>>   	rc = dev_pm_opp_set_rate(mmc_dev(host->mmc), desired_rate);
>>   	if (rc) {
>>   		pr_err("%s: Failed to set clock at rate %u at timing %d\n",
>> @@ -2372,6 +2380,8 @@ static inline void sdhci_msm_get_of_property(struct platform_device *pdev,
>>   
>>   	of_property_read_u32(node, "qcom,dll-config", &msm_host->dll_config);
>>   
>> +	msm_host->uses_level_shifter = of_property_read_bool(node, "qcom,use-level-shifter");
>> +
>>   	if (of_device_is_compatible(node, "qcom,msm8916-sdhci"))
>>   		host->quirks2 |= SDHCI_QUIRK2_BROKEN_64_BIT_DMA;
>>   }
>