dt-bindings: watchdog: marvell GTI system watchdog driver

PATCH Superseded Bharat Bhushan 2 participants 18 messages May 3, 2023 – May 5, 2023 1 message not in the archive

Subsystems (2)

Device tree bindings Watchdog device drivers

lore

Share & tools

lore

Bharat Bhushan

author

2023-05-03 12:10 UTC #1 lore raw

PATCH v5 1/2 dt-bindings: watchdog: marvell GTI system watchdog driver

In reply to a message not in the archive

Add binding documentation for the Marvell GTI system watchdog driver.

Signed-off-by Bharat Bhushan <[email protected]>

---
v5:
- Added wdt-timer-index property
- Get clock frequency from clocks/clock-name device tree property

 .../bindings/watchdog/marvell,gti-wdt.yaml    | 54 +++++++++++++++++++
 1 file changed, 54 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/watchdog/marvell,gti-wdt.yaml

Documentation/devicetree/bindings/watchdog/marvell,gti-wdt.yaml new file +54 -0

new file mode 100644
index 000000000000..e3315653f961
@@ -0,0 +1,54 @@
+# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
+%YAML 1.2
+---
+$id: http://devicetree.org/schemas/watchdog/marvell,gti-wdt.yaml#
+$schema: http://devicetree.org/meta-schemas/core.yaml#
+
+title: Marvell Global Timer (GTI) system watchdog
+
+allOf:
+  - $ref: watchdog.yaml#
+
+maintainers:
+  - Bharat Bhushan <[email protected]>
+
+properties:
+  compatible:
+    const: marvell,gti-wdt
+
+  reg:
+    maxItems: 1
+
+  interrupts:
+    maxItems: 1
+
+  wdt-timer-index:
+    maxItems: 1
+    description:
+      This contains the timer number out of total 64 timers supported
+      by GTI hardware block.
+
+required:
+  - compatible
+  - reg
+  - interrupts
+  - wdt-timer-index
+
+unevaluatedProperties: false
+
+examples:
+  - |
+    #include <dt-bindings/interrupt-controller/irq.h>
+    soc {
+        #address-cells = <2>;
+        #size-cells = <2>;
+
+        watchdog@802000040000 {
+            compatible = "marvell,gti-wdt";
+            reg = <0x00008020 0x00040000 0x00000000 0x00020000>;
+            interrupts = <0 38 IRQ_TYPE_EDGE_RISING>;
+            wdt-timer-index = <63>;
+        };
+    };
+
+...

Signature · 1 line

2.17.1

Bharat Bhushan

author

2023-05-03 12:10 UTC #2 lore raw

PATCH v5 2/2 Watchdog: Add marvell GTI watchdog driver

This patch add support for Marvell GTI watchdog. Global timer unit (GTI) support hardware watchdog timer. Software programs watchdog timer to generate interrupt on first timeout, second timeout is configured to be ignored and system reboots on third timeout.

Signed-off-by Bharat Bhushan <[email protected]>

---
v5:
- Get clock frequency from clocks/clock-name device tree property So removed arch_timer_get_cntfrq() and asm include arch_timer.h
- Removed GTI_CWD_GLOBAL_WDOG_IDX, use wdt-timer-index device tree property for timer
- Some other clean-up as per review comment

 drivers/watchdog/Kconfig           |  11 +
 drivers/watchdog/Makefile          |   1 +
 drivers/watchdog/marvell_gti_wdt.c | 326 +++++++++++++++++++++++++++++
 3 files changed, 338 insertions(+)
 create mode 100644 drivers/watchdog/marvell_gti_wdt.c

drivers/watchdog/Kconfig +11 -0

index f0872970daf9..a96e7d7c0ad2 100644
@@ -1779,6 +1779,17 @@ config OCTEON_WDT
 	  from the first interrupt, it is then only poked when the
 	  device is written.
 
+config MARVELL_GTI_WDT
+	tristate "Marvell GTI Watchdog driver"
+	depends on ARCH_THUNDER || COMPILE_TEST
+	help
+	 Marvell GTI hardware supports watchdog timer. First timeout
+	 works as watchdog pretimeout and installed interrupt handler
+	 will be called on first timeout. Hardware can generate interrupt
+	 to SCP on second timeout but it is not enabled, So second
+	 timeout is ignored. If device poke does not happen then system
+	 will reboot on third timeout.
+
 config BCM2835_WDT
 	tristate "Broadcom BCM2835 hardware watchdog"
 	depends on ARCH_BCM2835 || (OF && COMPILE_TEST)

drivers/watchdog/Makefile +1 -0

index 9cbf6580f16c..bd425408fcaa 100644
@@ -98,6 +98,7 @@ obj-$(CONFIG_VISCONTI_WATCHDOG) += visconti_wdt.o
 obj-$(CONFIG_MSC313E_WATCHDOG) += msc313e_wdt.o
 obj-$(CONFIG_APPLE_WATCHDOG) += apple_wdt.o
 obj-$(CONFIG_SUNPLUS_WATCHDOG) += sunplus_wdt.o
+obj-$(CONFIG_MARVELL_GTI_WDT) += marvell_gti_wdt.o
 
 # X86 (i386 + ia64 + x86_64) Architecture
 obj-$(CONFIG_ACQUIRE_WDT) += acquirewdt.o

drivers/watchdog/marvell_gti_wdt.c new file +326 -0

326 lines not on this page · raw message

Signature · 1 line

2.17.1

Krzysztof Kozlowski

2023-05-04 06:55 UTC #4 lore raw

In reply to Bharat Bhushan

Quoted 7 lines · Bharat Bhushan

Add binding documentation for the Marvell GTI system watchdog driver.
Signed-off-by Bharat Bhushan <[email protected]>
---
v5:
- Added wdt-timer-index property

I did not ask for it...

Quoted 1 line

- Get clock frequency from clocks/clock-name device tree property

Where? It's not possible in current code. I don't think you tested this at all.

Quoted 26 lines

 .../bindings/watchdog/marvell,gti-wdt.yaml    | 54 +++++++++++++++++++
 1 file changed, 54 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/watchdog/marvell,gti-wdt.yaml

Documentation/devicetree/bindings/watchdog/marvell,gti-wdt.yaml new file +15 -0

new file mode 100644
index 000000000000..e3315653f961
@@ -0,0 +1,54 @@
+# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
+%YAML 1.2
+---
+$id: http://devicetree.org/schemas/watchdog/marvell,gti-wdt.yaml#
+$schema: http://devicetree.org/meta-schemas/core.yaml#
+
+title: Marvell Global Timer (GTI) system watchdog
+
+allOf:
+  - $ref: watchdog.yaml#
+
+maintainers:
+  - Bharat Bhushan <[email protected]>
+
+properties:

Documentation/devicetree/bindings/watchdog/marvell,gti-wdt.yaml line 25

Quoted 10 lines

Documentation/devicetree/bindings/watchdog/marvell,gti-wdt.yaml new file +10 -0
+  compatible:
+    const: marvell,gti-wdt
+
+  reg:
+    maxItems: 1
+
+  interrupts:
+    maxItems: 1
+
+  wdt-timer-index:

missing vendor prefix

missing type

Documentation/devicetree/bindings/watchdog/marvell,gti-wdt.yaml line 26

Quoted 1 line

Documentation/devicetree/bindings/watchdog/marvell,gti-wdt.yaml new file +1 -0
+    maxItems: 1

???

Documentation/devicetree/bindings/watchdog/marvell,gti-wdt.yaml line 29

Quoted 3 lines

Documentation/devicetree/bindings/watchdog/marvell,gti-wdt.yaml new file +3 -0
+    description:
+      This contains the timer number out of total 64 timers supported
+      by GTI hardware block.

Why do you need it? What does it represent?

We do not keep indices of devices other than something in reg, so please justify why exception must be made here.

Quoted 10 lines

Documentation/devicetree/bindings/watchdog/marvell,gti-wdt.yaml new file +10 -0
+
+required:
+  - compatible
+  - reg
+  - interrupts
+  - wdt-timer-index
+
+unevaluatedProperties: false
+
+examples:

Documentation/devicetree/bindings/watchdog/marvell,gti-wdt.yaml line 49

Quoted 10 lines

Documentation/devicetree/bindings/watchdog/marvell,gti-wdt.yaml new file +10 -0

+  - |
+    #include <dt-bindings/interrupt-controller/irq.h>
+    soc {
+        #address-cells = <2>;
+        #size-cells = <2>;
+
+        watchdog@802000040000 {
+            compatible = "marvell,gti-wdt";
+            reg = <0x00008020 0x00040000 0x00000000 0x00020000>;
+            interrupts = <0 38 IRQ_TYPE_EDGE_RISING>;

Use defines for flags.

Documentation/devicetree/bindings/watchdog/marvell,gti-wdt.yaml line 50

Quoted 1 line

Documentation/devicetree/bindings/watchdog/marvell,gti-wdt.yaml new file +1 -0
+            wdt-timer-index = <63>;

Best regards,
Krzysztof

Krzysztof Kozlowski

2023-05-04 06:56 UTC #3 lore raw

Re: [PATCH 2/2 v5] Watchdog: Add marvell GTI watchdog driver

In reply to Bharat Bhushan

Quoted 14 lines · Bharat Bhushan

This patch add support for Marvell GTI watchdog. Global timer unit (GTI) support hardware watchdog timer. Software programs watchdog timer to generate interrupt on first timeout, second timeout is configured to be ignored and system reboots on third timeout.
Signed-off-by Bharat Bhushan <[email protected]>
drivers/watchdog/marvell_gti_wdt.c new file +4 -0
+
+static void gti_clk_disable_unprepare(void *data)
+{
+	clk_disable_unprepare(data);

drivers/watchdog/marvell_gti_wdt.c line 201

Quoted 10 lines

drivers/watchdog/marvell_gti_wdt.c new file +10 -0

+}
+
+static int gti_wdt_get_cntfrq(struct platform_device *pdev,
+			      struct gti_wdt_priv *priv)
+{
+	int err;
+
+	priv->sclk = devm_clk_get(&pdev->dev, NULL);
+	if (IS_ERR(priv->sclk))
+		return PTR_ERR(priv->sclk);

NAK. Undocumented property. Test your DTS. There is no way it was working. Either you did not test DTS against bindings or you did not test driver and DTS together.

Better actually to upstream your DTS...

Best regards,
Krzysztof

Bharat Bhushan

author

2023-05-04 09:02 UTC #5 lore raw

In reply to Krzysztof Kozlowski

Quoted 22 lines

-----Original Message-----
From: Krzysztof Kozlowski <[email protected]>
Sent: Thursday, May 4, 2023 12:25 PM
To: Bharat Bhushan <[email protected]>; [email protected]; [email protected]; [email protected]; [email protected]; [email protected]; [email protected]
Subject: [EXT] Re: [PATCH 1/2 v5] dt-bindings: watchdog: marvell GTI system watchdog driver
External Email
----------------------------------------------------------------------
Quoted 6 lines · Bharat Bhushan
Add binding documentation for the Marvell GTI system watchdog driver.
Signed-off-by Bharat Bhushan <[email protected]>
---
v5:
- Added wdt-timer-index property

Quoted 5 lines

I did not ask for it...
Quoted 1 line
- Get clock frequency from clocks/clock-name device tree property
Where? It's not possible in current code. I don't think you tested this at all.

My bad, Missed clock related binding changes while doing last minute rework. Will send updated patch adding the dt-binding properties.

It is testing exactly with below node:

                watchdog@802000040000 {
                        compatible = "marvell,gti-wdt";
                        reg = <0x8020 0x40000 0x0 0x20000>;
                        interrupts = <0 62 1>;
                        wdt-timer-index = <63>;
                        clocks = <&sclk>;
                        clock-names = "ref_clk";

Quoted 51 lines

Quoted 15 lines
 .../bindings/watchdog/marvell,gti-wdt.yaml    | 54 +++++++++++++++++++
 1 file changed, 54 insertions(+)
 create mode 100644
Documentation/devicetree/bindings/watchdog/marvell,gti-wdt.yaml
diff --git a/Documentation/devicetree/bindings/watchdog/marvell,gti-wdt.yaml b/Documentation/devicetree/bindings/watchdog/marvell,gti-wdt.yaml new file mode 100644
index 000000000000..e3315653f961
Documentation/devicetree/bindings/watchdog/marvell,gti-wdt.yaml line 5
Quoted 5 lines
Documentation/devicetree/bindings/watchdog/marvell,gti-wdt.yaml new file +5 -0
@@ -0,0 +1,54 @@
+# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause) %YAML 1.2
+---
+$id:
+https://urldefense.proofpoint.com/v2/url?u=http-3A__devicetree.org_sc
+hemas_watchdog_marvell-2Cgti-2Dwdt.yaml-
23&d=DwICaQ&c=nKjWec2b6R0mOyP
Quoted 1 line
+az7xtfQ&r=PAAlWswPe7d8gHlGbCLmy2YezyK7O3Hv_t2heGnouBw&m=ts41IUd c422tK
Quoted 1 line
+gF0cnYI7jTqJ1dvTm-FNq1pILCyrOuwqKu2UVnwWEVy-
aZAMsme&s=fVo903PvFGVqR_P
Documentation/devicetree/bindings/watchdog/marvell,gti-wdt.yaml
Quoted 4 lines
Documentation/devicetree/bindings/watchdog/marvell,gti-wdt.yaml new file +4 -0
+G6r91BBtzOTLz4Mixh1Tqu1GAp6E&e=
+$schema:
+https://urldefense.proofpoint.com/v2/url?u=http-3A__devicetree.org_me
+ta-2Dschemas_core.yaml-
23&d=DwICaQ&c=nKjWec2b6R0mOyPaz7xtfQ&r=PAAlWsw
Quoted 1 line
+Pe7d8gHlGbCLmy2YezyK7O3Hv_t2heGnouBw&m=ts41IUdc422tKgF0cnYI7jTqJ1 dvTm
Quoted 1 line
+-FNq1pILCyrOuwqKu2UVnwWEVy-
aZAMsme&s=ebh6bxE3wbSmwrOnHmUHMM_L77f6bY6W
Quoted 15 lines
Documentation/devicetree/bindings/watchdog/marvell,gti-wdt.yaml new file +15 -0
+Ifye_sXDNzI&e=
+
+title: Marvell Global Timer (GTI) system watchdog
+
+allOf:
+  - $ref: watchdog.yaml#
+
+maintainers:
+  - Bharat Bhushan <[email protected]>
+
+properties:
+  compatible:
+    const: marvell,gti-wdt
+
+  reg:

Quoted 8 lines

Documentation/devicetree/bindings/watchdog/marvell,gti-wdt.yaml line 25
Quoted 6 lines
Documentation/devicetree/bindings/watchdog/marvell,gti-wdt.yaml new file +6 -0
+    maxItems: 1
+
+  interrupts:
+    maxItems: 1
+
+  wdt-timer-index:
missing vendor prefix

ack

Quoted 2 lines

missing type

Will add

Quoted 4 lines

Quoted 2 lines
+ maxItems: 1
???

Removed

Quoted 9 lines

Documentation/devicetree/bindings/watchdog/marvell,gti-wdt.yaml line 29
Quoted 4 lines
Documentation/devicetree/bindings/watchdog/marvell,gti-wdt.yaml new file +3 -0
+    description:
+      This contains the timer number out of total 64 timers supported
+      by GTI hardware block.
Why do you need it? What does it represent?
We do not keep indices of devices other than something in reg, so please justify why exception must be made here.

Different platform have different number of GTI timers, for example some platform have total 64 timer and other have 32 timers. So which GTI timer will be used for watchdog will depend on platform. So added this property to enable this driver on platforms.

Quoted 15 lines

Documentation/devicetree/bindings/watchdog/marvell,gti-wdt.yaml new file +14 -0

+
+required:
+  - compatible
+  - reg
+  - interrupts
+  - wdt-timer-index
+
+unevaluatedProperties: false
+
+examples:
+  - |
+    #include <dt-bindings/interrupt-controller/irq.h>
+    soc {
+        #address-cells = <2>;

Quoted 8 lines

Documentation/devicetree/bindings/watchdog/marvell,gti-wdt.yaml line 49
Quoted 6 lines
Documentation/devicetree/bindings/watchdog/marvell,gti-wdt.yaml new file +6 -0
+        #size-cells = <2>;
+
+        watchdog@802000040000 {
+            compatible = "marvell,gti-wdt";
+            reg = <0x00008020 0x00040000 0x00000000 0x00020000>;
+            interrupts = <0 38 IRQ_TYPE_EDGE_RISING>;
Use defines for flags.

Okay.

Thanks
-Bharat

Quoted 6 lines

Quoted 2 lines
+ wdt-timer-index = <63>;
Best regards,
Krzysztof

Krzysztof Kozlowski

2023-05-04 11:07 UTC #6 lore raw

Quoted 76 lines · Bharat Bhushan

Quoted 22 lines
-----Original Message-----
From: Krzysztof Kozlowski <[email protected]>
Sent: Thursday, May 4, 2023 12:25 PM
To: Bharat Bhushan <[email protected]>; [email protected]; [email protected]; [email protected]; [email protected]; [email protected]; [email protected]
Subject: [EXT] Re: [PATCH 1/2 v5] dt-bindings: watchdog: marvell GTI system watchdog driver
External Email
----------------------------------------------------------------------
Quoted 6 lines · Bharat Bhushan
Add binding documentation for the Marvell GTI system watchdog driver.
Signed-off-by Bharat Bhushan <[email protected]>
---
v5:
- Added wdt-timer-index property
Quoted 5 lines
I did not ask for it...
Quoted 1 line
- Get clock frequency from clocks/clock-name device tree property
Where? It's not possible in current code. I don't think you tested this at all.
My bad, Missed clock related binding changes while doing last minute rework. Will send updated patch adding the dt-binding properties.
It is testing exactly with below node:
                watchdog@802000040000 {
                        compatible = "marvell,gti-wdt";
                        reg = <0x8020 0x40000 0x0 0x20000>;
                        interrupts = <0 62 1>;
                        wdt-timer-index = <63>;
                        clocks = <&sclk>;
                        clock-names = "ref_clk";
Quoted 36 lines
Quoted 15 lines
 .../bindings/watchdog/marvell,gti-wdt.yaml    | 54 +++++++++++++++++++
 1 file changed, 54 insertions(+)
 create mode 100644
Documentation/devicetree/bindings/watchdog/marvell,gti-wdt.yaml
diff --git a/Documentation/devicetree/bindings/watchdog/marvell,gti-wdt.yaml b/Documentation/devicetree/bindings/watchdog/marvell,gti-wdt.yaml new file mode 100644
index 000000000000..e3315653f961
Documentation/devicetree/bindings/watchdog/marvell,gti-wdt.yaml line 5
Quoted 5 lines
Documentation/devicetree/bindings/watchdog/marvell,gti-wdt.yaml new file +5 -0
@@ -0,0 +1,54 @@
+# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause) %YAML 1.2
+---
+$id:
+https://urldefense.proofpoint.com/v2/url?u=http-3A__devicetree.org_sc
+hemas_watchdog_marvell-2Cgti-2Dwdt.yaml-
23&d=DwICaQ&c=nKjWec2b6R0mOyP
Quoted 1 line
+az7xtfQ&r=PAAlWswPe7d8gHlGbCLmy2YezyK7O3Hv_t2heGnouBw&m=ts41IUd c422tK
Quoted 1 line
+gF0cnYI7jTqJ1dvTm-FNq1pILCyrOuwqKu2UVnwWEVy-
aZAMsme&s=fVo903PvFGVqR_P
Documentation/devicetree/bindings/watchdog/marvell,gti-wdt.yaml
Quoted 4 lines
Documentation/devicetree/bindings/watchdog/marvell,gti-wdt.yaml new file +4 -0
+G6r91BBtzOTLz4Mixh1Tqu1GAp6E&e=
+$schema:
+https://urldefense.proofpoint.com/v2/url?u=http-3A__devicetree.org_me
+ta-2Dschemas_core.yaml-
23&d=DwICaQ&c=nKjWec2b6R0mOyPaz7xtfQ&r=PAAlWsw
Quoted 1 line
+Pe7d8gHlGbCLmy2YezyK7O3Hv_t2heGnouBw&m=ts41IUdc422tKgF0cnYI7jTqJ1 dvTm
Quoted 1 line
+-FNq1pILCyrOuwqKu2UVnwWEVy-
aZAMsme&s=ebh6bxE3wbSmwrOnHmUHMM_L77f6bY6W

Quoted 13 lines

Quoted 5 lines
Quoted 5 lines
Documentation/devicetree/bindings/watchdog/marvell,gti-wdt.yaml new file +5 -0
+Ifye_sXDNzI&e=
+
+title: Marvell Global Timer (GTI) system watchdog
+
+allOf:
Quoted 8 lines
Documentation/devicetree/bindings/watchdog/marvell,gti-wdt.yaml line 17
Quoted 8 lines
Documentation/devicetree/bindings/watchdog/marvell,gti-wdt.yaml new file +8 -0
+  - $ref: watchdog.yaml#
+
+maintainers:
+  - Bharat Bhushan <[email protected]>
+
+properties:
+  compatible:
+    const: marvell,gti-wdt

So I guess we all thought "gti" means some soc. Now it is clear - you miss specific compatibles. Generic blocks alone or wildcards are not allowed.

And we have it clearly documented:

https://elixir.bootlin.com/linux/v6.1-rc1/source/Documentation/devicetree/bindings/writing-bindings.rst#L42

Quoted 30 lines

Quoted 10 lines
Documentation/devicetree/bindings/watchdog/marvell,gti-wdt.yaml line 25
Quoted 8 lines
Documentation/devicetree/bindings/watchdog/marvell,gti-wdt.yaml new file +8 -0
+
+  reg:
+    maxItems: 1
+
+  interrupts:
+    maxItems: 1
+
+  wdt-timer-index:
missing vendor prefix
ack
Quoted 2 lines
missing type
Will add
Quoted 4 lines
Quoted 2 lines
+ maxItems: 1
???
Removed
Quoted 5 lines
Quoted 4 lines
Documentation/devicetree/bindings/watchdog/marvell,gti-wdt.yaml new file +3 -0
+    description:
+      This contains the timer number out of total 64 timers supported
+      by GTI hardware block.

Quoted 7 lines

Quoted 4 lines
Why do you need it? What does it represent?
We do not keep indices of devices other than something in reg, so please justify why exception must be made here.
Different platform have different number of GTI timers, for example some platform have total 64 timer and other have 32 timers. So which GTI timer will be used for watchdog will depend on platform. So added this property to enable this driver on platforms.

This should be deducted from compatible.

Best regards,
Krzysztof

Bharat Bhushan

author

2023-05-04 17:11 UTC #7 lore raw

Quoted 113 lines

-----Original Message-----
From: Krzysztof Kozlowski <[email protected]>
Sent: Thursday, May 4, 2023 4:38 PM
To: Bharat Bhushan <[email protected]>; [email protected]; [email protected]; [email protected]; [email protected]; [email protected]; [email protected]
Subject: Re: [EXT] Re: [PATCH 1/2 v5] dt-bindings: watchdog: marvell GTI system watchdog driver
Quoted 32 lines · Bharat Bhushan
Quoted 23 lines
-----Original Message-----
From: Krzysztof Kozlowski <[email protected]>
Sent: Thursday, May 4, 2023 12:25 PM
To: Bharat Bhushan <[email protected]>; [email protected]; [email protected]; [email protected]; [email protected]; [email protected]; [email protected]
Subject: [EXT] Re: [PATCH 1/2 v5] dt-bindings: watchdog: marvell GTI system watchdog driver
External Email
---------------------------------------------------------------------
Quoted 6 lines · Bharat Bhushan
Add binding documentation for the Marvell GTI system watchdog driver.
Signed-off-by Bharat Bhushan <[email protected]>
---
v5:
- Added wdt-timer-index property
Quoted 5 lines
I did not ask for it...
Quoted 1 line
- Get clock frequency from clocks/clock-name device tree property
Where? It's not possible in current code. I don't think you tested this at all.
My bad, Missed clock related binding changes while doing last minute rework. Will send updated patch adding the dt-binding properties.
Quoted 34 lines
It is testing exactly with below node:
                watchdog@802000040000 {
                        compatible = "marvell,gti-wdt";
                        reg = <0x8020 0x40000 0x0 0x20000>;
                        interrupts = <0 62 1>;
                        wdt-timer-index = <63>;
                        clocks = <&sclk>;
                        clock-names = "ref_clk";
Quoted 25 lines
Quoted 16 lines
 .../bindings/watchdog/marvell,gti-wdt.yaml    | 54 +++++++++++++++++++
 1 file changed, 54 insertions(+)
 create mode 100644
Documentation/devicetree/bindings/watchdog/marvell,gti-wdt.yaml
diff --git a/Documentation/devicetree/bindings/watchdog/marvell,gti-wdt.yaml b/Documentation/devicetree/bindings/watchdog/marvell,gti-wdt.yaml new file mode 100644
index 000000000000..e3315653f961
Documentation/devicetree/bindings/watchdog/marvell,gti-wdt.yam new file +1 -0
+++ l
Documentation/devicetree/bindings/watchdog/marvell,gti-wdt.yam line 6
Quoted 6 lines
Documentation/devicetree/bindings/watchdog/marvell,gti-wdt.yam new file +6 -0
@@ -0,0 +1,54 @@
+# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause) %YAML 1.2
+---
+$id:
+https://urldefense.proofpoint.com/v2/url?u=http-3A__devicetree.org_
+sc
+hemas_watchdog_marvell-2Cgti-2Dwdt.yaml-
23&d=DwICaQ&c=nKjWec2b6R0mOyP
Quoted 1 line
+az7xtfQ&r=PAAlWswPe7d8gHlGbCLmy2YezyK7O3Hv_t2heGnouBw&m=ts41IUd
Quoted 11 lines
Quoted 11 lines
c422tK
Quoted 1 line
+gF0cnYI7jTqJ1dvTm-FNq1pILCyrOuwqKu2UVnwWEVy-
aZAMsme&s=fVo903PvFGVqR_P
Documentation/devicetree/bindings/watchdog/marvell,gti-wdt.yam
Quoted 5 lines
Documentation/devicetree/bindings/watchdog/marvell,gti-wdt.yam new file +5 -0
+G6r91BBtzOTLz4Mixh1Tqu1GAp6E&e=
+$schema:
+https://urldefense.proofpoint.com/v2/url?u=http-3A__devicetree.org_
+me
+ta-2Dschemas_core.yaml-
23&d=DwICaQ&c=nKjWec2b6R0mOyPaz7xtfQ&r=PAAlWsw
Quoted 1 line
+Pe7d8gHlGbCLmy2YezyK7O3Hv_t2heGnouBw&m=ts41IUdc422tKgF0cnYI7jTqJ1
Quoted 16 lines
Quoted 16 lines
dvTm
Quoted 1 line
+-FNq1pILCyrOuwqKu2UVnwWEVy-
aZAMsme&s=ebh6bxE3wbSmwrOnHmUHMM_L77f6bY6W
Documentation/devicetree/bindings/watchdog/marvell,gti-wdt.yam
Quoted 13 lines
Documentation/devicetree/bindings/watchdog/marvell,gti-wdt.yam new file +13 -0
+Ifye_sXDNzI&e=
+
+title: Marvell Global Timer (GTI) system watchdog
+
+allOf:
+  - $ref: watchdog.yaml#
+
+maintainers:
+  - Bharat Bhushan <[email protected]>
+
+properties:
+  compatible:
+    const: marvell,gti-wdt
So I guess we all thought "gti" means some soc. Now it is clear - you miss specific compatibles. Generic blocks alone or wildcards are not allowed.
And we have it clearly documented:

Quoted 7 lines

https://urldefense.proofpoint.com/v2/url?u=https-
3A__elixir.bootlin.com_linux_v6.1- 2Drc1_source_Documentation_devicetree_bindings_writing-2Dbindings.rst- 23L42&d=DwICaQ&c=nKjWec2b6R0mOyPaz7xtfQ&r=PAAlWswPe7d8gHlGbCLmy 2YezyK7O3Hv_t2heGnouBw&m=uy18AXnbGKMIcyPkTnWrPZoVBj8yzyjlGeNemR MFgMVqkT6-4JVU5hWsVbcKPzTJ&s=XkqN7nbFOrtRnOJVqrgEDA9zinZxML4- qSYQPElzVeg&e=

So Say currently Marvell have GTI watchdog timer in following series of SoCs
- octeon
- octeontx2
- cn10k

Compatible for octeon series is "marvell,octeon-wdt" Compatible for octeontx2 series is "marvell,octeontx2-wdt" Compatible for cn10k series is "marvell,cn10k-wdt"

So effectively we will have 3 compatibles, Is that correct?

Quoted 36 lines

Quoted 36 lines
Quoted 11 lines
Documentation/devicetree/bindings/watchdog/marvell,gti-wdt.yam
Quoted 9 lines
Documentation/devicetree/bindings/watchdog/marvell,gti-wdt.yam new file +8 -0
+
+  reg:
+    maxItems: 1
+
+  interrupts:
+    maxItems: 1
+
+  wdt-timer-index:
missing vendor prefix
ack
Quoted 2 lines
missing type
Will add
Quoted 4 lines
Quoted 2 lines
+ maxItems: 1
???
Removed
Quoted 9 lines
Documentation/devicetree/bindings/watchdog/marvell,gti-wdt.yam
Quoted 4 lines
Documentation/devicetree/bindings/watchdog/marvell,gti-wdt.yam new file +3 -0
+    description:
+      This contains the timer number out of total 64 timers supported
+      by GTI hardware block.
Why do you need it? What does it represent?
We do not keep indices of devices other than something in reg, so please justify why exception must be made here.

Quoted 6 lines

Quoted 1 line
Different platform have different number of GTI timers, for example some
platform have total 64 timer and other have 32 timers.
Quoted 1 line
So which GTI timer will be used for watchdog will depend on platform. So
added this property to enable this driver on platforms.
This should be deducted from compatible.

If I understood correctly, we should add different compatible for each soc and use same to get the information we tried to get using "wdt-timer-index" property, is that correct?

But each series have many socs (10s) and GTI hardware is same except number of timers they supports, so should we add that many compatibles or add a property like this?

Thanks
-Bharat

Quoted 3 lines

Best regards,
Krzysztof

Krzysztof Kozlowski

2023-05-05 06:38 UTC #8 lore raw

On 04/05/2023 19:10, Bharat Bhushan wrote:>>>>> +maintainers:

Quoted 24 lines

Quoted 11 lines
Quoted 5 lines
Quoted 5 lines
Documentation/devicetree/bindings/watchdog/marvell,gti-wdt.yaml line 17
Quoted 5 lines
Documentation/devicetree/bindings/watchdog/marvell,gti-wdt.yaml new file +5 -0
+  - Bharat Bhushan <[email protected]>
+
+properties:
+  compatible:
+    const: marvell,gti-wdt
So I guess we all thought "gti" means some soc. Now it is clear - you miss specific compatibles. Generic blocks alone or wildcards are not allowed.
And we have it clearly documented:
Quoted 7 lines
https://urldefense.proofpoint.com/v2/url?u=https-
3A__elixir.bootlin.com_linux_v6.1- 2Drc1_source_Documentation_devicetree_bindings_writing-2Dbindings.rst- 23L42&d=DwICaQ&c=nKjWec2b6R0mOyPaz7xtfQ&r=PAAlWswPe7d8gHlGbCLmy 2YezyK7O3Hv_t2heGnouBw&m=uy18AXnbGKMIcyPkTnWrPZoVBj8yzyjlGeNemR MFgMVqkT6-4JVU5hWsVbcKPzTJ&s=XkqN7nbFOrtRnOJVqrgEDA9zinZxML4- qSYQPElzVeg&e=
So Say currently Marvell have GTI watchdog timer in following series of SoCs
- octeon
- octeontx2
- cn10k

Quoted 5 lines

Compatible for octeon series is "marvell,octeon-wdt" Compatible for octeontx2 series is "marvell,octeontx2-wdt" Compatible for cn10k series is "marvell,cn10k-wdt"
So effectively we will have 3 compatibles, Is that correct?

I don't know your SoCs. I assume you should know.

Quoted 39 lines

Quoted 39 lines
Quoted 32 lines
Quoted 12 lines
Documentation/devicetree/bindings/watchdog/marvell,gti-wdt.yaml line 25
Quoted 10 lines
Documentation/devicetree/bindings/watchdog/marvell,gti-wdt.yaml new file +8 -0
+
+  reg:
+    maxItems: 1
+
+  interrupts:
+    maxItems: 1
+
+  wdt-timer-index:
missing vendor prefix
ack
Quoted 2 lines
missing type
Will add
Quoted 4 lines
Quoted 2 lines
+ maxItems: 1
???
Removed
Quoted 5 lines
Quoted 4 lines
Documentation/devicetree/bindings/watchdog/marvell,gti-wdt.yaml new file +3 -0
+    description:
+      This contains the timer number out of total 64 timers supported
+      by GTI hardware block.
Quoted 6 lines
Quoted 4 lines
Why do you need it? What does it represent?
We do not keep indices of devices other than something in reg, so please justify why exception must be made here.
Different platform have different number of GTI timers, for example some
platform have total 64 timer and other have 32 timers.

Quoted 8 lines

Quoted 4 lines
Quoted 1 line
So which GTI timer will be used for watchdog will depend on platform. So
added this property to enable this driver on platforms.
This should be deducted from compatible.
If I understood correctly, we should add different compatible for each soc and use same to get the information we tried to get using "wdt-timer-index" property, is that correct?
But each series have many socs (10s) and GTI hardware is same except number of timers they supports, so should we add that many compatibles or add a property like this?

Same story every time... and was discussed many, many times on the lists.

https://elixir.bootlin.com/linux/v6.1-rc1/source/Documentation/devicetree/bindings/writing-bindings.rst#L42

You need anyway SoC specific compatibles. Once you add proper compatibles, you will see that property is not needed.

Best regards,
Krzysztof

Bharat Bhushan

author

2023-05-05 07:18 UTC #9 lore raw

Quoted 111 lines

-----Original Message-----
From: Krzysztof Kozlowski <[email protected]>
Sent: Friday, May 5, 2023 12:08 PM
To: Bharat Bhushan <[email protected]>; [email protected]; [email protected]; [email protected]; [email protected]; [email protected]; [email protected]
Subject: Re: [EXT] Re: [PATCH 1/2 v5] dt-bindings: watchdog: marvell GTI system watchdog driver
On 04/05/2023 19:10, Bharat Bhushan wrote:>>>>> +maintainers:
Quoted 9 lines
Quoted 9 lines
Quoted 5 lines
Quoted 5 lines
Documentation/devicetree/bindings/watchdog/marvell,gti-wdt.yaml line 17
Quoted 5 lines
Documentation/devicetree/bindings/watchdog/marvell,gti-wdt.yaml new file +5 -0
+  - Bharat Bhushan <[email protected]>
+
+properties:
+  compatible:
+    const: marvell,gti-wdt
So I guess we all thought "gti" means some soc. Now it is clear - you miss specific compatibles. Generic blocks alone or wildcards are not allowed.
Quoted 6 lines
Quoted 6 lines
And we have it clearly documented:
https://urldefense.proofpoint.com/v2/url?u=https-
3A__elixir.bootlin.com_linux_v6.1- 2Drc1_source_Documentation_devicetree_bindings_writing-2Dbindings.rst -
23L42&d=DwICaQ&c=nKjWec2b6R0mOyPaz7xtfQ&r=PAAlWswPe7d8gHlGbCLmy
Quoted 1 line
Quoted 1 line
2YezyK7O3Hv_t2heGnouBw&m=uy18AXnbGKMIcyPkTnWrPZoVBj8yzyjlGeNemR
Quoted 9 lines
Quoted 2 lines
MFgMVqkT6-4JVU5hWsVbcKPzTJ&s=XkqN7nbFOrtRnOJVqrgEDA9zinZxML4- qSYQPElzVeg&e=
So Say currently Marvell have GTI watchdog timer in following series of SoCs
- octeon
- octeontx2
- cn10k
Quoted 5 lines
Compatible for octeon series is "marvell,octeon-wdt" Compatible for octeontx2 series is "marvell,octeontx2-wdt" Compatible for cn10k series is "marvell,cn10k-wdt"
So effectively we will have 3 compatibles, Is that correct?
I don't know your SoCs. I assume you should know.
Quoted 39 lines
Quoted 39 lines
Quoted 39 lines
Quoted 12 lines
Documentation/devicetree/bindings/watchdog/marvell,gti-wdt.yaml line 25
Quoted 10 lines
Documentation/devicetree/bindings/watchdog/marvell,gti-wdt.yaml new file +8 -0
+
+  reg:
+    maxItems: 1
+
+  interrupts:
+    maxItems: 1
+
+  wdt-timer-index:
missing vendor prefix
ack
Quoted 2 lines
missing type
Will add
Quoted 4 lines
Quoted 2 lines
+ maxItems: 1
???
Removed
Quoted 9 lines
Documentation/devicetree/bindings/watchdog/marvell,gti-wdt.yaml line 29
Quoted 4 lines
Documentation/devicetree/bindings/watchdog/marvell,gti-wdt.yaml new file +3 -0
+    description:
+      This contains the timer number out of total 64 timers supported
+      by GTI hardware block.
Why do you need it? What does it represent?
We do not keep indices of devices other than something in reg, so please justify why exception must be made here.
Different platform have different number of GTI timers, for example some
Quoted 8 lines
Quoted 6 lines
platform have total 64 timer and other have 32 timers.
Quoted 2 lines
So which GTI timer will be used for watchdog will depend on platform. So
added this property to enable this driver on platforms.
This should be deducted from compatible.
If I understood correctly, we should add different compatible for each soc and
use same to get the information we tried to get using "wdt-timer-index" property, is that correct?
Quoted 2 lines
But each series have many socs (10s) and GTI hardware is same except number
of timers they supports, so should we add that many compatibles or add a property like this?
Same story every time... and was discussed many, many times on the lists.
https://urldefense.proofpoint.com/v2/url?u=https-
3A__elixir.bootlin.com_linux_v6.1- 2Drc1_source_Documentation_devicetree_bindings_writing-2Dbindings.rst- 23L42&d=DwICaQ&c=nKjWec2b6R0mOyPaz7xtfQ&r=PAAlWswPe7d8gHlGbCLmy 2YezyK7O3Hv_t2heGnouBw&m=3aeejmHQ5C8TyLM5odlaq6KnLYHt4PhpJp70FQa qbNf7w15KFHA3fmeDR2V-en4m&s=FKeW5tpOG- CJoV_JKuqTa0k_tRrYWAQZG1UfqlW3c74&e=

Quoted 2 lines

You need anyway SoC specific compatibles. Once you add proper compatibles, you will see that property is not needed.

Looks odd to add N number of compatible for N socs belong to one class of soc, but anyways will do.

Thanks
-Bharat

Quoted 4 lines

Best regards,
Krzysztof

Bharat Bhushan

author

2023-05-05 07:55 UTC #11 lore raw

In reply to Krzysztof Kozlowski

Quoted 111 lines

-----Original Message-----
From: Krzysztof Kozlowski <[email protected]>
Sent: Friday, May 5, 2023 12:08 PM
To: Bharat Bhushan <[email protected]>; [email protected]; [email protected]; [email protected]; [email protected]; [email protected]; [email protected]
Subject: Re: [EXT] Re: [PATCH 1/2 v5] dt-bindings: watchdog: marvell GTI system watchdog driver
On 04/05/2023 19:10, Bharat Bhushan wrote:>>>>> +maintainers:
Quoted 9 lines
Quoted 9 lines
Quoted 5 lines
Quoted 5 lines
Documentation/devicetree/bindings/watchdog/marvell,gti-wdt.yaml line 17
Quoted 5 lines
Documentation/devicetree/bindings/watchdog/marvell,gti-wdt.yaml new file +5 -0
+  - Bharat Bhushan <[email protected]>
+
+properties:
+  compatible:
+    const: marvell,gti-wdt
So I guess we all thought "gti" means some soc. Now it is clear - you miss specific compatibles. Generic blocks alone or wildcards are not allowed.
Quoted 6 lines
Quoted 6 lines
And we have it clearly documented:
https://urldefense.proofpoint.com/v2/url?u=https-
3A__elixir.bootlin.com_linux_v6.1- 2Drc1_source_Documentation_devicetree_bindings_writing-2Dbindings.rst -
23L42&d=DwICaQ&c=nKjWec2b6R0mOyPaz7xtfQ&r=PAAlWswPe7d8gHlGbCLmy
Quoted 1 line
Quoted 1 line
2YezyK7O3Hv_t2heGnouBw&m=uy18AXnbGKMIcyPkTnWrPZoVBj8yzyjlGeNemR
Quoted 9 lines
Quoted 2 lines
MFgMVqkT6-4JVU5hWsVbcKPzTJ&s=XkqN7nbFOrtRnOJVqrgEDA9zinZxML4- qSYQPElzVeg&e=
So Say currently Marvell have GTI watchdog timer in following series of SoCs
- octeon
- octeontx2
- cn10k
Quoted 5 lines
Compatible for octeon series is "marvell,octeon-wdt" Compatible for octeontx2 series is "marvell,octeontx2-wdt" Compatible for cn10k series is "marvell,cn10k-wdt"
So effectively we will have 3 compatibles, Is that correct?
I don't know your SoCs. I assume you should know.
Quoted 39 lines
Quoted 39 lines
Quoted 39 lines
Quoted 12 lines
Documentation/devicetree/bindings/watchdog/marvell,gti-wdt.yaml line 25
Quoted 10 lines
Documentation/devicetree/bindings/watchdog/marvell,gti-wdt.yaml new file +8 -0
+
+  reg:
+    maxItems: 1
+
+  interrupts:
+    maxItems: 1
+
+  wdt-timer-index:
missing vendor prefix
ack
Quoted 2 lines
missing type
Will add
Quoted 4 lines
Quoted 2 lines
+ maxItems: 1
???
Removed
Quoted 9 lines
Documentation/devicetree/bindings/watchdog/marvell,gti-wdt.yaml line 29
Quoted 4 lines
Documentation/devicetree/bindings/watchdog/marvell,gti-wdt.yaml new file +3 -0
+    description:
+      This contains the timer number out of total 64 timers supported
+      by GTI hardware block.
Why do you need it? What does it represent?
We do not keep indices of devices other than something in reg, so please justify why exception must be made here.
Different platform have different number of GTI timers, for example some
Quoted 8 lines
Quoted 6 lines
platform have total 64 timer and other have 32 timers.
Quoted 2 lines
So which GTI timer will be used for watchdog will depend on platform. So
added this property to enable this driver on platforms.
This should be deducted from compatible.
If I understood correctly, we should add different compatible for each soc and
use same to get the information we tried to get using "wdt-timer-index" property, is that correct?
Quoted 2 lines
But each series have many socs (10s) and GTI hardware is same except number
of timers they supports, so should we add that many compatibles or add a property like this?
Same story every time... and was discussed many, many times on the lists.
https://urldefense.proofpoint.com/v2/url?u=https-
3A__elixir.bootlin.com_linux_v6.1- 2Drc1_source_Documentation_devicetree_bindings_writing-2Dbindings.rst- 23L42&d=DwICaQ&c=nKjWec2b6R0mOyPaz7xtfQ&r=PAAlWswPe7d8gHlGbCLmy 2YezyK7O3Hv_t2heGnouBw&m=3aeejmHQ5C8TyLM5odlaq6KnLYHt4PhpJp70FQa qbNf7w15KFHA3fmeDR2V-en4m&s=FKeW5tpOG- CJoV_JKuqTa0k_tRrYWAQZG1UfqlW3c74&e=

Quoted 2 lines

You need anyway SoC specific compatibles. Once you add proper compatibles, you will see that property is not needed.

Also on a given soc, firmware can configure one of 64 timer to be used as system watchdog time then compatible will not work.

Thanks
-Bharat

Quoted 4 lines

Best regards,
Krzysztof

Krzysztof Kozlowski

2023-05-05 10:31 UTC #10 lore raw

In reply to Bharat Bhushan

Quoted 10 lines · Bharat Bhushan

Quoted 10 lines
Same story every time... and was discussed many, many times on the lists.
https://urldefense.proofpoint.com/v2/url?u=https-
3A__elixir.bootlin.com_linux_v6.1- 2Drc1_source_Documentation_devicetree_bindings_writing-2Dbindings.rst- 23L42&d=DwICaQ&c=nKjWec2b6R0mOyPaz7xtfQ&r=PAAlWswPe7d8gHlGbCLmy 2YezyK7O3Hv_t2heGnouBw&m=3aeejmHQ5C8TyLM5odlaq6KnLYHt4PhpJp70FQa qbNf7w15KFHA3fmeDR2V-en4m&s=FKeW5tpOG- CJoV_JKuqTa0k_tRrYWAQZG1UfqlW3c74&e=

Quoted 4 lines

Quoted 2 lines
You need anyway SoC specific compatibles. Once you add proper compatibles, you will see that property is not needed.
Looks odd to add N number of compatible for N socs belong to one class of soc, but anyways will do.

Why this is odd? How does it differ from other SoCs?

Best regards,
Krzysztof

Krzysztof Kozlowski

2023-05-05 10:33 UTC #12 lore raw

In reply to Bharat Bhushan

Quoted 27 lines · Bharat Bhushan

Quoted 27 lines
Quoted 10 lines
Quoted 8 lines
Quoted 2 lines
Different platform have different number of GTI timers, for example some
platform have total 64 timer and other have 32 timers.
Quoted 2 lines
So which GTI timer will be used for watchdog will depend on platform. So
added this property to enable this driver on platforms.
This should be deducted from compatible.
If I understood correctly, we should add different compatible for each soc and
use same to get the information we tried to get using "wdt-timer-index" property, is that correct?
Quoted 2 lines
But each series have many socs (10s) and GTI hardware is same except number
of timers they supports, so should we add that many compatibles or add a property like this?
Same story every time... and was discussed many, many times on the lists.
https://urldefense.proofpoint.com/v2/url?u=https-
3A__elixir.bootlin.com_linux_v6.1- 2Drc1_source_Documentation_devicetree_bindings_writing-2Dbindings.rst- 23L42&d=DwICaQ&c=nKjWec2b6R0mOyPaz7xtfQ&r=PAAlWswPe7d8gHlGbCLmy 2YezyK7O3Hv_t2heGnouBw&m=3aeejmHQ5C8TyLM5odlaq6KnLYHt4PhpJp70FQa qbNf7w15KFHA3fmeDR2V-en4m&s=FKeW5tpOG- CJoV_JKuqTa0k_tRrYWAQZG1UfqlW3c74&e=

Quoted 4 lines

Quoted 2 lines
You need anyway SoC specific compatibles. Once you add proper compatibles, you will see that property is not needed.
Also on a given soc, firmware can configure one of 64 timer to be used as system watchdog time then compatible will not work.

Can't you query the firmware for that? Or can't you just choose first unused timer? DT is for non-discoverable properties.

Best regards,
Krzysztof

Bharat Bhushan

author

2023-05-05 10:41 UTC #13 lore raw

Quoted 43 lines

-----Original Message-----
From: Krzysztof Kozlowski <[email protected]>
Sent: Friday, May 5, 2023 4:04 PM
To: Bharat Bhushan <[email protected]>; [email protected]; [email protected]; [email protected]; [email protected]; [email protected]; [email protected]
Subject: Re: [EXT] Re: [PATCH 1/2 v5] dt-bindings: watchdog: marvell GTI system watchdog driver
Quoted 19 lines · Bharat Bhushan
Quoted 19 lines
Quoted 11 lines
Quoted 8 lines
Quoted 2 lines
Different platform have different number of GTI timers, for example some
platform have total 64 timer and other have 32 timers.
Quoted 2 lines
So which GTI timer will be used for watchdog will depend on platform. So
added this property to enable this driver on platforms.
This should be deducted from compatible.
If I understood correctly, we should add different compatible for each soc and
use same to get the information we tried to get using "wdt-timer-index" property, is that correct?
Quoted 3 lines
But each series have many socs (10s) and GTI hardware is same except number
of timers they supports, so should we add that many compatibles or add a property like this?
Quoted 6 lines
Quoted 6 lines
Same story every time... and was discussed many, many times on the lists.
https://urldefense.proofpoint.com/v2/url?u=https-
3A__elixir.bootlin.com_linux_v6.1- 2Drc1_source_Documentation_devicetree_bindings_writing-2Dbindings.rst -
23L42&d=DwICaQ&c=nKjWec2b6R0mOyPaz7xtfQ&r=PAAlWswPe7d8gHlGbCLmy
Quoted 1 line
Quoted 1 line
2YezyK7O3Hv_t2heGnouBw&m=3aeejmHQ5C8TyLM5odlaq6KnLYHt4PhpJp70FQa
Quoted 3 lines
Quoted 3 lines
qbNf7w15KFHA3fmeDR2V-en4m&s=FKeW5tpOG-
CJoV_JKuqTa0k_tRrYWAQZG1UfqlW3c74&e=

Quoted 8 lines

Quoted 4 lines
Quoted 2 lines
You need anyway SoC specific compatibles. Once you add proper compatibles, you will see that property is not needed.
Also on a given soc, firmware can configure one of 64 timer to be used as
system watchdog time then compatible will not work.
Can't you query the firmware for that? Or can't you just choose first unused timer? DT is for non-discoverable properties.

Query to firmware required arm SMC call, to me that does not look correct approach. Thought of using first one but that is already used and moving that is as same as this.

Hardcoding to 63 will make it work on some SoCs but not all.

Thanks
-Bharat

Quoted 3 lines

Best regards,
Krzysztof

Krzysztof Kozlowski

2023-05-05 10:57 UTC #14 lore raw

Quoted 5 lines · Bharat Bhushan

Quoted 5 lines
Quoted 5 lines
Quoted 3 lines
You need anyway SoC specific compatibles. Once you add proper compatibles, you will see that property is not needed.
Also on a given soc, firmware can configure one of 64 timer to be used as

Quoted 8 lines

Quoted 4 lines
system watchdog time then compatible will not work.
Can't you query the firmware for that? Or can't you just choose first unused timer? DT is for non-discoverable properties.
Query to firmware required arm SMC call, to me that does not look correct approach. Thought of using first one but that is already used and moving that is as same as this.
Hardcoding to 63 will make it work on some SoCs but not all.

But you know which one is started or is not. GTI_CWD_WDOG tells you this.

Best regards,
Krzysztof

Bharat Bhushan

author

2023-05-05 11:15 UTC #15 lore raw

Quoted 23 lines

-----Original Message-----
From: Krzysztof Kozlowski <[email protected]>
Sent: Friday, May 5, 2023 4:27 PM
To: Bharat Bhushan <[email protected]>; [email protected]; [email protected]; [email protected]; [email protected]; [email protected]; [email protected]
Subject: Re: [EXT] Re: [PATCH 1/2 v5] dt-bindings: watchdog: marvell GTI system watchdog driver
Quoted 11 lines · Bharat Bhushan
Quoted 10 lines
Quoted 6 lines
Quoted 3 lines
You need anyway SoC specific compatibles. Once you add proper compatibles, you will see that property is not needed.
Also on a given soc, firmware can configure one of 64 timer to be used as
system watchdog time then compatible will not work.
Can't you query the firmware for that? Or can't you just choose first unused timer? DT is for non-discoverable properties.

Quoted 7 lines

Quoted 1 line
Query to firmware required arm SMC call, to me that does not look correct
approach. Thought of using first one but that is already used and moving that is as same as this.
Quoted 2 lines
Hardcoding to 63 will make it work on some SoCs but not all.
But you know which one is started or is not. GTI_CWD_WDOG tells you this.

On a given SoC, Firmware can reserve and/or use one or more timer for some other use case (customer use) and configure one of the timer as watchdog timer. Linux have to use the configured timer only and cannot decide by its own.

Thanks
-Bharat

Quoted 3 lines

Best regards,
Krzysztof

Krzysztof Kozlowski

2023-05-05 11:57 UTC #16 lore raw

Quoted 24 lines · Bharat Bhushan

Quoted 24 lines
-----Original Message-----
From: Krzysztof Kozlowski <[email protected]>
Sent: Friday, May 5, 2023 4:27 PM
To: Bharat Bhushan <[email protected]>; [email protected]; [email protected]; [email protected]; [email protected]; [email protected]; [email protected]
Subject: Re: [EXT] Re: [PATCH 1/2 v5] dt-bindings: watchdog: marvell GTI system watchdog driver
Quoted 12 lines · Bharat Bhushan
Quoted 10 lines
Quoted 6 lines
Quoted 3 lines
You need anyway SoC specific compatibles. Once you add proper compatibles, you will see that property is not needed.
Also on a given soc, firmware can configure one of 64 timer to be used as
system watchdog time then compatible will not work.
Can't you query the firmware for that? Or can't you just choose first unused timer? DT is for non-discoverable properties.
Query to firmware required arm SMC call, to me that does not look correct

Quoted 8 lines

Quoted 6 lines
approach. Thought of using first one but that is already used and moving that is as same as this.
Quoted 2 lines
Hardcoding to 63 will make it work on some SoCs but not all.
But you know which one is started or is not. GTI_CWD_WDOG tells you this.
On a given SoC, Firmware can reserve and/or use one or more timer for some other use case (customer use) and configure one of the timer as watchdog timer. Linux have to use the configured timer only and cannot decide by its own.

Then the SoCs which have such firmware could use proposed property. Provide some rationale property description in your next version (adding necessary compatibles).

Best regards,
Krzysztof

Bharat Bhushan

author

2023-05-05 12:19 UTC #17 lore raw

Quoted 45 lines

-----Original Message-----
From: Krzysztof Kozlowski <[email protected]>
Sent: Friday, May 5, 2023 5:27 PM
To: Bharat Bhushan <[email protected]>; [email protected]; [email protected]; [email protected]; [email protected]; [email protected]; [email protected]
Subject: Re: [EXT] Re: [PATCH 1/2 v5] dt-bindings: watchdog: marvell GTI system watchdog driver
Quoted 33 lines · Bharat Bhushan
Quoted 32 lines
-----Original Message-----
From: Krzysztof Kozlowski <[email protected]>
Sent: Friday, May 5, 2023 4:27 PM
To: Bharat Bhushan <[email protected]>; [email protected]; [email protected]; [email protected]; [email protected]; [email protected]; [email protected]
Subject: Re: [EXT] Re: [PATCH 1/2 v5] dt-bindings: watchdog: marvell GTI system watchdog driver
Quoted 6 lines · Bharat Bhushan
Quoted 6 lines
Quoted 6 lines
Quoted 3 lines
You need anyway SoC specific compatibles. Once you add proper compatibles, you will see that property is not needed.
Also on a given soc, firmware can configure one of 64 timer to be used as
Quoted 7 lines
Quoted 4 lines
system watchdog time then compatible will not work.
Can't you query the firmware for that? Or can't you just choose first unused timer? DT is for non-discoverable properties.
Query to firmware required arm SMC call, to me that does not look correct
approach. Thought of using first one but that is already used and moving that is as same as this.
Quoted 2 lines
Hardcoding to 63 will make it work on some SoCs but not all.
But you know which one is started or is not. GTI_CWD_WDOG tells you this.

Quoted 7 lines

Quoted 1 line
On a given SoC, Firmware can reserve and/or use one or more timer for some
other use case (customer use) and configure one of the timer as watchdog timer. Linux have to use the configured timer only and cannot decide by its own.
Then the SoCs which have such firmware could use proposed property. Provide some rationale property description in your next version (adding necessary compatibles).

Will add compatible as below:

properties:
compatible:
enum:
- marvell,cn10k-wdt
- marvell,octeontx2-wdt

And define this as optional property
marvell,wdt-timer-index:
$ref: /schemas/types.yaml#/definitions/uint32
minimum: 0
maximum: 63
description:
An SoC have many timers (up to 64), firmware can reserve one or more timer for some other use case and configures one of the global timer as watchdog timer. Firmware will update this field with the timer number configured as watchdog timer.

Let me know if this looks good.

Thanks
-Bharat

Quoted 3 lines

Best regards,
Krzysztof

Krzysztof Kozlowski

2023-05-05 12:27 UTC #18 lore raw

Quoted 17 lines · Bharat Bhushan

Quoted 9 lines
Quoted 9 lines
Quoted 8 lines
Quoted 2 lines
Query to firmware required arm SMC call, to me that does not look correct
approach. Thought of using first one but that is already used and moving that is as same as this.
Quoted 2 lines
Hardcoding to 63 will make it work on some SoCs but not all.
But you know which one is started or is not. GTI_CWD_WDOG tells you this.
Quoted 7 lines
Quoted 1 line
On a given SoC, Firmware can reserve and/or use one or more timer for some
other use case (customer use) and configure one of the timer as watchdog timer. Linux have to use the configured timer only and cannot decide by its own.
Then the SoCs which have such firmware could use proposed property. Provide some rationale property description in your next version (adding necessary compatibles).

Quoted 7 lines

Will add compatible as below:
properties:
compatible:
enum:
- marvell,cn10k-wdt
- marvell,octeontx2-wdt

And the rest? You said you have 10 SoCs.

You made them not compatible with each other, so I assume on purpose and your driver will make use of it.

Quoted 11 lines

And define this as optional property
marvell,wdt-timer-index:
$ref: /schemas/types.yaml#/definitions/uint32
minimum: 0
maximum: 63
description:
An SoC have many timers (up to 64), firmware can reserve one or more timer for some other use case and configures one of the global timer as watchdog timer. Firmware will update this field with the timer number configured as watchdog timer.

Looks ok.

Best regards,
Krzysztof