eclipse-threadx / usbx

Eclipse ThreadX - USBX is a high-performance USB host, device, and on-the-go (OTG) embedded stack, that is fully integrated with Eclipse ThreadX RTOS
https://github.com/eclipse-threadx/rtos-docs/blob/main/rtos-docs/usbx/index.md
MIT License
148 stars 88 forks source link

Why check length at the end of ux_device_class_storage_inquiry? It seems not compliant with protocol. #95

Closed xuzihan351 closed 1 year ago

xuzihan351 commented 1 year ago

code place: ux_device_class_storage_inquiry.c:+249

/* Check length.  */
if (storage -> ux_slave_class_storage_host_length != inquiry_length)
{
    storage -> ux_slave_class_storage_csw_residue = storage -> ux_slave_class_storage_host_length - inquiry_length;
    _ux_device_stack_endpoint_stall(endpoint_in);
}

When deal with inquiry cmd, the transfer length may not equal with dCBWDataTransferLength, the code check it and stall the endpoint if it is not match.

Considered code in the same file, ux_device_class_storage_inquiry.c:+194

     /* Send a data payload with the inquiry response buffer.  */
    if (inquiry_length > 24)
        inquiry_length = 24;

the inquiry_length's value is changed, but storage -> ux_slave_class_storage_host_length is not. So the endpoint stall will occur once the code run into line 194.

xiaocq2001 commented 1 year ago

According to Section 6.7 The Thirteen Cases USB mass storage class bulk-only transport spec, the way to operate on device side in case host requested length not equal to device sending length is:

The case matrix: image

Case (5) and case (7) is the reference: image image

Your description is Hi > Di (case 5), the actual data back is not as expected and we are not padding data, so there should be a STALL.

Note the behavior here is checked by USB CV test tool.

xuzihan351 commented 1 year ago

Thanks for the explanation, it is indeed correct. Some other usb stacks do not make endpoint into stall status in the case, so that is their fault.