x-cube-azrtos-h7 icon indicating copy to clipboard operation
x-cube-azrtos-h7 copied to clipboard

usbx cdc acm, missing zlp

Open chr-btz opened this issue 2 years ago • 3 comments

Board: Nucleo F429zi IDE: CubeIDE

When the endpoints max packet size equals the data to send size, usbx does not send a zlp, therefore the host does not recognize the transfer as complete.

Reproduce:

  1. Ux_Device_CDC_ACM example

  2. Change usbx_cdc_acm_write_thread_entry to something like this:

     /* Get the device */
     device = &_ux_system_slave->ux_system_slave_device;
    
     /* Get the data interface */
     data_interface = device->ux_slave_device_first_interface->ux_slave_interface_next_interface;
    
     /* Get the cdc Instance */
     cdc_acm = data_interface->ux_slave_interface_class_instance;
    
     cdc_acm -> ux_slave_class_cdc_acm_transmission_status = UX_FALSE;
    
     buffsize = 0;
    
     for(int i = 0; i < testlength; i++)
     {
     	UserTxBufferFS[i] = '1';
     	buffsize++;
     }
    
     /* Send data over the class cdc_acm_write */
     ux_status = ux_device_class_cdc_acm_write(cdc_acm,
     								(UCHAR *)UserTxBufferFS,
     								buffsize, &actual_length);
    
     /* Check if dataset is correctly transmitted */
     if (ux_status)
     {
     	tx_thread_sleep(100);
     }
     tx_thread_sleep(100);
    
  3. Set testlength to endpoints max packet size

  4. Host wont recognize data

Analysis:

After investigation i found several issues,

host_length != slave_length will be false even if zlp is needed

https://github.com/STMicroelectronics/x-cube-azrtos-h7/blob/84ed811f56e0dd1e80ea7cec10e8ac62c5df6168/Middlewares/ST/usbx/common/core/src/ux_device_stack_transfer_request.c#L150-L168

even if transfer_request -> ux_slave_transfer_request_force_zlp gets set to UX_TRUE the zlp wont be sended because its exclusive to endpoint 0, see:

https://github.com/STMicroelectronics/x-cube-azrtos-h7/blob/84ed811f56e0dd1e80ea7cec10e8ac62c5df6168/Middlewares/ST/usbx/common/usbx_stm32_device_controllers/ux_dcd_stm32_callback.c#L237-L349

when removed the host_length != slave_length in issue one and use this code for HAL_PCD_DataInStageCallback its working:

void HAL_PCD_DataInStageCallback(PCD_HandleTypeDef *hpcd, uint8_t epnum)
{

UX_SLAVE_DCD            *dcd;
UX_DCD_STM32            *dcd_stm32;
UX_DCD_STM32_ED         *ed;
UX_SLAVE_TRANSFER       *transfer_request;
ULONG                   transfer_length;
UX_SLAVE_ENDPOINT       *endpoint;


    /* Get the pointer to the DCD.  */
    dcd =  &_ux_system_slave -> ux_system_slave_dcd;

    /* Get the pointer to the STM32 DCD.  */
    dcd_stm32 = (UX_DCD_STM32 *) dcd -> ux_slave_dcd_controller_hardware;

    /* Fetch the address of the physical endpoint.  */
    ed =  &dcd_stm32 -> ux_dcd_stm32_ed[epnum & 0xF];

    /* Get the pointer to the transfer request.  */
    transfer_request =  &(ed -> ux_dcd_stm32_ed_endpoint -> ux_slave_endpoint_transfer_request);

    /* Endpoint 0 is different.  */
    if (epnum == 0U)
    {

        /* Get the pointer to the logical endpoint from the transfer request.  */
        endpoint =  transfer_request -> ux_slave_transfer_request_endpoint;

        /* Check if we need to send data again on control endpoint. */
        if (ed -> ux_dcd_stm32_ed_state == UX_DCD_STM32_ED_STATE_DATA_TX)
        {

            /* Arm Status transfer.  */
            HAL_PCD_EP_Receive(hpcd, 0, 0, 0);

            /* Are we done with this transfer ? */
            if (transfer_request -> ux_slave_transfer_request_in_transfer_length <= endpoint -> ux_slave_endpoint_descriptor.wMaxPacketSize)
            {

                /* There is no data to send but we may need to send a Zero Length Packet.  */
                if (transfer_request -> ux_slave_transfer_request_force_zlp ==  UX_TRUE)
                {

                    /* Arm a ZLP packet on IN.  */
                    HAL_PCD_EP_Transmit(hpcd, 
                            endpoint->ux_slave_endpoint_descriptor.bEndpointAddress, 0, 0);

                    /* Reset the ZLP condition.  */
                    transfer_request -> ux_slave_transfer_request_force_zlp =  UX_FALSE;

                }
                else
                {

                    /* Set the completion code to no error.  */
                    transfer_request -> ux_slave_transfer_request_completion_code =  UX_SUCCESS;

                    /* The transfer is completed.  */
                    transfer_request -> ux_slave_transfer_request_status =  UX_TRANSFER_STATUS_COMPLETED;

                    /* We are using a Control endpoint, if there is a callback, invoke it. We are still under ISR.  */
                    if (transfer_request -> ux_slave_transfer_request_completion_function)  
                        transfer_request -> ux_slave_transfer_request_completion_function (transfer_request) ;

                    /* State is now STATUS RX.  */
                    ed -> ux_dcd_stm32_ed_state = UX_DCD_STM32_ED_STATE_STATUS_RX;
                }
            }
            else
            {

                /* Get the size of the transfer.  */
                transfer_length = transfer_request -> ux_slave_transfer_request_in_transfer_length - endpoint -> ux_slave_endpoint_descriptor.wMaxPacketSize;

                /* Check if the endpoint size is bigger that data requested. */
                if (transfer_length > endpoint -> ux_slave_endpoint_descriptor.wMaxPacketSize)
                {

                    /* Adjust the transfer size.  */
                    transfer_length =  endpoint -> ux_slave_endpoint_descriptor.wMaxPacketSize;
                }

                /* Adjust the data pointer.  */
                transfer_request -> ux_slave_transfer_request_current_data_pointer += endpoint -> ux_slave_endpoint_descriptor.wMaxPacketSize;

                /* Adjust the transfer length remaining.  */
                transfer_request -> ux_slave_transfer_request_in_transfer_length -= transfer_length;

                /* Transmit data.  */
                HAL_PCD_EP_Transmit(hpcd, 
                            endpoint->ux_slave_endpoint_descriptor.bEndpointAddress, 
                            transfer_request->ux_slave_transfer_request_current_data_pointer,
                            transfer_length);
            }
        }
    }
    else
    {
	    if (transfer_request -> ux_slave_transfer_request_force_zlp ==  UX_TRUE)
	    {
            /* Get the pointer to the logical endpoint from the transfer request.  */
            endpoint =  transfer_request -> ux_slave_transfer_request_endpoint;

		    /* Arm a ZLP packet on IN.  */
		    HAL_PCD_EP_Transmit(hpcd,
				    endpoint->ux_slave_endpoint_descriptor.bEndpointAddress, 0, 0);

		    /* Reset the ZLP condition.  */
		    transfer_request -> ux_slave_transfer_request_force_zlp =  UX_FALSE;

	    }else{
		    /* Set the completion code to no error.  */
		    transfer_request -> ux_slave_transfer_request_completion_code =  UX_SUCCESS;

		    /* The transfer is completed.  */
		    transfer_request -> ux_slave_transfer_request_status =  UX_TRANSFER_STATUS_COMPLETED;

		    /* Non control endpoint operation, use semaphore.  */
		    _ux_utility_semaphore_put(&transfer_request -> ux_slave_transfer_request_semaphore);
	    }
    }
}

chr-btz avatar Mar 01 '22 15:03 chr-btz

I am having the same issue using PIMA class. The ZLP is not transmitted. Using your modified callback function that send ZLP for other endpoint than 0 works for me. Thank you!

fhorinek avatar Apr 20 '22 06:04 fhorinek

Hi @chr-btz,

Thanks for your report, The issue you pointed out is confirmed. This will be fixed in the next release.

BR.

AYEDMSTM avatar May 05 '22 14:05 AYEDMSTM

hi @chr-btz , This issue fixed in V3.0.0 release, you need to define UX_DEVICE_CLASS_CDC_ACM_WRITE_AUTO_ZLP https://github.com/STMicroelectronics/x-cube-azrtos-h7/blob/57faa03d093cb37ad341798d76886af15decdc74/Projects/NUCLEO-H723ZG/Applications/USBX/Ux_Device_CDC_ACM/USBX/App/ux_user.h#L359

Best Regards.

AYEDMSTM avatar Dec 07 '22 16:12 AYEDMSTM