> Hi, > > On Wed, May 02, 2012 at 02:02:55PM +0900, jonghwa3.lee@samsung.com wrote: > > On 2012-04-30 18:17, Andi Shyti wrote: > > > Hi, > > > > > >> + mutex_lock(&max77686->iolock); > > >> + ret = i2c_smbus_read_i2c_block_data(i2c, reg, count, buf); > > >> + mutex_unlock(&max77686->iolock); > > > > > > Is it relly necessay to lock whenever you read/write from/to the > > > i2c bus? Considering also that these are exported function, > > > someone else may lock here before, so we can have a double > > > locking on the same mutex. > > > > These exported functions will be used in 77686 area only, so there is no > > overlap locking. > > OK... I think this could be a reason more to not over-use mutexes :) > > When you call i2c_smbus_* functions you are not accessing to any > private data, all the new data is allocated in a new area. The > smbus_xfer function should take care by himself that the global > data are locked correctly. If not, is not up to your driver to do > it. > If, instead, you are taking care about the concurrency in the > bus, this should be somehow managed by the chip itself. > In my opinion you are abusing a bit of mutex_lock/unlock. > > Andi > > P.S. copied and paste your reply at the bottom of my previous > comment. I expect MAX77686-PMIC(Regulator) driver will be using update_reg() heavily. That function requires mutexing such contexts to work correctly. You won't get correct update without a mutex as it will read a register and write to a register not atomically. Without this mutex, updating a register (i.e., update the third bit to 1) can be disasterous with regulators. Cheers! MyungJoo. > > > -- MyungJoo Ham (Ը), PHD System S/W Lab, S/W Platform Team, Software Center Samsung Electronics Cell: +82-10-6714-2858 {.n++%ݶw{.n+{G{ayʇڙ,jfhz_(階ݢj"mG?&~iOzv^m ?I