K22 USB device CDC and HID problem

[NicolasP]

Hi,

I'm working on a USB device instanciating 4 CDC virtual com and a HID. The 4 virtual com send and receive on 4 real UART/LPUART.

I do some stress test by sending and receiving data (115200 baud) continuously from the host at the maximum throughput possible. The UART TX pins are connected to UART RX pins on all channels. So everything sent from the host is received by the host.

In parallel, HID is used to transfer device state to the host.

Globally, everything works well.

However, at some point, HID IN packets are not sent any more by the device to the host. The USB_DeviceHidSend() function returns a "busy" status. HID OUT packets sent by the host are still received by the device.

I'm investigating...

Any help much appreciated.

Nicolas

[1920844004]

very good ! thanks!

[NicolasP]

Just found that without stress test, the problem still exists. So, this is not a performance related issue.

[NicolasP · Accepted solution]

After days of research, I fixed the bug locking the sending of HID IN packets.

The problem is with USB_DeviceHidSend() function.

Below is the current code :

usb_status_t USB_DeviceHidSend(class_handle_t handle, uint8_t ep, uint8_t *buffer, uint32_t length)
{
    usb_device_hid_struct_t *hidHandle;
    usb_status_t error = kStatus_USB_Error;

    if (!handle)
    {
        return kStatus_USB_InvalidHandle;
    }
    hidHandle = (usb_device_hid_struct_t *)handle;

    if (hidHandle->interruptInPipeBusy)
    {
        return kStatus_USB_Busy;
    }
    if (hidHandle->interruptInPipeStall)
    {
        hidHandle->interruptInPipeBusy = 1U;
        hidHandle->interruptInPipeDataBuffer = buffer;
        hidHandle->interruptInPipeDataLen = length;
        return kStatus_USB_Success;
    }
    error = USB_DeviceSendRequest(hidHandle->handle, ep, buffer, length);
    if (kStatus_USB_Success == error)
    {
        hidHandle->interruptInPipeBusy = 1U;
    }
    return error;
}
‍‍‍‍‍‍‍‍‍‍‍‍‍‍‍‍‍‍‍‍‍‍‍‍‍‍‍‍‍

There is a race condition between lines 23, 26 and IRQ handler.

The ideal sequence is :

- USB_DeviceSendRequest() requests the sending of a HID IN packet. (line 23)

- interruptInPipeBusy is set to one. (line 26)

- IRQ handler is executed because HID IN packet has been sent. The IRQ handler sets interruptInPipeBusy to 0.

If the IRQ handler is executed before line 26 is executed, interruptInPipeBusy is set to 1 too late.  This stalls the sending of HID IN packets. This can easily happen when the task calling USB_DeviceHidSend() has a low priority and higher priority tasks run quite intensively.

My fix is to add critical sections like this :

usb_status_t USB_DeviceHidSend(class_handle_t handle, uint8_t ep, uint8_t *buffer, uint32_t length)
{
    usb_device_hid_struct_t *hidHandle;
    usb_status_t error = kStatus_USB_Error;
    USB_OSA_SR_ALLOC();

    if (!handle)
    {
        return kStatus_USB_InvalidHandle;
    }
    hidHandle = (usb_device_hid_struct_t *)handle;

    USB_OSA_ENTER_CRITICAL();
    if (hidHandle->interruptInPipeBusy)
    {
        USB_OSA_EXIT_CRITICAL();
        return kStatus_USB_Busy;
    }
    if (hidHandle->interruptInPipeStall)
    {
        hidHandle->interruptInPipeBusy = 1U;
        hidHandle->interruptInPipeDataBuffer = buffer;
        hidHandle->interruptInPipeDataLen = length;
        USB_OSA_EXIT_CRITICAL();
        return kStatus_USB_Success;
    }
    error = USB_DeviceSendRequest(hidHandle->handle, ep, buffer, length);
    if (kStatus_USB_Success == error)
    {
        hidHandle->interruptInPipeBusy = 1U;
    }
    USB_OSA_EXIT_CRITICAL();
    return error;
}
‍‍‍‍‍‍‍‍‍‍‍‍‍‍‍‍‍‍‍‍‍‍‍‍‍‍‍‍‍‍‍‍‍‍

The receive function also has to be fixed :

usb_status_t USB_DeviceHidRecv(class_handle_t handle, uint8_t ep, uint8_t *buffer, uint32_t length)
{
    usb_device_hid_struct_t *hidHandle;
    usb_status_t error = kStatus_USB_Error;
    USB_OSA_SR_ALLOC();

    if (!handle)
    {
        return kStatus_USB_InvalidHandle;
    }
    hidHandle = (usb_device_hid_struct_t *)handle;

    if (hidHandle->interruptOutPipeBusy)
    {
        return kStatus_USB_Busy;
    }
    if (hidHandle->interruptOutPipeStall)
    {
        hidHandle->interruptOutPipeBusy = 1U;
        hidHandle->interruptOutPipeDataBuffer = buffer;
        hidHandle->interruptOutPipeDataLen = length;
        return kStatus_USB_Success;
    }
    USB_OSA_ENTER_CRITICAL();
    error = USB_DeviceRecvRequest(hidHandle->handle, ep, buffer, length);
    if (kStatus_USB_Success == error)
    {
        hidHandle->interruptOutPipeBusy = 1U;
    }
    USB_OSA_EXIT_CRITICAL();
    return error;
}
‍‍‍‍‍‍‍‍‍‍‍‍‍‍‍‍‍‍‍‍‍‍‍‍‍‍‍‍‍‍‍‍

[NicolasP]

I updated my project sources with last SDK (V2.5.0).

All critical sections in HID and CDC drivers have been removed !

They must not be removed since without them, code is not thread safe (and maybe not irq safe).