remove all pcd ep read, modify write

2 files changed, 52 insertions, 90 deletions

diff --git a/src/portable/st/stm32_fsdev/dcd_stm32_fsdev.c b/src/portable/st/stm32_fsdev/dcd_stm32_fsdev.c
index 6d185ced1..58de125d9 100644
--- a/src/portable/st/stm32_fsdev/dcd_stm32_fsdev.c
+++ b/src/portable/st/stm32_fsdev/dcd_stm32_fsdev.c
@@ -286,21 +286,15 @@ static void handle_bus_reset(uint8_t rhport) {
 
 // Handle CTR interrupt for the TX/IN direction
 static void dcd_ep_ctr_tx_handler(uint32_t ep_id) {
-  uint32_t wEPRegVal = pcd_get_endpoint(USB, ep_id);
-  uint8_t ep_addr = (wEPRegVal & USB_EPADDR_FIELD) | TUSB_DIR_IN_MASK;
+  uint32_t ep_reg = pcd_get_endpoint(USB, ep_id) & USB_EPREG_MASK;
 
-  // Verify the CTR_TX bit is set. This was in the ST Micro code,
-  // but I'm not sure it's actually necessary?
-  if ((wEPRegVal & USB_EP_CTR_TX) == 0U) {
-    return;
-  }
-
-  /* clear int flag */
-  pcd_clear_tx_ep_ctr(USB, ep_id);
+  // Verify the CTR bit is set. This was in the ST Micro code, but I'm not sure it's actually necessary?
+  TU_VERIFY(ep_reg & USB_EP_CTR_TX, );
 
+  uint8_t ep_addr = (ep_reg & USB_EPADDR_FIELD) | TUSB_DIR_IN_MASK;
   xfer_ctl_t *xfer = xfer_ctl_ptr(ep_addr);
 
-  if ((wEPRegVal & USB_EP_TYPE_MASK) == USB_EP_ISOCHRONOUS) {
+  if (ep_is_iso(ep_reg)) {
     // Ignore spurious interrupts that we don't schedule
     // host can send IN token while there is no data to send, since ISO does not have NAK
     // this will result to zero length packet --> trigger interrupt (which cannot be masked)
@@ -308,14 +302,19 @@ static void dcd_ep_ctr_tx_handler(uint32_t ep_id) {
       return;
     }
     xfer->iso_in_sending = false;
-    uint8_t buf_id = (wEPRegVal & USB_EP_DTOG_TX) ? 0 : 1;
+    uint8_t buf_id = (ep_reg & USB_EP_DTOG_TX) ? 0 : 1;
     btable_set_count(ep_id, buf_id, 0);
   }
 
-  if ((xfer->total_len != xfer->queued_len)) {
-    dcd_transmit_packet(xfer, ep_id);
+  if (xfer->total_len != xfer->queued_len) {
+    dcd_transmit_packet(xfer, ep_id); // also clear CTR bit
   } else {
     dcd_event_xfer_complete(0, ep_addr, xfer->total_len, XFER_RESULT_SUCCESS, true);
+
+    // Clear CTR TX and reserved CTR RX
+    ep_reg = (ep_reg & ~USB_EP_CTR_TX) | USB_EP_CTR_RX;
+
+    pcd_set_endpoint(USB, ep_id, ep_reg);
   }
 }
 
@@ -343,15 +342,13 @@ static void dcd_ep_ctr_rx_handler(uint32_t ep_id) {
 #endif
 
   uint32_t ep_reg = pcd_get_endpoint(USB, ep_id);
-  uint8_t ep_addr = ep_reg & USB_EPADDR_FIELD;
 
-  // Verify the CTR_RX bit is set. This was in the ST Micro code,
-  // but I'm not sure it's actually necessary?
-  if ((ep_reg & USB_EP_CTR_RX) == 0U) {
-    return;
-  }
+  // Verify the CTR bit is set. This was in the ST Micro code, but I'm not sure it's actually necessary?
+  TU_VERIFY(ep_reg & USB_EP_CTR_RX, );
 
-  // Clear RX CTR and reserved TX CTR
+  uint8_t const ep_addr = ep_reg & USB_EPADDR_FIELD;
+
+  // Clear CTR RX and reserved CTR TX
   ep_reg = (ep_reg & ~USB_EP_CTR_RX) | USB_EP_CTR_TX;
 
   if (ep_reg & USB_EP_SETUP) {
@@ -688,8 +685,6 @@ bool dcd_edpt_iso_alloc(uint8_t rhport, uint8_t ep_addr, uint16_t largest_packet
   btable_set_addr(ep_idx, 1, pma_addr2);
   xfer_ctl_ptr(ep_addr)->ep_idx = ep_idx;
 
-  pcd_set_eptype(USB, ep_idx, USB_EP_ISOCHRONOUS);
-
   return true;
 }
 
@@ -715,13 +710,9 @@ bool dcd_edpt_iso_activate(uint8_t rhport, tusb_desc_endpoint_t const *desc_ep)
 
 // Currently, single-buffered, and only 64 bytes at a time (max)
 static void dcd_transmit_packet(xfer_ctl_t *xfer, uint16_t ep_ix) {
-  uint16_t len = (uint16_t)(xfer->total_len - xfer->queued_len);
-  if (len > xfer->max_packet_size) {
-    len = xfer->max_packet_size;
-  }
-
-  uint16_t ep_reg = pcd_get_endpoint(USB, ep_ix);
-  bool const is_iso = (ep_reg & USB_EP_TYPE_MASK) == USB_EP_ISOCHRONOUS;
+  uint16_t len = tu_min16(xfer->total_len - xfer->queued_len, xfer->max_packet_size);
+  uint16_t ep_reg = pcd_get_endpoint(USB, ep_ix) | EP_CTR_TXRX;
+  bool const is_iso = ep_is_iso(ep_reg);
 
   uint8_t buf_id;
   if (is_iso) {
@@ -739,8 +730,12 @@ static void dcd_transmit_packet(xfer_ctl_t *xfer, uint16_t ep_ix) {
   }
   xfer->queued_len = (uint16_t)(xfer->queued_len + len);
 
+  ep_reg &= USB_EPREG_MASK | EP_STAT_MASK(TUSB_DIR_IN);
+  ep_reg = ep_add_status(ep_reg, TUSB_DIR_IN, EP_STAT_VALID);
+  ep_reg = ep_clear_ctr(ep_reg, TUSB_DIR_IN);
+
   dcd_int_disable(0);
-  pcd_set_ep_tx_status(USB, ep_ix, USB_EP_TX_VALID);
+  pcd_set_endpoint(USB, ep_ix, ep_reg);
   if (is_iso) {
     xfer->iso_in_sending = true;
   }
@@ -758,7 +753,9 @@ static bool edpt_xfer(uint8_t rhport, uint8_t ep_addr) {
     dcd_transmit_packet(xfer, ep_idx);
   } else {
     uint32_t cnt = (uint32_t) tu_min16(xfer->total_len, xfer->max_packet_size);
-    uint16_t ep_reg = pcd_get_endpoint(USB, ep_idx);
+    uint16_t ep_reg = pcd_get_endpoint(USB, ep_idx) | USB_EP_CTR_TX;
+    ep_reg &= USB_EPREG_MASK | EP_STAT_MASK(dir); // keep CTR TX, clear CTR RX
+    ep_reg = ep_add_status(ep_reg, dir, EP_STAT_VALID);
 
     if (ep_is_iso(ep_reg)) {
       btable_set_rx_bufsize(ep_idx, 0, cnt);
@@ -767,7 +764,7 @@ static bool edpt_xfer(uint8_t rhport, uint8_t ep_addr) {
       btable_set_rx_bufsize(ep_idx, BTABLE_BUF_RX, cnt);
     }
 
-    pcd_set_ep_rx_status(USB, ep_idx, USB_EP_RX_VALID);
+    pcd_set_endpoint(USB, ep_idx, ep_reg);
   }
 
   return true;
diff --git a/src/portable/st/stm32_fsdev/fsdev_common.h b/src/portable/st/stm32_fsdev/fsdev_common.h
index 73dc070ed..67ee72d0e 100644
--- a/src/portable/st/stm32_fsdev/fsdev_common.h
+++ b/src/portable/st/stm32_fsdev/fsdev_common.h
@@ -141,17 +141,20 @@ TU_VERIFY_STATIC(sizeof(fsdev_regs_t) == 0x5C, "Size is not correct");
 #endif
 
 #ifndef USB_EPTX_STAT_Pos
-#define USB_EPTX_STAT_Pos                       (4U)
+#define USB_EPTX_STAT_Pos    4u
 #endif
 
 #ifndef USB_EP_DTOG_TX_Pos
-#define USB_EP_DTOG_TX_Pos                      (6U)
+#define USB_EP_DTOG_TX_Pos   6u
 #endif
 
-#ifndef USB_EP_DTOG_RX_Pos
-#define USB_EP_DTOG_RX_Pos                      (14U)
+#ifndef USB_EP_CTR_TX_Pos
+#define USB_EP_CTR_TX_Pos    7u
 #endif
 
+
+#define EP_CTR_TXRX (USB_EP_CTR_TX | USB_EP_CTR_RX)
+
 typedef enum {
   EP_STAT_DISABLED = 0,
   EP_STAT_STALL = 1,
@@ -244,34 +247,23 @@ TU_ATTR_ALWAYS_INLINE static inline void btable_set_rx_bufsize(uint32_t ep_id, u
 // Endpoint
 //--------------------------------------------------------------------+
 
-TU_ATTR_ALWAYS_INLINE static inline void pcd_set_endpoint(USB_TypeDef * USBx, uint32_t bEpIdx, uint32_t wRegValue) {
-  (void) USBx;
-  FSDEV_REG->ep[bEpIdx].reg = (fsdev_bus_t) wRegValue;
-}
-
-TU_ATTR_ALWAYS_INLINE static inline uint32_t pcd_get_endpoint(USB_TypeDef * USBx, uint32_t bEpIdx) {
+TU_ATTR_ALWAYS_INLINE static inline void pcd_set_endpoint(USB_TypeDef * USBx, uint32_t ep_id, uint32_t value) {
   (void) USBx;
-  return FSDEV_REG->ep[bEpIdx].reg;
+  FSDEV_REG->ep[ep_id].reg = (fsdev_bus_t) value;
 }
 
-//TU_ATTR_ALWAYS_INLINE static inline void ep_write(uint32_t ep_id, uint32_t value) {
+// write ep register with clear CTR_RX and CTR_TX mask (0 is no clear)
+//TU_ATTR_ALWAYS_INLINE static inline void ep_write_with_clear_ctr(uint32_t ep_id, uint32_t value, uint32_t clear_ctr_mask) {
+//  value |= USB_EP_CTR_RX | USB_EP_CTR_TX;
+//  if (clear_ctr_mask) {
+//    value &= ~clear_ctr_mask;
+//  }
 //  FSDEV_REG->ep[ep_id].reg = (fsdev_bus_t) value;
 //}
 
-TU_ATTR_ALWAYS_INLINE static inline void pcd_set_eptype(USB_TypeDef * USBx, uint32_t bEpIdx, uint32_t wType) {
-  uint32_t regVal = pcd_get_endpoint(USBx, bEpIdx);
-  regVal &= (uint32_t)USB_EP_T_MASK;
-  regVal |= wType;
-  regVal |= USB_EP_CTR_RX | USB_EP_CTR_TX; // These clear on write0, so must set high
-  pcd_set_endpoint(USBx, bEpIdx, regVal);
-}
-
-TU_ATTR_ALWAYS_INLINE static inline void pcd_clear_tx_ep_ctr(USB_TypeDef * USBx, uint32_t bEpIdx) {
-  uint32_t regVal = pcd_get_endpoint(USBx, bEpIdx);
-  regVal &= USB_EPREG_MASK;
-  regVal &= ~USB_EP_CTR_TX;
-  regVal |= USB_EP_CTR_RX; // preserve CTR_RX (clears on writing 0)
-  pcd_set_endpoint(USBx, bEpIdx,regVal);
+TU_ATTR_ALWAYS_INLINE static inline uint32_t pcd_get_endpoint(USB_TypeDef * USBx, uint32_t ep_id) {
+  (void) USBx;
+  return FSDEV_REG->ep[ep_id].reg;
 }
 
 TU_ATTR_ALWAYS_INLINE static inline uint32_t ep_add_status(uint32_t reg, tusb_dir_t dir, ep_stat_t state) {
@@ -282,39 +274,12 @@ TU_ATTR_ALWAYS_INLINE static inline uint32_t ep_add_dtog(uint32_t reg, tusb_dir_
   return reg ^ (state << (USB_EP_DTOG_TX_Pos + (dir == TUSB_DIR_IN ? 0 : 8)));
 }
 
-TU_ATTR_ALWAYS_INLINE static inline bool ep_is_iso(uint32_t reg) {
-  return (reg & USB_EP_TYPE_MASK) == USB_EP_ISOCHRONOUS;
-}
-
-/**
-  * @brief  sets the status for tx transfer (bits STAT_TX[1:0]).
-  * @param  USBx USB peripheral instance register address.
-  * @param  bEpIdx Endpoint Number.
-  * @param  wState new state
-  * @retval None
-  */
-TU_ATTR_ALWAYS_INLINE static inline void pcd_set_ep_tx_status(USB_TypeDef * USBx,  uint32_t bEpIdx, uint32_t wState) {
-  uint32_t regVal = pcd_get_endpoint(USBx, bEpIdx);
-  regVal &= USB_EPTX_DTOGMASK;
-  regVal ^= wState;
-  regVal |= USB_EP_CTR_RX|USB_EP_CTR_TX;
-  pcd_set_endpoint(USBx, bEpIdx, regVal);
+TU_ATTR_ALWAYS_INLINE static inline uint32_t ep_clear_ctr(uint32_t reg, tusb_dir_t dir) {
+  return reg & ~(1 << (USB_EP_CTR_TX_Pos + (dir == TUSB_DIR_IN ? 0 : 8)));
 }
 
-/**
-  * @brief  sets the status for rx transfer (bits STAT_TX[1:0])
-  * @param  USBx USB peripheral instance register address.
-  * @param  bEpIdx Endpoint Number.
-  * @param  wState new state
-  * @retval None
-  */
-
-TU_ATTR_ALWAYS_INLINE static inline void pcd_set_ep_rx_status(USB_TypeDef * USBx,  uint32_t bEpIdx, uint32_t wState) {
-  uint32_t regVal = pcd_get_endpoint(USBx, bEpIdx);
-  regVal &= USB_EPRX_DTOGMASK;
-  regVal ^= wState;
-  regVal |= USB_EP_CTR_RX|USB_EP_CTR_TX;
-  pcd_set_endpoint(USBx, bEpIdx, regVal);
+TU_ATTR_ALWAYS_INLINE static inline bool ep_is_iso(uint32_t reg) {
+  return (reg & USB_EP_TYPE_MASK) == USB_EP_ISOCHRONOUS;
 }
 
 #ifdef __cplusplus