FSDEV fix/cleanup. #2574
FSDEV fix/cleanup. #2574
Conversation
HiFiPhile
force-pushed
the
fsdev
branch
3 times, most recently
from
eb34c2e to
34f3340
Compare
| @@ -1055,8 +977,17 @@ bool dcd_edpt_iso_alloc(uint8_t rhport, uint8_t ep_addr, uint16_t largest_packet | |||
|
|
|||
| pcd_set_eptype(USB, ep_idx, USB_EP_ISOCHRONOUS); | |||
|
|
|||
| pcd_set_ep_tx_address(USB, ep_idx, pma_addr); | |||
| pcd_set_ep_rx_address(USB, ep_idx, pma_addr); | |||
| // Disable double buffering | |||
There was a problem hiding this comment.
I would prefer to keep double buffering for ISO if possible. Otherwise it may not keep up to 1023 byte per packet. Maybe we could find anothe way to fix this ?
In fact currently double buffering is not configured correctly, it's enabled but both buffer are pointed to the same address. Maybe it's better to fix the buffer stuff to enable true double buffering, since on older chips there is no mention that ISO double buffering can be disabled: |
Yeah it's broken on L0... |
thanks for testing it out, seems like we will definitely need double buffer for ISO. |
![github-advanced-security[bot]](https://avatars.githubusercontent.com/in/57789?s=60&v=4){kind=small}
|
I've reworked the PR, now it has correct ISO double buffering handing. It's enabled only for device with 2048b PMA, smaller devices has 2 buffers point to the same address as before, otherwise they will fail to run some examples. In my tests I didn't see single buffering cause any issue. Tested on L0, G0, H5 to be sure nothing broken. I put each change in one commit hope it's easier to review... |
|
superb !! thank you, will review and test this out soon enough. |
| uint16_t remaining = xfer->total_len - xfer->queued_len; | ||
| uint16_t cnt = remaining >= xfer->max_packet_size ? xfer->max_packet_size : remaining; | ||
| pcd_set_ep_rx_cnt(USB, EPindex, cnt); | ||
| pcd_set_ep_rx_cnt(USB, EPindex, remaining); |
There was a problem hiding this comment.
the call with remaining should be removed
There was a problem hiding this comment.
Good spot !
In fact I'm not sure if it's even necessary, pcd_set_ep_rx_cnt() only reduce buffer size.
| if((xfer->total_len != xfer->queued_len)) /* TX not complete */ | ||
| xfer_ctl_t *xfer = xfer_ctl_ptr(ep_addr); | ||
|
|
||
| /* Ignore spurious int */ |
There was a problem hiding this comment.
do we still have issue with this spurious interrupt ? I wondered if it is caused by incorrect double buffer configuration previously.
There was a problem hiding this comment.
It's still necessary since Tx interrupt can't be masked, if I disable the EP the host will not happy seeing NAK.
Another way to fix is in classes driver's xfer_cb, always call usbd_edpt_xfer for iso even if there is nothing to send.
There was a problem hiding this comment.
No need to change the class driver, this is specifci to this dcd only. I see host send IN token while have nothing to send and ISO does not have NAK --> triggered interrupt with zero byte data. I think this is all good now. Though I think we should make it more ISO explicit thing (making changes atm)
- minor code format
Describe the PR
Tested on
NUCLEO-G0B1RE,STM32L0538DISCO,STM32H573i-DKwith audio, video andcdc_dual_portsexamples.