Skip to content

Commit

Permalink
add sndfifo owner info to skip rewriting data for retrying NAKed
Browse files Browse the repository at this point in the history
  • Loading branch information
@hathach
hathach committed Aug 26, 2024
1 parent c7851e8 commit cbc92bd
Show file tree
Hide file tree
Showing 3 changed files with 127 additions and 74 deletions.
8 changes: 7 additions & 1 deletion hw/bsp/family_support.cmake
View file
Original file line number Diff line number Diff line change
Expand Up @@ -236,7 +236,7 @@ function(family_configure_common TARGET RTOS)
if (NOT TARGET segger_rtt)
add_library(segger_rtt STATIC ${TOP}/lib/SEGGER_RTT/RTT/SEGGER_RTT.c)
target_include_directories(segger_rtt PUBLIC ${TOP}/lib/SEGGER_RTT/RTT)
# target_compile_definitions(segger_rtt PUBLIC SEGGER_RTT_MODE_DEFAULT=SEGGER_RTT_MODE_BLOCK_IF_FIFO_FULL)
target_compile_definitions(segger_rtt PUBLIC SEGGER_RTT_MODE_DEFAULT=SEGGER_RTT_MODE_BLOCK_IF_FIFO_FULL)
endif()
target_link_libraries(${TARGET} PUBLIC segger_rtt)
endif ()
Expand Down Expand Up @@ -418,6 +418,12 @@ exit"
COMMAND ${JLINKEXE} -device ${JLINK_DEVICE} ${OPTION_LIST} -if ${JLINK_IF} -JTAGConf -1,-1 -speed auto -CommandFile $<TARGET_FILE_DIR:${TARGET}>/${TARGET}.jlink
VERBATIM
)

# optional flash post build
# add_custom_command(TARGET ${TARGET} POST_BUILD
# COMMAND ${JLINKEXE} -device ${JLINK_DEVICE} ${OPTION_LIST} -if ${JLINK_IF} -JTAGConf -1,-1 -speed auto -CommandFile $<TARGET_FILE_DIR:${TARGET}>/${TARGET}.jlink
# VERBATIM
# )
endfunction()


Expand Down
2 changes: 1 addition & 1 deletion src/host/usbh.h
View file
Original file line number Diff line number Diff line change
Expand Up @@ -81,7 +81,7 @@ enum {
};

typedef struct {
uint8_t max_nak; // max NAK per endpoint per frame
uint8_t max_nak; // max NAK per endpoint per frame to save CPU/SPI bus usage
uint8_t cpuctl; // R16: CPU Control Register
uint8_t pinctl; // R17: Pin Control Register. FDUPSPI bit is ignored
} tuh_configure_max3421_t;
Expand Down
191 changes: 119 additions & 72 deletions src/portable/analog/max3421/hcd_max3421.c
View file
Original file line number Diff line number Diff line change
Expand Up @@ -168,32 +168,35 @@ enum {
};

enum {
MAX_NAK_DEFAULT = 1 // Number of NAK per endpoint per usb frame
MAX_NAK_DEFAULT = 1 // Number of NAK per endpoint per usb frame to save CPU/SPI bus usage
};

enum {
EP_STATE_IDLE = 0,
EP_STATE_COMPLETE = 1,
EP_STATE_ABORTING = 2,
EP_STATE_ATTEMPT_1 = 3, // pending 1st attempt
EP_STATE_ATTEMPT_1 = 3, // Number of attempts to transfer in a frame. Incremented after each NAK
EP_STATE_ATTEMPT_MAX = 15
};

//--------------------------------------------------------------------+
//
//--------------------------------------------------------------------+

typedef struct TU_ATTR_PACKED {
uint8_t ep_num : 4;
uint8_t is_setup : 1;
uint8_t is_out : 1;
uint8_t is_iso : 1;
} hxfr_bm_t;

TU_VERIFY_STATIC(sizeof(hxfr_bm_t) == 1, "size is not correct");

typedef struct {
uint8_t daddr;

union { ;
struct TU_ATTR_PACKED {
uint8_t ep_num : 4;
uint8_t is_setup : 1;
uint8_t is_out : 1;
uint8_t is_iso : 1;
}hxfr_bm;

union {
hxfr_bm_t hxfr_bm;
uint8_t hxfr;
};

Expand All @@ -203,13 +206,12 @@ typedef struct {
uint16_t packet_size : 11;
};

bool received_nak;
uint16_t total_len;
uint16_t xferred_len;
uint8_t* buf;
} max3421_ep_t;

TU_VERIFY_STATIC(sizeof(max3421_ep_t) == 16, "size is not correct");
TU_VERIFY_STATIC(sizeof(max3421_ep_t) == 12, "size is not correct");

typedef struct {
volatile uint16_t frame_count;
Expand All @@ -220,7 +222,16 @@ typedef struct {
uint8_t hien;
uint8_t mode;
uint8_t peraddr;
uint8_t hxfr;
union {
hxfr_bm_t hxfr_bm;
uint8_t hxfr;
};

// owner of data in SNDFIFO, for retrying NAKed without re-writing to FIFO
struct {
uint8_t daddr;
uint8_t hxfr;
}sndfifo_owner;

atomic_flag busy; // busy transferring

Expand Down Expand Up @@ -318,33 +329,9 @@ bool tuh_max3421_reg_write(uint8_t rhport, uint8_t reg, uint8_t data, bool in_is
return ret;
}

static void fifo_write(uint8_t rhport, uint8_t reg, uint8_t const * buffer, uint16_t len, bool in_isr) {
uint8_t hirq;
reg |= CMDBYTE_WRITE;

max3421_spi_lock(rhport, in_isr);

tuh_max3421_spi_xfer_api(rhport, &reg, &hirq, 1);
_hcd_data.hirq = hirq;
tuh_max3421_spi_xfer_api(rhport, buffer, NULL, len);

max3421_spi_unlock(rhport, in_isr);
}

static void fifo_read(uint8_t rhport, uint8_t * buffer, uint16_t len, bool in_isr) {
uint8_t hirq;
uint8_t const reg = RCVVFIFO_ADDR;

max3421_spi_lock(rhport, in_isr);

tuh_max3421_spi_xfer_api(rhport, &reg, &hirq, 1);
_hcd_data.hirq = hirq;
tuh_max3421_spi_xfer_api(rhport, NULL, buffer, len);

max3421_spi_unlock(rhport, in_isr);
}

//------------- register write helper -------------//
//--------------------------------------------------------------------
// Register helper
//--------------------------------------------------------------------
TU_ATTR_ALWAYS_INLINE static inline void hirq_write(uint8_t rhport, uint8_t data, bool in_isr) {
reg_write(rhport, HIRQ_ADDR, data, in_isr);
// HIRQ write 1 is clear
Expand Down Expand Up @@ -378,6 +365,47 @@ TU_ATTR_ALWAYS_INLINE static inline void sndbc_write(uint8_t rhport, uint8_t dat
reg_write(rhport, SNDBC_ADDR, data, in_isr);
}

//--------------------------------------------------------------------
// FIFO access (receive, send, setup)
//--------------------------------------------------------------------
static void hwfifo_write(uint8_t rhport, uint8_t reg, const uint8_t* buffer, uint16_t len, bool in_isr) {
uint8_t hirq;
reg |= CMDBYTE_WRITE;

max3421_spi_lock(rhport, in_isr);

tuh_max3421_spi_xfer_api(rhport, &reg, &hirq, 1);
_hcd_data.hirq = hirq;
tuh_max3421_spi_xfer_api(rhport, buffer, NULL, len);

max3421_spi_unlock(rhport, in_isr);
}

// Write to SNDFIFO if len > 0 and update SNDBC
TU_ATTR_ALWAYS_INLINE static inline void hwfifo_send(uint8_t rhport, const uint8_t* buffer, uint16_t len, bool in_isr) {
if (len) {
hwfifo_write(rhport, SNDFIFO_ADDR, buffer, len, in_isr);
}
sndbc_write(rhport, len, in_isr);
}

TU_ATTR_ALWAYS_INLINE static inline void hwfifo_setup(uint8_t rhport, const uint8_t* buffer, bool in_isr) {
hwfifo_write(rhport, SUDFIFO_ADDR, buffer, 8, in_isr);
}

static void hwfifo_receive(uint8_t rhport, uint8_t * buffer, uint16_t len, bool in_isr) {
uint8_t hirq;
uint8_t const reg = RCVVFIFO_ADDR;

max3421_spi_lock(rhport, in_isr);

tuh_max3421_spi_xfer_api(rhport, &reg, &hirq, 1);
_hcd_data.hirq = hirq;
tuh_max3421_spi_xfer_api(rhport, NULL, buffer, len);

max3421_spi_unlock(rhport, in_isr);
}

//--------------------------------------------------------------------+
// Endpoint helper
//--------------------------------------------------------------------+
Expand Down Expand Up @@ -418,7 +446,7 @@ static void free_ep(uint8_t daddr) {
}
}

// Check if endpoint has an queued transfer and not reach max NAK
// Check if endpoint has a queued transfer and not reach max NAK in this frame
TU_ATTR_ALWAYS_INLINE static inline bool is_ep_pending(max3421_ep_t const * ep) {
uint8_t const state = ep->state;
return ep->packet_size && (state >= EP_STATE_ATTEMPT_1) &&
Expand Down Expand Up @@ -488,6 +516,7 @@ bool hcd_init(uint8_t rhport) {
reg_write(rhport, PINCTL_ADDR, _tuh_cfg.pinctl | PINCTL_FDUPSPI, false);

// v1 is 0x01, v2 is 0x12, v3 is 0x13
// Note: v1 and v2 has host OUT errata whose workaround is not implemented in this driver
uint8_t const revision = reg_read(rhport, REVISION_ADDR, false);
TU_LOG2_HEX(revision);
TU_ASSERT(revision == 0x01 || revision == 0x12 || revision == 0x13, false);
Expand Down Expand Up @@ -616,27 +645,44 @@ bool hcd_edpt_open(uint8_t rhport, uint8_t daddr, tusb_desc_endpoint_t const * e
return true;
}

/* The microcontroller repeatedly writes the SNDFIFO register R2 to load the FIFO with up to 64 data bytes.
* Then the microcontroller writes the SNDBC register, which this does three things:
* 1. Tells the MAX3421E SIE (Serial Interface Engine) how many bytes in the FIFO to send.
* 2. Connects the SNDFIFO and SNDBC register to the USB logic for USB transmission.
* 3. Clears the SNDBAVIRQ interrupt flag. If the second FIFO is available for µC loading, the SNDBAVIRQ immediately re-asserts.
+-----------+
--->| SNDBC-A |
/ | SNDFIFO-A |
/ +-----------+
+------+ +-------------+ / +----------+
| MCU |------>| R2: SNDFIFO |---- << Write R7 Flip >> ---| MAX3241E |
|(hcd) | | R7: SNDBC | / | SIE |
+------+ +-------------+ / +----------+
+-----------+ /
| SNDBC-B | /
| SNDFIFO-B |<---
+-----------+
Note: xact_out() is called when starting a new transfer, continue a transfer (isr) or retry a transfer (NAK)
For NAK retry, we do not need to write to FIFO or SNDBC register again.
*/
static void xact_out(uint8_t rhport, max3421_ep_t *ep, bool switch_ep, bool in_isr) {
// Page 12: Programming BULK-OUT Transfers
// TODO double buffered
// TODO: double buffering for ISO transfer
if (switch_ep) {
peraddr_write(rhport, ep->daddr, in_isr);

uint8_t const hctl = (ep->data_toggle ? HCTL_SNDTOG1 : HCTL_SNDTOG0);
const uint8_t hctl = (ep->data_toggle ? HCTL_SNDTOG1 : HCTL_SNDTOG0);
reg_write(rhport, HCTL_ADDR, hctl, in_isr);
}

if (!ep->received_nak){
// do not write to fifo or sdnbc register again if previous attempt got NAK
uint8_t const xact_len = (uint8_t) tu_min16(ep->total_len - ep->xferred_len, ep->packet_size);
if (xact_len) {
// only check SNDBAV IRQ if there is data to send
TU_ASSERT(_hcd_data.hirq & HIRQ_SNDBAV_IRQ,);
fifo_write(rhport, SNDFIFO_ADDR, ep->buf, xact_len, in_isr);
}

sndbc_write(rhport, xact_len, in_isr);
// Only write to sndfifo and sdnbc register if it is not a NAKed retry
if (!(ep->daddr == _hcd_data.sndfifo_owner.daddr && ep->hxfr == _hcd_data.sndfifo_owner.hxfr)) {
// skip SNDBAV IRQ check, overwrite sndfifo if needed
const uint8_t xact_len = (uint8_t) tu_min16(ep->total_len - ep->xferred_len, ep->packet_size);
hwfifo_send(rhport, ep->buf, xact_len, in_isr);
}
_hcd_data.sndfifo_owner.daddr = ep->daddr;
_hcd_data.sndfifo_owner.hxfr = ep->hxfr;

hxfr_write(rhport, ep->hxfr, in_isr);
}
Expand All @@ -655,7 +701,7 @@ static void xact_in(uint8_t rhport, max3421_ep_t *ep, bool switch_ep, bool in_is

static void xact_setup(uint8_t rhport, max3421_ep_t *ep, bool in_isr) {
peraddr_write(rhport, ep->daddr, in_isr);
fifo_write(rhport, SUDFIFO_ADDR, ep->buf, 8, in_isr);
hwfifo_setup(rhport, ep->buf, in_isr);
hxfr_write(rhport, HXFR_SETUP, in_isr);
}

Expand All @@ -669,7 +715,7 @@ static void xact_generic(uint8_t rhport, max3421_ep_t *ep, bool switch_ep, bool

// status
if (ep->buf == NULL || ep->total_len == 0) {
uint8_t const hxfr = (uint8_t) (HXFR_HS | (ep->hxfr & HXFR_OUT_NIN));
const uint8_t hxfr = (uint8_t) (HXFR_HS | (ep->hxfr & HXFR_OUT_NIN));
peraddr_write(rhport, ep->daddr, in_isr);
hxfr_write(rhport, hxfr, in_isr);
return;
Expand Down Expand Up @@ -830,11 +876,11 @@ static void xfer_complete_isr(uint8_t rhport, max3421_ep_t *ep, xfer_result_t re
}

static void handle_xfer_done(uint8_t rhport, bool in_isr) {
uint8_t const hrsl = reg_read(rhport, HRSL_ADDR, in_isr);
uint8_t const hresult = hrsl & HRSL_RESULT_MASK;
uint8_t const ep_num = _hcd_data.hxfr & HXFR_EPNUM_MASK;
uint8_t const hxfr_type = _hcd_data.hxfr & 0xf0;
uint8_t const ep_dir = ((hxfr_type & HXFR_SETUP) || (hxfr_type & HXFR_OUT_NIN)) ? 0 : 1;
const uint8_t hrsl = reg_read(rhport, HRSL_ADDR, in_isr);
const uint8_t hresult = hrsl & HRSL_RESULT_MASK;
const uint8_t ep_num = _hcd_data.hxfr_bm.ep_num;
const uint8_t hxfr_type = _hcd_data.hxfr & 0xf0;
const uint8_t ep_dir = ((hxfr_type & HXFR_SETUP) || (hxfr_type & HXFR_OUT_NIN)) ? 0 : 1;

max3421_ep_t *ep = find_opened_ep(_hcd_data.peraddr, ep_num, ep_dir);
TU_VERIFY(ep, );
Expand All @@ -849,9 +895,9 @@ static void handle_xfer_done(uint8_t rhport, bool in_isr) {
// control endpoint -> retry immediately and return
hxfr_write(rhport, _hcd_data.hxfr, in_isr);
return;
} else if (EP_STATE_ATTEMPT_1 <= ep->state && ep->state < EP_STATE_ATTEMPT_MAX) {
}
if (EP_STATE_ATTEMPT_1 <= ep->state && ep->state < EP_STATE_ATTEMPT_MAX) {
ep->state++;
ep->received_nak = true;
}
}

Expand All @@ -861,7 +907,6 @@ static void handle_xfer_done(uint8_t rhport, bool in_isr) {
hxfr_write(rhport, _hcd_data.hxfr, in_isr);
} else if (next_ep) {
// switch to next pending endpoint
// TODO could have issue with double buffered if not clear previously out data
xact_generic(rhport, next_ep, true, in_isr);
} else {
// no more pending in this frame -> clear busy
Expand All @@ -875,7 +920,6 @@ static void handle_xfer_done(uint8_t rhport, bool in_isr) {

case HRSL_SUCCESS:
xfer_result = XFER_RESULT_SUCCESS;
ep->received_nak = false;
break;

case HRSL_STALL:
Expand Down Expand Up @@ -905,11 +949,15 @@ static void handle_xfer_done(uint8_t rhport, bool in_isr) {
if (ep->state == EP_STATE_COMPLETE) {
xfer_complete_isr(rhport, ep, xfer_result, hrsl, in_isr);
}else {
// more to transfer
hxfr_write(rhport, _hcd_data.hxfr, in_isr);
hxfr_write(rhport, _hcd_data.hxfr, in_isr); // more to transfer
}
} else {
// SETUP or OUT transfer

// clear sndfifo owner since data is sent
_hcd_data.sndfifo_owner.daddr = 0xff;
_hcd_data.sndfifo_owner.hxfr = 0xff;

uint8_t xact_len;

if (hxfr_type & HXFR_SETUP) {
Expand All @@ -926,8 +974,7 @@ static void handle_xfer_done(uint8_t rhport, bool in_isr) {
if (xact_len < ep->packet_size || ep->xferred_len >= ep->total_len) {
xfer_complete_isr(rhport, ep, xfer_result, hrsl, in_isr);
} else {
// more to transfer
xact_out(rhport, ep, false, in_isr);
xact_out(rhport, ep, false, in_isr); // more to transfer
}
}
}
Expand Down Expand Up @@ -987,16 +1034,16 @@ void hcd_int_handler(uint8_t rhport, bool in_isr) {
// not call this handler again. So we need to loop until all IRQ are cleared
while (hirq & (HIRQ_RCVDAV_IRQ | HIRQ_HXFRDN_IRQ)) {
if (hirq & HIRQ_RCVDAV_IRQ) {
uint8_t const ep_num = _hcd_data.hxfr & HXFR_EPNUM_MASK;
const uint8_t ep_num = _hcd_data.hxfr_bm.ep_num;
max3421_ep_t* ep = find_opened_ep(_hcd_data.peraddr, ep_num, 1);
uint8_t xact_len = 0;

// RCVDAV_IRQ can trigger 2 times (dual buffered)
while (hirq & HIRQ_RCVDAV_IRQ) {
uint8_t rcvbc = reg_read(rhport, RCVBC_ADDR, in_isr);
const uint8_t rcvbc = reg_read(rhport, RCVBC_ADDR, in_isr);
xact_len = (uint8_t) tu_min16(rcvbc, ep->total_len - ep->xferred_len);
if (xact_len) {
fifo_read(rhport, ep->buf, xact_len, in_isr);
hwfifo_receive(rhport, ep->buf, xact_len, in_isr);
ep->buf += xact_len;
ep->xferred_len += xact_len;
}
Expand Down

0 comments on commit cbc92bd

Please sign in to comment.