This is an automated email from the ASF dual-hosted git repository. xiaoxiang781216 pushed a commit to branch master in repository https://gitbox.apache.org/repos/asf/nuttx.git
commit 408c801ee4aaddd15101be3190aa8ed0164480f3 Author: Alan Carvalho de Assis <[email protected]> AuthorDate: Thu Oct 22 15:58:31 2026 -0300 drivers/usbhost: Fix MAX3421E host driver and HID mouse The MAX3421E driver did not build with assertions disabled and, once built, could not enumerate a device. These issues were found bringing it up on a stm32f103-minimum board with a Logitech Unifying receiver: - Make the "locked" debug field unconditional: DEBUGASSERT() still references it when CONFIG_DEBUG_ASSERTIONS is disabled. - Fix the inverted DEBUGASSERT in max3421e_chan_wakeup(): a transfer in progress has result == EBUSY. - Check the "locked" flag only after taking the SPI lock and clear it before releasing the lock, otherwise threads racing for the bus trigger false assertions. - Release the SPI lock on the getspeed() error path of enumerate() and drop the extra mutex unlock in getspeed(). - CONNIRQ is also raised by our own bus resets. Sample the bus and only act on a real attach/detach instead of resetting the bus again, which looped forever, and discard the CONNIRQ caused by the bus reset. - Register the waiter in the control status stages, and complete the HS-IN handshake on HXFRDNIRQ: a zero length packet never asserts RCVDAVIRQ. - Clear RCVDAVIRQ only after reading the RCVFIFO. Clearing it releases the buffer and switches the double-buffered FIFO to the next packet. - Data toggles: start every control DATA stage with DATA1, save the toggles from HRSL (the HCTL bits are write-only strobes), restore them with a plain write (a read-modify-write of HCTL can re-trigger other strobes) and also restore them before IN transfers. - Clear a stale HXFRDNIRQ before launching a transfer, otherwise it is taken as the completion of the new transfer, HXFRDNIE gets disabled and the transfer never completes. - Reserve channel 0 for EP0 of the root hub so that max3421e_epalloc() does not hand it out and redirect control transfers to another endpoint. - Do not spin forever waiting for OSCOKIRQ during initialization. Signed-off-by: Alan C. Assis <[email protected]> Assisted-by: Claude Code Opus 5.5 --- drivers/usbhost/usbhost_max3421e.c | 222 ++++++++++++++++++++++++++++--------- 1 file changed, 171 insertions(+), 51 deletions(-) diff --git a/drivers/usbhost/usbhost_max3421e.c b/drivers/usbhost/usbhost_max3421e.c index 4368d6d7c09..ea33b694d2d 100644 --- a/drivers/usbhost/usbhost_max3421e.c +++ b/drivers/usbhost/usbhost_max3421e.c @@ -195,9 +195,7 @@ struct max3421e_usbhost_s bool connected; /* Connected to device */ bool change; /* Connection change */ bool pscwait; /* True: Thread is waiting for a port event */ -#ifdef CONFIG_DEBUG_ASSERTIONS - bool locked; /* The SPI bus is locked */ -#endif + bool locked; /* The SPI bus is locked (debug only) */ uint8_t smstate; /* The state of the USB host state machine */ uint8_t irqset; /* Set of enabled interrupts */ uint8_t xfrtype; /* See enum mx3421e_hxfrdn_e */ @@ -749,10 +747,13 @@ static void max3421e_lock(FAR struct max3421e_usbhost_s *priv) FAR const struct max3421e_lowerhalf_s *lower = priv->lower; FAR struct spi_dev_s *spi; - DEBUGASSERT(lower != NULL && lower->spi != NULL && !priv->locked); + DEBUGASSERT(lower != NULL && lower->spi != NULL); spi = lower->spi; + /* Another thread may hold the bus; only check the flag once we own it */ + SPI_LOCK(spi, true); + DEBUGASSERT(!priv->locked); SPI_SETMODE(spi, lower->mode); SPI_SETBITS(spi, 8); SPI_HWFEATURES(spi, 0); @@ -778,13 +779,16 @@ static void max3421e_unlock(FAR struct max3421e_usbhost_s *priv) FAR const struct max3421e_lowerhalf_s *lower = priv->lower; DEBUGASSERT(lower != NULL && lower->spi != NULL && priv->locked); - SPI_LOCK(lower->spi, false); #ifdef CONFIG_DEBUG_ASSERTIONS - /* Mark the SPI bus as unlocked (for debug only) */ + /* Mark the SPI bus as unlocked (for debug only). This must be done + * before releasing the bus: a waiting thread may run immediately. + */ priv->locked = false; #endif + + SPI_LOCK(lower->spi, false); } /**************************************************************************** @@ -1319,7 +1323,7 @@ static void max3421e_chan_wakeup(FAR struct max3421e_usbhost_s *priv, { /* Save the result of the operation */ - DEBUGASSERT(priv->result != EBUSY); + DEBUGASSERT(priv->result == EBUSY); priv->result = result; /* Is there a thread waiting for this transfer to complete? */ @@ -1360,6 +1364,27 @@ static void max3421e_chan_wakeup(FAR struct max3421e_usbhost_s *priv, #endif } +/**************************************************************************** + * Name: max3421e_start_xfer + * + * Description: + * Launch a host transfer by writing the HXFR register. Any stale + * HXFRDNIRQ (e.g., left over from a previous transfer that completed + * through RCVDAVIRQ) is cleared first so that it is not mistaken for the + * completion of this transfer. + * + * Assumptions: + * The SPI bus is locked. + * + ****************************************************************************/ + +static void max3421e_start_xfer(FAR struct max3421e_usbhost_s *priv, + uint8_t hxfr) +{ + max3421e_int_clear(priv, USBHOST_HIRQ_HXFRDNIRQ); + max3421e_putreg(priv, MAX3421E_USBHOST_HXFR, hxfr); +} + /**************************************************************************** * Name: max3421e_save_toggles and max3421e_restore_toggles * @@ -1374,16 +1399,30 @@ static inline void max3421e_save_toggles(FAR struct max3421e_usbhost_s *priv, FAR struct max3421e_chan_s *chan) { - chan->toggles = max3421e_getreg(priv, MAX3421E_USBHOST_HCTL); + uint8_t regval; + + /* The HCTL toggle bits are write-only strobes; the current toggle values + * are read back from HRSL. Save them in HCTL strobe format. + */ + + regval = max3421e_getreg(priv, MAX3421E_USBHOST_HRSL); + + chan->toggles = ((regval & USBHOST_HRSL_RCVTOGRD) != 0 ? + USBHOST_HCTL_RCVTOG1 : USBHOST_HCTL_RCVTOG0) | + ((regval & USBHOST_HRSL_SNDTOGRD) != 0 ? + USBHOST_HCTL_SNDTOG1 : USBHOST_HCTL_SNDTOG0); } static inline void max3421e_restore_toggles(FAR struct max3421e_usbhost_s *priv, FAR struct max3421e_chan_s *chan) { - max3421e_modifyreg(priv, MAX3421E_USBHOST_HCTL, - USBHOST_HCTL_TOGGLES_MASK, - chan->toggles & USBHOST_HCTL_TOGGLES_MASK); + /* Write only the toggle strobes: a read-modify-write of HCTL could + * re-trigger the other strobe bits (BUSRST, FRMRST, BUSSAMPLE, SIGRSM). + */ + + max3421e_putreg(priv, MAX3421E_USBHOST_HCTL, + chan->toggles & USBHOST_HCTL_TOGGLES_MASK); } /**************************************************************************** @@ -1558,8 +1597,7 @@ static void max3421e_put_sndfifo(FAR struct max3421e_usbhost_s *priv, /* Send the OUT token */ - max3421e_putreg(priv, MAX3421E_USBHOST_HXFR, - USBHOST_HXFR_TOKEN_OUT | chan->epno); + max3421e_start_xfer(priv, USBHOST_HXFR_TOKEN_OUT | chan->epno); /* Increment the count of bytes "in-flight" in the SNDFIFO */ @@ -1722,8 +1760,7 @@ static void max3421e_send_start(FAR struct max3421e_usbhost_s *priv, /* Send the OUT token */ - max3421e_putreg(priv, MAX3421E_USBHOST_HXFR, - USBHOST_HXFR_TOKEN_OUT | chan->epno); + max3421e_start_xfer(priv, USBHOST_HXFR_TOKEN_OUT | chan->epno); /* Enable the HXFRDNIRQ to catch completion of the ZLP. */ @@ -2013,8 +2050,7 @@ static int max3421e_ctrl_sendsetup(FAR struct max3421e_usbhost_s *priv, /* Send the SETUP token (always EP0) */ - max3421e_putreg(priv, MAX3421E_USBHOST_HXFR, - USBHOST_HXFR_TOKEN_SETUP); + max3421e_start_xfer(priv, USBHOST_HXFR_TOKEN_SETUP); /* Increment the count of bytes "in-flight" in the SNDFIFO */ @@ -2090,11 +2126,12 @@ static int max3421e_ctrl_senddata(FAR struct max3421e_usbhost_s *priv, priv->buffer = buffer; priv->buflen = buflen; - /* The MAX4321E sends fixed DATA0 and DATA1 PID tokens for the various - * stages of a CONTROL transfer, regardless of the setting of the internal - * data toggle. + /* The SETUP and handshake stages use fixed DATA0/DATA1 PIDs, but the + * DATA stage uses the data toggles and must always start with DATA1. */ + chan->toggles = USBHOST_HCTL_RCVTOG1 | USBHOST_HCTL_SNDTOG1; + /* Set up for the wait BEFORE starting the transfer */ ret = max3421e_chan_waitsetup(priv, chan); @@ -2142,11 +2179,12 @@ static int max3421e_ctrl_recvdata(FAR struct max3421e_usbhost_s *priv, priv->buflen = buflen; priv->xfrd = 0; - /* The MAX4321E sends fixed DATA0 and DATA1 PID tokens for the various - * stages of a CONTROL transfer, regardless of the setting of the internal - * data toggle. + /* The SETUP and handshake stages use fixed DATA0/DATA1 PIDs, but the + * DATA stage uses the data toggles and must always start with DATA1. */ + chan->toggles = USBHOST_HCTL_RCVTOG1 | USBHOST_HCTL_SNDTOG1; + /* Set up for the wait BEFORE starting the transfer */ ret = max3421e_chan_waitsetup(priv, chan); @@ -2183,6 +2221,17 @@ static int max3421e_ctrl_recvdata(FAR struct max3421e_usbhost_s *priv, static int max3421e_ctrl_sendstatus(FAR struct max3421e_usbhost_s *priv, FAR struct max3421e_chan_s *chan) { + int ret; + + /* Set up for the wait BEFORE starting the transfer */ + + ret = max3421e_chan_waitsetup(priv, chan); + if (ret < 0) + { + usbhost_trace1(MAX3421E_TRACE1_DEVDISCONN3, 0); + return ret; + } + /* Set up the initial state of the transfer */ usbhost_vtrace2(MAX3421E_VTRACE2_SENDSTATUS, chan->chidx, priv->buflen); @@ -2203,8 +2252,7 @@ static int max3421e_ctrl_sendstatus(FAR struct max3421e_usbhost_s *priv, /* Send the HS-OUT token */ - max3421e_putreg(priv, MAX3421E_USBHOST_HXFR, - USBHOST_HXFR_TOKEN_OUTHS | chan->epno); + max3421e_start_xfer(priv, USBHOST_HXFR_TOKEN_OUTHS | chan->epno); /* Enable the HXFRDNIRQ to catch completion of the ZLP. */ @@ -2233,8 +2281,19 @@ static int max3421e_ctrl_sendstatus(FAR struct max3421e_usbhost_s *priv, static int max3421e_ctrl_recvstatus(FAR struct max3421e_usbhost_s *priv, FAR struct max3421e_chan_s *chan) { + int ret; + max3421e_pktdump("Sending", priv->buffer, priv->buflen); + /* Set up for the wait BEFORE starting the transfer */ + + ret = max3421e_chan_waitsetup(priv, chan); + if (ret < 0) + { + usbhost_trace1(MAX3421E_TRACE1_DEVDISCONN4, 0); + return ret; + } + /* Set up the initial state of the transfer */ usbhost_vtrace2(MAX3421E_VTRACE2_RECVSTATUS, chan->chidx, priv->buflen); @@ -2251,16 +2310,14 @@ static int max3421e_ctrl_recvstatus(FAR struct max3421e_usbhost_s *priv, /* Send the HS-IN token. */ - max3421e_putreg(priv, MAX3421E_USBHOST_HXFR, - USBHOST_HXFR_TOKEN_INHS | chan->epno); + max3421e_start_xfer(priv, USBHOST_HXFR_TOKEN_INHS | chan->epno); - /* Enable the RCVFIFO interrupt to handle the completion/continuation - * of transfer. Enable the HXFRDNIRQ to catch NAKs and transfer - * errors. + /* The handshake is a zero length packet: nothing lands in the RCVFIFO, + * so completion (or a NAK/error) is signalled by HXFRDNIRQ alone. */ - priv->xfrtype = HXFRDN_RCVFIFO; - max3421e_int_enable(priv, USBHOST_HIRQ_RCVDAVIRQ | USBHOST_HIRQ_HXFRDNIRQ); + priv->xfrtype = HXFRDN_SNDZLP; + max3421e_int_enable(priv, USBHOST_HIRQ_HXFRDNIRQ); max3421e_unlock(priv); /* Wait for the transfer to complete and return the result */ @@ -2458,8 +2515,7 @@ static void max3421e_recv_restart(FAR struct max3421e_usbhost_s *priv, { /* Send the IN token. */ - max3421e_putreg(priv, MAX3421E_USBHOST_HXFR, - USBHOST_HXFR_TOKEN_IN | chan->epno); + max3421e_start_xfer(priv, USBHOST_HXFR_TOKEN_IN | chan->epno); /* Enable the RCVFIFO interrupt to handle the completion/continuation * of transfer. Enable the HXFRDNIRQ to catch NAKs and transfer @@ -2504,13 +2560,17 @@ static void max3421e_recv_continue(FAR struct max3421e_usbhost_s *priv) { /* Terminate the transfer on any error. */ + max3421e_int_clear(priv, USBHOST_HIRQ_RCVDAVIRQ); max3421e_transfer_terminate(priv, chan, result); return; } - /* Transfer the data from the RCVFIFO to the user buffer */ + /* Transfer the data from the RCVFIFO to the user buffer, then release + * the RCVFIFO buffer by clearing RCVDAVIRQ. + */ nrcvd = max3421e_get_rcvfifo(priv, chan); + max3421e_int_clear(priv, USBHOST_HIRQ_RCVDAVIRQ); /* A partial or zero-length packet is an indication that the transfer * completed early. Terminate the transfer in those cases OR if all @@ -2561,6 +2621,10 @@ static void max3421e_recv_start(FAR struct max3421e_usbhost_s *priv, max3421e_putreg(priv, MAX3421E_USBHOST_PERADDR, chan->funcaddr); + /* Restore the data toggles of this endpoint */ + + max3421e_restore_toggles(priv, chan); + /* Start the transfer. */ max3421e_recv_restart(priv, chan); @@ -2658,6 +2722,7 @@ static ssize_t max3421e_in_transfer(FAR struct max3421e_usbhost_s *priv, */ clock_t elapsed = clock_systime_ticks() - start; + if (elapsed >= MAX3421E_DATANAK_DELAY) { /* Timeout out... break out returning the NAK as @@ -3007,6 +3072,37 @@ static void max3421e_disconnected(FAR struct max3421e_usbhost_s *priv) max3421e_disconnect_event(priv); } +/**************************************************************************** + * Name: max3421e_device_present + * + * Description: + * Sample the bus and return true if a device is attached (J or K state) + * without disturbing the bus with a reset. + * + * Assumptions: + * The SPI bus is locked. + * + ****************************************************************************/ + +static bool max3421e_device_present(FAR struct max3421e_usbhost_s *priv) +{ + uint8_t regval; + + max3421e_modifyreg(priv, MAX3421E_USBHOST_HCTL, 0, + USBHOST_HCTL_BUSSAMPLE); + + while ((max3421e_getreg(priv, MAX3421E_USBHOST_HCTL) & + USBHOST_HCTL_BUSSAMPLE) == 0) + { + nxsched_usleep(5); + } + + regval = max3421e_getreg(priv, MAX3421E_USBHOST_HRSL); + regval &= (USBHOST_HRSL_KSTATUS | USBHOST_HRSL_JSTATUS); + + return regval == USBHOST_HRSL_KSTATUS || regval == USBHOST_HRSL_JSTATUS; +} + /**************************************************************************** * Name: max3421e_irqwork * @@ -3090,6 +3186,7 @@ static void max3421e_irqwork(FAR void *arg) priv->xfrtype == HXFRDN_SNDZLP) { FAR struct max3421e_chan_s *chan = priv->waiter; + DEBUGASSERT(chan != NULL); max3421e_transfer_terminate(priv, chan, result); @@ -3104,13 +3201,23 @@ static void max3421e_irqwork(FAR void *arg) max3421e_int_clear(priv, USBHOST_HIRQ_CONNIRQ); - /* Check if a peripheral device has been connected */ + /* CONNIRQ is also raised by our own bus resets, so sample the + * bus and only act on a real attach or detach. + */ - ret = max3421e_connected(priv); - if (ret < 0) + if (max3421e_device_present(priv)) + { + if (!priv->connected) + { + ret = max3421e_connected(priv); + if (ret < 0) + { + max3421e_disconnected(priv); + } + } + } + else if (priv->connected) { - /* No.. then a device must have been disconnected. */ - max3421e_disconnected(priv); } } @@ -3144,16 +3251,11 @@ static void max3421e_irqwork(FAR void *arg) max3421e_int_disable(priv, USBHOST_HIRQ_RCVDAVIRQ | USBHOST_HIRQ_HXFRDNIRQ); - /* Clear the pending RCVDAV interrupt. The RCVFIFO is double - * buffered. If another packet is available in the other buffer, - * the MAX3421E will immediately re-assert RCVDAVIRQ and we - * will catch that the next time through this loop. + /* Handle the receipt of data. RCVDAVIRQ is cleared there, after + * the RCVFIFO has been read: clearing it releases the buffer + * and switches the double-buffered RCVFIFO to the next packet. */ - max3421e_int_clear(priv, USBHOST_HIRQ_RCVDAVIRQ); - - /* Handle the receipt of data */ - max3421e_recv_continue(priv); } } @@ -3428,7 +3530,6 @@ static int max3421e_getspeed(FAR struct max3421e_usbhost_s *priv, /* No, return an error */ usbhost_trace1(MAX3421E_TRACE1_DEVDISCONN6, 0); - nxrmutex_unlock(&priv->lock); return -ENODEV; } @@ -3511,6 +3612,7 @@ static int max3421e_enumerate(FAR struct usbhost_connection_s *conn, ret = max3421e_getspeed(priv, conn, hport); if (ret < 0) { + max3421e_unlock(priv); nxrmutex_unlock(&priv->lock); return ret; } @@ -4512,6 +4614,10 @@ static void max3421e_busreset(FAR struct max3421e_usbhost_s *priv) max3421e_modifyreg(priv, MAX3421E_USBHOST_HCTL, 0, USBHOST_HCTL_BUSRST); max3421e_int_wait(priv, USBHOST_HIRQ_BUSEVENTIRQ, 250); + + /* The reset (SE0) is seen as a connection change; discard it */ + + max3421e_int_clear(priv, USBHOST_HIRQ_CONNIRQ); } /**************************************************************************** @@ -4719,6 +4825,12 @@ static inline int max3421e_sw_initialize(FAR struct max3421e_usbhost_s *priv, chan->chidx = i; } + /* Channel 0 is permanently used by EP0 of the root hub port (see + * hport->ep0 above); do not let max3421e_epalloc() hand it out. + */ + + priv->chan[0].inuse = true; + /* Initialize the connection structure */ conn->conn.wait = max3421e_wait; @@ -4756,6 +4868,7 @@ static inline int max3421e_hw_initialize(FAR struct max3421e_usbhost_s *priv) uint8_t revision; uint8_t regval; int ret; + int i; /* Get exclusive access to the SPI bus */ @@ -4790,9 +4903,16 @@ static inline int max3421e_hw_initialize(FAR struct max3421e_usbhost_s *priv) /* Wait for the oscillator to become stable */ - while ((max3421e_getreg(priv, MAX3421E_USBHOST_USBIRQ) & - USBHOST_USBIRQ_OSCOKIRQ) == 0) + for (i = 0; (max3421e_getreg(priv, MAX3421E_USBHOST_USBIRQ) & + USBHOST_USBIRQ_OSCOKIRQ) == 0; i++) { + if (i >= 100) + { + uwarn("WARNING: Timeout waiting for OSCOKIRQ\n"); + break; + } + + nxsched_usleep(1000); } /* Disable interrupts, clear pending interrupts, and reset the interrupt
