Hi Edwin,
Below is an excerpt from USB_DeviceHidSend() function implementation:
main() cycle periodically calls USB_DeviceHidSend()
and USB ISR also calls USB_DeviceHidSend() periodically
USB ISR can occur/intervene at any arbitrary point in main() execution, including the point between checking busy-flag and setting it to 1.
As a result, we will get an inconsistent "parallel" code execution.
And SDK middleware has many functions similar to USB_DeviceHidSend() described. The same applies to example applications.
My proposal is not to change the drivers code, but to change the way the driver functions are used in the application.
I want to disable USB interrupts during USB_DeviceHidSend() is called from the context of main().
Is this a normal idea?
Hi Edwin,
First of all, I would like to apologize for indicating an unofficial repository. When I was just getting started with NXP-chips, a friend recommended setting up MCUXpresso IDE exactly with it. I had no idea it was an unofficial repository:(
In the official repository I don't see interruptInPipeStall and interruptInPipeDataBuffer fields, in the use of which I noticed a problem. So, I don't know if it makes sense to continue the discussion:)
The following code is executed in the main() context:
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 ( NULL == handle )
return kStatus_USB_InvalidHandle;
hidHandle = (usb_device_hid_struct_t*)handle;
if ( 0U != hidHandle->interruptInPipeBusy )
return kStatus_USB_Busy;
hidHandle->interruptInPipeBusy = 1U;
if ( 0U != hidHandle->interruptInPipeStall )
{
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 = 0U;
return error;
}
And the following code is executed under USB-ISR:
case kUSB_DeviceClassEventSetEndpointHalt:
if ( ( NULL == hidHandle->configStruct ) || ( NULL == hidHandle->interfaceHandle ) )
break;
/* Get the endpoint address */
temp8 = ( (uint8_t*)param );
for ( count = 0U; count < hidHandle->interfaceHandle->endpointList.count; count++ )
{
if ( *temp8 == hidHandle->interfaceHandle->endpointList.endpoint[count].endpointAddress )
{
/* Only stall the endpoint belongs to the class */
if ( USB_IN == ( ( hidHandle->interfaceHandle->endpointList.endpoint[count].endpointAddress & USB_DESCRIPTOR_ENDPOINT_ADDRESS_DIRECTION_MASK ) >> USB_DESCRIPTOR_ENDPOINT_ADDRESS_DIRECTION_SHIFT ) )
hidHandle->interruptInPipeStall = 1U;
else
hidHandle->interruptOutPipeStall = 1U;
error = USB_DeviceStallEndpoint( hidHandle->handle, *temp8 );
}
}
break;
case kUSB_DeviceClassEventClearEndpointHalt:
if ( ( NULL == hidHandle->configStruct ) || ( NULL == hidHandle->interfaceHandle ) )
break;
/* Get the endpoint address */
temp8 = ( (uint8_t*)param );
for ( count = 0U; count < hidHandle->interfaceHandle->endpointList.count; count++ )
{
if ( *temp8 == hidHandle->interfaceHandle->endpointList.endpoint[count].endpointAddress )
{
/* Only un-stall the endpoint belongs to the class */
error = USB_DeviceUnstallEndpoint( hidHandle->handle, *temp8 );
if ( USB_IN == ( ( ( *temp8 ) & USB_DESCRIPTOR_ENDPOINT_ADDRESS_DIRECTION_MASK ) >>
USB_DESCRIPTOR_ENDPOINT_ADDRESS_DIRECTION_SHIFT ) )
{
if ( 0U != hidHandle->interruptInPipeStall )
{
hidHandle->interruptInPipeStall = 0U;
if ( (uint8_t*)USB_INVALID_TRANSFER_BUFFER != hidHandle->interruptInPipeDataBuffer )
{
error = USB_DeviceSendRequest( hidHandle->handle, ( hidHandle->interfaceHandle->endpointList.endpoint[count].endpointAddress & USB_DESCRIPTOR_ENDPOINT_ADDRESS_NUMBER_MASK ), hidHandle->interruptInPipeDataBuffer, hidHandle->interruptInPipeDataLen );
if ( kStatus_USB_Success != error )
{
usb_device_endpoint_callback_message_struct_t endpointCallbackMessage;
endpointCallbackMessage.buffer = hidHandle->interruptInPipeDataBuffer;
endpointCallbackMessage.length = hidHandle->interruptInPipeDataLen;
endpointCallbackMessage.isSetup = 0U;
USB_DeviceHidInterruptIn( hidHandle->handle, (void*)&endpointCallbackMessage, handle );
}
hidHandle->interruptInPipeDataBuffer = (uint8_t*)USB_INVALID_TRANSFER_BUFFER;
hidHandle->interruptInPipeDataLen = 0U;
}
}
}
else
{
if ( 0U != hidHandle->interruptOutPipeStall )
{
hidHandle->interruptOutPipeStall = 0U;
if ( (uint8_t*)USB_INVALID_TRANSFER_BUFFER != hidHandle->interruptOutPipeDataBuffer )
{
error = USB_DeviceRecvRequest( hidHandle->handle, ( hidHandle->interfaceHandle->endpointList.endpoint[count].endpointAddress & USB_DESCRIPTOR_ENDPOINT_ADDRESS_NUMBER_MASK ), hidHandle->interruptOutPipeDataBuffer, hidHandle->interruptOutPipeDataLen );
if ( kStatus_USB_Success != error )
{
usb_device_endpoint_callback_message_struct_t endpointCallbackMessage;
endpointCallbackMessage.buffer = hidHandle->interruptOutPipeDataBuffer;
endpointCallbackMessage.length = hidHandle->interruptOutPipeDataLen;
endpointCallbackMessage.isSetup = 0U;
USB_DeviceHidInterruptOut( hidHandle->handle, (void*)&endpointCallbackMessage, handle );
}
hidHandle->interruptOutPipeDataBuffer = (uint8_t*)USB_INVALID_TRANSFER_BUFFER;
hidHandle->interruptOutPipeDataLen = 0U;
}
}
}
}
}
break;
If during main() execution an interrupt is triggered at this point:
, and this interrupt handles kUSB_DeviceClassEventClearEndpointHalt event, then the pointer to a some buffer will be saved in interruptInPipeDataBuffer and interruptInPipeDataLen fields.
And this information will be "hidden/waiting" until the next pair of events kUSB_DeviceClassEventSetEndpointHalt + kUSB_DeviceClassEventClearEndpointHalt. Which will unexpectedly send some content to the host (which at that moment will be in that buffer).
I understand that probability of such event is very low, especially in the case of a keyboard. But a similar operating principle is used in many classes/examples: phdc, com-port, etc. That's why I came up with idea of using a protection in the form of disable+enable USB interrupts inside main().
