> -----Original Message-----
> From: Krzysztof Kozlowski <krzysztof.kozlowski@xxxxxxxxxx>
> Sent: Thursday, May 4, 2023 12:25 PM
> To: Bharat Bhushan <bbhushan2@xxxxxxxxxxx>; wim@xxxxxxxxxxxxxxxxxx;
> linux@xxxxxxxxxxxx; robh+dt@xxxxxxxxxx; krzysztof.kozlowski+dt@xxxxxxxxxx;
> linux-watchdog@xxxxxxxxxxxxxxx; devicetree@xxxxxxxxxxxxxxx
> Subject: [EXT] Re: [PATCH 1/2 v5] dt-bindings: watchdog: marvell GTI system
> watchdog driver
>
> External Email
>
> ----------------------------------------------------------------------
> On 03/05/2023 14:10, Bharat Bhushan wrote:
> > Add binding documentation for the Marvell GTI system watchdog driver.
> >
> > Signed-off-by: Bharat Bhushan <bbhushan2@xxxxxxxxxxx>
> > ---
> > v5:
> > - Added wdt-timer-index property
>
> I did not ask for it...
>
> > - 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";
>
> >
> > .../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
> > --- /dev/null
> > +++ b/Documentation/devicetree/bindings/watchdog/marvell,gti-wdt.yaml
> > @@ -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
> >
> +az7xtfQ&r=PAAlWswPe7d8gHlGbCLmy2YezyK7O3Hv_t2heGnouBw&m=ts41IUd
> c422tK
> > +gF0cnYI7jTqJ1dvTm-FNq1pILCyrOuwqKu2UVnwWEVy-
> aZAMsme&s=fVo903PvFGVqR_P
> > +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
> >
> +Pe7d8gHlGbCLmy2YezyK7O3Hv_t2heGnouBw&m=ts41IUdc422tKgF0cnYI7jTqJ1
> dvTm
> > +-FNq1pILCyrOuwqKu2UVnwWEVy-
> aZAMsme&s=ebh6bxE3wbSmwrOnHmUHMM_L77f6bY6W
> > +Ifye_sXDNzI&e=
> > +
> > +title: Marvell Global Timer (GTI) system watchdog
> > +
> > +allOf:
> > + - $ref: watchdog.yaml#
> > +
> > +maintainers:
> > + - Bharat Bhushan <bbhushan2@xxxxxxxxxxx>
> > +
> > +properties:
> > + compatible:
> > + const: marvell,gti-wdt
> > +
> > + reg:
> > + maxItems: 1
> > +
> > + interrupts:
> > + maxItems: 1
> > +
> > + wdt-timer-index:
>
> missing vendor prefix
ack
>
> missing type
Will add
>
> > + maxItems: 1
>
> ???
Removed
>
> > + 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.
>
> > +
> > +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>;
>
> Use defines for flags.
Okay.
Thanks
-Bharat
>
> > + wdt-timer-index = <63>;
>
>
> Best regards,
> Krzysztof
