Re: [ 12/75] USB: EHCI: work around silicon bug in Intels EHCI controllers

From: Sven Joachim
Date: Tue Mar 19 2013 - 12:34:59 EST

Next message: Matthew Garrett: "Re: [PATCH] x86/efi: pull NV+BS variables out before we exit bootservices"
Previous message: David Miller: "Re: [PATCH] ixp4xx_eth: set the device dma_coherent_mask"
In reply to: Greg Kroah-Hartman: "[ 12/75] USB: EHCI: work around silicon bug in Intels EHCI controllers"
Next in thread: Alan Stern: "Re: [ 12/75] USB: EHCI: work around silicon bug in Intels EHCIcontrollers"
Messages sorted by: [ date ] [ thread ] [ subject ] [ author ]

On 2013-03-18 22:06 +0100, Greg Kroah-Hartman wrote:

> 3.8-stable review patch. If anyone has any objections, please let me know.

This patch breaks resume from suspend on my system if the following
device is plugged in:

,----
| $ lsusb | head -n1
| Bus 005 Device 002: ID 07d1:3c03 D-Link System AirPlus G DWL-G122 Wireless Adapter(rev.C1) [Ralink RT2571W]
`----

The same problem exists in 3.9-rc3, FWIW.

Detailed lsusb output follows, I'll be happy to provide more information
if needed.

,----
| # lsusb -s 005: -v
| Bus 005 Device 002: ID 07d1:3c03 D-Link System AirPlus G DWL-G122 Wireless Adapter(rev.C1) [Ralink RT2571W]
| Device Descriptor:
| bLength 18
| bDescriptorType 1
| bcdUSB 2.00
| bDeviceClass 0 (Defined at Interface level)
| bDeviceSubClass 0
| bDeviceProtocol 0
| bMaxPacketSize0 64
| idVendor 0x07d1 D-Link System
| idProduct 0x3c03 AirPlus G DWL-G122 Wireless Adapter(rev.C1) [Ralink RT2571W]
| bcdDevice 0.01
| iManufacturer 1 Ralink
| iProduct 2 802.11 bg WLAN
| iSerial 0
| bNumConfigurations 1
| Configuration Descriptor:
| bLength 9
| bDescriptorType 2
| wTotalLength 32
| bNumInterfaces 1
| bConfigurationValue 1
| iConfiguration 0
| bmAttributes 0x80
| (Bus Powered)
| MaxPower 300mA
| Interface Descriptor:
| bLength 9
| bDescriptorType 4
| bInterfaceNumber 0
| bAlternateSetting 0
| bNumEndpoints 2
| bInterfaceClass 255 Vendor Specific Class
| bInterfaceSubClass 255 Vendor Specific Subclass
| bInterfaceProtocol 255 Vendor Specific Protocol
| iInterface 0
| Endpoint Descriptor:
| bLength 7
| bDescriptorType 5
| bEndpointAddress 0x81 EP 1 IN
| bmAttributes 2
| Transfer Type Bulk
| Synch Type None
| Usage Type Data
| wMaxPacketSize 0x0200 1x 512 bytes
| bInterval 0
| Endpoint Descriptor:
| bLength 7
| bDescriptorType 5
| bEndpointAddress 0x01 EP 1 OUT
| bmAttributes 2
| Transfer Type Bulk
| Synch Type None
| Usage Type Data
| wMaxPacketSize 0x0200 1x 512 bytes
| bInterval 0
| Device Qualifier (for other device speed):
| bLength 10
| bDescriptorType 6
| bcdUSB 2.00
| bDeviceClass 0 (Defined at Interface level)
| bDeviceSubClass 0
| bDeviceProtocol 0
| bMaxPacketSize0 64
| bNumConfigurations 1
| Device Status: 0x0000
| (Bus Powered)
|
| Bus 005 Device 001: ID 1d6b:0002 Linux Foundation 2.0 root hub
| Device Descriptor:
| bLength 18
| bDescriptorType 1
| bcdUSB 2.00
| bDeviceClass 9 Hub
| bDeviceSubClass 0 Unused
| bDeviceProtocol 0 Full speed (or root) hub
| bMaxPacketSize0 64
| idVendor 0x1d6b Linux Foundation
| idProduct 0x0002 2.0 root hub
| bcdDevice 3.08
| iManufacturer 3 Linux 3.8.3-nouveau ehci_hcd
| iProduct 2 EHCI Host Controller
| iSerial 1 0000:00:1d.7
| bNumConfigurations 1
| Configuration Descriptor:
| bLength 9
| bDescriptorType 2
| wTotalLength 25
| bNumInterfaces 1
| bConfigurationValue 1
| iConfiguration 0
| bmAttributes 0xe0
| Self Powered
| Remote Wakeup
| MaxPower 0mA
| Interface Descriptor:
| bLength 9
| bDescriptorType 4
| bInterfaceNumber 0
| bAlternateSetting 0
| bNumEndpoints 1
| bInterfaceClass 9 Hub
| bInterfaceSubClass 0 Unused
| bInterfaceProtocol 0 Full speed (or root) hub
| iInterface 0
| Endpoint Descriptor:
| bLength 7
| bDescriptorType 5
| bEndpointAddress 0x81 EP 1 IN
| bmAttributes 3
| Transfer Type Interrupt
| Synch Type None
| Usage Type Data
| wMaxPacketSize 0x0004 1x 4 bytes
| bInterval 12
| Hub Descriptor:
| bLength 11
| bDescriptorType 41
| nNbrPorts 8
| wHubCharacteristic 0x000a
| No power switching (usb 1.0)
| Per-port overcurrent protection
| bPwrOn2PwrGood 10 * 2 milli seconds
| bHubContrCurrent 0 milli Ampere
| DeviceRemovable 0x00 0x00
| PortPwrCtrlMask 0xff 0xff
| Hub Port Status:
| Port 1: 0000.0100 power
| Port 2: 0000.0100 power
| Port 3: 0000.0100 power
| Port 4: 0000.0100 power
| Port 5: 0000.0100 power
| Port 6: 0000.0503 highspeed power enable connect
| Port 7: 0000.0100 power
| Port 8: 0000.0100 power
| Device Status: 0x0001
| Self Powered
`----

Cheers,
Sven

> From: Alan Stern <stern@xxxxxxxxxxxxxxxxxxx>
>
> commit 6402c796d3b4205d3d7296157956c5100a05d7d6 upstream.
>
> This patch (as1660) works around a hardware problem present in some
> (if not all) Intel EHCI controllers. After a QH has been unlinked
> from the async schedule and the corresponding IAA interrupt has
> occurred, the controller is not supposed access the QH and its qTDs.
> There certainly shouldn't be any more DMA writes to those structures.
> Nevertheless, Intel's controllers have been observed to perform a
> final writeback to the QH's overlay region and to the most recent qTD.
> For more information and a test program to determine whether this
> problem is present in a particular controller, see
>
> http://marc.info/?l=linux-usb&m=135492071812265&w=2
> http://marc.info/?l=linux-usb&m=136182570800963&w=2
>
> This patch works around the problem by always waiting for two IAA
> cycles when unlinking an async QH. The extra IAA delay gives the
> controller time to perform its final writeback.
>
> Surprisingly enough, the effects of this silicon bug have gone
> undetected until quite recently. More through luck than anything
> else, it hasn't caused any apparent problems. However, it does
> interact badly with the path that follows this one, so it needs to be
> addressed.
>
> This is the first part of a fix for the regression reported at:
>
> https://bugs.launchpad.net/bugs/1088733
>
> Signed-off-by: Alan Stern <stern@xxxxxxxxxxxxxxxxxxx>
> Tested-by: Stephen Thirlwall <sdt@xxxxxx>
> Signed-off-by: Greg Kroah-Hartman <gregkh@xxxxxxxxxxxxxxxxxxx>
>
> ---
> drivers/usb/host/ehci-hcd.c | 6 ++----
> drivers/usb/host/ehci-q.c | 18 ++++++++++++++----
> 2 files changed, 16 insertions(+), 8 deletions(-)
>
> --- a/drivers/usb/host/ehci-hcd.c
> +++ b/drivers/usb/host/ehci-hcd.c
> @@ -748,11 +748,9 @@ static irqreturn_t ehci_irq (struct usb_
> /* guard against (alleged) silicon errata */
> if (cmd & CMD_IAAD)
> ehci_dbg(ehci, "IAA with IAAD still set?\n");
> - if (ehci->async_iaa) {
> + if (ehci->async_iaa)
> COUNT(ehci->stats.iaa);
> - end_unlink_async(ehci);
> - } else
> - ehci_dbg(ehci, "IAA with nothing unlinked?\n");
> + end_unlink_async(ehci);
> }
>
> /* remote wakeup [4.3.1] */
> --- a/drivers/usb/host/ehci-q.c
> +++ b/drivers/usb/host/ehci-q.c
> @@ -1170,7 +1170,7 @@ static void single_unlink_async(struct e
> struct ehci_qh *prev;
>
> /* Add to the end of the list of QHs waiting for the next IAAD */
> - qh->qh_state = QH_STATE_UNLINK;
> + qh->qh_state = QH_STATE_UNLINK_WAIT;
> if (ehci->async_unlink)
> ehci->async_unlink_last->unlink_next = qh;
> else
> @@ -1213,9 +1213,19 @@ static void start_iaa_cycle(struct ehci_
>
> /* Do only the first waiting QH (nVidia bug?) */
> qh = ehci->async_unlink;
> - ehci->async_iaa = qh;
> - ehci->async_unlink = qh->unlink_next;
> - qh->unlink_next = NULL;
> +
> + /*
> + * Intel (?) bug: The HC can write back the overlay region
> + * even after the IAA interrupt occurs. In self-defense,
> + * always go through two IAA cycles for each QH.
> + */
> + if (qh->qh_state == QH_STATE_UNLINK_WAIT) {
> + qh->qh_state = QH_STATE_UNLINK;
> + } else {
> + ehci->async_iaa = qh;
> + ehci->async_unlink = qh->unlink_next;
> + qh->unlink_next = NULL;
> + }
>
> /* Make sure the unlinks are all visible to the hardware */
> wmb();
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/

Next message: Matthew Garrett: "Re: [PATCH] x86/efi: pull NV+BS variables out before we exit bootservices"
Previous message: David Miller: "Re: [PATCH] ixp4xx_eth: set the device dma_coherent_mask"
In reply to: Greg Kroah-Hartman: "[ 12/75] USB: EHCI: work around silicon bug in Intels EHCI controllers"
Next in thread: Alan Stern: "Re: [ 12/75] USB: EHCI: work around silicon bug in Intels EHCIcontrollers"
Messages sorted by: [ date ] [ thread ] [ subject ] [ author ]