Re: [PATCH 01/19] dt-bindings: clock: tegra124-dfll: Update DFLL binding for PWM regulator

Jon Hunter
2018-12-10T08:59:10Z

On 10/12/2018 08:49, Joseph Lo wrote:
> Hi Jon,
>
> Thanks for reviewing this series.
>
> On 12/7/18 9:41 PM, Jon Hunter wrote:
>>
>> On 04/12/2018 09:25, Joseph Lo wrote:
>>> From: Peter De Schrijver <pdeschrijver@nvidia.com>
>>>
>>> Add new properties to configure the DFLL PWM regulator support. Also
>>> add an example and make the I2C clock only required when I2C support is
>>> used.
>>>
>>> Cc: devicetree@vger.kernel.org
>>> Signed-off-by: Peter De Schrijver <pdeschrijver@nvidia.com>
>>> Signed-off-by: Joseph Lo <josephl@nvidia.com>
>>> ---
>>>   .../bindings/clock/nvidia,tegra124-dfll.txt   | 73 ++++++++++++++++++-
>>>   1 file changed, 71 insertions(+), 2 deletions(-)
>>>
>>> diff --git
>>> a/Documentation/devicetree/bindings/clock/nvidia,tegra124-dfll.txt
>>> b/Documentation/devicetree/bindings/clock/nvidia,tegra124-dfll.txt
>>> index dff236f524a7..8c97600d2bad 100644
>>> --- a/Documentation/devicetree/bindings/clock/nvidia,tegra124-dfll.txt
>>> +++ b/Documentation/devicetree/bindings/clock/nvidia,tegra124-dfll.txt
>>> @@ -8,7 +8,6 @@ the fast CPU cluster. It consists of a free-running
>>> voltage controlled
>>>   oscillator connected to the CPU voltage rail (VDD_CPU), and a
>>> closed loop
>>>   control module that will automatically adjust the VDD_CPU voltage by
>>>   communicating with an off-chip PMIC either via an I2C bus or via
>>> PWM signals.
>>> -Currently only the I2C mode is supported by these bindings.
>>>     Required properties:
>>>   - compatible : should be "nvidia,tegra124-dfll"
>>> @@ -45,10 +44,28 @@ Required properties for the control loop parameters:
>>>   Optional properties for the control loop parameters:
>>>   - nvidia,cg-scale: Boolean value, see the field
>>> DFLL_PARAMS_CG_SCALE in the TRM.
>>>   +Optional properties for mode selection:
>>> +- nvidia,pwm-to-pmic: Use PWM to control regulator rather then I2C.
>>> +
>>>   Required properties for I2C mode:
>>>   - nvidia,i2c-fs-rate: I2C transfer rate, if using full speed mode.
>>>   -Example:
>>> +Required properties for PWM mode:
>>> +- nvidia,pwm-period: period of PWM square wave in microseconds.
>>> +- nvidia,init-uv: Regulator voltage in micro volts when PWM control
>>> is disabled.
>>
>> Maybe consider 'pwm-inactive-voltage-microvolt'.
> Ah, I think I need to refine the description here. It should be
> something like below.
>  - nvidia,pwm-init-microvolt : Regulator voltage in micro volts when PWM
> control is initialized
>
> This is the initial voltage that when we just initialize the DFLL
> hardware for PWM output. And before we switch the CPU clock from PLLX to
> DFLL, we will enable DFLL hardware in closed loop mode which will aplly
> the DVFS table that was calculated from CVB table.
>
> The original description maybe make you think that it's the working
> voltage when it's under open-loop mode. But it's not. Sorry.
>
> When we working on open-loop mode which will switch to low voltage range
> which also follows the DVFS table. Not this one.

OK, but I am still not sure what this voltage is. I mean that I
understand it is the initial voltage, but how exactly do we define this
number? Where does it come from, how is this determined?

>>
>>> +- nvidia,align-offset-uv: Regulator voltage in micro volts when PWM
>>> control is
>>> +              enabled and PWM output is low.
>>
>> Would this be considered the minimum pwm active voltage?
> This would be used for minimum voltage for LUT table, which is the table
> that PMIC can output. The real minimum voltage in PWM mode still depends
> on the CVB table.
>
> So maybe change this one to 'nvidia,pwm-offset-uv'.

So is this the min supported by the PMIC? Maybe the name should reflect
that because the above name does not reflect this. Furthermore, if this
is a min then maybe the name should use 'min' as opposed to 'offset'.
for example, 'nvidia,pwm-pmic-min-microvolts'.

Does this need to be described in DT, can it not be queried from the PMIC?

Cheers
Jon

--
nvpublic

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org