| Message ID | 1350444308-27102-1-git-send-email-tangchen@cn.fujitsu.com |
|---|---|
| State | Not Applicable |
| Headers | show |
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
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 -- To unsubscribe from this list: send the line "unsubscribe linux-pci" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
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/ > -- To unsubscribe from this list: send the line "unsubscribe linux-pci" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
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 -- To unsubscribe from this list: send the line "unsubscribe linux-pci" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
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 > -- To unsubscribe from this list: send the line "unsubscribe linux-pci" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
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/ > -- To unsubscribe from this list: send the line "unsubscribe linux-pci" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
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.
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__ */
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