On 29.08.2016 08:26, Lu Baolu 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. 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. This code is designed to be only used for kernel debugging when machine crashes very early before the console code is initialized. For normal operation it is not recommended. Cc: Mathias Nyman <mathias.nyman@xxxxxxxxxxxxxxx> Signed-off-by: Lu Baolu <baolu.lu@xxxxxxxxxxxxxxx> ---
Some comments inline
+ +#ifdef XDBC_TRACE +#define xdbc_trace trace_printk +#else +static inline void xdbc_trace(const char *fmt, ...) { } +#endif /* XDBC_TRACE */
I guess there's probably no better way to enable/disable debug messages for this driver this early, and ehci-dbgp does the same. So I guess It's ok, but it still looks weird
+ +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); +} +
Glad to see the addition of this runtime_delay in addition to the hack of reading port 0x80 for a 1us delay. And that the early_delay is only used for as long as it must.
+static int xdbc_handle_external_reset(void) +{ + u32 ctrl; + int ret; + + writel(0, &xdbc.xdbc_reg->control); + ret = handshake(&xdbc.xdbc_reg->control, CTRL_DCE, 0, 100000, 10); + if (ret) + return ret; + + xdbc_mem_init(); + mmiowb(); + + 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, 10); + if (ret) + return ret; + + if (xdbc.vendor == PCI_VENDOR_ID_INTEL) + xdbc_do_reset_debug_port(xdbc.port_number, 1); + + /* wait for port connection */ + ret = handshake(&xdbc.xdbc_reg->portsc, + PORTSC_CCS, PORTSC_CCS, 5000000, 10); + if (ret) + return ret; + + /* wait for debug device to be configured */ + ret = handshake(&xdbc.xdbc_reg->control, + CTRL_DCR, CTRL_DCR, 5000000, 10); + if (ret) + return ret; + + xdbc.flags |= XDBC_FLAGS_INITIALIZED | XDBC_FLAGS_CONFIGURED; + + xdbc_bulk_transfer(NULL, XDBC_MAX_PACKET, true); + + return 0; +} +
...
+static int __init xdbc_early_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) { + pr_notice("falied 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) { + pr_notice("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) { + pr_notice("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)) { + pr_notice("invalid root hub port number\n"); + return -ENODEV; + } + + xdbc.port_number = DCST_DPN(status); + + pr_notice("DbC is running now, control 0x%08x port ID %d\n", + readl(&xdbc.xdbc_reg->control), xdbc.port_number); + + return 0; +}
xdbc_early_start() and xdbc_handle_external_reset() have a lot of duplicate code , checking DCE, resetting the port, wait for CCE and wait for DCR. Maybe put the common parts in a separate function?
+static void xdbc_scrub_function(struct work_struct *work) +{ + unsigned long flags; + int ret; + + /* + * DbC is running, check the event ring and + * handle the events. + */ + if (readl(&xdbc.xdbc_reg->control) & CTRL_DRC) { + spin_lock_irqsave(&xdbc.lock, flags); + xdbc_handle_events(); + spin_unlock_irqrestore(&xdbc.lock, flags); + } + + /* + * It's time to shutdown DbC, so that the debug + * port could be reused by the host controller. + */ + if (unlikely(early_xdbc_console.index == -1 || + (early_xdbc_console.flags & CON_BOOT))) { + writel(0, &xdbc.xdbc_reg->control); + xdbc_trace("hardware not used any more\n"); + return;
Shouldn't we free rings etc here as well? we won't queue anymore work after this, right?
+ } + + /* + * External reset happened. Need to restart the + * debugging hardware. + */ + if (unlikely(!(readl(&xdbc.xdbc_reg->control) & CTRL_DCE))) { + xdbc.flags &= ~(XDBC_FLAGS_INITIALIZED + | XDBC_FLAGS_CONFIGURED); + ret = xdbc_handle_external_reset(); + if (ret) { + xdbc_trace("hardware failed to recover\n"); + return;
And here?
+ } + } + + queue_delayed_work(xdbc_wq, &xdbc.scrub, usecs_to_jiffies(100)); +} +
-Mathias -- To unsubscribe from this list: send the line "unsubscribe linux-usb" in the body of a message to majordomo@xxxxxxxxxxxxxxx More majordomo info at http://vger.kernel.org/majordomo-info.html