| Message ID | 20220726113653.25024-1-peter.wang@mediatek.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | [v4] ufs: core: correct ufshcd_shutdown flow | expand |
On Tue, Jul 26, 2022 at 07:36:53PM +0800, peter.wang@mediatek.com wrote: > From: Peter Wang <peter.wang@mediatek.com> > > In normal case: ufshcd_wl_shutdown -> ufshcd_shtdown > ufshcd_shtdown should turn off clock/power after ufshcd_wl_shutdown > which set device power off and link off. > > Also remove pm_runtime_get_sync. > The reason why here can remove pm_runtime_get_sync is because, > (1) ufshcd_wl_shutdown -> pm_runtime_get_sync, will resume hba->dev too. > (2) device resume(turn on clk/power) is not required, even if device is in RPM_SUSPENDED. > > Signed-off-by: Peter Wang <peter.wang@mediatek.com> > --- > drivers/ufs/core/ufshcd.c | 6 +----- > 1 file changed, 1 insertion(+), 5 deletions(-) > > diff --git a/drivers/ufs/core/ufshcd.c b/drivers/ufs/core/ufshcd.c > index c7b337480e3e..d13c76983555 100644 > --- a/drivers/ufs/core/ufshcd.c > +++ b/drivers/ufs/core/ufshcd.c > @@ -9462,12 +9462,8 @@ EXPORT_SYMBOL(ufshcd_runtime_resume); > int ufshcd_shutdown(struct ufs_hba *hba) > { > if (ufshcd_is_ufs_dev_poweroff(hba) && ufshcd_is_link_off(hba)) > - goto out; > - > - pm_runtime_get_sync(hba->dev); > + ufshcd_suspend(hba); > > - ufshcd_suspend(hba); > -out: > hba->is_powered = false; > /* allow force shutdown even in case of errors */ > return 0; > -- > 2.18.0 > <formletter> This is not the correct way to submit patches for inclusion in the stable kernel tree. Please read: https://www.kernel.org/doc/html/latest/process/stable-kernel-rules.html for how to do this properly. </formletter>
On 7/26/22 04:36, peter.wang@mediatek.com wrote: > In normal case: ufshcd_wl_shutdown -> ufshcd_shtdown > ufshcd_shtdown should turn off clock/power after ufshcd_wl_shutdown > which set device power off and link off. The above sentence is confusing: it mentions "normal case" which suggests there is also an abnormal case. Isn't the order in which ufshcd_wl_shutdown() and ufshcd_shutdown() are called always the same? Otherwise this patch looks good to me. Thanks, Bart.
On 7/27/22 2:34 AM, Bart Van Assche wrote: > On 7/26/22 04:36, peter.wang@mediatek.com wrote: >> In normal case: ufshcd_wl_shutdown -> ufshcd_shtdown >> ufshcd_shtdown should turn off clock/power after ufshcd_wl_shutdown >> which set device power off and link off. > > The above sentence is confusing: it mentions "normal case" which > suggests there is also an abnormal case. Isn't the order in which > ufshcd_wl_shutdown() and ufshcd_shutdown() are called always the same? > > Otherwise this patch looks good to me. > > Thanks, > > Bart. Hi Bart, Yes, You are right, I' ll make patch description more clear. Thanks Peter
diff --git a/drivers/ufs/core/ufshcd.c b/drivers/ufs/core/ufshcd.c index c7b337480e3e..d13c76983555 100644 --- a/drivers/ufs/core/ufshcd.c +++ b/drivers/ufs/core/ufshcd.c @@ -9462,12 +9462,8 @@ EXPORT_SYMBOL(ufshcd_runtime_resume); int ufshcd_shutdown(struct ufs_hba *hba) { if (ufshcd_is_ufs_dev_poweroff(hba) && ufshcd_is_link_off(hba)) - goto out; - - pm_runtime_get_sync(hba->dev); + ufshcd_suspend(hba); - ufshcd_suspend(hba); -out: hba->is_powered = false; /* allow force shutdown even in case of errors */ return 0;