On 3/9/22 07:13, jimliu2 wrote:

Add watchdog controller driver for NPCM7xx/npcm8xx

Signed-off-by: jimliu2 JJLIU0@nuvoton.com

So what did you change in v2? Please add a changelog next time, as described here:

http://www.denx.de/wiki/view/U-Boot/Patches#Sending_updated_patch_versions

Also there is no need to send a single patch in a patch-series.

Some minor comments below...

---

drivers/watchdog/Kconfig | 6 ++ drivers/watchdog/Makefile | 1 + drivers/watchdog/npcm_wdt.c | 119 ++++++++++++++++++++++++++++++++++++ 3 files changed, 126 insertions(+) create mode 100644 drivers/watchdog/npcm_wdt.c

diff --git a/drivers/watchdog/Kconfig b/drivers/watchdog/Kconfig index f90f0ca02b..15edcf570f 100644 --- a/drivers/watchdog/Kconfig +++ b/drivers/watchdog/Kconfig @@ -196,6 +196,12 @@ config WDT_MTK The watchdog timer is stopped when initialized. It performs full SoC reset.

+config WDT_NPCM

• bool "Nuvoton watchdog timer support"
• depends on WDT && ARCH_NPCM
• help

•  Nuvoton npcm7xx/npcm8xx watchdog.
  

• config WDT_OCTEONTX bool "OcteonTX core watchdog support" depends on WDT && (ARCH_OCTEONTX || ARCH_OCTEONTX2)

diff --git a/drivers/watchdog/Makefile b/drivers/watchdog/Makefile index a35bd559f5..1089cd21f5 100644 --- a/drivers/watchdog/Makefile +++ b/drivers/watchdog/Makefile @@ -31,6 +31,7 @@ obj-$(CONFIG_WDT_MPC8xx) += mpc8xx_wdt.o obj-$(CONFIG_WDT_MT7620) += mt7620_wdt.o obj-$(CONFIG_WDT_MT7621) += mt7621_wdt.o obj-$(CONFIG_WDT_MTK) += mtk_wdt.o +obj-$(CONFIG_WDT_NPCM) += npcm_wdt.o obj-$(CONFIG_WDT_OCTEONTX) += octeontx_wdt.o obj-$(CONFIG_WDT_OMAP3) += omap_wdt.o obj-$(CONFIG_WDT_SBSA) += sbsa_gwdt.o diff --git a/drivers/watchdog/npcm_wdt.c b/drivers/watchdog/npcm_wdt.c new file mode 100644 index 0000000000..6abc146d99 --- /dev/null +++ b/drivers/watchdog/npcm_wdt.c @@ -0,0 +1,119 @@ +// SPDX-License-Identifier: GPL-2.0+ +/*

• • Copyright (c) 2022 Nuvoton Technology, Inc
• */

+#include <common.h>

Including common.h is not deprecated. Please remove in the next version.

+#include <dm.h> +#include <errno.h> +#include <log.h> +#include <wdt.h> +#include <asm/io.h> +#include <linux/delay.h> +#include <linux/err.h>

+#define NPCM_WTCLK (BIT(10) | BIT(11)) /* Clock divider */ +#define NPCM_WTE BIT(7) /* Enable */ +#define NPCM_WTIE BIT(6) /* Enable irq */ +#define NPCM_WTIS (BIT(4) | BIT(5)) /* Interval selection */ +#define NPCM_WTIF BIT(3) /* Interrupt flag*/ +#define NPCM_WTRF BIT(2) /* Reset flag */ +#define NPCM_WTRE BIT(1) /* Reset enable */ +#define NPCM_WTR BIT(0) /* Reset counter */

+struct npcm_wdt_priv {

• void __iomem *regs;

+};

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

• struct npcm_wdt_priv *priv = dev_get_priv(dev);
• u32 time_out, val;
• time_out = (u32)(timeout_ms) / 1000;
• if (time_out < 2)

• val = 0x800;
  

• else if (time_out < 3)

• val = 0x420;
  

• else if (time_out < 6)

• val = 0x810;
  

• else if (time_out < 11)

• val = 0x430;
  

• else if (time_out < 22)

• val = 0x820;
  

• else if (time_out < 44)

• val = 0xC00;
  

• else if (time_out < 87)

• val = 0x830;
  

• else if (time_out < 173)

• val = 0xC10;
  

• else if (time_out < 688)

• val = 0xC20;
  

• else

• val = 0xC30;
  

• val |= NPCM_WTRE | NPCM_WTE | NPCM_WTR | NPCM_WTIE;
• writel(val, priv->regs);
• return 0;

+}

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

• struct npcm_wdt_priv *priv = dev_get_priv(dev);
• writel(0, priv->regs);
• return 0;

+}

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

• struct npcm_wdt_priv *priv = dev_get_priv(dev);
• writel(NPCM_WTR | NPCM_WTRE | NPCM_WTE, priv->regs);
• udelay(1000);
• return 0;

+}

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

• struct npcm_wdt_priv *priv = dev_get_priv(dev);
• priv->regs = dev_read_addr_ptr(dev);
• if (!priv->regs)

• return -EINVAL;
  

• return 0;

+}

+static const struct wdt_ops npcm_wdt_ops = {

• .start = npcm_wdt_start,
• .reset = npcm_wdt_reset,
• .stop = npcm_wdt_stop,

+};

+static const struct udevice_id npcm_wdt_ids[] = {

• { .compatible = "nuvoton,npcm750-wdt" },
• { .compatible = "nuvoton,npcm845-wdt" },
• { }

+};

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

• debug("%s() wdt%u\n", __func__, dev_seq(dev));
• npcm_wdt_stop(dev);
• return 0;

+}

+U_BOOT_DRIVER(npcm_wdt) = {

• .name = "npcm_wdt",
• .id = UCLASS_WDT,
• .of_match = npcm_wdt_ids,
• .probe = npcm_wdt_probe,
• .priv_auto = sizeof(struct npcm_wdt_priv),
• .of_to_plat = npcm_wdt_of_to_plat,
• .ops = &npcm_wdt_ops,

+};

Other than that looks good:

Reviewed-by: Stefan Roese sr@denx.de

Thanks, Stefan