> -----Original Message----- > From: Wim Van Sebroeck [mailto:wim@xxxxxxxxx] > Sent: Thursday, July 14, 2011 3:59 PM > To: Ashish Jangam > Cc: linux-kernel@xxxxxxxxxxxxxxx; linux-watchdog@xxxxxxxxxxxxxxx; Dajun; > linaro-dev@xxxxxxxxxxxxxxxx > Subject: Re: [PATCH 07/11] Watchdog: DA9052 watchdog support v1 > > Hi Ashish, Dajun, > > > > This driver adds support for the watchdog functionality provided by the > Dialog > > > Semiconductor DA9052 PMIC chip. > > > > > > Signed-off-by: David Dajun Chen <dchen@xxxxxxxxxxx> > > > Signed-off-by: Ashish Jangam <ashish.jangam@xxxxxxxxxxxxxxx> > > > --- > > If there are no comments then can you ACK this patch? > > I remain with my comment of Wed 26 Jan 2011: > > The ioctl WDIOC_SETOPTIONS call is not in line with the watchdog API. > > the supported options are: > > #define WDIOS_DISABLECARD 0x0001 /* Turn off the watchdog timer */ > > #define WDIOS_ENABLECARD 0x0002 /* Turn on the watchdog timer */ > > #define WDIOS_TEMPPANIC 0x0004 /* Kernel panic on temperature trip > */ > > Your options are: > > #define DA9052_STROBING_FILTER_ENABLE 0x0001 > > #define DA9052_STROBING_FILTER_DISABLE 0x0002 > > #define DA9052_SET_STROBING_MODE_MANUAL 0x0004 > > #define DA9052_SET_STROBING_MODE_AUTO 0x0008 > > > > Please explain what you want to do. > > FYI: this is how the code looks now: > + case WDIOC_SETOPTIONS: > + if (get_user(new_value, p)) > + return -EFAULT; > + if (new_value == DA9052_ENABLE || new_value == DA9052_DISABLE) > + da9052_sm_set_strobing_filter(wdt, new_value); > + else > + wdt->pwdt->sm_strobe_mode_flag = new_value; > + return 0; > > Since the code is still not in line with the watchdog API and since I did not > get > an answer yet on what you (or Dajun) want to do, there is definitely NO ACK > yet. > If there is a good reasoning behind your needs, we might conclude that this is > indeed > worthwhile and extend the API in a proper way. But as it is now, it's not > correct. If strobe filter is "Enable" (set/reset thru ioctl) then only a new wdt time window is set else if strobe filter is "Disable", new wdt time window is set to a min value or wdt is disabled depending upon condition. If the logic is complex then I will simplify it. > > Kind regards, > Wim. > ��.n��������+%������w��{.n�����{���rh���ܨ}���Ơz�j:+v�����w����ޙ��&�)ߡ�a����z�ޗ���ݢj��w�f
