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

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



On 12/10/18 4:59 PM, 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?
IIRC, this is the same output voltage that bootloader configured with proper PLLX rate. So before DFLL handling the CPU clock, that is the output voltage. We configure the same when DFLL HW is just initialized.

1 volt here is pretty safe when CPU clock switched from bootloader to kernel at 6.912MHz@pllx.



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

Yes, either way needs to go with DT. The very initial patchset for DFLL-PWM support, which introduced a redundant pwm-dfll driver. And with the driver, we can describe the PWM vdd-cpu regulator in DT with the voltage table that the PMIC can output. But NAKed by reviewer.

Okay, 'nvidia,pwm-pmic-min-microvolts' looks fine. Thanks.

[1]: https://patchwork.ozlabs.org/patch/613524/




[Index of Archives]     [ARM Kernel]     [Linux ARM]     [Linux ARM MSM]     [Linux USB Devel]     [Video for Linux]     [Linux Audio Users]     [Yosemite News]     [Linux Kernel]     [Linux SCSI]

  Powered by Linux