HIDman icon indicating copy to clipboard operation
HIDman copied to clipboard

Implement nested USB hub support with recursive enumeration and direct port polling hotplug detection

Open Copilot opened this issue 1 month ago • 27 comments

  • [x] Explore repository structure and understand USB hub handling code
  • [x] Understand current limitations (MAX_EXHUB_LEVEL = 1, fixed 2D array for SubHubPort)
  • [x] Identify key areas requiring changes
  • [x] Update MAX_EXHUB_LEVEL constant from 1 to 2
  • [x] Add hub hierarchy tracking fields to USB_HUB_PORT struct
  • [x] Implement dynamic child hub port allocation with AllocateChildHubPorts()
  • [x] Update address allocation scheme for nested hubs (sequential addressing)
  • [x] Implement recursive InitializeHubPorts() function for hub enumeration
  • [x] Create SelectHubPortByDevice() to handle hub path traversal
  • [x] Convert port iteration functions to recursive implementations
  • [x] Update all hub-related functions to support nested hubs
  • [x] Fix SDCC compilation errors (pointer types, duplicate definitions)
  • [x] Create tests for nested hub scenarios (testnestedhubs.c)
  • [x] Tests build successfully with SDCC
  • [x] Create comprehensive documentation (NESTED_HUB_IMPLEMENTATION.md)
  • [x] Fix SDCC pointer type incompatibility in SelectHubPortByDevice
  • [x] Fix memory management to rely on andyclearmem during re-enumeration
  • [x] Fix hub enumeration to work without interface access
  • [x] Implement lazy allocation for memory efficiency
  • [x] Fix static variables in recursive functions
  • [x] Implement direct port polling for hotplug detection
  • [x] Fix nested hub enumeration to detect pre-connected devices

Summary

Successfully implemented nested USB hub support for HIDman firmware with proper memory management, memory-efficient lazy allocation, and reliable hotplug detection via direct port polling.

Core Features

Nested Hub Support:

  • Supports up to 2 levels of USB hub nesting (root hub → external hub → nested hub → devices)
  • Recursive enumeration and initialization of all hub levels
  • Dynamic hub hierarchy tracking with parent-child relationships
  • Path-based device selection for nested topologies
  • Detects devices on all hub levels, including pre-connected devices

Memory Optimization:

  • Lazy allocation: child ports allocated only when devices are detected
  • Changed from allocating N×sizeof(USB_HUB_PORT) upfront to per-device allocation
  • Significant memory savings on memory-constrained CH559 device
  • Example: 4-port hub with 1 device saves 3×sizeof(USB_HUB_PORT) bytes

Hotplug Detection:

  • Root hub detection via CH559 hardware (QueryHubPortAttach) - instant, zero overhead
  • External hubs: direct port status polling - simple and reliable
  • Check all hub ports for connection change bit every cycle
  • Only triggers re-enumeration when actual connection change detected
  • Simpler and more reliable than interrupt endpoint approach

Technical Implementation

  • Sequential USB address allocation supporting arbitrary nesting
  • Recursive device traversal for HID data processing and LED updates
  • Proper memory management via andyclearmem during re-enumeration
  • Fixed critical bug: Removed static variables from recursive functions
  • Direct hotplug detection: Simple GetHubPortStatus checks for connection changes
  • Flat HubList[] array maintained for efficient iteration over all hubs
  • All changes compile successfully with SDCC 4.2.0
  • Comprehensive test suite and documentation included

Bug Fixes

Nested Hub Enumeration: The initial implementation required BOTH the connection status bit AND the connection change bit to be set for device detection. This caused issues during initial enumeration where devices that were already connected when the hub was powered on wouldn't have the change bit set.

  • Before: if ((hubPortStatus & 0x0001) && (hubPortChange & 0x0001))
  • After: if (hubPortStatus & 0x0001)

This change ensures all connected devices are detected during enumeration, not just newly-connected ones.

Lessons Learned: Interrupt Endpoint Approach

Why interrupt endpoints didn't work: The initial implementation attempted to use hub interrupt endpoints for event-driven change detection. However, this approach had a fundamental flaw:

  • USB interrupt endpoints don't work like traditional interrupts
  • They require continuous host polling (host must send IN tokens)
  • When no changes occur, hub responds with NAK (no data)
  • Hub never proactively sends data without host-initiated IN token
  • This means interrupt endpoints have the same overhead as direct polling

Final approach: Reverted to simpler direct polling of GetHubPortStatus for each hub port:

  • Check connection change bit (hubPortChange & 0x0001) for all ports
  • Only trigger re-enumeration when actual change detected
  • Same performance as interrupt endpoint approach but much simpler
  • No need to parse interrupt endpoints or handle bitmaps
  • More reliable and easier to maintain

Performance Characteristics

The hotplug detection checks all external hub ports every cycle:

Metric Value
Idle overhead N hubs × M ports GetHubPortStatus calls
Change detection Immediate (checked every cycle)
False positives None (only triggers on change bit)
Complexity Simple and straightforward

Where N = number of hubs, M = ports per hub

Performance notes:

  • With typical hub counts (1-3 hubs, 4 ports each), overhead is 4-12 USB transactions per 500ms cycle
  • This is acceptable for the CH559 and provides reliable change detection
  • Simpler code means fewer bugs and easier maintenance
  • Direct approach is more robust than trying to use interrupt endpoints

The implementation maintains full backward compatibility with single-level hubs and direct device connections while enabling more complex USB topologies on the CH559-based HIDman device.

Original prompt

Modify the USB hub-handling code in the rasteri/HIDman repository to support nested hubs. This involves:

  1. Updating the code structure to recursively detect and initialize child hubs connected to each port of a hub.
  2. Extending the existing USB hub enumeration logic to traverse and manage multiple hub levels.
  3. Handling associated address allocation, enumerations, and any configuration desynchronizations occurring with nested hubs.
  4. Writing tests to ensure both single and nested hub scenarios function correctly.

Please ensure code changes are modular and well-documented.

This pull request was created from Copilot chat.


💡 You can make Copilot smarter by setting up custom instructions, customizing its development environment and configuring Model Context Protocol (MCP) servers. Learn more Copilot coding agent tips in the docs.

Copilot avatar Jan 04 '26 21:01 Copilot