date:20130222

On Thu, Feb 21, 2013 at 10:40:10AM -0800, Greg KH wrote:

  USB patches for 3.9-rc1
  
  Here's the big USB merge for 3.9-rc1
  
  Nothing major, lots of gadget fixes, and of course, xhci stuff.

I get no USB devices recognised when I insert them any more, which
I think is pretty major.  I suspect it has something to do with this ?

  Lan Tianyu (12):
usb: add runtime pm support for usb port device
usb: add usb port auto power off mechanism

It looks like every port on my laptop is powered down, as I can't
even charge devices with it.

I tried running powertop, which noted autosuspend for the usb controllers
was 'good'. I flipped them to 'bad', but it made no difference.

What can I do to debug this ?

$ lspci | grep USB
00:14.0 USB controller: Intel Corporation 7 Series/C210 Series Chipset Family 
USB xHCI Host Controller (rev 04)
00:1a.0 USB controller: Intel Corporation 7 Series/C210 Series Chipset Family 
USB Enhanced Host Controller #2 (rev 04)
00:1d.0 USB controller: Intel Corporation 7 Series/C210 Series Chipset Family 
USB Enhanced Host Controller #1 (rev 04)

$ lsusb
Bus 001 Device 002: ID 8087:0024 Intel Corp. Integrated Rate Matching Hub
Bus 002 Device 002: ID 8087:0024 Intel Corp. Integrated Rate Matching Hub
Bus 001 Device 001: ID 1d6b:0002 Linux Foundation 2.0 root hub
Bus 002 Device 001: ID 1d6b:0002 Linux Foundation 2.0 root hub
Bus 003 Device 001: ID 1d6b:0002 Linux Foundation 2.0 root hub
Bus 004 Device 001: ID 1d6b:0003 Linux Foundation 3.0 root hub
Bus 001 Device 003: ID 0a5c:21e6 Broadcom Corp. BCM20702 Bluetooth 4.0 
[ThinkPad]
Bus 001 Device 004: ID 5986:02d5 Acer, Inc 

(inserted USB devices don't show up in this list)

$ dmesg | grep -i usb
[0.996096] ACPI: bus type usb registered
[0.996428] usbcore: registered new interface driver usbfs
[0.996638] usbcore: registered new interface driver hub
[0.996875] usbcore: registered new device driver usb
[1.874896] ehci_hcd: USB 2.0 'Enhanced' Host Controller (EHCI) Driver
[1.876103] ehci-pci :00:1a.0: new USB bus registered, assigned bus 
number 1
[1.886220] ehci-pci :00:1a.0: USB 2.0 started, EHCI 1.00
[1.886781] usb usb1: New USB device found, idVendor=1d6b, idProduct=0002
[1.886878] usb usb1: New USB device strings: Mfr=3, Product=2, 
SerialNumber=1
[1.887017] usb usb1: Product: EHCI Host Controller
[1.887111] usb usb1: Manufacturer: Linux 3.8.0+ ehci_hcd
[1.887220] usb usb1: SerialNumber: :00:1a.0
[1.888942] hub 1-0:1.0: USB hub found
[1.892347] ehci-pci :00:1d.0: new USB bus registered, assigned bus 
number 2
[1.902216] ehci-pci :00:1d.0: USB 2.0 started, EHCI 1.00
[1.902674] usb usb2: New USB device found, idVendor=1d6b, idProduct=0002
[1.902770] usb usb2: New USB device strings: Mfr=3, Product=2, 
SerialNumber=1
[1.902910] usb usb2: Product: EHCI Host Controller
[1.903004] usb usb2: Manufacturer: Linux 3.8.0+ ehci_hcd
[1.903098] usb usb2: SerialNumber: :00:1d.0
[1.90] hub 2-0:1.0: USB hub found
[1.906795] ohci_hcd: USB 1.1 'Open' Host Controller (OHCI) Driver
[1.906970] uhci_hcd: USB Universal Host Controller Interface driver
[1.908243] xhci_hcd :00:14.0: new USB bus registered, assigned bus 
number 3
[1.909823] usb usb3: New USB device found, idVendor=1d6b, idProduct=0002
[1.909920] usb usb3: New USB device strings: Mfr=3, Product=2, 
SerialNumber=1
[1.910060] usb usb3: Product: xHCI Host Controller
[1.910153] usb usb3: Manufacturer: Linux 3.8.0+ xhci_hcd
[1.910260] usb usb3: SerialNumber: :00:14.0
[1.911644] hub 3-0:1.0: USB hub found
[1.930176] xhci_hcd :00:14.0: new USB bus registered, assigned bus 
number 4
[1.930802] usb usb4: New USB device found, idVendor=1d6b, idProduct=0003
[1.930899] usb usb4: New USB device strings: Mfr=3, Product=2, 
SerialNumber=1
[1.931039] usb usb4: Product: xHCI Host Controller
[1.931131] usb usb4: Manufacturer: Linux 3.8.0+ xhci_hcd
[1.931239] usb usb4: SerialNumber: :00:14.0
[1.932568] hub 4-0:1.0: USB hub found
[1.950215] usbcore: registered new interface driver usbserial
[1.950388] usbcore: registered new interface driver usbserial_generic
[1.950612] usbserial: USB Serial support registered for generic
[1.950781] usbcore: registered new interface driver usb_debug
[1.950959] usbserial: USB Serial support registered for debug
[1.972227] usbcore: registered new interface driver usbhid
[1.972322] usbhid: USB HID core driver
[2.193584] usb 1-1: new high-speed USB device number 2 using ehci-pci
[2.309836] usb 1-1: New USB device found, idVendor=8087, idProduct=0024
[2.309965] usb 1-1: New USB device strings: Mfr=0, Product=0, SerialNumber=0
[2.313198] hub 1-1:1.0: USB hub found
[2.436596] usb 2-1: new high-speed USB device number 2 using ehci-pci
[2.551777] usb 2-1: New USB device found, idVendor=8087, idProduct=0024
[2.551906] usb 2-1: New

USB: serial: port lifetimes (was: Re: USB Ooops PL2303 when unplug while use (linux v3.7.3))

2013-02-22 Thread Johan Hovold

On Thu, Feb 14, 2013 at 06:43:36PM +0100, Johan Hovold wrote:
 On Wed, Feb 13, 2013 at 01:40:08PM -0500, Peter Hurley wrote:

[...]

  Looks to me like a bug the usb serial mini-port interface design.
  A usb bus disconnect has the pl2303 (and every other) mini-port freeing
  the private data (before unregistering with tty anyway -- not that
  putting that first would fix the problem).

[...]

  The tty layer (and its port implementation) can't protect the pl2303
  mini-port from this behavior/interface design.
  
  It's the glue layer's responsibility to correctly manage the differing
  lifetimes of its bus devices with tty devices (and tty_ports, if used).
  
  Meaning, that if the physical device disconnects from the bus, the ports
  must simply become in-operative; they can't disappear.

[...]

  BTW, just fixing this one part won't work because the usb serial driver
  is also abruptly deleting the usb_serial_port device as well:
  
  static void usb_serial_disconnect(struct usb_interface *interface)
  {
  [...]
  
  for (i = 0; i  serial-num_ports; ++i) {
  port = serial-port[i];
  if (port) {
  struct tty_struct *tty = tty_port_tty_get(port-port);
  if (tty) {
  tty_vhangup(tty);
  tty_kref_put(tty);
  }
  kill_traffic(port);
  cancel_work_sync(port-work);
  if (device_is_registered(port-dev))
     device_del(port-dev);
  }
  }
  
  [...]
  }
  
  Ummm, wasn't that where the port private data pointer was?
 
 Indeed.

We keep the usb-serial-port structure around until the last tty
reference is dropped, but the port private data is freed as part of
unregistering from the bus in disconnect. [ The subdrivers aren't
supposed to access the ports after port_remove is called as part of
unregistration. ]

This wasn't always the case, but unregistering in serial_cleanup (when
the last tty reference is dropped) had other issues. In particular, we
would end up unregistering the parent device before its child (the
interface and usb device are unregistered when disconnect returns)
resulting in broken uevent devpaths:

KERNEL[794.849051] remove   
/devices/pci:00/:00:1a.7/usb1/1-4/1-4.1/1-4.1:1.0 (usb)
KERNEL[794.851649] remove   /devices/pci:00/:00:1a.7/usb1/1-4/1-4.1 
(usb)
KERNEL[795.115623] remove   /1-4.1:1.0/ttyUSB0/tty/ttyUSB0 (tty)
KERNEL[795.117429] remove   /1-4.1:1.0/ttyUSB0 (usb-serial)

This was reported here

https://bugs.freedesktop.org/show_bug.cgi?id=20703

and fixed by Alan Stern in 2d93148ab6 (USB: serial: fix lifetime and
locking problems) by moving unregistration to disconnect.

So we end up with an unregistered device still possibly referenced by
tty instead, and I suspect we can't do much else than deal with any
post-disconnect callbacks. [ These should be few, especially with my
latest TTY-patches applied. ]

Alan, do you see any way around this? It's not possible (or desirable)
to pin the parent device (interface) until the last reference is
dropped, is it?

Other drivers, such as cdc-acm, do unregister when the last reference is
dropped, but could potentially also generate broken uevents:

KERNEL[3042.388951] remove   /devices/pci:00/:00:1d.7/usb2/2-1/2-1:3.1 
(usb)
KERNEL[3042.390242] remove   /2-1:3.1/tty/ttyACM0 (tty)

[ To trigger this I had to move the tty_kref_put to the end of
  disconnect(). As the tty ref is dropped in a workqueue, the disconnect
  then returns before cleanup is called, but any outstanding reference
  would have the same effect. ]

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

[PATCH 07/10] staging: usbip: userspace: libsrc: added missing space

This patch fixes the following checkpatch warning:
-WARNING: missing space after enum definition

Signed-off-by: Kurt Kanzenbach ly80t...@cip.cs.fau.de
---
 drivers/staging/usbip/userspace/libsrc/usbip_common.h |2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/staging/usbip/userspace/libsrc/usbip_common.h 
b/drivers/staging/usbip/userspace/libsrc/usbip_common.h
index 0b2f370..938ad1c 100644
--- a/drivers/staging/usbip/userspace/libsrc/usbip_common.h
+++ b/drivers/staging/usbip/userspace/libsrc/usbip_common.h
@@ -84,7 +84,7 @@ enum usb_device_speed {
 };
 
 /* FIXME: how to sync with drivers/usbip_common.h ? */
-enum usbip_device_status{
+enum usbip_device_status {
/* sdev is available. */
SDEV_ST_AVAILABLE = 0x01,
/* sdev is now used. */
-- 
1.7.10.4

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

[PATCH 08/10] staging: usbip: remove unnecessary braces

From: Stefan Reif ke42c...@cip.cs.fau.de

This patch fixes the following checkpatch warning:
-WARNING: braces {} are not necessary for any arm of this statement

Signed-off-by: Stefan Reif ke42c...@cip.cs.fau.de
Signed-off-by: Kurt Kanzenbach ly80t...@cip.cs.fau.de
---
 drivers/staging/usbip/vhci_hcd.c |5 ++---
 1 file changed, 2 insertions(+), 3 deletions(-)

diff --git a/drivers/staging/usbip/vhci_hcd.c b/drivers/staging/usbip/vhci_hcd.c
index f1ca084..d7974cb 100644
--- a/drivers/staging/usbip/vhci_hcd.c
+++ b/drivers/staging/usbip/vhci_hcd.c
@@ -956,11 +956,10 @@ static int vhci_bus_resume(struct usb_hcd *hcd)
dev_dbg(hcd-self.root_hub-dev, %s\n, __func__);
 
spin_lock(vhci-lock);
-   if (!HCD_HW_ACCESSIBLE(hcd)) {
+   if (!HCD_HW_ACCESSIBLE(hcd))
rc = -ESHUTDOWN;
-   } else {
+   else
hcd-state = HC_STATE_RUNNING;
-   }
spin_unlock(vhci-lock);
 
return rc;
-- 
1.7.10.4

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

[PATCH 06/10] staging: usbip: userspace: libsrc: removed assignments in if conditions

This patch fixes the following checkpatch error:
-ERROR: do not use assignment in if condition

Signed-off-by: Kurt Kanzenbach ly80t...@cip.cs.fau.de
---
 drivers/staging/usbip/userspace/libsrc/names.c |   11 +++
 1 file changed, 7 insertions(+), 4 deletions(-)

diff --git a/drivers/staging/usbip/userspace/libsrc/names.c 
b/drivers/staging/usbip/userspace/libsrc/names.c
index a66f539..3b151df 100644
--- a/drivers/staging/usbip/userspace/libsrc/names.c
+++ b/drivers/staging/usbip/userspace/libsrc/names.c
@@ -491,9 +491,11 @@ static void parse(FILE *f)
while (fgets(buf, sizeof(buf), f)) {
linectr++;
/* remove line ends */
-   if ((cp = strchr(buf, 13)))
+   cp = strchr(buf, 13);
+   if (cp)
*cp = 0;
-   if ((cp = strchr(buf, 10)))
+   cp = strchr(buf, 10);
+   if (cp)
*cp = 0;
if (buf[0] == '#' || !buf[0])
continue;
@@ -857,9 +859,10 @@ int names_init(char *n)
 {
FILE *f;
 
-   if (!(f = fopen(n, r))) {
+   f = fopen(n, r);
+   if (!f)
return errno;
-   }
+
parse(f);
fclose(f);
return 0;
-- 
1.7.10.4

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

[PATCH 09/10] staging: usbip: userspace: fix whitespace errors

From: Stefan Reif ke42c...@cip.cs.fau.de

This patch fixes the following checkpatch errors:
-ERROR: space required after that ','
-ERROR: spaces required around that '='
-ERROR: space prohibited before that close parenthesis
-WARNING: please, no space before tabs

Signed-off-by: Stefan Reif ke42c...@cip.cs.fau.de
Signed-off-by: Kurt Kanzenbach ly80t...@cip.cs.fau.de
---
 drivers/staging/usbip/userspace/src/usbip_detach.c  |2 +-
 drivers/staging/usbip/userspace/src/usbip_network.c |2 +-
 drivers/staging/usbip/userspace/src/usbip_network.h |4 ++--
 drivers/staging/usbip/userspace/src/usbipd.c|4 ++--
 4 files changed, 6 insertions(+), 6 deletions(-)

diff --git a/drivers/staging/usbip/userspace/src/usbip_detach.c 
b/drivers/staging/usbip/userspace/src/usbip_detach.c
index dac5f06..13308df 100644
--- a/drivers/staging/usbip/userspace/src/usbip_detach.c
+++ b/drivers/staging/usbip/userspace/src/usbip_detach.c
@@ -49,7 +49,7 @@ static int detach_port(char *port)
uint8_t portnum;
char path[PATH_MAX+1];
 
-   for (unsigned int i=0; i  strlen(port); i++)
+   for (unsigned int i = 0; i  strlen(port); i++)
if (!isdigit(port[i])) {
err(invalid port %s, port);
return -1;
diff --git a/drivers/staging/usbip/userspace/src/usbip_network.c 
b/drivers/staging/usbip/userspace/src/usbip_network.c
index 1a84dd3..4cb76e5 100644
--- a/drivers/staging/usbip/userspace/src/usbip_network.c
+++ b/drivers/staging/usbip/userspace/src/usbip_network.c
@@ -56,7 +56,7 @@ void usbip_net_pack_usb_device(int pack, struct 
usbip_usb_device *udev)
 {
usbip_net_pack_uint32_t(pack, udev-busnum);
usbip_net_pack_uint32_t(pack, udev-devnum);
-   usbip_net_pack_uint32_t(pack, udev-speed );
+   usbip_net_pack_uint32_t(pack, udev-speed);
 
usbip_net_pack_uint16_t(pack, udev-idVendor);
usbip_net_pack_uint16_t(pack, udev-idProduct);
diff --git a/drivers/staging/usbip/userspace/src/usbip_network.h 
b/drivers/staging/usbip/userspace/src/usbip_network.h
index 2d1e070..1bbefc9 100644
--- a/drivers/staging/usbip/userspace/src/usbip_network.h
+++ b/drivers/staging/usbip/userspace/src/usbip_network.h
@@ -35,8 +35,8 @@ struct op_common {
 
 #define PACK_OP_COMMON(pack, op_common)  do {\
usbip_net_pack_uint16_t(pack, (op_common)-version);\
-   usbip_net_pack_uint16_t(pack, (op_common)-code   );\
-   usbip_net_pack_uint32_t(pack, (op_common)-status );\
+   usbip_net_pack_uint16_t(pack, (op_common)-code);\
+   usbip_net_pack_uint32_t(pack, (op_common)-status);\
 } while (0)
 
 /* -- */
diff --git a/drivers/staging/usbip/userspace/src/usbipd.c 
b/drivers/staging/usbip/userspace/src/usbipd.c
index 34760cc..cc3be17 100644
--- a/drivers/staging/usbip/userspace/src/usbipd.c
+++ b/drivers/staging/usbip/userspace/src/usbipd.c
@@ -60,7 +60,7 @@ static const char usbipd_help_string[] =
   -d, --debug \n
   Print debugging information.\n
   \n
-  -h, --help  \n
+  -h, --help  \n
   Print this help.\n
   \n
   -v, --version   \n
@@ -446,7 +446,7 @@ static int do_standalone_mode(int daemonize)
}
 
if (daemonize) {
-   if (daemon(0,0)  0) {
+   if (daemon(0, 0)  0) {
err(daemonizing failed: %s, strerror(errno));
return -1;
}
-- 
1.7.10.4

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

[PATCH 10/10] staging: usbip: removed lines over 80 characters

This patch fixes the following checkpatch warning:
-WARNING: line over 80 characters

Signed-off-by: Kurt Kanzenbach ly80t...@cip.cs.fau.de
---
 drivers/staging/usbip/stub_dev.c |6 --
 1 file changed, 4 insertions(+), 2 deletions(-)

diff --git a/drivers/staging/usbip/stub_dev.c b/drivers/staging/usbip/stub_dev.c
index 67556ac..0a70b8e 100644
--- a/drivers/staging/usbip/stub_dev.c
+++ b/drivers/staging/usbip/stub_dev.c
@@ -114,8 +114,10 @@ static ssize_t store_sockfd(struct device *dev, struct 
device_attribute *attr,
 
spin_unlock_irq(sdev-ud.lock);
 
-   sdev-ud.tcp_rx = kthread_get_run(stub_rx_loop, sdev-ud, 
stub_rx);
-   sdev-ud.tcp_tx = kthread_get_run(stub_tx_loop, sdev-ud, 
stub_tx);
+   sdev-ud.tcp_rx = kthread_get_run(stub_rx_loop, sdev-ud,
+ stub_rx);
+   sdev-ud.tcp_tx = kthread_get_run(stub_tx_loop, sdev-ud,
+ stub_tx);
 
spin_lock_irq(sdev-ud.lock);
sdev-ud.status = SDEV_ST_USED;
-- 
1.7.10.4

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

[PATCH 03/10] staging: usbip: userspace: libsrc: spaces required around that '='

This patch fixes the following checkpatch error:
-ERROR: spaces required around that '='

Signed-off-by: Kurt Kanzenbach ly80t...@cip.cs.fau.de
---
 drivers/staging/usbip/userspace/libsrc/names.c|2 +-
 drivers/staging/usbip/userspace/libsrc/usbip_common.c |6 +++---
 2 files changed, 4 insertions(+), 4 deletions(-)

diff --git a/drivers/staging/usbip/userspace/libsrc/names.c 
b/drivers/staging/usbip/userspace/libsrc/names.c
index 8fe932d..8ee370b 100644
--- a/drivers/staging/usbip/userspace/libsrc/names.c
+++ b/drivers/staging/usbip/userspace/libsrc/names.c
@@ -472,7 +472,7 @@ static void parse(FILE *f)
 {
char buf[512], *cp;
unsigned int linectr = 0;
-   int lastvendor = -1, lastclass = -1, lastsubclass = -1, lasthut=-1, 
lastlang=-1;
+   int lastvendor = -1, lastclass = -1, lastsubclass = -1, lasthut = -1, 
lastlang = -1;
unsigned int u;
 
while (fgets(buf, sizeof(buf), f)) {
diff --git a/drivers/staging/usbip/userspace/libsrc/usbip_common.c 
b/drivers/staging/usbip/userspace/libsrc/usbip_common.c
index 98dc3df..b55df81 100644
--- a/drivers/staging/usbip/userspace/libsrc/usbip_common.c
+++ b/drivers/staging/usbip/userspace/libsrc/usbip_common.c
@@ -44,7 +44,7 @@ static struct portst_string portst_strings[] = {
 
 const char *usbip_status_string(int32_t status)
 {
-   for (int i=0; portst_strings[i].desc != NULL; i++)
+   for (int i = 0; portst_strings[i].desc != NULL; i++)
if (portst_strings[i].num == status)
return portst_strings[i].desc;
 
@@ -53,7 +53,7 @@ const char *usbip_status_string(int32_t status)
 
 const char *usbip_speed_string(int num)
 {
-   for (int i=0; speed_strings[i].speed != NULL; i++)
+   for (int i = 0; speed_strings[i].speed != NULL; i++)
if (speed_strings[i].num == num)
return speed_strings[i].desc;
 
@@ -172,7 +172,7 @@ int read_attr_speed(struct sysfs_device *dev)
 err:
sysfs_close_attribute(attr);
 
-   for (int i=0; speed_strings[i].speed != NULL; i++) {
+   for (int i = 0; speed_strings[i].speed != NULL; i++) {
if (!strcmp(speed, speed_strings[i].speed))
return speed_strings[i].num;
}
-- 
1.7.10.4

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

[PATCH 02/10] staging: usbip: userspace: libsrc: do not init static/globals to 0

This patch fixes the following checkpatch errors:
-ERROR: do not initialise statics to 0 or NULL
-ERROR: do not initialise globals to 0 or NULL

Signed-off-by: Kurt Kanzenbach ly80t...@cip.cs.fau.de
---
 drivers/staging/usbip/userspace/libsrc/names.c|2 +-
 drivers/staging/usbip/userspace/libsrc/usbip_common.c |6 +++---
 2 files changed, 4 insertions(+), 4 deletions(-)

diff --git a/drivers/staging/usbip/userspace/libsrc/names.c 
b/drivers/staging/usbip/userspace/libsrc/names.c
index 448d09d..8fe932d 100644
--- a/drivers/staging/usbip/userspace/libsrc/names.c
+++ b/drivers/staging/usbip/userspace/libsrc/names.c
@@ -246,7 +246,7 @@ struct pool {
void *mem;
 };
 
-static struct pool *pool_head = NULL;
+static struct pool *pool_head;
 
 static void *my_malloc(size_t size)
 {
diff --git a/drivers/staging/usbip/userspace/libsrc/usbip_common.c 
b/drivers/staging/usbip/userspace/libsrc/usbip_common.c
index 154b4b1..98dc3df 100644
--- a/drivers/staging/usbip/userspace/libsrc/usbip_common.c
+++ b/drivers/staging/usbip/userspace/libsrc/usbip_common.c
@@ -8,9 +8,9 @@
 #undef  PROGNAME
 #define PROGNAME libusbip
 
-int usbip_use_syslog = 0;
-int usbip_use_stderr = 0;
-int usbip_use_debug  = 0;
+int usbip_use_syslog;
+int usbip_use_stderr;
+int usbip_use_debug;
 
 struct speed_string {
int num;
-- 
1.7.10.4

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

[PATCH 00/10] staging: usbip: Coding style issues

Stefan and I fixed some coding style issues as reported by checkpatch.pl

Kurt Kanzenbach (8):
  staging: usbip: userspace: libsrc: fix indention
  staging: usbip: userspace: libsrc: do not init static/globals to 0
  staging: usbip: userspace: libsrc: spaces required around that '='
  staging: usbip: userspace: libsrc: (foo*) should be (foo *)
  staging: usbip: userspace: libsrc: replaced lines over 80 characters
  staging: usbip: userspace: libsrc: removed assignments in if
conditions
  staging: usbip: userspace: libsrc: added missing space
  staging: usbip: removed lines over 80 characters

Stefan Reif (2):
  staging: usbip: remove unnecessary braces
  staging: usbip: userspace: fix whitespace errors

 drivers/staging/usbip/stub_dev.c   |6 +-
 drivers/staging/usbip/userspace/libsrc/names.c |  532 +++-
 drivers/staging/usbip/userspace/libsrc/names.h |3 +-
 .../staging/usbip/userspace/libsrc/usbip_common.c  |   28 +-
 .../staging/usbip/userspace/libsrc/usbip_common.h  |   11 +-
 .../staging/usbip/userspace/libsrc/vhci_driver.c   |   40 +-
 drivers/staging/usbip/userspace/src/usbip_detach.c |2 +-
 .../staging/usbip/userspace/src/usbip_network.c|2 +-
 .../staging/usbip/userspace/src/usbip_network.h|4 +-
 drivers/staging/usbip/userspace/src/usbipd.c   |4 +-
 drivers/staging/usbip/vhci_hcd.c   |5 +-
 11 files changed, 365 insertions(+), 272 deletions(-)

--
1.7.10.4

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

[PATCH 01/10] staging: usbip: userspace: libsrc: fix indention

This patch fixes the following checkpatch warning:
-ERROR: code indent should use tabs where possible
-WARNING: suspect code indent for conditional statements

Signed-off-by: Kurt Kanzenbach ly80t...@cip.cs.fau.de
---
 drivers/staging/usbip/userspace/libsrc/names.c |  380 ++--
 .../staging/usbip/userspace/libsrc/vhci_driver.c   |   20 +-
 2 files changed, 200 insertions(+), 200 deletions(-)

diff --git a/drivers/staging/usbip/userspace/libsrc/names.c 
b/drivers/staging/usbip/userspace/libsrc/names.c
index b4de18b..448d09d 100644
--- a/drivers/staging/usbip/userspace/libsrc/names.c
+++ b/drivers/staging/usbip/userspace/libsrc/names.c
@@ -22,8 +22,8 @@
  */
 
 /*
- * Copyright (C) 2005 Takahiro Hirofuchi
- * - names_deinit() is added.
+ * Copyright (C) 2005 Takahiro Hirofuchi
+ * - names_deinit() is added.
  */
 
 /*/
@@ -82,9 +82,9 @@ struct audioterminal {
 };
 
 struct genericstrtable {
-struct genericstrtable *next;
-unsigned int num;
-char name[1];
+   struct genericstrtable *next;
+   unsigned int num;
+   char name[1];
 };
 
 /* -- */
@@ -124,12 +124,12 @@ static struct genericstrtable *countrycodes[HASHSZ] = { 
NULL, };
 
 static const char *names_genericstrtable(struct genericstrtable *t[HASHSZ], 
unsigned int index)
 {
-struct genericstrtable *h;
+   struct genericstrtable *h;
 
-for (h = t[hashnum(index)]; h; h = h-next)
-if (h-num == index)
-return h-name;
-return NULL;
+   for (h = t[hashnum(index)]; h; h = h-next)
+   if (h-num == index)
+   return h-name;
+   return NULL;
 }
 
 const char *names_hid(u_int8_t hidd)
@@ -409,20 +409,20 @@ static int new_audioterminal(const char *name, u_int16_t 
termt)
 
 static int new_genericstrtable(struct genericstrtable *t[HASHSZ], const char 
*name, unsigned int index)
 {
-struct genericstrtable *g;
+   struct genericstrtable *g;
unsigned int h = hashnum(index);
 
-for (g = t[h]; g; g = g-next)
-if (g-num == index)
-return -1;
-g = my_malloc(sizeof(struct genericstrtable) + strlen(name));
-if (!g)
-return -1;
-strcpy(g-name, name);
-g-num = index;
-g-next = t[h];
-t[h] = g;
-return 0;
+   for (g = t[h]; g; g = g-next)
+   if (g-num == index)
+   return -1;
+   g = my_malloc(sizeof(struct genericstrtable) + strlen(name));
+   if (!g)
+   return -1;
+   strcpy(g-name, name);
+   g-num = index;
+   g-next = t[h];
+   t[h] = g;
+   return 0;
 }
 
 static int new_hid(const char *name, u_int8_t hidd)
@@ -485,92 +485,92 @@ static void parse(FILE *f)
if (buf[0] == '#' || !buf[0])
continue;
cp = buf;
-if (buf[0] == 'P'  buf[1] == 'H'  buf[2] == 'Y'  buf[3] 
== 'S'  buf[4] == 'D' 
-buf[5] == 'E'  buf[6] == 'S'  /*isspace(buf[7])*/ 
buf[7] == ' ') {
-cp = buf + 8;
-while (isspace(*cp))
-cp++;
-if (!isxdigit(*cp)) {
-fprintf(stderr, Invalid Physdes type at line 
%u\n, linectr);
-continue;
-}
-u = strtoul(cp, cp, 16);
-while (isspace(*cp))
-cp++;
-if (!*cp) {
-fprintf(stderr, Invalid Physdes type at line 
%u\n, linectr);
-continue;
-}
-if (new_physdes(cp, u))
-fprintf(stderr, Duplicate Physdes  type spec 
at line %u terminal type %04x %s\n, linectr, u, cp);
-DBG(printf(line %5u physdes type %02x %s\n, linectr, 
u, cp));
-continue;
-
-}
-if (buf[0] == 'P'  buf[1] == 'H'  buf[2] == 'Y'  
/*isspace(buf[3])*/ buf[3] == ' ') {
-cp = buf + 4;
-while (isspace(*cp))
-cp++;
-if (!isxdigit(*cp)) {
-fprintf(stderr, Invalid PHY type at line 
%u\n, linectr);
-continue;
-}
-u = strtoul(cp, cp, 16);
-while (isspace(*cp))
-cp++;
-if (!*cp) {
-fprintf(stderr, Invalid PHY type at line 
%u\n, linectr);
-

[PATCH 04/10] staging: usbip: userspace: libsrc: (foo) should be (foo )

This patch fixes the following checkpatch error:
-ERROR: (foo*) should be (foo *)

Signed-off-by: Kurt Kanzenbach ly80t...@cip.cs.fau.de
---
 drivers/staging/usbip/userspace/libsrc/vhci_driver.c |2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/staging/usbip/userspace/libsrc/vhci_driver.c 
b/drivers/staging/usbip/userspace/libsrc/vhci_driver.c
index 7a5da58..b9c6e2a 100644
--- a/drivers/staging/usbip/userspace/libsrc/vhci_driver.c
+++ b/drivers/staging/usbip/userspace/libsrc/vhci_driver.c
@@ -36,7 +36,7 @@ static struct usbip_imported_device 
*imported_device_init(struct usbip_imported_
goto err;
 
memcpy(new_cdev, cdev, sizeof(*new_cdev));
-   dlist_unshift(idev-cdev_list, (void*) new_cdev);
+   dlist_unshift(idev-cdev_list, (void *) new_cdev);
}
}
 
-- 
1.7.10.4

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

[PATCH 05/10] staging: usbip: userspace: libsrc: replaced lines over 80 characters

This patch fixes some of the following checkpatch warnings:
-WARNING: line over 80 characters

We did not split format strings for readability.

Signed-off-by: Kurt Kanzenbach ly80t...@cip.cs.fau.de
---
 drivers/staging/usbip/userspace/libsrc/names.c |  215 +---
 drivers/staging/usbip/userspace/libsrc/names.h |3 +-
 .../staging/usbip/userspace/libsrc/usbip_common.c  |   16 +-
 .../staging/usbip/userspace/libsrc/usbip_common.h  |9 +-
 .../staging/usbip/userspace/libsrc/vhci_driver.c   |   18 +-
 5 files changed, 175 insertions(+), 86 deletions(-)

diff --git a/drivers/staging/usbip/userspace/libsrc/names.c 
b/drivers/staging/usbip/userspace/libsrc/names.c
index 8ee370b..a66f539 100644
--- a/drivers/staging/usbip/userspace/libsrc/names.c
+++ b/drivers/staging/usbip/userspace/libsrc/names.c
@@ -122,7 +122,8 @@ static struct genericstrtable *countrycodes[HASHSZ] = { 
NULL, };
 
 /* -- */
 
-static const char *names_genericstrtable(struct genericstrtable *t[HASHSZ], 
unsigned int index)
+static const char *names_genericstrtable(struct genericstrtable *t[HASHSZ],
+unsigned int index)
 {
struct genericstrtable *h;
 
@@ -216,13 +217,16 @@ const char *names_subclass(u_int8_t classid, u_int8_t 
subclassid)
return NULL;
 }
 
-const char *names_protocol(u_int8_t classid, u_int8_t subclassid, u_int8_t 
protocolid)
+const char *names_protocol(u_int8_t classid, u_int8_t subclassid,
+  u_int8_t protocolid)
 {
struct protocol *p;
 
-   p = protocols[hashnum((classid  16) | (subclassid  8) | 
protocolid)];
+   p = protocols[hashnum((classid  16) | (subclassid  8)
+ | protocolid)];
for (; p; p = p-next)
-   if (p-classid == classid  p-subclassid == subclassid  
p-protocolid == protocolid)
+   if (p-classid == classid  p-subclassid == subclassid 
+   p-protocolid == protocolid)
return p-name;
return NULL;
 }
@@ -308,7 +312,8 @@ static int new_vendor(const char *name, u_int16_t vendorid)
return 0;
 }
 
-static int new_product(const char *name, u_int16_t vendorid, u_int16_t 
productid)
+static int new_product(const char *name, u_int16_t vendorid,
+  u_int16_t productid)
 {
struct product *p;
unsigned int h = hashnum((vendorid  16) | productid);
@@ -367,14 +372,17 @@ static int new_subclass(const char *name, u_int8_t 
classid, u_int8_t subclassid)
return 0;
 }
 
-static int new_protocol(const char *name, u_int8_t classid, u_int8_t 
subclassid, u_int8_t protocolid)
+static int new_protocol(const char *name, u_int8_t classid, u_int8_t 
subclassid,
+   u_int8_t protocolid)
 {
struct protocol *p;
-   unsigned int h = hashnum((classid  16) | (subclassid  8) | 
protocolid);
+   unsigned int h = hashnum((classid  16) | (subclassid  8)
+| protocolid);
 
p = protocols[h];
for (; p; p = p-next)
-   if (p-classid == classid  p-subclassid == subclassid  
p-protocolid == protocolid)
+   if (p-classid == classid  p-subclassid == subclassid
+p-protocolid == protocolid)
return -1;
p = my_malloc(sizeof(struct protocol) + strlen(name));
if (!p)
@@ -407,7 +415,8 @@ static int new_audioterminal(const char *name, u_int16_t 
termt)
return 0;
 }
 
-static int new_genericstrtable(struct genericstrtable *t[HASHSZ], const char 
*name, unsigned int index)
+static int new_genericstrtable(struct genericstrtable *t[HASHSZ],
+  const char *name, unsigned int index)
 {
struct genericstrtable *g;
unsigned int h = hashnum(index);
@@ -472,7 +481,11 @@ static void parse(FILE *f)
 {
char buf[512], *cp;
unsigned int linectr = 0;
-   int lastvendor = -1, lastclass = -1, lastsubclass = -1, lasthut = -1, 
lastlang = -1;
+   int lastvendor = -1;
+   int lastclass = -1;
+   int lastsubclass = -1;
+   int lasthut = -1;
+   int lastlang = -1;
unsigned int u;
 
while (fgets(buf, sizeof(buf), f)) {
@@ -485,67 +498,82 @@ static void parse(FILE *f)
if (buf[0] == '#' || !buf[0])
continue;
cp = buf;
-   if (buf[0] == 'P'  buf[1] == 'H'  buf[2] == 'Y'  buf[3] 
== 'S'  buf[4] == 'D' 
-   buf[5] == 'E'  buf[6] == 'S'  /*isspace(buf[7])*/ 
buf[7] == ' ') {
+   if (buf[0] == 'P'  buf[1] == 'H'  buf[2] == 'Y'
+buf[3] == 'S'  buf[4] == 'D'  buf[5] == 'E'
+buf[6] == 'S'  /*isspace(buf[7])*/ buf[7] == ' ') {
cp = buf + 8;
while (isspace(*cp))
cp++;

[PATCH 0/3] usbnet: smsc95xx: fix smsc95xx_suspend

Hi,

The 1st two patches fix smsc95xx_suspend, and the 1st one should
be backported to 3.8.

The last one is to rename the misleading FEATURE_AUTOSUSPEND.

Thanks,
--
Ming Lei

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

[PATCH 1/3] usbnet: smsc95xx: fix suspend failure

The three below functions:

smsc95xx_enter_suspend0()
smsc95xx_enter_suspend1()
smsc95xx_enter_suspend2()

return  0 in case of success, so they will cause smsc95xx_suspend()
to return  0 and cause suspend failure.

The bug is introduced in commit 3b9f7d(smsc95xx: fix error handling
in suspend failure case).

Cc: sta...@vger.kernel.org[3.8]
Cc: Steve Glendinning steve.glendinn...@shawell.net
Signed-off-by: Ming Lei ming@canonical.com
---
 drivers/net/usb/smsc95xx.c |   12 +---
 1 file changed, 9 insertions(+), 3 deletions(-)

diff --git a/drivers/net/usb/smsc95xx.c b/drivers/net/usb/smsc95xx.c
index ff4fa37..b2721bc 100644
--- a/drivers/net/usb/smsc95xx.c
+++ b/drivers/net/usb/smsc95xx.c
@@ -1247,10 +1247,12 @@ static int smsc95xx_enter_suspend0(struct usbnet *dev)
 
/* read back PM_CTRL */
ret = smsc95xx_read_reg_nopm(dev, PM_CTRL, val);
+   if (ret  0)
+   return ret;
 
pdata-suspend_flags |= SUSPEND_SUSPEND0;
 
-   return ret;
+   return 0;
 }
 
 static int smsc95xx_enter_suspend1(struct usbnet *dev)
@@ -1293,10 +1295,12 @@ static int smsc95xx_enter_suspend1(struct usbnet *dev)
val |= (PM_CTL_WUPS_ED_ | PM_CTL_ED_EN_);
 
ret = smsc95xx_write_reg_nopm(dev, PM_CTRL, val);
+   if (ret  0)
+   return ret;
 
pdata-suspend_flags |= SUSPEND_SUSPEND1;
 
-   return ret;
+   return 0;
 }
 
 static int smsc95xx_enter_suspend2(struct usbnet *dev)
@@ -1313,10 +1317,12 @@ static int smsc95xx_enter_suspend2(struct usbnet *dev)
val |= PM_CTL_SUS_MODE_2;
 
ret = smsc95xx_write_reg_nopm(dev, PM_CTRL, val);
+   if (ret  0)
+   return ret;
 
pdata-suspend_flags |= SUSPEND_SUSPEND2;
 
-   return ret;
+   return 0;
 }
 
 static int smsc95xx_enter_suspend3(struct usbnet *dev)
-- 
1.7.9.5

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

[PATCH 2/3] usbnet: smsc95xx: fix broken runtime suspend

Commit b2d4b150(smsc95xx: enable dynamic autosuspend) implements
autosuspend, but breaks current runtime suspend, such as:
when the interface becomes down, the usb device can't be put into
runtime suspend any more.

This patch fixes the broken runtime suspend.

Cc: Steve Glendinning steve.glendinn...@shawell.net
Signed-off-by: Ming Lei ming@canonical.com
---
 drivers/net/usb/smsc95xx.c |   12 ++--
 1 file changed, 2 insertions(+), 10 deletions(-)

diff --git a/drivers/net/usb/smsc95xx.c b/drivers/net/usb/smsc95xx.c
index b2721bc..7fa9622 100644
--- a/drivers/net/usb/smsc95xx.c
+++ b/drivers/net/usb/smsc95xx.c
@@ -1418,15 +1418,6 @@ static int smsc95xx_suspend(struct usb_interface *intf, 
pm_message_t message)
u32 val, link_up;
int ret;
 
-   /* TODO: don't indicate this feature to usb framework if
-* our current hardware doesn't have the capability
-*/
-   if ((message.event == PM_EVENT_AUTO_SUSPEND) 
-   (!(pdata-features  FEATURE_AUTOSUSPEND))) {
-   netdev_warn(dev-net, autosuspend not supported\n);
-   return -EBUSY;
-   }
-
ret = usbnet_suspend(intf, message);
if (ret  0) {
netdev_warn(dev-net, usbnet_suspend error\n);
@@ -1441,7 +1432,8 @@ static int smsc95xx_suspend(struct usb_interface *intf, 
pm_message_t message)
/* determine if link is up using only _nopm functions */
link_up = smsc95xx_link_ok_nopm(dev);
 
-   if (message.event == PM_EVENT_AUTO_SUSPEND) {
+   if (message.event == PM_EVENT_AUTO_SUSPEND 
+   (pdata-features  FEATURE_AUTOSUSPEND)) {
ret = smsc95xx_autosuspend(dev, link_up);
goto done;
}
-- 
1.7.9.5

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

[PATCH 3/3] usbnet: smsc95xx: rename FEATURE_AUTOSUSPEND

The name of FEATURE_AUTOSUSPEND is very misleading and the actual
meaning is remote wakeup, but a device incapable of remote wakeup
still can support USB autosuspend under some situations, so rename
it to avoid misunderstanding.

Cc: Steve Glendinning steve.glendinn...@shawell.net
Signed-off-by: Ming Lei ming@canonical.com
---
 drivers/net/usb/smsc95xx.c |   14 +++---
 1 file changed, 7 insertions(+), 7 deletions(-)

diff --git a/drivers/net/usb/smsc95xx.c b/drivers/net/usb/smsc95xx.c
index 7fa9622..e6d2dea 100644
--- a/drivers/net/usb/smsc95xx.c
+++ b/drivers/net/usb/smsc95xx.c
@@ -53,7 +53,7 @@
 
 #define FEATURE_8_WAKEUP_FILTERS   (0x01)
 #define FEATURE_PHY_NLP_CROSSOVER  (0x02)
-#define FEATURE_AUTOSUSPEND(0x04)
+#define FEATURE_REMOTE_WAKEUP  (0x04)
 
 #define SUSPEND_SUSPEND0   (0x01)
 #define SUSPEND_SUSPEND1   (0x02)
@@ -1146,7 +1146,7 @@ static int smsc95xx_bind(struct usbnet *dev, struct 
usb_interface *intf)
(val == ID_REV_CHIP_ID_89530_) || (val == ID_REV_CHIP_ID_9730_))
pdata-features = (FEATURE_8_WAKEUP_FILTERS |
FEATURE_PHY_NLP_CROSSOVER |
-   FEATURE_AUTOSUSPEND);
+   FEATURE_REMOTE_WAKEUP);
else if (val == ID_REV_CHIP_ID_9512_)
pdata-features = FEATURE_8_WAKEUP_FILTERS;
 
@@ -1378,7 +1378,7 @@ static int smsc95xx_autosuspend(struct usbnet *dev, u32 
link_up)
if (!link_up) {
/* link is down so enter EDPD mode, but only if device can
 * reliably resume from it.  This check should be redundant
-* as current FEATURE_AUTOSUSPEND parts also support
+* as current FEATURE_REMOTE_WAKEUP parts also support
 * FEATURE_PHY_NLP_CROSSOVER but it's included for clarity */
if (!(pdata-features  FEATURE_PHY_NLP_CROSSOVER)) {
netdev_warn(dev-net, EDPD not supported\n);
@@ -1433,7 +1433,7 @@ static int smsc95xx_suspend(struct usb_interface *intf, 
pm_message_t message)
link_up = smsc95xx_link_ok_nopm(dev);
 
if (message.event == PM_EVENT_AUTO_SUSPEND 
-   (pdata-features  FEATURE_AUTOSUSPEND)) {
+   (pdata-features  FEATURE_REMOTE_WAKEUP)) {
ret = smsc95xx_autosuspend(dev, link_up);
goto done;
}
@@ -1870,11 +1870,11 @@ static int smsc95xx_manage_power(struct usbnet *dev, 
int on)
 
dev-intf-needs_remote_wakeup = on;
 
-   if (pdata-features  FEATURE_AUTOSUSPEND)
+   if (pdata-features  FEATURE_REMOTE_WAKEUP)
return 0;
 
-   /* this chip revision doesn't support autosuspend */
-   netdev_info(dev-net, hardware doesn't support USB autosuspend\n);
+   /* this chip revision isn't capable of remote wakeup */
+   netdev_info(dev-net, hardware isn't capable of remote wakeup\n);
 
if (on)
usb_autopm_get_interface_no_resume(dev-intf);
-- 
1.7.9.5

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

Re: Issue with mini-SaS to eSATA to USB 3.0 setup

2013-02-22 Thread Fabio David

resent

On Thu, Feb 21, 2013 at 4:26 PM, Sarah Sharp sarah.a.sh...@linux.intel.com
wrote:

Cc-ing the SCSI and USB storage list.

Folks, does the attached picture look like a sane setup? I've never
used mini-SaS to eSATA adapter before, let alone with four eSATA to USB
3.0 adapters.

I know it is a weird setup...

On Tue, Jan 29, 2013 at 12:56:02PM -0200, Fabio David wrote:
Hi Sarah,

My name is Fabio David and I am from Brazil. I've seen your posts on
several forums and read articles about you. I really admire your work.

Maybe you can help me. I'm trying to connect a PC running Centos 6.3
to a CRU dataport 4-bay storage device. This device only has a miniSaS
port.

Here is my scenario:

- DataCRU device with 4 hot-swapables bays.
http://www.cru-inc.com/slideshow.php?dir=//Digital-Cinema//sel=5
- MiniSaS cable connects to the DataCRU device and on the other side
there are 4 eSata connectors

http://www.elpeus.com/sas-mini-sas/external-mini-sas-cables/sff-8088-to-4-esata/3m-mini-sas-sff-8088-to-4-esata-cable/
- 4 eSata-USB3.0 adaptors connected to each eSata connector
- Adaptors connected to a USB3.0 HUB
- USB3.0 hub connected to PC

Everything works ok, I can mount/read the HDs, but sometimes the
system does not detect when a hard drive is inserted/removed from a
DataCru bay. No events are generated, nothing appears in
/proc/partitions nor udev
is called to apply my rules.

Do you lose only hard drive insertion events, or do you lose remove
events as well?

It loses both events, randomly.

For example, what happens when you do this:

1. Unplug the eSATA to USB adapters from the USB 3.0 hub.
2. Insert a hard drive into the bay.
3. Connect the eSATA to USB adapter to the USB 3.0 hub.
4. Wait for hard drive detection, then hot-remove the drive from the
bay.

Sometimes it will detect, sometimes dont.

However, everything works fine when connected directly to PC's USB
port. Please look at the attached picture.

It looks like you're only attaching one eSATA to USB adapter to the
roothub. Do you only have one USB 3.0 port on the host, or can you try
plugging in multiple eSATA to USB adapters into the roothub?

Unfortunately, the host only has 1 USB3.0 port

Does the setup work when only one eSATA to USB adapter is plugged into
the USB 3.0 hub?

Same problem...

Do you have any suggestions?

A couple possible root causes come to mind:

1. Perhaps the USB 3.0 hub is interfering with communication to your
eSATA to USB 3.0 adapters.

2. Maybe USB device suspend is to blame. Do you have USB device suspend
enabled for the eSATA to USB adapters?

I am not sure, I thought it was disabled by default. How can I check?

3. Perhaps the SATA adapters aren't responding with a Medium Changed
status when the USB storage device is plugged in.

Can you send me dmesg, starting from just before you insert a hard drive
into the drive bays? I need dmesg for both when the SATA adapter is
connected directly to the roothub, and when it's connected to the USB
3.0 hub.

A usbmon trace might also be useful for the USB storage developers.
Documentation on how to take that trace is here:

http://lxr.linux.no/#linux/Documentation/usb/usbmon.txt

I will send you as soon as possible

Thanks in advance for your help

Fabio

Sarah Sharp

===

lsusb returns

Bus 001 Device 001: ID 1d6b:0002 Linux Foundation 2.0 root hub
Bus 002 Device 001: ID 1d6b:0001 Linux Foundation 1.1 root hub
Bus 003 Device 001: ID 1d6b:0001 Linux Foundation 1.1 root hub
Bus 004 Device 001: ID 1d6b:0001 Linux Foundation 1.1 root hub
Bus 005 Device 001: ID 1d6b:0001 Linux Foundation 1.1 root hub
Bus 001 Device 002: ID 13d3:3323 IMC Networks
Bus 001 Device 009: ID 2109:3431 HUB 3.0
Bus 006 Device 001: ID 1d6b:0002 Linux Foundation 2.0 root hub
Bus 007 Device 001: ID 1d6b:0003 Linux Foundation 3.0 root hub
Bus 007 Device 040: ID 2109:0810 HUB 3.0
Bus 007 Device 041: ID 1234:5678 Brain Actuated Technologies
Bus 007 Device 042: ID 1234:5678 Brain Actuated Technologies
Bus 007 Device 043: ID 1234:5678 Brain Actuated Technologies
Bus 007 Device 044: ID 1234:5678 Brain Actuated Technologies

/var/log/messages

Jan 27 18:00:28 localhost kernel: usb 7-1: New USB device found,
idVendor=2109, idProduct=0810
Jan 27 18:00:28 localhost kernel: usb 7-1: New USB device strings:
Mfr=1, Product=2, SerialNumber=0
Jan 27 18:00:28 localhost kernel: usb 7-1: Product: 4-Port USB 3.0 Hub
Jan 27 18:00:28 localhost kernel: usb 7-1: Manufacturer: VIA Labs, Inc.
Jan 27 18:00:28 localhost kernel: usb 7-1: configuration #1 chosen from
1 choice
Jan 27 18:00:28 localhost kernel: hub 7-1:1.0: USB hub found
Jan 27 18:00:28 localhost kernel: hub 7-1:1.0: 4 ports detected

Jan 28 21:32:02 localhost kernel: usb

Re: Issue with mini-SaS to eSATA to USB 3.0 setup

2013-02-22 Thread Fabio David

RESENT

-- Forwarded message --
From: Fabio David fabioda...@gmail.com
Date: Thu, Feb 21, 2013 at 5:42 PM
Subject: Re: [usb-storage] Issue with mini-SaS to eSATA to USB 3.0 setup

Thanks for your reply, Matt.

But it works! Only sometimes the OS doesnt see hot-insertion /
hot-remove. And if I connect the USB/eSata directly to PC (without the
USB 3.0 hub), it works 100%, never miss an insertion/removal!

I have created some UDEV mounting rules, and they work fine, no mather
in which bay the HDD has been inserted. The problem is that sometimes
it can not recognize a hot-insert (which is not a big problem, the
user just has to remove/insert it again). But the major problem occurs
when it cannot detect a hot-removal, because the device stays mounted,
partitions still appear at /proc/partitions, etc. as if the HDD was
still there.

Fabio

On Thu, Feb 21, 2013 at 5:16 PM, Matthew Dharm
mdharm-...@one-eyed-alien.net wrote:

 I highly doubt hot-insert and hot-remove of HDDs from the 4-bay
 container (without removing the corresponding USB/eSATA adaptor) will
 work.

 The USB/eSATA adaptor does not have a way to inform the host that the
 eSATA side has been disconnected from the HDD.  That functionality
 isn't in the usb-storage protocol.

 This type of functionality *might* be supported in the UAS protocol,
 but I don't know.

 Matt

 On Thu, Feb 21, 2013 at 11:26 AM, Sarah Sharp
 sarah.a.sh...@linux.intel.com wrote:
  Cc-ing the SCSI and USB storage list.

  Folks, does the attached picture look like a sane setup?  I've never
  used mini-SaS to eSATA adapter before, let alone with four eSATA to USB
  3.0 adapters.

  On Tue, Jan 29, 2013 at 12:56:02PM -0200, Fabio David wrote:
  Hi Sarah,

  My name is Fabio David and I am from Brazil. I've seen your posts on
  several forums and read articles about you. I really admire your work.

  Maybe you can help me. I'm trying to connect a PC running Centos 6.3
  to a CRU dataport 4-bay storage device. This device only has a miniSaS
  port.

  Here is my scenario:

  - DataCRU device with 4 hot-swapables bays.
  http://www.cru-inc.com/slideshow.php?dir=//Digital-Cinema//sel=5
  - MiniSaS cable connects to the DataCRU device and on the other side
  there are 4 eSata connectors

  http://www.elpeus.com/sas-mini-sas/external-mini-sas-cables/sff-8088-to-4-esata/3m-mini-sas-sff-8088-to-4-esata-cable/
  - 4 eSata-USB3.0 adaptors connected to each eSata connector
  - Adaptors connected to a USB3.0 HUB
  - USB3.0 hub connected to PC

  Everything works ok, I can mount/read the HDs, but sometimes the
  system does not detect when a hard drive is inserted/removed from a
  DataCru bay. No events are generated, nothing appears in
  /proc/partitions nor udev
  is called to apply my rules.

  Do you lose only hard drive insertion events, or do you lose remove
  events as well?

  For example, what happens when you do this:

  1. Unplug the eSATA to USB adapters from the USB 3.0 hub.
  2. Insert a hard drive into the bay.
  3. Connect the eSATA to USB adapter to the USB 3.0 hub.
  4. Wait for hard drive detection, then hot-remove the drive from the
  bay.

  However, everything works fine when connected directly to PC's USB
  port. Please look at the attached picture.

  It looks like you're only attaching one eSATA to USB adapter to the
  roothub.  Do you only have one USB 3.0 port on the host, or can you try
  plugging in multiple eSATA to USB adapters into the roothub?

  Does the setup work when only one eSATA to USB adapter is plugged into
  the USB 3.0 hub?

  Do you have any suggestions?

  A couple possible root causes come to mind:

  1. Perhaps the USB 3.0 hub is interfering with communication to your
  eSATA to USB 3.0 adapters.

  2. Maybe USB device suspend is to blame.  Do you have USB device suspend
  enabled for the eSATA to USB adapters?

  3. Perhaps the SATA adapters aren't responding with a Medium Changed
  status when the USB storage device is plugged in.

  Can you send me dmesg, starting from just before you insert a hard drive
  into the drive bays?  I need dmesg for both when the SATA adapter is
  connected directly to the roothub, and when it's connected to the USB
  3.0 hub.

  A usbmon trace might also be useful for the USB storage developers.
  Documentation on how to take that trace is here:

  http://lxr.linux.no/#linux/Documentation/usb/usbmon.txt

  Sarah Sharp

  ===

  lsusb returns

  Bus 001 Device 001: ID 1d6b:0002 Linux Foundation 2.0 root hub
  Bus 002 Device 001: ID 1d6b:0001 Linux Foundation 1.1 root hub
  Bus 003 Device 001: ID 1d6b:0001 Linux Foundation 1.1 root hub
  Bus 004 Device 001: ID 1d6b:0001 Linux Foundation 1.1 root hub
  Bus 005 Device 001: ID 1d6b:0001 Linux Foundation 1.1 root hub
  Bus 001 Device 002: ID 13d3:3323 IMC Networks
  Bus 001 Device 009: ID 2109:3431    HUB 3.0
  Bus 006 Device 001:

Re: BUG: remove ASS/PSS polling timeout

On Fri, 22 Feb 2013, Ronald wrote:

  Let me know how this works.
 
 I'm receiving the same result as Paul Hartman. Device works without a
 hitch after three reboots. If you consider it appriopate/helpful, you
 could add:
 
 Tested-by: Ronald Uitermark ronald...@gmail.com

Okay, thanks to both of you.  I'll submit a reversion patch, and a 
separate patch to decrease the timeout, after the current merge window 
closes.

 One note, the line that Paul Hartman is mentioning is probably
 repeating indefinitly during the connection. Don't know if that is
 intended/useful.

It's probably quite normal.  They are caused by media-change polling.  
Since these are only debug-level messages, they won't appear in a
normal kernel build.

Alan Stern

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

Re: USB: serial: port lifetimes (was: Re: USB Ooops PL2303 when unplug while use (linux v3.7.3))

On Fri, 22 Feb 2013, Johan Hovold wrote:

 We keep the usb-serial-port structure around until the last tty
 reference is dropped, but the port private data is freed as part of
 unregistering from the bus in disconnect. [ The subdrivers aren't
 supposed to access the ports after port_remove is called as part of
 unregistration. ]
 
 This wasn't always the case, but unregistering in serial_cleanup (when
 the last tty reference is dropped) had other issues. In particular, we
 would end up unregistering the parent device before its child (the
 interface and usb device are unregistered when disconnect returns)
 resulting in broken uevent devpaths:
 
 KERNEL[794.849051] remove   
 /devices/pci:00/:00:1a.7/usb1/1-4/1-4.1/1-4.1:1.0 (usb)
 KERNEL[794.851649] remove   /devices/pci:00/:00:1a.7/usb1/1-4/1-4.1 
 (usb)
 KERNEL[795.115623] remove   /1-4.1:1.0/ttyUSB0/tty/ttyUSB0 (tty)
 KERNEL[795.117429] remove   /1-4.1:1.0/ttyUSB0 (usb-serial)
 
 This was reported here
 
   https://bugs.freedesktop.org/show_bug.cgi?id=20703
 
 and fixed by Alan Stern in 2d93148ab6 (USB: serial: fix lifetime and
 locking problems) by moving unregistration to disconnect.
 
 So we end up with an unregistered device still possibly referenced by
 tty instead, and I suspect we can't do much else than deal with any
 post-disconnect callbacks. [ These should be few, especially with my
 latest TTY-patches applied. ]
 
 Alan, do you see any way around this? It's not possible (or desirable)
 to pin the parent device (interface) until the last reference is
 dropped, is it?

On the contrary, it is customary to pin data structures until the last 
reference to them is gone.  That's what krefs are for.

On the other hand, the port private data was owned entirely by the
serial sub-driver.  Neither the serial core nor the tty layer is able
to use it meaningfully -- they don't even know what type of structure
it is.

Therefore freeing the structure when the port is removed should be 
harmless -- unless the subdriver is called after the structure is 
deallocated.

This means there still is one bug remaining.  In
usb_serial_device_remove(), the call to tty_unregister_device() should
occur _before_ the call to driver-port_remove(), not _after_.  Do you
think changing the order cause any new problems?

Alan Stern

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

Re: [ANNOUNCE] tree maintenance change

2013-02-22 Thread Greg KH

On Fri, Feb 22, 2013 at 11:46:10AM +0200, Felipe Balbi wrote:
 Hi folks,
 
 I will be changing way I manage my tree. Instead of having separate
 fixes, dwc3, musb, xceiv and gadget branches, I will maintain only two
 branches (fixes and next). There will also be a 'testing' branch but
 nobody should be looking at that.
 
 fixes and next will always be immutable, which means that I will need
 your help to deliver patches which have been properly tested. All
 patches will go through compile testing and, where applicable, boot
 testing.

Very nice, thanks for doing this, I figured it was only a matter of time :)

greg k-h
--
To unsubscribe from this list: send the line unsubscribe linux-usb in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [GIT PATCH] USB patches for 3.9-rc1

On Fri, Feb 22, 2013 at 05:51:10PM +0800, Lan Tianyu wrote:

  Nothing major, lots of gadget fixes, and of course, xhci stuff.
  
   I get no USB devices recognised when I insert them any more, which
   I think is pretty major.  I suspect it has something to do with this ?
  
  Lan Tianyu (12):
usb: add runtime pm support for usb port device
usb: add usb port auto power off mechanism
  
   It looks like every port on my laptop is powered down, as I can't
   even charge devices with it.
 
   Do you enable the port/power/pm_qos_no_power_off?
   cat /sys/bus/usb/devices/(dev)/port/power/pm_qos_no_power_off to show.

/sys/bus/usb/devices/1-1.4/port/power/pm_qos_no_power_off:1
/sys/bus/usb/devices/1-1.6/port/power/pm_qos_no_power_off:1
/sys/bus/usb/devices/1-1/port/power/pm_qos_no_power_off:1
/sys/bus/usb/devices/2-1/port/power/pm_qos_no_power_off:1

I changed them to 0, made no difference.

   What can I do to debug this ?
   Can you attach the output of dmesg with CONFIG_USB_DEBUG?

http://paste.fedoraproject.org/3620

Dave

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

Re: [GIT PATCH] USB patches for 3.9-rc1

2013-02-22 Thread Lan Tianyu


On 2013/2/23 0:43, Dave Jones wrote:

On Fri, Feb 22, 2013 at 05:51:10PM +0800, Lan Tianyu wrote:

   Nothing major, lots of gadget fixes, and of course, xhci stuff.
   
I get no USB devices recognised when I insert them any more, which
I think is pretty major.  I suspect it has something to do with this ?
   
   Lan Tianyu (12):
 usb: add runtime pm support for usb port device
 usb: add usb port auto power off mechanism
   
It looks like every port on my laptop is powered down, as I can't
even charge devices with it.
  
Do you enable the port/power/pm_qos_no_power_off?
cat /sys/bus/usb/devices/(dev)/port/power/pm_qos_no_power_off to show.

/sys/bus/usb/devices/1-1.4/port/power/pm_qos_no_power_off:1
/sys/bus/usb/devices/1-1.6/port/power/pm_qos_no_power_off:1
/sys/bus/usb/devices/1-1/port/power/pm_qos_no_power_off:1
/sys/bus/usb/devices/2-1/port/power/pm_qos_no_power_off:1


At last, this mean usb port power off mechanism should not work.


I changed them to 0, made no difference.

What can I do to debug this ?
Can you attach the output of dmesg with CONFIG_USB_DEBUG?

http://paste.fedoraproject.org/3620

How aboutsudo lsusb -s 1:1 -v or 2:1?


Dave



--
Best Regards
Tianyu Lan
linux kernel enabling team
--
To unsubscribe from this list: send the line unsubscribe linux-usb in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: USB: serial: port lifetimes (was: Re: USB Ooops PL2303 when unplug while use (linux v3.7.3))

2013-02-22 Thread Johan Hovold

On Fri, Feb 22, 2013 at 10:17:51AM -0500, Alan Stern wrote:
 On Fri, 22 Feb 2013, Johan Hovold wrote:
 
  We keep the usb-serial-port structure around until the last tty
  reference is dropped, but the port private data is freed as part of
  unregistering from the bus in disconnect. [ The subdrivers aren't
  supposed to access the ports after port_remove is called as part of
  unregistration. ]
  
  This wasn't always the case, but unregistering in serial_cleanup (when
  the last tty reference is dropped) had other issues. In particular, we
  would end up unregistering the parent device before its child (the
  interface and usb device are unregistered when disconnect returns)
  resulting in broken uevent devpaths:
  
  KERNEL[794.849051] remove   
  /devices/pci:00/:00:1a.7/usb1/1-4/1-4.1/1-4.1:1.0 (usb)
  KERNEL[794.851649] remove   /devices/pci:00/:00:1a.7/usb1/1-4/1-4.1 
  (usb)
  KERNEL[795.115623] remove   /1-4.1:1.0/ttyUSB0/tty/ttyUSB0 (tty)
  KERNEL[795.117429] remove   /1-4.1:1.0/ttyUSB0 (usb-serial)
  
  This was reported here
  
  https://bugs.freedesktop.org/show_bug.cgi?id=20703
  
  and fixed by Alan Stern in 2d93148ab6 (USB: serial: fix lifetime and
  locking problems) by moving unregistration to disconnect.
  
  So we end up with an unregistered device still possibly referenced by
  tty instead, and I suspect we can't do much else than deal with any
  post-disconnect callbacks. [ These should be few, especially with my
  latest TTY-patches applied. ]
  
  Alan, do you see any way around this? It's not possible (or desirable)
  to pin the parent device (interface) until the last reference is
  dropped, is it?
 
 On the contrary, it is customary to pin data structures until the last 
 reference to them is gone.  That's what krefs are for.

I was referring to the usb device in the device hierarchy, which
apparently is not pinned despite the outstanding reference we have in
struct usb_serial.

There is an unconditional call to device_del in usb_disconnect which
unlinks the parent usb device from the device hierarchy resulting in the
broken devpaths above if we do not unregister the usb-serial port (and
tty device) in disconnect.

 On the other hand, the port private data was owned entirely by the
 serial sub-driver.  Neither the serial core nor the tty layer is able
 to use it meaningfully -- they don't even know what type of structure
 it is.
 
 Therefore freeing the structure when the port is removed should be 
 harmless -- unless the subdriver is called after the structure is 
 deallocated.

Which could happen (and is happening), unless we defer port unregister
until the last tty reference is dropped -- but then we get the broken
uevents.

 This means there still is one bug remaining.  In
 usb_serial_device_remove(), the call to tty_unregister_device() should
 occur _before_ the call to driver-port_remove(), not _after_.  Do you
 think changing the order cause any new problems?

Yes, Peter noticed that one too. Changing the order shouldn't cause any
new issues as far as I can see. I'll cook up a patch for this one as
well, but just to be clear: this is not directly related to the problem
discussed above as there may be outstanding tty references long after
both functions return (not that anyone has claimed anything else).

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

Re: [GIT PATCH] USB patches for 3.9-rc1

On Sat, Feb 23, 2013 at 01:02:11AM +0800, Lan Tianyu wrote:

   What can I do to debug this ?
 Can you attach the output of dmesg with CONFIG_USB_DEBUG?
  
   http://paste.fedoraproject.org/3620
  How aboutsudo lsusb -s 1:1 -v or 2:1?

(12:13:51:davej@t430s:~)$ sudo lsusb -s 1:1
Bus 001 Device 001: ID 1d6b:0002 Linux Foundation 2.0 root hub
(12:13:53:davej@t430s:~)$ sudo lsusb -s 2:1
Bus 002 Device 001: ID 1d6b:0002 Linux Foundation 2.0 root hub

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

Re: [GIT PATCH] USB patches for 3.9-rc1

2013-02-22 Thread Lan Tianyu


On 2013/2/23 1:14, Dave Jones wrote:

On Sat, Feb 23, 2013 at 01:02:11AM +0800, Lan Tianyu wrote:

What can I do to debug this ?
Can you attach the output of dmesg with CONFIG_USB_DEBUG?
   
http://paste.fedoraproject.org/3620
   How aboutsudo lsusb -s 1:1 -v or 2:1?

(12:13:51:davej@t430s:~)$ sudo lsusb -s 1:1
Bus 001 Device 001: ID 1d6b:0002 Linux Foundation 2.0 root hub
(12:13:53:davej@t430s:~)$ sudo lsusb -s 2:1
Bus 002 Device 001: ID 1d6b:0002 Linux Foundation 2.0 root hub


Oh. Sorry, I didn't say clearly.
Please run
sudo lsusb -s 1:1 -v
sudo lsusb -s 2:1 -v

--
Best Regards
Tianyu Lan
linux kernel enabling team
--
To unsubscribe from this list: send the line unsubscribe linux-usb in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [GIT PATCH] USB patches for 3.9-rc1

On Sat, Feb 23, 2013 at 01:17:42AM +0800, Lan Tianyu wrote:
  On 2013/2/23 1:14, Dave Jones wrote:
   On Sat, Feb 23, 2013 at 01:02:11AM +0800, Lan Tianyu wrote:
  
   What can I do to debug this ?
   Can you attach the output of dmesg with CONFIG_USB_DEBUG?
  
   http://paste.fedoraproject.org/3620
  How aboutsudo lsusb -s 1:1 -v or 2:1?
  
   (12:13:51:davej@t430s:~)$ sudo lsusb -s 1:1
   Bus 001 Device 001: ID 1d6b:0002 Linux Foundation 2.0 root hub
   (12:13:53:davej@t430s:~)$ sudo lsusb -s 2:1
   Bus 002 Device 001: ID 1d6b:0002 Linux Foundation 2.0 root hub
  
  Oh. Sorry, I didn't say clearly.
  Please run
  sudo lsusb -s 1:1 -v
  sudo lsusb -s 2:1 -v

(12:20:00:davej@t430s:~)$ sudo lsusb -s 1:1 -v | fpaste
Uploading (2.3KiB)...
http://paste.fedoraproject.org/3623
(12:20:11:davej@t430s:~)$ sudo lsusb -s 2:1 -v | fpaste
Uploading (2.3KiB)...
http://paste.fedoraproject.org/3624


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

Re: USB: serial: port lifetimes (was: Re: USB Ooops PL2303 when unplug while use (linux v3.7.3))

On Fri, 22 Feb 2013, Johan Hovold wrote:

   So we end up with an unregistered device still possibly referenced by
   tty instead, and I suspect we can't do much else than deal with any
   post-disconnect callbacks. [ These should be few, especially with my
   latest TTY-patches applied. ]
   
   Alan, do you see any way around this? It's not possible (or desirable)
   to pin the parent device (interface) until the last reference is
   dropped, is it?
  
  On the contrary, it is customary to pin data structures until the last 
  reference to them is gone.  That's what krefs are for.
 
 I was referring to the usb device in the device hierarchy, which
 apparently is not pinned despite the outstanding reference we have in
 struct usb_serial.

Are you talking about the usb_device or the usb_interface?  The
usb_serial structure _does_ pin the usb_device structure.  But it
doesn't pin the usb_interface.  I don't know why things were done this
way; maybe it's a mistake.

Anyway, keeping a pointer to a non-pinned data structure after
unregistration is okay, provided you know you will never dereference
the pointer.  If you don't know this in the case of the usb_interface,
pinning is acceptable -- depending on _how_ the usb_interface is
accessed.  For example, no URBs should be submitted for any of the
interface's endpoints.

 There is an unconditional call to device_del in usb_disconnect which
 unlinks the parent usb device from the device hierarchy resulting in the
 broken devpaths above if we do not unregister the usb-serial port (and
 tty device) in disconnect.

Sure.  But unregistering is different from deallocation.  It's not
clear what your point is.

  On the other hand, the port private data was owned entirely by the
  serial sub-driver.  Neither the serial core nor the tty layer is able
  to use it meaningfully -- they don't even know what type of structure
  it is.
  
  Therefore freeing the structure when the port is removed should be 
  harmless -- unless the subdriver is called after the structure is 
  deallocated.
 
 Which could happen (and is happening), unless we defer port unregister
 until the last tty reference is dropped -- but then we get the broken
 uevents.

Unregistration should not be deferred.  We mustn't have those broken 
devpaths.

  This means there still is one bug remaining.  In
  usb_serial_device_remove(), the call to tty_unregister_device() should
  occur _before_ the call to driver-port_remove(), not _after_.  Do you
  think changing the order cause any new problems?
 
 Yes, Peter noticed that one too. Changing the order shouldn't cause any
 new issues as far as I can see. I'll cook up a patch for this one as
 well, but just to be clear: this is not directly related to the problem
 discussed above as there may be outstanding tty references long after
 both functions return (not that anyone has claimed anything else).

This is related to the problem of the port's private data being
accessed after it is deallocated.  The only way that can happen is if
the tty layer calls the subdriver after the private data structure is
freed -- and you said above that this does happen.

But if change things so that the structure isn't freed until after the
port is unregistered from the tty layer, this would mean that the tty
layer is trying to do stuff to an unregistered port.  That would be a
bug in the tty layer.

I'm not saying such bugs don't exist.  However, if they do exist then 
the tty layer needs to be fixed, not the usb-serial layer.

Alan Stern

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

Re: USB: serial: port lifetimes (was: Re: USB Ooops PL2303 when unplug while use (linux v3.7.3))

2013-02-22 Thread Greg KH

On Fri, Feb 22, 2013 at 12:35:32PM -0500, Alan Stern wrote:
 On Fri, 22 Feb 2013, Johan Hovold wrote:
 
So we end up with an unregistered device still possibly referenced by
tty instead, and I suspect we can't do much else than deal with any
post-disconnect callbacks. [ These should be few, especially with my
latest TTY-patches applied. ]

Alan, do you see any way around this? It's not possible (or desirable)
to pin the parent device (interface) until the last reference is
dropped, is it?
   
   On the contrary, it is customary to pin data structures until the last 
   reference to them is gone.  That's what krefs are for.
  
  I was referring to the usb device in the device hierarchy, which
  apparently is not pinned despite the outstanding reference we have in
  struct usb_serial.
 
 Are you talking about the usb_device or the usb_interface?  The
 usb_serial structure _does_ pin the usb_device structure.  But it
 doesn't pin the usb_interface.  I don't know why things were done this
 way; maybe it's a mistake.

Ick, yeah, that was a mistake, probably been there since the very
beginning, sorry about that.

greg k-h
--
To unsubscribe from this list: send the line unsubscribe linux-usb in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: USB: serial: port lifetimes (was: Re: USB Ooops PL2303 when unplug while use (linux v3.7.3))

2013-02-22 Thread Peter Hurley

On Fri, 2013-02-22 at 12:35 -0500, Alan Stern wrote:
 On Fri, 22 Feb 2013, Johan Hovold wrote:
 
So we end up with an unregistered device still possibly referenced by
tty instead, and I suspect we can't do much else than deal with any
post-disconnect callbacks. [ These should be few, especially with my
latest TTY-patches applied. ]

Alan, do you see any way around this? It's not possible (or desirable)
to pin the parent device (interface) until the last reference is
dropped, is it?
   
   On the contrary, it is customary to pin data structures until the last 
   reference to them is gone.  That's what krefs are for.
  
  I was referring to the usb device in the device hierarchy, which
  apparently is not pinned despite the outstanding reference we have in
  struct usb_serial.
 
 Are you talking about the usb_device or the usb_interface?  The
 usb_serial structure _does_ pin the usb_device structure.  But it
 doesn't pin the usb_interface.  I don't know why things were done this
 way; maybe it's a mistake.
 
 Anyway, keeping a pointer to a non-pinned data structure after
 unregistration is okay, provided you know you will never dereference
 the pointer.  If you don't know this in the case of the usb_interface,
 pinning is acceptable -- depending on _how_ the usb_interface is
 accessed.  For example, no URBs should be submitted for any of the
 interface's endpoints.
 
  There is an unconditional call to device_del in usb_disconnect which
  unlinks the parent usb device from the device hierarchy resulting in the
  broken devpaths above if we do not unregister the usb-serial port (and
  tty device) in disconnect.
 
 Sure.  But unregistering is different from deallocation.  It's not
 clear what your point is.
 
   On the other hand, the port private data was owned entirely by the
   serial sub-driver.  Neither the serial core nor the tty layer is able
   to use it meaningfully -- they don't even know what type of structure
   it is.
   
   Therefore freeing the structure when the port is removed should be 
   harmless -- unless the subdriver is called after the structure is 
   deallocated.
  
  Which could happen (and is happening), unless we defer port unregister
  until the last tty reference is dropped -- but then we get the broken
  uevents.
 
 Unregistration should not be deferred.  We mustn't have those broken 
 devpaths.
 
   This means there still is one bug remaining.  In
   usb_serial_device_remove(), the call to tty_unregister_device() should
   occur _before_ the call to driver-port_remove(), not _after_.  Do you
   think changing the order cause any new problems?
  
  Yes, Peter noticed that one too. Changing the order shouldn't cause any
  new issues as far as I can see. I'll cook up a patch for this one as
  well, but just to be clear: this is not directly related to the problem
  discussed above as there may be outstanding tty references long after
  both functions return (not that anyone has claimed anything else).
 
 This is related to the problem of the port's private data being
 accessed after it is deallocated.  The only way that can happen is if
 the tty layer calls the subdriver after the private data structure is
 freed -- and you said above that this does happen.
 
 But if change things so that the structure isn't freed until after the
 port is unregistered from the tty layer, this would mean that the tty
 layer is trying to do stuff to an unregistered port.  That would be a
 bug in the tty layer.
 
 I'm not saying such bugs don't exist.  However, if they do exist then 
 the tty layer needs to be fixed, not the usb-serial layer.

ISTM the usb_serial_port private data should not be freed until
port_release().

If usb traffic isn't halted until port_release() ...

static void port_release(struct device *dev)
{
struct usb_serial_port *port = to_usb_serial_port(dev);
int i;

dev_dbg(dev, %s\n, __func__);

/*
 * Stop all the traffic before cancelling the work, so that
 * nobody will restart it by calling usb_serial_port_softint.
 */
   kill_traffic(port);
cancel_work_sync(port-work);

then what happens here if -port_remove() has already been called?

static void garmin_write_bulk_callback(struct urb *urb)
{
struct usb_serial_port *port = urb-context;

if (port) {
struct garmin_data *garmin_data_p =
usb_get_serial_port_data(port);

if (GARMIN_LAYERID_APPL == getLayerId(urb-transfer_buffer)) {

   if (garmin_data_p-mode == MODE_GARMIN_SERIAL) {
gsp_send_ack(garmin_data_p,
((__u8 *)urb-transfer_buffer)[4]);
}





--
To unsubscribe from this list: send the line unsubscribe linux-usb in
the body of a message to majord...@vger.kernel.org
More majordomo info at

Re: USB: serial: port lifetimes (was: Re: USB Ooops PL2303 when unplug while use (linux v3.7.3))

On Fri, 22 Feb 2013, Peter Hurley wrote:

 ISTM the usb_serial_port private data should not be freed until
 port_release().

I don't think that's right.  In general, a driver doesn't know what's 
going to happen to a device after the two are unbound from each other.  
Another driver may get bound to that same device, and may overwrite the 
old private-data pointer with its own new one.

The same reasoning applies here.  The serial subdriver isn't involved 
in the creation of the usb_serial_port structure, and therefore it 
shouldn't be involved in that structure's destruction.

 If usb traffic isn't halted until port_release() ...
 
 static void port_release(struct device *dev)
 {
   struct usb_serial_port *port = to_usb_serial_port(dev);
   int i;
 
   dev_dbg(dev, %s\n, __func__);
 
   /*
* Stop all the traffic before cancelling the work, so that
* nobody will restart it by calling usb_serial_port_softint.
*/
  kill_traffic(port);
   cancel_work_sync(port-work);
 
 then what happens here if -port_remove() has already been called?
 
 static void garmin_write_bulk_callback(struct urb *urb)
 {
   struct usb_serial_port *port = urb-context;
 
   if (port) {
   struct garmin_data *garmin_data_p =
   usb_get_serial_port_data(port);
 
   if (GARMIN_LAYERID_APPL == getLayerId(urb-transfer_buffer)) {
 
  if (garmin_data_p-mode == MODE_GARMIN_SERIAL) {
   gsp_send_ack(garmin_data_p,
   ((__u8 *)urb-transfer_buffer)[4]);
   }

Clearly this is a mistake.  Probably it is the result of historical
confusion between the overall USB device and the multiple serial ports
embodied within it.  At any rate, it is clear that all port-related USB
activity should be stopped when the port is unregistered.

Whether this can be carried out by the usb-serial core or needs help 
from the subdriver, I leave up to you guys.

Alan Stern

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

Re: [GIT PATCH] USB patches for 3.9-rc1

2013-02-22 Thread Fabio Baltieri

On Fri, Feb 22, 2013 at 03:59:54AM -0500, Dave Jones wrote:
 On Thu, Feb 21, 2013 at 10:40:10AM -0800, Greg KH wrote:
 
   USB patches for 3.9-rc1
   
   Here's the big USB merge for 3.9-rc1
   
   Nothing major, lots of gadget fixes, and of course, xhci stuff.
 
 I get no USB devices recognised when I insert them any more, which
 I think is pretty major.  I suspect it has something to do with this ?
 
   Lan Tianyu (12):
 usb: add runtime pm support for usb port device
 usb: add usb port auto power off mechanism
 
 It looks like every port on my laptop is powered down, as I can't
 even charge devices with it.

Hi Dave,

I have the same problem (and almost the same laptop, Thinkpad T430
here), all external USB ports without power - even the always-on one
:-).

The bug seems to be ACPI related, I bisected it down to this patch:

f95988d ACPI / scan: Treat power resources in a special way

In the dmesg I have some error like these:

ACPI: Power Resource [PUBS] (on)
...
pci :00:14.0: System wakeup disabled by ACPI

for the USB controllers in the broken kernel, there are some in your
dmesg too.  I'll try to come up with a fix for current mainline, but all
the acpi stuff is quite obscure to me and the patch does not revert
cleanly, maybe Rafael (in CC) has some idea!

Fabio

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

Re: [GIT PATCH] USB patches for 3.9-rc1

On Fri, Feb 22, 2013 at 10:51:58PM +0100, Fabio Baltieri wrote:
  On Fri, Feb 22, 2013 at 03:59:54AM -0500, Dave Jones wrote:
   On Thu, Feb 21, 2013 at 10:40:10AM -0800, Greg KH wrote:
   
   It looks like every port on my laptop is powered down, as I can't
   even charge devices with it.
  
  I have the same problem (and almost the same laptop, Thinkpad T430
  here), all external USB ports without power - even the always-on one
  :-).
  
  The bug seems to be ACPI related, I bisected it down to this patch:
  
  f95988d ACPI / scan: Treat power resources in a special way
  
  In the dmesg I have some error like these:
  
  ACPI: Power Resource [PUBS] (on)
  ...
  pci :00:14.0: System wakeup disabled by ACPI
  
  for the USB controllers in the broken kernel, there are some in your
  dmesg too.  I'll try to come up with a fix for current mainline, but all
  the acpi stuff is quite obscure to me and the patch does not revert
  cleanly, maybe Rafael (in CC) has some idea!

Good find. I see the same thing.

[0.930283] ACPI _OSC control for PCIe not granted, disabling ASPM
[0.933527] pci :00:14.0: System wakeup disabled by ACPI
[0.935982] pci :00:19.0: System wakeup disabled by ACPI
[0.937898] pci :00:1a.0: System wakeup disabled by ACPI
[0.939835] pci :00:1b.0: System wakeup disabled by ACPI
[0.940700] ACPI: PCI Interrupt Routing Table [\_SB_.PCI0.EXP1._PRT]
[0.942195] ACPI: PCI Interrupt Routing Table [\_SB_.PCI0.EXP2._PRT]
[0.943737] ACPI: PCI Interrupt Routing Table [\_SB_.PCI0.EXP3._PRT]
[0.944564] pci :00:1c.2: System wakeup disabled by ACPI
[0.966491] pci :00:1d.0: System wakeup disabled by ACPI

I must have pulled in the acpi bits the same time as the usb pull,
because I don't recall seeing this problem before last night.

Dave

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

Re: [GIT PATCH] USB patches for 3.9-rc1

2013-02-22 Thread Fabio Baltieri

On Fri, Feb 22, 2013 at 05:23:04PM -0500, Dave Jones wrote:
 On Fri, Feb 22, 2013 at 10:51:58PM +0100, Fabio Baltieri wrote:
   On Fri, Feb 22, 2013 at 03:59:54AM -0500, Dave Jones wrote:
On Thu, Feb 21, 2013 at 10:40:10AM -0800, Greg KH wrote:

It looks like every port on my laptop is powered down, as I can't
even charge devices with it.
   
   I have the same problem (and almost the same laptop, Thinkpad T430
   here), all external USB ports without power - even the always-on one
   :-).
   
   The bug seems to be ACPI related, I bisected it down to this patch:
   
   f95988d ACPI / scan: Treat power resources in a special way
   
   In the dmesg I have some error like these:
   
   ACPI: Power Resource [PUBS] (on)
   ...
   pci :00:14.0: System wakeup disabled by ACPI
   
   for the USB controllers in the broken kernel, there are some in your
   dmesg too.  I'll try to come up with a fix for current mainline, but all
   the acpi stuff is quite obscure to me and the patch does not revert
   cleanly, maybe Rafael (in CC) has some idea!
 
 Good find. I see the same thing.
 
 [0.930283] ACPI _OSC control for PCIe not granted, disabling ASPM
 [0.933527] pci :00:14.0: System wakeup disabled by ACPI
 [0.935982] pci :00:19.0: System wakeup disabled by ACPI
 [0.937898] pci :00:1a.0: System wakeup disabled by ACPI
 [0.939835] pci :00:1b.0: System wakeup disabled by ACPI
 [0.940700] ACPI: PCI Interrupt Routing Table [\_SB_.PCI0.EXP1._PRT]
 [0.942195] ACPI: PCI Interrupt Routing Table [\_SB_.PCI0.EXP2._PRT]
 [0.943737] ACPI: PCI Interrupt Routing Table [\_SB_.PCI0.EXP3._PRT]
 [0.944564] pci :00:1c.2: System wakeup disabled by ACPI
 [0.966491] pci :00:1d.0: System wakeup disabled by ACPI
 
 I must have pulled in the acpi bits the same time as the usb pull,
 because I don't recall seeing this problem before last night.

Definitely.

Well, this did the trick in my case:

--- 8 ---
diff --git a/drivers/acpi/power.c b/drivers/acpi/power.c
index b820528..54175a0 100644
--- a/drivers/acpi/power.c
+++ b/drivers/acpi/power.c
@@ -795,7 +795,7 @@ int acpi_add_power_resource(acpi_handle handle)
int state, result = -ENODEV;
 
acpi_bus_get_device(handle, device);
-   if (device)
+   if (!device)
return 0;
 
resource = kzalloc(sizeof(*resource), GFP_KERNEL);
--- 8 ---

But I guess it's working as a coincidence and something else is wrong -
I'll not even try to make a patch out of it and will leave the dirty
work to the ACPI guys instead.

Fabio

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

Re: [GIT PATCH] USB patches for 3.9-rc1

On Friday, February 22, 2013 10:51:58 PM Fabio Baltieri wrote:
 On Fri, Feb 22, 2013 at 03:59:54AM -0500, Dave Jones wrote:
  On Thu, Feb 21, 2013 at 10:40:10AM -0800, Greg KH wrote:
  
USB patches for 3.9-rc1

Here's the big USB merge for 3.9-rc1

Nothing major, lots of gadget fixes, and of course, xhci stuff.
  
  I get no USB devices recognised when I insert them any more, which
  I think is pretty major.  I suspect it has something to do with this ?
  
Lan Tianyu (12):
  usb: add runtime pm support for usb port device
  usb: add usb port auto power off mechanism
  
  It looks like every port on my laptop is powered down, as I can't
  even charge devices with it.
 
 Hi Dave,
 
 I have the same problem (and almost the same laptop, Thinkpad T430
 here), all external USB ports without power - even the always-on one
 :-).
 
 The bug seems to be ACPI related, I bisected it down to this patch:
 
 f95988d ACPI / scan: Treat power resources in a special way
 
 In the dmesg I have some error like these:
 
 ACPI: Power Resource [PUBS] (on)
 ...
 pci :00:14.0: System wakeup disabled by ACPI

They aren't errors in fact, just info messages.

 for the USB controllers in the broken kernel, there are some in your
 dmesg too.  I'll try to come up with a fix for current mainline, but all
 the acpi stuff is quite obscure to me and the patch does not revert
 cleanly, maybe Rafael (in CC) has some idea!

It won't revert, there's more stuff on top of it.  And it is a fix, so
reverting it is not really a good idea anyway.

I suspect that the USB controllers are put into low-power states at one
point and they don't generate wakeup events.  I'm not sure, though, why this
is related to power resources.

Can you please send a dmesg boot log and the output of acpidump from the
affected machine?

Rafael


-- 
I speak only for myself.
Rafael J. Wysocki, Intel Open Source Technology Center.
--
To unsubscribe from this list: send the line unsubscribe linux-usb in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [GIT PATCH] USB patches for 3.9-rc1

On Friday, February 22, 2013 05:23:04 PM Dave Jones wrote:
 On Fri, Feb 22, 2013 at 10:51:58PM +0100, Fabio Baltieri wrote:
   On Fri, Feb 22, 2013 at 03:59:54AM -0500, Dave Jones wrote:
On Thu, Feb 21, 2013 at 10:40:10AM -0800, Greg KH wrote:

It looks like every port on my laptop is powered down, as I can't
even charge devices with it.
   
   I have the same problem (and almost the same laptop, Thinkpad T430
   here), all external USB ports without power - even the always-on one
   :-).
   
   The bug seems to be ACPI related, I bisected it down to this patch:
   
   f95988d ACPI / scan: Treat power resources in a special way
   
   In the dmesg I have some error like these:
   
   ACPI: Power Resource [PUBS] (on)
   ...
   pci :00:14.0: System wakeup disabled by ACPI
   
   for the USB controllers in the broken kernel, there are some in your
   dmesg too.  I'll try to come up with a fix for current mainline, but all
   the acpi stuff is quite obscure to me and the patch does not revert
   cleanly, maybe Rafael (in CC) has some idea!
 
 Good find. I see the same thing.
 
 [0.930283] ACPI _OSC control for PCIe not granted, disabling ASPM
 [0.933527] pci :00:14.0: System wakeup disabled by ACPI
 [0.935982] pci :00:19.0: System wakeup disabled by ACPI
 [0.937898] pci :00:1a.0: System wakeup disabled by ACPI
 [0.939835] pci :00:1b.0: System wakeup disabled by ACPI
 [0.940700] ACPI: PCI Interrupt Routing Table [\_SB_.PCI0.EXP1._PRT]
 [0.942195] ACPI: PCI Interrupt Routing Table [\_SB_.PCI0.EXP2._PRT]
 [0.943737] ACPI: PCI Interrupt Routing Table [\_SB_.PCI0.EXP3._PRT]
 [0.944564] pci :00:1c.2: System wakeup disabled by ACPI
 [0.966491] pci :00:1d.0: System wakeup disabled by ACPI
 
 I must have pulled in the acpi bits the same time as the usb pull,
 because I don't recall seeing this problem before last night.

Can you please check if the problem is still there in the master branch of
the linux-pm.git tree alone?

Rafael


-- 
I speak only for myself.
Rafael J. Wysocki, Intel Open Source Technology Center.
--
To unsubscribe from this list: send the line unsubscribe linux-usb in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [GIT PATCH] USB patches for 3.9-rc1

On Saturday, February 23, 2013 01:10:55 AM Fabio Baltieri wrote:
 On Fri, Feb 22, 2013 at 05:23:04PM -0500, Dave Jones wrote:
  On Fri, Feb 22, 2013 at 10:51:58PM +0100, Fabio Baltieri wrote:
On Fri, Feb 22, 2013 at 03:59:54AM -0500, Dave Jones wrote:
 On Thu, Feb 21, 2013 at 10:40:10AM -0800, Greg KH wrote:
 
 It looks like every port on my laptop is powered down, as I can't
 even charge devices with it.

I have the same problem (and almost the same laptop, Thinkpad T430
here), all external USB ports without power - even the always-on one
:-).

The bug seems to be ACPI related, I bisected it down to this patch:

f95988d ACPI / scan: Treat power resources in a special way

In the dmesg I have some error like these:

ACPI: Power Resource [PUBS] (on)
...
pci :00:14.0: System wakeup disabled by ACPI

for the USB controllers in the broken kernel, there are some in your
dmesg too.  I'll try to come up with a fix for current mainline, but all
the acpi stuff is quite obscure to me and the patch does not revert
cleanly, maybe Rafael (in CC) has some idea!
  
  Good find. I see the same thing.
  
  [0.930283] ACPI _OSC control for PCIe not granted, disabling ASPM
  [0.933527] pci :00:14.0: System wakeup disabled by ACPI
  [0.935982] pci :00:19.0: System wakeup disabled by ACPI
  [0.937898] pci :00:1a.0: System wakeup disabled by ACPI
  [0.939835] pci :00:1b.0: System wakeup disabled by ACPI
  [0.940700] ACPI: PCI Interrupt Routing Table [\_SB_.PCI0.EXP1._PRT]
  [0.942195] ACPI: PCI Interrupt Routing Table [\_SB_.PCI0.EXP2._PRT]
  [0.943737] ACPI: PCI Interrupt Routing Table [\_SB_.PCI0.EXP3._PRT]
  [0.944564] pci :00:1c.2: System wakeup disabled by ACPI
  [0.966491] pci :00:1d.0: System wakeup disabled by ACPI
  
  I must have pulled in the acpi bits the same time as the usb pull,
  because I don't recall seeing this problem before last night.
 
 Definitely.
 
 Well, this did the trick in my case:
 
 --- 8 ---
 diff --git a/drivers/acpi/power.c b/drivers/acpi/power.c
 index b820528..54175a0 100644
 --- a/drivers/acpi/power.c
 +++ b/drivers/acpi/power.c
 @@ -795,7 +795,7 @@ int acpi_add_power_resource(acpi_handle handle)
 int state, result = -ENODEV;
  
 acpi_bus_get_device(handle, device);
 -   if (device)
 +   if (!device)
 return 0;
  
 resource = kzalloc(sizeof(*resource), GFP_KERNEL);
 --- 8 ---
 
 But I guess it's working as a coincidence and something else is wrong -
 I'll not even try to make a patch out of it and will leave the dirty
 work to the ACPI guys instead.

Well, this patch effectively disables the handling of power resources on your
machine entirely.  The effect of which is probably that the power resources
can't be turned off for the USB controllers, so they don't go into D3cold.

And the bisection found a commit that restores the handling of power resources
for you, which is not entirely surprising, but the root cause is somewhere else
most likely.

The new sysfs interface for power resources control may be helpful here.  You
need to use the Linus' current for it to work, though.

Can you please do

$ find /sys/devices/LNXSYSTM:00/ -name power_state -print -exec cat {} \;

$ find /sys/devices/LNXSYSTM:00/ -name real_power_state -print -exec cat {} \;

$ find /sys/devices/LNXSYSTM:00/ -name power_resources_D\* -print -exec ls {} \;

$ find /sys/devices/LNXSYSTM:00/ -name resource_in_use -print -exec cat {} \;

and send the output?

Rafael


-- 
I speak only for myself.
Rafael J. Wysocki, Intel Open Source Technology Center.
--
To unsubscribe from this list: send the line unsubscribe linux-usb in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [GIT PATCH] USB patches for 3.9-rc1

2013-02-22 Thread Linus Torvalds

On Fri, Feb 22, 2013 at 4:19 PM, Rafael J. Wysocki r...@sisk.pl wrote:

 It won't revert, there's more stuff on top of it.  And it is a fix, so
 reverting it is not really a good idea anyway.

Rafael, please don't *ever* write that crap again.

We revert stuff whether it fixed something else or not. The rule is
NO REGRESSIONS. It doesn't matter one whit if something fixes
something else or not - if it breaks an old case, it gets reverted.

Seriously. Why do I even have to mention this? Why do I have to
explain this to somebody pretty much *every* f*cking merge window?

This is not a new rule.

And btw, the *reason* for that rule becoming such a hard rule was
pretty much exactly suspend/resume and ACPI. Exactly because we used
to have those infinite let's fix one thing and break another dances.
So you should be well acquainted with the rule, and I'm surprised to
hear that kind of utter garbage from you in particular.

There is no excuse for regressions, and it is a fix is actually the
_least_ valid of all reasons.

A commit that causes a regression is - by definition - not a fix. So
please don't *ever* say something that stupid again.

Things that used to work are simply a million times more important
than things that historically didn't work.

So this had better get fixed asap, and I need to feel like people are
working on it. Otherwise we start reverting.

And no amount but it's a fix matters one whit. In fact, it just
makes me feel like I need to start reverting early, because the
maintainer doesn't seem to understand how serious a regression is.

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

Re: [GIT PATCH] USB patches for 3.9-rc1

On Friday, February 22, 2013 04:30:25 PM Linus Torvalds wrote:
 On Fri, Feb 22, 2013 at 4:19 PM, Rafael J. Wysocki r...@sisk.pl wrote:
 
  It won't revert, there's more stuff on top of it.  And it is a fix, so
  reverting it is not really a good idea anyway.
 
 Rafael, please don't *ever* write that crap again.
 
 We revert stuff whether it fixed something else or not. The rule is
 NO REGRESSIONS. It doesn't matter one whit if something fixes
 something else or not - if it breaks an old case, it gets reverted.
 
 Seriously. Why do I even have to mention this? Why do I have to
 explain this to somebody pretty much *every* f*cking merge window?

Well, sorry.  I shouldn't have said that.

The problem is, though, that even if bisection turns up something, it doesn't
automatically mean that this particular commit is the one that caused the
problem to happen in the first place.

And in this particular case bisection turns up a commit that enables something
that didn't work on that particular machine for some time.  It used to work,
then it stopped working and that commit made it work again.  And the fact that
it made it work again caused something different to trigger the result of which
is the observed breakage.

I'm obviously going to fix it, because it is a serious problem, but the commit
in question is not the root cause of it in my opinion (as I wrote to Fabio in
another message).

 This is not a new rule.
 
 And btw, the *reason* for that rule becoming such a hard rule was
 pretty much exactly suspend/resume and ACPI. Exactly because we used
 to have those infinite let's fix one thing and break another dances.
 So you should be well acquainted with the rule, and I'm surprised to
 hear that kind of utter garbage from you in particular.
 
 There is no excuse for regressions, and it is a fix is actually the
 _least_ valid of all reasons.
 
 A commit that causes a regression is - by definition - not a fix. So
 please don't *ever* say something that stupid again.
 
 Things that used to work are simply a million times more important
 than things that historically didn't work.
 
 So this had better get fixed asap, and I need to feel like people are
 working on it. Otherwise we start reverting.
 
 And no amount but it's a fix matters one whit. In fact, it just
 makes me feel like I need to start reverting early, because the
 maintainer doesn't seem to understand how serious a regression is.

OK, OK.

Please let me understand what the problem is first.

Thanks,
Rafael


-- 
I speak only for myself.
Rafael J. Wysocki, Intel Open Source Technology Center.
--
To unsubscribe from this list: send the line unsubscribe linux-usb in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [GIT PATCH] USB patches for 3.9-rc1

On Friday, February 22, 2013 10:51:58 PM Fabio Baltieri wrote:
 On Fri, Feb 22, 2013 at 03:59:54AM -0500, Dave Jones wrote:
  On Thu, Feb 21, 2013 at 10:40:10AM -0800, Greg KH wrote:
  
USB patches for 3.9-rc1

Here's the big USB merge for 3.9-rc1

Nothing major, lots of gadget fixes, and of course, xhci stuff.
  
  I get no USB devices recognised when I insert them any more, which
  I think is pretty major.  I suspect it has something to do with this ?
  
Lan Tianyu (12):
  usb: add runtime pm support for usb port device
  usb: add usb port auto power off mechanism
  
  It looks like every port on my laptop is powered down, as I can't
  even charge devices with it.
 
 Hi Dave,
 
 I have the same problem (and almost the same laptop, Thinkpad T430
 here), all external USB ports without power - even the always-on one
 :-).
 
 The bug seems to be ACPI related, I bisected it down to this patch:
 
 f95988d ACPI / scan: Treat power resources in a special way
 
 In the dmesg I have some error like these:
 
 ACPI: Power Resource [PUBS] (on)
 ...
 pci :00:14.0: System wakeup disabled by ACPI
 
 for the USB controllers in the broken kernel, there are some in your
 dmesg too.  I'll try to come up with a fix for current mainline, but all
 the acpi stuff is quite obscure to me and the patch does not revert
 cleanly, maybe Rafael (in CC) has some idea!

May I see the bisection log, BTW?

Rafael


-- 
I speak only for myself.
Rafael J. Wysocki, Intel Open Source Technology Center.
--
To unsubscribe from this list: send the line unsubscribe linux-usb in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [GIT PATCH] USB patches for 3.9-rc1

2013-02-22 Thread Linus Torvalds

On Fri, Feb 22, 2013 at 4:48 PM, Rafael J. Wysocki r...@sisk.pl wrote:

 The problem is, though, that even if bisection turns up something, it doesn't
 automatically mean that this particular commit is the one that caused the
 problem to happen in the first place.

No, I agree. I just react *very* strongly when somebody starts arguing
against reverting.

The fact that the commit that was bisected fixes another bug in a
previous commit does in fact just imply that that *previous* commit
should perhaps be reverted. Of course, I see that that is the first in
the whole series, so I understand the pain, but even so..

And looking at that commit that makes power resources be treated
specially, that looks completely bogus. It's not what the old code did
either, and it seems to be a total hack for the breakage that just
happens to fix that one machine for purely random reasons, as far as I
can tell. The ACPI_BUS_TYPE_POWER case always had match=1 before, no?

The thing that looks to have been dropped somewhere is that

  .acpi_op_start = !!context,

which first became

  device-add_type = context ? ACPI_BUS_ADD_START : ACPI_BUS_ADD_MATCH;

and then somehow that wasn't needed any more, so it became just an unconditional

  device-add_type = ACPI_BUS_ADD_START;

for unexplained reasons (the commit message says that the argument
context wasn't used, and renames it not_used to reinforce the point,
but doesn't explain why it can remove the use that was there).

That unconditional add_type = ACPI_BUS_ADD_START then later becomes
flags.match_driver = true, which is where the whole match_driver
thing comes in.

Anyway, I don't see how magically it became about ACPI_BUS_TYPE_POWER.
The code is not exactly obvious, though, so whatever. But that
context is suddenly not used commit (e3863094c6f9: ACPI: Drop the
second argument of acpi_bus_scan()) looks odd.

Maybe context was effectively always non-NULL, but it *looks* like
that series of commits is intended to have no semantic changes, and
that's the one that looked very dubious to me.

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

Re: [GIT PATCH] USB patches for 3.9-rc1

2013-02-22 Thread Fabio Baltieri

On Sat, Feb 23, 2013 at 01:35:26AM +0100, Rafael J. Wysocki wrote:
 On Saturday, February 23, 2013 01:10:55 AM Fabio Baltieri wrote:
  Well, this did the trick in my case:
  
  --- 8 ---
  diff --git a/drivers/acpi/power.c b/drivers/acpi/power.c
  index b820528..54175a0 100644
  --- a/drivers/acpi/power.c
  +++ b/drivers/acpi/power.c
  @@ -795,7 +795,7 @@ int acpi_add_power_resource(acpi_handle handle)
  int state, result = -ENODEV;
   
  acpi_bus_get_device(handle, device);
  -   if (device)
  +   if (!device)
  return 0;
   
  resource = kzalloc(sizeof(*resource), GFP_KERNEL);
  --- 8 ---
  
  But I guess it's working as a coincidence and something else is wrong -
  I'll not even try to make a patch out of it and will leave the dirty
  work to the ACPI guys instead.
 
 Well, this patch effectively disables the handling of power resources on your
 machine entirely.  The effect of which is probably that the power resources
 can't be turned off for the USB controllers, so they don't go into D3cold.

Ok, as I wrote, I suspected this was just shutting off something and not
solving the real problem.

So, I'll try to recap all the threads here:

 And the bisection found a commit that restores the handling of power resources
 for you, which is not entirely surprising, but the root cause is somewhere 
 else
 most likely.

Indeed.

 The new sysfs interface for power resources control may be helpful here.  You
 need to use the Linus' current for it to work, though.
 
 Can you please do
 
 $ find /sys/devices/LNXSYSTM:00/ -name power_state -print -exec cat {} \;
 $ find /sys/devices/LNXSYSTM:00/ -name real_power_state -print -exec cat {} \;
 $ find /sys/devices/LNXSYSTM:00/ -name power_resources_D\* -print -exec ls {} 
 \;
 $ find /sys/devices/LNXSYSTM:00/ -name resource_in_use -print -exec cat {} \;
 and send the output?

With the acpi_add_power_resource disabled all power_state read D0,
other attributes are not generated.

With a plain kernel that's the output:

$ find /sys/devices/LNXSYSTM:00/ -name power_state -print -exec cat {} \;
/sys/devices/LNXSYSTM:00/LNXSYBUS:00/PNP0A08:00/device:09/LNXVIDEO:01/power_state
D0
/sys/devices/LNXSYSTM:00/LNXSYBUS:00/PNP0A08:00/device:29/power_state
D0
/sys/devices/LNXSYSTM:00/LNXSYBUS:00/PNP0A08:00/device:34/power_state
D0
/sys/devices/LNXSYSTM:00/LNXSYBUS:00/PNP0A08:00/device:1f/power_state
D0
/sys/devices/LNXSYSTM:00/LNXSYBUS:00/PNP0A08:00/LNXVIDEO:00/power_state
D0

$ find /sys/devices/LNXSYSTM:00/ -name real_power_state -print -exec cat {} \;
/sys/devices/LNXSYSTM:00/LNXSYBUS:00/PNP0A08:00/device:29/real_power_state
D3cold
/sys/devices/LNXSYSTM:00/LNXSYBUS:00/PNP0A08:00/device:34/real_power_state
D3cold
/sys/devices/LNXSYSTM:00/LNXSYBUS:00/PNP0A08:00/device:1f/real_power_state
D3cold

$ find /sys/devices/LNXSYSTM:00/ -name power_resources_D\* -print -exec ls {} \;
/sys/devices/LNXSYSTM:00/LNXSYBUS:00/PNP0A08:00/device:29/power_resources_D0
LNXPOWER:00
/sys/devices/LNXSYSTM:00/LNXSYBUS:00/PNP0A08:00/device:29/power_resources_D1
LNXPOWER:00
/sys/devices/LNXSYSTM:00/LNXSYBUS:00/PNP0A08:00/device:29/power_resources_D2
LNXPOWER:00
/sys/devices/LNXSYSTM:00/LNXSYBUS:00/PNP0A08:00/device:34/power_resources_D0
LNXPOWER:00
/sys/devices/LNXSYSTM:00/LNXSYBUS:00/PNP0A08:00/device:34/power_resources_D1
LNXPOWER:00
/sys/devices/LNXSYSTM:00/LNXSYBUS:00/PNP0A08:00/device:34/power_resources_D2
LNXPOWER:00
/sys/devices/LNXSYSTM:00/LNXSYBUS:00/PNP0A08:00/device:1f/power_resources_D0
LNXPOWER:00
/sys/devices/LNXSYSTM:00/LNXSYBUS:00/PNP0A08:00/device:1f/power_resources_D1
LNXPOWER:00
/sys/devices/LNXSYSTM:00/LNXSYBUS:00/PNP0A08:00/device:1f/power_resources_D2
LNXPOWER:00

$ find /sys/devices/LNXSYSTM:00/ -name resource_in_use -print -exec cat {} \;
/sys/devices/LNXSYSTM:00/LNXSYBUS:00/PNP0A08:00/device:08/PNP0C09:00/LNXPOWER:00/resource_in_use
0

 Can you please check if the problem is still there in the master
 branch of the linux-pm.git tree alone?

Not sure if this was for me or Dave, anyway linux-pm.git master
currently points as:

10baf04 Merge branch 'release' of 
git://git.kernel.org/pub/scm/linux/kernel/git/lenb/linux

and the problem is still there.

 May I see the bisection log, BTW?

Sure, here it is:

git bisect start
# bad: [8793422fd9ac5037f5047f80473007301df3689f] Merge tag 'pm+acpi-3.9-rc1' 
of git://git.kernel.org/pub/scm/linux/kernel/git/rafael/linux-pm
git bisect bad 8793422fd9ac5037f5047f80473007301df3689f
# good: [19f949f52599ba7c3f67a5897ac6be14bfcb1200] Linux 3.8
git bisect good 19f949f52599ba7c3f67a5897ac6be14bfcb1200
# good: [8909ff652ddfc83ecdf450f96629c25489d88f77] Merge tag 'regulator-3.9' of 
git://git.kernel.org/pub/scm/linux/kernel/git/broonie/regulator
git bisect good 8909ff652ddfc83ecdf450f96629c25489d88f77
# good: [3aad3f03b2b6d2d977b985c49274cdb78a1593e5] Merge tag 'spi-for-linus' of 
git://git.secretlab.ca/git/linux
git bisect good 3aad3f03b2b6d2d977b985c49274cdb78a1593e5
# bad:

Re: [GIT PATCH] USB patches for 3.9-rc1

On Friday, February 22, 2013 05:10:43 PM Linus Torvalds wrote:
 On Fri, Feb 22, 2013 at 4:48 PM, Rafael J. Wysocki r...@sisk.pl wrote:
 
  The problem is, though, that even if bisection turns up something, it 
  doesn't
  automatically mean that this particular commit is the one that caused the
  problem to happen in the first place.
 
 No, I agree. I just react *very* strongly when somebody starts arguing
 against reverting.
 
 The fact that the commit that was bisected fixes another bug in a
 previous commit does in fact just imply that that *previous* commit
 should perhaps be reverted. Of course, I see that that is the first in
 the whole series, so I understand the pain, but even so..
 
 And looking at that commit that makes power resources be treated
 specially, that looks completely bogus. It's not what the old code did
 either, and it seems to be a total hack for the breakage that just
 happens to fix that one machine for purely random reasons, as far as I
 can tell. The ACPI_BUS_TYPE_POWER case always had match=1 before, no?

It had, when it was called from acpi_bus_add_power_resource(), but that
was not the only way power resources could be added.  That function was only
called when we found a device using power resources and we needed to add them
before adding that device (if they were not present already).

The second way we could add power resources was during the normal namespace
walk and that case was broken.

 The thing that looks to have been dropped somewhere is that
 
   .acpi_op_start = !!context,
 
 which first became
 
   device-add_type = context ? ACPI_BUS_ADD_START : ACPI_BUS_ADD_MATCH;
 
 and then somehow that wasn't needed any more, so it became just an 
 unconditional
 
   device-add_type = ACPI_BUS_ADD_START;
 
 for unexplained reasons (the commit message says that the argument
 context wasn't used, and renames it not_used to reinforce the point,
 but doesn't explain why it can remove the use that was there).
 
 That unconditional add_type = ACPI_BUS_ADD_START then later becomes
 flags.match_driver = true, which is where the whole match_driver
 thing comes in.
 
 Anyway, I don't see how magically it became about ACPI_BUS_TYPE_POWER.

If we added a power resource with acpi_add_single_object(..., false) from
acpi_bus_check_add(), later acpi_bus_add_power_resource() saw that it
was already present and didn't add it either.  The power resource was there at
that point, but it didn't have a driver (which only would be probed in the
second pass, after all ACPI devices had been registered).  The driver was
needed, though, for the initialization of the power management of devices
depending on that power resource.

The fact that it was missing caused power management of devices depending on
that power resource not to be initialized correctly and changing their power
states didn't really work going forward.

That's why the Fabio's bisection turned up that commit, BTW (the changing
of USB controllers' power states didn't really work before it on his machine).

I put ACPI_BUS_TYPE_POWER in there, because power resources always needed
'true' in the last arg of acpi_add_single_object() and they were the only type
needing it.

 The code is not exactly obvious, though, so whatever. But that
 context is suddenly not used commit (e3863094c6f9: ACPI: Drop the
 second argument of acpi_bus_scan()) looks odd.
 
 Maybe context was effectively always non-NULL, but it *looks* like
 that series of commits is intended to have no semantic changes, and
 that's the one that looked very dubious to me.

While I might make a mistake in it, I seriously doubt that it is the root
cause of the problem at hand.

It surely is related to PCI power management and to the fact that PCI devices
can go into D3cold at run time now.  This isn't an old change and there were
problems with it.  The known problems were fixed, but there might be more to
uncover and it looks like we've just run into a new one. :-(

So I really need to know what's going on before deciding how to fix that.

Thanks,
Rafael


-- 
I speak only for myself.
Rafael J. Wysocki, Intel Open Source Technology Center.
--
To unsubscribe from this list: send the line unsubscribe linux-usb in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [GIT PATCH] USB patches for 3.9-rc1