2019-03-21 14:45:43

by Paul Cercueil

[permalink] [raw]
Subject: [PATCH] usb: musb: Force-disable pullup on shutdown

When the musb is shutdown, for instance when the driver is unloaded,
force-disable the pullup. Otherwise, the host will still see the gadget
device even after the shutdown.

Signed-off-by: Paul Cercueil <[email protected]>
---
drivers/usb/musb/musb_gadget.c | 7 +++++++
1 file changed, 7 insertions(+)

diff --git a/drivers/usb/musb/musb_gadget.c b/drivers/usb/musb/musb_gadget.c
index ffe462a657b1..a78a7391031b 100644
--- a/drivers/usb/musb/musb_gadget.c
+++ b/drivers/usb/musb/musb_gadget.c
@@ -1808,11 +1808,18 @@ int musb_gadget_setup(struct musb *musb)

void musb_gadget_cleanup(struct musb *musb)
{
+ unsigned long flags;
+
if (musb->port_mode == MUSB_HOST)
return;

cancel_delayed_work_sync(&musb->gadget_work);
usb_del_gadget_udc(&musb->g);
+
+ /* Force-disable the pull-up */
+ spin_lock_irqsave(&musb->lock, flags);
+ musb_pullup(musb, 0);
+ spin_unlock_irqrestore(&musb->lock, flags);
}

/*
--
2.11.0



2019-04-01 17:18:28

by Bin Liu

[permalink] [raw]
Subject: Re: [PATCH] usb: musb: Force-disable pullup on shutdown

On Thu, Mar 21, 2019 at 03:42:46PM +0100, Paul Cercueil wrote:
> When the musb is shutdown, for instance when the driver is unloaded,
> force-disable the pullup. Otherwise, the host will still see the gadget
> device even after the shutdown.

how would this happen?

when musb-hdrc driver is unloaded, udc core removes the bound gadget
driver which calls musb_gadget_pullup() to disable the pullup.

Regards,
-Bin.

2019-04-01 17:48:34

by Paul Cercueil

[permalink] [raw]
Subject: Re: [PATCH] usb: musb: Force-disable pullup on shutdown



Le lun. 1 avril 2019 ? 19:17, Bin Liu <[email protected]> a ?crit :
> On Thu, Mar 21, 2019 at 03:42:46PM +0100, Paul Cercueil wrote:
>> When the musb is shutdown, for instance when the driver is unloaded,
>> force-disable the pullup. Otherwise, the host will still see the
>> gadget
>> device even after the shutdown.
>
> how would this happen?
>
> when musb-hdrc driver is unloaded, udc core removes the bound gadget
> driver which calls musb_gadget_pullup() to disable the pullup.

I'm testing with the jz4740-musb driver. I don't unload the module (it's
built-in) but unbind it from sysfs.

> Regards,
> -Bin.


2019-04-01 18:22:39

by Bin Liu

[permalink] [raw]
Subject: Re: [PATCH] usb: musb: Force-disable pullup on shutdown

On Mon, Apr 01, 2019 at 07:46:22PM +0200, Paul Cercueil wrote:
>
>
> Le lun. 1 avril 2019 ? 19:17, Bin Liu <[email protected]> a ?crit :
> >On Thu, Mar 21, 2019 at 03:42:46PM +0100, Paul Cercueil wrote:
> >> When the musb is shutdown, for instance when the driver is unloaded,
> >> force-disable the pullup. Otherwise, the host will still see
> >>the gadget
> >> device even after the shutdown.
> >
> >how would this happen?
> >
> >when musb-hdrc driver is unloaded, udc core removes the bound gadget
> >driver which calls musb_gadget_pullup() to disable the pullup.
>
> I'm testing with the jz4740-musb driver. I don't unload the module (it's
> built-in) but unbind it from sysfs.

I did unbind too.

root@am335x-evm:/sys/bus/platform/drivers/musb-hdrc# echo musb-hdrc.0 > unbind

or unbind the glue driver:

root@am335x-evm:/sys/bus/platform/drivers/musb-dsps# echo 47401400.usb > unbind

musb_gadget_pullup() is called in both cases.

[ 3880.597014] [<bf444ab0>] (musb_gadget_pullup [musb_hdrc]) from [<bf402cbc>] (usb_gadget_disconnect+0x3c/0xf4 [udc_core])
[ 3880.607959] [<bf402cbc>] (usb_gadget_disconnect [udc_core]) from [<bf403b28>] (usb_gadget_remove_driver+0x4c/0x90 [udc_core])
[ 3880.619338] [<bf403b28>] (usb_gadget_remove_driver [udc_core]) from [<bf403d20>] (usb_del_gadget_udc+0x5c/0xc0 [udc_core])
[ 3880.630471] [<bf403d20>] (usb_del_gadget_udc [udc_core]) from [<bf439ff8>] (musb_remove+0x50/0x134 [musb_hdrc])
[ 3880.640648] [<bf439ff8>] (musb_remove [musb_hdrc]) from [<c05668cc>] (platform_drv_remove+0x28/0x48)
[ 3880.649831] [<c05668cc>] (platform_drv_remove) from [<c0564e3c>] (device_release_driver_internal+0xe4/0x1b4)
[ 3880.659708] [<c0564e3c>] (device_release_driver_internal) from [<c05633e4>] (bus_remove_device+0xe0/0x140)
[ 3880.669409] [<c05633e4>] (bus_remove_device) from [<c055f358>] (device_del+0x140/0x374)
[ 3880.677455] [<c055f358>] (device_del) from [<c0566ff0>] (platform_device_del.part.3+0x18/0x80)
[ 3880.686110] [<c0566ff0>] (platform_device_del.part.3) from [<c0567098>] (platform_device_unregister+0x24/0x30)
[ 3880.696170] [<c0567098>] (platform_device_unregister) from [<bf48c3e0>] (dsps_remove+0x1c/0x38 [musb_dsps])
[ 3880.706010] [<bf48c3e0>] (dsps_remove [musb_dsps]) from [<c05668cc>] (platform_drv_remove+0x28/0x48)
[ 3880.715190] [<c05668cc>] (platform_drv_remove) from [<c0564e3c>] (device_release_driver_internal+0xe4/0x1b4)
[ 3880.725065] [<c0564e3c>] (device_release_driver_internal) from [<c0562b2c>] (unbind_store+0x64/0xd8)
[ 3880.734253] [<c0562b2c>] (unbind_store) from [<c0350c58>] (kernfs_fop_write+0xf4/0x1cc)

Regards,
-Bin.


2019-04-02 20:01:13

by Paul Cercueil

[permalink] [raw]
Subject: Re: [PATCH] usb: musb: Force-disable pullup on shutdown

Hi,

Le lun. 1 avril 2019 ? 20:20, Bin Liu <[email protected]> a ?crit :
> On Mon, Apr 01, 2019 at 07:46:22PM +0200, Paul Cercueil wrote:
>>
>>
>> Le lun. 1 avril 2019 ? 19:17, Bin Liu <[email protected]> a ?crit :
>> >On Thu, Mar 21, 2019 at 03:42:46PM +0100, Paul Cercueil wrote:
>> >> When the musb is shutdown, for instance when the driver is
>> unloaded,
>> >> force-disable the pullup. Otherwise, the host will still see
>> >>the gadget
>> >> device even after the shutdown.
>> >
>> >how would this happen?
>> >
>> >when musb-hdrc driver is unloaded, udc core removes the bound
>> gadget
>> >driver which calls musb_gadget_pullup() to disable the pullup.
>>
>> I'm testing with the jz4740-musb driver. I don't unload the module
>> (it's
>> built-in) but unbind it from sysfs.
>
> I did unbind too.
>
> root@am335x-evm:/sys/bus/platform/drivers/musb-hdrc# echo musb-hdrc.0
> > unbind
>
> or unbind the glue driver:
>
> root@am335x-evm:/sys/bus/platform/drivers/musb-dsps# echo
> 47401400.usb > unbind
>
> musb_gadget_pullup() is called in both cases.
>
> [ 3880.597014] [<bf444ab0>] (musb_gadget_pullup [musb_hdrc]) from
> [<bf402cbc>] (usb_gadget_disconnect+0x3c/0xf4 [udc_core])
> [ 3880.607959] [<bf402cbc>] (usb_gadget_disconnect [udc_core]) from
> [<bf403b28>] (usb_gadget_remove_driver+0x4c/0x90 [udc_core])
> [ 3880.619338] [<bf403b28>] (usb_gadget_remove_driver [udc_core])
> from [<bf403d20>] (usb_del_gadget_udc+0x5c/0xc0 [udc_core])

In my case this stops here, usb_del_gadget_udc() does not call
usb_gadget_remove_driver(), that's why the pullup is never disabled.

I guess that's because udc->driver is NULL; I'm testing with
CONFIG_USB_CONFIGFS,
and I don't configure anything in sysfs before unbinding the driver.

> [ 3880.630471] [<bf403d20>] (usb_del_gadget_udc [udc_core]) from
> [<bf439ff8>] (musb_remove+0x50/0x134 [musb_hdrc])
> [ 3880.640648] [<bf439ff8>] (musb_remove [musb_hdrc]) from
> [<c05668cc>] (platform_drv_remove+0x28/0x48)
> [ 3880.649831] [<c05668cc>] (platform_drv_remove) from [<c0564e3c>]
> (device_release_driver_internal+0xe4/0x1b4)
> [ 3880.659708] [<c0564e3c>] (device_release_driver_internal) from
> [<c05633e4>] (bus_remove_device+0xe0/0x140)
> [ 3880.669409] [<c05633e4>] (bus_remove_device) from [<c055f358>]
> (device_del+0x140/0x374)
> [ 3880.677455] [<c055f358>] (device_del) from [<c0566ff0>]
> (platform_device_del.part.3+0x18/0x80)
> [ 3880.686110] [<c0566ff0>] (platform_device_del.part.3) from
> [<c0567098>] (platform_device_unregister+0x24/0x30)
> [ 3880.696170] [<c0567098>] (platform_device_unregister) from
> [<bf48c3e0>] (dsps_remove+0x1c/0x38 [musb_dsps])
> [ 3880.706010] [<bf48c3e0>] (dsps_remove [musb_dsps]) from
> [<c05668cc>] (platform_drv_remove+0x28/0x48)
> [ 3880.715190] [<c05668cc>] (platform_drv_remove) from [<c0564e3c>]
> (device_release_driver_internal+0xe4/0x1b4)
> [ 3880.725065] [<c0564e3c>] (device_release_driver_internal) from
> [<c0562b2c>] (unbind_store+0x64/0xd8)
> [ 3880.734253] [<c0562b2c>] (unbind_store) from [<c0350c58>]
> (kernfs_fop_write+0xf4/0x1cc)
>
> Regards,
> -Bin.
>
>


2019-04-03 13:27:35

by Bin Liu

[permalink] [raw]
Subject: Re: [PATCH] usb: musb: Force-disable pullup on shutdown

On Tue, Apr 02, 2019 at 09:58:42PM +0200, Paul Cercueil wrote:
> Hi,
>
> Le lun. 1 avril 2019 ? 20:20, Bin Liu <[email protected]> a ?crit :
> >On Mon, Apr 01, 2019 at 07:46:22PM +0200, Paul Cercueil wrote:
> >>
> >>
> >> Le lun. 1 avril 2019 ? 19:17, Bin Liu <[email protected]> a ?crit :
> >> >On Thu, Mar 21, 2019 at 03:42:46PM +0100, Paul Cercueil wrote:
> >> >> When the musb is shutdown, for instance when the driver is
> >>unloaded,
> >> >> force-disable the pullup. Otherwise, the host will still see
> >> >>the gadget
> >> >> device even after the shutdown.
> >> >
> >> >how would this happen?
> >> >
> >> >when musb-hdrc driver is unloaded, udc core removes the bound
> >>gadget
> >> >driver which calls musb_gadget_pullup() to disable the pullup.
> >>
> >> I'm testing with the jz4740-musb driver. I don't unload the
> >>module (it's
> >> built-in) but unbind it from sysfs.
> >
> >I did unbind too.
> >
> >root@am335x-evm:/sys/bus/platform/drivers/musb-hdrc# echo
> >musb-hdrc.0 > unbind
> >
> >or unbind the glue driver:
> >
> >root@am335x-evm:/sys/bus/platform/drivers/musb-dsps# echo
> >47401400.usb > unbind
> >
> >musb_gadget_pullup() is called in both cases.
> >
> >[ 3880.597014] [<bf444ab0>] (musb_gadget_pullup [musb_hdrc]) from
> >[<bf402cbc>] (usb_gadget_disconnect+0x3c/0xf4 [udc_core])
> >[ 3880.607959] [<bf402cbc>] (usb_gadget_disconnect [udc_core])
> >from [<bf403b28>] (usb_gadget_remove_driver+0x4c/0x90 [udc_core])
> >[ 3880.619338] [<bf403b28>] (usb_gadget_remove_driver [udc_core])
> >from [<bf403d20>] (usb_del_gadget_udc+0x5c/0xc0 [udc_core])
>
> In my case this stops here, usb_del_gadget_udc() does not call
> usb_gadget_remove_driver(), that's why the pullup is never disabled.
>
> I guess that's because udc->driver is NULL; I'm testing with

then the pullup should be disable by now.

> CONFIG_USB_CONFIGFS,
> and I don't configure anything in sysfs before unbinding the driver.

I didn't check on this, but I could imagine that
- when a configfs gadget is bound to the udc, .pullup() is called;
- when the configfs gadget is unbound from the udc, .pullup should be
called again to disable the pullup.

-Bin.

2019-04-03 15:32:32

by Paul Cercueil

[permalink] [raw]
Subject: Re: [PATCH] usb: musb: Force-disable pullup on shutdown



Le mer. 3 avril 2019 ? 15:26, Bin Liu <[email protected]> a ?crit :
> On Tue, Apr 02, 2019 at 09:58:42PM +0200, Paul Cercueil wrote:
>> Hi,
>>
>> Le lun. 1 avril 2019 ? 20:20, Bin Liu <[email protected]> a ?crit :
>> >On Mon, Apr 01, 2019 at 07:46:22PM +0200, Paul Cercueil wrote:
>> >>
>> >>
>> >> Le lun. 1 avril 2019 ? 19:17, Bin Liu <[email protected]> a ?crit :
>> >> >On Thu, Mar 21, 2019 at 03:42:46PM +0100, Paul Cercueil wrote:
>> >> >> When the musb is shutdown, for instance when the driver is
>> >>unloaded,
>> >> >> force-disable the pullup. Otherwise, the host will still see
>> >> >>the gadget
>> >> >> device even after the shutdown.
>> >> >
>> >> >how would this happen?
>> >> >
>> >> >when musb-hdrc driver is unloaded, udc core removes the bound
>> >>gadget
>> >> >driver which calls musb_gadget_pullup() to disable the pullup.
>> >>
>> >> I'm testing with the jz4740-musb driver. I don't unload the
>> >>module (it's
>> >> built-in) but unbind it from sysfs.
>> >
>> >I did unbind too.
>> >
>> >root@am335x-evm:/sys/bus/platform/drivers/musb-hdrc# echo
>> >musb-hdrc.0 > unbind
>> >
>> >or unbind the glue driver:
>> >
>> >root@am335x-evm:/sys/bus/platform/drivers/musb-dsps# echo
>> >47401400.usb > unbind
>> >
>> >musb_gadget_pullup() is called in both cases.
>> >
>> >[ 3880.597014] [<bf444ab0>] (musb_gadget_pullup [musb_hdrc]) from
>> >[<bf402cbc>] (usb_gadget_disconnect+0x3c/0xf4 [udc_core])
>> >[ 3880.607959] [<bf402cbc>] (usb_gadget_disconnect [udc_core])
>> >from [<bf403b28>] (usb_gadget_remove_driver+0x4c/0x90 [udc_core])
>> >[ 3880.619338] [<bf403b28>] (usb_gadget_remove_driver [udc_core])
>> >from [<bf403d20>] (usb_del_gadget_udc+0x5c/0xc0 [udc_core])
>>
>> In my case this stops here, usb_del_gadget_udc() does not call
>> usb_gadget_remove_driver(), that's why the pullup is never disabled.
>>
>> I guess that's because udc->driver is NULL; I'm testing with
>
> then the pullup should be disable by now.
>
>> CONFIG_USB_CONFIGFS,
>> and I don't configure anything in sysfs before unbinding the driver.
>
> I didn't check on this, but I could imagine that
> - when a configfs gadget is bound to the udc, .pullup() is called;
> - when the configfs gadget is unbound from the udc, .pullup should be
> called again to disable the pullup.

An important thing that I did not mention, is that the SoC boots from
USB,
so the pullup is active before the musb driver loads. Since in my case a
configfs gadget is never bound, then .pullup() is never called, and when
I unbind the driver the pullup is still enabled.

-Paul


2019-04-03 15:48:11

by Bin Liu

[permalink] [raw]
Subject: Re: [PATCH] usb: musb: Force-disable pullup on shutdown

On Wed, Apr 03, 2019 at 05:31:31PM +0200, Paul Cercueil wrote:
>
>
> Le mer. 3 avril 2019 ? 15:26, Bin Liu <[email protected]> a ?crit :
> >On Tue, Apr 02, 2019 at 09:58:42PM +0200, Paul Cercueil wrote:
> >> Hi,
> >>
> >> Le lun. 1 avril 2019 ? 20:20, Bin Liu <[email protected]> a ?crit :
> >> >On Mon, Apr 01, 2019 at 07:46:22PM +0200, Paul Cercueil wrote:
> >> >>
> >> >>
> >> >> Le lun. 1 avril 2019 ? 19:17, Bin Liu <[email protected]> a ?crit :
> >> >> >On Thu, Mar 21, 2019 at 03:42:46PM +0100, Paul Cercueil wrote:
> >> >> >> When the musb is shutdown, for instance when the driver is
> >> >>unloaded,
> >> >> >> force-disable the pullup. Otherwise, the host will still see
> >> >> >>the gadget
> >> >> >> device even after the shutdown.
> >> >> >
> >> >> >how would this happen?
> >> >> >
> >> >> >when musb-hdrc driver is unloaded, udc core removes the bound
> >> >>gadget
> >> >> >driver which calls musb_gadget_pullup() to disable the pullup.
> >> >>
> >> >> I'm testing with the jz4740-musb driver. I don't unload the
> >> >>module (it's
> >> >> built-in) but unbind it from sysfs.
> >> >
> >> >I did unbind too.
> >> >
> >> >root@am335x-evm:/sys/bus/platform/drivers/musb-hdrc# echo
> >> >musb-hdrc.0 > unbind
> >> >
> >> >or unbind the glue driver:
> >> >
> >> >root@am335x-evm:/sys/bus/platform/drivers/musb-dsps# echo
> >> >47401400.usb > unbind
> >> >
> >> >musb_gadget_pullup() is called in both cases.
> >> >
> >> >[ 3880.597014] [<bf444ab0>] (musb_gadget_pullup [musb_hdrc]) from
> >> >[<bf402cbc>] (usb_gadget_disconnect+0x3c/0xf4 [udc_core])
> >> >[ 3880.607959] [<bf402cbc>] (usb_gadget_disconnect [udc_core])
> >> >from [<bf403b28>] (usb_gadget_remove_driver+0x4c/0x90 [udc_core])
> >> >[ 3880.619338] [<bf403b28>] (usb_gadget_remove_driver [udc_core])
> >> >from [<bf403d20>] (usb_del_gadget_udc+0x5c/0xc0 [udc_core])
> >>
> >> In my case this stops here, usb_del_gadget_udc() does not call
> >> usb_gadget_remove_driver(), that's why the pullup is never disabled.
> >>
> >> I guess that's because udc->driver is NULL; I'm testing with
> >
> >then the pullup should be disable by now.
> >
> >> CONFIG_USB_CONFIGFS,
> >> and I don't configure anything in sysfs before unbinding the driver.
> >
> >I didn't check on this, but I could imagine that
> >- when a configfs gadget is bound to the udc, .pullup() is called;
> >- when the configfs gadget is unbound from the udc, .pullup should be
> > called again to disable the pullup.
>
> An important thing that I did not mention, is that the SoC boots
> from USB,
> so the pullup is active before the musb driver loads. Since in my case a

It sounds to me that the musb driver should disable the pullup during
init. Isn't it?

> configfs gadget is never bound, then .pullup() is never called, and when
> I unbind the driver the pullup is still enabled.

Regards,
-Bin.

2019-04-03 15:53:34

by Paul Cercueil

[permalink] [raw]
Subject: Re: [PATCH] usb: musb: Force-disable pullup on shutdown



Le mer. 3 avril 2019 ? 17:46, Bin Liu <[email protected]> a ?crit :
> On Wed, Apr 03, 2019 at 05:31:31PM +0200, Paul Cercueil wrote:
>>
>>
>> Le mer. 3 avril 2019 ? 15:26, Bin Liu <[email protected]> a ?crit :
>> >On Tue, Apr 02, 2019 at 09:58:42PM +0200, Paul Cercueil wrote:
>> >> Hi,
>> >>
>> >> Le lun. 1 avril 2019 ? 20:20, Bin Liu <[email protected]> a ?crit :
>> >> >On Mon, Apr 01, 2019 at 07:46:22PM +0200, Paul Cercueil wrote:
>> >> >>
>> >> >>
>> >> >> Le lun. 1 avril 2019 ? 19:17, Bin Liu <[email protected]> a
>> ?crit :
>> >> >> >On Thu, Mar 21, 2019 at 03:42:46PM +0100, Paul Cercueil
>> wrote:
>> >> >> >> When the musb is shutdown, for instance when the driver is
>> >> >>unloaded,
>> >> >> >> force-disable the pullup. Otherwise, the host will still
>> see
>> >> >> >>the gadget
>> >> >> >> device even after the shutdown.
>> >> >> >
>> >> >> >how would this happen?
>> >> >> >
>> >> >> >when musb-hdrc driver is unloaded, udc core removes the bound
>> >> >>gadget
>> >> >> >driver which calls musb_gadget_pullup() to disable the
>> pullup.
>> >> >>
>> >> >> I'm testing with the jz4740-musb driver. I don't unload the
>> >> >>module (it's
>> >> >> built-in) but unbind it from sysfs.
>> >> >
>> >> >I did unbind too.
>> >> >
>> >> >root@am335x-evm:/sys/bus/platform/drivers/musb-hdrc# echo
>> >> >musb-hdrc.0 > unbind
>> >> >
>> >> >or unbind the glue driver:
>> >> >
>> >> >root@am335x-evm:/sys/bus/platform/drivers/musb-dsps# echo
>> >> >47401400.usb > unbind
>> >> >
>> >> >musb_gadget_pullup() is called in both cases.
>> >> >
>> >> >[ 3880.597014] [<bf444ab0>] (musb_gadget_pullup [musb_hdrc])
>> from
>> >> >[<bf402cbc>] (usb_gadget_disconnect+0x3c/0xf4 [udc_core])
>> >> >[ 3880.607959] [<bf402cbc>] (usb_gadget_disconnect [udc_core])
>> >> >from [<bf403b28>] (usb_gadget_remove_driver+0x4c/0x90
>> [udc_core])
>> >> >[ 3880.619338] [<bf403b28>] (usb_gadget_remove_driver
>> [udc_core])
>> >> >from [<bf403d20>] (usb_del_gadget_udc+0x5c/0xc0 [udc_core])
>> >>
>> >> In my case this stops here, usb_del_gadget_udc() does not call
>> >> usb_gadget_remove_driver(), that's why the pullup is never
>> disabled.
>> >>
>> >> I guess that's because udc->driver is NULL; I'm testing with
>> >
>> >then the pullup should be disable by now.
>> >
>> >> CONFIG_USB_CONFIGFS,
>> >> and I don't configure anything in sysfs before unbinding the
>> driver.
>> >
>> >I didn't check on this, but I could imagine that
>> >- when a configfs gadget is bound to the udc, .pullup() is called;
>> >- when the configfs gadget is unbound from the udc, .pullup should
>> be
>> > called again to disable the pullup.
>>
>> An important thing that I did not mention, is that the SoC boots
>> from USB,
>> so the pullup is active before the musb driver loads. Since in my
>> case a
>
> It sounds to me that the musb driver should disable the pullup during
> init. Isn't it?

Yes. I can move it to musb_gadget_setup(). Should I still protect it
with
the spinlock then?

>> configfs gadget is never bound, then .pullup() is never called, and
>> when
>> I unbind the driver the pullup is still enabled.
>
> Regards,
> -Bin.


2019-04-03 18:55:32

by Bin Liu

[permalink] [raw]
Subject: Re: [PATCH] usb: musb: Force-disable pullup on shutdown

On Wed, Apr 03, 2019 at 05:52:20PM +0200, Paul Cercueil wrote:
>
>
> Le mer. 3 avril 2019 ? 17:46, Bin Liu <[email protected]> a ?crit :
> >On Wed, Apr 03, 2019 at 05:31:31PM +0200, Paul Cercueil wrote:
> >>
> >>
> >> Le mer. 3 avril 2019 ? 15:26, Bin Liu <[email protected]> a ?crit :
> >> >On Tue, Apr 02, 2019 at 09:58:42PM +0200, Paul Cercueil wrote:
> >> >> Hi,
> >> >>
> >> >> Le lun. 1 avril 2019 ? 20:20, Bin Liu <[email protected]> a ?crit :
> >> >> >On Mon, Apr 01, 2019 at 07:46:22PM +0200, Paul Cercueil wrote:
> >> >> >>
> >> >> >>
> >> >> >> Le lun. 1 avril 2019 ? 19:17, Bin Liu <[email protected]> a
> >>?crit :
> >> >> >> >On Thu, Mar 21, 2019 at 03:42:46PM +0100, Paul Cercueil
> >>wrote:
> >> >> >> >> When the musb is shutdown, for instance when the driver is
> >> >> >>unloaded,
> >> >> >> >> force-disable the pullup. Otherwise, the host will
> >>still see
> >> >> >> >>the gadget
> >> >> >> >> device even after the shutdown.
> >> >> >> >
> >> >> >> >how would this happen?
> >> >> >> >
> >> >> >> >when musb-hdrc driver is unloaded, udc core removes the bound
> >> >> >>gadget
> >> >> >> >driver which calls musb_gadget_pullup() to disable the
> >>pullup.
> >> >> >>
> >> >> >> I'm testing with the jz4740-musb driver. I don't unload the
> >> >> >>module (it's
> >> >> >> built-in) but unbind it from sysfs.
> >> >> >
> >> >> >I did unbind too.
> >> >> >
> >> >> >root@am335x-evm:/sys/bus/platform/drivers/musb-hdrc# echo
> >> >> >musb-hdrc.0 > unbind
> >> >> >
> >> >> >or unbind the glue driver:
> >> >> >
> >> >> >root@am335x-evm:/sys/bus/platform/drivers/musb-dsps# echo
> >> >> >47401400.usb > unbind
> >> >> >
> >> >> >musb_gadget_pullup() is called in both cases.
> >> >> >
> >> >> >[ 3880.597014] [<bf444ab0>] (musb_gadget_pullup
> >>[musb_hdrc]) from
> >> >> >[<bf402cbc>] (usb_gadget_disconnect+0x3c/0xf4 [udc_core])
> >> >> >[ 3880.607959] [<bf402cbc>] (usb_gadget_disconnect [udc_core])
> >> >> >from [<bf403b28>] (usb_gadget_remove_driver+0x4c/0x90
> >>[udc_core])
> >> >> >[ 3880.619338] [<bf403b28>] (usb_gadget_remove_driver
> >>[udc_core])
> >> >> >from [<bf403d20>] (usb_del_gadget_udc+0x5c/0xc0 [udc_core])
> >> >>
> >> >> In my case this stops here, usb_del_gadget_udc() does not call
> >> >> usb_gadget_remove_driver(), that's why the pullup is never
> >>disabled.
> >> >>
> >> >> I guess that's because udc->driver is NULL; I'm testing with
> >> >
> >> >then the pullup should be disable by now.
> >> >
> >> >> CONFIG_USB_CONFIGFS,
> >> >> and I don't configure anything in sysfs before unbinding the
> >>driver.
> >> >
> >> >I didn't check on this, but I could imagine that
> >> >- when a configfs gadget is bound to the udc, .pullup() is called;
> >> >- when the configfs gadget is unbound from the udc, .pullup
> >>should be
> >> > called again to disable the pullup.
> >>
> >> An important thing that I did not mention, is that the SoC boots
> >> from USB,
> >> so the pullup is active before the musb driver loads. Since in
> >>my case a
> >
> >It sounds to me that the musb driver should disable the pullup during
> >init. Isn't it?
>
> Yes. I can move it to musb_gadget_setup(). Should I still protect it
> with
> the spinlock then?

Thanks. I don't think spinlock is necessary here.

-Bin.