[PATCH] usb: musb: Fix external abort in musb_remove

[PATCH] usb: musb: Fix external abort in musb_remove

[Merlijn Wajer]

This fixes an oops on unbind / module unload.

musb_remove function now calls musb_platform_exit before disabling
runtime pm.

Signed-off-by: Merlijn Wajer <merlijn@wizzup.org>
---
 drivers/usb/musb/musb_core.c | 5 +++--
 1 file changed, 3 insertions(+), 2 deletions(-)

diff --git a/drivers/usb/musb/musb_core.c b/drivers/usb/musb/musb_core.c
index e2e95071328a..cf90d34f7199 100644
--- a/drivers/usb/musb/musb_core.c
+++ b/drivers/usb/musb/musb_core.c
@@ -2472,13 +2472,14 @@ static int musb_remove(struct platform_device *pdev)
 	musb_platform_disable(musb);
 	spin_lock_irqsave(&musb->lock, flags);
 	musb_disable_interrupts(musb);
-	musb_writeb(musb->mregs, MUSB_DEVCTL, 0);
 	spin_unlock_irqrestore(&musb->lock, flags);
 
+	musb_writeb(musb->mregs, MUSB_DEVCTL, 0);
+	musb_platform_exit(musb);
+
 	pm_runtime_dont_use_autosuspend(musb->controller);
 	pm_runtime_put_sync(musb->controller);
 	pm_runtime_disable(musb->controller);
-	musb_platform_exit(musb);
 	musb_phy_callback = NULL;
 	if (musb->dma_controller)
 		musb_dma_controller_destroy(musb->dma_controller);
-- 
2.16.2

Re: [PATCH] usb: musb: Fix external abort in musb_remove

[Bin Liu]

Hi,

please add patch version numbers in the subject when necessary. This
helps cross-referencing.

On Thu, Mar 08, 2018 at 09:40:47PM +0100, Merlijn Wajer wrote:
> This fixes an oops on unbind / module unload.
> 
> musb_remove function now calls musb_platform_exit before disabling
> runtime pm.
> 
> Signed-off-by: Merlijn Wajer <merlijn@wizzup.org>
> ---
>  drivers/usb/musb/musb_core.c | 5 +++--
>  1 file changed, 3 insertions(+), 2 deletions(-)
> 
> diff --git a/drivers/usb/musb/musb_core.c b/drivers/usb/musb/musb_core.c
> index e2e95071328a..cf90d34f7199 100644
> --- a/drivers/usb/musb/musb_core.c
> +++ b/drivers/usb/musb/musb_core.c
> @@ -2472,13 +2472,14 @@ static int musb_remove(struct platform_device *pdev)
>  	musb_platform_disable(musb);
>  	spin_lock_irqsave(&musb->lock, flags);
>  	musb_disable_interrupts(musb);
> -	musb_writeb(musb->mregs, MUSB_DEVCTL, 0);
>  	spin_unlock_irqrestore(&musb->lock, flags);
>  
> +	musb_writeb(musb->mregs, MUSB_DEVCTL, 0);

Does it solve the issue if not moving this line? I'd like to have
minimum change if possible.

> +	musb_platform_exit(musb);
> +
>  	pm_runtime_dont_use_autosuspend(musb->controller);
>  	pm_runtime_put_sync(musb->controller);
>  	pm_runtime_disable(musb->controller);
> -	musb_platform_exit(musb);
>  	musb_phy_callback = NULL;
>  	if (musb->dma_controller)
>  		musb_dma_controller_destroy(musb->dma_controller);

Regards,
-Bin.

Re: [PATCH] usb: musb: Fix external abort in musb_remove

[Bin Liu]

On Thu, Mar 08, 2018 at 09:40:47PM +0100, Merlijn Wajer wrote:
> This fixes an oops on unbind / module unload.

Please also mention here that it happens on omap2430 platform.

(omap2430 is the only platform affected by this bug.)

Regards,
-Bin.

Re: [PATCH] usb: musb: Fix external abort in musb_remove

[Merlijn Wajer]

[-- Attachment #1.1: Type: text/plain, Size: 645 bytes --]

Hi,

On 08/03/18 22:15, Bin Liu wrote:

> please add patch version numbers in the subject when necessary. This
> helps cross-referencing.

Will do. I naively assumed that the first patch would implicitly be
number 1. Will send out v2 now.

>>  
>> +	musb_writeb(musb->mregs, MUSB_DEVCTL, 0);
> 
> Does it solve the issue if not moving this line? I'd like to have
> minimum change if possible.

Yes, it does. The only reason I moved musb_writeb is because I
understood you wanted me to move both (per previous message: "This can
be move down to out side of holding the spinlock").

Thanks for the tips.

Cheers,
Merlijn


[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 228 bytes --]

Re: [PATCH] usb: musb: Fix external abort in musb_remove

[Bin Liu]

On Thu, Mar 08, 2018 at 11:17:48PM +0100, Merlijn Wajer wrote:
> Hi,
> 
> On 08/03/18 22:15, Bin Liu wrote:
> 
> > please add patch version numbers in the subject when necessary. This
> > helps cross-referencing.
> 
> Will do. I naively assumed that the first patch would implicitly be
> number 1. Will send out v2 now.

Sorry, my bad, I forgot the first patch was a RFC. You are doing it
right.

> 
> >>  
> >> +	musb_writeb(musb->mregs, MUSB_DEVCTL, 0);
> > 
> > Does it solve the issue if not moving this line? I'd like to have
> > minimum change if possible.
> 
> Yes, it does. The only reason I moved musb_writeb is because I
> understood you wanted me to move both (per previous message: "This can
> be move down to out side of holding the spinlock").

Typically the comments would only applies to the modified code.
Otherwise the comments have to be explicit to avoid confusion.
No worries here anyway, confusion does happen sometimes ;)

Regards,
-Bin.