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