Skip to content

Commit 87d0711

Browse files
committed
qspibus: address code review feedback
- Reuse existing CircuitPython error messages to reduce translation burden: "QSPI command/color timeout" -> "Operation timed out", "QSPI send[color] failed: %d" / "SPI bus init failed: %d" / "Panel IO init failed: %d" -> "%q failure: %d", "Failed to allocate DMA buffers" / "QSPI DMA buffers unavailable" -> "Could not allocate DMA capable buffer", "Data buffer is null" -> "Buffer too small". Net removal of 8 unique translatable strings with 0 new ones. - Add _Static_assert in BusDisplay.c to guard the QSPI stack-allocated refresh buffer size (max 2048 uint32_t words = 8KB). - Add comment clarifying that inflight_transfers bookkeeping is task-context only (ISR only signals semaphore), so no atomics needed. - Fix SPDX file header format across all new qspibus files to match the CircuitPython project convention.
1 parent 1934060 commit 87d0711

11 files changed

Lines changed: 27 additions & 20 deletions

File tree

ports/espressif/boards/waveshare_esp32_s3_amoled_241/board.c

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,5 @@
1+
// This file is part of the CircuitPython project: https://circuitpython.org
12
// SPDX-FileCopyrightText: Copyright (c) 2026 Przemyslaw Patrick Socha
2-
//
33
// SPDX-License-Identifier: MIT
44

55
#include "supervisor/board.h"

ports/espressif/boards/waveshare_esp32_s3_amoled_241/mpconfigboard.h

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,5 @@
1+
// This file is part of the CircuitPython project: https://circuitpython.org
12
// SPDX-FileCopyrightText: Copyright (c) 2026 Przemyslaw Patrick Socha
2-
//
33
// SPDX-License-Identifier: MIT
44

55
#pragma once

ports/espressif/boards/waveshare_esp32_s3_amoled_241/pins.c

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,5 @@
1+
// This file is part of the CircuitPython project: https://circuitpython.org
12
// SPDX-FileCopyrightText: Copyright (c) 2026 Przemyslaw Patrick Socha
2-
//
33
// SPDX-License-Identifier: MIT
44

55
#include "py/obj.h"

ports/espressif/common-hal/qspibus/QSPIBus.c

Lines changed: 14 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,5 @@
1+
// This file is part of the CircuitPython project: https://circuitpython.org
12
// SPDX-FileCopyrightText: Copyright (c) 2026 Przemyslaw Patrick Socha
2-
//
33
// SPDX-License-Identifier: MIT
44

55
#include "shared-bindings/qspibus/QSPIBus.h"
@@ -121,15 +121,15 @@ static void qspibus_send_command_bytes(
121121
if (self->inflight_transfers >= QSPI_DMA_BUFFER_COUNT) {
122122
if (!qspibus_wait_one_transfer_done(self, pdMS_TO_TICKS(QSPI_COLOR_TIMEOUT_MS))) {
123123
qspibus_reset_transfer_state(self);
124-
mp_raise_OSError_msg(MP_ERROR_TEXT("QSPI command timeout"));
124+
mp_raise_OSError_msg(MP_ERROR_TEXT("Operation timed out"));
125125
}
126126
}
127127

128128
uint32_t packed_cmd = ((uint32_t)QSPI_OPCODE_WRITE_CMD << 24) | ((uint32_t)command << 8);
129129
esp_err_t err = esp_lcd_panel_io_tx_param(self->io_handle, packed_cmd, data, len);
130130
if (err != ESP_OK) {
131131
qspibus_reset_transfer_state(self);
132-
mp_raise_OSError_msg_varg(MP_ERROR_TEXT("QSPI send failed: %d"), err);
132+
mp_raise_OSError_msg_varg(MP_ERROR_TEXT("%q failure: %d"), MP_QSTR_QSPI, (int)err);
133133
}
134134
}
135135

@@ -148,7 +148,7 @@ static void qspibus_send_color_bytes(
148148
return;
149149
}
150150
if (data == NULL || self->dma_buffer_size == 0) {
151-
mp_raise_OSError_msg(MP_ERROR_TEXT("QSPI DMA buffers unavailable"));
151+
mp_raise_OSError_msg(MP_ERROR_TEXT("Could not allocate DMA capable buffer"));
152152
}
153153

154154
// RAMWR must transition to RAMWRC for continued payload chunks.
@@ -157,10 +157,13 @@ static void qspibus_send_color_bytes(
157157
size_t remaining = len;
158158

159159
while (remaining > 0) {
160+
// inflight_transfers is only modified in task context (never from ISR),
161+
// so no atomic/critical section is needed. The ISR only signals the
162+
// counting semaphore; all counter bookkeeping happens task-side.
160163
if (self->inflight_transfers >= QSPI_DMA_BUFFER_COUNT) {
161164
if (!qspibus_wait_one_transfer_done(self, pdMS_TO_TICKS(QSPI_COLOR_TIMEOUT_MS))) {
162165
qspibus_reset_transfer_state(self);
163-
mp_raise_OSError_msg(MP_ERROR_TEXT("QSPI color timeout"));
166+
mp_raise_OSError_msg(MP_ERROR_TEXT("Operation timed out"));
164167
}
165168
}
166169

@@ -176,7 +179,7 @@ static void qspibus_send_color_bytes(
176179
esp_err_t err = esp_lcd_panel_io_tx_color(self->io_handle, packed_cmd, buffer, chunk);
177180
if (err != ESP_OK) {
178181
qspibus_reset_transfer_state(self);
179-
mp_raise_OSError_msg_varg(MP_ERROR_TEXT("QSPI send color failed: %d"), err);
182+
mp_raise_OSError_msg_varg(MP_ERROR_TEXT("%q failure: %d"), MP_QSTR_QSPI, (int)err);
180183
}
181184

182185
self->inflight_transfers++;
@@ -195,7 +198,7 @@ static void qspibus_send_color_bytes(
195198
// after queued DMA chunks have completed.
196199
if (!qspibus_wait_all_transfers_done(self, pdMS_TO_TICKS(QSPI_COLOR_TIMEOUT_MS))) {
197200
qspibus_reset_transfer_state(self);
198-
mp_raise_OSError_msg(MP_ERROR_TEXT("QSPI color timeout"));
201+
mp_raise_OSError_msg(MP_ERROR_TEXT("Operation timed out"));
199202
}
200203
}
201204

@@ -290,7 +293,7 @@ void common_hal_qspibus_qspibus_construct(
290293
if (!qspibus_allocate_dma_buffers(self)) {
291294
vSemaphoreDelete(self->transfer_done_sem);
292295
self->transfer_done_sem = NULL;
293-
mp_raise_msg(&mp_type_MemoryError, MP_ERROR_TEXT("Failed to allocate DMA buffers"));
296+
mp_raise_msg(&mp_type_MemoryError, MP_ERROR_TEXT("Could not allocate DMA capable buffer"));
294297
}
295298

296299
const spi_bus_config_t bus_config = {
@@ -308,7 +311,7 @@ void common_hal_qspibus_qspibus_construct(
308311
qspibus_release_dma_buffers(self);
309312
vSemaphoreDelete(self->transfer_done_sem);
310313
self->transfer_done_sem = NULL;
311-
mp_raise_OSError_msg_varg(MP_ERROR_TEXT("SPI bus init failed: %d"), err);
314+
mp_raise_OSError_msg_varg(MP_ERROR_TEXT("%q failure: %d"), MP_QSTR_SPI, (int)err);
312315
}
313316

314317
const esp_lcd_panel_io_spi_config_t io_config = {
@@ -332,7 +335,7 @@ void common_hal_qspibus_qspibus_construct(
332335
qspibus_release_dma_buffers(self);
333336
vSemaphoreDelete(self->transfer_done_sem);
334337
self->transfer_done_sem = NULL;
335-
mp_raise_OSError_msg_varg(MP_ERROR_TEXT("Panel IO init failed: %d"), err);
338+
mp_raise_OSError_msg_varg(MP_ERROR_TEXT("%q failure: %d"), MP_QSTR_QSPI, (int)err);
336339
}
337340

338341
claim_pin(clock);
@@ -472,7 +475,7 @@ void common_hal_qspibus_qspibus_write_data(
472475
return;
473476
}
474477
if (data == NULL) {
475-
mp_raise_ValueError(MP_ERROR_TEXT("Data buffer is null"));
478+
mp_raise_ValueError(MP_ERROR_TEXT("Buffer too small"));
476479
}
477480
if (!self->has_pending_command) {
478481
mp_raise_ValueError(MP_ERROR_TEXT("No pending command"));

ports/espressif/common-hal/qspibus/QSPIBus.h

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,5 @@
1+
// This file is part of the CircuitPython project: https://circuitpython.org
12
// SPDX-FileCopyrightText: Copyright (c) 2026 Przemyslaw Patrick Socha
2-
//
33
// SPDX-License-Identifier: MIT
44

55
#pragma once
Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,3 +1,3 @@
1+
// This file is part of the CircuitPython project: https://circuitpython.org
12
// SPDX-FileCopyrightText: Copyright (c) 2026 Przemyslaw Patrick Socha
2-
//
33
// SPDX-License-Identifier: MIT
Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,3 +1,3 @@
1+
// This file is part of the CircuitPython project: https://circuitpython.org
12
// SPDX-FileCopyrightText: Copyright (c) 2026 Przemyslaw Patrick Socha
2-
//
33
// SPDX-License-Identifier: MIT

shared-bindings/qspibus/QSPIBus.c

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,5 @@
1+
// This file is part of the CircuitPython project: https://circuitpython.org
12
// SPDX-FileCopyrightText: Copyright (c) 2026 Przemyslaw Patrick Socha
2-
//
33
// SPDX-License-Identifier: MIT
44

55
#include <stdint.h>

shared-bindings/qspibus/QSPIBus.h

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,5 @@
1+
// This file is part of the CircuitPython project: https://circuitpython.org
12
// SPDX-FileCopyrightText: Copyright (c) 2026 Przemyslaw Patrick Socha
2-
//
33
// SPDX-License-Identifier: MIT
44

55
#pragma once

shared-bindings/qspibus/__init__.h

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,5 @@
1+
// This file is part of the CircuitPython project: https://circuitpython.org
12
// SPDX-FileCopyrightText: Copyright (c) 2026 Przemyslaw Patrick Socha
2-
//
33
// SPDX-License-Identifier: MIT
44

55
#pragma once

0 commit comments

Comments
 (0)