Merge branch 'refs/heads/master' into fork/ChrisDeadman/hcd-samd-support

4 files changed, 167 insertions, 43 deletions

diff --git a/src/portable/microchip/pic/README.md b/src/portable/microchip/pic/README.md
new file mode 100644
index 000000000..7ba6a4a96
--- /dev/null
+++ b/src/portable/microchip/pic/README.md
@@ -0,0 +1,51 @@
+# Microchip PIC Chipidea FS Driver
+
+This driver adds support for Microchip PIC microcontrollers with full-speed Chipidea USB peripheral to the TinyUSB stack. It supports the following families:
+
+- PIC32MX (untested)
+- PIC32MM
+- PIC32MK (untested)
+- PIC24FJ
+- PIC24EP (untested)
+- dsPIC33EP (untested)
+
+Currently only the device mode is supported.
+
+
+## Important Notes
+
+### Handling of shared VBUS & GPIO pin
+
+Some PICs have the USB VBUS pin bonded with a GPIO pin in the chip package. This driver does **NOT** handle the potential conflict between the VBUS and GPIO functionalities.
+
+Developers must ensure that the GPIO pin is tristated when the VBUS pin is managed by the USB peripheral in order to prevent damaging the chip.
+
+This design choice allows developers the flexibility to use the GPIO functionality for controlling VBUS in device mode if desired.
+
+
+## TODO
+
+### Handle USB remote wakeup timing correctly
+
+The Chipidea FS IP doesn't handle the RESUME signal automatically and it must be managed in software. It needs to be asserted for exactly 10ms, and this is impossible to do without per-device support due to BSP differences. For now, a simple for-based loop is used.
+
+### 8-bit PIC support
+
+The 8-bit PICs also uses the Chipidea FS IP. Technically it's possible to support them as well.
+
+Possible difficulties:
+- Memory size constraints (1KB/8KB ballpark)
+- A third BDT layout (now we have two)
+- Different compiler-specific directives
+- Compiler bugs if you use SDCC
+
+
+## Author
+[ReimuNotMoe](https://github.com/ReimuNotMoe) at SudoMaker, Ltd.
+
+
+## Credits
+
+This driver is based on:
+- Microchip's USB driver (usb_device.c)
+- TinyUSB's NXP KHCI driver (dcd_khci.c)
diff --git a/src/portable/microchip/pic/dcd_pic.c b/src/portable/microchip/pic/dcd_pic.c
index d40c4794a..b4a698199 100644
--- a/src/portable/microchip/pic/dcd_pic.c
+++ b/src/portable/microchip/pic/dcd_pic.c
@@ -1,8 +1,11 @@
 /*
  * The MIT License (MIT)
  *
- * Copyright (c) 2020 Koji Kitayama
- * Copyright (c) 2022 Reimu NotMoe <[email protected]>
+ * Copyright (c) 2022-2024 SudoMaker, Ltd.
+ * Author: Mike Yang (Reimu NotMoe) <[email protected]>
+ *
+ * Based on usb_device.c - Copyright (c) 2015 Microchip Technology Inc.
+ * Based on dcd_khci.c   - Copyright (c) 2020 Koji Kitayama
  *
  * Permission is hereby granted, free of charge, to any person obtaining a copy
  * of this software and associated documentation files (the "Software"), to deal
@@ -313,12 +316,23 @@ static void prepare_next_setup_packet(uint8_t rhport)
 {
   const unsigned out_odd = _dcd.endpoint[0][0].odd;
   const unsigned in_odd  = _dcd.endpoint[0][1].odd;
-  TU_ASSERT(0 == _dcd.bdt[0][0][out_odd].own, );
+
+  // Abandon any previous control transfers that might have been using EP0.
+  // Ordinarily, nothing actually needs abandoning, since the previous control
+  // transfer would have completed successfully prior to the host sending the
+  // next SETUP packet. However, in a timeout error case, or after an EP0
+  // STALL event, one or more UOWN bits might still be set. If so, we should
+  // clear the UOWN bits, so the EP0 IN/OUT endpoints are in a known inactive
+  // state, ready for re-arming by the `dcd_edpt_xfer' function that will be
+  // called next.
 
   _dcd.bdt[0][0][out_odd].data     = 0;
+  _dcd.bdt[0][0][out_odd].own      = 0;
   _dcd.bdt[0][0][out_odd ^ 1].data = 1;
   _dcd.bdt[0][1][in_odd].data      = 1;
+  _dcd.bdt[0][1][in_odd].own       = 0;
   _dcd.bdt[0][1][in_odd ^ 1].data  = 0;
+  _dcd.bdt[0][1][in_odd ^ 1].own   = 0;
   dcd_edpt_xfer(rhport, tu_edpt_addr(0, TUSB_DIR_OUT),
           _dcd.setup_packet, sizeof(_dcd.setup_packet));
 }
@@ -475,13 +489,12 @@ bool dcd_init(uint8_t rhport, const tusb_rhport_init_t* rh_init) {
   intr_disable(rhport);
   intr_clear(rhport);
 
-#if CFG_TUSB_MCU == OPT_MCU_PIC32MM
-  TRISBbits.TRISB6 = 1;
-#endif
-
   tu_memclr(&_dcd, sizeof(_dcd));
 
 #if TU_PIC_INT_SIZE == 4
+  // The USBBUSY bit is present on PIC32s and we're required to check it
+  // prior to powering on the USB peripheral (see DS61126F page 27)
+  while (U1PWRCbits.USBBUSY);
   U1PWRCSET = _U1PWRC_USBPWR_MASK;
 #else
   U1PWRCbits.USBPWR = 1;
@@ -505,6 +518,19 @@ bool dcd_init(uint8_t rhport, const tusb_rhport_init_t* rh_init) {
   return true;
 }
 
+bool dcd_deinit(uint8_t rhport)
+{
+  U1CON = 0;
+  U1IE = 0;
+  U1OTGIE = 0;
+#if TU_PIC_INT_SIZE == 4
+  U1PWRCCLR = _U1PWRC_USUSPEND_MASK | _U1PWRC_USBPWR_MASK;
+#else
+  U1PWRC &= ~(_U1PWRC_USUSPEND_MASK | _U1PWRC_USBPWR_MASK);
+#endif
+  return true;
+}
+
 void dcd_int_enable(uint8_t rhport)
 {
   intr_enable(rhport);
@@ -529,8 +555,25 @@ void dcd_remote_wakeup(uint8_t rhport)
 #else
   U1CONbits.RESUME = 1;
 #endif
-  unsigned cnt = 25000000 / 1000;
+
+  // FIXME: Assert RESUME signal correctly, requires device-specific handling
+  // For now we use a hardcoded cycle-based delay which attempts to delay 10ms
+  // at the most common CPU frequencies. On PIC32s we assume the loop body
+  // takes 3 cycles. On 16-bit PICs we assume the XC16 compiler is in use and
+  // use its `__delay_ms' function.
+
+#if CFG_TUSB_MCU == OPT_MCU_PIC32MM
+  uint32_t cnt = 24000000 / 1000 / 3;
+  while (cnt--) asm volatile("nop");
+#elif CFG_TUSB_MCU == OPT_MCU_PIC32MX
+  uint32_t cnt = 40000000 / 1000 / 3;
+  while (cnt--) asm volatile("nop");
+#elif CFG_TUSB_MCU == OPT_MCU_PIC32MK
+  uint32_t cnt = 120000000 / 1000 / 3;
   while (cnt--) asm volatile("nop");
+#else
+  __delay_ms(10);
+#endif
 
 #if TU_PIC_INT_SIZE == 4
   U1CONCLR = _U1CON_RESUME_MASK;
@@ -738,8 +781,11 @@ void dcd_edpt_clear_stall(uint8_t rhport, uint8_t ep_addr)
 //--------------------------------------------------------------------+
 void dcd_int_handler(uint8_t rhport)
 {
-  uint32_t is  = U1IR;
-  uint32_t msk = U1IE;
+  uint32_t is, msk;
+
+  // Part 1 - "USB interrupts"
+  is = U1IR;
+  msk = U1IE;
 
   U1IR = is & ~msk;
   is &= msk;
@@ -747,7 +793,7 @@ void dcd_int_handler(uint8_t rhport)
   if (is & _U1IR_UERRIF_MASK) {
     uint32_t es = U1EIR;
     U1EIR = es;
-    U1IR   = is; /* discard any pending events */
+    U1IR = is; /* discard any pending events */
   }
 
   if (is & _U1IR_URSTIF_MASK) {
@@ -758,29 +804,66 @@ void dcd_int_handler(uint8_t rhport)
   if (is & _U1IR_IDLEIF_MASK) {
     // Note Host usually has extra delay after bus reset (without SOF), which could falsely
     // detected as Sleep event. Though usbd has debouncing logic so we are good
+
+    /*
+     * NOTE: Do not clear U1OTGIRbits.ACTVIF here!
+     * Reason:
+     * ACTVIF is only generated once an IDLEIF has been generated.
+     * This is a 1:1 ratio interrupt generation.
+     * For every IDLEIF, there will be only one ACTVIF regardless of
+     * the number of subsequent bus transitions.
+     *
+     * If the ACTIF is cleared here, a problem could occur when:
+     * [       IDLE       ][bus activity ->
+     * <--- 3 ms ----->     ^
+     *                ^     ACTVIF=1
+     *                IDLEIF=1
+     *  #           #           #           #   (#=Program polling flags)
+     *                          ^
+     *                          This polling loop will see both
+     *                          IDLEIF=1 and ACTVIF=1.
+     *                          However, the program services IDLEIF first
+     *                          because ACTIVIE=0.
+     *                          If this routine clears the only ACTIVIF,
+     *                          then it can never get out of the suspend
+     *                          mode.
+     */
+    U1OTGIESET = _U1OTGIE_ACTVIE_MASK;
     U1IR = _U1IR_IDLEIF_MASK;
     process_bus_sleep(rhport);
   }
 
-  if (is & _U1IR_RESUMEIF_MASK) {
-    U1IR = _U1IR_RESUMEIF_MASK;
-    process_bus_resume(rhport);
-  }
-
   if (is & _U1IR_SOFIF_MASK) {
     U1IR = _U1IR_SOFIF_MASK;
     dcd_event_bus_signal(rhport, DCD_EVENT_SOF, true);
   }
 
   if (is & _U1IR_STALLIF_MASK) {
-    U1IR = _U1IR_STALLIF_MASK;
     process_stall(rhport);
+    U1IR = _U1IR_STALLIF_MASK;
   }
 
   if (is & _U1IR_TRNIF_MASK) {
     process_tokdne(rhport);
   }
 
+  // Part 2 - "USB OTG interrupts"
+  is = U1OTGIR;
+  msk = U1OTGIE;
+
+  U1OTGIR = is & ~msk;
+  is &= msk;
+
+  if (is & _U1OTGIR_ACTVIF_MASK) {
+#if TU_PIC_INT_SIZE == 4
+    U1OTGIECLR = _U1OTGIE_ACTVIE_MASK;
+#else
+    U1OTGIE &= ~_U1OTGIE_ACTVIE_MASK;
+#endif
+    U1OTGIR = _U1OTGIR_ACTVIF_MASK;
+    process_bus_resume(rhport);
+  }
+
   intr_clear(rhport);
 }
 
diff --git a/src/portable/microchip/samd/dcd_samd.c b/src/portable/microchip/samd/dcd_samd.c
index 0c96f6ac4..357aa1549 100644
--- a/src/portable/microchip/samd/dcd_samd.c
+++ b/src/portable/microchip/samd/dcd_samd.c
@@ -335,7 +335,7 @@ void dcd_edpt_clear_stall (uint8_t rhport, uint8_t ep_addr)
 //--------------------------------------------------------------------+
 // Interrupt Handler
 //--------------------------------------------------------------------+
-void maybe_transfer_complete(void) {
+static void maybe_transfer_complete(void) {
   uint32_t epints = USB->DEVICE.EPINTSMRY.reg;
 
   for (uint8_t epnum = 0; epnum < USB_EPT_NUM; epnum++) {
diff --git a/src/portable/microchip/samg/dcd_samg.c b/src/portable/microchip/samg/dcd_samg.c
index c88b31514..a5c768839 100644
--- a/src/portable/microchip/samg/dcd_samg.c
+++ b/src/portable/microchip/samg/dcd_samg.c
@@ -52,37 +52,31 @@ typedef struct
 // Endpoint 0-5, each can only be either OUT or In
 xfer_desc_t _dcd_xfer[EP_COUNT];
 
-void xfer_epsize_set(xfer_desc_t* xfer, uint16_t epsize)
-{
+TU_ATTR_ALWAYS_INLINE static inline void xfer_epsize_set(xfer_desc_t* xfer, uint16_t epsize) {
   xfer->epsize = epsize;
 }
 
-void xfer_begin(xfer_desc_t* xfer, uint8_t * buffer, uint16_t total_bytes)
-{
+TU_ATTR_ALWAYS_INLINE static inline void xfer_begin(xfer_desc_t* xfer, uint8_t * buffer, uint16_t total_bytes) {
   xfer->buffer     = buffer;
   // xfer->ff         = NULL; // TODO support dcd_edpt_xfer_fifo API
   xfer->total_len  = total_bytes;
   xfer->actual_len = 0;
 }
 
-void xfer_end(xfer_desc_t* xfer)
-{
+TU_ATTR_ALWAYS_INLINE static inline void xfer_end(xfer_desc_t* xfer) {
   xfer->buffer     = NULL;
   // xfer->ff         = NULL; // TODO support dcd_edpt_xfer_fifo API
   xfer->total_len  = 0;
   xfer->actual_len = 0;
 }
 
-uint16_t xfer_packet_len(xfer_desc_t* xfer)
-{
+TU_ATTR_ALWAYS_INLINE static inline uint16_t xfer_packet_len(xfer_desc_t* xfer) {
   // also cover zero-length packet
   return tu_min16(xfer->total_len - xfer->actual_len, xfer->epsize);
 }
 
-void xfer_packet_done(xfer_desc_t* xfer)
-{
+TU_ATTR_ALWAYS_INLINE static inline void xfer_packet_done(xfer_desc_t* xfer) {
   uint16_t const xact_len = xfer_packet_len(xfer);
-
   xfer->buffer += xact_len;
   xfer->actual_len += xact_len;
 }
@@ -90,19 +84,15 @@ void xfer_packet_done(xfer_desc_t* xfer)
 //------------- Transaction helpers -------------//
 
 // Write data to EP FIFO, return number of written bytes
-static void xact_ep_write(uint8_t epnum, uint8_t* buffer, uint16_t xact_len)
-{
-  for(uint16_t i=0; i<xact_len; i++)
-  {
+static void xact_ep_write(uint8_t epnum, uint8_t* buffer, uint16_t xact_len) {
+  for(uint16_t i=0; i<xact_len; i++) {
     UDP->UDP_FDR[epnum] = (uint32_t) buffer[i];
   }
 }
 
 // Read data from EP FIFO
-static void xact_ep_read(uint8_t epnum, uint8_t* buffer, uint16_t xact_len)
-{
-  for(uint16_t i=0; i<xact_len; i++)
-  {
+static void xact_ep_read(uint8_t epnum, uint8_t* buffer, uint16_t xact_len) {
+  for(uint16_t i=0; i<xact_len; i++) {
     buffer[i] = (uint8_t) UDP->UDP_FDR[epnum];
   }
 }
@@ -112,24 +102,24 @@ static void xact_ep_read(uint8_t epnum, uint8_t* buffer, uint16_t xact_len)
 #define CSR_NO_EFFECT_1_ALL (UDP_CSR_RX_DATA_BK0 | UDP_CSR_RX_DATA_BK1 | UDP_CSR_STALLSENT | UDP_CSR_RXSETUP | UDP_CSR_TXCOMP)
 
 // Per Specs: CSR need synchronization each write
-static inline void csr_write(uint8_t epnum, uint32_t value)
-{
+TU_ATTR_ALWAYS_INLINE static inline void csr_write(uint8_t epnum, uint32_t value) {
   uint32_t const csr = value;
   UDP->UDP_CSR[epnum] = csr;
 
   volatile uint32_t nop_count;
-  for (nop_count = 0; nop_count < 20; nop_count ++) __NOP();
+  for (nop_count = 0; nop_count < 20; nop_count ++) {
+    __NOP();
+  }
 }
 
 // Per Specs: CSR need synchronization each write
-static inline void csr_set(uint8_t epnum, uint32_t mask)
+TU_ATTR_ALWAYS_INLINE static inline void csr_set(uint8_t epnum, uint32_t mask)
 {
   csr_write(epnum, UDP->UDP_CSR[epnum] | CSR_NO_EFFECT_1_ALL | mask);
 }
 
 // Per Specs: CSR need synchronization each write
-static inline void csr_clear(uint8_t epnum, uint32_t mask)
-{
+TU_ATTR_ALWAYS_INLINE static inline void csr_clear(uint8_t epnum, uint32_t mask) {
   csr_write(epnum, (UDP->UDP_CSR[epnum] | CSR_NO_EFFECT_1_ALL) & ~mask);
 }