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

[Peter De Schrijver <pdeschrijver@nvidia.com>]

On Mon, Dec 10, 2018 at 08:59:10AM +0000, Jon Hunter wrote:
> 
> 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?
> 

It is set by a resistive divider on the board iirc.

> >>
> >>> +- 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?
> 

There is no interface to query anything from the OVR regulator.

Peter.

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel