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

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

[Paul Cercueil]

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 <paul@crapouillou.net>
---
 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

usb: musb: Force-disable pullup on shutdown

[Paul Cercueil]

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 <paul@crapouillou.net>
---
 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);
 }
 
 /*

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

[Bin Liu]

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.

usb: musb: Force-disable pullup on shutdown

[Bin Liu]

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.

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

[Paul Cercueil]

Le lun. 1 avril 2019 à 19:17, Bin Liu <b-liu@ti.com> 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.

usb: musb: Force-disable pullup on shutdown

[Paul Cercueil]

Le lun. 1 avril 2019 à 19:17, Bin Liu <b-liu@ti.com> 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.

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

[Bin Liu]

On Mon, Apr 01, 2019 at 07:46:22PM +0200, Paul Cercueil wrote:
> 
> 
> Le lun. 1 avril 2019 à 19:17, Bin Liu <b-liu@ti.com> 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.

usb: musb: Force-disable pullup on shutdown

[Bin Liu]

On Mon, Apr 01, 2019 at 07:46:22PM +0200, Paul Cercueil wrote:
> 
> 
> Le lun. 1 avril 2019 à 19:17, Bin Liu <b-liu@ti.com> 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.

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

[Paul Cercueil]

Hi,

Le lun. 1 avril 2019 à 20:20, Bin Liu <b-liu@ti.com> a écrit :
> On Mon, Apr 01, 2019 at 07:46:22PM +0200, Paul Cercueil wrote:
>> 
>> 
>>  Le lun. 1 avril 2019 à 19:17, Bin Liu <b-liu@ti.com> 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.
> 
> 

usb: musb: Force-disable pullup on shutdown

[Paul Cercueil]

Hi,

Le lun. 1 avril 2019 à 20:20, Bin Liu <b-liu@ti.com> a écrit :
> On Mon, Apr 01, 2019 at 07:46:22PM +0200, Paul Cercueil wrote:
>> 
>> 
>>  Le lun. 1 avril 2019 à 19:17, Bin Liu <b-liu@ti.com> 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.
> 
>

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

[Bin Liu]

On Tue, Apr 02, 2019 at 09:58:42PM +0200, Paul Cercueil wrote:
> Hi,
> 
> Le lun. 1 avril 2019 à 20:20, Bin Liu <b-liu@ti.com> a écrit :
> >On Mon, Apr 01, 2019 at 07:46:22PM +0200, Paul Cercueil wrote:
> >>
> >>
> >> Le lun. 1 avril 2019 à 19:17, Bin Liu <b-liu@ti.com> 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.

usb: musb: Force-disable pullup on shutdown

[Bin Liu]

On Tue, Apr 02, 2019 at 09:58:42PM +0200, Paul Cercueil wrote:
> Hi,
> 
> Le lun. 1 avril 2019 à 20:20, Bin Liu <b-liu@ti.com> a écrit :
> >On Mon, Apr 01, 2019 at 07:46:22PM +0200, Paul Cercueil wrote:
> >>
> >>
> >> Le lun. 1 avril 2019 à 19:17, Bin Liu <b-liu@ti.com> 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.

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

[Paul Cercueil]

Le mer. 3 avril 2019 à 15:26, Bin Liu <b-liu@ti.com> 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 <b-liu@ti.com> a écrit :
>>  >On Mon, Apr 01, 2019 at 07:46:22PM +0200, Paul Cercueil wrote:
>>  >>
>>  >>
>>  >> Le lun. 1 avril 2019 à 19:17, Bin Liu <b-liu@ti.com> 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

usb: musb: Force-disable pullup on shutdown

[Paul Cercueil]

Le mer. 3 avril 2019 à 15:26, Bin Liu <b-liu@ti.com> 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 <b-liu@ti.com> a écrit :
>>  >On Mon, Apr 01, 2019 at 07:46:22PM +0200, Paul Cercueil wrote:
>>  >>
>>  >>
>>  >> Le lun. 1 avril 2019 à 19:17, Bin Liu <b-liu@ti.com> 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

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

[Bin Liu]

On Wed, Apr 03, 2019 at 05:31:31PM +0200, Paul Cercueil wrote:
> 
> 
> Le mer. 3 avril 2019 à 15:26, Bin Liu <b-liu@ti.com> 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 <b-liu@ti.com> a écrit :
> >> >On Mon, Apr 01, 2019 at 07:46:22PM +0200, Paul Cercueil wrote:
> >> >>
> >> >>
> >> >> Le lun. 1 avril 2019 à 19:17, Bin Liu <b-liu@ti.com> 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.

usb: musb: Force-disable pullup on shutdown

[Bin Liu]

On Wed, Apr 03, 2019 at 05:31:31PM +0200, Paul Cercueil wrote:
> 
> 
> Le mer. 3 avril 2019 à 15:26, Bin Liu <b-liu@ti.com> 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 <b-liu@ti.com> a écrit :
> >> >On Mon, Apr 01, 2019 at 07:46:22PM +0200, Paul Cercueil wrote:
> >> >>
> >> >>
> >> >> Le lun. 1 avril 2019 à 19:17, Bin Liu <b-liu@ti.com> 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.

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

[Paul Cercueil]

Le mer. 3 avril 2019 à 17:46, Bin Liu <b-liu@ti.com> a écrit :
> On Wed, Apr 03, 2019 at 05:31:31PM +0200, Paul Cercueil wrote:
>> 
>> 
>>  Le mer. 3 avril 2019 à 15:26, Bin Liu <b-liu@ti.com> 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 <b-liu@ti.com> a écrit :
>>  >> >On Mon, Apr 01, 2019 at 07:46:22PM +0200, Paul Cercueil wrote:
>>  >> >>
>>  >> >>
>>  >> >> Le lun. 1 avril 2019 à 19:17, Bin Liu <b-liu@ti.com> 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.

usb: musb: Force-disable pullup on shutdown

[Paul Cercueil]

Le mer. 3 avril 2019 à 17:46, Bin Liu <b-liu@ti.com> a écrit :
> On Wed, Apr 03, 2019 at 05:31:31PM +0200, Paul Cercueil wrote:
>> 
>> 
>>  Le mer. 3 avril 2019 à 15:26, Bin Liu <b-liu@ti.com> 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 <b-liu@ti.com> a écrit :
>>  >> >On Mon, Apr 01, 2019 at 07:46:22PM +0200, Paul Cercueil wrote:
>>  >> >>
>>  >> >>
>>  >> >> Le lun. 1 avril 2019 à 19:17, Bin Liu <b-liu@ti.com> 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.

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

[Bin Liu]

On Wed, Apr 03, 2019 at 05:52:20PM +0200, Paul Cercueil wrote:
> 
> 
> Le mer. 3 avril 2019 à 17:46, Bin Liu <b-liu@ti.com> a écrit :
> >On Wed, Apr 03, 2019 at 05:31:31PM +0200, Paul Cercueil wrote:
> >>
> >>
> >> Le mer. 3 avril 2019 à 15:26, Bin Liu <b-liu@ti.com> 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 <b-liu@ti.com> a écrit :
> >> >> >On Mon, Apr 01, 2019 at 07:46:22PM +0200, Paul Cercueil wrote:
> >> >> >>
> >> >> >>
> >> >> >> Le lun. 1 avril 2019 à 19:17, Bin Liu <b-liu@ti.com> 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.

usb: musb: Force-disable pullup on shutdown

[Bin Liu]

On Wed, Apr 03, 2019 at 05:52:20PM +0200, Paul Cercueil wrote:
> 
> 
> Le mer. 3 avril 2019 à 17:46, Bin Liu <b-liu@ti.com> a écrit :
> >On Wed, Apr 03, 2019 at 05:31:31PM +0200, Paul Cercueil wrote:
> >>
> >>
> >> Le mer. 3 avril 2019 à 15:26, Bin Liu <b-liu@ti.com> 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 <b-liu@ti.com> a écrit :
> >> >> >On Mon, Apr 01, 2019 at 07:46:22PM +0200, Paul Cercueil wrote:
> >> >> >>
> >> >> >>
> >> >> >> Le lun. 1 avril 2019 à 19:17, Bin Liu <b-liu@ti.com> 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.