Re: [PATCH v4 3/8] scsi: ufs: core: Add a vop to map clock frequency to gear speed

[Ziqi Chen <quic_ziqichen@quicinc.com>]

On 2/11/2025 2:02 PM, Peter Wang (王信友) wrote:
> On Mon, 2025-02-10 at 18:02 +0800, Ziqi Chen wrote:
>>
>> External email : Please do not click links or open attachments until
>> you have verified the sender or the content.
>>
>>
>> From: Can Guo <quic_cang@quicinc.com>
>>
>> Add a vop to map UFS host controller clock frequencies to the
>> corresponding
>> maximum supported UFS high speed gear speeds. During clock scaling,
>> we can
>> map the target clock frequency, demanded by devfreq, to the maximum
>> supported gear speed, so that devfreq can scale the gear to the
>> highest
>> gear speed supported at the target clock frequency, instead of just
>> scaling
>> up/down the gear between the min and max gear speeds.
>>
>> Co-developed-by: Ziqi Chen <quic_ziqichen@quicinc.com>
>> Signed-off-by: Ziqi Chen <quic_ziqichen@quicinc.com>
>> Signed-off-by: Can Guo <quic_cang@quicinc.com>
>> Reviewed-by: Bean Huo <beanhuo@micron.com>
>> Reviewed-by: Bart Van Assche <bvanassche@acm.org>
>> Tested-by: Neil Armstrong <neil.armstrong@linaro.org>
>> ---
>> v2 ->v3:
>> 1. Remove the parameter 'gear' and use it as function return result.
>> 2. Change "vops" into "vop" in commit message.
>> ---
>>   drivers/ufs/core/ufshcd-priv.h | 8 ++++++++
>>   include/ufs/ufshcd.h           | 2 ++
>>   2 files changed, 10 insertions(+)
>>
>> diff --git a/drivers/ufs/core/ufshcd-priv.h
>> b/drivers/ufs/core/ufshcd-priv.h
>> index 0549b65f71ed..4da3e65c6735 100644
>> --- a/drivers/ufs/core/ufshcd-priv.h
>> +++ b/drivers/ufs/core/ufshcd-priv.h
>> @@ -277,6 +277,14 @@ static inline int
>> ufshcd_mcq_vops_config_esi(struct ufs_hba *hba)
>>          return -EOPNOTSUPP;
>>   }
>>
>> +static inline int ufshcd_vops_freq_to_gear_speed(struct ufs_hba
> 
> Hi Ziqi,
> 
> Gear speed type in struct ufs_pa_layer_attr is u32.
> I think it would be better to unify the type here.
> 
> 
Hi Peter,

I changed the type of new_gear since V3 sue to I use it as return result 
of vop ufshcd_vops_freq_to_gear_speed(). I think you already find the 
reason in patch 4/8 and 5/8. I think your suggestion in patch 5/8 is 
make sense. Let me discuss more with you in the patch 5/8.

-Ziqi

> 
>> *hba, unsigned long freq)
>> +{
>> +       if (hba->vops && hba->vops->freq_to_gear_speed)
>> +               return hba->vops->freq_to_gear_speed(hba, freq);
>> +
>> +       return -EOPNOTSUPP;
>> +}
>> +
>>   extern const struct ufs_pm_lvl_states ufs_pm_lvl_states[];
>>
>>   /**
>> diff --git a/include/ufs/ufshcd.h b/include/ufs/ufshcd.h
>> index f51d425696e7..cdb853f5b871 100644
>> --- a/include/ufs/ufshcd.h
>> +++ b/include/ufs/ufshcd.h
>> @@ -336,6 +336,7 @@ struct ufs_pwr_mode_info {
>>    * @get_outstanding_cqs: called to get outstanding completion queues
>>    * @config_esi: called to config Event Specific Interrupt
>>    * @config_scsi_dev: called to configure SCSI device parameters
>> + * @freq_to_gear_speed: called to map clock frequency to the max
>> supported gear speed
>>    */
>>   struct ufs_hba_variant_ops {
>>          const char *name;
>> @@ -387,6 +388,7 @@ struct ufs_hba_variant_ops {
>>                                         unsigned long *ocqs);
>>          int     (*config_esi)(struct ufs_hba *hba);
>>          void    (*config_scsi_dev)(struct scsi_device *sdev);
>> +       int (*freq_to_gear_speed)(struct ufs_hba *hba, unsigned long
>> freq);
>>
> 
> Please keep the indentation consistent too.
> 
> Thanks
> Peter
> 
>>   };
>>
>>   /* clock gating state  */
>> --
>> 2.34.1
>>
>