Groups | Search | Server Info | Keyboard shortcuts | Login | Register [http] [https] [nntp] [nntps]
Groups > linux.kernel > #1317219 > unrolled thread
| Started by | William Breathitt Gray <vilhelm.gray@gmail.com> |
|---|---|
| First post | 2016-01-25 20:10 +0100 |
| Last post | 2016-01-26 13:40 +0100 |
| Articles | 11 — 4 participants |
Back to article view | Back to linux.kernel
[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
| From | William Breathitt Gray <vilhelm.gray@gmail.com> |
|---|---|
| Date | 2016-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]
| From | One Thousand Gnomes <gnomes@lxorguk.ukuu.org.uk> |
|---|---|
| Date | 2016-01-25 20:30 +0100 |
| Subject | Re: [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]
| From | Guenter Roeck <linux@roeck-us.net> |
|---|---|
| Date | 2016-01-25 21:50 +0100 |
| Subject | Re: [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]
| From | William Breathitt Gray <vilhelm.gray@gmail.com> |
|---|---|
| Date | 2016-01-26 00:40 +0100 |
| Subject | Re: [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]
| From | Guenter Roeck <linux@roeck-us.net> |
|---|---|
| Date | 2016-01-26 02:30 +0100 |
| Subject | Re: [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]
| From | William Breathitt Gray <vilhelm.gray@gmail.com> |
|---|---|
| Date | 2016-01-27 00:40 +0100 |
| Subject | Re: [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]
| From | Guenter Roeck <linux@roeck-us.net> |
|---|---|
| Date | 2016-01-27 06:10 +0100 |
| Subject | Re: [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]
| From | Guenter Roeck <linux@roeck-us.net> |
|---|---|
| Date | 2016-01-26 03:00 +0100 |
| Subject | Re: [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]
| From | Guenter Roeck <linux@roeck-us.net> |
|---|---|
| Date | 2016-01-26 02:20 +0100 |
| Subject | Re: [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]
| From | Paul Bolle <pebolle@tiscali.nl> |
|---|---|
| Date | 2016-01-26 10:20 +0100 |
| Subject | Re: [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]
| From | William Breathitt Gray <vilhelm.gray@gmail.com> |
|---|---|
| Date | 2016-01-26 13:40 +0100 |
| Subject | Re: [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