fix(ci_fs): address code-review findings in host/device drivers

Host (hcd_ci_fs.c):
- Release the speculatively-armed sibling BDT on the NAK path (IN only) as
  well as on completion, so a NAKed multi-packet IN no longer leaks a BDT
  that stays own=1 and blocks every same-direction pipe. Both paths now go
  through a single release_sibling_bd() helper (was a copy-pasted disarm).
- Clear the ENTIRE shared BDT (both directions) on bus reset; clearing only
  the IN half left a stale OUT/SETUP descriptor after a disconnect mid-OUT,
  blocking the first control transfer on re-enumeration.
- Size bda[] to span the whole BDT (2*2*4) so STAT-indexed access is within
  the declared array bounds (was out-of-declared-bounds, benign via union).

Shared (ci_fs_type.h):
- Hoist buffer_descriptor_t and the TOK_PID enum out of the device and host
  drivers into the shared header so the identical definitions cannot drift.

Board (kinetis_k):
- Drop a redundant local in board_get_unique_id.

Build-verified: host + kinetis k/kl/k32l + MCX. HIL: frdm_k64f host 2/2
(cdc_msc_hid + device_info); frdm_kl25z device core suite green with the
relocated definitions.

4 files changed, 88 insertions, 95 deletions

diff --git a/hw/bsp/kinetis_k/family.c b/hw/bsp/kinetis_k/family.c
index 238123f30..2df3313b8 100644
--- a/hw/bsp/kinetis_k/family.c
+++ b/hw/bsp/kinetis_k/family.c
@@ -163,14 +163,13 @@ size_t board_get_unique_id(uint8_t id[], size_t max_len) {
   (void) max_len;
   // Kinetis 128-bit Unique Identification Register (SIM->UIDH/UIDMH/UIDML/UIDL)
   uint32_t* id32 = (uint32_t*) (uintptr_t) id;
-  uint8_t const len = 16;
 
   id32[0] = SIM->UIDH;
   id32[1] = SIM->UIDMH;
   id32[2] = SIM->UIDML;
   id32[3] = SIM->UIDL;
 
-  return len;
+  return 16;
 }
 
 #if CFG_TUSB_OS == OPT_OS_NONE
diff --git a/src/portable/chipidea/ci_fs/ci_fs_type.h b/src/portable/chipidea/ci_fs/ci_fs_type.h
index a525c96ca..857a75253 100644
--- a/src/portable/chipidea/ci_fs/ci_fs_type.h
+++ b/src/portable/chipidea/ci_fs/ci_fs_type.h
@@ -27,6 +27,60 @@ extern "C" {
 // align 4 is used to get rid of reserved fields
 #define _va32     volatile TU_ATTR_ALIGNED(4)
 
+//--------------------------------------------------------------------+
+// Buffer Descriptor Table (BDT) - shared by the device (dcd) and host (hcd) drivers
+// since both target the same ChipIdea-FS silicon. Keep the layout in one place so a
+// fix cannot silently drift between the two drivers.
+//--------------------------------------------------------------------+
+
+// Token PID values reported in the BDT tok_pid field / written to the TOKEN register.
+// The device driver only uses OUT/IN/SETUP; the rest are host-only.
+enum {
+  TOK_PID_OUT   = 0x1u,
+  TOK_PID_IN    = 0x9u,
+  TOK_PID_SETUP = 0xDu,
+  TOK_PID_DATA0 = 0x3u,
+  TOK_PID_DATA1 = 0xbu,
+  TOK_PID_ACK   = 0x2u,
+  TOK_PID_STALL = 0xeu,
+  TOK_PID_NAK   = 0xau,
+  TOK_PID_BUSTO = 0x0u,
+  TOK_PID_ERR   = 0xfu,
+};
+
+// Note: this header is included before the CMSIS device header, so use plain `volatile`
+// rather than CMSIS `__IO`.
+typedef struct TU_ATTR_PACKED
+{
+  union {
+    uint32_t head;
+    struct {
+      union {
+        struct {
+                   uint16_t           :  2;
+          volatile uint16_t tok_pid   :  4;
+                   uint16_t data      :  1;
+          volatile uint16_t own       :  1;
+                   uint16_t           :  8;
+        };
+        struct {
+                   uint16_t           :  2;
+                   uint16_t bdt_stall :  1;
+                   uint16_t dts       :  1;
+                   uint16_t ninc      :  1;
+                   uint16_t keep      :  1;
+                   uint16_t           : 10;
+        };
+      };
+      volatile uint16_t bc : 10;
+               uint16_t    :  6;
+    };
+  };
+  uint8_t *addr;
+}buffer_descriptor_t;
+
+TU_VERIFY_STATIC( sizeof(buffer_descriptor_t) == 8, "size is not correct" );
+
 typedef struct {
   _va32 uint8_t PER_ID;                 // [00] Peripheral ID register
   _va32 uint8_t ID_COMP;                // [04] Peripheral ID complement register
diff --git a/src/portable/chipidea/ci_fs/dcd_ci_fs.c b/src/portable/chipidea/ci_fs/dcd_ci_fs.c
index b03670551..295f2e578 100644
--- a/src/portable/chipidea/ci_fs/dcd_ci_fs.c
+++ b/src/portable/chipidea/ci_fs/dcd_ci_fs.c
@@ -24,43 +24,7 @@
 //--------------------------------------------------------------------+
 // MACRO TYPEDEF CONSTANT ENUM DECLARATION
 //--------------------------------------------------------------------+
-
-enum {
-  TOK_PID_OUT   = 0x1u,
-  TOK_PID_IN    = 0x9u,
-  TOK_PID_SETUP = 0xDu,
-};
-
-typedef struct TU_ATTR_PACKED
-{
-  union {
-    uint32_t head;
-    struct {
-      union {
-        struct {
-               uint16_t           :  2;
-          __IO uint16_t tok_pid   :  4;
-               uint16_t data      :  1;
-          __IO uint16_t own       :  1;
-               uint16_t           :  8;
-        };
-        struct {
-               uint16_t           :  2;
-               uint16_t bdt_stall :  1;
-               uint16_t dts       :  1;
-               uint16_t ninc      :  1;
-               uint16_t keep      :  1;
-               uint16_t           : 10;
-        };
-      };
-      __IO uint16_t bc : 10;
-           uint16_t    :  6;
-    };
-  };
-  uint8_t *addr;
-}buffer_descriptor_t;
-
-TU_VERIFY_STATIC( sizeof(buffer_descriptor_t) == 8, "size is not correct" );
+// TOK_PID_* and buffer_descriptor_t are shared with the host driver in ci_fs_type.h
 
 typedef struct TU_ATTR_PACKED
 {
diff --git a/src/portable/chipidea/ci_fs/hcd_ci_fs.c b/src/portable/chipidea/ci_fs/hcd_ci_fs.c
index 5c5d81521..a6f5405b5 100644
--- a/src/portable/chipidea/ci_fs/hcd_ci_fs.c
+++ b/src/portable/chipidea/ci_fs/hcd_ci_fs.c
@@ -31,50 +31,7 @@
 //--------------------------------------------------------------------+
 // MACRO TYPEDEF CONSTANT ENUM DECLARATION
 //--------------------------------------------------------------------+
-
-enum {
-  TOK_PID_OUT   = 0x1u,
-  TOK_PID_IN    = 0x9u,
-  TOK_PID_SETUP = 0xDu,
-  TOK_PID_DATA0 = 0x3u,
-  TOK_PID_DATA1 = 0xbu,
-  TOK_PID_ACK   = 0x2u,
-  TOK_PID_STALL = 0xeu,
-  TOK_PID_NAK   = 0xau,
-  TOK_PID_BUSTO = 0x0u,
-  TOK_PID_ERR   = 0xfu,
-};
-
-typedef struct TU_ATTR_PACKED
-{
-  union {
-    uint32_t head;
-    struct {
-      union {
-        struct {
-               uint16_t           :  2;
-          __IO uint16_t tok_pid   :  4;
-               uint16_t data      :  1;
-          __IO uint16_t own       :  1;
-               uint16_t           :  8;
-        };
-        struct {
-               uint16_t           :  2;
-               uint16_t bdt_stall :  1;
-               uint16_t dts       :  1;
-               uint16_t ninc      :  1;
-               uint16_t keep      :  1;
-               uint16_t           : 10;
-        };
-      };
-      __IO uint16_t bc : 10;
-           uint16_t    :  6;
-    };
-  };
-  uint8_t *addr;
-}buffer_descriptor_t;
-
-TU_VERIFY_STATIC( sizeof(buffer_descriptor_t) == 8, "size is not correct" );
+// TOK_PID_* and buffer_descriptor_t are shared with the device driver in ci_fs_type.h
 
 typedef struct TU_ATTR_PACKED
 {
@@ -115,7 +72,10 @@ typedef struct
   union {
     /* [OUT,IN][EVEN,ODD] */
     buffer_descriptor_t bdt[2][2];
-    uint16_t            bda[2*2];
+    /* bda aliases bdt for STAT-register indexing: STAT gives the byte-offset/2 of the
+     * completed BD, so it indexes bda[] in uint16_t units. Each buffer_descriptor_t is
+     * 4 uint16_t, hence 2*2*4 elements to span the whole table (must equal sizeof bdt). */
+    uint16_t            bda[2*2*4];
   };
   endpoint_state_t endpoint[2];
   pipe_state_t pipe[CFG_TUH_ENDPOINT_MAX * 2];
@@ -282,6 +242,22 @@ static void suspend_transfer(int pipenum, buffer_descriptor_t *bd)
   }
 }
 
+// Release the speculatively-armed sibling BDT of a multi-packet transfer.
+// prepare_packets arms the sibling (odd^1) BDT (own=1) so a multi-packet transfer can
+// ping-pong without NAKs. When the transfer ends - completes early on a short IN packet,
+// stalls/errors, or (for IN) is NAKed before the sibling's token is issued - that sibling
+// is left owned by the SIE. Because the host shares ONE BDT set across all pipes, a
+// leftover armed sibling blocks every other pipe forever (e.g. a 2nd device stuck
+// enumerating behind a hub). Release it - but ONLY for a multi-packet transfer: a
+// single-packet transfer never armed a sibling, so that BDT slot may legitimately belong
+// to another pipe's in-flight transfer.
+static inline void release_sibling_bd(unsigned s, const pipe_state_t *pipe)
+{
+  if (pipe->length > pipe->max_packet_size) {
+    ((buffer_descriptor_t *)&_hcd.bda[s ^ USB_STAT_ODD_MASK])->own = 0;
+  }
+}
+
 static void process_tokdne(uint8_t rhport)
 {
   (void)rhport;
@@ -316,6 +292,12 @@ static void process_tokdne(uint8_t rhport)
       result = XFER_RESULT_SUCCESS;
       break;
     case TOK_PID_NAK:
+      // Release the speculatively-armed sibling so the deferred retry (and any other pipe
+      // sharing the single BDT) can claim it; otherwise it stays own=1 forever and every
+      // same-direction transfer wedges. IN only: an IN issues just one token so the sibling
+      // was never put on the wire, whereas an OUT issues both tokens and its sibling may
+      // still be in flight - touching it there would race the SIE write-back.
+      if (TUSB_DIR_IN == dir_in) release_sibling_bd(s, &_hcd.pipe[pipenum]);
       suspend_transfer(pipenum, bd);
       next_pipenum = select_next_pipenum(pipenum);
       if (0 <= next_pipenum)
@@ -331,16 +313,7 @@ static void process_tokdne(uint8_t rhport)
   }
   _hcd.in_progress  &= ~TU_BIT(pipenum);
   pipe_state_t *pipe = &_hcd.pipe[ep->pipenum];
-  /* A multi-packet transfer speculatively arms the sibling (odd^1) BDT (see
-   * prepare_packets) to ping-pong without NAKs. When it ends early (a short IN packet)
-   * or fails, that sibling is still owned by the SIE; since the host shares a single
-   * BDT set across all pipes, a leftover armed sibling blocks every other pipe forever
-   * (e.g. a 2nd device stuck enumerating behind a hub). Release it - but ONLY for a
-   * multi-packet transfer: a single-packet transfer never armed a sibling, so that
-   * BDT slot may legitimately belong to another pipe's in-flight transfer. */
-  if (pipe->length > pipe->max_packet_size) {
-    ((buffer_descriptor_t *)&_hcd.bda[s ^ USB_STAT_ODD_MASK])->own = 0;
-  }
+  release_sibling_bd(s, pipe);
   hcd_event_xfer_complete(pipe->dev_addr,
                           tu_edpt_addr(CI_REG->TOKEN & USB_TOKEN_TOKENENDPT_MASK, dir_in),
                           pipe->length - pipe->remaining,
@@ -373,8 +346,11 @@ static void process_bus_reset(uint8_t rhport)
 
   _hcd.in_progress = 0;
   _hcd.pending     = 0;
+  // Clear the ENTIRE shared BDT (both directions, both even/odd). Clearing only the IN
+  // pair left a stale OUT/SETUP descriptor (own=1) after a disconnect mid-OUT, which then
+  // blocks the first control transfer on re-enumeration.
   buffer_descriptor_t *bd = &_hcd.bdt[0][0];
-  for (unsigned i = 0; i < 2; ++i, ++bd) {
+  for (unsigned i = 0; i < 2 * 2; ++i, ++bd) {
     bd->head = 0;
   }
 }