Groups | Search | Server Info | Keyboard shortcuts | Login | Register [http] [https] [nntp] [nntps]


Groups > linux.kernel > #1317219 > unrolled thread

[PATCH v3] watchdog: Add watchdog timer support for the WinSystems EBC-C384

Started byWilliam Breathitt Gray <vilhelm.gray@gmail.com>
First post2016-01-25 20:10 +0100
Last post2016-01-26 13:40 +0100
Articles 11 — 4 participants

Back to article view | Back to linux.kernel


Contents

  [PATCH v3] watchdog: Add watchdog timer support for the WinSystems  EBC-C384 William Breathitt Gray <vilhelm.gray@gmail.com> - 2016-01-25 20:10 +0100
    Re: [PATCH v3] watchdog: Add watchdog timer support for the  WinSystems EBC-C384 One Thousand Gnomes <gnomes@lxorguk.ukuu.org.uk> - 2016-01-25 20:30 +0100
      Re: [PATCH v3] watchdog: Add watchdog timer support for the  WinSystems EBC-C384 Guenter Roeck <linux@roeck-us.net> - 2016-01-25 21:50 +0100
        Re: [PATCH v3] watchdog: Add watchdog timer support for the  WinSystems EBC-C384 William Breathitt Gray <vilhelm.gray@gmail.com> - 2016-01-26 00:40 +0100
          Re: [PATCH v3] watchdog: Add watchdog timer support for the  WinSystems EBC-C384 Guenter Roeck <linux@roeck-us.net> - 2016-01-26 02:30 +0100
            Re: [PATCH v3] watchdog: Add watchdog timer support for the  WinSystems EBC-C384 William Breathitt Gray <vilhelm.gray@gmail.com> - 2016-01-27 00:40 +0100
              Re: [PATCH v3] watchdog: Add watchdog timer support for the  WinSystems EBC-C384 Guenter Roeck <linux@roeck-us.net> - 2016-01-27 06:10 +0100
          Re: [PATCH v3] watchdog: Add watchdog timer support for the  WinSystems EBC-C384 Guenter Roeck <linux@roeck-us.net> - 2016-01-26 03:00 +0100
    Re: [PATCH v3] watchdog: Add watchdog timer support for the  WinSystems EBC-C384 Guenter Roeck <linux@roeck-us.net> - 2016-01-26 02:20 +0100
    Re: [PATCH v3] watchdog: Add watchdog timer support for the  WinSystems EBC-C384 Paul Bolle <pebolle@tiscali.nl> - 2016-01-26 10:20 +0100
      Re: [PATCH v3] watchdog: Add watchdog timer support for the  WinSystems EBC-C384 William Breathitt Gray <vilhelm.gray@gmail.com> - 2016-01-26 13:40 +0100

#1317219 — [PATCH v3] watchdog: Add watchdog timer support for the WinSystems EBC-C384

FromWilliam Breathitt Gray <vilhelm.gray@gmail.com>
Date2016-01-25 20:10 +0100
Subject[PATCH v3] watchdog: Add watchdog timer support for the WinSystems EBC-C384
Message-ID<qUUGv-8W-29@gated-at.bofh.it>
The WinSystems EBC-C384 has an onboard watchdog timer. The timeout range
supported by the watchdog timer is 1 second to 255 minutes. Timeouts
under 256 seconds have a 1 second resolution, while the rest have a 1
minute resolution.

This driver adds watchdog timer support for this onboard watchdog timer.
The timeout may be configured via the timeout module parameter.

Signed-off-by: William Breathitt Gray <vilhelm.gray@gmail.com>
---
Changes in v3:
  - Remove unnecessary explicit initialization of the timeout parameter
  - Move dmi_match call to beginning of init function
  - Create WATCHDOG_MAX_TIMEOUT define and explain its magic number
  - Use platform_set_drvdata to match later call to platform_get_drvdata
  - Use MODULE_NAME as argument for the platform_device_alloc call
  - Remove duplicate return statement for the init function

 MAINTAINERS                     |   6 ++
 drivers/watchdog/Kconfig        |   9 ++
 drivers/watchdog/Makefile       |   1 +
 drivers/watchdog/ebc-c384_wdt.c | 187 ++++++++++++++++++++++++++++++++++++++++
 4 files changed, 203 insertions(+)
 create mode 100644 drivers/watchdog/ebc-c384_wdt.c

diff --git a/MAINTAINERS b/MAINTAINERS
index b1e3da7..c058abf 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -11629,6 +11629,12 @@ M:	David Härdeman <david@hardeman.nu>
 S:	Maintained
 F:	drivers/media/rc/winbond-cir.c
 
+WINSYSTEMS EBC-C384 WATCHDOG DRIVER
+M:	William Breathitt Gray <vilhelm.gray@gmail.com>
+L:	linux-watchdog@vger.kernel.org
+S:	Maintained
+F:	drivers/watchdog/ebc-c384_wdt.c
+
 WIMAX STACK
 M:	Inaky Perez-Gonzalez <inaky.perez-gonzalez@intel.com>
 M:	linux-wimax@intel.com
diff --git a/drivers/watchdog/Kconfig b/drivers/watchdog/Kconfig
index 4f0e7be..b5b1353 100644
--- a/drivers/watchdog/Kconfig
+++ b/drivers/watchdog/Kconfig
@@ -711,6 +711,15 @@ config ALIM7101_WDT
 
 	  Most people will say N.
 
+config EBC_C386_WDT
+	tristate "WinSystems EBC-C384 Watchdog Timer"
+	depends on X86
+	select WATCHDOG_CORE
+	help
+	  Enables watchdog timer support for the watchdog timer on the
+	  WinSystems EBC-C384 motherboard. The timeout may be configured via
+	  the timeout module parameter.
+
 config F71808E_WDT
 	tristate "Fintek F71808E, F71862FG, F71869, F71882FG and F71889FG Watchdog"
 	depends on X86
diff --git a/drivers/watchdog/Makefile b/drivers/watchdog/Makefile
index f566753..1522316 100644
--- a/drivers/watchdog/Makefile
+++ b/drivers/watchdog/Makefile
@@ -88,6 +88,7 @@ obj-$(CONFIG_ACQUIRE_WDT) += acquirewdt.o
 obj-$(CONFIG_ADVANTECH_WDT) += advantechwdt.o
 obj-$(CONFIG_ALIM1535_WDT) += alim1535_wdt.o
 obj-$(CONFIG_ALIM7101_WDT) += alim7101_wdt.o
+obj-$(CONFIG_EBC_C386_WDT) += ebc-c384_wdt.o
 obj-$(CONFIG_F71808E_WDT) += f71808e_wdt.o
 obj-$(CONFIG_SP5100_TCO) += sp5100_tco.o
 obj-$(CONFIG_GEODE_WDT) += geodewdt.o
diff --git a/drivers/watchdog/ebc-c384_wdt.c b/drivers/watchdog/ebc-c384_wdt.c
new file mode 100644
index 0000000..9166b02
--- /dev/null
+++ b/drivers/watchdog/ebc-c384_wdt.c
@@ -0,0 +1,187 @@
+/*
+ * Watchdog timer driver for the WinSystems EBC-C384
+ * Copyright (C) 2016 William Breathitt Gray
+ *
+ * This program is free software; you can redistribute it and/or modify
+ * it under the terms of the GNU General Public License, version 2, as
+ * published by the Free Software Foundation.
+ *
+ * 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.
+ */
+#include <linux/device.h>
+#include <linux/dmi.h>
+#include <linux/errno.h>
+#include <linux/io.h>
+#include <linux/ioport.h>
+#include <linux/module.h>
+#include <linux/moduleparam.h>
+#include <linux/platform_device.h>
+#include <linux/types.h>
+#include <linux/watchdog.h>
+
+#define MODULE_NAME "ebc-c384_wdt"
+#define WATCHDOG_TIMEOUT 60
+/* Since the timeout value in minutes must fit in a single byte when sent to the
+ * watchdog timer, the maximum timeout possible is 15300 (255 * 60) seconds
+ */
+#define WATCHDOG_MAX_TIMEOUT 15300
+#define BASE_ADDR 0x564
+#define ADDR_EXTENT 5
+#define CFG_ADDR (BASE_ADDR + 1)
+#define PET_ADDR (BASE_ADDR + 2)
+
+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 unsigned timeout;
+module_param(timeout, uint, 0);
+MODULE_PARM_DESC(timeout, "Watchdog timeout in seconds (default="
+	__MODULE_STRING(WATCHDOG_TIMEOUT) ")");
+
+static int ebc_c384_wdt_start(struct watchdog_device *wdev)
+{
+	unsigned t = wdev->timeout;
+
+	/* resolution is in minutes for timeouts greater than 255 seconds */
+	if (t > 255)
+		t /= 60;
+
+	outb(t, PET_ADDR);
+
+	return 0;
+}
+
+static int ebc_c384_wdt_stop(struct watchdog_device *wdev)
+{
+	outb(0x00, PET_ADDR);
+
+	return 0;
+}
+
+static int ebc_c384_wdt_set_timeout(struct watchdog_device *wdev, unsigned t)
+{
+	/* resolution is in minutes for timeouts greater than 255 seconds */
+	if (t > 255) {
+		/* truncate second resolution to minute resolution */
+		t /= 60;
+		wdev->timeout = t * 60;
+
+		/* set watchdog timer for minutes */
+		outb(0x00, CFG_ADDR);
+	} else {
+		wdev->timeout = t;
+
+		/* set watchdog timer for seconds */
+		outb(0x80, CFG_ADDR);
+	}
+
+	return 0;
+}
+
+static const struct watchdog_ops ebc_c384_wdt_ops = {
+	.start = ebc_c384_wdt_start,
+	.stop = ebc_c384_wdt_stop,
+	.set_timeout = ebc_c384_wdt_set_timeout
+};
+
+static const struct watchdog_info ebc_c384_wdt_info = {
+	.options = WDIOF_KEEPALIVEPING | WDIOF_MAGICCLOSE | WDIOF_SETTIMEOUT,
+	.identity = MODULE_NAME
+};
+
+static int __init ebc_c384_wdt_probe(struct platform_device *pdev)
+{
+	struct device *dev = &pdev->dev;
+	struct watchdog_device *wdd;
+
+	if (!devm_request_region(dev, BASE_ADDR, ADDR_EXTENT, dev_name(dev))) {
+		dev_err(dev, "Unable to lock port addresses (0x%X-0x%X)\n",
+			BASE_ADDR, BASE_ADDR + ADDR_EXTENT);
+		return -EBUSY;
+	}
+
+	wdd = devm_kzalloc(dev, sizeof(*wdd), GFP_KERNEL);
+	if (!wdd)
+		return -ENOMEM;
+
+	wdd->info = &ebc_c384_wdt_info;
+	wdd->ops = &ebc_c384_wdt_ops;
+	wdd->timeout = WATCHDOG_TIMEOUT;
+	wdd->min_timeout = 1;
+	wdd->max_timeout = WATCHDOG_MAX_TIMEOUT;
+
+	watchdog_set_nowayout(wdd, nowayout);
+
+	if (watchdog_init_timeout(wdd, timeout, dev))
+		dev_warn(dev, "Invalid timeout (%u seconds), using default (%u seconds)\n",
+			timeout, WATCHDOG_TIMEOUT);
+
+	platform_set_drvdata(pdev, wdd);
+
+	return watchdog_register_device(wdd);
+}
+
+static int ebc_c384_wdt_remove(struct platform_device *pdev)
+{
+	struct watchdog_device *wdd = platform_get_drvdata(pdev);
+
+	watchdog_unregister_device(wdd);
+
+	return 0;
+}
+
+static struct platform_driver ebc_c384_wdt_driver = {
+	.driver = {
+		.name = MODULE_NAME
+	},
+	.remove = ebc_c384_wdt_remove
+};
+
+static struct platform_device *ebc_c384_wdt_device;
+
+static int __init ebc_c384_wdt_init(void)
+{
+	int err;
+
+	if (!dmi_match(DMI_BOARD_NAME, "EBC-C384 SBC"))
+		return -ENODEV;
+
+	ebc_c384_wdt_device = platform_device_alloc(MODULE_NAME, -1);
+	if (!ebc_c384_wdt_device)
+		return -ENOMEM;
+
+	err = platform_device_add(ebc_c384_wdt_device);
+	if (err)
+		goto err_platform_device;
+
+	err = platform_driver_probe(&ebc_c384_wdt_driver, ebc_c384_wdt_probe);
+	if (err)
+		goto err_platform_driver;
+
+	return 0;
+
+err_platform_driver:
+	platform_device_del(ebc_c384_wdt_device);
+err_platform_device:
+	platform_device_put(ebc_c384_wdt_device);
+	return err;
+}
+
+static void __exit ebc_c384_wdt_exit(void)
+{
+	platform_device_unregister(ebc_c384_wdt_device);
+	platform_driver_unregister(&ebc_c384_wdt_driver);
+}
+
+module_init(ebc_c384_wdt_init);
+module_exit(ebc_c384_wdt_exit);
+
+MODULE_AUTHOR("William Breathitt Gray <vilhelm.gray@gmail.com>");
+MODULE_DESCRIPTION("WinSystems EBC-C384 watchdog timer driver");
+MODULE_LICENSE("GPL v2");
+MODULE_ALIAS("platform:" MODULE_NAME);
-- 
2.4.10

[toc] | [next] | [standalone]


#1317232 — Re: [PATCH v3] watchdog: Add watchdog timer support for the WinSystems EBC-C384

FromOne Thousand Gnomes <gnomes@lxorguk.ukuu.org.uk>
Date2016-01-25 20:30 +0100
SubjectRe: [PATCH v3] watchdog: Add watchdog timer support for the WinSystems EBC-C384
Message-ID<qUUZQ-gp-21@gated-at.bofh.it>
In reply to#1317219
> +static int ebc_c384_wdt_set_timeout(struct watchdog_device *wdev, unsigned t)
> +{
> +	/* resolution is in minutes for timeouts greater than 255 seconds */
> +	if (t > 255) {
> +		/* truncate second resolution to minute resolution */
> +		t /= 60;
> +		wdev->timeout = t * 60;
> +
> +		/* set watchdog timer for minutes */
> +		outb(0x00, CFG_ADDR);

If ask for 299 seconds surely I should get 300 not 240 ?
(Whether to round off or round up is an interesting question for the
middle range - does it go off early or late - I'd have said late but...)

> +static int __init ebc_c384_wdt_init(void)
> +{
> +	int err;
> +
> +	if (!dmi_match(DMI_BOARD_NAME, "EBC-C384 SBC"))
> +		return -ENODEV;
> +

Is there no ACPI entry for it ?

Alan

[toc] | [prev] | [next] | [standalone]


#1317297 — Re: [PATCH v3] watchdog: Add watchdog timer support for the WinSystems EBC-C384

FromGuenter Roeck <linux@roeck-us.net>
Date2016-01-25 21:50 +0100
SubjectRe: [PATCH v3] watchdog: Add watchdog timer support for the WinSystems EBC-C384
Message-ID<qUWfh-12K-35@gated-at.bofh.it>
In reply to#1317232
On 01/25/2016 11:28 AM, One Thousand Gnomes wrote:
>> +static int ebc_c384_wdt_set_timeout(struct watchdog_device *wdev, unsigned t)
>> +{
>> +	/* resolution is in minutes for timeouts greater than 255 seconds */
>> +	if (t > 255) {
>> +		/* truncate second resolution to minute resolution */
>> +		t /= 60;
>> +		wdev->timeout = t * 60;
>> +
>> +		/* set watchdog timer for minutes */
>> +		outb(0x00, CFG_ADDR);
>
> If ask for 299 seconds surely I should get 300 not 240 ?
> (Whether to round off or round up is an interesting question for the
> middle range - does it go off early or late - I'd have said late but...)
>

Matter of endless discussion. Some argue that the value should be rounded
up, some argue that it should be rounded down, some argue that it should
be rounded to the closest match. Each camp has its own valid arguments.
I usually leave it up to the driver's author to decide, with a slight
preference to never select a value larger than requested.

>> +static int __init ebc_c384_wdt_init(void)
>> +{
>> +	int err;
>> +
>> +	if (!dmi_match(DMI_BOARD_NAME, "EBC-C384 SBC"))
>> +		return -ENODEV;
>> +
>
> Is there no ACPI entry for it ?
>
Same here. As long as the board is identified, I tend to leave it up
to the driver author to decide _how_ to identify it.

Only question for me would be if the watchdog timer is implemented
in a Super-IO chip, and if so, if it would be possible to use the chip
identification instead of a DMI (or ACPI) entry to instantiate the driver.

Thanks,
Guenter

[toc] | [prev] | [next] | [standalone]


#1317408 — Re: [PATCH v3] watchdog: Add watchdog timer support for the WinSystems EBC-C384

FromWilliam Breathitt Gray <vilhelm.gray@gmail.com>
Date2016-01-26 00:40 +0100
SubjectRe: [PATCH v3] watchdog: Add watchdog timer support for the WinSystems EBC-C384
Message-ID<qUYTM-2XX-7@gated-at.bofh.it>
In reply to#1317297
On 01/25/2016 03:42 PM, Guenter Roeck wrote:
> On 01/25/2016 11:28 AM, One Thousand Gnomes wrote:
>> If ask for 299 seconds surely I should get 300 not 240 ?
>> (Whether to round off or round up is an interesting question for the
>> middle range - does it go off early or late - I'd have said late but...)
>>
> 
> Matter of endless discussion. Some argue that the value should be rounded
> up, some argue that it should be rounded down, some argue that it should
> be rounded to the closest match. Each camp has its own valid arguments.
> I usually leave it up to the driver's author to decide, with a slight
> preference to never select a value larger than requested.

I implemented it to round down simply because it was the simplest        
solution (i.e. integer truncation). Although I see merit in an           
implementation that rounds to the closest valid value, I'll keep the     
current implementation for now due to its simplicity; if enough users of 
the driver prefer a different implementation, then I'll add it in a      
later patch.

>> Is there no ACPI entry for it ?
>>
> Same here. As long as the board is identified, I tend to leave it up
> to the driver author to decide _how_ to identify it.
> 
> Only question for me would be if the watchdog timer is implemented
> in a Super-IO chip, and if so, if it would be possible to use the chip
> identification instead of a DMI (or ACPI) entry to instantiate the driver.

I do not believe there is an ACPI entry for it. Interestingly, the       
watchdog timer BIOS configuration option for this motherboard is listed  
under the Super I/O menu; perhaps this watchdog timer is implemented in  
the Super I/O chip.                                                      
                                                                         
The manual for this motherboard does not provide much information about  
the Super I/O chip (no model number, etc.), and neither sensors-detect    
nor superiotool was able to detect it. I've sent an email to the         
motherboard company (WinSystems) requesting further information about    
the Super I/O chip and whether the watchdog timer is built-in to the     
Super I/O chip.                                                          
                                                                         
William Breathitt Gray

[toc] | [prev] | [next] | [standalone]


#1317446 — Re: [PATCH v3] watchdog: Add watchdog timer support for the WinSystems EBC-C384

FromGuenter Roeck <linux@roeck-us.net>
Date2016-01-26 02:30 +0100
SubjectRe: [PATCH v3] watchdog: Add watchdog timer support for the WinSystems EBC-C384
Message-ID<qV0Ce-4ad-3@gated-at.bofh.it>
In reply to#1317408
On 01/25/2016 03:36 PM, William Breathitt Gray wrote:
> On 01/25/2016 03:42 PM, Guenter Roeck wrote:
>> On 01/25/2016 11:28 AM, One Thousand Gnomes wrote:
>>> If ask for 299 seconds surely I should get 300 not 240 ?
>>> (Whether to round off or round up is an interesting question for the
>>> middle range - does it go off early or late - I'd have said late but...)
>>>
>>
>> Matter of endless discussion. Some argue that the value should be rounded
>> up, some argue that it should be rounded down, some argue that it should
>> be rounded to the closest match. Each camp has its own valid arguments.
>> I usually leave it up to the driver's author to decide, with a slight
>> preference to never select a value larger than requested.
>
> I implemented it to round down simply because it was the simplest
> solution (i.e. integer truncation). Although I see merit in an
> implementation that rounds to the closest valid value, I'll keep the
> current implementation for now due to its simplicity; if enough users of
> the driver prefer a different implementation, then I'll add it in a
> later patch.
>
>>> Is there no ACPI entry for it ?
>>>
>> Same here. As long as the board is identified, I tend to leave it up
>> to the driver author to decide _how_ to identify it.
>>
>> Only question for me would be if the watchdog timer is implemented
>> in a Super-IO chip, and if so, if it would be possible to use the chip
>> identification instead of a DMI (or ACPI) entry to instantiate the driver.
>
> I do not believe there is an ACPI entry for it. Interestingly, the
> watchdog timer BIOS configuration option for this motherboard is listed
> under the Super I/O menu; perhaps this watchdog timer is implemented in
> the Super I/O chip.
>
Normally it is. Question is which one.

> The manual for this motherboard does not provide much information about
> the Super I/O chip (no model number, etc.), and neither sensors-detect
> nor superiotool was able to detect it. I've sent an email to the
> motherboard company (WinSystems) requesting further information about
> the Super I/O chip and whether the watchdog timer is built-in to the
> Super I/O chip.
>

Ah, I somehow thought you were associated with WinSystems, since you know
how to configure the chip.

Did you get any useful output from sensors-detect or superiotool
(like 'unknown chip xxxx'), or did those tools find nothing ?

Thanks,
Guenter

[toc] | [prev] | [next] | [standalone]


#1318475 — Re: [PATCH v3] watchdog: Add watchdog timer support for the WinSystems EBC-C384

FromWilliam Breathitt Gray <vilhelm.gray@gmail.com>
Date2016-01-27 00:40 +0100
SubjectRe: [PATCH v3] watchdog: Add watchdog timer support for the WinSystems EBC-C384
Message-ID<qVlnl-385-27@gated-at.bofh.it>
In reply to#1317446
On 01/25/2016 08:26 PM, Guenter Roeck wrote:
>> The manual for this motherboard does not provide much information about
>> the Super I/O chip (no model number, etc.), and neither sensors-detect
>> nor superiotool was able to detect it. I've sent an email to the
>> motherboard company (WinSystems) requesting further information about
>> the Super I/O chip and whether the watchdog timer is built-in to the
>> Super I/O chip.
>>
> 
> Ah, I somehow thought you were associated with WinSystems, since you know
> how to configure the chip.
> 
> Did you get any useful output from sensors-detect or superiotool
> (like 'unknown chip xxxx'), or did those tools find nothing ?

Unfortunately, the sensors-detect only reported "No" for each Super I/O  
chip test, while the superiotool gave an unhelpful "No Super I/O chip    
detected" message.                                                       
                                                                         
I haven't heard a response yet from WinSystems, but I'll give them a     
couple days before sending another email to their engineering            
department. For now, I'll submit a version 4 of this patch to get the    
minor updates I made out for review; for what its worth, I believe the   
dmi_match method will be sufficient until I get an update from           
WinSystems helping me get a proper check to identify the Super I/O chip. 
                                                                         
William Breathitt Gray

[toc] | [prev] | [next] | [standalone]


#1318632 — Re: [PATCH v3] watchdog: Add watchdog timer support for the WinSystems EBC-C384

FromGuenter Roeck <linux@roeck-us.net>
Date2016-01-27 06:10 +0100
SubjectRe: [PATCH v3] watchdog: Add watchdog timer support for the WinSystems EBC-C384
Message-ID<qVqwF-6UH-3@gated-at.bofh.it>
In reply to#1318475
On 01/26/2016 03:38 PM, William Breathitt Gray wrote:
> On 01/25/2016 08:26 PM, Guenter Roeck wrote:
>>> The manual for this motherboard does not provide much information about
>>> the Super I/O chip (no model number, etc.), and neither sensors-detect
>>> nor superiotool was able to detect it. I've sent an email to the
>>> motherboard company (WinSystems) requesting further information about
>>> the Super I/O chip and whether the watchdog timer is built-in to the
>>> Super I/O chip.
>>>
>>
>> Ah, I somehow thought you were associated with WinSystems, since you know
>> how to configure the chip.
>>
>> Did you get any useful output from sensors-detect or superiotool
>> (like 'unknown chip xxxx'), or did those tools find nothing ?
>
> Unfortunately, the sensors-detect only reported "No" for each Super I/O
> chip test, while the superiotool gave an unhelpful "No Super I/O chip
> detected" message.
>

Too bad. That suggests that the watchdog may in fact be implemented in the fpga.

> I haven't heard a response yet from WinSystems, but I'll give them a
> couple days before sending another email to their engineering
> department. For now, I'll submit a version 4 of this patch to get the
> minor updates I made out for review; for what its worth, I believe the
> dmi_match method will be sufficient until I get an update from
> WinSystems helping me get a proper check to identify the Super I/O chip.
>

I added v4 to my watchdog-next branch and will include it in my pull request
to Wim later in the release cycle. If there are enhancements (eg if WinSystems
comes back with an actual specification), we can implement those as incremental
improvements.

Thanks,
Guenter

[toc] | [prev] | [next] | [standalone]


#1317459 — Re: [PATCH v3] watchdog: Add watchdog timer support for the WinSystems EBC-C384

FromGuenter Roeck <linux@roeck-us.net>
Date2016-01-26 03:00 +0100
SubjectRe: [PATCH v3] watchdog: Add watchdog timer support for the WinSystems EBC-C384
Message-ID<qV15f-4mN-1@gated-at.bofh.it>
In reply to#1317408
On 01/25/2016 03:36 PM, William Breathitt Gray wrote:
> On 01/25/2016 03:42 PM, Guenter Roeck wrote:
>> On 01/25/2016 11:28 AM, One Thousand Gnomes wrote:
>>> If ask for 299 seconds surely I should get 300 not 240 ?
>>> (Whether to round off or round up is an interesting question for the
>>> middle range - does it go off early or late - I'd have said late but...)
>>>
>>
>> Matter of endless discussion. Some argue that the value should be rounded
>> up, some argue that it should be rounded down, some argue that it should
>> be rounded to the closest match. Each camp has its own valid arguments.
>> I usually leave it up to the driver's author to decide, with a slight
>> preference to never select a value larger than requested.
>
> I implemented it to round down simply because it was the simplest
> solution (i.e. integer truncation). Although I see merit in an
> implementation that rounds to the closest valid value, I'll keep the
> current implementation for now due to its simplicity; if enough users of
> the driver prefer a different implementation, then I'll add it in a
> later patch.
>
>>> Is there no ACPI entry for it ?
>>>
>> Same here. As long as the board is identified, I tend to leave it up
>> to the driver author to decide _how_ to identify it.
>>
>> Only question for me would be if the watchdog timer is implemented
>> in a Super-IO chip, and if so, if it would be possible to use the chip
>> identification instead of a DMI (or ACPI) entry to instantiate the driver.
>
> I do not believe there is an ACPI entry for it. Interestingly, the
> watchdog timer BIOS configuration option for this motherboard is listed
> under the Super I/O menu; perhaps this watchdog timer is implemented in
> the Super I/O chip.
>
> The manual for this motherboard does not provide much information about
> the Super I/O chip (no model number, etc.), and neither sensors-detect
> nor superiotool was able to detect it. I've sent an email to the
> motherboard company (WinSystems) requesting further information about
> the Super I/O chip and whether the watchdog timer is built-in to the
> Super I/O chip.
>

I had a close look at the board pictures in the manual. Looks like there
is a NCT7802Y hardware monitoring chip on the board. This suggests that the
Super-IO chip does probably not implement hardware monitoring. Not that
it helps much to know that, but it reduces the possibilities.

There is also a pretty large Lattice FPGA on the board, which makes it at
least somewhat likely that the functionality is implemented in that FPGA.

Guenter

[toc] | [prev] | [next] | [standalone]


#1317444 — Re: [PATCH v3] watchdog: Add watchdog timer support for the WinSystems EBC-C384

FromGuenter Roeck <linux@roeck-us.net>
Date2016-01-26 02:20 +0100
SubjectRe: [PATCH v3] watchdog: Add watchdog timer support for the WinSystems EBC-C384
Message-ID<qV0sz-46L-27@gated-at.bofh.it>
In reply to#1317219
On 01/25/2016 11:09 AM, William Breathitt Gray wrote:
> The WinSystems EBC-C384 has an onboard watchdog timer. The timeout range
> supported by the watchdog timer is 1 second to 255 minutes. Timeouts
> under 256 seconds have a 1 second resolution, while the rest have a 1
> minute resolution.
>
> This driver adds watchdog timer support for this onboard watchdog timer.
> The timeout may be configured via the timeout module parameter.
>
> Signed-off-by: William Breathitt Gray <vilhelm.gray@gmail.com>

Nitpicks only this time. Please fix and resubmit, and feel free to add

Reviewed-by: Guenter Roeck <linux@roeck-us.net>

Thanks,
Guenter

> ---
> Changes in v3:
>    - Remove unnecessary explicit initialization of the timeout parameter
>    - Move dmi_match call to beginning of init function
>    - Create WATCHDOG_MAX_TIMEOUT define and explain its magic number
>    - Use platform_set_drvdata to match later call to platform_get_drvdata
>    - Use MODULE_NAME as argument for the platform_device_alloc call
>    - Remove duplicate return statement for the init function
>
>   MAINTAINERS                     |   6 ++
>   drivers/watchdog/Kconfig        |   9 ++
>   drivers/watchdog/Makefile       |   1 +
>   drivers/watchdog/ebc-c384_wdt.c | 187 ++++++++++++++++++++++++++++++++++++++++
>   4 files changed, 203 insertions(+)
>   create mode 100644 drivers/watchdog/ebc-c384_wdt.c
>
> diff --git a/MAINTAINERS b/MAINTAINERS
> index b1e3da7..c058abf 100644
> --- a/MAINTAINERS
> +++ b/MAINTAINERS
> @@ -11629,6 +11629,12 @@ M:	David Härdeman <david@hardeman.nu>
>   S:	Maintained
>   F:	drivers/media/rc/winbond-cir.c
>
> +WINSYSTEMS EBC-C384 WATCHDOG DRIVER
> +M:	William Breathitt Gray <vilhelm.gray@gmail.com>
> +L:	linux-watchdog@vger.kernel.org
> +S:	Maintained
> +F:	drivers/watchdog/ebc-c384_wdt.c
> +
>   WIMAX STACK
>   M:	Inaky Perez-Gonzalez <inaky.perez-gonzalez@intel.com>
>   M:	linux-wimax@intel.com
> diff --git a/drivers/watchdog/Kconfig b/drivers/watchdog/Kconfig
> index 4f0e7be..b5b1353 100644
> --- a/drivers/watchdog/Kconfig
> +++ b/drivers/watchdog/Kconfig
> @@ -711,6 +711,15 @@ config ALIM7101_WDT
>
>   	  Most people will say N.
>
> +config EBC_C386_WDT
> +	tristate "WinSystems EBC-C384 Watchdog Timer"
> +	depends on X86
> +	select WATCHDOG_CORE
> +	help
> +	  Enables watchdog timer support for the watchdog timer on the
> +	  WinSystems EBC-C384 motherboard. The timeout may be configured via
> +	  the timeout module parameter.
> +
>   config F71808E_WDT
>   	tristate "Fintek F71808E, F71862FG, F71869, F71882FG and F71889FG Watchdog"
>   	depends on X86
> diff --git a/drivers/watchdog/Makefile b/drivers/watchdog/Makefile
> index f566753..1522316 100644
> --- a/drivers/watchdog/Makefile
> +++ b/drivers/watchdog/Makefile
> @@ -88,6 +88,7 @@ obj-$(CONFIG_ACQUIRE_WDT) += acquirewdt.o
>   obj-$(CONFIG_ADVANTECH_WDT) += advantechwdt.o
>   obj-$(CONFIG_ALIM1535_WDT) += alim1535_wdt.o
>   obj-$(CONFIG_ALIM7101_WDT) += alim7101_wdt.o
> +obj-$(CONFIG_EBC_C386_WDT) += ebc-c384_wdt.o
>   obj-$(CONFIG_F71808E_WDT) += f71808e_wdt.o
>   obj-$(CONFIG_SP5100_TCO) += sp5100_tco.o
>   obj-$(CONFIG_GEODE_WDT) += geodewdt.o
> diff --git a/drivers/watchdog/ebc-c384_wdt.c b/drivers/watchdog/ebc-c384_wdt.c
> new file mode 100644
> index 0000000..9166b02
> --- /dev/null
> +++ b/drivers/watchdog/ebc-c384_wdt.c
> @@ -0,0 +1,187 @@
> +/*
> + * Watchdog timer driver for the WinSystems EBC-C384
> + * Copyright (C) 2016 William Breathitt Gray
> + *
> + * This program is free software; you can redistribute it and/or modify
> + * it under the terms of the GNU General Public License, version 2, as
> + * published by the Free Software Foundation.
> + *
> + * 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.
> + */
> +#include <linux/device.h>
> +#include <linux/dmi.h>
> +#include <linux/errno.h>
> +#include <linux/io.h>
> +#include <linux/ioport.h>
> +#include <linux/module.h>
> +#include <linux/moduleparam.h>
> +#include <linux/platform_device.h>
> +#include <linux/types.h>
> +#include <linux/watchdog.h>
> +
> +#define MODULE_NAME "ebc-c384_wdt"
> +#define WATCHDOG_TIMEOUT 60
> +/* Since the timeout value in minutes must fit in a single byte when sent to the
> + * watchdog timer, the maximum timeout possible is 15300 (255 * 60) seconds
> + */

/*
  * Please use standard multi-line comments.
  */

> +#define WATCHDOG_MAX_TIMEOUT 15300
> +#define BASE_ADDR 0x564
> +#define ADDR_EXTENT 5
> +#define CFG_ADDR (BASE_ADDR + 1)
> +#define PET_ADDR (BASE_ADDR + 2)

Please use tabs before the values for alignment.

> +
> +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 unsigned timeout;
> +module_param(timeout, uint, 0);
> +MODULE_PARM_DESC(timeout, "Watchdog timeout in seconds (default="
> +	__MODULE_STRING(WATCHDOG_TIMEOUT) ")");
> +
> +static int ebc_c384_wdt_start(struct watchdog_device *wdev)
> +{
> +	unsigned t = wdev->timeout;
> +
> +	/* resolution is in minutes for timeouts greater than 255 seconds */
> +	if (t > 255)
> +		t /= 60;
> +
> +	outb(t, PET_ADDR);
> +
> +	return 0;
> +}
> +
> +static int ebc_c384_wdt_stop(struct watchdog_device *wdev)
> +{
> +	outb(0x00, PET_ADDR);
> +
> +	return 0;
> +}
> +
> +static int ebc_c384_wdt_set_timeout(struct watchdog_device *wdev, unsigned t)
> +{
> +	/* resolution is in minutes for timeouts greater than 255 seconds */
> +	if (t > 255) {
> +		/* truncate second resolution to minute resolution */
> +		t /= 60;
> +		wdev->timeout = t * 60;
> +
> +		/* set watchdog timer for minutes */
> +		outb(0x00, CFG_ADDR);
> +	} else {
> +		wdev->timeout = t;
> +
> +		/* set watchdog timer for seconds */
> +		outb(0x80, CFG_ADDR);
> +	}
> +
> +	return 0;
> +}
> +
> +static const struct watchdog_ops ebc_c384_wdt_ops = {
> +	.start = ebc_c384_wdt_start,
> +	.stop = ebc_c384_wdt_stop,
> +	.set_timeout = ebc_c384_wdt_set_timeout
> +};
> +
> +static const struct watchdog_info ebc_c384_wdt_info = {
> +	.options = WDIOF_KEEPALIVEPING | WDIOF_MAGICCLOSE | WDIOF_SETTIMEOUT,
> +	.identity = MODULE_NAME
> +};
> +
> +static int __init ebc_c384_wdt_probe(struct platform_device *pdev)
> +{
> +	struct device *dev = &pdev->dev;
> +	struct watchdog_device *wdd;
> +
> +	if (!devm_request_region(dev, BASE_ADDR, ADDR_EXTENT, dev_name(dev))) {
> +		dev_err(dev, "Unable to lock port addresses (0x%X-0x%X)\n",
> +			BASE_ADDR, BASE_ADDR + ADDR_EXTENT);
> +		return -EBUSY;
> +	}
> +
> +	wdd = devm_kzalloc(dev, sizeof(*wdd), GFP_KERNEL);
> +	if (!wdd)
> +		return -ENOMEM;
> +
> +	wdd->info = &ebc_c384_wdt_info;
> +	wdd->ops = &ebc_c384_wdt_ops;
> +	wdd->timeout = WATCHDOG_TIMEOUT;
> +	wdd->min_timeout = 1;
> +	wdd->max_timeout = WATCHDOG_MAX_TIMEOUT;
> +
> +	watchdog_set_nowayout(wdd, nowayout);
> +
> +	if (watchdog_init_timeout(wdd, timeout, dev))
> +		dev_warn(dev, "Invalid timeout (%u seconds), using default (%u seconds)\n",
> +			timeout, WATCHDOG_TIMEOUT);

Multi-line alignment is off by one character.

> +
> +	platform_set_drvdata(pdev, wdd);
> +
> +	return watchdog_register_device(wdd);
> +}
> +
> +static int ebc_c384_wdt_remove(struct platform_device *pdev)
> +{
> +	struct watchdog_device *wdd = platform_get_drvdata(pdev);
> +
> +	watchdog_unregister_device(wdd);
> +
> +	return 0;
> +}
> +
> +static struct platform_driver ebc_c384_wdt_driver = {
> +	.driver = {
> +		.name = MODULE_NAME
> +	},
> +	.remove = ebc_c384_wdt_remove
> +};
> +
> +static struct platform_device *ebc_c384_wdt_device;
> +
> +static int __init ebc_c384_wdt_init(void)
> +{
> +	int err;
> +
> +	if (!dmi_match(DMI_BOARD_NAME, "EBC-C384 SBC"))
> +		return -ENODEV;
> +
> +	ebc_c384_wdt_device = platform_device_alloc(MODULE_NAME, -1);
> +	if (!ebc_c384_wdt_device)
> +		return -ENOMEM;
> +
> +	err = platform_device_add(ebc_c384_wdt_device);
> +	if (err)
> +		goto err_platform_device;
> +
> +	err = platform_driver_probe(&ebc_c384_wdt_driver, ebc_c384_wdt_probe);
> +	if (err)
> +		goto err_platform_driver;
> +
> +	return 0;
> +
> +err_platform_driver:
> +	platform_device_del(ebc_c384_wdt_device);
> +err_platform_device:
> +	platform_device_put(ebc_c384_wdt_device);
> +	return err;
> +}
> +
> +static void __exit ebc_c384_wdt_exit(void)
> +{
> +	platform_device_unregister(ebc_c384_wdt_device);
> +	platform_driver_unregister(&ebc_c384_wdt_driver);
> +}
> +
> +module_init(ebc_c384_wdt_init);
> +module_exit(ebc_c384_wdt_exit);
> +
> +MODULE_AUTHOR("William Breathitt Gray <vilhelm.gray@gmail.com>");
> +MODULE_DESCRIPTION("WinSystems EBC-C384 watchdog timer driver");
> +MODULE_LICENSE("GPL v2");
> +MODULE_ALIAS("platform:" MODULE_NAME);
>

[toc] | [prev] | [next] | [standalone]


#1317657 — Re: [PATCH v3] watchdog: Add watchdog timer support for the WinSystems EBC-C384

FromPaul Bolle <pebolle@tiscali.nl>
Date2016-01-26 10:20 +0100
SubjectRe: [PATCH v3] watchdog: Add watchdog timer support for the WinSystems EBC-C384
Message-ID<qV7X4-1ZG-19@gated-at.bofh.it>
In reply to#1317219
On ma, 2016-01-25 at 14:09 -0500, William Breathitt Gray wrote:
> --- a/drivers/watchdog/Kconfig
> +++ b/drivers/watchdog/Kconfig

> +config EBC_C386_WDT
> +	tristate "WinSystems EBC-C384 Watchdog Timer"
> +	depends on X86
> +	select WATCHDOG_CORE
> +	help
> +	  Enables watchdog timer support for the watchdog timer on the
> +	  WinSystems EBC-C384 motherboard. The timeout may be configured via
> +	  the timeout module parameter.

It's utterly trivial, but I couldn't help noticing that this supports
the "EBC-C384" motherboard but the Kconfig symbol is EBC_C386_WDT (note
the 6). Any particular reason?


Thanks,


Paul Bolle

[toc] | [prev] | [next] | [standalone]


#1317837 — Re: [PATCH v3] watchdog: Add watchdog timer support for the WinSystems EBC-C384

FromWilliam Breathitt Gray <vilhelm.gray@gmail.com>
Date2016-01-26 13:40 +0100
SubjectRe: [PATCH v3] watchdog: Add watchdog timer support for the WinSystems EBC-C384
Message-ID<qVb4C-46X-19@gated-at.bofh.it>
In reply to#1317657
On 01/26/2016 04:09 AM, Paul Bolle wrote:
> On ma, 2016-01-25 at 14:09 -0500, William Breathitt Gray wrote:
>> --- a/drivers/watchdog/Kconfig
>> +++ b/drivers/watchdog/Kconfig
> 
>> +config EBC_C386_WDT
>> +	tristate "WinSystems EBC-C384 Watchdog Timer"
>> +	depends on X86
>> +	select WATCHDOG_CORE
>> +	help
>> +	  Enables watchdog timer support for the watchdog timer on the
>> +	  WinSystems EBC-C384 motherboard. The timeout may be configured via
>> +	  the timeout module parameter.
> 
> It's utterly trivial, but I couldn't help noticing that this supports
> the "EBC-C384" motherboard but the Kconfig symbol is EBC_C386_WDT (note
> the 6). Any particular reason?

Looks like you caught a typo of mine! I'll fix it in my next patch. Many 
thanks Paul for your good eye.                                           
                                                                         
William Breathitt Gray 

[toc] | [prev] | [standalone]


Back to top | Article view | linux.kernel


csiph-web