patch 8466489ef5ba48272ba4fa4ea9f8f403306de4c7 breaks Renesas USB3 controller functionality

[Marc Zyngier <marc.zyngier@arm.com>]

On 01/03/18 08:01, Bockholdt Arne wrote:
> 
> On Thu, 2018-02-15 at 19:29 +0000, Marc Zyngier wrote:
>> [+ Ard, who helped me chasing the initial issue]
>>
>> On 15/02/18 06:43, Bockholdt Arne wrote:
>>> Hi all,
>>>
>>> on our Intel Atom C2578 server with a SuperMicro A1SAi board and a
>>> Renesas uPD720201 USB 3.0 host controller the controller has
>>> stopped
>>> working since kernel 4.13.x. Before that kernel the dmesg messages
>>> from
>>> XHCI were:
>>>
>>> dmesg-4.12.1-serverv4.log:xhci_hcd 0000:03:00.0: xHCI Host
>>> Controller
>>> dmesg-4.12.1-serverv4.log:xhci_hcd 0000:03:00.0: new USB bus
>>> registered,
>>> assigned bus number 2
>>> dmesg-4.12.1-serverv4.log:xhci_hcd 0000:03:00.0: hcc params
>>> 0x014051cf
>>> hci version 0x100 quirks 0x00000010
>>> dmesg-4.12.1-serverv4.log:usb usb2: Manufacturer: Linux 4.12.1-
>>> serverv4
>>> xhci-hcd
>>> dmesg-4.12.1-serverv4.log:xhci_hcd 0000:03:00.0: xHCI Host
>>> Controller
>>> dmesg-4.12.1-serverv4.log:xhci_hcd 0000:03:00.0: new USB bus
>>> registered,
>>> assigned bus number 3
>>> dmesg-4.12.1-serverv4.log:usb usb3: Manufacturer: Linux 4.12.1-
>>> serverv4
>>> xhci-hcd
>>>
>>> After that the message look like that:
>>>
>>> dmesg-4.13.1-serverv4.log:xhci_hcd 0000:03:00.0: Resetting
>>> dmesg-4.13.1-serverv4.log:xhci_hcd 0000:03:00.0: Refused to change
>>> power
>>> state, currently in D3
>>> dmesg-4.13.1-serverv4.log:xhci_hcd 0000:03:00.0: xHCI Host
>>> Controller
>>> dmesg-4.13.1-serverv4.log:xhci_hcd 0000:03:00.0: new USB bus
>>> registered,
>>> assigned bus number 2
>>> dmesg-4.13.1-serverv4.log:xhci_hcd 0000:03:00.0: Host halt failed,
>>> -19
>>> dmesg-4.13.1-serverv4.log:xhci_hcd 0000:03:00.0: can't setup: -19
>>> dmesg-4.13.1-serverv4.log:xhci_hcd 0000:03:00.0: USB bus 2
>>> deregistered
>>> dmesg-4.13.1-serverv4.log:xhci_hcd 0000:03:00.0: init 0000:03:00.0
>>> fail, -19
>>>
>>> I've tested it with 4.15.3 too, it's still the same. I've narrowed
>>> it
>>> down to the following patch:
>>>
>>> commit 8466489ef5ba48272ba4fa4ea9f8f403306de4c7
>>> Author: Marc Zyngier <marc.zyngier@arm.com <mailto:marc.zyngier@arm
>>> .com>>
>>> Date:   Tue Aug 1 20:11:08 2017 -0500
>>>
>>>     xhci: Reset Renesas uPD72020x USB controller for 32-bit DMA
>>> issue
>>>
>>> Reverting the patch on top of 4.15.3 restores the USB3
>>> functionality on
>>> our server. Please let me know if there is anything I can do to fix
>>> the
>>> problem. Thank you.
>>
>> Hi Arne,
>>
>> This looks pretty bad. I suspect that once reset, the firmware is
>> lost.
>> I'll try to write a patch dumping some information about it.
>>
>> Ard: Do you know if the Cello board has a SPI flash connected to the
>> Renesas chip, from which it would load the firmware?
>>
>> Another possibility is that power management kicks in, and that the
>> endpoint is stuck there. Could also be firmware related, but not
>> only.
>> I'd welcome any idea on the subject, as I cannot reproduce this issue
>> on
>> the HW I have.
>>
>> It we cannot work out what exactly is causing this, we may have to
>> default to not resetting the part and rely on a command-line option
>> to
>> do it... I can't say I'm a fan.
>>
>> Thanks,
>>
>> 	M.
>>
> 
> Hi Marc,
> 
> I've tested it with 4.15.7 and it's still there. Is there anything that
> I can do to help fixing this problem?

Would you mind trying the following patch and let me know if it helps?
It is not exactly pretty, but we can polish it if that solves your issue.

Thanks,

	M.

From 9a253773b289f781b7114e887481939b3021bb30 Mon Sep 17 00:00:00 2001
From: Marc Zyngier <marc.zyngier@arm.com>
Date: Thu, 1 Mar 2018 17:27:42 +0000
Subject: [PATCH] xhci: Only reset uPD72020x if programmed with 64bit DMA
 addresses

The Renesas uPD72020x USB controller misbehaves when switching
from 64bit DMA addresses to 32bit ones, and can only accept a
new programming if hard reset first.

But it also misbehaves (in a different way) if reset for no
good reason. So let's restrict the resetting to the cases where
we detect the 64/32 issue, and leave the device alone in the
other cases.

Fixes: 8466489ef5ba ("xhci: Reset Renesas uPD72020x USB controller for 32-bit DMA issue")
Signed-off-by: Marc Zyngier <marc.zyngier@arm.com>
---
 drivers/usb/host/pci-quirks.c | 33 ++++++++++++++++++++++++++++++---
 1 file changed, 30 insertions(+), 3 deletions(-)

diff --git a/drivers/usb/host/pci-quirks.c b/drivers/usb/host/pci-quirks.c
index 67ad4bb6919a..b05a5c2ec08d 100644
--- a/drivers/usb/host/pci-quirks.c
+++ b/drivers/usb/host/pci-quirks.c
@@ -16,6 +16,7 @@
 #include <linux/export.h>
 #include <linux/acpi.h>
 #include <linux/dmi.h>
+#include <linux/io-64-nonatomic-lo-hi.h>
 #include "pci-quirks.h"
 #include "xhci-ext-caps.h"
 
@@ -1277,13 +1278,39 @@ bool usb_xhci_needs_pci_reset(struct pci_dev *pdev)
 	 * at the wrong addresses, as it keeps the top 32bit of some
 	 * addresses from its previous programming under obscure
 	 * circumstances.
-	 * Give it a good wack at probe time. Unfortunately, this
+	 * Give it a good whack at probe time if any of the ERST base
+	 * addresses contains a 64bit address. Unfortunately, this
 	 * needs to happen before we've had a chance to discover any
 	 * quirk, or the system will be in a rather bad state.
 	 */
 	if (pdev->vendor == PCI_VENDOR_ID_RENESAS &&
-	    (pdev->device == 0x0014 || pdev->device == 0x0015))
-		return true;
+	    (pdev->device == 0x0014 || pdev->device == 0x0015)) {
+		int len = pci_resource_len(pdev, 0);
+		void __iomem *base;
+		bool sf = false;
+		int i;
+
+		if (len < 0x720 || !mmio_resource_enabled(pdev, 0))
+			return false;
+
+		base = ioremap_nocache(pci_resource_start(pdev, 0), len);
+		if (!base)
+			return false;
+
+		for (i = 0; i < 7; i++) {
+			u64 val = lo_hi_readq(base + 0x600 + 0x30 + 20 * i);
+			if (!upper_32_bits(val))
+				continue;
+
+			dev_info(&pdev->dev,
+				 "Found a 64bit address in ERSTBA %d\n", i);
+			sf = true;
+			break;
+		}
+
+		iounmap(base);
+		return sf;
+	}
 
 	return false;
 }