On Sun, 2019-12-01 at 21:11 +0000, Jonathan Cameron wrote: > On Fri, 29 Nov 2019 17:53:14 +0100 > Marco Felsch <m.felsch@xxxxxxxxxxxxxx> wrote: > > > The device is always in a low-power state due to the hardware design. > > It > > wakes up upon a conversion request and goes back into the low-power > > state. The pm ops are added to disable the device completely and to > > free > > the regulator. Disbaling the device completely should be not that > > notable but freeing the regulator is important. Because if it is a > > shared > > power-rail the regulator won't be disabled during suspend-to-ram/disk > > and so all devices connected to that rail keeps on. > > > > Signed-off-by: Marco Felsch <m.felsch@xxxxxxxxxxxxxx> > Looks fine to me, but I'd like to leave it a little longer for Alex > and anyone else who is interested to have time to take a look. > Reviewed-by: Alexandru Ardelean <alexandru.ardelean@xxxxxxxxxx> > Thanks, > > Jonathan > > > --- > > Hi, > > > > this v3 contains the comments made on [1]. > > > > [1] https://www.spinics.net/lists/linux-iio/msg48135.html > > > > Changelog: > > v3: > > - add resync error check during resume > > > > v2: > > - squash patch 2 & 3 > > - call regulator_disable() unconditional during suspend() > > - drop dev_err() messages during suspend > > - fix error path within resume() > > --- > > drivers/iio/adc/ad799x.c | 66 ++++++++++++++++++++++++++++++++++++---- > > 1 file changed, 60 insertions(+), 6 deletions(-) > > > > diff --git a/drivers/iio/adc/ad799x.c b/drivers/iio/adc/ad799x.c > > index f658012baad8..ef013af1aec0 100644 > > --- a/drivers/iio/adc/ad799x.c > > +++ b/drivers/iio/adc/ad799x.c > > @@ -167,6 +167,21 @@ static int ad799x_read_config(struct ad799x_state > > *st) > > } > > } > > > > +static int ad799x_update_config(struct ad799x_state *st, u16 config) > > +{ > > + int ret; > > + > > + ret = ad799x_write_config(st, config); > > + if (ret < 0) > > + return ret; > > + ret = ad799x_read_config(st); > > + if (ret < 0) > > + return ret; > > + st->config = ret; > > + > > + return 0; > > +} > > + > > /** > > * ad799x_trigger_handler() bh of trigger launched polling to ring > > buffer > > * > > @@ -808,13 +823,9 @@ static int ad799x_probe(struct i2c_client *client, > > indio_dev->channels = st->chip_config->channel; > > indio_dev->num_channels = chip_info->num_channels; > > > > - ret = ad799x_write_config(st, st->chip_config->default_config); > > - if (ret < 0) > > - goto error_disable_vref; > > - ret = ad799x_read_config(st); > > - if (ret < 0) > > + ret = ad799x_update_config(st, st->chip_config->default_config); > > + if (ret) > > goto error_disable_vref; > > - st->config = ret; > > > > ret = iio_triggered_buffer_setup(indio_dev, NULL, > > &ad799x_trigger_handler, NULL); > > @@ -864,6 +875,48 @@ static int ad799x_remove(struct i2c_client > > *client) > > return 0; > > } > > > > +static int __maybe_unused ad799x_suspend(struct device *dev) > > +{ > > + struct iio_dev *indio_dev = i2c_get_clientdata(to_i2c_client(dev)); > > + struct ad799x_state *st = iio_priv(indio_dev); > > + > > + regulator_disable(st->vref); > > + regulator_disable(st->reg); > > + > > + return 0; > > +} > > + > > +static int __maybe_unused ad799x_resume(struct device *dev) > > +{ > > + struct iio_dev *indio_dev = i2c_get_clientdata(to_i2c_client(dev)); > > + struct ad799x_state *st = iio_priv(indio_dev); > > + int ret; > > + > > + ret = regulator_enable(st->reg); > > + if (ret) { > > + dev_err(dev, "Unable to enable vcc regulator\n"); > > + return ret; > > + } > > + ret = regulator_enable(st->vref); > > + if (ret) { > > + regulator_disable(st->reg); > > + dev_err(dev, "Unable to enable vref regulator\n"); > > + return ret; > > + } > > + > > + /* resync config */ > > + ret = ad799x_update_config(st, st->config); > > + if (ret) { > > + regulator_disable(st->vref); > > + regulator_disable(st->reg); > > + return ret; > > + } > > + > > + return 0; > > +} > > + > > +static SIMPLE_DEV_PM_OPS(ad799x_pm_ops, ad799x_suspend, > > ad799x_resume); > > + > > static const struct i2c_device_id ad799x_id[] = { > > { "ad7991", ad7991 }, > > { "ad7995", ad7995 }, > > @@ -881,6 +934,7 @@ MODULE_DEVICE_TABLE(i2c, ad799x_id); > > static struct i2c_driver ad799x_driver = { > > .driver = { > > .name = "ad799x", > > + .pm = &ad799x_pm_ops, > > }, > > .probe = ad799x_probe, > > .remove = ad799x_remove,
