[U-Boot] [PATCH v2 0/3] Xilinx Axi Watchdog driver conversion to driver model

[U-Boot] [PATCH v2 0/3] Xilinx Axi Watchdog driver conversion to driver model

[Shreenidhi Shedi]

[PATCH 1/3] Removed a bunch of WATCHDOG related macros
[PATCH 2/3] Support for watchdog_reset function for Microblaze init
[PATCH 3/3] Xilinx Axi watchdog driver to driver model

Changes in v2:
- Appropriate commit messages for each patch
- Cosmetic changes patch is taken out
- Changes in xilinx_tb_wdt.c based on review comments

Shreenidhi Shedi (3):
  microblaze: Delete Xilinx watchdog related macros
  microblaze: Support for watchdog_reset in Microblaze init
  watchdog: Convert Xilinx Axi watchdog driver to driver model

 .../microblaze-generic/microblaze-generic.c   |  42 ++++++-
 board/xilinx/microblaze-generic/xparameters.h |   4 -
 drivers/watchdog/Kconfig                      |   8 ++
 drivers/watchdog/xilinx_tb_wdt.c              | 105 ++++++++++++++----
 include/configs/microblaze-generic.h          |  10 --
 5 files changed, 127 insertions(+), 42 deletions(-)

--
2.17.1

[U-Boot] [PATCH v2 1/3] microblaze: Delete Xilinx watchdog related macros

[Shreenidhi Shedi]

Tested-by: monstr at monstr.eu
Reviewed-by: monstr at monstr.eu

These macros are not required anymore. These will be taken from
configuration file.

Changes in V1:
 - Removed reduntant macros.

Changes in V2:
 - Added appropriate commit message.

Signed-off-by: Shreenidhi Shedi <yesshedi@gmail.com>
---

Changes in v2:
- Appropriate commit messages for each patch
- Cosmetic changes patch is taken out
- Changes in xilinx_tb_wdt.c based on review comments

 board/xilinx/microblaze-generic/xparameters.h |  4 ----
 include/configs/microblaze-generic.h          | 10 ----------
 2 files changed, 14 deletions(-)

diff --git a/board/xilinx/microblaze-generic/xparameters.h b/board/xilinx/microblaze-generic/xparameters.h
index 51bca40e34..43aad1f8b0 100644
--- a/board/xilinx/microblaze-generic/xparameters.h
+++ b/board/xilinx/microblaze-generic/xparameters.h
@@ -21,7 +21,3 @@
 /* Flash Memory is FLASH_2Mx32 */
 #define XILINX_FLASH_START	0x2c000000
 #define XILINX_FLASH_SIZE	0x00800000
-
-/* Watchdog IP is wxi_timebase_wdt_0 */
-#define XILINX_WATCHDOG_BASEADDR	0x50000000
-#define XILINX_WATCHDOG_IRQ		1
diff --git a/include/configs/microblaze-generic.h b/include/configs/microblaze-generic.h
index 966feeeafd..212debc102 100644
--- a/include/configs/microblaze-generic.h
+++ b/include/configs/microblaze-generic.h
@@ -43,16 +43,6 @@
 # define CONFIG_SYS_GPIO_0_ADDR		XILINX_GPIO_BASEADDR
 #endif
 
-/* watchdog */
-#if defined(XILINX_WATCHDOG_BASEADDR) && defined(XILINX_WATCHDOG_IRQ)
-# define CONFIG_WATCHDOG_BASEADDR	XILINX_WATCHDOG_BASEADDR
-# define CONFIG_WATCHDOG_IRQ		XILINX_WATCHDOG_IRQ
-# ifndef CONFIG_SPL_BUILD
-#  define CONFIG_HW_WATCHDOG
-#  define CONFIG_XILINX_TB_WATCHDOG
-# endif
-#endif
-
 #define CONFIG_SYS_MALLOC_LEN	0xC0000
 
 /* Stack location before relocation */
-- 
2.17.1

[U-Boot] [PATCH v2 2/3] microblaze: Support for watchdog_reset in Microblaze init

[Shreenidhi Shedi]

We should support watchdog reset so that WATCHDOG_RESET will function
properly.

Changes in V1:
 -  Watchdog reset support, inorder to get WATCHDOG_RESET macro working

Changes in V2:
 - Moving wdt.h out of ifdef clause
 - Using CONFIG_WDT & CONFIG_WATCHDOG based on review comments

Signed-off-by: Shreenidhi Shedi <yesshedi@gmail.com>
---

Changes in v2: None

 .../microblaze-generic/microblaze-generic.c   | 42 +++++++++++++++++--
 1 file changed, 38 insertions(+), 4 deletions(-)

diff --git a/board/xilinx/microblaze-generic/microblaze-generic.c b/board/xilinx/microblaze-generic/microblaze-generic.c
index 889695f80e..f05a63d692 100644
--- a/board/xilinx/microblaze-generic/microblaze-generic.c
+++ b/board/xilinx/microblaze-generic/microblaze-generic.c
@@ -15,6 +15,8 @@
 #include <asm/microblaze_intc.h>
 #include <asm/asm.h>
 #include <asm/gpio.h>
+#include <dm/uclass.h>
+#include <wdt.h>
 
 DECLARE_GLOBAL_DATA_PTR;
 
@@ -22,6 +24,10 @@ DECLARE_GLOBAL_DATA_PTR;
 static int reset_pin = -1;
 #endif
 
+#if !defined(CONFIG_SPL_BUILD) && defined(CONFIG_WDT)
+static struct udevice *watchdog_dev;
+#endif /* !CONFIG_SPL_BUILD && CONFIG_WDT */
+
 ulong ram_base;
 
 int dram_init_banksize(void)
@@ -66,10 +72,6 @@ int do_reset(cmd_tbl_t *cmdtp, int flag, int argc, char * const argv[])
 	if (reset_pin != -1)
 		gpio_direction_output(reset_pin, 1);
 #endif
-
-#ifdef CONFIG_XILINX_TB_WATCHDOG
-	hw_watchdog_disable();
-#endif
 #endif
 	puts ("Reseting board\n");
 	__asm__ __volatile__ ("	mts rmsr, r0;" \
@@ -89,9 +91,41 @@ static int gpio_init(void)
 	return 0;
 }
 
+#ifdef CONFIG_WDT
+/* Called by macro WATCHDOG_RESET */
+void watchdog_reset(void)
+{
+#if !defined(CONFIG_SPL_BUILD)
+	ulong now;
+	static ulong next_reset;
+
+	if (!watchdog_dev)
+		return;
+
+	now = timer_get_us();
+
+	/* Do not reset the watchdog too often */
+	if (now > next_reset) {
+		wdt_reset(watchdog_dev);
+		next_reset = now + 1000;
+	}
+#endif /* !CONFIG_SPL_BUILD */
+}
+#endif /* CONFIG_WDT */
+
 int board_late_init(void)
 {
 	gpio_init();
 
+#if !defined(CONFIG_SPL_BUILD) && defined(CONFIG_WDT)
+	watchdog_dev = NULL;
+	if (uclass_get_device(UCLASS_WDT, 0, &watchdog_dev)) {
+		puts("Watchdog: Not found!\n");
+	} else {
+		wdt_start(watchdog_dev, 0, 0);
+		puts("Watchdog: Started\n");
+	}
+#endif /* !CONFIG_SPL_BUILD && CONFIG_WDT */
+
 	return 0;
 }
-- 
2.17.1

[U-Boot] [PATCH v2 3/3] watchdog: Convert Xilinx Axi watchdog driver to driver model

[Shreenidhi Shedi]

Xilinx Axi wdt driver conversion to driver model & Kconfig update
for the same.

Changes in V1:
 - Xilinx Axi wdt driver conversion to DM initial version

Changes in V2:
 - Rectified print message
 - Removed  stop instruction from probe

Signed-off-by: Shreenidhi Shedi <yesshedi@gmail.com>
---

Changes in v2: None

 drivers/watchdog/Kconfig         |   8 +++
 drivers/watchdog/xilinx_tb_wdt.c | 105 ++++++++++++++++++++++++-------
 2 files changed, 89 insertions(+), 24 deletions(-)

diff --git a/drivers/watchdog/Kconfig b/drivers/watchdog/Kconfig
index 148c6a0d68..351d2af8d9 100644
--- a/drivers/watchdog/Kconfig
+++ b/drivers/watchdog/Kconfig
@@ -103,4 +103,12 @@ config WDT_CDNS
 	   Select this to enable Cadence watchdog timer, which can be found on some
 	   Xilinx Microzed Platform.
 
+config XILINX_TB_WATCHDOG
+	bool "Xilinx Axi watchdog timer support"
+	depends on WDT
+	imply WATCHDOG
+	help
+	   Select this to enable Xilinx Axi watchdog timer, which can be found on some
+	   Xilinx Microblaze Platform.
+
 endmenu
diff --git a/drivers/watchdog/xilinx_tb_wdt.c b/drivers/watchdog/xilinx_tb_wdt.c
index 2274123e49..23ddbdfbee 100644
--- a/drivers/watchdog/xilinx_tb_wdt.c
+++ b/drivers/watchdog/xilinx_tb_wdt.c
@@ -1,13 +1,17 @@
 // SPDX-License-Identifier: GPL-2.0+
 /*
+ * Xilinx Axi platforms watchdog timer driver.
+ *
+ * Author(s):	Michal Å imek <michal.simek@xilinx.com>
+ *		Shreenidhi Shedi <yesshedi@gmail.com>
+ *
  * Copyright (c) 2011-2013 Xilinx Inc.
  */
 
 #include <common.h>
-#include <asm/io.h>
-#include <asm/microblaze_intc.h>
-#include <asm/processor.h>
-#include <watchdog.h>
+#include <dm.h>
+#include <wdt.h>
+#include <linux/io.h>
 
 #define XWT_CSR0_WRS_MASK	0x00000008 /* Reset status Mask */
 #define XWT_CSR0_WDS_MASK	0x00000004 /* Timer state Mask */
@@ -20,49 +24,102 @@ struct watchdog_regs {
 	u32 tbr; /* 0x8 */
 };
 
-static struct watchdog_regs *watchdog_base =
-			(struct watchdog_regs *)CONFIG_WATCHDOG_BASEADDR;
+struct xlnx_wdt_priv {
+	bool enable_once;
+	struct watchdog_regs *regs;
+};
 
-void hw_watchdog_reset(void)
+static int xlnx_wdt_reset(struct udevice *dev)
 {
 	u32 reg;
+	struct xlnx_wdt_priv *priv = dev_get_priv(dev);
+
+	debug("%s\n", __func__);
 
 	/* Read the current contents of TCSR0 */
-	reg = readl(&watchdog_base->twcsr0);
+	reg = readl(&priv->regs->twcsr0);
 
 	/* Clear the watchdog WDS bit */
 	if (reg & (XWT_CSR0_EWDT1_MASK | XWT_CSRX_EWDT2_MASK))
-		writel(reg | XWT_CSR0_WDS_MASK, &watchdog_base->twcsr0);
+		writel(reg | XWT_CSR0_WDS_MASK, &priv->regs->twcsr0);
+
+	return 0;
 }
 
-void hw_watchdog_disable(void)
+static int xlnx_wdt_stop(struct udevice *dev)
 {
 	u32 reg;
+	struct xlnx_wdt_priv *priv = dev_get_priv(dev);
+
+	if (priv->enable_once) {
+		puts("Can't stop Xilinx watchdog.\n");
+		return -1;
+	}
 
 	/* Read the current contents of TCSR0 */
-	reg = readl(&watchdog_base->twcsr0);
+	reg = readl(&priv->regs->twcsr0);
 
-	writel(reg & ~XWT_CSR0_EWDT1_MASK, &watchdog_base->twcsr0);
-	writel(~XWT_CSRX_EWDT2_MASK, &watchdog_base->twcsr1);
+	writel(reg & ~XWT_CSR0_EWDT1_MASK, &priv->regs->twcsr0);
+	writel(~XWT_CSRX_EWDT2_MASK, &priv->regs->twcsr1);
 
 	puts("Watchdog disabled!\n");
+
+	return 0;
 }
 
-static void hw_watchdog_isr(void *arg)
+static int xlnx_wdt_start(struct udevice *dev, u64 timeout, ulong flags)
 {
-	hw_watchdog_reset();
+	struct xlnx_wdt_priv *priv = dev_get_priv(dev);
+
+	writel((XWT_CSR0_WRS_MASK | XWT_CSR0_WDS_MASK | XWT_CSR0_EWDT1_MASK),
+	       &priv->regs->twcsr0);
+
+	writel(XWT_CSRX_EWDT2_MASK, &priv->regs->twcsr1);
+
+	return 0;
 }
 
-void hw_watchdog_init(void)
+static int xlnx_wdt_probe(struct udevice *dev)
 {
-	int ret;
+	debug("%s: Probing wdt%u\n", __func__, dev->seq);
 
-	writel((XWT_CSR0_WRS_MASK | XWT_CSR0_WDS_MASK | XWT_CSR0_EWDT1_MASK),
-	       &watchdog_base->twcsr0);
-	writel(XWT_CSRX_EWDT2_MASK, &watchdog_base->twcsr1);
+	return 0;
+}
 
-	ret = install_interrupt_handler(CONFIG_WATCHDOG_IRQ,
-						hw_watchdog_isr, NULL);
-	if (ret)
-		puts("Watchdog IRQ registration failed.");
+static int xlnx_wdt_ofdata_to_platdata(struct udevice *dev)
+{
+	struct xlnx_wdt_priv *priv = dev_get_priv(dev);
+
+	priv->regs = (struct watchdog_regs *)dev_read_addr(dev);
+	if (IS_ERR(priv->regs))
+		return PTR_ERR(priv->regs);
+
+	priv->enable_once = dev_read_u32_default(dev, "xlnx,wdt-enable-once",
+						 0);
+
+	debug("%s: wdt-enable-once %d\n", __func__, priv->enable_once);
+
+	return 0;
 }
+
+static const struct wdt_ops xlnx_wdt_ops = {
+	.start = xlnx_wdt_start,
+	.reset = xlnx_wdt_reset,
+	.stop = xlnx_wdt_stop,
+};
+
+static const struct udevice_id xlnx_wdt_ids[] = {
+	{ .compatible = "xlnx,xps-timebase-wdt-1.00.a", },
+	{ .compatible = "xlnx,xps-timebase-wdt-1.01.a", },
+	{},
+};
+
+U_BOOT_DRIVER(xlnx_wdt) = {
+	.name = "xlnx_wdt",
+	.id = UCLASS_WDT,
+	.of_match = xlnx_wdt_ids,
+	.probe = xlnx_wdt_probe,
+	.priv_auto_alloc_size = sizeof(struct xlnx_wdt_priv),
+	.ofdata_to_platdata = xlnx_wdt_ofdata_to_platdata,
+	.ops = &xlnx_wdt_ops,
+};
-- 
2.17.1

[U-Boot] [PATCH v2 0/3] Xilinx Axi Watchdog driver conversion to driver model

[Michal Simek]

On 14.7.2018 22:35, Shreenidhi Shedi wrote:
> 
> 
> [PATCH 1/3] Removed a bunch of WATCHDOG related macros
> [PATCH 2/3] Support for watchdog_reset function for Microblaze init
> [PATCH 3/3] Xilinx Axi watchdog driver to driver model
> 
> Changes in v2:
> - Appropriate commit messages for each patch
> - Cosmetic changes patch is taken out
> - Changes in xilinx_tb_wdt.c based on review comments
> 
> Shreenidhi Shedi (3):
>   microblaze: Delete Xilinx watchdog related macros
>   microblaze: Support for watchdog_reset in Microblaze init
>   watchdog: Convert Xilinx Axi watchdog driver to driver model
> 
>  .../microblaze-generic/microblaze-generic.c   |  42 ++++++-
>  board/xilinx/microblaze-generic/xparameters.h |   4 -
>  drivers/watchdog/Kconfig                      |   8 ++
>  drivers/watchdog/xilinx_tb_wdt.c              | 105 ++++++++++++++----
>  include/configs/microblaze-generic.h          |  10 --
>  5 files changed, 127 insertions(+), 42 deletions(-)
> 
> --
> 2.17.1
> 

There are several issues. I have taken your series and clean it up a
little bit and send v3. Please take a look at that changes I made.

Thanks,
Michal

[U-Boot] [PATCH v2 3/3] watchdog: Convert Xilinx Axi watchdog driver to driver model

[Simon Glass]

Hi Shreenidhi,

On 14 July 2018 at 14:35, Shreenidhi Shedi <yesshedi@gmail.com> wrote:
> Xilinx Axi wdt driver conversion to driver model & Kconfig update
> for the same.
>
> Changes in V1:
>  - Xilinx Axi wdt driver conversion to DM initial version
>
> Changes in V2:
>  - Rectified print message
>  - Removed  stop instruction from probe
>
> Signed-off-by: Shreenidhi Shedi <yesshedi@gmail.com>
> ---
>
> Changes in v2: None
>
>  drivers/watchdog/Kconfig         |   8 +++
>  drivers/watchdog/xilinx_tb_wdt.c | 105 ++++++++++++++++++++++++-------
>  2 files changed, 89 insertions(+), 24 deletions(-)
>
> diff --git a/drivers/watchdog/Kconfig b/drivers/watchdog/Kconfig
> index 148c6a0d68..351d2af8d9 100644
> --- a/drivers/watchdog/Kconfig
> +++ b/drivers/watchdog/Kconfig
> @@ -103,4 +103,12 @@ config WDT_CDNS
>            Select this to enable Cadence watchdog timer, which can be found on some
>            Xilinx Microzed Platform.
>
> +config XILINX_TB_WATCHDOG
> +       bool "Xilinx Axi watchdog timer support"
> +       depends on WDT
> +       imply WATCHDOG
> +       help
> +          Select this to enable Xilinx Axi watchdog timer, which can be found on some
> +          Xilinx Microblaze Platform.

platforms?

> +
>  endmenu
> diff --git a/drivers/watchdog/xilinx_tb_wdt.c b/drivers/watchdog/xilinx_tb_wdt.c
> index 2274123e49..23ddbdfbee 100644
> --- a/drivers/watchdog/xilinx_tb_wdt.c
> +++ b/drivers/watchdog/xilinx_tb_wdt.c
> @@ -1,13 +1,17 @@
>  // SPDX-License-Identifier: GPL-2.0+
>  /*
> + * Xilinx Axi platforms watchdog timer driver.
> + *
> + * Author(s):  Michal Å imek <michal.simek@xilinx.com>
> + *             Shreenidhi Shedi <yesshedi@gmail.com>
> + *
>   * Copyright (c) 2011-2013 Xilinx Inc.
>   */
>
>  #include <common.h>
> -#include <asm/io.h>
> -#include <asm/microblaze_intc.h>
> -#include <asm/processor.h>
> -#include <watchdog.h>
> +#include <dm.h>
> +#include <wdt.h>
> +#include <linux/io.h>
>
>  #define XWT_CSR0_WRS_MASK      0x00000008 /* Reset status Mask */
>  #define XWT_CSR0_WDS_MASK      0x00000004 /* Timer state Mask */
> @@ -20,49 +24,102 @@ struct watchdog_regs {
>         u32 tbr; /* 0x8 */
>  };
>
> -static struct watchdog_regs *watchdog_base =
> -                       (struct watchdog_regs *)CONFIG_WATCHDOG_BASEADDR;
> +struct xlnx_wdt_priv {
> +       bool enable_once;
> +       struct watchdog_regs *regs;
> +};
>
> -void hw_watchdog_reset(void)
> +static int xlnx_wdt_reset(struct udevice *dev)

You could pass just regs to this function? Same below.

>  {
>         u32 reg;
> +       struct xlnx_wdt_priv *priv = dev_get_priv(dev);
> +
> +       debug("%s\n", __func__);
>
>         /* Read the current contents of TCSR0 */
> -       reg = readl(&watchdog_base->twcsr0);
> +       reg = readl(&priv->regs->twcsr0);
>
>         /* Clear the watchdog WDS bit */
>         if (reg & (XWT_CSR0_EWDT1_MASK | XWT_CSRX_EWDT2_MASK))
> -               writel(reg | XWT_CSR0_WDS_MASK, &watchdog_base->twcsr0);
> +               writel(reg | XWT_CSR0_WDS_MASK, &priv->regs->twcsr0);
> +
> +       return 0;
>  }
>
> -void hw_watchdog_disable(void)
> +static int xlnx_wdt_stop(struct udevice *dev)
>  {
>         u32 reg;
> +       struct xlnx_wdt_priv *priv = dev_get_priv(dev);
> +
> +       if (priv->enable_once) {
> +               puts("Can't stop Xilinx watchdog.\n");

debug()

> +               return -1;

return -EBUSY

-1 is -EPERM which might not be correct.

> +       }
>
>         /* Read the current contents of TCSR0 */
> -       reg = readl(&watchdog_base->twcsr0);
> +       reg = readl(&priv->regs->twcsr0);
>
> -       writel(reg & ~XWT_CSR0_EWDT1_MASK, &watchdog_base->twcsr0);
> -       writel(~XWT_CSRX_EWDT2_MASK, &watchdog_base->twcsr1);
> +       writel(reg & ~XWT_CSR0_EWDT1_MASK, &priv->regs->twcsr0);
> +       writel(~XWT_CSRX_EWDT2_MASK, &priv->regs->twcsr1);
>
>         puts("Watchdog disabled!\n");

Drivers should not output to the console.

> +
> +       return 0;
>  }
>
> -static void hw_watchdog_isr(void *arg)
> +static int xlnx_wdt_start(struct udevice *dev, u64 timeout, ulong flags)
>  {
> -       hw_watchdog_reset();
> +       struct xlnx_wdt_priv *priv = dev_get_priv(dev);
> +
> +       writel((XWT_CSR0_WRS_MASK | XWT_CSR0_WDS_MASK | XWT_CSR0_EWDT1_MASK),
> +              &priv->regs->twcsr0);
> +
> +       writel(XWT_CSRX_EWDT2_MASK, &priv->regs->twcsr1);
> +
> +       return 0;
>  }
>
> -void hw_watchdog_init(void)
> +static int xlnx_wdt_probe(struct udevice *dev)
>  {
> -       int ret;
> +       debug("%s: Probing wdt%u\n", __func__, dev->seq);
>
> -       writel((XWT_CSR0_WRS_MASK | XWT_CSR0_WDS_MASK | XWT_CSR0_EWDT1_MASK),
> -              &watchdog_base->twcsr0);
> -       writel(XWT_CSRX_EWDT2_MASK, &watchdog_base->twcsr1);
> +       return 0;
> +}
>
> -       ret = install_interrupt_handler(CONFIG_WATCHDOG_IRQ,
> -                                               hw_watchdog_isr, NULL);
> -       if (ret)
> -               puts("Watchdog IRQ registration failed.");
> +static int xlnx_wdt_ofdata_to_platdata(struct udevice *dev)
> +{
> +       struct xlnx_wdt_priv *priv = dev_get_priv(dev);
> +
> +       priv->regs = (struct watchdog_regs *)dev_read_addr(dev);
> +       if (IS_ERR(priv->regs))
> +               return PTR_ERR(priv->regs);
> +
> +       priv->enable_once = dev_read_u32_default(dev, "xlnx,wdt-enable-once",
> +                                                0);

Should that not be dev_read_bool()? Does it actually using an integer
to indicate a boolean? Is there a binding file for this?

> +
> +       debug("%s: wdt-enable-once %d\n", __func__, priv->enable_once);
> +
> +       return 0;
>  }
> +
> +static const struct wdt_ops xlnx_wdt_ops = {
> +       .start = xlnx_wdt_start,
> +       .reset = xlnx_wdt_reset,
> +       .stop = xlnx_wdt_stop,
> +};
> +
> +static const struct udevice_id xlnx_wdt_ids[] = {
> +       { .compatible = "xlnx,xps-timebase-wdt-1.00.a", },
> +       { .compatible = "xlnx,xps-timebase-wdt-1.01.a", },
> +       {},
> +};
> +
> +U_BOOT_DRIVER(xlnx_wdt) = {
> +       .name = "xlnx_wdt",
> +       .id = UCLASS_WDT,
> +       .of_match = xlnx_wdt_ids,
> +       .probe = xlnx_wdt_probe,
> +       .priv_auto_alloc_size = sizeof(struct xlnx_wdt_priv),
> +       .ofdata_to_platdata = xlnx_wdt_ofdata_to_platdata,
> +       .ops = &xlnx_wdt_ops,
> +};
> --
> 2.17.1
>

Regards,
Simon

[U-Boot] [PATCH v2 3/3] watchdog: Convert Xilinx Axi watchdog driver to driver model

[Michal Simek]

Hi Simon,

On 19.7.2018 03:31, Simon Glass wrote:
> Hi Shreenidhi,
> 
> On 14 July 2018 at 14:35, Shreenidhi Shedi <yesshedi@gmail.com> wrote:
>> Xilinx Axi wdt driver conversion to driver model & Kconfig update
>> for the same.
>>
>> Changes in V1:
>>  - Xilinx Axi wdt driver conversion to DM initial version
>>
>> Changes in V2:
>>  - Rectified print message
>>  - Removed  stop instruction from probe
>>
>> Signed-off-by: Shreenidhi Shedi <yesshedi@gmail.com>
>> ---
>>
>> Changes in v2: None
>>
>>  drivers/watchdog/Kconfig         |   8 +++
>>  drivers/watchdog/xilinx_tb_wdt.c | 105 ++++++++++++++++++++++++-------
>>  2 files changed, 89 insertions(+), 24 deletions(-)
>>
>> diff --git a/drivers/watchdog/Kconfig b/drivers/watchdog/Kconfig
>> index 148c6a0d68..351d2af8d9 100644
>> --- a/drivers/watchdog/Kconfig
>> +++ b/drivers/watchdog/Kconfig
>> @@ -103,4 +103,12 @@ config WDT_CDNS
>>            Select this to enable Cadence watchdog timer, which can be found on some
>>            Xilinx Microzed Platform.
>>
>> +config XILINX_TB_WATCHDOG
>> +       bool "Xilinx Axi watchdog timer support"
>> +       depends on WDT
>> +       imply WATCHDOG
>> +       help
>> +          Select this to enable Xilinx Axi watchdog timer, which can be found on some
>> +          Xilinx Microblaze Platform.
> 
> platforms?

will fix.

> 
>> +
>>  endmenu
>> diff --git a/drivers/watchdog/xilinx_tb_wdt.c b/drivers/watchdog/xilinx_tb_wdt.c
>> index 2274123e49..23ddbdfbee 100644
>> --- a/drivers/watchdog/xilinx_tb_wdt.c
>> +++ b/drivers/watchdog/xilinx_tb_wdt.c
>> @@ -1,13 +1,17 @@
>>  // SPDX-License-Identifier: GPL-2.0+
>>  /*
>> + * Xilinx Axi platforms watchdog timer driver.
>> + *
>> + * Author(s):  Michal Å imek <michal.simek@xilinx.com>
>> + *             Shreenidhi Shedi <yesshedi@gmail.com>
>> + *
>>   * Copyright (c) 2011-2013 Xilinx Inc.
>>   */
>>
>>  #include <common.h>
>> -#include <asm/io.h>
>> -#include <asm/microblaze_intc.h>
>> -#include <asm/processor.h>
>> -#include <watchdog.h>
>> +#include <dm.h>
>> +#include <wdt.h>
>> +#include <linux/io.h>
>>
>>  #define XWT_CSR0_WRS_MASK      0x00000008 /* Reset status Mask */
>>  #define XWT_CSR0_WDS_MASK      0x00000004 /* Timer state Mask */
>> @@ -20,49 +24,102 @@ struct watchdog_regs {
>>         u32 tbr; /* 0x8 */
>>  };
>>
>> -static struct watchdog_regs *watchdog_base =
>> -                       (struct watchdog_regs *)CONFIG_WATCHDOG_BASEADDR;
>> +struct xlnx_wdt_priv {
>> +       bool enable_once;
>> +       struct watchdog_regs *regs;
>> +};
>>
>> -void hw_watchdog_reset(void)
>> +static int xlnx_wdt_reset(struct udevice *dev)
> 
> You could pass just regs to this function? Same below.

I don't think so. This function is the part of struct wdt_ops{}
which is

 82         /*
 83          * Reset the timer, typically restoring the counter to
 84          * the value configured by start()
 85          *
 86          * @dev: WDT Device
 87          * @return: 0 if OK, -ve on error
 88          */
 89         int (*reset)(struct udevice *dev);




> 
>>  {
>>         u32 reg;
>> +       struct xlnx_wdt_priv *priv = dev_get_priv(dev);
>> +
>> +       debug("%s\n", __func__);
>>
>>         /* Read the current contents of TCSR0 */
>> -       reg = readl(&watchdog_base->twcsr0);
>> +       reg = readl(&priv->regs->twcsr0);
>>
>>         /* Clear the watchdog WDS bit */
>>         if (reg & (XWT_CSR0_EWDT1_MASK | XWT_CSRX_EWDT2_MASK))
>> -               writel(reg | XWT_CSR0_WDS_MASK, &watchdog_base->twcsr0);
>> +               writel(reg | XWT_CSR0_WDS_MASK, &priv->regs->twcsr0);
>> +
>> +       return 0;
>>  }
>>
>> -void hw_watchdog_disable(void)
>> +static int xlnx_wdt_stop(struct udevice *dev)
>>  {
>>         u32 reg;
>> +       struct xlnx_wdt_priv *priv = dev_get_priv(dev);
>> +
>> +       if (priv->enable_once) {
>> +               puts("Can't stop Xilinx watchdog.\n");
> 
> debug()

will fix

> 
>> +               return -1;
> 
> return -EBUSY
> 
> -1 is -EPERM which might not be correct.

will fix

> 
>> +       }
>>
>>         /* Read the current contents of TCSR0 */
>> -       reg = readl(&watchdog_base->twcsr0);
>> +       reg = readl(&priv->regs->twcsr0);
>>
>> -       writel(reg & ~XWT_CSR0_EWDT1_MASK, &watchdog_base->twcsr0);
>> -       writel(~XWT_CSRX_EWDT2_MASK, &watchdog_base->twcsr1);
>> +       writel(reg & ~XWT_CSR0_EWDT1_MASK, &priv->regs->twcsr0);
>> +       writel(~XWT_CSRX_EWDT2_MASK, &priv->regs->twcsr1);
>>
>>         puts("Watchdog disabled!\n");
> 
> Drivers should not output to the console.

debug is fine here.

> 
>> +
>> +       return 0;
>>  }
>>
>> -static void hw_watchdog_isr(void *arg)
>> +static int xlnx_wdt_start(struct udevice *dev, u64 timeout, ulong flags)
>>  {
>> -       hw_watchdog_reset();
>> +       struct xlnx_wdt_priv *priv = dev_get_priv(dev);
>> +
>> +       writel((XWT_CSR0_WRS_MASK | XWT_CSR0_WDS_MASK | XWT_CSR0_EWDT1_MASK),
>> +              &priv->regs->twcsr0);
>> +
>> +       writel(XWT_CSRX_EWDT2_MASK, &priv->regs->twcsr1);
>> +
>> +       return 0;
>>  }
>>
>> -void hw_watchdog_init(void)
>> +static int xlnx_wdt_probe(struct udevice *dev)
>>  {
>> -       int ret;
>> +       debug("%s: Probing wdt%u\n", __func__, dev->seq);
>>
>> -       writel((XWT_CSR0_WRS_MASK | XWT_CSR0_WDS_MASK | XWT_CSR0_EWDT1_MASK),
>> -              &watchdog_base->twcsr0);
>> -       writel(XWT_CSRX_EWDT2_MASK, &watchdog_base->twcsr1);
>> +       return 0;
>> +}
>>
>> -       ret = install_interrupt_handler(CONFIG_WATCHDOG_IRQ,
>> -                                               hw_watchdog_isr, NULL);
>> -       if (ret)
>> -               puts("Watchdog IRQ registration failed.");
>> +static int xlnx_wdt_ofdata_to_platdata(struct udevice *dev)
>> +{
>> +       struct xlnx_wdt_priv *priv = dev_get_priv(dev);
>> +
>> +       priv->regs = (struct watchdog_regs *)dev_read_addr(dev);
>> +       if (IS_ERR(priv->regs))
>> +               return PTR_ERR(priv->regs);
>> +
>> +       priv->enable_once = dev_read_u32_default(dev, "xlnx,wdt-enable-once",
>> +                                                0);
> 
> Should that not be dev_read_bool()? Does it actually using an integer
> to indicate a boolean? Is there a binding file for this?

Unfortunately no. It is pretty old binding and it wasn't defined as
bool. Here is binding doc.

https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/tree/Documentation/devicetree/bindings/watchdog/of-xilinx-wdt.txt

> 
>> +
>> +       debug("%s: wdt-enable-once %d\n", __func__, priv->enable_once);
>> +
>> +       return 0;
>>  }
>> +
>> +static const struct wdt_ops xlnx_wdt_ops = {
>> +       .start = xlnx_wdt_start,
>> +       .reset = xlnx_wdt_reset,
>> +       .stop = xlnx_wdt_stop,
>> +};
>> +
>> +static const struct udevice_id xlnx_wdt_ids[] = {
>> +       { .compatible = "xlnx,xps-timebase-wdt-1.00.a", },
>> +       { .compatible = "xlnx,xps-timebase-wdt-1.01.a", },
>> +       {},
>> +};
>> +
>> +U_BOOT_DRIVER(xlnx_wdt) = {
>> +       .name = "xlnx_wdt",
>> +       .id = UCLASS_WDT,
>> +       .of_match = xlnx_wdt_ids,
>> +       .probe = xlnx_wdt_probe,
>> +       .priv_auto_alloc_size = sizeof(struct xlnx_wdt_priv),
>> +       .ofdata_to_platdata = xlnx_wdt_ofdata_to_platdata,
>> +       .ops = &xlnx_wdt_ops,
>> +};
>> --
>> 2.17.1
>>

Thanks,
Michal