2017-09-14 13:50:49

by Stefan Popa

Subject: [PATCH] staging: iio: ad7192: Use the dedicated reset function

SPI host drivers can use DMA to transfer data, so the buffer should be properly allocated.
Keeping it on the stack could cause an undefined behavior.

The dedicated reset function solves this issue.

Signed-off-by: Stefan Popa <[email protected]>
---
drivers/staging/iio/adc/ad7192.c | 4 +---
1 file changed, 1 insertion(+), 3 deletions(-)

diff --git a/drivers/staging/iio/adc/ad7192.c b/drivers/staging/iio/adc/ad7192.c
index d11c6de..6150d27 100644
--- a/drivers/staging/iio/adc/ad7192.c
+++ b/drivers/staging/iio/adc/ad7192.c
@@ -223,11 +223,9 @@ static int ad7192_setup(struct ad7192_state *st,
struct iio_dev *indio_dev = spi_get_drvdata(st->sd.spi);
unsigned long long scale_uv;
int i, ret, id;
- u8 ones[6];

/* reset the serial interface */
- memset(&ones, 0xFF, 6);
- ret = spi_write(st->sd.spi, &ones, 6);
+ ret = ad_sd_reset(&st->sd, 48);
if (ret < 0)
goto out;
usleep_range(500, 1000); /* Wait for at least 500us */
--
2.7.4

2017-09-14 13:57:43

by Lars-Peter Clausen

Subject: Re: [PATCH] staging: iio: ad7192: Use the dedicated reset function

On 09/14/2017 03:50 PM, Stefan Popa wrote:
> SPI host drivers can use DMA to transfer data, so the buffer should be properly allocated.
> Keeping it on the stack could cause an undefined behavior.
>
> The dedicated reset function solves this issue.
>
> Signed-off-by: Stefan Popa <[email protected]>

Acked-by: Lars-Peter Clausen <[email protected]>

Thanks.

> ---
> drivers/staging/iio/adc/ad7192.c | 4 +---
> 1 file changed, 1 insertion(+), 3 deletions(-)
>
> diff --git a/drivers/staging/iio/adc/ad7192.c b/drivers/staging/iio/adc/ad7192.c
> index d11c6de..6150d27 100644
> --- a/drivers/staging/iio/adc/ad7192.c
> +++ b/drivers/staging/iio/adc/ad7192.c
> @@ -223,11 +223,9 @@ static int ad7192_setup(struct ad7192_state *st,
> struct iio_dev *indio_dev = spi_get_drvdata(st->sd.spi);
> unsigned long long scale_uv;
> int i, ret, id;
> - u8 ones[6];
>
> /* reset the serial interface */
> - memset(&ones, 0xFF, 6);
> - ret = spi_write(st->sd.spi, &ones, 6);
> + ret = ad_sd_reset(&st->sd, 48);
> if (ret < 0)
> goto out;
> usleep_range(500, 1000); /* Wait for at least 500us */
>

2017-09-14 14:29:02

by Hennerich, Michael

Subject: Re: [PATCH] staging: iio: ad7192: Use the dedicated reset function

On 14.09.2017 15:50, Stefan Popa wrote:
> SPI host drivers can use DMA to transfer data, so the buffer should be properly allocated.
> Keeping it on the stack could cause an undefined behavior.
>
> The dedicated reset function solves this issue.
>
> Signed-off-by: Stefan Popa <[email protected]>

Acked-by: Michael Hennerich <[email protected]>

Well done!


> ---
> drivers/staging/iio/adc/ad7192.c | 4 +---
> 1 file changed, 1 insertion(+), 3 deletions(-)
>
> diff --git a/drivers/staging/iio/adc/ad7192.c b/drivers/staging/iio/adc/ad7192.c
> index d11c6de..6150d27 100644
> --- a/drivers/staging/iio/adc/ad7192.c
> +++ b/drivers/staging/iio/adc/ad7192.c
> @@ -223,11 +223,9 @@ static int ad7192_setup(struct ad7192_state *st,
> struct iio_dev *indio_dev = spi_get_drvdata(st->sd.spi);
> unsigned long long scale_uv;
> int i, ret, id;
> - u8 ones[6];
>
> /* reset the serial interface */
> - memset(&ones, 0xFF, 6);
> - ret = spi_write(st->sd.spi, &ones, 6);
> + ret = ad_sd_reset(&st->sd, 48);
> if (ret < 0)
> goto out;
> usleep_range(500, 1000); /* Wait for at least 500us */
>


--
Greetings,
Michael

--
Analog Devices GmbH Otl-Aicher Strasse 60-64 80807 München
Sitz der Gesellschaft München, Registergericht München HRB 40368,
Geschäftsführer: Peter Kolberg, Ali Raza Husain, Eileen Wynne

2017-09-16 22:22:29

by Jonathan Cameron

Subject: Re: [PATCH] staging: iio: ad7192: Use the dedicated reset function

On Thu, 14 Sep 2017 16:31:06 +0200
Michael Hennerich <[email protected]> wrote:

> On 14.09.2017 15:50, Stefan Popa wrote:
> > SPI host drivers can use DMA to transfer data, so the buffer should be properly allocated.
> > Keeping it on the stack could cause an undefined behavior.
> >
> > The dedicated reset function solves this issue.
> >
> > Signed-off-by: Stefan Popa <[email protected]>
>
> Acked-by: Michael Hennerich <[email protected]>

Applied to the togreg branch of iio.git rather than staging
branch as the reset functionality is reasonably recent and
not going to be available in stable kernels etc..

Good work. I was reading this on a plane the other day and
noticed the same issue - always nice when someone else
fixes something on your todo list ;)

Jonathan

>
> Well done!
>
>
> > ---
> > drivers/staging/iio/adc/ad7192.c | 4 +---
> > 1 file changed, 1 insertion(+), 3 deletions(-)
> >
> > diff --git a/drivers/staging/iio/adc/ad7192.c b/drivers/staging/iio/adc/ad7192.c
> > index d11c6de..6150d27 100644
> > --- a/drivers/staging/iio/adc/ad7192.c
> > +++ b/drivers/staging/iio/adc/ad7192.c
> > @@ -223,11 +223,9 @@ static int ad7192_setup(struct ad7192_state *st,
> > struct iio_dev *indio_dev = spi_get_drvdata(st->sd.spi);
> > unsigned long long scale_uv;
> > int i, ret, id;
> > - u8 ones[6];
> >
> > /* reset the serial interface */
> > - memset(&ones, 0xFF, 6);
> > - ret = spi_write(st->sd.spi, &ones, 6);
> > + ret = ad_sd_reset(&st->sd, 48);
> > if (ret < 0)
> > goto out;
> > usleep_range(500, 1000); /* Wait for at least 500us */
> >
>
>

2017-09-16 22:33:11

by Jonathan Cameron

Subject: Re: [PATCH] staging: iio: ad7192: Use the dedicated reset function

On Sat, 16 Sep 2017 15:22:23 -0700
Jonathan Cameron <[email protected]> wrote:

> On Thu, 14 Sep 2017 16:31:06 +0200
> Michael Hennerich <[email protected]> wrote:
>
> > On 14.09.2017 15:50, Stefan Popa wrote:
> > > SPI host drivers can use DMA to transfer data, so the buffer should be properly allocated.
> > > Keeping it on the stack could cause an undefined behavior.
> > >
> > > The dedicated reset function solves this issue.
> > >
> > > Signed-off-by: Stefan Popa <[email protected]>
> >
> > Acked-by: Michael Hennerich <[email protected]>
>
> Applied to the togreg branch of iio.git rather than staging
> branch as the reset functionality is reasonably recent and
> not going to be available in stable kernels etc..
>
> Good work. I was reading this on a plane the other day and
> noticed the same issue - always nice when someone else
> fixes something on your todo list ;)
>
> Jonathan

Doh, I had forgotten the support was part of a fix for a different
driver so is only in the fixes-togreg branch of iio.git.

I'll take this one the same way with a bit of a tweak to the patch
title to make it clear it is fixing something.

Jonathan
>
> >
> > Well done!
> >
> >
> > > ---
> > > drivers/staging/iio/adc/ad7192.c | 4 +---
> > > 1 file changed, 1 insertion(+), 3 deletions(-)
> > >
> > > diff --git a/drivers/staging/iio/adc/ad7192.c b/drivers/staging/iio/adc/ad7192.c
> > > index d11c6de..6150d27 100644
> > > --- a/drivers/staging/iio/adc/ad7192.c
> > > +++ b/drivers/staging/iio/adc/ad7192.c
> > > @@ -223,11 +223,9 @@ static int ad7192_setup(struct ad7192_state *st,
> > > struct iio_dev *indio_dev = spi_get_drvdata(st->sd.spi);
> > > unsigned long long scale_uv;
> > > int i, ret, id;
> > > - u8 ones[6];
> > >
> > > /* reset the serial interface */
> > > - memset(&ones, 0xFF, 6);
> > > - ret = spi_write(st->sd.spi, &ones, 6);
> > > + ret = ad_sd_reset(&st->sd, 48);
> > > if (ret < 0)
> > > goto out;
> > > usleep_range(500, 1000); /* Wait for at least 500us */
> > >
> >
> >
>
> --
> To unsubscribe from this list: send the line "unsubscribe linux-iio" in
> the body of a message to [email protected]
> More majordomo info at http://vger.kernel.org/majordomo-info.html