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

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



On Tuesday, August 03, 2010, Hidetoshi Seto wrote:
> (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.

OK

The assumption here is that the BIOS would only mask the bits it's not
going to grant control of and will do it in a consistent way.  So, I guess the
purpose of the change above would be to protect us from buggy BIOSes.

Thanks,
Rafael
_______________________________________________
linux-pm mailing list
linux-pm@xxxxxxxxxxxxxxxxxxxxxxxxxx
https://lists.linux-foundation.org/mailman/listinfo/linux-pm


[Index of Archives]     [Linux ACPI]     [Netdev]     [Ethernet Bridging]     [Linux Wireless]     [CPU Freq]     [Kernel Newbies]     [Fedora Kernel]     [Security]     [Linux for Hams]     [Netfilter]     [Bugtraq]     [Yosemite News]     [MIPS Linux]     [ARM Linux]     [Linux RAID]     [Linux Admin]     [Samba]

  Powered by Linux