Re: [PATCH 14/29] scsi: ufs: Make the config_scaling_param calls type safe

[Bjorn Andersson <bjorn.andersson@linaro.org>]

On Thu 31 Mar 15:34 PDT 2022, Bart Van Assche wrote:

> Pass the actual type to config_scaling_param callback as the third
> argment instead of a void pointer. Remove a superfluous NULL pointer
> check from ufs_qcom_config_scaling_param().
> 
> Signed-off-by: Bart Van Assche <bvanassche@acm.org>
> ---
>  drivers/scsi/ufs/ufs-qcom.c | 14 ++++----------
>  drivers/scsi/ufs/ufshcd.h   | 10 +++++-----
>  2 files changed, 9 insertions(+), 15 deletions(-)
> 
> diff --git a/drivers/scsi/ufs/ufs-qcom.c b/drivers/scsi/ufs/ufs-qcom.c
> index 808b677f6083..f24210652fe9 100644
> --- a/drivers/scsi/ufs/ufs-qcom.c
> +++ b/drivers/scsi/ufs/ufs-qcom.c
> @@ -1463,23 +1463,17 @@ static int ufs_qcom_device_reset(struct ufs_hba *hba)
>  
>  #if IS_ENABLED(CONFIG_DEVFREQ_GOV_SIMPLE_ONDEMAND)
>  static void ufs_qcom_config_scaling_param(struct ufs_hba *hba,
> -					  struct devfreq_dev_profile *p,
> -					  void *data)
> +		struct devfreq_dev_profile *p,
> +		struct devfreq_simple_ondemand_data *d)

This doesn't look to be properly indended to match the '('?
What does ./scripts/checkpatch.pl --strict say about the patch?


Other than that, the change looks good, so feel free to add my r-b once
you've double checked the indentation.

Reviewed-by: Bjorn Andersson <bjorn.andersson@linaro.org>

Regards,
Bjorn

>  {
> -	static struct devfreq_simple_ondemand_data *d;
> -
> -	if (!data)
> -		return;
> -
> -	d = (struct devfreq_simple_ondemand_data *)data;
>  	p->polling_ms = 60;
>  	d->upthreshold = 70;
>  	d->downdifferential = 5;
>  }
>  #else
>  static void ufs_qcom_config_scaling_param(struct ufs_hba *hba,
> -					  struct devfreq_dev_profile *p,
> -					  void *data)
> +		struct devfreq_dev_profile *p,
> +		struct devfreq_simple_ondemand_data *data)
>  {
>  }
>  #endif
> diff --git a/drivers/scsi/ufs/ufshcd.h b/drivers/scsi/ufs/ufshcd.h
> index 107d19e98d52..bb2624aabda2 100644
> --- a/drivers/scsi/ufs/ufshcd.h
> +++ b/drivers/scsi/ufs/ufshcd.h
> @@ -348,8 +348,8 @@ struct ufs_hba_variant_ops {
>  	int	(*phy_initialization)(struct ufs_hba *);
>  	int	(*device_reset)(struct ufs_hba *hba);
>  	void	(*config_scaling_param)(struct ufs_hba *hba,
> -					struct devfreq_dev_profile *profile,
> -					void *data);
> +				struct devfreq_dev_profile *profile,
> +				struct devfreq_simple_ondemand_data *data);
>  	int	(*program_key)(struct ufs_hba *hba,
>  			       const union ufs_crypto_cfg_entry *cfg, int slot);
>  	void	(*event_notify)(struct ufs_hba *hba,
> @@ -1360,11 +1360,11 @@ static inline int ufshcd_vops_device_reset(struct ufs_hba *hba)
>  }
>  
>  static inline void ufshcd_vops_config_scaling_param(struct ufs_hba *hba,
> -						    struct devfreq_dev_profile
> -						    *profile, void *data)
> +		struct devfreq_dev_profile *p,
> +		struct devfreq_simple_ondemand_data *data)
>  {
>  	if (hba->vops && hba->vops->config_scaling_param)
> -		hba->vops->config_scaling_param(hba, profile, data);
> +		hba->vops->config_scaling_param(hba, p, data);
>  }
>  
>  extern struct ufs_pm_lvl_states ufs_pm_lvl_states[];