Skip to contentSkip to conversation

watchdog: s3c2410: Fixes and improvements

PATCH In review Krzysztof Kozlowski 3 participants 29 messages Mar 11, 2017 – Mar 14, 2017

Subsystems (3)
lore
Share & tools
lore

Krzysztof Kozlowski

author maintainer
#0 lore raw

v2 cover watchdog: s3c2410: Fixes and improvements

Hi,

Minor cleanup and some fixes for the watchdog used on all Samsung SoCs.

The DTS patches should through samsung-soc tree but they depend on introducing new compatible. The patches waiting for new compatible will be deferred to next release.

Changes since v1:
=================
1. Add Guenter's review and Rob's ack.
2. Change compatible in patch 6/8 instead 3/8.

Best regards,
Krzysztof

Krzysztof Kozlowski (8):
watchdog: s3c2410: Remove confusing CONFIG prefix from local defines
watchdog: s3c2410: Fix infinite interrupt in soft mode
ARM: dts: exynos: Fix watchdog reset on Exynos4412
ARM: dts: s3c64xx: Enable watchdog on all S3C64xx boards
ARM: dts: exynos: Enable watchdog on all Exynos4 boards
ARM: dts: exynos: Fix infinite interrupt in soft mode on Exynos4210

    and Exynos5440

ARM: dts: s3c64xx: Fix infinite interrupt in soft mode
ARM: dts: s5pv210: Fix infinite interrupt in soft mode

 .../devicetree/bindings/watchdog/samsung-wdt.txt   |  9 +++---
 arch/arm/boot/dts/exynos4.dtsi                     |  9 ------
 arch/arm/boot/dts/exynos4210-origen.dts            |  4 ---
 arch/arm/boot/dts/exynos4210.dtsi                  |  8 +++++
 arch/arm/boot/dts/exynos4412-itop-scp-core.dtsi    |  4 ---
 arch/arm/boot/dts/exynos4412-odroid-common.dtsi    |  4 ---
 arch/arm/boot/dts/exynos4412-origen.dts            |  4 ---
 arch/arm/boot/dts/exynos4412.dtsi                  |  9 ++++++
 arch/arm/boot/dts/exynos5440.dtsi                  |  2 +-
 arch/arm/boot/dts/s3c64xx.dtsi                     |  3 +-
 arch/arm/boot/dts/s5pv210.dtsi                     |  2 +-
 drivers/watchdog/s3c2410_wdt.c                     | 37 +++++++++++++++-------
 12 files changed, 51 insertions(+), 44 deletions(-)
Signature · 1 line
2.9.3
author maintainer
#1 lore raw

PATCH v2 1/8 watchdog: s3c2410: Remove confusing CONFIG prefix from local defines

The CONFIG prefix from defines in the s3c2410_wdt.c might suggest that these constants come from Kconfig.

---
 drivers/watchdog/s3c2410_wdt.c | 16 ++++++++--------
 1 file changed, 8 insertions(+), 8 deletions(-)
drivers/watchdog/s3c2410_wdt.c +8 -8
index 59e95762a6de..3e216f54cb6f 100644
@@ -64,8 +64,8 @@
 #define S3C2410_WTCON_PRESCALE_MASK	(0xff << 8)
 #define S3C2410_WTCON_PRESCALE_MAX	0xff
 
-#define CONFIG_S3C2410_WATCHDOG_ATBOOT		(0)
-#define CONFIG_S3C2410_WATCHDOG_DEFAULT_TIME	(15)
+#define S3C2410_WATCHDOG_ATBOOT		(0)
+#define S3C2410_WATCHDOG_DEFAULT_TIME	(15)
 
 #define EXYNOS5_RST_STAT_REG_OFFSET		0x0404
 #define EXYNOS5_WDT_DISABLE_REG_OFFSET		0x0408
@@ -79,7 +79,7 @@
 
 static bool nowayout	= WATCHDOG_NOWAYOUT;
 static int tmr_margin;
-static int tmr_atboot	= CONFIG_S3C2410_WATCHDOG_ATBOOT;
+static int tmr_atboot	= S3C2410_WATCHDOG_ATBOOT;
 static int soft_noboot;
 static int debug;
 
@@ -90,10 +90,10 @@ module_param(soft_noboot, int, 0);
 module_param(debug,	  int, 0);
 
 MODULE_PARM_DESC(tmr_margin, "Watchdog tmr_margin in seconds. (default="
-		__MODULE_STRING(CONFIG_S3C2410_WATCHDOG_DEFAULT_TIME) ")");
+		__MODULE_STRING(S3C2410_WATCHDOG_DEFAULT_TIME) ")");
 MODULE_PARM_DESC(tmr_atboot,
 		"Watchdog is started@boot time if set to 1, default="
-			__MODULE_STRING(CONFIG_S3C2410_WATCHDOG_ATBOOT));
+			__MODULE_STRING(S3C2410_WATCHDOG_ATBOOT));
 MODULE_PARM_DESC(nowayout, "Watchdog cannot be stopped once started (default="
 			__MODULE_STRING(WATCHDOG_NOWAYOUT) ")");
 MODULE_PARM_DESC(soft_noboot, "Watchdog action, set to 1 to ignore reboots, "
@@ -406,7 +406,7 @@ static struct watchdog_ops s3c2410wdt_ops = {
 static struct watchdog_device s3c2410_wdd = {
 	.info = &s3c2410_wdt_ident,
 	.ops = &s3c2410wdt_ops,
-	.timeout = CONFIG_S3C2410_WATCHDOG_DEFAULT_TIME,
+	.timeout = S3C2410_WATCHDOG_DEFAULT_TIME,
 };
 
 /* interrupt handler code */
@@ -600,12 +600,12 @@ static int s3c2410wdt_probe(struct platform_device *pdev)
 					wdt->wdt_device.timeout);
 	if (ret) {
 		started = s3c2410wdt_set_heartbeat(&wdt->wdt_device,
-					CONFIG_S3C2410_WATCHDOG_DEFAULT_TIME);
+					S3C2410_WATCHDOG_DEFAULT_TIME);
 
 		if (started == 0)
 			dev_info(dev,
 			   "tmr_margin value out of range, default %d used\n",
-			       CONFIG_S3C2410_WATCHDOG_DEFAULT_TIME);
+			       S3C2410_WATCHDOG_DEFAULT_TIME);
 		else
 			dev_info(dev, "default timer value is out of range, "
 							"cannot start\n");
Signature · 1 line
2.9.3
author maintainer
#2 lore raw

Re: [PATCH v2 1/8] watchdog: s3c2410: Remove confusing CONFIG prefix from local defines

Quoted 9 lines · Krzysztof Kozlowski

The CONFIG prefix from defines in the s3c2410_wdt.c might suggest that these constants come from Kconfig.

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

Ah, my mistake, I looked at watchdog-master but this is already in watchdog-next. Sorry for confusion.

Best regards,
Krzysztof

author maintainer
#3 lore raw

PATCH v2 2/8 watchdog: s3c2410: Fix infinite interrupt in soft mode

In reply to Krzysztof Kozlowski

In soft (no-reboot) mode, the driver self-pings watchdog upon expiration of an interrupt. However the interrupt itself was not cleared thus on first hit, the system enters infinite interrupt handling loop.

On Odroid U3 (Exynos4412), when booted with s3c2410_wdt.soft_noboot=1 argument the console is flooded:

	# killall -9 watchdog
	[   60.523760] s3c2410-wdt 10060000.watchdog: watchdog timer expired (irq)
	[   60.536744] s3c2410-wdt 10060000.watchdog: watchdog timer expired (irq)

Fix this by writing something to the WTCLRINT register to clear the interrupt. The register WTCLRINT however appeared in S3C6410 so a new watchdog quirk and flavor are needed.

---
 .../devicetree/bindings/watchdog/samsung-wdt.txt    |  9 +++++----
 drivers/watchdog/s3c2410_wdt.c                      | 21 ++++++++++++++++++---
 2 files changed, 23 insertions(+), 7 deletions(-)
Documentation/devicetree/bindings/watchdog/samsung-wdt.txt +5 -4
index 8f3d96af81d7..1f6e101e299a 100644
@@ -6,10 +6,11 @@ occurred.
 
 Required properties:
 - compatible : should be one among the following
-	(a) "samsung,s3c2410-wdt" for Exynos4 and previous SoCs
-	(b) "samsung,exynos5250-wdt" for Exynos5250
-	(c) "samsung,exynos5420-wdt" for Exynos5420
-	(c) "samsung,exynos7-wdt" for Exynos7
+	- "samsung,s3c2410-wdt" for S3C2410
+	- "samsung,s3c6410-wdt" for S3C6410, S5PV210 and Exynos4
+	- "samsung,exynos5250-wdt" for Exynos5250
+	- "samsung,exynos5420-wdt" for Exynos5420
+	- "samsung,exynos7-wdt" for Exynos7
 
 - reg : base physical address of the controller and length of memory mapped
 	region.
drivers/watchdog/s3c2410_wdt.c +18 -3
index 3e216f54cb6f..7db7847a005c 100644
@@ -46,6 +46,7 @@
 #define S3C2410_WTCON		0x00
 #define S3C2410_WTDAT		0x04
 #define S3C2410_WTCNT		0x08
+#define S3C2410_WTCLRINT	0x0c
 
 #define S3C2410_WTCNT_MAXCNT	0xffff
 
@@ -72,6 +73,7 @@
 #define EXYNOS5_WDT_MASK_RESET_REG_OFFSET	0x040c
 #define QUIRK_HAS_PMU_CONFIG			(1 << 0)
 #define QUIRK_HAS_RST_STAT			(1 << 1)
+#define QUIRK_HAS_WTCLRINT_REG			(1 << 2)
 
 /* These quirks require that we have a PMU register map */
 #define QUIRKS_HAVE_PMUREG			(QUIRK_HAS_PMU_CONFIG | \
@@ -143,13 +145,18 @@ static const struct s3c2410_wdt_variant drv_data_s3c2410 = {
 };
 
 #ifdef CONFIG_OF
+static const struct s3c2410_wdt_variant drv_data_s3c6410 = {
+	.quirks = QUIRK_HAS_WTCLRINT_REG,
+};
+
 static const struct s3c2410_wdt_variant drv_data_exynos5250  = {
 	.disable_reg = EXYNOS5_WDT_DISABLE_REG_OFFSET,
 	.mask_reset_reg = EXYNOS5_WDT_MASK_RESET_REG_OFFSET,
 	.mask_bit = 20,
 	.rst_stat_reg = EXYNOS5_RST_STAT_REG_OFFSET,
 	.rst_stat_bit = 20,
-	.quirks = QUIRK_HAS_PMU_CONFIG | QUIRK_HAS_RST_STAT,
+	.quirks = QUIRK_HAS_PMU_CONFIG | QUIRK_HAS_RST_STAT \
+		  | QUIRK_HAS_WTCLRINT_REG,
 };
 
 static const struct s3c2410_wdt_variant drv_data_exynos5420 = {
@@ -158,7 +165,8 @@ static const struct s3c2410_wdt_variant drv_data_exynos5420 = {
 	.mask_bit = 0,
 	.rst_stat_reg = EXYNOS5_RST_STAT_REG_OFFSET,
 	.rst_stat_bit = 9,
-	.quirks = QUIRK_HAS_PMU_CONFIG | QUIRK_HAS_RST_STAT,
+	.quirks = QUIRK_HAS_PMU_CONFIG | QUIRK_HAS_RST_STAT \
+		  | QUIRK_HAS_WTCLRINT_REG,
 };
 
 static const struct s3c2410_wdt_variant drv_data_exynos7 = {
@@ -167,12 +175,15 @@ static const struct s3c2410_wdt_variant drv_data_exynos7 = {
 	.mask_bit = 23,
 	.rst_stat_reg = EXYNOS5_RST_STAT_REG_OFFSET,
 	.rst_stat_bit = 23,	/* A57 WDTRESET */
-	.quirks = QUIRK_HAS_PMU_CONFIG | QUIRK_HAS_RST_STAT,
+	.quirks = QUIRK_HAS_PMU_CONFIG | QUIRK_HAS_RST_STAT \
+		  | QUIRK_HAS_WTCLRINT_REG,
 };
 
 static const struct of_device_id s3c2410_wdt_match[] = {
 	{ .compatible = "samsung,s3c2410-wdt",
 	  .data = &drv_data_s3c2410 },
+	{ .compatible = "samsung,s3c6410-wdt",
+	  .data = &drv_data_s3c6410 },
 	{ .compatible = "samsung,exynos5250-wdt",
 	  .data = &drv_data_exynos5250 },
 	{ .compatible = "samsung,exynos5420-wdt",
@@ -418,6 +429,10 @@ static irqreturn_t s3c2410wdt_irq(int irqno, void *param)
 	dev_info(wdt->dev, "watchdog timer expired (irq)\n");
 
 	s3c2410wdt_keepalive(&wdt->wdt_device);
+
+	if (wdt->drv_data->quirks & QUIRK_HAS_WTCLRINT_REG)
+		writel(0x1, wdt->reg_base + S3C2410_WTCLRINT);
+
 	return IRQ_HANDLED;
 }
 
Signature · 1 line
2.9.3
author maintainer
#4 lore raw

Re: [PATCH v2 2/8] watchdog: s3c2410: Fix infinite interrupt in soft mode

Quoted 14 lines · Krzysztof Kozlowski

In soft (no-reboot) mode, the driver self-pings watchdog upon expiration of an interrupt. However the interrupt itself was not cleared thus on first hit, the system enters infinite interrupt handling loop.

On Odroid U3 (Exynos4412), when booted with s3c2410_wdt.soft_noboot=1 argument the console is flooded:

	# killall -9 watchdog
	[   60.523760] s3c2410-wdt 10060000.watchdog: watchdog timer expired (irq)
	[   60.536744] s3c2410-wdt 10060000.watchdog: watchdog timer expired (irq)

Fix this by writing something to the WTCLRINT register to clear the interrupt. The register WTCLRINT however appeared in S3C6410 so a new watchdog quirk and flavor are needed.

Quoted 8 lines
---
 .../devicetree/bindings/watchdog/samsung-wdt.txt    |  9 +++++----
 drivers/watchdog/s3c2410_wdt.c                      | 21 ++++++++++++++++++---
 2 files changed, 23 insertions(+), 7 deletions(-)

This can be ignored, already applied.

Best regards,
Krzysztof

author maintainer
#5 lore raw

PATCH v2 3/8 ARM: dts: exynos: Fix watchdog reset on Exynos4412

In reply to Krzysztof Kozlowski

The Exynos4412 has the same watchdog as newer SoCs (e.g. Exynos5250). Just like the others, for working it requires additional steps in Power Management Unit: unmasking the reset request and enabling the system reset. Without these additional steps in PMU, the watchdog will not be able to reset the system on expiration event.

Change the compatible of Exynos4412 watchdog device node to samsung,exynos5250-wdt which includes the additional PMU steps.

This will also fix infinite watchdog interrupt in soft mode (lack of interrupt clear) because it is also included in samsung,exynos5250-wdt.

---
 arch/arm/boot/dts/exynos4.dtsi    |  9 ---------
 arch/arm/boot/dts/exynos4210.dtsi |  9 +++++++++
 arch/arm/boot/dts/exynos4412.dtsi | 10 ++++++++++
 3 files changed, 19 insertions(+), 9 deletions(-)
arch/arm/boot/dts/exynos4.dtsi +0 -9
index 18def1c774d5..71e2cdae6068 100644
@@ -283,15 +283,6 @@
 		};
 	};
 
-	watchdog: watchdog at 10060000 {
-		compatible = "samsung,s3c2410-wdt";
-		reg = <0x10060000 0x100>;
-		interrupts = <GIC_SPI 43 IRQ_TYPE_LEVEL_HIGH>;
-		clocks = <&clock CLK_WDT>;
-		clock-names = "watchdog";
-		status = "disabled";
-	};
-
 	rtc: rtc at 10070000 {
 		compatible = "samsung,s3c6410-rtc";
 		reg = <0x10070000 0x100>;
arch/arm/boot/dts/exynos4210.dtsi +9 -0
index f9408188f97f..8bff2253acca 100644
@@ -119,6 +119,15 @@
 		};
 	};
 
+	watchdog: watchdog at 10060000 {
+		compatible = "samsung,s3c2410-wdt";
+		reg = <0x10060000 0x100>;
+		interrupts = <GIC_SPI 43 IRQ_TYPE_LEVEL_HIGH>;
+		clocks = <&clock CLK_WDT>;
+		clock-names = "watchdog";
+		status = "disabled";
+	};
+
 	clock: clock-controller at 10030000 {
 		compatible = "samsung,exynos4210-clock";
 		reg = <0x10030000 0x20000>;
arch/arm/boot/dts/exynos4412.dtsi +10 -0
index 235bbb69ad7c..6f47988a1ab5 100644
@@ -215,6 +215,16 @@
 		};
 	};
 
+	watchdog: watchdog at 10060000 {
+		compatible = "samsung,exynos5250-wdt";
+		reg = <0x10060000 0x100>;
+		interrupts = <GIC_SPI 43 IRQ_TYPE_LEVEL_HIGH>;
+		clocks = <&clock CLK_WDT>;
+		clock-names = "watchdog";
+		samsung,syscon-phandle = <&pmu_system_controller>;
+		status = "disabled";
+	};
+
 	adc: adc at 126C0000 {
 		compatible = "samsung,exynos-adc-v1";
 		reg = <0x126C0000 0x100>;
Signature · 1 line
2.9.3
maintainer Acked-by on v2 3/8
#6 lore raw

Re: [PATCH v2 3/8] ARM: dts: exynos: Fix watchdog reset on Exynos4412

Quoted 6 lines · Krzysztof Kozlowski

The Exynos4412 has the same watchdog as newer SoCs (e.g. Exynos5250). Just like the others, for working it requires additional steps in Power Management Unit: unmasking the reset request and enabling the system reset. Without these additional steps in PMU, the watchdog will not be able to reset the system on expiration event.

Quoted 7 lines

Change the compatible of Exynos4412 watchdog device node to samsung,exynos5250-wdt which includes the additional PMU steps.

This will also fix infinite watchdog interrupt in soft mode (lack of interrupt clear) because it is also included in samsung,exynos5250-wdt.

I assume this patch will be pushed through some arm tree.

Thanks,
Guenter

Quoted 68 lines
---
 arch/arm/boot/dts/exynos4.dtsi    |  9 ---------
 arch/arm/boot/dts/exynos4210.dtsi |  9 +++++++++
 arch/arm/boot/dts/exynos4412.dtsi | 10 ++++++++++
 3 files changed, 19 insertions(+), 9 deletions(-)
arch/arm/boot/dts/exynos4.dtsi +0 -9
index 18def1c774d5..71e2cdae6068 100644
@@ -283,15 +283,6 @@
 		};
 	};
 
-	watchdog: watchdog at 10060000 {
-		compatible = "samsung,s3c2410-wdt";
-		reg = <0x10060000 0x100>;
-		interrupts = <GIC_SPI 43 IRQ_TYPE_LEVEL_HIGH>;
-		clocks = <&clock CLK_WDT>;
-		clock-names = "watchdog";
-		status = "disabled";
-	};
-
 	rtc: rtc at 10070000 {
 		compatible = "samsung,s3c6410-rtc";
 		reg = <0x10070000 0x100>;
arch/arm/boot/dts/exynos4210.dtsi +9 -0
index f9408188f97f..8bff2253acca 100644
@@ -119,6 +119,15 @@
 		};
 	};
 
+	watchdog: watchdog at 10060000 {
+		compatible = "samsung,s3c2410-wdt";
+		reg = <0x10060000 0x100>;
+		interrupts = <GIC_SPI 43 IRQ_TYPE_LEVEL_HIGH>;
+		clocks = <&clock CLK_WDT>;
+		clock-names = "watchdog";
+		status = "disabled";
+	};
+
 	clock: clock-controller at 10030000 {
 		compatible = "samsung,exynos4210-clock";
 		reg = <0x10030000 0x20000>;
arch/arm/boot/dts/exynos4412.dtsi +10 -0
index 235bbb69ad7c..6f47988a1ab5 100644
@@ -215,6 +215,16 @@
 		};
 	};
 
+	watchdog: watchdog at 10060000 {
+		compatible = "samsung,exynos5250-wdt";
+		reg = <0x10060000 0x100>;
+		interrupts = <GIC_SPI 43 IRQ_TYPE_LEVEL_HIGH>;
+		clocks = <&clock CLK_WDT>;
+		clock-names = "watchdog";
+		samsung,syscon-phandle = <&pmu_system_controller>;
+		status = "disabled";
+	};
+
 	adc: adc at 126C0000 {
 		compatible = "samsung,exynos-adc-v1";
 		reg = <0x126C0000 0x100>;
author maintainer
#7 lore raw

Re: [PATCH v2 3/8] ARM: dts: exynos: Fix watchdog reset on Exynos4412

Quoted 14 lines · Guenter Roeck
Quoted 13 lines · Krzysztof Kozlowski

The Exynos4412 has the same watchdog as newer SoCs (e.g. Exynos5250). Just like the others, for working it requires additional steps in Power Management Unit: unmasking the reset request and enabling the system reset. Without these additional steps in PMU, the watchdog will not be able to reset the system on expiration event.

Change the compatible of Exynos4412 watchdog device node to samsung,exynos5250-wdt which includes the additional PMU steps.

This will also fix infinite watchdog interrupt in soft mode (lack of interrupt clear) because it is also included in samsung,exynos5250-wdt.

Quoted 6 lines
Quoted 1 line

I assume this patch will be pushed through some arm tree.

Yes, I will take it through samsung-soc.

Best regards,
Krzysztof

#8 lore raw

Re: [PATCH v2 3/8] ARM: dts: exynos: Fix watchdog reset on Exynos4412

In reply to Krzysztof Kozlowski

Hi,

Quoted 8 lines · Krzysztof Kozlowski

The Exynos4412 has the same watchdog as newer SoCs (e.g. Exynos5250). Just like the others, for working it requires additional steps in Power Management Unit: unmasking the reset request and enabling the system reset. Without these additional steps in PMU, the watchdog will not be able to reset the system on expiration event.

Change the compatible of Exynos4412 watchdog device node to samsung,exynos5250-wdt which includes the additional PMU steps.

This is going to confuse people. How's about doing it cleanly (by adding samsung,exynos4412-wdt compatible and convert existing users to use instead)?

Quoted 4 lines

This will also fix infinite watchdog interrupt in soft mode (lack of interrupt clear) because it is also included in samsung,exynos5250-wdt.

Best regards,
--
Bartlomiej Zolnierkiewicz
Samsung R&D Institute Poland
Samsung Electronics

author maintainer
#9 lore raw

Re: [PATCH v2 3/8] ARM: dts: exynos: Fix watchdog reset on Exynos4412

Quoted 10 lines · Bartlomiej Zolnierkiewicz

Hi,

Quoted 6 lines · Krzysztof Kozlowski

The Exynos4412 has the same watchdog as newer SoCs (e.g. Exynos5250). Just like the others, for working it requires additional steps in Power Management Unit: unmasking the reset request and enabling the system reset. Without these additional steps in PMU, the watchdog will not be able to reset the system on expiration event.

Quoted 6 lines
Quoted 2 lines

Change the compatible of Exynos4412 watchdog device node to samsung,exynos5250-wdt which includes the additional PMU steps.

This is going to confuse people. How's about doing it cleanly (by adding samsung,exynos4412-wdt compatible and convert existing users to use instead)?

I don't find usage of 5250 in Exynos4412 DTS as confusing because since long time I do not threat these numbers as having any meaning (e.g. 3250 is newer...). Yes, we can duplicate the compatible for that purpose. Care to send a patch for this?

Best regards,
Krzysztof

#10 lore raw

Re: [PATCH v2 3/8] ARM: dts: exynos: Fix watchdog reset on Exynos4412

Quoted 15 lines · Krzysztof Kozlowski
Quoted 13 lines · Bartlomiej Zolnierkiewicz

Hi,

Quoted 8 lines · Krzysztof Kozlowski

The Exynos4412 has the same watchdog as newer SoCs (e.g. Exynos5250). Just like the others, for working it requires additional steps in Power Management Unit: unmasking the reset request and enabling the system reset. Without these additional steps in PMU, the watchdog will not be able to reset the system on expiration event.

Change the compatible of Exynos4412 watchdog device node to samsung,exynos5250-wdt which includes the additional PMU steps.

Quoted 8 lines
Quoted 3 lines

This is going to confuse people. How's about doing it cleanly (by adding samsung,exynos4412-wdt compatible and convert existing users to use instead)?

I don't find usage of 5250 in Exynos4412 DTS as confusing because since long time I do not threat these numbers as having any meaning (e.g. 3250 is newer...). Yes, we can duplicate the compatible for that purpose. Care to send a patch for this?

I don't buy this argument. The point is not in the number itself but in the fact that you as developer know than 3250 is newer than 4412 (the same way as you know that 5250 is also newer than 4412). This convention is used for all Exynos bindings so please don't break it.

Best regards,
--
Bartlomiej Zolnierkiewicz
Samsung R&D Institute Poland
Samsung Electronics

author maintainer
#11 lore raw

PATCH v2 4/8 ARM: dts: s3c64xx: Enable watchdog on all S3C64xx boards

In reply to Krzysztof Kozlowski

Watchdog module does not have external dependencies so it can be safely enabled in s3c64xx.dtsi thus making it available for all S3C64xx-based boards.

---
 arch/arm/boot/dts/s3c64xx.dtsi | 1 -
 1 file changed, 1 deletion(-)
arch/arm/boot/dts/s3c64xx.dtsi +0 -1
index 0ccb414cd268..6caec5f27d38 100644
@@ -100,7 +100,6 @@
 			interrupts = <26>;
 			clock-names = "watchdog";
 			clocks = <&clocks PCLK_WDT>;
-			status = "disabled";
 		};
 
 		i2c0: i2c at 7f004000 {
Signature · 1 line
2.9.3
maintainer Acked-by on v2 4/8
#12 lore raw

Re: [PATCH v2 4/8] ARM: dts: s3c64xx: Enable watchdog on all S3C64xx boards

Quoted 5 lines · Krzysztof Kozlowski

Watchdog module does not have external dependencies so it can be safely enabled in s3c64xx.dtsi thus making it available for all S3C64xx-based boards.

Quoted 17 lines
---
 arch/arm/boot/dts/s3c64xx.dtsi | 1 -
 1 file changed, 1 deletion(-)
arch/arm/boot/dts/s3c64xx.dtsi +0 -1
index 0ccb414cd268..6caec5f27d38 100644
@@ -100,7 +100,6 @@
 			interrupts = <26>;
 			clock-names = "watchdog";
 			clocks = <&clocks PCLK_WDT>;
-			status = "disabled";
 		};
 
 		i2c0: i2c at 7f004000 {
#13 lore raw

Re: [PATCH v2 4/8] ARM: dts: s3c64xx: Enable watchdog on all S3C64xx boards

In reply to Krzysztof Kozlowski

Hi,

Quoted 5 lines · Krzysztof Kozlowski

Watchdog module does not have external dependencies so it can be safely enabled in s3c64xx.dtsi thus making it available for all S3C64xx-based boards.

Please first fix the driver w.r.t. infinite interrupt in soft mode issue and only then enable it on more hardware. IOW this patch should be placed after patch #7 in the series.

Best regards,
--
Bartlomiej Zolnierkiewicz
Samsung R&D Institute Poland
Samsung Electronics

author maintainer
#14 lore raw

Re: [PATCH v2 4/8] ARM: dts: s3c64xx: Enable watchdog on all S3C64xx boards

Quoted 8 lines · Bartlomiej Zolnierkiewicz

Hi,

Quoted 4 lines · Krzysztof Kozlowski

Watchdog module does not have external dependencies so it can be safely enabled in s3c64xx.dtsi thus making it available for all S3C64xx-based boards.

Quoted 5 lines
Quoted 1 line

Please first fix the driver w.r.t. infinite interrupt in soft mode issue and only then enable it on more hardware. IOW this patch should be placed after patch #7 in the series.

The soft-interrupt mode is not the default one so this should not cause any issues by default but I got your point. Makes sense. The pity is the timing. There wasn't much interest in the patchset (no comments since 24 of Feb) so I merged it yesterday evening. Does it brake any platform/board?

Best regards,
Krzysztof

#15 lore raw

Re: [PATCH v2 4/8] ARM: dts: s3c64xx: Enable watchdog on all S3C64xx boards

Quoted 16 lines · Krzysztof Kozlowski
Quoted 8 lines · Bartlomiej Zolnierkiewicz

Hi,

Quoted 4 lines · Krzysztof Kozlowski

Watchdog module does not have external dependencies so it can be safely enabled in s3c64xx.dtsi thus making it available for all S3C64xx-based boards.

Quoted 5 lines
Quoted 1 line

Please first fix the driver w.r.t. infinite interrupt in soft mode issue and only then enable it on more hardware. IOW this patch should be placed after patch #7 in the series.

Quoted 5 lines

The soft-interrupt mode is not the default one so this should not cause any issues by default but I got your point. Makes sense. The pity is the timing. There wasn't much interest in the patchset (no comments since 24 of Feb) so I merged it yesterday evening. Does it brake any platform/board?

OK, let it be as it as it doesn't break the default setup.

However please consider the fact that posting revised patches (v2 was posted *yesterday*) for a review and then merging them the same day is not a good practice..

Best regards,
--
Bartlomiej Zolnierkiewicz
Samsung R&D Institute Poland
Samsung Electronics

author maintainer
#16 lore raw

Re: [PATCH v2 4/8] ARM: dts: s3c64xx: Enable watchdog on all S3C64xx boards

Quoted 23 lines · Bartlomiej Zolnierkiewicz
Quoted 16 lines · Krzysztof Kozlowski
Quoted 8 lines · Bartlomiej Zolnierkiewicz

Hi,

Quoted 4 lines · Krzysztof Kozlowski

Watchdog module does not have external dependencies so it can be safely enabled in s3c64xx.dtsi thus making it available for all S3C64xx-based boards.

Quoted 5 lines
Quoted 1 line

Please first fix the driver w.r.t. infinite interrupt in soft mode issue and only then enable it on more hardware. IOW this patch should be placed after patch #7 in the series.

Quoted 5 lines

The soft-interrupt mode is not the default one so this should not cause any issues by default but I got your point. Makes sense. The pity is the timing. There wasn't much interest in the patchset (no comments since 24 of Feb) so I merged it yesterday evening. Does it brake any platform/board?

Quoted 5 lines

OK, let it be as it as it doesn't break the default setup.

However please consider the fact that posting revised patches (v2 was posted *yesterday*) for a review and then merging them the same day is not a good practice..

v2 was posted on March 11, not yesterday but I got the point that the rest of world is working from Monday not Sunday. :) And v1 was on 24 of Feb. Usually I try to give them more time but recently there is so less interest in this that I gave up waiting. I will give them more time in the future.

Thanks for review!

Krzysztof

author maintainer
#17 lore raw

PATCH v2 5/8 ARM: dts: exynos: Enable watchdog on all Exynos4 boards

In reply to Krzysztof Kozlowski

Watchdog module does not have external dependencies so it can be safely enabled in exynos4.dtsi thus making it available for all Exynos4-based boards.

---
 arch/arm/boot/dts/exynos4210-origen.dts         | 4 ----
 arch/arm/boot/dts/exynos4210.dtsi               | 1 -
 arch/arm/boot/dts/exynos4412-itop-scp-core.dtsi | 4 ----
 arch/arm/boot/dts/exynos4412-odroid-common.dtsi | 4 ----
 arch/arm/boot/dts/exynos4412-origen.dts         | 4 ----
 arch/arm/boot/dts/exynos4412.dtsi               | 1 -
 6 files changed, 18 deletions(-)
arch/arm/boot/dts/exynos4210-origen.dts +0 -4
index a2c6a13fe67b..312650e2450f 100644
@@ -328,7 +328,3 @@
 &tmu {
 	status = "okay";
 };
-
-&watchdog {
-	status = "okay";
-};
arch/arm/boot/dts/exynos4210.dtsi +0 -1
index 8bff2253acca..9d51d4d62d94 100644
@@ -125,7 +125,6 @@
 		interrupts = <GIC_SPI 43 IRQ_TYPE_LEVEL_HIGH>;
 		clocks = <&clock CLK_WDT>;
 		clock-names = "watchdog";
-		status = "disabled";
 	};
 
 	clock: clock-controller at 10030000 {
arch/arm/boot/dts/exynos4412-itop-scp-core.dtsi +0 -4
index a36cd36a26b8..4cd62487bb16 100644
@@ -495,7 +495,3 @@
 	vtmu-supply = <&ldo16_reg>;
 	status = "okay";
 };
-
-&watchdog {
-	status = "okay";
-};
arch/arm/boot/dts/exynos4412-odroid-common.dtsi +0 -4
index 78f118cb73d4..0f1ff792fe44 100644
@@ -555,7 +555,3 @@
 	vtmu-supply = <&ldo10_reg>;
 	status = "okay";
 };
-
-&watchdog {
-	status = "okay";
-};
arch/arm/boot/dts/exynos4412-origen.dts +0 -4
index a1ab6f94bb64..7a83e2df18a6 100644
@@ -541,7 +541,3 @@
 &serial_3 {
 	status = "okay";
 };
-
-&watchdog {
-	status = "okay";
-};
arch/arm/boot/dts/exynos4412.dtsi +0 -1
index 6f47988a1ab5..cc69466f17c0 100644
@@ -222,7 +222,6 @@
 		clocks = <&clock CLK_WDT>;
 		clock-names = "watchdog";
 		samsung,syscon-phandle = <&pmu_system_controller>;
-		status = "disabled";
 	};
 
 	adc: adc at 126C0000 {
Signature · 1 line
2.9.3
maintainer Acked-by on v2 5/8
#18 lore raw

Re: [PATCH v2 5/8] ARM: dts: exynos: Enable watchdog on all Exynos4 boards

Quoted 5 lines · Krzysztof Kozlowski

Watchdog module does not have external dependencies so it can be safely enabled in exynos4.dtsi thus making it available for all Exynos4-based boards.

Quoted 82 lines
---
 arch/arm/boot/dts/exynos4210-origen.dts         | 4 ----
 arch/arm/boot/dts/exynos4210.dtsi               | 1 -
 arch/arm/boot/dts/exynos4412-itop-scp-core.dtsi | 4 ----
 arch/arm/boot/dts/exynos4412-odroid-common.dtsi | 4 ----
 arch/arm/boot/dts/exynos4412-origen.dts         | 4 ----
 arch/arm/boot/dts/exynos4412.dtsi               | 1 -
 6 files changed, 18 deletions(-)
arch/arm/boot/dts/exynos4210-origen.dts +0 -4
index a2c6a13fe67b..312650e2450f 100644
@@ -328,7 +328,3 @@
 &tmu {
 	status = "okay";
 };
-
-&watchdog {
-	status = "okay";
-};
arch/arm/boot/dts/exynos4210.dtsi +0 -1
index 8bff2253acca..9d51d4d62d94 100644
@@ -125,7 +125,6 @@
 		interrupts = <GIC_SPI 43 IRQ_TYPE_LEVEL_HIGH>;
 		clocks = <&clock CLK_WDT>;
 		clock-names = "watchdog";
-		status = "disabled";
 	};
 
 	clock: clock-controller at 10030000 {
arch/arm/boot/dts/exynos4412-itop-scp-core.dtsi +0 -4
index a36cd36a26b8..4cd62487bb16 100644
@@ -495,7 +495,3 @@
 	vtmu-supply = <&ldo16_reg>;
 	status = "okay";
 };
-
-&watchdog {
-	status = "okay";
-};
arch/arm/boot/dts/exynos4412-odroid-common.dtsi +0 -4
index 78f118cb73d4..0f1ff792fe44 100644
@@ -555,7 +555,3 @@
 	vtmu-supply = <&ldo10_reg>;
 	status = "okay";
 };
-
-&watchdog {
-	status = "okay";
-};
arch/arm/boot/dts/exynos4412-origen.dts +0 -4
index a1ab6f94bb64..7a83e2df18a6 100644
@@ -541,7 +541,3 @@
 &serial_3 {
 	status = "okay";
 };
-
-&watchdog {
-	status = "okay";
-};
arch/arm/boot/dts/exynos4412.dtsi +0 -1
index 6f47988a1ab5..cc69466f17c0 100644
@@ -222,7 +222,6 @@
 		clocks = <&clock CLK_WDT>;
 		clock-names = "watchdog";
 		samsung,syscon-phandle = <&pmu_system_controller>;
-		status = "disabled";
 	};
 
 	adc: adc at 126C0000 {
#19 lore raw

Re: [PATCH v2 5/8] ARM: dts: exynos: Enable watchdog on all Exynos4 boards

In reply to Krzysztof Kozlowski

Hi,

Quoted 3 lines · Krzysztof Kozlowski

Watchdog module does not have external dependencies so it can be safely enabled in exynos4.dtsi thus making it available for all Exynos4-based boards.

Please fix the patch description (patch #3 moved watchdog node out from exynos4.dtsi).

Quoted 9 lines
---
 arch/arm/boot/dts/exynos4210-origen.dts         | 4 ----
 arch/arm/boot/dts/exynos4210.dtsi               | 1 -
 arch/arm/boot/dts/exynos4412-itop-scp-core.dtsi | 4 ----
 arch/arm/boot/dts/exynos4412-odroid-common.dtsi | 4 ----
 arch/arm/boot/dts/exynos4412-origen.dts         | 4 ----
 arch/arm/boot/dts/exynos4412.dtsi               | 1 -
 6 files changed, 18 deletions(-)

Best regards,
--
Bartlomiej Zolnierkiewicz
Samsung R&D Institute Poland
Samsung Electronics

author maintainer
#20 lore raw

PATCH v2 6/8 ARM: dts: exynos: Fix infinite interrupt in soft mode on Exynos4210 and Exynos5440

In reply to Krzysztof Kozlowski

In soft (no-reboot) mode, the driver self-pings watchdog upon expiration of an interrupt. The interrupt has to be cleared, because otherwise system enters infinite interrupt handling loop.

Use a samsung,s3c6410-wdt compatible to select appropriate quirk for clearing the watchdog interrupt.

---
 arch/arm/boot/dts/exynos4210.dtsi | 2 +-
 arch/arm/boot/dts/exynos5440.dtsi | 2 +-
 2 files changed, 2 insertions(+), 2 deletions(-)
arch/arm/boot/dts/exynos4210.dtsi +1 -1
index 9d51d4d62d94..e6e62103a71f 100644
@@ -120,7 +120,7 @@
 	};
 
 	watchdog: watchdog at 10060000 {
-		compatible = "samsung,s3c2410-wdt";
+		compatible = "samsung,s3c6410-wdt";
 		reg = <0x10060000 0x100>;
 		interrupts = <GIC_SPI 43 IRQ_TYPE_LEVEL_HIGH>;
 		clocks = <&clock CLK_WDT>;
arch/arm/boot/dts/exynos5440.dtsi +1 -1
index 77d35bb92950..abfe054b2187 100644
@@ -189,7 +189,7 @@
 	};
 
 	watchdog at 110000 {
-		compatible = "samsung,s3c2410-wdt";
+		compatible = "samsung,s3c6410-wdt";
 		reg = <0x110000 0x1000>;
 		interrupts = <GIC_SPI 1 IRQ_TYPE_LEVEL_HIGH>;
 		clocks = <&clock CLK_B_125>;
Signature · 1 line
2.9.3
maintainer Acked-by on v2 6/8
#21 lore raw

Re: [PATCH v2 6/8] ARM: dts: exynos: Fix infinite interrupt in soft mode on Exynos4210 and Exynos5440

Quoted 8 lines · Krzysztof Kozlowski

In soft (no-reboot) mode, the driver self-pings watchdog upon expiration of an interrupt. The interrupt has to be cleared, because otherwise system enters infinite interrupt handling loop.

Use a samsung,s3c6410-wdt compatible to select appropriate quirk for clearing the watchdog interrupt.

Quoted 32 lines
---
 arch/arm/boot/dts/exynos4210.dtsi | 2 +-
 arch/arm/boot/dts/exynos5440.dtsi | 2 +-
 2 files changed, 2 insertions(+), 2 deletions(-)
arch/arm/boot/dts/exynos4210.dtsi +1 -1
index 9d51d4d62d94..e6e62103a71f 100644
@@ -120,7 +120,7 @@
 	};
 
 	watchdog: watchdog at 10060000 {
-		compatible = "samsung,s3c2410-wdt";
+		compatible = "samsung,s3c6410-wdt";
 		reg = <0x10060000 0x100>;
 		interrupts = <GIC_SPI 43 IRQ_TYPE_LEVEL_HIGH>;
 		clocks = <&clock CLK_WDT>;
arch/arm/boot/dts/exynos5440.dtsi +1 -1
index 77d35bb92950..abfe054b2187 100644
@@ -189,7 +189,7 @@
 	};
 
 	watchdog at 110000 {
-		compatible = "samsung,s3c2410-wdt";
+		compatible = "samsung,s3c6410-wdt";
 		reg = <0x110000 0x1000>;
 		interrupts = <GIC_SPI 1 IRQ_TYPE_LEVEL_HIGH>;
 		clocks = <&clock CLK_B_125>;
Reviewed-by on v2 6/8
#22 lore raw

Re: [PATCH v2 6/8] ARM: dts: exynos: Fix infinite interrupt in soft mode on Exynos4210 and Exynos5440

In reply to Krzysztof Kozlowski
Quoted 8 lines · Krzysztof Kozlowski

In soft (no-reboot) mode, the driver self-pings watchdog upon expiration of an interrupt. The interrupt has to be cleared, because otherwise system enters infinite interrupt handling loop.

Use a samsung,s3c6410-wdt compatible to select appropriate quirk for clearing the watchdog interrupt.

Best regards,
--
Bartlomiej Zolnierkiewicz
Samsung R&D Institute Poland
Samsung Electronics

author maintainer
#23 lore raw

PATCH v2 7/8 ARM: dts: s3c64xx: Fix infinite interrupt in soft mode

In reply to Krzysztof Kozlowski

In soft (no-reboot) mode, the driver self-pings watchdog upon expiration of an interrupt. The interrupt has to be cleared, because otherwise system enters infinite interrupt handling loop.

Use a samsung,s3c6410-wdt compatible to select appropriate quirk for clearing the watchdog interrupt.

---
 arch/arm/boot/dts/s3c64xx.dtsi | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)
arch/arm/boot/dts/s3c64xx.dtsi +1 -1
index 6caec5f27d38..c55cbb3af2c0 100644
@@ -94,7 +94,7 @@
 		};
 
 		watchdog: watchdog at 7e004000 {
-			compatible = "samsung,s3c2410-wdt";
+			compatible = "samsung,s3c6410-wdt";
 			reg = <0x7e004000 0x1000>;
 			interrupt-parent = <&vic0>;
 			interrupts = <26>;
Signature · 1 line
2.9.3
maintainer Acked-by on v2 7/8
#24 lore raw

Re: [PATCH v2 7/8] ARM: dts: s3c64xx: Fix infinite interrupt in soft mode

Quoted 8 lines · Krzysztof Kozlowski

In soft (no-reboot) mode, the driver self-pings watchdog upon expiration of an interrupt. The interrupt has to be cleared, because otherwise system enters infinite interrupt handling loop.

Use a samsung,s3c6410-wdt compatible to select appropriate quirk for clearing the watchdog interrupt.

Quoted 18 lines
---
 arch/arm/boot/dts/s3c64xx.dtsi | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)
arch/arm/boot/dts/s3c64xx.dtsi +1 -1
index 6caec5f27d38..c55cbb3af2c0 100644
@@ -94,7 +94,7 @@
 		};
 
 		watchdog: watchdog at 7e004000 {
-			compatible = "samsung,s3c2410-wdt";
+			compatible = "samsung,s3c6410-wdt";
 			reg = <0x7e004000 0x1000>;
 			interrupt-parent = <&vic0>;
 			interrupts = <26>;
Reviewed-by on v2 7/8
#25 lore raw

Re: [PATCH v2 7/8] ARM: dts: s3c64xx: Fix infinite interrupt in soft mode

In reply to Krzysztof Kozlowski
Quoted 8 lines · Krzysztof Kozlowski

In soft (no-reboot) mode, the driver self-pings watchdog upon expiration of an interrupt. The interrupt has to be cleared, because otherwise system enters infinite interrupt handling loop.

Use a samsung,s3c6410-wdt compatible to select appropriate quirk for clearing the watchdog interrupt.

Best regards,
--
Bartlomiej Zolnierkiewicz
Samsung R&D Institute Poland
Samsung Electronics

author maintainer
#26 lore raw

PATCH v2 8/8 ARM: dts: s5pv210: Fix infinite interrupt in soft mode

In reply to Krzysztof Kozlowski

In soft (no-reboot) mode, the driver self-pings watchdog upon expiration of an interrupt. The interrupt has to be cleared, because otherwise system enters infinite interrupt handling loop.

Use a samsung,s3c6410-wdt compatible to select appropriate quirk for clearing the watchdog interrupt.

---
 arch/arm/boot/dts/s5pv210.dtsi | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)
arch/arm/boot/dts/s5pv210.dtsi +1 -1
index a853918be43f..726c5d0dbd5b 100644
@@ -310,7 +310,7 @@
 		};
 
 		watchdog: watchdog at e2700000 {
-			compatible = "samsung,s3c2410-wdt";
+			compatible = "samsung,s3c6410-wdt";
 			reg = <0xe2700000 0x1000>;
 			interrupt-parent = <&vic0>;
 			interrupts = <26>;
Signature · 1 line
2.9.3
maintainer Acked-by on v2 8/8
#27 lore raw

Re: [PATCH v2 8/8] ARM: dts: s5pv210: Fix infinite interrupt in soft mode

Quoted 8 lines · Krzysztof Kozlowski

In soft (no-reboot) mode, the driver self-pings watchdog upon expiration of an interrupt. The interrupt has to be cleared, because otherwise system enters infinite interrupt handling loop.

Use a samsung,s3c6410-wdt compatible to select appropriate quirk for clearing the watchdog interrupt.

Quoted 18 lines
---
 arch/arm/boot/dts/s5pv210.dtsi | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)
arch/arm/boot/dts/s5pv210.dtsi +1 -1
index a853918be43f..726c5d0dbd5b 100644
@@ -310,7 +310,7 @@
 		};
 
 		watchdog: watchdog at e2700000 {
-			compatible = "samsung,s3c2410-wdt";
+			compatible = "samsung,s3c6410-wdt";
 			reg = <0xe2700000 0x1000>;
 			interrupt-parent = <&vic0>;
 			interrupts = <26>;
Reviewed-by on v2 8/8
#28 lore raw

Re: [PATCH v2 8/8] ARM: dts: s5pv210: Fix infinite interrupt in soft mode

In reply to Krzysztof Kozlowski
Quoted 8 lines · Krzysztof Kozlowski

In soft (no-reboot) mode, the driver self-pings watchdog upon expiration of an interrupt. The interrupt has to be cleared, because otherwise system enters infinite interrupt handling loop.

Use a samsung,s3c6410-wdt compatible to select appropriate quirk for clearing the watchdog interrupt.

Best regards,
--
Bartlomiej Zolnierkiewicz
Samsung R&D Institute Poland
Samsung Electronics