Messages in this thread | | | Date | Wed, 02 May 2012 09:37:00 +0000 (GMT) | From | 함명주 <> | Subject | Re: Re: [PATCH] MFD : add MAX77686 mfd driver |
| |
> 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
| |