linux-kernel.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [PATCH v2] iio: humidity: hts221: Fix sensor reads after resume
@ 2018-06-28  8:22 Shrirang Bagul
  2018-06-28  9:05 ` Lorenzo Bianconi
  0 siblings, 1 reply; 3+ messages in thread
From: Shrirang Bagul @ 2018-06-28  8:22 UTC (permalink / raw)
  To: jic23, lorenzo.bianconi; +Cc: lorenzo.bianconi83, linux-iio, linux-kernel

AV_CONF register (RH & TEMP. oversampling ratio's) and CTRL1 register
(ODR & BDU settings) values are lost after suspend.

While the change in AV_CONF updates the sensor resolution modes
(overriding the user configuration before the device went to suspend);
loss of the contents of the CTRL1 register leads to failure in reading
sensor output.

This patch restores the AV_CONF & CTRL1 registers after resume.

Changes from v1:
- Don't drop enabling the sensor during resume
- Restore AV_CONF register along with CTRL1
  (As demonstrated with i2c register dumps of hts221 captured on Dell
  IoT Gateways 300x before suspend & after resume [1])

Fixes: ffebe74b7c95 (iio: humidity: hts221: avoid useless ODR reconfiguration)
Signed-off-by: Shrirang Bagul <shrirang.bagul@canonical.com>

[v1] https://marc.info/?l=linux-iio&m=152506543000442&w=2
[1] https://marc.info/?l=linux-iio&m=152534455701742&w=2

This patch is based on:
git://git.kernel.org/pub/scm/linux/kernel/git/jic23/iio.git -b testing
---
 drivers/iio/humidity/hts221_core.c | 31 +++++++++++++++++++++++++++++-
 1 file changed, 30 insertions(+), 1 deletion(-)

diff --git a/drivers/iio/humidity/hts221_core.c b/drivers/iio/humidity/hts221_core.c
index 166946d4978d..fc5599497f55 100644
--- a/drivers/iio/humidity/hts221_core.c
+++ b/drivers/iio/humidity/hts221_core.c
@@ -656,14 +656,43 @@ static int __maybe_unused hts221_resume(struct device *dev)
 {
 	struct iio_dev *iio_dev = dev_get_drvdata(dev);
 	struct hts221_hw *hw = iio_priv(iio_dev);
+	const struct hts221_avg *avg;
+	u8 data, idx;
 	int err = 0;
 
+	/* Restore contents of AV_CONF (RH & TEMP. oversampling ratio's) */
+	avg = &hts221_avg_list[HTS221_SENSOR_H];
+	idx = hw->sensors[HTS221_SENSOR_H].cur_avg_idx;
+	data = avg->avg_avl[idx];
+	err = hts221_update_avg(hw, HTS221_SENSOR_H, data);
+	if (err < 0)
+		goto fail_err;
+
+	avg = &hts221_avg_list[HTS221_SENSOR_T];
+	idx = hw->sensors[HTS221_SENSOR_T].cur_avg_idx;
+	data = avg->avg_avl[idx];
+	err = hts221_update_avg(hw, HTS221_SENSOR_T, data);
+	if (err < 0)
+		goto fail_err;
+
+	/* Restore contents of CTRL1 (BDU & ODR) */
+	err = regmap_update_bits(hw->regmap, HTS221_REG_CNTRL1_ADDR,
+				 HTS221_BDU_MASK,
+				 FIELD_PREP(HTS221_BDU_MASK, 1));
+	if (err < 0)
+		goto fail_err;
+
+	err = hts221_update_odr(hw, hw->odr);
+	if (err < 0)
+		goto fail_err;
+
 	if (hw->enabled)
 		err = regmap_update_bits(hw->regmap, HTS221_REG_CNTRL1_ADDR,
 					 HTS221_ENABLE_MASK,
 					 FIELD_PREP(HTS221_ENABLE_MASK,
 						    true));
-	return err;
+fail_err:
+	return err < 0 ? err : 0;
 }
 
 const struct dev_pm_ops hts221_pm_ops = {
-- 
2.17.1


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

* Re: [PATCH v2] iio: humidity: hts221: Fix sensor reads after resume
  2018-06-28  8:22 [PATCH v2] iio: humidity: hts221: Fix sensor reads after resume Shrirang Bagul
@ 2018-06-28  9:05 ` Lorenzo Bianconi
  2018-07-01  4:37   ` Shrirang Bagul
  0 siblings, 1 reply; 3+ messages in thread
From: Lorenzo Bianconi @ 2018-06-28  9:05 UTC (permalink / raw)
  To: Shrirang Bagul
  Cc: jic23, lorenzo.bianconi83, linux-iio, linux-kernel, mario.tesi,
	marco.bianco, armando.visconti

> AV_CONF register (RH & TEMP. oversampling ratio's) and CTRL1 register
> (ODR & BDU settings) values are lost after suspend.
> 
> While the change in AV_CONF updates the sensor resolution modes
> (overriding the user configuration before the device went to suspend);
> loss of the contents of the CTRL1 register leads to failure in reading
> sensor output.
> 
> This patch restores the AV_CONF & CTRL1 registers after resume.
> 

Hi Shrirang,

I still suspect we have some hw issue (IoT Gateway) here (cc ST folks
to get more info) and anyway I guess the proposed patch will fix the issue
completely since OD and HL configuration is not restored properly.
Could you please double check HL/OD regs are properly preserved after resume
phase?

Regards,
Lorenzo

> Changes from v1:
> - Don't drop enabling the sensor during resume
> - Restore AV_CONF register along with CTRL1
>   (As demonstrated with i2c register dumps of hts221 captured on Dell
>   IoT Gateways 300x before suspend & after resume [1])
> 
> Fixes: ffebe74b7c95 (iio: humidity: hts221: avoid useless ODR reconfiguration)
> Signed-off-by: Shrirang Bagul <shrirang.bagul@canonical.com>
> 
> [v1] https://marc.info/?l=linux-iio&m=152506543000442&w=2
> [1] https://marc.info/?l=linux-iio&m=152534455701742&w=2
> 
> This patch is based on:
> git://git.kernel.org/pub/scm/linux/kernel/git/jic23/iio.git -b testing
> ---
>  drivers/iio/humidity/hts221_core.c | 31 +++++++++++++++++++++++++++++-
>  1 file changed, 30 insertions(+), 1 deletion(-)
> 
> diff --git a/drivers/iio/humidity/hts221_core.c b/drivers/iio/humidity/hts221_core.c
> index 166946d4978d..fc5599497f55 100644
> --- a/drivers/iio/humidity/hts221_core.c
> +++ b/drivers/iio/humidity/hts221_core.c
> @@ -656,14 +656,43 @@ static int __maybe_unused hts221_resume(struct device *dev)
>  {
>  	struct iio_dev *iio_dev = dev_get_drvdata(dev);
>  	struct hts221_hw *hw = iio_priv(iio_dev);
> +	const struct hts221_avg *avg;
> +	u8 data, idx;
>  	int err = 0;
>  
> +	/* Restore contents of AV_CONF (RH & TEMP. oversampling ratio's) */
> +	avg = &hts221_avg_list[HTS221_SENSOR_H];
> +	idx = hw->sensors[HTS221_SENSOR_H].cur_avg_idx;
> +	data = avg->avg_avl[idx];
> +	err = hts221_update_avg(hw, HTS221_SENSOR_H, data);
> +	if (err < 0)
> +		goto fail_err;
> +
> +	avg = &hts221_avg_list[HTS221_SENSOR_T];
> +	idx = hw->sensors[HTS221_SENSOR_T].cur_avg_idx;
> +	data = avg->avg_avl[idx];
> +	err = hts221_update_avg(hw, HTS221_SENSOR_T, data);
> +	if (err < 0)
> +		goto fail_err;

just return err, no need for goto statement

> +
> +	/* Restore contents of CTRL1 (BDU & ODR) */
> +	err = regmap_update_bits(hw->regmap, HTS221_REG_CNTRL1_ADDR,
> +				 HTS221_BDU_MASK,
> +				 FIELD_PREP(HTS221_BDU_MASK, 1));
> +	if (err < 0)
> +		goto fail_err;
> +
> +	err = hts221_update_odr(hw, hw->odr);
> +	if (err < 0)
> +		goto fail_err;
> +

just return err, no need for goto statement

>  	if (hw->enabled)
>  		err = regmap_update_bits(hw->regmap, HTS221_REG_CNTRL1_ADDR,
>  					 HTS221_ENABLE_MASK,
>  					 FIELD_PREP(HTS221_ENABLE_MASK,
>  						    true));
> -	return err;
> +fail_err:
> +	return err < 0 ? err : 0;
>  }
>  
>  const struct dev_pm_ops hts221_pm_ops = {
> -- 
> 2.17.1
> 

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

* Re: [PATCH v2] iio: humidity: hts221: Fix sensor reads after resume
  2018-06-28  9:05 ` Lorenzo Bianconi
@ 2018-07-01  4:37   ` Shrirang Bagul
  0 siblings, 0 replies; 3+ messages in thread
From: Shrirang Bagul @ 2018-07-01  4:37 UTC (permalink / raw)
  To: Lorenzo Bianconi
  Cc: jic23, lorenzo.bianconi83, linux-iio, linux-kernel, mario.tesi,
	marco.bianco, armando.visconti

[-- Attachment #1: Type: text/plain, Size: 3698 bytes --]

On Thu, 2018-06-28 at 11:05 +0200, Lorenzo Bianconi wrote:
> > AV_CONF register (RH & TEMP. oversampling ratio's) and CTRL1 register
> > (ODR & BDU settings) values are lost after suspend.
> > 
> > While the change in AV_CONF updates the sensor resolution modes
> > (overriding the user configuration before the device went to suspend);
> > loss of the contents of the CTRL1 register leads to failure in reading
> > sensor output.
> > 
> > This patch restores the AV_CONF & CTRL1 registers after resume.
> > 
> 
> Hi Shrirang,
> 
> I still suspect we have some hw issue (IoT Gateway) here (cc ST folks
> to get more info) and anyway I guess the proposed patch will fix the issue
> completely since OD and HL configuration is not restored properly.
> Could you please double check HL/OD regs are properly preserved after resume
> phase?
Yes, certainly. Will include the i2c register dumps before and after the resume to support
the patch.

Regards,
Shrirang
> 
> Regards,
> Lorenzo
> 
> > Changes from v1:
> > - Don't drop enabling the sensor during resume
> > - Restore AV_CONF register along with CTRL1
> >   (As demonstrated with i2c register dumps of hts221 captured on Dell
> >   IoT Gateways 300x before suspend & after resume [1])
> > 
> > Fixes: ffebe74b7c95 (iio: humidity: hts221: avoid useless ODR reconfiguration)
> > Signed-off-by: Shrirang Bagul <shrirang.bagul@canonical.com>
> > 
> > [v1] https://marc.info/?l=linux-iio&m=152506543000442&w=2
> > [1] https://marc.info/?l=linux-iio&m=152534455701742&w=2
> > 
> > This patch is based on:
> > git://git.kernel.org/pub/scm/linux/kernel/git/jic23/iio.git -b testing
> > ---
> >  drivers/iio/humidity/hts221_core.c | 31 +++++++++++++++++++++++++++++-
> >  1 file changed, 30 insertions(+), 1 deletion(-)
> > 
> > diff --git a/drivers/iio/humidity/hts221_core.c b/drivers/iio/humidity/hts221_core.c
> > index 166946d4978d..fc5599497f55 100644
> > --- a/drivers/iio/humidity/hts221_core.c
> > +++ b/drivers/iio/humidity/hts221_core.c
> > @@ -656,14 +656,43 @@ static int __maybe_unused hts221_resume(struct device *dev)
> >  {
> >  	struct iio_dev *iio_dev = dev_get_drvdata(dev);
> >  	struct hts221_hw *hw = iio_priv(iio_dev);
> > +	const struct hts221_avg *avg;
> > +	u8 data, idx;
> >  	int err = 0;
> >  
> > +	/* Restore contents of AV_CONF (RH & TEMP. oversampling ratio's) */
> > +	avg = &hts221_avg_list[HTS221_SENSOR_H];
> > +	idx = hw->sensors[HTS221_SENSOR_H].cur_avg_idx;
> > +	data = avg->avg_avl[idx];
> > +	err = hts221_update_avg(hw, HTS221_SENSOR_H, data);
> > +	if (err < 0)
> > +		goto fail_err;
> > +
> > +	avg = &hts221_avg_list[HTS221_SENSOR_T];
> > +	idx = hw->sensors[HTS221_SENSOR_T].cur_avg_idx;
> > +	data = avg->avg_avl[idx];
> > +	err = hts221_update_avg(hw, HTS221_SENSOR_T, data);
> > +	if (err < 0)
> > +		goto fail_err;
> 
> just return err, no need for goto statement
Okay.
> 
> > +
> > +	/* Restore contents of CTRL1 (BDU & ODR) */
> > +	err = regmap_update_bits(hw->regmap, HTS221_REG_CNTRL1_ADDR,
> > +				 HTS221_BDU_MASK,
> > +				 FIELD_PREP(HTS221_BDU_MASK, 1));
> > +	if (err < 0)
> > +		goto fail_err;
> > +
> > +	err = hts221_update_odr(hw, hw->odr);
> > +	if (err < 0)
> > +		goto fail_err;
> > +
> 
> just return err, no need for goto statement
Okay.
> 
> >  	if (hw->enabled)
> >  		err = regmap_update_bits(hw->regmap, HTS221_REG_CNTRL1_ADDR,
> >  					 HTS221_ENABLE_MASK,
> >  					 FIELD_PREP(HTS221_ENABLE_MASK,
> >  						    true));
> > -	return err;
> > +fail_err:
> > +	return err < 0 ? err : 0;
> >  }
> >  
> >  const struct dev_pm_ops hts221_pm_ops = {
> > -- 
> > 2.17.1
> > 

[-- Attachment #2: This is a digitally signed message part --]
[-- Type: application/pgp-signature, Size: 833 bytes --]

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

end of thread, other threads:[~2018-07-01  4:37 UTC | newest]

Thread overview: 3+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2018-06-28  8:22 [PATCH v2] iio: humidity: hts221: Fix sensor reads after resume Shrirang Bagul
2018-06-28  9:05 ` Lorenzo Bianconi
2018-07-01  4:37   ` Shrirang Bagul

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