On Tue, Jul 28, 2020 at 1:40 PM Stephen Boyd <swboyd@xxxxxxxxxxxx> wrote: > > Quoting Daniel Campello (2020-07-28 08:12:53) > > @@ -368,13 +368,13 @@ static int sx9310_wait_for_sample(struct sx9310_data *data) > > static int sx9310_read_proximity(struct sx9310_data *data, > > const struct iio_chan_spec *chan, int *val) > > { > > - int ret = 0; > > + int ret; > > __be16 rawval; > > > > mutex_lock(&data->mutex); > > > > ret = sx9310_get_read_channel(data, chan->channel); > > - if (ret < 0) > > + if (ret) > > goto out; > > > > if (data->client->irq) { > > @@ -394,11 +394,11 @@ static int sx9310_read_proximity(struct sx9310_data *data, > > > > mutex_lock(&data->mutex); > > > > - if (ret < 0) > > + if (ret) > > goto out_disable_irq; > > Why is this condition checked after grabbing the mutex? Shouldn't it be > checked before grabbing the mutex? Or is that supposed to be a > mutex_unlock()? We acquire the lock before jumping to out_disable_irq which is before a mutex_unlock() > > > > > ret = sx9310_read_prox_data(data, chan, &rawval); > > - if (ret < 0) > > + if (ret) > > goto out_disable_irq; > > > > *val = sign_extend32(be16_to_cpu(rawval),