imx6: Implement external watchdog reset
cover imx6: Implement external watchdog reset
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 when it was at 400Mhz prior to reset.
This adds a new device-tree property 'ext-reset' to fsl-imx-wdt in order 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.
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
Tim Harvey (2):
watchdog: imx2_wdt: add external reset support via 'ext-reset' dt
property
ARM: dts: ventana: Add ext-reset support
.../devicetree/bindings/watchdog/fsl-imx-wdt.txt | 2 ++ 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 +++++++++++++ drivers/watchdog/imx2_wdt.c | 18 ++++++++++++++++-- 8 files changed, 100 insertions(+), 2 deletions(-)
Signature · 1 line
1.9.1
PATCH 1/2 watchdog: imx2_wdt: add external reset support via 'ext-reset' dt property
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
--- .../devicetree/bindings/watchdog/fsl-imx-wdt.txt | 2 ++ drivers/watchdog/imx2_wdt.c | 18 ++++++++++++++++-- 2 files changed, 18 insertions(+), 2 deletions(-)
Documentation/devicetree/bindings/watchdog/fsl-imx-wdt.txt +2 -0
@@ -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:
drivers/watchdog/imx2_wdt.c +16 -2
@@ -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",
Signature · 1 line
1.9.1
PATCH 1/2 watchdog: imx2_wdt: add external reset support via 'ext-reset' dt property
Am Donnerstag, den 28.05.2015, 15:54 -0700 schrieb Tim Harvey:
Quoted 25 lines
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_Binstead of an internal reset both on a watchdog timeout and insystem_restart.[1] http://lists.infradead.org/pipermail/linux-arm-kernel/2015-March/333689.html
Signed-off-by Tim Harvey <[email protected]>--- .../devicetree/bindings/watchdog/fsl-imx-wdt.txt | 2 ++ drivers/watchdog/imx2_wdt.c | 18 ++++++++++++++++-- 2 files changed, 18 insertions(+), 2 deletions(-)Documentation/devicetree/bindings/watchdog/fsl-imx-wdt.txt +0
-0
Documentation/devicetree/bindings/watchdog/fsl-imx-wdt.txt Optional property: · line 14
Quoted 6 lines
Documentation/devicetree/bindings/watchdog/fsl-imx-wdt.txt +2
-0@@ -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.
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".
Quoted 29 lines
Documentation/devicetree/bindings/watchdog/fsl-imx-wdt.txt +0
-0Examples:drivers/watchdog/imx2_wdt.c +4
-1@@ -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;
drivers/watchdog/imx2_wdt.c static const struct watchdog_info imx2_wdt_info = { · line 102
Quoted 10 lines
drivers/watchdog/imx2_wdt.c +7
-0struct 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;
drivers/watchdog/imx2_wdt.c static inline void imx2_wdt_setup(struct watchdog_device *wdog) · line 136
Quoted 12 lines
drivers/watchdog/imx2_wdt.c +4
-1/* 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.
Quoted 11 lines
drivers/watchdog/imx2_wdt.c +1
-0/* 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",
Signature · 2 lines
Pengutronix e.K. | Lucas Stach | Industrial Linux Solutions | http://www.pengutronix.de/ |
PATCH 2/2 ARM: dts: ventana: Add ext-reset support
In reply to Tim HarveyThe 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.
--- 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(+)
arch/arm/boot/dts/imx6qdl-gw51xx.dtsi +13 -0
@@ -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
arch/arm/boot/dts/imx6qdl-gw52xx.dtsi +13 -0
@@ -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
arch/arm/boot/dts/imx6qdl-gw53xx.dtsi +13 -0
@@ -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
arch/arm/boot/dts/imx6qdl-gw54xx.dtsi +18 -0
@@ -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
arch/arm/boot/dts/imx6qdl-gw551x.dtsi +12 -0
@@ -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
arch/arm/boot/dts/imx6qdl-gw552x.dtsi +13 -0
@@ -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
Signature · 1 line
1.9.1
PATCH 2/2 ARM: dts: ventana: Add ext-reset support
Hi,
Quoted 7 lines · 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_Bneeds 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
Quoted 180 lines
Signed-off-by Tim Harvey <[email protected]>--- 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(+)arch/arm/boot/dts/imx6qdl-gw51xx.dtsi +13
-0@@ -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 0x1b0b0arch/arm/boot/dts/imx6qdl-gw52xx.dtsi +13
-0@@ -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 0x130b0arch/arm/boot/dts/imx6qdl-gw53xx.dtsi +13
-0@@ -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 0x130b0arch/arm/boot/dts/imx6qdl-gw54xx.dtsi +18
-0@@ -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 0x130b0arch/arm/boot/dts/imx6qdl-gw551x.dtsi +12
-0@@ -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 0x1b0b1arch/arm/boot/dts/imx6qdl-gw552x.dtsi +13
-0@@ -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 0x1b0b0Signature · 7 lines
1.9.1 _______________________________________________ linux-arm-kernel mailing list linux-arm-kernel at lists.infradead.org http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
Signature · 11 lines
Pengutronix e.K. | | Industrial Linux Solutions | http://www.pengutronix.de/ | Peiner Str. 6-8, 31137 Hildesheim, Germany | Phone: +49-5121-206917-0 | Amtsgericht Hildesheim, HRA 2686 | Fax: +49-5121-206917-5555 | -------------- next part -------------- A non-text attachment was scrubbed... Name: signature.asc Type: application/pgp-signature Size: 819 bytes Desc: Digital signature URL: <http://lists.infradead.org/pipermail/linux-arm-kernel/attachments/20150529/c9c159ae/attachment.sig>
PATCH 2/2 ARM: dts: ventana: Add ext-reset support
Quoted 11 lines · Markus Pargmann
Hi,
Quoted 7 lines · 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_Bneeds to be configured which we add to a hog group for lack of a better location.
Quoted 7 lines
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
>;
};....
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
>;
};....
I'll do that in a future revision after feedback.
Thanks,
Tim
v2 cover imx6: Implement external watchdog reset
In reply to Tim HarveyThe 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 when it was at 400Mhz prior to reset.
This adds a new device-tree property 'ext-reset-output' to fsl-imx-wdt in order 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.
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
Tim Harvey (2):
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 | 17 +++++++++++++++-- 8 files changed, 95 insertions(+), 2 deletions(-)
Signature · 1 line
1.9.1
PATCH v2 1/2 watchdog: imx2_wdt: add external reset support via 'ext-reset-output' dt prop
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
---
v2:
- 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(-)
Documentation/devicetree/bindings/watchdog/fsl-imx-wdt.txt +2 -0
@@ -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:
drivers/watchdog/imx2_wdt.c +15 -2
@@ -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",
Signature · 1 line
1.9.1
PATCH v2 1/2 watchdog: imx2_wdt: add external reset support via 'ext-reset-output' dt prop
drivers/watchdog/imx2_wdt.c static inline void imx2_wdt_setup(struct watchdog_device *wdog) · line 134
Quoted 6 lines · Tim Harvey
drivers/watchdog/imx2_wdt.c +6
-0+/* 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
PATCH v2 1/2 watchdog: imx2_wdt: add external reset support via 'ext-reset-output' dt prop
Quoted 8 lines · Zhi Li
drivers/watchdog/imx2_wdt.c static inline void imx2_wdt_setup(struct watchdog_device *wdog) · line 134
Quoted 6 lines · Tim Harvey
drivers/watchdog/imx2_wdt.c +6
-0+/* 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 ;
Quoted 7 lines
I think you can always enable
IMX2_WDT_WCR_WRE.
So needn'text_reset.If you don't config
WDOG_Bpin mux,WDOG_Bdo 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
PATCH v2 1/2 watchdog: imx2_wdt: add external reset support via 'ext-reset-output' dt prop
In reply to Tim HarveyAm Donnerstag, den 04.06.2015, 13:15 -0700 schrieb Tim Harvey:
Quoted 13 lines
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_Binstead of an internal reset both on a watchdog timeout and insystem_restart.
Quoted 4 lines
[1] http://lists.infradead.org/pipermail/linux-arm-kernel/2015-March/333689.html
Cc Lucas Stach <[email protected]>Signed-off-by Tim Harvey <[email protected]>
Quoted 81 lines
---
v2:
- rename property to 'ext-reset-output' based on ML feedback
- simplify setting SRS bit if external-reset
- update comments and commit msgSigned-off-by Tim Harvey <[email protected]>--- .../devicetree/bindings/watchdog/fsl-imx-wdt.txt | 2 ++ drivers/watchdog/imx2_wdt.c | 17 +++++++++++++++-- 2 files changed, 17 insertions(+), 2 deletions(-)Documentation/devicetree/bindings/watchdog/fsl-imx-wdt.txt +2
-0@@ -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:drivers/watchdog/imx2_wdt.c +15
-2@@ -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",
Signature · 2 lines
Pengutronix e.K. | Lucas Stach | Industrial Linux Solutions | http://www.pengutronix.de/ |
PATCH v2 1/2 watchdog: imx2_wdt: add external reset support via 'ext-reset-output' dt prop
In reply to Tim HarveyQuoted 34 lines · 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_Binstead of an internal reset both on a watchdog timeout and insystem_restart.[1] http://lists.infradead.org/pipermail/linux-arm-kernel/2015-March/333689.html
Cc Lucas Stach <[email protected]>Signed-off-by Tim Harvey <[email protected]>---
v2:
- rename property to 'ext-reset-output' based on ML feedback
- simplify setting SRS bit if external-reset
- update comments and commit msgSigned-off-by Tim Harvey <[email protected]>--- .../devicetree/bindings/watchdog/fsl-imx-wdt.txt | 2 ++ drivers/watchdog/imx2_wdt.c | 17 +++++++++++++++-- 2 files changed, 17 insertions(+), 2 deletions(-)Documentation/devicetree/bindings/watchdog/fsl-imx-wdt.txt +0
-0
Documentation/devicetree/bindings/watchdog/fsl-imx-wdt.txt Optional property: · line 13
Quoted 5 lines
Documentation/devicetree/bindings/watchdog/fsl-imx-wdt.txt +2
-0@@ -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.
This is a vendor specific binding property rather than generic one, so it should have a vendor prefix, i.e. "fsl,". Other than that,
PATCH v2 1/2 watchdog: imx2_wdt: add external reset support via 'ext-reset-output' dt prop
In reply to Tim Harveydrivers/watchdog/imx2_wdt.c static int imx2_restart_handler(struct notifier_block *this, unsigned long mode, · line 99
Quoted 9 lines · Tim Harvey
drivers/watchdog/imx2_wdt.c +5
-0@@ -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
Quoted 3 lines
drivers/watchdog/imx2_wdt.c +0
-0/* Assert SRS signal */ regmap_write (wdev->regmap,0 , wcr_enable);/*
PATCH v2 1/2 watchdog: imx2_wdt: add external reset support via 'ext-reset-output' dt prop
Quoted 19 lines · Shawn Guo
drivers/watchdog/imx2_wdt.c static int imx2_restart_handler(struct notifier_block *this, unsigned long mode, · line 99
Quoted 9 lines · Tim Harvey
drivers/watchdog/imx2_wdt.c +5
-0@@ -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_resetflag to avoid asserting both internal and external reset?
Quoted 1 line
Shawn
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
Quoted 4 lines
Quoted 4 lines
drivers/watchdog/imx2_wdt.c +0
-0/* Assert SRS signal */ regmap_write (wdev->regmap,0 , wcr_enable);/*
PATCH v2 1/2 watchdog: imx2_wdt: add external reset support via 'ext-reset-output' dt prop
Quoted 22 lines · Tim Harvey
Quoted 19 lines · Shawn Guo
drivers/watchdog/imx2_wdt.c static int imx2_restart_handler(struct notifier_block *this, unsigned long mode, · line 99
Quoted 9 lines · Tim Harvey
drivers/watchdog/imx2_wdt.c +5
-0@@ -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_resetflag to avoid asserting both internal and external reset?Quoted 1 line
Shawn
Quoted 7 lines
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?:
Yes, that's what I'm saying.
Quoted 11 lines
drivers/watchdog/imx2_wdt.c +3
-0+/* Use external reset */ +if (wdev->ext_reset) + wcr_enable |=IMX2_WDT_WCR_SRS ;/* do not assert internal system reset */
(quoted diff) +2
-0+ 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
v3 cover imx6: Implement external watchdog reset
In reply to Tim HarveyThe 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 when it was at 400Mhz prior to reset.
This adds a new device-tree property 'ext-reset-output' to fsl-imx-wdt in order 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.
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
Tim Harvey (2):
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(-)
Signature · 1 line
1.9.1
PATCH v3 1/2 watchdog: imx2_wdt: add external reset support via 'ext-reset-output' dt prop
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
---
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
--- .../devicetree/bindings/watchdog/fsl-imx-wdt.txt | 2 ++ drivers/watchdog/imx2_wdt.c | 20 ++++++++++++++++++-- 2 files changed, 20 insertions(+), 2 deletions(-)
Documentation/devicetree/bindings/watchdog/fsl-imx-wdt.txt +2 -0
@@ -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:
drivers/watchdog/imx2_wdt.c +18 -2
@@ -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",
Signature · 1 line
1.9.1
PATCH v3 1/2 watchdog: imx2_wdt: add external reset support via 'ext-reset-output' dt prop
Quoted 13 lines · 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_Binstead of an internal reset both on a watchdog timeout and insystem_restart.
Quoted 4 lines
[1] http://lists.infradead.org/pipermail/linux-arm-kernel/2015-March/333689.html
Cc Lucas Stach <[email protected]>Signed-off-by Tim Harvey <[email protected]>
PATCH v3 1/2 watchdog: imx2_wdt: add external reset support via 'ext-reset-output' dt prop
In reply to Tim HarveyQuoted 105 lines · 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_Binstead of an internal reset both on a watchdog timeout and insystem_restart.[1] http://lists.infradead.org/pipermail/linux-arm-kernel/2015-March/333689.html
Cc Lucas Stach <[email protected]>Signed-off-by Tim Harvey <[email protected]>---
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 msgSigned-off-by Tim Harvey <[email protected]>--- .../devicetree/bindings/watchdog/fsl-imx-wdt.txt | 2 ++ drivers/watchdog/imx2_wdt.c | 20 ++++++++++++++++++-- 2 files changed, 20 insertions(+), 2 deletions(-)Documentation/devicetree/bindings/watchdog/fsl-imx-wdt.txt +2
-0@@ -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:drivers/watchdog/imx2_wdt.c +18
-3@@ -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", --
Quoted 2 lines
1.9.1
Wim,
I believe I missed sending this to the proper watchdog maintainer which appears to be you.
Regards,
Tim
PATCH v3 2/2 ARM: dts: ventana: Add ext-reset support
In reply to Tim HarveyThe 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 iomux.
---
v3:
- no changes
v2:
- moved pinctl to wdog group instead of hog
- updated property name
--- 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 ++++++++++++ 6 files changed, 78 insertions(+)
arch/arm/boot/dts/imx6qdl-gw51xx.dtsi +12 -0
@@ -210,6 +210,12 @@ status = "okay"; }; +&wdog1 { + pinctrl-names = "default"; + pinctrl-0 = <&pinctrl_wdog>; + ext-reset-output; +}; + &iomuxc { imx6qdl-gw51xx { pinctrl_enet: enetgrp {
@@ -328,5 +334,11 @@ MX6QDL_PAD_EIM_D22__GPIO3_IO22 0x1b0b0 /* OTG_PWR_EN */ >; }; + + pinctrl_wdog: wdoggrp { + fsl,pins = < + MX6QDL_PAD_DISP0_DAT8__WDOG1_B 0x1b0b0 + >; + }; }; };
arch/arm/boot/dts/imx6qdl-gw52xx.dtsi +12 -0
@@ -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 + >; + }; }; };
arch/arm/boot/dts/imx6qdl-gw53xx.dtsi +13 -0
@@ -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 + >; + }; }; };
arch/arm/boot/dts/imx6qdl-gw54xx.dtsi +17 -0
@@ -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 + >; + }; }; };
arch/arm/boot/dts/imx6qdl-gw551x.dtsi +12 -0
@@ -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 + >; + }; }; };
arch/arm/boot/dts/imx6qdl-gw552x.dtsi +12 -0
@@ -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 + >; + }; }; };
Signature · 1 line
1.9.1
v3 cover imx6: Implement external watchdog reset
In reply to Tim HarveyQuoted 9 lines · 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 voltate for the CPU when comming out of reset at 800Mhz when it was at 400Mhz prior to reset.
Quoted 5 lines
This adds a new device-tree property 'ext-reset-output' to fsl-imx-wdt in order to indicate the board has such a reset and to cause the watchdog to be configured to assert
WDOG_Binstead of an internal reset both on a watchdog timeout and insystem_restart.
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
Quoted 30 lines
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_Bto 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
Cc Fabio Estevam <[email protected]>Cc Lucas Stach <[email protected]>Cc Stefan Roese <[email protected]>Cc Iain Paton <[email protected]>Cc Sascha Hauer <[email protected]>Tim Harvey (2):
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(-)
v3 cover imx6: Implement external watchdog reset
Quoted 23 lines · Akshay Bhat
Quoted 13 lines · 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 voltate for
the CPU when comming out of reset at 800Mhz when it was at 400Mhz prior to reset.Quoted 6 lines
This adds a new device-tree property 'ext-reset-output' to fsl-imx-wdt in order to indicate the board has such a reset and to cause the watchdog to be
configured to assertWDOG_Binstead of an internal reset both on a watchdog timeout and insystem_restart.
Quoted 6 lines
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
PATCH v2 2/2 ARM: dts: ventana: Add ext-reset support
In reply to Tim HarveyThe 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 iomux.
---
v2:
- moved pinctl to wdog group instead of hog
- updated property name
--- 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 ++++++++++++ 6 files changed, 78 insertions(+)
arch/arm/boot/dts/imx6qdl-gw51xx.dtsi +12 -0
@@ -210,6 +210,12 @@ status = "okay"; }; +&wdog1 { + pinctrl-names = "default"; + pinctrl-0 = <&pinctrl_wdog>; + ext-reset-output; +}; + &iomuxc { imx6qdl-gw51xx { pinctrl_enet: enetgrp {
@@ -329,5 +335,11 @@ MX6QDL_PAD_EIM_D22__GPIO3_IO22 0x1b0b0 /* OTG_PWR_EN */ >; }; + + pinctrl_wdog: wdoggrp { + fsl,pins = < + MX6QDL_PAD_DISP0_DAT8__WDOG1_B 0x1b0b0 + >; + }; }; };
arch/arm/boot/dts/imx6qdl-gw52xx.dtsi +12 -0
@@ -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 + >; + }; }; };
arch/arm/boot/dts/imx6qdl-gw53xx.dtsi +13 -0
@@ -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 + >; + }; }; };
arch/arm/boot/dts/imx6qdl-gw54xx.dtsi +17 -0
@@ -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 + >; + }; }; };
arch/arm/boot/dts/imx6qdl-gw551x.dtsi +12 -0
@@ -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 + >; + }; }; };
arch/arm/boot/dts/imx6qdl-gw552x.dtsi +12 -0
@@ -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 + >; + }; }; };
Signature · 1 line
1.9.1