Index: uspace/drv/bus/usb/usbhub/port.c =================================================================== --- uspace/drv/bus/usb/usbhub/port.c (revision c0587d90d69ac91b4844e40a41481db1c0ed24ac) +++ uspace/drv/bus/usb/usbhub/port.c (revision 442fa6be67fbcc7894583a8dbb0b3f31de0ffa8f) @@ -54,9 +54,8 @@ }; -static void usb_hub_removed_device(usb_hub_info_t *hub, size_t port); -static void usb_hub_port_reset_completed(const usb_hub_info_t *hub, - size_t port, usb_port_status_t status); -static int get_port_status(usb_pipe_t *ctrl_pipe, size_t port, - usb_port_status_t *status); +static void usb_hub_port_removed_device(usb_hub_port_t *port); +static void usb_hub_port_reset_completed(usb_hub_port_t *port, + usb_port_status_t status); +static int get_port_status(usb_hub_port_t *port, usb_port_status_t *status); static int enable_port_callback(int port_no, void *arg); static int add_device_phase1_worker_fibril(void *arg); @@ -73,5 +72,5 @@ * @return Operation result */ -int usb_hub_clear_port_feature( +int usb_hub_port_clear_feature( usb_hub_port_t *port, usb_hub_class_feature_t feature) { @@ -97,5 +96,5 @@ * @return Operation result */ -int usb_hub_set_port_feature( +int usb_hub_port_set_feature( usb_hub_port_t *port, usb_hub_class_feature_t feature) { @@ -111,5 +110,15 @@ sizeof(clear_request), NULL, 0); } - +/*----------------------------------------------------------------------------*/ +void usb_hub_port_reset_fail(usb_hub_port_t *port) +{ + assert(port); + fibril_mutex_lock(&port->mutex); + port->reset_completed = true; + port->reset_okay = false; + fibril_condvar_broadcast(&port->reset_cv); + fibril_mutex_unlock(&port->mutex); +} +/*----------------------------------------------------------------------------*/ /** * Process interrupts on given hub port @@ -119,5 +128,5 @@ * @param port port number, starting from 1 */ -void usb_hub_process_port_interrupt(usb_hub_info_t *hub, size_t port) +void usb_hub_port_process_interrupt(usb_hub_info_t *hub, size_t port) { usb_log_debug("Interrupt at port %zu\n", port); @@ -125,5 +134,5 @@ usb_port_status_t status; const int opResult = - get_port_status(&hub->usb_device->ctrl_pipe, port, &status); + get_port_status(&hub->ports[port], &status); if (opResult != EOK) { usb_log_error("Failed to get port %zu status: %s.\n", @@ -140,5 +149,5 @@ /* ACK the change */ const int opResult = - usb_hub_clear_port_feature(&hub->ports[port], + usb_hub_port_clear_feature(&hub->ports[port], USB_HUB_FEATURE_C_PORT_CONNECTION); if (opResult != EOK) { @@ -157,5 +166,5 @@ } } else { - usb_hub_removed_device(hub, port); + usb_hub_port_removed_device(&hub->ports[port]); } } @@ -191,5 +200,5 @@ /* Port reset, set on port reset complete. */ if (status & USB_HUB_PORT_C_STATUS_RESET) { - usb_hub_port_reset_completed(hub, port, status); + usb_hub_port_reset_completed(&hub->ports[port], status); } @@ -206,19 +215,20 @@ * @param port port number, starting from 1 */ -static void usb_hub_removed_device(usb_hub_info_t *hub, size_t port) -{ - /** \TODO remove device from device manager - not yet implemented in - * devide manager - */ - usb_hub_port_t *the_port = hub->ports + port; - - fibril_mutex_lock(&hub->port_mutex); - - if (the_port->attached_device.address >= 0) { +static void usb_hub_port_removed_device(usb_hub_port_t *port) +{ + assert(port); + // TODO remove device from device manager + + + if (port->attached_device.address >= 0) { +#if 0 usb_log_warning("Device unplug on `%s' (port %zu): " \ "not implemented.\n", hub->usb_device->ddf_dev->name, (size_t) port); - the_port->attached_device.address = -1; - the_port->attached_device.handle = 0; +#endif + fibril_mutex_lock(&port->mutex); + port->attached_device.address = -1; + port->attached_device.handle = 0; + fibril_mutex_unlock(&port->mutex); } else { usb_log_warning("Device removed before being registered.\n"); @@ -229,12 +239,6 @@ * port reset callback from new device wrapper. */ - fibril_mutex_lock(&the_port->reset_mutex); - the_port->reset_completed = true; - the_port->reset_okay = false; - fibril_condvar_broadcast(&the_port->reset_cv); - fibril_mutex_unlock(&the_port->reset_mutex); - } - - fibril_mutex_unlock(&hub->port_mutex); + usb_hub_port_reset_fail(port); + } } @@ -248,28 +252,29 @@ * @param status */ -static void usb_hub_port_reset_completed(const usb_hub_info_t *hub, - size_t port, usb_port_status_t status) -{ - usb_hub_port_t *the_port = hub->ports + port; - fibril_mutex_lock(&the_port->reset_mutex); +static void usb_hub_port_reset_completed(usb_hub_port_t *port, + usb_port_status_t status) +{ + assert(port); + fibril_mutex_lock(&port->mutex); /* Finalize device adding. */ - the_port->reset_completed = true; - the_port->reset_okay = (status & USB_HUB_PORT_STATUS_ENABLED) != 0; - - if (the_port->reset_okay) { - usb_log_debug("Port %zu reset complete.\n", port); + port->reset_completed = true; + port->reset_okay = (status & USB_HUB_PORT_STATUS_ENABLED) != 0; + + if (port->reset_okay) { + usb_log_debug("Port %zu reset complete.\n", port->port_number); } else { usb_log_warning( - "Port %zu reset complete but port not enabled.\n", port); - } - fibril_condvar_broadcast(&the_port->reset_cv); - fibril_mutex_unlock(&the_port->reset_mutex); + "Port %zu reset complete but port not enabled.\n", + port->port_number); + } + fibril_condvar_broadcast(&port->reset_cv); + fibril_mutex_unlock(&port->mutex); /* Clear the port reset change. */ - int rc = usb_hub_clear_port_feature(&hub->ports[port], - USB_HUB_FEATURE_C_PORT_RESET); + int rc = usb_hub_port_clear_feature(port, USB_HUB_FEATURE_C_PORT_RESET); if (rc != EOK) { - usb_log_error("Failed to clear port %d reset feature: %s.\n", - port, str_error(rc)); + usb_log_error( + "Failed to clear port %d reset change feature: %s.\n", + port->port_number, str_error(rc)); } } @@ -282,9 +287,7 @@ * @return Error code. */ -static int get_port_status(usb_pipe_t *ctrl_pipe, size_t port, - usb_port_status_t *status) -{ - size_t recv_size; - usb_port_status_t status_tmp; +static int get_port_status(usb_hub_port_t *port, usb_port_status_t *status) +{ + assert(port); /* USB hub specific GET_PORT_STATUS request. See USB Spec 11.16.2.6 * Generic GET_STATUS request cannot be used because of the difference @@ -294,9 +297,11 @@ .request = USB_HUB_REQUEST_GET_STATUS, .value = 0, - .index = port, + .index = port->port_number, .length = sizeof(usb_port_status_t), }; - - const int rc = usb_pipe_control_read(ctrl_pipe, + size_t recv_size; + usb_port_status_t status_tmp; + + const int rc = usb_pipe_control_read(port->control_pipe, &request, sizeof(usb_device_request_setup_packet_t), &status_tmp, sizeof(status_tmp), &recv_size); @@ -329,5 +334,5 @@ usb_hub_port_t *port = arg; const int rc = - usb_hub_set_port_feature(port, USB_HUB_FEATURE_PORT_RESET); + usb_hub_port_set_feature(port, USB_HUB_FEATURE_PORT_RESET); if (rc != EOK) { usb_log_warning("Port reset failed: %s.\n", str_error(rc)); @@ -338,9 +343,9 @@ * Wait until reset completes. */ - fibril_mutex_lock(&port->reset_mutex); + fibril_mutex_lock(&port->mutex); while (!port->reset_completed) { - fibril_condvar_wait(&port->reset_cv, &port->reset_mutex); - } - fibril_mutex_unlock(&port->reset_mutex); + fibril_condvar_wait(&port->reset_cv, &port->mutex); + } + fibril_mutex_unlock(&port->mutex); if (port->reset_okay) { @@ -380,8 +385,8 @@ } - fibril_mutex_lock(&data->hub->port_mutex); + fibril_mutex_lock(&data->hub->ports[data->port].mutex); data->hub->ports[data->port].attached_device.handle = child_handle; data->hub->ports[data->port].attached_device.address = new_address; - fibril_mutex_unlock(&data->hub->port_mutex); + fibril_mutex_unlock(&data->hub->ports[data->port].mutex); usb_log_info("Detected new device on `%s' (port %zu), " @@ -426,7 +431,7 @@ usb_hub_port_t *the_port = hub->ports + port; - fibril_mutex_lock(&the_port->reset_mutex); + fibril_mutex_lock(&the_port->mutex); the_port->reset_completed = false; - fibril_mutex_unlock(&the_port->reset_mutex); + fibril_mutex_unlock(&the_port->mutex); fid_t fibril = fibril_create(add_device_phase1_worker_fibril, data); Index: uspace/drv/bus/usb/usbhub/port.h =================================================================== --- uspace/drv/bus/usb/usbhub/port.h (revision c0587d90d69ac91b4844e40a41481db1c0ed24ac) +++ uspace/drv/bus/usb/usbhub/port.h (revision 442fa6be67fbcc7894583a8dbb0b3f31de0ffa8f) @@ -46,6 +46,6 @@ size_t port_number; usb_pipe_t *control_pipe; - /** Mutex needed by CV for checking port reset. */ - fibril_mutex_t reset_mutex; + /** Mutex needed not only by CV for checking port reset. */ + fibril_mutex_t mutex; /** CV for waiting to port reset completion. */ fibril_condvar_t reset_cv; @@ -73,13 +73,14 @@ port->port_number = port_number; port->control_pipe = control_pipe; - fibril_mutex_initialize(&port->reset_mutex); + fibril_mutex_initialize(&port->mutex); fibril_condvar_initialize(&port->reset_cv); } +void usb_hub_port_reset_fail(usb_hub_port_t *port); -void usb_hub_process_port_interrupt(usb_hub_info_t *hub, size_t port); -int usb_hub_clear_port_feature( +void usb_hub_port_process_interrupt(usb_hub_info_t *hub, size_t port); +int usb_hub_port_clear_feature( usb_hub_port_t *port, usb_hub_class_feature_t feature); -int usb_hub_set_port_feature( +int usb_hub_port_set_feature( usb_hub_port_t *port, usb_hub_class_feature_t feature); Index: uspace/drv/bus/usb/usbhub/usbhub.c =================================================================== --- uspace/drv/bus/usb/usbhub/usbhub.c (revision c0587d90d69ac91b4844e40a41481db1c0ed24ac) +++ uspace/drv/bus/usb/usbhub/usbhub.c (revision 442fa6be67fbcc7894583a8dbb0b3f31de0ffa8f) @@ -187,5 +187,5 @@ const bool change = (change_bitmap[port / 8] >> (port % 8)) & 1; if (change) { - usb_hub_process_port_interrupt(hub, port); + usb_hub_port_process_interrupt(hub, port); } } @@ -217,5 +217,4 @@ info->ports = NULL; info->port_count = -1; - fibril_mutex_initialize(&info->port_mutex); fibril_mutex_initialize(&info->pending_ops_mutex); fibril_condvar_initialize(&info->pending_ops_cv); @@ -289,5 +288,5 @@ for (port = 1; port <= hub_info->port_count; ++port) { usb_log_debug("Powering port %zu.\n", port); - opResult = usb_hub_set_port_feature( + opResult = usb_hub_port_set_feature( &hub_info->ports[port], USB_HUB_FEATURE_PORT_POWER); if (opResult != EOK) { @@ -365,7 +364,9 @@ * switch them all off to prevent damage. */ //TODO Consider ganged power switching here. + //TODO Hub should have turned the ports off already, + //this is redundant. size_t port; for (port = 1; port <= hub_info->port_count; ++port) { - const int opResult = usb_hub_clear_port_feature( + const int opResult = usb_hub_port_clear_feature( &hub_info->ports[port], USB_HUB_FEATURE_PORT_POWER); if (opResult != EOK) { @@ -381,5 +382,5 @@ size_t port; for (port = 1; port <= hub_info->port_count; ++port) { - const int opResult = usb_hub_set_port_feature( + const int opResult = usb_hub_port_set_feature( &hub_info->ports[port], USB_HUB_FEATURE_PORT_POWER); if (opResult != EOK) { @@ -478,15 +479,8 @@ */ if (hub->pending_ops_count > 0) { - fibril_mutex_lock(&hub->port_mutex); size_t port; for (port = 0; port < hub->port_count; port++) { - usb_hub_port_t *the_port = hub->ports + port; - fibril_mutex_lock(&the_port->reset_mutex); - the_port->reset_completed = true; - the_port->reset_okay = false; - fibril_condvar_broadcast(&the_port->reset_cv); - fibril_mutex_unlock(&the_port->reset_mutex); + usb_hub_port_reset_fail(&hub->ports[port]); } - fibril_mutex_unlock(&hub->port_mutex); } /* And now wait for them. */ Index: uspace/drv/bus/usb/usbhub/usbhub.h =================================================================== --- uspace/drv/bus/usb/usbhub/usbhub.h (revision c0587d90d69ac91b4844e40a41481db1c0ed24ac) +++ uspace/drv/bus/usb/usbhub/usbhub.h (revision 442fa6be67fbcc7894583a8dbb0b3f31de0ffa8f) @@ -59,6 +59,4 @@ usb_hub_port_t *ports; - fibril_mutex_t port_mutex; - /** Connection to hcd */ usb_hc_connection_t connection;