diff mbox

usb: musb: Fix external abort in musb_remove

Message ID 20180308204049.29474-1-merlijn@wizzup.org (mailing list archive)
State New, archived
Headers show

Commit Message

Merlijn Wajer March 8, 2018, 8:40 p.m. UTC
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(-)

Comments

Bin Liu March 8, 2018, 9:15 p.m. UTC | #1
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.

--
To unsubscribe from this list: send the line "unsubscribe linux-omap" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Bin Liu March 8, 2018, 9:18 p.m. UTC | #2
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.
--
To unsubscribe from this list: send the line "unsubscribe linux-omap" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Merlijn Wajer March 8, 2018, 10:17 p.m. UTC | #3
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
Bin Liu March 9, 2018, 1:39 p.m. UTC | #4
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.
--
To unsubscribe from this list: send the line "unsubscribe linux-omap" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
diff mbox

Patch

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