Skip to content

Commit

Permalink
shared/tinyusb: Fix dynamic USB control callbacks for wLength==0.
Browse files Browse the repository at this point in the history
In the case where an OUT control transfer triggers with wLength==0 (i.e.
all data sent in the SETUP phase, and no additional data phase) the
callbacks were previously implemented to return b"" (i.e. an empty buffer
for the data phase).

However this didn't actually work as intended because b"" can't provide a
RW buffer (needed for OUT transfers with a data phase to write data into),
so actually the endpoint would stall.

The symptom was often that the device process the request (if processing
it in the SETUP phase when all information was already available), but the
host sees the endpoint stall and eventually returns an error.

This commit changes the behaviour so returning True from the SETUP phase of
a control transfer queues a zero length status response.

Signed-off-by: Angus Gratton <angus@redyak.com.au>
  • Loading branch information
projectgus authored and dpgeorge committed Apr 17, 2024
1 parent 53d0050 commit d11ca09
Show file tree
Hide file tree
Showing 2 changed files with 25 additions and 10 deletions.
20 changes: 15 additions & 5 deletions docs/library/machine.USBDevice.rst
Expand Up @@ -130,15 +130,25 @@ Methods

Second argument is a memoryview to read the USB control request
data for this stage. The memoryview is only valid until the
callback function returns.
callback function returns. Data in this memoryview will be the same
across each of the three stages of a single transfer.

A successful transfer consists of this callback being called in sequence
for the three stages. Generally speaking, if a device wants to do
something in response to a control request then it's best to wait until
the ACK stage to confirm the host controller completed the transfer as
expected.

The callback should return one of the following values:

- ``False`` to stall the endpoint and reject the transfer.
- ``False`` to stall the endpoint and reject the transfer. It won't
proceed to any remaining stages.
- ``True`` to continue the transfer to the next stage.
- A buffer object to provide data for this stage of the transfer.
This should be a writable buffer for an ``OUT`` direction transfer, or a
readable buffer with data for an ``IN`` direction transfer.
- A buffer object can be returned at the SETUP stage when the transfer
will send or receive additional data. Typically this is the case when
the ``wLength`` field in the request has a non-zero value. This should
be a writable buffer for an ``OUT`` direction transfer, or a readable
buffer with data for an ``IN`` direction transfer.

- ``xfer_cb`` - This callback is called whenever a non-control
transfer submitted by calling :func:`USBDevice.submit_xfer` completes.
Expand Down
15 changes: 10 additions & 5 deletions shared/tinyusb/mp_usbd_runtime.c
Expand Up @@ -295,6 +295,7 @@ static bool runtime_dev_control_xfer_cb(uint8_t rhport, uint8_t stage, tusb_cont
mp_obj_usb_device_t *usbd = MP_OBJ_TO_PTR(MP_STATE_VM(usbd));
tusb_dir_t dir = request->bmRequestType_bit.direction;
mp_buffer_info_t buf_info;
bool result;

if (!usbd) {
return false;
Expand All @@ -319,7 +320,7 @@ static bool runtime_dev_control_xfer_cb(uint8_t rhport, uint8_t stage, tusb_cont

// Check if callback returned any data to submit
if (mp_get_buffer(cb_res, &buf_info, dir == TUSB_DIR_IN ? MP_BUFFER_READ : MP_BUFFER_RW)) {
bool result = tud_control_xfer(USBD_RHPORT,
result = tud_control_xfer(USBD_RHPORT,
request,
buf_info.buf,
buf_info.len);
Expand All @@ -328,17 +329,21 @@ static bool runtime_dev_control_xfer_cb(uint8_t rhport, uint8_t stage, tusb_cont
// Keep buffer object alive until the transfer completes
usbd->xfer_data[0][dir] = cb_res;
}

return result;
} else {
// Expect True or False to stall or continue
result = mp_obj_is_true(cb_res);

if (stage == CONTROL_STAGE_ACK) {
if (stage == CONTROL_STAGE_SETUP && result) {
// If no additional data but callback says to continue transfer then
// queue a status response.
tud_control_status(rhport, request);
} else if (stage == CONTROL_STAGE_ACK) {
// Allow data to be GCed once it's no longer in use
usbd->xfer_data[0][dir] = mp_const_none;
}
return mp_obj_is_true(cb_res);
}

return result;
}

static bool runtime_dev_xfer_cb(uint8_t rhport, uint8_t ep_addr, xfer_result_t result, uint32_t xferred_bytes) {
Expand Down

0 comments on commit d11ca09

Please sign in to comment.