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

Split - Avoid race condition during matrix_init_quantum #8235

Merged
merged 2 commits into from
Feb 25, 2020

Conversation

zvecr
Copy link
Member

@zvecr zvecr commented Feb 23, 2020

Description

Made much worse by the SPLIT_USB_DETECT mode, there is a window where the slave side has is interrupt handler registered before its init process has finished. Given this interrupt can jump in at any point, it breaks some single threaded assumptions within the codebase.

Order of events:

  1. master powers on - begin init
  2. slave powers on - begin init
  3. master detects USB quickly - finish init
    • as part of init finish, RGB is set to eeprom values
  4. master runs post_init which triggers RGB sync
  5. slave receives sync and sets RGB
  6. slave times out on USB detect - finishes init
    • as part of init finish, RGB is set to eeprom values

What you expect is that both halves share the same RGB config, however what actually happens is the slave side has the eeprom values set till the next animation tick (30 seconds).

While difficult, i have managed to reproduce this without the SPLIT_USB_DETECT mode.

With this change, interrupt handlers are registered after init. Any rgb syncs will then fail, and be retried till the slave starts to respond (after its init process has completed).

Types of Changes

  • Core
  • Bugfix
  • New feature
  • Enhancement/optimization
  • Keyboard (addition or update)
  • Keymap/layout/userspace (addition or update)
  • Documentation

Checklist

  • My code follows the code style of this project.
  • My change requires a change to the documentation.
  • I have updated the documentation accordingly.
  • I have read the CONTRIBUTING document.
  • I have added tests to cover my changes.
  • I have tested the changes and verified that they work and don't break anything (as well as I can manage).

Sorry, something went wrong.

@fauxpark fauxpark requested a review from a team February 24, 2020 09:43
@zvecr zvecr marked this pull request as ready for review February 24, 2020 14:12
Copy link
Member

@nooges nooges left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks good other than typo

quantum/split_common/split_util.c Outdated Show resolved Hide resolved
@zvecr zvecr merged commit f6111d4 into qmk:master Feb 25, 2020
nesth pushed a commit to nesth/qmk_firmware that referenced this pull request Feb 27, 2020
* upstream/master: (26 commits)
  Fixed OS detection such that OSX doesn't take over the world (qmk#8248)
  [Keyboard] Add Prime_EXL Plus to handwired (qmk#8238)
  format code according to conventions [skip ci]
  New feature: PERMISSIVE_HOLD_PER_KEY (qmk#7994)
  Split - Avoid race condition during matrix_init_quantum (qmk#8235)
  Acheron VIA support (qmk#8204)
  `send_unicode_string()`: Add support for code points > 0xFFFF (qmk#8236)
  [Keyboard] Add Wete (qmk#8229)
  Improvements to extrakey HID descriptors (qmk#8156)
  Hineybush h87a lock indicators (qmk#8237)
  Add VIA support for Prime_L (qmk#8233)
  Hub16 - Bug removal + clean up code (qmk#8227)
  [Keyboard] ai03 Equinox (qmk#8224)
  [Keyboard] Add zfrontier/big_switch (qmk#8205)
  Gingham Update (qmk#8225)
  A proper `send_string()` for the Unicode feature (qmk#8155)
  Rollback PR qmk#7967 in preference of fixing I2C start/stop properly, in a followup PR. (qmk#8173)
  Add mouse support to SEND_STRING (qmk#8223)
  Add link to "Useful functions" in macro docs (qmk#7446)
  New functionality for cformat (qmk#7893)
  ...
HokieGeek pushed a commit to HokieGeek/qmk_firmware that referenced this pull request Mar 5, 2020
* Avoid race condition during matrix_init_quantum

* spelling is hard
c0psrul3 pushed a commit to c0psrul3/qmk_firmware that referenced this pull request Mar 23, 2020
* Avoid race condition during matrix_init_quantum

* spelling is hard
drashna pushed a commit to zsa/qmk_firmware that referenced this pull request Mar 26, 2020

Verified

This commit was signed with the committer’s verified signature.
drashna Drashna Jaelre
* Avoid race condition during matrix_init_quantum

* spelling is hard
sowbug pushed a commit to sowbug/qmk_firmware that referenced this pull request Apr 2, 2020

Unverified

This commit is not signed, but one or more authors requires that any commit attributed to them is signed.
* Avoid race condition during matrix_init_quantum

* spelling is hard
kylekuj pushed a commit to kylekuj/qmk_firmware that referenced this pull request Apr 21, 2020
* Avoid race condition during matrix_init_quantum

* spelling is hard
@zvecr zvecr deleted the feature/usb_detect_race branch April 28, 2020 01:14
jakeisnt pushed a commit to jakeisnt/qmk_firmware that referenced this pull request Aug 20, 2020
* Avoid race condition during matrix_init_quantum

* spelling is hard
@gsadams gsadams mentioned this pull request Feb 27, 2021
BorisTestov pushed a commit to BorisTestov/qmk_firmware that referenced this pull request May 23, 2024
* Avoid race condition during matrix_init_quantum

* spelling is hard
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants