Re: [PATCH v2 01/11] scsi: ufs: core: Introduce a new ufshcd vops negotiate_pwr_mode()

[Can Guo <can.guo@oss.qualcomm.com>]

Hi Bean,

On 3/6/2026 5:03 AM, Bean Huo wrote:
> On Wed, 2026-03-04 at 05:53 -0800, Can Guo wrote:
>> @@ -4747,6 +4745,22 @@ static int ufshcd_change_power_mode(struct ufs_hba
>> *hba,
>>          return ret;
>>   }
>>   
>> +int ufshcd_change_power_mode(struct ufs_hba *hba,
>> +                            struct ufs_pa_layer_attr *pwr_mode)
>> +{
>> +       int ret;
>> +
>> +       ufshcd_vops_pwr_change_notify(hba, PRE_CHANGE, pwr_mode);
>
> ufshcd_change_power_mode() calls pwr_change_notify(PRE_CHANGE) but ignores its
> return, this can continue with invalid vendor prep? I saw there is checkup
> before, do you think adding check result?
pwr_change_notify(PRE_CHANGE) was used by most vendor-specific 
implementations
to negotiate Power Mode negotiation, if pwr_change_notify(PRE_CHANGE) is not
implemented (returns -ENOTSUPP) or Power Mode negotiation returns error, 
the error
check was there in order to call the memcpy() to copy the desired Power 
Mode as the
final power mode, that is, an error return from 
pwr_change_notify(PRE_CHANGE) won't
lead to skipping the ufshcd_change_power_mode().

So, to introduce the new vops negotiate_pwr_mode() and keep the logic 
same as before,
in this patch, the error check and its error handling are kept and 
coming after the call to
ufshcd_vops_negotiate_pwr_mode():

int ufshcd_config_pwr_mode(struct ufs_hba *hba,
                 struct ufs_pa_layer_attr *desired_pwr_mode)
{
         struct ufs_pa_layer_attr final_params = { 0 };
         int ret;

         ret = ufshcd_vops_negotiate_pwr_mode(hba, desired_pwr_mode,
                                              &final_params);
         if (ret)
                 memcpy(&final_params, desired_pwr_mode, 
sizeof(final_params));

         return ufshcd_change_power_mode(hba, &final_params);
}

I hope your question is answered.

Thanks,
Can Guo.
>
>> +
>> +       ret = ufshcd_dme_change_power_mode(hba, pwr_mode);
>> +
>> +       if (!ret)
>> +               ufshcd_vops_pwr_change_notify(hba, POST_CHANGE, pwr_mode);
>> +
>> +       return ret;
>> +}
>> +EXPORT_SYMBOL_GPL(ufshcd_change_power_mode);
>
> Kind regards,
> Bean