On Wed, Dec 03, 2014 at 06:01:25PM +0200, Mika Westerberg wrote: > On Mon, Dec 01, 2014 at 04:09:32PM -0800, David E. Box wrote: > > Adds support for acquiring and releasing a hardware bus lock in the i2c > > designware core transfer function. This is needed for i2c bus controllers > > that are shared with but not controlled by the kernel. > > > > Signed-off-by: David E. Box <[email protected]> > > --- > > drivers/i2c/busses/i2c-designware-core.c | 11 +++++++++++ > > drivers/i2c/busses/i2c-designware-core.h | 6 ++++++ > > drivers/i2c/busses/i2c-designware-platdrv.c | 18 +++++++++++++----- > > 3 files changed, 30 insertions(+), 5 deletions(-) > > > > diff --git a/drivers/i2c/busses/i2c-designware-core.c > > b/drivers/i2c/busses/i2c-designware-core.c > > index 3c20e4b..377deeb 100644 > > --- a/drivers/i2c/busses/i2c-designware-core.c > > +++ b/drivers/i2c/busses/i2c-designware-core.c > > @@ -631,6 +631,14 @@ i2c_dw_xfer(struct i2c_adapter *adap, struct i2c_msg > > msgs[], int num) > > dev->abort_source = 0; > > dev->rx_outstanding = 0; > > > > + if (dev->acquire_lock) { > > + ret = dev->acquire_lock(dev); > > + if (ret) { > > + dev_err(dev->dev, "couldn't acquire bus ownership\n"); > > + goto done; > > I wonder what happens now since you failed to acquire the lock... > > > + } > > + } > > + > > ret = i2c_dw_wait_bus_not_busy(dev); > > if (ret < 0) > > goto done; > > @@ -676,6 +684,9 @@ i2c_dw_xfer(struct i2c_adapter *adap, struct i2c_msg > > msgs[], int num) > > ret = -EIO; > > > > done: > > + if (dev->release_lock) > > + dev->release_lock(dev); > > ... but here you unconditionally release it? >
Releasing the lock unconditionally isn't a problem, but it is an unnecessary path should we fail to acquire the lock. I'll add another label to skip it. Dave -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to [email protected] More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
