Movatterモバイル変換


[0]ホーム

URL:


Skip to content

Navigation Menu

Sign in
Appearance settings

Search code, repositories, users, issues, pull requests...

Provide feedback

We read every piece of feedback, and take your input very seriously.

Saved searches

Use saved searches to filter your results more quickly

Sign up
Appearance settings

chore(usb): cdc_queue now use uint32_t length#2550

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to ourterms of service andprivacy statement. We’ll occasionally send you account related emails.

Already on GitHub?Sign in to your account

Open
warmonkey wants to merge5 commits intostm32duino:main
base:main
Choose a base branch
Loading
fromwarmonkey:main

Conversation

warmonkey
Copy link
Contributor

@warmonkeywarmonkey commentedNov 4, 2024
edited by fpistm
Loading

raise cdc buffer size 32KB limit due to uint16_t length variables.

This PR fixes bug: cdc queue limited
Now CDC_TRANSMIT_QUEUE_BUFFER_PACKET_NUMBER and/or CDC_RECEIVE_QUEUE_BUFFER_PACKET_NUMBER both can be greater than 512. which every buffer size is 64B, 64*512=32768

…E_BUFFER_PACKET_NUMBER>512 or CDC_RECEIVE_QUEUE_BUFFER_PACKET_NUMBER>512.
which allows CDC_TRANSMIT_QUEUE_BUFFER_PACKET_NUMBER>512 orCDC_RECEIVE_QUEUE_BUFFER_PACKET_NUMBER>512.Signed-off-by: warmonkey <luoshumymail@gmail.com>
@fpistmfpistm reopened thisNov 5, 2024
@fpistmfpistm changed the titlecdc_queue.cpp change to uint32_t lengthchore(usb): cdc_queue now use uint32_t lengthNov 5, 2024
@fpistmfpistm added the enhancementNew feature or request labelNov 5, 2024
@fpistmfpistm added this to the2.9.0 milestoneNov 5, 2024
@fpistmfpistm removed this from the2.9.0 milestoneNov 22, 2024
@fpistm
Copy link
Member

Hi@warmonkey
Sorry for the delay. 😢
I've dig into HAL to see why it was uint16_t and found the reason, even if changed to uint32_t it will be always truncated to uint16_t when accessing the PMA (R/W).

voidUSB_WritePMA(USB_TypeDefconst*USBx,uint8_t*pbUsrBuf,uint16_twPMABufAddr,uint16_twNBytes)

So I will not merge this PR, sorry.

@fpistmfpistm closed thisFeb 18, 2025
@fpistmfpistm added the invalidThis doesn't seem right labelFeb 18, 2025
@warmonkey
Copy link
ContributorAuthor

warmonkey commentedMay 20, 2025
edited
Loading

Hi@warmonkey Sorry for the delay. 😢 I've dig into HAL to see why it was uint16_t and found the reason, even if changed to uint32_t it will be always truncated to uint16_t when accessing the PMA (R/W).

voidUSB_WritePMA(USB_TypeDefconst*USBx,uint8_t*pbUsrBuf,uint16_twPMABufAddr,uint16_twNBytes)

So I will not merge this PR, sorry.

On STM32F4 USB_WritePMA will not be called. The actual function to send data over USB is USBD_LL_Transmit() -> ... -> USB_EPStartXfer()

There two paths can trigger the USBD_LL_Transmit(): USBSerial::write() -> CDC_continue_transmit(), USBD_CDC_TransmitCplt() -> CDC_continue_transmit() -> USBD_CDC_TransmitPacket() -> USBD_LL_Transmit() -> HAL_PCD_EP_Transmit() -> USB_EPStartXfer()

In CDC_continue_transmit() file: usbd_cdc_if.c line 345, it's callingbuffer = CDC_TransmitQueue_ReadBlock(&TransmitQueue, &size); which returns the size to send.
In CDC_TransmitQueue_ReadBlock() file: cdc_queue.c line 88, the size value is limited to 65472 in my commit

as stated in stm32f4xx_ll_usb.c function USB_EPStartXfer line 782:

Program the transfer size and packet count as follows: xfersize = N * maxpacket + short_packet pktcnt = N + (short_packet_exist ? 1 : 0)...pktcnt = (uint16_t)((ep->xfer_len + ep->maxpacket - 1U) / ep->maxpacket);USBx_INEP(epnum)->DIEPTSIZ |= (USB_OTG_DIEPTSIZ_PKTCNT & (pktcnt << 19));...USBx_INEP(epnum)->DIEPTSIZ |= (USB_OTG_DIEPTSIZ_XFRSIZ & ep->xfer_len);

from the code above we know that the size to send from CDC_TransmitQueue_ReadBlock() previously, is assigned to ep->xfer_len (at stm32f4xx_hal_pcd.c line 1940 for stm32f4), then write into OTG_FS_DIEPTSIZx or OTG_HS_DIEPTSIZx register as:

PKTCNT = (int)(xfer_len/MAX_PACKET_SIZE)     //64 for FS, 512 for HSXFRSIZ = xfer_len

from RM0008 RM0090 RM0432 we know the PKTCNT has 10bits, max value 0x3ff (1023), XFRSIZ has 19bits, max value 0x7ffff (524287). 1023 * 512 = 523776 < 524287

but, it's not easy to know the USB is running in HS or FS mode.

In conclusion, we should limit the maximum size returned by CDC_TransmitQueue_ReadBlock() to OTG_MAX_PKTCNTUSB_FS_MAX_PACKET_SIZE which is 102364 (for now).

I will readpdev->ep_in[CDCInEpAdd & 0xFU].maxpacket instead in the next commit.


The CDC queue is a software buffer, it's not relevant to the PMA area. The PMA is located in0x4000 6000(1) - 0x4000 63FF Shared USB/CAN SRAM 512 bytes (RM0008 Page 52) it has only 512 bytes shared with multiple endpoints. User should copy data from/to PMA manually.

Before I submit this PR I checked multiple times, compiled and deployed on actual products. I ran the test for at least 24hrs.

@fpistmfpistm reopened thisMay 20, 2025
@fpistmfpistm removed the invalidThis doesn't seem right labelMay 20, 2025
@warmonkey
Copy link
ContributorAuthor

warmonkey commentedMay 20, 2025
edited
Loading

Im working on an improvement. Will usemaxpacket in USBD_EndpointTypeDef instead of use a fixed value 64. Just finished but i need more real world test.

Sign up for freeto join this conversation on GitHub. Already have an account?Sign in to comment
Reviewers
No reviews
Assignees
No one assigned
Labels
enhancementNew feature or request
Projects
Status: In progress
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

2 participants
@warmonkey@fpistm

[8]ページ先頭

©2009-2025 Movatter.jp