Merge #1947

1947: nrf52: usb: better check state on IN resume r=ppannuto a=bradjc

While running a modified `printf_long` test app that prints a long string in a loop on nano33ble, I got an assertion failure:

```
Kernel panic at chips/nrf52/src/usbd.rs:1749:
	"assertion failed: `(left == right)`
  left: `Some(InData)`,
 right: `Some(Init)`"
	Kernel version release-1.5-500-g9c520bd6b
```

This happens because the resume_in function was only checking the state of the DMA, but not of the IN endpoint, which might be finished with DMA but the entire transfer hasn't finished. I think this wasn't occurring before because with short IN transfers, the end of the DMA and the end of the transfer occur very close to each other and the endpoint wasn't in the `InData` state very long. However, with a longer transfer the `InData` state is longer and thus this assertion could fail.

The fix is just to check both states, and enqueue the resume in request if either is true.



### Testing Strategy

This pull request was tested by running printf_long in a loop on nano33ble. It actually works!


### TODO or Help Wanted

You can see my comment about the driver calling `packet_in()` as a direct result of the client calling `endpoint_resume_in()`. That should be fixed at some point, or the USB HIL should be changed so that there are no longer upcalls asking for data.


### Documentation Updated

- [x] Updated the relevant files in `/docs`, or no updates are required.

### Formatting

- [x] Ran `make prepush`.


Co-authored-by: Brad Campbell <bradjc5@gmail.com>

chips/nrf52/src/usbd.rs

@@ -1919,15 +1919,29 @@ impl<'a> hil::usb::UsbController<'a> for Usbd<'a> {
     fn endpoint_resume_in(&self, endpoint: usize) {
         debug_events!("endpoint_resume_in({})", endpoint);
 
+        // Get the state of the endpoint that the upper layer requested to start
+        // an IN transfer with for our state machine.
         let (_, in_state, _) = self.descriptors[endpoint].state.get().bulk_state();
+        // If the state is `None`, this endpoint is not configured and should
+        // not have been used to call `endpoint_resume_in()`.
         assert!(in_state.is_some());
 
-        if self.dma_pending.get() {
+        // If there is an active DMA request, or we are waiting on finishing up
+        // a previous IN transfer, we queue this request and it will be serviced
+        // after those complete.
+        if self.dma_pending.get() || in_state != Some(BulkInState::Init) {
             debug_events!("requesting resume_in[{}]", endpoint);
             // A DMA is already pending. Schedule the resume for later.
             self.descriptors[endpoint].request_transmit_in.set(true);
         } else {
-            // Trigger the transaction now.
+            // If we aren't waiting on anything, trigger the transaction now.
+            //
+            // NOTE! TODO! We can't actually do this. This leads to an upcall
+            // (`client.packet_in()`) happening as a direct result of a downcall
+            // (this `endpoint_resume_in()` call). Unfortunately, the nRF52
+            // doesn't give us a great interrupt to use to check the
+            // `request_transmit_in` flag if we were to queue unconditionally in
+            // `endpoint_resume_in()`.
             self.transmit_in(endpoint);
         }
     }