diff mbox series

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

Message ID 20210413112105.69458-6-nuno.sa@analog.com (mailing list archive)
State New
Headers show
Series Adis IRQ fixes and minor improvements | expand

Commit Message

Sa, Nuno April 13, 2021, 11:21 a.m. UTC
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(-)

Comments

Alexandru Ardelean April 14, 2021, 7:34 a.m. UTC | #1
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);
    }


>                 }
>         }
>
> --
> 2.31.1
>
Sa, Nuno April 15, 2021, 7:55 a.m. UTC | #2
> -----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!
diff mbox series

Patch

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;
+			}
 		}
 	}