All of lore.kernel.org
 help / color / mirror / Atom feed
* Fwd: [PATCH 2/8] watchdog: davinci: use davinci_wdt_device structure to hold device data
       [not found] <1383680783-12114-3-git-send-email-ivan.khoronzhuk@ti.com>
  2013-11-06 11:31   ` ivan.khoronzhuk
@ 2013-11-06 11:31   ` ivan.khoronzhuk
  0 siblings, 0 replies; 14+ messages in thread
From: ivan.khoronzhuk @ 2013-11-06 11:31 UTC (permalink / raw)
  To: Santosh Shilimkar, wim, nsekhar, linux-watchdog, devicetree
  Cc: grant.likely, rob.herring, pawel.moll, mark.rutland, swarren,
	galak, ijc+devicetree, linux-kernel, linux-arm-kernel

Some SoCs, like Keystone 2, can support more than one WDT and each
watchdog device has to use it's own base address, clock source,
wdd device, so add new davinci_wdt_device structure to hold device
data.

Signed-off-by: Ivan Khoronzhuk <ivan.khoronzhuk@ti.com>
---
 drivers/watchdog/davinci_wdt.c |   74 ++++++++++++++++++++++++++--------------
 1 file changed, 48 insertions(+), 26 deletions(-)

diff --git a/drivers/watchdog/davinci_wdt.c b/drivers/watchdog/davinci_wdt.c
index a6eef71..1fc2093 100644
--- a/drivers/watchdog/davinci_wdt.c
+++ b/drivers/watchdog/davinci_wdt.c
@@ -56,9 +56,18 @@
 #define WDKEY_SEQ1		(0xda7e << 16)
 
 static int heartbeat = MAX_HEARTBEAT + 1;
-static void __iomem	*wdt_base;
-struct clk		*wdt_clk;
-static struct watchdog_device	wdt_wdd;
+
+/*
+ * struct to hold data for each WDT device
+ * @base - base io address of WD device
+ * @clk - source clock of WDT
+ * @wdd - hold watchdog device as is in WDT core
+ */
+struct davinci_wdt_device {
+	void __iomem		*base;
+	struct clk		*clk;
+	struct watchdog_device	wdd;
+};
 
 /* davinci_wdt_start - enable watchdog */
 static int davinci_wdt_start(struct watchdog_device *wdd)
@@ -66,42 +75,45 @@ static int davinci_wdt_start(struct watchdog_device *wdd)
 	u32 tgcr;
 	u32 timer_margin;
 	unsigned long wdt_freq;
+	struct davinci_wdt_device *davinci_wdt = watchdog_get_drvdata(wdd);
 
-	wdt_freq = clk_get_rate(wdt_clk);
+	wdt_freq = clk_get_rate(davinci_wdt->clk);
 
 	/* disable, internal clock source */
-	iowrite32(0, wdt_base + TCR);
+	iowrite32(0, davinci_wdt->base + TCR);
 	/* reset timer, set mode to 64-bit watchdog, and unreset */
-	iowrite32(0, wdt_base + TGCR);
+	iowrite32(0, davinci_wdt->base + TGCR);
 	tgcr = TIMMODE_64BIT_WDOG | TIM12RS_UNRESET | TIM34RS_UNRESET;
-	iowrite32(tgcr, wdt_base + TGCR);
+	iowrite32(tgcr, davinci_wdt->base + TGCR);
 	/* clear counter regs */
-	iowrite32(0, wdt_base + TIM12);
-	iowrite32(0, wdt_base + TIM34);
+	iowrite32(0, davinci_wdt->base + TIM12);
+	iowrite32(0, davinci_wdt->base + TIM34);
 	/* set timeout period */
 	timer_margin = (((u64)wdd->timeout * wdt_freq) & 0xffffffff);
-	iowrite32(timer_margin, wdt_base + PRD12);
+	iowrite32(timer_margin, davinci_wdt->base + PRD12);
 	timer_margin = (((u64)wdd->timeout * wdt_freq) >> 32);
-	iowrite32(timer_margin, wdt_base + PRD34);
+	iowrite32(timer_margin, davinci_wdt->base + PRD34);
 	/* enable run continuously */
-	iowrite32(ENAMODE12_PERIODIC, wdt_base + TCR);
+	iowrite32(ENAMODE12_PERIODIC, davinci_wdt->base + TCR);
 	/* Once the WDT is in pre-active state write to
 	 * TIM12, TIM34, PRD12, PRD34, TCR, TGCR, WDTCR are
 	 * write protected (except for the WDKEY field)
 	 */
 	/* put watchdog in pre-active state */
-	iowrite32(WDKEY_SEQ0 | WDEN, wdt_base + WDTCR);
+	iowrite32(WDKEY_SEQ0 | WDEN, davinci_wdt->base + WDTCR);
 	/* put watchdog in active state */
-	iowrite32(WDKEY_SEQ1 | WDEN, wdt_base + WDTCR);
+	iowrite32(WDKEY_SEQ1 | WDEN, davinci_wdt->base + WDTCR);
 	return 0;
 }
 
 static int davinci_wdt_ping(struct watchdog_device *wdd)
 {
+	struct davinci_wdt_device *davinci_wdt = watchdog_get_drvdata(wdd);
+
 	/* put watchdog in service state */
-	iowrite32(WDKEY_SEQ0, wdt_base + WDTCR);
+	iowrite32(WDKEY_SEQ0, davinci_wdt->base + WDTCR);
 	/* put watchdog in active state */
-	iowrite32(WDKEY_SEQ1, wdt_base + WDTCR);
+	iowrite32(WDKEY_SEQ1, davinci_wdt->base + WDTCR);
 	return 0;
 }
 
@@ -123,14 +135,21 @@ static int davinci_wdt_probe(struct platform_device *pdev)
 	struct device *dev = &pdev->dev;
 	struct resource  *wdt_mem;
 	struct watchdog_device *wdd;
+	struct davinci_wdt_device *davinci_wdt;
+
+	davinci_wdt = devm_kzalloc(dev, sizeof(*davinci_wdt), GFP_KERNEL);
+	if (!davinci_wdt)
+		return -ENOMEM;
 
-	wdt_clk = devm_clk_get(dev, NULL);
-	if (WARN_ON(IS_ERR(wdt_clk)))
-		return PTR_ERR(wdt_clk);
+	davinci_wdt->clk = devm_clk_get(dev, NULL);
+	if (WARN_ON(IS_ERR(davinci_wdt->clk)))
+		return PTR_ERR(davinci_wdt->clk);
 
-	clk_prepare_enable(wdt_clk);
+	clk_prepare_enable(davinci_wdt->clk);
 
-	wdd			= &wdt_wdd;
+	platform_set_drvdata(pdev, davinci_wdt);
+
+	wdd			= &davinci_wdt->wdd;
 	wdd->info		= &davinci_wdt_info;
 	wdd->ops		= &davinci_wdt_ops;
 	wdd->min_timeout 	= 1;
@@ -142,12 +161,13 @@ static int davinci_wdt_probe(struct platform_device *pdev)
 
 	dev_info(dev, "heartbeat %d sec\n", wdd->timeout);
 
+	watchdog_set_drvdata(wdd, davinci_wdt);
 	watchdog_set_nowayout(wdd, WATCHDOG_NOWAYOUT);
 
 	wdt_mem = platform_get_resource(pdev, IORESOURCE_MEM, 0);
-	wdt_base = devm_ioremap_resource(dev, wdt_mem);
-	if (IS_ERR(wdt_base))
-		return PTR_ERR(wdt_base);
+	davinci_wdt->base = devm_ioremap_resource(dev, wdt_mem);
+	if (IS_ERR(davinci_wdt->base))
+		return PTR_ERR(davinci_wdt->base);
 
 	ret = watchdog_register_device(wdd);
 	if (ret < 0)
@@ -158,8 +178,10 @@ static int davinci_wdt_probe(struct platform_device *pdev)
 
 static int davinci_wdt_remove(struct platform_device *pdev)
 {
-	watchdog_unregister_device(&wdt_wdd);
-	clk_disable_unprepare(wdt_clk);
+	struct davinci_wdt_device *davinci_wdt = platform_get_drvdata(pdev);
+
+	watchdog_unregister_device(&davinci_wdt->wdd);
+	clk_disable_unprepare(davinci_wdt->clk);
 
 	return 0;
 }
-- 
1.7.9.5




^ permalink raw reply related	[flat|nested] 14+ messages in thread
* Fwd: [PATCH 2/8] watchdog: davinci: use davinci_wdt_device structure to hold device data
@ 2013-11-06 11:31   ` ivan.khoronzhuk
  0 siblings, 0 replies; 14+ messages in thread
From: ivan.khoronzhuk @ 2013-11-06 11:31 UTC (permalink / raw)
  To: Santosh Shilimkar, wim, nsekhar, linux-watchdog, devicetree
  Cc: mark.rutland, pawel.moll, swarren, ijc+devicetree, galak,
	rob.herring, linux-kernel, grant.likely, linux-arm-kernel

Some SoCs, like Keystone 2, can support more than one WDT and each
watchdog device has to use it's own base address, clock source,
wdd device, so add new davinci_wdt_device structure to hold device
data.

Signed-off-by: Ivan Khoronzhuk <ivan.khoronzhuk@ti.com>
---
 drivers/watchdog/davinci_wdt.c |   74 ++++++++++++++++++++++++++--------------
 1 file changed, 48 insertions(+), 26 deletions(-)

diff --git a/drivers/watchdog/davinci_wdt.c b/drivers/watchdog/davinci_wdt.c
index a6eef71..1fc2093 100644
--- a/drivers/watchdog/davinci_wdt.c
+++ b/drivers/watchdog/davinci_wdt.c
@@ -56,9 +56,18 @@
 #define WDKEY_SEQ1		(0xda7e << 16)
 
 static int heartbeat = MAX_HEARTBEAT + 1;
-static void __iomem	*wdt_base;
-struct clk		*wdt_clk;
-static struct watchdog_device	wdt_wdd;
+
+/*
+ * struct to hold data for each WDT device
+ * @base - base io address of WD device
+ * @clk - source clock of WDT
+ * @wdd - hold watchdog device as is in WDT core
+ */
+struct davinci_wdt_device {
+	void __iomem		*base;
+	struct clk		*clk;
+	struct watchdog_device	wdd;
+};
 
 /* davinci_wdt_start - enable watchdog */
 static int davinci_wdt_start(struct watchdog_device *wdd)
@@ -66,42 +75,45 @@ static int davinci_wdt_start(struct watchdog_device *wdd)
 	u32 tgcr;
 	u32 timer_margin;
 	unsigned long wdt_freq;
+	struct davinci_wdt_device *davinci_wdt = watchdog_get_drvdata(wdd);
 
-	wdt_freq = clk_get_rate(wdt_clk);
+	wdt_freq = clk_get_rate(davinci_wdt->clk);
 
 	/* disable, internal clock source */
-	iowrite32(0, wdt_base + TCR);
+	iowrite32(0, davinci_wdt->base + TCR);
 	/* reset timer, set mode to 64-bit watchdog, and unreset */
-	iowrite32(0, wdt_base + TGCR);
+	iowrite32(0, davinci_wdt->base + TGCR);
 	tgcr = TIMMODE_64BIT_WDOG | TIM12RS_UNRESET | TIM34RS_UNRESET;
-	iowrite32(tgcr, wdt_base + TGCR);
+	iowrite32(tgcr, davinci_wdt->base + TGCR);
 	/* clear counter regs */
-	iowrite32(0, wdt_base + TIM12);
-	iowrite32(0, wdt_base + TIM34);
+	iowrite32(0, davinci_wdt->base + TIM12);
+	iowrite32(0, davinci_wdt->base + TIM34);
 	/* set timeout period */
 	timer_margin = (((u64)wdd->timeout * wdt_freq) & 0xffffffff);
-	iowrite32(timer_margin, wdt_base + PRD12);
+	iowrite32(timer_margin, davinci_wdt->base + PRD12);
 	timer_margin = (((u64)wdd->timeout * wdt_freq) >> 32);
-	iowrite32(timer_margin, wdt_base + PRD34);
+	iowrite32(timer_margin, davinci_wdt->base + PRD34);
 	/* enable run continuously */
-	iowrite32(ENAMODE12_PERIODIC, wdt_base + TCR);
+	iowrite32(ENAMODE12_PERIODIC, davinci_wdt->base + TCR);
 	/* Once the WDT is in pre-active state write to
 	 * TIM12, TIM34, PRD12, PRD34, TCR, TGCR, WDTCR are
 	 * write protected (except for the WDKEY field)
 	 */
 	/* put watchdog in pre-active state */
-	iowrite32(WDKEY_SEQ0 | WDEN, wdt_base + WDTCR);
+	iowrite32(WDKEY_SEQ0 | WDEN, davinci_wdt->base + WDTCR);
 	/* put watchdog in active state */
-	iowrite32(WDKEY_SEQ1 | WDEN, wdt_base + WDTCR);
+	iowrite32(WDKEY_SEQ1 | WDEN, davinci_wdt->base + WDTCR);
 	return 0;
 }
 
 static int davinci_wdt_ping(struct watchdog_device *wdd)
 {
+	struct davinci_wdt_device *davinci_wdt = watchdog_get_drvdata(wdd);
+
 	/* put watchdog in service state */
-	iowrite32(WDKEY_SEQ0, wdt_base + WDTCR);
+	iowrite32(WDKEY_SEQ0, davinci_wdt->base + WDTCR);
 	/* put watchdog in active state */
-	iowrite32(WDKEY_SEQ1, wdt_base + WDTCR);
+	iowrite32(WDKEY_SEQ1, davinci_wdt->base + WDTCR);
 	return 0;
 }
 
@@ -123,14 +135,21 @@ static int davinci_wdt_probe(struct platform_device *pdev)
 	struct device *dev = &pdev->dev;
 	struct resource  *wdt_mem;
 	struct watchdog_device *wdd;
+	struct davinci_wdt_device *davinci_wdt;
+
+	davinci_wdt = devm_kzalloc(dev, sizeof(*davinci_wdt), GFP_KERNEL);
+	if (!davinci_wdt)
+		return -ENOMEM;
 
-	wdt_clk = devm_clk_get(dev, NULL);
-	if (WARN_ON(IS_ERR(wdt_clk)))
-		return PTR_ERR(wdt_clk);
+	davinci_wdt->clk = devm_clk_get(dev, NULL);
+	if (WARN_ON(IS_ERR(davinci_wdt->clk)))
+		return PTR_ERR(davinci_wdt->clk);
 
-	clk_prepare_enable(wdt_clk);
+	clk_prepare_enable(davinci_wdt->clk);
 
-	wdd			= &wdt_wdd;
+	platform_set_drvdata(pdev, davinci_wdt);
+
+	wdd			= &davinci_wdt->wdd;
 	wdd->info		= &davinci_wdt_info;
 	wdd->ops		= &davinci_wdt_ops;
 	wdd->min_timeout 	= 1;
@@ -142,12 +161,13 @@ static int davinci_wdt_probe(struct platform_device *pdev)
 
 	dev_info(dev, "heartbeat %d sec\n", wdd->timeout);
 
+	watchdog_set_drvdata(wdd, davinci_wdt);
 	watchdog_set_nowayout(wdd, WATCHDOG_NOWAYOUT);
 
 	wdt_mem = platform_get_resource(pdev, IORESOURCE_MEM, 0);
-	wdt_base = devm_ioremap_resource(dev, wdt_mem);
-	if (IS_ERR(wdt_base))
-		return PTR_ERR(wdt_base);
+	davinci_wdt->base = devm_ioremap_resource(dev, wdt_mem);
+	if (IS_ERR(davinci_wdt->base))
+		return PTR_ERR(davinci_wdt->base);
 
 	ret = watchdog_register_device(wdd);
 	if (ret < 0)
@@ -158,8 +178,10 @@ static int davinci_wdt_probe(struct platform_device *pdev)
 
 static int davinci_wdt_remove(struct platform_device *pdev)
 {
-	watchdog_unregister_device(&wdt_wdd);
-	clk_disable_unprepare(wdt_clk);
+	struct davinci_wdt_device *davinci_wdt = platform_get_drvdata(pdev);
+
+	watchdog_unregister_device(&davinci_wdt->wdd);
+	clk_disable_unprepare(davinci_wdt->clk);
 
 	return 0;
 }
-- 
1.7.9.5

^ permalink raw reply related	[flat|nested] 14+ messages in thread
* Fwd: [PATCH 2/8] watchdog: davinci: use davinci_wdt_device structure to hold device data
@ 2013-11-06 11:31   ` ivan.khoronzhuk
  0 siblings, 0 replies; 14+ messages in thread
From: ivan.khoronzhuk @ 2013-11-06 11:31 UTC (permalink / raw)
  To: linux-arm-kernel

Some SoCs, like Keystone 2, can support more than one WDT and each
watchdog device has to use it's own base address, clock source,
wdd device, so add new davinci_wdt_device structure to hold device
data.

Signed-off-by: Ivan Khoronzhuk <ivan.khoronzhuk@ti.com>
---
 drivers/watchdog/davinci_wdt.c |   74 ++++++++++++++++++++++++++--------------
 1 file changed, 48 insertions(+), 26 deletions(-)

diff --git a/drivers/watchdog/davinci_wdt.c b/drivers/watchdog/davinci_wdt.c
index a6eef71..1fc2093 100644
--- a/drivers/watchdog/davinci_wdt.c
+++ b/drivers/watchdog/davinci_wdt.c
@@ -56,9 +56,18 @@
 #define WDKEY_SEQ1		(0xda7e << 16)
 
 static int heartbeat = MAX_HEARTBEAT + 1;
-static void __iomem	*wdt_base;
-struct clk		*wdt_clk;
-static struct watchdog_device	wdt_wdd;
+
+/*
+ * struct to hold data for each WDT device
+ * @base - base io address of WD device
+ * @clk - source clock of WDT
+ * @wdd - hold watchdog device as is in WDT core
+ */
+struct davinci_wdt_device {
+	void __iomem		*base;
+	struct clk		*clk;
+	struct watchdog_device	wdd;
+};
 
 /* davinci_wdt_start - enable watchdog */
 static int davinci_wdt_start(struct watchdog_device *wdd)
@@ -66,42 +75,45 @@ static int davinci_wdt_start(struct watchdog_device *wdd)
 	u32 tgcr;
 	u32 timer_margin;
 	unsigned long wdt_freq;
+	struct davinci_wdt_device *davinci_wdt = watchdog_get_drvdata(wdd);
 
-	wdt_freq = clk_get_rate(wdt_clk);
+	wdt_freq = clk_get_rate(davinci_wdt->clk);
 
 	/* disable, internal clock source */
-	iowrite32(0, wdt_base + TCR);
+	iowrite32(0, davinci_wdt->base + TCR);
 	/* reset timer, set mode to 64-bit watchdog, and unreset */
-	iowrite32(0, wdt_base + TGCR);
+	iowrite32(0, davinci_wdt->base + TGCR);
 	tgcr = TIMMODE_64BIT_WDOG | TIM12RS_UNRESET | TIM34RS_UNRESET;
-	iowrite32(tgcr, wdt_base + TGCR);
+	iowrite32(tgcr, davinci_wdt->base + TGCR);
 	/* clear counter regs */
-	iowrite32(0, wdt_base + TIM12);
-	iowrite32(0, wdt_base + TIM34);
+	iowrite32(0, davinci_wdt->base + TIM12);
+	iowrite32(0, davinci_wdt->base + TIM34);
 	/* set timeout period */
 	timer_margin = (((u64)wdd->timeout * wdt_freq) & 0xffffffff);
-	iowrite32(timer_margin, wdt_base + PRD12);
+	iowrite32(timer_margin, davinci_wdt->base + PRD12);
 	timer_margin = (((u64)wdd->timeout * wdt_freq) >> 32);
-	iowrite32(timer_margin, wdt_base + PRD34);
+	iowrite32(timer_margin, davinci_wdt->base + PRD34);
 	/* enable run continuously */
-	iowrite32(ENAMODE12_PERIODIC, wdt_base + TCR);
+	iowrite32(ENAMODE12_PERIODIC, davinci_wdt->base + TCR);
 	/* Once the WDT is in pre-active state write to
 	 * TIM12, TIM34, PRD12, PRD34, TCR, TGCR, WDTCR are
 	 * write protected (except for the WDKEY field)
 	 */
 	/* put watchdog in pre-active state */
-	iowrite32(WDKEY_SEQ0 | WDEN, wdt_base + WDTCR);
+	iowrite32(WDKEY_SEQ0 | WDEN, davinci_wdt->base + WDTCR);
 	/* put watchdog in active state */
-	iowrite32(WDKEY_SEQ1 | WDEN, wdt_base + WDTCR);
+	iowrite32(WDKEY_SEQ1 | WDEN, davinci_wdt->base + WDTCR);
 	return 0;
 }
 
 static int davinci_wdt_ping(struct watchdog_device *wdd)
 {
+	struct davinci_wdt_device *davinci_wdt = watchdog_get_drvdata(wdd);
+
 	/* put watchdog in service state */
-	iowrite32(WDKEY_SEQ0, wdt_base + WDTCR);
+	iowrite32(WDKEY_SEQ0, davinci_wdt->base + WDTCR);
 	/* put watchdog in active state */
-	iowrite32(WDKEY_SEQ1, wdt_base + WDTCR);
+	iowrite32(WDKEY_SEQ1, davinci_wdt->base + WDTCR);
 	return 0;
 }
 
@@ -123,14 +135,21 @@ static int davinci_wdt_probe(struct platform_device *pdev)
 	struct device *dev = &pdev->dev;
 	struct resource  *wdt_mem;
 	struct watchdog_device *wdd;
+	struct davinci_wdt_device *davinci_wdt;
+
+	davinci_wdt = devm_kzalloc(dev, sizeof(*davinci_wdt), GFP_KERNEL);
+	if (!davinci_wdt)
+		return -ENOMEM;
 
-	wdt_clk = devm_clk_get(dev, NULL);
-	if (WARN_ON(IS_ERR(wdt_clk)))
-		return PTR_ERR(wdt_clk);
+	davinci_wdt->clk = devm_clk_get(dev, NULL);
+	if (WARN_ON(IS_ERR(davinci_wdt->clk)))
+		return PTR_ERR(davinci_wdt->clk);
 
-	clk_prepare_enable(wdt_clk);
+	clk_prepare_enable(davinci_wdt->clk);
 
-	wdd			= &wdt_wdd;
+	platform_set_drvdata(pdev, davinci_wdt);
+
+	wdd			= &davinci_wdt->wdd;
 	wdd->info		= &davinci_wdt_info;
 	wdd->ops		= &davinci_wdt_ops;
 	wdd->min_timeout 	= 1;
@@ -142,12 +161,13 @@ static int davinci_wdt_probe(struct platform_device *pdev)
 
 	dev_info(dev, "heartbeat %d sec\n", wdd->timeout);
 
+	watchdog_set_drvdata(wdd, davinci_wdt);
 	watchdog_set_nowayout(wdd, WATCHDOG_NOWAYOUT);
 
 	wdt_mem = platform_get_resource(pdev, IORESOURCE_MEM, 0);
-	wdt_base = devm_ioremap_resource(dev, wdt_mem);
-	if (IS_ERR(wdt_base))
-		return PTR_ERR(wdt_base);
+	davinci_wdt->base = devm_ioremap_resource(dev, wdt_mem);
+	if (IS_ERR(davinci_wdt->base))
+		return PTR_ERR(davinci_wdt->base);
 
 	ret = watchdog_register_device(wdd);
 	if (ret < 0)
@@ -158,8 +178,10 @@ static int davinci_wdt_probe(struct platform_device *pdev)
 
 static int davinci_wdt_remove(struct platform_device *pdev)
 {
-	watchdog_unregister_device(&wdt_wdd);
-	clk_disable_unprepare(wdt_clk);
+	struct davinci_wdt_device *davinci_wdt = platform_get_drvdata(pdev);
+
+	watchdog_unregister_device(&davinci_wdt->wdd);
+	clk_disable_unprepare(davinci_wdt->clk);
 
 	return 0;
 }
-- 
1.7.9.5

^ permalink raw reply related	[flat|nested] 14+ messages in thread
* Re: Fwd: [PATCH 2/8] watchdog: davinci: use davinci_wdt_device structure to hold device data
  2013-11-06 11:31   ` ivan.khoronzhuk
  (?)
@ 2013-11-12 15:37     ` Santosh Shilimkar
  -1 siblings, 0 replies; 14+ messages in thread
From: Santosh Shilimkar @ 2013-11-12 15:37 UTC (permalink / raw)
  To: ivan.khoronzhuk
  Cc: wim, nsekhar, linux-watchdog, devicetree, grant.likely,
	rob.herring, pawel.moll, mark.rutland, swarren, galak,
	ijc+devicetree, linux-kernel, linux-arm-kernel

On Wednesday 06 November 2013 06:31 AM, ivan.khoronzhuk wrote:
> Some SoCs, like Keystone 2, can support more than one WDT and each
> watchdog device has to use it's own base address, clock source,
> wdd device, so add new davinci_wdt_device structure to hold device
In commit avoid struct names ;)
s/wdd/watchdog device
> data.
> 
> Signed-off-by: Ivan Khoronzhuk <ivan.khoronzhuk@ti.com>
> ---
>  drivers/watchdog/davinci_wdt.c |   74 ++++++++++++++++++++++++++--------------
>  1 file changed, 48 insertions(+), 26 deletions(-)
> 
> diff --git a/drivers/watchdog/davinci_wdt.c b/drivers/watchdog/davinci_wdt.c
> index a6eef71..1fc2093 100644
> --- a/drivers/watchdog/davinci_wdt.c
> +++ b/drivers/watchdog/davinci_wdt.c

[...]

> @@ -123,14 +135,21 @@ static int davinci_wdt_probe(struct platform_device *pdev)
>  	struct device *dev = &pdev->dev;
>  	struct resource  *wdt_mem;
>  	struct watchdog_device *wdd;
> +	struct davinci_wdt_device *davinci_wdt;
> +
> +	davinci_wdt = devm_kzalloc(dev, sizeof(*davinci_wdt), GFP_KERNEL);
> +	if (!davinci_wdt)
> +		return -ENOMEM;
>  
> -	wdt_clk = devm_clk_get(dev, NULL);
> -	if (WARN_ON(IS_ERR(wdt_clk)))
> -		return PTR_ERR(wdt_clk);
> +	davinci_wdt->clk = devm_clk_get(dev, NULL);
> +	if (WARN_ON(IS_ERR(davinci_wdt->clk)))
> +		return PTR_ERR(davinci_wdt->clk);
>  
> -	clk_prepare_enable(wdt_clk);
> +	clk_prepare_enable(davinci_wdt->clk);
>  
> -	wdd			= &wdt_wdd;
> +	platform_set_drvdata(pdev, davinci_wdt);
> +
> +	wdd			= &davinci_wdt->wdd;
>  	wdd->info		= &davinci_wdt_info;
>  	wdd->ops		= &davinci_wdt_ops;
>  	wdd->min_timeout 	= 1;
> @@ -142,12 +161,13 @@ static int davinci_wdt_probe(struct platform_device *pdev)
>  
>  	dev_info(dev, "heartbeat %d sec\n", wdd->timeout);
>  
> +	watchdog_set_drvdata(wdd, davinci_wdt);
>  	watchdog_set_nowayout(wdd, WATCHDOG_NOWAYOUT);
>  
>  	wdt_mem = platform_get_resource(pdev, IORESOURCE_MEM, 0);
> -	wdt_base = devm_ioremap_resource(dev, wdt_mem);
> -	if (IS_ERR(wdt_base))
> -		return PTR_ERR(wdt_base);
> +	davinci_wdt->base = devm_ioremap_resource(dev, wdt_mem);
> +	if (IS_ERR(davinci_wdt->base))
> +		return PTR_ERR(davinci_wdt->base);
You should free up davinci_wdt memory before returning, right ?

Other than that patch looks fine to me. With above fixed,
Acked-by: Santosh Shilimkar <santosh.shilimkar@ti.com>

^ permalink raw reply	[flat|nested] 14+ messages in thread
* Re: Fwd: [PATCH 2/8] watchdog: davinci: use davinci_wdt_device structure to hold device data
@ 2013-11-12 15:37     ` Santosh Shilimkar
  0 siblings, 0 replies; 14+ messages in thread
From: Santosh Shilimkar @ 2013-11-12 15:37 UTC (permalink / raw)
  To: ivan.khoronzhuk
  Cc: mark.rutland, devicetree, linux-watchdog, pawel.moll, swarren,
	ijc+devicetree, nsekhar, galak, rob.herring, linux-kernel, wim,
	grant.likely, linux-arm-kernel

On Wednesday 06 November 2013 06:31 AM, ivan.khoronzhuk wrote:
> Some SoCs, like Keystone 2, can support more than one WDT and each
> watchdog device has to use it's own base address, clock source,
> wdd device, so add new davinci_wdt_device structure to hold device
In commit avoid struct names ;)
s/wdd/watchdog device
> data.
> 
> Signed-off-by: Ivan Khoronzhuk <ivan.khoronzhuk@ti.com>
> ---
>  drivers/watchdog/davinci_wdt.c |   74 ++++++++++++++++++++++++++--------------
>  1 file changed, 48 insertions(+), 26 deletions(-)
> 
> diff --git a/drivers/watchdog/davinci_wdt.c b/drivers/watchdog/davinci_wdt.c
> index a6eef71..1fc2093 100644
> --- a/drivers/watchdog/davinci_wdt.c
> +++ b/drivers/watchdog/davinci_wdt.c

[...]

> @@ -123,14 +135,21 @@ static int davinci_wdt_probe(struct platform_device *pdev)
>  	struct device *dev = &pdev->dev;
>  	struct resource  *wdt_mem;
>  	struct watchdog_device *wdd;
> +	struct davinci_wdt_device *davinci_wdt;
> +
> +	davinci_wdt = devm_kzalloc(dev, sizeof(*davinci_wdt), GFP_KERNEL);
> +	if (!davinci_wdt)
> +		return -ENOMEM;
>  
> -	wdt_clk = devm_clk_get(dev, NULL);
> -	if (WARN_ON(IS_ERR(wdt_clk)))
> -		return PTR_ERR(wdt_clk);
> +	davinci_wdt->clk = devm_clk_get(dev, NULL);
> +	if (WARN_ON(IS_ERR(davinci_wdt->clk)))
> +		return PTR_ERR(davinci_wdt->clk);
>  
> -	clk_prepare_enable(wdt_clk);
> +	clk_prepare_enable(davinci_wdt->clk);
>  
> -	wdd			= &wdt_wdd;
> +	platform_set_drvdata(pdev, davinci_wdt);
> +
> +	wdd			= &davinci_wdt->wdd;
>  	wdd->info		= &davinci_wdt_info;
>  	wdd->ops		= &davinci_wdt_ops;
>  	wdd->min_timeout 	= 1;
> @@ -142,12 +161,13 @@ static int davinci_wdt_probe(struct platform_device *pdev)
>  
>  	dev_info(dev, "heartbeat %d sec\n", wdd->timeout);
>  
> +	watchdog_set_drvdata(wdd, davinci_wdt);
>  	watchdog_set_nowayout(wdd, WATCHDOG_NOWAYOUT);
>  
>  	wdt_mem = platform_get_resource(pdev, IORESOURCE_MEM, 0);
> -	wdt_base = devm_ioremap_resource(dev, wdt_mem);
> -	if (IS_ERR(wdt_base))
> -		return PTR_ERR(wdt_base);
> +	davinci_wdt->base = devm_ioremap_resource(dev, wdt_mem);
> +	if (IS_ERR(davinci_wdt->base))
> +		return PTR_ERR(davinci_wdt->base);
You should free up davinci_wdt memory before returning, right ?

Other than that patch looks fine to me. With above fixed,
Acked-by: Santosh Shilimkar <santosh.shilimkar@ti.com>

^ permalink raw reply	[flat|nested] 14+ messages in thread
* Fwd: [PATCH 2/8] watchdog: davinci: use davinci_wdt_device structure to hold device data
@ 2013-11-12 15:37     ` Santosh Shilimkar
  0 siblings, 0 replies; 14+ messages in thread
From: Santosh Shilimkar @ 2013-11-12 15:37 UTC (permalink / raw)
  To: linux-arm-kernel

On Wednesday 06 November 2013 06:31 AM, ivan.khoronzhuk wrote:
> Some SoCs, like Keystone 2, can support more than one WDT and each
> watchdog device has to use it's own base address, clock source,
> wdd device, so add new davinci_wdt_device structure to hold device
In commit avoid struct names ;)
s/wdd/watchdog device
> data.
> 
> Signed-off-by: Ivan Khoronzhuk <ivan.khoronzhuk@ti.com>
> ---
>  drivers/watchdog/davinci_wdt.c |   74 ++++++++++++++++++++++++++--------------
>  1 file changed, 48 insertions(+), 26 deletions(-)
> 
> diff --git a/drivers/watchdog/davinci_wdt.c b/drivers/watchdog/davinci_wdt.c
> index a6eef71..1fc2093 100644
> --- a/drivers/watchdog/davinci_wdt.c
> +++ b/drivers/watchdog/davinci_wdt.c

[...]

> @@ -123,14 +135,21 @@ static int davinci_wdt_probe(struct platform_device *pdev)
>  	struct device *dev = &pdev->dev;
>  	struct resource  *wdt_mem;
>  	struct watchdog_device *wdd;
> +	struct davinci_wdt_device *davinci_wdt;
> +
> +	davinci_wdt = devm_kzalloc(dev, sizeof(*davinci_wdt), GFP_KERNEL);
> +	if (!davinci_wdt)
> +		return -ENOMEM;
>  
> -	wdt_clk = devm_clk_get(dev, NULL);
> -	if (WARN_ON(IS_ERR(wdt_clk)))
> -		return PTR_ERR(wdt_clk);
> +	davinci_wdt->clk = devm_clk_get(dev, NULL);
> +	if (WARN_ON(IS_ERR(davinci_wdt->clk)))
> +		return PTR_ERR(davinci_wdt->clk);
>  
> -	clk_prepare_enable(wdt_clk);
> +	clk_prepare_enable(davinci_wdt->clk);
>  
> -	wdd			= &wdt_wdd;
> +	platform_set_drvdata(pdev, davinci_wdt);
> +
> +	wdd			= &davinci_wdt->wdd;
>  	wdd->info		= &davinci_wdt_info;
>  	wdd->ops		= &davinci_wdt_ops;
>  	wdd->min_timeout 	= 1;
> @@ -142,12 +161,13 @@ static int davinci_wdt_probe(struct platform_device *pdev)
>  
>  	dev_info(dev, "heartbeat %d sec\n", wdd->timeout);
>  
> +	watchdog_set_drvdata(wdd, davinci_wdt);
>  	watchdog_set_nowayout(wdd, WATCHDOG_NOWAYOUT);
>  
>  	wdt_mem = platform_get_resource(pdev, IORESOURCE_MEM, 0);
> -	wdt_base = devm_ioremap_resource(dev, wdt_mem);
> -	if (IS_ERR(wdt_base))
> -		return PTR_ERR(wdt_base);
> +	davinci_wdt->base = devm_ioremap_resource(dev, wdt_mem);
> +	if (IS_ERR(davinci_wdt->base))
> +		return PTR_ERR(davinci_wdt->base);
You should free up davinci_wdt memory before returning, right ?

Other than that patch looks fine to me. With above fixed,
Acked-by: Santosh Shilimkar <santosh.shilimkar@ti.com>

^ permalink raw reply	[flat|nested] 14+ messages in thread
* Re: Fwd: [PATCH 2/8] watchdog: davinci: use davinci_wdt_device structure to hold device data
  2013-11-12 15:37     ` Santosh Shilimkar
@ 2013-11-12 16:27       ` Guenter Roeck
  -1 siblings, 0 replies; 14+ messages in thread
From: Guenter Roeck @ 2013-11-12 16:27 UTC (permalink / raw)
  To: Santosh Shilimkar
  Cc: ivan.khoronzhuk, wim, nsekhar, linux-watchdog, devicetree,
	grant.likely, rob.herring, pawel.moll, mark.rutland, swarren,
	galak, ijc+devicetree, linux-kernel, linux-arm-kernel

On Tue, Nov 12, 2013 at 10:37:04AM -0500, Santosh Shilimkar wrote:
> On Wednesday 06 November 2013 06:31 AM, ivan.khoronzhuk wrote:
> > Some SoCs, like Keystone 2, can support more than one WDT and each
> > watchdog device has to use it's own base address, clock source,
> > wdd device, so add new davinci_wdt_device structure to hold device
> In commit avoid struct names ;)
> s/wdd/watchdog device
> > data.
> > 
> > Signed-off-by: Ivan Khoronzhuk <ivan.khoronzhuk@ti.com>
> > ---
> >  drivers/watchdog/davinci_wdt.c |   74 ++++++++++++++++++++++++++--------------
> >  1 file changed, 48 insertions(+), 26 deletions(-)
> > 
> > diff --git a/drivers/watchdog/davinci_wdt.c b/drivers/watchdog/davinci_wdt.c
> > index a6eef71..1fc2093 100644
> > --- a/drivers/watchdog/davinci_wdt.c
> > +++ b/drivers/watchdog/davinci_wdt.c
> 
> [...]
> 
> > @@ -123,14 +135,21 @@ static int davinci_wdt_probe(struct platform_device *pdev)
> >  	struct device *dev = &pdev->dev;
> >  	struct resource  *wdt_mem;
> >  	struct watchdog_device *wdd;
> > +	struct davinci_wdt_device *davinci_wdt;
> > +
> > +	davinci_wdt = devm_kzalloc(dev, sizeof(*davinci_wdt), GFP_KERNEL);
> > +	if (!davinci_wdt)
> > +		return -ENOMEM;
> >  
> > -	wdt_clk = devm_clk_get(dev, NULL);
> > -	if (WARN_ON(IS_ERR(wdt_clk)))
> > -		return PTR_ERR(wdt_clk);
> > +	davinci_wdt->clk = devm_clk_get(dev, NULL);
> > +	if (WARN_ON(IS_ERR(davinci_wdt->clk)))
> > +		return PTR_ERR(davinci_wdt->clk);
> >  
> > -	clk_prepare_enable(wdt_clk);
> > +	clk_prepare_enable(davinci_wdt->clk);
> >  
> > -	wdd			= &wdt_wdd;
> > +	platform_set_drvdata(pdev, davinci_wdt);
> > +
> > +	wdd			= &davinci_wdt->wdd;
> >  	wdd->info		= &davinci_wdt_info;
> >  	wdd->ops		= &davinci_wdt_ops;
> >  	wdd->min_timeout 	= 1;
> > @@ -142,12 +161,13 @@ static int davinci_wdt_probe(struct platform_device *pdev)
> >  
> >  	dev_info(dev, "heartbeat %d sec\n", wdd->timeout);
> >  
> > +	watchdog_set_drvdata(wdd, davinci_wdt);
> >  	watchdog_set_nowayout(wdd, WATCHDOG_NOWAYOUT);
> >  
> >  	wdt_mem = platform_get_resource(pdev, IORESOURCE_MEM, 0);
> > -	wdt_base = devm_ioremap_resource(dev, wdt_mem);
> > -	if (IS_ERR(wdt_base))
> > -		return PTR_ERR(wdt_base);
> > +	davinci_wdt->base = devm_ioremap_resource(dev, wdt_mem);
> > +	if (IS_ERR(davinci_wdt->base))
> > +		return PTR_ERR(davinci_wdt->base);
> You should free up davinci_wdt memory before returning, right ?
> 
No, devm should take care of that.

Guenter

> Other than that patch looks fine to me. With above fixed,
> Acked-by: Santosh Shilimkar <santosh.shilimkar@ti.com>
> --
> To unsubscribe from this list: send the line "unsubscribe linux-watchdog" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html
> 

^ permalink raw reply	[flat|nested] 14+ messages in thread
* Fwd: [PATCH 2/8] watchdog: davinci: use davinci_wdt_device structure to hold device data
@ 2013-11-12 16:27       ` Guenter Roeck
  0 siblings, 0 replies; 14+ messages in thread
From: Guenter Roeck @ 2013-11-12 16:27 UTC (permalink / raw)
  To: linux-arm-kernel

On Tue, Nov 12, 2013 at 10:37:04AM -0500, Santosh Shilimkar wrote:
> On Wednesday 06 November 2013 06:31 AM, ivan.khoronzhuk wrote:
> > Some SoCs, like Keystone 2, can support more than one WDT and each
> > watchdog device has to use it's own base address, clock source,
> > wdd device, so add new davinci_wdt_device structure to hold device
> In commit avoid struct names ;)
> s/wdd/watchdog device
> > data.
> > 
> > Signed-off-by: Ivan Khoronzhuk <ivan.khoronzhuk@ti.com>
> > ---
> >  drivers/watchdog/davinci_wdt.c |   74 ++++++++++++++++++++++++++--------------
> >  1 file changed, 48 insertions(+), 26 deletions(-)
> > 
> > diff --git a/drivers/watchdog/davinci_wdt.c b/drivers/watchdog/davinci_wdt.c
> > index a6eef71..1fc2093 100644
> > --- a/drivers/watchdog/davinci_wdt.c
> > +++ b/drivers/watchdog/davinci_wdt.c
> 
> [...]
> 
> > @@ -123,14 +135,21 @@ static int davinci_wdt_probe(struct platform_device *pdev)
> >  	struct device *dev = &pdev->dev;
> >  	struct resource  *wdt_mem;
> >  	struct watchdog_device *wdd;
> > +	struct davinci_wdt_device *davinci_wdt;
> > +
> > +	davinci_wdt = devm_kzalloc(dev, sizeof(*davinci_wdt), GFP_KERNEL);
> > +	if (!davinci_wdt)
> > +		return -ENOMEM;
> >  
> > -	wdt_clk = devm_clk_get(dev, NULL);
> > -	if (WARN_ON(IS_ERR(wdt_clk)))
> > -		return PTR_ERR(wdt_clk);
> > +	davinci_wdt->clk = devm_clk_get(dev, NULL);
> > +	if (WARN_ON(IS_ERR(davinci_wdt->clk)))
> > +		return PTR_ERR(davinci_wdt->clk);
> >  
> > -	clk_prepare_enable(wdt_clk);
> > +	clk_prepare_enable(davinci_wdt->clk);
> >  
> > -	wdd			= &wdt_wdd;
> > +	platform_set_drvdata(pdev, davinci_wdt);
> > +
> > +	wdd			= &davinci_wdt->wdd;
> >  	wdd->info		= &davinci_wdt_info;
> >  	wdd->ops		= &davinci_wdt_ops;
> >  	wdd->min_timeout 	= 1;
> > @@ -142,12 +161,13 @@ static int davinci_wdt_probe(struct platform_device *pdev)
> >  
> >  	dev_info(dev, "heartbeat %d sec\n", wdd->timeout);
> >  
> > +	watchdog_set_drvdata(wdd, davinci_wdt);
> >  	watchdog_set_nowayout(wdd, WATCHDOG_NOWAYOUT);
> >  
> >  	wdt_mem = platform_get_resource(pdev, IORESOURCE_MEM, 0);
> > -	wdt_base = devm_ioremap_resource(dev, wdt_mem);
> > -	if (IS_ERR(wdt_base))
> > -		return PTR_ERR(wdt_base);
> > +	davinci_wdt->base = devm_ioremap_resource(dev, wdt_mem);
> > +	if (IS_ERR(davinci_wdt->base))
> > +		return PTR_ERR(davinci_wdt->base);
> You should free up davinci_wdt memory before returning, right ?
> 
No, devm should take care of that.

Guenter

> Other than that patch looks fine to me. With above fixed,
> Acked-by: Santosh Shilimkar <santosh.shilimkar@ti.com>
> --
> To unsubscribe from this list: send the line "unsubscribe linux-watchdog" in
> the body of a message to majordomo at vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html
> 

^ permalink raw reply	[flat|nested] 14+ messages in thread
* Re: Fwd: [PATCH 2/8] watchdog: davinci: use davinci_wdt_device structure to hold device data
  2013-11-12 16:27       ` Guenter Roeck
  (?)
@ 2013-11-12 16:28         ` Santosh Shilimkar
  -1 siblings, 0 replies; 14+ messages in thread
From: Santosh Shilimkar @ 2013-11-12 16:28 UTC (permalink / raw)
  To: Guenter Roeck
  Cc: ivan.khoronzhuk, wim, nsekhar, linux-watchdog, devicetree,
	grant.likely, rob.herring, pawel.moll, mark.rutland, swarren,
	galak, ijc+devicetree, linux-kernel, linux-arm-kernel

On Tuesday 12 November 2013 11:27 AM, Guenter Roeck wrote:
> On Tue, Nov 12, 2013 at 10:37:04AM -0500, Santosh Shilimkar wrote:
>> On Wednesday 06 November 2013 06:31 AM, ivan.khoronzhuk wrote:
>>> Some SoCs, like Keystone 2, can support more than one WDT and each
>>> watchdog device has to use it's own base address, clock source,
>>> wdd device, so add new davinci_wdt_device structure to hold device
>> In commit avoid struct names ;)
>> s/wdd/watchdog device
>>> data.
>>>
>>> Signed-off-by: Ivan Khoronzhuk <ivan.khoronzhuk@ti.com>
>>> ---
>>>  drivers/watchdog/davinci_wdt.c |   74 ++++++++++++++++++++++++++--------------
>>>  1 file changed, 48 insertions(+), 26 deletions(-)
>>>
>>> diff --git a/drivers/watchdog/davinci_wdt.c b/drivers/watchdog/davinci_wdt.c
>>> index a6eef71..1fc2093 100644
>>> --- a/drivers/watchdog/davinci_wdt.c
>>> +++ b/drivers/watchdog/davinci_wdt.c
>>
>> [...]
>>
>>> @@ -123,14 +135,21 @@ static int davinci_wdt_probe(struct platform_device *pdev)
>>>  	struct device *dev = &pdev->dev;
>>>  	struct resource  *wdt_mem;
>>>  	struct watchdog_device *wdd;
>>> +	struct davinci_wdt_device *davinci_wdt;
>>> +
>>> +	davinci_wdt = devm_kzalloc(dev, sizeof(*davinci_wdt), GFP_KERNEL);
>>> +	if (!davinci_wdt)
>>> +		return -ENOMEM;
>>>  
>>> -	wdt_clk = devm_clk_get(dev, NULL);
>>> -	if (WARN_ON(IS_ERR(wdt_clk)))
>>> -		return PTR_ERR(wdt_clk);
>>> +	davinci_wdt->clk = devm_clk_get(dev, NULL);
>>> +	if (WARN_ON(IS_ERR(davinci_wdt->clk)))
>>> +		return PTR_ERR(davinci_wdt->clk);
>>>  
>>> -	clk_prepare_enable(wdt_clk);
>>> +	clk_prepare_enable(davinci_wdt->clk);
>>>  
>>> -	wdd			= &wdt_wdd;
>>> +	platform_set_drvdata(pdev, davinci_wdt);
>>> +
>>> +	wdd			= &davinci_wdt->wdd;
>>>  	wdd->info		= &davinci_wdt_info;
>>>  	wdd->ops		= &davinci_wdt_ops;
>>>  	wdd->min_timeout 	= 1;
>>> @@ -142,12 +161,13 @@ static int davinci_wdt_probe(struct platform_device *pdev)
>>>  
>>>  	dev_info(dev, "heartbeat %d sec\n", wdd->timeout);
>>>  
>>> +	watchdog_set_drvdata(wdd, davinci_wdt);
>>>  	watchdog_set_nowayout(wdd, WATCHDOG_NOWAYOUT);
>>>  
>>>  	wdt_mem = platform_get_resource(pdev, IORESOURCE_MEM, 0);
>>> -	wdt_base = devm_ioremap_resource(dev, wdt_mem);
>>> -	if (IS_ERR(wdt_base))
>>> -		return PTR_ERR(wdt_base);
>>> +	davinci_wdt->base = devm_ioremap_resource(dev, wdt_mem);
>>> +	if (IS_ERR(davinci_wdt->base))
>>> +		return PTR_ERR(davinci_wdt->base);
>> You should free up davinci_wdt memory before returning, right ?
>>
> No, devm should take care of that.
> 
You are right. I didn't pay attention about the devm_*() usage.

Regards,
Santosh


^ permalink raw reply	[flat|nested] 14+ messages in thread
* Re: Fwd: [PATCH 2/8] watchdog: davinci: use davinci_wdt_device structure to hold device data
@ 2013-11-12 16:28         ` Santosh Shilimkar
  0 siblings, 0 replies; 14+ messages in thread
From: Santosh Shilimkar @ 2013-11-12 16:28 UTC (permalink / raw)
  To: Guenter Roeck
  Cc: mark.rutland, devicetree, linux-watchdog, pawel.moll, swarren,
	ijc+devicetree, nsekhar, galak, rob.herring, linux-kernel, wim,
	grant.likely, ivan.khoronzhuk, linux-arm-kernel

On Tuesday 12 November 2013 11:27 AM, Guenter Roeck wrote:
> On Tue, Nov 12, 2013 at 10:37:04AM -0500, Santosh Shilimkar wrote:
>> On Wednesday 06 November 2013 06:31 AM, ivan.khoronzhuk wrote:
>>> Some SoCs, like Keystone 2, can support more than one WDT and each
>>> watchdog device has to use it's own base address, clock source,
>>> wdd device, so add new davinci_wdt_device structure to hold device
>> In commit avoid struct names ;)
>> s/wdd/watchdog device
>>> data.
>>>
>>> Signed-off-by: Ivan Khoronzhuk <ivan.khoronzhuk@ti.com>
>>> ---
>>>  drivers/watchdog/davinci_wdt.c |   74 ++++++++++++++++++++++++++--------------
>>>  1 file changed, 48 insertions(+), 26 deletions(-)
>>>
>>> diff --git a/drivers/watchdog/davinci_wdt.c b/drivers/watchdog/davinci_wdt.c
>>> index a6eef71..1fc2093 100644
>>> --- a/drivers/watchdog/davinci_wdt.c
>>> +++ b/drivers/watchdog/davinci_wdt.c
>>
>> [...]
>>
>>> @@ -123,14 +135,21 @@ static int davinci_wdt_probe(struct platform_device *pdev)
>>>  	struct device *dev = &pdev->dev;
>>>  	struct resource  *wdt_mem;
>>>  	struct watchdog_device *wdd;
>>> +	struct davinci_wdt_device *davinci_wdt;
>>> +
>>> +	davinci_wdt = devm_kzalloc(dev, sizeof(*davinci_wdt), GFP_KERNEL);
>>> +	if (!davinci_wdt)
>>> +		return -ENOMEM;
>>>  
>>> -	wdt_clk = devm_clk_get(dev, NULL);
>>> -	if (WARN_ON(IS_ERR(wdt_clk)))
>>> -		return PTR_ERR(wdt_clk);
>>> +	davinci_wdt->clk = devm_clk_get(dev, NULL);
>>> +	if (WARN_ON(IS_ERR(davinci_wdt->clk)))
>>> +		return PTR_ERR(davinci_wdt->clk);
>>>  
>>> -	clk_prepare_enable(wdt_clk);
>>> +	clk_prepare_enable(davinci_wdt->clk);
>>>  
>>> -	wdd			= &wdt_wdd;
>>> +	platform_set_drvdata(pdev, davinci_wdt);
>>> +
>>> +	wdd			= &davinci_wdt->wdd;
>>>  	wdd->info		= &davinci_wdt_info;
>>>  	wdd->ops		= &davinci_wdt_ops;
>>>  	wdd->min_timeout 	= 1;
>>> @@ -142,12 +161,13 @@ static int davinci_wdt_probe(struct platform_device *pdev)
>>>  
>>>  	dev_info(dev, "heartbeat %d sec\n", wdd->timeout);
>>>  
>>> +	watchdog_set_drvdata(wdd, davinci_wdt);
>>>  	watchdog_set_nowayout(wdd, WATCHDOG_NOWAYOUT);
>>>  
>>>  	wdt_mem = platform_get_resource(pdev, IORESOURCE_MEM, 0);
>>> -	wdt_base = devm_ioremap_resource(dev, wdt_mem);
>>> -	if (IS_ERR(wdt_base))
>>> -		return PTR_ERR(wdt_base);
>>> +	davinci_wdt->base = devm_ioremap_resource(dev, wdt_mem);
>>> +	if (IS_ERR(davinci_wdt->base))
>>> +		return PTR_ERR(davinci_wdt->base);
>> You should free up davinci_wdt memory before returning, right ?
>>
> No, devm should take care of that.
> 
You are right. I didn't pay attention about the devm_*() usage.

Regards,
Santosh

^ permalink raw reply	[flat|nested] 14+ messages in thread
* Fwd: [PATCH 2/8] watchdog: davinci: use davinci_wdt_device structure to hold device data
@ 2013-11-12 16:28         ` Santosh Shilimkar
  0 siblings, 0 replies; 14+ messages in thread
From: Santosh Shilimkar @ 2013-11-12 16:28 UTC (permalink / raw)
  To: linux-arm-kernel

On Tuesday 12 November 2013 11:27 AM, Guenter Roeck wrote:
> On Tue, Nov 12, 2013 at 10:37:04AM -0500, Santosh Shilimkar wrote:
>> On Wednesday 06 November 2013 06:31 AM, ivan.khoronzhuk wrote:
>>> Some SoCs, like Keystone 2, can support more than one WDT and each
>>> watchdog device has to use it's own base address, clock source,
>>> wdd device, so add new davinci_wdt_device structure to hold device
>> In commit avoid struct names ;)
>> s/wdd/watchdog device
>>> data.
>>>
>>> Signed-off-by: Ivan Khoronzhuk <ivan.khoronzhuk@ti.com>
>>> ---
>>>  drivers/watchdog/davinci_wdt.c |   74 ++++++++++++++++++++++++++--------------
>>>  1 file changed, 48 insertions(+), 26 deletions(-)
>>>
>>> diff --git a/drivers/watchdog/davinci_wdt.c b/drivers/watchdog/davinci_wdt.c
>>> index a6eef71..1fc2093 100644
>>> --- a/drivers/watchdog/davinci_wdt.c
>>> +++ b/drivers/watchdog/davinci_wdt.c
>>
>> [...]
>>
>>> @@ -123,14 +135,21 @@ static int davinci_wdt_probe(struct platform_device *pdev)
>>>  	struct device *dev = &pdev->dev;
>>>  	struct resource  *wdt_mem;
>>>  	struct watchdog_device *wdd;
>>> +	struct davinci_wdt_device *davinci_wdt;
>>> +
>>> +	davinci_wdt = devm_kzalloc(dev, sizeof(*davinci_wdt), GFP_KERNEL);
>>> +	if (!davinci_wdt)
>>> +		return -ENOMEM;
>>>  
>>> -	wdt_clk = devm_clk_get(dev, NULL);
>>> -	if (WARN_ON(IS_ERR(wdt_clk)))
>>> -		return PTR_ERR(wdt_clk);
>>> +	davinci_wdt->clk = devm_clk_get(dev, NULL);
>>> +	if (WARN_ON(IS_ERR(davinci_wdt->clk)))
>>> +		return PTR_ERR(davinci_wdt->clk);
>>>  
>>> -	clk_prepare_enable(wdt_clk);
>>> +	clk_prepare_enable(davinci_wdt->clk);
>>>  
>>> -	wdd			= &wdt_wdd;
>>> +	platform_set_drvdata(pdev, davinci_wdt);
>>> +
>>> +	wdd			= &davinci_wdt->wdd;
>>>  	wdd->info		= &davinci_wdt_info;
>>>  	wdd->ops		= &davinci_wdt_ops;
>>>  	wdd->min_timeout 	= 1;
>>> @@ -142,12 +161,13 @@ static int davinci_wdt_probe(struct platform_device *pdev)
>>>  
>>>  	dev_info(dev, "heartbeat %d sec\n", wdd->timeout);
>>>  
>>> +	watchdog_set_drvdata(wdd, davinci_wdt);
>>>  	watchdog_set_nowayout(wdd, WATCHDOG_NOWAYOUT);
>>>  
>>>  	wdt_mem = platform_get_resource(pdev, IORESOURCE_MEM, 0);
>>> -	wdt_base = devm_ioremap_resource(dev, wdt_mem);
>>> -	if (IS_ERR(wdt_base))
>>> -		return PTR_ERR(wdt_base);
>>> +	davinci_wdt->base = devm_ioremap_resource(dev, wdt_mem);
>>> +	if (IS_ERR(davinci_wdt->base))
>>> +		return PTR_ERR(davinci_wdt->base);
>> You should free up davinci_wdt memory before returning, right ?
>>
> No, devm should take care of that.
> 
You are right. I didn't pay attention about the devm_*() usage.

Regards,
Santosh

^ permalink raw reply	[flat|nested] 14+ messages in thread
* Re: Fwd: [PATCH 2/8] watchdog: davinci: use davinci_wdt_device structure to hold device data
@ 2013-11-17  2:19     ` Guenter Roeck
  0 siblings, 0 replies; 14+ messages in thread
From: Guenter Roeck @ 2013-11-17  2:19 UTC (permalink / raw)
  To: ivan.khoronzhuk, Santosh Shilimkar, wim, nsekhar, linux-watchdog,
	devicetree
  Cc: grant.likely, rob.herring, pawel.moll, mark.rutland, swarren,
	galak, ijc+devicetree, linux-kernel, linux-arm-kernel

On 11/06/2013 03:31 AM, ivan.khoronzhuk wrote:
> Some SoCs, like Keystone 2, can support more than one WDT and each
> watchdog device has to use it's own base address, clock source,
> wdd device, so add new davinci_wdt_device structure to hold device
> data.
>
> Signed-off-by: Ivan Khoronzhuk <ivan.khoronzhuk@ti.com>

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


^ permalink raw reply	[flat|nested] 14+ messages in thread
* Re: Fwd: [PATCH 2/8] watchdog: davinci: use davinci_wdt_device structure to hold device data
@ 2013-11-17  2:19     ` Guenter Roeck
  0 siblings, 0 replies; 14+ messages in thread
From: Guenter Roeck @ 2013-11-17  2:19 UTC (permalink / raw)
  To: ivan.khoronzhuk, Santosh Shilimkar, wim-IQzOog9fTRqzQB+pC5nmwQ,
	nsekhar-l0cyMroinI0, linux-watchdog-u79uwXL29TY76Z2rM5mHXA,
	devicetree-u79uwXL29TY76Z2rM5mHXA
  Cc: grant.likely-QSEj5FYQhm4dnm+yROfE0A,
	rob.herring-bsGFqQB8/DxBDgjK7y7TUQ, pawel.moll-5wv7dgnIgG8,
	mark.rutland-5wv7dgnIgG8, swarren-3lzwWm7+Weoh9ZMKESR00Q,
	galak-XVmvHMARGAS8U2dJNN8I7kB+6BGkLq7r,
	ijc+devicetree-KcIKpvwj1kUDXYZnReoRVg,
	linux-kernel-u79uwXL29TY76Z2rM5mHXA,
	linux-arm-kernel-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r

On 11/06/2013 03:31 AM, ivan.khoronzhuk wrote:
> Some SoCs, like Keystone 2, can support more than one WDT and each
> watchdog device has to use it's own base address, clock source,
> wdd device, so add new davinci_wdt_device structure to hold device
> data.
>
> Signed-off-by: Ivan Khoronzhuk <ivan.khoronzhuk-l0cyMroinI0@public.gmane.org>

Reviewed-by: Guenter roeck <linux-0h96xk9xTtrk1uMJSBkQmQ@public.gmane.org>

--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

^ permalink raw reply	[flat|nested] 14+ messages in thread
* Fwd: [PATCH 2/8] watchdog: davinci: use davinci_wdt_device structure to hold device data
@ 2013-11-17  2:19     ` Guenter Roeck
  0 siblings, 0 replies; 14+ messages in thread
From: Guenter Roeck @ 2013-11-17  2:19 UTC (permalink / raw)
  To: linux-arm-kernel

On 11/06/2013 03:31 AM, ivan.khoronzhuk wrote:
> Some SoCs, like Keystone 2, can support more than one WDT and each
> watchdog device has to use it's own base address, clock source,
> wdd device, so add new davinci_wdt_device structure to hold device
> data.
>
> Signed-off-by: Ivan Khoronzhuk <ivan.khoronzhuk@ti.com>

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

^ permalink raw reply	[flat|nested] 14+ messages in thread
end of thread, other threads:[~2013-11-17  2:19 UTC | newest]

Thread overview: 14+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
     [not found] <1383680783-12114-3-git-send-email-ivan.khoronzhuk@ti.com>
2013-11-06 11:31 ` Fwd: [PATCH 2/8] watchdog: davinci: use davinci_wdt_device structure to hold device data ivan.khoronzhuk
2013-11-06 11:31   ` ivan.khoronzhuk
2013-11-06 11:31   ` ivan.khoronzhuk
2013-11-12 15:37   ` Santosh Shilimkar
2013-11-12 15:37     ` Santosh Shilimkar
2013-11-12 15:37     ` Santosh Shilimkar
2013-11-12 16:27     ` Guenter Roeck
2013-11-12 16:27       ` Guenter Roeck
2013-11-12 16:28       ` Santosh Shilimkar
2013-11-12 16:28         ` Santosh Shilimkar
2013-11-12 16:28         ` Santosh Shilimkar
2013-11-17  2:19   ` Guenter Roeck
2013-11-17  2:19     ` Guenter Roeck
2013-11-17  2:19     ` Guenter Roeck

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.