On Thu, 9 Jun 2022 11:32:08 +0300 Claudiu Beznea <claudiu.beznea@xxxxxxxxxxxxx> wrote: > Add a parameter to at91_adc_read_info_raw() to specify if st->lock mutex > need to be acquired. This prepares for the addition of temperature sensor > code which will re-use at91_adc_read_info_raw() function to read 2 voltages > for determining the real temperature. This looks like a potential lock dependency issue. iio_device_claim_direct_mode() takes an internal iio lock, and you then take st->lock. If you are going to invert that locking order in another path you have a deadlock. So rethink this. If you want to reuse the code you'll need to factor it out to a separate function that takes none of the locks then take all locks needed in each call path (in the same order). Jonathan > > Signed-off-by: Claudiu Beznea <claudiu.beznea@xxxxxxxxxxxxx> > --- > drivers/iio/adc/at91-sama5d2_adc.c | 15 ++++++++++----- > 1 file changed, 10 insertions(+), 5 deletions(-) > > diff --git a/drivers/iio/adc/at91-sama5d2_adc.c b/drivers/iio/adc/at91-sama5d2_adc.c > index 1283bcf4e682..8f8fef42de84 100644 > --- a/drivers/iio/adc/at91-sama5d2_adc.c > +++ b/drivers/iio/adc/at91-sama5d2_adc.c > @@ -1583,7 +1583,8 @@ static irqreturn_t at91_adc_interrupt(int irq, void *private) > } > > static int at91_adc_read_info_raw(struct iio_dev *indio_dev, > - struct iio_chan_spec const *chan, int *val) > + struct iio_chan_spec const *chan, int *val, > + bool lock) > { > struct at91_adc_state *st = iio_priv(indio_dev); > int (*fn)(struct at91_adc_state *, int, u16 *) = NULL; > @@ -1602,13 +1603,15 @@ static int at91_adc_read_info_raw(struct iio_dev *indio_dev, > ret = iio_device_claim_direct_mode(indio_dev); > if (ret) > return ret; > - mutex_lock(&st->lock); > + if (lock) > + mutex_lock(&st->lock); > > if (fn) { > ret = fn(st, chan->channel, &tmp_val); > *val = tmp_val; > ret = at91_adc_adjust_val_osr(st, val); > - mutex_unlock(&st->lock); > + if (lock) > + mutex_unlock(&st->lock); > iio_device_release_direct_mode(indio_dev); > > return ret; > @@ -1644,7 +1647,8 @@ static int at91_adc_read_info_raw(struct iio_dev *indio_dev, > /* Needed to ACK the DRDY interruption */ > at91_adc_readl(st, LCDR); > > - mutex_unlock(&st->lock); > + if (lock) > + mutex_unlock(&st->lock); > > iio_device_release_direct_mode(indio_dev); > return ret; > @@ -1658,7 +1662,8 @@ static int at91_adc_read_raw(struct iio_dev *indio_dev, > > switch (mask) { > case IIO_CHAN_INFO_RAW: > - return at91_adc_read_info_raw(indio_dev, chan, val); > + return at91_adc_read_info_raw(indio_dev, chan, val, true); > + > case IIO_CHAN_INFO_SCALE: > *val = st->vref_uv / 1000; > if (chan->differential)