Re: [PATCH 7/8] ACPI / PCI: Do not preserve _OSC control bits returned by a query (v2)

[Hidetoshi Seto <seto.hidetoshi@jp.fujitsu.com>]

(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