cpu/nrf52/i2c: use static mutex initializtion#8486
cpu/nrf52/i2c: use static mutex initializtion#8486PeterKietzmann merged 1 commit intoRIOT-OS:masterfrom
Conversation
| */ | ||
| static mutex_t locks[] = { | ||
| MUTEX_INIT, | ||
| MUTEX_INIT |
There was a problem hiding this comment.
Why is there two elements in the array ? What if we use only one I2C ? there is no need for the second lock then.
There was a problem hiding this comment.
that is correct. But the way the I2C devices are currently defined, there is no easy way to do on-demand static mutex initialization, so I rather waste 4 byte of memory here than to deal with potentially ugly code overhead. Keep in mind that something like #if I2C_NUMOF > 1 does not work here...
There was a problem hiding this comment.
There was a problem hiding this comment.
I agree with not wasting memory (in general)!
In this particular case though, I think we can live with it: this fix is rather temporary, as the driver is going to be slightly reworked once we finally start to tackle the adaption of #6576. With the changes coming there, we can easily go back to dynamic initialization of the mutex(es), as the init_master function will only be called once during system startup...
There was a problem hiding this comment.
this fix is rather temporary
Good to know.
once we finally start to tackle the adaption of #6576
Can't wait we start this one !
There was a problem hiding this comment.
Can't wait we start this one !
@dylad happy to hear that :-)! You seem quite experienced so it would be great to have you on board for that. Bit off-topic: are you aware of any kind of reference I2C-slave device/implementation?
|
I would ACK this PR, @dylad how about you? |
I would love to help you with this new API and start asap :)
I think I only used such implementation once, it was with an Arduino Uno. Maybe we can have a look there ?
I'm fine with the current change. I did not run any test, but IMHO this PR should go. |
PeterKietzmann
left a comment
There was a problem hiding this comment.
Tested together with #8385. ACK and go
Contribution description
The I2C driver breaks for certain device drivers, e.g. when they call
i2c_acquirebefore callingi2c_init_master(and also there could be concurrency issues down the line if not).Issues/PRs references
fixes parts of #8446