Re: [PATCH] versal: watchdog: Add support for Xilinx window watchdog

On 11.03.20 11:48, Michal Simek wrote:

From: Ashok Reddy Soma ashok.reddy.soma@xilinx.com

Add support for Xilinx window watchdog, which can be found on Versal platforms.

Signed-off-by: Ashok Reddy Soma ashok.reddy.soma@xilinx.com Signed-off-by: Michal Simek michal.simek@xilinx.com

---

MAINTAINERS | 1 + drivers/watchdog/Kconfig | 8 ++ drivers/watchdog/Makefile | 1 + drivers/watchdog/xilinx_wwdt.c | 185 +++++++++++++++++++++++++++++++++ 4 files changed, 195 insertions(+) create mode 100644 drivers/watchdog/xilinx_wwdt.c

diff --git a/MAINTAINERS b/MAINTAINERS index 37ff21a037b4..7cb1bc0957a3 100644 --- a/MAINTAINERS +++ b/MAINTAINERS @@ -433,6 +433,7 @@ M: Michal Simek michal.simek@xilinx.com S: Maintained T: git https://gitlab.denx.de/u-boot/custodians/u-boot-microblaze.git F: arch/arm/mach-versal/ +F: drivers/watchdog/xilinx_wwdt.c N: (?<!uni)versal

ARM VERSATILE EXPRESS DRIVERS diff --git a/drivers/watchdog/Kconfig b/drivers/watchdog/Kconfig index d24c1e48353f..b3911cf346ed 100644 --- a/drivers/watchdog/Kconfig +++ b/drivers/watchdog/Kconfig @@ -185,6 +185,14 @@ config XILINX_TB_WATCHDOG Select this to enable Xilinx Axi watchdog timer, which can be found on some Xilinx Microblaze Platforms.

+config WWDT_XILINX

• bool "Xilinx window watchdog timer support"
• depends on WDT && ARCH_VERSAL
• imply WATCHDOG
• help

•  Select this to enable Xilinx window watchdog timer, which can be found on
  

•  Xilinx Versal Platforms.
  

• config WDT_TANGIER bool "Intel Tangier watchdog timer support" depends on WDT && INTEL_MID

diff --git a/drivers/watchdog/Makefile b/drivers/watchdog/Makefile index 87f92a43b14e..dc25560dbadf 100644 --- a/drivers/watchdog/Makefile +++ b/drivers/watchdog/Makefile @@ -30,3 +30,4 @@ obj-$(CONFIG_WDT_OMAP3) += omap_wdt.o obj-$(CONFIG_WDT_SP805) += sp805_wdt.o obj-$(CONFIG_WDT_STM32MP) += stm32mp_wdt.o obj-$(CONFIG_WDT_TANGIER) += tangier_wdt.o +obj-$(CONFIG_WWDT_XILINX) += xilinx_wwdt.o diff --git a/drivers/watchdog/xilinx_wwdt.c b/drivers/watchdog/xilinx_wwdt.c new file mode 100644 index 000000000000..34018e2b2b34 --- /dev/null +++ b/drivers/watchdog/xilinx_wwdt.c @@ -0,0 +1,185 @@ +// SPDX-License-Identifier: GPL-2.0 +/*

• • Xilinx window watchdog timer driver.
• • Author(s): Michal Simek michal.simek@xilinx.com

• • Ashok Reddy Soma <ashokred@xilinx.com>
    

• • Copyright (c) 2020, Xilinx Inc.
• */

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

+/* Refresh Register Masks */ +#define XWT_WWREF_GWRR_MASK BIT(0) /* Refresh and start new period */

+/* Generic Control/Status Register Masks */ +#define XWT_WWCSR_GWEN_MASK BIT(0) /* Enable Bit */

+struct wwdt_regs {

• u32 reserved0[1024];
• u32 refresh; /* Refresh Register [0x1000] */
• u32 reserved1[1023];
• u32 csr; /* Control/Status Register [0x2000] */
• u32 reserved2;
• u32 offset; /* Offset Register [0x2008] */
• u32 reserved3;
• u32 cmp0; /* Compare Value Register0 [0x2010] */
• u32 cmp1; /* Compare Value Register1 [0x2014] */
• u32 reserved4[1006];
• u32 warmrst; /* Warm Reset Register [0x2FD0] */

+};

My understanding is, that we moved to using defines instead of structs for register definitions. So if you need to send a v2, then please consider using #defines here.

+struct xlnx_wwdt_priv {

• bool enable_once;
• struct wwdt_regs *regs;
• struct clk clk;

+};

+struct xlnx_wwdt_platdata {

• bool enable_once;
• phys_addr_t iobase;

+};

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

• struct xlnx_wwdt_priv *wdt = dev_get_priv(dev);
• dev_dbg(dev, "%s ", __func__);
• writel(XWT_WWREF_GWRR_MASK, &wdt->regs->refresh);
• return 0;

+}

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

• u32 csr;
• struct xlnx_wwdt_priv *wdt = dev_get_priv(dev);
• if (wdt->enable_once) {

• dev_warn(dev, "Can't stop Xilinx watchdog.\n");
  

• return -EBUSY;
  

• }
• /* Disable the generic watchdog timer */
• csr = readl(&wdt->regs->csr);
• csr &= ~(XWT_WWCSR_GWEN_MASK);
• writel(csr, &wdt->regs->csr);
• clk_disable(&wdt->clk);
• dev_dbg(dev, "Watchdog disabled!\n");
• return 0;

+}

+static int xlnx_wwdt_start(struct udevice *dev, u64 timeout, ulong flags) +{

• int ret;
• u32 csr;
• u64 count;
• unsigned long clock_f;
• struct xlnx_wwdt_priv *wdt = dev_get_priv(dev);
• dev_dbg(dev, "%s:\n", __func__);
• clock_f = clk_get_rate(&wdt->clk);
• if (IS_ERR_VALUE(clock_f)) {

• dev_err(dev, "failed to get rate\n");
  

• return clock_f;
  

• }
• dev_dbg(dev, "%s: CLK %ld\n", __func__, clock_f);
• /* Calculate timeout count */
• count = timeout * clock_f;
• /* clk_enable will return -ENOSYS when it is not implemented */
• ret = clk_enable(&wdt->clk);
• if (ret && ret != -ENOSYS) {

• dev_err(dev, "failed to enable clock\n");
  

• return ret;
  

• }
• /*

• * Timeout count is half as there are two windows
  

• * first window overflow is ignored (interrupt),
  

• * reset is only generated at second window overflow
  

• */
  

• count = count >> 1;
• /* Disable the generic watchdog timer */
• csr = readl(&wdt->regs->csr);
• csr &= ~(XWT_WWCSR_GWEN_MASK);
• writel(csr, &wdt->regs->csr);
• /* Set compare and offset registers for generic watchdog timeout */
• writel((u32)count, &wdt->regs->cmp0);
• writel((u32)0, &wdt->regs->cmp1);
• writel((u32)count, &wdt->regs->offset);
• /* Enable the generic watchdog timer */
• csr = readl(&wdt->regs->csr);
• csr |= (XWT_WWCSR_GWEN_MASK);
• writel(csr, &wdt->regs->csr);
• return 0;

+}

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

• int ret;
• struct xlnx_wwdt_platdata *platdata = dev_get_platdata(dev);
• struct xlnx_wwdt_priv *wdt = dev_get_priv(dev);
• dev_dbg(dev, "%s: Probing wdt%u\n", __func__, dev->seq);
• wdt->regs = (struct wwdt_regs *)ioremap(platdata->iobase,

• 				sizeof(struct wwdt_regs));
  

• wdt->enable_once = platdata->enable_once;
• ret = clk_get_by_index(dev, 0, &wdt->clk);
• if (ret < 0)

• dev_err(dev, "failed to get clock\n");
  

• return ret;

+}

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

• struct xlnx_wwdt_platdata *platdata = dev_get_platdata(dev);
• platdata->iobase = dev_read_addr(dev);
• platdata->enable_once = dev_read_u32_default(dev,

• 				     "xlnx,wdt-enable-once", 0);
  

• dev_dbg(dev, "wdt-enable-once %d\n", platdata->enable_once);
• return 0;

+}

+static const struct wdt_ops xlnx_wwdt_ops = {

• .start = xlnx_wwdt_start,
• .reset = xlnx_wwdt_reset,
• .stop = xlnx_wwdt_stop,

+};

+static const struct udevice_id xlnx_wwdt_ids[] = {

• { .compatible = "xlnx,versal-wwdt-1.0", },
• {},

+};

+U_BOOT_DRIVER(xlnx_wwdt) = {

• .name = "xlnx_wwdt",
• .id = UCLASS_WDT,
• .of_match = xlnx_wwdt_ids,
• .probe = xlnx_wwdt_probe,
• .priv_auto_alloc_size = sizeof(struct xlnx_wwdt_priv),
• .platdata_auto_alloc_size = sizeof(struct xlnx_wwdt_platdata),
• .ofdata_to_platdata = xlnx_wwdt_ofdata_to_platdata,
• .ops = &xlnx_wwdt_ops,

+};

Looks good otherwise, so:

Reviewed-By: Stefan Roese sr@denx.de

Thanks, Stefan