Re: [PATCH 0/2] musb-fixes for v4.9-rc2

Re: [PATCH 0/2] musb-fixes for v4.9-rc2

[Tony Lindgren]

Hi,

* Ladislav Michl <ladis-6z/3iImG2C8G8FEW9MqTrA@public.gmane.org> [161020 01:24]:
> On Wed, Oct 19, 2016 at 12:03:38PM -0500, Bin Liu wrote:
> > Hi Greg,
> > 
> > Here are musb pm fixes for v4.9-rc2.  Please let me know if any change is
> > needed.
> 
> Hi Bin, Tony,
> 
> just moved away from 4.6 where musb worked (well, not quite reliably, but...)
> in host only mode with dma on dm3730. Later kernels do not work at all,
> devices get enumerated, but after a while I get:
> [   23.750061] musb_host_rx 1970: Rx interrupt with no errors or packet!
> [   23.757232] musb_host_rx 1970: Rx interrupt with no errors or packet!
> [   23.764739] musb_host_rx 1970: Rx interrupt with no errors or packet!
> [   23.771850] musb_host_rx 1970: Rx interrupt with no errors or packet!
> [   23.778900] musb_host_rx 1970: Rx interrupt with no errors or packet!
> [   23.785980] musb_host_rx 1970: Rx interrupt with no errors or packet!
> [   23.793151] musb_host_rx 1970: Rx interrupt with no errors or packet!
> [   29.281494] udlfb: wait for urb interrupted: ffffffc2 available: 0
> 
> Last line is printed repeatedly even after I disconnect udlfb device, so
> driver is unnoticed about disconnect.

I don't think I've seen that error..

> Diffing linux-4.7 (not working) against 4.6 didn't show anything suspicious
> and 4.8 adds only some tracepoints over 4.7, so the above is with 4.8.2.
> 
> Any pointers how to best do my homework and find what's wrong?

There are few patches that we seem to need for v4.7 and v4.8 stable.
At least these two fixes that should be merged for v4.9 should be
in:

[PATCH 0/2] Fixes for two more musb regressions

Then two patches for phy-twl4030-usb.c:

b78ea84a7d45 ("phy-twl4030-usb: initialize charging-related stuff via
pm_runtime")
78489c7c48d4 ("phy-twl4030-usb: better handle musb_mailbox() failure")

Are you using the twl4030 phy or something else? Also, care to try
with v4.9-rc + [PATCH 0/2] Fixes for two more musb regressions?

Regards,

Tony

> $ lsusb -t
> /:  Bus 03.Port 1: Dev 1, Class=root_hub, Driver=ehci-omap/3p, 480M
> /:  Bus 02.Port 1: Dev 1, Class=root_hub, Driver=musb-hdrc/1p, 480M
>     |__ Port 1: Dev 2, If 0, Class=Hub, Driver=hub/4p, 480M
>         |__ Port 1: Dev 3, If 0, Class=Vendor Specific Class, Driver=udlfb, 480M
>         |__ Port 4: Dev 4, If 0, Class=Hub, Driver=hub/4p, 480M
>             |__ Port 4: Dev 5, If 1, Class=CDC Data, Driver=cdc_acm, 12M
>             |__ Port 4: Dev 5, If 0, Class=Communications, Driver=cdc_acm, 12M
> /:  Bus 01.Port 1: Dev 1, Class=root_hub, Driver=ohci-omap3/3p, 12M
--
To unsubscribe from this list: send the line "unsubscribe linux-usb" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH 0/2] musb-fixes for v4.9-rc2

[Ladislav Michl]

On Thu, Oct 20, 2016 at 05:35:24AM -0700, Tony Lindgren wrote:
> Hi,
> 
> * Ladislav Michl <ladis-6z/3iImG2C8G8FEW9MqTrA@public.gmane.org> [161020 01:24]:
> > On Wed, Oct 19, 2016 at 12:03:38PM -0500, Bin Liu wrote:
> > > Hi Greg,
> > > 
> > > Here are musb pm fixes for v4.9-rc2.  Please let me know if any change is
> > > needed.
> > 
> > Hi Bin, Tony,
> > 
> > just moved away from 4.6 where musb worked (well, not quite reliably, but...)
> > in host only mode with dma on dm3730. Later kernels do not work at all,
> > devices get enumerated, but after a while I get:
> > [   23.750061] musb_host_rx 1970: Rx interrupt with no errors or packet!
> > [   23.757232] musb_host_rx 1970: Rx interrupt with no errors or packet!
> > [   23.764739] musb_host_rx 1970: Rx interrupt with no errors or packet!
> > [   23.771850] musb_host_rx 1970: Rx interrupt with no errors or packet!
> > [   23.778900] musb_host_rx 1970: Rx interrupt with no errors or packet!
> > [   23.785980] musb_host_rx 1970: Rx interrupt with no errors or packet!
> > [   23.793151] musb_host_rx 1970: Rx interrupt with no errors or packet!
> > [   29.281494] udlfb: wait for urb interrupted: ffffffc2 available: 0

Also saw this now after hub unplug:
[   94.283813] musb_stage0_irq 883: unhandled DISCONNECT transition (a_idle)

> > Last line is printed repeatedly even after I disconnect udlfb device, so
> > driver is unnoticed about disconnect.
> 
> I don't think I've seen that error..

Comment in code reads: 'FIXME this is another "SHOULD NEVER HAPPEN"'

> > Diffing linux-4.7 (not working) against 4.6 didn't show anything suspicious
> > and 4.8 adds only some tracepoints over 4.7, so the above is with 4.8.2.
> > 
> > Any pointers how to best do my homework and find what's wrong?
> 
> There are few patches that we seem to need for v4.7 and v4.8 stable.
> At least these two fixes that should be merged for v4.9 should be
> in:
> 
> [PATCH 0/2] Fixes for two more musb regressions
> 
> Then two patches for phy-twl4030-usb.c:
> 
> b78ea84a7d45 ("phy-twl4030-usb: initialize charging-related stuff via
> pm_runtime")
> 78489c7c48d4 ("phy-twl4030-usb: better handle musb_mailbox() failure")
> 
> Are you using the twl4030 phy or something else? Also, care to try

twl4030.

> with v4.9-rc + [PATCH 0/2] Fixes for two more musb regressions?

Compiled recent Linus' git tree with those two patches on top of it.
It doesn't work either, but I found that when I unplug hub from musb
during bootup and connect is again after musb gets initialized, it works
normally. Well, almost... It does survive only few reconnects, then
ends with:
<connect>
[  135.150878] musb-hdrc musb-hdrc.0.auto: VBUS_ERROR in a_host (90, <VBusValid), retry #3, port1 0009010d
[  139.793579] usb usb2-port1: Cannot enable. Maybe the USB cable is bad?
[  144.133575] usb usb2-port1: Cannot enable. Maybe the USB cable is bad?
[  148.463653] usb usb2-port1: Cannot enable. Maybe the USB cable is bad?
[  152.793609] usb usb2-port1: Cannot enable. Maybe the USB cable is bad?
[  152.809936] usb usb2-port1: unable to enumerate USB device
[  153.063568] usb usb2-port1: over-current condition
<disconnect>
<connect>
[  159.343444] usb 2-1: new high-speed USB device number 12 using musb-hdrc
[  159.526763] usb 2-1: New USB device found, idVendor=05e3, idProduct=0608
[  159.533935] usb 2-1: New USB device strings: Mfr=0, Product=1, SerialNumber=0
[  159.541473] usb 2-1: Product: USB2.0 Hub
etc... working normaly...
<disconnect>
[  161.743743] udlfb: released /dev/fb0 user=1 count=1
[  167.515075] usb 2-1: USB disconnect, device number 12
[  167.520507] usb 2-1.4: USB disconnect, device number 13
[  167.526153] usb 2-1.4.2: USB disconnect, device number 14
[  167.532989] udlfb: USB disconnect starting
[  167.537414] udlfb: Freeing all render urbs
[  167.604949] udlfb: released /dev/fb0 user=1 count=0
[  167.611602] usb 2-1.4.4: USB disconnect, device number 15
[  167.618560] cdc_acm 2-1.4.4:1.0: failed to set dtr/rts
[  167.624237] cdc_acm 2-1.4.4:1.1: urb 5 failed submission with -19
[  167.630737] cdc_acm 2-1.4.4:1.1: urb 6 failed submission with -19
[  167.637329] cdc_acm 2-1.4.4:1.1: urb 7 failed submission with -19
[  167.643859] cdc_acm 2-1.4.4:1.1: urb 8 failed submission with -19
[  167.650299] cdc_acm 2-1.4.4:1.1: urb 9 failed submission with -19
[  167.656860] cdc_acm 2-1.4.4:1.1: urb 10 failed submission with -19
[  167.663482] cdc_acm 2-1.4.4:1.1: urb 11 failed submission with -19
[  167.670043] cdc_acm 2-1.4.4:1.1: urb 12 failed submission with -19
[  167.676696] cdc_acm 2-1.4.4:1.1: urb 13 failed submission with -19
[  167.683319] cdc_acm 2-1.4.4:1.1: urb 14 failed submission with -19
[  167.689880] cdc_acm 2-1.4.4:1.1: urb 15 failed submission with -19
[  168.643890] udlfb: fb_info for /dev/fb0 has been freed
[  168.649810] udlfb: freeing dlfb_data ce247800
<disconnect>
<connect>
[  171.763488] usb 2-1: new high-speed USB device number 16 using musb-hdrc
[  171.956817] usb 2-1: New USB device found, idVendor=05e3, idProduct=0608
[  171.964019] usb 2-1: New USB device strings: Mfr=0, Product=1, SerialNumber=0
[  171.971557] usb 2-1: Product: USB2.0 Hub
[  172.088592] hub 2-1:1.0: USB hub found
[  172.122711] hub 2-1:1.0: 4 ports detected
[  172.473419] usb 2-1.4: new high-speed USB device number 17 using musb-hdrc
[  172.626312] usb 2-1.4: New USB device found, idVendor=05e3, idProduct=0608
[  172.633697] usb 2-1.4: New USB device strings: Mfr=0, Product=1, SerialNumber=0
[  172.641418] usb 2-1.4: Product: USB2.0 Hub
[  172.714416] hub 2-1.4:1.0: USB hub found
[  172.749389] hub 2-1.4:1.0: 4 ports detected
[  173.103851] usb 2-1.4.2: new high-speed USB device number 18 using musb-hdrc
[  173.278533] usb 2-1.4.2: New USB device found, idVendor=17e9, idProduct=0335
[  173.286102] usb 2-1.4.2: New USB device strings: Mfr=1, Product=2, SerialNumber=3
[  173.294067] usb 2-1.4.2: Product: MIMO
[  173.298034] usb 2-1.4.2: Manufacturer: DisplayLink
[  173.303070] usb 2-1.4.2: SerialNumber: 1071007195
[  173.406311] udlfb: DisplayLink MIMO - serial #1071007195
[  173.411987] udlfb: vid_17e9&pid_0335&rev_0120 driver's dlfb_data struct at ce247800
[  173.420135] udlfb: console enable=1
[  173.423889] udlfb: fb_defio enable=1
[  173.427642] udlfb: shadow enable=1
[  173.515655] udlfb: vendor descriptor length:23 data:23 5f 01 00 21 00 04 04 07 00 01
[  173.523925] udlfb: DL chip limited to 1500000 pixel modes
[  173.563720] alloc_contig_range: [8ecd0, 8ece0) PFNs busy
[  173.583740] udlfb: allocated 4 65024 byte urbs
[  173.713562] usb 2-1.4.4: new full-speed USB device number 19 using musb-hdrc
[  173.745849] udlfb: 800x480 @ 59 Hz valid mode
[  173.750518] udlfb: Reallocating framebuffer. Addresses will change!
[  173.807769] udlfb: 800x480 @ 59 Hz valid mode
[  173.812438] udlfb: set_par mode 800x480
[  173.904418] udlfb: DisplayLink USB device /dev/fb0 attached. 800x480 resolution. Using 1504K framebuffer memory
[  173.927825] usb 2-1.4.4: New USB device found, idVendor=0483, idProduct=5740
[  173.935363] usb 2-1.4.4: New USB device strings: Mfr=1, Product=2, SerialNumber=3
[  173.943328] usb 2-1.4.4: Product: Virtual COM Port
[  173.948364] usb 2-1.4.4: Manufacturer: MEDIARESEARCH
[  173.953613] usb 2-1.4.4: SerialNumber: VCPB7FE64A63136
[  173.961181] udlfb: sysfs edid copy cd068f00 to ce2e5000, 128 bytes
[  174.020141] udlfb: open /dev/fb0 user=1 fb_info=cd0a2c00 count=1
[  174.182250] cdc_acm 2-1.4.4:1.0: ttyACM2: USB ACM device
[  174.401397] udlfb: open /dev/fb0 user=1 fb_info=cd0a2c00 count=2
<disconnect>
[  174.567047] udlfb: released /dev/fb0 user=1 count=1
[  179.981414] usb 2-1: USB disconnect, device number 16
[  179.986938] usb 2-1.4: USB disconnect, device number 17
[  179.992462] usb 2-1.4.2: USB disconnect, device number 18
[  179.999420] udlfb: USB disconnect starting
[  180.003784] udlfb: Freeing all render urbs
[  180.087982] udlfb: released /dev/fb0 user=1 count=0
[  180.112548] usb 2-1.4.4: USB disconnect, device number 19
[  180.119415] cdc_acm 2-1.4.4:1.0: failed to set dtr/rts
[  180.125183] cdc_acm 2-1.4.4:1.1: urb 15 failed submission with -19
[  181.123931] udlfb: fb_info for /dev/fb0 has been freed
[  181.129882] udlfb: freeing dlfb_data ce247800
[  186.457519] musb-hdrc musb-hdrc.0.auto: VBUS_ERROR in a_wait_bcon (90, <VBusValid), retry #3, port1 0008010c

And that's the end, since now it does not react on hub plug/unplug.

Also all that VBUS_ERROR conditions are strange as hub is powered separately
and power lines from phy are not used.

> Regards,
> 
> Tony
> 
> > $ lsusb -t
> > /:  Bus 03.Port 1: Dev 1, Class=root_hub, Driver=ehci-omap/3p, 480M
> > /:  Bus 02.Port 1: Dev 1, Class=root_hub, Driver=musb-hdrc/1p, 480M
> >     |__ Port 1: Dev 2, If 0, Class=Hub, Driver=hub/4p, 480M
> >         |__ Port 1: Dev 3, If 0, Class=Vendor Specific Class, Driver=udlfb, 480M
> >         |__ Port 4: Dev 4, If 0, Class=Hub, Driver=hub/4p, 480M
> >             |__ Port 4: Dev 5, If 1, Class=CDC Data, Driver=cdc_acm, 12M
> >             |__ Port 4: Dev 5, If 0, Class=Communications, Driver=cdc_acm, 12M
> > /:  Bus 01.Port 1: Dev 1, Class=root_hub, Driver=ohci-omap3/3p, 12M

Best regards,
	ladis
--
To unsubscribe from this list: send the line "unsubscribe linux-usb" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH 0/2] musb-fixes for v4.9-rc2

[Tony Lindgren]

* Ladislav Michl <ladis-6z/3iImG2C8G8FEW9MqTrA@public.gmane.org> [161020 12:37]:
> On Thu, Oct 20, 2016 at 05:35:24AM -0700, Tony Lindgren wrote:
> > I don't think I've seen that error..
> 
> Comment in code reads: 'FIXME this is another "SHOULD NEVER HAPPEN"'

Seems like it does though..

> > There are few patches that we seem to need for v4.7 and v4.8 stable.
> > At least these two fixes that should be merged for v4.9 should be
> > in:
> > 
> > [PATCH 0/2] Fixes for two more musb regressions
> > 
> > Then two patches for phy-twl4030-usb.c:
> > 
> > b78ea84a7d45 ("phy-twl4030-usb: initialize charging-related stuff via
> > pm_runtime")
> > 78489c7c48d4 ("phy-twl4030-usb: better handle musb_mailbox() failure")
> > 
> > Are you using the twl4030 phy or something else? Also, care to try
> 
> twl4030.
> 
> > with v4.9-rc + [PATCH 0/2] Fixes for two more musb regressions?
> 
> Compiled recent Linus' git tree with those two patches on top of it.
> It doesn't work either, but I found that when I unplug hub from musb
> during bootup and connect is again after musb gets initialized, it works
> normally. Well, almost... It does survive only few reconnects, then
> ends with:

OK interesting, care to check if things work normally with the fixes
mentioned and without the hub? Maybe that's the difference here
compared to my setup.
...

> [  186.457519] musb-hdrc musb-hdrc.0.auto: VBUS_ERROR in a_wait_bcon (90, <VBusValid), retry #3, port1 0008010c
> 
> And that's the end, since now it does not react on hub plug/unplug.
> 
> Also all that VBUS_ERROR conditions are strange as hub is powered separately
> and power lines from phy are not used.

Hmm yeah. I'd like to be able to reproduce this. Can you email me
your .config (again)? You have things in host mode with a powered
hub plus few devices with no USB gadgets configured?

Regards,

Tony
--
To unsubscribe from this list: send the line "unsubscribe linux-usb" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH 0/2] musb-fixes for v4.9-rc2

[Tony Lindgren]

Hi,

* Tony Lindgren <tony-4v6yS6AI5VpBDgjK7y7TUQ@public.gmane.org> [161021 00:18]:
> * Ladislav Michl <ladis-6z/3iImG2C8G8FEW9MqTrA@public.gmane.org> [161020 12:37]:
> > [  186.457519] musb-hdrc musb-hdrc.0.auto: VBUS_ERROR in a_wait_bcon (90, <VBusValid), retry #3, port1 0008010c
> > 
> > And that's the end, since now it does not react on hub plug/unplug.
> > 
> > Also all that VBUS_ERROR conditions are strange as hub is powered separately
> > and power lines from phy are not used.
> 
> Hmm yeah. I'd like to be able to reproduce this. Can you email me
> your .config (again)? You have things in host mode with a powered
> hub plus few devices with no USB gadgets configured?

Well I found your earlier .config so presumably that did not change.
Below patch seems to do the trick for me, but I need to test more.

Care to test if it helps for you? Please test with v4.9-rc2 and the
following two fixes heading in Greg's usb-linus branch:

cacaaf80c3a6 ("usb: musb: Call pm_runtime from musb_gadget_queue")
d8e5f0eca1e8 ("usb: musb: Fix hardirq-safe hardirq-unsafe lock order error")

I'll send a proper patch if that works for you.

Regards,

Tony

8< ------------------------
diff --git a/drivers/phy/phy-twl4030-usb.c b/drivers/phy/phy-twl4030-usb.c
--- a/drivers/phy/phy-twl4030-usb.c
+++ b/drivers/phy/phy-twl4030-usb.c
@@ -459,8 +459,6 @@ static int twl4030_phy_power_off(struct phy *phy)
 	struct twl4030_usb *twl = phy_get_drvdata(phy);
 
 	dev_dbg(twl->dev, "%s\n", __func__);
-	pm_runtime_mark_last_busy(twl->dev);
-	pm_runtime_put_autosuspend(twl->dev);
 
 	return 0;
 }
@@ -472,6 +470,8 @@ static int twl4030_phy_power_on(struct phy *phy)
 	dev_dbg(twl->dev, "%s\n", __func__);
 	pm_runtime_get_sync(twl->dev);
 	schedule_delayed_work(&twl->id_workaround_work, HZ);
+	pm_runtime_mark_last_busy(twl->dev);
+	pm_runtime_put_autosuspend(twl->dev);
 
 	return 0;
 }
-- 
2.9.3
--
To unsubscribe from this list: send the line "unsubscribe linux-usb" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH 0/2] musb-fixes for v4.9-rc2

[Ladislav Michl]

Hi,

On Mon, Oct 24, 2016 at 11:07:08AM -0700, Tony Lindgren wrote:
> Hi,
> 
> * Tony Lindgren <tony-4v6yS6AI5VpBDgjK7y7TUQ@public.gmane.org> [161021 00:18]:
> > * Ladislav Michl <ladis-6z/3iImG2C8G8FEW9MqTrA@public.gmane.org> [161020 12:37]:
> > > [  186.457519] musb-hdrc musb-hdrc.0.auto: VBUS_ERROR in a_wait_bcon (90, <VBusValid), retry #3, port1 0008010c
> > > 
> > > And that's the end, since now it does not react on hub plug/unplug.
> > > 
> > > Also all that VBUS_ERROR conditions are strange as hub is powered separately
> > > and power lines from phy are not used.
> > 
> > Hmm yeah. I'd like to be able to reproduce this. Can you email me
> > your .config (again)? You have things in host mode with a powered
> > hub plus few devices with no USB gadgets configured?
> 
> Well I found your earlier .config so presumably that did not change.
> Below patch seems to do the trick for me, but I need to test more.
> 
> Care to test if it helps for you? Please test with v4.9-rc2 and the
> following two fixes heading in Greg's usb-linus branch:
> 
> cacaaf80c3a6 ("usb: musb: Call pm_runtime from musb_gadget_queue")
> d8e5f0eca1e8 ("usb: musb: Fix hardirq-safe hardirq-unsafe lock order error")

tested with v4.9-rc3 which have these included.

> I'll send a proper patch if that works for you.

Unfortunately it's still the same. Direct connection (without hub) remains
untested as there's not enough power to supply display:
usb 2-1: USB disconnect, device number 2
usb 2-1: new high-speed USB device number 3 using musb-hdrc
usb 2-1: New USB device found, idVendor=17e9, idProduct=0335
usb 2-1: New USB device strings: Mfr=1, Product=2, SerialNumber=3
usb 2-1: Product: MIMO
usb 2-1: Manufacturer: DisplayLink
usb 2-1: SerialNumber: 1071007195
usb 2-1: rejected 1 configuration due to insufficient available bus power
usb 2-1: no configuration chosen from 1 choice

Regards,
	ladis

> Regards,
> 
> Tony
> 
> 8< ------------------------
> diff --git a/drivers/phy/phy-twl4030-usb.c b/drivers/phy/phy-twl4030-usb.c
> --- a/drivers/phy/phy-twl4030-usb.c
> +++ b/drivers/phy/phy-twl4030-usb.c
> @@ -459,8 +459,6 @@ static int twl4030_phy_power_off(struct phy *phy)
>  	struct twl4030_usb *twl = phy_get_drvdata(phy);
>  
>  	dev_dbg(twl->dev, "%s\n", __func__);
> -	pm_runtime_mark_last_busy(twl->dev);
> -	pm_runtime_put_autosuspend(twl->dev);
>  
>  	return 0;
>  }
> @@ -472,6 +470,8 @@ static int twl4030_phy_power_on(struct phy *phy)
>  	dev_dbg(twl->dev, "%s\n", __func__);
>  	pm_runtime_get_sync(twl->dev);
>  	schedule_delayed_work(&twl->id_workaround_work, HZ);
> +	pm_runtime_mark_last_busy(twl->dev);
> +	pm_runtime_put_autosuspend(twl->dev);
>  
>  	return 0;
>  }
> -- 
> 2.9.3
--
To unsubscribe from this list: send the line "unsubscribe linux-usb" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH 0/2] musb-fixes for v4.9-rc2

[Tony Lindgren]

* Ladislav Michl <ladis-6z/3iImG2C8G8FEW9MqTrA@public.gmane.org> [161101 14:14]:
> Hi,
> 
> On Mon, Oct 24, 2016 at 11:07:08AM -0700, Tony Lindgren wrote:
> > Hi,
> > 
> > * Tony Lindgren <tony-4v6yS6AI5VpBDgjK7y7TUQ@public.gmane.org> [161021 00:18]:
> > > * Ladislav Michl <ladis-6z/3iImG2C8G8FEW9MqTrA@public.gmane.org> [161020 12:37]:
> > > > [  186.457519] musb-hdrc musb-hdrc.0.auto: VBUS_ERROR in a_wait_bcon (90, <VBusValid), retry #3, port1 0008010c
> > > > 
> > > > And that's the end, since now it does not react on hub plug/unplug.
> > > > 
> > > > Also all that VBUS_ERROR conditions are strange as hub is powered separately
> > > > and power lines from phy are not used.
> > > 
> > > Hmm yeah. I'd like to be able to reproduce this. Can you email me
> > > your .config (again)? You have things in host mode with a powered
> > > hub plus few devices with no USB gadgets configured?
> > 
> > Well I found your earlier .config so presumably that did not change.
> > Below patch seems to do the trick for me, but I need to test more.
> > 
> > Care to test if it helps for you? Please test with v4.9-rc2 and the
> > following two fixes heading in Greg's usb-linus branch:
> > 
> > cacaaf80c3a6 ("usb: musb: Call pm_runtime from musb_gadget_queue")
> > d8e5f0eca1e8 ("usb: musb: Fix hardirq-safe hardirq-unsafe lock order error")
> 
> tested with v4.9-rc3 which have these included.

OK thanks.

> > I'll send a proper patch if that works for you.
> 
> Unfortunately it's still the same. Direct connection (without hub) remains
> untested as there's not enough power to supply display:
> usb 2-1: USB disconnect, device number 2
> usb 2-1: new high-speed USB device number 3 using musb-hdrc
> usb 2-1: New USB device found, idVendor=17e9, idProduct=0335
> usb 2-1: New USB device strings: Mfr=1, Product=2, SerialNumber=3
> usb 2-1: Product: MIMO
> usb 2-1: Manufacturer: DisplayLink
> usb 2-1: SerialNumber: 1071007195
> usb 2-1: rejected 1 configuration due to insufficient available bus power
> usb 2-1: no configuration chosen from 1 choice

Hmm yeah playing with a hub connected devices don't always enumerate.
When that happens, I get this:

usb 1-1: reset high-speed USB device number 45 using musb-hdrc
usb 1-1: reset high-speed USB device number 45 using musb-hdrc
usb 1-1: reset high-speed USB device number 45 using musb-hdrc
usb 1-1: USB disconnect, device number 45
usb 1-1: new high-speed USB device number 47 using musb-hdrc
usb 1-1: new high-speed USB device number 48 using musb-hdrc
...

And that keeps on going until I reconnect the hub.

Regards,

Tony
--
To unsubscribe from this list: send the line "unsubscribe linux-usb" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH 0/2] musb-fixes for v4.9-rc2

[Tony Lindgren]

* Tony Lindgren <tony-4v6yS6AI5VpBDgjK7y7TUQ@public.gmane.org> [161103 13:59]:
> * Ladislav Michl <ladis-6z/3iImG2C8G8FEW9MqTrA@public.gmane.org> [161101 14:14]:
> > > cacaaf80c3a6 ("usb: musb: Call pm_runtime from musb_gadget_queue")
> > > d8e5f0eca1e8 ("usb: musb: Fix hardirq-safe hardirq-unsafe lock order error")
> > 
> > tested with v4.9-rc3 which have these included.
> 
> OK thanks.

So here's something to test, v4.9-rc3 + the PHY patch I
posted + the patch below.

> Hmm yeah playing with a hub connected devices don't always enumerate.
> When that happens, I get this:
> 
> usb 1-1: reset high-speed USB device number 45 using musb-hdrc
> usb 1-1: reset high-speed USB device number 45 using musb-hdrc
> usb 1-1: reset high-speed USB device number 45 using musb-hdrc
> usb 1-1: USB disconnect, device number 45
> usb 1-1: new high-speed USB device number 47 using musb-hdrc
> usb 1-1: new high-speed USB device number 48 using musb-hdrc
> ...
> 
> And that keeps on going until I reconnect the hub.

The patch below seems to work with PM for me, except I
the dsps glue layer won't go to idle after disconnecting
the hub. On 2430 glue layer things idle for me properly
and I don't seem to get any more vbus errors.

Regards,

Tony

8< ------------------
diff --git a/drivers/usb/musb/musb_core.c b/drivers/usb/musb/musb_core.c
--- a/drivers/usb/musb/musb_core.c
+++ b/drivers/usb/musb/musb_core.c
@@ -987,7 +987,7 @@ static irqreturn_t musb_stage0_irq(struct musb *musb, u8 int_usb,
 	}
 #endif
 
-	schedule_work(&musb->irq_work);
+	schedule_delayed_work(&musb->irq_work, 0);
 
 	return handled;
 }
@@ -1864,12 +1864,8 @@ static void musb_pm_runtime_check_session(struct musb *musb)
 		}
 		break;
 	case MUSB_QUIRK_A_DISCONNECT_19:
-		if (!musb->session)
-			break;
-		musb_dbg(musb, "Allow PM on possible host mode disconnect");
-		pm_runtime_mark_last_busy(musb->controller);
-		pm_runtime_put_autosuspend(musb->controller);
-		musb->session = false;
+		musb_dbg(musb, "Poll devctl on possible host mode disconnect");
+		schedule_delayed_work(&musb->irq_work, msecs_to_jiffies(1000));
 		return;
 	default:
 		break;
@@ -1900,7 +1896,7 @@ static void musb_pm_runtime_check_session(struct musb *musb)
 /* Only used to provide driver mode change events */
 static void musb_irq_work(struct work_struct *data)
 {
-	struct musb *musb = container_of(data, struct musb, irq_work);
+	struct musb *musb = container_of(data, struct musb, irq_work.work);
 
 	musb_pm_runtime_check_session(musb);
 
@@ -2274,7 +2270,7 @@ musb_init_controller(struct device *dev, int nIrq, void __iomem *ctrl)
 	musb_generic_disable(musb);
 
 	/* Init IRQ workqueue before request_irq */
-	INIT_WORK(&musb->irq_work, musb_irq_work);
+	INIT_DELAYED_WORK(&musb->irq_work, musb_irq_work);
 	INIT_DELAYED_WORK(&musb->deassert_reset_work, musb_deassert_reset);
 	INIT_DELAYED_WORK(&musb->finish_resume_work, musb_pending_work);
 
@@ -2370,7 +2366,7 @@ musb_init_controller(struct device *dev, int nIrq, void __iomem *ctrl)
 	musb_host_cleanup(musb);
 
 fail3:
-	cancel_work_sync(&musb->irq_work);
+	cancel_delayed_work_sync(&musb->irq_work);
 	musb_cancel_resume_work(musb);
 	cancel_delayed_work_sync(&musb->deassert_reset_work);
 	if (musb->dma_controller)
@@ -2437,7 +2433,7 @@ static int musb_remove(struct platform_device *pdev)
 	 */
 	musb_exit_debugfs(musb);
 
-	cancel_work_sync(&musb->irq_work);
+	cancel_delayed_work_sync(&musb->irq_work);
 	musb_cancel_resume_work(musb);
 	cancel_delayed_work_sync(&musb->deassert_reset_work);
 	pm_runtime_get_sync(musb->controller);
diff --git a/drivers/usb/musb/musb_core.h b/drivers/usb/musb/musb_core.h
--- a/drivers/usb/musb/musb_core.h
+++ b/drivers/usb/musb/musb_core.h
@@ -310,7 +310,7 @@ struct musb {
 	struct musb_context_registers context;
 
 	irqreturn_t		(*isr)(int, void *);
-	struct work_struct	irq_work;
+	struct delayed_work	irq_work;
 	struct delayed_work	deassert_reset_work;
 	struct delayed_work	finish_resume_work;
 	u16			hwvers;
diff --git a/drivers/usb/musb/musb_gadget.c b/drivers/usb/musb/musb_gadget.c
--- a/drivers/usb/musb/musb_gadget.c
+++ b/drivers/usb/musb/musb_gadget.c
@@ -1114,7 +1114,7 @@ static int musb_gadget_enable(struct usb_ep *ep,
 			musb_ep->dma ? "dma, " : "",
 			musb_ep->packet_sz);
 
-	schedule_work(&musb->irq_work);
+	schedule_delayed_work(&musb->irq_work, 0);
 
 fail:
 	spin_unlock_irqrestore(&musb->lock, flags);
@@ -1158,7 +1158,7 @@ static int musb_gadget_disable(struct usb_ep *ep)
 	musb_ep->desc = NULL;
 	musb_ep->end_point.desc = NULL;
 
-	schedule_work(&musb->irq_work);
+	schedule_delayed_work(&musb->irq_work, 0);
 
 	spin_unlock_irqrestore(&(musb->lock), flags);
 
@@ -1982,7 +1982,7 @@ static int musb_gadget_stop(struct usb_gadget *g)
 	 */
 
 	/* Force check of devctl register for PM runtime */
-	schedule_work(&musb->irq_work);
+	schedule_delayed_work(&musb->irq_work, 0);
 
 	pm_runtime_mark_last_busy(musb->controller);
 	pm_runtime_put_autosuspend(musb->controller);
diff --git a/drivers/usb/musb/tusb6010.c b/drivers/usb/musb/tusb6010.c
--- a/drivers/usb/musb/tusb6010.c
+++ b/drivers/usb/musb/tusb6010.c
@@ -724,7 +724,7 @@ tusb_otg_ints(struct musb *musb, u32 int_src, void __iomem *tbase)
 			dev_dbg(musb->controller, "vbus change, %s, otg %03x\n",
 				usb_otg_state_string(musb->xceiv->otg->state), otg_stat);
 			idle_timeout = jiffies + (1 * HZ);
-			schedule_work(&musb->irq_work);
+			schedule_delayed_work(&musb->irq_work, 0);
 
 		} else /* A-dev state machine */ {
 			dev_dbg(musb->controller, "vbus change, %s, otg %03x\n",
@@ -814,7 +814,7 @@ tusb_otg_ints(struct musb *musb, u32 int_src, void __iomem *tbase)
 			break;
 		}
 	}
-	schedule_work(&musb->irq_work);
+	schedule_delayed_work(&musb->irq_work, 0);
 
 	return idle_timeout;
 }
@@ -864,7 +864,7 @@ static irqreturn_t tusb_musb_interrupt(int irq, void *__hci)
 		musb_writel(tbase, TUSB_PRCM_WAKEUP_CLEAR, reg);
 		if (reg & ~TUSB_PRCM_WNORCS) {
 			musb->is_active = 1;
-			schedule_work(&musb->irq_work);
+			schedule_delayed_work(&musb->irq_work, 0);
 		}
 		dev_dbg(musb->controller, "wake %sactive %02x\n",
 				musb->is_active ? "" : "in", reg);
-- 
2.10.2
--
To unsubscribe from this list: send the line "unsubscribe linux-usb" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH 0/2] musb-fixes for v4.9-rc2

[Ladislav Michl]

On Thu, Nov 03, 2016 at 03:55:32PM -0700, Tony Lindgren wrote:
> * Tony Lindgren <tony-4v6yS6AI5VpBDgjK7y7TUQ@public.gmane.org> [161103 13:59]:
> > * Ladislav Michl <ladis-6z/3iImG2C8G8FEW9MqTrA@public.gmane.org> [161101 14:14]:
> > > > cacaaf80c3a6 ("usb: musb: Call pm_runtime from musb_gadget_queue")
> > > > d8e5f0eca1e8 ("usb: musb: Fix hardirq-safe hardirq-unsafe lock order error")
> > > 
> > > tested with v4.9-rc3 which have these included.
> > 
> > OK thanks.
> 
> So here's something to test, v4.9-rc3 + the PHY patch I
> posted + the patch below.
> 
> > Hmm yeah playing with a hub connected devices don't always enumerate.
> > When that happens, I get this:
> > 
> > usb 1-1: reset high-speed USB device number 45 using musb-hdrc
> > usb 1-1: reset high-speed USB device number 45 using musb-hdrc
> > usb 1-1: reset high-speed USB device number 45 using musb-hdrc
> > usb 1-1: USB disconnect, device number 45
> > usb 1-1: new high-speed USB device number 47 using musb-hdrc
> > usb 1-1: new high-speed USB device number 48 using musb-hdrc
> > ...
> > 
> > And that keeps on going until I reconnect the hub.
> 
> The patch below seems to work with PM for me, except I
> the dsps glue layer won't go to idle after disconnecting
> the hub. On 2430 glue layer things idle for me properly
> and I don't seem to get any more vbus errors.

Well, at least musb reacts on hub disconnects now. Devices
get enumerated, but do not work. Also musb does notice
only hub connect/disconnect, but does not react on
disconnection of devices in hub.

Best regards,
	ladis
--
To unsubscribe from this list: send the line "unsubscribe linux-usb" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH 0/2] musb-fixes for v4.9-rc2

[Felipe Balbi]

[-- Attachment #1: Type: text/plain, Size: 2892 bytes --]


Hi,

Tony Lindgren <tony-4v6yS6AI5VpBDgjK7y7TUQ@public.gmane.org> writes:
>> > * Tony Lindgren <tony-4v6yS6AI5VpBDgjK7y7TUQ@public.gmane.org> [161021 00:18]:
>> > > * Ladislav Michl <ladis-6z/3iImG2C8G8FEW9MqTrA@public.gmane.org> [161020 12:37]:
>> > > > [  186.457519] musb-hdrc musb-hdrc.0.auto: VBUS_ERROR in a_wait_bcon (90, <VBusValid), retry #3, port1 0008010c
>> > > > 
>> > > > And that's the end, since now it does not react on hub plug/unplug.
>> > > > 
>> > > > Also all that VBUS_ERROR conditions are strange as hub is powered separately
>> > > > and power lines from phy are not used.
>> > > 
>> > > Hmm yeah. I'd like to be able to reproduce this. Can you email me
>> > > your .config (again)? You have things in host mode with a powered
>> > > hub plus few devices with no USB gadgets configured?
>> > 
>> > Well I found your earlier .config so presumably that did not change.
>> > Below patch seems to do the trick for me, but I need to test more.
>> > 
>> > Care to test if it helps for you? Please test with v4.9-rc2 and the
>> > following two fixes heading in Greg's usb-linus branch:
>> > 
>> > cacaaf80c3a6 ("usb: musb: Call pm_runtime from musb_gadget_queue")
>> > d8e5f0eca1e8 ("usb: musb: Fix hardirq-safe hardirq-unsafe lock order error")
>> 
>> tested with v4.9-rc3 which have these included.
>
> OK thanks.
>
>> > I'll send a proper patch if that works for you.
>> 
>> Unfortunately it's still the same. Direct connection (without hub) remains
>> untested as there's not enough power to supply display:
>> usb 2-1: USB disconnect, device number 2
>> usb 2-1: new high-speed USB device number 3 using musb-hdrc
>> usb 2-1: New USB device found, idVendor=17e9, idProduct=0335
>> usb 2-1: New USB device strings: Mfr=1, Product=2, SerialNumber=3
>> usb 2-1: Product: MIMO
>> usb 2-1: Manufacturer: DisplayLink
>> usb 2-1: SerialNumber: 1071007195
>> usb 2-1: rejected 1 configuration due to insufficient available bus power
>> usb 2-1: no configuration chosen from 1 choice
>
> Hmm yeah playing with a hub connected devices don't always enumerate.
> When that happens, I get this:
>
> usb 1-1: reset high-speed USB device number 45 using musb-hdrc
> usb 1-1: reset high-speed USB device number 45 using musb-hdrc
> usb 1-1: reset high-speed USB device number 45 using musb-hdrc
> usb 1-1: USB disconnect, device number 45
> usb 1-1: new high-speed USB device number 47 using musb-hdrc
> usb 1-1: new high-speed USB device number 48 using musb-hdrc
> ...
>
> And that keeps on going until I reconnect the hub.

Sounds like VBUS dropping to me. Remember, MUSB is really anal about
VBUS levels. If it drops enough for the PHY to report one of those 4
VBUS levels, then MUSB just gives up.

What we used to do back at Nokia was disable reporting of some of those
VBUS levels at the PHY driver.

-- 
balbi

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 800 bytes --]

Re: [PATCH 0/2] musb-fixes for v4.9-rc2

[Ladislav Michl]

On Fri, Nov 04, 2016 at 10:31:38AM +0200, Felipe Balbi wrote:
[snip]
> Sounds like VBUS dropping to me. Remember, MUSB is really anal about
> VBUS levels. If it drops enough for the PHY to report one of those 4
> VBUS levels, then MUSB just gives up.
> 
> What we used to do back at Nokia was disable reporting of some of those
> VBUS levels at the PHY driver.

Well, I was getting these even without anything connected to musb:
[ 1526.594696] musb-hdrc musb-hdrc.0.auto: VBUS_ERROR in a_idle (90, <VBusValid), retry #0, port1 00000104
[  485.244781] musb-hdrc musb-hdrc.0.auto: VBUS_ERROR in a_idle (90, <VBusValid), retry #0, port1 00000104
...and with the hub connected:
[  562.803833] musb-hdrc musb-hdrc.0.auto: VBUS_ERROR in a_idle (90, <VBusValid), retry #0, port1 00000507
[   50.068664] musb-hdrc musb-hdrc.0.auto: VBUS_ERROR in a_idle (90, <VBusValid), retry #0, port1 00000507
And indeed, after these errors musb gives up.

However these are gone with Tony's last patch. Bisecting now, approx 6 iterations left.

Best regards,
	ladis
--
To unsubscribe from this list: send the line "unsubscribe linux-usb" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH 0/2] musb-fixes for v4.9-rc2

[Ladislav Michl]

On Fri, Nov 04, 2016 at 10:46:24AM +0100, Ladislav Michl wrote:
> On Fri, Nov 04, 2016 at 10:31:38AM +0200, Felipe Balbi wrote:
> [snip]
> > Sounds like VBUS dropping to me. Remember, MUSB is really anal about
> > VBUS levels. If it drops enough for the PHY to report one of those 4
> > VBUS levels, then MUSB just gives up.
> > 
> > What we used to do back at Nokia was disable reporting of some of those
> > VBUS levels at the PHY driver.
> 
> Well, I was getting these even without anything connected to musb:
> [ 1526.594696] musb-hdrc musb-hdrc.0.auto: VBUS_ERROR in a_idle (90, <VBusValid), retry #0, port1 00000104
> [  485.244781] musb-hdrc musb-hdrc.0.auto: VBUS_ERROR in a_idle (90, <VBusValid), retry #0, port1 00000104
> ...and with the hub connected:
> [  562.803833] musb-hdrc musb-hdrc.0.auto: VBUS_ERROR in a_idle (90, <VBusValid), retry #0, port1 00000507
> [   50.068664] musb-hdrc musb-hdrc.0.auto: VBUS_ERROR in a_idle (90, <VBusValid), retry #0, port1 00000507
> And indeed, after these errors musb gives up.
> 
> However these are gone with Tony's last patch. Bisecting now, approx 6 iterations left.

So here we are:

87326e858448c40e32f142c0b8dcc59d7b27c641 is the first bad commit
commit 87326e858448c40e32f142c0b8dcc59d7b27c641
Author: Tony Lindgren <tony-4v6yS6AI5VpBDgjK7y7TUQ@public.gmane.org>
Date:   Tue May 31 10:05:20 2016 -0500

    usb: musb: Remove extra PM runtime calls from 2430 glue layer

    With PM runtime behaving, these are all now unnecessary.
    Doing pm_runtime_get(musb->controller) will keep the parent
    glue layer also active.

    Signed-off-by: Tony Lindgren <tony-4v6yS6AI5VpBDgjK7y7TUQ@public.gmane.org>
    Signed-off-by: Bin Liu <b-liu-l0cyMroinI0@public.gmane.org>
    Signed-off-by: Greg Kroah-Hartman <gregkh-hQyY1W1yCW8ekmWlsbkhG0B+6BGkLq7r@public.gmane.org>

:040000 040000 6b7553722d760982d40a3e211bca7595ff313c62 d007f683d33e61a9f0046661097849c976169670 M      drivers

--
To unsubscribe from this list: send the line "unsubscribe linux-usb" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH 0/2] musb-fixes for v4.9-rc2

[Ladislav Michl]

Hi Tony,

On Thu, Nov 03, 2016 at 03:55:32PM -0700, Tony Lindgren wrote:
> * Tony Lindgren <tony-4v6yS6AI5VpBDgjK7y7TUQ@public.gmane.org> [161103 13:59]:
> > * Ladislav Michl <ladis-6z/3iImG2C8G8FEW9MqTrA@public.gmane.org> [161101 14:14]:
> > > > cacaaf80c3a6 ("usb: musb: Call pm_runtime from musb_gadget_queue")
> > > > d8e5f0eca1e8 ("usb: musb: Fix hardirq-safe hardirq-unsafe lock order error")
> > > 
> > > tested with v4.9-rc3 which have these included.
> > 
> > OK thanks.
> 
> So here's something to test, v4.9-rc3 + the PHY patch I
> posted + the patch below.
> 
> > Hmm yeah playing with a hub connected devices don't always enumerate.
> > When that happens, I get this:
> > 
> > usb 1-1: reset high-speed USB device number 45 using musb-hdrc
> > usb 1-1: reset high-speed USB device number 45 using musb-hdrc
> > usb 1-1: reset high-speed USB device number 45 using musb-hdrc
> > usb 1-1: USB disconnect, device number 45
> > usb 1-1: new high-speed USB device number 47 using musb-hdrc
> > usb 1-1: new high-speed USB device number 48 using musb-hdrc
> > ...
> > 
> > And that keeps on going until I reconnect the hub.
> 
> The patch below seems to work with PM for me, except I
> the dsps glue layer won't go to idle after disconnecting
> the hub. On 2430 glue layer things idle for me properly
> and I don't seem to get any more vbus errors.

All your patches on top of current Linus' git and revert of:
commit 87326e858448c40e32f142c0b8dcc59d7b27c641
Author: Tony Lindgren <tony-4v6yS6AI5VpBDgjK7y7TUQ@public.gmane.org>
Date:   Tue May 31 10:05:20 2016 -0500

    usb: musb: Remove extra PM runtime calls from 2430 glue layer

does the trick for me (reverted patch which applies bellow). I also
alternatively tried to increase pm_runtime_set_autosuspend_delay up
to 1000 without luck.

Hope this helps,

	ladis

diff --git a/drivers/usb/musb/omap2430.c b/drivers/usb/musb/omap2430.c
index 451b372..a623e45 100644
--- a/drivers/usb/musb/omap2430.c
+++ b/drivers/usb/musb/omap2430.c
@@ -218,8 +218,13 @@ static void omap_musb_mailbox_work(struct work_struct *mailbox_work)
 {
 	struct omap2430_glue *glue = container_of(mailbox_work,
 				struct omap2430_glue, omap_musb_mailbox_work);
+	struct musb *musb = glue_to_musb(glue);
+	struct device *dev = musb->controller;
 
+	pm_runtime_get_sync(dev);
 	omap_musb_set_mailbox(glue);
+	pm_runtime_mark_last_busy(dev);
+	pm_runtime_put_autosuspend(dev);
 }
 
 static irqreturn_t omap2430_musb_interrupt(int irq, void *__hci)
@@ -289,6 +294,16 @@ static int omap2430_musb_init(struct musb *musb)
 	phy_init(musb->phy);
 	phy_power_on(musb->phy);
 
+	/*
+	 * Enable runtime PM for musb parent (this driver). We can't
+	 * do it earlier as struct musb is not yet allocated and we
+	 * need to touch the musb registers for runtime PM.
+	 */
+	pm_runtime_enable(glue->dev);
+	status = pm_runtime_get_sync(glue->dev);
+	if (status < 0)
+		goto err1;
+
 	l = musb_readl(musb->mregs, OTG_INTERFSEL);
 
 	if (data->interface_type == MUSB_INTERFACE_UTMI) {
@@ -312,7 +327,11 @@ static int omap2430_musb_init(struct musb *musb)
 	if (glue->status != MUSB_UNKNOWN)
 		omap_musb_set_mailbox(glue);
 
+	pm_runtime_put(glue->dev);
 	return 0;
+
+err1:
+	return status;
 }
 
 static void omap2430_musb_enable(struct musb *musb)
@@ -512,10 +531,6 @@ static int omap2430_probe(struct platform_device *pdev)
 		goto err2;
 	}
 
-	pm_runtime_enable(glue->dev);
-	pm_runtime_use_autosuspend(glue->dev);
-	pm_runtime_set_autosuspend_delay(glue->dev, 100);

Re: [PATCH 0/2] musb-fixes for v4.9-rc2

[Tony Lindgren]

* Ladislav Michl <ladis-6z/3iImG2C8G8FEW9MqTrA@public.gmane.org> [161104 02:47]:
> On Fri, Nov 04, 2016 at 10:31:38AM +0200, Felipe Balbi wrote:
> [snip]
> > Sounds like VBUS dropping to me. Remember, MUSB is really anal about
> > VBUS levels. If it drops enough for the PHY to report one of those 4
> > VBUS levels, then MUSB just gives up.
> > 
> > What we used to do back at Nokia was disable reporting of some of those
> > VBUS levels at the PHY driver.

Yeah that seems to be what Ladis is seeing.

> Well, I was getting these even without anything connected to musb:
> [ 1526.594696] musb-hdrc musb-hdrc.0.auto: VBUS_ERROR in a_idle (90, <VBusValid), retry #0, port1 00000104
> [  485.244781] musb-hdrc musb-hdrc.0.auto: VBUS_ERROR in a_idle (90, <VBusValid), retry #0, port1 00000104
> ...and with the hub connected:
> [  562.803833] musb-hdrc musb-hdrc.0.auto: VBUS_ERROR in a_idle (90, <VBusValid), retry #0, port1 00000507
> [   50.068664] musb-hdrc musb-hdrc.0.auto: VBUS_ERROR in a_idle (90, <VBusValid), retry #0, port1 00000507
> And indeed, after these errors musb gives up.
> 
> However these are gone with Tony's last patch. Bisecting now, approx 6 iterations left.

These were caused by devctl session bit taking few seconds to clear
so we have to poll that on disconnect until it clears.

Regards,

Tony
--
To unsubscribe from this list: send the line "unsubscribe linux-usb" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH 0/2] musb-fixes for v4.9-rc2

[Tony Lindgren]

* Ladislav Michl <ladis-6z/3iImG2C8G8FEW9MqTrA@public.gmane.org> [161104 03:41]:
> All your patches on top of current Linus' git and revert of:
> commit 87326e858448c40e32f142c0b8dcc59d7b27c641
> Author: Tony Lindgren <tony-4v6yS6AI5VpBDgjK7y7TUQ@public.gmane.org>
> Date:   Tue May 31 10:05:20 2016 -0500
> 
>     usb: musb: Remove extra PM runtime calls from 2430 glue layer
> 
> does the trick for me (reverted patch which applies bellow). I also
> alternatively tried to increase pm_runtime_set_autosuspend_delay up
> to 1000 without luck.

OK thanks for narrowing it down. The patch does not seem to change
anything for me with 2430 glue layer, things work for me with and
without it. Care to check if all hunks of the patch are needed
for you..

> diff --git a/drivers/usb/musb/omap2430.c b/drivers/usb/musb/omap2430.c
> index 451b372..a623e45 100644
> --- a/drivers/usb/musb/omap2430.c
> +++ b/drivers/usb/musb/omap2430.c
> @@ -218,8 +218,13 @@ static void omap_musb_mailbox_work(struct work_struct *mailbox_work)
>  {
>  	struct omap2430_glue *glue = container_of(mailbox_work,
>  				struct omap2430_glue, omap_musb_mailbox_work);
> +	struct musb *musb = glue_to_musb(glue);
> +	struct device *dev = musb->controller;
>  
> +	pm_runtime_get_sync(dev);
>  	omap_musb_set_mailbox(glue);
> +	pm_runtime_mark_last_busy(dev);
> +	pm_runtime_put_autosuspend(dev);
>  }
>  
>  static irqreturn_t omap2430_musb_interrupt(int irq, void *__hci)

..is just the above enough..

> @@ -289,6 +294,16 @@ static int omap2430_musb_init(struct musb *musb)
>  	phy_init(musb->phy);
>  	phy_power_on(musb->phy);
>  
> +	/*
> +	 * Enable runtime PM for musb parent (this driver). We can't
> +	 * do it earlier as struct musb is not yet allocated and we
> +	 * need to touch the musb registers for runtime PM.
> +	 */
> +	pm_runtime_enable(glue->dev);
> +	status = pm_runtime_get_sync(glue->dev);
> +	if (status < 0)
> +		goto err1;
> +
>  	l = musb_readl(musb->mregs, OTG_INTERFSEL);
>  
>  	if (data->interface_type == MUSB_INTERFACE_UTMI) {
> @@ -312,7 +327,11 @@ static int omap2430_musb_init(struct musb *musb)
>  	if (glue->status != MUSB_UNKNOWN)
>  		omap_musb_set_mailbox(glue);
>  
> +	pm_runtime_put(glue->dev);
>  	return 0;
> +
> +err1:
> +	return status;
>  }
>  
>  static void omap2430_musb_enable(struct musb *musb)
> @@ -512,10 +531,6 @@ static int omap2430_probe(struct platform_device *pdev)
>  		goto err2;
>  	}
>  
> -	pm_runtime_enable(glue->dev);
> -	pm_runtime_use_autosuspend(glue->dev);
> -	pm_runtime_set_autosuspend_delay(glue->dev, 100);
> -
>  	ret = platform_device_add(musb);
>  	if (ret) {
>  		dev_err(&pdev->dev, "failed to register musb device\n");
> @@ -538,7 +553,6 @@ static int omap2430_remove(struct platform_device *pdev)
>  	pm_runtime_get_sync(glue->dev);
>  	platform_device_unregister(glue->musb);
>  	pm_runtime_put_sync(glue->dev);
> -	pm_runtime_dont_use_autosuspend(glue->dev);
>  	pm_runtime_disable(glue->dev);
>  
>  	return 0;

..or are these init changes needed too?

Regards,

Tony
--
To unsubscribe from this list: send the line "unsubscribe linux-usb" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH 0/2] musb-fixes for v4.9-rc2

[Ladislav Michl]

On Fri, Nov 04, 2016 at 07:48:13AM -0700, Tony Lindgren wrote:
> * Ladislav Michl <ladis-6z/3iImG2C8G8FEW9MqTrA@public.gmane.org> [161104 03:41]:
> > All your patches on top of current Linus' git and revert of:
> > commit 87326e858448c40e32f142c0b8dcc59d7b27c641
> > Author: Tony Lindgren <tony-4v6yS6AI5VpBDgjK7y7TUQ@public.gmane.org>
> > Date:   Tue May 31 10:05:20 2016 -0500
> > 
> >     usb: musb: Remove extra PM runtime calls from 2430 glue layer
> > 
> > does the trick for me (reverted patch which applies bellow). I also
> > alternatively tried to increase pm_runtime_set_autosuspend_delay up
> > to 1000 without luck.
> 
> OK thanks for narrowing it down. The patch does not seem to change
> anything for me with 2430 glue layer, things work for me with and
> without it. Care to check if all hunks of the patch are needed
> for you..
> 
> > diff --git a/drivers/usb/musb/omap2430.c b/drivers/usb/musb/omap2430.c
> > index 451b372..a623e45 100644
> > --- a/drivers/usb/musb/omap2430.c
> > +++ b/drivers/usb/musb/omap2430.c
> > @@ -218,8 +218,13 @@ static void omap_musb_mailbox_work(struct work_struct *mailbox_work)
> >  {
> >  	struct omap2430_glue *glue = container_of(mailbox_work,
> >  				struct omap2430_glue, omap_musb_mailbox_work);
> > +	struct musb *musb = glue_to_musb(glue);
> > +	struct device *dev = musb->controller;
> >  
> > +	pm_runtime_get_sync(dev);
> >  	omap_musb_set_mailbox(glue);
> > +	pm_runtime_mark_last_busy(dev);
> > +	pm_runtime_put_autosuspend(dev);
> >  }
> >  
> >  static irqreturn_t omap2430_musb_interrupt(int irq, void *__hci)
> 
> ..is just the above enough..

No, does not work with only this hunk:
[   94.980102] udlfb: wait for urb interrupted: ffffffc2 available: 0
[  124.642456] musb_ep_program 921: broken !rx_reinit, ep2 csr 0003
[  124.648986] musb_host_rx 1912: RX2 dma busy, csr a203
[  124.654754] musb_host_rx 1970: Rx interrupt with no errors or packet!
[  124.661743] musb_host_rx 1970: Rx interrupt with no errors or packet!
[  124.668762] musb_host_rx 1970: Rx interrupt with no errors or packet!
[  124.675720] musb_host_rx 1970: Rx interrupt with no errors or packet!
[  124.682830] musb_ep_program 921: broken !rx_reinit, ep2 csr 0003
[  124.689178] musb_host_rx 1970: Rx interrupt with no errors or packet!
[  124.696105] musb_host_rx 1970: Rx interrupt with no errors or packet!
[  124.772552] udlfb: released /dev/fb0 user=1 count=0

> > @@ -289,6 +294,16 @@ static int omap2430_musb_init(struct musb *musb)
> >  	phy_init(musb->phy);
> >  	phy_power_on(musb->phy);
> >  
> > +	/*
> > +	 * Enable runtime PM for musb parent (this driver). We can't
> > +	 * do it earlier as struct musb is not yet allocated and we
> > +	 * need to touch the musb registers for runtime PM.
> > +	 */
> > +	pm_runtime_enable(glue->dev);
> > +	status = pm_runtime_get_sync(glue->dev);
> > +	if (status < 0)
> > +		goto err1;
> > +
> >  	l = musb_readl(musb->mregs, OTG_INTERFSEL);
> >  
> >  	if (data->interface_type == MUSB_INTERFACE_UTMI) {
> > @@ -312,7 +327,11 @@ static int omap2430_musb_init(struct musb *musb)
> >  	if (glue->status != MUSB_UNKNOWN)
> >  		omap_musb_set_mailbox(glue);
> >  
> > +	pm_runtime_put(glue->dev);
> >  	return 0;
> > +
> > +err1:
> > +	return status;
> >  }
> >  
> >  static void omap2430_musb_enable(struct musb *musb)
> > @@ -512,10 +531,6 @@ static int omap2430_probe(struct platform_device *pdev)
> >  		goto err2;
> >  	}
> >  
> > -	pm_runtime_enable(glue->dev);
> > -	pm_runtime_use_autosuspend(glue->dev);
> > -	pm_runtime_set_autosuspend_delay(glue->dev, 100);
> > -
> >  	ret = platform_device_add(musb);
> >  	if (ret) {
> >  		dev_err(&pdev->dev, "failed to register musb device\n");
> > @@ -538,7 +553,6 @@ static int omap2430_remove(struct platform_device *pdev)
> >  	pm_runtime_get_sync(glue->dev);
> >  	platform_device_unregister(glue->musb);
> >  	pm_runtime_put_sync(glue->dev);
> > -	pm_runtime_dont_use_autosuspend(glue->dev);
> >  	pm_runtime_disable(glue->dev);
> >  
> >  	return 0;
> 
> ..or are these init changes needed too?

Yes, also tried to move pm_runtime_use_autosuspend into omap2430_musb_init
and drop omap_musb_mailbox_work changes, but that does not work either.

	ladis
--
To unsubscribe from this list: send the line "unsubscribe linux-usb" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html