| Message ID | 1462797277-14969-3-git-send-email-joel@jms.id.au (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
Hi Joel, On 05/09/2016 05:34 AM, Joel Stanley wrote: > Provides generic watchdog features as well as reboot support for the > Apeed SoC. > > Signed-off-by: Joel Stanley <joel@jms.id.au> > --- > drivers/watchdog/Kconfig | 13 +++ > drivers/watchdog/Makefile | 1 + > drivers/watchdog/aspeed_wdt.c | 199 ++++++++++++++++++++++++++++++++++++++++++ > 3 files changed, 213 insertions(+) > create mode 100644 drivers/watchdog/aspeed_wdt.c > > diff --git a/drivers/watchdog/Kconfig b/drivers/watchdog/Kconfig > index fb947655badd..4f0e2ded4785 100644 > --- a/drivers/watchdog/Kconfig > +++ b/drivers/watchdog/Kconfig > @@ -661,6 +661,19 @@ config ATLAS7_WATCHDOG > To compile this driver as a module, choose M here: the > module will be called atlas7_wdt. > > +config ASPEED_WATCHDOG > + tristate "Aspeed 2400 watchdog support" > + depends on ARCH_ASPEED || COMPILE_TEST > + select WATCHDOG_CORE > + help > + Say Y here to include support for the watchdog timer > + in Apseed BMC SoCs. > + > + This driver is required to reboot the SoC. > + > + To compile this driver as a module, choose M here: the > + module will be called aspeed_wdt. > + > # AVR32 Architecture > > config AT32AP700X_WDT > diff --git a/drivers/watchdog/Makefile b/drivers/watchdog/Makefile > index feb6270fdbde..36855375e0ce 100644 > --- a/drivers/watchdog/Makefile > +++ b/drivers/watchdog/Makefile > @@ -73,6 +73,7 @@ obj-$(CONFIG_DIGICOLOR_WATCHDOG) += digicolor_wdt.o > obj-$(CONFIG_LPC18XX_WATCHDOG) += lpc18xx_wdt.o > obj-$(CONFIG_BCM7038_WDT) += bcm7038_wdt.o > obj-$(CONFIG_ATLAS7_WATCHDOG) += atlas7_wdt.o > +obj-$(CONFIG_ASPEED_WATCHDOG) += aspeed_wdt.o > > # AVR32 Architecture > obj-$(CONFIG_AT32AP700X_WDT) += at32ap700x_wdt.o > diff --git a/drivers/watchdog/aspeed_wdt.c b/drivers/watchdog/aspeed_wdt.c > new file mode 100644 > index 000000000000..46cc71963c62 > --- /dev/null > +++ b/drivers/watchdog/aspeed_wdt.c > @@ -0,0 +1,199 @@ > +/* > + * Copyright 2016 IBM Corporation > + * > + * Joel Stanley <joel@jms.id.au> > + * > + * Based on the qcom-watchdog driver > + * > + * This program is free software; you can redistribute it and/or > + * modify it under the terms of the GNU General Public License > + * as published by the Free Software Foundation; either version > + * 2 of the License, or (at your option) any later version. > + */ > +#include <linux/kernel.h> > +#include <linux/module.h> > +#include <linux/of.h> > +#include <linux/watchdog.h> > +#include <linux/platform_device.h> > +#include <linux/io.h> > + > +struct aspeed_wdt { > + struct watchdog_device wdd; > + unsigned long rate; > + void __iomem *base; > + u32 ctrl; > +}; > + > +static const struct of_device_id aspeed_wdt_of_table[] = { > + { .compatible = "aspeed,wdt" }, > + { }, > +}; > +MODULE_DEVICE_TABLE(of, aspeed_wdt_of_table); > + > +#define WDT_STATUS 0x00 > +#define WDT_RELOAD_VALUE 0x04 > +#define WDT_RESTART 0x08 > +#define WDT_CTRL 0x0C > +#define WDT_CTRL_RESET_MODE_SOC (0x00 << 5) > +#define WDT_CTRL_RESET_MODE_FULL_CHIP (0x01 << 5) > +#define WDT_CTRL_1MHZ_CLK BIT(4) > +#define WDT_CTRL_WDT_EXT BIT(3) > +#define WDT_CTRL_WDT_INTR BIT(2) > +#define WDT_CTRL_RESET_SYSTEM BIT(1) > +#define WDT_CTRL_ENABLE BIT(0) > + > +#define WDT_RESTART_MAGIC 0x4755 Please don't use tabs after #define. > + > +static struct aspeed_wdt *to_aspeed_wdt(struct watchdog_device *wdd) > +{ > + return container_of(wdd, struct aspeed_wdt, wdd); > +} > + > + Please no double empty lines. > +static void aspeed_wdt_enable(struct aspeed_wdt *wdt, int count) > +{ > + wdt->ctrl |= WDT_CTRL_ENABLE; > + > + writel(0, wdt->base + WDT_CTRL); > + writel(count, wdt->base + WDT_RELOAD_VALUE); > + writel(WDT_RESTART_MAGIC, wdt->base + WDT_RESTART); > + writel(wdt->ctrl, wdt->base + WDT_CTRL); Is it really necessary to write 0 into the ctrl register when enabling the watchdog ? Problem with it is two-fold: It changes the clock rate for a brief period of time, and it disables the watchdog while updating the timeout. Both is undesirable unless really needed. > +} > + > +static int aspeed_wdt_start(struct watchdog_device *wdd) > +{ > + struct aspeed_wdt *wdt = to_aspeed_wdt(wdd); > + > + dev_dbg(wdd->parent, "starting with timeout of %d (rate %lu)\n", > + wdd->timeout, wdt->rate); Are those dev_dbg() really useful ? > + aspeed_wdt_enable(wdt, wdd->timeout * wdt->rate); > + > + return 0; > +} > + > +static int aspeed_wdt_stop(struct watchdog_device *wdd) > +{ > + struct aspeed_wdt *wdt = to_aspeed_wdt(wdd); > + > + wdt->ctrl &= ~WDT_CTRL_ENABLE; > + writel(wdt->ctrl, wdt->base + WDT_CTRL); > + > + return 0; > +} > + > +static int aspeed_wdt_ping(struct watchdog_device *wdd) > +{ > + struct aspeed_wdt *wdt = to_aspeed_wdt(wdd); > + > + dev_dbg(wdd->parent, "ping\n"); > + writel(WDT_RESTART_MAGIC, wdt->base + WDT_RESTART); > + return 0; > +} > + > +static int aspeed_wdt_set_timeout(struct watchdog_device *wdd, > + unsigned int timeout) > +{ > + dev_dbg(wdd->parent, "timeout set to %u\n", timeout); > + wdd->timeout = timeout; > + > + return aspeed_wdt_start(wdd); > +} > + > +static int aspeed_wdt_restart(struct watchdog_device *wdd, > + unsigned long action, void *data) > +{ > + struct aspeed_wdt *wdt = to_aspeed_wdt(wdd); > + > + aspeed_wdt_enable(wdt, 128 * wdt->rate / 1000); > + > + return 0; > +} > + > +static const struct watchdog_ops aspeed_wdt_ops = { > + .start = aspeed_wdt_start, > + .stop = aspeed_wdt_stop, > + .ping = aspeed_wdt_ping, > + .set_timeout = aspeed_wdt_set_timeout, > + .restart = aspeed_wdt_restart, > + .owner = THIS_MODULE, > +}; > + > +static const struct watchdog_info aspeed_wdt_info = { > + .options = WDIOF_KEEPALIVEPING > + | WDIOF_MAGICCLOSE > + | WDIOF_SETTIMEOUT, > + .identity = KBUILD_MODNAME, > +}; > + > +static int aspeed_wdt_remove(struct platform_device *pdev) > +{ > + struct aspeed_wdt *wdt = platform_get_drvdata(pdev); > + > + watchdog_unregister_device(&wdt->wdd); > + > + return 0; > +} > + > +static int aspeed_wdt_probe(struct platform_device *pdev) > +{ > + struct aspeed_wdt *wdt; > + struct resource *res; > + int ret; > + > + wdt = devm_kzalloc(&pdev->dev, sizeof(*wdt), GFP_KERNEL); > + if (!wdt) > + return -ENOMEM; > + > + res = platform_get_resource(pdev, IORESOURCE_MEM, 0); > + wdt->base = devm_ioremap_resource(&pdev->dev, res); > + if (IS_ERR(wdt->base)) > + return PTR_ERR(wdt->base); > + > + /* > + * The ast2400 wdt can run at PCLK, or 1MHz. The ast2500 only > + * runs at 1MHz. We chose to always run at 1MHz, as there's no > + * good reason to have a faster watchdog counter. > + */ > + wdt->rate = 1000000; Why not just use a define ? > + wdt->wdd.info = &aspeed_wdt_info; > + wdt->wdd.ops = &aspeed_wdt_ops; > + wdt->wdd.min_timeout = 1; > + wdt->wdd.max_timeout = 0x10000000U / wdt->rate; > + wdt->wdd.parent = &pdev->dev; > + > + wdt->wdd.timeout = min(wdt->wdd.max_timeout, 30U); Pretty much the same here. Since max_timeout is a constant, it is well known that it is larger than 30 seconds. Might as well use defines for max_timeout and timeout. > + watchdog_init_timeout(&wdt->wdd, 0, &pdev->dev); > + > + wdt->ctrl = WDT_CTRL_RESET_MODE_SOC | Only reset tha SoC ? Are you sure this is what you want ? > + WDT_CTRL_1MHZ_CLK | > + WDT_CTRL_RESET_SYSTEM; > + > + /* Ensure hardware is in consistent state */ > + aspeed_wdt_stop(&wdt->wdd); > + An alternative might be to check if the watchdog is running, and inform the watchdog core about it so it can generate heartbeats as required until user space opens the watchdog device. This would ensure that the system is protected during boot. ctrl is really static except for the enable flag. Not really sure if having a variable for it has any real benefits - you might as well just read the register and update the enable flag instead as needed when starting or stopping the watchdog. Is there a reason for not doing that ? > + ret = watchdog_register_device(&wdt->wdd); > + if (ret) { > + dev_err(&pdev->dev, "failed to register\n"); > + goto err; Unnecessary goto. Just return. > + } > + > + dev_info(&pdev->dev, "rate %lu, max timeout %u, timeout %d\n", > + wdt->rate, wdt->wdd.max_timeout, wdt->wdd.timeout); > + > + platform_set_drvdata(pdev, wdt); > +err: > + return ret; > +} > + > +static struct platform_driver aspeed_watchdog_driver = { > + .probe = aspeed_wdt_probe, > + .remove = aspeed_wdt_remove, > + .driver = { > + .name = KBUILD_MODNAME, > + .of_match_table = of_match_ptr(aspeed_wdt_of_table), > + }, > +}; > +module_platform_driver(aspeed_watchdog_driver); > + > +MODULE_DESCRIPTION("Aspeed Watchdog Driver"); > +MODULE_LICENSE("GPL"); >
Hey Guenter, Thanks for the review. On Tue, May 10, 2016 at 12:18 AM, Guenter Roeck <linux@roeck-us.net> wrote: >> +#define WDT_STATUS 0x00 >> +#define WDT_RELOAD_VALUE 0x04 >> +#define WDT_RESTART 0x08 >> +#define WDT_CTRL 0x0C >> +#define WDT_CTRL_RESET_MODE_SOC (0x00 << 5) >> +#define WDT_CTRL_RESET_MODE_FULL_CHIP (0x01 << 5) >> +#define WDT_CTRL_1MHZ_CLK BIT(4) >> +#define WDT_CTRL_WDT_EXT BIT(3) >> +#define WDT_CTRL_WDT_INTR BIT(2) >> +#define WDT_CTRL_RESET_SYSTEM BIT(1) >> +#define WDT_CTRL_ENABLE BIT(0) >> + >> +#define WDT_RESTART_MAGIC 0x4755 > > > Please don't use tabs after #define. Oops, typo. Thanks. > >> + >> +static struct aspeed_wdt *to_aspeed_wdt(struct watchdog_device *wdd) >> +{ >> + return container_of(wdd, struct aspeed_wdt, wdd); >> +} >> + >> + > > Please no double empty lines. > >> +static void aspeed_wdt_enable(struct aspeed_wdt *wdt, int count) >> +{ >> + wdt->ctrl |= WDT_CTRL_ENABLE; >> + >> + writel(0, wdt->base + WDT_CTRL); >> + writel(count, wdt->base + WDT_RELOAD_VALUE); >> + writel(WDT_RESTART_MAGIC, wdt->base + WDT_RESTART); >> + writel(wdt->ctrl, wdt->base + WDT_CTRL); > > > Is it really necessary to write 0 into the ctrl register when enabling > the watchdog ? Problem with it is two-fold: It changes the clock rate > for a brief period of time, and it disables the watchdog while updating > the timeout. Both is undesirable unless really needed. In general I agree. However, the datasheet explicitly spells out these four steps in this order. Writing zero is a shortcut; I could write ctrl with the enable bit set to zero. We've been using the driver in this fashion for the past year. >> +} >> + >> +static int aspeed_wdt_start(struct watchdog_device *wdd) >> +{ >> + struct aspeed_wdt *wdt = to_aspeed_wdt(wdd); >> + >> + dev_dbg(wdd->parent, "starting with timeout of %d (rate %lu)\n", >> + wdd->timeout, wdt->rate); > > > Are those dev_dbg() really useful ? I did use them in writing the driver. They can go. > >> + aspeed_wdt_enable(wdt, wdd->timeout * wdt->rate); >> + >> + return 0; >> +} >> + >> +static int aspeed_wdt_stop(struct watchdog_device *wdd) >> +{ >> + struct aspeed_wdt *wdt = to_aspeed_wdt(wdd); >> + >> + wdt->ctrl &= ~WDT_CTRL_ENABLE; >> + writel(wdt->ctrl, wdt->base + WDT_CTRL); >> + >> + return 0; >> +} >> + >> +static int aspeed_wdt_ping(struct watchdog_device *wdd) >> +{ >> + struct aspeed_wdt *wdt = to_aspeed_wdt(wdd); >> + >> + dev_dbg(wdd->parent, "ping\n"); >> + writel(WDT_RESTART_MAGIC, wdt->base + WDT_RESTART); >> + return 0; >> +} >> + >> +static int aspeed_wdt_set_timeout(struct watchdog_device *wdd, >> + unsigned int timeout) >> +{ >> + dev_dbg(wdd->parent, "timeout set to %u\n", timeout); >> + wdd->timeout = timeout; >> + >> + return aspeed_wdt_start(wdd); >> +} >> + >> +static int aspeed_wdt_restart(struct watchdog_device *wdd, >> + unsigned long action, void *data) >> +{ >> + struct aspeed_wdt *wdt = to_aspeed_wdt(wdd); >> + >> + aspeed_wdt_enable(wdt, 128 * wdt->rate / 1000); >> + >> + return 0; >> +} >> + >> +static const struct watchdog_ops aspeed_wdt_ops = { >> + .start = aspeed_wdt_start, >> + .stop = aspeed_wdt_stop, >> + .ping = aspeed_wdt_ping, >> + .set_timeout = aspeed_wdt_set_timeout, >> + .restart = aspeed_wdt_restart, >> + .owner = THIS_MODULE, >> +}; >> + >> +static const struct watchdog_info aspeed_wdt_info = { >> + .options = WDIOF_KEEPALIVEPING >> + | WDIOF_MAGICCLOSE >> + | WDIOF_SETTIMEOUT, >> + .identity = KBUILD_MODNAME, >> +}; >> + >> +static int aspeed_wdt_remove(struct platform_device *pdev) >> +{ >> + struct aspeed_wdt *wdt = platform_get_drvdata(pdev); >> + >> + watchdog_unregister_device(&wdt->wdd); >> + >> + return 0; >> +} >> + >> +static int aspeed_wdt_probe(struct platform_device *pdev) >> +{ >> + struct aspeed_wdt *wdt; >> + struct resource *res; >> + int ret; >> + >> + wdt = devm_kzalloc(&pdev->dev, sizeof(*wdt), GFP_KERNEL); >> + if (!wdt) >> + return -ENOMEM; >> + >> + res = platform_get_resource(pdev, IORESOURCE_MEM, 0); >> + wdt->base = devm_ioremap_resource(&pdev->dev, res); >> + if (IS_ERR(wdt->base)) >> + return PTR_ERR(wdt->base); >> + >> + /* >> + * The ast2400 wdt can run at PCLK, or 1MHz. The ast2500 only >> + * runs at 1MHz. We chose to always run at 1MHz, as there's no >> + * good reason to have a faster watchdog counter. >> + */ >> + wdt->rate = 1000000; > > > Why not just use a define ? I will add one. The comment is informative though for people who read the ast2400 datasheet and wonder why we don't provide the option clocking with pclk, I will leave it in. >> + wdt->wdd.info = &aspeed_wdt_info; >> + wdt->wdd.ops = &aspeed_wdt_ops; >> + wdt->wdd.min_timeout = 1; >> + wdt->wdd.max_timeout = 0x10000000U / wdt->rate; >> + wdt->wdd.parent = &pdev->dev; >> + >> + wdt->wdd.timeout = min(wdt->wdd.max_timeout, 30U); > > > Pretty much the same here. Since max_timeout is a constant, > it is well known that it is larger than 30 seconds. > Might as well use defines for max_timeout and timeout. Ok. > >> + watchdog_init_timeout(&wdt->wdd, 0, &pdev->dev); >> + >> + wdt->ctrl = WDT_CTRL_RESET_MODE_SOC | > > > Only reset tha SoC ? Are you sure this is what you want ? I think this is left over from experiments when tracking down a reset bug in u-boot. I will confirm and change it back to FULL_CHIP. > >> + WDT_CTRL_1MHZ_CLK | >> + WDT_CTRL_RESET_SYSTEM; >> + >> + /* Ensure hardware is in consistent state */ >> + aspeed_wdt_stop(&wdt->wdd); >> + > > An alternative might be to check if the watchdog is running, and inform > the watchdog core about it so it can generate heartbeats as required > until user space opens the watchdog device. This would ensure that the > system is protected during boot. I will implement this. > ctrl is really static except for the enable flag. Not really sure > if having a variable for it has any real benefits - you might as well > just read the register and update the enable flag instead as needed when > starting or stopping the watchdog. Is there a reason for not doing that ? Not really. I find it cleaner to keep a copy of the register instead of read-modify-write, but if you feel strongly about it I can change. > >> + ret = watchdog_register_device(&wdt->wdd); >> + if (ret) { >> + dev_err(&pdev->dev, "failed to register\n"); >> + goto err; > > > Unnecessary goto. Just return. Ok. > > >> + } >> + >> + dev_info(&pdev->dev, "rate %lu, max timeout %u, timeout %d\n", >> + wdt->rate, wdt->wdd.max_timeout, wdt->wdd.timeout); >> + >> + platform_set_drvdata(pdev, wdt); >> +err: >> + return ret; >> +} >> + >> +static struct platform_driver aspeed_watchdog_driver = { >> + .probe = aspeed_wdt_probe, >> + .remove = aspeed_wdt_remove, >> + .driver = { >> + .name = KBUILD_MODNAME, >> + .of_match_table = of_match_ptr(aspeed_wdt_of_table), >> + }, >> +}; >> +module_platform_driver(aspeed_watchdog_driver); >> + >> +MODULE_DESCRIPTION("Aspeed Watchdog Driver"); >> +MODULE_LICENSE("GPL"); >> >
Hi Joel, On 05/10/2016 04:10 AM, Joel Stanley wrote: [ ... ] >>> + >>> + /* >>> + * The ast2400 wdt can run at PCLK, or 1MHz. The ast2500 only >>> + * runs at 1MHz. We chose to always run at 1MHz, as there's no >>> + * good reason to have a faster watchdog counter. >>> + */ >>> + wdt->rate = 1000000; >> >> >> Why not just use a define ? > > I will add one. > > The comment is informative though for people who read the ast2400 > datasheet and wonder why we don't provide the option clocking with > pclk, I will leave it in. > The comment is ok. I just find it unnecessary to have a variable instead of a constant. [ ... ] > >> ctrl is really static except for the enable flag. Not really sure >> if having a variable for it has any real benefits - you might as well >> just read the register and update the enable flag instead as needed when >> starting or stopping the watchdog. Is there a reason for not doing that ? > > Not really. I find it cleaner to keep a copy of the register instead > of read-modify-write, but if you feel strongly about it I can change. > No worries. Not worth arguing about. Guenter
diff --git a/drivers/watchdog/Kconfig b/drivers/watchdog/Kconfig index fb947655badd..4f0e2ded4785 100644 --- a/drivers/watchdog/Kconfig +++ b/drivers/watchdog/Kconfig @@ -661,6 +661,19 @@ config ATLAS7_WATCHDOG To compile this driver as a module, choose M here: the module will be called atlas7_wdt. +config ASPEED_WATCHDOG + tristate "Aspeed 2400 watchdog support" + depends on ARCH_ASPEED || COMPILE_TEST + select WATCHDOG_CORE + help + Say Y here to include support for the watchdog timer + in Apseed BMC SoCs. + + This driver is required to reboot the SoC. + + To compile this driver as a module, choose M here: the + module will be called aspeed_wdt. + # AVR32 Architecture config AT32AP700X_WDT diff --git a/drivers/watchdog/Makefile b/drivers/watchdog/Makefile index feb6270fdbde..36855375e0ce 100644 --- a/drivers/watchdog/Makefile +++ b/drivers/watchdog/Makefile @@ -73,6 +73,7 @@ obj-$(CONFIG_DIGICOLOR_WATCHDOG) += digicolor_wdt.o obj-$(CONFIG_LPC18XX_WATCHDOG) += lpc18xx_wdt.o obj-$(CONFIG_BCM7038_WDT) += bcm7038_wdt.o obj-$(CONFIG_ATLAS7_WATCHDOG) += atlas7_wdt.o +obj-$(CONFIG_ASPEED_WATCHDOG) += aspeed_wdt.o # AVR32 Architecture obj-$(CONFIG_AT32AP700X_WDT) += at32ap700x_wdt.o diff --git a/drivers/watchdog/aspeed_wdt.c b/drivers/watchdog/aspeed_wdt.c new file mode 100644 index 000000000000..46cc71963c62 --- /dev/null +++ b/drivers/watchdog/aspeed_wdt.c @@ -0,0 +1,199 @@ +/* + * Copyright 2016 IBM Corporation + * + * Joel Stanley <joel@jms.id.au> + * + * Based on the qcom-watchdog driver + * + * This program is free software; you can redistribute it and/or + * modify it under the terms of the GNU General Public License + * as published by the Free Software Foundation; either version + * 2 of the License, or (at your option) any later version. + */ +#include <linux/kernel.h> +#include <linux/module.h> +#include <linux/of.h> +#include <linux/watchdog.h> +#include <linux/platform_device.h> +#include <linux/io.h> + +struct aspeed_wdt { + struct watchdog_device wdd; + unsigned long rate; + void __iomem *base; + u32 ctrl; +}; + +static const struct of_device_id aspeed_wdt_of_table[] = { + { .compatible = "aspeed,wdt" }, + { }, +}; +MODULE_DEVICE_TABLE(of, aspeed_wdt_of_table); + +#define WDT_STATUS 0x00 +#define WDT_RELOAD_VALUE 0x04 +#define WDT_RESTART 0x08 +#define WDT_CTRL 0x0C +#define WDT_CTRL_RESET_MODE_SOC (0x00 << 5) +#define WDT_CTRL_RESET_MODE_FULL_CHIP (0x01 << 5) +#define WDT_CTRL_1MHZ_CLK BIT(4) +#define WDT_CTRL_WDT_EXT BIT(3) +#define WDT_CTRL_WDT_INTR BIT(2) +#define WDT_CTRL_RESET_SYSTEM BIT(1) +#define WDT_CTRL_ENABLE BIT(0) + +#define WDT_RESTART_MAGIC 0x4755 + +static struct aspeed_wdt *to_aspeed_wdt(struct watchdog_device *wdd) +{ + return container_of(wdd, struct aspeed_wdt, wdd); +} + + +static void aspeed_wdt_enable(struct aspeed_wdt *wdt, int count) +{ + wdt->ctrl |= WDT_CTRL_ENABLE; + + writel(0, wdt->base + WDT_CTRL); + writel(count, wdt->base + WDT_RELOAD_VALUE); + writel(WDT_RESTART_MAGIC, wdt->base + WDT_RESTART); + writel(wdt->ctrl, wdt->base + WDT_CTRL); +} + +static int aspeed_wdt_start(struct watchdog_device *wdd) +{ + struct aspeed_wdt *wdt = to_aspeed_wdt(wdd); + + dev_dbg(wdd->parent, "starting with timeout of %d (rate %lu)\n", + wdd->timeout, wdt->rate); + aspeed_wdt_enable(wdt, wdd->timeout * wdt->rate); + + return 0; +} + +static int aspeed_wdt_stop(struct watchdog_device *wdd) +{ + struct aspeed_wdt *wdt = to_aspeed_wdt(wdd); + + wdt->ctrl &= ~WDT_CTRL_ENABLE; + writel(wdt->ctrl, wdt->base + WDT_CTRL); + + return 0; +} + +static int aspeed_wdt_ping(struct watchdog_device *wdd) +{ + struct aspeed_wdt *wdt = to_aspeed_wdt(wdd); + + dev_dbg(wdd->parent, "ping\n"); + writel(WDT_RESTART_MAGIC, wdt->base + WDT_RESTART); + return 0; +} + +static int aspeed_wdt_set_timeout(struct watchdog_device *wdd, + unsigned int timeout) +{ + dev_dbg(wdd->parent, "timeout set to %u\n", timeout); + wdd->timeout = timeout; + + return aspeed_wdt_start(wdd); +} + +static int aspeed_wdt_restart(struct watchdog_device *wdd, + unsigned long action, void *data) +{ + struct aspeed_wdt *wdt = to_aspeed_wdt(wdd); + + aspeed_wdt_enable(wdt, 128 * wdt->rate / 1000); + + return 0; +} + +static const struct watchdog_ops aspeed_wdt_ops = { + .start = aspeed_wdt_start, + .stop = aspeed_wdt_stop, + .ping = aspeed_wdt_ping, + .set_timeout = aspeed_wdt_set_timeout, + .restart = aspeed_wdt_restart, + .owner = THIS_MODULE, +}; + +static const struct watchdog_info aspeed_wdt_info = { + .options = WDIOF_KEEPALIVEPING + | WDIOF_MAGICCLOSE + | WDIOF_SETTIMEOUT, + .identity = KBUILD_MODNAME, +}; + +static int aspeed_wdt_remove(struct platform_device *pdev) +{ + struct aspeed_wdt *wdt = platform_get_drvdata(pdev); + + watchdog_unregister_device(&wdt->wdd); + + return 0; +} + +static int aspeed_wdt_probe(struct platform_device *pdev) +{ + struct aspeed_wdt *wdt; + struct resource *res; + int ret; + + wdt = devm_kzalloc(&pdev->dev, sizeof(*wdt), GFP_KERNEL); + if (!wdt) + return -ENOMEM; + + res = platform_get_resource(pdev, IORESOURCE_MEM, 0); + wdt->base = devm_ioremap_resource(&pdev->dev, res); + if (IS_ERR(wdt->base)) + return PTR_ERR(wdt->base); + + /* + * The ast2400 wdt can run at PCLK, or 1MHz. The ast2500 only + * runs at 1MHz. We chose to always run at 1MHz, as there's no + * good reason to have a faster watchdog counter. + */ + wdt->rate = 1000000; + wdt->wdd.info = &aspeed_wdt_info; + wdt->wdd.ops = &aspeed_wdt_ops; + wdt->wdd.min_timeout = 1; + wdt->wdd.max_timeout = 0x10000000U / wdt->rate; + wdt->wdd.parent = &pdev->dev; + + wdt->wdd.timeout = min(wdt->wdd.max_timeout, 30U); + watchdog_init_timeout(&wdt->wdd, 0, &pdev->dev); + + wdt->ctrl = WDT_CTRL_RESET_MODE_SOC | + WDT_CTRL_1MHZ_CLK | + WDT_CTRL_RESET_SYSTEM; + + /* Ensure hardware is in consistent state */ + aspeed_wdt_stop(&wdt->wdd); + + ret = watchdog_register_device(&wdt->wdd); + if (ret) { + dev_err(&pdev->dev, "failed to register\n"); + goto err; + } + + dev_info(&pdev->dev, "rate %lu, max timeout %u, timeout %d\n", + wdt->rate, wdt->wdd.max_timeout, wdt->wdd.timeout); + + platform_set_drvdata(pdev, wdt); +err: + return ret; +} + +static struct platform_driver aspeed_watchdog_driver = { + .probe = aspeed_wdt_probe, + .remove = aspeed_wdt_remove, + .driver = { + .name = KBUILD_MODNAME, + .of_match_table = of_match_ptr(aspeed_wdt_of_table), + }, +}; +module_platform_driver(aspeed_watchdog_driver); + +MODULE_DESCRIPTION("Aspeed Watchdog Driver"); +MODULE_LICENSE("GPL");
Provides generic watchdog features as well as reboot support for the Apeed SoC. Signed-off-by: Joel Stanley <joel@jms.id.au> --- drivers/watchdog/Kconfig | 13 +++ drivers/watchdog/Makefile | 1 + drivers/watchdog/aspeed_wdt.c | 199 ++++++++++++++++++++++++++++++++++++++++++ 3 files changed, 213 insertions(+) create mode 100644 drivers/watchdog/aspeed_wdt.c