Hi Shreenidhi, On 14 July 2018 at 14:35, Shreenidhi Shedi <[email protected]> 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 <[email protected]> > --- > > 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 <[email protected]> > + * Shreenidhi Shedi <[email protected]> > + * > * 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 mailing list [email protected] https://lists.denx.de/listinfo/u-boot
