| Message ID | 1349159588-15029-2-git-send-email-yinghai@kernel.org |
|---|---|
| State | Rejected |
| Headers | show |
On Mon, Oct 01, 2012 at 11:32:59PM -0700, Yinghai Lu wrote: > To use to control the delay attach driver for specific device. > > Will use bus notifier to toggle this bits when needed. > > Signed-off-by: Yinghai Lu <yinghai@kernel.org> > Cc: Greg Kroah-Hartman <gregkh@linuxfoundation.org> Without a whole lot more context as to why you want this change, no, sorry, I'll not accept it. greg k-h -- 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 Tue, Oct 2, 2012 at 6:33 AM, Greg Kroah-Hartman <gregkh@linuxfoundation.org> wrote: > On Mon, Oct 01, 2012 at 11:32:59PM -0700, Yinghai Lu wrote: >> To use to control the delay attach driver for specific device. >> >> Will use bus notifier to toggle this bits when needed. >> >> Signed-off-by: Yinghai Lu <yinghai@kernel.org> >> Cc: Greg Kroah-Hartman <gregkh@linuxfoundation.org> > > Without a whole lot more context as to why you want this change, no, > sorry, I'll not accept it. Sorry, I should put you on the to list for the whole patchset. Those patches address Bjorn's request: 1. kill acpi_pci_root_start in drivers/acpi/pci_root.c 2. register pci device to device as soon as possible, so for_each_pci_device could be used early before pci_bus_add_devices. please check change log patch2-4. https://patchwork.kernel.org/patch/1535731/ https://patchwork.kernel.org/patch/1535821/ https://patchwork.kernel.org/patch/1535851/ 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 Tue, Oct 02, 2012 at 10:39:25AM -0700, Yinghai Lu wrote: > On Tue, Oct 2, 2012 at 6:33 AM, Greg Kroah-Hartman > <gregkh@linuxfoundation.org> wrote: > > On Mon, Oct 01, 2012 at 11:32:59PM -0700, Yinghai Lu wrote: > >> To use to control the delay attach driver for specific device. > >> > >> Will use bus notifier to toggle this bits when needed. > >> > >> Signed-off-by: Yinghai Lu <yinghai@kernel.org> > >> Cc: Greg Kroah-Hartman <gregkh@linuxfoundation.org> > > > > Without a whole lot more context as to why you want this change, no, > > sorry, I'll not accept it. > > Sorry, I should put you on the to list for the whole patchset. > > Those patches address Bjorn's request: > 1. kill acpi_pci_root_start in drivers/acpi/pci_root.c > 2. register pci device to device as soon as possible, so for_each_pci_device > could be used early before pci_bus_add_devices. > > please check change log patch2-4. Sorry, I still don't see what is so special about PCI that they have to do something different here. What am I missing? greg k-h -- 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 Tue, Oct 2, 2012 at 11:47 AM, Greg Kroah-Hartman <gregkh@linuxfoundation.org> wrote: > On Tue, Oct 02, 2012 at 10:39:25AM -0700, Yinghai Lu wrote: >> On Tue, Oct 2, 2012 at 6:33 AM, Greg Kroah-Hartman >> <gregkh@linuxfoundation.org> wrote: >> > On Mon, Oct 01, 2012 at 11:32:59PM -0700, Yinghai Lu wrote: >> >> To use to control the delay attach driver for specific device. >> >> >> >> Will use bus notifier to toggle this bits when needed. >> >> >> >> Signed-off-by: Yinghai Lu <yinghai@kernel.org> >> >> Cc: Greg Kroah-Hartman <gregkh@linuxfoundation.org> >> > >> > Without a whole lot more context as to why you want this change, no, >> > sorry, I'll not accept it. >> >> Sorry, I should put you on the to list for the whole patchset. >> >> Those patches address Bjorn's request: >> 1. kill acpi_pci_root_start in drivers/acpi/pci_root.c >> 2. register pci device to device as soon as possible, so for_each_pci_device >> could be used early before pci_bus_add_devices. Just to be clear, I'm in favor of eliminating acpi_pci_root_start() eventually, but I didn't suggest this approach, and I don't like it much either. I don't have much constructive feedback yet, but this feels like a special case that's not cleanly integrated into the existing infrastructure. >> please check change log patch2-4. > > Sorry, I still don't see what is so special about PCI that they have to > do something different here. What am I missing? -- 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 Tue, Oct 2, 2012 at 10:47 AM, Greg Kroah-Hartman <gregkh@linuxfoundation.org> wrote: > On Tue, Oct 02, 2012 at 10:39:25AM -0700, Yinghai Lu wrote: >> On Tue, Oct 2, 2012 at 6:33 AM, Greg Kroah-Hartman >> Those patches address Bjorn's request: >> 1. kill acpi_pci_root_start in drivers/acpi/pci_root.c >> 2. register pci device to device as soon as possible, so for_each_pci_device >> could be used early before pci_bus_add_devices. >> >> please check change log patch2-4. > > Sorry, I still don't see what is so special about PCI that they have to > do something different here. What am I missing? That is something that core related with hotplug: for booting and hotplug, we have different code path. 1. for booting, all device are enumerated and registered, and later driver get registered, driver is attached to the devices. 2. for hotplug, one device on the bus is found, and then registered and driver is attached, after that another device on the bus is found, and then registered and driver is attached... The reason for the difference, device driver is not registered later during booting path. We should make the two path have same sequence. the solution that I suggest is adding per device drivers_autoprobe. and thanking for bus_type notifier that we could toggle that bit during device_add, so could make all device on same bus get probed and registered but driver get skipped. and after that call device_attach for all device at one batch. That will remove those hotplug related hacks like acpi_pci_root_start that try to delay registering of pci devices. and make the code lesser and readable. 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 Tue, Oct 02, 2012 at 01:20:58PM -0700, Yinghai Lu wrote: > On Tue, Oct 2, 2012 at 10:47 AM, Greg Kroah-Hartman > <gregkh@linuxfoundation.org> wrote: > > On Tue, Oct 02, 2012 at 10:39:25AM -0700, Yinghai Lu wrote: > >> On Tue, Oct 2, 2012 at 6:33 AM, Greg Kroah-Hartman > >> Those patches address Bjorn's request: > >> 1. kill acpi_pci_root_start in drivers/acpi/pci_root.c > >> 2. register pci device to device as soon as possible, so for_each_pci_device > >> could be used early before pci_bus_add_devices. > >> > >> please check change log patch2-4. > > > > Sorry, I still don't see what is so special about PCI that they have to > > do something different here. What am I missing? > > That is something that core related with hotplug: > > for booting and hotplug, we have different code path. > 1. for booting, all device are enumerated and registered, and later > driver get registered, driver is attached to the devices. > 2. for hotplug, one device on the bus is found, and then registered > and driver is attached, after that > another device on the bus is found, and then > registered and driver is attached... > > The reason for the difference, device driver is not registered later > during booting path. But on each case, the code path in the driver core is the same, and you don't know if for 1) a driver is not already registered in the system (think drivers built into the kernel). > We should make the two path have same sequence. the solution that I > suggest is adding per device drivers_autoprobe. No, now you have 2 code paths in the driver core, do not do that. > and thanking for bus_type notifier that we could toggle that bit > during device_add, so could make all device on same > bus get probed and registered but driver get skipped. and after that > call device_attach for all device at one batch. > > That will remove those hotplug related hacks like acpi_pci_root_start > that try to delay registering of pci devices. > and make the code lesser and readable. I don't know what you are doing in the acpi code, but I do not think it requires a driver core change like this. Again, why is PCI different from any other bus type? (hint, it shouldn't be...) greg k-h -- 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 Tue, Oct 2, 2012 at 1:45 PM, Greg Kroah-Hartman <gregkh@linuxfoundation.org> wrote: > On Tue, Oct 02, 2012 at 01:20:58PM -0700, Yinghai Lu wrote: >> On Tue, Oct 2, 2012 at 10:47 AM, Greg Kroah-Hartman >> <gregkh@linuxfoundation.org> wrote: >> > On Tue, Oct 02, 2012 at 10:39:25AM -0700, Yinghai Lu wrote: >> >> On Tue, Oct 2, 2012 at 6:33 AM, Greg Kroah-Hartman >> >> Those patches address Bjorn's request: >> >> 1. kill acpi_pci_root_start in drivers/acpi/pci_root.c >> >> 2. register pci device to device as soon as possible, so for_each_pci_device >> >> could be used early before pci_bus_add_devices. >> >> >> >> please check change log patch2-4. >> > >> > Sorry, I still don't see what is so special about PCI that they have to >> > do something different here. What am I missing? >> >> That is something that core related with hotplug: >> >> for booting and hotplug, we have different code path. >> 1. for booting, all device are enumerated and registered, and later >> driver get registered, driver is attached to the devices. >> 2. for hotplug, one device on the bus is found, and then registered >> and driver is attached, after that >> another device on the bus is found, and then >> registered and driver is attached... >> >> The reason for the difference, device driver is not registered later >> during booting path. > > But on each case, the code path in the driver core is the same, and you > don't know if for 1) a driver is not already registered in the system > (think drivers built into the kernel). different level initcall or link sequence make sure that in order. aka bus_type come first before driver of that bus type. > >> We should make the two path have same sequence. the solution that I >> suggest is adding per device drivers_autoprobe. > > No, now you have 2 code paths in the driver core, do not do that. no, reduce 2 to 1. > >> and thanking for bus_type notifier that we could toggle that bit >> during device_add, so could make all device on same >> bus get probed and registered but driver get skipped. and after that >> call device_attach for all device at one batch. >> >> That will remove those hotplug related hacks like acpi_pci_root_start >> that try to delay registering of pci devices. >> and make the code lesser and readable. > > I don't know what you are doing in the acpi code, but I do not think it > requires a driver core change like this. > > Again, why is PCI different from any other bus type? (hint, it > shouldn't be...) it is not pci susbsystem problem. the problem come from device-driver core for hotplug handling, and acpi system is forcing pci system to split the scan and registering. aka: to scan all pci device under one pci_root from acpi_pci_root_add. then do pci_bus_add_devices for that bus in acpi_pci_root_start. on hotplug path: when acpi enumerating acpi devices, it will create acpi_device for pci_root, then probe driver for that device : acpi_pci_root_driver. <at that time no acpi_device under that pci_root acpi device is created> device_add for root acpi_device will call bus_probe_device and ... and execute acpi_pci_root_driver .add aka acpi_pci_root_add. acpi_pci_root_add will call pci_acpi_scan_root. that will scan pci root bus to find all pci devices but does not add those pci device into devices. If at the time device_add for pci device get called, platform_notify will try to bind pci_dev with acpi devices, but corresponding acpi device is not created yet. that bind will fail. So acpi_driver introduce .start in ops to add pci device later. The patches will hold that acpi_driver for root to be probed for root to early. aka make sure all acpi devices come first, then probe driver for acpi device one by. In that case we don' t need subsystem to introduce .start to hold add device for next level. Now HOTPLUG is enabled forcely, and bus_type add notifier help to control that device drivers_autoprobe during device hot add. 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 Tue, Oct 2, 2012 at 3:47 PM, Yinghai Lu <yinghai@kernel.org> wrote: > On Tue, Oct 2, 2012 at 1:45 PM, Greg Kroah-Hartman > <gregkh@linuxfoundation.org> wrote: >> I don't know what you are doing in the acpi code, but I do not think it >> requires a driver core change like this. >> >> Again, why is PCI different from any other bus type? (hint, it >> shouldn't be...) > > it is not pci susbsystem problem. > > the problem come from device-driver core for hotplug handling, and > acpi system is > forcing pci system to split the scan and registering. > aka: to scan all pci device under one pci_root from acpi_pci_root_add. > then do pci_bus_add_devices for that bus in acpi_pci_root_start. I don't believe there's anything in ACPI that forces this split between scanning and registering. Certainly we have Linux *implementation* issues that currently lead us to split them, but I don't think those are forced by the hardware or the spec. I'd rather address those implementation issues than add band-aids. > on hotplug path: > when acpi enumerating acpi devices, it will create acpi_device for pci_root, > then probe driver for that device : acpi_pci_root_driver. > <at that time no acpi_device under that pci_root acpi device is created> > device_add for root acpi_device will call bus_probe_device and ... and execute > acpi_pci_root_driver .add aka acpi_pci_root_add. > acpi_pci_root_add will call pci_acpi_scan_root. that will scan pci > root bus to find all > pci devices but does not add those pci device into devices. > If at the time device_add for pci device get called, platform_notify > will try to bind pci_dev > with acpi devices, but corresponding acpi device is not created yet. > that bind will fail. > So acpi_driver introduce .start in ops to add pci device later. We enumerate ACPI devices by doing a depth-first traversal of the namespace. We should be able to bind a driver to a device as soon as we discover it. For PNP0A03/08 host bridges, that will be the pci_root.c driver. That driver's .add() method enumerates all the PCI devices behind the host bridge, which is another depth-first traversal, this time of the PCI hierarchy. Similarly, we ought to be able to bind drivers to the PCI devices as we discover them. But the PCI/ACPI binding is an issue here, because the ACPI enumeration hasn't descended below the host bridge yet, so we have the pci_dev structs but the acpi_device structs don't exist yet. I think this is part of the reason for the .add()/.start() split. But I don't think the split is the correct solution. I think we need to think about the binding in a different way. Maybe we don't bind at all and instead search the namespace every time we need the association. Or maybe we do the binding but base it on the acpi_handle (which exists before the ACPI subtree has been enumerated) rather than the acpi_device (which doesn't exist until we enumerate the subtree). It's not even clear to me that we should build acpi_device structs for things that already have pci_dev structs. I don't know what the right answer is, but I do think it's better to resolve it inside PCI and ACPI rather than playing games with driver binding times in the driver core. Bjorn -- 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 Tue, Oct 2, 2012 at 3:38 PM, Bjorn Helgaas <bhelgaas@google.com> wrote: > On Tue, Oct 2, 2012 at 3:47 PM, Yinghai Lu <yinghai@kernel.org> wrote: >> On Tue, Oct 2, 2012 at 1:45 PM, Greg Kroah-Hartman > > We enumerate ACPI devices by doing a depth-first traversal of the > namespace. We should be able to bind a driver to a device as soon as > we discover it. For PNP0A03/08 host bridges, that will be the > pci_root.c driver. That driver's .add() method enumerates all the PCI > devices behind the host bridge, which is another depth-first > traversal, this time of the PCI hierarchy. Similarly, we ought to be > able to bind drivers to the PCI devices as we discover them. before introducing this per device .drivers_autoprobe, i did one acpi to pci bind version as attached patch. that mean that will have two version: for booting path: use pci to acpi binding. and hot plug path will use acpi to pci binding. Also that make code pretty ugly to consider platform_notifier at the same time. and acp_get_pci_dev in that acpi_pci_hp_notifier is not very efficient. -Yinghai
On Tue, Oct 2, 2012 at 3:38 PM, Bjorn Helgaas <bhelgaas@google.com> wrote: > I don't know what the right answer is, but I do think it's better to > resolve it inside PCI and ACPI rather than playing games with driver > binding times in the driver core. is that ok to save that drivers_autoprobe bit in device.archdata for ia64 and x86 that using acpi? -- 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 Tue, Oct 2, 2012 at 5:00 PM, Yinghai Lu <yinghai@kernel.org> wrote: > On Tue, Oct 2, 2012 at 3:38 PM, Bjorn Helgaas <bhelgaas@google.com> wrote: > >> I don't know what the right answer is, but I do think it's better to >> resolve it inside PCI and ACPI rather than playing games with driver >> binding times in the driver core. > > is that ok to save that drivers_autoprobe bit in device.archdata for > ia64 and x86 that using acpi? updated patch to put the bit in pci_dev and acpi_device instead. please check attached patches. 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 Tue, Oct 2, 2012 at 7:07 PM, Yinghai Lu <yinghai@kernel.org> wrote: > On Tue, Oct 2, 2012 at 5:00 PM, Yinghai Lu <yinghai@kernel.org> wrote: >> On Tue, Oct 2, 2012 at 3:38 PM, Bjorn Helgaas <bhelgaas@google.com> wrote: >> >>> I don't know what the right answer is, but I do think it's better to >>> resolve it inside PCI and ACPI rather than playing games with driver >>> binding times in the driver core. >> >> is that ok to save that drivers_autoprobe bit in device.archdata for >> ia64 and x86 that using acpi? > > updated patch to put the bit in pci_dev and acpi_device instead. > > please check attached patches. attached.
On Tue, Oct 2, 2012 at 6:00 PM, Yinghai Lu <yinghai@kernel.org> wrote: > On Tue, Oct 2, 2012 at 3:38 PM, Bjorn Helgaas <bhelgaas@google.com> wrote: > >> I don't know what the right answer is, but I do think it's better to >> resolve it inside PCI and ACPI rather than playing games with driver >> binding times in the driver core. > > is that ok to save that drivers_autoprobe bit in device.archdata for > ia64 and x86 that using acpi? You'll have to ask Len (cc'd) what he thinks about the ACPI piece, but I don't like the PCI piece. It's the same idea that Greg already said he wouldn't put in the driver core. Why would it be better to add this complexity in the ACPI and PCI cores? I still think you need to work on the ACPI/PCI binding issue. Yes, it's hard, but I'm not in favor of avoiding that hard work by adding kludges everywhere. Bjorn -- 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 3, 2012 at 12:48 PM, Bjorn Helgaas <bhelgaas@google.com> wrote: > > You'll have to ask Len (cc'd) what he thinks about the ACPI piece, but > I don't like the PCI piece. It's the same idea that Greg already said > he wouldn't put in the driver core. Why would it be better to add > this complexity in the ACPI and PCI cores? It should be simplify acpi code a little. I will reorder acpi parts as separate patcheset for review. > I still think you need to work on the ACPI/PCI binding issue. Yes, > it's hard, but I'm not in favor of avoiding that hard work by adding > kludges everywhere. acpi pci binding should be solved if len could accept acpi changes. for pci separate driver attach, that is for registering pci dev to tree early instead of waiting till pci_bus_add_devices. Let me know if you still want us to pursue that. Otherwise I'd like to move root bus and host bridge registering into pci_bus_add_devices(). 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
Now acpi_pci_root_driver has two ops: .add and .start, aka acpi_pci_root_add and acpi_pci_root_start. That is for hotplug handling: .add need to return early to make sure all acpi device could be created and added early. So .start could device_add pci device that are found in acpi_pci_root_add/pci_acpi_scan_root(). That is holding pci devics to be out of devices for while. We could use bus notifier to handle hotplug case. CONFIG_HOTPLUG is enabled always now. Need to add drivers_autoprobe bit in acpi_device to hold attaching drivers for acpi_devices, so could make sure all acpi_devices get created at first. Then acpi_bus_attach() that is called from acpi_bus_add will attach driver for all acpi_devices that are just created. That make the logic more simple: hotplug path handling just like booting path that drivers are attached after all acpi device get created. At last we could remove all acpi_bus_start workaround. Thanks Yinghai Yinghai Lu (4): ACPI: add drivers_autoprobe in struct acpi_device ACPI: use device drivers_autoprobe to delay loading acpi drivers PCI, ACPI: Remove not used acpi_pci_root_start() ACPI: remove acpi_op_start workaround drivers/acpi/pci_root.c | 27 +++------ drivers/acpi/scan.c | 145 ++++++++++++++++++++++------------------------- include/acpi/acpi_bus.h | 9 +--- 3 files changed, 77 insertions(+), 104 deletions(-)
On Wed, Oct 03, 2012 at 04:00:10PM -0700, Yinghai Lu wrote: > Now acpi_pci_root_driver has two ops: .add and .start, aka acpi_pci_root_add > and acpi_pci_root_start. > > That is for hotplug handling: .add need to return early to make sure all > acpi device could be created and added early. So .start could device_add > pci device that are found in acpi_pci_root_add/pci_acpi_scan_root(). > > That is holding pci devics to be out of devices for while. > > We could use bus notifier to handle hotplug case. > CONFIG_HOTPLUG is enabled always now. > Need to add drivers_autoprobe bit in acpi_device to hold attaching drivers > for acpi_devices, so could make sure all acpi_devices get created at first. > Then acpi_bus_attach() that is called from acpi_bus_add will attach driver > for all acpi_devices that are just created. > > That make the logic more simple: hotplug path handling just like booting path > that drivers are attached after all acpi device get created. > > At last we could remove all acpi_bus_start workaround. I think we should get rid of ACPI .start() methods, but I'm opposed to this approach of fiddling with driver binding order. At hot-plug time, the pci_root.c driver is already loaded, then we enumerate the new host bridge. Since pci_root.c is already loaded, we bind the driver before descending the ACPI tree below the host bridge. We need to make that same ordering work at boot-time. At boot-time, we currently enumerate ALL ACPI devices before registering the pci_root.c driver: acpi_init # subsys_initcall acpi_scan_init acpi_bus_scan # enumerate ACPI devices acpi_pci_root_init # subsys_initcall for pci_root.c acpi_bus_register_driver ... acpi_pci_root_add # pci_root.c add method This is a fundamental difference: at boot-time, all the ACPI devices below the host bridge already exist before the pci_root.c driver claims the bridge, while at hot-add time, pci_root.c claims the bridge *before* those ACPI devices exist. I think this is wrong. The hot-plug case (where the driver is already loaded and binds to the device as soon as it's discovered, before the ACPI hierarchy below it is enumerated) seems like the obviously correct order. I think we should change the boot-time order to match that, i.e., we should register pci_root.c *before* enumerating ACPI devices. I realize that this will require changes in the way we bind PCI and ACPI devices. I pointed that out in a previous response, appended below for convenience: On Tue, Oct 2, 2012 at 4:38 PM, Bjorn Helgaas <bhelgaas@google.com> wrote: >> We enumerate ACPI devices by doing a depth-first traversal of the >> namespace. We should be able to bind a driver to a device as soon as >> we discover it. For PNP0A03/08 host bridges, that will be the >> pci_root.c driver. That driver's .add() method enumerates all the PCI >> devices behind the host bridge, which is another depth-first >> traversal, this time of the PCI hierarchy. Similarly, we ought to be >> able to bind drivers to the PCI devices as we discover them. > >> But the PCI/ACPI binding is an issue here, because the ACPI >> enumeration hasn't descended below the host bridge yet, so we have the >> pci_dev structs but the acpi_device structs don't exist yet. I think >> this is part of the reason for the .add()/.start() split. But I don't >> think the split is the correct solution. I think we need to think >> about the binding in a different way. Maybe we don't bind at all and >> instead search the namespace every time we need the association. Or >> maybe we do the binding but base it on the acpi_handle (which exists >> before the ACPI subtree has been enumerated) rather than the >> acpi_device (which doesn't exist until we enumerate the subtree). >> It's not even clear to me that we should build acpi_device structs for >> things that already have pci_dev structs. -- 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 Thu, Oct 4, 2012 at 10:47 AM, Bjorn Helgaas <bhelgaas@google.com> wrote: > On Wed, Oct 03, 2012 at 04:00:10PM -0700, Yinghai Lu wrote: > This is a fundamental difference: at boot-time, all the ACPI devices below the > host bridge already exist before the pci_root.c driver claims the bridge, > while at hot-add time, pci_root.c claims the bridge *before* those ACPI > devices exist. > > I think this is wrong. The hot-plug case (where the driver is already > loaded and binds to the device as soon as it's discovered, before the > ACPI hierarchy below it is enumerated) seems like the obviously correct > order. I think we should change the boot-time order to match that, i.e., > we should register pci_root.c *before* enumerating ACPI devices. in booting path, all device get probed at first, and then register driver... do you want to register all pci driver before probing pci devices? 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 Thu, Oct 4, 2012 at 12:36 PM, Yinghai Lu <yinghai@kernel.org> wrote: > On Thu, Oct 4, 2012 at 10:47 AM, Bjorn Helgaas <bhelgaas@google.com> wrote: >> On Wed, Oct 03, 2012 at 04:00:10PM -0700, Yinghai Lu wrote: >> This is a fundamental difference: at boot-time, all the ACPI devices below the >> host bridge already exist before the pci_root.c driver claims the bridge, >> while at hot-add time, pci_root.c claims the bridge *before* those ACPI >> devices exist. >> >> I think this is wrong. The hot-plug case (where the driver is already >> loaded and binds to the device as soon as it's discovered, before the >> ACPI hierarchy below it is enumerated) seems like the obviously correct >> order. I think we should change the boot-time order to match that, i.e., >> we should register pci_root.c *before* enumerating ACPI devices. > > in booting path, all device get probed at first, and then register driver... > do you want to register all pci driver before probing pci devices? I don't think we should have dependencies either way. It shouldn't matter whether we enumerate devices first or we register the driver first. That's why I think the current PCI/ACPI binding is broken -- it assumes that we fully enumerate ACPI before the driver is registered. To answer your specific question, yes, I do think drivers that are statically built in probably should be registered before devices are enumerated. That way, the boot-time case is more similar to the hot-add case. Obviously, for drivers that can be modules, the reverse must work as well (enumerate devices, then load and register the driver). And then the other order (register driver, then enumerate device) must also work so future hot-adds of the same device type work. Bjorn -- 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 Thursday 04 of October 2012 11:47:46 Bjorn Helgaas wrote: > On Wed, Oct 03, 2012 at 04:00:10PM -0700, Yinghai Lu wrote: > > Now acpi_pci_root_driver has two ops: .add and .start, aka acpi_pci_root_add > > and acpi_pci_root_start. > > > > That is for hotplug handling: .add need to return early to make sure all > > acpi device could be created and added early. So .start could device_add > > pci device that are found in acpi_pci_root_add/pci_acpi_scan_root(). > > > > That is holding pci devics to be out of devices for while. > > > > We could use bus notifier to handle hotplug case. > > CONFIG_HOTPLUG is enabled always now. > > Need to add drivers_autoprobe bit in acpi_device to hold attaching drivers > > for acpi_devices, so could make sure all acpi_devices get created at first. > > Then acpi_bus_attach() that is called from acpi_bus_add will attach driver > > for all acpi_devices that are just created. > > > > That make the logic more simple: hotplug path handling just like booting path > > that drivers are attached after all acpi device get created. > > > > At last we could remove all acpi_bus_start workaround. > > I think we should get rid of ACPI .start() methods, but I'm opposed to this > approach of fiddling with driver binding order. > > At hot-plug time, the pci_root.c driver is already loaded, then we enumerate > the new host bridge. Since pci_root.c is already loaded, we bind the driver > before descending the ACPI tree below the host bridge. We need to make that > same ordering work at boot-time. > > At boot-time, we currently enumerate ALL ACPI devices before registering > the pci_root.c driver: > > acpi_init # subsys_initcall > acpi_scan_init > acpi_bus_scan # enumerate ACPI devices > > acpi_pci_root_init # subsys_initcall for pci_root.c > acpi_bus_register_driver > ... > acpi_pci_root_add # pci_root.c add method > > This is a fundamental difference: at boot-time, all the ACPI devices below the > host bridge already exist before the pci_root.c driver claims the bridge, > while at hot-add time, pci_root.c claims the bridge *before* those ACPI > devices exist. > > I think this is wrong. The hot-plug case (where the driver is already > loaded and binds to the device as soon as it's discovered, before the > ACPI hierarchy below it is enumerated) seems like the obviously correct > order. I think we should change the boot-time order to match that, i.e., > we should register pci_root.c *before* enumerating ACPI devices. > > I realize that this will require changes in the way we bind PCI and ACPI > devices. I pointed that out in a previous response, appended below for > convenience: > > On Tue, Oct 2, 2012 at 4:38 PM, Bjorn Helgaas <bhelgaas@google.com> wrote: > > >> We enumerate ACPI devices by doing a depth-first traversal of the > >> namespace. We should be able to bind a driver to a device as soon as > >> we discover it. For PNP0A03/08 host bridges, that will be the > >> pci_root.c driver. That driver's .add() method enumerates all the PCI > >> devices behind the host bridge, which is another depth-first > >> traversal, this time of the PCI hierarchy. Similarly, we ought to be > >> able to bind drivers to the PCI devices as we discover them. > > > >> But the PCI/ACPI binding is an issue here, because the ACPI > >> enumeration hasn't descended below the host bridge yet, so we have the > >> pci_dev structs but the acpi_device structs don't exist yet. I think > >> this is part of the reason for the .add()/.start() split. But I don't > >> think the split is the correct solution. I think we need to think > >> about the binding in a different way. Maybe we don't bind at all and > >> instead search the namespace every time we need the association. Or > >> maybe we do the binding but base it on the acpi_handle (which exists > >> before the ACPI subtree has been enumerated) rather than the > >> acpi_device (which doesn't exist until we enumerate the subtree). > >> It's not even clear to me that we should build acpi_device structs for > >> things that already have pci_dev structs. No, we shouldn't. We generally have problems in this area, not only with PCI. For example, some platform x86 drivers cannot bind to the devices they should handle, because there's a "generic" ACPI driver that binds to the device in question earlier. My personal opinion is that so called "ACPI devices" (i.e. things represented by struct acpi_device) are generally partially redundant, because either they already have "sibling" struct device objects representing "real" devices, like PCI, SATA or PNP, or there should be struct platform_device "sibling" objects corresponding to them. Moreover, all ACPI drivers can be replaced with platform drivers and the only problem is the .notify() callback that is not present in struct platform_driver. So perhaps we can create a "real device" for every "ACPI device" we discover (if there isn't one already) and stop using ACPI drivers entirely, eventually. That also may help to address the problem of hypothetical future systems with ACPI tables describing the same hardware that we already have device tree representation for. In those cases drivers should work regardless of whether the information on devices is read from ACPI tables or from device trees and quite frankly I don't see how we can achieve this with the existing ACPI drivers. Perhaps we don't even need the ACPI bus type and struct acpi_device objects can be treated simply as storage of information used by the generic ACPI power management code etc. I'm not sure how to get there yet in the least painful way, but I think it would make things generally more straightforward. Thanks, Rafael
On Thursday 04 of October 2012 13:44:42 Bjorn Helgaas wrote: > On Thu, Oct 4, 2012 at 12:36 PM, Yinghai Lu <yinghai@kernel.org> wrote: > > On Thu, Oct 4, 2012 at 10:47 AM, Bjorn Helgaas <bhelgaas@google.com> wrote: > >> On Wed, Oct 03, 2012 at 04:00:10PM -0700, Yinghai Lu wrote: > >> This is a fundamental difference: at boot-time, all the ACPI devices below the > >> host bridge already exist before the pci_root.c driver claims the bridge, > >> while at hot-add time, pci_root.c claims the bridge *before* those ACPI > >> devices exist. > >> > >> I think this is wrong. The hot-plug case (where the driver is already > >> loaded and binds to the device as soon as it's discovered, before the > >> ACPI hierarchy below it is enumerated) seems like the obviously correct > >> order. I think we should change the boot-time order to match that, i.e., > >> we should register pci_root.c *before* enumerating ACPI devices. > > > > in booting path, all device get probed at first, and then register driver... > > do you want to register all pci driver before probing pci devices? > > I don't think we should have dependencies either way. It shouldn't > matter whether we enumerate devices first or we register the driver > first. That's why I think the current PCI/ACPI binding is broken -- > it assumes that we fully enumerate ACPI before the driver is > registered. > > To answer your specific question, yes, I do think drivers that are > statically built in probably should be registered before devices are > enumerated. That way, the boot-time case is more similar to the > hot-add case. > > Obviously, for drivers that can be modules, the reverse must work as > well (enumerate devices, then load and register the driver). And then > the other order (register driver, then enumerate device) must also > work so future hot-adds of the same device type work. Agreed. Thanks, Rafael
On Thu, Oct 4, 2012 at 12:44 PM, Bjorn Helgaas <bhelgaas@google.com> wrote: > > To answer your specific question, yes, I do think drivers that are > statically built in probably should be registered before devices are > enumerated. That way, the boot-time case is more similar to the > hot-add case. > > Obviously, for drivers that can be modules, the reverse must work as > well (enumerate devices, then load and register the driver). And then > the other order (register driver, then enumerate device) must also > work so future hot-adds of the same device type work. so you will have to handle two paths instead one. current booting path sequence are tested more than hot add path. -- 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 Thu, Oct 4, 2012 at 2:14 PM, Yinghai Lu <yinghai@kernel.org> wrote: > On Thu, Oct 4, 2012 at 12:44 PM, Bjorn Helgaas <bhelgaas@google.com> wrote: >> >> To answer your specific question, yes, I do think drivers that are >> statically built in probably should be registered before devices are >> enumerated. That way, the boot-time case is more similar to the >> hot-add case. >> >> Obviously, for drivers that can be modules, the reverse must work as >> well (enumerate devices, then load and register the driver). And then >> the other order (register driver, then enumerate device) must also >> work so future hot-adds of the same device type work. > > so you will have to handle two paths instead one. I'm not proposing any additional requirements; I'm just describing the way Linux driver modules work. Any module *already* must support both orders (enumerate devices then register driver, as well as register driver then enumerate and bind to a hot-added device). And this should not be two paths, it should be one and the same path for both orders. > current booting path sequence are tested more than hot add path. True. You're proposing fiddling with driver binding order so we can use the current boot path ordering at hot-add time. I'm suggesting that the "register driver then enumerate device" order is something we have to support for hot-add anyway, and that we should use the same ordering at boot-time. That's more change for the boot-time path, but I think it's cleaner and more maintainable in the long term. -- 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 Wednesday 03 of October 2012 16:00:10 Yinghai Lu wrote: > Now acpi_pci_root_driver has two ops: .add and .start, aka acpi_pci_root_add > and acpi_pci_root_start. > > That is for hotplug handling: .add need to return early to make sure all > acpi device could be created and added early. So .start could device_add > pci device that are found in acpi_pci_root_add/pci_acpi_scan_root(). > > That is holding pci devics to be out of devices for while. > > We could use bus notifier to handle hotplug case. > CONFIG_HOTPLUG is enabled always now. > Need to add drivers_autoprobe bit in acpi_device to hold attaching drivers > for acpi_devices, so could make sure all acpi_devices get created at first. > Then acpi_bus_attach() that is called from acpi_bus_add will attach driver > for all acpi_devices that are just created. > > That make the logic more simple: hotplug path handling just like booting path > that drivers are attached after all acpi device get created. > > At last we could remove all acpi_bus_start workaround. Do I understand correctly that you just want to prevent acpi_pci_root_driver from binding to the host bridge's struct acpi_device created while we're walking the ACPI namespace? Rafael
On Thu, Oct 4, 2012 at 2:23 PM, Rafael J. Wysocki <rjw@sisk.pl> wrote: >> >> At last we could remove all acpi_bus_start workaround. > > Do I understand correctly that you just want to prevent acpi_pci_root_driver > from binding to the host bridge's struct acpi_device created while we're > walking the ACPI namespace? yes, during hot add path. 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 Thursday 04 of October 2012 14:31:46 Yinghai Lu wrote: > On Thu, Oct 4, 2012 at 2:23 PM, Rafael J. Wysocki <rjw@sisk.pl> wrote: > >> > >> At last we could remove all acpi_bus_start workaround. > > > > Do I understand correctly that you just want to prevent acpi_pci_root_driver > > from binding to the host bridge's struct acpi_device created while we're > > walking the ACPI namespace? > > yes, during hot add path. Why exactly do you want to prevent that from happening? Rafael
On Thu, Oct 4, 2012 at 2:53 PM, Rafael J. Wysocki <rjw@sisk.pl> wrote: > On Thursday 04 of October 2012 14:31:46 Yinghai Lu wrote: >> On Thu, Oct 4, 2012 at 2:23 PM, Rafael J. Wysocki <rjw@sisk.pl> wrote: >> >> >> >> At last we could remove all acpi_bus_start workaround. >> > >> > Do I understand correctly that you just want to prevent acpi_pci_root_driver >> > from binding to the host bridge's struct acpi_device created while we're >> > walking the ACPI namespace? >> >> yes, during hot add path. > > Why exactly do you want to prevent that from happening? > during adding pci root bus hotplug, Bjorn found some unsafe searching that caused by pci_bus_add_devices. pci devices are created during pci scan root, but until very late acpi_pci_root_start call pci_bus_add_devices. To fill the gap, we need to move pci_bus_add_devices to acpi_pci_root_add at first. but after we move that there, pci device will be added to device tree, and it will try to bind with acpi devices that should be under acpi pci root, but are not created yet. because device_add for acpi_device for acpi pci root is done yet. it still calling the .add in the acpi_driver aka acpi_pci_root_add. So I want to hold the driver attach for pci root acpi devices, and later attach it until pci devices created. booting path, all acpi devices get created, and attach driver for them one by one. -- 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 Thursday 04 of October 2012 15:01:21 Yinghai Lu wrote: > On Thu, Oct 4, 2012 at 2:53 PM, Rafael J. Wysocki <rjw@sisk.pl> wrote: > > On Thursday 04 of October 2012 14:31:46 Yinghai Lu wrote: > >> On Thu, Oct 4, 2012 at 2:23 PM, Rafael J. Wysocki <rjw@sisk.pl> wrote: > >> >> > >> >> At last we could remove all acpi_bus_start workaround. > >> > > >> > Do I understand correctly that you just want to prevent acpi_pci_root_driver > >> > from binding to the host bridge's struct acpi_device created while we're > >> > walking the ACPI namespace? > >> > >> yes, during hot add path. > > > > Why exactly do you want to prevent that from happening? > > > > during adding pci root bus hotplug, Bjorn found some unsafe searching > that caused by pci_bus_add_devices. Do you have a link to a description of that problem? > pci devices are created during pci scan root, but until very late > acpi_pci_root_start call pci_bus_add_devices. So you mean that pci_bus_add_devices() is called too late, right? > To fill the gap, we need to move pci_bus_add_devices to acpi_pci_root_add > at first. > > but after we move that there, pci device will be added to device tree, and it > will try to bind with acpi devices that should be under acpi pci root, > but are not > created yet. because device_add for acpi_device for acpi pci root is done yet. > it still calling the .add in the acpi_driver aka acpi_pci_root_add. Quite obviously, we haven't walked the ACPI namespace below the host bridge object yet at that point. > So I want to hold the driver attach for pci root acpi devices, and > later attach it > until pci devices created. > > booting path, all acpi devices get created, and attach driver for them > one by one. I see. Your patches seem to affect all devices in the ACPI namespace added after boot, though, not only host bridges. And the problem seems to be that the scanning of the ACPI namespace and configuring the host bridge are kind of independent operations now. What we should do, actually, seems to be something like this: (1) Configure the host bridge when discovered (i.e. do what the current acpi_pci_root_add() does. (2) Parse the ACPI namespace under the host bridge (without binding ACPI drivers to the struct acpi_device objects created in the process, because they are known to correspond to PCI devices). (3) Run pci_bus_add_devices() for the bridge. in one routine. What do you think? Rafael
On Thu, Oct 4, 2012 at 3:36 PM, Rafael J. Wysocki <rjw@sisk.pl> wrote: > On Thursday 04 of October 2012 15:01:21 Yinghai Lu wrote: >> during adding pci root bus hotplug, Bjorn found some unsafe searching >> that caused by pci_bus_add_devices. > > Do you have a link to a description of that problem? Maybe bjorn could expand it more. > >> pci devices are created during pci scan root, but until very late >> acpi_pci_root_start call pci_bus_add_devices. > > So you mean that pci_bus_add_devices() is called too late, right? yes. > >> To fill the gap, we need to move pci_bus_add_devices to acpi_pci_root_add >> at first. >> >> but after we move that there, pci device will be added to device tree, and it >> will try to bind with acpi devices that should be under acpi pci root, >> but are not >> created yet. because device_add for acpi_device for acpi pci root is done yet. >> it still calling the .add in the acpi_driver aka acpi_pci_root_add. > > Quite obviously, we haven't walked the ACPI namespace below the host bridge > object yet at that point. yes. > >> So I want to hold the driver attach for pci root acpi devices, and >> later attach it >> until pci devices created. >> >> booting path, all acpi devices get created, and attach driver for them >> one by one. > > I see. > > Your patches seem to affect all devices in the ACPI namespace added after > boot, though, not only host bridges. yes, but it still should be safe. > > And the problem seems to be that the scanning of the ACPI namespace and > configuring the host bridge are kind of independent operations now. What > we should do, actually, seems to be something like this: > > (1) Configure the host bridge when discovered (i.e. do what the current > acpi_pci_root_add() does. > (2) Parse the ACPI namespace under the host bridge (without binding ACPI > drivers to the struct acpi_device objects created in the process, > because they are known to correspond to PCI devices). > (3) Run pci_bus_add_devices() for the bridge. > > in one routine. problem is still there. if 1 still has acpi_pci_root_add and pci_acpi_scan_root that scan pci devices. what is need is we need to bind 1 and 3 together. or we could move pci_scan_root_scan from acpi_pci_root_add to acpi_pci_root_start? -- 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 Thursday 04 of October 2012 15:46:39 Yinghai Lu wrote: > On Thu, Oct 4, 2012 at 3:36 PM, Rafael J. Wysocki <rjw@sisk.pl> wrote: > > On Thursday 04 of October 2012 15:01:21 Yinghai Lu wrote: > >> during adding pci root bus hotplug, Bjorn found some unsafe searching > >> that caused by pci_bus_add_devices. > > > > Do you have a link to a description of that problem? > > Maybe bjorn could expand it more. > > > > >> pci devices are created during pci scan root, but until very late > >> acpi_pci_root_start call pci_bus_add_devices. > > > > So you mean that pci_bus_add_devices() is called too late, right? > > yes. > > > > >> To fill the gap, we need to move pci_bus_add_devices to acpi_pci_root_add > >> at first. > >> > >> but after we move that there, pci device will be added to device tree, and it > >> will try to bind with acpi devices that should be under acpi pci root, > >> but are not > >> created yet. because device_add for acpi_device for acpi pci root is done yet. > >> it still calling the .add in the acpi_driver aka acpi_pci_root_add. > > > > Quite obviously, we haven't walked the ACPI namespace below the host bridge > > object yet at that point. > > yes. > > > > >> So I want to hold the driver attach for pci root acpi devices, and > >> later attach it > >> until pci devices created. > >> > >> booting path, all acpi devices get created, and attach driver for them > >> one by one. > > > > I see. > > > > Your patches seem to affect all devices in the ACPI namespace added after > > boot, though, not only host bridges. > > yes, but it still should be safe. I'm not really sure of that (what about undock/dock, for exmaple?) and it's damn ugly. > > And the problem seems to be that the scanning of the ACPI namespace and > > configuring the host bridge are kind of independent operations now. What > > we should do, actually, seems to be something like this: > > > > (1) Configure the host bridge when discovered (i.e. do what the current > > acpi_pci_root_add() does. > > (2) Parse the ACPI namespace under the host bridge (without binding ACPI > > drivers to the struct acpi_device objects created in the process, > > because they are known to correspond to PCI devices). > > (3) Run pci_bus_add_devices() for the bridge. > > > > in one routine. > > problem is still there. if 1 still has acpi_pci_root_add and pci_acpi_scan_root OK, so why don't we do (2) in acpi_pci_root_add(), before pci_acpi_scan_root() is called? > that scan pci devices. what is need is we need to bind 1 and 3 together. I don't understand now. You said previously that we need the ACPI namespace below the bridge to be scanned before (3), so why do you want to do (3) before (2) now? > or we could move pci_scan_root_scan from acpi_pci_root_add to > acpi_pci_root_start? No, I don't think so, because we call acpi_pci_bind_root() after that. Thanks, Rafael
On Fri, Oct 5, 2012 at 4:01 PM, Rafael J. Wysocki <rjw@sisk.pl> wrote: > On Thursday 04 of October 2012 15:46:39 Yinghai Lu wrote: >> > Your patches seem to affect all devices in the ACPI namespace added after >> > boot, though, not only host bridges. >> >> yes, but it still should be safe. > > I'm not really sure of that (what about undock/dock, for exmaple?) and it's > damn ugly. only one acpi_driver has .start , that is acpi_pci_root_driver. should be clean than with .add/start pair. > >> > And the problem seems to be that the scanning of the ACPI namespace and >> > configuring the host bridge are kind of independent operations now. What >> > we should do, actually, seems to be something like this: >> > >> > (1) Configure the host bridge when discovered (i.e. do what the current >> > acpi_pci_root_add() does. >> > (2) Parse the ACPI namespace under the host bridge (without binding ACPI >> > drivers to the struct acpi_device objects created in the process, >> > because they are known to correspond to PCI devices). >> > (3) Run pci_bus_add_devices() for the bridge. >> > >> > in one routine. >> >> problem is still there. if 1 still has acpi_pci_root_add and pci_acpi_scan_root > > OK, so why don't we do (2) in acpi_pci_root_add(), before pci_acpi_scan_root() > is called? > >> that scan pci devices. what is need is we need to bind 1 and 3 together. some one already walk the acpi space, and during that it create acpi_device for pci_root and then attach driver for that, aka acpi_pci_root_add is executing. Now you want to walking the acpi acpi space to create children devices before device_add really done for that pci root acpi device. ? is that some kind of nesting? > > I don't understand now. You said previously that we need the ACPI namespace > below the bridge to be scanned before (3), so why do you want to do (3) before > (2) now? purpose is calling pci_bus_add_devices in pci_acpi_scan_root. 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 Friday 05 of October 2012 16:10:43 Yinghai Lu wrote: > On Fri, Oct 5, 2012 at 4:01 PM, Rafael J. Wysocki <rjw@sisk.pl> wrote: > > On Thursday 04 of October 2012 15:46:39 Yinghai Lu wrote: > >> > Your patches seem to affect all devices in the ACPI namespace added after > >> > boot, though, not only host bridges. > >> > >> yes, but it still should be safe. > > > > I'm not really sure of that (what about undock/dock, for exmaple?) and it's > > damn ugly. > > only one acpi_driver has .start , that is acpi_pci_root_driver. > > should be clean than with .add/start pair. > > > > >> > And the problem seems to be that the scanning of the ACPI namespace and > >> > configuring the host bridge are kind of independent operations now. What > >> > we should do, actually, seems to be something like this: > >> > > >> > (1) Configure the host bridge when discovered (i.e. do what the current > >> > acpi_pci_root_add() does. > >> > (2) Parse the ACPI namespace under the host bridge (without binding ACPI > >> > drivers to the struct acpi_device objects created in the process, > >> > because they are known to correspond to PCI devices). > >> > (3) Run pci_bus_add_devices() for the bridge. > >> > > >> > in one routine. > >> > >> problem is still there. if 1 still has acpi_pci_root_add and pci_acpi_scan_root > > > > OK, so why don't we do (2) in acpi_pci_root_add(), before pci_acpi_scan_root() > > is called? > > > >> that scan pci devices. what is need is we need to bind 1 and 3 together. > > some one already walk the acpi space, and during that it create > acpi_device for pci_root > and then attach driver for that, aka acpi_pci_root_add is executing. > > Now you want to walking the acpi acpi space to create children devices > before device_add really done for that pci root > acpi device. ? > > is that some kind of nesting? Yes, basically. The idea is to do the scan of the host bridge's children in the ACPI tree synchronously within acpi_pci_root_add() instead of trying to delay the execution of it until the children have been scanned (and using notifiers to kind of trigger the driver binding, i.e. the execution of .add()). > > I don't understand now. You said previously that we need the ACPI namespace > > below the bridge to be scanned before (3), so why do you want to do (3) before > > (2) now? > > purpose is calling pci_bus_add_devices in pci_acpi_scan_root. OK, but what's the reason? Rafael
diff --git a/drivers/base/bus.c b/drivers/base/bus.c index 181ed26..22b826a 100644 --- a/drivers/base/bus.c +++ b/drivers/base/bus.c @@ -538,7 +538,7 @@ void bus_probe_device(struct device *dev) if (!bus) return; - if (bus->p->drivers_autoprobe) { + if (bus->p->drivers_autoprobe && dev->drivers_autoprobe) { ret = device_attach(dev); WARN_ON(ret < 0); } diff --git a/drivers/base/core.c b/drivers/base/core.c index 5e6e00b..8a71f88 100644 --- a/drivers/base/core.c +++ b/drivers/base/core.c @@ -656,6 +656,7 @@ void device_initialize(struct device *dev) INIT_LIST_HEAD(&dev->devres_head); device_pm_init(dev); set_dev_node(dev, -1); + dev->drivers_autoprobe = true; } static struct kobject *virtual_device_parent(struct device *dev) diff --git a/include/linux/device.h b/include/linux/device.h index 52a5f15..e08db26 100644 --- a/include/linux/device.h +++ b/include/linux/device.h @@ -647,6 +647,7 @@ struct device { struct bus_type *bus; /* type of bus device is on */ struct device_driver *driver; /* which driver has allocated this device */ + bool drivers_autoprobe; void *platform_data; /* Platform specific data, device core doesn't touch it */ struct dev_pm_info power;
To use to control the delay attach driver for specific device. Will use bus notifier to toggle this bits when needed. Signed-off-by: Yinghai Lu <yinghai@kernel.org> Cc: Greg Kroah-Hartman <gregkh@linuxfoundation.org> --- drivers/base/bus.c | 2 +- drivers/base/core.c | 1 + include/linux/device.h | 1 + 3 files changed, 3 insertions(+), 1 deletions(-)