(2010/08/03 13:52), Kenji Kaneshige wrote:
> (2010/08/03 6:59), Rafael J. Wysocki wrote:
>> @@ -434,19 +432,6 @@ acpi_status acpi_pci_osc_control_set(acp
>> if ((root->osc_control_set& control_req) == control_req)
>> goto out;
>>
>> - /* Need to query controls first before requesting them */
>> - if (!root->osc_queried) {
>> - status = acpi_pci_query_osc(root, root->osc_support_set, NULL);
>> - if (ACPI_FAILURE(status))
>> - goto out;
>> - }
>> - if ((root->osc_control_qry& control_req) != control_req) {
>> - printk(KERN_DEBUG
>> - "Firmware did not grant requested _OSC control\n");
>> - status = AE_SUPPORT;
>> - goto out;
>> - }
>
> I think acpi_pci_osc_control_set() still need to query before commit
> to ensure all the requested controls are granted to OS.
>
> So the code needs to be
>
> status = acpi_pci_query_osc(root, root->osc_support_set, &control_req);
> if (ACPI_FAILURE(status))
> goto out;
Hum, since acpi_status acpi_pci_osc_control_set() is an exported
function, we cannot be too careful here.
OTOH, I think this second query should be done in caller too,
i.e. pcie_port_acpi_setup() in patch [4/8].
Now:
pcie_port_acpi_setup()
{
flags = A|B|C|D;
acpi_pci_osc_control_query(handle, &flags);
/* note: flags might be changed after query */
acpi_pci_osc_control_set(handle, flags);
}
Strictly, it could be like:
New:
pcie_port_acpi_setup()
{
flags = A|B|C|D;
do {
pre_flags = flags;
acpi_pci_osc_control_query(handle, &flags);
} while (flags && pre_flags != flags);
if (flags)
acpi_pci_osc_control_set(handle, flags);
}
IMHO these checks are kind of preventive guard for corner cases,
and I suppose it can be implemented by an incremental patch later.
Thanks,
H.Seto
--
To unsubscribe from this list: send the line "unsubscribe linux-pci" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
