View Issue Details

IDProjectCategoryView StatusLast Update
0015830CentOS-7kernelpublic2019-03-14 17:59
Reporterpgreco 
PrioritynormalSeverityminorReproducibilityalways
Status assignedResolutionopen 
Product Version7.6.1810 
Target VersionFixed in Version 
Summary0015830: rtc_cmos: probe of 00:01 failed with error -16
DescriptionThe following errors appear while booting, and system doesn't have access to rtc
rtc_cmos: probe of 00:01 failed with error -16
drivers/rtc/hctosys.c: unable to open rtc device (rtc0)

Local track of the upstream bug, just to be added to kernel-plus
TagsNo tags attached.
abrt_hash
URL

Activities

pgreco

pgreco

2019-02-15 15:18

developer   ~0033841

upstream bug https://bugzilla.redhat.com/show_bug.cgi?id=1663637
pgreco

pgreco

2019-02-15 15:20

developer   ~0033842

Backport fix from git.kernel.org

rtc_wdat.patch (12,216 bytes)
From c6e2c1e1138fa5cb5bb99381132855bbaf712029 Mon Sep 17 00:00:00 2001
From: Mika Westerberg <mika.westerberg@linux.intel.com>
Date: Thu, 22 Dec 2016 13:17:07 +0300
Subject: ACPI / watchdog: Print out error number when device creation fails

If the platform device creation fails for whichever reason the driver
prints out something like:

  [    0.978837] ACPI: watchdog: Failed to create platform device

However, that is quite confusing and does not include any information
why it failed. To make it more understandable, reword it like:

  [    0.978837] ACPI: watchdog: Device creation failed: -16

Which tells that we failed to create the watchdog device because some of
the resources were already reserved (-EBUSY).

Signed-off-by: Mika Westerberg <mika.westerberg@linux.intel.com>
Signed-off-by: Rafael J. Wysocki <rafael.j.wysocki@intel.com>
---
 drivers/acpi/acpi_watchdog.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/acpi/acpi_watchdog.c b/drivers/acpi/acpi_watchdog.c
index 13caebd679f5..8c4e0a18460a 100644
--- a/drivers/acpi/acpi_watchdog.c
+++ b/drivers/acpi/acpi_watchdog.c
@@ -114,7 +114,7 @@ void __init acpi_watchdog_init(void)
 	pdev = platform_device_register_simple("wdat_wdt", PLATFORM_DEVID_NONE,
 					       resources, nresources);
 	if (IS_ERR(pdev))
-		pr_err("Failed to create platform device\n");
+		pr_err("Device creation failed: %ld\n", PTR_ERR(pdev));
 
 	kfree(resources);
 
-- 
cgit 1.2-0.3.lf.el7

From 31e86cb99a3af0653f0e317fdd9c05b530c70af8 Mon Sep 17 00:00:00 2001
From: Ryan Kennedy <ryan5544@gmail.com>
Date: Sat, 15 Jul 2017 17:48:18 -0400
Subject: ACPI / watchdog: Fix init failure with overlapping register regions

Partially overlapping regions cause platform device creation
to fail. The latter of two overlapping resources will fail to be
reserved. Fix this by merging overlapping resource ranges while
enumerating WDAT table entries.

Signed-off-by: Ryan Kennedy <ryan5544@gmail.com>
Acked-by: Mika Westerberg <mika.westerberg@linux.intel.com>
Signed-off-by: Rafael J. Wysocki <rafael.j.wysocki@intel.com>
---
 drivers/acpi/acpi_watchdog.c | 7 ++++++-
 1 file changed, 6 insertions(+), 1 deletion(-)

diff --git a/drivers/acpi/acpi_watchdog.c b/drivers/acpi/acpi_watchdog.c
index 8c4e0a18460a..bf22c29d2517 100644
--- a/drivers/acpi/acpi_watchdog.c
+++ b/drivers/acpi/acpi_watchdog.c
@@ -86,7 +86,12 @@ void __init acpi_watchdog_init(void)
 
 		found = false;
 		resource_list_for_each_entry(rentry, &resource_list) {
-			if (resource_contains(rentry->res, &res)) {
+			if (rentry->res->flags == res.flags &&
+			    resource_overlaps(rentry->res, &res)) {
+				if (res.start < rentry->res->start)
+					rentry->res->start = res.start;
+				if (res.end > rentry->res->end)
+					rentry->res->end = res.end;
 				found = true;
 				break;
 			}
-- 
cgit 1.2-0.3.lf.el7

From 6ce14f6416c84bd9c81777edf899b57ac5000c87 Mon Sep 17 00:00:00 2001
From: Arnd Bergmann <arnd@arndb.de>
Date: Tue, 19 Sep 2017 01:49:02 +0200
Subject: ACPI / watchdog: properly initialize resources

We copy a local resource structure into a list, but only
initialize some of its members, as pointed out by gcc-4.4:

drivers/acpi/acpi_watchdog.c: In function 'acpi_watchdog_init':
drivers/acpi/acpi_watchdog.c:105: error: 'res.child' may be used uninitialized in this function
drivers/acpi/acpi_watchdog.c:105: error: 'res.sibling' may be used uninitialized in this function
drivers/acpi/acpi_watchdog.c:105: error: 'res.parent' may be used uninitialized in this function
drivers/acpi/acpi_watchdog.c:105: error: 'res.desc' may be used uninitialized in this function
drivers/acpi/acpi_watchdog.c:105: error: 'res.name' may be used uninitialized in this function

Newer compilers can presumably optimize the uninitialized access
away entirely and don't warn at all, but rely on the kzalloc()
to zero the structure first. This adds an explicit initialization
to force consistent behavior.

Fixes: 058dfc767008 (ACPI / watchdog: Add support for WDAT hardware watchdog)
Signed-off-by: Arnd Bergmann <arnd@arndb.de>
Acked-by: Guenter Roeck <linux@roeck-us.net>
Acked-by: Mika Westerberg <mika.westerberg@linux.intel.com>
Signed-off-by: Rafael J. Wysocki <rafael.j.wysocki@intel.com>
---
 drivers/acpi/acpi_watchdog.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/acpi/acpi_watchdog.c b/drivers/acpi/acpi_watchdog.c
index bf22c29d2517..11b113f8e367 100644
--- a/drivers/acpi/acpi_watchdog.c
+++ b/drivers/acpi/acpi_watchdog.c
@@ -66,7 +66,7 @@ void __init acpi_watchdog_init(void)
 	for (i = 0; i < wdat->entries; i++) {
 		const struct acpi_generic_address *gas;
 		struct resource_entry *rentry;
-		struct resource res;
+		struct resource res = {};
 		bool found;
 
 		gas = &entries[i].register_region;
-- 
cgit 1.2-0.3.lf.el7

From a0a37862a4e1844793d39aca9ccb8fecbdcb8659 Mon Sep 17 00:00:00 2001
From: Mika Westerberg <mika.westerberg@linux.intel.com>
Date: Mon, 23 Apr 2018 14:16:03 +0300
Subject: ACPI / watchdog: Prefer iTCO_wdt on Lenovo Z50-70

WDAT table on Lenovo Z50-70 is using RTC SRAM (ports 0x70 and 0x71) to
store state of the timer. This conflicts with Linux RTC driver
(rtc-cmos.c) who fails to reserve those ports for itself preventing RTC
from functioning. In addition the WDAT table seems not to be fully
functional because it does not reset the system when the watchdog times
out.

On this system iTCO_wdt works just fine so we simply prefer to use it
instead of WDAT. This makes RTC working again and also results working
watchdog via iTCO_wdt.

Reported-by: Peter Milley <pbmilley@gmail.com>
Link: https://bugzilla.kernel.org/show_bug.cgi?id=199033
Signed-off-by: Mika Westerberg <mika.westerberg@linux.intel.com>
Signed-off-by: Rafael J. Wysocki <rafael.j.wysocki@intel.com>
---
 drivers/acpi/acpi_watchdog.c | 59 ++++++++++++++++++++++++++++++++++++--------
 1 file changed, 49 insertions(+), 10 deletions(-)

diff --git a/drivers/acpi/acpi_watchdog.c b/drivers/acpi/acpi_watchdog.c
index ebb626ffb5fa..4bde16fb97d8 100644
--- a/drivers/acpi/acpi_watchdog.c
+++ b/drivers/acpi/acpi_watchdog.c
@@ -12,23 +12,64 @@
 #define pr_fmt(fmt) "ACPI: watchdog: " fmt
 
 #include <linux/acpi.h>
+#include <linux/dmi.h>
 #include <linux/ioport.h>
 #include <linux/platform_device.h>
 
 #include "internal.h"
 
+static const struct dmi_system_id acpi_watchdog_skip[] = {
+	{
+		/*
+		 * On Lenovo Z50-70 there are two issues with the WDAT
+		 * table. First some of the instructions use RTC SRAM
+		 * to store persistent information. This does not work well
+		 * with Linux RTC driver. Second, more important thing is
+		 * that the instructions do not actually reset the system.
+		 *
+		 * On this particular system iTCO_wdt seems to work just
+		 * fine so we prefer that over WDAT for now.
+		 *
+		 * See also https://bugzilla.kernel.org/show_bug.cgi?id=199033.
+		 */
+		.ident = "Lenovo Z50-70",
+		.matches = {
+			DMI_MATCH(DMI_SYS_VENDOR, "LENOVO"),
+			DMI_MATCH(DMI_PRODUCT_NAME, "20354"),
+			DMI_MATCH(DMI_PRODUCT_VERSION, "Lenovo Z50-70"),
+		},
+	},
+	{}
+};
+
+static const struct acpi_table_wdat *acpi_watchdog_get_wdat(void)
+{
+	const struct acpi_table_wdat *wdat = NULL;
+	acpi_status status;
+
+	if (acpi_disabled)
+		return NULL;
+
+	if (dmi_check_system(acpi_watchdog_skip))
+		return NULL;
+
+	status = acpi_get_table(ACPI_SIG_WDAT, 0,
+				(struct acpi_table_header **)&wdat);
+	if (ACPI_FAILURE(status)) {
+		/* It is fine if there is no WDAT */
+		return NULL;
+	}
+
+	return wdat;
+}
+
 /**
  * Returns true if this system should prefer ACPI based watchdog instead of
  * the native one (which are typically the same hardware).
  */
 bool acpi_has_watchdog(void)
 {
-	struct acpi_table_header hdr;
-
-	if (acpi_disabled)
-		return false;
-
-	return ACPI_SUCCESS(acpi_get_table_header(ACPI_SIG_WDAT, 0, &hdr));
+	return !!acpi_watchdog_get_wdat();
 }
 EXPORT_SYMBOL_GPL(acpi_has_watchdog);
 
@@ -41,12 +82,10 @@ void __init acpi_watchdog_init(void)
 	struct platform_device *pdev;
 	struct resource *resources;
 	size_t nresources = 0;
-	acpi_status status;
 	int i;
 
-	status = acpi_get_table(ACPI_SIG_WDAT, 0,
-				(struct acpi_table_header **)&wdat);
-	if (ACPI_FAILURE(status)) {
+	wdat = acpi_watchdog_get_wdat();
+	if (!wdat) {
 		/* It is fine if there is no WDAT */
 		return;
 	}
-- 
cgit 1.2-0.3.lf.el7

From 5a802a7a285c8877ca872e44eeb0f06afcb5212f Mon Sep 17 00:00:00 2001
From: Mika Westerberg <mika.westerberg@linux.intel.com>
Date: Tue, 22 May 2018 14:16:50 +0300
Subject: ACPI / watchdog: Prefer iTCO_wdt always when WDAT table uses RTC SRAM

After we added quirk for Lenovo Z50-70 it turns out there are at least
two more systems where WDAT table includes instructions accessing RTC
SRAM. Instead of quirking each system separately, look for such
instructions in the table and automatically prefer iTCO_wdt if found.

Link: https://bugzilla.kernel.org/show_bug.cgi?id=199033
Reported-by: Arnold Guy <aurnoldg@gmail.com>
Reported-by: Alois Nespor <nespor@fssp.cz>
Reported-by: Yury Pakin <zxwarior@gmail.com>
Reported-by: Ihor Chyhin <ihorchyhin@ukr.net>
Signed-off-by: Mika Westerberg <mika.westerberg@linux.intel.com>
Acked-by: Guenter Roeck <linux@roeck-us.net>
Signed-off-by: Rafael J. Wysocki <rafael.j.wysocki@intel.com>
---
 drivers/acpi/acpi_watchdog.c | 72 +++++++++++++++++++++++++++-----------------
 1 file changed, 45 insertions(+), 27 deletions(-)

diff --git a/drivers/acpi/acpi_watchdog.c b/drivers/acpi/acpi_watchdog.c
index 4bde16fb97d8..95600309ce42 100644
--- a/drivers/acpi/acpi_watchdog.c
+++ b/drivers/acpi/acpi_watchdog.c
@@ -12,35 +12,51 @@
 #define pr_fmt(fmt) "ACPI: watchdog: " fmt
 
 #include <linux/acpi.h>
-#include <linux/dmi.h>
 #include <linux/ioport.h>
 #include <linux/platform_device.h>
 
 #include "internal.h"
 
-static const struct dmi_system_id acpi_watchdog_skip[] = {
-	{
-		/*
-		 * On Lenovo Z50-70 there are two issues with the WDAT
-		 * table. First some of the instructions use RTC SRAM
-		 * to store persistent information. This does not work well
-		 * with Linux RTC driver. Second, more important thing is
-		 * that the instructions do not actually reset the system.
-		 *
-		 * On this particular system iTCO_wdt seems to work just
-		 * fine so we prefer that over WDAT for now.
-		 *
-		 * See also https://bugzilla.kernel.org/show_bug.cgi?id=199033.
-		 */
-		.ident = "Lenovo Z50-70",
-		.matches = {
-			DMI_MATCH(DMI_SYS_VENDOR, "LENOVO"),
-			DMI_MATCH(DMI_PRODUCT_NAME, "20354"),
-			DMI_MATCH(DMI_PRODUCT_VERSION, "Lenovo Z50-70"),
-		},
-	},
-	{}
-};
+#ifdef CONFIG_RTC_LIB
+#include <linux/mc146818rtc.h>
+
+/*
+ * There are several systems where the WDAT table is accessing RTC SRAM to
+ * store persistent information. This does not work well with the Linux RTC
+ * driver so on those systems we skip WDAT driver and prefer iTCO_wdt
+ * instead.
+ *
+ * See also https://bugzilla.kernel.org/show_bug.cgi?id=199033.
+ */
+static bool acpi_watchdog_uses_rtc(const struct acpi_table_wdat *wdat)
+{
+	const struct acpi_wdat_entry *entries;
+	int i;
+
+	entries = (struct acpi_wdat_entry *)(wdat + 1);
+	for (i = 0; i < wdat->entries; i++) {
+		const struct acpi_generic_address *gas;
+
+		gas = &entries[i].register_region;
+		if (gas->space_id == ACPI_ADR_SPACE_SYSTEM_IO) {
+			switch (gas->address) {
+			case RTC_PORT(0):
+			case RTC_PORT(1):
+			case RTC_PORT(2):
+			case RTC_PORT(3):
+				return true;
+			}
+		}
+	}
+
+	return false;
+}
+#else
+static bool acpi_watchdog_uses_rtc(const struct acpi_table_wdat *wdat)
+{
+	return false;
+}
+#endif
 
 static const struct acpi_table_wdat *acpi_watchdog_get_wdat(void)
 {
@@ -50,9 +66,6 @@ static const struct acpi_table_wdat *acpi_watchdog_get_wdat(void)
 	if (acpi_disabled)
 		return NULL;
 
-	if (dmi_check_system(acpi_watchdog_skip))
-		return NULL;
-
 	status = acpi_get_table(ACPI_SIG_WDAT, 0,
 				(struct acpi_table_header **)&wdat);
 	if (ACPI_FAILURE(status)) {
@@ -60,6 +73,11 @@ static const struct acpi_table_wdat *acpi_watchdog_get_wdat(void)
 		return NULL;
 	}
 
+	if (acpi_watchdog_uses_rtc(wdat)) {
+		pr_info("Skipping WDAT on this system because it uses RTC SRAM\n");
+		return NULL;
+	}
+
 	return wdat;
 }
 
-- 
cgit 1.2-0.3.lf.el7

rtc_wdat.patch (12,216 bytes)
toracat

toracat

2019-02-15 23:33

manager   ~0033847

The centosplus kernel (kernel-plus) with the patch applied is now available from:

https://people.centos.org/toracat/kernel/7/plus/bug15830/
pgreco

pgreco

2019-02-16 11:25

developer   ~0033850

@toracat just booted with that kernel and it seems to be working as expected
toracat

toracat

2019-03-14 17:59

manager   ~0034002

kernel-plus-3.10.0-957.10.1.el7.centos.plus that is to be released soon has the patch.

Issue History

Date Modified Username Field Change
2019-02-15 15:18 pgreco New Issue
2019-02-15 15:18 pgreco Note Added: 0033841
2019-02-15 15:19 pgreco Status new => assigned
2019-02-15 15:20 pgreco File Added: rtc_wdat.patch
2019-02-15 15:20 pgreco Note Added: 0033842
2019-02-15 23:33 toracat Note Added: 0033847
2019-02-16 11:25 pgreco Note Added: 0033850
2019-03-14 17:59 toracat Note Added: 0034002