* [PATCH 1/8] PCI: Separate out pci_assign_unassigned_bus_resources()
2012-09-27 8:11 [PATCH 0/8] PCI, ACPI, x86: pci root bus hotplug support pci_root.c related core changes Yinghai Lu
@ 2012-09-27 8:11 ` Yinghai Lu
2012-09-27 8:11 ` [PATCH 2/8] PCI: Move pci_rescan_bus() back to probe.c Yinghai Lu
` (6 subsequent siblings)
7 siblings, 0 replies; 26+ messages in thread
From: Yinghai Lu @ 2012-09-27 8:11 UTC (permalink / raw)
To: Bjorn Helgaas, Len Brown, Taku Izumi, Jiang Liu
Cc: linux-pci, linux-acpi, Yinghai Lu
It is main portion of pci_rescan_bus().
Separate it out and prepare to use it for pci root bus hot add later.
Signed-off-by: Yinghai Lu <yinghai@kernel.org>
---
drivers/pci/setup-bus.c | 32 ++++++++++++++++++--------------
include/linux/pci.h | 1 +
2 files changed, 19 insertions(+), 14 deletions(-)
diff --git a/drivers/pci/setup-bus.c b/drivers/pci/setup-bus.c
index 1e808ca..1f34929 100644
--- a/drivers/pci/setup-bus.c
+++ b/drivers/pci/setup-bus.c
@@ -1550,25 +1550,12 @@ enable_all:
}
EXPORT_SYMBOL_GPL(pci_assign_unassigned_bridge_resources);
-#ifdef CONFIG_HOTPLUG
-/**
- * pci_rescan_bus - scan a PCI bus for devices.
- * @bus: PCI bus to scan
- *
- * Scan a PCI bus and child buses for new devices, adds them,
- * and enables them.
- *
- * Returns the max number of subordinate bus discovered.
- */
-unsigned int __ref pci_rescan_bus(struct pci_bus *bus)
+void pci_assign_unassigned_bus_resources(struct pci_bus *bus)
{
- unsigned int max;
struct pci_dev *dev;
LIST_HEAD(add_list); /* list of resources that
want additional resources */
- max = pci_scan_child_bus(bus);
-
down_read(&pci_bus_sem);
list_for_each_entry(dev, &bus->devices, bus_list)
if (dev->hdr_type == PCI_HEADER_TYPE_BRIDGE ||
@@ -1581,6 +1568,23 @@ unsigned int __ref pci_rescan_bus(struct pci_bus *bus)
BUG_ON(!list_empty(&add_list));
pci_enable_bridges(bus);
+}
+#ifdef CONFIG_HOTPLUG
+/**
+ * pci_rescan_bus - scan a PCI bus for devices.
+ * @bus: PCI bus to scan
+ *
+ * Scan a PCI bus and child buses for new devices, adds them,
+ * and enables them.
+ *
+ * Returns the max number of subordinate bus discovered.
+ */
+unsigned int __ref pci_rescan_bus(struct pci_bus *bus)
+{
+ unsigned int max;
+
+ max = pci_scan_child_bus(bus);
+ pci_assign_unassigned_bus_resources(bus);
pci_bus_add_devices(bus);
return max;
diff --git a/include/linux/pci.h b/include/linux/pci.h
index be1de01..505c05a 100644
--- a/include/linux/pci.h
+++ b/include/linux/pci.h
@@ -980,6 +980,7 @@ void pci_bus_size_bridges(struct pci_bus *bus);
int pci_claim_resource(struct pci_dev *, int);
void pci_assign_unassigned_resources(void);
void pci_assign_unassigned_bridge_resources(struct pci_dev *bridge);
+void pci_assign_unassigned_bus_resources(struct pci_bus *bus);
void pdev_enable_device(struct pci_dev *);
int pci_enable_resources(struct pci_dev *, int mask);
void pci_fixup_irqs(u8 (*)(struct pci_dev *, u8 *),
--
1.7.7
^ permalink raw reply related [flat|nested] 26+ messages in thread* [PATCH 2/8] PCI: Move pci_rescan_bus() back to probe.c
2012-09-27 8:11 [PATCH 0/8] PCI, ACPI, x86: pci root bus hotplug support pci_root.c related core changes Yinghai Lu
2012-09-27 8:11 ` [PATCH 1/8] PCI: Separate out pci_assign_unassigned_bus_resources() Yinghai Lu
@ 2012-09-27 8:11 ` Yinghai Lu
2012-09-27 8:11 ` [PATCH 3/8] PCI: Move out pci_enable_bridges out of assign_unsigned_bus_res Yinghai Lu
` (5 subsequent siblings)
7 siblings, 0 replies; 26+ messages in thread
From: Yinghai Lu @ 2012-09-27 8:11 UTC (permalink / raw)
To: Bjorn Helgaas, Len Brown, Taku Izumi, Jiang Liu
Cc: linux-pci, linux-acpi, Yinghai Lu
We have pci_assign_unassigned_bus_resources() in as global function now.
Move back pci_rescan_bus to probe.c where it should be.
Signed-off-by: Yinghai Lu <yinghai@kernel.org>
---
drivers/pci/probe.c | 21 +++++++++++++++++++++
drivers/pci/setup-bus.c | 22 ----------------------
2 files changed, 21 insertions(+), 22 deletions(-)
diff --git a/drivers/pci/probe.c b/drivers/pci/probe.c
index ec909af..65f62e3 100644
--- a/drivers/pci/probe.c
+++ b/drivers/pci/probe.c
@@ -1890,6 +1890,27 @@ unsigned int __ref pci_rescan_bus_bridge_resize(struct pci_dev *bridge)
return max;
}
+/**
+ * pci_rescan_bus - scan a PCI bus for devices.
+ * @bus: PCI bus to scan
+ *
+ * Scan a PCI bus and child buses for new devices, adds them,
+ * and enables them.
+ *
+ * Returns the max number of subordinate bus discovered.
+ */
+unsigned int __ref pci_rescan_bus(struct pci_bus *bus)
+{
+ unsigned int max;
+
+ max = pci_scan_child_bus(bus);
+ pci_assign_unassigned_bus_resources(bus);
+ pci_bus_add_devices(bus);
+
+ return max;
+}
+EXPORT_SYMBOL_GPL(pci_rescan_bus);
+
EXPORT_SYMBOL(pci_add_new_bus);
EXPORT_SYMBOL(pci_scan_slot);
EXPORT_SYMBOL(pci_scan_bridge);
diff --git a/drivers/pci/setup-bus.c b/drivers/pci/setup-bus.c
index 1f34929..59e6c55 100644
--- a/drivers/pci/setup-bus.c
+++ b/drivers/pci/setup-bus.c
@@ -1569,25 +1569,3 @@ void pci_assign_unassigned_bus_resources(struct pci_bus *bus)
pci_enable_bridges(bus);
}
-#ifdef CONFIG_HOTPLUG
-/**
- * pci_rescan_bus - scan a PCI bus for devices.
- * @bus: PCI bus to scan
- *
- * Scan a PCI bus and child buses for new devices, adds them,
- * and enables them.
- *
- * Returns the max number of subordinate bus discovered.
- */
-unsigned int __ref pci_rescan_bus(struct pci_bus *bus)
-{
- unsigned int max;
-
- max = pci_scan_child_bus(bus);
- pci_assign_unassigned_bus_resources(bus);
- pci_bus_add_devices(bus);
-
- return max;
-}
-EXPORT_SYMBOL_GPL(pci_rescan_bus);
-#endif
--
1.7.7
^ permalink raw reply related [flat|nested] 26+ messages in thread* [PATCH 3/8] PCI: Move out pci_enable_bridges out of assign_unsigned_bus_res
2012-09-27 8:11 [PATCH 0/8] PCI, ACPI, x86: pci root bus hotplug support pci_root.c related core changes Yinghai Lu
2012-09-27 8:11 ` [PATCH 1/8] PCI: Separate out pci_assign_unassigned_bus_resources() Yinghai Lu
2012-09-27 8:11 ` [PATCH 2/8] PCI: Move pci_rescan_bus() back to probe.c Yinghai Lu
@ 2012-09-27 8:11 ` Yinghai Lu
2012-09-28 23:46 ` Bjorn Helgaas
2012-09-27 8:11 ` [PATCH 4/8] PCI, ACPI: assign unassigned resource for hot add root bus Yinghai Lu
` (4 subsequent siblings)
7 siblings, 1 reply; 26+ messages in thread
From: Yinghai Lu @ 2012-09-27 8:11 UTC (permalink / raw)
To: Bjorn Helgaas, Len Brown, Taku Izumi, Jiang Liu
Cc: linux-pci, linux-acpi, Yinghai Lu
So could use assign_unassigned_bus_res pci root bus add
Signed-off-by: Yinghai Lu <yinghai@kernel.org>
---
drivers/pci/probe.c | 1 +
drivers/pci/setup-bus.c | 2 --
2 files changed, 1 insertions(+), 2 deletions(-)
diff --git a/drivers/pci/probe.c b/drivers/pci/probe.c
index 65f62e3..59cf1ba 100644
--- a/drivers/pci/probe.c
+++ b/drivers/pci/probe.c
@@ -1905,6 +1905,7 @@ unsigned int __ref pci_rescan_bus(struct pci_bus *bus)
max = pci_scan_child_bus(bus);
pci_assign_unassigned_bus_resources(bus);
+ pci_enable_bridges(bus);
pci_bus_add_devices(bus);
return max;
diff --git a/drivers/pci/setup-bus.c b/drivers/pci/setup-bus.c
index 59e6c55..6d3591d 100644
--- a/drivers/pci/setup-bus.c
+++ b/drivers/pci/setup-bus.c
@@ -1566,6 +1566,4 @@ void pci_assign_unassigned_bus_resources(struct pci_bus *bus)
up_read(&pci_bus_sem);
__pci_bus_assign_resources(bus, &add_list, NULL);
BUG_ON(!list_empty(&add_list));
-
- pci_enable_bridges(bus);
}
--
1.7.7
^ permalink raw reply related [flat|nested] 26+ messages in thread* Re: [PATCH 3/8] PCI: Move out pci_enable_bridges out of assign_unsigned_bus_res
2012-09-27 8:11 ` [PATCH 3/8] PCI: Move out pci_enable_bridges out of assign_unsigned_bus_res Yinghai Lu
@ 2012-09-28 23:46 ` Bjorn Helgaas
2012-09-29 1:48 ` Yinghai Lu
0 siblings, 1 reply; 26+ messages in thread
From: Bjorn Helgaas @ 2012-09-28 23:46 UTC (permalink / raw)
To: Yinghai Lu; +Cc: Len Brown, Taku Izumi, Jiang Liu, linux-pci, linux-acpi
On Thu, Sep 27, 2012 at 2:11 AM, Yinghai Lu <yinghai@kernel.org> wrote:
> So could use assign_unassigned_bus_res pci root bus add
After your series, acpi_pci_root_start() looks like this:
pci_assign_unassigned_bus_resources
list_for_each_entry(driver, &acpi_pci_drivers, node)
driver->add(root);
pci_enable_bridges(root->bus);
so apparently it's important that the driver->add() methods be run
*before* the bridges are enabled. Why?
> Signed-off-by: Yinghai Lu <yinghai@kernel.org>
> ---
> drivers/pci/probe.c | 1 +
> drivers/pci/setup-bus.c | 2 --
> 2 files changed, 1 insertions(+), 2 deletions(-)
>
> diff --git a/drivers/pci/probe.c b/drivers/pci/probe.c
> index 65f62e3..59cf1ba 100644
> --- a/drivers/pci/probe.c
> +++ b/drivers/pci/probe.c
> @@ -1905,6 +1905,7 @@ unsigned int __ref pci_rescan_bus(struct pci_bus *bus)
>
> max = pci_scan_child_bus(bus);
> pci_assign_unassigned_bus_resources(bus);
> + pci_enable_bridges(bus);
> pci_bus_add_devices(bus);
>
> return max;
> diff --git a/drivers/pci/setup-bus.c b/drivers/pci/setup-bus.c
> index 59e6c55..6d3591d 100644
> --- a/drivers/pci/setup-bus.c
> +++ b/drivers/pci/setup-bus.c
> @@ -1566,6 +1566,4 @@ void pci_assign_unassigned_bus_resources(struct pci_bus *bus)
> up_read(&pci_bus_sem);
> __pci_bus_assign_resources(bus, &add_list, NULL);
> BUG_ON(!list_empty(&add_list));
> -
> - pci_enable_bridges(bus);
> }
> --
> 1.7.7
>
^ permalink raw reply [flat|nested] 26+ messages in thread* Re: [PATCH 3/8] PCI: Move out pci_enable_bridges out of assign_unsigned_bus_res
2012-09-28 23:46 ` Bjorn Helgaas
@ 2012-09-29 1:48 ` Yinghai Lu
2012-09-29 1:52 ` Yinghai Lu
0 siblings, 1 reply; 26+ messages in thread
From: Yinghai Lu @ 2012-09-29 1:48 UTC (permalink / raw)
To: Bjorn Helgaas; +Cc: Len Brown, Taku Izumi, Jiang Liu, linux-pci, linux-acpi
On Fri, Sep 28, 2012 at 4:46 PM, Bjorn Helgaas <bhelgaas@google.com> wrote:
> On Thu, Sep 27, 2012 at 2:11 AM, Yinghai Lu <yinghai@kernel.org> wrote:
>> So could use assign_unassigned_bus_res pci root bus add
>
> After your series, acpi_pci_root_start() looks like this:
>
> pci_assign_unassigned_bus_resources
> list_for_each_entry(driver, &acpi_pci_drivers, node)
> driver->add(root);
> pci_enable_bridges(root->bus);
>
> so apparently it's important that the driver->add() methods be run
> *before* the bridges are enabled. Why?
pci_enable_bridge
^ permalink raw reply [flat|nested] 26+ messages in thread
* Re: [PATCH 3/8] PCI: Move out pci_enable_bridges out of assign_unsigned_bus_res
2012-09-29 1:48 ` Yinghai Lu
@ 2012-09-29 1:52 ` Yinghai Lu
2012-09-29 3:27 ` Bjorn Helgaas
2012-09-29 4:13 ` Yinghai Lu
0 siblings, 2 replies; 26+ messages in thread
From: Yinghai Lu @ 2012-09-29 1:52 UTC (permalink / raw)
To: Bjorn Helgaas; +Cc: Len Brown, Taku Izumi, Jiang Liu, linux-pci, linux-acpi
On Fri, Sep 28, 2012 at 6:48 PM, Yinghai Lu <yinghai@kernel.org> wrote:
> On Fri, Sep 28, 2012 at 4:46 PM, Bjorn Helgaas <bhelgaas@google.com> wrote:
>> On Thu, Sep 27, 2012 at 2:11 AM, Yinghai Lu <yinghai@kernel.org> wrote:
>>> So could use assign_unassigned_bus_res pci root bus add
>>
>> After your series, acpi_pci_root_start() looks like this:
>>
>> pci_assign_unassigned_bus_resources
>> list_for_each_entry(driver, &acpi_pci_drivers, node)
>> driver->add(root);
>> pci_enable_bridges(root->bus);
>>
>> so apparently it's important that the driver->add() methods be run
>> *before* the bridges are enabled. Why?
>
During rebase, the changelog get lost.
pci_enable_bridges
==> pci_enable_device
==> ...
===> pcibios_enable_device
===> pcibios_enable_irq
and one of driver in acpi_pci_drivers, is ioapic drivers and it will
enable ioapic there.
So we need to enable bridges that later.
-Yinghai
^ permalink raw reply [flat|nested] 26+ messages in thread* Re: [PATCH 3/8] PCI: Move out pci_enable_bridges out of assign_unsigned_bus_res
2012-09-29 1:52 ` Yinghai Lu
@ 2012-09-29 3:27 ` Bjorn Helgaas
2012-09-29 4:04 ` Yinghai Lu
2012-09-29 4:13 ` Yinghai Lu
1 sibling, 1 reply; 26+ messages in thread
From: Bjorn Helgaas @ 2012-09-29 3:27 UTC (permalink / raw)
To: Yinghai Lu; +Cc: Len Brown, Taku Izumi, Jiang Liu, linux-pci, linux-acpi
On Fri, Sep 28, 2012 at 7:52 PM, Yinghai Lu <yinghai@kernel.org> wrote:
> On Fri, Sep 28, 2012 at 6:48 PM, Yinghai Lu <yinghai@kernel.org> wrote:
>> On Fri, Sep 28, 2012 at 4:46 PM, Bjorn Helgaas <bhelgaas@google.com> wrote:
>>> On Thu, Sep 27, 2012 at 2:11 AM, Yinghai Lu <yinghai@kernel.org> wrote:
>>>> So could use assign_unassigned_bus_res pci root bus add
>>>
>>> After your series, acpi_pci_root_start() looks like this:
>>>
>>> pci_assign_unassigned_bus_resources
>>> list_for_each_entry(driver, &acpi_pci_drivers, node)
>>> driver->add(root);
>>> pci_enable_bridges(root->bus);
>>>
>>> so apparently it's important that the driver->add() methods be run
>>> *before* the bridges are enabled. Why?
>>
>
> During rebase, the changelog get lost.
>
> pci_enable_bridges
> ==> pci_enable_device
> ==> ...
> ===> pcibios_enable_device
> ===> pcibios_enable_irq
>
> and one of driver in acpi_pci_drivers, is ioapic drivers and it will
> enable ioapic there.
>
> So we need to enable bridges that later.
Is it important that pci_assign_unassigned_bus_resources() be run
before the driver->add() methods? Why?
Do you have a pointer to this ioapic driver? I don't think it's in
the tree yet. I know about drivers/pci/ioapic.c, but it doesn't use
acpi_pci_register_driver().
^ permalink raw reply [flat|nested] 26+ messages in thread
* Re: [PATCH 3/8] PCI: Move out pci_enable_bridges out of assign_unsigned_bus_res
2012-09-29 3:27 ` Bjorn Helgaas
@ 2012-09-29 4:04 ` Yinghai Lu
2012-10-01 18:01 ` Bjorn Helgaas
0 siblings, 1 reply; 26+ messages in thread
From: Yinghai Lu @ 2012-09-29 4:04 UTC (permalink / raw)
To: Bjorn Helgaas; +Cc: Len Brown, Taku Izumi, Jiang Liu, linux-pci, linux-acpi
On Fri, Sep 28, 2012 at 8:27 PM, Bjorn Helgaas <bhelgaas@google.com> wrote:
>
> Is it important that pci_assign_unassigned_bus_resources() be run
> before the driver->add() methods? Why?
for ioapic controller that may need kernel to assign base bar.
>
> Do you have a pointer to this ioapic driver? I don't think it's in
> the tree yet. I know about drivers/pci/ioapic.c, but it doesn't use
> acpi_pci_register_driver().
it is in my for-x86-irq branch.
http://git.kernel.org/?p=linux/kernel/git/yinghai/linux-yinghai.git;a=shortlog;h=refs/heads/for-x86-irq
http://git.kernel.org/?p=linux/kernel/git/yinghai/linux-yinghai.git;a=commitdiff;h=c186df98f84336422088740ef8cedc1313ca2485
^ permalink raw reply [flat|nested] 26+ messages in thread
* Re: [PATCH 3/8] PCI: Move out pci_enable_bridges out of assign_unsigned_bus_res
2012-09-29 4:04 ` Yinghai Lu
@ 2012-10-01 18:01 ` Bjorn Helgaas
0 siblings, 0 replies; 26+ messages in thread
From: Bjorn Helgaas @ 2012-10-01 18:01 UTC (permalink / raw)
To: Yinghai Lu; +Cc: Len Brown, Taku Izumi, Jiang Liu, linux-pci, linux-acpi
On Fri, Sep 28, 2012 at 10:04 PM, Yinghai Lu <yinghai@kernel.org> wrote:
> On Fri, Sep 28, 2012 at 8:27 PM, Bjorn Helgaas <bhelgaas@google.com> wrote:
>>
>> Is it important that pci_assign_unassigned_bus_resources() be run
>> before the driver->add() methods? Why?
>
> for ioapic controller that may need kernel to assign base bar.
It seems like there are two cases of IOAPICs we need to worry about:
1) An IOAPIC device in the ACPI namespace (ACPI0009, ACPI000A, or
ACPI000B). This device has a _CRS that contains the device base
address. Since it's an ACPI device and has _CRS, I expect it should
*not* appear in PCI config space and would have no BARs.
2) An IOAPIC device that appears in PCI config space. This has a BAR
that we'll need to assign when hot-adding the device.
The spec (ACPI v5.0, sec 9.17) implies that the second type (a
bus-enumerated device) will not have an device in the ACPI namespace.
However, sec 6.2.6 says we need a _GSB for both types of IOAPICs, and
I don't know where the _GSB for bus-enumerated IOAPICs would live if
there's no ACPI device for them.
Since you're talking about assigning a BAR, I assume you're talking
about the second type -- a bus-enumerated IOAPIC. I would also assume
that a PCI IOAPIC would appear in the hierarchy *above* any devices
that are connected to it, so we'd enumerate it *before* those devices.
Maybe that will help ensure that we bind the driver to the IOAPIC
before binding drivers to devices that use it.
Can you help me understand this? What's the topology of the machine
you're working on? E.g., what does this part of the ACPI namespace
and PCI config space look like?
>> Do you have a pointer to this ioapic driver? I don't think it's in
>> the tree yet. I know about drivers/pci/ioapic.c, but it doesn't use
>> acpi_pci_register_driver().
>
> it is in my for-x86-irq branch.
>
> http://git.kernel.org/?p=linux/kernel/git/yinghai/linux-yinghai.git;a=shortlog;h=refs/heads/for-x86-irq
>
> http://git.kernel.org/?p=linux/kernel/git/yinghai/linux-yinghai.git;a=commitdiff;h=c186df98f84336422088740ef8cedc1313ca2485
^ permalink raw reply [flat|nested] 26+ messages in thread
* Re: [PATCH 3/8] PCI: Move out pci_enable_bridges out of assign_unsigned_bus_res
2012-09-29 1:52 ` Yinghai Lu
2012-09-29 3:27 ` Bjorn Helgaas
@ 2012-09-29 4:13 ` Yinghai Lu
1 sibling, 0 replies; 26+ messages in thread
From: Yinghai Lu @ 2012-09-29 4:13 UTC (permalink / raw)
To: Bjorn Helgaas; +Cc: Len Brown, Taku Izumi, Jiang Liu, linux-pci, linux-acpi
On Fri, Sep 28, 2012 at 6:52 PM, Yinghai Lu <yinghai@kernel.org> wrote:
> On Fri, Sep 28, 2012 at 6:48 PM, Yinghai Lu <yinghai@kernel.org> wrote:
>> On Fri, Sep 28, 2012 at 4:46 PM, Bjorn Helgaas <bhelgaas@google.com> wrote:
>>> On Thu, Sep 27, 2012 at 2:11 AM, Yinghai Lu <yinghai@kernel.org> wrote:
>>>> So could use assign_unassigned_bus_res pci root bus add
>>>
>>> After your series, acpi_pci_root_start() looks like this:
>>>
>>> pci_assign_unassigned_bus_resources
>>> list_for_each_entry(driver, &acpi_pci_drivers, node)
>>> driver->add(root);
>>> pci_enable_bridges(root->bus);
>>>
>>> so apparently it's important that the driver->add() methods be run
>>> *before* the bridges are enabled. Why?
>>
>
> During rebase, the changelog get lost.
>
> pci_enable_bridges
> ==> pci_enable_device
> ==> ...
> ===> pcibios_enable_device
> ===> pcibios_enable_irq
>
> and one of driver in acpi_pci_drivers, is ioapic drivers and it will
> enable ioapic there.
>
> So we need to enable bridges that later.
old changelog before rebase:
-----
Subject: [PATCH] PCI, x86: Move pci_enable_bridges() down
After we get hot-added ioapic registered.
pci_enable_bridges will try to enable ioapic irq for pci bridge.
So need to move it down.
Or We can move out pcibios_enable_irq() out of pci_enable_device()
and call pcibios_enable_irq in pci_bus_add_devices ?
also will need to move ...
pcibios_resource_survey_bus(root->bus);
pci_assign_unassigned_bus_resources(root->bus);
to the start add ....
----
^ permalink raw reply [flat|nested] 26+ messages in thread
* [PATCH 4/8] PCI, ACPI: assign unassigned resource for hot add root bus
2012-09-27 8:11 [PATCH 0/8] PCI, ACPI, x86: pci root bus hotplug support pci_root.c related core changes Yinghai Lu
` (2 preceding siblings ...)
2012-09-27 8:11 ` [PATCH 3/8] PCI: Move out pci_enable_bridges out of assign_unsigned_bus_res Yinghai Lu
@ 2012-09-27 8:11 ` Yinghai Lu
2012-09-28 23:46 ` Bjorn Helgaas
2012-09-27 8:11 ` [PATCH 5/8] PCI: Add pci_stop_and_remove_root_bus() Yinghai Lu
` (3 subsequent siblings)
7 siblings, 1 reply; 26+ messages in thread
From: Yinghai Lu @ 2012-09-27 8:11 UTC (permalink / raw)
To: Bjorn Helgaas, Len Brown, Taku Izumi, Jiang Liu
Cc: linux-pci, linux-acpi, Yinghai Lu
After we get hot-added ioapic registered.
pci_enable_bridges will try to enable ioapic irq for pci bridges.
Signed-off-by: Yinghai Lu <yinghai@kernel.org>
---
drivers/acpi/pci_root.c | 7 +++++++
1 files changed, 7 insertions(+), 0 deletions(-)
diff --git a/drivers/acpi/pci_root.c b/drivers/acpi/pci_root.c
index bce469c..27adbfd 100644
--- a/drivers/acpi/pci_root.c
+++ b/drivers/acpi/pci_root.c
@@ -644,12 +644,19 @@ static int acpi_pci_root_start(struct acpi_device *device)
struct acpi_pci_root *root = acpi_driver_data(device);
struct acpi_pci_driver *driver;
+ if (system_state != SYSTEM_BOOTING)
+ pci_assign_unassigned_bus_resources(root->bus);
+
mutex_lock(&acpi_pci_root_lock);
list_for_each_entry(driver, &acpi_pci_drivers, node)
if (driver->add)
driver->add(root);
mutex_unlock(&acpi_pci_root_lock);
+ /* need to after hot-added ioapic is registered */
+ if (system_state != SYSTEM_BOOTING)
+ pci_enable_bridges(root->bus);
+
pci_bus_add_devices(root->bus);
return 0;
--
1.7.7
^ permalink raw reply related [flat|nested] 26+ messages in thread* Re: [PATCH 4/8] PCI, ACPI: assign unassigned resource for hot add root bus
2012-09-27 8:11 ` [PATCH 4/8] PCI, ACPI: assign unassigned resource for hot add root bus Yinghai Lu
@ 2012-09-28 23:46 ` Bjorn Helgaas
2012-09-29 1:56 ` Yinghai Lu
0 siblings, 1 reply; 26+ messages in thread
From: Bjorn Helgaas @ 2012-09-28 23:46 UTC (permalink / raw)
To: Yinghai Lu; +Cc: Len Brown, Taku Izumi, Jiang Liu, linux-pci, linux-acpi
On Thu, Sep 27, 2012 at 2:11 AM, Yinghai Lu <yinghai@kernel.org> wrote:
> After we get hot-added ioapic registered.
> pci_enable_bridges will try to enable ioapic irq for pci bridges.
What makes this specifically related to IOAPICs?
> Signed-off-by: Yinghai Lu <yinghai@kernel.org>
> ---
> drivers/acpi/pci_root.c | 7 +++++++
> 1 files changed, 7 insertions(+), 0 deletions(-)
>
> diff --git a/drivers/acpi/pci_root.c b/drivers/acpi/pci_root.c
> index bce469c..27adbfd 100644
> --- a/drivers/acpi/pci_root.c
> +++ b/drivers/acpi/pci_root.c
> @@ -644,12 +644,19 @@ static int acpi_pci_root_start(struct acpi_device *device)
> struct acpi_pci_root *root = acpi_driver_data(device);
> struct acpi_pci_driver *driver;
>
> + if (system_state != SYSTEM_BOOTING)
> + pci_assign_unassigned_bus_resources(root->bus);
> +
> mutex_lock(&acpi_pci_root_lock);
> list_for_each_entry(driver, &acpi_pci_drivers, node)
> if (driver->add)
> driver->add(root);
> mutex_unlock(&acpi_pci_root_lock);
>
> + /* need to after hot-added ioapic is registered */
> + if (system_state != SYSTEM_BOOTING)
> + pci_enable_bridges(root->bus);
Theoretically, we should be able to assign resources and enable
bridges here regardless of the system_state.
I think the reason you don't want to do it while SYSTEM_BOOTING is
because we currently do this at boot-time:
acpi_pci_root_add
pci_scan_child_bus
acpi_pci_root_start
pci_bus_add_devices
<account for some motherboard (PNP0C01) resources>
pcibios_assign_resources # fs_initcall
pci_assign_unassigned_resources
pci_enable_bridges
and without the SYSTEM_BOOTING check, we might assign PCI resources at
boot-time that conflict with motherboard resources.
Is that right?
This is completely non-obvious and future readers deserve a hint about
what's going on here.
The right way to do this would be to pay attention to the host bridge
apertures, and assign resources from within the apertures. Then we
could always assign PCI resources when adding a host bridge instead of
in an fs_initcall. I understand we still have legacy issues and
machines where we still don't pay attention to _CRS. But if we're
doing things to work around a broken design, it's important to be
aware of how the design is broken so we have some hope of eventually
fixing the design.
> pci_bus_add_devices(root->bus);
>
> return 0;
> --
> 1.7.7
>
^ permalink raw reply [flat|nested] 26+ messages in thread* Re: [PATCH 4/8] PCI, ACPI: assign unassigned resource for hot add root bus
2012-09-28 23:46 ` Bjorn Helgaas
@ 2012-09-29 1:56 ` Yinghai Lu
2012-09-29 3:31 ` Bjorn Helgaas
0 siblings, 1 reply; 26+ messages in thread
From: Yinghai Lu @ 2012-09-29 1:56 UTC (permalink / raw)
To: Bjorn Helgaas; +Cc: Len Brown, Taku Izumi, Jiang Liu, linux-pci, linux-acpi
On Fri, Sep 28, 2012 at 4:46 PM, Bjorn Helgaas <bhelgaas@google.com> wrote:
> On Thu, Sep 27, 2012 at 2:11 AM, Yinghai Lu <yinghai@kernel.org> wrote:
>> After we get hot-added ioapic registered.
>> pci_enable_bridges will try to enable ioapic irq for pci bridges.
>
> What makes this specifically related to IOAPICs?
>
>> Signed-off-by: Yinghai Lu <yinghai@kernel.org>
>> ---
>> drivers/acpi/pci_root.c | 7 +++++++
>> 1 files changed, 7 insertions(+), 0 deletions(-)
>>
>> diff --git a/drivers/acpi/pci_root.c b/drivers/acpi/pci_root.c
>> index bce469c..27adbfd 100644
>> --- a/drivers/acpi/pci_root.c
>> +++ b/drivers/acpi/pci_root.c
>> @@ -644,12 +644,19 @@ static int acpi_pci_root_start(struct acpi_device *device)
>> struct acpi_pci_root *root = acpi_driver_data(device);
>> struct acpi_pci_driver *driver;
>>
>> + if (system_state != SYSTEM_BOOTING)
>> + pci_assign_unassigned_bus_resources(root->bus);
>> +
>> mutex_lock(&acpi_pci_root_lock);
>> list_for_each_entry(driver, &acpi_pci_drivers, node)
>> if (driver->add)
>> driver->add(root);
>> mutex_unlock(&acpi_pci_root_lock);
>>
>> + /* need to after hot-added ioapic is registered */
>> + if (system_state != SYSTEM_BOOTING)
>> + pci_enable_bridges(root->bus);
>
> Theoretically, we should be able to assign resources and enable
> bridges here regardless of the system_state.
>
> I think the reason you don't want to do it while SYSTEM_BOOTING is
> because we currently do this at boot-time:
>
> acpi_pci_root_add
> pci_scan_child_bus
> acpi_pci_root_start
> pci_bus_add_devices
> <account for some motherboard (PNP0C01) resources>
> pcibios_assign_resources # fs_initcall
> pci_assign_unassigned_resources
> pci_enable_bridges
>
> and without the SYSTEM_BOOTING check, we might assign PCI resources at
> boot-time that conflict with motherboard resources.
>
> Is that right?
yes.
>
> This is completely non-obvious and future readers deserve a hint about
> what's going on here.
>
> The right way to do this would be to pay attention to the host bridge
> apertures, and assign resources from within the apertures. Then we
> could always assign PCI resources when adding a host bridge instead of
> in an fs_initcall. I understand we still have legacy issues and
> machines where we still don't pay attention to _CRS. But if we're
> doing things to work around a broken design, it's important to be
> aware of how the design is broken so we have some hope of eventually
> fixing the design.
that could be another big topic.
^ permalink raw reply [flat|nested] 26+ messages in thread
* Re: [PATCH 4/8] PCI, ACPI: assign unassigned resource for hot add root bus
2012-09-29 1:56 ` Yinghai Lu
@ 2012-09-29 3:31 ` Bjorn Helgaas
2012-09-29 3:37 ` Jiang Liu
2012-09-29 4:08 ` Yinghai Lu
0 siblings, 2 replies; 26+ messages in thread
From: Bjorn Helgaas @ 2012-09-29 3:31 UTC (permalink / raw)
To: Yinghai Lu; +Cc: Len Brown, Taku Izumi, Jiang Liu, linux-pci, linux-acpi
On Fri, Sep 28, 2012 at 7:56 PM, Yinghai Lu <yinghai@kernel.org> wrote:
> On Fri, Sep 28, 2012 at 4:46 PM, Bjorn Helgaas <bhelgaas@google.com> wrote:
>> On Thu, Sep 27, 2012 at 2:11 AM, Yinghai Lu <yinghai@kernel.org> wrote:
>>> After we get hot-added ioapic registered.
>>> pci_enable_bridges will try to enable ioapic irq for pci bridges.
>>
>> What makes this specifically related to IOAPICs?
I think you missed this question.
>>> Signed-off-by: Yinghai Lu <yinghai@kernel.org>
>>> ---
>>> drivers/acpi/pci_root.c | 7 +++++++
>>> 1 files changed, 7 insertions(+), 0 deletions(-)
>>>
>>> diff --git a/drivers/acpi/pci_root.c b/drivers/acpi/pci_root.c
>>> index bce469c..27adbfd 100644
>>> --- a/drivers/acpi/pci_root.c
>>> +++ b/drivers/acpi/pci_root.c
>>> @@ -644,12 +644,19 @@ static int acpi_pci_root_start(struct acpi_device *device)
>>> struct acpi_pci_root *root = acpi_driver_data(device);
>>> struct acpi_pci_driver *driver;
>>>
>>> + if (system_state != SYSTEM_BOOTING)
>>> + pci_assign_unassigned_bus_resources(root->bus);
>>> +
>>> mutex_lock(&acpi_pci_root_lock);
>>> list_for_each_entry(driver, &acpi_pci_drivers, node)
>>> if (driver->add)
>>> driver->add(root);
>>> mutex_unlock(&acpi_pci_root_lock);
>>>
>>> + /* need to after hot-added ioapic is registered */
>>> + if (system_state != SYSTEM_BOOTING)
>>> + pci_enable_bridges(root->bus);
>>
>> Theoretically, we should be able to assign resources and enable
>> bridges here regardless of the system_state.
>>
>> I think the reason you don't want to do it while SYSTEM_BOOTING is
>> because we currently do this at boot-time:
>>
>> acpi_pci_root_add
>> pci_scan_child_bus
>> acpi_pci_root_start
>> pci_bus_add_devices
>> <account for some motherboard (PNP0C01) resources>
>> pcibios_assign_resources # fs_initcall
>> pci_assign_unassigned_resources
>> pci_enable_bridges
>>
>> and without the SYSTEM_BOOTING check, we might assign PCI resources at
>> boot-time that conflict with motherboard resources.
>>
>> Is that right?
>
> yes.
>
>>
>> This is completely non-obvious and future readers deserve a hint about
>> what's going on here.
>>
>> The right way to do this would be to pay attention to the host bridge
>> apertures, and assign resources from within the apertures. Then we
>> could always assign PCI resources when adding a host bridge instead of
>> in an fs_initcall. I understand we still have legacy issues and
>> machines where we still don't pay attention to _CRS. But if we're
>> doing things to work around a broken design, it's important to be
>> aware of how the design is broken so we have some hope of eventually
>> fixing the design.
>
> that could be another big topic.
I agree, and I'm not asking you to fix that. I'm just trying to
understand what's going on and write some comments and changelogs that
will help make this maintainable in the future.
Also note the question at the top -- your changelog calls out IOAPICs,
but I'm still not sure what this patch has to do with IOAPICs.
^ permalink raw reply [flat|nested] 26+ messages in thread
* Re: [PATCH 4/8] PCI, ACPI: assign unassigned resource for hot add root bus
2012-09-29 3:31 ` Bjorn Helgaas
@ 2012-09-29 3:37 ` Jiang Liu
2012-09-29 4:19 ` Yinghai Lu
2012-09-29 4:08 ` Yinghai Lu
1 sibling, 1 reply; 26+ messages in thread
From: Jiang Liu @ 2012-09-29 3:37 UTC (permalink / raw)
To: Bjorn Helgaas; +Cc: Yinghai Lu, Len Brown, Taku Izumi, linux-pci, linux-acpi
On 2012-9-29 11:31, Bjorn Helgaas wrote:
> On Fri, Sep 28, 2012 at 7:56 PM, Yinghai Lu <yinghai@kernel.org> wrote:
>> On Fri, Sep 28, 2012 at 4:46 PM, Bjorn Helgaas <bhelgaas@google.com> wrote:
>>> On Thu, Sep 27, 2012 at 2:11 AM, Yinghai Lu <yinghai@kernel.org> wrote:
>>>> After we get hot-added ioapic registered.
>>>> pci_enable_bridges will try to enable ioapic irq for pci bridges.
>>>
>>> What makes this specifically related to IOAPICs?
>
> I think you missed this question.
>
>>>> Signed-off-by: Yinghai Lu <yinghai@kernel.org>
>>>> ---
>>>> drivers/acpi/pci_root.c | 7 +++++++
>>>> 1 files changed, 7 insertions(+), 0 deletions(-)
>>>>
>>>> diff --git a/drivers/acpi/pci_root.c b/drivers/acpi/pci_root.c
>>>> index bce469c..27adbfd 100644
>>>> --- a/drivers/acpi/pci_root.c
>>>> +++ b/drivers/acpi/pci_root.c
>>>> @@ -644,12 +644,19 @@ static int acpi_pci_root_start(struct acpi_device *device)
>>>> struct acpi_pci_root *root = acpi_driver_data(device);
>>>> struct acpi_pci_driver *driver;
>>>>
>>>> + if (system_state != SYSTEM_BOOTING)
>>>> + pci_assign_unassigned_bus_resources(root->bus);
>>>> +
>>>> mutex_lock(&acpi_pci_root_lock);
>>>> list_for_each_entry(driver, &acpi_pci_drivers, node)
>>>> if (driver->add)
>>>> driver->add(root);
>>>> mutex_unlock(&acpi_pci_root_lock);
>>>>
>>>> + /* need to after hot-added ioapic is registered */
>>>> + if (system_state != SYSTEM_BOOTING)
>>>> + pci_enable_bridges(root->bus);
>>>
>>> Theoretically, we should be able to assign resources and enable
>>> bridges here regardless of the system_state.
>>>
>>> I think the reason you don't want to do it while SYSTEM_BOOTING is
>>> because we currently do this at boot-time:
>>>
>>> acpi_pci_root_add
>>> pci_scan_child_bus
>>> acpi_pci_root_start
>>> pci_bus_add_devices
>>> <account for some motherboard (PNP0C01) resources>
>>> pcibios_assign_resources # fs_initcall
>>> pci_assign_unassigned_resources
>>> pci_enable_bridges
>>>
>>> and without the SYSTEM_BOOTING check, we might assign PCI resources at
>>> boot-time that conflict with motherboard resources.
>>>
>>> Is that right?
>>
>> yes.
>>
>>>
>>> This is completely non-obvious and future readers deserve a hint about
>>> what's going on here.
>>>
>>> The right way to do this would be to pay attention to the host bridge
>>> apertures, and assign resources from within the apertures. Then we
>>> could always assign PCI resources when adding a host bridge instead of
>>> in an fs_initcall. I understand we still have legacy issues and
>>> machines where we still don't pay attention to _CRS. But if we're
>>> doing things to work around a broken design, it's important to be
>>> aware of how the design is broken so we have some hope of eventually
>>> fixing the design.
>>
>> that could be another big topic.
>
> I agree, and I'm not asking you to fix that. I'm just trying to
> understand what's going on and write some comments and changelogs that
> will help make this maintainable in the future.
>
> Also note the question at the top -- your changelog calls out IOAPICs,
> but I'm still not sure what this patch has to do with IOAPICs.
Hi Bjorn，
Another weay is to add an ACPI driver for IOAPIC, so we can control
the order between IOAPIC and PCI host bridge.
Thanks!
Gerry
^ permalink raw reply [flat|nested] 26+ messages in thread
* Re: [PATCH 4/8] PCI, ACPI: assign unassigned resource for hot add root bus
2012-09-29 3:37 ` Jiang Liu
@ 2012-09-29 4:19 ` Yinghai Lu
0 siblings, 0 replies; 26+ messages in thread
From: Yinghai Lu @ 2012-09-29 4:19 UTC (permalink / raw)
To: Jiang Liu; +Cc: Bjorn Helgaas, Len Brown, Taku Izumi, linux-pci, linux-acpi
On Fri, Sep 28, 2012 at 8:37 PM, Jiang Liu <jiang.liu@huawei.com> wrote:
> On 2012-9-29 11:31, Bjorn Helgaas wrote:
>> I agree, and I'm not asking you to fix that. I'm just trying to
>> understand what's going on and write some comments and changelogs that
>> will help make this maintainable in the future.
>>
>> Also note the question at the top -- your changelog calls out IOAPICs,
>> but I'm still not sure what this patch has to do with IOAPICs.
> Hi Bjorn，
> Another weay is to add an ACPI driver for IOAPIC, so we can control
> the order between IOAPIC and PCI host bridge.
use acpi_driver instead of acpi_pci_driver ?
^ permalink raw reply [flat|nested] 26+ messages in thread
* Re: [PATCH 4/8] PCI, ACPI: assign unassigned resource for hot add root bus
2012-09-29 3:31 ` Bjorn Helgaas
2012-09-29 3:37 ` Jiang Liu
@ 2012-09-29 4:08 ` Yinghai Lu
1 sibling, 0 replies; 26+ messages in thread
From: Yinghai Lu @ 2012-09-29 4:08 UTC (permalink / raw)
To: Bjorn Helgaas; +Cc: Len Brown, Taku Izumi, Jiang Liu, linux-pci, linux-acpi
On Fri, Sep 28, 2012 at 8:31 PM, Bjorn Helgaas <bhelgaas@google.com> wrote:
> On Fri, Sep 28, 2012 at 7:56 PM, Yinghai Lu <yinghai@kernel.org> wrote:
>> On Fri, Sep 28, 2012 at 4:46 PM, Bjorn Helgaas <bhelgaas@google.com> wrote:
>>> On Thu, Sep 27, 2012 at 2:11 AM, Yinghai Lu <yinghai@kernel.org> wrote:
>>>> After we get hot-added ioapic registered.
>>>> pci_enable_bridges will try to enable ioapic irq for pci bridges.
>>>
>>> What makes this specifically related to IOAPICs?
>
> I think you missed this question.
in another response:
pci_enable_bridges
==> pci_enable_device
==> ...
===> pcibios_enable_device
===> pcibios_enable_irq
and one of driver in acpi_pci_drivers, is ioapic drivers and it will
enable ioapic there.
So we need to enable bridges that later.
^ permalink raw reply [flat|nested] 26+ messages in thread
* [PATCH 5/8] PCI: Add pci_stop_and_remove_root_bus()
2012-09-27 8:11 [PATCH 0/8] PCI, ACPI, x86: pci root bus hotplug support pci_root.c related core changes Yinghai Lu
` (3 preceding siblings ...)
2012-09-27 8:11 ` [PATCH 4/8] PCI, ACPI: assign unassigned resource for hot add root bus Yinghai Lu
@ 2012-09-27 8:11 ` Yinghai Lu
2012-09-28 23:46 ` Bjorn Helgaas
2012-09-27 8:11 ` [PATCH 6/8] PCI, ACPI: Make acpi_pci_root_remove stop/remove pci root bus Yinghai Lu
` (2 subsequent siblings)
7 siblings, 1 reply; 26+ messages in thread
From: Yinghai Lu @ 2012-09-27 8:11 UTC (permalink / raw)
To: Bjorn Helgaas, Len Brown, Taku Izumi, Jiang Liu
Cc: linux-pci, linux-acpi, Yinghai Lu
It supports both pci root bus and pci bus under pci bridge.
-v2: clear pci_bridge's subordinate.
-v3: only handle root bus. and also put Jiang's get/put pair in
-v4: fold pci_stop/remove_bus_devices in... reducing confusing.
-v5: split device_register/unregister to avoid extra get...
also remove extra blank line.
Signed-off-by: Yinghai Lu <yinghai@kernel.org>
---
drivers/pci/remove.c | 36 ++++++++++++++++++++++++++++++++++++
include/linux/pci.h | 2 ++
2 files changed, 38 insertions(+), 0 deletions(-)
diff --git a/drivers/pci/remove.c b/drivers/pci/remove.c
index 513972f..7c0fd92 100644
--- a/drivers/pci/remove.c
+++ b/drivers/pci/remove.c
@@ -111,3 +111,39 @@ void pci_stop_and_remove_bus_device(struct pci_dev *dev)
pci_remove_bus_device(dev);
}
EXPORT_SYMBOL(pci_stop_and_remove_bus_device);
+
+void pci_stop_root_bus(struct pci_bus *bus)
+{
+ struct pci_dev *child, *tmp;
+ struct pci_host_bridge *host_bridge;
+
+ if (!pci_is_root_bus(bus))
+ return;
+
+ host_bridge = to_pci_host_bridge(bus->bridge);
+ list_for_each_entry_safe_reverse(child, tmp,
+ &bus->devices, bus_list)
+ pci_stop_bus_device(child);
+
+ /* stop the host bridge */
+ device_del(&host_bridge->dev);
+}
+
+void pci_remove_root_bus(struct pci_bus *bus)
+{
+ struct pci_dev *child, *tmp;
+ struct pci_host_bridge *host_bridge;
+
+ if (!pci_is_root_bus(bus))
+ return;
+
+ host_bridge = to_pci_host_bridge(bus->bridge);
+ list_for_each_entry_safe(child, tmp,
+ &bus->devices, bus_list)
+ pci_remove_bus_device(child);
+ pci_remove_bus(bus);
+ host_bridge->bus = NULL;
+
+ /* remove the host bridge */
+ put_device(&host_bridge->dev);
+}
diff --git a/include/linux/pci.h b/include/linux/pci.h
index 505c05a..a5cd03b 100644
--- a/include/linux/pci.h
+++ b/include/linux/pci.h
@@ -734,6 +734,8 @@ extern struct pci_dev *pci_dev_get(struct pci_dev *dev);
extern void pci_dev_put(struct pci_dev *dev);
extern void pci_remove_bus(struct pci_bus *b);
extern void pci_stop_and_remove_bus_device(struct pci_dev *dev);
+void pci_stop_root_bus(struct pci_bus *bus);
+void pci_remove_root_bus(struct pci_bus *bus);
void pci_setup_cardbus(struct pci_bus *bus);
extern void pci_sort_breadthfirst(void);
#define dev_is_pci(d) ((d)->bus == &pci_bus_type)
--
1.7.7
^ permalink raw reply related [flat|nested] 26+ messages in thread* Re: [PATCH 5/8] PCI: Add pci_stop_and_remove_root_bus()
2012-09-27 8:11 ` [PATCH 5/8] PCI: Add pci_stop_and_remove_root_bus() Yinghai Lu
@ 2012-09-28 23:46 ` Bjorn Helgaas
2012-09-29 2:05 ` Yinghai Lu
0 siblings, 1 reply; 26+ messages in thread
From: Bjorn Helgaas @ 2012-09-28 23:46 UTC (permalink / raw)
To: Yinghai Lu; +Cc: Len Brown, Taku Izumi, Jiang Liu, linux-pci, linux-acpi
On Thu, Sep 27, 2012 at 2:11 AM, Yinghai Lu <yinghai@kernel.org> wrote:
> It supports both pci root bus and pci bus under pci bridge.
>
> -v2: clear pci_bridge's subordinate.
> -v3: only handle root bus. and also put Jiang's get/put pair in
> -v4: fold pci_stop/remove_bus_devices in... reducing confusing.
> -v5: split device_register/unregister to avoid extra get...
> also remove extra blank line.
>
> Signed-off-by: Yinghai Lu <yinghai@kernel.org>
> ---
> drivers/pci/remove.c | 36 ++++++++++++++++++++++++++++++++++++
> include/linux/pci.h | 2 ++
> 2 files changed, 38 insertions(+), 0 deletions(-)
>
> diff --git a/drivers/pci/remove.c b/drivers/pci/remove.c
> index 513972f..7c0fd92 100644
> --- a/drivers/pci/remove.c
> +++ b/drivers/pci/remove.c
> @@ -111,3 +111,39 @@ void pci_stop_and_remove_bus_device(struct pci_dev *dev)
> pci_remove_bus_device(dev);
> }
> EXPORT_SYMBOL(pci_stop_and_remove_bus_device);
> +
> +void pci_stop_root_bus(struct pci_bus *bus)
> +{
> + struct pci_dev *child, *tmp;
> + struct pci_host_bridge *host_bridge;
> +
> + if (!pci_is_root_bus(bus))
> + return;
> +
> + host_bridge = to_pci_host_bridge(bus->bridge);
What if we made these functions just take a "struct pci_host_bridge *"
directly instead of a "struct pci_bus *"? Then the caller
(acpi_pci_root_remove()) could just look up the pci_host_bridge
pointer itself, or even keep that pointer in struct acpi_pci_root
instead of keeping the pci_bus pointer.
> + list_for_each_entry_safe_reverse(child, tmp,
> + &bus->devices, bus_list)
> + pci_stop_bus_device(child);
> +
> + /* stop the host bridge */
> + device_del(&host_bridge->dev);
> +}
> +
> +void pci_remove_root_bus(struct pci_bus *bus)
> +{
> + struct pci_dev *child, *tmp;
> + struct pci_host_bridge *host_bridge;
> +
> + if (!pci_is_root_bus(bus))
> + return;
> +
> + host_bridge = to_pci_host_bridge(bus->bridge);
> + list_for_each_entry_safe(child, tmp,
> + &bus->devices, bus_list)
> + pci_remove_bus_device(child);
> + pci_remove_bus(bus);
> + host_bridge->bus = NULL;
> +
> + /* remove the host bridge */
> + put_device(&host_bridge->dev);
> +}
> diff --git a/include/linux/pci.h b/include/linux/pci.h
> index 505c05a..a5cd03b 100644
> --- a/include/linux/pci.h
> +++ b/include/linux/pci.h
> @@ -734,6 +734,8 @@ extern struct pci_dev *pci_dev_get(struct pci_dev *dev);
> extern void pci_dev_put(struct pci_dev *dev);
> extern void pci_remove_bus(struct pci_bus *b);
> extern void pci_stop_and_remove_bus_device(struct pci_dev *dev);
> +void pci_stop_root_bus(struct pci_bus *bus);
> +void pci_remove_root_bus(struct pci_bus *bus);
> void pci_setup_cardbus(struct pci_bus *bus);
> extern void pci_sort_breadthfirst(void);
> #define dev_is_pci(d) ((d)->bus == &pci_bus_type)
> --
> 1.7.7
>
^ permalink raw reply [flat|nested] 26+ messages in thread* Re: [PATCH 5/8] PCI: Add pci_stop_and_remove_root_bus()
2012-09-28 23:46 ` Bjorn Helgaas
@ 2012-09-29 2:05 ` Yinghai Lu
0 siblings, 0 replies; 26+ messages in thread
From: Yinghai Lu @ 2012-09-29 2:05 UTC (permalink / raw)
To: Bjorn Helgaas; +Cc: Len Brown, Taku Izumi, Jiang Liu, linux-pci, linux-acpi
On Fri, Sep 28, 2012 at 4:46 PM, Bjorn Helgaas <bhelgaas@google.com> wrote:
> On Thu, Sep 27, 2012 at 2:11 AM, Yinghai Lu <yinghai@kernel.org> wrote:
>> It supports both pci root bus and pci bus under pci bridge.
>>
>> -v2: clear pci_bridge's subordinate.
>> -v3: only handle root bus. and also put Jiang's get/put pair in
>> -v4: fold pci_stop/remove_bus_devices in... reducing confusing.
>> -v5: split device_register/unregister to avoid extra get...
>> also remove extra blank line.
>>
>> Signed-off-by: Yinghai Lu <yinghai@kernel.org>
>> ---
>> drivers/pci/remove.c | 36 ++++++++++++++++++++++++++++++++++++
>> include/linux/pci.h | 2 ++
>> 2 files changed, 38 insertions(+), 0 deletions(-)
>>
>> diff --git a/drivers/pci/remove.c b/drivers/pci/remove.c
>> index 513972f..7c0fd92 100644
>> --- a/drivers/pci/remove.c
>> +++ b/drivers/pci/remove.c
>> @@ -111,3 +111,39 @@ void pci_stop_and_remove_bus_device(struct pci_dev *dev)
>> pci_remove_bus_device(dev);
>> }
>> EXPORT_SYMBOL(pci_stop_and_remove_bus_device);
>> +
>> +void pci_stop_root_bus(struct pci_bus *bus)
>> +{
>> + struct pci_dev *child, *tmp;
>> + struct pci_host_bridge *host_bridge;
>> +
>> + if (!pci_is_root_bus(bus))
>> + return;
>> +
>> + host_bridge = to_pci_host_bridge(bus->bridge);
>
> What if we made these functions just take a "struct pci_host_bridge *"
> directly instead of a "struct pci_bus *"? Then the caller
> (acpi_pci_root_remove()) could just look up the pci_host_bridge
> pointer itself,
yes, that could save pci_is_root_bus checking.
> or even keep that pointer in struct acpi_pci_root
> instead of keeping the pci_bus pointer.
that could be a lot of change. could be next one stage.
actually acpi root is bound to pci host bridge instead of root bus.
so it is more reasonable to save host_bridge pointer.
we also can think in another way.
host_bridge and pci bus should be one part.
we may could just embed pci bus into pci_host_bridge instead.
-Yinghai
^ permalink raw reply [flat|nested] 26+ messages in thread
* [PATCH 6/8] PCI, ACPI: Make acpi_pci_root_remove stop/remove pci root bus
2012-09-27 8:11 [PATCH 0/8] PCI, ACPI, x86: pci root bus hotplug support pci_root.c related core changes Yinghai Lu
` (4 preceding siblings ...)
2012-09-27 8:11 ` [PATCH 5/8] PCI: Add pci_stop_and_remove_root_bus() Yinghai Lu
@ 2012-09-27 8:11 ` Yinghai Lu
2012-09-27 8:11 ` [PATCH 7/8] PCI, ACPI: delete root bus prt during hot remove path Yinghai Lu
2012-09-27 8:11 ` [PATCH 8/8] PCI, ACPI: remove acpi_root_driver in reserse order Yinghai Lu
7 siblings, 0 replies; 26+ messages in thread
From: Yinghai Lu @ 2012-09-27 8:11 UTC (permalink / raw)
To: Bjorn Helgaas, Len Brown, Taku Izumi, Jiang Liu
Cc: linux-pci, linux-acpi, Yinghai Lu
It will call new added pci_stop_and_remove_bus() to stop/remove pci root bus.
Signed-off-by: Yinghai Lu <yinghai@kernel.org>
Cc: Len Brown <lenb@kernel.org>
Cc: linux-acpi@vger.kernel.org
---
drivers/acpi/pci_root.c | 6 ++++++
1 files changed, 6 insertions(+), 0 deletions(-)
diff --git a/drivers/acpi/pci_root.c b/drivers/acpi/pci_root.c
index 27adbfd..aed0953 100644
--- a/drivers/acpi/pci_root.c
+++ b/drivers/acpi/pci_root.c
@@ -667,14 +667,20 @@ static int acpi_pci_root_remove(struct acpi_device *device, int type)
struct acpi_pci_root *root = acpi_driver_data(device);
struct acpi_pci_driver *driver;
+ pci_stop_root_bus(root->bus);
+
mutex_lock(&acpi_pci_root_lock);
list_for_each_entry(driver, &acpi_pci_drivers, node)
if (driver->remove)
driver->remove(root);
+ mutex_unlock(&acpi_pci_root_lock);
device_set_run_wake(root->bus->bridge, false);
pci_acpi_remove_bus_pm_notifier(device);
+ pci_remove_root_bus(root->bus);
+
+ mutex_lock(&acpi_pci_root_lock);
list_del(&root->node);
mutex_unlock(&acpi_pci_root_lock);
kfree(root);
--
1.7.7
^ permalink raw reply related [flat|nested] 26+ messages in thread* [PATCH 7/8] PCI, ACPI: delete root bus prt during hot remove path
2012-09-27 8:11 [PATCH 0/8] PCI, ACPI, x86: pci root bus hotplug support pci_root.c related core changes Yinghai Lu
` (5 preceding siblings ...)
2012-09-27 8:11 ` [PATCH 6/8] PCI, ACPI: Make acpi_pci_root_remove stop/remove pci root bus Yinghai Lu
@ 2012-09-27 8:11 ` Yinghai Lu
2012-09-27 8:11 ` [PATCH 8/8] PCI, ACPI: remove acpi_root_driver in reserse order Yinghai Lu
7 siblings, 0 replies; 26+ messages in thread
From: Yinghai Lu @ 2012-09-27 8:11 UTC (permalink / raw)
To: Bjorn Helgaas, Len Brown, Taku Izumi, Jiang Liu
Cc: linux-pci, linux-acpi, Yinghai Lu
corresponding to add path.
Signed-off-by: Yinghai Lu <yinghai@kernel.org>
---
drivers/acpi/pci_root.c | 6 ++++++
1 files changed, 6 insertions(+), 0 deletions(-)
diff --git a/drivers/acpi/pci_root.c b/drivers/acpi/pci_root.c
index aed0953..a27cbb57 100644
--- a/drivers/acpi/pci_root.c
+++ b/drivers/acpi/pci_root.c
@@ -664,6 +664,8 @@ static int acpi_pci_root_start(struct acpi_device *device)
static int acpi_pci_root_remove(struct acpi_device *device, int type)
{
+ acpi_status status;
+ acpi_handle handle;
struct acpi_pci_root *root = acpi_driver_data(device);
struct acpi_pci_driver *driver;
@@ -678,6 +680,10 @@ static int acpi_pci_root_remove(struct acpi_device *device, int type)
device_set_run_wake(root->bus->bridge, false);
pci_acpi_remove_bus_pm_notifier(device);
+ status = acpi_get_handle(device->handle, METHOD_NAME__PRT, &handle);
+ if (ACPI_SUCCESS(status))
+ acpi_pci_irq_del_prt(root->bus);
+
pci_remove_root_bus(root->bus);
mutex_lock(&acpi_pci_root_lock);
--
1.7.7
^ permalink raw reply related [flat|nested] 26+ messages in thread* [PATCH 8/8] PCI, ACPI: remove acpi_root_driver in reserse order
2012-09-27 8:11 [PATCH 0/8] PCI, ACPI, x86: pci root bus hotplug support pci_root.c related core changes Yinghai Lu
` (6 preceding siblings ...)
2012-09-27 8:11 ` [PATCH 7/8] PCI, ACPI: delete root bus prt during hot remove path Yinghai Lu
@ 2012-09-27 8:11 ` Yinghai Lu
2012-09-28 23:46 ` Bjorn Helgaas
7 siblings, 1 reply; 26+ messages in thread
From: Yinghai Lu @ 2012-09-27 8:11 UTC (permalink / raw)
To: Bjorn Helgaas, Len Brown, Taku Izumi, Jiang Liu
Cc: linux-pci, linux-acpi, Yinghai Lu
aka stop acpi root driver in this sequence: acpiphp, iommu, ioapic.
Signed-off-by: Yinghai Lu <yinghai@kernel.org>
---
drivers/acpi/pci_root.c | 2 +-
1 files changed, 1 insertions(+), 1 deletions(-)
diff --git a/drivers/acpi/pci_root.c b/drivers/acpi/pci_root.c
index a27cbb57..012f40d 100644
--- a/drivers/acpi/pci_root.c
+++ b/drivers/acpi/pci_root.c
@@ -672,7 +672,7 @@ static int acpi_pci_root_remove(struct acpi_device *device, int type)
pci_stop_root_bus(root->bus);
mutex_lock(&acpi_pci_root_lock);
- list_for_each_entry(driver, &acpi_pci_drivers, node)
+ list_for_each_entry_reverse(driver, &acpi_pci_drivers, node)
if (driver->remove)
driver->remove(root);
mutex_unlock(&acpi_pci_root_lock);
--
1.7.7
^ permalink raw reply related [flat|nested] 26+ messages in thread* Re: [PATCH 8/8] PCI, ACPI: remove acpi_root_driver in reserse order
2012-09-27 8:11 ` [PATCH 8/8] PCI, ACPI: remove acpi_root_driver in reserse order Yinghai Lu
@ 2012-09-28 23:46 ` Bjorn Helgaas
2012-09-29 2:09 ` Yinghai Lu
0 siblings, 1 reply; 26+ messages in thread
From: Bjorn Helgaas @ 2012-09-28 23:46 UTC (permalink / raw)
To: Yinghai Lu; +Cc: Len Brown, Taku Izumi, Jiang Liu, linux-pci, linux-acpi
On Thu, Sep 27, 2012 at 2:11 AM, Yinghai Lu <yinghai@kernel.org> wrote:
> aka stop acpi root driver in this sequence: acpiphp, iommu, ioapic.
>
> Signed-off-by: Yinghai Lu <yinghai@kernel.org>
> ---
> drivers/acpi/pci_root.c | 2 +-
> 1 files changed, 1 insertions(+), 1 deletions(-)
>
> diff --git a/drivers/acpi/pci_root.c b/drivers/acpi/pci_root.c
> index a27cbb57..012f40d 100644
> --- a/drivers/acpi/pci_root.c
> +++ b/drivers/acpi/pci_root.c
> @@ -672,7 +672,7 @@ static int acpi_pci_root_remove(struct acpi_device *device, int type)
> pci_stop_root_bus(root->bus);
>
> mutex_lock(&acpi_pci_root_lock);
> - list_for_each_entry(driver, &acpi_pci_drivers, node)
> + list_for_each_entry_reverse(driver, &acpi_pci_drivers, node)
> if (driver->remove)
> driver->remove(root);
This seems sort of OK just because we call the ->add() methods in
forward order, so it is symmetric to call the ->remove() methods in
reverse order.
But I'm uncomfortable with using this as a way to enforce ->remove()
ordering across acpiphp, iommu, and ioapic. That seems fragile.
What are the dependencies between these drivers that require this ordering?
> mutex_unlock(&acpi_pci_root_lock);
> --
> 1.7.7
>
^ permalink raw reply [flat|nested] 26+ messages in thread
* Re: [PATCH 8/8] PCI, ACPI: remove acpi_root_driver in reserse order
2012-09-28 23:46 ` Bjorn Helgaas
@ 2012-09-29 2:09 ` Yinghai Lu
0 siblings, 0 replies; 26+ messages in thread
From: Yinghai Lu @ 2012-09-29 2:09 UTC (permalink / raw)
To: Bjorn Helgaas; +Cc: Len Brown, Taku Izumi, Jiang Liu, linux-pci, linux-acpi
On Fri, Sep 28, 2012 at 4:46 PM, Bjorn Helgaas <bhelgaas@google.com> wrote:
> On Thu, Sep 27, 2012 at 2:11 AM, Yinghai Lu <yinghai@kernel.org> wrote:
>> aka stop acpi root driver in this sequence: acpiphp, iommu, ioapic.
>>
>> Signed-off-by: Yinghai Lu <yinghai@kernel.org>
>> ---
>> drivers/acpi/pci_root.c | 2 +-
>> 1 files changed, 1 insertions(+), 1 deletions(-)
>>
>> diff --git a/drivers/acpi/pci_root.c b/drivers/acpi/pci_root.c
>> index a27cbb57..012f40d 100644
>> --- a/drivers/acpi/pci_root.c
>> +++ b/drivers/acpi/pci_root.c
>> @@ -672,7 +672,7 @@ static int acpi_pci_root_remove(struct acpi_device *device, int type)
>> pci_stop_root_bus(root->bus);
>>
>> mutex_lock(&acpi_pci_root_lock);
>> - list_for_each_entry(driver, &acpi_pci_drivers, node)
>> + list_for_each_entry_reverse(driver, &acpi_pci_drivers, node)
>> if (driver->remove)
>> driver->remove(root);
>
> This seems sort of OK just because we call the ->add() methods in
> forward order, so it is symmetric to call the ->remove() methods in
> reverse order.
>
> But I'm uncomfortable with using this as a way to enforce ->remove()
> ordering across acpiphp, iommu, and ioapic. That seems fragile.
>
> What are the dependencies between these drivers that require this ordering?
ioapic should come at first, and iommu and last is acpiphp.
iommu will check if intr-mapping is corresponding to ioapic controller number.
also dmar controller will need one normal irq without intr-remapping
for error handling.
need to use link sequence in Makefile to make sure iommu come at first.
-Yinghai
^ permalink raw reply [flat|nested] 26+ messages in thread