Skip to content

Commit 72a169b

Browse files
tore-espressiftore-espressif
committed
review changes
1 parent 82c46ae commit 72a169b

File tree

2 files changed

+36
-27
lines changed

2 files changed

+36
-27
lines changed

‎host/usb/src/hub.c‎

Lines changed: 31 additions & 25 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;
@@ -352,11 +353,12 @@ static bool root_port_callback(hcd_port_handle_t port_hdl, hcd_port_event_t port
352353
{
353354
root_hub_port_t *root_hub_port = (root_hub_port_t *)user_arg;
354355
HUB_DRIVER_ENTER_CRITICAL_SAFE();
355-
if (root_hub_port->constant.index == 0) {
356-
p_hub_driver_obj->dynamic.flags.actions |= HUB_DRIVER_ACTION_ROOT_EVENT;
357-
} else {
356+
p_hub_driver_obj->dynamic.flags.actions |= HUB_DRIVER_ACTION_ROOT0_EVENT;
357+
#if HCD_NUM_PORTS > 1
358+
if (root_hub_port->constant.index != 0) {
358359
p_hub_driver_obj->dynamic.flags.actions |= HUB_DRIVER_ACTION_ROOT1_EVENT;
359360
}
361+
#endif // HCD_NUM_PORTS > 1
360362
HUB_DRIVER_EXIT_CRITICAL_SAFE();
361363
assert(in_isr); // Currently, this callback should only ever be called from an ISR context
362364
return p_hub_driver_obj->constant.proc_req_cb(USB_PROC_REQ_SOURCE_HUB, in_isr, p_hub_driver_obj->constant.proc_req_cb_arg);
@@ -475,11 +477,12 @@ static void root_port_handle_events(root_hub_port_t *root_hub_port)
475477
case ROOT_PORT_STATE_DISABLED: // This occurred after the device has already been disabled
476478
// Therefore, there's no device object to clean up, and we can go straight to port recovery
477479
root_hub_port->dynamic.reqs |= PORT_REQ_RECOVER;
478-
if (root_hub_port->constant.index == 0) {
479-
p_hub_driver_obj->dynamic.flags.actions |= HUB_DRIVER_ACTION_ROOT_REQ;
480-
} else {
480+
p_hub_driver_obj->dynamic.flags.actions |= HUB_DRIVER_ACTION_ROOT0_REQ;
481+
#if HCD_NUM_PORTS > 1
482+
if (root_hub_port->constant.index != 0) {
481483
p_hub_driver_obj->dynamic.flags.actions |= HUB_DRIVER_ACTION_ROOT1_REQ;
482484
}
485+
#endif // HCD_NUM_PORTS > 1
483486
break;
484487
case ROOT_PORT_STATE_NOT_POWERED: // The user turned off ports' power. Indicate to USBH that the device is gone
485488
case ROOT_PORT_STATE_ENABLED: // There is an enabled (active) device. Indicate to USBH that the device is gone
@@ -522,12 +525,12 @@ static void root_port_req(root_hub_port_t *root_hub_port)
522525
HUB_DRIVER_EXIT_CRITICAL();
523526

524527
if (port_reqs & PORT_REQ_DISABLE) {
525-
ESP_LOGD(HUB_DRIVER_TAG, "Disabling root port");
528+
ESP_LOGD(HUB_DRIVER_TAG, "Disabling root port %d", root_hub_port->constant.index);
526529
// We allow this to fail in case a disconnect/port error happens while disabling.
527530
hcd_port_command(root_port_hdl, HCD_PORT_CMD_DISABLE);
528531
}
529532
if (port_reqs & PORT_REQ_RECOVER) {
530-
ESP_LOGD(HUB_DRIVER_TAG, "Recovering root port");
533+
ESP_LOGD(HUB_DRIVER_TAG, "Recovering root port %d", root_hub_port->constant.index);
531534
ESP_ERROR_CHECK(hcd_port_recover(root_port_hdl));
532535

533536
// In case the port's power was turned off with usb_host_lib_set_root_port_power(false)
@@ -544,7 +547,7 @@ static void root_port_req(root_hub_port_t *root_hub_port)
544547
}
545548
}
546549
if (port_reqs & PORT_REQ_SUSPEND) {
547-
ESP_LOGD(HUB_DRIVER_TAG, "Suspending the root port");
550+
ESP_LOGD(HUB_DRIVER_TAG, "Suspending root port %d", root_hub_port->constant.index);
548551

549552
HUB_DRIVER_ENTER_CRITICAL();
550553
const root_port_state_t root_state = root_hub_port->dynamic.state;
@@ -581,7 +584,7 @@ static void root_port_req(root_hub_port_t *root_hub_port)
581584
usbh_devs_set_pm_actions_all(USBH_DEV_SUSPEND_EVT);
582585
}
583586
if (port_reqs & PORT_REQ_RESUME) {
584-
ESP_LOGD(HUB_DRIVER_TAG, "Resuming the root port");
587+
ESP_LOGD(HUB_DRIVER_TAG, "Resuming root port %d", root_hub_port->constant.index);
585588

586589
HUB_DRIVER_ENTER_CRITICAL();
587590
const root_port_state_t root_state = root_hub_port->dynamic.state;
@@ -637,11 +640,12 @@ static esp_err_t root_port_recycle(root_hub_port_t *root_hub_port)
637640
abort(); // Should never occur
638641
break;
639642
}
640-
if (root_hub_port->constant.index == 0) {
641-
p_hub_driver_obj->dynamic.flags.actions |= HUB_DRIVER_ACTION_ROOT_REQ;
642-
} else {
643+
p_hub_driver_obj->dynamic.flags.actions |= HUB_DRIVER_ACTION_ROOT0_REQ;
644+
#if HCD_NUM_PORTS > 1
645+
if (root_hub_port->constant.index != 0) {
643646
p_hub_driver_obj->dynamic.flags.actions |= HUB_DRIVER_ACTION_ROOT1_REQ;
644647
}
648+
#endif // HCD_NUM_PORTS > 1
645649
HUB_DRIVER_EXIT_CRITICAL();
646650

647651
p_hub_driver_obj->constant.proc_req_cb(USB_PROC_REQ_SOURCE_HUB, false, p_hub_driver_obj->constant.proc_req_cb_arg);
@@ -656,11 +660,12 @@ static esp_err_t root_port_disable(root_hub_port_t *root_hub_port)
656660

657661
HUB_DRIVER_ENTER_CRITICAL();
658662
root_hub_port->dynamic.reqs |= PORT_REQ_DISABLE;
659-
if (root_hub_port->constant.index == 0) {
660-
p_hub_driver_obj->dynamic.flags.actions |= HUB_DRIVER_ACTION_ROOT_REQ;
661-
} else {
663+
p_hub_driver_obj->dynamic.flags.actions |= HUB_DRIVER_ACTION_ROOT0_REQ;
664+
#if HCD_NUM_PORTS > 1
665+
if (root_hub_port->constant.index != 0) {
662666
p_hub_driver_obj->dynamic.flags.actions |= HUB_DRIVER_ACTION_ROOT1_REQ;
663667
}
668+
#endif // HCD_NUM_PORTS > 1
664669
HUB_DRIVER_EXIT_CRITICAL();
665670

666671
p_hub_driver_obj->constant.proc_req_cb(USB_PROC_REQ_SOURCE_HUB, false, p_hub_driver_obj->constant.proc_req_cb_arg);
@@ -816,6 +821,7 @@ esp_err_t hub_root_start(void)
816821
}
817822
const esp_err_t port_ret = hcd_port_command(root_port_hdl, HCD_PORT_CMD_POWER_ON);
818823
if (port_ret != ESP_OK) {
824+
ESP_LOGD(HUB_DRIVER_TAG, "Root port %d power on failed", i);
819825
ret = port_ret;
820826
goto fail;
821827
}
@@ -959,7 +965,7 @@ esp_err_t hub_root_mark_suspend(void)
959965
// TODO: Suspend/Resume API is currently available only for single host configuration.
960966
// Pick the first root port that is enabled
961967
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;
968+
hub_flag_action_t action = HUB_DRIVER_ACTION_ROOT0_REQ;
963969
#if HCD_NUM_PORTS > 1
964970
if (!root_hub_port->constant.hdl) {
965971
root_hub_port = &p_hub_driver_obj->root_hub_ports[1];
@@ -987,7 +993,7 @@ esp_err_t hub_root_mark_resume(void)
987993
// TODO: Suspend/Resume API is currently available only for single host configuration.
988994
// Pick the first root port that is enabled
989995
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;
996+
hub_flag_action_t action = HUB_DRIVER_ACTION_ROOT0_REQ;
991997
#if HCD_NUM_PORTS > 1
992998
if (!root_hub_port->constant.hdl) {
993999
root_hub_port = &p_hub_driver_obj->root_hub_ports[1];
@@ -1202,10 +1208,10 @@ esp_err_t hub_process(void)
12021208
ESP_ERROR_CHECK(ext_hub_process());
12031209
}
12041210
#endif // ENABLE_USB_HUBS
1205-
if (action_flags & HUB_DRIVER_ACTION_ROOT_EVENT) {
1211+
if (action_flags & HUB_DRIVER_ACTION_ROOT0_EVENT) {
12061212
root_port_handle_events(&p_hub_driver_obj->root_hub_ports[0]);
12071213
}
1208-
if (action_flags & HUB_DRIVER_ACTION_ROOT_REQ) {
1214+
if (action_flags & HUB_DRIVER_ACTION_ROOT0_REQ) {
12091215
root_port_req(&p_hub_driver_obj->root_hub_ports[0]);
12101216
}
12111217
#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)