[usb] Handle port status changes received after failing to find a driver

Commit a60f2dd ("[usb] Try multiple USB device configurations")
changed the behaviour of register_usb() such that if no drivers are
found then the device will be closed and the memory used will be
freed.

If a port status change subsequently occurs while the device is still
physically attached, then usb_hotplug() will see this as a new device
having been attached, since there is no device recorded as being
currently attached to the port.  This can lead to spurious hotplug
events (or even endless loops of hotplug events, if the process of
opening and closing the device happens to generate a port status
change).

Fix by using a separate flag to indicate that a device is physically
attached (even if we have no corresponding struct usb_device).

Reported-by: Dan Ellis <Dan.Ellis@displaylink.com>
Signed-off-by: Michael Brown <mcb30@ipxe.org>

diff --git a/src/drivers/bus/usb.c b/src/drivers/bus/usb.c
index 8900324aafc3369e2c96f3432a855739d6e5d3be..397e5a84d01ed91775122b8684f3d9623cbe8c50 100644 (file)
--- a/src/drivers/bus/usb.c
+++ b/src/drivers/bus/usb.c
@@ -1314,6 +1314,9 @@ static int usb_attached ( struct usb_port *port ) {
        struct usb_device *usb;
        int rc;
 
+       /* Mark port as attached */
+       port->attached = 1;
+
        /* Sanity checks */
        assert ( port->usb == NULL );
 
@@ -1345,8 +1348,12 @@ static int usb_attached ( struct usb_port *port ) {
 static void usb_detached ( struct usb_port *port ) {
        struct usb_device *usb = port->usb;
 
-       /* Sanity checks */
-       assert ( port->usb != NULL );
+       /* Mark port as detached */
+       port->attached = 0;
+
+       /* Do nothing if we have no USB device */
+       if ( ! usb )
+               return;
 
        /* Unregister USB device */
        unregister_usb ( usb );
@@ -1373,10 +1380,10 @@ static int usb_hotplug ( struct usb_port *port ) {
        }
 
        /* Handle attached/detached device as applicable */
-       if ( port->speed && ! port->usb ) {
+       if ( port->speed && ! port->attached ) {
                /* Newly attached device */
                return usb_attached ( port );
-       } else if ( port->usb && ! port->speed ) {
+       } else if ( port->attached && ! port->speed ) {
                /* Newly detached device */
                usb_detached ( port );
                return 0;
@@ -1546,7 +1553,7 @@ void unregister_usb_hub ( struct usb_hub *hub ) {
        /* Detach all devices */
        for ( i = 1 ; i <= hub->ports ; i++ ) {
                port = usb_port ( hub, i );
-               if ( port->usb )
+               if ( port->attached )
                        usb_detached ( port );
        }
 
@@ -1576,6 +1583,7 @@ void free_usb_hub ( struct usb_hub *hub ) {
        /* Sanity checks */
        for ( i = 1 ; i <= hub->ports ; i++ ) {
                port = usb_port ( hub, i );
+               assert ( ! port->attached );
                assert ( port->usb == NULL );
                assert ( list_empty ( &port->list ) );
        }

diff --git a/src/include/ipxe/usb.h b/src/include/ipxe/usb.h
index 1f5a85ecd7ef9c15e80dd2a5f6bfe3e8a9322c4c..8e18241603216f28e0b54b9e84083048c5c391cc 100644 (file)
--- a/src/include/ipxe/usb.h
+++ b/src/include/ipxe/usb.h
@@ -702,7 +702,13 @@ struct usb_port {
        unsigned int protocol;
        /** Port speed */
        unsigned int speed;
-       /** Currently attached device (if any) */
+       /** Port has an attached device */
+       int attached;
+       /** Currently attached device (if in use)
+        *
+        * Note that this field will be NULL if the attached device
+        * has been freed (e.g. because there were no drivers found).
+        */
        struct usb_device *usb;
        /** List of changed ports */
        struct list_head list;