Re: [PATCH 1/3] watchdog: Add a driver for the Apple watchdog

On 11/14/21 12:19, Mark Kettenis wrote:

This driver supports the watchdog timer found on Apple's M1 SoC. On systems that use these SoC, the watchdog timer is the primary way to reboot the system.

Signed-off-by: Mark Kettenis kettenis@openbsd.org

MAINTAINERS | 1 + drivers/watchdog/Kconfig | 9 +++ drivers/watchdog/Makefile | 1 + drivers/watchdog/apple_wdt.c | 111 +++++++++++++++++++++++++++++++++++ 4 files changed, 122 insertions(+) create mode 100644 drivers/watchdog/apple_wdt.c

diff --git a/MAINTAINERS b/MAINTAINERS index 00ff572d4d..fe56ffed60 100644 --- a/MAINTAINERS +++ b/MAINTAINERS @@ -115,6 +115,7 @@ F: arch/arm/include/asm/arch-m1/ F: arch/arm/mach-apple/ F: configs/apple_m1_defconfig F: drivers/iommu/apple_dart.c +F: drivers/watchdog/apple_wdt.c F: include/configs/apple.h

ARM diff --git a/drivers/watchdog/Kconfig b/drivers/watchdog/Kconfig index 1177f17fd8..cabac29053 100644 --- a/drivers/watchdog/Kconfig +++ b/drivers/watchdog/Kconfig @@ -81,6 +81,15 @@ config WDT What exactly happens when the timer expires is up to a particular device/driver.

+config WDT_APPLE

• bool "Apple watchdog timer support"
• depends on WDT
• default y if ARCH_APPLE
• help

•  Enable support for the watchdog timer on Apple SoCs.
  

•  The watchdog will perform a full SoC reset resulting in a
  

•  reboot of the entire system.
  

• config WDT_ARMADA_37XX bool "Marvell Armada 37xx watchdog timer support" depends on WDT && ARMADA_3700

diff --git a/drivers/watchdog/Makefile b/drivers/watchdog/Makefile index fa7ce583ce..6d2b3822c0 100644 --- a/drivers/watchdog/Makefile +++ b/drivers/watchdog/Makefile @@ -17,6 +17,7 @@ obj-$(CONFIG_DESIGNWARE_WATCHDOG) += designware_wdt.o obj-$(CONFIG_ULP_WATCHDOG) += ulp_wdog.o obj-$(CONFIG_$(SPL_TPL_)WDT) += wdt-uclass.o obj-$(CONFIG_WDT_SANDBOX) += sandbox_wdt.o +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 diff --git a/drivers/watchdog/apple_wdt.c b/drivers/watchdog/apple_wdt.c new file mode 100644 index 0000000000..08eefcdaf9 --- /dev/null +++ b/drivers/watchdog/apple_wdt.c @@ -0,0 +1,111 @@ +// SPDX-License-Identifier: GPL-2.0+ +/*

• • Copyright (C) 2021 Mark Kettenis kettenis@openbsd.org
• */

+#include <clk.h> +#include <dm.h> +#include <wdt.h> +#include <asm/io.h> +#include <linux/delay.h>

+#define APPLE_WDT_CUR_TIME 0x10 +#define APPLE_WDT_BARK_TIME 0x14 +#define APPLE_WDT_CTRL 0x1c +#define APPLE_WDT_CTRL_RESET_EN BIT(2)

+struct apple_wdt_priv {

• void *base;
• ulong clk_rate;

+};

+static int apple_wdt_reset(struct udevice *dev) +{

• struct apple_wdt_priv *priv = dev_get_priv(dev);
• writel(0, priv->base + APPLE_WDT_CUR_TIME);
• return 0;

+}

+static int apple_wdt_start(struct udevice *dev, u64 timeout_ms, ulong flags) +{

• struct apple_wdt_priv *priv = dev_get_priv(dev);
• u64 timeout;
• timeout = (timeout_ms * priv->clk_rate) / 1000;
• if (timeout > U32_MAX)

• return -EINVAL;
  

• writel(0, priv->base + APPLE_WDT_CUR_TIME);
• writel(timeout, priv->base + APPLE_WDT_BARK_TIME);
• writel(APPLE_WDT_CTRL_RESET_EN, priv->base + APPLE_WDT_CTRL);
• return 0;

+}

+static int apple_wdt_stop(struct udevice *dev) +{

• struct apple_wdt_priv *priv = dev_get_priv(dev);
• writel(0, priv->base + APPLE_WDT_CTRL);
• return 0;

+}

+static int apple_wdt_expire_now(struct udevice *dev, ulong flags) +{

• int ret;
• ret = apple_wdt_start(dev, 0, flags);
• if (ret)

• return ret;
  

• mdelay(150);

Could you perhaps add a short comment here, why you chose 150ms?

Other than this:

Reviewed-by: Stefan Roese sr@denx.de

Thanks, Stefan

• return 0;

+}

+static const struct wdt_ops apple_wdt_ops = {

• .reset = apple_wdt_reset,
• .start = apple_wdt_start,
• .stop = apple_wdt_stop,
• .expire_now = apple_wdt_expire_now,

+};

+static const struct udevice_id apple_wdt_ids[] = {

• { .compatible = "apple,wdt" },
• { /* sentinel */ }

+};

+static int apple_wdt_probe(struct udevice *dev) +{

• struct apple_wdt_priv *priv = dev_get_priv(dev);
• struct clk clk;
• int ret;
• priv->base = dev_read_addr_ptr(dev);
• if (!priv->base)

• return -EINVAL;
  

• ret = clk_get_by_index(dev, 0, &clk);
• if (ret)

• return ret;
  

• ret = clk_enable(&clk);
• if (ret)

• return ret;
  

• priv->clk_rate = clk_get_rate(&clk);
• return 0;

+}

+U_BOOT_DRIVER(apple_wdt) = {

• .name = "apple_wdt",
• .id = UCLASS_WDT,
• .of_match = apple_wdt_ids,
• .priv_auto = sizeof(struct apple_wdt_priv),
• .probe = apple_wdt_probe,
• .ops = &apple_wdt_ops,

+};

Viele Grüße, Stefan Roese