Re: [PATCH v9 14/17] scsi: ufs: Rename ufshcd_auto_hibern8_enable() and make it static

Bao D. Nguyen
2023-08-17T18:51:04Z

On 8/16/2023 12:53 PM, Bart Van Assche wrote:
> Rename ufshcd_auto_hibern8_enable() into ufshcd_configure_auto_hibern8()
> since this function can enable or disable auto-hibernation. Since
> ufshcd_auto_hibern8_enable() is only used inside the UFSHCI driver core,
> declare it static. Additionally, move the definition of this function to
> just before its first caller.
>
> Suggested-by: Bao D. Nguyen <quic_nguyenb@quicinc.com>
> Signed-off-by: Bart Van Assche <bvanassche@acm.org>
> ---
> drivers/ufs/core/ufshcd.c | 24 +++++++++++-------------
> include/ufs/ufshcd.h | 1 -
> 2 files changed, 11 insertions(+), 14 deletions(-)
>
> diff --git a/drivers/ufs/core/ufshcd.c b/drivers/ufs/core/ufshcd.c
> index 129446775796..f1bba459b46f 100644
> --- a/drivers/ufs/core/ufshcd.c
> +++ b/drivers/ufs/core/ufshcd.c
> @@ -4337,6 +4337,14 @@ int ufshcd_uic_hibern8_exit(struct ufs_hba *hba)
> }
> EXPORT_SYMBOL_GPL(ufshcd_uic_hibern8_exit);
>
> +static void ufshcd_configure_auto_hibern8(struct ufs_hba *hba)
> +{
> + if (!ufshcd_is_auto_hibern8_supported(hba))
> + return;
> +
> + ufshcd_writel(hba, hba->ahit, REG_AUTO_HIBERNATE_IDLE_TIMER);
> +}
> +
> void ufshcd_auto_hibern8_update(struct ufs_hba *hba, u32 ahit)
> {
> unsigned long flags;
> @@ -4356,21 +4364,13 @@ void ufshcd_auto_hibern8_update(struct ufs_hba *hba, u32 ahit)
> !pm_runtime_suspended(&hba->ufs_device_wlun->sdev_gendev)) {
> ufshcd_rpm_get_sync(hba);
> ufshcd_hold(hba);
> - ufshcd_auto_hibern8_enable(hba);
> + ufshcd_configure_auto_hibern8(hba);
> ufshcd_release(hba);
> ufshcd_rpm_put_sync(hba);
> }
> }
> EXPORT_SYMBOL_GPL(ufshcd_auto_hibern8_update);
>
> -void ufshcd_auto_hibern8_enable(struct ufs_hba *hba)
> -{
> - if (!ufshcd_is_auto_hibern8_supported(hba))
> - return;
> -
> - ufshcd_writel(hba, hba->ahit, REG_AUTO_HIBERNATE_IDLE_TIMER);
> -}
> -
> /**
> * ufshcd_init_pwr_info - setting the POR (power on reset)
> * values in hba power info
> @@ -8815,8 +8815,7 @@ static int ufshcd_probe_hba(struct ufs_hba *hba, bool init_dev_params)
>
> if (hba->ee_usr_mask)
> ufshcd_write_ee_control(hba);
> - /* Enable Auto-Hibernate if configured */
> - ufshcd_auto_hibern8_enable(hba);
> + ufshcd_configure_auto_hibern8(hba);
>
> ufshpb_toggle_state(hba, HPB_RESET, HPB_PRESENT);
> out:
> @@ -9809,8 +9808,7 @@ static int __ufshcd_wl_resume(struct ufs_hba *hba, enum ufs_pm_op pm_op)
> cancel_delayed_work(&hba->rpm_dev_flush_recheck_work);
> }
>
> - /* Enable Auto-Hibernate if configured */
> - ufshcd_auto_hibern8_enable(hba);
> + ufshcd_configure_auto_hibern8(hba);
Is it possible to have a race between sysfs and syspend/resume trying to
update the auto_hibern8?

>
> ufshpb_resume(hba);
> goto out;
> diff --git a/include/ufs/ufshcd.h b/include/ufs/ufshcd.h
> index 6dc11fa0ebb1..040d66d99912 100644
> --- a/include/ufs/ufshcd.h
> +++ b/include/ufs/ufshcd.h
> @@ -1363,7 +1363,6 @@ static inline int ufshcd_disable_host_tx_lcc(struct ufs_hba *hba)
> return ufshcd_dme_set(hba, UIC_ARG_MIB(PA_LOCAL_TX_LCC_ENABLE), 0);
> }
>
> -void ufshcd_auto_hibern8_enable(struct ufs_hba *hba);
> void ufshcd_auto_hibern8_update(struct ufs_hba *hba, u32 ahit);
> void ufshcd_fixup_dev_quirks(struct ufs_hba *hba,
> const struct ufs_dev_quirk *fixups);