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 <dc...@diasemi.com> > > Signed-off-by: Ashish Jangam <ashish.jan...@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); + 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. Kind regards, Wim. _______________________________________________ linaro-dev mailing list linaro-dev@lists.linaro.org http://lists.linaro.org/mailman/listinfo/linaro-dev
