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

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <8b415442-283a-4fc8-ad0d-fbd8892d2ba2@intel.com>
Date: Mon, 11 Nov 2024 10:58:05 +0200
From: Adrian Hunter <adrian.hunter@...el.com>
To: Sarthak Garg <quic_sartgarg@...cinc.com>,
 Ulf Hansson <ulf.hansson@...aro.org>, Rob Herring <robh@...nel.org>,
 Krzysztof Kozlowski <krzk+dt@...nel.org>, Conor Dooley
 <conor+dt@...nel.org>, Bhupesh Sharma <bhupesh.sharma@...aro.org>
Cc: linux-mmc@...r.kernel.org, devicetree@...r.kernel.org,
 linux-kernel@...r.kernel.org, linux-arm-msm@...r.kernel.org,
 quic_cang@...cinc.com, quic_nguyenb@...cinc.com, quic_rampraka@...cinc.com,
 quic_pragalla@...cinc.com, quic_sayalil@...cinc.com,
 quic_nitirawa@...cinc.com, quic_sachgupt@...cinc.com,
 quic_bhaskarv@...cinc.com, quic_narepall@...cinc.com, kernel@...cinc.com
Subject: Re: [PATCH V1 3/3] mmc: sdhci-msm: Limit HS mode frequency to 37.5MHz

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@...cinc.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?

> +		&& 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


> +
>  	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;
>  }