Merge pull request #3246 from HiFiPhile/nxp_host

ehci: fix device removal and endpoint open on next plug

Author: hathach

hw/bsp/imxrt/boards/mimxrt1170_evkb/board.h

@@ -68,8 +68,6 @@ function(add_board_target BOARD_TARGET)
 
   if (NOT M4 STREQUAL "1")
     target_compile_definitions(${BOARD_TARGET} PUBLIC
-      __ARMVFP__=0
-      __ARMFPV5__=0
       XIP_EXTERNAL_FLASH=1
       XIP_BOOT_HEADER_ENABLE=1
       )

hw/bsp/imxrt/family.c

@@ -14,8 +14,6 @@ CFLAGS += \
 
 ifneq ($(M4), 1)
 CFLAGS += \
-  -D__ARMVFP__=0 \
-  -D__ARMFPV5__=0 \
   -DXIP_EXTERNAL_FLASH=1 \
   -DXIP_BOOT_HEADER_ENABLE=1
 endif

hw/bsp/imxrt/family.cmake

@@ -519,7 +519,8 @@ bool hidh_open(uint8_t rhport, uint8_t daddr, tusb_desc_interface_t const* desc_
 
   // Assume bNumDescriptors = 1
   p_hid->report_desc_type = desc_hid->bReportType;
-  p_hid->report_desc_len = tu_unaligned_read16(&desc_hid->wReportLength);
+  // Use offsetof to avoid pointer to the odd/misaligned address
+  p_hid->report_desc_len = tu_unaligned_read16((uint8_t const*)desc_hid + offsetof(tusb_hid_descriptor_hid_t, wReportLength));
 
   // Per HID Specs: default is Report protocol, though we will force Boot protocol when set_config
   p_hid->protocol_mode = _hidh_default_protocol;

hw/bsp/imxrt/family.mk

@@ -400,7 +400,7 @@ bool tuh_descriptor_get_device_local(uint8_t daddr, tusb_desc_device_t* desc_dev
 tusb_speed_t tuh_speed_get(uint8_t daddr) {
   tuh_bus_info_t bus_info;
   tuh_bus_info_get(daddr, &bus_info);
-  return bus_info.speed;
+  return (tusb_speed_t)bus_info.speed;
 }
 
 bool tuh_rhport_is_active(uint8_t rhport) {
@@ -651,7 +651,7 @@ void tuh_task_ext(uint32_t timeout_ms, bool in_isr) {
               tuh_xfer_t xfer = {
                   .daddr       = event.dev_addr,
                   .ep_addr     = ep_addr,
-                  .result      = event.xfer_complete.result,
+                  .result      = (xfer_result_t)event.xfer_complete.result,
                   .actual_len  = event.xfer_complete.len,
                   .buflen      = 0,    // not available
                   .buffer      = NULL, // not available
@@ -832,18 +832,19 @@ static bool usbh_control_xfer_cb (uint8_t daddr, uint8_t ep_addr, xfer_result_t
           }
           TU_ATTR_FALLTHROUGH;
 
-        case CONTROL_STAGE_DATA:
-          if (request->wLength) {
-            TU_LOG_USBH("[%u:%u] Control data:\r\n", rhport, daddr);
-            TU_LOG_MEM_USBH(ctrl_info->buffer, xferred_bytes, 2);
-          }
-          ctrl_info->actual_len = (uint16_t) xferred_bytes;
+        case CONTROL_STAGE_DATA: {
+            if (request->wLength) {
+              TU_LOG_USBH("[%u:%u] Control data:\r\n", rhport, daddr);
+              TU_LOG_MEM_USBH(ctrl_info->buffer, xferred_bytes, 2);
+            }
+            ctrl_info->actual_len = (uint16_t) xferred_bytes;
 
-          // ACK stage: toggle is always 1
-          _control_set_xfer_stage(CONTROL_STAGE_ACK);
-          const uint8_t ep_status = tu_edpt_addr(0, 1 - request->bmRequestType_bit.direction);
-          TU_ASSERT(hcd_edpt_xfer(rhport, daddr, ep_status, NULL, 0));
-          break;
+            // ACK stage: toggle is always 1
+            _control_set_xfer_stage(CONTROL_STAGE_ACK);
+            const uint8_t ep_status = tu_edpt_addr(0, 1 - request->bmRequestType_bit.direction);
+            TU_ASSERT(hcd_edpt_xfer(rhport, daddr, ep_status, NULL, 0));
+            break;
+          }
 
         case CONTROL_STAGE_ACK: {
           // Abort all pending transfers if SET_CONFIGURATION request

src/class/hid/hid_host.c

@@ -35,9 +35,15 @@
 
 #include "host/hcd.h"
 #include "host/usbh.h"
+#include "host/usbh_pvt.h"
 #include "ehci_api.h"
 #include "ehci.h"
 
+// NXP specific fixes
+#if TU_CHECK_MCU(OPT_MCU_MIMXRT1XXX, OPT_MCU_LPC55, OPT_MCU_MCXN9)
+#include "fsl_device_registers.h"
+#endif
+
 //--------------------------------------------------------------------+
 // MACRO CONSTANT TYPEDEF
 //--------------------------------------------------------------------+
@@ -179,12 +185,43 @@ static void ehci_enable_schedule(ehci_registers_t* regs, bool is_period) {
   }
 }
 
+#if ((defined FSL_FEATURE_SOC_USBPHY_COUNT) && (FSL_FEATURE_SOC_USBPHY_COUNT > 0U))
+static void nxp_usbphy_disconn_detector_set(uint8_t port, bool enable) {
+  // unify naming convention
+#if !defined(USBPHY1) && defined(USBPHY)
+  #define USBPHY1 USBPHY
+#endif
+
+  if (port == 0) {
+    if (enable) {
+      USBPHY1->CTRL_SET = USBPHY_CTRL_ENHOSTDISCONDETECT_MASK;
+    } else {
+      USBPHY1->CTRL_CLR = USBPHY_CTRL_ENHOSTDISCONDETECT_MASK;
+    }
+  }
+#if FSL_FEATURE_SOC_USBPHY_COUNT > 1U
+  else if (port == 1) {
+    if (enable) {
+      USBPHY2->CTRL_SET = USBPHY_CTRL_ENHOSTDISCONDETECT_MASK;
+    } else {
+      USBPHY2->CTRL_CLR = USBPHY_CTRL_ENHOSTDISCONDETECT_MASK;
+    }
+  }
+#endif
+
+#if !defined(USBPHY1) && defined(USBPHY)
+  #undef USBPHY1
+#endif
+}
+#endif
+
 //--------------------------------------------------------------------+
 // HCD API
 //--------------------------------------------------------------------+
 uint32_t hcd_frame_number(uint8_t rhport) {
   (void) rhport;
-  return (ehci_data.uframe_number + ehci_data.regs->frame_index) >> 3;
+  uint32_t uframe = ehci_data.regs->frame_index;
+  return (ehci_data.uframe_number + uframe) >> 3;
 }
 
 void hcd_port_reset(uint8_t rhport) {
@@ -212,16 +249,21 @@ void hcd_port_reset_end(uint8_t rhport) {
   (void) rhport;
   ehci_registers_t* regs = ehci_data.regs;
 
-  // skip if reset is already complete
-  if (!regs->portsc_bm.port_reset) {
-    return;
-  }
+  // stop reset only if is not complete yet
+  if (regs->portsc_bm.port_reset) {
+    // mask out all change bits since they are Write 1 to clear
+    uint32_t portsc = regs->portsc & ~EHCI_PORTSC_MASK_W1C;
+    portsc &= ~EHCI_PORTSC_MASK_PORT_RESET;
 
-  // mask out all change bits since they are Write 1 to clear
-  uint32_t portsc = regs->portsc & ~EHCI_PORTSC_MASK_W1C;
-  portsc &= ~EHCI_PORTSC_MASK_PORT_RESET;
+    regs->portsc = portsc;
+  }
 
-  regs->portsc = portsc;
+#if ((defined FSL_FEATURE_SOC_USBPHY_COUNT) && (FSL_FEATURE_SOC_USBPHY_COUNT > 0U))
+    // Enable disconnect detector for highspeed device only
+    if (hcd_port_speed_get(rhport) == TUSB_SPEED_HIGH) {
+      nxp_usbphy_disconn_detector_set(rhport, true);
+    }
+#endif
 }
 
 bool hcd_port_connect_status(uint8_t rhport) {
@@ -578,6 +620,10 @@ void port_connect_status_change_isr(uint8_t rhport) {
     hcd_event_device_attach(rhport, true);
   } else // device unplugged
   {
+#if ((defined FSL_FEATURE_SOC_USBPHY_COUNT) && (FSL_FEATURE_SOC_USBPHY_COUNT > 0U))
+    // Disable disconnect detector
+    nxp_usbphy_disconn_detector_set(rhport, false);
+#endif
     hcd_event_device_remove(rhport, true);
   }
 }
@@ -821,14 +867,21 @@ static ehci_qhd_t *qhd_get_from_addr(uint8_t dev_addr, uint8_t ep_addr) {
   }
 
   ehci_qhd_t *qhd_pool = ehci_data.qhd_pool;
+
+  // protect qhd_pool since 'used' and 'removing' can be changed in isr
+  ehci_qhd_t *result = NULL;
+  usbh_spin_lock(false);
   for (uint32_t i = 0; i < QHD_MAX; i++) {
     if ((qhd_pool[i].dev_addr == dev_addr) &&
-        ep_addr == qhd_ep_addr(&qhd_pool[i])) {
-      return &qhd_pool[i];
+        ep_addr == qhd_ep_addr(&qhd_pool[i]) &&
+        qhd_pool[i].used && !qhd_pool[i].removing) {
+      result = &qhd_pool[i];
+      break;
     }
   }
+  usbh_spin_unlock(false);
 
-  return NULL;
+  return result;
 }
 
 // Init queue head with endpoint descriptor
@@ -896,7 +949,7 @@ static void qhd_init(ehci_qhd_t *p_qhd, uint8_t dev_addr, tusb_desc_endpoint_t c
   p_qhd->used         = 1;
   p_qhd->removing     = 0;
   p_qhd->attached_qtd = NULL;
-  p_qhd->pid = tu_edpt_dir(ep_desc->bEndpointAddress) ? EHCI_PID_IN : EHCI_PID_OUT; // PID for TD under this endpoint
+  p_qhd->pid = tu_edpt_dir(ep_desc->bEndpointAddress) == TUSB_DIR_IN ? EHCI_PID_IN : EHCI_PID_OUT; // PID for TD under this endpoint
 
   //------------- active, but no TD list -------------//
   p_qhd->qtd_overlay.halted              = 0;