On 6/10/21 8:01 PM, Can Guo wrote: > Previously, without commit cb7e6f05fce67c965194ac04467e1ba7bc70b069, > ufshcd_resume() may turn off pwr and clk due to UFS error, e.g., link > transition failure and SSU error/abort (and these UFS error would > invoke error handling). When error handling kicks start, it should > re-enable the pwr and clk before proceeding. Now, commit > cb7e6f05fce67c965194ac04467e1ba7bc70b069 makes ufshcd_resume() > purely control pwr and clk, meaning if ufshcd_resume() fails, there > is nothing we can do about it - pwr or clk enabling must have failed, > and it is not because of UFS error. This is why I am removing the > re-enabling pwr/clk in error handling prepare. Why are link transition failures handled in the error handler instead of in the context where these errors are detected (ufshcd_resume())? Is it even possible to recover from a link transition failure or does this perhaps indicate a broken UFS controller? >> but what I really wonder is why we don't just do recovery directly >> in __ufshcd_wl_suspend() and __ufshcd_wl_resume() and strip all >> the PM complexity out of ufshcd_err_handling()? +1 > For system suspend/resume, since error handling has the same nature > like user access, so we are using host_sem to avoid concurrency of > error handling and system suspend/resume. Why is host_sem used for that purpose instead of lock_system_sleep() and unlock_system_sleep()? Thanks, Bart.