usb autosuspend is history after S2R (also on 2.6.31; UVC?)

usb autosuspend is history after S2R (also on 2.6.31; UVC?)

[Andreas Mohr]

Hi,

currently running 2.6.31, and what I've been suspecting for quite some
time (several weeks) is actually true: after S2R resume
all friendly USB autosuspend settings are history and we're back
to the dreaded old 100% activity (powertop -d):

Recent USB suspend statistics
Active  Device name
100.0%  USB device  5-5 : Acer Crystal Eye webcam (SuYin)
100.0%  USB device usb5 : EHCI Host Controller (Linux 2.6.31 ehci_hcd)
  0.0%  USB device usb4 : UHCI Host Controller (Linux 2.6.31 uhci_hcd)
  0.0%  USB device usb3 : UHCI Host Controller (Linux 2.6.31 uhci_hcd)
  0.0%  USB device usb2 : UHCI Host Controller (Linux 2.6.31 uhci_hcd)
  0.0%  USB device usb1 : UHCI Host Controller (Linux 2.6.31 uhci_hcd)

Is there any fix planned for this? This is possibly even more important
(power wasted due to wakeups, I'd judge it to be up to 1.5W on this 8.5W
Aspire One machine) than any other runtime PM implementation...

OTOH this problem is probably isolated to uvc devices, since:
# lsusb -t
/:  Bus 05.Port 1: Dev 1, Class=root_hub, Driver=ehci_hcd/8p, 480M
    |__ Port 5: Dev 7, If 0, Class='bInterfaceClass 0x0e not yet handled', Driver=uvcvideo, 480M
    |__ Port 5: Dev 7, If 1, Class='bInterfaceClass 0x0e not yet handled', Driver=uvcvideo, 480M
/:  Bus 04.Port 1: Dev 1, Class=root_hub, Driver=uhci_hcd/2p, 12M
/:  Bus 03.Port 1: Dev 1, Class=root_hub, Driver=uhci_hcd/2p, 12M
/:  Bus 02.Port 1: Dev 1, Class=root_hub, Driver=uhci_hcd/2p, 12M
/:  Bus 01.Port 1: Dev 1, Class=root_hub, Driver=uhci_hcd/2p, 12M

So, does the uvc driver need a bit of handholding here?
It likely shouldn't simply forget about its autosuspend status after
resume...

Or would Matthew's uvc autosuspend patch
http://osdir.com/ml/fedora-extras-commits/2009-07/msg06079.html
help here?

Thanks,

Andreas Mohr

Re: usb autosuspend is history after S2R (also on 2.6.31; UVC?)

[Greg KH]

On Tue, Sep 15, 2009 at 08:51:45PM +0200, Andreas Mohr wrote:
> Hi,
> 
> currently running 2.6.31, and what I've been suspecting for quite some
> time (several weeks) is actually true: after S2R resume
> all friendly USB autosuspend settings are history and we're back
> to the dreaded old 100% activity (powertop -d):
> 
> Recent USB suspend statistics
> Active  Device name
> 100.0%  USB device  5-5 : Acer Crystal Eye webcam (SuYin)
> 100.0%  USB device usb5 : EHCI Host Controller (Linux 2.6.31 ehci_hcd)
>   0.0%  USB device usb4 : UHCI Host Controller (Linux 2.6.31 uhci_hcd)
>   0.0%  USB device usb3 : UHCI Host Controller (Linux 2.6.31 uhci_hcd)
>   0.0%  USB device usb2 : UHCI Host Controller (Linux 2.6.31 uhci_hcd)
>   0.0%  USB device usb1 : UHCI Host Controller (Linux 2.6.31 uhci_hcd)
> 
> Is there any fix planned for this? This is possibly even more important
> (power wasted due to wakeups, I'd judge it to be up to 1.5W on this 8.5W
> Aspire One machine) than any other runtime PM implementation...
> 
> OTOH this problem is probably isolated to uvc devices, since:
> # lsusb -t
> /:  Bus 05.Port 1: Dev 1, Class=root_hub, Driver=ehci_hcd/8p, 480M
>     |__ Port 5: Dev 7, If 0, Class='bInterfaceClass 0x0e not yet handled', Driver=uvcvideo, 480M
>     |__ Port 5: Dev 7, If 1, Class='bInterfaceClass 0x0e not yet handled', Driver=uvcvideo, 480M
> /:  Bus 04.Port 1: Dev 1, Class=root_hub, Driver=uhci_hcd/2p, 12M
> /:  Bus 03.Port 1: Dev 1, Class=root_hub, Driver=uhci_hcd/2p, 12M
> /:  Bus 02.Port 1: Dev 1, Class=root_hub, Driver=uhci_hcd/2p, 12M
> /:  Bus 01.Port 1: Dev 1, Class=root_hub, Driver=uhci_hcd/2p, 12M
> 
> So, does the uvc driver need a bit of handholding here?
> It likely shouldn't simply forget about its autosuspend status after
> resume...
> 
> Or would Matthew's uvc autosuspend patch
> http://osdir.com/ml/fedora-extras-commits/2009-07/msg06079.html

Could you try that?  A certian hardware manufacturer is insisting that
this patch solves problems, which I have not been able to validate, and
it would be good to get some independant tests.

thanks,

greg k-h

Re: usb autosuspend is history after S2R (also on 2.6.31; UVC?)

[Matthew Garrett]

On Tue, Sep 15, 2009 at 12:00:45PM -0700, Greg KH wrote:

> Could you try that?  A certian hardware manufacturer is insisting that
> this patch solves problems, which I have not been able to validate, and
> it would be good to get some independant tests.

I'd be surprised if this actually fixed anything - it's equivalent to 
just echoing "enable" into the autosuspend file at device bind time. 
I've been waiting until we ship beta and get more user feedback before 
pushing it.

-- 
Matthew Garrett | mjg59@srcf.ucam.org

Re: usb autosuspend is history after S2R (also on 2.6.31; UVC?)

[Oliver Neukum]

Am Dienstag, 15. September 2009 20:51:45 schrieb Andreas Mohr:

> So, does the uvc driver need a bit of handholding here?
> It likely shouldn't simply forget about its autosuspend status after
> resume...

Can you please provide what you get when you read the "level" file
of the device's power directory?

Secondly, please verify, that it works before the STR cycle and post
a syslog of a suspend/resume cycle with CONFIG_USB_DEBUG and
only this device attached if possible. Thirdly, please verify that only
uvc is affected.

> Or would Matthew's uvc autosuspend patch
> http://osdir.com/ml/fedora-extras-commits/2009-07/msg06079.html
> help here?

Please test it. It shouldn't work, but if it works there's a fundamental
flaw.

	Regards
		Oliver

Re: usb autosuspend is history after S2R (also on 2.6.31; UVC?)

[Andreas Mohr]

Hi,

On Tue, Sep 15, 2009 at 09:48:28PM +0200, Oliver Neukum wrote:
> Am Dienstag, 15. September 2009 20:51:45 schrieb Andreas Mohr:
> 
> > So, does the uvc driver need a bit of handholding here?
> > It likely shouldn't simply forget about its autosuspend status after
> > resume...
> 
> Can you please provide what you get when you read the "level" file
> of the device's power directory?

After boot:

# cat 5-5/product
Acer Crystal Eye webcam
# cat 5-5/power/level
on

After re-enabling disabled /etc/rc.local autosuspend tweaks:

# cat 5-5/power/level
auto

> Secondly, please verify, that it works before the STR cycle and post

It does:

Recent USB suspend statistics
Active  Device name
  0.0%  USB device  5-5 : Acer Crystal Eye webcam (SuYin)
  0.0%  USB device usb5 : EHCI Host Controller (Linux 2.6.31 ehci_hcd)
  0.0%  USB device usb4 : UHCI Host Controller (Linux 2.6.31 uhci_hcd)
  0.0%  USB device usb3 : UHCI Host Controller (Linux 2.6.31 uhci_hcd)
  0.0%  USB device usb2 : UHCI Host Controller (Linux 2.6.31 uhci_hcd)
  0.0%  USB device usb1 : UHCI Host Controller (Linux 2.6.31 uhci_hcd)

> a syslog of a suspend/resume cycle with CONFIG_USB_DEBUG and
> only this device attached if possible. Thirdly, please verify that only
> uvc is affected.

Hmmmm...

I just did the first S2R after boot, and the result is this:

Recent USB suspend statistics
Active  Device name          
  0.0%  USB device  5-5 : Acer Crystal Eye webcam (SuYin)
  0.0%  USB device usb5 : EHCI Host Controller (Linux 2.6.31 ehci_hcd)
  0.0%  USB device usb4 : UHCI Host Controller (Linux 2.6.31 uhci_hcd)
  0.0%  USB device usb3 : UHCI Host Controller (Linux 2.6.31 uhci_hcd)
  0.0%  USB device usb2 : UHCI Host Controller (Linux 2.6.31 uhci_hcd)
  0.0%  USB device usb1 : UHCI Host Controller (Linux 2.6.31 uhci_hcd)
# cat 5-5/power/level           
auto

Weird. Either it was always working and I'm just _very_ wrong,
or it's erratic and gets reset every couple S2Rs only.

Anyway, doesn't change the fact that the r8169 WOL PHY setting is toast
after every S2R as well ;)
# ethtool -s eth0 wol d
# ethtool eth0
...
        Wake-on: d
...

which then reverts to
# ethtool eth0
...
        Wake-on: g
...
thus wasting a wee bit of power(, too).

> > Or would Matthew's uvc autosuspend patch
> > http://osdir.com/ml/fedora-extras-commits/2009-07/msg06079.html
> > help here?
> 
> Please test it. It shouldn't work, but if it works there's a fundamental
> flaw.

Hmm, rebuild (of patch plus CONFIG_USB_DEBUG) would take >> 2 hours,
will have to wait until later.

Thanks,

Andreas

Re: usb autosuspend is history after S2R (also on 2.6.31; UVC?)

[Francois Romieu]

Andreas Mohr <andi@lisas.de> :
[...]
> Anyway, doesn't change the fact that the r8169 WOL PHY setting is toast
> after every S2R as well ;)

Stealth tester, he ?

diff --git a/drivers/net/r8169.c b/drivers/net/r8169.c
index 50c6a3c..e0af22f 100644
--- a/drivers/net/r8169.c
+++ b/drivers/net/r8169.c
@@ -3962,8 +3962,7 @@ static int rtl8169_resume(struct device *device)
 		goto out;
 
 	netif_device_attach(dev);
-
-	rtl8169_schedule_work(dev, rtl8169_reset_task);
+	netif_wake_queue(dev);
 out:
 	return 0;
 }

-- 
Ueimor

Re: usb autosuspend is history after S2R (also on 2.6.31; UVC?)

[Andreas Mohr]

Hi,

On Wed, Sep 16, 2009 at 09:51:56PM +0200, Francois Romieu wrote:
> Andreas Mohr <andi@lisas.de> :
> [...]
> > Anyway, doesn't change the fact that the r8169 WOL PHY setting is toast
> > after every S2R as well ;)
> 
> Stealth tester, he ?

Wait, you mean this is the patch to fix it that you created just now,
and it's that easy?
I'd have expected needing to fiddle with PHY power-management registers
and such stuff (while remembering pre-suspend status)
upon resume() handlers.

And sorry for not reporting it earlier (provoking that "Stealth tester"
above, right?), but I had kept tabs on it.

> diff --git a/drivers/net/r8169.c b/drivers/net/r8169.c
> index 50c6a3c..e0af22f 100644
> --- a/drivers/net/r8169.c
> +++ b/drivers/net/r8169.c
> @@ -3962,8 +3962,7 @@ static int rtl8169_resume(struct device *device)
>  		goto out;
>  
>  	netif_device_attach(dev);
> -
> -	rtl8169_schedule_work(dev, rtl8169_reset_task);
> +	netif_wake_queue(dev);
>  out:
>  	return 0;
>  }

Crap, this is now the third thing waiting to get patched; looks like
I need to do another build for real now.

Thanks a lot,

Andreas Mohr

Re: usb autosuspend is history after S2R (also on 2.6.31; UVC?)

[Francois Romieu]

Andreas Mohr <andi@lisas.de> :
[...]
> Wait, you mean this is the patch to fix it that you created just now,
> and it's that easy?

I can not exclude that it will not swap a settings loss problem for
a (new) link loss after resume one or some completely random crash
but it deserves to be tested, yes.

> I'd have expected needing to fiddle with PHY power-management registers
> and such stuff (while remembering pre-suspend status)
> upon resume() handlers.

The current driver does not play directly with the PHY during
suspend / resume.

/me wonders...

Can you try the patch below too ? A PHY reset will probably be required
too now.

diff --git a/drivers/net/r8169.c b/drivers/net/r8169.c
index 50c6a3c..116797f 100644
--- a/drivers/net/r8169.c
+++ b/drivers/net/r8169.c
@@ -3934,11 +3934,19 @@ static struct net_device_stats *rtl8169_get_stats(struct net_device *dev)
 
 static void rtl8169_net_suspend(struct net_device *dev)
 {
+	struct rtl8169_private *tp = netdev_priv(dev);
+
 	if (!netif_running(dev))
 		return;
 
 	netif_device_detach(dev);
 	netif_stop_queue(dev);
+
+	if (!(tp->features & RTL_FEATURE_WOL)) {
+		void __iomem *ioaddr = tp->mmio_addr;
+
+		mdio_patch(ioaddr, MII_BMCR, BMCR_PDOWN);
+	}
 }
 
 #ifdef CONFIG_PM
@@ -3957,13 +3965,21 @@ static int rtl8169_resume(struct device *device)
 {
 	struct pci_dev *pdev = to_pci_dev(device);
 	struct net_device *dev = pci_get_drvdata(pdev);
+	struct rtl8169_private *tp = netdev_priv(dev);
 
 	if (!netif_running(dev))
 		goto out;
 
 	netif_device_attach(dev);
+	netif_wake_queue(dev);
 
-	rtl8169_schedule_work(dev, rtl8169_reset_task);
+	if (!(tp->features & RTL_FEATURE_WOL)) {
+		void __iomem *ioaddr = tp->mmio_addr;
+		unsigned int val;
+
+		val = mdio_read(ioaddr, MII_BMCR) & ~BMCR_PDOWN;
+		mdio_write(ioaddr, MII_BMCR, val & 0xffff);
+	}
 out:
 	return 0;
 }

> And sorry for not reporting it earlier (provoking that "Stealth tester"
> above, right?), but I had kept tabs on it.

No problem, really. :o)

-- 
Ueimor