[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index][Old Index]
kern/47179: major issues with i2c locking (iic_acquire_bus/iic_release_bus)
>Synopsis: major issues with i2c locking (iic_acquire_bus/iic_release_bus)
>Arrival-Date: Sat Nov 10 22:45:00 +0000 2012
>Originator: Matthew Orgass
examination of source code
Between the controller drivers and device drivers many types of errors in
iic_acquire_bus and iic_release_bus usage are represented and few combinations
are correct. I2C_F_POLL might always be handled incorrectly.
The return value of iic_acquire_bus is not documented in iic(9).
The actual return values are inconsistent and sometimes contradictory between
Some controller drivers seem to do something related to preparing the bus to be
accessed in iic_acquire_bus but do not actually lock anything.
Many controller drivers always use mutex_lock in iic_acquire_bus without
checking for I2C_F_POLL.
Some controller drivers always return what they think means failure if
I2C_F_POLL is specified to iic_acquire_bus.
Many device drivers do not check the return value of iic_acquire_bus even if
they specified I2C_F_POLL. Sometimes there is a layer of indirection that
returns the return value, which is then usually ignored by callers.
Some drivers have indirected read/write functions which lock the bus, read or
write, and then release the bus. These functions are usually called many times
in series, rather than locking the bus once and then reading and writing as
needed and releasing when done.
The return value of iic_release_bus is documented as int but it looks like
every controller driver returns void because that makes sense.
examine source code
I'll eventually submit a patch, but it is taking a while so I thought I would
send-pr the problem so it isn't lost. If someone wants to get to it faster, go
Basically, all usage of iic_acquire_bus and iic_release_bus in both controller
and device drivers needs to be checked and fixed and the documentation updated.
It seems like the return value of iic_acquire_bus should maybe be the return
value of mutex_tryenter (although this is contrary to what seems to be the most
common assumption now) and that it should be documented to always succeed if
I2C_F_POLL is not specified. An iic generic macro or function should probably
be provided to DTRT in controller iic_acquire_bus functions. Possibly, the
mutex could just be put in struct i2c_controller and handled in a completely
controller independent way, although that would be the only regularly modified
data in that structure and a few controllers seem to want hooks at those points
Main Index |
Thread Index |