| Message ID | 20210413112105.69458-8-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: > > With commit 41f2770a07e0 ("iio: adis_buffer: don't push data to buffers on > failure"), we return if 'spi_sync()' fails which would leave > 'adis->current_page' in an incoherent state. Hence, set this variable > right after we change the device page. > > Signed-off-by: Nuno Sa <nuno.sa@analog.com> > --- > drivers/iio/imu/adis_buffer.c | 6 +++--- > 1 file changed, 3 insertions(+), 3 deletions(-) > > diff --git a/drivers/iio/imu/adis_buffer.c b/drivers/iio/imu/adis_buffer.c > index 73790b71102b..aa37981c28f1 100644 > --- a/drivers/iio/imu/adis_buffer.c > +++ b/drivers/iio/imu/adis_buffer.c > @@ -142,6 +142,8 @@ static irqreturn_t adis_trigger_handler(int irq, void *p) > dev_err(&adis->spi->dev, "Failed to change device page: %d\n", ret); > goto irq_done; > } > + > + adis->current_page = 0; If the above spi_write() fails, this adis->current_page isn't reset. Maybe reset this as the first thing in this if block? > } > } > > @@ -151,10 +153,8 @@ static irqreturn_t adis_trigger_handler(int irq, void *p) > goto irq_done; > } > > - if (adis->data->has_paging) { > - adis->current_page = 0; > + if (adis->data->has_paging) > mutex_unlock(&adis->state_lock); > - } > > iio_push_to_buffers_with_timestamp(indio_dev, adis->buffer, > pf->timestamp); > -- > 2.31.1 >
> -----Original Message----- > From: Alexandru Ardelean <ardeleanalex@gmail.com> > Sent: Wednesday, April 14, 2021 9:39 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 7/7] iio: adis_buffer: update device page after > changing it > > [External] > > On Tue, Apr 13, 2021 at 5:45 PM Nuno Sa <nuno.sa@analog.com> > wrote: > > > > With commit 41f2770a07e0 ("iio: adis_buffer: don't push data to > buffers on > > failure"), we return if 'spi_sync()' fails which would leave > > 'adis->current_page' in an incoherent state. Hence, set this variable > > right after we change the device page. > > > > Signed-off-by: Nuno Sa <nuno.sa@analog.com> > > --- > > drivers/iio/imu/adis_buffer.c | 6 +++--- > > 1 file changed, 3 insertions(+), 3 deletions(-) > > > > diff --git a/drivers/iio/imu/adis_buffer.c > b/drivers/iio/imu/adis_buffer.c > > index 73790b71102b..aa37981c28f1 100644 > > --- a/drivers/iio/imu/adis_buffer.c > > +++ b/drivers/iio/imu/adis_buffer.c > > @@ -142,6 +142,8 @@ static irqreturn_t adis_trigger_handler(int irq, > void *p) > > dev_err(&adis->spi->dev, "Failed to change device > page: %d\n", ret); > > goto irq_done; > > } > > + > > + adis->current_page = 0; > > If the above spi_write() fails, this adis->current_page isn't reset. > Maybe reset this as the first thing in this if block? > If the 'spi_write()' fails it means that we could not change to page0, so we should not update the current_page...
On Thu, Apr 15, 2021 at 10:58 AM Sa, Nuno <Nuno.Sa@analog.com> wrote: > > > > > -----Original Message----- > > From: Alexandru Ardelean <ardeleanalex@gmail.com> > > Sent: Wednesday, April 14, 2021 9:39 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 7/7] iio: adis_buffer: update device page after > > changing it > > > > [External] > > > > On Tue, Apr 13, 2021 at 5:45 PM Nuno Sa <nuno.sa@analog.com> > > wrote: > > > > > > With commit 41f2770a07e0 ("iio: adis_buffer: don't push data to > > buffers on > > > failure"), we return if 'spi_sync()' fails which would leave > > > 'adis->current_page' in an incoherent state. Hence, set this variable > > > right after we change the device page. > > > > > > Signed-off-by: Nuno Sa <nuno.sa@analog.com> > > > --- > > > drivers/iio/imu/adis_buffer.c | 6 +++--- > > > 1 file changed, 3 insertions(+), 3 deletions(-) > > > > > > diff --git a/drivers/iio/imu/adis_buffer.c > > b/drivers/iio/imu/adis_buffer.c > > > index 73790b71102b..aa37981c28f1 100644 > > > --- a/drivers/iio/imu/adis_buffer.c > > > +++ b/drivers/iio/imu/adis_buffer.c > > > @@ -142,6 +142,8 @@ static irqreturn_t adis_trigger_handler(int irq, > > void *p) > > > dev_err(&adis->spi->dev, "Failed to change device > > page: %d\n", ret); > > > goto irq_done; > > > } > > > + > > > + adis->current_page = 0; > > > > If the above spi_write() fails, this adis->current_page isn't reset. > > Maybe reset this as the first thing in this if block? > > > > If the 'spi_write()' fails it means that we could not change to page0, > so we should not update the current_page... ack
diff --git a/drivers/iio/imu/adis_buffer.c b/drivers/iio/imu/adis_buffer.c index 73790b71102b..aa37981c28f1 100644 --- a/drivers/iio/imu/adis_buffer.c +++ b/drivers/iio/imu/adis_buffer.c @@ -142,6 +142,8 @@ static irqreturn_t adis_trigger_handler(int irq, void *p) dev_err(&adis->spi->dev, "Failed to change device page: %d\n", ret); goto irq_done; } + + adis->current_page = 0; } } @@ -151,10 +153,8 @@ static irqreturn_t adis_trigger_handler(int irq, void *p) goto irq_done; } - if (adis->data->has_paging) { - adis->current_page = 0; + if (adis->data->has_paging) mutex_unlock(&adis->state_lock); - } iio_push_to_buffers_with_timestamp(indio_dev, adis->buffer, pf->timestamp);
With commit 41f2770a07e0 ("iio: adis_buffer: don't push data to buffers on failure"), we return if 'spi_sync()' fails which would leave 'adis->current_page' in an incoherent state. Hence, set this variable right after we change the device page. Signed-off-by: Nuno Sa <nuno.sa@analog.com> --- drivers/iio/imu/adis_buffer.c | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-)