[PATCH] arm: dts: fix rk3066a based boards vdd_log voltage initialization

[Doug Anderson]

Hi,

On Mon, Sep 19, 2016 at 1:43 PM, Boris Brezillon
<boris.brezillon at free-electrons.com> wrote:
> On Mon, 19 Sep 2016 11:12:12 -0700
> Doug Anderson <dianders at chromium.org> wrote:
>
>> Hi,
>>
>> On Mon, Sep 19, 2016 at 11:06 AM, Boris Brezillon
>> <boris.brezillon at free-electrons.com> wrote:
>> > On Mon, 19 Sep 2016 10:52:51 -0700
>> > Doug Anderson <dianders at chromium.org> wrote:
>> >
>> >> Hi,
>> >>
>> >> On Mon, Sep 19, 2016 at 10:48 AM, Boris Brezillon
>> >> <boris.brezillon at free-electrons.com> wrote:
>> >> > The PWM chip has always claimed the pins and muxed them to the PWM IP.
>> >> > So, this means it's broken from the beginning, and my patch is only
>> >> > uncovering the problem (unless the pins stay configured as input until
>> >> > the PWM is enabled, which I'm not sure is the case).
>> >>
>> >> Such a solution is achievable with the pinctrl APIs pretty easily.
>> >> You might not be able to use the automatic "init" state but you can do
>> >> something similar and switch to an "active" state once the PWM is
>> >> actually turned on the first time.
>> >
>> > But is it really the case here (I don't see any code requesting a
>> > specific pinmux depending on the PWM state)?
>>
>> It is not happening right now as far as I know.  ...but that's a bug.
>>
>> > Anyway, we really need to handle this case, we should define the
>> > typical voltage when the PWM is disabled. Same as what you suggested
>> > with voltage-when-input, but with a different naming (since the concept
>> > of pinmux is PWM hardware/driver specific).
>> >
>> >         voltage-when-pwm-disabled = <...>;
>>
>> Voltage when disabled and voltage when input are two different states.
>> A disabled PWM will typically either drive high or low (depending on
>> where it was when you turned it off).  Not all "disabled" states will
>> mean that the pin is configured as an input.
>
> Okay, after reading again your first answer, I think I understand why
> you want to differentiate the when-disabled and when-input cases. You
> want to use the "init" and "default" pinctrl states. The "init" state
> (applied at probe time) would keep the PWM pin in gpio+input mode and
> the "default" state (applied when the PWM device is enabled for the
> first time) would mux the pin to the PWM device.
> Your solution requires that the pwm-regulator device knows in which
> state the pin attached to the PWM is, which IMO is breaking the
> layering we have right now: a PWM-regulator is assigned a PWM device
> which is assigned a pin and a pinmux config.
> Another solution would be to expose an additional information in the
> pwm_state: whether the PWM is in the INIT state (probed, but not yet
> configured by its user) or DEFAULT state (probed and already
> configured by its user). But again, by doing that we also expose
> internal PWM details to its user, which I'm not sure will help keep
> the PWM API simple.

One note is that probably using the "init" state would not be a good
idea because it would make assumptions about what state the firmware
left things.  Possibly a firmware update could cause a change from a
PWM being left as "input" to the PWM actually being initted.
...really we should be able to detect if the PWM was left on at
bootup.


> Actually, I had something slightly different in mind. I thought about
> having two new pinctrl states ("enabled" and "disabled"). The "enabled"
> state (pin muxed to the PWM device) would be applied each time the PWM
> is enabled, and "disabled" state (gpio+input mode) would be applied each
> time the PWM is disabled.

Adding "enabled" and "disabled" state is sane IMHO.  At the moment the
PWM regulator never actually puts things in "disabled" state, but I
suppose it could in the future.

> This way we can guarantee that even when the PWM is disabled, the
> PWM-driven regulator is configured to output a non-destructive voltage.
> Hence my suggestion to name the property 'voltage-when-pwm-disabled'.

That's fine with me, as long as the PWM starts up as "enabled" if the
pinctrl happened to be left initted by the BIOS.

-Doug