Skip to content

Commit 9c81504

Browse files
committed
switch usb buffer fix to static buffer
This modifies tannewt's DMA-safe buffer fix for usb.core.Device to use a single global static buffer instead of potentially many heap allocated buffers. Hopefully the shared buffer will avoid runtime allocation errors. CAUTION: The size of the buffer may not be big enough for some transferst that people might want to make. If the requested length of a transfer is too big, that length will be truncated to what can fit in the buffer. The actual amount of transferred data will be indicated by the length in the transfer result. The calling code _should definitely_ check to see if the result matched what they asked for.
1 parent a2c4dfa commit 9c81504

File tree

3 files changed

+51
-52
lines changed

3 files changed

+51
-52
lines changed

locale/circuitpython.pot

Lines changed: 0 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -844,10 +844,6 @@ msgstr ""
844844
msgid "Coordinate arrays types have different sizes"
845845
msgstr ""
846846

847-
#: shared-module/usb/core/Device.c
848-
msgid "Could not allocate DMA capable buffer"
849-
msgstr ""
850-
851847
#: ports/espressif/common-hal/rclcpy/Publisher.c
852848
msgid "Could not publish to ROS topic"
853849
msgstr ""

shared-module/usb/core/Device.c

Lines changed: 51 additions & 44 deletions
Original file line numberDiff line numberDiff line change
@@ -37,37 +37,42 @@ static xfer_result_t _xfer_result;
3737
static size_t _actual_len;
3838

3939
#if !CIRCUITPY_ALL_MEMORY_DMA_CAPABLE
40+
// Allocate a buffer in SRAM so it's safe to use with DMA
41+
#define USB_CORE_SRAM_BUF_SIZE (256)
42+
static uint8_t _dma_safe_buffer[USB_CORE_SRAM_BUF_SIZE];
43+
4044
// Helper to ensure buffer is DMA-capable for transfer operations
4145
static uint8_t *_ensure_dma_buffer(usb_core_device_obj_t *self, const uint8_t *buffer, size_t len, bool for_write) {
4246
if (port_buffer_is_dma_capable(buffer)) {
4347
return (uint8_t *)buffer; // Already DMA-capable, use directly
4448
}
4549

46-
// Need to allocate/reallocate temporary buffer in DMA-capable memory
47-
if (self->temp_buffer_size < len) {
48-
self->temp_buffer = port_realloc(self->temp_buffer, len, true); // true = DMA capable
49-
if (self->temp_buffer == NULL) {
50-
self->temp_buffer_size = 0;
51-
return NULL; // Allocation failed
52-
}
53-
self->temp_buffer_size = len;
54-
}
50+
// If buffer was in PSRAM, use DMA-safe static buffer from SRAM instead
5551

5652
// Copy data to DMA buffer if writing
5753
if (for_write && buffer != NULL) {
58-
memcpy(self->temp_buffer, buffer, len);
54+
// CAUTION: This will truncate the requested length if it is larger than the
55+
// size of the DMA-safe SRAM buffer. If that becomes a problem, then consider
56+
// increasing the size of USB_CORE_SRAM_BUF_SIZE.
57+
uint32_t dma_len = len;
58+
if (len > sizeof(_dma_safe_buffer)) {
59+
dma_len = sizeof(_dma_safe_buffer);
60+
}
61+
memcpy(_dma_safe_buffer, buffer, dma_len);
5962
}
6063

61-
return self->temp_buffer;
64+
return _dma_safe_buffer;
6265
}
6366

6467
// Copy data back from DMA buffer to original buffer after read
65-
static void _copy_from_dma_buffer(usb_core_device_obj_t *self, uint8_t *original_buffer, size_t len) {
66-
if (!port_buffer_is_dma_capable(original_buffer) && self->temp_buffer != NULL) {
67-
memcpy(original_buffer, self->temp_buffer, len);
68+
static void _copy_from_dma_buffer(uint8_t *original_buffer, size_t len) {
69+
if (original_buffer != _dma_safe_buffer) {
70+
size_t limit = (len <= sizeof(_dma_safe_buffer)) ? len : sizeof(_dma_safe_buffer);
71+
memcpy(original_buffer, _dma_safe_buffer, limit);
6872
}
6973
}
7074
#endif
75+
7176
bool common_hal_usb_core_device_construct(usb_core_device_obj_t *self, uint8_t device_address) {
7277
if (!tuh_inited()) {
7378
mp_raise_RuntimeError(MP_ERROR_TEXT("No usb host port initialized"));
@@ -81,10 +86,6 @@ bool common_hal_usb_core_device_construct(usb_core_device_obj_t *self, uint8_t d
8186
}
8287
self->device_address = device_address;
8388
self->first_langid = 0;
84-
#if !CIRCUITPY_ALL_MEMORY_DMA_CAPABLE
85-
self->temp_buffer = NULL;
86-
self->temp_buffer_size = 0;
87-
#endif
8889
_xfer_result = XFER_RESULT_INVALID;
8990
return true;
9091
}
@@ -104,14 +105,6 @@ void common_hal_usb_core_device_deinit(usb_core_device_obj_t *self) {
104105
self->open_endpoints[i] = 0;
105106
}
106107
}
107-
#if !CIRCUITPY_ALL_MEMORY_DMA_CAPABLE
108-
// Clean up temporary buffer
109-
if (self->temp_buffer != NULL) {
110-
port_free(self->temp_buffer);
111-
self->temp_buffer = NULL;
112-
self->temp_buffer_size = 0;
113-
}
114-
#endif
115108
self->device_address = 0;
116109
}
117110

@@ -450,19 +443,23 @@ mp_int_t common_hal_usb_core_device_write(usb_core_device_obj_t *self, mp_int_t
450443
#if !CIRCUITPY_ALL_MEMORY_DMA_CAPABLE
451444
// Ensure buffer is in DMA-capable memory
452445
uint8_t *dma_buffer = _ensure_dma_buffer(self, buffer, len, true); // true = for write
453-
if (dma_buffer == NULL) {
454-
mp_raise_msg(&mp_type_MemoryError, MP_ERROR_TEXT("Could not allocate DMA capable buffer"));
455-
return 0;
446+
// CAUTION: This may truncate the requested length if it is larger than the
447+
// size of the DMA-safe SRAM buffer. If that becomes a problem, then consider
448+
// increasing the size of USB_CORE_SRAM_BUF_SIZE.
449+
uint32_t dma_len = len;
450+
if ((dma_buffer == _dma_safe_buffer) && (dma_len > sizeof(_dma_safe_buffer))) {
451+
dma_len = sizeof(_dma_safe_buffer);
456452
}
457453
#else
458454
uint8_t *dma_buffer = (uint8_t *)buffer; // All memory is DMA-capable
455+
uint32_t dma_len = len;
459456
#endif
460457

461458
tuh_xfer_t xfer;
462459
xfer.daddr = self->device_address;
463460
xfer.ep_addr = endpoint;
464461
xfer.buffer = dma_buffer;
465-
xfer.buflen = len;
462+
xfer.buflen = dma_len;
466463
return _xfer(&xfer, timeout);
467464
}
468465

@@ -475,25 +472,30 @@ mp_int_t common_hal_usb_core_device_read(usb_core_device_obj_t *self, mp_int_t e
475472
#if !CIRCUITPY_ALL_MEMORY_DMA_CAPABLE
476473
// Ensure buffer is in DMA-capable memory
477474
uint8_t *dma_buffer = _ensure_dma_buffer(self, buffer, len, false); // false = for read
478-
if (dma_buffer == NULL) {
479-
mp_raise_msg(&mp_type_MemoryError, MP_ERROR_TEXT("Could not allocate DMA capable buffer"));
480-
return 0;
475+
// CAUTION: This may truncate the requested length if it is larger than the
476+
// size of the DMA-safe SRAM buffer. If that becomes a problem, then consider
477+
// increasing the size of USB_CORE_SRAM_BUF_SIZE.
478+
uint32_t dma_len = len;
479+
if ((dma_buffer == _dma_safe_buffer) && (dma_len > sizeof(_dma_safe_buffer))) {
480+
dma_len = sizeof(_dma_safe_buffer);
481481
}
482482
#else
483483
uint8_t *dma_buffer = buffer; // All memory is DMA-capable
484+
uint32_t dma_len = len;
484485
#endif
485486

486487
tuh_xfer_t xfer;
487488
xfer.daddr = self->device_address;
488489
xfer.ep_addr = endpoint;
489490
xfer.buffer = dma_buffer;
490-
xfer.buflen = len;
491-
mp_int_t result = _xfer(&xfer, timeout);
491+
xfer.buflen = dma_len;
492+
size_t result = _xfer(&xfer, timeout);
492493

493494
#if !CIRCUITPY_ALL_MEMORY_DMA_CAPABLE
494495
// Copy data back to original buffer if needed
495-
if (result > 0) {
496-
_copy_from_dma_buffer(self, buffer, result);
496+
if (result > 0 && (buffer != dma_buffer)) {
497+
size_t limit = (result <= dma_len) ? result : dma_len;
498+
_copy_from_dma_buffer(buffer, limit);
497499
}
498500
#endif
499501

@@ -514,21 +516,25 @@ mp_int_t common_hal_usb_core_device_ctrl_transfer(usb_core_device_obj_t *self,
514516
uint8_t *dma_buffer = NULL;
515517
if (len > 0 && buffer != NULL) {
516518
dma_buffer = _ensure_dma_buffer(self, buffer, len, is_write);
517-
if (dma_buffer == NULL) {
518-
mp_raise_msg(&mp_type_MemoryError, MP_ERROR_TEXT("Could not allocate DMA capable buffer"));
519-
return 0;
520-
}
519+
}
520+
// CAUTION: This may truncate the requested length if it is larger than the
521+
// size of the DMA-safe SRAM buffer. If that becomes a problem, then consider
522+
// increasing the size of USB_CORE_SRAM_BUF_SIZE.
523+
uint16_t dma_len = len;
524+
if ((dma_buffer == _dma_safe_buffer) && (dma_len > sizeof(_dma_safe_buffer))) {
525+
dma_len = sizeof(_dma_safe_buffer);
521526
}
522527
#else
523528
uint8_t *dma_buffer = buffer; // All memory is DMA-capable
529+
uint16_t dma_len = len;
524530
#endif
525531

526532
tusb_control_request_t request = {
527533
.bmRequestType = bmRequestType,
528534
.bRequest = bRequest,
529535
.wValue = wValue,
530536
.wIndex = wIndex,
531-
.wLength = len
537+
.wLength = dma_len
532538
};
533539
tuh_xfer_t xfer = {
534540
.daddr = self->device_address,
@@ -547,8 +553,9 @@ mp_int_t common_hal_usb_core_device_ctrl_transfer(usb_core_device_obj_t *self,
547553

548554
#if !CIRCUITPY_ALL_MEMORY_DMA_CAPABLE
549555
// Copy data back to original buffer if this was a read transfer and we got data
550-
if ((bmRequestType & 0x80) != 0 && result > 0 && buffer != NULL) { // Read transfer (device-to-host)
551-
_copy_from_dma_buffer(self, buffer, result);
556+
if ((bmRequestType & 0x80) != 0 && result > 0 && (buffer != dma_buffer)) { // Read transfer (device-to-host)
557+
size_t limit = (result <= dma_len) ? result : dma_len;
558+
_copy_from_dma_buffer(buffer, limit);
552559
}
553560
#endif
554561

shared-module/usb/core/Device.h

Lines changed: 0 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -15,8 +15,4 @@ typedef struct {
1515
uint8_t *configuration_descriptor; // Contains the length of the all descriptors.
1616
uint8_t open_endpoints[8];
1717
uint16_t first_langid;
18-
#if !CIRCUITPY_ALL_MEMORY_DMA_CAPABLE
19-
uint8_t *temp_buffer; // Temporary buffer for PSRAM data
20-
size_t temp_buffer_size; // Size of temporary buffer
21-
#endif
2218
} usb_core_device_obj_t;

0 commit comments

Comments
 (0)