2020-05-29 04:32:11

by Macpaul Lin

Subject: [PATCH] usb: host: xhci-mtk: avoid runtime suspend when removing hcd

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 <[email protected]>
---
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

2020-05-29 04:37:32

by Macpaul Lin

Subject: [PATCH v2] usb: host: xhci-mtk: avoid runtime suspend when removing hcd

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 <[email protected]>
---
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

2020-06-03 08:47:57

by Chunfeng Yun

Subject: Re: [PATCH v2] usb: host: xhci-mtk: avoid runtime suspend when removing hcd

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 <[email protected]>
> ---
> 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 <[email protected]>

Thanks

2020-06-03 11:46:59

by Mathias Nyman

Subject: Re: [PATCH] usb: host: xhci-mtk: avoid runtime suspend when removing hcd

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 <[email protected]>
> ---
> 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

2020-06-03 13:06:27

by Macpaul Lin

Subject: Re: [PATCH] usb: host: xhci-mtk: avoid runtime suspend when removing hcd

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 <[email protected]>
> > ---
> > 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

2020-06-03 13:14:31

by Macpaul Lin

Subject: [PATCH v3] usb: host: xhci-mtk: avoid runtime suspend when removing hcd

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 <[email protected]>
Reviewed-by: Chunfeng Yun <[email protected]>
---
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

2020-06-03 17:44:59

by Sergei Shtylyov

Subject: Re: [PATCH v3] usb: host: xhci-mtk: avoid runtime suspend when removing hcd

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 <[email protected]>
> Reviewed-by: Chunfeng Yun <[email protected]>
[...]

MBR, Sergei

2020-06-04 03:06:04

by Macpaul Lin

Subject: [PATCH v4] usb: host: xhci-mtk: avoid runtime suspend when removing hcd

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 <[email protected]>
Reviewed-by: Chunfeng Yun <[email protected]>
Cc: [email protected]
---
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

2020-06-04 09:13:53

by Mathias Nyman

Subject: Re: [PATCH v4] usb: host: xhci-mtk: avoid runtime suspend when removing hcd

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 <[email protected]>
> Reviewed-by: Chunfeng Yun <[email protected]>
> Cc: [email protected]
> ---
> 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