lists.openwall.net   lists  /  announce  owl-users  owl-dev  john-users  john-dev  passwdqc-users  yescrypt  popa3d-users  /  oss-security  kernel-hardening  musl  sabotage  tlsify  passwords  /  crypt-dev  xvendor  /  Bugtraq  Full-Disclosure  linux-kernel  linux-netdev  linux-ext4  linux-hardening  linux-cve-announce  PHC 
Open Source and information security mailing list archives
 
Hash Suite for Android: free password hash cracker in your pocket
[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <DM6PR02MB53866BB023B6B525D48B76F9AF639@DM6PR02MB5386.namprd02.prod.outlook.com>
Date:   Wed, 24 Mar 2021 06:04:21 +0000
From:   Srinivas Neeli <sneeli@...inx.com>
To:     Guenter Roeck <linux@...ck-us.net>,
        Michal Simek <michals@...inx.com>,
        Shubhrajyoti Datta <shubhraj@...inx.com>,
        Srinivas Goud <sgoud@...inx.com>
CC:     "wim@...ux-watchdog.org" <wim@...ux-watchdog.org>,
        "linux-watchdog@...r.kernel.org" <linux-watchdog@...r.kernel.org>,
        "linux-arm-kernel@...ts.infradead.org" 
        <linux-arm-kernel@...ts.infradead.org>,
        "linux-kernel@...r.kernel.org" <linux-kernel@...r.kernel.org>,
        git <git@...inx.com>
Subject: RE: [PATCH 7/9] watchdog: of_xilinx_wdt: Add Versal Window watchdog
 support

Hi Guenter,

Thanks for review

> -----Original Message-----
> From: Guenter Roeck <groeck7@...il.com> On Behalf Of Guenter Roeck
> Sent: Tuesday, March 16, 2021 8:01 AM
> To: Srinivas Neeli <sneeli@...inx.com>; Michal Simek <michals@...inx.com>;
> Shubhrajyoti Datta <shubhraj@...inx.com>; Srinivas Goud
> <sgoud@...inx.com>
> Cc: wim@...ux-watchdog.org; linux-watchdog@...r.kernel.org; linux-arm-
> kernel@...ts.infradead.org; linux-kernel@...r.kernel.org; git
> <git@...inx.com>
> Subject: Re: [PATCH 7/9] watchdog: of_xilinx_wdt: Add Versal Window
> watchdog support
> 
> On 3/15/21 3:46 AM, Srinivas Neeli wrote:
> > Versal watchdog driver uses Window watchdog mode. Window watchdog
> > timer(WWDT) contains closed(first) and open(second) window with
> > 32 bit width. WWDT will generate an interrupt after the first window
> > timeout and reset signal after the second window timeout. Timeout and
> > Pre-timeout configuration, Stop and Refresh trigger only in open
> > window.
> >
> > Signed-off-by: Srinivas Neeli <srinivas.neeli@...inx.com>
> 
> I think this should be a separate watchdog driver. There is pretty much no
> overlap with the existing driver.

Xilinx AXI Timebase Watchdog Timer supports two independent modes
1)Timebase Watchdog Mode
2)Window Watchdog Timer Mode.
Current of_xilinx_wdt.c driver already have support for Timebase Watchdog Mode, but Window watchdog timer Mode feature is missing.
Versal platform contains customized AXI Timebase Watchdog Timer, which supports Window Watchdog Timer Mode.
For that reason we are creating common driver for both the modes.

Thanks
Srinivas Neeli
> 
> Guenter
> 
> > ---
> >  drivers/watchdog/of_xilinx_wdt.c | 285
> > ++++++++++++++++++++++++++++++++++++++-
> >  1 file changed, 283 insertions(+), 2 deletions(-)
> >
> > diff --git a/drivers/watchdog/of_xilinx_wdt.c
> > b/drivers/watchdog/of_xilinx_wdt.c
> > index 3b93b60f1a00..3656e716b4f7 100644
> > --- a/drivers/watchdog/of_xilinx_wdt.c
> > +++ b/drivers/watchdog/of_xilinx_wdt.c
> > @@ -2,10 +2,11 @@
> >  /*
> >   * Watchdog Device Driver for Xilinx axi/xps_timebase_wdt
> >   *
> > - * (C) Copyright 2013 - 2014 Xilinx, Inc.
> > + * (C) Copyright 2013 - 2021 Xilinx, Inc.
> >   * (C) Copyright 2011 (Alejandro Cabrera <aldaya@...il.com>)
> >   */
> >
> > +#include <linux/bits.h>
> >  #include <linux/clk.h>
> >  #include <linux/err.h>
> >  #include <linux/module.h>
> > @@ -17,6 +18,11 @@
> >  #include <linux/of.h>
> >  #include <linux/of_device.h>
> >  #include <linux/of_address.h>
> > +#include <linux/interrupt.h>
> > +
> > +#define XWT_WWDT_DEFAULT_TIMEOUT	10
> > +#define XWT_WWDT_MIN_TIMEOUT		1
> > +#define XWT_WWDT_MAX_TIMEOUT		42
> >
> >  /* Register offsets for the Wdt device */
> >  #define XWT_TWCSR0_OFFSET   0x0 /* Control/Status Register0 */
> > @@ -35,15 +41,44 @@
> >  #define XWT_MAX_SELFTEST_LOOP_COUNT 0x00010000
> >  #define XWT_TIMER_FAILED            0xFFFFFFFF
> >
> > +/* Register offsets for the WWdt device */
> > +#define XWT_WWDT_MWR_OFFSET	0x00
> > +#define XWT_WWDT_ESR_OFFSET	0x04
> > +#define XWT_WWDT_FCR_OFFSET	0x08
> > +#define XWT_WWDT_FWR_OFFSET	0x0c
> > +#define XWT_WWDT_SWR_OFFSET	0x10
> > +
> > +/* Master Write Control Register Masks */
> > +#define XWT_WWDT_MWR_MASK	BIT(0)
> > +
> > +/* Enable and Status Register Masks */
> > +#define XWT_WWDT_ESR_WINT_MASK	BIT(16)
> > +#define XWT_WWDT_ESR_WSW_MASK	BIT(8)
> > +#define XWT_WWDT_ESR_WEN_MASK	BIT(0)
> > +
> > +/* Function control Register Masks */
> > +#define XWT_WWDT_SBC_MASK	0xFF00
> > +#define XWT_WWDT_SBC_SHIFT	16
> > +#define XWT_WWDT_BSS_MASK	0xC0
> > +
> >  #define WATCHDOG_NAME     "Xilinx Watchdog"
> >
> > +static int wdt_timeout;
> > +
> > +module_param(wdt_timeout, int, 0644);
> MODULE_PARM_DESC(wdt_timeout,
> > +		 "Watchdog time in seconds. (default="
> > +		 __MODULE_STRING(XWT_WWDT_DEFAULT_TIMEOUT) ")");
> > +
> >  /**
> >   * enum xwdt_ip_type - WDT IP type.
> >   *
> >   * @XWDT_WDT: Soft wdt ip.
> > + * @XWDT_WWDT: Window wdt ip.
> >   */
> >  enum xwdt_ip_type {
> >  	XWDT_WDT = 0,
> > +	XWDT_WWDT = 1,
> >  };
> >
> >  struct xwdt_devtype_data {
> > @@ -58,6 +93,7 @@ struct xwdt_device {
> >  	spinlock_t spinlock; /* spinlock for register handling */
> >  	struct watchdog_device xilinx_wdt_wdd;
> >  	struct clk		*clk;
> > +	int irq;
> >  };
> >
> >  static int xilinx_wdt_start(struct watchdog_device *wdd) @@ -145,6
> > +181,220 @@ static const struct watchdog_ops xilinx_wdt_ops = {
> >  	.ping = xilinx_wdt_keepalive,
> >  };
> >
> > +static int is_wwdt_in_closed_window(struct watchdog_device *wdd) {
> > +	u32 control_status_reg;
> > +	struct xwdt_device *xdev = watchdog_get_drvdata(wdd);
> > +
> > +	control_status_reg = ioread32(xdev->base +
> XWT_WWDT_ESR_OFFSET);
> > +	if (control_status_reg & XWT_WWDT_ESR_WEN_MASK)
> > +		if (!(control_status_reg & XWT_WWDT_ESR_WSW_MASK))
> > +			return 0;
> > +
> > +	return 1;
> > +}
> > +
> > +static int xilinx_wwdt_start(struct watchdog_device *wdd) {
> > +	int ret;
> > +	u32 control_status_reg, fcr;
> > +	u64 time_out, pre_timeout, count;
> > +	struct xwdt_device *xdev = watchdog_get_drvdata(wdd);
> > +	struct watchdog_device *xilinx_wdt_wdd = &xdev-
> >xilinx_wdt_wdd;
> > +
> > +	count = clk_get_rate(xdev->clk);
> > +	if (!count)
> > +		return -EINVAL;
> > +
> > +	/* Calculate timeout count */
> > +	pre_timeout = count * wdd->pretimeout;
> > +	time_out = count * wdd->timeout;
> > +	if (!watchdog_active(xilinx_wdt_wdd)) {
> > +		ret  = clk_enable(xdev->clk);
> > +		if (ret) {
> > +			dev_err(wdd->parent, "Failed to enable clock\n");
> > +			return ret;
> > +		}
> > +	}
> > +
> > +	spin_lock(&xdev->spinlock);
> > +	iowrite32(XWT_WWDT_MWR_MASK, xdev->base +
> XWT_WWDT_MWR_OFFSET);
> > +	iowrite32(~(u32)XWT_WWDT_ESR_WEN_MASK,
> > +		  xdev->base + XWT_WWDT_ESR_OFFSET);
> > +
> > +	if (pre_timeout) {
> > +		iowrite32((u32)(time_out - pre_timeout),
> > +			  xdev->base + XWT_WWDT_FWR_OFFSET);
> > +		iowrite32((u32)pre_timeout, xdev->base +
> XWT_WWDT_SWR_OFFSET);
> > +		fcr = ioread32(xdev->base + XWT_WWDT_SWR_OFFSET);
> > +		fcr = (fcr >> XWT_WWDT_SBC_SHIFT) &
> XWT_WWDT_SBC_MASK;
> > +		fcr = fcr | XWT_WWDT_BSS_MASK;
> > +		iowrite32(fcr, xdev->base + XWT_WWDT_FCR_OFFSET);
> > +	} else {
> > +		iowrite32((u32)pre_timeout,
> > +			  xdev->base + XWT_WWDT_FWR_OFFSET);
> > +		iowrite32((u32)time_out, xdev->base +
> XWT_WWDT_SWR_OFFSET);
> > +		iowrite32(0x0, xdev->base + XWT_WWDT_FCR_OFFSET);
> > +	}
> > +
> > +	/* Enable the window watchdog timer */
> > +	control_status_reg = ioread32(xdev->base +
> XWT_WWDT_ESR_OFFSET);
> > +	control_status_reg |= XWT_WWDT_ESR_WEN_MASK;
> > +	iowrite32(control_status_reg, xdev->base +
> XWT_WWDT_ESR_OFFSET);
> > +
> > +	spin_unlock(&xdev->spinlock);
> > +
> > +	dev_dbg(xilinx_wdt_wdd->parent, "Watchdog Started!\n");
> > +
> > +	return 0;
> > +}
> > +
> > +static int xilinx_wwdt_stop(struct watchdog_device *wdd) {
> > +	struct xwdt_device *xdev = watchdog_get_drvdata(wdd);
> > +	struct watchdog_device *xilinx_wdt_wdd = &xdev-
> >xilinx_wdt_wdd;
> > +
> > +	if (!is_wwdt_in_closed_window(wdd)) {
> > +		dev_warn(xilinx_wdt_wdd->parent, "timer in closed
> window");
> > +		return -EINVAL;
> > +	}
> > +
> > +	spin_lock(&xdev->spinlock);
> > +
> > +	iowrite32(XWT_WWDT_MWR_MASK, xdev->base +
> XWT_WWDT_MWR_OFFSET);
> > +	/* Disable the Window watchdog timer */
> > +	iowrite32(~(u32)XWT_WWDT_ESR_WEN_MASK,
> > +		  xdev->base + XWT_WWDT_ESR_OFFSET);
> > +
> > +	spin_unlock(&xdev->spinlock);
> > +
> > +	if (watchdog_active(xilinx_wdt_wdd))
> > +		clk_disable(xdev->clk);
> > +
> > +	dev_dbg(xilinx_wdt_wdd->parent, "Watchdog Stopped!\n");
> > +
> > +	return 0;
> > +}
> > +
> > +static int xilinx_wwdt_keepalive(struct watchdog_device *wdd) {
> > +	u32 control_status_reg;
> > +	struct xwdt_device *xdev = watchdog_get_drvdata(wdd);
> > +
> > +	/* Refresh in open window is ignored */
> > +	if (!is_wwdt_in_closed_window(wdd))
> > +		return 0;
> > +
> > +	spin_lock(&xdev->spinlock);
> > +
> > +	iowrite32(XWT_WWDT_MWR_MASK, xdev->base +
> XWT_WWDT_MWR_OFFSET);
> > +	control_status_reg = ioread32(xdev->base +
> XWT_WWDT_ESR_OFFSET);
> > +	control_status_reg |= XWT_WWDT_ESR_WINT_MASK;
> > +	control_status_reg &= ~XWT_WWDT_ESR_WSW_MASK;
> > +	iowrite32(control_status_reg, xdev->base +
> XWT_WWDT_ESR_OFFSET);
> > +	control_status_reg = ioread32(xdev->base +
> XWT_WWDT_ESR_OFFSET);
> > +	control_status_reg |= XWT_WWDT_ESR_WSW_MASK;
> > +	iowrite32(control_status_reg, xdev->base +
> XWT_WWDT_ESR_OFFSET);
> > +
> > +	spin_unlock(&xdev->spinlock);
> > +
> > +	return 0;
> > +}
> > +
> > +static int xilinx_wwdt_set_timeout(struct watchdog_device *wdd,
> > +				   unsigned int new_time)
> > +{
> > +	u32 ret = 0;
> > +	struct xwdt_device *xdev = watchdog_get_drvdata(wdd);
> > +	struct watchdog_device *xilinx_wdt_wdd = &xdev-
> >xilinx_wdt_wdd;
> > +
> > +	if (!is_wwdt_in_closed_window(wdd)) {
> > +		dev_warn(xilinx_wdt_wdd->parent, "timer in closed
> window");
> > +		return -EINVAL;
> > +	}
> > +
> > +	if (new_time < XWT_WWDT_MIN_TIMEOUT ||
> > +	    new_time > XWT_WWDT_MAX_TIMEOUT) {
> > +		dev_warn(xilinx_wdt_wdd->parent,
> > +			 "timeout value must be %d<=x<=%d, using %d\n",
> > +				XWT_WWDT_MIN_TIMEOUT,
> > +				XWT_WWDT_MAX_TIMEOUT, new_time);
> > +		return -EINVAL;
> > +	}
> > +
> > +	wdd->timeout = new_time;
> > +	wdd->pretimeout = 0;
> > +
> > +	if (watchdog_active(xilinx_wdt_wdd)) {
> > +		ret = xilinx_wwdt_start(wdd);
> > +		if (ret)
> > +			dev_dbg(xilinx_wdt_wdd->parent, "timer start
> failed");
> > +	}
> > +
> > +	return 0;
> > +}
> > +
> > +static int xilinx_wwdt_set_pretimeout(struct watchdog_device *wdd,
> > +				      u32 new_pretimeout)
> > +{
> > +	u32 ret = 0;
> > +	struct xwdt_device *xdev = watchdog_get_drvdata(wdd);
> > +	struct watchdog_device *xilinx_wdt_wdd = &xdev-
> >xilinx_wdt_wdd;
> > +
> > +	if (!is_wwdt_in_closed_window(wdd)) {
> > +		dev_warn(xilinx_wdt_wdd->parent, "timer in closed
> window");
> > +		return -EINVAL;
> > +	}
> > +
> > +	if (new_pretimeout < wdd->min_timeout ||
> > +	    new_pretimeout >= wdd->timeout)
> > +		return -EINVAL;
> > +
> > +	wdd->pretimeout = new_pretimeout;
> > +
> > +	if (watchdog_active(xilinx_wdt_wdd)) {
> > +		ret = xilinx_wwdt_start(wdd);
> > +		if (ret)
> > +			dev_dbg(xilinx_wdt_wdd->parent, "timer start
> failed");
> > +	}
> > +
> > +	return 0;
> > +}
> > +
> > +static irqreturn_t xilinx_wwdt_isr(int irq, void *wdog_arg) {
> > +	struct xwdt_device *xdev = wdog_arg;
> > +
> > +	watchdog_notify_pretimeout(&xdev->xilinx_wdt_wdd);
> > +	return IRQ_HANDLED;
> > +}
> > +
> > +static const struct watchdog_info xilinx_wwdt_ident = {
> > +	.options =  WDIOF_MAGICCLOSE |
> > +		WDIOF_KEEPALIVEPING |
> > +		WDIOF_SETTIMEOUT,
> > +	.firmware_version = 1,
> > +	.identity = "xlnx_wwdt watchdog",
> > +};
> > +
> > +static const struct watchdog_info xilinx_wwdt_pretimeout_ident = {
> > +	.options = WDIOF_MAGICCLOSE |
> > +		   WDIOF_KEEPALIVEPING |
> > +		   WDIOF_PRETIMEOUT |
> > +		   WDIOF_SETTIMEOUT,
> > +	.firmware_version = 1,
> > +	.identity = "xlnx_wwdt watchdog",
> > +};
> > +
> > +static const struct watchdog_ops xilinx_wwdt_ops = {
> > +	.owner = THIS_MODULE,
> > +	.start = xilinx_wwdt_start,
> > +	.stop = xilinx_wwdt_stop,
> > +	.ping = xilinx_wwdt_keepalive,
> > +	.set_timeout = xilinx_wwdt_set_timeout,
> > +	.set_pretimeout = xilinx_wwdt_set_pretimeout, };
> > +
> >  static u32 xwdt_selftest(struct xwdt_device *xdev)  {
> >  	int i;
> > @@ -181,11 +431,19 @@ static const struct xwdt_devtype_data
> xwdt_wdt_data = {
> >  	.xwdt_ops = &xilinx_wdt_ops,
> >  };
> >
> > +static const struct xwdt_devtype_data xwdt_wwdt_data = {
> > +	.wdttype = XWDT_WWDT,
> > +	.xwdt_info = &xilinx_wwdt_ident,
> > +	.xwdt_ops = &xilinx_wwdt_ops,
> > +};
> > +
> >  static const struct of_device_id xwdt_of_match[] = {
> >  	{ .compatible = "xlnx,xps-timebase-wdt-1.00.a",
> >  		.data = &xwdt_wdt_data },
> >  	{ .compatible = "xlnx,xps-timebase-wdt-1.01.a",
> >  		.data = &xwdt_wdt_data },
> > +	{ .compatible = "xlnx,versal-wwdt-1.0",
> > +		.data = &xwdt_wwdt_data },
> >  	{},
> >  };
> >  MODULE_DEVICE_TABLE(of, xwdt_of_match); @@ -194,7 +452,7 @@
> static
> > int xwdt_probe(struct platform_device *pdev)  {
> >  	struct device *dev = &pdev->dev;
> >  	int rc;
> > -	u32 pfreq = 0, enable_once = 0;
> > +	u32 pfreq = 0, enable_once = 0, pre_timeout = 0;
> >  	struct xwdt_device *xdev;
> >  	struct watchdog_device *xilinx_wdt_wdd;
> >  	const struct of_device_id *of_id;
> > @@ -236,6 +494,12 @@ static int xwdt_probe(struct platform_device
> *pdev)
> >  				 "Parameter \"xlnx,wdt-enable-once\" not
> found\n");
> >
> >  		watchdog_set_nowayout(xilinx_wdt_wdd, enable_once);
> > +	} else {
> > +		rc = of_property_read_u32(dev->of_node, "pretimeout-
> sec",
> > +					  &pre_timeout);
> > +		if (rc)
> > +			dev_dbg(dev,
> > +				"Parameter \"pretimeout-sec\" not
> found\n");
> >  	}
> >
> >  	xdev->clk = devm_clk_get(dev, NULL); @@ -268,6 +532,23 @@ static
> int
> > xwdt_probe(struct platform_device *pdev)
> >  			xilinx_wdt_wdd->timeout =
> >  				2 * ((1 << xdev->wdt_interval) /
> >  					pfreq);
> > +	} else {
> > +		xilinx_wdt_wdd->pretimeout = pre_timeout;
> > +		xilinx_wdt_wdd->timeout =
> XWT_WWDT_DEFAULT_TIMEOUT;
> > +		xilinx_wdt_wdd->min_timeout =
> XWT_WWDT_MIN_TIMEOUT;
> > +		xilinx_wdt_wdd->max_timeout =
> XWT_WWDT_MAX_TIMEOUT;
> > +		xdev->irq = platform_get_irq_byname(pdev, "wdt");
> > +		if (xdev->irq > 0) {
> > +			if (!devm_request_irq(dev, xdev->irq,
> xilinx_wwdt_isr,
> > +					      0, dev_name(dev), xdev)) {
> > +				xilinx_wdt_wdd->info =
> > +				&xilinx_wwdt_pretimeout_ident;
> > +			}
> > +		}
> > +		rc = watchdog_init_timeout(xilinx_wdt_wdd,
> > +					   wdt_timeout, &pdev->dev);
> > +		if (rc)
> > +			dev_warn(&pdev->dev, "unable to set timeout
> value\n");
> >  	}
> >
> >  	spin_lock_init(&xdev->spinlock);
> >

Powered by blists - more mailing lists

Powered by Openwall GNU/*/Linux Powered by OpenVZ