Messages in this thread Patch in this message | | | Date | Wed, 17 Jan 2018 12:53:41 +0100 | From | Takashi Iwai <> | Subject | Re: 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
| |