Re: [PATCH v3 1/2] scsi: ufs: pass device information to apply_dev_quirks

[Can Guo]

On 2020-01-11 15:11, Stanley Chu wrote:
> Pass UFS device information to vendor-specific variant callback
> "apply_dev_quirks" because some platform vendors need to know such
> information to apply special handlings or quirks in specific devices.
>
> In the same time, modify existed vendor implementations according to
> the new interface for those vendor drivers which will be built-in
> or built as a module alone with UFS core driver.
>
> Cc: Alim Akhtar <alim.akhtar@xxxxxxxxxxx>
> Cc: Asutosh Das <asutoshd@xxxxxxxxxxxxxx>
> Cc: Avri Altman <avri.altman@xxxxxxx>
> Cc: Bart Van Assche <bvanassche@xxxxxxx>
> Cc: Bean Huo <beanhuo@xxxxxxxxxx>
> Cc: Can Guo <cang@xxxxxxxxxxxxxx>
> Cc: Matthias Brugger <matthias.bgg@xxxxxxxxx>
> Reviewed-by: Avri Altman <avri.altman@xxxxxxx>
> Reviewed-by: Bean Huo <beanhuo@xxxxxxxxxx>
> Signed-off-by: Stanley Chu <stanley.chu@xxxxxxxxxxxx>
> ---
>  drivers/scsi/ufs/ufs-qcom.c | 3 ++-
>  drivers/scsi/ufs/ufshcd.c   | 8 ++++----
>  drivers/scsi/ufs/ufshcd.h   | 7 ++++---
>  3 files changed, 10 insertions(+), 8 deletions(-)
>
> diff --git a/drivers/scsi/ufs/ufs-qcom.c b/drivers/scsi/ufs/ufs-qcom.c
> index c69c29a1ceb9..ebb5c66e069f 100644
> --- a/drivers/scsi/ufs/ufs-qcom.c
> +++ b/drivers/scsi/ufs/ufs-qcom.c
> @@ -949,7 +949,8 @@ static int
> ufs_qcom_quirk_host_pa_saveconfigtime(struct ufs_hba *hba)
>  	return err;
>  }
>
> -static int ufs_qcom_apply_dev_quirks(struct ufs_hba *hba)
> +static int ufs_qcom_apply_dev_quirks(struct ufs_hba *hba,
> +				     struct ufs_dev_desc *card)
>  {
>  	int err = 0;
>
> diff --git a/drivers/scsi/ufs/ufshcd.c b/drivers/scsi/ufs/ufshcd.c
> index 1b97f2dc0b63..7c85c890594c 100644
> --- a/drivers/scsi/ufs/ufshcd.c
> +++ b/drivers/scsi/ufs/ufshcd.c
> @@ -6803,7 +6803,8 @@ static int
> ufshcd_quirk_tune_host_pa_tactivate(struct ufs_hba *hba)
>  	return ret;
>  }
>
> -static void ufshcd_tune_unipro_params(struct ufs_hba *hba)
> +static void ufshcd_tune_unipro_params(struct ufs_hba *hba,
> +				      struct ufs_dev_desc *card)
>  {
>  	if (ufshcd_is_unipro_pa_params_tuning_req(hba)) {
>  		ufshcd_tune_pa_tactivate(hba);
> @@ -6817,7 +6818,7 @@ static void ufshcd_tune_unipro_params(struct 
> ufs_hba *hba)
>  	if (hba->dev_quirks & UFS_DEVICE_QUIRK_HOST_PA_TACTIVATE)
>  		ufshcd_quirk_tune_host_pa_tactivate(hba);
>
> -	ufshcd_vops_apply_dev_quirks(hba);
> +	ufshcd_vops_apply_dev_quirks(hba, card);

Hi Stanley,

Is this series merged? If no, would you mind moving
ufshcd_vops_apply_dev_quirks(hba, card); a little bit? Like below.

@@ -6852,14 +6852,14 @@ static void ufshcd_tune_unipro_params(struct 
ufs_hba *hba)
                ufshcd_tune_pa_hibern8time(hba);
        }

+       ufshcd_vops_apply_dev_quirks(hba, card);
+
        if (hba->dev_quirks & UFS_DEVICE_QUIRK_PA_TACTIVATE)
                /* set 1ms timeout for PA_TACTIVATE */
                ufshcd_dme_set(hba, UIC_ARG_MIB(PA_TACTIVATE), 10);

In this way, vendor codes have a chance to modify the dev_quirks
before ufshcd_tune_unipro_params() does the rest of its job.

Thanks,
Can Guo.

>  }
>
>  static void ufshcd_clear_dbg_ufs_stats(struct ufs_hba *hba)
> @@ -6980,10 +6981,9 @@ static int ufshcd_probe_hba(struct ufs_hba *hba)
>  	}
>
>  	ufs_fixup_device_setup(hba, &card);
> +	ufshcd_tune_unipro_params(hba, &card);
>  	ufs_put_device_desc(&card);
>
> -	ufshcd_tune_unipro_params(hba);
> -
>  	/* UFS device is also active now */
>  	ufshcd_set_ufs_dev_active(hba);
>  	ufshcd_force_reset_auto_bkops(hba);
> diff --git a/drivers/scsi/ufs/ufshcd.h b/drivers/scsi/ufs/ufshcd.h
> index e05cafddc87b..4f3fa71303da 100644
> --- a/drivers/scsi/ufs/ufshcd.h
> +++ b/drivers/scsi/ufs/ufshcd.h
> @@ -320,7 +320,7 @@ struct ufs_hba_variant_ops {
>  	void	(*setup_task_mgmt)(struct ufs_hba *, int, u8);
>  	void    (*hibern8_notify)(struct ufs_hba *, enum uic_cmd_dme,
>  					enum ufs_notify_change_status);
> -	int	(*apply_dev_quirks)(struct ufs_hba *);
> +	int	(*apply_dev_quirks)(struct ufs_hba *, struct ufs_dev_desc *);
>  	int     (*suspend)(struct ufs_hba *, enum ufs_pm_op);
>  	int     (*resume)(struct ufs_hba *, enum ufs_pm_op);
>  	void	(*dbg_register_dump)(struct ufs_hba *hba);
> @@ -1052,10 +1052,11 @@ static inline void
> ufshcd_vops_hibern8_notify(struct ufs_hba *hba,
>  		return hba->vops->hibern8_notify(hba, cmd, status);
>  }
>
> -static inline int ufshcd_vops_apply_dev_quirks(struct ufs_hba *hba)
> +static inline int ufshcd_vops_apply_dev_quirks(struct ufs_hba *hba,
> +					       struct ufs_dev_desc *card)
>  {
>  	if (hba->vops && hba->vops->apply_dev_quirks)
> -		return hba->vops->apply_dev_quirks(hba);
> +		return hba->vops->apply_dev_quirks(hba, card);
>  	return 0;
>  }