-
Notifications
You must be signed in to change notification settings - Fork 7.4k
drivers/spi: Properly check for rx/tx and buffering on #5785
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
Conversation
Codecov Report
@@ Coverage Diff @@
## master #5785 +/- ##
==========================================
- Coverage 54.37% 54.32% -0.06%
==========================================
Files 457 457
Lines 43298 43298
Branches 8302 8302
==========================================
- Hits 23545 23521 -24
- Misses 19612 19638 +26
+ Partials 141 139 -2
Continue to review full report at Codecov.
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM
static ALWAYS_INLINE | ||
bool spi_context_rx_buf_on(struct spi_context *ctx) | ||
{ | ||
return !!(ctx->rx_buf && ctx->rx_len); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Hi @tbursztyka this might break the stm32 driver. Inside the shift-next-byte loop, the stm32 does the following:
data = read byte
if spi_context_rx_on(...) {
*ctx.rx_buf = data
spi_context_update_rx(...)
}
With this change, the above code will fail when rx_buf.buf = NULL
and rx_buf.len > 0
.
WDYT about documenting the spi_context API then we can go through and fix the current users?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I have already pushed a commit to fix it in PR #5413.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Ack
Current buffers might be configured to skip data, thus only len will be set, buf will be NULL. Buffer should be used if only len is > 0 and buffer is valid as well. tx/rx are "on" if len is > 0 tx/rx buf should be touched if only len is > 0 _and_ buf != NULL. Signed-off-by: Tomasz Bursztyka <[email protected]>
ab30dc0
to
f0ee1d3
Compare
Current buffers might be configured to skip data, thus only len will be
set, buf will be NULL. Buffer should be used if only len is > 0 and
buffer is valid as well.
tx/rx are "on" if len is > 0
tx/rx buf should be touched if only len is > 0 and buf != NULL.
Signed-off-by: Tomasz Bursztyka [email protected]