diff mbox series

[7/7] iio: adis_buffer: update device page after changing it

Message ID 20210413112105.69458-8-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
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(-)

Comments

Alexandru Ardelean April 14, 2021, 7:38 a.m. UTC | #1
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
>
Sa, Nuno April 15, 2021, 7:58 a.m. UTC | #2
> -----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...
Alexandru Ardelean April 15, 2021, 8:20 a.m. UTC | #3
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 mbox series

Patch

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);