usb: early: add support for early printk through USB3 debug port

xHCI debug capability (DbC) is an optional functionality provided by an xHCI host controller. Software learns this capability by walking through the extended capability list in mmio of the host.

This patch introduces the code to probe and initialize the debug capability hardware during early boot. With hardware initialization done, the debug target (system under debug which has DbC enabled) will present a debug device through the debug 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.

Signed-off-by Lu Baolu <[email protected]>

---
  MAINTAINERS                  |   7 +
  arch/x86/Kconfig.debug       |  12 +
  drivers/usb/early/Makefile   |   1 +
  drivers/usb/early/xhci-dbc.c | 774 +++++++++++++++++++++++++++++++++++++++++++
  include/linux/usb/xhci-dbc.h | 187 +++++++++++
  5 files changed, 981 insertions(+)

create mode 100644 drivers/usb/early/xhci-dbc.c
create mode 100644 include/linux/usb/xhci-dbc.h

MAINTAINERS +7 -0

index 30aca4a..e6d7076 100644
@@ -11505,6 +11505,13 @@ S:	Supported
  F:	drivers/usb/host/xhci*
  F:	drivers/usb/host/pci-quirks*
 
+USB XHCI DEBUG PORT
+M:	Lu Baolu <[email protected]>
+L:	[email protected]
+S:	Supported
+F:	drivers/usb/early/xhci-dbc.c
+F:	include/linux/usb/xhci-dbc.h
+
  USB ZD1201 DRIVER
  L:	[email protected]
  W:	http://linux-lc100020.sourceforge.net

arch/x86/Kconfig.debug +12 -0

index 9b18ed9..ba60cb1 100644
@@ -48,6 +48,18 @@ 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_XDBC
+	bool "Early printk via xHCI debug port"
+	depends on EARLY_PRINTK && PCI
+	---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
 

drivers/usb/early/Makefile +1 -0

index 24bbe51..2db5906 100644
@@ -3,3 +3,4 @@
  #
 
  obj-$(CONFIG_EARLY_PRINTK_DBGP) += ehci-dbgp.o
+obj-$(CONFIG_EARLY_PRINTK_XDBC) += xhci-dbc.o

drivers/usb/early/xhci-dbc.c new file +33 -0

new file mode 100644
index 0000000..254a0a8
@@ -0,0 +1,774 @@
+/**
+ * xhci-dbc.c - xHCI debug capability driver
+ *
+ * Copyright (C) 2015 Intel Corporation
+ *
+ * Author: Lu Baolu <[email protected]>
+ * Some code shared with EHCI debug port and xHCI driver.
+ *
+ * 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/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/usb/xhci-dbc.h>
+
+#include "../host/xhci.h"
+
+#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 */
+
+static struct xdbc_state xdbc_stat;
+static struct xdbc_state *xdbcp = &xdbc_stat;
+

drivers/usb/early/xhci-dbc.c new file +8 -0

+
+#else
+static inline void xdbc_trace(const char *fmt, ...) { }
+static inline void xdbc_dbg_dump_regs(char *str) { }
+static inline void xdbc_dbg_dump_data(char *str) { }
+#endif	/* DBC_DEBUG */
+
+/*

drivers/usb/early/xhci-dbc.c new file +109 -0

+
+static void __iomem *xdbc_map_pci_mmio(u32 bus,
+		u32 dev, u32 func, u8 bar, size_t *length)
+{
+	u32 val, sz;
+	u64 val64, sz64, mask64;
+	u8 byte;
+	unsigned long idx, max_idx;
+	void __iomem *base;
+
+	val = read_pci_config(bus, dev, func, bar);
+	write_pci_config(bus, dev, func, bar, ~0);
+	sz = read_pci_config(bus, dev, func, bar);
+	write_pci_config(bus, dev, func, bar, val);
+	if (val == 0xffffffff || sz == 0xffffffff) {
+		xdbc_trace("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, bar + 4);
+		write_pci_config(bus, dev, func, bar + 4, ~0);
+		sz = read_pci_config(bus, dev, func, bar + 4);
+		write_pci_config(bus, dev, func, bar + 4, val);
+
+		val64 |= ((u64)val << 32);
+		sz64 |= ((u64)sz << 32);
+		mask64 |= ((u64)~0 << 32);
+	}
+
+	sz64 &= mask64;
+
+	if (sizeof(dma_addr_t) < 8 || !sz64) {
+		xdbc_trace("can't handle 64bit BAR\n");
+		return NULL;
+	}
+
+	sz64 = 1ULL << __ffs64(sz64);
+
+	if (sz64 > (FIX_XDBC_END - FIX_XDBC_BASE + 1) * PAGE_SIZE) {
+		xdbc_trace("mmio size beyond 64k not supported\n");
+		return NULL;
+	}
+
+	xdbc_trace("bar: base 0x%llx size 0x%llx offset %03x\n",
+			val64, sz64, bar);
+
+	/* 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_trace("mmio for xhci enabled\n");
+	}
+
+	/* 64k mmio will be fix-mapped */
+	max_idx = FIX_XDBC_END - FIX_XDBC_BASE;
+	for (idx = 0; idx <= max_idx; idx++)
+		set_fixmap_nocache(FIX_XDBC_BASE + idx,
+			(val64 & PAGE_MASK) + (max_idx - idx) * PAGE_SIZE);
+	base = (void __iomem *)__fix_to_virt(FIX_XDBC_END);
+	base += val64 & ~PAGE_MASK;
+
+	/* save in the state block */
+	xdbcp->bus = bus;
+	xdbcp->dev = dev;
+	xdbcp->func = func;
+	xdbcp->bar = bar;
+	xdbcp->xhci_base = base;
+	xdbcp->xhci_length = sz64;
+
+	if (length)
+		*length = sz64;
+
+	return base;
+}
+
+/*
+ * FIXME: The bootmem allocator isn't ready at the time when DbC gets
+ *        initialized. Below implementation reserves DMA memory blocks
+ *        in the kernel static data segment.
+ */
+static void *xdbc_get_page(dma_addr_t *dma_addr,
+		enum xdbc_page_type type)
+{
+	void *virt;
+	static char event_page[PAGE_SIZE] __aligned(PAGE_SIZE);
+	static char in_ring_page[PAGE_SIZE] __aligned(PAGE_SIZE);
+	static char out_ring_page[PAGE_SIZE] __aligned(PAGE_SIZE);
+	static char table_page[PAGE_SIZE] __aligned(PAGE_SIZE);
+
+	switch (type) {
+	case XDBC_PAGE_EVENT:
+		virt = (void *)event_page;
+		break;
+	case XDBC_PAGE_TXIN:
+		virt = (void *)in_ring_page;
+		break;
+	case XDBC_PAGE_TXOUT:
+		virt = (void *)out_ring_page;
+		break;
+	case XDBC_PAGE_TABLE:
+		virt = (void *)table_page;
+		break;

drivers/usb/early/xhci-dbc.c new file +61 -0

+}
+
+typedef void (*xdbc_walk_excap_cb)(int cap_offset, void *data);
+
+/*
+ * xdbc_walk_excap:
+ *
+ * xHCI extended capability list walker.
+ *
+ * @bus - xHC PCI bus#
+ * @dev - xHC PCI dev#
+ * @func - xHC PCI function#
+ * @cap - capability ID
+ * @oneshot - return immediately once hit match
+ * @cb - call back
+ * @data - callback private data
+ *
+ * Return the last cap offset, otherwize 0.
+ */
+static u32 xdbc_walk_excap(u32 bus, u32 dev, u32 func, int cap,
+		bool oneshot, xdbc_walk_excap_cb cb, void *data)
+{
+	void __iomem *base;
+	int offset = 0;
+	size_t len = 0;
+
+	if (xdbcp->xhci_base && xdbcp->xhci_length) {
+		if (xdbcp->bus != bus ||
+				xdbcp->dev != dev ||
+				xdbcp->func != func) {
+			xdbc_trace("only one DbC can be used\n");
+			return 0;
+		}
+
+		len = xdbcp->xhci_length;
+		base = xdbcp->xhci_base;
+	} else {
+		base = xdbc_map_pci_mmio(bus, dev, func,
+				PCI_BASE_ADDRESS_0, &len);
+		if (!base)
+			return 0;
+	}
+
+	do {
+		offset = xhci_find_next_ext_cap(base, offset, cap);
+		if (!offset)
+			break;
+
+		if (cb)
+			cb(offset, data);
+		if (oneshot)
+			break;
+	} while (1);
+
+	return offset;
+}
+
+static u32 __init xdbc_find_dbgp(int xdbc_num,
+		u32 *rbus, u32 *rdev, u32 *rfunc)
+{
+	u32 bus, dev, func, class;

drivers/usb/early/xhci-dbc.c new file +34 -0

+				if (xdbc_num-- != 0)
+					continue;
+
+				cap = xdbc_walk_excap(bus, dev, func,
+						XHCI_EXT_CAPS_DEBUG,
+						true, NULL, NULL);
+				*rbus = bus;
+				*rdev = dev;
+				*rfunc = func;
+				return cap;
+			}
+		}
+	}
+
+	return 0;
+}
+
+static int handshake(void __iomem *ptr, u32 mask, u32 done,
+		int wait_usec, int delay_usec)
+{
+	u32	result;
+
+	do {
+		result = readl(ptr);
+		result &= mask;
+		if (result == done)
+			return 0;
+		xdbc_udelay(delay_usec);
+		wait_usec -= delay_usec;
+	} while (wait_usec > 0);
+
+	return -ETIMEDOUT;
+}
+

drivers/usb/early/xhci-dbc.c new file +23 -0

+/*
+ * xdbc_start: start DbC
+ *
+ * Set DbC enable bit and wait until DbC run bit being set or timed out.
+ */
+static int xdbc_start(void)
+{
+	u32 ctrl, status;
+
+	ctrl = readl(&xdbcp->xdbc_reg->control);
+	writel(ctrl | CTRL_DCE | CTRL_LSE, &xdbcp->xdbc_reg->control);
+
+	if (handshake(&xdbcp->xdbc_reg->control, CTRL_DCE,
+			CTRL_DCE, 100000, 100) < 0) {
+		xdbc_trace("falied to initialize hardware\n");
+		return -ENODEV;
+	}
+
+	/* wait for port connection */
+	if (handshake(&xdbcp->xdbc_reg->portsc, PORTSC_CCS,
+			PORTSC_CCS, 5000000, 100) < 0) {
+		xdbc_trace("waiting for connection timed out\n");
+		return -ETIMEDOUT;

drivers/usb/early/xhci-dbc.c new file +101 -0

+	/* port should have a valid port# */
+	status = readl(&xdbcp->xdbc_reg->status);
+	if (!DCST_DPN(status)) {
+		xdbc_trace("invalid root hub port number\n");
+		return -ENODEV;
+	}
+
+	xdbc_trace("root hub port number %d\n", DCST_DPN(status));
+
+	xdbc_trace("DbC is running now, control 0x%08x\n",
+			readl(&xdbcp->xdbc_reg->control));
+
+	return 0;
+}
+
+static int xdbc_setup(void)
+{
+	int ret;
+
+	writel(0, &xdbcp->xdbc_reg->control);
+	if (handshake(&xdbcp->xdbc_reg->control, CTRL_DCE,
+			0, 100000, 100) < 0) {
+		xdbc_trace("falied to initialize hardware\n");
+		return -ETIMEDOUT;
+	}
+
+	/* allocate and initialize all memory data structures */
+	ret = xdbc_mem_init();
+	if (ret < 0) {
+		xdbc_trace("failed to initialize memory\n");
+		return ret;
+	}
+
+	/*
+	 * Memory barrier to ensure hardware sees the bits
+	 * setting above.
+	 */
+	mmiowb();
+
+	/* dump registers and data structures */
+	xdbc_dbg_dump_regs("hardware setup completed");
+	xdbc_dbg_dump_data("hardware setup completed");
+
+	ret = xdbc_start();
+	if (ret < 0) {
+		xdbc_trace("failed to start DbC, cable connected?\n");
+		return ret;
+	}
+
+	return 0;
+}
+
+int __init early_xdbc_init(char *s)
+{
+	u32 bus = 0, dev = 0, func = 0;
+	unsigned long dbgp_num = 0;
+	u32 offset;
+	int ret;
+
+	if (!early_pci_allowed())
+		return -EPERM;
+
+	/* FIXME: early printk "keep" option will be supported later */
+	if (strstr(s, "keep"))
+		return -EPERM;
+
+	if (xdbcp->xdbc_reg)
+		return 0;
+
+	if (*s && kstrtoul(s, 0, &dbgp_num))
+		dbgp_num = 0;
+
+	xdbc_trace("dbgp_num: %lu\n", dbgp_num);
+
+	offset = xdbc_find_dbgp(dbgp_num, &bus, &dev, &func);
+	if (!offset)
+		return -ENODEV;
+
+	xdbc_trace("Found xHCI debug capability on %02x:%02x.%1x\n",
+			bus, dev, func);
+
+	if (!xdbcp->xhci_base)
+		return -EINVAL;
+
+	xdbcp->xdbc_reg = (struct xdbc_regs __iomem *)
+			(xdbcp->xhci_base + offset);
+	xdbc_dbg_dump_regs("debug capability located");
+
+	/* hand over the owner of host from BIOS */
+	xdbc_bios_handoff();
+
+	ret = xdbc_setup();
+	if (ret < 0) {
+		pr_notice("failed to setup xHCI DbC connection\n");
+		xdbcp->xhci_base = NULL;
+		xdbcp->xdbc_reg = NULL;
+		return ret;
+	}
+
+	return 0;
+}

include/linux/usb/xhci-dbc.h new file +3 -0

new file mode 100644
index 0000000..153fb87
@@ -0,0 +1,187 @@
+/*
+ * xHCI debug capability driver
+ *

Hi

Most of my concerns are due to the early printk restrictions. As you say in the documentation:
"Keep in mind that things such as interrupt, system memory, DMA memory, PCI configure space access, etc., are all different from a normal device driver."

I don't know the details of these subsystems, or if what is done here is done the "right" way.

DMA capable memory is allocated with static char xpage[PAGE_SIZE] __aligned, and DMA address is found using __pa()

xdbc_map_pci_mmio() does some pci magic and fixmapping that I don't know the details of.

writing to ioport 0x80 outb(1, 0x80) is used as 1us delay, sometimes up to 5 million times to get a 5 second timeout

Adding Bjorn Helgaas and Dan Williams in case they have an opinoin about the DMA and PCI cases.

more comments inline

Quoted 113 lines · Lu Baolu

xHCI debug capability (DbC) is an optional functionality provided by an xHCI host controller. Software learns this capability by walking through the extended capability list in mmio of the host.

This patch introduces the code to probe and initialize the debug capability hardware during early boot. With hardware initialization done, the debug target (system under debug which has DbC enabled) will present a debug device through the debug 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.

Signed-off-by Lu Baolu <[email protected]>

---
  MAINTAINERS                  |   7 +
  arch/x86/Kconfig.debug       |  12 +
  drivers/usb/early/Makefile   |   1 +
  drivers/usb/early/xhci-dbc.c | 774 +++++++++++++++++++++++++++++++++++++++++++
  include/linux/usb/xhci-dbc.h | 187 +++++++++++
  5 files changed, 981 insertions(+)

create mode 100644 drivers/usb/early/xhci-dbc.c
create mode 100644 include/linux/usb/xhci-dbc.h

MAINTAINERS +7 -0

index 30aca4a..e6d7076 100644
@@ -11505,6 +11505,13 @@ S:    Supported
  F:    drivers/usb/host/xhci*
  F:    drivers/usb/host/pci-quirks*
 
+USB XHCI DEBUG PORT
+M:    Lu Baolu <[email protected]>
+L:    [email protected]
+S:    Supported
+F:    drivers/usb/early/xhci-dbc.c
+F:    include/linux/usb/xhci-dbc.h
+
  USB ZD1201 DRIVER
  L:    [email protected]
  W:    http://linux-lc100020.sourceforge.net

arch/x86/Kconfig.debug +12 -0

index 9b18ed9..ba60cb1 100644
@@ -48,6 +48,18 @@ 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_XDBC
+    bool "Early printk via xHCI debug port"
+    depends on EARLY_PRINTK && PCI
+    ---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
 

drivers/usb/early/Makefile +1 -0

index 24bbe51..2db5906 100644
@@ -3,3 +3,4 @@
  #
 
  obj-$(CONFIG_EARLY_PRINTK_DBGP) += ehci-dbgp.o
+obj-$(CONFIG_EARLY_PRINTK_XDBC) += xhci-dbc.o

drivers/usb/early/xhci-dbc.c new file +33 -0

new file mode 100644
index 0000000..254a0a8
@@ -0,0 +1,774 @@
+/**
+ * xhci-dbc.c - xHCI debug capability driver
+ *
+ * Copyright (C) 2015 Intel Corporation
+ *
+ * Author: Lu Baolu <[email protected]>
+ * Some code shared with EHCI debug port and xHCI driver.
+ *
+ * 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/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/usb/xhci-dbc.h>
+
+#include "../host/xhci.h"
+
+#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 */
+
+static struct xdbc_state xdbc_stat;
+static struct xdbc_state *xdbcp = &xdbc_stat;
+

...

Quoted 8 lines

drivers/usb/early/xhci-dbc.c new file +8 -0

+
+#else
+static inline void xdbc_trace(const char *fmt, ...) { }
+static inline void xdbc_dbg_dump_regs(char *str) { }
+static inline void xdbc_dbg_dump_data(char *str) { }
+#endif    /* DBC_DEBUG */
+
+/*

drivers/usb/early/xhci-dbc.c line 182

Quoted 10 lines

drivers/usb/early/xhci-dbc.c new file +10 -0

+ * FIXME: kernel provided delay interfaces, like usleep, isn't ready yet
+ *        at the time DbC gets initialized. Below implementation is only
+ *        for x86 platform. Need to reconsider this when porting it onto
+ *        other architectures.
+ */
+static inline void xdbc_udelay(int us)
+{
+    while (us-- > 0)
+        outb(0x1, 0x80);
+}

This is one of the concerns, the FIXME text describes the problem but still wondering if there is any other solution. In many cases later in this file we will wait up to 5 seconds for something, ending up writing 5 million times to ioport 0x80.

Quoted 109 lines

drivers/usb/early/xhci-dbc.c new file +109 -0

+
+static void __iomem *xdbc_map_pci_mmio(u32 bus,
+        u32 dev, u32 func, u8 bar, size_t *length)
+{
+    u32 val, sz;
+    u64 val64, sz64, mask64;
+    u8 byte;
+    unsigned long idx, max_idx;
+    void __iomem *base;
+
+    val = read_pci_config(bus, dev, func, bar);
+    write_pci_config(bus, dev, func, bar, ~0);
+    sz = read_pci_config(bus, dev, func, bar);
+    write_pci_config(bus, dev, func, bar, val);
+    if (val == 0xffffffff || sz == 0xffffffff) {
+        xdbc_trace("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, bar + 4);
+        write_pci_config(bus, dev, func, bar + 4, ~0);
+        sz = read_pci_config(bus, dev, func, bar + 4);
+        write_pci_config(bus, dev, func, bar + 4, val);
+
+        val64 |= ((u64)val << 32);
+        sz64 |= ((u64)sz << 32);
+        mask64 |= ((u64)~0 << 32);
+    }
+
+    sz64 &= mask64;
+
+    if (sizeof(dma_addr_t) < 8 || !sz64) {
+        xdbc_trace("can't handle 64bit BAR\n");
+        return NULL;
+    }
+
+    sz64 = 1ULL << __ffs64(sz64);
+
+    if (sz64 > (FIX_XDBC_END - FIX_XDBC_BASE + 1) * PAGE_SIZE) {
+        xdbc_trace("mmio size beyond 64k not supported\n");
+        return NULL;
+    }
+
+    xdbc_trace("bar: base 0x%llx size 0x%llx offset %03x\n",
+            val64, sz64, bar);
+
+    /* 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_trace("mmio for xhci enabled\n");
+    }
+
+    /* 64k mmio will be fix-mapped */
+    max_idx = FIX_XDBC_END - FIX_XDBC_BASE;
+    for (idx = 0; idx <= max_idx; idx++)
+        set_fixmap_nocache(FIX_XDBC_BASE + idx,
+            (val64 & PAGE_MASK) + (max_idx - idx) * PAGE_SIZE);
+    base = (void __iomem *)__fix_to_virt(FIX_XDBC_END);
+    base += val64 & ~PAGE_MASK;
+
+    /* save in the state block */
+    xdbcp->bus = bus;
+    xdbcp->dev = dev;
+    xdbcp->func = func;
+    xdbcp->bar = bar;
+    xdbcp->xhci_base = base;
+    xdbcp->xhci_length = sz64;
+
+    if (length)
+        *length = sz64;
+
+    return base;
+}
+
+/*
+ * FIXME: The bootmem allocator isn't ready at the time when DbC gets
+ *        initialized. Below implementation reserves DMA memory blocks
+ *        in the kernel static data segment.
+ */
+static void *xdbc_get_page(dma_addr_t *dma_addr,
+        enum xdbc_page_type type)
+{
+    void *virt;
+    static char event_page[PAGE_SIZE] __aligned(PAGE_SIZE);
+    static char in_ring_page[PAGE_SIZE] __aligned(PAGE_SIZE);
+    static char out_ring_page[PAGE_SIZE] __aligned(PAGE_SIZE);
+    static char table_page[PAGE_SIZE] __aligned(PAGE_SIZE);
+
+    switch (type) {
+    case XDBC_PAGE_EVENT:
+        virt = (void *)event_page;
+        break;
+    case XDBC_PAGE_TXIN:
+        virt = (void *)in_ring_page;
+        break;
+    case XDBC_PAGE_TXOUT:
+        virt = (void *)out_ring_page;
+        break;
+    case XDBC_PAGE_TABLE:
+        virt = (void *)table_page;
+        break;

drivers/usb/early/xhci-dbc.c line 301

Quoted 10 lines

drivers/usb/early/xhci-dbc.c new file +10 -0

+    default:
+        return NULL;
+    }
+
+    memset(virt, 0, PAGE_SIZE);
+
+    if (dma_addr)
+        *dma_addr = (dma_addr_t)__pa(virt);
+
+    return virt;

another concern, I can't say if this is ok:

void *virt
static char event_page[PAGE_SIZE] __aligened(PAGE_SIZE) virt = (void *)event_page
*dma_addr = (dma_addr_t)__pa(virt)

Quoted 65 lines

drivers/usb/early/xhci-dbc.c new file +65 -0

+}
+
+typedef void (*xdbc_walk_excap_cb)(int cap_offset, void *data);
+
+/*
+ * xdbc_walk_excap:
+ *
+ * xHCI extended capability list walker.
+ *
+ * @bus - xHC PCI bus#
+ * @dev - xHC PCI dev#
+ * @func - xHC PCI function#
+ * @cap - capability ID
+ * @oneshot - return immediately once hit match
+ * @cb - call back
+ * @data - callback private data
+ *
+ * Return the last cap offset, otherwize 0.
+ */
+static u32 xdbc_walk_excap(u32 bus, u32 dev, u32 func, int cap,
+        bool oneshot, xdbc_walk_excap_cb cb, void *data)
+{
+    void __iomem *base;
+    int offset = 0;
+    size_t len = 0;
+
+    if (xdbcp->xhci_base && xdbcp->xhci_length) {
+        if (xdbcp->bus != bus ||
+                xdbcp->dev != dev ||
+                xdbcp->func != func) {
+            xdbc_trace("only one DbC can be used\n");
+            return 0;
+        }
+
+        len = xdbcp->xhci_length;
+        base = xdbcp->xhci_base;
+    } else {
+        base = xdbc_map_pci_mmio(bus, dev, func,
+                PCI_BASE_ADDRESS_0, &len);
+        if (!base)
+            return 0;
+    }
+
+    do {
+        offset = xhci_find_next_ext_cap(base, offset, cap);
+        if (!offset)
+            break;
+
+        if (cb)
+            cb(offset, data);
+        if (oneshot)
+            break;
+    } while (1);
+
+    return offset;
+}
+
+static u32 __init xdbc_find_dbgp(int xdbc_num,
+        u32 *rbus, u32 *rdev, u32 *rfunc)
+{
+    u32 bus, dev, func, class;
+    unsigned cap;
+
+    for (bus = 0; bus < XDBC_PCI_MAX_BUSES; bus++) {
+        for (dev = 0; dev < XDBC_PCI_MAX_DEVICES; dev++) {

drivers/usb/early/xhci-dbc.c line 372

Quoted 6 lines

drivers/usb/early/xhci-dbc.c new file +6 -0

+            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;
+

Can "for_each_pci_dev()" or similar be used here? Is it too early for that as well?

Quoted 35 lines

drivers/usb/early/xhci-dbc.c new file +34 -0

+                if (xdbc_num-- != 0)
+                    continue;
+
+                cap = xdbc_walk_excap(bus, dev, func,
+                        XHCI_EXT_CAPS_DEBUG,
+                        true, NULL, NULL);
+                *rbus = bus;
+                *rdev = dev;
+                *rfunc = func;
+                return cap;
+            }
+        }
+    }
+
+    return 0;
+}
+
+static int handshake(void __iomem *ptr, u32 mask, u32 done,
+        int wait_usec, int delay_usec)
+{
+    u32    result;
+
+    do {
+        result = readl(ptr);
+        result &= mask;
+        if (result == done)
+            return 0;
+        xdbc_udelay(delay_usec);
+        wait_usec -= delay_usec;
+    } while (wait_usec > 0);
+
+    return -ETIMEDOUT;
+}
+

...

Quoted 23 lines

drivers/usb/early/xhci-dbc.c new file +23 -0

+/*
+ * xdbc_start: start DbC
+ *
+ * Set DbC enable bit and wait until DbC run bit being set or timed out.
+ */
+static int xdbc_start(void)
+{
+    u32 ctrl, status;
+
+    ctrl = readl(&xdbcp->xdbc_reg->control);
+    writel(ctrl | CTRL_DCE | CTRL_LSE, &xdbcp->xdbc_reg->control);
+
+    if (handshake(&xdbcp->xdbc_reg->control, CTRL_DCE,
+            CTRL_DCE, 100000, 100) < 0) {
+        xdbc_trace("falied to initialize hardware\n");
+        return -ENODEV;
+    }
+
+    /* wait for port connection */
+    if (handshake(&xdbcp->xdbc_reg->portsc, PORTSC_CCS,
+            PORTSC_CCS, 5000000, 100) < 0) {
+        xdbc_trace("waiting for connection timed out\n");
+        return -ETIMEDOUT;

drivers/usb/early/xhci-dbc.c line 673

Quoted 10 lines

drivers/usb/early/xhci-dbc.c new file +10 -0

+    }
+    xdbc_trace("port connection detected\n");
+
+    /* wait for debug device to be configured */
+    if (handshake(&xdbcp->xdbc_reg->control, CTRL_DCR,
+            CTRL_DCR, 5000000, 100) < 0) {
+        xdbc_trace("waiting for device configuration timed out\n");
+        return -ETIMEDOUT;
+    }
+

If I follow this correcly the initialization goes like:

early_xdbc_init()
xdbc_setup()
write 0 to ctrl reg // clears (disables/stops) the debug capability.
xdbc_start()
writel DCE to ctrl reg // this will steal the first port from xhci host driver to debug use. wait max 5 sec for connection (PORTSC_CCS)
wait max 5 second for connected host machine to configure this "device" (CTRL_DCR)

Is this debug capability limited to work only when the debug device is rebooted and a separate debug host is connected?

If a host connection is not discoved in first 5 seconds, and configured in another 5 seconds, it gives up alltogether.

Does this work together with xhci_hcd driver? I think xhci_hcd does a HCRST when driver loads, leading to host driver taking back the first port (DCE = 0). Looks like loading xhci_hcd will kill the xdbc early printk.

xdbc should probably give back the port if connect / configure fails (set back DCE=0)

-Mathias

Leaving some of the initialization code below for reference

Quoted 110 lines

drivers/usb/early/xhci-dbc.c new file +101 -0

+    /* port should have a valid port# */
+    status = readl(&xdbcp->xdbc_reg->status);
+    if (!DCST_DPN(status)) {
+        xdbc_trace("invalid root hub port number\n");
+        return -ENODEV;
+    }
+
+    xdbc_trace("root hub port number %d\n", DCST_DPN(status));
+
+    xdbc_trace("DbC is running now, control 0x%08x\n",
+            readl(&xdbcp->xdbc_reg->control));
+
+    return 0;
+}
+
+static int xdbc_setup(void)
+{
+    int ret;
+
+    writel(0, &xdbcp->xdbc_reg->control);
+    if (handshake(&xdbcp->xdbc_reg->control, CTRL_DCE,
+            0, 100000, 100) < 0) {
+        xdbc_trace("falied to initialize hardware\n");
+        return -ETIMEDOUT;
+    }
+
+    /* allocate and initialize all memory data structures */
+    ret = xdbc_mem_init();
+    if (ret < 0) {
+        xdbc_trace("failed to initialize memory\n");
+        return ret;
+    }
+
+    /*
+     * Memory barrier to ensure hardware sees the bits
+     * setting above.
+     */
+    mmiowb();
+
+    /* dump registers and data structures */
+    xdbc_dbg_dump_regs("hardware setup completed");
+    xdbc_dbg_dump_data("hardware setup completed");
+
+    ret = xdbc_start();
+    if (ret < 0) {
+        xdbc_trace("failed to start DbC, cable connected?\n");
+        return ret;
+    }
+
+    return 0;
+}
+
+int __init early_xdbc_init(char *s)
+{
+    u32 bus = 0, dev = 0, func = 0;
+    unsigned long dbgp_num = 0;
+    u32 offset;
+    int ret;
+
+    if (!early_pci_allowed())
+        return -EPERM;
+
+    /* FIXME: early printk "keep" option will be supported later */
+    if (strstr(s, "keep"))
+        return -EPERM;
+
+    if (xdbcp->xdbc_reg)
+        return 0;
+
+    if (*s && kstrtoul(s, 0, &dbgp_num))
+        dbgp_num = 0;
+
+    xdbc_trace("dbgp_num: %lu\n", dbgp_num);
+
+    offset = xdbc_find_dbgp(dbgp_num, &bus, &dev, &func);
+    if (!offset)
+        return -ENODEV;
+
+    xdbc_trace("Found xHCI debug capability on %02x:%02x.%1x\n",
+            bus, dev, func);
+
+    if (!xdbcp->xhci_base)
+        return -EINVAL;
+
+    xdbcp->xdbc_reg = (struct xdbc_regs __iomem *)
+            (xdbcp->xhci_base + offset);
+    xdbc_dbg_dump_regs("debug capability located");
+
+    /* hand over the owner of host from BIOS */
+    xdbc_bios_handoff();
+
+    ret = xdbc_setup();
+    if (ret < 0) {
+        pr_notice("failed to setup xHCI DbC connection\n");
+        xdbcp->xhci_base = NULL;
+        xdbcp->xdbc_reg = NULL;
+        return ret;
+    }
+
+    return 0;
+}

include/linux/usb/xhci-dbc.h new file +3 -0

new file mode 100644
index 0000000..153fb87
@@ -0,0 +1,187 @@
+/*
+ * xHCI debug capability driver
+ *

"printk" is not suitable for dbc debugging especially when console is in usage. This patch adds a debug buffer in dbc driver and puts the debug messages in this local buffer. The debug buffer could be dumped whenever the console is not in use. This part of code will not be visible unless DBC_DEBUG is defined.

Signed-off-by Lu Baolu <[email protected]>

---
  drivers/usb/early/xhci-dbc.c | 62 ++++++++++++++++++++++++++++++++++++++++++--
  1 file changed, 60 insertions(+), 2 deletions(-)

drivers/usb/early/xhci-dbc.c +0 -0

index 41ce116..6855048 100644

drivers/usb/early/xhci-dbc.c +16 -0

+static char xdbc_debug_buf[XDBC_DEBUG_BUF_SIZE];
+static void xdbc_trace(const char *fmt, ...)
+{
+	int i, size;
+	va_list args;
+	static int pos;
+	char temp_buf[MSG_MAX_LINE];
+
+	if (pos >= XDBC_DEBUG_BUF_SIZE - 1)
+		return;
+
+	memset(temp_buf, 0, MSG_MAX_LINE);
+	va_start(args, fmt);
+	vsnprintf(temp_buf, MSG_MAX_LINE - 1, fmt, args);
+	va_end(args);
+

drivers/usb/early/xhci-dbc.c +9 -0

+}
+
+static void xdbc_dump_debug_buffer(void)
+{
+	int index = 0;
+	int count = 0;
+	char dump_buf[MSG_MAX_LINE];
+
+	xdbc_trace("The end of DbC trace buffer\n");

Quoted 18 lines · Lu Baolu

"printk" is not suitable for dbc debugging especially when console is in usage. This patch adds a debug buffer in dbc driver and puts the debug messages in this local buffer. The debug buffer could be dumped whenever the console is not in use. This part of code will not be visible unless DBC_DEBUG is defined.

Signed-off-by Lu Baolu <[email protected]>

---
  drivers/usb/early/xhci-dbc.c | 62 ++++++++++++++++++++++++++++++++++++++++++--
  1 file changed, 60 insertions(+), 2 deletions(-)

drivers/usb/early/xhci-dbc.c +0 -0

index 41ce116..6855048 100644
@@ -32,8 +32,64 @@ static struct xdbc_state xdbc_stat;
  static struct xdbc_state *xdbcp = &xdbc_stat;
 

drivers/usb/early/xhci-dbc.c static struct xdbc_state xdbc_stat; · line 35

Quoted 4 lines

drivers/usb/early/xhci-dbc.c +1 -2

  #ifdef DBC_DEBUG
-/* place holder */
-#define    xdbc_trace    printk
+#define    XDBC_DEBUG_BUF_SIZE    (PAGE_SIZE * 32)

Does it really need to be this huge? minimum 4096 * 32 ~ 128k The kernel ring buffer is about the same size (16k - 256k)

drivers/usb/early/xhci-dbc.c static struct xdbc_state xdbc_stat; · line 36

Quoted 2 lines

drivers/usb/early/xhci-dbc.c +1 -0

+#define    MSG_MAX_LINE        128

with 128 characters per line this would fit ~1000 lines

Quoted 16 lines

drivers/usb/early/xhci-dbc.c +16 -0

+static char xdbc_debug_buf[XDBC_DEBUG_BUF_SIZE];
+static void xdbc_trace(const char *fmt, ...)
+{
+    int i, size;
+    va_list args;
+    static int pos;
+    char temp_buf[MSG_MAX_LINE];
+
+    if (pos >= XDBC_DEBUG_BUF_SIZE - 1)
+        return;
+
+    memset(temp_buf, 0, MSG_MAX_LINE);
+    va_start(args, fmt);
+    vsnprintf(temp_buf, MSG_MAX_LINE - 1, fmt, args);
+    va_end(args);
+

drivers/usb/early/xhci-dbc.c static struct xdbc_state xdbc_stat; · line 62

Quoted 10 lines

drivers/usb/early/xhci-dbc.c +10 -0

+    i = 0;
+    size = strlen(temp_buf);
+    while (i < size) {
+        xdbc_debug_buf[pos] = temp_buf[i];
+        pos++;
+        i++;
+
+        if (pos >= XDBC_DEBUG_BUF_SIZE - 1)
+            break;
+    }

how about something like:

size = min(XDBC_DEBUG_BUF_SIZE - pos, size)
memcpy(xdbc_debug_buf + pos, temp_buf, size)
pos += size;

(might need some "-1" and off by one checking..)

Quoted 14 lines

drivers/usb/early/xhci-dbc.c +13 -0

+}
+
+static void xdbc_dump_debug_buffer(void)
+{
+    int index = 0;
+    int count = 0;
+    char dump_buf[MSG_MAX_LINE];
+
+    xdbc_trace("The end of DbC trace buffer\n");
+    pr_notice("DBC debug buffer:\n");
+    memset(dump_buf, 0, MSG_MAX_LINE);
+
+    while (index < XDBC_DEBUG_BUF_SIZE) {

drivers/usb/early/xhci-dbc.c static struct xdbc_state xdbc_stat; · line 81

Quoted 6 lines

drivers/usb/early/xhci-dbc.c +6 -0

+        if (!xdbc_debug_buf[index])
+            break;
+
+        if (xdbc_debug_buf[index] == '\n' ||
+                count >= MSG_MAX_LINE - 1) {
+            pr_notice("DBC: @%08x %s\n", index, dump_buf);

Is showing the he index (position in debug buffer) useful here?

drivers/usb/early/xhci-dbc.c static struct xdbc_state xdbc_stat; · line 90

Quoted 11 lines

drivers/usb/early/xhci-dbc.c +9 -0

+            memset(dump_buf, 0, MSG_MAX_LINE);
+            count = 0;
+        } else {
+            dump_buf[count] = xdbc_debug_buf[index];
+            count++;
+        }
+
+        index++;
+    }

So we have one huge buffer that xdbc keeps on filling as the initialization progresses. It is never emptied, or overwritten (circular).
When dumped it always dumps the whole thing, copying one character
at a time.

As this is only used for debugging during xdbc development/debugging, and never enabled even if xdbc early printk is used, I don't think optimization really matters.

Perhaps take a look if we really need PAGE_SIZE * 32 bytes, is xdbc driver even nearly writing that much debug data.

-Mathias

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

Signed-off-by Lu Baolu <[email protected]>

---
  drivers/usb/early/xhci-dbc.c | 373 +++++++++++++++++++++++++++++++++++++++++++
  include/linux/usb/xhci-dbc.h |  30 ++++
  2 files changed, 403 insertions(+)

drivers/usb/early/xhci-dbc.c +12 -0

+
+	xdbc_handle_events();
+
+	test_and_clear_bit(XDBC_ATOMIC_EVENT, &xdbcp->atomic_flags);
+
+	return 0;
 +}
+
+#define	BULK_IN_COMPLETED(p)	((xdbcp->in_pending == (p)) && \
+				 xdbcp->in_complete)
+#define	BULK_OUT_COMPLETED(p)	((xdbcp->out_pending == (p)) && \
+				 xdbcp->out_complete)
+

drivers/usb/early/xhci-dbc.c +8 -0

+
+	ret = xdbc_bulk_transfer(data, size, loops, true);
+
+	test_and_clear_bit(XDBC_ATOMIC_BULKIN, &xdbcp->atomic_flags);
+
+	return ret;
+}
+

Quoted 10 lines · Lu Baolu

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

Signed-off-by Lu Baolu <[email protected]>

---
  drivers/usb/early/xhci-dbc.c | 373 +++++++++++++++++++++++++++++++++++++++++++
  include/linux/usb/xhci-dbc.h |  30 ++++
  2 files changed, 403 insertions(+)

...

drivers/usb/early/xhci-dbc.c int __init early_xdbc_init(char *s) · line 1090

Quoted 13 lines

drivers/usb/early/xhci-dbc.c +13 -0

+
+/*
+ * Check and dispatch events in event ring. It also checks status
+ * of hardware. This function will be called from multiple threads.
+ * An atomic lock is applied to protect the access of event ring.
+ */
+static int xdbc_check_event(void)
+{
+    /* event ring is under checking by other thread? */
+    if (!test_bit(XDBC_ATOMIC_EVENT, &xdbcp->atomic_flags) &&
+            !test_and_set_bit(XDBC_ATOMIC_EVENT,
+            &xdbcp->atomic_flags))
+        return 0;

homemade trylock, can't the real ones be used?

Quoted 13 lines

drivers/usb/early/xhci-dbc.c +12 -0

+
+    xdbc_handle_events();
+
+    test_and_clear_bit(XDBC_ATOMIC_EVENT, &xdbcp->atomic_flags);
+
+    return 0;
 +}
+
+#define    BULK_IN_COMPLETED(p)    ((xdbcp->in_pending == (p)) && \
+                 xdbcp->in_complete)
+#define    BULK_OUT_COMPLETED(p)    ((xdbcp->out_pending == (p)) && \
+                 xdbcp->out_complete)
+

...

drivers/usb/early/xhci-dbc.c int __init early_xdbc_init(char *s) · line 1232

Quoted 12 lines

drivers/usb/early/xhci-dbc.c +12 -0

+}
+
+int xdbc_bulk_read(void *data, int size, int loops)
+{
+    int ret;
+
+    do {
+        if (!test_bit(XDBC_ATOMIC_BULKIN, &xdbcp->atomic_flags) &&
+                !test_and_set_bit(XDBC_ATOMIC_BULKIN,
+                &xdbcp->atomic_flags))
+            break;
+    } while (1);

homemeade spin_lock, can't the real one be used?

If the xdbc_bulk_write() can be accessed from interrupt context (handler, soft, timer) it may deadlock

Quoted 8 lines

drivers/usb/early/xhci-dbc.c +8 -0

+
+    ret = xdbc_bulk_transfer(data, size, loops, true);
+
+    test_and_clear_bit(XDBC_ATOMIC_BULKIN, &xdbcp->atomic_flags);
+
+    return ret;
+}
+

drivers/usb/early/xhci-dbc.c int __init early_xdbc_init(char *s) · line 1250

Quoted 10 lines

drivers/usb/early/xhci-dbc.c +10 -0

+int xdbc_bulk_write(const char *bytes, int size)
+{
+    int ret;
+
+    do {
+        if (!test_bit(XDBC_ATOMIC_BULKOUT, &xdbcp->atomic_flags) &&
+                !test_and_set_bit(XDBC_ATOMIC_BULKOUT,
+                &xdbcp->atomic_flags))
+            break;
+    } while (1);

Another homemeade spin_lock, can't the real one be used?

same issue here, deadlock if accessible from interrupt context.

Would it make sense to have only one spinlock, and start one separate thread for reading the event ring. The thread would, lock, handle pending events, unlock, then call shedule, in a loop. ehci early debug code has some variant of this.

So the lock would be taken while events are being handled.

The same lock would be used for bulk_read and bulk_write. Yes this would prevent read and write at the same time, and the read and writes need to be modified to not block until the reansfer is finished, just to write the TRBs on the ring, update ring pointers, and ring the doorbell.

Or is all this impossibe due to the earlyness of the code?

-Mathias

In case of endpoint stall, software is able to detect the situation by reading DCCTRL.HIT or DCCTRL.HOT bits. DbC follows the normal USB framework to handle endpoint stall. When software detects endpoint stall situation, it should wait until endpoint is recovered before read or write oprations.

Signed-off-by Lu Baolu <[email protected]>

---
  drivers/usb/early/xhci-dbc.c | 36 ++++++++++++++++++++++++++++++++++++
  1 file changed, 36 insertions(+)

drivers/usb/early/xhci-dbc.c +21 -0

index c81df40..344d93e 100644
@@ -1163,6 +1163,37 @@ static int xdbc_wait_until_dbc_configured(void)
  	return -ETIMEDOUT;
  }
 
+static int xdbc_wait_until_epstall_cleared(bool read)
+{
+	int timeout = 0;
+
+	if (read) {
+		do {
+			if (!(readl(&xdbcp->xdbc_reg->control) & CTRL_HIT)) {
+				xdbcp->in_ep_state = EP_RUNNING;
+
+				return 0;
+			}
+
+			xdbcp->in_ep_state = EP_HALTED;
+			xdbc_udelay(10);
+		} while (timeout++ < XDBC_LOOPS);
+	} else {
+		do {
+			if (!(readl(&xdbcp->xdbc_reg->control) & CTRL_HOT)) {
+				xdbcp->out_ep_state = EP_RUNNING;
+
+				return 0;

[ text/plain ]

Quoted 40 lines · Lu Baolu

In case of endpoint stall, software is able to detect the situation by reading DCCTRL.HIT or DCCTRL.HOT bits. DbC follows the normal USB framework to handle endpoint stall. When software detects endpoint stall situation, it should wait until endpoint is recovered before read or write oprations.

Signed-off-by Lu Baolu <[email protected]>

---
  drivers/usb/early/xhci-dbc.c | 36 ++++++++++++++++++++++++++++++++++++
  1 file changed, 36 insertions(+)

drivers/usb/early/xhci-dbc.c +21 -0

index c81df40..344d93e 100644
@@ -1163,6 +1163,37 @@ static int xdbc_wait_until_dbc_configured(void)
  	return -ETIMEDOUT;
  }
 
+static int xdbc_wait_until_epstall_cleared(bool read)
+{
+	int timeout = 0;
+
+	if (read) {
+		do {
+			if (!(readl(&xdbcp->xdbc_reg->control) & CTRL_HIT)) {
+				xdbcp->in_ep_state = EP_RUNNING;
+
+				return 0;
+			}
+
+			xdbcp->in_ep_state = EP_HALTED;
+			xdbc_udelay(10);
+		} while (timeout++ < XDBC_LOOPS);
+	} else {
+		do {
+			if (!(readl(&xdbcp->xdbc_reg->control) & CTRL_HOT)) {
+				xdbcp->out_ep_state = EP_RUNNING;
+
+				return 0;

drivers/usb/early/xhci-dbc.c static int xdbc_wait_until_dbc_configured(void) · line 1196

Quoted 10 lines

drivers/usb/early/xhci-dbc.c +9 -0

+			}
+
+			xdbcp->out_ep_state = EP_HALTED;
+			xdbc_udelay(10);
+		} while (timeout++ < XDBC_LOOPS);
+	}
+
+	return -ETIMEDOUT;
+}
 

how about something like:

enum xdbc_ep_state *ep_state;
u32 halt_bit;

if (read) {
	ep_state = &xdbcp->in_ep_state
	halt_bit = CTRL_HIT
} else {
  	ep_state = &xdbcp->out_ep_state
	halt_bit = CTRL_HOT
}
do {
	if (!(readl(..) & halt_bit)) {
		*ep_state = EP_RUNNING;
		return 0;
	}
	*ep_state = EP_HALTED;
	delay()
} while (..)

Quoted 4 lines

drivers/usb/early/xhci-dbc.c +0 -0

@@ -1182,6 +1213,11 @@ static int xdbc_bulk_transfer(void *data, int size, int loops, bool read)
  		return -EPERM;
  	}
 

drivers/usb/early/xhci-dbc.c static int xdbc_bulk_transfer(void *data, int size, int loops, bool read) · line 1218

Quoted 3 lines

drivers/usb/early/xhci-dbc.c +3 -0

+	if (xdbc_wait_until_epstall_cleared(read)) {
+		xdbc_trace("%s: endpoint not ready\n", __func__);
+		return -EPERM;

Is -EPERM appropriate here?

Not sure about what error codes the device side is using, but usually HALT is set due to some Data buffer/transmission/TRB error.

In this case the failure is that debug host failed to send a clearFeature(EP_HALT) request in time.

Hi,

Quoted 57 lines · Mathias Nyman

[ text/plain ]

Quoted 40 lines · Lu Baolu

In case of endpoint stall, software is able to detect the situation by reading DCCTRL.HIT or DCCTRL.HOT bits. DbC follows the normal USB framework to handle endpoint stall. When software detects endpoint stall situation, it should wait until endpoint is recovered before read or write oprations.

Signed-off-by Lu Baolu <[email protected]>

---
  drivers/usb/early/xhci-dbc.c | 36 ++++++++++++++++++++++++++++++++++++
  1 file changed, 36 insertions(+)

drivers/usb/early/xhci-dbc.c +21 -0

index c81df40..344d93e 100644
@@ -1163,6 +1163,37 @@ static int xdbc_wait_until_dbc_configured(void)
  	return -ETIMEDOUT;
  }
 
+static int xdbc_wait_until_epstall_cleared(bool read)
+{
+	int timeout = 0;
+
+	if (read) {
+		do {
+			if (!(readl(&xdbcp->xdbc_reg->control) & CTRL_HIT)) {
+				xdbcp->in_ep_state = EP_RUNNING;
+
+				return 0;
+			}
+
+			xdbcp->in_ep_state = EP_HALTED;
+			xdbc_udelay(10);
+		} while (timeout++ < XDBC_LOOPS);
+	} else {
+		do {
+			if (!(readl(&xdbcp->xdbc_reg->control) & CTRL_HOT)) {
+				xdbcp->out_ep_state = EP_RUNNING;
+
+				return 0;

drivers/usb/early/xhci-dbc.c static int xdbc_wait_until_dbc_configured(void) · line 1196

Quoted 10 lines

drivers/usb/early/xhci-dbc.c +9 -0

+			}
+
+			xdbcp->out_ep_state = EP_HALTED;
+			xdbc_udelay(10);
+		} while (timeout++ < XDBC_LOOPS);
+	}
+
+	return -ETIMEDOUT;
+}
 

how about something like:

enum xdbc_ep_state *ep_state;
u32 halt_bit;

Quoted 15 lines

if (read) {
	ep_state = &xdbcp->in_ep_state
	halt_bit = CTRL_HIT
} else {
  	ep_state = &xdbcp->out_ep_state
	halt_bit = CTRL_HOT
}
do {
	if (!(readl(..) & halt_bit)) {
		*ep_state = EP_RUNNING;
		return 0;
	}
	*ep_state = EP_HALTED;
	delay()
} while (..)

I'll agree, this looks better. Might also want to refactor the handshake loop to its own function:

static int xdbg_ep_state_handshake(enum xdbc_ep_state *ep_state, u32 halt_bit)

{
        do {
		if (!(readl(...) & halt_bit)) {
                	...
                }
                *ep_state = EP_HALTED;
                delay(...);
        } while (...)

Quoted 5 lines

Quoted 5 lines

drivers/usb/early/xhci-dbc.c +0 -0

@@ -1182,6 +1213,11 @@ static int xdbc_bulk_transfer(void *data, int size, int loops, bool read)
  		return -EPERM;
  	}
 

Quoted 7 lines

drivers/usb/early/xhci-dbc.c static int xdbc_bulk_transfer(void *data, int size, int loops, bool read) · line 1218

Quoted 3 lines

drivers/usb/early/xhci-dbc.c +3 -0

+	if (xdbc_wait_until_epstall_cleared(read)) {
+		xdbc_trace("%s: endpoint not ready\n", __func__);
+		return -EPERM;

Is -EPERM appropriate here?

Not sure about what error codes the device side is using, but usually HALT is set due to some Data buffer/transmission/TRB error.

EIO perhaps ?

Quoted 3 lines

In this case the failure is that debug host failed to send a clearFeature(EP_HALT) request in time.

I haven't read the spec, but does it define a maximum time for this to happen ?

Quoted 40 lines · Lu Baolu

In case of endpoint stall, software is able to detect the situation by reading DCCTRL.HIT or DCCTRL.HOT bits. DbC follows the normal USB framework to handle endpoint stall. When software detects endpoint stall situation, it should wait until endpoint is recovered before read or write oprations.

Signed-off-by Lu Baolu <[email protected]>

---
  drivers/usb/early/xhci-dbc.c | 36 ++++++++++++++++++++++++++++++++++++
  1 file changed, 36 insertions(+)

drivers/usb/early/xhci-dbc.c +21 -0

index c81df40..344d93e 100644
@@ -1163,6 +1163,37 @@ static int xdbc_wait_until_dbc_configured(void)
      return -ETIMEDOUT;
  }
 
+static int xdbc_wait_until_epstall_cleared(bool read)
+{
+    int timeout = 0;
+
+    if (read) {
+        do {
+            if (!(readl(&xdbcp->xdbc_reg->control) & CTRL_HIT)) {
+                xdbcp->in_ep_state = EP_RUNNING;
+
+                return 0;
+            }
+
+            xdbcp->in_ep_state = EP_HALTED;
+            xdbc_udelay(10);
+        } while (timeout++ < XDBC_LOOPS);
+    } else {
+        do {
+            if (!(readl(&xdbcp->xdbc_reg->control) & CTRL_HOT)) {
+                xdbcp->out_ep_state = EP_RUNNING;
+
+                return 0;

drivers/usb/early/xhci-dbc.c static int xdbc_wait_until_dbc_configured(void) · line 1196

Quoted 10 lines

drivers/usb/early/xhci-dbc.c +9 -0

+            }
+
+            xdbcp->out_ep_state = EP_HALTED;
+            xdbc_udelay(10);
+        } while (timeout++ < XDBC_LOOPS);
+    }
+
+    return -ETIMEDOUT;
+}
 

how about something like:

enum xdbc_ep_state *ep_state;
u32 halt_bit;

if (read) {
ep_state = &xdbcp->in_ep_state
halt_bit = CTRL_HIT
} else {
ep_state = &xdbcp->out_ep_state
halt_bit = CTRL_HOT
}

do {
    if (!(readl(..) & halt_bit)) {
        *ep_state = EP_RUNNING;
        return 0;
    }
    *ep_state = EP_HALTED;
    delay()
} while (..)

+

Quoted 7 lines

drivers/usb/early/xhci-dbc.c +0 -0

  static int xdbc_bulk_transfer(void *data, int size, int loops, bool read)
  {
      u64 addr;

@@ -1182,6 +1213,11 @@ static int xdbc_bulk_transfer(void *data, int size, int loops, bool read)
          return -EPERM;
      }
 

drivers/usb/early/xhci-dbc.c static int xdbc_bulk_transfer(void *data, int size, int loops, bool read) · line 1218

Quoted 3 lines

drivers/usb/early/xhci-dbc.c +3 -0

+    if (xdbc_wait_until_epstall_cleared(read)) {
+        xdbc_trace("%s: endpoint not ready\n", __func__);
+        return -EPERM;

Is -EPERM appropriate here?
Not sure about what error codes the device side is using, but usually HALT is set due to some Data buffer/transmission/TRB error.
In this case the failure is that debug host failed to send a clearFeature(EP_HALT) request in time.

-Mathias

Hi,

This patch series adds support for early printk through USB3 debug port. USB3 debug port is described in xHCI specification as an optional extended capability.

Patch 1 to 9 add the driver for xHCI debug capability. It interfaces with the register set and provides the required ops (read/write/control) to upper layers. Early printk is one consumer of these ops. The hooks for early printk are introduced in patch 8. This design is similar to what we have done in drivers/usb/early/ehci-dbgp.c.

Patch 10 is the design document and user guide.

Change log:
v1->v2:
(1) Patch 1 re-implemented. "debugfs" has been replaced with sysfs. The scope reduced from all extended capabilities to debug port specific.
(2) Patch 11 changed. Removed unnecessary .bulk_out_size setting.

v2->v3:
(1) Patch 11 got acked by Johan Hovold.

v3->v4:
(1) Patch 1 code refactored by using xhci_find_ext_cap_by_id() helper.
(2) Patch 3 "bus hung state" changed to "port hung state" in commit message.
(3) Patch 12 added verified platform information.

v4->v5:
(1) Patch 1 & 3 code refactored by using xhci_find_next_ext_cap() helper.
(2) Patch 12 updated with more user guide information.
(3) All patches rebased to the new usb-next branch which contains commit d5ddcdf(xhci: rework xhci extended capability list parsing functions).

v5->v6:
(1) refresh the patches against 4.5-rc1.
(2) change "usb: xhci: dbc" to "usb: dbc" in the patch titles as they are not part of xhci driver.

v6->v7:
(1) two patches removed from the patch series. One belongs to xHCI host controller driver, and another belongs to usb/serial.
(2) add fix for 0-DAY kernel test warning:
drivers/usb/early/xhci-dbc.c:917:6-10: WARNING:
Unsigned expression compared with zero: size > 0
(3) add fix for 0-DAY kernel test error:

    arch/x86/built-in.o: In function `setup_early_printk':
    early_printk.c:(.init.text+0xceba): undefined reference to `early_xdbc_init'
    early_printk.c:(.init.text+0xcec8): undefined reference to `early_xdbc_console'

Lu Baolu (10):
x86: fixmap: add permanent fixmap for xhci debug port
usb: dbc: probe and setup xhci debug capability
usb: dbc: add support for Intel xHCI dbc quirk
usb: dbc: add debug buffer
usb: dbc: add bulk out and bulk in interfaces
usb: dbc: handle dbc-configured exit
usb: dbc: handle endpoint stall
x86: early_printk: add USB3 debug port earlyprintk support
usb: dbc: add handshake between debug target and host
usb: doc: add document for xHCI DbC driver

 Documentation/kernel-parameters.txt |    1 +
 Documentation/usb/xhci-dbc.txt      |  350 +++++++++
 MAINTAINERS                         |    8 +
 arch/x86/Kconfig.debug              |   14 +
 arch/x86/include/asm/fixmap.h       |    4 +
 arch/x86/kernel/early_printk.c      |    5 +
 drivers/usb/Kconfig                 |    3 +
 drivers/usb/Makefile                |    2 +-
 drivers/usb/early/Makefile          |    1 +
 drivers/usb/early/xhci-dbc.c        | 1394 +++++++++++++++++++++++++++++++++++
 include/linux/usb/xhci-dbc.h        |  224 ++++++
 11 files changed, 2005 insertions(+), 1 deletion(-)
 create mode 100644 Documentation/usb/xhci-dbc.txt
 create mode 100644 drivers/usb/early/xhci-dbc.c
 create mode 100644 include/linux/usb/xhci-dbc.h

Quoted 51 lines · Lu Baolu

Hi,

This patch series adds support for early printk through USB3 debug port. USB3 debug port is described in xHCI specification as an optional extended capability.

Patch 1 to 9 add the driver for xHCI debug capability. It interfaces with the register set and provides the required ops (read/write/control) to upper layers. Early printk is one consumer of these ops. The hooks for early printk are introduced in patch 8. This design is similar to what we have done in drivers/usb/early/ehci-dbgp.c.

Patch 10 is the design document and user guide.

Change log:
v1->v2:
(1) Patch 1 re-implemented. "debugfs" has been replaced with sysfs. The scope reduced from all extended capabilities to debug port specific.
(2) Patch 11 changed. Removed unnecessary .bulk_out_size setting.

v2->v3:
(1) Patch 11 got acked by Johan Hovold.

v3->v4:
(1) Patch 1 code refactored by using xhci_find_ext_cap_by_id() helper.
(2) Patch 3 "bus hung state" changed to "port hung state" in commit message.
(3) Patch 12 added verified platform information.

v4->v5:
(1) Patch 1 & 3 code refactored by using xhci_find_next_ext_cap() helper.
(2) Patch 12 updated with more user guide information.
(3) All patches rebased to the new usb-next branch which contains commit d5ddcdf(xhci: rework xhci extended capability list parsing functions).

v5->v6:
(1) refresh the patches against 4.5-rc1.
(2) change "usb: xhci: dbc" to "usb: dbc" in the patch titles as they are not part of xhci driver.

v6->v7:
(1) two patches removed from the patch series. One belongs to xHCI host controller driver, and another belongs to usb/serial.
(2) add fix for 0-DAY kernel test warning:
drivers/usb/early/xhci-dbc.c:917:6-10: WARNING:
Unsigned expression compared with zero: size > 0
(3) add fix for 0-DAY kernel test error:

    arch/x86/built-in.o: In function `setup_early_printk':
    early_printk.c:(.init.text+0xceba): undefined reference to `early_xdbc_init'
    early_printk.c:(.init.text+0xcec8): undefined reference to `early_xdbc_console'

Quoted 11 lines

Lu Baolu (10):
x86: fixmap: add permanent fixmap for xhci debug port
usb: dbc: probe and setup xhci debug capability
usb: dbc: add support for Intel xHCI dbc quirk
usb: dbc: add debug buffer
usb: dbc: add bulk out and bulk in interfaces
usb: dbc: handle dbc-configured exit
usb: dbc: handle endpoint stall
x86: early_printk: add USB3 debug port earlyprintk support
usb: dbc: add handshake between debug target and host
usb: doc: add document for xHCI DbC driver

Hi Greg,

I removed two patches from this series which belong to usb/xhci and usb/serial. The remaining patches are all for usb3 debug port driver. Will you queue this for usb-next? Or, do you want me to do anything else?

Quoted 64 lines · Lu Baolu

Quoted 62 lines · Lu Baolu

Hi,

This patch series adds support for early printk through USB3 debug port. USB3 debug port is described in xHCI specification as an optional extended capability.

Patch 1 to 9 add the driver for xHCI debug capability. It interfaces with the register set and provides the required ops (read/write/control) to upper layers. Early printk is one consumer of these ops. The hooks for early printk are introduced in patch 8. This design is similar to what we have done in drivers/usb/early/ehci-dbgp.c.

Patch 10 is the design document and user guide.

Change log:
v1->v2:
(1) Patch 1 re-implemented. "debugfs" has been replaced with sysfs. The scope reduced from all extended capabilities to debug port specific.
(2) Patch 11 changed. Removed unnecessary .bulk_out_size setting.

v2->v3:
(1) Patch 11 got acked by Johan Hovold.

v3->v4:
(1) Patch 1 code refactored by using xhci_find_ext_cap_by_id() helper.
(2) Patch 3 "bus hung state" changed to "port hung state" in commit message.
(3) Patch 12 added verified platform information.

v4->v5:
(1) Patch 1 & 3 code refactored by using xhci_find_next_ext_cap() helper.
(2) Patch 12 updated with more user guide information.
(3) All patches rebased to the new usb-next branch which contains commit d5ddcdf(xhci: rework xhci extended capability list parsing functions).

v5->v6:
(1) refresh the patches against 4.5-rc1.
(2) change "usb: xhci: dbc" to "usb: dbc" in the patch titles as they are not part of xhci driver.

v6->v7:
(1) two patches removed from the patch series. One belongs to xHCI host controller driver, and another belongs to usb/serial.
(2) add fix for 0-DAY kernel test warning:
drivers/usb/early/xhci-dbc.c:917:6-10: WARNING:
Unsigned expression compared with zero: size > 0
(3) add fix for 0-DAY kernel test error:

    arch/x86/built-in.o: In function `setup_early_printk':
    early_printk.c:(.init.text+0xceba): undefined reference to `early_xdbc_init'
    early_printk.c:(.init.text+0xcec8): undefined reference to `early_xdbc_console'

Lu Baolu (10):
x86: fixmap: add permanent fixmap for xhci debug port
usb: dbc: probe and setup xhci debug capability
usb: dbc: add support for Intel xHCI dbc quirk
usb: dbc: add debug buffer
usb: dbc: add bulk out and bulk in interfaces
usb: dbc: handle dbc-configured exit
usb: dbc: handle endpoint stall
x86: early_printk: add USB3 debug port earlyprintk support
usb: dbc: add handshake between debug target and host
usb: doc: add document for xHCI DbC driver

Quoted 5 lines

Hi Greg,

I removed two patches from this series which belong to usb/xhci and usb/serial. The remaining patches are all for usb3 debug port driver. Will you queue this for usb-next? Or, do you want me to do anything else?

Given that these are touching xhci code, in a way, I need an ack from the XHCI maintainer before I can do anything with them, as I can't test them myself.