From f6604627ff71d42bb63a3d81c2986a9d296d55cb Mon Sep 17 00:00:00 2001 From: Michael Brown Date: Wed, 6 May 2015 16:38:28 +0100 Subject: [PATCH] [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 --- src/drivers/bus/usb.c | 22 +++++----- src/drivers/usb/ehci.c | 9 +++- src/drivers/usb/usbhub.c | 3 ++ src/drivers/usb/xhci.c | 89 ++++++++++++++++++++++------------------ src/include/ipxe/usb.h | 6 +++ 5 files changed, 78 insertions(+), 51 deletions(-) diff --git a/src/drivers/bus/usb.c b/src/drivers/bus/usb.c index 2191867a..2b9efa45 100644 --- 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 653e72c1..d64024da 100644 --- 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 6d0cdba4..8b5fa9c4 100644 --- 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 307a9912..8e5d6e60 100644 --- 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 991a6f44..50d8a6fd 100644 --- 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)