* [PATCH 0/3] ACPI/PCI Hotplug: acpiphp cleanup
@ 2009-07-10 19:42 Alex Chiang
2009-07-10 19:42 ` [PATCH 1/3] ACPI: export acpi_pci_root and friends Alex Chiang
` (3 more replies)
0 siblings, 4 replies; 8+ messages in thread
From: Alex Chiang @ 2009-07-10 19:42 UTC (permalink / raw)
To: jbarnes, lenb; +Cc: linux-pci, linux-kernel, linux-acpi
Hi Len, Jesse,
I'd like to have this patch series considered for the current release
because it clarifies some usage of the new interface acpi_get_pci_dev()
that was also introduced in this release cycle.
The short story is, acpi_get_pci_dev() doesn't always return a struct
pci_dev, because a PCI root bridge may not have an associated pci_dev on
platforms with non-materialized root bridges.
See this patch for more:
http://patchwork.kernel.org/patch/32613/
I had to export acpi_pci_root to accomplish my goal, but it did result
in a nice net cleanup in acpiphp.
It should probably go through Jesse's tree, but needs Len's ACK for
the acpi_pci_root bit.
Thanks.
/ac
---
Alex Chiang (3):
ACPI: export acpi_pci_root and friends
PCI Hotplug: acpiphp: find bridges the easy way
PCI Hotplug: convert acpi_pci_detect_ejectable() to take an acpi_handle
drivers/acpi/pci_root.c | 17 +------
drivers/pci/hotplug/acpi_pcihp.c | 9 +---
drivers/pci/hotplug/acpiphp_glue.c | 92 +++++++++---------------------------
drivers/pci/hotplug/pciehp_acpi.c | 5 +-
include/acpi/acpi_bus.h | 14 +++++
include/linux/pci_hotplug.h | 2 -
6 files changed, 47 insertions(+), 92 deletions(-)
^ permalink raw reply [flat|nested] 8+ messages in thread
* [PATCH 1/3] ACPI: export acpi_pci_root and friends
2009-07-10 19:42 [PATCH 0/3] ACPI/PCI Hotplug: acpiphp cleanup Alex Chiang
@ 2009-07-10 19:42 ` Alex Chiang
2009-07-10 19:42 ` [PATCH 2/3] PCI Hotplug: acpiphp: find bridges the easy way Alex Chiang
` (2 subsequent siblings)
3 siblings, 0 replies; 8+ messages in thread
From: Alex Chiang @ 2009-07-10 19:42 UTC (permalink / raw)
To: jbarnes, lenb; +Cc: linux-pci, Bjorn Helgaas, linux-kernel, linux-acpi
We can simplify ACPI drivers if we can tell whether a handle is an
ACPI PCI root or not.
Cc: Bjorn Helgaas <bjorn.helgaas@hp.com>
Signed-off-by: Alex Chiang <achiang@hp.com>
---
drivers/acpi/pci_root.c | 17 ++---------------
include/acpi/acpi_bus.h | 14 ++++++++++++++
2 files changed, 16 insertions(+), 15 deletions(-)
diff --git a/drivers/acpi/pci_root.c b/drivers/acpi/pci_root.c
index 55b5b90..31b961c 100644
--- a/drivers/acpi/pci_root.c
+++ b/drivers/acpi/pci_root.c
@@ -61,20 +61,6 @@ static struct acpi_driver acpi_pci_root_driver = {
},
};
-struct acpi_pci_root {
- struct list_head node;
- struct acpi_device *device;
- struct pci_bus *bus;
- u16 segment;
- u8 bus_nr;
-
- u32 osc_support_set; /* _OSC state of support bits */
- u32 osc_control_set; /* _OSC state of control bits */
- u32 osc_control_qry; /* the latest _OSC query result */
-
- u32 osc_queried:1; /* has _OSC control been queried? */
-};
-
static LIST_HEAD(acpi_pci_roots);
static struct acpi_pci_driver *sub_driver;
@@ -317,7 +303,7 @@ static acpi_status acpi_pci_osc_support(struct acpi_pci_root *root, u32 flags)
return status;
}
-static struct acpi_pci_root *acpi_pci_find_root(acpi_handle handle)
+struct acpi_pci_root *acpi_pci_find_root(acpi_handle handle)
{
struct acpi_pci_root *root;
@@ -327,6 +313,7 @@ static struct acpi_pci_root *acpi_pci_find_root(acpi_handle handle)
}
return NULL;
}
+EXPORT_SYMBOL_GPL(acpi_pci_find_root);
struct acpi_handle_node {
struct list_head node;
diff --git a/include/acpi/acpi_bus.h b/include/acpi/acpi_bus.h
index c65e4ce..21f7152 100644
--- a/include/acpi/acpi_bus.h
+++ b/include/acpi/acpi_bus.h
@@ -369,10 +369,24 @@ int register_acpi_bus_type(struct acpi_bus_type *);
int unregister_acpi_bus_type(struct acpi_bus_type *);
struct device *acpi_get_physical_device(acpi_handle);
+struct acpi_pci_root {
+ struct list_head node;
+ struct acpi_device * device;
+ struct acpi_pci_id id;
+ struct pci_bus *bus;
+
+ u32 osc_support_set; /* _OSC state of support bits */
+ u32 osc_control_set; /* _OSC state of control bits */
+ u32 osc_control_qry; /* the latest _OSC query result */
+
+ u32 osc_queried:1; /* has _OSC control been queried? */
+};
+
/* helper */
acpi_handle acpi_get_child(acpi_handle, acpi_integer);
int acpi_is_root_bridge(acpi_handle);
acpi_handle acpi_get_pci_rootbridge_handle(unsigned int, unsigned int);
+struct acpi_pci_root *acpi_pci_find_root(acpi_handle handle);
#define DEVICE_ACPI_HANDLE(dev) ((acpi_handle)((dev)->archdata.acpi_handle))
#ifdef CONFIG_PM_SLEEP
^ permalink raw reply related [flat|nested] 8+ messages in thread
* [PATCH 2/3] PCI Hotplug: acpiphp: find bridges the easy way
2009-07-10 19:42 [PATCH 0/3] ACPI/PCI Hotplug: acpiphp cleanup Alex Chiang
2009-07-10 19:42 ` [PATCH 1/3] ACPI: export acpi_pci_root and friends Alex Chiang
@ 2009-07-10 19:42 ` Alex Chiang
2009-07-10 19:42 ` [PATCH 3/3] PCI Hotplug: convert acpi_pci_detect_ejectable() to take an acpi_handle Alex Chiang
2009-07-14 19:27 ` [PATCH 0/3] ACPI/PCI Hotplug: acpiphp cleanup Jesse Barnes
3 siblings, 0 replies; 8+ messages in thread
From: Alex Chiang @ 2009-07-10 19:42 UTC (permalink / raw)
To: jbarnes, lenb; +Cc: linux-pci, linux-kernel, linux-acpi
Instead of constantly evaluating _ADR and _SEG over and over again,
let's simplify our lives by using:
acpi_pci_find_root() for root bridges
acpi_get_pci_dev() for p2p bridges
This change eliminates some copy 'n paste code and also allows us
to simplify some internal interfaces.
Signed-off-by: Alex Chiang <achiang@hp.com>
---
drivers/pci/hotplug/acpiphp_glue.c | 108 ++++++++++++------------------------
1 files changed, 37 insertions(+), 71 deletions(-)
diff --git a/drivers/pci/hotplug/acpiphp_glue.c b/drivers/pci/hotplug/acpiphp_glue.c
index 0cb0f83..de4afc6 100644
--- a/drivers/pci/hotplug/acpiphp_glue.c
+++ b/drivers/pci/hotplug/acpiphp_glue.c
@@ -62,6 +62,21 @@ static void acpiphp_sanitize_bus(struct pci_bus *bus);
static void acpiphp_set_hpp_values(acpi_handle handle, struct pci_bus *bus);
static void handle_hotplug_event_func(acpi_handle handle, u32 type, void *context);
+static struct pci_bus *pci_bus_from_handle(acpi_handle handle)
+{
+ struct pci_bus *pbus;
+
+ if (acpi_is_root_bridge(handle)) {
+ struct acpi_pci_root *root = acpi_pci_find_root(handle);
+ pbus = root->bus;
+ } else {
+ struct pci_dev *pdev = acpi_get_pci_dev(handle);
+ pbus = pdev->subordinate;
+ pci_dev_put(pdev);
+ }
+ return pbus;
+}
+
/* callback routine to check for the existence of a pci dock device */
static acpi_status
is_pci_dock_device(acpi_handle handle, u32 lvl, void *context, void **rv)
@@ -261,14 +276,15 @@ register_slot(acpi_handle handle, u32 lvl, void *context, void **rv)
/* see if it's worth looking at this bridge */
-static int detect_ejectable_slots(struct pci_bus *pbus)
+static int detect_ejectable_slots(acpi_handle handle)
{
- int found = acpi_pci_detect_ejectable(pbus);
+ int found;
+ struct pci_bus *pbus;
+
+ pbus = pci_bus_from_handle(handle);
+ found = acpi_pci_detect_ejectable(pbus);
if (!found) {
- acpi_handle bridge_handle = acpi_pci_get_bridge_handle(pbus);
- if (!bridge_handle)
- return 0;
- acpi_walk_namespace(ACPI_TYPE_DEVICE, bridge_handle, (u32)1,
+ acpi_walk_namespace(ACPI_TYPE_DEVICE, handle, (u32)1,
is_pci_dock_device, (void *)&found, NULL);
}
return found;
@@ -399,9 +415,10 @@ static inline void config_p2p_bridge_flags(struct acpiphp_bridge *bridge)
/* allocate and initialize host bridge data structure */
-static void add_host_bridge(acpi_handle *handle, struct pci_bus *pci_bus)
+static void add_host_bridge(acpi_handle *handle)
{
struct acpiphp_bridge *bridge;
+ struct acpi_pci_root *root = acpi_pci_find_root(handle);
bridge = kzalloc(sizeof(struct acpiphp_bridge), GFP_KERNEL);
if (bridge == NULL)
@@ -410,7 +427,7 @@ static void add_host_bridge(acpi_handle *handle, struct pci_bus *pci_bus)
bridge->type = BRIDGE_TYPE_HOST;
bridge->handle = handle;
- bridge->pci_bus = pci_bus;
+ bridge->pci_bus = root->bus;
spin_lock_init(&bridge->res_lock);
@@ -419,7 +436,7 @@ static void add_host_bridge(acpi_handle *handle, struct pci_bus *pci_bus)
/* allocate and initialize PCI-to-PCI bridge data structure */
-static void add_p2p_bridge(acpi_handle *handle, struct pci_dev *pci_dev)
+static void add_p2p_bridge(acpi_handle *handle)
{
struct acpiphp_bridge *bridge;
@@ -433,8 +450,8 @@ static void add_p2p_bridge(acpi_handle *handle, struct pci_dev *pci_dev)
bridge->handle = handle;
config_p2p_bridge_flags(bridge);
- bridge->pci_dev = pci_dev_get(pci_dev);
- bridge->pci_bus = pci_dev->subordinate;
+ bridge->pci_dev = acpi_get_pci_dev(handle);
+ bridge->pci_bus = bridge->pci_dev->subordinate;
if (!bridge->pci_bus) {
err("This is not a PCI-to-PCI bridge!\n");
goto err;
@@ -451,7 +468,7 @@ static void add_p2p_bridge(acpi_handle *handle, struct pci_dev *pci_dev)
init_bridge_misc(bridge);
return;
err:
- pci_dev_put(pci_dev);
+ pci_dev_put(bridge->pci_dev);
kfree(bridge);
return;
}
@@ -462,39 +479,21 @@ static acpi_status
find_p2p_bridge(acpi_handle handle, u32 lvl, void *context, void **rv)
{
acpi_status status;
- acpi_handle dummy_handle;
- unsigned long long tmp;
- int device, function;
struct pci_dev *dev;
- struct pci_bus *pci_bus = context;
-
- status = acpi_get_handle(handle, "_ADR", &dummy_handle);
- if (ACPI_FAILURE(status))
- return AE_OK; /* continue */
-
- status = acpi_evaluate_integer(handle, "_ADR", NULL, &tmp);
- if (ACPI_FAILURE(status)) {
- dbg("%s: _ADR evaluation failure\n", __func__);
- return AE_OK;
- }
-
- device = (tmp >> 16) & 0xffff;
- function = tmp & 0xffff;
-
- dev = pci_get_slot(pci_bus, PCI_DEVFN(device, function));
+ dev = acpi_get_pci_dev(handle);
if (!dev || !dev->subordinate)
goto out;
/* check if this bridge has ejectable slots */
- if ((detect_ejectable_slots(dev->subordinate) > 0)) {
+ if ((detect_ejectable_slots(handle) > 0)) {
dbg("found PCI-to-PCI bridge at PCI %s\n", pci_name(dev));
- add_p2p_bridge(handle, dev);
+ add_p2p_bridge(handle);
}
/* search P2P bridges under this p2p bridge */
status = acpi_walk_namespace(ACPI_TYPE_DEVICE, handle, (u32)1,
- find_p2p_bridge, dev->subordinate, NULL);
+ find_p2p_bridge, NULL, NULL);
if (ACPI_FAILURE(status))
warn("find_p2p_bridge failed (error code = 0x%x)\n", status);
@@ -509,9 +508,7 @@ static int add_bridge(acpi_handle handle)
{
acpi_status status;
unsigned long long tmp;
- int seg, bus;
acpi_handle dummy_handle;
- struct pci_bus *pci_bus;
/* if the bridge doesn't have _STA, we assume it is always there */
status = acpi_get_handle(handle, "_STA", &dummy_handle);
@@ -526,36 +523,15 @@ static int add_bridge(acpi_handle handle)
return 0;
}
- /* get PCI segment number */
- status = acpi_evaluate_integer(handle, "_SEG", NULL, &tmp);
-
- seg = ACPI_SUCCESS(status) ? tmp : 0;
-
- /* get PCI bus number */
- status = acpi_evaluate_integer(handle, "_BBN", NULL, &tmp);
-
- if (ACPI_SUCCESS(status)) {
- bus = tmp;
- } else {
- warn("can't get bus number, assuming 0\n");
- bus = 0;
- }
-
- pci_bus = pci_find_bus(seg, bus);
- if (!pci_bus) {
- err("Can't find bus %04x:%02x\n", seg, bus);
- return 0;
- }
-
/* check if this bridge has ejectable slots */
- if (detect_ejectable_slots(pci_bus) > 0) {
+ if (detect_ejectable_slots(handle) > 0) {
dbg("found PCI host-bus bridge with hot-pluggable slots\n");
- add_host_bridge(handle, pci_bus);
+ add_host_bridge(handle);
}
/* search P2P bridges under this host bridge */
status = acpi_walk_namespace(ACPI_TYPE_DEVICE, handle, (u32)1,
- find_p2p_bridge, pci_bus, NULL);
+ find_p2p_bridge, NULL, NULL);
if (ACPI_FAILURE(status))
warn("find_p2p_bridge failed (error code = 0x%x)\n", status);
@@ -1387,16 +1363,7 @@ static void acpiphp_sanitize_bus(struct pci_bus *bus)
/* Program resources in newly inserted bridge */
static int acpiphp_configure_bridge (acpi_handle handle)
{
- struct pci_dev *dev;
- struct pci_bus *bus;
-
- dev = acpi_get_pci_dev(handle);
- if (!dev) {
- err("cannot get PCI domain and bus number for bridge\n");
- return -EINVAL;
- }
-
- bus = dev->bus;
+ struct pci_bus *bus = pci_bus_from_handle(handle);
pci_bus_size_bridges(bus);
pci_bus_assign_resources(bus);
@@ -1404,7 +1371,6 @@ static int acpiphp_configure_bridge (acpi_handle handle)
acpiphp_set_hpp_values(handle, bus);
pci_enable_bridges(bus);
acpiphp_configure_ioapics(handle);
- pci_dev_put(dev);
return 0;
}
^ permalink raw reply related [flat|nested] 8+ messages in thread
* [PATCH 3/3] PCI Hotplug: convert acpi_pci_detect_ejectable() to take an acpi_handle
2009-07-10 19:42 [PATCH 0/3] ACPI/PCI Hotplug: acpiphp cleanup Alex Chiang
2009-07-10 19:42 ` [PATCH 1/3] ACPI: export acpi_pci_root and friends Alex Chiang
2009-07-10 19:42 ` [PATCH 2/3] PCI Hotplug: acpiphp: find bridges the easy way Alex Chiang
@ 2009-07-10 19:42 ` Alex Chiang
2009-07-14 19:27 ` [PATCH 0/3] ACPI/PCI Hotplug: acpiphp cleanup Jesse Barnes
3 siblings, 0 replies; 8+ messages in thread
From: Alex Chiang @ 2009-07-10 19:42 UTC (permalink / raw)
To: jbarnes, lenb; +Cc: linux-pci, linux-kernel, linux-acpi
acpi_pci_detect_ejectable() goes through effort to convert its
struct pci_bus arg to an acpi_handle, but every time we use this
interface, we already have the handle available.
So let's just use the handle instead of converting back and forth.
Signed-off-by: Alex Chiang <achiang@hp.com>
---
drivers/pci/hotplug/acpi_pcihp.c | 9 +++------
drivers/pci/hotplug/acpiphp_glue.c | 32 +++++++++++---------------------
drivers/pci/hotplug/pciehp_acpi.c | 5 +++--
include/linux/pci_hotplug.h | 2 +-
4 files changed, 18 insertions(+), 30 deletions(-)
diff --git a/drivers/pci/hotplug/acpi_pcihp.c b/drivers/pci/hotplug/acpi_pcihp.c
index eb15958..80af75c 100644
--- a/drivers/pci/hotplug/acpi_pcihp.c
+++ b/drivers/pci/hotplug/acpi_pcihp.c
@@ -500,18 +500,15 @@ check_hotplug(acpi_handle handle, u32 lvl, void *context, void **rv)
/**
* acpi_pci_detect_ejectable - check if the PCI bus has ejectable slots
- * @pbus - PCI bus to scan
+ * @handle - handle of the PCI bus to scan
*
* Returns 1 if the PCI bus has ACPI based ejectable slots, 0 otherwise.
*/
-int acpi_pci_detect_ejectable(struct pci_bus *pbus)
+int acpi_pci_detect_ejectable(acpi_handle handle)
{
- acpi_handle handle;
int found = 0;
- if (!(handle = acpi_pci_get_bridge_handle(pbus)))
- return 0;
- acpi_walk_namespace(ACPI_TYPE_DEVICE, handle, (u32)1,
+ acpi_walk_namespace(ACPI_TYPE_DEVICE, handle, 1,
check_hotplug, (void *)&found, NULL);
return found;
}
diff --git a/drivers/pci/hotplug/acpiphp_glue.c b/drivers/pci/hotplug/acpiphp_glue.c
index de4afc6..680c336 100644
--- a/drivers/pci/hotplug/acpiphp_glue.c
+++ b/drivers/pci/hotplug/acpiphp_glue.c
@@ -62,21 +62,6 @@ static void acpiphp_sanitize_bus(struct pci_bus *bus);
static void acpiphp_set_hpp_values(acpi_handle handle, struct pci_bus *bus);
static void handle_hotplug_event_func(acpi_handle handle, u32 type, void *context);
-static struct pci_bus *pci_bus_from_handle(acpi_handle handle)
-{
- struct pci_bus *pbus;
-
- if (acpi_is_root_bridge(handle)) {
- struct acpi_pci_root *root = acpi_pci_find_root(handle);
- pbus = root->bus;
- } else {
- struct pci_dev *pdev = acpi_get_pci_dev(handle);
- pbus = pdev->subordinate;
- pci_dev_put(pdev);
- }
- return pbus;
-}
-
/* callback routine to check for the existence of a pci dock device */
static acpi_status
is_pci_dock_device(acpi_handle handle, u32 lvl, void *context, void **rv)
@@ -278,11 +263,7 @@ register_slot(acpi_handle handle, u32 lvl, void *context, void **rv)
/* see if it's worth looking at this bridge */
static int detect_ejectable_slots(acpi_handle handle)
{
- int found;
- struct pci_bus *pbus;
-
- pbus = pci_bus_from_handle(handle);
- found = acpi_pci_detect_ejectable(pbus);
+ int found = acpi_pci_detect_ejectable(handle);
if (!found) {
acpi_walk_namespace(ACPI_TYPE_DEVICE, handle, (u32)1,
is_pci_dock_device, (void *)&found, NULL);
@@ -1363,7 +1344,16 @@ static void acpiphp_sanitize_bus(struct pci_bus *bus)
/* Program resources in newly inserted bridge */
static int acpiphp_configure_bridge (acpi_handle handle)
{
- struct pci_bus *bus = pci_bus_from_handle(handle);
+ struct pci_bus *bus;
+
+ if (acpi_is_root_bridge(handle)) {
+ struct acpi_pci_root *root = acpi_pci_find_root(handle);
+ bus = root->bus;
+ } else {
+ struct pci_dev *pdev = acpi_get_pci_dev(handle);
+ bus = pdev->subordinate;
+ pci_dev_put(pdev);
+ }
pci_bus_size_bridges(bus);
pci_bus_assign_resources(bus);
diff --git a/drivers/pci/hotplug/pciehp_acpi.c b/drivers/pci/hotplug/pciehp_acpi.c
index 9604801..0c6aa99 100644
--- a/drivers/pci/hotplug/pciehp_acpi.c
+++ b/drivers/pci/hotplug/pciehp_acpi.c
@@ -47,7 +47,7 @@ int pciehp_acpi_slot_detection_check(struct pci_dev *dev)
{
if (slot_detection_mode != PCIEHP_DETECT_ACPI)
return 0;
- if (acpi_pci_detect_ejectable(dev->subordinate))
+ if (acpi_pci_detect_ejectable(dev->dev.archdata.acpi_handle))
return 0;
return -ENODEV;
}
@@ -94,7 +94,8 @@ static int __init dummy_probe(struct pcie_device *dev)
dup_slot_id++;
}
list_add_tail(&slot->slot_list, &dummy_slots);
- if (!acpi_slot_detected && acpi_pci_detect_ejectable(pbus))
+ if (!acpi_slot_detected &&
+ acpi_pci_detect_ejectable(pbus->self->dev.archdata.acpi_handle))
acpi_slot_detected = 1;
return -ENODEV; /* dummy driver always returns error */
}
diff --git a/include/linux/pci_hotplug.h b/include/linux/pci_hotplug.h
index 4391741..43394fc 100644
--- a/include/linux/pci_hotplug.h
+++ b/include/linux/pci_hotplug.h
@@ -230,7 +230,7 @@ extern acpi_status acpi_get_hp_params_from_firmware(struct pci_bus *bus,
struct hotplug_params *hpp);
int acpi_get_hp_hw_control_from_firmware(struct pci_dev *dev, u32 flags);
int acpi_pci_check_ejectable(struct pci_bus *pbus, acpi_handle handle);
-int acpi_pci_detect_ejectable(struct pci_bus *pbus);
+int acpi_pci_detect_ejectable(acpi_handle handle);
#endif
#endif
^ permalink raw reply related [flat|nested] 8+ messages in thread
* Re: [PATCH 0/3] ACPI/PCI Hotplug: acpiphp cleanup
2009-07-10 19:42 [PATCH 0/3] ACPI/PCI Hotplug: acpiphp cleanup Alex Chiang
` (2 preceding siblings ...)
2009-07-10 19:42 ` [PATCH 3/3] PCI Hotplug: convert acpi_pci_detect_ejectable() to take an acpi_handle Alex Chiang
@ 2009-07-14 19:27 ` Jesse Barnes
2009-07-14 20:04 ` Alex Chiang
3 siblings, 1 reply; 8+ messages in thread
From: Jesse Barnes @ 2009-07-14 19:27 UTC (permalink / raw)
To: Alex Chiang; +Cc: lenb, linux-pci, linux-kernel, linux-acpi
On Fri, 10 Jul 2009 13:42:33 -0600
Alex Chiang <achiang@hp.com> wrote:
> Hi Len, Jesse,
>
> I'd like to have this patch series considered for the current release
> because it clarifies some usage of the new interface
> acpi_get_pci_dev() that was also introduced in this release cycle.
>
> The short story is, acpi_get_pci_dev() doesn't always return a struct
> pci_dev, because a PCI root bridge may not have an associated pci_dev
> on platforms with non-materialized root bridges.
>
> See this patch for more:
>
> http://patchwork.kernel.org/patch/32613/
>
> I had to export acpi_pci_root to accomplish my goal, but it did result
> in a nice net cleanup in acpiphp.
>
> It should probably go through Jesse's tree, but needs Len's ACK for
> the acpi_pci_root bit.
Haven't heard from Len yet, but I'm a bit nervous about adding this to
for-linus. We already have a fix upstream for the acpi_get_pci_dev
issue right? So there's no hurry on this set of (nice) cleanups is
there?
Thanks,
--
Jesse Barnes, Intel Open Source Technology Center
^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [PATCH 0/3] ACPI/PCI Hotplug: acpiphp cleanup
2009-07-14 19:27 ` [PATCH 0/3] ACPI/PCI Hotplug: acpiphp cleanup Jesse Barnes
@ 2009-07-14 20:04 ` Alex Chiang
2009-07-14 20:32 ` Jesse Barnes
2009-07-14 20:33 ` Jesse Barnes
0 siblings, 2 replies; 8+ messages in thread
From: Alex Chiang @ 2009-07-14 20:04 UTC (permalink / raw)
To: Jesse Barnes; +Cc: lenb, linux-pci, linux-kernel, linux-acpi
* Jesse Barnes <jbarnes@virtuousgeek.org>:
> Haven't heard from Len yet, but I'm a bit nervous about adding this to
> for-linus. We already have a fix upstream for the acpi_get_pci_dev
> issue right? So there's no hurry on this set of (nice) cleanups is
> there?
The problem is that I introduced an assumption in acpiphp_configure_bridge,
which calls acpi_get_pci_dev. That call may fail on systems
with non-materialized PCI root bridges.
Fixing that assumption was addressed by this hunk in patch 2/3:
@@ -1387,16 +1363,7 @@ static void acpiphp_sanitize_bus(struct pci_bus *bus)
/* Program resources in newly inserted bridge */
static int acpiphp_configure_bridge (acpi_handle handle)
{
- struct pci_dev *dev;
- struct pci_bus *bus;
-
- dev = acpi_get_pci_dev(handle);
- if (!dev) {
- err("cannot get PCI domain and bus number for bridge\n");
- return -EINVAL;
- }
-
- bus = dev->bus;
+ struct pci_bus *bus = pci_bus_from_handle(handle);
pci_bus_size_bridges(bus);
pci_bus_assign_resources(bus);
Now, my systems cannot do PCI root bridge hotplug, so that line
where we call acpi_get_pci_dev() won't cause us to fail, but I'm
concerned about machines in the wild that:
a) can do PCI root bridge hotplug
b) have non-materialized PCI root bridges
Maybe that is a small set of machines...
I agree that this series is on the large side for -rc4 and
beyond, but I'd prefer to fix it now.
The thing is, a "minimal" fix wouldn't save us that much, since
we'd still have to export acpi_pci_root stuff. We could maybe
drop patch 3/3 which makes the final diffstat look like this:
drivers/acpi/pci_root.c | 17 +-----
include/acpi/acpi_bus.h | 14 +++++
drivers/pci/hotplug/acpiphp_glue.c | 108 ++++++++++++------------------------
3 files changed, 53 insertions(+), 86 deletions(-)
Hm, actually, I can actually make that even smaller by separating
out the interface change.
Would you like me to do that and resubmit? I'd still need an ACK
from Len re: acpi_pci_root...
Thanks.
/ac
^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [PATCH 0/3] ACPI/PCI Hotplug: acpiphp cleanup
2009-07-14 20:04 ` Alex Chiang
@ 2009-07-14 20:32 ` Jesse Barnes
2009-07-14 20:33 ` Jesse Barnes
1 sibling, 0 replies; 8+ messages in thread
From: Jesse Barnes @ 2009-07-14 20:32 UTC (permalink / raw)
To: Alex Chiang; +Cc: lenb, linux-pci, linux-kernel, linux-acpi
On Tue, 14 Jul 2009 14:04:31 -0600
Alex Chiang <achiang@hp.com> wrote:
> * Jesse Barnes <jbarnes@virtuousgeek.org>:
> > Haven't heard from Len yet, but I'm a bit nervous about adding this
> > to for-linus. We already have a fix upstream for the
> > acpi_get_pci_dev issue right? So there's no hurry on this set of
> > (nice) cleanups is there?
>
> The problem is that I introduced an assumption in
> acpiphp_configure_bridge, which calls acpi_get_pci_dev. That call may
> fail on systems with non-materialized PCI root bridges.
>
> Fixing that assumption was addressed by this hunk in patch 2/3:
>
> @@ -1387,16 +1363,7 @@ static void acpiphp_sanitize_bus(struct
> pci_bus *bus) /* Program resources in newly inserted bridge */
> static int acpiphp_configure_bridge (acpi_handle handle)
> {
> - struct pci_dev *dev;
> - struct pci_bus *bus;
> -
> - dev = acpi_get_pci_dev(handle);
> - if (!dev) {
> - err("cannot get PCI domain and bus number for
> bridge\n");
> - return -EINVAL;
> - }
> -
> - bus = dev->bus;
> + struct pci_bus *bus = pci_bus_from_handle(handle);
>
> pci_bus_size_bridges(bus);
> pci_bus_assign_resources(bus);
>
> Now, my systems cannot do PCI root bridge hotplug, so that line
> where we call acpi_get_pci_dev() won't cause us to fail, but I'm
> concerned about machines in the wild that:
>
> a) can do PCI root bridge hotplug
> b) have non-materialized PCI root bridges
>
> Maybe that is a small set of machines...
>
> I agree that this series is on the large side for -rc4 and
> beyond, but I'd prefer to fix it now.
>
> The thing is, a "minimal" fix wouldn't save us that much, since
> we'd still have to export acpi_pci_root stuff. We could maybe
> drop patch 3/3 which makes the final diffstat look like this:
>
> drivers/acpi/pci_root.c | 17 +-----
> include/acpi/acpi_bus.h | 14 +++++
> drivers/pci/hotplug/acpiphp_glue.c | 108
> ++++++++++++------------------------ 3 files changed, 53
> insertions(+), 86 deletions(-)
>
> Hm, actually, I can actually make that even smaller by separating
> out the interface change.
>
> Would you like me to do that and resubmit? I'd still need an ACK
> from Len re: acpi_pci_root...
>
> Thanks.
>
> /ac
>
>
--
Jesse Barnes, Intel Open Source Technology Center
^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [PATCH 0/3] ACPI/PCI Hotplug: acpiphp cleanup
2009-07-14 20:04 ` Alex Chiang
2009-07-14 20:32 ` Jesse Barnes
@ 2009-07-14 20:33 ` Jesse Barnes
1 sibling, 0 replies; 8+ messages in thread
From: Jesse Barnes @ 2009-07-14 20:33 UTC (permalink / raw)
To: Alex Chiang; +Cc: lenb, linux-pci, linux-kernel, linux-acpi
On Tue, 14 Jul 2009 14:04:31 -0600
Alex Chiang <achiang@hp.com> wrote:
> * Jesse Barnes <jbarnes@virtuousgeek.org>:
> > Haven't heard from Len yet, but I'm a bit nervous about adding this
> > to for-linus. We already have a fix upstream for the
> > acpi_get_pci_dev issue right? So there's no hurry on this set of
> > (nice) cleanups is there?
>
> The problem is that I introduced an assumption in
> acpiphp_configure_bridge, which calls acpi_get_pci_dev. That call may
> fail on systems with non-materialized PCI root bridges.
>
> Fixing that assumption was addressed by this hunk in patch 2/3:
>
> @@ -1387,16 +1363,7 @@ static void acpiphp_sanitize_bus(struct
> pci_bus *bus) /* Program resources in newly inserted bridge */
> static int acpiphp_configure_bridge (acpi_handle handle)
> {
> - struct pci_dev *dev;
> - struct pci_bus *bus;
> -
> - dev = acpi_get_pci_dev(handle);
> - if (!dev) {
> - err("cannot get PCI domain and bus number for
> bridge\n");
> - return -EINVAL;
> - }
> -
> - bus = dev->bus;
> + struct pci_bus *bus = pci_bus_from_handle(handle);
>
> pci_bus_size_bridges(bus);
> pci_bus_assign_resources(bus);
>
> Now, my systems cannot do PCI root bridge hotplug, so that line
> where we call acpi_get_pci_dev() won't cause us to fail, but I'm
> concerned about machines in the wild that:
>
> a) can do PCI root bridge hotplug
> b) have non-materialized PCI root bridges
>
> Maybe that is a small set of machines...
>
> I agree that this series is on the large side for -rc4 and
> beyond, but I'd prefer to fix it now.
>
> The thing is, a "minimal" fix wouldn't save us that much, since
> we'd still have to export acpi_pci_root stuff. We could maybe
> drop patch 3/3 which makes the final diffstat look like this:
>
> drivers/acpi/pci_root.c | 17 +-----
> include/acpi/acpi_bus.h | 14 +++++
> drivers/pci/hotplug/acpiphp_glue.c | 108
> ++++++++++++------------------------ 3 files changed, 53
> insertions(+), 86 deletions(-)
>
> Hm, actually, I can actually make that even smaller by separating
> out the interface change.
>
> Would you like me to do that and resubmit? I'd still need an ACK
> from Len re: acpi_pci_root...
How about I actually send a reply this time...
No you don't need to re-submit; assuming Len comes back quickly we can
put this into my for-linus branch. Len, we just need your ack to apply
this series.
Thanks,
--
Jesse Barnes, Intel Open Source Technology Center
^ permalink raw reply [flat|nested] 8+ messages in thread
end of thread, other threads:[~2009-07-14 20:33 UTC | newest]
Thread overview: 8+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2009-07-10 19:42 [PATCH 0/3] ACPI/PCI Hotplug: acpiphp cleanup Alex Chiang
2009-07-10 19:42 ` [PATCH 1/3] ACPI: export acpi_pci_root and friends Alex Chiang
2009-07-10 19:42 ` [PATCH 2/3] PCI Hotplug: acpiphp: find bridges the easy way Alex Chiang
2009-07-10 19:42 ` [PATCH 3/3] PCI Hotplug: convert acpi_pci_detect_ejectable() to take an acpi_handle Alex Chiang
2009-07-14 19:27 ` [PATCH 0/3] ACPI/PCI Hotplug: acpiphp cleanup Jesse Barnes
2009-07-14 20:04 ` Alex Chiang
2009-07-14 20:32 ` Jesse Barnes
2009-07-14 20:33 ` Jesse Barnes
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.