Conversation
pmw3360_init() opened the SPI bus and then called pmw3360_reg_write() and pmw3360_reg_read(), each of which opens the bus again. That nesting was harmless until QMK 0.26.0, where qmk/qmk_firmware#23439 gave the ChibiOS spi_start()/spi_stop() bus mutex semantics: bool spi_start(...) { spiAcquireBus(&SPI_DRIVER); // acquired on every call if (spiStarted) { return false; // returns without releasing } Only spi_stop() releases the bus, so the inner call is a second spiAcquireBus() from the same thread. ChibiOS mutexes are not recursive, so it blocks forever. SPI_USE_MUTUAL_EXCLUSION defaults to TRUE and Keyball does not override it in a halconf.h. The deadlock happens during pointing device init, before the main loop runs, so the board enumerates over USB (interrupt driven) but nothing else works: no key scanning, no trackball, and the OLED renders one frame and then never updates again. It presents as a dead keyboard rather than a dead sensor, which makes it easy to misattribute to the matrix. The outer pair was redundant -- every reg_read()/reg_write() in the block already brackets itself -- so removing it is sufficient. pmw3360_motion_burst() and pmw3360_srom_upload() call these sequentially rather than nested and were already correct. Only ChibiOS-based (ARM) controllers, such as RP2040 boards, on QMK >= 0.26 are affected. AVR (Pro Micro) builds are not: QMK's AVR spi_start() has no bus mutex, so the nested call was just a no-op. On AVR, and on ChibiOS before 0.26, the first inner spi_stop() already closed the bus, so the outer pair had no effect beyond the first register write, and removing it leaves the bytes and chip-select transitions sent to the sensor unchanged. Tested on hardware, Keyball39 with Elite-Pi (RP2040) controllers, test keymap: QMK 0.22.14 works with this change (no regression) QMK 0.26.11 hangs without this change; works with it QMK 0.28.0 hangs without this change; works with it, including split with either half as master This is likely one reason Keyball has stayed pinned to QMK 0.22.14, but not the only one: RGBLIGHT builds on QMK >= 0.24 also fail to compile, because RGBLED_NUM was renamed to RGBLIGHT_LED_COUNT.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Hey there,
I just recently finished building a Keyball39 using Elite-Pi microcontrollers.
The docs here indicate that the firmware is known to work with QMK
0.22.14.While upgrading QMK versions, I found that compiling against QMK >=
0.26introduced an issue where the OLED freezes on its first frame, and the keys and trackball do nothing.The cause is in
pmw3360_init():pmw3360_spi_start()(which callsspi_start()) explicitlypmw3360_reg_write()(which also callspmw3360_spi_start(), therefore also callingspi_start())qmk/qmk_firmware#23439 added a mutex to
spi_start()on ARM controllers like the RP2040 (QMK runs these on ChibiOS), and it isn't recursive, so since that changepmw3360_init()deadlocks on the second call.The outer
spi_start()/spi_stop()pair wasn't doing anything, since every register read and write already opens and closes the bus on its own. So this PR just deletes those two lines.Tested on a Keyball39 with Elite-Pi controllers:
0.22.14: works with and without the patch (i.e. backwards compatible)0.26.11: hangs without this patch, works with it0.28.0: hangs without this patch, works with it, including split with either half as masterAVR (Pro Micro) builds aren't affected. QMK's AVR
spi_start()has no mutex, so the nested call was already a no-op there. I haven't tested it on AVR hardware, though.