* [PATCH 02/10] watchdog: xilinx: Move control_status_reg to functions
2014-01-31 14:18 [PATCH 01/10] watchdog: xilinx: Convert driver to the watchdog framework Michal Simek
@ 2014-01-31 14:18 ` Michal Simek
2014-02-09 20:05 ` Guenter Roeck
2014-01-31 14:18 ` [PATCH 03/10] watchdog: xilinx: Simplify probe and remove functions Michal Simek
` (8 subsequent siblings)
9 siblings, 1 reply; 38+ messages in thread
From: Michal Simek @ 2014-01-31 14:18 UTC (permalink / raw)
To: linux-kernel, monstr; +Cc: Wim Van Sebroeck, linux-watchdog, linux-arm-kernel
[-- 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 --]
^ permalink raw reply related [flat|nested] 38+ messages in thread
* [PATCH 03/10] watchdog: xilinx: Simplify probe and remove functions
2014-01-31 14:18 [PATCH 01/10] watchdog: xilinx: Convert driver to the watchdog framework Michal Simek
2014-01-31 14:18 ` [PATCH 02/10] watchdog: xilinx: Move control_status_reg to functions Michal Simek
@ 2014-01-31 14:18 ` Michal Simek
2014-02-09 20:08 ` [03/10] " Guenter Roeck
2014-01-31 14:18 ` [PATCH 04/10] watchdog: xilinx: Move no_timeout to probe function Michal Simek
` (7 subsequent siblings)
9 siblings, 1 reply; 38+ messages in thread
From: Michal Simek @ 2014-01-31 14:18 UTC (permalink / raw)
To: linux-kernel, monstr; +Cc: Wim Van Sebroeck, linux-watchdog, linux-arm-kernel
[-- 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 --]
^ permalink raw reply related [flat|nested] 38+ messages in thread
* [PATCH 04/10] watchdog: xilinx: Move no_timeout to probe function
2014-01-31 14:18 [PATCH 01/10] watchdog: xilinx: Convert driver to the watchdog framework Michal Simek
2014-01-31 14:18 ` [PATCH 02/10] watchdog: xilinx: Move control_status_reg to functions Michal Simek
2014-01-31 14:18 ` [PATCH 03/10] watchdog: xilinx: Simplify probe and remove functions Michal Simek
@ 2014-01-31 14:18 ` Michal Simek
2014-02-09 20:09 ` Guenter Roeck
2014-01-31 14:18 ` [PATCH 05/10] watchdog: xilinx: Allocate private structure per device Michal Simek
` (6 subsequent siblings)
9 siblings, 1 reply; 38+ messages in thread
From: Michal Simek @ 2014-01-31 14:18 UTC (permalink / raw)
To: linux-kernel, monstr; +Cc: Wim Van Sebroeck, linux-watchdog, linux-arm-kernel
[-- 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 --]
^ permalink raw reply related [flat|nested] 38+ messages in thread
* [PATCH 05/10] watchdog: xilinx: Allocate private structure per device
2014-01-31 14:18 [PATCH 01/10] watchdog: xilinx: Convert driver to the watchdog framework Michal Simek
` (2 preceding siblings ...)
2014-01-31 14:18 ` [PATCH 04/10] watchdog: xilinx: Move no_timeout to probe function Michal Simek
@ 2014-01-31 14:18 ` Michal Simek
2014-02-09 20:13 ` Guenter Roeck
2014-01-31 14:18 ` [PATCH 06/10] watchdog: xilinx: Fix all printk messages Michal Simek
` (5 subsequent siblings)
9 siblings, 1 reply; 38+ messages in thread
From: Michal Simek @ 2014-01-31 14:18 UTC (permalink / raw)
To: linux-kernel, monstr; +Cc: Wim Van Sebroeck, linux-watchdog, linux-arm-kernel
[-- 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 --]
^ permalink raw reply related [flat|nested] 38+ messages in thread
* [PATCH 06/10] watchdog: xilinx: Fix all printk messages
2014-01-31 14:18 [PATCH 01/10] watchdog: xilinx: Convert driver to the watchdog framework Michal Simek
` (3 preceding siblings ...)
2014-01-31 14:18 ` [PATCH 05/10] watchdog: xilinx: Allocate private structure per device Michal Simek
@ 2014-01-31 14:18 ` Michal Simek
2014-02-09 20:14 ` Guenter Roeck
2014-01-31 14:18 ` [PATCH 07/10] watchdog: xilinx: Fix OF binding Michal Simek
` (4 subsequent siblings)
9 siblings, 1 reply; 38+ messages in thread
From: Michal Simek @ 2014-01-31 14:18 UTC (permalink / raw)
To: linux-kernel, monstr; +Cc: Wim Van Sebroeck, linux-watchdog, linux-arm-kernel
[-- 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 --]
^ permalink raw reply related [flat|nested] 38+ messages in thread
* [PATCH 07/10] watchdog: xilinx: Fix OF binding
2014-01-31 14:18 [PATCH 01/10] watchdog: xilinx: Convert driver to the watchdog framework Michal Simek
` (4 preceding siblings ...)
2014-01-31 14:18 ` [PATCH 06/10] watchdog: xilinx: Fix all printk messages Michal Simek
@ 2014-01-31 14:18 ` Michal Simek
2014-01-31 17:33 ` Rob Herring
2014-02-09 20:18 ` Guenter Roeck
2014-01-31 14:18 ` [PATCH 08/10] watchdog: xilinx: Use correct comment indentation Michal Simek
` (3 subsequent siblings)
9 siblings, 2 replies; 38+ messages in thread
From: Michal Simek @ 2014-01-31 14:18 UTC (permalink / raw)
To: linux-kernel, monstr
Cc: Wim Van Sebroeck, Grant Likely, Rob Herring, linux-watchdog,
linux-arm-kernel, devicetree
[-- 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 --]
^ permalink raw reply related [flat|nested] 38+ messages in thread
* Re: [PATCH 07/10] watchdog: xilinx: Fix OF binding
2014-01-31 14:18 ` [PATCH 07/10] watchdog: xilinx: Fix OF binding Michal Simek
@ 2014-01-31 17:33 ` Rob Herring
2014-02-03 7:59 ` Michal Simek
2014-02-09 20:18 ` Guenter Roeck
1 sibling, 1 reply; 38+ messages in thread
From: Rob Herring @ 2014-01-31 17:33 UTC (permalink / raw)
To: Michal Simek
Cc: linux-kernel, Michal Simek, Wim Van Sebroeck, Grant Likely,
Rob Herring, linux-watchdog, linux-arm-kernel, devicetree
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
^ permalink raw reply [flat|nested] 38+ messages in thread
* Re: [PATCH 07/10] watchdog: xilinx: Fix OF binding
2014-01-31 17:33 ` Rob Herring
@ 2014-02-03 7:59 ` Michal Simek
2014-02-03 8:01 ` Michal Simek
0 siblings, 1 reply; 38+ messages in thread
From: Michal Simek @ 2014-02-03 7:59 UTC (permalink / raw)
To: Rob Herring
Cc: Michal Simek, linux-kernel, Wim Van Sebroeck, Grant Likely,
Rob Herring, linux-watchdog, linux-arm-kernel, devicetree
[-- 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 --]
^ permalink raw reply [flat|nested] 38+ messages in thread
* Re: [PATCH 07/10] watchdog: xilinx: Fix OF binding
2014-02-03 7:59 ` Michal Simek
@ 2014-02-03 8:01 ` Michal Simek
0 siblings, 0 replies; 38+ messages in thread
From: Michal Simek @ 2014-02-03 8:01 UTC (permalink / raw)
To: monstr
Cc: Rob Herring, Michal Simek, linux-kernel, Wim Van Sebroeck,
Grant Likely, Rob Herring, linux-watchdog, linux-arm-kernel,
devicetree
[-- 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 --]
^ permalink raw reply [flat|nested] 38+ messages in thread
* Re: [PATCH 07/10] watchdog: xilinx: Fix OF binding
2014-01-31 14:18 ` [PATCH 07/10] watchdog: xilinx: Fix OF binding Michal Simek
2014-01-31 17:33 ` Rob Herring
@ 2014-02-09 20:18 ` Guenter Roeck
1 sibling, 0 replies; 38+ messages in thread
From: Guenter Roeck @ 2014-02-09 20:18 UTC (permalink / raw)
To: Michal Simek, linux-kernel, monstr
Cc: Wim Van Sebroeck, Grant Likely, Rob Herring, linux-watchdog,
linux-arm-kernel, devicetree
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>
^ permalink raw reply [flat|nested] 38+ messages in thread
* [PATCH 08/10] watchdog: xilinx: Use correct comment indentation
2014-01-31 14:18 [PATCH 01/10] watchdog: xilinx: Convert driver to the watchdog framework Michal Simek
` (5 preceding siblings ...)
2014-01-31 14:18 ` [PATCH 07/10] watchdog: xilinx: Fix OF binding Michal Simek
@ 2014-01-31 14:18 ` Michal Simek
2014-02-09 20:19 ` Guenter Roeck
2014-01-31 14:18 ` [PATCH 09/10] watchdog: xilinx: Add missing binding Michal Simek
` (2 subsequent siblings)
9 siblings, 1 reply; 38+ messages in thread
From: Michal Simek @ 2014-01-31 14:18 UTC (permalink / raw)
To: linux-kernel, monstr; +Cc: Wim Van Sebroeck, linux-watchdog, linux-arm-kernel
[-- 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 --]
^ permalink raw reply related [flat|nested] 38+ messages in thread
* [PATCH 09/10] watchdog: xilinx: Add missing binding
2014-01-31 14:18 [PATCH 01/10] watchdog: xilinx: Convert driver to the watchdog framework Michal Simek
` (6 preceding siblings ...)
2014-01-31 14:18 ` [PATCH 08/10] watchdog: xilinx: Use correct comment indentation Michal Simek
@ 2014-01-31 14:18 ` Michal Simek
2014-02-03 15:06 ` Arnd Bergmann
2014-01-31 14:18 ` [PATCH 10/10] watchdog: xilinx: Enable this driver for Zynq Michal Simek
2014-02-09 20:03 ` [PATCH 01/10] watchdog: xilinx: Convert driver to the watchdog framework Guenter Roeck
9 siblings, 1 reply; 38+ messages in thread
From: Michal Simek @ 2014-01-31 14:18 UTC (permalink / raw)
To: linux-kernel, monstr
Cc: Rob Herring, Pawel Moll, Mark Rutland, Ian Campbell, Kumar Gala,
Rob Landley, devicetree, linux-doc, linux-arm-kernel
[-- 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 --]
^ permalink raw reply related [flat|nested] 38+ messages in thread
* Re: [PATCH 09/10] watchdog: xilinx: Add missing binding
2014-01-31 14:18 ` [PATCH 09/10] watchdog: xilinx: Add missing binding Michal Simek
@ 2014-02-03 15:06 ` Arnd Bergmann
2014-02-03 15:13 ` Michal Simek
0 siblings, 1 reply; 38+ messages in thread
From: Arnd Bergmann @ 2014-02-03 15:06 UTC (permalink / raw)
To: linux-arm-kernel
Cc: Michal Simek, linux-kernel, monstr, Mark Rutland, devicetree,
Pawel Moll, Ian Campbell, linux-doc, Rob Herring, Rob Landley,
Kumar Gala
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
^ permalink raw reply [flat|nested] 38+ messages in thread
* Re: [PATCH 09/10] watchdog: xilinx: Add missing binding
2014-02-03 15:06 ` Arnd Bergmann
@ 2014-02-03 15:13 ` Michal Simek
2014-02-03 15:32 ` Arnd Bergmann
0 siblings, 1 reply; 38+ messages in thread
From: Michal Simek @ 2014-02-03 15:13 UTC (permalink / raw)
To: Arnd Bergmann
Cc: linux-arm-kernel, Michal Simek, linux-kernel, monstr,
Mark Rutland, devicetree, Pawel Moll, Ian Campbell, linux-doc,
Rob Herring, Rob Landley, Kumar Gala
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
^ permalink raw reply [flat|nested] 38+ messages in thread
* Re: [PATCH 09/10] watchdog: xilinx: Add missing binding
2014-02-03 15:13 ` Michal Simek
@ 2014-02-03 15:32 ` Arnd Bergmann
2014-02-03 15:47 ` Michal Simek
0 siblings, 1 reply; 38+ messages in thread
From: Arnd Bergmann @ 2014-02-03 15:32 UTC (permalink / raw)
To: Michal Simek
Cc: linux-arm-kernel, linux-kernel, monstr, Mark Rutland, devicetree,
Pawel Moll, Ian Campbell, linux-doc, Rob Herring, Rob Landley,
Kumar Gala
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
^ permalink raw reply [flat|nested] 38+ messages in thread
* Re: [PATCH 09/10] watchdog: xilinx: Add missing binding
2014-02-03 15:32 ` Arnd Bergmann
@ 2014-02-03 15:47 ` Michal Simek
2014-02-04 19:27 ` Arnd Bergmann
0 siblings, 1 reply; 38+ messages in thread
From: Michal Simek @ 2014-02-03 15:47 UTC (permalink / raw)
To: Arnd Bergmann
Cc: Michal Simek, linux-arm-kernel, linux-kernel, monstr,
Mark Rutland, devicetree, Pawel Moll, Ian Campbell, linux-doc,
Rob Herring, Rob Landley, Kumar Gala
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
^ permalink raw reply [flat|nested] 38+ messages in thread
* Re: [PATCH 09/10] watchdog: xilinx: Add missing binding
2014-02-03 15:47 ` Michal Simek
@ 2014-02-04 19:27 ` Arnd Bergmann
2014-02-05 9:25 ` Michal Simek
0 siblings, 1 reply; 38+ messages in thread
From: Arnd Bergmann @ 2014-02-04 19:27 UTC (permalink / raw)
To: Michal Simek
Cc: linux-arm-kernel, linux-kernel, monstr, Mark Rutland, devicetree,
Pawel Moll, Ian Campbell, linux-doc, Rob Herring, Rob Landley,
Kumar Gala
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
^ permalink raw reply [flat|nested] 38+ messages in thread
* Re: [PATCH 09/10] watchdog: xilinx: Add missing binding
2014-02-04 19:27 ` Arnd Bergmann
@ 2014-02-05 9:25 ` Michal Simek
2014-02-05 9:36 ` Arnd Bergmann
0 siblings, 1 reply; 38+ messages in thread
From: Michal Simek @ 2014-02-05 9:25 UTC (permalink / raw)
To: Arnd Bergmann
Cc: Michal Simek, linux-arm-kernel, linux-kernel, monstr,
Mark Rutland, devicetree, Pawel Moll, Ian Campbell, linux-doc,
Rob Herring, Rob Landley, Kumar Gala
[-- 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 --]
^ permalink raw reply [flat|nested] 38+ messages in thread
* Re: [PATCH 09/10] watchdog: xilinx: Add missing binding
2014-02-05 9:25 ` Michal Simek
@ 2014-02-05 9:36 ` Arnd Bergmann
2014-02-05 9:41 ` Michal Simek
0 siblings, 1 reply; 38+ messages in thread
From: Arnd Bergmann @ 2014-02-05 9:36 UTC (permalink / raw)
To: linux-arm-kernel, monstr
Cc: Mark Rutland, devicetree, Pawel Moll, Ian Campbell, linux-doc,
Michal Simek, linux-kernel, Rob Herring, Rob Landley, Kumar Gala
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
^ permalink raw reply [flat|nested] 38+ messages in thread
* Re: [PATCH 09/10] watchdog: xilinx: Add missing binding
2014-02-05 9:36 ` Arnd Bergmann
@ 2014-02-05 9:41 ` Michal Simek
2014-02-05 14:00 ` Arnd Bergmann
0 siblings, 1 reply; 38+ messages in thread
From: Michal Simek @ 2014-02-05 9:41 UTC (permalink / raw)
To: Arnd Bergmann
Cc: linux-arm-kernel, monstr, Mark Rutland, devicetree, Pawel Moll,
Ian Campbell, linux-doc, Michal Simek, linux-kernel, Rob Herring,
Rob Landley, Kumar Gala
[-- 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 --]
^ permalink raw reply [flat|nested] 38+ messages in thread
* Re: [PATCH 09/10] watchdog: xilinx: Add missing binding
2014-02-05 9:41 ` Michal Simek
@ 2014-02-05 14:00 ` Arnd Bergmann
0 siblings, 0 replies; 38+ messages in thread
From: Arnd Bergmann @ 2014-02-05 14:00 UTC (permalink / raw)
To: monstr
Cc: linux-arm-kernel, Mark Rutland, devicetree, Pawel Moll,
Ian Campbell, linux-doc, Michal Simek, linux-kernel, Rob Herring,
Rob Landley, Kumar Gala
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
^ permalink raw reply [flat|nested] 38+ messages in thread
* [PATCH 10/10] watchdog: xilinx: Enable this driver for Zynq
2014-01-31 14:18 [PATCH 01/10] watchdog: xilinx: Convert driver to the watchdog framework Michal Simek
` (7 preceding siblings ...)
2014-01-31 14:18 ` [PATCH 09/10] watchdog: xilinx: Add missing binding Michal Simek
@ 2014-01-31 14:18 ` Michal Simek
2014-01-31 14:52 ` Guenter Roeck
2014-02-10 0:51 ` Guenter Roeck
2014-02-09 20:03 ` [PATCH 01/10] watchdog: xilinx: Convert driver to the watchdog framework Guenter Roeck
9 siblings, 2 replies; 38+ messages in thread
From: Michal Simek @ 2014-01-31 14:18 UTC (permalink / raw)
To: linux-kernel, monstr; +Cc: Wim Van Sebroeck, linux-watchdog
[-- 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 --]
^ permalink raw reply related [flat|nested] 38+ messages in thread
* Re: [PATCH 10/10] watchdog: xilinx: Enable this driver for Zynq
2014-01-31 14:18 ` [PATCH 10/10] watchdog: xilinx: Enable this driver for Zynq Michal Simek
@ 2014-01-31 14:52 ` Guenter Roeck
2014-02-03 7:01 ` Michal Simek
2014-02-10 0:51 ` Guenter Roeck
1 sibling, 1 reply; 38+ messages in thread
From: Guenter Roeck @ 2014-01-31 14:52 UTC (permalink / raw)
To: Michal Simek, linux-kernel, monstr; +Cc: Wim Van Sebroeck, linux-watchdog
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
^ permalink raw reply [flat|nested] 38+ messages in thread
* Re: [PATCH 10/10] watchdog: xilinx: Enable this driver for Zynq
2014-01-31 14:52 ` Guenter Roeck
@ 2014-02-03 7:01 ` Michal Simek
2014-02-03 8:26 ` Guenter Roeck
0 siblings, 1 reply; 38+ messages in thread
From: Michal Simek @ 2014-02-03 7:01 UTC (permalink / raw)
To: Guenter Roeck
Cc: Michal Simek, linux-kernel, Wim Van Sebroeck, linux-watchdog
[-- 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 --]
^ permalink raw reply [flat|nested] 38+ messages in thread
* Re: [PATCH 10/10] watchdog: xilinx: Enable this driver for Zynq
2014-02-03 7:01 ` Michal Simek
@ 2014-02-03 8:26 ` Guenter Roeck
2014-02-03 8:45 ` Michal Simek
0 siblings, 1 reply; 38+ messages in thread
From: Guenter Roeck @ 2014-02-03 8:26 UTC (permalink / raw)
To: monstr; +Cc: Michal Simek, linux-kernel, Wim Van Sebroeck, linux-watchdog
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
^ permalink raw reply [flat|nested] 38+ messages in thread
* Re: [PATCH 10/10] watchdog: xilinx: Enable this driver for Zynq
2014-02-03 8:26 ` Guenter Roeck
@ 2014-02-03 8:45 ` Michal Simek
0 siblings, 0 replies; 38+ messages in thread
From: Michal Simek @ 2014-02-03 8:45 UTC (permalink / raw)
To: Guenter Roeck
Cc: Michal Simek, linux-kernel, Wim Van Sebroeck, linux-watchdog
[-- 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 --]
^ permalink raw reply [flat|nested] 38+ messages in thread
* Re: [PATCH 10/10] watchdog: xilinx: Enable this driver for Zynq
2014-01-31 14:18 ` [PATCH 10/10] watchdog: xilinx: Enable this driver for Zynq Michal Simek
2014-01-31 14:52 ` Guenter Roeck
@ 2014-02-10 0:51 ` Guenter Roeck
2014-02-10 7:06 ` Michal Simek
1 sibling, 1 reply; 38+ messages in thread
From: Guenter Roeck @ 2014-02-10 0:51 UTC (permalink / raw)
To: Michal Simek, linux-kernel, monstr; +Cc: Wim Van Sebroeck, linux-watchdog
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
^ permalink raw reply [flat|nested] 38+ messages in thread
* Re: [PATCH 10/10] watchdog: xilinx: Enable this driver for Zynq
2014-02-10 0:51 ` Guenter Roeck
@ 2014-02-10 7:06 ` Michal Simek
0 siblings, 0 replies; 38+ messages in thread
From: Michal Simek @ 2014-02-10 7:06 UTC (permalink / raw)
To: Guenter Roeck
Cc: Michal Simek, linux-kernel, Wim Van Sebroeck, linux-watchdog
[-- 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 --]
^ permalink raw reply [flat|nested] 38+ messages in thread
* Re: [PATCH 01/10] watchdog: xilinx: Convert driver to the watchdog framework
2014-01-31 14:18 [PATCH 01/10] watchdog: xilinx: Convert driver to the watchdog framework Michal Simek
` (8 preceding siblings ...)
2014-01-31 14:18 ` [PATCH 10/10] watchdog: xilinx: Enable this driver for Zynq Michal Simek
@ 2014-02-09 20:03 ` Guenter Roeck
2014-02-10 7:03 ` Michal Simek
9 siblings, 1 reply; 38+ messages in thread
From: Guenter Roeck @ 2014-02-09 20:03 UTC (permalink / raw)
To: Michal Simek, linux-kernel, monstr
Cc: Wim Van Sebroeck, linux-watchdog, linux-arm-kernel
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
^ permalink raw reply [flat|nested] 38+ messages in thread
* Re: [PATCH 01/10] watchdog: xilinx: Convert driver to the watchdog framework
2014-02-09 20:03 ` [PATCH 01/10] watchdog: xilinx: Convert driver to the watchdog framework Guenter Roeck
@ 2014-02-10 7:03 ` Michal Simek
2014-02-10 17:18 ` Guenter Roeck
0 siblings, 1 reply; 38+ messages in thread
From: Michal Simek @ 2014-02-10 7:03 UTC (permalink / raw)
To: Guenter Roeck
Cc: Michal Simek, linux-kernel, Wim Van Sebroeck, linux-watchdog,
linux-arm-kernel
[-- 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 --]
^ permalink raw reply [flat|nested] 38+ messages in thread
* RE: [PATCH 01/10] watchdog: xilinx: Convert driver to the watchdog framework
2014-02-10 7:03 ` Michal Simek
@ 2014-02-10 17:18 ` Guenter Roeck
0 siblings, 0 replies; 38+ messages in thread
From: Guenter Roeck @ 2014-02-10 17:18 UTC (permalink / raw)
To: monstr, Guenter Roeck
Cc: Michal Simek, linux-kernel, Wim Van Sebroeck, linux-watchdog,
linux-arm-kernel
> -----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
^ permalink raw reply [flat|nested] 38+ messages in thread