Merge pull request #2731 from cumhuronat/master

Fix: Properly Handle NAK Response in MAX3421E driver
This commit is contained in:
Ha Thach 2024-08-30 18:10:15 +07:00 committed by GitHub
commit 29e025cbf5
No known key found for this signature in database
GPG Key ID: B5690EEEBB952194
4 changed files with 127 additions and 65 deletions

View File

@ -197,7 +197,7 @@ Docs
- `Structure`_
- `Porting`_
.. |Build Status| image:: https://github.com/hathach/tinyusb/actions/workflows/cmake_arm.yml/badge.svg
.. |Build Status| image:: https://github.com/hathach/tinyusb/actions/workflows/build.yml/badge.svg
:target: https://github.com/hathach/tinyusb/actions
.. |CircleCI Status| image:: https://dl.circleci.com/status-badge/img/circleci/4AYHvUhFxdnY4rA7LEsdqW/QmrpoL2AjGqetvFQNqtWyq/tree/master.svg?style=svg
:target: https://dl.circleci.com/status-badge/redirect/circleci/4AYHvUhFxdnY4rA7LEsdqW/QmrpoL2AjGqetvFQNqtWyq/tree/master

View File

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

View File

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

View File

@ -168,14 +168,14 @@ 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
};
@ -183,17 +183,20 @@ enum {
//
//--------------------------------------------------------------------+
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;
};
@ -219,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
@ -317,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
@ -377,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, uint8_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, uint8_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
//--------------------------------------------------------------------+
@ -417,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) &&
@ -487,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);
@ -615,22 +645,45 @@ 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);
}
uint8_t const xact_len = (uint8_t) tu_min16(ep->total_len - ep->xferred_len, ep->packet_size);
TU_ASSERT(_hcd_data.hirq & HIRQ_SNDBAV_IRQ,);
if (xact_len) {
fifo_write(rhport, SNDFIFO_ADDR, ep->buf, 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);
}
sndbc_write(rhport, 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);
}
@ -648,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);
}
@ -662,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;
@ -823,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, );
@ -842,7 +895,8 @@ 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++;
}
}
@ -853,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
@ -896,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) {
@ -917,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
}
}
}
@ -978,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;
}