[PATCH 0/4] Apple M1 watchdog timer

[PATCH 0/4] Apple M1 watchdog timer

[Mark Kettenis]

This series adds a driver for the watchdog timer found on Apple's
M1 SoC and replaces the reset_cpu() function in the board file
with sysreset.  The driver is based on the Linux binding proposed
in [1].

[1] https://patchwork.ozlabs.org/project/devicetree-bindings/patch/20211113094732.73889-1-sven@svenpeter.dev/

Mark Kettenis (3):
  watchdog: Add a driver for the Apple watchdog
  arm: dts: apple: Add watchdog timer node
  arm: apple: Use watchdog timer for system reset

Tom Rini (1):
  configs: Resync with savedefconfig

 arch/arm/Kconfig                              |   3 +
 arch/arm/dts/t8103.dtsi                       |  13 +-
 arch/arm/mach-apple/board.c                   |  24 ----
 configs/T1024RDB_NAND_defconfig               |   3 +-
 configs/T1024RDB_SDCARD_defconfig             |   2 +-
 configs/T1024RDB_SPIFLASH_defconfig           |   2 +-
 configs/T1024RDB_defconfig                    |   2 +-
 configs/T1042D4RDB_NAND_defconfig             |   2 +-
 configs/T1042D4RDB_SDCARD_defconfig           |   1 -
 configs/T1042D4RDB_SPIFLASH_defconfig         |   2 +-
 configs/T1042D4RDB_defconfig                  |   2 +-
 configs/T2080RDB_defconfig                    |   1 -
 configs/kmcent2_defconfig                     |   2 +-
 configs/kmtegr1_defconfig                     |   2 +-
 configs/kontron_sl28_defconfig                |   1 -
 configs/ls1021aiot_qspi_defconfig             |   1 -
 configs/ls1021aiot_sdcard_defconfig           |   2 +-
 configs/ls1021aqds_ddr4_nor_defconfig         |   2 +-
 configs/ls1021aqds_ddr4_nor_lpuart_defconfig  |   2 +-
 configs/ls1021aqds_nand_defconfig             |   1 -
 configs/ls1021aqds_nor_SECURE_BOOT_defconfig  |   2 +-
 configs/ls1021aqds_nor_defconfig              |   2 +-
 configs/ls1021aqds_nor_lpuart_defconfig       |   2 +-
 configs/ls1021aqds_qspi_defconfig             |   1 -
 configs/ls1021aqds_sdcard_ifc_defconfig       |   1 -
 configs/ls1021aqds_sdcard_qspi_defconfig      |   1 -
 configs/ls1021atwr_nor_SECURE_BOOT_defconfig  |   2 +-
 configs/ls1021atwr_nor_defconfig              |   2 +-
 configs/ls1021atwr_nor_lpuart_defconfig       |   2 +-
 configs/ls1021atwr_qspi_defconfig             |   1 -
 ...s1021atwr_sdcard_ifc_SECURE_BOOT_defconfig |   2 +-
 configs/ls1021atwr_sdcard_ifc_defconfig       |   1 -
 configs/ls1021atwr_sdcard_qspi_defconfig      |   1 -
 configs/ls1028aqds_tfa_SECURE_BOOT_defconfig  |   1 -
 configs/ls1028aqds_tfa_defconfig              |   1 -
 configs/ls1028ardb_tfa_SECURE_BOOT_defconfig  |   1 -
 configs/ls1028ardb_tfa_defconfig              |   1 -
 configs/ls1043aqds_defconfig                  |   2 +-
 configs/ls1043aqds_lpuart_defconfig           |   2 +-
 configs/ls1043aqds_nor_ddr3_defconfig         |   2 +-
 configs/ls1043aqds_sdcard_ifc_defconfig       |   2 +-
 configs/ls1043aqds_sdcard_qspi_defconfig      |   2 +-
 configs/ls1043aqds_tfa_SECURE_BOOT_defconfig  |   2 +-
 configs/ls1043aqds_tfa_defconfig              |   2 +-
 configs/ls1043ardb_SECURE_BOOT_defconfig      |   2 +-
 configs/ls1043ardb_defconfig                  |   2 +-
 .../ls1043ardb_sdcard_SECURE_BOOT_defconfig   |   2 +-
 configs/ls1043ardb_sdcard_defconfig           |   2 +-
 configs/ls1043ardb_tfa_SECURE_BOOT_defconfig  |   2 +-
 configs/ls1043ardb_tfa_defconfig              |   2 +-
 .../ls1046ardb_sdcard_SECURE_BOOT_defconfig   |   1 -
 configs/pg_wcom_expu1_defconfig               |   2 +-
 configs/pg_wcom_seli8_defconfig               |   2 +-
 configs/stm32mp15_dhcom_basic_defconfig       |   2 +-
 configs/stm32mp15_dhcor_basic_defconfig       |   2 +-
 configs/vinco_defconfig                       |   2 +-
 drivers/watchdog/Kconfig                      |   9 ++
 drivers/watchdog/Makefile                     |   1 +
 drivers/watchdog/apple_wdt.c                  | 111 ++++++++++++++++++
 scripts/config_whitelist.txt                  |   1 -
 60 files changed, 169 insertions(+), 84 deletions(-)
 create mode 100644 drivers/watchdog/apple_wdt.c

-- 
2.33.1

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

[Mark Kettenis]

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);
+
+	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,
+};
-- 
2.33.1

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

[Sven Peter]

On Sun, Nov 14, 2021, at 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>


For the code that drives the HW:

Reviewed-by: Sven Peter <sven@svenpeter.dev>

(The rest looks good as well but I'm not familiar with uboot WDT API)


Sven

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

[Simon Glass]

On Sun, 14 Nov 2021 at 04:19, Mark Kettenis <kettenis@openbsd.org> 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

Reviewed-by: Simon Glass <sjg@chromium.org>
Tested-on: Apple M1 Macbook
Tested-by: Simon Glass <sjg@chromium.org>

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

[Stefan Roese]

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

-- 
DENX Software Engineering GmbH,      Managing Director: Wolfgang Denk
HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany
Phone: (+49)-8142-66989-51 Fax: (+49)-8142-66989-80 Email: sr@denx.de

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

[Mark Kettenis]

> Date: Tue, 11 Jan 2022 09:57:12 +0100
> From: Stefan Roese <sr@denx.de>
> 
> 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?

Actually, the current version of the Linux driver uses 50ms, based on
an observation that it may take up to 25ms to actually reset the SoC.
So I've gone with that and added a comment.

> 
> 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
> 
> -- 
> DENX Software Engineering GmbH,      Managing Director: Wolfgang Denk
> HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany
> Phone: (+49)-8142-66989-51 Fax: (+49)-8142-66989-80 Email: sr@denx.de
> 

[PATCH 2/3] arm: dts: apple: Add watchdog timer node

[Mark Kettenis]

Add a node for the watchdog timer based on the proposed Linux
device tree bindings.

Signed-off-by: Sven Peter <sven@svenpeter.dev>
Signed-off-by: Mark Kettenis <kettenis@openbsd.org>
---
 arch/arm/dts/t8103.dtsi | 13 ++++++++-----
 1 file changed, 8 insertions(+), 5 deletions(-)

diff --git a/arch/arm/dts/t8103.dtsi b/arch/arm/dts/t8103.dtsi
index 7d9cb272f9..03d8436fec 100644
--- a/arch/arm/dts/t8103.dtsi
+++ b/arch/arm/dts/t8103.dtsi
@@ -328,6 +328,14 @@
 				     <AIC_IRQ 336 IRQ_TYPE_LEVEL_HIGH>;
 		};
 
+		wdt: watchdog@23d2b0000 {
+			compatible = "apple,t8103-wdt", "apple,wdt";
+			reg = <0x2 0x3d2b0000 0x0 0x4000>;
+			clocks = <&clkref>;
+			interrupt-parent = <&aic>;
+			interrupts = <AIC_IRQ 338 IRQ_TYPE_LEVEL_HIGH>;
+		};
+
 		pinctrl_smc: pinctrl@23e820000 {
 			compatible = "apple,t8103-pinctrl", "apple,pinctrl";
 			reg = <0x2 0x3e820000 0x0 0x4000>;
@@ -529,11 +537,6 @@
 			status = "disabled";
 		};
 
-		reboot@23d2b0000 {
-			compatible = "apple,reboot-v0";
-			reg = <0x2 0x3d2b0000 0x0 0x4000>;
-		};
-
 		spi@23510c000 {
 			compatible = "apple,t8103-spi", "apple,spi";
 			reg = <0x2 0x3510c000 0x0 0x4000>;
-- 
2.33.1

Re: [PATCH 2/3] arm: dts: apple: Add watchdog timer node

[Simon Glass]

On Sun, 14 Nov 2021 at 04:19, Mark Kettenis <kettenis@openbsd.org> wrote:
>
> Add a node for the watchdog timer based on the proposed Linux
> device tree bindings.
>
> Signed-off-by: Sven Peter <sven@svenpeter.dev>
> Signed-off-by: Mark Kettenis <kettenis@openbsd.org>
> ---
>  arch/arm/dts/t8103.dtsi | 13 ++++++++-----
>  1 file changed, 8 insertions(+), 5 deletions(-)
>

Reviewed-by: Simon Glass <sjg@chromium.org>
Tested-on: Apple M1 Macbook
Tested-by: Simon Glass <sjg@chromium.org>

Re: [PATCH 2/3] arm: dts: apple: Add watchdog timer node

[Stefan Roese]

On 11/14/21 12:19, Mark Kettenis wrote:
> Add a node for the watchdog timer based on the proposed Linux
> device tree bindings.
> 
> Signed-off-by: Sven Peter <sven@svenpeter.dev>
> Signed-off-by: Mark Kettenis <kettenis@openbsd.org>
> ---
>   arch/arm/dts/t8103.dtsi | 13 ++++++++-----
>   1 file changed, 8 insertions(+), 5 deletions(-)
> 
> diff --git a/arch/arm/dts/t8103.dtsi b/arch/arm/dts/t8103.dtsi
> index 7d9cb272f9..03d8436fec 100644
> --- a/arch/arm/dts/t8103.dtsi
> +++ b/arch/arm/dts/t8103.dtsi
> @@ -328,6 +328,14 @@
>   				     <AIC_IRQ 336 IRQ_TYPE_LEVEL_HIGH>;
>   		};
>   
> +		wdt: watchdog@23d2b0000 {
> +			compatible = "apple,t8103-wdt", "apple,wdt";
> +			reg = <0x2 0x3d2b0000 0x0 0x4000>;
> +			clocks = <&clkref>;
> +			interrupt-parent = <&aic>;
> +			interrupts = <AIC_IRQ 338 IRQ_TYPE_LEVEL_HIGH>;
> +		};
> +
>   		pinctrl_smc: pinctrl@23e820000 {
>   			compatible = "apple,t8103-pinctrl", "apple,pinctrl";
>   			reg = <0x2 0x3e820000 0x0 0x4000>;
> @@ -529,11 +537,6 @@
>   			status = "disabled";
>   		};
>   
> -		reboot@23d2b0000 {
> -			compatible = "apple,reboot-v0";
> -			reg = <0x2 0x3d2b0000 0x0 0x4000>;
> -		};
> -

It would be helpful IMHO, if you could add a small comment in the commit
text, why you are removing this DT node with this patch.

Other than this:

Reviewed-by: Stefan Roese <sr@denx.de>

Thanks,
Stefan

[PATCH 3/3] arm: apple: Use watchdog timer for system reset

[Mark Kettenis]

Rely on the new watchdog timer driver and the sysreset uclass to
reset the system.  This gets rid of hard-coded addresses and
should work on systems based on the new M1 Pro and M1 Max SoCs
as well.

Signed-off-by: Mark Kettenis <kettenis@openbsd.org>
---
 arch/arm/Kconfig            |  3 +++
 arch/arm/mach-apple/board.c | 24 ------------------------
 2 files changed, 3 insertions(+), 24 deletions(-)

diff --git a/arch/arm/Kconfig b/arch/arm/Kconfig
index f7f03837fe..4c58af45da 100644
--- a/arch/arm/Kconfig
+++ b/arch/arm/Kconfig
@@ -936,6 +936,9 @@ config ARCH_APPLE
 	select OF_CONTROL
 	select OF_BOARD
 	select POSITION_INDEPENDENT
+	select SYSRESET
+	select SYSRESET_WATCHDOG
+	select SYSRESET_WATCHDOG_AUTO
 	select USB
 	imply CMD_DM
 	imply CMD_GPT
diff --git a/arch/arm/mach-apple/board.c b/arch/arm/mach-apple/board.c
index 0bfbc473ec..b7e8d212f1 100644
--- a/arch/arm/mach-apple/board.c
+++ b/arch/arm/mach-apple/board.c
@@ -119,30 +119,6 @@ int dram_init_banksize(void)
 	return fdtdec_setup_memory_banksize();
 }
 
-#define APPLE_WDT_BASE		0x23d2b0000ULL
-
-#define APPLE_WDT_SYS_CTL_ENABLE	BIT(2)
-
-typedef struct apple_wdt {
-	u32	reserved0[3];
-	u32	chip_ctl;
-	u32	sys_tmr;
-	u32	sys_cmp;
-	u32	reserved1;
-	u32	sys_ctl;
-} apple_wdt_t;
-
-void reset_cpu(void)
-{
-	apple_wdt_t *wdt = (apple_wdt_t *)APPLE_WDT_BASE;
-
-	writel(0, &wdt->sys_cmp);
-	writel(APPLE_WDT_SYS_CTL_ENABLE, &wdt->sys_ctl);
-
-	while(1)
-		wfi();
-}
-
 extern long fw_dtb_pointer;
 
 void *board_fdt_blob_setup(int *err)
-- 
2.33.1

Re: [PATCH 3/3] arm: apple: Use watchdog timer for system reset

[Simon Glass]

On Sun, 14 Nov 2021 at 04:19, Mark Kettenis <kettenis@openbsd.org> wrote:
>
> Rely on the new watchdog timer driver and the sysreset uclass to
> reset the system.  This gets rid of hard-coded addresses and
> should work on systems based on the new M1 Pro and M1 Max SoCs
> as well.
>
> Signed-off-by: Mark Kettenis <kettenis@openbsd.org>
> ---
>  arch/arm/Kconfig            |  3 +++
>  arch/arm/mach-apple/board.c | 24 ------------------------
>  2 files changed, 3 insertions(+), 24 deletions(-)

Reviewed-by: Simon Glass <sjg@chromium.org>
Tested-on: Apple M1 Macbook
Tested-by: Simon Glass <sjg@chromium.org>

Re: [PATCH 3/3] arm: apple: Use watchdog timer for system reset

[Stefan Roese]

On 11/14/21 12:19, Mark Kettenis wrote:
> Rely on the new watchdog timer driver and the sysreset uclass to
> reset the system.  This gets rid of hard-coded addresses and
> should work on systems based on the new M1 Pro and M1 Max SoCs
> as well.
> 
> Signed-off-by: Mark Kettenis <kettenis@openbsd.org>

Reviewed-by: Stefan Roese <sr@denx.de>

Thanks,
Stefan

> ---
>   arch/arm/Kconfig            |  3 +++
>   arch/arm/mach-apple/board.c | 24 ------------------------
>   2 files changed, 3 insertions(+), 24 deletions(-)
> 
> diff --git a/arch/arm/Kconfig b/arch/arm/Kconfig
> index f7f03837fe..4c58af45da 100644
> --- a/arch/arm/Kconfig
> +++ b/arch/arm/Kconfig
> @@ -936,6 +936,9 @@ config ARCH_APPLE
>   	select OF_CONTROL
>   	select OF_BOARD
>   	select POSITION_INDEPENDENT
> +	select SYSRESET
> +	select SYSRESET_WATCHDOG
> +	select SYSRESET_WATCHDOG_AUTO
>   	select USB
>   	imply CMD_DM
>   	imply CMD_GPT
> diff --git a/arch/arm/mach-apple/board.c b/arch/arm/mach-apple/board.c
> index 0bfbc473ec..b7e8d212f1 100644
> --- a/arch/arm/mach-apple/board.c
> +++ b/arch/arm/mach-apple/board.c
> @@ -119,30 +119,6 @@ int dram_init_banksize(void)
>   	return fdtdec_setup_memory_banksize();
>   }
>   
> -#define APPLE_WDT_BASE		0x23d2b0000ULL
> -
> -#define APPLE_WDT_SYS_CTL_ENABLE	BIT(2)
> -
> -typedef struct apple_wdt {
> -	u32	reserved0[3];
> -	u32	chip_ctl;
> -	u32	sys_tmr;
> -	u32	sys_cmp;
> -	u32	reserved1;
> -	u32	sys_ctl;
> -} apple_wdt_t;
> -
> -void reset_cpu(void)
> -{
> -	apple_wdt_t *wdt = (apple_wdt_t *)APPLE_WDT_BASE;
> -
> -	writel(0, &wdt->sys_cmp);
> -	writel(APPLE_WDT_SYS_CTL_ENABLE, &wdt->sys_ctl);
> -
> -	while(1)
> -		wfi();
> -}
> -
>   extern long fw_dtb_pointer;
>   
>   void *board_fdt_blob_setup(int *err)
> 

Viele Grüße,
Stefan Roese

-- 
DENX Software Engineering GmbH,      Managing Director: Wolfgang Denk
HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany
Phone: (+49)-8142-66989-51 Fax: (+49)-8142-66989-80 Email: sr@denx.de

Re: [PATCH 0/4] Apple M1 watchdog timer

[Mark Kettenis]

> From: Mark Kettenis <kettenis@openbsd.org>
> Date: Sun, 14 Nov 2021 12:19:39 +0100
> 
> This series adds a driver for the watchdog timer found on Apple's
> M1 SoC and replaces the reset_cpu() function in the board file
> with sysreset.  The driver is based on the Linux binding proposed
> in [1].
> 
> [1] https://patchwork.ozlabs.org/project/devicetree-bindings/patch/20211113094732.73889-1-sven@svenpeter.dev/
> 
> Mark Kettenis (3):
>   watchdog: Add a driver for the Apple watchdog
>   arm: dts: apple: Add watchdog timer node
>   arm: apple: Use watchdog timer for system reset

Hi Stefan,

This series is delegated to you in patchwork[1].  Not sure why though.
But if watchdogs are indeed your thing, can you have a look now that
2022.01 is out of the door?

Thanks,

Mark

[1] https://patchwork.ozlabs.org/project/uboot/list/?series=271919

Re: [PATCH 0/4] Apple M1 watchdog timer

[Stefan Roese]

Hi Marc,

On 1/10/22 21:25, Mark Kettenis wrote:
>> From: Mark Kettenis <kettenis@openbsd.org>
>> Date: Sun, 14 Nov 2021 12:19:39 +0100
>>
>> This series adds a driver for the watchdog timer found on Apple's
>> M1 SoC and replaces the reset_cpu() function in the board file
>> with sysreset.  The driver is based on the Linux binding proposed
>> in [1].
>>
>> [1] https://patchwork.ozlabs.org/project/devicetree-bindings/patch/20211113094732.73889-1-sven@svenpeter.dev/
>>
>> Mark Kettenis (3):
>>    watchdog: Add a driver for the Apple watchdog
>>    arm: dts: apple: Add watchdog timer node
>>    arm: apple: Use watchdog timer for system reset
> 
> Hi Stefan,
> 
> This series is delegated to you in patchwork[1].  Not sure why though.

I'm the "unofficial" watchdog custodian since a few years. Perhaps its
time to make this a bit more "official".

> But if watchdogs are indeed your thing, can you have a look now that
> 2022.01 is out of the door?

Yes, its on my list - will get to it shortly. Sorry for the delay.

Thanks,
Stefan