[PATCH RFC/RFT 0/2] arm64: dts: ti: k3-j784s4: Mark tps659413 and children as bootph-all

[PATCH RFC/RFT 0/2] arm64: dts: ti: k3-j784s4: Mark tps659413 and children as bootph-all

[Andrew Halaney]

This series marks tps659413 and its children as bootph-all in order for
the nodes to be accessible during MCU's u-boot SPL.

This in turn is desired since the tps659413 needs its MCU ESM
state machine setup in order for the watchdog to reset the board.

This took me a little while to track down, as enabling the ESM, TPS6594,
etc in u-boot would result in the below boot failure:

    U-Boot SPL 2024.10-rc4-00007-g44b12cbcd1b3-dirty (Sep 06 2024 - 14:25:52 -0500)
    SYSFW ABI: 3.1 (firmware rev 0x0009 '9.2.4--v09.02.04 (Kool Koala)')
    Initialized 4 DRAM controllers
    SPL initial stack usage: 13408 bytes
    ### ERROR ### Please RESET the board ###

Which turns out to actually have failed far earlier in spl_early_init(),
due to these nodes not being accessible in u-boot. That's hard to tell
though since console isn't setup until later (and for that reason I
think spl_early_init()'s return value in j784s4_init.c isn't
evaluated since a panic() at that point would leave a user with *no*
information at all).

I've tested this in conjunction with a u-boot series which I'll link in
a follow-up response on the k3-j784s4-evm. I'd appreciate someone testing
on the k3-am69-sk at a minimum, as it should suffer the same fate if things
aren't setup appropriately.

Signed-off-by: Andrew Halaney <ahalaney@redhat.com>
---
Andrew Halaney (2):
      arm64: dts: ti: k3-j784s4-evm: Mark tps659413 and children as bootph-all
      arm64: dts: ti: k3-am69-sk: Mark tps659413 and children as bootph-all

 arch/arm64/boot/dts/ti/k3-am69-sk.dts    | 11 +++++++++++
 arch/arm64/boot/dts/ti/k3-j784s4-evm.dts | 11 +++++++++++
 2 files changed, 22 insertions(+)
---
base-commit: 9aaeb87ce1e966169a57f53a02ba05b30880ffb8
change-id: 20240906-j784s4-tps6594-bootph-19d3f00fb98a

Best regards,
-- 
Andrew Halaney <ahalaney@redhat.com>

[PATCH RFC/RFT 1/2] arm64: dts: ti: k3-j784s4-evm: Mark tps659413 and children as bootph-all

[Andrew Halaney]

In order for the MCU domain to access this PMIC and its children in
u-boot SPL, the nodes need to be marked appropriately otherwise they
are not seen by SPL.

This is necessary if the MCU domain is to program the TPS6594 MCU ESM
state machine, which is required to wire up the watchdog in a manner
that will reset the board.

Signed-off-by: Andrew Halaney <ahalaney@redhat.com>
---
 arch/arm64/boot/dts/ti/k3-j784s4-evm.dts | 11 +++++++++++
 1 file changed, 11 insertions(+)

diff --git a/arch/arm64/boot/dts/ti/k3-j784s4-evm.dts b/arch/arm64/boot/dts/ti/k3-j784s4-evm.dts
index 6695ebbcb4d0..044a428136df 100644
--- a/arch/arm64/boot/dts/ti/k3-j784s4-evm.dts
+++ b/arch/arm64/boot/dts/ti/k3-j784s4-evm.dts
@@ -642,6 +642,7 @@ eeprom@50 {
 	};
 
 	tps659413: pmic@48 {
+		bootph-all;
 		compatible = "ti,tps6594-q1";
 		reg = <0x48>;
 		system-power-controller;
@@ -662,7 +663,10 @@ tps659413: pmic@48 {
 		ldo4-supply = <&vsys_3v3>;
 
 		regulators {
+			bootph-all;
+
 			bucka12: buck12 {
+				bootph-all;
 				regulator-name = "vdd_ddr_1v1";
 				regulator-min-microvolt = <1100000>;
 				regulator-max-microvolt = <1100000>;
@@ -671,6 +675,7 @@ bucka12: buck12 {
 			};
 
 			bucka3: buck3 {
+				bootph-all;
 				regulator-name = "vdd_ram_0v85";
 				regulator-min-microvolt = <850000>;
 				regulator-max-microvolt = <850000>;
@@ -679,6 +684,7 @@ bucka3: buck3 {
 			};
 
 			bucka4: buck4 {
+				bootph-all;
 				regulator-name = "vdd_io_1v8";
 				regulator-min-microvolt = <1800000>;
 				regulator-max-microvolt = <1800000>;
@@ -687,6 +693,7 @@ bucka4: buck4 {
 			};
 
 			bucka5: buck5 {
+				bootph-all;
 				regulator-name = "vdd_mcu_0v85";
 				regulator-min-microvolt = <850000>;
 				regulator-max-microvolt = <850000>;
@@ -695,6 +702,7 @@ bucka5: buck5 {
 			};
 
 			ldoa1: ldo1 {
+				bootph-all;
 				regulator-name = "vdd_mcuio_1v8";
 				regulator-min-microvolt = <1800000>;
 				regulator-max-microvolt = <1800000>;
@@ -703,6 +711,7 @@ ldoa1: ldo1 {
 			};
 
 			ldoa2: ldo2 {
+				bootph-all;
 				regulator-name = "vdd_mcuio_3v3";
 				regulator-min-microvolt = <3300000>;
 				regulator-max-microvolt = <3300000>;
@@ -711,6 +720,7 @@ ldoa2: ldo2 {
 			};
 
 			ldoa3: ldo3 {
+				bootph-all;
 				regulator-name = "vds_dll_0v8";
 				regulator-min-microvolt = <800000>;
 				regulator-max-microvolt = <800000>;
@@ -719,6 +729,7 @@ ldoa3: ldo3 {
 			};
 
 			ldoa4: ldo4 {
+				bootph-all;
 				regulator-name = "vda_mcu_1v8";
 				regulator-min-microvolt = <1800000>;
 				regulator-max-microvolt = <1800000>;

-- 
2.46.0

[PATCH RFC/RFT 2/2] arm64: dts: ti: k3-am69-sk: Mark tps659413 and children as bootph-all

[Andrew Halaney]

In order for the MCU domain to access this PMIC and its children in
u-boot SPL, the nodes need to be marked appropriately otherwise they
are not seen by SPL.

This is necessary if the MCU domain is to program the TPS6594 MCU ESM
state machine, which is required to wire up the watchdog in a manner
that will reset the board.

Signed-off-by: Andrew Halaney <ahalaney@redhat.com>
---
 arch/arm64/boot/dts/ti/k3-am69-sk.dts | 11 +++++++++++
 1 file changed, 11 insertions(+)

diff --git a/arch/arm64/boot/dts/ti/k3-am69-sk.dts b/arch/arm64/boot/dts/ti/k3-am69-sk.dts
index 1e36965a1403..1c3427856982 100644
--- a/arch/arm64/boot/dts/ti/k3-am69-sk.dts
+++ b/arch/arm64/boot/dts/ti/k3-am69-sk.dts
@@ -729,6 +729,7 @@ eeprom@51 {
 	};
 
 	tps659413: pmic@48 {
+		bootph-all;
 		compatible = "ti,tps6594-q1";
 		reg = <0x48>;
 		system-power-controller;
@@ -749,7 +750,10 @@ tps659413: pmic@48 {
 		ldo4-supply = <&vsys_3v3>;
 
 		regulators {
+			bootph-all;
+
 			bucka12: buck12 {
+				bootph-all;
 				regulator-name = "vdd_ddr_1v1";
 				regulator-min-microvolt = <1100000>;
 				regulator-max-microvolt = <1100000>;
@@ -758,6 +762,7 @@ bucka12: buck12 {
 			};
 
 			bucka3: buck3 {
+				bootph-all;
 				regulator-name = "vdd_ram_0v85";
 				regulator-min-microvolt = <850000>;
 				regulator-max-microvolt = <850000>;
@@ -766,6 +771,7 @@ bucka3: buck3 {
 			};
 
 			bucka4: buck4 {
+				bootph-all;
 				regulator-name = "vdd_io_1v8";
 				regulator-min-microvolt = <1800000>;
 				regulator-max-microvolt = <1800000>;
@@ -774,6 +780,7 @@ bucka4: buck4 {
 			};
 
 			bucka5: buck5 {
+				bootph-all;
 				regulator-name = "vdd_mcu_0v85";
 				regulator-min-microvolt = <850000>;
 				regulator-max-microvolt = <850000>;
@@ -782,6 +789,7 @@ bucka5: buck5 {
 			};
 
 			ldoa1: ldo1 {
+				bootph-all;
 				regulator-name = "vdd_mcuio_1v8";
 				regulator-min-microvolt = <1800000>;
 				regulator-max-microvolt = <1800000>;
@@ -790,6 +798,7 @@ ldoa1: ldo1 {
 			};
 
 			ldoa2: ldo2 {
+				bootph-all;
 				regulator-name = "vdd_mcuio_3v3";
 				regulator-min-microvolt = <3300000>;
 				regulator-max-microvolt = <3300000>;
@@ -798,6 +807,7 @@ ldoa2: ldo2 {
 			};
 
 			ldoa3: ldo3 {
+				bootph-all;
 				regulator-name = "vds_dll_0v8";
 				regulator-min-microvolt = <800000>;
 				regulator-max-microvolt = <800000>;
@@ -806,6 +816,7 @@ ldoa3: ldo3 {
 			};
 
 			ldoa4: ldo4 {
+				bootph-all;
 				regulator-name = "vda_mcu_1v8";
 				regulator-min-microvolt = <1800000>;
 				regulator-max-microvolt = <1800000>;

-- 
2.46.0

Re: [PATCH RFC/RFT 1/2] arm64: dts: ti: k3-j784s4-evm: Mark tps659413 and children as bootph-all

[Kumar, Udit]

Thanks for your patch Andrew


On 9/7/2024 2:51 AM, Andrew Halaney wrote:
> In order for the MCU domain to access this PMIC and its children in
> u-boot SPL, the nodes need to be marked appropriately otherwise they
> are not seen by SPL.
>
> This is necessary if the MCU domain is to program the TPS6594 MCU ESM
> state machine, which is required to wire up the watchdog in a manner
> that will reset the board.
>
> Signed-off-by: Andrew Halaney <ahalaney@redhat.com>
> ---
>   arch/arm64/boot/dts/ti/k3-j784s4-evm.dts | 11 +++++++++++
>   1 file changed, 11 insertions(+)
>
> diff --git a/arch/arm64/boot/dts/ti/k3-j784s4-evm.dts b/arch/arm64/boot/dts/ti/k3-j784s4-evm.dts
> index 6695ebbcb4d0..044a428136df 100644
> --- a/arch/arm64/boot/dts/ti/k3-j784s4-evm.dts
> +++ b/arch/arm64/boot/dts/ti/k3-j784s4-evm.dts
> @@ -642,6 +642,7 @@ eeprom@50 {
>   	};
>   
>   	tps659413: pmic@48 {
> +		bootph-all;
>   		compatible = "ti,tps6594-q1";
>   		reg = <0x48>;
>   		system-power-controller;
> @@ -662,7 +663,10 @@ tps659413: pmic@48 {
>   		ldo4-supply = <&vsys_3v3>;
>   
>   		regulators {
> +			bootph-all;
> +
>   			bucka12: buck12 {
> +				bootph-all;


Add bootph in on regulator node should be enough,

As I see SPL/u-boot does not need all nodes.


FYI,

Similar series in review

https://lore.kernel.org/all/20240814-b4-upstream-bootph-all-v4-0-f2b462000f25@ti.com/ 


>   				regulator-name = "vdd_ddr_1v1";
>   				regulator-min-microvolt = <1100000>;
>   				regulator-max-microvolt = <1100000>;
> @@ -671,6 +675,7 @@ bucka12: buck12 {
>   			};
>   
>   			bucka3: buck3 {
> +				bootph-all;
>   				regulator-name = "vdd_ram_0v85";
>   				regulator-min-microvolt = <850000>;
>   				regulator-max-microvolt = <850000>;
> @@ -679,6 +684,7 @@ bucka3: buck3 {
>   			};
>   
>   			bucka4: buck4 {
> +				bootph-all;
>   				regulator-name = "vdd_io_1v8";
>   				regulator-min-microvolt = <1800000>;
>   				regulator-max-microvolt = <1800000>;
> @@ -687,6 +693,7 @@ bucka4: buck4 {
>   			};
>   
>   			bucka5: buck5 {
> +				bootph-all;
>   				regulator-name = "vdd_mcu_0v85";
>   				regulator-min-microvolt = <850000>;
>   				regulator-max-microvolt = <850000>;
> @@ -695,6 +702,7 @@ bucka5: buck5 {
>   			};
>   
>   			ldoa1: ldo1 {
> +				bootph-all;
>   				regulator-name = "vdd_mcuio_1v8";
>   				regulator-min-microvolt = <1800000>;
>   				regulator-max-microvolt = <1800000>;
> @@ -703,6 +711,7 @@ ldoa1: ldo1 {
>   			};
>   
>   			ldoa2: ldo2 {
> +				bootph-all;
>   				regulator-name = "vdd_mcuio_3v3";
>   				regulator-min-microvolt = <3300000>;
>   				regulator-max-microvolt = <3300000>;
> @@ -711,6 +720,7 @@ ldoa2: ldo2 {
>   			};
>   
>   			ldoa3: ldo3 {
> +				bootph-all;
>   				regulator-name = "vds_dll_0v8";
>   				regulator-min-microvolt = <800000>;
>   				regulator-max-microvolt = <800000>;
> @@ -719,6 +729,7 @@ ldoa3: ldo3 {
>   			};
>   
>   			ldoa4: ldo4 {
> +				bootph-all;
>   				regulator-name = "vda_mcu_1v8";
>   				regulator-min-microvolt = <1800000>;
>   				regulator-max-microvolt = <1800000>;
>

Re: [PATCH RFC/RFT 0/2] arm64: dts: ti: k3-j784s4: Mark tps659413 and children as bootph-all

[Andrew Halaney]

On Fri, Sep 06, 2024 at 04:21:01PM GMT, Andrew Halaney wrote:
> This series marks tps659413 and its children as bootph-all in order for
> the nodes to be accessible during MCU's u-boot SPL.
> 
> This in turn is desired since the tps659413 needs its MCU ESM
> state machine setup in order for the watchdog to reset the board.
> 
> This took me a little while to track down, as enabling the ESM, TPS6594,
> etc in u-boot would result in the below boot failure:
> 
>     U-Boot SPL 2024.10-rc4-00007-g44b12cbcd1b3-dirty (Sep 06 2024 - 14:25:52 -0500)
>     SYSFW ABI: 3.1 (firmware rev 0x0009 '9.2.4--v09.02.04 (Kool Koala)')
>     Initialized 4 DRAM controllers
>     SPL initial stack usage: 13408 bytes
>     ### ERROR ### Please RESET the board ###
> 
> Which turns out to actually have failed far earlier in spl_early_init(),
> due to these nodes not being accessible in u-boot. That's hard to tell
> though since console isn't setup until later (and for that reason I
> think spl_early_init()'s return value in j784s4_init.c isn't
> evaluated since a panic() at that point would leave a user with *no*
> information at all).
> 
> I've tested this in conjunction with a u-boot series which I'll link in
> a follow-up response on the k3-j784s4-evm. I'd appreciate someone testing
> on the k3-am69-sk at a minimum, as it should suffer the same fate if things
> aren't setup appropriately.
> 
> Signed-off-by: Andrew Halaney <ahalaney@redhat.com>

Better late than never...

Link: https://lore.kernel.org/u-boot/20240906-j784s4-esm-enable-v1-0-b83b17d5a744@redhat.com/

> ---
> Andrew Halaney (2):
>       arm64: dts: ti: k3-j784s4-evm: Mark tps659413 and children as bootph-all
>       arm64: dts: ti: k3-am69-sk: Mark tps659413 and children as bootph-all
> 
>  arch/arm64/boot/dts/ti/k3-am69-sk.dts    | 11 +++++++++++
>  arch/arm64/boot/dts/ti/k3-j784s4-evm.dts | 11 +++++++++++
>  2 files changed, 22 insertions(+)
> ---
> base-commit: 9aaeb87ce1e966169a57f53a02ba05b30880ffb8
> change-id: 20240906-j784s4-tps6594-bootph-19d3f00fb98a
> 
> Best regards,
> -- 
> Andrew Halaney <ahalaney@redhat.com>
> 

Re: [PATCH RFC/RFT 1/2] arm64: dts: ti: k3-j784s4-evm: Mark tps659413 and children as bootph-all

[Andrew Halaney]

On Sat, Sep 07, 2024 at 11:04:50AM GMT, Kumar, Udit wrote:
> Thanks for your patch Andrew
> 
> 
> On 9/7/2024 2:51 AM, Andrew Halaney wrote:
> > In order for the MCU domain to access this PMIC and its children in
> > u-boot SPL, the nodes need to be marked appropriately otherwise they
> > are not seen by SPL.
> > 
> > This is necessary if the MCU domain is to program the TPS6594 MCU ESM
> > state machine, which is required to wire up the watchdog in a manner
> > that will reset the board.
> > 
> > Signed-off-by: Andrew Halaney <ahalaney@redhat.com>
> > ---
> >   arch/arm64/boot/dts/ti/k3-j784s4-evm.dts | 11 +++++++++++
> >   1 file changed, 11 insertions(+)
> > 
> > diff --git a/arch/arm64/boot/dts/ti/k3-j784s4-evm.dts b/arch/arm64/boot/dts/ti/k3-j784s4-evm.dts
> > index 6695ebbcb4d0..044a428136df 100644
> > --- a/arch/arm64/boot/dts/ti/k3-j784s4-evm.dts
> > +++ b/arch/arm64/boot/dts/ti/k3-j784s4-evm.dts
> > @@ -642,6 +642,7 @@ eeprom@50 {
> >   	};
> >   	tps659413: pmic@48 {
> > +		bootph-all;
> >   		compatible = "ti,tps6594-q1";
> >   		reg = <0x48>;
> >   		system-power-controller;
> > @@ -662,7 +663,10 @@ tps659413: pmic@48 {
> >   		ldo4-supply = <&vsys_3v3>;
> >   		regulators {
> > +			bootph-all;
> > +
> >   			bucka12: buck12 {
> > +				bootph-all;
> 
> 
> Add bootph in on regulator node should be enough,
> 
> As I see SPL/u-boot does not need all nodes.

Ahhh, I finally see now, all parents of a bootph-* node get that
property. Makes sense.

Would you rather see it in the regulators node, or all of the actual
regulators (bucka12, buacka3... etc)?

The former is all that's *needed* to get the PMIC ESM probing and
programmed. The latter makes sense to me if we want to actual use the
regulators in the future in that context... Doing just *one* of the
regulators seems odd to me though, someone may want a different one,
so if we describe one to SPL we may as well describe all.

What are your thoughts?

Re: [PATCH RFC/RFT 1/2] arm64: dts: ti: k3-j784s4-evm: Mark tps659413 and children as bootph-all

[Kumar, Udit]

Hi Andrew,

On 9/10/2024 10:50 PM, Andrew Halaney wrote:
> On Sat, Sep 07, 2024 at 11:04:50AM GMT, Kumar, Udit wrote:
>> Thanks for your patch Andrew
>>
>>
>> On 9/7/2024 2:51 AM, Andrew Halaney wrote:
>>> In order for the MCU domain to access this PMIC and its children in
>>> u-boot SPL, the nodes need to be marked appropriately otherwise they
>>> are not seen by SPL.
>>>
>>> This is necessary if the MCU domain is to program the TPS6594 MCU ESM
>>> state machine, which is required to wire up the watchdog in a manner
>>> that will reset the board.
>>>
>>> Signed-off-by: Andrew Halaney <ahalaney@redhat.com>
>>> ---
>>>    arch/arm64/boot/dts/ti/k3-j784s4-evm.dts | 11 +++++++++++
>>>    1 file changed, 11 insertions(+)
>>>
>>> diff --git a/arch/arm64/boot/dts/ti/k3-j784s4-evm.dts b/arch/arm64/boot/dts/ti/k3-j784s4-evm.dts
>>> index 6695ebbcb4d0..044a428136df 100644
>>> --- a/arch/arm64/boot/dts/ti/k3-j784s4-evm.dts
>>> +++ b/arch/arm64/boot/dts/ti/k3-j784s4-evm.dts
>>> @@ -642,6 +642,7 @@ eeprom@50 {
>>>    	};
>>>    	tps659413: pmic@48 {
>>> +		bootph-all;
>>>    		compatible = "ti,tps6594-q1";
>>>    		reg = <0x48>;
>>>    		system-power-controller;
>>> @@ -662,7 +663,10 @@ tps659413: pmic@48 {
>>>    		ldo4-supply = <&vsys_3v3>;
>>>    		regulators {
>>> +			bootph-all;
>>> +
>>>    			bucka12: buck12 {
>>> +				bootph-all;
>>
>> Add bootph in on regulator node should be enough,
>>
>> As I see SPL/u-boot does not need all nodes.
> Ahhh, I finally see now, all parents of a bootph-* node get that
> property. Makes sense.
>
> Would you rather see it in the regulators node, or all of the actual
> regulators (bucka12, buacka3... etc)?
>
> The former is all that's *needed* to get the PMIC ESM probing and
> programmed. The latter makes sense to me if we want to actual use the
> regulators in the future in that context... Doing just *one* of the
> regulators seems odd to me though, someone may want a different one,
> so if we describe one to SPL we may as well describe all.
>
> What are your thoughts?


For now, adding boothph for bucka12 regulator is enough

but other nodes may be needed in future so i suggest to keep in

all regulators nodes ( bucka12, buacka3... etc)


>