On 4/15/2021 4:11 PM, Bart Van Assche wrote:
On 4/14/21 11:58 AM, Asutosh Das wrote:
[ ... ]
Hi Bart,
Thanks for the comments. I will fix the comments in the next version.
The following code is executed before ufshcd_async_scan() is called:
dev = hba->dev;
[ ... ]
/* Hold auto suspend until async scan completes */
pm_runtime_get_sync(dev);
That would only keep the hba runtime resumed. At this point of time the
luns are not detected yet.
and the following code occurs in ufshcd_add_lus():
pm_runtime_put_sync(hba->dev);
Isn't that sufficient to postpone enabling of runtime PM until LUN
scanning has finished? Or in other words, is adding a
pm_runtime_get_noresume() call in ufshcd_slave_configure() really necessary?
Yes, because the supplier (device wlun) may be suspended otherwise in
scsi_sysfs_add_sdev().
@@ -4979,15 +5035,9 @@ ufshcd_transfer_rsp_status(struct ufs_hba *hba, struct ufshcd_lrb *lrbp)
*/
if (!hba->pm_op_in_progress &&
!ufshcd_eh_in_progress(hba) &&
- ufshcd_is_exception_event(lrbp->ucd_rsp_ptr) &&
- schedule_work(&hba->eeh_work)) {
- /*
- * Prevent suspend once eeh_work is scheduled
- * to avoid deadlock between ufshcd_suspend
- * and exception event handler.
- */
- pm_runtime_get_noresume(hba->dev);
- }
+ ufshcd_is_exception_event(lrbp->ucd_rsp_ptr))
+ /* Flushed in suspend */
+ schedule_work(&hba->eeh_work);
What makes it safe to leave out the above pm_runtime_get_noresume() call?
The __ufshcd_wl_suspend() would flush this work so that it doesn't run
after suspend.
Thanks,
Bart.
--
The Qualcomm Innovation Center, Inc. is a member of the Code Aurora Forum,
Linux Foundation Collaborative Project