Fix nuc126 buffer copy with IAR #1024
Conversation
| @@ -153,7 +158,7 @@ static void dcd_in_xfer(struct xfer_ctl_t *xfer, USBD_EP_T *ep) | |||
| else | |||
| #endif | |||
| { | |||
| memcpy((uint8_t *)(USBD_BUF_BASE + ep->BUFSEG), xfer->data_ptr, bytes_now); | |||
| usb_memcpy((uint8_t *)(USBD_BUF_BASE + ep->BUFSEG), xfer->data_ptr, bytes_now); | |||
There was a problem hiding this comment.
to be honest, I feel a bit odd about this change, would it work if we cast the buffer with volatile uint8_t* instead of uint8_t*
PS: maybe the usb buffer is byte access only, I will try to check the datasheet later on to confirm this.
There was a problem hiding this comment.
volatile doesn't work as Error[Pe167]: argument of type "uint8_t volatile *" is incompatible with parameter of type "void *"
In fact read setup packet from buffer with memcpy (LDM instruction) works, only write doesn't work.
I read the manual, it didn't mention the access type.
There was a problem hiding this comment.
I checked the manual as well, it is not mentioned. But It is very likely that USB SRAM is byte access. So yeah, this PR is spot on and needed.
https://github.com/majbthrd/nuc_driver/blob/master/nuc126/StdDriver/inc/usbd.h#L524
Only 1 request, would you mind adding an comment at this line and usb_memcpy below to explain the reason something like although it is not mentioned in manual, but the USB SRAM seems to only support byte access and memcpy could possibly do it by words etc ...
| @@ -101,6 +101,11 @@ static void usb_detach(void) | |||
| USBD->SE0 |= USBD_SE0_SE0_Msk; | |||
| } | |||
|
|
|||
| static void usb_memcpy(uint8_t *dest, uint8_t *src, uint16_t size) | |||
There was a problem hiding this comment.
minor but maybe make this function inline
There was a problem hiding this comment.
I realized that dcd nuc120 is very similar to nuc121, and therefore also apply the same changes to it as well. This should be ready to merged once ci passed.
PS: due to the dcd similarity, I open #1029 as reminder to merge them together to make it easier to maintain in the future.
|
I've spotted another issue of device stopped functioning after PC reboot with set address failure. |
|
|
Describe the PR
DCD buffer copy doesn't work correctly with IAR, after investigation IAR use optimized STM/LDM instructions in
memcpybut there must be some access restrictions on USB buffer just like STM32 fsdev.Use byte copy just like StdDriver.
And fix a warning.