* [PATCH 0/2] watchdog: Add Nuvoton NPCM (Poleg) driver @ 2018-03-02 6:37 Joel Stanley 2018-03-02 6:37 ` [PATCH 1/2] dt-bindings: watchdog: Add Nuvoton NPCM description Joel Stanley 2018-03-02 6:37 ` [PATCH 2/2] watchdog: Add Nuvoton NPCM watchdog driver Joel Stanley 0 siblings, 2 replies; 8+ messages in thread From: Joel Stanley @ 2018-03-02 6:37 UTC (permalink / raw) To: Wim Van Sebroeck, Guenter Roeck, Rob Herring, Mark Rutland Cc: linux-watchdog, devicetree, linux-kernel, Tomer Maimon, Avi Fishman, Brendan Higgins This is a driver for the Poleg board that is in the process of being upstreamed. I have tested it on an evaluation board. The watchdog is a single register inside of the timer IP. This made me think about how to describe the device tree bindings for a while, but after some discussion I settled on describing the watchdog separately, and giving it's own compatible string, and it's own driver. The timer is being reviewed over here: https://lkml.org/lkml/2018/2/26/492 Joel Stanley (2): dt-bindings: watchdog: Add Nuvoton NPCM description watchdog: Add Nuvoton NPCM watchdog driver .../bindings/watchdog/nuvoton,npcm-wdt.txt | 25 +++ drivers/watchdog/Kconfig | 11 + drivers/watchdog/Makefile | 1 + drivers/watchdog/npcm_wdt.c | 223 +++++++++++++++++++++ 4 files changed, 260 insertions(+) create mode 100644 Documentation/devicetree/bindings/watchdog/nuvoton,npcm-wdt.txt create mode 100644 drivers/watchdog/npcm_wdt.c -- 2.15.1 ^ permalink raw reply [flat|nested] 8+ messages in thread
* [PATCH 1/2] dt-bindings: watchdog: Add Nuvoton NPCM description 2018-03-02 6:37 [PATCH 0/2] watchdog: Add Nuvoton NPCM (Poleg) driver Joel Stanley @ 2018-03-02 6:37 ` Joel Stanley 2018-03-02 11:24 ` Marcus Folkesson 2018-03-02 6:37 ` [PATCH 2/2] watchdog: Add Nuvoton NPCM watchdog driver Joel Stanley 1 sibling, 1 reply; 8+ messages in thread From: Joel Stanley @ 2018-03-02 6:37 UTC (permalink / raw) To: Wim Van Sebroeck, Guenter Roeck, Rob Herring, Mark Rutland Cc: linux-watchdog, devicetree, linux-kernel, Tomer Maimon, Avi Fishman, Brendan Higgins These bindings describe the watchdog IP as used by the Nuvoton NPCM750 (Poleg) BMC SoC. Signed-off-by: Joel Stanley <joel@jms.id.au> --- .../bindings/watchdog/nuvoton,npcm-wdt.txt | 25 ++++++++++++++++++++++ 1 file changed, 25 insertions(+) create mode 100644 Documentation/devicetree/bindings/watchdog/nuvoton,npcm-wdt.txt diff --git a/Documentation/devicetree/bindings/watchdog/nuvoton,npcm-wdt.txt b/Documentation/devicetree/bindings/watchdog/nuvoton,npcm-wdt.txt new file mode 100644 index 000000000000..599db288337e --- /dev/null +++ b/Documentation/devicetree/bindings/watchdog/nuvoton,npcm-wdt.txt @@ -0,0 +1,25 @@ +Nuvoton NPCM Watchdog + +Nuvoton NPCM timer module provides five 24-bit timer counters, and a watchdog. +The watchdog supports a pre-timeout interrupt that fires 10ms before the +expiry. + +Required properties: +- compatible : "nuvoton,npcm750-wdt" for NPCM750 (Poleg). +- reg : Offset and length of the register set for the device. +- interrupts : Contain the timer interrupt with flags for + falling edge. + +Required clocking property, have to be one of: +- clocks : phandle of timer reference clock. +- clock-frequency : The frequency in Hz of the clock that drives the NPCM7xx + timer (usually 25000000). + +Example: + +timer@f00080c1 { + compatible = "nuvoton,npcm750-wdt"; + interrupts = <GIC_SPI 47 IRQ_TYPE_LEVEL_HIGH>; + reg = <0xf00080c1 0x4>; + clocks = <&clk NPCM7XX_CLK_TIMER>; +}; -- 2.15.1 ^ permalink raw reply related [flat|nested] 8+ messages in thread
* Re: [PATCH 1/2] dt-bindings: watchdog: Add Nuvoton NPCM description 2018-03-02 6:37 ` [PATCH 1/2] dt-bindings: watchdog: Add Nuvoton NPCM description Joel Stanley @ 2018-03-02 11:24 ` Marcus Folkesson 0 siblings, 0 replies; 8+ messages in thread From: Marcus Folkesson @ 2018-03-02 11:24 UTC (permalink / raw) To: Joel Stanley Cc: Wim Van Sebroeck, Guenter Roeck, Rob Herring, Mark Rutland, linux-watchdog, devicetree, linux-kernel, Tomer Maimon, Avi Fishman, Brendan Higgins [-- Attachment #1: Type: text/plain, Size: 1885 bytes --] On Fri, Mar 02, 2018 at 05:07:45PM +1030, Joel Stanley wrote: > These bindings describe the watchdog IP as used by the Nuvoton NPCM750 > (Poleg) BMC SoC. > > Signed-off-by: Joel Stanley <joel@jms.id.au> > --- > .../bindings/watchdog/nuvoton,npcm-wdt.txt | 25 ++++++++++++++++++++++ > 1 file changed, 25 insertions(+) > create mode 100644 Documentation/devicetree/bindings/watchdog/nuvoton,npcm-wdt.txt > > diff --git a/Documentation/devicetree/bindings/watchdog/nuvoton,npcm-wdt.txt b/Documentation/devicetree/bindings/watchdog/nuvoton,npcm-wdt.txt > new file mode 100644 > index 000000000000..599db288337e > --- /dev/null > +++ b/Documentation/devicetree/bindings/watchdog/nuvoton,npcm-wdt.txt > @@ -0,0 +1,25 @@ > +Nuvoton NPCM Watchdog > + > +Nuvoton NPCM timer module provides five 24-bit timer counters, and a watchdog. > +The watchdog supports a pre-timeout interrupt that fires 10ms before the > +expiry. > + > +Required properties: > +- compatible : "nuvoton,npcm750-wdt" for NPCM750 (Poleg). > +- reg : Offset and length of the register set for the device. > +- interrupts : Contain the timer interrupt with flags for > + falling edge. > + > +Required clocking property, have to be one of: > +- clocks : phandle of timer reference clock. > +- clock-frequency : The frequency in Hz of the clock that drives the NPCM7xx > + timer (usually 25000000). Optional properties: - timeout-sec : Contains the watchdog timeout in seconds watchdog_init_timeout() will fix this for you. > + > +Example: > + > +timer@f00080c1 { > + compatible = "nuvoton,npcm750-wdt"; > + interrupts = <GIC_SPI 47 IRQ_TYPE_LEVEL_HIGH>; > + reg = <0xf00080c1 0x4>; > + clocks = <&clk NPCM7XX_CLK_TIMER>; > +}; > -- > 2.15.1 > Best regards Marcus Folkesson [-- Attachment #2: signature.asc --] [-- Type: application/pgp-signature, Size: 833 bytes --] ^ permalink raw reply [flat|nested] 8+ messages in thread
* [PATCH 2/2] watchdog: Add Nuvoton NPCM watchdog driver 2018-03-02 6:37 [PATCH 0/2] watchdog: Add Nuvoton NPCM (Poleg) driver Joel Stanley 2018-03-02 6:37 ` [PATCH 1/2] dt-bindings: watchdog: Add Nuvoton NPCM description Joel Stanley @ 2018-03-02 6:37 ` Joel Stanley 2018-03-02 11:20 ` Marcus Folkesson 2018-03-02 13:59 ` Guenter Roeck 1 sibling, 2 replies; 8+ messages in thread From: Joel Stanley @ 2018-03-02 6:37 UTC (permalink / raw) To: Wim Van Sebroeck, Guenter Roeck, Rob Herring, Mark Rutland Cc: linux-watchdog, devicetree, linux-kernel, Tomer Maimon, Avi Fishman, Brendan Higgins The Nuvoton NPCM750 has a watchdog implemented as a single register inside the timer peripheral. This driver exposes that watchdog as a standard watchdog device with coarse timeout intervals, limited by the combination of prescaler and counter that is provided by the hardware. The calculation is taken from the Nuvoton vendor tree. There is a pre-timeout IRQ that is wired up. This timeout always occurs 1024 clocks before the timeout. Signed-off-by: Joel Stanley <joel@jms.id.au> --- drivers/watchdog/Kconfig | 11 +++ drivers/watchdog/Makefile | 1 + drivers/watchdog/npcm_wdt.c | 223 ++++++++++++++++++++++++++++++++++++++++++++ 3 files changed, 235 insertions(+) create mode 100644 drivers/watchdog/npcm_wdt.c diff --git a/drivers/watchdog/Kconfig b/drivers/watchdog/Kconfig index aff773bcebdb..0c1cc68894e6 100644 --- a/drivers/watchdog/Kconfig +++ b/drivers/watchdog/Kconfig @@ -513,6 +513,17 @@ config COH901327_WATCHDOG This watchdog is used to reset the system and thus cannot be compiled as a module. +config NPCM7XX_WATCHDOG + bool "NPCM750 watchdog" + depends on ARCH_NPCM || COMPILE_TEST + default y if ARCH_NPCM750 + select WATCHDOG_CORE + help + Say Y here to include Watchdog timer support for the + watchdog embedded into the NPCM7xx. + This watchdog is used to reset the system and thus cannot be + compiled as a module. + config TWL4030_WATCHDOG tristate "TWL4030 Watchdog" depends on TWL4030_CORE diff --git a/drivers/watchdog/Makefile b/drivers/watchdog/Makefile index 0474d38aa854..97a5afb5cad2 100644 --- a/drivers/watchdog/Makefile +++ b/drivers/watchdog/Makefile @@ -61,6 +61,7 @@ obj-$(CONFIG_ORION_WATCHDOG) += orion_wdt.o obj-$(CONFIG_SUNXI_WATCHDOG) += sunxi_wdt.o obj-$(CONFIG_RN5T618_WATCHDOG) += rn5t618_wdt.o obj-$(CONFIG_COH901327_WATCHDOG) += coh901327_wdt.o +obj-$(CONFIG_NPCM7XX_WATCHDOG) += npcm_wdt.o obj-$(CONFIG_STMP3XXX_RTC_WATCHDOG) += stmp3xxx_rtc_wdt.o obj-$(CONFIG_NUC900_WATCHDOG) += nuc900_wdt.o obj-$(CONFIG_TS4800_WATCHDOG) += ts4800_wdt.o diff --git a/drivers/watchdog/npcm_wdt.c b/drivers/watchdog/npcm_wdt.c new file mode 100644 index 000000000000..71e3d4916514 --- /dev/null +++ b/drivers/watchdog/npcm_wdt.c @@ -0,0 +1,223 @@ +// SPDX-License-Identifier: GPL-2.0 +// Copyright (c) 2018 Nuvoton Technology corporation. +// Copyright (c) 2018 IBM Corp. + +#include <linux/delay.h> +#include <linux/kernel.h> +#include <linux/module.h> +#include <linux/platform_device.h> +#include <linux/slab.h> +#include <linux/interrupt.h> +#include <linux/watchdog.h> +#include <asm/fiq.h> +#include <linux/of_irq.h> + +#define NPCM_WTCR 0x1C + +#define NPCM_WTCLK (BIT(10) | BIT(11)) /* Clock divider */ +#define NPCM_WTE BIT(7) /* Enable */ +#define NPCM_WTIE BIT(6) /* Enable irq */ +#define NPCM_WTIS (BIT(4) | BIT(5)) /* Interval selection */ +#define NPCM_WTIF BIT(3) /* Interrupt flag*/ +#define NPCM_WTRF BIT(2) /* Reset flag */ +#define NPCM_WTRE BIT(1) /* Reset enable */ +#define NPCM_WTR BIT(0) /* Reset counter */ + +/* + * Watchdog timeouts + * + * 170 msec: WTCLK=01 WTIS=00 VAL= 0x400 + * 670 msec: WTCLK=01 WTIS=01 VAL= 0x410 + * 1360 msec: WTCLK=10 WTIS=00 VAL= 0x800 + * 2700 msec: WTCLK=01 WTIS=10 VAL= 0x420 + * 5360 msec: WTCLK=10 WTIS=01 VAL= 0x810 + * 10700 msec: WTCLK=01 WTIS=11 VAL= 0x430 + * 21600 msec: WTCLK=10 WTIS=10 VAL= 0x820 + * 43000 msec: WTCLK=11 WTIS=00 VAL= 0xC00 + * 85600 msec: WTCLK=10 WTIS=11 VAL= 0x830 + * 172000 msec: WTCLK=11 WTIS=01 VAL= 0xC10 + * 687000 msec: WTCLK=11 WTIS=10 VAL= 0xC20 + * 2750000 msec: WTCLK=11 WTIS=11 VAL= 0xC30 + */ + +struct npcm_wdt { + struct watchdog_device wdd; + struct device *dev; + void __iomem *reg; +}; + +static inline struct npcm_wdt *to_npcm_wdt(struct watchdog_device *wdd) +{ + return container_of(wdd, struct npcm_wdt, wdd); +} + +static int npcm_wdt_ping(struct watchdog_device *wdd) +{ + struct npcm_wdt *wdt = to_npcm_wdt(wdd); + u32 val; + + val = readl(wdt->reg); + writel(val | NPCM_WTR, wdt->reg); + + return 0; +} + +static int npcm_wdt_start(struct watchdog_device *wdd) +{ + struct npcm_wdt *wdt = to_npcm_wdt(wdd); + u32 val; + + val = NPCM_WTRE | NPCM_WTE | NPCM_WTR | NPCM_WTIE; + + if (wdd->timeout < 2) + val |= 0x800; + else if (wdd->timeout < 3) + val |= 0x420; + else if (wdd->timeout < 6) + val |= 0x810; + else if (wdd->timeout < 11) + val |= 0x430; + else if (wdd->timeout < 22) + val |= 0x820; + else if (wdd->timeout < 44) + val |= 0xC00; + else if (wdd->timeout < 87) + val |= 0x830; + else if (wdd->timeout < 173) + val |= 0xC10; + else if (wdd->timeout < 688) + val |= 0xC20; + else if (wdd->timeout < 2751) + val |= 0xC30; + else + val |= 0x830; + + writel(val, wdt->reg); + + return 0; +} + +static int npcm_wdt_stop(struct watchdog_device *wdd) +{ + struct npcm_wdt *wdt = to_npcm_wdt(wdd); + + writel(0, wdt->reg); + + return 0; +} + + +static int npcm_wdt_set_timeout(struct watchdog_device *wdd, + unsigned int timeout) +{ + wdd->timeout = timeout; /* New timeout */ + if (watchdog_active(wdd)) + npcm_wdt_start(wdd); + + return 0; +} + +static irqreturn_t npcm_wdt_interrupt(int irq, void *data) +{ + struct npcm_wdt *wdt = data; + + watchdog_notify_pretimeout(&wdt->wdd); + + return IRQ_HANDLED; +} + +static int npcm_wdt_restart(struct watchdog_device *wdd, + unsigned long action, void *data) +{ + struct npcm_wdt *wdt = to_npcm_wdt(wdd); + + writel(NPCM_WTR | NPCM_WTRE | NPCM_WTE, wdt->reg); + udelay(1000); + + return 0; +} + +static const struct watchdog_info npcm_wdt_info = { + .identity = KBUILD_MODNAME, + .options = WDIOF_SETTIMEOUT + | WDIOF_KEEPALIVEPING + | WDIOF_MAGICCLOSE, +}; + +static const struct watchdog_ops npcm_wdt_ops = { + .owner = THIS_MODULE, + .start = npcm_wdt_start, + .stop = npcm_wdt_stop, + .ping = npcm_wdt_ping, + .set_timeout = npcm_wdt_set_timeout, + .restart = npcm_wdt_restart, +}; + +static int npcm_wdt_probe(struct platform_device *pdev) +{ + struct device *dev = &pdev->dev; + struct npcm_wdt *wdt; + struct resource *res; + int irq; + int ret; + + wdt = devm_kzalloc(&pdev->dev, sizeof(*wdt), GFP_KERNEL); + if (!wdt) + return -ENOMEM; + + res = platform_get_resource(pdev, IORESOURCE_MEM, 0); + wdt->reg = devm_ioremap_resource(dev, res); + if (IS_ERR(wdt->reg)) + return PTR_ERR(wdt->reg); + + irq = platform_get_irq(pdev, 0); + if (!irq) + return -EINVAL; + + wdt->dev = dev; + wdt->wdd.info = &npcm_wdt_info; + wdt->wdd.ops = &npcm_wdt_ops; + wdt->wdd.min_timeout = 1; + wdt->wdd.max_timeout = 2751; + wdt->wdd.parent = dev; + + wdt->wdd.timeout = 86; + watchdog_init_timeout(&wdt->wdd, 0, dev); + + ret = devm_request_irq(dev, irq, npcm_wdt_interrupt, 0, + "watchdog", wdt); + if (ret) + return ret; + + ret = devm_watchdog_register_device(dev, &wdt->wdd); + if (ret) { + dev_err(dev, "failed to register watchdog\n"); + return ret; + } + + platform_set_drvdata(pdev, wdt); + dev_info(dev, "NPCM watchdog driver enabled\n"); + + return 0; +} + +#ifdef CONFIG_OF +static const struct of_device_id npcm_wdt_match[] = { + {.compatible = "nuvoton,npcm750-wdt"}, + {}, +}; +MODULE_DEVICE_TABLE(of, npcm_wdt_match); +#endif + +static struct platform_driver npcm_wdt_driver = { + .probe = npcm_wdt_probe, + .driver = { + .name = "npcm-wdt", + .of_match_table = of_match_ptr(npcm_wdt_match), + }, +}; +module_platform_driver(npcm_wdt_driver); + +MODULE_AUTHOR("Joel Stanley"); +MODULE_DESCRIPTION("Watchdog driver for NPCM"); +MODULE_LICENSE("GPL"); -- 2.15.1 ^ permalink raw reply related [flat|nested] 8+ messages in thread
* Re: [PATCH 2/2] watchdog: Add Nuvoton NPCM watchdog driver 2018-03-02 6:37 ` [PATCH 2/2] watchdog: Add Nuvoton NPCM watchdog driver Joel Stanley @ 2018-03-02 11:20 ` Marcus Folkesson 2018-03-05 5:42 ` Joel Stanley 2018-03-02 13:59 ` Guenter Roeck 1 sibling, 1 reply; 8+ messages in thread From: Marcus Folkesson @ 2018-03-02 11:20 UTC (permalink / raw) To: Joel Stanley Cc: Wim Van Sebroeck, Guenter Roeck, Rob Herring, Mark Rutland, linux-watchdog, devicetree, linux-kernel, Tomer Maimon, Avi Fishman, Brendan Higgins [-- Attachment #1: Type: text/plain, Size: 9438 bytes --] Hello Joel! On Fri, Mar 02, 2018 at 05:07:46PM +1030, Joel Stanley wrote: > The Nuvoton NPCM750 has a watchdog implemented as a single register > inside the timer peripheral. > > This driver exposes that watchdog as a standard watchdog device with > coarse timeout intervals, limited by the combination of prescaler and > counter that is provided by the hardware. The calculation is taken from > the Nuvoton vendor tree. > > There is a pre-timeout IRQ that is wired up. This timeout always occurs > 1024 clocks before the timeout. > > Signed-off-by: Joel Stanley <joel@jms.id.au> > --- > drivers/watchdog/Kconfig | 11 +++ > drivers/watchdog/Makefile | 1 + > drivers/watchdog/npcm_wdt.c | 223 ++++++++++++++++++++++++++++++++++++++++++++ > 3 files changed, 235 insertions(+) > create mode 100644 drivers/watchdog/npcm_wdt.c > > diff --git a/drivers/watchdog/Kconfig b/drivers/watchdog/Kconfig > index aff773bcebdb..0c1cc68894e6 100644 > --- a/drivers/watchdog/Kconfig > +++ b/drivers/watchdog/Kconfig > @@ -513,6 +513,17 @@ config COH901327_WATCHDOG > This watchdog is used to reset the system and thus cannot be > compiled as a module. > > +config NPCM7XX_WATCHDOG > + bool "NPCM750 watchdog" Just asking, we do not want to make it possible to build this as a module? > + depends on ARCH_NPCM || COMPILE_TEST > + default y if ARCH_NPCM750 > + select WATCHDOG_CORE > + help > + Say Y here to include Watchdog timer support for the > + watchdog embedded into the NPCM7xx. > + This watchdog is used to reset the system and thus cannot be > + compiled as a module. > + > config TWL4030_WATCHDOG > tristate "TWL4030 Watchdog" > depends on TWL4030_CORE > diff --git a/drivers/watchdog/Makefile b/drivers/watchdog/Makefile > index 0474d38aa854..97a5afb5cad2 100644 > --- a/drivers/watchdog/Makefile > +++ b/drivers/watchdog/Makefile > @@ -61,6 +61,7 @@ obj-$(CONFIG_ORION_WATCHDOG) += orion_wdt.o > obj-$(CONFIG_SUNXI_WATCHDOG) += sunxi_wdt.o > obj-$(CONFIG_RN5T618_WATCHDOG) += rn5t618_wdt.o > obj-$(CONFIG_COH901327_WATCHDOG) += coh901327_wdt.o > +obj-$(CONFIG_NPCM7XX_WATCHDOG) += npcm_wdt.o > obj-$(CONFIG_STMP3XXX_RTC_WATCHDOG) += stmp3xxx_rtc_wdt.o > obj-$(CONFIG_NUC900_WATCHDOG) += nuc900_wdt.o > obj-$(CONFIG_TS4800_WATCHDOG) += ts4800_wdt.o > diff --git a/drivers/watchdog/npcm_wdt.c b/drivers/watchdog/npcm_wdt.c > new file mode 100644 > index 000000000000..71e3d4916514 > --- /dev/null > +++ b/drivers/watchdog/npcm_wdt.c > @@ -0,0 +1,223 @@ > +// SPDX-License-Identifier: GPL-2.0 The SPDX identifier `GPL-2.0` is GPLv2 *only*, your MODULE_LICENSE is "GPLv2 or later". Please make the license info match. > +// Copyright (c) 2018 Nuvoton Technology corporation. > +// Copyright (c) 2018 IBM Corp. > + > +#include <linux/delay.h> > +#include <linux/kernel.h> > +#include <linux/module.h> > +#include <linux/platform_device.h> > +#include <linux/slab.h> > +#include <linux/interrupt.h> > +#include <linux/watchdog.h> > +#include <asm/fiq.h> > +#include <linux/of_irq.h> #include <linux/bitopts.h> Since you are using BIT() See Documentation/process/submit-checklist.rst: 1) If you use a facility then #include the file that defines/declares that facility. Don't depend on other header files pulling in ones that you use. > + > +#define NPCM_WTCR 0x1C > + > +#define NPCM_WTCLK (BIT(10) | BIT(11)) /* Clock divider */ > +#define NPCM_WTE BIT(7) /* Enable */ > +#define NPCM_WTIE BIT(6) /* Enable irq */ > +#define NPCM_WTIS (BIT(4) | BIT(5)) /* Interval selection */ > +#define NPCM_WTIF BIT(3) /* Interrupt flag*/ > +#define NPCM_WTRF BIT(2) /* Reset flag */ > +#define NPCM_WTRE BIT(1) /* Reset enable */ > +#define NPCM_WTR BIT(0) /* Reset counter */ > + > +/* > + * Watchdog timeouts > + * > + * 170 msec: WTCLK=01 WTIS=00 VAL= 0x400 > + * 670 msec: WTCLK=01 WTIS=01 VAL= 0x410 > + * 1360 msec: WTCLK=10 WTIS=00 VAL= 0x800 > + * 2700 msec: WTCLK=01 WTIS=10 VAL= 0x420 > + * 5360 msec: WTCLK=10 WTIS=01 VAL= 0x810 > + * 10700 msec: WTCLK=01 WTIS=11 VAL= 0x430 > + * 21600 msec: WTCLK=10 WTIS=10 VAL= 0x820 > + * 43000 msec: WTCLK=11 WTIS=00 VAL= 0xC00 > + * 85600 msec: WTCLK=10 WTIS=11 VAL= 0x830 > + * 172000 msec: WTCLK=11 WTIS=01 VAL= 0xC10 > + * 687000 msec: WTCLK=11 WTIS=10 VAL= 0xC20 > + * 2750000 msec: WTCLK=11 WTIS=11 VAL= 0xC30 > + */ > + > +struct npcm_wdt { > + struct watchdog_device wdd; > + struct device *dev; > + void __iomem *reg; > +}; > + > +static inline struct npcm_wdt *to_npcm_wdt(struct watchdog_device *wdd) > +{ > + return container_of(wdd, struct npcm_wdt, wdd); > +} > + > +static int npcm_wdt_ping(struct watchdog_device *wdd) > +{ > + struct npcm_wdt *wdt = to_npcm_wdt(wdd); > + u32 val; > + > + val = readl(wdt->reg); > + writel(val | NPCM_WTR, wdt->reg); > + > + return 0; > +} > + > +static int npcm_wdt_start(struct watchdog_device *wdd) > +{ > + struct npcm_wdt *wdt = to_npcm_wdt(wdd); > + u32 val; > + > + val = NPCM_WTRE | NPCM_WTE | NPCM_WTR | NPCM_WTIE; > + > + if (wdd->timeout < 2) > + val |= 0x800; > + else if (wdd->timeout < 3) > + val |= 0x420; > + else if (wdd->timeout < 6) > + val |= 0x810; > + else if (wdd->timeout < 11) > + val |= 0x430; > + else if (wdd->timeout < 22) > + val |= 0x820; > + else if (wdd->timeout < 44) > + val |= 0xC00; > + else if (wdd->timeout < 87) > + val |= 0x830; > + else if (wdd->timeout < 173) > + val |= 0xC10; > + else if (wdd->timeout < 688) > + val |= 0xC20; > + else if (wdd->timeout < 2751) > + val |= 0xC30; > + else > + val |= 0x830; > + > + writel(val, wdt->reg); > + > + return 0; > +} > + > +static int npcm_wdt_stop(struct watchdog_device *wdd) > +{ > + struct npcm_wdt *wdt = to_npcm_wdt(wdd); > + > + writel(0, wdt->reg); > + > + return 0; > +} > + > + > +static int npcm_wdt_set_timeout(struct watchdog_device *wdd, > + unsigned int timeout) > +{ > + wdd->timeout = timeout; /* New timeout */ > + if (watchdog_active(wdd)) > + npcm_wdt_start(wdd); > + > + return 0; > +} > + > +static irqreturn_t npcm_wdt_interrupt(int irq, void *data) > +{ > + struct npcm_wdt *wdt = data; > + > + watchdog_notify_pretimeout(&wdt->wdd); > + > + return IRQ_HANDLED; > +} > + > +static int npcm_wdt_restart(struct watchdog_device *wdd, > + unsigned long action, void *data) > +{ > + struct npcm_wdt *wdt = to_npcm_wdt(wdd); > + > + writel(NPCM_WTR | NPCM_WTRE | NPCM_WTE, wdt->reg); > + udelay(1000); > + > + return 0; > +} > + > +static const struct watchdog_info npcm_wdt_info = { > + .identity = KBUILD_MODNAME, > + .options = WDIOF_SETTIMEOUT > + | WDIOF_KEEPALIVEPING > + | WDIOF_MAGICCLOSE, > +}; > + > +static const struct watchdog_ops npcm_wdt_ops = { > + .owner = THIS_MODULE, > + .start = npcm_wdt_start, > + .stop = npcm_wdt_stop, > + .ping = npcm_wdt_ping, > + .set_timeout = npcm_wdt_set_timeout, > + .restart = npcm_wdt_restart, > +}; > + > +static int npcm_wdt_probe(struct platform_device *pdev) > +{ > + struct device *dev = &pdev->dev; > + struct npcm_wdt *wdt; > + struct resource *res; > + int irq; > + int ret; > + > + wdt = devm_kzalloc(&pdev->dev, sizeof(*wdt), GFP_KERNEL); > + if (!wdt) > + return -ENOMEM; > + > + res = platform_get_resource(pdev, IORESOURCE_MEM, 0); > + wdt->reg = devm_ioremap_resource(dev, res); > + if (IS_ERR(wdt->reg)) > + return PTR_ERR(wdt->reg); > + > + irq = platform_get_irq(pdev, 0); > + if (!irq) > + return -EINVAL; > + > + wdt->dev = dev; > + wdt->wdd.info = &npcm_wdt_info; > + wdt->wdd.ops = &npcm_wdt_ops; > + wdt->wdd.min_timeout = 1; > + wdt->wdd.max_timeout = 2751; > + wdt->wdd.parent = dev; > + > + wdt->wdd.timeout = 86; > + watchdog_init_timeout(&wdt->wdd, 0, dev); watchdog_init_timeout() will also read the `timeout-sec` property from the devicetree if present, which could be a good thing. See drivers/watchdog/watchdog_core.c for implementation. Just put it as an optional property in the dt-bindings. > + > + ret = devm_request_irq(dev, irq, npcm_wdt_interrupt, 0, > + "watchdog", wdt); > + if (ret) > + return ret; > + > + ret = devm_watchdog_register_device(dev, &wdt->wdd); > + if (ret) { > + dev_err(dev, "failed to register watchdog\n"); > + return ret; > + } > + > + platform_set_drvdata(pdev, wdt); Do we need to set drvdata? I don't see that we are using it? > + dev_info(dev, "NPCM watchdog driver enabled\n"); > + > + return 0; > +} > + > +#ifdef CONFIG_OF > +static const struct of_device_id npcm_wdt_match[] = { > + {.compatible = "nuvoton,npcm750-wdt"}, > + {}, > +}; > +MODULE_DEVICE_TABLE(of, npcm_wdt_match); > +#endif > + > +static struct platform_driver npcm_wdt_driver = { > + .probe = npcm_wdt_probe, > + .driver = { > + .name = "npcm-wdt", > + .of_match_table = of_match_ptr(npcm_wdt_match), > + }, > +}; > +module_platform_driver(npcm_wdt_driver); > + > +MODULE_AUTHOR("Joel Stanley"); > +MODULE_DESCRIPTION("Watchdog driver for NPCM"); > +MODULE_LICENSE("GPL"); ^^ Either "GPL v2" here or change SPDX identifier. > -- > 2.15.1 Overall I think it looks good. Thanks, Marcus Folkesson [-- Attachment #2: signature.asc --] [-- Type: application/pgp-signature, Size: 833 bytes --] ^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [PATCH 2/2] watchdog: Add Nuvoton NPCM watchdog driver 2018-03-02 11:20 ` Marcus Folkesson @ 2018-03-05 5:42 ` Joel Stanley 0 siblings, 0 replies; 8+ messages in thread From: Joel Stanley @ 2018-03-05 5:42 UTC (permalink / raw) To: Marcus Folkesson Cc: Wim Van Sebroeck, Guenter Roeck, Rob Herring, Mark Rutland, LINUXWATCHDOG, devicetree, Linux Kernel Mailing List, Tomer Maimon, Avi Fishman, Brendan Higgins On Fri, Mar 2, 2018 at 9:50 PM, Marcus Folkesson <marcus.folkesson@gmail.com> wrote: > Hello Joel! > > On Fri, Mar 02, 2018 at 05:07:46PM +1030, Joel Stanley wrote: >> The Nuvoton NPCM750 has a watchdog implemented as a single register >> inside the timer peripheral. >> >> This driver exposes that watchdog as a standard watchdog device with >> coarse timeout intervals, limited by the combination of prescaler and >> counter that is provided by the hardware. The calculation is taken from >> the Nuvoton vendor tree. >> >> There is a pre-timeout IRQ that is wired up. This timeout always occurs >> 1024 clocks before the timeout. >> >> Signed-off-by: Joel Stanley <joel@jms.id.au> >> --- >> drivers/watchdog/Kconfig | 11 +++ >> drivers/watchdog/Makefile | 1 + >> drivers/watchdog/npcm_wdt.c | 223 ++++++++++++++++++++++++++++++++++++++++++++ >> 3 files changed, 235 insertions(+) >> create mode 100644 drivers/watchdog/npcm_wdt.c >> >> diff --git a/drivers/watchdog/Kconfig b/drivers/watchdog/Kconfig >> index aff773bcebdb..0c1cc68894e6 100644 >> --- a/drivers/watchdog/Kconfig >> +++ b/drivers/watchdog/Kconfig >> @@ -513,6 +513,17 @@ config COH901327_WATCHDOG >> This watchdog is used to reset the system and thus cannot be >> compiled as a module. >> >> +config NPCM7XX_WATCHDOG >> + bool "NPCM750 watchdog" > > Just asking, we do not want to make it possible to build this as a module? The watchdog provides the SoC with a way to reboot itself, so I would expect all users of the kernel to have it built in in case the system cannot find it's initrd or root file system, and therefore is unable to load the watchdog module. I am happy to change it if you would prefer. Thanks for the review. Cheers, Joel ^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [PATCH 2/2] watchdog: Add Nuvoton NPCM watchdog driver 2018-03-02 6:37 ` [PATCH 2/2] watchdog: Add Nuvoton NPCM watchdog driver Joel Stanley 2018-03-02 11:20 ` Marcus Folkesson @ 2018-03-02 13:59 ` Guenter Roeck 2018-03-05 5:43 ` Joel Stanley 1 sibling, 1 reply; 8+ messages in thread From: Guenter Roeck @ 2018-03-02 13:59 UTC (permalink / raw) To: Joel Stanley, Wim Van Sebroeck, Rob Herring, Mark Rutland Cc: linux-watchdog, devicetree, linux-kernel, Tomer Maimon, Avi Fishman, Brendan Higgins On 03/01/2018 10:37 PM, Joel Stanley wrote: > The Nuvoton NPCM750 has a watchdog implemented as a single register > inside the timer peripheral. > > This driver exposes that watchdog as a standard watchdog device with > coarse timeout intervals, limited by the combination of prescaler and > counter that is provided by the hardware. The calculation is taken from > the Nuvoton vendor tree. > > There is a pre-timeout IRQ that is wired up. This timeout always occurs > 1024 clocks before the timeout. > > Signed-off-by: Joel Stanley <joel@jms.id.au> > --- > drivers/watchdog/Kconfig | 11 +++ > drivers/watchdog/Makefile | 1 + > drivers/watchdog/npcm_wdt.c | 223 ++++++++++++++++++++++++++++++++++++++++++++ > 3 files changed, 235 insertions(+) > create mode 100644 drivers/watchdog/npcm_wdt.c > > diff --git a/drivers/watchdog/Kconfig b/drivers/watchdog/Kconfig > index aff773bcebdb..0c1cc68894e6 100644 > --- a/drivers/watchdog/Kconfig > +++ b/drivers/watchdog/Kconfig > @@ -513,6 +513,17 @@ config COH901327_WATCHDOG > This watchdog is used to reset the system and thus cannot be > compiled as a module. > > +config NPCM7XX_WATCHDOG > + bool "NPCM750 watchdog" > + depends on ARCH_NPCM || COMPILE_TEST > + default y if ARCH_NPCM750 > + select WATCHDOG_CORE > + help > + Say Y here to include Watchdog timer support for the > + watchdog embedded into the NPCM7xx. > + This watchdog is used to reset the system and thus cannot be > + compiled as a module. > + > config TWL4030_WATCHDOG > tristate "TWL4030 Watchdog" > depends on TWL4030_CORE > diff --git a/drivers/watchdog/Makefile b/drivers/watchdog/Makefile > index 0474d38aa854..97a5afb5cad2 100644 > --- a/drivers/watchdog/Makefile > +++ b/drivers/watchdog/Makefile > @@ -61,6 +61,7 @@ obj-$(CONFIG_ORION_WATCHDOG) += orion_wdt.o > obj-$(CONFIG_SUNXI_WATCHDOG) += sunxi_wdt.o > obj-$(CONFIG_RN5T618_WATCHDOG) += rn5t618_wdt.o > obj-$(CONFIG_COH901327_WATCHDOG) += coh901327_wdt.o > +obj-$(CONFIG_NPCM7XX_WATCHDOG) += npcm_wdt.o > obj-$(CONFIG_STMP3XXX_RTC_WATCHDOG) += stmp3xxx_rtc_wdt.o > obj-$(CONFIG_NUC900_WATCHDOG) += nuc900_wdt.o > obj-$(CONFIG_TS4800_WATCHDOG) += ts4800_wdt.o > diff --git a/drivers/watchdog/npcm_wdt.c b/drivers/watchdog/npcm_wdt.c > new file mode 100644 > index 000000000000..71e3d4916514 > --- /dev/null > +++ b/drivers/watchdog/npcm_wdt.c > @@ -0,0 +1,223 @@ > +// SPDX-License-Identifier: GPL-2.0 > +// Copyright (c) 2018 Nuvoton Technology corporation. > +// Copyright (c) 2018 IBM Corp. > + > +#include <linux/delay.h> > +#include <linux/kernel.h> > +#include <linux/module.h> > +#include <linux/platform_device.h> > +#include <linux/slab.h> > +#include <linux/interrupt.h> > +#include <linux/watchdog.h> > +#include <asm/fiq.h> > +#include <linux/of_irq.h> Alphabetic order for the linux includes, please. > + > +#define NPCM_WTCR 0x1C > + > +#define NPCM_WTCLK (BIT(10) | BIT(11)) /* Clock divider */ > +#define NPCM_WTE BIT(7) /* Enable */ > +#define NPCM_WTIE BIT(6) /* Enable irq */ > +#define NPCM_WTIS (BIT(4) | BIT(5)) /* Interval selection */ > +#define NPCM_WTIF BIT(3) /* Interrupt flag*/ > +#define NPCM_WTRF BIT(2) /* Reset flag */ > +#define NPCM_WTRE BIT(1) /* Reset enable */ > +#define NPCM_WTR BIT(0) /* Reset counter */ > + > +/* > + * Watchdog timeouts > + * > + * 170 msec: WTCLK=01 WTIS=00 VAL= 0x400 > + * 670 msec: WTCLK=01 WTIS=01 VAL= 0x410 > + * 1360 msec: WTCLK=10 WTIS=00 VAL= 0x800 > + * 2700 msec: WTCLK=01 WTIS=10 VAL= 0x420 > + * 5360 msec: WTCLK=10 WTIS=01 VAL= 0x810 > + * 10700 msec: WTCLK=01 WTIS=11 VAL= 0x430 > + * 21600 msec: WTCLK=10 WTIS=10 VAL= 0x820 > + * 43000 msec: WTCLK=11 WTIS=00 VAL= 0xC00 > + * 85600 msec: WTCLK=10 WTIS=11 VAL= 0x830 > + * 172000 msec: WTCLK=11 WTIS=01 VAL= 0xC10 > + * 687000 msec: WTCLK=11 WTIS=10 VAL= 0xC20 > + * 2750000 msec: WTCLK=11 WTIS=11 VAL= 0xC30 > + */ > + > +struct npcm_wdt { > + struct watchdog_device wdd; > + struct device *dev; Unused > + void __iomem *reg; > +}; > + > +static inline struct npcm_wdt *to_npcm_wdt(struct watchdog_device *wdd) > +{ > + return container_of(wdd, struct npcm_wdt, wdd); > +} > + > +static int npcm_wdt_ping(struct watchdog_device *wdd) > +{ > + struct npcm_wdt *wdt = to_npcm_wdt(wdd); > + u32 val; > + > + val = readl(wdt->reg); > + writel(val | NPCM_WTR, wdt->reg); > + > + return 0; > +} > + > +static int npcm_wdt_start(struct watchdog_device *wdd) > +{ > + struct npcm_wdt *wdt = to_npcm_wdt(wdd); > + u32 val; > + > + val = NPCM_WTRE | NPCM_WTE | NPCM_WTR | NPCM_WTIE; > + > + if (wdd->timeout < 2) > + val |= 0x800; > + else if (wdd->timeout < 3) > + val |= 0x420; > + else if (wdd->timeout < 6) > + val |= 0x810; > + else if (wdd->timeout < 11) > + val |= 0x430; > + else if (wdd->timeout < 22) > + val |= 0x820; > + else if (wdd->timeout < 44) > + val |= 0xC00; > + else if (wdd->timeout < 87) > + val |= 0x830; > + else if (wdd->timeout < 173) > + val |= 0xC10; > + else if (wdd->timeout < 688) > + val |= 0xC20; > + else if (wdd->timeout < 2751) > + val |= 0xC30; > + else > + val |= 0x830; I am lost here. If the requested timeout is 2751, the actual timeout is set to 85.6 seconds. Why ? > + > + writel(val, wdt->reg); > + > + return 0; > +} > + > +static int npcm_wdt_stop(struct watchdog_device *wdd) > +{ > + struct npcm_wdt *wdt = to_npcm_wdt(wdd); > + > + writel(0, wdt->reg); > + > + return 0; > +} > + > + > +static int npcm_wdt_set_timeout(struct watchdog_device *wdd, > + unsigned int timeout) > +{ > + wdd->timeout = timeout; /* New timeout */ This must be set to the actual selected timeout, not to the requested timeout. > + if (watchdog_active(wdd)) > + npcm_wdt_start(wdd); > + > + return 0; > +} > + > +static irqreturn_t npcm_wdt_interrupt(int irq, void *data) > +{ > + struct npcm_wdt *wdt = data; > + > + watchdog_notify_pretimeout(&wdt->wdd); > + > + return IRQ_HANDLED; > +} > + > +static int npcm_wdt_restart(struct watchdog_device *wdd, > + unsigned long action, void *data) > +{ > + struct npcm_wdt *wdt = to_npcm_wdt(wdd); > + > + writel(NPCM_WTR | NPCM_WTRE | NPCM_WTE, wdt->reg); > + udelay(1000); > + > + return 0; > +} > + > +static const struct watchdog_info npcm_wdt_info = { > + .identity = KBUILD_MODNAME, > + .options = WDIOF_SETTIMEOUT > + | WDIOF_KEEPALIVEPING > + | WDIOF_MAGICCLOSE, > +}; > + > +static const struct watchdog_ops npcm_wdt_ops = { > + .owner = THIS_MODULE, > + .start = npcm_wdt_start, > + .stop = npcm_wdt_stop, > + .ping = npcm_wdt_ping, > + .set_timeout = npcm_wdt_set_timeout, > + .restart = npcm_wdt_restart, > +}; > + > +static int npcm_wdt_probe(struct platform_device *pdev) > +{ > + struct device *dev = &pdev->dev; > + struct npcm_wdt *wdt; > + struct resource *res; > + int irq; > + int ret; > + > + wdt = devm_kzalloc(&pdev->dev, sizeof(*wdt), GFP_KERNEL); > + if (!wdt) > + return -ENOMEM; > + > + res = platform_get_resource(pdev, IORESOURCE_MEM, 0); > + wdt->reg = devm_ioremap_resource(dev, res); > + if (IS_ERR(wdt->reg)) > + return PTR_ERR(wdt->reg); > + > + irq = platform_get_irq(pdev, 0); > + if (!irq) > + return -EINVAL; > + > + wdt->dev = dev; > + wdt->wdd.info = &npcm_wdt_info; > + wdt->wdd.ops = &npcm_wdt_ops; > + wdt->wdd.min_timeout = 1; > + wdt->wdd.max_timeout = 2751; > + wdt->wdd.parent = dev; > + > + wdt->wdd.timeout = 86; > + watchdog_init_timeout(&wdt->wdd, 0, dev); > + > + ret = devm_request_irq(dev, irq, npcm_wdt_interrupt, 0, > + "watchdog", wdt); > + if (ret) > + return ret; > + > + ret = devm_watchdog_register_device(dev, &wdt->wdd); > + if (ret) { > + dev_err(dev, "failed to register watchdog\n"); > + return ret; > + } > + > + platform_set_drvdata(pdev, wdt); I don't see where this is used. > + dev_info(dev, "NPCM watchdog driver enabled\n"); > + > + return 0; > +} > + > +#ifdef CONFIG_OF > +static const struct of_device_id npcm_wdt_match[] = { > + {.compatible = "nuvoton,npcm750-wdt"}, > + {}, > +}; > +MODULE_DEVICE_TABLE(of, npcm_wdt_match); > +#endif > + > +static struct platform_driver npcm_wdt_driver = { > + .probe = npcm_wdt_probe, > + .driver = { > + .name = "npcm-wdt", > + .of_match_table = of_match_ptr(npcm_wdt_match), > + }, > +}; > +module_platform_driver(npcm_wdt_driver); > + > +MODULE_AUTHOR("Joel Stanley"); > +MODULE_DESCRIPTION("Watchdog driver for NPCM"); > +MODULE_LICENSE("GPL"); > ^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [PATCH 2/2] watchdog: Add Nuvoton NPCM watchdog driver 2018-03-02 13:59 ` Guenter Roeck @ 2018-03-05 5:43 ` Joel Stanley 0 siblings, 0 replies; 8+ messages in thread From: Joel Stanley @ 2018-03-05 5:43 UTC (permalink / raw) To: Guenter Roeck Cc: Wim Van Sebroeck, Rob Herring, Mark Rutland, LINUXWATCHDOG, devicetree, Linux Kernel Mailing List, Tomer Maimon, Avi Fishman, Brendan Higgins On Sat, Mar 3, 2018 at 12:29 AM, Guenter Roeck <linux@roeck-us.net> wrote: >> +static int npcm_wdt_start(struct watchdog_device *wdd) >> +{ >> + struct npcm_wdt *wdt = to_npcm_wdt(wdd); >> + u32 val; >> + >> + val = NPCM_WTRE | NPCM_WTE | NPCM_WTR | NPCM_WTIE; >> + >> + if (wdd->timeout < 2) >> + val |= 0x800; >> + else if (wdd->timeout < 3) >> + val |= 0x420; >> + else if (wdd->timeout < 6) >> + val |= 0x810; >> + else if (wdd->timeout < 11) >> + val |= 0x430; >> + else if (wdd->timeout < 22) >> + val |= 0x820; >> + else if (wdd->timeout < 44) >> + val |= 0xC00; >> + else if (wdd->timeout < 87) >> + val |= 0x830; >> + else if (wdd->timeout < 173) >> + val |= 0xC10; >> + else if (wdd->timeout < 688) >> + val |= 0xC20; >> + else if (wdd->timeout < 2751) >> + val |= 0xC30; >> + else >> + val |= 0x830; > > > I am lost here. If the requested timeout is 2751, the actual timeout > is set to 85.6 seconds. Why ? > I suspect this was the default, so the original code decided to fall back to this value. I think it would make more sense to use 2750. Cheers, Joel ^ permalink raw reply [flat|nested] 8+ messages in thread
end of thread, other threads:[~2018-03-05 5:43 UTC | newest] Thread overview: 8+ messages (download: mbox.gz / follow: Atom feed) -- links below jump to the message on this page -- 2018-03-02 6:37 [PATCH 0/2] watchdog: Add Nuvoton NPCM (Poleg) driver Joel Stanley 2018-03-02 6:37 ` [PATCH 1/2] dt-bindings: watchdog: Add Nuvoton NPCM description Joel Stanley 2018-03-02 11:24 ` Marcus Folkesson 2018-03-02 6:37 ` [PATCH 2/2] watchdog: Add Nuvoton NPCM watchdog driver Joel Stanley 2018-03-02 11:20 ` Marcus Folkesson 2018-03-05 5:42 ` Joel Stanley 2018-03-02 13:59 ` Guenter Roeck 2018-03-05 5:43 ` Joel Stanley
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox; as well as URLs for NNTP newsgroup(s).