[PATCH 0/3] watchdog: add support for QCOM WDT

[Josh Cartwright]

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.

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 | 21 +++
drivers/watchdog/Kconfig | 10 ++
drivers/watchdog/Makefile | 1 +
drivers/watchdog/qcom-wdt.c | 176 +++++++++++++++++++++
4 files changed, 208 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

--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majo...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/

[Josh Cartwright]

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 <jo...@codeaurora.org>
---
drivers/watchdog/Kconfig | 10 +++
drivers/watchdog/Makefile | 1 +
drivers/watchdog/qcom-wdt.c | 145 ++++++++++++++++++++++++++++++++++++++++++++
3 files changed, 156 insertions(+)
create mode 100644 drivers/watchdog/qcom-wdt.c

diff --git a/drivers/watchdog/Kconfig b/drivers/watchdog/Kconfig
index 1d1330a..5ccb963 100644
--- a/drivers/watchdog/Kconfig
+++ b/drivers/watchdog/Kconfig
@@ -443,6 +443,16 @@ config TEGRA_WATCHDOG
To compile this driver as a module, choose M here: the
module will be called tegra_wdt.

+config QCOM_WDT
+ bool "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.
+
# 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..e9409f5
--- /dev/null
+++ b/drivers/watchdog/qcom-wdt.c
@@ -0,0 +1,145 @@
+/* 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/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;
+ unsigned long freq;
+ 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);
+
+ writel(0, wdt->base + WDT_EN);
+ writel(1, wdt->base + WDT_RST);
+ writel(wdd->timeout * wdt->freq, 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_watchdog_probe(struct platform_device *pdev)
+{
+ struct qcom_wdt *wdt;
+ struct resource *res;
+ u32 tmp;
+ int ret;
+
+ wdt = devm_kzalloc(&pdev->dev, sizeof(*wdt), GFP_KERNEL);
+ if (!wdt)
+ return -ENOMEM;
+
+ platform_set_drvdata(pdev, wdt);
+
+ 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);
+
+ ret = of_property_read_u32(pdev->dev.of_node, "clock-frequency", &tmp);
+ if (ret) {
+ dev_err(&pdev->dev, "unable to get clock-frequency\n");
+ return ret;
+ }
+
+ wdt->freq = tmp;
+
+ 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 / wdt->freq;
+ watchdog_init_timeout(&wdt->wdd, 0, &pdev->dev);
+
+ ret = watchdog_register_device(&wdt->wdd);
+ if (ret) {
+ dev_err(&pdev->dev, "failed to register watchdog\n");
+ return ret;
+ }
+
+ 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_watchdog_probe,
+ .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");

[Josh Cartwright]

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 <jo...@codeaurora.org>
---

.../devicetree/bindings/watchdog/qcom-wdt.txt | 21 +++++++++++++++++++++
1 file changed, 21 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..e65cb8e
--- /dev/null
+++ b/Documentation/devicetree/bindings/watchdog/qcom-wdt.txt
@@ -0,0 +1,21 @@
+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
+- clock-frequency : shall contain the frequency at which the watchdog ticks
+- timeout-sec : shall contain the default watchdog timeout in seconds
+
+Example:
+ watchdog@208a038 {
+ compatible = "qcom,kpss-wdt-ipq8064";
+ reg = <0x0208a038 0x40>;
+ clock-frequency = <32768>;
+ timeout-sec = <10>;
+ };

[Josh Cartwright]

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 <jo...@codeaurora.org>
---

drivers/watchdog/qcom-wdt.c | 31 +++++++++++++++++++++++++++++++
1 file changed, 31 insertions(+)

diff --git a/drivers/watchdog/qcom-wdt.c b/drivers/watchdog/qcom-wdt.c
index e9409f5..710ab43 100644
--- a/drivers/watchdog/qcom-wdt.c
+++ b/drivers/watchdog/qcom-wdt.c
@@ -15,6 +15,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
@@ -24,6 +25,7 @@
struct qcom_wdt {
struct watchdog_device wdd;
unsigned long freq;
+ struct notifier_block restart_nb;
void __iomem *base;
};

@@ -82,6 +84,24 @@ 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.
+ */
+ mutex_lock(&wdt->wdd.lock);
+ writel_relaxed(0, wdt->base + WDT_EN);
+ writel_relaxed(1, wdt->base + WDT_RST);
+ writel_relaxed(0x31F3, wdt->base + WDT_BITE_TIME);
+ writel_relaxed(1, wdt->base + WDT_EN);
+ mutex_unlock(&wdt->wdd.lock);
+ return NOTIFY_DONE;
+}
+
static int qcom_watchdog_probe(struct platform_device *pdev)
{
struct qcom_wdt *wdt;
@@ -121,6 +141,17 @@ static int qcom_watchdog_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");
+ watchdog_unregister_device(&wdt->wdd);

+ return ret;
+ }
+

return 0;

[Guenter Roeck]

On 09/18/2014 03:26 PM, 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 <jo...@codeaurora.org>

Hi Josh,

comments inline.

Thanks,
Guenter

You might want to use a clock property here, and the complete sequence of
devm_clk_get
clk_prepare_enable
clk_disable_unprepare
clk_get_rate

> + wdt->freq = tmp;
> +
> + 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 / wdt->freq;

As written, wdt->freq can be 0, which results in a nice division by zero here.

> + watchdog_init_timeout(&wdt->wdd, 0, &pdev->dev);

That leaves you with no default timeout if timeout-sec is not set in devicetree,
which if I understand the code correctly might result in an immediate reset.
Is this really what you want to happen ?

> +
> + ret = watchdog_register_device(&wdt->wdd);
> + if (ret) {
> + dev_err(&pdev->dev, "failed to register watchdog\n");
> + return ret;
> + }
> +
> + 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_watchdog_probe,

No remove function ?

Yes, you don't need it, because the driver can only be built into the kernel,
but there is a practical impact: It means the driver must always be built
into the kernel even if the image is supposed to be used on different systems,
some of which may not support this specific watchdog.

Sure, you might say that you don't care about images supporting more than one
hardware, but the tendency seems to be multi-target images nowadays.

> + .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");
>

--

[Guenter Roeck]

At this time you don't need to worry about locks.

Actually, this might be dangerous if the lock happens to be taken,
as it won't be released (there is no other code running anymore
when this function is called).

> + writel_relaxed(0, wdt->base + WDT_EN);
> + writel_relaxed(1, wdt->base + WDT_RST);
> + writel_relaxed(0x31F3, wdt->base + WDT_BITE_TIME);

What is the magic here, ie what does 0x31F3 stand for ?

> + writel_relaxed(1, wdt->base + WDT_EN);
> + mutex_unlock(&wdt->wdd.lock);
> + return NOTIFY_DONE;
> +}
> +
> static int qcom_watchdog_probe(struct platform_device *pdev)
> {
> struct qcom_wdt *wdt;
> @@ -121,6 +141,17 @@ static int qcom_watchdog_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");
> + watchdog_unregister_device(&wdt->wdd);
> + return ret;

Sure you want to return an error here ? The watchdog itself is still working,
and this is supposed to be a restart method of last resort. Causing the driver
to fail loading because it can not register its restart handler seems to be
a bit aggressive.

Thanks,
Guenter

> + }
> +
> return 0;
> }
>
>

--

[Josh Cartwright]

On Thu, Sep 18, 2014 at 07:47:54PM -0700, Guenter Roeck wrote:
> On 09/18/2014 03:27 PM, 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 <jo...@codeaurora.org>

[..]

> >+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.
> >+ */
> >+ mutex_lock(&wdt->wdd.lock);
>
> At this time you don't need to worry about locks.
>
> Actually, this might be dangerous if the lock happens to be taken,
> as it won't be released (there is no other code running anymore
> when this function is called).

Ah, great. I'll drop the locking.

> >+ writel_relaxed(0, wdt->base + WDT_EN);
> >+ writel_relaxed(1, wdt->base + WDT_RST);
> >+ writel_relaxed(0x31F3, wdt->base + WDT_BITE_TIME);
>
> What is the magic here, ie what does 0x31F3 stand for ?

Nothing magic, it's just a reasonably low value to set the bite time
counter at. It also happens to be the value at reset.

> >+ writel_relaxed(1, wdt->base + WDT_EN);
> >+ mutex_unlock(&wdt->wdd.lock);
> >+ return NOTIFY_DONE;
> >+}
> >+
> > static int qcom_watchdog_probe(struct platform_device *pdev)
> > {
> > struct qcom_wdt *wdt;
> >@@ -121,6 +141,17 @@ static int qcom_watchdog_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");
> >+ watchdog_unregister_device(&wdt->wdd);
> >+ return ret;
>
> Sure you want to return an error here ? The watchdog itself is still working,
> and this is supposed to be a restart method of last resort. Causing the driver
> to fail loading because it can not register its restart handler seems to be
> a bit aggressive.

It is a bit aggressive. I'll at least drop it to a dev_warn(); even
though it is a "last resort", on some boards it's the only available
mechanism for reliable restart.

Thanks!
Josh

--
Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum,
hosted by The Linux Foundation

[Josh Cartwright]

On Thu, Sep 18, 2014 at 07:41:17PM -0700, Guenter Roeck wrote:
> On 09/18/2014 03:26 PM, 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 <jo...@codeaurora.org>
>
> Hi Josh,
>
> comments inline.

Thanks for taking a look!

[..]

Agreed. I think this would be ideal. I'll need to take a closer look
at how this thing is clocked, and how/if the clocks are currently
being modelled.

> >+ wdt->freq = tmp;
> >+
> >+ 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 / wdt->freq;
>
> As written, wdt->freq can be 0, which results in a nice division by zero here.

Indeed. I'll add a check.

> >+ watchdog_init_timeout(&wdt->wdd, 0, &pdev->dev);
>
> That leaves you with no default timeout if timeout-sec is not set in devicetree,
> which if I understand the code correctly might result in an immediate reset.
> Is this really what you want to happen ?

I think I'd like to handle timeout-sec being unspecified as an error at
probe. If someone explicitly sets timeout-sec = <0>, then they get what
they ask for. I'll take another look to see how to make this happen.

This was motivated by the addition of the restart_handler bits in patch
3. For some reason I was thinking there were race conditions between
module unloading/the restart_handler mechanism, but looking at it again,
I'm not so sure. Is it safe to implement these handlers in modules? If
so, I'll revisit this.

Thanks again,
Josh

--
Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum,
hosted by The Linux Foundation

[Guenter Roeck]

I think you should be able to specify some kind of "fixed" clock.
Other watchdog drivers use the mechanism; maybe you can find some examples.

>>> + wdt->freq = tmp;
>>> +
>>> + 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 / wdt->freq;
>>
>> As written, wdt->freq can be 0, which results in a nice division by zero here.
>
> Indeed. I'll add a check.
>
>>> + watchdog_init_timeout(&wdt->wdd, 0, &pdev->dev);
>>
>> That leaves you with no default timeout if timeout-sec is not set in devicetree,
>> which if I understand the code correctly might result in an immediate reset.
>> Is this really what you want to happen ?
>
> I think I'd like to handle timeout-sec being unspecified as an error at
> probe. If someone explicitly sets timeout-sec = <0>, then they get what
> they ask for. I'll take another look to see how to make this happen.
>

Hmm.. kind of unusual. Usual would be to initialize the timeout together
with min_timeout / max_timeout above and only force the user to specify
a value if the default timeout is not desirable. You don't really gain
anything by making timeout-sec mandatory.

Yes, it is safe. To ensure there are no race conditions was one of the reasons
for implementing the restart handler as notifier call chain.

Guenter

[Guenter Roeck]

Can you add a note explaining that this reflects a reset delay of <n>
milliseconds or whatever it is ?

Sure, but the problem is that you don't let people use the watchdog just
because restart handler registration failed. dev_err or dev_warn is fine,
my concern was not the message but that you abort watchdog registration
in this case.

Kind of letting people not use their car just because one of its lights
failed.

Guenter

[Pramod Gurav]

Hi Josh,

Tested this patch on IFC6410 by disabling pinctrl ps_hold reset
implementation and on top of Guenter's patchset for restart_notifier.

Tested-by: pramod...@smartplayin.com

I picked up DT changes from bindings as they are from this patchset.

Thanks
Pramod

[Josh Cartwright]

On Thu, Sep 18, 2014 at 09:03:18PM -0700, Guenter Roeck wrote:
> On 09/18/2014 08:32 PM, Josh Cartwright wrote:
> >On Thu, Sep 18, 2014 at 07:47:54PM -0700, Guenter Roeck wrote:
> >>On 09/18/2014 03:27 PM, Josh Cartwright wrote:

[..]

> >
> >Nothing magic, it's just a reasonably low value to set the bite time
> >counter at. It also happens to be the value at reset.
> >
> Can you add a note explaining that this reflects a reset delay of <n>
> milliseconds or whatever it is ?

Certainly. Will do.

Thanks,

Josh

--
Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum,
hosted by The Linux Foundation

[Josh Cartwright]

On Thu, Sep 18, 2014 at 08:41:43PM -0700, Guenter Roeck wrote:
> On 09/18/2014 08:24 PM, Josh Cartwright wrote:
> >On Thu, Sep 18, 2014 at 07:41:17PM -0700, Guenter Roeck wrote:
> >>On 09/18/2014 03:26 PM, 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 <jo...@codeaurora.org>
> >>
> >>Hi Josh,
> >>
> >>comments inline.
> >
> >Thanks for taking a look!
> >
[..]

> >>>+ watchdog_init_timeout(&wdt->wdd, 0, &pdev->dev);
> >>
> >>That leaves you with no default timeout if timeout-sec is not set in devicetree,
> >>which if I understand the code correctly might result in an immediate reset.
> >>Is this really what you want to happen ?
> >
> >I think I'd like to handle timeout-sec being unspecified as an error at
> >probe. If someone explicitly sets timeout-sec = <0>, then they get what
> >they ask for. I'll take another look to see how to make this happen.
> >
>
> Hmm.. kind of unusual. Usual would be to initialize the timeout together
> with min_timeout / max_timeout above and only force the user to specify
> a value if the default timeout is not desirable. You don't really gain
> anything by making timeout-sec mandatory.

Making timeout-sec mandatory makes it so I don't have to decide what a
"sane default" is. :)

It's even less clear about what a sane default is looking at the other
watchdog drivers. From the drivers I looked at, it ranges any where
from 30s to 2mins. Am I just to choose? Why do these even differ
between all of the drivers?

--
Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum,
hosted by The Linux Foundation

[Guenter Roeck]

Bad excuse ;-). You just force others to make the decision for you.

> It's even less clear about what a sane default is looking at the other
> watchdog drivers. From the drivers I looked at, it ranges any where
> from 30s to 2mins. Am I just to choose? Why do these even differ
> between all of the drivers?
>

Sanity is defined as the majority opinion, as expressed quite nicely in
"Everyone is insane but me".

Just pick something in between. If you don't want to make a decision, pick
30 seconds and blame it on me.

Maybe someone at some point finds a common ground. Until then it is per driver.

Thanks,
Guenter

[Josh Cartwright]

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 <jo...@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

[Josh Cartwright]

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 <jo...@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");

[Josh Cartwright]

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 <jo...@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>;
+ };

[Josh Cartwright]

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

[Arnd Bergmann]

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

[Mark Rutland]

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.

[Guenter Roeck]

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 <jo...@codeaurora.org>

Hi Josh,

looks much better. Couple of comments inline.

Thanks,
Guenter

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.

> To unsubscribe from this list: send the line "unsubscribe linux-watchdog" in

[Guenter Roeck]

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

> To unsubscribe from this list: send the line "unsubscribe linux-watchdog" in

[Josh Cartwright]

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 <jo...@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

[..]

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
--

[Josh Cartwright]

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 v2:
- "input clock phandle" -> "input clock" in device tree documentation
- Fixup error handling paths during probe()
- Use a more sane timeout (128ms), and add msleep()
- Add some additional sanitation for clock rates

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 | 227 +++++++++++++++++++++
4 files changed, 263 insertions(+)

create mode 100644 Documentation/devicetree/bindings/watchdog/qcom-wdt.txt
create mode 100644 drivers/watchdog/qcom-wdt.c

[Josh Cartwright]

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 <jo...@codeaurora.org>
---

drivers/watchdog/qcom-wdt.c | 38 ++++++++++++++++++++++++++++++++++++++
1 file changed, 38 insertions(+)

diff --git a/drivers/watchdog/qcom-wdt.c b/drivers/watchdog/qcom-wdt.c
index 0f56ca3..8ce339f 100644
--- a/drivers/watchdog/qcom-wdt.c
+++ b/drivers/watchdog/qcom-wdt.c
@@ -10,12 +10,14 @@

* GNU General Public License for more details.

*
*/
+#include <linux/delay.h>
#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/reboot.h>
#include <linux/watchdog.h>

#define WDT_RST 0x0

@@ -26,6 +28,7 @@ struct qcom_wdt {

struct watchdog_device wdd;
struct clk *clk;

unsigned long rate;

+ struct notifier_block restart_nb;
void __iomem *base;
};

@@ -84,6 +87,32 @@ 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);

+ u32 timeout;
+
+ /*

+ * Trigger watchdog bite:

+ * Setup BITE_TIME to be 128ms, and enable WDT.
+ */
+ timeout = 128 * wdt->rate / 1000;
+

+ writel(0, wdt->base + WDT_EN);
+ writel(1, wdt->base + WDT_RST);

+ writel(timeout, wdt->base + WDT_BITE_TIME);

+ writel(1, wdt->base + WDT_EN);

+
+ /*
+ * Actually make sure the above sequence hits hardware before sleeping.
+ */
+ wmb();
+
+ msleep(150);

+ return NOTIFY_DONE;
+}
+
static int qcom_wdt_probe(struct platform_device *pdev)
{
struct qcom_wdt *wdt;

@@ -149,6 +178,14 @@ static int qcom_wdt_probe(struct platform_device *pdev)
goto err_clk_unprepare;

}

+ /*
+ * 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;

@@ -162,6 +199,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;

[Josh Cartwright]

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 <jo...@codeaurora.org>
---

drivers/watchdog/Kconfig | 13 +++
drivers/watchdog/Makefile | 1 +
drivers/watchdog/qcom-wdt.c | 189 ++++++++++++++++++++++++++++++++++++++++++++
3 files changed, 203 insertions(+)

create mode 100644 drivers/watchdog/qcom-wdt.c

diff --git a/drivers/watchdog/Kconfig b/drivers/watchdog/Kconfig

index 79d2589..c389ed7 100644
--- a/drivers/watchdog/Kconfig
+++ b/drivers/watchdog/Kconfig
@@ -421,6 +421,19 @@ config SIRFSOC_WATCHDOG
Support for CSR SiRFprimaII and SiRFatlasVI watchdog. When
the watchdog triggers the system will be reset.

+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 985a66c..cede21e 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

# AVR32 Architecture

diff --git a/drivers/watchdog/qcom-wdt.c b/drivers/watchdog/qcom-wdt.c
new file mode 100644

index 0000000..0f56ca3
--- /dev/null
+++ b/drivers/watchdog/qcom-wdt.c
@@ -0,0 +1,189 @@

+ unsigned long rate;

+ 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);

+
+ writel(0, wdt->base + WDT_EN);
+ writel(1, wdt->base + WDT_RST);

+ writel(wdd->timeout * wdt->rate, 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);

+static int qcom_wdt_probe(struct platform_device *pdev)

+{

+ struct qcom_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)) {

+ ret = PTR_ERR(wdt->base);
+ goto err_out;
+ }

+
+ wdt->clk = devm_clk_get(&pdev->dev, NULL);
+ if (IS_ERR(wdt->clk)) {

+ ret = PTR_ERR(wdt->clk);
+ goto err_out;
+ }

+
+ ret = clk_prepare_enable(wdt->clk);

+ if (ret) {
+ dev_err(&pdev->dev, "failed to setup clock\n");
+ goto err_out;
+ }
+
+ /*

+ * We use the clock rate to calculate the max timeout, so ensure it's
+ * not zero to avoid a divide-by-zero exception.
+ *

+ * WATCHDOG_CORE assumes units of seconds, if the WDT is clocked such
+ * that it would bite before a second elapses it's usefulness is
+ * limited. Bail if this is the case.
+ */
+ wdt->rate = clk_get_rate(wdt->clk);
+ if (wdt->rate == 0 ||
+ wdt->rate > 0x10000000U) {

+ dev_err(&pdev->dev, "invalid clock rate\n");

+ ret = -EINVAL;
+ goto err_clk_unprepare;
+ }
+

+ 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 / wdt->rate;
+
+ /*

+ * If 'timeout-sec' unspecified in devicetree, assume a 30 second

+ * default, unless the max timeout is less than 30 seconds, then use
+ * the max instead.
+ */
+ wdt->wdd.timeout = min(wdt->wdd.max_timeout, 30U);

+ watchdog_init_timeout(&wdt->wdd, 0, &pdev->dev);

+

+ ret = watchdog_register_device(&wdt->wdd);

+ if (ret) {

+ dev_err(&pdev->dev, "failed to register watchdog\n");

+ goto err_clk_unprepare;

+ }
+
+ platform_set_drvdata(pdev, wdt);
+ return 0;
+

+err_clk_unprepare:
+ clk_disable_unprepare(wdt->clk);
+err_out:

+ return ret;
+}
+

+static int qcom_wdt_remove(struct platform_device *pdev)

+{

[Josh Cartwright]

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 <jo...@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..c75566e

--- /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

+- 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>;
+ };

[Guenter Roeck]

On Thu, Sep 25, 2014 at 12:48:51PM -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 <jo...@codeaurora.org>

Hi Josh,

just a couple of minor comments this time (yes, I know,
I am being difficult ;-).

Thanks,
Guenter

This is really unnecessary. Just return PTR_ERR((wdt->base) as you did before.
No need to make the code more complicated than necessary.

Basic rule is: If you can return immediately, do it. Otherwise use goto
and have a single error handler.

> + }
> +
> + wdt->clk = devm_clk_get(&pdev->dev, NULL);
> + if (IS_ERR(wdt->clk)) {
> + ret = PTR_ERR(wdt->clk);
> + goto err_out;

Same here.

> + }
> +
> + ret = clk_prepare_enable(wdt->clk);
> + if (ret) {
> + dev_err(&pdev->dev, "failed to setup clock\n");
> + goto err_out;

and here.

Good idea.

[Guenter Roeck]

On Thu, Sep 25, 2014 at 12:48:53PM -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 <jo...@codeaurora.org>
> ---
> drivers/watchdog/qcom-wdt.c | 38 ++++++++++++++++++++++++++++++++++++++
> 1 file changed, 38 insertions(+)
>
> diff --git a/drivers/watchdog/qcom-wdt.c b/drivers/watchdog/qcom-wdt.c
> index 0f56ca3..8ce339f 100644
> --- a/drivers/watchdog/qcom-wdt.c
> +++ b/drivers/watchdog/qcom-wdt.c
> @@ -10,12 +10,14 @@
> * GNU General Public License for more details.
> *
> */
> +#include <linux/delay.h>

Nitpick: Please keep alphabetical order of include files.
That makes it easier to identify include files later on.

[Guenter Roeck]

Hi Josh,

timeout-sec is optional, not mandatory.

Thanks,
Guenter

[Josh Cartwright]

On Thu, Sep 25, 2014 at 11:43:14AM -0700, Guenter Roeck wrote:
> On Thu, Sep 25, 2014 at 12:48:52PM -0500, Josh Cartwright wrote:

[..]

> > +- timeout-sec : shall contain the default watchdog timeout in seconds,
> > + if unset, the default timeout is 30 seconds
>
> Hi Josh,
>
> timeout-sec is optional, not mandatory.

Indeed, I made this change in v2, but didn't reflect it in the document.
Good catch. Will fix.

Josh

[Josh Cartwright]

On Thu, Sep 25, 2014 at 11:38:57AM -0700, Guenter Roeck wrote:
> On Thu, Sep 25, 2014 at 12:48:51PM -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 <jo...@codeaurora.org>
>
> Hi Josh,
>
> just a couple of minor comments this time (yes, I know,
> I am being difficult ;-).

Difficult, maybe, but at least someone's taking a look! Thanks, again.

[..]
> > +++ b/drivers/watchdog/qcom-wdt.c
[..]

Okay, I can fix these up.

Josh

[Josh Cartwright]

On Thu, Sep 25, 2014 at 11:41:49AM -0700, Guenter Roeck wrote:
> On Thu, Sep 25, 2014 at 12:48:53PM -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 <jo...@codeaurora.org>
> > ---
> > drivers/watchdog/qcom-wdt.c | 38 ++++++++++++++++++++++++++++++++++++++
> > 1 file changed, 38 insertions(+)
> >
> > diff --git a/drivers/watchdog/qcom-wdt.c b/drivers/watchdog/qcom-wdt.c
> > index 0f56ca3..8ce339f 100644
> > --- a/drivers/watchdog/qcom-wdt.c
> > +++ b/drivers/watchdog/qcom-wdt.c
> > @@ -10,12 +10,14 @@
> > * GNU General Public License for more details.
> > *
> > */
> > +#include <linux/delay.h>
>
> Nitpick: Please keep alphabetical order of include files.
> That makes it easier to identify include files later on.

That was my intent, but apparently I fail at the alphabet :). Normally
instead of thinking I pipe the #include list through 'sort'. Not sure
why I didn't do so this time around.

Thanks,
Josh

[Josh Cartwright]

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 <jo...@codeaurora.org>
---
drivers/watchdog/qcom-wdt.c | 38 ++++++++++++++++++++++++++++++++++++++
1 file changed, 38 insertions(+)

diff --git a/drivers/watchdog/qcom-wdt.c b/drivers/watchdog/qcom-wdt.c

index 68db322..aa85618 100644
--- a/drivers/watchdog/qcom-wdt.c
+++ b/drivers/watchdog/qcom-wdt.c
@@ -11,11 +11,13 @@
*
*/
#include <linux/clk.h>
+#include <linux/delay.h>

#include <linux/io.h>
#include <linux/kernel.h>
#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
@@ -26,6 +28,7 @@ struct qcom_wdt {
struct watchdog_device wdd;
struct clk *clk;
unsigned long rate;
+ struct notifier_block restart_nb;
void __iomem *base;
};

@@ -84,6 +87,32 @@ 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)

+{

@@ -147,6 +176,14 @@ static int qcom_wdt_probe(struct platform_device *pdev)

goto err_clk_unprepare;
}

+ /*
+ * 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;

@@ -159,6 +196,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;

[Josh Cartwright]

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 v3:
- More error code cleanup, resolved nits
- Documented 'timeout-sec' property as optional

Changes since v2:
- "input clock phandle" -> "input clock" in device tree documentation
- Fixup error handling paths during probe()
- Use a more sane timeout (128ms), and add msleep()
- Add some additional sanitation for clock rates

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 | 24 +++

drivers/watchdog/Kconfig | 13 ++
drivers/watchdog/Makefile | 1 +

drivers/watchdog/qcom-wdt.c | 224 +++++++++++++++++++++
4 files changed, 262 insertions(+)
create mode 100644 Documentation/devicetree/bindings/watchdog/qcom-wdt.txt
create mode 100644 drivers/watchdog/qcom-wdt.c

[Josh Cartwright]

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 <jo...@codeaurora.org>
---

drivers/watchdog/Kconfig | 13 ++++
drivers/watchdog/Makefile | 1 +

drivers/watchdog/qcom-wdt.c | 186 ++++++++++++++++++++++++++++++++++++++++++++
3 files changed, 200 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..68db322
--- /dev/null
+++ b/drivers/watchdog/qcom-wdt.c
@@ -0,0 +1,186 @@

+{
+ struct qcom_wdt *wdt = to_qcom_wdt(wdd);

+
+ writel(0, wdt->base + WDT_EN);
+ writel(1, wdt->base + WDT_RST);

+ writel(wdd->timeout * wdt->rate, 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);

+static int qcom_wdt_probe(struct platform_device *pdev)

+{

+ struct qcom_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);

+
+ wdt->clk = devm_clk_get(&pdev->dev, NULL);
+ if (IS_ERR(wdt->clk)) {

+ dev_err(&pdev->dev, "failed to get input clock\n");
+ 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.
+ *
+ * WATCHDOG_CORE assumes units of seconds, if the WDT is clocked such
+ * that it would bite before a second elapses it's usefulness is
+ * limited. Bail if this is the case.
+ */
+ wdt->rate = clk_get_rate(wdt->clk);
+ if (wdt->rate == 0 ||
+ wdt->rate > 0x10000000U) {
+ dev_err(&pdev->dev, "invalid clock rate\n");
+ ret = -EINVAL;
+ goto err_clk_unprepare;
+ }
+
+ 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 / wdt->rate;

+
+ /*

+ * If 'timeout-sec' unspecified in devicetree, assume a 30 second
+ * default, unless the max timeout is less than 30 seconds, then use
+ * the max instead.
+ */
+ wdt->wdd.timeout = min(wdt->wdd.max_timeout, 30U);

+ watchdog_init_timeout(&wdt->wdd, 0, &pdev->dev);
+
+ ret = watchdog_register_device(&wdt->wdd);

+ if (ret) {

+ dev_err(&pdev->dev, "failed to register watchdog\n");
+ goto err_clk_unprepare;
+ }
+
+ platform_set_drvdata(pdev, wdt);
+ return 0;
+
+err_clk_unprepare:
+ clk_disable_unprepare(wdt->clk);

+ return ret;
+}
+
+static int qcom_wdt_remove(struct platform_device *pdev)

+{

[Josh Cartwright]

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 <jo...@codeaurora.org>
---

.../devicetree/bindings/watchdog/qcom-wdt.txt | 24 ++++++++++++++++++++++
1 file changed, 24 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..4726924
--- /dev/null
+++ b/Documentation/devicetree/bindings/watchdog/qcom-wdt.txt
@@ -0,0 +1,24 @@

+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
+

+Optional properties :

+- 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>;
+ };

[Guenter Roeck]

On 09/25/2014 03:51 PM, 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 <jo...@codeaurora.org>

Reviewed-by: Guenter Roeck <li...@roeck-us.net>

[Guenter Roeck]

On 09/25/2014 03:51 PM, 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 <jo...@codeaurora.org>

Reviewed-by: Guenter Roeck <li...@roeck-us.net>

[Guenter Roeck]

On 09/25/2014 03:51 PM, Josh Cartwright wrote:

> 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 <jo...@codeaurora.org>

Reviewed-by: Guenter Roeck <li...@roeck-us.net>