Repository navigation
Conversation
UDP_ISR still reports endpoints whose interrupt is masked (e.g. the bulk OUT endpoint while a received packet waits for the process). The handler picked the lowest one, so a masked endpoint could repeatedly sidetrack handling of the real interrupt (e.g. a completed bulk IN packet), which stayed asserted and re-entered the handler forever, hanging the hub. Select the endpoint from UDP_ISR & UDP_IMR, as the Linux at91_udc driver does. Reproduced by sending stdin bursts while the hub was printing, with the CSR fix from the next commit applied. A watchdog on the system tick caught the UDP interrupt firing 141086 times per second, in the masked bulk OUT branch of the handler. With this change, 15 of 15 rounds completed over three runs.
|
Thanks Mark! Would you be interested in having a look at getting rid of the blocking while loops altogether? If we have to turn off interrupts to block and wait for something to complete, that's usually a sign that something else isn't quite right. |
|
Can you also add which version you reproduced the original issue under? |
| * These helpers set/clear CSR flags, and then loop waiting for the | ||
| * controller to synchronize | ||
| /* A CSR write takes effect only once it has crossed into the USB clock | ||
| * domain, which takes up to 3 UDPCK and 5 MCK cycles (about 170 ns). Wait that |
There was a problem hiding this comment.
The datasheet (doc6175) says:
Note: In a preemptive environment, set or clear the flag and wait for a time of 1 UDPCK clock cycle and 1 peripheral clock cycle. However, RX_DATA_BLK0, TXPKTRDY, RX_DATA_BK1 require wait times of 3 UDPCK clock cycles and 3 peripheral clock cycles before accessing DPR.
So this seems a bit longer than necessary.
There was a problem hiding this comment.
Good catch, thanks. I'd taken the 3 UDPCK + 5 MCK figure from Atmel's ASF docs, which are for the newer SAM4S chip. The UDP block is the same but the sync time isn't. Updated to doc6175's 3 UDPCK + 3 MCK (125 ns), with a 150 ns wait. Retested on hardware with the I2C sensor: no freezes.
|
|
||
| static void pbdrv_usb_nxt_csr_write(uint8_t endpoint, uint32_t csr) { | ||
| AT91C_UDP_CSR[endpoint] = csr; | ||
| nx_systick_wait_ns(PBDRV_USB_NXT_CSR_SYNC_NS); |
There was a problem hiding this comment.
This is still a blocking wait though. We are gradually converting the nxos drivers to use the cooperative multitasking of pbio. Might be quite a bit of refactoring work though since this is driven from the ISR.
There was a problem hiding this comment.
It should just be on the order of 100 ns or so though, so not really harmful blocking. I think we are talking less than 10 cpu cycles. Doesn't seem worth it to yield here to me.
There was a problem hiding this comment.
Agree with David that yielding isn't worth it for ~6 cycles, and this runs inside the UDP ISR so it can't yield anyway. Happy to look at moving the endpoint handling out of the ISR into a pbio process as a separate PR.
The CSR helpers write a flag and then poll until the controller reports the change. Some flags are changed back by the controller itself (e.g. TXPKTRDY is cleared once the packet is sent). If an interrupt delays the poll for longer, the loop never sees the flag set and just waits indefinitely. This happened with an I2C sensor in use, since the bit-banged I2C interrupt runs every 26us. While reading the ultrasonic sensor, the hub froze within a few minutes. Recording the interrupted program counter from the system tick showed it spinning in pbdrv_usb_nxt_csr_set_flag() (which was called to set TXPKTRDY for a bulk IN packet). A CSR write only needs a fixed time to take effect (3 UDPCK and 3 MCK cycles), so wait that long instead of polling. This also replaces the loops waiting for endpoint configuration writes, which could hang the same way. With this change, the same sensor test ran 7 times without a freeze (it previously froze within 1-3 runs, 4 times out of 4).
3 participants
The NXT could freeze completely during USB communication (requiring a reset to recover). There were two separate causes:
Both freezes were reproduced on upstream master
0dba3bd6f, whichgit describelabels asv4.1.0b5-1-g0dba3bd6fTest plan
Both hangs were localized with a temporary debug build (not included) that used a system tick watchdog to show the interrupted program counters on the display when the main loop stalled.
Case 1: Masked endpoints
Write several stdin frames in a single host write while the hub is printing.
Case 2: CSR race
Read an ultrasonic sensor repeatedly using the NXT I2C driver (upcoming PR), whose interrupt runs every 26 µs.