Skip to content

Commit 50cc647

Browse files
tore-espressiftore-espressif
committed
review changes
1 parent 82c46ae commit 50cc647

File tree

2 files changed

+41
-20
lines changed

2 files changed

+41
-20
lines changed

‎host/usb/src/hub.c‎

Lines changed: 36 additions & 18 deletions
Original file line numberDiff line numberDiff line change
@@ -26,8 +26,7 @@ typedef struct ext_hub_s *ext_hub_handle_t;
2626
#endif // ENABLE_USB_HUBS
2727

2828
/*
29-
Implementation of the HUB driver that only supports the Root Hub with a single port. Therefore, we currently don't
30-
implement the bare minimum to control the root HCD port.
29+
Implementation of the HUB driver that supports Root HUB with 2 Root Ports.
3130
*/
3231

3332
#ifdef CONFIG_USB_HOST_HW_BUFFER_BIAS_IN
@@ -48,10 +47,12 @@ implement the bare minimum to control the root HCD port.
4847
*/
4948
typedef enum {
5049
HUB_DRIVER_ACTION_NONE = 0,
51-
HUB_DRIVER_ACTION_ROOT_EVENT = BIT0,
52-
HUB_DRIVER_ACTION_ROOT_REQ = BIT1,
50+
HUB_DRIVER_ACTION_ROOT0_EVENT = BIT0,
51+
HUB_DRIVER_ACTION_ROOT0_REQ = BIT1,
52+
#if HCD_NUM_PORTS > 1
5353
HUB_DRIVER_ACTION_ROOT1_EVENT = BIT2,
5454
HUB_DRIVER_ACTION_ROOT1_REQ = BIT3,
55+
#endif // HCD_NUM_PORTS > 1
5556
#if ENABLE_USB_HUBS
5657
HUB_DRIVER_ACTION_EXT_HUB = BIT6,
5758
HUB_DRIVER_ACTION_EXT_PORT = BIT7
@@ -190,7 +191,7 @@ static esp_err_t dev_tree_node_new(ext_hub_handle_t parent, uint8_t port_num, us
190191
esp_err_t ret;
191192

192193
// Get root port handle of the new device
193-
hcd_port_handle_t root_port_hdl;
194+
hcd_port_handle_t root_port_hdl = NULL;
194195
if (parent == NULL) {
195196
assert(port_num < HCD_NUM_PORTS);
196197
root_port_hdl = p_hub_driver_obj->root_hub_ports[port_num].constant.hdl;
@@ -353,9 +354,13 @@ static bool root_port_callback(hcd_port_handle_t port_hdl, hcd_port_event_t port
353354
root_hub_port_t *root_hub_port = (root_hub_port_t *)user_arg;
354355
HUB_DRIVER_ENTER_CRITICAL_SAFE();
355356
if (root_hub_port->constant.index == 0) {
356-
p_hub_driver_obj->dynamic.flags.actions |= HUB_DRIVER_ACTION_ROOT_EVENT;
357+
p_hub_driver_obj->dynamic.flags.actions |= HUB_DRIVER_ACTION_ROOT0_REQ;
357358
} else {
358-
p_hub_driver_obj->dynamic.flags.actions |= HUB_DRIVER_ACTION_ROOT1_EVENT;
359+
#if HCD_NUM_PORTS > 1
360+
p_hub_driver_obj->dynamic.flags.actions |= HUB_DRIVER_ACTION_ROOT1_REQ;
361+
#else
362+
abort(); // Should never occur
363+
#endif // HCD_NUM_PORTS > 1
359364
}
360365
HUB_DRIVER_EXIT_CRITICAL_SAFE();
361366
assert(in_isr); // Currently, this callback should only ever be called from an ISR context
@@ -476,9 +481,13 @@ static void root_port_handle_events(root_hub_port_t *root_hub_port)
476481
// Therefore, there's no device object to clean up, and we can go straight to port recovery
477482
root_hub_port->dynamic.reqs |= PORT_REQ_RECOVER;
478483
if (root_hub_port->constant.index == 0) {
479-
p_hub_driver_obj->dynamic.flags.actions |= HUB_DRIVER_ACTION_ROOT_REQ;
484+
p_hub_driver_obj->dynamic.flags.actions |= HUB_DRIVER_ACTION_ROOT0_REQ;
480485
} else {
486+
#if HCD_NUM_PORTS > 1
481487
p_hub_driver_obj->dynamic.flags.actions |= HUB_DRIVER_ACTION_ROOT1_REQ;
488+
#else
489+
abort(); // Should never occur
490+
#endif // HCD_NUM_PORTS > 1
482491
}
483492
break;
484493
case ROOT_PORT_STATE_NOT_POWERED: // The user turned off ports' power. Indicate to USBH that the device is gone
@@ -522,12 +531,12 @@ static void root_port_req(root_hub_port_t *root_hub_port)
522531
HUB_DRIVER_EXIT_CRITICAL();
523532

524533
if (port_reqs & PORT_REQ_DISABLE) {
525-
ESP_LOGD(HUB_DRIVER_TAG, "Disabling root port");
534+
ESP_LOGD(HUB_DRIVER_TAG, "Disabling root port %d", root_hub_port->constant.index);
526535
// We allow this to fail in case a disconnect/port error happens while disabling.
527536
hcd_port_command(root_port_hdl, HCD_PORT_CMD_DISABLE);
528537
}
529538
if (port_reqs & PORT_REQ_RECOVER) {
530-
ESP_LOGD(HUB_DRIVER_TAG, "Recovering root port");
539+
ESP_LOGD(HUB_DRIVER_TAG, "Recovering root port %d", root_hub_port->constant.index);
531540
ESP_ERROR_CHECK(hcd_port_recover(root_port_hdl));
532541

533542
// In case the port's power was turned off with usb_host_lib_set_root_port_power(false)
@@ -544,7 +553,7 @@ static void root_port_req(root_hub_port_t *root_hub_port)
544553
}
545554
}
546555
if (port_reqs & PORT_REQ_SUSPEND) {
547-
ESP_LOGD(HUB_DRIVER_TAG, "Suspending the root port");
556+
ESP_LOGD(HUB_DRIVER_TAG, "Suspending root port %d", root_hub_port->constant.index);
548557

549558
HUB_DRIVER_ENTER_CRITICAL();
550559
const root_port_state_t root_state = root_hub_port->dynamic.state;
@@ -581,7 +590,7 @@ static void root_port_req(root_hub_port_t *root_hub_port)
581590
usbh_devs_set_pm_actions_all(USBH_DEV_SUSPEND_EVT);
582591
}
583592
if (port_reqs & PORT_REQ_RESUME) {
584-
ESP_LOGD(HUB_DRIVER_TAG, "Resuming the root port");
593+
ESP_LOGD(HUB_DRIVER_TAG, "Resuming root port %d", root_hub_port->constant.index);
585594

586595
HUB_DRIVER_ENTER_CRITICAL();
587596
const root_port_state_t root_state = root_hub_port->dynamic.state;
@@ -638,9 +647,13 @@ static esp_err_t root_port_recycle(root_hub_port_t *root_hub_port)
638647
break;
639648
}
640649
if (root_hub_port->constant.index == 0) {
641-
p_hub_driver_obj->dynamic.flags.actions |= HUB_DRIVER_ACTION_ROOT_REQ;
650+
p_hub_driver_obj->dynamic.flags.actions |= HUB_DRIVER_ACTION_ROOT0_REQ;
642651
} else {
652+
#if HCD_NUM_PORTS > 1
643653
p_hub_driver_obj->dynamic.flags.actions |= HUB_DRIVER_ACTION_ROOT1_REQ;
654+
#else
655+
abort(); // Should never occur
656+
#endif // HCD_NUM_PORTS > 1
644657
}
645658
HUB_DRIVER_EXIT_CRITICAL();
646659

@@ -657,9 +670,13 @@ static esp_err_t root_port_disable(root_hub_port_t *root_hub_port)
657670
HUB_DRIVER_ENTER_CRITICAL();
658671
root_hub_port->dynamic.reqs |= PORT_REQ_DISABLE;
659672
if (root_hub_port->constant.index == 0) {
660-
p_hub_driver_obj->dynamic.flags.actions |= HUB_DRIVER_ACTION_ROOT_REQ;
673+
p_hub_driver_obj->dynamic.flags.actions |= HUB_DRIVER_ACTION_ROOT0_REQ;
661674
} else {
675+
#if HCD_NUM_PORTS > 1
662676
p_hub_driver_obj->dynamic.flags.actions |= HUB_DRIVER_ACTION_ROOT1_REQ;
677+
#else
678+
abort(); // Should never occur
679+
#endif // HCD_NUM_PORTS > 1
663680
}
664681
HUB_DRIVER_EXIT_CRITICAL();
665682

@@ -816,6 +833,7 @@ esp_err_t hub_root_start(void)
816833
}
817834
const esp_err_t port_ret = hcd_port_command(root_port_hdl, HCD_PORT_CMD_POWER_ON);
818835
if (port_ret != ESP_OK) {
836+
ESP_LOGD(HUB_DRIVER_TAG, "Root port %d power on failed", i);
819837
ret = port_ret;
820838
goto fail;
821839
}
@@ -959,7 +977,7 @@ esp_err_t hub_root_mark_suspend(void)
959977
// TODO: Suspend/Resume API is currently available only for single host configuration.
960978
// Pick the first root port that is enabled
961979
root_hub_port_t *root_hub_port = &p_hub_driver_obj->root_hub_ports[0];
962-
hub_flag_action_t action = HUB_DRIVER_ACTION_ROOT_REQ;
980+
hub_flag_action_t action = HUB_DRIVER_ACTION_ROOT0_REQ;
963981
#if HCD_NUM_PORTS > 1
964982
if (!root_hub_port->constant.hdl) {
965983
root_hub_port = &p_hub_driver_obj->root_hub_ports[1];
@@ -987,7 +1005,7 @@ esp_err_t hub_root_mark_resume(void)
9871005
// TODO: Suspend/Resume API is currently available only for single host configuration.
9881006
// Pick the first root port that is enabled
9891007
root_hub_port_t *root_hub_port = &p_hub_driver_obj->root_hub_ports[0];
990-
hub_flag_action_t action = HUB_DRIVER_ACTION_ROOT_REQ;
1008+
hub_flag_action_t action = HUB_DRIVER_ACTION_ROOT0_REQ;
9911009
#if HCD_NUM_PORTS > 1
9921010
if (!root_hub_port->constant.hdl) {
9931011
root_hub_port = &p_hub_driver_obj->root_hub_ports[1];
@@ -1202,10 +1220,10 @@ esp_err_t hub_process(void)
12021220
ESP_ERROR_CHECK(ext_hub_process());
12031221
}
12041222
#endif // ENABLE_USB_HUBS
1205-
if (action_flags & HUB_DRIVER_ACTION_ROOT_EVENT) {
1223+
if (action_flags & HUB_DRIVER_ACTION_ROOT0_EVENT) {
12061224
root_port_handle_events(&p_hub_driver_obj->root_hub_ports[0]);
12071225
}
1208-
if (action_flags & HUB_DRIVER_ACTION_ROOT_REQ) {
1226+
if (action_flags & HUB_DRIVER_ACTION_ROOT0_REQ) {
12091227
root_port_req(&p_hub_driver_obj->root_hub_ports[0]);
12101228
}
12111229
#if HCD_NUM_PORTS > 1

‎host/usb/src/usbh.c‎

Lines changed: 5 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -162,7 +162,7 @@ static device_t *_find_dev_from_uid(unsigned int uid)
162162
*/
163163
device_t *dev_iter;
164164

165-
// Search the device lists for a device with the specified address
165+
// Search the device lists for a device with the specified uid
166166
TAILQ_FOREACH(dev_iter, &p_usbh_obj->dynamic.devs_idle_tailq, dynamic.tailq_entry) {
167167
if (dev_iter->constant.uid == uid) {
168168
return dev_iter;
@@ -1132,8 +1132,11 @@ void usbh_devs_set_pm_actions_all(usbh_dev_ctrl_t device_ctrl)
11321132
}
11331133
}
11341134

1135-
esp_err_t _dev_open(device_t *dev_obj)
1135+
static esp_err_t _dev_open(device_t *dev_obj)
11361136
{
1137+
/*
1138+
THIS FUNCTION MUST BE CALLED FROM A CRITICAL SECTION
1139+
*/
11371140
esp_err_t ret;
11381141
if (dev_obj != NULL) {
11391142
// Check if the device is in a state to be opened

0 commit comments

Comments
 (0)