diff mbox

[1/3] watchdog: qcom: add support for KPSS WDT

Message ID 32d17907ad1dfdcafe4e76f33adc2ff22631cd28.1411078425.git.joshc@codeaurora.org (mailing list archive)
State Superseded, archived
Headers show

Commit Message

Josh Cartwright Sept. 18, 2014, 10:26 p.m. UTC 
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    |  10 +++
 drivers/watchdog/Makefile   |   1 +
 drivers/watchdog/qcom-wdt.c | 145 ++++++++++++++++++++++++++++++++++++++++++++
 3 files changed, 156 insertions(+)
 create mode 100644 drivers/watchdog/qcom-wdt.c

Comments

Guenter Roeck Sept. 19, 2014, 2:41 a.m. UTC | #1 
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 <joshc@codeaurora.org>

Hi Josh,

comments inline.

Thanks,
Guenter

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

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

--
To unsubscribe from this list: send the line "unsubscribe linux-arm-msm" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Josh Cartwright Sept. 19, 2014, 3:24 a.m. UTC | #2 
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 <joshc@codeaurora.org>
> 
> Hi Josh,
> 
> comments inline.

Thanks for taking a look!

[..]
> >+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;
> >+	}
> >+
>
> 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

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.

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

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
Guenter Roeck Sept. 19, 2014, 3:41 a.m. UTC | #3 
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 <joshc@codeaurora.org>
>>
>> Hi Josh,
>>
>> comments inline.
>
> Thanks for taking a look!
>
> [..]
>>> +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;
>>> +	}
>>> +
>>
>> 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
>
> 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.
>

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.

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

--
To unsubscribe from this list: send the line "unsubscribe linux-arm-msm" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Josh Cartwright Sept. 19, 2014, 4:25 p.m. UTC | #4 
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 <joshc@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?
Guenter Roeck Sept. 19, 2014, 4:56 p.m. UTC | #5 
On Fri, Sep 19, 2014 at 11:25:50AM -0500, Josh Cartwright wrote:
> 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 <joshc@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. :)
> 
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
--
To unsubscribe from this list: send the line "unsubscribe linux-arm-msm" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
diff mbox

Patch

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