| Message ID | 20180430051658.9856-1-shrirang.bagul@canonical.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
> 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 >
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 > > > > >
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 > > > > >
> 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 >> > >> >> >>
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 > > > > > > > > > > > > > > > >
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 > > > > > > > > > > > > > > > > > > > > >
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 >> > > > > >> > > > >> > > > >> > > > >> > >> >
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 > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > >
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 = {
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(-)