diff mbox series

usb: host: xhci-mtk: avoid runtime suspend when removing hcd

Message ID 1590726569-28248-1-git-send-email-macpaul.lin@mediatek.com (mailing list archive)
State Superseded
Headers show
Series usb: host: xhci-mtk: avoid runtime suspend when removing hcd | expand

Commit Message

Macpaul Lin May 29, 2020, 4:29 a.m. UTC
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(-)

Comments

Mathias Nyman June 3, 2020, 11:47 a.m. UTC | #1
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
Macpaul Lin June 3, 2020, 1:01 p.m. UTC | #2
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
diff mbox series

Patch

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;
 }