On Sun, 08 Jul 2012 02:04:32 -0700, Jonathan Nieder <jrnieder@xxxxxxxxx>
wrote:
So presumably it's the new writes to the ACPI_BITREG_ARB_DISABLE
register that cause trouble. The patch below tests that guess.
Given the complexity of the ACPI_DEBUG logging (particularly the
console part), I decided to give your patch a shot directly.
Result: the system no longer locks up. I paste the following info
so you can make sure I'm working with the correct commit:
[Sun Jul 08 02:41:37 -0700 -- alvarezp@octavio:~/src/linux]
$ git status
# Not currently on any branch.
# Changes not staged for commit:
# (use "git add <file>..." to update what will be committed)
# (use "git checkout -- <file>..." to discard changes in working
directory)
#
# modified: drivers/acpi/acpica/hwsleep.c
#
# Untracked files:
# (use "git add <file>..." to include in what will be committed)
#
# arch/x86/tools/relocs
no changes added to commit (use "git add" and/or "git commit -a")
[Sun Jul 08 02:41:41 -0700 -- alvarezp@octavio:~/src/linux]
$ git log HEAD^..HEAD
commit 2feec47d4c5f80b05f1650f5a24865718978eea4
Author: Bob Moore <robert.moore@xxxxxxxxx>
Date: Tue Feb 14 15:00:53 2012 +0800
ACPICA: ACPI 5: Support for new FADT SleepStatus, SleepControl
registers
Adds sleep and wake support for systems with these registers.
One new file, hwxfsleep.c
Signed-off-by: Bob Moore <robert.moore@xxxxxxxxx>
Signed-off-by: Len Brown <len.brown@xxxxxxxxx>
[Sun Jul 08 02:41:49 -0700 -- alvarezp@octavio:~/src/linux]
$ git diff
diff --git a/drivers/acpi/acpica/hwsleep.c b/drivers/acpi/acpica/hwsleep.c
index 59a2a6b..66dd2b8 100644
--- a/drivers/acpi/acpica/hwsleep.c
+++ b/drivers/acpi/acpica/hwsleep.c
@@ -241,18 +241,6 @@ acpi_status acpi_hw_legacy_sleep(u8 sleep_state)
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
@@ -498,16 +486,6 @@ acpi_status acpi_hw_legacy_wake(u8 sleep_state)
[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_SST(ACPI_SST_WORKING);
return_ACPI_STATUS(status);
}
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.
I have to back you up on this one. This patch could have been 3 easily.
Git bisect would have done a better job.
The question is: if he had dont it that way, would the kernel have
compiled in intermediate commits?
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);
}
Thanks for the patch.
--
Octavio.
--
To unsubscribe from this list: send the line "unsubscribe linux-acpi" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
