* [PATCH 1/1] Update acpi_root_bridge_list in container hotplug path.
@ 2012-10-17 3:25 Tang Chen
2012-10-17 5:18 ` Yinghai Lu
0 siblings, 1 reply; 8+ messages in thread
From: Tang Chen @ 2012-10-17 3:25 UTC (permalink / raw)
To: yinghai, jiang.liu, bhelgaas, lenb, izumi.taku, isimatu.yasuaki,
linux-acpi, linux-pci, linux-kernel
Cc: Tang Chen
Hi Yinghai,
List acpi_root_bridge_list is only updated when kernel is booting,
or in _handle_hotplug_event_root() when handling ACPI_NOTIFY_DEVICE_CHECK
event on a pci root bridge device. But when we hotplug a container, which
contains one or more pci root bridges, container_notify_cb() will be
called but not _handle_hotplug_event_root(). As a result,
acpi_root_bridge_list won't be updated.
This patch makes the following api and struct public in pci_root_hp.h,
struct acpi_root_bridge;
add_acpi_root_bridge()
remove_acpi_root_bridge()
acpi_root_handle_to_bridge()
and call add_acpi_root_bridge() in acpi_bus_check_add() and call
remove_acpi_root_bridge() in acpi_bus_remove().
This patch is based on Lu Yinghai's git tree branch for-pci-split-pci-root-hp-2.
Signed-off-by: Tang Chen <tangchen@cn.fujitsu.com>
---
drivers/acpi/pci_root_hp.c | 20 ++++++--------------
drivers/acpi/scan.c | 18 ++++++++++++++++--
include/acpi/pci_root_hp.h | 13 +++++++++++++
3 files changed, 35 insertions(+), 16 deletions(-)
create mode 100644 include/acpi/pci_root_hp.h
diff --git a/drivers/acpi/pci_root_hp.c b/drivers/acpi/pci_root_hp.c
index 01e71f6..6381a26 100644
--- a/drivers/acpi/pci_root_hp.c
+++ b/drivers/acpi/pci_root_hp.c
@@ -31,7 +31,7 @@ static const struct acpi_device_id root_device_ids[] = {
#define ACPI_STA_FUNCTIONING (0x00000008)
-static struct acpi_root_bridge *acpi_root_handle_to_bridge(acpi_handle handle)
+struct acpi_root_bridge *acpi_root_handle_to_bridge(acpi_handle handle)
{
struct acpi_root_bridge *bridge;
@@ -43,7 +43,7 @@ static struct acpi_root_bridge *acpi_root_handle_to_bridge(acpi_handle handle)
}
/* allocate and initialize host bridge data structure */
-static void add_acpi_root_bridge(acpi_handle handle)
+void add_acpi_root_bridge(acpi_handle handle)
{
struct acpi_root_bridge *bridge;
acpi_handle dummy_handle;
@@ -79,7 +79,7 @@ static void add_acpi_root_bridge(acpi_handle handle)
list_add(&bridge->list, &acpi_root_bridge_list);
}
-static void remove_acpi_root_bridge(struct acpi_root_bridge *bridge)
+void remove_acpi_root_bridge(struct acpi_root_bridge *bridge)
{
list_del(&bridge->list);
kfree(bridge);
@@ -172,10 +172,8 @@ static void handle_root_bridge_removal(acpi_handle handle,
u32 flags = 0;
struct acpi_device *device;
- if (bridge) {
+ if (bridge)
flags = bridge->flags;
- remove_acpi_root_bridge(bridge);
- }
if (!acpi_bus_get_device(handle, &device)) {
int ret_val;
@@ -223,10 +221,8 @@ static void _handle_hotplug_event_root(struct work_struct *work)
/* bus enumerate */
printk(KERN_DEBUG "%s: Bus check notify on %s\n", __func__,
objname);
- if (!bridge) {
+ if (!bridge)
handle_root_bridge_insertion(handle);
- add_acpi_root_bridge(handle);
- }
break;
@@ -234,10 +230,8 @@ static void _handle_hotplug_event_root(struct work_struct *work)
/* device check */
printk(KERN_DEBUG "%s: Device check notify on %s\n", __func__,
objname);
- if (!bridge) {
+ if (!bridge)
handle_root_bridge_insertion(handle);
- add_acpi_root_bridge(handle);
- }
break;
case ACPI_NOTIFY_EJECT_REQUEST:
@@ -304,8 +298,6 @@ find_root_bridges(acpi_handle handle, u32 lvl, void *context, void **rv)
printk(KERN_DEBUG "acpi root: %s notify handler is installed\n",
objname);
- add_acpi_root_bridge(handle);
-
return AE_OK;
}
diff --git a/drivers/acpi/scan.c b/drivers/acpi/scan.c
index 6ca2eaf..c258064 100644
--- a/drivers/acpi/scan.c
+++ b/drivers/acpi/scan.c
@@ -12,6 +12,7 @@
#include <linux/dmi.h>
#include <acpi/acpi_drivers.h>
+#include <acpi/pci_root_hp.h>
#include "internal.h"
@@ -1265,8 +1266,17 @@ int acpi_bus_remove(struct acpi_device *dev, int rmdevice)
dev->removal_type = ACPI_BUS_REMOVAL_EJECT;
device_release_driver(&dev->dev);
- if (rmdevice)
+ if (rmdevice) {
+ if (acpi_is_root_bridge(dev->handle)) {
+ struct acpi_root_bridge *bridge;
+
+ bridge = acpi_root_handle_to_bridge(dev->handle);
+ if (bridge)
+ remove_acpi_root_bridge(bridge);
+ }
+
acpi_device_unregister(dev, ACPI_BUS_REMOVAL_EJECT);
+ }
return 0;
}
@@ -1448,9 +1458,13 @@ static acpi_status acpi_bus_check_add(acpi_handle handle, u32 lvl,
*/
device = NULL;
acpi_bus_get_device(handle, &device);
- if (ops->acpi_op_add && !device)
+ if (ops->acpi_op_add && !device) {
acpi_add_single_object(&device, handle, type, sta, ops);
+ if (acpi_is_root_bridge(handle))
+ add_acpi_root_bridge(handle);
+ }
+
if (!device)
return AE_CTRL_DEPTH;
diff --git a/include/acpi/pci_root_hp.h b/include/acpi/pci_root_hp.h
new file mode 100644
index 0000000..2761add
--- /dev/null
+++ b/include/acpi/pci_root_hp.h
@@ -0,0 +1,13 @@
+
+/*PCI root bridge hotplug API */
+
+#ifndef __PCI_ROOT_HP__
+#define __PCI_ROOT_HP__
+
+struct acpi_root_bridge;
+
+void add_acpi_root_bridge(acpi_handle handle);
+void remove_acpi_root_bridge(struct acpi_root_bridge *bridge);
+struct acpi_root_bridge *acpi_root_handle_to_bridge(acpi_handle handle);
+
+#endif /* __PCI_ROOT_HP_H__ */
--
1.7.1
^ permalink raw reply related [flat|nested] 8+ messages in thread
* Re: [PATCH 1/1] Update acpi_root_bridge_list in container hotplug path.
2012-10-17 3:25 [PATCH 1/1] Update acpi_root_bridge_list in container hotplug path Tang Chen
@ 2012-10-17 5:18 ` Yinghai Lu
2012-10-17 7:39 ` Tang Chen
0 siblings, 1 reply; 8+ messages in thread
From: Yinghai Lu @ 2012-10-17 5:18 UTC (permalink / raw)
To: Tang Chen
Cc: jiang.liu, bhelgaas, lenb, izumi.taku, isimatu.yasuaki,
linux-acpi, linux-pci, linux-kernel
[-- Attachment #1: Type: text/plain, Size: 2068 bytes --]
On Tue, Oct 16, 2012 at 8:25 PM, Tang Chen <tangchen@cn.fujitsu.com> wrote:
> Hi Yinghai,
>
> List acpi_root_bridge_list is only updated when kernel is booting,
> or in _handle_hotplug_event_root() when handling ACPI_NOTIFY_DEVICE_CHECK
> event on a pci root bridge device. But when we hotplug a container, which
> contains one or more pci root bridges, container_notify_cb() will be
> called but not _handle_hotplug_event_root(). As a result,
> acpi_root_bridge_list won't be updated.
>
> This patch makes the following api and struct public in pci_root_hp.h,
> struct acpi_root_bridge;
> add_acpi_root_bridge()
> remove_acpi_root_bridge()
> acpi_root_handle_to_bridge()
> and call add_acpi_root_bridge() in acpi_bus_check_add() and call
> remove_acpi_root_bridge() in acpi_bus_remove().
>
> This patch is based on Lu Yinghai's git tree branch for-pci-split-pci-root-hp-2.
>
> Signed-off-by: Tang Chen <tangchen@cn.fujitsu.com>
> ---
> diff --git a/drivers/acpi/scan.c b/drivers/acpi/scan.c
> index 6ca2eaf..c258064 100644
> --- a/drivers/acpi/scan.c
> +++ b/drivers/acpi/scan.c
> @@ -12,6 +12,7 @@
> #include <linux/dmi.h>
>
> #include <acpi/acpi_drivers.h>
> +#include <acpi/pci_root_hp.h>
>
> #include "internal.h"
>
> @@ -1265,8 +1266,17 @@ int acpi_bus_remove(struct acpi_device *dev, int rmdevice)
> dev->removal_type = ACPI_BUS_REMOVAL_EJECT;
> device_release_driver(&dev->dev);
>
> - if (rmdevice)
> + if (rmdevice) {
> + if (acpi_is_root_bridge(dev->handle)) {
> + struct acpi_root_bridge *bridge;
> +
> + bridge = acpi_root_handle_to_bridge(dev->handle);
> + if (bridge)
> + remove_acpi_root_bridge(bridge);
> + }
> +
> acpi_device_unregister(dev, ACPI_BUS_REMOVAL_EJECT);
> + }
no, we don't need that.
after closely looking, it seems we can dump acpi_root_bridge.
please check if attached patch could work with container path.
Thanks
Yinghai
[-- Attachment #2: kill_remove_acpi_root_bridge.patch --]
[-- Type: application/octet-stream, Size: 5666 bytes --]
Subject: [PATCH] PCI, ACPI: remove acpi_root_bridge in pci_root_hp
Tang noticed that hotplug through container will not update acpi_root_bridge
list.
After closely checking, we don't need that for struct for tracking and
could use acpi_pci_root directly.
Reported-by: Tang Chen <tangchen@cn.fujitsu.com>
Signed-off-by: Yinghai Lu <yinghai@kernel.org>
---
drivers/acpi/pci_root_hp.c | 111 ++++++++-------------------------------------
1 file changed, 21 insertions(+), 90 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
@@ -12,13 +12,6 @@
#include <linux/slab.h>
#include <linux/acpi.h>
-static LIST_HEAD(acpi_root_bridge_list);
-struct acpi_root_bridge {
- struct list_head list;
- acpi_handle handle;
- u32 flags;
-};
-
static const struct acpi_device_id root_device_ids[] = {
{"PNP0A03", 0},
{"PNP0A08", 0},
@@ -31,60 +24,6 @@ static const struct acpi_device_id root_
#define ACPI_STA_FUNCTIONING (0x00000008)
-static struct acpi_root_bridge *acpi_root_handle_to_bridge(acpi_handle handle)
-{
- struct acpi_root_bridge *bridge;
-
- list_for_each_entry(bridge, &acpi_root_bridge_list, list)
- if (bridge->handle == handle)
- return bridge;
-
- return NULL;
-}
-
-/* allocate and initialize host bridge data structure */
-static void add_acpi_root_bridge(acpi_handle handle)
-{
- struct acpi_root_bridge *bridge;
- acpi_handle dummy_handle;
- acpi_status status;
-
- /* if the bridge doesn't have _STA, we assume it is always there */
- status = acpi_get_handle(handle, "_STA", &dummy_handle);
- if (ACPI_SUCCESS(status)) {
- unsigned long long tmp;
-
- status = acpi_evaluate_integer(handle, "_STA", NULL, &tmp);
- if (ACPI_FAILURE(status)) {
- printk(KERN_DEBUG "%s: _STA evaluation failure\n",
- __func__);
- return;
- }
- if ((tmp & ACPI_STA_FUNCTIONING) == 0)
- /* don't register this object */
- return;
- }
-
- bridge = kzalloc(sizeof(struct acpi_root_bridge), GFP_KERNEL);
- if (!bridge)
- return;
-
- bridge->handle = handle;
-
- if (ACPI_SUCCESS(acpi_get_handle(handle, "_EJ0", &dummy_handle)))
- bridge->flags |= ROOT_BRIDGE_HAS_EJ0;
- if (ACPI_SUCCESS(acpi_get_handle(handle, "_PS3", &dummy_handle)))
- bridge->flags |= ROOT_BRIDGE_HAS_PS3;
-
- 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;
@@ -166,28 +105,25 @@ static int acpi_root_evaluate_object(acp
return 0;
}
-static void handle_root_bridge_removal(acpi_handle handle,
- struct acpi_root_bridge *bridge)
+static void handle_root_bridge_removal(struct acpi_device *device)
{
+ int ret_val;
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;
+ acpi_handle dummy_handle;
+ acpi_handle handle = device->handle;
- /* remove pci devices at first */
- ret_val = acpi_bus_trim(device, 0);
- printk(KERN_DEBUG "acpi_bus_trim stop return %x\n", ret_val);
+ if (ACPI_SUCCESS(acpi_get_handle(handle, "_EJ0", &dummy_handle)))
+ flags |= ROOT_BRIDGE_HAS_EJ0;
+ if (ACPI_SUCCESS(acpi_get_handle(handle, "_PS3", &dummy_handle)))
+ flags |= ROOT_BRIDGE_HAS_PS3;
- /* remove acpi devices */
- ret_val = acpi_bus_trim(device, 1);
- printk(KERN_DEBUG "acpi_bus_trim remove return %x\n", 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;
@@ -202,7 +138,7 @@ static void handle_root_bridge_removal(a
static void _handle_hotplug_event_root(struct work_struct *work)
{
- struct acpi_root_bridge *bridge;
+ struct acpi_pci_root *root;
char objname[64];
struct acpi_buffer buffer = { .length = sizeof(objname),
.pointer = objname };
@@ -214,7 +150,7 @@ static void _handle_hotplug_event_root(s
handle = hp_work->handle;
type = hp_work->type;
- bridge = acpi_root_handle_to_bridge(handle);
+ root = acpi_pci_find_root(handle);
acpi_get_name(handle, ACPI_FULL_PATHNAME, &buffer);
@@ -223,10 +159,8 @@ static void _handle_hotplug_event_root(s
/* bus enumerate */
printk(KERN_DEBUG "%s: Bus check notify on %s\n", __func__,
objname);
- if (!bridge) {
+ if (!root)
handle_root_bridge_insertion(handle);
- add_acpi_root_bridge(handle);
- }
break;
@@ -234,17 +168,16 @@ static void _handle_hotplug_event_root(s
/* device check */
printk(KERN_DEBUG "%s: Device check notify on %s\n", __func__,
objname);
- if (!bridge) {
+ if (!root)
handle_root_bridge_insertion(handle);
- add_acpi_root_bridge(handle);
- }
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);
+ if (root)
+ handle_root_bridge_removal(root->device);
break;
default:
printk(KERN_WARNING "notify_handler: unknown event type 0x%x for %s\n",
@@ -304,8 +237,6 @@ find_root_bridges(acpi_handle handle, u3
printk(KERN_DEBUG "acpi root: %s notify handler is installed\n",
objname);
- add_acpi_root_bridge(handle);
-
return AE_OK;
}
^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [PATCH 1/1] Update acpi_root_bridge_list in container hotplug path.
2012-10-17 5:18 ` Yinghai Lu
@ 2012-10-17 7:39 ` Tang Chen
2012-10-17 7:42 ` Tang Chen
2012-10-17 16:28 ` Yinghai Lu
0 siblings, 2 replies; 8+ messages in thread
From: Tang Chen @ 2012-10-17 7:39 UTC (permalink / raw)
To: Yinghai Lu
Cc: jiang.liu, bhelgaas, lenb, izumi.taku, isimatu.yasuaki,
linux-acpi, linux-pci, linux-kernel
On 10/17/2012 01:18 PM, Yinghai Lu wrote:
>
> no, we don't need that.
>
> after closely looking, it seems we can dump acpi_root_bridge.
>
> please check if attached patch could work with container path.
Hi Yinghai,
Your patch seems working well.
BTW, I actually found that the two lists, acpi_root_bridge and
acpi_pci_roots in drivers/acpi/pci_root.c were similar, except
your acpi_root_bridge included PNP0A08.
In the beginning, I thought you would use it in some other situations,
and now it is clear that it can be removed.
Thanks. :)
And also, I have another 2 questions, maybe you can help me.
1) Do we need to put PNP0A08 into acpi_pci_roots ?
2) In container_notify_cb(), when it got a ACPI_NOTIFY_EJECT_REQUEST
event, it doesn't do the hot-remove things.
I use your sci emulator patch to test it. I did the following thing:
echo echo "\_SB_.LSB1" > /sys/kernel/debug/acpi/sci_notify
where \_SB_.LSB1 is a container, it just did nothing.
Do we need to support this operation ?
Thanks. :)
>
> Thanks
>
> Yinghai
^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [PATCH 1/1] Update acpi_root_bridge_list in container hotplug path.
2012-10-17 7:39 ` Tang Chen
@ 2012-10-17 7:42 ` Tang Chen
2012-10-17 16:28 ` Yinghai Lu
1 sibling, 0 replies; 8+ messages in thread
From: Tang Chen @ 2012-10-17 7:42 UTC (permalink / raw)
To: Yinghai Lu
Cc: jiang.liu, bhelgaas, lenb, izumi.taku, isimatu.yasuaki,
linux-acpi, linux-pci, linux-kernel
On 10/17/2012 03:39 PM, Tang Chen wrote:
> On 10/17/2012 01:18 PM, Yinghai Lu wrote:
>>
>> no, we don't need that.
>>
>> after closely looking, it seems we can dump acpi_root_bridge.
>>
>> please check if attached patch could work with container path.
>
> Hi Yinghai,
>
> Your patch seems working well.
>
> BTW, I actually found that the two lists, acpi_root_bridge and
> acpi_pci_roots in drivers/acpi/pci_root.c were similar, except
> your acpi_root_bridge included PNP0A08.
>
> In the beginning, I thought you would use it in some other situations,
> and now it is clear that it can be removed.
> Thanks. :)
>
> And also, I have another 2 questions, maybe you can help me.
> 1) Do we need to put PNP0A08 into acpi_pci_roots ?
> 2) In container_notify_cb(), when it got a ACPI_NOTIFY_EJECT_REQUEST
> event, it doesn't do the hot-remove things.
> I use your sci emulator patch to test it. I did the following thing:
> echo echo "\_SB_.LSB1" > /sys/kernel/debug/acpi/sci_notify
==> echo "\_SB_.LSB1 3" > /sys/kernel/debug/acpi/sci_notify
> where \_SB_.LSB1 is a container, it just did nothing.
> Do we need to support this operation ?
>
> Thanks. :)
>
>>
>> Thanks
>>
>> Yinghai
>
> --
> To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at http://vger.kernel.org/majordomo-info.html
> Please read the FAQ at http://www.tux.org/lkml/
>
^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [PATCH 1/1] Update acpi_root_bridge_list in container hotplug path.
2012-10-17 7:39 ` Tang Chen
2012-10-17 7:42 ` Tang Chen
@ 2012-10-17 16:28 ` Yinghai Lu
2012-10-18 1:00 ` Tang Chen
2012-10-19 8:49 ` Tang Chen
1 sibling, 2 replies; 8+ messages in thread
From: Yinghai Lu @ 2012-10-17 16:28 UTC (permalink / raw)
To: Tang Chen
Cc: jiang.liu, bhelgaas, lenb, izumi.taku, isimatu.yasuaki,
linux-acpi, linux-pci, linux-kernel
On Wed, Oct 17, 2012 at 12:39 AM, Tang Chen <tangchen@cn.fujitsu.com> wrote:
> On 10/17/2012 01:18 PM, Yinghai Lu wrote:
>
> And also, I have another 2 questions, maybe you can help me.
> 1) Do we need to put PNP0A08 into acpi_pci_roots ?
looks like we need to unify those two ids.
> 2) In container_notify_cb(), when it got a ACPI_NOTIFY_EJECT_REQUEST
> event, it doesn't do the hot-remove things.
> I use your sci emulator patch to test it. I did the following thing:
> echo echo "\_SB_.LSB1" > /sys/kernel/debug/acpi/sci_notify
> where \_SB_.LSB1 is a container, it just did nothing.
> Do we need to support this operation ?
yes, looks like need to add container_device_remove and call it under
container_notify_cb/ACPI_NOTIFY_EJECT_REQUEST
and should look like handle_root_bridge_removal to call acpi_bus_trim two times.
Thanks
Yinghai
^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [PATCH 1/1] Update acpi_root_bridge_list in container hotplug path.
2012-10-17 16:28 ` Yinghai Lu
@ 2012-10-18 1:00 ` Tang Chen
2012-10-19 8:49 ` Tang Chen
1 sibling, 0 replies; 8+ messages in thread
From: Tang Chen @ 2012-10-18 1:00 UTC (permalink / raw)
To: Yinghai Lu
Cc: jiang.liu, bhelgaas, lenb, izumi.taku, isimatu.yasuaki,
linux-acpi, linux-pci, linux-kernel
On 10/18/2012 12:28 AM, Yinghai Lu wrote:
> On Wed, Oct 17, 2012 at 12:39 AM, Tang Chen<tangchen@cn.fujitsu.com> wrote:
>> On 10/17/2012 01:18 PM, Yinghai Lu wrote:
>>
>> And also, I have another 2 questions, maybe you can help me.
>> 1) Do we need to put PNP0A08 into acpi_pci_roots ?
>
> looks like we need to unify those two ids.
>
>> 2) In container_notify_cb(), when it got a ACPI_NOTIFY_EJECT_REQUEST
>> event, it doesn't do the hot-remove things.
>> I use your sci emulator patch to test it. I did the following thing:
>> echo echo "\_SB_.LSB1"> /sys/kernel/debug/acpi/sci_notify
>> where \_SB_.LSB1 is a container, it just did nothing.
>> Do we need to support this operation ?
>
> yes, looks like need to add container_device_remove and call it under
> container_notify_cb/ACPI_NOTIFY_EJECT_REQUEST
>
> and should look like handle_root_bridge_removal to call acpi_bus_trim two times.
Hi Yinghai,
OK, I can do that. :)
And I'll send patches for that soon. :)
>
> Thanks
>
> Yinghai
>
^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [PATCH 1/1] Update acpi_root_bridge_list in container hotplug path.
2012-10-17 16:28 ` Yinghai Lu
2012-10-18 1:00 ` Tang Chen
@ 2012-10-19 8:49 ` Tang Chen
2012-10-19 20:36 ` Yinghai Lu
1 sibling, 1 reply; 8+ messages in thread
From: Tang Chen @ 2012-10-19 8:49 UTC (permalink / raw)
To: Yinghai Lu
Cc: jiang.liu, bhelgaas, lenb, izumi.taku, isimatu.yasuaki,
linux-acpi, linux-pci, linux-kernel
On 10/18/2012 12:28 AM, Yinghai Lu wrote:
> On Wed, Oct 17, 2012 at 12:39 AM, Tang Chen<tangchen@cn.fujitsu.com> wrote:
>> On 10/17/2012 01:18 PM, Yinghai Lu wrote:
>>
>> And also, I have another 2 questions, maybe you can help me.
>> 1) Do we need to put PNP0A08 into acpi_pci_roots ?
>
> looks like we need to unify those two ids.
>
>> 2) In container_notify_cb(), when it got a ACPI_NOTIFY_EJECT_REQUEST
>> event, it doesn't do the hot-remove things.
>> I use your sci emulator patch to test it. I did the following thing:
>> echo echo "\_SB_.LSB1"> /sys/kernel/debug/acpi/sci_notify
>> where \_SB_.LSB1 is a container, it just did nothing.
>> Do we need to support this operation ?
>
> yes, looks like need to add container_device_remove and call it under
> container_notify_cb/ACPI_NOTIFY_EJECT_REQUEST
>
> and should look like handle_root_bridge_removal to call acpi_bus_trim two times.
Hi Yinghai,
You said the following in another patch:
"[PATCH 27/40] ACPI: acpi_bus_trim to support two steps."
> For root bus hotremove support, we need to have pci device removed
> before acpi devices.
>
> So try to keep all acpi devices, and only stop drivers with them.
Is it just container and pci_root_bridge hot-remove need to call
acpi_bus_trim() twice ? For normal device without sub-device, I think
it is OK to call acpi_bus_trim(device, 1).
The reason why I'm asking this question is:
I saw in acpi_bus_hot_remove_device(), it almost does the same things
you did in handle_root_bridge_removal(), except calling acpi_bus_trim()
twice. And there are more than one path could do container hot-remove.
If I add a container_device_remove() doing the similar things, it could
be duplicated. So, shall we just remove handle_root_bridge_removal(),
and only use acpi_bus_hot_remove_device() ?
Of course, we need to call acpi_bus_trim() twice in
acpi_bus_hot_remove_device().
Thanks. :)
>
> Thanks
>
> Yinghai
> --
> To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at http://vger.kernel.org/majordomo-info.html
> Please read the FAQ at http://www.tux.org/lkml/
>
^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [PATCH 1/1] Update acpi_root_bridge_list in container hotplug path.
2012-10-19 8:49 ` Tang Chen
@ 2012-10-19 20:36 ` Yinghai Lu
0 siblings, 0 replies; 8+ messages in thread
From: Yinghai Lu @ 2012-10-19 20:36 UTC (permalink / raw)
To: Tang Chen
Cc: jiang.liu, bhelgaas, lenb, izumi.taku, isimatu.yasuaki,
linux-acpi, linux-pci, linux-kernel
[-- Attachment #1: Type: text/plain, Size: 1042 bytes --]
On Fri, Oct 19, 2012 at 1:49 AM, Tang Chen <tangchen@cn.fujitsu.com> wrote:
> Is it just container and pci_root_bridge hot-remove need to call
> acpi_bus_trim() twice ? For normal device without sub-device, I think
> it is OK to call acpi_bus_trim(device, 1).
>
> The reason why I'm asking this question is:
>
> I saw in acpi_bus_hot_remove_device(), it almost does the same things
> you did in handle_root_bridge_removal(), except calling acpi_bus_trim()
> twice. And there are more than one path could do container hot-remove.
>
> If I add a container_device_remove() doing the similar things, it could
> be duplicated. So, shall we just remove handle_root_bridge_removal(),
> and only use acpi_bus_hot_remove_device() ?
>
> Of course, we need to call acpi_bus_trim() twice in
> acpi_bus_hot_remove_device().
yes. we could use acpi_bus_hot_remove_device to simply
acpi_bus_hot_remove_device()
but that eject_event should have device instead of passing handle, and
later check if that device is there or not.
like attached two patches.
[-- Attachment #2: root_bridge_remove_1.patch --]
[-- Type: application/octet-stream, Size: 2197 bytes --]
Subject: [PATCH] ACPI: update ej_event interface to take acpi_device
Should use acpi_device pointer directly instead of use handle and
get the device pointer again.
Signed-off-by: Yinghai Lu <yinghai@kernel.org>
---
drivers/acpi/scan.c | 14 ++++----------
include/acpi/acpi_bus.h | 2 +-
2 files changed, 5 insertions(+), 11 deletions(-)
Index: linux-2.6/drivers/acpi/scan.c
===================================================================
--- linux-2.6.orig/drivers/acpi/scan.c
+++ linux-2.6/drivers/acpi/scan.c
@@ -95,19 +95,13 @@ static DEVICE_ATTR(modalias, 0444, acpi_
void acpi_bus_hot_remove_device(void *context)
{
struct acpi_eject_event *ej_event = (struct acpi_eject_event *) context;
- struct acpi_device *device;
- acpi_handle handle = ej_event->handle;
+ struct acpi_device *device = ej_event->device;
+ acpi_handle handle = device->handle;
struct acpi_object_list arg_list;
union acpi_object arg;
acpi_status status = AE_OK;
u32 ost_code = ACPI_OST_SC_NON_SPECIFIC_FAILURE; /* default */
- if (acpi_bus_get_device(handle, &device))
- goto err_out;
-
- if (!device)
- goto err_out;
-
ACPI_DEBUG_PRINT((ACPI_DB_INFO,
"Hot-removing device %s...\n", dev_name(&device->dev)));
@@ -189,7 +183,7 @@ acpi_eject_store(struct device *d, struc
goto err;
}
- ej_event->handle = acpi_device->handle;
+ ej_event->device = acpi_device;
if (acpi_device->flags.eject_pending) {
/* event originated from ACPI eject notification */
ej_event->event = ACPI_NOTIFY_EJECT_REQUEST;
@@ -197,7 +191,7 @@ acpi_eject_store(struct device *d, struc
} else {
/* event originated from user */
ej_event->event = ACPI_OST_EC_OSPM_EJECT;
- (void) acpi_evaluate_hotplug_ost(ej_event->handle,
+ (void) acpi_evaluate_hotplug_ost(acpi_device->handle,
ej_event->event, ACPI_OST_SC_EJECT_IN_PROGRESS, NULL);
}
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
@@ -304,7 +304,7 @@ struct acpi_bus_event {
};
struct acpi_eject_event {
- acpi_handle handle;
+ struct acpi_device *device;
u32 event;
};
[-- Attachment #3: root_bridge_remove_2.patch --]
[-- Type: application/octet-stream, Size: 2519 bytes --]
Subject: [PATCH] ACPI, PCI: Simplify handle_root_bridge_removal()
Tang Chen found handle_root_bridge_removal is very similiar to
acpi_bus_hot_remove_device().
Only difference is that it call trim two times.
Change to handle_root_bridge_removal to call trim one time and then
use acpi_bus_hot_remove_device.
Signed-off-by: Yinghai Lu <yinghai@kernel.org>
---
drivers/acpi/pci_root_hp.c | 51 +++++++--------------------------------------
1 file changed, 9 insertions(+), 42 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
@@ -56,56 +56,23 @@ 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)) {
- pr_warn("%s: %s to %d failed\n",
- __func__, cmd, val);
- return -1;
- }
-
- return 0;
-}
-
static void handle_root_bridge_removal(struct acpi_device *device)
{
int ret_val;
- u32 flags = 0;
- acpi_handle dummy_handle;
- acpi_handle handle = device->handle;
-
- if (ACPI_SUCCESS(acpi_get_handle(handle, "_EJ0", &dummy_handle)))
- flags |= ROOT_BRIDGE_HAS_EJ0;
- if (ACPI_SUCCESS(acpi_get_handle(handle, "_PS3", &dummy_handle)))
- flags |= ROOT_BRIDGE_HAS_PS3;
+ struct acpi_eject_event *ej_event;
+
+ ej_event = kmalloc(sizeof(*ej_event), GFP_KERNEL);
+ if (!ej_event)
+ return;
+
+ ej_event->device = device;
+ ej_event->event = ACPI_NOTIFY_EJECT_REQUEST;
/* 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))
- pr_warn("%s: _PS3 failed\n", __func__);
- }
- if (flags & ROOT_BRIDGE_HAS_EJ0)
- acpi_root_evaluate_object(handle, "_EJ0", 1);
+ acpi_bus_hot_remove_device(ej_event);
}
static void _handle_hotplug_event_root(struct work_struct *work)
^ permalink raw reply [flat|nested] 8+ messages in thread
end of thread, other threads:[~2012-10-19 20:36 UTC | newest]
Thread overview: 8+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2012-10-17 3:25 [PATCH 1/1] Update acpi_root_bridge_list in container hotplug path Tang Chen
2012-10-17 5:18 ` Yinghai Lu
2012-10-17 7:39 ` Tang Chen
2012-10-17 7:42 ` Tang Chen
2012-10-17 16:28 ` Yinghai Lu
2012-10-18 1:00 ` Tang Chen
2012-10-19 8:49 ` Tang Chen
2012-10-19 20:36 ` Yinghai Lu
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.