(2010/08/04 21:15), Rafael J. Wysocki wrote: > On Wednesday, August 04, 2010, Kenji Kaneshige wrote: >> (2010/08/04 17:43), Hidetoshi Seto wrote: >>> /** >>> * acpi_pci_osc_control_set - commit requested control to Firmware >>> * @handle: acpi_handle for the target ACPI object >>> @@ -411,14 +379,17 @@ acpi_status acpi_pci_osc_control_query(acpi_handle handle, u32 *mask) >>> * >>> * Attempt to take control from Firmware on requested control bits. >>> **/ >> >> Updating description of this function would be appreciated. OK, I will do it. >>> @@ -452,7 +427,10 @@ acpi_status acpi_pci_osc_control_set(acpi_handle handle, u32 flags) >>> capbuf[OSC_CONTROL_TYPE] = root->osc_control_set | control_req; >>> status = acpi_pci_run_osc(handle, capbuf,&result); >>> if (ACPI_SUCCESS(status)) >>> - root->osc_control_set = result; >>> + root->osc_control_set = *flags = result; >> >> I don't think we need to update *flags here, though it depends on the design >> of this function interface. >> IMHO, updating *flags only when AE_SUPPORT is returned is easy to understand. > > I think the patch is correct. We should update *flags in any case so that the > caller can check what's the full mask of granted controls even if it asked for > fewer control bits. My idea (that will be what I'll add to the description of function) is: On success, OS takes all of requested control(s) and *flags is updated with set of controls now OS have. If AE_SUPPORT returns, none of control settings are changed since Firmware rejects granting some of requested controls. *flags is updated with set of controls with rejected bits cleared. All other cases mean failure, none of control settings are changed, and *flags is unchanged. Thanks, H.Seto -- 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
