On Saturday, May 26, 2012, Oleksij Rempel wrote: > Am 26.05.2012 08:03, schrieb Oleksij Rempel (fishor): > > Hi, > > > > On 25.05.2012 22:00, Rafael J. Wysocki wrote: > >> On Friday, May 25, 2012, Alan Stern wrote: > >>> Oleksij: > >>> > >>> Please take a look at this bug report: > >>> > >>> https://bugzilla.kernel.org/show_bug.cgi?id=43278 > >>> > >>> Apparently your patch breaks wakeup on this machine by preventing the > >>> USB host controllers from being put into D3. > >> > >> I think the patch is incorrect, actually. > >> > >> First, if you look at the first hunk: > >> > >> - if (acpi_target_sleep_state> ACPI_STATE_S0) > >> + if (acpi_target_sleep_state> ACPI_STATE_S0) { > >> + acpi_status status; > >> + > >> acpi_evaluate_integer(handle, acpi_method, NULL,&d_min); > >> > >> + if (device_may_wakeup(dev)) { > >> + acpi_method[3] = 'W'; > >> + status = acpi_evaluate_integer(handle, > >> acpi_method, > >> + NULL,&d_max); > >> + if (ACPI_FAILURE(status)) > >> + d_max = d_min; > >> + } > >> + } > >> > >> it will do something like this: if the device is wakeup-capable, get > >> d_max > >> from _SxW, unless it fails. However, the code just below in that > >> function: > >> > >> if (acpi_target_sleep_state == ACPI_STATE_S0 || > >> (device_may_wakeup(dev)&& > >> adev->wakeup.sleep_state<= acpi_target_sleep_state)) { > >> acpi_status status; > >> > >> acpi_method[3] = 'W'; > >> status = acpi_evaluate_integer(handle, acpi_method, NULL, > >> &d_max); > >> if (ACPI_FAILURE(status)) { > >> if (acpi_target_sleep_state != ACPI_STATE_S0 || > >> status != AE_NOT_FOUND) > >> d_max = d_min; > >> } else if (d_max< d_min) { > >> /* Warn the user of the broken DSDT */ > >> printk(KERN_WARNING "ACPI: Wrong value from %s\n", > >> acpi_method); > >> /* Sanitize it */ > >> d_min = d_max; > >> } > >> } > >> > >> does _exactly_ the same thing (it only has a more sophisticated error > >> code path). > > > > Not really correct. The code below check _SxW state on wake up. This > > code checks _SxW on suspend. > > oops i misreaded the code. it really do the same. except this part is > not executed because of this check: "adev->wakeup.sleep_state <= > acpi_target_sleep_state" Precisely. Thanks, Rafael -- 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
