On Mon, 2 Dec 2019 06:26:21 +0000 "Ardelean, Alexandru" <alexandru.Ardelean@xxxxxxxxxx> wrote: > 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. Applied to the togreg branch of iio.git and pushed out as testing for the autobuilders to play with it. thanks, Jonathan > > > 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,
