OHCI problems with suspend/resume

OHCI problems with suspend/resume

[Pavel Machek]

Hi!

In 2.6.0-test1, OHCI is non-functional after first suspend/resume, and
kills machine during secon suspend/resume cycle.

What happens is that ohci_irq gets ohci->hcca == NULL, and kills
machine. Why is ohci->hcca == NULL? ohci_stop was called from
hcd_panic() and freed ohci->hcca.

I believe that we should

1) not free ohci->hcca so that system has better chance surviving
hcd_panic()

and 

2) inform user when hcd panics.

This patch does both, please apply,

[Okay, you probably want to kill the ifdefs, and kill the code,
leaving just a comment...]
								Pavel

--- /usr/src/tmp/linux/drivers/usb/core/hcd.c	2003-07-06 20:07:49.000000000 +0200
+++ /usr/src/linux/drivers/usb/core/hcd.c	2003-07-23 23:32:55.000000000 +0200
@@ -1486,6 +1486,7 @@
 static void hcd_panic (void *_hcd)
 {
 	struct usb_hcd *hcd = _hcd;
+	printk(KERN_CRIT "Host controller panicked\n");
 	hcd->driver->stop (hcd);
 }
 
--- /usr/src/tmp/linux/drivers/usb/host/ohci-hcd.c	2003-05-27 13:43:39.000000000 +0200
+++ /usr/src/linux/drivers/usb/host/ohci-hcd.c	2003-07-23 23:31:46.000000000 +0200
@@ -627,12 +632,18 @@
 	
 	remove_debug_files (ohci);
 	ohci_mem_cleanup (ohci);
+#if 0
+	/* Freeing hcca at this point is bad idea, because ohci_irq
+	   can't cope with ohci->hcca being NULL, and therefore will
+	   crash the machine after hcd_panic()
+	 */
 	if (ohci->hcca) {
 		pci_free_consistent (ohci->hcd.pdev, sizeof *ohci->hcca,
 					ohci->hcca, ohci->hcca_dma);
 		ohci->hcca = NULL;
 		ohci->hcca_dma = 0;
 	}
+#endif
 }
 
 /*-------------------------------------------------------------------------*/

-- 
When do you have a heart between your knees?
[Johanka's followup: and *two* hearts?]

Re: [linux-usb-devel] OHCI problems with suspend/resume

[David Brownell]

Pavel Machek wrote:
> Hi!
> 
> In 2.6.0-test1, OHCI is non-functional after first suspend/resume, and
> kills machine during secon suspend/resume cycle.

Hmm, last time I tested suspend/resume it worked fine.
That was 2.5.67, but the OHCI code hasn't had any
relevant changes since then.

Evidently your system used different suspend/resume paths
than mine did ... :)


> What happens is that ohci_irq gets ohci->hcca == NULL, and kills
> machine. Why is ohci->hcca == NULL? ohci_stop was called from
> hcd_panic() and freed ohci->hcca.

Then the problem is that an IRQ is still coming in after the
HCD panicked.


> I believe that we should
> 
> 1) not free ohci->hcca so that system has better chance surviving
> hcd_panic()

Not ever????

It's freed in exactly one place, after everything should be
shut down.  If it wasn't shut down, that was the problem.

Could you instead figure out why it wasn't shut down?


> and 
> 
> 2) inform user when hcd panics.

The OHCI code does that already on the normal panic path
(controller delivers a Unrecoverable Error interrupt), but
you're right that this would be better as a generic KERN_CRIT
diagnostic.  (But one saying which HCD panicked, rather than
leaving folk to guess which of the N it applied to...)  And
I'd print that message sooner, not waiting for that task to
be scheduled.

I'll merge that fix in with one I have in my queue.

- Dave

Re: [linux-usb-devel] OHCI problems with suspend/resume

[Pavel Machek]

Hi!

> >In 2.6.0-test1, OHCI is non-functional after first suspend/resume, and
> >kills machine during secon suspend/resume cycle.
> 
> Hmm, last time I tested suspend/resume it worked fine.
> That was 2.5.67, but the OHCI code hasn't had any
> relevant changes since then.

> Evidently your system used different suspend/resume paths
> than mine did ... :)

Can you try echo 4 > /proc/acpi/sleep? echo 3 breaks it, too, but that
is little harder to set up.

> >What happens is that ohci_irq gets ohci->hcca == NULL, and kills
> >machine. Why is ohci->hcca == NULL? ohci_stop was called from
> >hcd_panic() and freed ohci->hcca.
> 
> Then the problem is that an IRQ is still coming in after the
> HCD panicked.

Actually, as PCI interrupts are shared, I do not find that too
surprising. 

> >I believe that we should
> >
> >1) not free ohci->hcca so that system has better chance surviving
> >hcd_panic()
> 
> Not ever????
> 
> It's freed in exactly one place, after everything should be
> shut down.  If it wasn't shut down, that was the problem.
> 
> Could you instead figure out why it wasn't shut down?

In case of hcd_panic(), where is interrupt deallocated?

> >and 
> >
> >2) inform user when hcd panics.
> 
> The OHCI code does that already on the normal panic path
> (controller delivers a Unrecoverable Error interrupt), but
> you're right that this would be better as a generic KERN_CRIT
> diagnostic.  (But one saying which HCD panicked, rather than
> leaving folk to guess which of the N it applied to...)  And
> I'd print that message sooner, not waiting for that task to
> be scheduled.

That would be good. I definitely had another failure path, where it
did not tell me that hcd is no K.O...

								Pavel
-- 
When do you have a heart between your knees?
[Johanka's followup: and *two* hearts?]

Re: [linux-usb-devel] OHCI problems with suspend/resume

[David Brownell]

Pavel Machek wrote:
> Hi!

Good morning to you too!


>>>In 2.6.0-test1, OHCI is non-functional after first suspend/resume, and
>>>kills machine during secon suspend/resume cycle.
>>
>>Hmm, last time I tested suspend/resume it worked fine.
>>That was 2.5.67, but the OHCI code hasn't had any
>>relevant changes since then.
> 
> 
>>Evidently your system used different suspend/resume paths
>>than mine did ... :)
> 
> 
> Can you try echo 4 > /proc/acpi/sleep? echo 3 breaks it, too, but that
> is little harder to set up.

I usually test with "apm -s" ... since I've yet to come up with
an ACPI configuration that works properly.  IRQ misconfiguration
for USB is still a blocking issue for many people (not just me).

Going through ACPI would certainly explain some breakage; it's
been sufficiently troublesome with USB that it's not gotten much
testing at all.  I happened to notice this morning that ACPI's
USB IRQ problems are one of the longest-standing open 2.6 bugs:
http://bugme.osdl.org/show_bug.cgi?id=10 ... and it's now been
migrated into the 2.4.22-pre series (sigh).

Could you try reproducing this failure using just APM?  I could
believe there's a generic PM issue (I've been expecting 2.6-test
to eventually start shaking PM out); but given the amount of
trouble ACPI has caused, we should first rule that factor out.


>>>What happens is that ohci_irq gets ohci->hcca == NULL, and kills
>>>machine. Why is ohci->hcca == NULL? ohci_stop was called from
>>>hcd_panic() and freed ohci->hcca.
>>
>>Then the problem is that an IRQ is still coming in after the
>>HCD panicked.
> 
> 
> Actually, as PCI interrupts are shared, I do not find that too
> surprising. 

I do.  Sharing is irrelevant.  If it's been cleaned up, then
the IRQ should no longer be bound to that device.



>>>I believe that we should
>>>
>>>1) not free ohci->hcca so that system has better chance surviving
>>>hcd_panic()
>>
>>Not ever????
>>
>>It's freed in exactly one place, after everything should be
>>shut down.  If it wasn't shut down, that was the problem.
>>
>>Could you instead figure out why it wasn't shut down?
> 
> 
> In case of hcd_panic(), where is interrupt deallocated?

I'll have to check that out, but I noticed that one of my
usual development machines (on which suspend/resume can
actually be made to work) is now unusable due to some PCI
initialization issue with Cardbus.


>>>and 
>>>
>>>2) inform user when hcd panics.
>>
>>The OHCI code does that already on the normal panic path
>>(controller delivers a Unrecoverable Error interrupt), but
>>you're right that this would be better as a generic KERN_CRIT
>>diagnostic.  (But one saying which HCD panicked, rather than
>>leaving folk to guess which of the N it applied to...)  And
>>I'd print that message sooner, not waiting for that task to
>>be scheduled.
> 
> 
> That would be good. I definitely had another failure path, where it
> did not tell me that hcd is no K.O...

I'll likely submit that to Greg in the next few days, cc you.

- Dave




> 								Pavel

Re: [linux-usb-devel] OHCI problems with suspend/resume

[Pavel Machek]

Hi!

> >Can you try echo 4 > /proc/acpi/sleep? echo 3 breaks it, too, but that
> >is little harder to set up.
> 
> I usually test with "apm -s" ... since I've yet to come up with
> an ACPI configuration that works properly.  IRQ misconfiguration
> for USB is still a blocking issue for many people (not just me).

Actually, you do not need acpi to test swsusp (echo 4 >
/proc/acpi/sleep is the easiest way, but there's syscall to do the
same).

> Going through ACPI would certainly explain some breakage; it's
> been sufficiently troublesome with USB that it's not gotten much
> testing at all.  I happened to notice this morning that ACPI's
> USB IRQ problems are one of the longest-standing open 2.6 bugs:
> http://bugme.osdl.org/show_bug.cgi?id=10 ... and it's now been
> migrated into the 2.4.22-pre series (sigh).

That's okay, ACPI in 2.4.22-pre just replaced older ACPI and
even-more-buggy ACPI in 2.4.21.

> Could you try reproducing this failure using just APM?  I could
> believe there's a generic PM issue (I've been expecting 2.6-test
> to eventually start shaking PM out); but given the amount of
> trouble ACPI has caused, we should first rule that factor out.

ACPI is not much involved. If you want, I can (attempt to) make swsusp
work on your machine....

> >Actually, as PCI interrupts are shared, I do not find that too
> >surprising. 
> 
> I do.  Sharing is irrelevant.  If it's been cleaned up, then
> the IRQ should no longer be bound to that device.

ohci_stop() does not seem to unregister IRQ...

> >That would be good. I definitely had another failure path, where it
> >did not tell me that hcd is no K.O...
> 
> I'll likely submit that to Greg in the next few days, cc you.

Okay, thanx.
								Pavel
-- 
When do you have a heart between your knees?
[Johanka's followup: and *two* hearts?]

Re: OHCI problems with suspend/resume

[Dominik Brugger]

> In 2.6.0-test1, OHCI is non-functional after first suspend/resume, and
> kills machine during secon suspend/resume cycle.

I experience similar problems with UHCI:

Before suspending to S3 I unload usb modules and afterwards I reload them, but it does not work again.
(For testing I also plugged in an USB card reader with an LED.
The LED is on except when I resume from S3)

Here is some dmesg output

After booting:

drivers/usb/core/usb.c: registered new driver usbfs
drivers/usb/core/usb.c: registered new driver hub
drivers/usb/host/uhci-hcd.c: USB Universal Host Controller Interface driver v2.1
uhci-hcd 0000:00:11.2: VIA Technologies, In USB
uhci-hcd 0000:00:11.2: irq 10, io base 0000dc00
uhci-hcd 0000:00:11.2: new USB bus registered, assigned bus number 1
drivers/usb/host/uhci-hcd.c: detected 2 ports
uhci-hcd 0000:00:11.2: root hub device address 1
usb usb1: new device strings: Mfr=3, Product=2, SerialNumber=1
usb usb1: Product: VIA Technologies, In USB
usb usb1: Manufacturer: Linux 2.6.0-test1-bk2 uhci-hcd
usb usb1: SerialNumber: 0000:00:11.2
drivers/usb/core/usb.c: usb_hotplug
usb usb1: usb_new_device - registering interface 1-0:0
drivers/usb/core/usb.c: usb_hotplug
hub 1-0:0: usb_device_probe
hub 1-0:0: usb_device_probe - got id
hub 1-0:0: USB hub found
hub 1-0:0: 2 ports detected
hub 1-0:0: standalone hub
hub 1-0:0: ganged power switching
hub 1-0:0: global over-current protection
hub 1-0:0: Port indicators are not supported
hub 1-0:0: power on to power good time: 2ms
hub 1-0:0: hub controller current requirement: 0mA
hub 1-0:0: local power source is good
hub 1-0:0: no over-current condition exists
hub 1-0:0: enabling power on all ports
hub 1-0:0: port 1, status 100, change 3, 12 Mb/s
hub 1-0:0: port 2, status 301, change 3, 1.5 Mb/s
hub 1-0:0: debounce: port 2: delay 100ms stable 4 status 0x301
hub 1-0:0: new USB device on port 2, assigned address 2
usb 1-2: new device strings: Mfr=1, Product=2, SerialNumber=0
usb 1-2: Product: USB-PS/2 Optical Mouse
usb 1-2: Manufacturer: Logitech
drivers/usb/core/usb.c: usb_hotplug
usb 1-2: usb_new_device - registering interface 1-2:0
drivers/usb/core/usb.c: usb_hotplug
hub 1-0:0: port 1 enable change, status 100
hid 1-2:0: usb_device_probe
hid 1-2:0: usb_device_probe - got id
input: USB HID v1.10 Mouse [Logitech USB-PS/2 Optical Mouse] on usb-0000:00:11.2-2
drivers/usb/core/usb.c: registered new driver hid
drivers/usb/input/hid-core.c: v2.0:USB HID core driver

rmmod uhci_hcd hid usbcore:

uhci-hcd 0000:00:11.2: remove, state 3
uhci-hcd 0000:00:11.2: roothub graceful disconnect
usb usb1: USB disconnect, address 1
usb 1-2: USB disconnect, address 2
drivers/usb/core/usb.c: nuking urbs assigned to 1-2
usb 1-2: unregistering interfaces
drivers/usb/core/usb.c: nuking urbs assigned to 1-2
usb 1-2: hcd_unlink_urb df5047c0 fail -22
usb 1-2: hcd_unlink_urb df504740 fail -22
drivers/usb/core/usb.c: usb_hotplug
usb 1-2: unregistering device
drivers/usb/core/usb.c: usb_hotplug
drivers/usb/core/usb.c: nuking urbs assigned to usb1
uhci-hcd 0000:00:11.2: shutdown urb df504940 pipe 40408180 ep1in-intr
usb usb1: unregistering interfaces
drivers/usb/core/usb.c: nuking urbs assigned to usb1
usb usb1: hcd_unlink_urb df504940 fail -22
drivers/usb/core/usb.c: usb_hotplug
usb usb1: unregistering device
drivers/usb/core/usb.c: usb_hotplug
uhci-hcd 0000:00:11.2: USB bus 1 deregistered
drivers/usb/core/usb.c: deregistering driver hid
drivers/usb/core/usb.c: deregistering driver usbfs
drivers/usb/core/hub.c: hub_thread exiting
drivers/usb/core/usb.c: deregistering driver hub

echo 3 > /proc/acpi/sleep
modprobe usbcore uhci_hcd

(Sometimes the optical mouse blinks for a moment)

drivers/usb/core/usb.c: registered new driver usbfs
drivers/usb/core/usb.c: registered new driver hub
drivers/usb/host/uhci-hcd.c: USB Universal Host Controller Interface driver v2.1
uhci-hcd 0000:00:11.2: VIA Technologies, In USB
uhci-hcd 0000:00:11.2: irq 10, io base 0000dc00
uhci-hcd 0000:00:11.2: new USB bus registered, assigned bus number 1
drivers/usb/host/uhci-hcd.c: detected 2 ports
uhci-hcd 0000:00:11.2: root hub device address 1
usb usb1: new device strings: Mfr=3, Product=2, SerialNumber=1
usb usb1: Product: VIA Technologies, In USB
usb usb1: Manufacturer: Linux 2.6.0-test1-bk2 uhci-hcd
usb usb1: SerialNumber: 0000:00:11.2
drivers/usb/core/usb.c: usb_hotplug
usb usb1: usb_new_device - registering interface 1-0:0
drivers/usb/core/usb.c: usb_hotplug
hub 1-0:0: usb_device_probe
hub 1-0:0: usb_device_probe - got id
hub 1-0:0: USB hub found
hub 1-0:0: 2 ports detected
hub 1-0:0: standalone hub
hub 1-0:0: ganged power switching
hub 1-0:0: global over-current protection
hub 1-0:0: Port indicators are not supported
hub 1-0:0: power on to power good time: 2ms
hub 1-0:0: hub controller current requirement: 0mA
hub 1-0:0: local power source is good
hub 1-0:0: no over-current condition exists
hub 1-0:0: enabling power on all ports
hub 1-0:0: port 1, status 100, change 3, 12 Mb/s
hub 1-0:0: port 2, status 301, change 3, 1.5 Mb/s
hub 1-0:0: debounce: port 2: delay 100ms stable 4 status 0x301
hub 1-0:0: new USB device on port 2, assigned address 2
drivers/usb/core/message.c: usb_control/bulk_msg: timeout

-Dominik Brugger

Re: OHCI problems with suspend/resume

[Dominik Brugger]

Hello again,

I have a few things to add:

1) If uhci_hcd was never loaded before suspend, one has to press a key on the keyboard after `echo 3 > /proc/acpi/sleep` in order to get the machine suspended. Wakeup does not work in that case, reboot required.

2) It makes no difference whether the Logitech USB Mouse is connected or not.

-Dominik Brugger

Re: OHCI problems with suspend/resume

[Pavel Machek]

Hi!

> > In 2.6.0-test1, OHCI is non-functional after first suspend/resume, and
> > kills machine during secon suspend/resume cycle.
> 
> I experience similar problems with UHCI:

Strange, UHCI worked for me last time I tried. I'll try again soon...
								Pavel
-- 
When do you have a heart between your knees?
[Johanka's followup: and *two* hearts?]

Re: OHCI problems with suspend/resume

[Pavel Machek]

Hi!

> > In 2.6.0-test1, OHCI is non-functional after first suspend/resume, and
> > kills machine during secon suspend/resume cycle.
> 
> I experience similar problems with UHCI:
> 
> Before suspending to S3 I unload usb modules and afterwards I reload them, but it does not work again.
> (For testing I also plugged in an USB card reader with an LED.
> The LED is on except when I resume from S3)

Do not unload modules before suspend, they should handle that just
fine.

With uhci on toshiba satelite I can do both S3 and S4, and USB
survives that (at least it is able to power devices and detects
plugs/unplugs).
								Pavel
-- 
When do you have a heart between your knees?
[Johanka's followup: and *two* hearts?]

Re: OHCI problems with suspend/resume

[Dominik Brugger]

Good morning,

> Do not unload modules before suspend, they should handle that just
> fine.

If I keep them loaded during suspend:

[...]
Suspending devices
uhci-hcd 0000:00:11.2: suspend to state 3
drivers/usb/host/uhci-hcd.c: dc00: suspend_hc
 hwsleep-0257 [10] acpi_enter_sleep_state: Entering sleep state [S3]
Enabling SEP on CPU 0
Back to C!
Devices Resumed
uhci-hcd 0000:00:11.2: resume
[...]
drivers/usb/host/uhci-hcd.c: dc00: wakeup_hc

Mouse doesn't work, unplug:

hub 1-0:0: port 1, status 300, change 3, 1.5 Mb/s
usb 1-1: USB disconnect, address 2
drivers/usb/core/usb.c: nuking urbs assigned to 1-1
usb 1-1: unregistering interfaces
drivers/usb/core/usb.c: nuking urbs assigned to 1-1
usb 1-1: hcd_unlink_urb df7be640 fail -22
usb 1-1: hcd_unlink_urb df7be540 fail -22
drivers/usb/core/usb.c: usb_hotplug
usb 1-1: unregistering device
drivers/usb/core/usb.c: usb_hotplug
hub 1-0:0: port 1 enable change, status 300
drivers/usb/host/uhci-hcd.c: dc00: suspend_hc

replug:

hub 1-0:0: port 1, status 301, change 3, 1.5 Mb/s
hub 1-0:0: debounce: port 1: delay 100ms stable 4 status 0x301
hub 1-0:0: new USB device on port 1, assigned address 3
drivers/usb/core/message.c: usb_control/bulk_msg: timeout

Further attempts to un/re-plug the device show no response.

(Same with Canon Powershot instead of Logitech USB Mouse)

> With uhci on toshiba satelite I can do both S3 and S4, and USB
> survives that (at least it is able to power devices and detects
> plugs/unplugs).

In my case it is an Epox 8kha+ board, the problem should not be hardware specific since Suspend To RAM works under WinXP.

I will try S4 lateron.

-Dominik Brugger

Re: [linux-usb-devel] Re: OHCI problems with suspend/resume

[Alan Stern]

On Fri, 25 Jul 2003, Dominik Brugger wrote:

> Good morning,
> 
> > Do not unload modules before suspend, they should handle that just
> > fine.
> 
> If I keep them loaded during suspend:
> 
> [...]
> Suspending devices
> uhci-hcd 0000:00:11.2: suspend to state 3
> drivers/usb/host/uhci-hcd.c: dc00: suspend_hc
>  hwsleep-0257 [10] acpi_enter_sleep_state: Entering sleep state [S3]
> Enabling SEP on CPU 0
> Back to C!
> Devices Resumed
> uhci-hcd 0000:00:11.2: resume
> [...]
> drivers/usb/host/uhci-hcd.c: dc00: wakeup_hc
> 
> Mouse doesn't work, unplug:
> 
> hub 1-0:0: port 1, status 300, change 3, 1.5 Mb/s
> usb 1-1: USB disconnect, address 2
> drivers/usb/core/usb.c: nuking urbs assigned to 1-1
> usb 1-1: unregistering interfaces
> drivers/usb/core/usb.c: nuking urbs assigned to 1-1
> usb 1-1: hcd_unlink_urb df7be640 fail -22
> usb 1-1: hcd_unlink_urb df7be540 fail -22
> drivers/usb/core/usb.c: usb_hotplug
> usb 1-1: unregistering device
> drivers/usb/core/usb.c: usb_hotplug
> hub 1-0:0: port 1 enable change, status 300
> drivers/usb/host/uhci-hcd.c: dc00: suspend_hc
> 
> replug:
> 
> hub 1-0:0: port 1, status 301, change 3, 1.5 Mb/s
> hub 1-0:0: debounce: port 1: delay 100ms stable 4 status 0x301
> hub 1-0:0: new USB device on port 1, assigned address 3
> drivers/usb/core/message.c: usb_control/bulk_msg: timeout
> 
> Further attempts to un/re-plug the device show no response.
> 
> (Same with Canon Powershot instead of Logitech USB Mouse)
> 
> > With uhci on toshiba satelite I can do both S3 and S4, and USB
> > survives that (at least it is able to power devices and detects
> > plugs/unplugs).
> 
> In my case it is an Epox 8kha+ board, the problem should not be hardware specific since Suspend To RAM works under WinXP.
> 
> I will try S4 lateron.
> 
> -Dominik Brugger

This could very well be related to the problem reported by Paul Mackerras 
about a month or two ago, see:

http://sourceforge.net/mailarchive/message.php?msg_id=5176974

I think the hub driver's power management code may be at fault.  It needs
to cancel the status interrupt URB when suspending and resubmit it when
waking up; right now it probably does neither one.

Or maybe I'm wrong about that.  Perhaps it's okay to leave the URB active.  
If that's the case, then the root hub power management code needs to be 
changed to restart the status URB timer after a wakeup.

I'm not sure how the design is intended to work, but either way something 
needs to be fixed.

Alan Stern

Re: [linux-usb-devel] Re: OHCI problems with suspend/resume

[Benjamin Herrenschmidt]

> I think the hub driver's power management code may be at fault.  It needs
> to cancel the status interrupt URB when suspending and resubmit it when
> waking up; right now it probably does neither one.
> 
> Or maybe I'm wrong about that.  Perhaps it's okay to leave the URB active.  
> If that's the case, then the root hub power management code needs to be 
> changed to restart the status URB timer after a wakeup.
> 
> I'm not sure how the design is intended to work, but either way something 
> needs to be fixed.

Could well be. I need to spend some time auditing power management
in the USB drivers in general. The idea here is that a sub-driver
(USB device driver) should make sure it has no more pending URBs
when returning from suspend() and the HCD driver should just cancel
pending URBs if still any and reject any one that would be submited
before it's woken up. It's especially very important on OHCI to not
touch chip registers (like enabling bulk queue etc...) after the chip
have been put to suspend state. On some chips, that can cause random
bus mastering to main memory during sleep, which can cause all sorts
of interesting results especially when the north bridge is asleep...

Ben.

Re: [linux-usb-devel] Re: OHCI problems with suspend/resume

[David Brownell]

Benjamin Herrenschmidt wrote, responding to Alan Stern:
>>I think the hub driver's power management code may be at fault.  It needs
>>to cancel the status interrupt URB when suspending and resubmit it when
>>waking up; right now it probably does neither one.
>>
>>Or maybe I'm wrong about that.  Perhaps it's okay to leave the URB active.  
>>If that's the case, then the root hub power management code needs to be 
>>changed to restart the status URB timer after a wakeup.

I thought that patch got merged already ...


>>I'm not sure how the design is intended to work, but either way something 
>>needs to be fixed.

Yes, it seems like all the HCDs (and the hub driver) need attention.

Plus, the enumeration process should respect hubs' power budgets,
and handle overcurrent better.  I had a hub re-enumerate over forty
times not that long ago, just because it enabled too many things at
once and the surge currents made lots of trouble.  Plenty of power,
if it got turned on carefully enough... :)


> Could well be. I need to spend some time auditing power management
> in the USB drivers in general. The idea here is that a sub-driver
> (USB device driver) should make sure it has no more pending URBs
> when returning from suspend() and the HCD driver should just cancel
> pending URBs if still any and reject any one that would be submited

Agreed, this needs work.  Some USB device drivers will likely need to
implement suspend()/resume() callbacks, which thoughtfully enough the
driver model conversion already gave us.  At one point it was planned
to have it automatically traverse the devices and suspend, leaves up to
root; and resume in the reverse order.  Is that behaving now?

Suspend should likely enable remote wakeup (trevor.pering@intel.com
has been asking about that), at least as a config option.  That'll
be useful for things like keyboards and mice.

Now's probably a good time to work on this sort of integration issue.
I'll be glad to see contributions in this area.


> before it's woken up. It's especially very important on OHCI to not
> touch chip registers (like enabling bulk queue etc...) after the chip
> have been put to suspend state. On some chips, that can cause random
> bus mastering to main memory during sleep, which can cause all sorts
> of interesting results especially when the north bridge is asleep...

There's already code in the OHCI driver to handle that for some
of the Apple hardware, as you know.  And it should kick in on at
least some of the power management paths; although not all, as
Pavel has noted.

- Dave

Re: [linux-usb-devel] Re: OHCI problems with suspend/resume

[Alan Stern]

On Fri, 25 Jul 2003, David Brownell wrote:

> Benjamin Herrenschmidt wrote, responding to Alan Stern:
> >>I think the hub driver's power management code may be at fault.  It needs
> >>to cancel the status interrupt URB when suspending and resubmit it when
> >>waking up; right now it probably does neither one.
> >>
> >>Or maybe I'm wrong about that.  Perhaps it's okay to leave the URB active.  
> >>If that's the case, then the root hub power management code needs to be 
> >>changed to restart the status URB timer after a wakeup.
> 
> I thought that patch got merged already ...
> 
> 
> >>I'm not sure how the design is intended to work, but either way something 
> >>needs to be fixed.
> 
> Yes, it seems like all the HCDs (and the hub driver) need attention.

So far as I can see, there's no USB power management code at all apart 
from the HC drivers.  That includes the hub driver.

And I know that the UHCI suspend routine could use a little work.  There's 
a 10-Hz polling timer that it doesn't turn off.

Alan Stern

Re: [linux-usb-devel] Re: OHCI problems with suspend/resume

[Pavel Machek]

Hi!

> >>I'm not sure how the design is intended to work, but either way something 
> >>needs to be fixed.
> 
> Yes, it seems like all the HCDs (and the hub driver) need attention.

Why the hub driver?

For basic functionality, you simply power it down (doing virtual
unplug), and power it back up on resume (doing virtual plug of all
devices). That should work reasonably for everything but mass-storage.

> Plus, the enumeration process should respect hubs' power budgets,
> and handle overcurrent better.  I had a hub re-enumerate over forty
> times not that long ago, just because it enabled too many things at
> once and the surge currents made lots of trouble.  Plenty of power,
> if it got turned on carefully enough... :)

Havin enough juice in "common case", but not in "worst case" is not
too legal situation, is it?

> >Could well be. I need to spend some time auditing power management
> >in the USB drivers in general. The idea here is that a sub-driver
> >(USB device driver) should make sure it has no more pending URBs
> >when returning from suspend() and the HCD driver should just cancel
> >pending URBs if still any and reject any one that would be submited
> 
> Agreed, this needs work.  Some USB device drivers will likely need to
> implement suspend()/resume() callbacks, which thoughtfully enough the
> driver model conversion already gave us.  At one point it was planned
> to have it automatically traverse the devices and suspend, leaves up to
> root; and resume in the reverse order.  Is that behaving now?

Yes.

> Suspend should likely enable remote wakeup (trevor.pering@intel.com
> has been asking about that), at least as a config option.  That'll
> be useful for things like keyboards and mice.

Okay, but we need normal suspend/resume working first :-).

								Pavel
-- 
When do you have a heart between your knees?
[Johanka's followup: and *two* hearts?]

Re: [linux-usb-devel] Re: OHCI problems with suspend/resume

[David Brownell]

Pavel Machek wrote:
> Hi!
> 
> 
>>>>I'm not sure how the design is intended to work, but either way something 
>>>>needs to be fixed.
>>
>>Yes, it seems like all the HCDs (and the hub driver) need attention.
> 
> 
> Why the hub driver?
> 
> For basic functionality, you simply power it down (doing virtual
> unplug), and power it back up on resume (doing virtual plug of all
> devices). That should work reasonably for everything but mass-storage.

For non-basic functionality such as "remote wakeup", where you
can wake the system up from its suspension by doing things like
typing on the USB keyboard.


>>Plus, the enumeration process should respect hubs' power budgets,
>>and handle overcurrent better.  I had a hub re-enumerate over forty
>>times not that long ago, just because it enabled too many things at
>>once and the surge currents made lots of trouble.  Plenty of power,
>>if it got turned on carefully enough... :)
> 
> 
> Havin enough juice in "common case", but not in "worst case" is not
> too legal situation, is it?

This was a perfectly legal configuration, with what I recall as
four devices.  It was pretty far from "worst case".

- Dave

Re: [linux-usb-devel] Re: OHCI problems with suspend/resume

[Alan Stern]

On Sat, 26 Jul 2003, Pavel Machek wrote:

> Hi!
> 
> > >>I'm not sure how the design is intended to work, but either way something 
> > >>needs to be fixed.
> > 
> > Yes, it seems like all the HCDs (and the hub driver) need attention.
> 
> Why the hub driver?
> 
> For basic functionality, you simply power it down (doing virtual
> unplug), and power it back up on resume (doing virtual plug of all
> devices). That should work reasonably for everything but mass-storage.

Almost correct.  It's also necessary to stop/reinitialize a periodic
status request.

None of this functionality has been implemented yet.

Alan Stern

Re: [linux-usb-devel] Re: OHCI problems with suspend/resume

[David Brownell]

Pavel Machek wrote:

>>Agreed, this needs work.  Some USB device drivers will likely need to
>>implement suspend()/resume() callbacks, which thoughtfully enough the
>>driver model conversion already gave us.  At one point it was planned
>>to have it automatically traverse the devices and suspend, leaves up to
>>root; and resume in the reverse order.  Is that behaving now?
> 
> 
> Yes.

Well, partially; but it's not used consistently.  Could you
(or someone) explain what the plan is?  I see:

  - Three separate x86 PM "initiators":  APM, ACPI, swsusp.
    (Plus ones for ARM and MIPS.)

  - Two driver registration infrastructures, the driver model
    stuff and the older pm_*() stuff.

The pm_*() is how a handful of sound drivers and other random
stuff register themselves -- and how PCI does it.

I'd sure have expected PCI to only use the driver model stuff,
and I'll hope all those users will all be phased out by the
time that 2.6 gets near the end of its test cycle.


The "initiators" all talk to _both_ infrastructures, but they
don't talk to the driver model stuff in the same way.  For
example, on suspend:

  - ACPI issues a NOTIFY, which can veto the suspend;
    then SAVE_STATE, ditto; finally POWER_DOWN.

  - APM uses the pm_*() calls for a vetoable check,
    never issues SAVE_STATE, then goes POWER_DOWN.

  - While swsusp is more like ACPI except that it doesn't
    support vetoing from either NOTIFY or SAVE_STATE.

That all seems more problematic to me.  Seems to me that
APM should issue SAVE_STATE (and RESTORE_STATE), and all
three PM "initiators" should agree on vetoing.

For USB, the {SAVE,RESTORE}_STATE calls would be the most
natural place to do the "kill pending urbs" calls that
Alan Stern mentioned -- at least, for D3 or swsusp levels.
Likely for D1 and D2, devices with pending I/O won't really
be keen on from suspending.


Now, for the record I tried to suspend a test1 APM-works
system, with UHCI, and it wouldn't resume -- unclear why,
or if test2 will do the same.

A different APM-works system, with OHCI and test2, did
suspend/resume OK; but something went wrong with OHCI
even before any driver model PM calls happened, if the
OHCI driver was active (doing DMA):  the HCD got an
"Unrecoverable Error" IRQ, which generally means that
some kind DMA fault appeared.

All of which is a roundabout way of adding to what I
said:  the PM infrastructure USB will need to rely on
seems like it needs polishing yet!  :)

- Dave

Re: [linux-usb-devel] Re: OHCI problems with suspend/resume

[David Brownell]

David Brownell wrote:

> The pm_*() is how a handful of sound drivers and other random
> stuff register themselves -- and how PCI does it.

To clarify, before someone jumps all over me:  look inside
the PCI code, you'll see that PCI as a whole is registered
using the pm_*() stuff for power management.  Yes, when
those callbacks get made, they morph into driver model calls.

Which means that PCI devices (like OHCI) don't fit into the
suspend/resume sequence in the way one would expect.  It
might explain why I'm still seeing two equivalent suspend
notifications in the OHCI code...

- Dave


> I'd sure have expected PCI to only use the driver model stuff,
> and I'll hope all those users will all be phased out by the
> time that 2.6 gets near the end of its test cycle.

Re: [linux-usb-devel] Re: OHCI problems with suspend/resume

[Pavel Machek]

Hi!

> >>root; and resume in the reverse order.  Is that behaving now?
> >
> >
> >Yes.
> 
> Well, partially; but it's not used consistently.  Could you
> (or someone) explain what the plan is?  I see:
> 
>  - Three separate x86 PM "initiators":  APM, ACPI, swsusp.
>    (Plus ones for ARM and MIPS.)
> 
>  - Two driver registration infrastructures, the driver model
>    stuff and the older pm_*() stuff.
> 
> The pm_*() is how a handful of sound drivers and other random
> stuff register themselves -- and how PCI does it.
> 
> I'd sure have expected PCI to only use the driver model stuff,
> and I'll hope all those users will all be phased out by the
> time that 2.6 gets near the end of its test cycle.

Ouch, PCI really should use new driver model. I have not known about
that. I'll look into it.

> The "initiators" all talk to _both_ infrastructures, but they
> don't talk to the driver model stuff in the same way.  For
> example, on suspend:
> 
>  - ACPI issues a NOTIFY, which can veto the suspend;
>    then SAVE_STATE, ditto; finally POWER_DOWN.
> 
>  - APM uses the pm_*() calls for a vetoable check,
>    never issues SAVE_STATE, then goes POWER_DOWN.

IIRC, I had SAVE_STATE there at some point, and there was some
problem. I do not really use APM :-(.

>  - While swsusp is more like ACPI except that it doesn't
>    support vetoing from either NOTIFY or SAVE_STATE.

Good. I figured vetoing support is not critical. I should add it.

> That all seems more problematic to me.  Seems to me that
> APM should issue SAVE_STATE (and RESTORE_STATE), and all
> three PM "initiators" should agree on vetoing.

Yes, agreed.

> All of which is a roundabout way of adding to what I
> said:  the PM infrastructure USB will need to rely on
> seems like it needs polishing yet!  :)

Do you need vetoing? Otherwise it should be ready, except for APM.

								Pavel
-- 
When do you have a heart between your knees?
[Johanka's followup: and *two* hearts?]

Re: [linux-usb-devel] Re: OHCI problems with suspend/resume

[David Brownell]

Pavel Machek wrote:

>>All of which is a roundabout way of adding to what I
>>said:  the PM infrastructure USB will need to rely on
>>seems like it needs polishing yet!  :)
> 
> 
> Do you need vetoing? Otherwise it should be ready, except for APM.

USB drivers don't talk suspend/resume yet, so they
won't notice missing features there.  Regressions
are a different story though.

But I can imagine that usb-storage (or is that SCSI?)
might want to veto suspending devices that are being
used for some kinds of i/o.  Eventually it should exist.

- Dave

Re: [linux-usb-devel] Re: OHCI problems with suspend/resume

[Pavel Machek]

Hi!

> >>All of which is a roundabout way of adding to what I
> >>said:  the PM infrastructure USB will need to rely on
> >>seems like it needs polishing yet!  :)
> >
> >
> >Do you need vetoing? Otherwise it should be ready, except for APM.
> 
> USB drivers don't talk suspend/resume yet, so they
> won't notice missing features there.  Regressions
> are a different story though.
> 
> But I can imagine that usb-storage (or is that SCSI?)
> might want to veto suspending devices that are being
> used for some kinds of i/o.  Eventually it should exist.

For what kind of I/O? I do not see a reason for disk to veto
suspend. CD-burner might want to do that, but it still would be bad
idea... (Running on battery, battery goes low, and you destroy your CD
*and* your filesystem.
								Pavel
-- 
Horseback riding is like software...
...vgf orggre jura vgf serr.

Re: [linux-usb-devel] Re: OHCI problems with suspend/resume

[David Brownell]

Pavel Machek wrote:

>>USB drivers don't talk suspend/resume yet, so they
>>won't notice missing features there.  Regressions
>>are a different story though.
>>
>>But I can imagine that usb-storage (or is that SCSI?)
>>might want to veto suspending devices that are being
>>used for some kinds of i/o.  Eventually it should exist.
> 
> 
> For what kind of I/O? I do not see a reason for disk to veto
> suspend. CD-burner might want to do that, but it still would be bad
> idea... (Running on battery, battery goes low, and you destroy your CD
> *and* your filesystem.

If it's in the middle of any kind of write, suspending would
seem to be unwise.  Say, writing to a swap partition...

Mostly I'm just saying that if vetoing ever makes sense
(and I understand that it does), USB drivers will need
to understand it too.

- Dave

Re: [linux-usb-devel] Re: OHCI problems with suspend/resume

[Pavel Machek]

Hi!

> >>USB drivers don't talk suspend/resume yet, so they
> >>won't notice missing features there.  Regressions
> >>are a different story though.
> >>
> >>But I can imagine that usb-storage (or is that SCSI?)
> >>might want to veto suspending devices that are being
> >>used for some kinds of i/o.  Eventually it should exist.
> >
> >
> >For what kind of I/O? I do not see a reason for disk to veto
> >suspend. CD-burner might want to do that, but it still would be bad
> >idea... (Running on battery, battery goes low, and you destroy your CD
> >*and* your filesystem.
> 
> If it's in the middle of any kind of write, suspending would
> seem to be unwise.  Say, writing to a swap partition...

We already know it is not... We stopped all user tasks for this
purpose. Also the device has opportunity to simply wait inside
SAVE_STATE for a write to finish.

> Mostly I'm just saying that if vetoing ever makes sense
> (and I understand that it does), USB drivers will need
> to understand it too.

Agreed.
							Pavel

-- 
When do you have a heart between your knees?
[Johanka's followup: and *two* hearts?]

Re: [linux-usb-devel] Re: OHCI problems with suspend/resume

[Benjamin Herrenschmidt]

> If it's in the middle of any kind of write, suspending would
> seem to be unwise.  Say, writing to a swap partition...

No, you wait for pending requests to complete and keep further
ones in the queue. I will probably write the proper infrastructure
for SCSI hosts later this month, it has to be done like IDE, with
a special BIO "suspend" request so that things gets properly
synchronized.

> Mostly I'm just saying that if vetoing ever makes sense
> (and I understand that it does), USB drivers will need
> to understand it too.
> 
> - Dave
-- 
Benjamin Herrenschmidt <benh@kernel.crashing.org>

Re: [linux-usb-devel] Re: OHCI problems with suspend/resume

[Benjamin Herrenschmidt]

> For what kind of I/O? I do not see a reason for disk to veto
> suspend. CD-burner might want to do that, but it still would be bad
> idea... (Running on battery, battery goes low, and you destroy your CD
> *and* your filesystem.

Well... that's userland policy. You can have a notion of "severity" of
the suspend request, like vetoing a user-requested suspend request but
not one issuing from a very-low-battery warning...

This is a part of the PM playfield that still is mostly to be done...

Ben.

Re: [linux-usb-devel] Re: OHCI problems with suspend/resume

[Benjamin Herrenschmidt]

On Thu, 2003-07-31 at 15:37, David Brownell wrote:
> Pavel Machek wrote:
> 
> >>All of which is a roundabout way of adding to what I
> >>said:  the PM infrastructure USB will need to rely on
> >>seems like it needs polishing yet!  :)
> > 
> > 
> > Do you need vetoing? Otherwise it should be ready, except for APM.
> 
> USB drivers don't talk suspend/resume yet, so they
> won't notice missing features there.  Regressions
> are a different story though.
> 
> But I can imagine that usb-storage (or is that SCSI?)
> might want to veto suspending devices that are being
> used for some kinds of i/o.  Eventually it should exist.

The only place where a kernel driver may veto is when doing things
like firmware flashing.

If you want to veto suspend when burning a CD, it's userland policy. We
still need, I beleive, to define/write a proper userland-side API for
all these with the daemon that goes with it. That will be next step once
we have the kernel side working properly ;)

Ben.

Re: [linux-usb-devel] Re: OHCI problems with suspend/resume

[Pavel Machek]

Hi!

> Well, partially; but it's not used consistently.  Could you
> (or someone) explain what the plan is?  I see:
> 
>  - Three separate x86 PM "initiators":  APM, ACPI, swsusp.
>    (Plus ones for ARM and MIPS.)
> 
>  - Two driver registration infrastructures, the driver model
>    stuff and the older pm_*() stuff.
> 
> The pm_*() is how a handful of sound drivers and other random
> stuff register themselves -- and how PCI does it.
> 
> I'd sure have expected PCI to only use the driver model stuff,
> and I'll hope all those users will all be phased out by the
> time that 2.6 gets near the end of its test cycle.
> 
> 
> The "initiators" all talk to _both_ infrastructures, but they
> don't talk to the driver model stuff in the same way.  For
> example, on suspend:
> 
>  - ACPI issues a NOTIFY, which can veto the suspend;
>    then SAVE_STATE, ditto; finally POWER_DOWN.
> 
>  - APM uses the pm_*() calls for a vetoable check,
>    never issues SAVE_STATE, then goes POWER_DOWN.
> 
>  - While swsusp is more like ACPI except that it doesn't
>    support vetoing from either NOTIFY or SAVE_STATE.

Where does acpi call pm_*()? It seems like it does not and it seems
like a bug to me.
								Pavel
-- 
When do you have a heart between your knees?
[Johanka's followup: and *two* hearts?]

Re: [linux-usb-devel] Re: OHCI problems with suspend/resume

[David Brownell]

Pavel Machek wrote:
>>The "initiators" all talk to _both_ infrastructures, but they
>>don't talk to the driver model stuff in the same way.  For
>>example, on suspend:
>>
>> ...
> 
> Where does acpi call pm_*()? It seems like it does not and it seems
> like a bug to me.

Doesn't; it showed up in a 'grep' that I should have examined
more closely.  Sorry!  But both swsusp and APM have the "using
both registration schemes" issue (consistency matters here).

Why does it seem like a bug -- because it's using only the "new"
infrastructure, while there are still a few drivers that only
register to the old one?  I'd rather see those drivers (about
a dozen) have compile time #ifdef CONFIG_PM #warnings, and call
them the bug...

- Dave

Re: [linux-usb-devel] Re: OHCI problems with suspend/resume

[Pavel Machek]

Hi!

> >>The "initiators" all talk to _both_ infrastructures, but they
> >>don't talk to the driver model stuff in the same way.  For
> >>example, on suspend:

> >Where does acpi call pm_*()? It seems like it does not and it seems
> >like a bug to me.
> 
> Doesn't; it showed up in a 'grep' that I should have examined
> more closely.  Sorry!  But both swsusp and APM have the "using
> both registration schemes" issue (consistency matters here).
> 
> Why does it seem like a bug -- because it's using only the "new"
> infrastructure, while there are still a few drivers that only
> register to the old one?  

Yes.

> I'd rather see those drivers (about
> a dozen) have compile time #ifdef CONFIG_PM #warnings, and call
> them the bug...

I guess I like that, too.. But at this point whole PCI relies on
old-style pm_*(); that needs to be fixed, first.
							Pavel
-- 
When do you have a heart between your knees?
[Johanka's followup: and *two* hearts?]

Re: [linux-usb-devel] Re: OHCI problems with suspend/resume

[Pavel Machek]

Hi!

>  - APM uses the pm_*() calls for a vetoable check,
>    never issues SAVE_STATE, then goes POWER_DOWN.

I remember the reason... SAVE_STATE expects user processes to be
stopped, which is not the case in APM. Perhaps that is easy to fix
these days...
							Pavel
-- 
When do you have a heart between your knees?
[Johanka's followup: and *two* hearts?]

Re: [linux-usb-devel] Re: OHCI problems with suspend/resume

[David Brownell]

Pavel Machek wrote:
> Hi!
> 
> 
>> - APM uses the pm_*() calls for a vetoable check,
>>   never issues SAVE_STATE, then goes POWER_DOWN.
> 
> 
> I remember the reason... SAVE_STATE expects user processes to be
> stopped, which is not the case in APM. Perhaps that is easy to fix
> these days...
> 							Pavel


That SAVE_STATE restriction doesn't seem to be documented...

- Dave

Re: [linux-usb-devel] Re: OHCI problems with suspend/resume

[Pavel Machek]

Hi!

> >>- APM uses the pm_*() calls for a vetoable check,
> >>  never issues SAVE_STATE, then goes POWER_DOWN.
> >
> >
> >I remember the reason... SAVE_STATE expects user processes to be
> >stopped, which is not the case in APM. Perhaps that is easy to fix
> >these days...
> 
> That SAVE_STATE restriction doesn't seem to be documented...

It is not; but I could not convince myself that it is okay to call
SAVE_STATE with userland alive, and that's why I did not add
SAVE_STATE to apm.
							Pavel
-- 
When do you have a heart between your knees?
[Johanka's followup: and *two* hearts?]

Re: [linux-usb-devel] Re: OHCI problems with suspend/resume

[Benjamin Herrenschmidt]

On Thu, 2003-07-31 at 15:23, David Brownell wrote:
> Pavel Machek wrote:
> > Hi!
> > 
> > 
> >> - APM uses the pm_*() calls for a vetoable check,
> >>   never issues SAVE_STATE, then goes POWER_DOWN.
> > 
> > 
> > I remember the reason... SAVE_STATE expects user processes to be
> > stopped, which is not the case in APM. Perhaps that is easy to fix
> > these days...
> > 							Pavel
> 
> 
> That SAVE_STATE restriction doesn't seem to be documented...

Because it is plain wrong ;)

Ben.

Re: [linux-usb-devel] Re: OHCI problems with suspend/resume

[Benjamin Herrenschmidt]

On Thu, 2003-07-31 at 11:49, Pavel Machek wrote:
> Hi!
> 
> >  - APM uses the pm_*() calls for a vetoable check,
> >    never issues SAVE_STATE, then goes POWER_DOWN.
> 
> I remember the reason... SAVE_STATE expects user processes to be
> stopped, which is not the case in APM. Perhaps that is easy to fix
> these days...

No ! You may feel better stopping user processes (and actuallty you
may require that for swsusp, I don't know) but the whole PM scheme is
designed to make that unnecessary. I do NOT stop user processes on
suspend-to-RAM on PowerMacs, I don't think neither APM nor ACPI need
that (I may be wrong here, but if that is the case, then some drivers
need fixing).

Ben.

Re: [linux-usb-devel] Re: OHCI problems with suspend/resume

[Pavel Machek]

Hi!

> > >  - APM uses the pm_*() calls for a vetoable check,
> > >    never issues SAVE_STATE, then goes POWER_DOWN.
> > 
> > I remember the reason... SAVE_STATE expects user processes to be
> > stopped, which is not the case in APM. Perhaps that is easy to fix
> > these days...
> 
> No ! You may feel better stopping user processes (and actuallty you
> may require that for swsusp, I don't know) but the whole PM scheme is
> designed to make that unnecessary. I do NOT stop user processes on
> suspend-to-RAM on PowerMacs, I don't think neither APM nor ACPI need
> that (I may be wrong here, but if that is the case, then some drivers
> need fixing).

When all drivers are safe, stopping user processes on ACPI S3 will no
longer be needed.

But they are not safe for now, and I believe that stopping processes
makes some hw drivers safe where they were not safe before. At least
if process is sleeping within some driver, stopping user processes
will wait till it exits the driver. That should help for simple
devices.
								Pavel
-- 
When do you have a heart between your knees?
[Johanka's followup: and *two* hearts?]

Re: [linux-usb-devel] Re: OHCI problems with suspend/resume

[Benjamin Herrenschmidt]

> Well, partially; but it's not used consistently.  Could you
> (or someone) explain what the plan is?  I see:
> 
>   - Three separate x86 PM "initiators":  APM, ACPI, swsusp.
>     (Plus ones for ARM and MIPS.)

You should look at Patrick Mochel's stuff that shall be getting in
the official tree this month hopefully. It does that (among others)

>   - Two driver registration infrastructures, the driver model
>     stuff and the older pm_*() stuff.

The older pm_ model is deprecated.

> The pm_*() is how a handful of sound drivers and other random
> stuff register themselves -- and how PCI does it.
> 
> I'd sure have expected PCI to only use the driver model stuff,
> and I'll hope all those users will all be phased out by the
> time that 2.6 gets near the end of its test cycle.

PCI is broken since it does both (and thus, if we call both rounds
of notifiers, we end up suspending PCI twice, the second time without
any ordering constraints). In my trees, I comment out that "legacy"
stuff (though I also don't call the old-style pm_* stuff anymore
neither)

> The "initiators" all talk to _both_ infrastructures, but they
> don't talk to the driver model stuff in the same way.  For
> example, on suspend:
> 
>   - ACPI issues a NOTIFY, which can veto the suspend;
>     then SAVE_STATE, ditto; finally POWER_DOWN.
> 
>   - APM uses the pm_*() calls for a vetoable check,
>     never issues SAVE_STATE, then goes POWER_DOWN.
> 
>   - While swsusp is more like ACPI except that it doesn't
>     support vetoing from either NOTIFY or SAVE_STATE.

All that will change to a unified mecanism soon

> That all seems more problematic to me.  Seems to me that
> APM should issue SAVE_STATE (and RESTORE_STATE), and all
> three PM "initiators" should agree on vetoing.
> 
> For USB, the {SAVE,RESTORE}_STATE calls would be the most
> natural place to do the "kill pending urbs" calls that
> Alan Stern mentioned -- at least, for D3 or swsusp levels.
> Likely for D1 and D2, devices with pending I/O won't really
> be keen on from suspending.

SAVE_STATE is going away. The proper semantics are
SUSPEND and POWERDOWN (+/-). The later is optional and really only
needed by drivers who need to do their last powerdown step with
interrupts disabled.

The SUSPEND state is responsible for blocking further IOs, and
snapshoting the state. It can enter the HW suspend mode too, depending
on the "state" argument you pass.

The actual policy of what shall be done to "block" IOs is
function-specific. Typically, a network driver will just stop the Rx
side and drop any Tx packet (it just need to call netif_stop_queue()
actually, but it can drop pending packets in the Tx ring). A block
driver (IDE, SCSI) must complete any pending request and keep new
incoming ones held in the BIO queue. This has been implemented properly
for IDE already, SCSI still need work.
 
> 
> Now, for the record I tried to suspend a test1 APM-works
> system, with UHCI, and it wouldn't resume -- unclear why,
> or if test2 will do the same.
> 
> A different APM-works system, with OHCI and test2, did
> suspend/resume OK; but something went wrong with OHCI
> even before any driver model PM calls happened, if the
> OHCI driver was active (doing DMA):  the HCD got an
> "Unrecoverable Error" IRQ, which generally means that
> some kind DMA fault appeared.
> 
> All of which is a roundabout way of adding to what I
> said:  the PM infrastructure USB will need to rely on
> seems like it needs polishing yet!  :)

The USB "device" drivers shall just rely on the Device Model
infrastructure to have their suspend/resume callbacks be called at the
appropriate time. If they take care of finishing pending IOs (or
cancelling, as you like, depending on the driver function) and not
issuing new URBs until they are resumed, there shall be no problem, the
HCD should be idle when it's own suspend callback is reached. Which is
why I beleive it's safe/better to actually cancel pending URBs and
reject any new one in the HCD driver when it's in suspend state.

Ben.

Re: [linux-usb-devel] Re: OHCI problems with suspend/resume

[David Brownell]

Benjamin Herrenschmidt wrote:
> 
> You should look at Patrick Mochel's stuff that shall be getting in
> the official tree this month hopefully. It does that (among others)

OK, thanks for the update.  It was puzzling to see this stuff
in such a state ... I've got a few patches for USB PM that are
worth putting back in advance of that, but it seems like real
testing will need to wait until "next month".  (Tomorrow!)


> The USB "device" drivers shall just rely on the Device Model
> infrastructure to have their suspend/resume callbacks be called at the
> appropriate time. 

Yes, that's no problem.  It looks like:

   static int my_suspend(struct device *_intf, u32 state, u32 level)
   {
       struct usb_interface   *intf;
       int                    retval = 0;

       intf = to_usb_interface(_intf);
       switch (level) {
       ... cases are yet to stabilize ...
       }
       return retval;
   }

   ... similar for resume()

   static struct usb_driver my_driver {
        .owner =      THIS_MODULE,
        .name =       "MyFirstDriver",
        .probe =      my_probe,
        .disconnect = my_disconnect,
        .id_table =   my_id_table,
        .driver = {
              .suspend =    my_suspend,
              .resume =     my_resume,
         },
    };

But until the suspend()/resume() callback API stabilizes, it's
mostly useful to know that it'll be exactly that simple, and
that usbcore doesn't get in the way.


I suspect that USB should do some non-global PM stuff too.
Hub ports can be suspended when the devices connected to them
are idle for long enough ... that's not something I'd expect
system-wide PM policies to address.

- Dave

Re: [linux-usb-devel] Re: OHCI problems with suspend/resume

[Benjamin Herrenschmidt]

> I suspect that USB should do some non-global PM stuff too.
> Hub ports can be suspended when the devices connected to them
> are idle for long enough ... that's not something I'd expect
> system-wide PM policies to address.

Indeed, this is not addressed, though it may sense to have
a way in the device model to call the suspend/resume callbacks
of "childs" of a given hub only for that purpose, I don't think
this is implemented yet. Maybe talk to Patrick about it.

In general "local" power management (automatic disk spin down,
chipset idle-pm, etc...) is ... local to the driver, though you
can probably use the power state field of struct device to
store your current state if it maps to those semantics.

Ben.

Re: [linux-usb-devel] Re: OHCI problems with suspend/resume

[Pavel Machek]

Hi!

> > I suspect that USB should do some non-global PM stuff too.
> > Hub ports can be suspended when the devices connected to them
> > are idle for long enough ... that's not something I'd expect
> > system-wide PM policies to address.
> 
> Indeed, this is not addressed, though it may sense to have
> a way in the device model to call the suspend/resume callbacks
> of "childs" of a given hub only for that purpose, I don't think
> this is implemented yet. Maybe talk to Patrick about it.
> 
> In general "local" power management (automatic disk spin down,
> chipset idle-pm, etc...) is ... local to the driver, though you
> can probably use the power state field of struct device to
> store your current state if it maps to those semantics.

Is not disk spin-down policy, and thus belonging to userspace? Having
daemon poll for inactivity of hubs once every 5 minutes and sending
them to sleep should not hurt, too...
								Pavel
-- 
When do you have a heart between your knees?
[Johanka's followup: and *two* hearts?]

Re: [linux-usb-devel] Re: OHCI problems with suspend/resume

[Oliver Neukum]

 
> Is not disk spin-down policy, and thus belonging to userspace? Having
> daemon poll for inactivity of hubs once every 5 minutes and sending
> them to sleep should not hurt, too...
> 								Pavel

Taking precedents into account it is the kernel's job.
Screen blanking is done in kernel, as is afaik floppy
motor control.

	Regards
		Oliver

Re: [linux-usb-devel] Re: OHCI problems with suspend/resume

[Pavel Machek]

Hi!
> > Is not disk spin-down policy, and thus belonging to userspace? Having
> > daemon poll for inactivity of hubs once every 5 minutes and sending
> > them to sleep should not hurt, too...
> > 								Pavel
> 
> Taking precedents into account it is the kernel's job.
> Screen blanking is done in kernel, as is afaik floppy
> motor control.

Floppy motor was not designed to be "always on", and screen gets physically damaged
if you don't turn it off. OTOH harddrives were designed for always on, and if you spin up/down
too much you shorten their lives...
-- 
				Pavel
Written on sharp zaurus, because my Velo1 broke. If you have Velo you don't need...

Re: [linux-usb-devel] Re: OHCI problems with suspend/resume

[Pavel Machek]

Hi!

> > The pm_*() is how a handful of sound drivers and other random
> > stuff register themselves -- and how PCI does it.
> > 
> > I'd sure have expected PCI to only use the driver model stuff,
> > and I'll hope all those users will all be phased out by the
> > time that 2.6 gets near the end of its test cycle.
> 
> PCI is broken since it does both (and thus, if we call both rounds
> of notifiers, we end up suspending PCI twice, the second time without
> any ordering constraints). In my trees, I comment out that "legacy"
> stuff (though I also don't call the old-style pm_* stuff anymore
> neither)

Can you mail me a patch? [Where does PCI do its "second round"? From a
quick look I did not see that.]

								Pavel
-- 
When do you have a heart between your knees?
[Johanka's followup: and *two* hearts?]

Re: [linux-usb-devel] Re: OHCI problems with suspend/resume

[Benjamin Herrenschmidt]

On Fri, 2003-08-01 at 00:03, Pavel Machek wrote:

> Can you mail me a patch? [Where does PCI do its "second round"? From a
> quick look I did not see that.]

I just comment out the init code in drivers/pci/power.c

Ben.

Re: [linux-usb-devel] Re: OHCI problems with suspend/resume

[Pavel Machek]

Hi!

> > Can you mail me a patch? [Where does PCI do its "second round"? From a
> > quick look I did not see that.]
> 
> I just comment out the init code in drivers/pci/power.c

Hmm, in such case whole power.c file can be
killed. pci_register_driver() should take care of registering proper
callbacks.

Apm needs to be fixed to do save_state/resume.
								Pavel
-- 
When do you have a heart between your knees?
[Johanka's followup: and *two* hearts?]

Re: OHCI problems with suspend/resume

[Dominik Brugger]

Hello again,

> I will try S4 lateron.

After resuming from S4 uhci_hcd works fine (in opposition to S3 under exactly the same circumstances).
USB support was completely compiled into kernel and not as modules (therefore no unloading before suspend was done).

Interesting:

uhci_hcd wakes up again before power down.

Devices Resumed
Devices Resumed
uhci-hcd 0000:00:11.2: resume
uhci-hcd 0000:00:11.2: can't resume, not suspended!

(now using 2.6.0-test2-mm2)

-Dominik Brugger

Re: [linux-usb-devel] OHCI problems with suspend/resume

[David Brownell]

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

Pavel Machek wrote:
> Hi!
> 
> In 2.6.0-test1, OHCI is non-functional after first suspend/resume, and
> kills machine during secon suspend/resume cycle.

OK, I can see that in 2.6.0-test2 iff there's a device connected;
that's on OHCI hardware that doesn't retain power during suspend,
which means it uses the restart() path.  Hardware that retains
power depends on slightly different logic.


> What happens is that ohci_irq gets ohci->hcca == NULL, and kills
> machine. Why is ohci->hcca == NULL? ohci_stop was called from
> hcd_panic() and freed ohci->hcca.

Of course, the HC shouldn't have died and gone down those paths;
but the "HC died" paths need to work right too.


> I believe that we should
> 
> 1) not free ohci->hcca so that system has better chance surviving
> hcd_panic()

More like not calling stop() from hcd_panic.  Instead, all the
devices should be disconnected, and their urbs cleaned up.  That
way the controller will sit in a known and "safe" state (reset)
until the driver is shut down and gets stop()ped.  I think that
logic just "seemed to work" before, with subtle misbehaviors.

We're still working to make sure that we do all the right stuff
to shut down devices, no longer relying on USB device drivers to
shut themselves down properly in their disconnect() methods.
Many haven't, which can easily lead to oopsing on the shutdown
paths that don't get used very regularly.

Eventually I suspect that the HCD glue should grow logic to
try restarting drivers after the hardware dies/resets, but first
it's important to be sure they shut down properly.



> 2) inform user when hcd panics.

With a better diagnostic though.


Here's a patch that makes things slightly better.  It's still
not fully functional yet -- I forgot how many FIXMEs are in
those PM code paths! -- and shouldn't be merged as-is, but it
works slightly better:

  - Has a more informative diagnostic message (which HC died);

  - When HC dies, mark the whole tree as unavailable so that
    new URB submissions using that HC will just fail;

  - Then hcd_panic() just disconnects all the devices, still
    keeping the root hub around.

  - OHCI-specific (should be generic, hcd-pci.c):  don't
    try resuming a halted controller.

Where "better" means that it seems functional after the
first suspend/resume cycle, and re-enumerates the device
that's connected ... but there's still strangeness.  And
I can see how some of it would be generic.

- Dave



[-- Attachment #2: die.patch --]
[-- Type: text/plain, Size: 2244 bytes --]

--- 1.68/drivers/usb/core/hcd.c	Tue Jul 15 09:47:16 2003
+++ edited/drivers/usb/core/hcd.c	Sun Jul 27 19:42:46 2003
@@ -1487,8 +1488,26 @@
 
 static void hcd_panic (void *_hcd)
 {
-	struct usb_hcd *hcd = _hcd;
-	hcd->driver->stop (hcd);
+	struct usb_hcd		*hcd = _hcd;
+	struct usb_device	*hub = hcd->self.root_hub;
+
+	hub = usb_get_dev (hub);
+	usb_disconnect (&hub);
+
+	/* FIXME either try to restart, or arrange to clean up the 
+	 * hc-internal state, like usb_hcd_pci_remove() does
+	 */
+}
+
+void mark_gone (struct usb_device *dev)
+{
+	unsigned	i;
+
+	dev->state = USB_STATE_NOTATTACHED;
+	for (i = 0; i < dev->maxchild; i++) {
+		if (dev->children [i])
+			mark_gone (dev->children [i]);
+	}
 }
 
 /**
@@ -1501,29 +1520,12 @@
  */
 void usb_hc_died (struct usb_hcd *hcd)
 {
-	struct list_head	*devlist, *urblist;
-	struct hcd_dev		*dev;
-	struct urb		*urb;
-	unsigned long		flags;
-	
-	/* flag every pending urb as done */
-	spin_lock_irqsave (&hcd_data_lock, flags);
-	list_for_each (devlist, &hcd->dev_list) {
-		dev = list_entry (devlist, struct hcd_dev, dev_list);
-		list_for_each (urblist, &dev->urb_list) {
-			urb = list_entry (urblist, struct urb, urb_list);
-			dev_dbg (hcd->controller, "shutdown %s urb %p pipe %x, current status %d\n",
-				hcd->self.bus_name, urb, urb->pipe, urb->status);
-			if (urb->status == -EINPROGRESS)
-				urb->status = -ESHUTDOWN;
-		}
-	}
-	urb = (struct urb *) hcd->rh_timer.data;
-	if (urb)
-		urb->status = -ESHUTDOWN;
-	spin_unlock_irqrestore (&hcd_data_lock, flags);
+	dev_err (hcd->controller, "HC died; pending I/O will be aborted.\n");
 
-	/* hcd->stop() needs a task context */
+	/* prevent new submissions to devices in this tree */
+	mark_gone (hcd->self.root_hub);
+	
+	/* then usb_disconnect() them all, in a task context */
 	INIT_WORK (&hcd->work, hcd_panic, hcd);
 	(void) schedule_work (&hcd->work);
 }
--- 1.12/drivers/usb/host/ohci-pci.c	Mon Apr 14 02:51:40 2003
+++ edited/drivers/usb/host/ohci-pci.c	Sun Jul 27 18:51:21 2003
@@ -199,6 +199,11 @@
 	int			retval = 0;
 	unsigned long		flags;
 
+	if (hcd->state == USB_STATE_HALT) {
+		ohci_dbg (ohci, "USB restart of halted device\n");
+		return -EL3HLT;
+	}
+
 #ifdef CONFIG_PMAC_PBOOK
 	{
 		struct device_node *of_node;

Re: [linux-usb-devel] OHCI problems with suspend/resume

[Pavel Machek]

Hi!

> 
> Here's a patch that makes things slightly better.  It's still
> not fully functional yet -- I forgot how many FIXMEs are in
> those PM code paths! -- and shouldn't be merged as-is, but it
> works slightly better:
> 
>  - Has a more informative diagnostic message (which HC died);
> 
>  - When HC dies, mark the whole tree as unavailable so that
>    new URB submissions using that HC will just fail;
> 
>  - Then hcd_panic() just disconnects all the devices, still
>    keeping the root hub around.
> 
>  - OHCI-specific (should be generic, hcd-pci.c):  don't
>    try resuming a halted controller.
> 
> Where "better" means that it seems functional after the
> first suspend/resume cycle, and re-enumerates the device
> that's connected ... but there's still strangeness.  And
> I can see how some of it would be generic.

For me it:

reports problem after first suspend, and ohci stops working

oopses in hcd_panic during second suspend (IIRC).
							Pavel
-- 
Horseback riding is like software...
...vgf orggre jura vgf serr.

Re: [linux-usb-devel] OHCI problems with suspend/resume

[David Brownell]

Pavel Machek wrote:
> Hi!
> 
> 
>>Here's a patch that makes things slightly better. ....
>>
>>Where "better" means that it seems functional after the
>>first suspend/resume cycle, and re-enumerates the device
>>that's connected ... but there's still strangeness.  And
>>I can see how some of it would be generic.
> 
> For me it:
> 
> reports problem after first suspend, and ohci stops working

When you say "reports problem", what do you mean?

I can see problems _during_ the suspend, before any USB
suspend calls are even made.  Only if the controller has
work to do, so it's scanning the schedules (DMA) ... it's
fine until the suspend starts, then the controller gets
an "Unrecoverable Error" IRQ, which I'm used to seeing
for DMA errors.  (As if something started shutting down
hardware before the driver had a chance to clean up.)

If there's no DMA action before the suspend starts, the
current code behaved OK for me -- on an APM D3cold
suspend/resume cycle, which isn't so different from what
you'll see with swsusp.

That makes me think I'll be better off waiting for some
of those PM updates to get merged, before I spend much
more time looking at the usb parts of these issues.


> oopses in hcd_panic during second suspend (IIRC).

Likely because resuming a halted controller is nonsense,
and the code needs to reject such attempts.

Of course, if the controller weren't going bonkers before
it's told to start suspending, these particular faults
wouldn't be happening ... :)

- Dave

Re: [linux-usb-devel] OHCI problems with suspend/resume

[Pavel Machek]

Hi!

> >>Here's a patch that makes things slightly better. ....
> >>
> >>Where "better" means that it seems functional after the
> >>first suspend/resume cycle, and re-enumerates the device
> >>that's connected ... but there's still strangeness.  And
> >>I can see how some of it would be generic.
> >
> >For me it:
> >
> >reports problem after first suspend, and ohci stops working
> 
> When you say "reports problem", what do you mean?

I fixed PCI powermangment, and OHCI problems are gone (replaced by
"machine-feels-like-XT-because-keventd-spins"). I'm looking onto that.

								Pavel
-- 
When do you have a heart between your knees?
[Johanka's followup: and *two* hearts?]