Re: [PATCH v3] iio: adc: Add support for TI ADC108S102 and ADC128S102
From: Andy Shevchenko
Date: Fri May 05 2017 - 16:32:14 EST
On Fri, 2017-05-05 at 22:09 +0200, Jan Kiszka wrote:
> On 2017-05-05 20:52, Jonathan Cameron wrote:
> > On 05/05/17 11:39, Jan Kiszka wrote:
> > > On 2017-05-05 11:54, Andy Shevchenko wrote:
> > > > On Fri, 2017-05-05 at 08:31 +0200, Jan Kiszka wrote:
>>>>> + if (st->reg)
> > > > > + *val =
> > > > > regulator_get_voltage(st->reg)Â
> > > > > / 1000;
> > > > > + else
> > > > > + *val = st->va_millivolt;
> > > > > +
> > > >
> > > > Another way is to not just hard code the value, but create a
> > > > fixed
> > > > voltage regulator out of it. In this case you will have one way
> > > > to get
> > > > its value.
> > >
> > > That's a good idea.
> > Agreed. Make sure to cc Mark Brown though as I'll need an ack from
> > him
> > to have a fixed reg hiding in here.
> After diving deeper, it not longer appears to be a good idea:
> - pulls in a non-obvious requirement for CONFIG_REGULATOR on platforms
> Â that otherwise do not need it
Why is it a problem?
> - requires complex life-cycle management so that the fixed regulator
> Â instantiated on the first device creation and removed with the last
> Â one
Who cares if you register more than one?
> We better go with the static value assignment.
> I'll move that regulator_get_voltage into the probing function which
> will simplify things further (va_millivolt will carry the value for
Yes, it would be the way, if system has it's fixed.
But in this case you need to threat regulator as optional if we are
going to enable/disable them for PM.
Andy Shevchenko <andriy.shevchenko@xxxxxxxxxxxxxxx>
Intel Finland Oy