> I've studied the ACPI spec trying to understand better, but I'm > struggling with the question: > What is the maximum number (lowest power) permitted device power state > for a device that is configured as able to wake the system from S3, > **that does not implement the _S3W method**? Actually the ACPI spec has an answer for the case when _S3D is present. The lack of clarity is only over the situation when both _S3D and _S3W are missing - like on the platforms being worked on here. The _S3D docs say: > If the device can wake the system from the S3 system sleeping state (see > _PRW) then the device must support wake in the D-state returned by this > object. However, OSPM cannot assume wake from the S3 system sleeping state > is supported in any deeper D-state unless specified by a corresponding > _S3W object Looking at the design of the existing Linux code, it seems like this "max = min" assignment that is causing us trouble originates directly from an attempt to implement that logic: if we didn't get a response from _S3W, then we must clamp ourselves to the data we got from _S3D. If I modify the Linux code to be a little more specific in that logic (only applying when we actually got something from _S3D) then the problematic behaviour is avoided and USB wakeups work. I feel that this change makes the Linux implementation more directly mirror the wording in the ACPI spec and it's associated lack of clarity for when both methods are missing. Thoughts? --- drivers/acpi/device_pm.c | 11 ++++++++--- 1 file changed, 8 insertions(+), 3 deletions(-) diff --git a/drivers/acpi/device_pm.c b/drivers/acpi/device_pm.c index a4c8ad98560d..44f12c5c75ee 100644 --- a/drivers/acpi/device_pm.c +++ b/drivers/acpi/device_pm.c @@ -543,6 +543,7 @@ static int acpi_dev_pm_get_state(struct device *dev, struct acpi_device *adev, unsigned long long ret; int d_min, d_max; bool wakeup = false; + acpi_status sxd_status; acpi_status status; /* @@ -565,8 +566,8 @@ static int acpi_dev_pm_get_state(struct device *dev, struct acpi_device *adev, * provided if AE_NOT_FOUND is returned. */ ret = d_min; - status = acpi_evaluate_integer(handle, method, NULL, &ret); - if ((ACPI_FAILURE(status) && status != AE_NOT_FOUND) + sxd_status = acpi_evaluate_integer(handle, method, NULL, &ret); + if ((ACPI_FAILURE(sxd_status) && sxd_status != AE_NOT_FOUND) || ret > ACPI_STATE_D3_COLD) return -ENODATA; @@ -599,7 +600,11 @@ static int acpi_dev_pm_get_state(struct device *dev, struct acpi_device *adev, method[3] = 'W'; status = acpi_evaluate_integer(handle, method, NULL, &ret); if (status == AE_NOT_FOUND) { - if (target_state > ACPI_STATE_S0) + /* No _SxW. In this case, the ACPI spec says that we + * must not go into any power state deeper than the + * value returned from _SxD. + */ + if (sxd_status == AE_OK && target_state > ACPI_STATE_S0) d_max = d_min; } else if (ACPI_SUCCESS(status) && ret <= ACPI_STATE_D3_COLD) { /* Fall back to D3cold if ret is not a valid state. */ -- 2.14.1 -- 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