[PATCH v2] watchdog: Add a watchdog driver for Raspberry Pi boards

This driver supports the bcm2835 watchdog found on Raspberry Pi boards. It is derived from the Linux driver and was tested on two Raspberry Pi board versions (B+ and 3B+).
Signed-off-by: Etienne Dublé etienne.duble@imag.fr --- Changes for v2: - fixed whitespaces in email - moved a static variable to the priv struct
drivers/watchdog/Kconfig | 9 +++ drivers/watchdog/Makefile | 1 + drivers/watchdog/bcm2835_wdt.c | 140 +++++++++++++++++++++++++++++++++ 3 files changed, 150 insertions(+) create mode 100644 drivers/watchdog/bcm2835_wdt.c
diff --git a/drivers/watchdog/Kconfig b/drivers/watchdog/Kconfig index f1b1cf63ca..06c0d630c8 100644 --- a/drivers/watchdog/Kconfig +++ b/drivers/watchdog/Kconfig @@ -30,6 +30,7 @@ config WATCHDOG_TIMEOUT_MSECS default 128000 if ARCH_MX7 || ARCH_VF610 default 30000 if ARCH_SOCFPGA default 16000 if ARCH_SUNXI + default 15000 if ARCH_BCM283X default 60000 help Watchdog timeout in msec @@ -326,6 +327,14 @@ config WDT_SUNXI help Enable support for the watchdog timer in Allwinner sunxi SoCs.
+config WDT_BCM2835 + bool "Broadcom 2835 watchdog timer support" + depends on WDT && ARCH_BCM283X + default y + help + Enable support for the watchdog timer in Broadcom 283X SoCs such + as Raspberry Pi boards. + config XILINX_TB_WATCHDOG bool "Xilinx Axi watchdog timer support" depends on WDT diff --git a/drivers/watchdog/Makefile b/drivers/watchdog/Makefile index 446d961d7d..f99915960c 100644 --- a/drivers/watchdog/Makefile +++ b/drivers/watchdog/Makefile @@ -21,6 +21,7 @@ obj-$(CONFIG_WDT_APPLE) += apple_wdt.o obj-$(CONFIG_WDT_ARMADA_37XX) += armada-37xx-wdt.o obj-$(CONFIG_WDT_ASPEED) += ast_wdt.o obj-$(CONFIG_WDT_AST2600) += ast2600_wdt.o +obj-$(CONFIG_WDT_BCM2835) += bcm2835_wdt.o obj-$(CONFIG_WDT_BCM6345) += bcm6345_wdt.o obj-$(CONFIG_WDT_BOOKE) += booke_wdt.o obj-$(CONFIG_WDT_CORTINA) += cortina_wdt.o diff --git a/drivers/watchdog/bcm2835_wdt.c b/drivers/watchdog/bcm2835_wdt.c new file mode 100644 index 0000000000..00f7806cd5 --- /dev/null +++ b/drivers/watchdog/bcm2835_wdt.c @@ -0,0 +1,140 @@ +// SPDX-License-Identifier: GPL-2.0+ +/* + * Copyright (C) 2013 Lubomir Rintel lkundrak@v3.sk + * Copyright (C) 2023 Etienne Dublé (CNRS) etienne.duble@imag.fr + * + * This code is mostly derived from the linux driver. + */ + +#include <dm.h> +#include <wdt.h> +#include <asm/io.h> +#include <linux/delay.h> + +#define PM_RSTC 0x1c +#define PM_WDOG 0x24 + +#define PM_PASSWORD 0x5a000000 + +/* The hardware supports a maximum timeout value of 0xfffff ticks + * (just below 16 seconds). + * U-boot users specify the timeout as a number of milliseconds + * by using variable CONFIG_WATCHDOG_TIMEOUT_MSECS. + * The maximum value should be 15999 ms in our case. + * However, u-boot internally converts this config value to seconds, + * thus specifying 15999 actually means 15000 ms (0xf0000 ticks). + */ +#define PM_WDOG_MAX_TICKS 0x000f0000 +#define PM_RSTC_WRCFG_CLR 0xffffffcf +#define PM_RSTC_WRCFG_FULL_RESET 0x00000020 +#define PM_RSTC_RESET 0x00000102 + +#define MS_TO_WDOG_TICKS(x) (((x) << 16) / 1000) + +struct bcm2835_wdt_priv { + void __iomem *base; + u64 timeout_ticks; +}; + +static int bcm2835_wdt_start_ticks(struct udevice *dev, + u64 timeout_ticks, ulong flags) +{ + struct bcm2835_wdt_priv *priv = dev_get_priv(dev); + void __iomem *base = priv->base; + u32 cur; + + writel(PM_PASSWORD | timeout_ticks, base + PM_WDOG); + cur = readl(base + PM_RSTC); + writel(PM_PASSWORD | (cur & PM_RSTC_WRCFG_CLR) | + PM_RSTC_WRCFG_FULL_RESET, base + PM_RSTC); + + return 0; +} + +static int bcm2835_wdt_start(struct udevice *dev, u64 timeout_ms, ulong flags) +{ + struct bcm2835_wdt_priv *priv = dev_get_priv(dev); + + priv->timeout_ticks = MS_TO_WDOG_TICKS(timeout_ms); + + if (priv->timeout_ticks > PM_WDOG_MAX_TICKS) { + printf("WARNING: bcm2835_wdt cannot handle large timeout values.\n"); + printf(" Setting the max value of 15000 ms instead.\n"); + printf(" Set CONFIG_WATCHDOG_TIMEOUT_MSECS=15000 at most " + "to avoid this warning.\n"); + priv->timeout_ticks = PM_WDOG_MAX_TICKS; + } + + return bcm2835_wdt_start_ticks(dev, priv->timeout_ticks, flags); +} + +static int bcm2835_wdt_reset(struct udevice *dev) +{ + struct bcm2835_wdt_priv *priv = dev_get_priv(dev); + + /* restart the timer with the value of priv->timeout_ticks + * saved from the last bcm2835_wdt_start() call. + */ + return bcm2835_wdt_start_ticks(dev, priv->timeout_ticks, 0); +} + +static int bcm2835_wdt_stop(struct udevice *dev) +{ + struct bcm2835_wdt_priv *priv = dev_get_priv(dev); + void __iomem *base = priv->base; + + writel(PM_PASSWORD | PM_RSTC_RESET, base + PM_RSTC); + + return 0; +} + +static int bcm2835_wdt_expire_now(struct udevice *dev, ulong flags) +{ + int ret; + + /* use a timeout of 10 ticks (~150us) */ + ret = bcm2835_wdt_start_ticks(dev, 10, flags); + if (ret) + return ret; + + mdelay(500); + + return 0; +} + +static const struct wdt_ops bcm2835_wdt_ops = { + .reset = bcm2835_wdt_reset, + .start = bcm2835_wdt_start, + .stop = bcm2835_wdt_stop, + .expire_now = bcm2835_wdt_expire_now, +}; + +static const struct udevice_id bcm2835_wdt_ids[] = { + { .compatible = "brcm,bcm2835-pm" }, + { .compatible = "brcm,bcm2835-pm-wdt" }, + { /* sentinel */ } +}; + +static int bcm2835_wdt_probe(struct udevice *dev) +{ + struct bcm2835_wdt_priv *priv = dev_get_priv(dev); + + priv->base = dev_remap_addr(dev); + if (!priv->base) + return -EINVAL; + + priv->timeout_ticks = PM_WDOG_MAX_TICKS; + + bcm2835_wdt_stop(dev); + + return 0; +} + +U_BOOT_DRIVER(bcm2835_wdt) = { + .name = "bcm2835_wdt", + .id = UCLASS_WDT, + .of_match = bcm2835_wdt_ids, + .probe = bcm2835_wdt_probe, + .priv_auto = sizeof(struct bcm2835_wdt_priv), + .ops = &bcm2835_wdt_ops, +};

Hi Etienne,
On 1/24/23 15:55, ETIENNE DUBLE wrote:
This driver supports the bcm2835 watchdog found on Raspberry Pi boards. It is derived from the Linux driver and was tested on two Raspberry Pi board versions (B+ and 3B+).
Signed-off-by: Etienne Dublé etienne.duble@imag.fr
Changes for v2:
fixed whitespaces in email
moved a static variable to the priv struct
drivers/watchdog/Kconfig | 9 +++ drivers/watchdog/Makefile | 1 + drivers/watchdog/bcm2835_wdt.c | 140 +++++++++++++++++++++++++++++++++ 3 files changed, 150 insertions(+) create mode 100644 drivers/watchdog/bcm2835_wdt.c
diff --git a/drivers/watchdog/Kconfig b/drivers/watchdog/Kconfig index f1b1cf63ca..06c0d630c8 100644 --- a/drivers/watchdog/Kconfig +++ b/drivers/watchdog/Kconfig @@ -30,6 +30,7 @@ config WATCHDOG_TIMEOUT_MSECS default 128000 if ARCH_MX7 || ARCH_VF610 default 30000 if ARCH_SOCFPGA default 16000 if ARCH_SUNXI
- default 15000 if ARCH_BCM283X default 60000 help Watchdog timeout in msec
@@ -326,6 +327,14 @@ config WDT_SUNXI help Enable support for the watchdog timer in Allwinner sunxi SoCs.
+config WDT_BCM2835
- bool "Broadcom 2835 watchdog timer support"
- depends on WDT && ARCH_BCM283X
- default y
- help
Enable support for the watchdog timer in Broadcom 283X SoCs such
as Raspberry Pi boards.
- config XILINX_TB_WATCHDOG bool "Xilinx Axi watchdog timer support" depends on WDT
diff --git a/drivers/watchdog/Makefile b/drivers/watchdog/Makefile index 446d961d7d..f99915960c 100644 --- a/drivers/watchdog/Makefile +++ b/drivers/watchdog/Makefile @@ -21,6 +21,7 @@ obj-$(CONFIG_WDT_APPLE) += apple_wdt.o obj-$(CONFIG_WDT_ARMADA_37XX) += armada-37xx-wdt.o obj-$(CONFIG_WDT_ASPEED) += ast_wdt.o obj-$(CONFIG_WDT_AST2600) += ast2600_wdt.o +obj-$(CONFIG_WDT_BCM2835) += bcm2835_wdt.o obj-$(CONFIG_WDT_BCM6345) += bcm6345_wdt.o obj-$(CONFIG_WDT_BOOKE) += booke_wdt.o obj-$(CONFIG_WDT_CORTINA) += cortina_wdt.o diff --git a/drivers/watchdog/bcm2835_wdt.c b/drivers/watchdog/bcm2835_wdt.c new file mode 100644 index 0000000000..00f7806cd5 --- /dev/null +++ b/drivers/watchdog/bcm2835_wdt.c @@ -0,0 +1,140 @@ +// SPDX-License-Identifier: GPL-2.0+ +/*
- Copyright (C) 2013 Lubomir Rintel lkundrak@v3.sk
- Copyright (C) 2023 Etienne Dublé (CNRS) etienne.duble@imag.fr
- This code is mostly derived from the linux driver.
- */
+#include <dm.h> +#include <wdt.h> +#include <asm/io.h> +#include <linux/delay.h>
+#define PM_RSTC 0x1c +#define PM_WDOG 0x24
+#define PM_PASSWORD 0x5a000000
+/* The hardware supports a maximum timeout value of 0xfffff ticks
- (just below 16 seconds).
- U-boot users specify the timeout as a number of milliseconds
- by using variable CONFIG_WATCHDOG_TIMEOUT_MSECS.
- The maximum value should be 15999 ms in our case.
- However, u-boot internally converts this config value to seconds,
- thus specifying 15999 actually means 15000 ms (0xf0000 ticks).
means 15999 ms?
- */
+#define PM_WDOG_MAX_TICKS 0x000f0000 +#define PM_RSTC_WRCFG_CLR 0xffffffcf +#define PM_RSTC_WRCFG_FULL_RESET 0x00000020 +#define PM_RSTC_RESET 0x00000102
+#define MS_TO_WDOG_TICKS(x) (((x) << 16) / 1000)
+struct bcm2835_wdt_priv {
- void __iomem *base;
- u64 timeout_ticks;
+};
+static int bcm2835_wdt_start_ticks(struct udevice *dev,
u64 timeout_ticks, ulong flags)
+{
- struct bcm2835_wdt_priv *priv = dev_get_priv(dev);
- void __iomem *base = priv->base;
- u32 cur;
- writel(PM_PASSWORD | timeout_ticks, base + PM_WDOG);
- cur = readl(base + PM_RSTC);
- writel(PM_PASSWORD | (cur & PM_RSTC_WRCFG_CLR) |
PM_RSTC_WRCFG_FULL_RESET, base + PM_RSTC);
This does not seem to be aligned with the "(" from the line above.
BTW: Please use scripts/checkpatch.pl to see, if the patch has no more issues.
- return 0;
+}
+static int bcm2835_wdt_start(struct udevice *dev, u64 timeout_ms, ulong flags) +{
- struct bcm2835_wdt_priv *priv = dev_get_priv(dev);
- priv->timeout_ticks = MS_TO_WDOG_TICKS(timeout_ms);
- if (priv->timeout_ticks > PM_WDOG_MAX_TICKS) {
printf("WARNING: bcm2835_wdt cannot handle large timeout values.\n");
printf(" Setting the max value of 15000 ms instead.\n");
printf(" Set CONFIG_WATCHDOG_TIMEOUT_MSECS=15000 at most "
"to avoid this warning.\n");
This seems way too long for a warning from this driver. Please shorten it to one line. The comment above already covers this limitation AFAICT.
Thanks, Stefan
priv->timeout_ticks = PM_WDOG_MAX_TICKS;
- }
- return bcm2835_wdt_start_ticks(dev, priv->timeout_ticks, flags);
+}
+static int bcm2835_wdt_reset(struct udevice *dev) +{
- struct bcm2835_wdt_priv *priv = dev_get_priv(dev);
- /* restart the timer with the value of priv->timeout_ticks
* saved from the last bcm2835_wdt_start() call.
*/
- return bcm2835_wdt_start_ticks(dev, priv->timeout_ticks, 0);
+}
+static int bcm2835_wdt_stop(struct udevice *dev) +{
- struct bcm2835_wdt_priv *priv = dev_get_priv(dev);
- void __iomem *base = priv->base;
- writel(PM_PASSWORD | PM_RSTC_RESET, base + PM_RSTC);
- return 0;
+}
+static int bcm2835_wdt_expire_now(struct udevice *dev, ulong flags) +{
- int ret;
- /* use a timeout of 10 ticks (~150us) */
- ret = bcm2835_wdt_start_ticks(dev, 10, flags);
- if (ret)
return ret;
- mdelay(500);
- return 0;
+}
+static const struct wdt_ops bcm2835_wdt_ops = {
- .reset = bcm2835_wdt_reset,
- .start = bcm2835_wdt_start,
- .stop = bcm2835_wdt_stop,
- .expire_now = bcm2835_wdt_expire_now,
+};
+static const struct udevice_id bcm2835_wdt_ids[] = {
- { .compatible = "brcm,bcm2835-pm" },
- { .compatible = "brcm,bcm2835-pm-wdt" },
- { /* sentinel */ }
+};
+static int bcm2835_wdt_probe(struct udevice *dev) +{
- struct bcm2835_wdt_priv *priv = dev_get_priv(dev);
- priv->base = dev_remap_addr(dev);
- if (!priv->base)
return -EINVAL;
- priv->timeout_ticks = PM_WDOG_MAX_TICKS;
- bcm2835_wdt_stop(dev);
- return 0;
+}
+U_BOOT_DRIVER(bcm2835_wdt) = {
- .name = "bcm2835_wdt",
- .id = UCLASS_WDT,
- .of_match = bcm2835_wdt_ids,
- .probe = bcm2835_wdt_probe,
- .priv_auto = sizeof(struct bcm2835_wdt_priv),
- .ops = &bcm2835_wdt_ops,
+};
Viele Grüße, Stefan Roese

Hi Stefan,
+/* The hardware supports a maximum timeout value of 0xfffff ticks
- (just below 16 seconds).
- U-boot users specify the timeout as a number of milliseconds
- by using variable CONFIG_WATCHDOG_TIMEOUT_MSECS.
- The maximum value should be 15999 ms in our case.
- However, u-boot internally converts this config value to seconds,
- thus specifying 15999 actually means 15000 ms (0xf0000 ticks).
means 15999 ms?
No, 15000 ms. Actually I observed that whatever value 15xxx I set in .config, the driver code is called with value 15000. I suppose this is due to the code in wdt-uclass.c which converts the config value to an integer number of seconds. So the limit I wrote in the code (15000 ms, 0xf0000 ticks) is actually "the max value the user can specify in configuration", whereas the hardware limit is a little higher (0xfffff ticks).
However, thinking twice it is probably not a good idea to deal with the behavior of higher layers here. So I will change back the driver limit to 0xfffff ticks and replace this long explanation with a minimal comment.
+ writel(PM_PASSWORD | timeout_ticks, base + PM_WDOG); + cur = readl(base + PM_RSTC); + writel(PM_PASSWORD | (cur & PM_RSTC_WRCFG_CLR) | + PM_RSTC_WRCFG_FULL_RESET, base + PM_RSTC);
This does not seem to be aligned with the "(" from the line above.
BTW: Please use scripts/checkpatch.pl to see, if the patch has no more issues.
I did run it, but apparently it did not catch this one. I will fix it.
+ if (priv->timeout_ticks > PM_WDOG_MAX_TICKS) { + printf("WARNING: bcm2835_wdt cannot handle large timeout values.\n"); + printf(" Setting the max value of 15000 ms instead.\n"); + printf(" Set CONFIG_WATCHDOG_TIMEOUT_MSECS=15000 at most " + "to avoid this warning.\n");
This seems way too long for a warning from this driver. Please shorten it to one line. The comment above already covers this limitation AFAICT.
OK.
Thanks, Etienne
participants (3)
-
ETIENNE DUBLE
-
Etienne Dublé
-
Stefan Roese