* [PATCH v1 0/3] Add watchdog driver for StarFive RISC-V SoCs
@ 2022-12-02 9:39 xingu.wu
2022-12-02 9:39 ` [PATCH v1 1/3] dt-bindings: watchdog: Add watchdog for StarFive xingu.wu
` (3 more replies)
0 siblings, 4 replies; 13+ messages in thread
From: xingu.wu @ 2022-12-02 9:39 UTC (permalink / raw)
To: linux-riscv, devicetree, linux-watchdog, Wim Van Sebroeck,
Guenter Roeck
Cc: Rob Herring, Krzysztof Kozlowski, Paul Walmsley, Palmer Dabbelt,
Albert Ou, Philipp Zabel, Xingyu Wu, Samin Guo, linux-kernel
This patch serises are to add watchdog driver for the StarFive RISC-V SoCs
such as JH7110. The first patch adds docunmentation to describe device
tree bindings. The subsequent patch adds watchdog driver and support
JH7110 SoC. The last patch adds device node about watchdog to JH7110
dts.
The watchdog driver has been tested on the VisionFive 2 boards which
equip with JH7110 SoC and works normally.
This patchset should be applied after the patchset [1], [2], [3]:
[1] https://lore.kernel.org/all/20221118010627.70576-1-hal.feng@starfivetech.com/
[2] https://lore.kernel.org/all/20221118011108.70715-1-hal.feng@starfivetech.com/
[3] https://lore.kernel.org/all/20221118011714.70877-1-hal.feng@starfivetech.com/
Xingyu Wu (3):
dt-bindings: watchdog: Add watchdog for StarFive
drivers: watchdog: Add StarFive Watchdog driver
riscv: dts: starfive: jh7110: Add watchdog node
.../bindings/watchdog/starfive,wdt.yaml | 77 ++
MAINTAINERS | 7 +
arch/riscv/boot/dts/starfive/jh7110.dtsi | 14 +
drivers/watchdog/Kconfig | 11 +
drivers/watchdog/Makefile | 3 +
drivers/watchdog/starfive-wdt.c | 854 ++++++++++++++++++
6 files changed, 966 insertions(+)
create mode 100644 Documentation/devicetree/bindings/watchdog/starfive,wdt.yaml
create mode 100644 drivers/watchdog/starfive-wdt.c
base-commit: 094226ad94f471a9f19e8f8e7140a09c2625abaa
prerequisite-patch-id: 6b1b43a55b9773bec61ab6c1bbaa54dccbac0837
prerequisite-patch-id: 09c98554df52d17ba5fd604125f8cdd62cbe80d1
prerequisite-patch-id: 29fe0b0c19b6f0cd31114ee9fe17fe9732047f33
prerequisite-patch-id: c59d9908de90e09ba2b9a81aadbf9fb9f00c8f04
prerequisite-patch-id: 94ac03d518993921bcfc9cc9f58d7da0c3528b51
prerequisite-patch-id: 694f7400375f5b85581fc1821e427334507826f2
prerequisite-patch-id: 699d49c4439dadb4b7cf900857f027d050cd6093
prerequisite-patch-id: 40d773f5a19912f731ee5fd4739ed2e3c2157b07
prerequisite-patch-id: 2bc3fd6df5dda116efe882045863d6c88aa81b3a
prerequisite-patch-id: 735e62255c75801bdc4c0b4107850bce821ff7f5
prerequisite-patch-id: b2a923b922e661fa6085185f33c1f1e733db9110
prerequisite-patch-id: b2bbc28354075432f059344eba5a127a653475cf
prerequisite-patch-id: 70eab7b7eee728afcd90e40f6743d1356f6d81ab
prerequisite-patch-id: 6276b2a23818c65ff2ad3d65b562615690cffee9
prerequisite-patch-id: d834ece14ffb525b8c3e661e78736692f33fca9b
prerequisite-patch-id: 4c17a3ce4dae9b788795d915bf775630f5c43c53
prerequisite-patch-id: dabb913fd478e97593e45c23fee4be9fd807f851
prerequisite-patch-id: 22fa141f7f0f80a5d619e9f3f4cf161ad06f108e
prerequisite-patch-id: f306819c257ea73aff8e06b17b5731053cdddfc8
prerequisite-patch-id: 536114f0732646095ef5302a165672b3290d4c75
prerequisite-patch-id: 258ea5f9b8bf41b6981345dcc81795f25865d38f
prerequisite-patch-id: 8b6f2c9660c0ac0ee4e73e4c21aca8e6b75e81b9
prerequisite-patch-id: e09e995700a814a763aa304ad3881a7222acf556
prerequisite-patch-id: 05803238293fcc90c8e83018a1103c41133a816c
prerequisite-patch-id: d074c7ffa2917a9f754d5801e3f67bc980f9de4c
prerequisite-patch-id: ef23fdf3466b3c713b3826e8545c8dd2bc6cc9d7
prerequisite-patch-id: d5da3475c6a3588e11a1678feb565bdd459b548e
--
2.25.1
_______________________________________________
linux-riscv mailing list
linux-riscv@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-riscv
^ permalink raw reply [flat|nested] 13+ messages in thread* [PATCH v1 1/3] dt-bindings: watchdog: Add watchdog for StarFive 2022-12-02 9:39 [PATCH v1 0/3] Add watchdog driver for StarFive RISC-V SoCs xingu.wu @ 2022-12-02 9:39 ` xingu.wu 2022-12-02 10:46 ` Krzysztof Kozlowski 2022-12-02 9:39 ` [PATCH v1 2/3] drivers: watchdog: Add StarFive Watchdog driver xingu.wu ` (2 subsequent siblings) 3 siblings, 1 reply; 13+ messages in thread From: xingu.wu @ 2022-12-02 9:39 UTC (permalink / raw) To: linux-riscv, devicetree, linux-watchdog, Wim Van Sebroeck, Guenter Roeck Cc: Rob Herring, Krzysztof Kozlowski, Paul Walmsley, Palmer Dabbelt, Albert Ou, Philipp Zabel, Xingyu Wu, Samin Guo, linux-kernel 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 @@ -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>; + }; + -- 2.25.1 _______________________________________________ linux-riscv mailing list linux-riscv@lists.infradead.org http://lists.infradead.org/mailman/listinfo/linux-riscv ^ permalink raw reply related [flat|nested] 13+ messages in thread
* Re: [PATCH v1 1/3] dt-bindings: watchdog: Add watchdog for StarFive 2022-12-02 9:39 ` [PATCH v1 1/3] dt-bindings: watchdog: Add watchdog for StarFive xingu.wu @ 2022-12-02 10:46 ` Krzysztof Kozlowski 2022-12-05 3:49 ` Xingyu Wu 0 siblings, 1 reply; 13+ messages in thread From: Krzysztof Kozlowski @ 2022-12-02 10:46 UTC (permalink / raw) To: xingu.wu, linux-riscv, devicetree, linux-watchdog, Wim Van Sebroeck, Guenter Roeck Cc: Rob Herring, Krzysztof Kozlowski, Paul Walmsley, Palmer Dabbelt, Albert Ou, Philipp Zabel, Samin Guo, linux-kernel 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 _______________________________________________ linux-riscv mailing list linux-riscv@lists.infradead.org http://lists.infradead.org/mailman/listinfo/linux-riscv ^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: [PATCH v1 1/3] dt-bindings: watchdog: Add watchdog for StarFive 2022-12-02 10:46 ` Krzysztof Kozlowski @ 2022-12-05 3:49 ` Xingyu Wu 2022-12-05 7:30 ` Krzysztof Kozlowski 0 siblings, 1 reply; 13+ messages in thread From: Xingyu Wu @ 2022-12-05 3:49 UTC (permalink / raw) To: Krzysztof Kozlowski Cc: Rob Herring, Paul Walmsley, linux-riscv, devicetree, linux-watchdog, Palmer Dabbelt, Albert Ou, Wim Van Sebroeck, Philipp Zabel, Samin Guo, Guenter Roeck, linux-kernel 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 _______________________________________________ linux-riscv mailing list linux-riscv@lists.infradead.org http://lists.infradead.org/mailman/listinfo/linux-riscv ^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: [PATCH v1 1/3] dt-bindings: watchdog: Add watchdog for StarFive 2022-12-05 3:49 ` Xingyu Wu @ 2022-12-05 7:30 ` Krzysztof Kozlowski 2022-12-05 8:22 ` Xingyu Wu 0 siblings, 1 reply; 13+ messages in thread From: Krzysztof Kozlowski @ 2022-12-05 7:30 UTC (permalink / raw) To: Xingyu Wu Cc: Rob Herring, Paul Walmsley, linux-riscv, devicetree, linux-watchdog, Palmer Dabbelt, Albert Ou, Wim Van Sebroeck, Philipp Zabel, Samin Guo, Guenter Roeck, linux-kernel 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 _______________________________________________ linux-riscv mailing list linux-riscv@lists.infradead.org http://lists.infradead.org/mailman/listinfo/linux-riscv ^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: [PATCH v1 1/3] dt-bindings: watchdog: Add watchdog for StarFive 2022-12-05 7:30 ` Krzysztof Kozlowski @ 2022-12-05 8:22 ` Xingyu Wu 0 siblings, 0 replies; 13+ messages in thread From: Xingyu Wu @ 2022-12-05 8:22 UTC (permalink / raw) To: Krzysztof Kozlowski Cc: Rob Herring, Paul Walmsley, linux-riscv, devicetree, linux-watchdog, Palmer Dabbelt, Albert Ou, Wim Van Sebroeck, Philipp Zabel, Samin Guo, Guenter Roeck, linux-kernel 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 _______________________________________________ linux-riscv mailing list linux-riscv@lists.infradead.org http://lists.infradead.org/mailman/listinfo/linux-riscv ^ permalink raw reply [flat|nested] 13+ messages in thread
* [PATCH v1 2/3] drivers: watchdog: Add StarFive Watchdog driver 2022-12-02 9:39 [PATCH v1 0/3] Add watchdog driver for StarFive RISC-V SoCs xingu.wu 2022-12-02 9:39 ` [PATCH v1 1/3] dt-bindings: watchdog: Add watchdog for StarFive xingu.wu @ 2022-12-02 9:39 ` xingu.wu 2022-12-02 15:08 ` Guenter Roeck 2022-12-02 9:39 ` [PATCH v1 3/3] riscv: dts: starfive: jh7110: Add watchdog node xingu.wu 2022-12-02 11:52 ` [PATCH v1 0/3] Add watchdog driver for StarFive RISC-V SoCs Conor Dooley 3 siblings, 1 reply; 13+ messages in thread From: xingu.wu @ 2022-12-02 9:39 UTC (permalink / raw) To: linux-riscv, devicetree, linux-watchdog, Wim Van Sebroeck, Guenter Roeck Cc: Rob Herring, Krzysztof Kozlowski, Paul Walmsley, Palmer Dabbelt, Albert Ou, Philipp Zabel, Xingyu Wu, Samin Guo, linux-kernel From: Xingyu Wu <xingyu.wu@starfivetech.com> Add watchdog driver for the StarFive JH7110 SoC. Signed-off-by: Xingyu Wu <xingyu.wu@starfivetech.com> --- MAINTAINERS | 7 + drivers/watchdog/Kconfig | 11 + drivers/watchdog/Makefile | 3 + drivers/watchdog/starfive-wdt.c | 854 ++++++++++++++++++++++++++++++++ 4 files changed, 875 insertions(+) create mode 100644 drivers/watchdog/starfive-wdt.c diff --git a/MAINTAINERS b/MAINTAINERS index a70c1d0f303e..133214dd2f60 100644 --- a/MAINTAINERS +++ b/MAINTAINERS @@ -19623,6 +19623,13 @@ F: Documentation/devicetree/bindings/reset/starfive,jh7100-reset.yaml F: drivers/reset/starfive/ F: include/dt-bindings/reset/starfive* +STARFIVE WATCHDOG DRIVER +M: Xingyu Wu <xingyu.wu@starfivetech.com> +M: Samin Guo <samin.guo@starfivetech.com> +S: Supported +F: Documentation/devicetree/bindings/watchdog/starfive* +F: drivers/watchdog/starfive-wdt.c + STATIC BRANCH/CALL M: Peter Zijlstra <peterz@infradead.org> M: Josh Poimboeuf <jpoimboe@kernel.org> diff --git a/drivers/watchdog/Kconfig b/drivers/watchdog/Kconfig index b64bc49c7f30..658fc9fef4fa 100644 --- a/drivers/watchdog/Kconfig +++ b/drivers/watchdog/Kconfig @@ -2082,6 +2082,17 @@ config UML_WATCHDOG tristate "UML watchdog" depends on UML || COMPILE_TEST +# RISCV Architecture + +config STARFIVE_WATCHDOG + tristate "StarFive Watchdog support" + depends on RISCV + select WATCHDOG_CORE + default SOC_STARFIVE + help + Say Y here to support the StarFive watchdog. This driver can also + be built as a module if choose M. + # # ISA-based Watchdog Cards # diff --git a/drivers/watchdog/Makefile b/drivers/watchdog/Makefile index d41e5f830ae7..9c22f1a43d5c 100644 --- a/drivers/watchdog/Makefile +++ b/drivers/watchdog/Makefile @@ -210,6 +210,9 @@ obj-$(CONFIG_WATCHDOG_SUN4V) += sun4v_wdt.o # Xen obj-$(CONFIG_XEN_WDT) += xen_wdt.o +# RISCV Architecture +obj-$(CONFIG_STARFIVE_WATCHDOG) += starfive-wdt.o + # Architecture Independent obj-$(CONFIG_BD957XMUF_WATCHDOG) += bd9576_wdt.o obj-$(CONFIG_DA9052_WATCHDOG) += da9052_wdt.o diff --git a/drivers/watchdog/starfive-wdt.c b/drivers/watchdog/starfive-wdt.c new file mode 100644 index 000000000000..dcb6e693ba6c --- /dev/null +++ b/drivers/watchdog/starfive-wdt.c @@ -0,0 +1,854 @@ +// SPDX-License-Identifier: GPL-2.0 +/* + * Starfive Watchdog driver + * + * Copyright (C) 2022 StarFive Technology Co., Ltd. + */ + +#include <linux/clk.h> +#include <linux/delay.h> +#include <linux/err.h> +#include <linux/interrupt.h> +#include <linux/io.h> +#include <linux/iopoll.h> +#include <linux/module.h> +#include <linux/moduleparam.h> +#include <linux/of.h> +#include <linux/of_device.h> +#include <linux/platform_device.h> +#include <linux/pm_runtime.h> +#include <linux/reset.h> +#include <linux/reset-controller.h> +#include <linux/slab.h> +#include <linux/timer.h> +#include <linux/types.h> +#include <linux/uaccess.h> +#include <linux/watchdog.h> + +/* JH7110 WatchDog register define */ +#define STARFIVE_WDT_JH7110_LOAD 0x000 /* RW: Watchdog load register */ +#define STARFIVE_WDT_JH7110_VALUE 0x004 /* RO: The current value for the watchdog counter */ +#define STARFIVE_WDT_JH7110_CONTROL 0x008 /* + * RW: [0]: reset enable; + * [1]: int enable/wdt enable/reload counter; + * [31:2]: res. + */ +#define STARFIVE_WDT_JH7110_INTCLR 0x00c /* WO: clear intterupt && reload the counter */ +#define STARFIVE_WDT_JH7110_RIS 0x010 /* RO: Raw interrupt status from the counter */ +#define STARFIVE_WDT_JH7110_IMS 0x014 /* RO: Enabled interrupt status from the counter */ +#define STARFIVE_WDT_JH7110_LOCK 0xc00 /* + * RO: Enable write access to all other registers + * by writing 0x1ACCE551. + */ +#define STARFIVE_WDT_JH7110_ITCR 0xf00 /* + * RW: When set HIGH, + * places the Watchdog into integraeion test mode. + */ +#define STARFIVE_WDT_JH7110_ITOP 0xf04 /* + * WO: [0] WDOGRES value Integration Test Mode. + * Value output on WDOGRES when in + * Integration Test Mode. + * [1] Integration Test WDOGINT value. + * Value output on WDOGINT when in + * Integration Test Mode. + */ + +#define STARFIVE_WDT_JH7110_UNLOCK_KEY 0x1acce551 +#define STARFIVE_WDT_JH7110_RESEN_SHIFT 1 +#define STARFIVE_WDT_JH7110_EN_SHIFT 0 +#define STARFIVE_WDT_JH7110_INT_EN_SHIFT STARFIVE_WDT_JH7110_EN_SHIFT + +/* WDOGCONTROL */ +#define STARFIVE_WDT_INT_EN 0x0 +#define STARFIVE_WDT_INT_DIS BIT(0) +#define STARFIVE_WDT_RESET_EN 0x1 + +/* WDOGLOCK */ +#define STARFIVE_WDT_LOCKED BIT(0) + +#define STARFIVE_WDT_INTCLR 0x1 +#define STARFIVE_WDT_ENABLE 0x1 +#define STARFIVE_WDT_ATBOOT 0x0 +#define STARFIVE_WDT_MAXCNT 0xffffffff + +#define STARFIVE_WDT_DEFAULT_TIME (15) +#define STARFIVE_WDT_DELAY_US 0 +#define STARFIVE_WDT_TIMEOUT_US 10000 + +static bool nowayout = WATCHDOG_NOWAYOUT; +static int tmr_margin; +static int tmr_atboot = STARFIVE_WDT_ATBOOT; +static int soft_noboot; + +module_param(tmr_margin, int, 0); +module_param(tmr_atboot, int, 0); +module_param(nowayout, bool, 0); +module_param(soft_noboot, int, 0); + +MODULE_PARM_DESC(tmr_margin, "Watchdog tmr_margin in seconds. (default=" + __MODULE_STRING(STARFIVE_WDT_DEFAULT_TIME) ")"); +MODULE_PARM_DESC(tmr_atboot, + "Watchdog is started at boot time if set to 1, default=" + __MODULE_STRING(STARFIVE_WDT_ATBOOT)); +MODULE_PARM_DESC(nowayout, "Watchdog cannot be stopped once started (default=" + __MODULE_STRING(WATCHDOG_NOWAYOUT) ")"); +MODULE_PARM_DESC(soft_noboot, + "Watchdog action, set to 1 to ignore reboots, 0 to reboot (default 0)"); + +struct starfive_wdt_variant_t { + u32 unlock_key; + u8 enrst_shift; + u8 en_shift; + u8 intclr_check; + u8 intclr_ava_shift; +}; + +struct starfive_wdt_variant { + u32 control; + u32 load; + u32 enable; + u32 reload; + u32 value; + u32 int_clr; + u32 int_mask; + u32 unlock; + u32 enter_irq_status; + struct starfive_wdt_variant_t *variant; +}; + +struct starfive_wdt { + u64 freq; + struct device *dev; + struct watchdog_device wdt_device; + struct clk *core_clk; + struct clk *apb_clk; + struct reset_control *rsts; + const struct starfive_wdt_variant *drv_data; + u32 count; /*count of timeout*/ + u32 reload; /*restore the count*/ + void __iomem *base; + spinlock_t lock; /* spinlock for register handling */ + + int irq; +}; + +#ifdef CONFIG_OF +static struct starfive_wdt_variant_t jh7110_variant = { + .unlock_key = STARFIVE_WDT_JH7110_UNLOCK_KEY, + .enrst_shift = STARFIVE_WDT_JH7110_RESEN_SHIFT, + .en_shift = STARFIVE_WDT_JH7110_EN_SHIFT, +}; + +static const struct starfive_wdt_variant drv_data_jh7110 = { + .control = STARFIVE_WDT_JH7110_CONTROL, + .load = STARFIVE_WDT_JH7110_LOAD, + .enable = STARFIVE_WDT_JH7110_CONTROL, + .value = STARFIVE_WDT_JH7110_VALUE, + .int_clr = STARFIVE_WDT_JH7110_INTCLR, + .unlock = STARFIVE_WDT_JH7110_LOCK, + .enter_irq_status = STARFIVE_WDT_JH7110_IMS, + .variant = &jh7110_variant, +}; + +static const struct of_device_id starfive_wdt_match[] = { + { .compatible = "starfive,jh7110-wdt", .data = &drv_data_jh7110 }, + {} +}; +MODULE_DEVICE_TABLE(of, starfive_wdt_match); +#endif + +static const struct platform_device_id starfive_wdt_ids[] = { + { + .name = "starfive-wdt", + .driver_data = (unsigned long)&drv_data_jh7110, + }, + {} +}; +MODULE_DEVICE_TABLE(platform, starfive_wdt_ids); + +static int starfive_wdt_get_clock_rate(struct starfive_wdt *wdt) +{ + int ret; + u32 freq; + + if (!IS_ERR(wdt->core_clk)) { + wdt->freq = clk_get_rate(wdt->core_clk); + return 0; + } + dev_dbg(wdt->dev, "get rate failed, need clock-frequency define in dts.\n"); + + /* Next we try to get clock-frequency from dts.*/ + ret = of_property_read_u32(wdt->dev->of_node, "clock-frequency", &freq); + if (!ret) { + wdt->freq = (u64)freq; + return 0; + } + dev_err(wdt->dev, "get clock-frequency failed\n"); + + return -ENOENT; +} + +static int starfive_wdt_enable_clock(struct starfive_wdt *wdt) +{ + int ret = 0; + + wdt->apb_clk = devm_clk_get(wdt->dev, "apb_clk"); + if (!IS_ERR(wdt->apb_clk)) { + ret = clk_prepare_enable(wdt->apb_clk); + if (ret) { + dev_err(wdt->dev, "failed to enable apb_clk.\n"); + goto err; + } + } else { + dev_err(wdt->dev, "failed to get apb_clk.\n"); + ret = PTR_ERR(wdt->apb_clk); + goto err; + } + + wdt->core_clk = devm_clk_get(wdt->dev, "core_clk"); + if (!IS_ERR(wdt->core_clk)) { + ret = clk_prepare_enable(wdt->core_clk); + if (ret) { + dev_err(wdt->dev, "failed to enable core_clk.\n"); + goto err; + } + } else { + dev_err(wdt->dev, "failed to get core_clk.\n"); + ret = PTR_ERR(wdt->core_clk); + goto err; + } + + return 0; + +err: + return ret; +} + +static int starfive_wdt_reset_init(struct starfive_wdt *wdt) +{ + int ret = 0; + + wdt->rsts = devm_reset_control_array_get_exclusive(wdt->dev); + if (!IS_ERR(wdt->rsts)) { + ret = reset_control_deassert(wdt->rsts); + if (ret) { + dev_err(wdt->dev, "failed to deassert rsts.\n"); + goto err; + } + } else { + dev_err(wdt->dev, "failed to get rsts error.\n"); + ret = PTR_ERR(wdt->rsts); + goto err; + } + + return 0; + +err: + return ret; +} + +static u32 starfive_wdt_ticks_to_sec(struct starfive_wdt *wdt, u32 ticks) +{ + return DIV_ROUND_CLOSEST(ticks, wdt->freq); +} + +/* + * Write unlock-key to unlock. Write other value to lock. When lock bit is 1, + * external accesses to other watchdog registers are ignored. + */ +static int starfive_wdt_is_locked(struct starfive_wdt *wdt) +{ + u32 val; + + val = readl(wdt->base + wdt->drv_data->unlock); + return !!(val & STARFIVE_WDT_LOCKED); +} + +static void starfive_wdt_unlock(struct starfive_wdt *wdt) +{ + if (starfive_wdt_is_locked(wdt)) + writel(wdt->drv_data->variant->unlock_key, + wdt->base + wdt->drv_data->unlock); +} + +static void starfive_wdt_lock(struct starfive_wdt *wdt) +{ + if (!starfive_wdt_is_locked(wdt)) + writel(~wdt->drv_data->variant->unlock_key, + wdt->base + wdt->drv_data->unlock); +} + +/* enable watchdog interrupt */ +static inline void starfive_wdt_int_enable(struct starfive_wdt *wdt) +{ + u32 val; + + if (wdt->drv_data->int_mask) { + val = readl(wdt->base + wdt->drv_data->int_mask); + val &= ~STARFIVE_WDT_INT_DIS; + writel(val, wdt->base + wdt->drv_data->int_mask); + } +} + +/* disable watchdog interrupt */ +static inline void starfive_wdt_int_disable(struct starfive_wdt *wdt) +{ + u32 val; + + if (wdt->drv_data->int_mask) { + val = readl(wdt->base + wdt->drv_data->int_mask); + val |= STARFIVE_WDT_INT_DIS; + writel(val, wdt->base + wdt->drv_data->int_mask); + } +} + +/* enable watchdog interrupt to reset */ +static void starfive_wdt_enable_reset(struct starfive_wdt *wdt) +{ + u32 val; + + val = readl(wdt->base + wdt->drv_data->control); + val |= STARFIVE_WDT_RESET_EN << wdt->drv_data->variant->enrst_shift; + writel(val, wdt->base + wdt->drv_data->control); +} + +/* disable watchdog interrupt to reset */ +static void starfive_wdt_disable_reset(struct starfive_wdt *wdt) +{ + u32 val; + + val = readl(wdt->base + wdt->drv_data->control); + val &= ~(STARFIVE_WDT_RESET_EN << wdt->drv_data->variant->enrst_shift); + writel(val, wdt->base + wdt->drv_data->control); +} + +/* interrupt status whether has been raised from the counter */ +static bool starfive_wdt_raise_irq_status(struct starfive_wdt *wdt) +{ + return !!readl(wdt->base + wdt->drv_data->enter_irq_status); +} + +static void starfive_wdt_int_clr(struct starfive_wdt *wdt) +{ + void __iomem *addr; + u8 clr_check; + u8 clr_ava_shift; + u32 value; + int ret = 0; + + addr = wdt->base + wdt->drv_data->int_clr; + clr_ava_shift = wdt->drv_data->variant->intclr_ava_shift; + clr_check = wdt->drv_data->variant->intclr_check; + if (clr_check) { + /* waiting interrupt can be to clearing */ + value = readl(addr); + ret = readl_poll_timeout_atomic(addr, value, + !(value & BIT(clr_ava_shift)), + STARFIVE_WDT_DELAY_US, STARFIVE_WDT_TIMEOUT_US); + } + + if (!ret) + writel(STARFIVE_WDT_INTCLR, addr); + + if (starfive_wdt_raise_irq_status(wdt)) + enable_irq(wdt->irq); +} + +static inline void starfive_wdt_set_count(struct starfive_wdt *wdt, u32 val) +{ + writel(val, wdt->base + wdt->drv_data->load); +} + +static inline u32 starfive_wdt_get_count(struct starfive_wdt *wdt) +{ + return readl(wdt->base + wdt->drv_data->value); +} + +static inline void starfive_wdt_enable(struct starfive_wdt *wdt) +{ + u32 val; + + val = readl(wdt->base + wdt->drv_data->enable); + val |= STARFIVE_WDT_ENABLE << wdt->drv_data->variant->en_shift; + writel(val, wdt->base + wdt->drv_data->enable); +} + +static inline void starfive_wdt_disable(struct starfive_wdt *wdt) +{ + u32 val; + + val = readl(wdt->base + wdt->drv_data->enable); + val &= ~(STARFIVE_WDT_ENABLE << wdt->drv_data->variant->en_shift); + writel(val, wdt->base + wdt->drv_data->enable); +} + +static inline void starfive_wdt_set_relod_count(struct starfive_wdt *wdt, u32 count) +{ + writel(count, wdt->base + wdt->drv_data->load); + if (wdt->drv_data->reload) + writel(0x1, wdt->base + wdt->drv_data->reload); + else + /* jh7110 need enable controller to reload counter */ + starfive_wdt_enable(wdt); +} + +static void starfive_wdt_mask_and_disable_reset(struct starfive_wdt *wdt, bool mask) +{ + starfive_wdt_unlock(wdt); + + if (mask) + starfive_wdt_disable_reset(wdt); + else + starfive_wdt_enable_reset(wdt); + + starfive_wdt_lock(wdt); +} + +static unsigned int starfive_wdt_max_timeout(struct starfive_wdt *wdt) +{ + return DIV_ROUND_UP(STARFIVE_WDT_MAXCNT, wdt->freq) - 1; +} + +static unsigned int starfive_wdt_get_timeleft(struct watchdog_device *wdd) +{ + struct starfive_wdt *wdt = watchdog_get_drvdata(wdd); + u32 count; + u32 irq_status; + + starfive_wdt_unlock(wdt); + /* + * Because set half count value, + * timeleft value should add the count value before first timeout. + */ + irq_status = starfive_wdt_raise_irq_status(wdt) ? 1 : 0; + count = starfive_wdt_get_count(wdt) + (1 - irq_status) * wdt->count; + starfive_wdt_lock(wdt); + + return starfive_wdt_ticks_to_sec(wdt, count); +} + +static int starfive_wdt_keepalive(struct watchdog_device *wdd) +{ + struct starfive_wdt *wdt = watchdog_get_drvdata(wdd); + + spin_lock(&wdt->lock); + + starfive_wdt_unlock(wdt); + starfive_wdt_int_clr(wdt); + starfive_wdt_set_relod_count(wdt, wdt->count); + starfive_wdt_lock(wdt); + + spin_unlock(&wdt->lock); + + return 0; +} + +static irqreturn_t starfive_wdt_interrupt_handler(int irq, void *data) +{ + /* + * We don't clear the IRQ status. It's supposed to be done by the + * following ping operations. + */ + struct platform_device *pdev = data; + struct starfive_wdt *wdt = platform_get_drvdata(pdev); + + /* Disable the IRQ and avoid re-entry interrupt. */ + disable_irq_nosync(wdt->irq); + + return IRQ_HANDLED; +} + +static int starfive_wdt_stop(struct watchdog_device *wdd) +{ + struct starfive_wdt *wdt = watchdog_get_drvdata(wdd); + + spin_lock(&wdt->lock); + + starfive_wdt_unlock(wdt); + starfive_wdt_int_disable(wdt); + starfive_wdt_int_clr(wdt); + starfive_wdt_disable(wdt); + starfive_wdt_lock(wdt); + + spin_unlock(&wdt->lock); + + return 0; +} + +static int starfive_wdt_pm_stop(struct watchdog_device *wdd) +{ + struct starfive_wdt *wdt = watchdog_get_drvdata(wdd); + + starfive_wdt_stop(wdd); + pm_runtime_put(wdt->dev); + + return 0; +} + +static int starfive_wdt_start(struct watchdog_device *wdd) +{ + struct starfive_wdt *wdt = watchdog_get_drvdata(wdd); + + pm_runtime_get_sync(wdt->dev); + + spin_lock(&wdt->lock); + + starfive_wdt_unlock(wdt); + + if (soft_noboot) + starfive_wdt_disable_reset(wdt); + else + starfive_wdt_enable_reset(wdt); + + starfive_wdt_int_clr(wdt); + starfive_wdt_set_count(wdt, wdt->count); + starfive_wdt_int_enable(wdt); + starfive_wdt_enable(wdt); + + starfive_wdt_lock(wdt); + + spin_unlock(&wdt->lock); + + return 0; +} + +static int starfive_wdt_restart(struct watchdog_device *wdd, unsigned long action, + void *data) +{ + struct starfive_wdt *wdt = watchdog_get_drvdata(wdd); + + spin_lock(&wdt->lock); + starfive_wdt_unlock(wdt); + /* disable watchdog, to be safe */ + starfive_wdt_disable(wdt); + + if (soft_noboot) + starfive_wdt_disable_reset(wdt); + else + starfive_wdt_enable_reset(wdt); + + /* put initial values into count and data */ + starfive_wdt_set_count(wdt, wdt->count); + + /* set the watchdog to go and reset... */ + starfive_wdt_int_clr(wdt); + starfive_wdt_int_enable(wdt); + starfive_wdt_enable(wdt); + + starfive_wdt_lock(wdt); + spin_unlock(&wdt->lock); + + return 0; +} + +static int starfive_wdt_set_timeout(struct watchdog_device *wdd, + unsigned int timeout) +{ + struct starfive_wdt *wdt = watchdog_get_drvdata(wdd); + + unsigned long freq = wdt->freq; + unsigned int count; + + spin_lock(&wdt->lock); + + if (timeout < 1) + return -EINVAL; + + /* + * This watchdog takes twice timeouts to reset. + * In order to reduce time to reset, should set half count value. + */ + count = timeout * freq / 2; + + if (count > STARFIVE_WDT_MAXCNT) { + dev_warn(wdt->dev, "timeout %d too big,use the MAX-timeout set.\n", + timeout); + timeout = starfive_wdt_max_timeout(wdt); + count = timeout * freq; + } + + dev_info(wdt->dev, "Heartbeat: timeout=%d, count/2=%d (%08x)\n", + timeout, count, count); + + starfive_wdt_unlock(wdt); + starfive_wdt_disable(wdt); + starfive_wdt_set_relod_count(wdt, count); + starfive_wdt_enable(wdt); + starfive_wdt_lock(wdt); + + wdt->count = count; + wdd->timeout = timeout; + + spin_unlock(&wdt->lock); + + return 0; +} + +#define OPTIONS (WDIOF_SETTIMEOUT | WDIOF_KEEPALIVEPING | WDIOF_MAGICCLOSE) + +static const struct watchdog_info starfive_wdt_ident = { + .options = OPTIONS, + .firmware_version = 0, + .identity = "StarFive Watchdog", +}; + +static const struct watchdog_ops starfive_wdt_ops = { + .owner = THIS_MODULE, + .start = starfive_wdt_start, + .stop = starfive_wdt_pm_stop, + .ping = starfive_wdt_keepalive, + .set_timeout = starfive_wdt_set_timeout, + .restart = starfive_wdt_restart, + .get_timeleft = starfive_wdt_get_timeleft, +}; + +static const struct watchdog_device starfive_wdd = { + .info = &starfive_wdt_ident, + .ops = &starfive_wdt_ops, + .timeout = STARFIVE_WDT_DEFAULT_TIME, +}; + +static inline const struct starfive_wdt_variant * +starfive_wdt_get_drv_data(struct platform_device *pdev) +{ + const struct starfive_wdt_variant *variant; + + variant = of_device_get_match_data(&pdev->dev); + if (!variant) { + /* Device matched by platform_device_id */ + variant = (struct starfive_wdt_variant *) + platform_get_device_id(pdev)->driver_data; + } + + return variant; +} + +static int starfive_wdt_probe(struct platform_device *pdev) +{ + struct device *dev = &pdev->dev; + struct starfive_wdt *wdt; + int started = 0; + int ret; + + pm_runtime_enable(dev); + + wdt = devm_kzalloc(dev, sizeof(*wdt), GFP_KERNEL); + if (!wdt) + return -ENOMEM; + + wdt->dev = dev; + spin_lock_init(&wdt->lock); + wdt->wdt_device = starfive_wdd; + + wdt->drv_data = starfive_wdt_get_drv_data(pdev); + + wdt->irq = platform_get_irq(pdev, 0); + if (wdt->irq < 0) { + dev_err(dev, "can not find irq.\n"); + return wdt->irq; + } + + /* get the memory region for the watchdog timer */ + wdt->base = devm_platform_ioremap_resource(pdev, 0); + if (IS_ERR(wdt->base)) { + ret = PTR_ERR(wdt->base); + goto err; + } + + ret = starfive_wdt_enable_clock(wdt); + if (ret) + goto err; + + starfive_wdt_get_clock_rate(wdt); + + ret = starfive_wdt_reset_init(wdt); + if (ret) + goto err; + + wdt->wdt_device.min_timeout = 1; + wdt->wdt_device.max_timeout = starfive_wdt_max_timeout(wdt); + + watchdog_set_drvdata(&wdt->wdt_device, wdt); + + /* + * see if we can actually set the requested timer margin, + * and if not, try the default value. + */ + watchdog_init_timeout(&wdt->wdt_device, tmr_margin, dev); + + ret = starfive_wdt_set_timeout(&wdt->wdt_device, + wdt->wdt_device.timeout); + if (ret) { + dev_err(dev, "tmr_margin value out of range, default %d used\n", + STARFIVE_WDT_DEFAULT_TIME); + starfive_wdt_set_timeout(&wdt->wdt_device, + STARFIVE_WDT_DEFAULT_TIME); + } + + ret = devm_request_irq(dev, wdt->irq, starfive_wdt_interrupt_handler, 0, + pdev->name, pdev); + if (ret != 0) { + dev_err(dev, "failed to install irq (%d)\n", ret); + goto err; + } + + watchdog_set_nowayout(&wdt->wdt_device, nowayout); + watchdog_set_restart_priority(&wdt->wdt_device, 128); + + wdt->wdt_device.parent = dev; + + ret = watchdog_register_device(&wdt->wdt_device); + if (ret) + goto err; + + starfive_wdt_mask_and_disable_reset(wdt, false); + + if (tmr_atboot && started == 0) { + starfive_wdt_start(&wdt->wdt_device); + } else if (!tmr_atboot) { + /* + *if we're not enabling the watchdog, then ensure it is + * disabled if it has been left running from the bootloader + * or other source. + */ + starfive_wdt_stop(&wdt->wdt_device); + } + +#ifdef CONFIG_PM + clk_disable_unprepare(wdt->core_clk); + clk_disable_unprepare(wdt->apb_clk); +#endif + + platform_set_drvdata(pdev, wdt); + + return 0; + +err: + return ret; +} + +static int starfive_wdt_remove(struct platform_device *dev) +{ + struct starfive_wdt *wdt = platform_get_drvdata(dev); + + starfive_wdt_mask_and_disable_reset(wdt, true); + watchdog_unregister_device(&wdt->wdt_device); + + clk_disable_unprepare(wdt->core_clk); + clk_disable_unprepare(wdt->apb_clk); + pm_runtime_disable(wdt->dev); + + return 0; +} + +static void starfive_wdt_shutdown(struct platform_device *dev) +{ + struct starfive_wdt *wdt = platform_get_drvdata(dev); + + starfive_wdt_mask_and_disable_reset(wdt, true); + + starfive_wdt_pm_stop(&wdt->wdt_device); +} + +#ifdef CONFIG_PM_SLEEP +static int starfive_wdt_suspend(struct device *dev) +{ + int ret; + struct starfive_wdt *wdt = dev_get_drvdata(dev); + + starfive_wdt_unlock(wdt); + + /* Save watchdog state, and turn it off. */ + wdt->reload = starfive_wdt_get_count(wdt); + + starfive_wdt_mask_and_disable_reset(wdt, true); + + /* Note that WTCNT doesn't need to be saved. */ + starfive_wdt_stop(&wdt->wdt_device); + pm_runtime_force_suspend(dev); + + starfive_wdt_lock(wdt); + + return 0; +} + +static int starfive_wdt_resume(struct device *dev) +{ + int ret; + struct starfive_wdt *wdt = dev_get_drvdata(dev); + + starfive_wdt_unlock(wdt); + + pm_runtime_force_resume(dev); + + /* Restore watchdog state. */ + starfive_wdt_set_relod_count(wdt, wdt->reload); + + starfive_wdt_restart(&wdt->wdt_device, 0, NULL); + + starfive_wdt_mask_and_disable_reset(wdt, false); + + starfive_wdt_lock(wdt); + + return 0; +} +#endif /* CONFIG_PM_SLEEP */ + +#ifdef CONFIG_PM +static int starfive_wdt_runtime_suspend(struct device *dev) +{ + struct starfive_wdt *wdt = dev_get_drvdata(dev); + + clk_disable_unprepare(wdt->apb_clk); + clk_disable_unprepare(wdt->core_clk); + + return 0; +} + +static int starfive_wdt_runtime_resume(struct device *dev) +{ + struct starfive_wdt *wdt = dev_get_drvdata(dev); + int ret; + + ret = clk_prepare_enable(wdt->apb_clk); + if (ret) { + dev_err(wdt->dev, "failed to enable apb_clk.\n"); + goto err; + } + + ret = clk_prepare_enable(wdt->core_clk); + if (ret) { + dev_err(wdt->dev, "failed to enable core_clk.\n"); + goto err; + } + + return 0; + +err: + return ret; +} +#endif /* CONFIG_PM */ + +static const struct dev_pm_ops starfive_wdt_pm_ops = { + SET_RUNTIME_PM_OPS(starfive_wdt_runtime_suspend, starfive_wdt_runtime_resume, NULL) + SET_SYSTEM_SLEEP_PM_OPS(starfive_wdt_suspend, starfive_wdt_resume) +}; + +static struct platform_driver starfive_wdt_driver = { + .probe = starfive_wdt_probe, + .remove = starfive_wdt_remove, + .shutdown = starfive_wdt_shutdown, + .id_table = starfive_wdt_ids, + .driver = { + .name = "starfive-wdt", + .pm = &starfive_wdt_pm_ops, + .of_match_table = of_match_ptr(starfive_wdt_match), + }, +}; + +module_platform_driver(starfive_wdt_driver); + +MODULE_AUTHOR("Xingyu Wu <xingyu.wu@starfivetech.com>"); +MODULE_AUTHOR("Samin Guo <samin.guo@starfivetech.com>"); +MODULE_DESCRIPTION("StarFive Watchdog Device Driver"); +MODULE_LICENSE("GPL v2"); -- 2.25.1 _______________________________________________ linux-riscv mailing list linux-riscv@lists.infradead.org http://lists.infradead.org/mailman/listinfo/linux-riscv ^ permalink raw reply related [flat|nested] 13+ messages in thread
* Re: [PATCH v1 2/3] drivers: watchdog: Add StarFive Watchdog driver 2022-12-02 9:39 ` [PATCH v1 2/3] drivers: watchdog: Add StarFive Watchdog driver xingu.wu @ 2022-12-02 15:08 ` Guenter Roeck 2022-12-05 8:08 ` Xingyu Wu 0 siblings, 1 reply; 13+ messages in thread From: Guenter Roeck @ 2022-12-02 15:08 UTC (permalink / raw) To: xingu.wu, linux-riscv, devicetree, linux-watchdog, Wim Van Sebroeck Cc: Rob Herring, Krzysztof Kozlowski, Paul Walmsley, Palmer Dabbelt, Albert Ou, Philipp Zabel, Samin Guo, linux-kernel On 12/2/22 01:39, xingu.wu wrote: > From: Xingyu Wu <xingyu.wu@starfivetech.com> > > Add watchdog driver for the StarFive JH7110 SoC. > > Signed-off-by: Xingyu Wu <xingyu.wu@starfivetech.com> > --- > MAINTAINERS | 7 + > drivers/watchdog/Kconfig | 11 + > drivers/watchdog/Makefile | 3 + > drivers/watchdog/starfive-wdt.c | 854 ++++++++++++++++++++++++++++++++ > 4 files changed, 875 insertions(+) > create mode 100644 drivers/watchdog/starfive-wdt.c > > diff --git a/MAINTAINERS b/MAINTAINERS > index a70c1d0f303e..133214dd2f60 100644 > --- a/MAINTAINERS > +++ b/MAINTAINERS > @@ -19623,6 +19623,13 @@ F: Documentation/devicetree/bindings/reset/starfive,jh7100-reset.yaml > F: drivers/reset/starfive/ > F: include/dt-bindings/reset/starfive* > > +STARFIVE WATCHDOG DRIVER > +M: Xingyu Wu <xingyu.wu@starfivetech.com> > +M: Samin Guo <samin.guo@starfivetech.com> > +S: Supported > +F: Documentation/devicetree/bindings/watchdog/starfive* > +F: drivers/watchdog/starfive-wdt.c > + > STATIC BRANCH/CALL > M: Peter Zijlstra <peterz@infradead.org> > M: Josh Poimboeuf <jpoimboe@kernel.org> > diff --git a/drivers/watchdog/Kconfig b/drivers/watchdog/Kconfig > index b64bc49c7f30..658fc9fef4fa 100644 > --- a/drivers/watchdog/Kconfig > +++ b/drivers/watchdog/Kconfig > @@ -2082,6 +2082,17 @@ config UML_WATCHDOG > tristate "UML watchdog" > depends on UML || COMPILE_TEST > > +# RISCV Architecture > + > +config STARFIVE_WATCHDOG > + tristate "StarFive Watchdog support" > + depends on RISCV > + select WATCHDOG_CORE > + default SOC_STARFIVE > + help > + Say Y here to support the StarFive watchdog. This driver can also > + be built as a module if choose M. > + > # > # ISA-based Watchdog Cards > # > diff --git a/drivers/watchdog/Makefile b/drivers/watchdog/Makefile > index d41e5f830ae7..9c22f1a43d5c 100644 > --- a/drivers/watchdog/Makefile > +++ b/drivers/watchdog/Makefile > @@ -210,6 +210,9 @@ obj-$(CONFIG_WATCHDOG_SUN4V) += sun4v_wdt.o > # Xen > obj-$(CONFIG_XEN_WDT) += xen_wdt.o > > +# RISCV Architecture > +obj-$(CONFIG_STARFIVE_WATCHDOG) += starfive-wdt.o > + > # Architecture Independent > obj-$(CONFIG_BD957XMUF_WATCHDOG) += bd9576_wdt.o > obj-$(CONFIG_DA9052_WATCHDOG) += da9052_wdt.o > diff --git a/drivers/watchdog/starfive-wdt.c b/drivers/watchdog/starfive-wdt.c > new file mode 100644 > index 000000000000..dcb6e693ba6c > --- /dev/null > +++ b/drivers/watchdog/starfive-wdt.c > @@ -0,0 +1,854 @@ > +// SPDX-License-Identifier: GPL-2.0 > +/* > + * Starfive Watchdog driver > + * > + * Copyright (C) 2022 StarFive Technology Co., Ltd. > + */ > + > +#include <linux/clk.h> > +#include <linux/delay.h> > +#include <linux/err.h> > +#include <linux/interrupt.h> > +#include <linux/io.h> > +#include <linux/iopoll.h> > +#include <linux/module.h> > +#include <linux/moduleparam.h> > +#include <linux/of.h> > +#include <linux/of_device.h> > +#include <linux/platform_device.h> > +#include <linux/pm_runtime.h> > +#include <linux/reset.h> > +#include <linux/reset-controller.h> > +#include <linux/slab.h> > +#include <linux/timer.h> > +#include <linux/types.h> > +#include <linux/uaccess.h> > +#include <linux/watchdog.h> > + > +/* JH7110 WatchDog register define */ > +#define STARFIVE_WDT_JH7110_LOAD 0x000 /* RW: Watchdog load register */ > +#define STARFIVE_WDT_JH7110_VALUE 0x004 /* RO: The current value for the watchdog counter */ > +#define STARFIVE_WDT_JH7110_CONTROL 0x008 /* > + * RW: [0]: reset enable; > + * [1]: int enable/wdt enable/reload counter; > + * [31:2]: res. > + */ > +#define STARFIVE_WDT_JH7110_INTCLR 0x00c /* WO: clear intterupt && reload the counter */ > +#define STARFIVE_WDT_JH7110_RIS 0x010 /* RO: Raw interrupt status from the counter */ > +#define STARFIVE_WDT_JH7110_IMS 0x014 /* RO: Enabled interrupt status from the counter */ > +#define STARFIVE_WDT_JH7110_LOCK 0xc00 /* > + * RO: Enable write access to all other registers > + * by writing 0x1ACCE551. > + */ > +#define STARFIVE_WDT_JH7110_ITCR 0xf00 /* > + * RW: When set HIGH, > + * places the Watchdog into integraeion test mode.\\intetration > + */ > +#define STARFIVE_WDT_JH7110_ITOP 0xf04 /* > + * WO: [0] WDOGRES value Integration Test Mode. > + * Value output on WDOGRES when in > + * Integration Test Mode. > + * [1] Integration Test WDOGINT value. > + * Value output on WDOGINT when in > + * Integration Test Mode. > + */ > + > +#define STARFIVE_WDT_JH7110_UNLOCK_KEY 0x1acce551 > +#define STARFIVE_WDT_JH7110_RESEN_SHIFT 1 > +#define STARFIVE_WDT_JH7110_EN_SHIFT 0 > +#define STARFIVE_WDT_JH7110_INT_EN_SHIFT STARFIVE_WDT_JH7110_EN_SHIFT > + > +/* WDOGCONTROL */ > +#define STARFIVE_WDT_INT_EN 0x0 > +#define STARFIVE_WDT_INT_DIS BIT(0) > +#define STARFIVE_WDT_RESET_EN 0x1 > + > +/* WDOGLOCK */ > +#define STARFIVE_WDT_LOCKED BIT(0) > + > +#define STARFIVE_WDT_INTCLR 0x1 > +#define STARFIVE_WDT_ENABLE 0x1 > +#define STARFIVE_WDT_ATBOOT 0x0 > +#define STARFIVE_WDT_MAXCNT 0xffffffff > + > +#define STARFIVE_WDT_DEFAULT_TIME (15) > +#define STARFIVE_WDT_DELAY_US 0 > +#define STARFIVE_WDT_TIMEOUT_US 10000 > + > +static bool nowayout = WATCHDOG_NOWAYOUT; > +static int tmr_margin; > +static int tmr_atboot = STARFIVE_WDT_ATBOOT; > +static int soft_noboot; > + > +module_param(tmr_margin, int, 0); > +module_param(tmr_atboot, int, 0); > +module_param(nowayout, bool, 0); > +module_param(soft_noboot, int, 0); > + > +MODULE_PARM_DESC(tmr_margin, "Watchdog tmr_margin in seconds. (default=" > + __MODULE_STRING(STARFIVE_WDT_DEFAULT_TIME) ")"); > +MODULE_PARM_DESC(tmr_atboot, > + "Watchdog is started at boot time if set to 1, default=" > + __MODULE_STRING(STARFIVE_WDT_ATBOOT)); > +MODULE_PARM_DESC(nowayout, "Watchdog cannot be stopped once started (default=" > + __MODULE_STRING(WATCHDOG_NOWAYOUT) ")"); > +MODULE_PARM_DESC(soft_noboot, > + "Watchdog action, set to 1 to ignore reboots, 0 to reboot (default 0)"); > + > +struct starfive_wdt_variant_t { > + u32 unlock_key; > + u8 enrst_shift; > + u8 en_shift; > + u8 intclr_check; > + u8 intclr_ava_shift; > +}; > + > +struct starfive_wdt_variant { > + u32 control; > + u32 load; > + u32 enable; > + u32 reload; > + u32 value; > + u32 int_clr; > + u32 int_mask; > + u32 unlock; > + u32 enter_irq_status; > + struct starfive_wdt_variant_t *variant; > +}; > + > +struct starfive_wdt { > + u64 freq; clk_get_rate() returns unsigned long. Why would u64 be needed here ? > + struct device *dev; > + struct watchdog_device wdt_device; > + struct clk *core_clk; > + struct clk *apb_clk; > + struct reset_control *rsts; > + const struct starfive_wdt_variant *drv_data; > + u32 count; /*count of timeout*/ > + u32 reload; /*restore the count*/ > + void __iomem *base; > + spinlock_t lock; /* spinlock for register handling */ > + > + int irq; > +}; > + > +#ifdef CONFIG_OF > +static struct starfive_wdt_variant_t jh7110_variant = { > + .unlock_key = STARFIVE_WDT_JH7110_UNLOCK_KEY, > + .enrst_shift = STARFIVE_WDT_JH7110_RESEN_SHIFT, > + .en_shift = STARFIVE_WDT_JH7110_EN_SHIFT, > +}; > + > +static const struct starfive_wdt_variant drv_data_jh7110 = { > + .control = STARFIVE_WDT_JH7110_CONTROL, > + .load = STARFIVE_WDT_JH7110_LOAD, > + .enable = STARFIVE_WDT_JH7110_CONTROL, > + .value = STARFIVE_WDT_JH7110_VALUE, > + .int_clr = STARFIVE_WDT_JH7110_INTCLR, > + .unlock = STARFIVE_WDT_JH7110_LOCK, > + .enter_irq_status = STARFIVE_WDT_JH7110_IMS, > + .variant = &jh7110_variant, > +}; > + If there is no specific reason for all that complexity, drop it and introduce it if/when necessary. > +static const struct of_device_id starfive_wdt_match[] = { > + { .compatible = "starfive,jh7110-wdt", .data = &drv_data_jh7110 }, > + {} > +}; > +MODULE_DEVICE_TABLE(of, starfive_wdt_match); > +#endif > + > +static const struct platform_device_id starfive_wdt_ids[] = { > + { > + .name = "starfive-wdt", > + .driver_data = (unsigned long)&drv_data_jh7110, > + }, > + {} > +}; > +MODULE_DEVICE_TABLE(platform, starfive_wdt_ids); > + > +static int starfive_wdt_get_clock_rate(struct starfive_wdt *wdt) > +{ > + int ret; > + u32 freq; > + > + if (!IS_ERR(wdt->core_clk)) { > + wdt->freq = clk_get_rate(wdt->core_clk); > + return 0; > + } > + dev_dbg(wdt->dev, "get rate failed, need clock-frequency define in dts.\n"); > + > + /* Next we try to get clock-frequency from dts.*/ > + ret = of_property_read_u32(wdt->dev->of_node, "clock-frequency", &freq); > + if (!ret) { > + wdt->freq = (u64)freq; Unnecessary typecast. > + return 0; > + } > + dev_err(wdt->dev, "get clock-frequency failed\n"); > + > + return -ENOENT; > +} I have never seen this kind of code before. Why would it be needed ? > + > +static int starfive_wdt_enable_clock(struct starfive_wdt *wdt) > +{ > + int ret = 0; > + > + wdt->apb_clk = devm_clk_get(wdt->dev, "apb_clk"); > + if (!IS_ERR(wdt->apb_clk)) { > + ret = clk_prepare_enable(wdt->apb_clk); > + if (ret) { > + dev_err(wdt->dev, "failed to enable apb_clk.\n"); > + goto err; > + } > + } else { > + dev_err(wdt->dev, "failed to get apb_clk.\n"); > + ret = PTR_ERR(wdt->apb_clk); > + goto err; > + } Why not use devm_clk_get_enabled() ? > + > + wdt->core_clk = devm_clk_get(wdt->dev, "core_clk"); > + if (!IS_ERR(wdt->core_clk)) { > + ret = clk_prepare_enable(wdt->core_clk); > + if (ret) { > + dev_err(wdt->dev, "failed to enable core_clk.\n"); > + goto err; > + } > + } else { > + dev_err(wdt->dev, "failed to get core_clk.\n"); > + ret = PTR_ERR(wdt->core_clk); > + goto err; > + } > + > + return 0; > + > +err: > + return ret; Unnecessary. Return directly. > +} > + > +static int starfive_wdt_reset_init(struct starfive_wdt *wdt) > +{ > + int ret = 0; > + > + wdt->rsts = devm_reset_control_array_get_exclusive(wdt->dev); > + if (!IS_ERR(wdt->rsts)) { > + ret = reset_control_deassert(wdt->rsts); > + if (ret) { > + dev_err(wdt->dev, "failed to deassert rsts.\n"); > + goto err; > + } > + } else { > + dev_err(wdt->dev, "failed to get rsts error.\n"); failed ... error is redundant. Drop "errror". > + ret = PTR_ERR(wdt->rsts); > + goto err; > + } > + > + return 0; > + > +err: > + return ret; Unnecessary. Return directly. > +} > + > +static u32 starfive_wdt_ticks_to_sec(struct starfive_wdt *wdt, u32 ticks) > +{ > + return DIV_ROUND_CLOSEST(ticks, wdt->freq); > +} > + > +/* > + * Write unlock-key to unlock. Write other value to lock. When lock bit is 1, > + * external accesses to other watchdog registers are ignored. > + */ > +static int starfive_wdt_is_locked(struct starfive_wdt *wdt) bool ? > +{ > + u32 val; > + > + val = readl(wdt->base + wdt->drv_data->unlock); > + return !!(val & STARFIVE_WDT_LOCKED); > +} > + > +static void starfive_wdt_unlock(struct starfive_wdt *wdt) > +{ > + if (starfive_wdt_is_locked(wdt)) > + writel(wdt->drv_data->variant->unlock_key, > + wdt->base + wdt->drv_data->unlock); > +} > + > +static void starfive_wdt_lock(struct starfive_wdt *wdt) > +{ > + if (!starfive_wdt_is_locked(wdt)) > + writel(~wdt->drv_data->variant->unlock_key, > + wdt->base + wdt->drv_data->unlock); > +} > + > +/* enable watchdog interrupt */ > +static inline void starfive_wdt_int_enable(struct starfive_wdt *wdt) > +{ > + u32 val; > + > + if (wdt->drv_data->int_mask) { > + val = readl(wdt->base + wdt->drv_data->int_mask); > + val &= ~STARFIVE_WDT_INT_DIS; > + writel(val, wdt->base + wdt->drv_data->int_mask); > + } > +} > + > +/* disable watchdog interrupt */ > +static inline void starfive_wdt_int_disable(struct starfive_wdt *wdt) > +{ > + u32 val; > + > + if (wdt->drv_data->int_mask) { > + val = readl(wdt->base + wdt->drv_data->int_mask); > + val |= STARFIVE_WDT_INT_DIS; > + writel(val, wdt->base + wdt->drv_data->int_mask); > + } > +} > + > +/* enable watchdog interrupt to reset */ > +static void starfive_wdt_enable_reset(struct starfive_wdt *wdt) > +{ > + u32 val; > + > + val = readl(wdt->base + wdt->drv_data->control); > + val |= STARFIVE_WDT_RESET_EN << wdt->drv_data->variant->enrst_shift; > + writel(val, wdt->base + wdt->drv_data->control); > +} > + > +/* disable watchdog interrupt to reset */ > +static void starfive_wdt_disable_reset(struct starfive_wdt *wdt) > +{ > + u32 val; > + > + val = readl(wdt->base + wdt->drv_data->control); > + val &= ~(STARFIVE_WDT_RESET_EN << wdt->drv_data->variant->enrst_shift); > + writel(val, wdt->base + wdt->drv_data->control); > +} > + > +/* interrupt status whether has been raised from the counter */ > +static bool starfive_wdt_raise_irq_status(struct starfive_wdt *wdt) > +{ > + return !!readl(wdt->base + wdt->drv_data->enter_irq_status); > +} > + > +static void starfive_wdt_int_clr(struct starfive_wdt *wdt) > +{ > + void __iomem *addr; > + u8 clr_check; > + u8 clr_ava_shift; > + u32 value; > + int ret = 0; > + > + addr = wdt->base + wdt->drv_data->int_clr; > + clr_ava_shift = wdt->drv_data->variant->intclr_ava_shift; > + clr_check = wdt->drv_data->variant->intclr_check; > + if (clr_check) { > + /* waiting interrupt can be to clearing */ > + value = readl(addr); > + ret = readl_poll_timeout_atomic(addr, value, > + !(value & BIT(clr_ava_shift)), > + STARFIVE_WDT_DELAY_US, STARFIVE_WDT_TIMEOUT_US); > + } > + > + if (!ret) > + writel(STARFIVE_WDT_INTCLR, addr); > + > + if (starfive_wdt_raise_irq_status(wdt)) > + enable_irq(wdt->irq); > +} > + > +static inline void starfive_wdt_set_count(struct starfive_wdt *wdt, u32 val) > +{ > + writel(val, wdt->base + wdt->drv_data->load); > +} > + > +static inline u32 starfive_wdt_get_count(struct starfive_wdt *wdt) > +{ > + return readl(wdt->base + wdt->drv_data->value); > +} > + > +static inline void starfive_wdt_enable(struct starfive_wdt *wdt) > +{ > + u32 val; > + > + val = readl(wdt->base + wdt->drv_data->enable); > + val |= STARFIVE_WDT_ENABLE << wdt->drv_data->variant->en_shift; > + writel(val, wdt->base + wdt->drv_data->enable); > +} > + > +static inline void starfive_wdt_disable(struct starfive_wdt *wdt) > +{ > + u32 val; > + > + val = readl(wdt->base + wdt->drv_data->enable); > + val &= ~(STARFIVE_WDT_ENABLE << wdt->drv_data->variant->en_shift); > + writel(val, wdt->base + wdt->drv_data->enable); > +} > + > +static inline void starfive_wdt_set_relod_count(struct starfive_wdt *wdt, u32 count) > +{ > + writel(count, wdt->base + wdt->drv_data->load); > + if (wdt->drv_data->reload) > + writel(0x1, wdt->base + wdt->drv_data->reload); > + else > + /* jh7110 need enable controller to reload counter */ > + starfive_wdt_enable(wdt); > +} > + > +static void starfive_wdt_mask_and_disable_reset(struct starfive_wdt *wdt, bool mask) > +{ > + starfive_wdt_unlock(wdt); > + > + if (mask) > + starfive_wdt_disable_reset(wdt); > + else > + starfive_wdt_enable_reset(wdt); > + > + starfive_wdt_lock(wdt); > +} > + > +static unsigned int starfive_wdt_max_timeout(struct starfive_wdt *wdt) > +{ > + return DIV_ROUND_UP(STARFIVE_WDT_MAXCNT, wdt->freq) - 1; > +} > + > +static unsigned int starfive_wdt_get_timeleft(struct watchdog_device *wdd) > +{ > + struct starfive_wdt *wdt = watchdog_get_drvdata(wdd); > + u32 count; > + u32 irq_status; > + > + starfive_wdt_unlock(wdt); > + /* > + * Because set half count value, > + * timeleft value should add the count value before first timeout. > + */ > + irq_status = starfive_wdt_raise_irq_status(wdt) ? 1 : 0; > + count = starfive_wdt_get_count(wdt) + (1 - irq_status) * wdt->count; > + starfive_wdt_lock(wdt); > + > + return starfive_wdt_ticks_to_sec(wdt, count); > +} > + > +static int starfive_wdt_keepalive(struct watchdog_device *wdd) > +{ > + struct starfive_wdt *wdt = watchdog_get_drvdata(wdd); > + > + spin_lock(&wdt->lock); > + > + starfive_wdt_unlock(wdt); > + starfive_wdt_int_clr(wdt); > + starfive_wdt_set_relod_count(wdt, wdt->count); > + starfive_wdt_lock(wdt); > + > + spin_unlock(&wdt->lock); > + > + return 0; > +} > + > +static irqreturn_t starfive_wdt_interrupt_handler(int irq, void *data) > +{ > + /* > + * We don't clear the IRQ status. It's supposed to be done by the > + * following ping operations. > + */ > + struct platform_device *pdev = data; > + struct starfive_wdt *wdt = platform_get_drvdata(pdev); > + > + /* Disable the IRQ and avoid re-entry interrupt. */ > + disable_irq_nosync(wdt->irq); > + > + return IRQ_HANDLED; > +} What is the purpose of this interrupt ? It doesn't add any value as implemented. > + > +static int starfive_wdt_stop(struct watchdog_device *wdd) > +{ > + struct starfive_wdt *wdt = watchdog_get_drvdata(wdd); > + > + spin_lock(&wdt->lock); > + > + starfive_wdt_unlock(wdt); > + starfive_wdt_int_disable(wdt); > + starfive_wdt_int_clr(wdt); > + starfive_wdt_disable(wdt); > + starfive_wdt_lock(wdt); > + > + spin_unlock(&wdt->lock); > + > + return 0; > +} > + > +static int starfive_wdt_pm_stop(struct watchdog_device *wdd) > +{ > + struct starfive_wdt *wdt = watchdog_get_drvdata(wdd); > + > + starfive_wdt_stop(wdd); > + pm_runtime_put(wdt->dev); > + > + return 0; > +} > + > +static int starfive_wdt_start(struct watchdog_device *wdd) > +{ > + struct starfive_wdt *wdt = watchdog_get_drvdata(wdd); > + > + pm_runtime_get_sync(wdt->dev); > + > + spin_lock(&wdt->lock); > + > + starfive_wdt_unlock(wdt); > + > + if (soft_noboot) > + starfive_wdt_disable_reset(wdt); > + else > + starfive_wdt_enable_reset(wdt); > + > + starfive_wdt_int_clr(wdt); > + starfive_wdt_set_count(wdt, wdt->count); > + starfive_wdt_int_enable(wdt); > + starfive_wdt_enable(wdt); > + > + starfive_wdt_lock(wdt); > + > + spin_unlock(&wdt->lock); > + > + return 0; > +} > + > +static int starfive_wdt_restart(struct watchdog_device *wdd, unsigned long action, > + void *data) > +{ > + struct starfive_wdt *wdt = watchdog_get_drvdata(wdd); > + > + spin_lock(&wdt->lock); > + starfive_wdt_unlock(wdt); > + /* disable watchdog, to be safe */ > + starfive_wdt_disable(wdt); > + > + if (soft_noboot) > + starfive_wdt_disable_reset(wdt); > + else > + starfive_wdt_enable_reset(wdt); > + > + /* put initial values into count and data */ > + starfive_wdt_set_count(wdt, wdt->count); This is supposed to reset the system. Why write wdt->count instead of 0 or 1 ? > + > + /* set the watchdog to go and reset... */ > + starfive_wdt_int_clr(wdt); > + starfive_wdt_int_enable(wdt); > + starfive_wdt_enable(wdt); > + > + starfive_wdt_lock(wdt); > + spin_unlock(&wdt->lock); > + > + return 0; > +} > + > +static int starfive_wdt_set_timeout(struct watchdog_device *wdd, > + unsigned int timeout) > +{ > + struct starfive_wdt *wdt = watchdog_get_drvdata(wdd); > + > + unsigned long freq = wdt->freq; > + unsigned int count; > + > + spin_lock(&wdt->lock); > + > + if (timeout < 1) > + return -EINVAL; Can not happen if configured properly, ie if min_timeout is set *which it is). > + > + /* > + * This watchdog takes twice timeouts to reset. > + * In order to reduce time to reset, should set half count value. > + */ > + count = timeout * freq / 2; > + > + if (count > STARFIVE_WDT_MAXCNT) { > + dev_warn(wdt->dev, "timeout %d too big,use the MAX-timeout set.\n", > + timeout); > + timeout = starfive_wdt_max_timeout(wdt); > + count = timeout * freq; > + } > + > + dev_info(wdt->dev, "Heartbeat: timeout=%d, count/2=%d (%08x)\n", > + timeout, count, count); > + > + starfive_wdt_unlock(wdt); > + starfive_wdt_disable(wdt); > + starfive_wdt_set_relod_count(wdt, count); > + starfive_wdt_enable(wdt); > + starfive_wdt_lock(wdt); > + > + wdt->count = count; > + wdd->timeout = timeout; > + > + spin_unlock(&wdt->lock); > + > + return 0; > +} > + > +#define OPTIONS (WDIOF_SETTIMEOUT | WDIOF_KEEPALIVEPING | WDIOF_MAGICCLOSE) > + > +static const struct watchdog_info starfive_wdt_ident = { > + .options = OPTIONS, > + .firmware_version = 0, > + .identity = "StarFive Watchdog", > +}; > + > +static const struct watchdog_ops starfive_wdt_ops = { > + .owner = THIS_MODULE, > + .start = starfive_wdt_start, > + .stop = starfive_wdt_pm_stop, > + .ping = starfive_wdt_keepalive, > + .set_timeout = starfive_wdt_set_timeout, > + .restart = starfive_wdt_restart, > + .get_timeleft = starfive_wdt_get_timeleft, > +}; > + > +static const struct watchdog_device starfive_wdd = { > + .info = &starfive_wdt_ident, > + .ops = &starfive_wdt_ops, > + .timeout = STARFIVE_WDT_DEFAULT_TIME, > +}; > + > +static inline const struct starfive_wdt_variant * > +starfive_wdt_get_drv_data(struct platform_device *pdev) > +{ > + const struct starfive_wdt_variant *variant; > + > + variant = of_device_get_match_data(&pdev->dev); > + if (!variant) { > + /* Device matched by platform_device_id */ > + variant = (struct starfive_wdt_variant *) > + platform_get_device_id(pdev)->driver_data; > + } > + > + return variant; > +} > + > +static int starfive_wdt_probe(struct platform_device *pdev) > +{ > + struct device *dev = &pdev->dev; > + struct starfive_wdt *wdt; > + int started = 0; > + int ret; > + > + pm_runtime_enable(dev); > + > + wdt = devm_kzalloc(dev, sizeof(*wdt), GFP_KERNEL); > + if (!wdt) > + return -ENOMEM; > + > + wdt->dev = dev; > + spin_lock_init(&wdt->lock); > + wdt->wdt_device = starfive_wdd; > + > + wdt->drv_data = starfive_wdt_get_drv_data(pdev); > + > + wdt->irq = platform_get_irq(pdev, 0); > + if (wdt->irq < 0) { > + dev_err(dev, "can not find irq.\n"); > + return wdt->irq; > + } > + > + /* get the memory region for the watchdog timer */ > + wdt->base = devm_platform_ioremap_resource(pdev, 0); > + if (IS_ERR(wdt->base)) { > + ret = PTR_ERR(wdt->base); > + goto err; > + } > + > + ret = starfive_wdt_enable_clock(wdt); > + if (ret) > + goto err; > + > + starfive_wdt_get_clock_rate(wdt); > + This can fail, leaving the clock rate uninitialized. > + ret = starfive_wdt_reset_init(wdt); > + if (ret) > + goto err; > + > + wdt->wdt_device.min_timeout = 1; > + wdt->wdt_device.max_timeout = starfive_wdt_max_timeout(wdt); > + > + watchdog_set_drvdata(&wdt->wdt_device, wdt); > + > + /* > + * see if we can actually set the requested timer margin, > + * and if not, try the default value. > + */ > + watchdog_init_timeout(&wdt->wdt_device, tmr_margin, dev); > + > + ret = starfive_wdt_set_timeout(&wdt->wdt_device, > + wdt->wdt_device.timeout); > + if (ret) { > + dev_err(dev, "tmr_margin value out of range, default %d used\n", > + STARFIVE_WDT_DEFAULT_TIME); > + starfive_wdt_set_timeout(&wdt->wdt_device, > + STARFIVE_WDT_DEFAULT_TIME); > + } > + > + ret = devm_request_irq(dev, wdt->irq, starfive_wdt_interrupt_handler, 0, > + pdev->name, pdev); > + if (ret != 0) { > + dev_err(dev, "failed to install irq (%d)\n", ret); > + goto err; > + } > + That interrupt seems pointless. > + watchdog_set_nowayout(&wdt->wdt_device, nowayout); > + watchdog_set_restart_priority(&wdt->wdt_device, 128); > + > + wdt->wdt_device.parent = dev; > + > + ret = watchdog_register_device(&wdt->wdt_device); > + if (ret) > + goto err; > + > + starfive_wdt_mask_and_disable_reset(wdt, false); > + > + if (tmr_atboot && started == 0) { > + starfive_wdt_start(&wdt->wdt_device); > + } else if (!tmr_atboot) { > + /* > + *if we're not enabling the watchdog, then ensure it is > + * disabled if it has been left running from the bootloader > + * or other source. > + */ > + starfive_wdt_stop(&wdt->wdt_device); > + } This should be done before registering the watdhdog, and the watchdog core should be informed if the watchdog is running. > + > +#ifdef CONFIG_PM > + clk_disable_unprepare(wdt->core_clk); > + clk_disable_unprepare(wdt->apb_clk); > +#endif > + > + platform_set_drvdata(pdev, wdt); > + > + return 0; > + > +err: > + return ret; This is never acceptable. Return directly. > +} > + > +static int starfive_wdt_remove(struct platform_device *dev) > +{ > + struct starfive_wdt *wdt = platform_get_drvdata(dev); > + > + starfive_wdt_mask_and_disable_reset(wdt, true); > + watchdog_unregister_device(&wdt->wdt_device); > + > + clk_disable_unprepare(wdt->core_clk); > + clk_disable_unprepare(wdt->apb_clk); > + pm_runtime_disable(wdt->dev); This is potentially unbalanced. I would suggest to use watchdog_stop_on_unregister(). > + > + return 0; > +} > + > +static void starfive_wdt_shutdown(struct platform_device *dev) > +{ > + struct starfive_wdt *wdt = platform_get_drvdata(dev); > + > + starfive_wdt_mask_and_disable_reset(wdt, true); > + > + starfive_wdt_pm_stop(&wdt->wdt_device); Use watchdog_stop_on_reboot(). > +} > + > +#ifdef CONFIG_PM_SLEEP > +static int starfive_wdt_suspend(struct device *dev) > +{ > + int ret; > + struct starfive_wdt *wdt = dev_get_drvdata(dev); > + > + starfive_wdt_unlock(wdt); > + > + /* Save watchdog state, and turn it off. */ > + wdt->reload = starfive_wdt_get_count(wdt); > + > + starfive_wdt_mask_and_disable_reset(wdt, true); > + > + /* Note that WTCNT doesn't need to be saved. */ > + starfive_wdt_stop(&wdt->wdt_device); > + pm_runtime_force_suspend(dev); > + > + starfive_wdt_lock(wdt); > + > + return 0; > +} > + > +static int starfive_wdt_resume(struct device *dev) > +{ > + int ret; > + struct starfive_wdt *wdt = dev_get_drvdata(dev); > + > + starfive_wdt_unlock(wdt); > + > + pm_runtime_force_resume(dev); > + > + /* Restore watchdog state. */ > + starfive_wdt_set_relod_count(wdt, wdt->reload); > + > + starfive_wdt_restart(&wdt->wdt_device, 0, NULL); > + > + starfive_wdt_mask_and_disable_reset(wdt, false); > + > + starfive_wdt_lock(wdt); > + > + return 0; > +} > +#endif /* CONFIG_PM_SLEEP */ > + > +#ifdef CONFIG_PM > +static int starfive_wdt_runtime_suspend(struct device *dev) > +{ > + struct starfive_wdt *wdt = dev_get_drvdata(dev); > + > + clk_disable_unprepare(wdt->apb_clk); > + clk_disable_unprepare(wdt->core_clk); > + > + return 0; > +} > + > +static int starfive_wdt_runtime_resume(struct device *dev) > +{ > + struct starfive_wdt *wdt = dev_get_drvdata(dev); > + int ret; > + > + ret = clk_prepare_enable(wdt->apb_clk); > + if (ret) { > + dev_err(wdt->dev, "failed to enable apb_clk.\n"); > + goto err; > + } > + > + ret = clk_prepare_enable(wdt->core_clk); > + if (ret) { > + dev_err(wdt->dev, "failed to enable core_clk.\n"); > + goto err; > + } > + > + return 0; > + > +err: > + return ret; > +} > +#endif /* CONFIG_PM */ > + > +static const struct dev_pm_ops starfive_wdt_pm_ops = { > + SET_RUNTIME_PM_OPS(starfive_wdt_runtime_suspend, starfive_wdt_runtime_resume, NULL) > + SET_SYSTEM_SLEEP_PM_OPS(starfive_wdt_suspend, starfive_wdt_resume) > +}; > + > +static struct platform_driver starfive_wdt_driver = { > + .probe = starfive_wdt_probe, > + .remove = starfive_wdt_remove, > + .shutdown = starfive_wdt_shutdown, > + .id_table = starfive_wdt_ids, > + .driver = { > + .name = "starfive-wdt", > + .pm = &starfive_wdt_pm_ops, > + .of_match_table = of_match_ptr(starfive_wdt_match), > + }, > +}; > + > +module_platform_driver(starfive_wdt_driver); > + > +MODULE_AUTHOR("Xingyu Wu <xingyu.wu@starfivetech.com>"); > +MODULE_AUTHOR("Samin Guo <samin.guo@starfivetech.com>"); > +MODULE_DESCRIPTION("StarFive Watchdog Device Driver"); > +MODULE_LICENSE("GPL v2"); _______________________________________________ linux-riscv mailing list linux-riscv@lists.infradead.org http://lists.infradead.org/mailman/listinfo/linux-riscv ^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: [PATCH v1 2/3] drivers: watchdog: Add StarFive Watchdog driver 2022-12-02 15:08 ` Guenter Roeck @ 2022-12-05 8:08 ` Xingyu Wu 0 siblings, 0 replies; 13+ messages in thread From: Xingyu Wu @ 2022-12-05 8:08 UTC (permalink / raw) To: Guenter Roeck Cc: Rob Herring, Krzysztof Kozlowski, Paul Walmsley, Palmer Dabbelt, Albert Ou, Philipp Zabel, Samin Guo, linux-kernel, linux-riscv, linux-watchdog, Wim Van Sebroeck, devicetree On 2022/12/2 23:08, Guenter Roeck wrote: > On 12/2/22 01:39, xingu.wu wrote: >> From: Xingyu Wu <xingyu.wu@starfivetech.com> >> >> Add watchdog driver for the StarFive JH7110 SoC. >> >> Signed-off-by: Xingyu Wu <xingyu.wu@starfivetech.com> >> --- >> MAINTAINERS | 7 + >> drivers/watchdog/Kconfig | 11 + >> drivers/watchdog/Makefile | 3 + >> drivers/watchdog/starfive-wdt.c | 854 ++++++++++++++++++++++++++++++++ >> 4 files changed, 875 insertions(+) >> create mode 100644 drivers/watchdog/starfive-wdt.c >> >> diff --git a/MAINTAINERS b/MAINTAINERS >> index a70c1d0f303e..133214dd2f60 100644 >> --- a/MAINTAINERS >> +++ b/MAINTAINERS >> @@ -19623,6 +19623,13 @@ F: Documentation/devicetree/bindings/reset/starfive,jh7100-reset.yaml >> F: drivers/reset/starfive/ >> F: include/dt-bindings/reset/starfive* >> +STARFIVE WATCHDOG DRIVER >> +M: Xingyu Wu <xingyu.wu@starfivetech.com> >> +M: Samin Guo <samin.guo@starfivetech.com> >> +S: Supported >> +F: Documentation/devicetree/bindings/watchdog/starfive* >> +F: drivers/watchdog/starfive-wdt.c >> + >> STATIC BRANCH/CALL >> M: Peter Zijlstra <peterz@infradead.org> >> M: Josh Poimboeuf <jpoimboe@kernel.org> >> diff --git a/drivers/watchdog/Kconfig b/drivers/watchdog/Kconfig >> index b64bc49c7f30..658fc9fef4fa 100644 >> --- a/drivers/watchdog/Kconfig >> +++ b/drivers/watchdog/Kconfig >> @@ -2082,6 +2082,17 @@ config UML_WATCHDOG >> tristate "UML watchdog" >> depends on UML || COMPILE_TEST >> +# RISCV Architecture >> + >> +config STARFIVE_WATCHDOG >> + tristate "StarFive Watchdog support" >> + depends on RISCV >> + select WATCHDOG_CORE >> + default SOC_STARFIVE >> + help >> + Say Y here to support the StarFive watchdog. This driver can also >> + be built as a module if choose M. >> + >> # >> # ISA-based Watchdog Cards >> # >> diff --git a/drivers/watchdog/Makefile b/drivers/watchdog/Makefile >> index d41e5f830ae7..9c22f1a43d5c 100644 >> --- a/drivers/watchdog/Makefile >> +++ b/drivers/watchdog/Makefile >> @@ -210,6 +210,9 @@ obj-$(CONFIG_WATCHDOG_SUN4V) += sun4v_wdt.o >> # Xen >> obj-$(CONFIG_XEN_WDT) += xen_wdt.o >> +# RISCV Architecture >> +obj-$(CONFIG_STARFIVE_WATCHDOG) += starfive-wdt.o >> + >> # Architecture Independent >> obj-$(CONFIG_BD957XMUF_WATCHDOG) += bd9576_wdt.o >> obj-$(CONFIG_DA9052_WATCHDOG) += da9052_wdt.o >> diff --git a/drivers/watchdog/starfive-wdt.c b/drivers/watchdog/starfive-wdt.c >> new file mode 100644 >> index 000000000000..dcb6e693ba6c >> --- /dev/null >> +++ b/drivers/watchdog/starfive-wdt.c >> @@ -0,0 +1,854 @@ >> +// SPDX-License-Identifier: GPL-2.0 >> +/* >> + * Starfive Watchdog driver >> + * >> + * Copyright (C) 2022 StarFive Technology Co., Ltd. >> + */ >> + >> +#include <linux/clk.h> >> +#include <linux/delay.h> >> +#include <linux/err.h> >> +#include <linux/interrupt.h> >> +#include <linux/io.h> >> +#include <linux/iopoll.h> >> +#include <linux/module.h> >> +#include <linux/moduleparam.h> >> +#include <linux/of.h> >> +#include <linux/of_device.h> >> +#include <linux/platform_device.h> >> +#include <linux/pm_runtime.h> >> +#include <linux/reset.h> >> +#include <linux/reset-controller.h> >> +#include <linux/slab.h> >> +#include <linux/timer.h> >> +#include <linux/types.h> >> +#include <linux/uaccess.h> >> +#include <linux/watchdog.h> >> + >> +/* JH7110 WatchDog register define */ >> +#define STARFIVE_WDT_JH7110_LOAD 0x000 /* RW: Watchdog load register */ >> +#define STARFIVE_WDT_JH7110_VALUE 0x004 /* RO: The current value for the watchdog counter */ >> +#define STARFIVE_WDT_JH7110_CONTROL 0x008 /* >> + * RW: [0]: reset enable; >> + * [1]: int enable/wdt enable/reload counter; >> + * [31:2]: res. >> + */ >> +#define STARFIVE_WDT_JH7110_INTCLR 0x00c /* WO: clear intterupt && reload the counter */ >> +#define STARFIVE_WDT_JH7110_RIS 0x010 /* RO: Raw interrupt status from the counter */ >> +#define STARFIVE_WDT_JH7110_IMS 0x014 /* RO: Enabled interrupt status from the counter */ >> +#define STARFIVE_WDT_JH7110_LOCK 0xc00 /* >> + * RO: Enable write access to all other registers >> + * by writing 0x1ACCE551. >> + */ >> +#define STARFIVE_WDT_JH7110_ITCR 0xf00 /* >> + * RW: When set HIGH, >> + * places the Watchdog into integraeion test mode.\\intetration > >> + */ >> +#define STARFIVE_WDT_JH7110_ITOP 0xf04 /* >> + * WO: [0] WDOGRES value Integration Test Mode. >> + * Value output on WDOGRES when in >> + * Integration Test Mode. >> + * [1] Integration Test WDOGINT value. >> + * Value output on WDOGINT when in >> + * Integration Test Mode. >> + */ >> + >> +#define STARFIVE_WDT_JH7110_UNLOCK_KEY 0x1acce551 >> +#define STARFIVE_WDT_JH7110_RESEN_SHIFT 1 >> +#define STARFIVE_WDT_JH7110_EN_SHIFT 0 >> +#define STARFIVE_WDT_JH7110_INT_EN_SHIFT STARFIVE_WDT_JH7110_EN_SHIFT >> + >> +/* WDOGCONTROL */ >> +#define STARFIVE_WDT_INT_EN 0x0 >> +#define STARFIVE_WDT_INT_DIS BIT(0) >> +#define STARFIVE_WDT_RESET_EN 0x1 >> + >> +/* WDOGLOCK */ >> +#define STARFIVE_WDT_LOCKED BIT(0) >> + >> +#define STARFIVE_WDT_INTCLR 0x1 >> +#define STARFIVE_WDT_ENABLE 0x1 >> +#define STARFIVE_WDT_ATBOOT 0x0 >> +#define STARFIVE_WDT_MAXCNT 0xffffffff >> + >> +#define STARFIVE_WDT_DEFAULT_TIME (15) >> +#define STARFIVE_WDT_DELAY_US 0 >> +#define STARFIVE_WDT_TIMEOUT_US 10000 >> + >> +static bool nowayout = WATCHDOG_NOWAYOUT; >> +static int tmr_margin; >> +static int tmr_atboot = STARFIVE_WDT_ATBOOT; >> +static int soft_noboot; >> + >> +module_param(tmr_margin, int, 0); >> +module_param(tmr_atboot, int, 0); >> +module_param(nowayout, bool, 0); >> +module_param(soft_noboot, int, 0); >> + >> +MODULE_PARM_DESC(tmr_margin, "Watchdog tmr_margin in seconds. (default=" >> + __MODULE_STRING(STARFIVE_WDT_DEFAULT_TIME) ")"); >> +MODULE_PARM_DESC(tmr_atboot, >> + "Watchdog is started at boot time if set to 1, default=" >> + __MODULE_STRING(STARFIVE_WDT_ATBOOT)); >> +MODULE_PARM_DESC(nowayout, "Watchdog cannot be stopped once started (default=" >> + __MODULE_STRING(WATCHDOG_NOWAYOUT) ")"); >> +MODULE_PARM_DESC(soft_noboot, >> + "Watchdog action, set to 1 to ignore reboots, 0 to reboot (default 0)"); >> + >> +struct starfive_wdt_variant_t { >> + u32 unlock_key; >> + u8 enrst_shift; >> + u8 en_shift; >> + u8 intclr_check; >> + u8 intclr_ava_shift; >> +}; >> + >> +struct starfive_wdt_variant { >> + u32 control; >> + u32 load; >> + u32 enable; >> + u32 reload; >> + u32 value; >> + u32 int_clr; >> + u32 int_mask; >> + u32 unlock; >> + u32 enter_irq_status; >> + struct starfive_wdt_variant_t *variant; >> +}; >> + >> +struct starfive_wdt { >> + u64 freq; > > clk_get_rate() returns unsigned long. Why would u64 be needed here ? Will fix and use unsigned long instead of u64. > >> + struct device *dev; >> + struct watchdog_device wdt_device; >> + struct clk *core_clk; >> + struct clk *apb_clk; >> + struct reset_control *rsts; >> + const struct starfive_wdt_variant *drv_data; >> + u32 count; /*count of timeout*/ >> + u32 reload; /*restore the count*/ >> + void __iomem *base; >> + spinlock_t lock; /* spinlock for register handling */ >> + >> + int irq; >> +}; >> + >> +#ifdef CONFIG_OF >> +static struct starfive_wdt_variant_t jh7110_variant = { >> + .unlock_key = STARFIVE_WDT_JH7110_UNLOCK_KEY, >> + .enrst_shift = STARFIVE_WDT_JH7110_RESEN_SHIFT, >> + .en_shift = STARFIVE_WDT_JH7110_EN_SHIFT, >> +}; >> + >> +static const struct starfive_wdt_variant drv_data_jh7110 = { >> + .control = STARFIVE_WDT_JH7110_CONTROL, >> + .load = STARFIVE_WDT_JH7110_LOAD, >> + .enable = STARFIVE_WDT_JH7110_CONTROL, >> + .value = STARFIVE_WDT_JH7110_VALUE, >> + .int_clr = STARFIVE_WDT_JH7110_INTCLR, >> + .unlock = STARFIVE_WDT_JH7110_LOCK, >> + .enter_irq_status = STARFIVE_WDT_JH7110_IMS, >> + .variant = &jh7110_variant, >> +}; >> + > > If there is no specific reason for all that complexity, drop it and introduce > it if/when necessary. We have some other SoCs like JH7100 would use this watchdog driver, and use this struct to distinguish the register addresses of different SoCs. > >> +static const struct of_device_id starfive_wdt_match[] = { >> + { .compatible = "starfive,jh7110-wdt", .data = &drv_data_jh7110 }, >> + {} >> +}; >> +MODULE_DEVICE_TABLE(of, starfive_wdt_match); >> +#endif >> + >> +static const struct platform_device_id starfive_wdt_ids[] = { >> + { >> + .name = "starfive-wdt", >> + .driver_data = (unsigned long)&drv_data_jh7110, >> + }, >> + {} >> +}; >> +MODULE_DEVICE_TABLE(platform, starfive_wdt_ids); >> + >> +static int starfive_wdt_get_clock_rate(struct starfive_wdt *wdt) >> +{ >> + int ret; >> + u32 freq; >> + >> + if (!IS_ERR(wdt->core_clk)) { >> + wdt->freq = clk_get_rate(wdt->core_clk); >> + return 0; >> + } >> + dev_dbg(wdt->dev, "get rate failed, need clock-frequency define in dts.\n"); >> + >> + /* Next we try to get clock-frequency from dts.*/ >> + ret = of_property_read_u32(wdt->dev->of_node, "clock-frequency", &freq); >> + if (!ret) { >> + wdt->freq = (u64)freq; > > Unnecessary typecast. Will fix. > >> + return 0; >> + } >> + dev_err(wdt->dev, "get clock-frequency failed\n"); >> + >> + return -ENOENT; >> +} > > I have never seen this kind of code before. Why would it be needed ? I will drop it and only get clock rate by clk_get_rate(). > >> + >> +static int starfive_wdt_enable_clock(struct starfive_wdt *wdt) >> +{ >> + int ret = 0; >> + >> + wdt->apb_clk = devm_clk_get(wdt->dev, "apb_clk"); >> + if (!IS_ERR(wdt->apb_clk)) { >> + ret = clk_prepare_enable(wdt->apb_clk); >> + if (ret) { >> + dev_err(wdt->dev, "failed to enable apb_clk.\n"); >> + goto err; >> + } >> + } else { >> + dev_err(wdt->dev, "failed to get apb_clk.\n"); >> + ret = PTR_ERR(wdt->apb_clk); >> + goto err; >> + } > > Why not use devm_clk_get_enabled() ? Thanks for your advice. This driver was used in earlier Linux version before and I haven't seen this devm_clk_get_enabled(). Now I will try to use it. > >> + >> + wdt->core_clk = devm_clk_get(wdt->dev, "core_clk"); >> + if (!IS_ERR(wdt->core_clk)) { >> + ret = clk_prepare_enable(wdt->core_clk); >> + if (ret) { >> + dev_err(wdt->dev, "failed to enable core_clk.\n"); >> + goto err; >> + } >> + } else { >> + dev_err(wdt->dev, "failed to get core_clk.\n"); >> + ret = PTR_ERR(wdt->core_clk); >> + goto err; >> + } >> + >> + return 0; >> + >> +err: >> + return ret; > > Unnecessary. Return directly. Will fix. > >> +} >> + >> +static int starfive_wdt_reset_init(struct starfive_wdt *wdt) >> +{ >> + int ret = 0; >> + >> + wdt->rsts = devm_reset_control_array_get_exclusive(wdt->dev); >> + if (!IS_ERR(wdt->rsts)) { >> + ret = reset_control_deassert(wdt->rsts); >> + if (ret) { >> + dev_err(wdt->dev, "failed to deassert rsts.\n"); >> + goto err; >> + } >> + } else { >> + dev_err(wdt->dev, "failed to get rsts error.\n"); > > failed ... error is redundant. Drop "errror". Will fix. > >> + ret = PTR_ERR(wdt->rsts); >> + goto err; >> + } >> + >> + return 0; >> + >> +err: >> + return ret; > > Unnecessary. Return directly. Will fix. > >> +} >> + >> +static u32 starfive_wdt_ticks_to_sec(struct starfive_wdt *wdt, u32 ticks) >> +{ >> + return DIV_ROUND_CLOSEST(ticks, wdt->freq); >> +} >> + >> +/* >> + * Write unlock-key to unlock. Write other value to lock. When lock bit is 1, >> + * external accesses to other watchdog registers are ignored. >> + */ >> +static int starfive_wdt_is_locked(struct starfive_wdt *wdt) > > bool ? Will use bool better. > >> +{ >> + u32 val; >> + >> + val = readl(wdt->base + wdt->drv_data->unlock); >> + return !!(val & STARFIVE_WDT_LOCKED); >> +} >> + >> +static void starfive_wdt_unlock(struct starfive_wdt *wdt) >> +{ >> + if (starfive_wdt_is_locked(wdt)) >> + writel(wdt->drv_data->variant->unlock_key, >> + wdt->base + wdt->drv_data->unlock); >> +} >> + >> +static void starfive_wdt_lock(struct starfive_wdt *wdt) >> +{ >> + if (!starfive_wdt_is_locked(wdt)) >> + writel(~wdt->drv_data->variant->unlock_key, >> + wdt->base + wdt->drv_data->unlock); >> +} >> + >> +/* enable watchdog interrupt */ >> +static inline void starfive_wdt_int_enable(struct starfive_wdt *wdt) >> +{ >> + u32 val; >> + >> + if (wdt->drv_data->int_mask) { >> + val = readl(wdt->base + wdt->drv_data->int_mask); >> + val &= ~STARFIVE_WDT_INT_DIS; >> + writel(val, wdt->base + wdt->drv_data->int_mask); >> + } >> +} >> + >> +/* disable watchdog interrupt */ >> +static inline void starfive_wdt_int_disable(struct starfive_wdt *wdt) >> +{ >> + u32 val; >> + >> + if (wdt->drv_data->int_mask) { >> + val = readl(wdt->base + wdt->drv_data->int_mask); >> + val |= STARFIVE_WDT_INT_DIS; >> + writel(val, wdt->base + wdt->drv_data->int_mask); >> + } >> +} >> + >> +/* enable watchdog interrupt to reset */ >> +static void starfive_wdt_enable_reset(struct starfive_wdt *wdt) >> +{ >> + u32 val; >> + >> + val = readl(wdt->base + wdt->drv_data->control); >> + val |= STARFIVE_WDT_RESET_EN << wdt->drv_data->variant->enrst_shift; >> + writel(val, wdt->base + wdt->drv_data->control); >> +} >> + >> +/* disable watchdog interrupt to reset */ >> +static void starfive_wdt_disable_reset(struct starfive_wdt *wdt) >> +{ >> + u32 val; >> + >> + val = readl(wdt->base + wdt->drv_data->control); >> + val &= ~(STARFIVE_WDT_RESET_EN << wdt->drv_data->variant->enrst_shift); >> + writel(val, wdt->base + wdt->drv_data->control); >> +} >> + >> +/* interrupt status whether has been raised from the counter */ >> +static bool starfive_wdt_raise_irq_status(struct starfive_wdt *wdt) >> +{ >> + return !!readl(wdt->base + wdt->drv_data->enter_irq_status); >> +} >> + >> +static void starfive_wdt_int_clr(struct starfive_wdt *wdt) >> +{ >> + void __iomem *addr; >> + u8 clr_check; >> + u8 clr_ava_shift; >> + u32 value; >> + int ret = 0; >> + >> + addr = wdt->base + wdt->drv_data->int_clr; >> + clr_ava_shift = wdt->drv_data->variant->intclr_ava_shift; >> + clr_check = wdt->drv_data->variant->intclr_check; >> + if (clr_check) { >> + /* waiting interrupt can be to clearing */ >> + value = readl(addr); >> + ret = readl_poll_timeout_atomic(addr, value, >> + !(value & BIT(clr_ava_shift)), >> + STARFIVE_WDT_DELAY_US, STARFIVE_WDT_TIMEOUT_US); >> + } >> + >> + if (!ret) >> + writel(STARFIVE_WDT_INTCLR, addr); >> + >> + if (starfive_wdt_raise_irq_status(wdt)) >> + enable_irq(wdt->irq); >> +} >> + >> +static inline void starfive_wdt_set_count(struct starfive_wdt *wdt, u32 val) >> +{ >> + writel(val, wdt->base + wdt->drv_data->load); >> +} >> + >> +static inline u32 starfive_wdt_get_count(struct starfive_wdt *wdt) >> +{ >> + return readl(wdt->base + wdt->drv_data->value); >> +} >> + >> +static inline void starfive_wdt_enable(struct starfive_wdt *wdt) >> +{ >> + u32 val; >> + >> + val = readl(wdt->base + wdt->drv_data->enable); >> + val |= STARFIVE_WDT_ENABLE << wdt->drv_data->variant->en_shift; >> + writel(val, wdt->base + wdt->drv_data->enable); >> +} >> + >> +static inline void starfive_wdt_disable(struct starfive_wdt *wdt) >> +{ >> + u32 val; >> + >> + val = readl(wdt->base + wdt->drv_data->enable); >> + val &= ~(STARFIVE_WDT_ENABLE << wdt->drv_data->variant->en_shift); >> + writel(val, wdt->base + wdt->drv_data->enable); >> +} >> + >> +static inline void starfive_wdt_set_relod_count(struct starfive_wdt *wdt, u32 count) >> +{ >> + writel(count, wdt->base + wdt->drv_data->load); >> + if (wdt->drv_data->reload) >> + writel(0x1, wdt->base + wdt->drv_data->reload); >> + else >> + /* jh7110 need enable controller to reload counter */ >> + starfive_wdt_enable(wdt); >> +} >> + >> +static void starfive_wdt_mask_and_disable_reset(struct starfive_wdt *wdt, bool mask) >> +{ >> + starfive_wdt_unlock(wdt); >> + >> + if (mask) >> + starfive_wdt_disable_reset(wdt); >> + else >> + starfive_wdt_enable_reset(wdt); >> + >> + starfive_wdt_lock(wdt); >> +} >> + >> +static unsigned int starfive_wdt_max_timeout(struct starfive_wdt *wdt) >> +{ >> + return DIV_ROUND_UP(STARFIVE_WDT_MAXCNT, wdt->freq) - 1; >> +} >> + >> +static unsigned int starfive_wdt_get_timeleft(struct watchdog_device *wdd) >> +{ >> + struct starfive_wdt *wdt = watchdog_get_drvdata(wdd); >> + u32 count; >> + u32 irq_status; >> + >> + starfive_wdt_unlock(wdt); >> + /* >> + * Because set half count value, >> + * timeleft value should add the count value before first timeout. >> + */ >> + irq_status = starfive_wdt_raise_irq_status(wdt) ? 1 : 0; >> + count = starfive_wdt_get_count(wdt) + (1 - irq_status) * wdt->count; >> + starfive_wdt_lock(wdt); >> + >> + return starfive_wdt_ticks_to_sec(wdt, count); >> +} >> + >> +static int starfive_wdt_keepalive(struct watchdog_device *wdd) >> +{ >> + struct starfive_wdt *wdt = watchdog_get_drvdata(wdd); >> + >> + spin_lock(&wdt->lock); >> + >> + starfive_wdt_unlock(wdt); >> + starfive_wdt_int_clr(wdt); >> + starfive_wdt_set_relod_count(wdt, wdt->count); >> + starfive_wdt_lock(wdt); >> + >> + spin_unlock(&wdt->lock); >> + >> + return 0; >> +} >> + >> +static irqreturn_t starfive_wdt_interrupt_handler(int irq, void *data) >> +{ >> + /* >> + * We don't clear the IRQ status. It's supposed to be done by the >> + * following ping operations. >> + */ >> + struct platform_device *pdev = data; >> + struct starfive_wdt *wdt = platform_get_drvdata(pdev); >> + >> + /* Disable the IRQ and avoid re-entry interrupt. */ >> + disable_irq_nosync(wdt->irq); >> + >> + return IRQ_HANDLED; >> +} > > What is the purpose of this interrupt ? It doesn't add any value > as implemented. After your reminder, I think this is no longer necessary now and will drop it. > >> + >> +static int starfive_wdt_stop(struct watchdog_device *wdd) >> +{ >> + struct starfive_wdt *wdt = watchdog_get_drvdata(wdd); >> + >> + spin_lock(&wdt->lock); >> + >> + starfive_wdt_unlock(wdt); >> + starfive_wdt_int_disable(wdt); >> + starfive_wdt_int_clr(wdt); >> + starfive_wdt_disable(wdt); >> + starfive_wdt_lock(wdt); >> + >> + spin_unlock(&wdt->lock); >> + >> + return 0; >> +} >> + >> +static int starfive_wdt_pm_stop(struct watchdog_device *wdd) >> +{ >> + struct starfive_wdt *wdt = watchdog_get_drvdata(wdd); >> + >> + starfive_wdt_stop(wdd); >> + pm_runtime_put(wdt->dev); >> + >> + return 0; >> +} >> + >> +static int starfive_wdt_start(struct watchdog_device *wdd) >> +{ >> + struct starfive_wdt *wdt = watchdog_get_drvdata(wdd); >> + >> + pm_runtime_get_sync(wdt->dev); >> + >> + spin_lock(&wdt->lock); >> + >> + starfive_wdt_unlock(wdt); >> + >> + if (soft_noboot) >> + starfive_wdt_disable_reset(wdt); >> + else >> + starfive_wdt_enable_reset(wdt); >> + >> + starfive_wdt_int_clr(wdt); >> + starfive_wdt_set_count(wdt, wdt->count); >> + starfive_wdt_int_enable(wdt); >> + starfive_wdt_enable(wdt); >> + >> + starfive_wdt_lock(wdt); >> + >> + spin_unlock(&wdt->lock); >> + >> + return 0; >> +} >> + >> +static int starfive_wdt_restart(struct watchdog_device *wdd, unsigned long action, >> + void *data) >> +{ >> + struct starfive_wdt *wdt = watchdog_get_drvdata(wdd); >> + >> + spin_lock(&wdt->lock); >> + starfive_wdt_unlock(wdt); >> + /* disable watchdog, to be safe */ >> + starfive_wdt_disable(wdt); >> + >> + if (soft_noboot) >> + starfive_wdt_disable_reset(wdt); >> + else >> + starfive_wdt_enable_reset(wdt); >> + >> + /* put initial values into count and data */ >> + starfive_wdt_set_count(wdt, wdt->count); > > This is supposed to reset the system. Why write wdt->count > instead of 0 or 1 ? This is in order to make the watchdog work again and make sure the counter start from wdt->count which had been set before restart. > >> + >> + /* set the watchdog to go and reset... */ >> + starfive_wdt_int_clr(wdt); >> + starfive_wdt_int_enable(wdt); >> + starfive_wdt_enable(wdt); >> + >> + starfive_wdt_lock(wdt); >> + spin_unlock(&wdt->lock); >> + >> + return 0; >> +} >> + >> +static int starfive_wdt_set_timeout(struct watchdog_device *wdd, >> + unsigned int timeout) >> +{ >> + struct starfive_wdt *wdt = watchdog_get_drvdata(wdd); >> + >> + unsigned long freq = wdt->freq; >> + unsigned int count; >> + >> + spin_lock(&wdt->lock); >> + >> + if (timeout < 1) >> + return -EINVAL; > > Can not happen if configured properly, ie if min_timeout is set *which it is). Will use min_timeout instead of 1. > >> + >> + /* >> + * This watchdog takes twice timeouts to reset. >> + * In order to reduce time to reset, should set half count value. >> + */ >> + count = timeout * freq / 2; >> + >> + if (count > STARFIVE_WDT_MAXCNT) { >> + dev_warn(wdt->dev, "timeout %d too big,use the MAX-timeout set.\n", >> + timeout); >> + timeout = starfive_wdt_max_timeout(wdt); >> + count = timeout * freq; >> + } >> + >> + dev_info(wdt->dev, "Heartbeat: timeout=%d, count/2=%d (%08x)\n", >> + timeout, count, count); >> + >> + starfive_wdt_unlock(wdt); >> + starfive_wdt_disable(wdt); >> + starfive_wdt_set_relod_count(wdt, count); >> + starfive_wdt_enable(wdt); >> + starfive_wdt_lock(wdt); >> + >> + wdt->count = count; >> + wdd->timeout = timeout; >> + >> + spin_unlock(&wdt->lock); >> + >> + return 0; >> +} >> + >> +#define OPTIONS (WDIOF_SETTIMEOUT | WDIOF_KEEPALIVEPING | WDIOF_MAGICCLOSE) >> + >> +static const struct watchdog_info starfive_wdt_ident = { >> + .options = OPTIONS, >> + .firmware_version = 0, >> + .identity = "StarFive Watchdog", >> +}; >> + >> +static const struct watchdog_ops starfive_wdt_ops = { >> + .owner = THIS_MODULE, >> + .start = starfive_wdt_start, >> + .stop = starfive_wdt_pm_stop, >> + .ping = starfive_wdt_keepalive, >> + .set_timeout = starfive_wdt_set_timeout, >> + .restart = starfive_wdt_restart, >> + .get_timeleft = starfive_wdt_get_timeleft, >> +}; >> + >> +static const struct watchdog_device starfive_wdd = { >> + .info = &starfive_wdt_ident, >> + .ops = &starfive_wdt_ops, >> + .timeout = STARFIVE_WDT_DEFAULT_TIME, >> +}; >> + >> +static inline const struct starfive_wdt_variant * >> +starfive_wdt_get_drv_data(struct platform_device *pdev) >> +{ >> + const struct starfive_wdt_variant *variant; >> + >> + variant = of_device_get_match_data(&pdev->dev); >> + if (!variant) { >> + /* Device matched by platform_device_id */ >> + variant = (struct starfive_wdt_variant *) >> + platform_get_device_id(pdev)->driver_data; >> + } >> + >> + return variant; >> +} >> + >> +static int starfive_wdt_probe(struct platform_device *pdev) >> +{ >> + struct device *dev = &pdev->dev; >> + struct starfive_wdt *wdt; >> + int started = 0; >> + int ret; >> + >> + pm_runtime_enable(dev); >> + >> + wdt = devm_kzalloc(dev, sizeof(*wdt), GFP_KERNEL); >> + if (!wdt) >> + return -ENOMEM; >> + >> + wdt->dev = dev; >> + spin_lock_init(&wdt->lock); >> + wdt->wdt_device = starfive_wdd; >> + >> + wdt->drv_data = starfive_wdt_get_drv_data(pdev); >> + >> + wdt->irq = platform_get_irq(pdev, 0); >> + if (wdt->irq < 0) { >> + dev_err(dev, "can not find irq.\n"); >> + return wdt->irq; >> + } >> + >> + /* get the memory region for the watchdog timer */ >> + wdt->base = devm_platform_ioremap_resource(pdev, 0); >> + if (IS_ERR(wdt->base)) { >> + ret = PTR_ERR(wdt->base); >> + goto err; >> + } >> + >> + ret = starfive_wdt_enable_clock(wdt); >> + if (ret) >> + goto err; >> + >> + starfive_wdt_get_clock_rate(wdt); >> + > > This can fail, leaving the clock rate uninitialized. Will fix. > >> + ret = starfive_wdt_reset_init(wdt); >> + if (ret) >> + goto err; >> + >> + wdt->wdt_device.min_timeout = 1; >> + wdt->wdt_device.max_timeout = starfive_wdt_max_timeout(wdt); >> + >> + watchdog_set_drvdata(&wdt->wdt_device, wdt); >> + >> + /* >> + * see if we can actually set the requested timer margin, >> + * and if not, try the default value. >> + */ >> + watchdog_init_timeout(&wdt->wdt_device, tmr_margin, dev); >> + >> + ret = starfive_wdt_set_timeout(&wdt->wdt_device, >> + wdt->wdt_device.timeout); >> + if (ret) { >> + dev_err(dev, "tmr_margin value out of range, default %d used\n", >> + STARFIVE_WDT_DEFAULT_TIME); >> + starfive_wdt_set_timeout(&wdt->wdt_device, >> + STARFIVE_WDT_DEFAULT_TIME); >> + } >> + >> + ret = devm_request_irq(dev, wdt->irq, starfive_wdt_interrupt_handler, 0, >> + pdev->name, pdev); >> + if (ret != 0) { >> + dev_err(dev, "failed to install irq (%d)\n", ret); >> + goto err; >> + } >> + > > That interrupt seems pointless. Will drop it. > >> + watchdog_set_nowayout(&wdt->wdt_device, nowayout); >> + watchdog_set_restart_priority(&wdt->wdt_device, 128); >> + >> + wdt->wdt_device.parent = dev; >> + >> + ret = watchdog_register_device(&wdt->wdt_device); >> + if (ret) >> + goto err; >> + >> + starfive_wdt_mask_and_disable_reset(wdt, false); >> + >> + if (tmr_atboot && started == 0) { >> + starfive_wdt_start(&wdt->wdt_device); >> + } else if (!tmr_atboot) { >> + /* >> + *if we're not enabling the watchdog, then ensure it is >> + * disabled if it has been left running from the bootloader >> + * or other source. >> + */ >> + starfive_wdt_stop(&wdt->wdt_device); >> + } > > This should be done before registering the watdhdog, and the watchdog core > should be informed if the watchdog is running. This is supposed to decide whether to start the watchdog at the end of registration. It also can be stared by user after registering the watchdog. > >> + >> +#ifdef CONFIG_PM >> + clk_disable_unprepare(wdt->core_clk); >> + clk_disable_unprepare(wdt->apb_clk); >> +#endif >> + >> + platform_set_drvdata(pdev, wdt); >> + >> + return 0; >> + >> +err: >> + return ret; > > This is never acceptable. Return directly. Will fix. > >> +} >> + >> +static int starfive_wdt_remove(struct platform_device *dev) >> +{ >> + struct starfive_wdt *wdt = platform_get_drvdata(dev); >> + >> + starfive_wdt_mask_and_disable_reset(wdt, true); >> + watchdog_unregister_device(&wdt->wdt_device); >> + >> + clk_disable_unprepare(wdt->core_clk); >> + clk_disable_unprepare(wdt->apb_clk); >> + pm_runtime_disable(wdt->dev); > > This is potentially unbalanced. I would suggest to use > watchdog_stop_on_unregister(). Thank for your advice. I will add watchdog_stop_on_unregister() when registering the watchdog. > >> + >> + return 0; >> +} >> + >> +static void starfive_wdt_shutdown(struct platform_device *dev) >> +{ >> + struct starfive_wdt *wdt = platform_get_drvdata(dev); >> + >> + starfive_wdt_mask_and_disable_reset(wdt, true); >> + >> + starfive_wdt_pm_stop(&wdt->wdt_device); > > Use watchdog_stop_on_reboot(). I will add watchdog_stop_on_reboot() when registering the watchdog. > >> +} >> + >> +#ifdef CONFIG_PM_SLEEP >> +static int starfive_wdt_suspend(struct device *dev) >> +{ >> + int ret; >> + struct starfive_wdt *wdt = dev_get_drvdata(dev); >> + >> + starfive_wdt_unlock(wdt); >> + >> + /* Save watchdog state, and turn it off. */ >> + wdt->reload = starfive_wdt_get_count(wdt); >> + >> + starfive_wdt_mask_and_disable_reset(wdt, true); >> + >> + /* Note that WTCNT doesn't need to be saved. */ >> + starfive_wdt_stop(&wdt->wdt_device); >> + pm_runtime_force_suspend(dev); >> + >> + starfive_wdt_lock(wdt); >> + >> + return 0; >> +} >> + >> +static int starfive_wdt_resume(struct device *dev) >> +{ >> + int ret; >> + struct starfive_wdt *wdt = dev_get_drvdata(dev); >> + >> + starfive_wdt_unlock(wdt); >> + >> + pm_runtime_force_resume(dev); >> + >> + /* Restore watchdog state. */ >> + starfive_wdt_set_relod_count(wdt, wdt->reload); >> + >> + starfive_wdt_restart(&wdt->wdt_device, 0, NULL); >> + >> + starfive_wdt_mask_and_disable_reset(wdt, false); >> + >> + starfive_wdt_lock(wdt); >> + >> + return 0; >> +} >> +#endif /* CONFIG_PM_SLEEP */ >> + >> +#ifdef CONFIG_PM >> +static int starfive_wdt_runtime_suspend(struct device *dev) >> +{ >> + struct starfive_wdt *wdt = dev_get_drvdata(dev); >> + >> + clk_disable_unprepare(wdt->apb_clk); >> + clk_disable_unprepare(wdt->core_clk); >> + >> + return 0; >> +} >> + >> +static int starfive_wdt_runtime_resume(struct device *dev) >> +{ >> + struct starfive_wdt *wdt = dev_get_drvdata(dev); >> + int ret; >> + >> + ret = clk_prepare_enable(wdt->apb_clk); >> + if (ret) { >> + dev_err(wdt->dev, "failed to enable apb_clk.\n"); >> + goto err; >> + } >> + >> + ret = clk_prepare_enable(wdt->core_clk); >> + if (ret) { >> + dev_err(wdt->dev, "failed to enable core_clk.\n"); >> + goto err; >> + } >> + >> + return 0; >> + >> +err: >> + return ret; >> +} >> +#endif /* CONFIG_PM */ >> + >> +static const struct dev_pm_ops starfive_wdt_pm_ops = { >> + SET_RUNTIME_PM_OPS(starfive_wdt_runtime_suspend, starfive_wdt_runtime_resume, NULL) >> + SET_SYSTEM_SLEEP_PM_OPS(starfive_wdt_suspend, starfive_wdt_resume) >> +}; >> + >> +static struct platform_driver starfive_wdt_driver = { >> + .probe = starfive_wdt_probe, >> + .remove = starfive_wdt_remove, >> + .shutdown = starfive_wdt_shutdown, >> + .id_table = starfive_wdt_ids, >> + .driver = { >> + .name = "starfive-wdt", >> + .pm = &starfive_wdt_pm_ops, >> + .of_match_table = of_match_ptr(starfive_wdt_match), >> + }, >> +}; >> + >> +module_platform_driver(starfive_wdt_driver); >> + >> +MODULE_AUTHOR("Xingyu Wu <xingyu.wu@starfivetech.com>"); >> +MODULE_AUTHOR("Samin Guo <samin.guo@starfivetech.com>"); >> +MODULE_DESCRIPTION("StarFive Watchdog Device Driver"); >> +MODULE_LICENSE("GPL v2"); > Best regards, Xingyu Wu _______________________________________________ linux-riscv mailing list linux-riscv@lists.infradead.org http://lists.infradead.org/mailman/listinfo/linux-riscv ^ permalink raw reply [flat|nested] 13+ messages in thread
* [PATCH v1 3/3] riscv: dts: starfive: jh7110: Add watchdog node 2022-12-02 9:39 [PATCH v1 0/3] Add watchdog driver for StarFive RISC-V SoCs xingu.wu 2022-12-02 9:39 ` [PATCH v1 1/3] dt-bindings: watchdog: Add watchdog for StarFive xingu.wu 2022-12-02 9:39 ` [PATCH v1 2/3] drivers: watchdog: Add StarFive Watchdog driver xingu.wu @ 2022-12-02 9:39 ` xingu.wu 2022-12-02 10:48 ` Krzysztof Kozlowski 2022-12-02 11:52 ` [PATCH v1 0/3] Add watchdog driver for StarFive RISC-V SoCs Conor Dooley 3 siblings, 1 reply; 13+ messages in thread From: xingu.wu @ 2022-12-02 9:39 UTC (permalink / raw) To: linux-riscv, devicetree, linux-watchdog, Wim Van Sebroeck, Guenter Roeck Cc: Rob Herring, Krzysztof Kozlowski, Paul Walmsley, Palmer Dabbelt, Albert Ou, Philipp Zabel, Xingyu Wu, Samin Guo, linux-kernel From: Xingyu Wu <xingyu.wu@starfivetech.com> This adds the watchdog node for the Starfive JH7110 SoC. Signed-off-by: Xingyu Wu <xingyu.wu@starfivetech.com> --- arch/riscv/boot/dts/starfive/jh7110.dtsi | 14 ++++++++++++++ 1 file changed, 14 insertions(+) diff --git a/arch/riscv/boot/dts/starfive/jh7110.dtsi b/arch/riscv/boot/dts/starfive/jh7110.dtsi index c22e8f1d2640..22f5a37d691e 100644 --- a/arch/riscv/boot/dts/starfive/jh7110.dtsi +++ b/arch/riscv/boot/dts/starfive/jh7110.dtsi @@ -433,5 +433,19 @@ uart5: serial@12020000 { reg-shift = <2>; status = "disabled"; }; + + wdog: watchdog@13070000 { + compatible = "starfive,jh7110-wdt"; + reg = <0x0 0x13070000 0x0 0x10000>; + interrupts = <68>; + clocks = <&syscrg JH7110_SYSCLK_WDT_CORE>, + <&syscrg JH7110_SYSCLK_WDT_APB>; + clock-names = "core_clk", "apb_clk"; + resets = <&syscrg JH7110_SYSRST_WDT_APB>, + <&syscrg JH7110_SYSRST_WDT_CORE>; + reset-names = "rst_apb", "rst_core"; + timeout-sec = <15>; + status = "okay"; + }; }; }; -- 2.25.1 _______________________________________________ linux-riscv mailing list linux-riscv@lists.infradead.org http://lists.infradead.org/mailman/listinfo/linux-riscv ^ permalink raw reply related [flat|nested] 13+ messages in thread
* Re: [PATCH v1 3/3] riscv: dts: starfive: jh7110: Add watchdog node 2022-12-02 9:39 ` [PATCH v1 3/3] riscv: dts: starfive: jh7110: Add watchdog node xingu.wu @ 2022-12-02 10:48 ` Krzysztof Kozlowski 2022-12-05 8:27 ` Xingyu Wu 0 siblings, 1 reply; 13+ messages in thread From: Krzysztof Kozlowski @ 2022-12-02 10:48 UTC (permalink / raw) To: xingu.wu, linux-riscv, devicetree, linux-watchdog, Wim Van Sebroeck, Guenter Roeck Cc: Rob Herring, Krzysztof Kozlowski, Paul Walmsley, Palmer Dabbelt, Albert Ou, Philipp Zabel, Samin Guo, linux-kernel On 02/12/2022 10:39, xingu.wu wrote: > From: Xingyu Wu <xingyu.wu@starfivetech.com> > > This adds the watchdog node for the Starfive JH7110 SoC. Do not use "This commit/patch". https://elixir.bootlin.com/linux/v5.17.1/source/Documentation/process/submitting-patches.rst#L95 > > Signed-off-by: Xingyu Wu <xingyu.wu@starfivetech.com> > --- > arch/riscv/boot/dts/starfive/jh7110.dtsi | 14 ++++++++++++++ > 1 file changed, 14 insertions(+) > > diff --git a/arch/riscv/boot/dts/starfive/jh7110.dtsi b/arch/riscv/boot/dts/starfive/jh7110.dtsi > index c22e8f1d2640..22f5a37d691e 100644 > --- a/arch/riscv/boot/dts/starfive/jh7110.dtsi > +++ b/arch/riscv/boot/dts/starfive/jh7110.dtsi > @@ -433,5 +433,19 @@ uart5: serial@12020000 { > reg-shift = <2>; > status = "disabled"; > }; > + > + wdog: watchdog@13070000 { > + compatible = "starfive,jh7110-wdt"; > + reg = <0x0 0x13070000 0x0 0x10000>; > + interrupts = <68>; > + clocks = <&syscrg JH7110_SYSCLK_WDT_CORE>, > + <&syscrg JH7110_SYSCLK_WDT_APB>; > + clock-names = "core_clk", "apb_clk"; > + resets = <&syscrg JH7110_SYSRST_WDT_APB>, > + <&syscrg JH7110_SYSRST_WDT_CORE>; > + reset-names = "rst_apb", "rst_core"; > + timeout-sec = <15>; > + status = "okay"; Why? okay is by default Best regards, Krzysztof _______________________________________________ linux-riscv mailing list linux-riscv@lists.infradead.org http://lists.infradead.org/mailman/listinfo/linux-riscv ^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: [PATCH v1 3/3] riscv: dts: starfive: jh7110: Add watchdog node 2022-12-02 10:48 ` Krzysztof Kozlowski @ 2022-12-05 8:27 ` Xingyu Wu 0 siblings, 0 replies; 13+ messages in thread From: Xingyu Wu @ 2022-12-05 8:27 UTC (permalink / raw) To: Krzysztof Kozlowski Cc: Rob Herring, Paul Walmsley, Palmer Dabbelt, Albert Ou, Philipp Zabel, linux-watchdog, Samin Guo, linux-kernel, linux-riscv, devicetree, Wim Van Sebroeck, Guenter Roeck On 2022/12/2 18:48, Krzysztof Kozlowski wrote: > On 02/12/2022 10:39, xingu.wu wrote: >> From: Xingyu Wu <xingyu.wu@starfivetech.com> >> >> This adds the watchdog node for the Starfive JH7110 SoC. > > Do not use "This commit/patch". > https://elixir.bootlin.com/linux/v5.17.1/source/Documentation/process/submitting-patches.rst#L95 Will drop 'This'. > >> >> Signed-off-by: Xingyu Wu <xingyu.wu@starfivetech.com> >> --- >> arch/riscv/boot/dts/starfive/jh7110.dtsi | 14 ++++++++++++++ >> 1 file changed, 14 insertions(+) >> >> diff --git a/arch/riscv/boot/dts/starfive/jh7110.dtsi b/arch/riscv/boot/dts/starfive/jh7110.dtsi >> index c22e8f1d2640..22f5a37d691e 100644 >> --- a/arch/riscv/boot/dts/starfive/jh7110.dtsi >> +++ b/arch/riscv/boot/dts/starfive/jh7110.dtsi >> @@ -433,5 +433,19 @@ uart5: serial@12020000 { >> reg-shift = <2>; >> status = "disabled"; >> }; >> + >> + wdog: watchdog@13070000 { >> + compatible = "starfive,jh7110-wdt"; >> + reg = <0x0 0x13070000 0x0 0x10000>; >> + interrupts = <68>; >> + clocks = <&syscrg JH7110_SYSCLK_WDT_CORE>, >> + <&syscrg JH7110_SYSCLK_WDT_APB>; >> + clock-names = "core_clk", "apb_clk"; >> + resets = <&syscrg JH7110_SYSRST_WDT_APB>, >> + <&syscrg JH7110_SYSRST_WDT_CORE>; >> + reset-names = "rst_apb", "rst_core"; >> + timeout-sec = <15>; >> + status = "okay"; > > Why? okay is by default > Will drop it. Best regards, Xingyu Wu _______________________________________________ linux-riscv mailing list linux-riscv@lists.infradead.org http://lists.infradead.org/mailman/listinfo/linux-riscv ^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: [PATCH v1 0/3] Add watchdog driver for StarFive RISC-V SoCs 2022-12-02 9:39 [PATCH v1 0/3] Add watchdog driver for StarFive RISC-V SoCs xingu.wu ` (2 preceding siblings ...) 2022-12-02 9:39 ` [PATCH v1 3/3] riscv: dts: starfive: jh7110: Add watchdog node xingu.wu @ 2022-12-02 11:52 ` Conor Dooley 3 siblings, 0 replies; 13+ messages in thread From: Conor Dooley @ 2022-12-02 11:52 UTC (permalink / raw) To: xingu.wu Cc: linux-riscv, devicetree, linux-watchdog, Wim Van Sebroeck, Guenter Roeck, Rob Herring, Krzysztof Kozlowski, Paul Walmsley, Palmer Dabbelt, Albert Ou, Philipp Zabel, Samin Guo, linux-kernel, Hal Feng [-- Attachment #1.1: Type: text/plain, Size: 4459 bytes --] Hello! +CC Hal Feng On Fri, Dec 02, 2022 at 05:39:40PM +0800, xingu.wu wrote: > This patch serises are to add watchdog driver for the StarFive RISC-V SoCs > such as JH7110. The first patch adds docunmentation to describe device > tree bindings. The subsequent patch adds watchdog driver and support > JH7110 SoC. The last patch adds device node about watchdog to JH7110 > dts. > > The watchdog driver has been tested on the VisionFive 2 boards which > equip with JH7110 SoC and works normally. > > This patchset should be applied after the patchset [1], [2], [3]: > [1] https://lore.kernel.org/all/20221118010627.70576-1-hal.feng@starfivetech.com/ > [2] https://lore.kernel.org/all/20221118011108.70715-1-hal.feng@starfivetech.com/ > [3] https://lore.kernel.org/all/20221118011714.70877-1-hal.feng@starfivetech.com/ You say "patchset" but that's not really the case. Realistically, only one patch here has a real dependency on the above AFAICT. How about for v2, whenever you send that, in your dt-bindings you drop the uses of dt-bindings/*/starfive-jh7110.h & instead put the numbers? Same goes, IMO, for ~all the jh7110 dt-bindings, see here: https://lore.kernel.org/all/624cba7f-1be9-7100-91d7-f9232c855d9f@linaro.org/ Then the only person that has to care about prerequisite patches is then me for the dt patch and you will not have the driver itself delayed until the clock bindings are merged? Perhaps Guenter likes having the defines, but my opinion would be get as much as possible that does not have real dependencies on the clock & reset bindings capable of being merged in isolation. Thanks, Conor. > > Xingyu Wu (3): > dt-bindings: watchdog: Add watchdog for StarFive > drivers: watchdog: Add StarFive Watchdog driver > riscv: dts: starfive: jh7110: Add watchdog node > > .../bindings/watchdog/starfive,wdt.yaml | 77 ++ > MAINTAINERS | 7 + > arch/riscv/boot/dts/starfive/jh7110.dtsi | 14 + > drivers/watchdog/Kconfig | 11 + > drivers/watchdog/Makefile | 3 + > drivers/watchdog/starfive-wdt.c | 854 ++++++++++++++++++ > 6 files changed, 966 insertions(+) > create mode 100644 Documentation/devicetree/bindings/watchdog/starfive,wdt.yaml > create mode 100644 drivers/watchdog/starfive-wdt.c > > > base-commit: 094226ad94f471a9f19e8f8e7140a09c2625abaa > prerequisite-patch-id: 6b1b43a55b9773bec61ab6c1bbaa54dccbac0837 > prerequisite-patch-id: 09c98554df52d17ba5fd604125f8cdd62cbe80d1 > prerequisite-patch-id: 29fe0b0c19b6f0cd31114ee9fe17fe9732047f33 > prerequisite-patch-id: c59d9908de90e09ba2b9a81aadbf9fb9f00c8f04 > prerequisite-patch-id: 94ac03d518993921bcfc9cc9f58d7da0c3528b51 > prerequisite-patch-id: 694f7400375f5b85581fc1821e427334507826f2 > prerequisite-patch-id: 699d49c4439dadb4b7cf900857f027d050cd6093 > prerequisite-patch-id: 40d773f5a19912f731ee5fd4739ed2e3c2157b07 > prerequisite-patch-id: 2bc3fd6df5dda116efe882045863d6c88aa81b3a > prerequisite-patch-id: 735e62255c75801bdc4c0b4107850bce821ff7f5 > prerequisite-patch-id: b2a923b922e661fa6085185f33c1f1e733db9110 > prerequisite-patch-id: b2bbc28354075432f059344eba5a127a653475cf > prerequisite-patch-id: 70eab7b7eee728afcd90e40f6743d1356f6d81ab > prerequisite-patch-id: 6276b2a23818c65ff2ad3d65b562615690cffee9 > prerequisite-patch-id: d834ece14ffb525b8c3e661e78736692f33fca9b > prerequisite-patch-id: 4c17a3ce4dae9b788795d915bf775630f5c43c53 > prerequisite-patch-id: dabb913fd478e97593e45c23fee4be9fd807f851 > prerequisite-patch-id: 22fa141f7f0f80a5d619e9f3f4cf161ad06f108e > prerequisite-patch-id: f306819c257ea73aff8e06b17b5731053cdddfc8 > prerequisite-patch-id: 536114f0732646095ef5302a165672b3290d4c75 > prerequisite-patch-id: 258ea5f9b8bf41b6981345dcc81795f25865d38f > prerequisite-patch-id: 8b6f2c9660c0ac0ee4e73e4c21aca8e6b75e81b9 > prerequisite-patch-id: e09e995700a814a763aa304ad3881a7222acf556 > prerequisite-patch-id: 05803238293fcc90c8e83018a1103c41133a816c > prerequisite-patch-id: d074c7ffa2917a9f754d5801e3f67bc980f9de4c > prerequisite-patch-id: ef23fdf3466b3c713b3826e8545c8dd2bc6cc9d7 > prerequisite-patch-id: d5da3475c6a3588e11a1678feb565bdd459b548e > -- > 2.25.1 > > > _______________________________________________ > linux-riscv mailing list > linux-riscv@lists.infradead.org > http://lists.infradead.org/mailman/listinfo/linux-riscv [-- Attachment #1.2: signature.asc --] [-- Type: application/pgp-signature, Size: 228 bytes --] [-- Attachment #2: Type: text/plain, Size: 161 bytes --] _______________________________________________ linux-riscv mailing list linux-riscv@lists.infradead.org http://lists.infradead.org/mailman/listinfo/linux-riscv ^ permalink raw reply [flat|nested] 13+ messages in thread
end of thread, other threads:[~2022-12-05 8:31 UTC | newest] Thread overview: 13+ messages (download: mbox.gz follow: Atom feed -- links below jump to the message on this page -- 2022-12-02 9:39 [PATCH v1 0/3] Add watchdog driver for StarFive RISC-V SoCs xingu.wu 2022-12-02 9:39 ` [PATCH v1 1/3] dt-bindings: watchdog: Add watchdog for StarFive xingu.wu 2022-12-02 10:46 ` Krzysztof Kozlowski 2022-12-05 3:49 ` Xingyu Wu 2022-12-05 7:30 ` Krzysztof Kozlowski 2022-12-05 8:22 ` Xingyu Wu 2022-12-02 9:39 ` [PATCH v1 2/3] drivers: watchdog: Add StarFive Watchdog driver xingu.wu 2022-12-02 15:08 ` Guenter Roeck 2022-12-05 8:08 ` Xingyu Wu 2022-12-02 9:39 ` [PATCH v1 3/3] riscv: dts: starfive: jh7110: Add watchdog node xingu.wu 2022-12-02 10:48 ` Krzysztof Kozlowski 2022-12-05 8:27 ` Xingyu Wu 2022-12-02 11:52 ` [PATCH v1 0/3] Add watchdog driver for StarFive RISC-V SoCs Conor Dooley
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox