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
154 stars 89 forks source link

About cdc-acm support in standalone mode #47

Closed AlexYzhov closed 2 years ago

AlexYzhov commented 2 years ago

Hi there, I'm new to usbx and recently noticed that usbx added standalone mode support in the latest v6.1.10_rel release, so I wrote some codes to initialize standalone usbx device stack as cdc-acm class.

But when I tried to build the firmware, the compiler warned undefined references to ux_device_class_cdc_acmread and ux_device_class_cdc_acmwrite functions.

Then I saw the implementation of these two function were all isolated by UX_DEVICE_STANDALONE and UX_DEVICE_STANDALONE macros.

My question is, are these functions deprecated in standalone mode, or just are lacked with support temporarily ? If these two functions can't be used for now, is there something instead?

And where can I go for further details like these problems, I will be glad to have a migration guide.

Great thanks for your help

xiaocq2001 commented 2 years ago

With RTOS, _read and _write can be done in thread blocking way, without RTOS (standalone) the functions that need polling should be used (_read_run and _write_run functions for device side), While using _run functions, return code indicates how state machine is going (you can also check comments for the function implementation):

/*    State machine Status to check                                       */
/*    UX_STATE_NEXT                         Transfer done, to next state  */
/*    UX_STATE_EXIT                         Abnormal, to reset state      */
/*    (others)                              Keep running, waiting         */
AlexYzhov commented 2 years ago

With RTOS, _read and _write can be done in thread blocking way, without RTOS (standalone) the functions that need polling should be used (_read_run and _write_run functions for device side), While using _run functions, return code indicates how state machine is going (you can also check comments for the function implementation):

/*    State machine Status to check                                       */
/*    UX_STATE_NEXT                         Transfer done, to next state  */
/*    UX_STATE_EXIT                         Abnormal, to reset state      */
/*    (others)                              Keep running, waiting         */

Thanks for your reply!

After a workday I can now enumerate cdc-acm device normally on my stm32h750 board :) But still got some issues about device IO.

I checked the implementations of these two function, and it seems they all need _dcd->ux_slave_dcdfunction() in their call chain, which is registered by device controller implementation, such as ux_dcd_stm32_function.c in elder version of usbx.

What I'm confused is the meaning of _dcd->ux_slave_dcdfunction() return value seems changed in standalone mode.

For example, in standalone mode ux_device_stack_transfer_run() calls _dcd->ux_slave_dcdfuntion() by passing UX_DCD_TRANSFER_RUN flag, and checks the return value of _dcd->ux_slave_dcdfuntion() hook to push state machine.

But in the elder version of usbx, when function flag is given as UX_DCD_TRANSFER_RUN, the hook implementation provided by ux_dcd_stm32_function.c calls ux_dcd_stm32_transfer_request() , which only returns succuss or not. It's obvious that ux_dcd_stm32_transfer_request() can't be used to push state machine run.

So my question now is, is there something can guide me write a correct implementation of _dcd->ux_slave_dcdfunction() hook for standalone mode ?

Additionally, how does ux_system_tasks_run.c work ? Should I find a periodic context to run this fucntion ?

AlexYzhov commented 2 years ago

I hacked line 117 in ux_dcd_stm32_function.c :

status = (_ux_dcd_stm32_transfer_request(dcd_stm32, (UX_SLAVE_TRANSFER *) parameter) == UX_SUCCESS) ? (UX_STATE_NEXT) : (UX_STATE_ERROR);

And wrote a demo read function:

/* reset transmission status */
cdc_acm->ux_slave_class_cdc_acm_transmission_status = UX_FALSE;

while (1)
{
    uint32_t rlen = 0;
     UINT status = ux_device_class_cdc_acm_read_run(cdc_acm, &buf[0], len, &rlen);
     switch (status)
    {
        case UX_STATE_NEXT:
            return rlen;
        case UX_STATE_EXIT:
            return 0;
        default:
            break;
    }
};

And it seems now data from OUT endpoint (which is sent by PC in buffered mode) can be read into usbx internal buffer _transfer_request->ux_slave_transfer_request_datapointer by reading back memories: image

But because of _cdc_acm -> ux_device_class_cdc_acm_read_actuallength in line 228 of ux_device_class_cdc_acm_read_run.c always remains 0, no data was copied out to user buffer.

I guess there is something wrong with collecting data with multiple transfer. Still need a correct implementation of _dcd->ux_slave_dcdfunction()

xiaocq2001 commented 2 years ago

Yes. In standalone mode, there is some recommendation:

  1. handle UX_DCD_ISR_PENDING, as background task (called by ux_system_tasks_run, which is polled in main loop), to handle control requests outside of interrupt services. Actually without this change the enumeration is fine, but there may be issue if non-control transfer could be interrupted by control requests.
  2. Implement UX_DCD_TRANSFER_REQUEST in state machine way for _run functions (return _WAIT/_NEXT) for state progress, the parts may related is transfer implementation and ST HAL DCD callback implementation, since there could be status flags changes, the endpoint reset and abort process could also be changed.
AlexYzhov commented 2 years ago

Yes. In standalone mode, there is some recommendation:

  1. handle UX_DCD_ISR_PENDING, as background task (called by ux_system_tasks_run, which is polled in main loop), to handle control requests outside of interrupt services. Actually without this change the enumeration is fine, but there may be issue if non-control transfer could be interrupted by control requests.
  2. Implement UX_DCD_TRANSFER_REQUEST in state machine way for _run functions (return _WAIT/_NEXT) for state progress, the parts may related is transfer implementation and ST HAL DCD callback implementation, since there could be status flags changes, the endpoint reset and abort process could also be changed.

Thanks for your suggestion, I'll do more research this weekend.

AYEDMSTM commented 2 years ago

HI @AlexYzhov ,

This is a sample example showing support of USB CDC-ACM Standalone mode that can be accessed from ST github .

https://github.com/STMicroelectronics/x-cube-azrtos-h7/tree/dev/usbx/Projects/NUCLEO-H723ZG/Applications/USBX/Ux_Device_CDC_ACM_Standalone

Regards,