mirror/tinyusb
3
0
Fork 0
mirror of https://github.com/hathach/tinyusb.git synced 2026-08-18 11:02:16 +00:00
Code Issues Packages Projects Releases Wiki Activity

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

Browse Source 
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.
This commit is contained in:
hathach
2026-07-18 00:18:27 +07:00
parent 7d7444bd89
commit 4782770e7f
4 changed files with 88 additions and 95 deletions
Download Patch File Download Diff File Expand all files Collapse all files

3
hw/bsp/kinetis_k/family.c
View File

@ -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

54
src/portable/chipidea/ci_fs/ci_fs_type.h
View File

@ -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

38
src/portable/chipidea/ci_fs/dcd_ci_fs.c
View File

@ -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
{

88
src/portable/chipidea/ci_fs/hcd_ci_fs.c
View File

@ -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;
}
}