linux-iio.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [PATCH] iio: light: gp2ap002: Take runtime PM reference on light read
@ 2020-05-09  1:42 Jonathan Bakker
  2020-05-10 10:20 ` Jonathan Cameron
  2020-05-10 15:58 ` [PATCH v2] " Jonathan Bakker
  0 siblings, 2 replies; 5+ messages in thread
From: Jonathan Bakker @ 2020-05-09  1:42 UTC (permalink / raw)
  To: linus.walleij, jic23, knaack.h, lars, pmeerw, linux-iio, linux-kernel
  Cc: Jonathan Bakker

The light sensor needs the regulators to be enabled which means
the runtime PM needs to be on.  This only happened when the
proximity part of the chip was enabled.

As fallout from this change, only report changes to the prox
state in the interrupt handler when it is explicitly enabled.

Signed-off-by: Jonathan Bakker <xc-racer2@live.ca>
---
 drivers/iio/light/gp2ap002.c | 19 ++++++++++++++++---
 1 file changed, 16 insertions(+), 3 deletions(-)

diff --git a/drivers/iio/light/gp2ap002.c b/drivers/iio/light/gp2ap002.c
index b7ef16b28280..7a2679bdc987 100644
--- a/drivers/iio/light/gp2ap002.c
+++ b/drivers/iio/light/gp2ap002.c
@@ -158,6 +158,9 @@ static irqreturn_t gp2ap002_prox_irq(int irq, void *d)
 	int val;
 	int ret;
 
+	if (!gp2ap002->enabled)
+		goto err_retrig;
+
 	ret = regmap_read(gp2ap002->map, GP2AP002_PROX, &val);
 	if (ret) {
 		dev_err(gp2ap002->dev, "error reading proximity\n");
@@ -247,6 +250,8 @@ static int gp2ap002_read_raw(struct iio_dev *indio_dev,
 	struct gp2ap002 *gp2ap002 = iio_priv(indio_dev);
 	int ret;
 
+	pm_runtime_get_sync(gp2ap002->dev);
+
 	switch (mask) {
 	case IIO_CHAN_INFO_RAW:
 		switch (chan->type) {
@@ -255,13 +260,21 @@ static int gp2ap002_read_raw(struct iio_dev *indio_dev,
 			if (ret < 0)
 				return ret;
 			*val = ret;
-			return IIO_VAL_INT;
+			ret = IIO_VAL_INT;
+			goto out;
 		default:
-			return -EINVAL;
+			ret = -EINVAL;
+			goto out;
 		}
 	default:
-		return -EINVAL;
+		ret = -EINVAL;
 	}
+
+out:
+	pm_runtime_mark_last_busy(gp2ap002->dev);
+	pm_runtime_put_autosuspend(gp2ap002->dev);
+
+	return ret;
 }
 
 static int gp2ap002_init(struct gp2ap002 *gp2ap002)
-- 
2.20.1


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

* Re: [PATCH] iio: light: gp2ap002: Take runtime PM reference on light read
  2020-05-09  1:42 [PATCH] iio: light: gp2ap002: Take runtime PM reference on light read Jonathan Bakker
@ 2020-05-10 10:20 ` Jonathan Cameron
  2020-05-10 15:58 ` [PATCH v2] " Jonathan Bakker
  1 sibling, 0 replies; 5+ messages in thread
From: Jonathan Cameron @ 2020-05-10 10:20 UTC (permalink / raw)
  To: Jonathan Bakker
  Cc: linus.walleij, knaack.h, lars, pmeerw, linux-iio, linux-kernel

On Fri,  8 May 2020 18:42:21 -0700
Jonathan Bakker <xc-racer2@live.ca> wrote:

> The light sensor needs the regulators to be enabled which means
> the runtime PM needs to be on.  This only happened when the
> proximity part of the chip was enabled.
> 
> As fallout from this change, only report changes to the prox
> state in the interrupt handler when it is explicitly enabled.
> 
> Signed-off-by: Jonathan Bakker <xc-racer2@live.ca>
Looks good to me, but needs a fixes tag as we'll want to fix
this up in all kernels where it applies (just the current
RCs I think..)

Thanks,

Jonathan

> ---
>  drivers/iio/light/gp2ap002.c | 19 ++++++++++++++++---
>  1 file changed, 16 insertions(+), 3 deletions(-)
> 
> diff --git a/drivers/iio/light/gp2ap002.c b/drivers/iio/light/gp2ap002.c
> index b7ef16b28280..7a2679bdc987 100644
> --- a/drivers/iio/light/gp2ap002.c
> +++ b/drivers/iio/light/gp2ap002.c
> @@ -158,6 +158,9 @@ static irqreturn_t gp2ap002_prox_irq(int irq, void *d)
>  	int val;
>  	int ret;
>  
> +	if (!gp2ap002->enabled)
> +		goto err_retrig;
> +
>  	ret = regmap_read(gp2ap002->map, GP2AP002_PROX, &val);
>  	if (ret) {
>  		dev_err(gp2ap002->dev, "error reading proximity\n");
> @@ -247,6 +250,8 @@ static int gp2ap002_read_raw(struct iio_dev *indio_dev,
>  	struct gp2ap002 *gp2ap002 = iio_priv(indio_dev);
>  	int ret;
>  
> +	pm_runtime_get_sync(gp2ap002->dev);
> +
>  	switch (mask) {
>  	case IIO_CHAN_INFO_RAW:
>  		switch (chan->type) {
> @@ -255,13 +260,21 @@ static int gp2ap002_read_raw(struct iio_dev *indio_dev,
>  			if (ret < 0)
>  				return ret;
>  			*val = ret;
> -			return IIO_VAL_INT;
> +			ret = IIO_VAL_INT;
> +			goto out;
>  		default:
> -			return -EINVAL;
> +			ret = -EINVAL;
> +			goto out;
>  		}
>  	default:
> -		return -EINVAL;
> +		ret = -EINVAL;
>  	}
> +
> +out:
> +	pm_runtime_mark_last_busy(gp2ap002->dev);
> +	pm_runtime_put_autosuspend(gp2ap002->dev);
> +
> +	return ret;
>  }
>  
>  static int gp2ap002_init(struct gp2ap002 *gp2ap002)


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

* [PATCH v2] iio: light: gp2ap002: Take runtime PM reference on light read
  2020-05-09  1:42 [PATCH] iio: light: gp2ap002: Take runtime PM reference on light read Jonathan Bakker
  2020-05-10 10:20 ` Jonathan Cameron
@ 2020-05-10 15:58 ` Jonathan Bakker
  2020-05-12  9:03   ` Linus Walleij
  1 sibling, 1 reply; 5+ messages in thread
From: Jonathan Bakker @ 2020-05-10 15:58 UTC (permalink / raw)
  To: linus.walleij, jic23, knaack.h, lars, pmeerw, linux-iio, linux-kernel
  Cc: Jonathan Bakker

The light sensor needs the regulators to be enabled which means
the runtime PM needs to be on.  This only happened when the
proximity part of the chip was enabled.

As fallout from this change, only report changes to the prox
state in the interrupt handler when it is explicitly enabled.

Fixes: 97d642e23037 ("iio: light: Add a driver for Sharp GP2AP002x00F")
Signed-off-by: Jonathan Bakker <xc-racer2@live.ca>
---
Changes from v1:
- Add Fixes tag
---
 drivers/iio/light/gp2ap002.c | 19 ++++++++++++++++---
 1 file changed, 16 insertions(+), 3 deletions(-)

diff --git a/drivers/iio/light/gp2ap002.c b/drivers/iio/light/gp2ap002.c
index b7ef16b28280..7a2679bdc987 100644
--- a/drivers/iio/light/gp2ap002.c
+++ b/drivers/iio/light/gp2ap002.c
@@ -158,6 +158,9 @@ static irqreturn_t gp2ap002_prox_irq(int irq, void *d)
 	int val;
 	int ret;
 
+	if (!gp2ap002->enabled)
+		goto err_retrig;
+
 	ret = regmap_read(gp2ap002->map, GP2AP002_PROX, &val);
 	if (ret) {
 		dev_err(gp2ap002->dev, "error reading proximity\n");
@@ -247,6 +250,8 @@ static int gp2ap002_read_raw(struct iio_dev *indio_dev,
 	struct gp2ap002 *gp2ap002 = iio_priv(indio_dev);
 	int ret;
 
+	pm_runtime_get_sync(gp2ap002->dev);
+
 	switch (mask) {
 	case IIO_CHAN_INFO_RAW:
 		switch (chan->type) {
@@ -255,13 +260,21 @@ static int gp2ap002_read_raw(struct iio_dev *indio_dev,
 			if (ret < 0)
 				return ret;
 			*val = ret;
-			return IIO_VAL_INT;
+			ret = IIO_VAL_INT;
+			goto out;
 		default:
-			return -EINVAL;
+			ret = -EINVAL;
+			goto out;
 		}
 	default:
-		return -EINVAL;
+		ret = -EINVAL;
 	}
+
+out:
+	pm_runtime_mark_last_busy(gp2ap002->dev);
+	pm_runtime_put_autosuspend(gp2ap002->dev);
+
+	return ret;
 }
 
 static int gp2ap002_init(struct gp2ap002 *gp2ap002)
-- 
2.20.1


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

* Re: [PATCH v2] iio: light: gp2ap002: Take runtime PM reference on light read
  2020-05-10 15:58 ` [PATCH v2] " Jonathan Bakker
@ 2020-05-12  9:03   ` Linus Walleij
  2020-05-16 16:34     ` Jonathan Cameron
  0 siblings, 1 reply; 5+ messages in thread
From: Linus Walleij @ 2020-05-12  9:03 UTC (permalink / raw)
  To: Jonathan Bakker
  Cc: Jonathan Cameron, Hartmut Knaack, Lars-Peter Clausen,
	Peter Meerwald, linux-iio, linux-kernel

On Sun, May 10, 2020 at 5:58 PM Jonathan Bakker <xc-racer2@live.ca> wrote:

> The light sensor needs the regulators to be enabled which means
> the runtime PM needs to be on.  This only happened when the
> proximity part of the chip was enabled.
>
> As fallout from this change, only report changes to the prox
> state in the interrupt handler when it is explicitly enabled.
>
> Fixes: 97d642e23037 ("iio: light: Add a driver for Sharp GP2AP002x00F")
> Signed-off-by: Jonathan Bakker <xc-racer2@live.ca>

Reviewed-by: Linus Walleij <linus.walleij@linaro.org>

Sorry for missing this!

Yours,
Linus Walleij

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

* Re: [PATCH v2] iio: light: gp2ap002: Take runtime PM reference on light read
  2020-05-12  9:03   ` Linus Walleij
@ 2020-05-16 16:34     ` Jonathan Cameron
  0 siblings, 0 replies; 5+ messages in thread
From: Jonathan Cameron @ 2020-05-16 16:34 UTC (permalink / raw)
  To: Linus Walleij
  Cc: Jonathan Bakker, Hartmut Knaack, Lars-Peter Clausen,
	Peter Meerwald, linux-iio, linux-kernel

On Tue, 12 May 2020 11:03:22 +0200
Linus Walleij <linus.walleij@linaro.org> wrote:

> On Sun, May 10, 2020 at 5:58 PM Jonathan Bakker <xc-racer2@live.ca> wrote:
> 
> > The light sensor needs the regulators to be enabled which means
> > the runtime PM needs to be on.  This only happened when the
> > proximity part of the chip was enabled.
> >
> > As fallout from this change, only report changes to the prox
> > state in the interrupt handler when it is explicitly enabled.
> >
> > Fixes: 97d642e23037 ("iio: light: Add a driver for Sharp GP2AP002x00F")
> > Signed-off-by: Jonathan Bakker <xc-racer2@live.ca>  
> 
> Reviewed-by: Linus Walleij <linus.walleij@linaro.org>
> 
> Sorry for missing this!
> 
> Yours,
> Linus Walleij
Applied to the fixes-togreg branch of iio.git. 

Thanks,

Jonathan



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

end of thread, other threads:[~2020-05-16 16:34 UTC | newest]

Thread overview: 5+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2020-05-09  1:42 [PATCH] iio: light: gp2ap002: Take runtime PM reference on light read Jonathan Bakker
2020-05-10 10:20 ` Jonathan Cameron
2020-05-10 15:58 ` [PATCH v2] " Jonathan Bakker
2020-05-12  9:03   ` Linus Walleij
2020-05-16 16:34     ` 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).