linux-kernel.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [PATCH] usb: host: xhci-mtk: avoid runtime suspend when removing hcd
@ 2020-05-29  4:29 Macpaul Lin
  2020-05-29  4:32 ` [PATCH v2] " Macpaul Lin
  2020-06-03 11:47 ` [PATCH] " Mathias Nyman
  0 siblings, 2 replies; 9+ messages in thread
From: Macpaul Lin @ 2020-05-29  4:29 UTC (permalink / raw)
  To: Chunfeng Yun, Mathias Nyman, Greg Kroah-Hartman, Matthias Brugger
  Cc: Mediatek WSD Upstream, Macpaul Lin, Macpaul Lin, linux-kernel,
	linux-arm-kernel, linux-usb, linux-mediatek

When runtime suspend was enabled, runtime suspend might happened
when xhci is removing hcd. This might cause kernel panic when hcd
has been freed but runtime pm suspend related handle need to
reference it.

Change-Id: I70a5dc8006207caeecbac6955ce8e5345dcc70e6
Signed-off-by: Macpaul Lin <macpaul.lin@mediatek.com>
---
 drivers/usb/host/xhci-mtk.c |    5 +++--
 1 file changed, 3 insertions(+), 2 deletions(-)

diff --git a/drivers/usb/host/xhci-mtk.c b/drivers/usb/host/xhci-mtk.c
index bfbdb3c..641d24e 100644
--- a/drivers/usb/host/xhci-mtk.c
+++ b/drivers/usb/host/xhci-mtk.c
@@ -587,6 +587,9 @@ static int xhci_mtk_remove(struct platform_device *dev)
 	struct xhci_hcd	*xhci = hcd_to_xhci(hcd);
 	struct usb_hcd  *shared_hcd = xhci->shared_hcd;
 
+	pm_runtime_put_sync(&dev->dev);
+	pm_runtime_disable(&dev->dev);
+
 	usb_remove_hcd(shared_hcd);
 	xhci->shared_hcd = NULL;
 	device_init_wakeup(&dev->dev, false);
@@ -597,8 +600,6 @@ static int xhci_mtk_remove(struct platform_device *dev)
 	xhci_mtk_sch_exit(mtk);
 	xhci_mtk_clks_disable(mtk);
 	xhci_mtk_ldos_disable(mtk);
-	pm_runtime_put_sync(&dev->dev);
-	pm_runtime_disable(&dev->dev);
 
 	return 0;
 }
-- 
1.7.9.5

^ permalink raw reply related	[flat|nested] 9+ messages in thread

* [PATCH v2] usb: host: xhci-mtk: avoid runtime suspend when removing hcd
  2020-05-29  4:29 [PATCH] usb: host: xhci-mtk: avoid runtime suspend when removing hcd Macpaul Lin
@ 2020-05-29  4:32 ` Macpaul Lin
  2020-06-03  8:43   ` Chunfeng Yun
  2020-06-03 11:47 ` [PATCH] " Mathias Nyman
  1 sibling, 1 reply; 9+ messages in thread
From: Macpaul Lin @ 2020-05-29  4:32 UTC (permalink / raw)
  To: Chunfeng Yun, Mathias Nyman, Greg Kroah-Hartman, Matthias Brugger
  Cc: Mediatek WSD Upstream, Macpaul Lin, Macpaul Lin, linux-kernel,
	linux-arm-kernel, linux-usb, linux-mediatek

When runtime suspend was enabled, runtime suspend might happened
when xhci is removing hcd. This might cause kernel panic when hcd
has been freed but runtime pm suspend related handle need to
reference it.

Signed-off-by: Macpaul Lin <macpaul.lin@mediatek.com>
---
 drivers/usb/host/xhci-mtk.c |    5 +++--
 1 file changed, 3 insertions(+), 2 deletions(-)

diff --git a/drivers/usb/host/xhci-mtk.c b/drivers/usb/host/xhci-mtk.c
index bfbdb3c..641d24e 100644
--- a/drivers/usb/host/xhci-mtk.c
+++ b/drivers/usb/host/xhci-mtk.c
@@ -587,6 +587,9 @@ static int xhci_mtk_remove(struct platform_device *dev)
 	struct xhci_hcd	*xhci = hcd_to_xhci(hcd);
 	struct usb_hcd  *shared_hcd = xhci->shared_hcd;
 
+	pm_runtime_put_sync(&dev->dev);
+	pm_runtime_disable(&dev->dev);
+
 	usb_remove_hcd(shared_hcd);
 	xhci->shared_hcd = NULL;
 	device_init_wakeup(&dev->dev, false);
@@ -597,8 +600,6 @@ static int xhci_mtk_remove(struct platform_device *dev)
 	xhci_mtk_sch_exit(mtk);
 	xhci_mtk_clks_disable(mtk);
 	xhci_mtk_ldos_disable(mtk);
-	pm_runtime_put_sync(&dev->dev);
-	pm_runtime_disable(&dev->dev);
 
 	return 0;
 }
-- 
1.7.9.5

^ permalink raw reply related	[flat|nested] 9+ messages in thread

* Re: [PATCH v2] usb: host: xhci-mtk: avoid runtime suspend when removing hcd
  2020-05-29  4:32 ` [PATCH v2] " Macpaul Lin
@ 2020-06-03  8:43   ` Chunfeng Yun
  0 siblings, 0 replies; 9+ messages in thread
From: Chunfeng Yun @ 2020-06-03  8:43 UTC (permalink / raw)
  To: Macpaul Lin
  Cc: Mathias Nyman, Greg Kroah-Hartman, Matthias Brugger,
	Mediatek WSD Upstream, Macpaul Lin, linux-kernel,
	linux-arm-kernel, linux-usb, linux-mediatek

On Fri, 2020-05-29 at 12:32 +0800, Macpaul Lin wrote:
> When runtime suspend was enabled, runtime suspend might happened
> when xhci is removing hcd. This might cause kernel panic when hcd
> has been freed but runtime pm suspend related handle need to
> reference it.
> 
> Signed-off-by: Macpaul Lin <macpaul.lin@mediatek.com>
> ---
>  drivers/usb/host/xhci-mtk.c |    5 +++--
>  1 file changed, 3 insertions(+), 2 deletions(-)
> 
> diff --git a/drivers/usb/host/xhci-mtk.c b/drivers/usb/host/xhci-mtk.c
> index bfbdb3c..641d24e 100644
> --- a/drivers/usb/host/xhci-mtk.c
> +++ b/drivers/usb/host/xhci-mtk.c
> @@ -587,6 +587,9 @@ static int xhci_mtk_remove(struct platform_device *dev)
>  	struct xhci_hcd	*xhci = hcd_to_xhci(hcd);
>  	struct usb_hcd  *shared_hcd = xhci->shared_hcd;
>  
> +	pm_runtime_put_sync(&dev->dev);
> +	pm_runtime_disable(&dev->dev);
> +
>  	usb_remove_hcd(shared_hcd);
>  	xhci->shared_hcd = NULL;
>  	device_init_wakeup(&dev->dev, false);
> @@ -597,8 +600,6 @@ static int xhci_mtk_remove(struct platform_device *dev)
>  	xhci_mtk_sch_exit(mtk);
>  	xhci_mtk_clks_disable(mtk);
>  	xhci_mtk_ldos_disable(mtk);
> -	pm_runtime_put_sync(&dev->dev);
> -	pm_runtime_disable(&dev->dev);
>  
>  	return 0;
>  }

Reviewed-by: Chunfeng Yun <chunfeng.yun@mediatek.com>

Thanks



^ permalink raw reply	[flat|nested] 9+ messages in thread

* Re: [PATCH] usb: host: xhci-mtk: avoid runtime suspend when removing hcd
  2020-05-29  4:29 [PATCH] usb: host: xhci-mtk: avoid runtime suspend when removing hcd Macpaul Lin
  2020-05-29  4:32 ` [PATCH v2] " Macpaul Lin
@ 2020-06-03 11:47 ` Mathias Nyman
  2020-06-03 13:01   ` Macpaul Lin
  2020-06-03 13:09   ` [PATCH v3] " Macpaul Lin
  1 sibling, 2 replies; 9+ messages in thread
From: Mathias Nyman @ 2020-06-03 11:47 UTC (permalink / raw)
  To: Macpaul Lin, Chunfeng Yun, Mathias Nyman, Greg Kroah-Hartman,
	Matthias Brugger
  Cc: Mediatek WSD Upstream, Macpaul Lin, linux-kernel,
	linux-arm-kernel, linux-usb, linux-mediatek

On 29.5.2020 7.29, Macpaul Lin wrote:
> When runtime suspend was enabled, runtime suspend might happened
> when xhci is removing hcd. This might cause kernel panic when hcd
> has been freed but runtime pm suspend related handle need to
> reference it.
> 
> Change-Id: I70a5dc8006207caeecbac6955ce8e5345dcc70e6
> Signed-off-by: Macpaul Lin <macpaul.lin@mediatek.com>
> ---
>  drivers/usb/host/xhci-mtk.c |    5 +++--
>  1 file changed, 3 insertions(+), 2 deletions(-)
> 
> diff --git a/drivers/usb/host/xhci-mtk.c b/drivers/usb/host/xhci-mtk.c
> index bfbdb3c..641d24e 100644
> --- a/drivers/usb/host/xhci-mtk.c
> +++ b/drivers/usb/host/xhci-mtk.c
> @@ -587,6 +587,9 @@ static int xhci_mtk_remove(struct platform_device *dev)
>  	struct xhci_hcd	*xhci = hcd_to_xhci(hcd);
>  	struct usb_hcd  *shared_hcd = xhci->shared_hcd;
>  
> +	pm_runtime_put_sync(&dev->dev);

Might runtime suspend here.
It's a lot better than before, no panic as hcd isn't released, but a bit unnecessary.

how about this sequence instead:
pm_runtime_disable()
pm_runtime_put_noidle()

> +	pm_runtime_disable(&dev->dev);
> +

-Mathias

^ permalink raw reply	[flat|nested] 9+ messages in thread

* Re: [PATCH] usb: host: xhci-mtk: avoid runtime suspend when removing hcd
  2020-06-03 11:47 ` [PATCH] " Mathias Nyman
@ 2020-06-03 13:01   ` Macpaul Lin
  2020-06-03 13:09   ` [PATCH v3] " Macpaul Lin
  1 sibling, 0 replies; 9+ messages in thread
From: Macpaul Lin @ 2020-06-03 13:01 UTC (permalink / raw)
  To: Mathias Nyman
  Cc: Chunfeng Yun, Mathias Nyman, Greg Kroah-Hartman,
	Matthias Brugger, Mediatek WSD Upstream, Macpaul Lin,
	linux-kernel, linux-arm-kernel, linux-usb, linux-mediatek

On Wed, 2020-06-03 at 14:47 +0300, Mathias Nyman wrote:
> On 29.5.2020 7.29, Macpaul Lin wrote:
> > When runtime suspend was enabled, runtime suspend might happened
> > when xhci is removing hcd. This might cause kernel panic when hcd
> > has been freed but runtime pm suspend related handle need to
> > reference it.
> > 
> > Change-Id: I70a5dc8006207caeecbac6955ce8e5345dcc70e6
> > Signed-off-by: Macpaul Lin <macpaul.lin@mediatek.com>
> > ---
> >  drivers/usb/host/xhci-mtk.c |    5 +++--
> >  1 file changed, 3 insertions(+), 2 deletions(-)
> > 
> > diff --git a/drivers/usb/host/xhci-mtk.c b/drivers/usb/host/xhci-mtk.c
> > index bfbdb3c..641d24e 100644
> > --- a/drivers/usb/host/xhci-mtk.c
> > +++ b/drivers/usb/host/xhci-mtk.c
> > @@ -587,6 +587,9 @@ static int xhci_mtk_remove(struct platform_device *dev)
> >  	struct xhci_hcd	*xhci = hcd_to_xhci(hcd);
> >  	struct usb_hcd  *shared_hcd = xhci->shared_hcd;
> >  
> > +	pm_runtime_put_sync(&dev->dev);
> 
> Might runtime suspend here.
> It's a lot better than before, no panic as hcd isn't released, but a bit unnecessary.
> 
> how about this sequence instead:
> pm_runtime_disable()
> pm_runtime_put_noidle()
> 
> > +	pm_runtime_disable(&dev->dev);
> > +
> 
> -Mathias

Thanks for your suggestion!
Will it better to put no idle before disable? 
pm_runtime_put_noidle()
pm_runtime_disable()

I've found pm_runtime_put_noidle is called in pm_runtime_disable() when
there is a pending request.

I will send patch v3 as noidle() called earlier than disable(). Please
help to comment it if disable() should go before.

Thanks!
Macpaul Lin 

^ permalink raw reply	[flat|nested] 9+ messages in thread

* [PATCH v3] usb: host: xhci-mtk: avoid runtime suspend when removing hcd
  2020-06-03 11:47 ` [PATCH] " Mathias Nyman
  2020-06-03 13:01   ` Macpaul Lin
@ 2020-06-03 13:09   ` Macpaul Lin
  2020-06-03 17:33     ` Sergei Shtylyov
  2020-06-04  3:01     ` [PATCH v4] " Macpaul Lin
  1 sibling, 2 replies; 9+ messages in thread
From: Macpaul Lin @ 2020-06-03 13:09 UTC (permalink / raw)
  To: Chunfeng Yun, Mathias Nyman, Greg Kroah-Hartman,
	Matthias Brugger, stable
  Cc: Mediatek WSD Upstream, Macpaul Lin, Macpaul Lin, linux-kernel,
	linux-arm-kernel, linux-usb, linux-mediatek

When runtime suspend was enabled, runtime suspend might happened
when xhci is removing hcd. This might cause kernel panic when hcd
has been freed but runtime pm suspend related handle need to
reference it.

Signed-off-by: Macpaul Lin <macpaul.lin@mediatek.com>
Reviewed-by: Chunfeng Yun <chunfeng.yun@mediatek.com>
---
Changes for v3:
  - Replace better sequence for disabling the pm_runtime suspend.

 drivers/usb/host/xhci-mtk.c |    5 +++--
 1 file changed, 3 insertions(+), 2 deletions(-)

diff --git a/drivers/usb/host/xhci-mtk.c b/drivers/usb/host/xhci-mtk.c
index bfbdb3c..641d24e 100644
--- a/drivers/usb/host/xhci-mtk.c
+++ b/drivers/usb/host/xhci-mtk.c
@@ -587,6 +587,9 @@ static int xhci_mtk_remove(struct platform_device *dev)
 	struct xhci_hcd	*xhci = hcd_to_xhci(hcd);
 	struct usb_hcd  *shared_hcd = xhci->shared_hcd;
 
+	pm_runtime_put_noidle(&dev->dev);
+	pm_runtime_disable(&dev->dev);
+
 	usb_remove_hcd(shared_hcd);
 	xhci->shared_hcd = NULL;
 	device_init_wakeup(&dev->dev, false);
@@ -597,8 +600,6 @@ static int xhci_mtk_remove(struct platform_device *dev)
 	xhci_mtk_sch_exit(mtk);
 	xhci_mtk_clks_disable(mtk);
 	xhci_mtk_ldos_disable(mtk);
-	pm_runtime_put_sync(&dev->dev);
-	pm_runtime_disable(&dev->dev);
 
 	return 0;
 }
-- 
1.7.9.5

^ permalink raw reply related	[flat|nested] 9+ messages in thread

* Re: [PATCH v3] usb: host: xhci-mtk: avoid runtime suspend when removing hcd
  2020-06-03 13:09   ` [PATCH v3] " Macpaul Lin
@ 2020-06-03 17:33     ` Sergei Shtylyov
  2020-06-04  3:01     ` [PATCH v4] " Macpaul Lin
  1 sibling, 0 replies; 9+ messages in thread
From: Sergei Shtylyov @ 2020-06-03 17:33 UTC (permalink / raw)
  To: Macpaul Lin, Chunfeng Yun, Mathias Nyman, Greg Kroah-Hartman,
	Matthias Brugger, stable
  Cc: Mediatek WSD Upstream, Macpaul Lin, linux-kernel,
	linux-arm-kernel, linux-usb, linux-mediatek

Hello.

On 03.06.2020 16:09, Macpaul Lin wrote:

> When runtime suspend was enabled, runtime suspend might happened

    Happen.

> when xhci is removing hcd. This might cause kernel panic when hcd
> has been freed but runtime pm suspend related handle need to
> reference it.
> 
> Signed-off-by: Macpaul Lin <macpaul.lin@mediatek.com>
> Reviewed-by: Chunfeng Yun <chunfeng.yun@mediatek.com>
[...]

MBR, Sergei

^ permalink raw reply	[flat|nested] 9+ messages in thread

* [PATCH v4] usb: host: xhci-mtk: avoid runtime suspend when removing hcd
  2020-06-03 13:09   ` [PATCH v3] " Macpaul Lin
  2020-06-03 17:33     ` Sergei Shtylyov
@ 2020-06-04  3:01     ` Macpaul Lin
  2020-06-04  9:14       ` Mathias Nyman
  1 sibling, 1 reply; 9+ messages in thread
From: Macpaul Lin @ 2020-06-04  3:01 UTC (permalink / raw)
  To: Chunfeng Yun, Mathias Nyman, Greg Kroah-Hartman, Matthias Brugger
  Cc: Mediatek WSD Upstream, Macpaul Lin, Macpaul Lin, linux-kernel,
	linux-arm-kernel, linux-usb, linux-mediatek, stable

When runtime suspend was enabled, runtime suspend might happen
when xhci is removing hcd. This might cause kernel panic when hcd
has been freed but runtime pm suspend related handle need to
reference it.

Signed-off-by: Macpaul Lin <macpaul.lin@mediatek.com>
Reviewed-by: Chunfeng Yun <chunfeng.yun@mediatek.com>
Cc: stable@vger.kernel.org
---
Changes for v3:
  - Replace better sequence for disabling the pm_runtime suspend.
Changes for v4:
  - Thanks for Sergei's review, typo in commit description has been corrected.

 drivers/usb/host/xhci-mtk.c |    5 +++--
 1 file changed, 3 insertions(+), 2 deletions(-)

diff --git a/drivers/usb/host/xhci-mtk.c b/drivers/usb/host/xhci-mtk.c
index bfbdb3c..641d24e 100644
--- a/drivers/usb/host/xhci-mtk.c
+++ b/drivers/usb/host/xhci-mtk.c
@@ -587,6 +587,9 @@ static int xhci_mtk_remove(struct platform_device *dev)
 	struct xhci_hcd	*xhci = hcd_to_xhci(hcd);
 	struct usb_hcd  *shared_hcd = xhci->shared_hcd;
 
+	pm_runtime_put_noidle(&dev->dev);
+	pm_runtime_disable(&dev->dev);
+
 	usb_remove_hcd(shared_hcd);
 	xhci->shared_hcd = NULL;
 	device_init_wakeup(&dev->dev, false);
@@ -597,8 +600,6 @@ static int xhci_mtk_remove(struct platform_device *dev)
 	xhci_mtk_sch_exit(mtk);
 	xhci_mtk_clks_disable(mtk);
 	xhci_mtk_ldos_disable(mtk);
-	pm_runtime_put_sync(&dev->dev);
-	pm_runtime_disable(&dev->dev);
 
 	return 0;
 }
-- 
1.7.9.5

^ permalink raw reply related	[flat|nested] 9+ messages in thread

* Re: [PATCH v4] usb: host: xhci-mtk: avoid runtime suspend when removing hcd
  2020-06-04  3:01     ` [PATCH v4] " Macpaul Lin
@ 2020-06-04  9:14       ` Mathias Nyman
  0 siblings, 0 replies; 9+ messages in thread
From: Mathias Nyman @ 2020-06-04  9:14 UTC (permalink / raw)
  To: Macpaul Lin, Chunfeng Yun, Mathias Nyman, Greg Kroah-Hartman,
	Matthias Brugger
  Cc: Mediatek WSD Upstream, Macpaul Lin, linux-kernel,
	linux-arm-kernel, linux-usb, linux-mediatek, stable

On 4.6.2020 6.01, Macpaul Lin wrote:
> When runtime suspend was enabled, runtime suspend might happen
> when xhci is removing hcd. This might cause kernel panic when hcd
> has been freed but runtime pm suspend related handle need to
> reference it.
> 
> Signed-off-by: Macpaul Lin <macpaul.lin@mediatek.com>
> Reviewed-by: Chunfeng Yun <chunfeng.yun@mediatek.com>
> Cc: stable@vger.kernel.org
> ---
> Changes for v3:
>   - Replace better sequence for disabling the pm_runtime suspend.
> Changes for v4:
>   - Thanks for Sergei's review, typo in commit description has been corrected.
> 

Thanks, added to queue

-Mathias

^ permalink raw reply	[flat|nested] 9+ messages in thread

end of thread, other threads:[~2020-06-04  9:11 UTC | newest]

Thread overview: 9+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2020-05-29  4:29 [PATCH] usb: host: xhci-mtk: avoid runtime suspend when removing hcd Macpaul Lin
2020-05-29  4:32 ` [PATCH v2] " Macpaul Lin
2020-06-03  8:43   ` Chunfeng Yun
2020-06-03 11:47 ` [PATCH] " Mathias Nyman
2020-06-03 13:01   ` Macpaul Lin
2020-06-03 13:09   ` [PATCH v3] " Macpaul Lin
2020-06-03 17:33     ` Sergei Shtylyov
2020-06-04  3:01     ` [PATCH v4] " Macpaul Lin
2020-06-04  9:14       ` Mathias Nyman

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).