Skip to contentSkip to conversation

wdt: Fix spelling Resettting -> Resetting

PATCH New Andy Shevchenko 5 participants 16 messages Jul 5, 2017 – Jul 7, 2017 Watchdog device drivers

lore
Share & tools
lore
#0 lore raw

PATCH 1/3 wdt: Fix spelling Resettting -> Resetting

Fix spelling Resettting -> Resetting.
No functional change intended.

---
 drivers/watchdog/wdt-uclass.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)
drivers/watchdog/wdt-uclass.c +1 -1
index ab8a64c354..bb9ae80866 100644
@@ -48,7 +48,7 @@ int wdt_expire_now(struct udevice *dev, ulong flags)
 	int ret = 0;
 	const struct wdt_ops *ops;
 
-	debug("WDT Resettting: %lu\n", flags);
+	debug("WDT Resetting: %lu\n", flags);
 	ops = device_get_ops(dev);
 	if (ops->expire_now) {
 		return ops->expire_now(dev, flags);
Signature · 1 line
2.11.0
#1 lore raw

PATCH 2/3 wdt: Timeout better to be in microseconds

Timeout in some abstract ticks is not what we are applying to get deterministic behaviour.

Convert name to show explicitly that we are using microseconds (for watchdog it's more than precise).

No functional change intended.

---
 drivers/watchdog/wdt-uclass.c | 4 ++--
 include/wdt.h                 | 8 ++++----
 2 files changed, 6 insertions(+), 6 deletions(-)
drivers/watchdog/wdt-uclass.c +2 -2
index bb9ae80866..1715a98452 100644
@@ -13,14 +13,14 @@
 
 DECLARE_GLOBAL_DATA_PTR;
 
-int wdt_start(struct udevice *dev, u64 timeout, ulong flags)
+int wdt_start(struct udevice *dev, u64 timeout_us, ulong flags)
 {
 	const struct wdt_ops *ops = device_get_ops(dev);
 
 	if (!ops->start)
 		return -ENOSYS;
 
-	return ops->start(dev, timeout, flags);
+	return ops->start(dev, timeout_us, flags);
 }
 
 int wdt_stop(struct udevice *dev)
include/wdt.h +4 -4
index 0b5f05851a..115e8c6baf 100644
@@ -21,12 +21,12 @@
  * Start the timer
  *
  * @dev: WDT Device
- * @timeout: Number of ticks before timer expires
+ * @timeout_us: Number of microseconds before timer expires
  * @flags: Driver specific flags. This might be used to specify
  * which action needs to be executed when the timer expires
  * @return: 0 if OK, -ve on error
  */
-int wdt_start(struct udevice *dev, u64 timeout, ulong flags);
+int wdt_start(struct udevice *dev, u64 timeout_us, ulong flags);
 
 /*
  * Stop the timer, thus disabling the Watchdog. Use wdt_start to start it again.
@@ -67,12 +67,12 @@ struct wdt_ops {
 	 * Start the timer
 	 *
 	 * @dev: WDT Device
-	 * @timeout: Number of ticks before the timer expires
+	 * @timeout_us: Number of microseconds before the timer expires
 	 * @flags: Driver specific flags. This might be used to specify
 	 * which action needs to be executed when the timer expires
 	 * @return: 0 if OK, -ve on error
 	 */
-	int (*start)(struct udevice *dev, u64 timeout, ulong flags);
+	int (*start)(struct udevice *dev, u64 timeout_us, ulong flags);
 	/*
 	 * Stop the timer
 	 *
Signature · 1 line
2.11.0
#2 lore raw

PATCH 2/3 wdt: Timeout better to be in microseconds

Quoted 6 lines · Andy Shevchenko

Timeout in some abstract ticks is not what we are applying to get deterministic behaviour.

Convert name to show explicitly that we are using microseconds (for watchdog it's more than precise).

Quoted 8 lines

No functional change intended.

---
 drivers/watchdog/wdt-uclass.c | 4 ++--
 include/wdt.h                 | 8 ++++----
 2 files changed, 6 insertions(+), 6 deletions(-)
#3 lore raw

PATCH 2/3 wdt: Timeout better to be in microseconds

In reply to Andy Shevchenko

Hello Andy,

Am 05.07.2017 um 19:44 schrieb Andy Shevchenko:

Quoted 6 lines

Timeout in some abstract ticks is not what we are applying to get deterministic behaviour.

Convert name to show explicitly that we are using microseconds (for watchdog it's more than precise).

Quoted 7 lines

No functional change intended.

---
  drivers/watchdog/wdt-uclass.c | 4 ++--
  include/wdt.h                 | 8 ++++----
  2 files changed, 6 insertions(+), 6 deletions(-)

bye,
Heiko

Quoted 57 lines
drivers/watchdog/wdt-uclass.c +2 -2
index bb9ae80866..1715a98452 100644
@@ -13,14 +13,14 @@
 
  DECLARE_GLOBAL_DATA_PTR;
 
-int wdt_start(struct udevice *dev, u64 timeout, ulong flags)
+int wdt_start(struct udevice *dev, u64 timeout_us, ulong flags)
  {
  	const struct wdt_ops *ops = device_get_ops(dev);
 
  	if (!ops->start)
  		return -ENOSYS;
 
-	return ops->start(dev, timeout, flags);
+	return ops->start(dev, timeout_us, flags);
  }
 
  int wdt_stop(struct udevice *dev)
include/wdt.h +4 -4
index 0b5f05851a..115e8c6baf 100644
@@ -21,12 +21,12 @@
   * Start the timer
   *
   * @dev: WDT Device
- * @timeout: Number of ticks before timer expires
+ * @timeout_us: Number of microseconds before timer expires
   * @flags: Driver specific flags. This might be used to specify
   * which action needs to be executed when the timer expires
   * @return: 0 if OK, -ve on error
   */
-int wdt_start(struct udevice *dev, u64 timeout, ulong flags);
+int wdt_start(struct udevice *dev, u64 timeout_us, ulong flags);
 
  /*
   * Stop the timer, thus disabling the Watchdog. Use wdt_start to start it again.
@@ -67,12 +67,12 @@ struct wdt_ops {
  	 * Start the timer
  	 *
  	 * @dev: WDT Device
-	 * @timeout: Number of ticks before the timer expires
+	 * @timeout_us: Number of microseconds before the timer expires
  	 * @flags: Driver specific flags. This might be used to specify
  	 * which action needs to be executed when the timer expires
  	 * @return: 0 if OK, -ve on error
  	 */
-	int (*start)(struct udevice *dev, u64 timeout, ulong flags);
+	int (*start)(struct udevice *dev, u64 timeout_us, ulong flags);
  	/*
  	 * Stop the timer
  	 *
Signature · 2 lines
DENX Software Engineering GmbH,      Managing Director: Wolfgang Denk
HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany
#4 lore raw

PATCH 2/3 wdt: Timeout better to be in microseconds

In reply to Andy Shevchenko

Hi Andy,

Quoted 2 lines · Andy Shevchenko

Timeout in some abstract ticks is not what we are applying to get deterministic behaviour.

A tick is always milliseconds in U-Boot, as I understand it.

Quoted 3 lines

Convert name to show explicitly that we are using microseconds (for watchdog it's more than precise).

Do you want microseconds, or is milliseconds enough accuracy? I have a hard time imagining a case where a microsecond watchdog timeout is useful.

Quoted 9 lines

No functional change intended.

---
 drivers/watchdog/wdt-uclass.c | 4 ++--
 include/wdt.h                 | 8 ++++----
 2 files changed, 6 insertions(+), 6 deletions(-)

Regards,
Simon

#5 lore raw

PATCH 2/3 wdt: Timeout better to be in microseconds

Quoted 8 lines · Simon Glass

Hi Andy,

Quoted 2 lines · Andy Shevchenko

Timeout in some abstract ticks is not what we are applying to get deterministic behaviour.

A tick is always milliseconds in U-Boot, as I understand it.

You see, there is a confusion.
I would like to see units somewhere there, to make it clear.

Quoted 8 lines
Quoted 4 lines

Convert name to show explicitly that we are using microseconds (for watchdog it's more than precise).

Do you want microseconds, or is milliseconds enough accuracy? I have a hard time imagining a case where a microsecond watchdog timeout is useful.

For me ticks sounded like processor cycles (nanosecond-ish). So, milliseconds are better.

Since I have not much time, feel free to drop, modify, etc. Thanks for review.

Quoted 13 lines
Quoted 10 lines

No functional change intended.

---
 drivers/watchdog/wdt-uclass.c | 4 ++--
 include/wdt.h                 | 8 ++++----

 2 files changed, 6 insertions(+), 6 deletions(-)

Regards,
Simon

Signature · 2 lines
Andy Shevchenko <[email protected]>
Intel Finland Oy
#6 lore raw

PATCH 3/3 wdt: Unify option of timeout value

In reply to Andy Shevchenko

There is no need to duplicate same option with different name.

Kill HW_WATCHDOG_TIMEOUT_MS in favor of WATCHDOG_TIMEOUT_MSECS.

---
 drivers/watchdog/designware_wdt.c | 2 +-
 include/configs/socfpga_common.h  | 2 +-
 scripts/config_whitelist.txt      | 1 -
 3 files changed, 2 insertions(+), 3 deletions(-)
drivers/watchdog/designware_wdt.c +1 -1
index e788e1b65d..8ec094d994 100644
@@ -65,7 +65,7 @@ void hw_watchdog_init(void)
 	/* reset to disable the watchdog */
 	hw_watchdog_reset();
 	/* set timer in miliseconds */
-	designware_wdt_settimeout(CONFIG_HW_WATCHDOG_TIMEOUT_MS);
+	designware_wdt_settimeout(CONFIG_WATCHDOG_TIMEOUT_MSECS);
 	/* enable the watchdog */
 	designware_wdt_enable();
 	/* reset the watchdog */
include/configs/socfpga_common.h +1 -1
index fd18ae5f5d..c17814bf01 100644
@@ -131,7 +131,7 @@
 #define CONFIG_DESIGNWARE_WATCHDOG
 #define CONFIG_DW_WDT_BASE		SOCFPGA_L4WD0_ADDRESS
 #define CONFIG_DW_WDT_CLOCK_KHZ		25000
-#define CONFIG_HW_WATCHDOG_TIMEOUT_MS	30000
+#define CONFIG_WATCHDOG_TIMEOUT_MSECS	30000
 #endif
 
 /*
scripts/config_whitelist.txt +0 -1
index 95f27b6ead..73701bcaf8 100644
@@ -1121,7 +1121,6 @@ CONFIG_HUSH_INIT_VAR
 CONFIG_HVBOOT
 CONFIG_HWCONFIG
 CONFIG_HW_ENV_SETTINGS
-CONFIG_HW_WATCHDOG_TIMEOUT_MS
 CONFIG_I2C
 CONFIG_I2C_CHIPADDRESS
 CONFIG_I2C_CMD_TREE
Signature · 1 line
2.11.0
#7 lore raw

PATCH 3/3 wdt: Unify option of timeout value

Quoted 4 lines · Andy Shevchenko

There is no need to duplicate same option with different name.

Kill HW_WATCHDOG_TIMEOUT_MS in favor of WATCHDOG_TIMEOUT_MSECS.

Yes, I know about WATCHDOG_TIMEOUT, I spent too much time already on this crap. Feel free to make a follow up to move from WATCHDOG_TIMEOUT to WATCHDOG_TIMEOUT_MSECS.

Quoted 48 lines

---
 drivers/watchdog/designware_wdt.c | 2 +-
 include/configs/socfpga_common.h  | 2 +-
 scripts/config_whitelist.txt      | 1 -

 3 files changed, 2 insertions(+), 3 deletions(-)
drivers/watchdog/designware_wdt.c +1 -1
index e788e1b65d..8ec094d994 100644
@@ -65,7 +65,7 @@ void hw_watchdog_init(void)
  	/* reset to disable the watchdog */
  	hw_watchdog_reset();
  	/* set timer in miliseconds */
-	designware_wdt_settimeout(CONFIG_HW_WATCHDOG_TIMEOUT_MS);
+	designware_wdt_settimeout(CONFIG_WATCHDOG_TIMEOUT_MSECS);
  	/* enable the watchdog */
  	designware_wdt_enable();
  	/* reset the watchdog */
include/configs/socfpga_common.h +1 -1
index fd18ae5f5d..c17814bf01 100644
@@ -131,7 +131,7 @@
  #define CONFIG_DESIGNWARE_WATCHDOG
  #define CONFIG_DW_WDT_BASE		SOCFPGA_L4WD0_ADDRESS
  #define CONFIG_DW_WDT_CLOCK_KHZ		25000
-#define CONFIG_HW_WATCHDOG_TIMEOUT_MS	30000
+#define CONFIG_WATCHDOG_TIMEOUT_MSECS	30000
  #endif


 /*

scripts/config_whitelist.txt +0 -0
index 95f27b6ead..73701bcaf8 100644
@@ -1121,7 +1121,6 @@ CONFIG_HUSH_INIT_VAR

 CONFIG_HVBOOT
 CONFIG_HWCONFIG
 CONFIG_HW_ENV_SETTINGS
-CONFIG_HW_WATCHDOG_TIMEOUT_MS
 CONFIG_I2C
 CONFIG_I2C_CHIPADDRESS
 CONFIG_I2C_CMD_TREE

Signature · 2 lines
Andy Shevchenko <[email protected]>
Intel Finland Oy
#8 lore raw

PATCH 3/3 wdt: Unify option of timeout value

In reply to Andy Shevchenko
Quoted 11 lines · Andy Shevchenko

There is no need to duplicate same option with different name.

Kill HW_WATCHDOG_TIMEOUT_MS in favor of WATCHDOG_TIMEOUT_MSECS.

---
 drivers/watchdog/designware_wdt.c | 2 +-
 include/configs/socfpga_common.h  | 2 +-
 scripts/config_whitelist.txt      | 1 -
 3 files changed, 2 insertions(+), 3 deletions(-)
#9 lore raw

PATCH 3/3 wdt: Unify option of timeout value

In reply to Andy Shevchenko

Hello Andy,

Am 05.07.2017 um 19:44 schrieb Andy Shevchenko:

Quoted 10 lines

There is no need to duplicate same option with different name.

Kill HW_WATCHDOG_TIMEOUT_MS in favor of WATCHDOG_TIMEOUT_MSECS.

---
  drivers/watchdog/designware_wdt.c | 2 +-
  include/configs/socfpga_common.h  | 2 +-
  scripts/config_whitelist.txt      | 1 -
  3 files changed, 2 insertions(+), 3 deletions(-)

Good catch!

May I ask, if you find time to move CONFIG_WATCHDOG_TIMEOUT_MSECS to Kconfig?

bye,
Heiko

Quoted 40 lines
drivers/watchdog/designware_wdt.c +1 -1
index e788e1b65d..8ec094d994 100644
@@ -65,7 +65,7 @@ void hw_watchdog_init(void)
  	/* reset to disable the watchdog */
  	hw_watchdog_reset();
  	/* set timer in miliseconds */
-	designware_wdt_settimeout(CONFIG_HW_WATCHDOG_TIMEOUT_MS);
+	designware_wdt_settimeout(CONFIG_WATCHDOG_TIMEOUT_MSECS);
  	/* enable the watchdog */
  	designware_wdt_enable();
  	/* reset the watchdog */
include/configs/socfpga_common.h +1 -1
index fd18ae5f5d..c17814bf01 100644
@@ -131,7 +131,7 @@
  #define CONFIG_DESIGNWARE_WATCHDOG
  #define CONFIG_DW_WDT_BASE		SOCFPGA_L4WD0_ADDRESS
  #define CONFIG_DW_WDT_CLOCK_KHZ		25000
-#define CONFIG_HW_WATCHDOG_TIMEOUT_MS	30000
+#define CONFIG_WATCHDOG_TIMEOUT_MSECS	30000
  #endif
 
  /*
scripts/config_whitelist.txt +0 -1
index 95f27b6ead..73701bcaf8 100644
@@ -1121,7 +1121,6 @@ CONFIG_HUSH_INIT_VAR
  CONFIG_HVBOOT
  CONFIG_HWCONFIG
  CONFIG_HW_ENV_SETTINGS
-CONFIG_HW_WATCHDOG_TIMEOUT_MS
  CONFIG_I2C
  CONFIG_I2C_CHIPADDRESS
  CONFIG_I2C_CMD_TREE
Signature · 2 lines
DENX Software Engineering GmbH,      Managing Director: Wolfgang Denk
HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany
#10 lore raw

PATCH 3/3 wdt: Unify option of timeout value

In reply to Andy Shevchenko
Quoted 10 lines · Andy Shevchenko

There is no need to duplicate same option with different name.

Kill HW_WATCHDOG_TIMEOUT_MS in favor of WATCHDOG_TIMEOUT_MSECS.

---
 drivers/watchdog/designware_wdt.c | 2 +-
 include/configs/socfpga_common.h  | 2 +-
 scripts/config_whitelist.txt      | 1 -
 3 files changed, 2 insertions(+), 3 deletions(-)
#11 lore raw

PATCH 3/3 wdt: Unify option of timeout value

In reply to Andy Shevchenko
Quoted 7 lines · Andy Shevchenko

There is no need to duplicate same option with different name.

Kill HW_WATCHDOG_TIMEOUT_MS in favor of WATCHDOG_TIMEOUT_MSECS.

Applied to u-boot/master, thanks!

Signature · 8 lines
Tom
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 819 bytes
Desc: Digital signature
URL: <http://lists.denx.de/pipermail/u-boot/attachments/20170707/9eb6350e/attachment.sig>
#13 lore raw

PATCH 1/3 wdt: Fix spelling Resettting -> Resetting

In reply to Andy Shevchenko

Hello Andy,

Am 05.07.2017 um 19:44 schrieb Andy Shevchenko:

Quoted 7 lines

Fix spelling Resettting -> Resetting.
No functional change intended.

---
  drivers/watchdog/wdt-uclass.c | 2 +-
  1 file changed, 1 insertion(+), 1 deletion(-)

Thanks!

bye,
Heiko

Quoted 15 lines
drivers/watchdog/wdt-uclass.c +1 -1
index ab8a64c354..bb9ae80866 100644
@@ -48,7 +48,7 @@ int wdt_expire_now(struct udevice *dev, ulong flags)
  	int ret = 0;
  	const struct wdt_ops *ops;
 
-	debug("WDT Resettting: %lu\n", flags);
+	debug("WDT Resetting: %lu\n", flags);
  	ops = device_get_ops(dev);
  	if (ops->expire_now) {
  		return ops->expire_now(dev, flags);
Signature · 2 lines
DENX Software Engineering GmbH,      Managing Director: Wolfgang Denk
HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany
#15 lore raw

PATCH 1/3 wdt: Fix spelling Resettting -> Resetting

In reply to Andy Shevchenko
Quoted 6 lines · Andy Shevchenko

Fix spelling Resettting -> Resetting.
No functional change intended.

Applied to u-boot/master, thanks!

Signature · 8 lines
Tom
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 819 bytes
Desc: Digital signature
URL: <http://lists.denx.de/pipermail/u-boot/attachments/20170707/9d3f56d3/attachment.sig>