2013-03-10 23:15:10

by Guenter Roeck

[permalink] [raw]
Subject: [PATCH v2 0/8] watchdog: w83627hf: Convert to watchdog infrastructure

Convert to watchdog infrastructure, cleanup, add support for additional
chips, and merge with W83697HF and W83697UG watchdog drivers.

Tested with W83627UHG, NCT6775, NCT6776. Additional test feedback
for other chips would be appreciated.

Original idea was to prepare the driver for use as a client to an MFD driver,
but I found that requesting memory with request_muxed_region works just as well
and has less impact. v2 includes the knowledge gained from converting the
driver to an MFD client driver, but without the actual conversion.

v2: Tested with W83627UHG
Retain timeout module parameter; use watchdog_init_timeout to set it
Eliminate some cosmetic changes
Drop spinlock.h include
Keep "initialized" message
Don't try to configure WDTO for W83627UHG and W83627EHF
Don't report the nowayout option with the 'initializing' message
Use request_muxed_region to reserve memory range only while needed
Add support for W83627S, W83627DHG-P, W83667HG, and NCT6779


2013-03-10 23:15:16

by Guenter Roeck

[permalink] [raw]
Subject: [PATCH v2 6/7] watchdog: w83627hf: Add support for W83697HF and W83697UG

Major difference is that the watchdog control and counter registers
are different on both chips.

Signed-off-by: Guenter Roeck <[email protected]>
---
v2: Previously patch 7/8
No change

drivers/watchdog/Kconfig | 2 ++
drivers/watchdog/w83627hf_wdt.c | 62 +++++++++++++++++++++++++++++++--------
2 files changed, 51 insertions(+), 13 deletions(-)

diff --git a/drivers/watchdog/Kconfig b/drivers/watchdog/Kconfig
index ef41028..bf04b38 100644
--- a/drivers/watchdog/Kconfig
+++ b/drivers/watchdog/Kconfig
@@ -864,6 +864,8 @@ config W83627HF_WDT
W83637HF
W83667HG/HG-B
W83687THF
+ W83697HF
+ W83697UG
NCT6775
NCT6776
NCT6779
diff --git a/drivers/watchdog/w83627hf_wdt.c b/drivers/watchdog/w83627hf_wdt.c
index eb368f2..4c9f27a 100644
--- a/drivers/watchdog/w83627hf_wdt.c
+++ b/drivers/watchdog/w83627hf_wdt.c
@@ -45,10 +45,12 @@
#define WATCHDOG_TIMEOUT 60 /* 60 sec default timeout */

static int wdt_io;
+static int cr_wdt_timeout; /* WDT timeout register */
+static int cr_wdt_control; /* WDT control register */

-enum chips { w83627hf, w83627s, w83637hf, w83627thf, w83687thf,
- w83627ehf, w83627dhg, w83627uhg, w83667hg, w83627dhg_p, w83667hg_b,
- nct6775, nct6776, nct6779 };
+enum chips { w83627hf, w83627s, w83697hf, w83697ug, w83637hf, w83627thf,
+ w83687thf, w83627ehf, w83627dhg, w83627uhg, w83667hg, w83627dhg_p,
+ w83667hg_b, nct6775, nct6776, nct6779 };

static int timeout; /* in seconds */
module_param(timeout, int, 0);
@@ -69,6 +71,8 @@ MODULE_PARM_DESC(timeout,

#define W83627HF_ID 0x52
#define W83627S_ID 0x59
+#define W83697HF_ID 0x60
+#define W83697UG_ID 0x68
#define W83637HF_ID 0x70
#define W83627THF_ID 0x82
#define W83687THF_ID 0x85
@@ -82,6 +86,12 @@ MODULE_PARM_DESC(timeout,
#define NCT6776_ID 0xc3
#define NCT6779_ID 0xc5

+#define W83627HF_WDT_TIMEOUT 0xf6
+#define W83697HF_WDT_TIMEOUT 0xf4
+
+#define W83627HF_WDT_CONTROL 0xf5
+#define W83697HF_WDT_CONTROL 0xf3
+
static void superio_outb(int reg, int val)
{
outb(reg, WDT_EFER);
@@ -138,6 +148,17 @@ static int w83627hf_init(struct watchdog_device *wdog, enum chips chip)
t = superio_inb(0x2B) & ~0x10;
superio_outb(0x2B, t); /* set GPIO24 to WDT0 */
break;
+ case w83697hf:
+ /* Set pin 119 to WDTO# mode (= CR29, WDT0) */
+ t = superio_inb(0x29) & ~0x60;
+ t |= 0x20;
+ superio_outb(0x29, t);
+ break;
+ case w83697ug:
+ /* Set pin 118 to WDTO# mode */
+ t = superio_inb(0x2b) & ~0x04;
+ superio_outb(0x2b, t);
+ break;
case w83627thf:
t = (superio_inb(0x2B) & ~0x08) | 0x04;
superio_outb(0x2B, t); /* set GPIO3 to WDT0 */
@@ -146,10 +167,10 @@ static int w83627hf_init(struct watchdog_device *wdog, enum chips chip)
case w83627dhg_p:
t = superio_inb(0x2D) & ~0x01; /* PIN77 -> WDT0# */
superio_outb(0x2D, t); /* set GPIO5 to WDT0 */
- t = superio_inb(0xF5);
+ t = superio_inb(cr_wdt_control);
t |= 0x02; /* enable the WDTO# output low pulse
* to the KBRST# pin */
- superio_outb(0xF5, t);
+ superio_outb(cr_wdt_control, t);
break;
case w83637hf:
break;
@@ -170,25 +191,25 @@ static int w83627hf_init(struct watchdog_device *wdog, enum chips chip)
* Don't touch its configuration, and hope the BIOS
* does the right thing.
*/
- t = superio_inb(0xF5);
+ t = superio_inb(cr_wdt_control);
t |= 0x02; /* enable the WDTO# output low pulse
* to the KBRST# pin */
- superio_outb(0xF5, t);
+ superio_outb(cr_wdt_control, t);
break;
default:
break;
}

- t = superio_inb(0xF6);
+ t = superio_inb(cr_wdt_timeout);
if (t != 0) {
pr_info("Watchdog already running. Resetting timeout to %d sec\n",
wdog->timeout);
- superio_outb(0xF6, wdog->timeout);
+ superio_outb(cr_wdt_timeout, wdog->timeout);
}

/* set second mode & disable keyboard turning off watchdog */
- t = superio_inb(0xF5) & ~0x0C;
- superio_outb(0xF5, t);
+ t = superio_inb(cr_wdt_control) & ~0x0C;
+ superio_outb(cr_wdt_control, t);

/* disable keyboard & mouse turning off watchdog */
t = superio_inb(0xF7) & ~0xC0;
@@ -208,7 +229,7 @@ static int wdt_set_time(unsigned int timeout)
return ret;

superio_select(W83627HF_LD_WDT);
- superio_outb(0xF6, timeout);
+ superio_outb(cr_wdt_timeout, timeout);
superio_exit();

return 0;
@@ -242,7 +263,7 @@ static unsigned int wdt_get_time(struct watchdog_device *wdog)
return 0;

superio_select(W83627HF_LD_WDT);
- timeleft = superio_inb(0xF6);
+ timeleft = superio_inb(cr_wdt_timeout);
superio_exit();

return timeleft;
@@ -300,6 +321,9 @@ static int wdt_find(int addr)
u8 val;
int ret;

+ cr_wdt_timeout = W83627HF_WDT_TIMEOUT;
+ cr_wdt_control = W83627HF_WDT_CONTROL;
+
ret = superio_enter();
if (ret)
return ret;
@@ -312,6 +336,16 @@ static int wdt_find(int addr)
case W83627S_ID:
ret = w83627s;
break;
+ case W83697HF_ID:
+ ret = w83697hf;
+ cr_wdt_timeout = W83697HF_WDT_TIMEOUT;
+ cr_wdt_control = W83697HF_WDT_CONTROL;
+ break;
+ case W83697UG_ID:
+ ret = w83697ug;
+ cr_wdt_timeout = W83697HF_WDT_TIMEOUT;
+ cr_wdt_control = W83697HF_WDT_CONTROL;
+ break;
case W83637HF_ID:
ret = w83637hf;
break;
@@ -368,6 +402,8 @@ static int __init wdt_init(void)
const char * const chip_name[] = {
"W83627HF",
"W83627S",
+ "W83697HF",
+ "W83697UG",
"W83637HF",
"W83627THF",
"W83687THF",
--
1.7.9.7

2013-03-10 23:15:15

by Guenter Roeck

[permalink] [raw]
Subject: [PATCH v2 1/7] watchdog: w83627hf: Convert to watchdog infrastructure

Signed-off-by: Guenter Roeck <[email protected]>
---
v2: Keep timeout module parameter
Eliminate whitespace/cosmetic changes
Drop spinlock.h include
Keep "initialized" message

drivers/watchdog/Kconfig | 1 +
drivers/watchdog/w83627hf_wdt.c | 225 +++++++++------------------------------
2 files changed, 51 insertions(+), 175 deletions(-)

diff --git a/drivers/watchdog/Kconfig b/drivers/watchdog/Kconfig
index 9fcc70c..dd462af 100644
--- a/drivers/watchdog/Kconfig
+++ b/drivers/watchdog/Kconfig
@@ -856,6 +856,7 @@ config VIA_WDT
config W83627HF_WDT
tristate "W83627HF/W83627DHG Watchdog Timer"
depends on X86
+ select WATCHDOG_CORE
---help---
This is the driver for the hardware watchdog on the W83627HF chipset
as used in Advantech PC-9578 and Tyan S2721-533 motherboards
diff --git a/drivers/watchdog/w83627hf_wdt.c b/drivers/watchdog/w83627hf_wdt.c
index 92f1326..51a22ab 100644
--- a/drivers/watchdog/w83627hf_wdt.c
+++ b/drivers/watchdog/w83627hf_wdt.c
@@ -1,6 +1,9 @@
/*
* w83627hf/thf WDT driver
*
+ * (c) Copyright 2013 Guenter Roeck
+ * converted to watchdog infrastructure
+ *
* (c) Copyright 2007 Vlad Drukker <[email protected]>
* added support for W83627THF.
*
@@ -31,42 +34,27 @@
#include <linux/module.h>
#include <linux/moduleparam.h>
#include <linux/types.h>
-#include <linux/miscdevice.h>
#include <linux/watchdog.h>
-#include <linux/fs.h>
#include <linux/ioport.h>
#include <linux/notifier.h>
#include <linux/reboot.h>
#include <linux/init.h>
-#include <linux/spinlock.h>
#include <linux/io.h>
-#include <linux/uaccess.h>
-

#define WATCHDOG_NAME "w83627hf/thf/hg/dhg WDT"
#define WATCHDOG_TIMEOUT 60 /* 60 sec default timeout */

-static unsigned long wdt_is_open;
-static char expect_close;
-static DEFINE_SPINLOCK(io_lock);
-
/* You must set this - there is no sane way to probe for this board. */
static int wdt_io = 0x2E;
module_param(wdt_io, int, 0);
MODULE_PARM_DESC(wdt_io, "w83627hf/thf WDT io port (default 0x2E)");

-static int timeout = WATCHDOG_TIMEOUT; /* in seconds */
+static int timeout; /* in seconds */
module_param(timeout, int, 0);
MODULE_PARM_DESC(timeout,
"Watchdog timeout in seconds. 1 <= timeout <= 255, default="
__MODULE_STRING(WATCHDOG_TIMEOUT) ".");

-static bool nowayout = WATCHDOG_NOWAYOUT;
-module_param(nowayout, bool, 0);
-MODULE_PARM_DESC(nowayout,
- "Watchdog cannot be stopped once started (default="
- __MODULE_STRING(WATCHDOG_NOWAYOUT) ")");
-
/*
* Kernel methods.
*/
@@ -110,7 +98,7 @@ static void w83627hf_unselect_wd_register(void)
/* tyan motherboards seem to set F5 to 0x4C ?
* So explicitly init to appropriate value. */

-static void w83627hf_init(void)
+static void w83627hf_init(struct watchdog_device *wdog)
{
unsigned char t;

@@ -120,8 +108,8 @@ static void w83627hf_init(void)
t = inb_p(WDT_EFDR); /* read CRF6 */
if (t != 0) {
pr_info("Watchdog already running. Resetting timeout to %d sec\n",
- timeout);
- outb_p(timeout, WDT_EFDR); /* Write back to CRF6 */
+ wdog->timeout);
+ outb_p(wdog->timeout, WDT_EFDR); /* Write back to CRF6 */
}

outb_p(0xF5, WDT_EFER); /* Select CRF5 */
@@ -141,10 +129,8 @@ static void w83627hf_init(void)
w83627hf_unselect_wd_register();
}

-static void wdt_set_time(int timeout)
+static int wdt_set_time(unsigned int timeout)
{
- spin_lock(&io_lock);
-
w83627hf_select_wd_register();

outb_p(0xF6, WDT_EFER); /* Select CRF6 */
@@ -152,34 +138,30 @@ static void wdt_set_time(int timeout)

w83627hf_unselect_wd_register();

- spin_unlock(&io_lock);
+ return 0;
}

-static int wdt_ping(void)
+static int wdt_start(struct watchdog_device *wdog)
{
- wdt_set_time(timeout);
- return 0;
+ return wdt_set_time(wdog->timeout);
}

-static int wdt_disable(void)
+static int wdt_stop(struct watchdog_device *wdog)
{
- wdt_set_time(0);
- return 0;
+ return wdt_set_time(0);
}

-static int wdt_set_heartbeat(int t)
+static int wdt_set_timeout(struct watchdog_device *wdog, unsigned int timeout)
{
- if (t < 1 || t > 255)
- return -EINVAL;
- timeout = t;
+ wdt_set_time(timeout);
+ wdog->timeout = timeout;
+
return 0;
}

-static int wdt_get_time(void)
+static unsigned int wdt_get_time(struct watchdog_device *wdog)
{
- int timeleft;
-
- spin_lock(&io_lock);
+ unsigned int timeleft;

w83627hf_select_wd_register();

@@ -188,124 +170,17 @@ static int wdt_get_time(void)

w83627hf_unselect_wd_register();

- spin_unlock(&io_lock);
-
return timeleft;
}

-static ssize_t wdt_write(struct file *file, const char __user *buf,
- size_t count, loff_t *ppos)
-{
- if (count) {
- if (!nowayout) {
- size_t i;
-
- expect_close = 0;
-
- for (i = 0; i != count; i++) {
- char c;
- if (get_user(c, buf + i))
- return -EFAULT;
- if (c == 'V')
- expect_close = 42;
- }
- }
- wdt_ping();
- }
- return count;
-}
-
-static long wdt_ioctl(struct file *file, unsigned int cmd, unsigned long arg)
-{
- void __user *argp = (void __user *)arg;
- int __user *p = argp;
- int timeval;
- static const struct watchdog_info ident = {
- .options = WDIOF_KEEPALIVEPING | WDIOF_SETTIMEOUT |
- WDIOF_MAGICCLOSE,
- .firmware_version = 1,
- .identity = "W83627HF WDT",
- };
-
- switch (cmd) {
- case WDIOC_GETSUPPORT:
- if (copy_to_user(argp, &ident, sizeof(ident)))
- return -EFAULT;
- break;
- case WDIOC_GETSTATUS:
- case WDIOC_GETBOOTSTATUS:
- return put_user(0, p);
- case WDIOC_SETOPTIONS:
- {
- int options, retval = -EINVAL;
-
- if (get_user(options, p))
- return -EFAULT;
- if (options & WDIOS_DISABLECARD) {
- wdt_disable();
- retval = 0;
- }
- if (options & WDIOS_ENABLECARD) {
- wdt_ping();
- retval = 0;
- }
- return retval;
- }
- case WDIOC_KEEPALIVE:
- wdt_ping();
- break;
- case WDIOC_SETTIMEOUT:
- if (get_user(timeval, p))
- return -EFAULT;
- if (wdt_set_heartbeat(timeval))
- return -EINVAL;
- wdt_ping();
- /* Fall */
- case WDIOC_GETTIMEOUT:
- return put_user(timeout, p);
- case WDIOC_GETTIMELEFT:
- timeval = wdt_get_time();
- return put_user(timeval, p);
- default:
- return -ENOTTY;
- }
- return 0;
-}
-
-static int wdt_open(struct inode *inode, struct file *file)
-{
- if (test_and_set_bit(0, &wdt_is_open))
- return -EBUSY;
- /*
- * Activate
- */
-
- wdt_ping();
- return nonseekable_open(inode, file);
-}
-
-static int wdt_close(struct inode *inode, struct file *file)
-{
- if (expect_close == 42)
- wdt_disable();
- else {
- pr_crit("Unexpected close, not stopping watchdog!\n");
- wdt_ping();
- }
- expect_close = 0;
- clear_bit(0, &wdt_is_open);
- return 0;
-}
-
/*
* Notifier for system down
*/
-
static int wdt_notify_sys(struct notifier_block *this, unsigned long code,
void *unused)
{
if (code == SYS_DOWN || code == SYS_HALT)
- wdt_disable(); /* Turn the WDT off */
+ wdt_set_time(0); /* Turn the WDT off */

return NOTIFY_DONE;
}
@@ -314,19 +189,26 @@ static int wdt_notify_sys(struct notifier_block *this, unsigned long code,
* Kernel Interfaces
*/

-static const struct file_operations wdt_fops = {
- .owner = THIS_MODULE,
- .llseek = no_llseek,
- .write = wdt_write,
- .unlocked_ioctl = wdt_ioctl,
- .open = wdt_open,
- .release = wdt_close,
+static struct watchdog_info wdt_info = {
+ .options = WDIOF_SETTIMEOUT | WDIOF_KEEPALIVEPING | WDIOF_MAGICCLOSE,
+ .identity = "W83627HF Watchdog",
+};
+
+static struct watchdog_ops wdt_ops = {
+ .owner = THIS_MODULE,
+ .start = wdt_start,
+ .stop = wdt_stop,
+ .ping = wdt_start,
+ .set_timeout = wdt_set_timeout,
+ .get_timeleft = wdt_get_time,
};

-static struct miscdevice wdt_miscdev = {
- .minor = WATCHDOG_MINOR,
- .name = "watchdog",
- .fops = &wdt_fops,
+static struct watchdog_device wdt_dev = {
+ .info = &wdt_info,
+ .ops = &wdt_ops,
+ .timeout = WATCHDOG_TIMEOUT,
+ .min_timeout = 1,
+ .max_timeout = 255,
};

/*
@@ -341,22 +223,19 @@ static struct notifier_block wdt_notifier = {
static int __init wdt_init(void)
{
int ret;
+ bool nowayout = WATCHDOG_NOWAYOUT;

pr_info("WDT driver for the Winbond(TM) W83627HF/THF/HG/DHG Super I/O chip initialising\n");

- if (wdt_set_heartbeat(timeout)) {
- wdt_set_heartbeat(WATCHDOG_TIMEOUT);
- pr_info("timeout value must be 1 <= timeout <= 255, using %d\n",
- WATCHDOG_TIMEOUT);
- }
-
if (!request_region(wdt_io, 1, WATCHDOG_NAME)) {
pr_err("I/O address 0x%04x already in use\n", wdt_io);
- ret = -EIO;
- goto out;
+ return -EIO;
}

- w83627hf_init();
+ watchdog_init_timeout(&wdt_dev, timeout, NULL);
+ watchdog_set_nowayout(&wdt_dev, nowayout);
+
+ w83627hf_init(&wdt_dev);

ret = register_reboot_notifier(&wdt_notifier);
if (ret != 0) {
@@ -364,28 +243,25 @@ static int __init wdt_init(void)
goto unreg_regions;
}

- ret = misc_register(&wdt_miscdev);
- if (ret != 0) {
- pr_err("cannot register miscdev on minor=%d (err=%d)\n",
- WATCHDOG_MINOR, ret);
+ ret = watchdog_register_device(&wdt_dev);
+ if (ret)
goto unreg_reboot;
- }

pr_info("initialized. timeout=%d sec (nowayout=%d)\n",
- timeout, nowayout);
+ wdt_dev.timeout, nowayout);

-out:
return ret;
+
unreg_reboot:
unregister_reboot_notifier(&wdt_notifier);
unreg_regions:
release_region(wdt_io, 1);
- goto out;
+ return ret;
}

static void __exit wdt_exit(void)
{
- misc_deregister(&wdt_miscdev);
+ watchdog_unregister_device(&wdt_dev);
unregister_reboot_notifier(&wdt_notifier);
release_region(wdt_io, 1);
}
@@ -396,4 +272,3 @@ module_exit(wdt_exit);
MODULE_LICENSE("GPL");
MODULE_AUTHOR("Pádraig Brady <[email protected]>");
MODULE_DESCRIPTION("w83627hf/thf WDT driver");
-MODULE_ALIAS_MISCDEV(WATCHDOG_MINOR);
--
1.7.9.7

2013-03-10 23:15:13

by Guenter Roeck

[permalink] [raw]
Subject: [PATCH v2 3/7] watchdog: w83627hf: Enable watchdog device only if not already enabled

There is no need to enable the watchdog device if it is already enabled.
Also, when enabling the watchdog device, only set the watchdog device
enable bit and do not touch other bits; depending on the chip type,
those bits may enable other functionality.

Signed-off-by: Guenter Roeck <[email protected]>
---
v2: No change

drivers/watchdog/w83627hf_wdt.c | 4 +++-
1 file changed, 3 insertions(+), 1 deletion(-)

diff --git a/drivers/watchdog/w83627hf_wdt.c b/drivers/watchdog/w83627hf_wdt.c
index bb19368..13bad31 100644
--- a/drivers/watchdog/w83627hf_wdt.c
+++ b/drivers/watchdog/w83627hf_wdt.c
@@ -101,7 +101,9 @@ static void w83627hf_init(struct watchdog_device *wdog)
}

outb_p(0x30, WDT_EFER); /* select CR30 */
- outb_p(0x01, WDT_EFDR); /* set bit 0 to activate GPIO2 */
+ t = inb(WDT_EFDR);
+ if (!(t & 0x01))
+ outb_p(t | 0x01, WDT_EFDR); /* set bit 0 to activate GPIO2 */

outb_p(0xF6, WDT_EFER); /* Select CRF6 */
t = inb_p(WDT_EFDR); /* read CRF6 */
--
1.7.9.7

2013-03-10 23:15:12

by Guenter Roeck

[permalink] [raw]
Subject: [PATCH v2 2/7] watchdog: w83627hf: Enable watchdog only once

It is unnecessary to enable the logical device and WDT0 each time
the watchdog is accessed. Do it only once during initialization.

Signed-off-by: Guenter Roeck <[email protected]>
---
v2: No change

drivers/watchdog/w83627hf_wdt.c | 35 +++++++++++++++++------------------
1 file changed, 17 insertions(+), 18 deletions(-)

diff --git a/drivers/watchdog/w83627hf_wdt.c b/drivers/watchdog/w83627hf_wdt.c
index 51a22ab..bb19368 100644
--- a/drivers/watchdog/w83627hf_wdt.c
+++ b/drivers/watchdog/w83627hf_wdt.c
@@ -66,28 +66,10 @@ MODULE_PARM_DESC(timeout,

static void w83627hf_select_wd_register(void)
{
- unsigned char c;
outb_p(0x87, WDT_EFER); /* Enter extended function mode */
outb_p(0x87, WDT_EFER); /* Again according to manual */
-
- outb(0x20, WDT_EFER); /* check chip version */
- c = inb(WDT_EFDR);
- if (c == 0x82) { /* W83627THF */
- outb_p(0x2b, WDT_EFER); /* select GPIO3 */
- c = ((inb_p(WDT_EFDR) & 0xf7) | 0x04); /* select WDT0 */
- outb_p(0x2b, WDT_EFER);
- outb_p(c, WDT_EFDR); /* set GPIO3 to WDT0 */
- } else if (c == 0x88 || c == 0xa0) { /* W83627EHF / W83627DHG */
- outb_p(0x2d, WDT_EFER); /* select GPIO5 */
- c = inb_p(WDT_EFDR) & ~0x01; /* PIN77 -> WDT0# */
- outb_p(0x2d, WDT_EFER);
- outb_p(c, WDT_EFDR); /* set GPIO5 to WDT0 */
- }
-
outb_p(0x07, WDT_EFER); /* point to logical device number reg */
outb_p(0x08, WDT_EFDR); /* select logical device 8 (GPIO2) */
- outb_p(0x30, WDT_EFER); /* select CR30 */
- outb_p(0x01, WDT_EFDR); /* set bit 0 to activate GPIO2 */
}

static void w83627hf_unselect_wd_register(void)
@@ -104,6 +86,23 @@ static void w83627hf_init(struct watchdog_device *wdog)

w83627hf_select_wd_register();

+ outb(0x20, WDT_EFER); /* check chip version */
+ t = inb(WDT_EFDR);
+ if (t == 0x82) { /* W83627THF */
+ outb_p(0x2b, WDT_EFER); /* select GPIO3 */
+ t = ((inb_p(WDT_EFDR) & 0xf7) | 0x04); /* select WDT0 */
+ outb_p(0x2b, WDT_EFER);
+ outb_p(t, WDT_EFDR); /* set GPIO3 to WDT0 */
+ } else if (t == 0x88 || t == 0xa0) { /* W83627EHF / W83627DHG */
+ outb_p(0x2d, WDT_EFER); /* select GPIO5 */
+ t = inb_p(WDT_EFDR) & ~0x01; /* PIN77 -> WDT0# */
+ outb_p(0x2d, WDT_EFER);
+ outb_p(t, WDT_EFDR); /* set GPIO5 to WDT0 */
+ }
+
+ outb_p(0x30, WDT_EFER); /* select CR30 */
+ outb_p(0x01, WDT_EFDR); /* set bit 0 to activate GPIO2 */
+
outb_p(0xF6, WDT_EFER); /* Select CRF6 */
t = inb_p(WDT_EFDR); /* read CRF6 */
if (t != 0) {
--
1.7.9.7

2013-03-10 23:16:12

by Guenter Roeck

[permalink] [raw]
Subject: [PATCH v2 4/7] watchdog: w83627hf: Use helper functions to access superio registers

Use helper functions named similar to other drivers to access
superio registers.

Request memory region only when needed, and use request_muxed_region().
This lets other devices (hwmon, gpio) use the same region.

Signed-off-by: Guenter Roeck <[email protected]>
---
v2: Request memory region only when needed

drivers/watchdog/w83627hf_wdt.c | 132 ++++++++++++++++++++++-----------------
1 file changed, 76 insertions(+), 56 deletions(-)

diff --git a/drivers/watchdog/w83627hf_wdt.c b/drivers/watchdog/w83627hf_wdt.c
index 13bad31..04cf20f 100644
--- a/drivers/watchdog/w83627hf_wdt.c
+++ b/drivers/watchdog/w83627hf_wdt.c
@@ -64,80 +64,102 @@ MODULE_PARM_DESC(timeout,
(same as EFER) */
#define WDT_EFDR (WDT_EFIR+1) /* Extended Function Data Register */

-static void w83627hf_select_wd_register(void)
+#define W83627HF_LD_WDT 0x08
+
+static void superio_outb(int reg, int val)
+{
+ outb(reg, WDT_EFER);
+ outb(val, WDT_EFDR);
+}
+
+static inline int superio_inb(int reg)
+{
+ outb(reg, WDT_EFER);
+ return inb(WDT_EFDR);
+}
+
+static int superio_enter(void)
{
+ if (!request_muxed_region(wdt_io, 2, WATCHDOG_NAME))
+ return -EBUSY;
+
outb_p(0x87, WDT_EFER); /* Enter extended function mode */
outb_p(0x87, WDT_EFER); /* Again according to manual */
- outb_p(0x07, WDT_EFER); /* point to logical device number reg */
- outb_p(0x08, WDT_EFDR); /* select logical device 8 (GPIO2) */
+
+ return 0;
}

-static void w83627hf_unselect_wd_register(void)
+static void superio_select(int ld)
+{
+ superio_outb(0x07, ld);
+}
+
+static void superio_exit(void)
{
outb_p(0xAA, WDT_EFER); /* Leave extended function mode */
+ release_region(wdt_io, 2);
}

/* tyan motherboards seem to set F5 to 0x4C ?
* So explicitly init to appropriate value. */

-static void w83627hf_init(struct watchdog_device *wdog)
+static int w83627hf_init(struct watchdog_device *wdog)
{
+ int ret;
unsigned char t;

- w83627hf_select_wd_register();
+ ret = superio_enter();
+ if (ret)
+ return ret;

- outb(0x20, WDT_EFER); /* check chip version */
- t = inb(WDT_EFDR);
+ superio_select(W83627HF_LD_WDT);
+ t = superio_inb(0x20); /* check chip version */
if (t == 0x82) { /* W83627THF */
- outb_p(0x2b, WDT_EFER); /* select GPIO3 */
- t = ((inb_p(WDT_EFDR) & 0xf7) | 0x04); /* select WDT0 */
- outb_p(0x2b, WDT_EFER);
- outb_p(t, WDT_EFDR); /* set GPIO3 to WDT0 */
+ t = (superio_inb(0x2b) & 0xf7);
+ superio_outb(0x2b, t | 0x04); /* set GPIO3 to WDT0 */
} else if (t == 0x88 || t == 0xa0) { /* W83627EHF / W83627DHG */
- outb_p(0x2d, WDT_EFER); /* select GPIO5 */
- t = inb_p(WDT_EFDR) & ~0x01; /* PIN77 -> WDT0# */
- outb_p(0x2d, WDT_EFER);
- outb_p(t, WDT_EFDR); /* set GPIO5 to WDT0 */
+ t = superio_inb(0x2d);
+ superio_outb(0x2d, t & ~0x01); /* set GPIO5 to WDT0 */
}

- outb_p(0x30, WDT_EFER); /* select CR30 */
- t = inb(WDT_EFDR);
+ /* set CR30 bit 0 to activate GPIO2 */
+ t = superio_inb(0x30);
if (!(t & 0x01))
- outb_p(t | 0x01, WDT_EFDR); /* set bit 0 to activate GPIO2 */
+ superio_outb(0x30, t | 0x01);

- outb_p(0xF6, WDT_EFER); /* Select CRF6 */
- t = inb_p(WDT_EFDR); /* read CRF6 */
+ t = superio_inb(0xF6);
if (t != 0) {
pr_info("Watchdog already running. Resetting timeout to %d sec\n",
wdog->timeout);
- outb_p(wdog->timeout, WDT_EFDR); /* Write back to CRF6 */
+ superio_outb(0xF6, wdog->timeout);
}

- outb_p(0xF5, WDT_EFER); /* Select CRF5 */
- t = inb_p(WDT_EFDR); /* read CRF5 */
- t &= ~0x0C; /* set second mode & disable keyboard
- turning off watchdog */
- t |= 0x02; /* enable the WDTO# output low pulse
- to the KBRST# pin (PIN60) */
- outb_p(t, WDT_EFDR); /* Write back to CRF5 */
-
- outb_p(0xF7, WDT_EFER); /* Select CRF7 */
- t = inb_p(WDT_EFDR); /* read CRF7 */
- t &= ~0xC0; /* disable keyboard & mouse turning off
- watchdog */
- outb_p(t, WDT_EFDR); /* Write back to CRF7 */
-
- w83627hf_unselect_wd_register();
+ /* set second mode & disable keyboard turning off watchdog */
+ t = superio_inb(0xF5) & ~0x0C;
+ /* enable the WDTO# output low pulse to the KBRST# pin */
+ t |= 0x02;
+ superio_outb(0xF5, t);
+
+ /* disable keyboard & mouse turning off watchdog */
+ t = superio_inb(0xF7) & ~0xC0;
+ superio_outb(0xF7, t);
+
+ superio_exit();
+
+ return 0;
}

static int wdt_set_time(unsigned int timeout)
{
- w83627hf_select_wd_register();
+ int ret;

- outb_p(0xF6, WDT_EFER); /* Select CRF6 */
- outb_p(timeout, WDT_EFDR); /* Write Timeout counter to CRF6 */
+ ret = superio_enter();
+ if (ret)
+ return ret;

- w83627hf_unselect_wd_register();
+ superio_select(W83627HF_LD_WDT);
+ superio_outb(0xF6, timeout);
+ superio_exit();

return 0;
}
@@ -163,13 +185,15 @@ static int wdt_set_timeout(struct watchdog_device *wdog, unsigned int timeout)
static unsigned int wdt_get_time(struct watchdog_device *wdog)
{
unsigned int timeleft;
+ int ret;

- w83627hf_select_wd_register();
-
- outb_p(0xF6, WDT_EFER); /* Select CRF6 */
- timeleft = inb_p(WDT_EFDR); /* Read Timeout counter to CRF6 */
+ ret = superio_enter();
+ if (ret)
+ return 0;

- w83627hf_unselect_wd_register();
+ superio_select(W83627HF_LD_WDT);
+ timeleft = superio_inb(0xF6);
+ superio_exit();

return timeleft;
}
@@ -228,20 +252,19 @@ static int __init wdt_init(void)

pr_info("WDT driver for the Winbond(TM) W83627HF/THF/HG/DHG Super I/O chip initialising\n");

- if (!request_region(wdt_io, 1, WATCHDOG_NAME)) {
- pr_err("I/O address 0x%04x already in use\n", wdt_io);
- return -EIO;
- }
-
watchdog_init_timeout(&wdt_dev, timeout, NULL);
watchdog_set_nowayout(&wdt_dev, nowayout);

- w83627hf_init(&wdt_dev);
+ ret = w83627hf_init(&wdt_dev);
+ if (ret) {
+ pr_err("failed to initialize watchdog (err=%d)\n", ret);
+ return ret;
+ }

ret = register_reboot_notifier(&wdt_notifier);
if (ret != 0) {
pr_err("cannot register reboot notifier (err=%d)\n", ret);
- goto unreg_regions;
+ return ret;
}

ret = watchdog_register_device(&wdt_dev);
@@ -255,8 +278,6 @@ static int __init wdt_init(void)

unreg_reboot:
unregister_reboot_notifier(&wdt_notifier);
-unreg_regions:
- release_region(wdt_io, 1);
return ret;
}

@@ -264,7 +285,6 @@ static void __exit wdt_exit(void)
{
watchdog_unregister_device(&wdt_dev);
unregister_reboot_notifier(&wdt_notifier);
- release_region(wdt_io, 1);
}

module_init(wdt_init);
--
1.7.9.7

2013-03-10 23:16:10

by Guenter Roeck

[permalink] [raw]
Subject: [PATCH v2 5/7] watchdog: w83627hf: Auto-detect IO address and supported chips

Instead of requiring the user to provide an IO address per module
parameter, auto-detect it as well as supported chips.

Signed-off-by: Guenter Roeck <[email protected]>
---
v2: Merge with patch 6/8
Add support for W83627S, W83627DHG-P, W83667HG, and NCT6779

drivers/watchdog/Kconfig | 15 +++-
drivers/watchdog/w83627hf_wdt.c | 182 ++++++++++++++++++++++++++++++++++-----
2 files changed, 173 insertions(+), 24 deletions(-)

diff --git a/drivers/watchdog/Kconfig b/drivers/watchdog/Kconfig
index dd462af..ef41028 100644
--- a/drivers/watchdog/Kconfig
+++ b/drivers/watchdog/Kconfig
@@ -854,13 +854,20 @@ config VIA_WDT
Most people will say N.

config W83627HF_WDT
- tristate "W83627HF/W83627DHG Watchdog Timer"
+ tristate "Watchdog timer for W83627HF/W83627DHG and compatibles"
depends on X86
select WATCHDOG_CORE
---help---
- This is the driver for the hardware watchdog on the W83627HF chipset
- as used in Advantech PC-9578 and Tyan S2721-533 motherboards
- (and likely others). The driver also supports the W83627DHG chip.
+ This is the driver for the hardware watchdog on the following
+ Super I/O chips.
+ W83627DHG/DHG-P/EHF/EHG/F/G/HF/S/SF/THF/UHG/UG
+ W83637HF
+ W83667HG/HG-B
+ W83687THF
+ NCT6775
+ NCT6776
+ NCT6779
+
This watchdog simply watches your kernel to make sure it doesn't
freeze, and if it does, it reboots your computer after a certain
amount of time.
diff --git a/drivers/watchdog/w83627hf_wdt.c b/drivers/watchdog/w83627hf_wdt.c
index 04cf20f..eb368f2 100644
--- a/drivers/watchdog/w83627hf_wdt.c
+++ b/drivers/watchdog/w83627hf_wdt.c
@@ -44,10 +44,11 @@
#define WATCHDOG_NAME "w83627hf/thf/hg/dhg WDT"
#define WATCHDOG_TIMEOUT 60 /* 60 sec default timeout */

-/* You must set this - there is no sane way to probe for this board. */
-static int wdt_io = 0x2E;
-module_param(wdt_io, int, 0);
-MODULE_PARM_DESC(wdt_io, "w83627hf/thf WDT io port (default 0x2E)");
+static int wdt_io;
+
+enum chips { w83627hf, w83627s, w83637hf, w83627thf, w83687thf,
+ w83627ehf, w83627dhg, w83627uhg, w83667hg, w83627dhg_p, w83667hg_b,
+ nct6775, nct6776, nct6779 };

static int timeout; /* in seconds */
module_param(timeout, int, 0);
@@ -66,6 +67,21 @@ MODULE_PARM_DESC(timeout,

#define W83627HF_LD_WDT 0x08

+#define W83627HF_ID 0x52
+#define W83627S_ID 0x59
+#define W83637HF_ID 0x70
+#define W83627THF_ID 0x82
+#define W83687THF_ID 0x85
+#define W83627EHF_ID 0x88
+#define W83627DHG_ID 0xa0
+#define W83627UHG_ID 0xa2
+#define W83667HG_ID 0xa5
+#define W83627DHG_P_ID 0xb0
+#define W83667HG_B_ID 0xb3
+#define NCT6775_ID 0xb4
+#define NCT6776_ID 0xc3
+#define NCT6779_ID 0xc5
+
static void superio_outb(int reg, int val)
{
outb(reg, WDT_EFER);
@@ -100,10 +116,7 @@ static void superio_exit(void)
release_region(wdt_io, 2);
}

-/* tyan motherboards seem to set F5 to 0x4C ?
- * So explicitly init to appropriate value. */
-
-static int w83627hf_init(struct watchdog_device *wdog)
+static int w83627hf_init(struct watchdog_device *wdog, enum chips chip)
{
int ret;
unsigned char t;
@@ -113,20 +126,59 @@ static int w83627hf_init(struct watchdog_device *wdog)
return ret;

superio_select(W83627HF_LD_WDT);
- t = superio_inb(0x20); /* check chip version */
- if (t == 0x82) { /* W83627THF */
- t = (superio_inb(0x2b) & 0xf7);
- superio_outb(0x2b, t | 0x04); /* set GPIO3 to WDT0 */
- } else if (t == 0x88 || t == 0xa0) { /* W83627EHF / W83627DHG */
- t = superio_inb(0x2d);
- superio_outb(0x2d, t & ~0x01); /* set GPIO5 to WDT0 */
- }

/* set CR30 bit 0 to activate GPIO2 */
t = superio_inb(0x30);
if (!(t & 0x01))
superio_outb(0x30, t | 0x01);

+ switch (chip) {
+ case w83627hf:
+ case w83627s:
+ t = superio_inb(0x2B) & ~0x10;
+ superio_outb(0x2B, t); /* set GPIO24 to WDT0 */
+ break;
+ case w83627thf:
+ t = (superio_inb(0x2B) & ~0x08) | 0x04;
+ superio_outb(0x2B, t); /* set GPIO3 to WDT0 */
+ break;
+ case w83627dhg:
+ case w83627dhg_p:
+ t = superio_inb(0x2D) & ~0x01; /* PIN77 -> WDT0# */
+ superio_outb(0x2D, t); /* set GPIO5 to WDT0 */
+ t = superio_inb(0xF5);
+ t |= 0x02; /* enable the WDTO# output low pulse
+ * to the KBRST# pin */
+ superio_outb(0xF5, t);
+ break;
+ case w83637hf:
+ break;
+ case w83687thf:
+ t = superio_inb(0x2C) & ~0x80; /* PIN47 -> WDT0# */
+ superio_outb(0x2C, t);
+ break;
+ case w83627ehf:
+ case w83627uhg:
+ case w83667hg:
+ case w83667hg_b:
+ case nct6775:
+ case nct6776:
+ case nct6779:
+ /*
+ * These chips have a fixed WDTO# output pin (W83627UHG),
+ * or support more than one WDTO# output pin.
+ * Don't touch its configuration, and hope the BIOS
+ * does the right thing.
+ */
+ t = superio_inb(0xF5);
+ t |= 0x02; /* enable the WDTO# output low pulse
+ * to the KBRST# pin */
+ superio_outb(0xF5, t);
+ break;
+ default:
+ break;
+ }
+
t = superio_inb(0xF6);
if (t != 0) {
pr_info("Watchdog already running. Resetting timeout to %d sec\n",
@@ -136,8 +188,6 @@ static int w83627hf_init(struct watchdog_device *wdog)

/* set second mode & disable keyboard turning off watchdog */
t = superio_inb(0xF5) & ~0x0C;
- /* enable the WDTO# output low pulse to the KBRST# pin */
- t |= 0x02;
superio_outb(0xF5, t);

/* disable keyboard & mouse turning off watchdog */
@@ -245,17 +295,109 @@ static struct notifier_block wdt_notifier = {
.notifier_call = wdt_notify_sys,
};

+static int wdt_find(int addr)
+{
+ u8 val;
+ int ret;
+
+ ret = superio_enter();
+ if (ret)
+ return ret;
+ superio_select(W83627HF_LD_WDT);
+ val = superio_inb(0x20);
+ switch (val) {
+ case W83627HF_ID:
+ ret = w83627hf;
+ break;
+ case W83627S_ID:
+ ret = w83627s;
+ break;
+ case W83637HF_ID:
+ ret = w83637hf;
+ break;
+ case W83627THF_ID:
+ ret = w83627thf;
+ break;
+ case W83687THF_ID:
+ ret = w83687thf;
+ break;
+ case W83627EHF_ID:
+ ret = w83627ehf;
+ break;
+ case W83627DHG_ID:
+ ret = w83627dhg;
+ break;
+ case W83627DHG_P_ID:
+ ret = w83627dhg_p;
+ break;
+ case W83627UHG_ID:
+ ret = w83627uhg;
+ break;
+ case W83667HG_ID:
+ ret = w83667hg;
+ break;
+ case W83667HG_B_ID:
+ ret = w83667hg_b;
+ break;
+ case NCT6775_ID:
+ ret = nct6775;
+ break;
+ case NCT6776_ID:
+ ret = nct6776;
+ break;
+ case NCT6779_ID:
+ ret = nct6779;
+ break;
+ case 0xff:
+ ret = -ENODEV;
+ break;
+ default:
+ ret = -ENODEV;
+ pr_err("Unsupported chip ID: 0x%02x\n", val);
+ break;
+ }
+ superio_exit();
+ return ret;
+}
+
static int __init wdt_init(void)
{
int ret;
bool nowayout = WATCHDOG_NOWAYOUT;
+ int chip;
+ const char * const chip_name[] = {
+ "W83627HF",
+ "W83627S",
+ "W83637HF",
+ "W83627THF",
+ "W83687THF",
+ "W83627EHF",
+ "W83627DHG",
+ "W83627UHG",
+ "W83667HG",
+ "W83667DHG-P",
+ "W83667HG-B",
+ "NCT6775",
+ "NCT6776",
+ "NCT6779",
+ };
+
+ wdt_io = 0x2e;
+ chip = wdt_find(0x2e);
+ if (chip < 0) {
+ wdt_io = 0x4e;
+ chip = wdt_find(0x4e);
+ if (chip < 0)
+ return chip;
+ }

- pr_info("WDT driver for the Winbond(TM) W83627HF/THF/HG/DHG Super I/O chip initialising\n");
+ pr_info("WDT driver for %s Super I/O chip initialising\n",
+ chip_name[chip]);

watchdog_init_timeout(&wdt_dev, timeout, NULL);
watchdog_set_nowayout(&wdt_dev, nowayout);

- ret = w83627hf_init(&wdt_dev);
+ ret = w83627hf_init(&wdt_dev, chip);
if (ret) {
pr_err("failed to initialize watchdog (err=%d)\n", ret);
return ret;
--
1.7.9.7

2013-03-10 23:16:09

by Guenter Roeck

[permalink] [raw]
Subject: [PATCH v2 7/7] watchdog: Remove drivers for W83697HF and W83697UG

Since both chips are now supported by the w83627hf watchdog driver,
the chip specific drivers are no longer needed and can be removed.

Signed-off-by: Guenter Roeck <[email protected]>
---
v2: Previously patch 8/8
No change

drivers/watchdog/Kconfig | 30 ---
drivers/watchdog/Makefile | 2 -
drivers/watchdog/w83697hf_wdt.c | 461 ---------------------------------------
drivers/watchdog/w83697ug_wdt.c | 398 ---------------------------------
4 files changed, 891 deletions(-)
delete mode 100644 drivers/watchdog/w83697hf_wdt.c
delete mode 100644 drivers/watchdog/w83697ug_wdt.c

diff --git a/drivers/watchdog/Kconfig b/drivers/watchdog/Kconfig
index bf04b38..d9ded4e 100644
--- a/drivers/watchdog/Kconfig
+++ b/drivers/watchdog/Kconfig
@@ -879,36 +879,6 @@ config W83627HF_WDT

Most people will say N.

-config W83697HF_WDT
- tristate "W83697HF/W83697HG Watchdog Timer"
- depends on X86
- ---help---
- This is the driver for the hardware watchdog on the W83697HF/HG
- chipset as used in Dedibox/VIA motherboards (and likely others).
- This watchdog simply watches your kernel to make sure it doesn't
- freeze, and if it does, it reboots your computer after a certain
- amount of time.
-
- To compile this driver as a module, choose M here: the
- module will be called w83697hf_wdt.
-
- Most people will say N.
-
-config W83697UG_WDT
- tristate "W83697UG/W83697UF Watchdog Timer"
- depends on X86
- ---help---
- This is the driver for the hardware watchdog on the W83697UG/UF
- chipset as used in MSI Fuzzy CX700 VIA motherboards (and likely others).
- This watchdog simply watches your kernel to make sure it doesn't
- freeze, and if it does, it reboots your computer after a certain
- amount of time.
-
- To compile this driver as a module, choose M here: the
- module will be called w83697ug_wdt.
-
- Most people will say N.
-
config W83877F_WDT
tristate "W83877F (EMACS) Watchdog Timer"
depends on X86
diff --git a/drivers/watchdog/Makefile b/drivers/watchdog/Makefile
index a300b94..b330b1f 100644
--- a/drivers/watchdog/Makefile
+++ b/drivers/watchdog/Makefile
@@ -103,8 +103,6 @@ obj-$(CONFIG_SMSC_SCH311X_WDT) += sch311x_wdt.o
obj-$(CONFIG_SMSC37B787_WDT) += smsc37b787_wdt.o
obj-$(CONFIG_VIA_WDT) += via_wdt.o
obj-$(CONFIG_W83627HF_WDT) += w83627hf_wdt.o
-obj-$(CONFIG_W83697HF_WDT) += w83697hf_wdt.o
-obj-$(CONFIG_W83697UG_WDT) += w83697ug_wdt.o
obj-$(CONFIG_W83877F_WDT) += w83877f_wdt.o
obj-$(CONFIG_W83977F_WDT) += w83977f_wdt.o
obj-$(CONFIG_MACHZ_WDT) += machzwd.o
diff --git a/drivers/watchdog/w83697hf_wdt.c b/drivers/watchdog/w83697hf_wdt.c
deleted file mode 100644
index cd9f3c1..0000000
--- a/drivers/watchdog/w83697hf_wdt.c
+++ /dev/null
@@ -1,461 +0,0 @@
-/*
- * w83697hf/hg WDT driver
- *
- * (c) Copyright 2006 Samuel Tardieu <[email protected]>
- * (c) Copyright 2006 Marcus Junker <[email protected]>
- *
- * Based on w83627hf_wdt.c which is based on advantechwdt.c
- * which is based on wdt.c.
- * Original copyright messages:
- *
- * (c) Copyright 2003 Pádraig Brady <[email protected]>
- *
- * (c) Copyright 2000-2001 Marek Michalkiewicz <[email protected]>
- *
- * (c) Copyright 1996 Alan Cox <[email protected]>,
- * All Rights Reserved.
- *
- * This program is free software; you can redistribute it and/or
- * modify it under the terms of the GNU General Public License
- * as published by the Free Software Foundation; either version
- * 2 of the License, or (at your option) any later version.
- *
- * Neither Marcus Junker nor ANDURAS AG admit liability nor provide
- * warranty for any of this software. This material is provided
- * "AS-IS" and at no charge.
- */
-
-#define pr_fmt(fmt) KBUILD_MODNAME ": " fmt
-
-#include <linux/module.h>
-#include <linux/moduleparam.h>
-#include <linux/types.h>
-#include <linux/miscdevice.h>
-#include <linux/watchdog.h>
-#include <linux/fs.h>
-#include <linux/ioport.h>
-#include <linux/notifier.h>
-#include <linux/reboot.h>
-#include <linux/init.h>
-#include <linux/spinlock.h>
-#include <linux/io.h>
-#include <linux/uaccess.h>
-
-
-#define WATCHDOG_NAME "w83697hf/hg WDT"
-#define WATCHDOG_TIMEOUT 60 /* 60 sec default timeout */
-#define WATCHDOG_EARLY_DISABLE 1 /* Disable until userland kicks in */
-
-static unsigned long wdt_is_open;
-static char expect_close;
-static DEFINE_SPINLOCK(io_lock);
-
-/* You must set this - there is no sane way to probe for this board. */
-static int wdt_io = 0x2e;
-module_param(wdt_io, int, 0);
-MODULE_PARM_DESC(wdt_io,
- "w83697hf/hg WDT io port (default 0x2e, 0 = autodetect)");
-
-static int timeout = WATCHDOG_TIMEOUT; /* in seconds */
-module_param(timeout, int, 0);
-MODULE_PARM_DESC(timeout,
- "Watchdog timeout in seconds. 1<= timeout <=255 (default="
- __MODULE_STRING(WATCHDOG_TIMEOUT) ")");
-
-static bool nowayout = WATCHDOG_NOWAYOUT;
-module_param(nowayout, bool, 0);
-MODULE_PARM_DESC(nowayout,
- "Watchdog cannot be stopped once started (default="
- __MODULE_STRING(WATCHDOG_NOWAYOUT) ")");
-
-static int early_disable = WATCHDOG_EARLY_DISABLE;
-module_param(early_disable, int, 0);
-MODULE_PARM_DESC(early_disable,
- "Watchdog gets disabled at boot time (default="
- __MODULE_STRING(WATCHDOG_EARLY_DISABLE) ")");
-
-/*
- * Kernel methods.
- */
-
-#define W83697HF_EFER (wdt_io + 0) /* Extended Function Enable Register */
-#define W83697HF_EFIR (wdt_io + 0) /* Extended Function Index Register
- (same as EFER) */
-#define W83697HF_EFDR (wdt_io + 1) /* Extended Function Data Register */
-
-static inline void w83697hf_unlock(void)
-{
- outb_p(0x87, W83697HF_EFER); /* Enter extended function mode */
- outb_p(0x87, W83697HF_EFER); /* Again according to manual */
-}
-
-static inline void w83697hf_lock(void)
-{
- outb_p(0xAA, W83697HF_EFER); /* Leave extended function mode */
-}
-
-/*
- * The three functions w83697hf_get_reg(), w83697hf_set_reg() and
- * w83697hf_write_timeout() must be called with the device unlocked.
- */
-
-static unsigned char w83697hf_get_reg(unsigned char reg)
-{
- outb_p(reg, W83697HF_EFIR);
- return inb_p(W83697HF_EFDR);
-}
-
-static void w83697hf_set_reg(unsigned char reg, unsigned char data)
-{
- outb_p(reg, W83697HF_EFIR);
- outb_p(data, W83697HF_EFDR);
-}
-
-static void w83697hf_write_timeout(int timeout)
-{
- /* Write Timeout counter to CRF4 */
- w83697hf_set_reg(0xF4, timeout);
-}
-
-static void w83697hf_select_wdt(void)
-{
- w83697hf_unlock();
- w83697hf_set_reg(0x07, 0x08); /* Switch to logic device 8 (GPIO2) */
-}
-
-static inline void w83697hf_deselect_wdt(void)
-{
- w83697hf_lock();
-}
-
-static void w83697hf_init(void)
-{
- unsigned char bbuf;
-
- w83697hf_select_wdt();
-
- bbuf = w83697hf_get_reg(0x29);
- bbuf &= ~0x60;
- bbuf |= 0x20;
-
- /* Set pin 119 to WDTO# mode (= CR29, WDT0) */
- w83697hf_set_reg(0x29, bbuf);
-
- bbuf = w83697hf_get_reg(0xF3);
- bbuf &= ~0x04;
- w83697hf_set_reg(0xF3, bbuf); /* Count mode is seconds */
-
- w83697hf_deselect_wdt();
-}
-
-static void wdt_ping(void)
-{
- spin_lock(&io_lock);
- w83697hf_select_wdt();
-
- w83697hf_write_timeout(timeout);
-
- w83697hf_deselect_wdt();
- spin_unlock(&io_lock);
-}
-
-static void wdt_enable(void)
-{
- spin_lock(&io_lock);
- w83697hf_select_wdt();
-
- w83697hf_write_timeout(timeout);
- w83697hf_set_reg(0x30, 1); /* Enable timer */
-
- w83697hf_deselect_wdt();
- spin_unlock(&io_lock);
-}
-
-static void wdt_disable(void)
-{
- spin_lock(&io_lock);
- w83697hf_select_wdt();
-
- w83697hf_set_reg(0x30, 0); /* Disable timer */
- w83697hf_write_timeout(0);
-
- w83697hf_deselect_wdt();
- spin_unlock(&io_lock);
-}
-
-static unsigned char wdt_running(void)
-{
- unsigned char t;
-
- spin_lock(&io_lock);
- w83697hf_select_wdt();
-
- t = w83697hf_get_reg(0xF4); /* Read timer */
-
- w83697hf_deselect_wdt();
- spin_unlock(&io_lock);
-
- return t;
-}
-
-static int wdt_set_heartbeat(int t)
-{
- if (t < 1 || t > 255)
- return -EINVAL;
-
- timeout = t;
- return 0;
-}
-
-static ssize_t wdt_write(struct file *file, const char __user *buf,
- size_t count, loff_t *ppos)
-{
- if (count) {
- if (!nowayout) {
- size_t i;
-
- expect_close = 0;
-
- for (i = 0; i != count; i++) {
- char c;
- if (get_user(c, buf + i))
- return -EFAULT;
- if (c == 'V')
- expect_close = 42;
- }
- }
- wdt_ping();
- }
- return count;
-}
-
-static long wdt_ioctl(struct file *file, unsigned int cmd, unsigned long arg)
-{
- void __user *argp = (void __user *)arg;
- int __user *p = argp;
- int new_timeout;
- static const struct watchdog_info ident = {
- .options = WDIOF_KEEPALIVEPING | WDIOF_SETTIMEOUT
- | WDIOF_MAGICCLOSE,
- .firmware_version = 1,
- .identity = "W83697HF WDT",
- };
-
- switch (cmd) {
- case WDIOC_GETSUPPORT:
- if (copy_to_user(argp, &ident, sizeof(ident)))
- return -EFAULT;
- break;
-
- case WDIOC_GETSTATUS:
- case WDIOC_GETBOOTSTATUS:
- return put_user(0, p);
-
- case WDIOC_SETOPTIONS:
- {
- int options, retval = -EINVAL;
-
- if (get_user(options, p))
- return -EFAULT;
-
- if (options & WDIOS_DISABLECARD) {
- wdt_disable();
- retval = 0;
- }
-
- if (options & WDIOS_ENABLECARD) {
- wdt_enable();
- retval = 0;
- }
-
- return retval;
- }
-
- case WDIOC_KEEPALIVE:
- wdt_ping();
- break;
-
- case WDIOC_SETTIMEOUT:
- if (get_user(new_timeout, p))
- return -EFAULT;
- if (wdt_set_heartbeat(new_timeout))
- return -EINVAL;
- wdt_ping();
- /* Fall */
-
- case WDIOC_GETTIMEOUT:
- return put_user(timeout, p);
-
- default:
- return -ENOTTY;
- }
- return 0;
-}
-
-static int wdt_open(struct inode *inode, struct file *file)
-{
- if (test_and_set_bit(0, &wdt_is_open))
- return -EBUSY;
- /*
- * Activate
- */
-
- wdt_enable();
- return nonseekable_open(inode, file);
-}
-
-static int wdt_close(struct inode *inode, struct file *file)
-{
- if (expect_close == 42)
- wdt_disable();
- else {
- pr_crit("Unexpected close, not stopping watchdog!\n");
- wdt_ping();
- }
- expect_close = 0;
- clear_bit(0, &wdt_is_open);
- return 0;
-}
-
-/*
- * Notifier for system down
- */
-
-static int wdt_notify_sys(struct notifier_block *this, unsigned long code,
- void *unused)
-{
- if (code == SYS_DOWN || code == SYS_HALT)
- wdt_disable(); /* Turn the WDT off */
-
- return NOTIFY_DONE;
-}
-
-/*
- * Kernel Interfaces
- */
-
-static const struct file_operations wdt_fops = {
- .owner = THIS_MODULE,
- .llseek = no_llseek,
- .write = wdt_write,
- .unlocked_ioctl = wdt_ioctl,
- .open = wdt_open,
- .release = wdt_close,
-};
-
-static struct miscdevice wdt_miscdev = {
- .minor = WATCHDOG_MINOR,
- .name = "watchdog",
- .fops = &wdt_fops,
-};
-
-/*
- * The WDT needs to learn about soft shutdowns in order to
- * turn the timebomb registers off.
- */
-
-static struct notifier_block wdt_notifier = {
- .notifier_call = wdt_notify_sys,
-};
-
-static int w83697hf_check_wdt(void)
-{
- if (!request_region(wdt_io, 2, WATCHDOG_NAME)) {
- pr_err("I/O address 0x%x already in use\n", wdt_io);
- return -EIO;
- }
-
- pr_debug("Looking for watchdog at address 0x%x\n", wdt_io);
- w83697hf_unlock();
- if (w83697hf_get_reg(0x20) == 0x60) {
- pr_info("watchdog found at address 0x%x\n", wdt_io);
- w83697hf_lock();
- return 0;
- }
- /* Reprotect in case it was a compatible device */
- w83697hf_lock();
-
- pr_info("watchdog not found at address 0x%x\n", wdt_io);
- release_region(wdt_io, 2);
- return -EIO;
-}
-
-static int w83697hf_ioports[] = { 0x2e, 0x4e, 0x00 };
-
-static int __init wdt_init(void)
-{
- int ret, i, found = 0;
-
- pr_info("WDT driver for W83697HF/HG initializing\n");
-
- if (wdt_io == 0) {
- /* we will autodetect the W83697HF/HG watchdog */
- for (i = 0; ((!found) && (w83697hf_ioports[i] != 0)); i++) {
- wdt_io = w83697hf_ioports[i];
- if (!w83697hf_check_wdt())
- found++;
- }
- } else {
- if (!w83697hf_check_wdt())
- found++;
- }
-
- if (!found) {
- pr_err("No W83697HF/HG could be found\n");
- ret = -EIO;
- goto out;
- }
-
- w83697hf_init();
- if (early_disable) {
- if (wdt_running())
- pr_warn("Stopping previously enabled watchdog until userland kicks in\n");
- wdt_disable();
- }
-
- if (wdt_set_heartbeat(timeout)) {
- wdt_set_heartbeat(WATCHDOG_TIMEOUT);
- pr_info("timeout value must be 1 <= timeout <= 255, using %d\n",
- WATCHDOG_TIMEOUT);
- }
-
- ret = register_reboot_notifier(&wdt_notifier);
- if (ret != 0) {
- pr_err("cannot register reboot notifier (err=%d)\n", ret);
- goto unreg_regions;
- }
-
- ret = misc_register(&wdt_miscdev);
- if (ret != 0) {
- pr_err("cannot register miscdev on minor=%d (err=%d)\n",
- WATCHDOG_MINOR, ret);
- goto unreg_reboot;
- }
-
- pr_info("initialized. timeout=%d sec (nowayout=%d)\n",
- timeout, nowayout);
-
-out:
- return ret;
-unreg_reboot:
- unregister_reboot_notifier(&wdt_notifier);
-unreg_regions:
- release_region(wdt_io, 2);
- goto out;
-}
-
-static void __exit wdt_exit(void)
-{
- misc_deregister(&wdt_miscdev);
- unregister_reboot_notifier(&wdt_notifier);
- release_region(wdt_io, 2);
-}
-
-module_init(wdt_init);
-module_exit(wdt_exit);
-
-MODULE_LICENSE("GPL");
-MODULE_AUTHOR("Marcus Junker <[email protected]>, "
- "Samuel Tardieu <[email protected]>");
-MODULE_DESCRIPTION("w83697hf/hg WDT driver");
-MODULE_ALIAS_MISCDEV(WATCHDOG_MINOR);
diff --git a/drivers/watchdog/w83697ug_wdt.c b/drivers/watchdog/w83697ug_wdt.c
deleted file mode 100644
index 274be0b..0000000
--- a/drivers/watchdog/w83697ug_wdt.c
+++ /dev/null
@@ -1,398 +0,0 @@
-/*
- * w83697ug/uf WDT driver
- *
- * (c) Copyright 2008 Flemming Fransen <[email protected]>
- * reused original code to support w83697ug/uf.
- *
- * Based on w83627hf_wdt.c which is based on advantechwdt.c
- * which is based on wdt.c.
- * Original copyright messages:
- *
- * (c) Copyright 2007 Vlad Drukker <[email protected]>
- * added support for W83627THF.
- *
- * (c) Copyright 2003 Pádraig Brady <[email protected]>
- *
- * (c) Copyright 2000-2001 Marek Michalkiewicz <[email protected]>
- *
- * (c) Copyright 1996 Alan Cox <[email protected]>, All Rights Reserved.
- * http://www.redhat.com
- *
- * This program is free software; you can redistribute it and/or
- * modify it under the terms of the GNU General Public License
- * as published by the Free Software Foundation; either version
- * 2 of the License, or (at your option) any later version.
- *
- * Neither Alan Cox nor CymruNet Ltd. admit liability nor provide
- * warranty for any of this software. This material is provided
- * "AS-IS" and at no charge.
- *
- * (c) Copyright 1995 Alan Cox <[email protected]>
- */
-
-#define pr_fmt(fmt) KBUILD_MODNAME ": " fmt
-
-#include <linux/module.h>
-#include <linux/moduleparam.h>
-#include <linux/types.h>
-#include <linux/miscdevice.h>
-#include <linux/watchdog.h>
-#include <linux/fs.h>
-#include <linux/ioport.h>
-#include <linux/notifier.h>
-#include <linux/reboot.h>
-#include <linux/init.h>
-#include <linux/spinlock.h>
-#include <linux/io.h>
-#include <linux/uaccess.h>
-
-
-#define WATCHDOG_NAME "w83697ug/uf WDT"
-#define WATCHDOG_TIMEOUT 60 /* 60 sec default timeout */
-
-static unsigned long wdt_is_open;
-static char expect_close;
-static DEFINE_SPINLOCK(io_lock);
-
-static int wdt_io = 0x2e;
-module_param(wdt_io, int, 0);
-MODULE_PARM_DESC(wdt_io, "w83697ug/uf WDT io port (default 0x2e)");
-
-static int timeout = WATCHDOG_TIMEOUT; /* in seconds */
-module_param(timeout, int, 0);
-MODULE_PARM_DESC(timeout,
- "Watchdog timeout in seconds. 1<= timeout <=255 (default="
- __MODULE_STRING(WATCHDOG_TIMEOUT) ")");
-
-static bool nowayout = WATCHDOG_NOWAYOUT;
-module_param(nowayout, bool, 0);
-MODULE_PARM_DESC(nowayout,
- "Watchdog cannot be stopped once started (default="
- __MODULE_STRING(WATCHDOG_NOWAYOUT) ")");
-
-/*
- * Kernel methods.
- */
-
-#define WDT_EFER (wdt_io+0) /* Extended Function Enable Registers */
-#define WDT_EFIR (wdt_io+0) /* Extended Function Index Register
- (same as EFER) */
-#define WDT_EFDR (WDT_EFIR+1) /* Extended Function Data Register */
-
-static int w83697ug_select_wd_register(void)
-{
- unsigned char c;
- unsigned char version;
-
- outb_p(0x87, WDT_EFER); /* Enter extended function mode */
- outb_p(0x87, WDT_EFER); /* Again according to manual */
-
- outb(0x20, WDT_EFER); /* check chip version */
- version = inb(WDT_EFDR);
-
- if (version == 0x68) { /* W83697UG */
- pr_info("Watchdog chip version 0x%02x = W83697UG/UF found at 0x%04x\n",
- version, wdt_io);
-
- outb_p(0x2b, WDT_EFER);
- c = inb_p(WDT_EFDR); /* select WDT0 */
- c &= ~0x04;
- outb_p(0x2b, WDT_EFER);
- outb_p(c, WDT_EFDR); /* set pin118 to WDT0 */
-
- } else {
- pr_err("No W83697UG/UF could be found\n");
- return -ENODEV;
- }
-
- outb_p(0x07, WDT_EFER); /* point to logical device number reg */
- outb_p(0x08, WDT_EFDR); /* select logical device 8 (GPIO2) */
- outb_p(0x30, WDT_EFER); /* select CR30 */
- c = inb_p(WDT_EFDR);
- outb_p(c | 0x01, WDT_EFDR); /* set bit 0 to activate GPIO2 */
-
- return 0;
-}
-
-static void w83697ug_unselect_wd_register(void)
-{
- outb_p(0xAA, WDT_EFER); /* Leave extended function mode */
-}
-
-static int w83697ug_init(void)
-{
- int ret;
- unsigned char t;
-
- ret = w83697ug_select_wd_register();
- if (ret != 0)
- return ret;
-
- outb_p(0xF6, WDT_EFER); /* Select CRF6 */
- t = inb_p(WDT_EFDR); /* read CRF6 */
- if (t != 0) {
- pr_info("Watchdog already running. Resetting timeout to %d sec\n",
- timeout);
- outb_p(timeout, WDT_EFDR); /* Write back to CRF6 */
- }
- outb_p(0xF5, WDT_EFER); /* Select CRF5 */
- t = inb_p(WDT_EFDR); /* read CRF5 */
- t &= ~0x0C; /* set second mode &
- disable keyboard turning off watchdog */
- outb_p(t, WDT_EFDR); /* Write back to CRF5 */
-
- w83697ug_unselect_wd_register();
- return 0;
-}
-
-static void wdt_ctrl(int timeout)
-{
- spin_lock(&io_lock);
-
- if (w83697ug_select_wd_register() < 0) {
- spin_unlock(&io_lock);
- return;
- }
-
- outb_p(0xF4, WDT_EFER); /* Select CRF4 */
- outb_p(timeout, WDT_EFDR); /* Write Timeout counter to CRF4 */
-
- w83697ug_unselect_wd_register();
-
- spin_unlock(&io_lock);
-}
-
-static int wdt_ping(void)
-{
- wdt_ctrl(timeout);
- return 0;
-}
-
-static int wdt_disable(void)
-{
- wdt_ctrl(0);
- return 0;
-}
-
-static int wdt_set_heartbeat(int t)
-{
- if (t < 1 || t > 255)
- return -EINVAL;
-
- timeout = t;
- return 0;
-}
-
-static ssize_t wdt_write(struct file *file, const char __user *buf,
- size_t count, loff_t *ppos)
-{
- if (count) {
- if (!nowayout) {
- size_t i;
-
- expect_close = 0;
-
- for (i = 0; i != count; i++) {
- char c;
- if (get_user(c, buf + i))
- return -EFAULT;
- if (c == 'V')
- expect_close = 42;
- }
- }
- wdt_ping();
- }
- return count;
-}
-
-static long wdt_ioctl(struct file *file, unsigned int cmd, unsigned long arg)
-{
- void __user *argp = (void __user *)arg;
- int __user *p = argp;
- int new_timeout;
- static const struct watchdog_info ident = {
- .options = WDIOF_KEEPALIVEPING |
- WDIOF_SETTIMEOUT |
- WDIOF_MAGICCLOSE,
- .firmware_version = 1,
- .identity = "W83697UG WDT",
- };
-
- switch (cmd) {
- case WDIOC_GETSUPPORT:
- if (copy_to_user(argp, &ident, sizeof(ident)))
- return -EFAULT;
- break;
-
- case WDIOC_GETSTATUS:
- case WDIOC_GETBOOTSTATUS:
- return put_user(0, p);
-
- case WDIOC_SETOPTIONS:
- {
- int options, retval = -EINVAL;
-
- if (get_user(options, p))
- return -EFAULT;
-
- if (options & WDIOS_DISABLECARD) {
- wdt_disable();
- retval = 0;
- }
-
- if (options & WDIOS_ENABLECARD) {
- wdt_ping();
- retval = 0;
- }
-
- return retval;
- }
-
- case WDIOC_KEEPALIVE:
- wdt_ping();
- break;
-
- case WDIOC_SETTIMEOUT:
- if (get_user(new_timeout, p))
- return -EFAULT;
- if (wdt_set_heartbeat(new_timeout))
- return -EINVAL;
- wdt_ping();
- /* Fall */
-
- case WDIOC_GETTIMEOUT:
- return put_user(timeout, p);
-
- default:
- return -ENOTTY;
- }
- return 0;
-}
-
-static int wdt_open(struct inode *inode, struct file *file)
-{
- if (test_and_set_bit(0, &wdt_is_open))
- return -EBUSY;
- /*
- * Activate
- */
-
- wdt_ping();
- return nonseekable_open(inode, file);
-}
-
-static int wdt_close(struct inode *inode, struct file *file)
-{
- if (expect_close == 42)
- wdt_disable();
- else {
- pr_crit("Unexpected close, not stopping watchdog!\n");
- wdt_ping();
- }
- expect_close = 0;
- clear_bit(0, &wdt_is_open);
- return 0;
-}
-
-/*
- * Notifier for system down
- */
-
-static int wdt_notify_sys(struct notifier_block *this, unsigned long code,
- void *unused)
-{
- if (code == SYS_DOWN || code == SYS_HALT)
- wdt_disable(); /* Turn the WDT off */
-
- return NOTIFY_DONE;
-}
-
-/*
- * Kernel Interfaces
- */
-
-static const struct file_operations wdt_fops = {
- .owner = THIS_MODULE,
- .llseek = no_llseek,
- .write = wdt_write,
- .unlocked_ioctl = wdt_ioctl,
- .open = wdt_open,
- .release = wdt_close,
-};
-
-static struct miscdevice wdt_miscdev = {
- .minor = WATCHDOG_MINOR,
- .name = "watchdog",
- .fops = &wdt_fops,
-};
-
-/*
- * The WDT needs to learn about soft shutdowns in order to
- * turn the timebomb registers off.
- */
-
-static struct notifier_block wdt_notifier = {
- .notifier_call = wdt_notify_sys,
-};
-
-static int __init wdt_init(void)
-{
- int ret;
-
- pr_info("WDT driver for the Winbond(TM) W83697UG/UF Super I/O chip initialising\n");
-
- if (wdt_set_heartbeat(timeout)) {
- wdt_set_heartbeat(WATCHDOG_TIMEOUT);
- pr_info("timeout value must be 1<=timeout<=255, using %d\n",
- WATCHDOG_TIMEOUT);
- }
-
- if (!request_region(wdt_io, 1, WATCHDOG_NAME)) {
- pr_err("I/O address 0x%04x already in use\n", wdt_io);
- ret = -EIO;
- goto out;
- }
-
- ret = w83697ug_init();
- if (ret != 0)
- goto unreg_regions;
-
- ret = register_reboot_notifier(&wdt_notifier);
- if (ret != 0) {
- pr_err("cannot register reboot notifier (err=%d)\n", ret);
- goto unreg_regions;
- }
-
- ret = misc_register(&wdt_miscdev);
- if (ret != 0) {
- pr_err("cannot register miscdev on minor=%d (err=%d)\n",
- WATCHDOG_MINOR, ret);
- goto unreg_reboot;
- }
-
- pr_info("initialized. timeout=%d sec (nowayout=%d)\n",
- timeout, nowayout);
-
-out:
- return ret;
-unreg_reboot:
- unregister_reboot_notifier(&wdt_notifier);
-unreg_regions:
- release_region(wdt_io, 1);
- goto out;
-}
-
-static void __exit wdt_exit(void)
-{
- misc_deregister(&wdt_miscdev);
- unregister_reboot_notifier(&wdt_notifier);
- release_region(wdt_io, 1);
-}
-
-module_init(wdt_init);
-module_exit(wdt_exit);
-
-MODULE_LICENSE("GPL");
-MODULE_AUTHOR("Flemming Frandsen <[email protected]>");
-MODULE_DESCRIPTION("w83697ug/uf WDT driver");
-MODULE_ALIAS_MISCDEV(WATCHDOG_MINOR);
--
1.7.9.7

2013-03-19 17:26:39

by Pádraig Brady

[permalink] [raw]
Subject: Re: [PATCH v2 2/7] watchdog: w83627hf: Enable watchdog only once

On 03/10/2013 11:14 PM, Guenter Roeck wrote:
> It is unnecessary to enable the logical device and WDT0 each time
> the watchdog is accessed. Do it only once during initialization.

Is this also the case on systems where the superio
chip is used for other things? I've the impression
that this may break some systems (though I no longer
have the hardware to test). Arbitration of multiple
users of the superio device may be managed be a central
user space app, or by a kernel level arbitrator.

thanks,
P?draig.

2013-03-19 20:02:12

by Guenter Roeck

[permalink] [raw]
Subject: Re: [PATCH v2 2/7] watchdog: w83627hf: Enable watchdog only once

On Tue, Mar 19, 2013 at 05:26:26PM +0000, P?draig Brady wrote:
> On 03/10/2013 11:14 PM, Guenter Roeck wrote:
> > It is unnecessary to enable the logical device and WDT0 each time
> > the watchdog is accessed. Do it only once during initialization.
>
> Is this also the case on systems where the superio
> chip is used for other things? I've the impression
> that this may break some systems (though I no longer
> have the hardware to test). Arbitration of multiple
> users of the superio device may be managed be a central
> user space app, or by a kernel level arbitrator.
>
Not sure if I understand you correctly.

You mean some entity might actually disable the watchdog between accesses
to it by the watchdog driver ? That would make it pretty useless.
Might as well turn it off entirely if that is the case.

Or do you refer to _selecting_ the hwmon logical device ? If so, this patch
is about enabling it only once, not about selecting it only once.

Guenter

2013-03-21 18:41:03

by Pádraig Brady

[permalink] [raw]
Subject: Re: [PATCH v2 2/7] watchdog: w83627hf: Enable watchdog only once

On 03/19/2013 08:02 PM, Guenter Roeck wrote:
> On Tue, Mar 19, 2013 at 05:26:26PM +0000, P?draig Brady wrote:
>> On 03/10/2013 11:14 PM, Guenter Roeck wrote:
>>> It is unnecessary to enable the logical device and WDT0 each time
>>> the watchdog is accessed. Do it only once during initialization.
>>
>> Is this also the case on systems where the superio
>> chip is used for other things? I've the impression
>> that this may break some systems (though I no longer
>> have the hardware to test). Arbitration of multiple
>> users of the superio device may be managed be a central
>> user space app, or by a kernel level arbitrator.
>>
> Not sure if I understand you correctly.
>
> You mean some entity might actually disable the watchdog between accesses
> to it by the watchdog driver ? That would make it pretty useless.
> Might as well turn it off entirely if that is the case.
>
> Or do you refer to _selecting_ the hwmon logical device ? If so, this patch
> is about enabling it only once, not about selecting it only once.

I meant selecting.
Enabling only once is fine.

sorry for the noise,

thanks,
P?draig.

2013-03-22 20:53:02

by Wim Van Sebroeck

[permalink] [raw]
Subject: Re: [PATCH v2 0/8] watchdog: w83627hf: Convert to watchdog infrastructure

Hi Guenter,

> Convert to watchdog infrastructure, cleanup, add support for additional
> chips, and merge with W83697HF and W83697UG watchdog drivers.
>
> Tested with W83627UHG, NCT6775, NCT6776. Additional test feedback
> for other chips would be appreciated.
>
> Original idea was to prepare the driver for use as a client to an MFD driver,
> but I found that requesting memory with request_muxed_region works just as well
> and has less impact. v2 includes the knowledge gained from converting the
> driver to an MFD client driver, but without the actual conversion.
>
> v2: Tested with W83627UHG
> Retain timeout module parameter; use watchdog_init_timeout to set it
> Eliminate some cosmetic changes
> Drop spinlock.h include
> Keep "initialized" message
> Don't try to configure WDTO for W83627UHG and W83627EHF
> Don't report the nowayout option with the 'initializing' message
> Use request_muxed_region to reserve memory range only while needed
> Add support for W83627S, W83627DHG-P, W83667HG, and NCT6779

In 2011 I started something similar but then with the MFD approach in mind.
Goal was also to clean-up the w836* watchdog drivers and get a clean driver that
supports all Winbond super-I/O based watchdog drivers.

I dug op the development code again. I'll post it in a next e-mail so that we can
see what the best way forward is. Note: I took the MFD approach because:
1) all superio shares the similar functions for using the Super-I/O registers.
2) Goal is to have low-level driver that support the specific super-I/O chipsets
and that does the platform stuff for hwmon, watchdog, gpio, ...

Kind regards,
Wim.

2013-03-22 21:09:28

by Wim Van Sebroeck

[permalink] [raw]
Subject: [RFC] winbond Super-I/O MFD driver

As said in previous mail, I would post my old
development code for the winbond super-I/O MFD
device. It's development code and it definitely
can be improved. Goal at this point is to see
which direction we should take (with super-i/o
device drivers in general imho).

Kind regards,
Wim.
---
commit dc8987180ad87da86740448f033b8cfe46ea90dd
Author: Wim Van Sebroeck <[email protected]>
Date: Fri Mar 22 22:02:16 2013 +0100

Sample Winbond Super-I/O MFD device consisting out of
a lowel-level driver that does the detection and creates
the platform-data and a watchdog driver.

diff --git a/drivers/mfd/Kconfig b/drivers/mfd/Kconfig
index c346941..943399f 100644
--- a/drivers/mfd/Kconfig
+++ b/drivers/mfd/Kconfig
@@ -1119,6 +1119,14 @@ config MFD_AS3711
help
Support for the AS3711 PMIC from AMS

+config MFD_SIO_WINBOND
+ tristate "Winbond SuperIO chips"
+ select MFD_CORE
+ depends on X86
+ help
+ Say yes here to enable support for various functions of the
+ Winbond Super IO chipsets.
+
endmenu
endif

diff --git a/drivers/mfd/Makefile b/drivers/mfd/Makefile
index b90409c..3c7673b 100644
--- a/drivers/mfd/Makefile
+++ b/drivers/mfd/Makefile
@@ -148,3 +148,4 @@ obj-$(CONFIG_MFD_LM3533) += lm3533-core.o lm3533-ctrlbank.o
obj-$(CONFIG_VEXPRESS_CONFIG) += vexpress-config.o vexpress-sysreg.o
obj-$(CONFIG_MFD_RETU) += retu-mfd.o
obj-$(CONFIG_MFD_AS3711) += as3711.o
+obj-$(CONFIG_MFD_SIO_WINBOND) += winbond-superio.o
diff --git a/drivers/mfd/winbond-superio.c b/drivers/mfd/winbond-superio.c
new file mode 100644
index 0000000..dccb633
--- /dev/null
+++ b/drivers/mfd/winbond-superio.c
@@ -0,0 +1,327 @@
+/*
+ winbond_superio.c - Driver for Winbond Super-I/O chipsets.
+
+ (c) Copyright 2013 Wim Van Sebroeck <[email protected]>.
+
+ This program is free software; you can redistribute it and/or modify
+ it under the terms of the GNU General Public License as published by
+ the Free Software Foundation; either version 2 of the License, or
+ (at your option) any later version.
+
+ This program is distributed in the hope that it will be useful,
+ but WITHOUT ANY WARRANTY; without even the implied warranty of
+ MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
+ GNU General Public License for more details.
+
+ You should have received a copy of the GNU General Public License
+ along with this program; if not, write to the Free Software
+ Foundation, Inc., 675 Mass Ave, Cambridge, MA 02139, USA.
+*/
+
+/*
+ Supports following chips:
+
+ Chip id rev
+ w83627hf 0x52 0x??
+ w83627hg 0x52 0x17
+ w83627hj 0x52 0x3A
+ w83627ud-a 0x52 0x41
+ w83627sf 0x59 0x5X
+ w83697hf 0x60 0x1X
+ w83697f 0x60 0x1X
+ w83697sf 0x68 0x0X,0x1X
+ w83697uf 0x68 0x1X
+ w83637hf 0x70 0x8X
+ w83627thf 0x82 0x83
+ w83687thf 0x85 0x??
+ w83627ehf 0x88 0x5X
+ w83627ehf 0x88 0x6X
+ w83627dhg 0xA0 0x2X
+ w83627uhg 0xA2 0x3X
+ w83667hg 0xA5 0x1X
+ w83627dhg-p 0xB0 0x7X
+ w83667hg-b 0xB3 0x5X
+ nct6775f 0xB4 0x7X
+ nct6776f 0xC3 0x3X
+ nct6779f 0xC5 0x6X
+*/
+
+#define pr_fmt(fmt) KBUILD_MODNAME ": " fmt
+
+#include <linux/module.h>
+#include <linux/init.h>
+#include <linux/platform_device.h>
+#include <linux/err.h>
+#include <linux/ioport.h>
+#include <linux/acpi.h>
+#include <linux/io.h>
+#include <linux/spinlock.h>
+#include <linux/mfd/superio.h>
+#include <linux/mfd/winbond_superio.h>
+
+static struct platform_device *pdev;
+
+enum chips {
+ w83627hf,
+ w83627hg,
+ w83627hj,
+ w83627uda,
+ w83627sf,
+ w83697hf,
+/* w83697f, */
+ w83697sf,
+ w83697uf,
+ w83637hf,
+ w83687thf,
+ w83627ehf,
+ w83627ehg,
+ w83627thf,
+ w83627dhg,
+ w83627uhg,
+ w83667hg,
+ w83627dhgp,
+ w83667hgb,
+ nct6775,
+ nct6776,
+ nct6779,
+};
+
+static DEFINE_SPINLOCK(iolock);
+
+static unsigned short force_id;
+module_param(force_id, ushort, 0);
+MODULE_PARM_DESC(force_id, "Override the detected device ID");
+
+inline void winbond_superio_enter(int ioreg)
+{
+ spin_lock(&iolock);
+ outb(0x87, ioreg);
+ outb(0x87, ioreg);
+}
+EXPORT_SYMBOL_GPL(winbond_superio_enter);
+
+inline void winbond_superio_exit(int ioreg)
+{
+ outb(0xAA, ioreg);
+ spin_unlock(&iolock);
+}
+EXPORT_SYMBOL_GPL(winbond_superio_exit);
+
+static int __init winbond_add_watchdog(
+ const struct winbond_sio_data *sio_data)
+{
+ struct resource res = {
+ .start = sio_data->ioreg,
+ .end = sio_data->ioreg + WINB_REGION_SIZE - 1,
+ .name = "winbond_wdt",
+ .flags = IORESOURCE_IO,
+ };
+ struct winbond_wdt_pdata wdt_pdata;
+ int err;
+
+ if (!request_region(res.start, WINB_REGION_SIZE, "winbond_superio")) {
+ pr_err("Failed to request region 0x%lx-0x%lx\n",
+ (unsigned long)res.start,
+ (unsigned long)res.end);
+ err = -EBUSY;
+ goto exit;
+ }
+
+ wdt_pdata.siodata.ioreg = sio_data->ioreg;
+ wdt_pdata.siodata.id = sio_data->id;
+
+ pdev = platform_device_register_resndata(NULL, "winbond_wdt", -1,
+ &res, 1, &wdt_pdata, sizeof(struct winbond_wdt_pdata));
+ if (IS_ERR(pdev)) {
+ err = PTR_ERR(pdev);
+ goto exit_release;
+ }
+
+ return 0;
+
+exit_release:
+ release_region(res.start, WINB_REGION_SIZE);
+exit:
+ return err;
+}
+
+static int __init winbond_sio_find(int sioaddr,
+ struct winbond_sio_data *sio_data)
+{
+ int err = -ENODEV;
+ enum chips type;
+ u16 val;
+
+ static const __initdata char *names[] = {
+ "W83627HF",
+ "W83627HG",
+ "W83627HJ",
+ "W83627UD-A",
+ "W83627SF",
+ "W83697HF",
+/* "W83697F",*/
+ "W83697SF",
+ "W83697UF",
+ "W83637HF",
+ "W83687THF",
+ "W83627EHF",
+ "W83627EHF",
+ "W83627THF",
+ "W83627DHG",
+ "W83627UHG",
+ "W83667HG",
+ "W83627DHG-P",
+ "W83667HG-B",
+ "NCT6775",
+ "NCT6776",
+ "NCT6779",
+ };
+
+ winbond_superio_enter(sioaddr);
+ if (force_id)
+ val = force_id;
+ else {
+ val = (superio_inb(sioaddr, SIO_REG_DEVID) << 8)
+ | superio_inb(sioaddr, SIO_REG_DEVID + 1);
+ }
+ sio_data->id = val & SIO_ID_MASK;
+ switch (val & SIO_ID_MASK) {
+ case SIO_W83627SF_ID: /* 0x595X */
+ type = w83627sf;
+ break;
+ case SIO_W83697HF_ID: /* 0x601X */
+ type = w83697hf;
+ break;
+ case SIO_W83697SF_ID: /* 0x680X */
+ type = w83697sf;
+ break;
+ case SIO_W83697UF_ID: /* 0x681X */
+ type = w83697uf;
+ break;
+ case SIO_W83637HF_ID: /* 0x708X */
+ type = w83637hf;
+ break;
+ case SIO_W83627THF_ID: /* 0x8283 */
+ type = w83627thf;
+ break;
+ case SIO_W83627EHF_ID: /* 0x885X */
+ type = w83627ehf;
+ break;
+ case SIO_W83627EHG_ID: /* 0x886X */
+ type = w83627ehg;
+ break;
+ case SIO_W83627DHG_ID: /* 0xA02X */
+ type = w83627dhg;
+ break;
+ case SIO_W83627UHG_ID: /* 0xA23X */
+ type = w83627uhg;
+ break;
+ case SIO_W83667HG_ID: /* 0xA51X */
+ type = w83667hg;
+ break;
+ case SIO_W83627DHG_P_ID:/* 0xB07X */
+ type = w83627dhgp;
+ break;
+ case SIO_W83667HG_B_ID: /* 0xB35X */
+ type = w83667hgb;
+ break;
+ case SIO_NCT6775_ID: /* 0xB47X */
+ type = nct6775;
+ break;
+ case SIO_NCT6776_ID: /* 0xC33X */
+ type = nct6776;
+ break;
+ case SIO_NCT6779_ID: /* 0xC56X */
+ type = nct6779;
+ break;
+ default:
+ switch ((val >> 8) & 0xFF) {
+ case 0x52: /* 0x52XX */
+ switch (val & 0xFF) {
+ case 0x17:
+ type = w83627hg;
+ sio_data->id = SIO_W83627HG_ID;
+ break;
+ case 0x3A:
+ type = w83627hj;
+ sio_data->id = SIO_W83627HJ_ID;
+ break;
+ case 0x41:
+ type = w83627uda;
+ sio_data->id = SIO_W83627UD_A_ID;
+ break;
+ default:
+ type = w83627hf;
+ sio_data->id = SIO_W83627HF_ID;
+ }
+ break;
+ case 0x85: /* 0x85XX */
+ type = w83687thf;
+ sio_data->id = SIO_W83687THF_ID;
+ break;
+ case 0xff: /* No device at all */
+ sio_data->id = 0;
+ goto exit;
+ default:
+ sio_data->id = val;
+ pr_debug("Unsupported chip (ID=0x%04x)\n", val);
+ goto exit;
+ }
+ }
+ sio_data->ioreg = sioaddr;
+ pr_info("Found %s chip\n", names[type]);
+ err = 0;
+ exit:
+ winbond_superio_exit(sioaddr);
+ return err;
+}
+
+static int __init init_winbond_superio(void)
+{
+ int err;
+ struct winbond_sio_data sio_data;
+
+ if (winbond_sio_find(0x2e, &sio_data)
+ && winbond_sio_find(0x4e, &sio_data))
+ return -ENODEV;
+
+ switch (sio_data.id) {
+ case SIO_W83627HF_ID:
+ case SIO_W83627HG_ID:
+ case SIO_W83627HJ_ID:
+ case SIO_W83627UD_A_ID:
+ case SIO_W83627SF_ID:
+ case SIO_W83697HF_ID:
+ case SIO_W83697SF_ID:
+ case SIO_W83697UF_ID:
+ case SIO_W83637HF_ID:
+ case SIO_W83627THF_ID:
+ case SIO_W83627EHF_ID:
+ case SIO_W83627EHG_ID:
+ case SIO_W83627DHG_ID:
+ case SIO_W83627UHG_ID:
+ case SIO_W83627DHG_P_ID:
+ err = winbond_add_watchdog(&sio_data);
+ if (err)
+ goto exit;
+ }
+
+ return 0;
+
+exit:
+ return err;
+}
+module_init(init_winbond_superio);
+
+static void __exit exit_winbond_superio(void)
+{
+ struct resource *res = platform_get_resource(pdev, IORESOURCE_IO, 0);
+
+ platform_device_unregister(pdev);
+ release_region(res->start, WINB_REGION_SIZE);
+}
+module_exit(exit_winbond_superio);
+
+MODULE_AUTHOR("Wim Van Sebroeck <[email protected]>");
+MODULE_DESCRIPTION("Winbond SuperIO Driver");
+MODULE_LICENSE("GPL");
diff --git a/drivers/watchdog/Kconfig b/drivers/watchdog/Kconfig
index 9fcc70c..6886adb 100644
--- a/drivers/watchdog/Kconfig
+++ b/drivers/watchdog/Kconfig
@@ -927,6 +927,18 @@ config W83977F_WDT
To compile this driver as a module, choose M here: the
module will be called w83977f_wdt.

+config WINBOND_WDT
+ tristate "Winbond Watchdog Timer"
+ depends on X86
+ select WINBOND_SIO
+ select WATCHDOG_CORE
+ ---help---
+ This is the driver for the hardware watchdog on the Winbond Super I/O
+ chipsets.
+
+ To compile this driver as a module, choose M here: the
+ module will be called winbond_wdt.
+
config MACHZ_WDT
tristate "ZF MachZ Watchdog"
depends on X86
diff --git a/drivers/watchdog/Makefile b/drivers/watchdog/Makefile
index a300b94..fb46d37 100644
--- a/drivers/watchdog/Makefile
+++ b/drivers/watchdog/Makefile
@@ -107,6 +107,7 @@ obj-$(CONFIG_W83697HF_WDT) += w83697hf_wdt.o
obj-$(CONFIG_W83697UG_WDT) += w83697ug_wdt.o
obj-$(CONFIG_W83877F_WDT) += w83877f_wdt.o
obj-$(CONFIG_W83977F_WDT) += w83977f_wdt.o
+obj-$(CONFIG_WINBOND_WDT) += winbond_wdt.o
obj-$(CONFIG_MACHZ_WDT) += machzwd.o
obj-$(CONFIG_SBC_EPX_C3_WATCHDOG) += sbc_epx_c3.o
obj-$(CONFIG_INTEL_SCU_WATCHDOG) += intel_scu_watchdog.o
diff --git a/drivers/watchdog/winbond_wdt.c b/drivers/watchdog/winbond_wdt.c
new file mode 100644
index 0000000..64487e2
--- /dev/null
+++ b/drivers/watchdog/winbond_wdt.c
@@ -0,0 +1,284 @@
+/*
+ * Winbond(TM) Watch Dog Timer driver
+ *
+ * (c) Copyright 2003,2007 P?draig Brady <[email protected]>
+ * (c) Copyright 2013 Wim Van Sebroeck <[email protected]>
+ *
+ * This program is free software; you can redistribute it and/or modify
+ * it under the terms of the GNU General Public License as published by
+ * the Free Software Foundation; either version 2 of the License, or
+ * (at your option) any later version.
+ *
+ * This program is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
+ * GNU General Public License for more details.
+ *
+ * You should have received a copy of the GNU General Public License
+ * along with this program; if not, write to the Free Software
+ * Foundation, Inc., 675 Mass Ave, Cambridge, MA 02139, USA.
+ */
+
+#define pr_fmt(fmt) KBUILD_MODNAME ": " fmt
+
+#include <linux/module.h>
+#include <linux/moduleparam.h>
+#include <linux/types.h>
+#include <linux/errno.h>
+#include <linux/miscdevice.h>
+#include <linux/watchdog.h>
+#include <linux/init.h>
+#include <linux/platform_device.h>
+#include <linux/mfd/superio.h>
+#include <linux/mfd/winbond_superio.h>
+
+#define WATCHDOG_TIMEOUT 60 /* 60 sec default timeout */
+static unsigned int timeout = WATCHDOG_TIMEOUT; /* in seconds */
+module_param(timeout, uint, 0);
+MODULE_PARM_DESC(timeout,
+ "Watchdog timeout in seconds. 1 <= timeout <= 255, default="
+ __MODULE_STRING(WATCHDOG_TIMEOUT) ".");
+
+static bool nowayout = WATCHDOG_NOWAYOUT;
+module_param(nowayout, bool, 0);
+MODULE_PARM_DESC(nowayout,
+ "Watchdog cannot be stopped once started (default="
+ __MODULE_STRING(WATCHDOG_NOWAYOUT) ")");
+
+#define WATCHDOG_EARLY_DISABLE 1 /* Disable until userland kicks in */
+static int early_disable = WATCHDOG_EARLY_DISABLE;
+module_param(early_disable, int, 0);
+MODULE_PARM_DESC(early_disable,
+ "Watchdog gets disabled at boot time (default="
+ __MODULE_STRING(WATCHDOG_EARLY_DISABLE) ")");
+
+static void winbond_wdt_set_timer(struct winbond_wdt_pdata *wdt_pdata,
+ unsigned char timeout)
+{
+ unsigned char val;
+ int ioreg = wdt_pdata->siodata.ioreg;
+
+ winbond_superio_enter(ioreg);
+ superio_select(ioreg, 0x08);
+ superio_outb(ioreg, wdt_pdata->wdt_counter_reg, timeout);
+ winbond_superio_exit(ioreg);
+}
+
+static int winbond_wdt_start(struct watchdog_device *wdd)
+{
+ struct winbond_wdt_pdata *wdt_pdata = watchdog_get_drvdata(wdd);
+
+ winbond_wdt_set_timer(wdt_pdata, wdd->timeout);
+ return 0;
+}
+
+static int winbond_wdt_stop(struct watchdog_device *wdd)
+{
+ struct winbond_wdt_pdata *wdt_pdata = watchdog_get_drvdata(wdd);
+
+ winbond_wdt_set_timer(wdt_pdata, 0x00);
+ return 0;
+}
+
+static int winbond_wdt_set_timeout(struct watchdog_device *wdd,
+ unsigned int t)
+{
+ wdd->timeout = t;
+ return 0;
+}
+
+#define OPTIONS (WDIOF_SETTIMEOUT | WDIOF_KEEPALIVEPING | WDIOF_MAGICCLOSE)
+
+static const struct watchdog_info winbond_wdt_ident = {
+ .options = OPTIONS,
+ .firmware_version = 0,
+ .identity = "Winbond Watchdog",
+};
+
+static struct watchdog_ops winbond_wdt_ops = {
+ .owner = THIS_MODULE,
+ .start = winbond_wdt_start,
+ .stop = winbond_wdt_stop,
+ .set_timeout = winbond_wdt_set_timeout,
+};
+
+static struct watchdog_device winbond_wdd = {
+ .info = &winbond_wdt_ident,
+ .ops = &winbond_wdt_ops,
+ .timeout = WATCHDOG_TIMEOUT,
+ .min_timeout = 1,
+ .max_timeout = 255,
+};
+static int winbond_wdt_probe(struct platform_device *pdev)
+{
+ int ret;
+ unsigned char val;
+ struct winbond_wdt_pdata *pdata;
+
+ pdata = pdev->dev.platform_data;
+ if (!pdata) {
+ dev_err(&pdev->dev, "no platform data supplied\n");
+ return -ENODEV;
+ }
+
+ /* Set the timeout value from the module parameter */
+ if (watchdog_init_timeout(&winbond_wdd, timeout, &pdev->dev)) {
+ dev_warn(&pdev->dev,
+ "timout value must be 1<=timeout<=255, using %d\n",
+ WATCHDOG_TIMEOUT);
+ }
+
+ winbond_superio_enter(pdata->siodata.ioreg);
+ /* Multiplex WDTO as the output pin */
+ switch (pdata->siodata.id) {
+ case SIO_W83627HF_ID:
+ case SIO_W83627SF_ID:
+ case SIO_W83637HF_ID:
+ /* WDTO is multiplexed PIN89S. CR2B bit 4 = 0 */
+ val = superio_inb(pdata->siodata.ioreg, 0x2B);
+ val &= ~0x10;
+ superio_outb(pdata->siodata.ioreg, 0x2B, val);
+ break;
+ case SIO_W83627THF_ID:
+ /* WDTO is multiplexed PIN89S1+S0. CR2B Bit 3,2 = 01 */
+ val = superio_inb(pdata->siodata.ioreg, 0x2B);
+ val &= ~0x08;
+ val |= 0x04;
+ superio_outb(pdata->siodata.ioreg, 0x2B, val);
+ break;
+ case SIO_W83627EHF_ID:
+ case SIO_W83627EHG_ID:
+ case SIO_W83627DHG_ID:
+ case SIO_W83627DHG_P_ID:
+ /* WDTO is multiplexed PIN77. CR2D bit 0 = 0 */
+ val = superio_inb(pdata->siodata.ioreg, 0x2D);
+ val &= ~0x01;
+ superio_outb(pdata->siodata.ioreg, 0x2D, val);
+ break;
+ case SIO_W83697HF_ID:
+ /* WDTO is multiplexed PIN119. CR29 bit 6,5 = 01 */
+ val = superio_inb(pdata->siodata.ioreg, 0x29);
+ val &= ~0x40;
+ val |= 0x20;
+ superio_outb(pdata->siodata.ioreg, 0x29, val);
+ break;
+ case SIO_W83697UF_ID:
+ /* WDTO is multiplexed PIN118. CR2B bit 2 = 0 */
+ val = superio_inb(pdata->siodata.ioreg, 0x2B);
+ val &= ~0x04;
+ superio_outb(pdata->siodata.ioreg, 0x2B, val);
+ break;
+ }
+ /* Select Logical Device 8 */
+ superio_select(pdata->siodata.ioreg, 0x08);
+ /* Activate the Logical Device if not already active */
+ val = superio_inb(pdata->siodata.ioreg, WINB_ACT_REG);
+ if (!(val & 0x01)) {
+ dev_warn(&pdev->dev, "Enabling Winbond WDT logical device\n");
+ superio_outb(pdata->siodata.ioreg, WINB_ACT_REG, val | 0x01);
+ }
+ /* Configure Registers for WDTO usage */
+ switch (pdata->siodata.id) {
+ case SIO_W83627HF_ID:
+ case SIO_W83627SF_ID:
+ case SIO_W83637HF_ID:
+ case SIO_W83627THF_ID:
+ case SIO_W83627EHF_ID:
+ case SIO_W83627EHG_ID:
+ case SIO_W83627DHG_ID:
+ case SIO_W83627UHG_ID:
+ case SIO_W83627DHG_P_ID:
+ /* CRF6 = WDTO# Counter Register */
+ pdata->wdt_counter_reg = 0xF6;
+ /*
+ * CRF5 = PLED mode Register
+ * Bit 3: 0 = WDTO count mode select: Seconds
+ * Bit 2: 0 = Disable Keyboard Reset stopping WDT
+ * Bit 1: 1 = Enable the WDTO# output low pulse to the
+ * KBRST# pin
+ */
+ val = superio_inb(pdata->siodata.ioreg, 0xF5);
+ val &= ~0x0C;
+ val |= 0x02;
+ superio_outb(pdata->siodata.ioreg, 0xF5, val);
+ /*
+ * CRF7 = Watchdog Control/Status Register
+ * Bit 7: 0 = Watch Dog Timer not affected by Mouse int.
+ * Bit 7: 0 = Watch Dog Timer not affected by Keyb. int.
+ */
+ val = superio_inb(pdata->siodata.ioreg, 0xF7);
+ val &= ~0xC0;
+ superio_outb(pdata->siodata.ioreg, 0xF7, val);
+ break;
+ case SIO_W83697HF_ID:
+ case SIO_W83697UF_ID:
+ /* CRF4 = WDTO# Counter Register */
+ pdata->wdt_counter_reg = 0xF4;
+ /*
+ * CRF3 = PLED mode Register
+ * Bit 2: 0 = WDTO count mode select: Seconds
+ */
+ val = superio_inb(pdata->siodata.ioreg, 0xF3);
+ val &= ~0x04;
+ superio_outb(pdata->siodata.ioreg, 0xF3, val);
+ break;
+ }
+ /* Take action if watchdog timer is already running */
+ val = superio_inb(pdata->siodata.ioreg, pdata->wdt_counter_reg);
+ if (val) {
+ if (early_disable) {
+ winbond_wdt_set_timer(pdata, 0x00);
+ dev_info(&pdev->dev, "Stopping running watchdog\n");
+ } else
+ dev_warn(&pdev->dev, "Watchdog is running!\n");
+ }
+ winbond_superio_exit(pdata->siodata.ioreg);
+
+ watchdog_set_nowayout(&winbond_wdd, nowayout);
+ watchdog_set_drvdata(&winbond_wdd, pdata);
+ ret = watchdog_register_device(&winbond_wdd);
+ if (ret) {
+ dev_err(&pdev->dev, "cannot register watchdog (%d)\n", ret);
+ goto err;
+ }
+
+ dev_info(&pdev->dev,
+ "initialized. timeout=%d sec (nowayout=%d, early_disable=%d)\n",
+ winbond_wdd.timeout, nowayout, early_disable);
+
+ return 0;
+
+err:
+ return ret;
+}
+
+static int winbond_wdt_remove(struct platform_device *pdev)
+{
+ watchdog_unregister_device(&winbond_wdd);
+ return 0;
+}
+
+static void winbond_wdt_shutdown(struct platform_device *pdev)
+{
+ struct winbond_wdt_pdata *pdata = pdev->dev.platform_data;
+
+ winbond_wdt_set_timer(pdata, 0x00);
+}
+
+static struct platform_driver winbond_wdt_driver = {
+ .probe = winbond_wdt_probe,
+ .remove = winbond_wdt_remove,
+ .shutdown = winbond_wdt_shutdown,
+ .driver = {
+ .owner = THIS_MODULE,
+ .name = "winbond_wdt",
+ },
+};
+module_platform_driver(winbond_wdt_driver);
+
+MODULE_AUTHOR("P?draig Brady <[email protected]>");
+MODULE_AUTHOR("Wim Van Sebroeck <[email protected]>");
+MODULE_DESCRIPTION("Winbond Watch Dog Timer driver");
+MODULE_LICENSE("GPL");
+MODULE_ALIAS_MISCDEV(WATCHDOG_MINOR);
+MODULE_ALIAS("platform:winbond_wdt");
diff --git a/include/linux/mfd/superio.h b/include/linux/mfd/superio.h
new file mode 100644
index 0000000..4b6dee5
--- /dev/null
+++ b/include/linux/mfd/superio.h
@@ -0,0 +1,27 @@
+
+#ifndef __SUPERIO_MFD_H
+#define __SUPERIO_MFD_H
+
+#include <linux/io.h>
+
+#define SIO_LD_SEL 0x07 /* Super-I/O Logical Device Select */
+
+inline void superio_outb(int ioreg, int reg, int val)
+{
+ outb(reg, ioreg);
+ outb(val, ioreg + 1);
+}
+
+inline int superio_inb(int ioreg, int reg)
+{
+ outb(reg, ioreg);
+ return inb(ioreg + 1);
+}
+
+inline void superio_select(int ioreg, int ld)
+{
+ outb(SIO_LD_SEL, ioreg);
+ outb(ld, ioreg + 1);
+}
+
+#endif /* __SUPERIO_MFD_H */
diff --git a/include/linux/mfd/winbond_superio.h b/include/linux/mfd/winbond_superio.h
new file mode 100644
index 0000000..745b247
--- /dev/null
+++ b/include/linux/mfd/winbond_superio.h
@@ -0,0 +1,63 @@
+/*
+ * Definitions and platform data for Winbond(tm) Super-I/O
+ * chipsets (HWMON & Watchdog)
+ *
+ * Licensed under the GPL-2 or later.
+ */
+
+#ifndef __LINUX_MFD_WINBOND_SUPERIO_H
+#define __LINUX_MFD_WINBOND_SUPERIO_H
+
+#define WINB_REGION_SIZE 2
+
+/* Global Control Registers */
+#define SIO_REG_LDSEL 0x07 /* Logical device select */
+#define SIO_REG_DEVID 0x20 /* Device ID (2 bytes) */
+#define SIO_REG_ENABLE 0x30 /* Logical device enable */
+#define SIO_REG_ADDR 0x60 /* Logical device address (2 bytes) */
+
+/* Device ID values */
+#define SIO_W83627HF_ID 0x5200 /* w83627hf */
+#define SIO_W83627HG_ID 0x5217 /* w83627hg */
+#define SIO_W83627HJ_ID 0x523a /* w83627hj */
+#define SIO_W83627UD_A_ID 0x5241 /* w83627ud-a */
+#define SIO_W83627SF_ID 0x5950 /* w83627sf */
+#define SIO_W83697HF_ID 0x6010 /* w83697hf, w83697f */
+#define SIO_W83697SF_ID 0x6800 /* w83697sf */
+#define SIO_W83697UF_ID 0x6810 /* w83697sf, w83697uf */
+#define SIO_W83637HF_ID 0x7080 /* w83637hf */
+#define SIO_W83627THF_ID 0x8283 /* w83627thf */
+#define SIO_W83687THF_ID 0x8500 /* w83687thf */
+#define SIO_W83627EHF_ID 0x8850 /* w83627ehf */
+#define SIO_W83627EHG_ID 0x8860 /* w83627ehf */
+#define SIO_W83627DHG_ID 0xa020 /* w83627dhg */
+#define SIO_W83627UHG_ID 0xa230 /* w83627uhg */
+#define SIO_W83667HG_ID 0xa510 /* w83667hg */
+#define SIO_W83627DHG_P_ID 0xb070 /* w83627dhg-p */
+#define SIO_W83667HG_B_ID 0xb350 /* w83667hg-b */
+#define SIO_NCT6775_ID 0xb470 /* nct6775 */
+#define SIO_NCT6776_ID 0xc330 /* nct6776 */
+#define SIO_NCT6779_ID 0xc560 /* nct6779 */
+#define SIO_ID_MASK 0xFFF0
+
+/* Configuration Registers */
+#define WINB_ACT_REG 0x30 /* Device Activation Register */
+
+struct winbond_sio_data {
+ int ioreg;
+ unsigned short id;
+};
+
+struct winbond_wdt_pdata {
+ struct winbond_sio_data siodata;
+ unsigned char wdt_counter_reg;
+};
+
+/*
+ * Winbond Super-I/O functions
+ */
+
+extern inline void winbond_superio_enter(int ioreg);
+extern inline void winbond_superio_exit(int ioreg);
+
+#endif /* __LINUX_MFD_WINBOND_SUPERIO_H */

2013-03-23 00:28:08

by Guenter Roeck

[permalink] [raw]
Subject: Re: [PATCH v2 0/8] watchdog: w83627hf: Convert to watchdog infrastructure

On Fri, Mar 22, 2013 at 09:52:58PM +0100, Wim Van Sebroeck wrote:
> Hi Guenter,
>
> > Convert to watchdog infrastructure, cleanup, add support for additional
> > chips, and merge with W83697HF and W83697UG watchdog drivers.
> >
> > Tested with W83627UHG, NCT6775, NCT6776. Additional test feedback
> > for other chips would be appreciated.
> >
> > Original idea was to prepare the driver for use as a client to an MFD driver,
> > but I found that requesting memory with request_muxed_region works just as well
> > and has less impact. v2 includes the knowledge gained from converting the
> > driver to an MFD client driver, but without the actual conversion.
> >
> > v2: Tested with W83627UHG
> > Retain timeout module parameter; use watchdog_init_timeout to set it
> > Eliminate some cosmetic changes
> > Drop spinlock.h include
> > Keep "initialized" message
> > Don't try to configure WDTO for W83627UHG and W83627EHF
> > Don't report the nowayout option with the 'initializing' message
> > Use request_muxed_region to reserve memory range only while needed
> > Add support for W83627S, W83627DHG-P, W83667HG, and NCT6779
>
> In 2011 I started something similar but then with the MFD approach in mind.
> Goal was also to clean-up the w836* watchdog drivers and get a clean driver that
> supports all Winbond super-I/O based watchdog drivers.
>
> I dug op the development code again. I'll post it in a next e-mail so that we can
> see what the best way forward is. Note: I took the MFD approach because:
> 1) all superio shares the similar functions for using the Super-I/O registers.
> 2) Goal is to have low-level driver that support the specific super-I/O chipsets
> and that does the platform stuff for hwmon, watchdog, gpio, ...
>
Hi Wim,

I started with a similar approach, only I used mfd cells to pass on platform
specific information such as the device type and the superio base address.
I still have the patchset for the mfd driver, in case you are interested.
My code is based on the patches submitted by Rodolfo Giometti a couple
of years ago. Want me to post it ?

What I noticed in my testing is that the superio address range (2e or 4e),
which the drivers currently take as granted, is at least on my systems
(all three of them) reserved by ACPI. Unfortunately that means one can not
use the mfd infrastructure to pass on the superio memory region,
since it checks for acpi conflicts. With that I gave up on the idea and
reverted to using request_muxed_region. That seemed simpler and accomplish
the same as long as all drivers actually use it.

Ultimately, I am open to either approach.

Thanks,
Guenter

2013-03-23 12:57:48

by Wim Van Sebroeck

[permalink] [raw]
Subject: Re: [PATCH v2 0/8] watchdog: w83627hf: Convert to watchdog infrastructure

Hi Guenter,

> > In 2011 I started something similar but then with the MFD approach in mind.
> > Goal was also to clean-up the w836* watchdog drivers and get a clean driver that
> > supports all Winbond super-I/O based watchdog drivers.
> >
> > I dug op the development code again. I'll post it in a next e-mail so that we can
> > see what the best way forward is. Note: I took the MFD approach because:
> > 1) all superio shares the similar functions for using the Super-I/O registers.
> > 2) Goal is to have low-level driver that support the specific super-I/O chipsets
> > and that does the platform stuff for hwmon, watchdog, gpio, ...
> >
> Hi Wim,
>
> I started with a similar approach, only I used mfd cells to pass on platform
> specific information such as the device type and the superio base address.
> I still have the patchset for the mfd driver, in case you are interested.
> My code is based on the patches submitted by Rodolfo Giometti a couple
> of years ago. Want me to post it ?

If it's not v1 then I am interested.
I think it depends on the super-I/O chipset of what info you can pass. I would
not use mfd cells for the winbond driver but use platform data (similar to
drivers/mfd/adp5520.c) because you don't need more then just pass some data.

> What I noticed in my testing is that the superio address range (2e or 4e),
> which the drivers currently take as granted, is at least on my systems
> (all three of them) reserved by ACPI. Unfortunately that means one can not
> use the mfd infrastructure to pass on the superio memory region,
> since it checks for acpi conflicts. With that I gave up on the idea and
> reverted to using request_muxed_region. That seemed simpler and accomplish
> the same as long as all drivers actually use it.

Noticed the same. That's why passing the platform data together with the
superio-address and type seems the best way to go. I also kept the superio_enter
and superio_exit a function in the low level driver because I used a lock to
make sure that the hwmon code doesn't start doing things when the watchdog is
doing things. But I think that the request_muxed_region is doing something similar
also.

Kind regards,
Wim.

2013-03-23 22:43:45

by Guenter Roeck

[permalink] [raw]
Subject: Re: [PATCH v2 0/8] watchdog: w83627hf: Convert to watchdog infrastructure

On Sat, Mar 23, 2013 at 01:57:42PM +0100, Wim Van Sebroeck wrote:
> Hi Guenter,
>
> > > In 2011 I started something similar but then with the MFD approach in mind.
> > > Goal was also to clean-up the w836* watchdog drivers and get a clean driver that
> > > supports all Winbond super-I/O based watchdog drivers.
> > >
> > > I dug op the development code again. I'll post it in a next e-mail so that we can
> > > see what the best way forward is. Note: I took the MFD approach because:
> > > 1) all superio shares the similar functions for using the Super-I/O registers.
> > > 2) Goal is to have low-level driver that support the specific super-I/O chipsets
> > > and that does the platform stuff for hwmon, watchdog, gpio, ...
> > >
> > Hi Wim,
> >
> > I started with a similar approach, only I used mfd cells to pass on platform
> > specific information such as the device type and the superio base address.
> > I still have the patchset for the mfd driver, in case you are interested.
> > My code is based on the patches submitted by Rodolfo Giometti a couple
> > of years ago. Want me to post it ?
>
> If it's not v1 then I am interested.
> I think it depends on the super-I/O chipset of what info you can pass. I would
> not use mfd cells for the winbond driver but use platform data (similar to
> drivers/mfd/adp5520.c) because you don't need more then just pass some data.
>
Actually one can pass platform data through mfd cells, which is what I do.
I pass the SIO address, the chip type (as enum), and the chip name to the
platform driver.

> > What I noticed in my testing is that the superio address range (2e or 4e),
> > which the drivers currently take as granted, is at least on my systems
> > (all three of them) reserved by ACPI. Unfortunately that means one can not
> > use the mfd infrastructure to pass on the superio memory region,
> > since it checks for acpi conflicts. With that I gave up on the idea and
> > reverted to using request_muxed_region. That seemed simpler and accomplish
> > the same as long as all drivers actually use it.
>
> Noticed the same. That's why passing the platform data together with the
> superio-address and type seems the best way to go. I also kept the superio_enter
> and superio_exit a function in the low level driver because I used a lock to
> make sure that the hwmon code doesn't start doing things when the watchdog is
> doing things. But I think that the request_muxed_region is doing something similar
> also.
>
Yes, it does the same. Only difference is that using a function in the mfd
driver increases the use count on it when a client module is loaded.

Patch follows.

Thanks,
Guenter

2013-03-23 23:27:05

by Guenter Roeck

[permalink] [raw]
Subject: mfd: Core driver for W836{2389}7[T]HF

MFD core driver for various variants of Winbond/Nuvoton SuperIO chips.

Signed-off-by: Guenter Roeck <[email protected]>
---

My variant of an MFD core driver for Winbond chips.

Key differences to Wim's driver
- Uses mfd infrastructure
- Passes enum instead of raw device id to client drivers
- Uses request_muxed_region instead of an in-module function for
superio_enter
[ Using an in-module function and in-module locking may be better ]

TODO:
- Detect and instantiate superio chips on both addresses, not just one.
- Replace static variables 'pdev' and mfd_cells[].

drivers/mfd/Kconfig | 22 +++
drivers/mfd/Makefile | 1 +
drivers/mfd/w83627hf-core.c | 324 ++++++++++++++++++++++++++++++++++++++++++
include/linux/mfd/w83627hf.h | 131 +++++++++++++++++
4 files changed, 478 insertions(+)
create mode 100644 drivers/mfd/w83627hf-core.c
create mode 100644 include/linux/mfd/w83627hf.h

diff --git a/drivers/mfd/Kconfig b/drivers/mfd/Kconfig
index c346941..a141ef6 100644
--- a/drivers/mfd/Kconfig
+++ b/drivers/mfd/Kconfig
@@ -1119,6 +1119,28 @@ config MFD_AS3711
help
Support for the AS3711 PMIC from AMS

+config MFD_W83627HF
+ tristate "Winbond W83627HF and compatibles"
+ select MFD_CORE
+ help
+ If you say yes here you add support for the Winbond W836X7 and Nuvoton
+ NCT677X series of super-IO chips. The following chips are supported:
+ W83627F/HF/G/HG/DHG/DHG-P/EHF/EHG/S/SF/THF/UHG/UG
+ W83637HF
+ W83667HG/HG-B
+ W83687THF
+ W83697HF
+ NCT6775
+ NCT6776
+ NCT6779
+
+ This is a multi functional device and this support defines a new
+ platform device only. See other configuration submenus in order to
+ enable the drivers of Winbond chip's functionalities.
+
+ This driver can also be built as a module. If so, the module
+ will be called w83627hf-core.
+
endmenu
endif

diff --git a/drivers/mfd/Makefile b/drivers/mfd/Makefile
index b90409c..3e9e830 100644
--- a/drivers/mfd/Makefile
+++ b/drivers/mfd/Makefile
@@ -77,6 +77,7 @@ obj-$(CONFIG_MFD_MC13XXX_I2C) += mc13xxx-i2c.o
obj-$(CONFIG_MFD_CORE) += mfd-core.o

obj-$(CONFIG_EZX_PCAP) += ezx-pcap.o
+obj-$(CONFIG_MFD_W83627HF) += w83627hf-core.o

obj-$(CONFIG_MCP) += mcp-core.o
obj-$(CONFIG_MCP_SA11X0) += mcp-sa11x0.o
diff --git a/drivers/mfd/w83627hf-core.c b/drivers/mfd/w83627hf-core.c
new file mode 100644
index 0000000..d0be5b9
--- /dev/null
+++ b/drivers/mfd/w83627hf-core.c
@@ -0,0 +1,324 @@
+/*
+ * w83627hf.c - platform device support
+ *
+ * Copyright (c) 2013 Guenter Roeck <[email protected]>
+ *
+ * Based on earlier work by Rodolfo Giometti
+ *
+ * Copyright (c) 2009-2011 Rodolfo Giometti <[email protected]>
+ *
+ * Based on drivers/hwmon/w83627hf.c
+ *
+ * Original copyright note:
+ * Copyright (c) 1998 - 2003 Frodo Looijaard <[email protected]>,
+ * Philip Edelbrock <[email protected]>,
+ * and Mark Studebaker <[email protected]>
+ * Ported to 2.6 by Bernhard C. Schrenk <[email protected]>
+ * Copyright (c) 2007 Jean Delvare <[email protected]>
+ *
+ * This program is free software; you can redistribute it and/or modify
+ * it under the terms of the GNU General Public License as published by
+ * the Free Software Foundation; either version 2 of the License, or
+ * (at your option) any later version.
+ *
+ * This program is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
+ * GNU General Public License for more details.
+ *
+ * You should have received a copy of the GNU General Public License
+ * along with this program; if not, write to the Free Software
+ * Foundation, Inc., 675 Mass Ave, Cambridge, MA 02139, USA.
+ */
+
+#define pr_fmt(fmt) KBUILD_MODNAME ": " fmt
+
+#include <linux/module.h>
+#include <linux/init.h>
+#include <linux/platform_device.h>
+#include <linux/err.h>
+#include <linux/mutex.h>
+#include <linux/ioport.h>
+#include <linux/acpi.h>
+#include <linux/io.h>
+#include <linux/mfd/core.h>
+#include <linux/mfd/w83627hf.h>
+
+static unsigned short force_id;
+module_param(force_id, ushort, 0);
+MODULE_PARM_DESC(force_id, "Override the detected device ID");
+
+/*
+ * Devices definitions
+ */
+
+static struct platform_device *pdev;
+
+struct w83627hf_chip_data {
+ const char * const name;
+ const char * const chip;
+ const char * const hwmon_drvname;
+};
+
+static const struct w83627hf_chip_data w83627hf_chip_data[] = {
+ [w83627hf] = {
+ .name = __stringify(w83627hf),
+ .chip = "W83627HF",
+ .hwmon_drvname = W83627HF_HWMON_DRVNAME,
+ },
+ [w83627s] = {
+ .name = __stringify(w83627s),
+ .chip = "W83627S",
+ },
+ [w83627thf] = {
+ .name = __stringify(w83627thf),
+ .chip = "W83627THF",
+ .hwmon_drvname = W83627HF_HWMON_DRVNAME,
+ },
+ [w83697hf] = {
+ .name = __stringify(w83697hf),
+ .chip = "W83697HF",
+ .hwmon_drvname = W83627HF_HWMON_DRVNAME,
+ },
+ [w83697ug] = {
+ .name = __stringify(w83697ug),
+ .chip = "W83697SF/UG",
+ },
+ [w83637hf] = {
+ .name = __stringify(w83637hf),
+ .chip = "W83637HF",
+ .hwmon_drvname = W83627HF_HWMON_DRVNAME,
+ },
+ [w83687thf] = {
+ .name = __stringify(w83687thf),
+ .chip = "W83687THF",
+ .hwmon_drvname = W83627HF_HWMON_DRVNAME,
+ },
+ [w83627ehf] = {
+ .name = __stringify(w83627ehf),
+ .chip = "W83627EHF",
+ .hwmon_drvname = W83627EHF_HWMON_DRVNAME,
+ },
+ [w83627dhg] = {
+ .name = __stringify(w83627dhg),
+ .chip = "W83627DHG",
+ .hwmon_drvname = W83627EHF_HWMON_DRVNAME,
+ },
+ [w83627dhg_p] = {
+ .name = __stringify(w83627dhg),
+ .chip = "W83627DHG-P",
+ .hwmon_drvname = W83627EHF_HWMON_DRVNAME,
+ },
+ [w83627uhg] = {
+ .name = __stringify(w83627uhg),
+ .chip = "W83627UHG",
+ .hwmon_drvname = W83627EHF_HWMON_DRVNAME,
+ },
+ [w83667hg] = {
+ .name = __stringify(w83667hg),
+ .chip = "W83667HG",
+ .hwmon_drvname = W83627EHF_HWMON_DRVNAME,
+ },
+ [w83667hg_b] = {
+ .name = __stringify(w83667hg),
+ .chip = "W83667HG-B",
+ .hwmon_drvname = W83627EHF_HWMON_DRVNAME,
+ },
+ [nct6775] = {
+ .name = __stringify(nct6775),
+ .chip = "NCT6775",
+ .hwmon_drvname = NCT6775_HWMON_DRVNAME,
+ },
+ [nct6776] = {
+ .name = __stringify(nct6776),
+ .chip = "NCT6776",
+ .hwmon_drvname = NCT6775_HWMON_DRVNAME,
+ },
+ [nct6779] = {
+ .name = __stringify(nct6779),
+ .chip = "NCT6779",
+ .hwmon_drvname = NCT6775_HWMON_DRVNAME,
+ },
+};
+
+#define MFD_NUM_CELLS 2
+
+static struct mfd_cell mfd_cells[MFD_NUM_CELLS];
+
+#define W83627HF_CR_DEVID 0x20
+
+#define W83627HF_G_DEVID 0x5210
+#define W83627HF_J_DEVID 0x5230
+#define W83627HF_UD_DEVID 0x5240
+#define W83627S_DEVID 0x5950
+#define W83627THF_DEVID 0x8280
+#define W83697HF_DEVID 0x6010
+#define W83697SF_DEVID 0x6800
+#define W83697UG_DEVID 0x6810
+#define W83637HF_DEVID 0x7080
+#define W83687THF_DEVID 0x8540
+#define W83627EHF_DEVID 0x8850
+#define W83627EHG_DEVID 0x8860
+#define W83627DHG_DEVID 0xa020
+#define W83627DHG_P_DEVID 0xb070
+#define W83627UHG_DEVID 0xa230
+#define W83667HG_DEVID 0xa510
+#define W83667HG_B_DEVID 0xb350
+#define NCT6775_DEVID 0xb470
+#define NCT6776_DEVID 0xc330
+#define NCT6779_DEVID 0xc560
+
+#define DEVID_MASK 0xfff0
+
+static int __init w83627hf_find(int sioaddr,
+ struct w83627hf_platform_data *pdata)
+{
+ int err = 0;
+ u16 val;
+
+ err = superio_enter(sioaddr);
+ if (err)
+ return err;
+
+ val = force_id ? : ((superio_inb(sioaddr, W83627HF_CR_DEVID) << 8) |
+ superio_inb(sioaddr, W83627HF_CR_DEVID + 1));
+
+ switch (val & DEVID_MASK) {
+ case W83627HF_G_DEVID:
+ case W83627HF_J_DEVID:
+ case W83627HF_UD_DEVID:
+ pdata->type = w83627hf;
+ break;
+ case W83627S_DEVID:
+ pdata->type = w83627s;
+ break;
+ case W83627THF_DEVID:
+ pdata->type = w83627thf;
+ break;
+ case W83697HF_DEVID:
+ pdata->type = w83697hf;
+ break;
+ case W83697SF_DEVID:
+ case W83697UG_DEVID:
+ pdata->type = w83697ug;
+ break;
+ case W83637HF_DEVID:
+ pdata->type = w83637hf;
+ break;
+ case W83687THF_DEVID:
+ pdata->type = w83687thf;
+ break;
+ case W83627EHF_DEVID:
+ case W83627EHG_DEVID:
+ pdata->type = w83627ehf;
+ break;
+ case W83627DHG_DEVID:
+ pdata->type = w83627dhg;
+ break;
+ case W83627DHG_P_DEVID:
+ pdata->type = w83627dhg_p;
+ break;
+ case W83627UHG_DEVID:
+ pdata->type = w83627uhg;
+ break;
+ case W83667HG_DEVID:
+ pdata->type = w83667hg;
+ break;
+ case W83667HG_B_DEVID:
+ pdata->type = w83667hg_b;
+ break;
+ case NCT6775_DEVID:
+ pdata->type = nct6775;
+ break;
+ case NCT6776_DEVID:
+ pdata->type = nct6776;
+ break;
+ case NCT6779_DEVID:
+ pdata->type = nct6779;
+ break;
+ case 0xfff0: /* No device at all */
+ err = -ENODEV;
+ goto exit;
+ default:
+ err = -ENODEV;
+ pr_debug("Unsupported chip (DEVID=0x%04x)\n", val);
+ goto exit;
+ }
+ pdata->sioaddr = sioaddr;
+ pdata->name = w83627hf_chip_data[pdata->type].name;
+ pr_info("Found %s at %#x\n", w83627hf_chip_data[pdata->type].chip,
+ sioaddr);
+exit:
+ superio_exit(sioaddr);
+ return err;
+}
+
+static int __init w83627hf_device_add(struct w83627hf_platform_data *pdata)
+{
+ int err;
+ int cells = 0;
+
+ pdev = platform_device_alloc(W83627HF_CORE_DRVNAME, 0);
+ if (!pdev)
+ return -ENOMEM;
+
+ err = platform_device_add_data(pdev, pdata,
+ sizeof(struct w83627hf_platform_data));
+ if (err)
+ goto exit_device_put;
+
+ if (w83627hf_chip_data[pdata->type].hwmon_drvname) {
+ mfd_cells[cells].name =
+ w83627hf_chip_data[pdata->type].hwmon_drvname;
+ cells++;
+ }
+
+ mfd_cells[cells].name = W83627HF_WDT_DRVNAME;
+ cells++;
+
+ err = platform_device_add(pdev);
+ if (err)
+ goto exit_device_put;
+
+ err = mfd_add_devices(&pdev->dev, pdev->id, mfd_cells, cells,
+ NULL, -1, NULL);
+ if (err)
+ goto exit_device_unregister;
+
+ return 0;
+
+exit_device_unregister:
+ platform_device_unregister(pdev);
+exit_device_put:
+ platform_device_put(pdev);
+ return err;
+}
+
+static int __init w83627hf_init(void)
+{
+ struct w83627hf_platform_data pdata;
+ int ret;
+
+ ret = w83627hf_find(0x2e, &pdata);
+ if (ret) {
+ ret = w83627hf_find(0x4e, &pdata);
+ if (ret)
+ return -ENODEV;
+ }
+
+ /* Sets global pdev as a side effect */
+ return w83627hf_device_add(&pdata);
+}
+
+static void __exit w83627hf_exit(void)
+{
+ mfd_remove_devices(&pdev->dev);
+ platform_device_unregister(pdev);
+}
+
+MODULE_AUTHOR("Guenter Roeck <[email protected]>");
+MODULE_DESCRIPTION("W83627HF multi-function core driver");
+MODULE_LICENSE("GPL");
+
+module_init(w83627hf_init);
+module_exit(w83627hf_exit);
diff --git a/include/linux/mfd/w83627hf.h b/include/linux/mfd/w83627hf.h
new file mode 100644
index 0000000..6379126
--- /dev/null
+++ b/include/linux/mfd/w83627hf.h
@@ -0,0 +1,131 @@
+/*
+ * w83627hf.h - platform device support, header file
+ * Copyright (c) 2009 Rodolfo Giometti <[email protected]>
+ *
+ * This program is free software; you can redistribute it and/or modify
+ * it under the terms of the GNU General Public License as published by
+ * the Free Software Foundation; either version 2 of the License, or
+ * (at your option) any later version.
+ *
+ * This program is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
+ * GNU General Public License for more details.
+ *
+ * You should have received a copy of the GNU General Public License
+ * along with this program; if not, write to the Free Software
+ * Foundation, Inc., 675 Mass Ave, Cambridge, MA 02139, USA.
+ */
+
+#include <linux/mutex.h>
+#include <linux/io.h>
+
+#define W83627HF_CORE_DRVNAME "w83627hf_core"
+#define W83627HF_HWMON_DRVNAME "w83627hf_hwmon"
+#define W83627EHF_HWMON_DRVNAME "w83627ehf_hwmon"
+#define NCT6775_HWMON_DRVNAME "nct6775_hwmon"
+
+#define W83627HF_WDT_DRVNAME "w83627hf_wdt"
+
+enum chips {
+ w83627hf,
+ w83627s,
+ w83627thf,
+ w83697hf,
+ w83697ug,
+ w83637hf,
+ w83687thf,
+ w83627ehf,
+ w83627dhg,
+ w83627dhg_p,
+ w83627uhg,
+ w83667hg,
+ w83667hg_b,
+ nct6775,
+ nct6776,
+ nct6779,
+};
+
+struct w83627hf_platform_data {
+ int sioaddr;
+ enum chips type;
+ const char *name;
+};
+
+#define W83627HF_SELECT 0x07
+
+/* logical device numbers for superio_select */
+#define W83627HF_LD_FDC 0x00
+#define W83627HF_LD_PRT 0x01
+#define W83627HF_LD_UART1 0x02
+#define W83627HF_LD_UART2 0x03
+#define W83627HF_LD_KBC 0x05
+#define W83627HF_LD_CIR 0x06 /* w83627hf only */
+#define W83627HF_LD_GAME 0x07
+#define W83627HF_LD_MIDI 0x07
+#define W83627HF_LD_GPIO1 0x07
+#define W83627HF_LD_GPIO5 0x07 /* w83627thf only */
+#define W83627HF_LD_GPIO2 0x08
+#define W83627HF_LD_WDT 0x08
+#define W83627HF_LD_GPIO3 0x09
+#define W83627HF_LD_GPIO4 0x09 /* w83627thf only */
+#define W83627HF_LD_ACPI 0x0a
+#define W83627HF_LD_HWM 0x0b
+
+#define W83627HF_CR_ENABLE 0x30 /* Logical device enable register */
+
+#define W83627THF_GPIO5_IOSR 0xf3 /* w83627thf only */
+#define W83627THF_GPIO5_DR 0xf4 /* w83627thf only */
+
+#define W83687THF_VID_EN 0x29 /* w83687thf only */
+#define W83687THF_VID_CFG 0xF0 /* w83687thf only */
+#define W83687THF_VID_DATA 0xF1 /* w83687thf only */
+
+/*
+ * Common configuration registers access functions.
+ *
+ * These registers are special and they must me accessed by using a well
+ * specified protocol. Client drivers __must__ do as follow in order to
+ * get access correctly to these registers:
+ *
+ * superio_enter()
+ * superio_select()/superio_outb()/superio_inb()
+ * suprio_exit();
+ *
+ */
+
+static inline int superio_enter(int sioaddr)
+{
+ if (!request_muxed_region(sioaddr, 2, KBUILD_MODNAME))
+ return -EBUSY;
+
+ outb(0x87, sioaddr);
+ outb(0x87, sioaddr);
+
+ return 0;
+}
+
+static inline void superio_select(int sioaddr, int ld)
+{
+ outb(W83627HF_SELECT, sioaddr);
+ outb(ld, sioaddr + 1);
+}
+
+static inline void superio_outb(int sioaddr, int reg, int val)
+{
+ outb(reg, sioaddr);
+ outb(val, sioaddr + 1);
+}
+
+static inline int superio_inb(int sioaddr, int reg)
+{
+ outb(reg, sioaddr);
+ return inb(sioaddr + 1);
+}
+
+static inline void superio_exit(int sioaddr)
+{
+ outb(0xAA, sioaddr);
+
+ release_region(sioaddr, 2);
+}
--
1.7.9.7

2013-03-24 02:18:24

by Guenter Roeck

[permalink] [raw]
Subject: mfd: Core driver for Winbond chips

MFD core driver for various variants of Winbond/Nuvoton SuperIO chips.

Signed-off-by: Guenter Roeck <[email protected]>
---
drivers/mfd/Kconfig | 22 +++
drivers/mfd/Makefile | 1 +
drivers/mfd/w83627hf-core.c | 324 ++++++++++++++++++++++++++++++++++++++++++
include/linux/mfd/w83627hf.h | 131 +++++++++++++++++
4 files changed, 478 insertions(+)
create mode 100644 drivers/mfd/w83627hf-core.c
create mode 100644 include/linux/mfd/w83627hf.h

diff --git a/drivers/mfd/Kconfig b/drivers/mfd/Kconfig
index c346941..a141ef6 100644
--- a/drivers/mfd/Kconfig
+++ b/drivers/mfd/Kconfig
@@ -1119,6 +1119,28 @@ config MFD_AS3711
help
Support for the AS3711 PMIC from AMS

+config MFD_W83627HF
+ tristate "Winbond W83627HF and compatibles"
+ select MFD_CORE
+ help
+ If you say yes here you add support for the Winbond W836X7 and Nuvoton
+ NCT677X series of super-IO chips. The following chips are supported:
+ W83627F/HF/G/HG/DHG/DHG-P/EHF/EHG/S/SF/THF/UHG/UG
+ W83637HF
+ W83667HG/HG-B
+ W83687THF
+ W83697HF
+ NCT6775
+ NCT6776
+ NCT6779
+
+ This is a multi functional device and this support defines a new
+ platform device only. See other configuration submenus in order to
+ enable the drivers of Winbond chip's functionalities.
+
+ This driver can also be built as a module. If so, the module
+ will be called w83627hf-core.
+
endmenu
endif

diff --git a/drivers/mfd/Makefile b/drivers/mfd/Makefile
index b90409c..3e9e830 100644
--- a/drivers/mfd/Makefile
+++ b/drivers/mfd/Makefile
@@ -77,6 +77,7 @@ obj-$(CONFIG_MFD_MC13XXX_I2C) += mc13xxx-i2c.o
obj-$(CONFIG_MFD_CORE) += mfd-core.o

obj-$(CONFIG_EZX_PCAP) += ezx-pcap.o
+obj-$(CONFIG_MFD_W83627HF) += w83627hf-core.o

obj-$(CONFIG_MCP) += mcp-core.o
obj-$(CONFIG_MCP_SA11X0) += mcp-sa11x0.o
diff --git a/drivers/mfd/w83627hf-core.c b/drivers/mfd/w83627hf-core.c
new file mode 100644
index 0000000..d0be5b9
--- /dev/null
+++ b/drivers/mfd/w83627hf-core.c
@@ -0,0 +1,324 @@
+/*
+ * w83627hf.c - platform device support
+ *
+ * Copyright (c) 2013 Guenter Roeck <[email protected]>
+ *
+ * Based on earlier work by Rodolfo Giometti
+ *
+ * Copyright (c) 2009-2011 Rodolfo Giometti <[email protected]>
+ *
+ * Based on drivers/hwmon/w83627hf.c
+ *
+ * Original copyright note:
+ * Copyright (c) 1998 - 2003 Frodo Looijaard <[email protected]>,
+ * Philip Edelbrock <[email protected]>,
+ * and Mark Studebaker <[email protected]>
+ * Ported to 2.6 by Bernhard C. Schrenk <[email protected]>
+ * Copyright (c) 2007 Jean Delvare <[email protected]>
+ *
+ * This program is free software; you can redistribute it and/or modify
+ * it under the terms of the GNU General Public License as published by
+ * the Free Software Foundation; either version 2 of the License, or
+ * (at your option) any later version.
+ *
+ * This program is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
+ * GNU General Public License for more details.
+ *
+ * You should have received a copy of the GNU General Public License
+ * along with this program; if not, write to the Free Software
+ * Foundation, Inc., 675 Mass Ave, Cambridge, MA 02139, USA.
+ */
+
+#define pr_fmt(fmt) KBUILD_MODNAME ": " fmt
+
+#include <linux/module.h>
+#include <linux/init.h>
+#include <linux/platform_device.h>
+#include <linux/err.h>
+#include <linux/mutex.h>
+#include <linux/ioport.h>
+#include <linux/acpi.h>
+#include <linux/io.h>
+#include <linux/mfd/core.h>
+#include <linux/mfd/w83627hf.h>
+
+static unsigned short force_id;
+module_param(force_id, ushort, 0);
+MODULE_PARM_DESC(force_id, "Override the detected device ID");
+
+/*
+ * Devices definitions
+ */
+
+static struct platform_device *pdev;
+
+struct w83627hf_chip_data {
+ const char * const name;
+ const char * const chip;
+ const char * const hwmon_drvname;
+};
+
+static const struct w83627hf_chip_data w83627hf_chip_data[] = {
+ [w83627hf] = {
+ .name = __stringify(w83627hf),
+ .chip = "W83627HF",
+ .hwmon_drvname = W83627HF_HWMON_DRVNAME,
+ },
+ [w83627s] = {
+ .name = __stringify(w83627s),
+ .chip = "W83627S",
+ },
+ [w83627thf] = {
+ .name = __stringify(w83627thf),
+ .chip = "W83627THF",
+ .hwmon_drvname = W83627HF_HWMON_DRVNAME,
+ },
+ [w83697hf] = {
+ .name = __stringify(w83697hf),
+ .chip = "W83697HF",
+ .hwmon_drvname = W83627HF_HWMON_DRVNAME,
+ },
+ [w83697ug] = {
+ .name = __stringify(w83697ug),
+ .chip = "W83697SF/UG",
+ },
+ [w83637hf] = {
+ .name = __stringify(w83637hf),
+ .chip = "W83637HF",
+ .hwmon_drvname = W83627HF_HWMON_DRVNAME,
+ },
+ [w83687thf] = {
+ .name = __stringify(w83687thf),
+ .chip = "W83687THF",
+ .hwmon_drvname = W83627HF_HWMON_DRVNAME,
+ },
+ [w83627ehf] = {
+ .name = __stringify(w83627ehf),
+ .chip = "W83627EHF",
+ .hwmon_drvname = W83627EHF_HWMON_DRVNAME,
+ },
+ [w83627dhg] = {
+ .name = __stringify(w83627dhg),
+ .chip = "W83627DHG",
+ .hwmon_drvname = W83627EHF_HWMON_DRVNAME,
+ },
+ [w83627dhg_p] = {
+ .name = __stringify(w83627dhg),
+ .chip = "W83627DHG-P",
+ .hwmon_drvname = W83627EHF_HWMON_DRVNAME,
+ },
+ [w83627uhg] = {
+ .name = __stringify(w83627uhg),
+ .chip = "W83627UHG",
+ .hwmon_drvname = W83627EHF_HWMON_DRVNAME,
+ },
+ [w83667hg] = {
+ .name = __stringify(w83667hg),
+ .chip = "W83667HG",
+ .hwmon_drvname = W83627EHF_HWMON_DRVNAME,
+ },
+ [w83667hg_b] = {
+ .name = __stringify(w83667hg),
+ .chip = "W83667HG-B",
+ .hwmon_drvname = W83627EHF_HWMON_DRVNAME,
+ },
+ [nct6775] = {
+ .name = __stringify(nct6775),
+ .chip = "NCT6775",
+ .hwmon_drvname = NCT6775_HWMON_DRVNAME,
+ },
+ [nct6776] = {
+ .name = __stringify(nct6776),
+ .chip = "NCT6776",
+ .hwmon_drvname = NCT6775_HWMON_DRVNAME,
+ },
+ [nct6779] = {
+ .name = __stringify(nct6779),
+ .chip = "NCT6779",
+ .hwmon_drvname = NCT6775_HWMON_DRVNAME,
+ },
+};
+
+#define MFD_NUM_CELLS 2
+
+static struct mfd_cell mfd_cells[MFD_NUM_CELLS];
+
+#define W83627HF_CR_DEVID 0x20
+
+#define W83627HF_G_DEVID 0x5210
+#define W83627HF_J_DEVID 0x5230
+#define W83627HF_UD_DEVID 0x5240
+#define W83627S_DEVID 0x5950
+#define W83627THF_DEVID 0x8280
+#define W83697HF_DEVID 0x6010
+#define W83697SF_DEVID 0x6800
+#define W83697UG_DEVID 0x6810
+#define W83637HF_DEVID 0x7080
+#define W83687THF_DEVID 0x8540
+#define W83627EHF_DEVID 0x8850
+#define W83627EHG_DEVID 0x8860
+#define W83627DHG_DEVID 0xa020
+#define W83627DHG_P_DEVID 0xb070
+#define W83627UHG_DEVID 0xa230
+#define W83667HG_DEVID 0xa510
+#define W83667HG_B_DEVID 0xb350
+#define NCT6775_DEVID 0xb470
+#define NCT6776_DEVID 0xc330
+#define NCT6779_DEVID 0xc560
+
+#define DEVID_MASK 0xfff0
+
+static int __init w83627hf_find(int sioaddr,
+ struct w83627hf_platform_data *pdata)
+{
+ int err = 0;
+ u16 val;
+
+ err = superio_enter(sioaddr);
+ if (err)
+ return err;
+
+ val = force_id ? : ((superio_inb(sioaddr, W83627HF_CR_DEVID) << 8) |
+ superio_inb(sioaddr, W83627HF_CR_DEVID + 1));
+
+ switch (val & DEVID_MASK) {
+ case W83627HF_G_DEVID:
+ case W83627HF_J_DEVID:
+ case W83627HF_UD_DEVID:
+ pdata->type = w83627hf;
+ break;
+ case W83627S_DEVID:
+ pdata->type = w83627s;
+ break;
+ case W83627THF_DEVID:
+ pdata->type = w83627thf;
+ break;
+ case W83697HF_DEVID:
+ pdata->type = w83697hf;
+ break;
+ case W83697SF_DEVID:
+ case W83697UG_DEVID:
+ pdata->type = w83697ug;
+ break;
+ case W83637HF_DEVID:
+ pdata->type = w83637hf;
+ break;
+ case W83687THF_DEVID:
+ pdata->type = w83687thf;
+ break;
+ case W83627EHF_DEVID:
+ case W83627EHG_DEVID:
+ pdata->type = w83627ehf;
+ break;
+ case W83627DHG_DEVID:
+ pdata->type = w83627dhg;
+ break;
+ case W83627DHG_P_DEVID:
+ pdata->type = w83627dhg_p;
+ break;
+ case W83627UHG_DEVID:
+ pdata->type = w83627uhg;
+ break;
+ case W83667HG_DEVID:
+ pdata->type = w83667hg;
+ break;
+ case W83667HG_B_DEVID:
+ pdata->type = w83667hg_b;
+ break;
+ case NCT6775_DEVID:
+ pdata->type = nct6775;
+ break;
+ case NCT6776_DEVID:
+ pdata->type = nct6776;
+ break;
+ case NCT6779_DEVID:
+ pdata->type = nct6779;
+ break;
+ case 0xfff0: /* No device at all */
+ err = -ENODEV;
+ goto exit;
+ default:
+ err = -ENODEV;
+ pr_debug("Unsupported chip (DEVID=0x%04x)\n", val);
+ goto exit;
+ }
+ pdata->sioaddr = sioaddr;
+ pdata->name = w83627hf_chip_data[pdata->type].name;
+ pr_info("Found %s at %#x\n", w83627hf_chip_data[pdata->type].chip,
+ sioaddr);
+exit:
+ superio_exit(sioaddr);
+ return err;
+}
+
+static int __init w83627hf_device_add(struct w83627hf_platform_data *pdata)
+{
+ int err;
+ int cells = 0;
+
+ pdev = platform_device_alloc(W83627HF_CORE_DRVNAME, 0);
+ if (!pdev)
+ return -ENOMEM;
+
+ err = platform_device_add_data(pdev, pdata,
+ sizeof(struct w83627hf_platform_data));
+ if (err)
+ goto exit_device_put;
+
+ if (w83627hf_chip_data[pdata->type].hwmon_drvname) {
+ mfd_cells[cells].name =
+ w83627hf_chip_data[pdata->type].hwmon_drvname;
+ cells++;
+ }
+
+ mfd_cells[cells].name = W83627HF_WDT_DRVNAME;
+ cells++;
+
+ err = platform_device_add(pdev);
+ if (err)
+ goto exit_device_put;
+
+ err = mfd_add_devices(&pdev->dev, pdev->id, mfd_cells, cells,
+ NULL, -1, NULL);
+ if (err)
+ goto exit_device_unregister;
+
+ return 0;
+
+exit_device_unregister:
+ platform_device_unregister(pdev);
+exit_device_put:
+ platform_device_put(pdev);
+ return err;
+}
+
+static int __init w83627hf_init(void)
+{
+ struct w83627hf_platform_data pdata;
+ int ret;
+
+ ret = w83627hf_find(0x2e, &pdata);
+ if (ret) {
+ ret = w83627hf_find(0x4e, &pdata);
+ if (ret)
+ return -ENODEV;
+ }
+
+ /* Sets global pdev as a side effect */
+ return w83627hf_device_add(&pdata);
+}
+
+static void __exit w83627hf_exit(void)
+{
+ mfd_remove_devices(&pdev->dev);
+ platform_device_unregister(pdev);
+}
+
+MODULE_AUTHOR("Guenter Roeck <[email protected]>");
+MODULE_DESCRIPTION("W83627HF multi-function core driver");
+MODULE_LICENSE("GPL");
+
+module_init(w83627hf_init);
+module_exit(w83627hf_exit);
diff --git a/include/linux/mfd/w83627hf.h b/include/linux/mfd/w83627hf.h
new file mode 100644
index 0000000..6379126
--- /dev/null
+++ b/include/linux/mfd/w83627hf.h
@@ -0,0 +1,131 @@
+/*
+ * w83627hf.h - platform device support, header file
+ * Copyright (c) 2009 Rodolfo Giometti <[email protected]>
+ *
+ * This program is free software; you can redistribute it and/or modify
+ * it under the terms of the GNU General Public License as published by
+ * the Free Software Foundation; either version 2 of the License, or
+ * (at your option) any later version.
+ *
+ * This program is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
+ * GNU General Public License for more details.
+ *
+ * You should have received a copy of the GNU General Public License
+ * along with this program; if not, write to the Free Software
+ * Foundation, Inc., 675 Mass Ave, Cambridge, MA 02139, USA.
+ */
+
+#include <linux/mutex.h>
+#include <linux/io.h>
+
+#define W83627HF_CORE_DRVNAME "w83627hf_core"
+#define W83627HF_HWMON_DRVNAME "w83627hf_hwmon"
+#define W83627EHF_HWMON_DRVNAME "w83627ehf_hwmon"
+#define NCT6775_HWMON_DRVNAME "nct6775_hwmon"
+
+#define W83627HF_WDT_DRVNAME "w83627hf_wdt"
+
+enum chips {
+ w83627hf,
+ w83627s,
+ w83627thf,
+ w83697hf,
+ w83697ug,
+ w83637hf,
+ w83687thf,
+ w83627ehf,
+ w83627dhg,
+ w83627dhg_p,
+ w83627uhg,
+ w83667hg,
+ w83667hg_b,
+ nct6775,
+ nct6776,
+ nct6779,
+};
+
+struct w83627hf_platform_data {
+ int sioaddr;
+ enum chips type;
+ const char *name;
+};
+
+#define W83627HF_SELECT 0x07
+
+/* logical device numbers for superio_select */
+#define W83627HF_LD_FDC 0x00
+#define W83627HF_LD_PRT 0x01
+#define W83627HF_LD_UART1 0x02
+#define W83627HF_LD_UART2 0x03
+#define W83627HF_LD_KBC 0x05
+#define W83627HF_LD_CIR 0x06 /* w83627hf only */
+#define W83627HF_LD_GAME 0x07
+#define W83627HF_LD_MIDI 0x07
+#define W83627HF_LD_GPIO1 0x07
+#define W83627HF_LD_GPIO5 0x07 /* w83627thf only */
+#define W83627HF_LD_GPIO2 0x08
+#define W83627HF_LD_WDT 0x08
+#define W83627HF_LD_GPIO3 0x09
+#define W83627HF_LD_GPIO4 0x09 /* w83627thf only */
+#define W83627HF_LD_ACPI 0x0a
+#define W83627HF_LD_HWM 0x0b
+
+#define W83627HF_CR_ENABLE 0x30 /* Logical device enable register */
+
+#define W83627THF_GPIO5_IOSR 0xf3 /* w83627thf only */
+#define W83627THF_GPIO5_DR 0xf4 /* w83627thf only */
+
+#define W83687THF_VID_EN 0x29 /* w83687thf only */
+#define W83687THF_VID_CFG 0xF0 /* w83687thf only */
+#define W83687THF_VID_DATA 0xF1 /* w83687thf only */
+
+/*
+ * Common configuration registers access functions.
+ *
+ * These registers are special and they must me accessed by using a well
+ * specified protocol. Client drivers __must__ do as follow in order to
+ * get access correctly to these registers:
+ *
+ * superio_enter()
+ * superio_select()/superio_outb()/superio_inb()
+ * suprio_exit();
+ *
+ */
+
+static inline int superio_enter(int sioaddr)
+{
+ if (!request_muxed_region(sioaddr, 2, KBUILD_MODNAME))
+ return -EBUSY;
+
+ outb(0x87, sioaddr);
+ outb(0x87, sioaddr);
+
+ return 0;
+}
+
+static inline void superio_select(int sioaddr, int ld)
+{
+ outb(W83627HF_SELECT, sioaddr);
+ outb(ld, sioaddr + 1);
+}
+
+static inline void superio_outb(int sioaddr, int reg, int val)
+{
+ outb(reg, sioaddr);
+ outb(val, sioaddr + 1);
+}
+
+static inline int superio_inb(int sioaddr, int reg)
+{
+ outb(reg, sioaddr);
+ return inb(sioaddr + 1);
+}
+
+static inline void superio_exit(int sioaddr)
+{
+ outb(0xAA, sioaddr);
+
+ release_region(sioaddr, 2);
+}
--
1.7.9.7

2013-03-24 09:18:07

by Guenter Roeck

[permalink] [raw]
Subject: Re: mfd: Core driver for Winbond chips

On Sat, Mar 23, 2013 at 10:49:14AM -0700, Guenter Roeck wrote:
> MFD core driver for various variants of Winbond/Nuvoton SuperIO chips.
>
> Signed-off-by: Guenter Roeck <[email protected]>

Sorry for sending this twice. I seem to be having trouble with outgoing e-mail
being delayed for hours. The first patch didn't show up for a long time, and I
thought I did something wrong when sending it.

Guenter

2013-04-09 09:37:09

by Samuel Ortiz

[permalink] [raw]
Subject: Re: mfd: Core driver for Winbond chips

Hi Guenter,

On Sat, Mar 23, 2013 at 10:49:14AM -0700, Guenter Roeck wrote:
> MFD core driver for various variants of Winbond/Nuvoton SuperIO chips.
>
> Signed-off-by: Guenter Roeck <[email protected]>
> ---
> drivers/mfd/Kconfig | 22 +++
> drivers/mfd/Makefile | 1 +
> drivers/mfd/w83627hf-core.c | 324 ++++++++++++++++++++++++++++++++++++++++++
> include/linux/mfd/w83627hf.h | 131 +++++++++++++++++
> 4 files changed, 478 insertions(+)
> create mode 100644 drivers/mfd/w83627hf-core.c
> create mode 100644 include/linux/mfd/w83627hf.h
This driver looks overall good to me. Is this your final version, or should I
expect any follow up patch ?

Cheers,
Samuel.

--
Intel Open Source Technology Centre
http://oss.intel.com/

2013-04-09 11:36:37

by Guenter Roeck

[permalink] [raw]
Subject: Re: mfd: Core driver for Winbond chips

On Tue, Apr 09, 2013 at 11:37:01AM +0200, Samuel Ortiz wrote:
> Hi Guenter,
>
> On Sat, Mar 23, 2013 at 10:49:14AM -0700, Guenter Roeck wrote:
> > MFD core driver for various variants of Winbond/Nuvoton SuperIO chips.
> >
> > Signed-off-by: Guenter Roeck <[email protected]>
> > ---
> > drivers/mfd/Kconfig | 22 +++
> > drivers/mfd/Makefile | 1 +
> > drivers/mfd/w83627hf-core.c | 324 ++++++++++++++++++++++++++++++++++++++++++
> > include/linux/mfd/w83627hf.h | 131 +++++++++++++++++
> > 4 files changed, 478 insertions(+)
> > create mode 100644 drivers/mfd/w83627hf-core.c
> > create mode 100644 include/linux/mfd/w83627hf.h
> This driver looks overall good to me. Is this your final version, or should I
> expect any follow up patch ?
>
Hi Samuel,

I was waiting for feedback from Wim, who submitted a similar driver, about his
thoughts. Key question is how to reserve access to the shared resource - either
through an exported function in the mfd driver requesting a mutex, or through
request_muxed_region(). I am going back and forth myself on which one is better.

Maybe it does not really matter, but using a function has the slight advantage
that it auto-loads and locks the mfd module while one of its client modules
is loaded. If we use request_muxed_region, that is not the case and the client
module must use another means to request and lock the mfd module.

Maybe you have an opinion ?

Thanks,
Guenter

2013-04-09 11:37:46

by Wim Van Sebroeck

[permalink] [raw]
Subject: Re: mfd: Core driver for Winbond chips

Hi Samuel,

> > MFD core driver for various variants of Winbond/Nuvoton SuperIO chips.
> >
> > Signed-off-by: Guenter Roeck <[email protected]>
> > ---
> > drivers/mfd/Kconfig | 22 +++
> > drivers/mfd/Makefile | 1 +
> > drivers/mfd/w83627hf-core.c | 324 ++++++++++++++++++++++++++++++++++++++++++
> > include/linux/mfd/w83627hf.h | 131 +++++++++++++++++
> > 4 files changed, 478 insertions(+)
> > create mode 100644 drivers/mfd/w83627hf-core.c
> > create mode 100644 include/linux/mfd/w83627hf.h
> This driver looks overall good to me. Is this your final version, or should I
> expect any follow up patch ?

If possible could you wait a couple of days. I need to review it with what I had created
so that Guenter and Myself can come up with a final version.

Kind regards,
Wim.

2013-04-09 11:45:50

by Wim Van Sebroeck

[permalink] [raw]
Subject: Re: mfd: Core driver for Winbond chips

Hi Guenter,

> I was waiting for feedback from Wim, who submitted a similar driver, about his
> thoughts. Key question is how to reserve access to the shared resource - either
> through an exported function in the mfd driver requesting a mutex, or through
> request_muxed_region(). I am going back and forth myself on which one is better.
>
> Maybe it does not really matter, but using a function has the slight advantage
> that it auto-loads and locks the mfd module while one of its client modules
> is loaded. If we use request_muxed_region, that is not the case and the client
> module must use another means to request and lock the mfd module.
>
> Maybe you have an opinion ?

This is indeed the main issue that has to be solved. Both options will work.
I like the auto-load and lock, but I need to look at the request_muxed_region
code again first before I can see what the possible drawbacks are :-).

Kind regards,
Wim.

2013-04-09 16:18:34

by Guenter Roeck

[permalink] [raw]
Subject: Re: mfd: Core driver for Winbond chips

On Tue, Apr 09, 2013 at 01:45:44PM +0200, Wim Van Sebroeck wrote:
> Hi Guenter,
>
> > I was waiting for feedback from Wim, who submitted a similar driver, about his
> > thoughts. Key question is how to reserve access to the shared resource - either
> > through an exported function in the mfd driver requesting a mutex, or through
> > request_muxed_region(). I am going back and forth myself on which one is better.
> >
> > Maybe it does not really matter, but using a function has the slight advantage
> > that it auto-loads and locks the mfd module while one of its client modules
> > is loaded. If we use request_muxed_region, that is not the case and the client
> > module must use another means to request and lock the mfd module.
> >
> > Maybe you have an opinion ?
>
> This is indeed the main issue that has to be solved. Both options will work.
> I like the auto-load and lock, but I need to look at the request_muxed_region
> code again first before I can see what the possible drawbacks are :-).
>
One drawback of using request_muxed_region is that it needs a return value
from superio_enter. Also, it needs some code in the client driver init function
to ensure that the mfd driver gets loaded, and possibly a call to __module_get()
in the client driver probe function to keep the mfd driver loaded.

winbond_superio_enter() would not need a return value and could use
devm_request_region. We could also consider allocating the hwmon memory space in
the mfd driver and pass it as resource to the client drivers, which would remove
a few more lines of code from those.

Overall I am slightly in favor of using an exported function.

Guenter

2013-04-09 17:31:20

by Wim Van Sebroeck

[permalink] [raw]
Subject: Re: mfd: Core driver for Winbond chips

Hi Guenter,

> > > I was waiting for feedback from Wim, who submitted a similar driver, about his
> > > thoughts. Key question is how to reserve access to the shared resource - either
> > > through an exported function in the mfd driver requesting a mutex, or through
> > > request_muxed_region(). I am going back and forth myself on which one is better.
> > >
> > > Maybe it does not really matter, but using a function has the slight advantage
> > > that it auto-loads and locks the mfd module while one of its client modules
> > > is loaded. If we use request_muxed_region, that is not the case and the client
> > > module must use another means to request and lock the mfd module.
> > >
> > > Maybe you have an opinion ?
> >
> > This is indeed the main issue that has to be solved. Both options will work.
> > I like the auto-load and lock, but I need to look at the request_muxed_region
> > code again first before I can see what the possible drawbacks are :-).
> >
> One drawback of using request_muxed_region is that it needs a return value
> from superio_enter. Also, it needs some code in the client driver init function
> to ensure that the mfd driver gets loaded, and possibly a call to __module_get()
> in the client driver probe function to keep the mfd driver loaded.
>
> winbond_superio_enter() would not need a return value and could use
> devm_request_region. We could also consider allocating the hwmon memory space in
> the mfd driver and pass it as resource to the client drivers, which would remove
> a few more lines of code from those.
>
> Overall I am slightly in favor of using an exported function.

I looked at commit 8b6d043b7ee2d1b819dc833d677ea2aead71a0c0 (which implements
request_muxed_region). You indeed need some extra code for loading the lowl-level
mfd driver. So I am also in favour of the exported function.

Kind regards,
Wim.

2013-04-10 00:36:28

by Guenter Roeck

[permalink] [raw]
Subject: Re: mfd: Core driver for Winbond chips

On Tue, Apr 09, 2013 at 07:31:15PM +0200, Wim Van Sebroeck wrote:
> Hi Guenter,
>
> > > > I was waiting for feedback from Wim, who submitted a similar driver, about his
> > > > thoughts. Key question is how to reserve access to the shared resource - either
> > > > through an exported function in the mfd driver requesting a mutex, or through
> > > > request_muxed_region(). I am going back and forth myself on which one is better.
> > > >
> > > > Maybe it does not really matter, but using a function has the slight advantage
> > > > that it auto-loads and locks the mfd module while one of its client modules
> > > > is loaded. If we use request_muxed_region, that is not the case and the client
> > > > module must use another means to request and lock the mfd module.
> > > >
> > > > Maybe you have an opinion ?
> > >
> > > This is indeed the main issue that has to be solved. Both options will work.
> > > I like the auto-load and lock, but I need to look at the request_muxed_region
> > > code again first before I can see what the possible drawbacks are :-).
> > >
> > One drawback of using request_muxed_region is that it needs a return value
> > from superio_enter. Also, it needs some code in the client driver init function
> > to ensure that the mfd driver gets loaded, and possibly a call to __module_get()
> > in the client driver probe function to keep the mfd driver loaded.
> >
> > winbond_superio_enter() would not need a return value and could use
> > devm_request_region. We could also consider allocating the hwmon memory space in
> > the mfd driver and pass it as resource to the client drivers, which would remove
> > a few more lines of code from those.
> >
> > Overall I am slightly in favor of using an exported function.
>
> I looked at commit 8b6d043b7ee2d1b819dc833d677ea2aead71a0c0 (which implements
> request_muxed_region). You indeed need some extra code for loading the lowl-level
> mfd driver. So I am also in favour of the exported function.
>
So which way should we go ? Take your driver as a starting point or mine ?

One thing I'll want to add is support for both superio regions, as I have a use
case for it.

Thanks,
Guenter

2013-04-10 20:59:30

by Wim Van Sebroeck

[permalink] [raw]
Subject: Re: mfd: Core driver for Winbond chips

Hi Guenter,

> > I looked at commit 8b6d043b7ee2d1b819dc833d677ea2aead71a0c0 (which implements
> > request_muxed_region). You indeed need some extra code for loading the lowl-level
> > mfd driver. So I am also in favour of the exported function.
> >
> So which way should we go ? Take your driver as a starting point or mine ?
>
> One thing I'll want to add is support for both superio regions, as I have a use
> case for it.

I'll come back on this tomorrow. It's going to be a merge of your and my code anyway.

I agree it would be nice to add also hwmon + wdog support in it from the start.

Kind regards,
Wim.

2013-04-29 15:03:20

by Guenter Roeck

[permalink] [raw]
Subject: Re: mfd: Core driver for Winbond chips

On Wed, Apr 10, 2013 at 10:59:26PM +0200, Wim Van Sebroeck wrote:
> Hi Guenter,
>
> > > I looked at commit 8b6d043b7ee2d1b819dc833d677ea2aead71a0c0 (which implements
> > > request_muxed_region). You indeed need some extra code for loading the lowl-level
> > > mfd driver. So I am also in favour of the exported function.
> > >
> > So which way should we go ? Take your driver as a starting point or mine ?
> >
> > One thing I'll want to add is support for both superio regions, as I have a use
> > case for it.
>
> I'll come back on this tomorrow. It's going to be a merge of your and my code anyway.
>
> I agree it would be nice to add also hwmon + wdog support in it from the start.
>
Hi Wim,

looks like you are busy. Want me to take another stake at it and do the merge ?

Thanks,
Guenter