Hi Etienne,
On 1/24/23 14:20, Etienne Dublé 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
drivers/watchdog/Kconfig | 9 +++ drivers/watchdog/Makefile | 1 + drivers/watchdog/bcm2835_wdt.c | 135 +++++++++++++++++++++++++++++++++ 3 files changed, 145 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"
Your patch seems to have whitespace problems. Please make sure to send it e.g. via git send-email.
More comments below...
+ 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..48535c003f --- /dev/null +++ b/drivers/watchdog/bcm2835_wdt.c @@ -0,0 +1,135 @@ +// 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; +};
+static u64 timeout_ticks = PM_WDOG_MAX_TICKS;
Is this static variable really needed? Why not add this variable (if needed) into the priv struct above instead?
+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) +{ + timeout_ticks = MS_TO_WDOG_TICKS(timeout_ms);
Again, there should be no need for this static variable. Please move it to the priv struct instead.
Thanks, Stefan
+ if (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"); + timeout_ticks = PM_WDOG_MAX_TICKS; + }
+ return bcm2835_wdt_start_ticks(dev, timeout_ticks, flags); +}
+static int bcm2835_wdt_reset(struct udevice *dev) +{ + /* restart the timer with the static variable 'timeout_ticks' + * saved from the last bcm2835_wdt_start() call. + */ + return bcm2835_wdt_start_ticks(dev, 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;
+ 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