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