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 WDTimply WATCHDOGhelpSelect this to enable Xilinx Axi watchdog timer, which can be found on someXilinx 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/Docu...
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