linux-kernel.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [PATCH] iio: potentiostat: lmp9100: fix iio_triggered_buffer_{predisable,postenable} positions
@ 2020-03-04  9:36 Alexandru Ardelean
  2020-03-04  9:41 ` [PATCH v2] " Alexandru Ardelean
  0 siblings, 1 reply; 4+ messages in thread
From: Alexandru Ardelean @ 2020-03-04  9:36 UTC (permalink / raw)
  To: linux-iio, linux-kernel
  Cc: jic23, mranostay, matt.ranostay, Alexandru Ardelean

The iio_triggered_buffer_{predisable,postenable} functions attach/detach
the poll functions.

For the predisable hook, the disable code should occur before detaching
the poll func, and for the postenable hook, the poll func should be
attached before the enable code.

The lmp9100 was attaching a poll function but never detaching it via any
IIO disable hook.

This change adds the detaching of the poll function, and moves/renames
lmp91000_buffer_preenable() function to lmp91000_buffer_postenable().
The idea is to make it more symmetrical, so that when the
iio_triggered_buffer_{predisable,postenable} functions get removed, it's
easier to see.

Fixes: 67e17300dc1d7 ("iio: potentiostat: add LMP91000 support")
Signed-off-by: Alexandru Ardelean <alexandru.ardelean@analog.com>
---
 drivers/iio/potentiostat/lmp91000.c | 14 +++++++++-----
 1 file changed, 9 insertions(+), 5 deletions(-)

diff --git a/drivers/iio/potentiostat/lmp91000.c b/drivers/iio/potentiostat/lmp91000.c
index a0e5f530faa9..72c814fd63a7 100644
--- a/drivers/iio/potentiostat/lmp91000.c
+++ b/drivers/iio/potentiostat/lmp91000.c
@@ -275,11 +275,16 @@ static int lmp91000_buffer_cb(const void *val, void *private)
 static const struct iio_trigger_ops lmp91000_trigger_ops = {
 };
 
-static int lmp91000_buffer_preenable(struct iio_dev *indio_dev)
+static int lmp91000_buffer_postenable(struct iio_dev *indio_dev)
 {
 	struct lmp91000_data *data = iio_priv(indio_dev);
+	int err;
 
-	return iio_channel_start_all_cb(data->cb_buffer);
+	err = iio_channel_start_all_cb(data->cb_buffer);
+	if (err)
+		iio_triggered_buffer_predisable(indio_dev);
+
+	return err;
 }
 
 static int lmp91000_buffer_predisable(struct iio_dev *indio_dev)
@@ -288,12 +293,11 @@ static int lmp91000_buffer_predisable(struct iio_dev *indio_dev)
 
 	iio_channel_stop_all_cb(data->cb_buffer);
 
-	return 0;
+	return iio_triggered_buffer_predisable(indio_dev);
 }
 
 static const struct iio_buffer_setup_ops lmp91000_buffer_setup_ops = {
-	.preenable = lmp91000_buffer_preenable,
-	.postenable = iio_triggered_buffer_postenable,
+	.postenable = lmp91000_buffer_postenable,
 	.predisable = lmp91000_buffer_predisable,
 };
 
-- 
2.20.1


^ permalink raw reply related	[flat|nested] 4+ messages in thread

* [PATCH v2] iio: potentiostat: lmp9100: fix iio_triggered_buffer_{predisable,postenable} positions
  2020-03-04  9:36 [PATCH] iio: potentiostat: lmp9100: fix iio_triggered_buffer_{predisable,postenable} positions Alexandru Ardelean
@ 2020-03-04  9:41 ` Alexandru Ardelean
  2020-03-04 21:17   ` Matt Ranostay
  0 siblings, 1 reply; 4+ messages in thread
From: Alexandru Ardelean @ 2020-03-04  9:41 UTC (permalink / raw)
  To: linux-iio, linux-kernel
  Cc: jic23, mranostay, matt.ranostay, Alexandru Ardelean

The iio_triggered_buffer_{predisable,postenable} functions attach/detach
the poll functions.

For the predisable hook, the disable code should occur before detaching
the poll func, and for the postenable hook, the poll func should be
attached before the enable code.

The lmp9100 was attaching a poll function but never detaching it via any
IIO disable hook.

This change adds the detaching of the poll function, and moves/renames
lmp91000_buffer_preenable() function to lmp91000_buffer_postenable().
The idea is to make it more symmetrical, so that when the
iio_triggered_buffer_{predisable,postenable} functions get removed, it's
easier to see.

Fixes: 67e17300dc1d7 ("iio: potentiostat: add LMP91000 support")
Signed-off-by: Alexandru Ardelean <alexandru.ardelean@analog.com>
---

Changelog v1 -> v2:
* forgot to call iio_triggered_buffer_postenable() in
  lmp91000_buffer_postenable() in v1

 drivers/iio/potentiostat/lmp91000.c | 18 +++++++++++++-----
 1 file changed, 13 insertions(+), 5 deletions(-)

diff --git a/drivers/iio/potentiostat/lmp91000.c b/drivers/iio/potentiostat/lmp91000.c
index a0e5f530faa9..2cb11da18e0f 100644
--- a/drivers/iio/potentiostat/lmp91000.c
+++ b/drivers/iio/potentiostat/lmp91000.c
@@ -275,11 +275,20 @@ static int lmp91000_buffer_cb(const void *val, void *private)
 static const struct iio_trigger_ops lmp91000_trigger_ops = {
 };
 
-static int lmp91000_buffer_preenable(struct iio_dev *indio_dev)
+static int lmp91000_buffer_postenable(struct iio_dev *indio_dev)
 {
 	struct lmp91000_data *data = iio_priv(indio_dev);
+	int err;
 
-	return iio_channel_start_all_cb(data->cb_buffer);
+	err = iio_triggered_buffer_postenable(indio_dev);
+	if (err)
+		return err;
+
+	err = iio_channel_start_all_cb(data->cb_buffer);
+	if (err)
+		iio_triggered_buffer_predisable(indio_dev);
+
+	return err;
 }
 
 static int lmp91000_buffer_predisable(struct iio_dev *indio_dev)
@@ -288,12 +297,11 @@ static int lmp91000_buffer_predisable(struct iio_dev *indio_dev)
 
 	iio_channel_stop_all_cb(data->cb_buffer);
 
-	return 0;
+	return iio_triggered_buffer_predisable(indio_dev);
 }
 
 static const struct iio_buffer_setup_ops lmp91000_buffer_setup_ops = {
-	.preenable = lmp91000_buffer_preenable,
-	.postenable = iio_triggered_buffer_postenable,
+	.postenable = lmp91000_buffer_postenable,
 	.predisable = lmp91000_buffer_predisable,
 };
 
-- 
2.20.1


^ permalink raw reply related	[flat|nested] 4+ messages in thread

* Re: [PATCH v2] iio: potentiostat: lmp9100: fix iio_triggered_buffer_{predisable,postenable} positions
  2020-03-04  9:41 ` [PATCH v2] " Alexandru Ardelean
@ 2020-03-04 21:17   ` Matt Ranostay
  2020-03-07 15:25     ` Jonathan Cameron
  0 siblings, 1 reply; 4+ messages in thread
From: Matt Ranostay @ 2020-03-04 21:17 UTC (permalink / raw)
  To: Alexandru Ardelean
  Cc: linux-iio, linux-kernel, Jonathan Cameron, matt.ranostay

On Wed, Mar 4, 2020 at 1:38 AM Alexandru Ardelean
<alexandru.ardelean@analog.com> wrote:
>
> The iio_triggered_buffer_{predisable,postenable} functions attach/detach
> the poll functions.
>
> For the predisable hook, the disable code should occur before detaching
> the poll func, and for the postenable hook, the poll func should be
> attached before the enable code.
>
> The lmp9100 was attaching a poll function but never detaching it via any
> IIO disable hook.
>
> This change adds the detaching of the poll function, and moves/renames
> lmp91000_buffer_preenable() function to lmp91000_buffer_postenable().
> The idea is to make it more symmetrical, so that when the
> iio_triggered_buffer_{predisable,postenable} functions get removed, it's
> easier to see.
>
> Fixes: 67e17300dc1d7 ("iio: potentiostat: add LMP91000 support")
> Signed-off-by: Alexandru Ardelean <alexandru.ardelean@analog.com>

Acked-by: Matt Ranostay <matt.ranostay@konsulko.com>

> ---
>
> Changelog v1 -> v2:
> * forgot to call iio_triggered_buffer_postenable() in
>   lmp91000_buffer_postenable() in v1
>
>  drivers/iio/potentiostat/lmp91000.c | 18 +++++++++++++-----
>  1 file changed, 13 insertions(+), 5 deletions(-)
>
> diff --git a/drivers/iio/potentiostat/lmp91000.c b/drivers/iio/potentiostat/lmp91000.c
> index a0e5f530faa9..2cb11da18e0f 100644
> --- a/drivers/iio/potentiostat/lmp91000.c
> +++ b/drivers/iio/potentiostat/lmp91000.c
> @@ -275,11 +275,20 @@ static int lmp91000_buffer_cb(const void *val, void *private)
>  static const struct iio_trigger_ops lmp91000_trigger_ops = {
>  };
>
> -static int lmp91000_buffer_preenable(struct iio_dev *indio_dev)
> +static int lmp91000_buffer_postenable(struct iio_dev *indio_dev)
>  {
>         struct lmp91000_data *data = iio_priv(indio_dev);
> +       int err;
>
> -       return iio_channel_start_all_cb(data->cb_buffer);
> +       err = iio_triggered_buffer_postenable(indio_dev);
> +       if (err)
> +               return err;
> +
> +       err = iio_channel_start_all_cb(data->cb_buffer);
> +       if (err)
> +               iio_triggered_buffer_predisable(indio_dev);
> +
> +       return err;
>  }
>
>  static int lmp91000_buffer_predisable(struct iio_dev *indio_dev)
> @@ -288,12 +297,11 @@ static int lmp91000_buffer_predisable(struct iio_dev *indio_dev)
>
>         iio_channel_stop_all_cb(data->cb_buffer);
>
> -       return 0;
> +       return iio_triggered_buffer_predisable(indio_dev);
>  }
>
>  static const struct iio_buffer_setup_ops lmp91000_buffer_setup_ops = {
> -       .preenable = lmp91000_buffer_preenable,
> -       .postenable = iio_triggered_buffer_postenable,
> +       .postenable = lmp91000_buffer_postenable,
>         .predisable = lmp91000_buffer_predisable,
>  };
>
> --
> 2.20.1
>

^ permalink raw reply	[flat|nested] 4+ messages in thread

* Re: [PATCH v2] iio: potentiostat: lmp9100: fix iio_triggered_buffer_{predisable,postenable} positions
  2020-03-04 21:17   ` Matt Ranostay
@ 2020-03-07 15:25     ` Jonathan Cameron
  0 siblings, 0 replies; 4+ messages in thread
From: Jonathan Cameron @ 2020-03-07 15:25 UTC (permalink / raw)
  To: Matt Ranostay; +Cc: Alexandru Ardelean, linux-iio, linux-kernel, matt.ranostay

On Wed, 4 Mar 2020 13:17:36 -0800
Matt Ranostay <mranostay@gmail.com> wrote:

> On Wed, Mar 4, 2020 at 1:38 AM Alexandru Ardelean
> <alexandru.ardelean@analog.com> wrote:
> >
> > The iio_triggered_buffer_{predisable,postenable} functions attach/detach
> > the poll functions.
> >
> > For the predisable hook, the disable code should occur before detaching
> > the poll func, and for the postenable hook, the poll func should be
> > attached before the enable code.
> >
> > The lmp9100 was attaching a poll function but never detaching it via any
> > IIO disable hook.
> >
> > This change adds the detaching of the poll function, and moves/renames
> > lmp91000_buffer_preenable() function to lmp91000_buffer_postenable().
> > The idea is to make it more symmetrical, so that when the
> > iio_triggered_buffer_{predisable,postenable} functions get removed, it's
> > easier to see.
> >
> > Fixes: 67e17300dc1d7 ("iio: potentiostat: add LMP91000 support")
> > Signed-off-by: Alexandru Ardelean <alexandru.ardelean@analog.com>  
> 
> Acked-by: Matt Ranostay <matt.ranostay@konsulko.com>

I'm not going to rush this one in as it's been like this for a while

Applied to the togreg branch of iio.git and pushed out as testing for the
autobuilders to play with it,

Thanks,

Jonathan

> 
> > ---
> >
> > Changelog v1 -> v2:
> > * forgot to call iio_triggered_buffer_postenable() in
> >   lmp91000_buffer_postenable() in v1
> >
> >  drivers/iio/potentiostat/lmp91000.c | 18 +++++++++++++-----
> >  1 file changed, 13 insertions(+), 5 deletions(-)
> >
> > diff --git a/drivers/iio/potentiostat/lmp91000.c b/drivers/iio/potentiostat/lmp91000.c
> > index a0e5f530faa9..2cb11da18e0f 100644
> > --- a/drivers/iio/potentiostat/lmp91000.c
> > +++ b/drivers/iio/potentiostat/lmp91000.c
> > @@ -275,11 +275,20 @@ static int lmp91000_buffer_cb(const void *val, void *private)
> >  static const struct iio_trigger_ops lmp91000_trigger_ops = {
> >  };
> >
> > -static int lmp91000_buffer_preenable(struct iio_dev *indio_dev)
> > +static int lmp91000_buffer_postenable(struct iio_dev *indio_dev)
> >  {
> >         struct lmp91000_data *data = iio_priv(indio_dev);
> > +       int err;
> >
> > -       return iio_channel_start_all_cb(data->cb_buffer);
> > +       err = iio_triggered_buffer_postenable(indio_dev);
> > +       if (err)
> > +               return err;
> > +
> > +       err = iio_channel_start_all_cb(data->cb_buffer);
> > +       if (err)
> > +               iio_triggered_buffer_predisable(indio_dev);
> > +
> > +       return err;
> >  }
> >
> >  static int lmp91000_buffer_predisable(struct iio_dev *indio_dev)
> > @@ -288,12 +297,11 @@ static int lmp91000_buffer_predisable(struct iio_dev *indio_dev)
> >
> >         iio_channel_stop_all_cb(data->cb_buffer);
> >
> > -       return 0;
> > +       return iio_triggered_buffer_predisable(indio_dev);
> >  }
> >
> >  static const struct iio_buffer_setup_ops lmp91000_buffer_setup_ops = {
> > -       .preenable = lmp91000_buffer_preenable,
> > -       .postenable = iio_triggered_buffer_postenable,
> > +       .postenable = lmp91000_buffer_postenable,
> >         .predisable = lmp91000_buffer_predisable,
> >  };
> >
> > --
> > 2.20.1
> >  


^ permalink raw reply	[flat|nested] 4+ messages in thread

end of thread, other threads:[~2020-03-07 15:25 UTC | newest]

Thread overview: 4+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2020-03-04  9:36 [PATCH] iio: potentiostat: lmp9100: fix iio_triggered_buffer_{predisable,postenable} positions Alexandru Ardelean
2020-03-04  9:41 ` [PATCH v2] " Alexandru Ardelean
2020-03-04 21:17   ` Matt Ranostay
2020-03-07 15:25     ` Jonathan Cameron

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).