The series add the ESM & main domain watchdog nodes for j721s2,
j784s4 SOCs.
Changes in v7:
* Rebased on top of ti-next branch
* Reordered the watchdog nodes based on the addresses.
* Changed the watchdog numbering.
Changes in v5/v6:
* Updated commit log and added comments for MCU & non-A72 watchdog
instances disabling.
Changes in v4:
* Added bootph-pre-ram for all the ESM instances needed for SPL.
Changes in v3:
* Added all the RTI events for MAIN_ESM for j784s4 as 8 instances
are enabled.
* Rebased on top of 6.6-rc1
* Tested for the watchdog reset
RESEND series - corrected [email protected] ID
Changes in v2:
* Added all the instances of watchdog on j784s4/j721s2
* Fixed all 0x0 in dts to 0x00
* Fixed couple of ESM event numbers for j721s2
* Rebased to linux-next branch
Keerthy (7):
arm64: dts: ti: k3-j721s2: Add ESM instances
arm64: dts: ti: k3-j784s4: Add ESM instances
arm64: dts: ti: k3-j7200: Add MCU domain ESM instance
arm64: dts: ti: k3-j784s4-main: Add the main domain watchdog instances
arm64: dts: ti: k3-j784s4-mcu: Add the mcu domain watchdog instances
arm64: dts: ti: k3-j721s2-main: Add the main domain watchdog instances
arm64: dts: ti: k3-j712s2-mcu: Add the mcu domain watchdog instances
.../boot/dts/ti/k3-j7200-mcu-wakeup.dtsi | 7 +
arch/arm64/boot/dts/ti/k3-j721s2-main.dtsi | 100 +++++++++
.../boot/dts/ti/k3-j721s2-mcu-wakeup.dtsi | 38 ++++
arch/arm64/boot/dts/ti/k3-j784s4-main.dtsi | 195 ++++++++++++++++++
.../boot/dts/ti/k3-j784s4-mcu-wakeup.dtsi | 38 ++++
5 files changed, 378 insertions(+)
--
2.17.1
There are totally 19 instances of watchdog module. One each for the
8 A72 cores, one each for the 4 C7x cores, 1 for the GPU, 1 each
for the 6 R5F cores in the main domain. The non-A72 instances are
coupled with the R5Fs, C7x & GPU instances. Disabling them as they are
not used by Linux.
Signed-off-by: Keerthy <[email protected]>
---
arch/arm64/boot/dts/ti/k3-j784s4-main.dtsi | 187 +++++++++++++++++++++
1 file changed, 187 insertions(+)
diff --git a/arch/arm64/boot/dts/ti/k3-j784s4-main.dtsi b/arch/arm64/boot/dts/ti/k3-j784s4-main.dtsi
index 26dc3776f911..b32c3e668625 100644
--- a/arch/arm64/boot/dts/ti/k3-j784s4-main.dtsi
+++ b/arch/arm64/boot/dts/ti/k3-j784s4-main.dtsi
@@ -1576,4 +1576,191 @@
<695>;
bootph-pre-ram;
};
+
+ watchdog0: watchdog@2200000 {
+ compatible = "ti,j7-rti-wdt";
+ reg = <0x00 0x2200000 0x00 0x100>;
+ clocks = <&k3_clks 348 1>;
+ power-domains = <&k3_pds 348 TI_SCI_PD_EXCLUSIVE>;
+ assigned-clocks = <&k3_clks 348 0>;
+ assigned-clock-parents = <&k3_clks 348 4>;
+ };
+
+ watchdog1: watchdog@2210000 {
+ compatible = "ti,j7-rti-wdt";
+ reg = <0x00 0x2210000 0x00 0x100>;
+ clocks = <&k3_clks 349 1>;
+ power-domains = <&k3_pds 349 TI_SCI_PD_EXCLUSIVE>;
+ assigned-clocks = <&k3_clks 349 0>;
+ assigned-clock-parents = <&k3_clks 349 4>;
+ };
+
+ watchdog2: watchdog@2220000 {
+ compatible = "ti,j7-rti-wdt";
+ reg = <0x00 0x2220000 0x00 0x100>;
+ clocks = <&k3_clks 350 1>;
+ power-domains = <&k3_pds 350 TI_SCI_PD_EXCLUSIVE>;
+ assigned-clocks = <&k3_clks 350 0>;
+ assigned-clock-parents = <&k3_clks 350 4>;
+ };
+
+ watchdog3: watchdog@2230000 {
+ compatible = "ti,j7-rti-wdt";
+ reg = <0x00 0x2230000 0x00 0x100>;
+ clocks = <&k3_clks 351 1>;
+ power-domains = <&k3_pds 351 TI_SCI_PD_EXCLUSIVE>;
+ assigned-clocks = <&k3_clks 351 0>;
+ assigned-clock-parents = <&k3_clks 351 4>;
+ };
+
+ watchdog4: watchdog@2240000 {
+ compatible = "ti,j7-rti-wdt";
+ reg = <0x00 0x2240000 0x00 0x100>;
+ clocks = <&k3_clks 352 1>;
+ power-domains = <&k3_pds 352 TI_SCI_PD_EXCLUSIVE>;
+ assigned-clocks = <&k3_clks 352 0>;
+ assigned-clock-parents = <&k3_clks 352 4>;
+ };
+
+ watchdog5: watchdog@2250000 {
+ compatible = "ti,j7-rti-wdt";
+ reg = <0x00 0x2250000 0x00 0x100>;
+ clocks = <&k3_clks 353 1>;
+ power-domains = <&k3_pds 353 TI_SCI_PD_EXCLUSIVE>;
+ assigned-clocks = <&k3_clks 353 0>;
+ assigned-clock-parents = <&k3_clks 353 4>;
+ };
+
+ watchdog6: watchdog@2260000 {
+ compatible = "ti,j7-rti-wdt";
+ reg = <0x00 0x2260000 0x00 0x100>;
+ clocks = <&k3_clks 354 1>;
+ power-domains = <&k3_pds 354 TI_SCI_PD_EXCLUSIVE>;
+ assigned-clocks = <&k3_clks 354 0>;
+ assigned-clock-parents = <&k3_clks 354 4>;
+ };
+
+ watchdog7: watchdog@2270000 {
+ compatible = "ti,j7-rti-wdt";
+ reg = <0x00 0x2270000 0x00 0x100>;
+ clocks = <&k3_clks 355 1>;
+ power-domains = <&k3_pds 355 TI_SCI_PD_EXCLUSIVE>;
+ assigned-clocks = <&k3_clks 355 0>;
+ assigned-clock-parents = <&k3_clks 355 4>;
+ };
+
+ /*
+ * The following RTI instances are coupled with MCU R5Fs, c7x and
+ * GPU so keeping them disabled as these will be used by their
+ * respective firmware
+ */
+ watchdog8: watchdog@22f0000 {
+ compatible = "ti,j7-rti-wdt";
+ reg = <0x00 0x22f0000 0x00 0x100>;
+ clocks = <&k3_clks 360 1>;
+ power-domains = <&k3_pds 360 TI_SCI_PD_EXCLUSIVE>;
+ assigned-clocks = <&k3_clks 360 0>;
+ assigned-clock-parents = <&k3_clks 360 4>;
+ status = "disabled";
+ };
+
+ watchdog9: watchdog@2300000 {
+ compatible = "ti,j7-rti-wdt";
+ reg = <0x00 0x2300000 0x00 0x100>;
+ clocks = <&k3_clks 356 1>;
+ power-domains = <&k3_pds 356 TI_SCI_PD_EXCLUSIVE>;
+ assigned-clocks = <&k3_clks 356 0>;
+ assigned-clock-parents = <&k3_clks 356 4>;
+ status = "disabled";
+ };
+
+ watchdog10: watchdog@2310000 {
+ compatible = "ti,j7-rti-wdt";
+ reg = <0x00 0x2310000 0x00 0x100>;
+ clocks = <&k3_clks 357 1>;
+ power-domains = <&k3_pds 357 TI_SCI_PD_EXCLUSIVE>;
+ assigned-clocks = <&k3_clks 357 0>;
+ assigned-clock-parents = <&k3_clks 357 4>;
+ status = "disabled";
+ };
+
+ watchdog11: watchdog@2320000 {
+ compatible = "ti,j7-rti-wdt";
+ reg = <0x00 0x2320000 0x00 0x100>;
+ clocks = <&k3_clks 358 1>;
+ power-domains = <&k3_pds 358 TI_SCI_PD_EXCLUSIVE>;
+ assigned-clocks = <&k3_clks 358 0>;
+ assigned-clock-parents = <&k3_clks 358 4>;
+ status = "disabled";
+ };
+
+ watchdog12: watchdog@2330000 {
+ compatible = "ti,j7-rti-wdt";
+ reg = <0x00 0x2330000 0x00 0x100>;
+ clocks = <&k3_clks 359 1>;
+ power-domains = <&k3_pds 359 TI_SCI_PD_EXCLUSIVE>;
+ assigned-clocks = <&k3_clks 359 0>;
+ assigned-clock-parents = <&k3_clks 359 4>;
+ status = "disabled";
+ };
+
+ watchdog13: watchdog@23c0000 {
+ compatible = "ti,j7-rti-wdt";
+ reg = <0x00 0x23c0000 0x00 0x100>;
+ clocks = <&k3_clks 361 1>;
+ power-domains = <&k3_pds 361 TI_SCI_PD_EXCLUSIVE>;
+ assigned-clocks = <&k3_clks 361 0>;
+ assigned-clock-parents = <&k3_clks 361 4>;
+ status = "disabled";
+ };
+
+ watchdog14: watchdog@23d0000 {
+ compatible = "ti,j7-rti-wdt";
+ reg = <0x00 0x23d0000 0x00 0x100>;
+ clocks = <&k3_clks 362 1>;
+ power-domains = <&k3_pds 362 TI_SCI_PD_EXCLUSIVE>;
+ assigned-clocks = <&k3_clks 362 0>;
+ assigned-clock-parents = <&k3_clks 362 4>;
+ status = "disabled";
+ };
+
+ watchdog15: watchdog@23e0000 {
+ compatible = "ti,j7-rti-wdt";
+ reg = <0x00 0x23e0000 0x00 0x100>;
+ clocks = <&k3_clks 363 1>;
+ power-domains = <&k3_pds 363 TI_SCI_PD_EXCLUSIVE>;
+ assigned-clocks = <&k3_clks 363 0>;
+ assigned-clock-parents = <&k3_clks 363 4>;
+ status = "disabled";
+ };
+
+ watchdog16: watchdog@23f0000 {
+ compatible = "ti,j7-rti-wdt";
+ reg = <0x00 0x23f0000 0x00 0x100>;
+ clocks = <&k3_clks 364 1>;
+ power-domains = <&k3_pds 364 TI_SCI_PD_EXCLUSIVE>;
+ assigned-clocks = <&k3_clks 364 0>;
+ assigned-clock-parents = <&k3_clks 364 4>;
+ status = "disabled";
+ };
+
+ watchdog17: watchdog@2540000 {
+ compatible = "ti,j7-rti-wdt";
+ reg = <0x00 0x2540000 0x00 0x100>;
+ clocks = <&k3_clks 365 1>;
+ power-domains = <&k3_pds 365 TI_SCI_PD_EXCLUSIVE>;
+ assigned-clocks = <&k3_clks 365 0>;
+ assigned-clock-parents = <&k3_clks 366 4>;
+ status = "disabled";
+ };
+
+ watchdog18: watchdog@2550000 {
+ compatible = "ti,j7-rti-wdt";
+ reg = <0x00 0x2550000 0x00 0x100>;
+ clocks = <&k3_clks 366 1>;
+ power-domains = <&k3_pds 366 TI_SCI_PD_EXCLUSIVE>;
+ assigned-clocks = <&k3_clks 366 0>;
+ assigned-clock-parents = <&k3_clks 366 4>;
+ status = "disabled";
+ };
};
--
2.17.1
Patch adds the ESM instance for MCU domian of j7200.
Signed-off-by: Keerthy <[email protected]>
---
arch/arm64/boot/dts/ti/k3-j7200-mcu-wakeup.dtsi | 7 +++++++
1 file changed, 7 insertions(+)
diff --git a/arch/arm64/boot/dts/ti/k3-j7200-mcu-wakeup.dtsi b/arch/arm64/boot/dts/ti/k3-j7200-mcu-wakeup.dtsi
index 6ffaf85fa63f..711690c0cba4 100644
--- a/arch/arm64/boot/dts/ti/k3-j7200-mcu-wakeup.dtsi
+++ b/arch/arm64/boot/dts/ti/k3-j7200-mcu-wakeup.dtsi
@@ -637,4 +637,11 @@
power-domains = <&k3_pds 154 TI_SCI_PD_EXCLUSIVE>;
#thermal-sensor-cells = <1>;
};
+
+ mcu_esm: esm@40800000 {
+ compatible = "ti,j721e-esm";
+ reg = <0x00 0x40800000 0x00 0x1000>;
+ ti,esm-pins = <95>;
+ bootph-pre-ram;
+ };
};
--
2.17.1
There are totally 2 instances of watchdog module in MCU domain.
These instances are coupled with the MCU domain R5F instances.
Disabling them as they are not used by Linux.
Signed-off-by: Keerthy <[email protected]>
---
.../boot/dts/ti/k3-j784s4-mcu-wakeup.dtsi | 24 +++++++++++++++++++
1 file changed, 24 insertions(+)
diff --git a/arch/arm64/boot/dts/ti/k3-j784s4-mcu-wakeup.dtsi b/arch/arm64/boot/dts/ti/k3-j784s4-mcu-wakeup.dtsi
index a7b5c4cb7d3e..809a0b1cf038 100644
--- a/arch/arm64/boot/dts/ti/k3-j784s4-mcu-wakeup.dtsi
+++ b/arch/arm64/boot/dts/ti/k3-j784s4-mcu-wakeup.dtsi
@@ -714,4 +714,28 @@
ti,esm-pins = <63>;
bootph-pre-ram;
};
+
+ /*
+ * The 2 RTI instances are couple with MCU R5Fs so keeping them
+ * disabled as these will be used by their respective firmware
+ */
+ mcu_watchdog0: watchdog@40600000 {
+ compatible = "ti,j7-rti-wdt";
+ reg = <0x00 0x40600000 0x00 0x100>;
+ clocks = <&k3_clks 367 1>;
+ power-domains = <&k3_pds 367 TI_SCI_PD_EXCLUSIVE>;
+ assigned-clocks = <&k3_clks 367 0>;
+ assigned-clock-parents = <&k3_clks 367 4>;
+ status = "disabled";
+ };
+
+ mcu_watchdog1: watchdog@40610000 {
+ compatible = "ti,j7-rti-wdt";
+ reg = <0x00 0x40610000 0x00 0x100>;
+ clocks = <&k3_clks 368 1>;
+ power-domains = <&k3_pds 368 TI_SCI_PD_EXCLUSIVE>;
+ assigned-clocks = <&k3_clks 368 0>;
+ assigned-clock-parents = <&k3_clks 368 4>;
+ status = "disabled";
+ };
};
--
2.17.1
There are totally 2 instances of watchdog module in MCU domain.
These instances are coupled with the MCU domain R5F instances.
Disabling them as they are not used by Linux.
Signed-off-by: Keerthy <[email protected]>
---
.../boot/dts/ti/k3-j721s2-mcu-wakeup.dtsi | 24 +++++++++++++++++++
1 file changed, 24 insertions(+)
diff --git a/arch/arm64/boot/dts/ti/k3-j721s2-mcu-wakeup.dtsi b/arch/arm64/boot/dts/ti/k3-j721s2-mcu-wakeup.dtsi
index f94aa3a34e22..0ef6b7482371 100644
--- a/arch/arm64/boot/dts/ti/k3-j721s2-mcu-wakeup.dtsi
+++ b/arch/arm64/boot/dts/ti/k3-j721s2-mcu-wakeup.dtsi
@@ -709,4 +709,28 @@
ti,esm-pins = <63>;
bootph-pre-ram;
};
+
+ /*
+ * The 2 RTI instances are couple with MCU R5Fs so keeping them
+ * disabled as these will be used by their respective firmware
+ */
+ mcu_watchdog0: watchdog@40600000 {
+ compatible = "ti,j7-rti-wdt";
+ reg = <0x00 0x40600000 0x00 0x100>;
+ clocks = <&k3_clks 295 1>;
+ power-domains = <&k3_pds 295 TI_SCI_PD_EXCLUSIVE>;
+ assigned-clocks = <&k3_clks 295 1>;
+ assigned-clock-parents = <&k3_clks 295 5>;
+ status = "disabled";
+ };
+
+ mcu_watchdog1: watchdog@40610000 {
+ compatible = "ti,j7-rti-wdt";
+ reg = <0x00 0x40610000 0x00 0x100>;
+ clocks = <&k3_clks 296 1>;
+ power-domains = <&k3_pds 296 TI_SCI_PD_EXCLUSIVE>;
+ assigned-clocks = <&k3_clks 296 1>;
+ assigned-clock-parents = <&k3_clks 296 5>;
+ status = "disabled";
+ };
};
--
2.17.1
There are totally 9 instances of watchdog module. One each for the
2 A72 cores, one each for the 2 C7x cores, 1 for the GPU, 1 each
for the 4 R5F cores in the main domain. Keeping only the A72 instances
enabled and disabling the rest by default as they will be used by
their respective firmware.
Signed-off-by: Keerthy <[email protected]>
---
arch/arm64/boot/dts/ti/k3-j721s2-main.dtsi | 93 ++++++++++++++++++++++
1 file changed, 93 insertions(+)
diff --git a/arch/arm64/boot/dts/ti/k3-j721s2-main.dtsi b/arch/arm64/boot/dts/ti/k3-j721s2-main.dtsi
index 7ce802c6808d..0a05aad6be49 100644
--- a/arch/arm64/boot/dts/ti/k3-j721s2-main.dtsi
+++ b/arch/arm64/boot/dts/ti/k3-j721s2-main.dtsi
@@ -1808,4 +1808,97 @@
ti,esm-pins = <688>, <689>;
bootph-pre-ram;
};
+
+ watchdog0: watchdog@2200000 {
+ compatible = "ti,j7-rti-wdt";
+ reg = <0x00 0x2200000 0x00 0x100>;
+ clocks = <&k3_clks 286 1>;
+ power-domains = <&k3_pds 286 TI_SCI_PD_EXCLUSIVE>;
+ assigned-clocks = <&k3_clks 286 1>;
+ assigned-clock-parents = <&k3_clks 286 5>;
+ };
+
+ watchdog1: watchdog@2210000 {
+ compatible = "ti,j7-rti-wdt";
+ reg = <0x00 0x2210000 0x00 0x100>;
+ clocks = <&k3_clks 287 1>;
+ power-domains = <&k3_pds 287 TI_SCI_PD_EXCLUSIVE>;
+ assigned-clocks = <&k3_clks 287 1>;
+ assigned-clock-parents = <&k3_clks 287 5>;
+ };
+
+ /*
+ * The following RTI instances are coupled with MCU R5Fs, c7x and
+ * GPU so keeping them disabled as these will be used by their
+ * respective firmware
+ */
+ watchdog2: watchdog@22f0000 {
+ compatible = "ti,j7-rti-wdt";
+ reg = <0x00 0x22f0000 0x00 0x100>;
+ clocks = <&k3_clks 290 1>;
+ power-domains = <&k3_pds 290 TI_SCI_PD_EXCLUSIVE>;
+ assigned-clocks = <&k3_clks 290 1>;
+ assigned-clock-parents = <&k3_clks 290 5>;
+ status = "disabled";
+ };
+
+ watchdog3: watchdog@2300000 {
+ compatible = "ti,j7-rti-wdt";
+ reg = <0x00 0x2300000 0x00 0x100>;
+ clocks = <&k3_clks 288 1>;
+ power-domains = <&k3_pds 288 TI_SCI_PD_EXCLUSIVE>;
+ assigned-clocks = <&k3_clks 288 1>;
+ assigned-clock-parents = <&k3_clks 288 5>;
+ status = "disabled";
+ };
+
+ watchdog4: watchdog@2310000 {
+ compatible = "ti,j7-rti-wdt";
+ reg = <0x00 0x2310000 0x00 0x100>;
+ clocks = <&k3_clks 289 1>;
+ power-domains = <&k3_pds 289 TI_SCI_PD_EXCLUSIVE>;
+ assigned-clocks = <&k3_clks 289 1>;
+ assigned-clock-parents = <&k3_clks 289 5>;
+ status = "disabled";
+ };
+
+ watchdog5: watchdog@23c0000 {
+ compatible = "ti,j7-rti-wdt";
+ reg = <0x00 0x23c0000 0x00 0x100>;
+ clocks = <&k3_clks 291 1>;
+ power-domains = <&k3_pds 291 TI_SCI_PD_EXCLUSIVE>;
+ assigned-clocks = <&k3_clks 291 1>;
+ assigned-clock-parents = <&k3_clks 291 5>;
+ status = "disabled";
+ };
+
+ watchdog6: watchdog@23d0000 {
+ compatible = "ti,j7-rti-wdt";
+ reg = <0x00 0x23d0000 0x00 0x100>;
+ clocks = <&k3_clks 292 1>;
+ power-domains = <&k3_pds 292 TI_SCI_PD_EXCLUSIVE>;
+ assigned-clocks = <&k3_clks 292 1>;
+ assigned-clock-parents = <&k3_clks 292 5>;
+ status = "disabled";
+ };
+
+ watchdog7: watchdog@23e0000 {
+ compatible = "ti,j7-rti-wdt";
+ reg = <0x00 0x23e0000 0x00 0x100>;
+ clocks = <&k3_clks 293 1>;
+ power-domains = <&k3_pds 293 TI_SCI_PD_EXCLUSIVE>;
+ assigned-clocks = <&k3_clks 293 1>;
+ assigned-clock-parents = <&k3_clks 293 5>;
+ status = "disabled";
+ };
+
+ watchdog8: watchdog@23f0000 {
+ compatible = "ti,j7-rti-wdt";
+ reg = <0x00 0x23f0000 0x00 0x100>;
+ clocks = <&k3_clks 294 1>;
+ power-domains = <&k3_pds 294 TI_SCI_PD_EXCLUSIVE>;
+ assigned-clocks = <&k3_clks 294 1>;
+ assigned-clock-parents = <&k3_clks 294 5>;
+ status = "disabled";
+ };
};
--
2.17.1
On 09:58-20231006, Keerthy wrote:
> There are totally 2 instances of watchdog module in MCU domain.
> These instances are coupled with the MCU domain R5F instances.
> Disabling them as they are not used by Linux.
Device tree is hardware description - not tied to how Linux uses it.
Reason these wdts are disabled by default is because they are tightly
coupled with R5Fs.
>
> Signed-off-by: Keerthy <[email protected]>
> ---
> .../boot/dts/ti/k3-j784s4-mcu-wakeup.dtsi | 24 +++++++++++++++++++
> 1 file changed, 24 insertions(+)
>
> diff --git a/arch/arm64/boot/dts/ti/k3-j784s4-mcu-wakeup.dtsi b/arch/arm64/boot/dts/ti/k3-j784s4-mcu-wakeup.dtsi
> index a7b5c4cb7d3e..809a0b1cf038 100644
> --- a/arch/arm64/boot/dts/ti/k3-j784s4-mcu-wakeup.dtsi
> +++ b/arch/arm64/boot/dts/ti/k3-j784s4-mcu-wakeup.dtsi
> @@ -714,4 +714,28 @@
> ti,esm-pins = <63>;
> bootph-pre-ram;
> };
> +
> + /*
> + * The 2 RTI instances are couple with MCU R5Fs so keeping them
> + * disabled as these will be used by their respective firmware
> + */
> + mcu_watchdog0: watchdog@40600000 {
> + compatible = "ti,j7-rti-wdt";
> + reg = <0x00 0x40600000 0x00 0x100>;
> + clocks = <&k3_clks 367 1>;
> + power-domains = <&k3_pds 367 TI_SCI_PD_EXCLUSIVE>;
> + assigned-clocks = <&k3_clks 367 0>;
> + assigned-clock-parents = <&k3_clks 367 4>;
> + status = "disabled";
> + };
> +
> + mcu_watchdog1: watchdog@40610000 {
> + compatible = "ti,j7-rti-wdt";
> + reg = <0x00 0x40610000 0x00 0x100>;
> + clocks = <&k3_clks 368 1>;
> + power-domains = <&k3_pds 368 TI_SCI_PD_EXCLUSIVE>;
> + assigned-clocks = <&k3_clks 368 0>;
> + assigned-clock-parents = <&k3_clks 368 4>;
Please DONOT ignore the review comments - I did ask the documentation in
dts as well. reason being that this is what people will see rather than
dig up the commit log. it should be intutive when reading the dts why
nodes are disabled by default Vs the standard of leaving it enabled by
default. Given esp that these peripherals do not have anything to do
with board semantics (pinmux or something similar) to be complete.
> + status = "disabled";
> + };
> };
> --
> 2.17.1
>
--
Regards,
Nishanth Menon
Key (0xDDB5849D1736249D) / Fingerprint: F8A2 8693 54EB 8232 17A3 1A34 DDB5 849D 1736 249D
> > + mcu_watchdog1: watchdog@40610000 {
> > + compatible = "ti,j7-rti-wdt";
> > + reg = <0x00 0x40610000 0x00 0x100>;
> > + clocks = <&k3_clks 368 1>;
> > + power-domains = <&k3_pds 368 TI_SCI_PD_EXCLUSIVE>;
> > + assigned-clocks = <&k3_clks 368 0>;
> > + assigned-clock-parents = <&k3_clks 368 4>;
>
> Please DONOT ignore the review comments - I did ask the documentation in
> dts as well. reason being that this is what people will see rather than
> dig up the commit log. it should be intutive when reading the dts why
> nodes are disabled by default Vs the standard of leaving it enabled by
> default. Given esp that these peripherals do not have anything to do
> with board semantics (pinmux or something similar) to be complete.
>
> > + status = "disabled";
Just providing clarifying comment - something like this is probably more
appropriate:
/* Tightly coupled to R5F */
status = "reserved";
The rti needs to be handled by R5F and is reserved for firmware usage.
--
Regards,
Nishanth Menon
Key (0xDDB5849D1736249D) / Fingerprint: F8A2 8693 54EB 8232 17A3 1A34 DDB5 849D 1736 249D
On 10/6/2023 5:04 PM, Nishanth Menon wrote:
> On 09:58-20231006, Keerthy wrote:
>> There are totally 2 instances of watchdog module in MCU domain.
>> These instances are coupled with the MCU domain R5F instances.
>
>> Disabling them as they are not used by Linux.
> Device tree is hardware description - not tied to how Linux uses it.
>
> Reason these wdts are disabled by default is because they are tightly
> coupled with R5Fs.
>
>>
>> Signed-off-by: Keerthy <[email protected]>
>> ---
>> .../boot/dts/ti/k3-j784s4-mcu-wakeup.dtsi | 24 +++++++++++++++++++
>> 1 file changed, 24 insertions(+)
>>
>> diff --git a/arch/arm64/boot/dts/ti/k3-j784s4-mcu-wakeup.dtsi b/arch/arm64/boot/dts/ti/k3-j784s4-mcu-wakeup.dtsi
>> index a7b5c4cb7d3e..809a0b1cf038 100644
>> --- a/arch/arm64/boot/dts/ti/k3-j784s4-mcu-wakeup.dtsi
>> +++ b/arch/arm64/boot/dts/ti/k3-j784s4-mcu-wakeup.dtsi
>> @@ -714,4 +714,28 @@
>> ti,esm-pins = <63>;
>> bootph-pre-ram;
>> };
>> +
Nishanth,
Below i have addressed the coupling with R5Fs & MCU domains watcdogs.
>> + /*
>> + * The 2 RTI instances are couple with MCU R5Fs so keeping them
>> + * disabled as these will be used by their respective firmware
>> + */
>> + mcu_watchdog0: watchdog@40600000 {
>> + compatible = "ti,j7-rti-wdt";
>> + reg = <0x00 0x40600000 0x00 0x100>;
>> + clocks = <&k3_clks 367 1>;
>> + power-domains = <&k3_pds 367 TI_SCI_PD_EXCLUSIVE>;
>> + assigned-clocks = <&k3_clks 367 0>;
>> + assigned-clock-parents = <&k3_clks 367 4>;
>> + status = "disabled";
>> + };
>> +
>> + mcu_watchdog1: watchdog@40610000 {
>> + compatible = "ti,j7-rti-wdt";
>> + reg = <0x00 0x40610000 0x00 0x100>;
>> + clocks = <&k3_clks 368 1>;
>> + power-domains = <&k3_pds 368 TI_SCI_PD_EXCLUSIVE>;
>> + assigned-clocks = <&k3_clks 368 0>;
>> + assigned-clock-parents = <&k3_clks 368 4>;
>
> Please DONOT ignore the review comments - I did ask the documentation in
> dts as well. reason being that this is what people will see rather than
> dig up the commit log. it should be intutive when reading the dts why
> nodes are disabled by default Vs the standard of leaving it enabled by
> default. Given esp that these peripherals do not have anything to do
> with board semantics (pinmux or something similar) to be complete.
As mentioned above. I added single comment for addressing both the
watchdogs.
- Keerthy
>
>> + status = "disabled";
>> + };
>> };
>> --
>> 2.17.1
>>
>
On 10/6/2023 5:04 PM, Nishanth Menon wrote:
> On 09:58-20231006, Keerthy wrote:
>> There are totally 2 instances of watchdog module in MCU domain.
>> These instances are coupled with the MCU domain R5F instances.
>
>> Disabling them as they are not used by Linux.
> Device tree is hardware description - not tied to how Linux uses it.
>
> Reason these wdts are disabled by default is because they are tightly
> coupled with R5Fs.
Okay. I will rephrase that.
>
>>
>> Signed-off-by: Keerthy <[email protected]>
>> ---
>> .../boot/dts/ti/k3-j784s4-mcu-wakeup.dtsi | 24 +++++++++++++++++++
>> 1 file changed, 24 insertions(+)
>>
>> diff --git a/arch/arm64/boot/dts/ti/k3-j784s4-mcu-wakeup.dtsi b/arch/arm64/boot/dts/ti/k3-j784s4-mcu-wakeup.dtsi
>> index a7b5c4cb7d3e..809a0b1cf038 100644
>> --- a/arch/arm64/boot/dts/ti/k3-j784s4-mcu-wakeup.dtsi
>> +++ b/arch/arm64/boot/dts/ti/k3-j784s4-mcu-wakeup.dtsi
>> @@ -714,4 +714,28 @@
>> ti,esm-pins = <63>;
>> bootph-pre-ram;
>> };
>> +
>> + /*
>> + * The 2 RTI instances are couple with MCU R5Fs so keeping them
>> + * disabled as these will be used by their respective firmware
>> + */
>> + mcu_watchdog0: watchdog@40600000 {
>> + compatible = "ti,j7-rti-wdt";
>> + reg = <0x00 0x40600000 0x00 0x100>;
>> + clocks = <&k3_clks 367 1>;
>> + power-domains = <&k3_pds 367 TI_SCI_PD_EXCLUSIVE>;
>> + assigned-clocks = <&k3_clks 367 0>;
>> + assigned-clock-parents = <&k3_clks 367 4>;
>> + status = "disabled";
>> + };
>> +
>> + mcu_watchdog1: watchdog@40610000 {
>> + compatible = "ti,j7-rti-wdt";
>> + reg = <0x00 0x40610000 0x00 0x100>;
>> + clocks = <&k3_clks 368 1>;
>> + power-domains = <&k3_pds 368 TI_SCI_PD_EXCLUSIVE>;
>> + assigned-clocks = <&k3_clks 368 0>;
>> + assigned-clock-parents = <&k3_clks 368 4>;
>
> Please DONOT ignore the review comments - I did ask the documentation in
> dts as well. reason being that this is what people will see rather than
> dig up the commit log. it should be intutive when reading the dts why
> nodes are disabled by default Vs the standard of leaving it enabled by
> default. Given esp that these peripherals do not have anything to do
> with board semantics (pinmux or something similar) to be complete.
>
>> + status = "disabled";
>> + };
>> };
>> --
>> 2.17.1
>>
>
On 17:22-20231006, J, KEERTHY wrote:
>
>
> On 10/6/2023 5:04 PM, Nishanth Menon wrote:
> > On 09:58-20231006, Keerthy wrote:
> > > There are totally 2 instances of watchdog module in MCU domain.
> > > These instances are coupled with the MCU domain R5F instances.
> >
> > > Disabling them as they are not used by Linux.
> > Device tree is hardware description - not tied to how Linux uses it.
> >
> > Reason these wdts are disabled by default is because they are tightly
> > coupled with R5Fs.
> >
> > >
> > > Signed-off-by: Keerthy <[email protected]>
> > > ---
> > > .../boot/dts/ti/k3-j784s4-mcu-wakeup.dtsi | 24 +++++++++++++++++++
> > > 1 file changed, 24 insertions(+)
> > >
> > > diff --git a/arch/arm64/boot/dts/ti/k3-j784s4-mcu-wakeup.dtsi b/arch/arm64/boot/dts/ti/k3-j784s4-mcu-wakeup.dtsi
> > > index a7b5c4cb7d3e..809a0b1cf038 100644
> > > --- a/arch/arm64/boot/dts/ti/k3-j784s4-mcu-wakeup.dtsi
> > > +++ b/arch/arm64/boot/dts/ti/k3-j784s4-mcu-wakeup.dtsi
> > > @@ -714,4 +714,28 @@
> > > ti,esm-pins = <63>;
> > > bootph-pre-ram;
> > > };
> > > +
>
> Nishanth,
>
> Below i have addressed the coupling with R5Fs & MCU domains watcdogs.
>
> > > + /*
> > > + * The 2 RTI instances are couple with MCU R5Fs so keeping them
> > > + * disabled as these will be used by their respective firmware
This description is best in the commit message
> > > + */
> > > + mcu_watchdog0: watchdog@40600000 {
> > > + compatible = "ti,j7-rti-wdt";
> > > + reg = <0x00 0x40600000 0x00 0x100>;
> > > + clocks = <&k3_clks 367 1>;
> > > + power-domains = <&k3_pds 367 TI_SCI_PD_EXCLUSIVE>;
> > > + assigned-clocks = <&k3_clks 367 0>;
> > > + assigned-clock-parents = <&k3_clks 367 4>;
> > > + status = "disabled";
> > > + };
> > > +
> > > + mcu_watchdog1: watchdog@40610000 {
> > > + compatible = "ti,j7-rti-wdt";
> > > + reg = <0x00 0x40610000 0x00 0x100>;
> > > + clocks = <&k3_clks 368 1>;
> > > + power-domains = <&k3_pds 368 TI_SCI_PD_EXCLUSIVE>;
> > > + assigned-clocks = <&k3_clks 368 0>;
> > > + assigned-clock-parents = <&k3_clks 368 4>;
> >
> > Please DONOT ignore the review comments - I did ask the documentation in
> > dts as well. reason being that this is what people will see rather than
> > dig up the commit log. it should be intutive when reading the dts why
> > nodes are disabled by default Vs the standard of leaving it enabled by
> > default. Given esp that these peripherals do not have anything to do
> > with board semantics (pinmux or something similar) to be complete.
>
> As mentioned above. I added single comment for addressing both the
> watchdogs.
I missed it completely. Now that I think of it, I seem to have missed
having seen it in previous rev reviews as well, and there is a reason
for it: See [1] clarifying comment - nodes reserved for firmware usage
have convention of "reserved" as status and documentation immediately
above the status to help clarify the reason in-context. That is more
readable than having to scroll up to find the rationale.
[1] https://lore.kernel.org/all/20231006114422.avymeap7h5ocs6zq@dreadlock/
--
Regards,
Nishanth Menon
Key (0xDDB5849D1736249D) / Fingerprint: F8A2 8693 54EB 8232 17A3 1A34 DDB5 849D 1736 249D
On 10/6/2023 5:31 PM, Nishanth Menon wrote:
> On 17:22-20231006, J, KEERTHY wrote:
>>
>>
>> On 10/6/2023 5:04 PM, Nishanth Menon wrote:
>>> On 09:58-20231006, Keerthy wrote:
>>>> There are totally 2 instances of watchdog module in MCU domain.
>>>> These instances are coupled with the MCU domain R5F instances.
>>>
>>>> Disabling them as they are not used by Linux.
>>> Device tree is hardware description - not tied to how Linux uses it.
>>>
>>> Reason these wdts are disabled by default is because they are tightly
>>> coupled with R5Fs.
>>>
>>>>
>>>> Signed-off-by: Keerthy <[email protected]>
>>>> ---
>>>> .../boot/dts/ti/k3-j784s4-mcu-wakeup.dtsi | 24 +++++++++++++++++++
>>>> 1 file changed, 24 insertions(+)
>>>>
>>>> diff --git a/arch/arm64/boot/dts/ti/k3-j784s4-mcu-wakeup.dtsi b/arch/arm64/boot/dts/ti/k3-j784s4-mcu-wakeup.dtsi
>>>> index a7b5c4cb7d3e..809a0b1cf038 100644
>>>> --- a/arch/arm64/boot/dts/ti/k3-j784s4-mcu-wakeup.dtsi
>>>> +++ b/arch/arm64/boot/dts/ti/k3-j784s4-mcu-wakeup.dtsi
>>>> @@ -714,4 +714,28 @@
>>>> ti,esm-pins = <63>;
>>>> bootph-pre-ram;
>>>> };
>>>> +
>>
>> Nishanth,
>>
>> Below i have addressed the coupling with R5Fs & MCU domains watcdogs.
>>
>>>> + /*
>>>> + * The 2 RTI instances are couple with MCU R5Fs so keeping them
>>>> + * disabled as these will be used by their respective firmware
>
> This description is best in the commit message
>
>>>> + */
>>>> + mcu_watchdog0: watchdog@40600000 {
>>>> + compatible = "ti,j7-rti-wdt";
>>>> + reg = <0x00 0x40600000 0x00 0x100>;
>>>> + clocks = <&k3_clks 367 1>;
>>>> + power-domains = <&k3_pds 367 TI_SCI_PD_EXCLUSIVE>;
>>>> + assigned-clocks = <&k3_clks 367 0>;
>>>> + assigned-clock-parents = <&k3_clks 367 4>;
>>>> + status = "disabled";
>>>> + };
>>>> +
>>>> + mcu_watchdog1: watchdog@40610000 {
>>>> + compatible = "ti,j7-rti-wdt";
>>>> + reg = <0x00 0x40610000 0x00 0x100>;
>>>> + clocks = <&k3_clks 368 1>;
>>>> + power-domains = <&k3_pds 368 TI_SCI_PD_EXCLUSIVE>;
>>>> + assigned-clocks = <&k3_clks 368 0>;
>>>> + assigned-clock-parents = <&k3_clks 368 4>;
>>>
>>> Please DONOT ignore the review comments - I did ask the documentation in
>>> dts as well. reason being that this is what people will see rather than
>>> dig up the commit log. it should be intutive when reading the dts why
>>> nodes are disabled by default Vs the standard of leaving it enabled by
>>> default. Given esp that these peripherals do not have anything to do
>>> with board semantics (pinmux or something similar) to be complete.
>>
>> As mentioned above. I added single comment for addressing both the
>> watchdogs.
>
> I missed it completely. Now that I think of it, I seem to have missed
> having seen it in previous rev reviews as well, and there is a reason
> for it: See [1] clarifying comment - nodes reserved for firmware usage
> have convention of "reserved" as status and documentation immediately
> above the status to help clarify the reason in-context. That is more
> readable than having to scroll up to find the rationale.
>
> [1] https://lore.kernel.org/all/20231006114422.avymeap7h5ocs6zq@dreadlock/
Thanks Nishanth. I agree reserved is better and I will add a comment
something like below:
/* Tightly coupled to R5F */
status = "reserved";
- Keerthy
>