>-----Original Message----- >From: Guenter Roeck [mailto:groeck7@xxxxxxxxx] On Behalf Of Guenter Roeck >Sent: Thursday, September 11, 2014 11:55 PM >To: Lu Jingchang-B35083 >Cc: wim@xxxxxxxxx; Guo Shawn-R65073; arnd@xxxxxxxx; linux- >watchdog@xxxxxxxxxxxxxxx; linux-arm-kernel@xxxxxxxxxxxxxxxxxxx >Subject: Re: [PATCH] watchdog: imx2_wdt: add restart handler support > >On Thu, Sep 11, 2014 at 03:47:41PM +0800, Jingchang Lu wrote: >> Register the watchdog as the system restart function >> to the new introducing kernel restart call chain in the >> driver instead of providing the restart in machine desc. >> This restart handler function is from the mxc_restart() >> in arch/arm/mach-imx/system.c >> >> Signed-off-by: Jingchang Lu <jingchang.lu@xxxxxxxxxxxxx> >> --- >> drivers/watchdog/imx2_wdt.c | 37 +++++++++++++++++++++++++++++++++++++ >> 1 file changed, 37 insertions(+) >> >> diff --git a/drivers/watchdog/imx2_wdt.c b/drivers/watchdog/imx2_wdt.c >> index 68c3d37..fa723e8 100644 >> --- a/drivers/watchdog/imx2_wdt.c >> +++ b/drivers/watchdog/imx2_wdt.c >> @@ -22,14 +22,17 @@ >> */ >> >> #include <linux/clk.h> >> +#include <linux/delay.h> >> #include <linux/init.h> >> #include <linux/io.h> >> #include <linux/jiffies.h> >> #include <linux/kernel.h> >> #include <linux/module.h> >> #include <linux/moduleparam.h> >> +#include <linux/notifier.h> >> #include <linux/of_address.h> >> #include <linux/platform_device.h> >> +#include <linux/reboot.h> >> #include <linux/regmap.h> >> #include <linux/timer.h> >> #include <linux/watchdog.h> >> @@ -59,6 +62,7 @@ struct imx2_wdt_device { >> struct regmap *regmap; >> struct timer_list timer; /* Pings the watchdog when closed */ >> struct watchdog_device wdog; >> + struct notifier_block restart_handler; >> }; >> >> static bool nowayout = WATCHDOG_NOWAYOUT; >> @@ -77,6 +81,31 @@ static const struct watchdog_info imx2_wdt_info = { >> .options = WDIOF_KEEPALIVEPING | WDIOF_SETTIMEOUT | WDIOF_MAGICCLOSE, >> }; >> >> +static int imx2_restart_handler(struct notifier_block *this, unsigned >long mode, >> + void *cmd) >> +{ >> + unsigned int wcr_enable = IMX2_WDT_WCR_WDE; > >In the original code this value is conditional depending on cpu_is_mx1(). >Are there any implications for this reset mechanism ? Does mx1 have a >different watchdog driver ? > Hi, Shawn, is this driver also used by the mx1 SoC? The imx2_wdt.c says it is for IMX2 and later processors, so when moving the restart function, I removed the cpu_is_mx1() condition determination, Thanks. >> + struct imx2_wdt_device *wdev = container_of(this, >> + struct imx2_wdt_device, >> + restart_handler); > >Please align second lines with '('. Ok, thanks. Best Regards, Jingchang ��.n��������+%������w��{.n�����{���rh���ܨ}���Ơz�j:+v�����w����ޙ��&�)ߡ�a����z�ޗ���ݢj��w�f