| Message ID | 20210413112105.69458-6-nuno.sa@analog.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | Adis IRQ fixes and minor improvements | expand |
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 >
> -----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 --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; + } } }
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(-)