drivers: add i2c driver for nrf52#7557
Conversation
lebrush
left a comment
There was a problem hiding this comment.
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.
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.
This will always return -2 for speeds LOW, FAST_PLUS and HIGH.
dylad
left a comment
There was a problem hiding this comment.
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.
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.
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.
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.