From 2fb4cf92737dae0fbcea90939e4af5b534eb35d6 Mon Sep 17 00:00:00 2001 From: Diego Ismirlian Date: Mon, 31 Jul 2017 18:38:46 -0300 Subject: USBH: improved main driver - improved connection/disconnection handling - corrected bug on aborting non-pending URB - corrected bug on disconnecting multiple hubs - improved debug messages --- os/hal/src/hal_usbh.c | 105 ++++++++++++++++++++++++++++---------------------- 1 file changed, 58 insertions(+), 47 deletions(-) diff --git a/os/hal/src/hal_usbh.c b/os/hal/src/hal_usbh.c index 1aa35d0..a7ba236 100644 --- a/os/hal/src/hal_usbh.c +++ b/os/hal/src/hal_usbh.c @@ -249,24 +249,17 @@ void usbhURBSubmitI(usbh_urb_t *urb) { bool _usbh_urb_abortI(usbh_urb_t *urb, usbh_urbstatus_t status) { osalDbgCheckClassI(); _check_urb(urb); + osalDbgCheck(urb->status != USBH_URBSTATUS_UNINITIALIZED); - switch (urb->status) { -/* case USBH_URBSTATUS_UNINITIALIZED: - * case USBH_URBSTATUS_INITIALIZED: - * case USBH_URBSTATUS_ERROR: - * case USBH_URBSTATUS_TIMEOUT: - * case USBH_URBSTATUS_CANCELLED: - * case USBH_URBSTATUS_STALL: - * case USBH_URBSTATUS_DISCONNECTED: - * case USBH_URBSTATUS_OK: */ - default: - /* already finished */ - _usbh_urb_completeI(urb, status); - return TRUE; - - case USBH_URBSTATUS_PENDING: + if (urb->status == USBH_URBSTATUS_PENDING) { return usbh_lld_urb_abort(urb, status); } + + /* already finished or never submitted: + * USBH_URBSTATUS_INITIALIZED, USBH_URBSTATUS_ERROR, USBH_URBSTATUS_TIMEOUT, + * USBH_URBSTATUS_CANCELLED, USBH_URBSTATUS_STALL, USBH_URBSTATUS_DISCONNECTED + * USBH_URBSTATUS_OK */ + return TRUE; } void _usbh_urb_abort_and_waitS(usbh_urb_t *urb, usbh_urbstatus_t status) { @@ -430,7 +423,7 @@ usbh_urbstatus_t usbhControlRequest(usbh_device_t *dev, wIndex, wLength }; - return usbhControlRequestExtended(dev, &req, buff, NULL, TIME_INFINITE); + return usbhControlRequestExtended(dev, &req, buff, NULL, HAL_USBH_CONTROL_REQUEST_DEFAULT_TIMEOUT); } /*===========================================================================*/ @@ -896,46 +889,48 @@ static void _port_process_status_change(usbh_port_t *port) { _port_update_status(port); if (port->c_status & USBH_PORTSTATUS_C_CONNECTION) { - /* port connected status changed */ port->c_status &= ~USBH_PORTSTATUS_C_CONNECTION; usbhhubClearFeaturePort(port, USBH_PORT_FEAT_C_CONNECTION); - if ((port->status & (USBH_PORTSTATUS_CONNECTION | USBH_PORTSTATUS_ENABLE)) - == USBH_PORTSTATUS_CONNECTION) { - if (port->device.status != USBH_DEVSTATUS_DISCONNECTED) { + + if (port->device.status != USBH_DEVSTATUS_DISCONNECTED) { + if (!(port->status & USBH_PORTSTATUS_CONNECTION)) { _usbh_port_disconnected(port); } + } + } - /* connected, disabled */ + if (port->device.status == USBH_DEVSTATUS_DISCONNECTED) { + if (port->status & USBH_PORTSTATUS_CONNECTION) { _port_connected(port); - } else { - /* disconnected */ - _usbh_port_disconnected(port); } } if (port->c_status & USBH_PORTSTATUS_C_RESET) { port->c_status &= ~USBH_PORTSTATUS_C_RESET; usbhhubClearFeaturePort(port, USBH_PORT_FEAT_C_RESET); + uinfof("Port %d: reset=%d", port->number, port->status & USBH_PORTSTATUS_RESET ? 1 : 0); } if (port->c_status & USBH_PORTSTATUS_C_ENABLE) { port->c_status &= ~USBH_PORTSTATUS_C_ENABLE; usbhhubClearFeaturePort(port, USBH_PORT_FEAT_C_ENABLE); + uinfof("Port %d: enable=%d", port->number, port->status & USBH_PORTSTATUS_ENABLE ? 1 : 0); } if (port->c_status & USBH_PORTSTATUS_C_OVERCURRENT) { port->c_status &= ~USBH_PORTSTATUS_C_OVERCURRENT; usbhhubClearFeaturePort(port, USBH_PORT_FEAT_C_OVERCURRENT); + uwarnf("Port %d: overcurrent=%d", port->number, port->status & USBH_PORTSTATUS_OVERCURRENT ? 1 : 0); } if (port->c_status & USBH_PORTSTATUS_C_SUSPEND) { port->c_status &= ~USBH_PORTSTATUS_C_SUSPEND; usbhhubClearFeaturePort(port, USBH_PORT_FEAT_C_SUSPEND); + uinfof("Port %d: suspend=%d", port->number, port->status & USBH_PORTSTATUS_SUSPEND ? 1 : 0); } } - static void _port_connected(usbh_port_t *port) { /* connected */ @@ -945,9 +940,8 @@ static void _port_connected(usbh_port_t *port) { usbh_devspeed_t speed; USBH_DEFINE_BUFFER(usbh_string_descriptor_t strdesc); - uinfof("Port %d connected, wait debounce...", port->number); - port->device.status = USBH_DEVSTATUS_ATTACHED; + uinfof("Port %d: attached, wait debounce...", port->number); /* wait for attach de-bounce */ osalThreadSleepMilliseconds(HAL_USBH_PORT_DEBOUNCE_TIME); @@ -955,16 +949,26 @@ static void _port_connected(usbh_port_t *port) { /* check disconnection */ _port_update_status(port); if (port->c_status & USBH_PORTSTATUS_C_CONNECTION) { - /* connection state changed; abort */ + port->c_status &= ~USBH_PORTSTATUS_C_CONNECTION; + usbhhubClearFeaturePort(port, USBH_PORT_FEAT_C_CONNECTION); + uwarnf("Port %d: connection state changed; abort #1", port->number); + goto abort; + } + + /* make sure that the device is still connected */ + if ((port->status & USBH_PORTSTATUS_CONNECTION) == 0) { + uwarnf("Port %d: device is disconnected", port->number); goto abort; } + uinfof("Port %d: connected", port->number); port->device.status = USBH_DEVSTATUS_CONNECTED; retries = 3; reset: for (i = 0; i < 3; i++) { - uinfo("Try reset..."); + uinfof("Port %d: Try reset...", port->number); + /* TODO: check that port is actually disabled */ port->c_status &= ~(USBH_PORTSTATUS_C_RESET | USBH_PORTSTATUS_C_ENABLE); _port_reset(port); osalThreadSleepMilliseconds(20); /* give it some time to reset (min. 10ms) */ @@ -973,8 +977,12 @@ reset: _port_update_status(port); /* check for disconnection */ - if (port->c_status & USBH_PORTSTATUS_C_CONNECTION) + if (port->c_status & USBH_PORTSTATUS_C_CONNECTION) { + port->c_status &= ~USBH_PORTSTATUS_C_CONNECTION; + usbhhubClearFeaturePort(port, USBH_PORT_FEAT_C_CONNECTION); + uwarnf("Port %d: connection state changed; abort #2", port->number); goto abort; + } /* check for reset completion */ if (port->c_status & USBH_PORTSTATUS_C_RESET) { @@ -988,7 +996,10 @@ reset: } /* check for timeout */ - if (osalOsGetSystemTimeX() - start > HAL_USBH_PORT_RESET_TIMEOUT) break; + if (osalOsGetSystemTimeX() - start > HAL_USBH_PORT_RESET_TIMEOUT) { + uwarnf("Port %d: reset timeout", port->number); + break; + } } } @@ -996,8 +1007,7 @@ reset: goto abort; reset_success: - - uinfo("Reset OK, recovery..."); + uinfof("Port %d: Reset OK, recovery...", port->number); /* reset recovery */ osalThreadSleepMilliseconds(100); @@ -1014,19 +1024,22 @@ reset_success: usbhEPOpen(&port->device.ctrl); /* device with default address (0), try enumeration */ - if (_device_enumerate(&port->device)) { + if (_device_enumerate(&port->device) != HAL_SUCCESS) { /* enumeration failed */ usbhEPClose(&port->device.ctrl); - if (!--retries) + if (!--retries) { + uwarnf("Port %d: enumeration failed; abort", port->number); goto abort; + } /* retry reset & enumeration */ + uwarnf("Port %d: enumeration failed; retry reset & enumeration", port->number); goto reset; } /* load the default language ID */ - uinfo("Loading langID0..."); + uinfof("Port %d: Loading langID0...", port->number); if (!usbhStdReqGetStringDescriptor(&port->device, 0, 0, USBH_DT_STRING_SIZE, (uint8_t *)&strdesc) && (strdesc.bLength >= 4) @@ -1034,12 +1047,12 @@ reset_success: 4, (uint8_t *)&strdesc)) { port->device.langID0 = strdesc.wData[0]; - uinfof("langID0=%04x", port->device.langID0); + uinfof("Port %d: langID0=%04x", port->number, port->device.langID0); } /* check if the device has only one configuration */ if (port->device.devDesc.bNumConfigurations == 1) { - uinfo("Device has only one configuration"); + uinfof("Port %d: device has only one configuration", port->number); _device_configure(&port->device, 0); } @@ -1047,7 +1060,7 @@ reset_success: return; abort: - uerr("Abort"); + uerrf("Port %d: abort", port->number); port->device.status = USBH_DEVSTATUS_DISCONNECTED; } @@ -1055,14 +1068,14 @@ void _usbh_port_disconnected(usbh_port_t *port) { if (port->device.status == USBH_DEVSTATUS_DISCONNECTED) return; - uinfo("Port disconnected"); + uinfof("Port %d: disconnected", port->number); /* unload drivers */ while (port->device.drivers) { usbh_baseclassdriver_t *drv = port->device.drivers; /* unload */ - uinfof("Unload driver %s", drv->info->name); + uinfof("Port %d: unload driver %s", port->number, drv->info->name); drv->info->vmt->unload(drv); /* unlink */ @@ -1071,9 +1084,7 @@ void _usbh_port_disconnected(usbh_port_t *port) { } /* close control endpoint */ - osalSysLock(); - usbhEPCloseS(&port->device.ctrl); - osalSysUnlock(); + usbhEPClose(&port->device.ctrl); /* free address */ if (port->device.address) @@ -1113,7 +1124,7 @@ static void _hub_process_status_change(USBHDriver *host, USBHHubDriver *hub) { uinfo("Hub status change. GET_STATUS."); _hub_update_status(host, hub); - if (hub->c_status & USBH_HUBSTATUS_C_HUB_LOCAL_POWER) { + if (hub->c_status & USBH_HUBSTATUS_C_HUB_LOCAL_POWER) { hub->c_status &= ~USBH_HUBSTATUS_C_HUB_LOCAL_POWER; uinfo("Clear USBH_HUB_FEAT_C_HUB_LOCAL_POWER"); usbhhubClearFeatureHub(host, hub, USBH_HUB_FEAT_C_HUB_LOCAL_POWER); @@ -1196,8 +1207,8 @@ void usbhMainLoop(USBHDriver *usbh) { _hub_process(usbh, NULL); /* process connected hubs */ - USBHHubDriver *hub; - list_for_each_entry(hub, USBHHubDriver, &usbh->hubs, node) { + USBHHubDriver *hub, *temp; + list_for_each_entry_safe(hub, USBHHubDriver, temp, &usbh->hubs, node) { _hub_process(usbh, hub); } #else -- cgit v1.2.3