lkml.org 
[lkml]   [2018]   [Jan]   [17]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
Patch in this message
/
Date
From
SubjectRe: Missing watchdog after ACPI watchdog creation failure
On Wed, 10 Jan 2018 16:43:23 +0100,
Takashi Iwai wrote:
>
> On Wed, 10 Jan 2018 16:23:43 +0100,
> Mika Westerberg wrote:
> >
> > On Wed, Jan 10, 2018 at 04:13:51PM +0100, Takashi Iwai wrote:
> > > Hi,
> > >
> > > on the recent kernels, i2c-i801 skips the creation of iTCO wdt when
> > > ACPI WDAT is present. It's fine when ACPI really creates the watchdog
> > > device. But, we've got a report showing that the watchdog is missing
> > > on some machines because ACPI failed to create, and yet i2c-i801 still
> > > skips because acpi_has_watchdog() returns true.
> > >
> > > More specifically, the machine gets an error from acpi_watchdog.c
> > > like:
> > > platform wdat_wdt: failed to claim resource 3: [io 0x040a-0x040c]
> > > ACPI: watchdog: Device creation failed: -16
> > >
> > > where the region was registered by pnp,
> > > % cat /proc/ioports
> > > ....
> > > 0400-047f : pnp 00:01
> > >
> > >
> > > One may say that BIOS sucks, but OTOH, the complete lack of watchdog
> > > thereafter can be seen as a regression, too. It used to work on the
> > > older kernel as iTCO wdt was provided by i2c-i801.
> >
> > Hmm, if the resource is already taken I wonder how iTCO can work? Are
> > you sure iTCO works on those systems?
>
> Yes, that's the reason we got a bug report :)
> 4.4 kernel worked, and 4.12 (and later) don't.
>
> On 4.4.x,
> % /proc/ioports
> 0000-0cf7 : PCI Bus 0000:00
> ...
> 0400-047f : pnp 00:01
> 0400-041f : iTCO_wdt
> 0400-041f : iTCO_wdt
> 0500-0503 : ACPI PM1a_EVT_BLK
>
> On 4.12.x,
> % /proc/ioports
> 0000-0cf7 : PCI Bus 0000:00
> ...
> 0400-047f : pnp 00:01
> 0500-053f : pnp 00:01
>
> > > Shouldn't acpi_has_watchdog() rather checks whether the watchdog
> > > device creation succeeded or not?
> >
> > Yes, or rather we should first figure out what the actual problem is ;-)
> >
> > Are you able to get acpidump from that system with full dmesg?
>
> I'll ask the reporter.

Unfortunately we couldn't get approval yet, since it's a prototype
machine.

Meanwhile, the reporter tested the patch below and confirmed to work.
(It might be racy for acpi_has_watchdog() call during the probe, but
you see the idea.)


thanks,

Takashi

-- 8< --
From: Takashi Iwai <tiwai@suse.de>
Subject: [PATCH] ACPI / watchdog: Return probe fail state from acpi_has_watchdog()

The commit 058dfc767008 ("ACPI / watchdog: Add support for WDAT
hardware watchdog") introduced the WDAT watchdog support and prefers
it over iTCO if present. However, this introduced a regression on a
machine with funky BIOS that doesn't set up WDAT resources properly.
Since acpi_has_watchdog() checks only the presence of WDAT entry and
it doesn't care whether the acpi watchdog device creation succeeded or
not, it ends up with no watchdog.

This patch fixes acpi_has_wathdog() to return false if the acpi
watchdog probe failed, so that iTCO watchdog can be created as a
fallback.

Signed-off-by: Takashi Iwai <tiwai@suse.de>
---
drivers/acpi/acpi_watchdog.c | 8 +++++++-
1 file changed, 7 insertions(+), 1 deletion(-)

diff --git a/drivers/acpi/acpi_watchdog.c b/drivers/acpi/acpi_watchdog.c
index 11b113f8e367..b421081f0948 100644
--- a/drivers/acpi/acpi_watchdog.c
+++ b/drivers/acpi/acpi_watchdog.c
@@ -17,6 +17,8 @@

#include "internal.h"

+static bool watchdog_failed;
+
/**
* Returns true if this system should prefer ACPI based watchdog instead of
* the native one (which are typically the same hardware).
@@ -25,7 +27,7 @@ bool acpi_has_watchdog(void)
{
struct acpi_table_header hdr;

- if (acpi_disabled)
+ if (acpi_disabled || watchdog_failed)
return false;

return ACPI_SUCCESS(acpi_get_table_header(ACPI_SIG_WDAT, 0, &hdr));
@@ -44,6 +46,8 @@ void __init acpi_watchdog_init(void)
acpi_status status;
int i;

+ watchdog_failed = true;
+
status = acpi_get_table(ACPI_SIG_WDAT, 0,
(struct acpi_table_header **)&wdat);
if (ACPI_FAILURE(status)) {
@@ -120,6 +124,8 @@ void __init acpi_watchdog_init(void)
resources, nresources);
if (IS_ERR(pdev))
pr_err("Device creation failed: %ld\n", PTR_ERR(pdev));
+ else
+ watchdog_failed = false; /* probe success */

kfree(resources);

--
2.15.1
\
 
 \ /
  Last update: 2018-01-18 00:20    [W:0.134 / U:1.104 seconds]
©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site