Re: detach before suspend and attach after resume

2014-03-26 Thread Peter Kane
Hi Martin

I'm not sure if you hoped this would help the auto-resume on suspend with
flash drive attached problem that I mentioned in the Kernel error with
March 20th amd64 snapshot thread on @misc, but it doesn't. Suspend/resume
still works OK without a flash drive attached. Let me know if I can be of
further assistance.

Thanks, Peter


Re: detach before suspend and attach after resume

2014-03-26 Thread Martin Pieuchot
On 26/03/14(Wed) 19:45, Peter Kane wrote:
 Hi Martin
 
 I'm not sure if you hoped this would help the auto-resume on suspend with
 flash drive attached problem that I mentioned in the Kernel error with
 March 20th amd64 snapshot thread on @misc, but it doesn't. Suspend/resume
 still works OK without a flash drive attached. Let me know if I can be of
 further assistance.

Peter, thanks for testing my diff!  Next time please include your dmesg,
otherwise I've no idea on which hardware you've tested it ;)

Concerning your auto-resume problem, the best way to get my attention
on it, is by sending a report to bugs@ as described on the site [0].  In
your case, please include the complete dmesg after the auto-resume,
and if you can find a date (approximately) when this regression started
(by recompiling kernels), it would be very useful.

This thread is about testing a development diff, let's not use it for
debugging another problem :)  Thanks!

[0] http://www.openbsd.org/report.html



detach before suspend and attach after resume

2014-03-25 Thread Martin Pieuchot
This diff should help to avoid some races occurring when resuming
a machine with USB devices, please test!

Since the Host Controllers are halted during suspend and reset on
wakeup, all the connected USB devices are disconnected during a
suspend/resume cycle.

When a device is disconnected its pipes are aborted and closed,
right now this is done *after* resuming.  But since the HC have
already been reset and some timeout might also fire, that generates
a lot of problems.  So the diff below detaches the root hub of every
controller before suspending, so that no device stay logically
attached when suspended. 

Make sure you have my last commit to all *hci.c before trying this,
and report any regression/success story.

Index: usb.c
===
RCS file: /home/ncvs/src/sys/dev/usb/usb.c,v
retrieving revision 1.94
diff -u -p -r1.94 usb.c
--- usb.c   8 Mar 2014 11:49:19 -   1.94
+++ usb.c   25 Mar 2014 17:32:29 -
@@ -92,6 +92,7 @@ struct usb_softc {
struct devicesc_dev;/* base device */
struct usbd_bus  *sc_bus;   /* USB controller */
struct usbd_port sc_port;   /* dummy port for root hub */
+   int  sc_speed;
 
struct usb_task  sc_explore_task;
 
@@ -125,6 +126,9 @@ void usb_attach(struct device *, struc
 int usb_detach(struct device *, int); 
 int usb_activate(struct device *, int); 
 
+int usb_attach_roothub(struct usb_softc *);
+voidusb_detach_roothub(struct usb_softc *);
+
 struct cfdriver usb_cd = { 
NULL, usb, DV_DULL 
 }; 
@@ -147,10 +151,7 @@ void
 usb_attach(struct device *parent, struct device *self, void *aux)
 {
struct usb_softc *sc = (struct usb_softc *)self;
-   struct usbd_device *dev;
-   usbd_status err;
int usbrev;
-   int speed;
 
if (usb_nbuses == 0) {
rw_init(usbpalock, usbpalock);
@@ -171,13 +172,13 @@ usb_attach(struct device *parent, struct
switch (usbrev) {
case USBREV_1_0:
case USBREV_1_1:
-   speed = USB_SPEED_FULL;
+   sc-sc_speed = USB_SPEED_FULL;
break;
case USBREV_2_0:
-   speed = USB_SPEED_HIGH;
+   sc-sc_speed = USB_SPEED_HIGH;
break;
case USBREV_3_0:
-   speed = USB_SPEED_SUPER;
+   sc-sc_speed = USB_SPEED_SUPER;
break;
default:
printf(, not supported\n);
@@ -206,17 +207,10 @@ usb_attach(struct device *parent, struct
return;
}
 
-   err = usbd_new_device(sc-sc_dev, sc-sc_bus, 0, speed, 0,
- sc-sc_port);
-   if (!err) {
-   dev = sc-sc_port.device;
-   if (dev-hub == NULL) {
-   sc-sc_bus-dying = 1;
-   printf(%s: root device is not a hub\n,
-  sc-sc_dev.dv_xname);
-   return;
-   }
-   sc-sc_bus-root_hub = dev;
+
+
+   if (!usb_attach_roothub(sc)) {
+   struct usbd_device *dev = sc-sc_bus-root_hub;
 #if 1
/*
 * Turning this code off will delay attachment of USB devices
@@ -226,11 +220,8 @@ usb_attach(struct device *parent, struct
if (cold  (sc-sc_dev.dv_cfdata-cf_flags  1))
dev-hub-explore(sc-sc_bus-root_hub);
 #endif
-   } else {
-   printf(%s: root hub problem, error=%d\n,
-  sc-sc_dev.dv_xname, err);
-   sc-sc_bus-dying = 1;
}
+
if (cold)
sc-sc_bus-use_polling--;
 
@@ -243,6 +234,41 @@ usb_attach(struct device *parent, struct
}
 }
 
+int
+usb_attach_roothub(struct usb_softc *sc)
+{
+   struct usbd_device *dev;
+
+   if (usbd_new_device(sc-sc_dev, sc-sc_bus, 0, sc-sc_speed, 0,
+   sc-sc_port)) {
+   printf(%s: root hub problem\n, sc-sc_dev.dv_xname);
+   sc-sc_bus-dying = 1;
+   return (1);
+   }
+
+   dev = sc-sc_port.device;
+   if (dev-hub == NULL) {
+   printf(%s: root device is not a hub\n, sc-sc_dev.dv_xname);
+   sc-sc_bus-dying = 1;
+   return (1);
+   }
+   sc-sc_bus-root_hub = dev;
+
+   return (0);
+}
+
+void
+usb_detach_roothub(struct usb_softc *sc)
+{
+   /* Make all devices disconnect. */
+   if (sc-sc_port.device != NULL)
+   usb_disconnect_port(sc-sc_port, (struct device *)sc);
+
+   usb_rem_wait_task(sc-sc_bus-root_hub, sc-sc_explore_task);
+
+   sc-sc_bus-root_hub = NULL;
+}
+
 void
 usb_create_task_threads(void *arg)
 {
@@ -866,23 +892,22 @@ int
 usb_activate(struct device *self, int act)
 {
struct usb_softc *sc = (struct usb_softc *)self;
-   struct usbd_device *dev = sc-sc_port.device;
-   int i, rv = 0, 

Re: detach before suspend and attach after resume

2014-03-25 Thread Fred

On 03/25/14 20:42, Martin Pieuchot wrote:

This diff should help to avoid some races occurring when resuming
a machine with USB devices, please test!

Since the Host Controllers are halted during suspend and reset on
wakeup, all the connected USB devices are disconnected during a
suspend/resume cycle.

When a device is disconnected its pipes are aborted and closed,
right now this is done *after* resuming.  But since the HC have
already been reset and some timeout might also fire, that generates
a lot of problems.  So the diff below detaches the root hub of every
controller before suspending, so that no device stay logically
attached when suspended.

Make sure you have my last commit to all *hci.c before trying this,
and report any regression/success story.



Hi Martin,

This caused a regression on my amd64 laptop - dmesg with patch is below 
in [1] and normal dmesg is also below at [2] - the following is 
transcribed from pictures of the crash:


The fault was:
uvm_fault(0xfff81dc6f00, 0x0, 0, 1) - e
kernel: page fault trap, code=0
Stopped at  usbd_do_request_flags+0x33: movq 0(%rdi),%rax

ddb{0} trace
usbd_do_request_flags() at usb_do_request_flags+0x33
usbd_reset_port() at usbd_reset_port+0x49
usbd_new_device() at usbd_new_device+0x1b0
usb_attach_roothub() at usb_attach_roothub+0x21
usb_activate() at usb_activate+0x56
config_suspend() at config_suspend+0x37
config_activate_children() at config_activate_children+0x4d
config_suspend() at config_suspend+0x37
config_activate_children() at config_activate_children+0x4d
config_suspend() at config_suspend+0x37
config_activate_children() at config_activate_children+0x4d
config_suspend() at config_suspend+0x5c
acpi_sleep_state() at acpi_sleep_state+0xde
acpi_sleep_task() at acpi_sleept_task+0xf
acpi_dotask() at acpi_dotask+0x55
acpi_thread() at acpi_thread+0x11d
end trace frame: 0x0, count: -16

I will try and find away of capturing the panic as it will take me a 
month of sundays to transcribe all the output from ps.


Cheers

Fred

[1]
OpenBSD 5.5-current (usbGEN.MP) #0: Tue Mar 25 21:17:49 GMT 2014
f...@port.crowsons.com:/usr/src/sys/arch/amd64/compile/usbGEN.MP
real mem = 8447131648 (8055MB)
avail mem = 8213540864 (7833MB)
mainbus0 at root
bios0 at mainbus0: SMBIOS rev. 2.5 @ 0xaafd (43 entries)
bios0: vendor TOSHIBA version Version 3.60 date 01/24/2012
bios0: TOSHIBA TOSHIBA
acpi0 at bios0: rev 0
acpi0: sleep states S0 S3 S4 S5
acpi0: tables DSDT FACP HPET APIC MCFG ASF! TCPA BOOT SLIC SSDT SSDT 
SSDT SSDT
acpi0: wakeup devices LANC(S4) HDEF(S3) RP02(S4) PXSX(S4) RP04(S4) 
PXSX(S4) USBB(S4) USBC(S4) EHC1(S4) EHC2(S4) PWRB(S4) LID_(S4)

acpitimer0 at acpi0: 3579545 Hz, 24 bits
acpihpet0 at acpi0: 14318179 Hz
acpimadt0 at acpi0 addr 0xfee0: PC-AT compat
cpu0 at mainbus0: apid 0 (boot processor)
cpu0: Intel(R) Core(TM) i5-2520M CPU @ 2.50GHz, 2492.21 MHz
cpu0: 
FPU,VME,DE,PSE,TSC,MSR,PAE,MCE,CX8,APIC,SEP,MTRR,PGE,MCA,CMOV,PAT,PSE36,CFLUSH,DS,ACPI,MMX,FXSR,SSE,SSE2,SS,HTT,TM,PBE,SSE3,PCLMUL,DTES64,MWAIT,DS-CPL,VMX,SMX,EST,TM2,SSSE3,CX16,xTPR,PDCM,PCID,SSE4.1,SSE4.2,x2APIC,POPCNT,DEADLINE,AES,XSAVE,AVX,NXE,LONG,LAHF,PERF,ITSC

cpu0: 256KB 64b/line 8-way L2 cache
cpu0: smt 0, core 0, package 0
mtrr: Pentium Pro MTRR support, 10 var ranges, 88 fixed ranges
cpu0: apic clock running at 99MHz
cpu0: mwait min=64, max=64, C-substates=0.2.1.1.2, IBE
cpu1 at mainbus0: apid 1 (application processor)
cpu1: Intel(R) Core(TM) i5-2520M CPU @ 2.50GHz, 2491.91 MHz
cpu1: 
FPU,VME,DE,PSE,TSC,MSR,PAE,MCE,CX8,APIC,SEP,MTRR,PGE,MCA,CMOV,PAT,PSE36,CFLUSH,DS,ACPI,MMX,FXSR,SSE,SSE2,SS,HTT,TM,PBE,SSE3,PCLMUL,DTES64,MWAIT,DS-CPL,VMX,SMX,EST,TM2,SSSE3,CX16,xTPR,PDCM,PCID,SSE4.1,SSE4.2,x2APIC,POPCNT,DEADLINE,AES,XSAVE,AVX,NXE,LONG,LAHF,PERF,ITSC

cpu1: 256KB 64b/line 8-way L2 cache
cpu1: smt 1, core 0, package 0
cpu2 at mainbus0: apid 2 (application processor)
cpu2: Intel(R) Core(TM) i5-2520M CPU @ 2.50GHz, 2491.91 MHz
cpu2: 
FPU,VME,DE,PSE,TSC,MSR,PAE,MCE,CX8,APIC,SEP,MTRR,PGE,MCA,CMOV,PAT,PSE36,CFLUSH,DS,ACPI,MMX,FXSR,SSE,SSE2,SS,HTT,TM,PBE,SSE3,PCLMUL,DTES64,MWAIT,DS-CPL,VMX,SMX,EST,TM2,SSSE3,CX16,xTPR,PDCM,PCID,SSE4.1,SSE4.2,x2APIC,POPCNT,DEADLINE,AES,XSAVE,AVX,NXE,LONG,LAHF,PERF,ITSC

cpu2: 256KB 64b/line 8-way L2 cache
cpu2: smt 0, core 1, package 0
cpu3 at mainbus0: apid 3 (application processor)
cpu3: Intel(R) Core(TM) i5-2520M CPU @ 2.50GHz, 2491.91 MHz
cpu3: 
FPU,VME,DE,PSE,TSC,MSR,PAE,MCE,CX8,APIC,SEP,MTRR,PGE,MCA,CMOV,PAT,PSE36,CFLUSH,DS,ACPI,MMX,FXSR,SSE,SSE2,SS,HTT,TM,PBE,SSE3,PCLMUL,DTES64,MWAIT,DS-CPL,VMX,SMX,EST,TM2,SSSE3,CX16,xTPR,PDCM,PCID,SSE4.1,SSE4.2,x2APIC,POPCNT,DEADLINE,AES,XSAVE,AVX,NXE,LONG,LAHF,PERF,ITSC

cpu3: 256KB 64b/line 8-way L2 cache
cpu3: smt 1, core 1, package 0
ioapic0 at mainbus0: apid 2 pa 0xfec0, version 20, 24 pins
acpimcfg0 at acpi0 addr 0xf800, bus 0-63
acpiprt0 at acpi0: bus 0 (PCI0)
acpiprt1 at acpi0: bus -1 (PEGP)
acpiprt2 at acpi0: bus 1 (RP01)
acpiprt3 at acpi0: bus 2 (RP02)
acpiprt4 at acpi0: bus 4 (RP03)

Re: detach before suspend and attach after resume

2014-03-25 Thread Fred

On 03/25/14 22:56, Fred wrote:

On 03/25/14 20:42, Martin Pieuchot wrote:

This diff should help to avoid some races occurring when resuming
a machine with USB devices, please test!

Since the Host Controllers are halted during suspend and reset on
wakeup, all the connected USB devices are disconnected during a
suspend/resume cycle.

When a device is disconnected its pipes are aborted and closed,
right now this is done *after* resuming.  But since the HC have
already been reset and some timeout might also fire, that generates
a lot of problems.  So the diff below detaches the root hub of every
controller before suspending, so that no device stay logically
attached when suspended.

Make sure you have my last commit to all *hci.c before trying this,
and report any regression/success story.


/snipped

I've just noticed that my ehci.c is at v 1.145 and not 1.146 - I'll 
bring my source up to date and try again.


Sorry for the noise.

Fred




Re: detach before suspend and attach after resume

2014-03-25 Thread Fred

On 03/25/14 23:07, Fred wrote:

On 03/25/14 22:56, Fred wrote:

On 03/25/14 20:42, Martin Pieuchot wrote:

This diff should help to avoid some races occurring when resuming
a machine with USB devices, please test!

Since the Host Controllers are halted during suspend and reset on
wakeup, all the connected USB devices are disconnected during a
suspend/resume cycle.

When a device is disconnected its pipes are aborted and closed,
right now this is done *after* resuming.  But since the HC have
already been reset and some timeout might also fire, that generates
a lot of problems.  So the diff below detaches the root hub of every
controller before suspending, so that no device stay logically
attached when suspended.

Make sure you have my last commit to all *hci.c before trying this,
and report any regression/success story.


/snipped

I've just noticed that my ehci.c is at v 1.145 and not 1.146 - I'll
bring my source up to date and try again.

Sorry for the noise.

Fred



No regressions - once my source tree was up to date.

Cheers

Fred

--
dmesg:
OpenBSD 5.5-current (usbGEN.MP) #0: Tue Mar 25 21:17:49 GMT 2014
f...@port.crowsons.com:/usr/src/sys/arch/amd64/compile/usbGEN.MP
real mem = 8447131648 (8055MB)
avail mem = 8213540864 (7833MB)
mainbus0 at root
bios0 at mainbus0: SMBIOS rev. 2.5 @ 0xaafd (43 entries)
bios0: vendor TOSHIBA version Version 3.60 date 01/24/2012
bios0: TOSHIBA TOSHIBA
acpi0 at bios0: rev 0
acpi0: sleep states S0 S3 S4 S5
acpi0: tables DSDT FACP HPET APIC MCFG ASF! TCPA BOOT SLIC SSDT SSDT 
SSDT SSDT
acpi0: wakeup devices LANC(S4) HDEF(S3) RP02(S4) PXSX(S4) RP04(S4) 
PXSX(S4) USBB(S4) USBC(S4) EHC1(S4) EHC2(S4) PWRB(S4) LID_(S4)

acpitimer0 at acpi0: 3579545 Hz, 24 bits
acpihpet0 at acpi0: 14318179 Hz
acpimadt0 at acpi0 addr 0xfee0: PC-AT compat
cpu0 at mainbus0: apid 0 (boot processor)
cpu0: Intel(R) Core(TM) i5-2520M CPU @ 2.50GHz, 2492.21 MHz
cpu0: 
FPU,VME,DE,PSE,TSC,MSR,PAE,MCE,CX8,APIC,SEP,MTRR,PGE,MCA,CMOV,PAT,PSE36,CFLUSH,DS,ACPI,MMX,FXSR,SSE,SSE2,SS,HTT,TM,PBE,SSE3,PCLMUL,DTES64,MWAIT,DS-CPL,VMX,SMX,EST,TM2,SSSE3,CX16,xTPR,PDCM,PCID,SSE4.1,SSE4.2,x2APIC,POPCNT,DEADLINE,AES,XSAVE,AVX,NXE,LONG,LAHF,PERF,ITSC

cpu0: 256KB 64b/line 8-way L2 cache
cpu0: smt 0, core 0, package 0
mtrr: Pentium Pro MTRR support, 10 var ranges, 88 fixed ranges
cpu0: apic clock running at 99MHz
cpu0: mwait min=64, max=64, C-substates=0.2.1.1.2, IBE
cpu1 at mainbus0: apid 1 (application processor)
cpu1: Intel(R) Core(TM) i5-2520M CPU @ 2.50GHz, 2491.91 MHz
cpu1: 
FPU,VME,DE,PSE,TSC,MSR,PAE,MCE,CX8,APIC,SEP,MTRR,PGE,MCA,CMOV,PAT,PSE36,CFLUSH,DS,ACPI,MMX,FXSR,SSE,SSE2,SS,HTT,TM,PBE,SSE3,PCLMUL,DTES64,MWAIT,DS-CPL,VMX,SMX,EST,TM2,SSSE3,CX16,xTPR,PDCM,PCID,SSE4.1,SSE4.2,x2APIC,POPCNT,DEADLINE,AES,XSAVE,AVX,NXE,LONG,LAHF,PERF,ITSC

cpu1: 256KB 64b/line 8-way L2 cache
cpu1: smt 1, core 0, package 0
cpu2 at mainbus0: apid 2 (application processor)
cpu2: Intel(R) Core(TM) i5-2520M CPU @ 2.50GHz, 2491.91 MHz
cpu2: 
FPU,VME,DE,PSE,TSC,MSR,PAE,MCE,CX8,APIC,SEP,MTRR,PGE,MCA,CMOV,PAT,PSE36,CFLUSH,DS,ACPI,MMX,FXSR,SSE,SSE2,SS,HTT,TM,PBE,SSE3,PCLMUL,DTES64,MWAIT,DS-CPL,VMX,SMX,EST,TM2,SSSE3,CX16,xTPR,PDCM,PCID,SSE4.1,SSE4.2,x2APIC,POPCNT,DEADLINE,AES,XSAVE,AVX,NXE,LONG,LAHF,PERF,ITSC

cpu2: 256KB 64b/line 8-way L2 cache
cpu2: smt 0, core 1, package 0
cpu3 at mainbus0: apid 3 (application processor)
cpu3: Intel(R) Core(TM) i5-2520M CPU @ 2.50GHz, 2491.91 MHz
cpu3: 
FPU,VME,DE,PSE,TSC,MSR,PAE,MCE,CX8,APIC,SEP,MTRR,PGE,MCA,CMOV,PAT,PSE36,CFLUSH,DS,ACPI,MMX,FXSR,SSE,SSE2,SS,HTT,TM,PBE,SSE3,PCLMUL,DTES64,MWAIT,DS-CPL,VMX,SMX,EST,TM2,SSSE3,CX16,xTPR,PDCM,PCID,SSE4.1,SSE4.2,x2APIC,POPCNT,DEADLINE,AES,XSAVE,AVX,NXE,LONG,LAHF,PERF,ITSC

cpu3: 256KB 64b/line 8-way L2 cache
cpu3: smt 1, core 1, package 0
ioapic0 at mainbus0: apid 2 pa 0xfec0, version 20, 24 pins
acpimcfg0 at acpi0 addr 0xf800, bus 0-63
acpiprt0 at acpi0: bus 0 (PCI0)
acpiprt1 at acpi0: bus -1 (PEGP)
acpiprt2 at acpi0: bus 1 (RP01)
acpiprt3 at acpi0: bus 2 (RP02)
acpiprt4 at acpi0: bus 4 (RP03)
acpiprt5 at acpi0: bus -1 (RP04)
acpiprt6 at acpi0: bus 5 (RP05)
acpiprt7 at acpi0: bus 6 (RP06)
acpiprt8 at acpi0: bus 7 (RP07)
acpiprt9 at acpi0: bus -1 (RP08)
acpiprt10 at acpi0: bus -1 (PCIB)
acpicpu0 at acpi0: C1, PSS
acpicpu1 at acpi0: C1, PSS
acpicpu2 at acpi0: C1, PSS
acpicpu3 at acpi0: C1, PSS
acpipwrres0 at acpi0: PDOC, resource for DOCK
acpitz0 at acpi0: critical temperature is 102 degC
acpitoshiba0 at acpi0
acpiac0 at acpi0: AC unit online
acpibtn0 at acpi0: PWRB
acpibtn1 at acpi0: LID_
acpibat0 at acpi0: BAT1 model G71C000E4410 serial 001888 type 
Li-ION   oem 0

acpidock0 at acpi0: DOCK not docked (0)
cpu0: Enhanced SpeedStep 2492 MHz: speeds: 2501, 2500, 2000, 1800, 1600, 
1400, 1200, 1000, 800 MHz

pci0 at mainbus0 bus 0
pchb0 at pci0 dev 0 function 0 Intel Core 2G Host rev 0x09
vga1 at pci0 dev 2 function 0 Intel HD Graphics 3000 rev 0x09
intagp0 at vga1
agp0 at intagp0: aperture at 0xb000, size 0x1000
inteldrm0 at vga1
drm0