[PATCH 0/2] watchdog: imgpdc_wdt assorted fixes

[PATCH 0/2] watchdog: imgpdc_wdt assorted fixes

[Ezequiel Garcia]

Hi,

A couple more fixes for IMG watchdog driver.

This first patch corrects the max_timeout setting which was wrongly
set in cycles.

The second patch documents the driver's timeout round-up, which
might seem a bit unintuitive. See the patch for more details.

Both patches are based on the three imgpdc patches sent recently by
Andrew Bresticker [1].

[1] http://www.spinics.net/lists/linux-watchdog/msg06107.html

Thanks!

Ezequiel Garcia (2):
  watchdog: imgpdc: Fix max timeout
  watchdog: imgpdc: Add some documentation about the timeout

 drivers/watchdog/imgpdc_wdt.c | 35 ++++++++++++++++++++++++++++++++++-
 1 file changed, 34 insertions(+), 1 deletion(-)

-- 
2.3.3

[PATCH 1/2] watchdog: imgpdc: Fix max timeout

[Ezequiel Garcia]

Maximum timeout is currently set in clock cycles, but the watchdog
core expects it to be in seconds. Fix it.

Signed-off-by: Ezequiel Garcia <ezequiel.garcia@imgtec.com>
---
 drivers/watchdog/imgpdc_wdt.c | 6 +++++-
 1 file changed, 5 insertions(+), 1 deletion(-)

diff --git a/drivers/watchdog/imgpdc_wdt.c b/drivers/watchdog/imgpdc_wdt.c
index 28c10e2..56b8ebc 100644
--- a/drivers/watchdog/imgpdc_wdt.c
+++ b/drivers/watchdog/imgpdc_wdt.c
@@ -152,6 +152,7 @@ static int pdc_wdt_restart(struct notifier_block *this, unsigned long mode,
 
 static int pdc_wdt_probe(struct platform_device *pdev)
 {
+	u64 div;
 	int ret, val;
 	unsigned long clk_rate;
 	struct resource *res;
@@ -211,7 +212,10 @@ static int pdc_wdt_probe(struct platform_device *pdev)
 
 	pdc_wdt->wdt_dev.info = &pdc_wdt_info;
 	pdc_wdt->wdt_dev.ops = &pdc_wdt_ops;
-	pdc_wdt->wdt_dev.max_timeout = 1 << PDC_WDT_CONFIG_DELAY_MASK;
+
+	div = 1ULL << (PDC_WDT_CONFIG_DELAY_MASK + 1);
+	do_div(div, clk_rate);
+	pdc_wdt->wdt_dev.max_timeout = div;
 	pdc_wdt->wdt_dev.timeout = PDC_WDT_DEF_TIMEOUT;
 	pdc_wdt->wdt_dev.parent = &pdev->dev;
 	watchdog_set_drvdata(&pdc_wdt->wdt_dev, pdc_wdt);
-- 
2.3.3

Re: [PATCH 1/2] watchdog: imgpdc: Fix max timeout

[Guenter Roeck]

On 05/11/2015 10:41 AM, Ezequiel Garcia wrote:
> Maximum timeout is currently set in clock cycles, but the watchdog
> core expects it to be in seconds. Fix it.
>
> Signed-off-by: Ezequiel Garcia <ezequiel.garcia@imgtec.com>

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

RE: [PATCH 1/2] watchdog: imgpdc: Fix max timeout

[Naidu Tellapati]

Hi Guenter,

> On 05/11/2015 10:41 AM, Ezequiel Garcia wrote:
>> Maximum timeout is currently set in clock cycles, but the watchdog
>> core expects it to be in seconds. Fix it.
>>
>> Signed-off-by: Ezequiel Garcia <ezequiel.garcia@imgtec.com>

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

Many thanks for the review.

Regards,
Naidu.

[PATCH 2/2] watchdog: imgpdc: Add some documentation about the timeout

[Ezequiel Garcia]

This watchdog hardware can be configured in terms of power-of-two
clock cycles. Therefore, the watchdog timeout configured by the user
will be rounded-up to the next possible hardware timeout.

This commit adds a comment explaining this.

Signed-off-by: Ezequiel Garcia <ezequiel.garcia@imgtec.com>
---
 drivers/watchdog/imgpdc_wdt.c | 29 +++++++++++++++++++++++++++++
 1 file changed, 29 insertions(+)

diff --git a/drivers/watchdog/imgpdc_wdt.c b/drivers/watchdog/imgpdc_wdt.c
index 56b8ebc..0f73621 100644
--- a/drivers/watchdog/imgpdc_wdt.c
+++ b/drivers/watchdog/imgpdc_wdt.c
@@ -9,6 +9,35 @@
  *
  * Based on drivers/watchdog/sunxi_wdt.c Copyright (c) 2013 Carlo Caione
  *                                                     2012 Henrik Nordstrom
+ *
+ * Notes
+ * -----
+ * The timeout value is rounded to the next power of two clock cycles.
+ * This is configured using the PDC_WDT_CONFIG register, according to this
+ * formula:
+ *
+ *     timeout = 2^(delay + 1) clock cycles
+ *
+ * Where 'delay' is the value written in PDC_WDT_CONFIG register.
+ *
+ * Therefore, the hardware only allows to program watchdog timeouts, expressed
+ * as a power of two number of watchdog clock cycles. The current implementation
+ * guarantees that the actual watchdog timeout will be _at least_ the value
+ * programmed in the imgpdg_wdt driver.
+ *
+ * The following table shows how the user-configured timeout relates
+ * to the actual hardware timeout (watchdog clock @ 40000 Hz):
+ *
+ * input timeout | WD_DELAY | actual timeout
+ * -----------------------------------
+ *      10       |   18     |  13 seconds
+ *      20       |   19     |  26 seconds
+ *      30       |   20     |  52 seconds
+ *      60       |   21     |  104 seconds
+ *
+ * Albeit coarse, this granularity would suffice most watchdog uses.
+ * If the platform allows it, the user should be able to change the watchdog
+ * clock rate and achieve a finer timeout granularity.
  */
 
 #include <linux/clk.h>
-- 
2.3.3

Re: [PATCH 2/2] watchdog: imgpdc: Add some documentation about the timeout

[Guenter Roeck]

On 05/11/2015 10:41 AM, Ezequiel Garcia wrote:
> This watchdog hardware can be configured in terms of power-of-two
> clock cycles. Therefore, the watchdog timeout configured by the user
> will be rounded-up to the next possible hardware timeout.
>
> This commit adds a comment explaining this.
>
> Signed-off-by: Ezequiel Garcia <ezequiel.garcia@imgtec.com>

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

RE: [PATCH 2/2] watchdog: imgpdc: Add some documentation about the timeout

[Naidu Tellapati]

Hi Guenter,

> On 05/11/2015 10:41 AM, Ezequiel Garcia wrote:
>> This watchdog hardware can be configured in terms of power-of-two
>> clock cycles. Therefore, the watchdog timeout configured by the user
>> will be rounded-up to the next possible hardware timeout.
>>
>> This commit adds a comment explaining this.
>>
>> Signed-off-by: Ezequiel Garcia <ezequiel.garcia@imgtec.com>

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

Many thanks for the review.

Regards,
Naidu.

Re: [PATCH 0/2] watchdog: imgpdc_wdt assorted fixes

[Ezequiel Garcia]

Wim,

Do you plan to take this for v4.2?

Thanks!

On 05/11/2015 02:41 PM, Ezequiel Garcia wrote:
> Hi,
> 
> A couple more fixes for IMG watchdog driver.
> 
> This first patch corrects the max_timeout setting which was wrongly
> set in cycles.
> 
> The second patch documents the driver's timeout round-up, which
> might seem a bit unintuitive. See the patch for more details.
> 
> Both patches are based on the three imgpdc patches sent recently by
> Andrew Bresticker [1].
> 
> [1] http://www.spinics.net/lists/linux-watchdog/msg06107.html
> 
> Thanks!
> 
> Ezequiel Garcia (2):
>   watchdog: imgpdc: Fix max timeout
>   watchdog: imgpdc: Add some documentation about the timeout
> 

-- 
Ezequiel

Re: [PATCH 0/2] watchdog: imgpdc_wdt assorted fixes

[Ezequiel Garcia]

On 05/21/2015 10:57 AM, Ezequiel Garcia wrote:
> Wim,
> 
> Do you plan to take this for v4.2?
> 

Ping?

-- 
Ezequiel