Re: [PATCH v2] watchdog: Add Broadcom BCM2835 watchdog timer driver

2013-03-26 Thread Guenter Roeck
On Tue, Mar 26, 2013 at 06:47:55PM +0100, Lubomir Rintel wrote:
> On Sun, 2013-03-24 at 09:12 -0700, Guenter Roeck wrote:
> > On Sun, Mar 24, 2013 at 03:22:53PM +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.
> > > 
> > > 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 would suggest to put that into the coments at the top of the driver.
> > Also, if the original code has a copyright statement, you might want
> > to copy that.
> 
> Okay.
> 
> > > +#include 
> > 
> > Just noticed - you should not need that include.
> 
> Well, I think the bottommost line of the driver uses that and it's a
> good practice to provide a device number alias. Is that one completely
> useless?
> 
>   MODULE_ALIAS_MISCDEV(WATCHDOG_MINOR);
> 
Ah, I didn't realize this is still used. The documentation suggests that it can 
be
kept when converting drivers to the watchdog core, so I guess there must be a 
reason.

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


Re: [PATCH v2] watchdog: Add Broadcom BCM2835 watchdog timer driver

2013-03-26 Thread Lubomir Rintel
On Sun, 2013-03-24 at 09:12 -0700, Guenter Roeck wrote:
> On Sun, Mar 24, 2013 at 03:22:53PM +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.
> > 
> > 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 would suggest to put that into the coments at the top of the driver.
> Also, if the original code has a copyright statement, you might want
> to copy that.

Okay.

> > +#include 
> 
> Just noticed - you should not need that include.

Well, I think the bottommost line of the driver uses that and it's a
good practice to provide a device number alias. Is that one completely
useless?

  MODULE_ALIAS_MISCDEV(WATCHDOG_MINOR);

> > +#define PM_RSTS0x20
> 
> Not used

Dropped.

> > +#define PM_RSTC_WRCFG_MASK 0x0030
> 
> Not used

Dropped.

> > +#define PM_RSTS_HADWRH_SET 0x0040
> 
> Not used

Dropped.

> > +#define PM_RSTC_WRCFG_SET  0x0030
> 
> Not used 

Dropped.

> > +#define PM_RSTC_WRCFG_FULL_RESET   0x0020
> 
> Defined twice

Extra occurrence dropped.

> 
> > +#define PM_RSTC_RESET  0x0102
> > +
> > +#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 int heartbeat = -1;
> 
> The parameter passed to watchdog_init_timeout is an unsigned int, so you 
> should
> really use an unsigned int here. Depending on -1 being converted to maxuint 
> is a
> bit of side effect programming. Easier to just set it to 0, which the watchdog
> core interprets as invalid as well.

Done.

> > +   dev_info(dev, "Broadcom BCM2835 watchdog timer");
> > +
> I think this should come later, after you are sure that there will be
> no failure.

Moved.

> > +   platform_set_drvdata(pdev, NULL);
> 
> It is unnecessary to set this to NULL (the infrastructure does it for you).

Okay.

I've seen this being done on way too many places. I've really should
have checked that though.

Thank you for your response, I'll submit an updated revision shortly.
-- 
Lubomir Rintel 

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


Re: [PATCH v2] watchdog: Add Broadcom BCM2835 watchdog timer driver

2013-03-26 Thread Lubomir Rintel
On Sun, 2013-03-24 at 09:12 -0700, Guenter Roeck wrote:
 On Sun, Mar 24, 2013 at 03:22:53PM +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.
  
  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 would suggest to put that into the coments at the top of the driver.
 Also, if the original code has a copyright statement, you might want
 to copy that.

Okay.

  +#include linux/miscdevice.h
 
 Just noticed - you should not need that include.

Well, I think the bottommost line of the driver uses that and it's a
good practice to provide a device number alias. Is that one completely
useless?

  MODULE_ALIAS_MISCDEV(WATCHDOG_MINOR);

  +#define PM_RSTS0x20
 
 Not used

Dropped.

  +#define PM_RSTC_WRCFG_MASK 0x0030
 
 Not used

Dropped.

  +#define PM_RSTS_HADWRH_SET 0x0040
 
 Not used

Dropped.

  +#define PM_RSTC_WRCFG_SET  0x0030
 
 Not used 

Dropped.

  +#define PM_RSTC_WRCFG_FULL_RESET   0x0020
 
 Defined twice

Extra occurrence dropped.

 
  +#define PM_RSTC_RESET  0x0102
  +
  +#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 int heartbeat = -1;
 
 The parameter passed to watchdog_init_timeout is an unsigned int, so you 
 should
 really use an unsigned int here. Depending on -1 being converted to maxuint 
 is a
 bit of side effect programming. Easier to just set it to 0, which the watchdog
 core interprets as invalid as well.

Done.

  +   dev_info(dev, Broadcom BCM2835 watchdog timer);
  +
 I think this should come later, after you are sure that there will be
 no failure.

Moved.

  +   platform_set_drvdata(pdev, NULL);
 
 It is unnecessary to set this to NULL (the infrastructure does it for you).

Okay.

I've seen this being done on way too many places. I've really should
have checked that though.

Thank you for your response, I'll submit an updated revision shortly.
-- 
Lubomir Rintel lkund...@v3.sk

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


Re: [PATCH v2] watchdog: Add Broadcom BCM2835 watchdog timer driver

2013-03-26 Thread Guenter Roeck
On Tue, Mar 26, 2013 at 06:47:55PM +0100, Lubomir Rintel wrote:
 On Sun, 2013-03-24 at 09:12 -0700, Guenter Roeck wrote:
  On Sun, Mar 24, 2013 at 03:22:53PM +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.
   
   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 would suggest to put that into the coments at the top of the driver.
  Also, if the original code has a copyright statement, you might want
  to copy that.
 
 Okay.
 
   +#include linux/miscdevice.h
  
  Just noticed - you should not need that include.
 
 Well, I think the bottommost line of the driver uses that and it's a
 good practice to provide a device number alias. Is that one completely
 useless?
 
   MODULE_ALIAS_MISCDEV(WATCHDOG_MINOR);
 
Ah, I didn't realize this is still used. The documentation suggests that it can 
be
kept when converting drivers to the watchdog core, so I guess there must be a 
reason.

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


Re: [PATCH v2] watchdog: Add Broadcom BCM2835 watchdog timer driver

2013-03-24 Thread Guenter Roeck
On Sun, Mar 24, 2013 at 03:22:53PM +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.
> 
> 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 would suggest to put that into the coments at the top of the driver.
Also, if the original code has a copyright statement, you might want
to copy that.

> Signed-off-by: Lubomir Rintel 
> Cc: Stephen Warren 
> Cc: Wim Van Sebroeck 
> Cc: linux-rpi-ker...@lists.infradead.org
> Cc: linux-watch...@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
> 
>  .../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 = <0x7e10 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 000..3899638
> --- /dev/null
> +++ b/drivers/watchdog/bcm2835_wdt.c
> @@ -0,0 +1,189 @@
> +/*
> + *  Watchdog driver for Broadcom BCM2835
> + *
> + *  Copyright (C) 2013 Lubomir Rintel 
> + *
> + *  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 
> +#include 
> +#include 
> +#include 
> +#include 
> +#include 
> +#include 

Just noticed - you should not need that include.

> +
> +#define PM_RSTC  0x1c
> +#define PM_RSTS  0x20

Not used

> +#define PM_WDOG  0x24
> +
> +#define PM_PASSWORD  0x5a00
> +#define PM_RSTC_WRCFG_MASK   0x0030

Not used

> +#define PM_RSTC_WRCFG_FULL_RESET 0x0020
> +#define PM_RSTS_HADWRH_SET   0x0040

Not used

> +
> +#define PM_WDOG_TIME_SET 0x000f
> +#define PM_RSTC_WRCFG_CLR0xffcf
> +#define PM_RSTC_WRCFG_SET0x0030

Not used 

> +#define PM_RSTC_WRCFG_FULL_RESET 0x0020

Defined twice

> +#define PM_RSTC_RESET0x0102
> +
> +#define SECS_TO_WDOG_TICKS(x) ((x) << 16)
> +#define WDOG_TICKS_TO_SECS(x) ((x) >> 16)
> +
> +struct bcm2835_wdt {
> + 

[PATCH v2] watchdog: Add Broadcom BCM2835 watchdog timer driver

2013-03-24 Thread Lubomir Rintel
This adds a driver for watchdog timer hardware present on Broadcom BCM2835 SoC,
used in Raspberry Pi and Roku 2 devices.

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

Signed-off-by: Lubomir Rintel 
Cc: Stephen Warren 
Cc: Wim Van Sebroeck 
Cc: linux-rpi-ker...@lists.infradead.org
Cc: linux-watch...@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

 .../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 = <0x7e10 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 000..3899638
--- /dev/null
+++ b/drivers/watchdog/bcm2835_wdt.c
@@ -0,0 +1,189 @@
+/*
+ *  Watchdog driver for Broadcom BCM2835
+ *
+ *  Copyright (C) 2013 Lubomir Rintel 
+ *
+ *  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 
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+
+#define PM_RSTC0x1c
+#define PM_RSTS0x20
+#define PM_WDOG0x24
+
+#define PM_PASSWORD0x5a00
+#define PM_RSTC_WRCFG_MASK 0x0030
+#define PM_RSTC_WRCFG_FULL_RESET   0x0020
+#define PM_RSTS_HADWRH_SET 0x0040
+
+#define PM_WDOG_TIME_SET   0x000f
+#define PM_RSTC_WRCFG_CLR  0xffcf
+#define PM_RSTC_WRCFG_SET  0x0030
+#define PM_RSTC_WRCFG_FULL_RESET   0x0020
+#define PM_RSTC_RESET  0x0102
+
+#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 int heartbeat = -1;
+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(>lock, flags);
+
+   writel_relaxed(PM_PASSWORD | (SECS_TO_WDOG_TICKS(wdog->timeout) &
+   PM_WDOG_TIME_SET), wdt->base + PM_WDOG);
+   cur = 

[PATCH v2] watchdog: Add Broadcom BCM2835 watchdog timer driver

2013-03-24 Thread Lubomir Rintel
This adds a driver for watchdog timer hardware present on Broadcom BCM2835 SoC,
used in Raspberry Pi and Roku 2 devices.

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

Signed-off-by: Lubomir Rintel lkund...@v3.sk
Cc: Stephen Warren swar...@wwwdotorg.org
Cc: Wim Van Sebroeck w...@iguana.be
Cc: linux-rpi-ker...@lists.infradead.org
Cc: linux-watch...@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

 .../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 = 0x7e10 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 000..3899638
--- /dev/null
+++ b/drivers/watchdog/bcm2835_wdt.c
@@ -0,0 +1,189 @@
+/*
+ *  Watchdog driver for Broadcom BCM2835
+ *
+ *  Copyright (C) 2013 Lubomir Rintel lkund...@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_RSTC0x1c
+#define PM_RSTS0x20
+#define PM_WDOG0x24
+
+#define PM_PASSWORD0x5a00
+#define PM_RSTC_WRCFG_MASK 0x0030
+#define PM_RSTC_WRCFG_FULL_RESET   0x0020
+#define PM_RSTS_HADWRH_SET 0x0040
+
+#define PM_WDOG_TIME_SET   0x000f
+#define PM_RSTC_WRCFG_CLR  0xffcf
+#define PM_RSTC_WRCFG_SET  0x0030
+#define PM_RSTC_WRCFG_FULL_RESET   0x0020
+#define PM_RSTC_RESET  0x0102
+
+#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 int heartbeat = -1;
+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);
+
+   

Re: [PATCH v2] watchdog: Add Broadcom BCM2835 watchdog timer driver

2013-03-24 Thread Guenter Roeck
On Sun, Mar 24, 2013 at 03:22:53PM +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.
 
 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 would suggest to put that into the coments at the top of the driver.
Also, if the original code has a copyright statement, you might want
to copy that.

 Signed-off-by: Lubomir Rintel lkund...@v3.sk
 Cc: Stephen Warren swar...@wwwdotorg.org
 Cc: Wim Van Sebroeck w...@iguana.be
 Cc: linux-rpi-ker...@lists.infradead.org
 Cc: linux-watch...@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
 
  .../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 = 0x7e10 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 000..3899638
 --- /dev/null
 +++ b/drivers/watchdog/bcm2835_wdt.c
 @@ -0,0 +1,189 @@
 +/*
 + *  Watchdog driver for Broadcom BCM2835
 + *
 + *  Copyright (C) 2013 Lubomir Rintel lkund...@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

Just noticed - you should not need that include.

 +
 +#define PM_RSTC  0x1c
 +#define PM_RSTS  0x20

Not used

 +#define PM_WDOG  0x24
 +
 +#define PM_PASSWORD  0x5a00
 +#define PM_RSTC_WRCFG_MASK   0x0030

Not used

 +#define PM_RSTC_WRCFG_FULL_RESET 0x0020
 +#define PM_RSTS_HADWRH_SET   0x0040

Not used

 +
 +#define PM_WDOG_TIME_SET 0x000f
 +#define PM_RSTC_WRCFG_CLR0xffcf
 +#define PM_RSTC_WRCFG_SET0x0030

Not used 

 +#define PM_RSTC_WRCFG_FULL_RESET 0x0020

Defined twice

 +#define PM_RSTC_RESET0x0102
 +
 +#define SECS_TO_WDOG_TICKS(x) ((x)  16)
 +#define WDOG_TICKS_TO_SECS(x) ((x)  16)
 +
 +struct