| Message ID | 1590726569-28248-1-git-send-email-macpaul.lin@mediatek.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | usb: host: xhci-mtk: avoid runtime suspend when removing hcd | expand |
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
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 --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; }
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(-)