From ebf01660810c4ff923754323049d49d37321fd49 Mon Sep 17 00:00:00 2001 From: Michael Brown Date: Mon, 12 Oct 2020 15:21:25 +0100 Subject: [PATCH] [usb] Allow device halt to be cleared independently of host controller Closing and reopening a USB endpoint will clear any halt status recorded by the host controller, but may leave the endpoint halted at the device. This will cause the first packet submitted to the reopened endpoint to be lost, before the automatic stall recovery mechanism detects the halt and resets the endpoint. This is relatively harmless for USB network or HID devices, since the wire protocols will recover gracefully from dropped packets. Some protocols (e.g. for USB mass storage devices) assume zero packet loss and so would be adversely affected. Fix by allowing any device endpoint halt status to be cleared on a freshly opened endpoint. Signed-off-by: Michael Brown --- src/drivers/bus/usb.c | 45 +++++++++++++++++++++++++++++------------- src/include/ipxe/usb.h | 1 + 2 files changed, 32 insertions(+), 14 deletions(-) diff --git a/src/drivers/bus/usb.c b/src/drivers/bus/usb.c index 14eabb6b4..88df9c64a 100644 --- a/src/drivers/bus/usb.c +++ b/src/drivers/bus/usb.c @@ -362,6 +362,35 @@ static int usb_endpoint_clear_tt ( struct usb_endpoint *ep ) { return 0; } +/** + * Clear endpoint halt (if applicable) + * + * @v ep USB endpoint + * @ret rc Return status code + */ +int usb_endpoint_clear_halt ( struct usb_endpoint *ep ) { + struct usb_device *usb = ep->usb; + unsigned int type; + int rc; + + /* Clear transaction translator, if applicable */ + if ( ( rc = usb_endpoint_clear_tt ( ep ) ) != 0 ) + return rc; + + /* Clear endpoint halt (if applicable) */ + type = ( ep->attributes & USB_ENDPOINT_ATTR_TYPE_MASK ); + if ( ( type != USB_ENDPOINT_ATTR_CONTROL ) && + ( ( rc = usb_clear_feature ( usb, USB_RECIP_ENDPOINT, + USB_ENDPOINT_HALT, + ep->address ) ) != 0 ) ) { + DBGC ( usb, "USB %s %s could not clear endpoint halt: %s\n", + usb->name, usb_endpoint_name ( ep ), strerror ( rc ) ); + return rc; + } + + return 0; +} + /** * Close USB endpoint * @@ -399,27 +428,15 @@ void usb_endpoint_close ( struct usb_endpoint *ep ) { */ static int usb_endpoint_reset ( struct usb_endpoint *ep ) { struct usb_device *usb = ep->usb; - unsigned int type; int rc; /* Sanity check */ assert ( ! list_empty ( &ep->halted ) ); - /* Clear transaction translator, if applicable */ - if ( ( rc = usb_endpoint_clear_tt ( ep ) ) != 0 ) + /* Clear device halt, if applicable */ + if ( ( rc = usb_endpoint_clear_halt ( ep ) ) != 0 ) return rc; - /* Clear endpoint halt, if applicable */ - type = ( ep->attributes & USB_ENDPOINT_ATTR_TYPE_MASK ); - if ( ( type != USB_ENDPOINT_ATTR_CONTROL ) && - ( ( rc = usb_clear_feature ( usb, USB_RECIP_ENDPOINT, - USB_ENDPOINT_HALT, - ep->address ) ) != 0 ) ) { - DBGC ( usb, "USB %s %s could not clear endpoint halt: %s\n", - usb->name, usb_endpoint_name ( ep ), strerror ( rc ) ); - return rc; - } - /* Reset endpoint */ if ( ( rc = ep->host->reset ( ep ) ) != 0 ) { DBGC ( usb, "USB %s %s could not reset: %s\n", diff --git a/src/include/ipxe/usb.h b/src/include/ipxe/usb.h index 68289d26d..a05ac613c 100644 --- a/src/include/ipxe/usb.h +++ b/src/include/ipxe/usb.h @@ -580,6 +580,7 @@ usb_endpoint_described ( struct usb_endpoint *ep, struct usb_interface_descriptor *interface, unsigned int type, unsigned int index ); extern int usb_endpoint_open ( struct usb_endpoint *ep ); +extern int usb_endpoint_clear_halt ( struct usb_endpoint *ep ); extern void usb_endpoint_close ( struct usb_endpoint *ep ); extern int usb_message ( struct usb_endpoint *ep, unsigned int request, unsigned int value, unsigned int index,