* [PATCH v2 0/3] watchdog: add support for QCOM WDT @ 2014-09-23 23:04 ` Josh Cartwright 0 siblings, 0 replies; 21+ messages in thread From: Josh Cartwright @ 2014-09-23 23:04 UTC (permalink / raw) To: Wim Van Sebroeck, linux-watchdog, linux-kernel Cc: linux-arm-msm, linux-arm-kernel, Kumar Gala This patchset provides support for the Watchdog Timer (WDT) found in the Krait Processor Sub-system (KPSS) of the MSM8960, APQ8064, and IPQ8064 chips. This driver is implemented ontop of WATCHDOG_CORE, and therefore its primary interface is through userspace. The implemantion is currently very basic (i.e. it doesn't support PRETIMEOUT, even though it could be implemented through the WDT's BARK functionality). It should also be fairly easy to extend this driver in the future to support newer chipsets as well. Patch 3 also extends the driver to also register a restart_notifier, making it possible for the WDT to act as a restart mechanism if more favorable mechanisms don't work. This is important for some boards which don't support PS_HOLD, like the IPQ8064-based AP148 board. Changes since v1: - Make use of clock API instead of using a 'clock-frequency' property - Setup default timeout of 30 seconds when one is not specified - Add remove() function to allow for module unloading - Don't acquire/release watchdog lock on restart - Don't bail completely if restart_handler registration fails Josh Cartwright (3): watchdog: qcom: add support for KPSS WDT watchdog: qcom: document device tree bindings watchdog: qcom: register a restart notifier .../devicetree/bindings/watchdog/qcom-wdt.txt | 22 +++ drivers/watchdog/Kconfig | 13 ++ drivers/watchdog/Makefile | 1 + drivers/watchdog/qcom-wdt.c | 204 +++++++++++++++++++++ 4 files changed, 240 insertions(+) create mode 100644 Documentation/devicetree/bindings/watchdog/qcom-wdt.txt create mode 100644 drivers/watchdog/qcom-wdt.c -- Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum, hosted by The Linux Foundation ^ permalink raw reply [flat|nested] 21+ messages in thread
* [PATCH v2 0/3] watchdog: add support for QCOM WDT @ 2014-09-23 23:04 ` Josh Cartwright 0 siblings, 0 replies; 21+ messages in thread From: Josh Cartwright @ 2014-09-23 23:04 UTC (permalink / raw) To: linux-arm-kernel This patchset provides support for the Watchdog Timer (WDT) found in the Krait Processor Sub-system (KPSS) of the MSM8960, APQ8064, and IPQ8064 chips. This driver is implemented ontop of WATCHDOG_CORE, and therefore its primary interface is through userspace. The implemantion is currently very basic (i.e. it doesn't support PRETIMEOUT, even though it could be implemented through the WDT's BARK functionality). It should also be fairly easy to extend this driver in the future to support newer chipsets as well. Patch 3 also extends the driver to also register a restart_notifier, making it possible for the WDT to act as a restart mechanism if more favorable mechanisms don't work. This is important for some boards which don't support PS_HOLD, like the IPQ8064-based AP148 board. Changes since v1: - Make use of clock API instead of using a 'clock-frequency' property - Setup default timeout of 30 seconds when one is not specified - Add remove() function to allow for module unloading - Don't acquire/release watchdog lock on restart - Don't bail completely if restart_handler registration fails Josh Cartwright (3): watchdog: qcom: add support for KPSS WDT watchdog: qcom: document device tree bindings watchdog: qcom: register a restart notifier .../devicetree/bindings/watchdog/qcom-wdt.txt | 22 +++ drivers/watchdog/Kconfig | 13 ++ drivers/watchdog/Makefile | 1 + drivers/watchdog/qcom-wdt.c | 204 +++++++++++++++++++++ 4 files changed, 240 insertions(+) create mode 100644 Documentation/devicetree/bindings/watchdog/qcom-wdt.txt create mode 100644 drivers/watchdog/qcom-wdt.c -- Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum, hosted by The Linux Foundation ^ permalink raw reply [flat|nested] 21+ messages in thread
* [PATCH v2 1/3] watchdog: qcom: add support for KPSS WDT 2014-09-23 23:04 ` Josh Cartwright @ 2014-09-23 23:04 ` Josh Cartwright -1 siblings, 0 replies; 21+ messages in thread From: Josh Cartwright @ 2014-09-23 23:04 UTC (permalink / raw) To: Wim Van Sebroeck, linux-watchdog, Grant Likely, Rob Herring Cc: linux-arm-msm, linux-arm-kernel, Kumar Gala, linux-kernel, devicetree Add a driver for the watchdog timer block found in the Krait Processor Subsystem (KPSS) on the MSM8960, APQ8064, and IPQ8064. Signed-off-by: Josh Cartwright <joshc@codeaurora.org> --- drivers/watchdog/Kconfig | 13 ++++ drivers/watchdog/Makefile | 1 + drivers/watchdog/qcom-wdt.c | 176 ++++++++++++++++++++++++++++++++++++++++++++ 3 files changed, 190 insertions(+) create mode 100644 drivers/watchdog/qcom-wdt.c diff --git a/drivers/watchdog/Kconfig b/drivers/watchdog/Kconfig index 1d1330a..0479e3f 100644 --- a/drivers/watchdog/Kconfig +++ b/drivers/watchdog/Kconfig @@ -443,6 +443,19 @@ config TEGRA_WATCHDOG To compile this driver as a module, choose M here: the module will be called tegra_wdt. +config QCOM_WDT + tristate "QCOM watchdog" + depends on HAS_IOMEM + depends on ARCH_QCOM + select WATCHDOG_CORE + help + Say Y here to include Watchdog timer support for the watchdog found + on QCOM chipsets. Currently supported targets are the MSM8960, + APQ8064, and IPQ8064. + + To compile this driver as a module, choose M here: the + module will be called qcom_wdt. + # AVR32 Architecture config AT32AP700X_WDT diff --git a/drivers/watchdog/Makefile b/drivers/watchdog/Makefile index 468c320..d645448 100644 --- a/drivers/watchdog/Makefile +++ b/drivers/watchdog/Makefile @@ -57,6 +57,7 @@ obj-$(CONFIG_RETU_WATCHDOG) += retu_wdt.o obj-$(CONFIG_BCM2835_WDT) += bcm2835_wdt.o obj-$(CONFIG_MOXART_WDT) += moxart_wdt.o obj-$(CONFIG_SIRFSOC_WATCHDOG) += sirfsoc_wdt.o +obj-$(CONFIG_QCOM_WDT) += qcom-wdt.o obj-$(CONFIG_BCM_KONA_WDT) += bcm_kona_wdt.o obj-$(CONFIG_TEGRA_WATCHDOG) += tegra_wdt.o diff --git a/drivers/watchdog/qcom-wdt.c b/drivers/watchdog/qcom-wdt.c new file mode 100644 index 0000000..d5e46e2 --- /dev/null +++ b/drivers/watchdog/qcom-wdt.c @@ -0,0 +1,176 @@ +/* Copyright (c) 2014, The Linux Foundation. All rights reserved. + * + * This program is free software; you can redistribute it and/or modify + * it under the terms of the GNU General Public License version 2 and + * only version 2 as published by the Free Software Foundation. + * + * This program is distributed in the hope that it will be useful, + * but WITHOUT ANY WARRANTY; without even the implied warranty of + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the + * GNU General Public License for more details. + * + */ +#include <linux/clk.h> +#include <linux/io.h> +#include <linux/kernel.h> +#include <linux/module.h> +#include <linux/of.h> +#include <linux/platform_device.h> +#include <linux/watchdog.h> + +#define WDT_RST 0x0 +#define WDT_EN 0x8 +#define WDT_BITE_TIME 0x24 + +struct qcom_wdt { + struct watchdog_device wdd; + struct clk *clk; + void __iomem *base; +}; + +static inline +struct qcom_wdt *to_qcom_wdt(struct watchdog_device *wdd) +{ + return container_of(wdd, struct qcom_wdt, wdd); +} + +static int qcom_wdt_start(struct watchdog_device *wdd) +{ + struct qcom_wdt *wdt = to_qcom_wdt(wdd); + unsigned long bite_time; + + bite_time = wdd->timeout * clk_get_rate(wdt->clk); + + writel(0, wdt->base + WDT_EN); + writel(1, wdt->base + WDT_RST); + writel(bite_time, wdt->base + WDT_BITE_TIME); + writel(1, wdt->base + WDT_EN); + return 0; +} + +static int qcom_wdt_stop(struct watchdog_device *wdd) +{ + struct qcom_wdt *wdt = to_qcom_wdt(wdd); + + writel(0, wdt->base + WDT_EN); + return 0; +} + +static int qcom_wdt_ping(struct watchdog_device *wdd) +{ + struct qcom_wdt *wdt = to_qcom_wdt(wdd); + + writel(1, wdt->base + WDT_RST); + return 0; +} + +static int qcom_wdt_set_timeout(struct watchdog_device *wdd, + unsigned int timeout) +{ + wdd->timeout = timeout; + return qcom_wdt_start(wdd); +} + +static const struct watchdog_ops qcom_wdt_ops = { + .start = qcom_wdt_start, + .stop = qcom_wdt_stop, + .ping = qcom_wdt_ping, + .set_timeout = qcom_wdt_set_timeout, + .owner = THIS_MODULE, +}; + +static const struct watchdog_info qcom_wdt_info = { + .options = WDIOF_KEEPALIVEPING + | WDIOF_MAGICCLOSE + | WDIOF_SETTIMEOUT, + .identity = KBUILD_MODNAME, +}; + +static int qcom_wdt_probe(struct platform_device *pdev) +{ + struct qcom_wdt *wdt; + struct resource *res; + unsigned long freq; + 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); + + wdt->clk = devm_clk_get(&pdev->dev, NULL); + if (IS_ERR(wdt->clk)) + return PTR_ERR(wdt->clk); + + ret = clk_prepare_enable(wdt->clk); + if (ret) { + dev_err(&pdev->dev, "failed to setup clock\n"); + return ret; + } + + /* + * We use the clock rate to calculate the max timeout, so ensure it's + * not zero to avoid a divide-by-zero exception. + */ + freq = clk_get_rate(wdt->clk); + if (freq == 0) { + dev_err(&pdev->dev, "invalid clock rate\n"); + return -EINVAL; + } + + wdt->wdd.dev = &pdev->dev; + wdt->wdd.info = &qcom_wdt_info; + wdt->wdd.ops = &qcom_wdt_ops; + wdt->wdd.min_timeout = 1; + wdt->wdd.max_timeout = 0x10000000U / freq; + + /* + * If 'timeout-sec' unspecified in devicetree, assume a 30 second + * default. + */ + if (watchdog_init_timeout(&wdt->wdd, 0, &pdev->dev)) + wdt->wdd.timeout = 30; + + ret = watchdog_register_device(&wdt->wdd); + if (ret) { + dev_err(&pdev->dev, "failed to register watchdog\n"); + return ret; + } + + platform_set_drvdata(pdev, wdt); + return 0; +} + +static int qcom_wdt_remove(struct platform_device *pdev) +{ + struct qcom_wdt *wdt = platform_get_drvdata(pdev); + + watchdog_unregister_device(&wdt->wdd); + clk_disable_unprepare(wdt->clk); + return 0; +} + +static const struct of_device_id qcom_wdt_of_table[] = { + { .compatible = "qcom,kpss-wdt-msm8960", }, + { .compatible = "qcom,kpss-wdt-apq8064", }, + { .compatible = "qcom,kpss-wdt-ipq8064", }, + { }, +}; +MODULE_DEVICE_TABLE(of, qcom_wdt_of_table); + +static struct platform_driver qcom_watchdog_driver = { + .probe = qcom_wdt_probe, + .remove = qcom_wdt_remove, + .driver = { + .name = KBUILD_MODNAME, + .of_match_table = qcom_wdt_of_table, + }, +}; +module_platform_driver(qcom_watchdog_driver); + +MODULE_DESCRIPTION("QCOM KPSS Watchdog Driver"); +MODULE_LICENSE("GPL v2"); -- Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum, hosted by The Linux Foundation ^ permalink raw reply related [flat|nested] 21+ messages in thread
* [PATCH v2 1/3] watchdog: qcom: add support for KPSS WDT @ 2014-09-23 23:04 ` Josh Cartwright 0 siblings, 0 replies; 21+ messages in thread From: Josh Cartwright @ 2014-09-23 23:04 UTC (permalink / raw) To: linux-arm-kernel Add a driver for the watchdog timer block found in the Krait Processor Subsystem (KPSS) on the MSM8960, APQ8064, and IPQ8064. Signed-off-by: Josh Cartwright <joshc@codeaurora.org> --- drivers/watchdog/Kconfig | 13 ++++ drivers/watchdog/Makefile | 1 + drivers/watchdog/qcom-wdt.c | 176 ++++++++++++++++++++++++++++++++++++++++++++ 3 files changed, 190 insertions(+) create mode 100644 drivers/watchdog/qcom-wdt.c diff --git a/drivers/watchdog/Kconfig b/drivers/watchdog/Kconfig index 1d1330a..0479e3f 100644 --- a/drivers/watchdog/Kconfig +++ b/drivers/watchdog/Kconfig @@ -443,6 +443,19 @@ config TEGRA_WATCHDOG To compile this driver as a module, choose M here: the module will be called tegra_wdt. +config QCOM_WDT + tristate "QCOM watchdog" + depends on HAS_IOMEM + depends on ARCH_QCOM + select WATCHDOG_CORE + help + Say Y here to include Watchdog timer support for the watchdog found + on QCOM chipsets. Currently supported targets are the MSM8960, + APQ8064, and IPQ8064. + + To compile this driver as a module, choose M here: the + module will be called qcom_wdt. + # AVR32 Architecture config AT32AP700X_WDT diff --git a/drivers/watchdog/Makefile b/drivers/watchdog/Makefile index 468c320..d645448 100644 --- a/drivers/watchdog/Makefile +++ b/drivers/watchdog/Makefile @@ -57,6 +57,7 @@ obj-$(CONFIG_RETU_WATCHDOG) += retu_wdt.o obj-$(CONFIG_BCM2835_WDT) += bcm2835_wdt.o obj-$(CONFIG_MOXART_WDT) += moxart_wdt.o obj-$(CONFIG_SIRFSOC_WATCHDOG) += sirfsoc_wdt.o +obj-$(CONFIG_QCOM_WDT) += qcom-wdt.o obj-$(CONFIG_BCM_KONA_WDT) += bcm_kona_wdt.o obj-$(CONFIG_TEGRA_WATCHDOG) += tegra_wdt.o diff --git a/drivers/watchdog/qcom-wdt.c b/drivers/watchdog/qcom-wdt.c new file mode 100644 index 0000000..d5e46e2 --- /dev/null +++ b/drivers/watchdog/qcom-wdt.c @@ -0,0 +1,176 @@ +/* Copyright (c) 2014, The Linux Foundation. All rights reserved. + * + * This program is free software; you can redistribute it and/or modify + * it under the terms of the GNU General Public License version 2 and + * only version 2 as published by the Free Software Foundation. + * + * This program is distributed in the hope that it will be useful, + * but WITHOUT ANY WARRANTY; without even the implied warranty of + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the + * GNU General Public License for more details. + * + */ +#include <linux/clk.h> +#include <linux/io.h> +#include <linux/kernel.h> +#include <linux/module.h> +#include <linux/of.h> +#include <linux/platform_device.h> +#include <linux/watchdog.h> + +#define WDT_RST 0x0 +#define WDT_EN 0x8 +#define WDT_BITE_TIME 0x24 + +struct qcom_wdt { + struct watchdog_device wdd; + struct clk *clk; + void __iomem *base; +}; + +static inline +struct qcom_wdt *to_qcom_wdt(struct watchdog_device *wdd) +{ + return container_of(wdd, struct qcom_wdt, wdd); +} + +static int qcom_wdt_start(struct watchdog_device *wdd) +{ + struct qcom_wdt *wdt = to_qcom_wdt(wdd); + unsigned long bite_time; + + bite_time = wdd->timeout * clk_get_rate(wdt->clk); + + writel(0, wdt->base + WDT_EN); + writel(1, wdt->base + WDT_RST); + writel(bite_time, wdt->base + WDT_BITE_TIME); + writel(1, wdt->base + WDT_EN); + return 0; +} + +static int qcom_wdt_stop(struct watchdog_device *wdd) +{ + struct qcom_wdt *wdt = to_qcom_wdt(wdd); + + writel(0, wdt->base + WDT_EN); + return 0; +} + +static int qcom_wdt_ping(struct watchdog_device *wdd) +{ + struct qcom_wdt *wdt = to_qcom_wdt(wdd); + + writel(1, wdt->base + WDT_RST); + return 0; +} + +static int qcom_wdt_set_timeout(struct watchdog_device *wdd, + unsigned int timeout) +{ + wdd->timeout = timeout; + return qcom_wdt_start(wdd); +} + +static const struct watchdog_ops qcom_wdt_ops = { + .start = qcom_wdt_start, + .stop = qcom_wdt_stop, + .ping = qcom_wdt_ping, + .set_timeout = qcom_wdt_set_timeout, + .owner = THIS_MODULE, +}; + +static const struct watchdog_info qcom_wdt_info = { + .options = WDIOF_KEEPALIVEPING + | WDIOF_MAGICCLOSE + | WDIOF_SETTIMEOUT, + .identity = KBUILD_MODNAME, +}; + +static int qcom_wdt_probe(struct platform_device *pdev) +{ + struct qcom_wdt *wdt; + struct resource *res; + unsigned long freq; + 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); + + wdt->clk = devm_clk_get(&pdev->dev, NULL); + if (IS_ERR(wdt->clk)) + return PTR_ERR(wdt->clk); + + ret = clk_prepare_enable(wdt->clk); + if (ret) { + dev_err(&pdev->dev, "failed to setup clock\n"); + return ret; + } + + /* + * We use the clock rate to calculate the max timeout, so ensure it's + * not zero to avoid a divide-by-zero exception. + */ + freq = clk_get_rate(wdt->clk); + if (freq == 0) { + dev_err(&pdev->dev, "invalid clock rate\n"); + return -EINVAL; + } + + wdt->wdd.dev = &pdev->dev; + wdt->wdd.info = &qcom_wdt_info; + wdt->wdd.ops = &qcom_wdt_ops; + wdt->wdd.min_timeout = 1; + wdt->wdd.max_timeout = 0x10000000U / freq; + + /* + * If 'timeout-sec' unspecified in devicetree, assume a 30 second + * default. + */ + if (watchdog_init_timeout(&wdt->wdd, 0, &pdev->dev)) + wdt->wdd.timeout = 30; + + ret = watchdog_register_device(&wdt->wdd); + if (ret) { + dev_err(&pdev->dev, "failed to register watchdog\n"); + return ret; + } + + platform_set_drvdata(pdev, wdt); + return 0; +} + +static int qcom_wdt_remove(struct platform_device *pdev) +{ + struct qcom_wdt *wdt = platform_get_drvdata(pdev); + + watchdog_unregister_device(&wdt->wdd); + clk_disable_unprepare(wdt->clk); + return 0; +} + +static const struct of_device_id qcom_wdt_of_table[] = { + { .compatible = "qcom,kpss-wdt-msm8960", }, + { .compatible = "qcom,kpss-wdt-apq8064", }, + { .compatible = "qcom,kpss-wdt-ipq8064", }, + { }, +}; +MODULE_DEVICE_TABLE(of, qcom_wdt_of_table); + +static struct platform_driver qcom_watchdog_driver = { + .probe = qcom_wdt_probe, + .remove = qcom_wdt_remove, + .driver = { + .name = KBUILD_MODNAME, + .of_match_table = qcom_wdt_of_table, + }, +}; +module_platform_driver(qcom_watchdog_driver); + +MODULE_DESCRIPTION("QCOM KPSS Watchdog Driver"); +MODULE_LICENSE("GPL v2"); -- Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum, hosted by The Linux Foundation ^ permalink raw reply related [flat|nested] 21+ messages in thread
* Re: [PATCH v2 1/3] watchdog: qcom: add support for KPSS WDT 2014-09-23 23:04 ` Josh Cartwright @ 2014-09-24 15:58 ` Guenter Roeck -1 siblings, 0 replies; 21+ messages in thread From: Guenter Roeck @ 2014-09-24 15:58 UTC (permalink / raw) To: Josh Cartwright Cc: Wim Van Sebroeck, linux-watchdog, Grant Likely, Rob Herring, linux-arm-msm, linux-arm-kernel, Kumar Gala, linux-kernel, devicetree On Tue, Sep 23, 2014 at 06:04:36PM -0500, Josh Cartwright wrote: > Add a driver for the watchdog timer block found in the Krait Processor > Subsystem (KPSS) on the MSM8960, APQ8064, and IPQ8064. > > Signed-off-by: Josh Cartwright <joshc@codeaurora.org> Hi Josh, looks much better. Couple of comments inline. Thanks, Guenter > --- > drivers/watchdog/Kconfig | 13 ++++ > drivers/watchdog/Makefile | 1 + > drivers/watchdog/qcom-wdt.c | 176 ++++++++++++++++++++++++++++++++++++++++++++ > 3 files changed, 190 insertions(+) > create mode 100644 drivers/watchdog/qcom-wdt.c > > diff --git a/drivers/watchdog/Kconfig b/drivers/watchdog/Kconfig > index 1d1330a..0479e3f 100644 > --- a/drivers/watchdog/Kconfig > +++ b/drivers/watchdog/Kconfig > @@ -443,6 +443,19 @@ config TEGRA_WATCHDOG > To compile this driver as a module, choose M here: the > module will be called tegra_wdt. > > +config QCOM_WDT > + tristate "QCOM watchdog" > + depends on HAS_IOMEM > + depends on ARCH_QCOM > + select WATCHDOG_CORE > + help > + Say Y here to include Watchdog timer support for the watchdog found > + on QCOM chipsets. Currently supported targets are the MSM8960, > + APQ8064, and IPQ8064. > + > + To compile this driver as a module, choose M here: the > + module will be called qcom_wdt. > + > # AVR32 Architecture > > config AT32AP700X_WDT > diff --git a/drivers/watchdog/Makefile b/drivers/watchdog/Makefile > index 468c320..d645448 100644 > --- a/drivers/watchdog/Makefile > +++ b/drivers/watchdog/Makefile > @@ -57,6 +57,7 @@ obj-$(CONFIG_RETU_WATCHDOG) += retu_wdt.o > obj-$(CONFIG_BCM2835_WDT) += bcm2835_wdt.o > obj-$(CONFIG_MOXART_WDT) += moxart_wdt.o > obj-$(CONFIG_SIRFSOC_WATCHDOG) += sirfsoc_wdt.o > +obj-$(CONFIG_QCOM_WDT) += qcom-wdt.o > obj-$(CONFIG_BCM_KONA_WDT) += bcm_kona_wdt.o > obj-$(CONFIG_TEGRA_WATCHDOG) += tegra_wdt.o > > diff --git a/drivers/watchdog/qcom-wdt.c b/drivers/watchdog/qcom-wdt.c > new file mode 100644 > index 0000000..d5e46e2 > --- /dev/null > +++ b/drivers/watchdog/qcom-wdt.c > @@ -0,0 +1,176 @@ > +/* Copyright (c) 2014, The Linux Foundation. All rights reserved. > + * > + * This program is free software; you can redistribute it and/or modify > + * it under the terms of the GNU General Public License version 2 and > + * only version 2 as published by the Free Software Foundation. > + * > + * This program is distributed in the hope that it will be useful, > + * but WITHOUT ANY WARRANTY; without even the implied warranty of > + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the > + * GNU General Public License for more details. > + * > + */ > +#include <linux/clk.h> > +#include <linux/io.h> > +#include <linux/kernel.h> > +#include <linux/module.h> > +#include <linux/of.h> > +#include <linux/platform_device.h> > +#include <linux/watchdog.h> > + > +#define WDT_RST 0x0 > +#define WDT_EN 0x8 > +#define WDT_BITE_TIME 0x24 > + > +struct qcom_wdt { > + struct watchdog_device wdd; > + struct clk *clk; > + void __iomem *base; > +}; > + > +static inline > +struct qcom_wdt *to_qcom_wdt(struct watchdog_device *wdd) > +{ > + return container_of(wdd, struct qcom_wdt, wdd); > +} > + > +static int qcom_wdt_start(struct watchdog_device *wdd) > +{ > + struct qcom_wdt *wdt = to_qcom_wdt(wdd); > + unsigned long bite_time; > + > + bite_time = wdd->timeout * clk_get_rate(wdt->clk); > + > + writel(0, wdt->base + WDT_EN); > + writel(1, wdt->base + WDT_RST); > + writel(bite_time, wdt->base + WDT_BITE_TIME); > + writel(1, wdt->base + WDT_EN); > + return 0; > +} > + > +static int qcom_wdt_stop(struct watchdog_device *wdd) > +{ > + struct qcom_wdt *wdt = to_qcom_wdt(wdd); > + > + writel(0, wdt->base + WDT_EN); > + return 0; > +} > + > +static int qcom_wdt_ping(struct watchdog_device *wdd) > +{ > + struct qcom_wdt *wdt = to_qcom_wdt(wdd); > + > + writel(1, wdt->base + WDT_RST); > + return 0; > +} > + > +static int qcom_wdt_set_timeout(struct watchdog_device *wdd, > + unsigned int timeout) > +{ > + wdd->timeout = timeout; > + return qcom_wdt_start(wdd); > +} > + > +static const struct watchdog_ops qcom_wdt_ops = { > + .start = qcom_wdt_start, > + .stop = qcom_wdt_stop, > + .ping = qcom_wdt_ping, > + .set_timeout = qcom_wdt_set_timeout, > + .owner = THIS_MODULE, > +}; > + > +static const struct watchdog_info qcom_wdt_info = { > + .options = WDIOF_KEEPALIVEPING > + | WDIOF_MAGICCLOSE > + | WDIOF_SETTIMEOUT, > + .identity = KBUILD_MODNAME, > +}; > + > +static int qcom_wdt_probe(struct platform_device *pdev) > +{ > + struct qcom_wdt *wdt; > + struct resource *res; > + unsigned long freq; > + 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); > + > + wdt->clk = devm_clk_get(&pdev->dev, NULL); > + if (IS_ERR(wdt->clk)) > + return PTR_ERR(wdt->clk); > + > + ret = clk_prepare_enable(wdt->clk); > + if (ret) { > + dev_err(&pdev->dev, "failed to setup clock\n"); > + return ret; > + } > + > + /* > + * We use the clock rate to calculate the max timeout, so ensure it's > + * not zero to avoid a divide-by-zero exception. > + */ > + freq = clk_get_rate(wdt->clk); > + if (freq == 0) { > + dev_err(&pdev->dev, "invalid clock rate\n"); > + return -EINVAL; > + } This will need clk_disable_unprepare(). Since you are reading the frequency here, it might make sense to store it in struct qcom_wdt so you don't have to call clk_get_rate() again in the start function. > + > + wdt->wdd.dev = &pdev->dev; > + wdt->wdd.info = &qcom_wdt_info; > + wdt->wdd.ops = &qcom_wdt_ops; > + wdt->wdd.min_timeout = 1; > + wdt->wdd.max_timeout = 0x10000000U / freq; What if the frequency turns out to be larger than 8947848 Hz ? Then your maximum timeout is below the default timeout. And if the frequency is larger than 268435456 Hz, the maximum timeout would be 0. > + > + /* > + * If 'timeout-sec' unspecified in devicetree, assume a 30 second > + * default. > + */ > + if (watchdog_init_timeout(&wdt->wdd, 0, &pdev->dev)) > + wdt->wdd.timeout = 30; You can just initialize timeout above with 30 seconds. Saves you the if statement here. > + > + ret = watchdog_register_device(&wdt->wdd); > + if (ret) { > + dev_err(&pdev->dev, "failed to register watchdog\n"); This will need a clk_disable_unprepare(). Given that this is needed twice, you might want to consider using error exit code below, as suggested in CodingStyle. > + return ret; > + } > + > + platform_set_drvdata(pdev, wdt); > + return 0; > +} > + > +static int qcom_wdt_remove(struct platform_device *pdev) > +{ > + struct qcom_wdt *wdt = platform_get_drvdata(pdev); > + > + watchdog_unregister_device(&wdt->wdd); > + clk_disable_unprepare(wdt->clk); > + return 0; > +} > + > +static const struct of_device_id qcom_wdt_of_table[] = { > + { .compatible = "qcom,kpss-wdt-msm8960", }, > + { .compatible = "qcom,kpss-wdt-apq8064", }, > + { .compatible = "qcom,kpss-wdt-ipq8064", }, > + { }, > +}; > +MODULE_DEVICE_TABLE(of, qcom_wdt_of_table); > + > +static struct platform_driver qcom_watchdog_driver = { > + .probe = qcom_wdt_probe, > + .remove = qcom_wdt_remove, > + .driver = { > + .name = KBUILD_MODNAME, > + .of_match_table = qcom_wdt_of_table, > + }, > +}; > +module_platform_driver(qcom_watchdog_driver); > + > +MODULE_DESCRIPTION("QCOM KPSS Watchdog Driver"); > +MODULE_LICENSE("GPL v2"); > -- > Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum, > hosted by The Linux Foundation > > -- > To unsubscribe from this list: send the line "unsubscribe linux-watchdog" in > the body of a message to majordomo@vger.kernel.org > More majordomo info at http://vger.kernel.org/majordomo-info.html ^ permalink raw reply [flat|nested] 21+ messages in thread
* [PATCH v2 1/3] watchdog: qcom: add support for KPSS WDT @ 2014-09-24 15:58 ` Guenter Roeck 0 siblings, 0 replies; 21+ messages in thread From: Guenter Roeck @ 2014-09-24 15:58 UTC (permalink / raw) To: linux-arm-kernel On Tue, Sep 23, 2014 at 06:04:36PM -0500, Josh Cartwright wrote: > Add a driver for the watchdog timer block found in the Krait Processor > Subsystem (KPSS) on the MSM8960, APQ8064, and IPQ8064. > > Signed-off-by: Josh Cartwright <joshc@codeaurora.org> Hi Josh, looks much better. Couple of comments inline. Thanks, Guenter > --- > drivers/watchdog/Kconfig | 13 ++++ > drivers/watchdog/Makefile | 1 + > drivers/watchdog/qcom-wdt.c | 176 ++++++++++++++++++++++++++++++++++++++++++++ > 3 files changed, 190 insertions(+) > create mode 100644 drivers/watchdog/qcom-wdt.c > > diff --git a/drivers/watchdog/Kconfig b/drivers/watchdog/Kconfig > index 1d1330a..0479e3f 100644 > --- a/drivers/watchdog/Kconfig > +++ b/drivers/watchdog/Kconfig > @@ -443,6 +443,19 @@ config TEGRA_WATCHDOG > To compile this driver as a module, choose M here: the > module will be called tegra_wdt. > > +config QCOM_WDT > + tristate "QCOM watchdog" > + depends on HAS_IOMEM > + depends on ARCH_QCOM > + select WATCHDOG_CORE > + help > + Say Y here to include Watchdog timer support for the watchdog found > + on QCOM chipsets. Currently supported targets are the MSM8960, > + APQ8064, and IPQ8064. > + > + To compile this driver as a module, choose M here: the > + module will be called qcom_wdt. > + > # AVR32 Architecture > > config AT32AP700X_WDT > diff --git a/drivers/watchdog/Makefile b/drivers/watchdog/Makefile > index 468c320..d645448 100644 > --- a/drivers/watchdog/Makefile > +++ b/drivers/watchdog/Makefile > @@ -57,6 +57,7 @@ obj-$(CONFIG_RETU_WATCHDOG) += retu_wdt.o > obj-$(CONFIG_BCM2835_WDT) += bcm2835_wdt.o > obj-$(CONFIG_MOXART_WDT) += moxart_wdt.o > obj-$(CONFIG_SIRFSOC_WATCHDOG) += sirfsoc_wdt.o > +obj-$(CONFIG_QCOM_WDT) += qcom-wdt.o > obj-$(CONFIG_BCM_KONA_WDT) += bcm_kona_wdt.o > obj-$(CONFIG_TEGRA_WATCHDOG) += tegra_wdt.o > > diff --git a/drivers/watchdog/qcom-wdt.c b/drivers/watchdog/qcom-wdt.c > new file mode 100644 > index 0000000..d5e46e2 > --- /dev/null > +++ b/drivers/watchdog/qcom-wdt.c > @@ -0,0 +1,176 @@ > +/* Copyright (c) 2014, The Linux Foundation. All rights reserved. > + * > + * This program is free software; you can redistribute it and/or modify > + * it under the terms of the GNU General Public License version 2 and > + * only version 2 as published by the Free Software Foundation. > + * > + * This program is distributed in the hope that it will be useful, > + * but WITHOUT ANY WARRANTY; without even the implied warranty of > + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the > + * GNU General Public License for more details. > + * > + */ > +#include <linux/clk.h> > +#include <linux/io.h> > +#include <linux/kernel.h> > +#include <linux/module.h> > +#include <linux/of.h> > +#include <linux/platform_device.h> > +#include <linux/watchdog.h> > + > +#define WDT_RST 0x0 > +#define WDT_EN 0x8 > +#define WDT_BITE_TIME 0x24 > + > +struct qcom_wdt { > + struct watchdog_device wdd; > + struct clk *clk; > + void __iomem *base; > +}; > + > +static inline > +struct qcom_wdt *to_qcom_wdt(struct watchdog_device *wdd) > +{ > + return container_of(wdd, struct qcom_wdt, wdd); > +} > + > +static int qcom_wdt_start(struct watchdog_device *wdd) > +{ > + struct qcom_wdt *wdt = to_qcom_wdt(wdd); > + unsigned long bite_time; > + > + bite_time = wdd->timeout * clk_get_rate(wdt->clk); > + > + writel(0, wdt->base + WDT_EN); > + writel(1, wdt->base + WDT_RST); > + writel(bite_time, wdt->base + WDT_BITE_TIME); > + writel(1, wdt->base + WDT_EN); > + return 0; > +} > + > +static int qcom_wdt_stop(struct watchdog_device *wdd) > +{ > + struct qcom_wdt *wdt = to_qcom_wdt(wdd); > + > + writel(0, wdt->base + WDT_EN); > + return 0; > +} > + > +static int qcom_wdt_ping(struct watchdog_device *wdd) > +{ > + struct qcom_wdt *wdt = to_qcom_wdt(wdd); > + > + writel(1, wdt->base + WDT_RST); > + return 0; > +} > + > +static int qcom_wdt_set_timeout(struct watchdog_device *wdd, > + unsigned int timeout) > +{ > + wdd->timeout = timeout; > + return qcom_wdt_start(wdd); > +} > + > +static const struct watchdog_ops qcom_wdt_ops = { > + .start = qcom_wdt_start, > + .stop = qcom_wdt_stop, > + .ping = qcom_wdt_ping, > + .set_timeout = qcom_wdt_set_timeout, > + .owner = THIS_MODULE, > +}; > + > +static const struct watchdog_info qcom_wdt_info = { > + .options = WDIOF_KEEPALIVEPING > + | WDIOF_MAGICCLOSE > + | WDIOF_SETTIMEOUT, > + .identity = KBUILD_MODNAME, > +}; > + > +static int qcom_wdt_probe(struct platform_device *pdev) > +{ > + struct qcom_wdt *wdt; > + struct resource *res; > + unsigned long freq; > + 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); > + > + wdt->clk = devm_clk_get(&pdev->dev, NULL); > + if (IS_ERR(wdt->clk)) > + return PTR_ERR(wdt->clk); > + > + ret = clk_prepare_enable(wdt->clk); > + if (ret) { > + dev_err(&pdev->dev, "failed to setup clock\n"); > + return ret; > + } > + > + /* > + * We use the clock rate to calculate the max timeout, so ensure it's > + * not zero to avoid a divide-by-zero exception. > + */ > + freq = clk_get_rate(wdt->clk); > + if (freq == 0) { > + dev_err(&pdev->dev, "invalid clock rate\n"); > + return -EINVAL; > + } This will need clk_disable_unprepare(). Since you are reading the frequency here, it might make sense to store it in struct qcom_wdt so you don't have to call clk_get_rate() again in the start function. > + > + wdt->wdd.dev = &pdev->dev; > + wdt->wdd.info = &qcom_wdt_info; > + wdt->wdd.ops = &qcom_wdt_ops; > + wdt->wdd.min_timeout = 1; > + wdt->wdd.max_timeout = 0x10000000U / freq; What if the frequency turns out to be larger than 8947848 Hz ? Then your maximum timeout is below the default timeout. And if the frequency is larger than 268435456 Hz, the maximum timeout would be 0. > + > + /* > + * If 'timeout-sec' unspecified in devicetree, assume a 30 second > + * default. > + */ > + if (watchdog_init_timeout(&wdt->wdd, 0, &pdev->dev)) > + wdt->wdd.timeout = 30; You can just initialize timeout above with 30 seconds. Saves you the if statement here. > + > + ret = watchdog_register_device(&wdt->wdd); > + if (ret) { > + dev_err(&pdev->dev, "failed to register watchdog\n"); This will need a clk_disable_unprepare(). Given that this is needed twice, you might want to consider using error exit code below, as suggested in CodingStyle. > + return ret; > + } > + > + platform_set_drvdata(pdev, wdt); > + return 0; > +} > + > +static int qcom_wdt_remove(struct platform_device *pdev) > +{ > + struct qcom_wdt *wdt = platform_get_drvdata(pdev); > + > + watchdog_unregister_device(&wdt->wdd); > + clk_disable_unprepare(wdt->clk); > + return 0; > +} > + > +static const struct of_device_id qcom_wdt_of_table[] = { > + { .compatible = "qcom,kpss-wdt-msm8960", }, > + { .compatible = "qcom,kpss-wdt-apq8064", }, > + { .compatible = "qcom,kpss-wdt-ipq8064", }, > + { }, > +}; > +MODULE_DEVICE_TABLE(of, qcom_wdt_of_table); > + > +static struct platform_driver qcom_watchdog_driver = { > + .probe = qcom_wdt_probe, > + .remove = qcom_wdt_remove, > + .driver = { > + .name = KBUILD_MODNAME, > + .of_match_table = qcom_wdt_of_table, > + }, > +}; > +module_platform_driver(qcom_watchdog_driver); > + > +MODULE_DESCRIPTION("QCOM KPSS Watchdog Driver"); > +MODULE_LICENSE("GPL v2"); > -- > Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum, > hosted by The Linux Foundation > > -- > To unsubscribe from this list: send the line "unsubscribe linux-watchdog" in > the body of a message to majordomo at vger.kernel.org > More majordomo info at http://vger.kernel.org/majordomo-info.html ^ permalink raw reply [flat|nested] 21+ messages in thread
* Re: [PATCH v2 1/3] watchdog: qcom: add support for KPSS WDT 2014-09-24 15:58 ` Guenter Roeck @ 2014-09-24 18:30 ` Josh Cartwright -1 siblings, 0 replies; 21+ messages in thread From: Josh Cartwright @ 2014-09-24 18:30 UTC (permalink / raw) To: Guenter Roeck Cc: Wim Van Sebroeck, linux-watchdog, Grant Likely, Rob Herring, linux-arm-msm, linux-arm-kernel, Kumar Gala, linux-kernel, devicetree On Wed, Sep 24, 2014 at 08:58:54AM -0700, Guenter Roeck wrote: > On Tue, Sep 23, 2014 at 06:04:36PM -0500, Josh Cartwright wrote: > > Add a driver for the watchdog timer block found in the Krait Processor > > Subsystem (KPSS) on the MSM8960, APQ8064, and IPQ8064. > > > > Signed-off-by: Josh Cartwright <joshc@codeaurora.org> > > Hi Josh, > > looks much better. Couple of comments inline. Thanks for another review! [..] > > +++ b/drivers/watchdog/Kconfig > > @@ -443,6 +443,19 @@ config TEGRA_WATCHDOG > > To compile this driver as a module, choose M here: the > > module will be called tegra_wdt. > > > > +config QCOM_WDT > > + tristate "QCOM watchdog" > > + depends on HAS_IOMEM > > + depends on ARCH_QCOM > > + select WATCHDOG_CORE > > + help > > + Say Y here to include Watchdog timer support for the watchdog found > > + on QCOM chipsets. Currently supported targets are the MSM8960, > > + APQ8064, and IPQ8064. > > + > > + To compile this driver as a module, choose M here: the > > + module will be called qcom_wdt. > > + > > # AVR32 Architecture [..] > > +static int qcom_wdt_probe(struct platform_device *pdev) > > +{ > > + struct qcom_wdt *wdt; > > + struct resource *res; > > + unsigned long freq; > > + 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); > > + > > + wdt->clk = devm_clk_get(&pdev->dev, NULL); > > + if (IS_ERR(wdt->clk)) > > + return PTR_ERR(wdt->clk); > > + > > + ret = clk_prepare_enable(wdt->clk); > > + if (ret) { > > + dev_err(&pdev->dev, "failed to setup clock\n"); > > + return ret; > > + } > > + > > + /* > > + * We use the clock rate to calculate the max timeout, so ensure it's > > + * not zero to avoid a divide-by-zero exception. > > + */ > > + freq = clk_get_rate(wdt->clk); > > + if (freq == 0) { > > + dev_err(&pdev->dev, "invalid clock rate\n"); > > + return -EINVAL; > > + } > > This will need clk_disable_unprepare(). Yep. Nice catch. Will fix. > Since you are reading the frequency here, it might make sense to store it > in struct qcom_wdt so you don't have to call clk_get_rate() again in the > start function. Yeah, it doesn't save much, but I'll go ahead and add it. > > + wdt->wdd.dev = &pdev->dev; > > + wdt->wdd.info = &qcom_wdt_info; > > + wdt->wdd.ops = &qcom_wdt_ops; > > + wdt->wdd.min_timeout = 1; > > + wdt->wdd.max_timeout = 0x10000000U / freq; > > What if the frequency turns out to be larger than 8947848 Hz ? > Then your maximum timeout is below the default timeout. > And if the frequency is larger than 268435456 Hz, the maximum > timeout would be 0. Yes, I should be doing more sanity checking. I'll do so. > > + /* > > + * If 'timeout-sec' unspecified in devicetree, assume a 30 second > > + * default. > > + */ > > + if (watchdog_init_timeout(&wdt->wdd, 0, &pdev->dev)) > > + wdt->wdd.timeout = 30; > > You can just initialize timeout above with 30 seconds. Saves you the if > statement here. Great. Thanks. > > + ret = watchdog_register_device(&wdt->wdd); > > + if (ret) { > > + dev_err(&pdev->dev, "failed to register watchdog\n"); > > This will need a clk_disable_unprepare(). > > Given that this is needed twice, you might want to consider using > error exit code below, as suggested in CodingStyle. Indeed. Will do. Thanks again, Josh -- Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum, hosted by The Linux Foundation ^ permalink raw reply [flat|nested] 21+ messages in thread
* [PATCH v2 1/3] watchdog: qcom: add support for KPSS WDT @ 2014-09-24 18:30 ` Josh Cartwright 0 siblings, 0 replies; 21+ messages in thread From: Josh Cartwright @ 2014-09-24 18:30 UTC (permalink / raw) To: linux-arm-kernel On Wed, Sep 24, 2014 at 08:58:54AM -0700, Guenter Roeck wrote: > On Tue, Sep 23, 2014 at 06:04:36PM -0500, Josh Cartwright wrote: > > Add a driver for the watchdog timer block found in the Krait Processor > > Subsystem (KPSS) on the MSM8960, APQ8064, and IPQ8064. > > > > Signed-off-by: Josh Cartwright <joshc@codeaurora.org> > > Hi Josh, > > looks much better. Couple of comments inline. Thanks for another review! [..] > > +++ b/drivers/watchdog/Kconfig > > @@ -443,6 +443,19 @@ config TEGRA_WATCHDOG > > To compile this driver as a module, choose M here: the > > module will be called tegra_wdt. > > > > +config QCOM_WDT > > + tristate "QCOM watchdog" > > + depends on HAS_IOMEM > > + depends on ARCH_QCOM > > + select WATCHDOG_CORE > > + help > > + Say Y here to include Watchdog timer support for the watchdog found > > + on QCOM chipsets. Currently supported targets are the MSM8960, > > + APQ8064, and IPQ8064. > > + > > + To compile this driver as a module, choose M here: the > > + module will be called qcom_wdt. > > + > > # AVR32 Architecture [..] > > +static int qcom_wdt_probe(struct platform_device *pdev) > > +{ > > + struct qcom_wdt *wdt; > > + struct resource *res; > > + unsigned long freq; > > + 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); > > + > > + wdt->clk = devm_clk_get(&pdev->dev, NULL); > > + if (IS_ERR(wdt->clk)) > > + return PTR_ERR(wdt->clk); > > + > > + ret = clk_prepare_enable(wdt->clk); > > + if (ret) { > > + dev_err(&pdev->dev, "failed to setup clock\n"); > > + return ret; > > + } > > + > > + /* > > + * We use the clock rate to calculate the max timeout, so ensure it's > > + * not zero to avoid a divide-by-zero exception. > > + */ > > + freq = clk_get_rate(wdt->clk); > > + if (freq == 0) { > > + dev_err(&pdev->dev, "invalid clock rate\n"); > > + return -EINVAL; > > + } > > This will need clk_disable_unprepare(). Yep. Nice catch. Will fix. > Since you are reading the frequency here, it might make sense to store it > in struct qcom_wdt so you don't have to call clk_get_rate() again in the > start function. Yeah, it doesn't save much, but I'll go ahead and add it. > > + wdt->wdd.dev = &pdev->dev; > > + wdt->wdd.info = &qcom_wdt_info; > > + wdt->wdd.ops = &qcom_wdt_ops; > > + wdt->wdd.min_timeout = 1; > > + wdt->wdd.max_timeout = 0x10000000U / freq; > > What if the frequency turns out to be larger than 8947848 Hz ? > Then your maximum timeout is below the default timeout. > And if the frequency is larger than 268435456 Hz, the maximum > timeout would be 0. Yes, I should be doing more sanity checking. I'll do so. > > + /* > > + * If 'timeout-sec' unspecified in devicetree, assume a 30 second > > + * default. > > + */ > > + if (watchdog_init_timeout(&wdt->wdd, 0, &pdev->dev)) > > + wdt->wdd.timeout = 30; > > You can just initialize timeout above with 30 seconds. Saves you the if > statement here. Great. Thanks. > > + ret = watchdog_register_device(&wdt->wdd); > > + if (ret) { > > + dev_err(&pdev->dev, "failed to register watchdog\n"); > > This will need a clk_disable_unprepare(). > > Given that this is needed twice, you might want to consider using > error exit code below, as suggested in CodingStyle. Indeed. Will do. Thanks again, Josh -- Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum, hosted by The Linux Foundation ^ permalink raw reply [flat|nested] 21+ messages in thread
[parent not found: <cover.1411513109.git.joshc-sgV2jX0FEOL9JmXXK+q4OQ@public.gmane.org>]
* [PATCH v2 2/3] watchdog: qcom: document device tree bindings 2014-09-23 23:04 ` Josh Cartwright (?) @ 2014-09-23 23:04 ` Josh Cartwright -1 siblings, 0 replies; 21+ messages in thread From: Josh Cartwright @ 2014-09-23 23:04 UTC (permalink / raw) To: Wim Van Sebroeck, linux-watchdog-u79uwXL29TY76Z2rM5mHXA, linux-kernel-u79uwXL29TY76Z2rM5mHXA Cc: linux-arm-msm-u79uwXL29TY76Z2rM5mHXA, linux-arm-kernel-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r, Kumar Gala, Rob Herring, Pawel Moll, Mark Rutland, Ian Campbell, devicetree-u79uwXL29TY76Z2rM5mHXA The Qualcomm Krait Processor Sub-system (KPSS) contains one or more instances of the WDT. Provide documentation on how to describe these in the device tree. Signed-off-by: Josh Cartwright <joshc-sgV2jX0FEOL9JmXXK+q4OQ@public.gmane.org> --- .../devicetree/bindings/watchdog/qcom-wdt.txt | 22 ++++++++++++++++++++++ 1 file changed, 22 insertions(+) create mode 100644 Documentation/devicetree/bindings/watchdog/qcom-wdt.txt diff --git a/Documentation/devicetree/bindings/watchdog/qcom-wdt.txt b/Documentation/devicetree/bindings/watchdog/qcom-wdt.txt new file mode 100644 index 0000000..6a1d758 --- /dev/null +++ b/Documentation/devicetree/bindings/watchdog/qcom-wdt.txt @@ -0,0 +1,22 @@ +Qualcomm Krait Processor Sub-system (KPSS) Watchdog +--------------------------------------------------- + +Required properties : +- compatible : shall contain only one of the following: + + "qcom,kpss-wdt-msm8960" + "qcom,kpss-wdt-apq8064" + "qcom,kpss-wdt-ipq8064" + +- reg : shall contain base register location and length +- clocks : shall contain the input clock phandle +- timeout-sec : shall contain the default watchdog timeout in seconds, + if unset, the default timeout is 30 seconds + +Example: + watchdog@208a038 { + compatible = "qcom,kpss-wdt-ipq8064"; + reg = <0x0208a038 0x40>; + clocks = <&sleep_clk>; + timeout-sec = <10>; + }; -- Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum, hosted by The Linux Foundation -- To unsubscribe from this list: send the line "unsubscribe devicetree" in the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org More majordomo info at http://vger.kernel.org/majordomo-info.html ^ permalink raw reply related [flat|nested] 21+ messages in thread
* [PATCH v2 2/3] watchdog: qcom: document device tree bindings @ 2014-09-23 23:04 ` Josh Cartwright 0 siblings, 0 replies; 21+ messages in thread From: Josh Cartwright @ 2014-09-23 23:04 UTC (permalink / raw) To: linux-arm-kernel The Qualcomm Krait Processor Sub-system (KPSS) contains one or more instances of the WDT. Provide documentation on how to describe these in the device tree. Signed-off-by: Josh Cartwright <joshc@codeaurora.org> --- .../devicetree/bindings/watchdog/qcom-wdt.txt | 22 ++++++++++++++++++++++ 1 file changed, 22 insertions(+) create mode 100644 Documentation/devicetree/bindings/watchdog/qcom-wdt.txt diff --git a/Documentation/devicetree/bindings/watchdog/qcom-wdt.txt b/Documentation/devicetree/bindings/watchdog/qcom-wdt.txt new file mode 100644 index 0000000..6a1d758 --- /dev/null +++ b/Documentation/devicetree/bindings/watchdog/qcom-wdt.txt @@ -0,0 +1,22 @@ +Qualcomm Krait Processor Sub-system (KPSS) Watchdog +--------------------------------------------------- + +Required properties : +- compatible : shall contain only one of the following: + + "qcom,kpss-wdt-msm8960" + "qcom,kpss-wdt-apq8064" + "qcom,kpss-wdt-ipq8064" + +- reg : shall contain base register location and length +- clocks : shall contain the input clock phandle +- timeout-sec : shall contain the default watchdog timeout in seconds, + if unset, the default timeout is 30 seconds + +Example: + watchdog at 208a038 { + compatible = "qcom,kpss-wdt-ipq8064"; + reg = <0x0208a038 0x40>; + clocks = <&sleep_clk>; + timeout-sec = <10>; + }; -- Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum, hosted by The Linux Foundation ^ permalink raw reply related [flat|nested] 21+ messages in thread
* [PATCH v2 2/3] watchdog: qcom: document device tree bindings @ 2014-09-23 23:04 ` Josh Cartwright 0 siblings, 0 replies; 21+ messages in thread From: Josh Cartwright @ 2014-09-23 23:04 UTC (permalink / raw) To: Wim Van Sebroeck, linux-watchdog, linux-kernel Cc: linux-arm-msm, linux-arm-kernel, Kumar Gala, Rob Herring, Pawel Moll, Mark Rutland, Ian Campbell, devicetree The Qualcomm Krait Processor Sub-system (KPSS) contains one or more instances of the WDT. Provide documentation on how to describe these in the device tree. Signed-off-by: Josh Cartwright <joshc@codeaurora.org> --- .../devicetree/bindings/watchdog/qcom-wdt.txt | 22 ++++++++++++++++++++++ 1 file changed, 22 insertions(+) create mode 100644 Documentation/devicetree/bindings/watchdog/qcom-wdt.txt diff --git a/Documentation/devicetree/bindings/watchdog/qcom-wdt.txt b/Documentation/devicetree/bindings/watchdog/qcom-wdt.txt new file mode 100644 index 0000000..6a1d758 --- /dev/null +++ b/Documentation/devicetree/bindings/watchdog/qcom-wdt.txt @@ -0,0 +1,22 @@ +Qualcomm Krait Processor Sub-system (KPSS) Watchdog +--------------------------------------------------- + +Required properties : +- compatible : shall contain only one of the following: + + "qcom,kpss-wdt-msm8960" + "qcom,kpss-wdt-apq8064" + "qcom,kpss-wdt-ipq8064" + +- reg : shall contain base register location and length +- clocks : shall contain the input clock phandle +- timeout-sec : shall contain the default watchdog timeout in seconds, + if unset, the default timeout is 30 seconds + +Example: + watchdog@208a038 { + compatible = "qcom,kpss-wdt-ipq8064"; + reg = <0x0208a038 0x40>; + clocks = <&sleep_clk>; + timeout-sec = <10>; + }; -- Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum, hosted by The Linux Foundation ^ permalink raw reply related [flat|nested] 21+ messages in thread
[parent not found: <337231c0fc8b16b4f37e1e3b85cb0246f357a64d.1411513109.git.joshc-sgV2jX0FEOL9JmXXK+q4OQ@public.gmane.org>]
* Re: [PATCH v2 2/3] watchdog: qcom: document device tree bindings 2014-09-23 23:04 ` Josh Cartwright (?) @ 2014-09-24 10:44 ` Arnd Bergmann -1 siblings, 0 replies; 21+ messages in thread From: Arnd Bergmann @ 2014-09-24 10:44 UTC (permalink / raw) To: Josh Cartwright Cc: Wim Van Sebroeck, linux-watchdog-u79uwXL29TY76Z2rM5mHXA, linux-kernel-u79uwXL29TY76Z2rM5mHXA, linux-arm-msm-u79uwXL29TY76Z2rM5mHXA, linux-arm-kernel-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r, Kumar Gala, Rob Herring, Pawel Moll, Mark Rutland, Ian Campbell, devicetree-u79uwXL29TY76Z2rM5mHXA On Tuesday 23 September 2014 18:04:37 Josh Cartwright wrote: > +- clocks : shall contain the input clock phandle Just nitpicking, but this is not just a phandle, it's a clock descriptor, which is a phandle followed by a set of arguments, which may be empty, depending the on the clock controller implementation. I would just call it 'the input clock', rather than 'input clock phandle'. Arnd -- To unsubscribe from this list: send the line "unsubscribe devicetree" in the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org More majordomo info at http://vger.kernel.org/majordomo-info.html ^ permalink raw reply [flat|nested] 21+ messages in thread
* [PATCH v2 2/3] watchdog: qcom: document device tree bindings @ 2014-09-24 10:44 ` Arnd Bergmann 0 siblings, 0 replies; 21+ messages in thread From: Arnd Bergmann @ 2014-09-24 10:44 UTC (permalink / raw) To: linux-arm-kernel On Tuesday 23 September 2014 18:04:37 Josh Cartwright wrote: > +- clocks : shall contain the input clock phandle Just nitpicking, but this is not just a phandle, it's a clock descriptor, which is a phandle followed by a set of arguments, which may be empty, depending the on the clock controller implementation. I would just call it 'the input clock', rather than 'input clock phandle'. Arnd ^ permalink raw reply [flat|nested] 21+ messages in thread
* Re: [PATCH v2 2/3] watchdog: qcom: document device tree bindings @ 2014-09-24 10:44 ` Arnd Bergmann 0 siblings, 0 replies; 21+ messages in thread From: Arnd Bergmann @ 2014-09-24 10:44 UTC (permalink / raw) To: Josh Cartwright Cc: Wim Van Sebroeck, linux-watchdog, linux-kernel, linux-arm-msm, linux-arm-kernel, Kumar Gala, Rob Herring, Pawel Moll, Mark Rutland, Ian Campbell, devicetree On Tuesday 23 September 2014 18:04:37 Josh Cartwright wrote: > +- clocks : shall contain the input clock phandle Just nitpicking, but this is not just a phandle, it's a clock descriptor, which is a phandle followed by a set of arguments, which may be empty, depending the on the clock controller implementation. I would just call it 'the input clock', rather than 'input clock phandle'. Arnd ^ permalink raw reply [flat|nested] 21+ messages in thread
* Re: [PATCH v2 2/3] watchdog: qcom: document device tree bindings 2014-09-24 10:44 ` Arnd Bergmann (?) @ 2014-09-24 10:56 ` Mark Rutland -1 siblings, 0 replies; 21+ messages in thread From: Mark Rutland @ 2014-09-24 10:56 UTC (permalink / raw) To: Arnd Bergmann Cc: Josh Cartwright, Wim Van Sebroeck, linux-watchdog, linux-kernel, linux-arm-msm, linux-arm-kernel, Kumar Gala, Rob Herring, Pawel Moll, Ian Campbell, devicetree On Wed, Sep 24, 2014 at 11:44:44AM +0100, Arnd Bergmann wrote: > On Tuesday 23 September 2014 18:04:37 Josh Cartwright wrote: > > +- clocks : shall contain the input clock phandle > > Just nitpicking, but this is not just a phandle, it's a clock > descriptor, which is a phandle followed by a set of arguments, > which may be empty, depending the on the clock controller > implementation. We're _extremely_ inconsistent with terminology, so it's not confusing people don't know what to put here. Elsewhere people say "phandle + clock specifier" (which is correct), "clock reference" (which is ok, but sounds like a phandle), etc. From what I recall some documentation says some *-specifier properties include the phandle too. It would be really nice if we had one consistent way of refering to phandle+args style properties. > > I would just call it 'the input clock', rather than 'input clock > phandle'. That's fine by me, given clocks is a well-understood standard property. Mark. ^ permalink raw reply [flat|nested] 21+ messages in thread
* [PATCH v2 2/3] watchdog: qcom: document device tree bindings @ 2014-09-24 10:56 ` Mark Rutland 0 siblings, 0 replies; 21+ messages in thread From: Mark Rutland @ 2014-09-24 10:56 UTC (permalink / raw) To: linux-arm-kernel On Wed, Sep 24, 2014 at 11:44:44AM +0100, Arnd Bergmann wrote: > On Tuesday 23 September 2014 18:04:37 Josh Cartwright wrote: > > +- clocks : shall contain the input clock phandle > > Just nitpicking, but this is not just a phandle, it's a clock > descriptor, which is a phandle followed by a set of arguments, > which may be empty, depending the on the clock controller > implementation. We're _extremely_ inconsistent with terminology, so it's not confusing people don't know what to put here. Elsewhere people say "phandle + clock specifier" (which is correct), "clock reference" (which is ok, but sounds like a phandle), etc. From what I recall some documentation says some *-specifier properties include the phandle too. It would be really nice if we had one consistent way of refering to phandle+args style properties. > > I would just call it 'the input clock', rather than 'input clock > phandle'. That's fine by me, given clocks is a well-understood standard property. Mark. ^ permalink raw reply [flat|nested] 21+ messages in thread
* Re: [PATCH v2 2/3] watchdog: qcom: document device tree bindings @ 2014-09-24 10:56 ` Mark Rutland 0 siblings, 0 replies; 21+ messages in thread From: Mark Rutland @ 2014-09-24 10:56 UTC (permalink / raw) To: Arnd Bergmann Cc: Josh Cartwright, Wim Van Sebroeck, linux-watchdog, linux-kernel, linux-arm-msm, linux-arm-kernel, Kumar Gala, Rob Herring, Pawel Moll, Ian Campbell, devicetree On Wed, Sep 24, 2014 at 11:44:44AM +0100, Arnd Bergmann wrote: > On Tuesday 23 September 2014 18:04:37 Josh Cartwright wrote: > > +- clocks : shall contain the input clock phandle > > Just nitpicking, but this is not just a phandle, it's a clock > descriptor, which is a phandle followed by a set of arguments, > which may be empty, depending the on the clock controller > implementation. We're _extremely_ inconsistent with terminology, so it's not confusing people don't know what to put here. Elsewhere people say "phandle + clock specifier" (which is correct), "clock reference" (which is ok, but sounds like a phandle), etc. From what I recall some documentation says some *-specifier properties include the phandle too. It would be really nice if we had one consistent way of refering to phandle+args style properties. > > I would just call it 'the input clock', rather than 'input clock > phandle'. That's fine by me, given clocks is a well-understood standard property. Mark. ^ permalink raw reply [flat|nested] 21+ messages in thread
* [PATCH v2 3/3] watchdog: qcom: register a restart notifier 2014-09-23 23:04 ` Josh Cartwright @ 2014-09-23 23:04 ` Josh Cartwright -1 siblings, 0 replies; 21+ messages in thread From: Josh Cartwright @ 2014-09-23 23:04 UTC (permalink / raw) To: Wim Van Sebroeck, linux-watchdog Cc: linux-arm-msm, linux-arm-kernel, Kumar Gala, linux-kernel The WDT's BITE_TIME warm-reset behavior can be leveraged as a last resort mechanism for triggering chip reset. Usually, other restart methods (such as PS_HOLD) are preferrable for issuing a more complete reset of the chip. As such, keep the priority of the watchdog notifier low. Signed-off-by: Josh Cartwright <joshc@codeaurora.org> --- drivers/watchdog/qcom-wdt.c | 28 ++++++++++++++++++++++++++++ 1 file changed, 28 insertions(+) diff --git a/drivers/watchdog/qcom-wdt.c b/drivers/watchdog/qcom-wdt.c index d5e46e2..eba92ef 100644 --- a/drivers/watchdog/qcom-wdt.c +++ b/drivers/watchdog/qcom-wdt.c @@ -16,6 +16,7 @@ #include <linux/module.h> #include <linux/of.h> #include <linux/platform_device.h> +#include <linux/reboot.h> #include <linux/watchdog.h> #define WDT_RST 0x0 @@ -25,6 +26,7 @@ struct qcom_wdt { struct watchdog_device wdd; struct clk *clk; + struct notifier_block restart_nb; void __iomem *base; }; @@ -86,6 +88,23 @@ static const struct watchdog_info qcom_wdt_info = { .identity = KBUILD_MODNAME, }; +static int qcom_wdt_restart(struct notifier_block *nb, unsigned long action, + void *data) +{ + struct qcom_wdt *wdt = container_of(nb, struct qcom_wdt, restart_nb); + + /* + * Trigger watchdog bite: + * Setup BITE_TIME to be very low, and enable WDT. + * 0x31F3 => 390ms @ 32kHz, also value at reset + */ + writel(0, wdt->base + WDT_EN); + writel(1, wdt->base + WDT_RST); + writel(0x31F3, wdt->base + WDT_BITE_TIME); + writel(1, wdt->base + WDT_EN); + return NOTIFY_DONE; +} + static int qcom_wdt_probe(struct platform_device *pdev) { struct qcom_wdt *wdt; @@ -141,6 +160,14 @@ static int qcom_wdt_probe(struct platform_device *pdev) return ret; } + /* + * WDT restart notifier has priority 0 (use as a last resort) + */ + wdt->restart_nb.notifier_call = qcom_wdt_restart; + ret = register_restart_handler(&wdt->restart_nb); + if (ret) + dev_err(&pdev->dev, "failed to setup restart handler\n"); + platform_set_drvdata(pdev, wdt); return 0; } @@ -149,6 +176,7 @@ static int qcom_wdt_remove(struct platform_device *pdev) { struct qcom_wdt *wdt = platform_get_drvdata(pdev); + unregister_restart_handler(&wdt->restart_nb); watchdog_unregister_device(&wdt->wdd); clk_disable_unprepare(wdt->clk); return 0; -- Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum, hosted by The Linux Foundation ^ permalink raw reply related [flat|nested] 21+ messages in thread
* [PATCH v2 3/3] watchdog: qcom: register a restart notifier @ 2014-09-23 23:04 ` Josh Cartwright 0 siblings, 0 replies; 21+ messages in thread From: Josh Cartwright @ 2014-09-23 23:04 UTC (permalink / raw) To: linux-arm-kernel The WDT's BITE_TIME warm-reset behavior can be leveraged as a last resort mechanism for triggering chip reset. Usually, other restart methods (such as PS_HOLD) are preferrable for issuing a more complete reset of the chip. As such, keep the priority of the watchdog notifier low. Signed-off-by: Josh Cartwright <joshc@codeaurora.org> --- drivers/watchdog/qcom-wdt.c | 28 ++++++++++++++++++++++++++++ 1 file changed, 28 insertions(+) diff --git a/drivers/watchdog/qcom-wdt.c b/drivers/watchdog/qcom-wdt.c index d5e46e2..eba92ef 100644 --- a/drivers/watchdog/qcom-wdt.c +++ b/drivers/watchdog/qcom-wdt.c @@ -16,6 +16,7 @@ #include <linux/module.h> #include <linux/of.h> #include <linux/platform_device.h> +#include <linux/reboot.h> #include <linux/watchdog.h> #define WDT_RST 0x0 @@ -25,6 +26,7 @@ struct qcom_wdt { struct watchdog_device wdd; struct clk *clk; + struct notifier_block restart_nb; void __iomem *base; }; @@ -86,6 +88,23 @@ static const struct watchdog_info qcom_wdt_info = { .identity = KBUILD_MODNAME, }; +static int qcom_wdt_restart(struct notifier_block *nb, unsigned long action, + void *data) +{ + struct qcom_wdt *wdt = container_of(nb, struct qcom_wdt, restart_nb); + + /* + * Trigger watchdog bite: + * Setup BITE_TIME to be very low, and enable WDT. + * 0x31F3 => 390ms @ 32kHz, also value at reset + */ + writel(0, wdt->base + WDT_EN); + writel(1, wdt->base + WDT_RST); + writel(0x31F3, wdt->base + WDT_BITE_TIME); + writel(1, wdt->base + WDT_EN); + return NOTIFY_DONE; +} + static int qcom_wdt_probe(struct platform_device *pdev) { struct qcom_wdt *wdt; @@ -141,6 +160,14 @@ static int qcom_wdt_probe(struct platform_device *pdev) return ret; } + /* + * WDT restart notifier has priority 0 (use as a last resort) + */ + wdt->restart_nb.notifier_call = qcom_wdt_restart; + ret = register_restart_handler(&wdt->restart_nb); + if (ret) + dev_err(&pdev->dev, "failed to setup restart handler\n"); + platform_set_drvdata(pdev, wdt); return 0; } @@ -149,6 +176,7 @@ static int qcom_wdt_remove(struct platform_device *pdev) { struct qcom_wdt *wdt = platform_get_drvdata(pdev); + unregister_restart_handler(&wdt->restart_nb); watchdog_unregister_device(&wdt->wdd); clk_disable_unprepare(wdt->clk); return 0; -- Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum, hosted by The Linux Foundation ^ permalink raw reply related [flat|nested] 21+ messages in thread
* Re: [PATCH v2 3/3] watchdog: qcom: register a restart notifier 2014-09-23 23:04 ` Josh Cartwright @ 2014-09-24 17:05 ` Guenter Roeck -1 siblings, 0 replies; 21+ messages in thread From: Guenter Roeck @ 2014-09-24 17:05 UTC (permalink / raw) To: Josh Cartwright Cc: Wim Van Sebroeck, linux-watchdog, linux-arm-msm, linux-arm-kernel, Kumar Gala, linux-kernel On Tue, Sep 23, 2014 at 06:04:38PM -0500, Josh Cartwright wrote: > The WDT's BITE_TIME warm-reset behavior can be leveraged as a last > resort mechanism for triggering chip reset. Usually, other restart > methods (such as PS_HOLD) are preferrable for issuing a more complete > reset of the chip. As such, keep the priority of the watchdog notifier > low. > > Signed-off-by: Josh Cartwright <joshc@codeaurora.org> > --- > drivers/watchdog/qcom-wdt.c | 28 ++++++++++++++++++++++++++++ > 1 file changed, 28 insertions(+) > > diff --git a/drivers/watchdog/qcom-wdt.c b/drivers/watchdog/qcom-wdt.c > index d5e46e2..eba92ef 100644 > --- a/drivers/watchdog/qcom-wdt.c > +++ b/drivers/watchdog/qcom-wdt.c > @@ -16,6 +16,7 @@ > #include <linux/module.h> > #include <linux/of.h> > #include <linux/platform_device.h> > +#include <linux/reboot.h> > #include <linux/watchdog.h> > > #define WDT_RST 0x0 > @@ -25,6 +26,7 @@ > struct qcom_wdt { > struct watchdog_device wdd; > struct clk *clk; > + struct notifier_block restart_nb; > void __iomem *base; > }; > > @@ -86,6 +88,23 @@ static const struct watchdog_info qcom_wdt_info = { > .identity = KBUILD_MODNAME, > }; > > +static int qcom_wdt_restart(struct notifier_block *nb, unsigned long action, > + void *data) > +{ > + struct qcom_wdt *wdt = container_of(nb, struct qcom_wdt, restart_nb); > + > + /* > + * Trigger watchdog bite: > + * Setup BITE_TIME to be very low, and enable WDT. > + * 0x31F3 => 390ms @ 32kHz, also value at reset What happens if you set it to a lower value ? If you set it to, say, 0x100, would the timeout be ~8ms ? Wouldn't that make more sense ? Or at least something like 0x800, which should make it around 64 ms, or 0x400 for 32 ms. [ In other words, I don't really see the point for a 390 ms wait here ] > + */ > + writel(0, wdt->base + WDT_EN); > + writel(1, wdt->base + WDT_RST); > + writel(0x31F3, wdt->base + WDT_BITE_TIME); > + writel(1, wdt->base + WDT_EN); You should have some mdelay() here, larger than the expected timeout. Thanks, Guenter > + return NOTIFY_DONE; > +} > + > static int qcom_wdt_probe(struct platform_device *pdev) > { > struct qcom_wdt *wdt; > @@ -141,6 +160,14 @@ static int qcom_wdt_probe(struct platform_device *pdev) > return ret; > } > > + /* > + * WDT restart notifier has priority 0 (use as a last resort) > + */ > + wdt->restart_nb.notifier_call = qcom_wdt_restart; > + ret = register_restart_handler(&wdt->restart_nb); > + if (ret) > + dev_err(&pdev->dev, "failed to setup restart handler\n"); > + > platform_set_drvdata(pdev, wdt); > return 0; > } > @@ -149,6 +176,7 @@ static int qcom_wdt_remove(struct platform_device *pdev) > { > struct qcom_wdt *wdt = platform_get_drvdata(pdev); > > + unregister_restart_handler(&wdt->restart_nb); > watchdog_unregister_device(&wdt->wdd); > clk_disable_unprepare(wdt->clk); > return 0; > -- > Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum, > hosted by The Linux Foundation > > -- > To unsubscribe from this list: send the line "unsubscribe linux-watchdog" in > the body of a message to majordomo@vger.kernel.org > More majordomo info at http://vger.kernel.org/majordomo-info.html ^ permalink raw reply [flat|nested] 21+ messages in thread
* [PATCH v2 3/3] watchdog: qcom: register a restart notifier @ 2014-09-24 17:05 ` Guenter Roeck 0 siblings, 0 replies; 21+ messages in thread From: Guenter Roeck @ 2014-09-24 17:05 UTC (permalink / raw) To: linux-arm-kernel On Tue, Sep 23, 2014 at 06:04:38PM -0500, Josh Cartwright wrote: > The WDT's BITE_TIME warm-reset behavior can be leveraged as a last > resort mechanism for triggering chip reset. Usually, other restart > methods (such as PS_HOLD) are preferrable for issuing a more complete > reset of the chip. As such, keep the priority of the watchdog notifier > low. > > Signed-off-by: Josh Cartwright <joshc@codeaurora.org> > --- > drivers/watchdog/qcom-wdt.c | 28 ++++++++++++++++++++++++++++ > 1 file changed, 28 insertions(+) > > diff --git a/drivers/watchdog/qcom-wdt.c b/drivers/watchdog/qcom-wdt.c > index d5e46e2..eba92ef 100644 > --- a/drivers/watchdog/qcom-wdt.c > +++ b/drivers/watchdog/qcom-wdt.c > @@ -16,6 +16,7 @@ > #include <linux/module.h> > #include <linux/of.h> > #include <linux/platform_device.h> > +#include <linux/reboot.h> > #include <linux/watchdog.h> > > #define WDT_RST 0x0 > @@ -25,6 +26,7 @@ > struct qcom_wdt { > struct watchdog_device wdd; > struct clk *clk; > + struct notifier_block restart_nb; > void __iomem *base; > }; > > @@ -86,6 +88,23 @@ static const struct watchdog_info qcom_wdt_info = { > .identity = KBUILD_MODNAME, > }; > > +static int qcom_wdt_restart(struct notifier_block *nb, unsigned long action, > + void *data) > +{ > + struct qcom_wdt *wdt = container_of(nb, struct qcom_wdt, restart_nb); > + > + /* > + * Trigger watchdog bite: > + * Setup BITE_TIME to be very low, and enable WDT. > + * 0x31F3 => 390ms @ 32kHz, also value at reset What happens if you set it to a lower value ? If you set it to, say, 0x100, would the timeout be ~8ms ? Wouldn't that make more sense ? Or at least something like 0x800, which should make it around 64 ms, or 0x400 for 32 ms. [ In other words, I don't really see the point for a 390 ms wait here ] > + */ > + writel(0, wdt->base + WDT_EN); > + writel(1, wdt->base + WDT_RST); > + writel(0x31F3, wdt->base + WDT_BITE_TIME); > + writel(1, wdt->base + WDT_EN); You should have some mdelay() here, larger than the expected timeout. Thanks, Guenter > + return NOTIFY_DONE; > +} > + > static int qcom_wdt_probe(struct platform_device *pdev) > { > struct qcom_wdt *wdt; > @@ -141,6 +160,14 @@ static int qcom_wdt_probe(struct platform_device *pdev) > return ret; > } > > + /* > + * WDT restart notifier has priority 0 (use as a last resort) > + */ > + wdt->restart_nb.notifier_call = qcom_wdt_restart; > + ret = register_restart_handler(&wdt->restart_nb); > + if (ret) > + dev_err(&pdev->dev, "failed to setup restart handler\n"); > + > platform_set_drvdata(pdev, wdt); > return 0; > } > @@ -149,6 +176,7 @@ static int qcom_wdt_remove(struct platform_device *pdev) > { > struct qcom_wdt *wdt = platform_get_drvdata(pdev); > > + unregister_restart_handler(&wdt->restart_nb); > watchdog_unregister_device(&wdt->wdd); > clk_disable_unprepare(wdt->clk); > return 0; > -- > Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum, > hosted by The Linux Foundation > > -- > To unsubscribe from this list: send the line "unsubscribe linux-watchdog" in > the body of a message to majordomo at vger.kernel.org > More majordomo info at http://vger.kernel.org/majordomo-info.html ^ permalink raw reply [flat|nested] 21+ messages in thread
end of thread, other threads:[~2014-09-24 18:36 UTC | newest]
Thread overview: 21+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2014-09-23 23:04 [PATCH v2 0/3] watchdog: add support for QCOM WDT Josh Cartwright
2014-09-23 23:04 ` Josh Cartwright
2014-09-23 23:04 ` [PATCH v2 1/3] watchdog: qcom: add support for KPSS WDT Josh Cartwright
2014-09-23 23:04 ` Josh Cartwright
2014-09-24 15:58 ` Guenter Roeck
2014-09-24 15:58 ` Guenter Roeck
2014-09-24 18:30 ` Josh Cartwright
2014-09-24 18:30 ` Josh Cartwright
[not found] ` <cover.1411513109.git.joshc-sgV2jX0FEOL9JmXXK+q4OQ@public.gmane.org>
2014-09-23 23:04 ` [PATCH v2 2/3] watchdog: qcom: document device tree bindings Josh Cartwright
2014-09-23 23:04 ` Josh Cartwright
2014-09-23 23:04 ` Josh Cartwright
[not found] ` <337231c0fc8b16b4f37e1e3b85cb0246f357a64d.1411513109.git.joshc-sgV2jX0FEOL9JmXXK+q4OQ@public.gmane.org>
2014-09-24 10:44 ` Arnd Bergmann
2014-09-24 10:44 ` Arnd Bergmann
2014-09-24 10:44 ` Arnd Bergmann
2014-09-24 10:56 ` Mark Rutland
2014-09-24 10:56 ` Mark Rutland
2014-09-24 10:56 ` Mark Rutland
2014-09-23 23:04 ` [PATCH v2 3/3] watchdog: qcom: register a restart notifier Josh Cartwright
2014-09-23 23:04 ` Josh Cartwright
2014-09-24 17:05 ` Guenter Roeck
2014-09-24 17:05 ` Guenter Roeck
This is an external index of several public inboxes, see mirroring instructions on how to clone and mirror all data and code used by this external index.