Re: [PATCH 1/2] watchdog: bcm281xx: Watchdog Driver

2013-11-12 Thread Guenter Roeck

On 11/12/2013 02:17 PM, Markus Mayer wrote:


+
+ if (!timeout)
+ dev_info(wdog->dev, "Watchdog timer stopped");
+

All that noise.


Would it be acceptable to turn these calls into dev_dbg() calls, here
and elsewhere?



Ok with me.



+
+ wdt->resolution = SECWDOG_DEFAULT_RESOLUTION;
+ ret = bcm_kona_wdt_set_resolution_reg(wdt);
+ if (ret) {
+ dev_err(dev, "Failed to set resolution (error: %d)", ret);


ret can be -EAGAIN or -EINVAL. -EINVAL suggests a bad internale error (hopefully
SECWDOG_DEFAULT_RESOLUTION is defined to be smaller than SECWDOG_MAX_RES),
and if it is -EAGAIN there should be no error message.

Actually, bcm_kona_wdt_set_resolution_reg is only called from here, meaning the
error check in the function is really unnecessary.


This again goes back to making resolution available to userland. Then
bcm_kona_wdt_set_resolution_reg() would be called from elsewhere. Why
is it bad to print an error message on timeout? Would this still apply


That was related to -EAGAIN. Which would be bad here anyway as it could result
in an endless loop if there is a problem with the chip.


if I switch the code to -ETIMEDOUT?


That is one option, or -EIO if the condition indicates a chip error.


+ return ret;
+ }
+
+ spin_lock_init(&wdt->lock);
+ platform_set_drvdata(pdev, wdt);
+ watchdog_set_drvdata(&bcm_kona_wdt_wdd, wdt);
+
+ ret = bcm_kona_wdt_set_timeout_reg(&bcm_kona_wdt_wdd);
+ if (ret) {
+ dev_err(dev, "Failed set watchdog timeout");


The only error case is -EAGAIN. I don't think there should be an error mesasge
in this case (though I am not sure what the reaction should be).


I am thinking that probe() needs to return an error if setting the
timeout fails, as it can't really rely on the watchdog timer or let
the system use it. Shouldn't that be accompanied by an error message
letting the user know what happened?


Oh, I agree it should return an error, and an error message is ok as well.
I am just sure it should not be -EAGAIN, but I don't know what it should be.
Maybe -ETIMEDOUT, or -EIO.


+ return ret;
+ }
+
+ ret = watchdog_register_device(&bcm_kona_wdt_wdd);
+ if (ret) {
+ dev_err(dev, "Failed to register watchdog device");
+ return ret;
+ }
+
+#ifdef CONFIG_BCM_KONA_WDT_DEBUG
+ wdt->debugfs = bcm_kona_wdt_debugfs_init(wdt, &bcm_kona_wdt_wdd);
+#endif
+ dev_info(dev, "Broadcom Kona Watchdog Timer");
+

Such messages are in general considered nuisance nowadays. I would suggest to
remove it (or ask Greg KH for advice).



Referring to your other mail seems those messages are falling out of favor.
I consider it a nuisance, though so far I let it go through. The messages do
increase boot time, especially on systems with slow serial console, and IMO
do not provide any real value. Users either don't care, or can check if the
driver is loaded by other means. I would suggest to at least make it dev_dbg.

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 1/2] watchdog: bcm281xx: Watchdog Driver

2013-11-12 Thread Markus Mayer
On 12 November 2013 15:39, One Thousand Gnomes
 wrote:
>
>> +static int bcm_kona_wdt_set_resolution_reg(struct bcm_kona_wdt *wdt)
>> +{
>> + uint32_t val;
>> + int timeout;
>> + unsigned long flags;
>> + int ret = 0;
>> +
>> + if (wdt->resolution > SECWDOG_MAX_RES)
>> + return -EINVAL;
>> +
>> + spin_lock_irqsave(&wdt->lock, flags);
>> +
>> + val = secure_register_read(wdt->base + SECWDOG_CTRL_REG, &timeout);
>> + if (!timeout) {
>> + val &= ~SECWDOG_RES_MASK;
>> + val |= wdt->resolution << SECWDOG_CLKS_SHIFT;
>> + writel_relaxed(val, wdt->base + SECWDOG_CTRL_REG);
>> + } else {
>> + ret = -EAGAIN;
>
> This is I think the wrong choice of return. If the register fails to read
> then presumably the device is b*ggered ? In which case return something
> like -EIO and log something nasty.
>
> EAGAIN has fairly specific semantics around signals and/or specific
> requests for an I/O operation not to wait.

I will change that based on Guenter's and your comments.

>> + spin_lock_irqsave(&wdt->lock, flags);
>> +
>> + val = secure_register_read(wdt->base + SECWDOG_CTRL_REG, &timeout);
>> + if (!timeout) {
>> + val &= ~SECWDOG_COUNT_MASK;
>> + val |= SECS_TO_TICKS(wdog->timeout, wdt);
>> + writel_relaxed(val, wdt->base + SECWDOG_CTRL_REG);
>> + } else {
>> + ret = -EAGAIN;
>> + }
>> +
>> + spin_unlock_irqrestore(&wdt->lock, flags);
>
> As an aside you could fold most of these functions into one single helper
> method that read CTRL_REG, did the and and or and wrote the result back
> rather than repeating yourself. But hey if you like typing...

I'll look into that.

>> + res = platform_get_resource(pdev, IORESOURCE_MEM, 0);
>
> What if there is no resource present ?

Then devm_ioremap_resource() will print an error message and return
with ERR_PTR(-EINVAL). Subsequently probe() will fail. So it'll work
as intended.

>> + dev_info(dev, "Broadcom Kona Watchdog Timer");
>
> The module load succeeded - the user knows it loaded from that. Likewise
> the unload spewage is unwanted and the kernel would just drown in such
> messages if we didn't keep them in check. If you need the for debug then
> mark them dev_dbg but otherwise bin them.

I already changed them all to "debug" in my local tree. For this one
message, I am feeling a bit ambivalent, though. The user only knows
loading of the module succeeded if the user manually loaded the module
-- which, I imagine, would almost never be the case. The driver is
either going to be built in or loaded automatically by some facility.
In which case it might be nice to have some confirmation of that fact
in the logs.

Regards,
-Markus

-- 
Markus Mayer
Broadcom Landing Team
--
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 1/2] watchdog: bcm281xx: Watchdog Driver

2013-11-12 Thread One Thousand Gnomes

> +static int bcm_kona_wdt_set_resolution_reg(struct bcm_kona_wdt *wdt)
> +{
> + uint32_t val;
> + int timeout;
> + unsigned long flags;
> + int ret = 0;
> +
> + if (wdt->resolution > SECWDOG_MAX_RES)
> + return -EINVAL;
> +
> + spin_lock_irqsave(&wdt->lock, flags);
> +
> + val = secure_register_read(wdt->base + SECWDOG_CTRL_REG, &timeout);
> + if (!timeout) {
> + val &= ~SECWDOG_RES_MASK;
> + val |= wdt->resolution << SECWDOG_CLKS_SHIFT;
> + writel_relaxed(val, wdt->base + SECWDOG_CTRL_REG);
> + } else {
> + ret = -EAGAIN;

This is I think the wrong choice of return. If the register fails to read
then presumably the device is b*ggered ? In which case return something
like -EIO and log something nasty.

EAGAIN has fairly specific semantics around signals and/or specific
requests for an I/O operation not to wait.

> + spin_lock_irqsave(&wdt->lock, flags);
> +
> + val = secure_register_read(wdt->base + SECWDOG_CTRL_REG, &timeout);
> + if (!timeout) {
> + val &= ~SECWDOG_COUNT_MASK;
> + val |= SECS_TO_TICKS(wdog->timeout, wdt);
> + writel_relaxed(val, wdt->base + SECWDOG_CTRL_REG);
> + } else {
> + ret = -EAGAIN;
> + }
> +
> + spin_unlock_irqrestore(&wdt->lock, flags);

As an aside you could fold most of these functions into one single helper
method that read CTRL_REG, did the and and or and wrote the result back
rather than repeating yourself. But hey if you like typing...

> + res = platform_get_resource(pdev, IORESOURCE_MEM, 0);

What if there is no resource present ?

> + dev_info(dev, "Broadcom Kona Watchdog Timer");

The module load succeeded - the user knows it loaded from that. Likewise
the unload spewage is unwanted and the kernel would just drown in such
messages if we didn't keep them in check. If you need the for debug then
mark them dev_dbg but otherwise bin them.

Alan
--
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 1/2] watchdog: bcm281xx: Watchdog Driver

2013-11-12 Thread Markus Mayer
On 11 November 2013 09:34, Guenter Roeck  wrote:
> On Fri, Nov 08, 2013 at 12:44:47PM -0800, Markus Mayer wrote:
>> This commit adds support for the watchdog timer used on the BCM281xx
>> family of SoCs.
>>
>> Signed-off-by: Markus Mayer 
>> Reviewed-by: Matt Porter 
>
> Overall I am a bit concerned about the heavy use of -EAGAIN, which is a first
> for watchdog drivers. The condition seems to be around secure_register_read.
> If that is stuck for some reason, it may cause endless retries. Not sure if
> that is a good idea. -ETIMEDOUT might be a betetr idea.
>
>> ---
>>  drivers/watchdog/Kconfig|   21 +++
>>  drivers/watchdog/Makefile   |1 +
>>  drivers/watchdog/bcm_kona_wdt.c |  399 
>> +++
>>  3 files changed, 421 insertions(+)
>>  create mode 100644 drivers/watchdog/bcm_kona_wdt.c
>>
>> diff --git a/drivers/watchdog/Kconfig b/drivers/watchdog/Kconfig
>> index d1d53f3..59013f6 100644
>> --- a/drivers/watchdog/Kconfig
>> +++ b/drivers/watchdog/Kconfig
>> @@ -1121,6 +1121,27 @@ config BCM2835_WDT
>> To compile this driver as a loadable module, choose M here.
>> The module will be called bcm2835_wdt.
>>
>> +config BCM_KONA_WDT
>> + tristate "BCM Kona Watchdog"
>> + depends on ARCH_BCM
>> + select WATCHDOG_CORE
>> + help
>> +   Support for the watchdog timer on the following Broadcom BCM281xx
>> +   family, which includes BCM11130, BCM11140, BCM11351, BCM28145 and
>> +   BCM28155 variants.
>> +
>> +   Say 'Y' or 'M' here to enable the driver.
>> +
> It is customary to state the module name if drivers are built as module.
>
>> +config BCM_KONA_WDT_DEBUG
>> + bool "DEBUGFS support for BCM Kona Watchdog"
>> + depends on BCM_KONA_WDT
>> + help
>> +   If enabled, adds /sys/kernel/debug/bcm-kona-wdt/info which provides
>> +   access to the driver's internal data structures as well as watchdog
>> +   timer hardware registres.
>> +
>> +   If in doubt, say 'N'.
>> +
>>  config LANTIQ_WDT
>>   tristate "Lantiq SoC watchdog"
>>   depends on LANTIQ
>> diff --git a/drivers/watchdog/Makefile b/drivers/watchdog/Makefile
>> index 6c5bb27..7c860ca 100644
>> --- a/drivers/watchdog/Makefile
>> +++ b/drivers/watchdog/Makefile
>> @@ -55,6 +55,7 @@ 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
>> +obj-$(CONFIG_BCM_KONA_WDT) += bcm_kona_wdt.o
>>
>>  # AVR32 Architecture
>>  obj-$(CONFIG_AT32AP700X_WDT) += at32ap700x_wdt.o
>> diff --git a/drivers/watchdog/bcm_kona_wdt.c 
>> b/drivers/watchdog/bcm_kona_wdt.c
>> new file mode 100644
>> index 000..c47ac77
>> --- /dev/null
>> +++ b/drivers/watchdog/bcm_kona_wdt.c
>> @@ -0,0 +1,399 @@
>> +/*
>> + * Copyright (C) 2013 Broadcom Corporation
>> + *
>> + * 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 version 2.
>> + *
>> + * This program is distributed "as is" WITHOUT ANY WARRANTY of any
>> + * kind, whether express or implied; without even the implied warranty
>> + * of MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
>> + * GNU General Public License for more details.
>> + */
>> +
>> +#include 
>> +#include 
>> +#include 
>> +#include 
>> +#include 
>> +#include 
>> +#include 
>> +#include 
>> +
>> +#define SECWDOG_CTRL_REG 0x
>> +#define SECWDOG_COUNT_REG0x0004
>> +
>> +#define SECWDOG_RESERVED_MASK0x1dff
>> +#define SECWDOG_WD_LOAD_FLAG_MASK0x1000
>> +#define SECWDOG_EN_MASK  0x0800
>> +#define SECWDOG_SRSTEN_MASK  0x0400
>> +#define SECWDOG_RES_MASK 0x00f0
>> +#define SECWDOG_COUNT_MASK   0x000f
>> +
>> +#define SECWDOG_MAX_COUNTSECWDOG_COUNT_MASK
>> +#define SECWDOG_CLKS_SHIFT   20
>> +#define SECWDOG_MAX_RES  15
>> +#define SECWDOG_DEFAULT_RESOLUTION   4
>> +#define SECWDOG_MAX_TRY  1
>> +
>> +#define SECS_TO_TICKS(x, w)  ((x) << (w)->resolution)
>> +#define TICKS_TO_SECS(x, w)  ((x) >> (w)->resolution)
>> +
>> +#define BCM_KONA_WDT_NAME"bcm-kona-wdt"
>> +
>> +struct bcm_kona_wdt {
>> + void __iomem *base;
>> + int resolution;
>> + spinlock_t lock;
>> +#ifdef CONFIG_BCM_KONA_WDT_DEBUG
>> + struct dentry *debugfs;
>> +#endif
>> +};
>> +
>> +static uint32_t secure_register_read(void __iomem *addr, int *timeout)
>> +{
> From the context, it appears that timeout is really a boolean and does not
> reflect a count (as one might think). I would suggest to make it a boolean.
>
>> + uint32_t val;
>> + unsigned count = 0;
>> +
>> + do {
>> + val = readl_relaxed(addr);
>> + count++;
>> + } while ((val & SECWDOG_

Re: [PATCH 1/2] watchdog: bcm281xx: Watchdog Driver

2013-11-11 Thread Guenter Roeck
On Fri, Nov 08, 2013 at 12:44:47PM -0800, Markus Mayer wrote:
> This commit adds support for the watchdog timer used on the BCM281xx
> family of SoCs.
> 
> Signed-off-by: Markus Mayer 
> Reviewed-by: Matt Porter 

Overall I am a bit concerned about the heavy use of -EAGAIN, which is a first
for watchdog drivers. The condition seems to be around secure_register_read.
If that is stuck for some reason, it may cause endless retries. Not sure if
that is a good idea. -ETIMEDOUT might be a betetr idea.

> ---
>  drivers/watchdog/Kconfig|   21 +++
>  drivers/watchdog/Makefile   |1 +
>  drivers/watchdog/bcm_kona_wdt.c |  399 
> +++
>  3 files changed, 421 insertions(+)
>  create mode 100644 drivers/watchdog/bcm_kona_wdt.c
> 
> diff --git a/drivers/watchdog/Kconfig b/drivers/watchdog/Kconfig
> index d1d53f3..59013f6 100644
> --- a/drivers/watchdog/Kconfig
> +++ b/drivers/watchdog/Kconfig
> @@ -1121,6 +1121,27 @@ config BCM2835_WDT
> To compile this driver as a loadable module, choose M here.
> The module will be called bcm2835_wdt.
>  
> +config BCM_KONA_WDT
> + tristate "BCM Kona Watchdog"
> + depends on ARCH_BCM
> + select WATCHDOG_CORE
> + help
> +   Support for the watchdog timer on the following Broadcom BCM281xx 
> +   family, which includes BCM11130, BCM11140, BCM11351, BCM28145 and
> +   BCM28155 variants.
> +
> +   Say 'Y' or 'M' here to enable the driver.
> +
It is customary to state the module name if drivers are built as module.

> +config BCM_KONA_WDT_DEBUG
> + bool "DEBUGFS support for BCM Kona Watchdog"
> + depends on BCM_KONA_WDT
> + help
> +   If enabled, adds /sys/kernel/debug/bcm-kona-wdt/info which provides
> +   access to the driver's internal data structures as well as watchdog
> +   timer hardware registres.
> +
> +   If in doubt, say 'N'.
> +
>  config LANTIQ_WDT
>   tristate "Lantiq SoC watchdog"
>   depends on LANTIQ
> diff --git a/drivers/watchdog/Makefile b/drivers/watchdog/Makefile
> index 6c5bb27..7c860ca 100644
> --- a/drivers/watchdog/Makefile
> +++ b/drivers/watchdog/Makefile
> @@ -55,6 +55,7 @@ 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
> +obj-$(CONFIG_BCM_KONA_WDT) += bcm_kona_wdt.o
>  
>  # AVR32 Architecture
>  obj-$(CONFIG_AT32AP700X_WDT) += at32ap700x_wdt.o
> diff --git a/drivers/watchdog/bcm_kona_wdt.c b/drivers/watchdog/bcm_kona_wdt.c
> new file mode 100644
> index 000..c47ac77
> --- /dev/null
> +++ b/drivers/watchdog/bcm_kona_wdt.c
> @@ -0,0 +1,399 @@
> +/*
> + * Copyright (C) 2013 Broadcom Corporation
> + *
> + * 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 version 2.
> + *
> + * This program is distributed "as is" WITHOUT ANY WARRANTY of any
> + * kind, whether express or implied; without even the implied warranty
> + * of MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
> + * GNU General Public License for more details.
> + */
> +
> +#include 
> +#include 
> +#include 
> +#include 
> +#include 
> +#include 
> +#include 
> +#include 
> +
> +#define SECWDOG_CTRL_REG 0x
> +#define SECWDOG_COUNT_REG0x0004
> +
> +#define SECWDOG_RESERVED_MASK0x1dff
> +#define SECWDOG_WD_LOAD_FLAG_MASK0x1000
> +#define SECWDOG_EN_MASK  0x0800
> +#define SECWDOG_SRSTEN_MASK  0x0400
> +#define SECWDOG_RES_MASK 0x00f0
> +#define SECWDOG_COUNT_MASK   0x000f
> +
> +#define SECWDOG_MAX_COUNTSECWDOG_COUNT_MASK
> +#define SECWDOG_CLKS_SHIFT   20
> +#define SECWDOG_MAX_RES  15
> +#define SECWDOG_DEFAULT_RESOLUTION   4
> +#define SECWDOG_MAX_TRY  1
> +
> +#define SECS_TO_TICKS(x, w)  ((x) << (w)->resolution)
> +#define TICKS_TO_SECS(x, w)  ((x) >> (w)->resolution)
> +
> +#define BCM_KONA_WDT_NAME"bcm-kona-wdt"
> +
> +struct bcm_kona_wdt {
> + void __iomem *base;
> + int resolution;
> + spinlock_t lock;
> +#ifdef CONFIG_BCM_KONA_WDT_DEBUG
> + struct dentry *debugfs;
> +#endif
> +};
> +
> +static uint32_t secure_register_read(void __iomem *addr, int *timeout)
> +{
>From the context, it appears that timeout is really a boolean and does not
reflect a count (as one might think). I would suggest to make it a boolean.

> + uint32_t val;
> + unsigned count = 0;
> +
> + do {
> + val = readl_relaxed(addr);
> + count++;
> + } while ((val & SECWDOG_WD_LOAD_FLAG_MASK) != 0 &&

The "!= 0" is really unnecessary here.

Also, wonder if there should be a specific delay in the loop. Looping without
delay means this will time out faster on fa

[PATCH 1/2] watchdog: bcm281xx: Watchdog Driver

2013-11-08 Thread Markus Mayer
This commit adds support for the watchdog timer used on the BCM281xx
family of SoCs.

Signed-off-by: Markus Mayer 
Reviewed-by: Matt Porter 
---
 drivers/watchdog/Kconfig|   21 +++
 drivers/watchdog/Makefile   |1 +
 drivers/watchdog/bcm_kona_wdt.c |  399 +++
 3 files changed, 421 insertions(+)
 create mode 100644 drivers/watchdog/bcm_kona_wdt.c

diff --git a/drivers/watchdog/Kconfig b/drivers/watchdog/Kconfig
index d1d53f3..59013f6 100644
--- a/drivers/watchdog/Kconfig
+++ b/drivers/watchdog/Kconfig
@@ -1121,6 +1121,27 @@ config BCM2835_WDT
  To compile this driver as a loadable module, choose M here.
  The module will be called bcm2835_wdt.
 
+config BCM_KONA_WDT
+   tristate "BCM Kona Watchdog"
+   depends on ARCH_BCM
+   select WATCHDOG_CORE
+   help
+ Support for the watchdog timer on the following Broadcom BCM281xx 
+ family, which includes BCM11130, BCM11140, BCM11351, BCM28145 and
+ BCM28155 variants.
+
+ Say 'Y' or 'M' here to enable the driver.
+
+config BCM_KONA_WDT_DEBUG
+   bool "DEBUGFS support for BCM Kona Watchdog"
+   depends on BCM_KONA_WDT
+   help
+ If enabled, adds /sys/kernel/debug/bcm-kona-wdt/info which provides
+ access to the driver's internal data structures as well as watchdog
+ timer hardware registres.
+
+ If in doubt, say 'N'.
+
 config LANTIQ_WDT
tristate "Lantiq SoC watchdog"
depends on LANTIQ
diff --git a/drivers/watchdog/Makefile b/drivers/watchdog/Makefile
index 6c5bb27..7c860ca 100644
--- a/drivers/watchdog/Makefile
+++ b/drivers/watchdog/Makefile
@@ -55,6 +55,7 @@ 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
+obj-$(CONFIG_BCM_KONA_WDT) += bcm_kona_wdt.o
 
 # AVR32 Architecture
 obj-$(CONFIG_AT32AP700X_WDT) += at32ap700x_wdt.o
diff --git a/drivers/watchdog/bcm_kona_wdt.c b/drivers/watchdog/bcm_kona_wdt.c
new file mode 100644
index 000..c47ac77
--- /dev/null
+++ b/drivers/watchdog/bcm_kona_wdt.c
@@ -0,0 +1,399 @@
+/*
+ * Copyright (C) 2013 Broadcom Corporation
+ *
+ * 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 version 2.
+ *
+ * This program is distributed "as is" WITHOUT ANY WARRANTY of any
+ * kind, whether express or implied; without even the implied warranty
+ * of MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+ * GNU General Public License for more details.
+ */
+
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+
+#define SECWDOG_CTRL_REG   0x
+#define SECWDOG_COUNT_REG  0x0004
+
+#define SECWDOG_RESERVED_MASK  0x1dff
+#define SECWDOG_WD_LOAD_FLAG_MASK  0x1000
+#define SECWDOG_EN_MASK0x0800
+#define SECWDOG_SRSTEN_MASK0x0400
+#define SECWDOG_RES_MASK   0x00f0
+#define SECWDOG_COUNT_MASK 0x000f
+
+#define SECWDOG_MAX_COUNT  SECWDOG_COUNT_MASK
+#define SECWDOG_CLKS_SHIFT 20
+#define SECWDOG_MAX_RES15
+#define SECWDOG_DEFAULT_RESOLUTION 4
+#define SECWDOG_MAX_TRY1
+
+#define SECS_TO_TICKS(x, w)((x) << (w)->resolution)
+#define TICKS_TO_SECS(x, w)((x) >> (w)->resolution)
+
+#define BCM_KONA_WDT_NAME  "bcm-kona-wdt"
+
+struct bcm_kona_wdt {
+   void __iomem *base;
+   int resolution;
+   spinlock_t lock;
+#ifdef CONFIG_BCM_KONA_WDT_DEBUG
+   struct dentry *debugfs;
+#endif
+};
+
+static uint32_t secure_register_read(void __iomem *addr, int *timeout)
+{
+   uint32_t val;
+   unsigned count = 0;
+
+   do {
+   val = readl_relaxed(addr);
+   count++;
+   } while ((val & SECWDOG_WD_LOAD_FLAG_MASK) != 0 &&
+   count < SECWDOG_MAX_TRY);
+   if (timeout)
+   *timeout = ((val & SECWDOG_WD_LOAD_FLAG_MASK) != 0);
+
+   /* We always mask out reserved bits before returning the value. */
+   val &= SECWDOG_RESERVED_MASK;
+
+   return val;
+}
+
+
+#ifdef CONFIG_BCM_KONA_WDT_DEBUG
+
+static int bcm_kona_wdt_dbg_show(struct seq_file *s, void *data)
+{
+   uint32_t ctl_val, cur_val;
+   int ret, ctl_timeout, cur_timeout;
+   unsigned long flags;
+   struct bcm_kona_wdt *wdt = s->private;
+
+   if (!wdt)
+   return seq_printf(s, "No device pointer\n");
+
+   spin_lock_irqsave(&wdt->lock, flags);
+   ctl_val = secure_register_read(wdt->base + SECWDOG_CTRL_REG,
+   &ctl_timeout);
+   cur_val = secure_register_read(wdt->base + SECWDOG_COUNT_REG,
+   &cur_timeout);
+   sp