Re: [3.4-rc5 -> 3.4-rc6 regression] Asus P5NSLI: lockup on resume from suspend

[Jonathan Nieder <jrnieder@gmail.com>]

Octavio Alvarez wrote:

> The problem is a full system lock-up on resume.
[...]
> I bisected the problem as requested by Alan Stern, and tracked it
> down to the following commit:
>
> commit 2feec47d4c5f80b05f1650f5a24865718978eea4
[...]
>     ACPICA: ACPI 5: Support for new FADT SleepStatus, SleepControl registers

Parsing that commit:

 * new ACPICA-internal functions:
   acpi_hw_execute_SST, acpi_hw_extended_sleep, acpi_hw_legacy_sleep,
   acpi_hw_extended_wake_prep, acpi_hw_extended_wake,
   acpi_hw_legacy_wake_prep, acpi_hw_legacy_wake.

 * Various functions moved to hwxfsleep.c.

 * acpi_enter_sleep_state_s4bios was moved to before
   acpi_enter_sleep_state and acpi_enter_sleep_state_prep for no
   apparent reason.  Unchanged.

 * acpi_enter_sleep_state_prep: factored out execution of _SST
   (which sets System Status) into a helper.  No functional change.

 * acpi_enter_sleep_state: in the !(ACPI 5) case, just checks that
   the sleep types are not out of bounds and calls
   acpi_hw_legacy_sleep().  Functional change:

   - disable bus master arbitration when entering sleep states other
     than S5

   No other functional change.  _GTS ("Going To Sleep" hook) call
   factored out into a helper.

 * acpi_leave_sleep_state_prep: in the !(ACPI 5) case, just calls
   acpi_hw_legacy_wake_prep().  _BFS ("Back From Sleep" method) call
   factored out into a helper.  No functional change.

 * acpi_leave_sleep_state: in the !(ACPI 5) case, just calls
   acpi_hw_legacy_wake().  _SST calls factored out.  _WAK call
   factored out.  Functional change:

   - enable bus master arbitaration

   No other functional change.

So presumably it's the new writes to the ACPI_BITREG_ARB_DISABLE
register that cause trouble.  The patch below tests that guess.

Now for a complaint.  This would have been a lot easier if cleanups
that do not change behavior were split into separate commits --- one
commit per change.  That makes it easy to verify that each patch
correctly does what it promises with no unintended side effects.

Though I understand that hacking ACPICA is hard, and there is probably
a lot to the story of its development I don't know.

Thanks,
Jonathan

diff --git i/drivers/acpi/acpica/hwsleep.c w/drivers/acpi/acpica/hwsleep.c
index 0ed85cac3231..615996a36bed 100644
--- i/drivers/acpi/acpica/hwsleep.c
+++ w/drivers/acpi/acpica/hwsleep.c
@@ -95,18 +95,6 @@ acpi_status acpi_hw_legacy_sleep(u8 sleep_state, u8 flags)
 		return_ACPI_STATUS(status);
 	}
 
-	if (sleep_state != ACPI_STATE_S5) {
-		/*
-		 * Disable BM arbitration. This feature is contained within an
-		 * optional register (PM2 Control), so ignore a BAD_ADDRESS
-		 * exception.
-		 */
-		status = acpi_write_bit_register(ACPI_BITREG_ARB_DISABLE, 1);
-		if (ACPI_FAILURE(status) && (status != AE_BAD_ADDRESS)) {
-			return_ACPI_STATUS(status);
-		}
-	}
-
 	/*
 	 * 1) Disable/Clear all GPEs
 	 * 2) Enable all wakeup GPEs
@@ -364,16 +352,6 @@ acpi_status acpi_hw_legacy_wake(u8 sleep_state, u8 flags)
 				    [ACPI_EVENT_POWER_BUTTON].
 				    status_register_id, ACPI_CLEAR_STATUS);
 
-	/*
-	 * Enable BM arbitration. This feature is contained within an
-	 * optional register (PM2 Control), so ignore a BAD_ADDRESS
-	 * exception.
-	 */
-	status = acpi_write_bit_register(ACPI_BITREG_ARB_DISABLE, 0);
-	if (ACPI_FAILURE(status) && (status != AE_BAD_ADDRESS)) {
-		return_ACPI_STATUS(status);
-	}
-
 	acpi_hw_execute_sleep_method(METHOD_PATHNAME__SST, ACPI_SST_WORKING);
 	return_ACPI_STATUS(status);
 }