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@xxxxxxxxxx> > >>> > >>> 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@xxxxxxxxxxxxxxx > >>> Signed-off-by: Peter De Schrijver <pdeschrijver@xxxxxxxxxx> > >>> Signed-off-by: Joseph Lo <josephl@xxxxxxxxxx> > >>> --- > >>> .../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.
