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.ny...@linux.intel.com>
Signed-off-by: Lu Baolu <baolu...@linux.intel.com>
---

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 majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Reply via email to