Linux-Watchdog Archive mirror
 help / color / mirror / Atom feed
* [PATCH] watchdog: via_wdt: Partial fix for programming error and support additional southbridges
@ 2015-05-18 10:50 Nick & John
  2015-05-18 13:15 ` Guenter Roeck
  2015-05-18 13:17 ` Guenter Roeck
  0 siblings, 2 replies; 3+ messages in thread
From: Nick & John @ 2015-05-18 10:50 UTC (permalink / raw
  To: linux-watchdog

    This patch fixes a programming error by limiting WDT_TIMEOUT_MAX to 255.
    That is done to avoid invalid settings. Putting a value bigger than 255
    results in non working watchdog and reset is not performed. We are trying
    to fit a 10bit value to an 8bit area! To support 1023 seconds MMIO addr+5
    bit1:0 must be used to store the last(higher) two bits of count number.
    Strangely no error was logged at the system when using values bigger than
    255, the driver just ignores the time count.

    Also it adds support for VT8235 and VT8237 series southbridges. Tested with
    VT8235M, VT8237 and VT8237S boards.

    Sending patch to stable too since it seems like an overflow error if big
    timeout values are chosen and because those chipsets were very popular in
    x86 boards supporting processors from all vendors (AMD, intel and VIA).

    Signed-off-by: Ioannis Barkas <risc4all@yahoo.com>
    Signed-off-by: Nikos Barkas <levelwol@gmail.com>
    Cc: <stable@vger.kernel.org>

---

Hello we are Ioannis Barkas (risc4all@yahoo.com) and Nikos Barkas
(levelwol@gmail.com).

 We have been poking with the watchdog subsystem. We tested sp5100_tco
on our AMD boards
with various results. On one of those boards MMIO was disabled.
Opening the MMIO space
results in system freeze!! As a result we continued with the remaining
drivers. A quick
search at our datasheets proved that some of our VIA boards have
watchdog timer but none
was supported yet by the linux driver. Reading the via_wdt code to add
missing southbridge
pointed out another error! A patch(see above) was created to fix it
and add what was missing.
The patch was tested with 3.18.13 but will apply to the latest kernel
since this driver has
not changed for quite some time.
 After creating and testing the patch, we remembered an old kernel
patch at PCI quirks
enabling HPET on VIA chipsets. That patch was targeted on chipsets
where HPET support
was undocumented.  With that in mind, we will test our sole VT8233
board for watchdog
even though the datasheet does not mention watchdog support! Maybe an
early version of
the IP used in next southbridges is present and possibly functional.
 This driver should also support power off support using VIA_WDT_PWROFF.

 In our ASRock 8235M board, ACPI had to be disabled to avoid IO ports
address conflict that
prevented the driver from enabling the device. Without ACPI the driver
was functional. The
ACPI issue is caused by poor consumer grade generic ASL code in AMI
BIOS. The value of
PBlockAddress(810) at processor name scope having length of 6 is used
again at an IO Port
Descriptor(800) having length 80. As a result 810 to 815 is claimed twice!


--- linux-3.18.13/drivers/watchdog/Kconfig    2015-05-05
19:39:05.000000000 +0300
+++ linux-3.18.13-via-dog/drivers/watchdog/Kconfig    2015-05-13
14:41:05.858355595 +0300
@@ -982,7 +982,9 @@ config VIA_WDT
     select WATCHDOG_CORE
     ---help---
     This is the driver for the hardware watchdog timer on VIA
-    southbridge chipset CX700, VX800/VX820 or VX855/VX875.
+    single chip chipsets: CX700, VX800/VX820, VX855/VX875 and
+    on VIA southbridges: VT8235 series, VT8237 series and
+        VT8237S.

     To compile this driver as a module, choose M here; the module
     will be called via_wdt.
diff -upr linux-3.18.13/drivers/watchdog/via_wdt.c
linux-3.18.13-via-dog/drivers/watchdog/via_wdt.c
--- linux-3.18.13/drivers/watchdog/via_wdt.c    2015-05-05
19:39:05.000000000 +0300
+++ linux-3.18.13-via-dog/drivers/watchdog/via_wdt.c    2015-05-13
14:34:02.602339475 +0300
@@ -51,11 +51,15 @@
 #define WDT_HEARTBEAT    (HZ/2)    /* should be <= ((WDT_HW_HEARTBEAT*HZ)/2) */

 /* User space timeout in seconds */
-#define WDT_TIMEOUT_MAX    1023    /* approx. 17 min. */
+#define WDT_TIMEOUT_MAX    255    /* approx. 4 min.(255 sec) */
+/*
+ * Hardware can support up to 17 minutes(1023 seconds) but MMIO addr+5
+ * bits 1:0 must be used too.
+ */
 #define WDT_TIMEOUT    60
 static int timeout = WDT_TIMEOUT;
 module_param(timeout, int, 0);
-MODULE_PARM_DESC(timeout, "Watchdog timeout in seconds, between 1 and 1023 "
+MODULE_PARM_DESC(timeout, "Watchdog timeout in seconds, between 1 and 255 "
     "(default = " __MODULE_STRING(WDT_TIMEOUT) ")");

 static bool nowayout = WATCHDOG_NOWAYOUT;
@@ -240,6 +244,10 @@ static void wdt_remove(struct pci_dev *p
 }

 static const struct pci_device_id wdt_pci_table[] = {
+    { PCI_DEVICE(PCI_VENDOR_ID_VIA, PCI_DEVICE_ID_VIA_8235) },
+    { PCI_DEVICE(PCI_VENDOR_ID_VIA, PCI_DEVICE_ID_VIA_8237) },
+    { PCI_DEVICE(PCI_VENDOR_ID_VIA, PCI_DEVICE_ID_VIA_8237A) },
+    { PCI_DEVICE(PCI_VENDOR_ID_VIA, PCI_DEVICE_ID_VIA_8237S) },
     { PCI_DEVICE(PCI_VENDOR_ID_VIA, PCI_DEVICE_ID_VIA_CX700) },
     { PCI_DEVICE(PCI_VENDOR_ID_VIA, PCI_DEVICE_ID_VIA_VX800) },
     { PCI_DEVICE(PCI_VENDOR_ID_VIA, PCI_DEVICE_ID_VIA_VX855) },

^ permalink raw reply	[flat|nested] 3+ messages in thread

* Re: [PATCH] watchdog: via_wdt: Partial fix for programming error and support additional southbridges
  2015-05-18 10:50 [PATCH] watchdog: via_wdt: Partial fix for programming error and support additional southbridges Nick & John
@ 2015-05-18 13:15 ` Guenter Roeck
  2015-05-18 13:17 ` Guenter Roeck
  1 sibling, 0 replies; 3+ messages in thread
From: Guenter Roeck @ 2015-05-18 13:15 UTC (permalink / raw
  To: Nick & John, linux-watchdog

On 05/18/2015 03:50 AM, Nick & John wrote:
>      This patch fixes a programming error by limiting WDT_TIMEOUT_MAX to 255.
>      That is done to avoid invalid settings. Putting a value bigger than 255
>      results in non working watchdog and reset is not performed. We are trying
>      to fit a 10bit value to an 8bit area! To support 1023 seconds MMIO addr+5
>      bit1:0 must be used to store the last(higher) two bits of count number.
>      Strangely no error was logged at the system when using values bigger than
>      255, the driver just ignores the time count.
>

While I have no idea why, the watchdog in this driver is pinged using a soft timer,
and it is always pinged every 0.5 seconds. So reducing the maximum timeout is not
necessary; in fact the driver could support a higher timeout.

The real fix, if it is really necessary, would be to either store the upper bits
as suggested above, or to just program the chip to a timeout of 255 seconds
if the desired timeout is larger than 255.

>      Also it adds support for VT8235 and VT8237 series southbridges. Tested with
>      VT8235M, VT8237 and VT8237S boards.
>
This should be a separate patch; see Documentation/SubmittingPatches, chapter 3.

Thanks,
Guenter


^ permalink raw reply	[flat|nested] 3+ messages in thread

* Re: [PATCH] watchdog: via_wdt: Partial fix for programming error and support additional southbridges
  2015-05-18 10:50 [PATCH] watchdog: via_wdt: Partial fix for programming error and support additional southbridges Nick & John
  2015-05-18 13:15 ` Guenter Roeck
@ 2015-05-18 13:17 ` Guenter Roeck
  1 sibling, 0 replies; 3+ messages in thread
From: Guenter Roeck @ 2015-05-18 13:17 UTC (permalink / raw
  To: Nick & John, linux-watchdog

On 05/18/2015 03:50 AM, Nick & John wrote:
>      This patch fixes a programming error by limiting WDT_TIMEOUT_MAX to 255.
>      That is done to avoid invalid settings. Putting a value bigger than 255
>      results in non working watchdog and reset is not performed. We are trying
>      to fit a 10bit value to an 8bit area! To support 1023 seconds MMIO addr+5
>      bit1:0 must be used to store the last(higher) two bits of count number.
>      Strangely no error was logged at the system when using values bigger than
>      255, the driver just ignores the time count.
>
>      Also it adds support for VT8235 and VT8237 series southbridges. Tested with
>      VT8235M, VT8237 and VT8237S boards.
>
>      Sending patch to stable too since it seems like an overflow error if big
>      timeout values are chosen and because those chipsets were very popular in
>      x86 boards supporting processors from all vendors (AMD, intel and VIA).
>
>      Signed-off-by: Ioannis Barkas <risc4all@yahoo.com>
>      Signed-off-by: Nikos Barkas <levelwol@gmail.com>

Another note: From: must contain "Nikos Barkas", not "Nick & John".

Guenter


^ permalink raw reply	[flat|nested] 3+ messages in thread

end of thread, other threads:[~2015-05-18 13:17 UTC | newest]

Thread overview: 3+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2015-05-18 10:50 [PATCH] watchdog: via_wdt: Partial fix for programming error and support additional southbridges Nick & John
2015-05-18 13:15 ` Guenter Roeck
2015-05-18 13:17 ` Guenter Roeck

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).