linux-kernel - Re: [PATCH v1 1/3] dt-bindings: watchdog: Add watchdog for StarFive

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: <e0551ed7-2208-6d08-235b-993702f0d89b@starfivetech.com>
Date:   Mon, 5 Dec 2022 11:49:07 +0800
From:   Xingyu Wu <xingyu.wu@...rfivetech.com>
To:     Krzysztof Kozlowski <krzysztof.kozlowski@...aro.org>
CC:     Rob Herring <robh+dt@...nel.org>,
        Paul Walmsley <paul.walmsley@...ive.com>,
        <linux-riscv@...ts.infradead.org>, <devicetree@...r.kernel.org>,
        <linux-watchdog@...r.kernel.org>,
        "Palmer Dabbelt" <palmer@...belt.com>,
        Albert Ou <aou@...s.berkeley.edu>,
        "Wim Van Sebroeck" <wim@...ux-watchdog.org>,
        Philipp Zabel <p.zabel@...gutronix.de>,
        Samin Guo <samin.guo@...rfivetech.com>,
        Guenter Roeck <linux@...ck-us.net>,
        <linux-kernel@...r.kernel.org>
Subject: Re: [PATCH v1 1/3] dt-bindings: watchdog: Add watchdog for StarFive

On 2022/12/2 18:46, Krzysztof Kozlowski wrote:
> On 02/12/2022 10:39, xingu.wu wrote:
>> From: Xingyu Wu <xingyu.wu@...rfivetech.com>
>> 
>> Add bindings to describe the watchdog for the StarFive SoCs.
>> 
>> Signed-off-by: Xingyu Wu <xingyu.wu@...rfivetech.com>
>> ---
>>  .../bindings/watchdog/starfive,wdt.yaml       | 77 +++++++++++++++++++
>>  1 file changed, 77 insertions(+)
>>  create mode 100644 Documentation/devicetree/bindings/watchdog/starfive,wdt.yaml
>> 
>> diff --git a/Documentation/devicetree/bindings/watchdog/starfive,wdt.yaml b/Documentation/devicetree/bindings/watchdog/starfive,wdt.yaml
>> new file mode 100644
>> index 000000000000..ece3e80153a0
>> --- /dev/null
>> +++ b/Documentation/devicetree/bindings/watchdog/starfive,wdt.yaml
> 
> Filename should be based on compatible. You do not allow here any other
> models... If you intent and you are 100% sure you will grow with new
> models, make it maybe a family-based name.

First, thank you for your reply. We have some other SoCs like JH7100 would use
this watchdog driver, but we now use JH7110 as our main release chip.
As you say, should we use "starfive,jh71xx-wdt.yaml" as filename?


>> @@ -0,0 +1,77 @@
>> +# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
>> +%YAML 1.2
>> +---
>> +$id: http://devicetree.org/schemas/watchdog/starfive,wdt.yaml#
>> +$schema: http://devicetree.org/meta-schemas/core.yaml#
>> +
>> +title: StarFive Watchdog
>> +
>> +allOf:
>> +  - $ref: "watchdog.yaml#"
> 
> Drop quotes.

Will fix.
>> +
>> +maintainers:
>> +  - Samin Guo <samin.guo@...rfivetech.com>
>> +  - Xingyu Wu <xingyu.wu@...rfivetech.com>
>> +
>> +properties:
>> +  compatible:
>> +    const: starfive,jh7110-wdt
>> +
>> +  reg:
>> +    maxItems: 1
>> +
>> +  interrupts:
>> +    maxItems: 1
>> +
>> +  clocks:
>> +    items:
>> +      - description: Core clock
>> +      - description: APB clock
>> +
>> +  clock-names:
>> +    items:
>> +      - const: core_clk
> 
> Drop _clk

Will fix.

> 
>> +      - const: apb_clk
> 
> Drop _clk

Will fix.

> 
>> +
>> +  resets:
>> +    items:
>> +      - description: APB reset
>> +      - description: Core reset
>> +
>> +  reset-names:
>> +    items:
>> +      - const: rst_apb
> 
> Drop rst_

Will fix.

> 
>> +      - const: rst_core
> 
> Ditto

Will fix.

> 
>> +
>> +  timeout-sec: true
>> +
>> +required:
>> +  - compatible
>> +  - reg
>> +  - interrupts
>> +  - clocks
>> +  - clock-names
>> +  - resets
>> +  - reset-names
>> +  - timeout-sec
>> +
>> +additionalProperties: false
>> +
>> +examples:
>> +  - |
>> +    #include <dt-bindings/clock/starfive-jh7110.h>
>> +    #include <dt-bindings/reset/starfive-jh7110.h>
>> +
>> +    watchdog@...70000 {
>> +            compatible = "starfive,jh7110-wdt";
> 
> Use 4 spaces for example indentation.

Will fix.

> 
>> +            reg = <0x13070000 0x10000>;
>> +            interrupts = <68>;
>> +            clocks = <&syscrg_clk JH7110_SYSCLK_WDT_CORE>,
>> +                     <&syscrg_clk JH7110_SYSCLK_WDT_APB>;
>> +            clock-names = "core_clk", "apb_clk";
>> +            resets = <&syscrg_rst JH7110_SYSRST_WDT_APB>,
>> +                     <&syscrg_rst JH7110_SYSRST_WDT_CORE>;
>> +            reset-names = "rst_apb", "rst_core";
>> +            timeout-sec = <15>;
>> +    };
>> +
> 
> Drop trailing line.

Will fix.

Best regards,
Xingyu Wu