From: Alex Chiang <achiang@hp.com>
To: Jesse Barnes <jbarnes@virtuousgeek.org>
Cc: lenb@kernel.org, linux-pci@vger.kernel.org,
linux-kernel@vger.kernel.org, linux-acpi@vger.kernel.org
Subject: Re: [PATCH 0/3] ACPI/PCI Hotplug: acpiphp cleanup
Date: Tue, 14 Jul 2009 14:04:31 -0600 [thread overview]
Message-ID: <20090714200431.GC15021@ldl.fc.hp.com> (raw)
In-Reply-To: <20090714122731.1ebea158@jbarnes-g45>
* 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
next prev parent reply other threads:[~2009-07-14 20:04 UTC|newest]
Thread overview: 8+ messages / expand[flat|nested] mbox.gz Atom feed top
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 [this message]
2009-07-14 20:32 ` Jesse Barnes
2009-07-14 20:33 ` Jesse Barnes
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=20090714200431.GC15021@ldl.fc.hp.com \
--to=achiang@hp.com \
--cc=jbarnes@virtuousgeek.org \
--cc=lenb@kernel.org \
--cc=linux-acpi@vger.kernel.org \
--cc=linux-kernel@vger.kernel.org \
--cc=linux-pci@vger.kernel.org \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox