Skip to contentSkip to conversation

ACPI / watchdog: Fix init failure with overlapping register regions

PATCH In review Ryan Kennedy 3 participants 5 messages Jul 15, 2017 – Jul 26, 2017 ACPI

lore
Share & tools
lore

Ryan Kennedy

author
#0 lore raw

PATCH 1/1 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 <[email protected]>
---
 drivers/acpi/acpi_watchdog.c | 7 ++++++-
 1 file changed, 6 insertions(+), 1 deletion(-)
drivers/acpi/acpi_watchdog.c +6 -1
index 8c4e0a1..bf22c29 100644
@@ -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;
 			}
Signature · 1 line
2.9.4
#1 lore raw
Quoted 19 lines · Ryan Kennedy

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 <[email protected]>
---
 drivers/acpi/acpi_watchdog.c | 7 ++++++-
 1 file changed, 6 insertions(+), 1 deletion(-)
drivers/acpi/acpi_watchdog.c +0 -0
index 8c4e0a1..bf22c29 100644
@@ -86,7 +86,12 @@ void __init acpi_watchdog_init(void)
 
 		found = false;
 		resource_list_for_each_entry(rentry, &resource_list) {

drivers/acpi/acpi_watchdog.c void __init acpi_watchdog_init(void) · line 97

Quoted 10 lines
drivers/acpi/acpi_watchdog.c +6 -1
-			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;
 			}

Mika, can you have a look at this, please?

#2 lore raw
Quoted 17 lines · Rafael J. Wysocki
Quoted 16 lines · Ryan Kennedy

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 <[email protected]>
---
 drivers/acpi/acpi_watchdog.c | 7 ++++++-
 1 file changed, 6 insertions(+), 1 deletion(-)
drivers/acpi/acpi_watchdog.c +0 -0
index 8c4e0a1..bf22c29 100644
@@ -86,7 +86,12 @@ void __init acpi_watchdog_init(void)
Quoted 15 lines

drivers/acpi/acpi_watchdog.c void __init acpi_watchdog_init(void) · line 97

Quoted 13 lines
drivers/acpi/acpi_watchdog.c +6 -1
 		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;
 			}

Mika, can you have a look at this, please?

I wonder why we need to do this?

+ if (res.start < rentry->res->start)
+ rentry->res->start = res.start;
+ if (res.end > rentry->res->end)
+ rentry->res->end = res.end;

IIRC we don't use rentry for anything else when we go out of the loop. Tuning those limits seems unnecessary.

Acked-by
#3 lore raw
Quoted 9 lines · Mika Westerberg

I wonder why we need to do this?

drivers/acpi/acpi_watchdog.c +4 -0
+                           if (res.start < rentry->res->start)
+                                   rentry->res->start = res.start;
+                           if (res.end > rentry->res->end)
+                                   rentry->res->end = res.end;

IIRC we don't use rentry for anything else when we go out of the loop. Tuning those limits seems unnecessary.

OK, Ryan pointed out that the later code then uses the adjusted rentry so this is indeed needed.

Acked-by Mika Westerberg <[email protected]>
#4 lore raw
Quoted 8 lines · Mika Westerberg
Quoted 7 lines · Mika Westerberg

I wonder why we need to do this?

drivers/acpi/acpi_watchdog.c +4 -0
+                           if (res.start < rentry->res->start)
+                                   rentry->res->start = res.start;
+                           if (res.end > rentry->res->end)
+                                   rentry->res->end = res.end;
Quoted 7 lines
Quoted 2 lines

IIRC we don't use rentry for anything else when we go out of the loop. Tuning those limits seems unnecessary.

OK, Ryan pointed out that the later code then uses the adjusted rentry so this is indeed needed.

Acked-by Mika Westerberg <[email protected]>

OK, patch applied, thanks!