[usb] Detect missed disconnections

The USB core will currently fail to detect disconnections if a new
device has attached by the time the port is examined in
usb_hotplug().

Fix by recording the fact that a disconnection has taken place
whenever the "connection status changed" (CSC) bit is observed to be
set.  (Whether the change represents a disconnection or a
reconnection, it indicates that the port has experienced some time of
being disconnected.)

Note that the time at which a disconnection can be detected varies by
hub type.  In particular: root hubs can observe the CSC bit when
polling, and so will record the disconnection before calling
usb_port_changed(), but USB hubs read the port status (and hence the
CSC bit) only during the call to hub_speed(), long after the call to
usb_port_changed().

Signed-off-by: Michael Brown <mcb30@ipxe.org>

diff --git a/src/drivers/bus/usb.c b/src/drivers/bus/usb.c
index 2191867ae27bb045c72aabb145f9617c8325fb89..2b9efa4549c7219e9094a670bf59b34421a197bd 100644 (file)
--- a/src/drivers/bus/usb.c
+++ b/src/drivers/bus/usb.c
@@ -1589,18 +1589,20 @@ static int usb_hotplug ( struct usb_port *port ) {
                return rc;
        }
 
-       /* Handle attached/detached device as applicable */
-       if ( port->speed && ! port->attached ) {
-               /* Newly attached device */
-               return usb_attached ( port );
-       } else if ( port->attached && ! port->speed ) {
-               /* Newly detached device */
+       /* Detach device, if applicable */
+       if ( port->attached && ( port->disconnected || ! port->speed ) )
                usb_detached ( port );
-               return 0;
-       } else {
-               /* Ignore */
-               return 0;
+
+       /* Attach device, if applicable */
+       if ( port->speed && ! port->attached ) {
+               if ( ( rc = usb_attached ( port ) ) != 0 )
+                       return rc;
        }
+
+       /* Clear any recorded disconnections */
+       port->disconnected = 0;
+
+       return 0;
 }
 
 /******************************************************************************

diff --git a/src/drivers/usb/ehci.c b/src/drivers/usb/ehci.c
index 653e72c13256f8fc27af222f0fe9397032f6e068..d64024da27060283620189d18bf1ae2b8527e269 100644 (file)
--- a/src/drivers/usb/ehci.c
+++ b/src/drivers/usb/ehci.c
@@ -1498,6 +1498,7 @@ static int ehci_root_speed ( struct usb_hub *hub, struct usb_port *port ) {
        unsigned int speed;
        unsigned int line;
        int ccs;
+       int csc;
        int ped;
 
        /* Read port status */
@@ -1505,9 +1506,14 @@ static int ehci_root_speed ( struct usb_hub *hub, struct usb_port *port ) {
        DBGC2 ( ehci, "EHCI %p port %d status is %08x\n",
                ehci, port->address, portsc );
        ccs = ( portsc & EHCI_PORTSC_CCS );
+       csc = ( portsc & EHCI_PORTSC_CSC );
        ped = ( portsc & EHCI_PORTSC_PED );
        line = EHCI_PORTSC_LINE_STATUS ( portsc );
 
+       /* Record disconnections and clear changes */
+       port->disconnected |= csc;
+       writel ( portsc, ehci->op + EHCI_OP_PORTSC ( port->address ) );
+
        /* Determine port speed */
        if ( ! ccs ) {
                /* Port not connected */
@@ -1564,7 +1570,8 @@ static void ehci_root_poll ( struct usb_hub *hub, struct usb_port *port ) {
        if ( ! change )
                return;
 
-       /* Acknowledge changes */
+       /* Record disconnections and clear changes */
+       port->disconnected |= ( portsc & EHCI_PORTSC_CSC );
        writel ( portsc, ehci->op + EHCI_OP_PORTSC ( port->address ) );
 
        /* Report port status change */

diff --git a/src/drivers/usb/usbhub.c b/src/drivers/usb/usbhub.c
index 6d0cdba454f052d377fdd7e31f121d763264f503..8b5fa9c4d1ecdb64c05bcfdcb9cfb75ee109cfc1 100644 (file)
--- a/src/drivers/usb/usbhub.c
+++ b/src/drivers/usb/usbhub.c
@@ -331,6 +331,9 @@ static int hub_speed ( struct usb_hub *hub, struct usb_port *port ) {
                port->speed = USB_SPEED_NONE;
        }
 
+       /* Record disconnections */
+       port->disconnected |= ( changed & ( 1 << USB_HUB_PORT_CONNECTION ) );
+
        /* Clear port status change bits */
        if ( ( rc = hub_clear_changes ( hubdev, port->address, changed ) ) != 0)
                return rc;

diff --git a/src/drivers/usb/xhci.c b/src/drivers/usb/xhci.c
index 307a99120d116f4a8babff6ef20d4757cdb2f39a..8e5d6e608953b067e68b633bf179d683635f101d 100644 (file)
--- a/src/drivers/usb/xhci.c
+++ b/src/drivers/usb/xhci.c
@@ -1532,10 +1532,10 @@ static void xhci_event_free ( struct xhci_device *xhci ) {
  * Handle transfer event
  *
  * @v xhci             xHCI device
- * @v transfer         Transfer event TRB
+ * @v trb              Transfer event TRB
  */
 static void xhci_transfer ( struct xhci_device *xhci,
-                           struct xhci_trb_transfer *transfer ) {
+                           struct xhci_trb_transfer *trb ) {
        struct xhci_slot *slot;
        struct xhci_endpoint *endpoint;
        struct io_buffer *iobuf;
@@ -1545,20 +1545,20 @@ static void xhci_transfer ( struct xhci_device *xhci,
        profile_start ( &xhci_transfer_profiler );
 
        /* Identify slot */
-       if ( ( transfer->slot > xhci->slots ) ||
-            ( ( slot = xhci->slot[transfer->slot] ) == NULL ) ) {
+       if ( ( trb->slot > xhci->slots ) ||
+            ( ( slot = xhci->slot[trb->slot] ) == NULL ) ) {
                DBGC ( xhci, "XHCI %p transfer event invalid slot %d:\n",
-                      xhci, transfer->slot );
-               DBGC_HDA ( xhci, 0, transfer, sizeof ( *transfer ) );
+                      xhci, trb->slot );
+               DBGC_HDA ( xhci, 0, trb, sizeof ( *trb ) );
                return;
        }
 
        /* Identify endpoint */
-       if ( ( transfer->endpoint > XHCI_CTX_END ) ||
-            ( ( endpoint = slot->endpoint[transfer->endpoint] ) == NULL ) ) {
+       if ( ( trb->endpoint > XHCI_CTX_END ) ||
+            ( ( endpoint = slot->endpoint[trb->endpoint] ) == NULL ) ) {
                DBGC ( xhci, "XHCI %p slot %d transfer event invalid epid "
-                      "%d:\n", xhci, slot->id, transfer->endpoint );
-               DBGC_HDA ( xhci, 0, transfer, sizeof ( *transfer ) );
+                      "%d:\n", xhci, slot->id, trb->endpoint );
+               DBGC_HDA ( xhci, 0, trb, sizeof ( *trb ) );
                return;
        }
 
@@ -1567,15 +1567,15 @@ static void xhci_transfer ( struct xhci_device *xhci,
        assert ( iobuf != NULL );
 
        /* Check for errors */
-       if ( ! ( ( transfer->code == XHCI_CMPLT_SUCCESS ) ||
-                ( transfer->code == XHCI_CMPLT_SHORT ) ) ) {
+       if ( ! ( ( trb->code == XHCI_CMPLT_SUCCESS ) ||
+                ( trb->code == XHCI_CMPLT_SHORT ) ) ) {
 
                /* Construct error */
-               rc = -ECODE ( transfer->code );
+               rc = -ECODE ( trb->code );
                DBGC ( xhci, "XHCI %p slot %d ctx %d failed (code %d): %s\n",
-                      xhci, slot->id, endpoint->ctx, transfer->code,
+                      xhci, slot->id, endpoint->ctx, trb->code,
                       strerror ( rc ) );
-               DBGC_HDA ( xhci, 0, transfer, sizeof ( *transfer ) );
+               DBGC_HDA ( xhci, 0, trb, sizeof ( *trb ) );
 
                /* Sanity check */
                assert ( ( endpoint->context->state & XHCI_ENDPOINT_STATE_MASK )
@@ -1587,11 +1587,11 @@ static void xhci_transfer ( struct xhci_device *xhci,
        }
 
        /* Record actual transfer size */
-       iob_unput ( iobuf, le16_to_cpu ( transfer->residual ) );
+       iob_unput ( iobuf, le16_to_cpu ( trb->residual ) );
 
        /* Sanity check (for successful completions only) */
        assert ( xhci_ring_consumed ( &endpoint->ring ) ==
-                le64_to_cpu ( transfer->transfer ) );
+                le64_to_cpu ( trb->transfer ) );
 
        /* Report completion to USB core */
        usb_complete ( endpoint->ep, iobuf );
@@ -1602,24 +1602,24 @@ static void xhci_transfer ( struct xhci_device *xhci,
  * Handle command completion event
  *
  * @v xhci             xHCI device
- * @v complete         Command completion event
+ * @v trb              Command completion event
  */
 static void xhci_complete ( struct xhci_device *xhci,
-                           struct xhci_trb_complete *complete ) {
+                           struct xhci_trb_complete *trb ) {
        int rc;
 
        /* Ignore "command ring stopped" notifications */
-       if ( complete->code == XHCI_CMPLT_CMD_STOPPED ) {
+       if ( trb->code == XHCI_CMPLT_CMD_STOPPED ) {
                DBGC2 ( xhci, "XHCI %p command ring stopped\n", xhci );
                return;
        }
 
        /* Ignore unexpected completions */
        if ( ! xhci->pending ) {
-               rc = -ECODE ( complete->code );
+               rc = -ECODE ( trb->code );
                DBGC ( xhci, "XHCI %p unexpected completion (code %d): %s\n",
-                      xhci, complete->code, strerror ( rc ) );
-               DBGC_HDA ( xhci, 0, complete, sizeof ( *complete ) );
+                      xhci, trb->code, strerror ( rc ) );
+               DBGC_HDA ( xhci, 0, trb, sizeof ( *trb ) );
                return;
        }
 
@@ -1628,10 +1628,10 @@ static void xhci_complete ( struct xhci_device *xhci,
 
        /* Sanity check */
        assert ( xhci_ring_consumed ( &xhci->command ) ==
-                le64_to_cpu ( complete->command ) );
+                le64_to_cpu ( trb->command ) );
 
        /* Record completion */
-       memcpy ( xhci->pending, complete, sizeof ( *xhci->pending ) );
+       memcpy ( xhci->pending, trb, sizeof ( *xhci->pending ) );
        xhci->pending = NULL;
 }
 
@@ -1639,38 +1639,40 @@ static void xhci_complete ( struct xhci_device *xhci,
  * Handle port status event
  *
  * @v xhci             xHCI device
- * @v port             Port status event
+ * @v trb              Port status event
  */
 static void xhci_port_status ( struct xhci_device *xhci,
-                              struct xhci_trb_port_status *port ) {
+                              struct xhci_trb_port_status *trb ) {
+       struct usb_port *port = usb_port ( xhci->bus->hub, trb->port );
        uint32_t portsc;
 
        /* Sanity check */
-       assert ( ( port->port > 0 ) && ( port->port <= xhci->ports ) );
+       assert ( ( trb->port > 0 ) && ( trb->port <= xhci->ports ) );
 
-       /* Clear port status change bits */
-       portsc = readl ( xhci->op + XHCI_OP_PORTSC ( port->port ) );
+       /* Record disconnections and clear changes */
+       portsc = readl ( xhci->op + XHCI_OP_PORTSC ( trb->port ) );
+       port->disconnected |= ( portsc & XHCI_PORTSC_CSC );
        portsc &= ( XHCI_PORTSC_PRESERVE | XHCI_PORTSC_CHANGE );
-       writel ( portsc, xhci->op + XHCI_OP_PORTSC ( port->port ) );
+       writel ( portsc, xhci->op + XHCI_OP_PORTSC ( trb->port ) );
 
        /* Report port status change */
-       usb_port_changed ( usb_port ( xhci->bus->hub, port->port ) );
+       usb_port_changed ( port );
 }
 
 /**
  * Handle host controller event
  *
  * @v xhci             xHCI device
- * @v host             Host controller event
+ * @v trb              Host controller event
  */
 static void xhci_host_controller ( struct xhci_device *xhci,
-                                  struct xhci_trb_host_controller *host ) {
+                                  struct xhci_trb_host_controller *trb ) {
        int rc;
 
        /* Construct error */
-       rc = -ECODE ( host->code );
+       rc = -ECODE ( trb->code );
        DBGC ( xhci, "XHCI %p host controller event (code %d): %s\n",
-              xhci, host->code, strerror ( rc ) );
+              xhci, trb->code, strerror ( rc ) );
 }
 
 /**
@@ -3014,6 +3016,7 @@ static int xhci_root_speed ( struct usb_hub *hub, struct usb_port *port ) {
        unsigned int psiv;
        int ccs;
        int ped;
+       int csc;
        int speed;
        int rc;
 
@@ -3021,9 +3024,17 @@ static int xhci_root_speed ( struct usb_hub *hub, struct usb_port *port ) {
        portsc = readl ( xhci->op + XHCI_OP_PORTSC ( port->address ) );
        DBGC2 ( xhci, "XHCI %p port %d status is %08x\n",
                xhci, port->address, portsc );
-
-       /* Check whether or not port is connected */
        ccs = ( portsc & XHCI_PORTSC_CCS );
+       ped = ( portsc & XHCI_PORTSC_PED );
+       csc = ( portsc & XHCI_PORTSC_CSC );
+       psiv = XHCI_PORTSC_PSIV ( portsc );
+
+       /* Record disconnections and clear changes */
+       port->disconnected |= csc;
+       portsc &= ( XHCI_PORTSC_PRESERVE | XHCI_PORTSC_CHANGE );
+       writel ( portsc, xhci->op + XHCI_OP_PORTSC ( port->address ) );
+
+       /* Port speed is not valid unless port is connected */
        if ( ! ccs ) {
                port->speed = USB_SPEED_NONE;
                return 0;
@@ -3032,14 +3043,12 @@ static int xhci_root_speed ( struct usb_hub *hub, struct usb_port *port ) {
        /* For USB2 ports, the PSIV field is not valid until the port
         * completes reset and becomes enabled.
         */
-       ped = ( portsc & XHCI_PORTSC_PED );
        if ( ( port->protocol < USB_PROTO_3_0 ) && ! ped ) {
                port->speed = USB_SPEED_FULL;
                return 0;
        }
 
        /* Get port speed and map to generic USB speed */
-       psiv = XHCI_PORTSC_PSIV ( portsc );
        speed = xhci_port_speed ( xhci, port->address, psiv );
        if ( speed < 0 ) {
                rc = speed;

diff --git a/src/include/ipxe/usb.h b/src/include/ipxe/usb.h
index 991a6f44ebfb84b0d5a8866ca5463306d1b9df53..50d8a6fd8e8e20a3ae5967c299cd9c37fa82e1b3 100644 (file)
--- a/src/include/ipxe/usb.h
+++ b/src/include/ipxe/usb.h
@@ -747,6 +747,12 @@ struct usb_port {
        unsigned int protocol;
        /** Port speed */
        unsigned int speed;
+       /** Port disconnection has been detected
+        *
+        * This should be set whenever the underlying hardware reports
+        * a connection status change.
+        */
+       int disconnected;
        /** Port has an attached device */
        int attached;
        /** Currently attached device (if in use)