2024-05-27 12:44:53

by Neha Malcom Francis

[permalink] [raw]
Subject: [PATCH v2 0/3] arm64: ti: Add TPS6287 nodes

Add nodes for TPS6287x present in AM68 SK, AM69 SK and J784S4 EVM. They
are a family of synchronous step-down DC/DC converters. These converters
are used to enable AVS (Adaptive Voltage Scaling) for these devices.

Also since AM68 SK lacks DT node of it's PMIC, LP8733; make use of this
series to add that in as well.

Data sheet: https://www.ti.com/lit/ds/slvsgc5a/slvsgc5a.pdf
Boot logs (v2): https://gist.github.com/nehamalcom/1a288f534d730e8af43c48a175919b19

Changes since v1:
- remove changing the compatibility of the existing driver (Rob and
Mark)
- remove unnecessary bootph-pre-ram, have them only for VDD_CPU_AVS
(Udit)

Neha Malcom Francis (3):
arm64: boot: dts: ti: k3-am68-sk-base-board: Add LP8733 and TPS6287
nodes
arm64: boot: dts: ti: k3-am69-sk: Add TPS62873 node
arm64: boot: dts: ti: k3-j784s4-evm: Add TPS62873 node

.../boot/dts/ti/k3-am68-sk-base-board.dts | 77 +++++++++++++++++++
arch/arm64/boot/dts/ti/k3-am69-sk.dts | 21 +++++
arch/arm64/boot/dts/ti/k3-j784s4-evm.dts | 21 +++++
3 files changed, 119 insertions(+)

--
2.34.1



2024-05-27 12:44:54

by Neha Malcom Francis

[permalink] [raw]
Subject: [PATCH v2 1/3] arm64: boot: dts: ti: k3-am68-sk-base-board: Add LP8733 and TPS6287 nodes

Add DTS node for LP87334E PMIC and two TPS6287x high current buck
converters.

LP87334E is responsible for supplying power to the MCU and MAIN domains
as well as to LPDDR4. The two TPS6287x supply power to the MAIN
domain for AVS and other core supplies.

Signed-off-by: Neha Malcom Francis <[email protected]>
Link: https://www.ti.com/lit/pdf/slda060
---
.../boot/dts/ti/k3-am68-sk-base-board.dts | 77 +++++++++++++++++++
1 file changed, 77 insertions(+)

diff --git a/arch/arm64/boot/dts/ti/k3-am68-sk-base-board.dts b/arch/arm64/boot/dts/ti/k3-am68-sk-base-board.dts
index d743f023cdd9..74e59035013c 100644
--- a/arch/arm64/boot/dts/ti/k3-am68-sk-base-board.dts
+++ b/arch/arm64/boot/dts/ti/k3-am68-sk-base-board.dts
@@ -414,6 +414,83 @@ &wkup_uart0 {
pinctrl-0 = <&wkup_uart0_pins_default>;
};

+&wkup_i2c0 {
+ bootph-all;
+ status = "okay";
+ pinctrl-names = "default";
+ pinctrl-0 = <&wkup_i2c0_pins_default>;
+ clock-frequency = <400000>;
+
+ lp8733: pmic@60 {
+ compatible = "ti,lp8733";
+ reg = <0x60>;
+
+ buck0-in-supply = <&vsys_3v3>;
+ buck1-in-supply = <&vsys_3v3>;
+ ldo0-in-supply = <&vsys_3v3>;
+ ldo1-in-supply = <&vsys_3v3>;
+
+ lp8733_regulators: regulators {
+ lp8733_buck0_reg: buck0 {
+ /* FB_B0 -> LP8733-BUCK1 - VDD_MCU_0V85 */
+ regulator-name = "lp8733-buck0";
+ regulator-min-microvolt = <850000>;
+ regulator-max-microvolt = <850000>;
+ regulator-always-on;
+ regulator-boot-on;
+ };
+
+ lp8733_buck1_reg: buck1 {
+ /* FB_B1 -> LP8733-BUCK2 - VDD_DDR_1V1 */
+ regulator-name = "lp8733-buck1";
+ regulator-min-microvolt = <1100000>;
+ regulator-max-microvolt = <1100000>;
+ regulator-always-on;
+ regulator-boot-on;
+ };
+
+ lp8733_ldo0_reg: ldo0 {
+ /* LDO0 -> LP8733-LDO1 - VDA_DLL_0V8 */
+ regulator-name = "lp8733-ldo0";
+ regulator-min-microvolt = <800000>;
+ regulator-max-microvolt = <800000>;
+ regulator-boot-on;
+ regulator-always-on;
+ };
+
+ lp8733_ldo1_reg: ldo1 {
+ /* LDO1 -> LP8733-LDO2 - VDA_LN_1V8 */
+ regulator-name = "lp8733-ldo1";
+ regulator-min-microvolt = <1800000>;
+ regulator-max-microvolt = <1800000>;
+ regulator-always-on;
+ regulator-boot-on;
+ };
+ };
+ };
+
+ tps62873a: tps62873@40 {
+ compatible = "ti,tps62873";
+ bootph-pre-ram;
+ reg = <0x40>;
+ regulator-name = "VDD_CPU_AVS";
+ regulator-min-microvolt = <600000>;
+ regulator-max-microvolt = <900000>;
+ regulator-boot-on;
+ regulator-always-on;
+ };
+
+ tps62873b: tps62873@43 {
+ compatible = "ti,tps62873";
+ reg = <0x43>;
+ regulator-name = "VDD_CORE_0V8";
+ regulator-min-microvolt = <800000>;
+ regulator-max-microvolt = <800000>;
+ regulator-boot-on;
+ regulator-always-on;
+ };
+};
+
&mcu_uart0 {
status = "okay";
pinctrl-names = "default";
--
2.34.1


2024-05-27 12:45:12

by Neha Malcom Francis

[permalink] [raw]
Subject: [PATCH v2 2/3] arm64: boot: dts: ti: k3-am69-sk: Add TPS62873 node

Add DTS node for two TPS6287x high current buck convertors.

The two TPS6287x supply power to the MAIN domain for AVS and other core
supplies.

Signed-off-by: Neha Malcom Francis <[email protected]>
---
arch/arm64/boot/dts/ti/k3-am69-sk.dts | 21 +++++++++++++++++++++
1 file changed, 21 insertions(+)

diff --git a/arch/arm64/boot/dts/ti/k3-am69-sk.dts b/arch/arm64/boot/dts/ti/k3-am69-sk.dts
index d88651c297a2..d5132b2029af 100644
--- a/arch/arm64/boot/dts/ti/k3-am69-sk.dts
+++ b/arch/arm64/boot/dts/ti/k3-am69-sk.dts
@@ -814,6 +814,27 @@ ldoa4: ldo4 {
};
};
};
+
+ tps62873a: tps62873@40 {
+ compatible = "ti,tps62873";
+ bootph-pre-ram;
+ reg = <0x40>;
+ regulator-name = "VDD_CPU_AVS";
+ regulator-min-microvolt = <600000>;
+ regulator-max-microvolt = <900000>;
+ regulator-boot-on;
+ regulator-always-on;
+ };
+
+ tps62873b: tps62873@43 {
+ compatible = "ti,tps62873";
+ reg = <0x43>;
+ regulator-name = "VDD_CORE_0V8";
+ regulator-min-microvolt = <760000>;
+ regulator-max-microvolt = <840000>;
+ regulator-boot-on;
+ regulator-always-on;
+ };
};

&wkup_gpio0 {
--
2.34.1


2024-05-27 12:45:24

by Neha Malcom Francis

[permalink] [raw]
Subject: [PATCH v2 3/3] arm64: boot: dts: ti: k3-j784s4-evm: Add TPS62873 node

Add Tulip TPS62873 nodes for J784S4 EVM. These are step-down regulators
that supply VDD_CPU_AVS and VDD_CORE_0V8 to the SoC.

Signed-off-by: Neha Malcom Francis <[email protected]>
---
arch/arm64/boot/dts/ti/k3-j784s4-evm.dts | 21 +++++++++++++++++++++
1 file changed, 21 insertions(+)

diff --git a/arch/arm64/boot/dts/ti/k3-j784s4-evm.dts b/arch/arm64/boot/dts/ti/k3-j784s4-evm.dts
index d511b25d62e3..3884be9ca7f7 100644
--- a/arch/arm64/boot/dts/ti/k3-j784s4-evm.dts
+++ b/arch/arm64/boot/dts/ti/k3-j784s4-evm.dts
@@ -579,6 +579,27 @@ ldoa4: ldo4 {
};
};
};
+
+ tps62873a: tps62873@40 {
+ compatible = "ti,tps62873";
+ bootph-pre-ram;
+ reg = <0x40>;
+ regulator-name = "VDD_CPU_AVS";
+ regulator-min-microvolt = <750000>;
+ regulator-max-microvolt = <1330000>;
+ regulator-boot-on;
+ regulator-always-on;
+ };
+
+ tps62873b: tps62873@43 {
+ compatible = "ti,tps62873";
+ reg = <0x43>;
+ regulator-name = "VDD_CORE_0V8";
+ regulator-min-microvolt = <760000>;
+ regulator-max-microvolt = <840000>;
+ regulator-boot-on;
+ regulator-always-on;
+ };
};

&mcu_uart0 {
--
2.34.1


2024-05-27 15:01:12

by Krzysztof Kozlowski

[permalink] [raw]
Subject: Re: [PATCH v2 1/3] arm64: boot: dts: ti: k3-am68-sk-base-board: Add LP8733 and TPS6287 nodes

On 27/05/2024 14:44, Neha Malcom Francis wrote:
> Add DTS node for LP87334E PMIC and two TPS6287x high current buck
> converters.
>
> LP87334E is responsible for supplying power to the MCU and MAIN domains
> as well as to LPDDR4. The two TPS6287x supply power to the MAIN
> domain for AVS and other core supplies.

Please use subject prefixes matching the subsystem. You can get them for
example with `git log --oneline -- DIRECTORY_OR_FILE` on the directory
your patch is touching. For bindings, the preferred subjects are
explained here:
https://www.kernel.org/doc/html/latest/devicetree/bindings/submitting-patches.html#i-for-patch-submitters

>
> Signed-off-by: Neha Malcom Francis <[email protected]>
> Link: https://www.ti.com/lit/pdf/slda060
> ---
> .../boot/dts/ti/k3-am68-sk-base-board.dts | 77 +++++++++++++++++++
> 1 file changed, 77 insertions(+)
>
> diff --git a/arch/arm64/boot/dts/ti/k3-am68-sk-base-board.dts b/arch/arm64/boot/dts/ti/k3-am68-sk-base-board.dts
> index d743f023cdd9..74e59035013c 100644
> --- a/arch/arm64/boot/dts/ti/k3-am68-sk-base-board.dts
> +++ b/arch/arm64/boot/dts/ti/k3-am68-sk-base-board.dts
> @@ -414,6 +414,83 @@ &wkup_uart0 {
> pinctrl-0 = <&wkup_uart0_pins_default>;
> };
>
> +&wkup_i2c0 {
> + bootph-all;
> + status = "okay";

Please order the properties according to DTS coding style:
https://www.kernel.org/doc/html/latest/devicetree/bindings/dts-coding-style.html#order-of-properties-in-device-node

> + pinctrl-names = "default";
> + pinctrl-0 = <&wkup_i2c0_pins_default>;
> + clock-frequency = <400000>;
> +
> + lp8733: pmic@60 {
> + compatible = "ti,lp8733";
> + reg = <0x60>;
> +
> + buck0-in-supply = <&vsys_3v3>;
> + buck1-in-supply = <&vsys_3v3>;
> + ldo0-in-supply = <&vsys_3v3>;
> + ldo1-in-supply = <&vsys_3v3>;
> +
> + lp8733_regulators: regulators {
> + lp8733_buck0_reg: buck0 {
> + /* FB_B0 -> LP8733-BUCK1 - VDD_MCU_0V85 */
> + regulator-name = "lp8733-buck0";
> + regulator-min-microvolt = <850000>;
> + regulator-max-microvolt = <850000>;
> + regulator-always-on;
> + regulator-boot-on;
> + };
> +
> + lp8733_buck1_reg: buck1 {
> + /* FB_B1 -> LP8733-BUCK2 - VDD_DDR_1V1 */
> + regulator-name = "lp8733-buck1";
> + regulator-min-microvolt = <1100000>;
> + regulator-max-microvolt = <1100000>;
> + regulator-always-on;
> + regulator-boot-on;
> + };
> +
> + lp8733_ldo0_reg: ldo0 {
> + /* LDO0 -> LP8733-LDO1 - VDA_DLL_0V8 */
> + regulator-name = "lp8733-ldo0";
> + regulator-min-microvolt = <800000>;
> + regulator-max-microvolt = <800000>;
> + regulator-boot-on;
> + regulator-always-on;
> + };
> +
> + lp8733_ldo1_reg: ldo1 {
> + /* LDO1 -> LP8733-LDO2 - VDA_LN_1V8 */
> + regulator-name = "lp8733-ldo1";
> + regulator-min-microvolt = <1800000>;
> + regulator-max-microvolt = <1800000>;
> + regulator-always-on;
> + regulator-boot-on;
> + };
> + };
> + };
> +
> + tps62873a: tps62873@40 {

Node names should be generic. See also an explanation and list of
examples (not exhaustive) in DT specification:
https://devicetree-specification.readthedocs.io/en/latest/chapter2-devicetree-basics.html#generic-names-recommendation


> + compatible = "ti,tps62873";
> + bootph-pre-ram;
> + reg = <0x40>;
> + regulator-name = "VDD_CPU_AVS";
> + regulator-min-microvolt = <600000>;
> + regulator-max-microvolt = <900000>;
> + regulator-boot-on;
> + regulator-always-on;
> + };
> +
> + tps62873b: tps62873@43 {

Node names should be generic. See also an explanation and list of
examples (not exhaustive) in DT specification:
https://devicetree-specification.readthedocs.io/en/latest/chapter2-devicetree-basics.html#generic-names-recommendation




Best regards,
Krzysztof


2024-05-28 04:03:36

by Neha Malcom Francis

[permalink] [raw]
Subject: Re: [PATCH v2 1/3] arm64: boot: dts: ti: k3-am68-sk-base-board: Add LP8733 and TPS6287 nodes

Hi Krzysztof,

On 27/05/24 19:59, Krzysztof Kozlowski wrote:
> On 27/05/2024 14:44, Neha Malcom Francis wrote:
>> Add DTS node for LP87334E PMIC and two TPS6287x high current buck
>> converters.
>>
>> LP87334E is responsible for supplying power to the MCU and MAIN domains
>> as well as to LPDDR4. The two TPS6287x supply power to the MAIN
>> domain for AVS and other core supplies.
>
> Please use subject prefixes matching the subsystem. You can get them for
> example with `git log --oneline -- DIRECTORY_OR_FILE` on the directory
> your patch is touching. For bindings, the preferred subjects are
> explained here:
> https://www.kernel.org/doc/html/latest/devicetree/bindings/submitting-patches.html#i-for-patch-submitters
>
>>
>> Signed-off-by: Neha Malcom Francis <[email protected]>
>> Link: https://www.ti.com/lit/pdf/slda060
>> ---
>> .../boot/dts/ti/k3-am68-sk-base-board.dts | 77 +++++++++++++++++++
>> 1 file changed, 77 insertions(+)
>>
>> diff --git a/arch/arm64/boot/dts/ti/k3-am68-sk-base-board.dts b/arch/arm64/boot/dts/ti/k3-am68-sk-base-board.dts
>> index d743f023cdd9..74e59035013c 100644
>> --- a/arch/arm64/boot/dts/ti/k3-am68-sk-base-board.dts
>> +++ b/arch/arm64/boot/dts/ti/k3-am68-sk-base-board.dts
>> @@ -414,6 +414,83 @@ &wkup_uart0 {
>> pinctrl-0 = <&wkup_uart0_pins_default>;
>> };
>>
>> +&wkup_i2c0 {
>> + bootph-all;
>> + status = "okay";
>
> Please order the properties according to DTS coding style:
> https://www.kernel.org/doc/html/latest/devicetree/bindings/dts-coding-style.html#order-of-properties-in-device-node
>
>> + pinctrl-names = "default";
>> + pinctrl-0 = <&wkup_i2c0_pins_default>;
>> + clock-frequency = <400000>;
>> +
>> + lp8733: pmic@60 {
>> + compatible = "ti,lp8733";
>> + reg = <0x60>;
>> +
>> + buck0-in-supply = <&vsys_3v3>;
>> + buck1-in-supply = <&vsys_3v3>;
>> + ldo0-in-supply = <&vsys_3v3>;
>> + ldo1-in-supply = <&vsys_3v3>;
>> +
>> + lp8733_regulators: regulators {
>> + lp8733_buck0_reg: buck0 {
>> + /* FB_B0 -> LP8733-BUCK1 - VDD_MCU_0V85 */
>> + regulator-name = "lp8733-buck0";
>> + regulator-min-microvolt = <850000>;
>> + regulator-max-microvolt = <850000>;
>> + regulator-always-on;
>> + regulator-boot-on;
>> + };
>> +
>> + lp8733_buck1_reg: buck1 {
>> + /* FB_B1 -> LP8733-BUCK2 - VDD_DDR_1V1 */
>> + regulator-name = "lp8733-buck1";
>> + regulator-min-microvolt = <1100000>;
>> + regulator-max-microvolt = <1100000>;
>> + regulator-always-on;
>> + regulator-boot-on;
>> + };
>> +
>> + lp8733_ldo0_reg: ldo0 {
>> + /* LDO0 -> LP8733-LDO1 - VDA_DLL_0V8 */
>> + regulator-name = "lp8733-ldo0";
>> + regulator-min-microvolt = <800000>;
>> + regulator-max-microvolt = <800000>;
>> + regulator-boot-on;
>> + regulator-always-on;
>> + };
>> +
>> + lp8733_ldo1_reg: ldo1 {
>> + /* LDO1 -> LP8733-LDO2 - VDA_LN_1V8 */
>> + regulator-name = "lp8733-ldo1";
>> + regulator-min-microvolt = <1800000>;
>> + regulator-max-microvolt = <1800000>;
>> + regulator-always-on;
>> + regulator-boot-on;
>> + };
>> + };
>> + };
>> +
>> + tps62873a: tps62873@40 {
>
> Node names should be generic. See also an explanation and list of
> examples (not exhaustive) in DT specification:
> https://devicetree-specification.readthedocs.io/en/latest/chapter2-devicetree-basics.html#generic-names-recommendation
>
>
>> + compatible = "ti,tps62873";
>> + bootph-pre-ram;
>> + reg = <0x40>;
>> + regulator-name = "VDD_CPU_AVS";
>> + regulator-min-microvolt = <600000>;
>> + regulator-max-microvolt = <900000>;
>> + regulator-boot-on;
>> + regulator-always-on;
>> + };
>> +
>> + tps62873b: tps62873@43 {
>
> Node names should be generic. See also an explanation and list of
> examples (not exhaustive) in DT specification:
> https://devicetree-specification.readthedocs.io/en/latest/chapter2-devicetree-basics.html#generic-names-recommendation
>
>
>
>
> Best regards,
> Krzysztof
>

Thanks for the review! I have posted v3.

--
Thanking You
Neha Malcom Francis