| Message ID | 20230816195447.3703954-15-bvanassche@acm.org |
|---|---|
| State | Superseded |
| Headers | show |
| Series | Improve performance for zoned UFS devices | expand |
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);
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.
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 --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);
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(-)