diff mbox series

[v4] ufs: core: correct ufshcd_shutdown flow

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

Commit Message

Peter Wang (王信友) July 26, 2022, 11:36 a.m. UTC
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(-)

Comments

Greg KH July 26, 2022, 3:05 p.m. UTC | #1
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>
Bart Van Assche July 26, 2022, 6:34 p.m. UTC | #2
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.
Peter Wang (王信友) July 27, 2022, 2:34 a.m. UTC | #3
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 mbox series

Patch

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;