Re: [PATCH V1 1/2] scsi: ufs: ufs-qcom: Add specified gear support for multi gear scaling

[Nitin Rawat]

On 9/2/2026 9:32 PM, Manivannan Sadhasivam wrote:
> On Sat, Aug 29, 2026 at 01:13:54PM +0530, Nitin Rawat wrote:
> > From: Ziqi Chen <ziqi.chen@xxxxxxxxxxxxxxxx>
> >
> > The UFS clock frequency and gear speed do not necessarily have a strict
> > one-to-one correspondence on all platforms. Introduce a device tree
> > based configuration interface that allows specifying the HS gear
> > speed for each supported operating frequency via the "opp-level"
> > property in the OPP table. When this property is not configured, the
> > driver falls back to the default frequency-to-gear mapping table.
> >
> > Signed-off-by: Ziqi Chen <ziqi.chen@xxxxxxxxxxxxxxxx>
> > Signed-off-by: Nitin Rawat <nitin.rawat@xxxxxxxxxxxxxxxx>
> > ---
> >   drivers/ufs/host/ufs-qcom.c | 30 ++++++++++++++++++++++++++----
> >   1 file changed, 26 insertions(+), 4 deletions(-)
> >
> > diff --git a/drivers/ufs/host/ufs-qcom.c b/drivers/ufs/host/ufs-qcom.c
> > index 0f2e083b04fd..aa2ac2cd2b69 100644
> > --- a/drivers/ufs/host/ufs-qcom.c
> > +++ b/drivers/ufs/host/ufs-qcom.c
> > @@ -2460,8 +2460,9 @@ static unsigned long ufs_qcom_opp_freq_to_clk_freq(struct ufs_hba *hba,
> >   	bool found = false;
> >
> >   	opp = dev_pm_opp_find_freq_exact_indexed(hba->dev, freq, 0, true);
> > -	if (IS_ERR(opp)) {
> > -		dev_err(hba->dev, "Failed to find OPP for exact frequency %lu\n", freq);
> > +	if (IS_ERR_OR_NULL(opp)) {
> > +		dev_err(hba->dev, "%s: Failed to find OPP for exact frequency %lu\n",
> > +			__func__, freq);
>
> Don't bring back the '__func__' marking please...
Sure, will take in next patchset


> >   		return 0;
> >   	}
> >
> > @@ -2489,12 +2490,32 @@ static unsigned long ufs_qcom_opp_freq_to_clk_freq(struct ufs_hba *hba,
> >
> >   static u32 ufs_qcom_freq_to_gear_speed(struct ufs_hba *hba, unsigned long freq)
> >   {
> > -	u32 gear = UFS_HS_DONT_CHANGE;
> > +	struct dev_pm_opp *opp;
> >   	unsigned long unipro_freq;
> > +	u32 gear = UFS_HS_DONT_CHANGE;
>
> Nit: Preserve reverse Xmas order.
Sure, will take in next patchset

> >   	if (!hba->use_pm_opp)
> >   		return gear;
> >
> > +	opp = dev_pm_opp_find_freq_exact_indexed(hba->dev, freq, 0, true);
> > +	if (IS_ERR_OR_NULL(opp)) {
> > +		dev_err(hba->dev, "%s: Failed to find OPP for exact frequency %lu\n",
> > +			__func__, freq);
>
> Drop '__func__' here and below.
Sure, will take in next patchset

> > +		return gear;
> > +	}
> > +
> > +	/* Get HS gear speed from 'opp-level' */
> > +	gear = dev_pm_opp_get_level(opp);
> > +	dev_pm_opp_put(opp);
> > +
> > +	/*
> > +	 * Greater than max gear means that there is no specified gear configured in DT
> > +	 * or the specified gear is invalid.
> > +	 */
> > +	if (gear <= hba->max_pwr_info.info.gear_rx)
> > +		return gear;
>
> Sashiko pointed out a valid concern with this check, please take a look.

I reviewed the bot's comment. The concern raised applies to the case 
where the gear value provided through the device tree is higher (for 
example, 5) than what a UFS 3.x device can support. After link startup 
and negotiation, hba->max_pwr_info.info.gear_rx would be 4. In this 
scenario, the condition below evaluates to false:


> +	if (gear <= hba->max_pwr_info.info.gear_rx)
> +		return gear;

Execution then falls back to the switch-case logic. If the current 
frequency does not match any of the predefined entries, the function 
returns UFS_HS_DONT_CHANGE, which effectively maps to the minimum gear. 
Shahiko's suggestion is to instead return the device's maximum 
negotiated gear using:

A couple of points to note:

1. If the current frequency does not match any of the expected frequency 
entries, that is already an existing issue. In such a case, simply 
returning the maximum negotiated gear may not be correct because we do 
not know the actual gear corresponding to the currently programmed 
frequency.

2. The patch under review does not change this existing behavior. It 
only addresses the handling of gear values that are within the 
negotiated device capabilities and does not alter the fallback path when 
the frequency lookup fails.


Considering the above points, I believe no changes are required for this 
patch at this time.
We can revisit this behavior separately and evaluate potential 
optimizations in a future patch if needed.

Please let me know your opinion.

Thanks,
Nitin






> - Mani