From fb75fc709121aaae104a9c20f4e4d4d7a3aa2e8d Mon Sep 17 00:00:00 2001 From: Gergely Nagy Date: Tue, 29 Jan 2019 23:35:20 +0100 Subject: [PATCH] ATMegaKeyboard: Disable loop unrolling for readCols() We want to give the hardware enough time to give us a stable read of the pins. If we unroll the loop, we will not have that. In practice, this leads to entire rows or columns behaving erratically. To fix that, we tell the compiler not to unroll loops in the `readCols()` method (it's free to unroll them elsewhere). This makes the function a tiny bit slower, but gives us a stable read. Signed-off-by: Gergely Nagy --- src/kaleidoscope/hardware/ATMegaKeyboard.cpp | 14 ++++++++++++++ 1 file changed, 14 insertions(+) diff --git a/src/kaleidoscope/hardware/ATMegaKeyboard.cpp b/src/kaleidoscope/hardware/ATMegaKeyboard.cpp index 59f17401..724319e8 100644 --- a/src/kaleidoscope/hardware/ATMegaKeyboard.cpp +++ b/src/kaleidoscope/hardware/ATMegaKeyboard.cpp @@ -129,6 +129,20 @@ bool ATMegaKeyboard::isKeyMasked(byte row, byte col) { return bitRead(KeyboardHardware.masks_[row], col); } +/* + * This function has loop unrolling disabled on purpose: we want to give the + * hardware enough time to produce stable PIN reads for us. If we unroll the + * loop, we will not have that, because even with the NOP, the codepath is too + * fast. If we don't have stable reads, then entire rows or columns will behave + * erratically. + * + * For this reason, we ask the compiler to not unroll our loop, which in turn, + * gives hardware enough time to produce stable reads, at the cost of a little + * bit of speed. + * + * Do not remove the attribute! + */ +__attribute__((optimize("no-unroll-loops"))) uint16_t ATMegaKeyboard::readCols() { uint16_t results = 0x00 ; for (uint8_t i = 0; i < KeyboardHardware.matrix_columns; i++) {