| Message ID | 20240906-j784s4-tps6594-bootph-v1-1-c5b58d43bf04@redhat.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | arm64: dts: ti: k3-j784s4: Mark tps659413 and children as bootph-all | expand |
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>; >
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?
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) >
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>;
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(+)