On Fri, 2024-07-12 at 10:34 -0700, Bart Van Assche wrote:
- /* Update RTC only when there are no requests in progress and UFSHCI is operational */ - if (!ufshcd_is_ufs_dev_busy(hba) && hba->ufshcd_state == UFSHCD_STATE_OPERATIONAL) + /* + * Update RTC only when + * 1. there are no requests in progress + * 2. UFSHCI is operational + * 3. pm operation is not in progress + */ + if (!ufshcd_is_ufs_dev_busy(hba) && + hba->ufshcd_state == UFSHCD_STATE_OPERATIONAL && + !hba->pm_op_in_progress) ufshcd_update_rtc(hba); if (ufshcd_is_ufs_dev_active(hba) && hba-
dev_info.rtc_update_period)
The above seems racy to me. I don't think there is any mechanism that prevents that hba->pm_op_in_progress is set after it has been checked and before ufshcd_update_rtc() is called. Has it been considered to add an ufshcd_rpm_get_sync_nowait() call before the hba-
pm_op_in_progress
check and a ufshcd_rpm_put_sync() call after the ufshcd_update_rtc() call?
Thanks,
Bart.
Bart,
do you want this:
diff --git a/drivers/ufs/core/ufshcd-priv.h b/drivers/ufs/core/ufshcd- priv.h index ce36154ce963..2b74d6329b9d 100644 --- a/drivers/ufs/core/ufshcd-priv.h +++ b/drivers/ufs/core/ufshcd-priv.h @@ -311,6 +311,25 @@ static inline int ufshcd_update_ee_usr_mask(struct ufs_hba *hba, &hba->ee_drv_mask, set, clr); }
+static inline int ufshcd_rpm_get_sync_nowait(struct ufs_hba *hba) +{ + int ret = 0; + struct device *dev = &hba->ufs_device_wlun->sdev_gendev; + + pm_runtime_get_noresume(dev); + + /* Check if the device is already active */ + if (pm_runtime_active(dev)) + return 0; + + /* Attempt to resume the device without blocking */ + ret = pm_request_resume(dev); + if (ret < 0) + pm_runtime_put_noidle(dev); + + return ret; +} + static inline int ufshcd_rpm_get_sync(struct ufs_hba *hba) { return pm_runtime_get_sync(&hba->ufs_device_wlun->sdev_gendev); diff --git a/drivers/ufs/core/ufshcd.c b/drivers/ufs/core/ufshcd.c index bea00e069e9a..1b7fc4ce9e5c 100644 --- a/drivers/ufs/core/ufshcd.c +++ b/drivers/ufs/core/ufshcd.c @@ -8209,10 +8209,8 @@ static void ufshcd_update_rtc(struct ufs_hba *hba) */ val = ts64.tv_sec - hba->dev_info.rtc_time_baseline;
- ufshcd_rpm_get_sync(hba); err = ufshcd_query_attr(hba, UPIU_QUERY_OPCODE_WRITE_ATTR, QUERY_ATTR_IDN_SECONDS_PASSED, 0, 0, &val); - ufshcd_rpm_put_sync(hba);
if (err) dev_err(hba->dev, "%s: Failed to update rtc %d\n", __func__, err); @@ -8226,10 +8224,14 @@ static void ufshcd_rtc_work(struct work_struct *work)
hba = container_of(to_delayed_work(work), struct ufs_hba, ufs_rtc_update_work);
+ if (ufshcd_rpm_get_sync_nowait(hba)) + goto out; + /* Update RTC only when there are no requests in progress and UFSHCI is operational */ if (!ufshcd_is_ufs_dev_busy(hba) && hba->ufshcd_state == UFSHCD_STATE_OPERATIONAL) ufshcd_update_rtc(hba); - + ufshcd_rpm_put_sync(hba); +out: if (ufshcd_is_ufs_dev_active(hba) && hba-
dev_info.rtc_update_period)
schedule_delayed_work(&hba->ufs_rtc_update_work, msecs_to_jiffies(hba-
dev_info.rtc_update_period));
(END)
or can we change cancel_delayed_work_sync(&hba->ufs_rtc_update_work); to cancel_delayed_work(&hba->ufs_rtc_update_work); ??
kind regards, Bean