Re: [PATCH v2 02/10] driver core: Functional dependencies tracking support
On Thursday, July 28, 2016 05:28:31 PM Lukas Wunner wrote: > On Thu, Jul 28, 2016 at 02:30:31AM +0200, Rafael J. Wysocki wrote: > > On Monday, July 25, 2016 12:48:32 AM Lukas Wunner wrote: > > > On Thu, Jul 21, 2016 at 02:25:15AM +0200, Rafael J. Wysocki wrote: > > > > On Thursday, July 21, 2016 01:25:53 AM Lukas Wunner wrote: > > > > > I guess I could amend portdrv to return -EPROBE_DEFER on Macs if > > > > > no driver is bound to the NHI. Doesn't feel pretty to me though. > > > > > > > > > > Ultimately this seems to be the same issue as with calling > > > > > dev_pm_domain_set() for a bound device. Perhaps device_link_add() > > > > > can likewise be allowed if a runtime PM ref is held for the devices > > > > > and the call happens under lock_system_sleep()? > > > > > > > > No, the whole synchronization scheme in the links code would have had > > > > to be > > > > changed for that to really work. > > > > > > > > And it really is about what is needed (at least in principle) to run > > > > your > > > > device. If you think you need device X with a driver to handle device Y > > > > correctly, then either you need it all the time, from probe to remove, > > > > or > > > > you just don't really need it at all. > > > > > > Real life isn't as simple as that. > > > > > > In this case, we have consumers (hotplug ports) which are doing fine > > > if the driver for the supplier (NHI) is not loaded. But once it loads, > > > the links must be in place. > > > > Hmm. > > > > What if it is not loaded and the system suspends. Will everything work > > as expected after the subsequent resume? > > The short answer is yes. OK I think it's possible to add a link flag to address this case. Namely, if that flag is passed to device_link_add(), the link will be added in the DEVICE_LINK_ACTIVE state right away, but that will need to be synchronized against all possible transitions of the consumer device (at least). It's better to do that in a separate patch for this reason IMO. Thanks, Rafael ___ iommu mailing list iommu@lists.linux-foundation.org https://lists.linuxfoundation.org/mailman/listinfo/iommu
Re: [PATCH v2 02/10] driver core: Functional dependencies tracking support
On Thu, Jul 28, 2016 at 02:30:31AM +0200, Rafael J. Wysocki wrote: > On Monday, July 25, 2016 12:48:32 AM Lukas Wunner wrote: > > On Thu, Jul 21, 2016 at 02:25:15AM +0200, Rafael J. Wysocki wrote: > > > On Thursday, July 21, 2016 01:25:53 AM Lukas Wunner wrote: > > > > I guess I could amend portdrv to return -EPROBE_DEFER on Macs if > > > > no driver is bound to the NHI. Doesn't feel pretty to me though. > > > > > > > > Ultimately this seems to be the same issue as with calling > > > > dev_pm_domain_set() for a bound device. Perhaps device_link_add() > > > > can likewise be allowed if a runtime PM ref is held for the devices > > > > and the call happens under lock_system_sleep()? > > > > > > No, the whole synchronization scheme in the links code would have had to > > > be > > > changed for that to really work. > > > > > > And it really is about what is needed (at least in principle) to run your > > > device. If you think you need device X with a driver to handle device Y > > > correctly, then either you need it all the time, from probe to remove, or > > > you just don't really need it at all. > > > > Real life isn't as simple as that. > > > > In this case, we have consumers (hotplug ports) which are doing fine > > if the driver for the supplier (NHI) is not loaded. But once it loads, > > the links must be in place. > > Hmm. > > What if it is not loaded and the system suspends. Will everything work > as expected after the subsequent resume? The short answer is yes. Long answer: With Thunderbolt, the switch fabric is told to set up PCI tunnels through the NHI (Native Host Interface). Once set up, the tunnels stay as they are and attached devices are reachable. However after a power cycle of the controller (suspend/resume), the tunnels are gone and need to be re-established. On Macs, there are two software components communicating with the NHI: The first one is an EFI driver which sets up tunnels to all devices present on boot and lights up all attached DP-over-Thunderbolt displays. Once ExitBootServices is called, the EFI driver is shut down but the configured tunnels stay as they are. The kernel is thus able to enumerate attached PCI devices. The second component is the OS driver, thunderbolt.ko. It is needed to set up tunnels to hot-plugged devices (i.e., not present at boot). It is also needed to re-establish tunnels after suspend/resume. The necessity of quirk_apple_wait_for_thunderbolt() arises because we walk the entire PCI hierarchy during ->resume_noirq and call pci_power_up() and pci_restore_state() for each device. Now remember, the PCI tunnels are gone after a power cycle, so the attached devices aren't reachable. Waking them and restoring their state will fail unless the thunderbolt driver reconfigures the switch fabric first. => So if there are no devices attached and thunderbolt.ko isn't loaded, everything is fine. No device link needed. => If devices are attached and thunderbolt.ko is loaded, then the hotplug ports need to wait for re-establishment of the PCI tunnels. Device link is needed. => If devices were attached on boot and thunderbolt.ko isn't loaded, they will be unreachable after resume. Nothing we can do about that. No device link needed. So this is a case of a "weak" device link, "weak" referring to the fact that it's only needed if the supplier is bound. All that said, I don't know if this case exists often enough that it's worth making allowances for it in the driver core. Sorry for the wall of text, just want to make sure we're on the same page and all possible use cases of device links are discussed and considered. Thanks, Lukas ___ iommu mailing list iommu@lists.linux-foundation.org https://lists.linuxfoundation.org/mailman/listinfo/iommu
Re: [PATCH v2 02/10] driver core: Functional dependencies tracking support
On Monday, July 25, 2016 12:48:32 AM Lukas Wunner wrote: > On Thu, Jul 21, 2016 at 02:25:15AM +0200, Rafael J. Wysocki wrote: > > On Thursday, July 21, 2016 01:25:53 AM Lukas Wunner wrote: > > > On Thu, Jul 21, 2016 at 12:51:31AM +0200, Rafael J. Wysocki wrote: > > > > On Wednesday, July 20, 2016 05:23:40 PM Lukas Wunner wrote: > > > > > On Wed, Jul 20, 2016 at 02:52:42PM +0200, Rafael J. Wysocki wrote: > > > > > > On Wednesday, July 20, 2016 08:24:50 AM Lukas Wunner wrote: > > > > > > > On Wed, Jul 20, 2016 at 02:33:18AM +0200, Rafael J. Wysocki wrote: > > > > > > > > On Friday, June 17, 2016 04:07:38 PM Lukas Wunner wrote: > > > > > > > > > On Fri, Jun 17, 2016 at 02:54:56PM +0200, Rafael J. Wysocki > > > > > > > > > wrote: > > > > > > > > > > On Fri, Jun 17, 2016 at 12:36 PM, Lukas Wunner > > > > > > > > > > wrote: > > > > > > > > > > > On Fri, Jun 17, 2016 at 08:26:52AM +0200, Marek > > > > > > > > > > > Szyprowski wrote: > > > > > > > > > > > > From: "Rafael J. Wysocki" > > > > > > > > > > > We also have such a functional dependency for Thunderbolt > > > > > > > > > > > on Macs: > > > > > > > > > > > On resume from system sleep, the PCIe hotplug ports may > > > > > > > > > > > not resume > > > > > > > > > > > before the thunderbolt driver has reestablished the PCI > > > > > > > > > > > tunnels. > > > > > > > > > > > Currently this is enforced by > > > > > > > > > > > quirk_apple_wait_for_thunderbolt() > > > > > > > > > > > in drivers/pci/quirks.c. It would be good if we could > > > > > > > > > > > represent > > > > > > > > > > > this dependency using something like Rafael's approach > > > > > > > > > > > instead of > > > > > > > > > > > open coding it, however one detail in Rafael's patches is > > > > > > > > > > > problematic: > > > > > > > > > > > > > > > > > > > > > > > New links are added by calling device_link_add() which > > > > > > > > > > > > may happen > > > > > > > > > > > > either before the consumer device is probed or when > > > > > > > > > > > > probing it, in > > > > > > > > > > > > which case the caller needs to ensure that the driver > > > > > > > > > > > > of the > > > > > > > > > > > > supplier device is present and functional and the > > > > > > > > > > > > DEVICE_LINK_PROBE_TIME > > > > > > > > > > > > flag should be passed to device_link_add() to reflect > > > > > > > > > > > > that. > > > > > > > > > > > > > > > > > > > > > > The thunderbolt driver cannot call device_link_add() > > > > > > > > > > > before the > > > > > > > > > > > PCIe hotplug ports are bound to a driver unless we amend > > > > > > > > > > > portdrv > > > > > > > > > > > to return -EPROBE_DEFER for Thunderbolt hotplug ports on > > > > > > > > > > > Macs > > > > > > > > > > > if the thunderbolt driver isn't loaded. > > > > > > > > > > > > > > > > > > > > > > It would therefore be beneficial if device_link_add() can > > > > > > > > > > > be > > > > > > > > > > > called even *after* the consumer is bound. > > > > > > > > > > > > > > > > > > > > I don't quite follow. > > > > > > > > > > > > > > > > > > > > Who's the provider and who's the consumer here? > > > > > > > > > > > > > > > > > > thunderbolt.ko is the supplier. > > > > > > > > > > > > > > > > But it binds to the children of the ports that are supposed to > > > > > > > > be its > > > > > > > > consumers? > > > > > > > > > > > > > > > > Why is that even expected to work? > > > > > > > > > > > > > > No, the consumers are aunts (or uncles) of the supplier, if you > > > > > > > will. :-) > > > > > > > > > > > > > > The consumers are the hotplug ports (named "Downstream Bridge 1 / > > > > > > > 2" in > > > > > > > the drawing below). The supplier is the NHI: > > > > > > > > > > > > > > (Root Port) Upstream Bridge --+-- Downstream Bridge 0 > > > > > > > NHI > > > > > > > +-- Downstream Bridge 1 > > > > > > > -- > > > > > > > +-- Downstream Bridge 2 > > > > > > > -- > > > > > > > ... > > > > > > > > > > > > > > We're calling pci_power_up() and pci_restore_state() from > > > > > > > pci_pm_resume_noirq(). And that will fail for devices below > > > > > > > the hotplug ports if the PCI tunnels haven't been re-established > > > > > > > yet by the NHI. > > > > > > > > > > > > So the NHI is a PCIe device, right? > > > > > > > > > > > > Does the Thunderbolt driver bind to that device? > > > > > > > > > > The NHI is a PCI device but not a bridge. It has class 0x88000. > > > > > Yes, thunderbolt.ko binds to the NHI. > > > > > > > > > > And portdrv binds to the upstream bridge and downstream bridges. > > > > > Those have class 0x60400. > > > > > > > > OK, so why would there be a problem with creating links from the NHI > > > > (producer) to the ports (consumers) before binding portdrv to them? > > > > > > Because the ordering in which drivers bind isn't guaranteed. At least > > > on my machine (D
Re: [PATCH v2 02/10] driver core: Functional dependencies tracking support
On Thu, Jul 21, 2016 at 02:25:15AM +0200, Rafael J. Wysocki wrote: > On Thursday, July 21, 2016 01:25:53 AM Lukas Wunner wrote: > > On Thu, Jul 21, 2016 at 12:51:31AM +0200, Rafael J. Wysocki wrote: > > > On Wednesday, July 20, 2016 05:23:40 PM Lukas Wunner wrote: > > > > On Wed, Jul 20, 2016 at 02:52:42PM +0200, Rafael J. Wysocki wrote: > > > > > On Wednesday, July 20, 2016 08:24:50 AM Lukas Wunner wrote: > > > > > > On Wed, Jul 20, 2016 at 02:33:18AM +0200, Rafael J. Wysocki wrote: > > > > > > > On Friday, June 17, 2016 04:07:38 PM Lukas Wunner wrote: > > > > > > > > On Fri, Jun 17, 2016 at 02:54:56PM +0200, Rafael J. Wysocki > > > > > > > > wrote: > > > > > > > > > On Fri, Jun 17, 2016 at 12:36 PM, Lukas Wunner > > > > > > > > > wrote: > > > > > > > > > > On Fri, Jun 17, 2016 at 08:26:52AM +0200, Marek Szyprowski > > > > > > > > > > wrote: > > > > > > > > > > > From: "Rafael J. Wysocki" > > > > > > > > > > We also have such a functional dependency for Thunderbolt > > > > > > > > > > on Macs: > > > > > > > > > > On resume from system sleep, the PCIe hotplug ports may not > > > > > > > > > > resume > > > > > > > > > > before the thunderbolt driver has reestablished the PCI > > > > > > > > > > tunnels. > > > > > > > > > > Currently this is enforced by > > > > > > > > > > quirk_apple_wait_for_thunderbolt() > > > > > > > > > > in drivers/pci/quirks.c. It would be good if we could > > > > > > > > > > represent > > > > > > > > > > this dependency using something like Rafael's approach > > > > > > > > > > instead of > > > > > > > > > > open coding it, however one detail in Rafael's patches is > > > > > > > > > > problematic: > > > > > > > > > > > > > > > > > > > > > New links are added by calling device_link_add() which > > > > > > > > > > > may happen > > > > > > > > > > > either before the consumer device is probed or when > > > > > > > > > > > probing it, in > > > > > > > > > > > which case the caller needs to ensure that the driver of > > > > > > > > > > > the > > > > > > > > > > > supplier device is present and functional and the > > > > > > > > > > > DEVICE_LINK_PROBE_TIME > > > > > > > > > > > flag should be passed to device_link_add() to reflect > > > > > > > > > > > that. > > > > > > > > > > > > > > > > > > > > The thunderbolt driver cannot call device_link_add() before > > > > > > > > > > the > > > > > > > > > > PCIe hotplug ports are bound to a driver unless we amend > > > > > > > > > > portdrv > > > > > > > > > > to return -EPROBE_DEFER for Thunderbolt hotplug ports on > > > > > > > > > > Macs > > > > > > > > > > if the thunderbolt driver isn't loaded. > > > > > > > > > > > > > > > > > > > > It would therefore be beneficial if device_link_add() can be > > > > > > > > > > called even *after* the consumer is bound. > > > > > > > > > > > > > > > > > > I don't quite follow. > > > > > > > > > > > > > > > > > > Who's the provider and who's the consumer here? > > > > > > > > > > > > > > > > thunderbolt.ko is the supplier. > > > > > > > > > > > > > > But it binds to the children of the ports that are supposed to be > > > > > > > its > > > > > > > consumers? > > > > > > > > > > > > > > Why is that even expected to work? > > > > > > > > > > > > No, the consumers are aunts (or uncles) of the supplier, if you > > > > > > will. :-) > > > > > > > > > > > > The consumers are the hotplug ports (named "Downstream Bridge 1 / > > > > > > 2" in > > > > > > the drawing below). The supplier is the NHI: > > > > > > > > > > > > (Root Port) Upstream Bridge --+-- Downstream Bridge 0 > > > > > > NHI > > > > > > +-- Downstream Bridge 1 -- > > > > > > +-- Downstream Bridge 2 -- > > > > > > ... > > > > > > > > > > > > We're calling pci_power_up() and pci_restore_state() from > > > > > > pci_pm_resume_noirq(). And that will fail for devices below > > > > > > the hotplug ports if the PCI tunnels haven't been re-established > > > > > > yet by the NHI. > > > > > > > > > > So the NHI is a PCIe device, right? > > > > > > > > > > Does the Thunderbolt driver bind to that device? > > > > > > > > The NHI is a PCI device but not a bridge. It has class 0x88000. > > > > Yes, thunderbolt.ko binds to the NHI. > > > > > > > > And portdrv binds to the upstream bridge and downstream bridges. > > > > Those have class 0x60400. > > > > > > OK, so why would there be a problem with creating links from the NHI > > > (producer) to the ports (consumers) before binding portdrv to them? > > > > Because the ordering in which drivers bind isn't guaranteed. At least > > on my machine (Debian), portdrv always binds before thunderbolt. > > But what drivers have to do with that really? Do you need drivers to > know that the dependency is there? > > Just add likns *before* even probing for drivers (yes, you can do that) > and the core will handle that for you. Forgive me for being den
Re: [PATCH v2 02/10] driver core: Functional dependencies tracking support
On Thursday, July 21, 2016 01:25:53 AM Lukas Wunner wrote: > On Thu, Jul 21, 2016 at 12:51:31AM +0200, Rafael J. Wysocki wrote: > > On Wednesday, July 20, 2016 05:23:40 PM Lukas Wunner wrote: > > > On Wed, Jul 20, 2016 at 02:52:42PM +0200, Rafael J. Wysocki wrote: > > > > On Wednesday, July 20, 2016 08:24:50 AM Lukas Wunner wrote: > > > > > On Wed, Jul 20, 2016 at 02:33:18AM +0200, Rafael J. Wysocki wrote: > > > > > > On Friday, June 17, 2016 04:07:38 PM Lukas Wunner wrote: > > > > > > > On Fri, Jun 17, 2016 at 02:54:56PM +0200, Rafael J. Wysocki wrote: > > > > > > > > On Fri, Jun 17, 2016 at 12:36 PM, Lukas Wunner > > > > > > > > wrote: > > > > > > > > > On Fri, Jun 17, 2016 at 08:26:52AM +0200, Marek Szyprowski > > > > > > > > > wrote: > > > > > > > > > > From: "Rafael J. Wysocki" > > > > > > > > > We also have such a functional dependency for Thunderbolt on > > > > > > > > > Macs: > > > > > > > > > On resume from system sleep, the PCIe hotplug ports may not > > > > > > > > > resume > > > > > > > > > before the thunderbolt driver has reestablished the PCI > > > > > > > > > tunnels. > > > > > > > > > Currently this is enforced by > > > > > > > > > quirk_apple_wait_for_thunderbolt() > > > > > > > > > in drivers/pci/quirks.c. It would be good if we could > > > > > > > > > represent > > > > > > > > > this dependency using something like Rafael's approach > > > > > > > > > instead of > > > > > > > > > open coding it, however one detail in Rafael's patches is > > > > > > > > > problematic: > > > > > > > > > > > > > > > > > > > New links are added by calling device_link_add() which may > > > > > > > > > > happen > > > > > > > > > > either before the consumer device is probed or when probing > > > > > > > > > > it, in > > > > > > > > > > which case the caller needs to ensure that the driver of the > > > > > > > > > > supplier device is present and functional and the > > > > > > > > > > DEVICE_LINK_PROBE_TIME > > > > > > > > > > flag should be passed to device_link_add() to reflect that. > > > > > > > > > > > > > > > > > > The thunderbolt driver cannot call device_link_add() before > > > > > > > > > the > > > > > > > > > PCIe hotplug ports are bound to a driver unless we amend > > > > > > > > > portdrv > > > > > > > > > to return -EPROBE_DEFER for Thunderbolt hotplug ports on Macs > > > > > > > > > if the thunderbolt driver isn't loaded. > > > > > > > > > > > > > > > > > > It would therefore be beneficial if device_link_add() can be > > > > > > > > > called even *after* the consumer is bound. > > > > > > > > > > > > > > > > I don't quite follow. > > > > > > > > > > > > > > > > Who's the provider and who's the consumer here? > > > > > > > > > > > > > > thunderbolt.ko is the supplier. > > > > > > > > > > > > But it binds to the children of the ports that are supposed to be > > > > > > its > > > > > > consumers? > > > > > > > > > > > > Why is that even expected to work? > > > > > > > > > > No, the consumers are aunts (or uncles) of the supplier, if you will. > > > > > :-) > > > > > > > > > > The consumers are the hotplug ports (named "Downstream Bridge 1 / 2" > > > > > in > > > > > the drawing below). The supplier is the NHI: > > > > > > > > > > (Root Port) Upstream Bridge --+-- Downstream Bridge 0 > > > > > NHI > > > > > +-- Downstream Bridge 1 -- > > > > > +-- Downstream Bridge 2 -- > > > > > ... > > > > > > > > > > We're calling pci_power_up() and pci_restore_state() from > > > > > pci_pm_resume_noirq(). And that will fail for devices below > > > > > the hotplug ports if the PCI tunnels haven't been re-established > > > > > yet by the NHI. > > > > > > > > So the NHI is a PCIe device, right? > > > > > > > > Does the Thunderbolt driver bind to that device? > > > > > > The NHI is a PCI device but not a bridge. It has class 0x88000. > > > Yes, thunderbolt.ko binds to the NHI. > > > > > > And portdrv binds to the upstream bridge and downstream bridges. > > > Those have class 0x60400. > > > > OK, so why would there be a problem with creating links from the NHI > > (producer) to the ports (consumers) before binding portdrv to them? > > Because the ordering in which drivers bind isn't guaranteed. At least > on my machine (Debian), portdrv always binds before thunderbolt. But what drivers have to do with that really? Do you need drivers to know that the dependency is there? Just add likns *before* even probing for drivers (yes, you can do that) and the core will handle that for you. > I guess I could amend portdrv to return -EPROBE_DEFER on Macs if > no driver is bound to the NHI. Doesn't feel pretty to me though. > > Ultimately this seems to be the same issue as with calling > dev_pm_domain_set() for a bound device. Perhaps device_link_add() > can likewise be allowed if a runtime PM ref is held for the devices > and the call happens under lock_s
Re: [PATCH v2 02/10] driver core: Functional dependencies tracking support
On Thu, Jul 21, 2016 at 12:51:31AM +0200, Rafael J. Wysocki wrote: > On Wednesday, July 20, 2016 05:23:40 PM Lukas Wunner wrote: > > On Wed, Jul 20, 2016 at 02:52:42PM +0200, Rafael J. Wysocki wrote: > > > On Wednesday, July 20, 2016 08:24:50 AM Lukas Wunner wrote: > > > > On Wed, Jul 20, 2016 at 02:33:18AM +0200, Rafael J. Wysocki wrote: > > > > > On Friday, June 17, 2016 04:07:38 PM Lukas Wunner wrote: > > > > > > On Fri, Jun 17, 2016 at 02:54:56PM +0200, Rafael J. Wysocki wrote: > > > > > > > On Fri, Jun 17, 2016 at 12:36 PM, Lukas Wunner > > > > > > > wrote: > > > > > > > > On Fri, Jun 17, 2016 at 08:26:52AM +0200, Marek Szyprowski > > > > > > > > wrote: > > > > > > > > > From: "Rafael J. Wysocki" > > > > > > > > We also have such a functional dependency for Thunderbolt on > > > > > > > > Macs: > > > > > > > > On resume from system sleep, the PCIe hotplug ports may not > > > > > > > > resume > > > > > > > > before the thunderbolt driver has reestablished the PCI tunnels. > > > > > > > > Currently this is enforced by quirk_apple_wait_for_thunderbolt() > > > > > > > > in drivers/pci/quirks.c. It would be good if we could represent > > > > > > > > this dependency using something like Rafael's approach instead > > > > > > > > of > > > > > > > > open coding it, however one detail in Rafael's patches is > > > > > > > > problematic: > > > > > > > > > > > > > > > > > New links are added by calling device_link_add() which may > > > > > > > > > happen > > > > > > > > > either before the consumer device is probed or when probing > > > > > > > > > it, in > > > > > > > > > which case the caller needs to ensure that the driver of the > > > > > > > > > supplier device is present and functional and the > > > > > > > > > DEVICE_LINK_PROBE_TIME > > > > > > > > > flag should be passed to device_link_add() to reflect that. > > > > > > > > > > > > > > > > The thunderbolt driver cannot call device_link_add() before the > > > > > > > > PCIe hotplug ports are bound to a driver unless we amend portdrv > > > > > > > > to return -EPROBE_DEFER for Thunderbolt hotplug ports on Macs > > > > > > > > if the thunderbolt driver isn't loaded. > > > > > > > > > > > > > > > > It would therefore be beneficial if device_link_add() can be > > > > > > > > called even *after* the consumer is bound. > > > > > > > > > > > > > > I don't quite follow. > > > > > > > > > > > > > > Who's the provider and who's the consumer here? > > > > > > > > > > > > thunderbolt.ko is the supplier. > > > > > > > > > > But it binds to the children of the ports that are supposed to be its > > > > > consumers? > > > > > > > > > > Why is that even expected to work? > > > > > > > > No, the consumers are aunts (or uncles) of the supplier, if you will. > > > > :-) > > > > > > > > The consumers are the hotplug ports (named "Downstream Bridge 1 / 2" in > > > > the drawing below). The supplier is the NHI: > > > > > > > > (Root Port) Upstream Bridge --+-- Downstream Bridge 0 > > > > NHI > > > > +-- Downstream Bridge 1 -- > > > > +-- Downstream Bridge 2 -- > > > > ... > > > > > > > > We're calling pci_power_up() and pci_restore_state() from > > > > pci_pm_resume_noirq(). And that will fail for devices below > > > > the hotplug ports if the PCI tunnels haven't been re-established > > > > yet by the NHI. > > > > > > So the NHI is a PCIe device, right? > > > > > > Does the Thunderbolt driver bind to that device? > > > > The NHI is a PCI device but not a bridge. It has class 0x88000. > > Yes, thunderbolt.ko binds to the NHI. > > > > And portdrv binds to the upstream bridge and downstream bridges. > > Those have class 0x60400. > > OK, so why would there be a problem with creating links from the NHI > (producer) to the ports (consumers) before binding portdrv to them? Because the ordering in which drivers bind isn't guaranteed. At least on my machine (Debian), portdrv always binds before thunderbolt. I guess I could amend portdrv to return -EPROBE_DEFER on Macs if no driver is bound to the NHI. Doesn't feel pretty to me though. Ultimately this seems to be the same issue as with calling dev_pm_domain_set() for a bound device. Perhaps device_link_add() can likewise be allowed if a runtime PM ref is held for the devices and the call happens under lock_system_sleep()? Thanks, Lukas ___ iommu mailing list iommu@lists.linux-foundation.org https://lists.linuxfoundation.org/mailman/listinfo/iommu
Re: [PATCH v2 02/10] driver core: Functional dependencies tracking support
On Wednesday, July 20, 2016 05:23:40 PM Lukas Wunner wrote: > On Wed, Jul 20, 2016 at 02:52:42PM +0200, Rafael J. Wysocki wrote: > > On Wednesday, July 20, 2016 08:24:50 AM Lukas Wunner wrote: > > > On Wed, Jul 20, 2016 at 02:33:18AM +0200, Rafael J. Wysocki wrote: > > > > On Friday, June 17, 2016 04:07:38 PM Lukas Wunner wrote: > > > > > On Fri, Jun 17, 2016 at 02:54:56PM +0200, Rafael J. Wysocki wrote: > > > > > > On Fri, Jun 17, 2016 at 12:36 PM, Lukas Wunner > > > > > > wrote: > > > > > > > On Fri, Jun 17, 2016 at 08:26:52AM +0200, Marek Szyprowski wrote: > > > > > > > > From: "Rafael J. Wysocki" > > > > > > > We also have such a functional dependency for Thunderbolt on Macs: > > > > > > > On resume from system sleep, the PCIe hotplug ports may not resume > > > > > > > before the thunderbolt driver has reestablished the PCI tunnels. > > > > > > > Currently this is enforced by quirk_apple_wait_for_thunderbolt() > > > > > > > in drivers/pci/quirks.c. It would be good if we could represent > > > > > > > this dependency using something like Rafael's approach instead of > > > > > > > open coding it, however one detail in Rafael's patches is > > > > > > > problematic: > > > > > > > > > > > > > > > New links are added by calling device_link_add() which may > > > > > > > > happen > > > > > > > > either before the consumer device is probed or when probing it, > > > > > > > > in > > > > > > > > which case the caller needs to ensure that the driver of the > > > > > > > > supplier device is present and functional and the > > > > > > > > DEVICE_LINK_PROBE_TIME > > > > > > > > flag should be passed to device_link_add() to reflect that. > > > > > > > > > > > > > > The thunderbolt driver cannot call device_link_add() before the > > > > > > > PCIe hotplug ports are bound to a driver unless we amend portdrv > > > > > > > to return -EPROBE_DEFER for Thunderbolt hotplug ports on Macs > > > > > > > if the thunderbolt driver isn't loaded. > > > > > > > > > > > > > > It would therefore be beneficial if device_link_add() can be > > > > > > > called even *after* the consumer is bound. > > > > > > > > > > > > I don't quite follow. > > > > > > > > > > > > Who's the provider and who's the consumer here? > > > > > > > > > > thunderbolt.ko is the supplier. > > > > > > > > But it binds to the children of the ports that are supposed to be its > > > > consumers? > > > > > > > > Why is that even expected to work? > > > > > > No, the consumers are aunts (or uncles) of the supplier, if you will. :-) > > > > > > The consumers are the hotplug ports (named "Downstream Bridge 1 / 2" in > > > the drawing below). The supplier is the NHI: > > > > > > (Root Port) Upstream Bridge --+-- Downstream Bridge 0 NHI > > > +-- Downstream Bridge 1 -- > > > +-- Downstream Bridge 2 -- > > > ... > > > > > > We're calling pci_power_up() and pci_restore_state() from > > > pci_pm_resume_noirq(). And that will fail for devices below > > > the hotplug ports if the PCI tunnels haven't been re-established > > > yet by the NHI. > > > > So the NHI is a PCIe device, right? > > > > Does the Thunderbolt driver bind to that device? > > The NHI is a PCI device but not a bridge. It has class 0x88000. > Yes, thunderbolt.ko binds to the NHI. > > And portdrv binds to the upstream bridge and downstream bridges. > Those have class 0x60400. OK, so why would there be a problem with creating links from the NHI (producer) to the ports (consumers) before binding portdrv to them? Thanks, Rafael ___ iommu mailing list iommu@lists.linux-foundation.org https://lists.linuxfoundation.org/mailman/listinfo/iommu
Re: [PATCH v2 02/10] driver core: Functional dependencies tracking support
On Wed, Jul 20, 2016 at 02:52:42PM +0200, Rafael J. Wysocki wrote: > On Wednesday, July 20, 2016 08:24:50 AM Lukas Wunner wrote: > > On Wed, Jul 20, 2016 at 02:33:18AM +0200, Rafael J. Wysocki wrote: > > > On Friday, June 17, 2016 04:07:38 PM Lukas Wunner wrote: > > > > On Fri, Jun 17, 2016 at 02:54:56PM +0200, Rafael J. Wysocki wrote: > > > > > On Fri, Jun 17, 2016 at 12:36 PM, Lukas Wunner > > > > > wrote: > > > > > > On Fri, Jun 17, 2016 at 08:26:52AM +0200, Marek Szyprowski wrote: > > > > > > > From: "Rafael J. Wysocki" > > > > > > We also have such a functional dependency for Thunderbolt on Macs: > > > > > > On resume from system sleep, the PCIe hotplug ports may not resume > > > > > > before the thunderbolt driver has reestablished the PCI tunnels. > > > > > > Currently this is enforced by quirk_apple_wait_for_thunderbolt() > > > > > > in drivers/pci/quirks.c. It would be good if we could represent > > > > > > this dependency using something like Rafael's approach instead of > > > > > > open coding it, however one detail in Rafael's patches is > > > > > > problematic: > > > > > > > > > > > > > New links are added by calling device_link_add() which may happen > > > > > > > either before the consumer device is probed or when probing it, in > > > > > > > which case the caller needs to ensure that the driver of the > > > > > > > supplier device is present and functional and the > > > > > > > DEVICE_LINK_PROBE_TIME > > > > > > > flag should be passed to device_link_add() to reflect that. > > > > > > > > > > > > The thunderbolt driver cannot call device_link_add() before the > > > > > > PCIe hotplug ports are bound to a driver unless we amend portdrv > > > > > > to return -EPROBE_DEFER for Thunderbolt hotplug ports on Macs > > > > > > if the thunderbolt driver isn't loaded. > > > > > > > > > > > > It would therefore be beneficial if device_link_add() can be > > > > > > called even *after* the consumer is bound. > > > > > > > > > > I don't quite follow. > > > > > > > > > > Who's the provider and who's the consumer here? > > > > > > > > thunderbolt.ko is the supplier. > > > > > > But it binds to the children of the ports that are supposed to be its > > > consumers? > > > > > > Why is that even expected to work? > > > > No, the consumers are aunts (or uncles) of the supplier, if you will. :-) > > > > The consumers are the hotplug ports (named "Downstream Bridge 1 / 2" in > > the drawing below). The supplier is the NHI: > > > > (Root Port) Upstream Bridge --+-- Downstream Bridge 0 NHI > > +-- Downstream Bridge 1 -- > > +-- Downstream Bridge 2 -- > > ... > > > > We're calling pci_power_up() and pci_restore_state() from > > pci_pm_resume_noirq(). And that will fail for devices below > > the hotplug ports if the PCI tunnels haven't been re-established > > yet by the NHI. > > So the NHI is a PCIe device, right? > > Does the Thunderbolt driver bind to that device? The NHI is a PCI device but not a bridge. It has class 0x88000. Yes, thunderbolt.ko binds to the NHI. And portdrv binds to the upstream bridge and downstream bridges. Those have class 0x60400. Best regards, Lukas ___ iommu mailing list iommu@lists.linux-foundation.org https://lists.linuxfoundation.org/mailman/listinfo/iommu
Re: [PATCH v2 02/10] driver core: Functional dependencies tracking support
On Wednesday, July 20, 2016 08:24:50 AM Lukas Wunner wrote: > On Wed, Jul 20, 2016 at 02:33:18AM +0200, Rafael J. Wysocki wrote: > > On Friday, June 17, 2016 04:07:38 PM Lukas Wunner wrote: > > > On Fri, Jun 17, 2016 at 02:54:56PM +0200, Rafael J. Wysocki wrote: > > > > On Fri, Jun 17, 2016 at 12:36 PM, Lukas Wunner wrote: > > > > > On Fri, Jun 17, 2016 at 08:26:52AM +0200, Marek Szyprowski wrote: > > > > > > From: "Rafael J. Wysocki" > > > > > We also have such a functional dependency for Thunderbolt on Macs: > > > > > On resume from system sleep, the PCIe hotplug ports may not resume > > > > > before the thunderbolt driver has reestablished the PCI tunnels. > > > > > Currently this is enforced by quirk_apple_wait_for_thunderbolt() > > > > > in drivers/pci/quirks.c. It would be good if we could represent > > > > > this dependency using something like Rafael's approach instead of > > > > > open coding it, however one detail in Rafael's patches is problematic: > > > > > > > > > > > New links are added by calling device_link_add() which may happen > > > > > > either before the consumer device is probed or when probing it, in > > > > > > which case the caller needs to ensure that the driver of the > > > > > > supplier device is present and functional and the > > > > > > DEVICE_LINK_PROBE_TIME > > > > > > flag should be passed to device_link_add() to reflect that. > > > > > > > > > > The thunderbolt driver cannot call device_link_add() before the > > > > > PCIe hotplug ports are bound to a driver unless we amend portdrv > > > > > to return -EPROBE_DEFER for Thunderbolt hotplug ports on Macs > > > > > if the thunderbolt driver isn't loaded. > > > > > > > > > > It would therefore be beneficial if device_link_add() can be > > > > > called even *after* the consumer is bound. > > > > > > > > I don't quite follow. > > > > > > > > Who's the provider and who's the consumer here? > > > > > > thunderbolt.ko is the supplier. > > > > But it binds to the children of the ports that are supposed to be its > > consumers? > > > > Why is that even expected to work? > > No, the consumers are aunts (or uncles) of the supplier, if you will. :-) > > The consumers are the hotplug ports (named "Downstream Bridge 1 / 2" in > the drawing below). The supplier is the NHI: > > (Root Port) Upstream Bridge --+-- Downstream Bridge 0 NHI > +-- Downstream Bridge 1 -- > +-- Downstream Bridge 2 -- > ... > > We're calling pci_power_up() and pci_restore_state() from > pci_pm_resume_noirq(). And that will fail for devices below > the hotplug ports if the PCI tunnels haven't been re-established > yet by the NHI. So the NHI is a PCIe device, right? Does the Thunderbolt driver bind to that device? > Currently we achieve that via quirk_apple_wait_for_thunderbolt() in > drivers/pci/quirks.c. It would be more elegant if we could make this > relationship explicit with "device links" and let the core handle it. > > Or am I mistaken and this particular use case is not what "device links" > are intended for? I'm not sure yet. Thanks, Rafael ___ iommu mailing list iommu@lists.linux-foundation.org https://lists.linuxfoundation.org/mailman/listinfo/iommu
Re: [PATCH v2 02/10] driver core: Functional dependencies tracking support
On Wed, Jul 20, 2016 at 02:33:18AM +0200, Rafael J. Wysocki wrote: > On Friday, June 17, 2016 04:07:38 PM Lukas Wunner wrote: > > On Fri, Jun 17, 2016 at 02:54:56PM +0200, Rafael J. Wysocki wrote: > > > On Fri, Jun 17, 2016 at 12:36 PM, Lukas Wunner wrote: > > > > On Fri, Jun 17, 2016 at 08:26:52AM +0200, Marek Szyprowski wrote: > > > > > From: "Rafael J. Wysocki" > > > > We also have such a functional dependency for Thunderbolt on Macs: > > > > On resume from system sleep, the PCIe hotplug ports may not resume > > > > before the thunderbolt driver has reestablished the PCI tunnels. > > > > Currently this is enforced by quirk_apple_wait_for_thunderbolt() > > > > in drivers/pci/quirks.c. It would be good if we could represent > > > > this dependency using something like Rafael's approach instead of > > > > open coding it, however one detail in Rafael's patches is problematic: > > > > > > > > > New links are added by calling device_link_add() which may happen > > > > > either before the consumer device is probed or when probing it, in > > > > > which case the caller needs to ensure that the driver of the > > > > > supplier device is present and functional and the > > > > > DEVICE_LINK_PROBE_TIME > > > > > flag should be passed to device_link_add() to reflect that. > > > > > > > > The thunderbolt driver cannot call device_link_add() before the > > > > PCIe hotplug ports are bound to a driver unless we amend portdrv > > > > to return -EPROBE_DEFER for Thunderbolt hotplug ports on Macs > > > > if the thunderbolt driver isn't loaded. > > > > > > > > It would therefore be beneficial if device_link_add() can be > > > > called even *after* the consumer is bound. > > > > > > I don't quite follow. > > > > > > Who's the provider and who's the consumer here? > > > > thunderbolt.ko is the supplier. > > But it binds to the children of the ports that are supposed to be its > consumers? > > Why is that even expected to work? No, the consumers are aunts (or uncles) of the supplier, if you will. :-) The consumers are the hotplug ports (named "Downstream Bridge 1 / 2" in the drawing below). The supplier is the NHI: (Root Port) Upstream Bridge --+-- Downstream Bridge 0 NHI +-- Downstream Bridge 1 -- +-- Downstream Bridge 2 -- ... We're calling pci_power_up() and pci_restore_state() from pci_pm_resume_noirq(). And that will fail for devices below the hotplug ports if the PCI tunnels haven't been re-established yet by the NHI. Currently we achieve that via quirk_apple_wait_for_thunderbolt() in drivers/pci/quirks.c. It would be more elegant if we could make this relationship explicit with "device links" and let the core handle it. Or am I mistaken and this particular use case is not what "device links" are intended for? Thanks, Lukas ___ iommu mailing list iommu@lists.linux-foundation.org https://lists.linuxfoundation.org/mailman/listinfo/iommu
Re: [PATCH v2 02/10] driver core: Functional dependencies tracking support
On Friday, June 17, 2016 04:07:38 PM Lukas Wunner wrote: > On Fri, Jun 17, 2016 at 02:54:56PM +0200, Rafael J. Wysocki wrote: > > On Fri, Jun 17, 2016 at 12:36 PM, Lukas Wunner wrote: > > > On Fri, Jun 17, 2016 at 08:26:52AM +0200, Marek Szyprowski wrote: > > > > From: "Rafael J. Wysocki" > > > We also have such a functional dependency for Thunderbolt on Macs: > > > On resume from system sleep, the PCIe hotplug ports may not resume > > > before the thunderbolt driver has reestablished the PCI tunnels. > > > Currently this is enforced by quirk_apple_wait_for_thunderbolt() > > > in drivers/pci/quirks.c. It would be good if we could represent > > > this dependency using something like Rafael's approach instead of > > > open coding it, however one detail in Rafael's patches is problematic: > > > > > > > New links are added by calling device_link_add() which may happen > > > > either before the consumer device is probed or when probing it, in > > > > which case the caller needs to ensure that the driver of the > > > > supplier device is present and functional and the DEVICE_LINK_PROBE_TIME > > > > flag should be passed to device_link_add() to reflect that. > > > > > > The thunderbolt driver cannot call device_link_add() before the > > > PCIe hotplug ports are bound to a driver unless we amend portdrv > > > to return -EPROBE_DEFER for Thunderbolt hotplug ports on Macs > > > if the thunderbolt driver isn't loaded. > > > > > > It would therefore be beneficial if device_link_add() can be > > > called even *after* the consumer is bound. > > > > I don't quite follow. > > > > Who's the provider and who's the consumer here? > > thunderbolt.ko is the supplier. But it binds to the children of the ports that are supposed to be its consumers? Why is that even expected to work? Thanks, Rafael ___ iommu mailing list iommu@lists.linux-foundation.org https://lists.linuxfoundation.org/mailman/listinfo/iommu
Re: [PATCH v2 02/10] driver core: Functional dependencies tracking support
On Fri, Jun 17, 2016 at 02:54:56PM +0200, Rafael J. Wysocki wrote: > On Fri, Jun 17, 2016 at 12:36 PM, Lukas Wunner wrote: > > On Fri, Jun 17, 2016 at 08:26:52AM +0200, Marek Szyprowski wrote: > > > From: "Rafael J. Wysocki" > > We also have such a functional dependency for Thunderbolt on Macs: > > On resume from system sleep, the PCIe hotplug ports may not resume > > before the thunderbolt driver has reestablished the PCI tunnels. > > Currently this is enforced by quirk_apple_wait_for_thunderbolt() > > in drivers/pci/quirks.c. It would be good if we could represent > > this dependency using something like Rafael's approach instead of > > open coding it, however one detail in Rafael's patches is problematic: > > > > > New links are added by calling device_link_add() which may happen > > > either before the consumer device is probed or when probing it, in > > > which case the caller needs to ensure that the driver of the > > > supplier device is present and functional and the DEVICE_LINK_PROBE_TIME > > > flag should be passed to device_link_add() to reflect that. > > > > The thunderbolt driver cannot call device_link_add() before the > > PCIe hotplug ports are bound to a driver unless we amend portdrv > > to return -EPROBE_DEFER for Thunderbolt hotplug ports on Macs > > if the thunderbolt driver isn't loaded. > > > > It would therefore be beneficial if device_link_add() can be > > called even *after* the consumer is bound. > > I don't quite follow. > > Who's the provider and who's the consumer here? thunderbolt.ko is the supplier. The PCIe hotplug ports of Thunderbolt host controllers are the consumers. If thunderbolt.ko is not loaded, no special precautions are needed for the hotplug ports. However *if* it is loaded and the system is suspended and there are Thunderbolt devices plugged in, then on resume the hotplug ports must wait for thunderbolt.ko to reestablish the PCI tunnels. If they do not wait, things will go south because when pci_pm_resume_noirq() is called for the devices *below* the hotplug port, we will try to put them into D0 (via pci_pm_default_resume_early()) and call pci_restore_state() even though those hotplugged devices are not yet reachable. So what I'd like to do is call device_link_add() when thunderbolt.ko is loaded to shove a dependency onto the hotplug ports. But the hotplug ports will long since have been bound at that point (to portdrv). So it should be allowed to call device_link_add() ex post facto, after the consumer has been bound. Let me know if I still failed to convey the problem in an intelligible way. I had a similar problem with the Runtime PM for Thunderbolt series (which BTW is still awaiting reviews): http://www.spinics.net/lists/linux-pci/msg51158.html The PM core allows calls to dev_pm_domain_set() only during ->probe or if the device is unbound. Same constraint as with device_link_add(). This constraint was a major obstacle for me and I had to jump through numerous additional hoops to work around it: Thunderbolt controllers on Macs can be put into D3cold, but not with standard ACPI methods, so platform_pci_power_manageable() is false for these devices. Now this nothing unusual, the same applies to dual GPU laptops (custom ACPI DSMs for Nvidia Optimus and AMD PowerXpress). For such discrete GPUs, a struct dev_pm_domain is set during ->probe (drivers/gpu/vga/vga_switcheroo.c: vga_switcheroo_init_domain_pm_ops()). But I can't do that for Thunderbolt because the device in question is a PCIe upstream port. Ideally I would shove a dev_pm_domain on the upstream port when thunderbolt.ko loads, but again at that point the port has long since been bound (to portdrv) so I can't call dev_pm_domain_set(). The workaround I settled on is to attach to the upstream port as a service driver, call down to the service driver's ->runtime_suspend hooks when the port runtime suspends and power the controller down. In addition, I had to make sure that saved_state isn't clobbered on resume (patch [09/13]). It would be good if someone who has a bird's eye view of the PM core (i.e., you) could make a decision (1) if it's possible and worthwhile to allow dev_pm_domain_set() for already bound devices, and (2) if the PCI core should be be able to deal with devices which are runtime suspended to D3cold but not by the platform (because that's what patch [09/13] does). If the answer to (2) is yes then there's some more stuff to fix: Discrete GPUs on dual GPU laptops can be manually suspended to D3cold behind the PM core's back by loading nouveau / radeon / amdgpu with runpm=0 and writing OFF to the vga_switcheroo debugfs interface. This is currently broken in conjunction with system sleep (the GPU is no longer accessible after resume) because pci_pm_resume_noirq() calls pci_restore_state() even though the device is in D3cold and not power-manageable by the platform. Thanks, Lukas ___ iommu mailing list iommu@lists.linux-fou
Re: [PATCH v2 02/10] driver core: Functional dependencies tracking support
On Fri, Jun 17, 2016 at 12:36 PM, Lukas Wunner wrote: > Hi Marek, > > On Fri, Jun 17, 2016 at 08:26:52AM +0200, Marek Szyprowski wrote: >> From: "Rafael J. Wysocki" >> >> Currently, there is a problem with handling cases where functional >> dependencies between devices are involved. >> >> What I mean by a "functional dependency" is when the driver of device >> B needs both device A and its driver to be present and functional to >> be able to work. This implies that the driver of A needs to be >> working for B to be probed successfully and it cannot be unbound from >> the device before the B's driver. This also has certain consequences >> for power management of these devices (suspend/resume and runtime PM >> ordering). >> >> Add support for representing those functional dependencies between >> devices to allow the driver core to track them and act on them in >> certain cases where they matter. > > Rafael has indicated that he intends to respin this series: > https://lkml.org/lkml/2016/6/8/1061 That's OK. > We also have such a functional dependency for Thunderbolt on Macs: > On resume from system sleep, the PCIe hotplug ports may not resume > before the thunderbolt driver has reestablished the PCI tunnels. > Currently this is enforced by quirk_apple_wait_for_thunderbolt() > in drivers/pci/quirks.c. It would be good if we could represent > this dependency using something like Rafael's approach instead of > open coding it, however one detail in Rafael's patches is problematic: > >> New links are added by calling device_link_add() which may happen >> either before the consumer device is probed or when probing it, in >> which case the caller needs to ensure that the driver of the >> supplier device is present and functional and the DEVICE_LINK_PROBE_TIME >> flag should be passed to device_link_add() to reflect that. > > The thunderbolt driver cannot call device_link_add() before the > PCIe hotplug ports are bound to a driver unless we amend portdrv > to return -EPROBE_DEFER for Thunderbolt hotplug ports on Macs > if the thunderbolt driver isn't loaded. > > It would therefore be beneficial if device_link_add() can be > called even *after* the consumer is bound. I don't quite follow. Who's the provider and who's the consumer here? Thanks, Rafael ___ iommu mailing list iommu@lists.linux-foundation.org https://lists.linuxfoundation.org/mailman/listinfo/iommu
Re: [PATCH v2 02/10] driver core: Functional dependencies tracking support
Hi Marek,
On Fri, Jun 17, 2016 at 08:26:52AM +0200, Marek Szyprowski wrote:
> From: "Rafael J. Wysocki"
>
> Currently, there is a problem with handling cases where functional
> dependencies between devices are involved.
>
> What I mean by a "functional dependency" is when the driver of device
> B needs both device A and its driver to be present and functional to
> be able to work. This implies that the driver of A needs to be
> working for B to be probed successfully and it cannot be unbound from
> the device before the B's driver. This also has certain consequences
> for power management of these devices (suspend/resume and runtime PM
> ordering).
>
> Add support for representing those functional dependencies between
> devices to allow the driver core to track them and act on them in
> certain cases where they matter.
Rafael has indicated that he intends to respin this series:
https://lkml.org/lkml/2016/6/8/1061
We also have such a functional dependency for Thunderbolt on Macs:
On resume from system sleep, the PCIe hotplug ports may not resume
before the thunderbolt driver has reestablished the PCI tunnels.
Currently this is enforced by quirk_apple_wait_for_thunderbolt()
in drivers/pci/quirks.c. It would be good if we could represent
this dependency using something like Rafael's approach instead of
open coding it, however one detail in Rafael's patches is problematic:
> New links are added by calling device_link_add() which may happen
> either before the consumer device is probed or when probing it, in
> which case the caller needs to ensure that the driver of the
> supplier device is present and functional and the DEVICE_LINK_PROBE_TIME
> flag should be passed to device_link_add() to reflect that.
The thunderbolt driver cannot call device_link_add() before the
PCIe hotplug ports are bound to a driver unless we amend portdrv
to return -EPROBE_DEFER for Thunderbolt hotplug ports on Macs
if the thunderbolt driver isn't loaded.
It would therefore be beneficial if device_link_add() can be
called even *after* the consumer is bound.
Thanks,
Lukas
>
> Link objects are deleted either explicitly, by calling
> device_link_del() on the link object in question, or automatically,
> when the consumer device is unbound from its driver or when one
> of the target devices is deleted, depending on the link type.
>
> There are two types of link objects, persistent and non-persistent.
> The difference between them is that the persistent links stay around
> until one of the target devices is deleted, which the non-persistent
> ones are deleted when the consumer driver is unbound from its device
> (ie. they are assumed to be valid only as long as the consumer device
> has a driver bound to it). The DEVICE_LINK_PERSISTENT flag has to
> be passed to device_link_add() so as to create a persistent link.
>
> One of the actions carried out by device_link_add() is to reorder
> the lists used for device shutdown and system suspend/resume to
> put the consumer device along with all of its children and all of
> its consumers (and so on, recursively) to the ends of those list
> in order to ensure the right ordering between the all of the supplier
> and consumer devices.
>
> Signed-off-by: Rafael J. Wysocki
> Signed-off-by: Marek Szyprowski
> ---
> drivers/base/base.h| 11 ++
> drivers/base/core.c| 386
> +
> drivers/base/dd.c | 42 +-
> include/linux/device.h | 36 +
> 4 files changed, 470 insertions(+), 5 deletions(-)
>
> diff --git a/drivers/base/base.h b/drivers/base/base.h
> index e05db388bd1c..cccb1d211541 100644
> --- a/drivers/base/base.h
> +++ b/drivers/base/base.h
> @@ -107,6 +107,9 @@ extern void bus_remove_device(struct device *dev);
>
> extern int bus_add_driver(struct device_driver *drv);
> extern void bus_remove_driver(struct device_driver *drv);
> +extern void device_release_driver_internal(struct device *dev,
> +struct device_driver *drv,
> +struct device *parent);
>
> extern void driver_detach(struct device_driver *drv);
> extern int driver_probe_device(struct device_driver *drv, struct device
> *dev);
> @@ -152,3 +155,11 @@ extern int devtmpfs_init(void);
> #else
> static inline int devtmpfs_init(void) { return 0; }
> #endif
> +
> +/* Device links */
> +extern int device_links_check_suppliers(struct device *dev);
> +extern void device_links_driver_bound(struct device *dev);
> +extern void device_links_driver_gone(struct device *dev);
> +extern void device_links_no_driver(struct device *dev);
> +extern bool device_links_busy(struct device *dev);
> +extern void device_links_unbind_consumers(struct device *dev);
> diff --git a/drivers/base/core.c b/drivers/base/core.c
> index 0a8bdade53f2..416341df3268 100644
> --- a/drivers/base/core.c
> +++ b/drivers/base/core.c
> @@ -44,6 +44,367 @@ static int __init sysfs_deprecated_setup(char *arg)
[PATCH v2 02/10] driver core: Functional dependencies tracking support
From: "Rafael J. Wysocki"
Currently, there is a problem with handling cases where functional
dependencies between devices are involved.
What I mean by a "functional dependency" is when the driver of device
B needs both device A and its driver to be present and functional to
be able to work. This implies that the driver of A needs to be
working for B to be probed successfully and it cannot be unbound from
the device before the B's driver. This also has certain consequences
for power management of these devices (suspend/resume and runtime PM
ordering).
Add support for representing those functional dependencies between
devices to allow the driver core to track them and act on them in
certain cases where they matter.
The argument for doing that in the driver core is that there are
quite a few distinct use cases related to that, they are relatively
hard to get right in a driver (if one wants to address all of them
properly) and it only gets worse if multiplied by the number of
drivers potentially needing to do it. Morever, at least one case
(asynchronous system suspend/resume) cannot be handled in a single
driver at all, because it requires the driver of A to wait for B to
suspend (during system suspend) and the driver of B to wait for
A to resume (during system resume).
To that end, represent links between devices (or more precisely
between device+driver combos) as a struct devlink object containing
pointers to the devices in question, a list node for each of them,
status information, flags, a lock and an RCU head for synchronization.
Also add two new list heads, supplier_links and consumer_links, to
struct device to represent the lists of links to the devices that
depend on the given one (consumers) and to the devices depended on
by it (suppliers), respectively.
The entire data structure consisting of all of the lists of link
objects for all devices is protected by SRCU (for list walking)
and a by mutex (for link object addition/removal). In addition
to that, each link object has an internal status field whose
value reflects what's happening to the devices pointed to by
the link. That status field is protected by an internal spinlock.
New links are added by calling device_link_add() which may happen
either before the consumer device is probed or when probing it, in
which case the caller needs to ensure that the driver of the
supplier device is present and functional and the DEVICE_LINK_PROBE_TIME
flag should be passed to device_link_add() to reflect that.
Link objects are deleted either explicitly, by calling
device_link_del() on the link object in question, or automatically,
when the consumer device is unbound from its driver or when one
of the target devices is deleted, depending on the link type.
There are two types of link objects, persistent and non-persistent.
The difference between them is that the persistent links stay around
until one of the target devices is deleted, which the non-persistent
ones are deleted when the consumer driver is unbound from its device
(ie. they are assumed to be valid only as long as the consumer device
has a driver bound to it). The DEVICE_LINK_PERSISTENT flag has to
be passed to device_link_add() so as to create a persistent link.
One of the actions carried out by device_link_add() is to reorder
the lists used for device shutdown and system suspend/resume to
put the consumer device along with all of its children and all of
its consumers (and so on, recursively) to the ends of those list
in order to ensure the right ordering between the all of the supplier
and consumer devices.
Signed-off-by: Rafael J. Wysocki
Signed-off-by: Marek Szyprowski
---
drivers/base/base.h| 11 ++
drivers/base/core.c| 386 +
drivers/base/dd.c | 42 +-
include/linux/device.h | 36 +
4 files changed, 470 insertions(+), 5 deletions(-)
diff --git a/drivers/base/base.h b/drivers/base/base.h
index e05db388bd1c..cccb1d211541 100644
--- a/drivers/base/base.h
+++ b/drivers/base/base.h
@@ -107,6 +107,9 @@ extern void bus_remove_device(struct device *dev);
extern int bus_add_driver(struct device_driver *drv);
extern void bus_remove_driver(struct device_driver *drv);
+extern void device_release_driver_internal(struct device *dev,
+ struct device_driver *drv,
+ struct device *parent);
extern void driver_detach(struct device_driver *drv);
extern int driver_probe_device(struct device_driver *drv, struct device *dev);
@@ -152,3 +155,11 @@ extern int devtmpfs_init(void);
#else
static inline int devtmpfs_init(void) { return 0; }
#endif
+
+/* Device links */
+extern int device_links_check_suppliers(struct device *dev);
+extern void device_links_driver_bound(struct device *dev);
+extern void device_links_driver_gone(struct device *dev);
+extern void device_links_no_driver(struct device *dev);
+extern bool device_links_busy(struct dev
