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 error handling in SPI master driver. #11122

Merged
merged 1 commit into from
Dec 6, 2020
Merged

Conversation

dpapavas
Copy link
Contributor

@dpapavas dpapavas commented Dec 4, 2020

Description

This PR fixes two issues with the AVR SPI master driver, specifically in the error handling in spi_receive. As it is, errors are only detected in the last byte and, more importantly, received zero bytes are ignored. The fix follows the simplest path of returning immediately in case of error, operating under the assumption that partial reads would be useless anyway.

I have currently no easy way to test this as my keyboard doesn't use the spi_master driver, or any of the QMK functionality that currently depends on it, so I have only verified that it compiles properly and that it looks like a reasonable fix. Sorry for that.

Let me know if you like me to make any changes.

Types of Changes

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

@github-actions github-actions bot added the core label Dec 4, 2020
@tzarc tzarc requested review from fauxpark and a team December 5, 2020 04:57
@fauxpark
Copy link
Member

fauxpark commented Dec 5, 2020

This logic was copied from i2c_master; I don't know if that needs fixing as well.

@dpapavas
Copy link
Contributor Author

dpapavas commented Dec 5, 2020

Not as far as I can see. The relevant lines are here and it seems to be ok on both counts (status >= 0 is used and the loop is terminated early on error). The code seems to have changed a lot, so it was probably fixed at some point.

I did notice though that spi_transmit shares one of the problems (that of catching errors at the last byte only). I'll force push an amended commit for that as well and edit the PR title accordingly.

@dpapavas dpapavas changed the title Fix error handling in SPI master receive. Fix error handling in SPI master driver. Dec 5, 2020
@fauxpark fauxpark requested a review from a team December 5, 2020 13:42
@tzarc tzarc merged commit 5cf70f3 into qmk:master Dec 6, 2020
@dpapavas dpapavas deleted the spi_master_fix branch January 4, 2021 10:23
xgnxs pushed a commit to xgnxs/qmk_firmware that referenced this pull request Jan 9, 2021
drashna pushed a commit to zsa/qmk_firmware that referenced this pull request Jan 13, 2021
mgmanzella pushed a commit to mgmanzella/qmk_firmware that referenced this pull request Feb 16, 2021
BorisTestov pushed a commit to BorisTestov/qmk_firmware that referenced this pull request May 23, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants