Re: [PATCH v2 3/4] scsi: ufs: core: Record the frequency the controller starts at

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [Medium] Unprotected concurrent modification of the globally shared `ufs_hba_vps` structure during asynchronous probe.
--

commit d1d1b66402811c53b2fbc0d56ed07cd4f77540ee
Author: Bean Huo <beanhuo@micron.com>

scsi: ufs: core: Record the frequency the controller starts at

This commit records the maximum frequency during ufshcd_devfreq_init(),
preventing devfreq from reporting an initial 0 Hz frequency. It avoids
an unnecessary governor scale that resets the same frequency and gear.
It addresses the issue for both modes with and without OPPs.

> diff --git a/drivers/ufs/core/ufshcd.c b/drivers/ufs/core/ufshcd.c
> index 2ba244cf40ac7..351c76094b9ff 100644
> --- a/drivers/ufs/core/ufshcd.c
> +++ b/drivers/ufs/core/ufshcd.c

[ ... ]

> @@ -1727,12 +1722,21 @@ static int ufshcd_devfreq_init(struct ufs_hba *hba)
>  	if (list_empty(clk_list))
>  		return 0;
>  
> +	clki = list_first_entry(clk_list, struct ufs_clk_info, list);
> +
>  	if (!hba->use_pm_opp) {
> -		clki = list_first_entry(clk_list, struct ufs_clk_info, list);
>  		dev_pm_opp_add(hba->dev, clki->min_freq, 0);
>  		dev_pm_opp_add(hba->dev, clki->max_freq, 0);
>  	}
>  
> +	/*
> +	 * ufshcd_init_clocks() has already set the clocks to the highest
> +	 * frequency, and nothing has changed them since. Save that frequency,
> +	 * so that devfreq and the clock scaling code know where we start.
> +	 */
> +	hba->clk_scaling.target_freq = clki->max_freq;
> +	hba->vps->devfreq_profile.initial_freq = clki->max_freq;

[Severity: Medium]
Could this introduce a data race during asynchronous probe?

Because ufs_hba_vps is a globally shared static structure assigned to all UFS
host controllers by default (hba->vps = &ufs_hba_vps), writing directly to
hba->vps->devfreq_profile.initial_freq mutates this shared state.

If multiple controllers probe concurrently during system initialization
(e.g. via ufshcd_async_scan), they will race to overwrite this field.
If the controllers have different maximum frequencies, they might overwrite
each other's initial devfreq settings, causing devfreq to start with an
incorrect frequency on one or more controllers.

> +
>  	ufshcd_vops_config_scaling_param(hba, &hba->vps->devfreq_profile,
>  					 &hba->vps->ondemand_data);
>  	devfreq = devfreq_add_device(hba->dev,

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260907192140.2701755-1-beanhuo@iokpp.de?part=3