Hi guys. Protecting the register-accesses with mutexes protects against concurrent writing, just as intended (like I implemented in commit 55c1724). However, when adapting the I2C-dependent sensor drivers I was wondering if we should better lock the whole function context to prevent to process outdated values (like I implemented in commit e3983bf). An example: Suppose a function that reads a register, adapts this value and writes this back to the register. Procedure:

Context 1:
i2c_acquire
value = i2c_read_reg(A)
i2c_release
process(value)

[Context switch to Context 2 here]

Context 2:
i2c_acquire
i2c_write_reg(A)
i2c_release

[Context switch back to Context 1 here]

Context 1:
i2c_acquire
i2c_write_reg(A)
i2c_release

This would result in working with an outdated value.

Is this a realistic example? What is your opinion on that?

@PeterKietzmann My first hunch would be to add a separate mutex for the driver's own data, to protect against those kinds of races. If the processing is small, e.g. increment the value read and write back the result, I guess it would be possible to keep the SPI bus acquired during the whole read-modify-write cycle, but it would still be misuse of the SPI mutex.

Thanks for your advice @gebart. Any other opinions on that so I can complete this PR soon?

I couldn't think of a case yet but could this cascade of mutexes lead to a deadlock? If not I'd prefer a separated mutex too.

After your comments I tend to "just" use spi_acquire and spi_release in this PR, to safe the unique bus access as intended. The rest we should keep in mind. Are you fine with this for the moment?

I couldn't think of a case yet but could this cascade of mutexes lead to a deadlock? If not I'd prefer a separated mutex too.

If you write the threads so that they lock the mutexes in inverse order compared to each other it can lead to a deadlock. Lock the mutexes in the same order in every situation. The rule of thumb here should be something along the lines of: lock the driver data mutex first, then acquire the bus.

@jfischer-phytec-iot I will work on this PR this evening again. Thx for the look up.

@thomaseichinger, I adapted all I2C-based drivers to new "thread safe" model. I decided to leave the mutex locked during a "read-modify-write" cycle when the modifying part is short.

@thomaseichinger did you already have a look at this PR?

Do you really need to include the mutex header? The rest looks good to me.

@thomaseichinger no it is not needed. I removed these includes.

@thomaseichinger I squashed the last "SQUASH ME-labeled"commit and removed the "NEEDS SQUASHING" label. ACK?

Code looks good to me. As I don't have most of the sensors, did you test exhaustively? ;)

I compiled all and spot-tested some. Neither I have all of these sensors but many of them you can find on the phytec board :-)

ACK when Travis passed.

Kicked Travis.

Travis is happy :-)