linux-kernel - RE: [EXT] Re: [PATCH 1/2 v7] dt-bindings: watchdog: marvell GTI system watchdog driver

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <DM5PR1801MB1883175A2538B85FEF571B7AE3769@DM5PR1801MB1883.namprd18.prod.outlook.com>
Date:   Tue, 9 May 2023 07:26:33 +0000
From:   Bharat Bhushan <bbhushan2@...vell.com>
To:     Krzysztof Kozlowski <krzysztof.kozlowski@...aro.org>,
        "wim@...ux-watchdog.org" <wim@...ux-watchdog.org>,
        "linux@...ck-us.net" <linux@...ck-us.net>,
        "robh+dt@...nel.org" <robh+dt@...nel.org>,
        "krzysztof.kozlowski+dt@...aro.org" 
        <krzysztof.kozlowski+dt@...aro.org>,
        "linux-watchdog@...r.kernel.org" <linux-watchdog@...r.kernel.org>,
        "devicetree@...r.kernel.org" <devicetree@...r.kernel.org>,
        "linux-kernel@...r.kernel.org" <linux-kernel@...r.kernel.org>,
        Sunil Kovvuri Goutham <sgoutham@...vell.com>
Subject: RE: [EXT] Re: [PATCH 1/2 v7] dt-bindings: watchdog: marvell GTI
 system watchdog driver



> -----Original Message-----
> From: Krzysztof Kozlowski <krzysztof.kozlowski@...aro.org>
> Sent: Tuesday, May 9, 2023 12:23 PM
> To: Bharat Bhushan <bbhushan2@...vell.com>; wim@...ux-watchdog.org;
> linux@...ck-us.net; robh+dt@...nel.org; krzysztof.kozlowski+dt@...aro.org;
> linux-watchdog@...r.kernel.org; devicetree@...r.kernel.org; linux-
> kernel@...r.kernel.org; Sunil Kovvuri Goutham <sgoutham@...vell.com>
> Subject: [EXT] Re: [PATCH 1/2 v7] dt-bindings: watchdog: marvell GTI system
> watchdog driver
> 
> External Email
> 
> ----------------------------------------------------------------------
> On 08/05/2023 15:15, Bharat Bhushan wrote:
> > Add binding documentation for the Marvell GTI system watchdog driver.
> >
> > Signed-off-by: Bharat Bhushan <bbhushan2@...vell.com>
> > ---
> > v7:
> >  - Corrected compatible to have soc name
> >  - Converted marvell,wdt-timer-index to optional
> >
> >  .../watchdog/marvell,octeontx2-wdt.yaml       | 80 +++++++++++++++++++
> >  1 file changed, 80 insertions(+)
> >  create mode 100644
> > Documentation/devicetree/bindings/watchdog/marvell,octeontx2-wdt.yaml
> >
> > diff --git
> > a/Documentation/devicetree/bindings/watchdog/marvell,octeontx2-wdt.yam
> > l
> > b/Documentation/devicetree/bindings/watchdog/marvell,octeontx2-wdt.yam
> > l
> > new file mode 100644
> > index 000000000000..72951b10f1f3
> > --- /dev/null
> > +++ b/Documentation/devicetree/bindings/watchdog/marvell,octeontx2-wdt
> > +++ .yaml
> > @@ -0,0 +1,80 @@
> > +# 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-2Cocteontx2-2Dwdt.yaml-
> 23&d=DwICaQ&c=nKjWec2b6
> >
> +R0mOyPaz7xtfQ&r=PAAlWswPe7d8gHlGbCLmy2YezyK7O3Hv_t2heGnouBw&m
> =4PmICMz
> >
> +jaooxZnBaJszNihWBPjS99OeVsnJTSsOxNyAhC83AsKQuNVano9_YP2Oj&s=s1E00
> h2dU
> > +roSz4xdrUQMO3_19ren2IoeshKFZjDWKrw&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=4PmICMzjaooxZnBaJszNihW
> BPjS99O
> >
> +eVsnJTSsOxNyAhC83AsKQuNVano9_YP2Oj&s=MFuqeS8S00OEgao3hV2RkzNGX
> 2p0Jycp
> > +nZo_sLygcbk&e=
> > +
> > +title: Marvell Global Timer (GTI) system watchdog
> > +
> > +allOf:
> > +  - $ref: watchdog.yaml#
> 
> Put allOf after maintainers:.

Ok

> 
> > +
> > +maintainers:
> > +  - Bharat Bhushan <bbhushan2@...vell.com>
> > +
> > +properties:
> > +  compatible:
> > +    oneOf:
> > +      - const: marvell,octeontx2-wdt
> 
> Why is this alone? Judging by the enum below, octeontx2 is not specific.
> 
> > +      - items:
> > +          - enum:
> > +              - marvell,octeontx2-95xx-wdt
> > +              - marvell,octeontx2-96xx-wdt
> > +              - marvell,octeontx2-98xx-wdt
> 
> We don't allow wildcards in general

Marvell have octeontx2 series of processor which have watchdog timer.
In 95xx,98xx,96xx are the processors in octeontx2 series of processor. So octeontx2-95xx is on soc, octeontx2-96xx is another and so on.

> 
> > +          - const: marvell,octeontx2-wdt
> > +      - const: marvell,cn10k-wdt
> 
> Same question - why is this alone?
Same here, Marvell have cn10k series of processors and cn10kx and cnf10kx are the processor in this series.

One of the difference between octeontx2 and cn10k series processor is number of timers available. Which within the available set of timers one of the timer is programmed to be watchdog timer. 

Can you please propose how you want these compatible to be defined?

> 
> Second question - it should be rather part of enum with the first one if accepted.
> 
> > +      - items:
> > +          - enum:
> > +              - marvell,cn10kx-wdt
> > +              - marvell,cnf10kx-wdt
> > +          - const: marvell,cn10k-wdt
> > +
> > +  reg:
> > +    maxItems: 1
> > +
> > +  interrupts:
> > +    maxItems: 1
> > +
> > +  clocks:
> > +    minItems: 1
> 
> maxItems instead. You see it is different than above properties?

Okay,

> 
> > +
> > +  clock-names:
> > +    minItems: 1
> 
> Need to define names.

Okay,

Thanks
-Bharat

> 
> Best regards,
> Krzysztof