>-----Original Message----- >From: Guenter Roeck [mailto:linux@xxxxxxxxxxxx] >Sent: Friday, September 12, 2014 10:49 AM >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 09/11/2014 07:42 PM, Jingchang Lu wrote: >>> -----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. >> >I assumed that the original code would be removed. If you have to keep it >around for mx1, the conversion doesn't seem to make much sense as you just >end up having the code twice. Am I missing something ? > >Thanks, >Guenter Do you mean removing the code of mxc_restart() in arch/arm/mach-imx/system.c ? Yes, when this patch is accepted, all platform compatible with the imx2_wdt can migrate to the restart handler. I only add the restart handler for imx2_wdt and reserve the original code for that still uses it currently. Thanks. Best Regards, Jingchang ��.n��������+%������w��{.n�����{���rh���ܨ}���Ơz�j:+v�����w����ޙ��&�)ߡ�a����z�ޗ���ݢj��w�f