RE: [PATCH 5/7] iio: adis_buffer: check return value on page change

Sa, Nuno
2021-04-15T07:56:01Z

> -----Original Message-----
> From: Alexandru Ardelean <ardeleanalex@gmail.com>
> Sent: Wednesday, April 14, 2021 9:34 AM
> To: Sa, Nuno <Nuno.Sa@analog.com>
> Cc: linux-iio <linux-iio@vger.kernel.org>; Jonathan Cameron
> <jic23@kernel.org>; Hennerich, Michael
> <Michael.Hennerich@analog.com>; Lars-Peter Clausen
> <lars@metafoo.de>
> Subject: Re: [PATCH 5/7] iio: adis_buffer: check return value on page
> change
>
> [External]
>
> On Tue, Apr 13, 2021 at 5:45 PM Nuno Sa <nuno.sa@analog.com>
> wrote:
> >
> > On the trigger handler, we might need to change the device page.
> Hence,
> > we should check the return value from 'spi_write()' and act
> accordingly.
> >
> > Signed-off-by: Nuno Sa <nuno.sa@analog.com>
> > ---
> > drivers/iio/imu/adis_buffer.c | 6 +++++-
> > 1 file changed, 5 insertions(+), 1 deletion(-)
> >
> > diff --git a/drivers/iio/imu/adis_buffer.c
> b/drivers/iio/imu/adis_buffer.c
> > index f89bce10090a..7ab15c08889f 100644
> > --- a/drivers/iio/imu/adis_buffer.c
> > +++ b/drivers/iio/imu/adis_buffer.c
> > @@ -137,7 +137,11 @@ static irqreturn_t adis_trigger_handler(int
> irq, void *p)
> > if (adis->current_page != 0) {
> > adis->tx[0] = ADIS_WRITE_REG(ADIS_REG_PAGE_ID);
> > adis->tx[1] = 0;
> > - spi_write(adis->spi, adis->tx, 2);
> > + ret = spi_write(adis->spi, adis->tx, 2);
> > + if (ret) {
> > + dev_err(&adis->spi->dev, "Failed to change device
> page: %d\n", ret);
> > + goto irq_done;
> > + }
>
> The &adis->state_lock is not being unlocked on this error path.
> This probably needs a bit of re-organization of this block:
> if (adis->data->has_paging) {
> adis->current_page = 0;
> mutex_unlock(&adis->state_lock);
> }
>

Arghhh, good catch. Not sure where I had my head to not see this...

Thanks!