* [PATCH v2 1/9] PCI: make PCI device create/destroy logic symmetric
2012-09-15 3:05 [PATCH v2 0/9] enhance PCI related drivers to handle hotplug events Jiang Liu
@ 2012-09-15 3:05 ` Jiang Liu
2012-09-15 3:05 ` [PATCH v2 2/9] PCI: split registration of PCI bus devices into two stages Jiang Liu
` (7 subsequent siblings)
8 siblings, 0 replies; 26+ messages in thread
From: Jiang Liu @ 2012-09-15 3:05 UTC (permalink / raw)
To: Bjorn Helgaas
Cc: Tony Luck, Jiang Liu, Yinghai Lu, Kenji Kaneshige, Yijing Wang,
Jiang Liu, linux-kernel, linux-pci
According to device model documentation, the way to create/destroy PCI
devices should be symmetric.
/**
* device_del - delete device from system.
* @dev: device.
*
* This is the first part of the device unregistration
* sequence. This removes the device from the lists we control
* from here, has it removed from the other driver model
* subsystems it was added to in device_add(), and removes it
* from the kobject hierarchy.
*
* NOTE: this should be called manually _iff_ device_add() was
* also called manually.
*/
The rule is to either use
1) device_register()/device_unregister()
or
2) device_initialize()/device_add()/device_del()/put_device().
So change PCI core logic to follow the rule and get rid of the redundant
pci_dev_get()/pci_dev_put() pair.
Signed-off-by: Jiang Liu <jiang.liu@huawei.com>
Acked-by: Bjorn Helgaas <bhelgaas@google.com>
---
drivers/pci/probe.c | 1 -
drivers/pci/remove.c | 4 ++--
2 files changed, 2 insertions(+), 3 deletions(-)
diff --git a/drivers/pci/probe.c b/drivers/pci/probe.c
index 2396111..5dbad03 100644
--- a/drivers/pci/probe.c
+++ b/drivers/pci/probe.c
@@ -1293,7 +1293,6 @@ void pci_device_add(struct pci_dev *dev, struct pci_bus *bus)
{
device_initialize(&dev->dev);
dev->dev.release = pci_release_dev;
- pci_dev_get(dev);
dev->dev.dma_mask = &dev->dma_mask;
dev->dev.dma_parms = &dev->dma_parms;
diff --git a/drivers/pci/remove.c b/drivers/pci/remove.c
index 4f9ca91..37b9407 100644
--- a/drivers/pci/remove.c
+++ b/drivers/pci/remove.c
@@ -22,7 +22,7 @@ static void pci_stop_dev(struct pci_dev *dev)
if (dev->is_added) {
pci_proc_detach_device(dev);
pci_remove_sysfs_dev_files(dev);
- device_unregister(&dev->dev);
+ device_del(&dev->dev);
dev->is_added = 0;
}
@@ -37,7 +37,7 @@ static void pci_destroy_dev(struct pci_dev *dev)
up_write(&pci_bus_sem);
pci_free_resources(dev);
- pci_dev_put(dev);
+ put_device(&dev->dev);
}
void pci_remove_bus(struct pci_bus *bus)
--
1.7.9.5
^ permalink raw reply related [flat|nested] 26+ messages in thread* [PATCH v2 2/9] PCI: split registration of PCI bus devices into two stages
2012-09-15 3:05 [PATCH v2 0/9] enhance PCI related drivers to handle hotplug events Jiang Liu
2012-09-15 3:05 ` [PATCH v2 1/9] PCI: make PCI device create/destroy logic symmetric Jiang Liu
@ 2012-09-15 3:05 ` Jiang Liu
2012-09-15 8:03 ` Yinghai Lu
2012-09-15 3:05 ` [PATCH v2 3/9] PCI: preserve dev->subordinate until pci_stop_dev() has been called Jiang Liu
` (6 subsequent siblings)
8 siblings, 1 reply; 26+ messages in thread
From: Jiang Liu @ 2012-09-15 3:05 UTC (permalink / raw)
To: Bjorn Helgaas
Cc: Tony Luck, Jiang Liu, Yinghai Lu, Kenji Kaneshige, Yijing Wang,
Jiang Liu, linux-kernel, linux-pci
When handling BUS_NOTIFY_ADD_DEVICE event for a PCI bridge device,
the notification handler can't hold reference count to the new PCI bus
because the device object for the new bus (pci_dev->subordinate->dev)
hasn't been initialized yet.
Split the registration of PCI bus device into two stages as below,
so that the event handler could hold reference count to the new PCI bus
when handling BUS_NOTIFY_ADD_DEVICE event.
1) device_initialize(&pci_dev->dev)
2) device_initialize(&pci_dev->subordinate->dev)
3) notify BUS_NOTIFY_ADD_DEVICE event for pci_dev
4) device_add(&pci_dev->dev)
5) device_add(&pci_dev->subordinate->dev)
Signed-off-by: Jiang Liu <jiang.liu@huawei.com>
---
drivers/pci/bus.c | 2 +-
drivers/pci/probe.c | 3 ++-
drivers/pci/remove.c | 10 +++++-----
3 files changed, 8 insertions(+), 7 deletions(-)
diff --git a/drivers/pci/bus.c b/drivers/pci/bus.c
index 4b0970b..11a5c28 100644
--- a/drivers/pci/bus.c
+++ b/drivers/pci/bus.c
@@ -189,7 +189,7 @@ int pci_bus_add_child(struct pci_bus *bus)
if (bus->bridge)
bus->dev.parent = bus->bridge;
- retval = device_register(&bus->dev);
+ retval = device_add(&bus->dev);
if (retval)
return retval;
diff --git a/drivers/pci/probe.c b/drivers/pci/probe.c
index 5dbad03..ddc8f7f 100644
--- a/drivers/pci/probe.c
+++ b/drivers/pci/probe.c
@@ -464,6 +464,7 @@ static struct pci_bus * pci_alloc_bus(void)
INIT_LIST_HEAD(&b->resources);
b->max_bus_speed = PCI_SPEED_UNKNOWN;
b->cur_bus_speed = PCI_SPEED_UNKNOWN;
+ device_initialize(&b->dev);
}
return b;
}
@@ -1672,7 +1673,7 @@ struct pci_bus *pci_create_root_bus(struct device *parent, int bus,
b->dev.class = &pcibus_class;
b->dev.parent = b->bridge;
dev_set_name(&b->dev, "%04x:%02x", pci_domain_nr(b), bus);
- error = device_register(&b->dev);
+ error = device_add(&b->dev);
if (error)
goto class_dev_reg_err;
diff --git a/drivers/pci/remove.c b/drivers/pci/remove.c
index 37b9407..86a4636 100644
--- a/drivers/pci/remove.c
+++ b/drivers/pci/remove.c
@@ -48,11 +48,11 @@ void pci_remove_bus(struct pci_bus *bus)
list_del(&bus->node);
pci_bus_release_busn_res(bus);
up_write(&pci_bus_sem);
- if (!bus->is_added)
- return;
-
- pci_remove_legacy_files(bus);
- device_unregister(&bus->dev);
+ if (bus->is_added) {
+ pci_remove_legacy_files(bus);
+ device_del(&bus->dev);
+ }
+ put_device(&bus->dev);
}
EXPORT_SYMBOL(pci_remove_bus);
--
1.7.9.5
^ permalink raw reply related [flat|nested] 26+ messages in thread* Re: [PATCH v2 2/9] PCI: split registration of PCI bus devices into two stages
2012-09-15 3:05 ` [PATCH v2 2/9] PCI: split registration of PCI bus devices into two stages Jiang Liu
@ 2012-09-15 8:03 ` Yinghai Lu
2012-09-15 18:47 ` Yinghai Lu
0 siblings, 1 reply; 26+ messages in thread
From: Yinghai Lu @ 2012-09-15 8:03 UTC (permalink / raw)
To: Jiang Liu
Cc: Bjorn Helgaas, Tony Luck, Jiang Liu, Kenji Kaneshige,
Yijing Wang, linux-kernel, linux-pci
On Fri, Sep 14, 2012 at 8:05 PM, Jiang Liu <liuj97@gmail.com> wrote:
> When handling BUS_NOTIFY_ADD_DEVICE event for a PCI bridge device,
> the notification handler can't hold reference count to the new PCI bus
> because the device object for the new bus (pci_dev->subordinate->dev)
> hasn't been initialized yet.
>
> Split the registration of PCI bus device into two stages as below,
> so that the event handler could hold reference count to the new PCI bus
> when handling BUS_NOTIFY_ADD_DEVICE event.
>
> 1) device_initialize(&pci_dev->dev)
> 2) device_initialize(&pci_dev->subordinate->dev)
> 3) notify BUS_NOTIFY_ADD_DEVICE event for pci_dev
> 4) device_add(&pci_dev->dev)
> 5) device_add(&pci_dev->subordinate->dev)
>
> Signed-off-by: Jiang Liu <jiang.liu@huawei.com>
> ---
> drivers/pci/bus.c | 2 +-
> drivers/pci/probe.c | 3 ++-
> drivers/pci/remove.c | 10 +++++-----
> 3 files changed, 8 insertions(+), 7 deletions(-)
>
> diff --git a/drivers/pci/bus.c b/drivers/pci/bus.c
> index 4b0970b..11a5c28 100644
> --- a/drivers/pci/bus.c
> +++ b/drivers/pci/bus.c
> @@ -189,7 +189,7 @@ int pci_bus_add_child(struct pci_bus *bus)
> if (bus->bridge)
> bus->dev.parent = bus->bridge;
>
> - retval = device_register(&bus->dev);
> + retval = device_add(&bus->dev);
> if (retval)
> return retval;
>
> diff --git a/drivers/pci/probe.c b/drivers/pci/probe.c
> index 5dbad03..ddc8f7f 100644
> --- a/drivers/pci/probe.c
> +++ b/drivers/pci/probe.c
> @@ -464,6 +464,7 @@ static struct pci_bus * pci_alloc_bus(void)
> INIT_LIST_HEAD(&b->resources);
> b->max_bus_speed = PCI_SPEED_UNKNOWN;
> b->cur_bus_speed = PCI_SPEED_UNKNOWN;
> + device_initialize(&b->dev);
> }
> return b;
> }
> @@ -1672,7 +1673,7 @@ struct pci_bus *pci_create_root_bus(struct device *parent, int bus,
> b->dev.class = &pcibus_class;
> b->dev.parent = b->bridge;
> dev_set_name(&b->dev, "%04x:%02x", pci_domain_nr(b), bus);
> - error = device_register(&b->dev);
> + error = device_add(&b->dev);
> if (error)
> goto class_dev_reg_err;
failing path in pci_create_root_bus is not handled.
you could try to move
device_initialize(&b->dev);
to
pci_add_new_bus and pci_create_root_bus instead.
>
> diff --git a/drivers/pci/remove.c b/drivers/pci/remove.c
> index 37b9407..86a4636 100644
> --- a/drivers/pci/remove.c
> +++ b/drivers/pci/remove.c
> @@ -48,11 +48,11 @@ void pci_remove_bus(struct pci_bus *bus)
> list_del(&bus->node);
> pci_bus_release_busn_res(bus);
> up_write(&pci_bus_sem);
> - if (!bus->is_added)
> - return;
> -
> - pci_remove_legacy_files(bus);
> - device_unregister(&bus->dev);
> + if (bus->is_added) {
> + pci_remove_legacy_files(bus);
> + device_del(&bus->dev);
> + }
> + put_device(&bus->dev);
> }
> EXPORT_SYMBOL(pci_remove_bus);
>
> --
> 1.7.9.5
>
^ permalink raw reply [flat|nested] 26+ messages in thread* Re: [PATCH v2 2/9] PCI: split registration of PCI bus devices into two stages
2012-09-15 8:03 ` Yinghai Lu
@ 2012-09-15 18:47 ` Yinghai Lu
2012-09-16 13:16 ` [PATCH v3] " Jiang Liu
0 siblings, 1 reply; 26+ messages in thread
From: Yinghai Lu @ 2012-09-15 18:47 UTC (permalink / raw)
To: Jiang Liu
Cc: Bjorn Helgaas, Tony Luck, Jiang Liu, Kenji Kaneshige,
Yijing Wang, linux-kernel, linux-pci
[-- Attachment #1: Type: text/plain, Size: 347 bytes --]
On Sat, Sep 15, 2012 at 1:03 AM, Yinghai Lu <yinghai@kernel.org> wrote:
> On Fri, Sep 14, 2012 at 8:05 PM, Jiang Liu <liuj97@gmail.com> wrote:
>
> failing path in pci_create_root_bus is not handled.
>
> you could try to move
>
> device_initialize(&b->dev);
>
> to
> pci_add_new_bus and pci_create_root_bus instead.
sth like attached...
[-- Attachment #2: jiang_hotplug_rework_2.patch --]
[-- Type: application/octet-stream, Size: 6513 bytes --]
Delivered-To: yhlu.kernel@gmail.com
Received: by 10.205.37.138 with SMTP id te10csp185645bkb;
Fri, 14 Sep 2012 20:13:03 -0700 (PDT)
Received: by 10.68.203.230 with SMTP id kt6mr7970238pbc.163.1347678782659;
Fri, 14 Sep 2012 20:13:02 -0700 (PDT)
Return-Path: <liuj97@gmail.com>
Received: from mail.kernel.org (mail.kernel.org. [198.145.19.201])
by mx.google.com with ESMTP id vn9si5410909pbc.317.2012.09.14.20.13.01;
Fri, 14 Sep 2012 20:13:02 -0700 (PDT)
Received-SPF: neutral (google.com: 198.145.19.201 is neither permitted nor denied by domain of liuj97@gmail.com) client-ip=198.145.19.201;
Authentication-Results: mx.google.com; spf=neutral (google.com: 198.145.19.201 is neither permitted nor denied by domain of liuj97@gmail.com) smtp.mail=liuj97@gmail.com; dkim=pass header.i=@gmail.com
Received: by mail.kernel.org (Postfix)
id C785B20172; Sat, 15 Sep 2012 03:13:01 +0000 (UTC)
Delivered-To: yinghai@kernel.org
Received: from mail.kernel.org (localhost [127.0.0.1])
by mail.kernel.org (Postfix) with ESMTP id 24CED2017B
for <yinghai@kernel.org>; Sat, 15 Sep 2012 03:13:01 +0000 (UTC)
Received: from mail-pb0-f44.google.com (mail-pb0-f44.google.com [209.85.160.44])
by mail.kernel.org (Postfix) with ESMTP id A7ACE20172
for <yinghai@kernel.org>; Sat, 15 Sep 2012 03:12:56 +0000 (UTC)
Received: by mail-pb0-f44.google.com with SMTP id rr4so6641777pbb.3
for <yinghai@kernel.org>; Fri, 14 Sep 2012 20:12:56 -0700 (PDT)
DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed;
d=gmail.com; s=20120113;
h=from:to:cc:subject:date:message-id:x-mailer:in-reply-to:references;
bh=7rI/5ErxmvnHqd6Af99njMHdZQSxcg1oSVXxgmJquvs=;
b=Wle4FFXnXo4hq0F3+7hDY+JVZQKwYc0/TW1xcAOF7doqwbf6rGkLcoYUzPQF+oJU1u
dDORpq5BCeVhi/LWROGltmJUZ4FAzjRJzc/3s/YFIb01K5PtXqchf1GgHUgnlLvywL0Z
yarvhCwUWL9WTqd4h4ahVB4dOp00E9ygJDuYkmqe0R7oQuw8YmUxMVqeSLQLzIC3t2/U
6L97tQ6GauyqrChnzyrmXNLcC9Hij4GaquWUXWzz71KxCejEWRPrvzFEjeAAOZ+TIeU8
b+QI4m1eC2OV3zODrNtZK7B/0W3SswGvXN6bVZ147VYv74nrzXP4WgRH6MN3XbaBlj/p
Q+mA==
Received: by 10.66.90.38 with SMTP id bt6mr6924001pab.53.1347678776513;
Fri, 14 Sep 2012 20:12:56 -0700 (PDT)
Received: from localhost.localdomain ([221.221.18.122])
by mx.google.com with ESMTPS id jz10sm2092777pbc.8.2012.09.14.20.12.43
(version=TLSv1/SSLv3 cipher=OTHER);
Fri, 14 Sep 2012 20:12:49 -0700 (PDT)
From: Jiang Liu <liuj97@gmail.com>
To: Bjorn Helgaas <bhelgaas@google.com>
Cc: Tony Luck <tony.luck@intel.com>,
Jiang Liu <jiang.liu@huawei.com>,
Yinghai Lu <yinghai@kernel.org>,
Kenji Kaneshige <kaneshige.kenji@jp.fujitsu.com>,
Yijing Wang <wangyijing@huawei.com>,
Jiang Liu <liuj97@gmail.com>,
linux-kernel@vger.kernel.org,
linux-pci@vger.kernel.org
Subject: [PATCH v2 2/9] PCI: split registration of PCI bus devices into two stages
Date: Sat, 15 Sep 2012 11:05:05 +0800
Message-Id: <1347678312-11124-3-git-send-email-jiang.liu@huawei.com>
X-Mailer: git-send-email 1.7.9.5
In-Reply-To: <1347678312-11124-1-git-send-email-jiang.liu@huawei.com>
References: <1347678312-11124-1-git-send-email-jiang.liu@huawei.com>
X-Spam-Status: No, score=-2.4 required=5.0 tests=BAYES_00,DKIM_SIGNED,
DKIM_VALID,DKIM_VALID_AU,FREEMAIL_ENVFROM_END_DIGIT,FREEMAIL_FROM,
RCVD_IN_DNSWL_LOW,UNPARSEABLE_RELAY autolearn=ham version=3.3.1
X-Spam-Checker-Version: SpamAssassin 3.3.1 (2010-03-16) on mail.kernel.org
X-Virus-Scanned: ClamAV using ClamSMTP
When handling BUS_NOTIFY_ADD_DEVICE event for a PCI bridge device,
the notification handler can't hold reference count to the new PCI bus
because the device object for the new bus (pci_dev->subordinate->dev)
hasn't been initialized yet.
Split the registration of PCI bus device into two stages as below,
so that the event handler could hold reference count to the new PCI bus
when handling BUS_NOTIFY_ADD_DEVICE event.
1) device_initialize(&pci_dev->dev)
2) device_initialize(&pci_dev->subordinate->dev)
3) notify BUS_NOTIFY_ADD_DEVICE event for pci_dev
4) device_add(&pci_dev->dev)
5) device_add(&pci_dev->subordinate->dev)
Signed-off-by: Jiang Liu <jiang.liu@huawei.com>
---
drivers/pci/bus.c | 2 +-
drivers/pci/probe.c | 4 +++-
drivers/pci/remove.c | 10 +++++-----
3 files changed, 9 insertions(+), 7 deletions(-)
Index: linux-2.6/drivers/pci/bus.c
===================================================================
--- linux-2.6.orig/drivers/pci/bus.c
+++ linux-2.6/drivers/pci/bus.c
@@ -193,7 +193,7 @@ int pci_bus_add_child(struct pci_bus *bu
if (bus->bridge)
bus->dev.parent = bus->bridge;
- retval = device_register(&bus->dev);
+ retval = device_add(&bus->dev);
if (retval)
return retval;
Index: linux-2.6/drivers/pci/probe.c
===================================================================
--- linux-2.6.orig/drivers/pci/probe.c
+++ linux-2.6/drivers/pci/probe.c
@@ -637,6 +637,7 @@ static struct pci_bus *pci_alloc_child_b
* now as the parent is not properly set up yet. This device will get
* registered later in pci_bus_add_devices()
*/
+ device_initialize(&child->dev);
child->dev.class = &pcibus_class;
dev_set_name(&child->dev, "%04x:%02x", pci_domain_nr(child), busnr);
@@ -1669,10 +1670,11 @@ struct pci_bus *pci_create_root_bus(stru
if (!parent)
set_dev_node(b->bridge, pcibus_to_node(b));
+ device_initialize(&b->dev);
b->dev.class = &pcibus_class;
b->dev.parent = b->bridge;
dev_set_name(&b->dev, "%04x:%02x", pci_domain_nr(b), bus);
- error = device_register(&b->dev);
+ error = device_add(&b->dev);
if (error)
goto class_dev_reg_err;
Index: linux-2.6/drivers/pci/remove.c
===================================================================
--- linux-2.6.orig/drivers/pci/remove.c
+++ linux-2.6/drivers/pci/remove.c
@@ -48,11 +48,11 @@ void pci_remove_bus(struct pci_bus *bus)
list_del(&bus->node);
pci_bus_release_busn_res(bus);
up_write(&pci_bus_sem);
- if (!bus->is_added)
- return;
-
- pci_remove_legacy_files(bus);
- device_unregister(&bus->dev);
+ if (bus->is_added) {
+ pci_remove_legacy_files(bus);
+ device_del(&bus->dev);
+ }
+ put_device(&bus->dev);
}
EXPORT_SYMBOL(pci_remove_bus);
^ permalink raw reply [flat|nested] 26+ messages in thread* [PATCH v3] PCI: split registration of PCI bus devices into two stages
2012-09-15 18:47 ` Yinghai Lu
@ 2012-09-16 13:16 ` Jiang Liu
0 siblings, 0 replies; 26+ messages in thread
From: Jiang Liu @ 2012-09-16 13:16 UTC (permalink / raw)
To: Bjorn Helgaas
Cc: Jiang Liu, Yinghai Lu, Kenji Kaneshige, Yijing Wang, Jiang Liu,
linux-kernel, linux-pci
When handling BUS_NOTIFY_ADD_DEVICE event for a PCI bridge device,
the notification handler can't hold reference count to the new PCI bus
because the device object for the new bus (pci_dev->subordinate->dev)
hasn't been initialized yet.
Split the registration of PCI bus device into two stages as below,
so that the event handler could hold reference count to the new PCI bus
when handling BUS_NOTIFY_ADD_DEVICE event.
1) device_initialize(&pci_dev->dev)
2) device_initialize(&pci_dev->subordinate->dev)
3) notify BUS_NOTIFY_ADD_DEVICE event for pci_dev
4) device_add(&pci_dev->dev)
5) device_add(&pci_dev->subordinate->dev)
Signed-off-by: Jiang Liu <jiang.liu@huawei.com>
Signed-off-by: Yinghai Lu <yinghai@kernel.org>
---
V2->V3
Fix a failing in error recover path according to Yinghai's suggestion.
---
drivers/pci/bus.c | 2 +-
drivers/pci/probe.c | 4 +++-
drivers/pci/remove.c | 10 +++++-----
3 files changed, 9 insertions(+), 7 deletions(-)
diff --git a/drivers/pci/bus.c b/drivers/pci/bus.c
index 4b0970b..11a5c28 100644
--- a/drivers/pci/bus.c
+++ b/drivers/pci/bus.c
@@ -189,7 +189,7 @@ int pci_bus_add_child(struct pci_bus *bus)
if (bus->bridge)
bus->dev.parent = bus->bridge;
- retval = device_register(&bus->dev);
+ retval = device_add(&bus->dev);
if (retval)
return retval;
diff --git a/drivers/pci/probe.c b/drivers/pci/probe.c
index 5dbad03..7ef8c1b 100644
--- a/drivers/pci/probe.c
+++ b/drivers/pci/probe.c
@@ -639,6 +639,7 @@ static struct pci_bus *pci_alloc_child_bus(struct pci_bus *parent,
*/
child->dev.class = &pcibus_class;
dev_set_name(&child->dev, "%04x:%02x", pci_domain_nr(child), busnr);
+ device_initialize(&child->dev);
/*
* Set up the primary, secondary and subordinate
@@ -1672,7 +1673,8 @@ struct pci_bus *pci_create_root_bus(struct device *parent, int bus,
b->dev.class = &pcibus_class;
b->dev.parent = b->bridge;
dev_set_name(&b->dev, "%04x:%02x", pci_domain_nr(b), bus);
- error = device_register(&b->dev);
+ device_initialize(&b->dev);
+ error = device_add(&b->dev);
if (error)
goto class_dev_reg_err;
diff --git a/drivers/pci/remove.c b/drivers/pci/remove.c
index 37b9407..86a4636 100644
--- a/drivers/pci/remove.c
+++ b/drivers/pci/remove.c
@@ -48,11 +48,11 @@ void pci_remove_bus(struct pci_bus *bus)
list_del(&bus->node);
pci_bus_release_busn_res(bus);
up_write(&pci_bus_sem);
- if (!bus->is_added)
- return;
-
- pci_remove_legacy_files(bus);
- device_unregister(&bus->dev);
+ if (bus->is_added) {
+ pci_remove_legacy_files(bus);
+ device_del(&bus->dev);
+ }
+ put_device(&bus->dev);
}
EXPORT_SYMBOL(pci_remove_bus);
--
1.7.9.5
^ permalink raw reply related [flat|nested] 26+ messages in thread
* [PATCH v2 3/9] PCI: preserve dev->subordinate until pci_stop_dev() has been called
2012-09-15 3:05 [PATCH v2 0/9] enhance PCI related drivers to handle hotplug events Jiang Liu
2012-09-15 3:05 ` [PATCH v2 1/9] PCI: make PCI device create/destroy logic symmetric Jiang Liu
2012-09-15 3:05 ` [PATCH v2 2/9] PCI: split registration of PCI bus devices into two stages Jiang Liu
@ 2012-09-15 3:05 ` Jiang Liu
2012-09-15 5:09 ` Yinghai Lu
2012-09-15 3:05 ` [PATCH v2 4/9] ACPI/pci_bind: correctly update binding relationship for PCI hotplug Jiang Liu
` (5 subsequent siblings)
8 siblings, 1 reply; 26+ messages in thread
From: Jiang Liu @ 2012-09-15 3:05 UTC (permalink / raw)
To: Bjorn Helgaas
Cc: Tony Luck, Yijing Wang, Yinghai Lu, Kenji Kaneshige, Jiang Liu,
linux-kernel, linux-pci, Jiang Liu
From: Yijing Wang <wangyijing@huawei.com>
Changeset 2ed168eeb3edec029aa0eca5cb981d6376f931f9 "PCI: Fold stop and
remove helpers into their callers" has changed the behavior when
removing a PCI device.
Previously, for a PCI bridge device with secondary bus, dev->subordinate
is valid when calling PCI bus notification callbacks for
BUS_NOTIFY_ADD_DEVICE/BUS_NOTIFY_DEL_DEVICE events. Now dev->subordinate
has been reset to NULL when calling callbacks for BUS_NOTIFY_DEL_DEVICE
events, which may break some PCI bus notification callbacks.
So revert to the original behavior to keep dev->subordinate valid for
both BUS_NOTIFY_ADD_DEVICE events and BUS_NOTIFY_DEL_DEVICE events.
Signed-off-by: Yijing Wang <wangyijing@huawei.com>
Signed-off-by: Jiang Liu <jiang.liu@huawei.com>
---
drivers/pci/remove.c | 6 +++---
1 file changed, 3 insertions(+), 3 deletions(-)
diff --git a/drivers/pci/remove.c b/drivers/pci/remove.c
index 86a4636..6244956 100644
--- a/drivers/pci/remove.c
+++ b/drivers/pci/remove.c
@@ -79,16 +79,16 @@ void pci_stop_and_remove_bus_device(struct pci_dev *dev)
* iterator. Therefore, iterate in reverse so we remove the VFs
* first, then the PF.
*/
- if (bus) {
+ if (bus)
list_for_each_entry_safe_reverse(child, tmp,
&bus->devices, bus_list)
pci_stop_and_remove_bus_device(child);
+ pci_stop_dev(dev);
+ if (bus) {
pci_remove_bus(bus);
dev->subordinate = NULL;
}
-
- pci_stop_dev(dev);
pci_destroy_dev(dev);
}
EXPORT_SYMBOL(pci_stop_and_remove_bus_device);
--
1.7.9.5
^ permalink raw reply related [flat|nested] 26+ messages in thread* Re: [PATCH v2 3/9] PCI: preserve dev->subordinate until pci_stop_dev() has been called
2012-09-15 3:05 ` [PATCH v2 3/9] PCI: preserve dev->subordinate until pci_stop_dev() has been called Jiang Liu
@ 2012-09-15 5:09 ` Yinghai Lu
2012-09-15 7:02 ` Jiang Liu
0 siblings, 1 reply; 26+ messages in thread
From: Yinghai Lu @ 2012-09-15 5:09 UTC (permalink / raw)
To: Jiang Liu
Cc: Bjorn Helgaas, Tony Luck, Yijing Wang, Kenji Kaneshige,
linux-kernel, linux-pci, Jiang Liu
On Fri, Sep 14, 2012 at 8:05 PM, Jiang Liu <liuj97@gmail.com> wrote:
> From: Yijing Wang <wangyijing@huawei.com>
>
> Changeset 2ed168eeb3edec029aa0eca5cb981d6376f931f9 "PCI: Fold stop and
> remove helpers into their callers" has changed the behavior when
> removing a PCI device.
>
> Previously, for a PCI bridge device with secondary bus, dev->subordinate
> is valid when calling PCI bus notification callbacks for
> BUS_NOTIFY_ADD_DEVICE/BUS_NOTIFY_DEL_DEVICE events. Now dev->subordinate
> has been reset to NULL when calling callbacks for BUS_NOTIFY_DEL_DEVICE
> events, which may break some PCI bus notification callbacks.
>
> So revert to the original behavior to keep dev->subordinate valid for
> both BUS_NOTIFY_ADD_DEVICE events and BUS_NOTIFY_DEL_DEVICE events.
>
> Signed-off-by: Yijing Wang <wangyijing@huawei.com>
> Signed-off-by: Jiang Liu <jiang.liu@huawei.com>
> ---
> drivers/pci/remove.c | 6 +++---
> 1 file changed, 3 insertions(+), 3 deletions(-)
>
> diff --git a/drivers/pci/remove.c b/drivers/pci/remove.c
> index 86a4636..6244956 100644
> --- a/drivers/pci/remove.c
> +++ b/drivers/pci/remove.c
> @@ -79,16 +79,16 @@ void pci_stop_and_remove_bus_device(struct pci_dev *dev)
> * iterator. Therefore, iterate in reverse so we remove the VFs
> * first, then the PF.
> */
> - if (bus) {
> + if (bus)
> list_for_each_entry_safe_reverse(child, tmp,
> &bus->devices, bus_list)
> pci_stop_and_remove_bus_device(child);
> + pci_stop_dev(dev);
>
> + if (bus) {
> pci_remove_bus(bus);
> dev->subordinate = NULL;
> }
> -
> - pci_stop_dev(dev);
> pci_destroy_dev(dev);
> }
> EXPORT_SYMBOL(pci_stop_and_remove_bus_device);
for support root bus support with acpi_pci_root (ioapic/iommu) will
need pci_stop_bus_devices()
so I just put those function back...
http://git.kernel.org/?p=linux/kernel/git/yinghai/linux-yinghai.git;a=commitdiff;h=247a40206cf44488f21bc6074cf0ba2805d4d840
Thanks
Yinghai
^ permalink raw reply [flat|nested] 26+ messages in thread* Re: [PATCH v2 3/9] PCI: preserve dev->subordinate until pci_stop_dev() has been called
2012-09-15 5:09 ` Yinghai Lu
@ 2012-09-15 7:02 ` Jiang Liu
0 siblings, 0 replies; 26+ messages in thread
From: Jiang Liu @ 2012-09-15 7:02 UTC (permalink / raw)
To: Yinghai Lu
Cc: Bjorn Helgaas, Tony Luck, Yijing Wang, Kenji Kaneshige,
linux-kernel, linux-pci, Jiang Liu
On 09/15/2012 01:09 PM, Yinghai Lu wrote:
> On Fri, Sep 14, 2012 at 8:05 PM, Jiang Liu <liuj97@gmail.com> wrote:
>> From: Yijing Wang <wangyijing@huawei.com>
>>
>> Changeset 2ed168eeb3edec029aa0eca5cb981d6376f931f9 "PCI: Fold stop and
>> remove helpers into their callers" has changed the behavior when
>> removing a PCI device.
>>
>> Previously, for a PCI bridge device with secondary bus, dev->subordinate
>> is valid when calling PCI bus notification callbacks for
>> BUS_NOTIFY_ADD_DEVICE/BUS_NOTIFY_DEL_DEVICE events. Now dev->subordinate
>> has been reset to NULL when calling callbacks for BUS_NOTIFY_DEL_DEVICE
>> events, which may break some PCI bus notification callbacks.
>>
>> So revert to the original behavior to keep dev->subordinate valid for
>> both BUS_NOTIFY_ADD_DEVICE events and BUS_NOTIFY_DEL_DEVICE events.
>>
>> Signed-off-by: Yijing Wang <wangyijing@huawei.com>
>> Signed-off-by: Jiang Liu <jiang.liu@huawei.com>
>> ---
>> drivers/pci/remove.c | 6 +++---
>> 1 file changed, 3 insertions(+), 3 deletions(-)
>>
>> diff --git a/drivers/pci/remove.c b/drivers/pci/remove.c
>> index 86a4636..6244956 100644
>> --- a/drivers/pci/remove.c
>> +++ b/drivers/pci/remove.c
>> @@ -79,16 +79,16 @@ void pci_stop_and_remove_bus_device(struct pci_dev *dev)
>> * iterator. Therefore, iterate in reverse so we remove the VFs
>> * first, then the PF.
>> */
>> - if (bus) {
>> + if (bus)
>> list_for_each_entry_safe_reverse(child, tmp,
>> &bus->devices, bus_list)
>> pci_stop_and_remove_bus_device(child);
>> + pci_stop_dev(dev);
>>
>> + if (bus) {
>> pci_remove_bus(bus);
>> dev->subordinate = NULL;
>> }
>> -
>> - pci_stop_dev(dev);
>> pci_destroy_dev(dev);
>> }
>> EXPORT_SYMBOL(pci_stop_and_remove_bus_device);
>
> for support root bus support with acpi_pci_root (ioapic/iommu) will
> need pci_stop_bus_devices()
>
> so I just put those function back...
>
> http://git.kernel.org/?p=linux/kernel/git/yinghai/linux-yinghai.git;a=commitdiff;h=247a40206cf44488f21bc6074cf0ba2805d4d840
Hi Yinghai,
I will just drop this patch when merging with your pci root bus hotplug work.
--Gerry
^ permalink raw reply [flat|nested] 26+ messages in thread
* [PATCH v2 4/9] ACPI/pci_bind: correctly update binding relationship for PCI hotplug
2012-09-15 3:05 [PATCH v2 0/9] enhance PCI related drivers to handle hotplug events Jiang Liu
` (2 preceding siblings ...)
2012-09-15 3:05 ` [PATCH v2 3/9] PCI: preserve dev->subordinate until pci_stop_dev() has been called Jiang Liu
@ 2012-09-15 3:05 ` Jiang Liu
2012-09-15 3:05 ` [PATCH v2 5/9] ACPI/pci-bind: remove bind/unbind callbacks from acpi_device_ops Jiang Liu
` (4 subsequent siblings)
8 siblings, 0 replies; 26+ messages in thread
From: Jiang Liu @ 2012-09-15 3:05 UTC (permalink / raw)
To: Bjorn Helgaas, Len Brown
Cc: Tony Luck, Jiang Liu, Yinghai Lu, Kenji Kaneshige, Yijing Wang,
Jiang Liu, linux-kernel, linux-pci, linux-acpi
From: Jiang Liu <jiang.liu@huawei.com>
Currently pci_bind.c is used to maintain binding relationship between
ACPI and PCI devices. But it's broken when handling PCI hotplug events.
For the acpiphp driver, it's designed to update the binding relationship
when PCI hotplug event happens, but the implementation is broken.
For PCI device hot-adding:
enable_device()
pci_scan_slot()
acpiphp_bus_add()
acpi_bus_add()
acpi_pci_bind()
acpi_get_pci_dev()
return NULL because dev->archdata.acpi_handle is
still unset
return without updating actual binding relationship
pci_bus_add_devices()
pci_bus_add_device()
device_add()
platform_notify()
acpi_bind_one()
set dev->archdata.acpi_handle
For PCI device hot-removal,
disable_device()
pci_stop_bus_device()
__pci_remove_bus_device()
acpiphp_bus_trim()
acpi_bus_remove()
acpi_pci_unbind()
return without really unbinding because PCI device has
already been destroyed
For other PCI hotplug drivers, they even don't bother to update binding
relationships. So hook into acpi_bind_one()/acpi_unbind_one() in
drivers/acpi/glue.c to update PCI<->ACPI binding relationship.
Signed-off-by: Jiang Liu <jiang.liu@huawei.com>
Signed-off-by: Yijing Wang <wangyijing@huawei.com>
---
drivers/acpi/glue.c | 6 ++-
drivers/acpi/internal.h | 2 +
drivers/acpi/pci_bind.c | 101 +++++++++++++++++++++++++++++++----------------
drivers/acpi/pci_root.c | 40 +++++--------------
4 files changed, 81 insertions(+), 68 deletions(-)
diff --git a/drivers/acpi/glue.c b/drivers/acpi/glue.c
index 243ee85..4904700 100644
--- a/drivers/acpi/glue.c
+++ b/drivers/acpi/glue.c
@@ -121,7 +121,6 @@ acpi_handle acpi_get_child(acpi_handle parent, u64 address)
1, do_acpi_find_child, NULL, &find, NULL);
return find.handle;
}
-
EXPORT_SYMBOL(acpi_get_child);
/* Link ACPI devices with physical devices */
@@ -142,7 +141,6 @@ struct device *acpi_get_physical_device(acpi_handle handle)
return get_device(dev);
return NULL;
}
-
EXPORT_SYMBOL(acpi_get_physical_device);
static int acpi_bind_one(struct device *dev, acpi_handle handle)
@@ -162,6 +160,8 @@ static int acpi_bind_one(struct device *dev, acpi_handle handle)
}
dev->archdata.acpi_handle = handle;
+ acpi_pci_bind_notify(dev->archdata.acpi_handle, dev, true);
+
status = acpi_bus_get_device(handle, &acpi_dev);
if (!ACPI_FAILURE(status)) {
int ret;
@@ -193,6 +193,8 @@ static int acpi_unbind_one(struct device *dev)
sysfs_remove_link(&acpi_dev->dev.kobj, "physical_node");
}
+ acpi_pci_bind_notify(dev->archdata.acpi_handle, dev, false);
+
acpi_detach_data(dev->archdata.acpi_handle,
acpi_glue_data_handler);
dev->archdata.acpi_handle = NULL;
diff --git a/drivers/acpi/internal.h b/drivers/acpi/internal.h
index ca75b9c..5396b20 100644
--- a/drivers/acpi/internal.h
+++ b/drivers/acpi/internal.h
@@ -93,4 +93,6 @@ static inline int suspend_nvs_save(void) { return 0; }
static inline void suspend_nvs_restore(void) {}
#endif
+void acpi_pci_bind_notify(acpi_handle handle, struct device *dev, bool bind);
+
#endif /* _ACPI_INTERNAL_H_ */
diff --git a/drivers/acpi/pci_bind.c b/drivers/acpi/pci_bind.c
index 2ef0409..320e698 100644
--- a/drivers/acpi/pci_bind.c
+++ b/drivers/acpi/pci_bind.c
@@ -35,40 +35,41 @@
#define _COMPONENT ACPI_PCI_COMPONENT
ACPI_MODULE_NAME("pci_bind");
-static int acpi_pci_unbind(struct acpi_device *device)
-{
- struct pci_dev *dev;
-
- dev = acpi_get_pci_dev(device->handle);
- if (!dev)
- goto out;
+static int acpi_pci_bind_cb(struct acpi_device *device);
+static int acpi_pci_unbind(struct acpi_device *device, struct pci_dev *dev)
+{
device_set_run_wake(&dev->dev, false);
pci_acpi_remove_pm_notifier(device);
- if (!dev->subordinate)
- goto out;
+ if (dev->subordinate) {
+ acpi_pci_irq_del_prt(dev->subordinate);
+ device->ops.bind = NULL;
+ device->ops.unbind = NULL;
+ }
+
+ return 0;
+}
- acpi_pci_irq_del_prt(dev->subordinate);
+static int acpi_pci_unbind_cb(struct acpi_device *device)
+{
+ int rc = 0;
+ struct pci_dev *dev;
- device->ops.bind = NULL;
- device->ops.unbind = NULL;
+ dev = acpi_get_pci_dev(device->handle);
+ if (dev) {
+ rc = acpi_pci_unbind(device, dev);
+ pci_dev_put(dev);
+ }
-out:
- pci_dev_put(dev);
- return 0;
+ return rc;
}
-static int acpi_pci_bind(struct acpi_device *device)
+static int acpi_pci_bind(struct acpi_device *device, struct pci_dev *dev)
{
acpi_status status;
acpi_handle handle;
struct pci_bus *bus;
- struct pci_dev *dev;
-
- dev = acpi_get_pci_dev(device->handle);
- if (!dev)
- return 0;
pci_acpi_add_pm_notifier(device, dev);
if (device->wakeup.flags.run_wake)
@@ -83,8 +84,8 @@ static int acpi_pci_bind(struct acpi_device *device)
"Device %04x:%02x:%02x.%d is a PCI bridge\n",
pci_domain_nr(dev->bus), dev->bus->number,
PCI_SLOT(dev->devfn), PCI_FUNC(dev->devfn)));
- device->ops.bind = acpi_pci_bind;
- device->ops.unbind = acpi_pci_unbind;
+ device->ops.bind = acpi_pci_bind_cb;
+ device->ops.unbind = acpi_pci_unbind_cb;
}
/*
@@ -96,25 +97,55 @@ static int acpi_pci_bind(struct acpi_device *device)
* TBD: Can _PRTs exist within the scope of non-bridge PCI devices?
*/
status = acpi_get_handle(device->handle, METHOD_NAME__PRT, &handle);
- if (ACPI_FAILURE(status))
- goto out;
+ if (ACPI_SUCCESS(status)) {
+ if (dev->subordinate)
+ bus = dev->subordinate;
+ else
+ bus = dev->bus;
+ acpi_pci_irq_add_prt(device->handle, bus);
+ }
- if (dev->subordinate)
- bus = dev->subordinate;
- else
- bus = dev->bus;
+ return 0;
+}
- acpi_pci_irq_add_prt(device->handle, bus);
+static int acpi_pci_bind_cb(struct acpi_device *device)
+{
+ int rc = 0;
+ struct pci_dev *dev;
-out:
- pci_dev_put(dev);
- return 0;
+ dev = acpi_get_pci_dev(device->handle);
+ if (dev) {
+ rc = acpi_pci_bind(device, dev);
+ pci_dev_put(dev);
+ }
+
+ return rc;
}
int acpi_pci_bind_root(struct acpi_device *device)
{
- device->ops.bind = acpi_pci_bind;
- device->ops.unbind = acpi_pci_unbind;
+ device->ops.bind = acpi_pci_bind_cb;
+ device->ops.unbind = acpi_pci_unbind_cb;
return 0;
}
+
+void acpi_pci_bind_notify(acpi_handle handle, struct device *dev, bool bind)
+{
+ struct acpi_device *acpi_dev;
+
+ if (!dev_is_pci(dev))
+ return;
+ if (acpi_bus_get_device(handle, &acpi_dev) || !acpi_dev)
+ return;
+
+ if (acpi_dev->parent) {
+ if (bind) {
+ if (acpi_dev->parent->ops.bind)
+ acpi_pci_bind(acpi_dev, to_pci_dev(dev));
+ } else {
+ if (acpi_dev->parent->ops.unbind)
+ acpi_pci_unbind(acpi_dev, to_pci_dev(dev));
+ }
+ }
+}
diff --git a/drivers/acpi/pci_root.c b/drivers/acpi/pci_root.c
index 72a2c98..7509034 100644
--- a/drivers/acpi/pci_root.c
+++ b/drivers/acpi/pci_root.c
@@ -195,21 +195,6 @@ static acpi_status try_get_root_bridge_busnr(acpi_handle handle,
return AE_OK;
}
-static void acpi_pci_bridge_scan(struct acpi_device *device)
-{
- int status;
- struct acpi_device *child = NULL;
-
- if (device->flags.bus_address)
- if (device->parent && device->parent->ops.bind) {
- status = device->parent->ops.bind(device);
- if (!status) {
- list_for_each_entry(child, &device->children, node)
- acpi_pci_bridge_scan(child);
- }
- }
-}
-
static u8 pci_osc_uuid_str[] = "33DB4D5B-1FF7-401C-9657-7441C03DD766";
static acpi_status acpi_pci_run_osc(acpi_handle handle,
@@ -456,7 +441,6 @@ static int __devinit acpi_pci_root_add(struct acpi_device *device)
int result;
struct acpi_pci_root *root;
acpi_handle handle;
- struct acpi_device *child;
u32 flags, base_flags;
root = kzalloc(sizeof(struct acpi_pci_root), GFP_KERNEL);
@@ -521,6 +505,15 @@ static int __devinit acpi_pci_root_add(struct acpi_device *device)
/* TBD: Locking */
list_add_tail(&root->node, &acpi_pci_roots);
+ /*
+ * Attach ACPI-PCI Context
+ * -----------------------
+ * Thus binding the ACPI and PCI devices.
+ */
+ result = acpi_pci_bind_root(device);
+ if (result)
+ goto end;
+
printk(KERN_INFO PREFIX "%s [%s] (domain %04x %pR)\n",
acpi_device_name(device), acpi_device_bid(device),
root->segment, &root->secondary);
@@ -542,15 +535,6 @@ static int __devinit acpi_pci_root_add(struct acpi_device *device)
}
/*
- * Attach ACPI-PCI Context
- * -----------------------
- * Thus binding the ACPI and PCI devices.
- */
- result = acpi_pci_bind_root(device);
- if (result)
- goto end;
-
- /*
* PCI Routing Table
* -----------------
* Evaluate and parse _PRT, if exists.
@@ -559,12 +543,6 @@ static int __devinit acpi_pci_root_add(struct acpi_device *device)
if (ACPI_SUCCESS(status))
result = acpi_pci_irq_add_prt(device->handle, root->bus);
- /*
- * Scan and bind all _ADR-Based Devices
- */
- list_for_each_entry(child, &device->children, node)
- acpi_pci_bridge_scan(child);
-
/* Indicate support for various _OSC capabilities. */
if (pci_ext_cfg_avail(root->bus->self))
flags |= OSC_EXT_PCI_CONFIG_SUPPORT;
--
1.7.9.5
^ permalink raw reply related [flat|nested] 26+ messages in thread* [PATCH v2 5/9] ACPI/pci-bind: remove bind/unbind callbacks from acpi_device_ops
2012-09-15 3:05 [PATCH v2 0/9] enhance PCI related drivers to handle hotplug events Jiang Liu
` (3 preceding siblings ...)
2012-09-15 3:05 ` [PATCH v2 4/9] ACPI/pci_bind: correctly update binding relationship for PCI hotplug Jiang Liu
@ 2012-09-15 3:05 ` Jiang Liu
2012-09-15 18:53 ` Yinghai Lu
2012-09-15 23:27 ` [PATCH v2 5/9] " Yinghai Lu
2012-09-15 3:05 ` [PATCH v2 6/9] ACPI/pci_slot: update PCI slot information when PCI hotplug event happens Jiang Liu
` (3 subsequent siblings)
8 siblings, 2 replies; 26+ messages in thread
From: Jiang Liu @ 2012-09-15 3:05 UTC (permalink / raw)
To: Bjorn Helgaas, Len Brown
Cc: Tony Luck, Jiang Liu, Yinghai Lu, Kenji Kaneshige, Yijing Wang,
Jiang Liu, linux-kernel, linux-pci, linux-acpi
From: Jiang Liu <jiang.liu@huawei.com>
Now ACPI devices are created before/destroyed after corresponding PCI
devices, and acpi_platform_notify/acpi_platform_notify_remove will
update PCI<->ACPI binding relationship when creating/destroying PCI
devices, there's no need to invoke bind/unbind callbacks from ACPI
device probe/destroy routines anymore. So remove bind/unbind callbacks
from acpi_device_ops.
Signed-off-by: Jiang Liu <jiang.liu@huawei.com>
Signed-off-by: Yijing Wang <wangyijing@huawei.com>
---
drivers/acpi/pci_bind.c | 100 +++++++++----------------------------------
drivers/acpi/pci_root.c | 9 ----
drivers/acpi/scan.c | 21 +--------
include/acpi/acpi_bus.h | 4 --
include/acpi/acpi_drivers.h | 1 -
5 files changed, 23 insertions(+), 112 deletions(-)
diff --git a/drivers/acpi/pci_bind.c b/drivers/acpi/pci_bind.c
index 320e698..d18316a 100644
--- a/drivers/acpi/pci_bind.c
+++ b/drivers/acpi/pci_bind.c
@@ -35,57 +35,31 @@
#define _COMPONENT ACPI_PCI_COMPONENT
ACPI_MODULE_NAME("pci_bind");
-static int acpi_pci_bind_cb(struct acpi_device *device);
-
-static int acpi_pci_unbind(struct acpi_device *device, struct pci_dev *dev)
+static int acpi_pci_unbind(acpi_handle handle, struct pci_dev *dev)
{
- device_set_run_wake(&dev->dev, false);
- pci_acpi_remove_pm_notifier(device);
+ struct acpi_device *acpi_dev;
- if (dev->subordinate) {
- acpi_pci_irq_del_prt(dev->subordinate);
- device->ops.bind = NULL;
- device->ops.unbind = NULL;
+ if (!acpi_bus_get_device(handle, &acpi_dev) && acpi_dev) {
+ device_set_run_wake(&dev->dev, false);
+ pci_acpi_remove_pm_notifier(acpi_dev);
}
- return 0;
-}
-
-static int acpi_pci_unbind_cb(struct acpi_device *device)
-{
- int rc = 0;
- struct pci_dev *dev;
-
- dev = acpi_get_pci_dev(device->handle);
- if (dev) {
- rc = acpi_pci_unbind(device, dev);
- pci_dev_put(dev);
- }
+ if (dev->subordinate)
+ acpi_pci_irq_del_prt(dev->subordinate);
- return rc;
+ return 0;
}
-static int acpi_pci_bind(struct acpi_device *device, struct pci_dev *dev)
+static int acpi_pci_bind(acpi_handle handle, struct pci_dev *dev)
{
acpi_status status;
- acpi_handle handle;
struct pci_bus *bus;
+ struct acpi_device *acpi_dev;
- pci_acpi_add_pm_notifier(device, dev);
- if (device->wakeup.flags.run_wake)
- device_set_run_wake(&dev->dev, true);
-
- /*
- * Install the 'bind' function to facilitate callbacks for
- * children of the P2P bridge.
- */
- if (dev->subordinate) {
- ACPI_DEBUG_PRINT((ACPI_DB_INFO,
- "Device %04x:%02x:%02x.%d is a PCI bridge\n",
- pci_domain_nr(dev->bus), dev->bus->number,
- PCI_SLOT(dev->devfn), PCI_FUNC(dev->devfn)));
- device->ops.bind = acpi_pci_bind_cb;
- device->ops.unbind = acpi_pci_unbind_cb;
+ if (!acpi_bus_get_device(handle, &acpi_dev) && acpi_dev) {
+ pci_acpi_add_pm_notifier(acpi_dev, dev);
+ if (acpi_dev->wakeup.flags.run_wake)
+ device_set_run_wake(&dev->dev, true);
}
/*
@@ -96,56 +70,24 @@ static int acpi_pci_bind(struct acpi_device *device, struct pci_dev *dev)
*
* TBD: Can _PRTs exist within the scope of non-bridge PCI devices?
*/
- status = acpi_get_handle(device->handle, METHOD_NAME__PRT, &handle);
+ status = acpi_get_handle(handle, METHOD_NAME__PRT, &handle);
if (ACPI_SUCCESS(status)) {
if (dev->subordinate)
bus = dev->subordinate;
else
bus = dev->bus;
- acpi_pci_irq_add_prt(device->handle, bus);
+ acpi_pci_irq_add_prt(handle, bus);
}
return 0;
}
-static int acpi_pci_bind_cb(struct acpi_device *device)
-{
- int rc = 0;
- struct pci_dev *dev;
-
- dev = acpi_get_pci_dev(device->handle);
- if (dev) {
- rc = acpi_pci_bind(device, dev);
- pci_dev_put(dev);
- }
-
- return rc;
-}
-
-int acpi_pci_bind_root(struct acpi_device *device)
-{
- device->ops.bind = acpi_pci_bind_cb;
- device->ops.unbind = acpi_pci_unbind_cb;
-
- return 0;
-}
-
void acpi_pci_bind_notify(acpi_handle handle, struct device *dev, bool bind)
{
- struct acpi_device *acpi_dev;
-
- if (!dev_is_pci(dev))
- return;
- if (acpi_bus_get_device(handle, &acpi_dev) || !acpi_dev)
- return;
-
- if (acpi_dev->parent) {
- if (bind) {
- if (acpi_dev->parent->ops.bind)
- acpi_pci_bind(acpi_dev, to_pci_dev(dev));
- } else {
- if (acpi_dev->parent->ops.unbind)
- acpi_pci_unbind(acpi_dev, to_pci_dev(dev));
- }
+ if (dev_is_pci(dev)) {
+ if (bind)
+ acpi_pci_bind(handle, to_pci_dev(dev));
+ else
+ acpi_pci_unbind(handle, to_pci_dev(dev));
}
}
diff --git a/drivers/acpi/pci_root.c b/drivers/acpi/pci_root.c
index 7509034..25d8ad4 100644
--- a/drivers/acpi/pci_root.c
+++ b/drivers/acpi/pci_root.c
@@ -505,15 +505,6 @@ static int __devinit acpi_pci_root_add(struct acpi_device *device)
/* TBD: Locking */
list_add_tail(&root->node, &acpi_pci_roots);
- /*
- * Attach ACPI-PCI Context
- * -----------------------
- * Thus binding the ACPI and PCI devices.
- */
- result = acpi_pci_bind_root(device);
- if (result)
- goto end;
-
printk(KERN_INFO PREFIX "%s [%s] (domain %04x %pR)\n",
acpi_device_name(device), acpi_device_bid(device),
root->segment, &root->secondary);
diff --git a/drivers/acpi/scan.c b/drivers/acpi/scan.c
index d1ecca2..f31cb2f 100644
--- a/drivers/acpi/scan.c
+++ b/drivers/acpi/scan.c
@@ -1232,17 +1232,8 @@ static int acpi_bus_remove(struct acpi_device *dev, int rmdevice)
dev->removal_type = ACPI_BUS_REMOVAL_EJECT;
device_release_driver(&dev->dev);
- if (!rmdevice)
- return 0;
-
- /*
- * unbind _ADR-Based Devices when hot removal
- */
- if (dev->flags.bus_address) {
- if ((dev->parent) && (dev->parent->ops.unbind))
- dev->parent->ops.unbind(dev);
- }
- acpi_device_unregister(dev, ACPI_BUS_REMOVAL_EJECT);
+ if (rmdevice)
+ acpi_device_unregister(dev, ACPI_BUS_REMOVAL_EJECT);
return 0;
}
@@ -1319,14 +1310,6 @@ static int acpi_add_single_object(struct acpi_device **child,
result = acpi_device_register(device);
- /*
- * Bind _ADR-Based Devices when hot add
- */
- if (device->flags.bus_address) {
- if (device->parent && device->parent->ops.bind)
- device->parent->ops.bind(device);
- }
-
end:
if (!result) {
acpi_get_name(handle, ACPI_FULL_PATHNAME, &buffer);
diff --git a/include/acpi/acpi_bus.h b/include/acpi/acpi_bus.h
index bde976e..ef5babf 100644
--- a/include/acpi/acpi_bus.h
+++ b/include/acpi/acpi_bus.h
@@ -120,8 +120,6 @@ struct acpi_device;
typedef int (*acpi_op_add) (struct acpi_device * device);
typedef int (*acpi_op_remove) (struct acpi_device * device, int type);
typedef int (*acpi_op_start) (struct acpi_device * device);
-typedef int (*acpi_op_bind) (struct acpi_device * device);
-typedef int (*acpi_op_unbind) (struct acpi_device * device);
typedef void (*acpi_op_notify) (struct acpi_device * device, u32 event);
struct acpi_bus_ops {
@@ -133,8 +131,6 @@ struct acpi_device_ops {
acpi_op_add add;
acpi_op_remove remove;
acpi_op_start start;
- acpi_op_bind bind;
- acpi_op_unbind unbind;
acpi_op_notify notify;
};
diff --git a/include/acpi/acpi_drivers.h b/include/acpi/acpi_drivers.h
index bb145e4..fb1c0d5 100644
--- a/include/acpi/acpi_drivers.h
+++ b/include/acpi/acpi_drivers.h
@@ -100,7 +100,6 @@ void acpi_pci_irq_del_prt(struct pci_bus *bus);
struct pci_bus;
struct pci_dev *acpi_get_pci_dev(acpi_handle);
-int acpi_pci_bind_root(struct acpi_device *device);
/* Arch-defined function to add a bus to the system */
--
1.7.9.5
^ permalink raw reply related [flat|nested] 26+ messages in thread* Re: [PATCH v2 5/9] ACPI/pci-bind: remove bind/unbind callbacks from acpi_device_ops
2012-09-15 3:05 ` [PATCH v2 5/9] ACPI/pci-bind: remove bind/unbind callbacks from acpi_device_ops Jiang Liu
@ 2012-09-15 18:53 ` Yinghai Lu
2012-09-16 14:09 ` [PATCH v3] " Jiang Liu
2012-09-15 23:27 ` [PATCH v2 5/9] " Yinghai Lu
1 sibling, 1 reply; 26+ messages in thread
From: Yinghai Lu @ 2012-09-15 18:53 UTC (permalink / raw)
To: Jiang Liu
Cc: Bjorn Helgaas, Len Brown, Tony Luck, Jiang Liu, Kenji Kaneshige,
Yijing Wang, linux-kernel, linux-pci, linux-acpi
On Fri, Sep 14, 2012 at 8:05 PM, Jiang Liu <liuj97@gmail.com> wrote:
> From: Jiang Liu <jiang.liu@huawei.com>
>
> Now ACPI devices are created before/destroyed after corresponding PCI
> devices, and acpi_platform_notify/acpi_platform_notify_remove will
> update PCI<->ACPI binding relationship when creating/destroying PCI
> devices, there's no need to invoke bind/unbind callbacks from ACPI
> device probe/destroy routines anymore. So remove bind/unbind callbacks
> from acpi_device_ops.
>
> Signed-off-by: Jiang Liu <jiang.liu@huawei.com>
> Signed-off-by: Yijing Wang <wangyijing@huawei.com>
> ---
> drivers/acpi/pci_bind.c | 100 +++++++++----------------------------------
> drivers/acpi/pci_root.c | 9 ----
> drivers/acpi/scan.c | 21 +--------
> include/acpi/acpi_bus.h | 4 --
> include/acpi/acpi_drivers.h | 1 -
> 5 files changed, 23 insertions(+), 112 deletions(-)
>
> diff --git a/drivers/acpi/pci_bind.c b/drivers/acpi/pci_bind.c
> index 320e698..d18316a 100644
> --- a/drivers/acpi/pci_bind.c
> +++ b/drivers/acpi/pci_bind.c
> @@ -35,57 +35,31 @@
> #define _COMPONENT ACPI_PCI_COMPONENT
> ACPI_MODULE_NAME("pci_bind");
>
> -static int acpi_pci_bind_cb(struct acpi_device *device);
> -
> -static int acpi_pci_unbind(struct acpi_device *device, struct pci_dev *dev)
> +static int acpi_pci_unbind(acpi_handle handle, struct pci_dev *dev)
> {
> - device_set_run_wake(&dev->dev, false);
> - pci_acpi_remove_pm_notifier(device);
> + struct acpi_device *acpi_dev;
>
> - if (dev->subordinate) {
> - acpi_pci_irq_del_prt(dev->subordinate);
> - device->ops.bind = NULL;
> - device->ops.unbind = NULL;
> + if (!acpi_bus_get_device(handle, &acpi_dev) && acpi_dev) {
> + device_set_run_wake(&dev->dev, false);
> + pci_acpi_remove_pm_notifier(acpi_dev);
> }
>
> - return 0;
> -}
> -
> -static int acpi_pci_unbind_cb(struct acpi_device *device)
> -{
> - int rc = 0;
> - struct pci_dev *dev;
> -
> - dev = acpi_get_pci_dev(device->handle);
> - if (dev) {
> - rc = acpi_pci_unbind(device, dev);
> - pci_dev_put(dev);
> - }
> + if (dev->subordinate)
> + acpi_pci_irq_del_prt(dev->subordinate);
>
> - return rc;
> + return 0;
> }
>
> -static int acpi_pci_bind(struct acpi_device *device, struct pci_dev *dev)
> +static int acpi_pci_bind(acpi_handle handle, struct pci_dev *dev)
> {
> acpi_status status;
> - acpi_handle handle;
> struct pci_bus *bus;
> + struct acpi_device *acpi_dev;
>
> - pci_acpi_add_pm_notifier(device, dev);
> - if (device->wakeup.flags.run_wake)
> - device_set_run_wake(&dev->dev, true);
> -
> - /*
> - * Install the 'bind' function to facilitate callbacks for
> - * children of the P2P bridge.
> - */
> - if (dev->subordinate) {
> - ACPI_DEBUG_PRINT((ACPI_DB_INFO,
> - "Device %04x:%02x:%02x.%d is a PCI bridge\n",
> - pci_domain_nr(dev->bus), dev->bus->number,
> - PCI_SLOT(dev->devfn), PCI_FUNC(dev->devfn)));
> - device->ops.bind = acpi_pci_bind_cb;
> - device->ops.unbind = acpi_pci_unbind_cb;
> + if (!acpi_bus_get_device(handle, &acpi_dev) && acpi_dev) {
> + pci_acpi_add_pm_notifier(acpi_dev, dev);
> + if (acpi_dev->wakeup.flags.run_wake)
> + device_set_run_wake(&dev->dev, true);
why not just keep acpi_device *device in function ? so you can avoid
extra acpi_bus_get_device() calling.
> }
>
> /*
> @@ -96,56 +70,24 @@ static int acpi_pci_bind(struct acpi_device *device, struct pci_dev *dev)
> *
> * TBD: Can _PRTs exist within the scope of non-bridge PCI devices?
> */
> - status = acpi_get_handle(device->handle, METHOD_NAME__PRT, &handle);
> + status = acpi_get_handle(handle, METHOD_NAME__PRT, &handle);
> if (ACPI_SUCCESS(status)) {
> if (dev->subordinate)
> bus = dev->subordinate;
> else
> bus = dev->bus;
> - acpi_pci_irq_add_prt(device->handle, bus);
> + acpi_pci_irq_add_prt(handle, bus);
> }
if not _PRT, handle will become NULL? better to use &handle_tmp etc.
>
> return 0;
> }
>
> -static int acpi_pci_bind_cb(struct acpi_device *device)
> -{
> - int rc = 0;
> - struct pci_dev *dev;
> -
> - dev = acpi_get_pci_dev(device->handle);
> - if (dev) {
> - rc = acpi_pci_bind(device, dev);
> - pci_dev_put(dev);
> - }
> -
> - return rc;
> -}
> -
> -int acpi_pci_bind_root(struct acpi_device *device)
> -{
> - device->ops.bind = acpi_pci_bind_cb;
> - device->ops.unbind = acpi_pci_unbind_cb;
> -
> - return 0;
> -}
> -
> void acpi_pci_bind_notify(acpi_handle handle, struct device *dev, bool bind)
> {
> - struct acpi_device *acpi_dev;
> -
> - if (!dev_is_pci(dev))
> - return;
> - if (acpi_bus_get_device(handle, &acpi_dev) || !acpi_dev)
> - return;
> -
> - if (acpi_dev->parent) {
> - if (bind) {
> - if (acpi_dev->parent->ops.bind)
> - acpi_pci_bind(acpi_dev, to_pci_dev(dev));
> - } else {
> - if (acpi_dev->parent->ops.unbind)
> - acpi_pci_unbind(acpi_dev, to_pci_dev(dev));
> - }
> + if (dev_is_pci(dev)) {
> + if (bind)
> + acpi_pci_bind(handle, to_pci_dev(dev));
> + else
> + acpi_pci_unbind(handle, to_pci_dev(dev));
> }
> }
> diff --git a/drivers/acpi/pci_root.c b/drivers/acpi/pci_root.c
> index 7509034..25d8ad4 100644
> --- a/drivers/acpi/pci_root.c
> +++ b/drivers/acpi/pci_root.c
> @@ -505,15 +505,6 @@ static int __devinit acpi_pci_root_add(struct acpi_device *device)
> /* TBD: Locking */
> list_add_tail(&root->node, &acpi_pci_roots);
>
> - /*
> - * Attach ACPI-PCI Context
> - * -----------------------
> - * Thus binding the ACPI and PCI devices.
> - */
> - result = acpi_pci_bind_root(device);
> - if (result)
> - goto end;
> -
> printk(KERN_INFO PREFIX "%s [%s] (domain %04x %pR)\n",
> acpi_device_name(device), acpi_device_bid(device),
> root->segment, &root->secondary);
> diff --git a/drivers/acpi/scan.c b/drivers/acpi/scan.c
> index d1ecca2..f31cb2f 100644
> --- a/drivers/acpi/scan.c
> +++ b/drivers/acpi/scan.c
> @@ -1232,17 +1232,8 @@ static int acpi_bus_remove(struct acpi_device *dev, int rmdevice)
> dev->removal_type = ACPI_BUS_REMOVAL_EJECT;
> device_release_driver(&dev->dev);
>
> - if (!rmdevice)
> - return 0;
> -
> - /*
> - * unbind _ADR-Based Devices when hot removal
> - */
> - if (dev->flags.bus_address) {
> - if ((dev->parent) && (dev->parent->ops.unbind))
> - dev->parent->ops.unbind(dev);
> - }
> - acpi_device_unregister(dev, ACPI_BUS_REMOVAL_EJECT);
> + if (rmdevice)
> + acpi_device_unregister(dev, ACPI_BUS_REMOVAL_EJECT);
>
> return 0;
> }
> @@ -1319,14 +1310,6 @@ static int acpi_add_single_object(struct acpi_device **child,
>
> result = acpi_device_register(device);
>
> - /*
> - * Bind _ADR-Based Devices when hot add
> - */
> - if (device->flags.bus_address) {
> - if (device->parent && device->parent->ops.bind)
> - device->parent->ops.bind(device);
> - }
> -
> end:
> if (!result) {
> acpi_get_name(handle, ACPI_FULL_PATHNAME, &buffer);
> diff --git a/include/acpi/acpi_bus.h b/include/acpi/acpi_bus.h
> index bde976e..ef5babf 100644
> --- a/include/acpi/acpi_bus.h
> +++ b/include/acpi/acpi_bus.h
> @@ -120,8 +120,6 @@ struct acpi_device;
> typedef int (*acpi_op_add) (struct acpi_device * device);
> typedef int (*acpi_op_remove) (struct acpi_device * device, int type);
> typedef int (*acpi_op_start) (struct acpi_device * device);
> -typedef int (*acpi_op_bind) (struct acpi_device * device);
> -typedef int (*acpi_op_unbind) (struct acpi_device * device);
> typedef void (*acpi_op_notify) (struct acpi_device * device, u32 event);
>
> struct acpi_bus_ops {
> @@ -133,8 +131,6 @@ struct acpi_device_ops {
> acpi_op_add add;
> acpi_op_remove remove;
> acpi_op_start start;
> - acpi_op_bind bind;
> - acpi_op_unbind unbind;
> acpi_op_notify notify;
> };
>
> diff --git a/include/acpi/acpi_drivers.h b/include/acpi/acpi_drivers.h
> index bb145e4..fb1c0d5 100644
> --- a/include/acpi/acpi_drivers.h
> +++ b/include/acpi/acpi_drivers.h
> @@ -100,7 +100,6 @@ void acpi_pci_irq_del_prt(struct pci_bus *bus);
> struct pci_bus;
>
> struct pci_dev *acpi_get_pci_dev(acpi_handle);
> -int acpi_pci_bind_root(struct acpi_device *device);
>
> /* Arch-defined function to add a bus to the system */
>
> --
> 1.7.9.5
>
^ permalink raw reply [flat|nested] 26+ messages in thread* [PATCH v3] ACPI/pci-bind: remove bind/unbind callbacks from acpi_device_ops
2012-09-15 18:53 ` Yinghai Lu
@ 2012-09-16 14:09 ` Jiang Liu
2012-09-16 16:49 ` Yinghai Lu
0 siblings, 1 reply; 26+ messages in thread
From: Jiang Liu @ 2012-09-16 14:09 UTC (permalink / raw)
To: Bjorn Helgaas
Cc: Jiang Liu, Yinghai Lu, Kenji Kaneshige, Yijing Wang, Jiang Liu,
linux-kernel, linux-pci
Now ACPI devices are created before/destroyed after corresponding PCI
devices, and acpi_platform_notify/acpi_platform_notify_remove will
update PCI<->ACPI binding relationship when creating/destroying PCI
devices, there's no need to invoke bind/unbind callbacks from ACPI
device probe/destroy routines anymore. So remove bind/unbind callbacks
from acpi_device_ops.
Signed-off-by: Jiang Liu <jiang.liu@huawei.com>
Signed-off-by: Yijing Wang <wangyijing@huawei.com>
---
V2->V3:
Simplify implementation according to Yinghai's suggestion.
---
drivers/acpi/glue.c | 15 ++++---
drivers/acpi/internal.h | 3 +-
drivers/acpi/pci_bind.c | 102 ++++++++++---------------------------------
drivers/acpi/pci_root.c | 9 ----
drivers/acpi/scan.c | 21 +--------
include/acpi/acpi_bus.h | 4 --
include/acpi/acpi_drivers.h | 1 -
7 files changed, 36 insertions(+), 119 deletions(-)
diff --git a/drivers/acpi/glue.c b/drivers/acpi/glue.c
index 4904700..9779fb7 100644
--- a/drivers/acpi/glue.c
+++ b/drivers/acpi/glue.c
@@ -160,10 +160,13 @@ static int acpi_bind_one(struct device *dev, acpi_handle handle)
}
dev->archdata.acpi_handle = handle;
- acpi_pci_bind_notify(dev->archdata.acpi_handle, dev, true);
-
status = acpi_bus_get_device(handle, &acpi_dev);
- if (!ACPI_FAILURE(status)) {
+ if (!ACPI_FAILURE(status))
+ acpi_dev = NULL;
+
+ acpi_pci_bind_notify(dev->archdata.acpi_handle, acpi_dev, dev, true);
+
+ if (acpi_dev) {
int ret;
ret = sysfs_create_link(&dev->kobj, &acpi_dev->dev.kobj,
@@ -191,9 +194,11 @@ static int acpi_unbind_one(struct device *dev)
&acpi_dev)) {
sysfs_remove_link(&dev->kobj, "firmware_node");
sysfs_remove_link(&acpi_dev->dev.kobj, "physical_node");
- }
+ } else
+ acpi_dev = NULL;
- acpi_pci_bind_notify(dev->archdata.acpi_handle, dev, false);
+ acpi_pci_bind_notify(dev->archdata.acpi_handle, acpi_dev, dev,
+ false);
acpi_detach_data(dev->archdata.acpi_handle,
acpi_glue_data_handler);
diff --git a/drivers/acpi/internal.h b/drivers/acpi/internal.h
index 5396b20..ac7e647 100644
--- a/drivers/acpi/internal.h
+++ b/drivers/acpi/internal.h
@@ -93,6 +93,7 @@ static inline int suspend_nvs_save(void) { return 0; }
static inline void suspend_nvs_restore(void) {}
#endif
-void acpi_pci_bind_notify(acpi_handle handle, struct device *dev, bool bind);
+void acpi_pci_bind_notify(acpi_handle handle, struct acpi_device *acpi_dev,
+ struct device *dev, bool bind);
#endif /* _ACPI_INTERNAL_H_ */
diff --git a/drivers/acpi/pci_bind.c b/drivers/acpi/pci_bind.c
index 320e698..75846a7 100644
--- a/drivers/acpi/pci_bind.c
+++ b/drivers/acpi/pci_bind.c
@@ -35,57 +35,30 @@
#define _COMPONENT ACPI_PCI_COMPONENT
ACPI_MODULE_NAME("pci_bind");
-static int acpi_pci_bind_cb(struct acpi_device *device);
-
-static int acpi_pci_unbind(struct acpi_device *device, struct pci_dev *dev)
+static int acpi_pci_unbind(struct acpi_device *acpi_dev, struct pci_dev *dev)
{
- device_set_run_wake(&dev->dev, false);
- pci_acpi_remove_pm_notifier(device);
+ if (acpi_dev) {
+ device_set_run_wake(&dev->dev, false);
+ pci_acpi_remove_pm_notifier(acpi_dev);
+ }
- if (dev->subordinate) {
+ if (dev->subordinate)
acpi_pci_irq_del_prt(dev->subordinate);
- device->ops.bind = NULL;
- device->ops.unbind = NULL;
- }
return 0;
}
-static int acpi_pci_unbind_cb(struct acpi_device *device)
-{
- int rc = 0;
- struct pci_dev *dev;
-
- dev = acpi_get_pci_dev(device->handle);
- if (dev) {
- rc = acpi_pci_unbind(device, dev);
- pci_dev_put(dev);
- }
-
- return rc;
-}
-
-static int acpi_pci_bind(struct acpi_device *device, struct pci_dev *dev)
+static int acpi_pci_bind(acpi_handle handle, struct acpi_device *acpi_dev,
+ struct pci_dev *dev)
{
acpi_status status;
- acpi_handle handle;
struct pci_bus *bus;
+ acpi_handle tmp_hdl;
- pci_acpi_add_pm_notifier(device, dev);
- if (device->wakeup.flags.run_wake)
- device_set_run_wake(&dev->dev, true);
-
- /*
- * Install the 'bind' function to facilitate callbacks for
- * children of the P2P bridge.
- */
- if (dev->subordinate) {
- ACPI_DEBUG_PRINT((ACPI_DB_INFO,
- "Device %04x:%02x:%02x.%d is a PCI bridge\n",
- pci_domain_nr(dev->bus), dev->bus->number,
- PCI_SLOT(dev->devfn), PCI_FUNC(dev->devfn)));
- device->ops.bind = acpi_pci_bind_cb;
- device->ops.unbind = acpi_pci_unbind_cb;
+ if (acpi_dev) {
+ pci_acpi_add_pm_notifier(acpi_dev, dev);
+ if (acpi_dev->wakeup.flags.run_wake)
+ device_set_run_wake(&dev->dev, true);
}
/*
@@ -96,56 +69,25 @@ static int acpi_pci_bind(struct acpi_device *device, struct pci_dev *dev)
*
* TBD: Can _PRTs exist within the scope of non-bridge PCI devices?
*/
- status = acpi_get_handle(device->handle, METHOD_NAME__PRT, &handle);
+ status = acpi_get_handle(handle, METHOD_NAME__PRT, &tmp_hdl);
if (ACPI_SUCCESS(status)) {
if (dev->subordinate)
bus = dev->subordinate;
else
bus = dev->bus;
- acpi_pci_irq_add_prt(device->handle, bus);
- }
-
- return 0;
-}
-
-static int acpi_pci_bind_cb(struct acpi_device *device)
-{
- int rc = 0;
- struct pci_dev *dev;
-
- dev = acpi_get_pci_dev(device->handle);
- if (dev) {
- rc = acpi_pci_bind(device, dev);
- pci_dev_put(dev);
+ acpi_pci_irq_add_prt(handle, bus);
}
- return rc;
-}
-
-int acpi_pci_bind_root(struct acpi_device *device)
-{
- device->ops.bind = acpi_pci_bind_cb;
- device->ops.unbind = acpi_pci_unbind_cb;
-
return 0;
}
-void acpi_pci_bind_notify(acpi_handle handle, struct device *dev, bool bind)
+void acpi_pci_bind_notify(acpi_handle handle, struct acpi_device *acpi_dev,
+ struct device *dev, bool bind)
{
- struct acpi_device *acpi_dev;
-
- if (!dev_is_pci(dev))
- return;
- if (acpi_bus_get_device(handle, &acpi_dev) || !acpi_dev)
- return;
-
- if (acpi_dev->parent) {
- if (bind) {
- if (acpi_dev->parent->ops.bind)
- acpi_pci_bind(acpi_dev, to_pci_dev(dev));
- } else {
- if (acpi_dev->parent->ops.unbind)
- acpi_pci_unbind(acpi_dev, to_pci_dev(dev));
- }
+ if (dev_is_pci(dev)) {
+ if (bind)
+ acpi_pci_bind(handle, acpi_dev, to_pci_dev(dev));
+ else
+ acpi_pci_unbind(acpi_dev, to_pci_dev(dev));
}
}
diff --git a/drivers/acpi/pci_root.c b/drivers/acpi/pci_root.c
index 7509034..25d8ad4 100644
--- a/drivers/acpi/pci_root.c
+++ b/drivers/acpi/pci_root.c
@@ -505,15 +505,6 @@ static int __devinit acpi_pci_root_add(struct acpi_device *device)
/* TBD: Locking */
list_add_tail(&root->node, &acpi_pci_roots);
- /*
- * Attach ACPI-PCI Context
- * -----------------------
- * Thus binding the ACPI and PCI devices.
- */
- result = acpi_pci_bind_root(device);
- if (result)
- goto end;
-
printk(KERN_INFO PREFIX "%s [%s] (domain %04x %pR)\n",
acpi_device_name(device), acpi_device_bid(device),
root->segment, &root->secondary);
diff --git a/drivers/acpi/scan.c b/drivers/acpi/scan.c
index d1ecca2..f31cb2f 100644
--- a/drivers/acpi/scan.c
+++ b/drivers/acpi/scan.c
@@ -1232,17 +1232,8 @@ static int acpi_bus_remove(struct acpi_device *dev, int rmdevice)
dev->removal_type = ACPI_BUS_REMOVAL_EJECT;
device_release_driver(&dev->dev);
- if (!rmdevice)
- return 0;
-
- /*
- * unbind _ADR-Based Devices when hot removal
- */
- if (dev->flags.bus_address) {
- if ((dev->parent) && (dev->parent->ops.unbind))
- dev->parent->ops.unbind(dev);
- }
- acpi_device_unregister(dev, ACPI_BUS_REMOVAL_EJECT);
+ if (rmdevice)
+ acpi_device_unregister(dev, ACPI_BUS_REMOVAL_EJECT);
return 0;
}
@@ -1319,14 +1310,6 @@ static int acpi_add_single_object(struct acpi_device **child,
result = acpi_device_register(device);
- /*
- * Bind _ADR-Based Devices when hot add
- */
- if (device->flags.bus_address) {
- if (device->parent && device->parent->ops.bind)
- device->parent->ops.bind(device);
- }
-
end:
if (!result) {
acpi_get_name(handle, ACPI_FULL_PATHNAME, &buffer);
diff --git a/include/acpi/acpi_bus.h b/include/acpi/acpi_bus.h
index bde976e..ef5babf 100644
--- a/include/acpi/acpi_bus.h
+++ b/include/acpi/acpi_bus.h
@@ -120,8 +120,6 @@ struct acpi_device;
typedef int (*acpi_op_add) (struct acpi_device * device);
typedef int (*acpi_op_remove) (struct acpi_device * device, int type);
typedef int (*acpi_op_start) (struct acpi_device * device);
-typedef int (*acpi_op_bind) (struct acpi_device * device);
-typedef int (*acpi_op_unbind) (struct acpi_device * device);
typedef void (*acpi_op_notify) (struct acpi_device * device, u32 event);
struct acpi_bus_ops {
@@ -133,8 +131,6 @@ struct acpi_device_ops {
acpi_op_add add;
acpi_op_remove remove;
acpi_op_start start;
- acpi_op_bind bind;
- acpi_op_unbind unbind;
acpi_op_notify notify;
};
diff --git a/include/acpi/acpi_drivers.h b/include/acpi/acpi_drivers.h
index bb145e4..fb1c0d5 100644
--- a/include/acpi/acpi_drivers.h
+++ b/include/acpi/acpi_drivers.h
@@ -100,7 +100,6 @@ void acpi_pci_irq_del_prt(struct pci_bus *bus);
struct pci_bus;
struct pci_dev *acpi_get_pci_dev(acpi_handle);
-int acpi_pci_bind_root(struct acpi_device *device);
/* Arch-defined function to add a bus to the system */
--
1.7.9.5
^ permalink raw reply related [flat|nested] 26+ messages in thread* Re: [PATCH v3] ACPI/pci-bind: remove bind/unbind callbacks from acpi_device_ops
2012-09-16 14:09 ` [PATCH v3] " Jiang Liu
@ 2012-09-16 16:49 ` Yinghai Lu
2012-09-16 18:02 ` Yinghai Lu
0 siblings, 1 reply; 26+ messages in thread
From: Yinghai Lu @ 2012-09-16 16:49 UTC (permalink / raw)
To: Jiang Liu
Cc: Bjorn Helgaas, Jiang Liu, Kenji Kaneshige, Yijing Wang,
linux-kernel, linux-pci
On Sun, Sep 16, 2012 at 7:09 AM, Jiang Liu <liuj97@gmail.com> wrote:
> Now ACPI devices are created before/destroyed after corresponding PCI
> devices, and acpi_platform_notify/acpi_platform_notify_remove will
> update PCI<->ACPI binding relationship when creating/destroying PCI
> devices, there's no need to invoke bind/unbind callbacks from ACPI
> device probe/destroy routines anymore. So remove bind/unbind callbacks
> from acpi_device_ops.
>
> Signed-off-by: Jiang Liu <jiang.liu@huawei.com>
> Signed-off-by: Yijing Wang <wangyijing@huawei.com>
> ---
> V2->V3:
> Simplify implementation according to Yinghai's suggestion.
> ---
> drivers/acpi/glue.c | 15 ++++---
> drivers/acpi/internal.h | 3 +-
> drivers/acpi/pci_bind.c | 102 ++++++++++---------------------------------
> drivers/acpi/pci_root.c | 9 ----
> drivers/acpi/scan.c | 21 +--------
> include/acpi/acpi_bus.h | 4 --
> include/acpi/acpi_drivers.h | 1 -
> 7 files changed, 36 insertions(+), 119 deletions(-)
>
> diff --git a/drivers/acpi/glue.c b/drivers/acpi/glue.c
> index 4904700..9779fb7 100644
> --- a/drivers/acpi/glue.c
> +++ b/drivers/acpi/glue.c
> @@ -160,10 +160,13 @@ static int acpi_bind_one(struct device *dev, acpi_handle handle)
> }
> dev->archdata.acpi_handle = handle;
>
> - acpi_pci_bind_notify(dev->archdata.acpi_handle, dev, true);
> -
> status = acpi_bus_get_device(handle, &acpi_dev);
> - if (!ACPI_FAILURE(status)) {
> + if (!ACPI_FAILURE(status))
> + acpi_dev = NULL;
> +
> + acpi_pci_bind_notify(dev->archdata.acpi_handle, acpi_dev, dev, true);
It seems you can do
acpi_pci_bind_notify(acpi_dev, dev, true);
instead.
aka:
Do you have test case acpi_dev is not created before pci device?
Thanks
Yinghai
^ permalink raw reply [flat|nested] 26+ messages in thread* Re: [PATCH v3] ACPI/pci-bind: remove bind/unbind callbacks from acpi_device_ops
2012-09-16 16:49 ` Yinghai Lu
@ 2012-09-16 18:02 ` Yinghai Lu
2012-09-17 3:06 ` Yinghai Lu
2012-09-17 15:26 ` Jiang Liu
0 siblings, 2 replies; 26+ messages in thread
From: Yinghai Lu @ 2012-09-16 18:02 UTC (permalink / raw)
To: Jiang Liu
Cc: Bjorn Helgaas, Jiang Liu, Kenji Kaneshige, Yijing Wang,
linux-kernel, linux-pci
[-- Attachment #1: Type: text/plain, Size: 922 bytes --]
On Sun, Sep 16, 2012 at 9:49 AM, Yinghai Lu <yinghai@kernel.org> wrote:
> On Sun, Sep 16, 2012 at 7:09 AM, Jiang Liu <liuj97@gmail.com> wrote:
>> @@ -160,10 +160,13 @@ static int acpi_bind_one(struct device *dev, acpi_handle handle)
>> }
>> dev->archdata.acpi_handle = handle;
>>
>> - acpi_pci_bind_notify(dev->archdata.acpi_handle, dev, true);
>> -
>> status = acpi_bus_get_device(handle, &acpi_dev);
>> - if (!ACPI_FAILURE(status)) {
>> + if (!ACPI_FAILURE(status))
>> + acpi_dev = NULL;
>> +
>> + acpi_pci_bind_notify(dev->archdata.acpi_handle, acpi_dev, dev, true);
>
> It seems you can do
> acpi_pci_bind_notify(acpi_dev, dev, true);
> instead.
>
> aka:
> Do you have test case acpi_dev is not created before pci device?
>
updated your patch, and retrieve handle if needed from acpi_dev->handle or
pci_dev->dev.archdata.acpi_handle.
Thanks
Yinghai
[-- Attachment #2: jiang_hotplug_rework_5.patch --]
[-- Type: application/octet-stream, Size: 13030 bytes --]
Delivered-To: yhlu.kernel@gmail.com
Received: by 10.205.37.138 with SMTP id te10csp235723bkb;
Sun, 16 Sep 2012 07:11:25 -0700 (PDT)
Received: by 10.50.207.36 with SMTP id lt4mr4177206igc.66.1347804683432;
Sun, 16 Sep 2012 07:11:23 -0700 (PDT)
Return-Path: <liuj97@gmail.com>
Received: from mail.kernel.org (mail.kernel.org. [198.145.19.201])
by mx.google.com with ESMTP id f3si6506496ign.11.2012.09.16.07.11.22;
Sun, 16 Sep 2012 07:11:23 -0700 (PDT)
Received-SPF: neutral (google.com: 198.145.19.201 is neither permitted nor denied by domain of liuj97@gmail.com) client-ip=198.145.19.201;
Authentication-Results: mx.google.com; spf=neutral (google.com: 198.145.19.201 is neither permitted nor denied by domain of liuj97@gmail.com) smtp.mail=liuj97@gmail.com; dkim=pass header.i=@gmail.com
Received: by mail.kernel.org (Postfix)
id 8461B201C7; Sun, 16 Sep 2012 14:11:22 +0000 (UTC)
Delivered-To: yinghai@kernel.org
Received: from mail.kernel.org (localhost [127.0.0.1])
by mail.kernel.org (Postfix) with ESMTP id 7B313201C8
for <yinghai@kernel.org>; Sun, 16 Sep 2012 14:11:21 +0000 (UTC)
Received: from mail-pb0-f44.google.com (mail-pb0-f44.google.com [209.85.160.44])
by mail.kernel.org (Postfix) with ESMTP id 72EC0201C7
for <yinghai@kernel.org>; Sun, 16 Sep 2012 14:11:20 +0000 (UTC)
Received: by pbbrr4 with SMTP id rr4so8055480pbb.3
for <yinghai@kernel.org>; Sun, 16 Sep 2012 07:11:20 -0700 (PDT)
DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed;
d=gmail.com; s=20120113;
h=from:to:cc:subject:date:message-id:x-mailer:in-reply-to:references;
bh=fS1xyrF8eXCaLy4Tn+hOt9rmpMLcQ49ea9y8DXMukLI=;
b=XKOUTY7hRn4NBfJmYIichX5zMcWwr6doD9xulUuTUUsJagFeWGeMMfkuRivVoZruVZ
Ag3LjxXWqVj/SFdZAke58J0/dMw+e7AlVyMWU0Jij2/kxPXuwtJOzAD2FKP4gGLR5FSJ
WUm5u5F8CFIRipqfWgTHYs64bYBSHICW6jto6f8xJLUu/A5jeS6vOnVFCRl4EFyyR2Nh
h8hSpM7m+dZoJgp6EB3rimZmKbjMkMazxn4UmlQOhr9HfxyM2M9w6uXwSnicd4wyRw6z
l8rGXdIdWoiJlDChjQ6yc6sPsu8Njc2AwRAZvaamj1cqnNDWg3UIikxI/5Pkor9Kc7tg
J42g==
Received: by 10.68.222.42 with SMTP id qj10mr16338751pbc.117.1347804680249;
Sun, 16 Sep 2012 07:11:20 -0700 (PDT)
Received: from localhost.localdomain ([221.221.18.122])
by mx.google.com with ESMTPS id pi1sm5103465pbb.7.2012.09.16.07.11.11
(version=TLSv1/SSLv3 cipher=OTHER);
Sun, 16 Sep 2012 07:11:18 -0700 (PDT)
From: Jiang Liu <liuj97@gmail.com>
To: Bjorn Helgaas <bhelgaas@google.com>
Cc: Jiang Liu <jiang.liu@huawei.com>,
Yinghai Lu <yinghai@kernel.org>,
Kenji Kaneshige <kaneshige.kenji@jp.fujitsu.com>,
Yijing Wang <wangyijing@huawei.com>,
Jiang Liu <liuj97@gmail.com>,
linux-kernel@vger.kernel.org,
linux-pci@vger.kernel.org
Subject: [PATCH v3] ACPI/pci-bind: remove bind/unbind callbacks from acpi_device_ops
Date: Sun, 16 Sep 2012 22:09:37 +0800
Message-Id: <1347804577-23015-1-git-send-email-jiang.liu@huawei.com>
X-Mailer: git-send-email 1.7.9.5
In-Reply-To: <CAE9FiQWBYmRj5--pg8eAwH_RKZsCWo+CVGN_KpeGXeE_3cSq9g@mail.gmail.com>
References: <CAE9FiQWBYmRj5--pg8eAwH_RKZsCWo+CVGN_KpeGXeE_3cSq9g@mail.gmail.com>
X-Spam-Status: No, score=-2.4 required=5.0 tests=BAYES_00,DKIM_SIGNED,
DKIM_VALID,DKIM_VALID_AU,FREEMAIL_ENVFROM_END_DIGIT,FREEMAIL_FROM,
RCVD_IN_DNSWL_LOW,UNPARSEABLE_RELAY autolearn=ham version=3.3.1
X-Spam-Checker-Version: SpamAssassin 3.3.1 (2010-03-16) on mail.kernel.org
X-Virus-Scanned: ClamAV using ClamSMTP
Now ACPI devices are created before/destroyed after corresponding PCI
devices, and acpi_platform_notify/acpi_platform_notify_remove will
update PCI<->ACPI binding relationship when creating/destroying PCI
devices, there's no need to invoke bind/unbind callbacks from ACPI
device probe/destroy routines anymore. So remove bind/unbind callbacks
from acpi_device_ops.
Signed-off-by: Jiang Liu <jiang.liu@huawei.com>
Signed-off-by: Yijing Wang <wangyijing@huawei.com>
---
V2->V3:
Simplify implementation according to Yinghai's suggestion.
---
drivers/acpi/glue.c | 14 +++--
drivers/acpi/internal.h | 3 -
drivers/acpi/pci_bind.c | 106 ++++++++++----------------------------------
drivers/acpi/pci_root.c | 9 ---
drivers/acpi/scan.c | 21 --------
include/acpi/acpi_bus.h | 4 -
include/acpi/acpi_drivers.h | 1
7 files changed, 38 insertions(+), 120 deletions(-)
Index: linux-2.6/drivers/acpi/glue.c
===================================================================
--- linux-2.6.orig/drivers/acpi/glue.c
+++ linux-2.6/drivers/acpi/glue.c
@@ -160,10 +160,13 @@ static int acpi_bind_one(struct device *
}
dev->archdata.acpi_handle = handle;
- acpi_pci_bind_notify(dev->archdata.acpi_handle, dev, true);
-
status = acpi_bus_get_device(handle, &acpi_dev);
- if (!ACPI_FAILURE(status)) {
+ if (!ACPI_FAILURE(status))
+ acpi_dev = NULL;
+
+ acpi_pci_bind_notify(acpi_dev, dev, true);
+
+ if (acpi_dev) {
int ret;
ret = sysfs_create_link(&dev->kobj, &acpi_dev->dev.kobj,
@@ -191,9 +194,10 @@ static int acpi_unbind_one(struct device
&acpi_dev)) {
sysfs_remove_link(&dev->kobj, "firmware_node");
sysfs_remove_link(&acpi_dev->dev.kobj, "physical_node");
- }
+ } else
+ acpi_dev = NULL;
- acpi_pci_bind_notify(dev->archdata.acpi_handle, dev, false);
+ acpi_pci_bind_notify(acpi_dev, dev, false);
acpi_detach_data(dev->archdata.acpi_handle,
acpi_glue_data_handler);
Index: linux-2.6/drivers/acpi/internal.h
===================================================================
--- linux-2.6.orig/drivers/acpi/internal.h
+++ linux-2.6/drivers/acpi/internal.h
@@ -93,6 +93,7 @@ static inline int suspend_nvs_save(void)
static inline void suspend_nvs_restore(void) {}
#endif
-void acpi_pci_bind_notify(acpi_handle handle, struct device *dev, bool bind);
+void acpi_pci_bind_notify(struct acpi_device *acpi_dev,
+ struct device *dev, bool bind);
#endif /* _ACPI_INTERNAL_H_ */
Index: linux-2.6/drivers/acpi/pci_bind.c
===================================================================
--- linux-2.6.orig/drivers/acpi/pci_bind.c
+++ linux-2.6/drivers/acpi/pci_bind.c
@@ -35,58 +35,33 @@
#define _COMPONENT ACPI_PCI_COMPONENT
ACPI_MODULE_NAME("pci_bind");
-static int acpi_pci_bind_cb(struct acpi_device *device);
-
-static int acpi_pci_unbind(struct acpi_device *device, struct pci_dev *dev)
+static int acpi_pci_unbind(struct acpi_device *acpi_dev, struct pci_dev *dev)
{
- device_set_run_wake(&dev->dev, false);
- pci_acpi_remove_pm_notifier(device);
+ if (acpi_dev) {
+ device_set_run_wake(&dev->dev, false);
+ pci_acpi_remove_pm_notifier(acpi_dev);
+ }
- if (dev->subordinate) {
+ if (dev->subordinate)
acpi_pci_irq_del_prt(dev->subordinate);
- device->ops.bind = NULL;
- device->ops.unbind = NULL;
- }
return 0;
}
-static int acpi_pci_unbind_cb(struct acpi_device *device)
-{
- int rc = 0;
- struct pci_dev *dev;
-
- dev = acpi_get_pci_dev(device->handle);
- if (dev) {
- rc = acpi_pci_unbind(device, dev);
- pci_dev_put(dev);
- }
-
- return rc;
-}
-
-static int acpi_pci_bind(struct acpi_device *device, struct pci_dev *dev)
+static int acpi_pci_bind(struct acpi_device *acpi_dev, struct pci_dev *dev)
{
acpi_status status;
- acpi_handle handle;
struct pci_bus *bus;
+ acpi_handle handle;
+ acpi_handle tmp_hdl;
- pci_acpi_add_pm_notifier(device, dev);
- if (device->wakeup.flags.run_wake)
- device_set_run_wake(&dev->dev, true);
-
- /*
- * Install the 'bind' function to facilitate callbacks for
- * children of the P2P bridge.
- */
- if (dev->subordinate) {
- ACPI_DEBUG_PRINT((ACPI_DB_INFO,
- "Device %04x:%02x:%02x.%d is a PCI bridge\n",
- pci_domain_nr(dev->bus), dev->bus->number,
- PCI_SLOT(dev->devfn), PCI_FUNC(dev->devfn)));
- device->ops.bind = acpi_pci_bind_cb;
- device->ops.unbind = acpi_pci_unbind_cb;
- }
+ if (acpi_dev) {
+ pci_acpi_add_pm_notifier(acpi_dev, dev);
+ if (acpi_dev->wakeup.flags.run_wake)
+ device_set_run_wake(&dev->dev, true);
+ handle = acpi_dev->handle;
+ } else
+ handle = dev->dev.archdata.acpi_handle;
/*
* Evaluate and parse _PRT, if exists. This code allows parsing of
@@ -96,56 +71,25 @@ static int acpi_pci_bind(struct acpi_dev
*
* TBD: Can _PRTs exist within the scope of non-bridge PCI devices?
*/
- status = acpi_get_handle(device->handle, METHOD_NAME__PRT, &handle);
+ status = acpi_get_handle(handle, METHOD_NAME__PRT, &tmp_hdl);
if (ACPI_SUCCESS(status)) {
if (dev->subordinate)
bus = dev->subordinate;
else
bus = dev->bus;
- acpi_pci_irq_add_prt(device->handle, bus);
- }
-
- return 0;
-}
-
-static int acpi_pci_bind_cb(struct acpi_device *device)
-{
- int rc = 0;
- struct pci_dev *dev;
-
- dev = acpi_get_pci_dev(device->handle);
- if (dev) {
- rc = acpi_pci_bind(device, dev);
- pci_dev_put(dev);
+ acpi_pci_irq_add_prt(handle, bus);
}
- return rc;
-}
-
-int acpi_pci_bind_root(struct acpi_device *device)
-{
- device->ops.bind = acpi_pci_bind_cb;
- device->ops.unbind = acpi_pci_unbind_cb;
-
return 0;
}
-void acpi_pci_bind_notify(acpi_handle handle, struct device *dev, bool bind)
+void acpi_pci_bind_notify(struct acpi_device *acpi_dev,
+ struct device *dev, bool bind)
{
- struct acpi_device *acpi_dev;
-
- if (!dev_is_pci(dev))
- return;
- if (acpi_bus_get_device(handle, &acpi_dev) || !acpi_dev)
- return;
-
- if (acpi_dev->parent) {
- if (bind) {
- if (acpi_dev->parent->ops.bind)
- acpi_pci_bind(acpi_dev, to_pci_dev(dev));
- } else {
- if (acpi_dev->parent->ops.unbind)
- acpi_pci_unbind(acpi_dev, to_pci_dev(dev));
- }
+ if (dev_is_pci(dev)) {
+ if (bind)
+ acpi_pci_bind(acpi_dev, to_pci_dev(dev));
+ else
+ acpi_pci_unbind(acpi_dev, to_pci_dev(dev));
}
}
Index: linux-2.6/drivers/acpi/pci_root.c
===================================================================
--- linux-2.6.orig/drivers/acpi/pci_root.c
+++ linux-2.6/drivers/acpi/pci_root.c
@@ -505,15 +505,6 @@ static int __devinit acpi_pci_root_add(s
/* TBD: Locking */
list_add_tail(&root->node, &acpi_pci_roots);
- /*
- * Attach ACPI-PCI Context
- * -----------------------
- * Thus binding the ACPI and PCI devices.
- */
- result = acpi_pci_bind_root(device);
- if (result)
- goto end;
-
printk(KERN_INFO PREFIX "%s [%s] (domain %04x %pR)\n",
acpi_device_name(device), acpi_device_bid(device),
root->segment, &root->secondary);
Index: linux-2.6/drivers/acpi/scan.c
===================================================================
--- linux-2.6.orig/drivers/acpi/scan.c
+++ linux-2.6/drivers/acpi/scan.c
@@ -1232,17 +1232,8 @@ static int acpi_bus_remove(struct acpi_d
dev->removal_type = ACPI_BUS_REMOVAL_EJECT;
device_release_driver(&dev->dev);
- if (!rmdevice)
- return 0;
-
- /*
- * unbind _ADR-Based Devices when hot removal
- */
- if (dev->flags.bus_address) {
- if ((dev->parent) && (dev->parent->ops.unbind))
- dev->parent->ops.unbind(dev);
- }
- acpi_device_unregister(dev, ACPI_BUS_REMOVAL_EJECT);
+ if (rmdevice)
+ acpi_device_unregister(dev, ACPI_BUS_REMOVAL_EJECT);
return 0;
}
@@ -1319,14 +1310,6 @@ static int acpi_add_single_object(struct
result = acpi_device_register(device);
- /*
- * Bind _ADR-Based Devices when hot add
- */
- if (device->flags.bus_address) {
- if (device->parent && device->parent->ops.bind)
- device->parent->ops.bind(device);
- }
-
end:
if (!result) {
acpi_get_name(handle, ACPI_FULL_PATHNAME, &buffer);
Index: linux-2.6/include/acpi/acpi_bus.h
===================================================================
--- linux-2.6.orig/include/acpi/acpi_bus.h
+++ linux-2.6/include/acpi/acpi_bus.h
@@ -120,8 +120,6 @@ struct acpi_device;
typedef int (*acpi_op_add) (struct acpi_device * device);
typedef int (*acpi_op_remove) (struct acpi_device * device, int type);
typedef int (*acpi_op_start) (struct acpi_device * device);
-typedef int (*acpi_op_bind) (struct acpi_device * device);
-typedef int (*acpi_op_unbind) (struct acpi_device * device);
typedef void (*acpi_op_notify) (struct acpi_device * device, u32 event);
struct acpi_bus_ops {
@@ -133,8 +131,6 @@ struct acpi_device_ops {
acpi_op_add add;
acpi_op_remove remove;
acpi_op_start start;
- acpi_op_bind bind;
- acpi_op_unbind unbind;
acpi_op_notify notify;
};
Index: linux-2.6/include/acpi/acpi_drivers.h
===================================================================
--- linux-2.6.orig/include/acpi/acpi_drivers.h
+++ linux-2.6/include/acpi/acpi_drivers.h
@@ -100,7 +100,6 @@ void acpi_pci_irq_del_prt(struct pci_bus
struct pci_bus;
struct pci_dev *acpi_get_pci_dev(acpi_handle);
-int acpi_pci_bind_root(struct acpi_device *device);
/* Arch-defined function to add a bus to the system */
^ permalink raw reply [flat|nested] 26+ messages in thread* Re: [PATCH v3] ACPI/pci-bind: remove bind/unbind callbacks from acpi_device_ops
2012-09-16 18:02 ` Yinghai Lu
@ 2012-09-17 3:06 ` Yinghai Lu
2012-09-17 15:26 ` Jiang Liu
1 sibling, 0 replies; 26+ messages in thread
From: Yinghai Lu @ 2012-09-17 3:06 UTC (permalink / raw)
To: Jiang Liu
Cc: Bjorn Helgaas, Jiang Liu, Kenji Kaneshige, Yijing Wang,
linux-kernel, linux-pci
On Sun, Sep 16, 2012 at 11:02 AM, Yinghai Lu <yinghai@kernel.org> wrote:
> On Sun, Sep 16, 2012 at 9:49 AM, Yinghai Lu <yinghai@kernel.org> wrote:
>> On Sun, Sep 16, 2012 at 7:09 AM, Jiang Liu <liuj97@gmail.com> wrote:
>>> @@ -160,10 +160,13 @@ static int acpi_bind_one(struct device *dev, acpi_handle handle)
>>> }
>>> dev->archdata.acpi_handle = handle;
>>>
>>> - acpi_pci_bind_notify(dev->archdata.acpi_handle, dev, true);
>>> -
>>> status = acpi_bus_get_device(handle, &acpi_dev);
>>> - if (!ACPI_FAILURE(status)) {
>>> + if (!ACPI_FAILURE(status))
>>> + acpi_dev = NULL;
>>> +
>>> + acpi_pci_bind_notify(dev->archdata.acpi_handle, acpi_dev, dev, true);
>>
>> It seems you can do
>> acpi_pci_bind_notify(acpi_dev, dev, true);
>> instead.
>>
>> aka:
>> Do you have test case acpi_dev is not created before pci device?
>>
>
> updated your patch, and retrieve handle if needed from acpi_dev->handle or
> pci_dev->dev.archdata.acpi_handle.
please use this updated version.
Also you could add
Reviewed-by: Yinghai Lu <yinghai@kernel.org>
for your patch 1, 2, 4, 5.
Thanks
Yinghai
^ permalink raw reply [flat|nested] 26+ messages in thread* Re: [PATCH v3] ACPI/pci-bind: remove bind/unbind callbacks from acpi_device_ops
2012-09-16 18:02 ` Yinghai Lu
2012-09-17 3:06 ` Yinghai Lu
@ 2012-09-17 15:26 ` Jiang Liu
1 sibling, 0 replies; 26+ messages in thread
From: Jiang Liu @ 2012-09-17 15:26 UTC (permalink / raw)
To: Yinghai Lu
Cc: Bjorn Helgaas, Jiang Liu, Kenji Kaneshige, Yijing Wang,
linux-kernel, linux-pci
On 09/17/2012 02:02 AM, Yinghai Lu wrote:
> On Sun, Sep 16, 2012 at 9:49 AM, Yinghai Lu <yinghai@kernel.org> wrote:
>> On Sun, Sep 16, 2012 at 7:09 AM, Jiang Liu <liuj97@gmail.com> wrote:
>>> @@ -160,10 +160,13 @@ static int acpi_bind_one(struct device *dev, acpi_handle handle)
>>> }
>>> dev->archdata.acpi_handle = handle;
>>>
>>> - acpi_pci_bind_notify(dev->archdata.acpi_handle, dev, true);
>>> -
>>> status = acpi_bus_get_device(handle, &acpi_dev);
>>> - if (!ACPI_FAILURE(status)) {
>>> + if (!ACPI_FAILURE(status))
>>> + acpi_dev = NULL;
>>> +
>>> + acpi_pci_bind_notify(dev->archdata.acpi_handle, acpi_dev, dev, true);
>>
>> It seems you can do
>> acpi_pci_bind_notify(acpi_dev, dev, true);
>> instead.
>>
>> aka:
>> Do you have test case acpi_dev is not created before pci device?
>>
>
> updated your patch, and retrieve handle if needed from acpi_dev->handle or
> pci_dev->dev.archdata.acpi_handle.
Hi Yinghai,
Thanks for your review. I have made some changes to the attached patch
to better support bisect. But the final result should be the same.
--Gerry
>
> Thanks
>
> Yinghai
>
^ permalink raw reply [flat|nested] 26+ messages in thread
* Re: [PATCH v2 5/9] ACPI/pci-bind: remove bind/unbind callbacks from acpi_device_ops
2012-09-15 3:05 ` [PATCH v2 5/9] ACPI/pci-bind: remove bind/unbind callbacks from acpi_device_ops Jiang Liu
2012-09-15 18:53 ` Yinghai Lu
@ 2012-09-15 23:27 ` Yinghai Lu
2012-09-17 3:03 ` Yinghai Lu
2012-09-17 14:31 ` Jiang Liu
1 sibling, 2 replies; 26+ messages in thread
From: Yinghai Lu @ 2012-09-15 23:27 UTC (permalink / raw)
To: Jiang Liu
Cc: Bjorn Helgaas, Len Brown, Tony Luck, Jiang Liu, Kenji Kaneshige,
Yijing Wang, linux-kernel, linux-pci, linux-acpi
On Fri, Sep 14, 2012 at 8:05 PM, Jiang Liu <liuj97@gmail.com> wrote:
> From: Jiang Liu <jiang.liu@huawei.com>
>
> Now ACPI devices are created before/destroyed after corresponding PCI
> devices, and acpi_platform_notify/acpi_platform_notify_remove will
> update PCI<->ACPI binding relationship when creating/destroying PCI
> devices, there's no need to invoke bind/unbind callbacks from ACPI
> device probe/destroy routines anymore. So remove bind/unbind callbacks
> from acpi_device_ops.
>
> Signed-off-by: Jiang Liu <jiang.liu@huawei.com>
> Signed-off-by: Yijing Wang <wangyijing@huawei.com>
> ---
> drivers/acpi/pci_bind.c | 100 +++++++++----------------------------------
> drivers/acpi/pci_root.c | 9 ----
> drivers/acpi/scan.c | 21 +--------
> include/acpi/acpi_bus.h | 4 --
> include/acpi/acpi_drivers.h | 1 -
> 5 files changed, 23 insertions(+), 112 deletions(-)
> diff --git a/drivers/acpi/scan.c b/drivers/acpi/scan.c
> index d1ecca2..f31cb2f 100644
> --- a/drivers/acpi/scan.c
> +++ b/drivers/acpi/scan.c
> @@ -1232,17 +1232,8 @@ static int acpi_bus_remove(struct acpi_device *dev, int rmdevice)
> dev->removal_type = ACPI_BUS_REMOVAL_EJECT;
> device_release_driver(&dev->dev);
>
> - if (!rmdevice)
> - return 0;
> -
> - /*
> - * unbind _ADR-Based Devices when hot removal
> - */
> - if (dev->flags.bus_address) {
> - if ((dev->parent) && (dev->parent->ops.unbind))
> - dev->parent->ops.unbind(dev);
> - }
> - acpi_device_unregister(dev, ACPI_BUS_REMOVAL_EJECT);
> + if (rmdevice)
> + acpi_device_unregister(dev, ACPI_BUS_REMOVAL_EJECT);
>
> return 0;
> }
for pci root bus, acpi_bus_trim() is used to remove acpi_device.
and later in acpi_pci_root_remove to stop pci drivers/ioapic driver,
iommu driver then remove pci devices.
if call back is removed there, then could some functions in
acpi_pci_unbind() will be skipped.
I really do not want to add pci_stop_bus_devices() in
pci_root_hp.c::handle_root_bridge_removal before
calling acpi_bus_trim...
Thanks
Yinghai
^ permalink raw reply [flat|nested] 26+ messages in thread* Re: [PATCH v2 5/9] ACPI/pci-bind: remove bind/unbind callbacks from acpi_device_ops
2012-09-15 23:27 ` [PATCH v2 5/9] " Yinghai Lu
@ 2012-09-17 3:03 ` Yinghai Lu
2012-09-17 14:22 ` Jiang Liu
2012-09-17 14:31 ` Jiang Liu
1 sibling, 1 reply; 26+ messages in thread
From: Yinghai Lu @ 2012-09-17 3:03 UTC (permalink / raw)
To: Jiang Liu
Cc: Bjorn Helgaas, Len Brown, Tony Luck, Jiang Liu, Kenji Kaneshige,
Yijing Wang, linux-kernel, linux-pci, linux-acpi
[-- Attachment #1: Type: text/plain, Size: 1148 bytes --]
On Sat, Sep 15, 2012 at 4:27 PM, Yinghai Lu <yinghai@kernel.org> wrote:
> On Fri, Sep 14, 2012 at 8:05 PM, Jiang Liu <liuj97@gmail.com> wrote:
>> From: Jiang Liu <jiang.liu@huawei.com>
>>
>> Now ACPI devices are created before/destroyed after corresponding PCI
>> devices, and acpi_platform_notify/acpi_platform_notify_remove will
>> update PCI<->ACPI binding relationship when creating/destroying PCI
>> devices, there's no need to invoke bind/unbind callbacks from ACPI
>> device probe/destroy routines anymore. So remove bind/unbind callbacks
>> from acpi_device_ops.
> for pci root bus, acpi_bus_trim() is used to remove acpi_device.
>
> and later in acpi_pci_root_remove to stop pci drivers/ioapic driver,
> iommu driver then remove pci devices.
>
> if call back is removed there, then could some functions in
> acpi_pci_unbind() will be skipped.
>
> I really do not want to add pci_stop_bus_devices() in
> pci_root_hp.c::handle_root_bridge_removal before
> calling acpi_bus_trim...
FYI, I solved the problem. will call apci_bus_remove() two times. it
will make sure pci devices
get removed at first before acpi devices...
Thanks
Yinghai
[-- Attachment #2: pci_root_hp_1.patch --]
[-- Type: application/octet-stream, Size: 3604 bytes --]
Subject: [PATCH] PCI, ACPI: Add pci_root_hp hot removal notification support.
Add missing hot_remove support for root device.
How to use it?
Find out root bus number to acpi root name mapping from dmesg or /sys
echo "\_SB.PCIB 3" > /sys/kernel/debug/acpi/sci_notify
to remove root bus
-v2: separate stop and remove, so could use acpi_pci_bind_notify()...
Signed-off-by: Yinghai Lu <yinghai@kernel.org>
Cc: Len Brown <lenb@kernel.org>
Cc: linux-acpi@vger.kernel.org
---
drivers/acpi/pci_root_hp.c | 67 +++++++++++++++++++++++++++++++++++++++++++++
drivers/acpi/scan.c | 5 ---
2 files changed, 68 insertions(+), 4 deletions(-)
Index: linux-2.6/drivers/acpi/pci_root_hp.c
===================================================================
--- linux-2.6.orig/drivers/acpi/pci_root_hp.c
+++ linux-2.6/drivers/acpi/pci_root_hp.c
@@ -73,6 +73,12 @@ static void add_acpi_root_bridge(acpi_ha
list_add(&bridge->list, &acpi_root_bridge_list);
}
+static void remove_acpi_root_bridge(struct acpi_root_bridge *bridge)
+{
+ list_del(&bridge->list);
+ kfree(bridge);
+}
+
struct acpi_root_hp_work {
struct work_struct work;
acpi_handle handle;
@@ -143,6 +149,61 @@ static void handle_root_bridge_insertion
printk(KERN_ERR "cannot start bridge\n");
}
+static int acpi_root_evaluate_object(acpi_handle handle, char *cmd, int val)
+{
+ acpi_status status;
+ struct acpi_object_list arg_list;
+ union acpi_object arg;
+
+ arg_list.count = 1;
+ arg_list.pointer = &arg;
+ arg.type = ACPI_TYPE_INTEGER;
+ arg.integer.value = val;
+
+ status = acpi_evaluate_object(handle, cmd, &arg_list, NULL);
+ if (ACPI_FAILURE(status)) {
+ printk(KERN_WARNING "%s: %s to %d failed\n",
+ __func__, cmd, val);
+ return -1;
+ }
+
+ return 0;
+}
+
+static void handle_root_bridge_removal(acpi_handle handle,
+ struct acpi_root_bridge *bridge)
+{
+ u32 flags = 0;
+ struct acpi_device *device;
+
+ if (bridge) {
+ flags = bridge->flags;
+ remove_acpi_root_bridge(bridge);
+ }
+
+ if (!acpi_bus_get_device(handle, &device)) {
+ int ret_val;
+
+ /* remove pci devices at first */
+ ret_val = acpi_bus_trim(device, 0);
+ printk(KERN_DEBUG "acpi_bus_trim stop return %x\n", ret_val);
+
+ /* remove acpi devices */
+ ret_val = acpi_bus_trim(device, 1);
+ printk(KERN_DEBUG "acpi_bus_trim remove return %x\n", ret_val);
+ }
+
+ if (flags & ROOT_BRIDGE_HAS_PS3) {
+ acpi_status status;
+
+ status = acpi_evaluate_object(handle, "_PS3", NULL, NULL);
+ if (ACPI_FAILURE(status))
+ printk(KERN_WARNING "%s: _PS3 failed\n", __func__);
+ }
+ if (flags & ROOT_BRIDGE_HAS_EJ0)
+ acpi_root_evaluate_object(handle, "_EJ0", 1);
+}
+
static void _handle_hotplug_event_root(struct work_struct *work)
{
struct acpi_root_bridge *bridge;
@@ -183,6 +244,12 @@ static void _handle_hotplug_event_root(s
}
break;
+ case ACPI_NOTIFY_EJECT_REQUEST:
+ /* request device eject */
+ printk(KERN_DEBUG "%s: Device eject notify on %s\n", __func__,
+ objname);
+ handle_root_bridge_removal(handle, bridge);
+ break;
default:
printk(KERN_WARNING "notify_handler: unknown event type 0x%x for %s\n",
type, objname);
Index: linux-2.6/drivers/acpi/scan.c
===================================================================
--- linux-2.6.orig/drivers/acpi/scan.c
+++ linux-2.6/drivers/acpi/scan.c
@@ -1522,10 +1522,7 @@ int acpi_bus_trim(struct acpi_device *st
child = parent;
parent = parent->parent;
- if (level == 0)
- err = acpi_bus_remove(child, rmdevice);
- else
- err = acpi_bus_remove(child, 1);
+ err = acpi_bus_remove(child, rmdevice);
continue;
}
^ permalink raw reply [flat|nested] 26+ messages in thread* Re: [PATCH v2 5/9] ACPI/pci-bind: remove bind/unbind callbacks from acpi_device_ops
2012-09-17 3:03 ` Yinghai Lu
@ 2012-09-17 14:22 ` Jiang Liu
0 siblings, 0 replies; 26+ messages in thread
From: Jiang Liu @ 2012-09-17 14:22 UTC (permalink / raw)
To: Yinghai Lu
Cc: Bjorn Helgaas, Len Brown, Tony Luck, Jiang Liu, Kenji Kaneshige,
Yijing Wang, linux-kernel, linux-pci, linux-acpi
On 09/17/2012 11:03 AM, Yinghai Lu wrote:
> On Sat, Sep 15, 2012 at 4:27 PM, Yinghai Lu <yinghai@kernel.org> wrote:
>> On Fri, Sep 14, 2012 at 8:05 PM, Jiang Liu <liuj97@gmail.com> wrote:
>>> From: Jiang Liu <jiang.liu@huawei.com>
>>>
>>> Now ACPI devices are created before/destroyed after corresponding PCI
>>> devices, and acpi_platform_notify/acpi_platform_notify_remove will
>>> update PCI<->ACPI binding relationship when creating/destroying PCI
>>> devices, there's no need to invoke bind/unbind callbacks from ACPI
>>> device probe/destroy routines anymore. So remove bind/unbind callbacks
>>> from acpi_device_ops.
>> for pci root bus, acpi_bus_trim() is used to remove acpi_device.
>>
>> and later in acpi_pci_root_remove to stop pci drivers/ioapic driver,
>> iommu driver then remove pci devices.
>>
>> if call back is removed there, then could some functions in
>> acpi_pci_unbind() will be skipped.
>>
>> I really do not want to add pci_stop_bus_devices() in
>> pci_root_hp.c::handle_root_bridge_removal before
>> calling acpi_bus_trim...
>
> FYI, I solved the problem. will call apci_bus_remove() two times. it
> will make sure pci devices
> get removed at first before acpi devices...
Hi Yinghai,
Great! So we could remove the acpi<->PCI logic now.
Thanks!
Gerry
^ permalink raw reply [flat|nested] 26+ messages in thread
* Re: [PATCH v2 5/9] ACPI/pci-bind: remove bind/unbind callbacks from acpi_device_ops
2012-09-15 23:27 ` [PATCH v2 5/9] " Yinghai Lu
2012-09-17 3:03 ` Yinghai Lu
@ 2012-09-17 14:31 ` Jiang Liu
2012-09-17 15:41 ` Yinghai Lu
1 sibling, 1 reply; 26+ messages in thread
From: Jiang Liu @ 2012-09-17 14:31 UTC (permalink / raw)
To: Yinghai Lu
Cc: Bjorn Helgaas, Len Brown, Tony Luck, Jiang Liu, Kenji Kaneshige,
Yijing Wang, linux-kernel, linux-pci, linux-acpi
On 09/16/2012 07:27 AM, Yinghai Lu wrote:
> On Fri, Sep 14, 2012 at 8:05 PM, Jiang Liu <liuj97@gmail.com> wrote:
>> From: Jiang Liu <jiang.liu@huawei.com>
>>
>> Now ACPI devices are created before/destroyed after corresponding PCI
>> devices, and acpi_platform_notify/acpi_platform_notify_remove will
>> update PCI<->ACPI binding relationship when creating/destroying PCI
>> devices, there's no need to invoke bind/unbind callbacks from ACPI
>> device probe/destroy routines anymore. So remove bind/unbind callbacks
>> from acpi_device_ops.
>>
>> Signed-off-by: Jiang Liu <jiang.liu@huawei.com>
>> Signed-off-by: Yijing Wang <wangyijing@huawei.com>
>> ---
>> drivers/acpi/pci_bind.c | 100 +++++++++----------------------------------
>> drivers/acpi/pci_root.c | 9 ----
>> drivers/acpi/scan.c | 21 +--------
>> include/acpi/acpi_bus.h | 4 --
>> include/acpi/acpi_drivers.h | 1 -
>> 5 files changed, 23 insertions(+), 112 deletions(-)
>> diff --git a/drivers/acpi/scan.c b/drivers/acpi/scan.c
>> index d1ecca2..f31cb2f 100644
>> --- a/drivers/acpi/scan.c
>> +++ b/drivers/acpi/scan.c
>> @@ -1232,17 +1232,8 @@ static int acpi_bus_remove(struct acpi_device *dev, int rmdevice)
>> dev->removal_type = ACPI_BUS_REMOVAL_EJECT;
>> device_release_driver(&dev->dev);
>>
>> - if (!rmdevice)
>> - return 0;
>> -
>> - /*
>> - * unbind _ADR-Based Devices when hot removal
>> - */
>> - if (dev->flags.bus_address) {
>> - if ((dev->parent) && (dev->parent->ops.unbind))
>> - dev->parent->ops.unbind(dev);
>> - }
>> - acpi_device_unregister(dev, ACPI_BUS_REMOVAL_EJECT);
>> + if (rmdevice)
>> + acpi_device_unregister(dev, ACPI_BUS_REMOVAL_EJECT);
>>
>> return 0;
>> }
>
> for pci root bus, acpi_bus_trim() is used to remove acpi_device.
>
> and later in acpi_pci_root_remove to stop pci drivers/ioapic driver,
> iommu driver then remove pci devices.
>
> if call back is removed there, then could some functions in
> acpi_pci_unbind() will be skipped.
>
> I really do not want to add pci_stop_bus_devices() in
> pci_root_hp.c::handle_root_bridge_removal before
> calling acpi_bus_trim...
Hi Yinghai,
We are working on a ACPI based hotplug framework for CPU, memory and IOH.
With our framework, the sequence to remove a IOH is:
1) unbind all PCI device drivers for affected PCI devices.
2) stop and remove all affected PCI devices.
3) stop ACPI ioapic drivers(a new driver like PCI version).
4) destroy all ACPI devices.
--Gerry
>
> Thanks
>
> Yinghai
>
^ permalink raw reply [flat|nested] 26+ messages in thread* Re: [PATCH v2 5/9] ACPI/pci-bind: remove bind/unbind callbacks from acpi_device_ops
2012-09-17 14:31 ` Jiang Liu
@ 2012-09-17 15:41 ` Yinghai Lu
0 siblings, 0 replies; 26+ messages in thread
From: Yinghai Lu @ 2012-09-17 15:41 UTC (permalink / raw)
To: Jiang Liu
Cc: Bjorn Helgaas, Len Brown, Tony Luck, Jiang Liu, Kenji Kaneshige,
Yijing Wang, linux-kernel, linux-pci, linux-acpi
On Mon, Sep 17, 2012 at 7:31 AM, Jiang Liu <liuj97@gmail.com> wrote:
> On 09/16/2012 07:27 AM, Yinghai Lu wrote:
>>> --- a/drivers/acpi/scan.c
>>> +++ b/drivers/acpi/scan.c
>>> @@ -1232,17 +1232,8 @@ static int acpi_bus_remove(struct acpi_device *dev, int rmdevice)
>>> dev->removal_type = ACPI_BUS_REMOVAL_EJECT;
>>> device_release_driver(&dev->dev);
>>>
>>> - if (!rmdevice)
>>> - return 0;
>>> -
>>> - /*
>>> - * unbind _ADR-Based Devices when hot removal
>>> - */
>>> - if (dev->flags.bus_address) {
>>> - if ((dev->parent) && (dev->parent->ops.unbind))
>>> - dev->parent->ops.unbind(dev);
>>> - }
>>> - acpi_device_unregister(dev, ACPI_BUS_REMOVAL_EJECT);
>>> + if (rmdevice)
>>> + acpi_device_unregister(dev, ACPI_BUS_REMOVAL_EJECT);
>>>
>>> return 0;
>>> }
>>
>> for pci root bus, acpi_bus_trim() is used to remove acpi_device.
>>
>> and later in acpi_pci_root_remove to stop pci drivers/ioapic driver,
>> iommu driver then remove pci devices.
>>
>> if call back is removed there, then could some functions in
>> acpi_pci_unbind() will be skipped.
>>
>> I really do not want to add pci_stop_bus_devices() in
>> pci_root_hp.c::handle_root_bridge_removal before
>> calling acpi_bus_trim...
> Hi Yinghai,
> We are working on a ACPI based hotplug framework for CPU, memory and IOH.
> With our framework, the sequence to remove a IOH is:
> 1) unbind all PCI device drivers for affected PCI devices.
> 2) stop and remove all affected PCI devices.
> 3) stop ACPI ioapic drivers(a new driver like PCI version).
> 4) destroy all ACPI devices.
i solved the problem...
http://git.kernel.org/?p=linux/kernel/git/yinghai/linux-yinghai.git;a=commitdiff;h=15967ed108f9543c160422a7aaec18de30a53373
+ if (!acpi_bus_get_device(handle, &device)) {
+ int ret_val;
+
+ /* remove pci devices at first */
+ ret_val = acpi_bus_trim(device, 0);
+ printk(KERN_DEBUG "acpi_bus_trim stop return %x\n", ret_val);
+
+ /* remove acpi devices */
+ ret_val = acpi_bus_trim(device, 1);
+ printk(KERN_DEBUG "acpi_bus_trim remove return %x\n", ret_val);
+ }
...
also need to update acpi_bus_trim
http://git.kernel.org/?p=linux/kernel/git/yinghai/linux-yinghai.git;a=commitdiff;h=c8805fe6ec46c46f352cdbffb13191116ea794ba
BTW, memory hot-remove may need some work.
Thanks
Yinghai
^ permalink raw reply [flat|nested] 26+ messages in thread
* [PATCH v2 6/9] ACPI/pci_slot: update PCI slot information when PCI hotplug event happens
2012-09-15 3:05 [PATCH v2 0/9] enhance PCI related drivers to handle hotplug events Jiang Liu
` (4 preceding siblings ...)
2012-09-15 3:05 ` [PATCH v2 5/9] ACPI/pci-bind: remove bind/unbind callbacks from acpi_device_ops Jiang Liu
@ 2012-09-15 3:05 ` Jiang Liu
2012-09-15 3:05 ` [PATCH v2 7/9] PCI/acpiphp: update ACPI hotplug slot information when PCI hotplug happens Jiang Liu
` (2 subsequent siblings)
8 siblings, 0 replies; 26+ messages in thread
From: Jiang Liu @ 2012-09-15 3:05 UTC (permalink / raw)
To: Bjorn Helgaas, Len Brown
Cc: Tony Luck, Jiang Liu, Yinghai Lu, Kenji Kaneshige, Yijing Wang,
Jiang Liu, linux-kernel, linux-pci, linux-acpi
Currently the pci_slot driver doesn't update PCI slot information
when PCI device hotplug event happens, which may cause memory leak
and returning stale information to user.
So hook the BUS_NOTIFY_ADD_DEVICE/BUS_NOTIFY_DEL_DEVICE events to
update PCI slot information when PCI hotplug event happens.
Signed-off-by: Jiang Liu <jiang.liu@huawei.com>
Signed-off-by: Yijing Wang <wangyijing@huawei.com>
---
drivers/acpi/pci_slot.c | 86 ++++++++++++++++++++++++++++++++++++++++++-----
1 file changed, 78 insertions(+), 8 deletions(-)
diff --git a/drivers/acpi/pci_slot.c b/drivers/acpi/pci_slot.c
index e50e31a..96dcc19 100644
--- a/drivers/acpi/pci_slot.c
+++ b/drivers/acpi/pci_slot.c
@@ -32,6 +32,7 @@
#include <linux/acpi.h>
#include <acpi/acpi_bus.h>
#include <acpi/acpi_drivers.h>
+#include <linux/pci-acpi.h>
#include <linux/dmi.h>
static bool debug;
@@ -123,12 +124,7 @@ struct callback_args {
/*
* register_slot
*
- * Called once for each SxFy object in the namespace. Don't worry about
- * calling pci_create_slot multiple times for the same pci_bus:device,
- * since each subsequent call simply bumps the refcount on the pci_slot.
- *
- * The number of calls to pci_destroy_slot from unregister_slot is
- * symmetrical.
+ * Called once for each SxFy object in the namespace.
*/
static acpi_status
register_slot(acpi_handle handle, u32 lvl, void *context, void **rv)
@@ -145,6 +141,15 @@ register_slot(acpi_handle handle, u32 lvl, void *context, void **rv)
if (device < 0)
return AE_OK;
+ /* Avoid duplicated records for the same slot */
+ list_for_each_entry(slot, &slot_list, list) {
+ pci_slot = slot->pci_slot;
+ if (pci_slot && pci_slot->bus == pci_bus &&
+ pci_slot->number == device) {
+ return AE_OK;
+ }
+ }
+
slot = kmalloc(sizeof(*slot), GFP_KERNEL);
if (!slot) {
err("%s: cannot allocate memory\n", __func__);
@@ -162,9 +167,7 @@ register_slot(acpi_handle handle, u32 lvl, void *context, void **rv)
slot->root_handle = parent_context->root_handle;
slot->pci_slot = pci_slot;
INIT_LIST_HEAD(&slot->list);
- mutex_lock(&slot_list_lock);
list_add(&slot->list, &slot_list);
- mutex_unlock(&slot_list_lock);
get_device(&pci_bus->dev);
@@ -299,7 +302,9 @@ acpi_pci_slot_add(acpi_handle handle)
{
acpi_status status;
+ mutex_lock(&slot_list_lock);
status = walk_root_bridge(handle, register_slot);
+ mutex_unlock(&slot_list_lock);
if (ACPI_FAILURE(status))
err("%s: register_slot failure - %d\n", __func__, status);
@@ -354,17 +359,82 @@ static struct dmi_system_id acpi_pci_slot_dmi_table[] __initdata = {
{}
};
+static void acpi_pci_slot_notify_add(struct pci_dev *dev)
+{
+ acpi_handle handle;
+ struct callback_args context;
+
+ if (!dev->subordinate)
+ return;
+
+ mutex_lock(&slot_list_lock);
+ handle = DEVICE_ACPI_HANDLE(&dev->dev);
+ context.root_handle = acpi_find_root_bridge_handle(dev);
+ if (handle && context.root_handle) {
+ context.pci_bus = dev->subordinate;
+ context.user_function = register_slot;
+ acpi_walk_namespace(ACPI_TYPE_DEVICE, handle, (u32)1,
+ register_slot, NULL, &context, NULL);
+ }
+ mutex_unlock(&slot_list_lock);
+}
+
+static void acpi_pci_slot_notify_del(struct pci_dev *dev)
+{
+ struct acpi_pci_slot *slot, *tmp;
+ struct pci_bus *bus = dev->subordinate;
+
+ if (!bus)
+ return;
+
+ mutex_lock(&slot_list_lock);
+ list_for_each_entry_safe(slot, tmp, &slot_list, list)
+ if (slot->pci_slot && slot->pci_slot->bus == bus) {
+ list_del(&slot->list);
+ pci_destroy_slot(slot->pci_slot);
+ put_device(&bus->dev);
+ kfree(slot);
+ }
+ mutex_unlock(&slot_list_lock);
+}
+
+static int acpi_pci_slot_notify_fn(struct notifier_block *nb,
+ unsigned long event, void *data)
+{
+ struct device *dev = data;
+
+ switch (event) {
+ case BUS_NOTIFY_ADD_DEVICE:
+ acpi_pci_slot_notify_add(to_pci_dev(dev));
+ break;
+ case BUS_NOTIFY_DEL_DEVICE:
+ acpi_pci_slot_notify_del(to_pci_dev(dev));
+ break;
+ default:
+ return NOTIFY_DONE;
+ }
+
+ return NOTIFY_OK;
+}
+
+static struct notifier_block acpi_pci_slot_notifier = {
+ .notifier_call = &acpi_pci_slot_notify_fn,
+};
+
static int __init
acpi_pci_slot_init(void)
{
dmi_check_system(acpi_pci_slot_dmi_table);
acpi_pci_register_driver(&acpi_pci_slot_driver);
+ bus_register_notifier(&pci_bus_type, &acpi_pci_slot_notifier);
+
return 0;
}
static void __exit
acpi_pci_slot_exit(void)
{
+ bus_unregister_notifier(&pci_bus_type, &acpi_pci_slot_notifier);
acpi_pci_unregister_driver(&acpi_pci_slot_driver);
}
--
1.7.9.5
^ permalink raw reply related [flat|nested] 26+ messages in thread* [PATCH v2 7/9] PCI/acpiphp: update ACPI hotplug slot information when PCI hotplug happens
2012-09-15 3:05 [PATCH v2 0/9] enhance PCI related drivers to handle hotplug events Jiang Liu
` (5 preceding siblings ...)
2012-09-15 3:05 ` [PATCH v2 6/9] ACPI/pci_slot: update PCI slot information when PCI hotplug event happens Jiang Liu
@ 2012-09-15 3:05 ` Jiang Liu
2012-09-15 3:05 ` [PATCH v2 8/9] PCI/acpiphp: serialize access to the bridge_list list Jiang Liu
2012-09-15 3:05 ` [PATCH v2 9/9] PCI/AER: update AER configuration when PCI hotplug event happens Jiang Liu
8 siblings, 0 replies; 26+ messages in thread
From: Jiang Liu @ 2012-09-15 3:05 UTC (permalink / raw)
To: Bjorn Helgaas
Cc: Tony Luck, Jiang Liu, Yinghai Lu, Kenji Kaneshige, Yijing Wang,
Jiang Liu, linux-kernel, linux-pci
Currently the acpiphp driver fails to update hotplug slot information under
several conditions, such as:
1) The bridge device is removed through /sys/bus/pci/devices/.../remove
2) The bridge device is added/removed by PCI hotplug driver other than the
acpiphp driver itself. For example, if an IO expansion box with ACPI
hotplug slots available is hot-added by the pciehp driver, all ACPI
hotplug slots won't be discovered by the acpiphp driver.
So hook the BUS_NOTIFY_ADD_DEVICE/BUS_NOTIFY_DEL_DEVICE events to
update ACPI hotplug slot information when PCI hotplug event happens.
Signed-off-by: Jiang Liu <jiang.liu@huawei.com>
Signed-off-by: Yijing Wang <wangyijing@huawei.com>
---
drivers/pci/hotplug/acpiphp_glue.c | 64 ++++++++++++++++++++++++++++++++++--
1 file changed, 61 insertions(+), 3 deletions(-)
diff --git a/drivers/pci/hotplug/acpiphp_glue.c b/drivers/pci/hotplug/acpiphp_glue.c
index 7be4ca5..1fb0eb7 100644
--- a/drivers/pci/hotplug/acpiphp_glue.c
+++ b/drivers/pci/hotplug/acpiphp_glue.c
@@ -61,6 +61,7 @@ static void handle_hotplug_event_bridge (acpi_handle, u32, void *);
static void acpiphp_sanitize_bus(struct pci_bus *bus);
static void acpiphp_set_hpp_values(struct pci_bus *bus);
static void handle_hotplug_event_func(acpi_handle handle, u32 type, void *context);
+static struct acpiphp_bridge *acpiphp_handle_to_bridge(acpi_handle handle);
/* callback routine to check for the existence of a pci dock device */
static acpi_status
@@ -382,7 +383,7 @@ 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)
+static void add_host_bridge(acpi_handle handle)
{
struct acpiphp_bridge *bridge;
struct acpi_pci_root *root = acpi_pci_find_root(handle);
@@ -399,12 +400,14 @@ static void add_host_bridge(acpi_handle *handle)
init_bridge_misc(bridge);
}
-
/* allocate and initialize PCI-to-PCI bridge data structure */
-static void add_p2p_bridge(acpi_handle *handle)
+static void add_p2p_bridge(acpi_handle handle)
{
struct acpiphp_bridge *bridge;
+ if (acpiphp_handle_to_bridge(handle))
+ return;
+
bridge = kzalloc(sizeof(struct acpiphp_bridge), GFP_KERNEL);
if (bridge == NULL) {
err("out of memory\n");
@@ -1405,6 +1408,58 @@ find_root_bridges(acpi_handle handle, u32 lvl, void *context, void **rv)
return AE_OK ;
}
+static void acpiphp_hp_notify_add(struct pci_dev *dev)
+{
+ acpi_handle handle = DEVICE_ACPI_HANDLE(&dev->dev);
+
+ if (!dev->subordinate || !handle)
+ return;
+
+ /* check if this bridge has ejectable slots */
+ if (detect_ejectable_slots(handle) > 0) {
+ dbg("found PCI-to-PCI bridge at PCI %s\n", pci_name(dev));
+ add_p2p_bridge(handle);
+ }
+}
+
+static void acpiphp_hp_notify_del(struct pci_dev *dev)
+{
+ struct acpiphp_bridge *bridge, *tmp;
+ struct pci_bus *bus = dev->subordinate;
+
+ if (!bus)
+ return;
+
+ list_for_each_entry_safe(bridge, tmp, &bridge_list, list)
+ if (bridge->pci_bus == bus) {
+ cleanup_bridge(bridge);
+ break;
+ }
+}
+
+static int acpi_pci_hp_notify_fn(struct notifier_block *nb,
+ unsigned long event, void *data)
+{
+ struct device *dev = data;
+
+ switch (event) {
+ case BUS_NOTIFY_ADD_DEVICE:
+ acpiphp_hp_notify_add(to_pci_dev(dev));
+ break;
+ case BUS_NOTIFY_DEL_DEVICE:
+ acpiphp_hp_notify_del(to_pci_dev(dev));
+ break;
+ default:
+ return NOTIFY_DONE;
+ }
+
+ return NOTIFY_OK;
+}
+
+static struct notifier_block acpi_pci_hp_notifier = {
+ .notifier_call = &acpi_pci_hp_notify_fn,
+};
+
static struct acpi_pci_driver acpi_pci_hp_driver = {
.add = add_bridge,
.remove = remove_bridge,
@@ -1425,6 +1480,8 @@ int __init acpiphp_glue_init(void)
else
acpi_pci_register_driver(&acpi_pci_hp_driver);
+ bus_register_notifier(&pci_bus_type, &acpi_pci_hp_notifier);
+
return 0;
}
@@ -1436,6 +1493,7 @@ int __init acpiphp_glue_init(void)
*/
void acpiphp_glue_exit(void)
{
+ bus_unregister_notifier(&pci_bus_type, &acpi_pci_hp_notifier);
acpi_pci_unregister_driver(&acpi_pci_hp_driver);
}
--
1.7.9.5
^ permalink raw reply related [flat|nested] 26+ messages in thread* [PATCH v2 8/9] PCI/acpiphp: serialize access to the bridge_list list
2012-09-15 3:05 [PATCH v2 0/9] enhance PCI related drivers to handle hotplug events Jiang Liu
` (6 preceding siblings ...)
2012-09-15 3:05 ` [PATCH v2 7/9] PCI/acpiphp: update ACPI hotplug slot information when PCI hotplug happens Jiang Liu
@ 2012-09-15 3:05 ` Jiang Liu
2012-09-15 3:05 ` [PATCH v2 9/9] PCI/AER: update AER configuration when PCI hotplug event happens Jiang Liu
8 siblings, 0 replies; 26+ messages in thread
From: Jiang Liu @ 2012-09-15 3:05 UTC (permalink / raw)
To: Bjorn Helgaas
Cc: Tony Luck, Jiang Liu, Yinghai Lu, Kenji Kaneshige, Yijing Wang,
Jiang Liu, linux-kernel, linux-pci
Serialize access to the bridge_list in the acpiphp driver.
Signed-off-by: Jiang Liu <jiang.liu@huawei.com>
Signed-off-by: Yijing Wang <wangyijing@huawei.com>
---
drivers/pci/hotplug/acpiphp_glue.c | 11 +++++++++++
1 file changed, 11 insertions(+)
diff --git a/drivers/pci/hotplug/acpiphp_glue.c b/drivers/pci/hotplug/acpiphp_glue.c
index 1fb0eb7..348afd7 100644
--- a/drivers/pci/hotplug/acpiphp_glue.c
+++ b/drivers/pci/hotplug/acpiphp_glue.c
@@ -53,6 +53,7 @@
#include "../pci.h"
#include "acpiphp.h"
+static DEFINE_MUTEX(bridge_mutex);
static LIST_HEAD(bridge_list);
#define MY_NAME "acpiphp_glue"
@@ -497,8 +498,10 @@ static int add_bridge(acpi_handle handle)
}
/* search P2P bridges under this host bridge */
+ mutex_lock(&bridge_mutex);
status = acpi_walk_namespace(ACPI_TYPE_DEVICE, handle, (u32)1,
find_p2p_bridge, NULL, NULL, NULL);
+ mutex_unlock(&bridge_mutex);
if (ACPI_FAILURE(status))
warn("find_p2p_bridge failed (error code = 0x%x)\n", status);
@@ -595,6 +598,8 @@ static void remove_bridge(acpi_handle handle)
{
struct acpiphp_bridge *bridge;
+ mutex_lock(&bridge_mutex);
+
/* cleanup p2p bridges under this host bridge
in a depth-first manner */
acpi_walk_namespace(ACPI_TYPE_DEVICE, handle,
@@ -613,6 +618,8 @@ static void remove_bridge(acpi_handle handle)
else
acpi_remove_notify_handler(handle, ACPI_SYSTEM_NOTIFY,
handle_hotplug_event_bridge);
+
+ mutex_unlock(&bridge_mutex);
}
static int power_on_slot(struct acpiphp_slot *slot)
@@ -1415,11 +1422,13 @@ static void acpiphp_hp_notify_add(struct pci_dev *dev)
if (!dev->subordinate || !handle)
return;
+ mutex_lock(&bridge_mutex);
/* check if this bridge has ejectable slots */
if (detect_ejectable_slots(handle) > 0) {
dbg("found PCI-to-PCI bridge at PCI %s\n", pci_name(dev));
add_p2p_bridge(handle);
}
+ mutex_unlock(&bridge_mutex);
}
static void acpiphp_hp_notify_del(struct pci_dev *dev)
@@ -1430,11 +1439,13 @@ static void acpiphp_hp_notify_del(struct pci_dev *dev)
if (!bus)
return;
+ mutex_lock(&bridge_mutex);
list_for_each_entry_safe(bridge, tmp, &bridge_list, list)
if (bridge->pci_bus == bus) {
cleanup_bridge(bridge);
break;
}
+ mutex_unlock(&bridge_mutex);
}
static int acpi_pci_hp_notify_fn(struct notifier_block *nb,
--
1.7.9.5
^ permalink raw reply related [flat|nested] 26+ messages in thread* [PATCH v2 9/9] PCI/AER: update AER configuration when PCI hotplug event happens
2012-09-15 3:05 [PATCH v2 0/9] enhance PCI related drivers to handle hotplug events Jiang Liu
` (7 preceding siblings ...)
2012-09-15 3:05 ` [PATCH v2 8/9] PCI/acpiphp: serialize access to the bridge_list list Jiang Liu
@ 2012-09-15 3:05 ` Jiang Liu
8 siblings, 0 replies; 26+ messages in thread
From: Jiang Liu @ 2012-09-15 3:05 UTC (permalink / raw)
To: Bjorn Helgaas
Cc: Tony Luck, Yijing Wang, Yinghai Lu, Kenji Kaneshige, Jiang Liu,
linux-kernel, linux-pci, Jiang Liu
From: Yijing Wang <wangyijing@huawei.com>
The AER driver only configures downstream PCIe devices at driver
binding time and all hot-added PCIe devices won't be managed by
the AER driver. So hook PCIe device hotplug events to setup AER
configuration for hot-added PCIe devices.
Signed-off-by: Yijing Wang <wangyijing@huawei.com>
Signed-off-by: Jiang Liu <jiang.liu@huawei.com>
---
drivers/pci/pcie/aer/aerdrv.c | 63 ++++++++++++++++++++++++++++++++++++++++-
1 file changed, 62 insertions(+), 1 deletion(-)
diff --git a/drivers/pci/pcie/aer/aerdrv.c b/drivers/pci/pcie/aer/aerdrv.c
index 030cf12..b70ec84 100644
--- a/drivers/pci/pcie/aer/aerdrv.c
+++ b/drivers/pci/pcie/aer/aerdrv.c
@@ -408,6 +408,55 @@ static void aer_error_resume(struct pci_dev *dev)
pci_write_config_dword(dev, pos + PCI_ERR_UNCOR_STATUS, status);
}
+static void acpi_pcie_aer_notify_dev(struct pcie_device *dev, bool enable)
+{
+ int rc, pos;
+ u32 val = 0;
+ struct pci_dev *pdev = dev->port;
+
+ if (dev->service != PCIE_PORT_SERVICE_AER)
+ return;
+ if (pci_pcie_type(pdev) == PCI_EXP_TYPE_ROOT_PORT)
+ return;
+ while (pci_pcie_type(pdev) != PCI_EXP_TYPE_ROOT_PORT) {
+ pdev = pdev->bus->self;
+ if (!pdev)
+ return;
+ }
+
+ pos = pci_find_ext_capability(pdev, PCI_EXT_CAP_ID_ERR);
+ if (!pos)
+ return;
+ rc = pci_read_config_dword(pdev, pos + PCI_ERR_ROOT_COMMAND, &val);
+ if (rc || val == 0xFFFFFFFF || !(val & ROOT_PORT_INTR_ON_MESG_MASK))
+ return;
+
+ set_device_error_reporting(dev->port, &enable);
+}
+
+static int acpi_pcie_aer_notify(struct notifier_block *nb,
+ unsigned long event, void *data)
+{
+ struct device *dev = data;
+
+ switch (event) {
+ case BUS_NOTIFY_ADD_DEVICE:
+ acpi_pcie_aer_notify_dev(to_pcie_device(dev), true);
+ break;
+ case BUS_NOTIFY_DEL_DEVICE:
+ acpi_pcie_aer_notify_dev(to_pcie_device(dev), false);
+ break;
+ default:
+ return NOTIFY_DONE;
+ }
+
+ return NOTIFY_OK;
+}
+
+static struct notifier_block acpi_pcie_aer_notifier = {
+ .notifier_call = &acpi_pcie_aer_notify,
+};
+
/**
* aer_service_init - register AER root service driver
*
@@ -415,9 +464,20 @@ static void aer_error_resume(struct pci_dev *dev)
*/
static int __init aer_service_init(void)
{
+ int ret;
+
if (!pci_aer_available() || aer_acpi_firmware_first())
return -ENXIO;
- return pcie_port_service_register(&aerdriver);
+
+ ret = pcie_port_service_register(&aerdriver);
+ if (!ret) {
+ ret = bus_register_notifier(&pcie_port_bus_type,
+ &acpi_pcie_aer_notifier);
+ if (ret)
+ pcie_port_service_unregister(&aerdriver);
+ }
+
+ return ret;
}
/**
@@ -427,6 +487,7 @@ static int __init aer_service_init(void)
*/
static void __exit aer_service_exit(void)
{
+ bus_unregister_notifier(&pcie_port_bus_type, &acpi_pcie_aer_notifier);
pcie_port_service_unregister(&aerdriver);
}
--
1.7.9.5
^ permalink raw reply related [flat|nested] 26+ messages in thread