[PATCH v3 0/5] Add support for Fujitsu USB host controller

[PATCH v3 0/5] Add support for Fujitsu USB host controller

[Sneeker Yeh]

These patches add support for XHCI compliant Host controller found
on Fujitsu Socs, and are based on http://lwn.net/Articles/629162/
The first patch is to add Fujitsu glue layer of Synopsis DesignWare USB3 driver
and last four patch is about quirk implementation of errata in Synopsis
DesignWare USB3 IP.

Patch 1 introduces a quirk with device disconnection management necessary
Synopsys Designware USB3 IP with versions < 3.00a and hardware configuration
DWC_USB3_SUSPEND_ON_DISCONNECT_EN=1. It solves a problem where without the
quirk, that host controller will die after a usb device is disconnected from
port of root hub.

Patch 2 is to set Synopsis quirk in xhci platform driver based on xhci platform
data.

Patch 3 is to add a revison number 2.90a and 3.00a of Synopsis DesignWare USB3
IP core driver.

Patch 4 introduces using a quirk based on a errata of Synopsis
DesignWare USB3 IP which is versions < 3.00a and has hardware configuration
DWC_USB3_SUSPEND_ON_DISCONNECT_EN=1, which cannot be read from software. As a
result this quirk has to be enabled via platform data or device tree.

Patch 5 introduces Fujitsu Specific Glue layer in Synopsis DesignWare USB3 IP
driver. 

Successfully tested on Fujitsu mb86s7x board.

Changes since v2 (RFC):
[https://lkml.org/lkml/2014/12/15/929]
 - based on Felipe's comment, re-order patches to avoid breaking bisectability,
 - based on Felipe's comment, add comment to structure's member, and sort it
   alphabetically,
 - based on Felipe's comment, add another v2.90 revision number in dwc3 IP.

Changes since v1 (RFC):
[https://lkml.org/lkml/2014/12/15/929]
 - based on Arnd's comment, remove entire unnecessary Fujitsu EHCI/OHCI glue,
 - based on Felipe's comment, fix mis-using of runtime-pm API and setting dma
   mask, remove unnecessary comment, and refactor suspend/resume handler in
   Fujitsu Specific Glue layer in dwc3,
 - based on Felipe's comment, add more commit log and comments in Synopsis
   quirk implementation, and separate it into four patches.

Sneeker Yeh (5):
  xhci: add a quirk for device disconnection errata for Synopsis
    Designware USB3     core
  xhci: Platform: Set Synopsis device disconnection quirk based on
    platform data
  usb: dwc3: add revision number DWC3_REVISION_290A and
    DWC3_REVISION_300A
  usb: dwc3: Add quirk for Synopsis device disconnection errata
  usb: dwc3: add Fujitsu Specific Glue layer

 Documentation/devicetree/bindings/usb/dwc3.txt     |   17 ++
 .../devicetree/bindings/usb/fujitsu-dwc3.txt       |   33 ++++
 drivers/usb/dwc3/Kconfig                           |   11 ++
 drivers/usb/dwc3/Makefile                          |    1 +
 drivers/usb/dwc3/core.c                            |    6 +
 drivers/usb/dwc3/core.h                            |    6 +
 drivers/usb/dwc3/dwc3-mb86s70.c                    |  206 ++++++++++++++++++++
 drivers/usb/dwc3/host.c                            |    4 +
 drivers/usb/dwc3/platform_data.h                   |    8 +
 drivers/usb/host/xhci-hub.c                        |    4 +
 drivers/usb/host/xhci-plat.c                       |    3 +
 drivers/usb/host/xhci.c                            |   29 +++
 drivers/usb/host/xhci.h                            |   24 +++
 include/linux/usb/xhci_pdriver.h                   |    4 +
 14 files changed, 356 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/usb/fujitsu-dwc3.txt
 create mode 100644 drivers/usb/dwc3/dwc3-mb86s70.c

-- 
1.7.9.5

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

[Sneeker Yeh]

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.

this defect would result in this when device detached:
-------------------------------------------------------
[   99.603544] usb 4-1: USB disconnect, device number 2
[  104.615254] xhci-hcd xhci-hcd.0.auto: xHCI host not responding to stop endpoint command.
[  104.623362] xhci-hcd xhci-hcd.0.auto: Assuming host is dying, halting host.
[  104.653261] xhci-hcd xhci-hcd.0.auto: Host not halted after 16000 microseconds.
[  104.660584] xhci-hcd xhci-hcd.0.auto: Non-responsive xHCI host is not halting.
[  104.667817] xhci-hcd xhci-hcd.0.auto: Completing active URBs anyway.
[  104.674198] xhci-hcd xhci-hcd.0.auto: HC died; cleaning up
------------------------------------------------------
As a result, when device detached, we desired to postpone "PORTCSC clear" behind
"disable slot". it's found that all executed ep command related to disconnetion,
are executed before "disable slot".

Signed-off-by: Sneeker Yeh <Sneeker.Yeh@tw.fujitsu.com>
---
 drivers/usb/host/xhci-hub.c |    4 ++++
 drivers/usb/host/xhci.c     |   29 +++++++++++++++++++++++++++++
 drivers/usb/host/xhci.h     |   24 ++++++++++++++++++++++++
 3 files changed, 57 insertions(+)

diff --git a/drivers/usb/host/xhci-hub.c b/drivers/usb/host/xhci-hub.c
index a7865c4..3b8f7fc 100644
--- a/drivers/usb/host/xhci-hub.c
+++ b/drivers/usb/host/xhci-hub.c
@@ -368,6 +368,10 @@ static void xhci_clear_port_change_bit(struct xhci_hcd *xhci, u16 wValue,
 		port_change_bit = "warm(BH) reset";
 		break;
 	case USB_PORT_FEAT_C_CONNECTION:
+		if ((xhci->quirks & XHCI_DISCONNECT_QUIRK) &&
+		    !(readl(addr) & PORT_CONNECT))
+			return;
+
 		status = PORT_CSC;
 		port_change_bit = "connect";
 		break;
diff --git a/drivers/usb/host/xhci.c b/drivers/usb/host/xhci.c
index c50d8d2..aa8e02a 100644
--- a/drivers/usb/host/xhci.c
+++ b/drivers/usb/host/xhci.c
@@ -3584,6 +3584,33 @@ command_cleanup:
 	return ret;
 }
 
+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]);
+}
+
 /*
  * At this point, the struct usb_device is about to go away, the device has
  * disconnected, and all traffic has been stopped and the endpoints have been
@@ -3649,6 +3676,8 @@ void xhci_free_dev(struct usb_hcd *hcd, struct usb_device *udev)
 	xhci_ring_cmd_db(xhci);
 	spin_unlock_irqrestore(&xhci->lock, flags);
 
+	if (xhci->quirks & XHCI_DISCONNECT_QUIRK)
+		xhci_try_to_clear_csc(hcd, udev->portnum);
 	/*
 	 * Event command completion handler will free any data structures
 	 * associated with the slot.  XXX Can free sleep?
diff --git a/drivers/usb/host/xhci.h b/drivers/usb/host/xhci.h
index cc7c5bb..65a65cc 100644
--- a/drivers/usb/host/xhci.h
+++ b/drivers/usb/host/xhci.h
@@ -1560,6 +1560,30 @@ struct xhci_hcd {
 #define XHCI_SPURIOUS_WAKEUP	(1 << 18)
 /* For controllers with a broken beyond repair streams implementation */
 #define XHCI_BROKEN_STREAMS	(1 << 19)
+/*
+ * This issue is defined by a three-way race at disconnect in Synopsis USB3 IP,
+ * 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.
+ *
+ * As a result, when device detached, it's needed to postpone "PORTCSC clear"
+ * behind "disable slot". it's found that all executed ep command related to
+ * disconnetion, are executed before "disable slot".
+ */
+#define XHCI_DISCONNECT_QUIRK	(1 << 20)
 	unsigned int		num_active_eps;
 	unsigned int		limit_active_eps;
 	/* There are two roothubs to keep track of bus suspend info for */
-- 
1.7.9.5

[PATCH v3 2/5] xhci: Platform: Set Synopsis device disconnection quirk based on platform data

[Sneeker Yeh]

If an xhci platform has Synopsis device disconnection errata then enable
XHCI_DISCONNECT_QUIRK quirk flag.

Signed-off-by: Sneeker Yeh <Sneeker.Yeh@tw.fujitsu.com>
---
 drivers/usb/host/xhci-plat.c     |    3 +++
 include/linux/usb/xhci_pdriver.h |    4 ++++
 2 files changed, 7 insertions(+)

diff --git a/drivers/usb/host/xhci-plat.c b/drivers/usb/host/xhci-plat.c
index 08d402b..40beb95 100644
--- a/drivers/usb/host/xhci-plat.c
+++ b/drivers/usb/host/xhci-plat.c
@@ -147,6 +147,9 @@ static int xhci_plat_probe(struct platform_device *pdev)
 	if ((node && of_property_read_bool(node, "usb3-lpm-capable")) ||
 			(pdata && pdata->usb3_lpm_capable))
 		xhci->quirks |= XHCI_LPM_SUPPORT;
+
+	if (pdata && pdata->delay_portcsc_clear)
+		xhci->quirks |= XHCI_DISCONNECT_QUIRK;
 	/*
 	 * Set the xHCI pointer before xhci_plat_setup() (aka hcd_driver.reset)
 	 * is called by usb_add_hcd().
diff --git a/include/linux/usb/xhci_pdriver.h b/include/linux/usb/xhci_pdriver.h
index 376654b..a37a3a5 100644
--- a/include/linux/usb/xhci_pdriver.h
+++ b/include/linux/usb/xhci_pdriver.h
@@ -18,10 +18,14 @@
  *
  * @usb3_lpm_capable:	determines if this xhci platform supports USB3
  *			LPM capability
+ * @delay_portcsc_clear:	determines if Synopsis USB3 core has errata in
+ *				"DWC_USB3_SUSPEND_ON_DISCONNECT_EN=1" hardware
+ *				configuration.
  *
  */
 struct usb_xhci_pdata {
 	unsigned	usb3_lpm_capable:1;
+	unsigned	delay_portcsc_clear:1;
 };
 
 #endif /* __USB_CORE_XHCI_PDRIVER_H */
-- 
1.7.9.5

[PATCH v3 3/5] usb: dwc3: add revision number DWC3_REVISION_290A and DWC3_REVISION_300A

[Sneeker Yeh]

Add the contstant for v2.90a and v3.00a dwc3 IP detection

Signed-off-by: Sneeker Yeh <Sneeker.Yeh@tw.fujitsu.com>
---
 drivers/usb/dwc3/core.h |    2 ++
 1 file changed, 2 insertions(+)

diff --git a/drivers/usb/dwc3/core.h b/drivers/usb/dwc3/core.h
index 4bb9aa6..46f9f9a 100644
--- a/drivers/usb/dwc3/core.h
+++ b/drivers/usb/dwc3/core.h
@@ -776,6 +776,8 @@ struct dwc3 {
 #define DWC3_REVISION_260A	0x5533260a
 #define DWC3_REVISION_270A	0x5533270a
 #define DWC3_REVISION_280A	0x5533280a
+#define DWC3_REVISION_290A	0x5533290a
+#define DWC3_REVISION_300A	0x5533300a
 
 	enum dwc3_ep0_next	ep0_next_event;
 	enum dwc3_ep0_state	ep0state;
-- 
1.7.9.5

[PATCH v3 4/5] usb: dwc3: Add quirk for Synopsis device disconnection errata

[Sneeker Yeh]

Synopsis Designware USB3 IP earlier than v3.00a which is configured in silicon
with DWC_USB3_SUSPEND_ON_DISCONNECT_EN=1, would need a specific quirk to prevent
xhci host controller from dying when device is disconnected.

Since DWC_USB3_SUSPEND_ON_DISCONNECT_EN is an IP configuration whose state
cannot be checked from software in runtime, it has to be enabled via platform
data or device tree.

Signed-off-by: Sneeker Yeh <Sneeker.Yeh@tw.fujitsu.com>
---
 Documentation/devicetree/bindings/usb/dwc3.txt |   17 +++++++++++++++++
 drivers/usb/dwc3/core.c                        |    6 ++++++
 drivers/usb/dwc3/core.h                        |    4 ++++
 drivers/usb/dwc3/host.c                        |    4 ++++
 drivers/usb/dwc3/platform_data.h               |    8 ++++++++
 5 files changed, 39 insertions(+)

diff --git a/Documentation/devicetree/bindings/usb/dwc3.txt b/Documentation/devicetree/bindings/usb/dwc3.txt
index cd7f045..1b78b29 100644
--- a/Documentation/devicetree/bindings/usb/dwc3.txt
+++ b/Documentation/devicetree/bindings/usb/dwc3.txt
@@ -37,6 +37,23 @@ Optional properties:
  - snps,is-utmi-l1-suspend: true when DWC3 asserts output signal
 			utmi_l1_suspend_n, false when asserts utmi_sleep_n
  - snps,hird-threshold: HIRD threshold
+ - snps,has_suspend_on_disconnect: true when IP is configured in silicon with
+			DWC_USB3_SUSPEND_ON_DISCONNECT_EN=1, it can inject a
+			specific quirk to prevent xhci host controller from
+			dying when usb device is disconnected from root hub.
+			Since DWC_USB3_SUSPEND_ON_DISCONNECT_EN is an IP
+			configuration whose state cannot be checked from
+			software in runtime, it has to be enabled via platform
+			data or device tree.
+
+			xhci host dying symptom here is caused by that
+			DWC_USB3_SUSPEND_ON_DISCONNECT_EN=1
+			configuration makes IP auto-suspended after PORTCSC is
+			cleared when usb device detached, then an asynchronous
+			disconnection procedure might fail using endpoint
+			command that suspened IP won't have any response to.
+
+			this issue is fixed when IP version >= 3.00a.
 
 This is usually a subnode to DWC3 glue to which it is connected.
 
diff --git a/drivers/usb/dwc3/core.c b/drivers/usb/dwc3/core.c
index 25ddc39..fbceab1 100644
--- a/drivers/usb/dwc3/core.c
+++ b/drivers/usb/dwc3/core.c
@@ -838,6 +838,9 @@ static int dwc3_probe(struct platform_device *pdev)
 				"snps,tx_de_emphasis_quirk");
 		of_property_read_u8(node, "snps,tx_de_emphasis",
 				&tx_de_emphasis);
+
+		dwc->suspend_on_disconnect_quirk = of_property_read_bool(node,
+				"snps,has_suspend_on_disconnect");
 	} else if (pdata) {
 		dwc->maximum_speed = pdata->maximum_speed;
 		dwc->has_lpm_erratum = pdata->has_lpm_erratum;
@@ -864,6 +867,9 @@ static int dwc3_probe(struct platform_device *pdev)
 		dwc->tx_de_emphasis_quirk = pdata->tx_de_emphasis_quirk;
 		if (pdata->tx_de_emphasis)
 			tx_de_emphasis = pdata->tx_de_emphasis;
+
+		dwc->suspend_on_disconnect_quirk =
+			pdata->has_suspend_on_disconnect;
 	}
 
 	/* default to superspeed if no maximum_speed passed */
diff --git a/drivers/usb/dwc3/core.h b/drivers/usb/dwc3/core.h
index 46f9f9a..c730190 100644
--- a/drivers/usb/dwc3/core.h
+++ b/drivers/usb/dwc3/core.h
@@ -692,6 +692,9 @@ struct dwc3_scratchpad_array {
  * @resize_fifos: tells us it's ok to reconfigure our TxFIFO sizes.
  * @setup_packet_pending: true when there's a Setup Packet in FIFO. Workaround
  * @start_config_issued: true when StartConfig command has been issued
+ * @suspend_on_disconnect_quirk: set if core was configured with
+ *			DWC_USB3_SUSPEND_ON_DISCONNECT_EN=1. Note that there's
+ *			now way for software to detect this in runtime.
  * @three_stage_setup: set if we perform a three phase setup
  * @disable_scramble_quirk: set if we enable the disable scramble quirk
  * @u2exit_lfps_quirk: set if we enable u2exit lfps quirk
@@ -818,6 +821,7 @@ struct dwc3 {
 	unsigned		resize_fifos:1;
 	unsigned		setup_packet_pending:1;
 	unsigned		start_config_issued:1;
+	unsigned		suspend_on_disconnect_quirk:1;
 	unsigned		three_stage_setup:1;
 
 	unsigned		disable_scramble_quirk:1;
diff --git a/drivers/usb/dwc3/host.c b/drivers/usb/dwc3/host.c
index 12bfd3c..9c42074 100644
--- a/drivers/usb/dwc3/host.c
+++ b/drivers/usb/dwc3/host.c
@@ -53,6 +53,10 @@ int dwc3_host_init(struct dwc3 *dwc)
 	pdata.usb3_lpm_capable = 1;
 #endif
 
+	if ((dwc->revision < DWC3_REVISION_300A) &&
+	    dwc->suspend_on_disconnect_quirk)
+		pdata.delay_portcsc_clear = 1;
+
 	ret = platform_device_add_data(xhci, &pdata, sizeof(pdata));
 	if (ret) {
 		dev_err(dwc->dev, "couldn't add platform data to xHCI device\n");
diff --git a/drivers/usb/dwc3/platform_data.h b/drivers/usb/dwc3/platform_data.h
index a3a3b6d..ebc8eb4 100644
--- a/drivers/usb/dwc3/platform_data.h
+++ b/drivers/usb/dwc3/platform_data.h
@@ -20,6 +20,13 @@
 #include <linux/usb/ch9.h>
 #include <linux/usb/otg.h>
 
+/**
+ * struct dwc3_platform_data - platform data of dwc3 controller
+ * @has_suspend_on_disconnect: true if core was configured with
+ *			DWC_USB3_SUSPEND_ON_DISCONNECT_EN=1. Note that there's
+ *			now way for software to detect this in runtime.
+ */
+
 struct dwc3_platform_data {
 	enum usb_device_speed maximum_speed;
 	enum usb_dr_mode dr_mode;
@@ -32,6 +39,7 @@ struct dwc3_platform_data {
 
 	unsigned disable_scramble_quirk:1;
 	unsigned has_lpm_erratum:1;
+	unsigned has_suspend_on_disconnect:1;
 	unsigned u2exit_lfps_quirk:1;
 	unsigned u2ss_inp3_quirk:1;
 	unsigned req_p1p2p3_quirk:1;
-- 
1.7.9.5

[PATCH v3 5/5] usb: dwc3: add Fujitsu Specific Glue layer

[Sneeker Yeh]

This patch adds support for Synopsis DesignWare USB3 IP Core found
on Fujitsu Socs.

Signed-off-by: Sneeker Yeh <Sneeker.Yeh@tw.fujitsu.com>
---
 .../devicetree/bindings/usb/fujitsu-dwc3.txt       |   33 ++++
 drivers/usb/dwc3/Kconfig                           |   11 ++
 drivers/usb/dwc3/Makefile                          |    1 +
 drivers/usb/dwc3/dwc3-mb86s70.c                    |  206 ++++++++++++++++++++
 4 files changed, 251 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/usb/fujitsu-dwc3.txt
 create mode 100644 drivers/usb/dwc3/dwc3-mb86s70.c

diff --git a/Documentation/devicetree/bindings/usb/fujitsu-dwc3.txt b/Documentation/devicetree/bindings/usb/fujitsu-dwc3.txt
new file mode 100644
index 0000000..be091eb
--- /dev/null
+++ b/Documentation/devicetree/bindings/usb/fujitsu-dwc3.txt
@@ -0,0 +1,33 @@
+FUJITSU GLUE COMPONENTS
+
+MB86S7x DWC3 GLUE
+- compatible:	Should be "fujitsu,mb86s70-dwc3"
+- clocks:	from common clock binding, handle to usb clock.
+- clock-names:	Should contain the following:
+  "core"	Master/Core clock needs to run at a minimum of 125 MHz to
+		support a 4 Gbps IN or 4 Gbps OUT
+		transfer at a given time.
+
+Sub-nodes:
+The dwc3 core should be added as subnode to MB86S7x dwc3 glue.
+- dwc3 :
+   The binding details of dwc3 can be found in:
+   Documentation/devicetree/bindings/usb/dwc3.txt
+
+Example device nodes:
+
+	usb3host: mb86s70_usb3host {
+		compatible = "fujitsu,mb86s70-dwc3";
+		clocks = <&clk_alw_1_1>;
+		clock-names = "core";
+		#address-cells = <2>;
+		#size-cells = <1>;
+		ranges;
+
+		dwc3@32200000 {
+			compatible = "synopsys,dwc3";
+			reg = <0 0x32300000 0x100000>;
+			interrupts = <0 412 0x4>,
+				<0 414 0x4>;
+		};
+	};
diff --git a/drivers/usb/dwc3/Kconfig b/drivers/usb/dwc3/Kconfig
index 58b5b2c..3390d42 100644
--- a/drivers/usb/dwc3/Kconfig
+++ b/drivers/usb/dwc3/Kconfig
@@ -61,6 +61,17 @@ config USB_DWC3_EXYNOS
 	  Recent Exynos5 SoCs ship with one DesignWare Core USB3 IP inside,
 	  say 'Y' or 'M' if you have one such device.
 
+config USB_DWC3_MB86S70
+	tristate "MB86S70 Designware USB3 Platform code"
+	default USB_DWC3
+	help
+	  MB86S7X SOC ship with DesignWare Core USB3 IP inside,
+	  this implementation also integrated Fujitsu USB PHY inside
+	  this Core USB3 IP.
+
+	  say 'Y' or 'M' if you have one such device.
+
+
 config USB_DWC3_PCI
 	tristate "PCIe-based Platforms"
 	depends on PCI
diff --git a/drivers/usb/dwc3/Makefile b/drivers/usb/dwc3/Makefile
index bb34fbc..05d1de2 100644
--- a/drivers/usb/dwc3/Makefile
+++ b/drivers/usb/dwc3/Makefile
@@ -38,3 +38,4 @@ obj-$(CONFIG_USB_DWC3_PCI)		+= dwc3-pci.o
 obj-$(CONFIG_USB_DWC3_KEYSTONE)		+= dwc3-keystone.o
 obj-$(CONFIG_USB_DWC3_QCOM)		+= dwc3-qcom.o
 obj-$(CONFIG_USB_DWC3_ST)		+= dwc3-st.o
+obj-$(CONFIG_USB_DWC3_MB86S70)		+= dwc3-mb86s70.o
diff --git a/drivers/usb/dwc3/dwc3-mb86s70.c b/drivers/usb/dwc3/dwc3-mb86s70.c
new file mode 100644
index 0000000..301be76
--- /dev/null
+++ b/drivers/usb/dwc3/dwc3-mb86s70.c
@@ -0,0 +1,206 @@
+/**
+ * dwc3-mb86s70.c - Fujitsu mb86s70 DWC3 Specific Glue layer
+ *
+ * Copyright (c) 2013 - 2014 FUJITSU SEMICONDUCTOR LIMITED
+ *		http://jp.fujitsu.com/group/fsl
+ *
+ * Authors: Alice Chan <Alice.Chan@tw.fujitsu.com>
+ *	    Sneeker Yeh <Sneeker.Yeh@tw.fujitsu.com>
+ * based on dwc3-exynos.c
+ *
+ * This program is free software; you can redistribute it and/or modify
+ * it under the terms of the GNU General Public License as published by
+ * the Free Software Foundation; either version 2 of the License, or
+ * (at your option) any later version.
+ */
+
+#include <linux/module.h>
+#include <linux/kernel.h>
+#include <linux/slab.h>
+#include <linux/platform_device.h>
+#include <linux/dma-mapping.h>
+#include <linux/interrupt.h>
+#include <linux/of.h>
+#include <linux/of_platform.h>
+#include <linux/pm_runtime.h>
+#include <linux/clk.h>
+
+struct dwc3_mb86s70 {
+	struct device	*dev;
+	struct clk	*clks[5];
+	u8		clk_cnt;
+};
+
+static int dwc3_mb86s70_clk_control(struct device *dev, bool on)
+{
+	struct dwc3_mb86s70 *priv = dev_get_drvdata(dev);
+	int ret, i = priv->clk_cnt;
+
+	if (!on)
+		goto clock_off;
+
+	for (i = 0; i < priv->clk_cnt; i++) {
+		ret = clk_prepare_enable(priv->clks[i]);
+		if (ret) {
+			dev_err(dev, "failed to enable clock[%d]\n", i);
+			on = ret;
+			goto clock_off;
+		}
+	}
+
+	return 0;
+
+clock_off:
+	for (; i > 0;)
+		clk_disable_unprepare(priv->clks[--i]);
+
+	return on;
+}
+
+static int dwc3_mb86s70_remove_child(struct device *dev, void *unused)
+{
+	struct platform_device *pdev = to_platform_device(dev);
+
+	of_device_unregister(pdev);
+
+	return 0;
+}
+
+static int dwc3_mb86s70_probe(struct platform_device *pdev)
+{
+	struct dwc3_mb86s70	*priv;
+	struct device		*dev = &pdev->dev;
+	struct device_node	*node = dev->of_node;
+	int			ret, i;
+	struct clk		*clk;
+
+	priv = devm_kzalloc(dev, sizeof(*priv), GFP_KERNEL);
+	if (!priv) {
+		ret = -ENOMEM;
+		goto err;
+	}
+
+	platform_set_drvdata(pdev, priv);
+	priv->dev = dev;
+
+	dma_coerce_mask_and_coherent(&pdev->dev, DMA_BIT_MASK(32));
+
+	for (i = 0; i < ARRAY_SIZE(priv->clks); i++) {
+		clk = of_clk_get(dev->of_node, i);
+		if (IS_ERR(clk))
+			break;
+		priv->clks[i] = clk;
+	}
+	priv->clk_cnt = i;
+	if (!i) {
+		dev_err(dev, "clock not found\n");
+		ret = PTR_ERR(clk);
+		goto err;
+	}
+
+	ret = dwc3_mb86s70_clk_control(dev, true);
+	if (ret)
+		goto err_clk;
+
+	pm_runtime_set_active(dev);
+	pm_runtime_enable(dev);
+	ret = pm_runtime_get(dev);
+	if (ret < 0) {
+		dev_err(dev, "get_sync failed with err %d\n", ret);
+		ret = -ENODEV;
+		goto err_pm;
+	}
+
+	if (node) {
+		ret = of_platform_populate(node, NULL, NULL, dev);
+		if (!ret)
+			return 0;
+		dev_err(dev, "failed to add dwc3 core\n");
+	}
+	dev_err(dev, "no device node, failed to add dwc3 core\n");
+	ret = -ENODEV;
+
+	pm_runtime_put_sync(dev);
+err_pm:
+	pm_runtime_disable(dev);
+err_clk:
+	for (i = 0; i < priv->clk_cnt; i++)
+		clk_put(priv->clks[i]);
+err:
+
+	return ret;
+}
+
+static int dwc3_mb86s70_remove(struct platform_device *pdev)
+{
+	struct dwc3_mb86s70 *priv = dev_get_drvdata(&pdev->dev);
+	int i;
+
+	device_for_each_child(&pdev->dev, NULL, dwc3_mb86s70_remove_child);
+
+	pm_runtime_put_sync(&pdev->dev);
+	pm_runtime_disable(&pdev->dev);
+
+	dwc3_mb86s70_clk_control(&pdev->dev, false);
+
+	for (i = 0; i < priv->clk_cnt ; i++)
+		clk_put(priv->clks[i]);
+
+	return 0;
+}
+
+static const struct of_device_id mb86s70_dwc3_match[] = {
+	{ .compatible = "fujitsu,mb86s70-dwc3" },
+	{},
+};
+MODULE_DEVICE_TABLE(of, mb86s70_dwc3_match);
+
+#ifdef CONFIG_PM_SLEEP
+static int dwc3_mb86s70_suspend(struct device *dev)
+{
+	dwc3_mb86s70_clk_control(dev, false);
+
+	return 0;
+}
+
+static int dwc3_mb86s70_resume(struct device *dev)
+{
+	int ret;
+
+	ret = dwc3_mb86s70_clk_control(dev, true);
+	if (ret)
+		return ret;
+
+	/* runtime set active to reflect active state. */
+	pm_runtime_disable(dev);
+	pm_runtime_set_active(dev);
+	pm_runtime_enable(dev);
+
+	return 0;
+}
+
+static const struct dev_pm_ops dwc3_mb86s70_dev_pm_ops = {
+	SET_SYSTEM_SLEEP_PM_OPS(dwc3_mb86s70_suspend, dwc3_mb86s70_resume)
+};
+
+#define DEV_PM_OPS	(&dwc3_mb86s70_dev_pm_ops)
+#else
+#define DEV_PM_OPS	NULL
+#endif /* CONFIG_PM_SLEEP */
+
+static struct platform_driver dwc3_mb86s70_driver = {
+	.probe		= dwc3_mb86s70_probe,
+	.remove		= dwc3_mb86s70_remove,
+	.driver		= {
+		.name	= "mb86s70-dwc3",
+		.of_match_table = of_match_ptr(mb86s70_dwc3_match),
+		.pm	= DEV_PM_OPS,
+	},
+};
+
+module_platform_driver(dwc3_mb86s70_driver);
+
+MODULE_ALIAS("platform:mb86s70-dwc3");
+MODULE_AUTHOR("Alice Chan <Alice.Chan@tw.fujitsu.com>");
+MODULE_LICENSE("GPL");
+MODULE_DESCRIPTION("DesignWare USB3 mb86s70 Glue Layer");
-- 
1.7.9.5

Re: [PATCH v3 0/5] Add support for Fujitsu USB host controller

[Felipe Balbi]

[-- Attachment #1: Type: text/plain, Size: 1469 bytes --]

Hi,

On Sun, Jan 25, 2015 at 04:13:23PM +0800, Sneeker Yeh wrote:
> These patches add support for XHCI compliant Host controller found
> on Fujitsu Socs, and are based on http://lwn.net/Articles/629162/
> The first patch is to add Fujitsu glue layer of Synopsis DesignWare USB3 driver
> and last four patch is about quirk implementation of errata in Synopsis
> DesignWare USB3 IP.
> 
> Patch 1 introduces a quirk with device disconnection management necessary
> Synopsys Designware USB3 IP with versions < 3.00a and hardware configuration
> DWC_USB3_SUSPEND_ON_DISCONNECT_EN=1. It solves a problem where without the
> quirk, that host controller will die after a usb device is disconnected from
> port of root hub.
> 
> Patch 2 is to set Synopsis quirk in xhci platform driver based on xhci platform
> data.
> 
> Patch 3 is to add a revison number 2.90a and 3.00a of Synopsis DesignWare USB3
> IP core driver.
> 
> Patch 4 introduces using a quirk based on a errata of Synopsis
> DesignWare USB3 IP which is versions < 3.00a and has hardware configuration
> DWC_USB3_SUSPEND_ON_DISCONNECT_EN=1, which cannot be read from software. As a
> result this quirk has to be enabled via platform data or device tree.
> 
> Patch 5 introduces Fujitsu Specific Glue layer in Synopsis DesignWare USB3 IP
> driver. 
> 

Mathias, let me know how you want to handle this. Either I take them
all, or you take them all. What do you prefer ?

-- 
balbi

[-- Attachment #2: Digital signature --]
[-- Type: application/pgp-signature, Size: 819 bytes --]

Re: [PATCH v3 0/5] Add support for Fujitsu USB host controller

[Felipe Balbi]

[-- Attachment #1: Type: text/plain, Size: 1609 bytes --]

On Tue, Jan 27, 2015 at 09:22:50AM -0600, Felipe Balbi wrote:
> Hi,
> 
> On Sun, Jan 25, 2015 at 04:13:23PM +0800, Sneeker Yeh wrote:
> > These patches add support for XHCI compliant Host controller found
> > on Fujitsu Socs, and are based on http://lwn.net/Articles/629162/
> > The first patch is to add Fujitsu glue layer of Synopsis DesignWare USB3 driver
> > and last four patch is about quirk implementation of errata in Synopsis
> > DesignWare USB3 IP.
> > 
> > Patch 1 introduces a quirk with device disconnection management necessary
> > Synopsys Designware USB3 IP with versions < 3.00a and hardware configuration
> > DWC_USB3_SUSPEND_ON_DISCONNECT_EN=1. It solves a problem where without the
> > quirk, that host controller will die after a usb device is disconnected from
> > port of root hub.
> > 
> > Patch 2 is to set Synopsis quirk in xhci platform driver based on xhci platform
> > data.
> > 
> > Patch 3 is to add a revison number 2.90a and 3.00a of Synopsis DesignWare USB3
> > IP core driver.
> > 
> > Patch 4 introduces using a quirk based on a errata of Synopsis
> > DesignWare USB3 IP which is versions < 3.00a and has hardware configuration
> > DWC_USB3_SUSPEND_ON_DISCONNECT_EN=1, which cannot be read from software. As a
> > result this quirk has to be enabled via platform data or device tree.
> > 
> > Patch 5 introduces Fujitsu Specific Glue layer in Synopsis DesignWare USB3 IP
> > driver. 
> > 
> 
> Mathias, let me know how you want to handle this. Either I take them
> all, or you take them all. What do you prefer ?

Mathias ?

-- 
balbi

[-- Attachment #2: Digital signature --]
[-- Type: application/pgp-signature, Size: 819 bytes --]

Re: [PATCH v3 0/5] Add support for Fujitsu USB host controller

[Felipe Balbi]

[-- Attachment #1: Type: text/plain, Size: 1788 bytes --]

Hi,

On Thu, Jan 29, 2015 at 10:23:12AM -0600, Felipe Balbi wrote:
> On Tue, Jan 27, 2015 at 09:22:50AM -0600, Felipe Balbi wrote:
> > Hi,
> > 
> > On Sun, Jan 25, 2015 at 04:13:23PM +0800, Sneeker Yeh wrote:
> > > These patches add support for XHCI compliant Host controller found
> > > on Fujitsu Socs, and are based on http://lwn.net/Articles/629162/
> > > The first patch is to add Fujitsu glue layer of Synopsis DesignWare USB3 driver
> > > and last four patch is about quirk implementation of errata in Synopsis
> > > DesignWare USB3 IP.
> > > 
> > > Patch 1 introduces a quirk with device disconnection management necessary
> > > Synopsys Designware USB3 IP with versions < 3.00a and hardware configuration
> > > DWC_USB3_SUSPEND_ON_DISCONNECT_EN=1. It solves a problem where without the
> > > quirk, that host controller will die after a usb device is disconnected from
> > > port of root hub.
> > > 
> > > Patch 2 is to set Synopsis quirk in xhci platform driver based on xhci platform
> > > data.
> > > 
> > > Patch 3 is to add a revison number 2.90a and 3.00a of Synopsis DesignWare USB3
> > > IP core driver.
> > > 
> > > Patch 4 introduces using a quirk based on a errata of Synopsis
> > > DesignWare USB3 IP which is versions < 3.00a and has hardware configuration
> > > DWC_USB3_SUSPEND_ON_DISCONNECT_EN=1, which cannot be read from software. As a
> > > result this quirk has to be enabled via platform data or device tree.
> > > 
> > > Patch 5 introduces Fujitsu Specific Glue layer in Synopsis DesignWare USB3 IP
> > > driver. 
> > > 
> > 
> > Mathias, let me know how you want to handle this. Either I take them
> > all, or you take them all. What do you prefer ?
> 
> Mathias ?

Mathias, a reminder on this series.

-- 
balbi

[-- Attachment #2: Digital signature --]
[-- Type: application/pgp-signature, Size: 819 bytes --]

Re: [PATCH v3 0/5] Add support for Fujitsu USB host controller

[Sneeker Yeh]

Hi

2015-01-31 0:38 GMT+08:00 Felipe Balbi <balbi@ti.com>:
> Hi,
>
> On Thu, Jan 29, 2015 at 10:23:12AM -0600, Felipe Balbi wrote:
>> On Tue, Jan 27, 2015 at 09:22:50AM -0600, Felipe Balbi wrote:
>> > Hi,
>> >
>> > On Sun, Jan 25, 2015 at 04:13:23PM +0800, Sneeker Yeh wrote:
>> > > These patches add support for XHCI compliant Host controller found
>> > > on Fujitsu Socs, and are based on http://lwn.net/Articles/629162/
>> > > The first patch is to add Fujitsu glue layer of Synopsis DesignWare USB3 driver
>> > > and last four patch is about quirk implementation of errata in Synopsis
>> > > DesignWare USB3 IP.
>> > >
>> > > Patch 1 introduces a quirk with device disconnection management necessary
>> > > Synopsys Designware USB3 IP with versions < 3.00a and hardware configuration
>> > > DWC_USB3_SUSPEND_ON_DISCONNECT_EN=1. It solves a problem where without the
>> > > quirk, that host controller will die after a usb device is disconnected from
>> > > port of root hub.
>> > >
>> > > Patch 2 is to set Synopsis quirk in xhci platform driver based on xhci platform
>> > > data.
>> > >
>> > > Patch 3 is to add a revison number 2.90a and 3.00a of Synopsis DesignWare USB3
>> > > IP core driver.
>> > >
>> > > Patch 4 introduces using a quirk based on a errata of Synopsis
>> > > DesignWare USB3 IP which is versions < 3.00a and has hardware configuration
>> > > DWC_USB3_SUSPEND_ON_DISCONNECT_EN=1, which cannot be read from software. As a
>> > > result this quirk has to be enabled via platform data or device tree.
>> > >
>> > > Patch 5 introduces Fujitsu Specific Glue layer in Synopsis DesignWare USB3 IP
>> > > driver.
>> > >
>> >
>> > Mathias, let me know how you want to handle this. Either I take them
>> > all, or you take them all. What do you prefer ?
>>
>> Mathias ?
>
> Mathias, a reminder on this series.

Would any problem is still in my patchset?
e.g. I might still not arrange these patch in a appropriate order so
that Mathias cannot review and accept these?

BR,
Sneeker, sincerely

>
> --
> balbi

Re: [PATCH v3 0/5] Add support for Fujitsu USB host controller

[Mathias Nyman]

On 10.02.2015 17:43, Sneeker Yeh wrote:
> Hi
> 
> 2015-01-31 0:38 GMT+08:00 Felipe Balbi <balbi@ti.com>:
>> Hi,
>>
>> On Thu, Jan 29, 2015 at 10:23:12AM -0600, Felipe Balbi wrote:
>>> On Tue, Jan 27, 2015 at 09:22:50AM -0600, Felipe Balbi wrote:
>>>> Hi,
>>>>
>>>> On Sun, Jan 25, 2015 at 04:13:23PM +0800, Sneeker Yeh wrote:
>>>>> These patches add support for XHCI compliant Host controller found
>>>>> on Fujitsu Socs, and are based on http://lwn.net/Articles/629162/
>>>>> The first patch is to add Fujitsu glue layer of Synopsis DesignWare USB3 driver
>>>>> and last four patch is about quirk implementation of errata in Synopsis
>>>>> DesignWare USB3 IP.
>>>>>
>>>>> Patch 1 introduces a quirk with device disconnection management necessary
>>>>> Synopsys Designware USB3 IP with versions < 3.00a and hardware configuration
>>>>> DWC_USB3_SUSPEND_ON_DISCONNECT_EN=1. It solves a problem where without the
>>>>> quirk, that host controller will die after a usb device is disconnected from
>>>>> port of root hub.
>>>>>
>>>>> Patch 2 is to set Synopsis quirk in xhci platform driver based on xhci platform
>>>>> data.
>>>>>
>>>>> Patch 3 is to add a revison number 2.90a and 3.00a of Synopsis DesignWare USB3
>>>>> IP core driver.
>>>>>
>>>>> Patch 4 introduces using a quirk based on a errata of Synopsis
>>>>> DesignWare USB3 IP which is versions < 3.00a and has hardware configuration
>>>>> DWC_USB3_SUSPEND_ON_DISCONNECT_EN=1, which cannot be read from software. As a
>>>>> result this quirk has to be enabled via platform data or device tree.
>>>>>
>>>>> Patch 5 introduces Fujitsu Specific Glue layer in Synopsis DesignWare USB3 IP
>>>>> driver.
>>>>>
>>>>
>>>> Mathias, let me know how you want to handle this. Either I take them
>>>> all, or you take them all. What do you prefer ?
>>>
>>> Mathias ?
>>
>> Mathias, a reminder on this series.
> 
> Would any problem is still in my patchset?
> e.g. I might still not arrange these patch in a appropriate order so
> that Mathias cannot review and accept these?
> 

Sorry about the delay.
Let me take a better look at the first patch and reasons behind the
race. I want to check if a quirk is enough or if we need to dig deeper into xhci.

I'll try to do it today still

-Mathias

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

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?

Would it make sense to check those bits in xhci_enqueue_urb, and just return error
in the xhci_urb_enqueue() if device is not connected? Then there wouldn't be a need for any quirk
at all.

If that doesn't work then this patch looks good in general. See comments below

> this defect would result in this when device detached:
> -------------------------------------------------------
> [   99.603544] usb 4-1: USB disconnect, device number 2
> [  104.615254] xhci-hcd xhci-hcd.0.auto: xHCI host not responding to stop endpoint command.
> [  104.623362] xhci-hcd xhci-hcd.0.auto: Assuming host is dying, halting host.
> [  104.653261] xhci-hcd xhci-hcd.0.auto: Host not halted after 16000 microseconds.
> [  104.660584] xhci-hcd xhci-hcd.0.auto: Non-responsive xHCI host is not halting.
> [  104.667817] xhci-hcd xhci-hcd.0.auto: Completing active URBs anyway.
> [  104.674198] xhci-hcd xhci-hcd.0.auto: HC died; cleaning up
> ------------------------------------------------------
> As a result, when device detached, we desired to postpone "PORTCSC clear" behind
> "disable slot". it's found that all executed ep command related to disconnetion,
> are executed before "disable slot".
> 
> Signed-off-by: Sneeker Yeh <Sneeker.Yeh@tw.fujitsu.com>
> ---
>  drivers/usb/host/xhci-hub.c |    4 ++++
>  drivers/usb/host/xhci.c     |   29 +++++++++++++++++++++++++++++
>  drivers/usb/host/xhci.h     |   24 ++++++++++++++++++++++++
>  3 files changed, 57 insertions(+)
> 
> diff --git a/drivers/usb/host/xhci-hub.c b/drivers/usb/host/xhci-hub.c
> index a7865c4..3b8f7fc 100644
> --- a/drivers/usb/host/xhci-hub.c
> +++ b/drivers/usb/host/xhci-hub.c
> @@ -368,6 +368,10 @@ static void xhci_clear_port_change_bit(struct xhci_hcd *xhci, u16 wValue,
>  		port_change_bit = "warm(BH) reset";
>  		break;
>  	case USB_PORT_FEAT_C_CONNECTION:
> +		if ((xhci->quirks & XHCI_DISCONNECT_QUIRK) &&
> +		    !(readl(addr) & PORT_CONNECT))
> +			return;
> +

New port status event are prevented until CSC is cleared, not sure if there's any harm in that?

>  		status = PORT_CSC;
>  		port_change_bit = "connect";
>  		break;
> diff --git a/drivers/usb/host/xhci.c b/drivers/usb/host/xhci.c
> index c50d8d2..aa8e02a 100644
> --- a/drivers/usb/host/xhci.c
> +++ b/drivers/usb/host/xhci.c
> @@ -3584,6 +3584,33 @@ command_cleanup:
>  	return ret;
>  }
>  
> +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]);

Shouldn't this be writel(...,port_array[dev_port_num - 1]) ?

-Mathias

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

[Alan Stern]

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?

> Would it make sense to check those bits in xhci_enqueue_urb, and just return error
> in the xhci_urb_enqueue() if device is not connected? Then there wouldn't be a need for any quirk
> at all.

That wouldn't help URBs that were already enqueued when the disconnect 
occurred.

Alan Stern

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


 

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 

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

[Sneeker Yeh]

hi Mathias:

thanks for reviewing these patch,
and sorry for replying lately~

2015-02-12 21:50 GMT+08:00 Mathias Nyman <mathias.nyman@intel.com>:
> 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.
>>
>
> 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?

yes.

>
> 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?
>
> Would it make sense to check those bits in xhci_enqueue_urb, and just return error
> in the xhci_urb_enqueue() if device is not connected? Then there wouldn't be a need for any quirk
> at all.

ya I tried this before, it would work if i stop enqueue when i found
device detached,
but sometime it won't work when device might be detached just right
after i done enqueue, just like Alan mentioned.

>
> If that doesn't work then this patch looks good in general. See comments below
>
>> this defect would result in this when device detached:
>> -------------------------------------------------------
>> [   99.603544] usb 4-1: USB disconnect, device number 2
>> [  104.615254] xhci-hcd xhci-hcd.0.auto: xHCI host not responding to stop endpoint command.
>> [  104.623362] xhci-hcd xhci-hcd.0.auto: Assuming host is dying, halting host.
>> [  104.653261] xhci-hcd xhci-hcd.0.auto: Host not halted after 16000 microseconds.
>> [  104.660584] xhci-hcd xhci-hcd.0.auto: Non-responsive xHCI host is not halting.
>> [  104.667817] xhci-hcd xhci-hcd.0.auto: Completing active URBs anyway.
>> [  104.674198] xhci-hcd xhci-hcd.0.auto: HC died; cleaning up
>> ------------------------------------------------------
>> As a result, when device detached, we desired to postpone "PORTCSC clear" behind
>> "disable slot". it's found that all executed ep command related to disconnetion,
>> are executed before "disable slot".
>>
>> Signed-off-by: Sneeker Yeh <Sneeker.Yeh@tw.fujitsu.com>
>> ---
>>  drivers/usb/host/xhci-hub.c |    4 ++++
>>  drivers/usb/host/xhci.c     |   29 +++++++++++++++++++++++++++++
>>  drivers/usb/host/xhci.h     |   24 ++++++++++++++++++++++++
>>  3 files changed, 57 insertions(+)
>>
>> diff --git a/drivers/usb/host/xhci-hub.c b/drivers/usb/host/xhci-hub.c
>> index a7865c4..3b8f7fc 100644
>> --- a/drivers/usb/host/xhci-hub.c
>> +++ b/drivers/usb/host/xhci-hub.c
>> @@ -368,6 +368,10 @@ static void xhci_clear_port_change_bit(struct xhci_hcd *xhci, u16 wValue,
>>               port_change_bit = "warm(BH) reset";
>>               break;
>>       case USB_PORT_FEAT_C_CONNECTION:
>> +             if ((xhci->quirks & XHCI_DISCONNECT_QUIRK) &&
>> +                 !(readl(addr) & PORT_CONNECT))
>> +                     return;
>> +
>
> New port status event are prevented until CSC is cleared, not sure if there's any harm in that?

hub thread would be aware of device detach before he try to clear PORTCSC, IIUC,
Despite I skip clearing PORTCSC here, whole device detach procedure
still behave normally.

>
>>               status = PORT_CSC;
>>               port_change_bit = "connect";
>>               break;
>> diff --git a/drivers/usb/host/xhci.c b/drivers/usb/host/xhci.c
>> index c50d8d2..aa8e02a 100644
>> --- a/drivers/usb/host/xhci.c
>> +++ b/drivers/usb/host/xhci.c
>> @@ -3584,6 +3584,33 @@ command_cleanup:
>>       return ret;
>>  }
>>
>> +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]);
>
> Shouldn't this be writel(...,port_array[dev_port_num - 1]) ?

yes, thanks for correcting this,
and I also would like to add xhci_port_state_to_neutral() you mentioned.
what would you think if I modify it like this?

+       /* write 1 to clear */
+       if (!(status & PORT_CONNECT) && (status & PORT_CSC)) {
+               status = xhci_port_state_to_neutral(status);
+               writel(status | PORT_CSC, port_array[dev_port_num - 1]);
+       }

Much appreciate,
Sneeker


>
> -Mathias

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

[Sneeker Yeh]

Hi Alan:

thanks for comment it,
and sorry that a little bit late for replying,

2015-02-12 23:18 GMT+08:00 Alan Stern <stern@rowland.harvard.edu>:
> 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?

it's an IP parameter that can't be turn off via any register ><.

I guess Synopsis can insisted that xHCI specification doesn't
explicitly state that hardware designer shouldn't do auto-suspend when
device is attached, especially that definition of the auto-suspend is
their own specific low power state.
IIRC, this is point of view from Synopsis.

I wonder maybe they just didn't have a good assumption when
implementing auto-suspend, e.g. they shouldn't take PORTCSC clearing
as a commitment that software has done all the device detach task, so
that IP can go into suspend. it seems more reasonable to take
slot-disabling as a commitment to a start of auto-suspend.

anyway the "errata" would be disclosed recently, and Synopsis plan to
fix it in their new silicon version IP.
Old version IP has to live happily with some patch that can workaround
this monster i think.

Much appreciate,
Sneeker

>
>> 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?
>
>> Would it make sense to check those bits in xhci_enqueue_urb, and just return error
>> in the xhci_urb_enqueue() if device is not connected? Then there wouldn't be a need for any quirk
>> at all.
>
> That wouldn't help URBs that were already enqueued when the disconnect
> occurred.
>
> Alan Stern
>

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

[Mathias Nyman]

On 15.02.2015 16:29, Sneeker Yeh wrote:
> hi Mathias:
> 
> thanks for reviewing these patch,
> and sorry for replying lately~
> 

>>> +     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]);
>>
>> Shouldn't this be writel(...,port_array[dev_port_num - 1]) ?
> 
> yes, thanks for correcting this,
> and I also would like to add xhci_port_state_to_neutral() you mentioned.
> what would you think if I modify it like this?
> 
> +       /* write 1 to clear */
> +       if (!(status & PORT_CONNECT) && (status & PORT_CSC)) {
> +               status = xhci_port_state_to_neutral(status);
> +               writel(status | PORT_CSC, port_array[dev_port_num - 1]);
> +       }
> 

Looks good to me

-Mathias