Skip to contentSkip to conversation

Watchdog: DA9052 watchdog support v1

PATCH In review ashishj3 2 participants 4 messages Jul 5, 2011 – Jul 25, 2011

Subsystems (2)
Watchdog device drivers Dialog semiconductor drivers
lore
Share & tools
lore

ashishj3

author
#0 lore raw

PATCH 7/11 Watchdog: DA9052 watchdog support v1

This driver adds support for the watchdog functionality provided by the Dialog Semiconductor DA9052 PMIC chip.

Signed-off-by David Dajun Chen <[email protected]>
Signed-off-by Ashish Jangam <[email protected]>
---
 drivers/watchdog/Kconfig      |    6 +
 drivers/watchdog/Makefile     |    1 +
 drivers/watchdog/da9052_wdt.c |  472 +++++++++++++++++++++++++++++++++++++++++
 3 files changed, 479 insertions(+), 0 deletions(-)
 create mode 100755 drivers/watchdog/da9052_wdt.c
drivers/watchdog/Kconfig +6 -0
index 022f9eb..6689061 100644
@@ -45,6 +45,12 @@ comment "Watchdog Device Drivers"
 
 # Architecture Independent
 
+config DA9052_WATCHDOG
+        tristate "Dialog DA9052 Watchdog"
+        depends on PMIC_DA9052
+        help
+          Support for the watchdog in the DA9052 PMIC.
+
 config SOFT_WATCHDOG
 	tristate "Software watchdog"
 	help
drivers/watchdog/Makefile +1 -0
index ed26f70..85f3c42 100644
@@ -158,4 +158,5 @@ obj-$(CONFIG_XEN_WDT) += xen_wdt.o
 obj-$(CONFIG_WM831X_WATCHDOG) += wm831x_wdt.o
 obj-$(CONFIG_WM8350_WATCHDOG) += wm8350_wdt.o
 obj-$(CONFIG_MAX63XX_WATCHDOG) += max63xx_wdt.o
+obj-$(CONFIG_DA9052_WATCHDOG) += da9052_wdt.o
 obj-$(CONFIG_SOFT_WATCHDOG) += softdog.o
drivers/watchdog/da9052_wdt.c new file +472 -0

472 lines not on this page · raw message

#2 lore raw

Hi Ashish, Dajun,

Quoted 7 lines
Quoted 6 lines

This driver adds support for the watchdog functionality provided by the Dialog Semiconductor DA9052 PMIC chip.

Signed-off-by David Dajun Chen <[email protected]>
Signed-off-by Ashish Jangam <[email protected]>

---

If there are no comments then can you ACK this patch?

I remain with my comment of Wed 26 Jan 2011:

Quoted 5 lines

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 */
Quoted 7 lines

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.

author
#3 lore raw
Quoted 45 lines

-----Original Message-----
From: Wim Van Sebroeck [mailto:[email protected]]
Sent: Thursday, July 14, 2011 3:59 PM
To: Ashish Jangam
Cc: [email protected]; [email protected]; Dajun; [email protected]
Subject: Re: [PATCH 07/11] Watchdog: DA9052 watchdog support v1

Hi Ashish, Dajun,

Quoted 1 line
Quoted 1 line

This driver adds support for the watchdog functionality provided by the

Dialog

Quoted 6 lines
Quoted 5 lines

Semiconductor DA9052 PMIC chip.

Signed-off-by David Dajun Chen <[email protected]>
Signed-off-by Ashish Jangam <[email protected]>

---

If there are no comments then can you ACK this patch?

I remain with my comment of Wed 26 Jan 2011:

Quoted 5 lines

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

*/

Quoted 7 lines

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:

drivers/watchdog/da9052_wdt.c new file +8 -0
+       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;
Quoted 8 lines

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.

Quoted 4 lines

Kind regards,
Wim.

���{.n�+�������+%�����ݶ��w��{.n�+����{��G�����{ay� ʇڙ�,j��f���h��������z_�(�階�ݢj"���m������G������?����&���~��iO��z��v�^�m���� ������?�I�