-----Original Message----- From: Wim Van Sebroeck [mailto:wim@iguana.be] Sent: Thursday, July 14, 2011 3:59 PM To: Ashish Jangam Cc: linux-kernel@vger.kernel.org; linux-watchdog@vger.kernel.org; Dajun; linaro-dev@lists.linaro.org 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@diasemi.com Signed-off-by: Ashish Jangam ashish.jangam@kpitcummins.com
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);elsewdt->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.