Skip to content

Commit f749124

Browse files
ppsxppsx
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. - Regenerate locale/circuitpython.pot to remove stale entries. - 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 89c8186 commit f749124

13 files changed

Lines changed: 32 additions & 58 deletions

File tree

‎locale/circuitpython.pot‎

Lines changed: 4 additions & 37 deletions
Original file line numberDiff line numberDiff line change
@@ -99,6 +99,7 @@ msgid "%q contains duplicate pins"
9999
msgstr ""
100100

101101
#: ports/atmel-samd/common-hal/sdioio/SDCard.c
102+
#: ports/espressif/common-hal/qspibus/QSPIBus.c
102103
msgid "%q failure: %d"
103104
msgstr ""
104105

@@ -710,6 +711,7 @@ msgid "Buffer too short by %d bytes"
710711
msgstr ""
711712

712713
#: ports/cxd56/common-hal/camera/Camera.c
714+
#: ports/espressif/common-hal/qspibus/QSPIBus.c
713715
#: shared-bindings/busdisplay/BusDisplay.c
714716
#: shared-bindings/framebufferio/FramebufferDisplay.c
715717
#: shared-bindings/struct/__init__.c shared-module/struct/__init__.c
@@ -872,7 +874,7 @@ msgstr ""
872874
msgid "Coordinate arrays types have different sizes"
873875
msgstr ""
874876

875-
#: shared-module/usb/core/Device.c
877+
#: ports/espressif/common-hal/qspibus/QSPIBus.c shared-module/usb/core/Device.c
876878
msgid "Could not allocate DMA capable buffer"
877879
msgstr ""
878880

@@ -1029,10 +1031,6 @@ msgstr ""
10291031
msgid "Failed to allocate %q buffer"
10301032
msgstr ""
10311033

1032-
#: ports/espressif/common-hal/qspibus/QSPIBus.c
1033-
msgid "Failed to allocate DMA buffers"
1034-
msgstr ""
1035-
10361034
#: ports/espressif/common-hal/wifi/__init__.c
10371035
msgid "Failed to allocate Wifi memory"
10381036
msgstr ""
@@ -1770,6 +1768,7 @@ msgid "Operation or feature not supported"
17701768
msgstr ""
17711769

17721770
#: ports/espressif/common-hal/espidf/__init__.c
1771+
#: ports/espressif/common-hal/qspibus/QSPIBus.c
17731772
msgid "Operation timed out"
17741773
msgstr ""
17751774

@@ -1807,11 +1806,6 @@ msgstr ""
18071806
msgid "Packet buffers for an SPI transfer must have the same length."
18081807
msgstr ""
18091808

1810-
#: ports/espressif/common-hal/qspibus/QSPIBus.c
1811-
#, c-format
1812-
msgid "Panel IO init failed: %d"
1813-
msgstr ""
1814-
18151809
#: shared-module/jpegio/JpegDecoder.c
18161810
msgid "Parameter error"
18171811
msgstr ""
@@ -1918,28 +1912,6 @@ msgstr ""
19181912
msgid "Pull not used when direction is output."
19191913
msgstr ""
19201914

1921-
#: ports/espressif/common-hal/qspibus/QSPIBus.c
1922-
msgid "QSPI DMA buffers unavailable"
1923-
msgstr ""
1924-
1925-
#: ports/espressif/common-hal/qspibus/QSPIBus.c
1926-
msgid "QSPI color timeout"
1927-
msgstr ""
1928-
1929-
#: ports/espressif/common-hal/qspibus/QSPIBus.c
1930-
msgid "QSPI command timeout"
1931-
msgstr ""
1932-
1933-
#: ports/espressif/common-hal/qspibus/QSPIBus.c
1934-
#, c-format
1935-
msgid "QSPI send color failed: %d"
1936-
msgstr ""
1937-
1938-
#: ports/espressif/common-hal/qspibus/QSPIBus.c
1939-
#, c-format
1940-
msgid "QSPI send failed: %d"
1941-
msgstr ""
1942-
19431915
#: ports/raspberrypi/common-hal/countio/Counter.c
19441916
msgid "RISE_AND_FALL not available on this chip"
19451917
msgstr ""
@@ -2061,11 +2033,6 @@ msgstr ""
20612033
msgid "SDIO Init Error %x"
20622034
msgstr ""
20632035

2064-
#: ports/espressif/common-hal/qspibus/QSPIBus.c
2065-
#, c-format
2066-
msgid "SPI bus init failed: %d"
2067-
msgstr ""
2068-
20692036
#: ports/espressif/common-hal/busio/SPI.c
20702037
msgid "SPI configuration failed"
20712038
msgstr ""

‎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

‎ports/espressif/common-hal/qspibus/__init__.c‎

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

‎ports/espressif/common-hal/qspibus/__init__.h‎

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

0 commit comments

Comments
 (0)