-
Notifications
You must be signed in to change notification settings - Fork 2.1k
drivers: add i2c driver for nrf52 #7557
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
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.
You should remove the board stuff from this PR, maybe add the i2c information to the board PR and get this one merged first.
/** | ||
* @brief Initialized bus locks (we have a maximum of two devices...) | ||
*/ | ||
static mutex_t locks[] = { |
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.
Please use I2C_NUMOF
(if it complicates the code the initialization can be done in the .._init
function) since a board might be interested in using only one.
if (bus >= I2C_NUMOF) { | ||
return -1; | ||
} | ||
if (speed & INVALID_SPEED_MASK) { |
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.
This will always return -2
for speeds LOW
, FAST_PLUS
and HIGH
.
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.
This speeds are not supported.
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.
Hey nice work ! and sorry for coming that late, I'm really new to nrf52 architecture but I would love to have a working I2C driver !
Do you still have time to work on this ? I did a quick review, a few small changes are required.
|
||
static inline NRF_TWIM_Type *dev(i2c_t bus) | ||
{ | ||
return i2c_config[bus].dev; |
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.
Could you define this struct within cpu/nrf52/include
please ?
Moreover, I have a compilation error here :
error: control reaches end of non-void function [-Werror=return-type]
#define ENABLE_DEBUG (0) | ||
#include "debug.h" | ||
|
||
#ifdef I2C_NUMOF |
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.
This headerguard is no longer necessary.
*/ | ||
#define HAVE_I2C_SPEED_T | ||
typedef enum { | ||
I2C_SPEED_LOW = 0x01, /**< not supported */ |
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 think it would be better to define them as 0xFF, 0xFE, ... if those values are not supported rather than 0x01, 0x02, ...
Closing in favor of #8318 |
This PR includes the I2C driver for nrf52.
Based on PR #7501.