2012-05-16 22:46:03

by Sarah Sharp

[permalink] [raw]
Subject: [RFC 13/13] USB: Disable hub-initiated LPM for comms devices.

[Resending with a smaller Cc list]

Hub-initiated LPM is not good for USB communications devices. Comms
devices should be able to tell when their link can go into a lower power
state, because they know when an incoming transmission is finished.
Ideally, these devices would slam their links into a lower power state,
using the device-initiated LPM, after finishing the last packet of their
data transfer.

If we enable the idle timeouts for the parent hubs to enable
hub-initiated LPM, we will get a lot of useless LPM packets on the bus
as the devices reject LPM transitions when they're in the middle of
receiving data. Worse, some devices might blindly accept the
hub-initiated LPM and power down their radios while they're in the
middle of receiving a transmission.

The Intel Windows folks are disabling hub-initiated LPM for all USB
communications devices under a xHCI USB 3.0 host. In order to keep
the Linux behavior as close as possible to Windows, we need to do the
same in Linux.

Set the disable_hub_initiated_lpm flag for for all USB communications
drivers. I know there aren't currently any USB 3.0 devices that
implement these class specifications, but we should be ready if they do.

Signed-off-by: Sarah Sharp <[email protected]>
Cc: Marcel Holtmann <[email protected]>
Cc: Gustavo Padovan <[email protected]>
Cc: Johan Hedberg <[email protected]>
Cc: Hansjoerg Lipp <[email protected]>
Cc: Tilman Schmidt <[email protected]>
Cc: Karsten Keil <[email protected]>
Cc: Oliver Neukum <[email protected]>
Cc: Peter Korsgaard <[email protected]>
Cc: Jan Dumon <[email protected]>
Cc: Petko Manolov <[email protected]>
Cc: Steve Glendinning <[email protected]>
Cc: "John W. Linville" <[email protected]>
Cc: Kalle Valo <[email protected]>
Cc: "Luis R. Rodriguez" <[email protected]>
Cc: Jouni Malinen <[email protected]>
Cc: Vasanthakumar Thiagarajan <[email protected]>
Cc: Senthil Balasubramanian <[email protected]>
Cc: Christian Lamparter <[email protected]>
Cc: Brett Rudley <[email protected]>
Cc: Roland Vossen <[email protected]>
Cc: Arend van Spriel <[email protected]>
Cc: "Franky (Zhenhui) Lin" <[email protected]>
Cc: Kan Yan <[email protected]>
Cc: Dan Williams <[email protected]>
Cc: Jussi Kivilinna <[email protected]>
Cc: Ivo van Doorn <[email protected]>
Cc: Gertjan van Wingerde <[email protected]>
Cc: Helmut Schaa <[email protected]>
Cc: Herton Ronaldo Krzesinski <[email protected]>
Cc: Hin-Tak Leung <[email protected]>
Cc: Larry Finger <[email protected]>
Cc: Chaoming Li <[email protected]>
Cc: Daniel Drake <[email protected]>
Cc: Ulrich Kunitz <[email protected]>
Cc: [email protected]
Cc: [email protected]
Cc: [email protected]
Cc: [email protected]
Cc: [email protected]
Cc: [email protected]
Cc: [email protected]
Cc: [email protected]
---
drivers/bluetooth/ath3k.c | 1 +
drivers/bluetooth/bcm203x.c | 1 +
drivers/bluetooth/bfusb.c | 1 +
drivers/bluetooth/bpa10x.c | 1 +
drivers/bluetooth/btusb.c | 1 +
drivers/isdn/gigaset/bas-gigaset.c | 1 +
drivers/isdn/gigaset/usb-gigaset.c | 1 +
drivers/isdn/hardware/mISDN/hfcsusb.c | 1 +
drivers/isdn/hisax/hfc_usb.c | 1 +
drivers/isdn/hisax/st5481_init.c | 1 +
drivers/net/usb/asix.c | 1 +
drivers/net/usb/catc.c | 1 +
drivers/net/usb/cdc-phonet.c | 1 +
drivers/net/usb/cdc_eem.c | 1 +
drivers/net/usb/cdc_ether.c | 1 +
drivers/net/usb/cdc_ncm.c | 1 +
drivers/net/usb/cdc_subset.c | 1 +
drivers/net/usb/cx82310_eth.c | 1 +
drivers/net/usb/dm9601.c | 1 +
drivers/net/usb/gl620a.c | 1 +
drivers/net/usb/hso.c | 1 +
drivers/net/usb/int51x1.c | 1 +
drivers/net/usb/ipheth.c | 1 +
drivers/net/usb/kalmia.c | 3 ++-
drivers/net/usb/kaweth.c | 1 +
drivers/net/usb/lg-vl600.c | 1 +
drivers/net/usb/mcs7830.c | 1 +
drivers/net/usb/net1080.c | 1 +
drivers/net/usb/pegasus.c | 1 +
drivers/net/usb/plusb.c | 1 +
drivers/net/usb/qmi_wwan.c | 1 +
drivers/net/usb/rndis_host.c | 1 +
drivers/net/usb/rtl8150.c | 3 ++-
drivers/net/usb/sierra_net.c | 1 +
drivers/net/usb/smsc75xx.c | 1 +
drivers/net/usb/smsc95xx.c | 1 +
drivers/net/usb/zaurus.c | 1 +
drivers/net/wireless/at76c50x-usb.c | 1 +
drivers/net/wireless/ath/ath6kl/usb.c | 1 +
drivers/net/wireless/ath/ath9k/hif_usb.c | 1 +
drivers/net/wireless/ath/carl9170/usb.c | 1 +
drivers/net/wireless/brcm80211/brcmfmac/usb.c | 1 +
drivers/net/wireless/libertas/if_usb.c | 1 +
drivers/net/wireless/libertas_tf/if_usb.c | 1 +
drivers/net/wireless/orinoco/orinoco_usb.c | 1 +
drivers/net/wireless/p54/p54usb.c | 1 +
drivers/net/wireless/rndis_wlan.c | 1 +
drivers/net/wireless/rt2x00/rt2500usb.c | 1 +
drivers/net/wireless/rt2x00/rt2800usb.c | 1 +
drivers/net/wireless/rt2x00/rt73usb.c | 1 +
drivers/net/wireless/rtl818x/rtl8187/dev.c | 1 +
drivers/net/wireless/rtlwifi/rtl8192cu/sw.c | 1 +
drivers/net/wireless/zd1201.c | 1 +
drivers/net/wireless/zd1211rw/zd_usb.c | 1 +
drivers/usb/class/cdc-acm.c | 1 +
drivers/usb/class/cdc-wdm.c | 1 +
56 files changed, 58 insertions(+), 2 deletions(-)

diff --git a/drivers/bluetooth/ath3k.c b/drivers/bluetooth/ath3k.c
index 57fd867..2812b15 100644
--- a/drivers/bluetooth/ath3k.c
+++ b/drivers/bluetooth/ath3k.c
@@ -439,6 +439,7 @@ static struct usb_driver ath3k_driver = {
.probe = ath3k_probe,
.disconnect = ath3k_disconnect,
.id_table = ath3k_table,
+ .disable_hub_initiated_lpm = 1,
};

module_usb_driver(ath3k_driver);
diff --git a/drivers/bluetooth/bcm203x.c b/drivers/bluetooth/bcm203x.c
index 1e742a5..37ae175 100644
--- a/drivers/bluetooth/bcm203x.c
+++ b/drivers/bluetooth/bcm203x.c
@@ -279,6 +279,7 @@ static struct usb_driver bcm203x_driver = {
.probe = bcm203x_probe,
.disconnect = bcm203x_disconnect,
.id_table = bcm203x_table,
+ .disable_hub_initiated_lpm = 1,
};

module_usb_driver(bcm203x_driver);
diff --git a/drivers/bluetooth/bfusb.c b/drivers/bluetooth/bfusb.c
index b8ac1c5..32e8251 100644
--- a/drivers/bluetooth/bfusb.c
+++ b/drivers/bluetooth/bfusb.c
@@ -749,6 +749,7 @@ static struct usb_driver bfusb_driver = {
.probe = bfusb_probe,
.disconnect = bfusb_disconnect,
.id_table = bfusb_table,
+ .disable_hub_initiated_lpm = 1,
};

module_usb_driver(bfusb_driver);
diff --git a/drivers/bluetooth/bpa10x.c b/drivers/bluetooth/bpa10x.c
index d894340..609861a 100644
--- a/drivers/bluetooth/bpa10x.c
+++ b/drivers/bluetooth/bpa10x.c
@@ -508,6 +508,7 @@ static struct usb_driver bpa10x_driver = {
.probe = bpa10x_probe,
.disconnect = bpa10x_disconnect,
.id_table = bpa10x_table,
+ .disable_hub_initiated_lpm = 1,
};

module_usb_driver(bpa10x_driver);
diff --git a/drivers/bluetooth/btusb.c b/drivers/bluetooth/btusb.c
index 9217121..461c68b 100644
--- a/drivers/bluetooth/btusb.c
+++ b/drivers/bluetooth/btusb.c
@@ -1218,6 +1218,7 @@ static struct usb_driver btusb_driver = {
#endif
.id_table = btusb_table,
.supports_autosuspend = 1,
+ .disable_hub_initiated_lpm = 1,
};

module_usb_driver(btusb_driver);
diff --git a/drivers/isdn/gigaset/bas-gigaset.c b/drivers/isdn/gigaset/bas-gigaset.c
index afa0802..17ea017 100644
--- a/drivers/isdn/gigaset/bas-gigaset.c
+++ b/drivers/isdn/gigaset/bas-gigaset.c
@@ -148,6 +148,7 @@ static struct usb_driver gigaset_usb_driver = {
.reset_resume = gigaset_post_reset,
.pre_reset = gigaset_pre_reset,
.post_reset = gigaset_post_reset,
+ .disable_hub_initiated_lpm = 1,
};

/* get message text for usb_submit_urb return code
diff --git a/drivers/isdn/gigaset/usb-gigaset.c b/drivers/isdn/gigaset/usb-gigaset.c
index 049da67..78f81e8 100644
--- a/drivers/isdn/gigaset/usb-gigaset.c
+++ b/drivers/isdn/gigaset/usb-gigaset.c
@@ -124,6 +124,7 @@ static struct usb_driver gigaset_usb_driver = {
.reset_resume = gigaset_resume,
.pre_reset = gigaset_pre_reset,
.post_reset = gigaset_resume,
+ .disable_hub_initiated_lpm = 1,
};

struct usb_cardstate {
diff --git a/drivers/isdn/hardware/mISDN/hfcsusb.c b/drivers/isdn/hardware/mISDN/hfcsusb.c
index 8cde2a0..cddb769 100644
--- a/drivers/isdn/hardware/mISDN/hfcsusb.c
+++ b/drivers/isdn/hardware/mISDN/hfcsusb.c
@@ -2151,6 +2151,7 @@ static struct usb_driver hfcsusb_drv = {
.id_table = hfcsusb_idtab,
.probe = hfcsusb_probe,
.disconnect = hfcsusb_disconnect,
+ .disable_hub_initiated_lpm = 1,
};

module_usb_driver(hfcsusb_drv);
diff --git a/drivers/isdn/hisax/hfc_usb.c b/drivers/isdn/hisax/hfc_usb.c
index 62c65bd..84f9c81 100644
--- a/drivers/isdn/hisax/hfc_usb.c
+++ b/drivers/isdn/hisax/hfc_usb.c
@@ -1568,6 +1568,7 @@ static struct usb_driver hfc_drv = {
.id_table = hfcusb_idtab,
.probe = hfc_usb_probe,
.disconnect = hfc_usb_disconnect,
+ .disable_hub_initiated_lpm = 1,
};

static void __exit
diff --git a/drivers/isdn/hisax/st5481_init.c b/drivers/isdn/hisax/st5481_init.c
index 100296e..54ef9e4 100644
--- a/drivers/isdn/hisax/st5481_init.c
+++ b/drivers/isdn/hisax/st5481_init.c
@@ -182,6 +182,7 @@ static struct usb_driver st5481_usb_driver = {
.probe = probe_st5481,
.disconnect = disconnect_st5481,
.id_table = st5481_ids,
+ .disable_hub_initiated_lpm = 1,
};

static int __init st5481_usb_init(void)
diff --git a/drivers/net/usb/asix.c b/drivers/net/usb/asix.c
index 42b5151..71e2b05 100644
--- a/drivers/net/usb/asix.c
+++ b/drivers/net/usb/asix.c
@@ -1647,6 +1647,7 @@ static struct usb_driver asix_driver = {
.resume = usbnet_resume,
.disconnect = usbnet_disconnect,
.supports_autosuspend = 1,
+ .disable_hub_initiated_lpm = 1,
};

module_usb_driver(asix_driver);
diff --git a/drivers/net/usb/catc.c b/drivers/net/usb/catc.c
index 5a73730..26c5beb 100644
--- a/drivers/net/usb/catc.c
+++ b/drivers/net/usb/catc.c
@@ -952,6 +952,7 @@ static struct usb_driver catc_driver = {
.probe = catc_probe,
.disconnect = catc_disconnect,
.id_table = catc_id_table,
+ .disable_hub_initiated_lpm = 1,
};

module_usb_driver(catc_driver);
diff --git a/drivers/net/usb/cdc-phonet.c b/drivers/net/usb/cdc-phonet.c
index 3e41b00..d848d4d 100644
--- a/drivers/net/usb/cdc-phonet.c
+++ b/drivers/net/usb/cdc-phonet.c
@@ -457,6 +457,7 @@ static struct usb_driver usbpn_driver = {
.probe = usbpn_probe,
.disconnect = usbpn_disconnect,
.id_table = usbpn_ids,
+ .disable_hub_initiated_lpm = 1,
};

module_usb_driver(usbpn_driver);
diff --git a/drivers/net/usb/cdc_eem.c b/drivers/net/usb/cdc_eem.c
index 685a4e2..434d5af 100644
--- a/drivers/net/usb/cdc_eem.c
+++ b/drivers/net/usb/cdc_eem.c
@@ -368,6 +368,7 @@ static struct usb_driver eem_driver = {
.disconnect = usbnet_disconnect,
.suspend = usbnet_suspend,
.resume = usbnet_resume,
+ .disable_hub_initiated_lpm = 1,
};

module_usb_driver(eem_driver);
diff --git a/drivers/net/usb/cdc_ether.c b/drivers/net/usb/cdc_ether.c
index 90a3002..7dc470c 100644
--- a/drivers/net/usb/cdc_ether.c
+++ b/drivers/net/usb/cdc_ether.c
@@ -623,6 +623,7 @@ static struct usb_driver cdc_driver = {
.resume = usbnet_resume,
.reset_resume = usbnet_resume,
.supports_autosuspend = 1,
+ .disable_hub_initiated_lpm = 1,
};

module_usb_driver(cdc_driver);
diff --git a/drivers/net/usb/cdc_ncm.c b/drivers/net/usb/cdc_ncm.c
index 7adc9f6..4b9513f 100644
--- a/drivers/net/usb/cdc_ncm.c
+++ b/drivers/net/usb/cdc_ncm.c
@@ -1212,6 +1212,7 @@ static struct usb_driver cdc_ncm_driver = {
.resume = usbnet_resume,
.reset_resume = usbnet_resume,
.supports_autosuspend = 1,
+ .disable_hub_initiated_lpm = 1,
};

static const struct ethtool_ops cdc_ncm_ethtool_ops = {
diff --git a/drivers/net/usb/cdc_subset.c b/drivers/net/usb/cdc_subset.c
index b403d93..0d1fe89 100644
--- a/drivers/net/usb/cdc_subset.c
+++ b/drivers/net/usb/cdc_subset.c
@@ -336,6 +336,7 @@ static struct usb_driver cdc_subset_driver = {
.resume = usbnet_resume,
.disconnect = usbnet_disconnect,
.id_table = products,
+ .disable_hub_initiated_lpm = 1,
};

module_usb_driver(cdc_subset_driver);
diff --git a/drivers/net/usb/cx82310_eth.c b/drivers/net/usb/cx82310_eth.c
index 0e05313..49ab45e 100644
--- a/drivers/net/usb/cx82310_eth.c
+++ b/drivers/net/usb/cx82310_eth.c
@@ -327,6 +327,7 @@ static struct usb_driver cx82310_driver = {
.disconnect = usbnet_disconnect,
.suspend = usbnet_suspend,
.resume = usbnet_resume,
+ .disable_hub_initiated_lpm = 1,
};

module_usb_driver(cx82310_driver);
diff --git a/drivers/net/usb/dm9601.c b/drivers/net/usb/dm9601.c
index b972263..e0433ce 100644
--- a/drivers/net/usb/dm9601.c
+++ b/drivers/net/usb/dm9601.c
@@ -670,6 +670,7 @@ static struct usb_driver dm9601_driver = {
.disconnect = usbnet_disconnect,
.suspend = usbnet_suspend,
.resume = usbnet_resume,
+ .disable_hub_initiated_lpm = 1,
};

module_usb_driver(dm9601_driver);
diff --git a/drivers/net/usb/gl620a.c b/drivers/net/usb/gl620a.c
index 38266bd..db3c802 100644
--- a/drivers/net/usb/gl620a.c
+++ b/drivers/net/usb/gl620a.c
@@ -225,6 +225,7 @@ static struct usb_driver gl620a_driver = {
.disconnect = usbnet_disconnect,
.suspend = usbnet_suspend,
.resume = usbnet_resume,
+ .disable_hub_initiated_lpm = 1,
};

module_usb_driver(gl620a_driver);
diff --git a/drivers/net/usb/hso.c b/drivers/net/usb/hso.c
index 2d2a688..042c1a9 100644
--- a/drivers/net/usb/hso.c
+++ b/drivers/net/usb/hso.c
@@ -3291,6 +3291,7 @@ static struct usb_driver hso_driver = {
.resume = hso_resume,
.reset_resume = hso_resume,
.supports_autosuspend = 1,
+ .disable_hub_initiated_lpm = 1,
};

static int __init hso_init(void)
diff --git a/drivers/net/usb/int51x1.c b/drivers/net/usb/int51x1.c
index 12a22a4..8de6417 100644
--- a/drivers/net/usb/int51x1.c
+++ b/drivers/net/usb/int51x1.c
@@ -236,6 +236,7 @@ static struct usb_driver int51x1_driver = {
.disconnect = usbnet_disconnect,
.suspend = usbnet_suspend,
.resume = usbnet_resume,
+ .disable_hub_initiated_lpm = 1,
};

module_usb_driver(int51x1_driver);
diff --git a/drivers/net/usb/ipheth.c b/drivers/net/usb/ipheth.c
index 32519e5..964031e 100644
--- a/drivers/net/usb/ipheth.c
+++ b/drivers/net/usb/ipheth.c
@@ -554,6 +554,7 @@ static struct usb_driver ipheth_driver = {
.probe = ipheth_probe,
.disconnect = ipheth_disconnect,
.id_table = ipheth_table,
+ .disable_hub_initiated_lpm = 1,
};

module_usb_driver(ipheth_driver);
diff --git a/drivers/net/usb/kalmia.c b/drivers/net/usb/kalmia.c
index 7562649..92c49e0 100644
--- a/drivers/net/usb/kalmia.c
+++ b/drivers/net/usb/kalmia.c
@@ -372,7 +372,8 @@ static struct usb_driver kalmia_driver = {
.probe = usbnet_probe,
.disconnect = usbnet_disconnect,
.suspend = usbnet_suspend,
- .resume = usbnet_resume
+ .resume = usbnet_resume,
+ .disable_hub_initiated_lpm = 1,
};

module_usb_driver(kalmia_driver);
diff --git a/drivers/net/usb/kaweth.c b/drivers/net/usb/kaweth.c
index b8baf08..d8ad552 100644
--- a/drivers/net/usb/kaweth.c
+++ b/drivers/net/usb/kaweth.c
@@ -179,6 +179,7 @@ static struct usb_driver kaweth_driver = {
.resume = kaweth_resume,
.id_table = usb_klsi_table,
.supports_autosuspend = 1,
+ .disable_hub_initiated_lpm = 1,
};

typedef __u8 eth_addr_t[6];
diff --git a/drivers/net/usb/lg-vl600.c b/drivers/net/usb/lg-vl600.c
index 45a981f..808d650 100644
--- a/drivers/net/usb/lg-vl600.c
+++ b/drivers/net/usb/lg-vl600.c
@@ -344,6 +344,7 @@ static struct usb_driver lg_vl600_driver = {
.disconnect = usbnet_disconnect,
.suspend = usbnet_suspend,
.resume = usbnet_resume,
+ .disable_hub_initiated_lpm = 1,
};

module_usb_driver(lg_vl600_driver);
diff --git a/drivers/net/usb/mcs7830.c b/drivers/net/usb/mcs7830.c
index c434b6b..add1064 100644
--- a/drivers/net/usb/mcs7830.c
+++ b/drivers/net/usb/mcs7830.c
@@ -690,6 +690,7 @@ static struct usb_driver mcs7830_driver = {
.suspend = usbnet_suspend,
.resume = usbnet_resume,
.reset_resume = mcs7830_reset_resume,
+ .disable_hub_initiated_lpm = 1,
};

module_usb_driver(mcs7830_driver);
diff --git a/drivers/net/usb/net1080.c b/drivers/net/usb/net1080.c
index 83f965c..28c4d51 100644
--- a/drivers/net/usb/net1080.c
+++ b/drivers/net/usb/net1080.c
@@ -587,6 +587,7 @@ static struct usb_driver net1080_driver = {
.disconnect = usbnet_disconnect,
.suspend = usbnet_suspend,
.resume = usbnet_resume,
+ .disable_hub_initiated_lpm = 1,
};

module_usb_driver(net1080_driver);
diff --git a/drivers/net/usb/pegasus.c b/drivers/net/usb/pegasus.c
index 7523930..7023220 100644
--- a/drivers/net/usb/pegasus.c
+++ b/drivers/net/usb/pegasus.c
@@ -1489,6 +1489,7 @@ static struct usb_driver pegasus_driver = {
.id_table = pegasus_ids,
.suspend = pegasus_suspend,
.resume = pegasus_resume,
+ .disable_hub_initiated_lpm = 1,
};

static void __init parse_id(char *id)
diff --git a/drivers/net/usb/plusb.c b/drivers/net/usb/plusb.c
index b2b035e..4584b9a 100644
--- a/drivers/net/usb/plusb.c
+++ b/drivers/net/usb/plusb.c
@@ -152,6 +152,7 @@ static struct usb_driver plusb_driver = {
.disconnect = usbnet_disconnect,
.suspend = usbnet_suspend,
.resume = usbnet_resume,
+ .disable_hub_initiated_lpm = 1,
};

module_usb_driver(plusb_driver);
diff --git a/drivers/net/usb/qmi_wwan.c b/drivers/net/usb/qmi_wwan.c
index d316503b..9048efe 100644
--- a/drivers/net/usb/qmi_wwan.c
+++ b/drivers/net/usb/qmi_wwan.c
@@ -525,6 +525,7 @@ static struct usb_driver qmi_wwan_driver = {
.resume = qmi_wwan_resume,
.reset_resume = qmi_wwan_resume,
.supports_autosuspend = 1,
+ .disable_hub_initiated_lpm = 1,
};

static int __init qmi_wwan_init(void)
diff --git a/drivers/net/usb/rndis_host.c b/drivers/net/usb/rndis_host.c
index c8f1b5b..446d074 100644
--- a/drivers/net/usb/rndis_host.c
+++ b/drivers/net/usb/rndis_host.c
@@ -633,6 +633,7 @@ static struct usb_driver rndis_driver = {
.disconnect = usbnet_disconnect,
.suspend = usbnet_suspend,
.resume = usbnet_resume,
+ .disable_hub_initiated_lpm = 1,
};

module_usb_driver(rndis_driver);
diff --git a/drivers/net/usb/rtl8150.c b/drivers/net/usb/rtl8150.c
index 65854cd..0e2c92e 100644
--- a/drivers/net/usb/rtl8150.c
+++ b/drivers/net/usb/rtl8150.c
@@ -948,7 +948,8 @@ static struct usb_driver rtl8150_driver = {
.disconnect = rtl8150_disconnect,
.id_table = rtl8150_table,
.suspend = rtl8150_suspend,
- .resume = rtl8150_resume
+ .resume = rtl8150_resume,
+ .disable_hub_initiated_lpm = 1,
};

module_usb_driver(rtl8150_driver);
diff --git a/drivers/net/usb/sierra_net.c b/drivers/net/usb/sierra_net.c
index b59cf20..3faef56 100644
--- a/drivers/net/usb/sierra_net.c
+++ b/drivers/net/usb/sierra_net.c
@@ -982,6 +982,7 @@ static struct usb_driver sierra_net_driver = {
.suspend = usbnet_suspend,
.resume = usbnet_resume,
.no_dynamic_id = 1,
+ .disable_hub_initiated_lpm = 1,
};

module_usb_driver(sierra_net_driver);
diff --git a/drivers/net/usb/smsc75xx.c b/drivers/net/usb/smsc75xx.c
index 00103a8..1fb4ddb 100644
--- a/drivers/net/usb/smsc75xx.c
+++ b/drivers/net/usb/smsc75xx.c
@@ -1250,6 +1250,7 @@ static struct usb_driver smsc75xx_driver = {
.suspend = usbnet_suspend,
.resume = usbnet_resume,
.disconnect = usbnet_disconnect,
+ .disable_hub_initiated_lpm = 1,
};

module_usb_driver(smsc75xx_driver);
diff --git a/drivers/net/usb/smsc95xx.c b/drivers/net/usb/smsc95xx.c
index 94ae669..b1112e7 100644
--- a/drivers/net/usb/smsc95xx.c
+++ b/drivers/net/usb/smsc95xx.c
@@ -1297,6 +1297,7 @@ static struct usb_driver smsc95xx_driver = {
.suspend = usbnet_suspend,
.resume = usbnet_resume,
.disconnect = usbnet_disconnect,
+ .disable_hub_initiated_lpm = 1,
};

module_usb_driver(smsc95xx_driver);
diff --git a/drivers/net/usb/zaurus.c b/drivers/net/usb/zaurus.c
index 34db195..35c9030 100644
--- a/drivers/net/usb/zaurus.c
+++ b/drivers/net/usb/zaurus.c
@@ -377,6 +377,7 @@ static struct usb_driver zaurus_driver = {
.disconnect = usbnet_disconnect,
.suspend = usbnet_suspend,
.resume = usbnet_resume,
+ .disable_hub_initiated_lpm = 1,
};

module_usb_driver(zaurus_driver);
diff --git a/drivers/net/wireless/at76c50x-usb.c b/drivers/net/wireless/at76c50x-usb.c
index cc741b3..9dcd49c 100644
--- a/drivers/net/wireless/at76c50x-usb.c
+++ b/drivers/net/wireless/at76c50x-usb.c
@@ -2486,6 +2486,7 @@ static struct usb_driver at76_driver = {
.probe = at76_probe,
.disconnect = at76_disconnect,
.id_table = dev_table,
+ .disable_hub_initiated_lpm = 1,
};

static int __init at76_mod_init(void)
diff --git a/drivers/net/wireless/ath/ath6kl/usb.c b/drivers/net/wireless/ath/ath6kl/usb.c
index 325b122..f8a27db 100644
--- a/drivers/net/wireless/ath/ath6kl/usb.c
+++ b/drivers/net/wireless/ath/ath6kl/usb.c
@@ -405,6 +405,7 @@ static struct usb_driver ath6kl_usb_driver = {
.probe = ath6kl_usb_probe,
.disconnect = ath6kl_usb_remove,
.id_table = ath6kl_usb_ids,
+ .disable_hub_initiated_lpm = 1,
};

static int ath6kl_usb_init(void)
diff --git a/drivers/net/wireless/ath/ath9k/hif_usb.c b/drivers/net/wireless/ath/ath9k/hif_usb.c
index 424aabb..dea53de 100644
--- a/drivers/net/wireless/ath/ath9k/hif_usb.c
+++ b/drivers/net/wireless/ath/ath9k/hif_usb.c
@@ -1356,6 +1356,7 @@ static struct usb_driver ath9k_hif_usb_driver = {
#endif
.id_table = ath9k_hif_usb_ids,
.soft_unbind = 1,
+ .disable_hub_initiated_lpm = 1,
};

int ath9k_hif_usb_init(void)
diff --git a/drivers/net/wireless/ath/carl9170/usb.c b/drivers/net/wireless/ath/carl9170/usb.c
index 89821e4..888152c 100644
--- a/drivers/net/wireless/ath/carl9170/usb.c
+++ b/drivers/net/wireless/ath/carl9170/usb.c
@@ -1159,6 +1159,7 @@ static struct usb_driver carl9170_driver = {
.resume = carl9170_usb_resume,
.reset_resume = carl9170_usb_resume,
#endif /* CONFIG_PM */
+ .disable_hub_initiated_lpm = 1,
};

module_usb_driver(carl9170_driver);
diff --git a/drivers/net/wireless/brcm80211/brcmfmac/usb.c b/drivers/net/wireless/brcm80211/brcmfmac/usb.c
index 8236422..8852d23 100644
--- a/drivers/net/wireless/brcm80211/brcmfmac/usb.c
+++ b/drivers/net/wireless/brcm80211/brcmfmac/usb.c
@@ -1605,6 +1605,7 @@ static struct usb_driver brcmf_usbdrvr = {
.suspend = brcmf_usb_suspend,
.resume = brcmf_usb_resume,
.supports_autosuspend = 1
+ .disable_hub_initiated_lpm = 1,
};

void brcmf_usb_exit(void)
diff --git a/drivers/net/wireless/libertas/if_usb.c b/drivers/net/wireless/libertas/if_usb.c
index 74da5f1..76ea66d 100644
--- a/drivers/net/wireless/libertas/if_usb.c
+++ b/drivers/net/wireless/libertas/if_usb.c
@@ -1180,6 +1180,7 @@ static struct usb_driver if_usb_driver = {
.suspend = if_usb_suspend,
.resume = if_usb_resume,
.reset_resume = if_usb_resume,
+ .disable_hub_initiated_lpm = 1,
};

module_usb_driver(if_usb_driver);
diff --git a/drivers/net/wireless/libertas_tf/if_usb.c b/drivers/net/wireless/libertas_tf/if_usb.c
index 7ced130..19a5a92 100644
--- a/drivers/net/wireless/libertas_tf/if_usb.c
+++ b/drivers/net/wireless/libertas_tf/if_usb.c
@@ -920,6 +920,7 @@ static struct usb_driver if_usb_driver = {
.id_table = if_usb_table,
.suspend = if_usb_suspend,
.resume = if_usb_resume,
+ .disable_hub_initiated_lpm = 1,
};

module_usb_driver(if_usb_driver);
diff --git a/drivers/net/wireless/orinoco/orinoco_usb.c b/drivers/net/wireless/orinoco/orinoco_usb.c
index f634d45..7f53cea2 100644
--- a/drivers/net/wireless/orinoco/orinoco_usb.c
+++ b/drivers/net/wireless/orinoco/orinoco_usb.c
@@ -1752,6 +1752,7 @@ static struct usb_driver orinoco_driver = {
.probe = ezusb_probe,
.disconnect = ezusb_disconnect,
.id_table = ezusb_table,
+ .disable_hub_initiated_lpm = 1,
};

module_usb_driver(orinoco_driver);
diff --git a/drivers/net/wireless/p54/p54usb.c b/drivers/net/wireless/p54/p54usb.c
index f4d28c3..d14dc81 100644
--- a/drivers/net/wireless/p54/p54usb.c
+++ b/drivers/net/wireless/p54/p54usb.c
@@ -1081,6 +1081,7 @@ static struct usb_driver p54u_driver = {
.reset_resume = p54u_resume,
#endif /* CONFIG_PM */
.soft_unbind = 1,
+ .disable_hub_initiated_lpm = 1,
};

module_usb_driver(p54u_driver);
diff --git a/drivers/net/wireless/rndis_wlan.c b/drivers/net/wireless/rndis_wlan.c
index d66e298..748a89d 100644
--- a/drivers/net/wireless/rndis_wlan.c
+++ b/drivers/net/wireless/rndis_wlan.c
@@ -3776,6 +3776,7 @@ static struct usb_driver rndis_wlan_driver = {
.disconnect = usbnet_disconnect,
.suspend = usbnet_suspend,
.resume = usbnet_resume,
+ .disable_hub_initiated_lpm = 1,
};

module_usb_driver(rndis_wlan_driver);
diff --git a/drivers/net/wireless/rt2x00/rt2500usb.c b/drivers/net/wireless/rt2x00/rt2500usb.c
index 1de9c75..d432e49 100644
--- a/drivers/net/wireless/rt2x00/rt2500usb.c
+++ b/drivers/net/wireless/rt2x00/rt2500usb.c
@@ -1980,6 +1980,7 @@ static struct usb_driver rt2500usb_driver = {
.disconnect = rt2x00usb_disconnect,
.suspend = rt2x00usb_suspend,
.resume = rt2x00usb_resume,
+ .disable_hub_initiated_lpm = 1,
};

module_usb_driver(rt2500usb_driver);
diff --git a/drivers/net/wireless/rt2x00/rt2800usb.c b/drivers/net/wireless/rt2x00/rt2800usb.c
index 001735f..d60d4e2 100644
--- a/drivers/net/wireless/rt2x00/rt2800usb.c
+++ b/drivers/net/wireless/rt2x00/rt2800usb.c
@@ -1293,6 +1293,7 @@ static struct usb_driver rt2800usb_driver = {
.disconnect = rt2x00usb_disconnect,
.suspend = rt2x00usb_suspend,
.resume = rt2x00usb_resume,
+ .disable_hub_initiated_lpm = 1,
};

module_usb_driver(rt2800usb_driver);
diff --git a/drivers/net/wireless/rt2x00/rt73usb.c b/drivers/net/wireless/rt2x00/rt73usb.c
index e477a96..f813de6 100644
--- a/drivers/net/wireless/rt2x00/rt73usb.c
+++ b/drivers/net/wireless/rt2x00/rt73usb.c
@@ -2526,6 +2526,7 @@ static struct usb_driver rt73usb_driver = {
.disconnect = rt2x00usb_disconnect,
.suspend = rt2x00usb_suspend,
.resume = rt2x00usb_resume,
+ .disable_hub_initiated_lpm = 1,
};

module_usb_driver(rt73usb_driver);
diff --git a/drivers/net/wireless/rtl818x/rtl8187/dev.c b/drivers/net/wireless/rtl818x/rtl8187/dev.c
index cf53ac9..c2d2a21 100644
--- a/drivers/net/wireless/rtl818x/rtl8187/dev.c
+++ b/drivers/net/wireless/rtl818x/rtl8187/dev.c
@@ -1662,6 +1662,7 @@ static struct usb_driver rtl8187_driver = {
.id_table = rtl8187_table,
.probe = rtl8187_probe,
.disconnect = __devexit_p(rtl8187_disconnect),
+ .disable_hub_initiated_lpm = 1,
};

module_usb_driver(rtl8187_driver);
diff --git a/drivers/net/wireless/rtlwifi/rtl8192cu/sw.c b/drivers/net/wireless/rtlwifi/rtl8192cu/sw.c
index 82c85286..0face8b 100644
--- a/drivers/net/wireless/rtlwifi/rtl8192cu/sw.c
+++ b/drivers/net/wireless/rtlwifi/rtl8192cu/sw.c
@@ -372,6 +372,7 @@ static struct usb_driver rtl8192cu_driver = {
#ifdef CONFIG_AUTOSUSPEND
.supports_autosuspend = 1,
#endif
+ .disable_hub_initiated_lpm = 1,
};

module_usb_driver(rtl8192cu_driver);
diff --git a/drivers/net/wireless/zd1201.c b/drivers/net/wireless/zd1201.c
index a66b93b..48273dd 100644
--- a/drivers/net/wireless/zd1201.c
+++ b/drivers/net/wireless/zd1201.c
@@ -1905,6 +1905,7 @@ static struct usb_driver zd1201_usb = {
.id_table = zd1201_table,
.suspend = zd1201_suspend,
.resume = zd1201_resume,
+ .disable_hub_initiated_lpm = 1,
};

module_usb_driver(zd1201_usb);
diff --git a/drivers/net/wireless/zd1211rw/zd_usb.c b/drivers/net/wireless/zd1211rw/zd_usb.c
index f766b3e..af83c43 100644
--- a/drivers/net/wireless/zd1211rw/zd_usb.c
+++ b/drivers/net/wireless/zd1211rw/zd_usb.c
@@ -1542,6 +1542,7 @@ static struct usb_driver driver = {
.disconnect = disconnect,
.pre_reset = pre_reset,
.post_reset = post_reset,
+ .disable_hub_initiated_lpm = 1,
};

struct workqueue_struct *zd_workqueue;
diff --git a/drivers/usb/class/cdc-acm.c b/drivers/usb/class/cdc-acm.c
index b32ccb4..f2a120e 100644
--- a/drivers/usb/class/cdc-acm.c
+++ b/drivers/usb/class/cdc-acm.c
@@ -1664,6 +1664,7 @@ static struct usb_driver acm_driver = {
#ifdef CONFIG_PM
.supports_autosuspend = 1,
#endif
+ .disable_hub_initiated_lpm = 1,
};

/*
diff --git a/drivers/usb/class/cdc-wdm.c b/drivers/usb/class/cdc-wdm.c
index 5d15165..205b1f8 100644
--- a/drivers/usb/class/cdc-wdm.c
+++ b/drivers/usb/class/cdc-wdm.c
@@ -1025,6 +1025,7 @@ static struct usb_driver wdm_driver = {
.post_reset = wdm_post_reset,
.id_table = wdm_ids,
.supports_autosuspend = 1,
+ .disable_hub_initiated_lpm = 1,
};

module_usb_driver(wdm_driver);
--
1.7.9



2012-05-17 04:52:23

by Sarah Sharp

[permalink] [raw]
Subject: Re: [RFC 13/13] USB: Disable hub-initiated LPM for comms devices.

On Wed, May 16, 2012 at 04:20:19PM -0700, Greg Kroah-Hartman wrote:
> On Wed, May 16, 2012 at 03:45:28PM -0700, Sarah Sharp wrote:
> > [Resending with a smaller Cc list]
> >
> > Hub-initiated LPM is not good for USB communications devices. Comms
> > devices should be able to tell when their link can go into a lower power
> > state, because they know when an incoming transmission is finished.
> > Ideally, these devices would slam their links into a lower power state,
> > using the device-initiated LPM, after finishing the last packet of their
> > data transfer.
> >
> > If we enable the idle timeouts for the parent hubs to enable
> > hub-initiated LPM, we will get a lot of useless LPM packets on the bus
> > as the devices reject LPM transitions when they're in the middle of
> > receiving data. Worse, some devices might blindly accept the
> > hub-initiated LPM and power down their radios while they're in the
> > middle of receiving a transmission.
> >
> > The Intel Windows folks are disabling hub-initiated LPM for all USB
> > communications devices under a xHCI USB 3.0 host. In order to keep
> > the Linux behavior as close as possible to Windows, we need to do the
> > same in Linux.
>
> How is the USB core on Windows determining that LPM should be turned off
> for these devices? Surely they aren't modifying each individual driver
> like this is, right? Any way we also can do this in the core?

No, I don't think they're modifying individual drivers. Maybe they
placed a shim/filter driver below other drivers?

Basically, I don't know the exact details of what the Windows folks are
doing. The recommendation from the Intel Windows team was simply to
turn hub-initiated LPM off for "all communications devices". Perhaps
the Windows USB core is looking for specific USB class codes? Or maybe
it has some older API that lets the core know it's a communications
device?

I'm not really sure we can do it in the USB core with out basically
duplicating all the class/PID/VID matching in the communications driver.
I think just adding a flag might be the best way. I'm open to
suggestions though.

> Or, turn it around the other way, and only enable it if we know it's
> safe to do so, in each driver, but I guess that would be even messier.

Yeah, I think it would be messier.

Sarah Sharp

2012-05-16 23:23:25

by Greg Kroah-Hartman

[permalink] [raw]
Subject: Re: [RFC 13/13] USB: Disable hub-initiated LPM for comms devices.

On Wed, May 16, 2012 at 03:45:28PM -0700, Sarah Sharp wrote:
> [Resending with a smaller Cc list]
>
> Hub-initiated LPM is not good for USB communications devices. Comms
> devices should be able to tell when their link can go into a lower power
> state, because they know when an incoming transmission is finished.
> Ideally, these devices would slam their links into a lower power state,
> using the device-initiated LPM, after finishing the last packet of their
> data transfer.
>
> If we enable the idle timeouts for the parent hubs to enable
> hub-initiated LPM, we will get a lot of useless LPM packets on the bus
> as the devices reject LPM transitions when they're in the middle of
> receiving data. Worse, some devices might blindly accept the
> hub-initiated LPM and power down their radios while they're in the
> middle of receiving a transmission.
>
> The Intel Windows folks are disabling hub-initiated LPM for all USB
> communications devices under a xHCI USB 3.0 host. In order to keep
> the Linux behavior as close as possible to Windows, we need to do the
> same in Linux.

How is the USB core on Windows determining that LPM should be turned off
for these devices? Surely they aren't modifying each individual driver
like this is, right? Any way we also can do this in the core?

Or, turn it around the other way, and only enable it if we know it's
safe to do so, in each driver, but I guess that would be even messier.

thanks,

greg k-h

2012-05-17 17:31:50

by Sarah Sharp

[permalink] [raw]
Subject: Re: [RFC 13/13] USB: Disable hub-initiated LPM for comms devices.

On Thu, May 17, 2012 at 07:07:32PM +0200, Tilman Schmidt wrote:
> Am 16.05.2012 23:55, schrieb Sarah Sharp:
> > Set the disable_hub_initiated_lpm flag for for all USB communications
> > drivers. I know there aren't currently any USB 3.0 devices that
> > implement these class specifications, but we should be ready if they do.
>
> I follow the argument for class drivers. But this patch also
> modifies drivers for specific existing USB 2.0 only devices
> which are unlikely to ever grow USB 3.0 support, such as the
> Gigaset ISDN driver:
>
> > drivers/isdn/gigaset/bas-gigaset.c | 1 +
> > drivers/isdn/gigaset/usb-gigaset.c | 1 +

Is there a particular reason why you think that driver is unlikely to
ever get USB 3.0 support? I pretty much grabbed any USB driver that
looked like a communications driver without looking too closely at the
code.

> What is the interest of setting the disable_hub_initiated_lpm
> flag for these?

It's partially to lay the foundation for anyone who wants to make a USB
3.0 communications driver in the future. They're likely to start from
some USB 2.0 class driver, and copy a lot of code. If they notice that
flag is set in all the USB communications class drivers, they're likely
to set it as well.

I'm not quite sure where the best place to provide documentation on the
flag is. I've added the kernel doc comments to the structure, but maybe
it needs to be documented somewhere in Documentation/usb/?

Sarah Sharp

2012-05-17 16:39:30

by Greg Kroah-Hartman

[permalink] [raw]
Subject: Re: [RFC 13/13] USB: Disable hub-initiated LPM for comms devices.

On Thu, May 17, 2012 at 09:29:22AM -0700, Sarah Sharp wrote:
> One of the problems with this patch is that several mailing lists
> rejected it because of the large number of recipients in the Cc list,
> even when just the mailing lists were on the Cc list. Should I break it
> up into a series of patches per driver?

No need to do that, I can just take it.

> Also, do I need to wait on acks from the other maintainers for this
> change? My instinct is no, since it shouldn't change the behavior of
> their drivers with the USB devices that are out there today, but I don't
> want to step on anyone's toes.

Don't worry about it, we change USB drivers in other trees all the time :)

greg k-h

2012-05-17 14:49:56

by Greg Kroah-Hartman

[permalink] [raw]
Subject: Re: [RFC 13/13] USB: Disable hub-initiated LPM for comms devices.

On Wed, May 16, 2012 at 09:52:20PM -0700, Sarah Sharp wrote:
> On Wed, May 16, 2012 at 04:20:19PM -0700, Greg Kroah-Hartman wrote:
> > On Wed, May 16, 2012 at 03:45:28PM -0700, Sarah Sharp wrote:
> > > [Resending with a smaller Cc list]
> > >
> > > Hub-initiated LPM is not good for USB communications devices. Comms
> > > devices should be able to tell when their link can go into a lower power
> > > state, because they know when an incoming transmission is finished.
> > > Ideally, these devices would slam their links into a lower power state,
> > > using the device-initiated LPM, after finishing the last packet of their
> > > data transfer.
> > >
> > > If we enable the idle timeouts for the parent hubs to enable
> > > hub-initiated LPM, we will get a lot of useless LPM packets on the bus
> > > as the devices reject LPM transitions when they're in the middle of
> > > receiving data. Worse, some devices might blindly accept the
> > > hub-initiated LPM and power down their radios while they're in the
> > > middle of receiving a transmission.
> > >
> > > The Intel Windows folks are disabling hub-initiated LPM for all USB
> > > communications devices under a xHCI USB 3.0 host. In order to keep
> > > the Linux behavior as close as possible to Windows, we need to do the
> > > same in Linux.
> >
> > How is the USB core on Windows determining that LPM should be turned off
> > for these devices? Surely they aren't modifying each individual driver
> > like this is, right? Any way we also can do this in the core?
>
> No, I don't think they're modifying individual drivers. Maybe they
> placed a shim/filter driver below other drivers?

They can do this in their driver by just watching the device class type.

> Basically, I don't know the exact details of what the Windows folks are
> doing. The recommendation from the Intel Windows team was simply to
> turn hub-initiated LPM off for "all communications devices". Perhaps
> the Windows USB core is looking for specific USB class codes? Or maybe
> it has some older API that lets the core know it's a communications
> device?
>
> I'm not really sure we can do it in the USB core with out basically
> duplicating all the class/PID/VID matching in the communications driver.
> I think just adding a flag might be the best way. I'm open to
> suggestions though.

You can detect something as "simple" as a class type, which I bet is all
that Windows is going to be able to do as well.

> > Or, turn it around the other way, and only enable it if we know it's
> > safe to do so, in each driver, but I guess that would be even messier.
>
> Yeah, I think it would be messier.

Ok, this is probably the best solution for us as well, sorry for the
noise.

greg k-h

2012-05-18 23:10:06

by Sarah Sharp

[permalink] [raw]
Subject: Re: [RFC 13/13] USB: Disable hub-initiated LPM for comms devices.

On Sat, May 19, 2012 at 12:38:41AM +0200, Tilman Schmidt wrote:
> Am 17.05.2012 19:31, schrieb Sarah Sharp:
> > On Thu, May 17, 2012 at 07:07:32PM +0200, Tilman Schmidt wrote:
> >>
> >> I follow the argument for class drivers. But this patch also
> >> modifies drivers for specific existing USB 2.0 only devices
> >> which are unlikely to ever grow USB 3.0 support, such as the
> >> Gigaset ISDN driver:
> >>
> >>> drivers/isdn/gigaset/bas-gigaset.c | 1 +
> >>> drivers/isdn/gigaset/usb-gigaset.c | 1 +
> >
> > Is there a particular reason why you think that driver is unlikely to
> > ever get USB 3.0 support?
>
> Actually, there is. :-)
> - The USB devices driven by this driver aren't built anymore.
> - Their USB interface design is quite, um, idiosyncratic, and it's
> pretty unlikely that anyone will reuse it. (At least I truly hope
> no one will.)
> - Their successor models have completely different and incompatible
> USB interfaces which this driver is unable to handle.

I see!

> >> What is the interest of setting the disable_hub_initiated_lpm
> >> flag for these?
> >
> > It's partially to lay the foundation for anyone who wants to make a USB
> > 3.0 communications driver in the future. They're likely to start from
> > some USB 2.0 class driver, and copy a lot of code. If they notice that
> > flag is set in all the USB communications class drivers, they're likely
> > to set it as well.
>
> You've got a point there.
>
> > I'm not quite sure where the best place to provide documentation on the
> > flag is. I've added the kernel doc comments to the structure, but maybe
> > it needs to be documented somewhere in Documentation/usb/?
>
> Documentation/usb/power-management.txt would seem like a natural
> place. Although it appears to limit itself to "suspending" in its
> first paragraph, it does have a section "xHCI hardware link PM"
> at the end already, added by Andiry Xu on 2011-09-23.

Ok, I'll send a separate patch to add documentation here, after the 3.5
merge window. (I just sent the pull request for the USB 3.0 LPM patches
off to Greg.)

> Hmmm, that section seems to suggest that LPM exists for USB2, too.
> Perhaps I should reconsider my attitude towards your patch.

The patchset doesn't change the USB 2.0 LPM behavior at all. USB 2.0
doesn't have hub initiated LPM, because that would mean changing
existing USB 2.0 hub IP. Only devices attached directly to the roothub
will be able to do USB 2.0 LPM. Also, devices have to specifically say
they implement the USB 2.1 LPM extensions, so it's unlikely devices
supported by your driver implement USB 2.0 LPM.

Sarah Sharp

2012-05-17 17:07:36

by Tilman Schmidt

[permalink] [raw]
Subject: Re: [RFC 13/13] USB: Disable hub-initiated LPM for comms devices.

Am 16.05.2012 23:55, schrieb Sarah Sharp:
> Set the disable_hub_initiated_lpm flag for for all USB communications
> drivers. I know there aren't currently any USB 3.0 devices that
> implement these class specifications, but we should be ready if they do.

I follow the argument for class drivers. But this patch also
modifies drivers for specific existing USB 2.0 only devices
which are unlikely to ever grow USB 3.0 support, such as the
Gigaset ISDN driver:

> drivers/isdn/gigaset/bas-gigaset.c | 1 +
> drivers/isdn/gigaset/usb-gigaset.c | 1 +

What is the interest of setting the disable_hub_initiated_lpm
flag for these?

Thanks,
Tilman

--
Tilman Schmidt E-Mail: [email protected]
Bonn, Germany
In theory there is no difference between theory and practice.
In practice there is. (Yogi Berra)


Attachments:
signature.asc (262.00 B)
OpenPGP digital signature

2012-05-17 16:29:41

by Sarah Sharp

[permalink] [raw]
Subject: Re: [RFC 13/13] USB: Disable hub-initiated LPM for comms devices.

On Thu, May 17, 2012 at 07:49:51AM -0700, Greg Kroah-Hartman wrote:
> On Wed, May 16, 2012 at 09:52:20PM -0700, Sarah Sharp wrote:
> > On Wed, May 16, 2012 at 04:20:19PM -0700, Greg Kroah-Hartman wrote:
> > > On Wed, May 16, 2012 at 03:45:28PM -0700, Sarah Sharp wrote:
> > > > The Intel Windows folks are disabling hub-initiated LPM for all USB
> > > > communications devices under a xHCI USB 3.0 host. In order to keep
> > > > the Linux behavior as close as possible to Windows, we need to do the
> > > > same in Linux.
> > >
> > > How is the USB core on Windows determining that LPM should be turned off
> > > for these devices? Surely they aren't modifying each individual driver
> > > like this is, right? Any way we also can do this in the core?
> >
> > No, I don't think they're modifying individual drivers. Maybe they
> > placed a shim/filter driver below other drivers?
>
> They can do this in their driver by just watching the device class type.
>
> > Basically, I don't know the exact details of what the Windows folks are
> > doing. The recommendation from the Intel Windows team was simply to
> > turn hub-initiated LPM off for "all communications devices". Perhaps
> > the Windows USB core is looking for specific USB class codes? Or maybe
> > it has some older API that lets the core know it's a communications
> > device?
> >
> > I'm not really sure we can do it in the USB core with out basically
> > duplicating all the class/PID/VID matching in the communications driver.
> > I think just adding a flag might be the best way. I'm open to
> > suggestions though.
>
> You can detect something as "simple" as a class type, which I bet is all
> that Windows is going to be able to do as well.
>
> > > Or, turn it around the other way, and only enable it if we know it's
> > > safe to do so, in each driver, but I guess that would be even messier.
> >
> > Yeah, I think it would be messier.
>
> Ok, this is probably the best solution for us as well, sorry for the
> noise.

No worries. :)

One of the problems with this patch is that several mailing lists
rejected it because of the large number of recipients in the Cc list,
even when just the mailing lists were on the Cc list. Should I break it
up into a series of patches per driver?

Also, do I need to wait on acks from the other maintainers for this
change? My instinct is no, since it shouldn't change the behavior of
their drivers with the USB devices that are out there today, but I don't
want to step on anyone's toes.

Sarah Sharp

2012-05-18 22:38:46

by Tilman Schmidt

[permalink] [raw]
Subject: Re: [RFC 13/13] USB: Disable hub-initiated LPM for comms devices.

Am 17.05.2012 19:31, schrieb Sarah Sharp:
> On Thu, May 17, 2012 at 07:07:32PM +0200, Tilman Schmidt wrote:
>>
>> I follow the argument for class drivers. But this patch also
>> modifies drivers for specific existing USB 2.0 only devices
>> which are unlikely to ever grow USB 3.0 support, such as the
>> Gigaset ISDN driver:
>>
>>> drivers/isdn/gigaset/bas-gigaset.c | 1 +
>>> drivers/isdn/gigaset/usb-gigaset.c | 1 +
>
> Is there a particular reason why you think that driver is unlikely to
> ever get USB 3.0 support?

Actually, there is. :-)
- The USB devices driven by this driver aren't built anymore.
- Their USB interface design is quite, um, idiosyncratic, and it's
pretty unlikely that anyone will reuse it. (At least I truly hope
no one will.)
- Their successor models have completely different and incompatible
USB interfaces which this driver is unable to handle.

>> What is the interest of setting the disable_hub_initiated_lpm
>> flag for these?
>
> It's partially to lay the foundation for anyone who wants to make a USB
> 3.0 communications driver in the future. They're likely to start from
> some USB 2.0 class driver, and copy a lot of code. If they notice that
> flag is set in all the USB communications class drivers, they're likely
> to set it as well.

You've got a point there.

> I'm not quite sure where the best place to provide documentation on the
> flag is. I've added the kernel doc comments to the structure, but maybe
> it needs to be documented somewhere in Documentation/usb/?

Documentation/usb/power-management.txt would seem like a natural
place. Although it appears to limit itself to "suspending" in its
first paragraph, it does have a section "xHCI hardware link PM"
at the end already, added by Andiry Xu on 2011-09-23.

Hmmm, that section seems to suggest that LPM exists for USB2, too.
Perhaps I should reconsider my attitude towards your patch.

Thanks,
Tilman

--
Tilman Schmidt E-Mail: [email protected]
Bonn, Germany
Diese Nachricht besteht zu 100% aus wiederverwerteten Bits.
Unge?ffnet mindestens haltbar bis: (siehe R?ckseite)


Attachments:
signature.asc (262.00 B)
OpenPGP digital signature