[PATCH 01/10] watchdog: xilinx: Convert driver to the watchdog framework

[PATCH 01/10] watchdog: xilinx: Convert driver to the watchdog framework

[Michal Simek]

[-- Attachment #1: Type: text/plain, Size: 7998 bytes --]

- Remove uneeded headers, fops functions
- Use xilinx_wdt prefix in start/stop/keepalive functions
  and in new structures

Signed-off-by: Michal Simek <michal.simek@xilinx.com>
---

 drivers/watchdog/Kconfig         |   1 +
 drivers/watchdog/of_xilinx_wdt.c | 204 ++++++---------------------------------
 2 files changed, 33 insertions(+), 172 deletions(-)

diff --git a/drivers/watchdog/Kconfig b/drivers/watchdog/Kconfig
index 4c4c566..9db5d3c 100644
--- a/drivers/watchdog/Kconfig
+++ b/drivers/watchdog/Kconfig
@@ -1025,6 +1025,7 @@ config M54xx_WATCHDOG
 config XILINX_WATCHDOG
 	tristate "Xilinx Watchdog timer"
 	depends on MICROBLAZE
+	select WATCHDOG_CORE
 	---help---
 	  Watchdog driver for the xps_timebase_wdt ip core.

diff --git a/drivers/watchdog/of_xilinx_wdt.c b/drivers/watchdog/of_xilinx_wdt.c
index fb57103..8c2814e 100644
--- a/drivers/watchdog/of_xilinx_wdt.c
+++ b/drivers/watchdog/of_xilinx_wdt.c
@@ -1,6 +1,7 @@
 /*
  * Watchdog Device Driver for Xilinx axi/xps_timebase_wdt
  *
+ * (C) Copyright 2013 - 2014 Xilinx, Inc.
  * (C) Copyright 2011 (Alejandro Cabrera <aldaya@gmail.com>)
  *
  * This program is free software; you can redistribute it and/or
@@ -14,13 +15,10 @@
 #include <linux/module.h>
 #include <linux/types.h>
 #include <linux/kernel.h>
-#include <linux/fs.h>
-#include <linux/miscdevice.h>
 #include <linux/init.h>
 #include <linux/ioport.h>
 #include <linux/watchdog.h>
 #include <linux/io.h>
-#include <linux/uaccess.h>
 #include <linux/of.h>
 #include <linux/of_device.h>
 #include <linux/of_address.h>
@@ -48,22 +46,18 @@
 struct xwdt_device {
 	struct resource  res;
 	void __iomem *base;
-	u32 nowayout;
 	u32 wdt_interval;
-	u32 boot_status;
 };

 static struct xwdt_device xdev;

 static  u32 timeout;
 static  u32 control_status_reg;
-static  u8  expect_close;
 static  u8  no_timeout;
-static unsigned long driver_open;

 static  DEFINE_SPINLOCK(spinlock);

-static void xwdt_start(void)
+static int xilinx_wdt_start(struct watchdog_device *wdd)
 {
 	spin_lock(&spinlock);

@@ -77,9 +71,11 @@ static void xwdt_start(void)
 	iowrite32(XWT_CSRX_EWDT2_MASK, xdev.base + XWT_TWCSR1_OFFSET);

 	spin_unlock(&spinlock);
+
+	return 0;
 }

-static void xwdt_stop(void)
+static int xilinx_wdt_stop(struct watchdog_device *wdd)
 {
 	spin_lock(&spinlock);

@@ -92,9 +88,11 @@ static void xwdt_stop(void)

 	spin_unlock(&spinlock);
 	pr_info("Stopped!\n");
+
+	return 0;
 }

-static void xwdt_keepalive(void)
+static int xilinx_wdt_keepalive(struct watchdog_device *wdd)
 {
 	spin_lock(&spinlock);

@@ -103,23 +101,28 @@ static void xwdt_keepalive(void)
 	iowrite32(control_status_reg, xdev.base + XWT_TWCSR0_OFFSET);

 	spin_unlock(&spinlock);
-}

-static void xwdt_get_status(int *status)
-{
-	int new_status;
+	return 0;
+}

-	spin_lock(&spinlock);
+static const struct watchdog_info xilinx_wdt_ident = {
+	.options =  WDIOF_MAGICCLOSE |
+		    WDIOF_KEEPALIVEPING,
+	.firmware_version =	1,
+	.identity =	WATCHDOG_NAME,
+};

-	control_status_reg = ioread32(xdev.base + XWT_TWCSR0_OFFSET);
-	new_status = ((control_status_reg &
-			(XWT_CSR0_WRS_MASK | XWT_CSR0_WDS_MASK)) != 0);
-	spin_unlock(&spinlock);
+static const struct watchdog_ops xilinx_wdt_ops = {
+	.owner = THIS_MODULE,
+	.start = xilinx_wdt_start,
+	.stop = xilinx_wdt_stop,
+	.ping = xilinx_wdt_keepalive,
+};

-	*status = 0;
-	if (new_status & 1)
-		*status |= WDIOF_CARDRESET;
-}
+static struct watchdog_device xilinx_wdt_wdd = {
+	.info = &xilinx_wdt_ident,
+	.ops = &xilinx_wdt_ops,
+};

 static u32 xwdt_selftest(void)
 {
@@ -146,139 +149,6 @@ static u32 xwdt_selftest(void)
 		return XWT_TIMER_FAILED;
 }

-static int xwdt_open(struct inode *inode, struct file *file)
-{
-	/* Only one process can handle the wdt at a time */
-	if (test_and_set_bit(0, &driver_open))
-		return -EBUSY;
-
-	/* Make sure that the module are always loaded...*/
-	if (xdev.nowayout)
-		__module_get(THIS_MODULE);
-
-	xwdt_start();
-	pr_info("Started...\n");
-
-	return nonseekable_open(inode, file);
-}
-
-static int xwdt_release(struct inode *inode, struct file *file)
-{
-	if (expect_close == 42) {
-		xwdt_stop();
-	} else {
-		pr_crit("Unexpected close, not stopping watchdog!\n");
-		xwdt_keepalive();
-	}
-
-	clear_bit(0, &driver_open);
-	expect_close = 0;
-	return 0;
-}
-
-/*
- *      xwdt_write:
- *      @file: file handle to the watchdog
- *      @buf: buffer to write (unused as data does not matter here
- *      @count: count of bytes
- *      @ppos: pointer to the position to write. No seeks allowed
- *
- *      A write to a watchdog device is defined as a keepalive signal. Any
- *      write of data will do, as we don't define content meaning.
- */
-static ssize_t xwdt_write(struct file *file, const char __user *buf,
-						size_t len, loff_t *ppos)
-{
-	if (len) {
-		if (!xdev.nowayout) {
-			size_t i;
-
-			/* In case it was set long ago */
-			expect_close = 0;
-
-			for (i = 0; i != len; i++) {
-				char c;
-
-				if (get_user(c, buf + i))
-					return -EFAULT;
-				if (c == 'V')
-					expect_close = 42;
-			}
-		}
-		xwdt_keepalive();
-	}
-	return len;
-}
-
-static const struct watchdog_info ident = {
-	.options =  WDIOF_MAGICCLOSE |
-		    WDIOF_KEEPALIVEPING,
-	.firmware_version =	1,
-	.identity =	WATCHDOG_NAME,
-};
-
-/*
- *      xwdt_ioctl:
- *      @file: file handle to the device
- *      @cmd: watchdog command
- *      @arg: argument pointer
- *
- *      The watchdog API defines a common set of functions for all watchdogs
- *      according to their available features.
- */
-static long xwdt_ioctl(struct file *file, unsigned int cmd, unsigned long arg)
-{
-	int status;
-
-	union {
-		struct watchdog_info __user *ident;
-		int __user *i;
-	} uarg;
-
-	uarg.i = (int __user *)arg;
-
-	switch (cmd) {
-	case WDIOC_GETSUPPORT:
-		return copy_to_user(uarg.ident, &ident,
-					sizeof(ident)) ? -EFAULT : 0;
-
-	case WDIOC_GETBOOTSTATUS:
-		return put_user(xdev.boot_status, uarg.i);
-
-	case WDIOC_GETSTATUS:
-		xwdt_get_status(&status);
-		return put_user(status, uarg.i);
-
-	case WDIOC_KEEPALIVE:
-		xwdt_keepalive();
-		return 0;
-
-	case WDIOC_GETTIMEOUT:
-		if (no_timeout)
-			return -ENOTTY;
-		else
-			return put_user(timeout, uarg.i);
-
-	default:
-		return -ENOTTY;
-	}
-}
-
-static const struct file_operations xwdt_fops = {
-	.owner      = THIS_MODULE,
-	.llseek     = no_llseek,
-	.write      = xwdt_write,
-	.open       = xwdt_open,
-	.release    = xwdt_release,
-	.unlocked_ioctl = xwdt_ioctl,
-};
-
-static struct miscdevice xwdt_miscdev = {
-	.minor      = WATCHDOG_MINOR,
-	.name       = "watchdog",
-	.fops       = &xwdt_fops,
-};
-
 static int xwdt_probe(struct platform_device *pdev)
 {
 	int rc;
@@ -314,7 +184,7 @@ static int xwdt_probe(struct platform_device *pdev)
 					"xlnx,wdt-enable-once", NULL);
 	if (tmptr == NULL) {
 		pr_warn("Parameter \"xlnx,wdt-enable-once\" not found in device tree!\n");
-		xdev.nowayout = WATCHDOG_NOWAYOUT;
+		watchdog_set_nowayout(&xilinx_wdt_wdd, true);
 	}

 /*
@@ -344,24 +214,14 @@ static int xwdt_probe(struct platform_device *pdev)
 		goto unmap_io;
 	}

-	xwdt_get_status(&xdev.boot_status);
-
-	rc = misc_register(&xwdt_miscdev);
+	rc = watchdog_register_device(&xilinx_wdt_wdd);
 	if (rc) {
-		pr_err("cannot register miscdev on minor=%d (err=%d)\n",
-		       xwdt_miscdev.minor, rc);
+		pr_err("cannot register watchdog (err=%d)\n", rc);
 		goto unmap_io;
 	}

-	if (no_timeout)
-		pr_info("driver loaded (timeout=? sec, nowayout=%d)\n",
-			xdev.nowayout);
-	else
-		pr_info("driver loaded (timeout=%d sec, nowayout=%d)\n",
-			timeout, xdev.nowayout);
-
-	expect_close = 0;
-	clear_bit(0, &driver_open);
+	dev_info(&pdev->dev, "Xilinx Watchdog Timer at %p with timeout %ds\n",
+		 xdev.base, timeout);

 	return 0;

@@ -375,7 +235,7 @@ err_out:

 static int xwdt_remove(struct platform_device *dev)
 {
-	misc_deregister(&xwdt_miscdev);
+	watchdog_unregister_device(&xilinx_wdt_wdd);
 	iounmap(xdev.base);
 	release_mem_region(xdev.res.start, resource_size(&xdev.res));

--
1.8.2.3


[-- Attachment #2: Type: application/pgp-signature, Size: 198 bytes --]

[PATCH 02/10] watchdog: xilinx: Move control_status_reg to functions

[Michal Simek]

[-- Attachment #1: Type: text/plain, Size: 1319 bytes --]

control_status_reg is temp variables and should be
used locally by specific function.

Signed-off-by: Michal Simek <michal.simek@xilinx.com>
---

 drivers/watchdog/of_xilinx_wdt.c | 7 ++++++-
 1 file changed, 6 insertions(+), 1 deletion(-)

diff --git a/drivers/watchdog/of_xilinx_wdt.c b/drivers/watchdog/of_xilinx_wdt.c
index 8c2814e..aca9bab 100644
--- a/drivers/watchdog/of_xilinx_wdt.c
+++ b/drivers/watchdog/of_xilinx_wdt.c
@@ -52,13 +52,14 @@ struct xwdt_device {
 static struct xwdt_device xdev;

 static  u32 timeout;
-static  u32 control_status_reg;
 static  u8  no_timeout;

 static  DEFINE_SPINLOCK(spinlock);

 static int xilinx_wdt_start(struct watchdog_device *wdd)
 {
+	u32 control_status_reg;
+
 	spin_lock(&spinlock);

 	/* Clean previous status and enable the watchdog timer */
@@ -77,6 +78,8 @@ static int xilinx_wdt_start(struct watchdog_device *wdd)

 static int xilinx_wdt_stop(struct watchdog_device *wdd)
 {
+	u32 control_status_reg;
+
 	spin_lock(&spinlock);

 	control_status_reg = ioread32(xdev.base + XWT_TWCSR0_OFFSET);
@@ -94,6 +97,8 @@ static int xilinx_wdt_stop(struct watchdog_device *wdd)

 static int xilinx_wdt_keepalive(struct watchdog_device *wdd)
 {
+	u32 control_status_reg;
+
 	spin_lock(&spinlock);

 	control_status_reg = ioread32(xdev.base + XWT_TWCSR0_OFFSET);
--
1.8.2.3


[-- Attachment #2: Type: application/pgp-signature, Size: 198 bytes --]

[PATCH 03/10] watchdog: xilinx: Simplify probe and remove functions

[Michal Simek]

[-- Attachment #1: Type: text/plain, Size: 2800 bytes --]

Use devm_ helper function to simplify probe and error path.
Move ioremap to the beginning of probe function.

Signed-off-by: Michal Simek <michal.simek@xilinx.com>
---

 drivers/watchdog/of_xilinx_wdt.c | 41 +++++++++-------------------------------
 1 file changed, 9 insertions(+), 32 deletions(-)

diff --git a/drivers/watchdog/of_xilinx_wdt.c b/drivers/watchdog/of_xilinx_wdt.c
index aca9bab..7f371ed 100644
--- a/drivers/watchdog/of_xilinx_wdt.c
+++ b/drivers/watchdog/of_xilinx_wdt.c
@@ -12,6 +12,7 @@

 #define pr_fmt(fmt) KBUILD_MODNAME ": " fmt

+#include <linux/err.h>
 #include <linux/module.h>
 #include <linux/types.h>
 #include <linux/kernel.h>
@@ -44,7 +45,6 @@
 #define PFX WATCHDOG_NAME ": "

 struct xwdt_device {
-	struct resource  res;
 	void __iomem *base;
 	u32 wdt_interval;
 };
@@ -159,9 +159,15 @@ static int xwdt_probe(struct platform_device *pdev)
 	int rc;
 	u32 *tmptr;
 	u32 *pfreq;
+	struct resource *res;

 	no_timeout = 0;

+	res = platform_get_resource(pdev, IORESOURCE_MEM, 0);
+	xdev.base = devm_ioremap_resource(&pdev->dev, res);
+	if (IS_ERR(xdev.base))
+		return PTR_ERR(xdev.base);
+
 	pfreq = (u32 *)of_get_property(pdev->dev.of_node,
 					"clock-frequency", NULL);

@@ -170,12 +176,6 @@ static int xwdt_probe(struct platform_device *pdev)
 		no_timeout = 1;
 	}

-	rc = of_address_to_resource(pdev->dev.of_node, 0, &xdev.res);
-	if (rc) {
-		pr_warn("invalid address!\n");
-		return rc;
-	}
-
 	tmptr = (u32 *)of_get_property(pdev->dev.of_node,
 					"xlnx,wdt-interval", NULL);
 	if (tmptr == NULL) {
@@ -199,50 +199,27 @@ static int xwdt_probe(struct platform_device *pdev)
 	if (!no_timeout)
 		timeout = 2 * ((1<<xdev.wdt_interval) / *pfreq);

-	if (!request_mem_region(xdev.res.start,
-			xdev.res.end - xdev.res.start + 1, WATCHDOG_NAME)) {
-		rc = -ENXIO;
-		pr_err("memory request failure!\n");
-		goto err_out;
-	}
-
-	xdev.base = ioremap(xdev.res.start, xdev.res.end - xdev.res.start + 1);
-	if (xdev.base == NULL) {
-		rc = -ENOMEM;
-		pr_err("ioremap failure!\n");
-		goto release_mem;
-	}
-
 	rc = xwdt_selftest();
 	if (rc == XWT_TIMER_FAILED) {
 		pr_err("SelfTest routine error!\n");
-		goto unmap_io;
+		return rc;
 	}

 	rc = watchdog_register_device(&xilinx_wdt_wdd);
 	if (rc) {
 		pr_err("cannot register watchdog (err=%d)\n", rc);
-		goto unmap_io;
+		return rc;
 	}

 	dev_info(&pdev->dev, "Xilinx Watchdog Timer at %p with timeout %ds\n",
 		 xdev.base, timeout);

 	return 0;
-
-unmap_io:
-	iounmap(xdev.base);
-release_mem:
-	release_mem_region(xdev.res.start, resource_size(&xdev.res));
-err_out:
-	return rc;
 }

 static int xwdt_remove(struct platform_device *dev)
 {
 	watchdog_unregister_device(&xilinx_wdt_wdd);
-	iounmap(xdev.base);
-	release_mem_region(xdev.res.start, resource_size(&xdev.res));

 	return 0;
 }
--
1.8.2.3


[-- Attachment #2: Type: application/pgp-signature, Size: 198 bytes --]

[PATCH 04/10] watchdog: xilinx: Move no_timeout to probe function

[Michal Simek]

[-- Attachment #1: Type: text/plain, Size: 1358 bytes --]

no_timeout should be local variable because it is used
only in probe function.

Signed-off-by: Michal Simek <michal.simek@xilinx.com>
---

 drivers/watchdog/of_xilinx_wdt.c | 8 +++-----
 1 file changed, 3 insertions(+), 5 deletions(-)

diff --git a/drivers/watchdog/of_xilinx_wdt.c b/drivers/watchdog/of_xilinx_wdt.c
index 7f371ed..1f7ad91 100644
--- a/drivers/watchdog/of_xilinx_wdt.c
+++ b/drivers/watchdog/of_xilinx_wdt.c
@@ -52,7 +52,6 @@ struct xwdt_device {
 static struct xwdt_device xdev;

 static  u32 timeout;
-static  u8  no_timeout;

 static  DEFINE_SPINLOCK(spinlock);

@@ -160,8 +159,7 @@ static int xwdt_probe(struct platform_device *pdev)
 	u32 *tmptr;
 	u32 *pfreq;
 	struct resource *res;
-
-	no_timeout = 0;
+	bool no_timeout = false;

 	res = platform_get_resource(pdev, IORESOURCE_MEM, 0);
 	xdev.base = devm_ioremap_resource(&pdev->dev, res);
@@ -173,14 +171,14 @@ static int xwdt_probe(struct platform_device *pdev)

 	if (pfreq == NULL) {
 		pr_warn("The watchdog clock frequency cannot be obtained!\n");
-		no_timeout = 1;
+		no_timeout = true;
 	}

 	tmptr = (u32 *)of_get_property(pdev->dev.of_node,
 					"xlnx,wdt-interval", NULL);
 	if (tmptr == NULL) {
 		pr_warn("Parameter \"xlnx,wdt-interval\" not found in device tree!\n");
-		no_timeout = 1;
+		no_timeout = true;
 	} else {
 		xdev.wdt_interval = *tmptr;
 	}
--
1.8.2.3


[-- Attachment #2: Type: application/pgp-signature, Size: 198 bytes --]

[PATCH 05/10] watchdog: xilinx: Allocate private structure per device

[Michal Simek]

[-- Attachment #1: Type: text/plain, Size: 6542 bytes --]

Only one watchdog could be used by this driver.
Create driver private data structure and move there
all variables for one instance.

Signed-off-by: Michal Simek <michal.simek@xilinx.com>
---

 drivers/watchdog/of_xilinx_wdt.c | 97 +++++++++++++++++++++++-----------------
 1 file changed, 55 insertions(+), 42 deletions(-)

diff --git a/drivers/watchdog/of_xilinx_wdt.c b/drivers/watchdog/of_xilinx_wdt.c
index 1f7ad91..d28bd3f 100644
--- a/drivers/watchdog/of_xilinx_wdt.c
+++ b/drivers/watchdog/of_xilinx_wdt.c
@@ -47,30 +47,27 @@
 struct xwdt_device {
 	void __iomem *base;
 	u32 wdt_interval;
+	spinlock_t spinlock;
+	struct watchdog_device xilinx_wdt_wdd;
 };

-static struct xwdt_device xdev;
-
-static  u32 timeout;
-
-static  DEFINE_SPINLOCK(spinlock);
-
 static int xilinx_wdt_start(struct watchdog_device *wdd)
 {
 	u32 control_status_reg;
+	struct xwdt_device *xdev = watchdog_get_drvdata(wdd);

-	spin_lock(&spinlock);
+	spin_lock(&xdev->spinlock);

 	/* Clean previous status and enable the watchdog timer */
-	control_status_reg = ioread32(xdev.base + XWT_TWCSR0_OFFSET);
+	control_status_reg = ioread32(xdev->base + XWT_TWCSR0_OFFSET);
 	control_status_reg |= (XWT_CSR0_WRS_MASK | XWT_CSR0_WDS_MASK);

 	iowrite32((control_status_reg | XWT_CSR0_EWDT1_MASK),
-				xdev.base + XWT_TWCSR0_OFFSET);
+		  xdev->base + XWT_TWCSR0_OFFSET);

-	iowrite32(XWT_CSRX_EWDT2_MASK, xdev.base + XWT_TWCSR1_OFFSET);
+	iowrite32(XWT_CSRX_EWDT2_MASK, xdev->base + XWT_TWCSR1_OFFSET);

-	spin_unlock(&spinlock);
+	spin_unlock(&xdev->spinlock);

 	return 0;
 }
@@ -78,17 +75,18 @@ static int xilinx_wdt_start(struct watchdog_device *wdd)
 static int xilinx_wdt_stop(struct watchdog_device *wdd)
 {
 	u32 control_status_reg;
+	struct xwdt_device *xdev = watchdog_get_drvdata(wdd);

-	spin_lock(&spinlock);
+	spin_lock(&xdev->spinlock);

-	control_status_reg = ioread32(xdev.base + XWT_TWCSR0_OFFSET);
+	control_status_reg = ioread32(xdev->base + XWT_TWCSR0_OFFSET);

 	iowrite32((control_status_reg & ~XWT_CSR0_EWDT1_MASK),
-				xdev.base + XWT_TWCSR0_OFFSET);
+		  xdev->base + XWT_TWCSR0_OFFSET);

-	iowrite32(0, xdev.base + XWT_TWCSR1_OFFSET);
+	iowrite32(0, xdev->base + XWT_TWCSR1_OFFSET);

-	spin_unlock(&spinlock);
+	spin_unlock(&xdev->spinlock);
 	pr_info("Stopped!\n");

 	return 0;
@@ -97,14 +95,15 @@ static int xilinx_wdt_stop(struct watchdog_device *wdd)
 static int xilinx_wdt_keepalive(struct watchdog_device *wdd)
 {
 	u32 control_status_reg;
+	struct xwdt_device *xdev = watchdog_get_drvdata(wdd);

-	spin_lock(&spinlock);
+	spin_lock(&xdev->spinlock);

-	control_status_reg = ioread32(xdev.base + XWT_TWCSR0_OFFSET);
+	control_status_reg = ioread32(xdev->base + XWT_TWCSR0_OFFSET);
 	control_status_reg |= (XWT_CSR0_WRS_MASK | XWT_CSR0_WDS_MASK);
-	iowrite32(control_status_reg, xdev.base + XWT_TWCSR0_OFFSET);
+	iowrite32(control_status_reg, xdev->base + XWT_TWCSR0_OFFSET);

-	spin_unlock(&spinlock);
+	spin_unlock(&xdev->spinlock);

 	return 0;
 }
@@ -123,29 +122,24 @@ static const struct watchdog_ops xilinx_wdt_ops = {
 	.ping = xilinx_wdt_keepalive,
 };

-static struct watchdog_device xilinx_wdt_wdd = {
-	.info = &xilinx_wdt_ident,
-	.ops = &xilinx_wdt_ops,
-};
-
-static u32 xwdt_selftest(void)
+static u32 xwdt_selftest(struct xwdt_device *xdev)
 {
 	int i;
 	u32 timer_value1;
 	u32 timer_value2;

-	spin_lock(&spinlock);
+	spin_lock(&xdev->spinlock);

-	timer_value1 = ioread32(xdev.base + XWT_TBR_OFFSET);
-	timer_value2 = ioread32(xdev.base + XWT_TBR_OFFSET);
+	timer_value1 = ioread32(xdev->base + XWT_TBR_OFFSET);
+	timer_value2 = ioread32(xdev->base + XWT_TBR_OFFSET);

 	for (i = 0;
 		((i <= XWT_MAX_SELFTEST_LOOP_COUNT) &&
 			(timer_value2 == timer_value1)); i++) {
-		timer_value2 = ioread32(xdev.base + XWT_TBR_OFFSET);
+		timer_value2 = ioread32(xdev->base + XWT_TBR_OFFSET);
 	}

-	spin_unlock(&spinlock);
+	spin_unlock(&xdev->spinlock);

 	if (timer_value2 != timer_value1)
 		return ~XWT_TIMER_FAILED;
@@ -159,12 +153,23 @@ static int xwdt_probe(struct platform_device *pdev)
 	u32 *tmptr;
 	u32 *pfreq;
 	struct resource *res;
+	struct xwdt_device *xdev;
 	bool no_timeout = false;
+	struct watchdog_device *xilinx_wdt_wdd;
+
+	xdev = devm_kzalloc(&pdev->dev, sizeof(*xdev), GFP_KERNEL);
+	if (!xdev)
+		return -ENOMEM;
+
+	xilinx_wdt_wdd = &xdev->xilinx_wdt_wdd;
+	xilinx_wdt_wdd->info = &xilinx_wdt_ident;
+	xilinx_wdt_wdd->ops = &xilinx_wdt_ops;
+	xilinx_wdt_wdd->parent = &pdev->dev;

 	res = platform_get_resource(pdev, IORESOURCE_MEM, 0);
-	xdev.base = devm_ioremap_resource(&pdev->dev, res);
-	if (IS_ERR(xdev.base))
-		return PTR_ERR(xdev.base);
+	xdev->base = devm_ioremap_resource(&pdev->dev, res);
+	if (IS_ERR(xdev->base))
+		return PTR_ERR(xdev->base);

 	pfreq = (u32 *)of_get_property(pdev->dev.of_node,
 					"clock-frequency", NULL);
@@ -180,14 +185,14 @@ static int xwdt_probe(struct platform_device *pdev)
 		pr_warn("Parameter \"xlnx,wdt-interval\" not found in device tree!\n");
 		no_timeout = true;
 	} else {
-		xdev.wdt_interval = *tmptr;
+		xdev->wdt_interval = *tmptr;
 	}

 	tmptr = (u32 *)of_get_property(pdev->dev.of_node,
 					"xlnx,wdt-enable-once", NULL);
 	if (tmptr == NULL) {
 		pr_warn("Parameter \"xlnx,wdt-enable-once\" not found in device tree!\n");
-		watchdog_set_nowayout(&xilinx_wdt_wdd, true);
+		watchdog_set_nowayout(xilinx_wdt_wdd, true);
 	}

 /*
@@ -195,29 +200,37 @@ static int xwdt_probe(struct platform_device *pdev)
  *  ignored (interrupt), reset is only generated at second wdt overflow
  */
 	if (!no_timeout)
-		timeout = 2 * ((1<<xdev.wdt_interval) / *pfreq);
+		xilinx_wdt_wdd->timeout = 2 * ((1 << xdev->wdt_interval) /
+					  *pfreq);
+
+	spin_lock_init(&xdev->spinlock);
+	watchdog_set_drvdata(xilinx_wdt_wdd, xdev);

-	rc = xwdt_selftest();
+	rc = xwdt_selftest(xdev);
 	if (rc == XWT_TIMER_FAILED) {
 		pr_err("SelfTest routine error!\n");
 		return rc;
 	}

-	rc = watchdog_register_device(&xilinx_wdt_wdd);
+	rc = watchdog_register_device(xilinx_wdt_wdd);
 	if (rc) {
 		pr_err("cannot register watchdog (err=%d)\n", rc);
 		return rc;
 	}

 	dev_info(&pdev->dev, "Xilinx Watchdog Timer at %p with timeout %ds\n",
-		 xdev.base, timeout);
+		 xdev->base, xilinx_wdt_wdd->timeout);
+
+	platform_set_drvdata(pdev, xdev);

 	return 0;
 }

-static int xwdt_remove(struct platform_device *dev)
+static int xwdt_remove(struct platform_device *pdev)
 {
-	watchdog_unregister_device(&xilinx_wdt_wdd);
+	struct xwdt_device *xdev = platform_get_drvdata(pdev);
+
+	watchdog_unregister_device(&xdev->xilinx_wdt_wdd);

 	return 0;
 }
--
1.8.2.3


[-- Attachment #2: Type: application/pgp-signature, Size: 198 bytes --]

[PATCH 06/10] watchdog: xilinx: Fix all printk messages

[Michal Simek]

[-- Attachment #1: Type: text/plain, Size: 2279 bytes --]

Use dev_ functions for printk messages.

Signed-off-by: Michal Simek <michal.simek@xilinx.com>
---

 drivers/watchdog/of_xilinx_wdt.c | 16 ++++++++--------
 1 file changed, 8 insertions(+), 8 deletions(-)

diff --git a/drivers/watchdog/of_xilinx_wdt.c b/drivers/watchdog/of_xilinx_wdt.c
index d28bd3f..c229cc4 100644
--- a/drivers/watchdog/of_xilinx_wdt.c
+++ b/drivers/watchdog/of_xilinx_wdt.c
@@ -10,8 +10,6 @@
  * 2 of the License, or (at your option) any later version.
  */

-#define pr_fmt(fmt) KBUILD_MODNAME ": " fmt
-
 #include <linux/err.h>
 #include <linux/module.h>
 #include <linux/types.h>
@@ -42,7 +40,6 @@
 #define XWT_TIMER_FAILED            0xFFFFFFFF

 #define WATCHDOG_NAME     "Xilinx Watchdog"
-#define PFX WATCHDOG_NAME ": "

 struct xwdt_device {
 	void __iomem *base;
@@ -175,14 +172,16 @@ static int xwdt_probe(struct platform_device *pdev)
 					"clock-frequency", NULL);

 	if (pfreq == NULL) {
-		pr_warn("The watchdog clock frequency cannot be obtained!\n");
+		dev_warn(&pdev->dev,
+			 "The watchdog clock frequency cannot be obtained\n");
 		no_timeout = true;
 	}

 	tmptr = (u32 *)of_get_property(pdev->dev.of_node,
 					"xlnx,wdt-interval", NULL);
 	if (tmptr == NULL) {
-		pr_warn("Parameter \"xlnx,wdt-interval\" not found in device tree!\n");
+		dev_warn(&pdev->dev,
+			 "Parameter \"xlnx,wdt-interval\" not found\n");
 		no_timeout = true;
 	} else {
 		xdev->wdt_interval = *tmptr;
@@ -191,7 +190,8 @@ static int xwdt_probe(struct platform_device *pdev)
 	tmptr = (u32 *)of_get_property(pdev->dev.of_node,
 					"xlnx,wdt-enable-once", NULL);
 	if (tmptr == NULL) {
-		pr_warn("Parameter \"xlnx,wdt-enable-once\" not found in device tree!\n");
+		dev_warn(&pdev->dev,
+			 "Parameter \"xlnx,wdt-enable-once\" not found\n");
 		watchdog_set_nowayout(xilinx_wdt_wdd, true);
 	}

@@ -208,13 +208,13 @@ static int xwdt_probe(struct platform_device *pdev)

 	rc = xwdt_selftest(xdev);
 	if (rc == XWT_TIMER_FAILED) {
-		pr_err("SelfTest routine error!\n");
+		dev_err(&pdev->dev, "SelfTest routine error\n");
 		return rc;
 	}

 	rc = watchdog_register_device(xilinx_wdt_wdd);
 	if (rc) {
-		pr_err("cannot register watchdog (err=%d)\n", rc);
+		dev_err(&pdev->dev, "Cannot register watchdog (err=%d)\n", rc);
 		return rc;
 	}

--
1.8.2.3


[-- Attachment #2: Type: application/pgp-signature, Size: 198 bytes --]

[PATCH 07/10] watchdog: xilinx: Fix OF binding

[Michal Simek]

[-- Attachment #1: Type: text/plain, Size: 2142 bytes --]

Use of_property_read_u32 functions to clean OF probing.

Signed-off-by: Michal Simek <michal.simek@xilinx.com>
---

 drivers/watchdog/of_xilinx_wdt.c | 25 ++++++++++---------------
 1 file changed, 10 insertions(+), 15 deletions(-)

diff --git a/drivers/watchdog/of_xilinx_wdt.c b/drivers/watchdog/of_xilinx_wdt.c
index c229cc4..475440a6 100644
--- a/drivers/watchdog/of_xilinx_wdt.c
+++ b/drivers/watchdog/of_xilinx_wdt.c
@@ -147,8 +147,7 @@ static u32 xwdt_selftest(struct xwdt_device *xdev)
 static int xwdt_probe(struct platform_device *pdev)
 {
 	int rc;
-	u32 *tmptr;
-	u32 *pfreq;
+	u32 pfreq, enable_once;
 	struct resource *res;
 	struct xwdt_device *xdev;
 	bool no_timeout = false;
@@ -168,28 +167,24 @@ static int xwdt_probe(struct platform_device *pdev)
 	if (IS_ERR(xdev->base))
 		return PTR_ERR(xdev->base);

-	pfreq = (u32 *)of_get_property(pdev->dev.of_node,
-					"clock-frequency", NULL);
-
-	if (pfreq == NULL) {
+	rc = of_property_read_u32(pdev->dev.of_node, "clock-frequency", &pfreq);
+	if (rc) {
 		dev_warn(&pdev->dev,
 			 "The watchdog clock frequency cannot be obtained\n");
 		no_timeout = true;
 	}

-	tmptr = (u32 *)of_get_property(pdev->dev.of_node,
-					"xlnx,wdt-interval", NULL);
-	if (tmptr == NULL) {
+	rc = of_property_read_u32(pdev->dev.of_node, "xlnx,wdt-interval",
+				  &xdev->wdt_interval);
+	if (rc) {
 		dev_warn(&pdev->dev,
 			 "Parameter \"xlnx,wdt-interval\" not found\n");
 		no_timeout = true;
-	} else {
-		xdev->wdt_interval = *tmptr;
 	}

-	tmptr = (u32 *)of_get_property(pdev->dev.of_node,
-					"xlnx,wdt-enable-once", NULL);
-	if (tmptr == NULL) {
+	rc = of_property_read_u32(pdev->dev.of_node, "xlnx,wdt-enable-once",
+				  &enable_once);
+	if (!rc && enable_once) {
 		dev_warn(&pdev->dev,
 			 "Parameter \"xlnx,wdt-enable-once\" not found\n");
 		watchdog_set_nowayout(xilinx_wdt_wdd, true);
@@ -201,7 +196,7 @@ static int xwdt_probe(struct platform_device *pdev)
  */
 	if (!no_timeout)
 		xilinx_wdt_wdd->timeout = 2 * ((1 << xdev->wdt_interval) /
-					  *pfreq);
+					  pfreq);

 	spin_lock_init(&xdev->spinlock);
 	watchdog_set_drvdata(xilinx_wdt_wdd, xdev);
--
1.8.2.3


[-- Attachment #2: Type: application/pgp-signature, Size: 198 bytes --]

[PATCH 08/10] watchdog: xilinx: Use correct comment indentation

[Michal Simek]

[-- Attachment #1: Type: text/plain, Size: 922 bytes --]

No functional changes.

Signed-off-by: Michal Simek <michal.simek@xilinx.com>
---

 drivers/watchdog/of_xilinx_wdt.c | 8 ++++----
 1 file changed, 4 insertions(+), 4 deletions(-)

diff --git a/drivers/watchdog/of_xilinx_wdt.c b/drivers/watchdog/of_xilinx_wdt.c
index 475440a6..7948def 100644
--- a/drivers/watchdog/of_xilinx_wdt.c
+++ b/drivers/watchdog/of_xilinx_wdt.c
@@ -190,10 +190,10 @@ static int xwdt_probe(struct platform_device *pdev)
 		watchdog_set_nowayout(xilinx_wdt_wdd, true);
 	}

-/*
- *  Twice of the 2^wdt_interval / freq  because the first wdt overflow is
- *  ignored (interrupt), reset is only generated at second wdt overflow
- */
+	/*
+	 * Twice of the 2^wdt_interval / freq  because the first wdt overflow is
+	 * ignored (interrupt), reset is only generated at second wdt overflow
+	 */
 	if (!no_timeout)
 		xilinx_wdt_wdd->timeout = 2 * ((1 << xdev->wdt_interval) /
 					  pfreq);
--
1.8.2.3


[-- Attachment #2: Type: application/pgp-signature, Size: 198 bytes --]

[PATCH 09/10] watchdog: xilinx: Add missing binding

[Michal Simek]

[-- Attachment #1: Type: text/plain, Size: 1358 bytes --]

Document current driver binding.

Signed-off-by: Michal Simek <michal.simek@xilinx.com>
---

 .../devicetree/bindings/watchdog/of-xilinx-wdt.txt | 23 ++++++++++++++++++++++
 1 file changed, 23 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/watchdog/of-xilinx-wdt.txt

diff --git a/Documentation/devicetree/bindings/watchdog/of-xilinx-wdt.txt b/Documentation/devicetree/bindings/watchdog/of-xilinx-wdt.txt
new file mode 100644
index 0000000..6d63782
--- /dev/null
+++ b/Documentation/devicetree/bindings/watchdog/of-xilinx-wdt.txt
@@ -0,0 +1,23 @@
+Xilinx AXI/PLB soft-core watchdog Device Tree Bindings
+---------------------------------------------------------
+
+Required properties:
+- compatible		: Should be "xlnx,xps-timebase-wdt-1.00.a" or
+			  "xlnx,xps-timebase-wdt-1.01.a".
+- reg			: Physical base address and size
+
+Optional properties:
+- clock-frequency	: Frequency of clock in Hz
+- xlnx,wdt-enable-once	: 0 - Watchdog can be restarted
+			  1 - Watchdog can be enabled just once
+- xlnx,wdt-interval	: Watchdog timeout interval in 2^<val> clock cycles,
+			  <val> is integer from 8 to 31.
+
+Example:
+axi-timebase-wdt@40100000 {
+	clock-frequency = <50000000>;
+	compatible = "xlnx,xps-timebase-wdt-1.00.a";
+	reg = <0x40100000 0x10000>;
+	xlnx,wdt-enable-once = <0x0>;
+	xlnx,wdt-interval = <0x1b>;
+} ;
--
1.8.2.3


[-- Attachment #2: Type: application/pgp-signature, Size: 198 bytes --]

[PATCH 10/10] watchdog: xilinx: Enable this driver for Zynq

[Michal Simek]

[-- Attachment #1: Type: text/plain, Size: 1407 bytes --]

Enable this driver for Zynq.
Move it to architecture independent Kconfig part.

Signed-off-by: Michal Simek <michal.simek@xilinx.com>
---

Build tested by zero day testing system.
---
 drivers/watchdog/Kconfig | 22 +++++++++-------------
 1 file changed, 9 insertions(+), 13 deletions(-)

diff --git a/drivers/watchdog/Kconfig b/drivers/watchdog/Kconfig
index 9db5d3c..6120403 100644
--- a/drivers/watchdog/Kconfig
+++ b/drivers/watchdog/Kconfig
@@ -111,6 +111,15 @@ config WM8350_WATCHDOG
 	  Support for the watchdog in the WM8350 AudioPlus PMIC.  When
 	  the watchdog triggers the system will be reset.

+config XILINX_WATCHDOG
+	tristate "Xilinx Watchdog timer"
+	select WATCHDOG_CORE
+	help
+	  Watchdog driver for the xps_timebase_wdt ip core.
+
+	  To compile this driver as a module, choose M here: the
+	  module will be called of_xilinx_wdt.
+
 # ALPHA Architecture

 # ARM Architecture
@@ -1022,19 +1031,6 @@ config M54xx_WATCHDOG

 # MicroBlaze Architecture

-config XILINX_WATCHDOG
-	tristate "Xilinx Watchdog timer"
-	depends on MICROBLAZE
-	select WATCHDOG_CORE
-	---help---
-	  Watchdog driver for the xps_timebase_wdt ip core.
-
-	  IMPORTANT: The xps_timebase_wdt parent must have the property
-	  "clock-frequency" at device tree.
-
-	  To compile this driver as a module, choose M here: the
-	  module will be called of_xilinx_wdt.
-
 # MIPS Architecture

 config ATH79_WDT
--
1.8.2.3


[-- Attachment #2: Type: application/pgp-signature, Size: 198 bytes --]

Re: [PATCH 10/10] watchdog: xilinx: Enable this driver for Zynq

[Guenter Roeck]

On 01/31/2014 06:18 AM, Michal Simek wrote:
> Enable this driver for Zynq.
> Move it to architecture independent Kconfig part.
>
> Signed-off-by: Michal Simek <michal.simek@xilinx.com>
> ---
>
> Build tested by zero day testing system.
> ---
>   drivers/watchdog/Kconfig | 22 +++++++++-------------
>   1 file changed, 9 insertions(+), 13 deletions(-)
>
> diff --git a/drivers/watchdog/Kconfig b/drivers/watchdog/Kconfig
> index 9db5d3c..6120403 100644
> --- a/drivers/watchdog/Kconfig
> +++ b/drivers/watchdog/Kconfig
> @@ -111,6 +111,15 @@ config WM8350_WATCHDOG
>   	  Support for the watchdog in the WM8350 AudioPlus PMIC.  When
>   	  the watchdog triggers the system will be reset.
>
> +config XILINX_WATCHDOG
> +	tristate "Xilinx Watchdog timer"
> +	select WATCHDOG_CORE

This needs to depend on HAS_IOMEM.

Thanks,
Guenter

Re: [PATCH 07/10] watchdog: xilinx: Fix OF binding

[Rob Herring]

On Fri, Jan 31, 2014 at 8:18 AM, Michal Simek <michal.simek@xilinx.com> wrote:
> Use of_property_read_u32 functions to clean OF probing.

The subject is a bit misleading as this doesn't really fix anything.

>
> Signed-off-by: Michal Simek <michal.simek@xilinx.com>
> ---
>
>  drivers/watchdog/of_xilinx_wdt.c | 25 ++++++++++---------------
>  1 file changed, 10 insertions(+), 15 deletions(-)
>
> diff --git a/drivers/watchdog/of_xilinx_wdt.c b/drivers/watchdog/of_xilinx_wdt.c
> index c229cc4..475440a6 100644
> --- a/drivers/watchdog/of_xilinx_wdt.c
> +++ b/drivers/watchdog/of_xilinx_wdt.c
> @@ -147,8 +147,7 @@ static u32 xwdt_selftest(struct xwdt_device *xdev)
>  static int xwdt_probe(struct platform_device *pdev)
>  {
>         int rc;
> -       u32 *tmptr;
> -       u32 *pfreq;
> +       u32 pfreq, enable_once;
>         struct resource *res;
>         struct xwdt_device *xdev;
>         bool no_timeout = false;
> @@ -168,28 +167,24 @@ static int xwdt_probe(struct platform_device *pdev)
>         if (IS_ERR(xdev->base))
>                 return PTR_ERR(xdev->base);
>
> -       pfreq = (u32 *)of_get_property(pdev->dev.of_node,
> -                                       "clock-frequency", NULL);
> -
> -       if (pfreq == NULL) {
> +       rc = of_property_read_u32(pdev->dev.of_node, "clock-frequency", &pfreq);
> +       if (rc) {
>                 dev_warn(&pdev->dev,
>                          "The watchdog clock frequency cannot be obtained\n");
>                 no_timeout = true;

You can kill this...

>         }
>
> -       tmptr = (u32 *)of_get_property(pdev->dev.of_node,
> -                                       "xlnx,wdt-interval", NULL);
> -       if (tmptr == NULL) {
> +       rc = of_property_read_u32(pdev->dev.of_node, "xlnx,wdt-interval",
> +                                 &xdev->wdt_interval);
> +       if (rc) {
>                 dev_warn(&pdev->dev,
>                          "Parameter \"xlnx,wdt-interval\" not found\n");
>                 no_timeout = true;

and this...

> -       } else {
> -               xdev->wdt_interval = *tmptr;
>         }
>
> -       tmptr = (u32 *)of_get_property(pdev->dev.of_node,
> -                                       "xlnx,wdt-enable-once", NULL);
> -       if (tmptr == NULL) {
> +       rc = of_property_read_u32(pdev->dev.of_node, "xlnx,wdt-enable-once",
> +                                 &enable_once);
> +       if (!rc && enable_once) {
>                 dev_warn(&pdev->dev,
>                          "Parameter \"xlnx,wdt-enable-once\" not found\n");
>                 watchdog_set_nowayout(xilinx_wdt_wdd, true);
> @@ -201,7 +196,7 @@ static int xwdt_probe(struct platform_device *pdev)
>   */
>         if (!no_timeout)

and use "if (pfreq && xdev->wdt_interval)" if you initialize pfreq to 0.

>                 xilinx_wdt_wdd->timeout = 2 * ((1 << xdev->wdt_interval) /
> -                                         *pfreq);
> +                                         pfreq);

Is the wdog really usable if the timeout properties are missing? Seems
like you should fail to probe rather than warn.

Rob

Re: [PATCH 10/10] watchdog: xilinx: Enable this driver for Zynq

[Michal Simek]

[-- Attachment #1: Type: text/plain, Size: 7511 bytes --]

On 01/31/2014 03:52 PM, Guenter Roeck wrote:
> On 01/31/2014 06:18 AM, Michal Simek wrote:
>> Enable this driver for Zynq.
>> Move it to architecture independent Kconfig part.
>>
>> Signed-off-by: Michal Simek <michal.simek@xilinx.com>
>> ---
>>
>> Build tested by zero day testing system.
>> ---
>>   drivers/watchdog/Kconfig | 22 +++++++++-------------
>>   1 file changed, 9 insertions(+), 13 deletions(-)
>>
>> diff --git a/drivers/watchdog/Kconfig b/drivers/watchdog/Kconfig
>> index 9db5d3c..6120403 100644
>> --- a/drivers/watchdog/Kconfig
>> +++ b/drivers/watchdog/Kconfig
>> @@ -111,6 +111,15 @@ config WM8350_WATCHDOG
>>         Support for the watchdog in the WM8350 AudioPlus PMIC.  When
>>         the watchdog triggers the system will be reset.
>>
>> +config XILINX_WATCHDOG
>> +    tristate "Xilinx Watchdog timer"
>> +    select WATCHDOG_CORE
> 
> This needs to depend on HAS_IOMEM.

Are you sure?
I have no problem to do this change.
Zero day testing system doesn't report any problem with it.

I have checked dependencies and only score, tile and um has NO_IOMEM option
enables. And below in log is tile with allyesconfig that's why I believe
this driver has been also tested without any issue.

Thanks,
Michal


git://git.monstr.eu/linux-2.6-microblaze  xnext/watchdog
f7bdfada576e93eaab8f6dc2ecd881da8f43911c  watchdog: xilinx: Enable this driver for Zynq

elapsed time: 81m

configs tested: 122

alpha                               defconfig
parisc                            allnoconfig
parisc                         b180_defconfig
parisc                        c3000_defconfig
parisc                              defconfig
arm                               allnoconfig
arm                               almodconfig
arm                         at91_dt_defconfig
arm                       imx_v6_v7_defconfig
arm                          marzen_defconfig
arm                       omap2plus_defconfig
arm                          prima2_defconfig
arm                         s3c2410_defconfig
arm                       spear13xx_defconfig
arm                           tegra_defconfig
m32r                       m32104ut_defconfig
m32r                     mappi3.smp_defconfig
m32r                         opsput_defconfig
m32r                           usrv_defconfig
xtensa                       common_defconfig
xtensa                          iss_defconfig
x86_64                            allnoconfig
sh                                allnoconfig
sh                          rsk7269_defconfig
sh                  sh7785lcr_32bit_defconfig
sh                            titan_defconfig
x86_64                     randconfig-c0-0131
x86_64                     randconfig-c1-0131
x86_64                     randconfig-c2-0131
x86_64                     randconfig-c3-0131
x86_64                     randconfig-c4-0131
x86_64                     randconfig-c5-0131
x86_64                     randconfig-c6-0131
x86_64                     randconfig-c7-0131
x86_64                     randconfig-c8-0131
x86_64                     randconfig-c9-0131
x86_64                           allyesconfig
alpha                            allyesconfig
avr32                            allyesconfig
blackfin                         allyesconfig
cris                             allyesconfig
ia64                             allyesconfig
m68k                             allyesconfig
mips                             allyesconfig
parisc                           allyesconfig
powerpc                          allyesconfig
s390                             allyesconfig
sh                               allyesconfig
sparc                            allyesconfig
sparc64                          allyesconfig
tile                             allyesconfig
xtensa                           allyesconfig
ia64                             alldefconfig
ia64                             allmodconfig
ia64                              allnoconfig
ia64                                defconfig
x86_64                                    lkp
powerpc                      chroma_defconfig
powerpc               corenet64_smp_defconfig
powerpc                    gamecube_defconfig
powerpc                 linkstation_defconfig
powerpc                         wii_defconfig
x86_64                     randconfig-j0-0131
x86_64                     randconfig-j1-0131
x86_64                     randconfig-j2-0131
x86_64                     randconfig-j3-0131
x86_64                     randconfig-j4-0131
x86_64                     randconfig-j5-0131
m68k                             allmodconfig
m68k                          amiga_defconfig
m68k                       m5475evb_defconfig
m68k                          multi_defconfig
blackfin                BF526-EZBRD_defconfig
blackfin                BF533-EZKIT_defconfig
blackfin            BF561-EZKIT-SMP_defconfig
blackfin                  TCM-BF537_defconfig
cris                 etrax-100lx_v2_defconfig
i386                       randconfig-r0-0131
i386                       randconfig-r1-0131
i386                       randconfig-r2-0131
i386                       randconfig-r3-0131
i386                       randconfig-r4-0131
i386                       randconfig-r5-0131
s390                             allmodconfig
s390                              allnoconfig
s390                                defconfig
i386                             allyesconfig
x86_64                           allmodconfig
i386                             alldefconfig
i386                             allmodconfig
i386                              allnoconfig
i386                                defconfig
x86_64                        randconfig-x000
x86_64                        randconfig-x001
x86_64                        randconfig-x002
x86_64                        randconfig-x003
x86_64                        randconfig-x004
x86_64                        randconfig-x005
x86_64                        randconfig-x006
x86_64                        randconfig-x007
x86_64                        randconfig-x008
x86_64                        randconfig-x009
i386                          randconfig-x000
i386                          randconfig-x001
i386                          randconfig-x002
i386                          randconfig-x003
i386                          randconfig-x004
i386                          randconfig-x005
i386                          randconfig-x006
i386                          randconfig-x007
i386                          randconfig-x008
i386                          randconfig-x009
powerpc                          allmodconfig
powerpc                           allnoconfig
powerpc                             defconfig
powerpc                       ppc64_defconfig
x86_64                             acpi-redef
x86_64                           allyesdebian
x86_64                                nfsroot
microblaze                       allyesconfig
microblaze                      mmu_defconfig
microblaze                    nommu_defconfig



-- 
Michal Simek, Ing. (M.Eng), OpenPGP -> KeyID: FE3D1F91
w: www.monstr.eu p: +42-0-721842854
Maintainer of Linux kernel - Microblaze cpu - http://www.monstr.eu/fdt/
Maintainer of Linux kernel - Xilinx Zynq ARM architecture
Microblaze U-BOOT custodian and responsible for u-boot arm zynq platform



[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 263 bytes --]

Re: [PATCH 07/10] watchdog: xilinx: Fix OF binding

[Michal Simek]

[-- Attachment #1: Type: text/plain, Size: 4214 bytes --]

On 01/31/2014 06:33 PM, Rob Herring wrote:
> On Fri, Jan 31, 2014 at 8:18 AM, Michal Simek <michal.simek@xilinx.com> wrote:
>> Use of_property_read_u32 functions to clean OF probing.
> 
> The subject is a bit misleading as this doesn't really fix anything.

fair enough. Will change it.

> 
>>
>> Signed-off-by: Michal Simek <michal.simek@xilinx.com>
>> ---
>>
>>  drivers/watchdog/of_xilinx_wdt.c | 25 ++++++++++---------------
>>  1 file changed, 10 insertions(+), 15 deletions(-)
>>
>> diff --git a/drivers/watchdog/of_xilinx_wdt.c b/drivers/watchdog/of_xilinx_wdt.c
>> index c229cc4..475440a6 100644
>> --- a/drivers/watchdog/of_xilinx_wdt.c
>> +++ b/drivers/watchdog/of_xilinx_wdt.c
>> @@ -147,8 +147,7 @@ static u32 xwdt_selftest(struct xwdt_device *xdev)
>>  static int xwdt_probe(struct platform_device *pdev)
>>  {
>>         int rc;
>> -       u32 *tmptr;
>> -       u32 *pfreq;
>> +       u32 pfreq, enable_once;
>>         struct resource *res;
>>         struct xwdt_device *xdev;
>>         bool no_timeout = false;
>> @@ -168,28 +167,24 @@ static int xwdt_probe(struct platform_device *pdev)
>>         if (IS_ERR(xdev->base))
>>                 return PTR_ERR(xdev->base);
>>
>> -       pfreq = (u32 *)of_get_property(pdev->dev.of_node,
>> -                                       "clock-frequency", NULL);
>> -
>> -       if (pfreq == NULL) {
>> +       rc = of_property_read_u32(pdev->dev.of_node, "clock-frequency", &pfreq);
>> +       if (rc) {
>>                 dev_warn(&pdev->dev,
>>                          "The watchdog clock frequency cannot be obtained\n");
>>                 no_timeout = true;
> 
> You can kill this...
> 
>>         }
>>
>> -       tmptr = (u32 *)of_get_property(pdev->dev.of_node,
>> -                                       "xlnx,wdt-interval", NULL);
>> -       if (tmptr == NULL) {
>> +       rc = of_property_read_u32(pdev->dev.of_node, "xlnx,wdt-interval",
>> +                                 &xdev->wdt_interval);
>> +       if (rc) {
>>                 dev_warn(&pdev->dev,
>>                          "Parameter \"xlnx,wdt-interval\" not found\n");
>>                 no_timeout = true;
> 
> and this...
> 
>> -       } else {
>> -               xdev->wdt_interval = *tmptr;
>>         }
>>
>> -       tmptr = (u32 *)of_get_property(pdev->dev.of_node,
>> -                                       "xlnx,wdt-enable-once", NULL);
>> -       if (tmptr == NULL) {
>> +       rc = of_property_read_u32(pdev->dev.of_node, "xlnx,wdt-enable-once",
>> +                                 &enable_once);
>> +       if (!rc && enable_once) {
>>                 dev_warn(&pdev->dev,
>>                          "Parameter \"xlnx,wdt-enable-once\" not found\n");
>>                 watchdog_set_nowayout(xilinx_wdt_wdd, true);
>> @@ -201,7 +196,7 @@ static int xwdt_probe(struct platform_device *pdev)
>>   */
>>         if (!no_timeout)
> 
> and use "if (pfreq && xdev->wdt_interval)" if you initialize pfreq to 0.


I just wanted to to change functions not logic in the driver.
But it can be changed too.

>>                 xilinx_wdt_wdd->timeout = 2 * ((1 << xdev->wdt_interval) /
>> -                                         *pfreq);
>> +                                         pfreq);
> 
> Is the wdog really usable if the timeout properties are missing? Seems
> like you should fail to probe rather than warn.

There are 2 things.
1. timeout - you don't need pfreq and wdt_interval to use this driver
but for that there should be module parameter timeout there.
It should be added.

2. about warn - based on 1 I don't think driver should failed
but I am looking at logic above which I have added there but should be different.

u32 enable_once = 0;
if (!rc)
	dev_warn

if (enable_once)
	watchdog_set_nowayout(xilinx_wdt_wdd, true);

Thanks,
Michal

-- 
Michal Simek, Ing. (M.Eng), OpenPGP -> KeyID: FE3D1F91
w: www.monstr.eu p: +42-0-721842854
Maintainer of Linux kernel - Microblaze cpu - http://www.monstr.eu/fdt/
Maintainer of Linux kernel - Xilinx Zynq ARM architecture
Microblaze U-BOOT custodian and responsible for u-boot arm zynq platform



[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 263 bytes --]

Re: [PATCH 07/10] watchdog: xilinx: Fix OF binding

[Michal Simek]

[-- Attachment #1: Type: text/plain, Size: 4333 bytes --]

On 02/03/2014 08:59 AM, Michal Simek wrote:
> On 01/31/2014 06:33 PM, Rob Herring wrote:
>> On Fri, Jan 31, 2014 at 8:18 AM, Michal Simek <michal.simek@xilinx.com> wrote:
>>> Use of_property_read_u32 functions to clean OF probing.
>>
>> The subject is a bit misleading as this doesn't really fix anything.
> 
> fair enough. Will change it.
> 
>>
>>>
>>> Signed-off-by: Michal Simek <michal.simek@xilinx.com>
>>> ---
>>>
>>>  drivers/watchdog/of_xilinx_wdt.c | 25 ++++++++++---------------
>>>  1 file changed, 10 insertions(+), 15 deletions(-)
>>>
>>> diff --git a/drivers/watchdog/of_xilinx_wdt.c b/drivers/watchdog/of_xilinx_wdt.c
>>> index c229cc4..475440a6 100644
>>> --- a/drivers/watchdog/of_xilinx_wdt.c
>>> +++ b/drivers/watchdog/of_xilinx_wdt.c
>>> @@ -147,8 +147,7 @@ static u32 xwdt_selftest(struct xwdt_device *xdev)
>>>  static int xwdt_probe(struct platform_device *pdev)
>>>  {
>>>         int rc;
>>> -       u32 *tmptr;
>>> -       u32 *pfreq;
>>> +       u32 pfreq, enable_once;
>>>         struct resource *res;
>>>         struct xwdt_device *xdev;
>>>         bool no_timeout = false;
>>> @@ -168,28 +167,24 @@ static int xwdt_probe(struct platform_device *pdev)
>>>         if (IS_ERR(xdev->base))
>>>                 return PTR_ERR(xdev->base);
>>>
>>> -       pfreq = (u32 *)of_get_property(pdev->dev.of_node,
>>> -                                       "clock-frequency", NULL);
>>> -
>>> -       if (pfreq == NULL) {
>>> +       rc = of_property_read_u32(pdev->dev.of_node, "clock-frequency", &pfreq);
>>> +       if (rc) {
>>>                 dev_warn(&pdev->dev,
>>>                          "The watchdog clock frequency cannot be obtained\n");
>>>                 no_timeout = true;
>>
>> You can kill this...
>>
>>>         }
>>>
>>> -       tmptr = (u32 *)of_get_property(pdev->dev.of_node,
>>> -                                       "xlnx,wdt-interval", NULL);
>>> -       if (tmptr == NULL) {
>>> +       rc = of_property_read_u32(pdev->dev.of_node, "xlnx,wdt-interval",
>>> +                                 &xdev->wdt_interval);
>>> +       if (rc) {
>>>                 dev_warn(&pdev->dev,
>>>                          "Parameter \"xlnx,wdt-interval\" not found\n");
>>>                 no_timeout = true;
>>
>> and this...
>>
>>> -       } else {
>>> -               xdev->wdt_interval = *tmptr;
>>>         }
>>>
>>> -       tmptr = (u32 *)of_get_property(pdev->dev.of_node,
>>> -                                       "xlnx,wdt-enable-once", NULL);
>>> -       if (tmptr == NULL) {
>>> +       rc = of_property_read_u32(pdev->dev.of_node, "xlnx,wdt-enable-once",
>>> +                                 &enable_once);
>>> +       if (!rc && enable_once) {
>>>                 dev_warn(&pdev->dev,
>>>                          "Parameter \"xlnx,wdt-enable-once\" not found\n");
>>>                 watchdog_set_nowayout(xilinx_wdt_wdd, true);
>>> @@ -201,7 +196,7 @@ static int xwdt_probe(struct platform_device *pdev)
>>>   */
>>>         if (!no_timeout)
>>
>> and use "if (pfreq && xdev->wdt_interval)" if you initialize pfreq to 0.
> 
> 
> I just wanted to to change functions not logic in the driver.
> But it can be changed too.
> 
>>>                 xilinx_wdt_wdd->timeout = 2 * ((1 << xdev->wdt_interval) /
>>> -                                         *pfreq);
>>> +                                         pfreq);
>>
>> Is the wdog really usable if the timeout properties are missing? Seems
>> like you should fail to probe rather than warn.
> 
> There are 2 things.
> 1. timeout - you don't need pfreq and wdt_interval to use this driver
> but for that there should be module parameter timeout there.
> It should be added.
> 
> 2. about warn - based on 1 I don't think driver should failed
> but I am looking at logic above which I have added there but should be different.
> 
> u32 enable_once = 0;
> if (!rc)
> 	dev_warn
> 

if (rc) here sorry.

Thanks,
Michal

-- 
Michal Simek, Ing. (M.Eng), OpenPGP -> KeyID: FE3D1F91
w: www.monstr.eu p: +42-0-721842854
Maintainer of Linux kernel - Microblaze cpu - http://www.monstr.eu/fdt/
Maintainer of Linux kernel - Xilinx Zynq ARM architecture
Microblaze U-BOOT custodian and responsible for u-boot arm zynq platform



[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 263 bytes --]

Re: [PATCH 10/10] watchdog: xilinx: Enable this driver for Zynq

[Guenter Roeck]

On 02/02/2014 11:01 PM, Michal Simek wrote:
> On 01/31/2014 03:52 PM, Guenter Roeck wrote:
>> On 01/31/2014 06:18 AM, Michal Simek wrote:
>>> Enable this driver for Zynq.
>>> Move it to architecture independent Kconfig part.
>>>
>>> Signed-off-by: Michal Simek <michal.simek@xilinx.com>
>>> ---
>>>
>>> Build tested by zero day testing system.
>>> ---
>>>    drivers/watchdog/Kconfig | 22 +++++++++-------------
>>>    1 file changed, 9 insertions(+), 13 deletions(-)
>>>
>>> diff --git a/drivers/watchdog/Kconfig b/drivers/watchdog/Kconfig
>>> index 9db5d3c..6120403 100644
>>> --- a/drivers/watchdog/Kconfig
>>> +++ b/drivers/watchdog/Kconfig
>>> @@ -111,6 +111,15 @@ config WM8350_WATCHDOG
>>>          Support for the watchdog in the WM8350 AudioPlus PMIC.  When
>>>          the watchdog triggers the system will be reset.
>>>
>>> +config XILINX_WATCHDOG
>>> +    tristate "Xilinx Watchdog timer"
>>> +    select WATCHDOG_CORE
>>
>> This needs to depend on HAS_IOMEM.
>
> Are you sure?
> I have no problem to do this change.
> Zero day testing system doesn't report any problem with it.
>
> I have checked dependencies and only score, tile and um has NO_IOMEM option
> enables. And below in log is tile with allyesconfig that's why I believe
> this driver has been also tested without any issue.
>

https://lkml.org/lkml/2014/1/31/123

I would have assumed the same applies here, but if you are sure that it doesn't I am fine.

Thanks,
Guenter

Re: [PATCH 10/10] watchdog: xilinx: Enable this driver for Zynq

[Michal Simek]

[-- Attachment #1: Type: text/plain, Size: 1988 bytes --]

On 02/03/2014 09:26 AM, Guenter Roeck wrote:
> On 02/02/2014 11:01 PM, Michal Simek wrote:
>> On 01/31/2014 03:52 PM, Guenter Roeck wrote:
>>> On 01/31/2014 06:18 AM, Michal Simek wrote:
>>>> Enable this driver for Zynq.
>>>> Move it to architecture independent Kconfig part.
>>>>
>>>> Signed-off-by: Michal Simek <michal.simek@xilinx.com>
>>>> ---
>>>>
>>>> Build tested by zero day testing system.
>>>> ---
>>>>    drivers/watchdog/Kconfig | 22 +++++++++-------------
>>>>    1 file changed, 9 insertions(+), 13 deletions(-)
>>>>
>>>> diff --git a/drivers/watchdog/Kconfig b/drivers/watchdog/Kconfig
>>>> index 9db5d3c..6120403 100644
>>>> --- a/drivers/watchdog/Kconfig
>>>> +++ b/drivers/watchdog/Kconfig
>>>> @@ -111,6 +111,15 @@ config WM8350_WATCHDOG
>>>>          Support for the watchdog in the WM8350 AudioPlus PMIC.  When
>>>>          the watchdog triggers the system will be reset.
>>>>
>>>> +config XILINX_WATCHDOG
>>>> +    tristate "Xilinx Watchdog timer"
>>>> +    select WATCHDOG_CORE
>>>
>>> This needs to depend on HAS_IOMEM.
>>
>> Are you sure?
>> I have no problem to do this change.
>> Zero day testing system doesn't report any problem with it.
>>
>> I have checked dependencies and only score, tile and um has NO_IOMEM option
>> enables. And below in log is tile with allyesconfig that's why I believe
>> this driver has been also tested without any issue.
>>
> 
> https://lkml.org/lkml/2014/1/31/123
> 
> I would have assumed the same applies here, but if you are sure that it doesn't I am fine.

I am not 100% sure because I didn't compile it myself for that 3 archs
but one was just fine.

Thanks,
Michal
-- 
Michal Simek, Ing. (M.Eng), OpenPGP -> KeyID: FE3D1F91
w: www.monstr.eu p: +42-0-721842854
Maintainer of Linux kernel - Microblaze cpu - http://www.monstr.eu/fdt/
Maintainer of Linux kernel - Xilinx Zynq ARM architecture
Microblaze U-BOOT custodian and responsible for u-boot arm zynq platform



[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 263 bytes --]

Re: [PATCH 09/10] watchdog: xilinx: Add missing binding

[Arnd Bergmann]

On Friday 31 January 2014, Michal Simek wrote:
> +Optional properties:
> +- clock-frequency      : Frequency of clock in Hz
> +- xlnx,wdt-enable-once : 0 - Watchdog can be restarted
> +                         1 - Watchdog can be enabled just once
> +- xlnx,wdt-interval    : Watchdog timeout interval in 2^<val> clock cycles,
> +                         <val> is integer from 8 to 31.
> +

The latter two don't really seem to be xilinx specific, it would be
reasonable to have a standard watchdog binding that mandates a common
format for them.

I'm not sure about the enable-once flag, which seems to just map to the
"nowayout" watchdog option that is not a hardware feature at all
and should probably be kept as a software setting only, rather than
settable through DT. If it is kept, it should have a standard name and
get turned into a boolean (present/absent) property rather than a
0/1 integer property.

The interval should really be specified in terms of seconds or miliseconds,
not in clock cycles.

	Arnd

Re: [PATCH 09/10] watchdog: xilinx: Add missing binding

[Michal Simek]

On 02/03/2014 04:06 PM, Arnd Bergmann wrote:
> On Friday 31 January 2014, Michal Simek wrote:
>> +Optional properties:
>> +- clock-frequency      : Frequency of clock in Hz
>> +- xlnx,wdt-enable-once : 0 - Watchdog can be restarted
>> +                         1 - Watchdog can be enabled just once
>> +- xlnx,wdt-interval    : Watchdog timeout interval in 2^<val> clock cycles,
>> +                         <val> is integer from 8 to 31.
>> +
> 
> The latter two don't really seem to be xilinx specific, it would be
> reasonable to have a standard watchdog binding that mandates a common
> format for them.
> 
> I'm not sure about the enable-once flag, which seems to just map to the
> "nowayout" watchdog option that is not a hardware feature at all
> and should probably be kept as a software setting only, rather than
> settable through DT. If it is kept, it should have a standard name and
> get turned into a boolean (present/absent) property rather than a
> 0/1 integer property.
> 
> The interval should really be specified in terms of seconds or miliseconds,
> not in clock cycles.

Intention wasn't to fix binding but document current one
which is in mainline for a long time.

Apart of this - yes, wdt-enable-once is nowayout and wdt-interval should be timeout
is seconds, and clock-frequency should go out and use CCF for getting clock.

Thanks,
Michal

Re: [PATCH 09/10] watchdog: xilinx: Add missing binding

[Arnd Bergmann]

On Monday 03 February 2014 16:13:47 Michal Simek wrote:
> Intention wasn't to fix binding but document current one
> which is in mainline for a long time.

Ok, I see.

> Apart of this - yes, wdt-enable-once is nowayout and wdt-interval should be timeout
> is seconds, and clock-frequency should go out and use CCF for getting clock.

Could we make a common binding then, and document that the xilinx
watchdog can optionally provide either one?

	Arnd

Re: [PATCH 09/10] watchdog: xilinx: Add missing binding

[Michal Simek]

On 02/03/2014 04:32 PM, Arnd Bergmann wrote:
> On Monday 03 February 2014 16:13:47 Michal Simek wrote:
>> Intention wasn't to fix binding but document current one
>> which is in mainline for a long time.
> 
> Ok, I see.
> 
>> Apart of this - yes, wdt-enable-once is nowayout and wdt-interval should be timeout
>> is seconds, and clock-frequency should go out and use CCF for getting clock.
> 
> Could we make a common binding then, and document that the xilinx
> watchdog can optionally provide either one?

Do you mean to have 2 DT bindings?

This binding is used from 2011-07.
It means it was generated for all hw designs at least from this time.
I would say from DT usage on Microblaze because it is not special case
in our dt generator.

xlnx,XXX are XXX parameters which you have to setup in tools
and get synthesized. This is valid for all xilinx IPs. We have full
IP description by generating xlnx,XXX parameters directly from tools
because we know all variants which can happen.

Just back to your previous post:
"I'm not sure about the enable-once flag, which seems to just map to the
"nowayout" watchdog option that is not a hardware feature at all"
this is hw feature which you can select in tools because this is fpga. :-)

Thanks,
Michal

Re: [PATCH 09/10] watchdog: xilinx: Add missing binding

[Arnd Bergmann]

On Monday 03 February 2014, Michal Simek wrote:
> On 02/03/2014 04:32 PM, Arnd Bergmann wrote:
> > On Monday 03 February 2014 16:13:47 Michal Simek wrote:
> >> Intention wasn't to fix binding but document current one
> >> which is in mainline for a long time.
> > 
> > Ok, I see.
> > 
> >> Apart of this - yes, wdt-enable-once is nowayout and wdt-interval should be timeout
> >> is seconds, and clock-frequency should go out and use CCF for getting clock.
> > 
> > Could we make a common binding then, and document that the xilinx
> > watchdog can optionally provide either one?
> 
> Do you mean to have 2 DT bindings?
> 
> This binding is used from 2011-07.
> It means it was generated for all hw designs at least from this time.
> I would say from DT usage on Microblaze because it is not special case
> in our dt generator.

I certainly wasn't suggesting to break the binding, quite the contrary.
What I tried to say is that the properties look like they should be
useful for different kinds of watchdogs, not just xilinx, so it would
be good to have a common definition using generic strings.

The xilinx driver would definitely have to keep supporting the traditional
property names, but it could also support the generic names in the
future.

> xlnx,XXX are XXX parameters which you have to setup in tools
> and get synthesized. This is valid for all xilinx IPs. We have full
> IP description by generating xlnx,XXX parameters directly from tools
> because we know all variants which can happen.
>
> Just back to your previous post:
> "I'm not sure about the enable-once flag, which seems to just map to the
> "nowayout" watchdog option that is not a hardware feature at all"
> this is hw feature which you can select in tools because this is fpga. :-)

Ah, so you mean the properties are not settings that the driver
programs into the hardware, but they are hardware properties that the
driver reports to user space?

	Arnd

Re: [PATCH 09/10] watchdog: xilinx: Add missing binding

[Michal Simek]

[-- Attachment #1: Type: text/plain, Size: 2898 bytes --]

On 02/04/2014 08:27 PM, Arnd Bergmann wrote:
> On Monday 03 February 2014, Michal Simek wrote:
>> On 02/03/2014 04:32 PM, Arnd Bergmann wrote:
>>> On Monday 03 February 2014 16:13:47 Michal Simek wrote:
>>>> Intention wasn't to fix binding but document current one
>>>> which is in mainline for a long time.
>>>
>>> Ok, I see.
>>>
>>>> Apart of this - yes, wdt-enable-once is nowayout and wdt-interval should be timeout
>>>> is seconds, and clock-frequency should go out and use CCF for getting clock.
>>>
>>> Could we make a common binding then, and document that the xilinx
>>> watchdog can optionally provide either one?
>>
>> Do you mean to have 2 DT bindings?
>>
>> This binding is used from 2011-07.
>> It means it was generated for all hw designs at least from this time.
>> I would say from DT usage on Microblaze because it is not special case
>> in our dt generator.
> 
> I certainly wasn't suggesting to break the binding, quite the contrary.
> What I tried to say is that the properties look like they should be
> useful for different kinds of watchdogs, not just xilinx, so it would
> be good to have a common definition using generic strings.
> 
> The xilinx driver would definitely have to keep supporting the traditional
> property names, but it could also support the generic names in the
> future.

No problem with to do in future.

>> xlnx,XXX are XXX parameters which you have to setup in tools
>> and get synthesized. This is valid for all xilinx IPs. We have full
>> IP description by generating xlnx,XXX parameters directly from tools
>> because we know all variants which can happen.
>>
>> Just back to your previous post:
>> "I'm not sure about the enable-once flag, which seems to just map to the
>> "nowayout" watchdog option that is not a hardware feature at all"
>> this is hw feature which you can select in tools because this is fpga. :-)
> 
> Ah, so you mean the properties are not settings that the driver
> programs into the hardware, but they are hardware properties that the
> driver reports to user space?

yes, they are hardware properties which you can choose based on your
configuration. Every user just decides if this watchdog can be started just
once and how long it takes in synthesis time. There is no option to program
this by software.

I am not quite sure what you mean by reports to user space.
If you mean to get timeout through ioctl for example - then yes it is working
through standard watchdog ioctl interface and timeout is calculated
from hardware setup.

Thanks,
Michal


-- 
Michal Simek, Ing. (M.Eng), OpenPGP -> KeyID: FE3D1F91
w: www.monstr.eu p: +42-0-721842854
Maintainer of Linux kernel - Microblaze cpu - http://www.monstr.eu/fdt/
Maintainer of Linux kernel - Xilinx Zynq ARM architecture
Microblaze U-BOOT custodian and responsible for u-boot arm zynq platform



[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 263 bytes --]

Re: [PATCH 09/10] watchdog: xilinx: Add missing binding

[Arnd Bergmann]

On Wednesday 05 February 2014, Michal Simek wrote:
> I am not quite sure what you mean by reports to user space.
> If you mean to get timeout through ioctl for example - then yes it is working
> through standard watchdog ioctl interface and timeout is calculated
> from hardware setup.

Yes, that is what I meant. I believe most other watchdogs let
you program the timeout, but I don't see anything wrong with
having that fixed in the FPGA in your case.

Still, the choice of putting the timeout into DT in terms of
cycles rather than miliseconds wasn't ideal from an interface
perspective and we should change that if/when we do a generic
binding. I can definitely see where it's coming from for your
case, as the cycle count totally makes sense from an FPGA
tool perspective...

	Arnd

Re: [PATCH 09/10] watchdog: xilinx: Add missing binding

[Michal Simek]

[-- Attachment #1: Type: text/plain, Size: 1269 bytes --]

On 02/05/2014 10:36 AM, Arnd Bergmann wrote:
> On Wednesday 05 February 2014, Michal Simek wrote:
>> I am not quite sure what you mean by reports to user space.
>> If you mean to get timeout through ioctl for example - then yes it is working
>> through standard watchdog ioctl interface and timeout is calculated
>> from hardware setup.
> 
> Yes, that is what I meant. I believe most other watchdogs let
> you program the timeout, but I don't see anything wrong with
> having that fixed in the FPGA in your case.
> 
> Still, the choice of putting the timeout into DT in terms of
> cycles rather than miliseconds wasn't ideal from an interface
> perspective and we should change that if/when we do a generic
> binding. I can definitely see where it's coming from for your
> case, as the cycle count totally makes sense from an FPGA
> tool perspective...

Thanks. I take this like ACK for this current binding description.

Thanks,
Michal

-- 
Michal Simek, Ing. (M.Eng), OpenPGP -> KeyID: FE3D1F91
w: www.monstr.eu p: +42-0-721842854
Maintainer of Linux kernel - Microblaze cpu - http://www.monstr.eu/fdt/
Maintainer of Linux kernel - Xilinx Zynq ARM architecture
Microblaze U-BOOT custodian and responsible for u-boot arm zynq platform



[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 263 bytes --]

Re: [PATCH 09/10] watchdog: xilinx: Add missing binding

[Arnd Bergmann]

On Wednesday 05 February 2014, Michal Simek wrote:
> On 02/05/2014 10:36 AM, Arnd Bergmann wrote:
> > On Wednesday 05 February 2014, Michal Simek wrote:
> > Still, the choice of putting the timeout into DT in terms of
> > cycles rather than miliseconds wasn't ideal from an interface
> > perspective and we should change that if/when we do a generic
> > binding. I can definitely see where it's coming from for your
> > case, as the cycle count totally makes sense from an FPGA
> > tool perspective...
> 
> Thanks. I take this like ACK for this current binding description.
> 

Yes, please add my 'Acked-by'.

	Arnd

Re: [PATCH 01/10] watchdog: xilinx: Convert driver to the watchdog framework

[Guenter Roeck]

On 01/31/2014 06:18 AM, Michal Simek wrote:
> - Remove uneeded headers, fops functions
> - Use xilinx_wdt prefix in start/stop/keepalive functions
>    and in new structures
>
> Signed-off-by: Michal Simek <michal.simek@xilinx.com>

Hi Michal,

>   static int xwdt_probe(struct platform_device *pdev)
>   {
>   	int rc;
> @@ -314,7 +184,7 @@ static int xwdt_probe(struct platform_device *pdev)
>   					"xlnx,wdt-enable-once", NULL);
>   	if (tmptr == NULL) {
>   		pr_warn("Parameter \"xlnx,wdt-enable-once\" not found in device tree!\n");
> -		xdev.nowayout = WATCHDOG_NOWAYOUT;
> +		watchdog_set_nowayout(&xilinx_wdt_wdd, true);

Sure you want to set this to always true instead of using WATCHDOG_NOWAYOUT ?

Assuming this is what you want:

Reviewed-by: Guenter Roeck <linux@roeck-us.net>

Thanks,
Guenter

Re: [PATCH 02/10] watchdog: xilinx: Move control_status_reg to functions

[Guenter Roeck]

On 01/31/2014 06:18 AM, Michal Simek wrote:
> control_status_reg is temp variables and should be
> used locally by specific function.
>
> Signed-off-by: Michal Simek <michal.simek@xilinx.com>

Reviewed-by: Guenter Roeck <linux@roeck-us.net>

Re: [03/10] watchdog: xilinx: Simplify probe and remove functions

[Guenter Roeck]

On Fri, Jan 31, 2014 at 10:18:12PM -0000, Michal Simek wrote:
> Use devm_ helper function to simplify probe and error path.
> Move ioremap to the beginning of probe function.
> 
> Signed-off-by: Michal Simek <michal.simek@xilinx.com>
> 
Reviewed-by: Guenter Roeck <linux@roeck-us.net>

Re: [PATCH 04/10] watchdog: xilinx: Move no_timeout to probe function

[Guenter Roeck]

On 01/31/2014 06:18 AM, Michal Simek wrote:
> no_timeout should be local variable because it is used
> only in probe function.
>
> Signed-off-by: Michal Simek <michal.simek@xilinx.com>

Reviewed-by: Guenter Roeck <linux@roeck-us.net>

Re: [PATCH 05/10] watchdog: xilinx: Allocate private structure per device

[Guenter Roeck]

On 01/31/2014 06:18 AM, Michal Simek wrote:
> Only one watchdog could be used by this driver.
> Create driver private data structure and move there
> all variables for one instance.
>
> Signed-off-by: Michal Simek <michal.simek@xilinx.com>

Reviewed-by: Guenter Roeck <linux@roeck-us.net>

Re: [PATCH 06/10] watchdog: xilinx: Fix all printk messages

[Guenter Roeck]

On 01/31/2014 06:18 AM, Michal Simek wrote:
> Use dev_ functions for printk messages.
>
> Signed-off-by: Michal Simek <michal.simek@xilinx.com>

Reviewed-by: Guenter Roeck <linux@roeck-us.net>

Re: [PATCH 07/10] watchdog: xilinx: Fix OF binding

[Guenter Roeck]

On 01/31/2014 06:18 AM, Michal Simek wrote:
> Use of_property_read_u32 functions to clean OF probing.
>
> Signed-off-by: Michal Simek <michal.simek@xilinx.com>

Reviewed-by: Guenter Roeck <linux@roeck-us.net>

Re: [PATCH 08/10] watchdog: xilinx: Use correct comment indentation

[Guenter Roeck]

On 01/31/2014 06:18 AM, Michal Simek wrote:
> No functional changes.
>
> Signed-off-by: Michal Simek <michal.simek@xilinx.com>

Reviewed-by: Guenter Roeck <linux@roeck-us.net>

Re: [PATCH 10/10] watchdog: xilinx: Enable this driver for Zynq

[Guenter Roeck]

On 01/31/2014 06:18 AM, Michal Simek wrote:
> Enable this driver for Zynq.
> Move it to architecture independent Kconfig part.
>
> Signed-off-by: Michal Simek <michal.simek@xilinx.com>
> ---
>
> Build tested by zero day testing system.
> ---

Hi Michal,

I tried myself, and I don't see any build failures on any platform.
So let's assume this works as-is.

Reviewed-by: Guenter Roeck <linux@roeck-us.net>

Guenter

Re: [PATCH 01/10] watchdog: xilinx: Convert driver to the watchdog framework

[Michal Simek]

[-- Attachment #1: Type: text/plain, Size: 1756 bytes --]

On 02/09/2014 09:03 PM, Guenter Roeck wrote:
> On 01/31/2014 06:18 AM, Michal Simek wrote:
>> - Remove uneeded headers, fops functions
>> - Use xilinx_wdt prefix in start/stop/keepalive functions
>>    and in new structures
>>
>> Signed-off-by: Michal Simek <michal.simek@xilinx.com>
> 
> Hi Michal,
> 
>>   static int xwdt_probe(struct platform_device *pdev)
>>   {
>>       int rc;
>> @@ -314,7 +184,7 @@ static int xwdt_probe(struct platform_device *pdev)
>>                       "xlnx,wdt-enable-once", NULL);
>>       if (tmptr == NULL) {
>>           pr_warn("Parameter \"xlnx,wdt-enable-once\" not found in device tree!\n");
>> -        xdev.nowayout = WATCHDOG_NOWAYOUT;
>> +        watchdog_set_nowayout(&xilinx_wdt_wdd, true);
> 
> Sure you want to set this to always true instead of using WATCHDOG_NOWAYOUT ?

I have checked it and
option CONFIG_WATCHDOG_NOWAYOUT - Disable watchdog shutdown on close

with this part in the header

100 #ifdef CONFIG_WATCHDOG_NOWAYOUT
101 #define WATCHDOG_NOWAYOUT               1
102 #define WATCHDOG_NOWAYOUT_INIT_STATUS   (1 << WDOG_NO_WAY_OUT)
103 #else
104 #define WATCHDOG_NOWAYOUT               0
105 #define WATCHDOG_NOWAYOUT_INIT_STATUS   0
106 #endif

enable once is hardware option and it means when this option is setup in
hw you can't stop watchdog. That's why I think that setting up true
instead of WATCHDOG_NOWAYOUT is correct.

Thanks,
Michal

-- 
Michal Simek, Ing. (M.Eng), OpenPGP -> KeyID: FE3D1F91
w: www.monstr.eu p: +42-0-721842854
Maintainer of Linux kernel - Microblaze cpu - http://www.monstr.eu/fdt/
Maintainer of Linux kernel - Xilinx Zynq ARM architecture
Microblaze U-BOOT custodian and responsible for u-boot arm zynq platform



[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 263 bytes --]

Re: [PATCH 10/10] watchdog: xilinx: Enable this driver for Zynq

[Michal Simek]

[-- Attachment #1: Type: text/plain, Size: 1007 bytes --]

On 02/10/2014 01:51 AM, Guenter Roeck wrote:
> On 01/31/2014 06:18 AM, Michal Simek wrote:
>> Enable this driver for Zynq.
>> Move it to architecture independent Kconfig part.
>>
>> Signed-off-by: Michal Simek <michal.simek@xilinx.com>
>> ---
>>
>> Build tested by zero day testing system.
>> ---
> 
> Hi Michal,
> 
> I tried myself, and I don't see any build failures on any platform.
> So let's assume this works as-is.
> 
> Reviewed-by: Guenter Roeck <linux@roeck-us.net>
> 

Thanks for your review. I have fixed comments from Rob and
also add one more patch which was suggested by Rob.
I will wait for your reactions for 1/10 and then will send
v2.

Thanks,
Michal


-- 
Michal Simek, Ing. (M.Eng), OpenPGP -> KeyID: FE3D1F91
w: www.monstr.eu p: +42-0-721842854
Maintainer of Linux kernel - Microblaze cpu - http://www.monstr.eu/fdt/
Maintainer of Linux kernel - Xilinx Zynq ARM architecture
Microblaze U-BOOT custodian and responsible for u-boot arm zynq platform



[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 263 bytes --]

RE: [PATCH 01/10] watchdog: xilinx: Convert driver to the watchdog framework

[Guenter Roeck]

> -----Original Message-----
> From: Michal Simek [mailto:monstr@monstr.eu]
> Sent: Sunday, February 09, 2014 11:04 PM
> To: Guenter Roeck
> Cc: Michal Simek; linux-kernel@vger.kernel.org; Wim Van Sebroeck;
> linux-watchdog@vger.kernel.org; linux-arm-kernel@lists.infradead.org
> Subject: Re: [PATCH 01/10] watchdog: xilinx: Convert driver to the
> watchdog framework
> 
> On 02/09/2014 09:03 PM, Guenter Roeck wrote:
> > On 01/31/2014 06:18 AM, Michal Simek wrote:
> >> - Remove uneeded headers, fops functions
> >> - Use xilinx_wdt prefix in start/stop/keepalive functions
> >>    and in new structures
> >>
> >> Signed-off-by: Michal Simek <michal.simek@xilinx.com>
> >
> > Hi Michal,
> >
> >>   static int xwdt_probe(struct platform_device *pdev)
> >>   {
> >>       int rc;
> >> @@ -314,7 +184,7 @@ static int xwdt_probe(struct platform_device
> *pdev)
> >>                       "xlnx,wdt-enable-once", NULL);
> >>       if (tmptr == NULL) {
> >>           pr_warn("Parameter \"xlnx,wdt-enable-once\" not found in
> device tree!\n");
> >> -        xdev.nowayout = WATCHDOG_NOWAYOUT;
> >> +        watchdog_set_nowayout(&xilinx_wdt_wdd, true);
> >
> > Sure you want to set this to always true instead of using
> WATCHDOG_NOWAYOUT ?
> 
> I have checked it and
> option CONFIG_WATCHDOG_NOWAYOUT - Disable watchdog shutdown on close
> 
> with this part in the header
> 
> 100 #ifdef CONFIG_WATCHDOG_NOWAYOUT
> 101 #define WATCHDOG_NOWAYOUT               1
> 102 #define WATCHDOG_NOWAYOUT_INIT_STATUS   (1 << WDOG_NO_WAY_OUT)
> 103 #else
> 104 #define WATCHDOG_NOWAYOUT               0
> 105 #define WATCHDOG_NOWAYOUT_INIT_STATUS   0
> 106 #endif
> 
> enable once is hardware option and it means when this option is setup
> in hw you can't stop watchdog. That's why I think that setting up true
> instead of WATCHDOG_NOWAYOUT is correct.
> 

Ok, makes sense.

Guenter