On 05/18/11 17:02, Guenter Roeck wrote: > On Wed, May 18, 2011 at 11:39:31AM -0400, Jonathan Cameron wrote: >> From: Fabien Marteau <fabien.marteau-d2DlULPkwbNWk0Htik3J/w@xxxxxxxxxxxxxxxx> >> >> Signed-off-by: Jonathan Cameron <jic23-KWPb1pKIrIJaa/9Udqfwiw@xxxxxxxxxxxxxxxx> > > Hi Jonathan, > > nice job. Thanks a lot for the effort. > > [ ... ] > >> +static int as1531_read_raw(struct iio_dev *indio_dev, >> + struct iio_chan_spec const *chan, >> + int *val, >> + int *val2, >> + long m) >> +{ >> + > Extra blank line > >> + int status = 0; >> + int ret_value = 0; >> + struct as1531_state *st = iio_priv(indio_dev); > > Move it to here, maybe ? > >> + if (mutex_lock_interruptible(&st->lock)) >> + return -ERESTARTSYS; >> + >> + status = as1531_message(st->spi, >> + AS1531_START_BIT | chan->address | >> + AS1531_RANGE_0_TO_VREF | AS1531_MODE_COM | >> + AS1531_POWER_NORMAL, >> + &ret_value); >> + mutex_unlock(&st->lock); >> + if (status < 0) >> + goto out; >> + >> + *val = ret_value*2500/4096; >> + >> + return IIO_VAL_INT; >> +out: >> + mutex_unlock(&st->lock); > > Mutex was unlocked above already. Maybe just return status above ? Hmm.. Sometimes working without testing shows just how bad first versions of code really can be ;) Thanks for the other review as well. Have fixed up both. Thanks. Jonathan -- To unsubscribe from this list: send the line "unsubscribe linux-iio" in the body of a message to majordomo@xxxxxxxxxxxxxxx More majordomo info at http://vger.kernel.org/majordomo-info.html