| Message ID | 20240911-j784s4-tps6594-bootph-v2-1-a83526264ab1@redhat.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | arm64: dts: ti: k3-j784s4: Mark tps659413 regulators as bootph-all | expand |
Hi Andrew, On 11/09/24 22:49, Andrew Halaney wrote: > In order for the MCU domain to access this PMIC, a regulator > needs to be marked appropriately otherwise it is not seen by SPL and > therefore not configured. > > 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 | 8 ++++++++ > 1 file changed, 8 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..6ed628c2884e 100644 > --- a/arch/arm64/boot/dts/ti/k3-j784s4-evm.dts > +++ b/arch/arm64/boot/dts/ti/k3-j784s4-evm.dts > @@ -663,6 +663,7 @@ tps659413: pmic@48 { > > regulators { > bucka12: buck12 { > + bootph-all; > regulator-name = "vdd_ddr_1v1"; > regulator-min-microvolt = <1100000>; > regulator-max-microvolt = <1100000>; In my opinion, bootph-all property should come after other standard properties like regulator-name etc., as it is least important to Linux. Same comment for other nodes wherever applicable. What is your opinion? https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/tree/Documentation/devicetree/bindings/dts-coding-style.rst#n130 Thanks, Beleswar > @@ -671,6 +672,7 @@ bucka12: buck12 { > }; > > bucka3: buck3 { > + bootph-all; > regulator-name = "vdd_ram_0v85"; > regulator-min-microvolt = <850000>; > regulator-max-microvolt = <850000>; > @@ -679,6 +681,7 @@ bucka3: buck3 { > }; > > bucka4: buck4 { > + bootph-all; > regulator-name = "vdd_io_1v8"; > regulator-min-microvolt = <1800000>; > regulator-max-microvolt = <1800000>; > @@ -687,6 +690,7 @@ bucka4: buck4 { > }; > > bucka5: buck5 { > + bootph-all; > regulator-name = "vdd_mcu_0v85"; > regulator-min-microvolt = <850000>; > regulator-max-microvolt = <850000>; > @@ -695,6 +699,7 @@ bucka5: buck5 { > }; > > ldoa1: ldo1 { > + bootph-all; > regulator-name = "vdd_mcuio_1v8"; > regulator-min-microvolt = <1800000>; > regulator-max-microvolt = <1800000>; > @@ -703,6 +708,7 @@ ldoa1: ldo1 { > }; > > ldoa2: ldo2 { > + bootph-all; > regulator-name = "vdd_mcuio_3v3"; > regulator-min-microvolt = <3300000>; > regulator-max-microvolt = <3300000>; > @@ -711,6 +717,7 @@ ldoa2: ldo2 { > }; > > ldoa3: ldo3 { > + bootph-all; > regulator-name = "vds_dll_0v8"; > regulator-min-microvolt = <800000>; > regulator-max-microvolt = <800000>; > @@ -719,6 +726,7 @@ ldoa3: ldo3 { > }; > > ldoa4: ldo4 { > + bootph-all; > regulator-name = "vda_mcu_1v8"; > regulator-min-microvolt = <1800000>; > regulator-max-microvolt = <1800000>; >
On Fri, Sep 13, 2024 at 04:27:47PM GMT, Beleswar Prasad Padhi wrote: > Hi Andrew, > > On 11/09/24 22:49, Andrew Halaney wrote: > > In order for the MCU domain to access this PMIC, a regulator > > needs to be marked appropriately otherwise it is not seen by SPL and > > therefore not configured. > > > > 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 | 8 ++++++++ > > 1 file changed, 8 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..6ed628c2884e 100644 > > --- a/arch/arm64/boot/dts/ti/k3-j784s4-evm.dts > > +++ b/arch/arm64/boot/dts/ti/k3-j784s4-evm.dts > > @@ -663,6 +663,7 @@ tps659413: pmic@48 { > > regulators { > > bucka12: buck12 { > > + bootph-all; > > regulator-name = "vdd_ddr_1v1"; > > regulator-min-microvolt = <1100000>; > > regulator-max-microvolt = <1100000>; > > > In my opinion, bootph-all property should come after other standard > properties like regulator-name etc., as it is least important to Linux. Same > comment for other nodes wherever applicable. What is your opinion? > > > https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/tree/Documentation/devicetree/bindings/dts-coding-style.rst#n130 I think that does align better with the dts-coding-style doc! Looking at the tree though, the standard currently in the TI folder is to put it first. In my opinion if changing the ordering is desired it should be done in one fell swoop (outside this series). I'd do it one big patch, but I'm curious if that's decided the way forward what the TI maintainers would like to see. I can send that patch if desired. For now I think sticking with the current practice in this series makes sense until that fell swoop happens. Please let me know if you feel strongly otherwise. Thanks, Andrew
On 14/09/24 00:27, Andrew Halaney wrote: > On Fri, Sep 13, 2024 at 04:27:47PM GMT, Beleswar Prasad Padhi wrote: >> Hi Andrew, >> >> On 11/09/24 22:49, Andrew Halaney wrote: >>> In order for the MCU domain to access this PMIC, a regulator >>> needs to be marked appropriately otherwise it is not seen by SPL and >>> therefore not configured. >>> >>> 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 | 8 ++++++++ >>> 1 file changed, 8 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..6ed628c2884e 100644 >>> --- a/arch/arm64/boot/dts/ti/k3-j784s4-evm.dts >>> +++ b/arch/arm64/boot/dts/ti/k3-j784s4-evm.dts >>> @@ -663,6 +663,7 @@ tps659413: pmic@48 { >>> regulators { >>> bucka12: buck12 { >>> + bootph-all; >>> regulator-name = "vdd_ddr_1v1"; >>> regulator-min-microvolt = <1100000>; >>> regulator-max-microvolt = <1100000>; >> >> In my opinion, bootph-all property should come after other standard >> properties like regulator-name etc., as it is least important to Linux. Same >> comment for other nodes wherever applicable. What is your opinion? >> >> >> https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/tree/Documentation/devicetree/bindings/dts-coding-style.rst#n130 > I think that does align better with the dts-coding-style doc! > > Looking at the tree though, the standard currently in the TI folder > is to put it first. In my opinion if changing the ordering is desired > it should be done in one fell swoop (outside this series). I'd do There is a series[0] under review which takes care of this bootph- addition and order correction. In that series, looks like bootph- is placed at the end of the list of all standard properties. So, it is better if we align these patches to follow the same. [0]: https://lore.kernel.org/all/20240814-b4-upstream-bootph-all-v4-2-f2b462000f25@ti.com/ Thanks, Beleswar > it one big patch, but I'm curious if that's decided the way forward what > the TI maintainers would like to see. I can send that patch if desired. > > For now I think sticking with the current practice in this series > makes sense until that fell swoop happens. > > Please let me know if you feel strongly otherwise. > > Thanks, > Andrew >
On Mon, Sep 16, 2024 at 04:14:43PM GMT, Beleswar Prasad Padhi wrote: > > On 14/09/24 00:27, Andrew Halaney wrote: > > On Fri, Sep 13, 2024 at 04:27:47PM GMT, Beleswar Prasad Padhi wrote: > > > Hi Andrew, > > > > > > On 11/09/24 22:49, Andrew Halaney wrote: > > > > In order for the MCU domain to access this PMIC, a regulator > > > > needs to be marked appropriately otherwise it is not seen by SPL and > > > > therefore not configured. > > > > > > > > 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 | 8 ++++++++ > > > > 1 file changed, 8 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..6ed628c2884e 100644 > > > > --- a/arch/arm64/boot/dts/ti/k3-j784s4-evm.dts > > > > +++ b/arch/arm64/boot/dts/ti/k3-j784s4-evm.dts > > > > @@ -663,6 +663,7 @@ tps659413: pmic@48 { > > > > regulators { > > > > bucka12: buck12 { > > > > + bootph-all; > > > > regulator-name = "vdd_ddr_1v1"; > > > > regulator-min-microvolt = <1100000>; > > > > regulator-max-microvolt = <1100000>; > > > > > > In my opinion, bootph-all property should come after other standard > > > properties like regulator-name etc., as it is least important to Linux. Same > > > comment for other nodes wherever applicable. What is your opinion? > > > > > > > > > https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/tree/Documentation/devicetree/bindings/dts-coding-style.rst#n130 > > I think that does align better with the dts-coding-style doc! > > > > Looking at the tree though, the standard currently in the TI folder > > is to put it first. In my opinion if changing the ordering is desired > > it should be done in one fell swoop (outside this series). I'd do > > > There is a series[0] under review which takes care of this bootph- addition > and order correction. In that series, looks like bootph- is placed at the > end of the list of all standard properties. So, it is better if we align > these patches to follow the same. > > [0]: https://lore.kernel.org/all/20240814-b4-upstream-bootph-all-v4-2-f2b462000f25@ti.com/ > Ahh, ok. I'll post v3 with things ordered in that fashion! Thanks, Andrew
diff --git a/arch/arm64/boot/dts/ti/k3-j784s4-evm.dts b/arch/arm64/boot/dts/ti/k3-j784s4-evm.dts index 6695ebbcb4d0..6ed628c2884e 100644 --- a/arch/arm64/boot/dts/ti/k3-j784s4-evm.dts +++ b/arch/arm64/boot/dts/ti/k3-j784s4-evm.dts @@ -663,6 +663,7 @@ tps659413: pmic@48 { regulators { bucka12: buck12 { + bootph-all; regulator-name = "vdd_ddr_1v1"; regulator-min-microvolt = <1100000>; regulator-max-microvolt = <1100000>; @@ -671,6 +672,7 @@ bucka12: buck12 { }; bucka3: buck3 { + bootph-all; regulator-name = "vdd_ram_0v85"; regulator-min-microvolt = <850000>; regulator-max-microvolt = <850000>; @@ -679,6 +681,7 @@ bucka3: buck3 { }; bucka4: buck4 { + bootph-all; regulator-name = "vdd_io_1v8"; regulator-min-microvolt = <1800000>; regulator-max-microvolt = <1800000>; @@ -687,6 +690,7 @@ bucka4: buck4 { }; bucka5: buck5 { + bootph-all; regulator-name = "vdd_mcu_0v85"; regulator-min-microvolt = <850000>; regulator-max-microvolt = <850000>; @@ -695,6 +699,7 @@ bucka5: buck5 { }; ldoa1: ldo1 { + bootph-all; regulator-name = "vdd_mcuio_1v8"; regulator-min-microvolt = <1800000>; regulator-max-microvolt = <1800000>; @@ -703,6 +708,7 @@ ldoa1: ldo1 { }; ldoa2: ldo2 { + bootph-all; regulator-name = "vdd_mcuio_3v3"; regulator-min-microvolt = <3300000>; regulator-max-microvolt = <3300000>; @@ -711,6 +717,7 @@ ldoa2: ldo2 { }; ldoa3: ldo3 { + bootph-all; regulator-name = "vds_dll_0v8"; regulator-min-microvolt = <800000>; regulator-max-microvolt = <800000>; @@ -719,6 +726,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, a regulator needs to be marked appropriately otherwise it is not seen by SPL and therefore not configured. 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 | 8 ++++++++ 1 file changed, 8 insertions(+)