| Message ID | 20221202093943.149674-2-xingyu.wu@starfivetech.com (mailing list archive) |
|---|---|
| State | Not Applicable |
| Headers | show |
| Series | Add watchdog driver for StarFive RISC-V SoCs | expand |
| Context | Check | Description |
|---|---|---|
| conchuod/tree_selection | fail | Guessing tree name failed |
On 02/12/2022 10:39, xingu.wu wrote: > From: Xingyu Wu <xingyu.wu@starfivetech.com> > > Add bindings to describe the watchdog for the StarFive SoCs. > > Signed-off-by: Xingyu Wu <xingyu.wu@starfivetech.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. > @@ -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. > + > +maintainers: > + - Samin Guo <samin.guo@starfivetech.com> > + - Xingyu Wu <xingyu.wu@starfivetech.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 > + - const: apb_clk Drop _clk > + > + resets: > + items: > + - description: APB reset > + - description: Core reset > + > + reset-names: > + items: > + - const: rst_apb Drop rst_ > + - const: rst_core Ditto > + > + 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@13070000 { > + compatible = "starfive,jh7110-wdt"; Use 4 spaces for example indentation. > + 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. Best regards, Krzysztof
On 2022/12/2 18:46, Krzysztof Kozlowski wrote: > On 02/12/2022 10:39, xingu.wu wrote: >> From: Xingyu Wu <xingyu.wu@starfivetech.com> >> >> Add bindings to describe the watchdog for the StarFive SoCs. >> >> Signed-off-by: Xingyu Wu <xingyu.wu@starfivetech.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@starfivetech.com> >> + - Xingyu Wu <xingyu.wu@starfivetech.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@13070000 { >> + 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
On 05/12/2022 04:49, Xingyu Wu wrote: > On 2022/12/2 18:46, Krzysztof Kozlowski wrote: >> On 02/12/2022 10:39, xingu.wu wrote: >>> From: Xingyu Wu <xingyu.wu@starfivetech.com> >>> >>> Add bindings to describe the watchdog for the StarFive SoCs. >>> >>> Signed-off-by: Xingyu Wu <xingyu.wu@starfivetech.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? starfive,jh7110-wdt.yaml I would say because you do not expect any other models (const for compatible, not enum) Best regards, Krzysztof
On 2022/12/5 15:30, Krzysztof Kozlowski wrote: > On 05/12/2022 04:49, Xingyu Wu wrote: >> On 2022/12/2 18:46, Krzysztof Kozlowski wrote: >>> On 02/12/2022 10:39, xingu.wu wrote: >>>> From: Xingyu Wu <xingyu.wu@starfivetech.com> >>>> >>>> Add bindings to describe the watchdog for the StarFive SoCs. >>>> >>>> Signed-off-by: Xingyu Wu <xingyu.wu@starfivetech.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? > > starfive,jh7110-wdt.yaml > I would say because you do not expect any other models (const for > compatible, not enum) > Thanks, I will rename the file in the patches for next version. Best regards, Xingyu Wu
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 @@ -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#" + +maintainers: + - Samin Guo <samin.guo@starfivetech.com> + - Xingyu Wu <xingyu.wu@starfivetech.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 + - const: apb_clk + + resets: + items: + - description: APB reset + - description: Core reset + + reset-names: + items: + - const: rst_apb + - const: rst_core + + 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@13070000 { + compatible = "starfive,jh7110-wdt"; + 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>; + }; +