(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(-)
diff --git a/drivers/acpi/pci_root.c b/drivers/acpi/pci_root.c
index 061e6f4..9a288df 100644
--- a/drivers/acpi/pci_root.c
+++ b/drivers/acpi/pci_root.c
@@ -372,38 +372,6 @@ out:
}
EXPORT_SYMBOL_GPL(acpi_get_pci_dev);
- /**
- * acpi_pci_osc_control_query - Get the _OSC bits the kernel can control.
- * @handle: ACPI handle of a PCI root bridge (or PCIe Root Complex).
- * @mask: Mask of _OSC bits to query and the place to put the result into.
- **/
-acpi_status acpi_pci_osc_control_query(acpi_handle handle, u32 *mask)
-{
- struct acpi_pci_root *root;
- acpi_handle tmp;
- acpi_status status = AE_OK;
-
- if (!mask || !(*mask & OSC_PCI_CONTROL_MASKS))
- return AE_BAD_PARAMETER;
-
- root = acpi_pci_find_root(handle);
- if (!root)
- return AE_NOT_EXIST;
-
- status = acpi_get_handle(handle, "_OSC", &tmp);
- if (ACPI_FAILURE(status))
- return status;
-
- mutex_lock(&osc_lock);
- if ((*mask & root->osc_control_set) == *mask)
- *mask = root->osc_control_set;
- else
- status = acpi_pci_query_osc(root, root->osc_support_set, mask);
- mutex_unlock(&osc_lock);
-
- return status;
-}
-
/**
* 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.
**/
-acpi_status acpi_pci_osc_control_set(acpi_handle handle, u32 flags)
+acpi_status acpi_pci_osc_control_set(acpi_handle handle, u32 *flags)
{
acpi_status status;
u32 control_req, result, capbuf[3];
acpi_handle tmp;
struct acpi_pci_root *root;
- control_req = (flags & OSC_PCI_CONTROL_MASKS);
+ if (!flags)
+ return AE_BAD_PARAMETER;
+
+ control_req = (*flags & OSC_PCI_CONTROL_MASKS);
if (!control_req)
return AE_TYPE;
@@ -432,8 +403,10 @@ acpi_status acpi_pci_osc_control_set(acpi_handle handle, u32 flags)
mutex_lock(&osc_lock);
/* No need to evaluate _OSC if the control was already granted. */
- if ((root->osc_control_set & control_req) == control_req)
+ if ((root->osc_control_set & control_req) == control_req) {
+ *flags = root->osc_control_set;
goto out;
+ }
/* Need to query controls first before requesting them */
result = control_req;
@@ -441,8 +414,10 @@ acpi_status acpi_pci_osc_control_set(acpi_handle handle, u32 flags)
if (ACPI_FAILURE(status))
goto out;
if ((result & control_req) != control_req) {
- printk(KERN_DEBUG
- "Firmware did not grant requested _OSC control\n");
+ printk(KERN_DEBUG PREFIX
+ "Firmware did not grant requested _OSC control: %x/%x\n",
+ control_req, result);
+ *flags = result;
status = AE_SUPPORT;
goto out;
}
@@ -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;
+ else
+ printk(KERN_WARNING FW_BUG PREFIX
+ "_OSC did not grant controls that passed query\n");
out:
mutex_unlock(&osc_lock);
return status;
diff --git a/drivers/pci/hotplug/acpi_pcihp.c b/drivers/pci/hotplug/acpi_pcihp.c
index 12ea415..675181d 100644
--- a/drivers/pci/hotplug/acpi_pcihp.c
+++ b/drivers/pci/hotplug/acpi_pcihp.c
@@ -358,7 +358,7 @@ int acpi_get_hp_hw_control_from_firmware(struct pci_dev *pdev, u32 flags)
acpi_get_name(handle, ACPI_FULL_PATHNAME, &string);
dbg("Trying to get hotplug control for %s\n",
(char *)string.pointer);
- status = acpi_pci_osc_control_set(handle, flags);
+ status = acpi_pci_osc_control_set(handle, &flags);
if (ACPI_SUCCESS(status))
goto got_one;
if (status == AE_SUPPORT)
diff --git a/drivers/pci/pcie/portdrv_acpi.c b/drivers/pci/pcie/portdrv_acpi.c
index a07a70e..764d0b3 100644
--- a/drivers/pci/pcie/portdrv_acpi.c
+++ b/drivers/pci/pcie/portdrv_acpi.c
@@ -55,20 +55,17 @@ int pcie_port_acpi_setup(struct pci_dev *port, int *srv_mask)
flags |= OSC_PCI_EXPRESS_AER_CONTROL;
}
- status = acpi_pci_osc_control_query(handle, &flags);
- if (ACPI_FAILURE(status)) {
- dev_dbg(&port->dev, "ACPI _OSC query failed (code %d)\n",
- status);
- return -ENODEV;
+retry:
+ status = acpi_pci_osc_control_set(handle, &flags);
+ if (status == AE_SUPPORT) {
+ if (!(flags & OSC_PCI_EXPRESS_CAP_STRUCTURE_CONTROL)) {
+ dev_dbg(&port->dev, "BIOS refuses to grant control of "
+ "PCIe Capability Structure\n");
+ return -EACCES;
+ }
+ if (flags & ~OSC_PCI_EXPRESS_CAP_STRUCTURE_CONTROL)
+ goto retry;
}
-
- if (!(flags & OSC_PCI_EXPRESS_CAP_STRUCTURE_CONTROL)) {
- dev_dbg(&port->dev, "BIOS refuses to grant control of PCIe "
- "Capability Structure\n");
- return -EACCES;
- }
-
- status = acpi_pci_osc_control_set(handle, flags);
if (ACPI_FAILURE(status)) {
dev_dbg(&port->dev, "ACPI _OSC request failed (code %d)\n",
status);
diff --git a/include/linux/acpi.h b/include/linux/acpi.h
index a9afe9c..1b5aac6 100644
--- a/include/linux/acpi.h
+++ b/include/linux/acpi.h
@@ -305,8 +305,7 @@ acpi_status acpi_run_osc(acpi_handle handle, struct acpi_osc_context *context);
OSC_PCI_EXPRESS_AER_CONTROL | \
OSC_PCI_EXPRESS_CAP_STRUCTURE_CONTROL)
-extern acpi_status acpi_pci_osc_control_query(acpi_handle handle, u32 *mask);
-extern acpi_status acpi_pci_osc_control_set(acpi_handle handle, u32 flags);
+extern acpi_status acpi_pci_osc_control_set(acpi_handle handle, u32 *flags);
extern void acpi_early_init(void);
#else /* !CONFIG_ACPI */
--
1.7.2
--
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
