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

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

[Joseph Lo]

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.
+- nvidia,align-offset-uv: Regulator voltage in micro volts when PWM control is
+			  enabled and PWM output is low.
+- nvidia,align-step-uv: Voltage increase in micro volts corresponding to a
+			1/33th increase in duty cycle. Eg the voltage for 2/33th
+			duty cycle would be:
+			nvidia,align-offset-uv + nvidia,align-step-uv * 2.
+- pinctrl-0: I/O pad configuration when PWM control is enabled.
+- pinctrl-1: I/O pad configuration when PWM control is disabled.
+- pinctrl-names: must include the following entries:
+  - dvfs_pwm_enable: I/O pad configuration when PWM control is enabled.
+  - dvfs_pwm_disable: I/O pad configuration when PWM control is disabled.
+
+Example for I2C:
 
 clock@70110000 {
         compatible = "nvidia,tegra124-dfll";
@@ -76,3 +93,55 @@ clock@70110000 {
 
         nvidia,i2c-fs-rate = <400000>;
 };
+
+Example for PWM:
+
+clock@70110000 {
+	compatible = "nvidia,tegra124-dfll";
+	reg = <0 0x70110000 0 0x100>, /* DFLL control */
+	      <0 0x70110000 0 0x100>, /* I2C output control */
+	      <0 0x70110100 0 0x100>, /* Integrated I2C controller */
+	      <0 0x70110200 0 0x100>; /* Look-up table RAM */
+	interrupts = <GIC_SPI 62 IRQ_TYPE_LEVEL_HIGH>;
+	clocks = <&tegra_car TEGRA210_CLK_DFLL_SOC>,
+	         <&tegra_car TEGRA210_CLK_DFLL_REF>,
+		 <&tegra_car TEGRA124_CLK_I2C5>;;
+	clock-names = "soc", "ref", "i2c";
+	resets = <&tegra_car TEGRA124_RST_DFLL_DVCO>;
+	reset-names = "dvco";
+	#clock-cells = <0>;
+	clock-output-names = "dfllCPU_out";
+	nvidia,pwm-to-pmic;
+	nvidia,init-uv = <1000000>;
+	nvidia,align-step-uv = <19200>; /* 19.2mV */
+	nvidia,align-offset-uv = <708000>; /* 708mV */
+	nvidia,sample-rate = <25000>;
+	nvidia,droop-ctrl = <0x00000f00>;
+	nvidia,force-mode = <1>;
+	nvidia,cf = <6>;
+	nvidia,ci = <0>;
+	nvidia,cg = <2>;
+	nvidia,pwm-period = <2500>; /* 2.5us */
+	pinctrl-names = "dvfs_pwm_enable", "dvfs_pwm_disable";
+	pinctrl-0 = <&dvfs_pwm_active_state>;
+	pinctrl-1 = <&dvfs_pwm_inactive_state>;
+};
+
+/* pinmux nodes added for completeness. Binding doc can be found in:
+ * Documentation/devicetree/bindings/pinctrl/nvidia,tegra210-pinmux.txt
+ */
+
+pinmux: pinmux@700008d4 {
+	dvfs_pwm_active_state: dvfs_pwm_active {
+		dvfs_pwm_pbb1 {
+			nvidia,pins = "dvfs_pwm_pbb1";
+			nvidia,tristate = <TEGRA_PIN_DISABLE>;
+		};
+	};
+	dvfs_pwm_inactive_state: dvfs_pwm_inactive {
+		dvfs_pwm_pbb1 {
+			nvidia,pins = "dvfs_pwm_pbb1";
+			nvidia,tristate = <TEGRA_PIN_ENABLE>;
+		};
+	};
+};
-- 
2.19.2

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

[Jon Hunter]

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'.

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

> +- nvidia,align-step-uv: Voltage increase in micro volts corresponding to a
> +			1/33th increase in duty cycle. Eg the voltage for 2/33th
> +			duty cycle would be:

Maybe consider 'pwm-voltage-step-microvolt'.

> +			nvidia,align-offset-uv + nvidia,align-step-uv * 2.
> +- pinctrl-0: I/O pad configuration when PWM control is enabled.
> +- pinctrl-1: I/O pad configuration when PWM control is disabled.
> +- pinctrl-names: must include the following entries:
> +  - dvfs_pwm_enable: I/O pad configuration when PWM control is enabled.
> +  - dvfs_pwm_disable: I/O pad configuration when PWM control is disabled.

Please see Rob's feedback on the above [0].

Cheers
Jon

[0] https://lore.kernel.org/patchwork/patch/885328/

-- 
nvpublic

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

[Joseph Lo]

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.

> 
>> +- 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'.

> 
>> +- nvidia,align-step-uv: Voltage increase in micro volts corresponding to a
>> +			1/33th increase in duty cycle. Eg the voltage for 2/33th
>> +			duty cycle would be:
> 
> Maybe consider 'pwm-voltage-step-microvolt'.
Okay.

> 
>> +			nvidia,align-offset-uv + nvidia,align-step-uv * 2.
>> +- pinctrl-0: I/O pad configuration when PWM control is enabled.
>> +- pinctrl-1: I/O pad configuration when PWM control is disabled.
>> +- pinctrl-names: must include the following entries:
>> +  - dvfs_pwm_enable: I/O pad configuration when PWM control is enabled.
>> +  - dvfs_pwm_disable: I/O pad configuration when PWM control is disabled.
> 
> Please see Rob's feedback on the above [0].
Yes, I did refer that comments in this patch. And fixed that in 
description but missed for bindings ...

Thanks,
Joseph

> 
> Cheers
> Jon
> 
> [0] https://lore.kernel.org/patchwork/patch/885328/
> 

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

[Jon Hunter]

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
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

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

[Joseph Lo]

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@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?
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/

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

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

[Jon Hunter]

On 10/12/2018 09:31, Joseph Lo wrote:
> 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@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?
> 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.

OK, now I understand. Seems a little fragile, but maybe there is no
better alternative here.

Please describe what this voltage is in the binding doc, so if anyone
happened to modify their bootloader to run at a different speed that it
is stated that this voltage should be high enough to support initial
boot frequency.

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

Thierry, what are your thoughts here? I guess you had asked initially
why we needed a phandle to the PMIC. Seems we need to comprehend the min
voltage supported by the PMIC for creating the LUT.

Cheers
Jon

-- 
nvpublic

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

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

[Joseph Lo]

On 12/10/18 5:44 PM, Jon Hunter wrote:
> 
> On 10/12/2018 09:31, Joseph Lo wrote:
>> 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@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?
>> 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.
> 
> OK, now I understand. Seems a little fragile, but maybe there is no
> better alternative here.
> 
> Please describe what this voltage is in the binding doc, so if anyone
> happened to modify their bootloader to run at a different speed that it
> is stated that this voltage should be high enough to support initial
> boot frequency.
> 
>>>
>>>>>
>>>>>> +- 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/
> 
> Thierry, what are your thoughts here? I guess you had asked initially
> why we needed a phandle to the PMIC. Seems we need to comprehend the min
> voltage supported by the PMIC for creating the LUT.
> 

Jon,

Notice that this is not a real "PWM" driver it just provides an 
interface that makes us can provide a PWM regulator node with voltage 
table. Then you can get LUT from that. But the PWM driver itself is a 
fake pwm driver as the DFLL-PWM is hardware driven not SW.

And as you saw in [1], the DFLL control code should remain in DFLL 
driver, not in the PWM driver. So basically, I still prefer the way we 
presented in this series.

Thanks,
Joseph

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

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

[Peter De Schrijver]

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.

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

[Joseph Lo]

On 12/11/18 5:16 PM, Peter De Schrijver wrote:
> 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.

Yes, correct. The previous reply I did was the case of I2C regulator. 
Sorry, I made the confusion here.

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

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

[Peter De Schrijver]

On Fri, Dec 07, 2018 at 01:41:57PM +0000, 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'.
> 

Inactive is not very accurate. The OVR regulator will output
nvidia,align-offset-uv when the PWM input is driven low but will output
nvidia,init-uv when the PWM input is in tristate mode.

> > +- 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?
> 
> > +- nvidia,align-step-uv: Voltage increase in micro volts corresponding to a
> > +			1/33th increase in duty cycle. Eg the voltage for 2/33th
> > +			duty cycle would be:
> 
> Maybe consider 'pwm-voltage-step-microvolt'.
> 
> > +			nvidia,align-offset-uv + nvidia,align-step-uv * 2.
> > +- pinctrl-0: I/O pad configuration when PWM control is enabled.
> > +- pinctrl-1: I/O pad configuration when PWM control is disabled.
> > +- pinctrl-names: must include the following entries:
> > +  - dvfs_pwm_enable: I/O pad configuration when PWM control is enabled.
> > +  - dvfs_pwm_disable: I/O pad configuration when PWM control is disabled.
> 
> Please see Rob's feedback on the above [0].
> 
> Cheers
> Jon
> 
> [0] https://lore.kernel.org/patchwork/patch/885328/
> 
> -- 
> nvpublic

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

[Jon Hunter]

On 11/12/2018 09:15, Peter De Schrijver wrote:
> On Fri, Dec 07, 2018 at 01:41:57PM +0000, 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'.
>>
> 
> Inactive is not very accurate. The OVR regulator will output
> nvidia,align-offset-uv when the PWM input is driven low but will output
> nvidia,init-uv when the PWM input is in tristate mode.

Maybe but I really don't find 'nvidia,align-offset-uv' and
'nvidia,init-uv' very descriptive either. We need to make sure that the
names and description make it clear what these are and where they come
from to anyone reading that documentation that has never laid eyes on
this before.

Sounds like the align-offset-uv is the minimum voltage when PWM is
active/enabled and init-uv is the default voltage the regulator outputs
when PWM control is disabled. Would the following be any better ...

- nvidia,pwm-tristate-microvolts: Regulator voltage in micro volts when
  PWM control is disabled and the PWM output is tristated. Note that
  this voltage is configured in hardware, typically via a resistor
  divider.
- nvidia,pwm-min-microvolts: Regulator voltage in micro volts when PWM
  control is enabled and PWM output is low. Hence, this is the minimum
  output voltage that the regulator supports when PWM control is
  enabled.

Jon

-- 
nvpublic

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

[Joseph Lo]

On 12/11/18 7:52 PM, Jon Hunter wrote:
> 
> On 11/12/2018 09:15, Peter De Schrijver wrote:
>> On Fri, Dec 07, 2018 at 01:41:57PM +0000, 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'.
>>>
>>
>> Inactive is not very accurate. The OVR regulator will output
>> nvidia,align-offset-uv when the PWM input is driven low but will output
>> nvidia,init-uv when the PWM input is in tristate mode.
> 
> Maybe but I really don't find 'nvidia,align-offset-uv' and
> 'nvidia,init-uv' very descriptive either. We need to make sure that the
> names and description make it clear what these are and where they come
> from to anyone reading that documentation that has never laid eyes on
> this before.
> 
> Sounds like the align-offset-uv is the minimum voltage when PWM is
> active/enabled and init-uv is the default voltage the regulator outputs
> when PWM control is disabled. Would the following be any better ...
> 
> - nvidia,pwm-tristate-microvolts: Regulator voltage in micro volts when
>    PWM control is disabled and the PWM output is tristated. Note that
>    this voltage is configured in hardware, typically via a resistor
>    divider.
> - nvidia,pwm-min-microvolts: Regulator voltage in micro volts when PWM
>    control is enabled and PWM output is low. Hence, this is the minimum
>    output voltage that the regulator supports when PWM control is
>    enabled.

Thanks, Jon and Peter.

That's more clear indeed. In the discussion, that also comes up with one 
more reason that we shouldn't handle vdd-cpu rail with regulator API in 
cpufreq driver that showed in other patches in this patchset. With 
PWM-based vdd-cpu regulator, we are not able to control the output in 
software.

[PATCH 02/19] dt-bindings: clock: tegra124-dfll: add Tegra210 support

[Joseph Lo]

Add Tegra210 support for DFLL clock.

Cc: devicetree@vger.kernel.org
Signed-off-by: Joseph Lo <josephl@nvidia.com>
---
 .../devicetree/bindings/clock/nvidia,tegra124-dfll.txt        | 4 +++-
 1 file changed, 3 insertions(+), 1 deletion(-)

diff --git a/Documentation/devicetree/bindings/clock/nvidia,tegra124-dfll.txt b/Documentation/devicetree/bindings/clock/nvidia,tegra124-dfll.txt
index 8c97600d2bad..4bd44dd7ec1e 100644
--- a/Documentation/devicetree/bindings/clock/nvidia,tegra124-dfll.txt
+++ b/Documentation/devicetree/bindings/clock/nvidia,tegra124-dfll.txt
@@ -10,7 +10,9 @@ 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.
 
 Required properties:
-- compatible : should be "nvidia,tegra124-dfll"
+- compatible : should be one of:
+  - "nvidia,tegra124-dfll": for Tegra124
+  - "nvidia,tegra210-dfll": for Tegra210
 - reg : Defines the following set of registers, in the order listed:
         - registers for the DFLL control logic.
         - registers for the I2C output logic.
-- 
2.19.2

Re: [PATCH 02/19] dt-bindings: clock: tegra124-dfll: add Tegra210 support

[Jon Hunter]

On 04/12/2018 09:25, Joseph Lo wrote:
> Add Tegra210 support for DFLL clock.
> 
> Cc: devicetree@vger.kernel.org
> Signed-off-by: Joseph Lo <josephl@nvidia.com>
> ---
>  .../devicetree/bindings/clock/nvidia,tegra124-dfll.txt        | 4 +++-
>  1 file changed, 3 insertions(+), 1 deletion(-)
> 
> diff --git a/Documentation/devicetree/bindings/clock/nvidia,tegra124-dfll.txt b/Documentation/devicetree/bindings/clock/nvidia,tegra124-dfll.txt
> index 8c97600d2bad..4bd44dd7ec1e 100644
> --- a/Documentation/devicetree/bindings/clock/nvidia,tegra124-dfll.txt
> +++ b/Documentation/devicetree/bindings/clock/nvidia,tegra124-dfll.txt
> @@ -10,7 +10,9 @@ 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.
>  
>  Required properties:
> -- compatible : should be "nvidia,tegra124-dfll"
> +- compatible : should be one of:
> +  - "nvidia,tegra124-dfll": for Tegra124
> +  - "nvidia,tegra210-dfll": for Tegra210
>  - reg : Defines the following set of registers, in the order listed:
>          - registers for the DFLL control logic.
>          - registers for the I2C output logic.

Acked-by: Jon Hunter <jonathanh@nvidia.com>

Cheers
Jon

-- 
nvpublic

[PATCH 03/19] dt-bindings: cpufreq: tegra124: remove vdd-cpu-supply from required properties

[Joseph Lo]

The Tegra124 cpufreq driver works only with DFLL clock, which is a
hardware-based frequency/voltage controller. The driver doesn't need to
control the regulator itself. Hence remove that.

Cc: devicetree@vger.kernel.org
Signed-off-by: Joseph Lo <josephl@nvidia.com>
---
 .../devicetree/bindings/cpufreq/nvidia,tegra124-cpufreq.txt     | 2 --
 1 file changed, 2 deletions(-)

diff --git a/Documentation/devicetree/bindings/cpufreq/nvidia,tegra124-cpufreq.txt b/Documentation/devicetree/bindings/cpufreq/nvidia,tegra124-cpufreq.txt
index b1669fbfb740..031545a29caf 100644
--- a/Documentation/devicetree/bindings/cpufreq/nvidia,tegra124-cpufreq.txt
+++ b/Documentation/devicetree/bindings/cpufreq/nvidia,tegra124-cpufreq.txt
@@ -13,7 +13,6 @@ Required properties:
   - pll_x: Fast PLL clocksource.
   - pll_p: Auxiliary PLL used during fast PLL rate changes.
   - dfll: Fast DFLL clocksource that also automatically scales CPU voltage.
-- vdd-cpu-supply: Regulator for CPU voltage
 
 Optional properties:
 - clock-latency: Specify the possible maximum transition latency for clock,
@@ -37,7 +36,6 @@ cpus {
 			 <&dfll>;
 		clock-names = "cpu_g", "cpu_lp", "pll_x", "pll_p", "dfll";
 		clock-latency = <300000>;
-		vdd-cpu-supply: <&vdd_cpu>;
 	};
 
 	<...>
-- 
2.19.2

Re: [PATCH 03/19] dt-bindings: cpufreq: tegra124: remove vdd-cpu-supply from required properties

[Peter De Schrijver]

On Tue, Dec 04, 2018 at 05:25:32PM +0800, Joseph Lo wrote:
> The Tegra124 cpufreq driver works only with DFLL clock, which is a
> hardware-based frequency/voltage controller. The driver doesn't need to
> control the regulator itself. Hence remove that.
> 

I think this is required for DFLL controlled I2C regulators because the
regulator is queried for voltage selectors and I2C slave ID?

Peter.

> Cc: devicetree@vger.kernel.org
> Signed-off-by: Joseph Lo <josephl@nvidia.com>
> ---
>  .../devicetree/bindings/cpufreq/nvidia,tegra124-cpufreq.txt     | 2 --
>  1 file changed, 2 deletions(-)
> 
> diff --git a/Documentation/devicetree/bindings/cpufreq/nvidia,tegra124-cpufreq.txt b/Documentation/devicetree/bindings/cpufreq/nvidia,tegra124-cpufreq.txt
> index b1669fbfb740..031545a29caf 100644
> --- a/Documentation/devicetree/bindings/cpufreq/nvidia,tegra124-cpufreq.txt
> +++ b/Documentation/devicetree/bindings/cpufreq/nvidia,tegra124-cpufreq.txt
> @@ -13,7 +13,6 @@ Required properties:
>    - pll_x: Fast PLL clocksource.
>    - pll_p: Auxiliary PLL used during fast PLL rate changes.
>    - dfll: Fast DFLL clocksource that also automatically scales CPU voltage.
> -- vdd-cpu-supply: Regulator for CPU voltage
>  
>  Optional properties:
>  - clock-latency: Specify the possible maximum transition latency for clock,
> @@ -37,7 +36,6 @@ cpus {
>  			 <&dfll>;
>  		clock-names = "cpu_g", "cpu_lp", "pll_x", "pll_p", "dfll";
>  		clock-latency = <300000>;
> -		vdd-cpu-supply: <&vdd_cpu>;
>  	};
>  
>  	<...>
> -- 
> 2.19.2
> 

Re: [PATCH 03/19] dt-bindings: cpufreq: tegra124: remove vdd-cpu-supply from required properties

[Joseph Lo]

On 12/4/18 11:36 PM, Peter De Schrijver wrote:
> On Tue, Dec 04, 2018 at 05:25:32PM +0800, Joseph Lo wrote:
>> The Tegra124 cpufreq driver works only with DFLL clock, which is a
>> hardware-based frequency/voltage controller. The driver doesn't need to
>> control the regulator itself. Hence remove that.
>>
> 
> I think this is required for DFLL controlled I2C regulators because the
> regulator is queried for voltage selectors and I2C slave ID?
> 

Hi Peter,

Yes, it's required for DFLL-I2C mode and defined in DFLL node. It's not 
needed here in the CPU node for CPU freq driver to handle that. Hence 
remove that.

Thanks,
Joseph


> 
>> Cc: devicetree@vger.kernel.org
>> Signed-off-by: Joseph Lo <josephl@nvidia.com>
>> ---
>>   .../devicetree/bindings/cpufreq/nvidia,tegra124-cpufreq.txt     | 2 --
>>   1 file changed, 2 deletions(-)
>>
>> diff --git a/Documentation/devicetree/bindings/cpufreq/nvidia,tegra124-cpufreq.txt b/Documentation/devicetree/bindings/cpufreq/nvidia,tegra124-cpufreq.txt
>> index b1669fbfb740..031545a29caf 100644
>> --- a/Documentation/devicetree/bindings/cpufreq/nvidia,tegra124-cpufreq.txt
>> +++ b/Documentation/devicetree/bindings/cpufreq/nvidia,tegra124-cpufreq.txt
>> @@ -13,7 +13,6 @@ Required properties:
>>     - pll_x: Fast PLL clocksource.
>>     - pll_p: Auxiliary PLL used during fast PLL rate changes.
>>     - dfll: Fast DFLL clocksource that also automatically scales CPU voltage.
>> -- vdd-cpu-supply: Regulator for CPU voltage
>>   
>>   Optional properties:
>>   - clock-latency: Specify the possible maximum transition latency for clock,
>> @@ -37,7 +36,6 @@ cpus {
>>   			 <&dfll>;
>>   		clock-names = "cpu_g", "cpu_lp", "pll_x", "pll_p", "dfll";
>>   		clock-latency = <300000>;
>> -		vdd-cpu-supply: <&vdd_cpu>;
>>   	};
>>   
>>   	<...>
>> -- 
>> 2.19.2
>>

Re: [PATCH 03/19] dt-bindings: cpufreq: tegra124: remove vdd-cpu-supply from required properties

[Peter De Schrijver]

On Wed, Dec 05, 2018 at 11:05:58AM +0800, Joseph Lo wrote:
> On 12/4/18 11:36 PM, Peter De Schrijver wrote:
> > On Tue, Dec 04, 2018 at 05:25:32PM +0800, Joseph Lo wrote:
> > > The Tegra124 cpufreq driver works only with DFLL clock, which is a
> > > hardware-based frequency/voltage controller. The driver doesn't need to
> > > control the regulator itself. Hence remove that.
> > > 
> > 
> > I think this is required for DFLL controlled I2C regulators because the
> > regulator is queried for voltage selectors and I2C slave ID?
> > 
> 
> Hi Peter,
> 
> Yes, it's required for DFLL-I2C mode and defined in DFLL node. It's not
> needed here in the CPU node for CPU freq driver to handle that. Hence remove
> that.
> 

Ah right. So yes, this is fine then.

Peter.

> Thanks,
> Joseph
> 
> 
> > 
> > > Cc: devicetree@vger.kernel.org
> > > Signed-off-by: Joseph Lo <josephl@nvidia.com>
> > > ---
> > >   .../devicetree/bindings/cpufreq/nvidia,tegra124-cpufreq.txt     | 2 --
> > >   1 file changed, 2 deletions(-)
> > > 
> > > diff --git a/Documentation/devicetree/bindings/cpufreq/nvidia,tegra124-cpufreq.txt b/Documentation/devicetree/bindings/cpufreq/nvidia,tegra124-cpufreq.txt
> > > index b1669fbfb740..031545a29caf 100644
> > > --- a/Documentation/devicetree/bindings/cpufreq/nvidia,tegra124-cpufreq.txt
> > > +++ b/Documentation/devicetree/bindings/cpufreq/nvidia,tegra124-cpufreq.txt
> > > @@ -13,7 +13,6 @@ Required properties:
> > >     - pll_x: Fast PLL clocksource.
> > >     - pll_p: Auxiliary PLL used during fast PLL rate changes.
> > >     - dfll: Fast DFLL clocksource that also automatically scales CPU voltage.
> > > -- vdd-cpu-supply: Regulator for CPU voltage
> > >   Optional properties:
> > >   - clock-latency: Specify the possible maximum transition latency for clock,
> > > @@ -37,7 +36,6 @@ cpus {
> > >   			 <&dfll>;
> > >   		clock-names = "cpu_g", "cpu_lp", "pll_x", "pll_p", "dfll";
> > >   		clock-latency = <300000>;
> > > -		vdd-cpu-supply: <&vdd_cpu>;
> > >   	};
> > >   	<...>
> > > -- 
> > > 2.19.2
> > > 

Re: [PATCH 03/19] dt-bindings: cpufreq: tegra124: remove vdd-cpu-supply from required properties

[Jon Hunter]

On 04/12/2018 09:25, Joseph Lo wrote:
> The Tegra124 cpufreq driver works only with DFLL clock, which is a
> hardware-based frequency/voltage controller. The driver doesn't need to
> control the regulator itself. Hence remove that.
> 
> Cc: devicetree@vger.kernel.org
> Signed-off-by: Joseph Lo <josephl@nvidia.com>
> ---
>  .../devicetree/bindings/cpufreq/nvidia,tegra124-cpufreq.txt     | 2 --
>  1 file changed, 2 deletions(-)
> 
> diff --git a/Documentation/devicetree/bindings/cpufreq/nvidia,tegra124-cpufreq.txt b/Documentation/devicetree/bindings/cpufreq/nvidia,tegra124-cpufreq.txt
> index b1669fbfb740..031545a29caf 100644
> --- a/Documentation/devicetree/bindings/cpufreq/nvidia,tegra124-cpufreq.txt
> +++ b/Documentation/devicetree/bindings/cpufreq/nvidia,tegra124-cpufreq.txt
> @@ -13,7 +13,6 @@ Required properties:
>    - pll_x: Fast PLL clocksource.
>    - pll_p: Auxiliary PLL used during fast PLL rate changes.
>    - dfll: Fast DFLL clocksource that also automatically scales CPU voltage.
> -- vdd-cpu-supply: Regulator for CPU voltage
>  
>  Optional properties:
>  - clock-latency: Specify the possible maximum transition latency for clock,
> @@ -37,7 +36,6 @@ cpus {
>  			 <&dfll>;
>  		clock-names = "cpu_g", "cpu_lp", "pll_x", "pll_p", "dfll";
>  		clock-latency = <300000>;
> -		vdd-cpu-supply: <&vdd_cpu>;
>  	};
>  
>  	<...>
> 

Acked-by: Jon Hunter <jonathanh@nvidia.com>

Cheers
Jon

-- 
nvpublic

[PATCH 04/19] dt-bindings: cpufreq: tegra124: remove cpu_lp clock from required properties

[Joseph Lo]

The cpu_lp clock property is only needed when the CPUfreq driver
supports CPU cluster switching. But it was not a design for this driver
and it didn't handle that as well. So removing this property.

Cc: devicetree@vger.kernel.org
Signed-off-by: Joseph Lo <josephl@nvidia.com>
---
 .../devicetree/bindings/cpufreq/nvidia,tegra124-cpufreq.txt   | 4 +---
 1 file changed, 1 insertion(+), 3 deletions(-)

diff --git a/Documentation/devicetree/bindings/cpufreq/nvidia,tegra124-cpufreq.txt b/Documentation/devicetree/bindings/cpufreq/nvidia,tegra124-cpufreq.txt
index 031545a29caf..03196d5ea515 100644
--- a/Documentation/devicetree/bindings/cpufreq/nvidia,tegra124-cpufreq.txt
+++ b/Documentation/devicetree/bindings/cpufreq/nvidia,tegra124-cpufreq.txt
@@ -9,7 +9,6 @@ Required properties:
   See ../clocks/clock-bindings.txt for details.
 - clock-names: Must include the following entries:
   - cpu_g: Clock mux for the fast CPU cluster.
-  - cpu_lp: Clock mux for the low-power CPU cluster.
   - pll_x: Fast PLL clocksource.
   - pll_p: Auxiliary PLL used during fast PLL rate changes.
   - dfll: Fast DFLL clocksource that also automatically scales CPU voltage.
@@ -30,11 +29,10 @@ cpus {
 		reg = <0>;
 
 		clocks = <&tegra_car TEGRA124_CLK_CCLK_G>,
-			 <&tegra_car TEGRA124_CLK_CCLK_LP>,
 			 <&tegra_car TEGRA124_CLK_PLL_X>,
 			 <&tegra_car TEGRA124_CLK_PLL_P>,
 			 <&dfll>;
-		clock-names = "cpu_g", "cpu_lp", "pll_x", "pll_p", "dfll";
+		clock-names = "cpu_g", "pll_x", "pll_p", "dfll";
 		clock-latency = <300000>;
 	};
 
-- 
2.19.2

Re: [PATCH 04/19] dt-bindings: cpufreq: tegra124: remove cpu_lp clock from required properties

[Peter De Schrijver]

On Tue, Dec 04, 2018 at 05:25:33PM +0800, Joseph Lo wrote:
> The cpu_lp clock property is only needed when the CPUfreq driver
> supports CPU cluster switching. But it was not a design for this driver
> and it didn't handle that as well. So removing this property.
> 

I would mark it optional. This means current DTs will still be
technically compatible with this binding doc.

Peter.

> Cc: devicetree@vger.kernel.org
> Signed-off-by: Joseph Lo <josephl@nvidia.com>
> ---
>  .../devicetree/bindings/cpufreq/nvidia,tegra124-cpufreq.txt   | 4 +---
>  1 file changed, 1 insertion(+), 3 deletions(-)
> 
> diff --git a/Documentation/devicetree/bindings/cpufreq/nvidia,tegra124-cpufreq.txt b/Documentation/devicetree/bindings/cpufreq/nvidia,tegra124-cpufreq.txt
> index 031545a29caf..03196d5ea515 100644
> --- a/Documentation/devicetree/bindings/cpufreq/nvidia,tegra124-cpufreq.txt
> +++ b/Documentation/devicetree/bindings/cpufreq/nvidia,tegra124-cpufreq.txt
> @@ -9,7 +9,6 @@ Required properties:
>    See ../clocks/clock-bindings.txt for details.
>  - clock-names: Must include the following entries:
>    - cpu_g: Clock mux for the fast CPU cluster.
> -  - cpu_lp: Clock mux for the low-power CPU cluster.
>    - pll_x: Fast PLL clocksource.
>    - pll_p: Auxiliary PLL used during fast PLL rate changes.
>    - dfll: Fast DFLL clocksource that also automatically scales CPU voltage.
> @@ -30,11 +29,10 @@ cpus {
>  		reg = <0>;
>  
>  		clocks = <&tegra_car TEGRA124_CLK_CCLK_G>,
> -			 <&tegra_car TEGRA124_CLK_CCLK_LP>,
>  			 <&tegra_car TEGRA124_CLK_PLL_X>,
>  			 <&tegra_car TEGRA124_CLK_PLL_P>,
>  			 <&dfll>;
> -		clock-names = "cpu_g", "cpu_lp", "pll_x", "pll_p", "dfll";
> +		clock-names = "cpu_g", "pll_x", "pll_p", "dfll";
>  		clock-latency = <300000>;
>  	};
>  
> -- 
> 2.19.2
> 

Re: [PATCH 04/19] dt-bindings: cpufreq: tegra124: remove cpu_lp clock from required properties

[Joseph Lo]

On 12/4/18 11:37 PM, Peter De Schrijver wrote:
> On Tue, Dec 04, 2018 at 05:25:33PM +0800, Joseph Lo wrote:
>> The cpu_lp clock property is only needed when the CPUfreq driver
>> supports CPU cluster switching. But it was not a design for this driver
>> and it didn't handle that as well. So removing this property.
>>
> 
> I would mark it optional. This means current DTs will still be
> technically compatible with this binding doc.

Hi Peter,

There is no compatible issue of this property. Because the driver 
doesn't use this clock at all. Removing this won't cause any 
backward-compatible issue.

Same as previous patch for removing vdd-cpu-supply property once we fix 
that in the driver. The old dt binding still works with the new driver.

> 
>> Cc: devicetree@vger.kernel.org
>> Signed-off-by: Joseph Lo <josephl@nvidia.com>
>> ---
>>   .../devicetree/bindings/cpufreq/nvidia,tegra124-cpufreq.txt   | 4 +---
>>   1 file changed, 1 insertion(+), 3 deletions(-)
>>
>> diff --git a/Documentation/devicetree/bindings/cpufreq/nvidia,tegra124-cpufreq.txt b/Documentation/devicetree/bindings/cpufreq/nvidia,tegra124-cpufreq.txt
>> index 031545a29caf..03196d5ea515 100644
>> --- a/Documentation/devicetree/bindings/cpufreq/nvidia,tegra124-cpufreq.txt
>> +++ b/Documentation/devicetree/bindings/cpufreq/nvidia,tegra124-cpufreq.txt
>> @@ -9,7 +9,6 @@ Required properties:
>>     See ../clocks/clock-bindings.txt for details.
>>   - clock-names: Must include the following entries:
>>     - cpu_g: Clock mux for the fast CPU cluster.
>> -  - cpu_lp: Clock mux for the low-power CPU cluster.
>>     - pll_x: Fast PLL clocksource.
>>     - pll_p: Auxiliary PLL used during fast PLL rate changes.
>>     - dfll: Fast DFLL clocksource that also automatically scales CPU voltage.
>> @@ -30,11 +29,10 @@ cpus {
>>   		reg = <0>;
>>   
>>   		clocks = <&tegra_car TEGRA124_CLK_CCLK_G>,
>> -			 <&tegra_car TEGRA124_CLK_CCLK_LP>,
>>   			 <&tegra_car TEGRA124_CLK_PLL_X>,
>>   			 <&tegra_car TEGRA124_CLK_PLL_P>,
>>   			 <&dfll>;
>> -		clock-names = "cpu_g", "cpu_lp", "pll_x", "pll_p", "dfll";
>> +		clock-names = "cpu_g", "pll_x", "pll_p", "dfll";
>>   		clock-latency = <300000>;
>>   	};
>>   
>> -- 
>> 2.19.2
>>

Re: [PATCH 04/19] dt-bindings: cpufreq: tegra124: remove cpu_lp clock from required properties

[Jon Hunter]

On 04/12/2018 09:25, Joseph Lo wrote:
> The cpu_lp clock property is only needed when the CPUfreq driver
> supports CPU cluster switching. But it was not a design for this driver
> and it didn't handle that as well. So removing this property.
> 
> Cc: devicetree@vger.kernel.org
> Signed-off-by: Joseph Lo <josephl@nvidia.com>
> ---
>  .../devicetree/bindings/cpufreq/nvidia,tegra124-cpufreq.txt   | 4 +---
>  1 file changed, 1 insertion(+), 3 deletions(-)
> 
> diff --git a/Documentation/devicetree/bindings/cpufreq/nvidia,tegra124-cpufreq.txt b/Documentation/devicetree/bindings/cpufreq/nvidia,tegra124-cpufreq.txt
> index 031545a29caf..03196d5ea515 100644
> --- a/Documentation/devicetree/bindings/cpufreq/nvidia,tegra124-cpufreq.txt
> +++ b/Documentation/devicetree/bindings/cpufreq/nvidia,tegra124-cpufreq.txt
> @@ -9,7 +9,6 @@ Required properties:
>    See ../clocks/clock-bindings.txt for details.
>  - clock-names: Must include the following entries:
>    - cpu_g: Clock mux for the fast CPU cluster.
> -  - cpu_lp: Clock mux for the low-power CPU cluster.
>    - pll_x: Fast PLL clocksource.
>    - pll_p: Auxiliary PLL used during fast PLL rate changes.
>    - dfll: Fast DFLL clocksource that also automatically scales CPU voltage.
> @@ -30,11 +29,10 @@ cpus {
>  		reg = <0>;
>  
>  		clocks = <&tegra_car TEGRA124_CLK_CCLK_G>,
> -			 <&tegra_car TEGRA124_CLK_CCLK_LP>,
>  			 <&tegra_car TEGRA124_CLK_PLL_X>,
>  			 <&tegra_car TEGRA124_CLK_PLL_P>,
>  			 <&dfll>;
> -		clock-names = "cpu_g", "cpu_lp", "pll_x", "pll_p", "dfll";
> +		clock-names = "cpu_g", "pll_x", "pll_p", "dfll";
>  		clock-latency = <300000>;
>  	};
>  
> 

Acked-by: Jon Hunter <jonathanh@nvidia.com>

Cheers
Jon

-- 
nvpublic