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 AVR I2C master 1ms timeout #17174

Merged
merged 2 commits into from
Jun 21, 2022
Merged

Conversation

dkao
Copy link
Contributor

@dkao dkao commented May 21, 2022

i2c_start() produces a minimum time_slice of 1ms for use as timeout value.
The timer granularity is 1ms, it is entirely possible for timer_count to tick up immediately after the last timer read and falsely trigger timeout with a >= 1 comparison.

Description

Types of Changes

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

Issues Fixed or Closed by This PR

Checklist

  • My code follows the code style of this project: C, Python
  • I have read the PR Checklist document and have made the appropriate changes.
  • 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).

@github-actions github-actions bot added core documentation keyboard keymap via Adds via keymap and/or updates keyboard for via support labels May 21, 2022
i2c_start() produces a minimum time_slice of 1ms for use as timeout
value.
The timer granularity is 1ms, it is entirely possible for timer_count
to tick up immediately after the last timer read and falsely trigger
timeout with a '>= 1' comparison.
@dkao dkao force-pushed the avr_i2c_master_timeout branch from 72b7aa0 to 23aadba Compare May 21, 2022 05:15
@github-actions github-actions bot removed keyboard via Adds via keymap and/or updates keyboard for via support documentation keymap labels May 21, 2022
@drashna drashna requested a review from a team May 21, 2022 16:58
@drashna drashna added the bug label May 21, 2022
@drashna drashna requested a review from a team May 21, 2022 17:00
@Kriechi
Copy link
Contributor

Kriechi commented May 21, 2022

Tested and verified on ProMicro - this fixes all glitches with the Cirque trackpads on I2C that caused sudden position jumps.

Copy link
Member

@KarlK90 KarlK90 left a comment

Choose a reason for hiding this comment

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

LGTM, as part of the correction you could swap all places to the ´timer_elapsed´ convenience function.

platforms/avr/drivers/i2c_master.c Outdated Show resolved Hide resolved
platforms/avr/drivers/i2c_master.c Outdated Show resolved Hide resolved
platforms/avr/drivers/i2c_master.c Outdated Show resolved Hide resolved
platforms/avr/drivers/i2c_master.c Outdated Show resolved Hide resolved
platforms/avr/drivers/i2c_master.c Outdated Show resolved Hide resolved
Copy link
Member

@KarlK90 KarlK90 left a comment

Choose a reason for hiding this comment

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

Thanks for the fix and quick response.

@KarlK90 KarlK90 merged commit 608404f into qmk:develop Jun 21, 2022
0xcharly pushed a commit to Bastardkb/bastardkb-qmk that referenced this pull request Jul 4, 2022
* avr i2c_master: Fix 1ms timeout

i2c_start() produces a minimum time_slice of 1ms for use as timeout
value.
The timer granularity is 1ms, it is entirely possible for timer_count
to tick up immediately after the last timer read and falsely trigger
timeout with a '>= 1' comparison.

* avr/drivers/i2c_master: Use timer_elapsed()
nolanseaton pushed a commit to nolanseaton/qmk_firmware that referenced this pull request Jan 23, 2023
* avr i2c_master: Fix 1ms timeout

i2c_start() produces a minimum time_slice of 1ms for use as timeout
value.
The timer granularity is 1ms, it is entirely possible for timer_count
to tick up immediately after the last timer read and falsely trigger
timeout with a '>= 1' comparison.

* avr/drivers/i2c_master: Use timer_elapsed()
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.

4 participants