Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

fix matrix_io_delay() timing in quantum/matrix.c #9603

Merged
merged 17 commits into from
Jan 13, 2021
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
14 changes: 10 additions & 4 deletions quantum/matrix.c
Original file line number Diff line number Diff line change
Expand Up @@ -107,9 +107,9 @@ static bool read_cols_on_row(matrix_row_t current_matrix[], uint8_t current_row)
// Start with a clear matrix row
matrix_row_t current_row_value = 0;

// Select row and wait for row selecton to stabilize
// Select row
select_row(current_row);
matrix_io_delay();
matrix_output_select_delay();

// For each col...
for (uint8_t col_index = 0; col_index < MATRIX_COLS; col_index++) {
Expand All @@ -122,6 +122,9 @@ static bool read_cols_on_row(matrix_row_t current_matrix[], uint8_t current_row)

// Unselect row
unselect_row(current_row);
if (current_row + 1 < MATRIX_ROWS) {
matrix_output_unselect_delay(); // wait for row signal to go HIGH
}

// If the row has changed, store the row and return the changed flag.
if (current_matrix[current_row] != current_row_value) {
Expand Down Expand Up @@ -157,9 +160,9 @@ static void init_pins(void) {
static bool read_rows_on_col(matrix_row_t current_matrix[], uint8_t current_col) {
bool matrix_changed = false;

// Select col and wait for col selecton to stabilize
// Select col
select_col(current_col);
matrix_io_delay();
matrix_output_select_delay();

// For each row...
for (uint8_t row_index = 0; row_index < MATRIX_ROWS; row_index++) {
Expand All @@ -185,6 +188,9 @@ static bool read_rows_on_col(matrix_row_t current_matrix[], uint8_t current_col)

// Unselect col
unselect_col(current_col);
if (current_col + 1 < MATRIX_COLS) {
matrix_output_unselect_delay(); // wait for col signal to go HIGH
}

return matrix_changed;
}
Expand Down
5 changes: 5 additions & 0 deletions quantum/matrix_common.c
Original file line number Diff line number Diff line change
@@ -1,3 +1,4 @@
#include "quantum.h"
#include "matrix.h"
#include "debounce.h"
#include "wait.h"
Expand Down Expand Up @@ -83,8 +84,12 @@ uint8_t matrix_key_count(void) {
return count;
}

/* `matrix_io_delay ()` exists for backwards compatibility. From now on, use matrix_output_unselect_delay(). */
__attribute__((weak)) void matrix_io_delay(void) { wait_us(MATRIX_IO_DELAY); }

__attribute__((weak)) void matrix_output_select_delay(void) { waitInputPinDelay(); }
__attribute__((weak)) void matrix_output_unselect_delay(void) { matrix_io_delay(); }

// CUSTOM MATRIX 'LITE'
__attribute__((weak)) void matrix_init_custom(void) {}

Expand Down
29 changes: 29 additions & 0 deletions quantum/quantum.h
Original file line number Diff line number Diff line change
Expand Up @@ -208,6 +208,13 @@ typedef uint8_t pin_t;

# define togglePin(pin) (PORTx_ADDRESS(pin) ^= _BV((pin)&0xF))

/* The AVR series GPIOs have a one clock read delay for changes in the digital input signal.
* But here's more margin to make it two clocks. */
# if !defined(GPIO_INPUT_PIN_DELAY)
# define GPIO_INPUT_PIN_DELAY 2
# endif
# define waitInputPinDelay() wait_cpuclock(GPIO_INPUT_PIN_DELAY)

#elif defined(PROTOCOL_CHIBIOS)
typedef ioline_t pin_t;

Expand All @@ -223,6 +230,28 @@ typedef ioline_t pin_t;
# define readPin(pin) palReadLine(pin)

# define togglePin(pin) palToggleLine(pin)

#endif

#if defined(__ARMEL__) || defined(__ARMEB__)
/* For GPIOs on ARM-based MCUs, the input pins are sampled by the clock of the bus
* to which the GPIO is connected.
* The connected buses differ depending on the various series of MCUs.
* And since the instruction execution clock of the CPU and the bus clock of GPIO are different,
* there is a delay of several clocks to read the change of the input signal.
*
* Define this delay with the GPIO_INPUT_PIN_DELAY macro.
* If the GPIO_INPUT_PIN_DELAY macro is not defined, the following default values will be used.
* (A fairly large value of 0.25 microseconds is set.)
*/
# if !defined(GPIO_INPUT_PIN_DELAY)
# if defined(STM32_SYSCLK)
# define GPIO_INPUT_PIN_DELAY (STM32_SYSCLK/1000000L / 4)
# elif defined(KINETIS_SYSCLK_FREQUENCY)
# define GPIO_INPUT_PIN_DELAY (KINETIS_SYSCLK_FREQUENCY/1000000L / 4)
# endif
# endif
# define waitInputPinDelay() wait_cpuclock(GPIO_INPUT_PIN_DELAY)
#endif

// Atomic macro to help make GPIO and other controls atomic.
Expand Down
14 changes: 10 additions & 4 deletions quantum/split_common/matrix.c
Original file line number Diff line number Diff line change
Expand Up @@ -120,9 +120,9 @@ static bool read_cols_on_row(matrix_row_t current_matrix[], uint8_t current_row)
// Start with a clear matrix row
matrix_row_t current_row_value = 0;

// Select row and wait for row selecton to stabilize
// Select row
select_row(current_row);
matrix_io_delay();
matrix_output_select_delay();

// For each col...
for (uint8_t col_index = 0; col_index < MATRIX_COLS; col_index++) {
Expand All @@ -135,6 +135,9 @@ static bool read_cols_on_row(matrix_row_t current_matrix[], uint8_t current_row)

// Unselect row
unselect_row(current_row);
if (current_row + 1 < MATRIX_ROWS) {
matrix_output_unselect_delay(); // wait for row signal to go HIGH
}

// If the row has changed, store the row and return the changed flag.
if (current_matrix[current_row] != current_row_value) {
Expand Down Expand Up @@ -170,9 +173,9 @@ static void init_pins(void) {
static bool read_rows_on_col(matrix_row_t current_matrix[], uint8_t current_col) {
bool matrix_changed = false;

// Select col and wait for col selecton to stabilize
// Select col
select_col(current_col);
matrix_io_delay();
matrix_output_select_delay();

// For each row...
for (uint8_t row_index = 0; row_index < ROWS_PER_HAND; row_index++) {
Expand All @@ -198,6 +201,9 @@ static bool read_rows_on_col(matrix_row_t current_matrix[], uint8_t current_col)

// Unselect col
unselect_col(current_col);
if (current_col + 1 < MATRIX_COLS) {
matrix_output_unselect_delay(); // wait for col signal to go HIGH
}

return matrix_changed;
}
Expand Down
3 changes: 3 additions & 0 deletions tmk_core/common/matrix.h
Original file line number Diff line number Diff line change
Expand Up @@ -57,6 +57,9 @@ matrix_row_t matrix_get_row(uint8_t row);
/* print matrix for debug */
void matrix_print(void);
/* delay between changing matrix pin state and reading values */
void matrix_output_select_delay(void);
void matrix_output_unselect_delay(void);
/* only for backwards compatibility. delay between changing matrix pin state and reading values */
void matrix_io_delay(void);

/* power control */
Expand Down
52 changes: 52 additions & 0 deletions tmk_core/common/wait.h
Original file line number Diff line number Diff line change
Expand Up @@ -7,10 +7,62 @@
extern "C" {
#endif

#if defined(__ARMEL__) || defined(__ARMEB__)
# ifndef __OPTIMIZE__
# pragma message "Compiler optimizations disabled; wait_cpuclock() won't work as designed"
# endif

# define wait_cpuclock(x) wait_cpuclock_allnop(x)

# define CLOCK_DELAY_NOP8 "nop\n\t nop\n\t nop\n\t nop\n\t nop\n\t nop\n\t nop\n\t nop\n\t"

__attribute__((always_inline))
static inline void wait_cpuclock_allnop(unsigned int n) { /* n: 1..135 */
/* The argument n must be a constant expression.
* That way, compiler optimization will remove unnecessary code. */
if (n < 1) { return; }
if (n > 8) {
unsigned int n8 = n/8;
n = n - n8*8;
switch (n8) {
case 16: asm volatile (CLOCK_DELAY_NOP8::: "memory");
case 15: asm volatile (CLOCK_DELAY_NOP8::: "memory");
case 14: asm volatile (CLOCK_DELAY_NOP8::: "memory");
case 13: asm volatile (CLOCK_DELAY_NOP8::: "memory");
case 12: asm volatile (CLOCK_DELAY_NOP8::: "memory");
case 11: asm volatile (CLOCK_DELAY_NOP8::: "memory");
case 10: asm volatile (CLOCK_DELAY_NOP8::: "memory");
case 9: asm volatile (CLOCK_DELAY_NOP8::: "memory");
case 8: asm volatile (CLOCK_DELAY_NOP8::: "memory");
case 7: asm volatile (CLOCK_DELAY_NOP8::: "memory");
case 6: asm volatile (CLOCK_DELAY_NOP8::: "memory");
case 5: asm volatile (CLOCK_DELAY_NOP8::: "memory");
case 4: asm volatile (CLOCK_DELAY_NOP8::: "memory");
case 3: asm volatile (CLOCK_DELAY_NOP8::: "memory");
case 2: asm volatile (CLOCK_DELAY_NOP8::: "memory");
case 1: asm volatile (CLOCK_DELAY_NOP8::: "memory");
case 0: break;
}
}
switch (n) {
case 8: asm volatile ("nop"::: "memory");
case 7: asm volatile ("nop"::: "memory");
case 6: asm volatile ("nop"::: "memory");
case 5: asm volatile ("nop"::: "memory");
case 4: asm volatile ("nop"::: "memory");
case 3: asm volatile ("nop"::: "memory");
case 2: asm volatile ("nop"::: "memory");
case 1: asm volatile ("nop"::: "memory");
case 0: break;
}
}
#endif

#if defined(__AVR__)
# include <util/delay.h>
# define wait_ms(ms) _delay_ms(ms)
# define wait_us(us) _delay_us(us)
# define wait_cpuclock(x) __builtin_avr_delay_cycles(x)
#elif defined PROTOCOL_CHIBIOS
# include "ch.h"
# define wait_ms(ms) \
Expand Down