1. Patchew
  2. linux
  3. [PATCH v2 0/3] arm64: ti: Add TPS6287 nodes
  4. View patch

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

Neha Malcom Francis posted 3 patches 1 year, 4 months ago
There is a newer version of this series
[PATCH v2 1/3] arm64: boot: dts: ti: k3-am68-sk-base-board: Add LP8733 and TPS6287 nodes
Posted by Neha Malcom Francis 1 year, 4 months ago 
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 <n-francis@ti.com>
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
Re: [PATCH v2 1/3] arm64: boot: dts: ti: k3-am68-sk-base-board: Add LP8733 and TPS6287 nodes
Posted by Krzysztof Kozlowski 1 year, 4 months ago 
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 <n-francis@ti.com>
> 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
Re: [PATCH v2 1/3] arm64: boot: dts: ti: k3-am68-sk-base-board: Add LP8733 and TPS6287 nodes
Posted by Neha Malcom Francis 1 year, 4 months ago 
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 <n-francis@ti.com>
>> 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

Patchew 2.1-devel-b67e4c2

© 2016 - 2025 Red Hat, Inc.