[PATCH 0/2] imx6: Implement external watchdog reset

Discussion:

Tim Harvey

2015-05-28 22:54:35 UTC

Permalink

--
1.9.1

Tim Harvey

2015-05-28 22:54:36 UTC

Permalink

The IMX6 watchdog supports assertion of a signal (WDOG_B) which
can be pinmux'd to an external pin. This is typically used for boards that
have PMIC's in control of the IMX6 power rails. In fact, failure to use
such an external reset on boards with external PMIC's can result in various
hangs due to the IMX6 not being fully reset [1] as well as the board failing
to reset because its PMIC has not been reset to provide adequate voltate for
the CPU when comming out of reset at 800Mhz.

This uses a new device-tree property 'ext-reset' to indicate the board has
such a reset and to cause the watchdog to be configured to assert WDOG_B
instead of an internal reset both on a watchdog timeout and in system_restart.

[1] http://lists.infradead.org/pipermail/linux-arm-kernel/2015-March/333689.html

Signed-off-by: Tim Harvey <***@gateworks.com>
---
.../devicetree/bindings/watchdog/fsl-imx-wdt.txt | 2 ++
drivers/watchdog/imx2_wdt.c | 18 ++++++++++++++++--
2 files changed, 18 insertions(+), 2 deletions(-)

diff --git a/Documentation/devicetree/bindings/watchdog/fsl-imx-wdt.txt b/Documentation/devicetree/bindings/watchdog/fsl-imx-wdt.txt
index 8dab6fd..14e04ac 100644
--- a/Documentation/devicetree/bindings/watchdog/fsl-imx-wdt.txt
+++ b/Documentation/devicetree/bindings/watchdog/fsl-imx-wdt.txt
@@ -9,6 +9,8 @@ Optional property:
- big-endian: If present the watchdog device's registers are implemented
in big endian mode, otherwise in native mode(same with CPU), for more
detail please see: Documentation/devicetree/bindings/regmap/regmap.txt.
+- ext-reset: If present the watchdog device is configured to assert its
+ external reset (WDOG_B) instead of issuing a software reset.

Examples:

diff --git a/drivers/watchdog/imx2_wdt.c b/drivers/watchdog/imx2_wdt.c
index 5e6d808..9ffd516 100644
--- a/drivers/watchdog/imx2_wdt.c
+++ b/drivers/watchdog/imx2_wdt.c
@@ -41,6 +41,8 @@

#define IMX2_WDT_WCR 0x00 /* Control Register */
#define IMX2_WDT_WCR_WT (0xFF << 8) /* -> Watchdog Timeout Field */
+#define IMX2_WDT_WCR_WDA (1 << 5) /* -> External reset Assert */
+#define IMX2_WDT_WCR_SRS (1 << 4) /* -> Software Reset Signal */
#define IMX2_WDT_WCR_WRE (1 << 3) /* -> WDOG Reset Enable */
#define IMX2_WDT_WCR_WDE (1 << 2) /* -> Watchdog Enable */
#define IMX2_WDT_WCR_WDZST (1 << 0) /* -> Watchdog timer Suspend */
@@ -65,6 +67,7 @@ struct imx2_wdt_device {
struct timer_list timer; /* Pings the watchdog when closed */
struct watchdog_device wdog;
struct notifier_block restart_handler;
+ bool ext_reset;
};

static bool nowayout = WATCHDOG_NOWAYOUT;
@@ -86,10 +89,17 @@ static const struct watchdog_info imx2_wdt_info = {
static int imx2_restart_handler(struct notifier_block *this, unsigned long mode,
void *cmd)
{
- unsigned int wcr_enable = IMX2_WDT_WCR_WDE;
+ unsigned int wcr_enable;
struct imx2_wdt_device *wdev = container_of(this,
struct imx2_wdt_device,
restart_handler);
+
+ if (!wdev->ext_reset)
+ wcr_enable = IMX2_WDT_WCR_WDE;
+ /* Use external reset */
+ else
+ wcr_enable = IMX2_WDT_WCR_WDE | IMX2_WDT_WCR_SRS;
+
/* Assert SRS signal */
regmap_write(wdev->regmap, 0, wcr_enable);
/*
@@ -120,7 +130,10 @@ static inline void imx2_wdt_setup(struct watchdog_device *wdog)
/* Strip the old watchdog Time-Out value */
val &= ~IMX2_WDT_WCR_WT;
/* Generate reset if WDOG times out */
- val &= ~IMX2_WDT_WCR_WRE;
+ if (!wdev->ext_reset)
+ val &= ~IMX2_WDT_WCR_WRE;
+ else
+ val |= IMX2_WDT_WCR_WRE; /* assert WDOG_B on time-out */
/* Keep Watchdog Disabled */
val &= ~IMX2_WDT_WCR_WDE;
/* Set the watchdog's Time-Out value */
@@ -262,6 +275,7 @@ static int __init imx2_wdt_probe(struct platform_device *pdev)
regmap_read(wdev->regmap, IMX2_WDT_WRSR, &val);
wdog->bootstatus = val & IMX2_WDT_WRSR_TOUT ? WDIOF_CARDRESET : 0;

+ wdev->ext_reset = of_property_read_bool(pdev->dev.of_node, "ext-reset");
wdog->timeout = clamp_t(unsigned, timeout, 1, IMX2_WDT_MAX_TIME);
if (wdog->timeout != timeout)
dev_warn(&pdev->dev, "Initial timeout out of range! Clamped from %u to %u\n",

--
1.9.1

Lucas Stach

2015-05-29 16:37:38 UTC

Permalink

Probably a bit of personal taste, but I would prefer the naming to
include the property that this is a output. So something like
"ext-reset-output".

Post by Tim Harvey
diff --git a/drivers/watchdog/imx2_wdt.c b/drivers/watchdog/imx2_wdt.c
index 5e6d808..9ffd516 100644
--- a/drivers/watchdog/imx2_wdt.c
+++ b/drivers/watchdog/imx2_wdt.c
@@ -41,6 +41,8 @@
#define IMX2_WDT_WCR 0x00 /* Control Register */
#define IMX2_WDT_WCR_WT (0xFF << 8) /* -> Watchdog Timeout Field */
+#define IMX2_WDT_WCR_WDA (1 << 5) /* -> External reset Assert */
+#define IMX2_WDT_WCR_SRS (1 << 4) /* -> Software Reset Signal */
#define IMX2_WDT_WCR_WRE (1 << 3) /* -> WDOG Reset Enable */
#define IMX2_WDT_WCR_WDE (1 << 2) /* -> Watchdog Enable */
#define IMX2_WDT_WCR_WDZST (1 << 0) /* -> Watchdog timer Suspend */
@@ -65,6 +67,7 @@ struct imx2_wdt_device {
struct timer_list timer; /* Pings the watchdog when closed */
struct watchdog_device wdog;
struct notifier_block restart_handler;
+ bool ext_reset;
};
static bool nowayout = WATCHDOG_NOWAYOUT;
@@ -86,10 +89,17 @@ static const struct watchdog_info imx2_wdt_info = {
static int imx2_restart_handler(struct notifier_block *this, unsigned long mode,
void *cmd)
{
- unsigned int wcr_enable = IMX2_WDT_WCR_WDE;
+ unsigned int wcr_enable;
struct imx2_wdt_device *wdev = container_of(this,
struct imx2_wdt_device,
restart_handler);
+
+ if (!wdev->ext_reset)
+ wcr_enable = IMX2_WDT_WCR_WDE;
+ /* Use external reset */
+ else
+ wcr_enable = IMX2_WDT_WCR_WDE | IMX2_WDT_WCR_SRS;
+

This doesn't look nice. Please write this simply as:

if (wdev->ext_reset)
wcr_enable |= IMX2_WDT_WCR_SRS;

Post by Tim Harvey
/* Assert SRS signal */
regmap_write(wdev->regmap, 0, wcr_enable);
/*
@@ -120,7 +130,10 @@ static inline void imx2_wdt_setup(struct watchdog_device *wdog)
/* Strip the old watchdog Time-Out value */
val &= ~IMX2_WDT_WCR_WT;
/* Generate reset if WDOG times out */
- val &= ~IMX2_WDT_WCR_WRE;
+ if (!wdev->ext_reset)
+ val &= ~IMX2_WDT_WCR_WRE;
+ else
+ val |= IMX2_WDT_WCR_WRE; /* assert WDOG_B on time-out */

The comment above this sections doesn't seem to match the code anymore
after your change.

Post by Tim Harvey
/* Keep Watchdog Disabled */
val &= ~IMX2_WDT_WCR_WDE;
/* Set the watchdog's Time-Out value */
@@ -262,6 +275,7 @@ static int __init imx2_wdt_probe(struct platform_device *pdev)
regmap_read(wdev->regmap, IMX2_WDT_WRSR, &val);
wdog->bootstatus = val & IMX2_WDT_WRSR_TOUT ? WDIOF_CARDRESET : 0;
+ wdev->ext_reset = of_property_read_bool(pdev->dev.of_node, "ext-reset");
wdog->timeout = clamp_t(unsigned, timeout, 1, IMX2_WDT_MAX_TIME);
if (wdog->timeout != timeout)
dev_warn(&pdev->dev, "Initial timeout out of range! Clamped from %u to %u\n",

--
Pengutronix e.K. | Lucas Stach |
Industrial Linux Solutions | http://www.pengutronix.de/ |

Tim Harvey

2015-05-28 22:54:37 UTC

Permalink

The Gateworks Ventana boards have a PMIC that can be used to regulate the
CPU voltage rails for DVFS support. In order to ensure this PMIC is properly
reset the watchdog needs to be configured to assert its external reset
signal.

Additionally the pad used for WDOG_B needs to be configured which we add to
a hog group for lack of a better location.

Signed-off-by: Tim Harvey <***@gateworks.com>
---
arch/arm/boot/dts/imx6qdl-gw51xx.dtsi | 13 +++++++++++++
arch/arm/boot/dts/imx6qdl-gw52xx.dtsi | 13 +++++++++++++
arch/arm/boot/dts/imx6qdl-gw53xx.dtsi | 13 +++++++++++++
arch/arm/boot/dts/imx6qdl-gw54xx.dtsi | 18 ++++++++++++++++++
arch/arm/boot/dts/imx6qdl-gw551x.dtsi | 12 ++++++++++++
arch/arm/boot/dts/imx6qdl-gw552x.dtsi | 13 +++++++++++++
6 files changed, 82 insertions(+)

diff --git a/arch/arm/boot/dts/imx6qdl-gw51xx.dtsi b/arch/arm/boot/dts/imx6qdl-gw51xx.dtsi
index f2867c4..740c6c4 100644
--- a/arch/arm/boot/dts/imx6qdl-gw51xx.dtsi
+++ b/arch/arm/boot/dts/imx6qdl-gw51xx.dtsi
@@ -210,8 +210,21 @@
status = "okay";
};

+&wdog1 {
+ ext-reset;
+};
+
&iomuxc {
+ pinctrl-names = "default";
+ pinctrl-0 = <&pinctrl_hog>;
+
imx6qdl-gw51xx {
+ pinctrl_hog: hoggrp {
+ fsl,pins = <
+ MX6QDL_PAD_DISP0_DAT8__WDOG1_B 0x1b0b0
+ >;
+ };
+
pinctrl_enet: enetgrp {
fsl,pins = <
MX6QDL_PAD_RGMII_RXC__RGMII_RXC 0x1b0b0
diff --git a/arch/arm/boot/dts/imx6qdl-gw52xx.dtsi b/arch/arm/boot/dts/imx6qdl-gw52xx.dtsi
index b5756c2..1b06f18 100644
--- a/arch/arm/boot/dts/imx6qdl-gw52xx.dtsi
+++ b/arch/arm/boot/dts/imx6qdl-gw52xx.dtsi
@@ -323,8 +323,21 @@
status = "okay";
};

+&wdog1 {
+ ext-reset;
+};
+
&iomuxc {
+ pinctrl-names = "default";
+ pinctrl-0 = <&pinctrl_hog>;
+
imx6qdl-gw52xx {
+ pinctrl_hog: hoggrp {
+ fsl,pins = <
+ MX6QDL_PAD_DISP0_DAT8__WDOG1_B 0x1b0b0
+ >;
+ };
+
pinctrl_audmux: audmuxgrp {
fsl,pins = <
MX6QDL_PAD_SD2_DAT0__AUD4_RXD 0x130b0
diff --git a/arch/arm/boot/dts/imx6qdl-gw53xx.dtsi b/arch/arm/boot/dts/imx6qdl-gw53xx.dtsi
index 86f03c1..65e5c01 100644
--- a/arch/arm/boot/dts/imx6qdl-gw53xx.dtsi
+++ b/arch/arm/boot/dts/imx6qdl-gw53xx.dtsi
@@ -329,8 +329,21 @@
status = "okay";
};

+&wdog1 {
+ ext-reset;
+};
+
&iomuxc {
+ pinctrl-names = "default";
+ pinctrl-0 = <&pinctrl_hog>;
+
imx6qdl-gw53xx {
+ pinctrl_hog: hoggrp {
+ fsl,pins = <
+ MX6QDL_PAD_DISP0_DAT8__WDOG1_B 0x1b0b0
+ >;
+ };
+
pinctrl_audmux: audmuxgrp {
fsl,pins = <
MX6QDL_PAD_SD2_DAT0__AUD4_RXD 0x130b0
diff --git a/arch/arm/boot/dts/imx6qdl-gw54xx.dtsi b/arch/arm/boot/dts/imx6qdl-gw54xx.dtsi
index 4a8d97f..f108781 100644
--- a/arch/arm/boot/dts/imx6qdl-gw54xx.dtsi
+++ b/arch/arm/boot/dts/imx6qdl-gw54xx.dtsi
@@ -422,8 +422,26 @@
status = "okay";
};

+&wdog1 {
+ status = "disabled";
+};
+
+&wdog2 {
+ status = "okay";
+ ext-reset;
+};
+
&iomuxc {
+ pinctrl-names = "default";
+ pinctrl-0 = <&pinctrl_hog>;
+
imx6qdl-gw54xx {
+ pinctrl_hog: hoggrp {
+ fsl,pins = <
+ MX6QDL_PAD_SD1_DAT3__WDOG2_B 0x1b0b0
+ >;
+ };
+
pinctrl_audmux: audmuxgrp {
fsl,pins = <
MX6QDL_PAD_SD2_DAT0__AUD4_RXD 0x130b0
diff --git a/arch/arm/boot/dts/imx6qdl-gw551x.dtsi b/arch/arm/boot/dts/imx6qdl-gw551x.dtsi
index d1866a0..ae3c2a4 100644
--- a/arch/arm/boot/dts/imx6qdl-gw551x.dtsi
+++ b/arch/arm/boot/dts/imx6qdl-gw551x.dtsi
@@ -227,8 +227,20 @@
status = "okay";
};

+&wdog1 {
+ ext-reset;
+};
+
&iomuxc {
+ pinctrl-names = "default";
+ pinctrl-0 = <&pinctrl_hog>;
imx6qdl-gw51xx {
+ pinctrl_hog: hoggrp {
+ fsl,pins = <
+ MX6QDL_PAD_DISP0_DAT8__WDOG1_B 0x1b0b0
+ >;
+ };
+
pinctrl_flexcan1: flexcan1grp {
fsl,pins = <
MX6QDL_PAD_KEY_ROW2__FLEXCAN1_RX 0x1b0b1
diff --git a/arch/arm/boot/dts/imx6qdl-gw552x.dtsi b/arch/arm/boot/dts/imx6qdl-gw552x.dtsi
index 5c6587f..52eaf42 100644
--- a/arch/arm/boot/dts/imx6qdl-gw552x.dtsi
+++ b/arch/arm/boot/dts/imx6qdl-gw552x.dtsi
@@ -185,8 +185,21 @@
status = "okay";
};

+&wdog1 {
+ ext-reset;
+};
+
&iomuxc {
+ pinctrl-names = "default";
+ pinctrl-0 = <&pinctrl_hog>;
+
imx6qdl-gw552x {
+ pinctrl_hog: hoggrp {
+ fsl,pins = <
+ MX6QDL_PAD_DISP0_DAT8__WDOG1_B 0x1b0b0
+ >;
+ };
+
pinctrl_gpio_leds: gpioledsgrp {
fsl,pins = <
MX6QDL_PAD_KEY_COL0__GPIO4_IO06 0x1b0b0

--
1.9.1

Markus Pargmann

2015-05-29 06:17:57 UTC

Permalink

Hi,

Post by Tim Harvey
The Gateworks Ventana boards have a PMIC that can be used to regulate the
CPU voltage rails for DVFS support. In order to ensure this PMIC is properly
reset the watchdog needs to be configured to assert its external reset
signal.
Additionally the pad used for WDOG_B needs to be configured which we add to
a hog group for lack of a better location.

Why don't you add this as pinctrl of the watchdog device node? It won't
have any influence anyway until the watchdog driver was setup.

Regards,

Markus

Post by Tim Harvey
---
arch/arm/boot/dts/imx6qdl-gw51xx.dtsi | 13 +++++++++++++
arch/arm/boot/dts/imx6qdl-gw52xx.dtsi | 13 +++++++++++++
arch/arm/boot/dts/imx6qdl-gw53xx.dtsi | 13 +++++++++++++
arch/arm/boot/dts/imx6qdl-gw54xx.dtsi | 18 ++++++++++++++++++
arch/arm/boot/dts/imx6qdl-gw551x.dtsi | 12 ++++++++++++
arch/arm/boot/dts/imx6qdl-gw552x.dtsi | 13 +++++++++++++
6 files changed, 82 insertions(+)
diff --git a/arch/arm/boot/dts/imx6qdl-gw51xx.dtsi b/arch/arm/boot/dts/imx6qdl-gw51xx.dtsi
index f2867c4..740c6c4 100644
--- a/arch/arm/boot/dts/imx6qdl-gw51xx.dtsi
+++ b/arch/arm/boot/dts/imx6qdl-gw51xx.dtsi
@@ -210,8 +210,21 @@
status = "okay";
};
+&wdog1 {
+ ext-reset;
+};
+
&iomuxc {
+ pinctrl-names = "default";
+ pinctrl-0 = <&pinctrl_hog>;
+
imx6qdl-gw51xx {
+ pinctrl_hog: hoggrp {
+ fsl,pins = <
+ MX6QDL_PAD_DISP0_DAT8__WDOG1_B 0x1b0b0
+ >;
+ };
+
pinctrl_enet: enetgrp {
fsl,pins = <
MX6QDL_PAD_RGMII_RXC__RGMII_RXC 0x1b0b0
diff --git a/arch/arm/boot/dts/imx6qdl-gw52xx.dtsi b/arch/arm/boot/dts/imx6qdl-gw52xx.dtsi
index b5756c2..1b06f18 100644
--- a/arch/arm/boot/dts/imx6qdl-gw52xx.dtsi
+++ b/arch/arm/boot/dts/imx6qdl-gw52xx.dtsi
@@ -323,8 +323,21 @@
status = "okay";
};
+&wdog1 {
+ ext-reset;
+};
+
&iomuxc {
+ pinctrl-names = "default";
+ pinctrl-0 = <&pinctrl_hog>;
+
imx6qdl-gw52xx {
+ pinctrl_hog: hoggrp {
+ fsl,pins = <
+ MX6QDL_PAD_DISP0_DAT8__WDOG1_B 0x1b0b0
+ >;
+ };
+
pinctrl_audmux: audmuxgrp {
fsl,pins = <
MX6QDL_PAD_SD2_DAT0__AUD4_RXD 0x130b0
diff --git a/arch/arm/boot/dts/imx6qdl-gw53xx.dtsi b/arch/arm/boot/dts/imx6qdl-gw53xx.dtsi
index 86f03c1..65e5c01 100644
--- a/arch/arm/boot/dts/imx6qdl-gw53xx.dtsi
+++ b/arch/arm/boot/dts/imx6qdl-gw53xx.dtsi
@@ -329,8 +329,21 @@
status = "okay";
};
+&wdog1 {
+ ext-reset;
+};
+
&iomuxc {
+ pinctrl-names = "default";
+ pinctrl-0 = <&pinctrl_hog>;
+
imx6qdl-gw53xx {
+ pinctrl_hog: hoggrp {
+ fsl,pins = <
+ MX6QDL_PAD_DISP0_DAT8__WDOG1_B 0x1b0b0
+ >;
+ };
+
pinctrl_audmux: audmuxgrp {
fsl,pins = <
MX6QDL_PAD_SD2_DAT0__AUD4_RXD 0x130b0
diff --git a/arch/arm/boot/dts/imx6qdl-gw54xx.dtsi b/arch/arm/boot/dts/imx6qdl-gw54xx.dtsi
index 4a8d97f..f108781 100644
--- a/arch/arm/boot/dts/imx6qdl-gw54xx.dtsi
+++ b/arch/arm/boot/dts/imx6qdl-gw54xx.dtsi
@@ -422,8 +422,26 @@
status = "okay";
};
+&wdog1 {
+ status = "disabled";
+};
+
+&wdog2 {
+ status = "okay";
+ ext-reset;
+};
+
&iomuxc {
+ pinctrl-names = "default";
+ pinctrl-0 = <&pinctrl_hog>;
+
imx6qdl-gw54xx {
+ pinctrl_hog: hoggrp {
+ fsl,pins = <
+ MX6QDL_PAD_SD1_DAT3__WDOG2_B 0x1b0b0
+ >;
+ };
+
pinctrl_audmux: audmuxgrp {
fsl,pins = <
MX6QDL_PAD_SD2_DAT0__AUD4_RXD 0x130b0
diff --git a/arch/arm/boot/dts/imx6qdl-gw551x.dtsi b/arch/arm/boot/dts/imx6qdl-gw551x.dtsi
index d1866a0..ae3c2a4 100644
--- a/arch/arm/boot/dts/imx6qdl-gw551x.dtsi
+++ b/arch/arm/boot/dts/imx6qdl-gw551x.dtsi
@@ -227,8 +227,20 @@
status = "okay";
};
+&wdog1 {
+ ext-reset;
+};
+
&iomuxc {
+ pinctrl-names = "default";
+ pinctrl-0 = <&pinctrl_hog>;
imx6qdl-gw51xx {
+ pinctrl_hog: hoggrp {
+ fsl,pins = <
+ MX6QDL_PAD_DISP0_DAT8__WDOG1_B 0x1b0b0
+ >;
+ };
+
pinctrl_flexcan1: flexcan1grp {
fsl,pins = <
MX6QDL_PAD_KEY_ROW2__FLEXCAN1_RX 0x1b0b1
diff --git a/arch/arm/boot/dts/imx6qdl-gw552x.dtsi b/arch/arm/boot/dts/imx6qdl-gw552x.dtsi
index 5c6587f..52eaf42 100644
--- a/arch/arm/boot/dts/imx6qdl-gw552x.dtsi
+++ b/arch/arm/boot/dts/imx6qdl-gw552x.dtsi
@@ -185,8 +185,21 @@
status = "okay";
};
+&wdog1 {
+ ext-reset;
+};
+
&iomuxc {
+ pinctrl-names = "default";
+ pinctrl-0 = <&pinctrl_hog>;
+
imx6qdl-gw552x {
+ pinctrl_hog: hoggrp {
+ fsl,pins = <
+ MX6QDL_PAD_DISP0_DAT8__WDOG1_B 0x1b0b0
+ >;
+ };
+
pinctrl_gpio_leds: gpioledsgrp {
fsl,pins = <
MX6QDL_PAD_KEY_COL0__GPIO4_IO06 0x1b0b0
--
1.9.1
_______________________________________________
linux-arm-kernel mailing list
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

Tim Harvey

2015-05-29 14:01:00 UTC

Permalink

Post by Markus Pargmann
Hi,

Why don't you add this as pinctrl of the watchdog device node? It won't
have any influence anyway until the watchdog driver was setup.
Regards,
Markus

Markus,

That makes great sense:

&wdog1 {
pinctrl-names = "default";
pinctrl-0 = <&pinctrl_wdog>;
ext-reset;
};

&iomuxc {
imx6qdl-gw51xx {
pinctrl_wdog: wdoggrp {
fsl,pins = <
MX6QDL_PAD_DISP0_DAT8__WDOG1_B 0x1b0b0

Post by Markus Pargmann
;

};
....

instead of

&wdog1 {
ext-reset;
};

&iomuxc {
pinctrl-names = "default";
pinctrl-0 = <&pinctrl_hog>;

imx6qdl-gw51xx {
pinctrl_hog: hoggrp {
fsl,pins = <
MX6QDL_PAD_DISP0_DAT8__WDOG1_B 0x1b0b0

Post by Markus Pargmann
;

};
....

I'll do that in a future revision after feedback.

Thanks,

Tim

Tim Harvey

2015-06-04 20:15:58 UTC

Permalink

--
1.9.1

Tim Harvey

2015-06-04 20:15:59 UTC

Permalink

The IMX6 watchdog supports assertion of a signal (WDOG_B) which
can be pinmux'd to an external pin. This is typically used for boards that
have PMIC's in control of the IMX6 power rails. In fact, failure to use
such an external reset on boards with external PMIC's can result in various
hangs due to the IMX6 not being fully reset [1] as well as the board failing
to reset because its PMIC has not been reset to provide adequate voltage for
the CPU when coming out of reset at 800Mhz.

This uses a new device-tree property 'ext-reset-output' to indicate the
board has such a reset and to cause the watchdog to be configured to assert
WDOG_B instead of an internal reset both on a watchdog timeout and in
system_restart.

[1] http://lists.infradead.org/pipermail/linux-arm-kernel/2015-March/333689.html

Cc: Lucas Stach <***@pengutronix.de>
Signed-off-by: Tim Harvey <***@gateworks.com>
---
v2:
- rename property to 'ext-reset-output' based on ML feedback
- simplify setting SRS bit if external-reset
- update comments and commit msg

Signed-off-by: Tim Harvey <***@gateworks.com>
---
.../devicetree/bindings/watchdog/fsl-imx-wdt.txt | 2 ++
drivers/watchdog/imx2_wdt.c | 17 +++++++++++++++--
2 files changed, 17 insertions(+), 2 deletions(-)

diff --git a/Documentation/devicetree/bindings/watchdog/fsl-imx-wdt.txt b/Documentation/devicetree/bindings/watchdog/fsl-imx-wdt.txt
index 8dab6fd..9b89b3a 100644
--- a/Documentation/devicetree/bindings/watchdog/fsl-imx-wdt.txt
+++ b/Documentation/devicetree/bindings/watchdog/fsl-imx-wdt.txt
@@ -9,6 +9,8 @@ Optional property:
- big-endian: If present the watchdog device's registers are implemented
in big endian mode, otherwise in native mode(same with CPU), for more
detail please see: Documentation/devicetree/bindings/regmap/regmap.txt.
+- ext-reset-output: If present the watchdog device is configured to assert its
+ external reset (WDOG_B) instead of issuing a software reset.

Examples:

diff --git a/drivers/watchdog/imx2_wdt.c b/drivers/watchdog/imx2_wdt.c
index 5e6d808..7b8eeaf 100644
--- a/drivers/watchdog/imx2_wdt.c
+++ b/drivers/watchdog/imx2_wdt.c
@@ -41,6 +41,7 @@

#define IMX2_WDT_WCR 0x00 /* Control Register */
#define IMX2_WDT_WCR_WT (0xFF << 8) /* -> Watchdog Timeout Field */
+#define IMX2_WDT_WCR_SRS (1 << 4) /* -> Software Reset Signal */
#define IMX2_WDT_WCR_WRE (1 << 3) /* -> WDOG Reset Enable */
#define IMX2_WDT_WCR_WDE (1 << 2) /* -> Watchdog Enable */
#define IMX2_WDT_WCR_WDZST (1 << 0) /* -> Watchdog timer Suspend */
@@ -65,6 +66,7 @@ struct imx2_wdt_device {
struct timer_list timer; /* Pings the watchdog when closed */
struct watchdog_device wdog;
struct notifier_block restart_handler;
+ bool ext_reset;
};

static bool nowayout = WATCHDOG_NOWAYOUT;
@@ -90,6 +92,11 @@ static int imx2_restart_handler(struct notifier_block *this, unsigned long mode,
struct imx2_wdt_device *wdev = container_of(this,
struct imx2_wdt_device,
restart_handler);
+
+ /* Use external reset */
+ if (wdev->ext_reset)
+ wcr_enable |= IMX2_WDT_WCR_SRS;
+
/* Assert SRS signal */
regmap_write(wdev->regmap, 0, wcr_enable);
/*
@@ -119,8 +126,12 @@ static inline void imx2_wdt_setup(struct watchdog_device *wdog)
val |= IMX2_WDT_WCR_WDZST;
/* Strip the old watchdog Time-Out value */
val &= ~IMX2_WDT_WCR_WT;
- /* Generate reset if WDOG times out */
- val &= ~IMX2_WDT_WCR_WRE;
+ /* Generate internal chip-level reset if WDOG times out */
+ if (!wdev->ext_reset)
+ val &= ~IMX2_WDT_WCR_WRE;
+ /* Or if external-reset assert WDOG_B reset only on time-out */
+ else
+ val |= IMX2_WDT_WCR_WRE;
/* Keep Watchdog Disabled */
val &= ~IMX2_WDT_WCR_WDE;
/* Set the watchdog's Time-Out value */
@@ -262,6 +273,8 @@ static int __init imx2_wdt_probe(struct platform_device *pdev)
regmap_read(wdev->regmap, IMX2_WDT_WRSR, &val);
wdog->bootstatus = val & IMX2_WDT_WRSR_TOUT ? WDIOF_CARDRESET : 0;

+ wdev->ext_reset = of_property_read_bool(pdev->dev.of_node,
+ "ext-reset-output");
wdog->timeout = clamp_t(unsigned, timeout, 1, IMX2_WDT_MAX_TIME);
if (wdog->timeout != timeout)
dev_warn(&pdev->dev, "Initial timeout out of range! Clamped from %u to %u\n",

--
1.9.1

Zhi Li

2015-06-05 16:38:31 UTC

Permalink

+ /* Generate internal chip-level reset if WDOG times out */
+ if (!wdev->ext_reset)
+ val &= ~IMX2_WDT_WCR_WRE;
+ /* Or if external-reset assert WDOG_B reset only on time-out */
+ else
+ val |= IMX2_WDT_WCR_WRE;

I think you can always enable IMX2_WDT_WCR_WRE.
So needn't ext_reset.

If you don't config WDOG_B pin mux, WDOG_B do nothing.

best regards
Frank Li

Tim Harvey

2015-06-25 23:22:22 UTC

Permalink

Post by Zhi Li

I think you can always enable IMX2_WDT_WCR_WRE.
So needn't ext_reset.
If you don't config WDOG_B pin mux, WDOG_B do nothing.
best regards
Frank Li

Frank,

No - If you generate both an IMX6 internal reset and an external
reset, the external reset signal is released immediately after the
IMX6 goes into reset which may be too quick for whatever hardware you
have that routed to.

Tim

Lucas Stach

2015-06-22 09:22:12 UTC

Permalink

Post by Tim Harvey
The IMX6 watchdog supports assertion of a signal (WDOG_B) which
can be pinmux'd to an external pin. This is typically used for boards that
have PMIC's in control of the IMX6 power rails. In fact, failure to use
such an external reset on boards with external PMIC's can result in various
hangs due to the IMX6 not being fully reset [1] as well as the board failing
to reset because its PMIC has not been reset to provide adequate voltage for
the CPU when coming out of reset at 800Mhz.
This uses a new device-tree property 'ext-reset-output' to indicate the
board has such a reset and to cause the watchdog to be configured to assert
WDOG_B instead of an internal reset both on a watchdog timeout and in
system_restart.
[1] http://lists.infradead.org/pipermail/linux-arm-kernel/2015-March/333689.html
---
- rename property to 'ext-reset-output' based on ML feedback
- simplify setting SRS bit if external-reset
- update comments and commit msg
---
.../devicetree/bindings/watchdog/fsl-imx-wdt.txt | 2 ++
drivers/watchdog/imx2_wdt.c | 17 +++++++++++++++--
2 files changed, 17 insertions(+), 2 deletions(-)
diff --git a/Documentation/devicetree/bindings/watchdog/fsl-imx-wdt.txt b/Documentation/devicetree/bindings/watchdog/fsl-imx-wdt.txt
index 8dab6fd..9b89b3a 100644
--- a/Documentation/devicetree/bindings/watchdog/fsl-imx-wdt.txt
+++ b/Documentation/devicetree/bindings/watchdog/fsl-imx-wdt.txt
- big-endian: If present the watchdog device's registers are implemented
in big endian mode, otherwise in native mode(same with CPU), for more
detail please see: Documentation/devicetree/bindings/regmap/regmap.txt.
+- ext-reset-output: If present the watchdog device is configured to assert its
+ external reset (WDOG_B) instead of issuing a software reset.
diff --git a/drivers/watchdog/imx2_wdt.c b/drivers/watchdog/imx2_wdt.c
index 5e6d808..7b8eeaf 100644
--- a/drivers/watchdog/imx2_wdt.c
+++ b/drivers/watchdog/imx2_wdt.c
@@ -41,6 +41,7 @@
#define IMX2_WDT_WCR 0x00 /* Control Register */
#define IMX2_WDT_WCR_WT (0xFF << 8) /* -> Watchdog Timeout Field */
+#define IMX2_WDT_WCR_SRS (1 << 4) /* -> Software Reset Signal */
#define IMX2_WDT_WCR_WRE (1 << 3) /* -> WDOG Reset Enable */
#define IMX2_WDT_WCR_WDE (1 << 2) /* -> Watchdog Enable */
#define IMX2_WDT_WCR_WDZST (1 << 0) /* -> Watchdog timer Suspend */
@@ -65,6 +66,7 @@ struct imx2_wdt_device {
struct timer_list timer; /* Pings the watchdog when closed */
struct watchdog_device wdog;
struct notifier_block restart_handler;
+ bool ext_reset;
};
static bool nowayout = WATCHDOG_NOWAYOUT;
@@ -90,6 +92,11 @@ static int imx2_restart_handler(struct notifier_block *this, unsigned long mode,
struct imx2_wdt_device *wdev = container_of(this,
struct imx2_wdt_device,
restart_handler);
+
+ /* Use external reset */
+ if (wdev->ext_reset)
+ wcr_enable |= IMX2_WDT_WCR_SRS;
+
/* Assert SRS signal */
regmap_write(wdev->regmap, 0, wcr_enable);
/*
@@ -119,8 +126,12 @@ static inline void imx2_wdt_setup(struct watchdog_device *wdog)
val |= IMX2_WDT_WCR_WDZST;
/* Strip the old watchdog Time-Out value */
val &= ~IMX2_WDT_WCR_WT;
- /* Generate reset if WDOG times out */
- val &= ~IMX2_WDT_WCR_WRE;
+ /* Generate internal chip-level reset if WDOG times out */
+ if (!wdev->ext_reset)
+ val &= ~IMX2_WDT_WCR_WRE;
+ /* Or if external-reset assert WDOG_B reset only on time-out */
+ else
+ val |= IMX2_WDT_WCR_WRE;
/* Keep Watchdog Disabled */
val &= ~IMX2_WDT_WCR_WDE;
/* Set the watchdog's Time-Out value */
@@ -262,6 +273,8 @@ static int __init imx2_wdt_probe(struct platform_device *pdev)
regmap_read(wdev->regmap, IMX2_WDT_WRSR, &val);
wdog->bootstatus = val & IMX2_WDT_WRSR_TOUT ? WDIOF_CARDRESET : 0;
+ wdev->ext_reset = of_property_read_bool(pdev->dev.of_node,
+ "ext-reset-output");
wdog->timeout = clamp_t(unsigned, timeout, 1, IMX2_WDT_MAX_TIME);
if (wdog->timeout != timeout)
dev_warn(&pdev->dev, "Initial timeout out of range! Clamped from %u to %u\n",

--
Pengutronix e.K. | Lucas Stach |
Industrial Linux Solutions | http://www.pengutronix.de/ |

Shawn Guo

2015-06-28 15:07:42 UTC

Permalink

This is a vendor specific binding property rather than generic one, so
it should have a vendor prefix, i.e. "fsl,". Other than that,

Acked-by: Shawn Guo <***@linaro.org>

Shawn Guo

2015-06-29 01:17:37 UTC

Permalink

Post by Tim Harvey
@@ -90,6 +92,11 @@ static int imx2_restart_handler(struct notifier_block *this, unsigned long mode,
struct imx2_wdt_device *wdev = container_of(this,
struct imx2_wdt_device,
restart_handler);
+
+ /* Use external reset */
+ if (wdev->ext_reset)
+ wcr_enable |= IMX2_WDT_WCR_SRS;
+

The existing code simply writes the register with only WDE (Watchdog
Enable) bit set. That said, bit WDA and SRS are cleared at the same
time. What's the result of that? Both internal reset and external
reset will be asserted? If this is the case, it's unsafe per your reply
to Frank's comment, right? So we should only clear one bit between
these two based on ext_reset flag to avoid asserting both internal and
external reset?

Shawn

Post by Tim Harvey
/* Assert SRS signal */
regmap_write(wdev->regmap, 0, wcr_enable);
/*

Tim Harvey

2015-07-02 16:49:20 UTC

Permalink

Post by Shawn Guo

Shawn,

Correct.

Are you saying that you think we should change the above to something
like the following to ensure in the internal reset case external reset
is not also being asserted?:

+ /* Use external reset */
+ if (wdev->ext_reset)
+ wcr_enable |= IMX2_WDT_WCR_SRS; /* do not assert
internal system reset */
+ else
+ wcr_enable |= IMX2_WDT_WCR_WDA; /* do not assert external reset */

I was not worried about the else case above because if you hit that
path your device-tree is saying you are not using external reset
anyway.

Tim

Post by Shawn Guo

Post by Tim Harvey
/* Assert SRS signal */
regmap_write(wdev->regmap, 0, wcr_enable);
/*

Shawn Guo

2015-07-03 01:02:31 UTC

Permalink

Post by Tim Harvey

Post by Shawn Guo

Shawn,
Correct.
Are you saying that you think we should change the above to something
like the following to ensure in the internal reset case external reset

Yes, that's what I'm saying.

Post by Tim Harvey
+ /* Use external reset */
+ if (wdev->ext_reset)
+ wcr_enable |= IMX2_WDT_WCR_SRS; /* do not assert
internal system reset */
+ else
+ wcr_enable |= IMX2_WDT_WCR_WDA; /* do not assert external reset */
I was not worried about the else case above because if you hit that
path your device-tree is saying you are not using external reset
anyway.

It's a safeguard for platforms which have WDOG_B signal routed to some
hardware, while their device trees haven't specified ext_reset_output
property.

Shawn

Tim Harvey

2015-07-28 15:27:08 UTC

Permalink

--
1.9.1

Tim Harvey

2015-07-28 15:27:09 UTC

Permalink

The IMX6 watchdog supports assertion of a signal (WDOG_B) which
can be pinmux'd to an external pin. This is typically used for boards that
have PMIC's in control of the IMX6 power rails. In fact, failure to use
such an external reset on boards with external PMIC's can result in various
hangs due to the IMX6 not being fully reset [1] as well as the board failing
to reset because its PMIC has not been reset to provide adequate voltage for
the CPU when coming out of reset at 800Mhz.

This uses a new device-tree property 'ext-reset-output' to indicate the
board has such a reset and to cause the watchdog to be configured to assert
WDOG_B instead of an internal reset both on a watchdog timeout and in
system_restart.

[1] http://lists.infradead.org/pipermail/linux-arm-kernel/2015-March/333689.html

Cc: Lucas Stach <***@pengutronix.de>
Signed-off-by: Tim Harvey <***@gateworks.com>
---
v3:
- mandate use of 'either' internal or external reset but not both
simultaneously
v2:
- rename property to 'ext-reset-output' based on ML feedback
- simplify setting SRS bit if external-reset
- update comments and commit msg

Signed-off-by: Tim Harvey <***@gateworks.com>
---
.../devicetree/bindings/watchdog/fsl-imx-wdt.txt | 2 ++
drivers/watchdog/imx2_wdt.c | 20 ++++++++++++++++++--
2 files changed, 20 insertions(+), 2 deletions(-)

diff --git a/Documentation/devicetree/bindings/watchdog/fsl-imx-wdt.txt b/Documentation/devicetree/bindings/watchdog/fsl-imx-wdt.txt
index 8dab6fd..9b89b3a 100644
--- a/Documentation/devicetree/bindings/watchdog/fsl-imx-wdt.txt
+++ b/Documentation/devicetree/bindings/watchdog/fsl-imx-wdt.txt
@@ -9,6 +9,8 @@ Optional property:
- big-endian: If present the watchdog device's registers are implemented
in big endian mode, otherwise in native mode(same with CPU), for more
detail please see: Documentation/devicetree/bindings/regmap/regmap.txt.
+- ext-reset-output: If present the watchdog device is configured to assert its
+ external reset (WDOG_B) instead of issuing a software reset.

Examples:

diff --git a/drivers/watchdog/imx2_wdt.c b/drivers/watchdog/imx2_wdt.c
index 0bb1a1d..5d9ccf6 100644
--- a/drivers/watchdog/imx2_wdt.c
+++ b/drivers/watchdog/imx2_wdt.c
@@ -41,6 +41,8 @@

#define IMX2_WDT_WCR 0x00 /* Control Register */
#define IMX2_WDT_WCR_WT (0xFF << 8) /* -> Watchdog Timeout Field */
+#define IMX2_WDT_WCR_WDA (1 << 5) /* -> External Reset WDOG_B */
+#define IMX2_WDT_WCR_SRS (1 << 4) /* -> Software Reset Signal */
#define IMX2_WDT_WCR_WRE (1 << 3) /* -> WDOG Reset Enable */
#define IMX2_WDT_WCR_WDE (1 << 2) /* -> Watchdog Enable */
#define IMX2_WDT_WCR_WDZST (1 << 0) /* -> Watchdog timer Suspend */
@@ -65,6 +67,7 @@ struct imx2_wdt_device {
struct timer_list timer; /* Pings the watchdog when closed */
struct watchdog_device wdog;
struct notifier_block restart_handler;
+ bool ext_reset;
};

static bool nowayout = WATCHDOG_NOWAYOUT;
@@ -90,6 +93,13 @@ static int imx2_restart_handler(struct notifier_block *this, unsigned long mode,
struct imx2_wdt_device *wdev = container_of(this,
struct imx2_wdt_device,
restart_handler);
+
+ /* Use internal reset or external - not both */
+ if (wdev->ext_reset)
+ wcr_enable |= IMX2_WDT_WCR_SRS; /* do not assert int reset */
+ else
+ wcr_enable |= IMX2_WDT_WCR_WDA; /* do not assert ext-reset */
+
/* Assert SRS signal */
regmap_write(wdev->regmap, 0, wcr_enable);
/*
@@ -119,8 +129,12 @@ static inline void imx2_wdt_setup(struct watchdog_device *wdog)
val |= IMX2_WDT_WCR_WDZST;
/* Strip the old watchdog Time-Out value */
val &= ~IMX2_WDT_WCR_WT;
- /* Generate reset if WDOG times out */
- val &= ~IMX2_WDT_WCR_WRE;
+ /* Generate internal chip-level reset if WDOG times out */
+ if (!wdev->ext_reset)
+ val &= ~IMX2_WDT_WCR_WRE;
+ /* Or if external-reset assert WDOG_B reset only on time-out */
+ else
+ val |= IMX2_WDT_WCR_WRE;
/* Keep Watchdog Disabled */
val &= ~IMX2_WDT_WCR_WDE;
/* Set the watchdog's Time-Out value */
@@ -267,6 +281,8 @@ static int __init imx2_wdt_probe(struct platform_device *pdev)
regmap_read(wdev->regmap, IMX2_WDT_WRSR, &val);
wdog->bootstatus = val & IMX2_WDT_WRSR_TOUT ? WDIOF_CARDRESET : 0;

+ wdev->ext_reset = of_property_read_bool(pdev->dev.of_node,
+ "ext-reset-output");
wdog->timeout = clamp_t(unsigned, timeout, 1, IMX2_WDT_MAX_TIME);
if (wdog->timeout != timeout)
dev_warn(&pdev->dev, "Initial timeout out of range! Clamped from %u to %u\n",

--
1.9.1

Shawn Guo

2015-08-05 13:01:16 UTC

Permalink

Acked-by: Shawn Guo <***@kernel.org>

Tim Harvey

2015-10-13 14:11:58 UTC

Permalink

Post by Tim Harvey
The IMX6 watchdog supports assertion of a signal (WDOG_B) which
can be pinmux'd to an external pin. This is typically used for boards that
have PMIC's in control of the IMX6 power rails. In fact, failure to use
such an external reset on boards with external PMIC's can result in various
hangs due to the IMX6 not being fully reset [1] as well as the board failing
to reset because its PMIC has not been reset to provide adequate voltage for
the CPU when coming out of reset at 800Mhz.
This uses a new device-tree property 'ext-reset-output' to indicate the
board has such a reset and to cause the watchdog to be configured to assert
WDOG_B instead of an internal reset both on a watchdog timeout and in
system_restart.
[1] http://lists.infradead.org/pipermail/linux-arm-kernel/2015-March/333689.html
---
- mandate use of 'either' internal or external reset but not both
simultaneously
- rename property to 'ext-reset-output' based on ML feedback
- simplify setting SRS bit if external-reset
- update comments and commit msg
---
.../devicetree/bindings/watchdog/fsl-imx-wdt.txt | 2 ++
drivers/watchdog/imx2_wdt.c | 20 ++++++++++++++++++--
2 files changed, 20 insertions(+), 2 deletions(-)
diff --git a/Documentation/devicetree/bindings/watchdog/fsl-imx-wdt.txt b/Documentation/devicetree/bindings/watchdog/fsl-imx-wdt.txt
index 8dab6fd..9b89b3a 100644
--- a/Documentation/devicetree/bindings/watchdog/fsl-imx-wdt.txt
+++ b/Documentation/devicetree/bindings/watchdog/fsl-imx-wdt.txt
- big-endian: If present the watchdog device's registers are implemented
in big endian mode, otherwise in native mode(same with CPU), for more
detail please see: Documentation/devicetree/bindings/regmap/regmap.txt.
+- ext-reset-output: If present the watchdog device is configured to assert its
+ external reset (WDOG_B) instead of issuing a software reset.
diff --git a/drivers/watchdog/imx2_wdt.c b/drivers/watchdog/imx2_wdt.c
index 0bb1a1d..5d9ccf6 100644
--- a/drivers/watchdog/imx2_wdt.c
+++ b/drivers/watchdog/imx2_wdt.c
@@ -41,6 +41,8 @@
#define IMX2_WDT_WCR 0x00 /* Control Register */
#define IMX2_WDT_WCR_WT (0xFF << 8) /* -> Watchdog Timeout Field */
+#define IMX2_WDT_WCR_WDA (1 << 5) /* -> External Reset WDOG_B */
+#define IMX2_WDT_WCR_SRS (1 << 4) /* -> Software Reset Signal */
#define IMX2_WDT_WCR_WRE (1 << 3) /* -> WDOG Reset Enable */
#define IMX2_WDT_WCR_WDE (1 << 2) /* -> Watchdog Enable */
#define IMX2_WDT_WCR_WDZST (1 << 0) /* -> Watchdog timer Suspend */
@@ -65,6 +67,7 @@ struct imx2_wdt_device {
struct timer_list timer; /* Pings the watchdog when closed */
struct watchdog_device wdog;
struct notifier_block restart_handler;
+ bool ext_reset;
};
static bool nowayout = WATCHDOG_NOWAYOUT;
@@ -90,6 +93,13 @@ static int imx2_restart_handler(struct notifier_block *this, unsigned long mode,
struct imx2_wdt_device *wdev = container_of(this,
struct imx2_wdt_device,
restart_handler);
+
+ /* Use internal reset or external - not both */
+ if (wdev->ext_reset)
+ wcr_enable |= IMX2_WDT_WCR_SRS; /* do not assert int reset */
+ else
+ wcr_enable |= IMX2_WDT_WCR_WDA; /* do not assert ext-reset */
+
/* Assert SRS signal */
regmap_write(wdev->regmap, 0, wcr_enable);
/*
@@ -119,8 +129,12 @@ static inline void imx2_wdt_setup(struct watchdog_device *wdog)
val |= IMX2_WDT_WCR_WDZST;
/* Strip the old watchdog Time-Out value */
val &= ~IMX2_WDT_WCR_WT;
- /* Generate reset if WDOG times out */
- val &= ~IMX2_WDT_WCR_WRE;
+ /* Generate internal chip-level reset if WDOG times out */
+ if (!wdev->ext_reset)
+ val &= ~IMX2_WDT_WCR_WRE;
+ /* Or if external-reset assert WDOG_B reset only on time-out */
+ else
+ val |= IMX2_WDT_WCR_WRE;
/* Keep Watchdog Disabled */
val &= ~IMX2_WDT_WCR_WDE;
/* Set the watchdog's Time-Out value */
@@ -267,6 +281,8 @@ static int __init imx2_wdt_probe(struct platform_device *pdev)
regmap_read(wdev->regmap, IMX2_WDT_WRSR, &val);
wdog->bootstatus = val & IMX2_WDT_WRSR_TOUT ? WDIOF_CARDRESET : 0;
+ wdev->ext_reset = of_property_read_bool(pdev->dev.of_node,
+ "ext-reset-output");
wdog->timeout = clamp_t(unsigned, timeout, 1, IMX2_WDT_MAX_TIME);
if (wdog->timeout != timeout)
dev_warn(&pdev->dev, "Initial timeout out of range! Clamped from %u to %u\n",
--
1.9.1

Wim,

I believe I missed sending this to the proper watchdog maintainer
which appears to be you.

Regards,

Tim

Tim Harvey

2015-07-28 15:27:10 UTC

Permalink

;

};
+
+ pinctrl_wdog: wdoggrp {
+ fsl,pins = <
+ MX6QDL_PAD_DISP0_DAT8__WDOG1_B 0x1b0b0
+ >;
+ };
};
};
diff --git a/arch/arm/boot/dts/imx6qdl-gw52xx.dtsi b/arch/arm/boot/dts/imx6qdl-gw52xx.dtsi
index 1b66328..0d8f201 100644
--- a/arch/arm/boot/dts/imx6qdl-gw52xx.dtsi
+++ b/arch/arm/boot/dts/imx6qdl-gw52xx.dtsi
@@ -326,6 +326,12 @@
status = "okay";
};

+&wdog1 {
+ pinctrl-names = "default";
+ pinctrl-0 = <&pinctrl_wdog>;
+ ext-reset-output;
+};
+
&iomuxc {
imx6qdl-gw52xx {
pinctrl_audmux: audmuxgrp {
@@ -501,5 +507,11 @@
MX6QDL_PAD_NANDF_CS1__SD3_VSELECT 0x170f9

;

};
+
+ pinctrl_wdog: wdoggrp {
+ fsl,pins = <
+ MX6QDL_PAD_DISP0_DAT8__WDOG1_B 0x1b0b0
+ >;
+ };
};
};
diff --git a/arch/arm/boot/dts/imx6qdl-gw53xx.dtsi b/arch/arm/boot/dts/imx6qdl-gw53xx.dtsi
index 7c51839..36f9ec6 100644
--- a/arch/arm/boot/dts/imx6qdl-gw53xx.dtsi
+++ b/arch/arm/boot/dts/imx6qdl-gw53xx.dtsi
@@ -332,7 +332,14 @@
status = "okay";
};

+&wdog1 {
+ pinctrl-names = "default";
+ pinctrl-0 = <&pinctrl_wdog>;
+ ext-reset-output;
+};
+
&iomuxc {
+
imx6qdl-gw53xx {
pinctrl_audmux: audmuxgrp {
fsl,pins = <
@@ -508,5 +515,11 @@
MX6QDL_PAD_NANDF_CS1__SD3_VSELECT 0x170f9

;

};
+
+ pinctrl_wdog: wdoggrp {
+ fsl,pins = <
+ MX6QDL_PAD_DISP0_DAT8__WDOG1_B 0x1b0b0
+ >;
+ };
};
};
diff --git a/arch/arm/boot/dts/imx6qdl-gw54xx.dtsi b/arch/arm/boot/dts/imx6qdl-gw54xx.dtsi
index 929e0b3..0c1150f 100644
--- a/arch/arm/boot/dts/imx6qdl-gw54xx.dtsi
+++ b/arch/arm/boot/dts/imx6qdl-gw54xx.dtsi
@@ -425,6 +425,17 @@
status = "okay";
};

+&wdog1 {
+ status = "disabled";
+};
+
+&wdog2 {
+ pinctrl-names = "default";
+ pinctrl-0 = <&pinctrl_wdog>;
+ ext-reset-output;
+ status = "okay";
+};
+
&iomuxc {
imx6qdl-gw54xx {
pinctrl_audmux: audmuxgrp {
@@ -600,5 +611,11 @@
MX6QDL_PAD_NANDF_CS1__SD3_VSELECT 0x170f9

;

};
+
+ pinctrl_wdog: wdoggrp {
+ fsl,pins = <
+ MX6QDL_PAD_SD1_DAT3__WDOG2_B 0x1b0b0
+ >;
+ };
};
};
diff --git a/arch/arm/boot/dts/imx6qdl-gw551x.dtsi b/arch/arm/boot/dts/imx6qdl-gw551x.dtsi
index 741f3d5..883e577 100644
--- a/arch/arm/boot/dts/imx6qdl-gw551x.dtsi
+++ b/arch/arm/boot/dts/imx6qdl-gw551x.dtsi
@@ -227,6 +227,12 @@
status = "okay";
};

+&wdog1 {
+ pinctrl-names = "default";
+ pinctrl-0 = <&pinctrl_wdog>;
+ ext-reset-output;
+};
+
&iomuxc {
imx6qdl-gw51xx {
pinctrl_flexcan1: flexcan1grp {
@@ -309,5 +315,11 @@
MX6QDL_PAD_GPIO_1__USB_OTG_ID 0x17059

;

};
+
+ pinctrl_wdog: wdoggrp {
+ fsl,pins = <
+ MX6QDL_PAD_DISP0_DAT8__WDOG1_B 0x1b0b0
+ >;
+ };
};
};
diff --git a/arch/arm/boot/dts/imx6qdl-gw552x.dtsi b/arch/arm/boot/dts/imx6qdl-gw552x.dtsi
index d1e5048..07674c2 100644
--- a/arch/arm/boot/dts/imx6qdl-gw552x.dtsi
+++ b/arch/arm/boot/dts/imx6qdl-gw552x.dtsi
@@ -185,6 +185,12 @@
status = "okay";
};

+&wdog1 {
+ pinctrl-names = "default";
+ pinctrl-0 = <&pinctrl_wdog>;
+ ext-reset-output;
+};
+
&iomuxc {
imx6qdl-gw552x {
pinctrl_gpio_leds: gpioledsgrp {
@@ -262,5 +268,11 @@
MX6QDL_PAD_KEY_ROW1__UART5_RX_DATA 0x1b0b1

;

};
+
+ pinctrl_wdog: wdoggrp {
+ fsl,pins = <
+ MX6QDL_PAD_DISP0_DAT8__WDOG1_B 0x1b0b0
+ >;
+ };
};
};

--
1.9.1

Akshay Bhat

2015-10-30 17:00:11 UTC

Permalink

Hi, was there a reason why this patch did not make it to the upstream
kernel? We would like to use this feature on a Advantech/GE board and I
can rebase the first patch onto the latest linux-next, resubmit if needed.

Thanks, Akshay

Post by Tim Harvey
The second patch adds the watchdog configuration and pinmux for Gateworks
Ventana boards.
I would expect that maintainers of other IMX6 boards that use PMIC's that
are resettable via WDOG_B to follow-up with similar device-tree patches.
I've attempted to Cc those individuals here.
[1] http://lists.infradead.org/pipermail/linux-arm-kernel/2015-March/333689.html
watchdog: imx2_wdt: add external reset support via 'ext-reset-output'
dt prop
ARM: dts: ventana: Add ext-reset support
.../devicetree/bindings/watchdog/fsl-imx-wdt.txt | 2 ++
arch/arm/boot/dts/imx6qdl-gw51xx.dtsi | 12 ++++++++++++
arch/arm/boot/dts/imx6qdl-gw52xx.dtsi | 12 ++++++++++++
arch/arm/boot/dts/imx6qdl-gw53xx.dtsi | 13 +++++++++++++
arch/arm/boot/dts/imx6qdl-gw54xx.dtsi | 17 +++++++++++++++++
arch/arm/boot/dts/imx6qdl-gw551x.dtsi | 12 ++++++++++++
arch/arm/boot/dts/imx6qdl-gw552x.dtsi | 12 ++++++++++++
drivers/watchdog/imx2_wdt.c | 20 ++++++++++++++++++--
8 files changed, 98 insertions(+), 2 deletions(-)

Tim Harvey

2015-10-30 17:31:24 UTC

Permalink

Post by Akshay Bhat

Hi, was there a reason why this patch did not make it to the upstream
kernel? We would like to use this feature on a Advantech/GE board and I can
rebase the first patch onto the latest linux-next, resubmit if needed.
Thanks, Akshay

Akshay,

I have no idea why it hasn't made it upstream yet. I realized a couple
of weeks ago that I never sent it to Wim who is listed as the
maintainer of watchdog device drivers or the linux-watchdog maillist
so I did so at that time and haven't heard anything yet.

The original patch was:
http://lists.infradead.org/pipermail/linux-arm-kernel/2015-May/347168.html
The 2nd version was:
http://lists.infradead.org/pipermail/linux-arm-kernel/2015-June/348761.html

It's likely that a rebase/resubmit (including the proper list) may
result in a better outcome.

Tim

Tim Harvey

2015-06-04 20:16:00 UTC

Permalink

;

};
+
+ pinctrl_wdog: wdoggrp {
+ fsl,pins = <
+ MX6QDL_PAD_DISP0_DAT8__WDOG1_B 0x1b0b0
+ >;
+ };
};
};
diff --git a/arch/arm/boot/dts/imx6qdl-gw52xx.dtsi b/arch/arm/boot/dts/imx6qdl-gw52xx.dtsi
index b5756c2..e4752a7 100644
--- a/arch/arm/boot/dts/imx6qdl-gw52xx.dtsi
+++ b/arch/arm/boot/dts/imx6qdl-gw52xx.dtsi
@@ -323,6 +323,12 @@
status = "okay";
};

+&wdog1 {
+ pinctrl-names = "default";
+ pinctrl-0 = <&pinctrl_wdog>;
+ ext-reset-output;
+};
+
&iomuxc {
imx6qdl-gw52xx {
pinctrl_audmux: audmuxgrp {
@@ -472,5 +478,11 @@
MX6QDL_PAD_SD3_DAT5__GPIO7_IO00 0x1b0b0 /* CD */

;

};
+
+ pinctrl_wdog: wdoggrp {
+ fsl,pins = <
+ MX6QDL_PAD_DISP0_DAT8__WDOG1_B 0x1b0b0
+ >;
+ };
};
};
diff --git a/arch/arm/boot/dts/imx6qdl-gw53xx.dtsi b/arch/arm/boot/dts/imx6qdl-gw53xx.dtsi
index 86f03c1..7061e75 100644
--- a/arch/arm/boot/dts/imx6qdl-gw53xx.dtsi
+++ b/arch/arm/boot/dts/imx6qdl-gw53xx.dtsi
@@ -329,7 +329,14 @@
status = "okay";
};

+&wdog1 {
+ pinctrl-names = "default";
+ pinctrl-0 = <&pinctrl_wdog>;
+ ext-reset-output;
+};
+
&iomuxc {
+
imx6qdl-gw53xx {
pinctrl_audmux: audmuxgrp {
fsl,pins = <
@@ -479,5 +486,11 @@
MX6QDL_PAD_SD3_DAT5__GPIO7_IO00 0x1b0b0 /* CD */

;

};
+
+ pinctrl_wdog: wdoggrp {
+ fsl,pins = <
+ MX6QDL_PAD_DISP0_DAT8__WDOG1_B 0x1b0b0
+ >;
+ };
};
};
diff --git a/arch/arm/boot/dts/imx6qdl-gw54xx.dtsi b/arch/arm/boot/dts/imx6qdl-gw54xx.dtsi
index 4a8d97f..98439e1 100644
--- a/arch/arm/boot/dts/imx6qdl-gw54xx.dtsi
+++ b/arch/arm/boot/dts/imx6qdl-gw54xx.dtsi
@@ -422,6 +422,17 @@
status = "okay";
};

+&wdog1 {
+ status = "disabled";
+};
+
+&wdog2 {
+ pinctrl-names = "default";
+ pinctrl-0 = <&pinctrl_wdog>;
+ ext-reset-output;
+ status = "okay";
+};
+
&iomuxc {
imx6qdl-gw54xx {
pinctrl_audmux: audmuxgrp {
@@ -570,5 +581,11 @@
MX6QDL_PAD_SD3_DAT3__SD3_DATA3 0x17059

;

};
+
+ pinctrl_wdog: wdoggrp {
+ fsl,pins = <
+ MX6QDL_PAD_SD1_DAT3__WDOG2_B 0x1b0b0
+ >;
+ };
};
};
diff --git a/arch/arm/boot/dts/imx6qdl-gw551x.dtsi b/arch/arm/boot/dts/imx6qdl-gw551x.dtsi
index d1866a0..0924b6b 100644
--- a/arch/arm/boot/dts/imx6qdl-gw551x.dtsi
+++ b/arch/arm/boot/dts/imx6qdl-gw551x.dtsi
@@ -227,6 +227,12 @@
status = "okay";
};

+&wdog1 {
+ pinctrl-names = "default";
+ pinctrl-0 = <&pinctrl_wdog>;
+ ext-reset-output;
+};
+
&iomuxc {
imx6qdl-gw51xx {
pinctrl_flexcan1: flexcan1grp {
@@ -310,5 +316,11 @@
MX6QDL_PAD_GPIO_1__USB_OTG_ID 0x17059

;

};
+
+ pinctrl_wdog: wdoggrp {
+ fsl,pins = <
+ MX6QDL_PAD_DISP0_DAT8__WDOG1_B 0x1b0b0
+ >;
+ };
};
};
diff --git a/arch/arm/boot/dts/imx6qdl-gw552x.dtsi b/arch/arm/boot/dts/imx6qdl-gw552x.dtsi
index 5c6587f..4ba859f 100644
--- a/arch/arm/boot/dts/imx6qdl-gw552x.dtsi
+++ b/arch/arm/boot/dts/imx6qdl-gw552x.dtsi
@@ -185,6 +185,12 @@
status = "okay";
};

+&wdog1 {
+ pinctrl-names = "default";
+ pinctrl-0 = <&pinctrl_wdog>;
+ ext-reset-output;
+};
+
&iomuxc {
imx6qdl-gw552x {
pinctrl_gpio_leds: gpioledsgrp {
@@ -263,5 +269,11 @@
MX6QDL_PAD_KEY_ROW1__UART5_RX_DATA 0x1b0b1

;

};
+
+ pinctrl_wdog: wdoggrp {
+ fsl,pins = <
+ MX6QDL_PAD_DISP0_DAT8__WDOG1_B 0x1b0b0
+ >;
+ };
};
};

--
1.9.1