Re: [PATCH v3 2/3] usb: xhci: Add DbC support in xHCI driver

From: Mathias Nyman
Date: Mon Nov 13 2017 - 10:23:52 EST


Ok, review last part

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

extra whitespace i think

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


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

Thanks
-Mathias