On Tuesday, December 18, 2012 03:15:12 PM Toshi Kani wrote:

On Tue, 2012-12-18 at 22:57 +0100, Rafael J. Wysocki wrote:

quoted

On Tuesday, December 18, 2012 09:10:41 AM Toshi Kani wrote:

quoted

On Tue, 2012-12-18 at 02:48 +0100, Rafael J. Wysocki wrote:

 :

quoted

quoted

We need to decide which module is responsible for calling .bind().  I
think it should be the ACPI scan module, not the ACPI PCI root bridge
driver, because:
 - bind() needs to be called when _ADR device is added.  The ACPI scan
module can scan any devices, while the PCI root driver can only scan
when it is added.
 - acpi_bus_remove() calls unbind() at hot-remove.  The same module
should be responsible for both bind() and unbind() handling.
 - It is cleaner to keep struct acpi_device_ops interface to be called
by the ACPI core.

I agree with that. :-)

Moreover, I don't think we need acpi_pci_bind() and acpi_pci_unbind() at all.

quoted

So, I would propose the following changes.

 - Move the acpi_hot_add_bind() call back to the original place after
the device_attach() call.
 - Rename the name of acpi_hot_add_bind() to something like
acpi_bind_adr_device() since it is no longer hot-add only (and is
specific to _ADR devices).
 - Create its pair function, acpi_unbind_adr_device(), which is called
from acpi_bus_remove().  When a constructor interface is introduced, its
destructor should be introduced as well. 
 - Remove the binding procedure from acpi_pci_root_add().  This should
be done in patch [2/6].

Well, what about moving the code from acpi_pci_bind()/acpi_pci_unbind()
somewhere else and removing those things altogether?

Sounds nice.  It will be bonus point if you can do that. :-)

I think I can, but I need a few more patches on top of what I've already posted
to do that.

I think that https://patchwork.kernel.org/patch/1889821/ and
https://patchwork.kernel.org/patch/1884701/ can stay as they are, since there's
some material on top of them already and I'll cut the new patches on top of all
that.  I'll repost the whole series some time later this week, stay tuned. :-)

Thanks,
Rafael


-- 
I speak only for myself.
Rafael J. Wysocki, Intel Open Source Technology Center.