diff options
author | patacongo <patacongo@42af7a65-404d-4744-a932-0658087f49c3> | 2011-12-17 19:08:49 +0000 |
---|---|---|
committer | patacongo <patacongo@42af7a65-404d-4744-a932-0658087f49c3> | 2011-12-17 19:08:49 +0000 |
commit | 9a958200a692d239f9f67b27ec8ba672ce2188ef (patch) | |
tree | bfec353e0ff6652e40bc645034121fb12cd6f102 | |
parent | 5101245355c24f5c3921d929977bcbae59ec9675 (diff) | |
download | px4-nuttx-9a958200a692d239f9f67b27ec8ba672ce2188ef.tar.gz px4-nuttx-9a958200a692d239f9f67b27ec8ba672ce2188ef.tar.bz2 px4-nuttx-9a958200a692d239f9f67b27ec8ba672ce2188ef.zip |
Fix a critical bug in the STM32 USB device-side driver
git-svn-id: svn://svn.code.sf.net/p/nuttx/code/trunk@4196 42af7a65-404d-4744-a932-0658087f49c3
-rw-r--r-- | nuttx/ChangeLog | 8 | ||||
-rw-r--r-- | nuttx/arch/arm/src/stm32/stm32_usbdev.c | 84 |
2 files changed, 53 insertions, 39 deletions
diff --git a/nuttx/ChangeLog b/nuttx/ChangeLog index 8b8a2d5e4..b494164c4 100644 --- a/nuttx/ChangeLog +++ b/nuttx/ChangeLog @@ -2272,3 +2272,11 @@ "upper half" driver for PWM output. * arch/arm/src/stm32/stm32_pwm.c: Added a PWM "lower half" driver for the STM32. The initial check-in is little more than a framework for the driver. + * arch/arm/src/stm32/stm32_usbdev.c: Corrected two CRITICAL errors in the USB + device-side driver: (1) Handling of data overrun condition was wrong. When + there was no further memory to accept further OUT endpoint data, the driver + would hang with infinite interrupts; (2) the logic in setting toggle bits + was not correct. However, this driver has functioned for a long time until + the particular condition that revealed the bug occurred. My impression is + that this latter bugfix also fixes some STM32 USB performance problems. + diff --git a/nuttx/arch/arm/src/stm32/stm32_usbdev.c b/nuttx/arch/arm/src/stm32/stm32_usbdev.c index c1d6da803..c10ac104b 100644 --- a/nuttx/arch/arm/src/stm32/stm32_usbdev.c +++ b/nuttx/arch/arm/src/stm32/stm32_usbdev.c @@ -938,22 +938,19 @@ static void stm32_seteptxstatus(uint8_t epno, uint16_t state) uint32_t epaddr = STM32_USB_EPR(epno); uint16_t regval; - regval = stm32_getreg(epaddr) & EPR_TXDTOG_MASK; - - /* Toggle first bit */ - - if ((USB_EPR_STATTX_DTOG1 & state) != 0) - { - regval ^= USB_EPR_STATTX_DTOG1; - } + /* The bits in the STAT_TX field can be toggled by software to set their + * value. When set to 0, the value remains unchanged; when set to one, + * value toggles. + */ - /* Toggle second bit */ + regval = stm32_getreg(epaddr); - if ((USB_EPR_STATTX_DTOG2 & state) != 0) - { - regval ^= USB_EPR_STATTX_DTOG2; - } + /* The exclusive OR will set STAT_TX bits to 1 if there value is different + * from the bits requested in 'state' + */ + regval ^= state; + regval &= EPR_TXDTOG_MASK; stm32_putreg(regval, epaddr); } @@ -966,22 +963,19 @@ static void stm32_seteprxstatus(uint8_t epno, uint16_t state) uint32_t epaddr = STM32_USB_EPR(epno); uint16_t regval; - regval = stm32_getreg(epaddr) & EPR_RXDTOG_MASK; - - /* Toggle first bit */ - - if ((USB_EPR_STATRX_DTOG1 & state) != 0) - { - regval ^= USB_EPR_STATRX_DTOG1; - } + /* The bits in the STAT_RX field can be toggled by software to set their + * value. When set to 0, the value remains unchanged; when set to one, + * value toggles. + */ - /* Toggle second bit */ + regval = stm32_getreg(epaddr); - if ((USB_EPR_STATRX_DTOG2 & state) != 0) - { - regval ^= USB_EPR_STATRX_DTOG2; - } + /* The exclusive OR will set STAT_RX bits to 1 if there value is different + * from the bits requested in 'state' + */ + regval ^= state; + regval &= EPR_RXDTOG_MASK; stm32_putreg(regval, epaddr); } @@ -1435,23 +1429,32 @@ static void stm32_epdone(struct stm32_usbdev_s *priv, uint8_t epno) { usbtrace(TRACE_INTDECODE(STM32_TRACEINTID_EPOUTDONE), epr); - /* Handle read requests: Read host data into the current read request */ + /* Handle read requests. First check if a read request is available to + * accept the host data. + */ - priv->rxstatus = USB_EPR_STATRX_VALID; if (!stm32_rqempty(privep)) { + /* Read host data into the current read request */ + stm32_rdrequest(priv, privep); - /* Clear the interrupt status */ + /* "After the received data is processed, the application software + * should set the STAT_RX bits to '11' (Valid) in the USB_EPnR, + * enabling further transactions. " + */ - stm32_clrepctrrx(epno); + priv->rxstatus = USB_EPR_STATRX_VALID; } else { usbtrace(TRACE_INTDECODE(STM32_TRACEINTID_EPOUTPENDING), (uint16_t)epno); /* Mark the RX processing as pending and NAK any OUT actions - * on this endpoint + * on this endpoint. "While the STAT_RX bits are equal to '10' + * (NAK), any OUT request addressed to that endpoint is NAKed, + * indicating a flow control condition: the USB host will retry + * the transaction until it succeeds." */ priv->rxstatus = USB_EPR_STATRX_NAK; @@ -1460,6 +1463,7 @@ static void stm32_epdone(struct stm32_usbdev_s *priv, uint8_t epno) /* Clear the interrupt status and set the new RX status */ + stm32_clrepctrrx(epno); stm32_seteprxstatus(epno, priv->rxstatus); } @@ -2116,7 +2120,7 @@ static inline void stm32_ep0done(struct stm32_usbdev_s *priv, uint16_t istr) /* Was a transmission started? If so, txstatus will be VALID. The * only special case to handle is when both are set to NAK. In that - * case, we need to for RX status to VALID in order to accept the next + * case, we need to set RX status to VALID in order to accept the next * SETUP request. */ @@ -2845,17 +2849,19 @@ static int stm32_epsubmit(struct usbdev_ep_s *ep, struct usbdev_req_s *req) if (priv->rxpending) { + /* Set STAT_RX bits to '11' in the USB_EPnR, enabling further + * transactions. "While the STAT_RX bits are equal to '10' + * (NAK), any OUT request addressed to that endpoint is NAKed, + * indicating a flow control condition: the USB host will retry + * the transaction until it succeeds." + */ + priv->rxstatus = USB_EPR_STATRX_VALID; - ret = stm32_rdrequest(priv, privep); + stm32_seteprxstatus(epno, priv->rxstatus); - /* Clear the pending interrupt status */ + /* Data is no longer pending */ - stm32_clrepctrrx(epno); priv->rxpending = false; - - /* Set the new RX status */ - - stm32_seteprxstatus(epno, priv->rxstatus); } } |