All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH v6 0/4] usb: early: add support for early printk through USB3 debug port
@ 2017-01-22  6:19 Lu Baolu
  2017-01-22  6:19 ` [PATCH v6 1/4] usb: dbc: early driver for xhci debug capability Lu Baolu
                   ` (3 more replies)
  0 siblings, 4 replies; 7+ messages in thread
From: Lu Baolu @ 2017-01-22  6:19 UTC (permalink / raw)
  To: Greg Kroah-Hartman, Ingo Molnar
  Cc: Mathias Nyman, tglx, peterz, linux-usb, x86, linux-kernel, Lu Baolu

xHCI debug capability (DbC) is an optional but standalone
functionality provided by an xHCI host controller. With DbC
hardware initialized, the system will present a debug device
through the USB3 debug port (normally the first USB3 port).
The debug device is fully compliant with the USB framework
and provides the equivalent of a very high performance (USB3)
full-duplex serial link between the debug host and target.
The DbC functionality is independent of xHCI host. There
isn't any precondition from xHCI host side for DbC to work.

This patch set adds support for early printk functionality
through a USB3 debug port by 1) initializing and enabling
the DbC hardware during early boot; 2) registering a boot
console to the system so that early printk messages can go
through the USB3 debug port. It also includes some lines
of changes in usb_debug driver so that it can be bound when
a USB3 debug device is enumerated.

---
Change log:
v5->v6:
  - rebase the patches on top of the latest 4.10-rc4
  - run successfully in a 32-bit kernel
  - [PATCH 1/4]
    - remove ugly linebreaks to make code more readable
    - rename config names to make them consistency
    - move sleep-able ioremap() out of the lock area
    - rename reserved fields of register structures
    - make the vertical tabulation of struct fields consistent
  - [PATCH 2/4]
    - remove "#ifdef" in the generic code by creating proper
      wrappers in header file
  - [PATCH 3/4]
    - refine the title and commit message
  - [PATCH 4/4]
    - fix several typos and grammar errors in the document

v4->v5:
  - add raw_spin_lock to make xdbc_bulk_write() reentrant.

v3->v4:
  - Rename the document with .dst suffix.
  - Add the list of hardware that has been succesfuly
    tested on in the document.

v2->v3:
  - Removed spinlock usage.
  - Removed work queue usage.
  - Refined the user guide document.

v1->v2:
  - Refactor the duplicate code in xdbc_early_start() and
    xdbc_handle_external_reset().
  - Free resources when hardware not used any more.
  - Refine the user guide document.

Lu Baolu (4):
  usb: dbc: early driver for xhci debug capability
  x86: add support for earlyprintk via USB3 debug port
  usb: serial: add dbc debug device support to usb_debug
  usb: doc: add document for USB3 debug port usage

 Documentation/admin-guide/kernel-parameters.txt |    1 +
 Documentation/usb/usb3-debug-port.rst           |   98 +++
 arch/x86/Kconfig.debug                          |   17 +
 arch/x86/kernel/early_printk.c                  |    5 +
 arch/x86/kernel/setup.c                         |    4 +
 drivers/usb/Kconfig                             |    3 +
 drivers/usb/Makefile                            |    2 +-
 drivers/usb/early/Makefile                      |    1 +
 drivers/usb/early/xhci-dbc.c                    | 1027 +++++++++++++++++++++++
 drivers/usb/early/xhci-dbc.h                    |  205 +++++
 drivers/usb/serial/usb_debug.c                  |   28 +-
 include/linux/usb/xhci-dbgp.h                   |   29 +
 12 files changed, 1416 insertions(+), 4 deletions(-)
 create mode 100644 Documentation/usb/usb3-debug-port.rst
 create mode 100644 drivers/usb/early/xhci-dbc.c
 create mode 100644 drivers/usb/early/xhci-dbc.h
 create mode 100644 include/linux/usb/xhci-dbgp.h

-- 
2.1.4

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

* [PATCH v6 1/4] usb: dbc: early driver for xhci debug capability
  2017-01-22  6:19 [PATCH v6 0/4] usb: early: add support for early printk through USB3 debug port Lu Baolu
@ 2017-01-22  6:19 ` Lu Baolu
  2017-01-22  9:31   ` Ingo Molnar
  2017-01-22  6:19 ` [PATCH v6 2/4] x86: add support for earlyprintk via USB3 debug port Lu Baolu
                   ` (2 subsequent siblings)
  3 siblings, 1 reply; 7+ messages in thread
From: Lu Baolu @ 2017-01-22  6:19 UTC (permalink / raw)
  To: Greg Kroah-Hartman, Ingo Molnar
  Cc: Mathias Nyman, tglx, peterz, linux-usb, x86, linux-kernel, Lu Baolu

xHCI debug capability (DbC) is an optional but standalone
functionality provided by an xHCI host controller. Software
learns this capability by walking through the extended
capability list of the host. xHCI specification describes
DbC in section 7.6.

This patch introduces the code to probe and initialize the
debug capability hardware during early boot. With hardware
initialized, the debug target (system on which this code is
running) will present a debug device through the debug port
(normally the first USB3 port). The debug device is fully
compliant with the USB framework and provides the equivalent
of a very high performance (USB3) full-duplex serial link
between the debug host and target. The DbC functionality is
independent of xHCI host. There isn't any precondition from
xHCI host side for DbC to work.

This patch also includes bulk out and bulk in interfaces.
These interfaces could be used to implement early printk
bootconsole or hook to various system debuggers.

Cc: Mathias Nyman <mathias.nyman@linux.intel.com>
Signed-off-by: Lu Baolu <baolu.lu@linux.intel.com>
---
 arch/x86/Kconfig.debug        |   17 +
 drivers/usb/Kconfig           |    3 +
 drivers/usb/Makefile          |    2 +-
 drivers/usb/early/Makefile    |    1 +
 drivers/usb/early/xhci-dbc.c  | 1027 +++++++++++++++++++++++++++++++++++++++++
 drivers/usb/early/xhci-dbc.h  |  205 ++++++++
 include/linux/usb/xhci-dbgp.h |   29 ++
 7 files changed, 1283 insertions(+), 1 deletion(-)
 create mode 100644 drivers/usb/early/xhci-dbc.c
 create mode 100644 drivers/usb/early/xhci-dbc.h
 create mode 100644 include/linux/usb/xhci-dbgp.h

diff --git a/arch/x86/Kconfig.debug b/arch/x86/Kconfig.debug
index 67eec55..ab0178f 100644
--- a/arch/x86/Kconfig.debug
+++ b/arch/x86/Kconfig.debug
@@ -5,6 +5,9 @@ config TRACE_IRQFLAGS_SUPPORT
 
 source "lib/Kconfig.debug"
 
+config EARLY_PRINTK_USB
+	bool
+
 config X86_VERBOSE_BOOTUP
 	bool "Enable verbose x86 bootup info messages"
 	default y
@@ -29,6 +32,7 @@ config EARLY_PRINTK
 config EARLY_PRINTK_DBGP
 	bool "Early printk via EHCI debug port"
 	depends on EARLY_PRINTK && PCI
+	select EARLY_PRINTK_USB
 	---help---
 	  Write kernel log output directly into the EHCI debug port.
 
@@ -48,6 +52,19 @@ config EARLY_PRINTK_EFI
 	  This is useful for kernel debugging when your machine crashes very
 	  early before the console code is initialized.
 
+config EARLY_PRINTK_USB_XDBC
+	bool "Early printk via xHCI debug port"
+	depends on EARLY_PRINTK && PCI
+	select EARLY_PRINTK_USB
+	---help---
+	  Write kernel log output directly into the xHCI debug port.
+
+	  This is useful for kernel debugging when your machine crashes very
+	  early before the console code is initialized. For normal operation
+	  it is not recommended because it looks ugly and doesn't cooperate
+	  with klogd/syslogd or the X server. You should normally N here,
+	  unless you want to debug such a crash.
+
 config X86_PTDUMP_CORE
 	def_bool n
 
diff --git a/drivers/usb/Kconfig b/drivers/usb/Kconfig
index fbe493d..9313fff 100644
--- a/drivers/usb/Kconfig
+++ b/drivers/usb/Kconfig
@@ -19,6 +19,9 @@ config USB_EHCI_BIG_ENDIAN_MMIO
 config USB_EHCI_BIG_ENDIAN_DESC
 	bool
 
+config USB_EARLY_PRINTK
+	bool
+
 menuconfig USB_SUPPORT
 	bool "USB support"
 	depends on HAS_IOMEM
diff --git a/drivers/usb/Makefile b/drivers/usb/Makefile
index 7791af6..53d1356 100644
--- a/drivers/usb/Makefile
+++ b/drivers/usb/Makefile
@@ -49,7 +49,7 @@ obj-$(CONFIG_USB_MICROTEK)	+= image/
 obj-$(CONFIG_USB_SERIAL)	+= serial/
 
 obj-$(CONFIG_USB)		+= misc/
-obj-$(CONFIG_EARLY_PRINTK_DBGP)	+= early/
+obj-$(CONFIG_EARLY_PRINTK_USB)	+= early/
 
 obj-$(CONFIG_USB_ATM)		+= atm/
 obj-$(CONFIG_USB_SPEEDTOUCH)	+= atm/
diff --git a/drivers/usb/early/Makefile b/drivers/usb/early/Makefile
index 24bbe51..fcde228 100644
--- a/drivers/usb/early/Makefile
+++ b/drivers/usb/early/Makefile
@@ -3,3 +3,4 @@
 #
 
 obj-$(CONFIG_EARLY_PRINTK_DBGP) += ehci-dbgp.o
+obj-$(CONFIG_EARLY_PRINTK_USB_XDBC) += xhci-dbc.o
diff --git a/drivers/usb/early/xhci-dbc.c b/drivers/usb/early/xhci-dbc.c
new file mode 100644
index 0000000..72480c4
--- /dev/null
+++ b/drivers/usb/early/xhci-dbc.c
@@ -0,0 +1,1027 @@
+/**
+ * xhci-dbc.c - xHCI debug capability early driver
+ *
+ * Copyright (C) 2016 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.
+ */
+
+#define pr_fmt(fmt)	KBUILD_MODNAME ":%s: " fmt, __func__
+
+#include <linux/console.h>
+#include <linux/pci_regs.h>
+#include <linux/pci_ids.h>
+#include <linux/bootmem.h>
+#include <linux/io.h>
+#include <asm/pci-direct.h>
+#include <asm/fixmap.h>
+#include <linux/bcd.h>
+#include <linux/export.h>
+#include <linux/version.h>
+#include <linux/module.h>
+#include <linux/delay.h>
+
+#include "../host/xhci.h"
+#include "xhci-dbc.h"
+
+static struct xdbc_state xdbc;
+static int early_console_keep;
+
+#ifdef XDBC_TRACE
+#define	xdbc_trace	trace_printk
+#else
+static inline void xdbc_trace(const char *fmt, ...) { }
+#endif /* XDBC_TRACE */
+
+static int xdbc_bulk_transfer(void *data, int size, bool read);
+
+static void __iomem * __init xdbc_map_pci_mmio(u32 bus, u32 dev, u32 func)
+{
+	u32 val, sz;
+	u64 val64, sz64, mask64;
+	u8 byte;
+	void __iomem *base;
+
+	val = read_pci_config(bus, dev, func, PCI_BASE_ADDRESS_0);
+	write_pci_config(bus, dev, func, PCI_BASE_ADDRESS_0, ~0);
+	sz = read_pci_config(bus, dev, func, PCI_BASE_ADDRESS_0);
+	write_pci_config(bus, dev, func, PCI_BASE_ADDRESS_0, val);
+	if (val == 0xffffffff || sz == 0xffffffff) {
+		pr_notice("invalid mmio bar\n");
+		return NULL;
+	}
+
+	val64 = val & PCI_BASE_ADDRESS_MEM_MASK;
+	sz64 = sz & PCI_BASE_ADDRESS_MEM_MASK;
+	mask64 = (u32)PCI_BASE_ADDRESS_MEM_MASK;
+
+	if ((val & PCI_BASE_ADDRESS_MEM_TYPE_MASK) == PCI_BASE_ADDRESS_MEM_TYPE_64) {
+		val = read_pci_config(bus, dev, func, PCI_BASE_ADDRESS_0 + 4);
+		write_pci_config(bus, dev, func, PCI_BASE_ADDRESS_0 + 4, ~0);
+		sz = read_pci_config(bus, dev, func, PCI_BASE_ADDRESS_0 + 4);
+		write_pci_config(bus, dev, func, PCI_BASE_ADDRESS_0 + 4, val);
+
+		val64 |= (u64)val << 32;
+		sz64 |= (u64)sz << 32;
+		mask64 |= (u64)~0 << 32;
+	}
+
+	sz64 &= mask64;
+
+	if (!sz64) {
+		pr_notice("invalid mmio address\n");
+		return NULL;
+	}
+
+	sz64 = 1ULL << __ffs64(sz64);
+
+	/* check if the mem space is enabled */
+	byte = read_pci_config_byte(bus, dev, func, PCI_COMMAND);
+	if (!(byte & PCI_COMMAND_MEMORY)) {
+		byte |= PCI_COMMAND_MEMORY;
+		write_pci_config_byte(bus, dev, func, PCI_COMMAND, byte);
+	}
+
+	xdbc.xhci_start = val64;
+	xdbc.xhci_length = sz64;
+	base = early_ioremap(val64, sz64);
+
+	return base;
+}
+
+static void * __init xdbc_get_page(dma_addr_t *dma_addr)
+{
+	void *virt;
+
+	virt = alloc_bootmem_pages_nopanic(PAGE_SIZE);
+	if (!virt)
+		return NULL;
+
+	if (dma_addr)
+		*dma_addr = (dma_addr_t)__pa(virt);
+
+	return virt;
+}
+
+static u32 __init xdbc_find_dbgp(int xdbc_num, u32 *b, u32 *d, u32 *f)
+{
+	u32 bus, dev, func, class;
+
+	for (bus = 0; bus < XDBC_PCI_MAX_BUSES; bus++) {
+		for (dev = 0; dev < XDBC_PCI_MAX_DEVICES; dev++) {
+			for (func = 0; func < XDBC_PCI_MAX_FUNCTION; func++) {
+				class = read_pci_config(bus, dev, func, PCI_CLASS_REVISION);
+				if ((class >> 8) != PCI_CLASS_SERIAL_USB_XHCI)
+					continue;
+
+				if (xdbc_num-- != 0)
+					continue;
+
+				*b = bus;
+				*d = dev;
+				*f = func;
+
+				return 0;
+			}
+		}
+	}
+
+	return -1;
+}
+
+static void xdbc_early_delay(unsigned long count)
+{
+	u8 val;
+
+	val = inb(0x80);
+	while (count-- > 0)
+		outb(val, 0x80);
+}
+
+static void xdbc_runtime_delay(unsigned long count)
+{
+	udelay(count);
+}
+
+static void (*xdbc_delay)(unsigned long) = xdbc_early_delay;
+
+static int handshake(void __iomem *ptr, u32 mask, u32 done, int wait, int delay)
+{
+	u32 result;
+
+	do {
+		result = readl(ptr);
+		result &= mask;
+		if (result == done)
+			return 0;
+		xdbc_delay(delay);
+		wait -= delay;
+	} while (wait > 0);
+
+	return -ETIMEDOUT;
+}
+
+static void __init xdbc_bios_handoff(void)
+{
+	int ext_cap_offset;
+	int timeout;
+	u32 val;
+
+	ext_cap_offset = xhci_find_next_ext_cap(xdbc.xhci_base,
+						0, XHCI_EXT_CAPS_LEGACY);
+	val = readl(xdbc.xhci_base + ext_cap_offset);
+
+	/* If the BIOS owns the HC, signal that the OS wants it, and wait */
+	if (val & XHCI_HC_BIOS_OWNED) {
+		writel(val | XHCI_HC_OS_OWNED, xdbc.xhci_base + ext_cap_offset);
+		timeout = handshake(xdbc.xhci_base + ext_cap_offset, XHCI_HC_BIOS_OWNED, 0, 5000, 10);
+
+		/* Assume a buggy BIOS and take HC ownership anyway */
+		if (timeout) {
+			pr_notice("xHCI BIOS handoff failed\n");
+			writel(val & ~XHCI_HC_BIOS_OWNED, xdbc.xhci_base + ext_cap_offset);
+		}
+	}
+
+	/* Disable any BIOS SMIs and clear all SMI events*/
+	val = readl(xdbc.xhci_base + ext_cap_offset + XHCI_LEGACY_CONTROL_OFFSET);
+	val &= XHCI_LEGACY_DISABLE_SMI;
+	val |= XHCI_LEGACY_SMI_EVENTS;
+	writel(val, xdbc.xhci_base + ext_cap_offset + XHCI_LEGACY_CONTROL_OFFSET);
+}
+
+static int __init xdbc_alloc_ring(struct xdbc_segment *seg,
+				  struct xdbc_ring *ring)
+{
+	seg->trbs = xdbc_get_page(&seg->dma);
+	if (!seg->trbs)
+		return -ENOMEM;
+
+	ring->segment = seg;
+
+	return 0;
+}
+
+static void __init xdbc_free_ring(struct xdbc_ring *ring)
+{
+	struct xdbc_segment *seg = ring->segment;
+
+	if (!seg)
+		return;
+
+	free_bootmem(seg->dma, PAGE_SIZE);
+	ring->segment = NULL;
+}
+
+static void xdbc_reset_ring(struct xdbc_ring *ring)
+{
+	struct xdbc_trb *link_trb;
+	struct xdbc_segment *seg = ring->segment;
+
+	memset(seg->trbs, 0, PAGE_SIZE);
+
+	ring->enqueue = seg->trbs;
+	ring->dequeue = seg->trbs;
+	ring->cycle_state = 1;
+
+	if (ring != &xdbc.evt_ring) {
+		link_trb = &seg->trbs[XDBC_TRBS_PER_SEGMENT - 1];
+		link_trb->field[0] = cpu_to_le32(lower_32_bits(seg->dma));
+		link_trb->field[1] = cpu_to_le32(upper_32_bits(seg->dma));
+		link_trb->field[3] = cpu_to_le32(TRB_TYPE(TRB_LINK)) | cpu_to_le32(LINK_TOGGLE);
+	}
+}
+
+static inline void xdbc_put_utf16(u16 *s, const char *c, size_t size)
+{
+	int i;
+
+	for (i = 0; i < size; i++)
+		s[i] = cpu_to_le16(c[i]);
+}
+
+static void xdbc_mem_init(void)
+{
+	struct xdbc_erst_entry *entry;
+	struct xdbc_strings *strings;
+	struct xdbc_context *context;
+	struct xdbc_ep_context *ep_in, *ep_out;
+	struct usb_string_descriptor *s_desc;
+	unsigned int max_burst;
+	u32 string_length;
+	int index = 0;
+	u32 dev_info;
+
+	xdbc_reset_ring(&xdbc.evt_ring);
+	xdbc_reset_ring(&xdbc.in_ring);
+	xdbc_reset_ring(&xdbc.out_ring);
+	memset(xdbc.table_base, 0, PAGE_SIZE);
+	memset(xdbc.out_buf, 0, PAGE_SIZE);
+
+	/* Initialize event ring segment table */
+	xdbc.erst_size = 16;
+	xdbc.erst_base = xdbc.table_base + index * XDBC_TABLE_ENTRY_SIZE;
+	xdbc.erst_dma = xdbc.table_dma + index * XDBC_TABLE_ENTRY_SIZE;
+	index += XDBC_ERST_ENTRY_NUM;
+
+	/* Initialize Event Ring Segment Table */
+	entry = (struct xdbc_erst_entry *)xdbc.erst_base;
+	entry->seg_addr = cpu_to_le64(xdbc.evt_seg.dma);
+	entry->seg_size = cpu_to_le32(XDBC_TRBS_PER_SEGMENT);
+	entry->__reserved_0 = 0;
+
+	/* Initialize ERST registers */
+	writel(1, &xdbc.xdbc_reg->ersts);
+	xdbc_write64(xdbc.erst_dma, &xdbc.xdbc_reg->erstba);
+	xdbc_write64(xdbc.evt_seg.dma, &xdbc.xdbc_reg->erdp);
+
+	/* debug capability contexts */
+	BUILD_BUG_ON(sizeof(struct xdbc_info_context) != 64);
+	BUILD_BUG_ON(sizeof(struct xdbc_ep_context) != 64);
+	BUILD_BUG_ON(sizeof(struct xdbc_context) != 64 * 3);
+
+	xdbc.dbcc_size = 64 * 3;
+	xdbc.dbcc_base = xdbc.table_base + index * XDBC_TABLE_ENTRY_SIZE;
+	xdbc.dbcc_dma = xdbc.table_dma + index * XDBC_TABLE_ENTRY_SIZE;
+	index += XDBC_DBCC_ENTRY_NUM;
+
+	/* strings */
+	xdbc.string_size = sizeof(struct xdbc_strings);
+	xdbc.string_base = xdbc.table_base + index * XDBC_TABLE_ENTRY_SIZE;
+	xdbc.string_dma = xdbc.table_dma + index * XDBC_TABLE_ENTRY_SIZE;
+	index += XDBC_STRING_ENTRY_NUM;
+
+	strings = (struct xdbc_strings *)xdbc.string_base;
+
+	/* serial string */
+	s_desc = (struct usb_string_descriptor *)strings->serial;
+	s_desc->bLength = (strlen(XDBC_STRING_SERIAL) + 1) * 2;
+	s_desc->bDescriptorType = USB_DT_STRING;
+	xdbc_put_utf16(s_desc->wData, XDBC_STRING_SERIAL, strlen(XDBC_STRING_SERIAL));
+
+	string_length = s_desc->bLength;
+	string_length <<= 8;
+
+	/* product string */
+	s_desc = (struct usb_string_descriptor *)strings->product;
+	s_desc->bLength = (strlen(XDBC_STRING_PRODUCT) + 1) * 2;
+	s_desc->bDescriptorType = USB_DT_STRING;
+	xdbc_put_utf16(s_desc->wData, XDBC_STRING_PRODUCT, strlen(XDBC_STRING_PRODUCT));
+
+	string_length += s_desc->bLength;
+	string_length <<= 8;
+
+	/* manufacturer string */
+	s_desc = (struct usb_string_descriptor *)strings->manufacturer;
+	s_desc->bLength = (strlen(XDBC_STRING_MANUFACTURER) + 1) * 2;
+	s_desc->bDescriptorType = USB_DT_STRING;
+	xdbc_put_utf16(s_desc->wData, XDBC_STRING_MANUFACTURER, strlen(XDBC_STRING_MANUFACTURER));
+
+	string_length += s_desc->bLength;
+	string_length <<= 8;
+
+	/* string 0 */
+	strings->string0[0] = 4;
+	strings->string0[1] = USB_DT_STRING;
+	strings->string0[2] = 0x09;
+	strings->string0[3] = 0x04;
+
+	string_length += 4;
+
+	/* populate the contexts */
+	context = (struct xdbc_context *)xdbc.dbcc_base;
+	context->info.string0 = cpu_to_le64(xdbc.string_dma);
+	context->info.manufacturer = cpu_to_le64(xdbc.string_dma + XDBC_MAX_STRING_LENGTH);
+	context->info.product = cpu_to_le64(xdbc.string_dma + XDBC_MAX_STRING_LENGTH * 2);
+	context->info.serial = cpu_to_le64(xdbc.string_dma + XDBC_MAX_STRING_LENGTH * 3);
+	context->info.length = cpu_to_le32(string_length);
+
+	max_burst = DEBUG_MAX_BURST(readl(&xdbc.xdbc_reg->control));
+	ep_out = (struct xdbc_ep_context *)&context->out;
+	ep_out->ep_info1 = 0;
+	ep_out->ep_info2 = cpu_to_le32(EP_TYPE(BULK_OUT_EP) | MAX_PACKET(1024) | MAX_BURST(max_burst));
+	ep_out->deq = cpu_to_le64(xdbc.out_seg.dma | xdbc.out_ring.cycle_state);
+
+	ep_in = (struct xdbc_ep_context *)&context->in;
+	ep_in->ep_info1 = 0;
+	ep_in->ep_info2 = cpu_to_le32(EP_TYPE(BULK_OUT_EP) | MAX_PACKET(1024) | MAX_BURST(max_burst));
+	ep_in->deq = cpu_to_le64(xdbc.in_seg.dma | xdbc.in_ring.cycle_state);
+
+	/* write DbC context pointer register */
+	xdbc_write64(xdbc.dbcc_dma, &xdbc.xdbc_reg->dccp);
+
+	/* device descriptor info registers */
+	dev_info = cpu_to_le32((XDBC_VENDOR_ID << 16) | XDBC_PROTOCOL);
+	writel(dev_info, &xdbc.xdbc_reg->devinfo1);
+	dev_info = cpu_to_le32((XDBC_DEVICE_REV << 16) | XDBC_PRODUCT_ID);
+	writel(dev_info, &xdbc.xdbc_reg->devinfo2);
+
+	xdbc.in_buf = xdbc.out_buf + XDBC_MAX_PACKET;
+	xdbc.in_dma = xdbc.out_dma + XDBC_MAX_PACKET;
+}
+
+static void xdbc_do_reset_debug_port(u32 id, u32 count)
+{
+	u32 val, cap_length;
+	void __iomem *ops_reg;
+	void __iomem *portsc;
+	int i;
+
+	cap_length = readl(xdbc.xhci_base) & 0xff;
+	ops_reg = xdbc.xhci_base + cap_length;
+
+	id--;
+	for (i = id; i < (id + count); i++) {
+		portsc = ops_reg + 0x400 + i * 0x10;
+		val = readl(portsc);
+		/* reset the port if CCS bit is cleared */
+		if (!(val & PORT_CONNECT))
+			writel(val | PORT_RESET, portsc);
+	}
+}
+
+static void __init xdbc_reset_debug_port(void)
+{
+	int offset = 0;
+	u32 val, port_offset, port_count;
+
+	do {
+		offset = xhci_find_next_ext_cap(xdbc.xhci_base,
+						offset,
+						XHCI_EXT_CAPS_PROTOCOL);
+		if (!offset)
+			break;
+
+		val = readl(xdbc.xhci_base + offset);
+		if (XHCI_EXT_PORT_MAJOR(val) != 0x3)
+			continue;
+
+		val = readl(xdbc.xhci_base + offset + 8);
+		port_offset = XHCI_EXT_PORT_OFF(val);
+		port_count = XHCI_EXT_PORT_COUNT(val);
+
+		xdbc_do_reset_debug_port(port_offset, port_count);
+	} while (1);
+}
+
+static void
+xdbc_queue_trb(struct xdbc_ring *ring, u32 field1, u32 field2, u32 field3, u32 field4)
+{
+	struct xdbc_trb *trb, *link_trb;
+
+	trb = ring->enqueue;
+	trb->field[0] = cpu_to_le32(field1);
+	trb->field[1] = cpu_to_le32(field2);
+	trb->field[2] = cpu_to_le32(field3);
+	trb->field[3] = cpu_to_le32(field4);
+
+	++(ring->enqueue);
+	if (ring->enqueue >= &ring->segment->trbs[TRBS_PER_SEGMENT - 1]) {
+		link_trb = ring->enqueue;
+		if (ring->cycle_state)
+			link_trb->field[3] |= cpu_to_le32(TRB_CYCLE);
+		else
+			link_trb->field[3] &= cpu_to_le32(~TRB_CYCLE);
+
+		ring->enqueue = ring->segment->trbs;
+		ring->cycle_state ^= 1;
+	}
+}
+
+static void xdbc_ring_doorbell(int target)
+{
+	writel(DOOR_BELL_TARGET(target), &xdbc.xdbc_reg->doorbell);
+}
+
+static int xdbc_start(void)
+{
+	u32 ctrl, status;
+	int ret;
+
+	ctrl = readl(&xdbc.xdbc_reg->control);
+	writel(ctrl | CTRL_DCE | CTRL_PED, &xdbc.xdbc_reg->control);
+	ret = handshake(&xdbc.xdbc_reg->control, CTRL_DCE, CTRL_DCE, 100000, 100);
+	if (ret) {
+		xdbc_trace("failed to initialize hardware\n");
+		return ret;
+	}
+
+	/* reset port to avoid bus hang */
+	if (xdbc.vendor == PCI_VENDOR_ID_INTEL)
+		xdbc_reset_debug_port();
+
+	/* wait for port connection */
+	ret = handshake(&xdbc.xdbc_reg->portsc, PORTSC_CCS, PORTSC_CCS, 5000000, 100);
+	if (ret) {
+		xdbc_trace("waiting for connection timed out\n");
+		return ret;
+	}
+
+	/* wait for debug device to be configured */
+	ret = handshake(&xdbc.xdbc_reg->control, CTRL_DCR, CTRL_DCR, 5000000, 100);
+	if (ret) {
+		xdbc_trace("waiting for device configuration timed out\n");
+		return ret;
+	}
+
+	/* port should have a valid port# */
+	status = readl(&xdbc.xdbc_reg->status);
+	if (!DCST_DPN(status)) {
+		xdbc_trace("invalid root hub port number\n");
+		return -ENODEV;
+	}
+
+	xdbc.port_number = DCST_DPN(status);
+
+	xdbc_trace("DbC is running now, control 0x%08x port ID %d\n",
+		   readl(&xdbc.xdbc_reg->control), xdbc.port_number);
+
+	return 0;
+}
+
+static int xdbc_handle_external_reset(void)
+{
+	int ret = 0;
+	static int failure_count;
+
+	xdbc_trace("external reset detected\n");
+
+	if (failure_count >= 5)
+		return -ENODEV;
+
+	xdbc.flags = 0;
+	writel(0, &xdbc.xdbc_reg->control);
+	ret = handshake(&xdbc.xdbc_reg->control, CTRL_DCE, 0, 100000, 10);
+	if (ret)
+		goto count_and_out;
+
+	xdbc_mem_init();
+
+	mmiowb();
+
+	ret = xdbc_start();
+	if (ret < 0)
+		goto count_and_out;
+
+	xdbc_trace("dbc recovered\n");
+
+	xdbc.flags |= XDBC_INITIALIZED | XDBC_CONFIGURED;
+
+	xdbc_bulk_transfer(NULL, XDBC_MAX_PACKET, true);
+
+	return 0;
+
+count_and_out:
+	failure_count++;
+	xdbc_trace("failed to recover from external reset, retried %d\n", failure_count);
+	return ret;
+}
+
+static int xdbc_bulk_transfer(void *data, int size, bool read)
+{
+	u64 addr;
+	u32 length, control;
+	struct xdbc_trb *trb;
+	struct xdbc_ring *ring;
+	u32 cycle;
+	int ret;
+
+	if (size > XDBC_MAX_PACKET) {
+		xdbc_trace("oops: bad parameter, size %d\n", size);
+		return -EINVAL;
+	}
+
+	if (!(readl(&xdbc.xdbc_reg->control) & CTRL_DCE)) {
+		ret = xdbc_handle_external_reset();
+		if (ret) {
+			xdbc_trace("oops: hardware failed to recover\n");
+			return -EIO;
+		}
+	}
+
+	if (!(xdbc.flags & XDBC_INITIALIZED) ||
+	    !(xdbc.flags & XDBC_CONFIGURED) ||
+	    (!read && (xdbc.flags & XDBC_BULKOUT_STALL)) ||
+	    (read && (xdbc.flags & XDBC_BULKIN_STALL))) {
+		xdbc_trace("oops: hardware not ready %08x\n", xdbc.flags);
+		return -EIO;
+	}
+
+	ring = (read ? &xdbc.in_ring : &xdbc.out_ring);
+	trb = ring->enqueue;
+	cycle = ring->cycle_state;
+
+	length = TRB_LEN(size);
+	control = TRB_TYPE(TRB_NORMAL) | TRB_IOC;
+
+	if (cycle)
+		control &= cpu_to_le32(~TRB_CYCLE);
+	else
+		control |= cpu_to_le32(TRB_CYCLE);
+
+	if (read) {
+		memset(xdbc.in_buf, 0, XDBC_MAX_PACKET);
+		addr = xdbc.in_dma;
+		xdbc.flags |= XDBC_IN_PROCESS;
+	} else {
+		memset(xdbc.out_buf, 0, XDBC_MAX_PACKET);
+		memcpy(xdbc.out_buf, data, size);
+		addr = xdbc.out_dma;
+		xdbc.flags |= XDBC_OUT_PROCESS;
+	}
+
+	xdbc_queue_trb(ring, lower_32_bits(addr), upper_32_bits(addr),length, control);
+
+	/*
+	 * Memory barrier to ensure hardware sees the trbs
+	 * enqueued above.
+	 */
+	wmb();
+	if (cycle)
+		trb->field[3] |= cpu_to_le32(cycle);
+	else
+		trb->field[3] &= cpu_to_le32(~TRB_CYCLE);
+
+	xdbc_ring_doorbell(read ? IN_EP_DOORBELL : OUT_EP_DOORBELL);
+
+	return size;
+}
+
+static int __init xdbc_early_setup(void)
+{
+	int ret;
+
+	writel(0, &xdbc.xdbc_reg->control);
+	ret = handshake(&xdbc.xdbc_reg->control, CTRL_DCE, 0, 100000, 100);
+	if (ret)
+		return ret;
+
+	/* allocate table page */
+	xdbc.table_base = xdbc_get_page(&xdbc.table_dma);
+	if (!xdbc.table_base)
+		return -ENOMEM;
+
+	/* get and store the transfer buffer */
+	xdbc.out_buf = xdbc_get_page(&xdbc.out_dma);
+	if (!xdbc.out_buf)
+		return -ENOMEM;
+
+	/* allocate event ring */
+	ret = xdbc_alloc_ring(&xdbc.evt_seg, &xdbc.evt_ring);
+	if (ret < 0)
+		return ret;
+
+	/* IN/OUT endpoint transfer ring */
+	ret = xdbc_alloc_ring(&xdbc.in_seg, &xdbc.in_ring);
+	if (ret < 0)
+		return ret;
+
+	ret = xdbc_alloc_ring(&xdbc.out_seg, &xdbc.out_ring);
+	if (ret < 0)
+		return ret;
+
+	xdbc_mem_init();
+
+	/*
+	 * Memory barrier to ensure hardware sees the bits
+	 * setting above.
+	 */
+	mmiowb();
+
+	ret = xdbc_start();
+	if (ret < 0) {
+		/* give the shared port back to host */
+		writel(0, &xdbc.xdbc_reg->control);
+		return ret;
+	}
+
+	xdbc.flags |= XDBC_INITIALIZED | XDBC_CONFIGURED;
+
+	xdbc_bulk_transfer(NULL, XDBC_MAX_PACKET, true);
+
+	return 0;
+}
+
+int __init early_xdbc_parse_parameter(char *s)
+{
+	unsigned long dbgp_num = 0;
+	u32 bus, dev, func, offset;
+	int ret;
+
+	if (!early_pci_allowed())
+		return -EPERM;
+
+	if (strstr(s, "keep"))
+		early_console_keep = 1;
+
+	if (xdbc.xdbc_reg)
+		return 0;
+
+	if (*s && kstrtoul(s, 0, &dbgp_num))
+		dbgp_num = 0;
+
+	pr_notice("dbgp_num: %lu\n", dbgp_num);
+
+	/* Locate the host controller */
+	ret = xdbc_find_dbgp(dbgp_num, &bus, &dev, &func);
+	if (ret) {
+		pr_notice("no host controller found in your system\n");
+		return -ENODEV;
+	}
+	xdbc.vendor = read_pci_config_16(bus, dev, func, PCI_VENDOR_ID);
+	xdbc.device = read_pci_config_16(bus, dev, func, PCI_DEVICE_ID);
+	xdbc.bus = bus;
+	xdbc.dev = dev;
+	xdbc.func = func;
+
+	/* Map the IO memory */
+	xdbc.xhci_base = xdbc_map_pci_mmio(bus, dev, func);
+	if (!xdbc.xhci_base)
+		return -EINVAL;
+
+	/* Locate DbC registers */
+	offset = xhci_find_next_ext_cap(xdbc.xhci_base, 0, XHCI_EXT_CAPS_DEBUG);
+	if (!offset) {
+		pr_notice("DbC wasn't found in your host controller\n");
+		early_iounmap(xdbc.xhci_base, xdbc.xhci_length);
+		xdbc.xhci_base = NULL;
+		xdbc.xhci_length = 0;
+
+		return -ENODEV;
+	}
+	xdbc.xdbc_reg = (struct xdbc_regs __iomem *)(xdbc.xhci_base + offset);
+
+	return 0;
+}
+
+int __init early_xdbc_setup_hardware(void)
+{
+	int ret;
+
+	if (!xdbc.xdbc_reg)
+		return -ENODEV;
+
+	/* hand over the owner of host from BIOS */
+	xdbc_bios_handoff();
+
+	raw_spin_lock_init(&xdbc.lock);
+
+	ret = xdbc_early_setup();
+	if (ret) {
+		pr_notice("failed to setup DbC hardware\n");
+
+		xdbc_free_ring(&xdbc.evt_ring);
+		xdbc_free_ring(&xdbc.out_ring);
+		xdbc_free_ring(&xdbc.in_ring);
+
+		if (xdbc.table_dma)
+			free_bootmem(xdbc.table_dma, PAGE_SIZE);
+
+		if (xdbc.out_dma)
+			free_bootmem(xdbc.out_dma, PAGE_SIZE);
+
+		xdbc.table_base = NULL;
+		xdbc.out_buf = NULL;
+	}
+
+	return ret;
+}
+
+static void xdbc_handle_port_status(struct xdbc_trb *evt_trb)
+{
+	u32 port_reg;
+
+	port_reg = readl(&xdbc.xdbc_reg->portsc);
+
+	if (port_reg & PORTSC_CSC) {
+		xdbc_trace("%s: connect status change event\n", __func__);
+		writel(port_reg | PORTSC_CSC, &xdbc.xdbc_reg->portsc);
+		port_reg = readl(&xdbc.xdbc_reg->portsc);
+	}
+
+	if (port_reg & PORTSC_PRC) {
+		xdbc_trace("%s: port reset change event\n", __func__);
+		writel(port_reg | PORTSC_PRC, &xdbc.xdbc_reg->portsc);
+		port_reg = readl(&xdbc.xdbc_reg->portsc);
+	}
+
+	if (port_reg & PORTSC_PLC) {
+		xdbc_trace("%s: port link status change event\n", __func__);
+		writel(port_reg | PORTSC_PLC, &xdbc.xdbc_reg->portsc);
+		port_reg = readl(&xdbc.xdbc_reg->portsc);
+	}
+
+	if (port_reg & PORTSC_CEC) {
+		xdbc_trace("%s: config error change\n", __func__);
+		writel(port_reg | PORTSC_CEC, &xdbc.xdbc_reg->portsc);
+		port_reg = readl(&xdbc.xdbc_reg->portsc);
+	}
+}
+
+static void xdbc_handle_tx_event(struct xdbc_trb *evt_trb)
+{
+	u32 comp_code;
+	size_t remain_length;
+	int ep_id;
+
+	comp_code = GET_COMP_CODE(le32_to_cpu(evt_trb->field[2]));
+	remain_length = EVENT_TRB_LEN(le32_to_cpu(evt_trb->field[2]));
+	ep_id = TRB_TO_EP_ID(le32_to_cpu(evt_trb->field[3]));
+
+	/*
+	 * Possible Completion Codes for DbC Transfer Event are Success,
+	 * Stall Error, USB Transaction Error, Babble Detected Error,
+	 * TRB Error, Short Packet, Undefined Error, Event Ring Full Error,
+	 * and Vendor Defined Error. TRB error, undefined error and vendor
+	 * defined error will result in HOT/HIT set and be handled the same
+	 * way as Stall error.
+	 */
+	switch (comp_code) {
+	case COMP_SUCCESS:
+		remain_length = 0;
+	case COMP_SHORT_TX:
+		xdbc_trace("%s: endpoint %d remains %ld bytes\n", __func__,
+			   ep_id, remain_length);
+		break;
+	case COMP_TRB_ERR:
+	case COMP_BABBLE:
+	case COMP_TX_ERR:
+	case COMP_STALL:
+	default:
+		if (ep_id == XDBC_EPID_OUT)
+			xdbc.flags |= XDBC_BULKOUT_STALL;
+		if (ep_id == XDBC_EPID_IN)
+			xdbc.flags |= XDBC_BULKIN_STALL;
+
+		xdbc_trace("%s: endpoint %d stalled\n", __func__, ep_id);
+		break;
+	}
+
+	if (ep_id == XDBC_EPID_IN) {
+		xdbc.flags &= ~XDBC_IN_PROCESS;
+		xdbc_bulk_transfer(NULL, XDBC_MAX_PACKET, true);
+	} else if (ep_id == XDBC_EPID_OUT) {
+		xdbc.flags &= ~XDBC_OUT_PROCESS;
+	} else {
+		xdbc_trace("%s: invalid endpoint id %d\n", __func__, ep_id);
+	}
+}
+
+static void xdbc_handle_events(void)
+{
+	struct xdbc_trb *evt_trb;
+	bool update_erdp = false;
+	u8 command;
+	u32 reg;
+
+	command = read_pci_config_byte(xdbc.bus, xdbc.dev, xdbc.func, PCI_COMMAND);
+	if (!(command & PCI_COMMAND_MASTER)) {
+		command |= PCI_COMMAND_MASTER | PCI_COMMAND_MEMORY;
+		write_pci_config_byte(xdbc.bus, xdbc.dev, xdbc.func, PCI_COMMAND, command);
+	}
+
+	/* check and handle configure-exit event */
+	reg = readl(&xdbc.xdbc_reg->control);
+	if (reg & CTRL_DRC) {
+		xdbc.flags &= ~XDBC_CONFIGURED;
+		writel(reg | CTRL_DRC, &xdbc.xdbc_reg->control);
+	} else {
+		xdbc.flags |= XDBC_CONFIGURED;
+	}
+
+	/* check endpoint stall event */
+	reg = readl(&xdbc.xdbc_reg->control);
+	if (reg & CTRL_HIT) {
+		xdbc.flags |= XDBC_BULKIN_STALL;
+	} else {
+		xdbc.flags &= ~XDBC_BULKIN_STALL;
+		if (!(xdbc.flags & XDBC_IN_PROCESS))
+			xdbc_bulk_transfer(NULL, XDBC_MAX_PACKET, true);
+	}
+
+	if (reg & CTRL_HOT)
+		xdbc.flags |= XDBC_BULKOUT_STALL;
+	else
+		xdbc.flags &= ~XDBC_BULKOUT_STALL;
+
+	evt_trb = xdbc.evt_ring.dequeue;
+	while ((le32_to_cpu(evt_trb->field[3]) & TRB_CYCLE) == xdbc.evt_ring.cycle_state) {
+		/*
+		 * Memory barrier to ensure software sees the trbs
+		 * enqueued by hardware.
+		 */
+		rmb();
+
+		switch ((le32_to_cpu(evt_trb->field[3]) & TRB_TYPE_BITMASK)) {
+		case TRB_TYPE(TRB_PORT_STATUS):
+			xdbc_handle_port_status(evt_trb);
+			break;
+		case TRB_TYPE(TRB_TRANSFER):
+			xdbc_handle_tx_event(evt_trb);
+			break;
+		default:
+			break;
+		}
+
+		/* advance to the next trb */
+		++(xdbc.evt_ring.dequeue);
+		if (xdbc.evt_ring.dequeue == &xdbc.evt_seg.trbs[TRBS_PER_SEGMENT]) {
+			xdbc.evt_ring.dequeue = xdbc.evt_seg.trbs;
+			xdbc.evt_ring.cycle_state ^= 1;
+		}
+
+		evt_trb = xdbc.evt_ring.dequeue;
+		update_erdp = true;
+	}
+
+	/* update event ring dequeue pointer */
+	if (update_erdp)
+		xdbc_write64(__pa(xdbc.evt_ring.dequeue), &xdbc.xdbc_reg->erdp);
+}
+
+static int xdbc_bulk_write(const char *bytes, int size)
+{
+	unsigned long flags;
+	int ret, timeout = 0;
+
+retry:
+	if (in_nmi()) {
+		if (!raw_spin_trylock_irqsave(&xdbc.lock, flags))
+			return -EAGAIN;
+	} else {
+		raw_spin_lock_irqsave(&xdbc.lock, flags);
+	}
+
+	xdbc_handle_events();
+
+	/* Check completion of the previous request. */
+	if ((xdbc.flags & XDBC_OUT_PROCESS) && (timeout < 1000000)) {
+		raw_spin_unlock_irqrestore(&xdbc.lock, flags);
+		xdbc_delay(100);
+		timeout += 100;
+		goto retry;
+	}
+
+	if (xdbc.flags & XDBC_OUT_PROCESS) {
+		raw_spin_unlock_irqrestore(&xdbc.lock, flags);
+
+		/*
+		 * Oops, hardware wasn't able to complete the
+		 * previous transfer.
+		 */
+		xdbc_trace("oops: previous transfer not completed yet\n");
+
+		return -ETIMEDOUT;
+	}
+
+	ret = xdbc_bulk_transfer((void *)bytes, size, false);
+
+	raw_spin_unlock_irqrestore(&xdbc.lock, flags);
+
+	return ret;
+}
+
+static void early_xdbc_write(struct console *con, const char *str, u32 n)
+{
+	int chunk, ret;
+	static char buf[XDBC_MAX_PACKET];
+	int use_cr = 0;
+
+	if (!xdbc.xdbc_reg)
+		return;
+	memset(buf, 0, XDBC_MAX_PACKET);
+	while (n > 0) {
+		for (chunk = 0; chunk < XDBC_MAX_PACKET && n > 0; str++, chunk++, n--) {
+			if (!use_cr && *str == '\n') {
+				use_cr = 1;
+				buf[chunk] = '\r';
+				str--;
+				n++;
+				continue;
+			}
+			if (use_cr)
+				use_cr = 0;
+			buf[chunk] = *str;
+		}
+		if (chunk > 0) {
+			ret = xdbc_bulk_write(buf, chunk);
+			if (ret < 0)
+				break;
+		}
+	}
+}
+
+static struct console early_xdbc_console = {
+	.name =		"earlyxdbc",
+	.write =	early_xdbc_write,
+	.flags =	CON_PRINTBUFFER,
+	.index =	-1,
+};
+
+void __init early_xdbc_register_console(void)
+{
+	if (early_console)
+		return;
+
+	early_console = &early_xdbc_console;
+	if (early_console_keep)
+		early_console->flags &= ~CON_BOOT;
+	else
+		early_console->flags |= CON_BOOT;
+	register_console(early_console);
+}
+
+static int __init xdbc_init(void)
+{
+	unsigned long flags;
+	void __iomem *base;
+	u32 offset;
+	int ret = 0;
+
+	if (!(xdbc.flags & XDBC_INITIALIZED))
+		return 0;
+
+	xdbc_delay = xdbc_runtime_delay;
+
+	/*
+	 * It's time to shut down the DbC, so that the debug
+	 * port can be reused by the host controller.
+	 */
+	if (early_xdbc_console.index == -1 ||
+	    (early_xdbc_console.flags & CON_BOOT)) {
+		xdbc_trace("hardware not used anymore\n");
+		goto free_and_quit;
+	}
+
+	base = ioremap_nocache(xdbc.xhci_start, xdbc.xhci_length);
+	if (!base) {
+		xdbc_trace("failed to remap the io address\n");
+		ret = -ENOMEM;
+		goto free_and_quit;
+	}
+
+	raw_spin_lock_irqsave(&xdbc.lock, flags);
+	early_iounmap(xdbc.xhci_base, xdbc.xhci_length);
+	xdbc.xhci_base = base;
+	offset = xhci_find_next_ext_cap(xdbc.xhci_base, 0, XHCI_EXT_CAPS_DEBUG);
+	xdbc.xdbc_reg = (struct xdbc_regs __iomem *)(xdbc.xhci_base + offset);
+	raw_spin_unlock_irqrestore(&xdbc.lock, flags);
+
+	return 0;
+
+free_and_quit:
+	xdbc_free_ring(&xdbc.evt_ring);
+	xdbc_free_ring(&xdbc.out_ring);
+	xdbc_free_ring(&xdbc.in_ring);
+	free_bootmem(xdbc.table_dma, PAGE_SIZE);
+	free_bootmem(xdbc.out_dma, PAGE_SIZE);
+	writel(0, &xdbc.xdbc_reg->control);
+	early_iounmap(xdbc.xhci_base, xdbc.xhci_length);
+
+	return ret;
+}
+subsys_initcall(xdbc_init);
diff --git a/drivers/usb/early/xhci-dbc.h b/drivers/usb/early/xhci-dbc.h
new file mode 100644
index 0000000..896cf04
--- /dev/null
+++ b/drivers/usb/early/xhci-dbc.h
@@ -0,0 +1,205 @@
+/*
+ * xhci-dbc.h - xHCI debug capability early driver
+ *
+ * Copyright (C) 2016 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_DBC_H
+#define __LINUX_XHCI_DBC_H
+
+#include <linux/types.h>
+#include <linux/usb/ch9.h>
+
+/**
+ * struct xdbc_regs - xHCI Debug Capability Register interface.
+ */
+struct xdbc_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;
+#define DEBUG_MAX_BURST(p)	(((p) >> 16) & 0xff)
+#define CTRL_DCR		BIT(0)		/* DbC Run */
+#define CTRL_PED		BIT(1)		/* Port Enable/Disable */
+#define CTRL_HOT		BIT(2)		/* Halt Out TR */
+#define CTRL_HIT		BIT(3)		/* Halt In TR */
+#define CTRL_DRC		BIT(4)		/* DbC run change */
+#define CTRL_DCE		BIT(31)		/* DbC enable */
+	__le32	status;
+#define DCST_DPN(p)		(((p) >> 24) & 0xff)
+	__le32	portsc;		/* Port status and control */
+#define PORTSC_CCS		BIT(0)
+#define PORTSC_CSC		BIT(17)
+#define PORTSC_PRC		BIT(21)
+#define PORTSC_PLC		BIT(22)
+#define PORTSC_CEC		BIT(23)
+	__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 */
+};
+
+/*
+ * xHCI Debug Capability data structures
+ */
+struct xdbc_trb {
+	__le32 field[4];
+};
+
+struct xdbc_erst_entry {
+	__le64	seg_addr;
+	__le32	seg_size;
+	__le32	__reserved_0;
+};
+
+struct xdbc_info_context {
+	__le64	string0;
+	__le64	manufacturer;
+	__le64	product;
+	__le64	serial;
+	__le32	length;
+	__le32	__reserved_0[7];
+};
+
+struct xdbc_ep_context {
+	__le32	ep_info1;
+	__le32	ep_info2;
+	__le64	deq;
+	__le32	tx_info;
+	__le32	__reserved_0[11];
+};
+
+struct xdbc_context {
+	struct xdbc_info_context	info;
+	struct xdbc_ep_context		out;
+	struct xdbc_ep_context		in;
+};
+
+#define XDBC_INFO_CONTEXT_SIZE		48
+#define XDBC_MAX_STRING_LENGTH		64
+#define XDBC_STRING_MANUFACTURER	"Linux"
+#define XDBC_STRING_PRODUCT		"Remote GDB"
+#define XDBC_STRING_SERIAL		"0001"
+
+struct xdbc_strings {
+	char	string0[XDBC_MAX_STRING_LENGTH];
+	char	manufacturer[XDBC_MAX_STRING_LENGTH];
+	char	product[XDBC_MAX_STRING_LENGTH];
+	char	serial[XDBC_MAX_STRING_LENGTH];
+};
+
+#define XDBC_PROTOCOL		1	/* GNU Remote Debug Command Set */
+#define XDBC_VENDOR_ID		0x1d6b	/* Linux Foundation 0x1d6b */
+#define XDBC_PRODUCT_ID		0x0004	/* __le16 idProduct; device 0004 */
+#define XDBC_DEVICE_REV		0x0010	/* 0.10 */
+
+/*
+ * software state structure
+ */
+struct xdbc_segment {
+	struct xdbc_trb		*trbs;
+	dma_addr_t		dma;
+};
+
+#define XDBC_TRBS_PER_SEGMENT	256
+
+struct xdbc_ring {
+	struct xdbc_segment	*segment;
+	struct xdbc_trb		*enqueue;
+	struct xdbc_trb		*dequeue;
+	u32			cycle_state;
+};
+
+#define XDBC_EPID_OUT	2
+#define XDBC_EPID_IN	3
+
+struct xdbc_state {
+	/* pci device info*/
+	u16			vendor;
+	u16			device;
+	u32			bus;
+	u32			dev;
+	u32			func;
+	void __iomem		*xhci_base;
+	u64			xhci_start;
+	size_t			xhci_length;
+	int			port_number;
+#define XDBC_PCI_MAX_BUSES	256
+#define XDBC_PCI_MAX_DEVICES	32
+#define XDBC_PCI_MAX_FUNCTION	8
+
+	/* DbC register base */
+	struct xdbc_regs __iomem *xdbc_reg;
+
+	/* DbC table page */
+	dma_addr_t		table_dma;
+	void			*table_base;
+
+#define XDBC_TABLE_ENTRY_SIZE	64
+#define XDBC_ERST_ENTRY_NUM	1
+#define XDBC_DBCC_ENTRY_NUM	3
+#define XDBC_STRING_ENTRY_NUM	4
+
+	/* event ring segment table */
+	dma_addr_t		erst_dma;
+	size_t			erst_size;
+	void			*erst_base;
+
+	/* event ring segments */
+	struct xdbc_ring	evt_ring;
+	struct xdbc_segment	evt_seg;
+
+	/* debug capability contexts */
+	dma_addr_t		dbcc_dma;
+	size_t			dbcc_size;
+	void			*dbcc_base;
+
+	/* descriptor strings */
+	dma_addr_t		string_dma;
+	size_t			string_size;
+	void			*string_base;
+
+	/* bulk OUT endpoint */
+	struct xdbc_ring	out_ring;
+	struct xdbc_segment	out_seg;
+	void			*out_buf;
+	dma_addr_t		out_dma;
+
+	/* bulk IN endpoint */
+	struct xdbc_ring	in_ring;
+	struct xdbc_segment	in_seg;
+	void			*in_buf;
+	dma_addr_t		in_dma;
+
+	u32			flags;
+#define XDBC_INITIALIZED	BIT(0)
+#define XDBC_BULKIN_STALL	BIT(1)
+#define XDBC_BULKOUT_STALL	BIT(2)
+#define XDBC_IN_PROCESS		BIT(3)
+#define XDBC_OUT_PROCESS	BIT(4)
+#define XDBC_CONFIGURED		BIT(5)
+
+	/* spinlock for early_xdbc_write() reentrancy */
+	raw_spinlock_t		lock;
+};
+
+#define XDBC_MAX_PACKET		1024
+
+/* door bell target */
+#define OUT_EP_DOORBELL		0
+#define IN_EP_DOORBELL		1
+#define DOOR_BELL_TARGET(p)	(((p) & 0xff) << 8)
+
+#define xdbc_read64(regs)	xhci_read_64(NULL, (regs))
+#define xdbc_write64(val, regs)	xhci_write_64(NULL, (val), (regs))
+
+#endif /* __LINUX_XHCI_DBC_H */
diff --git a/include/linux/usb/xhci-dbgp.h b/include/linux/usb/xhci-dbgp.h
new file mode 100644
index 0000000..80c1cca
--- /dev/null
+++ b/include/linux/usb/xhci-dbgp.h
@@ -0,0 +1,29 @@
+/*
+ * Standalone xHCI debug capability driver
+ *
+ * Copyright (C) 2016 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_DBGP_H
+#define __LINUX_XHCI_DBGP_H
+
+#ifdef CONFIG_EARLY_PRINTK_USB_XDBC
+int __init early_xdbc_parse_parameter(char *s);
+int __init early_xdbc_setup_hardware(void);
+void __init early_xdbc_register_console(void);
+#else
+static inline int __init early_xdbc_setup_hardware(void)
+{
+	return -ENODEV;
+}
+static inline void __init early_xdbc_register_console(void)
+{
+}
+#endif /* CONFIG_EARLY_PRINTK_USB_XDBC */
+#endif /* __LINUX_XHCI_DBGP_H */
-- 
2.1.4

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

* [PATCH v6 2/4] x86: add support for earlyprintk via USB3 debug port
  2017-01-22  6:19 [PATCH v6 0/4] usb: early: add support for early printk through USB3 debug port Lu Baolu
  2017-01-22  6:19 ` [PATCH v6 1/4] usb: dbc: early driver for xhci debug capability Lu Baolu
@ 2017-01-22  6:19 ` Lu Baolu
  2017-01-22  6:19 ` [PATCH v6 3/4] usb: serial: add dbc debug device support to usb_debug Lu Baolu
  2017-01-22  6:19 ` [PATCH v6 4/4] usb: doc: add document for USB3 debug port usage Lu Baolu
  3 siblings, 0 replies; 7+ messages in thread
From: Lu Baolu @ 2017-01-22  6:19 UTC (permalink / raw)
  To: Greg Kroah-Hartman, Ingo Molnar
  Cc: Mathias Nyman, tglx, peterz, linux-usb, x86, linux-kernel, Lu Baolu

Add support for early printk by writing debug messages to the
USB3 debug port.   Users can use this type of early printk by
specifying kernel parameter of "earlyprintk=xdbc". This gives
users a chance of providing debug output.

The hardware for USB3 debug port requires DMA memory blocks.
This requires to delay setting up debugging hardware and
registering boot console until the memblocks are filled.

Cc: Ingo Molnar <mingo@redhat.com>
Cc: x86@kernel.org
Signed-off-by: Lu Baolu <baolu.lu@linux.intel.com>
---
 Documentation/admin-guide/kernel-parameters.txt | 1 +
 arch/x86/kernel/early_printk.c                  | 5 +++++
 arch/x86/kernel/setup.c                         | 4 ++++
 3 files changed, 10 insertions(+)

diff --git a/Documentation/admin-guide/kernel-parameters.txt b/Documentation/admin-guide/kernel-parameters.txt
index be7c0d9..4bf6138 100644
--- a/Documentation/admin-guide/kernel-parameters.txt
+++ b/Documentation/admin-guide/kernel-parameters.txt
@@ -988,6 +988,7 @@
 			earlyprintk=ttySn[,baudrate]
 			earlyprintk=dbgp[debugController#]
 			earlyprintk=pciserial,bus:device.function[,baudrate]
+			earlyprintk=xdbc[xhciController#]
 
 			earlyprintk is useful when the kernel crashes before
 			the normal console is initialized. It is not enabled by
diff --git a/arch/x86/kernel/early_printk.c b/arch/x86/kernel/early_printk.c
index 8a12199..0f08403 100644
--- a/arch/x86/kernel/early_printk.c
+++ b/arch/x86/kernel/early_printk.c
@@ -17,6 +17,7 @@
 #include <asm/intel-mid.h>
 #include <asm/pgtable.h>
 #include <linux/usb/ehci_def.h>
+#include <linux/usb/xhci-dbgp.h>
 #include <linux/efi.h>
 #include <asm/efi.h>
 #include <asm/pci_x86.h>
@@ -381,6 +382,10 @@ static int __init setup_early_printk(char *buf)
 		if (!strncmp(buf, "efi", 3))
 			early_console_register(&early_efi_console, keep);
 #endif
+#ifdef CONFIG_EARLY_PRINTK_USB_XDBC
+		if (!strncmp(buf, "xdbc", 4))
+			early_xdbc_parse_parameter(buf + 4);
+#endif
 
 		buf++;
 	}
diff --git a/arch/x86/kernel/setup.c b/arch/x86/kernel/setup.c
index 4cfba94..1f27d91 100644
--- a/arch/x86/kernel/setup.c
+++ b/arch/x86/kernel/setup.c
@@ -70,6 +70,7 @@
 #include <linux/tboot.h>
 #include <linux/jiffies.h>
 
+#include <linux/usb/xhci-dbgp.h>
 #include <video/edid.h>
 
 #include <asm/mtrr.h>
@@ -1120,6 +1121,9 @@ void __init setup_arch(char **cmdline_p)
 	memblock_set_current_limit(ISA_END_ADDRESS);
 	memblock_x86_fill();
 
+	if (!early_xdbc_setup_hardware())
+		early_xdbc_register_console();
+
 	reserve_bios_regions();
 
 	if (efi_enabled(EFI_MEMMAP)) {
-- 
2.1.4

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

* [PATCH v6 3/4] usb: serial: add dbc debug device support to usb_debug
  2017-01-22  6:19 [PATCH v6 0/4] usb: early: add support for early printk through USB3 debug port Lu Baolu
  2017-01-22  6:19 ` [PATCH v6 1/4] usb: dbc: early driver for xhci debug capability Lu Baolu
  2017-01-22  6:19 ` [PATCH v6 2/4] x86: add support for earlyprintk via USB3 debug port Lu Baolu
@ 2017-01-22  6:19 ` Lu Baolu
  2017-01-22  6:19 ` [PATCH v6 4/4] usb: doc: add document for USB3 debug port usage Lu Baolu
  3 siblings, 0 replies; 7+ messages in thread
From: Lu Baolu @ 2017-01-22  6:19 UTC (permalink / raw)
  To: Greg Kroah-Hartman, Ingo Molnar
  Cc: Mathias Nyman, tglx, peterz, linux-usb, x86, linux-kernel, Lu Baolu

This patch adds dbc debug device support to the usb_debug driver.

Signed-off-by: Lu Baolu <baolu.lu@linux.intel.com>
Acked-by: Johan Hovold <johan@kernel.org>
---
 drivers/usb/serial/usb_debug.c | 28 +++++++++++++++++++++++++---
 1 file changed, 25 insertions(+), 3 deletions(-)

diff --git a/drivers/usb/serial/usb_debug.c b/drivers/usb/serial/usb_debug.c
index ca2fa5b..92f7e5c 100644
--- a/drivers/usb/serial/usb_debug.c
+++ b/drivers/usb/serial/usb_debug.c
@@ -32,7 +32,18 @@ static const struct usb_device_id id_table[] = {
 	{ USB_DEVICE(0x0525, 0x127a) },
 	{ },
 };
-MODULE_DEVICE_TABLE(usb, id_table);
+
+static const struct usb_device_id dbc_id_table[] = {
+	{ USB_DEVICE(0x1d6b, 0x0004) },
+	{ },
+};
+
+static const struct usb_device_id id_table_combined[] = {
+	{ USB_DEVICE(0x0525, 0x127a) },
+	{ USB_DEVICE(0x1d6b, 0x0004) },
+	{ },
+};
+MODULE_DEVICE_TABLE(usb, id_table_combined);
 
 /* This HW really does not support a serial break, so one will be
  * emulated when ever the break state is set to true.
@@ -71,9 +82,20 @@ static struct usb_serial_driver debug_device = {
 	.process_read_urb =	usb_debug_process_read_urb,
 };
 
+static struct usb_serial_driver dbc_device = {
+	.driver = {
+		.owner =	THIS_MODULE,
+		.name =		"xhci_dbc",
+	},
+	.id_table =		dbc_id_table,
+	.num_ports =		1,
+	.break_ctl =		usb_debug_break_ctl,
+	.process_read_urb =	usb_debug_process_read_urb,
+};
+
 static struct usb_serial_driver * const serial_drivers[] = {
-	&debug_device, NULL
+	&debug_device, &dbc_device, NULL
 };
 
-module_usb_serial_driver(serial_drivers, id_table);
+module_usb_serial_driver(serial_drivers, id_table_combined);
 MODULE_LICENSE("GPL");
-- 
2.1.4

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

* [PATCH v6 4/4] usb: doc: add document for USB3 debug port usage
  2017-01-22  6:19 [PATCH v6 0/4] usb: early: add support for early printk through USB3 debug port Lu Baolu
                   ` (2 preceding siblings ...)
  2017-01-22  6:19 ` [PATCH v6 3/4] usb: serial: add dbc debug device support to usb_debug Lu Baolu
@ 2017-01-22  6:19 ` Lu Baolu
  3 siblings, 0 replies; 7+ messages in thread
From: Lu Baolu @ 2017-01-22  6:19 UTC (permalink / raw)
  To: Greg Kroah-Hartman, Ingo Molnar
  Cc: Mathias Nyman, tglx, peterz, linux-usb, x86, linux-kernel,
	Lu Baolu, linux-doc

Add Documentation/usb/usb3-debug-port.rst. This document includes
the user guide for USB3 debug port.

Cc: linux-doc@vger.kernel.org
Signed-off-by: Lu Baolu <baolu.lu@linux.intel.com>
---
 Documentation/usb/usb3-debug-port.rst | 98 +++++++++++++++++++++++++++++++++++
 1 file changed, 98 insertions(+)
 create mode 100644 Documentation/usb/usb3-debug-port.rst

diff --git a/Documentation/usb/usb3-debug-port.rst b/Documentation/usb/usb3-debug-port.rst
new file mode 100644
index 0000000..9eddb3a
--- /dev/null
+++ b/Documentation/usb/usb3-debug-port.rst
@@ -0,0 +1,98 @@
+===============
+USB3 debug port
+===============
+
+:Author: Lu Baolu <baolu.lu@linux.intel.com>
+:Date: January 2017
+
+GENERAL
+=======
+
+This is a HOWTO for using USB3 debug port on x86 systems.
+
+Before using any kernel debugging functionality based on USB3
+debug port, you need to check 1) whether debug port is supported
+by the xHCI host; 2) which port is used for debugging purposes
+(normally the first USB3 root port). You must have a USB 3.0
+super-speed A-to-A debugging cable to connect the debug target
+with a debug host. In this document, "debug target" stands for
+the system under debugging, and "debug host" stands for a
+stand-alone system that is able to talk to the debugging target
+through the USB3 debug port.
+
+EARLY PRINTK
+============
+
+On the debug target system, you need to customize a debugging
+kernel with CONFIG_EARLY_PRINTK_USB_XDBC enabled. And, add
+below kernel boot parameter::
+
+	"earlyprintk=xdbc"
+
+If there are multiple xHCI controllers in the system, you can
+append a host contoller index to this kernel parameter. This
+index starts from 0.
+
+If you are going to use the "keep" option defined by the
+early printk framework to keep the boot console alive after
+early boot, you'd better add below kernel boot parameter::
+
+	"usbcore.autosuspend=-1"
+
+On the debug host side, you don't need to customize the kernel,
+but you'd better disable usb subsystem runtime power management
+by adding below kernel boot parameter::
+
+	"usbcore.autosuspend=-1"
+
+Before starting the debug target, you should connect the debug
+port on the debug target with a root port or port of any external
+hub on the debug host. The cable used to connect these two ports
+should be a USB 3.0 super-speed A-to-A debugging cable.
+
+During early boot of the debug target, DbC (xHCI debug engine for
+USB3 debug port) hardware will be detected and initialized. After
+initialization, the debug host should be able to enumerate the
+debug target as a debug device. The debug host will then bind the
+debug device with the usb_debug driver module and create the
+/dev/ttyUSB0 device.
+
+If the debug device enumeration goes smoothly, you should be able
+to see below kernel messages on the debug host::
+
+	# tail -f /var/log/kern.log
+	[ 1815.983374] usb 4-3: new SuperSpeed USB device number 4 using xhci_hcd
+	[ 1815.999595] usb 4-3: LPM exit latency is zeroed, disabling LPM.
+	[ 1815.999899] usb 4-3: New USB device found, idVendor=1d6b, idProduct=0004
+	[ 1815.999902] usb 4-3: New USB device strings: Mfr=1, Product=2, SerialNumber=3
+	[ 1815.999903] usb 4-3: Product: Remote GDB
+	[ 1815.999904] usb 4-3: Manufacturer: Linux
+	[ 1815.999905] usb 4-3: SerialNumber: 0001
+	[ 1816.000240] usb_debug 4-3:1.0: xhci_dbc converter detected
+	[ 1816.000360] usb 4-3: xhci_dbc converter now attached to ttyUSB0
+
+You can run below bash scripts on the debug host to read the kernel
+log sent from debug target.
+
+.. code-block:: sh
+
+	===== start of bash scripts =============
+	#!/bin/bash
+
+	while true ; do
+		while [ ! -d /sys/class/tty/ttyUSB0 ] ; do
+			:
+	done
+	cat /dev/ttyUSB0 >> xdbc.log
+	done
+	===== end of bash scripts ===============
+
+You should be able to see the early boot message in xdbc.log.
+
+If it doesn't work, please ask it on the <linux-usb@vger.kernel.org>
+mailing list.
+
+Below USB hosts have been verified to work::
+
+	Intel Corporation Sunrise Point-H USB 3.0 xHCI Controller
+	Intel Corporation Wildcat Point-LP USB xHCI Controller
-- 
2.1.4

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

* Re: [PATCH v6 1/4] usb: dbc: early driver for xhci debug capability
  2017-01-22  6:19 ` [PATCH v6 1/4] usb: dbc: early driver for xhci debug capability Lu Baolu
@ 2017-01-22  9:31   ` Ingo Molnar
  2017-01-24  4:49     ` Lu Baolu
  0 siblings, 1 reply; 7+ messages in thread
From: Ingo Molnar @ 2017-01-22  9:31 UTC (permalink / raw)
  To: Lu Baolu
  Cc: Greg Kroah-Hartman, Ingo Molnar, Mathias Nyman, tglx, peterz,
	linux-usb, x86, linux-kernel


* Lu Baolu <baolu.lu@linux.intel.com> wrote:

> xHCI debug capability (DbC) is an optional but standalone
> functionality provided by an xHCI host controller. Software
> learns this capability by walking through the extended
> capability list of the host. xHCI specification describes
> DbC in section 7.6.
> 
> This patch introduces the code to probe and initialize the
> debug capability hardware during early boot. With hardware
> initialized, the debug target (system on which this code is
> running) will present a debug device through the debug port
> (normally the first USB3 port). The debug device is fully
> compliant with the USB framework and provides the equivalent
> of a very high performance (USB3) full-duplex serial link
> between the debug host and target. The DbC functionality is
> independent of xHCI host. There isn't any precondition from
> xHCI host side for DbC to work.

s/xHCI host/the xHCI host

> This patch also includes bulk out and bulk in interfaces.
> These interfaces could be used to implement early printk
> bootconsole or hook to various system debuggers.

s/out/output
s/in/input

> +config EARLY_PRINTK_USB_XDBC
> +	bool "Early printk via xHCI debug port"

s/xHCI/the xHCI

I remarked on this in my first review as well - mind checking the whole series for 
uses of 'xHCI'?

> +	depends on EARLY_PRINTK && PCI
> +	select EARLY_PRINTK_USB
> +	---help---
> +	  Write kernel log output directly into the xHCI debug port.
> +
> +	  This is useful for kernel debugging when your machine crashes very
> +	  early before the console code is initialized. For normal operation
> +	  it is not recommended because it looks ugly and doesn't cooperate
> +	  with klogd/syslogd or the X server. You should normally N here,
> +	  unless you want to debug such a crash.
> +
>  config X86_PTDUMP_CORE
>  	def_bool n
>  
> diff --git a/drivers/usb/Kconfig b/drivers/usb/Kconfig
> index fbe493d..9313fff 100644
> --- a/drivers/usb/Kconfig
> +++ b/drivers/usb/Kconfig
> @@ -19,6 +19,9 @@ config USB_EHCI_BIG_ENDIAN_MMIO
>  config USB_EHCI_BIG_ENDIAN_DESC
>  	bool
>  
> +config USB_EARLY_PRINTK
> +	bool
> +
>  menuconfig USB_SUPPORT
>  	bool "USB support"
>  	depends on HAS_IOMEM
> diff --git a/drivers/usb/Makefile b/drivers/usb/Makefile
> index 7791af6..53d1356 100644
> --- a/drivers/usb/Makefile
> +++ b/drivers/usb/Makefile
> @@ -49,7 +49,7 @@ obj-$(CONFIG_USB_MICROTEK)	+= image/
>  obj-$(CONFIG_USB_SERIAL)	+= serial/
>  
>  obj-$(CONFIG_USB)		+= misc/
> -obj-$(CONFIG_EARLY_PRINTK_DBGP)	+= early/
> +obj-$(CONFIG_EARLY_PRINTK_USB)	+= early/
>  
>  obj-$(CONFIG_USB_ATM)		+= atm/
>  obj-$(CONFIG_USB_SPEEDTOUCH)	+= atm/
> diff --git a/drivers/usb/early/Makefile b/drivers/usb/early/Makefile
> index 24bbe51..fcde228 100644
> --- a/drivers/usb/early/Makefile
> +++ b/drivers/usb/early/Makefile
> @@ -3,3 +3,4 @@
>  #
>  
>  obj-$(CONFIG_EARLY_PRINTK_DBGP) += ehci-dbgp.o
> +obj-$(CONFIG_EARLY_PRINTK_USB_XDBC) += xhci-dbc.o
> diff --git a/drivers/usb/early/xhci-dbc.c b/drivers/usb/early/xhci-dbc.c
> new file mode 100644
> index 0000000..72480c4
> --- /dev/null
> +++ b/drivers/usb/early/xhci-dbc.c
> @@ -0,0 +1,1027 @@
> +/**
> + * xhci-dbc.c - xHCI debug capability early driver
> + *
> + * Copyright (C) 2016 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.
> + */
> +
> +#define pr_fmt(fmt)	KBUILD_MODNAME ":%s: " fmt, __func__
> +
> +#include <linux/console.h>
> +#include <linux/pci_regs.h>
> +#include <linux/pci_ids.h>
> +#include <linux/bootmem.h>
> +#include <linux/io.h>
> +#include <asm/pci-direct.h>
> +#include <asm/fixmap.h>
> +#include <linux/bcd.h>
> +#include <linux/export.h>
> +#include <linux/version.h>
> +#include <linux/module.h>
> +#include <linux/delay.h>
> +
> +#include "../host/xhci.h"
> +#include "xhci-dbc.h"
> +
> +static struct xdbc_state xdbc;
> +static int early_console_keep;
> +
> +#ifdef XDBC_TRACE
> +#define	xdbc_trace	trace_printk
> +#else
> +static inline void xdbc_trace(const char *fmt, ...) { }
> +#endif /* XDBC_TRACE */
> +
> +static int xdbc_bulk_transfer(void *data, int size, bool read);
> +
> +static void __iomem * __init xdbc_map_pci_mmio(u32 bus, u32 dev, u32 func)
> +{
> +	u32 val, sz;
> +	u64 val64, sz64, mask64;
> +	u8 byte;
> +	void __iomem *base;
> +
> +	val = read_pci_config(bus, dev, func, PCI_BASE_ADDRESS_0);
> +	write_pci_config(bus, dev, func, PCI_BASE_ADDRESS_0, ~0);
> +	sz = read_pci_config(bus, dev, func, PCI_BASE_ADDRESS_0);
> +	write_pci_config(bus, dev, func, PCI_BASE_ADDRESS_0, val);
> +	if (val == 0xffffffff || sz == 0xffffffff) {
> +		pr_notice("invalid mmio bar\n");
> +		return NULL;
> +	}
> +
> +	val64 = val & PCI_BASE_ADDRESS_MEM_MASK;
> +	sz64 = sz & PCI_BASE_ADDRESS_MEM_MASK;
> +	mask64 = (u32)PCI_BASE_ADDRESS_MEM_MASK;

Is this cast necessary?

> +
> +	if ((val & PCI_BASE_ADDRESS_MEM_TYPE_MASK) == PCI_BASE_ADDRESS_MEM_TYPE_64) {
> +		val = read_pci_config(bus, dev, func, PCI_BASE_ADDRESS_0 + 4);
> +		write_pci_config(bus, dev, func, PCI_BASE_ADDRESS_0 + 4, ~0);
> +		sz = read_pci_config(bus, dev, func, PCI_BASE_ADDRESS_0 + 4);
> +		write_pci_config(bus, dev, func, PCI_BASE_ADDRESS_0 + 4, val);
> +
> +		val64 |= (u64)val << 32;
> +		sz64 |= (u64)sz << 32;

Could these type casts be avoided by declaring 'val' and 'sz' as u64 to begin 
with?

> +		mask64 |= (u64)~0 << 32;

Type cast could be avoided by using 0L.

> +static u32 __init xdbc_find_dbgp(int xdbc_num, u32 *b, u32 *d, u32 *f)
> +{
> +	u32 bus, dev, func, class;
> +
> +	for (bus = 0; bus < XDBC_PCI_MAX_BUSES; bus++) {
> +		for (dev = 0; dev < XDBC_PCI_MAX_DEVICES; dev++) {
> +			for (func = 0; func < XDBC_PCI_MAX_FUNCTION; func++) {
> +				class = read_pci_config(bus, dev, func, PCI_CLASS_REVISION);
> +				if ((class >> 8) != PCI_CLASS_SERIAL_USB_XHCI)
> +					continue;
> +
> +				if (xdbc_num-- != 0)
> +					continue;
> +
> +				*b = bus;
> +				*d = dev;
> +				*f = func;
> +
> +				return 0;
> +			}
> +		}
> +	}

Nit: to me it would look more balanced visually if the body of the innermost loop 
started with an empty newline.

> +	ext_cap_offset = xhci_find_next_ext_cap(xdbc.xhci_base,
> +						0, XHCI_EXT_CAPS_LEGACY);

Please don't break this line. You can shorten the variable name if you want to 
save line length.

> +	/* populate the contexts */
> +	context = (struct xdbc_context *)xdbc.dbcc_base;
> +	context->info.string0 = cpu_to_le64(xdbc.string_dma);
> +	context->info.manufacturer = cpu_to_le64(xdbc.string_dma + XDBC_MAX_STRING_LENGTH);
> +	context->info.product = cpu_to_le64(xdbc.string_dma + XDBC_MAX_STRING_LENGTH * 2);
> +	context->info.serial = cpu_to_le64(xdbc.string_dma + XDBC_MAX_STRING_LENGTH * 3);
> +	context->info.length = cpu_to_le32(string_length);

Wouldn't this (and some of the other initializations) look nicer this way:

	/* Populate the contexts: */
	context = (struct xdbc_context *)xdbc.dbcc_base;

	context->info.string0		= cpu_to_le64(xdbc.string_dma);
	context->info.manufacturer	= cpu_to_le64(xdbc.string_dma + XDBC_MAX_STRING_LENGTH);
	context->info.product		= cpu_to_le64(xdbc.string_dma + XDBC_MAX_STRING_LENGTH * 2);
	context->info.serial		= cpu_to_le64(xdbc.string_dma + XDBC_MAX_STRING_LENGTH * 3);
	context->info.length		= cpu_to_le32(string_length);

?

> +	/* Check completion of the previous request. */

Could you please standardize the capitalization and punctuation of all the 
comments in the patches? Some are lower case, some are upper case, some have full 
stops, some don't.

One good style would be to use this form when a comment refers to what the next 
statement does:

	/* Check completion of the previous request: */

> +/**
> + * struct xdbc_regs - xHCI Debug Capability Register interface.
> + */
> +struct xdbc_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;
> +#define DEBUG_MAX_BURST(p)	(((p) >> 16) & 0xff)
> +#define CTRL_DCR		BIT(0)		/* DbC Run */
> +#define CTRL_PED		BIT(1)		/* Port Enable/Disable */
> +#define CTRL_HOT		BIT(2)		/* Halt Out TR */
> +#define CTRL_HIT		BIT(3)		/* Halt In TR */
> +#define CTRL_DRC		BIT(4)		/* DbC run change */
> +#define CTRL_DCE		BIT(31)		/* DbC enable */
> +	__le32	status;
> +#define DCST_DPN(p)		(((p) >> 24) & 0xff)
> +	__le32	portsc;		/* Port status and control */
> +#define PORTSC_CCS		BIT(0)
> +#define PORTSC_CSC		BIT(17)
> +#define PORTSC_PRC		BIT(21)
> +#define PORTSC_PLC		BIT(22)
> +#define PORTSC_CEC		BIT(23)
> +	__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 */
> +};

So why are defines intermixed with the fields? If the defines relate to the field 
then their naming sure does not express that ...

I was thinking of something like:

/**
 * struct xdbc_regs - xHCI Debug Capability Register interface.
 */
struct xdbc_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 */
};

#define DEBUG_MAX_BURST(p)	(((p) >> 16) & 0xff)

#define	CTRL_DCR		BIT(0)		/* DbC Run */
#define	CTRL_PED		BIT(1)		/* Port Enable/Disable */
#define	CTRL_HOT		BIT(2)		/* Halt Out TR */
#define	CTRL_HIT		BIT(3)		/* Halt In TR */
#define	CTRL_DRC		BIT(4)		/* DbC run change */
#define CTRL_DCE		BIT(31)		/* DbC enable */

#define DCST_DPN(p)		(((p) >> 24) & 0xff)

#define PORTSC_CCS		BIT(0)
#define PORTSC_CSC		BIT(17)
#define PORTSC_PRC		BIT(21)
#define PORTSC_PLC		BIT(22)
#define PORTSC_CEC		BIT(23)

Also, the abbreviations suck big time. What's wrong with:

#define	CTRL_DBC_RUN		BIT(0)
#define	CTRL_PORT_ENABLE	BIT(1)
#define	CTRL_HALT_OUT_TR	BIT(2)
#define	CTRL_HALT_IN_TR		BIT(3)
#define	CTRL_DBC_RUN_CHANGE	BIT(4)
#define CTRL_DBC_ENABLE		BIT(31)

?

Also note how the comments became superfluous once descriptive names are chosen...

Please review the rest of the defines and series for similar problems and fix 
them, there are more of the same type.

Thanks,

	Ingo

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

* Re: [PATCH v6 1/4] usb: dbc: early driver for xhci debug capability
  2017-01-22  9:31   ` Ingo Molnar
@ 2017-01-24  4:49     ` Lu Baolu
  0 siblings, 0 replies; 7+ messages in thread
From: Lu Baolu @ 2017-01-24  4:49 UTC (permalink / raw)
  To: Ingo Molnar
  Cc: Greg Kroah-Hartman, Ingo Molnar, Mathias Nyman, tglx, peterz,
	linux-usb, x86, linux-kernel

Hi Ingo,

On 01/22/2017 05:31 PM, Ingo Molnar wrote:
> * Lu Baolu <baolu.lu@linux.intel.com> wrote:
>
>> xHCI debug capability (DbC) is an optional but standalone
>> functionality provided by an xHCI host controller. Software
>> learns this capability by walking through the extended
>> capability list of the host. xHCI specification describes
>> DbC in section 7.6.
>>
>> This patch introduces the code to probe and initialize the
>> debug capability hardware during early boot. With hardware
>> initialized, the debug target (system on which this code is
>> running) will present a debug device through the debug port
>> (normally the first USB3 port). The debug device is fully
>> compliant with the USB framework and provides the equivalent
>> of a very high performance (USB3) full-duplex serial link
>> between the debug host and target. The DbC functionality is
>> independent of xHCI host. There isn't any precondition from
>> xHCI host side for DbC to work.
> s/xHCI host/the xHCI host
>
>> This patch also includes bulk out and bulk in interfaces.
>> These interfaces could be used to implement early printk
>> bootconsole or hook to various system debuggers.
> s/out/output
> s/in/input
>
>> +config EARLY_PRINTK_USB_XDBC
>> +	bool "Early printk via xHCI debug port"
> s/xHCI/the xHCI
>
> I remarked on this in my first review as well - mind checking the whole series for 
> uses of 'xHCI'?
>
>> +	depends on EARLY_PRINTK && PCI
>> +	select EARLY_PRINTK_USB
>> +	---help---
>> +	  Write kernel log output directly into the xHCI debug port.
>> +
>> +	  This is useful for kernel debugging when your machine crashes very
>> +	  early before the console code is initialized. For normal operation
>> +	  it is not recommended because it looks ugly and doesn't cooperate
>> +	  with klogd/syslogd or the X server. You should normally N here,
>> +	  unless you want to debug such a crash.
>> +
>>  config X86_PTDUMP_CORE
>>  	def_bool n
>>  
>> diff --git a/drivers/usb/Kconfig b/drivers/usb/Kconfig
>> index fbe493d..9313fff 100644
>> --- a/drivers/usb/Kconfig
>> +++ b/drivers/usb/Kconfig
>> @@ -19,6 +19,9 @@ config USB_EHCI_BIG_ENDIAN_MMIO
>>  config USB_EHCI_BIG_ENDIAN_DESC
>>  	bool
>>  
>> +config USB_EARLY_PRINTK
>> +	bool
>> +
>>  menuconfig USB_SUPPORT
>>  	bool "USB support"
>>  	depends on HAS_IOMEM
>> diff --git a/drivers/usb/Makefile b/drivers/usb/Makefile
>> index 7791af6..53d1356 100644
>> --- a/drivers/usb/Makefile
>> +++ b/drivers/usb/Makefile
>> @@ -49,7 +49,7 @@ obj-$(CONFIG_USB_MICROTEK)	+= image/
>>  obj-$(CONFIG_USB_SERIAL)	+= serial/
>>  
>>  obj-$(CONFIG_USB)		+= misc/
>> -obj-$(CONFIG_EARLY_PRINTK_DBGP)	+= early/
>> +obj-$(CONFIG_EARLY_PRINTK_USB)	+= early/
>>  
>>  obj-$(CONFIG_USB_ATM)		+= atm/
>>  obj-$(CONFIG_USB_SPEEDTOUCH)	+= atm/
>> diff --git a/drivers/usb/early/Makefile b/drivers/usb/early/Makefile
>> index 24bbe51..fcde228 100644
>> --- a/drivers/usb/early/Makefile
>> +++ b/drivers/usb/early/Makefile
>> @@ -3,3 +3,4 @@
>>  #
>>  
>>  obj-$(CONFIG_EARLY_PRINTK_DBGP) += ehci-dbgp.o
>> +obj-$(CONFIG_EARLY_PRINTK_USB_XDBC) += xhci-dbc.o
>> diff --git a/drivers/usb/early/xhci-dbc.c b/drivers/usb/early/xhci-dbc.c
>> new file mode 100644
>> index 0000000..72480c4
>> --- /dev/null
>> +++ b/drivers/usb/early/xhci-dbc.c
>> @@ -0,0 +1,1027 @@
>> +/**
>> + * xhci-dbc.c - xHCI debug capability early driver
>> + *
>> + * Copyright (C) 2016 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.
>> + */
>> +
>> +#define pr_fmt(fmt)	KBUILD_MODNAME ":%s: " fmt, __func__
>> +
>> +#include <linux/console.h>
>> +#include <linux/pci_regs.h>
>> +#include <linux/pci_ids.h>
>> +#include <linux/bootmem.h>
>> +#include <linux/io.h>
>> +#include <asm/pci-direct.h>
>> +#include <asm/fixmap.h>
>> +#include <linux/bcd.h>
>> +#include <linux/export.h>
>> +#include <linux/version.h>
>> +#include <linux/module.h>
>> +#include <linux/delay.h>
>> +
>> +#include "../host/xhci.h"
>> +#include "xhci-dbc.h"
>> +
>> +static struct xdbc_state xdbc;
>> +static int early_console_keep;
>> +
>> +#ifdef XDBC_TRACE
>> +#define	xdbc_trace	trace_printk
>> +#else
>> +static inline void xdbc_trace(const char *fmt, ...) { }
>> +#endif /* XDBC_TRACE */
>> +
>> +static int xdbc_bulk_transfer(void *data, int size, bool read);
>> +
>> +static void __iomem * __init xdbc_map_pci_mmio(u32 bus, u32 dev, u32 func)
>> +{
>> +	u32 val, sz;
>> +	u64 val64, sz64, mask64;
>> +	u8 byte;
>> +	void __iomem *base;
>> +
>> +	val = read_pci_config(bus, dev, func, PCI_BASE_ADDRESS_0);
>> +	write_pci_config(bus, dev, func, PCI_BASE_ADDRESS_0, ~0);
>> +	sz = read_pci_config(bus, dev, func, PCI_BASE_ADDRESS_0);
>> +	write_pci_config(bus, dev, func, PCI_BASE_ADDRESS_0, val);
>> +	if (val == 0xffffffff || sz == 0xffffffff) {
>> +		pr_notice("invalid mmio bar\n");
>> +		return NULL;
>> +	}
>> +
>> +	val64 = val & PCI_BASE_ADDRESS_MEM_MASK;
>> +	sz64 = sz & PCI_BASE_ADDRESS_MEM_MASK;
>> +	mask64 = (u32)PCI_BASE_ADDRESS_MEM_MASK;
> Is this cast necessary?
>
>> +
>> +	if ((val & PCI_BASE_ADDRESS_MEM_TYPE_MASK) == PCI_BASE_ADDRESS_MEM_TYPE_64) {
>> +		val = read_pci_config(bus, dev, func, PCI_BASE_ADDRESS_0 + 4);
>> +		write_pci_config(bus, dev, func, PCI_BASE_ADDRESS_0 + 4, ~0);
>> +		sz = read_pci_config(bus, dev, func, PCI_BASE_ADDRESS_0 + 4);
>> +		write_pci_config(bus, dev, func, PCI_BASE_ADDRESS_0 + 4, val);
>> +
>> +		val64 |= (u64)val << 32;
>> +		sz64 |= (u64)sz << 32;
> Could these type casts be avoided by declaring 'val' and 'sz' as u64 to begin 
> with?
>
>> +		mask64 |= (u64)~0 << 32;
> Type cast could be avoided by using 0L.
>
>> +static u32 __init xdbc_find_dbgp(int xdbc_num, u32 *b, u32 *d, u32 *f)
>> +{
>> +	u32 bus, dev, func, class;
>> +
>> +	for (bus = 0; bus < XDBC_PCI_MAX_BUSES; bus++) {
>> +		for (dev = 0; dev < XDBC_PCI_MAX_DEVICES; dev++) {
>> +			for (func = 0; func < XDBC_PCI_MAX_FUNCTION; func++) {
>> +				class = read_pci_config(bus, dev, func, PCI_CLASS_REVISION);
>> +				if ((class >> 8) != PCI_CLASS_SERIAL_USB_XHCI)
>> +					continue;
>> +
>> +				if (xdbc_num-- != 0)
>> +					continue;
>> +
>> +				*b = bus;
>> +				*d = dev;
>> +				*f = func;
>> +
>> +				return 0;
>> +			}
>> +		}
>> +	}
> Nit: to me it would look more balanced visually if the body of the innermost loop 
> started with an empty newline.
>
>> +	ext_cap_offset = xhci_find_next_ext_cap(xdbc.xhci_base,
>> +						0, XHCI_EXT_CAPS_LEGACY);
> Please don't break this line. You can shorten the variable name if you want to 
> save line length.
>
>> +	/* populate the contexts */
>> +	context = (struct xdbc_context *)xdbc.dbcc_base;
>> +	context->info.string0 = cpu_to_le64(xdbc.string_dma);
>> +	context->info.manufacturer = cpu_to_le64(xdbc.string_dma + XDBC_MAX_STRING_LENGTH);
>> +	context->info.product = cpu_to_le64(xdbc.string_dma + XDBC_MAX_STRING_LENGTH * 2);
>> +	context->info.serial = cpu_to_le64(xdbc.string_dma + XDBC_MAX_STRING_LENGTH * 3);
>> +	context->info.length = cpu_to_le32(string_length);
> Wouldn't this (and some of the other initializations) look nicer this way:
>
> 	/* Populate the contexts: */
> 	context = (struct xdbc_context *)xdbc.dbcc_base;
>
> 	context->info.string0		= cpu_to_le64(xdbc.string_dma);
> 	context->info.manufacturer	= cpu_to_le64(xdbc.string_dma + XDBC_MAX_STRING_LENGTH);
> 	context->info.product		= cpu_to_le64(xdbc.string_dma + XDBC_MAX_STRING_LENGTH * 2);
> 	context->info.serial		= cpu_to_le64(xdbc.string_dma + XDBC_MAX_STRING_LENGTH * 3);
> 	context->info.length		= cpu_to_le32(string_length);
>
> ?
>
>> +	/* Check completion of the previous request. */
> Could you please standardize the capitalization and punctuation of all the 
> comments in the patches? Some are lower case, some are upper case, some have full 
> stops, some don't.
>
> One good style would be to use this form when a comment refers to what the next 
> statement does:
>
> 	/* Check completion of the previous request: */
>
>> +/**
>> + * struct xdbc_regs - xHCI Debug Capability Register interface.
>> + */
>> +struct xdbc_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;
>> +#define DEBUG_MAX_BURST(p)	(((p) >> 16) & 0xff)
>> +#define CTRL_DCR		BIT(0)		/* DbC Run */
>> +#define CTRL_PED		BIT(1)		/* Port Enable/Disable */
>> +#define CTRL_HOT		BIT(2)		/* Halt Out TR */
>> +#define CTRL_HIT		BIT(3)		/* Halt In TR */
>> +#define CTRL_DRC		BIT(4)		/* DbC run change */
>> +#define CTRL_DCE		BIT(31)		/* DbC enable */
>> +	__le32	status;
>> +#define DCST_DPN(p)		(((p) >> 24) & 0xff)
>> +	__le32	portsc;		/* Port status and control */
>> +#define PORTSC_CCS		BIT(0)
>> +#define PORTSC_CSC		BIT(17)
>> +#define PORTSC_PRC		BIT(21)
>> +#define PORTSC_PLC		BIT(22)
>> +#define PORTSC_CEC		BIT(23)
>> +	__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 */
>> +};
> So why are defines intermixed with the fields? If the defines relate to the field 
> then their naming sure does not express that ...
>
> I was thinking of something like:
>
> /**
>  * struct xdbc_regs - xHCI Debug Capability Register interface.
>  */
> struct xdbc_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 */
> };
>
> #define DEBUG_MAX_BURST(p)	(((p) >> 16) & 0xff)
>
> #define	CTRL_DCR		BIT(0)		/* DbC Run */
> #define	CTRL_PED		BIT(1)		/* Port Enable/Disable */
> #define	CTRL_HOT		BIT(2)		/* Halt Out TR */
> #define	CTRL_HIT		BIT(3)		/* Halt In TR */
> #define	CTRL_DRC		BIT(4)		/* DbC run change */
> #define CTRL_DCE		BIT(31)		/* DbC enable */
>
> #define DCST_DPN(p)		(((p) >> 24) & 0xff)
>
> #define PORTSC_CCS		BIT(0)
> #define PORTSC_CSC		BIT(17)
> #define PORTSC_PRC		BIT(21)
> #define PORTSC_PLC		BIT(22)
> #define PORTSC_CEC		BIT(23)
>
> Also, the abbreviations suck big time. What's wrong with:
>
> #define	CTRL_DBC_RUN		BIT(0)
> #define	CTRL_PORT_ENABLE	BIT(1)
> #define	CTRL_HALT_OUT_TR	BIT(2)
> #define	CTRL_HALT_IN_TR		BIT(3)
> #define	CTRL_DBC_RUN_CHANGE	BIT(4)
> #define CTRL_DBC_ENABLE		BIT(31)
>
> ?
>
> Also note how the comments became superfluous once descriptive names are chosen...
>
> Please review the rest of the defines and series for similar problems and fix 
> them, there are more of the same type.

Thank you for the comments.

I will correct all the code style issues you pointed out here. And, I will
review all patches and fix the similar code styles.

Best regards,
Lu Baolu

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

end of thread, other threads:[~2017-01-24  4:49 UTC | newest]

Thread overview: 7+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2017-01-22  6:19 [PATCH v6 0/4] usb: early: add support for early printk through USB3 debug port Lu Baolu
2017-01-22  6:19 ` [PATCH v6 1/4] usb: dbc: early driver for xhci debug capability Lu Baolu
2017-01-22  9:31   ` Ingo Molnar
2017-01-24  4:49     ` Lu Baolu
2017-01-22  6:19 ` [PATCH v6 2/4] x86: add support for earlyprintk via USB3 debug port Lu Baolu
2017-01-22  6:19 ` [PATCH v6 3/4] usb: serial: add dbc debug device support to usb_debug Lu Baolu
2017-01-22  6:19 ` [PATCH v6 4/4] usb: doc: add 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.