Skip to content

Commit eb95c7d

Browse files
roma-jamroma-jam
committed
refactor(hub): Removed parent_dev_hdl from dev tree node structure
1 parent d7ff211 commit eb95c7d

File tree

2 files changed

+47
-48
lines changed

2 files changed

+47
-48
lines changed

‎host/usb/private_include/hub.h‎

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -186,10 +186,10 @@ esp_err_t hub_node_active(unsigned int node_uid);
186186
* @param[in] node_uid Device's node unique ID
187187
*
188188
* @return
189-
* - ESP_OK: Port has been disabled without error
190-
* - ESP_ERR_INVALID_STATE: Port can't be disabled in current state
191-
* - ESP_ERR_NOT_SUPPORTED: This function is not support by the selected port
192-
* - ESP_ERR_NOT_FOUND: Device's node is not found
189+
* - ESP_ERR_INVALID_STATE if Hub driver is not installed
190+
* - ESP_ERR_NOT_SUPPORTED if the function is not support by the selected port
191+
* - ESP_ERR_NOT_FOUND if device's tree node is not found by uid
192+
* - ESP_OK if Port has been disabled without error
193193
*/
194194
esp_err_t hub_node_disable(unsigned int node_uid);
195195

‎host/usb/src/hub.c‎

Lines changed: 43 additions & 44 deletions
Original file line numberDiff line numberDiff line change
@@ -69,9 +69,9 @@ typedef enum {
6969
*/
7070
struct dev_tree_node_s {
7171
TAILQ_ENTRY(dev_tree_node_s) tailq_entry; /**< Entry for the device tree node object tailq */
72-
unsigned int uid; /**< Device's unique ID */
73-
usb_device_handle_t parent_dev_hdl; /**< Device's parent handle */
74-
uint8_t parent_port_num; /**< Device's parent port number */
72+
unsigned int uid; /**< Device's unique ID */
73+
void* parent; /**< Device's parent context */
74+
uint8_t port_num; /**< Device's parent port number */
7575
};
7676

7777
typedef struct dev_tree_node_s dev_tree_node_t;
@@ -145,7 +145,7 @@ static bool root_port_callback(hcd_port_handle_t port_hdl, hcd_port_event_t port
145145

146146
// ---------------------- Internal Logic ------------------------
147147

148-
dev_tree_node_t *dev_tree_node_get_by_uid(unsigned int node_uid)
148+
static dev_tree_node_t *dev_tree_node_get_by_uid(unsigned int node_uid)
149149
{
150150
dev_tree_node_t *dev_tree_node = NULL;
151151
dev_tree_node_t *dev_tree_iter;
@@ -164,7 +164,7 @@ dev_tree_node_t *dev_tree_node_get_by_uid(unsigned int node_uid)
164164
*
165165
* @return esp_err_t
166166
*/
167-
static esp_err_t dev_tree_node_new(usb_device_handle_t parent_dev_hdl, uint8_t parent_port_num, usb_speed_t speed)
167+
static esp_err_t dev_tree_node_new(void* parent, uint8_t port_num, usb_speed_t speed)
168168
{
169169
esp_err_t ret;
170170
// Allocate memory for a new device tree node
@@ -182,17 +182,17 @@ static esp_err_t dev_tree_node_new(usb_device_handle_t parent_dev_hdl, uint8_t p
182182
assert(dev_tree_node->uid != 0); // No overflow possible
183183
}
184184

185-
dev_tree_node->parent_dev_hdl = parent_dev_hdl;
186-
dev_tree_node->parent_port_num = parent_port_num;
185+
dev_tree_node->parent = parent;
186+
dev_tree_node->port_num = port_num;
187187

188188
// Initialize and register a new USBH Device with the assigned UID
189189
usbh_dev_params_t params = {
190190
.uid = dev_tree_node->uid,
191191
.speed = speed,
192192
.root_port_hdl = p_hub_driver_obj->constant.root_port_hdl, // Always the same for all devices
193193
// TODO: IDF-10023 Move parent-child tree management responsibility to Hub Driver
194-
.parent_dev_hdl = parent_dev_hdl,
195-
.parent_port_num = parent_port_num,
194+
.parent_dev_hdl = NULL,
195+
.parent_port_num = port_num,
196196
};
197197

198198
ret = usbh_devs_add(&params);
@@ -203,7 +203,7 @@ static esp_err_t dev_tree_node_new(usb_device_handle_t parent_dev_hdl, uint8_t p
203203

204204
TAILQ_INSERT_TAIL(&p_hub_driver_obj->single_thread.dev_nodes_tailq, dev_tree_node, tailq_entry);
205205

206-
ESP_LOGD(HUB_DRIVER_TAG, "Device tree node (uid=%d): new", dev_tree_node->uid);
206+
ESP_LOGD(HUB_DRIVER_TAG, "Device tree node (port %d, uid %d): new", dev_tree_node->port_num, dev_tree_node->uid);
207207

208208
hub_event_data_t event_data = {
209209
.event = HUB_EVENT_CONNECTED,
@@ -220,21 +220,21 @@ static esp_err_t dev_tree_node_new(usb_device_handle_t parent_dev_hdl, uint8_t p
220220
return ret;
221221
}
222222

223-
static esp_err_t dev_tree_node_reset_completed(usb_device_handle_t parent_dev_hdl, uint8_t parent_port_num)
223+
static esp_err_t dev_tree_node_reset_completed(void* parent, uint8_t port_num)
224224
{
225225
dev_tree_node_t *dev_tree_node = NULL;
226226
dev_tree_node_t *dev_tree_iter;
227227
// Search the device tree nodes list for a device node with the specified parent
228228
TAILQ_FOREACH(dev_tree_iter, &p_hub_driver_obj->single_thread.dev_nodes_tailq, tailq_entry) {
229-
if (dev_tree_iter->parent_dev_hdl == parent_dev_hdl &&
230-
dev_tree_iter->parent_port_num == parent_port_num) {
229+
if (dev_tree_iter->parent == parent &&
230+
dev_tree_iter->port_num == port_num) {
231231
dev_tree_node = dev_tree_iter;
232232
break;
233233
}
234234
}
235235

236236
if (dev_tree_node == NULL) {
237-
ESP_LOGE(HUB_DRIVER_TAG, "Reset completed, but device tree node with port=%d not found", parent_port_num);
237+
ESP_LOGE(HUB_DRIVER_TAG, "Reset completed, but device tree node (port %d) not found", port_num);
238238
return ESP_ERR_NOT_FOUND;
239239
}
240240

@@ -248,25 +248,25 @@ static esp_err_t dev_tree_node_reset_completed(usb_device_handle_t parent_dev_hd
248248
return ESP_OK;
249249
}
250250

251-
static esp_err_t dev_tree_node_dev_gone(usb_device_handle_t parent_dev_hdl, uint8_t parent_port_num)
251+
static esp_err_t dev_tree_node_dev_gone(void* parent, uint8_t port_num)
252252
{
253253
dev_tree_node_t *dev_tree_node = NULL;
254254
dev_tree_node_t *dev_tree_iter;
255255
// Search the device tree nodes list for a device node with the specified parent
256256
TAILQ_FOREACH(dev_tree_iter, &p_hub_driver_obj->single_thread.dev_nodes_tailq, tailq_entry) {
257-
if (dev_tree_iter->parent_dev_hdl == parent_dev_hdl &&
258-
dev_tree_iter->parent_port_num == parent_port_num) {
257+
if (dev_tree_iter->parent == parent &&
258+
dev_tree_iter->port_num == port_num) {
259259
dev_tree_node = dev_tree_iter;
260260
break;
261261
}
262262
}
263263

264264
if (dev_tree_node == NULL) {
265-
ESP_LOGW(HUB_DRIVER_TAG, "Device tree node (parent_port=%d): not found", parent_port_num);
265+
ESP_LOGW(HUB_DRIVER_TAG, "Device tree node (port %d): not found", port_num);
266266
return ESP_ERR_NOT_FOUND;
267267
}
268268

269-
ESP_LOGD(HUB_DRIVER_TAG, "Device tree node (uid=%d): device gone", dev_tree_node->uid);
269+
ESP_LOGD(HUB_DRIVER_TAG, "Device tree node (port %d, uid=%d): device gone", port_num, dev_tree_node->uid);
270270

271271
hub_event_data_t event_data = {
272272
.event = HUB_EVENT_DISCONNECTED,
@@ -284,25 +284,25 @@ static esp_err_t dev_tree_node_dev_gone(usb_device_handle_t parent_dev_hdl, uint
284284
*
285285
* @return esp_err_t
286286
*/
287-
static esp_err_t dev_tree_node_remove_by_parent(usb_device_handle_t parent_dev_hdl, uint8_t parent_port_num)
287+
static esp_err_t dev_tree_node_remove_by_parent(void* parent, uint8_t port_num)
288288
{
289289
dev_tree_node_t *dev_tree_node = NULL;
290290
dev_tree_node_t *dev_tree_iter;
291291
// Search the device tree nodes list for a device node with the specified parent
292292
TAILQ_FOREACH(dev_tree_iter, &p_hub_driver_obj->single_thread.dev_nodes_tailq, tailq_entry) {
293-
if (dev_tree_iter->parent_dev_hdl == parent_dev_hdl &&
294-
dev_tree_iter->parent_port_num == parent_port_num) {
293+
if (dev_tree_iter->parent == parent &&
294+
dev_tree_iter->port_num == port_num) {
295295
dev_tree_node = dev_tree_iter;
296296
break;
297297
}
298298
}
299299

300300
if (dev_tree_node == NULL) {
301-
ESP_LOGW(HUB_DRIVER_TAG, "Device tree node (parent_port=%d): not found", parent_port_num);
301+
ESP_LOGW(HUB_DRIVER_TAG, "Device tree node (port %d): not found", port_num);
302302
return ESP_ERR_NOT_FOUND;
303303
}
304304

305-
ESP_LOGD(HUB_DRIVER_TAG, "Device tree node (uid=%d): freeing", dev_tree_node->uid);
305+
ESP_LOGD(HUB_DRIVER_TAG, "Device tree node (port %d, uid %d): freeing", port_num, dev_tree_node->uid);
306306

307307
TAILQ_REMOVE(&p_hub_driver_obj->single_thread.dev_nodes_tailq, dev_tree_node, tailq_entry);
308308
heap_caps_free(dev_tree_node);
@@ -365,7 +365,7 @@ static void ext_port_event_callback(ext_port_hdl_t port_hdl, ext_port_event_data
365365
}
366366
}
367367

368-
if (dev_tree_node_new(event_data->connected.parent_dev_hdl, event_data->connected.parent_port_num, port_speed) != ESP_OK) {
368+
if (dev_tree_node_new(ext_hub_hdl, event_data->connected.parent_port_num, port_speed) != ESP_OK) {
369369
ESP_LOGE(HUB_DRIVER_TAG, "Failed to add new downstream device");
370370
goto new_ds_dev_err;
371371
}
@@ -374,11 +374,11 @@ static void ext_port_event_callback(ext_port_hdl_t port_hdl, ext_port_event_data
374374
ext_hub_port_disable(ext_hub_hdl, event_data->connected.parent_port_num);
375375
break;
376376
case EXT_PORT_RESET_COMPLETED:
377-
ESP_ERROR_CHECK(dev_tree_node_reset_completed(event_data->reset_completed.parent_dev_hdl, event_data->reset_completed.parent_port_num));
377+
ESP_ERROR_CHECK(dev_tree_node_reset_completed(ext_hub_hdl, event_data->reset_completed.parent_port_num));
378378
break;
379379
case EXT_PORT_DISCONNECTED:
380380
// The node could be freed by now, no need to verify the result here
381-
dev_tree_node_dev_gone(event_data->disconnected.parent_dev_hdl, event_data->disconnected.parent_port_num);
381+
dev_tree_node_dev_gone(ext_hub_hdl, event_data->disconnected.parent_port_num);
382382
break;
383383
default:
384384
// Should never occur
@@ -691,13 +691,12 @@ esp_err_t hub_node_recycle(unsigned int node_uid)
691691
dev_tree_node_t *dev_tree_node = dev_tree_node_get_by_uid(node_uid);
692692
HUB_DRIVER_CHECK(dev_tree_node != NULL, ESP_ERR_NOT_FOUND);
693693

694-
if (dev_tree_node->parent_dev_hdl == NULL) {
694+
if (dev_tree_node->parent == NULL) {
695+
// Root Hub Ports
695696
ret = root_port_recycle();
696697
} else {
697698
#if ENABLE_USB_HUBS
698-
ext_hub_handle_t ext_hub_hdl = NULL;
699-
ext_hub_get_handle(dev_tree_node->parent_dev_hdl, &ext_hub_hdl);
700-
ret = ext_hub_port_recycle(ext_hub_hdl, dev_tree_node->parent_port_num);
699+
ret = ext_hub_port_recycle((ext_hub_handle_t) dev_tree_node->parent, dev_tree_node->port_num);
701700
if (ret != ESP_OK) {
702701
ESP_LOGE(HUB_DRIVER_TAG, "Ext hub port recycle error: %s", esp_err_to_name(ret));
703702
}
@@ -708,7 +707,7 @@ esp_err_t hub_node_recycle(unsigned int node_uid)
708707
}
709708

710709
if (ret == ESP_OK) {
711-
ESP_ERROR_CHECK(dev_tree_node_remove_by_parent(dev_tree_node->parent_dev_hdl, dev_tree_node->parent_port_num));
710+
ESP_ERROR_CHECK(dev_tree_node_remove_by_parent(dev_tree_node->parent, dev_tree_node->port_num));
712711
}
713712

714713
return ret;
@@ -724,7 +723,7 @@ esp_err_t hub_node_reset(unsigned int node_uid)
724723
dev_tree_node_t *dev_tree_node = dev_tree_node_get_by_uid(node_uid);
725724
HUB_DRIVER_CHECK(dev_tree_node != NULL, ESP_ERR_NOT_FOUND);
726725

727-
if (dev_tree_node->parent_dev_hdl == NULL) {
726+
if (dev_tree_node->parent == NULL) {
728727
ret = hcd_port_command(p_hub_driver_obj->constant.root_port_hdl, HCD_PORT_CMD_RESET);
729728
if (ret != ESP_OK) {
730729
ESP_LOGE(HUB_DRIVER_TAG, "Failed to issue root port reset");
@@ -733,9 +732,7 @@ esp_err_t hub_node_reset(unsigned int node_uid)
733732
}
734733
} else {
735734
#if ENABLE_USB_HUBS
736-
ext_hub_handle_t ext_hub_hdl = NULL;
737-
ext_hub_get_handle(dev_tree_node->parent_dev_hdl, &ext_hub_hdl);
738-
ret = ext_hub_port_reset(ext_hub_hdl, dev_tree_node->parent_port_num);
735+
ret = ext_hub_port_reset((ext_hub_handle_t) dev_tree_node->parent, dev_tree_node->port_num);
739736
if (ret != ESP_OK) {
740737
ESP_LOGE(HUB_DRIVER_TAG, "Ext hub port reset error: %s", esp_err_to_name(ret));
741738
}
@@ -749,20 +746,21 @@ esp_err_t hub_node_reset(unsigned int node_uid)
749746

750747
esp_err_t hub_node_active(unsigned int node_uid)
751748
{
749+
HUB_DRIVER_ENTER_CRITICAL();
750+
HUB_DRIVER_CHECK_FROM_CRIT(p_hub_driver_obj != NULL, ESP_ERR_INVALID_STATE);
751+
HUB_DRIVER_EXIT_CRITICAL();
752752
esp_err_t ret;
753753

754754
dev_tree_node_t *dev_tree_node = dev_tree_node_get_by_uid(node_uid);
755755
HUB_DRIVER_CHECK(dev_tree_node != NULL, ESP_ERR_NOT_FOUND);
756756

757-
if (dev_tree_node->parent_dev_hdl == NULL) {
757+
if (dev_tree_node->parent == NULL) {
758758
// Root port no need to be activated
759759
ret = ESP_OK;
760760
} else {
761761
#if ENABLE_USB_HUBS
762762
// External Hub port
763-
ext_hub_handle_t ext_hub_hdl = NULL;
764-
ext_hub_get_handle(dev_tree_node->parent_dev_hdl, &ext_hub_hdl);
765-
ret = ext_hub_port_active(ext_hub_hdl, dev_tree_node->parent_port_num);
763+
ret = ext_hub_port_active((ext_hub_handle_t) dev_tree_node->parent, dev_tree_node->port_num);
766764
if (ret != ESP_OK) {
767765
ESP_LOGE(HUB_DRIVER_TAG, "Ext hub port activation error: %s", esp_err_to_name(ret));
768766
}
@@ -776,19 +774,20 @@ esp_err_t hub_node_active(unsigned int node_uid)
776774

777775
esp_err_t hub_node_disable(unsigned int node_uid)
778776
{
777+
HUB_DRIVER_ENTER_CRITICAL();
778+
HUB_DRIVER_CHECK_FROM_CRIT(p_hub_driver_obj != NULL, ESP_ERR_INVALID_STATE);
779+
HUB_DRIVER_EXIT_CRITICAL();
779780
esp_err_t ret;
780781

781782
dev_tree_node_t *dev_tree_node = dev_tree_node_get_by_uid(node_uid);
782783
HUB_DRIVER_CHECK(dev_tree_node != NULL, ESP_ERR_NOT_FOUND);
783784

784-
if (dev_tree_node->parent_dev_hdl == NULL) {
785+
if (dev_tree_node->parent == NULL) {
785786
ret = root_port_disable();
786787
} else {
787788
#if ENABLE_USB_HUBS
788789
// External Hub port
789-
ext_hub_handle_t ext_hub_hdl = NULL;
790-
ext_hub_get_handle(dev_tree_node->parent_dev_hdl, &ext_hub_hdl);
791-
ret = ext_hub_port_disable(ext_hub_hdl, dev_tree_node->parent_port_num);
790+
ret = ext_hub_port_disable((ext_hub_handle_t) dev_tree_node->parent, dev_tree_node->port_num);
792791
#else
793792
ESP_LOGW(HUB_DRIVER_TAG, "Disabling External Port is not available (External Hub support disabled)");
794793
ret = ESP_ERR_NOT_SUPPORTED;

0 commit comments

Comments
 (0)