(2010/08/04 17:43), Hidetoshi Seto wrote:
> (2010/08/04 14:46), Kenji Kaneshige wrote:
>> By the way, I think it is getting confusing regarding who query the
>> controls. IMO, querying controls to ensure all the requested controls
>> are granted to OS should be done in acpi_pci_osc_control_set(), as
>> I said above. On the other hand, PCIe port driver need to query
>> controls for other reason... Now I think it might be better to change
>> acpi_pci_osc_control_set() like below instead of introducing
>> acpi_pci_osc_control_query().
>>
>> acpi_status acpi_pci_osc_control_set(acpi_handle handle, u32 *flags)
>> {
>> ...
>> query = control_req;
>> status = acpi_pci_query_osc(root, root->osc_support_set,&query);
>> if (ACPI_FAILURE(status))
>> goto out;
>> if ((query& control_req) != control_req) {
>> printk_(KERN_DEBUG
>> "Firmware did not grant requested _OSC control\n");
>> status = AE_SUPPORT;
>> *flags = (query& control_req);
>> goto out;
>> }
>> ...
>> }
>>
>> And do as follows in pcie_port_acpi_setup()
>>
>> status = acpi_pci_osc_control_set(handle,&flags);
>> if (status == AE_SUPPORT) {
>> /* 2nd try */
>> status = acpi_pci_osc_control_set(handle,&flags);
>> }
>> if (ACPI_FAILURE(status)) {
>> ...
>>
>> What do you think about this?
>
> I think it makes sense, though some minor cares are required.
>
> Does this incremental patch (apply after [8/8]) looks good?
> If it is OK, I'll test these 8+1 patches within the next 2days.
>
> Thanks,
> H.Seto
>
> =====
> Subject: ACPI/PCI: Unify acpi_pci_osc_control_*()
>
> Now AE_SUPPORT of acpi_pci_osc_control_set() tells not only
> that query fails with the requested control bits but also that
> the result of query is stored into the pointed place.
>
> This allow user to retry acpi_pci_osc_control_set() with the
> result of query.
>
> Signed-off-by: Hidetoshi Seto<seto.hidetoshi@xxxxxxxxxxxxxx>
> ---
> drivers/acpi/pci_root.c | 54 +++++++++++--------------------------
> drivers/pci/hotplug/acpi_pcihp.c | 2 +-
> drivers/pci/pcie/portdrv_acpi.c | 23 +++++++---------
> include/linux/acpi.h | 3 +-
> 4 files changed, 28 insertions(+), 54 deletions(-)
<snip.>
> /**
> * 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.
<snip.>
> @@ -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.
Others looks good to me.
Thanks,
Kenji Kaneshige
--
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
