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

From: Neha Malcom Francis
Date: Tue May 28 2024 - 00:03:36 EST


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@xxxxxx>
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