From b85614cb9e5912b3fc0b11cec82417efaad3b1a7 Mon Sep 17 00:00:00 2001
From: Peter Wu <peter@lekensteyn.nl>
Date: Sun, 16 Mar 2014 12:55:19 +0100
Subject: usbdump: comments update

And I was wondering why the interrupt packets in dev-hid.c had a
reversed type marking...
---
 hw/usb/dump.c |  9 ++++++++-
 hw/usb/dump.h | 22 ++++++++++++++++++++--
 2 files changed, 28 insertions(+), 3 deletions(-)

(limited to 'hw')

diff --git a/hw/usb/dump.c b/hw/usb/dump.c
index 672204d9c1..26af6dff59 100644
--- a/hw/usb/dump.c
+++ b/hw/usb/dump.c
@@ -260,6 +260,8 @@ static void init_from_usbpacket(UsbDumpState *s, struct usbmon_packet *u,
     }
 
     /* int32_t      - 48: Only for Interrupt and ISO */
+    /* NOTE: the value filled in here is the upper bound, the OS determines the
+     * actual value. */
     u->interval     = get_ep_interval(p->ep);
 
     /* int32_t      - 52: For ISO */
@@ -290,7 +292,8 @@ void usb_dump_submit(UsbDumpState *s, const USBPacket *p)
     }
 
     if (ep_type == USB_ENDPOINT_XFER_CONTROL) {
-        // cannot get called for OUT
+        /* PID is IN because this is the status stage for OUT, it cannot get
+         * called for OUT (that would be the data packet themselves) */
         assert(pid == USB_TOKEN_IN || pid == USB_TOKEN_SETUP);
 
         if (pid == USB_TOKEN_IN) {
@@ -326,6 +329,7 @@ void usb_dump_complete(UsbDumpState *s, const USBPacket *p)
         assert(pid == USB_TOKEN_IN || pid == USB_TOKEN_SETUP);
 
         if (pid == USB_TOKEN_SETUP) {
+            /* this was an IN request, data is now available. */
             assert(dev->setup_buf[0] & USB_DIR_IN);
             datalen = p->actual_length;
         }
@@ -428,6 +432,9 @@ int usb_dump_init(UsbDumpState *s, const char *filename)
     s->fd = fd;
     s->pcap_caplen = snaplen;
 
+    // FIXME: timing is incorrect, this should be set to the start time of the
+    // host, not the current time. Otherwise the clock is completely off when
+    // adding a device in the monitor.
     s->start_ts = qemu_clock_get_us(QEMU_CLOCK_HOST);
 
     return 0;
diff --git a/hw/usb/dump.h b/hw/usb/dump.h
index a54bc5862d..247016f259 100644
--- a/hw/usb/dump.h
+++ b/hw/usb/dump.h
@@ -54,9 +54,27 @@ typedef struct UsbDumpState UsbDumpState;
  * - setup(setup_len == 0, OUT) / status(OUT) (BROKEN!)
  *
  * handle_data is called for all non-EP0 transfers. PID is either IN or OUT.
- * Sequence for the PID cases:
- * - IN: submit(); make data; complete(data); usb_packet_copy(data)
+ * Sequence for the PID cases (interrupt):
+ * - IN: make data; complete(data); usb_packet_copy(data); submit()
  * - OUT: usb_packet_copy(data); submit(data); process data; complete()
+ *
+ * If there is no data for IN, just return NAK without dumping to avoid log
+ * flood (due to the periodic polling by interrupts and isoc).
+ * Isoc has probably the same sequence as interrupt; bulk less so.
+ * In the IN sequence, note that complete(data) is called before submit. See the
+ * below diagram for the reason.
+ *             ___________
+ *       _____ \ ev_type /
+ *      | dir \ | S | C |
+ *      |       +---+---+
+ *      |  IN   | n | y | (whether the packet has data for this
+ *      |  OUT  | y | n |  combination of ev_type and direction)
+ *      --------+---+---+
+ *
+ * Control: DONE
+ * Int: IN PROGRESS
+ * Bulk: TODO
+ * Isoc: TODO
  */
 
 /*
-- 
cgit v1.2.1