Hi Dmitry,
thanks for the quick review!
I fixed most stuff, just one minor comment:
On Tue, Mar 10, 2020 at 6:08 PM Dmitry Torokhov
<dmitry.torokhov@xxxxxxxxx> wrote:
> > + /* According to datasheet this should be in the 2.7-3.6 V range */
> > + ret = regulator_set_voltage(ts->vcpin, 2700000, 3600000);
> > + if (ret) {
> > + dev_err(dev, "failed to set VCPIN voltage\n");
> > + return ret;
> > + }
>
> Shouldn't this already be in DT? We typically do not configure voltage
> on various rail unless in very specific circumstances.
Certainly the DT defines the voltage range on the regulator
on the producer side.
This is the consumer range, and DT has no facility to put
restrictions on the consumer voltage window.
I think it is pretty natural to do in the code.
The typical usecase is when two components share the
same line, and in this case two voltage consumer inputs
*may* be connected to the same producer regulator.
The regulator framework will the infer that the producer
can produce a voltage that fulfil the constraints on all the
consumers.
The fact that few devices issue regulator_set_voltage()
is a combination of "good enough" and general sloppiness,
I think it should reflect the operation voltages of the component
and stop people from shooting themselves in the foot.
But I CC Mark on this and see what he says. (I might be wrong.)
Yours,
Linus Walleij
