* [PATCH][for iio-for-4.17b] iio: humidity: hts221: Fix sensor reads after resume
@ 2018-04-30 5:16 Shrirang Bagul
2018-04-30 7:48 ` Lorenzo Bianconi
0 siblings, 1 reply; 9+ messages in thread
From: Shrirang Bagul @ 2018-04-30 5:16 UTC (permalink / raw)
To: jic23; +Cc: lorenzo.bianconi83, linux-iio
CTRL1 register (ODR & BDU settings) gets reset after system comes back
from suspend, causing subsequent reads from the sensor to fail.
This patch restores the CTRL1 register after resume.
Based on:
git://git.kernel.org/pub/scm/linux/kernel/git/jic23/iio.git iio-for-4.17b
Fixes: ffebe74b7c95 (iio: humidity: hts221: avoid useless ODR reconfiguration)
Signed-off-by: Shrirang Bagul <shrirang.bagul@canonical.com>
---
drivers/iio/humidity/hts221_core.c | 13 +++++++------
1 file changed, 7 insertions(+), 6 deletions(-)
diff --git a/drivers/iio/humidity/hts221_core.c b/drivers/iio/humidity/hts221_core.c
index 166946d4978d..5d7a652aa6d9 100644
--- a/drivers/iio/humidity/hts221_core.c
+++ b/drivers/iio/humidity/hts221_core.c
@@ -658,12 +658,13 @@ static int __maybe_unused hts221_resume(struct device *dev)
struct hts221_hw *hw = iio_priv(iio_dev);
int err = 0;
- if (hw->enabled)
- err = regmap_update_bits(hw->regmap, HTS221_REG_CNTRL1_ADDR,
- HTS221_ENABLE_MASK,
- FIELD_PREP(HTS221_ENABLE_MASK,
- true));
- return err;
+ err = regmap_update_bits(hw->regmap, HTS221_REG_CNTRL1_ADDR,
+ HTS221_BDU_MASK,
+ FIELD_PREP(HTS221_BDU_MASK, 1));
+ if (err < 0)
+ return err;
+
+ return hts221_update_odr(hw, hw->odr);
}
const struct dev_pm_ops hts221_pm_ops = {
--
2.14.1
^ permalink raw reply related [flat|nested] 9+ messages in thread
* Re: [PATCH][for iio-for-4.17b] iio: humidity: hts221: Fix sensor reads after resume
2018-04-30 5:16 [PATCH][for iio-for-4.17b] iio: humidity: hts221: Fix sensor reads after resume Shrirang Bagul
@ 2018-04-30 7:48 ` Lorenzo Bianconi
2018-04-30 8:39 ` Shrirang Bagul
2018-04-30 8:49 ` Shrirang Bagul
0 siblings, 2 replies; 9+ messages in thread
From: Lorenzo Bianconi @ 2018-04-30 7:48 UTC (permalink / raw)
To: Shrirang Bagul; +Cc: Jonathan Cameron, linux-iio
> CTRL1 register (ODR & BDU settings) gets reset after system comes back
> from suspend, causing subsequent reads from the sensor to fail.
>
> This patch restores the CTRL1 register after resume.
>
Hi Shrirang,
is just CTRL1 reset after suspend/resume or also other registers? are
IIO triggers enabled before the suspend or
are you reading data from sysfs?
> Based on:
> git://git.kernel.org/pub/scm/linux/kernel/git/jic23/iio.git iio-for-4.17b
>
> Fixes: ffebe74b7c95 (iio: humidity: hts221: avoid useless ODR reconfiguration)
> Signed-off-by: Shrirang Bagul <shrirang.bagul@canonical.com>
> ---
> drivers/iio/humidity/hts221_core.c | 13 +++++++------
> 1 file changed, 7 insertions(+), 6 deletions(-)
>
> diff --git a/drivers/iio/humidity/hts221_core.c b/drivers/iio/humidity/hts221_core.c
> index 166946d4978d..5d7a652aa6d9 100644
> --- a/drivers/iio/humidity/hts221_core.c
> +++ b/drivers/iio/humidity/hts221_core.c
> @@ -658,12 +658,13 @@ static int __maybe_unused hts221_resume(struct device *dev)
> struct hts221_hw *hw = iio_priv(iio_dev);
> int err = 0;
>
> - if (hw->enabled)
> - err = regmap_update_bits(hw->regmap, HTS221_REG_CNTRL1_ADDR,
> - HTS221_ENABLE_MASK,
> - FIELD_PREP(HTS221_ENABLE_MASK,
> - true));
> - return err;
> + err = regmap_update_bits(hw->regmap, HTS221_REG_CNTRL1_ADDR,
> + HTS221_BDU_MASK,
> + FIELD_PREP(HTS221_BDU_MASK, 1));
> + if (err < 0)
> + return err;
> +
> + return hts221_update_odr(hw, hw->odr);
> }
>
Here you need to re-enable the sensor if it was running before the
suspend operation.
Regards,
Lorenzo
> const struct dev_pm_ops hts221_pm_ops = {
> --
> 2.14.1
>
--
UNIX is Sexy: who | grep -i blonde | talk; cd ~; wine; talk; touch;
unzip; touch; strip; gasp; finger; gasp; mount; fsck; more; yes; gasp;
umount; make clean; sleep
^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [PATCH][for iio-for-4.17b] iio: humidity: hts221: Fix sensor reads after resume
2018-04-30 7:48 ` Lorenzo Bianconi
@ 2018-04-30 8:39 ` Shrirang Bagul
2018-04-30 8:49 ` Shrirang Bagul
1 sibling, 0 replies; 9+ messages in thread
From: Shrirang Bagul @ 2018-04-30 8:39 UTC (permalink / raw)
To: Lorenzo Bianconi; +Cc: Jonathan Cameron, linux-iio
[-- Attachment #1: Type: text/plain, Size: 2327 bytes --]
Hi Lorenzo,
On Mon, 2018-04-30 at 09:48 +0200, Lorenzo Bianconi wrote:
> > CTRL1 register (ODR & BDU settings) gets reset after system comes back
> > from suspend, causing subsequent reads from the sensor to fail.
> >
> > This patch restores the CTRL1 register after resume.
> >
>
> Hi Shrirang,
>
> is just CTRL1 reset after suspend/resume or also other registers? are
I have checked, contents of other registers survive the suspend/resume
(both suspend-to-ram & suspend-to-disk)cycle. It's only CTRL1 which resets.
> IIO triggers enabled before the suspend or
No IIO triggers are enabled.
> are you reading data from sysfs?
Yes.
>
> > Based on:
> > git://git.kernel.org/pub/scm/linux/kernel/git/jic23/iio.git iio-for-4.17b
> >
> > Fixes: ffebe74b7c95 (iio: humidity: hts221: avoid useless ODR reconfiguration)
> > Signed-off-by: Shrirang Bagul <shrirang.bagul@canonical.com>
> > ---
> > drivers/iio/humidity/hts221_core.c | 13 +++++++------
> > 1 file changed, 7 insertions(+), 6 deletions(-)
> >
> > diff --git a/drivers/iio/humidity/hts221_core.c b/drivers/iio/humidity/hts221_core.c
> > index 166946d4978d..5d7a652aa6d9 100644
> > --- a/drivers/iio/humidity/hts221_core.c
> > +++ b/drivers/iio/humidity/hts221_core.c
> > @@ -658,12 +658,13 @@ static int __maybe_unused hts221_resume(struct device *dev)
> > struct hts221_hw *hw = iio_priv(iio_dev);
> > int err = 0;
> >
> > - if (hw->enabled)
> > - err = regmap_update_bits(hw->regmap, HTS221_REG_CNTRL1_ADDR,
> > - HTS221_ENABLE_MASK,
> > - FIELD_PREP(HTS221_ENABLE_MASK,
> > - true));
> > - return err;
> > + err = regmap_update_bits(hw->regmap, HTS221_REG_CNTRL1_ADDR,
> > + HTS221_BDU_MASK,
> > + FIELD_PREP(HTS221_BDU_MASK, 1));
> > + if (err < 0)
> > + return err;
> > +
> > + return hts221_update_odr(hw, hw->odr);
> > }
> >
>
> Here you need to re-enable the sensor if it was running before the
> suspend operation.
> Regards,
>
> Lorenzo
>
> > const struct dev_pm_ops hts221_pm_ops = {
> > --
> > 2.14.1
> >
>
>
>
[-- Attachment #2: This is a digitally signed message part --]
[-- Type: application/pgp-signature, Size: 801 bytes --]
^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [PATCH][for iio-for-4.17b] iio: humidity: hts221: Fix sensor reads after resume
2018-04-30 7:48 ` Lorenzo Bianconi
2018-04-30 8:39 ` Shrirang Bagul
@ 2018-04-30 8:49 ` Shrirang Bagul
2018-04-30 9:05 ` Lorenzo Bianconi
1 sibling, 1 reply; 9+ messages in thread
From: Shrirang Bagul @ 2018-04-30 8:49 UTC (permalink / raw)
To: Lorenzo Bianconi; +Cc: Jonathan Cameron, linux-iio
[-- Attachment #1: Type: text/plain, Size: 2595 bytes --]
On Mon, 2018-04-30 at 09:48 +0200, Lorenzo Bianconi wrote:
> > CTRL1 register (ODR & BDU settings) gets reset after system comes back
> > from suspend, causing subsequent reads from the sensor to fail.
> >
> > This patch restores the CTRL1 register after resume.
> >
>
> Hi Shrirang,
>
> is just CTRL1 reset after suspend/resume or also other registers? are
> IIO triggers enabled before the suspend or
> are you reading data from sysfs?
>
> > Based on:
> > git://git.kernel.org/pub/scm/linux/kernel/git/jic23/iio.git iio-for-4.17b
> >
> > Fixes: ffebe74b7c95 (iio: humidity: hts221: avoid useless ODR reconfiguration)
> > Signed-off-by: Shrirang Bagul <shrirang.bagul@canonical.com>
> > ---
> > drivers/iio/humidity/hts221_core.c | 13 +++++++------
> > 1 file changed, 7 insertions(+), 6 deletions(-)
> >
> > diff --git a/drivers/iio/humidity/hts221_core.c b/drivers/iio/humidity/hts221_core.c
> > index 166946d4978d..5d7a652aa6d9 100644
> > --- a/drivers/iio/humidity/hts221_core.c
> > +++ b/drivers/iio/humidity/hts221_core.c
> > @@ -658,12 +658,13 @@ static int __maybe_unused hts221_resume(struct device *dev)
> > struct hts221_hw *hw = iio_priv(iio_dev);
> > int err = 0;
> >
> > - if (hw->enabled)
> > - err = regmap_update_bits(hw->regmap, HTS221_REG_CNTRL1_ADDR,
> > - HTS221_ENABLE_MASK,
> > - FIELD_PREP(HTS221_ENABLE_MASK,
> > - true));
> > - return err;
> > + err = regmap_update_bits(hw->regmap, HTS221_REG_CNTRL1_ADDR,
> > + HTS221_BDU_MASK,
> > + FIELD_PREP(HTS221_BDU_MASK, 1));
> > + if (err < 0)
> > + return err;
> > +
> > + return hts221_update_odr(hw, hw->odr);
> > }
> >
No need to re-enable the sensor here. hts221_read_oneshot() [called from hts221_read_raw()] does that every time before read:
static int hts221_read_oneshot(struct hts221_hw *hw, u8 addr, int *val)
{
__le16 data;
int err;
err = hts221_set_enable(hw, true);
if (err < 0)
return err;
msleep(50);
err = regmap_bulk_read(hw->regmap, addr, &data, sizeof(data));
if (err < 0)
return err;
hts221_set_enable(hw, false);
Regards,
Shrirang
>
> Here you need to re-enable the sensor if it was running before the
> suspend operation.
> Regards,
>
> Lorenzo
>
> > const struct dev_pm_ops hts221_pm_ops = {
> > --
> > 2.14.1
> >
>
>
>
[-- Attachment #2: This is a digitally signed message part --]
[-- Type: application/pgp-signature, Size: 801 bytes --]
^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [PATCH][for iio-for-4.17b] iio: humidity: hts221: Fix sensor reads after resume
2018-04-30 8:49 ` Shrirang Bagul
@ 2018-04-30 9:05 ` Lorenzo Bianconi
2018-04-30 9:12 ` Shrirang Bagul
0 siblings, 1 reply; 9+ messages in thread
From: Lorenzo Bianconi @ 2018-04-30 9:05 UTC (permalink / raw)
To: Shrirang Bagul; +Cc: Jonathan Cameron, linux-iio
> On Mon, 2018-04-30 at 09:48 +0200, Lorenzo Bianconi wrote:
>> > CTRL1 register (ODR & BDU settings) gets reset after system comes back
>> > from suspend, causing subsequent reads from the sensor to fail.
>> >
>> > This patch restores the CTRL1 register after resume.
>> >
>>
>> Hi Shrirang,
>>
>> is just CTRL1 reset after suspend/resume or also other registers? are
>> IIO triggers enabled before the suspend or
>> are you reading data from sysfs?
>>
>> > Based on:
>> > git://git.kernel.org/pub/scm/linux/kernel/git/jic23/iio.git iio-for-4.17b
>> >
>> > Fixes: ffebe74b7c95 (iio: humidity: hts221: avoid useless ODR reconfiguration)
>> > Signed-off-by: Shrirang Bagul <shrirang.bagul@canonical.com>
>> > ---
>> > drivers/iio/humidity/hts221_core.c | 13 +++++++------
>> > 1 file changed, 7 insertions(+), 6 deletions(-)
>> >
>> > diff --git a/drivers/iio/humidity/hts221_core.c b/drivers/iio/humidity/hts221_core.c
>> > index 166946d4978d..5d7a652aa6d9 100644
>> > --- a/drivers/iio/humidity/hts221_core.c
>> > +++ b/drivers/iio/humidity/hts221_core.c
>> > @@ -658,12 +658,13 @@ static int __maybe_unused hts221_resume(struct device *dev)
>> > struct hts221_hw *hw = iio_priv(iio_dev);
>> > int err = 0;
>> >
>> > - if (hw->enabled)
>> > - err = regmap_update_bits(hw->regmap, HTS221_REG_CNTRL1_ADDR,
>> > - HTS221_ENABLE_MASK,
>> > - FIELD_PREP(HTS221_ENABLE_MASK,
>> > - true));
>> > - return err;
>> > + err = regmap_update_bits(hw->regmap, HTS221_REG_CNTRL1_ADDR,
>> > + HTS221_BDU_MASK,
>> > + FIELD_PREP(HTS221_BDU_MASK, 1));
>> > + if (err < 0)
>> > + return err;
>> > +
>> > + return hts221_update_odr(hw, hw->odr);
>> > }
>> >
> No need to re-enable the sensor here. hts221_read_oneshot() [called from hts221_read_raw()] does that every time before read:
>
Could please try to change other registers (e.g. rh or temp gain) and
double check values are properly configured resuming the sensor?
It is necessary to re-enable the sensor if you use IIO triggers and
the sensor was enabled before the suspend operation
> static int hts221_read_oneshot(struct hts221_hw *hw, u8 addr, int *val)
> {
> __le16 data;
> int err;
>
> err = hts221_set_enable(hw, true);
> if (err < 0)
> return err;
>
> msleep(50);
>
> err = regmap_bulk_read(hw->regmap, addr, &data, sizeof(data));
> if (err < 0)
> return err;
>
> hts221_set_enable(hw, false);
>
> Regards,
> Shrirang
>>
>> Here you need to re-enable the sensor if it was running before the
>> suspend operation.
>> Regards,
>>
>> Lorenzo
>>
>> > const struct dev_pm_ops hts221_pm_ops = {
>> > --
>> > 2.14.1
>> >
>>
>>
>>
--
UNIX is Sexy: who | grep -i blonde | talk; cd ~; wine; talk; touch;
unzip; touch; strip; gasp; finger; gasp; mount; fsck; more; yes; gasp;
umount; make clean; sleep
^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [PATCH][for iio-for-4.17b] iio: humidity: hts221: Fix sensor reads after resume
2018-04-30 9:05 ` Lorenzo Bianconi
@ 2018-04-30 9:12 ` Shrirang Bagul
2018-05-03 7:39 ` Shrirang Bagul
0 siblings, 1 reply; 9+ messages in thread
From: Shrirang Bagul @ 2018-04-30 9:12 UTC (permalink / raw)
To: Lorenzo Bianconi; +Cc: Jonathan Cameron, linux-iio
[-- Attachment #1: Type: text/plain, Size: 3408 bytes --]
On Mon, 2018-04-30 at 11:05 +0200, Lorenzo Bianconi wrote:
> > On Mon, 2018-04-30 at 09:48 +0200, Lorenzo Bianconi wrote:
> > > > CTRL1 register (ODR & BDU settings) gets reset after system comes back
> > > > from suspend, causing subsequent reads from the sensor to fail.
> > > >
> > > > This patch restores the CTRL1 register after resume.
> > > >
> > >
> > > Hi Shrirang,
> > >
> > > is just CTRL1 reset after suspend/resume or also other registers? are
> > > IIO triggers enabled before the suspend or
> > > are you reading data from sysfs?
> > >
> > > > Based on:
> > > > git://git.kernel.org/pub/scm/linux/kernel/git/jic23/iio.git iio-for-4.17b
> > > >
> > > > Fixes: ffebe74b7c95 (iio: humidity: hts221: avoid useless ODR reconfiguration)
> > > > Signed-off-by: Shrirang Bagul <shrirang.bagul@canonical.com>
> > > > ---
> > > > drivers/iio/humidity/hts221_core.c | 13 +++++++------
> > > > 1 file changed, 7 insertions(+), 6 deletions(-)
> > > >
> > > > diff --git a/drivers/iio/humidity/hts221_core.c b/drivers/iio/humidity/hts221_core.c
> > > > index 166946d4978d..5d7a652aa6d9 100644
> > > > --- a/drivers/iio/humidity/hts221_core.c
> > > > +++ b/drivers/iio/humidity/hts221_core.c
> > > > @@ -658,12 +658,13 @@ static int __maybe_unused hts221_resume(struct device *dev)
> > > > struct hts221_hw *hw = iio_priv(iio_dev);
> > > > int err = 0;
> > > >
> > > > - if (hw->enabled)
> > > > - err = regmap_update_bits(hw->regmap, HTS221_REG_CNTRL1_ADDR,
> > > > - HTS221_ENABLE_MASK,
> > > > - FIELD_PREP(HTS221_ENABLE_MASK,
> > > > - true));
> > > > - return err;
> > > > + err = regmap_update_bits(hw->regmap, HTS221_REG_CNTRL1_ADDR,
> > > > + HTS221_BDU_MASK,
> > > > + FIELD_PREP(HTS221_BDU_MASK, 1));
> > > > + if (err < 0)
> > > > + return err;
> > > > +
> > > > + return hts221_update_odr(hw, hw->odr);
> > > > }
> > > >
> >
> > No need to re-enable the sensor here. hts221_read_oneshot() [called from hts221_read_raw()] does that every time before read:
> >
>
> Could please try to change other registers (e.g. rh or temp gain) and
> double check values are properly configured resuming the sensor?
> It is necessary to re-enable the sensor if you use IIO triggers and
> the sensor was enabled before the suspend operation
Sure. I'll do some more testing and share the results.
>
> > static int hts221_read_oneshot(struct hts221_hw *hw, u8 addr, int *val)
> > {
> > __le16 data;
> > int err;
> >
> > err = hts221_set_enable(hw, true);
> > if (err < 0)
> > return err;
> >
> > msleep(50);
> >
> > err = regmap_bulk_read(hw->regmap, addr, &data, sizeof(data));
> > if (err < 0)
> > return err;
> >
> > hts221_set_enable(hw, false);
> >
> > Regards,
> > Shrirang
> > >
> > > Here you need to re-enable the sensor if it was running before the
> > > suspend operation.
> > > Regards,
> > >
> > > Lorenzo
> > >
> > > > const struct dev_pm_ops hts221_pm_ops = {
> > > > --
> > > > 2.14.1
> > > >
> > >
> > >
> > >
>
>
>
[-- Attachment #2: This is a digitally signed message part --]
[-- Type: application/pgp-signature, Size: 801 bytes --]
^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [PATCH][for iio-for-4.17b] iio: humidity: hts221: Fix sensor reads after resume
2018-04-30 9:12 ` Shrirang Bagul
@ 2018-05-03 7:39 ` Shrirang Bagul
2018-05-03 9:30 ` Lorenzo Bianconi
0 siblings, 1 reply; 9+ messages in thread
From: Shrirang Bagul @ 2018-05-03 7:39 UTC (permalink / raw)
To: Lorenzo Bianconi; +Cc: linux-iio
[-- Attachment #1: Type: text/plain, Size: 4164 bytes --]
On Mon, 2018-04-30 at 17:12 +0800, Shrirang Bagul wrote:
> On Mon, 2018-04-30 at 11:05 +0200, Lorenzo Bianconi wrote:
> > > On Mon, 2018-04-30 at 09:48 +0200, Lorenzo Bianconi wrote:
> > > > > CTRL1 register (ODR & BDU settings) gets reset after system comes back
> > > > > from suspend, causing subsequent reads from the sensor to fail.
> > > > >
> > > > > This patch restores the CTRL1 register after resume.
> > > > >
> > > >
> > > > Hi Shrirang,
> > > >
> > > > is just CTRL1 reset after suspend/resume or also other registers? are
> > > > IIO triggers enabled before the suspend or
> > > > are you reading data from sysfs?
> > > >
> > > > > Based on:
> > > > > git://git.kernel.org/pub/scm/linux/kernel/git/jic23/iio.git iio-for-4.17b
> > > > >
> > > > > Fixes: ffebe74b7c95 (iio: humidity: hts221: avoid useless ODR reconfiguration)
> > > > > Signed-off-by: Shrirang Bagul <shrirang.bagul@canonical.com>
> > > > > ---
> > > > > drivers/iio/humidity/hts221_core.c | 13 +++++++------
> > > > > 1 file changed, 7 insertions(+), 6 deletions(-)
> > > > >
> > > > > diff --git a/drivers/iio/humidity/hts221_core.c b/drivers/iio/humidity/hts221_core.c
> > > > > index 166946d4978d..5d7a652aa6d9 100644
> > > > > --- a/drivers/iio/humidity/hts221_core.c
> > > > > +++ b/drivers/iio/humidity/hts221_core.c
> > > > > @@ -658,12 +658,13 @@ static int __maybe_unused hts221_resume(struct device *dev)
> > > > > struct hts221_hw *hw = iio_priv(iio_dev);
> > > > > int err = 0;
> > > > >
> > > > > - if (hw->enabled)
> > > > > - err = regmap_update_bits(hw->regmap, HTS221_REG_CNTRL1_ADDR,
> > > > > - HTS221_ENABLE_MASK,
> > > > > - FIELD_PREP(HTS221_ENABLE_MASK,
> > > > > - true));
> > > > > - return err;
> > > > > + err = regmap_update_bits(hw->regmap, HTS221_REG_CNTRL1_ADDR,
> > > > > + HTS221_BDU_MASK,
> > > > > + FIELD_PREP(HTS221_BDU_MASK, 1));
> > > > > + if (err < 0)
> > > > > + return err;
> > > > > +
> > > > > + return hts221_update_odr(hw, hw->odr);
> > > > > }
> > > > >
> > >
> > > No need to re-enable the sensor here. hts221_read_oneshot() [called from hts221_read_raw()] does that every time before read:
> > >
> >
> > Could please try to change other registers (e.g. rh or temp gain) and
> > double check values are properly configured resuming the sensor?
Further testing revealed that the contents of AV_CONF register value gets
overwritten (set to 0x3F) every time system comes out of suspend.
> > It is necessary to re-enable the sensor if you use IIO triggers and
> > the sensor was enabled before the suspend operation
Yes, I understand this is required, will handle this and restoring the AV_CONF
register in the next version.
I've tried using sysfs_trig_iio to test reading the sensor output from devfs
(using iio_generic_buffer) but looks like this is not supported in the current
driver. HW interrupts cannot be used on my system.
Please suggest if there are other alternatives?
>
> Sure. I'll do some more testing and share the results.
> >
> > > static int hts221_read_oneshot(struct hts221_hw *hw, u8 addr, int *val)
> > > {
> > > __le16 data;
> > > int err;
> > >
> > > err = hts221_set_enable(hw, true);
> > > if (err < 0)
> > > return err;
> > >
> > > msleep(50);
> > >
> > > err = regmap_bulk_read(hw->regmap, addr, &data, sizeof(data));
> > > if (err < 0)
> > > return err;
> > >
> > > hts221_set_enable(hw, false);
> > >
> > > Regards,
> > > Shrirang
> > > >
> > > > Here you need to re-enable the sensor if it was running before the
> > > > suspend operation.
> > > > Regards,
> > > >
> > > > Lorenzo
> > > >
> > > > > const struct dev_pm_ops hts221_pm_ops = {
> > > > > --
> > > > > 2.14.1
> > > > >
> > > >
> > > >
> > > >
> >
> >
[-- Attachment #2: This is a digitally signed message part --]
[-- Type: application/pgp-signature, Size: 801 bytes --]
^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [PATCH][for iio-for-4.17b] iio: humidity: hts221: Fix sensor reads after resume
2018-05-03 7:39 ` Shrirang Bagul
@ 2018-05-03 9:30 ` Lorenzo Bianconi
2018-05-03 10:49 ` Shrirang Bagul
0 siblings, 1 reply; 9+ messages in thread
From: Lorenzo Bianconi @ 2018-05-03 9:30 UTC (permalink / raw)
To: Shrirang Bagul; +Cc: linux-iio
On Thu, May 3, 2018 at 9:39 AM, Shrirang Bagul
<shrirang.bagul@canonical.com> wrote:
> On Mon, 2018-04-30 at 17:12 +0800, Shrirang Bagul wrote:
>> On Mon, 2018-04-30 at 11:05 +0200, Lorenzo Bianconi wrote:
>> > > On Mon, 2018-04-30 at 09:48 +0200, Lorenzo Bianconi wrote:
>> > > > > CTRL1 register (ODR & BDU settings) gets reset after system comes back
>> > > > > from suspend, causing subsequent reads from the sensor to fail.
>> > > > >
>> > > > > This patch restores the CTRL1 register after resume.
>> > > > >
>> > > >
>> > > > Hi Shrirang,
>> > > >
>> > > > is just CTRL1 reset after suspend/resume or also other registers? are
>> > > > IIO triggers enabled before the suspend or
>> > > > are you reading data from sysfs?
>> > > >
>> > > > > Based on:
>> > > > > git://git.kernel.org/pub/scm/linux/kernel/git/jic23/iio.git iio-for-4.17b
>> > > > >
>> > > > > Fixes: ffebe74b7c95 (iio: humidity: hts221: avoid useless ODR reconfiguration)
>> > > > > Signed-off-by: Shrirang Bagul <shrirang.bagul@canonical.com>
>> > > > > ---
>> > > > > drivers/iio/humidity/hts221_core.c | 13 +++++++------
>> > > > > 1 file changed, 7 insertions(+), 6 deletions(-)
>> > > > >
>> > > > > diff --git a/drivers/iio/humidity/hts221_core.c b/drivers/iio/humidity/hts221_core.c
>> > > > > index 166946d4978d..5d7a652aa6d9 100644
>> > > > > --- a/drivers/iio/humidity/hts221_core.c
>> > > > > +++ b/drivers/iio/humidity/hts221_core.c
>> > > > > @@ -658,12 +658,13 @@ static int __maybe_unused hts221_resume(struct device *dev)
>> > > > > struct hts221_hw *hw = iio_priv(iio_dev);
>> > > > > int err = 0;
>> > > > >
>> > > > > - if (hw->enabled)
>> > > > > - err = regmap_update_bits(hw->regmap, HTS221_REG_CNTRL1_ADDR,
>> > > > > - HTS221_ENABLE_MASK,
>> > > > > - FIELD_PREP(HTS221_ENABLE_MASK,
>> > > > > - true));
>> > > > > - return err;
>> > > > > + err = regmap_update_bits(hw->regmap, HTS221_REG_CNTRL1_ADDR,
>> > > > > + HTS221_BDU_MASK,
>> > > > > + FIELD_PREP(HTS221_BDU_MASK, 1));
>> > > > > + if (err < 0)
>> > > > > + return err;
>> > > > > +
>> > > > > + return hts221_update_odr(hw, hw->odr);
>> > > > > }
>> > > > >
>> > >
>> > > No need to re-enable the sensor here. hts221_read_oneshot() [called from hts221_read_raw()] does that every time before read:
>> > >
>> >
>> > Could please try to change other registers (e.g. rh or temp gain) and
>> > double check values are properly configured resuming the sensor?
> Further testing revealed that the contents of AV_CONF register value gets
> overwritten (set to 0x3F) every time system comes out of suspend.
Do you mean register HTS221_REG_AVG_ADDR (0x10)? 0x3f means oversampling_ratio
is set to max allowed value. It is a little bit weird since I tested
the sensor with an oscilloscope
before and after the suspend/resume operations and it seemed to work properly.
Could you please double-check the sensor is properly powered-up during
the suspend phase
(VDD line)?
Regards,
Lorenzo
>> > It is necessary to re-enable the sensor if you use IIO triggers and
>> > the sensor was enabled before the suspend operation
> Yes, I understand this is required, will handle this and restoring the AV_CONF
> register in the next version.
>
> I've tried using sysfs_trig_iio to test reading the sensor output from devfs
> (using iio_generic_buffer) but looks like this is not supported in the current
> driver. HW interrupts cannot be used on my system.
> Please suggest if there are other alternatives?
>>
>> Sure. I'll do some more testing and share the results.
>
>> >
>> > > static int hts221_read_oneshot(struct hts221_hw *hw, u8 addr, int *val)
>> > > {
>> > > __le16 data;
>> > > int err;
>> > >
>> > > err = hts221_set_enable(hw, true);
>> > > if (err < 0)
>> > > return err;
>> > >
>> > > msleep(50);
>> > >
>> > > err = regmap_bulk_read(hw->regmap, addr, &data, sizeof(data));
>> > > if (err < 0)
>> > > return err;
>> > >
>> > > hts221_set_enable(hw, false);
>> > >
>> > > Regards,
>> > > Shrirang
>> > > >
>> > > > Here you need to re-enable the sensor if it was running before the
>> > > > suspend operation.
>> > > > Regards,
>> > > >
>> > > > Lorenzo
>> > > >
>> > > > > const struct dev_pm_ops hts221_pm_ops = {
>> > > > > --
>> > > > > 2.14.1
>> > > > >
>> > > >
>> > > >
>> > > >
>> >
>> >
--
UNIX is Sexy: who | grep -i blonde | talk; cd ~; wine; talk; touch;
unzip; touch; strip; gasp; finger; gasp; mount; fsck; more; yes; gasp;
umount; make clean; sleep
^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [PATCH][for iio-for-4.17b] iio: humidity: hts221: Fix sensor reads after resume
2018-05-03 9:30 ` Lorenzo Bianconi
@ 2018-05-03 10:49 ` Shrirang Bagul
0 siblings, 0 replies; 9+ messages in thread
From: Shrirang Bagul @ 2018-05-03 10:49 UTC (permalink / raw)
To: Lorenzo Bianconi; +Cc: linux-iio
[-- Attachment #1.1: Type: text/plain, Size: 5746 bytes --]
On Thu, 2018-05-03 at 11:30 +0200, Lorenzo Bianconi wrote:
> On Thu, May 3, 2018 at 9:39 AM, Shrirang Bagul
> <shrirang.bagul@canonical.com> wrote:
> > On Mon, 2018-04-30 at 17:12 +0800, Shrirang Bagul wrote:
> > > On Mon, 2018-04-30 at 11:05 +0200, Lorenzo Bianconi wrote:
> > > > > On Mon, 2018-04-30 at 09:48 +0200, Lorenzo Bianconi wrote:
> > > > > > > CTRL1 register (ODR & BDU settings) gets reset after system comes back
> > > > > > > from suspend, causing subsequent reads from the sensor to fail.
> > > > > > >
> > > > > > > This patch restores the CTRL1 register after resume.
> > > > > > >
> > > > > >
> > > > > > Hi Shrirang,
> > > > > >
> > > > > > is just CTRL1 reset after suspend/resume or also other registers? are
> > > > > > IIO triggers enabled before the suspend or
> > > > > > are you reading data from sysfs?
> > > > > >
> > > > > > > Based on:
> > > > > > > git://git.kernel.org/pub/scm/linux/kernel/git/jic23/iio.git iio-for-4.17b
> > > > > > >
> > > > > > > Fixes: ffebe74b7c95 (iio: humidity: hts221: avoid useless ODR reconfiguration)
> > > > > > > Signed-off-by: Shrirang Bagul <shrirang.bagul@canonical.com>
> > > > > > > ---
> > > > > > > drivers/iio/humidity/hts221_core.c | 13 +++++++------
> > > > > > > 1 file changed, 7 insertions(+), 6 deletions(-)
> > > > > > >
> > > > > > > diff --git a/drivers/iio/humidity/hts221_core.c b/drivers/iio/humidity/hts221_core.c
> > > > > > > index 166946d4978d..5d7a652aa6d9 100644
> > > > > > > --- a/drivers/iio/humidity/hts221_core.c
> > > > > > > +++ b/drivers/iio/humidity/hts221_core.c
> > > > > > > @@ -658,12 +658,13 @@ static int __maybe_unused hts221_resume(struct device *dev)
> > > > > > > struct hts221_hw *hw = iio_priv(iio_dev);
> > > > > > > int err = 0;
> > > > > > >
> > > > > > > - if (hw->enabled)
> > > > > > > - err = regmap_update_bits(hw->regmap, HTS221_REG_CNTRL1_ADDR,
> > > > > > > - HTS221_ENABLE_MASK,
> > > > > > > - FIELD_PREP(HTS221_ENABLE_MASK,
> > > > > > > - true));
> > > > > > > - return err;
> > > > > > > + err = regmap_update_bits(hw->regmap, HTS221_REG_CNTRL1_ADDR,
> > > > > > > + HTS221_BDU_MASK,
> > > > > > > + FIELD_PREP(HTS221_BDU_MASK, 1));
> > > > > > > + if (err < 0)
> > > > > > > + return err;
> > > > > > > +
> > > > > > > + return hts221_update_odr(hw, hw->odr);
> > > > > > > }
> > > > > > >
> > > > >
> > > > > No need to re-enable the sensor here. hts221_read_oneshot() [called from hts221_read_raw()] does that every time before read:
> > > > >
> > > >
> > > > Could please try to change other registers (e.g. rh or temp gain) and
> > > > double check values are properly configured resuming the sensor?
> >
> > Further testing revealed that the contents of AV_CONF register value gets
> > overwritten (set to 0x3F) every time system comes out of suspend.
>
> Do you mean register HTS221_REG_AVG_ADDR (0x10)? 0x3f means oversampling_ratio
Yes, HTS221_REG_AVG_ADDR (0x10) register which controls the sensor resolution modes.
> is set to max allowed value. It is a little bit weird since I tested
> the sensor with an oscilloscope
> before and after the suspend/resume operations and it seemed to work properly.
> Could you please double-check the sensor is properly powered-up during
> the suspend phase
> (VDD line)?
Unfortunately, I don't have the equipment required to test the hardware.
Your observation is correct, the minimum change required is restoring CTRL1 register after
suspend/resume to get the sensor reads working.
The discrepancy with HTS221_REG_AVG_ADDR (0x10) is that the HW register values (read
over i2c) are no longer in sync with the corresponding config. when compared with related fields
of hts221_hw struct (see attached console log with output from i2cdump).
Regards,
Shrirang.
>
> Regards,
> Lorenzo
>
> > > > It is necessary to re-enable the sensor if you use IIO triggers and
> > > > the sensor was enabled before the suspend operation
> >
> > Yes, I understand this is required, will handle this and restoring the AV_CONF
> > register in the next version.
> >
> > I've tried using sysfs_trig_iio to test reading the sensor output from devfs
> > (using iio_generic_buffer) but looks like this is not supported in the current
> > driver. HW interrupts cannot be used on my system.
> > Please suggest if there are other alternatives?
> > >
> > > Sure. I'll do some more testing and share the results.
> > > >
> > > > > static int hts221_read_oneshot(struct hts221_hw *hw, u8 addr, int *val)
> > > > > {
> > > > > __le16 data;
> > > > > int err;
> > > > >
> > > > > err = hts221_set_enable(hw, true);
> > > > > if (err < 0)
> > > > > return err;
> > > > >
> > > > > msleep(50);
> > > > >
> > > > > err = regmap_bulk_read(hw->regmap, addr, &data, sizeof(data));
> > > > > if (err < 0)
> > > > > return err;
> > > > >
> > > > > hts221_set_enable(hw, false);
> > > > >
> > > > > Regards,
> > > > > Shrirang
> > > > > >
> > > > > > Here you need to re-enable the sensor if it was running before the
> > > > > > suspend operation.
> > > > > > Regards,
> > > > > >
> > > > > > Lorenzo
> > > > > >
> > > > > > > const struct dev_pm_ops hts221_pm_ops = {
> > > > > > > --
> > > > > > > 2.14.1
> > > > > > >
> > > > > >
> > > > > >
> > > > > >
> > > >
> > > >
>
>
>
[-- Attachment #1.2: hts221_s3_fail.log --]
[-- Type: text/x-log, Size: 3605 bytes --]
root@caracalla:/sys/bus/iio/devices/iio:device0# cat in_*_raw
-2032
1023
root@caracalla:/sys/bus/iio/devices/iio:device0# cat in_*_raw
-2024
1022
root@caracalla:/sys/bus/iio/devices/iio:device0# /home/admin/i2c-tools-4.0/tools/i2cdump -f -y 0 0x5f b
0 1 2 3 4 5 6 7 8 9 a b c d e f 0123456789abcdef
00: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 bc ...............?
10: 1b 00 65 32 99 be 32 a2 9e b2 fb 00 e8 01 00 82 ?.e2??2????.??.?
20: 05 00 00 00 00 00 00 02 db f7 fe 03 c7 fb 16 04 ?......?????????
30: 37 85 a8 1b 00 c4 e8 ff 86 03 80 ce ff ff 19 03 7???.??.????..??
40: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 ................
50: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 ................
60: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 ................
70: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 ................
80: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 bc ...............?
90: 1b 00 65 32 99 be 32 a2 9e b2 fb 00 e8 01 00 82 ?.e2??2????.??.?
a0: 05 00 00 00 00 00 00 00 db f7 fe 03 c7 fb 16 04 ?.......????????
b0: 37 85 a8 1b 00 c4 e8 ff 86 03 80 ce ff ff 19 03 7???.??.????..??
c0: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 ................
d0: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 ................
e0: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 ................
f0: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 ................
root@caracalla:/sys/bus/iio/devices/iio:device0# rtcwake -a -v -s 40 -m mem
rtcwake: assuming RTC uses UTC ...
Using UTC time.
delta = 0
tzone = 0
tzname = UTC
systime = 1525343782, (UTC) Thu May 3 10:36:22 2018
rtctime = 1525343782, (UTC) Thu May 3 10:36:22 2018
alarm 0, sys_time 1525343782, rtc_time 1525343782, seconds 40
rtcwake: wakeup from "mem" using /dev/rtc0 at Thu May 3 10:37:03 2018
suspend mode: mem; suspending system
root@caracalla:/sys/bus/iio/devices/iio:device0# /home/admin/i2c-tools-4.0/tools/i2cdump -f -y 0 0x5f b
0 1 2 3 4 5 6 7 8 9 a b c d e f 0123456789abcdef
00: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 bc ...............?
10: 3f 00 65 32 99 be 32 a2 9e b2 fb 00 e8 01 80 82 ?.e2??2????.????
20: 00 00 00 00 00 00 00 03 7e f6 78 03 f4 f9 90 03 .......?~?x?????
30: 37 85 a8 1b 00 c4 e8 ff 86 03 80 ce ff ff 19 03 7???.??.????..??
40: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 ................
50: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 ................
60: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 ................
70: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 ................
80: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 bc ...............?
90: 3f 00 65 32 99 be 32 a2 9e b2 fb 00 e8 01 80 82 ?.e2??2????.????
a0: 00 00 00 00 00 00 00 00 7e f6 78 03 f4 f9 90 03 ........~?x?????
b0: 37 85 a8 1b 00 c4 e8 ff 86 03 80 ce ff ff 19 03 7???.??.????..??
c0: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 ................
d0: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 ................
e0: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 ................
f0: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 ................
root@caracalla:/sys/bus/iio/devices/iio:device0# cat in_*_raw
-2434
888
root@caracalla:/sys/bus/iio/devices/iio:device0# cat in_*_raw
-2434
888
root@caracalla:/sys/bus/iio/devices/iio:device0# cat in_humidityrelative_oversampling_ratio
32
root@caracalla:/sys/bus/iio/devices/iio:device0# cat in_temp_oversampling_ratio
16
[-- Attachment #2: This is a digitally signed message part --]
[-- Type: application/pgp-signature, Size: 801 bytes --]
^ permalink raw reply [flat|nested] 9+ messages in thread
end of thread, other threads:[~2018-05-03 10:49 UTC | newest]
Thread overview: 9+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2018-04-30 5:16 [PATCH][for iio-for-4.17b] iio: humidity: hts221: Fix sensor reads after resume Shrirang Bagul
2018-04-30 7:48 ` Lorenzo Bianconi
2018-04-30 8:39 ` Shrirang Bagul
2018-04-30 8:49 ` Shrirang Bagul
2018-04-30 9:05 ` Lorenzo Bianconi
2018-04-30 9:12 ` Shrirang Bagul
2018-05-03 7:39 ` Shrirang Bagul
2018-05-03 9:30 ` Lorenzo Bianconi
2018-05-03 10:49 ` Shrirang Bagul
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.