Re: [PATCH v3 4/6] PCI/acpiphp: update ACPI hotplug slot information when PCI hotplug happens

[Rafael J. Wysocki]

On Thursday, January 10, 2013 12:29:55 AM Jiang Liu wrote:
> On 01/09/2013 08:04 AM, Rafael J. Wysocki wrote:
> > On Wednesday, January 09, 2013 12:52:23 AM Jiang Liu wrote:
> >> 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@xxxxxxxxxx>
> >> Signed-off-by: Yijing Wang <wangyijing@xxxxxxxxxx>
> >> ---
> >>  drivers/pci/hotplug/acpiphp_glue.c |   62 ++++++++++++++++++++++++++++++++++--
> >>  1 file changed, 60 insertions(+), 2 deletions(-)
> >>
> snip
> >> +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,
> >> @@ -1448,6 +1503,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);
> > 
> > Your previous patch adds a PCI bus notifier for a similar thing.  Why are
> > you adding another one here?
> Hi Rafael,
> 	The acpiphp driver could be built as a module, and this bus notifier
> is used to setup/tear down ACPI based PCI hotplug controllers under a PCI bridge.
> The previous patch is to add/remove PCI slots. The PCI slot and PCI hotplug
> controller are related but different objects, so we use two notifiers to support
> them all.

I would be much happier if those notifiers were not needed.  At least, I'm not
sure why they need to be invoked by the driver core during device registration.

In my opinion it would be much better if they were called only for the devices
that might need them instead of all PCI devices.

Thanks,
Rafael


-- 
I speak only for myself.
Rafael J. Wysocki, Intel Open Source Technology Center.
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/