Repository navigation
spi: H7 and F7 send bytes back to back, and two H7 transfer faults fixed - #12061
Conversation
On H7 and F7 an SPI transfer sent one byte and waited for its echo before the next, so the bus stood idle about as long as it was busy, and a register access took two transfers. Now the next byte goes out while the one before is coming back, with at most two in flight, and the address and the data go in one transfer. A 6-byte gyro read on a TBS Lucid H7 Wing took 7.9 us and takes 5.4 us. F4 and AT32 have no FIFO and keep one byte at a time: a second byte in flight would be lost to an overrun whenever an interrupt kept the loop away for a byte's time. Two H7 faults on the way, which the old code happened not to reach: spiSetSpeed() left the SPI enabled, so the next transfer's size was ignored and it ran with the size of the one before; and a transfer that timed out left the SPI enabled, so every later transfer on that bus failed too. The SPI is now left disabled after a speed change and disabled again before each transfer is sized, and a timed-out transfer leaves the SPI ready for the next one: disabled with its flags cleared on H7, its FIFOs emptied on F7. The wait for the end of an H7 transfer is bounded like the others.
|
ⓘ Qodo reviews are paused because the subscription is no longer active. Ask your workspace admin to reactivate the subscription to resume reviews. Manage billing |
PR Summary by QodoPipeline H7/F7 SPI bytes and recover from transfer faults
AI Description
Diagram
High-Level Assessment
Files changed (3)
|
Code Review by Qodo
1.
|
|
RAM / Flash usage vs. base commit
See RAM/flash optimization guide for techniques to reduce usage. |
|
Test firmware build ready — commit Download firmware for PR #12061 251 targets built. Find your board's
|
6b15c92 to
7dab589
Compare
7dab589 to
6b15c92
Compare
Only a reset empties the F7 transmit FIFO: left there, the bytes reached the next transfer's device ahead of its command. The device's prescaler is carried over the reset.
|
@sensei-hacker tested on two more boards today, images merged on the current maintenance-10.x: an F7, Flywoo F722 Pro V2 (ICM-42688-P on SPI1, MAX7456 on SPI2, W25Q128 on SPI3: all detected, attitude follows the board, blackbox to flash and back), and a second H7, Holybro Kakute H7 Mini with its BMI270 on SPI4 (the 8-byte FIFO: IMU at rest 1 g with 0.5 of noise, GYRO task 26 -> 18 us). A third board, a Diatone Mamba F722 (ICM-42688-P, DPS310, W25Q128), ran it as part of #12065 with the same outcome. Details in the description under "Tested on more boards". Qodo's finding on the F7 abort path is completed in 2edc811 (a reset when bytes stay queued), and the branch now has maintenance-10.x merged in. Is there anything else you would like tested? |
|
Thanks! I tested: AEDROXH7 (H7, ICM42605 on SPI2): DAKEFPVF722X8 (F7, ICM42605 on SPI1): |
On H7 and F7 an SPI transfer sent a byte, then waited for its echo before sending the next, so the clock stood idle between bytes about as long as it ran. A register access also took two transfers, the address and then the data; on H7 each one is enabled, sized and closed on its own.
What changes
H7 and F7:
busRead(),busReadBuf(),busWrite()andbusWriteBuf()send the address and the data in one transfer.F4 and AT32F43x are unchanged. Their SPI has no FIFO, only a data register. With a second byte in flight, an interrupt keeping the loop away for one byte's time (under 1 µs) would end in an overrun and a lost byte, so they keep sending one byte at a time.
Two H7 fixes the faster transfers needed. Both are in the current code, where they happen not to show:
spiSetSpeed()re-enabled the SPI after changing its prescaler. An H7 SPI takes a transfer size only while disabled, so the first transfer after a speed change ran with the size of the transfer before it.BUS_SPEED_FASTran with a size of 2 and stopped after two of seven bytes, once per boot on each gyro bus.Measured
Setup:
BUS_SPEED_FAST(15 MHz), a MAX7456 on SPI2.Bytes in flight, set at runtime in the bench build, SPI1 gyro read:
Robustness:
spiSetSpeed()fix, every boot lost one transfer on each gyro bus.Size against maintenance-10.x:
Not tested, testing wanted
Two H7 boards and two F7 boards have run this (see Tested on more boards below). Still untested: the F7 abort path, which only runs when a transfer times out.
Wanted:
What to check:
statusshows the gyro, accelerometer, barometer and OSD detected.tasksreports a lower GYRO time than before.A report of which board and what was checked is very welcome, even if everything simply works.
After review
Built on F4, F7, H7, AT32 and SITL; KAKUTEF7 and KAKUTEF7HDV fit their ITCM. On the TBS Lucid H7 Wing (H7) the gyro reads are unchanged after review; the F7 abort path is untested (it only runs when a transfer times out).
Related PRs
Tested on more boards (2026-10-06)
Testing: the branch has maintenance-10.x merged in since 6e524e0, with #12032 (H7 SPI2-4 divisors) and #11943 (flash flush), so an image from this head builds as tested below.
Tested 2026-10-06 on two more boards, both merged on maintenance-10.x f6579d2:
attitude follows the board; blackbox to the flash logs and reads back; GYRO task 12 us average as before (the F7
gain shows with data-ready, gyro: data-ready reads on F4 and F7 #12065).
(46, -4, 510) with 0.5 of noise, gyro 0; GYRO task 26 -> 18 us average.