linux-watchdog.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [PATCH v3] watchdog: Add Broadcom BCM2835 watchdog timer driver
       [not found] <1364135137-26691-1-git-send-email-lkundrak@v3.sk>
@ 2013-03-26 17:50 ` Lubomir Rintel
  2013-03-26 21:03   ` Guenter Roeck
                     ` (3 more replies)
  0 siblings, 4 replies; 17+ messages in thread
From: Lubomir Rintel @ 2013-03-26 17:50 UTC (permalink / raw)
  To: linux-kernel
  Cc: Lubomir Rintel, Stephen Warren, Wim Van Sebroeck, Guenter Roeck,
	linux-rpi-kernel, linux-watchdog

This adds a driver for watchdog timer hardware present on Broadcom BCM2835 SoC,
used in Raspberry Pi and Roku 2 devices.

Signed-off-by: Lubomir Rintel <lkundrak@v3.sk>
Cc: Stephen Warren <swarren@wwwdotorg.org>
Cc: Wim Van Sebroeck <wim@iguana.be>
Cc: Guenter Roeck <linux@roeck-us.net>
Cc: linux-rpi-kernel@lists.infradead.org
Cc: linux-watchdog@vger.kernel.org
---
Changes for v2:
    - Use per-device structure instead of global variables for lock and memory base
    - Fix default timeout setting
    - Do not leak memory if probe fails
    - Style fixes
    - Split out defconfig into a separate commit
    - Meaningful commit message with credit

Changes for v3:
    - Add proper copyright notice to header
    - Drop status constants, we don't use them
    - Fix heartbeat parameter's type
    - Log driver initialization message once the device is probed

 .../bindings/watchdog/brcm,bcm2835-pm-wdog.txt     |    5 +
 drivers/watchdog/Kconfig                           |   11 ++
 drivers/watchdog/Makefile                          |    1 +
 drivers/watchdog/bcm2835_wdt.c                     |  190 ++++++++++++++++++++
 4 files changed, 207 insertions(+), 0 deletions(-)
 create mode 100644 drivers/watchdog/bcm2835_wdt.c

diff --git a/Documentation/devicetree/bindings/watchdog/brcm,bcm2835-pm-wdog.txt b/Documentation/devicetree/bindings/watchdog/brcm,bcm2835-pm-wdog.txt
index d209366..f801d71 100644
--- a/Documentation/devicetree/bindings/watchdog/brcm,bcm2835-pm-wdog.txt
+++ b/Documentation/devicetree/bindings/watchdog/brcm,bcm2835-pm-wdog.txt
@@ -5,9 +5,14 @@ Required properties:
 - compatible : should be "brcm,bcm2835-pm-wdt"
 - reg : Specifies base physical address and size of the registers.
 
+Optional properties:
+
+- timeout-sec   : Contains the watchdog timeout in seconds
+
 Example:
 
 watchdog {
 	compatible = "brcm,bcm2835-pm-wdt";
 	reg = <0x7e100000 0x28>;
+	timeout-sec = <10>;
 };
diff --git a/drivers/watchdog/Kconfig b/drivers/watchdog/Kconfig
index 9fcc70c..f4e4a8e 100644
--- a/drivers/watchdog/Kconfig
+++ b/drivers/watchdog/Kconfig
@@ -1098,6 +1098,17 @@ config BCM63XX_WDT
 	  To compile this driver as a loadable module, choose M here.
 	  The module will be called bcm63xx_wdt.
 
+config BCM2835_WDT
+	tristate "Broadcom BCM2835 hardware watchdog"
+	depends on ARCH_BCM2835
+	select WATCHDOG_CORE
+	help
+	  Watchdog driver for the built in watchdog hardware in Broadcom
+	  BCM2835 SoC.
+
+	  To compile this driver as a loadable module, choose M here.
+	  The module will be called bcm2835_wdt.
+
 config LANTIQ_WDT
 	tristate "Lantiq SoC watchdog"
 	depends on LANTIQ
diff --git a/drivers/watchdog/Makefile b/drivers/watchdog/Makefile
index a300b94..1bf5675 100644
--- a/drivers/watchdog/Makefile
+++ b/drivers/watchdog/Makefile
@@ -54,6 +54,7 @@ obj-$(CONFIG_TS72XX_WATCHDOG) += ts72xx_wdt.o
 obj-$(CONFIG_IMX2_WDT) += imx2_wdt.o
 obj-$(CONFIG_UX500_WATCHDOG) += ux500_wdt.o
 obj-$(CONFIG_RETU_WATCHDOG) += retu_wdt.o
+obj-$(CONFIG_BCM2835_WDT) += bcm2835_wdt.o
 
 # AVR32 Architecture
 obj-$(CONFIG_AT32AP700X_WDT) += at32ap700x_wdt.o
diff --git a/drivers/watchdog/bcm2835_wdt.c b/drivers/watchdog/bcm2835_wdt.c
new file mode 100644
index 0000000..13a8c00
--- /dev/null
+++ b/drivers/watchdog/bcm2835_wdt.c
@@ -0,0 +1,190 @@
+/*
+ * Watchdog driver for Broadcom BCM2835
+ *
+ * Interface to the Broadcom BCM2835 watchdog timer hardware is based on
+ * "bcm2708_wdog" driver written by Luke Diamand that was obtained from branch
+ * "rpi-3.6.y" of git://github.com/raspberrypi/linux.git
+ *
+ * (c) Copyright 2010 Broadcom Europe Ltd
+ * Copyright (C) 2013 Lubomir Rintel <lkundrak@v3.sk>
+ *
+ * This program is free software; you can redistribute it and/or
+ * modify it under the terms of the GNU General Public License
+ * as published by the Free Software Foundation; either version
+ * 2 of the License, or (at your option) any later version.
+ */
+
+#include <linux/types.h>
+#include <linux/module.h>
+#include <linux/io.h>
+#include <linux/watchdog.h>
+#include <linux/platform_device.h>
+#include <linux/of_address.h>
+#include <linux/miscdevice.h>
+
+#define PM_RSTC				0x1c
+#define PM_WDOG				0x24
+
+#define PM_PASSWORD			0x5a000000
+
+#define PM_WDOG_TIME_SET		0x000fffff
+#define PM_RSTC_WRCFG_CLR		0xffffffcf
+#define PM_RSTC_WRCFG_SET		0x00000030
+#define PM_RSTC_WRCFG_FULL_RESET	0x00000020
+#define PM_RSTC_RESET			0x00000102
+
+#define SECS_TO_WDOG_TICKS(x) ((x) << 16)
+#define WDOG_TICKS_TO_SECS(x) ((x) >> 16)
+
+struct bcm2835_wdt {
+	void __iomem		*base;
+	spinlock_t		lock;
+};
+
+static unsigned int heartbeat = 0;
+static bool nowayout = WATCHDOG_NOWAYOUT;
+
+static int bcm2835_wdt_start(struct watchdog_device *wdog)
+{
+	struct bcm2835_wdt *wdt = watchdog_get_drvdata(wdog);
+	uint32_t cur;
+	unsigned long flags;
+
+	spin_lock_irqsave(&wdt->lock, flags);
+
+	writel_relaxed(PM_PASSWORD | (SECS_TO_WDOG_TICKS(wdog->timeout) &
+				PM_WDOG_TIME_SET), wdt->base + PM_WDOG);
+	cur = readl_relaxed(wdt->base + PM_RSTC);
+	writel_relaxed(PM_PASSWORD | (cur & PM_RSTC_WRCFG_CLR) |
+		  PM_RSTC_WRCFG_FULL_RESET, wdt->base + PM_RSTC);
+
+	spin_unlock_irqrestore(&wdt->lock, flags);
+
+	return 0;
+}
+
+static int bcm2835_wdt_stop(struct watchdog_device *wdog)
+{
+	struct bcm2835_wdt *wdt = watchdog_get_drvdata(wdog);
+
+	writel_relaxed(PM_PASSWORD | PM_RSTC_RESET, wdt->base + PM_RSTC);
+	dev_info(wdog->dev, "Watchdog timer stopped");
+	return 0;
+}
+
+static int bcm2835_wdt_set_timeout(struct watchdog_device *wdog, unsigned int t)
+{
+	wdog->timeout = t;
+	return 0;
+}
+
+static unsigned int bcm2835_wdt_get_timeleft(struct watchdog_device *wdog)
+{
+	struct bcm2835_wdt *wdt = watchdog_get_drvdata(wdog);
+
+	uint32_t ret = readl_relaxed(wdt->base + PM_WDOG);
+	return WDOG_TICKS_TO_SECS(ret & PM_WDOG_TIME_SET);
+}
+
+static struct watchdog_ops bcm2835_wdt_ops = {
+	.owner =	THIS_MODULE,
+	.start =	bcm2835_wdt_start,
+	.stop =		bcm2835_wdt_stop,
+	.set_timeout =	bcm2835_wdt_set_timeout,
+	.get_timeleft =	bcm2835_wdt_get_timeleft,
+};
+
+static struct watchdog_info bcm2835_wdt_info = {
+	.options =	WDIOF_SETTIMEOUT | WDIOF_MAGICCLOSE |
+			WDIOF_KEEPALIVEPING,
+	.identity =	"Broadcom BCM2835 Watchdog timer",
+};
+
+static struct watchdog_device bcm2835_wdt_wdd = {
+	.info =		&bcm2835_wdt_info,
+	.ops =		&bcm2835_wdt_ops,
+	.min_timeout =	1,
+	.max_timeout =	WDOG_TICKS_TO_SECS(PM_WDOG_TIME_SET),
+	.timeout =	WDOG_TICKS_TO_SECS(PM_WDOG_TIME_SET),
+};
+
+static int bcm2835_wdt_probe(struct platform_device *pdev)
+{
+	struct device *dev = &pdev->dev;
+	struct device_node *np = dev->of_node;
+	struct bcm2835_wdt *wdt;
+	int err;
+
+	wdt = devm_kzalloc(dev, sizeof(struct bcm2835_wdt), GFP_KERNEL);
+	if (!wdt) {
+		dev_err(dev, "Failed to allocate memory for watchdog device");
+		return -ENOMEM;
+	}
+
+	spin_lock_init(&wdt->lock);
+
+	wdt->base = of_iomap(np, 0);
+	if (!wdt->base) {
+		dev_err(dev, "Failed to remap watchdog regs");
+		return -ENODEV;
+	}
+
+	platform_set_drvdata(pdev, wdt);
+	watchdog_set_drvdata(&bcm2835_wdt_wdd, wdt);
+	watchdog_init_timeout(&bcm2835_wdt_wdd, heartbeat, dev);
+	watchdog_set_nowayout(&bcm2835_wdt_wdd, nowayout);
+	err = watchdog_register_device(&bcm2835_wdt_wdd);
+	if (err) {
+		dev_err(dev, "Failed to register watchdog device");
+		iounmap(wdt->base);
+	}
+	dev_info(dev, "Broadcom BCM2835 watchdog timer");
+
+	return err;
+}
+
+static int bcm2835_wdt_remove(struct platform_device *pdev)
+{
+	struct bcm2835_wdt *wdt = platform_get_drvdata(pdev);
+
+	platform_set_drvdata(pdev, NULL);
+	watchdog_unregister_device(&bcm2835_wdt_wdd);
+	iounmap(wdt->base);
+
+	return 0;
+}
+
+static void bcm2835_wdt_shutdown(struct platform_device *pdev)
+{
+	bcm2835_wdt_stop(&bcm2835_wdt_wdd);
+}
+
+static const struct of_device_id bcm2835_wdt_of_match[] = {
+	{ .compatible = "brcm,bcm2835-pm-wdt", },
+	{},
+};
+MODULE_DEVICE_TABLE(of, bcm2835_wdt_of_match);
+
+static struct platform_driver bcm2835_wdt_driver = {
+	.probe		= bcm2835_wdt_probe,
+	.remove		= bcm2835_wdt_remove,
+	.shutdown	= bcm2835_wdt_shutdown,
+	.driver = {
+		.name =		"bcm2835-wdt",
+		.owner =	THIS_MODULE,
+		.of_match_table = bcm2835_wdt_of_match,
+	},
+};
+module_platform_driver(bcm2835_wdt_driver);
+
+module_param(heartbeat, uint, 0);
+MODULE_PARM_DESC(heartbeat, "Initial watchdog heartbeat in seconds");
+
+module_param(nowayout, bool, 0);
+MODULE_PARM_DESC(nowayout, "Watchdog cannot be stopped once started (default="
+				__MODULE_STRING(WATCHDOG_NOWAYOUT) ")");
+
+MODULE_AUTHOR("Lubomir Rintel <lkundrak@v3.sk>");
+MODULE_DESCRIPTION("Driver for Broadcom BCM2835 watchdog timer");
+MODULE_LICENSE("GPL");
+MODULE_ALIAS_MISCDEV(WATCHDOG_MINOR);
-- 
1.7.1

^ permalink raw reply related	[flat|nested] 17+ messages in thread

* Re: [PATCH v3] watchdog: Add Broadcom BCM2835 watchdog timer driver
  2013-03-26 17:50 ` [PATCH v3] watchdog: Add Broadcom BCM2835 watchdog timer driver Lubomir Rintel
@ 2013-03-26 21:03   ` Guenter Roeck
  2013-03-27 16:39     ` Lubomir Rintel
  2013-03-27  3:40   ` Stephen Warren
                     ` (2 subsequent siblings)
  3 siblings, 1 reply; 17+ messages in thread
From: Guenter Roeck @ 2013-03-26 21:03 UTC (permalink / raw)
  To: Lubomir Rintel
  Cc: linux-kernel, Stephen Warren, Wim Van Sebroeck, linux-rpi-kernel,
	linux-watchdog

On Tue, Mar 26, 2013 at 06:50:00PM +0100, Lubomir Rintel wrote:
> This adds a driver for watchdog timer hardware present on Broadcom BCM2835 SoC,
> used in Raspberry Pi and Roku 2 devices.
> 
> Signed-off-by: Lubomir Rintel <lkundrak@v3.sk>
> Cc: Stephen Warren <swarren@wwwdotorg.org>
> Cc: Wim Van Sebroeck <wim@iguana.be>
> Cc: Guenter Roeck <linux@roeck-us.net>
> Cc: linux-rpi-kernel@lists.infradead.org
> Cc: linux-watchdog@vger.kernel.org
> ---
> Changes for v2:
>     - Use per-device structure instead of global variables for lock and memory base
>     - Fix default timeout setting
>     - Do not leak memory if probe fails
>     - Style fixes
>     - Split out defconfig into a separate commit
>     - Meaningful commit message with credit
> 
> Changes for v3:
>     - Add proper copyright notice to header
>     - Drop status constants, we don't use them
>     - Fix heartbeat parameter's type
>     - Log driver initialization message once the device is probed
> 
>  .../bindings/watchdog/brcm,bcm2835-pm-wdog.txt     |    5 +
>  drivers/watchdog/Kconfig                           |   11 ++
>  drivers/watchdog/Makefile                          |    1 +
>  drivers/watchdog/bcm2835_wdt.c                     |  190 ++++++++++++++++++++
>  4 files changed, 207 insertions(+), 0 deletions(-)
>  create mode 100644 drivers/watchdog/bcm2835_wdt.c
> 
> diff --git a/Documentation/devicetree/bindings/watchdog/brcm,bcm2835-pm-wdog.txt b/Documentation/devicetree/bindings/watchdog/brcm,bcm2835-pm-wdog.txt
> index d209366..f801d71 100644
> --- a/Documentation/devicetree/bindings/watchdog/brcm,bcm2835-pm-wdog.txt
> +++ b/Documentation/devicetree/bindings/watchdog/brcm,bcm2835-pm-wdog.txt
> @@ -5,9 +5,14 @@ Required properties:
>  - compatible : should be "brcm,bcm2835-pm-wdt"
>  - reg : Specifies base physical address and size of the registers.
>  
> +Optional properties:
> +
> +- timeout-sec   : Contains the watchdog timeout in seconds
> +
>  Example:
>  
>  watchdog {
>  	compatible = "brcm,bcm2835-pm-wdt";
>  	reg = <0x7e100000 0x28>;
> +	timeout-sec = <10>;
>  };
> diff --git a/drivers/watchdog/Kconfig b/drivers/watchdog/Kconfig
> index 9fcc70c..f4e4a8e 100644
> --- a/drivers/watchdog/Kconfig
> +++ b/drivers/watchdog/Kconfig
> @@ -1098,6 +1098,17 @@ config BCM63XX_WDT
>  	  To compile this driver as a loadable module, choose M here.
>  	  The module will be called bcm63xx_wdt.
>  
> +config BCM2835_WDT
> +	tristate "Broadcom BCM2835 hardware watchdog"
> +	depends on ARCH_BCM2835
> +	select WATCHDOG_CORE
> +	help
> +	  Watchdog driver for the built in watchdog hardware in Broadcom
> +	  BCM2835 SoC.
> +
> +	  To compile this driver as a loadable module, choose M here.
> +	  The module will be called bcm2835_wdt.
> +
>  config LANTIQ_WDT
>  	tristate "Lantiq SoC watchdog"
>  	depends on LANTIQ
> diff --git a/drivers/watchdog/Makefile b/drivers/watchdog/Makefile
> index a300b94..1bf5675 100644
> --- a/drivers/watchdog/Makefile
> +++ b/drivers/watchdog/Makefile
> @@ -54,6 +54,7 @@ obj-$(CONFIG_TS72XX_WATCHDOG) += ts72xx_wdt.o
>  obj-$(CONFIG_IMX2_WDT) += imx2_wdt.o
>  obj-$(CONFIG_UX500_WATCHDOG) += ux500_wdt.o
>  obj-$(CONFIG_RETU_WATCHDOG) += retu_wdt.o
> +obj-$(CONFIG_BCM2835_WDT) += bcm2835_wdt.o
>  
>  # AVR32 Architecture
>  obj-$(CONFIG_AT32AP700X_WDT) += at32ap700x_wdt.o
> diff --git a/drivers/watchdog/bcm2835_wdt.c b/drivers/watchdog/bcm2835_wdt.c
> new file mode 100644
> index 0000000..13a8c00
> --- /dev/null
> +++ b/drivers/watchdog/bcm2835_wdt.c
> @@ -0,0 +1,190 @@
> +/*
> + * Watchdog driver for Broadcom BCM2835
> + *
> + * Interface to the Broadcom BCM2835 watchdog timer hardware is based on
> + * "bcm2708_wdog" driver written by Luke Diamand that was obtained from branch
> + * "rpi-3.6.y" of git://github.com/raspberrypi/linux.git
> + *
> + * (c) Copyright 2010 Broadcom Europe Ltd
> + * Copyright (C) 2013 Lubomir Rintel <lkundrak@v3.sk>
> + *
> + * This program is free software; you can redistribute it and/or
> + * modify it under the terms of the GNU General Public License
> + * as published by the Free Software Foundation; either version
> + * 2 of the License, or (at your option) any later version.
> + */
> +
> +#include <linux/types.h>
> +#include <linux/module.h>
> +#include <linux/io.h>
> +#include <linux/watchdog.h>
> +#include <linux/platform_device.h>
> +#include <linux/of_address.h>
> +#include <linux/miscdevice.h>
> +
> +#define PM_RSTC				0x1c
> +#define PM_WDOG				0x24
> +
> +#define PM_PASSWORD			0x5a000000
> +
> +#define PM_WDOG_TIME_SET		0x000fffff
> +#define PM_RSTC_WRCFG_CLR		0xffffffcf
> +#define PM_RSTC_WRCFG_SET		0x00000030
> +#define PM_RSTC_WRCFG_FULL_RESET	0x00000020
> +#define PM_RSTC_RESET			0x00000102
> +
> +#define SECS_TO_WDOG_TICKS(x) ((x) << 16)
> +#define WDOG_TICKS_TO_SECS(x) ((x) >> 16)
> +
> +struct bcm2835_wdt {
> +	void __iomem		*base;
> +	spinlock_t		lock;
> +};
> +
> +static unsigned int heartbeat = 0;

checkpatch.pl says:

ERROR: do not initialise statics to 0 or NULL
#178: FILE: drivers/watchdog/bcm2835_wdt.c:44:
+static unsigned int heartbeat = 0;

> +static bool nowayout = WATCHDOG_NOWAYOUT;
> +
> +static int bcm2835_wdt_start(struct watchdog_device *wdog)
> +{
> +	struct bcm2835_wdt *wdt = watchdog_get_drvdata(wdog);
> +	uint32_t cur;
> +	unsigned long flags;
> +
> +	spin_lock_irqsave(&wdt->lock, flags);
> +
> +	writel_relaxed(PM_PASSWORD | (SECS_TO_WDOG_TICKS(wdog->timeout) &
> +				PM_WDOG_TIME_SET), wdt->base + PM_WDOG);
> +	cur = readl_relaxed(wdt->base + PM_RSTC);
> +	writel_relaxed(PM_PASSWORD | (cur & PM_RSTC_WRCFG_CLR) |
> +		  PM_RSTC_WRCFG_FULL_RESET, wdt->base + PM_RSTC);
> +
> +	spin_unlock_irqrestore(&wdt->lock, flags);
> +
> +	return 0;
> +}
> +
> +static int bcm2835_wdt_stop(struct watchdog_device *wdog)
> +{
> +	struct bcm2835_wdt *wdt = watchdog_get_drvdata(wdog);
> +
> +	writel_relaxed(PM_PASSWORD | PM_RSTC_RESET, wdt->base + PM_RSTC);
> +	dev_info(wdog->dev, "Watchdog timer stopped");
> +	return 0;
> +}
> +
> +static int bcm2835_wdt_set_timeout(struct watchdog_device *wdog, unsigned int t)
> +{
> +	wdog->timeout = t;
> +	return 0;
> +}
> +
> +static unsigned int bcm2835_wdt_get_timeleft(struct watchdog_device *wdog)
> +{
> +	struct bcm2835_wdt *wdt = watchdog_get_drvdata(wdog);
> +
> +	uint32_t ret = readl_relaxed(wdt->base + PM_WDOG);
> +	return WDOG_TICKS_TO_SECS(ret & PM_WDOG_TIME_SET);
> +}
> +
> +static struct watchdog_ops bcm2835_wdt_ops = {
> +	.owner =	THIS_MODULE,
> +	.start =	bcm2835_wdt_start,
> +	.stop =		bcm2835_wdt_stop,
> +	.set_timeout =	bcm2835_wdt_set_timeout,
> +	.get_timeleft =	bcm2835_wdt_get_timeleft,
> +};
> +
> +static struct watchdog_info bcm2835_wdt_info = {
> +	.options =	WDIOF_SETTIMEOUT | WDIOF_MAGICCLOSE |
> +			WDIOF_KEEPALIVEPING,
> +	.identity =	"Broadcom BCM2835 Watchdog timer",
> +};
> +
> +static struct watchdog_device bcm2835_wdt_wdd = {
> +	.info =		&bcm2835_wdt_info,
> +	.ops =		&bcm2835_wdt_ops,
> +	.min_timeout =	1,
> +	.max_timeout =	WDOG_TICKS_TO_SECS(PM_WDOG_TIME_SET),
> +	.timeout =	WDOG_TICKS_TO_SECS(PM_WDOG_TIME_SET),
> +};
> +
> +static int bcm2835_wdt_probe(struct platform_device *pdev)
> +{
> +	struct device *dev = &pdev->dev;
> +	struct device_node *np = dev->of_node;
> +	struct bcm2835_wdt *wdt;
> +	int err;
> +
> +	wdt = devm_kzalloc(dev, sizeof(struct bcm2835_wdt), GFP_KERNEL);
> +	if (!wdt) {
> +		dev_err(dev, "Failed to allocate memory for watchdog device");
> +		return -ENOMEM;
> +	}
> +
> +	spin_lock_init(&wdt->lock);
> +
> +	wdt->base = of_iomap(np, 0);
> +	if (!wdt->base) {
> +		dev_err(dev, "Failed to remap watchdog regs");
> +		return -ENODEV;
> +	}
> +
> +	platform_set_drvdata(pdev, wdt);
> +	watchdog_set_drvdata(&bcm2835_wdt_wdd, wdt);
> +	watchdog_init_timeout(&bcm2835_wdt_wdd, heartbeat, dev);
> +	watchdog_set_nowayout(&bcm2835_wdt_wdd, nowayout);
> +	err = watchdog_register_device(&bcm2835_wdt_wdd);
> +	if (err) {
> +		dev_err(dev, "Failed to register watchdog device");
> +		iounmap(wdt->base);
> +	}
> +	dev_info(dev, "Broadcom BCM2835 watchdog timer");
> +
Hmm .. that means you'll get the info message even in the error case.
Is that intentional ? It is inconsistent with the remap error message
above, which does not result in the info message.

> +	return err;
> +}
> +
> +static int bcm2835_wdt_remove(struct platform_device *pdev)
> +{
> +	struct bcm2835_wdt *wdt = platform_get_drvdata(pdev);
> +
> +	platform_set_drvdata(pdev, NULL);

Still unnecessary.

Thanks,
Guenter

^ permalink raw reply	[flat|nested] 17+ messages in thread

* Re: [PATCH v3] watchdog: Add Broadcom BCM2835 watchdog timer driver
  2013-03-26 17:50 ` [PATCH v3] watchdog: Add Broadcom BCM2835 watchdog timer driver Lubomir Rintel
  2013-03-26 21:03   ` Guenter Roeck
@ 2013-03-27  3:40   ` Stephen Warren
  2013-03-27 16:36     ` Lubomir Rintel
  2013-03-27  3:49   ` Stephen Warren
  2013-03-27 16:40   ` [PATCH v4] " Lubomir Rintel
  3 siblings, 1 reply; 17+ messages in thread
From: Stephen Warren @ 2013-03-27  3:40 UTC (permalink / raw)
  To: Lubomir Rintel
  Cc: linux-kernel, Wim Van Sebroeck, Guenter Roeck, linux-rpi-kernel,
	linux-watchdog

On 03/26/2013 11:50 AM, Lubomir Rintel wrote:
> This adds a driver for watchdog timer hardware present on Broadcom BCM2835 SoC,
> used in Raspberry Pi and Roku 2 devices.

Since this patch defines a new DT binding, you should send it to
devicetree-discuss@lists.ozlabs.org too.

> diff --git a/drivers/watchdog/bcm2835_wdt.c b/drivers/watchdog/bcm2835_wdt.c

> +/*
> + * Watchdog driver for Broadcom BCM2835
> + *
> + * Interface to the Broadcom BCM2835 watchdog timer hardware is based on
> + * "bcm2708_wdog" driver written by Luke Diamand that was obtained from branch
> + * "rpi-3.6.y" of git://github.com/raspberrypi/linux.git

I see that the patch isn't S-o-b Luke in the downstream kernel. However,
it is S-o-b Dom Cobley (popcornmix), and they both work for Broadcom, so
I think that's OK.

> +static int bcm2835_wdt_probe(struct platform_device *pdev)
> +{
> +	struct device *dev = &pdev->dev;
> +	struct device_node *np = dev->of_node;
> +	struct bcm2835_wdt *wdt;
> +	int err;
> +
> +	wdt = devm_kzalloc(dev, sizeof(struct bcm2835_wdt), GFP_KERNEL);
> +	if (!wdt) {
> +		dev_err(dev, "Failed to allocate memory for watchdog device");
> +		return -ENOMEM;
> +	}
> +
> +	spin_lock_init(&wdt->lock);
> +
> +	wdt->base = of_iomap(np, 0);
> +	if (!wdt->base) {
> +		dev_err(dev, "Failed to remap watchdog regs");
> +		return -ENODEV;
> +	}
> +
> +	platform_set_drvdata(pdev, wdt);
> +	watchdog_set_drvdata(&bcm2835_wdt_wdd, wdt);

Do you really need both of those? I would have thought just one would
have been enough.

I'd be tempted to put the platform_set_drvdata() call right after the
devm_kzalloc() of wdt, but it's not a big deal either way.

^ permalink raw reply	[flat|nested] 17+ messages in thread

* Re: [PATCH v3] watchdog: Add Broadcom BCM2835 watchdog timer driver
  2013-03-26 17:50 ` [PATCH v3] watchdog: Add Broadcom BCM2835 watchdog timer driver Lubomir Rintel
  2013-03-26 21:03   ` Guenter Roeck
  2013-03-27  3:40   ` Stephen Warren
@ 2013-03-27  3:49   ` Stephen Warren
  2013-03-27 16:40   ` [PATCH v4] " Lubomir Rintel
  3 siblings, 0 replies; 17+ messages in thread
From: Stephen Warren @ 2013-03-27  3:49 UTC (permalink / raw)
  To: Lubomir Rintel
  Cc: linux-kernel, Wim Van Sebroeck, Guenter Roeck, linux-rpi-kernel,
	linux-watchdog

On 03/26/2013 11:50 AM, Lubomir Rintel wrote:
> This adds a driver for watchdog timer hardware present on Broadcom BCM2835 SoC,
> used in Raspberry Pi and Roku 2 devices.

Tested-by: Stephen Warren <swarren@wwwdotorg.org>

^ permalink raw reply	[flat|nested] 17+ messages in thread

* Re: [PATCH v3] watchdog: Add Broadcom BCM2835 watchdog timer driver
  2013-03-27  3:40   ` Stephen Warren
@ 2013-03-27 16:36     ` Lubomir Rintel
  0 siblings, 0 replies; 17+ messages in thread
From: Lubomir Rintel @ 2013-03-27 16:36 UTC (permalink / raw)
  To: Stephen Warren
  Cc: linux-kernel, Wim Van Sebroeck, Guenter Roeck, linux-rpi-kernel,
	linux-watchdog

On Tue, 2013-03-26 at 21:40 -0600, Stephen Warren wrote:
> On 03/26/2013 11:50 AM, Lubomir Rintel wrote:
> > This adds a driver for watchdog timer hardware present on Broadcom BCM2835 SoC,
> > used in Raspberry Pi and Roku 2 devices.
> 
> Since this patch defines a new DT binding, you should send it to
> devicetree-discuss@lists.ozlabs.org too.

Okay.

...
> > + * Interface to the Broadcom BCM2835 watchdog timer hardware is based on
> > + * "bcm2708_wdog" driver written by Luke Diamand that was obtained from branch
> > + * "rpi-3.6.y" of git://github.com/raspberrypi/linux.git
> 
> I see that the patch isn't S-o-b Luke in the downstream kernel. However,
> it is S-o-b Dom Cobley (popcornmix), and they both work for Broadcom, so
> I think that's OK.

Okay, I'll add it.

...
> > +	platform_set_drvdata(pdev, wdt);
> > +	watchdog_set_drvdata(&bcm2835_wdt_wdd, wdt);
> 
> Do you really need both of those? I would have thought just one would
> have been enough.

I think so; I would like to get rid of the platform_*() one, but we need
that in remove hook. All of the existing drivers in drivers/watchdog/
tree either use global data to represent a single device or use two of
the *_set_drvdata() functions -- the watchdog_*() one and dev_*(),
platform_*(), amba_*() or the like.

> I'd be tempted to put the platform_set_drvdata() call right after the
> devm_kzalloc() of wdt, but it's not a big deal either way.

Done.

I'm actually rather thankful for style fixes such as this, since I'm
only getting familiar with what's customary to do there.

Updated patch will follow shortly.

-- 
Lubomir Rintel <lkundrak@v3.sk>


^ permalink raw reply	[flat|nested] 17+ messages in thread

* Re: [PATCH v3] watchdog: Add Broadcom BCM2835 watchdog timer driver
  2013-03-26 21:03   ` Guenter Roeck
@ 2013-03-27 16:39     ` Lubomir Rintel
  0 siblings, 0 replies; 17+ messages in thread
From: Lubomir Rintel @ 2013-03-27 16:39 UTC (permalink / raw)
  To: Guenter Roeck
  Cc: linux-kernel, Stephen Warren, Wim Van Sebroeck, linux-rpi-kernel,
	linux-watchdog

On Tue, 2013-03-26 at 14:03 -0700, Guenter Roeck wrote:
> On Tue, Mar 26, 2013 at 06:50:00PM +0100, Lubomir Rintel wrote:
> > This adds a driver for watchdog timer hardware present on Broadcom BCM2835 SoC,
> > used in Raspberry Pi and Roku 2 devices.

...
> > +static unsigned int heartbeat = 0;
> 
> checkpatch.pl says:
> 
> ERROR: do not initialise statics to 0 or NULL
> #178: FILE: drivers/watchdog/bcm2835_wdt.c:44:
> +static unsigned int heartbeat = 0;

Whoops. Will remove.

> > +	dev_info(dev, "Broadcom BCM2835 watchdog timer");
> > +
> Hmm .. that means you'll get the info message even in the error case.
> Is that intentional ? It is inconsistent with the remap error message
> above, which does not result in the info message.

That was unintentional -- will change that.

> > +	platform_set_drvdata(pdev, NULL);
> 
> Still unnecessary.

Oops -- I'll actually remove it this time.

-- 
Lubomir Rintel <lkundrak@v3.sk>


^ permalink raw reply	[flat|nested] 17+ messages in thread

* [PATCH v4] watchdog: Add Broadcom BCM2835 watchdog timer driver
  2013-03-26 17:50 ` [PATCH v3] watchdog: Add Broadcom BCM2835 watchdog timer driver Lubomir Rintel
                     ` (2 preceding siblings ...)
  2013-03-27  3:49   ` Stephen Warren
@ 2013-03-27 16:40   ` Lubomir Rintel
  2013-03-27 19:52     ` Guenter Roeck
                       ` (2 more replies)
  3 siblings, 3 replies; 17+ messages in thread
From: Lubomir Rintel @ 2013-03-27 16:40 UTC (permalink / raw)
  To: linux-kernel
  Cc: Lubomir Rintel, Dom Cobley, Stephen Warren, Wim Van Sebroeck,
	Guenter Roeck, linux-rpi-kernel, linux-watchdog

This adds a driver for watchdog timer hardware present on Broadcom BCM2835 SoC,
used in Raspberry Pi and Roku 2 devices.

Signed-off-by: Lubomir Rintel <lkundrak@v3.sk>
Signed-off-by: Dom Cobley <popcornmix@gmail.com>
Tested-by: Stephen Warren <swarren@wwwdotorg.org>
Cc: Stephen Warren <swarren@wwwdotorg.org>
Cc: Wim Van Sebroeck <wim@iguana.be>
Cc: Guenter Roeck <linux@roeck-us.net>
Cc: linux-rpi-kernel@lists.infradead.org
Cc: linux-watchdog@vger.kernel.org
---
Changes for v2:
    - Use per-device structure instead of global variables for lock and memory base
    - Fix default timeout setting
    - Do not leak memory if probe fails
    - Style fixes
    - Split out defconfig into a separate commit
    - Meaningful commit message with credit

Changes for v3:
    - Add proper copyright notice to header
    - Drop status constants, we don't use them
    - Fix heartbeat parameter's type
    - Log driver initialization message once the device is probed

Changes for v4:
    - Drop an useless initializer
    - Add a signoff from downstream
    - Do not announce the driver to the log if we failed to probe
    - Set platform data earlier and do not explicitly unset it

 .../bindings/watchdog/brcm,bcm2835-pm-wdog.txt     |    5 +
 drivers/watchdog/Kconfig                           |   11 ++
 drivers/watchdog/Makefile                          |    1 +
 drivers/watchdog/bcm2835_wdt.c                     |  190 ++++++++++++++++++++
 4 files changed, 207 insertions(+), 0 deletions(-)
 create mode 100644 drivers/watchdog/bcm2835_wdt.c

diff --git a/Documentation/devicetree/bindings/watchdog/brcm,bcm2835-pm-wdog.txt b/Documentation/devicetree/bindings/watchdog/brcm,bcm2835-pm-wdog.txt
index d209366..f801d71 100644
--- a/Documentation/devicetree/bindings/watchdog/brcm,bcm2835-pm-wdog.txt
+++ b/Documentation/devicetree/bindings/watchdog/brcm,bcm2835-pm-wdog.txt
@@ -5,9 +5,14 @@ Required properties:
 - compatible : should be "brcm,bcm2835-pm-wdt"
 - reg : Specifies base physical address and size of the registers.
 
+Optional properties:
+
+- timeout-sec   : Contains the watchdog timeout in seconds
+
 Example:
 
 watchdog {
 	compatible = "brcm,bcm2835-pm-wdt";
 	reg = <0x7e100000 0x28>;
+	timeout-sec = <10>;
 };
diff --git a/drivers/watchdog/Kconfig b/drivers/watchdog/Kconfig
index 9fcc70c..f4e4a8e 100644
--- a/drivers/watchdog/Kconfig
+++ b/drivers/watchdog/Kconfig
@@ -1098,6 +1098,17 @@ config BCM63XX_WDT
 	  To compile this driver as a loadable module, choose M here.
 	  The module will be called bcm63xx_wdt.
 
+config BCM2835_WDT
+	tristate "Broadcom BCM2835 hardware watchdog"
+	depends on ARCH_BCM2835
+	select WATCHDOG_CORE
+	help
+	  Watchdog driver for the built in watchdog hardware in Broadcom
+	  BCM2835 SoC.
+
+	  To compile this driver as a loadable module, choose M here.
+	  The module will be called bcm2835_wdt.
+
 config LANTIQ_WDT
 	tristate "Lantiq SoC watchdog"
 	depends on LANTIQ
diff --git a/drivers/watchdog/Makefile b/drivers/watchdog/Makefile
index a300b94..1bf5675 100644
--- a/drivers/watchdog/Makefile
+++ b/drivers/watchdog/Makefile
@@ -54,6 +54,7 @@ obj-$(CONFIG_TS72XX_WATCHDOG) += ts72xx_wdt.o
 obj-$(CONFIG_IMX2_WDT) += imx2_wdt.o
 obj-$(CONFIG_UX500_WATCHDOG) += ux500_wdt.o
 obj-$(CONFIG_RETU_WATCHDOG) += retu_wdt.o
+obj-$(CONFIG_BCM2835_WDT) += bcm2835_wdt.o
 
 # AVR32 Architecture
 obj-$(CONFIG_AT32AP700X_WDT) += at32ap700x_wdt.o
diff --git a/drivers/watchdog/bcm2835_wdt.c b/drivers/watchdog/bcm2835_wdt.c
new file mode 100644
index 0000000..d47c842
--- /dev/null
+++ b/drivers/watchdog/bcm2835_wdt.c
@@ -0,0 +1,190 @@
+/*
+ * Watchdog driver for Broadcom BCM2835
+ *
+ * Interface to the Broadcom BCM2835 watchdog timer hardware is based on
+ * "bcm2708_wdog" driver written by Luke Diamand that was obtained from branch
+ * "rpi-3.6.y" of git://github.com/raspberrypi/linux.git
+ *
+ * (c) Copyright 2010 Broadcom Europe Ltd
+ * Copyright (C) 2013 Lubomir Rintel <lkundrak@v3.sk>
+ *
+ * This program is free software; you can redistribute it and/or
+ * modify it under the terms of the GNU General Public License
+ * as published by the Free Software Foundation; either version
+ * 2 of the License, or (at your option) any later version.
+ */
+
+#include <linux/types.h>
+#include <linux/module.h>
+#include <linux/io.h>
+#include <linux/watchdog.h>
+#include <linux/platform_device.h>
+#include <linux/of_address.h>
+#include <linux/miscdevice.h>
+
+#define PM_RSTC				0x1c
+#define PM_WDOG				0x24
+
+#define PM_PASSWORD			0x5a000000
+
+#define PM_WDOG_TIME_SET		0x000fffff
+#define PM_RSTC_WRCFG_CLR		0xffffffcf
+#define PM_RSTC_WRCFG_SET		0x00000030
+#define PM_RSTC_WRCFG_FULL_RESET	0x00000020
+#define PM_RSTC_RESET			0x00000102
+
+#define SECS_TO_WDOG_TICKS(x) ((x) << 16)
+#define WDOG_TICKS_TO_SECS(x) ((x) >> 16)
+
+struct bcm2835_wdt {
+	void __iomem		*base;
+	spinlock_t		lock;
+};
+
+static unsigned int heartbeat;
+static bool nowayout = WATCHDOG_NOWAYOUT;
+
+static int bcm2835_wdt_start(struct watchdog_device *wdog)
+{
+	struct bcm2835_wdt *wdt = watchdog_get_drvdata(wdog);
+	uint32_t cur;
+	unsigned long flags;
+
+	spin_lock_irqsave(&wdt->lock, flags);
+
+	writel_relaxed(PM_PASSWORD | (SECS_TO_WDOG_TICKS(wdog->timeout) &
+				PM_WDOG_TIME_SET), wdt->base + PM_WDOG);
+	cur = readl_relaxed(wdt->base + PM_RSTC);
+	writel_relaxed(PM_PASSWORD | (cur & PM_RSTC_WRCFG_CLR) |
+		  PM_RSTC_WRCFG_FULL_RESET, wdt->base + PM_RSTC);
+
+	spin_unlock_irqrestore(&wdt->lock, flags);
+
+	return 0;
+}
+
+static int bcm2835_wdt_stop(struct watchdog_device *wdog)
+{
+	struct bcm2835_wdt *wdt = watchdog_get_drvdata(wdog);
+
+	writel_relaxed(PM_PASSWORD | PM_RSTC_RESET, wdt->base + PM_RSTC);
+	dev_info(wdog->dev, "Watchdog timer stopped");
+	return 0;
+}
+
+static int bcm2835_wdt_set_timeout(struct watchdog_device *wdog, unsigned int t)
+{
+	wdog->timeout = t;
+	return 0;
+}
+
+static unsigned int bcm2835_wdt_get_timeleft(struct watchdog_device *wdog)
+{
+	struct bcm2835_wdt *wdt = watchdog_get_drvdata(wdog);
+
+	uint32_t ret = readl_relaxed(wdt->base + PM_WDOG);
+	return WDOG_TICKS_TO_SECS(ret & PM_WDOG_TIME_SET);
+}
+
+static struct watchdog_ops bcm2835_wdt_ops = {
+	.owner =	THIS_MODULE,
+	.start =	bcm2835_wdt_start,
+	.stop =		bcm2835_wdt_stop,
+	.set_timeout =	bcm2835_wdt_set_timeout,
+	.get_timeleft =	bcm2835_wdt_get_timeleft,
+};
+
+static struct watchdog_info bcm2835_wdt_info = {
+	.options =	WDIOF_SETTIMEOUT | WDIOF_MAGICCLOSE |
+			WDIOF_KEEPALIVEPING,
+	.identity =	"Broadcom BCM2835 Watchdog timer",
+};
+
+static struct watchdog_device bcm2835_wdt_wdd = {
+	.info =		&bcm2835_wdt_info,
+	.ops =		&bcm2835_wdt_ops,
+	.min_timeout =	1,
+	.max_timeout =	WDOG_TICKS_TO_SECS(PM_WDOG_TIME_SET),
+	.timeout =	WDOG_TICKS_TO_SECS(PM_WDOG_TIME_SET),
+};
+
+static int bcm2835_wdt_probe(struct platform_device *pdev)
+{
+	struct device *dev = &pdev->dev;
+	struct device_node *np = dev->of_node;
+	struct bcm2835_wdt *wdt;
+	int err;
+
+	wdt = devm_kzalloc(dev, sizeof(struct bcm2835_wdt), GFP_KERNEL);
+	if (!wdt) {
+		dev_err(dev, "Failed to allocate memory for watchdog device");
+		return -ENOMEM;
+	}
+	platform_set_drvdata(pdev, wdt);
+
+	spin_lock_init(&wdt->lock);
+
+	wdt->base = of_iomap(np, 0);
+	if (!wdt->base) {
+		dev_err(dev, "Failed to remap watchdog regs");
+		return -ENODEV;
+	}
+
+	watchdog_set_drvdata(&bcm2835_wdt_wdd, wdt);
+	watchdog_init_timeout(&bcm2835_wdt_wdd, heartbeat, dev);
+	watchdog_set_nowayout(&bcm2835_wdt_wdd, nowayout);
+	err = watchdog_register_device(&bcm2835_wdt_wdd);
+	if (err) {
+		dev_err(dev, "Failed to register watchdog device");
+		iounmap(wdt->base);
+		return err;
+	}
+
+	dev_info(dev, "Broadcom BCM2835 watchdog timer");
+	return 0;
+}
+
+static int bcm2835_wdt_remove(struct platform_device *pdev)
+{
+	struct bcm2835_wdt *wdt = platform_get_drvdata(pdev);
+
+	watchdog_unregister_device(&bcm2835_wdt_wdd);
+	iounmap(wdt->base);
+
+	return 0;
+}
+
+static void bcm2835_wdt_shutdown(struct platform_device *pdev)
+{
+	bcm2835_wdt_stop(&bcm2835_wdt_wdd);
+}
+
+static const struct of_device_id bcm2835_wdt_of_match[] = {
+	{ .compatible = "brcm,bcm2835-pm-wdt", },
+	{},
+};
+MODULE_DEVICE_TABLE(of, bcm2835_wdt_of_match);
+
+static struct platform_driver bcm2835_wdt_driver = {
+	.probe		= bcm2835_wdt_probe,
+	.remove		= bcm2835_wdt_remove,
+	.shutdown	= bcm2835_wdt_shutdown,
+	.driver = {
+		.name =		"bcm2835-wdt",
+		.owner =	THIS_MODULE,
+		.of_match_table = bcm2835_wdt_of_match,
+	},
+};
+module_platform_driver(bcm2835_wdt_driver);
+
+module_param(heartbeat, uint, 0);
+MODULE_PARM_DESC(heartbeat, "Initial watchdog heartbeat in seconds");
+
+module_param(nowayout, bool, 0);
+MODULE_PARM_DESC(nowayout, "Watchdog cannot be stopped once started (default="
+				__MODULE_STRING(WATCHDOG_NOWAYOUT) ")");
+
+MODULE_AUTHOR("Lubomir Rintel <lkundrak@v3.sk>");
+MODULE_DESCRIPTION("Driver for Broadcom BCM2835 watchdog timer");
+MODULE_LICENSE("GPL");
+MODULE_ALIAS_MISCDEV(WATCHDOG_MINOR);
-- 
1.7.1

^ permalink raw reply related	[flat|nested] 17+ messages in thread

* Re: [PATCH v4] watchdog: Add Broadcom BCM2835 watchdog timer driver
  2013-03-27 16:40   ` [PATCH v4] " Lubomir Rintel
@ 2013-03-27 19:52     ` Guenter Roeck
  2013-03-28  3:00     ` Stephen Warren
  2013-05-17  2:59     ` [PATCH v4] " Stephen Warren
  2 siblings, 0 replies; 17+ messages in thread
From: Guenter Roeck @ 2013-03-27 19:52 UTC (permalink / raw)
  To: Lubomir Rintel
  Cc: linux-kernel, Dom Cobley, Stephen Warren, Wim Van Sebroeck,
	linux-rpi-kernel, linux-watchdog

On Wed, Mar 27, 2013 at 05:40:24PM +0100, Lubomir Rintel wrote:
> This adds a driver for watchdog timer hardware present on Broadcom BCM2835 SoC,
> used in Raspberry Pi and Roku 2 devices.
> 
> Signed-off-by: Lubomir Rintel <lkundrak@v3.sk>
> Signed-off-by: Dom Cobley <popcornmix@gmail.com>
> Tested-by: Stephen Warren <swarren@wwwdotorg.org>
> Cc: Stephen Warren <swarren@wwwdotorg.org>
> Cc: Wim Van Sebroeck <wim@iguana.be>
> Cc: Guenter Roeck <linux@roeck-us.net>

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

^ permalink raw reply	[flat|nested] 17+ messages in thread

* Re: [PATCH v4] watchdog: Add Broadcom BCM2835 watchdog timer driver
  2013-03-27 16:40   ` [PATCH v4] " Lubomir Rintel
  2013-03-27 19:52     ` Guenter Roeck
@ 2013-03-28  3:00     ` Stephen Warren
  2013-03-28  3:50       ` Guenter Roeck
  2013-05-26 14:22       ` Wim Van Sebroeck
  2013-05-17  2:59     ` [PATCH v4] " Stephen Warren
  2 siblings, 2 replies; 17+ messages in thread
From: Stephen Warren @ 2013-03-28  3:00 UTC (permalink / raw)
  To: Lubomir Rintel
  Cc: linux-kernel, Dom Cobley, Wim Van Sebroeck, Guenter Roeck,
	linux-rpi-kernel, linux-watchdog

On 03/27/2013 10:40 AM, Lubomir Rintel wrote:
> This adds a driver for watchdog timer hardware present on Broadcom BCM2835 SoC,
> used in Raspberry Pi and Roku 2 devices.
> 
> Signed-off-by: Lubomir Rintel <lkundrak@v3.sk>
> Signed-off-by: Dom Cobley <popcornmix@gmail.com>

Those two s-o-b lines should be swapped, since if Dom did sign off on
any part of this patch, he did it before you did.

That said...

I wonder if it's actually appropriate to include Dom's s-o-b here, since
I don't think he really wrote this patch itself. I think you mentioned
that you hadn't use much of the downstream driver except for some defines?

To be clear, I mentioned the existence of the S-o-b line downstream
simply to demonstrate that the commits you were getting information from
had correctly followed the process described in
Documentation/SubmittingPatches, and so it was OK to use that
information while creating a GPL'd driver.

So there are a couple of ways that this patch could have been created:

1) You took the downstream commit itself, cherry-picked it into the
upstream kernel, modified it to suit upstream, and then submitted that.
The modifications might be extensive, such as renaming the file,
removing parts of the code that the upstream watchdog core now handles,
adding some new features, fixing bugs, cleanup, etc.; whatever is needed
to upstream the patch.

In this case, I believe it would be appropriate to maintain any S-o-b
lines from the original downstream commit, and add yours. But, I believe
you should also (a) maintain the git author field from the original
downstream commit (b) include a list of the changes you made to the
patch in the commit description, so you can be blamed for them rather
than the original author:-)

2) You read the downstream commit for information, but created a
completely new driver for the upstream kernel, using the downstream
driver just as a reference. In this case, I believe it's fine for the
git author field to be you, and for the only s-o-b line present to be
yours, since you really did write the patch from scratch. However, you
should credit the downstream work in the (c) header and/or commit
description.

This current patch sees to be a slight hybrid of both approaches (you're
listed as the git author, but have included Dom's s-o-b line on a patch
I don't think he created, and wasn't directly derived from one he created).

I'm not sure if I'm being too picky. I guess I'll leave it up to Wim Van
Sebroeck, since he's the watchdog maintainer and would be the person who
applies this patch.


^ permalink raw reply	[flat|nested] 17+ messages in thread

* Re: [PATCH v4] watchdog: Add Broadcom BCM2835 watchdog timer driver
  2013-03-28  3:00     ` Stephen Warren
@ 2013-03-28  3:50       ` Guenter Roeck
  2013-05-26 14:22       ` Wim Van Sebroeck
  1 sibling, 0 replies; 17+ messages in thread
From: Guenter Roeck @ 2013-03-28  3:50 UTC (permalink / raw)
  To: Stephen Warren
  Cc: Lubomir Rintel, linux-kernel, Dom Cobley, Wim Van Sebroeck,
	linux-rpi-kernel, linux-watchdog

On Wed, Mar 27, 2013 at 09:00:29PM -0600, Stephen Warren wrote:
> On 03/27/2013 10:40 AM, Lubomir Rintel wrote:
> > This adds a driver for watchdog timer hardware present on Broadcom BCM2835 SoC,
> > used in Raspberry Pi and Roku 2 devices.
> > 
> > Signed-off-by: Lubomir Rintel <lkundrak@v3.sk>
> > Signed-off-by: Dom Cobley <popcornmix@gmail.com>
> 
> Those two s-o-b lines should be swapped, since if Dom did sign off on
> any part of this patch, he did it before you did.
> 
> That said...
> 
> I wonder if it's actually appropriate to include Dom's s-o-b here, since
> I don't think he really wrote this patch itself. I think you mentioned
> that you hadn't use much of the downstream driver except for some defines?
> 
> To be clear, I mentioned the existence of the S-o-b line downstream
> simply to demonstrate that the commits you were getting information from
> had correctly followed the process described in
> Documentation/SubmittingPatches, and so it was OK to use that
> information while creating a GPL'd driver.
> 
> So there are a couple of ways that this patch could have been created:
> 
> 1) You took the downstream commit itself, cherry-picked it into the
> upstream kernel, modified it to suit upstream, and then submitted that.
> The modifications might be extensive, such as renaming the file,
> removing parts of the code that the upstream watchdog core now handles,
> adding some new features, fixing bugs, cleanup, etc.; whatever is needed
> to upstream the patch.
> 
> In this case, I believe it would be appropriate to maintain any S-o-b
> lines from the original downstream commit, and add yours. But, I believe
> you should also (a) maintain the git author field from the original
> downstream commit (b) include a list of the changes you made to the
> patch in the commit description, so you can be blamed for them rather
> than the original author:-)
> 
> 2) You read the downstream commit for information, but created a
> completely new driver for the upstream kernel, using the downstream
> driver just as a reference. In this case, I believe it's fine for the
> git author field to be you, and for the only s-o-b line present to be
> yours, since you really did write the patch from scratch. However, you
> should credit the downstream work in the (c) header and/or commit
> description.
> 
> This current patch sees to be a slight hybrid of both approaches (you're
> listed as the git author, but have included Dom's s-o-b line on a patch
> I don't think he created, and wasn't directly derived from one he created).
> 
> I'm not sure if I'm being too picky. I guess I'll leave it up to Wim Van
> Sebroeck, since he's the watchdog maintainer and would be the person who
> applies this patch.
> 
I wondered about the same.

I think 2) would be more appropriate. My approach would have been to reference
previous work in the file header, something along the line of "derived from
xxx", and add a copyright statement from the original work if there was one -
pretty much what you propose above.

Guenter

^ permalink raw reply	[flat|nested] 17+ messages in thread

* Re: [PATCH v4] watchdog: Add Broadcom BCM2835 watchdog timer driver
  2013-03-27 16:40   ` [PATCH v4] " Lubomir Rintel
  2013-03-27 19:52     ` Guenter Roeck
  2013-03-28  3:00     ` Stephen Warren
@ 2013-05-17  2:59     ` Stephen Warren
  2 siblings, 0 replies; 17+ messages in thread
From: Stephen Warren @ 2013-05-17  2:59 UTC (permalink / raw)
  To: Lubomir Rintel
  Cc: linux-kernel, Dom Cobley, Wim Van Sebroeck, Guenter Roeck,
	linux-rpi-kernel, linux-watchdog

On 03/27/2013 10:40 AM, Lubomir Rintel wrote:
> This adds a driver for watchdog timer hardware present on Broadcom BCM2835 SoC,
> used in Raspberry Pi and Roku 2 devices.

Lubomir, I don't believe this patch was applied for 3.10. Are you
planning to fix up any remaining issues and repost for 3.11?

^ permalink raw reply	[flat|nested] 17+ messages in thread

* Re: [PATCH v4] watchdog: Add Broadcom BCM2835 watchdog timer driver
  2013-03-28  3:00     ` Stephen Warren
  2013-03-28  3:50       ` Guenter Roeck
@ 2013-05-26 14:22       ` Wim Van Sebroeck
  2013-06-18 16:50         ` Lubomir Rintel
  1 sibling, 1 reply; 17+ messages in thread
From: Wim Van Sebroeck @ 2013-05-26 14:22 UTC (permalink / raw)
  To: Stephen Warren
  Cc: Lubomir Rintel, linux-kernel, Dom Cobley, Guenter Roeck,
	linux-rpi-kernel, linux-watchdog

Hi Lubomir,

> On 03/27/2013 10:40 AM, Lubomir Rintel wrote:
> > This adds a driver for watchdog timer hardware present on Broadcom BCM2835 SoC,
> > used in Raspberry Pi and Roku 2 devices.
> > 
> > Signed-off-by: Lubomir Rintel <lkundrak@v3.sk>
> > Signed-off-by: Dom Cobley <popcornmix@gmail.com>
> 
> Those two s-o-b lines should be swapped, since if Dom did sign off on
> any part of this patch, he did it before you did.
> 
> That said...
> 
> I wonder if it's actually appropriate to include Dom's s-o-b here, since
> I don't think he really wrote this patch itself. I think you mentioned
> that you hadn't use much of the downstream driver except for some defines?
> 
> To be clear, I mentioned the existence of the S-o-b line downstream
> simply to demonstrate that the commits you were getting information from
> had correctly followed the process described in
> Documentation/SubmittingPatches, and so it was OK to use that
> information while creating a GPL'd driver.
> 
> So there are a couple of ways that this patch could have been created:
> 
> 1) You took the downstream commit itself, cherry-picked it into the
> upstream kernel, modified it to suit upstream, and then submitted that.
> The modifications might be extensive, such as renaming the file,
> removing parts of the code that the upstream watchdog core now handles,
> adding some new features, fixing bugs, cleanup, etc.; whatever is needed
> to upstream the patch.
> 
> In this case, I believe it would be appropriate to maintain any S-o-b
> lines from the original downstream commit, and add yours. But, I believe
> you should also (a) maintain the git author field from the original
> downstream commit (b) include a list of the changes you made to the
> patch in the commit description, so you can be blamed for them rather
> than the original author:-)
> 
> 2) You read the downstream commit for information, but created a
> completely new driver for the upstream kernel, using the downstream
> driver just as a reference. In this case, I believe it's fine for the
> git author field to be you, and for the only s-o-b line present to be
> yours, since you really did write the patch from scratch. However, you
> should credit the downstream work in the (c) header and/or commit
> description.
> 
> This current patch sees to be a slight hybrid of both approaches (you're
> listed as the git author, but have included Dom's s-o-b line on a patch
> I don't think he created, and wasn't directly derived from one he created).
> 
> I'm not sure if I'm being too picky. I guess I'll leave it up to Wim Van
> Sebroeck, since he's the watchdog maintainer and would be the person who
> applies this patch.

Can you create a patch v5 with yourselve as author and add the necessary (c)
and references in it so that I can do the final review? (That is if situation
2 is indeed the case. If however it is situation 1 then we should get the
original code and have that as a patch and then add a second patch with your
modifications.).

Kind regards,
Wim.


^ permalink raw reply	[flat|nested] 17+ messages in thread

* Re: [PATCH v4] watchdog: Add Broadcom BCM2835 watchdog timer driver
  2013-05-26 14:22       ` Wim Van Sebroeck
@ 2013-06-18 16:50         ` Lubomir Rintel
  2013-06-18 17:10           ` Stephen Warren
  0 siblings, 1 reply; 17+ messages in thread
From: Lubomir Rintel @ 2013-06-18 16:50 UTC (permalink / raw)
  To: Wim Van Sebroeck
  Cc: Stephen Warren, linux-kernel, Dom Cobley, Guenter Roeck,
	linux-rpi-kernel, linux-watchdog

Hi Wim,

On Sun, 2013-05-26 at 16:22 +0200, Wim Van Sebroeck wrote:
> Hi Lubomir,
> 
> > On 03/27/2013 10:40 AM, Lubomir Rintel wrote:
> > > This adds a driver for watchdog timer hardware present on Broadcom BCM2835 SoC,
> > > used in Raspberry Pi and Roku 2 devices.
> > > 
> > > Signed-off-by: Lubomir Rintel <lkundrak@v3.sk>
> > > Signed-off-by: Dom Cobley <popcornmix@gmail.com>
> > 
> > Those two s-o-b lines should be swapped, since if Dom did sign off on
> > any part of this patch, he did it before you did.
> > 
> > That said...
> > 
> > I wonder if it's actually appropriate to include Dom's s-o-b here, since
> > I don't think he really wrote this patch itself. I think you mentioned
> > that you hadn't use much of the downstream driver except for some defines?
> > 
> > To be clear, I mentioned the existence of the S-o-b line downstream
> > simply to demonstrate that the commits you were getting information from
> > had correctly followed the process described in
> > Documentation/SubmittingPatches, and so it was OK to use that
> > information while creating a GPL'd driver.
> > 
> > So there are a couple of ways that this patch could have been created:
> > 
> > 1) You took the downstream commit itself, cherry-picked it into the
> > upstream kernel, modified it to suit upstream, and then submitted that.
> > The modifications might be extensive, such as renaming the file,
> > removing parts of the code that the upstream watchdog core now handles,
> > adding some new features, fixing bugs, cleanup, etc.; whatever is needed
> > to upstream the patch.
> > 
> > In this case, I believe it would be appropriate to maintain any S-o-b
> > lines from the original downstream commit, and add yours. But, I believe
> > you should also (a) maintain the git author field from the original
> > downstream commit (b) include a list of the changes you made to the
> > patch in the commit description, so you can be blamed for them rather
> > than the original author:-)
> > 
> > 2) You read the downstream commit for information, but created a
> > completely new driver for the upstream kernel, using the downstream
> > driver just as a reference. In this case, I believe it's fine for the
> > git author field to be you, and for the only s-o-b line present to be
> > yours, since you really did write the patch from scratch. However, you
> > should credit the downstream work in the (c) header and/or commit
> > description.
> > 
> > This current patch sees to be a slight hybrid of both approaches (you're
> > listed as the git author, but have included Dom's s-o-b line on a patch
> > I don't think he created, and wasn't directly derived from one he created).
> > 
> > I'm not sure if I'm being too picky. I guess I'll leave it up to Wim Van
> > Sebroeck, since he's the watchdog maintainer and would be the person who
> > applies this patch.
> 
> Can you create a patch v5 with yourselve as author and add the necessary (c)
> and references in it so that I can do the final review? (That is if situation
> 2 is indeed the case. If however it is situation 1 then we should get the
> original code and have that as a patch and then add a second patch with your
> modifications.).

I'm still not sure what to do. For most part, the driver is written from
scratch, since the original one was not utilizing recent enough
frameworks (such as watchdog-core or device tree bindings). Thus the
patch against the original driver would not make any sense at all.

On the other hand, I've referred to the original driver for hardware
information and copied a couple of defines as they are (such as
SECS_TO_WDOG_TICKS), thus I figured out it's a required to include the
original driver's sign-off and copyright information.

Thus neither 1) nor 2) is the case, and I can't figure out myself what
needs to be changed at this point. I'm wondering if you could help me
decide?

Thank you,
-- 
Lubomir Rintel <lkundrak@v3.sk>


^ permalink raw reply	[flat|nested] 17+ messages in thread

* Re: [PATCH v4] watchdog: Add Broadcom BCM2835 watchdog timer driver
  2013-06-18 16:50         ` Lubomir Rintel
@ 2013-06-18 17:10           ` Stephen Warren
  2013-06-18 17:44             ` [PATCH v5] " Lubomir Rintel
  0 siblings, 1 reply; 17+ messages in thread
From: Stephen Warren @ 2013-06-18 17:10 UTC (permalink / raw)
  To: Lubomir Rintel
  Cc: Wim Van Sebroeck, linux-kernel, Dom Cobley, Guenter Roeck,
	linux-rpi-kernel, linux-watchdog

On 06/18/2013 10:50 AM, Lubomir Rintel wrote:
> Hi Wim,
> 
> On Sun, 2013-05-26 at 16:22 +0200, Wim Van Sebroeck wrote:
>> Hi Lubomir,
>>
>>> On 03/27/2013 10:40 AM, Lubomir Rintel wrote:
>>>> This adds a driver for watchdog timer hardware present on Broadcom BCM2835 SoC,
>>>> used in Raspberry Pi and Roku 2 devices.
>>>>
>>>> Signed-off-by: Lubomir Rintel <lkundrak@v3.sk>
>>>> Signed-off-by: Dom Cobley <popcornmix@gmail.com>
>>>
>>> Those two s-o-b lines should be swapped, since if Dom did sign off on
>>> any part of this patch, he did it before you did.
>>>
>>> That said...
>>>
>>> I wonder if it's actually appropriate to include Dom's s-o-b here, since
>>> I don't think he really wrote this patch itself. I think you mentioned
>>> that you hadn't use much of the downstream driver except for some defines?
>>>
>>> To be clear, I mentioned the existence of the S-o-b line downstream
>>> simply to demonstrate that the commits you were getting information from
>>> had correctly followed the process described in
>>> Documentation/SubmittingPatches, and so it was OK to use that
>>> information while creating a GPL'd driver.
>>>
>>> So there are a couple of ways that this patch could have been created:
>>>
>>> 1) You took the downstream commit itself, cherry-picked it into the
>>> upstream kernel, modified it to suit upstream, and then submitted that.
>>> The modifications might be extensive, such as renaming the file,
>>> removing parts of the code that the upstream watchdog core now handles,
>>> adding some new features, fixing bugs, cleanup, etc.; whatever is needed
>>> to upstream the patch.
>>>
>>> In this case, I believe it would be appropriate to maintain any S-o-b
>>> lines from the original downstream commit, and add yours. But, I believe
>>> you should also (a) maintain the git author field from the original
>>> downstream commit (b) include a list of the changes you made to the
>>> patch in the commit description, so you can be blamed for them rather
>>> than the original author:-)
>>>
>>> 2) You read the downstream commit for information, but created a
>>> completely new driver for the upstream kernel, using the downstream
>>> driver just as a reference. In this case, I believe it's fine for the
>>> git author field to be you, and for the only s-o-b line present to be
>>> yours, since you really did write the patch from scratch. However, you
>>> should credit the downstream work in the (c) header and/or commit
>>> description.
>>>
>>> This current patch sees to be a slight hybrid of both approaches (you're
>>> listed as the git author, but have included Dom's s-o-b line on a patch
>>> I don't think he created, and wasn't directly derived from one he created).
>>>
>>> I'm not sure if I'm being too picky. I guess I'll leave it up to Wim Van
>>> Sebroeck, since he's the watchdog maintainer and would be the person who
>>> applies this patch.
>>
>> Can you create a patch v5 with yourselve as author and add the necessary (c)
>> and references in it so that I can do the final review? (That is if situation
>> 2 is indeed the case. If however it is situation 1 then we should get the
>> original code and have that as a patch and then add a second patch with your
>> modifications.).
> 
> I'm still not sure what to do. For most part, the driver is written from
> scratch, since the original one was not utilizing recent enough
> frameworks (such as watchdog-core or device tree bindings). Thus the
> patch against the original driver would not make any sense at all.
> 
> On the other hand, I've referred to the original driver for hardware
> information and copied a couple of defines as they are (such as
> SECS_TO_WDOG_TICKS), thus I figured out it's a required to include the
> original driver's sign-off and copyright information.
> 
> Thus neither 1) nor 2) is the case, and I can't figure out myself what
> needs to be changed at this point. I'm wondering if you could help me
> decide?

This sounds exactly like case (2) to me; copying a few defines for HW
constants doesn't really sound like having derived the driver from the
original commit.

^ permalink raw reply	[flat|nested] 17+ messages in thread

* [PATCH v5] watchdog: Add Broadcom BCM2835 watchdog timer driver
  2013-06-18 17:10           ` Stephen Warren
@ 2013-06-18 17:44             ` Lubomir Rintel
  2013-06-18 18:24               ` Guenter Roeck
  2013-06-27 20:25               ` Wim Van Sebroeck
  0 siblings, 2 replies; 17+ messages in thread
From: Lubomir Rintel @ 2013-06-18 17:44 UTC (permalink / raw)
  To: linux-kernel
  Cc: Wim Van Sebroeck, Lubomir Rintel, Stephen Warren, Guenter Roeck,
	linux-rpi-kernel, linux-watchdog, devicetree-discuss

This adds a driver for watchdog timer hardware present on Broadcom BCM2835 SoC,
used in Raspberry Pi and Roku 2 devices.

Signed-off-by: Lubomir Rintel <lkundrak@v3.sk>
Tested-by: Stephen Warren <swarren@wwwdotorg.org>
Cc: Stephen Warren <swarren@wwwdotorg.org>
Cc: Wim Van Sebroeck <wim@iguana.be>
Cc: Guenter Roeck <linux@roeck-us.net>
Cc: linux-rpi-kernel@lists.infradead.org
Cc: linux-watchdog@vger.kernel.org
Cc: devicetree-discuss@lists.ozlabs.org
---
Changes for v2:
    - Use per-device structure instead of global variables for lock and memory base
    - Fix default timeout setting
    - Do not leak memory if probe fails
    - Style fixes
    - Split out defconfig into a separate commit
    - Meaningful commit message with credit

Changes for v3:
    - Add proper copyright notice to header
    - Drop status constants, we don't use them
    - Fix heartbeat parameter's type
    - Log driver initialization message once the device is probed

Changes for v4:
    - Drop an useless initializer
    - Add a signoff from downstream
    - Do not announce the driver to the log if we failed to probe
    - Set platform data earlier and do not explicitly unset it

Changes for v5:
    - Copyright clarification

 .../bindings/watchdog/brcm,bcm2835-pm-wdog.txt     |    5 +
 drivers/watchdog/Kconfig                           |   11 ++
 drivers/watchdog/Makefile                          |    1 +
 drivers/watchdog/bcm2835_wdt.c                     |  189 ++++++++++++++++++++
 4 files changed, 206 insertions(+), 0 deletions(-)
 create mode 100644 drivers/watchdog/bcm2835_wdt.c

diff --git a/Documentation/devicetree/bindings/watchdog/brcm,bcm2835-pm-wdog.txt b/Documentation/devicetree/bindings/watchdog/brcm,bcm2835-pm-wdog.txt
index d209366..f801d71 100644
--- a/Documentation/devicetree/bindings/watchdog/brcm,bcm2835-pm-wdog.txt
+++ b/Documentation/devicetree/bindings/watchdog/brcm,bcm2835-pm-wdog.txt
@@ -5,9 +5,14 @@ Required properties:
 - compatible : should be "brcm,bcm2835-pm-wdt"
 - reg : Specifies base physical address and size of the registers.
 
+Optional properties:
+
+- timeout-sec   : Contains the watchdog timeout in seconds
+
 Example:
 
 watchdog {
 	compatible = "brcm,bcm2835-pm-wdt";
 	reg = <0x7e100000 0x28>;
+	timeout-sec = <10>;
 };
diff --git a/drivers/watchdog/Kconfig b/drivers/watchdog/Kconfig
index e89fc31..c3d3b16 100644
--- a/drivers/watchdog/Kconfig
+++ b/drivers/watchdog/Kconfig
@@ -1098,6 +1098,17 @@ config BCM63XX_WDT
 	  To compile this driver as a loadable module, choose M here.
 	  The module will be called bcm63xx_wdt.
 
+config BCM2835_WDT
+	tristate "Broadcom BCM2835 hardware watchdog"
+	depends on ARCH_BCM2835
+	select WATCHDOG_CORE
+	help
+	  Watchdog driver for the built in watchdog hardware in Broadcom
+	  BCM2835 SoC.
+
+	  To compile this driver as a loadable module, choose M here.
+	  The module will be called bcm2835_wdt.
+
 config LANTIQ_WDT
 	tristate "Lantiq SoC watchdog"
 	depends on LANTIQ
diff --git a/drivers/watchdog/Makefile b/drivers/watchdog/Makefile
index a300b94..1bf5675 100644
--- a/drivers/watchdog/Makefile
+++ b/drivers/watchdog/Makefile
@@ -54,6 +54,7 @@ obj-$(CONFIG_TS72XX_WATCHDOG) += ts72xx_wdt.o
 obj-$(CONFIG_IMX2_WDT) += imx2_wdt.o
 obj-$(CONFIG_UX500_WATCHDOG) += ux500_wdt.o
 obj-$(CONFIG_RETU_WATCHDOG) += retu_wdt.o
+obj-$(CONFIG_BCM2835_WDT) += bcm2835_wdt.o
 
 # AVR32 Architecture
 obj-$(CONFIG_AT32AP700X_WDT) += at32ap700x_wdt.o
diff --git a/drivers/watchdog/bcm2835_wdt.c b/drivers/watchdog/bcm2835_wdt.c
new file mode 100644
index 0000000..61566fc
--- /dev/null
+++ b/drivers/watchdog/bcm2835_wdt.c
@@ -0,0 +1,189 @@
+/*
+ * Watchdog driver for Broadcom BCM2835
+ *
+ * "bcm2708_wdog" driver written by Luke Diamand that was obtained from
+ * branch "rpi-3.6.y" of git://github.com/raspberrypi/linux.git was used
+ * as a hardware reference for the Broadcom BCM2835 watchdog timer.
+ *
+ * Copyright (C) 2013 Lubomir Rintel <lkundrak@v3.sk>
+ *
+ * This program is free software; you can redistribute it and/or modify it
+ * under the terms of the GNU General Public License as published by the
+ * Free Software Foundation; either version 2 of the License, or (at your
+ * option) any later version.
+ */
+
+#include <linux/types.h>
+#include <linux/module.h>
+#include <linux/io.h>
+#include <linux/watchdog.h>
+#include <linux/platform_device.h>
+#include <linux/of_address.h>
+#include <linux/miscdevice.h>
+
+#define PM_RSTC				0x1c
+#define PM_WDOG				0x24
+
+#define PM_PASSWORD			0x5a000000
+
+#define PM_WDOG_TIME_SET		0x000fffff
+#define PM_RSTC_WRCFG_CLR		0xffffffcf
+#define PM_RSTC_WRCFG_SET		0x00000030
+#define PM_RSTC_WRCFG_FULL_RESET	0x00000020
+#define PM_RSTC_RESET			0x00000102
+
+#define SECS_TO_WDOG_TICKS(x) ((x) << 16)
+#define WDOG_TICKS_TO_SECS(x) ((x) >> 16)
+
+struct bcm2835_wdt {
+	void __iomem		*base;
+	spinlock_t		lock;
+};
+
+static unsigned int heartbeat;
+static bool nowayout = WATCHDOG_NOWAYOUT;
+
+static int bcm2835_wdt_start(struct watchdog_device *wdog)
+{
+	struct bcm2835_wdt *wdt = watchdog_get_drvdata(wdog);
+	uint32_t cur;
+	unsigned long flags;
+
+	spin_lock_irqsave(&wdt->lock, flags);
+
+	writel_relaxed(PM_PASSWORD | (SECS_TO_WDOG_TICKS(wdog->timeout) &
+				PM_WDOG_TIME_SET), wdt->base + PM_WDOG);
+	cur = readl_relaxed(wdt->base + PM_RSTC);
+	writel_relaxed(PM_PASSWORD | (cur & PM_RSTC_WRCFG_CLR) |
+		  PM_RSTC_WRCFG_FULL_RESET, wdt->base + PM_RSTC);
+
+	spin_unlock_irqrestore(&wdt->lock, flags);
+
+	return 0;
+}
+
+static int bcm2835_wdt_stop(struct watchdog_device *wdog)
+{
+	struct bcm2835_wdt *wdt = watchdog_get_drvdata(wdog);
+
+	writel_relaxed(PM_PASSWORD | PM_RSTC_RESET, wdt->base + PM_RSTC);
+	dev_info(wdog->dev, "Watchdog timer stopped");
+	return 0;
+}
+
+static int bcm2835_wdt_set_timeout(struct watchdog_device *wdog, unsigned int t)
+{
+	wdog->timeout = t;
+	return 0;
+}
+
+static unsigned int bcm2835_wdt_get_timeleft(struct watchdog_device *wdog)
+{
+	struct bcm2835_wdt *wdt = watchdog_get_drvdata(wdog);
+
+	uint32_t ret = readl_relaxed(wdt->base + PM_WDOG);
+	return WDOG_TICKS_TO_SECS(ret & PM_WDOG_TIME_SET);
+}
+
+static struct watchdog_ops bcm2835_wdt_ops = {
+	.owner =	THIS_MODULE,
+	.start =	bcm2835_wdt_start,
+	.stop =		bcm2835_wdt_stop,
+	.set_timeout =	bcm2835_wdt_set_timeout,
+	.get_timeleft =	bcm2835_wdt_get_timeleft,
+};
+
+static struct watchdog_info bcm2835_wdt_info = {
+	.options =	WDIOF_SETTIMEOUT | WDIOF_MAGICCLOSE |
+			WDIOF_KEEPALIVEPING,
+	.identity =	"Broadcom BCM2835 Watchdog timer",
+};
+
+static struct watchdog_device bcm2835_wdt_wdd = {
+	.info =		&bcm2835_wdt_info,
+	.ops =		&bcm2835_wdt_ops,
+	.min_timeout =	1,
+	.max_timeout =	WDOG_TICKS_TO_SECS(PM_WDOG_TIME_SET),
+	.timeout =	WDOG_TICKS_TO_SECS(PM_WDOG_TIME_SET),
+};
+
+static int bcm2835_wdt_probe(struct platform_device *pdev)
+{
+	struct device *dev = &pdev->dev;
+	struct device_node *np = dev->of_node;
+	struct bcm2835_wdt *wdt;
+	int err;
+
+	wdt = devm_kzalloc(dev, sizeof(struct bcm2835_wdt), GFP_KERNEL);
+	if (!wdt) {
+		dev_err(dev, "Failed to allocate memory for watchdog device");
+		return -ENOMEM;
+	}
+	platform_set_drvdata(pdev, wdt);
+
+	spin_lock_init(&wdt->lock);
+
+	wdt->base = of_iomap(np, 0);
+	if (!wdt->base) {
+		dev_err(dev, "Failed to remap watchdog regs");
+		return -ENODEV;
+	}
+
+	watchdog_set_drvdata(&bcm2835_wdt_wdd, wdt);
+	watchdog_init_timeout(&bcm2835_wdt_wdd, heartbeat, dev);
+	watchdog_set_nowayout(&bcm2835_wdt_wdd, nowayout);
+	err = watchdog_register_device(&bcm2835_wdt_wdd);
+	if (err) {
+		dev_err(dev, "Failed to register watchdog device");
+		iounmap(wdt->base);
+		return err;
+	}
+
+	dev_info(dev, "Broadcom BCM2835 watchdog timer");
+	return 0;
+}
+
+static int bcm2835_wdt_remove(struct platform_device *pdev)
+{
+	struct bcm2835_wdt *wdt = platform_get_drvdata(pdev);
+
+	watchdog_unregister_device(&bcm2835_wdt_wdd);
+	iounmap(wdt->base);
+
+	return 0;
+}
+
+static void bcm2835_wdt_shutdown(struct platform_device *pdev)
+{
+	bcm2835_wdt_stop(&bcm2835_wdt_wdd);
+}
+
+static const struct of_device_id bcm2835_wdt_of_match[] = {
+	{ .compatible = "brcm,bcm2835-pm-wdt", },
+	{},
+};
+MODULE_DEVICE_TABLE(of, bcm2835_wdt_of_match);
+
+static struct platform_driver bcm2835_wdt_driver = {
+	.probe		= bcm2835_wdt_probe,
+	.remove		= bcm2835_wdt_remove,
+	.shutdown	= bcm2835_wdt_shutdown,
+	.driver = {
+		.name =		"bcm2835-wdt",
+		.owner =	THIS_MODULE,
+		.of_match_table = bcm2835_wdt_of_match,
+	},
+};
+module_platform_driver(bcm2835_wdt_driver);
+
+module_param(heartbeat, uint, 0);
+MODULE_PARM_DESC(heartbeat, "Initial watchdog heartbeat in seconds");
+
+module_param(nowayout, bool, 0);
+MODULE_PARM_DESC(nowayout, "Watchdog cannot be stopped once started (default="
+				__MODULE_STRING(WATCHDOG_NOWAYOUT) ")");
+
+MODULE_AUTHOR("Lubomir Rintel <lkundrak@v3.sk>");
+MODULE_DESCRIPTION("Driver for Broadcom BCM2835 watchdog timer");
+MODULE_LICENSE("GPL");
+MODULE_ALIAS_MISCDEV(WATCHDOG_MINOR);
-- 
1.7.1

^ permalink raw reply related	[flat|nested] 17+ messages in thread

* Re: [PATCH v5] watchdog: Add Broadcom BCM2835 watchdog timer driver
  2013-06-18 17:44             ` [PATCH v5] " Lubomir Rintel
@ 2013-06-18 18:24               ` Guenter Roeck
  2013-06-27 20:25               ` Wim Van Sebroeck
  1 sibling, 0 replies; 17+ messages in thread
From: Guenter Roeck @ 2013-06-18 18:24 UTC (permalink / raw)
  To: Lubomir Rintel
  Cc: linux-kernel, Wim Van Sebroeck, Stephen Warren, linux-rpi-kernel,
	linux-watchdog, devicetree-discuss

On Tue, Jun 18, 2013 at 07:44:48PM +0200, Lubomir Rintel wrote:
> This adds a driver for watchdog timer hardware present on Broadcom BCM2835 SoC,
> used in Raspberry Pi and Roku 2 devices.
> 
> Signed-off-by: Lubomir Rintel <lkundrak@v3.sk>
> Tested-by: Stephen Warren <swarren@wwwdotorg.org>
> Cc: Stephen Warren <swarren@wwwdotorg.org>
> Cc: Wim Van Sebroeck <wim@iguana.be>
> Cc: Guenter Roeck <linux@roeck-us.net>
> Cc: linux-rpi-kernel@lists.infradead.org
> Cc: linux-watchdog@vger.kernel.org
> Cc: devicetree-discuss@lists.ozlabs.org

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

> ---
> Changes for v2:
>     - Use per-device structure instead of global variables for lock and memory base
>     - Fix default timeout setting
>     - Do not leak memory if probe fails
>     - Style fixes
>     - Split out defconfig into a separate commit
>     - Meaningful commit message with credit
> 
> Changes for v3:
>     - Add proper copyright notice to header
>     - Drop status constants, we don't use them
>     - Fix heartbeat parameter's type
>     - Log driver initialization message once the device is probed
> 
> Changes for v4:
>     - Drop an useless initializer
>     - Add a signoff from downstream
>     - Do not announce the driver to the log if we failed to probe
>     - Set platform data earlier and do not explicitly unset it
> 
> Changes for v5:
>     - Copyright clarification
> 
>  .../bindings/watchdog/brcm,bcm2835-pm-wdog.txt     |    5 +
>  drivers/watchdog/Kconfig                           |   11 ++
>  drivers/watchdog/Makefile                          |    1 +
>  drivers/watchdog/bcm2835_wdt.c                     |  189 ++++++++++++++++++++
>  4 files changed, 206 insertions(+), 0 deletions(-)
>  create mode 100644 drivers/watchdog/bcm2835_wdt.c
> 
> diff --git a/Documentation/devicetree/bindings/watchdog/brcm,bcm2835-pm-wdog.txt b/Documentation/devicetree/bindings/watchdog/brcm,bcm2835-pm-wdog.txt
> index d209366..f801d71 100644
> --- a/Documentation/devicetree/bindings/watchdog/brcm,bcm2835-pm-wdog.txt
> +++ b/Documentation/devicetree/bindings/watchdog/brcm,bcm2835-pm-wdog.txt
> @@ -5,9 +5,14 @@ Required properties:
>  - compatible : should be "brcm,bcm2835-pm-wdt"
>  - reg : Specifies base physical address and size of the registers.
>  
> +Optional properties:
> +
> +- timeout-sec   : Contains the watchdog timeout in seconds
> +
>  Example:
>  
>  watchdog {
>  	compatible = "brcm,bcm2835-pm-wdt";
>  	reg = <0x7e100000 0x28>;
> +	timeout-sec = <10>;
>  };
> diff --git a/drivers/watchdog/Kconfig b/drivers/watchdog/Kconfig
> index e89fc31..c3d3b16 100644
> --- a/drivers/watchdog/Kconfig
> +++ b/drivers/watchdog/Kconfig
> @@ -1098,6 +1098,17 @@ config BCM63XX_WDT
>  	  To compile this driver as a loadable module, choose M here.
>  	  The module will be called bcm63xx_wdt.
>  
> +config BCM2835_WDT
> +	tristate "Broadcom BCM2835 hardware watchdog"
> +	depends on ARCH_BCM2835
> +	select WATCHDOG_CORE
> +	help
> +	  Watchdog driver for the built in watchdog hardware in Broadcom
> +	  BCM2835 SoC.
> +
> +	  To compile this driver as a loadable module, choose M here.
> +	  The module will be called bcm2835_wdt.
> +
>  config LANTIQ_WDT
>  	tristate "Lantiq SoC watchdog"
>  	depends on LANTIQ
> diff --git a/drivers/watchdog/Makefile b/drivers/watchdog/Makefile
> index a300b94..1bf5675 100644
> --- a/drivers/watchdog/Makefile
> +++ b/drivers/watchdog/Makefile
> @@ -54,6 +54,7 @@ obj-$(CONFIG_TS72XX_WATCHDOG) += ts72xx_wdt.o
>  obj-$(CONFIG_IMX2_WDT) += imx2_wdt.o
>  obj-$(CONFIG_UX500_WATCHDOG) += ux500_wdt.o
>  obj-$(CONFIG_RETU_WATCHDOG) += retu_wdt.o
> +obj-$(CONFIG_BCM2835_WDT) += bcm2835_wdt.o
>  
>  # AVR32 Architecture
>  obj-$(CONFIG_AT32AP700X_WDT) += at32ap700x_wdt.o
> diff --git a/drivers/watchdog/bcm2835_wdt.c b/drivers/watchdog/bcm2835_wdt.c
> new file mode 100644
> index 0000000..61566fc
> --- /dev/null
> +++ b/drivers/watchdog/bcm2835_wdt.c
> @@ -0,0 +1,189 @@
> +/*
> + * Watchdog driver for Broadcom BCM2835
> + *
> + * "bcm2708_wdog" driver written by Luke Diamand that was obtained from
> + * branch "rpi-3.6.y" of git://github.com/raspberrypi/linux.git was used
> + * as a hardware reference for the Broadcom BCM2835 watchdog timer.
> + *
> + * Copyright (C) 2013 Lubomir Rintel <lkundrak@v3.sk>
> + *
> + * This program is free software; you can redistribute it and/or modify it
> + * under the terms of the GNU General Public License as published by the
> + * Free Software Foundation; either version 2 of the License, or (at your
> + * option) any later version.
> + */
> +
> +#include <linux/types.h>
> +#include <linux/module.h>
> +#include <linux/io.h>
> +#include <linux/watchdog.h>
> +#include <linux/platform_device.h>
> +#include <linux/of_address.h>
> +#include <linux/miscdevice.h>
> +
> +#define PM_RSTC				0x1c
> +#define PM_WDOG				0x24
> +
> +#define PM_PASSWORD			0x5a000000
> +
> +#define PM_WDOG_TIME_SET		0x000fffff
> +#define PM_RSTC_WRCFG_CLR		0xffffffcf
> +#define PM_RSTC_WRCFG_SET		0x00000030
> +#define PM_RSTC_WRCFG_FULL_RESET	0x00000020
> +#define PM_RSTC_RESET			0x00000102
> +
> +#define SECS_TO_WDOG_TICKS(x) ((x) << 16)
> +#define WDOG_TICKS_TO_SECS(x) ((x) >> 16)
> +
> +struct bcm2835_wdt {
> +	void __iomem		*base;
> +	spinlock_t		lock;
> +};
> +
> +static unsigned int heartbeat;
> +static bool nowayout = WATCHDOG_NOWAYOUT;
> +
> +static int bcm2835_wdt_start(struct watchdog_device *wdog)
> +{
> +	struct bcm2835_wdt *wdt = watchdog_get_drvdata(wdog);
> +	uint32_t cur;
> +	unsigned long flags;
> +
> +	spin_lock_irqsave(&wdt->lock, flags);
> +
> +	writel_relaxed(PM_PASSWORD | (SECS_TO_WDOG_TICKS(wdog->timeout) &
> +				PM_WDOG_TIME_SET), wdt->base + PM_WDOG);
> +	cur = readl_relaxed(wdt->base + PM_RSTC);
> +	writel_relaxed(PM_PASSWORD | (cur & PM_RSTC_WRCFG_CLR) |
> +		  PM_RSTC_WRCFG_FULL_RESET, wdt->base + PM_RSTC);
> +
> +	spin_unlock_irqrestore(&wdt->lock, flags);
> +
> +	return 0;
> +}
> +
> +static int bcm2835_wdt_stop(struct watchdog_device *wdog)
> +{
> +	struct bcm2835_wdt *wdt = watchdog_get_drvdata(wdog);
> +
> +	writel_relaxed(PM_PASSWORD | PM_RSTC_RESET, wdt->base + PM_RSTC);
> +	dev_info(wdog->dev, "Watchdog timer stopped");
> +	return 0;
> +}
> +
> +static int bcm2835_wdt_set_timeout(struct watchdog_device *wdog, unsigned int t)
> +{
> +	wdog->timeout = t;
> +	return 0;
> +}
> +
> +static unsigned int bcm2835_wdt_get_timeleft(struct watchdog_device *wdog)
> +{
> +	struct bcm2835_wdt *wdt = watchdog_get_drvdata(wdog);
> +
> +	uint32_t ret = readl_relaxed(wdt->base + PM_WDOG);
> +	return WDOG_TICKS_TO_SECS(ret & PM_WDOG_TIME_SET);
> +}
> +
> +static struct watchdog_ops bcm2835_wdt_ops = {
> +	.owner =	THIS_MODULE,
> +	.start =	bcm2835_wdt_start,
> +	.stop =		bcm2835_wdt_stop,
> +	.set_timeout =	bcm2835_wdt_set_timeout,
> +	.get_timeleft =	bcm2835_wdt_get_timeleft,
> +};
> +
> +static struct watchdog_info bcm2835_wdt_info = {
> +	.options =	WDIOF_SETTIMEOUT | WDIOF_MAGICCLOSE |
> +			WDIOF_KEEPALIVEPING,
> +	.identity =	"Broadcom BCM2835 Watchdog timer",
> +};
> +
> +static struct watchdog_device bcm2835_wdt_wdd = {
> +	.info =		&bcm2835_wdt_info,
> +	.ops =		&bcm2835_wdt_ops,
> +	.min_timeout =	1,
> +	.max_timeout =	WDOG_TICKS_TO_SECS(PM_WDOG_TIME_SET),
> +	.timeout =	WDOG_TICKS_TO_SECS(PM_WDOG_TIME_SET),
> +};
> +
> +static int bcm2835_wdt_probe(struct platform_device *pdev)
> +{
> +	struct device *dev = &pdev->dev;
> +	struct device_node *np = dev->of_node;
> +	struct bcm2835_wdt *wdt;
> +	int err;
> +
> +	wdt = devm_kzalloc(dev, sizeof(struct bcm2835_wdt), GFP_KERNEL);
> +	if (!wdt) {
> +		dev_err(dev, "Failed to allocate memory for watchdog device");
> +		return -ENOMEM;
> +	}
> +	platform_set_drvdata(pdev, wdt);
> +
> +	spin_lock_init(&wdt->lock);
> +
> +	wdt->base = of_iomap(np, 0);
> +	if (!wdt->base) {
> +		dev_err(dev, "Failed to remap watchdog regs");
> +		return -ENODEV;
> +	}
> +
> +	watchdog_set_drvdata(&bcm2835_wdt_wdd, wdt);
> +	watchdog_init_timeout(&bcm2835_wdt_wdd, heartbeat, dev);
> +	watchdog_set_nowayout(&bcm2835_wdt_wdd, nowayout);
> +	err = watchdog_register_device(&bcm2835_wdt_wdd);
> +	if (err) {
> +		dev_err(dev, "Failed to register watchdog device");
> +		iounmap(wdt->base);
> +		return err;
> +	}
> +
> +	dev_info(dev, "Broadcom BCM2835 watchdog timer");
> +	return 0;
> +}
> +
> +static int bcm2835_wdt_remove(struct platform_device *pdev)
> +{
> +	struct bcm2835_wdt *wdt = platform_get_drvdata(pdev);
> +
> +	watchdog_unregister_device(&bcm2835_wdt_wdd);
> +	iounmap(wdt->base);
> +
> +	return 0;
> +}
> +
> +static void bcm2835_wdt_shutdown(struct platform_device *pdev)
> +{
> +	bcm2835_wdt_stop(&bcm2835_wdt_wdd);
> +}
> +
> +static const struct of_device_id bcm2835_wdt_of_match[] = {
> +	{ .compatible = "brcm,bcm2835-pm-wdt", },
> +	{},
> +};
> +MODULE_DEVICE_TABLE(of, bcm2835_wdt_of_match);
> +
> +static struct platform_driver bcm2835_wdt_driver = {
> +	.probe		= bcm2835_wdt_probe,
> +	.remove		= bcm2835_wdt_remove,
> +	.shutdown	= bcm2835_wdt_shutdown,
> +	.driver = {
> +		.name =		"bcm2835-wdt",
> +		.owner =	THIS_MODULE,
> +		.of_match_table = bcm2835_wdt_of_match,
> +	},
> +};
> +module_platform_driver(bcm2835_wdt_driver);
> +
> +module_param(heartbeat, uint, 0);
> +MODULE_PARM_DESC(heartbeat, "Initial watchdog heartbeat in seconds");
> +
> +module_param(nowayout, bool, 0);
> +MODULE_PARM_DESC(nowayout, "Watchdog cannot be stopped once started (default="
> +				__MODULE_STRING(WATCHDOG_NOWAYOUT) ")");
> +
> +MODULE_AUTHOR("Lubomir Rintel <lkundrak@v3.sk>");
> +MODULE_DESCRIPTION("Driver for Broadcom BCM2835 watchdog timer");
> +MODULE_LICENSE("GPL");
> +MODULE_ALIAS_MISCDEV(WATCHDOG_MINOR);
> -- 
> 1.7.1
> 
> 

^ permalink raw reply	[flat|nested] 17+ messages in thread

* Re: [PATCH v5] watchdog: Add Broadcom BCM2835 watchdog timer driver
  2013-06-18 17:44             ` [PATCH v5] " Lubomir Rintel
  2013-06-18 18:24               ` Guenter Roeck
@ 2013-06-27 20:25               ` Wim Van Sebroeck
  1 sibling, 0 replies; 17+ messages in thread
From: Wim Van Sebroeck @ 2013-06-27 20:25 UTC (permalink / raw)
  To: Lubomir Rintel
  Cc: linux-kernel, Stephen Warren, Guenter Roeck, linux-rpi-kernel,
	linux-watchdog, devicetree-discuss

Hi Lubomir,

> This adds a driver for watchdog timer hardware present on Broadcom BCM2835 SoC,
> used in Raspberry Pi and Roku 2 devices.
> 
> Signed-off-by: Lubomir Rintel <lkundrak@v3.sk>
> Tested-by: Stephen Warren <swarren@wwwdotorg.org>
> Cc: Stephen Warren <swarren@wwwdotorg.org>
> Cc: Wim Van Sebroeck <wim@iguana.be>
> Cc: Guenter Roeck <linux@roeck-us.net>
> Cc: linux-rpi-kernel@lists.infradead.org
> Cc: linux-watchdog@vger.kernel.org
> Cc: devicetree-discuss@lists.ozlabs.org

Added to linux-watchdog-next.

Kind regards,
Wim.

^ permalink raw reply	[flat|nested] 17+ messages in thread

end of thread, other threads:[~2013-06-27 20:25 UTC | newest]

Thread overview: 17+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
     [not found] <1364135137-26691-1-git-send-email-lkundrak@v3.sk>
2013-03-26 17:50 ` [PATCH v3] watchdog: Add Broadcom BCM2835 watchdog timer driver Lubomir Rintel
2013-03-26 21:03   ` Guenter Roeck
2013-03-27 16:39     ` Lubomir Rintel
2013-03-27  3:40   ` Stephen Warren
2013-03-27 16:36     ` Lubomir Rintel
2013-03-27  3:49   ` Stephen Warren
2013-03-27 16:40   ` [PATCH v4] " Lubomir Rintel
2013-03-27 19:52     ` Guenter Roeck
2013-03-28  3:00     ` Stephen Warren
2013-03-28  3:50       ` Guenter Roeck
2013-05-26 14:22       ` Wim Van Sebroeck
2013-06-18 16:50         ` Lubomir Rintel
2013-06-18 17:10           ` Stephen Warren
2013-06-18 17:44             ` [PATCH v5] " Lubomir Rintel
2013-06-18 18:24               ` Guenter Roeck
2013-06-27 20:25               ` Wim Van Sebroeck
2013-05-17  2:59     ` [PATCH v4] " Stephen Warren

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).