diff mbox series

arm64: dts: ti: k3-j784s4-main: align watchdog clocks

Message ID 20240805174330.2132717-2-echanude@redhat.com (mailing list archive)
State New, archived
Headers show
Series arm64: dts: ti: k3-j784s4-main: align watchdog clocks | expand

Commit Message

Eric Chanudet Aug. 5, 2024, 5:42 p.m. UTC
assigned-clock sets DEV_RTIx_RTI_CLK(id:0) whereas clocks sets
DEV_RTIx_RTI_CLK_PARENT_GLUELOGIC_HFOSC0_CLKOUT(id:1)[1]. This does not
look right, the timers in the driver assume a max frequency of 32kHz for
the heartbeat (HFOSC0 is 19.2MHz on j784s4-evm).

With this change, WDIOC_GETTIMELEFT return coherent time left
(DEFAULT_HEARTBEAT=60, reports 60s upon opening the cdev).

[1] http://downloads.ti.com/tisci/esd/latest/5_soc_doc/j784s4/clocks.html#clocks-for-rti0-device

Fixes: caae599de8c6 ("arm64: dts: ti: k3-j784s4-main: Add the main domain watchdog instances")
Suggested-by: Andrew Halaney <ahalaney@redhat.com>
Signed-off-by: Eric Chanudet <echanude@redhat.com>
---
I could not get the watchdog to do more than reporting 0x32 in
RTIWDSTATUS. Setting RTIWWDRXCTRL[0:3] to generate a reset instead of an
interrupt (0x5) didn't trigger a reset either when the window expired.

 arch/arm64/boot/dts/ti/k3-j784s4-main.dtsi | 38 +++++++++++-----------
 1 file changed, 19 insertions(+), 19 deletions(-)

Comments

Eric Chanudet Aug. 20, 2024, 10:01 p.m. UTC | #1
On Mon, Aug 05, 2024 at 01:42:51PM GMT, Eric Chanudet wrote:
> assigned-clock sets DEV_RTIx_RTI_CLK(id:0) whereas clocks sets
> DEV_RTIx_RTI_CLK_PARENT_GLUELOGIC_HFOSC0_CLKOUT(id:1)[1]. This does not
> look right, the timers in the driver assume a max frequency of 32kHz for
> the heartbeat (HFOSC0 is 19.2MHz on j784s4-evm).
> 
> With this change, WDIOC_GETTIMELEFT return coherent time left
> (DEFAULT_HEARTBEAT=60, reports 60s upon opening the cdev).
> 
> [1] http://downloads.ti.com/tisci/esd/latest/5_soc_doc/j784s4/clocks.html#clocks-for-rti0-device
> 
> Fixes: caae599de8c6 ("arm64: dts: ti: k3-j784s4-main: Add the main domain watchdog instances")
> Suggested-by: Andrew Halaney <ahalaney@redhat.com>
> Signed-off-by: Eric Chanudet <echanude@redhat.com>

Gentle ping and update to the test comment.

> ---
> I could not get the watchdog to do more than reporting 0x32 in
> RTIWDSTATUS. Setting RTIWWDRXCTRL[0:3] to generate a reset instead of an
> interrupt (0x5) didn't trigger a reset either when the window expired.

Re-testing using u-boot from the BSP (2023.04) has the board reset as
expected when the watchdog expires and WDIOC_GETTIMELEFT report the time
left coherently with this patch until that happens.

I initially had a u-boot with a DT lacking:
	"mcu_esm: esm@40800000"
and I could reproduce the board not resetting by commenting in its
description:
	"ti,esm-pins = <95>;"

I don't understand why that is on the other hand. The TRM says ESM0
ERR_O drives the SOC_SAFETY_ERRORn pin, which goes to the PMIC GPIO3 on
the schematic _and_ to MCU_ESM0 as an error input event. The tps6594-esm
module is probing successfully and it sets both ESM_SOC_EN|ESM_SOC_ENDRV
and ESM_SOC_START, so I would expect the PMIC to reset the board without
MCU_ESM0 being described or configured by u-boot.

Best,

> 
>  arch/arm64/boot/dts/ti/k3-j784s4-main.dtsi | 38 +++++++++++-----------
>  1 file changed, 19 insertions(+), 19 deletions(-)
> 
> diff --git a/arch/arm64/boot/dts/ti/k3-j784s4-main.dtsi b/arch/arm64/boot/dts/ti/k3-j784s4-main.dtsi
> index f170f80f00c1..6c014d335f2c 100644
> --- a/arch/arm64/boot/dts/ti/k3-j784s4-main.dtsi
> +++ b/arch/arm64/boot/dts/ti/k3-j784s4-main.dtsi
> @@ -2429,7 +2429,7 @@ main_esm: esm@700000 {
>  	watchdog0: watchdog@2200000 {
>  		compatible = "ti,j7-rti-wdt";
>  		reg = <0x00 0x2200000 0x00 0x100>;
> -		clocks = <&k3_clks 348 1>;
> +		clocks = <&k3_clks 348 0>;
>  		power-domains = <&k3_pds 348 TI_SCI_PD_EXCLUSIVE>;
>  		assigned-clocks = <&k3_clks 348 0>;
>  		assigned-clock-parents = <&k3_clks 348 4>;
> @@ -2438,7 +2438,7 @@ watchdog0: watchdog@2200000 {
>  	watchdog1: watchdog@2210000 {
>  		compatible = "ti,j7-rti-wdt";
>  		reg = <0x00 0x2210000 0x00 0x100>;
> -		clocks = <&k3_clks 349 1>;
> +		clocks = <&k3_clks 349 0>;
>  		power-domains = <&k3_pds 349 TI_SCI_PD_EXCLUSIVE>;
>  		assigned-clocks = <&k3_clks 349 0>;
>  		assigned-clock-parents = <&k3_clks 349 4>;
> @@ -2447,7 +2447,7 @@ watchdog1: watchdog@2210000 {
>  	watchdog2: watchdog@2220000 {
>  		compatible = "ti,j7-rti-wdt";
>  		reg = <0x00 0x2220000 0x00 0x100>;
> -		clocks = <&k3_clks 350 1>;
> +		clocks = <&k3_clks 350 0>;
>  		power-domains = <&k3_pds 350 TI_SCI_PD_EXCLUSIVE>;
>  		assigned-clocks = <&k3_clks 350 0>;
>  		assigned-clock-parents = <&k3_clks 350 4>;
> @@ -2456,7 +2456,7 @@ watchdog2: watchdog@2220000 {
>  	watchdog3: watchdog@2230000 {
>  		compatible = "ti,j7-rti-wdt";
>  		reg = <0x00 0x2230000 0x00 0x100>;
> -		clocks = <&k3_clks 351 1>;
> +		clocks = <&k3_clks 351 0>;
>  		power-domains = <&k3_pds 351 TI_SCI_PD_EXCLUSIVE>;
>  		assigned-clocks = <&k3_clks 351 0>;
>  		assigned-clock-parents = <&k3_clks 351 4>;
> @@ -2465,7 +2465,7 @@ watchdog3: watchdog@2230000 {
>  	watchdog4: watchdog@2240000 {
>  		compatible = "ti,j7-rti-wdt";
>  		reg = <0x00 0x2240000 0x00 0x100>;
> -		clocks = <&k3_clks 352 1>;
> +		clocks = <&k3_clks 352 0>;
>  		power-domains = <&k3_pds 352 TI_SCI_PD_EXCLUSIVE>;
>  		assigned-clocks = <&k3_clks 352 0>;
>  		assigned-clock-parents = <&k3_clks 352 4>;
> @@ -2474,7 +2474,7 @@ watchdog4: watchdog@2240000 {
>  	watchdog5: watchdog@2250000 {
>  		compatible = "ti,j7-rti-wdt";
>  		reg = <0x00 0x2250000 0x00 0x100>;
> -		clocks = <&k3_clks 353 1>;
> +		clocks = <&k3_clks 353 0>;
>  		power-domains = <&k3_pds 353 TI_SCI_PD_EXCLUSIVE>;
>  		assigned-clocks = <&k3_clks 353 0>;
>  		assigned-clock-parents = <&k3_clks 353 4>;
> @@ -2483,7 +2483,7 @@ watchdog5: watchdog@2250000 {
>  	watchdog6: watchdog@2260000 {
>  		compatible = "ti,j7-rti-wdt";
>  		reg = <0x00 0x2260000 0x00 0x100>;
> -		clocks = <&k3_clks 354 1>;
> +		clocks = <&k3_clks 354 0>;
>  		power-domains = <&k3_pds 354 TI_SCI_PD_EXCLUSIVE>;
>  		assigned-clocks = <&k3_clks 354 0>;
>  		assigned-clock-parents = <&k3_clks 354 4>;
> @@ -2492,7 +2492,7 @@ watchdog6: watchdog@2260000 {
>  	watchdog7: watchdog@2270000 {
>  		compatible = "ti,j7-rti-wdt";
>  		reg = <0x00 0x2270000 0x00 0x100>;
> -		clocks = <&k3_clks 355 1>;
> +		clocks = <&k3_clks 355 0>;
>  		power-domains = <&k3_pds 355 TI_SCI_PD_EXCLUSIVE>;
>  		assigned-clocks = <&k3_clks 355 0>;
>  		assigned-clock-parents = <&k3_clks 355 4>;
> @@ -2506,7 +2506,7 @@ watchdog7: watchdog@2270000 {
>  	watchdog8: watchdog@22f0000 {
>  		compatible = "ti,j7-rti-wdt";
>  		reg = <0x00 0x22f0000 0x00 0x100>;
> -		clocks = <&k3_clks 360 1>;
> +		clocks = <&k3_clks 360 0>;
>  		power-domains = <&k3_pds 360 TI_SCI_PD_EXCLUSIVE>;
>  		assigned-clocks = <&k3_clks 360 0>;
>  		assigned-clock-parents = <&k3_clks 360 4>;
> @@ -2517,7 +2517,7 @@ watchdog8: watchdog@22f0000 {
>  	watchdog9: watchdog@2300000 {
>  		compatible = "ti,j7-rti-wdt";
>  		reg = <0x00 0x2300000 0x00 0x100>;
> -		clocks = <&k3_clks 356 1>;
> +		clocks = <&k3_clks 356 0>;
>  		power-domains = <&k3_pds 356 TI_SCI_PD_EXCLUSIVE>;
>  		assigned-clocks = <&k3_clks 356 0>;
>  		assigned-clock-parents = <&k3_clks 356 4>;
> @@ -2528,7 +2528,7 @@ watchdog9: watchdog@2300000 {
>  	watchdog10: watchdog@2310000 {
>  		compatible = "ti,j7-rti-wdt";
>  		reg = <0x00 0x2310000 0x00 0x100>;
> -		clocks = <&k3_clks 357 1>;
> +		clocks = <&k3_clks 357 0>;
>  		power-domains = <&k3_pds 357 TI_SCI_PD_EXCLUSIVE>;
>  		assigned-clocks = <&k3_clks 357 0>;
>  		assigned-clock-parents = <&k3_clks 357 4>;
> @@ -2539,7 +2539,7 @@ watchdog10: watchdog@2310000 {
>  	watchdog11: watchdog@2320000 {
>  		compatible = "ti,j7-rti-wdt";
>  		reg = <0x00 0x2320000 0x00 0x100>;
> -		clocks = <&k3_clks 358 1>;
> +		clocks = <&k3_clks 358 0>;
>  		power-domains = <&k3_pds 358 TI_SCI_PD_EXCLUSIVE>;
>  		assigned-clocks = <&k3_clks 358 0>;
>  		assigned-clock-parents = <&k3_clks 358 4>;
> @@ -2550,7 +2550,7 @@ watchdog11: watchdog@2320000 {
>  	watchdog12: watchdog@2330000 {
>  		compatible = "ti,j7-rti-wdt";
>  		reg = <0x00 0x2330000 0x00 0x100>;
> -		clocks = <&k3_clks 359 1>;
> +		clocks = <&k3_clks 359 0>;
>  		power-domains = <&k3_pds 359 TI_SCI_PD_EXCLUSIVE>;
>  		assigned-clocks = <&k3_clks 359 0>;
>  		assigned-clock-parents = <&k3_clks 359 4>;
> @@ -2561,7 +2561,7 @@ watchdog12: watchdog@2330000 {
>  	watchdog13: watchdog@23c0000 {
>  		compatible = "ti,j7-rti-wdt";
>  		reg = <0x00 0x23c0000 0x00 0x100>;
> -		clocks = <&k3_clks 361 1>;
> +		clocks = <&k3_clks 361 0>;
>  		power-domains = <&k3_pds 361 TI_SCI_PD_EXCLUSIVE>;
>  		assigned-clocks = <&k3_clks 361 0>;
>  		assigned-clock-parents = <&k3_clks 361 4>;
> @@ -2572,7 +2572,7 @@ watchdog13: watchdog@23c0000 {
>  	watchdog14: watchdog@23d0000 {
>  		compatible = "ti,j7-rti-wdt";
>  		reg = <0x00 0x23d0000 0x00 0x100>;
> -		clocks = <&k3_clks 362 1>;
> +		clocks = <&k3_clks 362 0>;
>  		power-domains = <&k3_pds 362 TI_SCI_PD_EXCLUSIVE>;
>  		assigned-clocks = <&k3_clks 362 0>;
>  		assigned-clock-parents = <&k3_clks 362 4>;
> @@ -2583,7 +2583,7 @@ watchdog14: watchdog@23d0000 {
>  	watchdog15: watchdog@23e0000 {
>  		compatible = "ti,j7-rti-wdt";
>  		reg = <0x00 0x23e0000 0x00 0x100>;
> -		clocks = <&k3_clks 363 1>;
> +		clocks = <&k3_clks 363 0>;
>  		power-domains = <&k3_pds 363 TI_SCI_PD_EXCLUSIVE>;
>  		assigned-clocks = <&k3_clks 363 0>;
>  		assigned-clock-parents = <&k3_clks 363 4>;
> @@ -2594,7 +2594,7 @@ watchdog15: watchdog@23e0000 {
>  	watchdog16: watchdog@23f0000 {
>  		compatible = "ti,j7-rti-wdt";
>  		reg = <0x00 0x23f0000 0x00 0x100>;
> -		clocks = <&k3_clks 364 1>;
> +		clocks = <&k3_clks 364 0>;
>  		power-domains = <&k3_pds 364 TI_SCI_PD_EXCLUSIVE>;
>  		assigned-clocks = <&k3_clks 364 0>;
>  		assigned-clock-parents = <&k3_clks 364 4>;
> @@ -2605,7 +2605,7 @@ watchdog16: watchdog@23f0000 {
>  	watchdog17: watchdog@2540000 {
>  		compatible = "ti,j7-rti-wdt";
>  		reg = <0x00 0x2540000 0x00 0x100>;
> -		clocks = <&k3_clks 365 1>;
> +		clocks = <&k3_clks 365 0>;
>  		power-domains = <&k3_pds 365 TI_SCI_PD_EXCLUSIVE>;
>  		assigned-clocks = <&k3_clks 365 0>;
>  		assigned-clock-parents = <&k3_clks 366 4>;
> @@ -2616,7 +2616,7 @@ watchdog17: watchdog@2540000 {
>  	watchdog18: watchdog@2550000 {
>  		compatible = "ti,j7-rti-wdt";
>  		reg = <0x00 0x2550000 0x00 0x100>;
> -		clocks = <&k3_clks 366 1>;
> +		clocks = <&k3_clks 366 0>;
>  		power-domains = <&k3_pds 366 TI_SCI_PD_EXCLUSIVE>;
>  		assigned-clocks = <&k3_clks 366 0>;
>  		assigned-clock-parents = <&k3_clks 366 4>;
> -- 
> 2.45.2
>
Andrew Halaney Aug. 26, 2024, 4:48 p.m. UTC | #2
On Tue, Aug 20, 2024 at 06:01:34PM GMT, Eric Chanudet wrote:
> On Mon, Aug 05, 2024 at 01:42:51PM GMT, Eric Chanudet wrote:
> > assigned-clock sets DEV_RTIx_RTI_CLK(id:0) whereas clocks sets
> > DEV_RTIx_RTI_CLK_PARENT_GLUELOGIC_HFOSC0_CLKOUT(id:1)[1]. This does not
> > look right, the timers in the driver assume a max frequency of 32kHz for
> > the heartbeat (HFOSC0 is 19.2MHz on j784s4-evm).
> > 
> > With this change, WDIOC_GETTIMELEFT return coherent time left
> > (DEFAULT_HEARTBEAT=60, reports 60s upon opening the cdev).
> > 
> > [1] http://downloads.ti.com/tisci/esd/latest/5_soc_doc/j784s4/clocks.html#clocks-for-rti0-device
> > 
> > Fixes: caae599de8c6 ("arm64: dts: ti: k3-j784s4-main: Add the main domain watchdog instances")
> > Suggested-by: Andrew Halaney <ahalaney@redhat.com>
> > Signed-off-by: Eric Chanudet <echanude@redhat.com>
> 
> Gentle ping and update to the test comment.
> 
> > ---
> > I could not get the watchdog to do more than reporting 0x32 in
> > RTIWDSTATUS. Setting RTIWWDRXCTRL[0:3] to generate a reset instead of an
> > interrupt (0x5) didn't trigger a reset either when the window expired.
> 
> Re-testing using u-boot from the BSP (2023.04) has the board reset as
> expected when the watchdog expires and WDIOC_GETTIMELEFT report the time
> left coherently with this patch until that happens.
> 
> I initially had a u-boot with a DT lacking:
> 	"mcu_esm: esm@40800000"
> and I could reproduce the board not resetting by commenting in its
> description:
> 	"ti,esm-pins = <95>;"
> 
> I don't understand why that is on the other hand. The TRM says ESM0
> ERR_O drives the SOC_SAFETY_ERRORn pin, which goes to the PMIC GPIO3 on
> the schematic _and_ to MCU_ESM0 as an error input event. The tps6594-esm
> module is probing successfully and it sets both ESM_SOC_EN|ESM_SOC_ENDRV
> and ESM_SOC_START, so I would expect the PMIC to reset the board without
> MCU_ESM0 being described or configured by u-boot.
> 

So, just so I understand (after writing/reading this ten times basically
I think we're saying the same thing, with mine just being 10x the words
:P )... This patch is needed to fix the clock for RTIx's else they tick at
the wrong speed and things go wrong.

Further with respect to actually resetting the board...
When an RTIx wdog trips, it sends the error to the associated
ESM (MAIN, MCU, WKUP, depending on the RTI instance). The ESM's need
to be programmed to take that as an error input ("ti,esm-pins").
The 3 ESMs are daisy chained too, and we assume that ti,esm-pins = <95>
describes the MCU_ESM0 and ESM0 connection (IIRC we couldn't find that explictly
documented, in fact we think that pin is not described in the TRM docs but
the others next to it were...).

The MAIN ESM, and the WKUP ESM, have a SOC_SAFETY_ERRORn/MCU_SAFETY_ERRORn
output from the SoC as well. That goes to tps6594's GPIO3/GPIO7. So if the
daisy chaining is setup you get ESM0 kicking all the way to WKUP_ESM0
which asserts out GPIO7/MCU_SAFETY_ERRORn. Without the daisy chaining you just
get GPIO3/SOC_SAFETY_ERRORn.

The tps6594 has a state machine (ESM again) that will reset the SoC
if those lines are asserted for long enough. That state machine needs
programming. Upstream linux programs that with
tps6594-esm + ESM_SOC_EN + ESM_SOC_START. The MCU_SOC_EN etc bits are only
referenced in upstream u-boots esm_pmic.c (using compatible = "ti,tps659413-esm").
At the moment, that's not setup for the j784s4 in upstream u-boot:
    halaney@x1gen2nano ~/git/u-boot (git)-[master] % git grep tps659413-esm
    arch/arm/dts/k3-j721e-r5-common-proc-board.dts:                 compatible = "ti,tps659413-esm";
    arch/arm/dts/k3-j721e-r5-sk.dts:                        compatible = "ti,tps659413-esm";
    ...
but it is in the BSP u-boot version you mentioned:
    tisdk@cd6a76441f0d:~/yocto-build/build/arago-tmp-default-baremetal-k3r5/work/j784s4_evm_k3r5-oe-eabi/u-boot-ti-staging/1_2023.04+gitAUTOINC+f9b966c674-r0_tisdk_3_edgeai_4/git$ git grep tps659413-esm | grep j784s4
    arch/arm/dts/k3-j784s4-r5-evm.dts:			compatible = "ti,tps659413-esm";

You (and I) were expecting to that even without the ESM daisy chaining,
the RTI0 instance if tripped would assert to pin 688 to ESM0 (which needs
to be programmed for ESM0 to pay attention to), and that would assert
SOC_SAFETY_ERRORn to tps6594's GPIO3, which Linux should have finished the
setup for to ensure the state machine is ran thru and the board is reset...
but you did not see that happen in practice, and needed to ensure MCU_ESM0
(via pin 95) was also programmed to get the board reset.

Dumb question, but there's so many pieces and domains at play I'll ask, did
you ensure that ESM0 was programmed in this test? Right now if you're using
upstream u-boot and upstream linux, nobody seems to be configured by default
to do that (u-boot needs to do that with k3_esm.c - meaning the defconfig needs
CONFIG_ESM_K3 && the dts needs ti,j721-esm with the RTI0-7 pins (688-695)
described). I think you probably did (since it sounds like adding the
daisy chaining pin 95 description back in made things reset), but just
making sure since I'm getting confused about the contexts here!

Below I grep through upstream u-boot to see if CONFIG_ESM_K3 is set
for j784s4 + to see if the devicetree used describes the ESMs:

    ahalaney@x1gen2nano ~/git/u-boot (git)-[master] % # Note how j784s4 isn't in the list... also note the r5 handles this usually on other socs             :(
    ahalaney@x1gen2nano ~/git/u-boot (git)-[master] % git grep ESM_K3                                                                                        :(
    ...
    configs/am62x_beagleplay_r5_defconfig:CONFIG_ESM_K3=y
    configs/am62x_evm_r5_defconfig:CONFIG_ESM_K3=y
    configs/am64x_evm_r5_defconfig:CONFIG_ESM_K3=y
    configs/j721e_evm_r5_defconfig:CONFIG_ESM_K3=y
    configs/phycore_am62x_r5_defconfig:CONFIG_ESM_K3=y
    configs/phycore_am64x_r5_defconfig:CONFIG_ESM_K3=y
    configs/verdin-am62_r5_defconfig:CONFIG_ESM_K3=y
    ahalaney@x1gen2nano ~/git/u-boot (git)-[master] % git grep DEVICE_TREE configs/j784s4_evm_r5_defconfig
    configs/j784s4_evm_r5_defconfig:CONFIG_DEFAULT_DEVICE_TREE="k3-j784s4-r5-evm"
    ahalaney@x1gen2nano ~/git/u-boot (git)-[master] % git grep j721e-esm | grep j784s4                                                                     :(
    dts/upstream/src/arm64/ti/k3-j784s4-main.dtsi:		compatible = "ti,j721e-esm";
    dts/upstream/src/arm64/ti/k3-j784s4-mcu-wakeup.dtsi:		compatible = "ti,j721e-esm";
    dts/upstream/src/arm64/ti/k3-j784s4-mcu-wakeup.dtsi:		compatible = "ti,j721e-esm";
    ahalaney@x1gen2nano ~/git/u-boot (git)-[master] % git grep k3-j784s4-mcu-wakeup.dtsi
    dts/upstream/src/arm64/ti/k3-j784s4.dtsi:#include "k3-j784s4-mcu-wakeup.dtsi"
    ahalaney@x1gen2nano ~/git/u-boot (git)-[master] % git grep k3-j784s4.dtsi
    dts/upstream/src/arm64/ti/k3-am69-sk.dts:#include "k3-j784s4.dtsi"
    dts/upstream/src/arm64/ti/k3-j784s4-evm.dts:#include "k3-j784s4.dtsi"
    ahalaney@x1gen2nano ~/git/u-boot (git)-[master] % git grep k3-j784s4-evm.dts
    arch/arm/dts/k3-j784s4-r5-evm.dts:#include "k3-j784s4-evm.dts"
    ahalaney@x1gen2nano ~/git/u-boot (git)-[master] % # Just to make sure there's not another file with that name in the tree..
    ahalaney@x1gen2nano ~/git/u-boot (git)-[master] % git ls-files | grep k3-j784s4-evm.dts
    dts/upstream/src/arm64/ti/k3-j784s4-evm.dts
    ahalaney@x1gen2nano ~/git/u-boot (git)-[master] % # Nope, ok, j784s4 r5 dts inherits the upstream one!

So upstream u-boot currently doesn't touch the ESMs on j784s4 at all as
far as I can tell due to lacking CONFIG_ESM_K3.

With all that in my scrambled mind, I feel we need to:

1. Take this patch to fix the RTI clocks and make that block work
   appropriately
2. Enable the ESM_K3 in upstream u-boot for the r5 build

The u-boot bits above are already done in the BSP u-boot, which is why I suppose things
work:
    tisdk@cd6a76441f0d:$ # Verify the r5 dts in the BSP describes the ESMs
    tisdk@cd6a76441f0d:$ git grep DEVICE_TREE configs/j784s4_evm_r5_defconfig 
    configs/j784s4_evm_r5_defconfig:CONFIG_DEFAULT_DEVICE_TREE="k3-j784s4-r5-evm"
    tisdk@cd6a76441f0d:$ git grep ti,j721e-esm | grep j784s4
    arch/arm/dts/k3-j784s4-main.dtsi:		compatible = "ti,j721e-esm";
    arch/arm/dts/k3-j784s4-mcu-wakeup.dtsi:		compatible = "ti,j721e-esm";
    arch/arm/dts/k3-j784s4-mcu-wakeup.dtsi:		compatible = "ti,j721e-esm";
    tisdk@cd6a76441f0d:$ git grep k3-j784s4-mcu-wakeup.dtsi 
    arch/arm/dts/k3-j784s4.dtsi:#include "k3-j784s4-mcu-wakeup.dtsi"
    tisdk@cd6a76441f0d:$ git grep k3-j784s4.dtsi
    arch/arm/dts/k3-am69-sk.dts:#include "k3-j784s4.dtsi"
    arch/arm/dts/k3-j784s4-evm.dts:#include "k3-j784s4.dtsi"
    tisdk@cd6a76441f0d:$ git grep k3-j784s4-evm.dts
    arch/arm/dts/k3-j784s4-r5-evm.dts:#include "k3-j784s4-evm.dts"
    # verify that the driver is built
    tisdk@cd6a76441f0d:$ git grep ESM_K3 | grep j784s4
    board/ti/j784s4/evm.c:	if (IS_ENABLED(CONFIG_ESM_K3)) {
    configs/j784s4_evm_r5_defconfig:CONFIG_ESM_K3=y

There's also that board file bit right above this comment, but I'm not
entirely sure if that's needed or not. Its not in upstream u-boot at least
for j784s4 (but is for j721e).

Here's how I view the connections mentally in case that garbled mess
above makes no sense. RTI0 is driven by the MAIN domain with linux,
ESMs are driven (if they are at all) by the MCU/r5 domains in u-boot,
TPS6594 is driven by linux wrt GPIO3's state machine, u-boot (if at all)
wrt GPIO7's state machine:

rti0 ---> ESM0 pin 688 --SOC_SAFETY_ERRORn--> TPS6594 GPIO3
				|
				|
				--> MCU_ESM0 pin 95 --> WKUP_ESM0 pin 63 --MCU_SAFETY_ERRORn--> TPS6584 GPIO7

Please let me know if that matches your view Eric... if so hopefully someone
from TI who's a little more intimate with the platform can comment on our view
and we can wrap this up for all the different contexts at play here!

Thanks,
Andrew
Kumar, Udit Aug. 26, 2024, 6:23 p.m. UTC | #3
Hello Eric

On 8/21/2024 3:31 AM, Eric Chanudet wrote:
> On Mon, Aug 05, 2024 at 01:42:51PM GMT, Eric Chanudet wrote:
>> assigned-clock sets DEV_RTIx_RTI_CLK(id:0) whereas clocks sets
>> DEV_RTIx_RTI_CLK_PARENT_GLUELOGIC_HFOSC0_CLKOUT(id:1)[1]. This does not
>> look right, the timers in the driver assume a max frequency of 32kHz for
>> the heartbeat (HFOSC0 is 19.2MHz on j784s4-evm).
>>
>> With this change, WDIOC_GETTIMELEFT return coherent time left
>> (DEFAULT_HEARTBEAT=60, reports 60s upon opening the cdev).
>>
>> [1] http://downloads.ti.com/tisci/esd/latest/5_soc_doc/j784s4/clocks.html#clocks-for-rti0-device
>>
>> Fixes: caae599de8c6 ("arm64: dts: ti: k3-j784s4-main: Add the main domain watchdog instances")
>> Suggested-by: Andrew Halaney <ahalaney@redhat.com>
>> Signed-off-by: Eric Chanudet <echanude@redhat.com>
> Gentle ping and update to the test comment.
>
>> ---
>> I could not get the watchdog to do more than reporting 0x32 in
>> RTIWDSTATUS. Setting RTIWWDRXCTRL[0:3] to generate a reset instead of an
>> interrupt (0x5) didn't trigger a reset either when the window expired.
> Re-testing using u-boot from the BSP (2023.04) has the board reset as
> expected when the watchdog expires and WDIOC_GETTIMELEFT report the time
> left coherently with this patch until that happens.
>
> I initially had a u-boot with a DT lacking:
> 	"mcu_esm: esm@40800000"
> and I could reproduce the board not resetting by commenting in its
> description:
> 	"ti,esm-pins = <95>;"
>
> I don't understand why that is on the other hand. The TRM says ESM0
> ERR_O drives the SOC_SAFETY_ERRORn pin, which goes to the PMIC GPIO3 on
> the schematic _and_ to MCU_ESM0 as an error input event. The tps6594-esm
> module is probing successfully and it sets both ESM_SOC_EN|ESM_SOC_ENDRV
> and ESM_SOC_START, so I would expect the PMIC to reset the board without
> MCU_ESM0 being described or configured by u-boot.

AFAIK, Keerthy correct me. GPIO-7 of PMIC should reset the boards.

If you see figure 5-27 of TRM then SOC_SAFETY_ERRORn goes to GPIO-3 of 
PMIC (schematic)

Same time this is cascaded to MCU-ESM and WKUP-ESM to generate 
MCU_SAFETY_ERRORn (from Wkup_ESM)

and MCU_SAFETY_ERRORn is connected to GPIO-7.

Unlike other device J721E (for reference)

SOC_SAFETY_ERRORn is generated by Main ESM and MCU_SAFETY_ERRORn can be 
generated by WKUP_ESM and main_ESM.

Please look at schematic of J721E SOM [0], both SOC_SAFETY_ERRZ and 
MCU_SAFETY_ERRZ both are connected to GPIO-7 of PMIC.

So on this device and board, only main ESM configuration is working for us.

[0] https://www.ti.com/tool/J721EXSOMXEVM#tech-docs

> Best,
>
>>   arch/arm64/boot/dts/ti/k3-j784s4-main.dtsi | 38 +++++++++++-----------
>>   1 file changed, 19 insertions(+), 19 deletions(-)
>>
>> diff --git a/arch/arm64/boot/dts/ti/k3-j784s4-main.dtsi b/arch/arm64/boot/dts/ti/k3-j784s4-main.dtsi
>> index f170f80f00c1..6c014d335f2c 100644
>> --- a/arch/arm64/boot/dts/ti/k3-j784s4-main.dtsi
>> +++ b/arch/arm64/boot/dts/ti/k3-j784s4-main.dtsi
>> @@ -2429,7 +2429,7 @@ main_esm: esm@700000 {
>>   	watchdog0: watchdog@2200000 {
>>   		compatible = "ti,j7-rti-wdt";
>>   		reg = <0x00 0x2200000 0x00 0x100>;
>> -		clocks = <&k3_clks 348 1>;
>> +		clocks = <&k3_clks 348 0>;
>>   		power-domains = <&k3_pds 348 TI_SCI_PD_EXCLUSIVE>;
>>   		assigned-clocks = <&k3_clks 348 0>;
>>   		assigned-clock-parents = <&k3_clks 348 4>;
>> @@ -2438,7 +2438,7 @@ watchdog0: watchdog@2200000 {
>>   	watchdog1: watchdog@2210000 {
>>   		compatible = "ti,j7-rti-wdt";
>>   		reg = <0x00 0x2210000 0x00 0x100>;
>> -		clocks = <&k3_clks 349 1>;
>> +		clocks = <&k3_clks 349 0>;
>>   		power-domains = <&k3_pds 349 TI_SCI_PD_EXCLUSIVE>;
>>   		assigned-clocks = <&k3_clks 349 0>;
>>   		assigned-clock-parents = <&k3_clks 349 4>;
>> @@ -2447,7 +2447,7 @@ watchdog1: watchdog@2210000 {
>>   	watchdog2: watchdog@2220000 {
>>   		compatible = "ti,j7-rti-wdt";
>>   		reg = <0x00 0x2220000 0x00 0x100>;
>> -		clocks = <&k3_clks 350 1>;
>> +		clocks = <&k3_clks 350 0>;
>>   		power-domains = <&k3_pds 350 TI_SCI_PD_EXCLUSIVE>;
>>   		assigned-clocks = <&k3_clks 350 0>;
>>   		assigned-clock-parents = <&k3_clks 350 4>;
>> @@ -2456,7 +2456,7 @@ watchdog2: watchdog@2220000 {
>>   	watchdog3: watchdog@2230000 {
>>   		compatible = "ti,j7-rti-wdt";
>>   		reg = <0x00 0x2230000 0x00 0x100>;
>> -		clocks = <&k3_clks 351 1>;
>> +		clocks = <&k3_clks 351 0>;
>>   		power-domains = <&k3_pds 351 TI_SCI_PD_EXCLUSIVE>;
>>   		assigned-clocks = <&k3_clks 351 0>;
>>   		assigned-clock-parents = <&k3_clks 351 4>;
>> @@ -2465,7 +2465,7 @@ watchdog3: watchdog@2230000 {
>>   	watchdog4: watchdog@2240000 {
>>   		compatible = "ti,j7-rti-wdt";
>>   		reg = <0x00 0x2240000 0x00 0x100>;
>> -		clocks = <&k3_clks 352 1>;
>> +		clocks = <&k3_clks 352 0>;
>>   		power-domains = <&k3_pds 352 TI_SCI_PD_EXCLUSIVE>;
>>   		assigned-clocks = <&k3_clks 352 0>;
>>   		assigned-clock-parents = <&k3_clks 352 4>;
>> @@ -2474,7 +2474,7 @@ watchdog4: watchdog@2240000 {
>>   	watchdog5: watchdog@2250000 {
>>   		compatible = "ti,j7-rti-wdt";
>>   		reg = <0x00 0x2250000 0x00 0x100>;
>> -		clocks = <&k3_clks 353 1>;
>> +		clocks = <&k3_clks 353 0>;
>>   		power-domains = <&k3_pds 353 TI_SCI_PD_EXCLUSIVE>;
>>   		assigned-clocks = <&k3_clks 353 0>;
>>   		assigned-clock-parents = <&k3_clks 353 4>;
>> @@ -2483,7 +2483,7 @@ watchdog5: watchdog@2250000 {
>>   	watchdog6: watchdog@2260000 {
>>   		compatible = "ti,j7-rti-wdt";
>>   		reg = <0x00 0x2260000 0x00 0x100>;
>> -		clocks = <&k3_clks 354 1>;
>> +		clocks = <&k3_clks 354 0>;
>>   		power-domains = <&k3_pds 354 TI_SCI_PD_EXCLUSIVE>;
>>   		assigned-clocks = <&k3_clks 354 0>;
>>   		assigned-clock-parents = <&k3_clks 354 4>;
>> @@ -2492,7 +2492,7 @@ watchdog6: watchdog@2260000 {
>>   	watchdog7: watchdog@2270000 {
>>   		compatible = "ti,j7-rti-wdt";
>>   		reg = <0x00 0x2270000 0x00 0x100>;
>> -		clocks = <&k3_clks 355 1>;
>> +		clocks = <&k3_clks 355 0>;
>>   		power-domains = <&k3_pds 355 TI_SCI_PD_EXCLUSIVE>;
>>   		assigned-clocks = <&k3_clks 355 0>;
>>   		assigned-clock-parents = <&k3_clks 355 4>;
>> @@ -2506,7 +2506,7 @@ watchdog7: watchdog@2270000 {
>>   	watchdog8: watchdog@22f0000 {
>>   		compatible = "ti,j7-rti-wdt";
>>   		reg = <0x00 0x22f0000 0x00 0x100>;
>> -		clocks = <&k3_clks 360 1>;
>> +		clocks = <&k3_clks 360 0>;
>>   		power-domains = <&k3_pds 360 TI_SCI_PD_EXCLUSIVE>;
>>   		assigned-clocks = <&k3_clks 360 0>;
>>   		assigned-clock-parents = <&k3_clks 360 4>;
>> @@ -2517,7 +2517,7 @@ watchdog8: watchdog@22f0000 {
>>   	watchdog9: watchdog@2300000 {
>>   		compatible = "ti,j7-rti-wdt";
>>   		reg = <0x00 0x2300000 0x00 0x100>;
>> -		clocks = <&k3_clks 356 1>;
>> +		clocks = <&k3_clks 356 0>;
>>   		power-domains = <&k3_pds 356 TI_SCI_PD_EXCLUSIVE>;
>>   		assigned-clocks = <&k3_clks 356 0>;
>>   		assigned-clock-parents = <&k3_clks 356 4>;
>> @@ -2528,7 +2528,7 @@ watchdog9: watchdog@2300000 {
>>   	watchdog10: watchdog@2310000 {
>>   		compatible = "ti,j7-rti-wdt";
>>   		reg = <0x00 0x2310000 0x00 0x100>;
>> -		clocks = <&k3_clks 357 1>;
>> +		clocks = <&k3_clks 357 0>;
>>   		power-domains = <&k3_pds 357 TI_SCI_PD_EXCLUSIVE>;
>>   		assigned-clocks = <&k3_clks 357 0>;
>>   		assigned-clock-parents = <&k3_clks 357 4>;
>> @@ -2539,7 +2539,7 @@ watchdog10: watchdog@2310000 {
>>   	watchdog11: watchdog@2320000 {
>>   		compatible = "ti,j7-rti-wdt";
>>   		reg = <0x00 0x2320000 0x00 0x100>;
>> -		clocks = <&k3_clks 358 1>;
>> +		clocks = <&k3_clks 358 0>;
>>   		power-domains = <&k3_pds 358 TI_SCI_PD_EXCLUSIVE>;
>>   		assigned-clocks = <&k3_clks 358 0>;
>>   		assigned-clock-parents = <&k3_clks 358 4>;
>> @@ -2550,7 +2550,7 @@ watchdog11: watchdog@2320000 {
>>   	watchdog12: watchdog@2330000 {
>>   		compatible = "ti,j7-rti-wdt";
>>   		reg = <0x00 0x2330000 0x00 0x100>;
>> -		clocks = <&k3_clks 359 1>;
>> +		clocks = <&k3_clks 359 0>;
>>   		power-domains = <&k3_pds 359 TI_SCI_PD_EXCLUSIVE>;
>>   		assigned-clocks = <&k3_clks 359 0>;
>>   		assigned-clock-parents = <&k3_clks 359 4>;
>> @@ -2561,7 +2561,7 @@ watchdog12: watchdog@2330000 {
>>   	watchdog13: watchdog@23c0000 {
>>   		compatible = "ti,j7-rti-wdt";
>>   		reg = <0x00 0x23c0000 0x00 0x100>;
>> -		clocks = <&k3_clks 361 1>;
>> +		clocks = <&k3_clks 361 0>;
>>   		power-domains = <&k3_pds 361 TI_SCI_PD_EXCLUSIVE>;
>>   		assigned-clocks = <&k3_clks 361 0>;
>>   		assigned-clock-parents = <&k3_clks 361 4>;
>> @@ -2572,7 +2572,7 @@ watchdog13: watchdog@23c0000 {
>>   	watchdog14: watchdog@23d0000 {
>>   		compatible = "ti,j7-rti-wdt";
>>   		reg = <0x00 0x23d0000 0x00 0x100>;
>> -		clocks = <&k3_clks 362 1>;
>> +		clocks = <&k3_clks 362 0>;
>>   		power-domains = <&k3_pds 362 TI_SCI_PD_EXCLUSIVE>;
>>   		assigned-clocks = <&k3_clks 362 0>;
>>   		assigned-clock-parents = <&k3_clks 362 4>;
>> @@ -2583,7 +2583,7 @@ watchdog14: watchdog@23d0000 {
>>   	watchdog15: watchdog@23e0000 {
>>   		compatible = "ti,j7-rti-wdt";
>>   		reg = <0x00 0x23e0000 0x00 0x100>;
>> -		clocks = <&k3_clks 363 1>;
>> +		clocks = <&k3_clks 363 0>;
>>   		power-domains = <&k3_pds 363 TI_SCI_PD_EXCLUSIVE>;
>>   		assigned-clocks = <&k3_clks 363 0>;
>>   		assigned-clock-parents = <&k3_clks 363 4>;
>> @@ -2594,7 +2594,7 @@ watchdog15: watchdog@23e0000 {
>>   	watchdog16: watchdog@23f0000 {
>>   		compatible = "ti,j7-rti-wdt";
>>   		reg = <0x00 0x23f0000 0x00 0x100>;
>> -		clocks = <&k3_clks 364 1>;
>> +		clocks = <&k3_clks 364 0>;
>>   		power-domains = <&k3_pds 364 TI_SCI_PD_EXCLUSIVE>;
>>   		assigned-clocks = <&k3_clks 364 0>;
>>   		assigned-clock-parents = <&k3_clks 364 4>;
>> @@ -2605,7 +2605,7 @@ watchdog16: watchdog@23f0000 {
>>   	watchdog17: watchdog@2540000 {
>>   		compatible = "ti,j7-rti-wdt";
>>   		reg = <0x00 0x2540000 0x00 0x100>;
>> -		clocks = <&k3_clks 365 1>;
>> +		clocks = <&k3_clks 365 0>;
>>   		power-domains = <&k3_pds 365 TI_SCI_PD_EXCLUSIVE>;
>>   		assigned-clocks = <&k3_clks 365 0>;
>>   		assigned-clock-parents = <&k3_clks 366 4>;
>> @@ -2616,7 +2616,7 @@ watchdog17: watchdog@2540000 {
>>   	watchdog18: watchdog@2550000 {
>>   		compatible = "ti,j7-rti-wdt";
>>   		reg = <0x00 0x2550000 0x00 0x100>;
>> -		clocks = <&k3_clks 366 1>;
>> +		clocks = <&k3_clks 366 0>;
>>   		power-domains = <&k3_pds 366 TI_SCI_PD_EXCLUSIVE>;
>>   		assigned-clocks = <&k3_clks 366 0>;
>>   		assigned-clock-parents = <&k3_clks 366 4>;
>> -- 
>> 2.45.2
>>
Kumar, Udit Aug. 26, 2024, 6:31 p.m. UTC | #4
On 8/5/2024 11:12 PM, Eric Chanudet wrote:
> assigned-clock sets DEV_RTIx_RTI_CLK(id:0) whereas clocks sets
> DEV_RTIx_RTI_CLK_PARENT_GLUELOGIC_HFOSC0_CLKOUT(id:1)[1]. This does not
> look right, the timers in the driver assume a max frequency of 32kHz for
> the heartbeat (HFOSC0 is 19.2MHz on j784s4-evm).
>
> With this change, WDIOC_GETTIMELEFT return coherent time left
> (DEFAULT_HEARTBEAT=60, reports 60s upon opening the cdev).
>
> [1] http://downloads.ti.com/tisci/esd/latest/5_soc_doc/j784s4/clocks.html#clocks-for-rti0-device
>
> Fixes: caae599de8c6 ("arm64: dts: ti: k3-j784s4-main: Add the main domain watchdog instances")
> Suggested-by: Andrew Halaney <ahalaney@redhat.com>
> Signed-off-by: Eric Chanudet <echanude@redhat.com>
> ---
> I could not get the watchdog to do more than reporting 0x32 in
> RTIWDSTATUS. Setting RTIWWDRXCTRL[0:3] to generate a reset instead of an
> interrupt (0x5) didn't trigger a reset either when the window expired.


Tested-by: Udit Kumar <u-kumar1@ti.com>

Able to reset the board

https://gist.github.com/uditkumarti/1d1b66b250949a911b008728445db9c2


>   arch/arm64/boot/dts/ti/k3-j784s4-main.dtsi | 38 +++++++++++-----------
>   1 file changed, 19 insertions(+), 19 deletions(-)
>
> diff --git a/arch/arm64/boot/dts/ti/k3-j784s4-main.dtsi b/arch/arm64/boot/dts/ti/k3-j784s4-main.dtsi
> index f170f80f00c1..6c014d335f2c 100644
> --- a/arch/arm64/boot/dts/ti/k3-j784s4-main.dtsi
> +++ b/arch/arm64/boot/dts/ti/k3-j784s4-main.dtsi
> @@ -2429,7 +2429,7 @@ main_esm: esm@700000 {
>   	watchdog0: watchdog@2200000 {
>   		compatible = "ti,j7-rti-wdt";
>   		reg = <0x00 0x2200000 0x00 0x100>;
> -		clocks = <&k3_clks 348 1>;
> +		clocks = <&k3_clks 348 0>;
>   		power-domains = <&k3_pds 348 TI_SCI_PD_EXCLUSIVE>;
> [...]
Eric Chanudet Aug. 26, 2024, 10:20 p.m. UTC | #5
On Mon, Aug 26, 2024 at 11:53:56PM GMT, Kumar, Udit wrote:
> Hello Eric
> 
> On 8/21/2024 3:31 AM, Eric Chanudet wrote:
> > On Mon, Aug 05, 2024 at 01:42:51PM GMT, Eric Chanudet wrote:
> > > ---
> > > I could not get the watchdog to do more than reporting 0x32 in
> > > RTIWDSTATUS. Setting RTIWWDRXCTRL[0:3] to generate a reset instead of an
> > > interrupt (0x5) didn't trigger a reset either when the window expired.
> > Re-testing using u-boot from the BSP (2023.04) has the board reset as
> > expected when the watchdog expires and WDIOC_GETTIMELEFT report the time
> > left coherently with this patch until that happens.
> > 
> > I initially had a u-boot with a DT lacking:
> > 	"mcu_esm: esm@40800000"
> > and I could reproduce the board not resetting by commenting in its
> > description:
> > 	"ti,esm-pins = <95>;"
> > 
> > I don't understand why that is on the other hand. The TRM says ESM0
> > ERR_O drives the SOC_SAFETY_ERRORn pin, which goes to the PMIC GPIO3 on
> > the schematic _and_ to MCU_ESM0 as an error input event. The tps6594-esm
> > module is probing successfully and it sets both ESM_SOC_EN|ESM_SOC_ENDRV
> > and ESM_SOC_START, so I would expect the PMIC to reset the board without
> > MCU_ESM0 being described or configured by u-boot.
> 
> AFAIK, Keerthy correct me. GPIO-7 of PMIC should reset the boards.

That is what I'm seeing too, MCU_ESM0 is able to reset the board.

> If you see figure 5-27 of TRM then SOC_SAFETY_ERRORn goes to GPIO-3 of
> PMIC (schematic)
> 
> Same time this is cascaded to MCU-ESM and WKUP-ESM to generate
> MCU_SAFETY_ERRORn (from Wkup_ESM)
> 
> and MCU_SAFETY_ERRORn is connected to GPIO-7.

Agreed (Figure 5-25, in TRM "SPRUJ52" for J784S4).

> Unlike other device J721E (for reference)
> 
> SOC_SAFETY_ERRORn is generated by Main ESM and MCU_SAFETY_ERRORn can be
> generated by WKUP_ESM and main_ESM.
> 
> Please look at schematic of J721E SOM [0], both SOC_SAFETY_ERRZ and
> MCU_SAFETY_ERRZ both are connected to GPIO-7 of PMIC.
> 
> So on this device and board, only main ESM configuration is working for us.
> 
> [0] https://www.ti.com/tool/J721EXSOMXEVM#tech-docs

Sure, but I am using J784S4[1] and the schematic of that board
(PROC141E4(001)_SCH) shows SOC_SAFETY_ERRZ going to PMIC GPIO3.

So when u-boot _does not_ configure MCU_ESM0 chaining through pin95, I
would still expect the board to reboot, because ESM0 raised
SOC_SAFETY_ERRORn on TPS6594 GPIO3 which should reset the board. Yet
that does not seem to happen.

[1] https://www.ti.com/tool/J784S4XEVM#tech-docs

On Mon, Aug 26, 2024 at 11:48:34AM GMT, Andrew Halaney wrote:
> rti0 ---> ESM0 pin 688 --SOC_SAFETY_ERRORn--> TPS6594 GPIO3
> 				|
> 				|
> 				--> MCU_ESM0 pin 95 --> WKUP_ESM0 pin 63 --MCU_SAFETY_ERRORn--> TPS6584 GPIO7

Using Andrew's drawing as it matches my understanding as well. So the
PMIC should reset the board even if MCU_ESM0 isn't configured with pin95
to chain SOC_SAFETY_ERRORn.

Am I misunderstanding this?

As well, since it is mentioned in Andrew's reply:

On Mon, Aug 26, 2024 at 11:48:34AM GMT, Andrew Halaney wrote:
> did you ensure that ESM0 was programmed in this test? Right now if
> you're using upstream u-boot and upstream linux, nobody seems to be
> configured by default to do that

I am using the BSP u-boot (2023.04-f9b966c674) for this test, which has
CONFIG_ESM_K3=y and esm@700000's description with pin688.

Best,
Kumar, Udit Aug. 27, 2024, 4:11 a.m. UTC | #6
Hi Eric

On 8/27/2024 3:50 AM, Eric Chanudet wrote:
> On Mon, Aug 26, 2024 at 11:53:56PM GMT, Kumar, Udit wrote:
>> Hello Eric
>>
>> On 8/21/2024 3:31 AM, Eric Chanudet wrote:
>>> On Mon, Aug 05, 2024 at 01:42:51PM GMT, Eric Chanudet wrote:
>>>> ---
>>>> I could not get the watchdog to do more than reporting 0x32 in
>>>> RTIWDSTATUS. Setting RTIWWDRXCTRL[0:3] to generate a reset instead of an
>>>> interrupt (0x5) didn't trigger a reset either when the window expired.
>>> Re-testing using u-boot from the BSP (2023.04) has the board reset as
>>> expected when the watchdog expires and WDIOC_GETTIMELEFT report the time
>>> left coherently with this patch until that happens.
>>>
>>> I initially had a u-boot with a DT lacking:
>>> 	"mcu_esm: esm@40800000"
>>> and I could reproduce the board not resetting by commenting in its
>>> description:
>>> 	"ti,esm-pins = <95>;"
>>>
>>> I don't understand why that is on the other hand. The TRM says ESM0
>>> ERR_O drives the SOC_SAFETY_ERRORn pin, which goes to the PMIC GPIO3 on
>>> the schematic _and_ to MCU_ESM0 as an error input event. The tps6594-esm
>>> module is probing successfully and it sets both ESM_SOC_EN|ESM_SOC_ENDRV
>>> and ESM_SOC_START, so I would expect the PMIC to reset the board without
>>> MCU_ESM0 being described or configured by u-boot.
>> AFAIK, Keerthy correct me. GPIO-7 of PMIC should reset the boards.
> That is what I'm seeing too, MCU_ESM0 is able to reset the board.
>
>> If you see figure 5-27 of TRM then SOC_SAFETY_ERRORn goes to GPIO-3 of
>> PMIC (schematic)
>>
>> Same time this is cascaded to MCU-ESM and WKUP-ESM to generate
>> MCU_SAFETY_ERRORn (from Wkup_ESM)
>>
>> and MCU_SAFETY_ERRORn is connected to GPIO-7.
> Agreed (Figure 5-25, in TRM "SPRUJ52" for J784S4).
>
>> Unlike other device J721E (for reference)
>>
>> SOC_SAFETY_ERRORn is generated by Main ESM and MCU_SAFETY_ERRORn can be
>> generated by WKUP_ESM and main_ESM.
>>
>> Please look at schematic of J721E SOM [0], both SOC_SAFETY_ERRZ and
>> MCU_SAFETY_ERRZ both are connected to GPIO-7 of PMIC.
>>
>> So on this device and board, only main ESM configuration is working for us.
>>
>> [0] https://www.ti.com/tool/J721EXSOMXEVM#tech-docs
> Sure, but I am using J784S4[1] and the schematic of that board
> (PROC141E4(001)_SCH) shows SOC_SAFETY_ERRZ going to PMIC GPIO3.
>
> So when u-boot _does not_ configure MCU_ESM0 chaining through pin95, I
> would still expect the board to reboot, because ESM0 raised
> SOC_SAFETY_ERRORn on TPS6594 GPIO3 which should reset the board. Yet
> that does not seem to happen.

I think, we are saying same thing .

Reset is happening with GPIO-7 pin of PMIC not with GPIO-3 PIN.


> [1] https://www.ti.com/tool/J784S4XEVM#tech-docs
>
> On Mon, Aug 26, 2024 at 11:48:34AM GMT, Andrew Halaney wrote:
>> rti0 ---> ESM0 pin 688 --SOC_SAFETY_ERRORn--> TPS6594 GPIO3
>> 				|
>> 				|
>> 				--> MCU_ESM0 pin 95 --> WKUP_ESM0 pin 63 --MCU_SAFETY_ERRORn--> TPS6584 GPIO7
> Using Andrew's drawing as it matches my understanding as well. So the
> PMIC should reset the board even if MCU_ESM0 isn't configured with pin95
> to chain SOC_SAFETY_ERRORn.
>
> Am I misunderstanding this?


I am not PMIC expert,

but based upon my results on other SOC/EVM. SOC_SAFETY_ERRORn signal can 
reset the board, if this is connected with GPIO-7 pin of PMIC.


>
> As well, since it is mentioned in Andrew's reply:
>
> On Mon, Aug 26, 2024 at 11:48:34AM GMT, Andrew Halaney wrote:
>> did you ensure that ESM0 was programmed in this test? Right now if
>> you're using upstream u-boot and upstream linux, nobody seems to be
>> configured by default to do that
> I am using the BSP u-boot (2023.04-f9b966c674) for this test, which has
> CONFIG_ESM_K3=y and esm@700000's description with pin688.
>
> Best,
>
Andrew Halaney Aug. 27, 2024, 9:56 p.m. UTC | #7
On Mon, Aug 05, 2024 at 01:42:51PM GMT, Eric Chanudet wrote:
> assigned-clock sets DEV_RTIx_RTI_CLK(id:0) whereas clocks sets
> DEV_RTIx_RTI_CLK_PARENT_GLUELOGIC_HFOSC0_CLKOUT(id:1)[1]. This does not
> look right, the timers in the driver assume a max frequency of 32kHz for
> the heartbeat (HFOSC0 is 19.2MHz on j784s4-evm).
> 
> With this change, WDIOC_GETTIMELEFT return coherent time left
> (DEFAULT_HEARTBEAT=60, reports 60s upon opening the cdev).
> 
> [1] http://downloads.ti.com/tisci/esd/latest/5_soc_doc/j784s4/clocks.html#clocks-for-rti0-device
> 
> Fixes: caae599de8c6 ("arm64: dts: ti: k3-j784s4-main: Add the main domain watchdog instances")
> Suggested-by: Andrew Halaney <ahalaney@redhat.com>
> Signed-off-by: Eric Chanudet <echanude@redhat.com>

Tested-by: Andrew Halaney <ahalaney@redhat.com>

As mentioned throughout the other thread, you need the r5 u-boot build to
configure the ESMs and the PMIC properly to get the board to actually
reset (not in upstream u-boot at the moment).

This patch fixes the watchdog itself though, prior timeleft was
bogus etc but with this patch it starts at 60 sec (default) and reboots
when we hit 0.

Still an open question about the ESM(s) and their relationship with the
PMIC, but that's an entirely independent subject to this fix.

Thanks,
Andrew
Nishanth Menon Aug. 28, 2024, 6:39 p.m. UTC | #8
Hi Eric Chanudet,

On Mon, 05 Aug 2024 13:42:51 -0400, Eric Chanudet wrote:
> assigned-clock sets DEV_RTIx_RTI_CLK(id:0) whereas clocks sets
> DEV_RTIx_RTI_CLK_PARENT_GLUELOGIC_HFOSC0_CLKOUT(id:1)[1]. This does not
> look right, the timers in the driver assume a max frequency of 32kHz for
> the heartbeat (HFOSC0 is 19.2MHz on j784s4-evm).
> 
> With this change, WDIOC_GETTIMELEFT return coherent time left
> (DEFAULT_HEARTBEAT=60, reports 60s upon opening the cdev).
> 
> [...]

I have applied the following to branch ti-k3-dts-next on [1].
Thank you!

[1/1] arm64: dts: ti: k3-j784s4-main: align watchdog clocks
      commit: 549833b697534a6f61840941b7c847669cfd77fa

PS: I did replace the http url with the tisci https:// url in the commit
log. This is inline with the base url used in kernel documentation
till date.

All being well this means that it will be integrated into the linux-next
tree (usually sometime in the next 24 hours) and sent up the chain during
the next merge window (or sooner if it is a relevant bug fix), however if
problems are discovered then the patch may be dropped or reverted.

You may get further e-mails resulting from automated or manual testing
and review of the tree, please engage with people reporting problems and
send followup patches addressing any issues that are reported if needed.

If any updates are required or you are submitting further changes they
should be sent as incremental updates against current git, existing
patches will not be replaced.

Please add any relevant lists and maintainers to the CCs when replying
to this mail.

[1] https://git.kernel.org/pub/scm/linux/kernel/git/ti/linux.git
diff mbox series

Patch

diff --git a/arch/arm64/boot/dts/ti/k3-j784s4-main.dtsi b/arch/arm64/boot/dts/ti/k3-j784s4-main.dtsi
index f170f80f00c1..6c014d335f2c 100644
--- a/arch/arm64/boot/dts/ti/k3-j784s4-main.dtsi
+++ b/arch/arm64/boot/dts/ti/k3-j784s4-main.dtsi
@@ -2429,7 +2429,7 @@  main_esm: esm@700000 {
 	watchdog0: watchdog@2200000 {
 		compatible = "ti,j7-rti-wdt";
 		reg = <0x00 0x2200000 0x00 0x100>;
-		clocks = <&k3_clks 348 1>;
+		clocks = <&k3_clks 348 0>;
 		power-domains = <&k3_pds 348 TI_SCI_PD_EXCLUSIVE>;
 		assigned-clocks = <&k3_clks 348 0>;
 		assigned-clock-parents = <&k3_clks 348 4>;
@@ -2438,7 +2438,7 @@  watchdog0: watchdog@2200000 {
 	watchdog1: watchdog@2210000 {
 		compatible = "ti,j7-rti-wdt";
 		reg = <0x00 0x2210000 0x00 0x100>;
-		clocks = <&k3_clks 349 1>;
+		clocks = <&k3_clks 349 0>;
 		power-domains = <&k3_pds 349 TI_SCI_PD_EXCLUSIVE>;
 		assigned-clocks = <&k3_clks 349 0>;
 		assigned-clock-parents = <&k3_clks 349 4>;
@@ -2447,7 +2447,7 @@  watchdog1: watchdog@2210000 {
 	watchdog2: watchdog@2220000 {
 		compatible = "ti,j7-rti-wdt";
 		reg = <0x00 0x2220000 0x00 0x100>;
-		clocks = <&k3_clks 350 1>;
+		clocks = <&k3_clks 350 0>;
 		power-domains = <&k3_pds 350 TI_SCI_PD_EXCLUSIVE>;
 		assigned-clocks = <&k3_clks 350 0>;
 		assigned-clock-parents = <&k3_clks 350 4>;
@@ -2456,7 +2456,7 @@  watchdog2: watchdog@2220000 {
 	watchdog3: watchdog@2230000 {
 		compatible = "ti,j7-rti-wdt";
 		reg = <0x00 0x2230000 0x00 0x100>;
-		clocks = <&k3_clks 351 1>;
+		clocks = <&k3_clks 351 0>;
 		power-domains = <&k3_pds 351 TI_SCI_PD_EXCLUSIVE>;
 		assigned-clocks = <&k3_clks 351 0>;
 		assigned-clock-parents = <&k3_clks 351 4>;
@@ -2465,7 +2465,7 @@  watchdog3: watchdog@2230000 {
 	watchdog4: watchdog@2240000 {
 		compatible = "ti,j7-rti-wdt";
 		reg = <0x00 0x2240000 0x00 0x100>;
-		clocks = <&k3_clks 352 1>;
+		clocks = <&k3_clks 352 0>;
 		power-domains = <&k3_pds 352 TI_SCI_PD_EXCLUSIVE>;
 		assigned-clocks = <&k3_clks 352 0>;
 		assigned-clock-parents = <&k3_clks 352 4>;
@@ -2474,7 +2474,7 @@  watchdog4: watchdog@2240000 {
 	watchdog5: watchdog@2250000 {
 		compatible = "ti,j7-rti-wdt";
 		reg = <0x00 0x2250000 0x00 0x100>;
-		clocks = <&k3_clks 353 1>;
+		clocks = <&k3_clks 353 0>;
 		power-domains = <&k3_pds 353 TI_SCI_PD_EXCLUSIVE>;
 		assigned-clocks = <&k3_clks 353 0>;
 		assigned-clock-parents = <&k3_clks 353 4>;
@@ -2483,7 +2483,7 @@  watchdog5: watchdog@2250000 {
 	watchdog6: watchdog@2260000 {
 		compatible = "ti,j7-rti-wdt";
 		reg = <0x00 0x2260000 0x00 0x100>;
-		clocks = <&k3_clks 354 1>;
+		clocks = <&k3_clks 354 0>;
 		power-domains = <&k3_pds 354 TI_SCI_PD_EXCLUSIVE>;
 		assigned-clocks = <&k3_clks 354 0>;
 		assigned-clock-parents = <&k3_clks 354 4>;
@@ -2492,7 +2492,7 @@  watchdog6: watchdog@2260000 {
 	watchdog7: watchdog@2270000 {
 		compatible = "ti,j7-rti-wdt";
 		reg = <0x00 0x2270000 0x00 0x100>;
-		clocks = <&k3_clks 355 1>;
+		clocks = <&k3_clks 355 0>;
 		power-domains = <&k3_pds 355 TI_SCI_PD_EXCLUSIVE>;
 		assigned-clocks = <&k3_clks 355 0>;
 		assigned-clock-parents = <&k3_clks 355 4>;
@@ -2506,7 +2506,7 @@  watchdog7: watchdog@2270000 {
 	watchdog8: watchdog@22f0000 {
 		compatible = "ti,j7-rti-wdt";
 		reg = <0x00 0x22f0000 0x00 0x100>;
-		clocks = <&k3_clks 360 1>;
+		clocks = <&k3_clks 360 0>;
 		power-domains = <&k3_pds 360 TI_SCI_PD_EXCLUSIVE>;
 		assigned-clocks = <&k3_clks 360 0>;
 		assigned-clock-parents = <&k3_clks 360 4>;
@@ -2517,7 +2517,7 @@  watchdog8: watchdog@22f0000 {
 	watchdog9: watchdog@2300000 {
 		compatible = "ti,j7-rti-wdt";
 		reg = <0x00 0x2300000 0x00 0x100>;
-		clocks = <&k3_clks 356 1>;
+		clocks = <&k3_clks 356 0>;
 		power-domains = <&k3_pds 356 TI_SCI_PD_EXCLUSIVE>;
 		assigned-clocks = <&k3_clks 356 0>;
 		assigned-clock-parents = <&k3_clks 356 4>;
@@ -2528,7 +2528,7 @@  watchdog9: watchdog@2300000 {
 	watchdog10: watchdog@2310000 {
 		compatible = "ti,j7-rti-wdt";
 		reg = <0x00 0x2310000 0x00 0x100>;
-		clocks = <&k3_clks 357 1>;
+		clocks = <&k3_clks 357 0>;
 		power-domains = <&k3_pds 357 TI_SCI_PD_EXCLUSIVE>;
 		assigned-clocks = <&k3_clks 357 0>;
 		assigned-clock-parents = <&k3_clks 357 4>;
@@ -2539,7 +2539,7 @@  watchdog10: watchdog@2310000 {
 	watchdog11: watchdog@2320000 {
 		compatible = "ti,j7-rti-wdt";
 		reg = <0x00 0x2320000 0x00 0x100>;
-		clocks = <&k3_clks 358 1>;
+		clocks = <&k3_clks 358 0>;
 		power-domains = <&k3_pds 358 TI_SCI_PD_EXCLUSIVE>;
 		assigned-clocks = <&k3_clks 358 0>;
 		assigned-clock-parents = <&k3_clks 358 4>;
@@ -2550,7 +2550,7 @@  watchdog11: watchdog@2320000 {
 	watchdog12: watchdog@2330000 {
 		compatible = "ti,j7-rti-wdt";
 		reg = <0x00 0x2330000 0x00 0x100>;
-		clocks = <&k3_clks 359 1>;
+		clocks = <&k3_clks 359 0>;
 		power-domains = <&k3_pds 359 TI_SCI_PD_EXCLUSIVE>;
 		assigned-clocks = <&k3_clks 359 0>;
 		assigned-clock-parents = <&k3_clks 359 4>;
@@ -2561,7 +2561,7 @@  watchdog12: watchdog@2330000 {
 	watchdog13: watchdog@23c0000 {
 		compatible = "ti,j7-rti-wdt";
 		reg = <0x00 0x23c0000 0x00 0x100>;
-		clocks = <&k3_clks 361 1>;
+		clocks = <&k3_clks 361 0>;
 		power-domains = <&k3_pds 361 TI_SCI_PD_EXCLUSIVE>;
 		assigned-clocks = <&k3_clks 361 0>;
 		assigned-clock-parents = <&k3_clks 361 4>;
@@ -2572,7 +2572,7 @@  watchdog13: watchdog@23c0000 {
 	watchdog14: watchdog@23d0000 {
 		compatible = "ti,j7-rti-wdt";
 		reg = <0x00 0x23d0000 0x00 0x100>;
-		clocks = <&k3_clks 362 1>;
+		clocks = <&k3_clks 362 0>;
 		power-domains = <&k3_pds 362 TI_SCI_PD_EXCLUSIVE>;
 		assigned-clocks = <&k3_clks 362 0>;
 		assigned-clock-parents = <&k3_clks 362 4>;
@@ -2583,7 +2583,7 @@  watchdog14: watchdog@23d0000 {
 	watchdog15: watchdog@23e0000 {
 		compatible = "ti,j7-rti-wdt";
 		reg = <0x00 0x23e0000 0x00 0x100>;
-		clocks = <&k3_clks 363 1>;
+		clocks = <&k3_clks 363 0>;
 		power-domains = <&k3_pds 363 TI_SCI_PD_EXCLUSIVE>;
 		assigned-clocks = <&k3_clks 363 0>;
 		assigned-clock-parents = <&k3_clks 363 4>;
@@ -2594,7 +2594,7 @@  watchdog15: watchdog@23e0000 {
 	watchdog16: watchdog@23f0000 {
 		compatible = "ti,j7-rti-wdt";
 		reg = <0x00 0x23f0000 0x00 0x100>;
-		clocks = <&k3_clks 364 1>;
+		clocks = <&k3_clks 364 0>;
 		power-domains = <&k3_pds 364 TI_SCI_PD_EXCLUSIVE>;
 		assigned-clocks = <&k3_clks 364 0>;
 		assigned-clock-parents = <&k3_clks 364 4>;
@@ -2605,7 +2605,7 @@  watchdog16: watchdog@23f0000 {
 	watchdog17: watchdog@2540000 {
 		compatible = "ti,j7-rti-wdt";
 		reg = <0x00 0x2540000 0x00 0x100>;
-		clocks = <&k3_clks 365 1>;
+		clocks = <&k3_clks 365 0>;
 		power-domains = <&k3_pds 365 TI_SCI_PD_EXCLUSIVE>;
 		assigned-clocks = <&k3_clks 365 0>;
 		assigned-clock-parents = <&k3_clks 366 4>;
@@ -2616,7 +2616,7 @@  watchdog17: watchdog@2540000 {
 	watchdog18: watchdog@2550000 {
 		compatible = "ti,j7-rti-wdt";
 		reg = <0x00 0x2550000 0x00 0x100>;
-		clocks = <&k3_clks 366 1>;
+		clocks = <&k3_clks 366 0>;
 		power-domains = <&k3_pds 366 TI_SCI_PD_EXCLUSIVE>;
 		assigned-clocks = <&k3_clks 366 0>;
 		assigned-clock-parents = <&k3_clks 366 4>;