All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH v3 0/3] usb: xhci: Add debug capability support in xhci
@ 2017-09-05  1:58 Lu Baolu
  2017-09-05  1:58 ` [PATCH v3 1/3] usb: xhci: Make some static functions global Lu Baolu
                   ` (2 more replies)
  0 siblings, 3 replies; 21+ messages in thread
From: Lu Baolu @ 2017-09-05  1:58 UTC (permalink / raw)
  To: Mathias Nyman, Greg Kroah-Hartman; +Cc: linux-usb, linux-kernel, Lu Baolu

Hi,

This series is for xHCI debug capability (spec section 7.6.8) support
in the xHCI driver.

xHCI compatible USB host controllers(i.e. super-speed USB3 controllers)
can be implemented with the Debug Capability(DbC). It presents a debug
device which is fully compliant with the USB framework and provides the
equivalent of a very high performance full-duplex serial link. The debug
capability operation model and registers interface are defined in 7.6.8
of the xHCI specification, revision 1.1.

The DbC debug device shares a root port with the xHCI host. By default,
the debug capability is disabled and the root port is assigned to xHCI.
When the DbC is enabled, the root port will be assigned to the DbC debug
device, and the xHCI sees nothing on this port. This implementation uses
a sysfs node named <dbc> under the xHCI device to manage the enabling
and disabling of the debug capability.

When the debug capability is enabled, it will present a debug device
through the debug port. This debug device is fully compliant with the
USB3 framework, and it can be enumerated by a debug host on the other
end of the USB link. As soon as the debug device is configured, a TTY
serial device named /dev/ttyGSn will be created.

One use of this link is running a login service on the debug target.
Hence it can be remote accessed by a debug host. Another use case can
probably be found in servers. It provides a peer-to-peer USB link
between two host-only machines. This provides a reasonable out-of-band
communication method between two servers.

Best regards,
Lu Baolu

---
Change log:
v1->v2:
  - Add a new patch to move u_serial.c from drivers/usb/gadget/function
    to drivers/usb/common/ and move u_serial.h to include/linux/usb/.
v2->v3:
  - Remove the use of u_serial and add a new tty glue for debug capability.

Lu Baolu (3):
  usb: xhci: Make some static functions global
  usb: xhci: Add DbC support in xHCI driver
  usb: doc: Update document for USB3 debug port usage

 .../ABI/testing/sysfs-bus-pci-drivers-xhci_hcd     |   25 +
 Documentation/driver-api/usb/usb3-debug-port.rst   |   68 ++
 drivers/usb/host/Kconfig                           |    9 +
 drivers/usb/host/Makefile                          |    5 +
 drivers/usb/host/xhci-dbgcap.c                     | 1016 ++++++++++++++++++++
 drivers/usb/host/xhci-dbgcap.h                     |  247 +++++
 drivers/usb/host/xhci-dbgtty.c                     |  586 +++++++++++
 drivers/usb/host/xhci-mem.c                        |   94 +-
 drivers/usb/host/xhci-ring.c                       |    4 +-
 drivers/usb/host/xhci-trace.h                      |   60 ++
 drivers/usb/host/xhci.c                            |   10 +
 drivers/usb/host/xhci.h                            |   17 +-
 12 files changed, 2099 insertions(+), 42 deletions(-)
 create mode 100644 Documentation/ABI/testing/sysfs-bus-pci-drivers-xhci_hcd
 create mode 100644 drivers/usb/host/xhci-dbgcap.c
 create mode 100644 drivers/usb/host/xhci-dbgcap.h
 create mode 100644 drivers/usb/host/xhci-dbgtty.c

-- 
2.7.4

^ permalink raw reply	[flat|nested] 21+ messages in thread

* [PATCH v3 1/3] usb: xhci: Make some static functions global
  2017-09-05  1:58 [PATCH v3 0/3] usb: xhci: Add debug capability support in xhci Lu Baolu
@ 2017-09-05  1:58 ` Lu Baolu
  2017-09-05  1:58 ` [PATCH v3 2/3] usb: xhci: Add DbC support in xHCI driver Lu Baolu
  2017-09-05  1:59 ` [PATCH v3 3/3] usb: doc: Update document for USB3 debug port usage Lu Baolu
  2 siblings, 0 replies; 21+ messages in thread
From: Lu Baolu @ 2017-09-05  1:58 UTC (permalink / raw)
  To: Mathias Nyman, Greg Kroah-Hartman; +Cc: linux-usb, linux-kernel, Lu Baolu

This patch makes some static functions global to avoid duplications
in different files. These functions can be used in the implementation
of xHCI debug capability. There is no functional change.

Signed-off-by: Lu Baolu <baolu.lu@linux.intel.com>
---
 drivers/usb/host/xhci-mem.c  | 94 ++++++++++++++++++++++++++------------------
 drivers/usb/host/xhci-ring.c |  4 +-
 drivers/usb/host/xhci.h      | 16 +++++++-
 3 files changed, 72 insertions(+), 42 deletions(-)

diff --git a/drivers/usb/host/xhci-mem.c b/drivers/usb/host/xhci-mem.c
index 2a82c92..1ccb1c5 100644
--- a/drivers/usb/host/xhci-mem.c
+++ b/drivers/usb/host/xhci-mem.c
@@ -368,7 +368,7 @@ static int xhci_alloc_segments_for_ring(struct xhci_hcd *xhci,
  * Set the end flag and the cycle toggle bit on the last segment.
  * See section 4.9.1 and figures 15 and 16.
  */
-static struct xhci_ring *xhci_ring_alloc(struct xhci_hcd *xhci,
+struct xhci_ring *xhci_ring_alloc(struct xhci_hcd *xhci,
 		unsigned int num_segs, unsigned int cycle_state,
 		enum xhci_ring_type type, unsigned int max_packet, gfp_t flags)
 {
@@ -467,7 +467,7 @@ int xhci_ring_expansion(struct xhci_hcd *xhci, struct xhci_ring *ring,
 
 #define CTX_SIZE(_hcc) (HCC_64BYTE_CONTEXT(_hcc) ? 64 : 32)
 
-static struct xhci_container_ctx *xhci_alloc_container_ctx(struct xhci_hcd *xhci,
+struct xhci_container_ctx *xhci_alloc_container_ctx(struct xhci_hcd *xhci,
 						    int type, gfp_t flags)
 {
 	struct xhci_container_ctx *ctx;
@@ -492,7 +492,7 @@ static struct xhci_container_ctx *xhci_alloc_container_ctx(struct xhci_hcd *xhci
 	return ctx;
 }
 
-static void xhci_free_container_ctx(struct xhci_hcd *xhci,
+void xhci_free_container_ctx(struct xhci_hcd *xhci,
 			     struct xhci_container_ctx *ctx)
 {
 	if (!ctx)
@@ -1762,21 +1762,61 @@ void xhci_free_command(struct xhci_hcd *xhci,
 	kfree(command);
 }
 
+int xhci_alloc_erst(struct xhci_hcd *xhci,
+		    struct xhci_ring *evt_ring,
+		    struct xhci_erst *erst,
+		    gfp_t flags)
+{
+	size_t size;
+	unsigned int val;
+	struct xhci_segment *seg;
+	struct xhci_erst_entry *entry;
+
+	size = sizeof(struct xhci_erst_entry) * evt_ring->num_segs;
+	erst->entries = dma_alloc_coherent(xhci_to_hcd(xhci)->self.sysdev,
+					   size,
+					   &erst->erst_dma_addr,
+					   flags);
+	if (!erst->entries)
+		return -ENOMEM;
+
+	memset(erst->entries, 0, size);
+	erst->num_entries = evt_ring->num_segs;
+
+	seg = evt_ring->first_seg;
+	for (val = 0; val < evt_ring->num_segs; val++) {
+		entry = &erst->entries[val];
+		entry->seg_addr = cpu_to_le64(seg->dma);
+		entry->seg_size = cpu_to_le32(TRBS_PER_SEGMENT);
+		entry->rsvd = 0;
+		seg = seg->next;
+	}
+
+	return 0;
+}
+
+void xhci_free_erst(struct xhci_hcd *xhci, struct xhci_erst *erst)
+{
+	size_t size;
+	struct device *dev = xhci_to_hcd(xhci)->self.sysdev;
+
+	size = sizeof(struct xhci_erst_entry) * (erst->num_entries);
+	if (erst->entries)
+		dma_free_coherent(dev, size,
+				erst->entries,
+				erst->erst_dma_addr);
+	erst->entries = NULL;
+}
+
 void xhci_mem_cleanup(struct xhci_hcd *xhci)
 {
 	struct device	*dev = xhci_to_hcd(xhci)->self.sysdev;
-	int size;
 	int i, j, num_ports;
 
 	cancel_delayed_work_sync(&xhci->cmd_timer);
 
-	/* Free the Event Ring Segment Table and the actual Event Ring */
-	size = sizeof(struct xhci_erst_entry)*(xhci->erst.num_entries);
-	if (xhci->erst.entries)
-		dma_free_coherent(dev, size,
-				xhci->erst.entries, xhci->erst.erst_dma_addr);
-	xhci->erst.entries = NULL;
-	xhci_dbg_trace(xhci, trace_xhci_dbg_init, "Freed ERST");
+	xhci_free_erst(xhci, &xhci->erst);
+
 	if (xhci->event_ring)
 		xhci_ring_free(xhci, xhci->event_ring);
 	xhci->event_ring = NULL;
@@ -2313,9 +2353,8 @@ int xhci_mem_init(struct xhci_hcd *xhci, gfp_t flags)
 	struct device	*dev = xhci_to_hcd(xhci)->self.sysdev;
 	unsigned int	val, val2;
 	u64		val_64;
-	struct xhci_segment	*seg;
-	u32 page_size, temp;
-	int i;
+	u32		page_size, temp;
+	int		i, ret;
 
 	INIT_LIST_HEAD(&xhci->cmd_list);
 
@@ -2454,32 +2493,9 @@ int xhci_mem_init(struct xhci_hcd *xhci, gfp_t flags)
 	if (xhci_check_trb_in_td_math(xhci) < 0)
 		goto fail;
 
-	xhci->erst.entries = dma_alloc_coherent(dev,
-			sizeof(struct xhci_erst_entry) * ERST_NUM_SEGS, &dma,
-			flags);
-	if (!xhci->erst.entries)
+	ret = xhci_alloc_erst(xhci, xhci->event_ring, &xhci->erst, flags);
+	if (ret)
 		goto fail;
-	xhci_dbg_trace(xhci, trace_xhci_dbg_init,
-			"// Allocated event ring segment table at 0x%llx",
-			(unsigned long long)dma);
-
-	memset(xhci->erst.entries, 0, sizeof(struct xhci_erst_entry)*ERST_NUM_SEGS);
-	xhci->erst.num_entries = ERST_NUM_SEGS;
-	xhci->erst.erst_dma_addr = dma;
-	xhci_dbg_trace(xhci, trace_xhci_dbg_init,
-			"Set ERST to 0; private num segs = %i, virt addr = %p, dma addr = 0x%llx",
-			xhci->erst.num_entries,
-			xhci->erst.entries,
-			(unsigned long long)xhci->erst.erst_dma_addr);
-
-	/* set ring base address and size for each segment table entry */
-	for (val = 0, seg = xhci->event_ring->first_seg; val < ERST_NUM_SEGS; val++) {
-		struct xhci_erst_entry *entry = &xhci->erst.entries[val];
-		entry->seg_addr = cpu_to_le64(seg->dma);
-		entry->seg_size = cpu_to_le32(TRBS_PER_SEGMENT);
-		entry->rsvd = 0;
-		seg = seg->next;
-	}
 
 	/* set ERST count with the number of entries in the segment table */
 	val = readl(&xhci->ir_set->erst_size);
diff --git a/drivers/usb/host/xhci-ring.c b/drivers/usb/host/xhci-ring.c
index cc368ad..a55e377 100644
--- a/drivers/usb/host/xhci-ring.c
+++ b/drivers/usb/host/xhci-ring.c
@@ -165,7 +165,7 @@ static void next_trb(struct xhci_hcd *xhci,
  * See Cycle bit rules. SW is the consumer for the event ring only.
  * Don't make a ring full of link TRBs.  That would be dumb and this would loop.
  */
-static void inc_deq(struct xhci_hcd *xhci, struct xhci_ring *ring)
+void inc_deq(struct xhci_hcd *xhci, struct xhci_ring *ring)
 {
 	/* event ring doesn't have link trbs, check for last trb */
 	if (ring->type == TYPE_EVENT) {
@@ -2961,7 +2961,7 @@ static int prepare_transfer(struct xhci_hcd *xhci,
 	return 0;
 }
 
-static unsigned int count_trbs(u64 addr, u64 len)
+unsigned int count_trbs(u64 addr, u64 len)
 {
 	unsigned int num_trbs;
 
diff --git a/drivers/usb/host/xhci.h b/drivers/usb/host/xhci.h
index e3e9352..62bcdcd 100644
--- a/drivers/usb/host/xhci.h
+++ b/drivers/usb/host/xhci.h
@@ -1959,9 +1959,17 @@ void xhci_slot_copy(struct xhci_hcd *xhci,
 int xhci_endpoint_init(struct xhci_hcd *xhci, struct xhci_virt_device *virt_dev,
 		struct usb_device *udev, struct usb_host_endpoint *ep,
 		gfp_t mem_flags);
+struct xhci_ring *xhci_ring_alloc(struct xhci_hcd *xhci,
+		unsigned int num_segs, unsigned int cycle_state,
+		enum xhci_ring_type type, unsigned int max_packet, gfp_t flags);
 void xhci_ring_free(struct xhci_hcd *xhci, struct xhci_ring *ring);
 int xhci_ring_expansion(struct xhci_hcd *xhci, struct xhci_ring *ring,
-				unsigned int num_trbs, gfp_t flags);
+		unsigned int num_trbs, gfp_t flags);
+int xhci_alloc_erst(struct xhci_hcd *xhci,
+		struct xhci_ring *evt_ring,
+		struct xhci_erst *erst,
+		gfp_t flags);
+void xhci_free_erst(struct xhci_hcd *xhci, struct xhci_erst *erst);
 void xhci_free_endpoint_ring(struct xhci_hcd *xhci,
 		struct xhci_virt_device *virt_dev,
 		unsigned int ep_index);
@@ -1991,6 +1999,10 @@ struct xhci_command *xhci_alloc_command(struct xhci_hcd *xhci,
 void xhci_urb_free_priv(struct urb_priv *urb_priv);
 void xhci_free_command(struct xhci_hcd *xhci,
 		struct xhci_command *command);
+struct xhci_container_ctx *xhci_alloc_container_ctx(struct xhci_hcd *xhci,
+		int type, gfp_t flags);
+void xhci_free_container_ctx(struct xhci_hcd *xhci,
+		struct xhci_container_ctx *ctx);
 
 /* xHCI host controller glue */
 typedef void (*xhci_get_quirks_t)(struct device *, struct xhci_hcd *);
@@ -2065,6 +2077,8 @@ void xhci_handle_command_timeout(struct work_struct *work);
 void xhci_ring_ep_doorbell(struct xhci_hcd *xhci, unsigned int slot_id,
 		unsigned int ep_index, unsigned int stream_id);
 void xhci_cleanup_command_queue(struct xhci_hcd *xhci);
+void inc_deq(struct xhci_hcd *xhci, struct xhci_ring *ring);
+unsigned int count_trbs(u64 addr, u64 len);
 
 /* xHCI roothub code */
 void xhci_set_link_state(struct xhci_hcd *xhci, __le32 __iomem **port_array,
-- 
2.7.4

^ permalink raw reply related	[flat|nested] 21+ messages in thread

* [PATCH v3 2/3] usb: xhci: Add DbC support in xHCI driver
  2017-09-05  1:58 [PATCH v3 0/3] usb: xhci: Add debug capability support in xhci Lu Baolu
  2017-09-05  1:58 ` [PATCH v3 1/3] usb: xhci: Make some static functions global Lu Baolu
@ 2017-09-05  1:58 ` Lu Baolu
  2017-10-24 17:03   ` Mathias Nyman
                     ` (2 more replies)
  2017-09-05  1:59 ` [PATCH v3 3/3] usb: doc: Update document for USB3 debug port usage Lu Baolu
  2 siblings, 3 replies; 21+ messages in thread
From: Lu Baolu @ 2017-09-05  1:58 UTC (permalink / raw)
  To: Mathias Nyman, Greg Kroah-Hartman; +Cc: linux-usb, linux-kernel, Lu Baolu

xHCI compatible USB host controllers(i.e. super-speed USB3 controllers)
can be implemented with the Debug Capability(DbC). It presents a debug
device which is fully compliant with the USB framework and provides the
equivalent of a very high performance full-duplex serial link. The debug
capability operation model and registers interface are defined in 7.6.8
of the xHCI specification, revision 1.1.

The DbC debug device shares a root port with the xHCI host. By default,
the debug capability is disabled and the root port is assigned to xHCI.
When the DbC is enabled, the root port will be assigned to the DbC debug
device, and the xHCI sees nothing on this port. This implementation uses
a sysfs node named <dbc> under the xHCI device to manage the enabling
and disabling of the debug capability.

When the debug capability is enabled, it will present a debug device
through the debug port. This debug device is fully compliant with the
USB3 framework, and it can be enumerated by a debug host on the other
end of the USB link. As soon as the debug device is configured, a TTY
serial device named /dev/ttyDBC0 will be created.

One use of this link is running a login service on the debug target.
Hence it can be remote accessed by a debug host. Another use case can
probably be found in servers. It provides a peer-to-peer USB link
between two host-only machines. This provides a reasonable out-of-band
communication method between two servers.

Signed-off-by: Lu Baolu <baolu.lu@linux.intel.com>
---
 .../ABI/testing/sysfs-bus-pci-drivers-xhci_hcd     |   25 +
 drivers/usb/host/Kconfig                           |    9 +
 drivers/usb/host/Makefile                          |    5 +
 drivers/usb/host/xhci-dbgcap.c                     | 1016 ++++++++++++++++++++
 drivers/usb/host/xhci-dbgcap.h                     |  247 +++++
 drivers/usb/host/xhci-dbgtty.c                     |  586 +++++++++++
 drivers/usb/host/xhci-trace.h                      |   60 ++
 drivers/usb/host/xhci.c                            |   10 +
 drivers/usb/host/xhci.h                            |    1 +
 9 files changed, 1959 insertions(+)
 create mode 100644 Documentation/ABI/testing/sysfs-bus-pci-drivers-xhci_hcd
 create mode 100644 drivers/usb/host/xhci-dbgcap.c
 create mode 100644 drivers/usb/host/xhci-dbgcap.h
 create mode 100644 drivers/usb/host/xhci-dbgtty.c

diff --git a/Documentation/ABI/testing/sysfs-bus-pci-drivers-xhci_hcd b/Documentation/ABI/testing/sysfs-bus-pci-drivers-xhci_hcd
new file mode 100644
index 0000000..0088aba
--- /dev/null
+++ b/Documentation/ABI/testing/sysfs-bus-pci-drivers-xhci_hcd
@@ -0,0 +1,25 @@
+What:		/sys/bus/pci/drivers/xhci_hcd/.../dbc
+Date:		June 2017
+Contact:	Lu Baolu <baolu.lu@linux.intel.com>
+Description:
+		xHCI compatible USB host controllers (i.e. super-speed
+		USB3 controllers) are often implemented with the Debug
+		Capability (DbC). It can present a debug device which
+		is fully compliant with the USB framework and provides
+		the equivalent of a very high performance full-duplex
+		serial link for debug purpose.
+
+		The DbC debug device shares a root port with xHCI host.
+		When the DbC is enabled, the root port will be assigned
+		to the Debug Capability. Otherwise, it will be assigned
+		to xHCI.
+
+		Writing "enable" to this attribute will enable the DbC
+		functionality and the shared root port will be assigned
+		to the DbC device. Writing "disable" to this attribute
+		will disable the DbC functionality and the shared root
+		port will roll back to the xHCI.
+
+		Reading this attribute gives the state of the DbC. It
+		can be one of the following states: disabled, enabled,
+		initialized, connected, configured and stalled.
diff --git a/drivers/usb/host/Kconfig b/drivers/usb/host/Kconfig
index fa5692d..968a196 100644
--- a/drivers/usb/host/Kconfig
+++ b/drivers/usb/host/Kconfig
@@ -27,6 +27,15 @@ config USB_XHCI_HCD
 	  module will be called xhci-hcd.
 
 if USB_XHCI_HCD
+config USB_XHCI_DBGCAP
+	bool "xHCI support for debug capability"
+	depends on TTY
+	select USB_U_SERIAL
+	---help---
+	  Say 'Y' to enable the support for the xHCI debug capability. Make
+	  sure that your xHCI host supports the extended debug capability and
+	  you want a TTY serial device based on the xHCI debug capability
+	  before enabling this option. If unsure, say 'N'.
 
 config USB_XHCI_PCI
        tristate
diff --git a/drivers/usb/host/Makefile b/drivers/usb/host/Makefile
index cf2691f..175c80a 100644
--- a/drivers/usb/host/Makefile
+++ b/drivers/usb/host/Makefile
@@ -13,6 +13,11 @@ fhci-$(CONFIG_FHCI_DEBUG) += fhci-dbg.o
 xhci-hcd-y := xhci.o xhci-mem.o
 xhci-hcd-y += xhci-ring.o xhci-hub.o xhci-dbg.o
 xhci-hcd-y += xhci-trace.o
+
+ifneq ($(CONFIG_USB_XHCI_DBGCAP), )
+	xhci-hcd-y += xhci-dbgcap.o xhci-dbgtty.o
+endif
+
 ifneq ($(CONFIG_USB_XHCI_MTK), )
 	xhci-hcd-y += xhci-mtk-sch.o
 endif
diff --git a/drivers/usb/host/xhci-dbgcap.c b/drivers/usb/host/xhci-dbgcap.c
new file mode 100644
index 0000000..9816085
--- /dev/null
+++ b/drivers/usb/host/xhci-dbgcap.c
@@ -0,0 +1,1016 @@
+/**
+ * xhci-dbgcap.c - xHCI debug capability support
+ *
+ * Copyright (C) 2017 Intel Corporation
+ *
+ * Author: Lu Baolu <baolu.lu@linux.intel.com>
+ *
+ * This program is free software; you can redistribute it and/or modify
+ * it under the terms of the GNU General Public License version 2 as
+ * published by the Free Software Foundation.
+ */
+#include <linux/dma-mapping.h>
+#include <linux/slab.h>
+
+#include "xhci.h"
+#include "xhci-trace.h"
+#include "xhci-dbgcap.h"
+
+static inline void *
+dbc_dma_alloc_coherent(struct xhci_hcd *xhci, size_t size,
+		       dma_addr_t *dma_handle, gfp_t flags)
+{
+	void		*vaddr;
+
+	vaddr = dma_alloc_coherent(xhci_to_hcd(xhci)->self.sysdev,
+				   size, dma_handle, flags);
+	memset(vaddr, 0, size);
+	return vaddr;
+}
+
+static inline void
+dbc_dma_free_coherent(struct xhci_hcd *xhci, size_t size,
+		      void *cpu_addr, dma_addr_t dma_handle)
+{
+	if (cpu_addr)
+		dma_free_coherent(xhci_to_hcd(xhci)->self.sysdev,
+				  size, cpu_addr, dma_handle);
+}
+
+static inline void xhci_put_utf16(u16 *s, const char *c)
+{
+	int		i;
+	size_t		size;
+
+	size = strlen(c);
+	for (i = 0; i < size; i++)
+		s[i] = cpu_to_le16(c[i]);
+}
+
+static u32 xhci_dbc_populate_strings(struct dbc_strings *strings)
+{
+	struct usb_string_descriptor	*s_desc;
+	u32				string_length;
+
+	/* Serial string: */
+	s_desc = (struct usb_string_descriptor *)strings->serial;
+	xhci_put_utf16(s_desc->wData, DBC_STRING_SERIAL);
+
+	s_desc->bLength		= (strlen(DBC_STRING_SERIAL) + 1) * 2;
+	s_desc->bDescriptorType	= USB_DT_STRING;
+	string_length		= s_desc->bLength;
+	string_length		<<= 8;
+
+	/* Product string: */
+	s_desc = (struct usb_string_descriptor *)strings->product;
+	xhci_put_utf16(s_desc->wData, DBC_STRING_PRODUCT);
+
+	s_desc->bLength		= (strlen(DBC_STRING_PRODUCT) + 1) * 2;
+	s_desc->bDescriptorType	= USB_DT_STRING;
+	string_length		+= s_desc->bLength;
+	string_length		<<= 8;
+
+	/* Manufacture string: */
+	s_desc = (struct usb_string_descriptor *)strings->manufacturer;
+	xhci_put_utf16(s_desc->wData, DBC_STRING_MANUFACTURER);
+
+	s_desc->bLength		= (strlen(DBC_STRING_MANUFACTURER) + 1) * 2;
+	s_desc->bDescriptorType	= USB_DT_STRING;
+	string_length		+= s_desc->bLength;
+	string_length		<<= 8;
+
+	/* String0: */
+	strings->string0[0]	= 4;
+	strings->string0[1]	= USB_DT_STRING;
+	strings->string0[2]	= 0x09;
+	strings->string0[3]	= 0x04;
+	string_length		+= 4;
+
+	return string_length;
+}
+
+static void xhci_dbc_populate_contexts(struct xhci_hcd *xhci, u32 string_length)
+{
+	struct xhci_dbc		*dbc;
+	struct dbc_info_context	*info;
+	struct xhci_ep_ctx	*ep_ctx;
+	u32			dev_info;
+	dma_addr_t		deq, dma;
+	unsigned int		max_burst;
+
+	dbc = xhci->dbc;
+	if (!dbc)
+		return;
+
+	/* Populate info Context: */
+	info			= (struct dbc_info_context *)dbc->ctx->bytes;
+	dma			= dbc->string_dma;
+	info->string0		= cpu_to_le64(dma);
+	info->manufacturer	= cpu_to_le64(dma + DBC_MAX_STRING_LENGTH);
+	info->product		= cpu_to_le64(dma + DBC_MAX_STRING_LENGTH * 2);
+	info->serial		= cpu_to_le64(dma + DBC_MAX_STRING_LENGTH * 3);
+	info->length		= cpu_to_le32(string_length);
+
+	/* Populate bulk out endpoint context: */
+	ep_ctx			= dbc_bulkout_ctx(dbc);
+	max_burst		= DBC_CTRL_MAXBURST(readl(&dbc->regs->control));
+	deq			= dbc_bulkout_enq(dbc);
+	ep_ctx->ep_info		= 0;
+	ep_ctx->ep_info2	= dbc_epctx_info2(BULK_OUT_EP, 1024, max_burst);
+	ep_ctx->deq		= cpu_to_le64(deq | dbc->ring_out->cycle_state);
+
+	/* Populate bulk in endpoint context: */
+	ep_ctx			= dbc_bulkin_ctx(dbc);
+	deq			= dbc_bulkin_enq(dbc);
+	ep_ctx->ep_info		= 0;
+	ep_ctx->ep_info2	= dbc_epctx_info2(BULK_IN_EP, 1024, max_burst);
+	ep_ctx->deq		= cpu_to_le64(deq | dbc->ring_in->cycle_state);
+
+	/* Set DbC context and info registers: */
+	xhci_write_64(xhci, dbc->ctx->dma, &dbc->regs->dccp);
+
+	dev_info = cpu_to_le32((DBC_VENDOR_ID << 16) | DBC_PROTOCOL);
+	writel(dev_info, &dbc->regs->devinfo1);
+
+	dev_info = cpu_to_le32((DBC_DEVICE_REV << 16) | DBC_PRODUCT_ID);
+	writel(dev_info, &dbc->regs->devinfo2);
+}
+
+static void xhci_dbc_giveback(struct dbc_request *req, int status)
+	__releases(&dbc->lock)
+	__acquires(&dbc->lock)
+{
+	struct dbc_ep		*dep = req->dep;
+	struct xhci_dbc		*dbc = dep->dbc;
+	struct xhci_hcd		*xhci = dbc->xhci;
+	struct device		*dev = xhci_to_hcd(dbc->xhci)->self.sysdev;
+
+	trace_xhci_dbc_giveback_request(req);
+
+	list_del_init(&req->list_pending);
+	req->trb_dma = 0;
+	req->trb = NULL;
+
+	if (req->status == -EINPROGRESS)
+		req->status = status;
+
+	dma_unmap_single(dev,
+			 req->dma,
+			 req->length,
+			 dbc_ep_dma_direction(dep));
+
+	/* Give back the transfer request: */
+	spin_unlock(&dbc->lock);
+	req->complete(xhci, req);
+	spin_lock(&dbc->lock);
+}
+
+static void xhci_dbc_flush_single_request(struct dbc_request *req)
+{
+	union xhci_trb	*trb = req->trb;
+
+	trb->generic.field[0]	= 0;
+	trb->generic.field[1]	= 0;
+	trb->generic.field[2]	= 0;
+	trb->generic.field[3]	&= cpu_to_le32(TRB_CYCLE);
+	trb->generic.field[3]	|= cpu_to_le32(TRB_TYPE(TRB_TR_NOOP));
+
+	xhci_dbc_giveback(req, -ESHUTDOWN);
+}
+
+static void xhci_dbc_flush_endpoint_requests(struct dbc_ep *dep)
+{
+	struct dbc_request	*req, *tmp;
+
+	list_for_each_entry_safe(req, tmp, &dep->list_pending, list_pending)
+		xhci_dbc_flush_single_request(req);
+}
+
+static void xhci_dbc_flush_reqests(struct xhci_dbc *dbc)
+{
+	int			i;
+	struct dbc_ep		*dep;
+
+	for (i = BULK_OUT; i <= BULK_IN; i++) {
+		dep = &dbc->eps[i];
+		xhci_dbc_flush_endpoint_requests(dep);
+	}
+}
+
+static struct dbc_request *
+dbc_ep_alloc_request(struct dbc_ep *dep, gfp_t gfp_flags)
+{
+	struct dbc_request	*req;
+
+	req = kzalloc(sizeof(*req), gfp_flags);
+	if (!req)
+		return NULL;
+
+	req->dep = dep;
+	INIT_LIST_HEAD(&req->list_pending);
+	INIT_LIST_HEAD(&req->list_pool);
+
+	trace_xhci_dbc_alloc_request(req);
+
+	return req;
+}
+
+static void
+dbc_ep_free_request(struct dbc_ep *dep, struct dbc_request *req)
+{
+	trace_xhci_dbc_free_request(req);
+
+	kfree(req);
+}
+
+static void
+xhci_dbc_queue_trb(struct xhci_ring *ring, u32 field1,
+		   u32 field2, u32 field3, u32 field4)
+{
+	union xhci_trb		*trb, *next;
+
+	trb = ring->enqueue;
+	trb->generic.field[0]	= cpu_to_le32(field1);
+	trb->generic.field[1]	= cpu_to_le32(field2);
+	trb->generic.field[2]	= cpu_to_le32(field3);
+	trb->generic.field[3]	= cpu_to_le32(field4);
+
+	trace_xhci_dbc_gadget_ep_queue(ring, &trb->generic);
+
+	ring->num_trbs_free--;
+	next = ++(ring->enqueue);
+	if (TRB_TYPE_LINK_LE32(next->link.control)) {
+		next->link.control ^= cpu_to_le32(TRB_CYCLE);
+		ring->enqueue = ring->enq_seg->trbs;
+		ring->cycle_state ^= 1;
+	}
+}
+
+static int xhci_dbc_queue_bulk_tx(struct dbc_ep *dep,
+				  struct dbc_request *req)
+{
+	u64			addr;
+	union xhci_trb		*trb;
+	unsigned int		num_trbs;
+	struct xhci_dbc		*dbc = dep->dbc;
+	struct xhci_ring	*ring = dep->ring;
+	u32			length, control, cycle;
+
+	num_trbs = count_trbs(req->dma, req->length);
+	WARN_ON(num_trbs != 1);
+	if (ring->num_trbs_free < num_trbs)
+		return -EBUSY;
+
+	addr	= req->dma;
+	trb	= ring->enqueue;
+	cycle	= ring->cycle_state;
+	length	= TRB_LEN(req->length);
+	control	= TRB_TYPE(TRB_NORMAL) | TRB_IOC;
+
+	if (cycle)
+		control &= cpu_to_le32(~TRB_CYCLE);
+	else
+		control |= cpu_to_le32(TRB_CYCLE);
+
+	req->trb = ring->enqueue;
+	req->trb_dma = xhci_trb_virt_to_dma(ring->enq_seg, ring->enqueue);
+	xhci_dbc_queue_trb(ring,
+			   lower_32_bits(addr),
+			   upper_32_bits(addr),
+			   length, control);
+
+	/*
+	 * Add a barrier between writes of trb fields and flipping
+	 * the cycle bit:
+	 */
+	wmb();
+
+	if (cycle)
+		trb->generic.field[3] |= cpu_to_le32(TRB_CYCLE);
+	else
+		trb->generic.field[3] &= cpu_to_le32(~TRB_CYCLE);
+
+	writel(DBC_DOOR_BELL_TARGET(dep->direction), &dbc->regs->doorbell);
+
+	return 0;
+}
+
+static int
+dbc_ep_do_queue(struct dbc_ep *dep, struct dbc_request *req)
+{
+	int			ret;
+	struct device		*dev;
+	struct xhci_dbc		*dbc = dep->dbc;
+	struct xhci_hcd		*xhci = dbc->xhci;
+
+	dev = xhci_to_hcd(xhci)->self.sysdev;
+
+	if (!req->length || !req->buf)
+		return -EINVAL;
+
+	req->actual		= 0;
+	req->status		= -EINPROGRESS;
+
+	req->dma = dma_map_single(dev,
+				  req->buf,
+				  req->length,
+				  dbc_ep_dma_direction(dep));
+	if (dma_mapping_error(dev, req->dma)) {
+		xhci_err(xhci, "failed to map buffer\n");
+		return -EFAULT;
+	}
+
+	ret = xhci_dbc_queue_bulk_tx(dep, req);
+	if (ret) {
+		xhci_err(xhci, "failed to queue trbs\n");
+		dma_unmap_single(dev,
+				 req->dma,
+				 req->length,
+				 dbc_ep_dma_direction(dep));
+		return -EFAULT;
+	}
+
+	list_add_tail(&req->list_pending, &dep->list_pending);
+
+	return 0;
+}
+
+static int dbc_ep_queue(struct dbc_ep *dep,
+			struct dbc_request *req,
+			gfp_t gfp_flags)
+{
+	struct xhci_dbc		*dbc = dep->dbc;
+	int			ret = -ESHUTDOWN;
+
+	spin_lock(&dbc->lock);
+	if (dbc->state == DS_CONFIGURED)
+		ret = dbc_ep_do_queue(dep, req);
+	spin_unlock(&dbc->lock);
+
+	trace_xhci_dbc_queue_request(req);
+
+	return ret;
+}
+
+static inline void xhci_dbc_do_eps_init(struct xhci_hcd *xhci, bool direction)
+{
+	struct dbc_ep		*dep;
+	struct xhci_dbc		*dbc = xhci->dbc;
+
+	dep			= &dbc->eps[direction];
+	dep->dbc		= dbc;
+	dep->direction		= direction;
+	dep->ring		= direction ? dbc->ring_in : dbc->ring_out;
+	dep->alloc_request	= dbc_ep_alloc_request;
+	dep->free_request	= dbc_ep_free_request;
+	dep->queue		= dbc_ep_queue;
+
+	INIT_LIST_HEAD(&dep->list_pending);
+}
+
+static void xhci_dbc_eps_init(struct xhci_hcd *xhci)
+{
+	xhci_dbc_do_eps_init(xhci, BULK_OUT);
+	xhci_dbc_do_eps_init(xhci, BULK_IN);
+}
+
+static void xhci_dbc_eps_exit(struct xhci_hcd *xhci)
+{
+	struct xhci_dbc		*dbc = xhci->dbc;
+
+	memset(dbc->eps, 0, ARRAY_SIZE(dbc->eps));
+}
+
+static int xhci_dbc_mem_init(struct xhci_hcd *xhci, gfp_t flags)
+{
+	int			ret;
+	dma_addr_t		deq;
+	u32			string_length;
+	struct xhci_dbc		*dbc = xhci->dbc;
+
+	/* Allocate various rings for events and transfers: */
+	dbc->ring_evt = xhci_ring_alloc(xhci, 1, 1, TYPE_EVENT, 0, flags);
+	if (!dbc->ring_evt)
+		goto evt_fail;
+
+	dbc->ring_in = xhci_ring_alloc(xhci, 1, 1, TYPE_BULK, 0, flags);
+	if (!dbc->ring_in)
+		goto in_fail;
+
+	dbc->ring_out = xhci_ring_alloc(xhci, 1, 1, TYPE_BULK, 0, flags);
+	if (!dbc->ring_out)
+		goto out_fail;
+
+	/* Allocate and populate ERST: */
+	ret = xhci_alloc_erst(xhci, dbc->ring_evt, &dbc->erst, flags);
+	if (ret)
+		goto erst_fail;
+
+	/* Allocate context data structure: */
+	dbc->ctx = xhci_alloc_container_ctx(xhci, XHCI_CTX_TYPE_DEVICE, flags);
+	if (!dbc->ctx)
+		goto ctx_fail;
+
+	/* Allocate the string table: */
+	dbc->string_size = sizeof(struct dbc_strings);
+	dbc->string = dbc_dma_alloc_coherent(xhci,
+					     dbc->string_size,
+					     &dbc->string_dma,
+					     flags);
+	if (!dbc->string)
+		goto string_fail;
+
+	/* Setup ERST register: */
+	writel(dbc->erst.erst_size, &dbc->regs->ersts);
+	xhci_write_64(xhci, dbc->erst.erst_dma_addr, &dbc->regs->erstba);
+	deq = xhci_trb_virt_to_dma(dbc->ring_evt->deq_seg,
+				   dbc->ring_evt->dequeue);
+	xhci_write_64(xhci, deq, &dbc->regs->erdp);
+
+	/* Setup strings and contexts: */
+	string_length = xhci_dbc_populate_strings(dbc->string);
+	xhci_dbc_populate_contexts(xhci, string_length);
+
+	mmiowb();
+
+	xhci_dbc_eps_init(xhci);
+	dbc->state = DS_INITIALIZED;
+
+	return 0;
+
+string_fail:
+	xhci_free_container_ctx(xhci, dbc->ctx);
+	dbc->ctx = NULL;
+ctx_fail:
+	xhci_free_erst(xhci, &dbc->erst);
+erst_fail:
+	xhci_ring_free(xhci, dbc->ring_out);
+	dbc->ring_out = NULL;
+out_fail:
+	xhci_ring_free(xhci, dbc->ring_in);
+	dbc->ring_in = NULL;
+in_fail:
+	xhci_ring_free(xhci, dbc->ring_evt);
+	dbc->ring_evt = NULL;
+evt_fail:
+	return -ENOMEM;
+}
+
+static void xhci_dbc_mem_cleanup(struct xhci_hcd *xhci)
+{
+	struct xhci_dbc		*dbc = xhci->dbc;
+
+	if (!dbc)
+		return;
+
+	xhci_dbc_eps_exit(xhci);
+
+	if (dbc->string) {
+		dbc_dma_free_coherent(xhci,
+				      dbc->string_size,
+				      dbc->string, dbc->string_dma);
+		dbc->string = NULL;
+	}
+
+	xhci_free_container_ctx(xhci, dbc->ctx);
+	dbc->ctx = NULL;
+
+	xhci_free_erst(xhci, &dbc->erst);
+	xhci_ring_free(xhci, dbc->ring_out);
+	xhci_ring_free(xhci, dbc->ring_in);
+	xhci_ring_free(xhci, dbc->ring_evt);
+	dbc->ring_in = NULL;
+	dbc->ring_out = NULL;
+	dbc->ring_evt = NULL;
+}
+
+static void xhci_dbc_reset_debug_port(struct xhci_hcd *xhci)
+{
+	u32			val;
+	unsigned long		flags;
+	int			port_index;
+	__le32 __iomem		**port_array;
+
+	spin_lock_irqsave(&xhci->lock, flags);
+
+	port_index = xhci->num_usb3_ports;
+	port_array = xhci->usb3_ports;
+	if (port_index <= 0) {
+		spin_unlock_irqrestore(&xhci->lock, flags);
+		return;
+	}
+
+	while (port_index--) {
+		val = readl(port_array[port_index]);
+		if (!(val & PORT_CONNECT))
+			writel(val | PORT_RESET, port_array[port_index]);
+	}
+
+	spin_unlock_irqrestore(&xhci->lock, flags);
+}
+
+static int xhci_do_dbc_start(struct xhci_hcd *xhci)
+{
+	int			ret;
+	u32			ctrl;
+	struct xhci_dbc		*dbc = xhci->dbc;
+
+	if (dbc->state != DS_DISABLED)
+		return -EINVAL;
+
+	writel(0, &dbc->regs->control);
+	ret = xhci_handshake(&dbc->regs->control,
+			     DBC_CTRL_DBC_ENABLE,
+			     0, 1000);
+	if (ret)
+		return ret;
+
+	ret = xhci_dbc_mem_init(xhci, GFP_ATOMIC);
+	if (ret)
+		return ret;
+
+	ctrl = readl(&dbc->regs->control);
+	writel(ctrl | DBC_CTRL_DBC_ENABLE | DBC_CTRL_PORT_ENABLE,
+	       &dbc->regs->control);
+	ret = xhci_handshake(&dbc->regs->control,
+			     DBC_CTRL_DBC_ENABLE,
+			     DBC_CTRL_DBC_ENABLE, 1000);
+	if (ret)
+		return ret;
+
+	xhci_dbc_reset_debug_port(xhci);
+	dbc->state = DS_ENABLED;
+
+	return 0;
+}
+
+static void xhci_do_dbc_stop(struct xhci_hcd *xhci)
+{
+	struct xhci_dbc		*dbc = xhci->dbc;
+
+	if (dbc->state == DS_DISABLED)
+		return;
+
+	writel(0, &dbc->regs->control);
+	xhci_dbc_mem_cleanup(xhci);
+	dbc->state = DS_DISABLED;
+}
+
+static int xhci_dbc_start(struct xhci_hcd *xhci)
+{
+	int			ret;
+	struct xhci_dbc		*dbc = xhci->dbc;
+
+	WARN_ON(!dbc);
+
+	spin_lock(&dbc->lock);
+	ret = xhci_do_dbc_start(xhci);
+	spin_unlock(&dbc->lock);
+
+	if (!ret) {
+		pm_runtime_get_sync(xhci_to_hcd(xhci)->self.controller);
+		ret = mod_delayed_work(system_wq, &dbc->event_work, 1);
+	}
+
+	return ret;
+}
+
+static void xhci_dbc_stop(struct xhci_hcd *xhci)
+{
+	struct xhci_dbc		*dbc = xhci->dbc;
+	struct dbc_port		*port = &dbc->port;
+
+	WARN_ON(!dbc);
+
+	cancel_delayed_work_sync(&dbc->event_work);
+
+	if (port->registered)
+		xhci_dbc_tty_unregister_device(xhci);
+
+	spin_lock(&dbc->lock);
+	xhci_do_dbc_stop(xhci);
+	spin_unlock(&dbc->lock);
+
+	pm_runtime_put_sync(xhci_to_hcd(xhci)->self.controller);
+}
+
+static void
+dbc_handle_port_status(struct xhci_hcd *xhci, union xhci_trb *event)
+{
+	u32			portsc;
+	struct xhci_dbc		*dbc = xhci->dbc;
+
+	portsc = readl(&dbc->regs->portsc);
+	if (portsc & DBC_PORTSC_CONN_CHANGE)
+		xhci_info(xhci, "DbC port connect change\n");
+
+	if (portsc & DBC_PORTSC_RESET_CHANGE)
+		xhci_info(xhci, "DbC port reset change\n");
+
+	if (portsc & DBC_PORTSC_LINK_CHANGE)
+		xhci_info(xhci, "DbC port link status change\n");
+
+	if (portsc & DBC_PORTSC_CONFIG_CHANGE)
+		xhci_info(xhci, "DbC config error change\n");
+
+	/* Port reset change bit will be cleared in other place: */
+	writel(portsc & ~DBC_PORTSC_RESET_CHANGE, &dbc->regs->portsc);
+}
+
+static void dbc_handle_tx_event(struct xhci_hcd *xhci, union xhci_trb *event)
+{
+	struct dbc_ep		*dep;
+	struct xhci_ring	*ring;
+	int			ep_id;
+	int			status;
+	u32			comp_code;
+	size_t			remain_length;
+	struct dbc_request	*req = NULL, *r;
+
+	comp_code	= GET_COMP_CODE(le32_to_cpu(event->generic.field[2]));
+	remain_length	= EVENT_TRB_LEN(le32_to_cpu(event->generic.field[2]));
+	ep_id		= TRB_TO_EP_ID(le32_to_cpu(event->generic.field[3]));
+	dep		= (ep_id == EPID_OUT) ?
+				get_out_ep(xhci) : get_in_ep(xhci);
+	ring		= dep->ring;
+
+	switch (comp_code) {
+	case COMP_SUCCESS:
+		remain_length = 0;
+	/* FALLTHROUGH */
+	case COMP_SHORT_PACKET:
+		status = 0;
+		break;
+	case COMP_TRB_ERROR:
+	case COMP_BABBLE_DETECTED_ERROR:
+	case COMP_USB_TRANSACTION_ERROR:
+	case COMP_STALL_ERROR:
+		xhci_warn(xhci, "tx error %d detected\n", comp_code);
+		status = -comp_code;
+		break;
+	default:
+		xhci_err(xhci, "unknown tx error %d\n", comp_code);
+		status = -comp_code;
+		break;
+	}
+
+	/* Match the pending request: */
+	list_for_each_entry(r, &dep->list_pending, list_pending) {
+		if (r->trb_dma == event->trans_event.buffer) {
+			req = r;
+			break;
+		}
+	}
+
+	if (!req) {
+		xhci_warn(xhci, "no matched request\n");
+		return;
+	}
+
+	trace_xhci_dbc_handle_transfer(ring, &req->trb->generic);
+
+	ring->num_trbs_free++;
+	req->actual = req->length - remain_length;
+	xhci_dbc_giveback(req, status);
+}
+
+static enum evtreturn xhci_dbc_do_handle_events(struct xhci_dbc *dbc)
+{
+	dma_addr_t		deq;
+	struct dbc_ep		*dep;
+	union xhci_trb		*evt;
+	u32			ctrl, portsc;
+	struct xhci_hcd		*xhci = dbc->xhci;
+	bool			update_erdp = false;
+
+	/* DbC state machine: */
+	switch (dbc->state) {
+	case DS_DISABLED:
+	case DS_INITIALIZED:
+
+		return EVT_ERR;
+	case DS_ENABLED:
+		portsc = readl(&dbc->regs->portsc);
+		if (portsc & DBC_PORTSC_CONN_STATUS) {
+			dbc->state = DS_CONNECTED;
+			xhci_info(xhci, "DbC connected\n");
+		}
+
+		return EVT_DONE;
+	case DS_CONNECTED:
+		ctrl = readl(&dbc->regs->control);
+		if (ctrl & DBC_CTRL_DBC_RUN) {
+			dbc->state = DS_CONFIGURED;
+			xhci_info(xhci, "DbC configured\n");
+			portsc = readl(&dbc->regs->portsc);
+			writel(portsc, &dbc->regs->portsc);
+			return EVT_GSER;
+		}
+
+		return EVT_DONE;
+	case DS_CONFIGURED:
+		/* Handle cable unplug event: */
+		portsc = readl(&dbc->regs->portsc);
+		if (!(portsc & DBC_PORTSC_PORT_ENABLED) &&
+		    !(portsc & DBC_PORTSC_CONN_STATUS)) {
+			xhci_info(xhci, "DbC cable unplugged\n");
+			dbc->state = DS_ENABLED;
+			xhci_dbc_flush_reqests(dbc);
+
+			return EVT_DISC;
+		}
+
+		/* Handle debug port reset event: */
+		if (portsc & DBC_PORTSC_RESET_CHANGE) {
+			xhci_info(xhci, "DbC port reset\n");
+			writel(portsc, &dbc->regs->portsc);
+			dbc->state = DS_ENABLED;
+			xhci_dbc_flush_reqests(dbc);
+
+			return EVT_DISC;
+		}
+
+		/* Handle endpoint stall event: */
+		ctrl = readl(&dbc->regs->control);
+		if ((ctrl & DBC_CTRL_HALT_IN_TR) ||
+		    (ctrl & DBC_CTRL_HALT_OUT_TR)) {
+			xhci_info(xhci, "DbC Endpoint stall\n");
+			dbc->state = DS_STALLED;
+
+			if (ctrl & DBC_CTRL_HALT_IN_TR) {
+				dep = get_in_ep(xhci);
+				xhci_dbc_flush_endpoint_requests(dep);
+			}
+
+			if (ctrl & DBC_CTRL_HALT_OUT_TR) {
+				dep = get_out_ep(xhci);
+				xhci_dbc_flush_endpoint_requests(dep);
+			}
+
+			return EVT_DONE;
+		}
+
+		/* Clear DbC run change bit: */
+		if (ctrl & DBC_CTRL_DBC_RUN_CHANGE) {
+			writel(ctrl, &dbc->regs->control);
+			ctrl = readl(&dbc->regs->control);
+		}
+
+		break;
+	case DS_STALLED:
+		ctrl = readl(&dbc->regs->control);
+		if (!(ctrl & DBC_CTRL_HALT_IN_TR) &&
+		    !(ctrl & DBC_CTRL_HALT_OUT_TR) &&
+		    (ctrl & DBC_CTRL_DBC_RUN)) {
+			dbc->state = DS_CONFIGURED;
+			break;
+		}
+
+		return EVT_DONE;
+	default:
+		xhci_err(xhci, "Unknown DbC state %d\n", dbc->state);
+		break;
+	}
+
+	/* Handle the events in the event ring: */
+	evt = dbc->ring_evt->dequeue;
+	while ((le32_to_cpu(evt->event_cmd.flags) & TRB_CYCLE) ==
+			dbc->ring_evt->cycle_state) {
+		/*
+		 * Add a barrier between reading the cycle flag and any
+		 * reads of the event's flags/data below:
+		 */
+		rmb();
+
+		trace_xhci_dbc_handle_event(dbc->ring_evt, &evt->generic);
+
+		switch (le32_to_cpu(evt->event_cmd.flags) & TRB_TYPE_BITMASK) {
+		case TRB_TYPE(TRB_PORT_STATUS):
+			dbc_handle_port_status(xhci, evt);
+			break;
+		case TRB_TYPE(TRB_TRANSFER):
+			dbc_handle_tx_event(xhci, evt);
+			break;
+		default:
+			break;
+		}
+
+		inc_deq(xhci, dbc->ring_evt);
+		evt = dbc->ring_evt->dequeue;
+		update_erdp = true;
+	}
+
+	/* Update event ring dequeue pointer: */
+	if (update_erdp) {
+		deq = xhci_trb_virt_to_dma(dbc->ring_evt->deq_seg,
+					   dbc->ring_evt->dequeue);
+		xhci_write_64(xhci, deq, &dbc->regs->erdp);
+	}
+
+	return EVT_DONE;
+}
+
+static void xhci_dbc_handle_events(struct work_struct *work)
+{
+	int			ret;
+	enum evtreturn		evtr;
+	struct xhci_dbc		*dbc;
+	struct xhci_hcd		*xhci;
+
+	dbc = container_of(to_delayed_work(work), struct xhci_dbc, event_work);
+	xhci = dbc->xhci;
+
+	spin_lock(&dbc->lock);
+	evtr = xhci_dbc_do_handle_events(dbc);
+	spin_unlock(&dbc->lock);
+
+	switch (evtr) {
+	case EVT_GSER:
+		ret = xhci_dbc_tty_register_device(xhci);
+		if (ret) {
+			xhci_err(xhci, "failed to alloc tty device\n");
+			break;
+		}
+
+		xhci_info(xhci, "DbC now attached to /dev/ttyDBC0\n");
+		mod_delayed_work(system_wq, &dbc->event_work, 1);
+		break;
+	case EVT_DISC:
+		xhci_dbc_tty_unregister_device(xhci);
+		mod_delayed_work(system_wq, &dbc->event_work, 1);
+		break;
+	case EVT_DONE:
+		mod_delayed_work(system_wq, &dbc->event_work, 1);
+		break;
+	default:
+		xhci_info(xhci, "stop handling dbc events\n");
+	}
+}
+
+static void xhci_dbc_exit(struct xhci_hcd *xhci)
+{
+	unsigned long		flags;
+
+	spin_lock_irqsave(&xhci->lock, flags);
+	kfree(xhci->dbc);
+	xhci->dbc = NULL;
+	spin_unlock_irqrestore(&xhci->lock, flags);
+}
+
+static int xhci_dbc_init(struct xhci_hcd *xhci)
+{
+	u32			reg;
+	struct xhci_dbc		*dbc;
+	unsigned long		flags;
+	void __iomem		*base;
+	int			dbc_cap_offs;
+
+	base = &xhci->cap_regs->hc_capbase;
+	dbc_cap_offs = xhci_find_next_ext_cap(base, 0, XHCI_EXT_CAPS_DEBUG);
+	if (!dbc_cap_offs)
+		return -ENODEV;
+
+	dbc = kzalloc(sizeof(*dbc), GFP_KERNEL);
+	if (!dbc)
+		return -ENOMEM;
+
+	dbc->regs = base + dbc_cap_offs;
+
+	/* We will avoid using DbC in xhci driver if it's in use. */
+	reg = readl(&dbc->regs->control);
+	if (reg & DBC_CTRL_DBC_ENABLE) {
+		kfree(dbc);
+		return -EBUSY;
+	}
+
+	spin_lock_irqsave(&xhci->lock, flags);
+	if (xhci->dbc) {
+		spin_unlock_irqrestore(&xhci->lock, flags);
+		kfree(dbc);
+		return -EBUSY;
+	}
+	xhci->dbc = dbc;
+	spin_unlock_irqrestore(&xhci->lock, flags);
+
+	dbc->xhci = xhci;
+	INIT_DELAYED_WORK(&dbc->event_work, xhci_dbc_handle_events);
+	spin_lock_init(&dbc->lock);
+
+	return 0;
+}
+
+static ssize_t dbc_show(struct device *dev,
+			struct device_attribute *attr,
+			char *buf)
+{
+	const char		*p;
+	struct xhci_dbc		*dbc;
+	struct xhci_hcd		*xhci;
+
+	xhci = hcd_to_xhci(dev_get_drvdata(dev));
+	dbc = xhci->dbc;
+
+	switch (dbc->state) {
+	case DS_DISABLED:
+		p = "disabled";
+		break;
+	case DS_INITIALIZED:
+		p = "initialized";
+		break;
+	case DS_ENABLED:
+		p = "enabled";
+		break;
+	case DS_CONNECTED:
+		p = "connected";
+		break;
+	case DS_CONFIGURED:
+		p = "configured";
+		break;
+	case DS_STALLED:
+		p = "stalled";
+		break;
+	default:
+		p = "unknown";
+	}
+
+	return sprintf(buf, "%s\n", p);
+}
+
+static ssize_t dbc_store(struct device *dev,
+			 struct device_attribute *attr,
+			 const char *buf, size_t count)
+{
+	struct xhci_dbc		*dbc;
+	struct xhci_hcd		*xhci;
+
+	xhci = hcd_to_xhci(dev_get_drvdata(dev));
+	dbc = xhci->dbc;
+
+	if (!strncmp(buf, "enable", 6))
+		xhci_dbc_start(xhci);
+	else if (!strncmp(buf, "disable", 7))
+		xhci_dbc_stop(xhci);
+	else
+		return -EINVAL;
+
+	return count;
+}
+
+static DEVICE_ATTR(dbc, 0644, dbc_show, dbc_store);
+
+int dbc_create_sysfs_file(struct xhci_hcd *xhci)
+{
+	int			ret;
+	struct device		*dev = xhci_to_hcd(xhci)->self.controller;
+
+	ret = xhci_dbc_init(xhci);
+	if (ret)
+		return ret;
+
+	xhci_dbc_tty_register_driver(xhci);
+
+	return device_create_file(dev, &dev_attr_dbc);
+}
+
+void dbc_remove_sysfs_file(struct xhci_hcd *xhci)
+{
+	struct device		*dev = xhci_to_hcd(xhci)->self.controller;
+
+	device_remove_file(dev, &dev_attr_dbc);
+	xhci_dbc_tty_unregister_driver();
+	xhci_dbc_stop(xhci);
+	xhci_dbc_exit(xhci);
+}
+
+#ifdef CONFIG_PM
+int xhci_dbc_suspend(struct xhci_hcd *xhci)
+{
+	struct xhci_dbc		*dbc = xhci->dbc;
+
+	if (!dbc)
+		return 0;
+
+	if (dbc->state == DS_CONFIGURED)
+		dbc->resume_required = 1;
+
+	xhci_dbc_stop(xhci);
+
+	return 0;
+}
+
+int xhci_dbc_resume(struct xhci_hcd *xhci)
+{
+	int			ret = 0;
+	struct xhci_dbc		*dbc = xhci->dbc;
+
+	if (!dbc)
+		return 0;
+
+	if (dbc->resume_required) {
+		dbc->resume_required = 0;
+		xhci_dbc_start(xhci);
+	}
+
+	return ret;
+}
+#endif /* CONFIG_PM */
diff --git a/drivers/usb/host/xhci-dbgcap.h b/drivers/usb/host/xhci-dbgcap.h
new file mode 100644
index 0000000..adf5fab
--- /dev/null
+++ b/drivers/usb/host/xhci-dbgcap.h
@@ -0,0 +1,247 @@
+
+/**
+ * xhci-dbgcap.h - xHCI debug capability support
+ *
+ * Copyright (C) 2017 Intel Corporation
+ *
+ * Author: Lu Baolu <baolu.lu@linux.intel.com>
+ *
+ * This program is free software; you can redistribute it and/or modify
+ * it under the terms of the GNU General Public License version 2 as
+ * published by the Free Software Foundation.
+ */
+#ifndef __LINUX_XHCI_DBGCAP_H
+#define __LINUX_XHCI_DBGCAP_H
+
+#include <linux/tty.h>
+
+struct dbc_regs {
+	__le32	capability;
+	__le32	doorbell;
+	__le32	ersts;		/* Event Ring Segment Table Size*/
+	__le32	__reserved_0;	/* 0c~0f reserved bits */
+	__le64	erstba;		/* Event Ring Segment Table Base Address */
+	__le64	erdp;		/* Event Ring Dequeue Pointer */
+	__le32	control;
+	__le32	status;
+	__le32	portsc;		/* Port status and control */
+	__le32	__reserved_1;	/* 2b~28 reserved bits */
+	__le64	dccp;		/* Debug Capability Context Pointer */
+	__le32	devinfo1;	/* Device Descriptor Info Register 1 */
+	__le32	devinfo2;	/* Device Descriptor Info Register 2 */
+};
+
+struct dbc_info_context {
+	__le64	string0;
+	__le64	manufacturer;
+	__le64	product;
+	__le64	serial;
+	__le32	length;
+	__le32	__reserved_0[7];
+};
+
+#define DBC_CTRL_DBC_RUN		BIT(0)
+#define DBC_CTRL_PORT_ENABLE		BIT(1)
+#define DBC_CTRL_HALT_OUT_TR		BIT(2)
+#define DBC_CTRL_HALT_IN_TR		BIT(3)
+#define DBC_CTRL_DBC_RUN_CHANGE		BIT(4)
+#define DBC_CTRL_DBC_ENABLE		BIT(31)
+#define DBC_CTRL_MAXBURST(p)		(((p) >> 16) & 0xff)
+#define DBC_DOOR_BELL_TARGET(p)		(((p) & 0xff) << 8)
+
+#define DBC_MAX_PACKET			1024
+#define DBC_MAX_STRING_LENGTH		64
+#define DBC_STRING_MANUFACTURER		"Linux"
+#define DBC_STRING_PRODUCT		"Remote GDB"
+#define DBC_STRING_SERIAL		"0001"
+#define	DBC_CONTEXT_SIZE		64
+
+/*
+ * Port status:
+ */
+#define DBC_PORTSC_CONN_STATUS		BIT(0)
+#define DBC_PORTSC_PORT_ENABLED		BIT(1)
+#define DBC_PORTSC_CONN_CHANGE		BIT(17)
+#define DBC_PORTSC_RESET_CHANGE		BIT(21)
+#define DBC_PORTSC_LINK_CHANGE		BIT(22)
+#define DBC_PORTSC_CONFIG_CHANGE	BIT(23)
+
+struct dbc_strings {
+	char	string0[DBC_MAX_STRING_LENGTH];
+	char	manufacturer[DBC_MAX_STRING_LENGTH];
+	char	product[DBC_MAX_STRING_LENGTH];
+	char	serial[DBC_MAX_STRING_LENGTH];
+};
+
+#define DBC_PROTOCOL			1	/* GNU Remote Debug Command */
+#define DBC_VENDOR_ID			0x1d6b	/* Linux Foundation 0x1d6b */
+#define DBC_PRODUCT_ID			0x0004	/* device 0004 */
+#define DBC_DEVICE_REV			0x0010	/* 0.10 */
+
+enum dbc_state {
+	DS_DISABLED = 0,
+	DS_INITIALIZED,
+	DS_ENABLED,
+	DS_CONNECTED,
+	DS_CONFIGURED,
+	DS_STALLED,
+};
+
+struct dbc_request {
+	void				*buf;
+	unsigned int			length;
+	dma_addr_t			dma;
+	void				(*complete)(struct xhci_hcd *xhci,
+						    struct dbc_request *req);
+	struct list_head		list_pool;
+	int				status;
+	unsigned int			actual;
+
+	struct dbc_ep			*dep;
+	struct list_head		list_pending;
+	dma_addr_t			trb_dma;
+	union xhci_trb			*trb;
+	unsigned			direction:1;
+};
+
+struct dbc_ep {
+	struct xhci_dbc			*dbc;
+	struct list_head		list_pending;
+	struct xhci_ring		*ring;
+	unsigned			direction:1;
+
+	struct dbc_request		*(*alloc_request)(struct dbc_ep *ep,
+							  gfp_t gfp_flags);
+	void				(*free_request)(struct dbc_ep *ep,
+							struct dbc_request *r);
+
+	int				(*queue)(struct dbc_ep *ep,
+						 struct dbc_request *req,
+						 gfp_t gfp_flags);
+};
+
+struct dbc_buf {
+	unsigned int			buf_size;
+	char				*buf_buf;
+	char				*buf_get;
+	char				*buf_put;
+};
+
+#define QUEUE_SIZE			16
+#define WRITE_BUF_SIZE			8192
+#define DBC_CLOSE_TIMEOUT		15
+
+/*
+ * Private structure for DbC hardware state:
+ */
+struct dbc_port {
+	struct tty_port			port;
+	spinlock_t			port_lock;	/* port access */
+
+	struct list_head		read_pool;
+	struct list_head		read_queue;
+	unsigned int			n_read;
+	struct tasklet_struct		push;
+
+	struct list_head		write_pool;
+	struct dbc_buf			port_write_buf;
+
+	bool				registered;
+	struct dbc_ep			*in;
+	struct dbc_ep			*out;
+};
+
+struct xhci_dbc {
+	spinlock_t			lock;		/* device access */
+	struct xhci_hcd			*xhci;
+	struct dbc_regs __iomem		*regs;
+	struct xhci_ring		*ring_evt;
+	struct xhci_ring		*ring_in;
+	struct xhci_ring		*ring_out;
+	struct xhci_erst		erst;
+	struct xhci_container_ctx	*ctx;
+
+	struct dbc_strings		*string;
+	dma_addr_t			string_dma;
+	size_t				string_size;
+
+	enum dbc_state			state;
+	struct delayed_work		event_work;
+	unsigned			resume_required:1;
+	struct dbc_ep			eps[2];
+
+	struct dbc_port			port;
+};
+
+#define dbc_bulkout_ctx(d)		\
+	((struct xhci_ep_ctx *)((d)->ctx->bytes + DBC_CONTEXT_SIZE))
+#define dbc_bulkin_ctx(d)		\
+	((struct xhci_ep_ctx *)((d)->ctx->bytes + DBC_CONTEXT_SIZE * 2))
+#define dbc_bulkout_enq(d)		\
+	xhci_trb_virt_to_dma((d)->ring_out->enq_seg, (d)->ring_out->enqueue)
+#define dbc_bulkin_enq(d)		\
+	xhci_trb_virt_to_dma((d)->ring_in->enq_seg, (d)->ring_in->enqueue)
+#define dbc_epctx_info2(t, p, b)	\
+	cpu_to_le32(EP_TYPE(t) | MAX_PACKET(p) | MAX_BURST(b))
+#define dbc_ep_dma_direction(d)		\
+	((d)->direction ? DMA_FROM_DEVICE : DMA_TO_DEVICE)
+
+#define GSPORT_INVAL			((unsigned char)(-1))
+#define BULK_OUT			0
+#define BULK_IN				1
+#define EPID_OUT			2
+#define EPID_IN				3
+
+enum evtreturn {
+	EVT_ERR	= -1,
+	EVT_DONE,
+	EVT_GSER,
+	EVT_DISC,
+};
+
+static inline struct dbc_ep *get_in_ep(struct xhci_hcd *xhci)
+{
+	struct xhci_dbc		*dbc = xhci->dbc;
+
+	return &dbc->eps[BULK_IN];
+}
+
+static inline struct dbc_ep *get_out_ep(struct xhci_hcd *xhci)
+{
+	struct xhci_dbc		*dbc = xhci->dbc;
+
+	return &dbc->eps[BULK_OUT];
+}
+
+#ifdef CONFIG_USB_XHCI_DBGCAP
+int dbc_create_sysfs_file(struct xhci_hcd *xhci);
+void dbc_remove_sysfs_file(struct xhci_hcd *xhci);
+int xhci_dbc_tty_register_driver(struct xhci_hcd *xhci);
+void xhci_dbc_tty_unregister_driver(void);
+int xhci_dbc_tty_register_device(struct xhci_hcd *xhci);
+void xhci_dbc_tty_unregister_device(struct xhci_hcd *xhci);
+#ifdef CONFIG_PM
+int xhci_dbc_suspend(struct xhci_hcd *xhci);
+int xhci_dbc_resume(struct xhci_hcd *xhci);
+#endif /* CONFIG_PM */
+#else
+static inline int dbc_create_sysfs_file(struct xhci_hcd *xhci)
+{
+	return 0;
+}
+
+static inline void dbc_remove_sysfs_file(struct xhci_hcd *xhci)
+{
+}
+
+static inline int xhci_dbc_suspend(struct xhci_hcd *xhci)
+{
+	return 0;
+}
+
+static inline int xhci_dbc_resume(struct xhci_hcd *xhci)
+{
+	return 0;
+}
+#endif /* CONFIG_USB_XHCI_DBGCAP */
+#endif /* __LINUX_XHCI_DBGCAP_H */
diff --git a/drivers/usb/host/xhci-dbgtty.c b/drivers/usb/host/xhci-dbgtty.c
new file mode 100644
index 0000000..fe5fef0
--- /dev/null
+++ b/drivers/usb/host/xhci-dbgtty.c
@@ -0,0 +1,586 @@
+/**
+ * xhci-dbgtty.c - tty glue for xHCI debug capability
+ *
+ * Copyright (C) 2017 Intel Corporation
+ *
+ * Author: Lu Baolu <baolu.lu@linux.intel.com>
+ *
+ * This program is free software; you can redistribute it and/or modify
+ * it under the terms of the GNU General Public License version 2 as
+ * published by the Free Software Foundation.
+ */
+
+#include <linux/slab.h>
+#include <linux/tty.h>
+#include <linux/tty_flip.h>
+
+#include "xhci.h"
+#include "xhci-dbgcap.h"
+
+static int dbc_buf_alloc(struct dbc_buf *db, unsigned int size)
+{
+	db->buf_buf = kzalloc(size, GFP_KERNEL);
+	if (!db->buf_buf)
+		return -ENOMEM;
+
+	db->buf_size = size;
+	db->buf_put = db->buf_buf;
+	db->buf_get = db->buf_buf;
+
+	return 0;
+}
+
+static void dbc_buf_free(struct dbc_buf *db)
+{
+	kfree(db->buf_buf);
+	db->buf_buf = NULL;
+}
+
+static unsigned int dbc_buf_data_avail(struct dbc_buf *db)
+{
+	return (db->buf_size + db->buf_put - db->buf_get) % db->buf_size;
+}
+
+static unsigned int dbc_buf_space_avail(struct dbc_buf *db)
+{
+	return (db->buf_size + db->buf_get - db->buf_put - 1) % db->buf_size;
+}
+
+static unsigned int
+dbc_buf_put(struct dbc_buf *db, const char *buf, unsigned int count)
+{
+	unsigned int	len;
+
+	len  = dbc_buf_space_avail(db);
+	if (count > len)
+		count = len;
+
+	if (count == 0)
+		return 0;
+
+	len = db->buf_buf + db->buf_size - db->buf_put;
+	if (count > len) {
+		memcpy(db->buf_put, buf, len);
+		memcpy(db->buf_buf, buf + len, count - len);
+		db->buf_put = db->buf_buf + count - len;
+	} else {
+		memcpy(db->buf_put, buf, count);
+		if (count < len)
+			db->buf_put += count;
+		else
+			db->buf_put = db->buf_buf;
+	}
+
+	return count;
+}
+
+static unsigned int
+dbc_buf_get(struct dbc_buf *db, char *buf, unsigned int count)
+{
+	unsigned int	len;
+
+	len = dbc_buf_data_avail(db);
+	if (count > len)
+		count = len;
+
+	if (count == 0)
+		return 0;
+
+	len = db->buf_buf + db->buf_size - db->buf_get;
+	if (count > len) {
+		memcpy(buf, db->buf_get, len);
+		memcpy(buf + len, db->buf_buf, count - len);
+		db->buf_get = db->buf_buf + count - len;
+	} else {
+		memcpy(buf, db->buf_get, count);
+		if (count < len)
+			db->buf_get += count;
+		else
+			db->buf_get = db->buf_buf;
+	}
+
+	return count;
+}
+
+static unsigned int
+dbc_send_packet(struct dbc_port *port, char *packet, unsigned int size)
+{
+	unsigned int		len;
+
+	len = dbc_buf_data_avail(&port->port_write_buf);
+	if (len < size)
+		size = len;
+	if (size != 0)
+		size = dbc_buf_get(&port->port_write_buf, packet, size);
+	return size;
+}
+
+static int dbc_start_tx(struct dbc_port *port)
+	__releases(&port->port_lock)
+	__acquires(&port->port_lock)
+{
+	int			len;
+	struct dbc_request	*req;
+	int			status = 0;
+	bool			do_tty_wake = false;
+	struct list_head	*pool = &port->write_pool;
+
+	while (!list_empty(pool)) {
+		req = list_entry(pool->next, struct dbc_request, list_pool);
+		len = dbc_send_packet(port, req->buf, DBC_MAX_PACKET);
+		if (len == 0)
+			break;
+		do_tty_wake = true;
+
+		req->length = len;
+		list_del(&req->list_pool);
+
+		spin_unlock(&port->port_lock);
+		status = port->out->queue(port->out, req, GFP_ATOMIC);
+		spin_lock(&port->port_lock);
+
+		if (status) {
+			list_add(&req->list_pool, pool);
+			break;
+		}
+	}
+
+	if (do_tty_wake && port->port.tty)
+		tty_wakeup(port->port.tty);
+
+	return status;
+}
+
+static void dbc_start_rx(struct dbc_port *port)
+	__releases(&port->port_lock)
+	__acquires(&port->port_lock)
+{
+	struct dbc_request	*req;
+	int			status;
+	struct list_head	*pool = &port->read_pool;
+
+	while (!list_empty(pool)) {
+		if (!port->port.tty)
+			break;
+
+		req = list_entry(pool->next, struct dbc_request, list_pool);
+		list_del(&req->list_pool);
+		req->length = DBC_MAX_PACKET;
+
+		spin_unlock(&port->port_lock);
+		status = port->in->queue(port->in, req, GFP_ATOMIC);
+		spin_lock(&port->port_lock);
+
+		if (status) {
+			list_add(&req->list_pool, pool);
+			break;
+		}
+	}
+}
+
+static void
+dbc_read_complete(struct xhci_hcd *xhci, struct dbc_request *req)
+{
+	struct xhci_dbc		*dbc = xhci->dbc;
+	struct dbc_port		*port = &dbc->port;
+
+	spin_lock(&port->port_lock);
+	list_add_tail(&req->list_pool, &port->read_queue);
+	tasklet_schedule(&port->push);
+	spin_unlock(&port->port_lock);
+}
+
+static void dbc_write_complete(struct xhci_hcd *xhci, struct dbc_request *req)
+{
+	struct xhci_dbc		*dbc = xhci->dbc;
+	struct dbc_port		*port = &dbc->port;
+
+	spin_lock(&port->port_lock);
+	list_add(&req->list_pool, &port->write_pool);
+	switch (req->status) {
+	case 0:
+		dbc_start_tx(port);
+		break;
+	case -ESHUTDOWN:
+		break;
+	default:
+		xhci_warn(xhci, "unexpected write complete status %d\n",
+			  req->status);
+		break;
+	}
+	spin_unlock(&port->port_lock);
+}
+
+void xhci_dbc_free_req(struct dbc_ep *dep, struct dbc_request *req)
+{
+	kfree(req->buf);
+	dep->free_request(dep, req);
+}
+
+static int
+xhci_dbc_alloc_requests(struct dbc_ep *dep, struct list_head *head,
+			void (*fn)(struct xhci_hcd *, struct dbc_request *))
+{
+	int			i;
+	struct dbc_request	*req;
+
+	for (i = 0; i < QUEUE_SIZE; i++) {
+		req = dep->alloc_request(dep, GFP_ATOMIC);
+		if (!req)
+			break;
+
+		req->length = DBC_MAX_PACKET;
+		req->buf = kmalloc(req->length, GFP_KERNEL);
+		if (!req->buf) {
+			xhci_dbc_free_req(dep, req);
+			break;
+		}
+
+		req->complete = fn;
+		list_add_tail(&req->list_pool, head);
+	}
+
+	return list_empty(head) ? -ENOMEM : 0;
+}
+
+static void
+xhci_dbc_free_requests(struct dbc_ep *dep, struct list_head *head)
+{
+	struct dbc_request	*req;
+
+	while (!list_empty(head)) {
+		req = list_entry(head->next, struct dbc_request, list_pool);
+		list_del(&req->list_pool);
+		xhci_dbc_free_req(dep, req);
+	}
+}
+
+static int dbc_tty_install(struct tty_driver *driver, struct tty_struct *tty)
+{
+	struct dbc_port		*port = driver->driver_state;
+
+	tty->driver_data = port;
+
+	return tty_port_install(&port->port, driver, tty);
+}
+
+static int dbc_tty_open(struct tty_struct *tty, struct file *file)
+{
+	struct dbc_port		*port = tty->driver_data;
+
+	return tty_port_open(&port->port, tty, file);
+}
+
+static void dbc_tty_close(struct tty_struct *tty, struct file *file)
+{
+	struct dbc_port		*port = tty->driver_data;
+
+	tty_port_close(&port->port, tty, file);
+}
+
+static int dbc_tty_write(struct tty_struct *tty,
+			 const unsigned char *buf,
+			 int count)
+{
+	struct dbc_port		*port = tty->driver_data;
+	unsigned long		flags;
+
+	spin_lock_irqsave(&port->port_lock, flags);
+	if (count)
+		count = dbc_buf_put(&port->port_write_buf, buf, count);
+	dbc_start_tx(port);
+	spin_unlock_irqrestore(&port->port_lock, flags);
+
+	return count;
+}
+
+static int dbc_tty_put_char(struct tty_struct *tty, unsigned char ch)
+{
+	struct dbc_port		*port = tty->driver_data;
+	unsigned long		flags;
+	int			status;
+
+	spin_lock_irqsave(&port->port_lock, flags);
+	status = dbc_buf_put(&port->port_write_buf, &ch, 1);
+	spin_unlock_irqrestore(&port->port_lock, flags);
+
+	return status;
+}
+
+static void dbc_tty_flush_chars(struct tty_struct *tty)
+{
+	struct dbc_port		*port = tty->driver_data;
+	unsigned long		flags;
+
+	spin_lock_irqsave(&port->port_lock, flags);
+	dbc_start_tx(port);
+	spin_unlock_irqrestore(&port->port_lock, flags);
+}
+
+static int dbc_tty_write_room(struct tty_struct *tty)
+{
+	struct dbc_port		*port = tty->driver_data;
+	unsigned long		flags;
+	int			room = 0;
+
+	spin_lock_irqsave(&port->port_lock, flags);
+	room = dbc_buf_space_avail(&port->port_write_buf);
+	spin_unlock_irqrestore(&port->port_lock, flags);
+
+	return room;
+}
+
+static int dbc_tty_chars_in_buffer(struct tty_struct *tty)
+{
+	struct dbc_port		*port = tty->driver_data;
+	unsigned long		flags;
+	int			chars = 0;
+
+	spin_lock_irqsave(&port->port_lock, flags);
+	chars = dbc_buf_data_avail(&port->port_write_buf);
+	spin_unlock_irqrestore(&port->port_lock, flags);
+
+	return chars;
+}
+
+static void dbc_tty_unthrottle(struct tty_struct *tty)
+{
+	struct dbc_port		*port = tty->driver_data;
+	unsigned long		flags;
+
+	spin_lock_irqsave(&port->port_lock, flags);
+	tasklet_schedule(&port->push);
+	spin_unlock_irqrestore(&port->port_lock, flags);
+}
+
+static const struct tty_operations dbc_tty_ops = {
+	.install		= dbc_tty_install,
+	.open			= dbc_tty_open,
+	.close			= dbc_tty_close,
+	.write			= dbc_tty_write,
+	.put_char		= dbc_tty_put_char,
+	.flush_chars		= dbc_tty_flush_chars,
+	.write_room		= dbc_tty_write_room,
+	.chars_in_buffer	= dbc_tty_chars_in_buffer,
+	.unthrottle		= dbc_tty_unthrottle,
+};
+
+static struct tty_driver *dbc_tty_driver;
+
+int xhci_dbc_tty_register_driver(struct xhci_hcd *xhci)
+{
+	int			status;
+	struct xhci_dbc		*dbc = xhci->dbc;
+
+	dbc_tty_driver = tty_alloc_driver(1, TTY_DRIVER_REAL_RAW |
+					  TTY_DRIVER_DYNAMIC_DEV);
+	if (IS_ERR(dbc_tty_driver)) {
+		status = PTR_ERR(dbc_tty_driver);
+		dbc_tty_driver = NULL;
+		return status;
+	}
+
+	dbc_tty_driver->driver_name = "dbc_serial";
+	dbc_tty_driver->name = "ttyDBC";
+
+	dbc_tty_driver->type = TTY_DRIVER_TYPE_SERIAL;
+	dbc_tty_driver->subtype = SERIAL_TYPE_NORMAL;
+	dbc_tty_driver->init_termios = tty_std_termios;
+	dbc_tty_driver->init_termios.c_cflag =
+			B9600 | CS8 | CREAD | HUPCL | CLOCAL;
+	dbc_tty_driver->init_termios.c_ispeed = 9600;
+	dbc_tty_driver->init_termios.c_ospeed = 9600;
+	dbc_tty_driver->driver_state = &dbc->port;
+
+	tty_set_operations(dbc_tty_driver, &dbc_tty_ops);
+
+	status = tty_register_driver(dbc_tty_driver);
+	if (status) {
+		xhci_err(xhci,
+			 "can't register dbc tty driver, err %d\n", status);
+		put_tty_driver(dbc_tty_driver);
+		dbc_tty_driver = NULL;
+	}
+
+	return status;
+}
+
+void xhci_dbc_tty_unregister_driver(void)
+{
+	tty_unregister_driver(dbc_tty_driver);
+	put_tty_driver(dbc_tty_driver);
+	dbc_tty_driver = NULL;
+}
+
+static void dbc_rx_push(unsigned long _port)
+{
+	struct dbc_request	*req;
+	struct tty_struct	*tty;
+	bool			do_push = false;
+	bool			disconnect = false;
+	struct dbc_port		*port = (void *)_port;
+	struct list_head	*queue = &port->read_queue;
+
+	spin_lock_irq(&port->port_lock);
+	tty = port->port.tty;
+	while (!list_empty(queue)) {
+		req = list_first_entry(queue, struct dbc_request, list_pool);
+
+		if (tty && tty_throttled(tty))
+			break;
+
+		switch (req->status) {
+		case 0:
+			break;
+		case -ESHUTDOWN:
+			disconnect = true;
+			break;
+		default:
+			pr_warn("ttyDBC0: unexpected RX status %d\n",
+				req->status);
+			break;
+		}
+
+		if (req->actual) {
+			char		*packet = req->buf;
+			unsigned int	n, size = req->actual;
+			int		count;
+
+			n = port->n_read;
+			if (n) {
+				packet += n;
+				size -= n;
+			}
+
+			count = tty_insert_flip_string(&port->port, packet,
+						       size);
+			if (count)
+				do_push = true;
+			if (count != size) {
+				port->n_read += count;
+				break;
+			}
+			port->n_read = 0;
+		}
+
+		list_move(&req->list_pool, &port->read_pool);
+	}
+
+	if (do_push)
+		tty_flip_buffer_push(&port->port);
+
+	if (!list_empty(queue) && tty) {
+		if (!tty_throttled(tty)) {
+			if (do_push)
+				tasklet_schedule(&port->push);
+			else
+				pr_warn("ttyDBC0: RX not scheduled?\n");
+		}
+	}
+
+	if (!disconnect)
+		dbc_start_rx(port);
+
+	spin_unlock_irq(&port->port_lock);
+}
+
+static int dbc_port_activate(struct tty_port *_port, struct tty_struct *tty)
+{
+	struct dbc_port	*port = container_of(_port, struct dbc_port, port);
+
+	spin_lock_irq(&port->port_lock);
+	dbc_start_rx(port);
+	spin_unlock_irq(&port->port_lock);
+
+	return 0;
+}
+
+static const struct tty_port_operations dbc_port_ops = {
+	.activate =	dbc_port_activate,
+};
+
+static void
+xhci_dbc_tty_init_port(struct xhci_hcd *xhci, struct dbc_port *port)
+{
+	tty_port_init(&port->port);
+	spin_lock_init(&port->port_lock);
+	tasklet_init(&port->push, dbc_rx_push, (unsigned long)port);
+	INIT_LIST_HEAD(&port->read_pool);
+	INIT_LIST_HEAD(&port->read_queue);
+	INIT_LIST_HEAD(&port->write_pool);
+
+	port->in =		get_in_ep(xhci);
+	port->out =		get_out_ep(xhci);
+	port->port.ops =	&dbc_port_ops;
+	port->n_read =		0;
+}
+
+static void
+xhci_dbc_tty_exit_port(struct dbc_port *port)
+{
+	tasklet_kill(&port->push);
+	tty_port_destroy(&port->port);
+}
+
+int xhci_dbc_tty_register_device(struct xhci_hcd *xhci)
+{
+	int			ret;
+	struct device		*tty_dev;
+	struct xhci_dbc		*dbc = xhci->dbc;
+	struct dbc_port		*port = &dbc->port;
+
+	xhci_dbc_tty_init_port(xhci, port);
+	tty_dev = tty_port_register_device(&port->port,
+					   dbc_tty_driver, 0, NULL);
+	ret = IS_ERR_OR_NULL(tty_dev);
+	if (ret)
+		goto register_fail;
+
+	ret = dbc_buf_alloc(&port->port_write_buf, WRITE_BUF_SIZE);
+	if (ret)
+		goto buf_alloc_fail;
+
+	ret = xhci_dbc_alloc_requests(port->in, &port->read_pool,
+				      dbc_read_complete);
+	if (ret)
+		goto request_fail;
+
+	ret = xhci_dbc_alloc_requests(port->out, &port->write_pool,
+				      dbc_write_complete);
+	if (ret)
+		goto request_fail;
+
+	port->registered = true;
+
+	return 0;
+
+request_fail:
+	xhci_dbc_free_requests(port->in, &port->read_pool);
+	xhci_dbc_free_requests(port->out, &port->write_pool);
+	dbc_buf_free(&port->port_write_buf);
+
+buf_alloc_fail:
+	tty_unregister_device(dbc_tty_driver, 0);
+
+register_fail:
+	xhci_dbc_tty_exit_port(port);
+
+	xhci_err(xhci, "can't register tty port, err %d\n", ret);
+
+	return ret;
+}
+
+void xhci_dbc_tty_unregister_device(struct xhci_hcd *xhci)
+{
+	struct xhci_dbc		*dbc = xhci->dbc;
+	struct dbc_port		*port = &dbc->port;
+
+	tty_unregister_device(dbc_tty_driver, 0);
+	xhci_dbc_tty_exit_port(port);
+	port->registered = false;
+
+	dbc_buf_free(&port->port_write_buf);
+	xhci_dbc_free_requests(get_out_ep(xhci), &port->read_pool);
+	xhci_dbc_free_requests(get_out_ep(xhci), &port->read_queue);
+	xhci_dbc_free_requests(get_in_ep(xhci), &port->write_pool);
+}
diff --git a/drivers/usb/host/xhci-trace.h b/drivers/usb/host/xhci-trace.h
index 8ce96de..bf2013c 100644
--- a/drivers/usb/host/xhci-trace.h
+++ b/drivers/usb/host/xhci-trace.h
@@ -26,6 +26,7 @@
 
 #include <linux/tracepoint.h>
 #include "xhci.h"
+#include "xhci-dbgcap.h"
 
 #define XHCI_MSG_MAX	500
 
@@ -158,6 +159,21 @@ DEFINE_EVENT(xhci_log_trb, xhci_queue_trb,
 	TP_ARGS(ring, trb)
 );
 
+DEFINE_EVENT(xhci_log_trb, xhci_dbc_handle_event,
+	TP_PROTO(struct xhci_ring *ring, struct xhci_generic_trb *trb),
+	TP_ARGS(ring, trb)
+);
+
+DEFINE_EVENT(xhci_log_trb, xhci_dbc_handle_transfer,
+	TP_PROTO(struct xhci_ring *ring, struct xhci_generic_trb *trb),
+	TP_ARGS(ring, trb)
+);
+
+DEFINE_EVENT(xhci_log_trb, xhci_dbc_gadget_ep_queue,
+	TP_PROTO(struct xhci_ring *ring, struct xhci_generic_trb *trb),
+	TP_ARGS(ring, trb)
+);
+
 DECLARE_EVENT_CLASS(xhci_log_virt_dev,
 	TP_PROTO(struct xhci_virt_device *vdev),
 	TP_ARGS(vdev),
@@ -453,6 +469,50 @@ DEFINE_EVENT(xhci_log_ring, xhci_inc_deq,
 	TP_PROTO(struct xhci_ring *ring),
 	TP_ARGS(ring)
 );
+
+DECLARE_EVENT_CLASS(xhci_dbc_log_request,
+	TP_PROTO(struct dbc_request *req),
+	TP_ARGS(req),
+	TP_STRUCT__entry(
+		__field(struct dbc_request *, req)
+		__field(bool, dir)
+		__field(unsigned int, actual)
+		__field(unsigned int, length)
+		__field(int, status)
+	),
+	TP_fast_assign(
+		__entry->req = req;
+		__entry->dir = req->direction;
+		__entry->actual = req->actual;
+		__entry->length = req->length;
+		__entry->status = req->status;
+	),
+	TP_printk("%s: req %p length %u/%u ==> %d",
+		__entry->dir ? "bulk-in" : "bulk-out",
+		__entry->req, __entry->actual,
+		__entry->length, __entry->status
+	)
+);
+
+DEFINE_EVENT(xhci_dbc_log_request, xhci_dbc_alloc_request,
+	TP_PROTO(struct dbc_request *req),
+	TP_ARGS(req)
+);
+
+DEFINE_EVENT(xhci_dbc_log_request, xhci_dbc_free_request,
+	TP_PROTO(struct dbc_request *req),
+	TP_ARGS(req)
+);
+
+DEFINE_EVENT(xhci_dbc_log_request, xhci_dbc_queue_request,
+	TP_PROTO(struct dbc_request *req),
+	TP_ARGS(req)
+);
+
+DEFINE_EVENT(xhci_dbc_log_request, xhci_dbc_giveback_request,
+	TP_PROTO(struct dbc_request *req),
+	TP_ARGS(req)
+);
 #endif /* __XHCI_TRACE_H */
 
 /* this part must be outside header guard */
diff --git a/drivers/usb/host/xhci.c b/drivers/usb/host/xhci.c
index b2ff1ff..39a9a88 100644
--- a/drivers/usb/host/xhci.c
+++ b/drivers/usb/host/xhci.c
@@ -32,6 +32,7 @@
 #include "xhci.h"
 #include "xhci-trace.h"
 #include "xhci-mtk.h"
+#include "xhci-dbgcap.h"
 
 #define DRIVER_AUTHOR "Sarah Sharp"
 #define DRIVER_DESC "'eXtensible' Host Controller (xHC) Driver"
@@ -632,6 +633,9 @@ int xhci_run(struct usb_hcd *hcd)
 	}
 	xhci_dbg_trace(xhci, trace_xhci_dbg_init,
 			"Finished xhci_run for USB2 roothub");
+
+	dbc_create_sysfs_file(xhci);
+
 	return 0;
 }
 EXPORT_SYMBOL_GPL(xhci_run);
@@ -660,6 +664,8 @@ static void xhci_stop(struct usb_hcd *hcd)
 		return;
 	}
 
+	dbc_remove_sysfs_file(xhci);
+
 	spin_lock_irq(&xhci->lock);
 	xhci->xhc_state |= XHCI_STATE_HALTED;
 	xhci->cmd_ring_state = CMD_RING_STATE_STOPPED;
@@ -876,6 +882,8 @@ int xhci_suspend(struct xhci_hcd *xhci, bool do_wakeup)
 			xhci->shared_hcd->state != HC_STATE_SUSPENDED)
 		return -EINVAL;
 
+	xhci_dbc_suspend(xhci);
+
 	/* Clear root port wake on bits if wakeup not allowed. */
 	if (!do_wakeup)
 		xhci_disable_port_wake_on_bits(xhci);
@@ -1071,6 +1079,8 @@ int xhci_resume(struct xhci_hcd *xhci, bool hibernated)
 
 	spin_unlock_irq(&xhci->lock);
 
+	xhci_dbc_resume(xhci);
+
  done:
 	if (retval == 0) {
 		/* Resume root hubs only when have pending events. */
diff --git a/drivers/usb/host/xhci.h b/drivers/usb/host/xhci.h
index 62bcdcd..bae2538 100644
--- a/drivers/usb/host/xhci.h
+++ b/drivers/usb/host/xhci.h
@@ -1850,6 +1850,7 @@ struct xhci_hcd {
 /* Compliance Mode Timer Triggered every 2 seconds */
 #define COMP_MODE_RCVRY_MSECS 2000
 
+	void			*dbc;
 	/* platform-specific data -- must come last */
 	unsigned long		priv[0] __aligned(sizeof(s64));
 };
-- 
2.7.4

^ permalink raw reply related	[flat|nested] 21+ messages in thread

* [PATCH v3 3/3] usb: doc: Update document for USB3 debug port usage
  2017-09-05  1:58 [PATCH v3 0/3] usb: xhci: Add debug capability support in xhci Lu Baolu
  2017-09-05  1:58 ` [PATCH v3 1/3] usb: xhci: Make some static functions global Lu Baolu
  2017-09-05  1:58 ` [PATCH v3 2/3] usb: xhci: Add DbC support in xHCI driver Lu Baolu
@ 2017-09-05  1:59 ` Lu Baolu
  2 siblings, 0 replies; 21+ messages in thread
From: Lu Baolu @ 2017-09-05  1:59 UTC (permalink / raw)
  To: Mathias Nyman, Greg Kroah-Hartman; +Cc: linux-usb, linux-kernel, Lu Baolu

Update Documentation/driver-api/usb/usb3-debug-port.rst. This update
includes the guide for using xHCI debug capability based TTY serial
link.

Signed-off-by: Lu Baolu <baolu.lu@linux.intel.com>
---
 Documentation/driver-api/usb/usb3-debug-port.rst | 68 ++++++++++++++++++++++++
 1 file changed, 68 insertions(+)

diff --git a/Documentation/driver-api/usb/usb3-debug-port.rst b/Documentation/driver-api/usb/usb3-debug-port.rst
index feb1a36..4ad4787 100644
--- a/Documentation/driver-api/usb/usb3-debug-port.rst
+++ b/Documentation/driver-api/usb/usb3-debug-port.rst
@@ -98,3 +98,71 @@ you to check the sanity of the setup.
 	cat /dev/ttyUSB0
 	done
 	===== end of bash scripts ===============
+
+Serial TTY
+==========
+
+DbC has also been designed for a serial TTY device at runtime.
+One use of this is running a login service on the debug target.
+Hence it can be remote accessed by the debug host. Another use
+can probably be found in servers. It provides a peer-to-peer USB
+link between two host-only machines. This provides a reasonable
+out-of-band communication method between two servers.
+
+In order to use this, you need to make sure your kernel has been
+configured to support USB_XHCI_DBGCAP. A sysfs attribute under
+the xHCI device node is used to enable or disable DbC. By default,
+DbC is disabled::
+
+	root@target:/sys/bus/pci/devices/0000:00:14.0# cat dbc
+	disabled
+
+Enable DbC with the following command::
+
+	root@target:/sys/bus/pci/devices/0000:00:14.0# echo enable > dbc
+
+You can check the DbC state at anytime::
+
+	root@target:/sys/bus/pci/devices/0000:00:14.0# cat dbc
+	enabled
+
+Connect the debug target to the debug host with a USB 3.0 super-
+speed A-to-A debugging cable. You can see the /dev/ttyDBC0 created
+on the debug target. You will see below kernel message lines::
+
+	root@target: tail -f /var/log/kern.log
+	[  182.730103] xhci_hcd 0000:00:14.0: DbC connected
+	[  191.169420] xhci_hcd 0000:00:14.0: DbC configured
+	[  191.169597] xhci_hcd 0000:00:14.0: DbC now attached to /dev/ttyDBC0
+
+Accordingly, the DbC state has been brought up to::
+
+	root@host:/sys/bus/pci/devices/0000:00:14.0# cat dbc
+	configured
+
+On the debug host, you will see the debug device has been enumerated.
+You will see below kernel message lines::
+
+	root@host: tail -f /var/log/kern.log
+	[   79.454780] usb 2-2.1: new SuperSpeed USB device number 3 using xhci_hcd
+	[   79.475003] usb 2-2.1: LPM exit latency is zeroed, disabling LPM.
+	[   79.475389] usb 2-2.1: New USB device found, idVendor=1d6b, idProduct=0004
+	[   79.475390] usb 2-2.1: New USB device strings: Mfr=1, Product=2, SerialNumber=3
+	[   79.475391] usb 2-2.1: Product: Remote GDB
+	[   79.475392] usb 2-2.1: Manufacturer: Linux
+	[   79.475393] usb 2-2.1: SerialNumber: 0001
+	[   79.660368] usb_debug 2-2.1:1.0: xhci_dbc converter detected
+	[   79.660439] usb 2-2.1: xhci_dbc converter now attached to ttyUSB0
+
+You can simply verify whether it works by::
+
+	# On target side
+	root@target: echo "Hello world" > /dev/ttyDBC0
+
+	# On host side
+	root@host: cat /dev/ttyUSB0
+	Hello world
+
+	# vice versa
+
+You have a workable serial link over USB now.
-- 
2.7.4

^ permalink raw reply related	[flat|nested] 21+ messages in thread

* Re: [PATCH v3 2/3] usb: xhci: Add DbC support in xHCI driver
  2017-09-05  1:58 ` [PATCH v3 2/3] usb: xhci: Add DbC support in xHCI driver Lu Baolu
@ 2017-10-24 17:03   ` Mathias Nyman
  2017-10-25  2:15     ` Lu Baolu
  2017-11-02  1:36   ` Lu Baolu
  2017-11-13 15:26   ` Mathias Nyman
  2 siblings, 1 reply; 21+ messages in thread
From: Mathias Nyman @ 2017-10-24 17:03 UTC (permalink / raw)
  To: Lu Baolu; +Cc: linux-kernel

Hi

Skipping lists, partial review.

Many of the questions are real questions that I was wondering about
while lookinga the code. Not necessarily thing that must be changed.

On 05.09.2017 04:58, Lu Baolu wrote:
> xHCI compatible USB host controllers(i.e. super-speed USB3 controllers)
> can be implemented with the Debug Capability(DbC). It presents a debug
> device which is fully compliant with the USB framework and provides the
> equivalent of a very high performance full-duplex serial link. The debug
> capability operation model and registers interface are defined in 7.6.8
> of the xHCI specification, revision 1.1.
>
> The DbC debug device shares a root port with the xHCI host. By default,
> the debug capability is disabled and the root port is assigned to xHCI.
> When the DbC is enabled, the root port will be assigned to the DbC debug
> device, and the xHCI sees nothing on this port. This implementation uses
> a sysfs node named <dbc> under the xHCI device to manage the enabling
> and disabling of the debug capability.
>
> When the debug capability is enabled, it will present a debug device
> through the debug port. This debug device is fully compliant with the
> USB3 framework, and it can be enumerated by a debug host on the other
> end of the USB link. As soon as the debug device is configured, a TTY
> serial device named /dev/ttyDBC0 will be created.
>
> One use of this link is running a login service on the debug target.
> Hence it can be remote accessed by a debug host. Another use case can
> probably be found in servers. It provides a peer-to-peer USB link
> between two host-only machines. This provides a reasonable out-of-band
> communication method between two servers.
>
> Signed-off-by: Lu Baolu <baolu.lu@linux.intel.com>
> ---
>   .../ABI/testing/sysfs-bus-pci-drivers-xhci_hcd     |   25 +
>   drivers/usb/host/Kconfig                           |    9 +
>   drivers/usb/host/Makefile                          |    5 +
>   drivers/usb/host/xhci-dbgcap.c                     | 1016 ++++++++++++++++++++
>   drivers/usb/host/xhci-dbgcap.h                     |  247 +++++
>   drivers/usb/host/xhci-dbgtty.c                     |  586 +++++++++++
>   drivers/usb/host/xhci-trace.h                      |   60 ++
>   drivers/usb/host/xhci.c                            |   10 +
>   drivers/usb/host/xhci.h                            |    1 +
>   9 files changed, 1959 insertions(+)
>   create mode 100644 Documentation/ABI/testing/sysfs-bus-pci-drivers-xhci_hcd
>   create mode 100644 drivers/usb/host/xhci-dbgcap.c
>   create mode 100644 drivers/usb/host/xhci-dbgcap.h
>   create mode 100644 drivers/usb/host/xhci-dbgtty.c
>
> diff --git a/Documentation/ABI/testing/sysfs-bus-pci-drivers-xhci_hcd b/Documentation/ABI/testing/sysfs-bus-pci-drivers-xhci_hcd
> new file mode 100644
> index 0000000..0088aba
> --- /dev/null
> +++ b/Documentation/ABI/testing/sysfs-bus-pci-drivers-xhci_hcd
> @@ -0,0 +1,25 @@
> +What:		/sys/bus/pci/drivers/xhci_hcd/.../dbc
> +Date:		June 2017
> +Contact:	Lu Baolu <baolu.lu@linux.intel.com>
> +Description:
> +		xHCI compatible USB host controllers (i.e. super-speed
> +		USB3 controllers) are often implemented with the Debug
> +		Capability (DbC). It can present a debug device which
> +		is fully compliant with the USB framework and provides
> +		the equivalent of a very high performance full-duplex
> +		serial link for debug purpose.
> +
> +		The DbC debug device shares a root port with xHCI host.
> +		When the DbC is enabled, the root port will be assigned
> +		to the Debug Capability. Otherwise, it will be assigned
> +		to xHCI.
> +
> +		Writing "enable" to this attribute will enable the DbC
> +		functionality and the shared root port will be assigned
> +		to the DbC device. Writing "disable" to this attribute
> +		will disable the DbC functionality and the shared root
> +		port will roll back to the xHCI.
> +
> +		Reading this attribute gives the state of the DbC. It
> +		can be one of the following states: disabled, enabled,
> +		initialized, connected, configured and stalled.
> diff --git a/drivers/usb/host/Kconfig b/drivers/usb/host/Kconfig
> index fa5692d..968a196 100644
> --- a/drivers/usb/host/Kconfig
> +++ b/drivers/usb/host/Kconfig
> @@ -27,6 +27,15 @@ config USB_XHCI_HCD
>   	  module will be called xhci-hcd.
>
>   if USB_XHCI_HCD
> +config USB_XHCI_DBGCAP
> +	bool "xHCI support for debug capability"
> +	depends on TTY
> +	select USB_U_SERIAL
> +	---help---
> +	  Say 'Y' to enable the support for the xHCI debug capability. Make
> +	  sure that your xHCI host supports the extended debug capability and
> +	  you want a TTY serial device based on the xHCI debug capability
> +	  before enabling this option. If unsure, say 'N'.
>
>   config USB_XHCI_PCI
>          tristate
> diff --git a/drivers/usb/host/Makefile b/drivers/usb/host/Makefile
> index cf2691f..175c80a 100644
> --- a/drivers/usb/host/Makefile
> +++ b/drivers/usb/host/Makefile
> @@ -13,6 +13,11 @@ fhci-$(CONFIG_FHCI_DEBUG) += fhci-dbg.o
>   xhci-hcd-y := xhci.o xhci-mem.o
>   xhci-hcd-y += xhci-ring.o xhci-hub.o xhci-dbg.o
>   xhci-hcd-y += xhci-trace.o
> +
> +ifneq ($(CONFIG_USB_XHCI_DBGCAP), )
> +	xhci-hcd-y += xhci-dbgcap.o xhci-dbgtty.o
> +endif
> +
>   ifneq ($(CONFIG_USB_XHCI_MTK), )
>   	xhci-hcd-y += xhci-mtk-sch.o
>   endif
> diff --git a/drivers/usb/host/xhci-dbgcap.c b/drivers/usb/host/xhci-dbgcap.c
> new file mode 100644
> index 0000000..9816085
> --- /dev/null
> +++ b/drivers/usb/host/xhci-dbgcap.c
> @@ -0,0 +1,1016 @@
> +/**
> + * xhci-dbgcap.c - xHCI debug capability support
> + *
> + * Copyright (C) 2017 Intel Corporation
> + *
> + * Author: Lu Baolu <baolu.lu@linux.intel.com>
> + *
> + * This program is free software; you can redistribute it and/or modify
> + * it under the terms of the GNU General Public License version 2 as
> + * published by the Free Software Foundation.
> + */
> +#include <linux/dma-mapping.h>
> +#include <linux/slab.h>
> +
> +#include "xhci.h"
> +#include "xhci-trace.h"
> +#include "xhci-dbgcap.h"
> +
> +static inline void *
> +dbc_dma_alloc_coherent(struct xhci_hcd *xhci, size_t size,
> +		       dma_addr_t *dma_handle, gfp_t flags)
> +{
> +	void		*vaddr;
> +
> +	vaddr = dma_alloc_coherent(xhci_to_hcd(xhci)->self.sysdev,
> +				   size, dma_handle, flags);
> +	memset(vaddr, 0, size);
> +	return vaddr;
> +}
> +
> +static inline void
> +dbc_dma_free_coherent(struct xhci_hcd *xhci, size_t size,
> +		      void *cpu_addr, dma_addr_t dma_handle)
> +{
> +	if (cpu_addr)
> +		dma_free_coherent(xhci_to_hcd(xhci)->self.sysdev,
> +				  size, cpu_addr, dma_handle);
> +}
> +

Is there any benefit in having these dbc wrapped variants
of dma alloc/free coherent functions?
Can't dma_zalloc_coherent() and dma_free_coherent() be used directly?

> +static inline void xhci_put_utf16(u16 *s, const char *c)
> +{
> +	int		i;
> +	size_t		size;
> +
> +	size = strlen(c);
> +	for (i = 0; i < size; i++)
> +		s[i] = cpu_to_le16(c[i]);
> +}
> +

could utf8s_to_utf16s() be used instead?
At least drivers/usb/misc/usb251xb.c is using it to set some string descriptors.

> +static u32 xhci_dbc_populate_strings(struct dbc_strings *strings)
> +{
> +	struct usb_string_descriptor	*s_desc;
> +	u32				string_length;
> +
> +	/* Serial string: */
> +	s_desc = (struct usb_string_descriptor *)strings->serial;

Took me a second to understand that dbc_strings is not just strings, but char
arrays where we store entire string descriptors with length, type and UTF16LE string.

small thing, but maybe it structure could be called struct dbc_str_descs or similar

> +	xhci_put_utf16(s_desc->wData, DBC_STRING_SERIAL);
> +
> +	s_desc->bLength		= (strlen(DBC_STRING_SERIAL) + 1) * 2;
> +	s_desc->bDescriptorType	= USB_DT_STRING;
> +	string_length		= s_desc->bLength;
> +	string_length		<<= 8;
> +
> +	/* Product string: */
> +	s_desc = (struct usb_string_descriptor *)strings->product;
> +	xhci_put_utf16(s_desc->wData, DBC_STRING_PRODUCT);
> +
> +	s_desc->bLength		= (strlen(DBC_STRING_PRODUCT) + 1) * 2;
> +	s_desc->bDescriptorType	= USB_DT_STRING;
> +	string_length		+= s_desc->bLength;
> +	string_length		<<= 8;
> +
> +	/* Manufacture string: */
> +	s_desc = (struct usb_string_descriptor *)strings->manufacturer;
> +	xhci_put_utf16(s_desc->wData, DBC_STRING_MANUFACTURER);
> +
> +	s_desc->bLength		= (strlen(DBC_STRING_MANUFACTURER) + 1) * 2;
> +	s_desc->bDescriptorType	= USB_DT_STRING;
> +	string_length		+= s_desc->bLength;
> +	string_length		<<= 8;
> +
> +	/* String0: */
> +	strings->string0[0]	= 4;
> +	strings->string0[1]	= USB_DT_STRING;
> +	strings->string0[2]	= 0x09;
> +	strings->string0[3]	= 0x04;
> +	string_length		+= 4;
> +
> +	return string_length;
> +}
> +
> +static void xhci_dbc_populate_contexts(struct xhci_hcd *xhci, u32 string_length)
> +{
> +	struct xhci_dbc		*dbc;
> +	struct dbc_info_context	*info;
> +	struct xhci_ep_ctx	*ep_ctx;
> +	u32			dev_info;
> +	dma_addr_t		deq, dma;
> +	unsigned int		max_burst;
> +
> +	dbc = xhci->dbc;
> +	if (!dbc)
> +		return;
> +
> +	/* Populate info Context: */
> +	info			= (struct dbc_info_context *)dbc->ctx->bytes;
> +	dma			= dbc->string_dma;
> +	info->string0		= cpu_to_le64(dma);
> +	info->manufacturer	= cpu_to_le64(dma + DBC_MAX_STRING_LENGTH);
> +	info->product		= cpu_to_le64(dma + DBC_MAX_STRING_LENGTH * 2);
> +	info->serial		= cpu_to_le64(dma + DBC_MAX_STRING_LENGTH * 3);
> +	info->length		= cpu_to_le32(string_length);
> +
> +	/* Populate bulk out endpoint context: */
> +	ep_ctx			= dbc_bulkout_ctx(dbc);
> +	max_burst		= DBC_CTRL_MAXBURST(readl(&dbc->regs->control));
> +	deq			= dbc_bulkout_enq(dbc);
> +	ep_ctx->ep_info		= 0;
> +	ep_ctx->ep_info2	= dbc_epctx_info2(BULK_OUT_EP, 1024, max_burst);
> +	ep_ctx->deq		= cpu_to_le64(deq | dbc->ring_out->cycle_state);
> +
> +	/* Populate bulk in endpoint context: */
> +	ep_ctx			= dbc_bulkin_ctx(dbc);
> +	deq			= dbc_bulkin_enq(dbc);
> +	ep_ctx->ep_info		= 0;
> +	ep_ctx->ep_info2	= dbc_epctx_info2(BULK_IN_EP, 1024, max_burst);
> +	ep_ctx->deq		= cpu_to_le64(deq | dbc->ring_in->cycle_state);

Are we setting the ep_ctx->deq to ring deq instead of
segment start because we are going to call this runtime/mid tranfer?

> +
> +	/* Set DbC context and info registers: */
> +	xhci_write_64(xhci, dbc->ctx->dma, &dbc->regs->dccp);
> +
> +	dev_info = cpu_to_le32((DBC_VENDOR_ID << 16) | DBC_PROTOCOL);
> +	writel(dev_info, &dbc->regs->devinfo1);
> +
> +	dev_info = cpu_to_le32((DBC_DEVICE_REV << 16) | DBC_PRODUCT_ID);
> +	writel(dev_info, &dbc->regs->devinfo2);

Minor thing again but this function initializes a couple registers in addition
to just populating the contexts. Maybe split or rename function

> +}
> +
> +static void xhci_dbc_giveback(struct dbc_request *req, int status)
> +	__releases(&dbc->lock)
> +	__acquires(&dbc->lock)
> +{
> +	struct dbc_ep		*dep = req->dep;
> +	struct xhci_dbc		*dbc = dep->dbc;
> +	struct xhci_hcd		*xhci = dbc->xhci;
> +	struct device		*dev = xhci_to_hcd(dbc->xhci)->self.sysdev;
> +
> +	trace_xhci_dbc_giveback_request(req);
> +
> +	list_del_init(&req->list_pending);

why do we need to reinitialize the entry, is this entry reused
or accessed after later afetr this. would list_del() work?

> +	req->trb_dma = 0;
> +	req->trb = NULL;
> +
> +	if (req->status == -EINPROGRESS)
> +		req->status = status;
> +
> +	dma_unmap_single(dev,
> +			 req->dma,
> +			 req->length,
> +			 dbc_ep_dma_direction(dep));
> +
> +	/* Give back the transfer request: */
> +	spin_unlock(&dbc->lock);
> +	req->complete(xhci, req);
> +	spin_lock(&dbc->lock);
> +}
> +
> +static void xhci_dbc_flush_single_request(struct dbc_request *req)
> +{
> +	union xhci_trb	*trb = req->trb;
> +
> +	trb->generic.field[0]	= 0;
> +	trb->generic.field[1]	= 0;
> +	trb->generic.field[2]	= 0;
> +	trb->generic.field[3]	&= cpu_to_le32(TRB_CYCLE);
> +	trb->generic.field[3]	|= cpu_to_le32(TRB_TYPE(TRB_TR_NOOP));
> +
> +	xhci_dbc_giveback(req, -ESHUTDOWN);
> +}
> +
> +static void xhci_dbc_flush_endpoint_requests(struct dbc_ep *dep)
> +{
> +	struct dbc_request	*req, *tmp;
> +
> +	list_for_each_entry_safe(req, tmp, &dep->list_pending, list_pending)
> +		xhci_dbc_flush_single_request(req);
> +}
> +
> +static void xhci_dbc_flush_reqests(struct xhci_dbc *dbc)
> +{
> +	int			i;
> +	struct dbc_ep		*dep;
> +
> +	for (i = BULK_OUT; i <= BULK_IN; i++) {
> +		dep = &dbc->eps[i];
> +		xhci_dbc_flush_endpoint_requests(dep);
> +	}

we only have one in and one out endpiont, how about just
	xhci_dbc_flush_endpoint_requests(&dbc->eps[BULK_OUT]);
	xhci_dbc_flush_endpoint_requests(&dbc->eps[BULK_IN]);

> +}
> +
> +static struct dbc_request *
> +dbc_ep_alloc_request(struct dbc_ep *dep, gfp_t gfp_flags)
> +{
> +	struct dbc_request	*req;
> +
> +	req = kzalloc(sizeof(*req), gfp_flags);
> +	if (!req)
> +		return NULL;
> +
> +	req->dep = dep;
> +	INIT_LIST_HEAD(&req->list_pending);
> +	INIT_LIST_HEAD(&req->list_pool);
> +
> +	trace_xhci_dbc_alloc_request(req);
> +
> +	return req;
> +}
> +
> +static void
> +dbc_ep_free_request(struct dbc_ep *dep, struct dbc_request *req)
> +{
> +	trace_xhci_dbc_free_request(req);
> +
> +	kfree(req);
> +}
> +
> +static void
> +xhci_dbc_queue_trb(struct xhci_ring *ring, u32 field1,
> +		   u32 field2, u32 field3, u32 field4)
> +{
> +	union xhci_trb		*trb, *next;
> +
> +	trb = ring->enqueue;
> +	trb->generic.field[0]	= cpu_to_le32(field1);
> +	trb->generic.field[1]	= cpu_to_le32(field2);
> +	trb->generic.field[2]	= cpu_to_le32(field3);
> +	trb->generic.field[3]	= cpu_to_le32(field4);
> +
> +	trace_xhci_dbc_gadget_ep_queue(ring, &trb->generic);
> +
> +	ring->num_trbs_free--;
> +	next = ++(ring->enqueue);
> +	if (TRB_TYPE_LINK_LE32(next->link.control)) {
> +		next->link.control ^= cpu_to_le32(TRB_CYCLE);
> +		ring->enqueue = ring->enq_seg->trbs;
> +		ring->cycle_state ^= 1;
> +	}
> +}
> +
> +static int xhci_dbc_queue_bulk_tx(struct dbc_ep *dep,
> +				  struct dbc_request *req)
> +{
> +	u64			addr;
> +	union xhci_trb		*trb;
> +	unsigned int		num_trbs;
> +	struct xhci_dbc		*dbc = dep->dbc;
> +	struct xhci_ring	*ring = dep->ring;
> +	u32			length, control, cycle;
> +
> +	num_trbs = count_trbs(req->dma, req->length);
> +	WARN_ON(num_trbs != 1);
> +	if (ring->num_trbs_free < num_trbs)
> +		return -EBUSY;
> +
> +	addr	= req->dma;
> +	trb	= ring->enqueue;
> +	cycle	= ring->cycle_state;
> +	length	= TRB_LEN(req->length);
> +	control	= TRB_TYPE(TRB_NORMAL) | TRB_IOC;
> +
> +	if (cycle)
> +		control &= cpu_to_le32(~TRB_CYCLE);
> +	else
> +		control |= cpu_to_le32(TRB_CYCLE);
> +
> +	req->trb = ring->enqueue;
> +	req->trb_dma = xhci_trb_virt_to_dma(ring->enq_seg, ring->enqueue);
> +	xhci_dbc_queue_trb(ring,
> +			   lower_32_bits(addr),
> +			   upper_32_bits(addr),
> +			   length, control);
> +
> +	/*
> +	 * Add a barrier between writes of trb fields and flipping
> +	 * the cycle bit:
> +	 */
> +	wmb();
> +
> +	if (cycle)
> +		trb->generic.field[3] |= cpu_to_le32(TRB_CYCLE);
> +	else
> +		trb->generic.field[3] &= cpu_to_le32(~TRB_CYCLE);
> +
> +	writel(DBC_DOOR_BELL_TARGET(dep->direction), &dbc->regs->doorbell);
> +
> +	return 0;
> +}
> +
> +static int
> +dbc_ep_do_queue(struct dbc_ep *dep, struct dbc_request *req)
> +{
> +	int			ret;
> +	struct device		*dev;
> +	struct xhci_dbc		*dbc = dep->dbc;
> +	struct xhci_hcd		*xhci = dbc->xhci;
> +
> +	dev = xhci_to_hcd(xhci)->self.sysdev;
> +
> +	if (!req->length || !req->buf)
> +		return -EINVAL;
> +
> +	req->actual		= 0;
> +	req->status		= -EINPROGRESS;
> +
> +	req->dma = dma_map_single(dev,
> +				  req->buf,
> +				  req->length,
> +				  dbc_ep_dma_direction(dep));
> +	if (dma_mapping_error(dev, req->dma)) {
> +		xhci_err(xhci, "failed to map buffer\n");
> +		return -EFAULT;
> +	}
> +
> +	ret = xhci_dbc_queue_bulk_tx(dep, req);
> +	if (ret) {
> +		xhci_err(xhci, "failed to queue trbs\n");
> +		dma_unmap_single(dev,
> +				 req->dma,
> +				 req->length,
> +				 dbc_ep_dma_direction(dep));
> +		return -EFAULT;
> +	}
> +
> +	list_add_tail(&req->list_pending, &dep->list_pending);
> +
> +	return 0;
> +}
> +
> +static int dbc_ep_queue(struct dbc_ep *dep,
> +			struct dbc_request *req,
> +			gfp_t gfp_flags)
> +{
> +	struct xhci_dbc		*dbc = dep->dbc;
> +	int			ret = -ESHUTDOWN;
> +
> +	spin_lock(&dbc->lock);
> +	if (dbc->state == DS_CONFIGURED)
> +		ret = dbc_ep_do_queue(dep, req);
> +	spin_unlock(&dbc->lock);
> +
> +	trace_xhci_dbc_queue_request(req);
> +
> +	return ret;
> +}
> +
> +static inline void xhci_dbc_do_eps_init(struct xhci_hcd *xhci, bool direction)
> +{
> +	struct dbc_ep		*dep;
> +	struct xhci_dbc		*dbc = xhci->dbc;
> +
> +	dep			= &dbc->eps[direction];
> +	dep->dbc		= dbc;
> +	dep->direction		= direction;
> +	dep->ring		= direction ? dbc->ring_in : dbc->ring_out;
> +	dep->alloc_request	= dbc_ep_alloc_request;
> +	dep->free_request	= dbc_ep_free_request;
> +	dep->queue		= dbc_ep_queue;

why do we need the alloc_request, free_request and queue function pointers in struct dbc_ep?
They are always pointing to the same functions.

> +
> +	INIT_LIST_HEAD(&dep->list_pending);
> +}
> +
> +static void xhci_dbc_eps_init(struct xhci_hcd *xhci)
> +{
> +	xhci_dbc_do_eps_init(xhci, BULK_OUT);
> +	xhci_dbc_do_eps_init(xhci, BULK_IN);
> +}
> +
> +static void xhci_dbc_eps_exit(struct xhci_hcd *xhci)
> +{
> +	struct xhci_dbc		*dbc = xhci->dbc;
> +
> +	memset(dbc->eps, 0, ARRAY_SIZE(dbc->eps));
> +}
> +
> +static int xhci_dbc_mem_init(struct xhci_hcd *xhci, gfp_t flags)
> +{
> +	int			ret;
> +	dma_addr_t		deq;
> +	u32			string_length;
> +	struct xhci_dbc		*dbc = xhci->dbc;
> +
> +	/* Allocate various rings for events and transfers: */
> +	dbc->ring_evt = xhci_ring_alloc(xhci, 1, 1, TYPE_EVENT, 0, flags);
> +	if (!dbc->ring_evt)
> +		goto evt_fail;
> +
> +	dbc->ring_in = xhci_ring_alloc(xhci, 1, 1, TYPE_BULK, 0, flags);
> +	if (!dbc->ring_in)
> +		goto in_fail;
> +
> +	dbc->ring_out = xhci_ring_alloc(xhci, 1, 1, TYPE_BULK, 0, flags);
> +	if (!dbc->ring_out)
> +		goto out_fail;
> +
> +	/* Allocate and populate ERST: */
> +	ret = xhci_alloc_erst(xhci, dbc->ring_evt, &dbc->erst, flags);
> +	if (ret)
> +		goto erst_fail;
> +
> +	/* Allocate context data structure: */
> +	dbc->ctx = xhci_alloc_container_ctx(xhci, XHCI_CTX_TYPE_DEVICE, flags);
> +	if (!dbc->ctx)
> +		goto ctx_fail;
> +
> +	/* Allocate the string table: */
> +	dbc->string_size = sizeof(struct dbc_strings);
> +	dbc->string = dbc_dma_alloc_coherent(xhci,
> +					     dbc->string_size,
> +					     &dbc->string_dma,
> +					     flags);
> +	if (!dbc->string)
> +		goto string_fail;
> +
> +	/* Setup ERST register: */
> +	writel(dbc->erst.erst_size, &dbc->regs->ersts);
> +	xhci_write_64(xhci, dbc->erst.erst_dma_addr, &dbc->regs->erstba);
> +	deq = xhci_trb_virt_to_dma(dbc->ring_evt->deq_seg,
> +				   dbc->ring_evt->dequeue);
> +	xhci_write_64(xhci, deq, &dbc->regs->erdp);
> +
> +	/* Setup strings and contexts: */
> +	string_length = xhci_dbc_populate_strings(dbc->string);
> +	xhci_dbc_populate_contexts(xhci, string_length);
> +
> +	mmiowb();
> +
> +	xhci_dbc_eps_init(xhci);
> +	dbc->state = DS_INITIALIZED;
> +
> +	return 0;
> +
> +string_fail:
> +	xhci_free_container_ctx(xhci, dbc->ctx);
> +	dbc->ctx = NULL;
> +ctx_fail:
> +	xhci_free_erst(xhci, &dbc->erst);
> +erst_fail:
> +	xhci_ring_free(xhci, dbc->ring_out);
> +	dbc->ring_out = NULL;
> +out_fail:
> +	xhci_ring_free(xhci, dbc->ring_in);
> +	dbc->ring_in = NULL;
> +in_fail:
> +	xhci_ring_free(xhci, dbc->ring_evt);
> +	dbc->ring_evt = NULL;
> +evt_fail:
> +	return -ENOMEM;
> +}
> +
> +static void xhci_dbc_mem_cleanup(struct xhci_hcd *xhci)
> +{
> +	struct xhci_dbc		*dbc = xhci->dbc;
> +
> +	if (!dbc)
> +		return;
> +
> +	xhci_dbc_eps_exit(xhci);
> +
> +	if (dbc->string) {
> +		dbc_dma_free_coherent(xhci,
> +				      dbc->string_size,
> +				      dbc->string, dbc->string_dma);
> +		dbc->string = NULL;
> +	}
> +
> +	xhci_free_container_ctx(xhci, dbc->ctx);
> +	dbc->ctx = NULL;
> +
> +	xhci_free_erst(xhci, &dbc->erst);
> +	xhci_ring_free(xhci, dbc->ring_out);
> +	xhci_ring_free(xhci, dbc->ring_in);
> +	xhci_ring_free(xhci, dbc->ring_evt);
> +	dbc->ring_in = NULL;
> +	dbc->ring_out = NULL;
> +	dbc->ring_evt = NULL;
> +}
> +
> +static void xhci_dbc_reset_debug_port(struct xhci_hcd *xhci)
> +{
> +	u32			val;
> +	unsigned long		flags;
> +	int			port_index;
> +	__le32 __iomem		**port_array;
> +
> +	spin_lock_irqsave(&xhci->lock, flags);
> +
> +	port_index = xhci->num_usb3_ports;
> +	port_array = xhci->usb3_ports;
> +	if (port_index <= 0) {
> +		spin_unlock_irqrestore(&xhci->lock, flags);
> +		return;
> +	}

we could take the spin lock here, and avoid the above unlock case.

>+
> +	while (port_index--) {
> +		val = readl(port_array[port_index]);
> +		if (!(val & PORT_CONNECT))
> +			writel(val | PORT_RESET, port_array[port_index]);
> +	}

This resets every USB3 port that does not have a device connected.
Shouldn't the debug port be the first USB3 port in the root hub, or do I remeber wrong?
That should the be xhci->usb3_ports[0]. Can't we reset just that one?

> +
> +	spin_unlock_irqrestore(&xhci->lock, flags);
> +}
> +
> +static int xhci_do_dbc_start(struct xhci_hcd *xhci)
> +{
> +	int			ret;
> +	u32			ctrl;
> +	struct xhci_dbc		*dbc = xhci->dbc;
> +
> +	if (dbc->state != DS_DISABLED)
> +		return -EINVAL;
> +
> +	writel(0, &dbc->regs->control);
> +	ret = xhci_handshake(&dbc->regs->control,
> +			     DBC_CTRL_DBC_ENABLE,
> +			     0, 1000);
> +	if (ret)
> +		return ret;
> +
> +	ret = xhci_dbc_mem_init(xhci, GFP_ATOMIC);
> +	if (ret)
> +		return ret;
> +
> +	ctrl = readl(&dbc->regs->control);
> +	writel(ctrl | DBC_CTRL_DBC_ENABLE | DBC_CTRL_PORT_ENABLE,
> +	       &dbc->regs->control);
> +	ret = xhci_handshake(&dbc->regs->control,
> +			     DBC_CTRL_DBC_ENABLE,
> +			     DBC_CTRL_DBC_ENABLE, 1000);
> +	if (ret)
> +		return ret;
> +
> +	xhci_dbc_reset_debug_port(xhci);
> +	dbc->state = DS_ENABLED;
> +
> +	return 0;
> +}
> +
> +static void xhci_do_dbc_stop(struct xhci_hcd *xhci)
> +{
> +	struct xhci_dbc		*dbc = xhci->dbc;
> +
> +	if (dbc->state == DS_DISABLED)
> +		return;
> +
> +	writel(0, &dbc->regs->control);
> +	xhci_dbc_mem_cleanup(xhci);
> +	dbc->state = DS_DISABLED;
> +}
> +
> +static int xhci_dbc_start(struct xhci_hcd *xhci)
> +{
> +	int			ret;
> +	struct xhci_dbc		*dbc = xhci->dbc;
> +
> +	WARN_ON(!dbc);
> +
> +	spin_lock(&dbc->lock);

Maybe call the pm_runtime_get_sync() already here to prevent runtime pm from
stopping the controller while starting dbc

we then need to pm_runtme_put it do_dbc_start() fails

> +	ret = xhci_do_dbc_start(xhci);
> +	spin_unlock(&dbc->lock);
> +
> +	if (!ret) {
> +		pm_runtime_get_sync(xhci_to_hcd(xhci)->self.controller);
> +		ret = mod_delayed_work(system_wq, &dbc->event_work, 1);
> +	}
> +
> +	return ret;
> +}
> +
> +static void xhci_dbc_stop(struct xhci_hcd *xhci)
> +{
> +	struct xhci_dbc		*dbc = xhci->dbc;
> +	struct dbc_port		*port = &dbc->port;
> +
> +	WARN_ON(!dbc);
> +
> +	cancel_delayed_work_sync(&dbc->event_work);
> +
> +	if (port->registered)
> +		xhci_dbc_tty_unregister_device(xhci);
> +
> +	spin_lock(&dbc->lock);
> +	xhci_do_dbc_stop(xhci);
> +	spin_unlock(&dbc->lock);
> +
> +	pm_runtime_put_sync(xhci_to_hcd(xhci)->self.controller);
> +}
> +
> +static void
> +dbc_handle_port_status(struct xhci_hcd *xhci, union xhci_trb *event)
> +{
> +	u32			portsc;
> +	struct xhci_dbc		*dbc = xhci->dbc;
> +
> +	portsc = readl(&dbc->regs->portsc);
> +	if (portsc & DBC_PORTSC_CONN_CHANGE)
> +		xhci_info(xhci, "DbC port connect change\n");
> +
> +	if (portsc & DBC_PORTSC_RESET_CHANGE)
> +		xhci_info(xhci, "DbC port reset change\n");
> +
> +	if (portsc & DBC_PORTSC_LINK_CHANGE)
> +		xhci_info(xhci, "DbC port link status change\n");
> +
> +	if (portsc & DBC_PORTSC_CONFIG_CHANGE)
> +		xhci_info(xhci, "DbC config error change\n");
> +

Are these messages info or debug?
If these messages are the only messages seen when connecting a
debug host (and not constantly spamming dmesg) then they are ok.

A bit like usb core annoncing "new usb device detected"

> +	/* Port reset change bit will be cleared in other place: */
> +	writel(portsc & ~DBC_PORTSC_RESET_CHANGE, &dbc->regs->portsc);
> +}
> +
> +static void dbc_handle_tx_event(struct xhci_hcd *xhci, union xhci_trb *event)

Ah, here you have the opportunity to properly name this function handle_xfer_event() or
handle_transfer_event().

The handle_tx_event() we use in xhci is confusing because of the tx/rx association.

> +{
> +	struct dbc_ep		*dep;
> +	struct xhci_ring	*ring;
> +	int			ep_id;
> +	int			status;
> +	u32			comp_code;
> +	size_t			remain_length;
> +	struct dbc_request	*req = NULL, *r;
> +
> +	comp_code	= GET_COMP_CODE(le32_to_cpu(event->generic.field[2]));
> +	remain_length	= EVENT_TRB_LEN(le32_to_cpu(event->generic.field[2]));
> +	ep_id		= TRB_TO_EP_ID(le32_to_cpu(event->generic.field[3]));
> +	dep		= (ep_id == EPID_OUT) ?
> +				get_out_ep(xhci) : get_in_ep(xhci);
> +	ring		= dep->ring;
> +
> +	switch (comp_code) {
> +	case COMP_SUCCESS:
> +		remain_length = 0;
> +	/* FALLTHROUGH */
> +	case COMP_SHORT_PACKET:
> +		status = 0;
> +		break;
> +	case COMP_TRB_ERROR:
> +	case COMP_BABBLE_DETECTED_ERROR:
> +	case COMP_USB_TRANSACTION_ERROR:
> +	case COMP_STALL_ERROR:
> +		xhci_warn(xhci, "tx error %d detected\n", comp_code);
> +		status = -comp_code;
> +		break;
> +	default:
> +		xhci_err(xhci, "unknown tx error %d\n", comp_code);
> +		status = -comp_code;
> +		break;
> +	}
> +
> +	/* Match the pending request: */
> +	list_for_each_entry(r, &dep->list_pending, list_pending) {
> +		if (r->trb_dma == event->trans_event.buffer) {
> +			req = r;
> +			break;
> +		}
> +	}

I like that it matches the request even if they are out of order.

Just out of curiosiry, was there some issue with transfer event not always
matching the next entry in dep->list_pending ?

> +
> +	if (!req) {
> +		xhci_warn(xhci, "no matched request\n");
> +		return;
> +	}
> +
> +	trace_xhci_dbc_handle_transfer(ring, &req->trb->generic);
> +
> +	ring->num_trbs_free++;
> +	req->actual = req->length - remain_length;
> +	xhci_dbc_giveback(req, status);
> +}
> +
> +static enum evtreturn xhci_dbc_do_handle_events(struct xhci_dbc *dbc)
> +{
> +	dma_addr_t		deq;
> +	struct dbc_ep		*dep;
> +	union xhci_trb		*evt;
> +	u32			ctrl, portsc;
> +	struct xhci_hcd		*xhci = dbc->xhci;
> +	bool			update_erdp = false;
> +
> +	/* DbC state machine: */
> +	switch (dbc->state) {
> +	case DS_DISABLED:
> +	case DS_INITIALIZED:
> +
> +		return EVT_ERR;
> +	case DS_ENABLED:
> +		portsc = readl(&dbc->regs->portsc);
> +		if (portsc & DBC_PORTSC_CONN_STATUS) {
> +			dbc->state = DS_CONNECTED;
> +			xhci_info(xhci, "DbC connected\n");
> +		}
> +
> +		return EVT_DONE;
> +	case DS_CONNECTED:
> +		ctrl = readl(&dbc->regs->control);
> +		if (ctrl & DBC_CTRL_DBC_RUN) {
> +			dbc->state = DS_CONFIGURED;
> +			xhci_info(xhci, "DbC configured\n");
> +			portsc = readl(&dbc->regs->portsc);
> +			writel(portsc, &dbc->regs->portsc);
> +			return EVT_GSER;
> +		}
> +
> +		return EVT_DONE;
> +	case DS_CONFIGURED:
> +		/* Handle cable unplug event: */
> +		portsc = readl(&dbc->regs->portsc);
> +		if (!(portsc & DBC_PORTSC_PORT_ENABLED) &&
> +		    !(portsc & DBC_PORTSC_CONN_STATUS)) {
> +			xhci_info(xhci, "DbC cable unplugged\n");
> +			dbc->state = DS_ENABLED;
> +			xhci_dbc_flush_reqests(dbc);
> +
> +			return EVT_DISC;
> +		}
> +
> +		/* Handle debug port reset event: */
> +		if (portsc & DBC_PORTSC_RESET_CHANGE) {
> +			xhci_info(xhci, "DbC port reset\n");
> +			writel(portsc, &dbc->regs->portsc);
> +			dbc->state = DS_ENABLED;
> +			xhci_dbc_flush_reqests(dbc);
> +
> +			return EVT_DISC;
> +		}
> +
> +		/* Handle endpoint stall event: */
> +		ctrl = readl(&dbc->regs->control);
> +		if ((ctrl & DBC_CTRL_HALT_IN_TR) ||
> +		    (ctrl & DBC_CTRL_HALT_OUT_TR)) {
> +			xhci_info(xhci, "DbC Endpoint stall\n");
> +			dbc->state = DS_STALLED;
> +
> +			if (ctrl & DBC_CTRL_HALT_IN_TR) {
> +				dep = get_in_ep(xhci);
> +				xhci_dbc_flush_endpoint_requests(dep);
> +			}
> +
> +			if (ctrl & DBC_CTRL_HALT_OUT_TR) {
> +				dep = get_out_ep(xhci);
> +				xhci_dbc_flush_endpoint_requests(dep);
> +			}
> +
> +			return EVT_DONE;
> +		}
> +
> +		/* Clear DbC run change bit: */
> +		if (ctrl & DBC_CTRL_DBC_RUN_CHANGE) {
> +			writel(ctrl, &dbc->regs->control);
> +			ctrl = readl(&dbc->regs->control);
> +		}
> +
> +		break;
> +	case DS_STALLED:
> +		ctrl = readl(&dbc->regs->control);
> +		if (!(ctrl & DBC_CTRL_HALT_IN_TR) &&
> +		    !(ctrl & DBC_CTRL_HALT_OUT_TR) &&
> +		    (ctrl & DBC_CTRL_DBC_RUN)) {
> +			dbc->state = DS_CONFIGURED;
> +			break;
> +		}
> +
> +		return EVT_DONE;
> +	default:
> +		xhci_err(xhci, "Unknown DbC state %d\n", dbc->state);
> +		break;
> +	}
> +
> +	/* Handle the events in the event ring: */
> +	evt = dbc->ring_evt->dequeue;
> +	while ((le32_to_cpu(evt->event_cmd.flags) & TRB_CYCLE) ==
> +			dbc->ring_evt->cycle_state) {
> +		/*
> +		 * Add a barrier between reading the cycle flag and any
> +		 * reads of the event's flags/data below:
> +		 */
> +		rmb();
> +
> +		trace_xhci_dbc_handle_event(dbc->ring_evt, &evt->generic);
> +
> +		switch (le32_to_cpu(evt->event_cmd.flags) & TRB_TYPE_BITMASK) {
> +		case TRB_TYPE(TRB_PORT_STATUS):
> +			dbc_handle_port_status(xhci, evt);
> +			break;
> +		case TRB_TYPE(TRB_TRANSFER):
> +			dbc_handle_tx_event(xhci, evt);
> +			break;
> +		default:
> +			break;
> +		}
> +
> +		inc_deq(xhci, dbc->ring_evt);
> +		evt = dbc->ring_evt->dequeue;
> +		update_erdp = true;
> +	}
> +
> +	/* Update event ring dequeue pointer: */
> +	if (update_erdp) {
> +		deq = xhci_trb_virt_to_dma(dbc->ring_evt->deq_seg,
> +					   dbc->ring_evt->dequeue);
> +		xhci_write_64(xhci, deq, &dbc->regs->erdp);
> +	}
> +
> +	return EVT_DONE;
> +}
> +
> +static void xhci_dbc_handle_events(struct work_struct *work)
> +{
> +	int			ret;
> +	enum evtreturn		evtr;
> +	struct xhci_dbc		*dbc;
> +	struct xhci_hcd		*xhci;
> +
> +	dbc = container_of(to_delayed_work(work), struct xhci_dbc, event_work);
> +	xhci = dbc->xhci;
> +
> +	spin_lock(&dbc->lock);
> +	evtr = xhci_dbc_do_handle_events(dbc);
> +	spin_unlock(&dbc->lock);
> +
> +	switch (evtr) {
> +	case EVT_GSER:
> +		ret = xhci_dbc_tty_register_device(xhci);
> +		if (ret) {
> +			xhci_err(xhci, "failed to alloc tty device\n");
> +			break;
> +		}
> +
> +		xhci_info(xhci, "DbC now attached to /dev/ttyDBC0\n");
> +		mod_delayed_work(system_wq, &dbc->event_work, 1);
> +		break;
> +	case EVT_DISC:
> +		xhci_dbc_tty_unregister_device(xhci);
> +		mod_delayed_work(system_wq, &dbc->event_work, 1);
> +		break;
> +	case EVT_DONE:
> +		mod_delayed_work(system_wq, &dbc->event_work, 1);
> +		break;
> +	default:
> +		xhci_info(xhci, "stop handling dbc events\n");

	return; here

> +	}

mod_delayed_work(); here, and remove it from above cases.

> +}
> +
> +static void xhci_dbc_exit(struct xhci_hcd *xhci)
> +{
> +	unsigned long		flags;
> +
> +	spin_lock_irqsave(&xhci->lock, flags);
> +	kfree(xhci->dbc);
> +	xhci->dbc = NULL;
> +	spin_unlock_irqrestore(&xhci->lock, flags);
> +}
> +
> +static int xhci_dbc_init(struct xhci_hcd *xhci)
> +{
> +	u32			reg;
> +	struct xhci_dbc		*dbc;
> +	unsigned long		flags;
> +	void __iomem		*base;
> +	int			dbc_cap_offs;
> +
> +	base = &xhci->cap_regs->hc_capbase;
> +	dbc_cap_offs = xhci_find_next_ext_cap(base, 0, XHCI_EXT_CAPS_DEBUG);
> +	if (!dbc_cap_offs)
> +		return -ENODEV;
> +
> +	dbc = kzalloc(sizeof(*dbc), GFP_KERNEL);
> +	if (!dbc)
> +		return -ENOMEM;
> +
> +	dbc->regs = base + dbc_cap_offs;
> +
> +	/* We will avoid using DbC in xhci driver if it's in use. */
> +	reg = readl(&dbc->regs->control);
> +	if (reg & DBC_CTRL_DBC_ENABLE) {
> +		kfree(dbc);
> +		return -EBUSY;
> +	}
> +
> +	spin_lock_irqsave(&xhci->lock, flags);
> +	if (xhci->dbc) {
> +		spin_unlock_irqrestore(&xhci->lock, flags);
> +		kfree(dbc);
> +		return -EBUSY;
> +	}
> +	xhci->dbc = dbc;
> +	spin_unlock_irqrestore(&xhci->lock, flags);
> +
> +	dbc->xhci = xhci;
> +	INIT_DELAYED_WORK(&dbc->event_work, xhci_dbc_handle_events);
> +	spin_lock_init(&dbc->lock);
> +
> +	return 0;
> +}
> +
> +static ssize_t dbc_show(struct device *dev,
> +			struct device_attribute *attr,
> +			char *buf)
> +{
> +	const char		*p;
> +	struct xhci_dbc		*dbc;
> +	struct xhci_hcd		*xhci;
> +
> +	xhci = hcd_to_xhci(dev_get_drvdata(dev));
> +	dbc = xhci->dbc;
> +
> +	switch (dbc->state) {
> +	case DS_DISABLED:
> +		p = "disabled";
> +		break;
> +	case DS_INITIALIZED:
> +		p = "initialized";
> +		break;
> +	case DS_ENABLED:
> +		p = "enabled";
> +		break;
> +	case DS_CONNECTED:
> +		p = "connected";
> +		break;
> +	case DS_CONFIGURED:
> +		p = "configured";
> +		break;
> +	case DS_STALLED:
> +		p = "stalled";
> +		break;
> +	default:
> +		p = "unknown";
> +	}
> +
> +	return sprintf(buf, "%s\n", p);
> +}
> +
> +static ssize_t dbc_store(struct device *dev,
> +			 struct device_attribute *attr,
> +			 const char *buf, size_t count)
> +{
> +	struct xhci_dbc		*dbc;
> +	struct xhci_hcd		*xhci;
> +
> +	xhci = hcd_to_xhci(dev_get_drvdata(dev));
> +	dbc = xhci->dbc;
> +
> +	if (!strncmp(buf, "enable", 6))
> +		xhci_dbc_start(xhci);
> +	else if (!strncmp(buf, "disable", 7))
> +		xhci_dbc_stop(xhci);
> +	else
> +		return -EINVAL;
> +
> +	return count;
> +}
> +
> +static DEVICE_ATTR(dbc, 0644, dbc_show, dbc_store);
> +
> +int dbc_create_sysfs_file(struct xhci_hcd *xhci)
> +{
> +	int			ret;
> +	struct device		*dev = xhci_to_hcd(xhci)->self.controller;
> +
> +	ret = xhci_dbc_init(xhci);
> +	if (ret)
> +		return ret;
> +
> +	xhci_dbc_tty_register_driver(xhci);

The function name "dbc_create_sysfs_file()" doesn't reveal anything about
that is also registers the tty driver

> +
> +	return device_create_file(dev, &dev_attr_dbc);
> +}
> +
> +void dbc_remove_sysfs_file(struct xhci_hcd *xhci)
> +{
> +	struct device		*dev = xhci_to_hcd(xhci)->self.controller;
> +
> +	device_remove_file(dev, &dev_attr_dbc);
> +	xhci_dbc_tty_unregister_driver();
> +	xhci_dbc_stop(xhci);
> +	xhci_dbc_exit(xhci);
> +}
> +
> +#ifdef CONFIG_PM
> +int xhci_dbc_suspend(struct xhci_hcd *xhci)
> +{
> +	struct xhci_dbc		*dbc = xhci->dbc;
> +
> +	if (!dbc)
> +		return 0;
> +
> +	if (dbc->state == DS_CONFIGURED)
> +		dbc->resume_required = 1;
> +
> +	xhci_dbc_stop(xhci);
> +
> +	return 0;
> +}
> +
> +int xhci_dbc_resume(struct xhci_hcd *xhci)
> +{
> +	int			ret = 0;
> +	struct xhci_dbc		*dbc = xhci->dbc;
> +
> +	if (!dbc)
> +		return 0;
> +
> +	if (dbc->resume_required) {
> +		dbc->resume_required = 0;
> +		xhci_dbc_start(xhci);
> +	}
> +
> +	return ret;

So far the dbc driver looks really good, it's getting late,
I need to reviewing the other files later.

I can't say much about the TTY parts as I never needed to work on those

-Mathias

^ permalink raw reply	[flat|nested] 21+ messages in thread

* Re: [PATCH v3 2/3] usb: xhci: Add DbC support in xHCI driver
  2017-10-24 17:03   ` Mathias Nyman
@ 2017-10-25  2:15     ` Lu Baolu
  0 siblings, 0 replies; 21+ messages in thread
From: Lu Baolu @ 2017-10-25  2:15 UTC (permalink / raw)
  To: Mathias Nyman; +Cc: linux-kernel

Hi Mathias,

Thanks for your time.

On 10/25/2017 01:03 AM, Mathias Nyman wrote:
> Hi
>
> Skipping lists, partial review.
> Many of the questions are real questions that I was wondering about
> while lookinga the code. Not necessarily thing that must be changed.

No problem.

>
> On 05.09.2017 04:58, Lu Baolu wrote:
>> xHCI compatible USB host controllers(i.e. super-speed USB3 controllers)
>> can be implemented with the Debug Capability(DbC). It presents a debug
>> device which is fully compliant with the USB framework and provides the
>> equivalent of a very high performance full-duplex serial link. The debug
>> capability operation model and registers interface are defined in 7.6.8
>> of the xHCI specification, revision 1.1.
>>
>> The DbC debug device shares a root port with the xHCI host. By default,
>> the debug capability is disabled and the root port is assigned to xHCI.
>> When the DbC is enabled, the root port will be assigned to the DbC debug
>> device, and the xHCI sees nothing on this port. This implementation uses
>> a sysfs node named <dbc> under the xHCI device to manage the enabling
>> and disabling of the debug capability.
>>
>> When the debug capability is enabled, it will present a debug device
>> through the debug port. This debug device is fully compliant with the
>> USB3 framework, and it can be enumerated by a debug host on the other
>> end of the USB link. As soon as the debug device is configured, a TTY
>> serial device named /dev/ttyDBC0 will be created.
>>
>> One use of this link is running a login service on the debug target.
>> Hence it can be remote accessed by a debug host. Another use case can
>> probably be found in servers. It provides a peer-to-peer USB link
>> between two host-only machines. This provides a reasonable out-of-band
>> communication method between two servers.
>>
>> Signed-off-by: Lu Baolu <baolu.lu@linux.intel.com>
>> ---
>>   .../ABI/testing/sysfs-bus-pci-drivers-xhci_hcd     |   25 +
>>   drivers/usb/host/Kconfig                           |    9 +
>>   drivers/usb/host/Makefile                          |    5 +
>>   drivers/usb/host/xhci-dbgcap.c                     | 1016 ++++++++++++++++++++
>>   drivers/usb/host/xhci-dbgcap.h                     |  247 +++++
>>   drivers/usb/host/xhci-dbgtty.c                     |  586 +++++++++++
>>   drivers/usb/host/xhci-trace.h                      |   60 ++
>>   drivers/usb/host/xhci.c                            |   10 +
>>   drivers/usb/host/xhci.h                            |    1 +
>>   9 files changed, 1959 insertions(+)
>>   create mode 100644 Documentation/ABI/testing/sysfs-bus-pci-drivers-xhci_hcd
>>   create mode 100644 drivers/usb/host/xhci-dbgcap.c
>>   create mode 100644 drivers/usb/host/xhci-dbgcap.h
>>   create mode 100644 drivers/usb/host/xhci-dbgtty.c
>>
>> diff --git a/Documentation/ABI/testing/sysfs-bus-pci-drivers-xhci_hcd b/Documentation/ABI/testing/sysfs-bus-pci-drivers-xhci_hcd
>> new file mode 100644
>> index 0000000..0088aba
>> --- /dev/null
>> +++ b/Documentation/ABI/testing/sysfs-bus-pci-drivers-xhci_hcd
>> @@ -0,0 +1,25 @@
>> +What:        /sys/bus/pci/drivers/xhci_hcd/.../dbc
>> +Date:        June 2017
>> +Contact:    Lu Baolu <baolu.lu@linux.intel.com>
>> +Description:
>> +        xHCI compatible USB host controllers (i.e. super-speed
>> +        USB3 controllers) are often implemented with the Debug
>> +        Capability (DbC). It can present a debug device which
>> +        is fully compliant with the USB framework and provides
>> +        the equivalent of a very high performance full-duplex
>> +        serial link for debug purpose.
>> +
>> +        The DbC debug device shares a root port with xHCI host.
>> +        When the DbC is enabled, the root port will be assigned
>> +        to the Debug Capability. Otherwise, it will be assigned
>> +        to xHCI.
>> +
>> +        Writing "enable" to this attribute will enable the DbC
>> +        functionality and the shared root port will be assigned
>> +        to the DbC device. Writing "disable" to this attribute
>> +        will disable the DbC functionality and the shared root
>> +        port will roll back to the xHCI.
>> +
>> +        Reading this attribute gives the state of the DbC. It
>> +        can be one of the following states: disabled, enabled,
>> +        initialized, connected, configured and stalled.
>> diff --git a/drivers/usb/host/Kconfig b/drivers/usb/host/Kconfig
>> index fa5692d..968a196 100644
>> --- a/drivers/usb/host/Kconfig
>> +++ b/drivers/usb/host/Kconfig
>> @@ -27,6 +27,15 @@ config USB_XHCI_HCD
>>         module will be called xhci-hcd.
>>
>>   if USB_XHCI_HCD
>> +config USB_XHCI_DBGCAP
>> +    bool "xHCI support for debug capability"
>> +    depends on TTY
>> +    select USB_U_SERIAL
>> +    ---help---
>> +      Say 'Y' to enable the support for the xHCI debug capability. Make
>> +      sure that your xHCI host supports the extended debug capability and
>> +      you want a TTY serial device based on the xHCI debug capability
>> +      before enabling this option. If unsure, say 'N'.
>>
>>   config USB_XHCI_PCI
>>          tristate
>> diff --git a/drivers/usb/host/Makefile b/drivers/usb/host/Makefile
>> index cf2691f..175c80a 100644
>> --- a/drivers/usb/host/Makefile
>> +++ b/drivers/usb/host/Makefile
>> @@ -13,6 +13,11 @@ fhci-$(CONFIG_FHCI_DEBUG) += fhci-dbg.o
>>   xhci-hcd-y := xhci.o xhci-mem.o
>>   xhci-hcd-y += xhci-ring.o xhci-hub.o xhci-dbg.o
>>   xhci-hcd-y += xhci-trace.o
>> +
>> +ifneq ($(CONFIG_USB_XHCI_DBGCAP), )
>> +    xhci-hcd-y += xhci-dbgcap.o xhci-dbgtty.o
>> +endif
>> +
>>   ifneq ($(CONFIG_USB_XHCI_MTK), )
>>       xhci-hcd-y += xhci-mtk-sch.o
>>   endif
>> diff --git a/drivers/usb/host/xhci-dbgcap.c b/drivers/usb/host/xhci-dbgcap.c
>> new file mode 100644
>> index 0000000..9816085
>> --- /dev/null
>> +++ b/drivers/usb/host/xhci-dbgcap.c
>> @@ -0,0 +1,1016 @@
>> +/**
>> + * xhci-dbgcap.c - xHCI debug capability support
>> + *
>> + * Copyright (C) 2017 Intel Corporation
>> + *
>> + * Author: Lu Baolu <baolu.lu@linux.intel.com>
>> + *
>> + * This program is free software; you can redistribute it and/or modify
>> + * it under the terms of the GNU General Public License version 2 as
>> + * published by the Free Software Foundation.
>> + */
>> +#include <linux/dma-mapping.h>
>> +#include <linux/slab.h>
>> +
>> +#include "xhci.h"
>> +#include "xhci-trace.h"
>> +#include "xhci-dbgcap.h"
>> +
>> +static inline void *
>> +dbc_dma_alloc_coherent(struct xhci_hcd *xhci, size_t size,
>> +               dma_addr_t *dma_handle, gfp_t flags)
>> +{
>> +    void        *vaddr;
>> +
>> +    vaddr = dma_alloc_coherent(xhci_to_hcd(xhci)->self.sysdev,
>> +                   size, dma_handle, flags);
>> +    memset(vaddr, 0, size);
>> +    return vaddr;
>> +}
>> +
>> +static inline void
>> +dbc_dma_free_coherent(struct xhci_hcd *xhci, size_t size,
>> +              void *cpu_addr, dma_addr_t dma_handle)
>> +{
>> +    if (cpu_addr)
>> +        dma_free_coherent(xhci_to_hcd(xhci)->self.sysdev,
>> +                  size, cpu_addr, dma_handle);
>> +}
>> +
>
> Is there any benefit in having these dbc wrapped variants
> of dma alloc/free coherent functions?
> Can't dma_zalloc_coherent() and dma_free_coherent() be used directly?

The benefit of this wrapper is to zero out the dma space during allocation.

>
>> +static inline void xhci_put_utf16(u16 *s, const char *c)
>> +{
>> +    int        i;
>> +    size_t        size;
>> +
>> +    size = strlen(c);
>> +    for (i = 0; i < size; i++)
>> +        s[i] = cpu_to_le16(c[i]);
>> +}
>> +
>
> could utf8s_to_utf16s() be used instead?
> At least drivers/usb/misc/usb251xb.c is using it to set some string descriptors.

Good suggestion. I will try this in code.

>
>> +static u32 xhci_dbc_populate_strings(struct dbc_strings *strings)
>> +{
>> +    struct usb_string_descriptor    *s_desc;
>> +    u32                string_length;
>> +
>> +    /* Serial string: */
>> +    s_desc = (struct usb_string_descriptor *)strings->serial;
>
> Took me a second to understand that dbc_strings is not just strings, but char
> arrays where we store entire string descriptors with length, type and UTF16LE string.
>
> small thing, but maybe it structure could be called struct dbc_str_descs or similar

Yes, dbc_str_descs is easier for understanding.

>
>> +    xhci_put_utf16(s_desc->wData, DBC_STRING_SERIAL);
>> +
>> +    s_desc->bLength        = (strlen(DBC_STRING_SERIAL) + 1) * 2;
>> +    s_desc->bDescriptorType    = USB_DT_STRING;
>> +    string_length        = s_desc->bLength;
>> +    string_length        <<= 8;
>> +
>> +    /* Product string: */
>> +    s_desc = (struct usb_string_descriptor *)strings->product;
>> +    xhci_put_utf16(s_desc->wData, DBC_STRING_PRODUCT);
>> +
>> +    s_desc->bLength        = (strlen(DBC_STRING_PRODUCT) + 1) * 2;
>> +    s_desc->bDescriptorType    = USB_DT_STRING;
>> +    string_length        += s_desc->bLength;
>> +    string_length        <<= 8;
>> +
>> +    /* Manufacture string: */
>> +    s_desc = (struct usb_string_descriptor *)strings->manufacturer;
>> +    xhci_put_utf16(s_desc->wData, DBC_STRING_MANUFACTURER);
>> +
>> +    s_desc->bLength        = (strlen(DBC_STRING_MANUFACTURER) + 1) * 2;
>> +    s_desc->bDescriptorType    = USB_DT_STRING;
>> +    string_length        += s_desc->bLength;
>> +    string_length        <<= 8;
>> +
>> +    /* String0: */
>> +    strings->string0[0]    = 4;
>> +    strings->string0[1]    = USB_DT_STRING;
>> +    strings->string0[2]    = 0x09;
>> +    strings->string0[3]    = 0x04;
>> +    string_length        += 4;
>> +
>> +    return string_length;
>> +}
>> +
>> +static void xhci_dbc_populate_contexts(struct xhci_hcd *xhci, u32 string_length)
>> +{
>> +    struct xhci_dbc        *dbc;
>> +    struct dbc_info_context    *info;
>> +    struct xhci_ep_ctx    *ep_ctx;
>> +    u32            dev_info;
>> +    dma_addr_t        deq, dma;
>> +    unsigned int        max_burst;
>> +
>> +    dbc = xhci->dbc;
>> +    if (!dbc)
>> +        return;
>> +
>> +    /* Populate info Context: */
>> +    info            = (struct dbc_info_context *)dbc->ctx->bytes;
>> +    dma            = dbc->string_dma;
>> +    info->string0        = cpu_to_le64(dma);
>> +    info->manufacturer    = cpu_to_le64(dma + DBC_MAX_STRING_LENGTH);
>> +    info->product        = cpu_to_le64(dma + DBC_MAX_STRING_LENGTH * 2);
>> +    info->serial        = cpu_to_le64(dma + DBC_MAX_STRING_LENGTH * 3);
>> +    info->length        = cpu_to_le32(string_length);
>> +
>> +    /* Populate bulk out endpoint context: */
>> +    ep_ctx            = dbc_bulkout_ctx(dbc);
>> +    max_burst        = DBC_CTRL_MAXBURST(readl(&dbc->regs->control));
>> +    deq            = dbc_bulkout_enq(dbc);
>> +    ep_ctx->ep_info        = 0;
>> +    ep_ctx->ep_info2    = dbc_epctx_info2(BULK_OUT_EP, 1024, max_burst);
>> +    ep_ctx->deq        = cpu_to_le64(deq | dbc->ring_out->cycle_state);
>> +
>> +    /* Populate bulk in endpoint context: */
>> +    ep_ctx            = dbc_bulkin_ctx(dbc);
>> +    deq            = dbc_bulkin_enq(dbc);
>> +    ep_ctx->ep_info        = 0;
>> +    ep_ctx->ep_info2    = dbc_epctx_info2(BULK_IN_EP, 1024, max_burst);
>> +    ep_ctx->deq        = cpu_to_le64(deq | dbc->ring_in->cycle_state);
>
> Are we setting the ep_ctx->deq to ring deq instead of
> segment start because we are going to call this runtime/mid tranfer?

Ah, that's my fault. I should set it to the segment start although they
have the same values.

This function can't be called during DbC runtime. DbC must be stopped
and reinitialized before re-enabling it.

>
>> +
>> +    /* Set DbC context and info registers: */
>> +    xhci_write_64(xhci, dbc->ctx->dma, &dbc->regs->dccp);
>> +
>> +    dev_info = cpu_to_le32((DBC_VENDOR_ID << 16) | DBC_PROTOCOL);
>> +    writel(dev_info, &dbc->regs->devinfo1);
>> +
>> +    dev_info = cpu_to_le32((DBC_DEVICE_REV << 16) | DBC_PRODUCT_ID);
>> +    writel(dev_info, &dbc->regs->devinfo2);
>
> Minor thing again but this function initializes a couple registers in addition
> to just populating the contexts. Maybe split or rename function

Okay, will clean it.

>
>> +}
>> +
>> +static void xhci_dbc_giveback(struct dbc_request *req, int status)
>> +    __releases(&dbc->lock)
>> +    __acquires(&dbc->lock)
>> +{
>> +    struct dbc_ep        *dep = req->dep;
>> +    struct xhci_dbc        *dbc = dep->dbc;
>> +    struct xhci_hcd        *xhci = dbc->xhci;
>> +    struct device        *dev = xhci_to_hcd(dbc->xhci)->self.sysdev;
>> +
>> +    trace_xhci_dbc_giveback_request(req);
>> +
>> +    list_del_init(&req->list_pending);
>
> why do we need to reinitialize the entry, is this entry reused
> or accessed after later afetr this. would list_del() work?

The requests are preallocated in a pool, so they can be reused.

>
>> +    req->trb_dma = 0;
>> +    req->trb = NULL;
>> +
>> +    if (req->status == -EINPROGRESS)
>> +        req->status = status;
>> +
>> +    dma_unmap_single(dev,
>> +             req->dma,
>> +             req->length,
>> +             dbc_ep_dma_direction(dep));
>> +
>> +    /* Give back the transfer request: */
>> +    spin_unlock(&dbc->lock);
>> +    req->complete(xhci, req);
>> +    spin_lock(&dbc->lock);
>> +}
>> +
>> +static void xhci_dbc_flush_single_request(struct dbc_request *req)
>> +{
>> +    union xhci_trb    *trb = req->trb;
>> +
>> +    trb->generic.field[0]    = 0;
>> +    trb->generic.field[1]    = 0;
>> +    trb->generic.field[2]    = 0;
>> +    trb->generic.field[3]    &= cpu_to_le32(TRB_CYCLE);
>> +    trb->generic.field[3]    |= cpu_to_le32(TRB_TYPE(TRB_TR_NOOP));
>> +
>> +    xhci_dbc_giveback(req, -ESHUTDOWN);
>> +}
>> +
>> +static void xhci_dbc_flush_endpoint_requests(struct dbc_ep *dep)
>> +{
>> +    struct dbc_request    *req, *tmp;
>> +
>> +    list_for_each_entry_safe(req, tmp, &dep->list_pending, list_pending)
>> +        xhci_dbc_flush_single_request(req);
>> +}
>> +
>> +static void xhci_dbc_flush_reqests(struct xhci_dbc *dbc)
>> +{
>> +    int            i;
>> +    struct dbc_ep        *dep;
>> +
>> +    for (i = BULK_OUT; i <= BULK_IN; i++) {
>> +        dep = &dbc->eps[i];
>> +        xhci_dbc_flush_endpoint_requests(dep);
>> +    }
>
> we only have one in and one out endpiont, how about just
>     xhci_dbc_flush_endpoint_requests(&dbc->eps[BULK_OUT]);
>     xhci_dbc_flush_endpoint_requests(&dbc->eps[BULK_IN]);

Yes, will clean it.

>
>> +}
>> +
>> +static struct dbc_request *
>> +dbc_ep_alloc_request(struct dbc_ep *dep, gfp_t gfp_flags)
>> +{
>> +    struct dbc_request    *req;
>> +
>> +    req = kzalloc(sizeof(*req), gfp_flags);
>> +    if (!req)
>> +        return NULL;
>> +
>> +    req->dep = dep;
>> +    INIT_LIST_HEAD(&req->list_pending);
>> +    INIT_LIST_HEAD(&req->list_pool);
>> +
>> +    trace_xhci_dbc_alloc_request(req);
>> +
>> +    return req;
>> +}
>> +
>> +static void
>> +dbc_ep_free_request(struct dbc_ep *dep, struct dbc_request *req)
>> +{
>> +    trace_xhci_dbc_free_request(req);
>> +
>> +    kfree(req);
>> +}
>> +
>> +static void
>> +xhci_dbc_queue_trb(struct xhci_ring *ring, u32 field1,
>> +           u32 field2, u32 field3, u32 field4)
>> +{
>> +    union xhci_trb        *trb, *next;
>> +
>> +    trb = ring->enqueue;
>> +    trb->generic.field[0]    = cpu_to_le32(field1);
>> +    trb->generic.field[1]    = cpu_to_le32(field2);
>> +    trb->generic.field[2]    = cpu_to_le32(field3);
>> +    trb->generic.field[3]    = cpu_to_le32(field4);
>> +
>> +    trace_xhci_dbc_gadget_ep_queue(ring, &trb->generic);
>> +
>> +    ring->num_trbs_free--;
>> +    next = ++(ring->enqueue);
>> +    if (TRB_TYPE_LINK_LE32(next->link.control)) {
>> +        next->link.control ^= cpu_to_le32(TRB_CYCLE);
>> +        ring->enqueue = ring->enq_seg->trbs;
>> +        ring->cycle_state ^= 1;
>> +    }
>> +}
>> +
>> +static int xhci_dbc_queue_bulk_tx(struct dbc_ep *dep,
>> +                  struct dbc_request *req)
>> +{
>> +    u64            addr;
>> +    union xhci_trb        *trb;
>> +    unsigned int        num_trbs;
>> +    struct xhci_dbc        *dbc = dep->dbc;
>> +    struct xhci_ring    *ring = dep->ring;
>> +    u32            length, control, cycle;
>> +
>> +    num_trbs = count_trbs(req->dma, req->length);
>> +    WARN_ON(num_trbs != 1);
>> +    if (ring->num_trbs_free < num_trbs)
>> +        return -EBUSY;
>> +
>> +    addr    = req->dma;
>> +    trb    = ring->enqueue;
>> +    cycle    = ring->cycle_state;
>> +    length    = TRB_LEN(req->length);
>> +    control    = TRB_TYPE(TRB_NORMAL) | TRB_IOC;
>> +
>> +    if (cycle)
>> +        control &= cpu_to_le32(~TRB_CYCLE);
>> +    else
>> +        control |= cpu_to_le32(TRB_CYCLE);
>> +
>> +    req->trb = ring->enqueue;
>> +    req->trb_dma = xhci_trb_virt_to_dma(ring->enq_seg, ring->enqueue);
>> +    xhci_dbc_queue_trb(ring,
>> +               lower_32_bits(addr),
>> +               upper_32_bits(addr),
>> +               length, control);
>> +
>> +    /*
>> +     * Add a barrier between writes of trb fields and flipping
>> +     * the cycle bit:
>> +     */
>> +    wmb();
>> +
>> +    if (cycle)
>> +        trb->generic.field[3] |= cpu_to_le32(TRB_CYCLE);
>> +    else
>> +        trb->generic.field[3] &= cpu_to_le32(~TRB_CYCLE);
>> +
>> +    writel(DBC_DOOR_BELL_TARGET(dep->direction), &dbc->regs->doorbell);
>> +
>> +    return 0;
>> +}
>> +
>> +static int
>> +dbc_ep_do_queue(struct dbc_ep *dep, struct dbc_request *req)
>> +{
>> +    int            ret;
>> +    struct device        *dev;
>> +    struct xhci_dbc        *dbc = dep->dbc;
>> +    struct xhci_hcd        *xhci = dbc->xhci;
>> +
>> +    dev = xhci_to_hcd(xhci)->self.sysdev;
>> +
>> +    if (!req->length || !req->buf)
>> +        return -EINVAL;
>> +
>> +    req->actual        = 0;
>> +    req->status        = -EINPROGRESS;
>> +
>> +    req->dma = dma_map_single(dev,
>> +                  req->buf,
>> +                  req->length,
>> +                  dbc_ep_dma_direction(dep));
>> +    if (dma_mapping_error(dev, req->dma)) {
>> +        xhci_err(xhci, "failed to map buffer\n");
>> +        return -EFAULT;
>> +    }
>> +
>> +    ret = xhci_dbc_queue_bulk_tx(dep, req);
>> +    if (ret) {
>> +        xhci_err(xhci, "failed to queue trbs\n");
>> +        dma_unmap_single(dev,
>> +                 req->dma,
>> +                 req->length,
>> +                 dbc_ep_dma_direction(dep));
>> +        return -EFAULT;
>> +    }
>> +
>> +    list_add_tail(&req->list_pending, &dep->list_pending);
>> +
>> +    return 0;
>> +}
>> +
>> +static int dbc_ep_queue(struct dbc_ep *dep,
>> +            struct dbc_request *req,
>> +            gfp_t gfp_flags)
>> +{
>> +    struct xhci_dbc        *dbc = dep->dbc;
>> +    int            ret = -ESHUTDOWN;
>> +
>> +    spin_lock(&dbc->lock);
>> +    if (dbc->state == DS_CONFIGURED)
>> +        ret = dbc_ep_do_queue(dep, req);
>> +    spin_unlock(&dbc->lock);
>> +
>> +    trace_xhci_dbc_queue_request(req);
>> +
>> +    return ret;
>> +}
>> +
>> +static inline void xhci_dbc_do_eps_init(struct xhci_hcd *xhci, bool direction)
>> +{
>> +    struct dbc_ep        *dep;
>> +    struct xhci_dbc        *dbc = xhci->dbc;
>> +
>> +    dep            = &dbc->eps[direction];
>> +    dep->dbc        = dbc;
>> +    dep->direction        = direction;
>> +    dep->ring        = direction ? dbc->ring_in : dbc->ring_out;
>> +    dep->alloc_request    = dbc_ep_alloc_request;
>> +    dep->free_request    = dbc_ep_free_request;
>> +    dep->queue        = dbc_ep_queue;
>
> why do we need the alloc_request, free_request and queue function pointers in struct dbc_ep?
> They are always pointing to the same functions.

I did this in order to make DbC base driver and upper glue layer independent.
It's unnecessary now. I will re-factor this code to make it simple.

>
>> +
>> +    INIT_LIST_HEAD(&dep->list_pending);
>> +}
>> +
>> +static void xhci_dbc_eps_init(struct xhci_hcd *xhci)
>> +{
>> +    xhci_dbc_do_eps_init(xhci, BULK_OUT);
>> +    xhci_dbc_do_eps_init(xhci, BULK_IN);
>> +}
>> +
>> +static void xhci_dbc_eps_exit(struct xhci_hcd *xhci)
>> +{
>> +    struct xhci_dbc        *dbc = xhci->dbc;
>> +
>> +    memset(dbc->eps, 0, ARRAY_SIZE(dbc->eps));
>> +}
>> +
>> +static int xhci_dbc_mem_init(struct xhci_hcd *xhci, gfp_t flags)
>> +{
>> +    int            ret;
>> +    dma_addr_t        deq;
>> +    u32            string_length;
>> +    struct xhci_dbc        *dbc = xhci->dbc;
>> +
>> +    /* Allocate various rings for events and transfers: */
>> +    dbc->ring_evt = xhci_ring_alloc(xhci, 1, 1, TYPE_EVENT, 0, flags);
>> +    if (!dbc->ring_evt)
>> +        goto evt_fail;
>> +
>> +    dbc->ring_in = xhci_ring_alloc(xhci, 1, 1, TYPE_BULK, 0, flags);
>> +    if (!dbc->ring_in)
>> +        goto in_fail;
>> +
>> +    dbc->ring_out = xhci_ring_alloc(xhci, 1, 1, TYPE_BULK, 0, flags);
>> +    if (!dbc->ring_out)
>> +        goto out_fail;
>> +
>> +    /* Allocate and populate ERST: */
>> +    ret = xhci_alloc_erst(xhci, dbc->ring_evt, &dbc->erst, flags);
>> +    if (ret)
>> +        goto erst_fail;
>> +
>> +    /* Allocate context data structure: */
>> +    dbc->ctx = xhci_alloc_container_ctx(xhci, XHCI_CTX_TYPE_DEVICE, flags);
>> +    if (!dbc->ctx)
>> +        goto ctx_fail;
>> +
>> +    /* Allocate the string table: */
>> +    dbc->string_size = sizeof(struct dbc_strings);
>> +    dbc->string = dbc_dma_alloc_coherent(xhci,
>> +                         dbc->string_size,
>> +                         &dbc->string_dma,
>> +                         flags);
>> +    if (!dbc->string)
>> +        goto string_fail;
>> +
>> +    /* Setup ERST register: */
>> +    writel(dbc->erst.erst_size, &dbc->regs->ersts);
>> +    xhci_write_64(xhci, dbc->erst.erst_dma_addr, &dbc->regs->erstba);
>> +    deq = xhci_trb_virt_to_dma(dbc->ring_evt->deq_seg,
>> +                   dbc->ring_evt->dequeue);
>> +    xhci_write_64(xhci, deq, &dbc->regs->erdp);
>> +
>> +    /* Setup strings and contexts: */
>> +    string_length = xhci_dbc_populate_strings(dbc->string);
>> +    xhci_dbc_populate_contexts(xhci, string_length);
>> +
>> +    mmiowb();
>> +
>> +    xhci_dbc_eps_init(xhci);
>> +    dbc->state = DS_INITIALIZED;
>> +
>> +    return 0;
>> +
>> +string_fail:
>> +    xhci_free_container_ctx(xhci, dbc->ctx);
>> +    dbc->ctx = NULL;
>> +ctx_fail:
>> +    xhci_free_erst(xhci, &dbc->erst);
>> +erst_fail:
>> +    xhci_ring_free(xhci, dbc->ring_out);
>> +    dbc->ring_out = NULL;
>> +out_fail:
>> +    xhci_ring_free(xhci, dbc->ring_in);
>> +    dbc->ring_in = NULL;
>> +in_fail:
>> +    xhci_ring_free(xhci, dbc->ring_evt);
>> +    dbc->ring_evt = NULL;
>> +evt_fail:
>> +    return -ENOMEM;
>> +}
>> +
>> +static void xhci_dbc_mem_cleanup(struct xhci_hcd *xhci)
>> +{
>> +    struct xhci_dbc        *dbc = xhci->dbc;
>> +
>> +    if (!dbc)
>> +        return;
>> +
>> +    xhci_dbc_eps_exit(xhci);
>> +
>> +    if (dbc->string) {
>> +        dbc_dma_free_coherent(xhci,
>> +                      dbc->string_size,
>> +                      dbc->string, dbc->string_dma);
>> +        dbc->string = NULL;
>> +    }
>> +
>> +    xhci_free_container_ctx(xhci, dbc->ctx);
>> +    dbc->ctx = NULL;
>> +
>> +    xhci_free_erst(xhci, &dbc->erst);
>> +    xhci_ring_free(xhci, dbc->ring_out);
>> +    xhci_ring_free(xhci, dbc->ring_in);
>> +    xhci_ring_free(xhci, dbc->ring_evt);
>> +    dbc->ring_in = NULL;
>> +    dbc->ring_out = NULL;
>> +    dbc->ring_evt = NULL;
>> +}
>> +
>> +static void xhci_dbc_reset_debug_port(struct xhci_hcd *xhci)
>> +{
>> +    u32            val;
>> +    unsigned long        flags;
>> +    int            port_index;
>> +    __le32 __iomem        **port_array;
>> +
>> +    spin_lock_irqsave(&xhci->lock, flags);
>> +
>> +    port_index = xhci->num_usb3_ports;
>> +    port_array = xhci->usb3_ports;
>> +    if (port_index <= 0) {
>> +        spin_unlock_irqrestore(&xhci->lock, flags);
>> +        return;
>> +    }
>
> we could take the spin lock here, and avoid the above unlock case.

Yes, will re-factor it.

>
>> +
>> +    while (port_index--) {
>> +        val = readl(port_array[port_index]);
>> +        if (!(val & PORT_CONNECT))
>> +            writel(val | PORT_RESET, port_array[port_index]);
>> +    }
>
> This resets every USB3 port that does not have a device connected.
> Shouldn't the debug port be the first USB3 port in the root hub, or do I remeber wrong?
> That should the be xhci->usb3_ports[0]. Can't we reset just that one?

Yes. I will revisit this function. I even think that we don't need it at all.

>
>> +
>> +    spin_unlock_irqrestore(&xhci->lock, flags);
>> +}
>> +
>> +static int xhci_do_dbc_start(struct xhci_hcd *xhci)
>> +{
>> +    int            ret;
>> +    u32            ctrl;
>> +    struct xhci_dbc        *dbc = xhci->dbc;
>> +
>> +    if (dbc->state != DS_DISABLED)
>> +        return -EINVAL;
>> +
>> +    writel(0, &dbc->regs->control);
>> +    ret = xhci_handshake(&dbc->regs->control,
>> +                 DBC_CTRL_DBC_ENABLE,
>> +                 0, 1000);
>> +    if (ret)
>> +        return ret;
>> +
>> +    ret = xhci_dbc_mem_init(xhci, GFP_ATOMIC);
>> +    if (ret)
>> +        return ret;
>> +
>> +    ctrl = readl(&dbc->regs->control);
>> +    writel(ctrl | DBC_CTRL_DBC_ENABLE | DBC_CTRL_PORT_ENABLE,
>> +           &dbc->regs->control);
>> +    ret = xhci_handshake(&dbc->regs->control,
>> +                 DBC_CTRL_DBC_ENABLE,
>> +                 DBC_CTRL_DBC_ENABLE, 1000);
>> +    if (ret)
>> +        return ret;
>> +
>> +    xhci_dbc_reset_debug_port(xhci);
>> +    dbc->state = DS_ENABLED;
>> +
>> +    return 0;
>> +}
>> +
>> +static void xhci_do_dbc_stop(struct xhci_hcd *xhci)
>> +{
>> +    struct xhci_dbc        *dbc = xhci->dbc;
>> +
>> +    if (dbc->state == DS_DISABLED)
>> +        return;
>> +
>> +    writel(0, &dbc->regs->control);
>> +    xhci_dbc_mem_cleanup(xhci);
>> +    dbc->state = DS_DISABLED;
>> +}
>> +
>> +static int xhci_dbc_start(struct xhci_hcd *xhci)
>> +{
>> +    int            ret;
>> +    struct xhci_dbc        *dbc = xhci->dbc;
>> +
>> +    WARN_ON(!dbc);
>> +
>> +    spin_lock(&dbc->lock);
>
> Maybe call the pm_runtime_get_sync() already here to prevent runtime pm from
> stopping the controller while starting dbc
>
> we then need to pm_runtme_put it do_dbc_start() fails

Yes. Great! We should avoid host suspending while starting DbC.

>
>> +    ret = xhci_do_dbc_start(xhci);
>> +    spin_unlock(&dbc->lock);
>> +
>> +    if (!ret) {
>> +        pm_runtime_get_sync(xhci_to_hcd(xhci)->self.controller);
>> +        ret = mod_delayed_work(system_wq, &dbc->event_work, 1);
>> +    }
>> +
>> +    return ret;
>> +}
>> +
>> +static void xhci_dbc_stop(struct xhci_hcd *xhci)
>> +{
>> +    struct xhci_dbc        *dbc = xhci->dbc;
>> +    struct dbc_port        *port = &dbc->port;
>> +
>> +    WARN_ON(!dbc);
>> +
>> +    cancel_delayed_work_sync(&dbc->event_work);
>> +
>> +    if (port->registered)
>> +        xhci_dbc_tty_unregister_device(xhci);
>> +
>> +    spin_lock(&dbc->lock);
>> +    xhci_do_dbc_stop(xhci);
>> +    spin_unlock(&dbc->lock);
>> +
>> +    pm_runtime_put_sync(xhci_to_hcd(xhci)->self.controller);
>> +}
>> +
>> +static void
>> +dbc_handle_port_status(struct xhci_hcd *xhci, union xhci_trb *event)
>> +{
>> +    u32            portsc;
>> +    struct xhci_dbc        *dbc = xhci->dbc;
>> +
>> +    portsc = readl(&dbc->regs->portsc);
>> +    if (portsc & DBC_PORTSC_CONN_CHANGE)
>> +        xhci_info(xhci, "DbC port connect change\n");
>> +
>> +    if (portsc & DBC_PORTSC_RESET_CHANGE)
>> +        xhci_info(xhci, "DbC port reset change\n");
>> +
>> +    if (portsc & DBC_PORTSC_LINK_CHANGE)
>> +        xhci_info(xhci, "DbC port link status change\n");
>> +
>> +    if (portsc & DBC_PORTSC_CONFIG_CHANGE)
>> +        xhci_info(xhci, "DbC config error change\n");
>> +
>
> Are these messages info or debug?

For information. Users could read this in dmesg when they
connect DbC to a host.

> If these messages are the only messages seen when connecting a
> debug host (and not constantly spamming dmesg) then they are ok.

There are no spam messages. They only print when connecting to a
debug host.

>
> A bit like usb core annoncing "new usb device detected"

Yes.

>
>> +    /* Port reset change bit will be cleared in other place: */
>> +    writel(portsc & ~DBC_PORTSC_RESET_CHANGE, &dbc->regs->portsc);
>> +}
>> +
>> +static void dbc_handle_tx_event(struct xhci_hcd *xhci, union xhci_trb *event)
>
> Ah, here you have the opportunity to properly name this function handle_xfer_event() or
> handle_transfer_event().
>
> The handle_tx_event() we use in xhci is confusing because of the tx/rx association.

Yes, I will clean it.

>
>> +{
>> +    struct dbc_ep        *dep;
>> +    struct xhci_ring    *ring;
>> +    int            ep_id;
>> +    int            status;
>> +    u32            comp_code;
>> +    size_t            remain_length;
>> +    struct dbc_request    *req = NULL, *r;
>> +
>> +    comp_code    = GET_COMP_CODE(le32_to_cpu(event->generic.field[2]));
>> +    remain_length    = EVENT_TRB_LEN(le32_to_cpu(event->generic.field[2]));
>> +    ep_id        = TRB_TO_EP_ID(le32_to_cpu(event->generic.field[3]));
>> +    dep        = (ep_id == EPID_OUT) ?
>> +                get_out_ep(xhci) : get_in_ep(xhci);
>> +    ring        = dep->ring;
>> +
>> +    switch (comp_code) {
>> +    case COMP_SUCCESS:
>> +        remain_length = 0;
>> +    /* FALLTHROUGH */
>> +    case COMP_SHORT_PACKET:
>> +        status = 0;
>> +        break;
>> +    case COMP_TRB_ERROR:
>> +    case COMP_BABBLE_DETECTED_ERROR:
>> +    case COMP_USB_TRANSACTION_ERROR:
>> +    case COMP_STALL_ERROR:
>> +        xhci_warn(xhci, "tx error %d detected\n", comp_code);
>> +        status = -comp_code;
>> +        break;
>> +    default:
>> +        xhci_err(xhci, "unknown tx error %d\n", comp_code);
>> +        status = -comp_code;
>> +        break;
>> +    }
>> +
>> +    /* Match the pending request: */
>> +    list_for_each_entry(r, &dep->list_pending, list_pending) {
>> +        if (r->trb_dma == event->trans_event.buffer) {
>> +            req = r;
>> +            break;
>> +        }
>> +    }
>
> I like that it matches the request even if they are out of order.
>
> Just out of curiosiry, was there some issue with transfer event not always
> matching the next entry in dep->list_pending ?

I didn't catch such issue. The only possibilities are 1) the dep->list_pending
is not listed in the same order as trbs in ring; 2) transfer error happens and
the event trb doesn't match the next entry well. Anyway, the conservative
way is not always considering only the next entry.

>
>> +
>> +    if (!req) {
>> +        xhci_warn(xhci, "no matched request\n");
>> +        return;
>> +    }
>> +
>> +    trace_xhci_dbc_handle_transfer(ring, &req->trb->generic);
>> +
>> +    ring->num_trbs_free++;
>> +    req->actual = req->length - remain_length;
>> +    xhci_dbc_giveback(req, status);
>> +}
>> +
>> +static enum evtreturn xhci_dbc_do_handle_events(struct xhci_dbc *dbc)
>> +{
>> +    dma_addr_t        deq;
>> +    struct dbc_ep        *dep;
>> +    union xhci_trb        *evt;
>> +    u32            ctrl, portsc;
>> +    struct xhci_hcd        *xhci = dbc->xhci;
>> +    bool            update_erdp = false;
>> +
>> +    /* DbC state machine: */
>> +    switch (dbc->state) {
>> +    case DS_DISABLED:
>> +    case DS_INITIALIZED:
>> +
>> +        return EVT_ERR;
>> +    case DS_ENABLED:
>> +        portsc = readl(&dbc->regs->portsc);
>> +        if (portsc & DBC_PORTSC_CONN_STATUS) {
>> +            dbc->state = DS_CONNECTED;
>> +            xhci_info(xhci, "DbC connected\n");
>> +        }
>> +
>> +        return EVT_DONE;
>> +    case DS_CONNECTED:
>> +        ctrl = readl(&dbc->regs->control);
>> +        if (ctrl & DBC_CTRL_DBC_RUN) {
>> +            dbc->state = DS_CONFIGURED;
>> +            xhci_info(xhci, "DbC configured\n");
>> +            portsc = readl(&dbc->regs->portsc);
>> +            writel(portsc, &dbc->regs->portsc);
>> +            return EVT_GSER;
>> +        }
>> +
>> +        return EVT_DONE;
>> +    case DS_CONFIGURED:
>> +        /* Handle cable unplug event: */
>> +        portsc = readl(&dbc->regs->portsc);
>> +        if (!(portsc & DBC_PORTSC_PORT_ENABLED) &&
>> +            !(portsc & DBC_PORTSC_CONN_STATUS)) {
>> +            xhci_info(xhci, "DbC cable unplugged\n");
>> +            dbc->state = DS_ENABLED;
>> +            xhci_dbc_flush_reqests(dbc);
>> +
>> +            return EVT_DISC;
>> +        }
>> +
>> +        /* Handle debug port reset event: */
>> +        if (portsc & DBC_PORTSC_RESET_CHANGE) {
>> +            xhci_info(xhci, "DbC port reset\n");
>> +            writel(portsc, &dbc->regs->portsc);
>> +            dbc->state = DS_ENABLED;
>> +            xhci_dbc_flush_reqests(dbc);
>> +
>> +            return EVT_DISC;
>> +        }
>> +
>> +        /* Handle endpoint stall event: */
>> +        ctrl = readl(&dbc->regs->control);
>> +        if ((ctrl & DBC_CTRL_HALT_IN_TR) ||
>> +            (ctrl & DBC_CTRL_HALT_OUT_TR)) {
>> +            xhci_info(xhci, "DbC Endpoint stall\n");
>> +            dbc->state = DS_STALLED;
>> +
>> +            if (ctrl & DBC_CTRL_HALT_IN_TR) {
>> +                dep = get_in_ep(xhci);
>> +                xhci_dbc_flush_endpoint_requests(dep);
>> +            }
>> +
>> +            if (ctrl & DBC_CTRL_HALT_OUT_TR) {
>> +                dep = get_out_ep(xhci);
>> +                xhci_dbc_flush_endpoint_requests(dep);
>> +            }
>> +
>> +            return EVT_DONE;
>> +        }
>> +
>> +        /* Clear DbC run change bit: */
>> +        if (ctrl & DBC_CTRL_DBC_RUN_CHANGE) {
>> +            writel(ctrl, &dbc->regs->control);
>> +            ctrl = readl(&dbc->regs->control);
>> +        }
>> +
>> +        break;
>> +    case DS_STALLED:
>> +        ctrl = readl(&dbc->regs->control);
>> +        if (!(ctrl & DBC_CTRL_HALT_IN_TR) &&
>> +            !(ctrl & DBC_CTRL_HALT_OUT_TR) &&
>> +            (ctrl & DBC_CTRL_DBC_RUN)) {
>> +            dbc->state = DS_CONFIGURED;
>> +            break;
>> +        }
>> +
>> +        return EVT_DONE;
>> +    default:
>> +        xhci_err(xhci, "Unknown DbC state %d\n", dbc->state);
>> +        break;
>> +    }
>> +
>> +    /* Handle the events in the event ring: */
>> +    evt = dbc->ring_evt->dequeue;
>> +    while ((le32_to_cpu(evt->event_cmd.flags) & TRB_CYCLE) ==
>> +            dbc->ring_evt->cycle_state) {
>> +        /*
>> +         * Add a barrier between reading the cycle flag and any
>> +         * reads of the event's flags/data below:
>> +         */
>> +        rmb();
>> +
>> +        trace_xhci_dbc_handle_event(dbc->ring_evt, &evt->generic);
>> +
>> +        switch (le32_to_cpu(evt->event_cmd.flags) & TRB_TYPE_BITMASK) {
>> +        case TRB_TYPE(TRB_PORT_STATUS):
>> +            dbc_handle_port_status(xhci, evt);
>> +            break;
>> +        case TRB_TYPE(TRB_TRANSFER):
>> +            dbc_handle_tx_event(xhci, evt);
>> +            break;
>> +        default:
>> +            break;
>> +        }
>> +
>> +        inc_deq(xhci, dbc->ring_evt);
>> +        evt = dbc->ring_evt->dequeue;
>> +        update_erdp = true;
>> +    }
>> +
>> +    /* Update event ring dequeue pointer: */
>> +    if (update_erdp) {
>> +        deq = xhci_trb_virt_to_dma(dbc->ring_evt->deq_seg,
>> +                       dbc->ring_evt->dequeue);
>> +        xhci_write_64(xhci, deq, &dbc->regs->erdp);
>> +    }
>> +
>> +    return EVT_DONE;
>> +}
>> +
>> +static void xhci_dbc_handle_events(struct work_struct *work)
>> +{
>> +    int            ret;
>> +    enum evtreturn        evtr;
>> +    struct xhci_dbc        *dbc;
>> +    struct xhci_hcd        *xhci;
>> +
>> +    dbc = container_of(to_delayed_work(work), struct xhci_dbc, event_work);
>> +    xhci = dbc->xhci;
>> +
>> +    spin_lock(&dbc->lock);
>> +    evtr = xhci_dbc_do_handle_events(dbc);
>> +    spin_unlock(&dbc->lock);
>> +
>> +    switch (evtr) {
>> +    case EVT_GSER:
>> +        ret = xhci_dbc_tty_register_device(xhci);
>> +        if (ret) {
>> +            xhci_err(xhci, "failed to alloc tty device\n");
>> +            break;
>> +        }
>> +
>> +        xhci_info(xhci, "DbC now attached to /dev/ttyDBC0\n");
>> +        mod_delayed_work(system_wq, &dbc->event_work, 1);
>> +        break;
>> +    case EVT_DISC:
>> +        xhci_dbc_tty_unregister_device(xhci);
>> +        mod_delayed_work(system_wq, &dbc->event_work, 1);
>> +        break;
>> +    case EVT_DONE:
>> +        mod_delayed_work(system_wq, &dbc->event_work, 1);
>> +        break;
>> +    default:
>> +        xhci_info(xhci, "stop handling dbc events\n");
>
>     return; here
>
>> +    }
>
> mod_delayed_work(); here, and remove it from above cases.

Yes, will clean it.

>
>> +}
>> +
>> +static void xhci_dbc_exit(struct xhci_hcd *xhci)
>> +{
>> +    unsigned long        flags;
>> +
>> +    spin_lock_irqsave(&xhci->lock, flags);
>> +    kfree(xhci->dbc);
>> +    xhci->dbc = NULL;
>> +    spin_unlock_irqrestore(&xhci->lock, flags);
>> +}
>> +
>> +static int xhci_dbc_init(struct xhci_hcd *xhci)
>> +{
>> +    u32            reg;
>> +    struct xhci_dbc        *dbc;
>> +    unsigned long        flags;
>> +    void __iomem        *base;
>> +    int            dbc_cap_offs;
>> +
>> +    base = &xhci->cap_regs->hc_capbase;
>> +    dbc_cap_offs = xhci_find_next_ext_cap(base, 0, XHCI_EXT_CAPS_DEBUG);
>> +    if (!dbc_cap_offs)
>> +        return -ENODEV;
>> +
>> +    dbc = kzalloc(sizeof(*dbc), GFP_KERNEL);
>> +    if (!dbc)
>> +        return -ENOMEM;
>> +
>> +    dbc->regs = base + dbc_cap_offs;
>> +
>> +    /* We will avoid using DbC in xhci driver if it's in use. */
>> +    reg = readl(&dbc->regs->control);
>> +    if (reg & DBC_CTRL_DBC_ENABLE) {
>> +        kfree(dbc);
>> +        return -EBUSY;
>> +    }
>> +
>> +    spin_lock_irqsave(&xhci->lock, flags);
>> +    if (xhci->dbc) {
>> +        spin_unlock_irqrestore(&xhci->lock, flags);
>> +        kfree(dbc);
>> +        return -EBUSY;
>> +    }
>> +    xhci->dbc = dbc;
>> +    spin_unlock_irqrestore(&xhci->lock, flags);
>> +
>> +    dbc->xhci = xhci;
>> +    INIT_DELAYED_WORK(&dbc->event_work, xhci_dbc_handle_events);
>> +    spin_lock_init(&dbc->lock);
>> +
>> +    return 0;
>> +}
>> +
>> +static ssize_t dbc_show(struct device *dev,
>> +            struct device_attribute *attr,
>> +            char *buf)
>> +{
>> +    const char        *p;
>> +    struct xhci_dbc        *dbc;
>> +    struct xhci_hcd        *xhci;
>> +
>> +    xhci = hcd_to_xhci(dev_get_drvdata(dev));
>> +    dbc = xhci->dbc;
>> +
>> +    switch (dbc->state) {
>> +    case DS_DISABLED:
>> +        p = "disabled";
>> +        break;
>> +    case DS_INITIALIZED:
>> +        p = "initialized";
>> +        break;
>> +    case DS_ENABLED:
>> +        p = "enabled";
>> +        break;
>> +    case DS_CONNECTED:
>> +        p = "connected";
>> +        break;
>> +    case DS_CONFIGURED:
>> +        p = "configured";
>> +        break;
>> +    case DS_STALLED:
>> +        p = "stalled";
>> +        break;
>> +    default:
>> +        p = "unknown";
>> +    }
>> +
>> +    return sprintf(buf, "%s\n", p);
>> +}
>> +
>> +static ssize_t dbc_store(struct device *dev,
>> +             struct device_attribute *attr,
>> +             const char *buf, size_t count)
>> +{
>> +    struct xhci_dbc        *dbc;
>> +    struct xhci_hcd        *xhci;
>> +
>> +    xhci = hcd_to_xhci(dev_get_drvdata(dev));
>> +    dbc = xhci->dbc;
>> +
>> +    if (!strncmp(buf, "enable", 6))
>> +        xhci_dbc_start(xhci);
>> +    else if (!strncmp(buf, "disable", 7))
>> +        xhci_dbc_stop(xhci);
>> +    else
>> +        return -EINVAL;
>> +
>> +    return count;
>> +}
>> +
>> +static DEVICE_ATTR(dbc, 0644, dbc_show, dbc_store);
>> +
>> +int dbc_create_sysfs_file(struct xhci_hcd *xhci)
>> +{
>> +    int            ret;
>> +    struct device        *dev = xhci_to_hcd(xhci)->self.controller;
>> +
>> +    ret = xhci_dbc_init(xhci);
>> +    if (ret)
>> +        return ret;
>> +
>> +    xhci_dbc_tty_register_driver(xhci);
>
> The function name "dbc_create_sysfs_file()" doesn't reveal anything about
> that is also registers the tty driver

Yes. Maybe xhci_dbc_init/exit() sounds better.

I will refine it.

>
>> +
>> +    return device_create_file(dev, &dev_attr_dbc);
>> +}
>> +
>> +void dbc_remove_sysfs_file(struct xhci_hcd *xhci)
>> +{
>> +    struct device        *dev = xhci_to_hcd(xhci)->self.controller;
>> +
>> +    device_remove_file(dev, &dev_attr_dbc);
>> +    xhci_dbc_tty_unregister_driver();
>> +    xhci_dbc_stop(xhci);
>> +    xhci_dbc_exit(xhci);
>> +}
>> +
>> +#ifdef CONFIG_PM
>> +int xhci_dbc_suspend(struct xhci_hcd *xhci)
>> +{
>> +    struct xhci_dbc        *dbc = xhci->dbc;
>> +
>> +    if (!dbc)
>> +        return 0;
>> +
>> +    if (dbc->state == DS_CONFIGURED)
>> +        dbc->resume_required = 1;
>> +
>> +    xhci_dbc_stop(xhci);
>> +
>> +    return 0;
>> +}
>> +
>> +int xhci_dbc_resume(struct xhci_hcd *xhci)
>> +{
>> +    int            ret = 0;
>> +    struct xhci_dbc        *dbc = xhci->dbc;
>> +
>> +    if (!dbc)
>> +        return 0;
>> +
>> +    if (dbc->resume_required) {
>> +        dbc->resume_required = 0;
>> +        xhci_dbc_start(xhci);
>> +    }
>> +
>> +    return ret;
>
> So far the dbc driver looks really good, it's getting late,
> I need to reviewing the other files later.
>
> I can't say much about the TTY parts as I never needed to work on those
>
> -Mathias

Thanks for your time. I will change the code according to your comments.
The new version(v4) will come out after 4.15-rc1 is out.

Best regards,
Lu Baolu

^ permalink raw reply	[flat|nested] 21+ messages in thread

* Re: [PATCH v3 2/3] usb: xhci: Add DbC support in xHCI driver
  2017-09-05  1:58 ` [PATCH v3 2/3] usb: xhci: Add DbC support in xHCI driver Lu Baolu
  2017-10-24 17:03   ` Mathias Nyman
@ 2017-11-02  1:36   ` Lu Baolu
  2017-11-02  8:17     ` Greg Kroah-Hartman
  2017-11-13 15:26   ` Mathias Nyman
  2 siblings, 1 reply; 21+ messages in thread
From: Lu Baolu @ 2017-11-02  1:36 UTC (permalink / raw)
  To: Greg Kroah-Hartman
  Cc: 'Mathias Nyman', Felipe Balbi, linux-usb, linux-kernel

Hi Greg,

On 09/05/2017 09:58 AM, Lu Baolu wrote:
> xHCI compatible USB host controllers(i.e. super-speed USB3 controllers)
> can be implemented with the Debug Capability(DbC). It presents a debug
> device which is fully compliant with the USB framework and provides the
> equivalent of a very high performance full-duplex serial link. The debug
> capability operation model and registers interface are defined in 7.6.8
> of the xHCI specification, revision 1.1.
>
> The DbC debug device shares a root port with the xHCI host. By default,
> the debug capability is disabled and the root port is assigned to xHCI.
> When the DbC is enabled, the root port will be assigned to the DbC debug
> device, and the xHCI sees nothing on this port. This implementation uses
> a sysfs node named <dbc> under the xHCI device to manage the enabling
> and disabling of the debug capability.
>
> When the debug capability is enabled, it will present a debug device
> through the debug port. This debug device is fully compliant with the
> USB3 framework, and it can be enumerated by a debug host on the other
> end of the USB link. As soon as the debug device is configured, a TTY
> serial device named /dev/ttyDBC0 will be created.
>
> One use of this link is running a login service on the debug target.
> Hence it can be remote accessed by a debug host. Another use case can
> probably be found in servers. It provides a peer-to-peer USB link
> between two host-only machines. This provides a reasonable out-of-band
> communication method between two servers.
>
> Signed-off-by: Lu Baolu <baolu.lu@linux.intel.com>
> ---
>  .../ABI/testing/sysfs-bus-pci-drivers-xhci_hcd     |   25 +
>  drivers/usb/host/Kconfig                           |    9 +
>  drivers/usb/host/Makefile                          |    5 +
>  drivers/usb/host/xhci-dbgcap.c                     | 1016 ++++++++++++++++++++
>  drivers/usb/host/xhci-dbgcap.h                     |  247 +++++
>  drivers/usb/host/xhci-dbgtty.c                     |  586 +++++++++++
>  drivers/usb/host/xhci-trace.h                      |   60 ++
>  drivers/usb/host/xhci.c                            |   10 +
>  drivers/usb/host/xhci.h                            |    1 +
>  9 files changed, 1959 insertions(+)
>  create mode 100644 Documentation/ABI/testing/sysfs-bus-pci-drivers-xhci_hcd
>  create mode 100644 drivers/usb/host/xhci-dbgcap.c
>  create mode 100644 drivers/usb/host/xhci-dbgcap.h
>  create mode 100644 drivers/usb/host/xhci-dbgtty.c
>

[snip]

> +#define DBC_VENDOR_ID			0x1d6b	/* Linux Foundation 0x1d6b */
> +#define DBC_PRODUCT_ID			0x0004	/* device 0004 */
>

The DbC (xHCI DeBug Capability) is an optional functionality in
some xHCI host controllers. It will present a super-speed debug
device through the debug port after it is enabled.

The DbC register set defines an interface for system software
to specify the vendor id and product id of the debug device.
These two values will be presented by the debug device in its
device descriptor idVendor and idProduct fields.

Microsoft Windows have a well established protocol for
debugging over DbC. And it assigns below values for its use.

USB\VID_045E&PID_062D.DeviceDesc="Microsoft USB Debug Target"

I'm going to use 0x1d6b/0x0004 value pair for DbC use in
Linux. Do you approve me to do so?

Best regards,
Lu Baolu

^ permalink raw reply	[flat|nested] 21+ messages in thread

* Re: [PATCH v3 2/3] usb: xhci: Add DbC support in xHCI driver
  2017-11-02  1:36   ` Lu Baolu
@ 2017-11-02  8:17     ` Greg Kroah-Hartman
  2017-11-02  9:15       ` Felipe Balbi
  0 siblings, 1 reply; 21+ messages in thread
From: Greg Kroah-Hartman @ 2017-11-02  8:17 UTC (permalink / raw)
  To: Lu Baolu; +Cc: 'Mathias Nyman', Felipe Balbi, linux-usb, linux-kernel

On Thu, Nov 02, 2017 at 09:36:42AM +0800, Lu Baolu wrote:
> Hi Greg,
> 
> On 09/05/2017 09:58 AM, Lu Baolu wrote:
> > xHCI compatible USB host controllers(i.e. super-speed USB3 controllers)
> > can be implemented with the Debug Capability(DbC). It presents a debug
> > device which is fully compliant with the USB framework and provides the
> > equivalent of a very high performance full-duplex serial link. The debug
> > capability operation model and registers interface are defined in 7.6.8
> > of the xHCI specification, revision 1.1.
> >
> > The DbC debug device shares a root port with the xHCI host. By default,
> > the debug capability is disabled and the root port is assigned to xHCI.
> > When the DbC is enabled, the root port will be assigned to the DbC debug
> > device, and the xHCI sees nothing on this port. This implementation uses
> > a sysfs node named <dbc> under the xHCI device to manage the enabling
> > and disabling of the debug capability.
> >
> > When the debug capability is enabled, it will present a debug device
> > through the debug port. This debug device is fully compliant with the
> > USB3 framework, and it can be enumerated by a debug host on the other
> > end of the USB link. As soon as the debug device is configured, a TTY
> > serial device named /dev/ttyDBC0 will be created.
> >
> > One use of this link is running a login service on the debug target.
> > Hence it can be remote accessed by a debug host. Another use case can
> > probably be found in servers. It provides a peer-to-peer USB link
> > between two host-only machines. This provides a reasonable out-of-band
> > communication method between two servers.
> >
> > Signed-off-by: Lu Baolu <baolu.lu@linux.intel.com>
> > ---
> >  .../ABI/testing/sysfs-bus-pci-drivers-xhci_hcd     |   25 +
> >  drivers/usb/host/Kconfig                           |    9 +
> >  drivers/usb/host/Makefile                          |    5 +
> >  drivers/usb/host/xhci-dbgcap.c                     | 1016 ++++++++++++++++++++
> >  drivers/usb/host/xhci-dbgcap.h                     |  247 +++++
> >  drivers/usb/host/xhci-dbgtty.c                     |  586 +++++++++++
> >  drivers/usb/host/xhci-trace.h                      |   60 ++
> >  drivers/usb/host/xhci.c                            |   10 +
> >  drivers/usb/host/xhci.h                            |    1 +
> >  9 files changed, 1959 insertions(+)
> >  create mode 100644 Documentation/ABI/testing/sysfs-bus-pci-drivers-xhci_hcd
> >  create mode 100644 drivers/usb/host/xhci-dbgcap.c
> >  create mode 100644 drivers/usb/host/xhci-dbgcap.h
> >  create mode 100644 drivers/usb/host/xhci-dbgtty.c
> >
> 
> [snip]
> 
> > +#define DBC_VENDOR_ID			0x1d6b	/* Linux Foundation 0x1d6b */
> > +#define DBC_PRODUCT_ID			0x0004	/* device 0004 */
> >
> 
> The DbC (xHCI DeBug Capability) is an optional functionality in
> some xHCI host controllers. It will present a super-speed debug
> device through the debug port after it is enabled.
> 
> The DbC register set defines an interface for system software
> to specify the vendor id and product id of the debug device.
> These two values will be presented by the debug device in its
> device descriptor idVendor and idProduct fields.
> 
> Microsoft Windows have a well established protocol for
> debugging over DbC. And it assigns below values for its use.
> 
> USB\VID_045E&PID_062D.DeviceDesc="Microsoft USB Debug Target"
> 
> I'm going to use 0x1d6b/0x0004 value pair for DbC use in
> Linux. Do you approve me to do so?

No.  Why can't you use the same ids as Windows?  This is implementing
the same protocol, right?

If you have to use a OS-specific id, I can give you a new one, but 0004
is not ok to use for this.

thanks,

greg k-h

^ permalink raw reply	[flat|nested] 21+ messages in thread

* Re: [PATCH v3 2/3] usb: xhci: Add DbC support in xHCI driver
  2017-11-02  8:17     ` Greg Kroah-Hartman
@ 2017-11-02  9:15       ` Felipe Balbi
  2017-11-02  9:32         ` Greg Kroah-Hartman
  0 siblings, 1 reply; 21+ messages in thread
From: Felipe Balbi @ 2017-11-02  9:15 UTC (permalink / raw)
  To: Greg Kroah-Hartman, Lu Baolu
  Cc: 'Mathias Nyman', linux-usb, linux-kernel

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


Hi,

Greg Kroah-Hartman <gregkh@linuxfoundation.org> writes:
>> > xHCI compatible USB host controllers(i.e. super-speed USB3 controllers)
>> > can be implemented with the Debug Capability(DbC). It presents a debug
>> > device which is fully compliant with the USB framework and provides the
>> > equivalent of a very high performance full-duplex serial link. The debug
>> > capability operation model and registers interface are defined in 7.6.8
>> > of the xHCI specification, revision 1.1.
>> >
>> > The DbC debug device shares a root port with the xHCI host. By default,
>> > the debug capability is disabled and the root port is assigned to xHCI.
>> > When the DbC is enabled, the root port will be assigned to the DbC debug
>> > device, and the xHCI sees nothing on this port. This implementation uses
>> > a sysfs node named <dbc> under the xHCI device to manage the enabling
>> > and disabling of the debug capability.
>> >
>> > When the debug capability is enabled, it will present a debug device
>> > through the debug port. This debug device is fully compliant with the
>> > USB3 framework, and it can be enumerated by a debug host on the other
>> > end of the USB link. As soon as the debug device is configured, a TTY
>> > serial device named /dev/ttyDBC0 will be created.
>> >
>> > One use of this link is running a login service on the debug target.
>> > Hence it can be remote accessed by a debug host. Another use case can
>> > probably be found in servers. It provides a peer-to-peer USB link
>> > between two host-only machines. This provides a reasonable out-of-band
>> > communication method between two servers.
>> >
>> > Signed-off-by: Lu Baolu <baolu.lu@linux.intel.com>
>> > ---
>> >  .../ABI/testing/sysfs-bus-pci-drivers-xhci_hcd     |   25 +
>> >  drivers/usb/host/Kconfig                           |    9 +
>> >  drivers/usb/host/Makefile                          |    5 +
>> >  drivers/usb/host/xhci-dbgcap.c                     | 1016 ++++++++++++++++++++
>> >  drivers/usb/host/xhci-dbgcap.h                     |  247 +++++
>> >  drivers/usb/host/xhci-dbgtty.c                     |  586 +++++++++++
>> >  drivers/usb/host/xhci-trace.h                      |   60 ++
>> >  drivers/usb/host/xhci.c                            |   10 +
>> >  drivers/usb/host/xhci.h                            |    1 +
>> >  9 files changed, 1959 insertions(+)
>> >  create mode 100644 Documentation/ABI/testing/sysfs-bus-pci-drivers-xhci_hcd
>> >  create mode 100644 drivers/usb/host/xhci-dbgcap.c
>> >  create mode 100644 drivers/usb/host/xhci-dbgcap.h
>> >  create mode 100644 drivers/usb/host/xhci-dbgtty.c
>> >
>> 
>> [snip]
>> 
>> > +#define DBC_VENDOR_ID			0x1d6b	/* Linux Foundation 0x1d6b */
>> > +#define DBC_PRODUCT_ID			0x0004	/* device 0004 */
>> >
>> 
>> The DbC (xHCI DeBug Capability) is an optional functionality in
>> some xHCI host controllers. It will present a super-speed debug
>> device through the debug port after it is enabled.
>> 
>> The DbC register set defines an interface for system software
>> to specify the vendor id and product id of the debug device.
>> These two values will be presented by the debug device in its
>> device descriptor idVendor and idProduct fields.
>> 
>> Microsoft Windows have a well established protocol for
>> debugging over DbC. And it assigns below values for its use.
>> 
>> USB\VID_045E&PID_062D.DeviceDesc="Microsoft USB Debug Target"
>> 
>> I'm going to use 0x1d6b/0x0004 value pair for DbC use in
>> Linux. Do you approve me to do so?
>
> No.  Why can't you use the same ids as Windows?  This is implementing
> the same protocol, right?

the protocol running on top is 100% vendor specific. More than likely,
we would just run kgdb on top of this, right? We really don't support
microsoft's debug architecture.

-- 
balbi

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 832 bytes --]

^ permalink raw reply	[flat|nested] 21+ messages in thread

* Re: [PATCH v3 2/3] usb: xhci: Add DbC support in xHCI driver
  2017-11-02  9:15       ` Felipe Balbi
@ 2017-11-02  9:32         ` Greg Kroah-Hartman
  2017-11-02 10:38           ` Felipe Balbi
  0 siblings, 1 reply; 21+ messages in thread
From: Greg Kroah-Hartman @ 2017-11-02  9:32 UTC (permalink / raw)
  To: Felipe Balbi; +Cc: Lu Baolu, 'Mathias Nyman', linux-usb, linux-kernel

On Thu, Nov 02, 2017 at 11:15:43AM +0200, Felipe Balbi wrote:
> 
> Hi,
> 
> Greg Kroah-Hartman <gregkh@linuxfoundation.org> writes:
> >> > xHCI compatible USB host controllers(i.e. super-speed USB3 controllers)
> >> > can be implemented with the Debug Capability(DbC). It presents a debug
> >> > device which is fully compliant with the USB framework and provides the
> >> > equivalent of a very high performance full-duplex serial link. The debug
> >> > capability operation model and registers interface are defined in 7.6.8
> >> > of the xHCI specification, revision 1.1.
> >> >
> >> > The DbC debug device shares a root port with the xHCI host. By default,
> >> > the debug capability is disabled and the root port is assigned to xHCI.
> >> > When the DbC is enabled, the root port will be assigned to the DbC debug
> >> > device, and the xHCI sees nothing on this port. This implementation uses
> >> > a sysfs node named <dbc> under the xHCI device to manage the enabling
> >> > and disabling of the debug capability.
> >> >
> >> > When the debug capability is enabled, it will present a debug device
> >> > through the debug port. This debug device is fully compliant with the
> >> > USB3 framework, and it can be enumerated by a debug host on the other
> >> > end of the USB link. As soon as the debug device is configured, a TTY
> >> > serial device named /dev/ttyDBC0 will be created.
> >> >
> >> > One use of this link is running a login service on the debug target.
> >> > Hence it can be remote accessed by a debug host. Another use case can
> >> > probably be found in servers. It provides a peer-to-peer USB link
> >> > between two host-only machines. This provides a reasonable out-of-band
> >> > communication method between two servers.
> >> >
> >> > Signed-off-by: Lu Baolu <baolu.lu@linux.intel.com>
> >> > ---
> >> >  .../ABI/testing/sysfs-bus-pci-drivers-xhci_hcd     |   25 +
> >> >  drivers/usb/host/Kconfig                           |    9 +
> >> >  drivers/usb/host/Makefile                          |    5 +
> >> >  drivers/usb/host/xhci-dbgcap.c                     | 1016 ++++++++++++++++++++
> >> >  drivers/usb/host/xhci-dbgcap.h                     |  247 +++++
> >> >  drivers/usb/host/xhci-dbgtty.c                     |  586 +++++++++++
> >> >  drivers/usb/host/xhci-trace.h                      |   60 ++
> >> >  drivers/usb/host/xhci.c                            |   10 +
> >> >  drivers/usb/host/xhci.h                            |    1 +
> >> >  9 files changed, 1959 insertions(+)
> >> >  create mode 100644 Documentation/ABI/testing/sysfs-bus-pci-drivers-xhci_hcd
> >> >  create mode 100644 drivers/usb/host/xhci-dbgcap.c
> >> >  create mode 100644 drivers/usb/host/xhci-dbgcap.h
> >> >  create mode 100644 drivers/usb/host/xhci-dbgtty.c
> >> >
> >> 
> >> [snip]
> >> 
> >> > +#define DBC_VENDOR_ID			0x1d6b	/* Linux Foundation 0x1d6b */
> >> > +#define DBC_PRODUCT_ID			0x0004	/* device 0004 */
> >> >
> >> 
> >> The DbC (xHCI DeBug Capability) is an optional functionality in
> >> some xHCI host controllers. It will present a super-speed debug
> >> device through the debug port after it is enabled.
> >> 
> >> The DbC register set defines an interface for system software
> >> to specify the vendor id and product id of the debug device.
> >> These two values will be presented by the debug device in its
> >> device descriptor idVendor and idProduct fields.
> >> 
> >> Microsoft Windows have a well established protocol for
> >> debugging over DbC. And it assigns below values for its use.
> >> 
> >> USB\VID_045E&PID_062D.DeviceDesc="Microsoft USB Debug Target"
> >> 
> >> I'm going to use 0x1d6b/0x0004 value pair for DbC use in
> >> Linux. Do you approve me to do so?
> >
> > No.  Why can't you use the same ids as Windows?  This is implementing
> > the same protocol, right?
> 
> the protocol running on top is 100% vendor specific. More than likely,
> we would just run kgdb on top of this, right? We really don't support
> microsoft's debug architecture.

Ah, I didn't know about the protocol specifics here, if it is
vendor-specific, then yes, we need our own id.

As the above text said "well established protocol", I assumed we
implemented the same thing :)

thanks,

greg k-h

^ permalink raw reply	[flat|nested] 21+ messages in thread

* Re: [PATCH v3 2/3] usb: xhci: Add DbC support in xHCI driver
  2017-11-02  9:32         ` Greg Kroah-Hartman
@ 2017-11-02 10:38           ` Felipe Balbi
  2017-11-02 16:51             ` Greg Kroah-Hartman
  0 siblings, 1 reply; 21+ messages in thread
From: Felipe Balbi @ 2017-11-02 10:38 UTC (permalink / raw)
  To: Greg Kroah-Hartman
  Cc: Lu Baolu, 'Mathias Nyman', linux-usb, linux-kernel

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


Hi,

Greg Kroah-Hartman <gregkh@linuxfoundation.org> writes:
>> Greg Kroah-Hartman <gregkh@linuxfoundation.org> writes:
>> >> > xHCI compatible USB host controllers(i.e. super-speed USB3 controllers)
>> >> > can be implemented with the Debug Capability(DbC). It presents a debug
>> >> > device which is fully compliant with the USB framework and provides the
>> >> > equivalent of a very high performance full-duplex serial link. The debug
>> >> > capability operation model and registers interface are defined in 7.6.8
>> >> > of the xHCI specification, revision 1.1.
>> >> >
>> >> > The DbC debug device shares a root port with the xHCI host. By default,
>> >> > the debug capability is disabled and the root port is assigned to xHCI.
>> >> > When the DbC is enabled, the root port will be assigned to the DbC debug
>> >> > device, and the xHCI sees nothing on this port. This implementation uses
>> >> > a sysfs node named <dbc> under the xHCI device to manage the enabling
>> >> > and disabling of the debug capability.
>> >> >
>> >> > When the debug capability is enabled, it will present a debug device
>> >> > through the debug port. This debug device is fully compliant with the
>> >> > USB3 framework, and it can be enumerated by a debug host on the other
>> >> > end of the USB link. As soon as the debug device is configured, a TTY
>> >> > serial device named /dev/ttyDBC0 will be created.
>> >> >
>> >> > One use of this link is running a login service on the debug target.
>> >> > Hence it can be remote accessed by a debug host. Another use case can
>> >> > probably be found in servers. It provides a peer-to-peer USB link
>> >> > between two host-only machines. This provides a reasonable out-of-band
>> >> > communication method between two servers.
>> >> >
>> >> > Signed-off-by: Lu Baolu <baolu.lu@linux.intel.com>
>> >> > ---
>> >> >  .../ABI/testing/sysfs-bus-pci-drivers-xhci_hcd     |   25 +
>> >> >  drivers/usb/host/Kconfig                           |    9 +
>> >> >  drivers/usb/host/Makefile                          |    5 +
>> >> >  drivers/usb/host/xhci-dbgcap.c                     | 1016 ++++++++++++++++++++
>> >> >  drivers/usb/host/xhci-dbgcap.h                     |  247 +++++
>> >> >  drivers/usb/host/xhci-dbgtty.c                     |  586 +++++++++++
>> >> >  drivers/usb/host/xhci-trace.h                      |   60 ++
>> >> >  drivers/usb/host/xhci.c                            |   10 +
>> >> >  drivers/usb/host/xhci.h                            |    1 +
>> >> >  9 files changed, 1959 insertions(+)
>> >> >  create mode 100644 Documentation/ABI/testing/sysfs-bus-pci-drivers-xhci_hcd
>> >> >  create mode 100644 drivers/usb/host/xhci-dbgcap.c
>> >> >  create mode 100644 drivers/usb/host/xhci-dbgcap.h
>> >> >  create mode 100644 drivers/usb/host/xhci-dbgtty.c
>> >> >
>> >> 
>> >> [snip]
>> >> 
>> >> > +#define DBC_VENDOR_ID			0x1d6b	/* Linux Foundation 0x1d6b */
>> >> > +#define DBC_PRODUCT_ID			0x0004	/* device 0004 */
>> >> >
>> >> 
>> >> The DbC (xHCI DeBug Capability) is an optional functionality in
>> >> some xHCI host controllers. It will present a super-speed debug
>> >> device through the debug port after it is enabled.
>> >> 
>> >> The DbC register set defines an interface for system software
>> >> to specify the vendor id and product id of the debug device.
>> >> These two values will be presented by the debug device in its
>> >> device descriptor idVendor and idProduct fields.
>> >> 
>> >> Microsoft Windows have a well established protocol for
>> >> debugging over DbC. And it assigns below values for its use.
>> >> 
>> >> USB\VID_045E&PID_062D.DeviceDesc="Microsoft USB Debug Target"
>> >> 
>> >> I'm going to use 0x1d6b/0x0004 value pair for DbC use in
>> >> Linux. Do you approve me to do so?
>> >
>> > No.  Why can't you use the same ids as Windows?  This is implementing
>> > the same protocol, right?
>> 
>> the protocol running on top is 100% vendor specific. More than likely,
>> we would just run kgdb on top of this, right? We really don't support
>> microsoft's debug architecture.
>
> Ah, I didn't know about the protocol specifics here, if it is
> vendor-specific, then yes, we need our own id.

Great, thanks :-)

Let us know which one we're allowed to use and I'm sure Baolu can respin
the patch in no time.

> As the above text said "well established protocol", I assumed we
> implemented the same thing :)

It's "well established" from Microsoft's point of view :-) They have
that same protocol running over USB, TCP, UDP...

It would be nice if we could ditch TTY and teach gdb about other
transports, but then again, why bother if we can reuse code that already
works? :-)

-- 
balbi

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 832 bytes --]

^ permalink raw reply	[flat|nested] 21+ messages in thread

* Re: [PATCH v3 2/3] usb: xhci: Add DbC support in xHCI driver
  2017-11-02 10:38           ` Felipe Balbi
@ 2017-11-02 16:51             ` Greg Kroah-Hartman
  2017-11-03  0:45               ` Lu Baolu
  0 siblings, 1 reply; 21+ messages in thread
From: Greg Kroah-Hartman @ 2017-11-02 16:51 UTC (permalink / raw)
  To: Felipe Balbi; +Cc: Lu Baolu, 'Mathias Nyman', linux-usb, linux-kernel

On Thu, Nov 02, 2017 at 12:38:57PM +0200, Felipe Balbi wrote:
> 
> Hi,
> 
> Greg Kroah-Hartman <gregkh@linuxfoundation.org> writes:
> >> Greg Kroah-Hartman <gregkh@linuxfoundation.org> writes:
> >> >> > xHCI compatible USB host controllers(i.e. super-speed USB3 controllers)
> >> >> > can be implemented with the Debug Capability(DbC). It presents a debug
> >> >> > device which is fully compliant with the USB framework and provides the
> >> >> > equivalent of a very high performance full-duplex serial link. The debug
> >> >> > capability operation model and registers interface are defined in 7.6.8
> >> >> > of the xHCI specification, revision 1.1.
> >> >> >
> >> >> > The DbC debug device shares a root port with the xHCI host. By default,
> >> >> > the debug capability is disabled and the root port is assigned to xHCI.
> >> >> > When the DbC is enabled, the root port will be assigned to the DbC debug
> >> >> > device, and the xHCI sees nothing on this port. This implementation uses
> >> >> > a sysfs node named <dbc> under the xHCI device to manage the enabling
> >> >> > and disabling of the debug capability.
> >> >> >
> >> >> > When the debug capability is enabled, it will present a debug device
> >> >> > through the debug port. This debug device is fully compliant with the
> >> >> > USB3 framework, and it can be enumerated by a debug host on the other
> >> >> > end of the USB link. As soon as the debug device is configured, a TTY
> >> >> > serial device named /dev/ttyDBC0 will be created.
> >> >> >
> >> >> > One use of this link is running a login service on the debug target.
> >> >> > Hence it can be remote accessed by a debug host. Another use case can
> >> >> > probably be found in servers. It provides a peer-to-peer USB link
> >> >> > between two host-only machines. This provides a reasonable out-of-band
> >> >> > communication method between two servers.
> >> >> >
> >> >> > Signed-off-by: Lu Baolu <baolu.lu@linux.intel.com>
> >> >> > ---
> >> >> >  .../ABI/testing/sysfs-bus-pci-drivers-xhci_hcd     |   25 +
> >> >> >  drivers/usb/host/Kconfig                           |    9 +
> >> >> >  drivers/usb/host/Makefile                          |    5 +
> >> >> >  drivers/usb/host/xhci-dbgcap.c                     | 1016 ++++++++++++++++++++
> >> >> >  drivers/usb/host/xhci-dbgcap.h                     |  247 +++++
> >> >> >  drivers/usb/host/xhci-dbgtty.c                     |  586 +++++++++++
> >> >> >  drivers/usb/host/xhci-trace.h                      |   60 ++
> >> >> >  drivers/usb/host/xhci.c                            |   10 +
> >> >> >  drivers/usb/host/xhci.h                            |    1 +
> >> >> >  9 files changed, 1959 insertions(+)
> >> >> >  create mode 100644 Documentation/ABI/testing/sysfs-bus-pci-drivers-xhci_hcd
> >> >> >  create mode 100644 drivers/usb/host/xhci-dbgcap.c
> >> >> >  create mode 100644 drivers/usb/host/xhci-dbgcap.h
> >> >> >  create mode 100644 drivers/usb/host/xhci-dbgtty.c
> >> >> >
> >> >> 
> >> >> [snip]
> >> >> 
> >> >> > +#define DBC_VENDOR_ID			0x1d6b	/* Linux Foundation 0x1d6b */
> >> >> > +#define DBC_PRODUCT_ID			0x0004	/* device 0004 */
> >> >> >
> >> >> 
> >> >> The DbC (xHCI DeBug Capability) is an optional functionality in
> >> >> some xHCI host controllers. It will present a super-speed debug
> >> >> device through the debug port after it is enabled.
> >> >> 
> >> >> The DbC register set defines an interface for system software
> >> >> to specify the vendor id and product id of the debug device.
> >> >> These two values will be presented by the debug device in its
> >> >> device descriptor idVendor and idProduct fields.
> >> >> 
> >> >> Microsoft Windows have a well established protocol for
> >> >> debugging over DbC. And it assigns below values for its use.
> >> >> 
> >> >> USB\VID_045E&PID_062D.DeviceDesc="Microsoft USB Debug Target"
> >> >> 
> >> >> I'm going to use 0x1d6b/0x0004 value pair for DbC use in
> >> >> Linux. Do you approve me to do so?
> >> >
> >> > No.  Why can't you use the same ids as Windows?  This is implementing
> >> > the same protocol, right?
> >> 
> >> the protocol running on top is 100% vendor specific. More than likely,
> >> we would just run kgdb on top of this, right? We really don't support
> >> microsoft's debug architecture.
> >
> > Ah, I didn't know about the protocol specifics here, if it is
> > vendor-specific, then yes, we need our own id.
> 
> Great, thanks :-)
> 
> Let us know which one we're allowed to use and I'm sure Baolu can respin
> the patch in no time.

Can I get a "full" description of what string this device id will
reference?  Is it "Linux USB Debug Target" or something else?

> > As the above text said "well established protocol", I assumed we
> > implemented the same thing :)
> 
> It's "well established" from Microsoft's point of view :-) They have
> that same protocol running over USB, TCP, UDP...
> 
> It would be nice if we could ditch TTY and teach gdb about other
> transports, but then again, why bother if we can reuse code that already
> works? :-)

gdb knows about other transports, but I don't know about the kernel
interface to it :)

thanks,

greg k-h

^ permalink raw reply	[flat|nested] 21+ messages in thread

* Re: [PATCH v3 2/3] usb: xhci: Add DbC support in xHCI driver
  2017-11-02 16:51             ` Greg Kroah-Hartman
@ 2017-11-03  0:45               ` Lu Baolu
  2017-11-03  6:27                 ` Greg Kroah-Hartman
  0 siblings, 1 reply; 21+ messages in thread
From: Lu Baolu @ 2017-11-03  0:45 UTC (permalink / raw)
  To: Greg Kroah-Hartman, Felipe Balbi
  Cc: 'Mathias Nyman', linux-usb, linux-kernel

Hi,

On 11/03/2017 12:51 AM, Greg Kroah-Hartman wrote:
> On Thu, Nov 02, 2017 at 12:38:57PM +0200, Felipe Balbi wrote:
>> > 
>> > Hi,
>> > 
>> > Greg Kroah-Hartman <gregkh@linuxfoundation.org> writes:
>>>> > >> Greg Kroah-Hartman <gregkh@linuxfoundation.org> writes:
>>>>>>> > >> >> > xHCI compatible USB host controllers(i.e. super-speed USB3 controllers)
>>>>>>> > >> >> > can be implemented with the Debug Capability(DbC). It presents a debug
>>>>>>> > >> >> > device which is fully compliant with the USB framework and provides the
>>>>>>> > >> >> > equivalent of a very high performance full-duplex serial link. The debug
>>>>>>> > >> >> > capability operation model and registers interface are defined in 7.6.8
>>>>>>> > >> >> > of the xHCI specification, revision 1.1.
>>>>>>> > >> >> >
>>>>>>> > >> >> > The DbC debug device shares a root port with the xHCI host. By default,
>>>>>>> > >> >> > the debug capability is disabled and the root port is assigned to xHCI.
>>>>>>> > >> >> > When the DbC is enabled, the root port will be assigned to the DbC debug
>>>>>>> > >> >> > device, and the xHCI sees nothing on this port. This implementation uses
>>>>>>> > >> >> > a sysfs node named <dbc> under the xHCI device to manage the enabling
>>>>>>> > >> >> > and disabling of the debug capability.
>>>>>>> > >> >> >
>>>>>>> > >> >> > When the debug capability is enabled, it will present a debug device
>>>>>>> > >> >> > through the debug port. This debug device is fully compliant with the
>>>>>>> > >> >> > USB3 framework, and it can be enumerated by a debug host on the other
>>>>>>> > >> >> > end of the USB link. As soon as the debug device is configured, a TTY
>>>>>>> > >> >> > serial device named /dev/ttyDBC0 will be created.
>>>>>>> > >> >> >
>>>>>>> > >> >> > One use of this link is running a login service on the debug target.
>>>>>>> > >> >> > Hence it can be remote accessed by a debug host. Another use case can
>>>>>>> > >> >> > probably be found in servers. It provides a peer-to-peer USB link
>>>>>>> > >> >> > between two host-only machines. This provides a reasonable out-of-band
>>>>>>> > >> >> > communication method between two servers.
>>>>>>> > >> >> >
>>>>>>> > >> >> > Signed-off-by: Lu Baolu <baolu.lu@linux.intel.com>
>>>>>>> > >> >> > ---
>>>>>>> > >> >> >  .../ABI/testing/sysfs-bus-pci-drivers-xhci_hcd     |   25 +
>>>>>>> > >> >> >  drivers/usb/host/Kconfig                           |    9 +
>>>>>>> > >> >> >  drivers/usb/host/Makefile                          |    5 +
>>>>>>> > >> >> >  drivers/usb/host/xhci-dbgcap.c                     | 1016 ++++++++++++++++++++
>>>>>>> > >> >> >  drivers/usb/host/xhci-dbgcap.h                     |  247 +++++
>>>>>>> > >> >> >  drivers/usb/host/xhci-dbgtty.c                     |  586 +++++++++++
>>>>>>> > >> >> >  drivers/usb/host/xhci-trace.h                      |   60 ++
>>>>>>> > >> >> >  drivers/usb/host/xhci.c                            |   10 +
>>>>>>> > >> >> >  drivers/usb/host/xhci.h                            |    1 +
>>>>>>> > >> >> >  9 files changed, 1959 insertions(+)
>>>>>>> > >> >> >  create mode 100644 Documentation/ABI/testing/sysfs-bus-pci-drivers-xhci_hcd
>>>>>>> > >> >> >  create mode 100644 drivers/usb/host/xhci-dbgcap.c
>>>>>>> > >> >> >  create mode 100644 drivers/usb/host/xhci-dbgcap.h
>>>>>>> > >> >> >  create mode 100644 drivers/usb/host/xhci-dbgtty.c
>>>>>>> > >> >> >
>>>>>> > >> >> 
>>>>>> > >> >> [snip]
>>>>>> > >> >> 
>>>>>>> > >> >> > +#define DBC_VENDOR_ID			0x1d6b	/* Linux Foundation 0x1d6b */
>>>>>>> > >> >> > +#define DBC_PRODUCT_ID			0x0004	/* device 0004 */
>>>>>>> > >> >> >
>>>>>> > >> >> 
>>>>>> > >> >> The DbC (xHCI DeBug Capability) is an optional functionality in
>>>>>> > >> >> some xHCI host controllers. It will present a super-speed debug
>>>>>> > >> >> device through the debug port after it is enabled.
>>>>>> > >> >> 
>>>>>> > >> >> The DbC register set defines an interface for system software
>>>>>> > >> >> to specify the vendor id and product id of the debug device.
>>>>>> > >> >> These two values will be presented by the debug device in its
>>>>>> > >> >> device descriptor idVendor and idProduct fields.
>>>>>> > >> >> 
>>>>>> > >> >> Microsoft Windows have a well established protocol for
>>>>>> > >> >> debugging over DbC. And it assigns below values for its use.
>>>>>> > >> >> 
>>>>>> > >> >> USB\VID_045E&PID_062D.DeviceDesc="Microsoft USB Debug Target"
>>>>>> > >> >> 
>>>>>> > >> >> I'm going to use 0x1d6b/0x0004 value pair for DbC use in
>>>>>> > >> >> Linux. Do you approve me to do so?
>>>>> > >> >
>>>>> > >> > No.  Why can't you use the same ids as Windows?  This is implementing
>>>>> > >> > the same protocol, right?
>>>> > >> 
>>>> > >> the protocol running on top is 100% vendor specific. More than likely,
>>>> > >> we would just run kgdb on top of this, right? We really don't support
>>>> > >> microsoft's debug architecture.
>>> > >
>>> > > Ah, I didn't know about the protocol specifics here, if it is
>>> > > vendor-specific, then yes, we need our own id.
>> > 
>> > Great, thanks :-)
>> > 
>> > Let us know which one we're allowed to use and I'm sure Baolu can respin
>> > the patch in no time.
> Can I get a "full" description of what string this device id will
> reference?  Is it "Linux USB Debug Target" or something else?
>

Current manufacturer and product strings are set like this.

+#define DBC_STRING_MANUFACTURER	"Linux"
+#define DBC_STRING_PRODUCT		"Remote	GDB"

These are also place holders. We can change them to more meaningful strings.

Best regards,
Lu Baolu

^ permalink raw reply	[flat|nested] 21+ messages in thread

* Re: [PATCH v3 2/3] usb: xhci: Add DbC support in xHCI driver
  2017-11-03  0:45               ` Lu Baolu
@ 2017-11-03  6:27                 ` Greg Kroah-Hartman
  2017-11-03 10:52                   ` Felipe Balbi
  2017-11-06  0:35                   ` Lu Baolu
  0 siblings, 2 replies; 21+ messages in thread
From: Greg Kroah-Hartman @ 2017-11-03  6:27 UTC (permalink / raw)
  To: Lu Baolu; +Cc: Felipe Balbi, 'Mathias Nyman', linux-usb, linux-kernel

On Fri, Nov 03, 2017 at 08:45:46AM +0800, Lu Baolu wrote:
> Hi,
> 
> On 11/03/2017 12:51 AM, Greg Kroah-Hartman wrote:
> > On Thu, Nov 02, 2017 at 12:38:57PM +0200, Felipe Balbi wrote:
> >> > 
> >> > Hi,
> >> > 
> >> > Greg Kroah-Hartman <gregkh@linuxfoundation.org> writes:
> >>>> > >> Greg Kroah-Hartman <gregkh@linuxfoundation.org> writes:
> >>>>>>> > >> >> > xHCI compatible USB host controllers(i.e. super-speed USB3 controllers)
> >>>>>>> > >> >> > can be implemented with the Debug Capability(DbC). It presents a debug
> >>>>>>> > >> >> > device which is fully compliant with the USB framework and provides the
> >>>>>>> > >> >> > equivalent of a very high performance full-duplex serial link. The debug
> >>>>>>> > >> >> > capability operation model and registers interface are defined in 7.6.8
> >>>>>>> > >> >> > of the xHCI specification, revision 1.1.
> >>>>>>> > >> >> >
> >>>>>>> > >> >> > The DbC debug device shares a root port with the xHCI host. By default,
> >>>>>>> > >> >> > the debug capability is disabled and the root port is assigned to xHCI.
> >>>>>>> > >> >> > When the DbC is enabled, the root port will be assigned to the DbC debug
> >>>>>>> > >> >> > device, and the xHCI sees nothing on this port. This implementation uses
> >>>>>>> > >> >> > a sysfs node named <dbc> under the xHCI device to manage the enabling
> >>>>>>> > >> >> > and disabling of the debug capability.
> >>>>>>> > >> >> >
> >>>>>>> > >> >> > When the debug capability is enabled, it will present a debug device
> >>>>>>> > >> >> > through the debug port. This debug device is fully compliant with the
> >>>>>>> > >> >> > USB3 framework, and it can be enumerated by a debug host on the other
> >>>>>>> > >> >> > end of the USB link. As soon as the debug device is configured, a TTY
> >>>>>>> > >> >> > serial device named /dev/ttyDBC0 will be created.
> >>>>>>> > >> >> >
> >>>>>>> > >> >> > One use of this link is running a login service on the debug target.
> >>>>>>> > >> >> > Hence it can be remote accessed by a debug host. Another use case can
> >>>>>>> > >> >> > probably be found in servers. It provides a peer-to-peer USB link
> >>>>>>> > >> >> > between two host-only machines. This provides a reasonable out-of-band
> >>>>>>> > >> >> > communication method between two servers.
> >>>>>>> > >> >> >
> >>>>>>> > >> >> > Signed-off-by: Lu Baolu <baolu.lu@linux.intel.com>
> >>>>>>> > >> >> > ---
> >>>>>>> > >> >> >  .../ABI/testing/sysfs-bus-pci-drivers-xhci_hcd     |   25 +
> >>>>>>> > >> >> >  drivers/usb/host/Kconfig                           |    9 +
> >>>>>>> > >> >> >  drivers/usb/host/Makefile                          |    5 +
> >>>>>>> > >> >> >  drivers/usb/host/xhci-dbgcap.c                     | 1016 ++++++++++++++++++++
> >>>>>>> > >> >> >  drivers/usb/host/xhci-dbgcap.h                     |  247 +++++
> >>>>>>> > >> >> >  drivers/usb/host/xhci-dbgtty.c                     |  586 +++++++++++
> >>>>>>> > >> >> >  drivers/usb/host/xhci-trace.h                      |   60 ++
> >>>>>>> > >> >> >  drivers/usb/host/xhci.c                            |   10 +
> >>>>>>> > >> >> >  drivers/usb/host/xhci.h                            |    1 +
> >>>>>>> > >> >> >  9 files changed, 1959 insertions(+)
> >>>>>>> > >> >> >  create mode 100644 Documentation/ABI/testing/sysfs-bus-pci-drivers-xhci_hcd
> >>>>>>> > >> >> >  create mode 100644 drivers/usb/host/xhci-dbgcap.c
> >>>>>>> > >> >> >  create mode 100644 drivers/usb/host/xhci-dbgcap.h
> >>>>>>> > >> >> >  create mode 100644 drivers/usb/host/xhci-dbgtty.c
> >>>>>>> > >> >> >
> >>>>>> > >> >> 
> >>>>>> > >> >> [snip]
> >>>>>> > >> >> 
> >>>>>>> > >> >> > +#define DBC_VENDOR_ID			0x1d6b	/* Linux Foundation 0x1d6b */
> >>>>>>> > >> >> > +#define DBC_PRODUCT_ID			0x0004	/* device 0004 */
> >>>>>>> > >> >> >
> >>>>>> > >> >> 
> >>>>>> > >> >> The DbC (xHCI DeBug Capability) is an optional functionality in
> >>>>>> > >> >> some xHCI host controllers. It will present a super-speed debug
> >>>>>> > >> >> device through the debug port after it is enabled.
> >>>>>> > >> >> 
> >>>>>> > >> >> The DbC register set defines an interface for system software
> >>>>>> > >> >> to specify the vendor id and product id of the debug device.
> >>>>>> > >> >> These two values will be presented by the debug device in its
> >>>>>> > >> >> device descriptor idVendor and idProduct fields.
> >>>>>> > >> >> 
> >>>>>> > >> >> Microsoft Windows have a well established protocol for
> >>>>>> > >> >> debugging over DbC. And it assigns below values for its use.
> >>>>>> > >> >> 
> >>>>>> > >> >> USB\VID_045E&PID_062D.DeviceDesc="Microsoft USB Debug Target"
> >>>>>> > >> >> 
> >>>>>> > >> >> I'm going to use 0x1d6b/0x0004 value pair for DbC use in
> >>>>>> > >> >> Linux. Do you approve me to do so?
> >>>>> > >> >
> >>>>> > >> > No.  Why can't you use the same ids as Windows?  This is implementing
> >>>>> > >> > the same protocol, right?
> >>>> > >> 
> >>>> > >> the protocol running on top is 100% vendor specific. More than likely,
> >>>> > >> we would just run kgdb on top of this, right? We really don't support
> >>>> > >> microsoft's debug architecture.
> >>> > >
> >>> > > Ah, I didn't know about the protocol specifics here, if it is
> >>> > > vendor-specific, then yes, we need our own id.
> >> > 
> >> > Great, thanks :-)
> >> > 
> >> > Let us know which one we're allowed to use and I'm sure Baolu can respin
> >> > the patch in no time.
> > Can I get a "full" description of what string this device id will
> > reference?  Is it "Linux USB Debug Target" or something else?
> >
> 
> Current manufacturer and product strings are set like this.
> 
> +#define DBC_STRING_MANUFACTURER	"Linux"
> +#define DBC_STRING_PRODUCT		"Remote	GDB"
> 
> These are also place holders. We can change them to more meaningful strings.

As the vendor id is assigned to "Linux Foundation" can we change that
string please?

And why not match what Microsoft does here, "USB Debug Target" makes
more sense than "Remote GDB", right?

If so, please use device id 0x0010, I've reserved that for the driver
now.

thanks,

greg k-h

^ permalink raw reply	[flat|nested] 21+ messages in thread

* Re: [PATCH v3 2/3] usb: xhci: Add DbC support in xHCI driver
  2017-11-03  6:27                 ` Greg Kroah-Hartman
@ 2017-11-03 10:52                   ` Felipe Balbi
  2017-11-06  0:35                   ` Lu Baolu
  1 sibling, 0 replies; 21+ messages in thread
From: Felipe Balbi @ 2017-11-03 10:52 UTC (permalink / raw)
  To: Greg Kroah-Hartman, Lu Baolu
  Cc: 'Mathias Nyman', linux-usb, linux-kernel

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


Hi,

Greg Kroah-Hartman <gregkh@linuxfoundation.org> writes:
>> >>>> > >> Greg Kroah-Hartman <gregkh@linuxfoundation.org> writes:
>> >>>>>>> > >> >> > xHCI compatible USB host controllers(i.e. super-speed USB3 controllers)
>> >>>>>>> > >> >> > can be implemented with the Debug Capability(DbC). It presents a debug
>> >>>>>>> > >> >> > device which is fully compliant with the USB framework and provides the
>> >>>>>>> > >> >> > equivalent of a very high performance full-duplex serial link. The debug
>> >>>>>>> > >> >> > capability operation model and registers interface are defined in 7.6.8
>> >>>>>>> > >> >> > of the xHCI specification, revision 1.1.
>> >>>>>>> > >> >> >
>> >>>>>>> > >> >> > The DbC debug device shares a root port with the xHCI host. By default,
>> >>>>>>> > >> >> > the debug capability is disabled and the root port is assigned to xHCI.
>> >>>>>>> > >> >> > When the DbC is enabled, the root port will be assigned to the DbC debug
>> >>>>>>> > >> >> > device, and the xHCI sees nothing on this port. This implementation uses
>> >>>>>>> > >> >> > a sysfs node named <dbc> under the xHCI device to manage the enabling
>> >>>>>>> > >> >> > and disabling of the debug capability.
>> >>>>>>> > >> >> >
>> >>>>>>> > >> >> > When the debug capability is enabled, it will present a debug device
>> >>>>>>> > >> >> > through the debug port. This debug device is fully compliant with the
>> >>>>>>> > >> >> > USB3 framework, and it can be enumerated by a debug host on the other
>> >>>>>>> > >> >> > end of the USB link. As soon as the debug device is configured, a TTY
>> >>>>>>> > >> >> > serial device named /dev/ttyDBC0 will be created.
>> >>>>>>> > >> >> >
>> >>>>>>> > >> >> > One use of this link is running a login service on the debug target.
>> >>>>>>> > >> >> > Hence it can be remote accessed by a debug host. Another use case can
>> >>>>>>> > >> >> > probably be found in servers. It provides a peer-to-peer USB link
>> >>>>>>> > >> >> > between two host-only machines. This provides a reasonable out-of-band
>> >>>>>>> > >> >> > communication method between two servers.
>> >>>>>>> > >> >> >
>> >>>>>>> > >> >> > Signed-off-by: Lu Baolu <baolu.lu@linux.intel.com>
>> >>>>>>> > >> >> > ---
>> >>>>>>> > >> >> >  .../ABI/testing/sysfs-bus-pci-drivers-xhci_hcd     |   25 +
>> >>>>>>> > >> >> >  drivers/usb/host/Kconfig                           |    9 +
>> >>>>>>> > >> >> >  drivers/usb/host/Makefile                          |    5 +
>> >>>>>>> > >> >> >  drivers/usb/host/xhci-dbgcap.c                     | 1016 ++++++++++++++++++++
>> >>>>>>> > >> >> >  drivers/usb/host/xhci-dbgcap.h                     |  247 +++++
>> >>>>>>> > >> >> >  drivers/usb/host/xhci-dbgtty.c                     |  586 +++++++++++
>> >>>>>>> > >> >> >  drivers/usb/host/xhci-trace.h                      |   60 ++
>> >>>>>>> > >> >> >  drivers/usb/host/xhci.c                            |   10 +
>> >>>>>>> > >> >> >  drivers/usb/host/xhci.h                            |    1 +
>> >>>>>>> > >> >> >  9 files changed, 1959 insertions(+)
>> >>>>>>> > >> >> >  create mode 100644 Documentation/ABI/testing/sysfs-bus-pci-drivers-xhci_hcd
>> >>>>>>> > >> >> >  create mode 100644 drivers/usb/host/xhci-dbgcap.c
>> >>>>>>> > >> >> >  create mode 100644 drivers/usb/host/xhci-dbgcap.h
>> >>>>>>> > >> >> >  create mode 100644 drivers/usb/host/xhci-dbgtty.c
>> >>>>>>> > >> >> >
>> >>>>>> > >> >> 
>> >>>>>> > >> >> [snip]
>> >>>>>> > >> >> 
>> >>>>>>> > >> >> > +#define DBC_VENDOR_ID			0x1d6b	/* Linux Foundation 0x1d6b */
>> >>>>>>> > >> >> > +#define DBC_PRODUCT_ID			0x0004	/* device 0004 */
>> >>>>>>> > >> >> >
>> >>>>>> > >> >> 
>> >>>>>> > >> >> The DbC (xHCI DeBug Capability) is an optional functionality in
>> >>>>>> > >> >> some xHCI host controllers. It will present a super-speed debug
>> >>>>>> > >> >> device through the debug port after it is enabled.
>> >>>>>> > >> >> 
>> >>>>>> > >> >> The DbC register set defines an interface for system software
>> >>>>>> > >> >> to specify the vendor id and product id of the debug device.
>> >>>>>> > >> >> These two values will be presented by the debug device in its
>> >>>>>> > >> >> device descriptor idVendor and idProduct fields.
>> >>>>>> > >> >> 
>> >>>>>> > >> >> Microsoft Windows have a well established protocol for
>> >>>>>> > >> >> debugging over DbC. And it assigns below values for its use.
>> >>>>>> > >> >> 
>> >>>>>> > >> >> USB\VID_045E&PID_062D.DeviceDesc="Microsoft USB Debug Target"
>> >>>>>> > >> >> 
>> >>>>>> > >> >> I'm going to use 0x1d6b/0x0004 value pair for DbC use in
>> >>>>>> > >> >> Linux. Do you approve me to do so?
>> >>>>> > >> >
>> >>>>> > >> > No.  Why can't you use the same ids as Windows?  This is implementing
>> >>>>> > >> > the same protocol, right?
>> >>>> > >> 
>> >>>> > >> the protocol running on top is 100% vendor specific. More than likely,
>> >>>> > >> we would just run kgdb on top of this, right? We really don't support
>> >>>> > >> microsoft's debug architecture.
>> >>> > >
>> >>> > > Ah, I didn't know about the protocol specifics here, if it is
>> >>> > > vendor-specific, then yes, we need our own id.
>> >> > 
>> >> > Great, thanks :-)
>> >> > 
>> >> > Let us know which one we're allowed to use and I'm sure Baolu can respin
>> >> > the patch in no time.
>> > Can I get a "full" description of what string this device id will
>> > reference?  Is it "Linux USB Debug Target" or something else?
>> >
>> 
>> Current manufacturer and product strings are set like this.
>> 
>> +#define DBC_STRING_MANUFACTURER	"Linux"
>> +#define DBC_STRING_PRODUCT		"Remote	GDB"
>> 
>> These are also place holders. We can change them to more meaningful strings.
>
> As the vendor id is assigned to "Linux Foundation" can we change that
> string please?
>
> And why not match what Microsoft does here, "USB Debug Target" makes
> more sense than "Remote GDB", right?

I agree. It just happens to talk remote GDB protocol through KGDB, but
we could run KDB directly or, even, come up with a new protocol.

> If so, please use device id 0x0010, I've reserved that for the driver
> now.

thanks Greg

-- 
balbi

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 832 bytes --]

^ permalink raw reply	[flat|nested] 21+ messages in thread

* Re: [PATCH v3 2/3] usb: xhci: Add DbC support in xHCI driver
  2017-11-03  6:27                 ` Greg Kroah-Hartman
  2017-11-03 10:52                   ` Felipe Balbi
@ 2017-11-06  0:35                   ` Lu Baolu
  2017-11-06  8:00                     ` Greg Kroah-Hartman
  1 sibling, 1 reply; 21+ messages in thread
From: Lu Baolu @ 2017-11-06  0:35 UTC (permalink / raw)
  To: Greg Kroah-Hartman
  Cc: Felipe Balbi, 'Mathias Nyman', linux-usb, linux-kernel

Hi,

On 11/03/2017 02:27 PM, Greg Kroah-Hartman wrote:
> On Fri, Nov 03, 2017 at 08:45:46AM +0800, Lu Baolu wrote:
>> Hi,
>>
>> On 11/03/2017 12:51 AM, Greg Kroah-Hartman wrote:
>>> On Thu, Nov 02, 2017 at 12:38:57PM +0200, Felipe Balbi wrote:
>>>>> Hi,
>>>>>
>>>>> Greg Kroah-Hartman <gregkh@linuxfoundation.org> writes:
>>>>>>>>> Greg Kroah-Hartman <gregkh@linuxfoundation.org> writes:
>>>>>>>>>>>>>>> xHCI compatible USB host controllers(i.e. super-speed USB3 controllers)
>>>>>>>>>>>>>>> can be implemented with the Debug Capability(DbC). It presents a debug
>>>>>>>>>>>>>>> device which is fully compliant with the USB framework and provides the
>>>>>>>>>>>>>>> equivalent of a very high performance full-duplex serial link. The debug
>>>>>>>>>>>>>>> capability operation model and registers interface are defined in 7.6.8
>>>>>>>>>>>>>>> of the xHCI specification, revision 1.1.
>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>> The DbC debug device shares a root port with the xHCI host. By default,
>>>>>>>>>>>>>>> the debug capability is disabled and the root port is assigned to xHCI.
>>>>>>>>>>>>>>> When the DbC is enabled, the root port will be assigned to the DbC debug
>>>>>>>>>>>>>>> device, and the xHCI sees nothing on this port. This implementation uses
>>>>>>>>>>>>>>> a sysfs node named <dbc> under the xHCI device to manage the enabling
>>>>>>>>>>>>>>> and disabling of the debug capability.
>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>> When the debug capability is enabled, it will present a debug device
>>>>>>>>>>>>>>> through the debug port. This debug device is fully compliant with the
>>>>>>>>>>>>>>> USB3 framework, and it can be enumerated by a debug host on the other
>>>>>>>>>>>>>>> end of the USB link. As soon as the debug device is configured, a TTY
>>>>>>>>>>>>>>> serial device named /dev/ttyDBC0 will be created.
>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>> One use of this link is running a login service on the debug target.
>>>>>>>>>>>>>>> Hence it can be remote accessed by a debug host. Another use case can
>>>>>>>>>>>>>>> probably be found in servers. It provides a peer-to-peer USB link
>>>>>>>>>>>>>>> between two host-only machines. This provides a reasonable out-of-band
>>>>>>>>>>>>>>> communication method between two servers.
>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>> Signed-off-by: Lu Baolu <baolu.lu@linux.intel.com>
>>>>>>>>>>>>>>> ---
>>>>>>>>>>>>>>>  .../ABI/testing/sysfs-bus-pci-drivers-xhci_hcd     |   25 +
>>>>>>>>>>>>>>>  drivers/usb/host/Kconfig                           |    9 +
>>>>>>>>>>>>>>>  drivers/usb/host/Makefile                          |    5 +
>>>>>>>>>>>>>>>  drivers/usb/host/xhci-dbgcap.c                     | 1016 ++++++++++++++++++++
>>>>>>>>>>>>>>>  drivers/usb/host/xhci-dbgcap.h                     |  247 +++++
>>>>>>>>>>>>>>>  drivers/usb/host/xhci-dbgtty.c                     |  586 +++++++++++
>>>>>>>>>>>>>>>  drivers/usb/host/xhci-trace.h                      |   60 ++
>>>>>>>>>>>>>>>  drivers/usb/host/xhci.c                            |   10 +
>>>>>>>>>>>>>>>  drivers/usb/host/xhci.h                            |    1 +
>>>>>>>>>>>>>>>  9 files changed, 1959 insertions(+)
>>>>>>>>>>>>>>>  create mode 100644 Documentation/ABI/testing/sysfs-bus-pci-drivers-xhci_hcd
>>>>>>>>>>>>>>>  create mode 100644 drivers/usb/host/xhci-dbgcap.c
>>>>>>>>>>>>>>>  create mode 100644 drivers/usb/host/xhci-dbgcap.h
>>>>>>>>>>>>>>>  create mode 100644 drivers/usb/host/xhci-dbgtty.c
>>>>>>>>>>>>>>>
>>>>>>>>>>>>> [snip]
>>>>>>>>>>>>>
>>>>>>>>>>>>>>> +#define DBC_VENDOR_ID			0x1d6b	/* Linux Foundation 0x1d6b */
>>>>>>>>>>>>>>> +#define DBC_PRODUCT_ID			0x0004	/* device 0004 */
>>>>>>>>>>>>>>>
>>>>>>>>>>>>> The DbC (xHCI DeBug Capability) is an optional functionality in
>>>>>>>>>>>>> some xHCI host controllers. It will present a super-speed debug
>>>>>>>>>>>>> device through the debug port after it is enabled.
>>>>>>>>>>>>>
>>>>>>>>>>>>> The DbC register set defines an interface for system software
>>>>>>>>>>>>> to specify the vendor id and product id of the debug device.
>>>>>>>>>>>>> These two values will be presented by the debug device in its
>>>>>>>>>>>>> device descriptor idVendor and idProduct fields.
>>>>>>>>>>>>>
>>>>>>>>>>>>> Microsoft Windows have a well established protocol for
>>>>>>>>>>>>> debugging over DbC. And it assigns below values for its use.
>>>>>>>>>>>>>
>>>>>>>>>>>>> USB\VID_045E&PID_062D.DeviceDesc="Microsoft USB Debug Target"
>>>>>>>>>>>>>
>>>>>>>>>>>>> I'm going to use 0x1d6b/0x0004 value pair for DbC use in
>>>>>>>>>>>>> Linux. Do you approve me to do so?
>>>>>>>>>>> No.  Why can't you use the same ids as Windows?  This is implementing
>>>>>>>>>>> the same protocol, right?
>>>>>>>>> the protocol running on top is 100% vendor specific. More than likely,
>>>>>>>>> we would just run kgdb on top of this, right? We really don't support
>>>>>>>>> microsoft's debug architecture.
>>>>>>> Ah, I didn't know about the protocol specifics here, if it is
>>>>>>> vendor-specific, then yes, we need our own id.
>>>>> Great, thanks :-)
>>>>>
>>>>> Let us know which one we're allowed to use and I'm sure Baolu can respin
>>>>> the patch in no time.
>>> Can I get a "full" description of what string this device id will
>>> reference?  Is it "Linux USB Debug Target" or something else?
>>>
>> Current manufacturer and product strings are set like this.
>>
>> +#define DBC_STRING_MANUFACTURER	"Linux"
>> +#define DBC_STRING_PRODUCT		"Remote	GDB"
>>
>> These are also place holders. We can change them to more meaningful strings.
> As the vendor id is assigned to "Linux Foundation" can we change that
> string please?
>
> And why not match what Microsoft does here, "USB Debug Target" makes
> more sense than "Remote GDB", right?
>
> If so, please use device id 0x0010, I've reserved that for the driver
> now.
>

I will change the strings and use the allocated product id.
Thank you very much.

Another DbC usage is to enable DbC during early boot time,
and run GDB protocol data or early printk message over it.
The early printk implementation has been merged in 4.12-rc1.

commit aeb9dd1 <usb/early: Add driver for xhci debug capability>

In this code, the DbC product id is still a place holder (0x0004).
I am really sorry. I should apply a valid product id from you during
the code review time. But I forgot it since almost all attention was
paid to early driver itself at that time.

We could use the same ID as you allocated here or, preferably,
use a different ID since we're dealing with different functionality. 

Best regards,
Lu Baolu

^ permalink raw reply	[flat|nested] 21+ messages in thread

* Re: [PATCH v3 2/3] usb: xhci: Add DbC support in xHCI driver
  2017-11-06  0:35                   ` Lu Baolu
@ 2017-11-06  8:00                     ` Greg Kroah-Hartman
  2017-11-07  2:32                       ` Lu Baolu
  0 siblings, 1 reply; 21+ messages in thread
From: Greg Kroah-Hartman @ 2017-11-06  8:00 UTC (permalink / raw)
  To: Lu Baolu; +Cc: Felipe Balbi, 'Mathias Nyman', linux-usb, linux-kernel

On Mon, Nov 06, 2017 at 08:35:41AM +0800, Lu Baolu wrote:
> Hi,
> 
> On 11/03/2017 02:27 PM, Greg Kroah-Hartman wrote:
> > On Fri, Nov 03, 2017 at 08:45:46AM +0800, Lu Baolu wrote:
> >> Hi,
> >>
> >> On 11/03/2017 12:51 AM, Greg Kroah-Hartman wrote:
> >>> On Thu, Nov 02, 2017 at 12:38:57PM +0200, Felipe Balbi wrote:
> >>>>> Hi,
> >>>>>
> >>>>> Greg Kroah-Hartman <gregkh@linuxfoundation.org> writes:
> >>>>>>>>> Greg Kroah-Hartman <gregkh@linuxfoundation.org> writes:
> >>>>>>>>>>>>>>> xHCI compatible USB host controllers(i.e. super-speed USB3 controllers)
> >>>>>>>>>>>>>>> can be implemented with the Debug Capability(DbC). It presents a debug
> >>>>>>>>>>>>>>> device which is fully compliant with the USB framework and provides the
> >>>>>>>>>>>>>>> equivalent of a very high performance full-duplex serial link. The debug
> >>>>>>>>>>>>>>> capability operation model and registers interface are defined in 7.6.8
> >>>>>>>>>>>>>>> of the xHCI specification, revision 1.1.
> >>>>>>>>>>>>>>>
> >>>>>>>>>>>>>>> The DbC debug device shares a root port with the xHCI host. By default,
> >>>>>>>>>>>>>>> the debug capability is disabled and the root port is assigned to xHCI.
> >>>>>>>>>>>>>>> When the DbC is enabled, the root port will be assigned to the DbC debug
> >>>>>>>>>>>>>>> device, and the xHCI sees nothing on this port. This implementation uses
> >>>>>>>>>>>>>>> a sysfs node named <dbc> under the xHCI device to manage the enabling
> >>>>>>>>>>>>>>> and disabling of the debug capability.
> >>>>>>>>>>>>>>>
> >>>>>>>>>>>>>>> When the debug capability is enabled, it will present a debug device
> >>>>>>>>>>>>>>> through the debug port. This debug device is fully compliant with the
> >>>>>>>>>>>>>>> USB3 framework, and it can be enumerated by a debug host on the other
> >>>>>>>>>>>>>>> end of the USB link. As soon as the debug device is configured, a TTY
> >>>>>>>>>>>>>>> serial device named /dev/ttyDBC0 will be created.
> >>>>>>>>>>>>>>>
> >>>>>>>>>>>>>>> One use of this link is running a login service on the debug target.
> >>>>>>>>>>>>>>> Hence it can be remote accessed by a debug host. Another use case can
> >>>>>>>>>>>>>>> probably be found in servers. It provides a peer-to-peer USB link
> >>>>>>>>>>>>>>> between two host-only machines. This provides a reasonable out-of-band
> >>>>>>>>>>>>>>> communication method between two servers.
> >>>>>>>>>>>>>>>
> >>>>>>>>>>>>>>> Signed-off-by: Lu Baolu <baolu.lu@linux.intel.com>
> >>>>>>>>>>>>>>> ---
> >>>>>>>>>>>>>>>  .../ABI/testing/sysfs-bus-pci-drivers-xhci_hcd     |   25 +
> >>>>>>>>>>>>>>>  drivers/usb/host/Kconfig                           |    9 +
> >>>>>>>>>>>>>>>  drivers/usb/host/Makefile                          |    5 +
> >>>>>>>>>>>>>>>  drivers/usb/host/xhci-dbgcap.c                     | 1016 ++++++++++++++++++++
> >>>>>>>>>>>>>>>  drivers/usb/host/xhci-dbgcap.h                     |  247 +++++
> >>>>>>>>>>>>>>>  drivers/usb/host/xhci-dbgtty.c                     |  586 +++++++++++
> >>>>>>>>>>>>>>>  drivers/usb/host/xhci-trace.h                      |   60 ++
> >>>>>>>>>>>>>>>  drivers/usb/host/xhci.c                            |   10 +
> >>>>>>>>>>>>>>>  drivers/usb/host/xhci.h                            |    1 +
> >>>>>>>>>>>>>>>  9 files changed, 1959 insertions(+)
> >>>>>>>>>>>>>>>  create mode 100644 Documentation/ABI/testing/sysfs-bus-pci-drivers-xhci_hcd
> >>>>>>>>>>>>>>>  create mode 100644 drivers/usb/host/xhci-dbgcap.c
> >>>>>>>>>>>>>>>  create mode 100644 drivers/usb/host/xhci-dbgcap.h
> >>>>>>>>>>>>>>>  create mode 100644 drivers/usb/host/xhci-dbgtty.c
> >>>>>>>>>>>>>>>
> >>>>>>>>>>>>> [snip]
> >>>>>>>>>>>>>
> >>>>>>>>>>>>>>> +#define DBC_VENDOR_ID			0x1d6b	/* Linux Foundation 0x1d6b */
> >>>>>>>>>>>>>>> +#define DBC_PRODUCT_ID			0x0004	/* device 0004 */
> >>>>>>>>>>>>>>>
> >>>>>>>>>>>>> The DbC (xHCI DeBug Capability) is an optional functionality in
> >>>>>>>>>>>>> some xHCI host controllers. It will present a super-speed debug
> >>>>>>>>>>>>> device through the debug port after it is enabled.
> >>>>>>>>>>>>>
> >>>>>>>>>>>>> The DbC register set defines an interface for system software
> >>>>>>>>>>>>> to specify the vendor id and product id of the debug device.
> >>>>>>>>>>>>> These two values will be presented by the debug device in its
> >>>>>>>>>>>>> device descriptor idVendor and idProduct fields.
> >>>>>>>>>>>>>
> >>>>>>>>>>>>> Microsoft Windows have a well established protocol for
> >>>>>>>>>>>>> debugging over DbC. And it assigns below values for its use.
> >>>>>>>>>>>>>
> >>>>>>>>>>>>> USB\VID_045E&PID_062D.DeviceDesc="Microsoft USB Debug Target"
> >>>>>>>>>>>>>
> >>>>>>>>>>>>> I'm going to use 0x1d6b/0x0004 value pair for DbC use in
> >>>>>>>>>>>>> Linux. Do you approve me to do so?
> >>>>>>>>>>> No.  Why can't you use the same ids as Windows?  This is implementing
> >>>>>>>>>>> the same protocol, right?
> >>>>>>>>> the protocol running on top is 100% vendor specific. More than likely,
> >>>>>>>>> we would just run kgdb on top of this, right? We really don't support
> >>>>>>>>> microsoft's debug architecture.
> >>>>>>> Ah, I didn't know about the protocol specifics here, if it is
> >>>>>>> vendor-specific, then yes, we need our own id.
> >>>>> Great, thanks :-)
> >>>>>
> >>>>> Let us know which one we're allowed to use and I'm sure Baolu can respin
> >>>>> the patch in no time.
> >>> Can I get a "full" description of what string this device id will
> >>> reference?  Is it "Linux USB Debug Target" or something else?
> >>>
> >> Current manufacturer and product strings are set like this.
> >>
> >> +#define DBC_STRING_MANUFACTURER	"Linux"
> >> +#define DBC_STRING_PRODUCT		"Remote	GDB"
> >>
> >> These are also place holders. We can change them to more meaningful strings.
> > As the vendor id is assigned to "Linux Foundation" can we change that
> > string please?
> >
> > And why not match what Microsoft does here, "USB Debug Target" makes
> > more sense than "Remote GDB", right?
> >
> > If so, please use device id 0x0010, I've reserved that for the driver
> > now.
> >
> 
> I will change the strings and use the allocated product id.
> Thank you very much.
> 
> Another DbC usage is to enable DbC during early boot time,
> and run GDB protocol data or early printk message over it.
> The early printk implementation has been merged in 4.12-rc1.
> 
> commit aeb9dd1 <usb/early: Add driver for xhci debug capability>
> 
> In this code, the DbC product id is still a place holder (0x0004).
> I am really sorry. I should apply a valid product id from you during
> the code review time. But I forgot it since almost all attention was
> paid to early driver itself at that time.
> 
> We could use the same ID as you allocated here or, preferably,
> use a different ID since we're dealing with different functionality. 

A different one would be best, please give me a good device string to
use and I will assign you a new ID, probably 0011.  Also send a patch
now for this please.

thanks,

greg k-h

^ permalink raw reply	[flat|nested] 21+ messages in thread

* Re: [PATCH v3 2/3] usb: xhci: Add DbC support in xHCI driver
  2017-11-06  8:00                     ` Greg Kroah-Hartman
@ 2017-11-07  2:32                       ` Lu Baolu
  0 siblings, 0 replies; 21+ messages in thread
From: Lu Baolu @ 2017-11-07  2:32 UTC (permalink / raw)
  To: Greg Kroah-Hartman
  Cc: Felipe Balbi, 'Mathias Nyman', linux-usb, linux-kernel

Hi,

On 11/06/2017 04:00 PM, Greg Kroah-Hartman wrote:
> On Mon, Nov 06, 2017 at 08:35:41AM +0800, Lu Baolu wrote:
>> Hi,
>>
>> On 11/03/2017 02:27 PM, Greg Kroah-Hartman wrote:
>>> On Fri, Nov 03, 2017 at 08:45:46AM +0800, Lu Baolu wrote:
>>>> Hi,
>>>>
>>>> On 11/03/2017 12:51 AM, Greg Kroah-Hartman wrote:
>>>>> On Thu, Nov 02, 2017 at 12:38:57PM +0200, Felipe Balbi wrote:
>>>>>>> Hi,
>>>>>>>
>>>>>>> Greg Kroah-Hartman <gregkh@linuxfoundation.org> writes:
>>>>>>>>>>> Greg Kroah-Hartman <gregkh@linuxfoundation.org> writes:
>>>>>>>>>>>>>>>>> xHCI compatible USB host controllers(i.e. super-speed USB3 controllers)
>>>>>>>>>>>>>>>>> can be implemented with the Debug Capability(DbC). It presents a debug
>>>>>>>>>>>>>>>>> device which is fully compliant with the USB framework and provides the
>>>>>>>>>>>>>>>>> equivalent of a very high performance full-duplex serial link. The debug
>>>>>>>>>>>>>>>>> capability operation model and registers interface are defined in 7.6.8
>>>>>>>>>>>>>>>>> of the xHCI specification, revision 1.1.
>>>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>>>> The DbC debug device shares a root port with the xHCI host. By default,
>>>>>>>>>>>>>>>>> the debug capability is disabled and the root port is assigned to xHCI.
>>>>>>>>>>>>>>>>> When the DbC is enabled, the root port will be assigned to the DbC debug
>>>>>>>>>>>>>>>>> device, and the xHCI sees nothing on this port. This implementation uses
>>>>>>>>>>>>>>>>> a sysfs node named <dbc> under the xHCI device to manage the enabling
>>>>>>>>>>>>>>>>> and disabling of the debug capability.
>>>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>>>> When the debug capability is enabled, it will present a debug device
>>>>>>>>>>>>>>>>> through the debug port. This debug device is fully compliant with the
>>>>>>>>>>>>>>>>> USB3 framework, and it can be enumerated by a debug host on the other
>>>>>>>>>>>>>>>>> end of the USB link. As soon as the debug device is configured, a TTY
>>>>>>>>>>>>>>>>> serial device named /dev/ttyDBC0 will be created.
>>>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>>>> One use of this link is running a login service on the debug target.
>>>>>>>>>>>>>>>>> Hence it can be remote accessed by a debug host. Another use case can
>>>>>>>>>>>>>>>>> probably be found in servers. It provides a peer-to-peer USB link
>>>>>>>>>>>>>>>>> between two host-only machines. This provides a reasonable out-of-band
>>>>>>>>>>>>>>>>> communication method between two servers.
>>>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>>>> Signed-off-by: Lu Baolu <baolu.lu@linux.intel.com>
>>>>>>>>>>>>>>>>> ---
>>>>>>>>>>>>>>>>>  .../ABI/testing/sysfs-bus-pci-drivers-xhci_hcd     |   25 +
>>>>>>>>>>>>>>>>>  drivers/usb/host/Kconfig                           |    9 +
>>>>>>>>>>>>>>>>>  drivers/usb/host/Makefile                          |    5 +
>>>>>>>>>>>>>>>>>  drivers/usb/host/xhci-dbgcap.c                     | 1016 ++++++++++++++++++++
>>>>>>>>>>>>>>>>>  drivers/usb/host/xhci-dbgcap.h                     |  247 +++++
>>>>>>>>>>>>>>>>>  drivers/usb/host/xhci-dbgtty.c                     |  586 +++++++++++
>>>>>>>>>>>>>>>>>  drivers/usb/host/xhci-trace.h                      |   60 ++
>>>>>>>>>>>>>>>>>  drivers/usb/host/xhci.c                            |   10 +
>>>>>>>>>>>>>>>>>  drivers/usb/host/xhci.h                            |    1 +
>>>>>>>>>>>>>>>>>  9 files changed, 1959 insertions(+)
>>>>>>>>>>>>>>>>>  create mode 100644 Documentation/ABI/testing/sysfs-bus-pci-drivers-xhci_hcd
>>>>>>>>>>>>>>>>>  create mode 100644 drivers/usb/host/xhci-dbgcap.c
>>>>>>>>>>>>>>>>>  create mode 100644 drivers/usb/host/xhci-dbgcap.h
>>>>>>>>>>>>>>>>>  create mode 100644 drivers/usb/host/xhci-dbgtty.c
>>>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>> [snip]
>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>>>> +#define DBC_VENDOR_ID			0x1d6b	/* Linux Foundation 0x1d6b */
>>>>>>>>>>>>>>>>> +#define DBC_PRODUCT_ID			0x0004	/* device 0004 */
>>>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>> The DbC (xHCI DeBug Capability) is an optional functionality in
>>>>>>>>>>>>>>> some xHCI host controllers. It will present a super-speed debug
>>>>>>>>>>>>>>> device through the debug port after it is enabled.
>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>> The DbC register set defines an interface for system software
>>>>>>>>>>>>>>> to specify the vendor id and product id of the debug device.
>>>>>>>>>>>>>>> These two values will be presented by the debug device in its
>>>>>>>>>>>>>>> device descriptor idVendor and idProduct fields.
>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>> Microsoft Windows have a well established protocol for
>>>>>>>>>>>>>>> debugging over DbC. And it assigns below values for its use.
>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>> USB\VID_045E&PID_062D.DeviceDesc="Microsoft USB Debug Target"
>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>> I'm going to use 0x1d6b/0x0004 value pair for DbC use in
>>>>>>>>>>>>>>> Linux. Do you approve me to do so?
>>>>>>>>>>>>> No.  Why can't you use the same ids as Windows?  This is implementing
>>>>>>>>>>>>> the same protocol, right?
>>>>>>>>>>> the protocol running on top is 100% vendor specific. More than likely,
>>>>>>>>>>> we would just run kgdb on top of this, right? We really don't support
>>>>>>>>>>> microsoft's debug architecture.
>>>>>>>>> Ah, I didn't know about the protocol specifics here, if it is
>>>>>>>>> vendor-specific, then yes, we need our own id.
>>>>>>> Great, thanks :-)
>>>>>>>
>>>>>>> Let us know which one we're allowed to use and I'm sure Baolu can respin
>>>>>>> the patch in no time.
>>>>> Can I get a "full" description of what string this device id will
>>>>> reference?  Is it "Linux USB Debug Target" or something else?
>>>>>
>>>> Current manufacturer and product strings are set like this.
>>>>
>>>> +#define DBC_STRING_MANUFACTURER	"Linux"
>>>> +#define DBC_STRING_PRODUCT		"Remote	GDB"
>>>>
>>>> These are also place holders. We can change them to more meaningful strings.
>>> As the vendor id is assigned to "Linux Foundation" can we change that
>>> string please?
>>>
>>> And why not match what Microsoft does here, "USB Debug Target" makes
>>> more sense than "Remote GDB", right?
>>>
>>> If so, please use device id 0x0010, I've reserved that for the driver
>>> now.
>>>
>> I will change the strings and use the allocated product id.
>> Thank you very much.
>>
>> Another DbC usage is to enable DbC during early boot time,
>> and run GDB protocol data or early printk message over it.
>> The early printk implementation has been merged in 4.12-rc1.
>>
>> commit aeb9dd1 <usb/early: Add driver for xhci debug capability>
>>
>> In this code, the DbC product id is still a place holder (0x0004).
>> I am really sorry. I should apply a valid product id from you during
>> the code review time. But I forgot it since almost all attention was
>> paid to early driver itself at that time.
>>
>> We could use the same ID as you allocated here or, preferably,
>> use a different ID since we're dealing with different functionality. 
> A different one would be best, please give me a good device string to
> use and I will assign you a new ID, probably 0011.  Also send a patch
> now for this please.
>

Thanks, Greg.

How about below strings:

Manufacturer: Linux Foundation
Product: Linux USB GDB Target

I will send out the patches soon.

Best regards,
Lu Baolu

^ permalink raw reply	[flat|nested] 21+ messages in thread

* Re: [PATCH v3 2/3] usb: xhci: Add DbC support in xHCI driver
  2017-09-05  1:58 ` [PATCH v3 2/3] usb: xhci: Add DbC support in xHCI driver Lu Baolu
  2017-10-24 17:03   ` Mathias Nyman
  2017-11-02  1:36   ` Lu Baolu
@ 2017-11-13 15:26   ` Mathias Nyman
  2017-11-14  7:28     ` Felipe Balbi
  2 siblings, 1 reply; 21+ messages in thread
From: Mathias Nyman @ 2017-11-13 15:26 UTC (permalink / raw)
  To: Lu Baolu, Greg Kroah-Hartman; +Cc: linux-usb, linux-kernel

Ok, review last part

> diff --git a/drivers/usb/host/xhci-dbgtty.c b/drivers/usb/host/xhci-dbgtty.c
> new file mode 100644
> index 0000000..fe5fef0
> --- /dev/null
> +++ b/drivers/usb/host/xhci-dbgtty.c
> @@ -0,0 +1,586 @@
> +/**
> + * xhci-dbgtty.c - tty glue for xHCI debug capability
> + *
> + * Copyright (C) 2017 Intel Corporation
> + *
> + * Author: Lu Baolu <baolu.lu@linux.intel.com>
> + *
> + * This program is free software; you can redistribute it and/or modify
> + * it under the terms of the GNU General Public License version 2 as
> + * published by the Free Software Foundation.
> + */
> +
> +#include <linux/slab.h>
> +#include <linux/tty.h>
> +#include <linux/tty_flip.h>
> +
> +#include "xhci.h"
> +#include "xhci-dbgcap.h"
> +
> +static int dbc_buf_alloc(struct dbc_buf *db, unsigned int size)
> +{
> +	db->buf_buf = kzalloc(size, GFP_KERNEL);
> +	if (!db->buf_buf)
> +		return -ENOMEM;
> +
> +	db->buf_size = size;
> +	db->buf_put = db->buf_buf;
> +	db->buf_get = db->buf_buf;
> +
> +	return 0;
> +}
> +
> +static void dbc_buf_free(struct dbc_buf *db)
> +{
> +	kfree(db->buf_buf);
> +	db->buf_buf = NULL;
> +}
> +
> +static unsigned int dbc_buf_data_avail(struct dbc_buf *db)
> +{
> +	return (db->buf_size + db->buf_put - db->buf_get) % db->buf_size;
> +}
> +
> +static unsigned int dbc_buf_space_avail(struct dbc_buf *db)
> +{
> +	return (db->buf_size + db->buf_get - db->buf_put - 1) % db->buf_size;
> +}
> +
> +static unsigned int
> +dbc_buf_put(struct dbc_buf *db, const char *buf, unsigned int count)
> +{
> +	unsigned int	len;
> +
> +	len  = dbc_buf_space_avail(db);

extra whitespace i think

> +	if (count > len)
> +		count = len;
> +
> +	if (count == 0)
> +		return 0;
> +
> +	len = db->buf_buf + db->buf_size - db->buf_put;
> +	if (count > len) {
> +		memcpy(db->buf_put, buf, len);
> +		memcpy(db->buf_buf, buf + len, count - len);
> +		db->buf_put = db->buf_buf + count - len;
> +	} else {
> +		memcpy(db->buf_put, buf, count);
> +		if (count < len)
> +			db->buf_put += count;
> +		else
> +			db->buf_put = db->buf_buf;
> +	}
> +
> +	return count;
> +}
> +
> +static unsigned int
> +dbc_buf_get(struct dbc_buf *db, char *buf, unsigned int count)
> +{
> +	unsigned int	len;
> +
> +	len = dbc_buf_data_avail(db);
> +	if (count > len)
> +		count = len;
> +
> +	if (count == 0)
> +		return 0;
> +
> +	len = db->buf_buf + db->buf_size - db->buf_get;
> +	if (count > len) {
> +		memcpy(buf, db->buf_get, len);
> +		memcpy(buf + len, db->buf_buf, count - len);
> +		db->buf_get = db->buf_buf + count - len;
> +	} else {
> +		memcpy(buf, db->buf_get, count);
> +		if (count < len)
> +			db->buf_get += count;
> +		else
> +			db->buf_get = db->buf_buf;
> +	}
> +
> +	return count;
> +}
> +
> +static unsigned int
> +dbc_send_packet(struct dbc_port *port, char *packet, unsigned int size)
> +{
> +	unsigned int		len;
> +
> +	len = dbc_buf_data_avail(&port->port_write_buf);
> +	if (len < size)
> +		size = len;
> +	if (size != 0)
> +		size = dbc_buf_get(&port->port_write_buf, packet, size);
> +	return size;
> +}
> +
> +static int dbc_start_tx(struct dbc_port *port)
> +	__releases(&port->port_lock)
> +	__acquires(&port->port_lock)
> +{
> +	int			len;
> +	struct dbc_request	*req;
> +	int			status = 0;
> +	bool			do_tty_wake = false;
> +	struct list_head	*pool = &port->write_pool;
> +
> +	while (!list_empty(pool)) {
> +		req = list_entry(pool->next, struct dbc_request, list_pool);
> +		len = dbc_send_packet(port, req->buf, DBC_MAX_PACKET);
> +		if (len == 0)
> +			break;
> +		do_tty_wake = true;
> +
> +		req->length = len;
> +		list_del(&req->list_pool);
> +
> +		spin_unlock(&port->port_lock);
> +		status = port->out->queue(port->out, req, GFP_ATOMIC);
> +		spin_lock(&port->port_lock);
> +
> +		if (status) {
> +			list_add(&req->list_pool, pool);
> +			break;
> +		}
> +	}
> +
> +	if (do_tty_wake && port->port.tty)
> +		tty_wakeup(port->port.tty);
> +
> +	return status;
> +}
> +
> +static void dbc_start_rx(struct dbc_port *port)
> +	__releases(&port->port_lock)
> +	__acquires(&port->port_lock)
> +{
> +	struct dbc_request	*req;
> +	int			status;
> +	struct list_head	*pool = &port->read_pool;
> +
> +	while (!list_empty(pool)) {
> +		if (!port->port.tty)
> +			break;
> +
> +		req = list_entry(pool->next, struct dbc_request, list_pool);
> +		list_del(&req->list_pool);
> +		req->length = DBC_MAX_PACKET;
> +
> +		spin_unlock(&port->port_lock);
> +		status = port->in->queue(port->in, req, GFP_ATOMIC);
> +		spin_lock(&port->port_lock);
> +
> +		if (status) {
> +			list_add(&req->list_pool, pool);
> +			break;
> +		}
> +	}
> +}
> +
> +static void
> +dbc_read_complete(struct xhci_hcd *xhci, struct dbc_request *req)
> +{
> +	struct xhci_dbc		*dbc = xhci->dbc;
> +	struct dbc_port		*port = &dbc->port;
> +
> +	spin_lock(&port->port_lock);
> +	list_add_tail(&req->list_pool, &port->read_queue);
> +	tasklet_schedule(&port->push);
> +	spin_unlock(&port->port_lock);
> +}
> +
> +static void dbc_write_complete(struct xhci_hcd *xhci, struct dbc_request *req)
> +{
> +	struct xhci_dbc		*dbc = xhci->dbc;
> +	struct dbc_port		*port = &dbc->port;
> +
> +	spin_lock(&port->port_lock);
> +	list_add(&req->list_pool, &port->write_pool);
> +	switch (req->status) {
> +	case 0:
> +		dbc_start_tx(port);
> +		break;
> +	case -ESHUTDOWN:
> +		break;
> +	default:
> +		xhci_warn(xhci, "unexpected write complete status %d\n",
> +			  req->status);
> +		break;
> +	}
> +	spin_unlock(&port->port_lock);
> +}
> +
> +void xhci_dbc_free_req(struct dbc_ep *dep, struct dbc_request *req)
> +{
> +	kfree(req->buf);
> +	dep->free_request(dep, req);
> +}
> +
> +static int
> +xhci_dbc_alloc_requests(struct dbc_ep *dep, struct list_head *head,
> +			void (*fn)(struct xhci_hcd *, struct dbc_request *))
> +{
> +	int			i;
> +	struct dbc_request	*req;
> +
> +	for (i = 0; i < QUEUE_SIZE; i++) {
> +		req = dep->alloc_request(dep, GFP_ATOMIC);
> +		if (!req)
> +			break;
> +
> +		req->length = DBC_MAX_PACKET;
> +		req->buf = kmalloc(req->length, GFP_KERNEL);
> +		if (!req->buf) {
> +			xhci_dbc_free_req(dep, req);
> +			break;
> +		}
> +
> +		req->complete = fn;
> +		list_add_tail(&req->list_pool, head);
> +	}
> +
> +	return list_empty(head) ? -ENOMEM : 0;
> +}
> +
> +static void
> +xhci_dbc_free_requests(struct dbc_ep *dep, struct list_head *head)
> +{
> +	struct dbc_request	*req;
> +
> +	while (!list_empty(head)) {
> +		req = list_entry(head->next, struct dbc_request, list_pool);
> +		list_del(&req->list_pool);
> +		xhci_dbc_free_req(dep, req);
> +	}
> +}
> +
> +static int dbc_tty_install(struct tty_driver *driver, struct tty_struct *tty)
> +{
> +	struct dbc_port		*port = driver->driver_state;
> +
> +	tty->driver_data = port;
> +
> +	return tty_port_install(&port->port, driver, tty);
> +}
> +
> +static int dbc_tty_open(struct tty_struct *tty, struct file *file)
> +{
> +	struct dbc_port		*port = tty->driver_data;
> +
> +	return tty_port_open(&port->port, tty, file);
> +}
> +
> +static void dbc_tty_close(struct tty_struct *tty, struct file *file)
> +{
> +	struct dbc_port		*port = tty->driver_data;
> +
> +	tty_port_close(&port->port, tty, file);
> +}
> +
> +static int dbc_tty_write(struct tty_struct *tty,
> +			 const unsigned char *buf,
> +			 int count)
> +{
> +	struct dbc_port		*port = tty->driver_data;
> +	unsigned long		flags;
> +
> +	spin_lock_irqsave(&port->port_lock, flags);
> +	if (count)
> +		count = dbc_buf_put(&port->port_write_buf, buf, count);
> +	dbc_start_tx(port);
> +	spin_unlock_irqrestore(&port->port_lock, flags);
> +
> +	return count;
> +}
> +
> +static int dbc_tty_put_char(struct tty_struct *tty, unsigned char ch)
> +{
> +	struct dbc_port		*port = tty->driver_data;
> +	unsigned long		flags;
> +	int			status;
> +
> +	spin_lock_irqsave(&port->port_lock, flags);
> +	status = dbc_buf_put(&port->port_write_buf, &ch, 1);
> +	spin_unlock_irqrestore(&port->port_lock, flags);
> +
> +	return status;
> +}
> +
> +static void dbc_tty_flush_chars(struct tty_struct *tty)
> +{
> +	struct dbc_port		*port = tty->driver_data;
> +	unsigned long		flags;
> +
> +	spin_lock_irqsave(&port->port_lock, flags);
> +	dbc_start_tx(port);
> +	spin_unlock_irqrestore(&port->port_lock, flags);
> +}
> +
> +static int dbc_tty_write_room(struct tty_struct *tty)
> +{
> +	struct dbc_port		*port = tty->driver_data;
> +	unsigned long		flags;
> +	int			room = 0;
> +
> +	spin_lock_irqsave(&port->port_lock, flags);
> +	room = dbc_buf_space_avail(&port->port_write_buf);
> +	spin_unlock_irqrestore(&port->port_lock, flags);
> +
> +	return room;
> +}
> +
> +static int dbc_tty_chars_in_buffer(struct tty_struct *tty)
> +{
> +	struct dbc_port		*port = tty->driver_data;
> +	unsigned long		flags;
> +	int			chars = 0;
> +
> +	spin_lock_irqsave(&port->port_lock, flags);
> +	chars = dbc_buf_data_avail(&port->port_write_buf);
> +	spin_unlock_irqrestore(&port->port_lock, flags);
> +
> +	return chars;
> +}
> +
> +static void dbc_tty_unthrottle(struct tty_struct *tty)
> +{
> +	struct dbc_port		*port = tty->driver_data;
> +	unsigned long		flags;
> +
> +	spin_lock_irqsave(&port->port_lock, flags);
> +	tasklet_schedule(&port->push);
> +	spin_unlock_irqrestore(&port->port_lock, flags);
> +}
> +
> +static const struct tty_operations dbc_tty_ops = {
> +	.install		= dbc_tty_install,
> +	.open			= dbc_tty_open,
> +	.close			= dbc_tty_close,
> +	.write			= dbc_tty_write,
> +	.put_char		= dbc_tty_put_char,
> +	.flush_chars		= dbc_tty_flush_chars,
> +	.write_room		= dbc_tty_write_room,
> +	.chars_in_buffer	= dbc_tty_chars_in_buffer,
> +	.unthrottle		= dbc_tty_unthrottle,
> +};
> +
> +static struct tty_driver *dbc_tty_driver;
> +
> +int xhci_dbc_tty_register_driver(struct xhci_hcd *xhci)
> +{
> +	int			status;
> +	struct xhci_dbc		*dbc = xhci->dbc;
> +
> +	dbc_tty_driver = tty_alloc_driver(1, TTY_DRIVER_REAL_RAW |
> +					  TTY_DRIVER_DYNAMIC_DEV);
> +	if (IS_ERR(dbc_tty_driver)) {
> +		status = PTR_ERR(dbc_tty_driver);
> +		dbc_tty_driver = NULL;
> +		return status;
> +	}
> +
> +	dbc_tty_driver->driver_name = "dbc_serial";
> +	dbc_tty_driver->name = "ttyDBC";
> +
> +	dbc_tty_driver->type = TTY_DRIVER_TYPE_SERIAL;
> +	dbc_tty_driver->subtype = SERIAL_TYPE_NORMAL;
> +	dbc_tty_driver->init_termios = tty_std_termios;
> +	dbc_tty_driver->init_termios.c_cflag =
> +			B9600 | CS8 | CREAD | HUPCL | CLOCAL;
> +	dbc_tty_driver->init_termios.c_ispeed = 9600;
> +	dbc_tty_driver->init_termios.c_ospeed = 9600;
> +	dbc_tty_driver->driver_state = &dbc->port;
> +
> +	tty_set_operations(dbc_tty_driver, &dbc_tty_ops);
> +
> +	status = tty_register_driver(dbc_tty_driver);
> +	if (status) {
> +		xhci_err(xhci,
> +			 "can't register dbc tty driver, err %d\n", status);
> +		put_tty_driver(dbc_tty_driver);
> +		dbc_tty_driver = NULL;
> +	}
> +
> +	return status;
> +}
> +
> +void xhci_dbc_tty_unregister_driver(void)
> +{
> +	tty_unregister_driver(dbc_tty_driver);
> +	put_tty_driver(dbc_tty_driver);
> +	dbc_tty_driver = NULL;
> +}
> +
> +static void dbc_rx_push(unsigned long _port)
> +{
> +	struct dbc_request	*req;
> +	struct tty_struct	*tty;
> +	bool			do_push = false;
> +	bool			disconnect = false;
> +	struct dbc_port		*port = (void *)_port;
> +	struct list_head	*queue = &port->read_queue;
> +
> +	spin_lock_irq(&port->port_lock);
> +	tty = port->port.tty;
> +	while (!list_empty(queue)) {
> +		req = list_first_entry(queue, struct dbc_request, list_pool);
> +
> +		if (tty && tty_throttled(tty))
> +			break;
> +
> +		switch (req->status) {
> +		case 0:
> +			break;
> +		case -ESHUTDOWN:
> +			disconnect = true;
> +			break;
> +		default:
> +			pr_warn("ttyDBC0: unexpected RX status %d\n",
> +				req->status);
> +			break;
> +		}
> +
> +		if (req->actual) {
> +			char		*packet = req->buf;
> +			unsigned int	n, size = req->actual;
> +			int		count;
> +
> +			n = port->n_read;
> +			if (n) {
> +				packet += n;
> +				size -= n;
> +			}
> +
> +			count = tty_insert_flip_string(&port->port, packet,
> +						       size);
> +			if (count)
> +				do_push = true;
> +			if (count != size) {
> +				port->n_read += count;
> +				break;
> +			}
> +			port->n_read = 0;
> +		}
> +
> +		list_move(&req->list_pool, &port->read_pool);
> +	}
> +
> +	if (do_push)
> +		tty_flip_buffer_push(&port->port);
> +
> +	if (!list_empty(queue) && tty) {
> +		if (!tty_throttled(tty)) {
> +			if (do_push)
> +				tasklet_schedule(&port->push);
> +			else
> +				pr_warn("ttyDBC0: RX not scheduled?\n");
> +		}
> +	}
> +
> +	if (!disconnect)
> +		dbc_start_rx(port);
> +
> +	spin_unlock_irq(&port->port_lock);
> +}
> +
> +static int dbc_port_activate(struct tty_port *_port, struct tty_struct *tty)
> +{
> +	struct dbc_port	*port = container_of(_port, struct dbc_port, port);
> +
> +	spin_lock_irq(&port->port_lock);
> +	dbc_start_rx(port);
> +	spin_unlock_irq(&port->port_lock);
> +
> +	return 0;
> +}
> +
> +static const struct tty_port_operations dbc_port_ops = {
> +	.activate =	dbc_port_activate,
> +};
> +
> +static void
> +xhci_dbc_tty_init_port(struct xhci_hcd *xhci, struct dbc_port *port)
> +{
> +	tty_port_init(&port->port);
> +	spin_lock_init(&port->port_lock);
> +	tasklet_init(&port->push, dbc_rx_push, (unsigned long)port);
> +	INIT_LIST_HEAD(&port->read_pool);
> +	INIT_LIST_HEAD(&port->read_queue);
> +	INIT_LIST_HEAD(&port->write_pool);
> +
> +	port->in =		get_in_ep(xhci);
> +	port->out =		get_out_ep(xhci);
> +	port->port.ops =	&dbc_port_ops;
> +	port->n_read =		0;
> +}
> +
> +static void
> +xhci_dbc_tty_exit_port(struct dbc_port *port)
> +{
> +	tasklet_kill(&port->push);
> +	tty_port_destroy(&port->port);
> +}
> +
> +int xhci_dbc_tty_register_device(struct xhci_hcd *xhci)
> +{
> +	int			ret;
> +	struct device		*tty_dev;
> +	struct xhci_dbc		*dbc = xhci->dbc;
> +	struct dbc_port		*port = &dbc->port;
> +
> +	xhci_dbc_tty_init_port(xhci, port);
> +	tty_dev = tty_port_register_device(&port->port,
> +					   dbc_tty_driver, 0, NULL);
> +	ret = IS_ERR_OR_NULL(tty_dev);
> +	if (ret)
> +		goto register_fail;
> +
> +	ret = dbc_buf_alloc(&port->port_write_buf, WRITE_BUF_SIZE);
> +	if (ret)
> +		goto buf_alloc_fail;
> +
> +	ret = xhci_dbc_alloc_requests(port->in, &port->read_pool,
> +				      dbc_read_complete);
> +	if (ret)
> +		goto request_fail;
> +
> +	ret = xhci_dbc_alloc_requests(port->out, &port->write_pool,
> +				      dbc_write_complete);
> +	if (ret)
> +		goto request_fail;
> +
> +	port->registered = true;
> +
> +	return 0;
> +
> +request_fail:
> +	xhci_dbc_free_requests(port->in, &port->read_pool);
> +	xhci_dbc_free_requests(port->out, &port->write_pool);
> +	dbc_buf_free(&port->port_write_buf);
> +
> +buf_alloc_fail:
> +	tty_unregister_device(dbc_tty_driver, 0);
> +
> +register_fail:
> +	xhci_dbc_tty_exit_port(port);
> +
> +	xhci_err(xhci, "can't register tty port, err %d\n", ret);
> +
> +	return ret;
> +}
> +
> +void xhci_dbc_tty_unregister_device(struct xhci_hcd *xhci)
> +{
> +	struct xhci_dbc		*dbc = xhci->dbc;
> +	struct dbc_port		*port = &dbc->port;
> +
> +	tty_unregister_device(dbc_tty_driver, 0);
> +	xhci_dbc_tty_exit_port(port);
> +	port->registered = false;
> +
> +	dbc_buf_free(&port->port_write_buf);
> +	xhci_dbc_free_requests(get_out_ep(xhci), &port->read_pool);
> +	xhci_dbc_free_requests(get_out_ep(xhci), &port->read_queue);
> +	xhci_dbc_free_requests(get_in_ep(xhci), &port->write_pool);
> +}
> diff --git a/drivers/usb/host/xhci-trace.h b/drivers/usb/host/xhci-trace.h
> index 8ce96de..bf2013c 100644
> --- a/drivers/usb/host/xhci-trace.h
> +++ b/drivers/usb/host/xhci-trace.h
> @@ -26,6 +26,7 @@
>   
>   #include <linux/tracepoint.h>
>   #include "xhci.h"
> +#include "xhci-dbgcap.h"
>   
>   #define XHCI_MSG_MAX	500
>   
> @@ -158,6 +159,21 @@ DEFINE_EVENT(xhci_log_trb, xhci_queue_trb,
>   	TP_ARGS(ring, trb)
>   );
>   
> +DEFINE_EVENT(xhci_log_trb, xhci_dbc_handle_event,
> +	TP_PROTO(struct xhci_ring *ring, struct xhci_generic_trb *trb),
> +	TP_ARGS(ring, trb)
> +);
> +
> +DEFINE_EVENT(xhci_log_trb, xhci_dbc_handle_transfer,
> +	TP_PROTO(struct xhci_ring *ring, struct xhci_generic_trb *trb),
> +	TP_ARGS(ring, trb)
> +);
> +
> +DEFINE_EVENT(xhci_log_trb, xhci_dbc_gadget_ep_queue,
> +	TP_PROTO(struct xhci_ring *ring, struct xhci_generic_trb *trb),
> +	TP_ARGS(ring, trb)
> +);
> +
>   DECLARE_EVENT_CLASS(xhci_log_virt_dev,
>   	TP_PROTO(struct xhci_virt_device *vdev),
>   	TP_ARGS(vdev),
> @@ -453,6 +469,50 @@ DEFINE_EVENT(xhci_log_ring, xhci_inc_deq,
>   	TP_PROTO(struct xhci_ring *ring),
>   	TP_ARGS(ring)
>   );
> +
> +DECLARE_EVENT_CLASS(xhci_dbc_log_request,
> +	TP_PROTO(struct dbc_request *req),
> +	TP_ARGS(req),
> +	TP_STRUCT__entry(
> +		__field(struct dbc_request *, req)
> +		__field(bool, dir)
> +		__field(unsigned int, actual)
> +		__field(unsigned int, length)
> +		__field(int, status)
> +	),
> +	TP_fast_assign(
> +		__entry->req = req;
> +		__entry->dir = req->direction;
> +		__entry->actual = req->actual;
> +		__entry->length = req->length;
> +		__entry->status = req->status;
> +	),
> +	TP_printk("%s: req %p length %u/%u ==> %d",
> +		__entry->dir ? "bulk-in" : "bulk-out",
> +		__entry->req, __entry->actual,
> +		__entry->length, __entry->status
> +	)
> +);
> +
> +DEFINE_EVENT(xhci_dbc_log_request, xhci_dbc_alloc_request,
> +	TP_PROTO(struct dbc_request *req),
> +	TP_ARGS(req)
> +);
> +
> +DEFINE_EVENT(xhci_dbc_log_request, xhci_dbc_free_request,
> +	TP_PROTO(struct dbc_request *req),
> +	TP_ARGS(req)
> +);
> +
> +DEFINE_EVENT(xhci_dbc_log_request, xhci_dbc_queue_request,
> +	TP_PROTO(struct dbc_request *req),
> +	TP_ARGS(req)
> +);
> +
> +DEFINE_EVENT(xhci_dbc_log_request, xhci_dbc_giveback_request,
> +	TP_PROTO(struct dbc_request *req),
> +	TP_ARGS(req)
> +);
>   #endif /* __XHCI_TRACE_H */
>   
>   /* this part must be outside header guard */
> diff --git a/drivers/usb/host/xhci.c b/drivers/usb/host/xhci.c
> index b2ff1ff..39a9a88 100644
> --- a/drivers/usb/host/xhci.c
> +++ b/drivers/usb/host/xhci.c
> @@ -32,6 +32,7 @@
>   #include "xhci.h"
>   #include "xhci-trace.h"
>   #include "xhci-mtk.h"
> +#include "xhci-dbgcap.h"
>   
>   #define DRIVER_AUTHOR "Sarah Sharp"
>   #define DRIVER_DESC "'eXtensible' Host Controller (xHC) Driver"
> @@ -632,6 +633,9 @@ int xhci_run(struct usb_hcd *hcd)
>   	}
>   	xhci_dbg_trace(xhci, trace_xhci_dbg_init,
>   			"Finished xhci_run for USB2 roothub");
> +
> +	dbc_create_sysfs_file(xhci);
> +
>   	return 0;
>   }
>   EXPORT_SYMBOL_GPL(xhci_run);
> @@ -660,6 +664,8 @@ static void xhci_stop(struct usb_hcd *hcd)
>   		return;
>   	}
>   
> +	dbc_remove_sysfs_file(xhci);
> +
>   	spin_lock_irq(&xhci->lock);
>   	xhci->xhc_state |= XHCI_STATE_HALTED;
>   	xhci->cmd_ring_state = CMD_RING_STATE_STOPPED;
> @@ -876,6 +882,8 @@ int xhci_suspend(struct xhci_hcd *xhci, bool do_wakeup)
>   			xhci->shared_hcd->state != HC_STATE_SUSPENDED)
>   		return -EINVAL;
>   
> +	xhci_dbc_suspend(xhci);
> +
>   	/* Clear root port wake on bits if wakeup not allowed. */
>   	if (!do_wakeup)
>   		xhci_disable_port_wake_on_bits(xhci);
> @@ -1071,6 +1079,8 @@ int xhci_resume(struct xhci_hcd *xhci, bool hibernated)
>   
>   	spin_unlock_irq(&xhci->lock);
>   
> +	xhci_dbc_resume(xhci);
> +
>    done:
>   	if (retval == 0) {
>   		/* Resume root hubs only when have pending events. */
> diff --git a/drivers/usb/host/xhci.h b/drivers/usb/host/xhci.h
> index 62bcdcd..bae2538 100644
> --- a/drivers/usb/host/xhci.h
> +++ b/drivers/usb/host/xhci.h
> @@ -1850,6 +1850,7 @@ struct xhci_hcd {
>   /* Compliance Mode Timer Triggered every 2 seconds */
>   #define COMP_MODE_RCVRY_MSECS 2000
>   
> +	void			*dbc;
>   	/* platform-specific data -- must come last */
>   	unsigned long		priv[0] __aligned(sizeof(s64));
>   };
> 

Did only a really quick overview of this, It all looks fine.
If you make a new revision I'll send it out after rc1

Thanks
-Mathias

^ permalink raw reply	[flat|nested] 21+ messages in thread

* Re: [PATCH v3 2/3] usb: xhci: Add DbC support in xHCI driver
  2017-11-13 15:26   ` Mathias Nyman
@ 2017-11-14  7:28     ` Felipe Balbi
  2017-11-15  7:25       ` Lu Baolu
  0 siblings, 1 reply; 21+ messages in thread
From: Felipe Balbi @ 2017-11-14  7:28 UTC (permalink / raw)
  To: Mathias Nyman, Lu Baolu, Greg Kroah-Hartman; +Cc: linux-usb, linux-kernel


Hi,

Mathias Nyman <mathias.nyman@linux.intel.com> writes:
>> +static int dbc_buf_alloc(struct dbc_buf *db, unsigned int size)
>> +{
>> +	db->buf_buf = kzalloc(size, GFP_KERNEL);
>> +	if (!db->buf_buf)
>> +		return -ENOMEM;
>> +
>> +	db->buf_size = size;
>> +	db->buf_put = db->buf_buf;
>> +	db->buf_get = db->buf_buf;
>> +
>> +	return 0;
>> +}

you may wanna have a look at kfifo.

-- 
balbi

^ permalink raw reply	[flat|nested] 21+ messages in thread

* Re: [PATCH v3 2/3] usb: xhci: Add DbC support in xHCI driver
  2017-11-14  7:28     ` Felipe Balbi
@ 2017-11-15  7:25       ` Lu Baolu
  0 siblings, 0 replies; 21+ messages in thread
From: Lu Baolu @ 2017-11-15  7:25 UTC (permalink / raw)
  To: Felipe Balbi, Mathias Nyman, Greg Kroah-Hartman; +Cc: linux-usb, linux-kernel

Hi,

On 11/14/2017 03:28 PM, Felipe Balbi wrote:
> Hi,
>
> Mathias Nyman <mathias.nyman@linux.intel.com> writes:
>>> +static int dbc_buf_alloc(struct dbc_buf *db, unsigned int size)
>>> +{
>>> +	db->buf_buf = kzalloc(size, GFP_KERNEL);
>>> +	if (!db->buf_buf)
>>> +		return -ENOMEM;
>>> +
>>> +	db->buf_size = size;
>>> +	db->buf_put = db->buf_buf;
>>> +	db->buf_get = db->buf_buf;
>>> +
>>> +	return 0;
>>> +}
> you may wanna have a look at kfifo.
>

Yeah! kfifo gives me exactly what I want here.

I will replace it with kfifo. Thank you.

Best regards,
Lu Baolu

^ permalink raw reply	[flat|nested] 21+ messages in thread

end of thread, other threads:[~2017-11-15  7:25 UTC | newest]

Thread overview: 21+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2017-09-05  1:58 [PATCH v3 0/3] usb: xhci: Add debug capability support in xhci Lu Baolu
2017-09-05  1:58 ` [PATCH v3 1/3] usb: xhci: Make some static functions global Lu Baolu
2017-09-05  1:58 ` [PATCH v3 2/3] usb: xhci: Add DbC support in xHCI driver Lu Baolu
2017-10-24 17:03   ` Mathias Nyman
2017-10-25  2:15     ` Lu Baolu
2017-11-02  1:36   ` Lu Baolu
2017-11-02  8:17     ` Greg Kroah-Hartman
2017-11-02  9:15       ` Felipe Balbi
2017-11-02  9:32         ` Greg Kroah-Hartman
2017-11-02 10:38           ` Felipe Balbi
2017-11-02 16:51             ` Greg Kroah-Hartman
2017-11-03  0:45               ` Lu Baolu
2017-11-03  6:27                 ` Greg Kroah-Hartman
2017-11-03 10:52                   ` Felipe Balbi
2017-11-06  0:35                   ` Lu Baolu
2017-11-06  8:00                     ` Greg Kroah-Hartman
2017-11-07  2:32                       ` Lu Baolu
2017-11-13 15:26   ` Mathias Nyman
2017-11-14  7:28     ` Felipe Balbi
2017-11-15  7:25       ` Lu Baolu
2017-09-05  1:59 ` [PATCH v3 3/3] usb: doc: Update document for USB3 debug port usage Lu Baolu

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.