Re: [PATCH v3 1/5] xhci: add a quirk for device disconnection errata for Synopsis Designware USB3 core

[Mathias Nyman]

On 25.01.2015 10:13, Sneeker Yeh wrote:

Oh, and one more thing:

  
 +static void xhci_try_to_clear_csc(struct usb_hcd *hcd, int dev_port_num)
 +{
 + int max_ports;
 + struct xhci_hcd *xhci = hcd_to_xhci(hcd);
 + __le32 __iomem **port_array;
 + u32 status;
 +
 + /* print debug info */
 + if (hcd-speed == HCD_USB3) {
 + max_ports = xhci-num_usb3_ports;
 + port_array = xhci-usb3_ports;
 + } else {
 + max_ports = xhci-num_usb2_ports;
 + port_array = xhci-usb2_ports;
 + }
 +
 + if (dev_port_num  max_ports) {
 + xhci_err(xhci, %s() port number invalid, __func__);
 + return;
 + }
 + status = readl(port_array[dev_port_num - 1]);
 +
 + /* write 1 to clear */
 + if (!(status  PORT_CONNECT)  (status  PORT_CSC))
 + writel(status  PORT_CSC, port_array[0]);

We don't want to write back all the bits we read from the PORTSC register.
Use the xhci_put_state_to_neutral(status) helper function to mask out some RW1C 
bits.

This in addition to writing to the correct PORTSC register as I previously 
mentioned.

-Mathias 
--
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 v4] USB:ftdi_sio: Added Pids for Actisense Usb Devices

[Mark Glover]

From: Mark Glover m...@actisense.com

Only Email comments have changed since the last version of this patch.

These product identifiers (PID) all deal with marine NMEA format data used on 
motor boats and yachts
We supply the programmed devices to Chetco, for use inside their equipment.
The PIDs are a direct copy of our Windows device drivers  (FTDI drivers with 
altered PIDs)

Signed-off-by: Mark Glover m...@actisense.com

---
 drivers/usb/serial/ftdi_sio.c |   17 +
 drivers/usb/serial/ftdi_sio_ids.h |   21 +
 2 files changed, 38 insertions(+)

diff --git a/drivers/usb/serial/ftdi_sio.c b/drivers/usb/serial/ftdi_sio.c
index 1ebb351..651dc1b 100644
--- a/drivers/usb/serial/ftdi_sio.c
+++ b/drivers/usb/serial/ftdi_sio.c
@@ -978,6 +978,23 @@ static const struct usb_device_id id_table_combined[] = {
{ USB_DEVICE_INTERFACE_NUMBER(INFINEON_VID, INFINEON_TRIBOARD_PID, 1) },
/* GE Healthcare devices */
{ USB_DEVICE(GE_HEALTHCARE_VID, GE_HEALTHCARE_NEMO_TRACKER_PID) },
+   /* Active Research (Actisense) devices */
+   { USB_DEVICE(FTDI_VID, ACTISENSE_NDC_PID) },
+   { USB_DEVICE(FTDI_VID, ACTISENSE_USG_PID) },
+   { USB_DEVICE(FTDI_VID, ACTISENSE_NGT_PID) },
+   { USB_DEVICE(FTDI_VID, ACTISENSE_NGW_PID) },
+   { USB_DEVICE(FTDI_VID, ACTISENSE_D9AC_PID) },
+   { USB_DEVICE(FTDI_VID, ACTISENSE_D9AD_PID) },
+   { USB_DEVICE(FTDI_VID, ACTISENSE_D9AE_PID) },
+   { USB_DEVICE(FTDI_VID, ACTISENSE_D9AF_PID) },
+   { USB_DEVICE(FTDI_VID, CHETCO_SEAGAUGE_PID) },
+   { USB_DEVICE(FTDI_VID, CHETCO_SEASWITCH_PID) },
+   { USB_DEVICE(FTDI_VID, CHETCO_SEASMART_NMEA2000_PID) },
+   { USB_DEVICE(FTDI_VID, CHETCO_SEASMART_ETHERNET_PID) },
+   { USB_DEVICE(FTDI_VID, CHETCO_SEASMART_WIFI_PID) },
+   { USB_DEVICE(FTDI_VID, CHETCO_SEASMART_DISPLAY_PID) },
+   { USB_DEVICE(FTDI_VID, CHETCO_SEASMART_LITE_PID) },
+   { USB_DEVICE(FTDI_VID, CHETCO_SEASMART_ANALOG_PID) },
{ } /* Terminating entry */
 };

diff --git a/drivers/usb/serial/ftdi_sio_ids.h 
b/drivers/usb/serial/ftdi_sio_ids.h
index e52409c..87d9ed7 100644
--- a/drivers/usb/serial/ftdi_sio_ids.h
+++ b/drivers/usb/serial/ftdi_sio_ids.h
@@ -1438,3 +1438,23 @@
  */
 #define GE_HEALTHCARE_VID  0x1901
 #define GE_HEALTHCARE_NEMO_TRACKER_PID 0x0015
+
+/*
+ * Active Research (Actisense) devices
+ */
+#define ACTISENSE_NDC_PID  0xD9A8 /* NDC USB Serial Adapter */
+#define ACTISENSE_USG_PID  0xD9A9 /* USG USB Serial Adapter */
+#define ACTISENSE_NGT_PID  0xD9AA /* NGT NMEA2000 Interface */
+#define ACTISENSE_NGW_PID  0xD9AB /* NGW NMEA2000 Gateway */
+#define ACTISENSE_D9AC_PID 0xD9AC /* Actisense Reserved */
+#define ACTISENSE_D9AD_PID 0xD9AD /* Actisense Reserved */
+#define ACTISENSE_D9AE_PID 0xD9AE /* Actisense Reserved */
+#define ACTISENSE_D9AF_PID 0xD9AF /* Actisense Reserved */
+#define CHETCO_SEAGAUGE_PID0xA548 /* SeaGauge USB Adapter */
+#define CHETCO_SEASWITCH_PID   0xA549 /* SeaSwitch USB Adapter */
+#define CHETCO_SEASMART_NMEA2000_PID   0xA54A /* SeaSmart NMEA2000 Gateway */
+#define CHETCO_SEASMART_ETHERNET_PID   0xA54B /* SeaSmart Ethernet Gateway */
+#define CHETCO_SEASMART_WIFI_PID   0xA5AC /* SeaSmart Wifi Gateway */
+#define CHETCO_SEASMART_DISPLAY_PID0xA5AD /* SeaSmart NMEA2000 Display */
+#define CHETCO_SEASMART_LITE_PID   0xA5AE /* SeaSmart Lite USB Adapter */
+#define CHETCO_SEASMART_ANALOG_PID 0xA5AF /* SeaSmart Analog Adapter */
--
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: [PATCH v3 1/5] xhci: add a quirk for device disconnection errata for Synopsis Designware USB3 core

[Mathias Nyman]

On 12.02.2015 17:18, Alan Stern wrote:
 On Thu, 12 Feb 2015, Mathias Nyman wrote:
 
 On 25.01.2015 10:13, Sneeker Yeh wrote:
 This issue is defined by a three-way race at disconnect, between
 1) Class driver interrupt endpoint resheduling attempts if the ISR gave an 
 ep
error event due to device detach (it would try 3 times)
 2) Disconnect interrupt on PORTSC_CSC, which is cleared by hub thread
asynchronously
 3) The hardware IP was configured in silicon with
- DWC_USB3_SUSPEND_ON_DISCONNECT_EN=1
- Synopsys IP version is  3.00a
 The IP will auto-suspend itself on device detach with some phy-specific 
 interval
 after CSC is cleared by 2)

 If 2) and 3) complete before 1), the interrupts it expects will not be 
 generated
 by the autosuspended IP, leading to a deadlock. Even later disconnection
 procedure would detect that corresponding urb is still in-progress and 
 issue a
 ep stop command, auto-suspended IP still won't respond to that command.
 
 If the Synopsys IP provides a way to do it, it would be better to turn
 off the autosuspend feature entirely.  Doesn't autosuspend violate the
 xHCI specification?
 
 So did I understand correctly that the class driver submits a new urb which
 is enqueued by xhci_urb_enqueue() before the hub thread notices the device 
 is disconnected.
 Then hub thread clears CSC bit, controller suspends and the new urb is never 
 given back?

 Doesn't the CSC bit and PORT_CONNECT bit show the device is disconnected 
 when we enter
 xhci_enqueue_urb(), even it the hub thread doesn't know this yet?
 
 What if the device disconnects _after_ the new URB is enqueued?

True, those URBs would be left hanging.

So if possible, tweaking the Synopsis autosuspend feature would be nicer.

If its not possible then I guess a quirk patch like this will do.

-Mathias


 



--
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: gadget: configfs: OS Extended Compatibility descriptors support

[Andrzej Pietrasiewicz]

W dniu 13.02.2015 o 09:06, Dan Carpenter pisze:

Hello Andrzej Pietrasiewicz,



Hello Dan,

Thank you for finding the problem.


The patch da4243145fb1: usb: gadget: configfs: OS Extended
Compatibility descriptors support from May 8, 2014, leads to the
following Smatch warning:

drivers/usb/gadget/configfs.c:1195 interf_grp_sub_compatible_id_store()
error: buffer overflow 'desc-ext_compat_id' 16 = 16



snip



Then we are putting the NULL terminator one space beyond the end of the
array.  -ext_compat_id is set in rndis_alloc_inst().

This is not a false postive, but I'm not positive how we should fix it.



I know how to fix it and will do it soon.

AP

--
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

Kernel 3.6.0 works, newer doesn't

[Tim-Hinnerk Heuer]

Hi there,

Sorry, I'm new to this list, so excuse my newbie question.

I have Linux 3.6.0-030600-generic #201209302035 SMP Mon Oct 1 00:36:01
UTC 2012 x86_64 x86_64 x86_64 GNU/Linux installed and it works great.
My WLAN driver works with this version, but not with any newer version
of the kernel. Am I right that backports can help me with that?

Can I somehow upgrade to a newer version of the kernel and keep my
WIFI USB driver and maybe even upgrade to newer kernels? Or even
better, does someone here have the capacity to fix the problem in
newer kernels?

My Wifi USB Card is the
Asus USB N13

```
lsusb
Bus 002 Device 002: ID 8087:0024 Intel Corp. Integrated Rate Matching Hub
Bus 002 Device 001: ID 1d6b:0002 Linux Foundation 2.0 root hub
Bus 006 Device 001: ID 1d6b:0003 Linux Foundation 3.0 root hub

Bus 005 Device 001: ID 1d6b:0002 Linux Foundation 2.0 root hub
Bus 001 Device 004: ID 0b05:1784 ASUSTek Computer, Inc. USB-N13
802.11n Network Adapter (rev. A1) [Ralink RT3072]

Bus 001 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 004 Device 001: ID 1d6b:0003 Linux Foundation 3.0 root hub

Bus 003 Device 001: ID 1d6b:0002 Linux Foundation 2.0 root hub
```

Thanks for your patience.
Tim

--
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] usb: gadget: configfs: don't NUL-terminate (sub)compatible ids

[Andrzej Pietrasiewicz]

The Extended Compat ID OS Feature Descriptor Specification does not
require the (sub)compatible ids to be NUL-terminated, because they
are placed in a fixed-size buffer and only unused parts of it should
contain NULs. If the buffer is fully utilized, there is no place for NULs.

Consequently, the code which uses desc-ext_compat_id never expects the
data contained to be NUL terminated.

If the compatible id is stored after sub-compatible id, and the compatible
id is full length (8 bytes), the (useless) NUL terminator overwrites the
first byte of the sub-compatible id.

If the sub-compatible id is full length (8 bytes), the (useless) NUL
terminator ends up out of the buffer. The situation can happen in the RNDIS
function, where the buffer is a part of struct f_rndis_opts. The next
member of struct f_rndis_opts is a mutex, so its first byte gets
overwritten. The said byte is a part of a mutex'es member which contains
the information on whether the muext is locked or not. This can lead to a
deadlock, because, in a configfs-composed gadget when a function is linked
into a configuration with config_usb_cfg_link(), usb_get_function()
is called, which then calls rndis_alloc(), which tries locking the same
mutex and (wrongly) finds it already locked.

This patch eliminates NUL terminating of the (sub)compatible id.

Cc: sta...@vger.kernel.org # v3.16+
Fixes: da4243145fb1: usb: gadget: configfs: OS Extended Compatibility 
descriptors support
Reported-by: Dan Carpenter dan.carpen...@oracle.com
Signed-off-by: Andrzej Pietrasiewicz andrze...@samsung.com
---
 drivers/usb/gadget/configfs.c | 2 --
 1 file changed, 2 deletions(-)

diff --git a/drivers/usb/gadget/configfs.c b/drivers/usb/gadget/configfs.c
index 7564814..c42765b3a0 100644
--- a/drivers/usb/gadget/configfs.c
+++ b/drivers/usb/gadget/configfs.c
@@ -1161,7 +1161,6 @@ static ssize_t interf_grp_compatible_id_store(struct 
usb_os_desc *desc,
if (desc-opts_mutex)
mutex_lock(desc-opts_mutex);
memcpy(desc-ext_compat_id, page, l);
-   desc-ext_compat_id[l] = '\0';
 
if (desc-opts_mutex)
mutex_unlock(desc-opts_mutex);
@@ -1192,7 +1191,6 @@ static ssize_t interf_grp_sub_compatible_id_store(struct 
usb_os_desc *desc,
if (desc-opts_mutex)
mutex_lock(desc-opts_mutex);
memcpy(desc-ext_compat_id + 8, page, l);
-   desc-ext_compat_id[l + 8] = '\0';
 
if (desc-opts_mutex)
mutex_unlock(desc-opts_mutex);
-- 
1.9.1

--
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: gadget: configfs: OS Extended Compatibility descriptors support

[Dan Carpenter]

Hello Andrzej Pietrasiewicz,

The patch da4243145fb1: usb: gadget: configfs: OS Extended
Compatibility descriptors support from May 8, 2014, leads to the
following Smatch warning:

drivers/usb/gadget/configfs.c:1195 interf_grp_sub_compatible_id_store()
error: buffer overflow 'desc-ext_compat_id' 16 = 16

drivers/usb/gadget/configfs.c
  1184  static ssize_t interf_grp_sub_compatible_id_store(struct usb_os_desc 
*desc,
  1185const char *page, 
size_t len)
  1186  {
  1187  int l;
  1188  
  1189  l = min_t(int, 8, len);

Let's assume l is 8.

  1190  if (page[l - 1] == '\n')
  1191  --l;
  1192  if (desc-opts_mutex)
  1193  mutex_lock(desc-opts_mutex);
  1194  memcpy(desc-ext_compat_id + 8, page, l);
  1195  desc-ext_compat_id[l + 8] = '\0';

Then we are putting the NULL terminator one space beyond the end of the
array.  -ext_compat_id is set in rndis_alloc_inst().

This is not a false postive, but I'm not positive how we should fix it.

  1196  
  1197  if (desc-opts_mutex)
  1198  mutex_unlock(desc-opts_mutex);
  1199  
  1200  return len;
  1201  }

regards,
dan carpenter
--
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: [PATCH 1/8] usb: add bus type for USB ULPI

[Heikki Krogerus]

On Thu, Feb 12, 2015 at 05:44:20PM -0800, Stephen Boyd wrote:
 On 01/23/15 07:12, Heikki Krogerus wrote:
  diff --git a/scripts/mod/file2alias.c b/scripts/mod/file2alias.c
  index e614ef6..753cb08 100644
  --- a/scripts/mod/file2alias.c
  +++ b/scripts/mod/file2alias.c
  @@ -1176,6 +1176,19 @@ static int do_rio_entry(const char *filename,
   }
   ADD_TO_DEVTABLE(rapidio, rio_device_id, do_rio_entry);
   
  +/* Looks like: mei:S */
 
 This comment doesn't look right.

Oops. Thanks for catching that one.

  +static int do_ulpi_entry(const char *filename, void *symval,
  +char *alias)
  +{
  +   DEF_FIELD(symval, ulpi_device_id, vendor);
  +   DEF_FIELD(symval, ulpi_device_id, product);
  +
  +   sprintf(alias, ulpi:v%04xp%04x, vendor, product);
  +
  +   return 1;
  +}
  +ADD_TO_DEVTABLE(ulpi, ulpi_device_id, do_ulpi_entry);


-- 
heikki
--
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: [PATCH 6/8] usb: dwc3: add ULPI interface support

[Heikki Krogerus]

On Thu, Feb 12, 2015 at 05:41:30PM -0800, David Cohen wrote:
 On Thu, Feb 12, 2015 at 02:12:14PM +0200, Heikki Krogerus wrote:
diff --git a/drivers/usb/dwc3/core.c b/drivers/usb/dwc3/core.c
index a8c9062..66cbf38 100644
--- a/drivers/usb/dwc3/core.c
+++ b/drivers/usb/dwc3/core.c
@@ -879,6 +879,10 @@ static int dwc3_probe(struct platform_device *pdev)
platform_set_drvdata(pdev, dwc);
dwc3_cache_hwparams(dwc);
 
+   ret = dwc3_ulpi_init(dwc);
   
   If I understood correctly, this call will result in enumerating the phy
   via ULPI bus, then registering the correct ULPI device.
   Can you guarantee ULPI will be always accessible at this point if we
   remove dwc3 module and load it again?
  
  OK, got it. So yes, I can guarantee that ULPI will be acessible at
  this point. If we are in a state where we could soft reset dwc3, we
  know we can access ULPI. The fact that dwc3 itself expects to be able
  to write to the ULPI registers at that point guarantees it for us.
 
 I just double checked DWC3 TRM.
 You are correct, by the time we're executing dwc3_core_soft_reset() ULPI
 bus is already accessible. But the TRM also specifies an ULPI phy would
 be reset by DCTL's core soft reset, which is executed by dwc3_core_init()
 before calling dwc3_core_soft_reset(). It does mention DWC3 writes data
 to an ULPI phy register during reset, but it also mentions the clock
 sync happens during that time.
 
 That makes no even OK, but more correct IMO to power on phy before
 core's soft reset (i.e. by ACPI methods). But it lets us in a grey area
 whether ULPI is reliably accessible before core's soft reset.
 
 I believe if you move the dwc3_ulpi_init() to dwc3_core_init(), after
 core's soft reset we've got no more grey area.

Well, we have already requested the phys in dwc3_probe before that so
some refactoring will be needed. It's probable no a problem.

Btw, I'm sorry about telling this so late, but I'm going to be on
vacation for the next two weeks.


Cheers,

-- 
heikki
--
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: [PATCH] hso: fix rx parsing logic when skb allocation fails

[David Miller]

From: Aleksander Morgado aleksan...@aleksander.es
Date: Fri, 13 Feb 2015 14:51:02 +0100

 If skb allocation fails once the IP header has been received, the rx state is
 being set to WAIT_SYNC. The logic, though, shouldn't directly return, as the
 buffer may contain a full packet, and therefore the WAIT_SYNC state needs to 
 be
 processed (resetting state to WAIT_IP, clearing rx_buf_size and 
 re-initializing
 rx_buf_missing).
 
 So, just let the while loop continue so that in the next iteration the 
 WAIT_SYNC
 state cleanly stops the loop. The WAIT_SYNC processing will be done just after
 that, only if the end of packet is flagged.
 
 Signed-off-by: Aleksander Morgado aleksan...@aleksander.es

Applied, thanks.
--
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: Kernel 3.6.0 works, newer doesn't

[Greg KH]

On Fri, Feb 13, 2015 at 11:20:33PM +1300, Tim-Hinnerk Heuer wrote:
 Hi there,
 
 Sorry, I'm new to this list, so excuse my newbie question.
 
 I have Linux 3.6.0-030600-generic #201209302035 SMP Mon Oct 1 00:36:01
 UTC 2012 x86_64 x86_64 x86_64 GNU/Linux installed and it works great.
 My WLAN driver works with this version, but not with any newer version
 of the kernel. Am I right that backports can help me with that?

I suggest working with the driver authors of that driver to fix it to
work with the latest kernel version.  It is merged into the main
kernel.org repo, right?  Which one is it?

The networking mailing list should be able to help you out the best, try
sending this to the linux-wirel...@vger.kernel.org

hope this helps,

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

[PATCH] hso: fix rx parsing logic when skb allocation fails

[Aleksander Morgado]

If skb allocation fails once the IP header has been received, the rx state is
being set to WAIT_SYNC. The logic, though, shouldn't directly return, as the
buffer may contain a full packet, and therefore the WAIT_SYNC state needs to be
processed (resetting state to WAIT_IP, clearing rx_buf_size and re-initializing
rx_buf_missing).

So, just let the while loop continue so that in the next iteration the WAIT_SYNC
state cleanly stops the loop. The WAIT_SYNC processing will be done just after
that, only if the end of packet is flagged.

Signed-off-by: Aleksander Morgado aleksan...@aleksander.es
---
 drivers/net/usb/hso.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/net/usb/hso.c b/drivers/net/usb/hso.c
index 6b8efca..9cdfb3f 100644
--- a/drivers/net/usb/hso.c
+++ b/drivers/net/usb/hso.c
@@ -914,7 +914,7 @@ static void packetizeRx(struct hso_net *odev, unsigned char 
*ip_pkt,
/* We got no receive buffer. */
D1(could not allocate memory);
odev-rx_parse_state = WAIT_SYNC;
-   return;
+   continue;
}
 
/* Copy what we got so far. make room for iphdr
-- 
2.3.0

--
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: [PATCH 8/8] phy: add driver for TI TUSB1210 ULPI PHY

[Felipe Balbi]

On Fri, Feb 13, 2015 at 02:35:16PM +0200, Heikki Krogerus wrote:
 Hi,
 
 On Thu, Feb 12, 2015 at 05:52:42PM -0800, David Cohen wrote:
  Hi Heikki,
  
  Sorry I am starting a new branch on this thread.
  I need to go back to another topic on this same patch.
  
  On Fri, Jan 23, 2015 at 05:12:58PM +0200, Heikki Krogerus wrote:
   TUSB1210 ULPI PHY has vendor specific register for eye
   diagram tuning. On some platforms the system firmware has
   set optimized value to it. In order to not loose the
   optimized value, the driver stores it during probe and
   restores it every time the PHY is powered back on.
   
   Signed-off-by: Heikki Krogerus heikki.kroge...@linux.intel.com
   Cc: Kishon Vijay Abraham I kis...@ti.com
   ---
  
  [snip]
  
   + /* Store initial eye diagram optimisation value */
   + ret = ulpi_read(ulpi, TUSB1210_VENDOR_SPECIFIC2);
  
  We can't rely on eye diagram optimization value here. It's possible the
  phy went through reset (e.g. module unloading/loading).
  We need a more consistent method. Could we use _DSD instead? That would
  be more compatible with DT too.
 
 I'm don't have any problem with getting the value from a property.
 Sounds like the correct thing to do. Are you thinking about putting
 that _DSD method to the ACPI device object for the dwc3? The correct
 place would probable be separate device object for the PHY, but if we
 do that we need to consider how that object is bound to the ulpi
 device. There are few ways we can do that, so it requires some
 thinking.
 
 In any case this driver we can already implement so that it expects to
 get the value from property. We just need the name for it.
 
 The register has actually separate fields for High speed output
 impedance configuration and High speed output driver strength
 configuration for eye diagram tuning, plus control bit for DP/DM swap.
 
 Maybe we should actually get them from three separate properties:
 
 device_property_read_u8(dev, datapolarity, val);
 ...
 device_property_read_u8(dev, zhsdrv, val);
 ...
 device_property_read_u8(dev, ihstx, val);
 ...
 
 What do you think?

it's probably better to pass each value and let the driver calculate the
register contents like this.

-- 
balbi


signature.asc
Description: Digital signature

[PATCH] USB: usbfs: don't leak kernel data in siginfo

[Alan Stern]

When a signal is delivered, the information in the siginfo structure
is copied to userspace.  Good security practice dicatates that the
unused fields in this structure should be initialized to 0 so that
random kernel stack data isn't exposed to the user.  This patch adds
such an initialization to the two places where usbfs raises signals.

Signed-off-by: Alan Stern st...@rowland.harvard.edu
Reported-by: Dave Mielke d...@mielke.cc
CC: sta...@vger.kernel.org

---


[as1776]


 drivers/usb/core/devio.c |2 ++
 1 file changed, 2 insertions(+)

Index: usb-3.19/drivers/usb/core/devio.c
===
--- usb-3.19.orig/drivers/usb/core/devio.c
+++ usb-3.19/drivers/usb/core/devio.c
@@ -501,6 +501,7 @@ static void async_completed(struct urb *
as-status = urb-status;
signr = as-signr;
if (signr) {
+   memset(sinfo, 0, sizeof(sinfo));
sinfo.si_signo = as-signr;
sinfo.si_errno = as-status;
sinfo.si_code = SI_ASYNCIO;
@@ -2382,6 +2383,7 @@ static void usbdev_remove(struct usb_dev
wake_up_all(ps-wait);
list_del_init(ps-list);
if (ps-discsignr) {
+   memset(sinfo, 0, sizeof(sinfo));
sinfo.si_signo = ps-discsignr;
sinfo.si_errno = EPIPE;
sinfo.si_code = SI_ASYNCIO;

--
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/13] usb: host: fotg210: use new USB_RESUME_TIMEOUT

[Felipe Balbi]

Make sure we're using the new macro, so our
resume signaling will always pass certification.

Signed-off-by: Felipe Balbi ba...@ti.com
---
 drivers/usb/host/fotg210-hcd.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/usb/host/fotg210-hcd.c b/drivers/usb/host/fotg210-hcd.c
index ecf02b2623e8..c4794e378ea7 100644
--- a/drivers/usb/host/fotg210-hcd.c
+++ b/drivers/usb/host/fotg210-hcd.c
@@ -1595,7 +1595,7 @@ static int fotg210_hub_control(
/* resume signaling for 20 msec */
fotg210_writel(fotg210, temp | PORT_RESUME, status_reg);
fotg210-reset_done[wIndex] = jiffies
-   + msecs_to_jiffies(20);
+   + msecs_to_jiffies(USB_RESUME_TIMEOUT);
break;
case USB_PORT_FEAT_C_SUSPEND:
clear_bit(wIndex, fotg210-port_c_suspend);
-- 
2.3.0

--
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/13] usb: host: fusbh200: use new USB_RESUME_TIMEOUT

[Felipe Balbi]

Make sure we're using the new macro, so our
resume signaling will always pass certification.

Signed-off-by: Felipe Balbi ba...@ti.com
---
 drivers/usb/host/fusbh200-hcd.c | 3 +--
 1 file changed, 1 insertion(+), 2 deletions(-)

diff --git a/drivers/usb/host/fusbh200-hcd.c b/drivers/usb/host/fusbh200-hcd.c
index 664d2aa1239c..c9eb18b9973c 100644
--- a/drivers/usb/host/fusbh200-hcd.c
+++ b/drivers/usb/host/fusbh200-hcd.c
@@ -1550,10 +1550,9 @@ static int fusbh200_hub_control (
if ((temp  PORT_PE) == 0)
goto error;
 
-   /* resume signaling for 20 msec */
fusbh200_writel(fusbh200, temp | PORT_RESUME, 
status_reg);
fusbh200-reset_done[wIndex] = jiffies
-   + msecs_to_jiffies(20);
+   + msecs_to_jiffies(USB_RESUME_TIMEOUT);
break;
case USB_PORT_FEAT_C_SUSPEND:
clear_bit(wIndex, fusbh200-port_c_suspend);
-- 
2.3.0

--
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 13/13] usb: isp1760: hcd: use new USB_RESUME_TIMEOUT

[Felipe Balbi]

Make sure we're using the new macro, so our
resume signaling will always pass certification.

Signed-off-by: Felipe Balbi ba...@ti.com
---
 drivers/usb/isp1760/isp1760-hcd.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/usb/isp1760/isp1760-hcd.c 
b/drivers/usb/isp1760/isp1760-hcd.c
index 996b2c157d12..c96387abf6e9 100644
--- a/drivers/usb/isp1760/isp1760-hcd.c
+++ b/drivers/usb/isp1760/isp1760-hcd.c
@@ -1871,7 +1871,7 @@ static int isp1760_hub_control(struct usb_hcd *hcd, u16 
typeReq,
reg_write32(hcd-regs, HC_PORTSC1,
temp | PORT_RESUME);
priv-reset_done = jiffies +
-   msecs_to_jiffies(20);
+   msecs_to_jiffies(USB_RESUME_TIMEOUT);
}
break;
case USB_PORT_FEAT_C_SUSPEND:
-- 
2.3.0

--
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 12/13] usb: dwc2: hcd: use new USB_RESUME_TIMEOUT

[Felipe Balbi]

Make sure we're using the new macro, so our
resume signaling will always pass certification.

Signed-off-by: Felipe Balbi ba...@ti.com
---
 drivers/usb/dwc2/hcd.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/usb/dwc2/hcd.c b/drivers/usb/dwc2/hcd.c
index c78c8740db1d..758b7e0380f6 100644
--- a/drivers/usb/dwc2/hcd.c
+++ b/drivers/usb/dwc2/hcd.c
@@ -1521,7 +1521,7 @@ static int dwc2_hcd_hub_control(struct dwc2_hsotg *hsotg, 
u16 typereq,
dev_dbg(hsotg-dev,
ClearPortFeature USB_PORT_FEAT_SUSPEND\n);
writel(0, hsotg-regs + PCGCTL);
-   usleep_range(2, 4);
+   msleep(USB_RESUME_TIMEOUT);
 
hprt0 = dwc2_read_hprt0(hsotg);
hprt0 |= HPRT0_RES;
-- 
2.3.0

--
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/13] usb: host: isp116x: use new USB_RESUME_TIMEOUT

[Felipe Balbi]

Make sure we're using the new macro, so our
resume signaling will always pass certification.

Signed-off-by: Felipe Balbi ba...@ti.com
---
 drivers/usb/host/isp116x-hcd.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/usb/host/isp116x-hcd.c b/drivers/usb/host/isp116x-hcd.c
index 31c9c4d0fa0b..1613a1f69480 100644
--- a/drivers/usb/host/isp116x-hcd.c
+++ b/drivers/usb/host/isp116x-hcd.c
@@ -1487,7 +1487,7 @@ static int isp116x_bus_resume(struct usb_hcd *hcd)
spin_unlock_irq(isp116x-lock);
 
hcd-state = HC_STATE_RESUMING;
-   msleep(20);
+   msleep(USB_RESUME_TIMEOUT);
 
/* Go operational */
spin_lock_irq(isp116x-lock);
-- 
2.3.0

--
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/13] usb: host: ehci: use new USB_RESUME_TIMEOUT

[Felipe Balbi]

Make sure we're using the new macro, so our
resume signaling will always pass certification.

Signed-off-by: Felipe Balbi ba...@ti.com
---
 drivers/usb/host/ehci-hub.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/usb/host/ehci-hub.c b/drivers/usb/host/ehci-hub.c
index 118edb7bdca2..11e487b6fac4 100644
--- a/drivers/usb/host/ehci-hub.c
+++ b/drivers/usb/host/ehci-hub.c
@@ -942,7 +942,7 @@ int ehci_hub_control(
temp = ~PORT_WAKE_BITS;
ehci_writel(ehci, temp | PORT_RESUME, status_reg);
ehci-reset_done[wIndex] = jiffies
-   + msecs_to_jiffies(20);
+   + msecs_to_jiffies(USB_RESUME_TIMEOUT);
set_bit(wIndex, ehci-resuming_ports);
usb_hcd_start_port_resume(hcd-self, wIndex);
break;
-- 
2.3.0

--
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/13] usb: host: r8a66597: use new USB_RESUME_TIMEOUT

[Felipe Balbi]

While this driver was already using a 50ms resume
timeout, let's make sure everybody uses the same
macro so it's easy to fix later should anything
go wrong.

It also gives a more stable expectation to Linux
users.

Signed-off-by: Felipe Balbi ba...@ti.com
---
 drivers/usb/host/r8a66597-hcd.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/usb/host/r8a66597-hcd.c b/drivers/usb/host/r8a66597-hcd.c
index c4bcfaedeec9..2dcf197d304c 100644
--- a/drivers/usb/host/r8a66597-hcd.c
+++ b/drivers/usb/host/r8a66597-hcd.c
@@ -2300,7 +2300,7 @@ static int r8a66597_bus_resume(struct usb_hcd *hcd)
rh-port = ~USB_PORT_STAT_SUSPEND;
rh-port |= USB_PORT_STAT_C_SUSPEND  16;
r8a66597_mdfy(r8a66597, RESUME, RESUME | UACT, dvstctr_reg);
-   msleep(50);
+   msleep(USB_RESUME_TIMEOUT);
r8a66597_mdfy(r8a66597, UACT, RESUME | UACT, dvstctr_reg);
}
 
-- 
2.3.0

--
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 11/13] usb: host: sl811: use new USB_RESUME_TIMEOUT

[Felipe Balbi]

Make sure we're using the new macro, so our
resume signaling will always pass certification.

Signed-off-by: Felipe Balbi ba...@ti.com
---
 drivers/usb/host/sl811-hcd.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/usb/host/sl811-hcd.c b/drivers/usb/host/sl811-hcd.c
index 25fb1da8d3d7..573355498193 100644
--- a/drivers/usb/host/sl811-hcd.c
+++ b/drivers/usb/host/sl811-hcd.c
@@ -1259,7 +1259,7 @@ sl811h_hub_control(
sl811_write(sl811, SL11H_CTLREG1, sl811-ctrl1);
 
mod_timer(sl811-timer, jiffies
-   + msecs_to_jiffies(20));
+   + msecs_to_jiffies(USB_RESUME_TIMEOUT));
break;
case USB_PORT_FEAT_POWER:
port_power(sl811, 0);
-- 
2.3.0

--
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: [PATCH 01/13] usb: define a generic USB_RESUME_TIMEOUT macro

[Felipe Balbi]

On Fri, Feb 13, 2015 at 04:22:12PM -0500, Alan Stern wrote:
 On Fri, 13 Feb 2015, Felipe Balbi wrote:
 
  Every USB Host controller should use this new
  macro to define for how long resume signalling
  should be driven on the bus.
  
  Currently, almost every single USB controller
  is using a 20ms timeout for resume signalling.
  
  That's problematic for two reasons:
  
  a) sometimes that 20ms timer expires a little
  before 20ms, which makes us fail certification
  
  b) some (many) devices actually need more than
  20ms resume signalling.
  
  Sure, in case of (b) we can state that the device
  is against the USB spec, but the fact is that
  we have no control over which device the certification
  lab will use. We also have no control over which host
  they will use. Most likely they'll be using a Windows
  PC which, again, we have no control over how that
  USB stack is written and how long resume signalling
  they are using.
  
  At the end of the day, we must make sure Linux passes
  electrical compliance when working as Host or as Device
  and currently we don't pass compliance as host because
  we're driving resume signallig for exactly 20ms and
  that confuses certification test setup resulting in
  Certification failure.
  
  Signed-off-by: Felipe Balbi ba...@ti.com
  ---
   include/linux/usb.h | 3 +++
   1 file changed, 3 insertions(+)
  
  diff --git a/include/linux/usb.h b/include/linux/usb.h
  index f89c24a03bd9..1dd89f0db344 100644
  --- a/include/linux/usb.h
  +++ b/include/linux/usb.h
  @@ -210,6 +210,9 @@ void usb_put_intf(struct usb_interface *intf);
   #define USB_MAXINTERFACES  32
   #define USB_MAXIADS(USB_MAXINTERFACES/2)
   
  +/* USB Resume Timer */
  +#define USB_RESUME_TIMEOUT 40 /* ms */
  +
 
 How about adding a comment explaining briefly why this value is larger
 than 20 ms?

will do, but wait for other comments before resending.

-- 
balbi


signature.asc
Description: Digital signature

Re: [PATCH 03/13] usb: host: ehci: use new USB_RESUME_TIMEOUT

[Alan Stern]

On Fri, 13 Feb 2015, Felipe Balbi wrote:

 Make sure we're using the new macro, so our
 resume signaling will always pass certification.
 
 Signed-off-by: Felipe Balbi ba...@ti.com
 ---
  drivers/usb/host/ehci-hub.c | 2 +-
  1 file changed, 1 insertion(+), 1 deletion(-)
 
 diff --git a/drivers/usb/host/ehci-hub.c b/drivers/usb/host/ehci-hub.c
 index 118edb7bdca2..11e487b6fac4 100644
 --- a/drivers/usb/host/ehci-hub.c
 +++ b/drivers/usb/host/ehci-hub.c
 @@ -942,7 +942,7 @@ int ehci_hub_control(
   temp = ~PORT_WAKE_BITS;
   ehci_writel(ehci, temp | PORT_RESUME, status_reg);
   ehci-reset_done[wIndex] = jiffies
 - + msecs_to_jiffies(20);
 + + msecs_to_jiffies(USB_RESUME_TIMEOUT);
   set_bit(wIndex, ehci-resuming_ports);
   usb_hcd_start_port_resume(hcd-self, wIndex);
   break;

You missed a second occurrence in ehci_irq(), just following a comment 
that says:

/* start 20 msec resume signaling from this port,
 * and make hub_wq collect PORT_STAT_C_SUSPEND to
 * stop that signaling.  Use 5 ms extra for safety,
 * like usb_port_resume() does.
 */
 
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: [PATCH 04/13] usb: host: uhci: use new USB_RESUME_TIMEOUT

[Felipe Balbi]

On Fri, Feb 13, 2015 at 03:38:04PM -0600, Felipe Balbi wrote:
 On Fri, Feb 13, 2015 at 04:30:15PM -0500, Alan Stern wrote:
  On Fri, 13 Feb 2015, Felipe Balbi wrote:
  
   Make sure we're using the new macro, so our
   resume signaling will always pass certification.
   
   Signed-off-by: Felipe Balbi ba...@ti.com
   ---
drivers/usb/host/uhci-hub.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
   
   diff --git a/drivers/usb/host/uhci-hub.c b/drivers/usb/host/uhci-hub.c
   index 93e17b12fb33..fd809e2cace1 100644
   --- a/drivers/usb/host/uhci-hub.c
   +++ b/drivers/usb/host/uhci-hub.c
   @@ -165,7 +165,7 @@ static void uhci_check_ports(struct uhci_hcd *uhci)
 /* Port received a wakeup request */
 set_bit(port, uhci-resuming_ports);
 uhci-ports_timeout = jiffies +
   - msecs_to_jiffies(25);
   + msecs_to_jiffies(USB_RESUME_TIMEOUT);
 usb_hcd_start_port_resume(
 uhci_to_hcd(uhci)-self, port);
  
  You missed a second occurrence in uhci_hub_control():
  
  else
  /* USB v2.0 7.1.7.7 */
  uhci-ports_timeout = jiffies +
  msecs_to_jiffies(20);
 
 fixed:

and I've added a patch to hub.c too:

commit 314fa03c55a333552178e57008609ed3b2f06415
Author: Felipe Balbi ba...@ti.com
Date:   Fri Feb 13 15:38:33 2015 -0600

usb: core: hub: use new USB_RESUME_TIMEOUT

Make sure we're using the new macro, so our
resume signaling will always pass certification.

Signed-off-by: Felipe Balbi ba...@ti.com

diff --git a/drivers/usb/core/hub.c b/drivers/usb/core/hub.c
index d7c3d5a35946..3b7151687776 100644
--- a/drivers/usb/core/hub.c
+++ b/drivers/usb/core/hub.c
@@ -3406,10 +3406,10 @@ int usb_port_resume(struct usb_device *udev, 
pm_message_t msg)
if (status) {
dev_dbg(port_dev-dev, can't resume, status %d\n, status);
} else {
-   /* drive resume for at least 20 msec */
+   /* drive resume for USB_RESUME_TIMEOUT msec */
dev_dbg(udev-dev, usb %sresume\n,
(PMSG_IS_AUTO(msg) ? auto- : ));
-   msleep(25);
+   msleep(USB_RESUME_TIMEOUT);
 
/* Virtual root hubs can trigger on GET_PORT_STATUS to
 * stop resume signaling.  Then finish the resume


I'll wait until Monday or Tuesday for more comments before resending.
All patches are available on my k.org account if anybody wants to test:

git://git.kernel.org/pub/scm/linux/kernel/git/balbi/usb.git 
usb-generic-resume-timeout

-- 
balbi


signature.asc
Description: Digital signature

Re: [PATCH 8/8] phy: add driver for TI TUSB1210 ULPI PHY

[David Cohen]

Hi Felipe,

[snip]

 diff --git a/drivers/usb/dwc3/dwc3-pci.c b/drivers/usb/dwc3/dwc3-pci.c
 index 8d95056..53902ea 100644
 --- a/drivers/usb/dwc3/dwc3-pci.c
 +++ b/drivers/usb/dwc3/dwc3-pci.c
 @@ -21,6 +21,7 @@
  #include linux/slab.h
  #include linux/pci.h
  #include linux/platform_device.h
 +#include linux/gpio/consumer.h
  
  #include platform_data.h
  
 @@ -35,6 +36,24 @@
  
  static int dwc3_pci_quirks(struct pci_dev *pdev)
  {
 + if (pdev-vendor == PCI_VENDOR_ID_INTEL 
 + pdev-device == PCI_DEVICE_ID_INTEL_BYT) {
 + struct gpio_desc *gpio;
 +
 + gpio = gpiod_get_index(pdev-dev, reset, 0);
 + if (!IS_ERR(gpio)) {
 + gpiod_direction_output(gpio, 0);
 + gpiod_set_value_cansleep(gpio, 1);
 + gpiod_put(gpio);
 + }
 + gpio = gpiod_get_index(pdev-dev, cs, 1);
 + if (!IS_ERR(gpio)) {
 + gpiod_direction_output(gpio, 0);
 + gpiod_set_value_cansleep(gpio, 1);
 + gpiod_put(gpio);
 + }
 + }
 +

A lot has been discussed in other branches of this thread.

But in resume, this is the last open point to make Heikki's proposal
good on my side. If you accept this ugly quirk (but necessary for
current BYT-CR products when ULPI bus enumerates phy), everything seems
good to me. If you don't accept, we need to figure out a way to get the
platform driver code back to give gpio to phy as platform data in a way
that it could live together with ULPI bus (BYT-CR needs the ULPI bus too).

Br, David

   if (pdev-vendor == PCI_VENDOR_ID_AMD 
   pdev-device == PCI_DEVICE_ID_AMD_NL_USB) {
   struct dwc3_platform_data pdata;

--
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/13] usb: host: xhci: use new USB_RESUME_TIMEOUT

[Felipe Balbi]

Make sure we're using the new macro, so our
resume signaling will always pass certification.

Signed-off-by: Felipe Balbi ba...@ti.com
---
 drivers/usb/host/xhci-ring.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/usb/host/xhci-ring.c b/drivers/usb/host/xhci-ring.c
index e692e769c50c..775bb54ef277 100644
--- a/drivers/usb/host/xhci-ring.c
+++ b/drivers/usb/host/xhci-ring.c
@@ -1574,7 +1574,7 @@ static void handle_port_status(struct xhci_hcd *xhci,
} else {
xhci_dbg(xhci, resume HS port %d\n, port_id);
bus_state-resume_done[faked_port_index] = jiffies +
-   msecs_to_jiffies(20);
+   msecs_to_jiffies(USB_RESUME_TIMEOUT);
set_bit(faked_port_index, bus_state-resuming_ports);
mod_timer(hcd-rh_timer,
  bus_state-resume_done[faked_port_index]);
-- 
2.3.0

--
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/13] increase resume timeout for all controllers

[Felipe Balbi]

Hi all,

Because every year or so we go through some brokeness
related to resume signalling, let's make sure everybody
uses a single resume signalling timeout value which is
rather large (40ms as of now).

The reasons for doing this are explained in patch 1 of
the series.

Series has been tested with MUSB and DWC3 on at least
4 different boards. More testers are very welcome.

Felipe Balbi (13):
  usb: define a generic USB_RESUME_TIMEOUT macro
  usb: host: xhci: use new USB_RESUME_TIMEOUT
  usb: host: ehci: use new USB_RESUME_TIMEOUT
  usb: host: uhci: use new USB_RESUME_TIMEOUT
  usb: musb: use new USB_RESUME_TIMEOUT
  usb: host: isp116x: use new USB_RESUME_TIMEOUT
  usb: host: fotg210: use new USB_RESUME_TIMEOUT
  usb: host: fusbh200: use new USB_RESUME_TIMEOUT
  usb: host: oxu210hp: use new USB_RESUME_TIMEOUT
  usb: host: r8a66597: use new USB_RESUME_TIMEOUT
  usb: host: sl811: use new USB_RESUME_TIMEOUT
  usb: dwc2: hcd: use new USB_RESUME_TIMEOUT
  usb: isp1760: hcd: use new USB_RESUME_TIMEOUT

 drivers/usb/dwc2/hcd.c| 2 +-
 drivers/usb/host/ehci-hub.c   | 2 +-
 drivers/usb/host/fotg210-hcd.c| 2 +-
 drivers/usb/host/fusbh200-hcd.c   | 3 +--
 drivers/usb/host/isp116x-hcd.c| 2 +-
 drivers/usb/host/oxu210hp-hcd.c   | 7 ---
 drivers/usb/host/r8a66597-hcd.c   | 2 +-
 drivers/usb/host/sl811-hcd.c  | 2 +-
 drivers/usb/host/uhci-hub.c   | 2 +-
 drivers/usb/host/xhci-ring.c  | 2 +-
 drivers/usb/isp1760/isp1760-hcd.c | 2 +-
 drivers/usb/musb/musb_core.c  | 5 +++--
 drivers/usb/musb/musb_virthub.c   | 2 +-
 include/linux/usb.h   | 3 +++
 14 files changed, 21 insertions(+), 17 deletions(-)

-- 
2.3.0

--
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/13] usb: musb: use new USB_RESUME_TIMEOUT

[Felipe Balbi]

Make sure we're using the new macro, so our
resume signaling will always pass certification.

Based on original work by Bin Liu Bin Liu b-...@ti.com

Cc: Bin Liu b-...@ti.com
Signed-off-by: Felipe Balbi ba...@ti.com
---
 drivers/usb/musb/musb_core.c| 5 +++--
 drivers/usb/musb/musb_virthub.c | 2 +-
 2 files changed, 4 insertions(+), 3 deletions(-)

diff --git a/drivers/usb/musb/musb_core.c b/drivers/usb/musb/musb_core.c
index e6f4cbfeed97..799d383095d0 100644
--- a/drivers/usb/musb/musb_core.c
+++ b/drivers/usb/musb/musb_core.c
@@ -99,6 +99,7 @@
 #include linux/platform_device.h
 #include linux/io.h
 #include linux/dma-mapping.h
+#include linux/usb.h
 
 #include musb_core.h
 
@@ -2461,7 +2462,7 @@ static int musb_resume(struct device *dev)
if (musb-need_finish_resume) {
musb-need_finish_resume = 0;
schedule_delayed_work(musb-finish_resume_work,
- msecs_to_jiffies(20));
+ msecs_to_jiffies(USB_RESUME_TIMEOUT));
}
 
/*
@@ -2504,7 +2505,7 @@ static int musb_runtime_resume(struct device *dev)
if (musb-need_finish_resume) {
musb-need_finish_resume = 0;
schedule_delayed_work(musb-finish_resume_work,
-   msecs_to_jiffies(20));
+   msecs_to_jiffies(USB_RESUME_TIMEOUT));
}
 
return 0;
diff --git a/drivers/usb/musb/musb_virthub.c b/drivers/usb/musb/musb_virthub.c
index 294e159f4afe..5428ed11440d 100644
--- a/drivers/usb/musb/musb_virthub.c
+++ b/drivers/usb/musb/musb_virthub.c
@@ -136,7 +136,7 @@ void musb_port_suspend(struct musb *musb, bool do_suspend)
/* later, GetPortStatus will stop RESUME signaling */
musb-port1_status |= MUSB_PORT_STAT_RESUME;
schedule_delayed_work(musb-finish_resume_work,
- msecs_to_jiffies(20));
+ msecs_to_jiffies(USB_RESUME_TIMEOUT));
}
 }
 
-- 
2.3.0

--
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 04/13] usb: host: uhci: use new USB_RESUME_TIMEOUT

[Felipe Balbi]

Make sure we're using the new macro, so our
resume signaling will always pass certification.

Signed-off-by: Felipe Balbi ba...@ti.com
---
 drivers/usb/host/uhci-hub.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/usb/host/uhci-hub.c b/drivers/usb/host/uhci-hub.c
index 93e17b12fb33..fd809e2cace1 100644
--- a/drivers/usb/host/uhci-hub.c
+++ b/drivers/usb/host/uhci-hub.c
@@ -165,7 +165,7 @@ static void uhci_check_ports(struct uhci_hcd *uhci)
/* Port received a wakeup request */
set_bit(port, uhci-resuming_ports);
uhci-ports_timeout = jiffies +
-   msecs_to_jiffies(25);
+   msecs_to_jiffies(USB_RESUME_TIMEOUT);
usb_hcd_start_port_resume(
uhci_to_hcd(uhci)-self, port);
 
-- 
2.3.0

--
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/13] usb: define a generic USB_RESUME_TIMEOUT macro

[Felipe Balbi]

Every USB Host controller should use this new
macro to define for how long resume signalling
should be driven on the bus.

Currently, almost every single USB controller
is using a 20ms timeout for resume signalling.

That's problematic for two reasons:

a) sometimes that 20ms timer expires a little
before 20ms, which makes us fail certification

b) some (many) devices actually need more than
20ms resume signalling.

Sure, in case of (b) we can state that the device
is against the USB spec, but the fact is that
we have no control over which device the certification
lab will use. We also have no control over which host
they will use. Most likely they'll be using a Windows
PC which, again, we have no control over how that
USB stack is written and how long resume signalling
they are using.

At the end of the day, we must make sure Linux passes
electrical compliance when working as Host or as Device
and currently we don't pass compliance as host because
we're driving resume signallig for exactly 20ms and
that confuses certification test setup resulting in
Certification failure.

Signed-off-by: Felipe Balbi ba...@ti.com
---
 include/linux/usb.h | 3 +++
 1 file changed, 3 insertions(+)

diff --git a/include/linux/usb.h b/include/linux/usb.h
index f89c24a03bd9..1dd89f0db344 100644
--- a/include/linux/usb.h
+++ b/include/linux/usb.h
@@ -210,6 +210,9 @@ void usb_put_intf(struct usb_interface *intf);
 #define USB_MAXINTERFACES  32
 #define USB_MAXIADS(USB_MAXINTERFACES/2)
 
+/* USB Resume Timer */
+#define USB_RESUME_TIMEOUT 40 /* ms */
+
 /**
  * struct usb_interface_cache - long-term representation of a device interface
  * @num_altsetting: number of altsettings defined.
-- 
2.3.0

--
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: [PATCH 03/13] usb: host: ehci: use new USB_RESUME_TIMEOUT

[Felipe Balbi]

On Fri, Feb 13, 2015 at 04:25:53PM -0500, Alan Stern wrote:
 On Fri, 13 Feb 2015, Felipe Balbi wrote:
 
  Make sure we're using the new macro, so our
  resume signaling will always pass certification.
  
  Signed-off-by: Felipe Balbi ba...@ti.com
  ---
   drivers/usb/host/ehci-hub.c | 2 +-
   1 file changed, 1 insertion(+), 1 deletion(-)
  
  diff --git a/drivers/usb/host/ehci-hub.c b/drivers/usb/host/ehci-hub.c
  index 118edb7bdca2..11e487b6fac4 100644
  --- a/drivers/usb/host/ehci-hub.c
  +++ b/drivers/usb/host/ehci-hub.c
  @@ -942,7 +942,7 @@ int ehci_hub_control(
  temp = ~PORT_WAKE_BITS;
  ehci_writel(ehci, temp | PORT_RESUME, status_reg);
  ehci-reset_done[wIndex] = jiffies
  -   + msecs_to_jiffies(20);
  +   + msecs_to_jiffies(USB_RESUME_TIMEOUT);
  set_bit(wIndex, ehci-resuming_ports);
  usb_hcd_start_port_resume(hcd-self, wIndex);
  break;
 
 You missed a second occurrence in ehci_irq(), just following a comment 
 that says:
 
   /* start 20 msec resume signaling from this port,
* and make hub_wq collect PORT_STAT_C_SUSPEND to
* stop that signaling.  Use 5 ms extra for safety,
* like usb_port_resume() does.
*/

fixed:

commit 4639c95ca9d62b3146ef6809687f2a962e167771
Author: Felipe Balbi ba...@ti.com
Date:   Fri Feb 13 14:42:25 2015 -0600

usb: host: ehci: use new USB_RESUME_TIMEOUT

Make sure we're using the new macro, so our
resume signaling will always pass certification.

Signed-off-by: Felipe Balbi ba...@ti.com

diff --git a/drivers/usb/host/ehci-hcd.c b/drivers/usb/host/ehci-hcd.c
index 85e56d1abd23..f4d88dfb26a7 100644
--- a/drivers/usb/host/ehci-hcd.c
+++ b/drivers/usb/host/ehci-hcd.c
@@ -792,12 +792,12 @@ static irqreturn_t ehci_irq (struct usb_hcd *hcd)
ehci-reset_done[i] == 0))
continue;
 
-   /* start 20 msec resume signaling from this port,
-* and make hub_wq collect PORT_STAT_C_SUSPEND to
-* stop that signaling.  Use 5 ms extra for safety,
-* like usb_port_resume() does.
+   /* start USB_RESUME_TIMEOUT msec resume signaling from
+* this port, and make hub_wq collect
+* PORT_STAT_C_SUSPEND to stop that signaling.
 */
-   ehci-reset_done[i] = jiffies + msecs_to_jiffies(25);
+   ehci-reset_done[i] = jiffies +
+   msecs_to_jiffies(USB_RESUME_TIMEOUT);
set_bit(i, ehci-resuming_ports);
ehci_dbg (ehci, port %d remote wakeup\n, i + 1);
usb_hcd_start_port_resume(hcd-self, i);
diff --git a/drivers/usb/host/ehci-hub.c b/drivers/usb/host/ehci-hub.c
index 87cf86f38b36..b303a4b978f1 100644
--- a/drivers/usb/host/ehci-hub.c
+++ b/drivers/usb/host/ehci-hub.c
@@ -942,7 +942,7 @@ int ehci_hub_control(
temp = ~PORT_WAKE_BITS;
ehci_writel(ehci, temp | PORT_RESUME, status_reg);
ehci-reset_done[wIndex] = jiffies
-   + msecs_to_jiffies(20);
+   + msecs_to_jiffies(USB_RESUME_TIMEOUT);
set_bit(wIndex, ehci-resuming_ports);
usb_hcd_start_port_resume(hcd-self, wIndex);
break;

-- 
balbi


signature.asc
Description: Digital signature

Re: [PATCH 03/13] usb: host: ehci: use new USB_RESUME_TIMEOUT

[Alan Stern]

On Fri, 13 Feb 2015, Alan Stern wrote:

 On Fri, 13 Feb 2015, Felipe Balbi wrote:
 
  Make sure we're using the new macro, so our
  resume signaling will always pass certification.
  
  Signed-off-by: Felipe Balbi ba...@ti.com
  ---
   drivers/usb/host/ehci-hub.c | 2 +-
   1 file changed, 1 insertion(+), 1 deletion(-)
  
  diff --git a/drivers/usb/host/ehci-hub.c b/drivers/usb/host/ehci-hub.c
  index 118edb7bdca2..11e487b6fac4 100644
  --- a/drivers/usb/host/ehci-hub.c
  +++ b/drivers/usb/host/ehci-hub.c
  @@ -942,7 +942,7 @@ int ehci_hub_control(
  temp = ~PORT_WAKE_BITS;
  ehci_writel(ehci, temp | PORT_RESUME, status_reg);
  ehci-reset_done[wIndex] = jiffies
  -   + msecs_to_jiffies(20);
  +   + msecs_to_jiffies(USB_RESUME_TIMEOUT);
  set_bit(wIndex, ehci-resuming_ports);
  usb_hcd_start_port_resume(hcd-self, wIndex);
  break;
 
 You missed a second occurrence in ehci_irq(), just following a comment 
 that says:
 
   /* start 20 msec resume signaling from this port,
* and make hub_wq collect PORT_STAT_C_SUSPEND to
* stop that signaling.  Use 5 ms extra for safety,
* like usb_port_resume() does.
*/

There's a third occurrence as well, in ehci_bus_resume():

/* msleep for 20ms only if code is trying to resume port */
if (resume_needed) {
spin_unlock_irq(ehci-lock);
msleep(20);
spin_lock_irq(ehci-lock);
if (ehci-shutdown)
goto shutdown;
}

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: [PATCH 6/8] usb: dwc3: add ULPI interface support

[Felipe Balbi]

On Fri, Feb 13, 2015 at 02:03:45PM -0800, David Cohen wrote:
 Hi Heikki,
 
 On Fri, Feb 13, 2015 at 03:16:40PM +0200, Heikki Krogerus wrote:
  On Thu, Feb 12, 2015 at 05:41:30PM -0800, David Cohen wrote:
   On Thu, Feb 12, 2015 at 02:12:14PM +0200, Heikki Krogerus wrote:
  diff --git a/drivers/usb/dwc3/core.c b/drivers/usb/dwc3/core.c
  index a8c9062..66cbf38 100644
  --- a/drivers/usb/dwc3/core.c
  +++ b/drivers/usb/dwc3/core.c
  @@ -879,6 +879,10 @@ static int dwc3_probe(struct platform_device 
  *pdev)
  platform_set_drvdata(pdev, dwc);
  dwc3_cache_hwparams(dwc);
   
  +   ret = dwc3_ulpi_init(dwc);
 
 If I understood correctly, this call will result in enumerating the 
 phy
 via ULPI bus, then registering the correct ULPI device.
 Can you guarantee ULPI will be always accessible at this point if we
 remove dwc3 module and load it again?

OK, got it. So yes, I can guarantee that ULPI will be acessible at
this point. If we are in a state where we could soft reset dwc3, we
know we can access ULPI. The fact that dwc3 itself expects to be able
to write to the ULPI registers at that point guarantees it for us.
   
   I just double checked DWC3 TRM.
   You are correct, by the time we're executing dwc3_core_soft_reset() ULPI
   bus is already accessible. But the TRM also specifies an ULPI phy would
   be reset by DCTL's core soft reset, which is executed by dwc3_core_init()
   before calling dwc3_core_soft_reset(). It does mention DWC3 writes data
   to an ULPI phy register during reset, but it also mentions the clock
   sync happens during that time.
   
   That makes no even OK, but more correct IMO to power on phy before
   core's soft reset (i.e. by ACPI methods). But it lets us in a grey area
   whether ULPI is reliably accessible before core's soft reset.
   
   I believe if you move the dwc3_ulpi_init() to dwc3_core_init(), after
   core's soft reset we've got no more grey area.
  
  Well, we have already requested the phys in dwc3_probe before that so
  some refactoring will be needed. It's probable no a problem.
 
 Sounds good :)
 
  
  Btw, I'm sorry about telling this so late, but I'm going to be on
  vacation for the next two weeks.
 
 Thanks for the notice.

you'll be back right around merge window closing time :-) Not to worry,
we'll have time to get this into the next merge window.

-- 
balbi


signature.asc
Description: Digital signature

Re: [PATCH 01/13] usb: define a generic USB_RESUME_TIMEOUT macro

[Alan Stern]

On Fri, 13 Feb 2015, Felipe Balbi wrote:

 Every USB Host controller should use this new
 macro to define for how long resume signalling
 should be driven on the bus.
 
 Currently, almost every single USB controller
 is using a 20ms timeout for resume signalling.
 
 That's problematic for two reasons:
 
 a) sometimes that 20ms timer expires a little
 before 20ms, which makes us fail certification
 
 b) some (many) devices actually need more than
 20ms resume signalling.
 
 Sure, in case of (b) we can state that the device
 is against the USB spec, but the fact is that
 we have no control over which device the certification
 lab will use. We also have no control over which host
 they will use. Most likely they'll be using a Windows
 PC which, again, we have no control over how that
 USB stack is written and how long resume signalling
 they are using.
 
 At the end of the day, we must make sure Linux passes
 electrical compliance when working as Host or as Device
 and currently we don't pass compliance as host because
 we're driving resume signallig for exactly 20ms and
 that confuses certification test setup resulting in
 Certification failure.
 
 Signed-off-by: Felipe Balbi ba...@ti.com
 ---
  include/linux/usb.h | 3 +++
  1 file changed, 3 insertions(+)
 
 diff --git a/include/linux/usb.h b/include/linux/usb.h
 index f89c24a03bd9..1dd89f0db344 100644
 --- a/include/linux/usb.h
 +++ b/include/linux/usb.h
 @@ -210,6 +210,9 @@ void usb_put_intf(struct usb_interface *intf);
  #define USB_MAXINTERFACES32
  #define USB_MAXIADS  (USB_MAXINTERFACES/2)
  
 +/* USB Resume Timer */
 +#define USB_RESUME_TIMEOUT   40 /* ms */
 +

How about adding a comment explaining briefly why this value is larger
than 20 ms?

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: [PATCH 04/13] usb: host: uhci: use new USB_RESUME_TIMEOUT

[Alan Stern]

On Fri, 13 Feb 2015, Felipe Balbi wrote:

 Make sure we're using the new macro, so our
 resume signaling will always pass certification.
 
 Signed-off-by: Felipe Balbi ba...@ti.com
 ---
  drivers/usb/host/uhci-hub.c | 2 +-
  1 file changed, 1 insertion(+), 1 deletion(-)
 
 diff --git a/drivers/usb/host/uhci-hub.c b/drivers/usb/host/uhci-hub.c
 index 93e17b12fb33..fd809e2cace1 100644
 --- a/drivers/usb/host/uhci-hub.c
 +++ b/drivers/usb/host/uhci-hub.c
 @@ -165,7 +165,7 @@ static void uhci_check_ports(struct uhci_hcd *uhci)
   /* Port received a wakeup request */
   set_bit(port, uhci-resuming_ports);
   uhci-ports_timeout = jiffies +
 - msecs_to_jiffies(25);
 + msecs_to_jiffies(USB_RESUME_TIMEOUT);
   usb_hcd_start_port_resume(
   uhci_to_hcd(uhci)-self, port);

You missed a second occurrence in uhci_hub_control():

else
/* USB v2.0 7.1.7.7 */
uhci-ports_timeout = jiffies +
msecs_to_jiffies(20);

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: [PATCH 04/13] usb: host: uhci: use new USB_RESUME_TIMEOUT

[Felipe Balbi]

On Fri, Feb 13, 2015 at 04:30:15PM -0500, Alan Stern wrote:
 On Fri, 13 Feb 2015, Felipe Balbi wrote:
 
  Make sure we're using the new macro, so our
  resume signaling will always pass certification.
  
  Signed-off-by: Felipe Balbi ba...@ti.com
  ---
   drivers/usb/host/uhci-hub.c | 2 +-
   1 file changed, 1 insertion(+), 1 deletion(-)
  
  diff --git a/drivers/usb/host/uhci-hub.c b/drivers/usb/host/uhci-hub.c
  index 93e17b12fb33..fd809e2cace1 100644
  --- a/drivers/usb/host/uhci-hub.c
  +++ b/drivers/usb/host/uhci-hub.c
  @@ -165,7 +165,7 @@ static void uhci_check_ports(struct uhci_hcd *uhci)
  /* Port received a wakeup request */
  set_bit(port, uhci-resuming_ports);
  uhci-ports_timeout = jiffies +
  -   msecs_to_jiffies(25);
  +   msecs_to_jiffies(USB_RESUME_TIMEOUT);
  usb_hcd_start_port_resume(
  uhci_to_hcd(uhci)-self, port);
 
 You missed a second occurrence in uhci_hub_control():
 
   else
   /* USB v2.0 7.1.7.7 */
   uhci-ports_timeout = jiffies +
   msecs_to_jiffies(20);

fixed:

commit 9b0fd8b46f4621257ee42bd89d58ca41c12e0a91
Author: Felipe Balbi ba...@ti.com
Date:   Fri Feb 13 14:44:17 2015 -0600

usb: host: uhci: use new USB_RESUME_TIMEOUT

Make sure we're using the new macro, so our
resume signaling will always pass certification.

Signed-off-by: Felipe Balbi ba...@ti.com

diff --git a/drivers/usb/host/uhci-hub.c b/drivers/usb/host/uhci-hub.c
index 19ba5eafb31e..7b3d1afcc14a 100644
--- a/drivers/usb/host/uhci-hub.c
+++ b/drivers/usb/host/uhci-hub.c
@@ -166,7 +166,7 @@ static void uhci_check_ports(struct uhci_hcd *uhci)
/* Port received a wakeup request */
set_bit(port, uhci-resuming_ports);
uhci-ports_timeout = jiffies +
-   msecs_to_jiffies(25);
+   msecs_to_jiffies(USB_RESUME_TIMEOUT);
usb_hcd_start_port_resume(
uhci_to_hcd(uhci)-self, port);
 
@@ -338,7 +338,8 @@ static int uhci_hub_control(struct usb_hcd *hcd, u16 
typeReq, u16 wValue,
uhci_finish_suspend(uhci, port, port_addr);
 
/* USB v2.0 7.1.7.5 */
-   uhci-ports_timeout = jiffies + msecs_to_jiffies(50);
+   uhci-ports_timeout = jiffies +
+   msecs_to_jiffies(USB_RESUME_TIMEOUT);
break;
case USB_PORT_FEAT_POWER:
/* UHCI has no power switching */

-- 
balbi


signature.asc
Description: Digital signature

Re: [PATCH 6/8] usb: dwc3: add ULPI interface support

[David Cohen]

Hi Heikki,

On Fri, Feb 13, 2015 at 03:16:40PM +0200, Heikki Krogerus wrote:
 On Thu, Feb 12, 2015 at 05:41:30PM -0800, David Cohen wrote:
  On Thu, Feb 12, 2015 at 02:12:14PM +0200, Heikki Krogerus wrote:
 diff --git a/drivers/usb/dwc3/core.c b/drivers/usb/dwc3/core.c
 index a8c9062..66cbf38 100644
 --- a/drivers/usb/dwc3/core.c
 +++ b/drivers/usb/dwc3/core.c
 @@ -879,6 +879,10 @@ static int dwc3_probe(struct platform_device 
 *pdev)
   platform_set_drvdata(pdev, dwc);
   dwc3_cache_hwparams(dwc);
  
 + ret = dwc3_ulpi_init(dwc);

If I understood correctly, this call will result in enumerating the phy
via ULPI bus, then registering the correct ULPI device.
Can you guarantee ULPI will be always accessible at this point if we
remove dwc3 module and load it again?
   
   OK, got it. So yes, I can guarantee that ULPI will be acessible at
   this point. If we are in a state where we could soft reset dwc3, we
   know we can access ULPI. The fact that dwc3 itself expects to be able
   to write to the ULPI registers at that point guarantees it for us.
  
  I just double checked DWC3 TRM.
  You are correct, by the time we're executing dwc3_core_soft_reset() ULPI
  bus is already accessible. But the TRM also specifies an ULPI phy would
  be reset by DCTL's core soft reset, which is executed by dwc3_core_init()
  before calling dwc3_core_soft_reset(). It does mention DWC3 writes data
  to an ULPI phy register during reset, but it also mentions the clock
  sync happens during that time.
  
  That makes no even OK, but more correct IMO to power on phy before
  core's soft reset (i.e. by ACPI methods). But it lets us in a grey area
  whether ULPI is reliably accessible before core's soft reset.
  
  I believe if you move the dwc3_ulpi_init() to dwc3_core_init(), after
  core's soft reset we've got no more grey area.
 
 Well, we have already requested the phys in dwc3_probe before that so
 some refactoring will be needed. It's probable no a problem.

Sounds good :)

 
 Btw, I'm sorry about telling this so late, but I'm going to be on
 vacation for the next two weeks.

Thanks for the notice.

Br, David

 
 
 Cheers,
 
 -- 
 heikki
--
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: [PATCH 8/8] phy: add driver for TI TUSB1210 ULPI PHY

[Felipe Balbi]

On Fri, Feb 13, 2015 at 02:02:11PM -0800, David Cohen wrote:
 Hi Felipe,
 
 [snip]
 
  diff --git a/drivers/usb/dwc3/dwc3-pci.c b/drivers/usb/dwc3/dwc3-pci.c
  index 8d95056..53902ea 100644
  --- a/drivers/usb/dwc3/dwc3-pci.c
  +++ b/drivers/usb/dwc3/dwc3-pci.c
  @@ -21,6 +21,7 @@
   #include linux/slab.h
   #include linux/pci.h
   #include linux/platform_device.h
  +#include linux/gpio/consumer.h
   
   #include platform_data.h
   
  @@ -35,6 +36,24 @@
   
   static int dwc3_pci_quirks(struct pci_dev *pdev)
   {
  +   if (pdev-vendor == PCI_VENDOR_ID_INTEL 
  +   pdev-device == PCI_DEVICE_ID_INTEL_BYT) {
  +   struct gpio_desc *gpio;
  +
  +   gpio = gpiod_get_index(pdev-dev, reset, 0);
  +   if (!IS_ERR(gpio)) {
  +   gpiod_direction_output(gpio, 0);
  +   gpiod_set_value_cansleep(gpio, 1);
  +   gpiod_put(gpio);
  +   }
  +   gpio = gpiod_get_index(pdev-dev, cs, 1);
  +   if (!IS_ERR(gpio)) {
  +   gpiod_direction_output(gpio, 0);
  +   gpiod_set_value_cansleep(gpio, 1);
  +   gpiod_put(gpio);
  +   }
  +   }
  +
 
 A lot has been discussed in other branches of this thread.
 
 But in resume, this is the last open point to make Heikki's proposal
 good on my side. If you accept this ugly quirk (but necessary for
 current BYT-CR products when ULPI bus enumerates phy), everything seems
 good to me. If you don't accept, we need to figure out a way to get the
 platform driver code back to give gpio to phy as platform data in a way
 that it could live together with ULPI bus (BYT-CR needs the ULPI bus too).

Is this needed to *all* Baytrail devices or could we have devices with
updated firmware which won't need this ? I wonder if we should apply the
quirk for each known product that actually needs this.

Also, I will only accept the series, after I'm shown logs that it works
with your known-to-be-broken device.

cheers

-- 
balbi


signature.asc
Description: Digital signature

Re: [PATCH 8/8] phy: add driver for TI TUSB1210 ULPI PHY

[David Cohen]

On Fri, Feb 13, 2015 at 04:03:57PM -0600, Felipe Balbi wrote:
 On Fri, Feb 13, 2015 at 02:02:11PM -0800, David Cohen wrote:
  Hi Felipe,
  
  [snip]
  
   diff --git a/drivers/usb/dwc3/dwc3-pci.c b/drivers/usb/dwc3/dwc3-pci.c
   index 8d95056..53902ea 100644
   --- a/drivers/usb/dwc3/dwc3-pci.c
   +++ b/drivers/usb/dwc3/dwc3-pci.c
   @@ -21,6 +21,7 @@
#include linux/slab.h
#include linux/pci.h
#include linux/platform_device.h
   +#include linux/gpio/consumer.h

#include platform_data.h

   @@ -35,6 +36,24 @@

static int dwc3_pci_quirks(struct pci_dev *pdev)
{
   + if (pdev-vendor == PCI_VENDOR_ID_INTEL 
   + pdev-device == PCI_DEVICE_ID_INTEL_BYT) {
   + struct gpio_desc *gpio;
   +
   + gpio = gpiod_get_index(pdev-dev, reset, 0);
   + if (!IS_ERR(gpio)) {
   + gpiod_direction_output(gpio, 0);
   + gpiod_set_value_cansleep(gpio, 1);
   + gpiod_put(gpio);
   + }
   + gpio = gpiod_get_index(pdev-dev, cs, 1);
   + if (!IS_ERR(gpio)) {
   + gpiod_direction_output(gpio, 0);
   + gpiod_set_value_cansleep(gpio, 1);
   + gpiod_put(gpio);
   + }
   + }
   +
  
  A lot has been discussed in other branches of this thread.
  
  But in resume, this is the last open point to make Heikki's proposal
  good on my side. If you accept this ugly quirk (but necessary for
  current BYT-CR products when ULPI bus enumerates phy), everything seems
  good to me. If you don't accept, we need to figure out a way to get the
  platform driver code back to give gpio to phy as platform data in a way
  that it could live together with ULPI bus (BYT-CR needs the ULPI bus too).
 
 Is this needed to *all* Baytrail devices or could we have devices with
 updated firmware which won't need this ? I wonder if we should apply the
 quirk for each known product that actually needs this.

The products that need this quirk will have the gpios on DSDT, otherwise
it won't be there. Except for the order of gpios (CS needs to be enabled
first and it's index 0 AFAIR), the quirk should follow Heikki's logic
here: if gpio isn't found we silently ignore it.

 
 Also, I will only accept the series, after I'm shown logs that it works
 with your known-to-be-broken device.

I can provide that when Heikki resends his patch set.

Br, David

 
 cheers
 
 -- 
 balbi


--
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/13] usb: host: oxu210hp: use new USB_RESUME_TIMEOUT

[Felipe Balbi]

Make sure we're using the new macro, so our
resume signaling will always pass certification.

Signed-off-by: Felipe Balbi ba...@ti.com
---
 drivers/usb/host/oxu210hp-hcd.c | 7 ---
 1 file changed, 4 insertions(+), 3 deletions(-)

diff --git a/drivers/usb/host/oxu210hp-hcd.c b/drivers/usb/host/oxu210hp-hcd.c
index 036924e640f5..3fc560c30d08 100644
--- a/drivers/usb/host/oxu210hp-hcd.c
+++ b/drivers/usb/host/oxu210hp-hcd.c
@@ -2500,11 +2500,12 @@ static irqreturn_t oxu210_hcd_irq(struct usb_hcd *hcd)
|| oxu-reset_done[i] != 0)
continue;
 
-   /* start 20 msec resume signaling from this port,
-* and make hub_wq collect PORT_STAT_C_SUSPEND to
+   /* start USB_RESUME_TIMEOUT resume signaling from this
+* port, and make hub_wq collect PORT_STAT_C_SUSPEND to
 * stop that signaling.
 */
-   oxu-reset_done[i] = jiffies + msecs_to_jiffies(20);
+   oxu-reset_done[i] = jiffies +
+   msecs_to_jiffies(USB_RESUME_TIMEOUT);
oxu_dbg(oxu, port %d remote wakeup\n, i + 1);
mod_timer(hcd-rh_timer, oxu-reset_done[i]);
}
-- 
2.3.0

--
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: [PATCH 01/13] usb: define a generic USB_RESUME_TIMEOUT macro

[Felipe Balbi]

On Fri, Feb 13, 2015 at 03:07:43PM -0600, Felipe Balbi wrote:
 Every USB Host controller should use this new
 macro to define for how long resume signalling
 should be driven on the bus.
 
 Currently, almost every single USB controller
 is using a 20ms timeout for resume signalling.
 
 That's problematic for two reasons:
 
 a) sometimes that 20ms timer expires a little
 before 20ms, which makes us fail certification
 
 b) some (many) devices actually need more than
 20ms resume signalling.
 
 Sure, in case of (b) we can state that the device
 is against the USB spec, but the fact is that
 we have no control over which device the certification
 lab will use. We also have no control over which host
 they will use. Most likely they'll be using a Windows
 PC which, again, we have no control over how that
 USB stack is written and how long resume signalling
 they are using.
 
 At the end of the day, we must make sure Linux passes
 electrical compliance when working as Host or as Device
 and currently we don't pass compliance as host because
 we're driving resume signallig for exactly 20ms and
 that confuses certification test setup resulting in
 Certification failure.
 
 Signed-off-by: Felipe Balbi ba...@ti.com
 ---

btw, the electrical test bug was reproduced with both MUSB and XHCI
(AM335x BeagleBone Black and AM437x Starter Kit)

-- 
balbi


signature.asc
Description: Digital signature

Re: [PATCH 03/13] usb: host: ehci: use new USB_RESUME_TIMEOUT

[Felipe Balbi]

On Fri, Feb 13, 2015 at 04:40:09PM -0500, Alan Stern wrote:
 On Fri, 13 Feb 2015, Alan Stern wrote:
 
  On Fri, 13 Feb 2015, Felipe Balbi wrote:
  
   Make sure we're using the new macro, so our
   resume signaling will always pass certification.
   
   Signed-off-by: Felipe Balbi ba...@ti.com
   ---
drivers/usb/host/ehci-hub.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
   
   diff --git a/drivers/usb/host/ehci-hub.c b/drivers/usb/host/ehci-hub.c
   index 118edb7bdca2..11e487b6fac4 100644
   --- a/drivers/usb/host/ehci-hub.c
   +++ b/drivers/usb/host/ehci-hub.c
   @@ -942,7 +942,7 @@ int ehci_hub_control(
 temp = ~PORT_WAKE_BITS;
 ehci_writel(ehci, temp | PORT_RESUME, status_reg);
 ehci-reset_done[wIndex] = jiffies
   - + msecs_to_jiffies(20);
   + + msecs_to_jiffies(USB_RESUME_TIMEOUT);
 set_bit(wIndex, ehci-resuming_ports);
 usb_hcd_start_port_resume(hcd-self, wIndex);
 break;
  
  You missed a second occurrence in ehci_irq(), just following a comment 
  that says:
  
  /* start 20 msec resume signaling from this port,
   * and make hub_wq collect PORT_STAT_C_SUSPEND to
   * stop that signaling.  Use 5 ms extra for safety,
   * like usb_port_resume() does.
   */
 
 There's a third occurrence as well, in ehci_bus_resume():
 
   /* msleep for 20ms only if code is trying to resume port */
   if (resume_needed) {
   spin_unlock_irq(ehci-lock);
   msleep(20);
   spin_lock_irq(ehci-lock);
   if (ehci-shutdown)
   goto shutdown;
   }

fixed this one too, thanks :-)

-- 
balbi


signature.asc
Description: Digital signature

Re: [PATCH 01/13] usb: define a generic USB_RESUME_TIMEOUT macro

[Greg KH]

On Fri, Feb 13, 2015 at 03:07:43PM -0600, Felipe Balbi wrote:
 Every USB Host controller should use this new
 macro to define for how long resume signalling
 should be driven on the bus.
 
 Currently, almost every single USB controller
 is using a 20ms timeout for resume signalling.
 
 That's problematic for two reasons:
 
 a) sometimes that 20ms timer expires a little
 before 20ms, which makes us fail certification
 
 b) some (many) devices actually need more than
 20ms resume signalling.
 
 Sure, in case of (b) we can state that the device
 is against the USB spec, but the fact is that
 we have no control over which device the certification
 lab will use. We also have no control over which host
 they will use. Most likely they'll be using a Windows
 PC which, again, we have no control over how that
 USB stack is written and how long resume signalling
 they are using.
 
 At the end of the day, we must make sure Linux passes
 electrical compliance when working as Host or as Device
 and currently we don't pass compliance as host because
 we're driving resume signallig for exactly 20ms and
 that confuses certification test setup resulting in
 Certification failure.
 
 Signed-off-by: Felipe Balbi ba...@ti.com
 ---
  include/linux/usb.h | 3 +++
  1 file changed, 3 insertions(+)
 
 diff --git a/include/linux/usb.h b/include/linux/usb.h
 index f89c24a03bd9..1dd89f0db344 100644
 --- a/include/linux/usb.h
 +++ b/include/linux/usb.h
 @@ -210,6 +210,9 @@ void usb_put_intf(struct usb_interface *intf);
  #define USB_MAXINTERFACES32
  #define USB_MAXIADS  (USB_MAXINTERFACES/2)
  
 +/* USB Resume Timer */
 +#define USB_RESUME_TIMEOUT   40 /* ms */
 +

If you add the comment as Alan suggested, feel free to add:

Acked-by: Greg Kroah-Hartman gre...@linuxfoundation.org

--
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: [PATCH 8/8] phy: add driver for TI TUSB1210 ULPI PHY

[Heikki Krogerus]

Hi,

On Thu, Feb 12, 2015 at 05:52:42PM -0800, David Cohen wrote:
 Hi Heikki,
 
 Sorry I am starting a new branch on this thread.
 I need to go back to another topic on this same patch.
 
 On Fri, Jan 23, 2015 at 05:12:58PM +0200, Heikki Krogerus wrote:
  TUSB1210 ULPI PHY has vendor specific register for eye
  diagram tuning. On some platforms the system firmware has
  set optimized value to it. In order to not loose the
  optimized value, the driver stores it during probe and
  restores it every time the PHY is powered back on.
  
  Signed-off-by: Heikki Krogerus heikki.kroge...@linux.intel.com
  Cc: Kishon Vijay Abraham I kis...@ti.com
  ---
 
 [snip]
 
  +   /* Store initial eye diagram optimisation value */
  +   ret = ulpi_read(ulpi, TUSB1210_VENDOR_SPECIFIC2);
 
 We can't rely on eye diagram optimization value here. It's possible the
 phy went through reset (e.g. module unloading/loading).
 We need a more consistent method. Could we use _DSD instead? That would
 be more compatible with DT too.

I'm don't have any problem with getting the value from a property.
Sounds like the correct thing to do. Are you thinking about putting
that _DSD method to the ACPI device object for the dwc3? The correct
place would probable be separate device object for the PHY, but if we
do that we need to consider how that object is bound to the ulpi
device. There are few ways we can do that, so it requires some
thinking.

In any case this driver we can already implement so that it expects to
get the value from property. We just need the name for it.

The register has actually separate fields for High speed output
impedance configuration and High speed output driver strength
configuration for eye diagram tuning, plus control bit for DP/DM swap.

Maybe we should actually get them from three separate properties:

device_property_read_u8(dev, datapolarity, val);
...
device_property_read_u8(dev, zhsdrv, val);
...
device_property_read_u8(dev, ihstx, val);
...

What do you think?


Cheers,

-- 
heikki
--
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: [PATCH 01/13] usb: define a generic USB_RESUME_TIMEOUT macro

[Felipe Balbi]

On Fri, Feb 13, 2015 at 09:07:40PM -0600, Bin Liu wrote:
 Greg,
 
 On Fri, Feb 13, 2015 at 8:41 PM, Greg KH gre...@linuxfoundation.org wrote:
  On Fri, Feb 13, 2015 at 07:42:22PM -0600, Felipe Balbi wrote:
  On Sat, Feb 14, 2015 at 08:13:36AM +0800, Greg KH wrote:
   On Fri, Feb 13, 2015 at 03:07:43PM -0600, Felipe Balbi wrote:
Every USB Host controller should use this new
macro to define for how long resume signalling
should be driven on the bus.
   
Currently, almost every single USB controller
is using a 20ms timeout for resume signalling.
   
That's problematic for two reasons:
   
a) sometimes that 20ms timer expires a little
before 20ms, which makes us fail certification
   
b) some (many) devices actually need more than
20ms resume signalling.
   
Sure, in case of (b) we can state that the device
is against the USB spec, but the fact is that
we have no control over which device the certification
lab will use. We also have no control over which host
they will use. Most likely they'll be using a Windows
PC which, again, we have no control over how that
USB stack is written and how long resume signalling
they are using.
   
At the end of the day, we must make sure Linux passes
electrical compliance when working as Host or as Device
and currently we don't pass compliance as host because
we're driving resume signallig for exactly 20ms and
that confuses certification test setup resulting in
Certification failure.
   
Signed-off-by: Felipe Balbi ba...@ti.com
---
 include/linux/usb.h | 3 +++
 1 file changed, 3 insertions(+)
   
diff --git a/include/linux/usb.h b/include/linux/usb.h
index f89c24a03bd9..1dd89f0db344 100644
--- a/include/linux/usb.h
+++ b/include/linux/usb.h
@@ -210,6 +210,9 @@ void usb_put_intf(struct usb_interface *intf);
 #define USB_MAXINTERFACES32
 #define USB_MAXIADS  (USB_MAXINTERFACES/2)
   
+/* USB Resume Timer */
+#define USB_RESUME_TIMEOUT   40 /* ms */
+
  
   If you add the comment as Alan suggested, feel free to add:
  
   Acked-by: Greg Kroah-Hartman gre...@linuxfoundation.org
 
  Is this good enough ?
 
  commit 3154122f81c8411b432d5f44a196302243d5af3d
  Author: Felipe Balbi ba...@ti.com
  Date:   Fri Feb 13 14:34:25 2015 -0600
 
  usb: define a generic USB_RESUME_TIMEOUT macro
 
  Every USB Host controller should use this new
  macro to define for how long resume signalling
  should be driven on the bus.
 
  Currently, almost every single USB controller
  is using a 20ms timeout for resume signalling.
 
  That's problematic for two reasons:
 
  a) sometimes that 20ms timer expires a little
  before 20ms, which makes us fail certification
 
  b) some (many) devices actually need more than
  20ms resume signalling.
 
  Sure, in case of (b) we can state that the device
  is against the USB spec, but the fact is that
  we have no control over which device the certification
  lab will use. We also have no control over which host
  they will use. Most likely they'll be using a Windows
  PC which, again, we have no control over how that
  USB stack is written and how long resume signalling
  they are using.
 
  At the end of the day, we must make sure Linux passes
  electrical compliance when working as Host or as Device
  and currently we don't pass compliance as host because
  we're driving resume signallig for exactly 20ms and
  that confuses certification test setup resulting in
  Certification failure.
 
  Signed-off-by: Felipe Balbi ba...@ti.com
 
  diff --git a/include/linux/usb.h b/include/linux/usb.h
  index 7ee1b5c3b4cb..447fe29b55b4 100644
  --- a/include/linux/usb.h
  +++ b/include/linux/usb.h
  @@ -205,6 +205,32 @@ void usb_put_intf(struct usb_interface *intf);
   #define USB_MAXINTERFACES32
   #define USB_MAXIADS  (USB_MAXINTERFACES/2)
 
  +/*
  + * USB Resume Timer: Every Host controller driver should drive the resume
  + * signalling on the bus for the amount of time defined by this macro.
  + *
  + * That way we will have a 'stable' behavior among all HCDs supported by 
  Linux.
  + *
  + * Note that the USB Specification states we should drive resume for *at 
  least*
  + * 20 ms, but it doesn't give an upper bound. This creates two possible
  + * situations which we want to avoid:
  + *
  + * (a) sometimes an msleep(20) might expire slightly before 20 ms, which 
  causes
  + * us to fail USB Electrical Tests, thus failing Certification
  + *
  + * (b) Some (many) devices actually need more than 20 ms of resume 
  signalling,
  + * and while we can argue that's against the USB Specification, we don't 
  have
  + * control over which devices a certification laboratory will be using for
  + * certification. If CertLab uses a device which was tested against 
  Windows and
  + * 

[no subject]

[Leanne Armstrong]

You were selected for QATAR Foundation 2015 beneficiaries contact 
RodFalusi(rodrigofalus...@yahoo.co.zamailto:rodrigofalus...@yahoo.co.za)
--
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: [PATCH 01/13] usb: define a generic USB_RESUME_TIMEOUT macro

[Felipe Balbi]

On Sat, Feb 14, 2015 at 08:13:36AM +0800, Greg KH wrote:
 On Fri, Feb 13, 2015 at 03:07:43PM -0600, Felipe Balbi wrote:
  Every USB Host controller should use this new
  macro to define for how long resume signalling
  should be driven on the bus.
  
  Currently, almost every single USB controller
  is using a 20ms timeout for resume signalling.
  
  That's problematic for two reasons:
  
  a) sometimes that 20ms timer expires a little
  before 20ms, which makes us fail certification
  
  b) some (many) devices actually need more than
  20ms resume signalling.
  
  Sure, in case of (b) we can state that the device
  is against the USB spec, but the fact is that
  we have no control over which device the certification
  lab will use. We also have no control over which host
  they will use. Most likely they'll be using a Windows
  PC which, again, we have no control over how that
  USB stack is written and how long resume signalling
  they are using.
  
  At the end of the day, we must make sure Linux passes
  electrical compliance when working as Host or as Device
  and currently we don't pass compliance as host because
  we're driving resume signallig for exactly 20ms and
  that confuses certification test setup resulting in
  Certification failure.
  
  Signed-off-by: Felipe Balbi ba...@ti.com
  ---
   include/linux/usb.h | 3 +++
   1 file changed, 3 insertions(+)
  
  diff --git a/include/linux/usb.h b/include/linux/usb.h
  index f89c24a03bd9..1dd89f0db344 100644
  --- a/include/linux/usb.h
  +++ b/include/linux/usb.h
  @@ -210,6 +210,9 @@ void usb_put_intf(struct usb_interface *intf);
   #define USB_MAXINTERFACES  32
   #define USB_MAXIADS(USB_MAXINTERFACES/2)
   
  +/* USB Resume Timer */
  +#define USB_RESUME_TIMEOUT 40 /* ms */
  +
 
 If you add the comment as Alan suggested, feel free to add:
 
 Acked-by: Greg Kroah-Hartman gre...@linuxfoundation.org

Is this good enough ?

commit 3154122f81c8411b432d5f44a196302243d5af3d
Author: Felipe Balbi ba...@ti.com
Date:   Fri Feb 13 14:34:25 2015 -0600

usb: define a generic USB_RESUME_TIMEOUT macro

Every USB Host controller should use this new
macro to define for how long resume signalling
should be driven on the bus.

Currently, almost every single USB controller
is using a 20ms timeout for resume signalling.

That's problematic for two reasons:

a) sometimes that 20ms timer expires a little
before 20ms, which makes us fail certification

b) some (many) devices actually need more than
20ms resume signalling.

Sure, in case of (b) we can state that the device
is against the USB spec, but the fact is that
we have no control over which device the certification
lab will use. We also have no control over which host
they will use. Most likely they'll be using a Windows
PC which, again, we have no control over how that
USB stack is written and how long resume signalling
they are using.

At the end of the day, we must make sure Linux passes
electrical compliance when working as Host or as Device
and currently we don't pass compliance as host because
we're driving resume signallig for exactly 20ms and
that confuses certification test setup resulting in
Certification failure.

Signed-off-by: Felipe Balbi ba...@ti.com

diff --git a/include/linux/usb.h b/include/linux/usb.h
index 7ee1b5c3b4cb..447fe29b55b4 100644
--- a/include/linux/usb.h
+++ b/include/linux/usb.h
@@ -205,6 +205,32 @@ void usb_put_intf(struct usb_interface *intf);
 #define USB_MAXINTERFACES  32
 #define USB_MAXIADS(USB_MAXINTERFACES/2)
 
+/*
+ * USB Resume Timer: Every Host controller driver should drive the resume
+ * signalling on the bus for the amount of time defined by this macro.
+ *
+ * That way we will have a 'stable' behavior among all HCDs supported by Linux.
+ *
+ * Note that the USB Specification states we should drive resume for *at least*
+ * 20 ms, but it doesn't give an upper bound. This creates two possible
+ * situations which we want to avoid:
+ *
+ * (a) sometimes an msleep(20) might expire slightly before 20 ms, which causes
+ * us to fail USB Electrical Tests, thus failing Certification
+ *
+ * (b) Some (many) devices actually need more than 20 ms of resume signalling,
+ * and while we can argue that's against the USB Specification, we don't have
+ * control over which devices a certification laboratory will be using for
+ * certification. If CertLab uses a device which was tested against Windows and
+ * that happens to have relaxed resume signalling rules, we might fall into
+ * situations where we fail interoperability and electrical tests.
+ *
+ * In order to avoid both conditions, we're using a 40 ms resume timeout, which
+ * should cope with both LPJ calibration errors and devices not following every
+ * detail of the USB Specification.
+ */
+#define USB_RESUME_TIMEOUT 40 /* ms */
+
 /**
  * struct 

Re: [PATCH 01/13] usb: define a generic USB_RESUME_TIMEOUT macro

[Greg KH]

On Fri, Feb 13, 2015 at 07:42:22PM -0600, Felipe Balbi wrote:
 On Sat, Feb 14, 2015 at 08:13:36AM +0800, Greg KH wrote:
  On Fri, Feb 13, 2015 at 03:07:43PM -0600, Felipe Balbi wrote:
   Every USB Host controller should use this new
   macro to define for how long resume signalling
   should be driven on the bus.
   
   Currently, almost every single USB controller
   is using a 20ms timeout for resume signalling.
   
   That's problematic for two reasons:
   
   a) sometimes that 20ms timer expires a little
   before 20ms, which makes us fail certification
   
   b) some (many) devices actually need more than
   20ms resume signalling.
   
   Sure, in case of (b) we can state that the device
   is against the USB spec, but the fact is that
   we have no control over which device the certification
   lab will use. We also have no control over which host
   they will use. Most likely they'll be using a Windows
   PC which, again, we have no control over how that
   USB stack is written and how long resume signalling
   they are using.
   
   At the end of the day, we must make sure Linux passes
   electrical compliance when working as Host or as Device
   and currently we don't pass compliance as host because
   we're driving resume signallig for exactly 20ms and
   that confuses certification test setup resulting in
   Certification failure.
   
   Signed-off-by: Felipe Balbi ba...@ti.com
   ---
include/linux/usb.h | 3 +++
1 file changed, 3 insertions(+)
   
   diff --git a/include/linux/usb.h b/include/linux/usb.h
   index f89c24a03bd9..1dd89f0db344 100644
   --- a/include/linux/usb.h
   +++ b/include/linux/usb.h
   @@ -210,6 +210,9 @@ void usb_put_intf(struct usb_interface *intf);
#define USB_MAXINTERFACES32
#define USB_MAXIADS  (USB_MAXINTERFACES/2)

   +/* USB Resume Timer */
   +#define USB_RESUME_TIMEOUT   40 /* ms */
   +
  
  If you add the comment as Alan suggested, feel free to add:
  
  Acked-by: Greg Kroah-Hartman gre...@linuxfoundation.org
 
 Is this good enough ?
 
 commit 3154122f81c8411b432d5f44a196302243d5af3d
 Author: Felipe Balbi ba...@ti.com
 Date:   Fri Feb 13 14:34:25 2015 -0600
 
 usb: define a generic USB_RESUME_TIMEOUT macro
 
 Every USB Host controller should use this new
 macro to define for how long resume signalling
 should be driven on the bus.
 
 Currently, almost every single USB controller
 is using a 20ms timeout for resume signalling.
 
 That's problematic for two reasons:
 
 a) sometimes that 20ms timer expires a little
 before 20ms, which makes us fail certification
 
 b) some (many) devices actually need more than
 20ms resume signalling.
 
 Sure, in case of (b) we can state that the device
 is against the USB spec, but the fact is that
 we have no control over which device the certification
 lab will use. We also have no control over which host
 they will use. Most likely they'll be using a Windows
 PC which, again, we have no control over how that
 USB stack is written and how long resume signalling
 they are using.
 
 At the end of the day, we must make sure Linux passes
 electrical compliance when working as Host or as Device
 and currently we don't pass compliance as host because
 we're driving resume signallig for exactly 20ms and
 that confuses certification test setup resulting in
 Certification failure.
 
 Signed-off-by: Felipe Balbi ba...@ti.com
 
 diff --git a/include/linux/usb.h b/include/linux/usb.h
 index 7ee1b5c3b4cb..447fe29b55b4 100644
 --- a/include/linux/usb.h
 +++ b/include/linux/usb.h
 @@ -205,6 +205,32 @@ void usb_put_intf(struct usb_interface *intf);
  #define USB_MAXINTERFACES32
  #define USB_MAXIADS  (USB_MAXINTERFACES/2)
  
 +/*
 + * USB Resume Timer: Every Host controller driver should drive the resume
 + * signalling on the bus for the amount of time defined by this macro.
 + *
 + * That way we will have a 'stable' behavior among all HCDs supported by 
 Linux.
 + *
 + * Note that the USB Specification states we should drive resume for *at 
 least*
 + * 20 ms, but it doesn't give an upper bound. This creates two possible
 + * situations which we want to avoid:
 + *
 + * (a) sometimes an msleep(20) might expire slightly before 20 ms, which 
 causes
 + * us to fail USB Electrical Tests, thus failing Certification
 + *
 + * (b) Some (many) devices actually need more than 20 ms of resume 
 signalling,
 + * and while we can argue that's against the USB Specification, we don't have
 + * control over which devices a certification laboratory will be using for
 + * certification. If CertLab uses a device which was tested against Windows 
 and
 + * that happens to have relaxed resume signalling rules, we might fall into
 + * situations where we fail interoperability and electrical tests.
 + *
 + * In order to avoid both conditions, we're using a 40 ms resume 

Re: [PATCH 01/13] usb: define a generic USB_RESUME_TIMEOUT macro

[Bin Liu]

Greg,

On Fri, Feb 13, 2015 at 8:41 PM, Greg KH gre...@linuxfoundation.org wrote:
 On Fri, Feb 13, 2015 at 07:42:22PM -0600, Felipe Balbi wrote:
 On Sat, Feb 14, 2015 at 08:13:36AM +0800, Greg KH wrote:
  On Fri, Feb 13, 2015 at 03:07:43PM -0600, Felipe Balbi wrote:
   Every USB Host controller should use this new
   macro to define for how long resume signalling
   should be driven on the bus.
  
   Currently, almost every single USB controller
   is using a 20ms timeout for resume signalling.
  
   That's problematic for two reasons:
  
   a) sometimes that 20ms timer expires a little
   before 20ms, which makes us fail certification
  
   b) some (many) devices actually need more than
   20ms resume signalling.
  
   Sure, in case of (b) we can state that the device
   is against the USB spec, but the fact is that
   we have no control over which device the certification
   lab will use. We also have no control over which host
   they will use. Most likely they'll be using a Windows
   PC which, again, we have no control over how that
   USB stack is written and how long resume signalling
   they are using.
  
   At the end of the day, we must make sure Linux passes
   electrical compliance when working as Host or as Device
   and currently we don't pass compliance as host because
   we're driving resume signallig for exactly 20ms and
   that confuses certification test setup resulting in
   Certification failure.
  
   Signed-off-by: Felipe Balbi ba...@ti.com
   ---
include/linux/usb.h | 3 +++
1 file changed, 3 insertions(+)
  
   diff --git a/include/linux/usb.h b/include/linux/usb.h
   index f89c24a03bd9..1dd89f0db344 100644
   --- a/include/linux/usb.h
   +++ b/include/linux/usb.h
   @@ -210,6 +210,9 @@ void usb_put_intf(struct usb_interface *intf);
#define USB_MAXINTERFACES32
#define USB_MAXIADS  (USB_MAXINTERFACES/2)
  
   +/* USB Resume Timer */
   +#define USB_RESUME_TIMEOUT   40 /* ms */
   +
 
  If you add the comment as Alan suggested, feel free to add:
 
  Acked-by: Greg Kroah-Hartman gre...@linuxfoundation.org

 Is this good enough ?

 commit 3154122f81c8411b432d5f44a196302243d5af3d
 Author: Felipe Balbi ba...@ti.com
 Date:   Fri Feb 13 14:34:25 2015 -0600

 usb: define a generic USB_RESUME_TIMEOUT macro

 Every USB Host controller should use this new
 macro to define for how long resume signalling
 should be driven on the bus.

 Currently, almost every single USB controller
 is using a 20ms timeout for resume signalling.

 That's problematic for two reasons:

 a) sometimes that 20ms timer expires a little
 before 20ms, which makes us fail certification

 b) some (many) devices actually need more than
 20ms resume signalling.

 Sure, in case of (b) we can state that the device
 is against the USB spec, but the fact is that
 we have no control over which device the certification
 lab will use. We also have no control over which host
 they will use. Most likely they'll be using a Windows
 PC which, again, we have no control over how that
 USB stack is written and how long resume signalling
 they are using.

 At the end of the day, we must make sure Linux passes
 electrical compliance when working as Host or as Device
 and currently we don't pass compliance as host because
 we're driving resume signallig for exactly 20ms and
 that confuses certification test setup resulting in
 Certification failure.

 Signed-off-by: Felipe Balbi ba...@ti.com

 diff --git a/include/linux/usb.h b/include/linux/usb.h
 index 7ee1b5c3b4cb..447fe29b55b4 100644
 --- a/include/linux/usb.h
 +++ b/include/linux/usb.h
 @@ -205,6 +205,32 @@ void usb_put_intf(struct usb_interface *intf);
  #define USB_MAXINTERFACES32
  #define USB_MAXIADS  (USB_MAXINTERFACES/2)

 +/*
 + * USB Resume Timer: Every Host controller driver should drive the resume
 + * signalling on the bus for the amount of time defined by this macro.
 + *
 + * That way we will have a 'stable' behavior among all HCDs supported by 
 Linux.
 + *
 + * Note that the USB Specification states we should drive resume for *at 
 least*
 + * 20 ms, but it doesn't give an upper bound. This creates two possible
 + * situations which we want to avoid:
 + *
 + * (a) sometimes an msleep(20) might expire slightly before 20 ms, which 
 causes
 + * us to fail USB Electrical Tests, thus failing Certification
 + *
 + * (b) Some (many) devices actually need more than 20 ms of resume 
 signalling,
 + * and while we can argue that's against the USB Specification, we don't 
 have
 + * control over which devices a certification laboratory will be using for
 + * certification. If CertLab uses a device which was tested against Windows 
 and
 + * that happens to have relaxed resume signalling rules, we might fall into
 + * situations where we fail interoperability and electrical tests.
 + *
 + * In order to avoid both conditions,