diff mbox series

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

Message ID 20230816195447.3703954-15-bvanassche@acm.org (mailing list archive)
State Superseded
Headers show
Series Improve performance for zoned UFS devices | expand

Commit Message

Bart Van Assche Aug. 16, 2023, 7:53 p.m. UTC
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(-)

Comments

Bao D. Nguyen Aug. 17, 2023, 6:49 p.m. UTC | #1
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);
Bart Van Assche Aug. 17, 2023, 7:16 p.m. UTC | #2
On 8/17/23 11:49, Bao D. Nguyen wrote:
> On 8/16/2023 12:53 PM, Bart Van Assche wrote:
>> -    /* 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?

Only user-space software should write into sysfs. Kernel code should
not do this. User-space code is paused before the block driver suspend
callbacks are invoked and resuming block devices completes before
user space software is resumed. I don't think that sysfs writes can
race with suspend or resume callbacks.

Thanks,

Bart.
Bao D. Nguyen Aug. 17, 2023, 9:48 p.m. UTC | #3
On 8/17/2023 12:16 PM, Bart Van Assche wrote:
> On 8/17/23 11:49, Bao D. Nguyen wrote:
>> On 8/16/2023 12:53 PM, Bart Van Assche wrote:
>>> -    /* 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?
> 
> Only user-space software should write into sysfs. Kernel code should
> not do this. User-space code is paused before the block driver suspend
> callbacks are invoked and resuming block devices completes before
> user space software is resumed. I don't think that sysfs writes can
> race with suspend or resume callbacks.
>Thank you for the explanation.

Thanks
Bao
> Thanks,
> 
> Bart.
>
diff mbox series

Patch

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);
 
 	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);