* [RFD] Functional dependencies between devices @ 2015-10-27 15:24 Rafael J. Wysocki 2015-10-27 15:20 ` Tomeu Vizoso ` (7 more replies) 0 siblings, 8 replies; 55+ messages in thread From: Rafael J. Wysocki @ 2015-10-27 15:24 UTC (permalink / raw) To: Linux PM list Cc: Greg Kroah-Hartman, Linux Kernel Mailing List, Alan Stern, Grant Likely, Mark Brown, Rob Herring, Tomeu Vizoso, Thierry Reding, Dmitry Torokhov, Geert Uytterhoeven, Michael Turquette Hi All, As discussed in the recent "On-demand device probing" thread and in a Kernel Summit session earlier today, 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). So I want to be able to represent those functional dependencies between devices and I'd like 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). My idea is to represent a supplier-consumer dependency between devices (or more precisely between device+driver combos) as a "link" object containing pointers to the devices in question, a list node for each of them and some additional information related to the management of those objects, ie. something like: struct device_link { struct device *supplier; struct list_head supplier_node; struct device *consumer; struct list_head consumer_node; <flags, status etc> }; In general, there will be two lists of those things per device, one list of links to consumers and one list of links to suppliers. In that picture, links will be created by calling, say: int device_add_link(struct device *me, struct device *my_supplier, unsigned int flags); and they will be deleted by the driver core when not needed any more. The creation of a link should also cause dpm_list and the list used during shutdown to be reordered if needed. In principle, it seems usefult to consider two types of links, one created at device registration time (when registering the second device from the linked pair, whichever it is) and one created at probe time (of the consumer device). I'll refer to them as "permanent" and "probe-time" links, respectively. The permanent links (created at device registration time) will stay around until one of the linked devices is unregistered (at which time the driver core will drop the link along with the device going away). The probe-time ones will be dropped (automatically) at the consumer device driver unbind time. There's a question about what if the supplier device is being unbound before the consumer one (for example, as a result of a hotplug event). My current view on that is that the consumer needs to be force-unbound in that case too, but I guess I may be persuaded otherwise given sufficiently convincing arguments. Anyway, there are reasons to do that, like for example it may help with the synchronization. Namely, if there's a rule that suppliers cannot be unbound before any consumers linked to them, than the list of links to suppliers for a consumer can only change at its registration/probe or unbind/remove times (which simplifies things quite a bit). With that, the permanent links existing at the probe time for a consumer device can be used to check whether or not to defer the probing of it even before executing its probe callback. In turn, system suspend synchronization should be a matter of calling device_pm_wait_for_dev() for all consumers of a supplier device, in analogy with dpm_wait_for_children(), and so on. Of course, the new lists have to be stable during those operations and ensuring that is going to be somewhat tricky (AFAICS right now at least), but apart from that the whole concept looks reasonably straightforward to me. So, the question to everybody is whether or not this sounds reasonable or there are concerns about it and if so what they are. At this point I mostly need to know if I'm not overlooking anything fundamental at the general level. Thanks, Rafael ^ permalink raw reply [flat|nested] 55+ messages in thread
* Re: [RFD] Functional dependencies between devices 2015-10-27 15:24 [RFD] Functional dependencies between devices Rafael J. Wysocki @ 2015-10-27 15:20 ` Tomeu Vizoso 2015-10-28 2:15 ` Rafael J. Wysocki 2015-10-29 0:15 ` Mark Brown ` (6 subsequent siblings) 7 siblings, 1 reply; 55+ messages in thread From: Tomeu Vizoso @ 2015-10-27 15:20 UTC (permalink / raw) To: Rafael J. Wysocki Cc: Linux PM list, Greg Kroah-Hartman, Linux Kernel Mailing List, Alan Stern, Grant Likely, Mark Brown, Rob Herring, Thierry Reding, Dmitry Torokhov, Geert Uytterhoeven, Michael Turquette On 27 October 2015 at 16:24, Rafael J. Wysocki <rjw@rjwysocki.net> wrote: > Hi All, > > As discussed in the recent "On-demand device probing" thread and in a Kernel > Summit session earlier today, 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). > > So I want to be able to represent those functional dependencies between devices > and I'd like 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). > > My idea is to represent a supplier-consumer dependency between devices (or > more precisely between device+driver combos) as a "link" object containing > pointers to the devices in question, a list node for each of them and some > additional information related to the management of those objects, ie. > something like: > > struct device_link { > struct device *supplier; > struct list_head supplier_node; > struct device *consumer; > struct list_head consumer_node; > <flags, status etc> > }; > > In general, there will be two lists of those things per device, one list > of links to consumers and one list of links to suppliers. > > In that picture, links will be created by calling, say: > > int device_add_link(struct device *me, struct device *my_supplier, unsigned int flags); > > and they will be deleted by the driver core when not needed any more. The > creation of a link should also cause dpm_list and the list used during shutdown > to be reordered if needed. > > In principle, it seems usefult to consider two types of links, one created > at device registration time (when registering the second device from the linked > pair, whichever it is) and one created at probe time (of the consumer device). > I'll refer to them as "permanent" and "probe-time" links, respectively. > > The permanent links (created at device registration time) will stay around > until one of the linked devices is unregistered (at which time the driver > core will drop the link along with the device going away). The probe-time > ones will be dropped (automatically) at the consumer device driver unbind time. > > There's a question about what if the supplier device is being unbound before > the consumer one (for example, as a result of a hotplug event). My current > view on that is that the consumer needs to be force-unbound in that case too, > but I guess I may be persuaded otherwise given sufficiently convincing > arguments. Anyway, there are reasons to do that, like for example it may > help with the synchronization. Namely, if there's a rule that suppliers > cannot be unbound before any consumers linked to them, than the list of links > to suppliers for a consumer can only change at its registration/probe or > unbind/remove times (which simplifies things quite a bit). > > With that, the permanent links existing at the probe time for a consumer > device can be used to check whether or not to defer the probing of it > even before executing its probe callback. In turn, system suspend > synchronization should be a matter of calling device_pm_wait_for_dev() > for all consumers of a supplier device, in analogy with dpm_wait_for_children(), > and so on. > > Of course, the new lists have to be stable during those operations and ensuring > that is going to be somewhat tricky (AFAICS right now at least), but apart from > that the whole concept looks reasonably straightforward to me. > > So, the question to everybody is whether or not this sounds reasonable or there > are concerns about it and if so what they are. At this point I mostly need to > know if I'm not overlooking anything fundamental at the general level. Sounds really great to me at the conceptual level, but wonder if you have already thought of how the permanent links will be inferred. When I looked at computing dependencies of a device before it's probed, the concern was that the code that finds the dependencies duplicated part of the logic when looking resources up. Because each subsystem has its own code for looking up dependencies for potentially each of DT, ACPI and board files, it will be a bit of a big task to refactor things to avoid that duplication. Fwnode could help with this, but it doesn't as of yet and I'm not sure if that's still the plan. Also wonder if you have considered setting the permanent links also during probe, as the on-demand probe series did (device_add_link would be "sprinkled" around as of_device_probe was). That would avoid the problem with code duplication because the links would be established from the functions that do resource lookup. Thanks, Tomeu > Thanks, > Rafael > > -- > To unsubscribe from this list: send the line "unsubscribe linux-kernel" in > the body of a message to majordomo@vger.kernel.org > More majordomo info at http://vger.kernel.org/majordomo-info.html > Please read the FAQ at http://www.tux.org/lkml/ ^ permalink raw reply [flat|nested] 55+ messages in thread
* Re: [RFD] Functional dependencies between devices 2015-10-27 15:20 ` Tomeu Vizoso @ 2015-10-28 2:15 ` Rafael J. Wysocki 2015-10-28 14:26 ` Tomeu Vizoso 0 siblings, 1 reply; 55+ messages in thread From: Rafael J. Wysocki @ 2015-10-28 2:15 UTC (permalink / raw) To: Tomeu Vizoso Cc: Linux PM list, Greg Kroah-Hartman, Linux Kernel Mailing List, Alan Stern, Grant Likely, Mark Brown, Rob Herring, Thierry Reding, Dmitry Torokhov, Geert Uytterhoeven, Michael Turquette On Tuesday, October 27, 2015 04:20:51 PM Tomeu Vizoso wrote: > On 27 October 2015 at 16:24, Rafael J. Wysocki <rjw@rjwysocki.net> wrote: > > Hi All, > > > > As discussed in the recent "On-demand device probing" thread and in a Kernel > > Summit session earlier today, 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). > > > > So I want to be able to represent those functional dependencies between devices > > and I'd like 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). > > > > My idea is to represent a supplier-consumer dependency between devices (or > > more precisely between device+driver combos) as a "link" object containing > > pointers to the devices in question, a list node for each of them and some > > additional information related to the management of those objects, ie. > > something like: > > > > struct device_link { > > struct device *supplier; > > struct list_head supplier_node; > > struct device *consumer; > > struct list_head consumer_node; > > <flags, status etc> > > }; > > > > In general, there will be two lists of those things per device, one list > > of links to consumers and one list of links to suppliers. > > > > In that picture, links will be created by calling, say: > > > > int device_add_link(struct device *me, struct device *my_supplier, unsigned int flags); > > > > and they will be deleted by the driver core when not needed any more. The > > creation of a link should also cause dpm_list and the list used during shutdown > > to be reordered if needed. > > > > In principle, it seems usefult to consider two types of links, one created > > at device registration time (when registering the second device from the linked > > pair, whichever it is) and one created at probe time (of the consumer device). > > I'll refer to them as "permanent" and "probe-time" links, respectively. > > > > The permanent links (created at device registration time) will stay around > > until one of the linked devices is unregistered (at which time the driver > > core will drop the link along with the device going away). The probe-time > > ones will be dropped (automatically) at the consumer device driver unbind time. > > > > There's a question about what if the supplier device is being unbound before > > the consumer one (for example, as a result of a hotplug event). My current > > view on that is that the consumer needs to be force-unbound in that case too, > > but I guess I may be persuaded otherwise given sufficiently convincing > > arguments. Anyway, there are reasons to do that, like for example it may > > help with the synchronization. Namely, if there's a rule that suppliers > > cannot be unbound before any consumers linked to them, than the list of links > > to suppliers for a consumer can only change at its registration/probe or > > unbind/remove times (which simplifies things quite a bit). > > > > With that, the permanent links existing at the probe time for a consumer > > device can be used to check whether or not to defer the probing of it > > even before executing its probe callback. In turn, system suspend > > synchronization should be a matter of calling device_pm_wait_for_dev() > > for all consumers of a supplier device, in analogy with dpm_wait_for_children(), > > and so on. > > > > Of course, the new lists have to be stable during those operations and ensuring > > that is going to be somewhat tricky (AFAICS right now at least), but apart from > > that the whole concept looks reasonably straightforward to me. > > > > So, the question to everybody is whether or not this sounds reasonable or there > > are concerns about it and if so what they are. At this point I mostly need to > > know if I'm not overlooking anything fundamental at the general level. > > Sounds really great to me at the conceptual level, but wonder if you > have already thought of how the permanent links will be inferred. In ACPI there is a mechanism for that already. In DT it would require walking the phandle dependency graph I suppose. The point is, though, that it doesn't have to be mandatory to have any permanent links created. If you can find a dependency at device registration time, great. Create a permanent link for it and use it. If you can't, it's fine too. You'll find it at probe time and create a link for it then. > When I looked at computing dependencies of a device before it's > probed, the concern was that the code that finds the dependencies > duplicated part of the logic when looking resources up. Because each > subsystem has its own code for looking up dependencies for potentially > each of DT, ACPI and board files, it will be a bit of a big task to > refactor things to avoid that duplication. Fwnode could help with > this, but it doesn't as of yet and I'm not sure if that's still the > plan. That almost certainly is going to be a fair amount of work, but that doesn't mean we should avoid doing it. If it leads to better code eventually, it's worth doing. > Also wonder if you have considered setting the permanent links also > during probe, as the on-demand probe series did (device_add_link would > be "sprinkled" around as of_device_probe was). That would avoid the > problem with code duplication because the links would be established > from the functions that do resource lookup. I have considered that, but at this point I have some concerns about lifecycle management related to that. In any case, if the given link is really permanent, there should be enough information available to find it at device registration time, although that may require some additional computations to be carried out. Thanks, Rafael ^ permalink raw reply [flat|nested] 55+ messages in thread
* Re: [RFD] Functional dependencies between devices 2015-10-28 2:15 ` Rafael J. Wysocki @ 2015-10-28 14:26 ` Tomeu Vizoso 2015-10-28 15:54 ` Rafael J. Wysocki 0 siblings, 1 reply; 55+ messages in thread From: Tomeu Vizoso @ 2015-10-28 14:26 UTC (permalink / raw) To: Rafael J. Wysocki Cc: Linux PM list, Greg Kroah-Hartman, Linux Kernel Mailing List, Alan Stern, Grant Likely, Mark Brown, Rob Herring, Thierry Reding, Dmitry Torokhov, Geert Uytterhoeven, Michael Turquette On 28 October 2015 at 03:15, Rafael J. Wysocki <rjw@rjwysocki.net> wrote: > On Tuesday, October 27, 2015 04:20:51 PM Tomeu Vizoso wrote: >> On 27 October 2015 at 16:24, Rafael J. Wysocki <rjw@rjwysocki.net> wrote: >> > Hi All, >> > >> > As discussed in the recent "On-demand device probing" thread and in a Kernel >> > Summit session earlier today, 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). >> > >> > So I want to be able to represent those functional dependencies between devices >> > and I'd like 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). >> > >> > My idea is to represent a supplier-consumer dependency between devices (or >> > more precisely between device+driver combos) as a "link" object containing >> > pointers to the devices in question, a list node for each of them and some >> > additional information related to the management of those objects, ie. >> > something like: >> > >> > struct device_link { >> > struct device *supplier; >> > struct list_head supplier_node; >> > struct device *consumer; >> > struct list_head consumer_node; >> > <flags, status etc> >> > }; >> > >> > In general, there will be two lists of those things per device, one list >> > of links to consumers and one list of links to suppliers. >> > >> > In that picture, links will be created by calling, say: >> > >> > int device_add_link(struct device *me, struct device *my_supplier, unsigned int flags); >> > >> > and they will be deleted by the driver core when not needed any more. The >> > creation of a link should also cause dpm_list and the list used during shutdown >> > to be reordered if needed. >> > >> > In principle, it seems usefult to consider two types of links, one created >> > at device registration time (when registering the second device from the linked >> > pair, whichever it is) and one created at probe time (of the consumer device). >> > I'll refer to them as "permanent" and "probe-time" links, respectively. >> > >> > The permanent links (created at device registration time) will stay around >> > until one of the linked devices is unregistered (at which time the driver >> > core will drop the link along with the device going away). The probe-time >> > ones will be dropped (automatically) at the consumer device driver unbind time. >> > >> > There's a question about what if the supplier device is being unbound before >> > the consumer one (for example, as a result of a hotplug event). My current >> > view on that is that the consumer needs to be force-unbound in that case too, >> > but I guess I may be persuaded otherwise given sufficiently convincing >> > arguments. Anyway, there are reasons to do that, like for example it may >> > help with the synchronization. Namely, if there's a rule that suppliers >> > cannot be unbound before any consumers linked to them, than the list of links >> > to suppliers for a consumer can only change at its registration/probe or >> > unbind/remove times (which simplifies things quite a bit). >> > >> > With that, the permanent links existing at the probe time for a consumer >> > device can be used to check whether or not to defer the probing of it >> > even before executing its probe callback. In turn, system suspend >> > synchronization should be a matter of calling device_pm_wait_for_dev() >> > for all consumers of a supplier device, in analogy with dpm_wait_for_children(), >> > and so on. >> > >> > Of course, the new lists have to be stable during those operations and ensuring >> > that is going to be somewhat tricky (AFAICS right now at least), but apart from >> > that the whole concept looks reasonably straightforward to me. >> > >> > So, the question to everybody is whether or not this sounds reasonable or there >> > are concerns about it and if so what they are. At this point I mostly need to >> > know if I'm not overlooking anything fundamental at the general level. >> >> Sounds really great to me at the conceptual level, but wonder if you >> have already thought of how the permanent links will be inferred. > > In ACPI there is a mechanism for that already. In DT it would require walking > the phandle dependency graph I suppose. > > The point is, though, that it doesn't have to be mandatory to have any > permanent links created. If you can find a dependency at device registration > time, great. Create a permanent link for it and use it. If you can't, > it's fine too. You'll find it at probe time and create a link for it then. > >> When I looked at computing dependencies of a device before it's >> probed, the concern was that the code that finds the dependencies >> duplicated part of the logic when looking resources up. Because each >> subsystem has its own code for looking up dependencies for potentially >> each of DT, ACPI and board files, it will be a bit of a big task to >> refactor things to avoid that duplication. Fwnode could help with >> this, but it doesn't as of yet and I'm not sure if that's still the >> plan. > > That almost certainly is going to be a fair amount of work, but that > doesn't mean we should avoid doing it. If it leads to better code > eventually, it's worth doing. > >> Also wonder if you have considered setting the permanent links also >> during probe, as the on-demand probe series did (device_add_link would >> be "sprinkled" around as of_device_probe was). That would avoid the >> problem with code duplication because the links would be established >> from the functions that do resource lookup. > > I have considered that, but at this point I have some concerns about > lifecycle management related to that. Hi Rafael, could you extend on why do you prefer inferring the permanent links before probe as opposed to collecting them during probe from the lookup functions? Also, have you considered that not only drivers request resources? For example, the on-demand probing series would probe a device that is needed by an initcall, simplifying synchronization. Regards, Tomeu > In any case, if the given link is really permanent, there should be enough > information available to find it at device registration time, although that > may require some additional computations to be carried out. > > Thanks, > Rafael > > -- > To unsubscribe from this list: send the line "unsubscribe linux-kernel" in > the body of a message to majordomo@vger.kernel.org > More majordomo info at http://vger.kernel.org/majordomo-info.html > Please read the FAQ at http://www.tux.org/lkml/ ^ permalink raw reply [flat|nested] 55+ messages in thread
* Re: [RFD] Functional dependencies between devices 2015-10-28 14:26 ` Tomeu Vizoso @ 2015-10-28 15:54 ` Rafael J. Wysocki 2015-10-29 0:18 ` Mark Brown 2015-10-29 14:03 ` Tomeu Vizoso 0 siblings, 2 replies; 55+ messages in thread From: Rafael J. Wysocki @ 2015-10-28 15:54 UTC (permalink / raw) To: Tomeu Vizoso Cc: Linux PM list, Greg Kroah-Hartman, Linux Kernel Mailing List, Alan Stern, Grant Likely, Mark Brown, Rob Herring, Thierry Reding, Dmitry Torokhov, Geert Uytterhoeven, Michael Turquette On Wednesday, October 28, 2015 03:26:14 PM Tomeu Vizoso wrote: > On 28 October 2015 at 03:15, Rafael J. Wysocki <rjw@rjwysocki.net> wrote: > > On Tuesday, October 27, 2015 04:20:51 PM Tomeu Vizoso wrote: > >> On 27 October 2015 at 16:24, Rafael J. Wysocki <rjw@rjwysocki.net> wrote: > >> > Hi All, > >> > > >> > As discussed in the recent "On-demand device probing" thread and in a Kernel > >> > Summit session earlier today, 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). > >> > > >> > So I want to be able to represent those functional dependencies between devices > >> > and I'd like 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). > >> > > >> > My idea is to represent a supplier-consumer dependency between devices (or > >> > more precisely between device+driver combos) as a "link" object containing > >> > pointers to the devices in question, a list node for each of them and some > >> > additional information related to the management of those objects, ie. > >> > something like: > >> > > >> > struct device_link { > >> > struct device *supplier; > >> > struct list_head supplier_node; > >> > struct device *consumer; > >> > struct list_head consumer_node; > >> > <flags, status etc> > >> > }; > >> > > >> > In general, there will be two lists of those things per device, one list > >> > of links to consumers and one list of links to suppliers. > >> > > >> > In that picture, links will be created by calling, say: > >> > > >> > int device_add_link(struct device *me, struct device *my_supplier, unsigned int flags); > >> > > >> > and they will be deleted by the driver core when not needed any more. The > >> > creation of a link should also cause dpm_list and the list used during shutdown > >> > to be reordered if needed. > >> > > >> > In principle, it seems usefult to consider two types of links, one created > >> > at device registration time (when registering the second device from the linked > >> > pair, whichever it is) and one created at probe time (of the consumer device). > >> > I'll refer to them as "permanent" and "probe-time" links, respectively. > >> > > >> > The permanent links (created at device registration time) will stay around > >> > until one of the linked devices is unregistered (at which time the driver > >> > core will drop the link along with the device going away). The probe-time > >> > ones will be dropped (automatically) at the consumer device driver unbind time. > >> > > >> > There's a question about what if the supplier device is being unbound before > >> > the consumer one (for example, as a result of a hotplug event). My current > >> > view on that is that the consumer needs to be force-unbound in that case too, > >> > but I guess I may be persuaded otherwise given sufficiently convincing > >> > arguments. Anyway, there are reasons to do that, like for example it may > >> > help with the synchronization. Namely, if there's a rule that suppliers > >> > cannot be unbound before any consumers linked to them, than the list of links > >> > to suppliers for a consumer can only change at its registration/probe or > >> > unbind/remove times (which simplifies things quite a bit). > >> > > >> > With that, the permanent links existing at the probe time for a consumer > >> > device can be used to check whether or not to defer the probing of it > >> > even before executing its probe callback. In turn, system suspend > >> > synchronization should be a matter of calling device_pm_wait_for_dev() > >> > for all consumers of a supplier device, in analogy with dpm_wait_for_children(), > >> > and so on. > >> > > >> > Of course, the new lists have to be stable during those operations and ensuring > >> > that is going to be somewhat tricky (AFAICS right now at least), but apart from > >> > that the whole concept looks reasonably straightforward to me. > >> > > >> > So, the question to everybody is whether or not this sounds reasonable or there > >> > are concerns about it and if so what they are. At this point I mostly need to > >> > know if I'm not overlooking anything fundamental at the general level. > >> > >> Sounds really great to me at the conceptual level, but wonder if you > >> have already thought of how the permanent links will be inferred. > > > > In ACPI there is a mechanism for that already. In DT it would require walking > > the phandle dependency graph I suppose. > > > > The point is, though, that it doesn't have to be mandatory to have any > > permanent links created. If you can find a dependency at device registration > > time, great. Create a permanent link for it and use it. If you can't, > > it's fine too. You'll find it at probe time and create a link for it then. > > > >> When I looked at computing dependencies of a device before it's > >> probed, the concern was that the code that finds the dependencies > >> duplicated part of the logic when looking resources up. Because each > >> subsystem has its own code for looking up dependencies for potentially > >> each of DT, ACPI and board files, it will be a bit of a big task to > >> refactor things to avoid that duplication. Fwnode could help with > >> this, but it doesn't as of yet and I'm not sure if that's still the > >> plan. > > > > That almost certainly is going to be a fair amount of work, but that > > doesn't mean we should avoid doing it. If it leads to better code > > eventually, it's worth doing. > > > >> Also wonder if you have considered setting the permanent links also > >> during probe, as the on-demand probe series did (device_add_link would > >> be "sprinkled" around as of_device_probe was). That would avoid the > >> problem with code duplication because the links would be established > >> from the functions that do resource lookup. > > > > I have considered that, but at this point I have some concerns about > > lifecycle management related to that. > > Hi Rafael, > > could you extend on why do you prefer inferring the permanent links > before probe as opposed to collecting them during probe from the > lookup functions? Information that is already available at the device registration time should be used at that time or it makes things harder to follow. But that really is a tradeoff. If collecting that information requires too much effort, it may not be worth it. > Also, have you considered that not only drivers request resources? For > example, the on-demand probing series would probe a device that is > needed by an initcall, simplifying synchronization. You really need to explain what you mean here or maybe give an example. Thanks, Rafael ^ permalink raw reply [flat|nested] 55+ messages in thread
* Re: [RFD] Functional dependencies between devices 2015-10-28 15:54 ` Rafael J. Wysocki @ 2015-10-29 0:18 ` Mark Brown 2015-10-29 14:03 ` Tomeu Vizoso 1 sibling, 0 replies; 55+ messages in thread From: Mark Brown @ 2015-10-29 0:18 UTC (permalink / raw) To: Rafael J. Wysocki Cc: Tomeu Vizoso, Linux PM list, Greg Kroah-Hartman, Linux Kernel Mailing List, Alan Stern, Grant Likely, Rob Herring, Thierry Reding, Dmitry Torokhov, Geert Uytterhoeven, Michael Turquette [-- Attachment #1: Type: text/plain, Size: 495 bytes --] On Wed, Oct 28, 2015 at 04:54:04PM +0100, Rafael J. Wysocki wrote: > Information that is already available at the device registration time should > be used at that time or it makes things harder to follow. > But that really is a tradeoff. If collecting that information requires too > much effort, it may not be worth it. For DT it's going to be a lot eaiser to reliably collect everything in driver specific code, the property names to look at do follow conventions but are driver defined. [-- Attachment #2: signature.asc --] [-- Type: application/pgp-signature, Size: 473 bytes --] ^ permalink raw reply [flat|nested] 55+ messages in thread
* Re: [RFD] Functional dependencies between devices 2015-10-28 15:54 ` Rafael J. Wysocki 2015-10-29 0:18 ` Mark Brown @ 2015-10-29 14:03 ` Tomeu Vizoso 2015-10-29 14:31 ` Alan Stern 1 sibling, 1 reply; 55+ messages in thread From: Tomeu Vizoso @ 2015-10-29 14:03 UTC (permalink / raw) To: Rafael J. Wysocki Cc: Linux PM list, Greg Kroah-Hartman, Linux Kernel Mailing List, Alan Stern, Grant Likely, Mark Brown, Rob Herring, Thierry Reding, Dmitry Torokhov, Geert Uytterhoeven, Michael Turquette On 28 October 2015 at 16:54, Rafael J. Wysocki <rjw@rjwysocki.net> wrote: > On Wednesday, October 28, 2015 03:26:14 PM Tomeu Vizoso wrote: >> On 28 October 2015 at 03:15, Rafael J. Wysocki <rjw@rjwysocki.net> wrote: >> > On Tuesday, October 27, 2015 04:20:51 PM Tomeu Vizoso wrote: >> >> On 27 October 2015 at 16:24, Rafael J. Wysocki <rjw@rjwysocki.net> wrote: >> >> > Hi All, >> >> > >> >> > As discussed in the recent "On-demand device probing" thread and in a Kernel >> >> > Summit session earlier today, 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). >> >> > >> >> > So I want to be able to represent those functional dependencies between devices >> >> > and I'd like 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). >> >> > >> >> > My idea is to represent a supplier-consumer dependency between devices (or >> >> > more precisely between device+driver combos) as a "link" object containing >> >> > pointers to the devices in question, a list node for each of them and some >> >> > additional information related to the management of those objects, ie. >> >> > something like: >> >> > >> >> > struct device_link { >> >> > struct device *supplier; >> >> > struct list_head supplier_node; >> >> > struct device *consumer; >> >> > struct list_head consumer_node; >> >> > <flags, status etc> >> >> > }; >> >> > >> >> > In general, there will be two lists of those things per device, one list >> >> > of links to consumers and one list of links to suppliers. >> >> > >> >> > In that picture, links will be created by calling, say: >> >> > >> >> > int device_add_link(struct device *me, struct device *my_supplier, unsigned int flags); >> >> > >> >> > and they will be deleted by the driver core when not needed any more. The >> >> > creation of a link should also cause dpm_list and the list used during shutdown >> >> > to be reordered if needed. >> >> > >> >> > In principle, it seems usefult to consider two types of links, one created >> >> > at device registration time (when registering the second device from the linked >> >> > pair, whichever it is) and one created at probe time (of the consumer device). >> >> > I'll refer to them as "permanent" and "probe-time" links, respectively. >> >> > >> >> > The permanent links (created at device registration time) will stay around >> >> > until one of the linked devices is unregistered (at which time the driver >> >> > core will drop the link along with the device going away). The probe-time >> >> > ones will be dropped (automatically) at the consumer device driver unbind time. >> >> > >> >> > There's a question about what if the supplier device is being unbound before >> >> > the consumer one (for example, as a result of a hotplug event). My current >> >> > view on that is that the consumer needs to be force-unbound in that case too, >> >> > but I guess I may be persuaded otherwise given sufficiently convincing >> >> > arguments. Anyway, there are reasons to do that, like for example it may >> >> > help with the synchronization. Namely, if there's a rule that suppliers >> >> > cannot be unbound before any consumers linked to them, than the list of links >> >> > to suppliers for a consumer can only change at its registration/probe or >> >> > unbind/remove times (which simplifies things quite a bit). >> >> > >> >> > With that, the permanent links existing at the probe time for a consumer >> >> > device can be used to check whether or not to defer the probing of it >> >> > even before executing its probe callback. In turn, system suspend >> >> > synchronization should be a matter of calling device_pm_wait_for_dev() >> >> > for all consumers of a supplier device, in analogy with dpm_wait_for_children(), >> >> > and so on. >> >> > >> >> > Of course, the new lists have to be stable during those operations and ensuring >> >> > that is going to be somewhat tricky (AFAICS right now at least), but apart from >> >> > that the whole concept looks reasonably straightforward to me. >> >> > >> >> > So, the question to everybody is whether or not this sounds reasonable or there >> >> > are concerns about it and if so what they are. At this point I mostly need to >> >> > know if I'm not overlooking anything fundamental at the general level. >> >> >> >> Sounds really great to me at the conceptual level, but wonder if you >> >> have already thought of how the permanent links will be inferred. >> > >> > In ACPI there is a mechanism for that already. In DT it would require walking >> > the phandle dependency graph I suppose. >> > >> > The point is, though, that it doesn't have to be mandatory to have any >> > permanent links created. If you can find a dependency at device registration >> > time, great. Create a permanent link for it and use it. If you can't, >> > it's fine too. You'll find it at probe time and create a link for it then. >> > >> >> When I looked at computing dependencies of a device before it's >> >> probed, the concern was that the code that finds the dependencies >> >> duplicated part of the logic when looking resources up. Because each >> >> subsystem has its own code for looking up dependencies for potentially >> >> each of DT, ACPI and board files, it will be a bit of a big task to >> >> refactor things to avoid that duplication. Fwnode could help with >> >> this, but it doesn't as of yet and I'm not sure if that's still the >> >> plan. >> > >> > That almost certainly is going to be a fair amount of work, but that >> > doesn't mean we should avoid doing it. If it leads to better code >> > eventually, it's worth doing. >> > >> >> Also wonder if you have considered setting the permanent links also >> >> during probe, as the on-demand probe series did (device_add_link would >> >> be "sprinkled" around as of_device_probe was). That would avoid the >> >> problem with code duplication because the links would be established >> >> from the functions that do resource lookup. >> > >> > I have considered that, but at this point I have some concerns about >> > lifecycle management related to that. >> >> Hi Rafael, >> >> could you extend on why do you prefer inferring the permanent links >> before probe as opposed to collecting them during probe from the >> lookup functions? > > Information that is already available at the device registration time should > be used at that time or it makes things harder to follow. > > But that really is a tradeoff. If collecting that information requires too > much effort, it may not be worth it. > >> Also, have you considered that not only drivers request resources? For >> example, the on-demand probing series would probe a device that is >> needed by an initcall, simplifying synchronization. > > You really need to explain what you mean here or maybe give an example. There are initcalls that assume that a given resource is available. Because of async probes, or because the resource's driver being built as a module, or because the resource's driver gained a dependency (direct or not), those initcalls break unexpectedly at times. If resource getters could probe dependencies on-demand, those initcalls would be more robust to changes in other parts of the codebase. AFAIUI, your proposal would help with a device's dependencies being there when it's probed, but initcalls could still run into unfulfilled dependencies. I would personally prefer if those initcalls were turned into drivers because that's the component model we have in the kernel, but that doesn't seem to be a popular idea. Regards, Tomeu > Thanks, > Rafael > > -- > To unsubscribe from this list: send the line "unsubscribe linux-kernel" in > the body of a message to majordomo@vger.kernel.org > More majordomo info at http://vger.kernel.org/majordomo-info.html > Please read the FAQ at http://www.tux.org/lkml/ ^ permalink raw reply [flat|nested] 55+ messages in thread
* Re: [RFD] Functional dependencies between devices 2015-10-29 14:03 ` Tomeu Vizoso @ 2015-10-29 14:31 ` Alan Stern 2015-10-31 2:23 ` Rafael J. Wysocki 0 siblings, 1 reply; 55+ messages in thread From: Alan Stern @ 2015-10-29 14:31 UTC (permalink / raw) To: Tomeu Vizoso Cc: Rafael J. Wysocki, Linux PM list, Greg Kroah-Hartman, Linux Kernel Mailing List, Grant Likely, Mark Brown, Rob Herring, Thierry Reding, Dmitry Torokhov, Geert Uytterhoeven, Michael Turquette Good grief, don't you guys ever trim unwanted material from your emails? I had to erase more than 4 screens worth of useless stuff before getting to the relevant portions. On Thu, 29 Oct 2015, Tomeu Vizoso wrote: > >> Also, have you considered that not only drivers request resources? For > >> example, the on-demand probing series would probe a device that is > >> needed by an initcall, simplifying synchronization. Did Rafael ever say that only drivers could create these functional dependencies? I don't recall seeing that anywhere. Presumably any part of the kernel will be allowed to do it. > > You really need to explain what you mean here or maybe give an example. > > There are initcalls that assume that a given resource is available. > Because of async probes, or because the resource's driver being built > as a module, or because the resource's driver gained a dependency > (direct or not), those initcalls break unexpectedly at times. > > If resource getters could probe dependencies on-demand, those > initcalls would be more robust to changes in other parts of the > codebase. > > AFAIUI, your proposal would help with a device's dependencies being > there when it's probed, but initcalls could still run into unfulfilled > dependencies. One possible approach is to have a "wait_for_driver" flag, along with a timeout value (or perhaps using a fixed timeout value). When a dependency gets registered with this flag set, the function call wouldn't return until the target device is bound to a driver or the timeout has elapsed. This would make it easy to insert dependencies at probe time without relying on deferred probing. Alan Stern ^ permalink raw reply [flat|nested] 55+ messages in thread
* Re: [RFD] Functional dependencies between devices 2015-10-29 14:31 ` Alan Stern @ 2015-10-31 2:23 ` Rafael J. Wysocki 2015-10-31 15:22 ` Alan Stern 0 siblings, 1 reply; 55+ messages in thread From: Rafael J. Wysocki @ 2015-10-31 2:23 UTC (permalink / raw) To: Alan Stern Cc: Tomeu Vizoso, Linux PM list, Greg Kroah-Hartman, Linux Kernel Mailing List, Grant Likely, Mark Brown, Rob Herring, Thierry Reding, Dmitry Torokhov, Geert Uytterhoeven, Michael Turquette On Thursday, October 29, 2015 10:31:08 AM Alan Stern wrote: > Good grief, don't you guys ever trim unwanted material from your > emails? I had to erase more than 4 screens worth of useless stuff > before getting to the relevant portions. > > On Thu, 29 Oct 2015, Tomeu Vizoso wrote: > > > >> Also, have you considered that not only drivers request resources? For > > >> example, the on-demand probing series would probe a device that is > > >> needed by an initcall, simplifying synchronization. > > Did Rafael ever say that only drivers could create these functional > dependencies? I don't recall seeing that anywhere. Presumably any > part of the kernel will be allowed to do it. Right. > > > You really need to explain what you mean here or maybe give an example. > > > > There are initcalls that assume that a given resource is available. > > Because of async probes, or because the resource's driver being built > > as a module, or because the resource's driver gained a dependency > > (direct or not), those initcalls break unexpectedly at times. Well, I'm still unsure what initcalls are in question here. > > If resource getters could probe dependencies on-demand, those > > initcalls would be more robust to changes in other parts of the > > codebase. > > > > AFAIUI, your proposal would help with a device's dependencies being > > there when it's probed, but initcalls could still run into unfulfilled > > dependencies. > > One possible approach is to have a "wait_for_driver" flag, along with a > timeout value (or perhaps using a fixed timeout value). When a > dependency gets registered with this flag set, the function call > wouldn't return until the target device is bound to a driver or the > timeout has elapsed. > > This would make it easy to insert dependencies at probe time without > relying on deferred probing. I'm not sure about this to be honest. It seems like implementing it might be sort of tricky. Thanks, Rafael ^ permalink raw reply [flat|nested] 55+ messages in thread
* Re: [RFD] Functional dependencies between devices 2015-10-31 2:23 ` Rafael J. Wysocki @ 2015-10-31 15:22 ` Alan Stern 0 siblings, 0 replies; 55+ messages in thread From: Alan Stern @ 2015-10-31 15:22 UTC (permalink / raw) To: Rafael J. Wysocki Cc: Tomeu Vizoso, Linux PM list, Greg Kroah-Hartman, Linux Kernel Mailing List, Grant Likely, Mark Brown, Rob Herring, Thierry Reding, Dmitry Torokhov, Geert Uytterhoeven, Michael Turquette On Sat, 31 Oct 2015, Rafael J. Wysocki wrote: > > One possible approach is to have a "wait_for_driver" flag, along with a > > timeout value (or perhaps using a fixed timeout value). When a > > dependency gets registered with this flag set, the function call > > wouldn't return until the target device is bound to a driver or the > > timeout has elapsed. > > > > This would make it easy to insert dependencies at probe time without > > relying on deferred probing. > > I'm not sure about this to be honest. It seems like implementing it might > be sort of tricky. Well, clearly it wouldn't work if probes were serialized and one driver was dependent on another that was going to be probed later. But if probes are asynchronous then it ought to be okay (unless there is a dependency cycle, in which case the situation is hopeless anyway). Alan Stern ^ permalink raw reply [flat|nested] 55+ messages in thread
* Re: [RFD] Functional dependencies between devices 2015-10-27 15:24 [RFD] Functional dependencies between devices Rafael J. Wysocki 2015-10-27 15:20 ` Tomeu Vizoso @ 2015-10-29 0:15 ` Mark Brown 2015-10-31 2:13 ` Rafael J. Wysocki 2015-10-30 9:50 ` Linus Walleij ` (5 subsequent siblings) 7 siblings, 1 reply; 55+ messages in thread From: Mark Brown @ 2015-10-29 0:15 UTC (permalink / raw) To: Rafael J. Wysocki Cc: Linux PM list, Greg Kroah-Hartman, Linux Kernel Mailing List, Alan Stern, Grant Likely, Rob Herring, Tomeu Vizoso, Thierry Reding, Dmitry Torokhov, Geert Uytterhoeven, Michael Turquette [-- Attachment #1: Type: text/plain, Size: 864 bytes --] On Tue, Oct 27, 2015 at 04:24:14PM +0100, Rafael J. Wysocki wrote: > So, the question to everybody is whether or not this sounds reasonable or there > are concerns about it and if so what they are. At this point I mostly need to > know if I'm not overlooking anything fundamental at the general level. This seems like a good plan to me however I am concerned that only allowing links to be created at device registration time will prove restrictive - it means we're going to ignore anything we figure out later on in the boot sequence. I would be very surprised if we didn't need that, either from things that get missed or from things that get allocated dynamically at runtime on systems with flexible hardware, and it'd also mean that systems can start to benefit from this for suspend and resume without needing the updates to the firmware parsing support. [-- Attachment #2: signature.asc --] [-- Type: application/pgp-signature, Size: 473 bytes --] ^ permalink raw reply [flat|nested] 55+ messages in thread
* Re: [RFD] Functional dependencies between devices 2015-10-29 0:15 ` Mark Brown @ 2015-10-31 2:13 ` Rafael J. Wysocki 2015-10-31 2:40 ` Mark Brown 0 siblings, 1 reply; 55+ messages in thread From: Rafael J. Wysocki @ 2015-10-31 2:13 UTC (permalink / raw) To: Mark Brown Cc: Linux PM list, Greg Kroah-Hartman, Linux Kernel Mailing List, Alan Stern, Grant Likely, Rob Herring, Tomeu Vizoso, Thierry Reding, Dmitry Torokhov, Geert Uytterhoeven, Michael Turquette [-- Attachment #1: Type: text/plain, Size: 1718 bytes --] On Thursday, October 29, 2015 09:15:09 AM Mark Brown wrote: > On Tue, Oct 27, 2015 at 04:24:14PM +0100, Rafael J. Wysocki wrote: > > > So, the question to everybody is whether or not this sounds reasonable or there > > are concerns about it and if so what they are. At this point I mostly need to > > know if I'm not overlooking anything fundamental at the general level. > > This seems like a good plan to me however I am concerned that only > allowing links to be created at device registration time will prove > restrictive - it means we're going to ignore anything we figure out > later on in the boot sequence. That's not the plan, though. :-) I'm talking about device registration time or device probe time (for dependencies that aren't known at the registration time). > I would be very surprised if we didn't > need that, either from things that get missed or from things that get > allocated dynamically at runtime on systems with flexible hardware, and > it'd also mean that systems can start to benefit from this for suspend > and resume without needing the updates to the firmware parsing support. The reason why I think it should be restricted to the probe/remove and registration/unregistration times is because of PM. More specifically, adding a link to a "supplier" causes additional actions to be taken during PM operations (eg. if the supplier device is suspended at the consumer device resume time, it needs to be resumed in the runtime PM case or waited for in the system resume case) which may be regarded as a change in a way PM is handled. Such changes are only expected to happen at the points I mentioned and may lead to rather undesirable outcomes if made elsewhere. Thanks, Rafael [-- Attachment #2: This is a digitally signed message part. --] [-- Type: application/pgp-signature, Size: 836 bytes --] ^ permalink raw reply [flat|nested] 55+ messages in thread
* Re: [RFD] Functional dependencies between devices 2015-10-31 2:13 ` Rafael J. Wysocki @ 2015-10-31 2:40 ` Mark Brown 0 siblings, 0 replies; 55+ messages in thread From: Mark Brown @ 2015-10-31 2:40 UTC (permalink / raw) To: Rafael J. Wysocki Cc: Linux PM list, Greg Kroah-Hartman, Linux Kernel Mailing List, Alan Stern, Grant Likely, Rob Herring, Tomeu Vizoso, Thierry Reding, Dmitry Torokhov, Geert Uytterhoeven, Michael Turquette [-- Attachment #1: Type: text/plain, Size: 708 bytes --] On Sat, Oct 31, 2015 at 03:13:09AM +0100, Rafael J. Wysocki wrote: > On Thursday, October 29, 2015 09:15:09 AM Mark Brown wrote: > > This seems like a good plan to me however I am concerned that only > > allowing links to be created at device registration time will prove > > restrictive - it means we're going to ignore anything we figure out > > later on in the boot sequence. > That's not the plan, though. :-) > I'm talking about device registration time or device probe time (for > dependencies that aren't known at the registration time). Oh, so I think allowing things to be added at probe time addresses my concerns - for some reason I'd parsed your mail as talking about registration time only. [-- Attachment #2: signature.asc --] [-- Type: application/pgp-signature, Size: 473 bytes --] ^ permalink raw reply [flat|nested] 55+ messages in thread
* Re: [RFD] Functional dependencies between devices 2015-10-27 15:24 [RFD] Functional dependencies between devices Rafael J. Wysocki 2015-10-27 15:20 ` Tomeu Vizoso 2015-10-29 0:15 ` Mark Brown @ 2015-10-30 9:50 ` Linus Walleij 2015-10-30 22:52 ` Greg Kroah-Hartman ` (4 subsequent siblings) 7 siblings, 0 replies; 55+ messages in thread From: Linus Walleij @ 2015-10-30 9:50 UTC (permalink / raw) To: Rafael J. Wysocki Cc: Linux PM list, Greg Kroah-Hartman, Linux Kernel Mailing List, Alan Stern, Grant Likely, Mark Brown, Rob Herring, Tomeu Vizoso, Thierry Reding, Dmitry Torokhov, Geert Uytterhoeven, Michael Turquette On Tue, Oct 27, 2015 at 4:24 PM, Rafael J. Wysocki <rjw@rjwysocki.net> wrote: > My idea is to represent a supplier-consumer dependency between devices (or > more precisely between device+driver combos) as a "link" object containing > pointers to the devices in question, a list node for each of them and some > additional information related to the management of those objects, ie. > something like: > > struct device_link { > struct device *supplier; > struct list_head supplier_node; > struct device *consumer; > struct list_head consumer_node; > <flags, status etc> > }; > > In general, there will be two lists of those things per device, one list > of links to consumers and one list of links to suppliers. I like this idea. I earlier have written that the device core needs to know the dependencies of devices as a graph. This mechanism does that. IIUC the mechanism does not inheritly protect against creating cyclic graphs (you can get stuck in a loop) but that is just the nature of the things and even deferred probe has the ability to shoot oneself in the foot. Besides we're not doing computer science here, we're solving practical problems. What I further like about the approach is that it can even be used by archs still doing boardfiles and not using any HW description mechanism: it is there for anyone. Those platforms can define dependencies with good old code. Yours, Linus Walleij ^ permalink raw reply [flat|nested] 55+ messages in thread
* Re: [RFD] Functional dependencies between devices 2015-10-27 15:24 [RFD] Functional dependencies between devices Rafael J. Wysocki ` (2 preceding siblings ...) 2015-10-30 9:50 ` Linus Walleij @ 2015-10-30 22:52 ` Greg Kroah-Hartman 2016-01-07 14:55 ` Tomeu Vizoso 2015-11-09 12:32 ` Thierry Reding ` (3 subsequent siblings) 7 siblings, 1 reply; 55+ messages in thread From: Greg Kroah-Hartman @ 2015-10-30 22:52 UTC (permalink / raw) To: Rafael J. Wysocki Cc: Linux PM list, Linux Kernel Mailing List, Alan Stern, Grant Likely, Mark Brown, Rob Herring, Tomeu Vizoso, Thierry Reding, Dmitry Torokhov, Geert Uytterhoeven, Michael Turquette On Tue, Oct 27, 2015 at 04:24:14PM +0100, Rafael J. Wysocki wrote: > My idea is to represent a supplier-consumer dependency between devices (or > more precisely between device+driver combos) as a "link" object containing > pointers to the devices in question, a list node for each of them and some > additional information related to the management of those objects, ie. > something like: > > struct device_link { > struct device *supplier; > struct list_head supplier_node; > struct device *consumer; > struct list_head consumer_node; > <flags, status etc> > }; > > In general, there will be two lists of those things per device, one list > of links to consumers and one list of links to suppliers. > > In that picture, links will be created by calling, say: > > int device_add_link(struct device *me, struct device *my_supplier, unsigned int flags); At first glance, I like this, nice. Now to see how well it can be implemented :) Again, nice job. greg k-h ^ permalink raw reply [flat|nested] 55+ messages in thread
* Re: [RFD] Functional dependencies between devices 2015-10-30 22:52 ` Greg Kroah-Hartman @ 2016-01-07 14:55 ` Tomeu Vizoso 2016-01-07 21:29 ` Greg Kroah-Hartman 0 siblings, 1 reply; 55+ messages in thread From: Tomeu Vizoso @ 2016-01-07 14:55 UTC (permalink / raw) To: Greg Kroah-Hartman Cc: Rafael J. Wysocki, Linux PM list, Linux Kernel Mailing List, Alan Stern, Grant Likely, Mark Brown, Rob Herring, Thierry Reding, Dmitry Torokhov, Geert Uytterhoeven, Michael Turquette On 30 October 2015 at 23:52, Greg Kroah-Hartman <gregkh@linuxfoundation.org> wrote: > On Tue, Oct 27, 2015 at 04:24:14PM +0100, Rafael J. Wysocki wrote: >> My idea is to represent a supplier-consumer dependency between devices (or >> more precisely between device+driver combos) as a "link" object containing >> pointers to the devices in question, a list node for each of them and some >> additional information related to the management of those objects, ie. >> something like: >> >> struct device_link { >> struct device *supplier; >> struct list_head supplier_node; >> struct device *consumer; >> struct list_head consumer_node; >> <flags, status etc> >> }; >> >> In general, there will be two lists of those things per device, one list >> of links to consumers and one list of links to suppliers. >> >> In that picture, links will be created by calling, say: >> >> int device_add_link(struct device *me, struct device *my_supplier, unsigned int flags); > > At first glance, I like this, nice. Now to see how well it can be > implemented :) Hi Greg, what's your opinion on using this to order device probes so we don't try to probe a device that we know it has unfulfilled dependencies? Regards, Tomeu ^ permalink raw reply [flat|nested] 55+ messages in thread
* Re: [RFD] Functional dependencies between devices 2016-01-07 14:55 ` Tomeu Vizoso @ 2016-01-07 21:29 ` Greg Kroah-Hartman 2016-01-08 7:28 ` Tomeu Vizoso 0 siblings, 1 reply; 55+ messages in thread From: Greg Kroah-Hartman @ 2016-01-07 21:29 UTC (permalink / raw) To: Tomeu Vizoso Cc: Rafael J. Wysocki, Linux PM list, Linux Kernel Mailing List, Alan Stern, Grant Likely, Mark Brown, Rob Herring, Thierry Reding, Dmitry Torokhov, Geert Uytterhoeven, Michael Turquette On Thu, Jan 07, 2016 at 03:55:43PM +0100, Tomeu Vizoso wrote: > On 30 October 2015 at 23:52, Greg Kroah-Hartman > <gregkh@linuxfoundation.org> wrote: > > On Tue, Oct 27, 2015 at 04:24:14PM +0100, Rafael J. Wysocki wrote: > >> My idea is to represent a supplier-consumer dependency between devices (or > >> more precisely between device+driver combos) as a "link" object containing > >> pointers to the devices in question, a list node for each of them and some > >> additional information related to the management of those objects, ie. > >> something like: > >> > >> struct device_link { > >> struct device *supplier; > >> struct list_head supplier_node; > >> struct device *consumer; > >> struct list_head consumer_node; > >> <flags, status etc> > >> }; > >> > >> In general, there will be two lists of those things per device, one list > >> of links to consumers and one list of links to suppliers. > >> > >> In that picture, links will be created by calling, say: > >> > >> int device_add_link(struct device *me, struct device *my_supplier, unsigned int flags); > > > > At first glance, I like this, nice. Now to see how well it can be > > implemented :) > > Hi Greg, > > what's your opinion on using this to order device probes so we don't > try to probe a device that we know it has unfulfilled dependencies? Why would that matter, unless you can prove it's faster, I wouldn't bother. greg k-h ^ permalink raw reply [flat|nested] 55+ messages in thread
* Re: [RFD] Functional dependencies between devices 2016-01-07 21:29 ` Greg Kroah-Hartman @ 2016-01-08 7:28 ` Tomeu Vizoso 2016-01-08 15:15 ` Greg Kroah-Hartman 0 siblings, 1 reply; 55+ messages in thread From: Tomeu Vizoso @ 2016-01-08 7:28 UTC (permalink / raw) To: Greg Kroah-Hartman Cc: Rafael J. Wysocki, Linux PM list, Linux Kernel Mailing List, Alan Stern, Grant Likely, Mark Brown, Rob Herring, Thierry Reding, Dmitry Torokhov, Geert Uytterhoeven, Michael Turquette On 7 January 2016 at 22:29, Greg Kroah-Hartman <gregkh@linuxfoundation.org> wrote: > On Thu, Jan 07, 2016 at 03:55:43PM +0100, Tomeu Vizoso wrote: >> On 30 October 2015 at 23:52, Greg Kroah-Hartman >> <gregkh@linuxfoundation.org> wrote: >> > On Tue, Oct 27, 2015 at 04:24:14PM +0100, Rafael J. Wysocki wrote: >> >> My idea is to represent a supplier-consumer dependency between devices (or >> >> more precisely between device+driver combos) as a "link" object containing >> >> pointers to the devices in question, a list node for each of them and some >> >> additional information related to the management of those objects, ie. >> >> something like: >> >> >> >> struct device_link { >> >> struct device *supplier; >> >> struct list_head supplier_node; >> >> struct device *consumer; >> >> struct list_head consumer_node; >> >> <flags, status etc> >> >> }; >> >> >> >> In general, there will be two lists of those things per device, one list >> >> of links to consumers and one list of links to suppliers. >> >> >> >> In that picture, links will be created by calling, say: >> >> >> >> int device_add_link(struct device *me, struct device *my_supplier, unsigned int flags); >> > >> > At first glance, I like this, nice. Now to see how well it can be >> > implemented :) >> >> Hi Greg, >> >> what's your opinion on using this to order device probes so we don't >> try to probe a device that we know it has unfulfilled dependencies? > > Why would that matter, unless you can prove it's faster, I wouldn't > bother. I gave you the bootlog you asked in the post below, could you please comment there? https://lkml.kernel.org/g/562A280A.3040002@collabora.com Thanks, Tomeu ^ permalink raw reply [flat|nested] 55+ messages in thread
* Re: [RFD] Functional dependencies between devices 2016-01-08 7:28 ` Tomeu Vizoso @ 2016-01-08 15:15 ` Greg Kroah-Hartman 0 siblings, 0 replies; 55+ messages in thread From: Greg Kroah-Hartman @ 2016-01-08 15:15 UTC (permalink / raw) To: Tomeu Vizoso Cc: Rafael J. Wysocki, Linux PM list, Linux Kernel Mailing List, Alan Stern, Grant Likely, Mark Brown, Rob Herring, Thierry Reding, Dmitry Torokhov, Geert Uytterhoeven, Michael Turquette On Fri, Jan 08, 2016 at 08:28:15AM +0100, Tomeu Vizoso wrote: > On 7 January 2016 at 22:29, Greg Kroah-Hartman > <gregkh@linuxfoundation.org> wrote: > > On Thu, Jan 07, 2016 at 03:55:43PM +0100, Tomeu Vizoso wrote: > >> On 30 October 2015 at 23:52, Greg Kroah-Hartman > >> <gregkh@linuxfoundation.org> wrote: > >> > On Tue, Oct 27, 2015 at 04:24:14PM +0100, Rafael J. Wysocki wrote: > >> >> My idea is to represent a supplier-consumer dependency between devices (or > >> >> more precisely between device+driver combos) as a "link" object containing > >> >> pointers to the devices in question, a list node for each of them and some > >> >> additional information related to the management of those objects, ie. > >> >> something like: > >> >> > >> >> struct device_link { > >> >> struct device *supplier; > >> >> struct list_head supplier_node; > >> >> struct device *consumer; > >> >> struct list_head consumer_node; > >> >> <flags, status etc> > >> >> }; > >> >> > >> >> In general, there will be two lists of those things per device, one list > >> >> of links to consumers and one list of links to suppliers. > >> >> > >> >> In that picture, links will be created by calling, say: > >> >> > >> >> int device_add_link(struct device *me, struct device *my_supplier, unsigned int flags); > >> > > >> > At first glance, I like this, nice. Now to see how well it can be > >> > implemented :) > >> > >> Hi Greg, > >> > >> what's your opinion on using this to order device probes so we don't > >> try to probe a device that we know it has unfulfilled dependencies? > > > > Why would that matter, unless you can prove it's faster, I wouldn't > > bother. > > I gave you the bootlog you asked in the post below, could you please > comment there? > > https://lkml.kernel.org/g/562A280A.3040002@collabora.com that made no sense at all... ^ permalink raw reply [flat|nested] 55+ messages in thread
* Re: [RFD] Functional dependencies between devices 2015-10-27 15:24 [RFD] Functional dependencies between devices Rafael J. Wysocki ` (3 preceding siblings ...) 2015-10-30 22:52 ` Greg Kroah-Hartman @ 2015-11-09 12:32 ` Thierry Reding 2015-11-09 21:42 ` Rafael J. Wysocki 2015-11-17 12:44 ` Andrzej Hajda ` (2 subsequent siblings) 7 siblings, 1 reply; 55+ messages in thread From: Thierry Reding @ 2015-11-09 12:32 UTC (permalink / raw) To: Rafael J. Wysocki Cc: Linux PM list, Greg Kroah-Hartman, Linux Kernel Mailing List, Alan Stern, Grant Likely, Mark Brown, Rob Herring, Tomeu Vizoso, Dmitry Torokhov, Geert Uytterhoeven, Michael Turquette [-- Attachment #1: Type: text/plain, Size: 2367 bytes --] On Tue, Oct 27, 2015 at 04:24:14PM +0100, Rafael J. Wysocki wrote: [...] > There's a question about what if the supplier device is being unbound before > the consumer one (for example, as a result of a hotplug event). My current > view on that is that the consumer needs to be force-unbound in that case too, > but I guess I may be persuaded otherwise given sufficiently convincing > arguments. I think this would be a huge step towards making the kernel more robust with little driver or subsystem code having to be duplicated. Currently most provider/consumer subsystems are fragile in that there isn't proper reference counting. Many subsystems will happily allow you to remove any of the provider, regardless of whether or not it has consumers. Most of the subsystems will make sure that modules can't be unloaded, but beyond that won't be able to prevent drivers from being unbound (either when a device is unplugged or unbound via sysfs). Even with proper reference counting there is no easy way to deal with devices going away (you'd need some sort of revoke semantics implemented for all providers, and consumers must be able to handle that situation gracefully). Implementing a force-unbind policy would make this a whole lot easier. Dangling resources will automatically become a thing of the past. The downside of course is that force-unbinding consumers may not always be the most user-friendly course of action. Consider an SD/MMC slot that uses a GPIO as card-detect pin. Unbinding the provider of the GPIO would cause the SD/ MMC controller to be unbound, hence unmounting the filesystem that it provided. That filesystem might have been the root filesystem. We discussed similar use-cases a while back and you proposed making the force-unbind policy be two-staged: reject unbind (-EBUSY) if there are any consumers, and force-unbind consumers if the provider was forcibly unbound (or caused by hot-unplug of the backing device). That sounds like a good compromise to me. That said I can also imagine subsystems where a reliable mechanism is in place to properly hotplug and -unplug providers. The good thing about the functional dependencies mechanism you propose here is that it's an optional mechanism that drivers use from ->probe(). Subsystems where a better mechanism exists can simply choose to do without functional dependencies. Thierry [-- Attachment #2: signature.asc --] [-- Type: application/pgp-signature, Size: 819 bytes --] ^ permalink raw reply [flat|nested] 55+ messages in thread
* Re: [RFD] Functional dependencies between devices 2015-11-09 12:32 ` Thierry Reding @ 2015-11-09 21:42 ` Rafael J. Wysocki 0 siblings, 0 replies; 55+ messages in thread From: Rafael J. Wysocki @ 2015-11-09 21:42 UTC (permalink / raw) To: Thierry Reding Cc: Linux PM list, Greg Kroah-Hartman, Linux Kernel Mailing List, Alan Stern, Grant Likely, Mark Brown, Rob Herring, Tomeu Vizoso, Dmitry Torokhov, Geert Uytterhoeven, Michael Turquette [-- Attachment #1: Type: text/plain, Size: 3314 bytes --] On Monday, November 09, 2015 01:32:04 PM Thierry Reding wrote: > On Tue, Oct 27, 2015 at 04:24:14PM +0100, Rafael J. Wysocki wrote: > [...] > > There's a question about what if the supplier device is being unbound before > > the consumer one (for example, as a result of a hotplug event). My current > > view on that is that the consumer needs to be force-unbound in that case too, > > but I guess I may be persuaded otherwise given sufficiently convincing > > arguments. > > I think this would be a huge step towards making the kernel more robust > with little driver or subsystem code having to be duplicated. Currently > most provider/consumer subsystems are fragile in that there isn't proper > reference counting. Many subsystems will happily allow you to remove any > of the provider, regardless of whether or not it has consumers. Most of > the subsystems will make sure that modules can't be unloaded, but beyond > that won't be able to prevent drivers from being unbound (either when a > device is unplugged or unbound via sysfs). Even with proper reference > counting there is no easy way to deal with devices going away (you'd > need some sort of revoke semantics implemented for all providers, and > consumers must be able to handle that situation gracefully). > > Implementing a force-unbind policy would make this a whole lot easier. > Dangling resources will automatically become a thing of the past. The > downside of course is that force-unbinding consumers may not always be > the most user-friendly course of action. Consider an SD/MMC slot that > uses a GPIO as card-detect pin. Unbinding the provider of the GPIO > would cause the SD/ MMC controller to be unbound, hence unmounting the > filesystem that it provided. That filesystem might have been the root > filesystem. Well, the problem is that device_release_driver() cannot fail, so it pretty much has to unbind everything that is not going to work after the driver is unbound from the device. > We discussed similar use-cases a while back and you proposed making the > force-unbind policy be two-staged: reject unbind (-EBUSY) if there are > any consumers, and force-unbind consumers if the provider was forcibly > unbound (or caused by hot-unplug of the backing device). That sounds > like a good compromise to me. That can be done for bus types having device_offline/online() support, but the number of these is quite limited at this point. The "offline" operation, as opposed to device_release_driver(), can return an error code to indicate that the device cannot be taken offline at this time. So, if offlining a supplier would require offlining all consumers of it, that may be made fail in certain situation. However, that would require quite a bit of additional structure (and complexity) in pretty much all bus types, so I wouldn't start with it at least. > That said I can also imagine subsystems where a reliable mechanism is in > place to properly hotplug and -unplug providers. The good thing about > the functional dependencies mechanism you propose here is that it's an > optional mechanism that drivers use from ->probe(). Subsystems where a > better mechanism exists can simply choose to do without functional > dependencies. I actually think that those things are at least partly orthogonal. Thanks, Rafael [-- Attachment #2: This is a digitally signed message part. --] [-- Type: application/pgp-signature, Size: 836 bytes --] ^ permalink raw reply [flat|nested] 55+ messages in thread
* Re: [RFD] Functional dependencies between devices 2015-10-27 15:24 [RFD] Functional dependencies between devices Rafael J. Wysocki ` (4 preceding siblings ...) 2015-11-09 12:32 ` Thierry Reding @ 2015-11-17 12:44 ` Andrzej Hajda 2015-11-18 2:17 ` Rafael J. Wysocki 2015-11-17 12:49 ` Andrzej Hajda 2016-01-14 1:52 ` [RFC][PATCH 0/5] " Rafael J. Wysocki 7 siblings, 1 reply; 55+ messages in thread From: Andrzej Hajda @ 2015-11-17 12:44 UTC (permalink / raw) To: Rafael J. Wysocki, Linux PM list Cc: Greg Kroah-Hartman, Linux Kernel Mailing List, Alan Stern, Grant Likely, Mark Brown, Rob Herring, Tomeu Vizoso, Thierry Reding, Dmitry Torokhov, Geert Uytterhoeven, Michael Turquette Hi Rafael, Please forgive me late reply, but I have missed this thread before. On 10/27/2015 04:24 PM, Rafael J. Wysocki wrote: > Hi All, > > As discussed in the recent "On-demand device probing" thread and in a Kernel > Summit session earlier today, 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). I think the real dependency is when some entity asks for some resource (irq, clock, gpio,...). Usually the entity is some device driver during probing and the resource is provided by some bound device but there are many exceptions for this scenario: - many clock providers, irq domains are not provided by devices, - there are also dependencies between clock providers, ie. some clock provider requires clocks provided by another clock provider, so the entity is also not a device driver, - there are resources which can be requested after probe - case of componentized devices (DRM for example), more precisely they can be requested during probe of random component or master of componentized device, - another case are requests for some additional/optional resources after device driver probe, for example phone usually does not require HDMI related resources until user attach HDMI cable, - (semi-)circular dependencies - 1st device provides clock used by other devices which provides other resources used by the 1st device, scenario present in some video pipelines, like camera subsystem + sensors. These examples shows that dependencies between bound device drivers are just subset of bigger issue, maybe it is worth to look for more general solution. > > So I want to be able to represent those functional dependencies between devices > and I'd like 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). Could you elaborate these distinct use cases. I am curious because I have proposed resource tracking framework [1] which should solve most of the issues described here. It was not designed to solve suspend/resume issues, but it could be easily extended to support it, I suppose. [1]: https://lkml.org/lkml/2014/12/10/342 > > My idea is to represent a supplier-consumer dependency between devices (or > more precisely between device+driver combos) as a "link" object containing > pointers to the devices in question, a list node for each of them and some > additional information related to the management of those objects, ie. > something like: > > struct device_link { > struct device *supplier; > struct list_head supplier_node; > struct device *consumer; > struct list_head consumer_node; > <flags, status etc> > }; > > In general, there will be two lists of those things per device, one list > of links to consumers and one list of links to suppliers. > > In that picture, links will be created by calling, say: > > int device_add_link(struct device *me, struct device *my_supplier, unsigned int flags); > > and they will be deleted by the driver core when not needed any more. The > creation of a link should also cause dpm_list and the list used during shutdown > to be reordered if needed. > > In principle, it seems usefult to consider two types of links, one created > at device registration time (when registering the second device from the linked > pair, whichever it is) and one created at probe time (of the consumer device). > I'll refer to them as "permanent" and "probe-time" links, respectively. > > The permanent links (created at device registration time) will stay around > until one of the linked devices is unregistered (at which time the driver > core will drop the link along with the device going away). The probe-time > ones will be dropped (automatically) at the consumer device driver unbind time. What about permanent links in case provider is unregistered? Should they disappear? It will not make consumers happy. What if the provider will be re-registered. > > There's a question about what if the supplier device is being unbound before > the consumer one (for example, as a result of a hotplug event). My current > view on that is that the consumer needs to be force-unbound in that case too, > but I guess I may be persuaded otherwise given sufficiently convincing > arguments. Some devices can have 'weak' dependencies - they will be still functional without some resources. In fact two last examples from my 1st paragraph are counter-examples for this. I suspect there should be some kind of notification for them about removal of the resource. > Anyway, there are reasons to do that, like for example it may > help with the synchronization. Namely, if there's a rule that suppliers > cannot be unbound before any consumers linked to them, than the list of links > to suppliers for a consumer can only change at its registration/probe or > unbind/remove times (which simplifies things quite a bit). > > With that, the permanent links existing at the probe time for a consumer > device can be used to check whether or not to defer the probing of it > even before executing its probe callback. In turn, system suspend > synchronization should be a matter of calling device_pm_wait_for_dev() > for all consumers of a supplier device, in analogy with dpm_wait_for_children(), > and so on. > > Of course, the new lists have to be stable during those operations and ensuring > that is going to be somewhat tricky (AFAICS right now at least), but apart from > that the whole concept looks reasonably straightforward to me. > > So, the question to everybody is whether or not this sounds reasonable or there > are concerns about it and if so what they are. At this point I mostly need to > know if I'm not overlooking anything fundamental at the general level. Regarding fundamental things, maybe it is just my impression but parsing private DT device nodes by kernel core assumes that convention about using resource specifiers in DT is a strict rule, it should not be true. As I wrote before I have send some early RFC with framework which solves most of the problems described here[1], the missing part is suspend/resume support which should be quite easy to add, I suspect. Moreover it solves problem of device driver hot bind/unbind. Could you take a look at it, I will be glad to know it is worth to continue work on it? [1]: https://lkml.org/lkml/2014/12/10/342 Regards Andrzej > > Thanks, > Rafael > > -- > To unsubscribe from this list: send the line "unsubscribe linux-pm" in > the body of a message to majordomo@vger.kernel.org > More majordomo info at http://vger.kernel.org/majordomo-info.html > ^ permalink raw reply [flat|nested] 55+ messages in thread
* Re: [RFD] Functional dependencies between devices 2015-11-17 12:44 ` Andrzej Hajda @ 2015-11-18 2:17 ` Rafael J. Wysocki 2015-11-19 9:08 ` Andrzej Hajda 0 siblings, 1 reply; 55+ messages in thread From: Rafael J. Wysocki @ 2015-11-18 2:17 UTC (permalink / raw) To: Andrzej Hajda Cc: Linux PM list, Greg Kroah-Hartman, Linux Kernel Mailing List, Alan Stern, Grant Likely, Mark Brown, Rob Herring, Tomeu Vizoso, Thierry Reding, Dmitry Torokhov, Geert Uytterhoeven, Michael Turquette On Tuesday, November 17, 2015 01:44:59 PM Andrzej Hajda wrote: > Hi Rafael, > > Please forgive me late reply, but I have missed this thread before. > > On 10/27/2015 04:24 PM, Rafael J. Wysocki wrote: > > Hi All, > > > > As discussed in the recent "On-demand device probing" thread and in a Kernel > > Summit session earlier today, 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). > > I think the real dependency is when some entity asks for some resource (irq, > clock, gpio,...). Well, a dependency is when one entity uses a resource provided by another one, not only when it explicitly asks for that resource. But that's a very high level of abstraction IMO. > Usually the entity is some device driver during probing and > the resource is provided by some bound device but there are many exceptions for > this scenario: > - many clock providers, irq domains are not provided by devices, > - there are also dependencies between clock providers, ie. some clock provider > requires clocks provided by another clock provider, so the entity is also not a > device driver, > - there are resources which can be requested after probe - case of componentized > devices (DRM for example), more precisely they can be requested during probe of > random component or master of componentized device, > - another case are requests for some additional/optional resources after device > driver probe, for example phone usually does not require HDMI related resources > until user attach HDMI cable, > - (semi-)circular dependencies - 1st device provides clock used by other devices > which provides other resources used by the 1st device, scenario present in some > video pipelines, like camera subsystem + sensors. > > These examples shows that dependencies between bound device drivers are just > subset of bigger issue, maybe it is worth to look for more general solution. That really depends on the goal. The goal here is to add a mechanism allowing the driver core to carry out certain operations in the right order. The operations in question are carried out on devices using drivers (and perhaps bus types, PM domains etc), so using a representation of links between devices seems adequate to me. > > So I want to be able to represent those functional dependencies between devices > > and I'd like 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). > > Could you elaborate these distinct use cases. I am curious because I have > proposed resource tracking framework [1] which should solve most of the issues > described here. It was not designed to solve suspend/resume issues, but it could > be easily extended to support it, I suppose. > > [1]: https://lkml.org/lkml/2014/12/10/342 So the operations that need to be taken care of are: - Probe (suppliers need to be probed before consumers if the dependencies are known beforehand). - System suspend/resume (suppliers need to be suspended after consumers and resumed before them) which may be asynchronous (so simple re-ordering doesn't help). - Runtime PM (suppliers should not be suspended if the consumers are not suspended). - System shutdown (shutdown callbacks should be executed for consumers first). - Driver unbind (a supplier driver cannot be unbound before any of its consumer drivers). In principle you can use resource tracking to figure out all of the involved dependencies, but that would require walking complicated data structures unless you add an intermediate "device dependency" layer which is going to be analogous to the one discussed here. > > My idea is to represent a supplier-consumer dependency between devices (or > > more precisely between device+driver combos) as a "link" object containing > > pointers to the devices in question, a list node for each of them and some > > additional information related to the management of those objects, ie. > > something like: > > > > struct device_link { > > struct device *supplier; > > struct list_head supplier_node; > > struct device *consumer; > > struct list_head consumer_node; > > <flags, status etc> > > }; > > > > In general, there will be two lists of those things per device, one list > > of links to consumers and one list of links to suppliers. > > > > In that picture, links will be created by calling, say: > > > > int device_add_link(struct device *me, struct device *my_supplier, unsigned int flags); > > > > and they will be deleted by the driver core when not needed any more. The > > creation of a link should also cause dpm_list and the list used during shutdown > > to be reordered if needed. > > > > In principle, it seems usefult to consider two types of links, one created > > at device registration time (when registering the second device from the linked > > pair, whichever it is) and one created at probe time (of the consumer device). > > I'll refer to them as "permanent" and "probe-time" links, respectively. > > > > The permanent links (created at device registration time) will stay around > > until one of the linked devices is unregistered (at which time the driver > > core will drop the link along with the device going away). The probe-time > > ones will be dropped (automatically) at the consumer device driver unbind time. > > What about permanent links in case provider is unregistered? Should they > disappear? It will not make consumers happy. What if the provider will be > re-registered. If the device object is gone, it cannot be pointed to by any links (on any end) any more. That's just physically impossible. :-) > > There's a question about what if the supplier device is being unbound before > > the consumer one (for example, as a result of a hotplug event). My current > > view on that is that the consumer needs to be force-unbound in that case too, > > but I guess I may be persuaded otherwise given sufficiently convincing > > arguments. > > Some devices can have 'weak' dependencies - they will be still functional > without some resources. Right. That's on my radar. > In fact two last examples from my 1st paragraph are > counter-examples for this. I suspect there should be some kind of notification > for them about removal of the resource. > > > Anyway, there are reasons to do that, like for example it may > > help with the synchronization. Namely, if there's a rule that suppliers > > cannot be unbound before any consumers linked to them, than the list of links > > to suppliers for a consumer can only change at its registration/probe or > > unbind/remove times (which simplifies things quite a bit). > > > > With that, the permanent links existing at the probe time for a consumer > > device can be used to check whether or not to defer the probing of it > > even before executing its probe callback. In turn, system suspend > > synchronization should be a matter of calling device_pm_wait_for_dev() > > for all consumers of a supplier device, in analogy with dpm_wait_for_children(), > > and so on. > > > > Of course, the new lists have to be stable during those operations and ensuring > > that is going to be somewhat tricky (AFAICS right now at least), but apart from > > that the whole concept looks reasonably straightforward to me. > > > > So, the question to everybody is whether or not this sounds reasonable or there > > are concerns about it and if so what they are. At this point I mostly need to > > know if I'm not overlooking anything fundamental at the general level. > > Regarding fundamental things, maybe it is just my impression but parsing private > DT device nodes by kernel core assumes that convention about using resource > specifiers in DT is a strict rule, it should not be true. I really am not sure what you mean here, sorry. > As I wrote before I have send some early RFC with framework which solves most of > the problems described here[1], the missing part is suspend/resume support which > should be quite easy to add, I suspect. Moreover it solves problem of device > driver hot bind/unbind. > Could you take a look at it, I will be glad to know it is worth to continue work > on it? > > [1]: https://lkml.org/lkml/2014/12/10/342 I'm not sure to be honest. I'm not a big fan of notification-based mechanisms in general, because they depend on everyone registering those notifiers to implement them correctly and it gets additionally complicated if the ordering matters etc. So I personally wouldn't take that route. I guess some way of resource tracking will be necessary at one point, but what shape it should take is a good question. Thanks, Rafael ^ permalink raw reply [flat|nested] 55+ messages in thread
* Re: [RFD] Functional dependencies between devices 2015-11-18 2:17 ` Rafael J. Wysocki @ 2015-11-19 9:08 ` Andrzej Hajda 2015-11-19 22:04 ` Rafael J. Wysocki 0 siblings, 1 reply; 55+ messages in thread From: Andrzej Hajda @ 2015-11-19 9:08 UTC (permalink / raw) To: Rafael J. Wysocki Cc: Linux PM list, Greg Kroah-Hartman, Linux Kernel Mailing List, Alan Stern, Grant Likely, Mark Brown, Rob Herring, Tomeu Vizoso, Thierry Reding, Dmitry Torokhov, Geert Uytterhoeven, Michael Turquette On 11/18/2015 03:17 AM, Rafael J. Wysocki wrote: > On Tuesday, November 17, 2015 01:44:59 PM Andrzej Hajda wrote: >> Hi Rafael, >> >> Please forgive me late reply, but I have missed this thread before. >> >> On 10/27/2015 04:24 PM, Rafael J. Wysocki wrote: >>> Hi All, >>> >>> As discussed in the recent "On-demand device probing" thread and in a Kernel >>> Summit session earlier today, 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). >> I think the real dependency is when some entity asks for some resource (irq, >> clock, gpio,...). > Well, a dependency is when one entity uses a resource provided by another one, > not only when it explicitly asks for that resource. But that's a very high > level of abstraction IMO. > >> Usually the entity is some device driver during probing and >> the resource is provided by some bound device but there are many exceptions for >> this scenario: >> - many clock providers, irq domains are not provided by devices, >> - there are also dependencies between clock providers, ie. some clock provider >> requires clocks provided by another clock provider, so the entity is also not a >> device driver, >> - there are resources which can be requested after probe - case of componentized >> devices (DRM for example), more precisely they can be requested during probe of >> random component or master of componentized device, >> - another case are requests for some additional/optional resources after device >> driver probe, for example phone usually does not require HDMI related resources >> until user attach HDMI cable, >> - (semi-)circular dependencies - 1st device provides clock used by other devices >> which provides other resources used by the 1st device, scenario present in some >> video pipelines, like camera subsystem + sensors. >> >> These examples shows that dependencies between bound device drivers are just >> subset of bigger issue, maybe it is worth to look for more general solution. > That really depends on the goal. > > The goal here is to add a mechanism allowing the driver core to carry out > certain operations in the right order. The operations in question are carried > out on devices using drivers (and perhaps bus types, PM domains etc), so using > a representation of links between devices seems adequate to me. > >>> So I want to be able to represent those functional dependencies between devices >>> and I'd like 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). >> Could you elaborate these distinct use cases. I am curious because I have >> proposed resource tracking framework [1] which should solve most of the issues >> described here. It was not designed to solve suspend/resume issues, but it could >> be easily extended to support it, I suppose. >> >> [1]: https://lkml.org/lkml/2014/12/10/342 > So the operations that need to be taken care of are: > - Probe (suppliers need to be probed before consumers if the dependencies are > known beforehand). > - System suspend/resume (suppliers need to be suspended after consumers and > resumed before them) which may be asynchronous (so simple re-ordering doesn't > help). > - Runtime PM (suppliers should not be suspended if the consumers are not > suspended). I though provider's frameworks are taking care of it already. For example clock provider cannot suspend until there are prepared/enabled clocks. Similar enabled regulators, phys should block provider from runtime pm suspending. Are there situations/frameworks which requires additional care? > - System shutdown (shutdown callbacks should be executed for consumers first). > - Driver unbind (a supplier driver cannot be unbound before any of its consumer > drivers). > > In principle you can use resource tracking to figure out all of the involved > dependencies, but that would require walking complicated data structures unless > you add an intermediate "device dependency" layer which is going to be analogous > to the one discussed here. It should be enough if provider notifies consumers that the resource will be unavailable. > >>> My idea is to represent a supplier-consumer dependency between devices (or >>> more precisely between device+driver combos) as a "link" object containing >>> pointers to the devices in question, a list node for each of them and some >>> additional information related to the management of those objects, ie. >>> something like: >>> >>> struct device_link { >>> struct device *supplier; >>> struct list_head supplier_node; >>> struct device *consumer; >>> struct list_head consumer_node; >>> <flags, status etc> >>> }; >>> >>> In general, there will be two lists of those things per device, one list >>> of links to consumers and one list of links to suppliers. >>> >>> In that picture, links will be created by calling, say: >>> >>> int device_add_link(struct device *me, struct device *my_supplier, unsigned int flags); >>> >>> and they will be deleted by the driver core when not needed any more. The >>> creation of a link should also cause dpm_list and the list used during shutdown >>> to be reordered if needed. >>> >>> In principle, it seems usefult to consider two types of links, one created >>> at device registration time (when registering the second device from the linked >>> pair, whichever it is) and one created at probe time (of the consumer device). >>> I'll refer to them as "permanent" and "probe-time" links, respectively. >>> >>> The permanent links (created at device registration time) will stay around >>> until one of the linked devices is unregistered (at which time the driver >>> core will drop the link along with the device going away). The probe-time >>> ones will be dropped (automatically) at the consumer device driver unbind time. >> What about permanent links in case provider is unregistered? Should they >> disappear? It will not make consumers happy. What if the provider will be >> re-registered. > If the device object is gone, it cannot be pointed to by any links (on any end) > any more. That's just physically impossible. :-) So the link will disappear and the 'consumer' will have dependencies fulfilled. It will be then probed? Is it OK? Or am I missing something? > >>> There's a question about what if the supplier device is being unbound before >>> the consumer one (for example, as a result of a hotplug event). My current >>> view on that is that the consumer needs to be force-unbound in that case too, >>> but I guess I may be persuaded otherwise given sufficiently convincing >>> arguments. >> Some devices can have 'weak' dependencies - they will be still functional >> without some resources. > Right. That's on my radar. > >> In fact two last examples from my 1st paragraph are >> counter-examples for this. I suspect there should be some kind of notification >> for them about removal of the resource. >> >>> Anyway, there are reasons to do that, like for example it may >>> help with the synchronization. Namely, if there's a rule that suppliers >>> cannot be unbound before any consumers linked to them, than the list of links >>> to suppliers for a consumer can only change at its registration/probe or >>> unbind/remove times (which simplifies things quite a bit). >>> >>> With that, the permanent links existing at the probe time for a consumer >>> device can be used to check whether or not to defer the probing of it >>> even before executing its probe callback. In turn, system suspend >>> synchronization should be a matter of calling device_pm_wait_for_dev() >>> for all consumers of a supplier device, in analogy with dpm_wait_for_children(), >>> and so on. >>> >>> Of course, the new lists have to be stable during those operations and ensuring >>> that is going to be somewhat tricky (AFAICS right now at least), but apart from >>> that the whole concept looks reasonably straightforward to me. >>> >>> So, the question to everybody is whether or not this sounds reasonable or there >>> are concerns about it and if so what they are. At this point I mostly need to >>> know if I'm not overlooking anything fundamental at the general level. >> Regarding fundamental things, maybe it is just my impression but parsing private >> DT device nodes by kernel core assumes that convention about using resource >> specifiers in DT is a strict rule, it should not be true. > I really am not sure what you mean here, sorry. Device tree bindings are defined per device so theoretically only device driver should parse them(except few basic properties). This is of course only my impression, but even in this thread Mark made similar statement [1]. Assuming this, permanent links should not be used with device tree, as a result deferred probing will be still a problem. [1]: http://permalink.gmane.org/gmane.linux.power-management.general/67593 > >> As I wrote before I have send some early RFC with framework which solves most of >> the problems described here[1], the missing part is suspend/resume support which >> should be quite easy to add, I suspect. Moreover it solves problem of device >> driver hot bind/unbind. >> Could you take a look at it, I will be glad to know it is worth to continue work >> on it? >> >> [1]: https://lkml.org/lkml/2014/12/10/342 > I'm not sure to be honest. > > I'm not a big fan of notification-based mechanisms in general, because they > depend on everyone registering those notifiers to implement them correctly and > it gets additionally complicated if the ordering matters etc. So I personally > wouldn't take that route. > > I guess some way of resource tracking will be necessary at one point, but what > shape it should take is a good question. Any callback provided by a driver including probe/remove are in fact notification mechanisms :) And they should be also correctly implemented. Ordering in case of resource tracking is enforced by the framework, so I do not see complication here. Anyway if we take two assumptions which are already true: - device bound to driver can provide resources, - device driver can be unloaded/unbound at any time. Then notifications/callbacks seems to me the only solution. Regards Andrzej > > Thanks, > Rafael > > > ^ permalink raw reply [flat|nested] 55+ messages in thread
* Re: [RFD] Functional dependencies between devices 2015-11-19 9:08 ` Andrzej Hajda @ 2015-11-19 22:04 ` Rafael J. Wysocki 2015-11-20 1:11 ` Rafael J. Wysocki 2015-11-24 14:57 ` Andrzej Hajda 0 siblings, 2 replies; 55+ messages in thread From: Rafael J. Wysocki @ 2015-11-19 22:04 UTC (permalink / raw) To: Andrzej Hajda Cc: Linux PM list, Greg Kroah-Hartman, Linux Kernel Mailing List, Alan Stern, Grant Likely, Mark Brown, Rob Herring, Tomeu Vizoso, Thierry Reding, Dmitry Torokhov, Geert Uytterhoeven, Michael Turquette On Thursday, November 19, 2015 10:08:43 AM Andrzej Hajda wrote: > On 11/18/2015 03:17 AM, Rafael J. Wysocki wrote: > > On Tuesday, November 17, 2015 01:44:59 PM Andrzej Hajda wrote: > >> Hi Rafael, > >> [cut] > > So the operations that need to be taken care of are: > > - Probe (suppliers need to be probed before consumers if the dependencies are > > known beforehand). > > - System suspend/resume (suppliers need to be suspended after consumers and > > resumed before them) which may be asynchronous (so simple re-ordering doesn't > > help). > > - Runtime PM (suppliers should not be suspended if the consumers are not > > suspended). > I though provider's frameworks are taking care of it already. For example > clock provider cannot suspend until there are prepared/enabled clocks. > Similar enabled regulators, phys should block provider from runtime pm > suspending. > > Are there situations/frameworks which requires additional care? Yes, there are, AFAICS. A somewhat extreme example of this is when an AML routine needed for power management of one device uses something like a GPIO line or an I2C link provided by another one. We don't even have a way to track that kind of thing at the provider framework level and the only information we can get from the platform firmware is "this device depends on that one". Plus, even if the frameworks track those things, when a device suspend is requested, the question really is "Are there any devices that have to be suspended before this one?" rather than "Are other devices using resources provided by this one?". Of course, you may argue that answering the second one will allow you to answer the first one too (that is based on the assumption that you can always track all cases of resource utilization which may not be entirely realistic), but getting that answer in a non-racy way may be rather expensive. > > - System shutdown (shutdown callbacks should be executed for consumers first). > > - Driver unbind (a supplier driver cannot be unbound before any of its consumer > > drivers). > > > > In principle you can use resource tracking to figure out all of the involved > > dependencies, but that would require walking complicated data structures unless > > you add an intermediate "device dependency" layer which is going to be analogous > > to the one discussed here. > > It should be enough if provider notifies consumers that the resource > will be unavailable. To me, this isn't going in the right direction. You should be asking "Am I allowed to suspend now?" instead of saying "I'm suspending and now you deal with it" to somebody. Why is that so? Because the other end may simply be unable to deal with the situation in the first place. > > > >>> My idea is to represent a supplier-consumer dependency between devices (or > >>> more precisely between device+driver combos) as a "link" object containing > >>> pointers to the devices in question, a list node for each of them and some > >>> additional information related to the management of those objects, ie. > >>> something like: > >>> > >>> struct device_link { > >>> struct device *supplier; > >>> struct list_head supplier_node; > >>> struct device *consumer; > >>> struct list_head consumer_node; > >>> <flags, status etc> > >>> }; > >>> > >>> In general, there will be two lists of those things per device, one list > >>> of links to consumers and one list of links to suppliers. > >>> > >>> In that picture, links will be created by calling, say: > >>> > >>> int device_add_link(struct device *me, struct device *my_supplier, unsigned int flags); > >>> > >>> and they will be deleted by the driver core when not needed any more. The > >>> creation of a link should also cause dpm_list and the list used during shutdown > >>> to be reordered if needed. > >>> > >>> In principle, it seems usefult to consider two types of links, one created > >>> at device registration time (when registering the second device from the linked > >>> pair, whichever it is) and one created at probe time (of the consumer device). > >>> I'll refer to them as "permanent" and "probe-time" links, respectively. > >>> > >>> The permanent links (created at device registration time) will stay around > >>> until one of the linked devices is unregistered (at which time the driver > >>> core will drop the link along with the device going away). The probe-time > >>> ones will be dropped (automatically) at the consumer device driver unbind time. > >> What about permanent links in case provider is unregistered? Should they > >> disappear? It will not make consumers happy. What if the provider will be > >> re-registered. > > If the device object is gone, it cannot be pointed to by any links (on any end) > > any more. That's just physically impossible. :-) > > So the link will disappear and the 'consumer' will have dependencies > fulfilled. That's why in my opinion the rule should be that all consumers are unbound from their drivers before the supplier is unbound from its driver. > It will be then probed? Is it OK? Or am I missing something? If one driver depends on a service provided by another one for correctness, then it won't work anyway if its supplier goes away no matter what. > >>> There's a question about what if the supplier device is being unbound before > >>> the consumer one (for example, as a result of a hotplug event). My current > >>> view on that is that the consumer needs to be force-unbound in that case too, > >>> but I guess I may be persuaded otherwise given sufficiently convincing > >>> arguments. > >> Some devices can have 'weak' dependencies - they will be still functional > >> without some resources. > > Right. That's on my radar. > > > >> In fact two last examples from my 1st paragraph are > >> counter-examples for this. I suspect there should be some kind of notification > >> for them about removal of the resource. > >> > >>> Anyway, there are reasons to do that, like for example it may > >>> help with the synchronization. Namely, if there's a rule that suppliers > >>> cannot be unbound before any consumers linked to them, than the list of links > >>> to suppliers for a consumer can only change at its registration/probe or > >>> unbind/remove times (which simplifies things quite a bit). > >>> > >>> With that, the permanent links existing at the probe time for a consumer > >>> device can be used to check whether or not to defer the probing of it > >>> even before executing its probe callback. In turn, system suspend > >>> synchronization should be a matter of calling device_pm_wait_for_dev() > >>> for all consumers of a supplier device, in analogy with dpm_wait_for_children(), > >>> and so on. > >>> > >>> Of course, the new lists have to be stable during those operations and ensuring > >>> that is going to be somewhat tricky (AFAICS right now at least), but apart from > >>> that the whole concept looks reasonably straightforward to me. > >>> > >>> So, the question to everybody is whether or not this sounds reasonable or there > >>> are concerns about it and if so what they are. At this point I mostly need to > >>> know if I'm not overlooking anything fundamental at the general level. > >> Regarding fundamental things, maybe it is just my impression but parsing private > >> DT device nodes by kernel core assumes that convention about using resource > >> specifiers in DT is a strict rule, it should not be true. > > I really am not sure what you mean here, sorry. > > Device tree bindings are defined per device so theoretically only device > driver > should parse them(except few basic properties). This is of course only my > impression, but even in this thread Mark made similar statement [1]. No, DT bindings are not for exclusive use of a device driver. They provide information about the system configuration and layout to the OS as a whole. Some of that information may only be relevant to device drivers, but not all of it. Some types of resources need to be tracked globally (take address ranges in the memory address space, for example), some are used by frameworks without drivers' knowledge etc. > Assuming this, permanent links should not be used with device tree, as a > result > deferred probing will be still a problem. > > [1]: http://permalink.gmane.org/gmane.linux.power-management.general/67593 I'm not sure how you have derived this conclusion. It seems to reach too far to me. > >> As I wrote before I have send some early RFC with framework which solves most of > >> the problems described here[1], the missing part is suspend/resume support which > >> should be quite easy to add, I suspect. Moreover it solves problem of device > >> driver hot bind/unbind. > >> Could you take a look at it, I will be glad to know it is worth to continue work > >> on it? > >> > >> [1]: https://lkml.org/lkml/2014/12/10/342 > > I'm not sure to be honest. > > > > I'm not a big fan of notification-based mechanisms in general, because they > > depend on everyone registering those notifiers to implement them correctly and > > it gets additionally complicated if the ordering matters etc. So I personally > > wouldn't take that route. > > > > I guess some way of resource tracking will be necessary at one point, but what > > shape it should take is a good question. > > Any callback provided by a driver including probe/remove are in fact > notification mechanisms :) And they should be also correctly implemented. > Ordering in case of resource tracking is enforced by the framework, so I do > not see complication here. > > Anyway if we take two assumptions which are already true: > - device bound to driver can provide resources, > - device driver can be unloaded/unbound at any time. > Then notifications/callbacks seems to me the only solution. Unbinding a supplier driver can cause consumer drivers to be unbound automatically too. As I said above, if one driver depends on a service provided by another one for correctness, it won't work after the first one is unbound no matter what. Since device_release_driver() works unconditionally, there is no choice but to make it unbind all consumers with hard dependencies before unbinding the device itself. Notifying them won't help if they can't recover from that condition anyway. It may be a good idea to reprobe them then in case they can work without the missing supplier too, or to put them into the deferred probe list in case the supplier appears again. All of that is sort of academic, though, unless we have real use cases like that to deal with. Thanks, Rafael ^ permalink raw reply [flat|nested] 55+ messages in thread
* Re: [RFD] Functional dependencies between devices 2015-11-19 22:04 ` Rafael J. Wysocki @ 2015-11-20 1:11 ` Rafael J. Wysocki 2015-11-24 14:57 ` Andrzej Hajda 1 sibling, 0 replies; 55+ messages in thread From: Rafael J. Wysocki @ 2015-11-20 1:11 UTC (permalink / raw) To: Andrzej Hajda Cc: Linux PM list, Greg Kroah-Hartman, Linux Kernel Mailing List, Alan Stern, Grant Likely, Mark Brown, Rob Herring, Tomeu Vizoso, Thierry Reding, Dmitry Torokhov, Geert Uytterhoeven, Michael Turquette On Thursday, November 19, 2015 11:04:00 PM Rafael J. Wysocki wrote: > On Thursday, November 19, 2015 10:08:43 AM Andrzej Hajda wrote: > > On 11/18/2015 03:17 AM, Rafael J. Wysocki wrote: > > > On Tuesday, November 17, 2015 01:44:59 PM Andrzej Hajda wrote: > > >> Hi Rafael, > > >> > > [cut] > > > > So the operations that need to be taken care of are: > > > - Probe (suppliers need to be probed before consumers if the dependencies are > > > known beforehand). > > > - System suspend/resume (suppliers need to be suspended after consumers and > > > resumed before them) which may be asynchronous (so simple re-ordering doesn't > > > help). > > > - Runtime PM (suppliers should not be suspended if the consumers are not > > > suspended). > > I though provider's frameworks are taking care of it already. For example > > clock provider cannot suspend until there are prepared/enabled clocks. > > Similar enabled regulators, phys should block provider from runtime pm > > suspending. > > > > Are there situations/frameworks which requires additional care? > > Yes, there are, AFAICS. > > A somewhat extreme example of this is when an AML routine needed for power > management of one device uses something like a GPIO line or an I2C link > provided by another one. We don't even have a way to track that kind of > thing at the provider framework level and the only information we can get > from the platform firmware is "this device depends on that one". > > Plus, even if the frameworks track those things, when a device suspend is > requested, the question really is "Are there any devices that have to be > suspended before this one?" rather than "Are other devices using resources > provided by this one?". Of course, you may argue that answering the second > one will allow you to answer the first one too (that is based on the assumption > that you can always track all cases of resource utilization which may not be > entirely realistic), but getting that answer in a non-racy way may be rather > expensive. More importantly, the point here is not to help drivers etc to do the right things. It is to make it possible for them to provide the driver core with enough information so it can take care of doing the right things by itself. If that works as intended, the creation of a link between two devices will automatically cause the driver core to take care of ordering things in the right way etc in all of the cases I listed, so the drivers of those two devices don't need to worry about that any more. Thanks, Rafael ^ permalink raw reply [flat|nested] 55+ messages in thread
* Re: [RFD] Functional dependencies between devices 2015-11-19 22:04 ` Rafael J. Wysocki 2015-11-20 1:11 ` Rafael J. Wysocki @ 2015-11-24 14:57 ` Andrzej Hajda 2015-11-24 16:28 ` Rafael J. Wysocki 1 sibling, 1 reply; 55+ messages in thread From: Andrzej Hajda @ 2015-11-24 14:57 UTC (permalink / raw) To: Rafael J. Wysocki Cc: Linux PM list, Greg Kroah-Hartman, Linux Kernel Mailing List, Alan Stern, Grant Likely, Mark Brown, Rob Herring, Tomeu Vizoso, Thierry Reding, Dmitry Torokhov, Geert Uytterhoeven, Michael Turquette On 11/19/2015 11:04 PM, Rafael J. Wysocki wrote: > On Thursday, November 19, 2015 10:08:43 AM Andrzej Hajda wrote: >> On 11/18/2015 03:17 AM, Rafael J. Wysocki wrote: >>> On Tuesday, November 17, 2015 01:44:59 PM Andrzej Hajda wrote: >>>> Hi Rafael, >>>> > [cut] > >>> So the operations that need to be taken care of are: >>> - Probe (suppliers need to be probed before consumers if the dependencies are >>> known beforehand). >>> - System suspend/resume (suppliers need to be suspended after consumers and >>> resumed before them) which may be asynchronous (so simple re-ordering doesn't >>> help). >>> - Runtime PM (suppliers should not be suspended if the consumers are not >>> suspended). >> I though provider's frameworks are taking care of it already. For example >> clock provider cannot suspend until there are prepared/enabled clocks. >> Similar enabled regulators, phys should block provider from runtime pm >> suspending. >> >> Are there situations/frameworks which requires additional care? > Yes, there are, AFAICS. > > A somewhat extreme example of this is when an AML routine needed for power > management of one device uses something like a GPIO line or an I2C link > provided by another one. We don't even have a way to track that kind of > thing at the provider framework level and the only information we can get > from the platform firmware is "this device depends on that one". > > Plus, even if the frameworks track those things, when a device suspend is > requested, the question really is "Are there any devices that have to be > suspended before this one?" rather than "Are other devices using resources > provided by this one?". Of course, you may argue that answering the second > one will allow you to answer the first one too (that is based on the assumption > that you can always track all cases of resource utilization which may not be > entirely realistic), but getting that answer in a non-racy way may be rather > expensive. In such extreme case the device itself can play a role of resource. But in my proposal I do not try to answer which devices/resource depends on which ones, we do not need such info. It is just matter of notifying direct consumers about change of availability of given resource, and this notification is necessary anyway if we want to support hot resource/drivers (un-)plugging. > >>> - System shutdown (shutdown callbacks should be executed for consumers first). >>> - Driver unbind (a supplier driver cannot be unbound before any of its consumer >>> drivers). >>> >>> In principle you can use resource tracking to figure out all of the involved >>> dependencies, but that would require walking complicated data structures unless >>> you add an intermediate "device dependency" layer which is going to be analogous >>> to the one discussed here. >> It should be enough if provider notifies consumers that the resource >> will be unavailable. > To me, this isn't going in the right direction. You should be asking "Am I > allowed to suspend now?" instead of saying "I'm suspending and now you deal > with it" to somebody. Why is that so? Because the other end may simply be > unable to deal with the situation in the first place. No. It is just saying "I want to suspend now, please not use my resources". In such case consumer should unprepare clocks, disable regulators, etc. But if it is not able to do so it just ignores the request. Provider will know anyway that his resources are in use and will not suspend. > >>>>> My idea is to represent a supplier-consumer dependency between devices (or >>>>> more precisely between device+driver combos) as a "link" object containing >>>>> pointers to the devices in question, a list node for each of them and some >>>>> additional information related to the management of those objects, ie. >>>>> something like: >>>>> >>>>> struct device_link { >>>>> struct device *supplier; >>>>> struct list_head supplier_node; >>>>> struct device *consumer; >>>>> struct list_head consumer_node; >>>>> <flags, status etc> >>>>> }; >>>>> >>>>> In general, there will be two lists of those things per device, one list >>>>> of links to consumers and one list of links to suppliers. >>>>> >>>>> In that picture, links will be created by calling, say: >>>>> >>>>> int device_add_link(struct device *me, struct device *my_supplier, unsigned int flags); >>>>> >>>>> and they will be deleted by the driver core when not needed any more. The >>>>> creation of a link should also cause dpm_list and the list used during shutdown >>>>> to be reordered if needed. >>>>> >>>>> In principle, it seems usefult to consider two types of links, one created >>>>> at device registration time (when registering the second device from the linked >>>>> pair, whichever it is) and one created at probe time (of the consumer device). >>>>> I'll refer to them as "permanent" and "probe-time" links, respectively. >>>>> >>>>> The permanent links (created at device registration time) will stay around >>>>> until one of the linked devices is unregistered (at which time the driver >>>>> core will drop the link along with the device going away). The probe-time >>>>> ones will be dropped (automatically) at the consumer device driver unbind time. >>>> What about permanent links in case provider is unregistered? Should they >>>> disappear? It will not make consumers happy. What if the provider will be >>>> re-registered. >>> If the device object is gone, it cannot be pointed to by any links (on any end) >>> any more. That's just physically impossible. :-) >> So the link will disappear and the 'consumer' will have dependencies >> fulfilled. > That's why in my opinion the rule should be that all consumers are unbound from > their drivers before the supplier is unbound from its driver. But the rule will not work with 'weak' dependencies. > >> It will be then probed? Is it OK? Or am I missing something? > If one driver depends on a service provided by another one for correctness, > then it won't work anyway if its supplier goes away no matter what. > >>>>> There's a question about what if the supplier device is being unbound before >>>>> the consumer one (for example, as a result of a hotplug event). My current >>>>> view on that is that the consumer needs to be force-unbound in that case too, >>>>> but I guess I may be persuaded otherwise given sufficiently convincing >>>>> arguments. >>>> Some devices can have 'weak' dependencies - they will be still functional >>>> without some resources. >>> Right. That's on my radar. >>> >>>> In fact two last examples from my 1st paragraph are >>>> counter-examples for this. I suspect there should be some kind of notification >>>> for them about removal of the resource. >>>> >>>>> Anyway, there are reasons to do that, like for example it may >>>>> help with the synchronization. Namely, if there's a rule that suppliers >>>>> cannot be unbound before any consumers linked to them, than the list of links >>>>> to suppliers for a consumer can only change at its registration/probe or >>>>> unbind/remove times (which simplifies things quite a bit). >>>>> >>>>> With that, the permanent links existing at the probe time for a consumer >>>>> device can be used to check whether or not to defer the probing of it >>>>> even before executing its probe callback. In turn, system suspend >>>>> synchronization should be a matter of calling device_pm_wait_for_dev() >>>>> for all consumers of a supplier device, in analogy with dpm_wait_for_children(), >>>>> and so on. >>>>> >>>>> Of course, the new lists have to be stable during those operations and ensuring >>>>> that is going to be somewhat tricky (AFAICS right now at least), but apart from >>>>> that the whole concept looks reasonably straightforward to me. >>>>> >>>>> So, the question to everybody is whether or not this sounds reasonable or there >>>>> are concerns about it and if so what they are. At this point I mostly need to >>>>> know if I'm not overlooking anything fundamental at the general level. >>>> Regarding fundamental things, maybe it is just my impression but parsing private >>>> DT device nodes by kernel core assumes that convention about using resource >>>> specifiers in DT is a strict rule, it should not be true. >>> I really am not sure what you mean here, sorry. >> Device tree bindings are defined per device so theoretically only device >> driver >> should parse them(except few basic properties). This is of course only my >> impression, but even in this thread Mark made similar statement [1]. > No, DT bindings are not for exclusive use of a device driver. They provide > information about the system configuration and layout to the OS as a whole. > Some of that information may only be relevant to device drivers, but not all > of it. > > Some types of resources need to be tracked globally (take address ranges in > the memory address space, for example), some are used by frameworks without > drivers' knowledge etc. > >> Assuming this, permanent links should not be used with device tree, as a >> result >> deferred probing will be still a problem. >> >> [1]: http://permalink.gmane.org/gmane.linux.power-management.general/67593 > I'm not sure how you have derived this conclusion. It seems to reach too far > to me. Lets drop my impressions, as there is no specification to verify it. What about resources which are present in device node, but driver do not use for some reason? Only driver knows which ones it requires in the specific scenario. Real example: HDMI node can contain links to SPDIF/audio clocks but since kernel is compiled without audio it will not use them at all. How do driver core will know about it. > >>>> As I wrote before I have send some early RFC with framework which solves most of >>>> the problems described here[1], the missing part is suspend/resume support which >>>> should be quite easy to add, I suspect. Moreover it solves problem of device >>>> driver hot bind/unbind. >>>> Could you take a look at it, I will be glad to know it is worth to continue work >>>> on it? >>>> >>>> [1]: https://lkml.org/lkml/2014/12/10/342 >>> I'm not sure to be honest. >>> >>> I'm not a big fan of notification-based mechanisms in general, because they >>> depend on everyone registering those notifiers to implement them correctly and >>> it gets additionally complicated if the ordering matters etc. So I personally >>> wouldn't take that route. >>> >>> I guess some way of resource tracking will be necessary at one point, but what >>> shape it should take is a good question. >> Any callback provided by a driver including probe/remove are in fact >> notification mechanisms :) And they should be also correctly implemented. >> Ordering in case of resource tracking is enforced by the framework, so I do >> not see complication here. >> >> Anyway if we take two assumptions which are already true: >> - device bound to driver can provide resources, >> - device driver can be unloaded/unbound at any time. >> Then notifications/callbacks seems to me the only solution. > Unbinding a supplier driver can cause consumer drivers to be unbound > automatically too. As I said before, it will not work with 'weak' dependencies. > > As I said above, if one driver depends on a service provided by another one > for correctness, it won't work after the first one is unbound no matter what. > Since device_release_driver() works unconditionally, there is no choice but to > make it unbind all consumers with hard dependencies before unbinding the > device itself. > > Notifying them won't help if they can't recover from that condition anyway. But there are cases devices can work without some resources. > > It may be a good idea to reprobe them then in case they can work without the > missing supplier too, or to put them into the deferred probe list in case the > supplier appears again. All of that is sort of academic, though, unless we > have real use cases like that to deal with. Real hardware case(it is not correctly modeled in drivers): HDMI generates clock, which is used by Display Controller. Display Controller then generates new clock based on the 1st one. The latter is used by HDMI. This is real example from latest Exynos SoCs. How do you want to model these dependencies using devices? Regards Andrzej > > Thanks, > Rafael > > ^ permalink raw reply [flat|nested] 55+ messages in thread
* Re: [RFD] Functional dependencies between devices 2015-11-24 14:57 ` Andrzej Hajda @ 2015-11-24 16:28 ` Rafael J. Wysocki 2015-11-30 7:16 ` Andrzej Hajda 0 siblings, 1 reply; 55+ messages in thread From: Rafael J. Wysocki @ 2015-11-24 16:28 UTC (permalink / raw) To: Andrzej Hajda Cc: Linux PM list, Greg Kroah-Hartman, Linux Kernel Mailing List, Alan Stern, Grant Likely, Mark Brown, Rob Herring, Tomeu Vizoso, Thierry Reding, Dmitry Torokhov, Geert Uytterhoeven, Michael Turquette On Tuesday, November 24, 2015 03:57:09 PM Andrzej Hajda wrote: > On 11/19/2015 11:04 PM, Rafael J. Wysocki wrote: > > On Thursday, November 19, 2015 10:08:43 AM Andrzej Hajda wrote: > >> On 11/18/2015 03:17 AM, Rafael J. Wysocki wrote: > >>> On Tuesday, November 17, 2015 01:44:59 PM Andrzej Hajda wrote: > >>>> Hi Rafael, > >>>> > > [cut] > > > >>> So the operations that need to be taken care of are: > >>> - Probe (suppliers need to be probed before consumers if the dependencies are > >>> known beforehand). > >>> - System suspend/resume (suppliers need to be suspended after consumers and > >>> resumed before them) which may be asynchronous (so simple re-ordering doesn't > >>> help). > >>> - Runtime PM (suppliers should not be suspended if the consumers are not > >>> suspended). > >> I though provider's frameworks are taking care of it already. For example > >> clock provider cannot suspend until there are prepared/enabled clocks. > >> Similar enabled regulators, phys should block provider from runtime pm > >> suspending. > >> > >> Are there situations/frameworks which requires additional care? > > Yes, there are, AFAICS. > > > > A somewhat extreme example of this is when an AML routine needed for power > > management of one device uses something like a GPIO line or an I2C link > > provided by another one. We don't even have a way to track that kind of > > thing at the provider framework level and the only information we can get > > from the platform firmware is "this device depends on that one". > > > > Plus, even if the frameworks track those things, when a device suspend is > > requested, the question really is "Are there any devices that have to be > > suspended before this one?" rather than "Are other devices using resources > > provided by this one?". Of course, you may argue that answering the second > > one will allow you to answer the first one too (that is based on the assumption > > that you can always track all cases of resource utilization which may not be > > entirely realistic), but getting that answer in a non-racy way may be rather > > expensive. > > In such extreme case the device itself can play a role of resource. > But in my proposal I do not try to answer which devices/resource depends > on which ones, we do not need such info. > It is just matter of notifying direct consumers about change of availability > of given resource, and this notification is necessary anyway if we want > to support hot resource/drivers (un-)plugging. Well, we've been supporting hotplug for quite a while without that ... You seem to be referring to situations in which individual resources may go away and drivers are supposed to reconfigure themselves on the fly. This is not what the $subject proposal is about. > > > >>> - System shutdown (shutdown callbacks should be executed for consumers first). > >>> - Driver unbind (a supplier driver cannot be unbound before any of its consumer > >>> drivers). > >>> > >>> In principle you can use resource tracking to figure out all of the involved > >>> dependencies, but that would require walking complicated data structures unless > >>> you add an intermediate "device dependency" layer which is going to be analogous > >>> to the one discussed here. > >> It should be enough if provider notifies consumers that the resource > >> will be unavailable. > > To me, this isn't going in the right direction. You should be asking "Am I > > allowed to suspend now?" instead of saying "I'm suspending and now you deal > > with it" to somebody. Why is that so? Because the other end may simply be > > unable to deal with the situation in the first place. > > No. It is just saying "I want to suspend now, please not use my resources". > In such case consumer should unprepare clocks, disable regulators, etc. > But if it is not able to do so it just ignores the request. Provider > will know > anyway that his resources are in use and will not suspend. This goes beyond the runtime PM framework which is based on device reference counting and for system suspend it's not practical at all, because one driver refusing to suspend aborts the entire operation system-wide. > >>>>> My idea is to represent a supplier-consumer dependency between devices (or > >>>>> more precisely between device+driver combos) as a "link" object containing > >>>>> pointers to the devices in question, a list node for each of them and some > >>>>> additional information related to the management of those objects, ie. > >>>>> something like: > >>>>> > >>>>> struct device_link { > >>>>> struct device *supplier; > >>>>> struct list_head supplier_node; > >>>>> struct device *consumer; > >>>>> struct list_head consumer_node; > >>>>> <flags, status etc> > >>>>> }; > >>>>> > >>>>> In general, there will be two lists of those things per device, one list > >>>>> of links to consumers and one list of links to suppliers. > >>>>> > >>>>> In that picture, links will be created by calling, say: > >>>>> > >>>>> int device_add_link(struct device *me, struct device *my_supplier, unsigned int flags); > >>>>> > >>>>> and they will be deleted by the driver core when not needed any more. The > >>>>> creation of a link should also cause dpm_list and the list used during shutdown > >>>>> to be reordered if needed. > >>>>> > >>>>> In principle, it seems usefult to consider two types of links, one created > >>>>> at device registration time (when registering the second device from the linked > >>>>> pair, whichever it is) and one created at probe time (of the consumer device). > >>>>> I'll refer to them as "permanent" and "probe-time" links, respectively. > >>>>> > >>>>> The permanent links (created at device registration time) will stay around > >>>>> until one of the linked devices is unregistered (at which time the driver > >>>>> core will drop the link along with the device going away). The probe-time > >>>>> ones will be dropped (automatically) at the consumer device driver unbind time. > >>>> What about permanent links in case provider is unregistered? Should they > >>>> disappear? It will not make consumers happy. What if the provider will be > >>>> re-registered. > >>> If the device object is gone, it cannot be pointed to by any links (on any end) > >>> any more. That's just physically impossible. :-) > >> So the link will disappear and the 'consumer' will have dependencies > >> fulfilled. > > That's why in my opinion the rule should be that all consumers are unbound from > > their drivers before the supplier is unbound from its driver. > > But the rule will not work with 'weak' dependencies. No, it won't. Enough of them are actually hard, though, for this to be a relevant case anyway. [cut] > > Lets drop my impressions, as there is no specification to verify it. > > What about resources which are present in device node, but driver do > not use for some reason? Only driver knows which ones it requires in > the specific scenario. Real example: HDMI node can contain links > to SPDIF/audio clocks but since kernel is compiled without audio it > will not use them at all. How do driver core will know about it. It won't know about then automatically. It needs to be told about whether or not a dependency is there, either by a driver or by a bus type or a framework of some sort And since the driver core works with device objects in general, this is the "granularity" it can handle. [cut] > > But there are cases devices can work without some resources. > Yes, there are. > > It may be a good idea to reprobe them then in case they can work without the > > missing supplier too, or to put them into the deferred probe list in case the > > supplier appears again. All of that is sort of academic, though, unless we > > have real use cases like that to deal with. > > Real hardware case(it is not correctly modeled in drivers): > HDMI generates clock, which is used by Display Controller. > Display Controller then generates new clock based on the 1st one. > The latter is used by HDMI. > This is real example from latest Exynos SoCs. > How do you want to model these dependencies using devices? I don't want to model them with devices at all and this is not the point here. The point (as I said once already) is to make it possible to tell the driver core of a functional dependency between *devices* in which case it will take that dependency into account automatically in several important situations, so the drivers of those devices won't need to worry about their respective ordering etc. You seem to be saying that this is not useful and quite honestly I'm not really sure why. Thanks, Rafael ^ permalink raw reply [flat|nested] 55+ messages in thread
* Re: [RFD] Functional dependencies between devices 2015-11-24 16:28 ` Rafael J. Wysocki @ 2015-11-30 7:16 ` Andrzej Hajda 0 siblings, 0 replies; 55+ messages in thread From: Andrzej Hajda @ 2015-11-30 7:16 UTC (permalink / raw) To: Rafael J. Wysocki Cc: Linux PM list, Greg Kroah-Hartman, Linux Kernel Mailing List, Alan Stern, Grant Likely, Mark Brown, Rob Herring, Tomeu Vizoso, Thierry Reding, Dmitry Torokhov, Geert Uytterhoeven, Michael Turquette Hi, Sorry for late response. On 11/24/2015 05:28 PM, Rafael J. Wysocki wrote: > On Tuesday, November 24, 2015 03:57:09 PM Andrzej Hajda wrote: >> On 11/19/2015 11:04 PM, Rafael J. Wysocki wrote: >>> On Thursday, November 19, 2015 10:08:43 AM Andrzej Hajda wrote: >>>> On 11/18/2015 03:17 AM, Rafael J. Wysocki wrote: >>>>> On Tuesday, November 17, 2015 01:44:59 PM Andrzej Hajda wrote: >>>>>> Hi Rafael, >>>>>> >>> [cut] >>> >>>>> So the operations that need to be taken care of are: >>>>> - Probe (suppliers need to be probed before consumers if the dependencies are >>>>> known beforehand). >>>>> - System suspend/resume (suppliers need to be suspended after consumers and >>>>> resumed before them) which may be asynchronous (so simple re-ordering doesn't >>>>> help). >>>>> - Runtime PM (suppliers should not be suspended if the consumers are not >>>>> suspended). >>>> I though provider's frameworks are taking care of it already. For example >>>> clock provider cannot suspend until there are prepared/enabled clocks. >>>> Similar enabled regulators, phys should block provider from runtime pm >>>> suspending. >>>> >>>> Are there situations/frameworks which requires additional care? >>> Yes, there are, AFAICS. >>> >>> A somewhat extreme example of this is when an AML routine needed for power >>> management of one device uses something like a GPIO line or an I2C link >>> provided by another one. We don't even have a way to track that kind of >>> thing at the provider framework level and the only information we can get >>> from the platform firmware is "this device depends on that one". >>> >>> Plus, even if the frameworks track those things, when a device suspend is >>> requested, the question really is "Are there any devices that have to be >>> suspended before this one?" rather than "Are other devices using resources >>> provided by this one?". Of course, you may argue that answering the second >>> one will allow you to answer the first one too (that is based on the assumption >>> that you can always track all cases of resource utilization which may not be >>> entirely realistic), but getting that answer in a non-racy way may be rather >>> expensive. >> In such extreme case the device itself can play a role of resource. >> But in my proposal I do not try to answer which devices/resource depends >> on which ones, we do not need such info. >> It is just matter of notifying direct consumers about change of availability >> of given resource, and this notification is necessary anyway if we want >> to support hot resource/drivers (un-)plugging. > Well, we've been supporting hotplug for quite a while without that ... > > You seem to be referring to situations in which individual resources may go > away and drivers are supposed to reconfigure themselves on the fly. > > This is not what the $subject proposal is about. Currently if you undbind some driver from the device and the driver is a provider with active consumers usually it results in crashes/oopses. So I wouldn't say that hot resources/drivers unplugging is supported. > >>>>> - System shutdown (shutdown callbacks should be executed for consumers first). >>>>> - Driver unbind (a supplier driver cannot be unbound before any of its consumer >>>>> drivers). >>>>> >>>>> In principle you can use resource tracking to figure out all of the involved >>>>> dependencies, but that would require walking complicated data structures unless >>>>> you add an intermediate "device dependency" layer which is going to be analogous >>>>> to the one discussed here. >>>> It should be enough if provider notifies consumers that the resource >>>> will be unavailable. >>> To me, this isn't going in the right direction. You should be asking "Am I >>> allowed to suspend now?" instead of saying "I'm suspending and now you deal >>> with it" to somebody. Why is that so? Because the other end may simply be >>> unable to deal with the situation in the first place. >> No. It is just saying "I want to suspend now, please not use my resources". >> In such case consumer should unprepare clocks, disable regulators, etc. >> But if it is not able to do so it just ignores the request. Provider >> will know >> anyway that his resources are in use and will not suspend. > This goes beyond the runtime PM framework which is based on device reference > counting and for system suspend it's not practical at all, because one driver > refusing to suspend aborts the entire operation system-wide. Aren't current suspend callbacks designed that way? Documentation/power/devices.txt says clearly: "If any of these callbacks returns an error, the system won't enter the desired low-power state. Instead the PM core will unwind its actions by resuming all the devices that were suspended." > >>>>>>> My idea is to represent a supplier-consumer dependency between devices (or >>>>>>> more precisely between device+driver combos) as a "link" object containing >>>>>>> pointers to the devices in question, a list node for each of them and some >>>>>>> additional information related to the management of those objects, ie. >>>>>>> something like: >>>>>>> >>>>>>> struct device_link { >>>>>>> struct device *supplier; >>>>>>> struct list_head supplier_node; >>>>>>> struct device *consumer; >>>>>>> struct list_head consumer_node; >>>>>>> <flags, status etc> >>>>>>> }; >>>>>>> >>>>>>> In general, there will be two lists of those things per device, one list >>>>>>> of links to consumers and one list of links to suppliers. >>>>>>> >>>>>>> In that picture, links will be created by calling, say: >>>>>>> >>>>>>> int device_add_link(struct device *me, struct device *my_supplier, unsigned int flags); >>>>>>> >>>>>>> and they will be deleted by the driver core when not needed any more. The >>>>>>> creation of a link should also cause dpm_list and the list used during shutdown >>>>>>> to be reordered if needed. >>>>>>> >>>>>>> In principle, it seems usefult to consider two types of links, one created >>>>>>> at device registration time (when registering the second device from the linked >>>>>>> pair, whichever it is) and one created at probe time (of the consumer device). >>>>>>> I'll refer to them as "permanent" and "probe-time" links, respectively. >>>>>>> >>>>>>> The permanent links (created at device registration time) will stay around >>>>>>> until one of the linked devices is unregistered (at which time the driver >>>>>>> core will drop the link along with the device going away). The probe-time >>>>>>> ones will be dropped (automatically) at the consumer device driver unbind time. >>>>>> What about permanent links in case provider is unregistered? Should they >>>>>> disappear? It will not make consumers happy. What if the provider will be >>>>>> re-registered. >>>>> If the device object is gone, it cannot be pointed to by any links (on any end) >>>>> any more. That's just physically impossible. :-) >>>> So the link will disappear and the 'consumer' will have dependencies >>>> fulfilled. >>> That's why in my opinion the rule should be that all consumers are unbound from >>> their drivers before the supplier is unbound from its driver. >> But the rule will not work with 'weak' dependencies. > No, it won't. Enough of them are actually hard, though, for this to be > a relevant case anyway. > > [cut] > >> Lets drop my impressions, as there is no specification to verify it. >> >> What about resources which are present in device node, but driver do >> not use for some reason? Only driver knows which ones it requires in >> the specific scenario. Real example: HDMI node can contain links >> to SPDIF/audio clocks but since kernel is compiled without audio it >> will not use them at all. How do driver core will know about it. > It won't know about then automatically. It needs to be told about > whether or not a dependency is there, either by a driver or by a bus type > or a framework of some sort And since the driver core works with device > objects in general, this is the "granularity" it can handle. > > [cut] > >> But there are cases devices can work without some resources. >> > Yes, there are. > >>> It may be a good idea to reprobe them then in case they can work without the >>> missing supplier too, or to put them into the deferred probe list in case the >>> supplier appears again. All of that is sort of academic, though, unless we >>> have real use cases like that to deal with. >> Real hardware case(it is not correctly modeled in drivers): >> HDMI generates clock, which is used by Display Controller. >> Display Controller then generates new clock based on the 1st one. >> The latter is used by HDMI. >> This is real example from latest Exynos SoCs. >> How do you want to model these dependencies using devices? > I don't want to model them with devices at all and this is not the point here. > > The point (as I said once already) is to make it possible to tell the driver > core of a functional dependency between *devices* in which case it will take > that dependency into account automatically in several important situations, > so the drivers of those devices won't need to worry about their respective > ordering etc. > > You seem to be saying that this is not useful and quite honestly I'm not > really sure why. I just want to clarify behavior of the framework in different scenarios. Regards Andrzej > > Thanks, > Rafael > > ^ permalink raw reply [flat|nested] 55+ messages in thread
* Re: [RFD] Functional dependencies between devices 2015-10-27 15:24 [RFD] Functional dependencies between devices Rafael J. Wysocki ` (5 preceding siblings ...) 2015-11-17 12:44 ` Andrzej Hajda @ 2015-11-17 12:49 ` Andrzej Hajda 2015-11-17 13:55 ` Mark Brown 2015-11-17 20:31 ` Alan Stern 2016-01-14 1:52 ` [RFC][PATCH 0/5] " Rafael J. Wysocki 7 siblings, 2 replies; 55+ messages in thread From: Andrzej Hajda @ 2015-11-17 12:49 UTC (permalink / raw) To: Rafael J. Wysocki, Linux PM list Cc: Greg Kroah-Hartman, Linux Kernel Mailing List, Alan Stern, Grant Likely, Mark Brown, Rob Herring, Tomeu Vizoso, Thierry Reding, Dmitry Torokhov, Geert Uytterhoeven, Michael Turquette Hi Rafael, It is just re-send of the previous message with fixed e-mail. Please forgive me late reply, but I have missed this thread before. On 10/27/2015 04:24 PM, Rafael J. Wysocki wrote: > Hi All, > > As discussed in the recent "On-demand device probing" thread and in a Kernel > Summit session earlier today, 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). I think the real dependency is when some entity asks for some resource (irq, clock, gpio,...). Usually the entity is some device driver during probing and the resource is provided by some bound device but there are many exceptions for this scenario: - many clock providers, irq domains are not provided by devices, - there are also dependencies between clock providers, ie. some clock provider requires clocks provided by another clock provider, so the entity is also not a device driver, - there are resources which can be requested after probe - case of componentized devices (DRM for example), more precisely they can be requested during probe of random component or master of componentized device, - another case are requests for some additional/optional resources after device driver probe, for example phone usually does not require HDMI related resources until user attach HDMI cable, - (semi-)circular dependencies - 1st device provides clock used by other devices which provides other resources used by the 1st device, scenario present in some video pipelines, like camera subsystem + sensors. These examples shows that dependencies between bound device drivers are just subset of bigger issue, maybe it is worth to look for more general solution. > > So I want to be able to represent those functional dependencies between devices > and I'd like 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). Could you elaborate these distinct use cases. I am curious because I have proposed resource tracking framework [1] which should solve most of the issues described here. It was not designed to solve suspend/resume issues, but it could be easily extended to support it, I suppose. [1]: https://lkml.org/lkml/2014/12/10/342 > > My idea is to represent a supplier-consumer dependency between devices (or > more precisely between device+driver combos) as a "link" object containing > pointers to the devices in question, a list node for each of them and some > additional information related to the management of those objects, ie. > something like: > > struct device_link { > struct device *supplier; > struct list_head supplier_node; > struct device *consumer; > struct list_head consumer_node; > <flags, status etc> > }; > > In general, there will be two lists of those things per device, one list > of links to consumers and one list of links to suppliers. > > In that picture, links will be created by calling, say: > > int device_add_link(struct device *me, struct device *my_supplier, unsigned int flags); > > and they will be deleted by the driver core when not needed any more. The > creation of a link should also cause dpm_list and the list used during shutdown > to be reordered if needed. > > In principle, it seems usefult to consider two types of links, one created > at device registration time (when registering the second device from the linked > pair, whichever it is) and one created at probe time (of the consumer device). > I'll refer to them as "permanent" and "probe-time" links, respectively. > > The permanent links (created at device registration time) will stay around > until one of the linked devices is unregistered (at which time the driver > core will drop the link along with the device going away). The probe-time > ones will be dropped (automatically) at the consumer device driver unbind time. What about permanent links in case provider is unregistered? Should they disappear? It will not make consumers happy. What if the provider will be re-registered. > > There's a question about what if the supplier device is being unbound before > the consumer one (for example, as a result of a hotplug event). My current > view on that is that the consumer needs to be force-unbound in that case too, > but I guess I may be persuaded otherwise given sufficiently convincing > arguments. Some devices can have 'weak' dependencies - they will be still functional without some resources. In fact two last examples from my 1st paragraph are counter-examples for this. I suspect there should be some kind of notification for them about removal of the resource. > Anyway, there are reasons to do that, like for example it may > help with the synchronization. Namely, if there's a rule that suppliers > cannot be unbound before any consumers linked to them, than the list of links > to suppliers for a consumer can only change at its registration/probe or > unbind/remove times (which simplifies things quite a bit). > > With that, the permanent links existing at the probe time for a consumer > device can be used to check whether or not to defer the probing of it > even before executing its probe callback. In turn, system suspend > synchronization should be a matter of calling device_pm_wait_for_dev() > for all consumers of a supplier device, in analogy with dpm_wait_for_children(), > and so on. > > Of course, the new lists have to be stable during those operations and ensuring > that is going to be somewhat tricky (AFAICS right now at least), but apart from > that the whole concept looks reasonably straightforward to me. > > So, the question to everybody is whether or not this sounds reasonable or there > are concerns about it and if so what they are. At this point I mostly need to > know if I'm not overlooking anything fundamental at the general level. Regarding fundamental things, maybe it is just my impression but parsing private DT device nodes by kernel core assumes that convention about using resource specifiers in DT is a strict rule, it should not be true. As I wrote before I have send some early RFC with framework which solves most of the problems described here[1], the missing part is suspend/resume support which should be quite easy to add, I suspect. Moreover it solves problem of device driver hot bind/unbind. Could you take a look at it, I will be glad to know it is worth to continue work on it? [1]: https://lkml.org/lkml/2014/12/10/342 Regards Andrzej > > Thanks, > Rafael > > -- > To unsubscribe from this list: send the line "unsubscribe linux-pm" in > the body of a message to majordomo@vger.kernel.org > More majordomo info at http://vger.kernel.org/majordomo-info.html > ^ permalink raw reply [flat|nested] 55+ messages in thread
* Re: [RFD] Functional dependencies between devices 2015-11-17 12:49 ` Andrzej Hajda @ 2015-11-17 13:55 ` Mark Brown 2015-11-19 6:50 ` Andrzej Hajda 2015-11-19 13:18 ` Thierry Reding 2015-11-17 20:31 ` Alan Stern 1 sibling, 2 replies; 55+ messages in thread From: Mark Brown @ 2015-11-17 13:55 UTC (permalink / raw) To: Andrzej Hajda Cc: Rafael J. Wysocki, Linux PM list, Greg Kroah-Hartman, Linux Kernel Mailing List, Alan Stern, Grant Likely, Rob Herring, Tomeu Vizoso, Thierry Reding, Dmitry Torokhov, Geert Uytterhoeven, Michael Turquette [-- Attachment #1: Type: text/plain, Size: 1248 bytes --] On Tue, Nov 17, 2015 at 01:49:17PM +0100, Andrzej Hajda wrote: Please fix your mail client to word wrap within paragraphs at something substantially less than 80 columns. Doing this makes your messages much easier to read and reply to. > On 10/27/2015 04:24 PM, Rafael J. Wysocki wrote: > this scenario: > - many clock providers, irq domains are not provided by devices, That seems like something we can and possibly should change if we want. > - there are also dependencies between clock providers, ie. some clock provider > requires clocks provided by another clock provider, so the entity is also not a > device driver, This is going to be really common but I'm not sure I see a problem with it in terms of what Raphael is proposing - could you go into more detail on the problem you see here? > - another case are requests for some additional/optional resources after device > driver probe, for example phone usually does not require HDMI related resources > until user attach HDMI cable, Normally the drivers we need would all be loaded based on the hardware we have in the system, it would be very unusual to dynamically request new resources at runtime to deal with a reconfiguration. Doing so seems likely to result in fragility. [-- Attachment #2: signature.asc --] [-- Type: application/pgp-signature, Size: 473 bytes --] ^ permalink raw reply [flat|nested] 55+ messages in thread
* Re: [RFD] Functional dependencies between devices 2015-11-17 13:55 ` Mark Brown @ 2015-11-19 6:50 ` Andrzej Hajda 2015-11-21 14:04 ` Mark Brown 2015-11-19 13:18 ` Thierry Reding 1 sibling, 1 reply; 55+ messages in thread From: Andrzej Hajda @ 2015-11-19 6:50 UTC (permalink / raw) To: Mark Brown Cc: Rafael J. Wysocki, Linux PM list, Greg Kroah-Hartman, Linux Kernel Mailing List, Alan Stern, Grant Likely, Rob Herring, Tomeu Vizoso, Thierry Reding, Dmitry Torokhov, Geert Uytterhoeven, Michael Turquette On 11/17/2015 02:55 PM, Mark Brown wrote: > On Tue, Nov 17, 2015 at 01:49:17PM +0100, Andrzej Hajda wrote: > > Please fix your mail client to word wrap within paragraphs at something > substantially less than 80 columns. Doing this makes your messages much > easier to read and reply to. > >> On 10/27/2015 04:24 PM, Rafael J. Wysocki wrote: >> this scenario: >> - many clock providers, irq domains are not provided by devices, > That seems like something we can and possibly should change if we want. > >> - there are also dependencies between clock providers, ie. some clock provider >> requires clocks provided by another clock provider, so the entity is also not a >> device driver, > This is going to be really common but I'm not sure I see a problem with > it in terms of what Raphael is proposing - could you go into more detail > on the problem you see here? If clock provider is not a device driver and it depends on clocks of another clock provider you cannot 'translate' this dependency as dependency between devices, so this RFD does not cover them. Additionally if you look into kernel there are many calls in form 'clk_get(NULL, name)', it suggests that not only clock providers are consumers without underlying device driver. Regards Andrzej ^ permalink raw reply [flat|nested] 55+ messages in thread
* Re: [RFD] Functional dependencies between devices 2015-11-19 6:50 ` Andrzej Hajda @ 2015-11-21 14:04 ` Mark Brown 2015-11-24 13:56 ` Andrzej Hajda 0 siblings, 1 reply; 55+ messages in thread From: Mark Brown @ 2015-11-21 14:04 UTC (permalink / raw) To: Andrzej Hajda Cc: Rafael J. Wysocki, Linux PM list, Greg Kroah-Hartman, Linux Kernel Mailing List, Alan Stern, Grant Likely, Rob Herring, Tomeu Vizoso, Thierry Reding, Dmitry Torokhov, Geert Uytterhoeven, Michael Turquette [-- Attachment #1: Type: text/plain, Size: 1062 bytes --] On Thu, Nov 19, 2015 at 07:50:45AM +0100, Andrzej Hajda wrote: > On 11/17/2015 02:55 PM, Mark Brown wrote: > > This is going to be really common but I'm not sure I see a problem with > > it in terms of what Raphael is proposing - could you go into more detail > > on the problem you see here? > If clock provider is not a device driver and it depends on clocks of > another clock > provider you cannot 'translate' this dependency as dependency between > devices, What makes you say that this is the case? There should be nothing stopping us having dependencies between two devices of the same type. > so this RFD does not cover them. > Additionally if you look into kernel there are many calls in form > 'clk_get(NULL, name)', > it suggests that not only clock providers are consumers without > underlying device driver. Like I said in my earlier reply: | > - many clock providers, irq domains are not provided by devices, | That seems like something we can and possibly should change if we want. This applies just as much to consumers as to providers. [-- Attachment #2: signature.asc --] [-- Type: application/pgp-signature, Size: 473 bytes --] ^ permalink raw reply [flat|nested] 55+ messages in thread
* Re: [RFD] Functional dependencies between devices 2015-11-21 14:04 ` Mark Brown @ 2015-11-24 13:56 ` Andrzej Hajda 0 siblings, 0 replies; 55+ messages in thread From: Andrzej Hajda @ 2015-11-24 13:56 UTC (permalink / raw) To: Mark Brown Cc: Rafael J. Wysocki, Linux PM list, Greg Kroah-Hartman, Linux Kernel Mailing List, Alan Stern, Grant Likely, Rob Herring, Tomeu Vizoso, Thierry Reding, Dmitry Torokhov, Geert Uytterhoeven, Michael Turquette On 11/21/2015 03:04 PM, Mark Brown wrote: > On Thu, Nov 19, 2015 at 07:50:45AM +0100, Andrzej Hajda wrote: >> On 11/17/2015 02:55 PM, Mark Brown wrote: >>> This is going to be really common but I'm not sure I see a problem with >>> it in terms of what Raphael is proposing - could you go into more detail >>> on the problem you see here? >> If clock provider is not a device driver and it depends on clocks of >> another clock >> provider you cannot 'translate' this dependency as dependency between >> devices, > What makes you say that this is the case? There should be nothing > stopping us having dependencies between two devices of the same type. To be clear I described situation that one clock provider uses clock of another clock provider and consumer is not modeled as device. > >> so this RFD does not cover them. >> Additionally if you look into kernel there are many calls in form >> 'clk_get(NULL, name)', >> it suggests that not only clock providers are consumers without >> underlying device driver. > Like I said in my earlier reply: > > | > - many clock providers, irq domains are not provided by devices, > > | That seems like something we can and possibly should change if we want. > > This applies just as much to consumers as to providers. OK, then it is just something to do :) Regards Andrzej ^ permalink raw reply [flat|nested] 55+ messages in thread
* Re: [RFD] Functional dependencies between devices 2015-11-17 13:55 ` Mark Brown 2015-11-19 6:50 ` Andrzej Hajda @ 2015-11-19 13:18 ` Thierry Reding 2015-11-21 13:26 ` Mark Brown 1 sibling, 1 reply; 55+ messages in thread From: Thierry Reding @ 2015-11-19 13:18 UTC (permalink / raw) To: Mark Brown Cc: Andrzej Hajda, Rafael J. Wysocki, Linux PM list, Greg Kroah-Hartman, Linux Kernel Mailing List, Alan Stern, Grant Likely, Rob Herring, Tomeu Vizoso, Dmitry Torokhov, Geert Uytterhoeven, Michael Turquette [-- Attachment #1: Type: text/plain, Size: 1348 bytes --] On Tue, Nov 17, 2015 at 01:55:49PM +0000, Mark Brown wrote: > On Tue, Nov 17, 2015 at 01:49:17PM +0100, Andrzej Hajda wrote: > > On 10/27/2015 04:24 PM, Rafael J. Wysocki wrote: > > > this scenario: > > - many clock providers, irq domains are not provided by devices, > > That seems like something we can and possibly should change if we want. It's not very trivial, unfortunately. I had a crack at that a long time ago, but the problem is that these devices all need to be available very early during boot, at which point devices aren't registered yet. With all the progress on probe deferral and the on-demand probing work this might be less of an issue nowadays, I haven't looked at it for quite a while. But I fully agree that the current state of setting up providers without a device structure is suboptimal precisely because it prevents generic infrastructure like the one Rafael proposed from just working. That said, one technique I've occasionally resorted to is to have some early code, be it one of the OF table things or an initcall, set up a basic environment, typically using global variables (yuck!), but then provide a proper driver that knows how to take these things over when its time comes. That's not a perfect solution, but at least it gives you a proper struct device to work with. Thierry [-- Attachment #2: signature.asc --] [-- Type: application/pgp-signature, Size: 819 bytes --] ^ permalink raw reply [flat|nested] 55+ messages in thread
* Re: [RFD] Functional dependencies between devices 2015-11-19 13:18 ` Thierry Reding @ 2015-11-21 13:26 ` Mark Brown 0 siblings, 0 replies; 55+ messages in thread From: Mark Brown @ 2015-11-21 13:26 UTC (permalink / raw) To: Thierry Reding Cc: Andrzej Hajda, Rafael J. Wysocki, Linux PM list, Greg Kroah-Hartman, Linux Kernel Mailing List, Alan Stern, Grant Likely, Rob Herring, Tomeu Vizoso, Dmitry Torokhov, Geert Uytterhoeven, Michael Turquette [-- Attachment #1: Type: text/plain, Size: 1551 bytes --] On Thu, Nov 19, 2015 at 02:18:59PM +0100, Thierry Reding wrote: > On Tue, Nov 17, 2015 at 01:55:49PM +0000, Mark Brown wrote: > > On Tue, Nov 17, 2015 at 01:49:17PM +0100, Andrzej Hajda wrote: > > > On 10/27/2015 04:24 PM, Rafael J. Wysocki wrote: > > > this scenario: > > > - many clock providers, irq domains are not provided by devices, > > That seems like something we can and possibly should change if we want. > It's not very trivial, unfortunately. I had a crack at that a long time > ago, but the problem is that these devices all need to be available very > early during boot, at which point devices aren't registered yet. With > all the progress on probe deferral and the on-demand probing work this > might be less of an issue nowadays, I haven't looked at it for quite a > while. I believe it's a lot easier to do that now but ICBW. We've started needing to put these things into firmware so that we can refer to them from clients so we pretty much have to deal with it. I've not had to worry about it too much directly myself recently though. > That said, one technique I've occasionally resorted to is to have some > early code, be it one of the OF table things or an initcall, set up a > basic environment, typically using global variables (yuck!), but then > provide a proper driver that knows how to take these things over when > its time comes. That's not a perfect solution, but at least it gives you > a proper struct device to work with. Indeed, that's also how things like the console work. [-- Attachment #2: signature.asc --] [-- Type: application/pgp-signature, Size: 473 bytes --] ^ permalink raw reply [flat|nested] 55+ messages in thread
* Re: [RFD] Functional dependencies between devices 2015-11-17 12:49 ` Andrzej Hajda 2015-11-17 13:55 ` Mark Brown @ 2015-11-17 20:31 ` Alan Stern 2015-11-17 22:47 ` Mark Brown 1 sibling, 1 reply; 55+ messages in thread From: Alan Stern @ 2015-11-17 20:31 UTC (permalink / raw) To: Andrzej Hajda Cc: Rafael J. Wysocki, Linux PM list, Greg Kroah-Hartman, Linux Kernel Mailing List, Grant Likely, Mark Brown, Rob Herring, Tomeu Vizoso, Thierry Reding, Dmitry Torokhov, Geert Uytterhoeven, Michael Turquette On Tue, 17 Nov 2015, Andrzej Hajda wrote: > > and I'd like 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). > > Could you elaborate these distinct use cases. I am curious because I have > proposed resource tracking framework [1] which should solve most of the issues > described here. It was not designed to solve suspend/resume issues, but it could > be easily extended to support it, I suppose. > > [1]: https://lkml.org/lkml/2014/12/10/342 The dependencies Rafael has in mind include the following (and undoubtedly include more): The consumer device requires some resource from a provider device before it can be probed. Resources can be clocks, phys, gpios, and so on. The consumer device can't be at full power unless a provider device is also at full power. > Regarding fundamental things, maybe it is just my impression but parsing private > DT device nodes by kernel core assumes that convention about using resource > specifiers in DT is a strict rule, it should not be true. > > As I wrote before I have send some early RFC with framework which solves most of > the problems described here[1], the missing part is suspend/resume support which > should be quite easy to add, I suspect. Moreover it solves problem of device > driver hot bind/unbind. > Could you take a look at it, I will be glad to know it is worth to continue work > on it? It looks like the major difference is that you propose to use callbacks or notifications to do a lot of the work. For example, a driver probing a device could register a bunch of dependencies on various resources, and a callback would be invoked when all those dependencies were satisfied, at which point the probe procedure could complete. No need for deferrals. It's an interesting idea. I'm not sure how well it applies to power dependencies, though. Also, there's a real problem that needs to be solved concerning how resources are identified in the absence of DT (or ACPI or something similar). Alan Stern ^ permalink raw reply [flat|nested] 55+ messages in thread
* Re: [RFD] Functional dependencies between devices 2015-11-17 20:31 ` Alan Stern @ 2015-11-17 22:47 ` Mark Brown 0 siblings, 0 replies; 55+ messages in thread From: Mark Brown @ 2015-11-17 22:47 UTC (permalink / raw) To: Alan Stern Cc: Andrzej Hajda, Rafael J. Wysocki, Linux PM list, Greg Kroah-Hartman, Linux Kernel Mailing List, Grant Likely, Rob Herring, Tomeu Vizoso, Thierry Reding, Dmitry Torokhov, Geert Uytterhoeven, Michael Turquette [-- Attachment #1: Type: text/plain, Size: 480 bytes --] On Tue, Nov 17, 2015 at 03:31:29PM -0500, Alan Stern wrote: > Also, there's a real problem that needs to be solved concerning how > resources are identified in the absence of DT (or ACPI or something > similar). So long as we can add new dependencies at probe time we should always be able to figure things out when the driver figures out it needs the resources. Realistically we'll always have some information - if we don't have firmware we'll have a board file. [-- Attachment #2: signature.asc --] [-- Type: application/pgp-signature, Size: 473 bytes --] ^ permalink raw reply [flat|nested] 55+ messages in thread
* [RFC][PATCH 0/5] Functional dependencies between devices 2015-10-27 15:24 [RFD] Functional dependencies between devices Rafael J. Wysocki ` (6 preceding siblings ...) 2015-11-17 12:49 ` Andrzej Hajda @ 2016-01-14 1:52 ` Rafael J. Wysocki 2016-01-14 1:53 ` [RFC][PATCH 1/5] driver core: Add a wrapper around __device_release_driver() Rafael J. Wysocki ` (6 more replies) 7 siblings, 7 replies; 55+ messages in thread From: Rafael J. Wysocki @ 2016-01-14 1:52 UTC (permalink / raw) To: Linux PM list Cc: Greg Kroah-Hartman, Linux Kernel Mailing List, Alan Stern, Grant Likely, Mark Brown, Rob Herring, Tomeu Vizoso, Thierry Reding, Dmitry Torokhov, Geert Uytterhoeven, Michael Turquette On Tuesday, October 27, 2015 04:24:14 PM Rafael J. Wysocki wrote: > Hi All, > > As discussed in the recent "On-demand device probing" thread and in a Kernel > Summit session earlier today, 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). > > So I want to be able to represent those functional dependencies between devices > and I'd like 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). > > My idea is to represent a supplier-consumer dependency between devices (or > more precisely between device+driver combos) as a "link" object containing > pointers to the devices in question, a list node for each of them and some > additional information related to the management of those objects, ie. > something like: > > struct device_link { > struct device *supplier; > struct list_head supplier_node; > struct device *consumer; > struct list_head consumer_node; > <flags, status etc> > }; > > In general, there will be two lists of those things per device, one list > of links to consumers and one list of links to suppliers. > > In that picture, links will be created by calling, say: > > int device_add_link(struct device *me, struct device *my_supplier, unsigned int flags); > > and they will be deleted by the driver core when not needed any more. The > creation of a link should also cause dpm_list and the list used during shutdown > to be reordered if needed. > > In principle, it seems usefult to consider two types of links, one created > at device registration time (when registering the second device from the linked > pair, whichever it is) and one created at probe time (of the consumer device). > I'll refer to them as "permanent" and "probe-time" links, respectively. > > The permanent links (created at device registration time) will stay around > until one of the linked devices is unregistered (at which time the driver > core will drop the link along with the device going away). The probe-time > ones will be dropped (automatically) at the consumer device driver unbind time. > > There's a question about what if the supplier device is being unbound before > the consumer one (for example, as a result of a hotplug event). My current > view on that is that the consumer needs to be force-unbound in that case too, > but I guess I may be persuaded otherwise given sufficiently convincing > arguments. Anyway, there are reasons to do that, like for example it may > help with the synchronization. Namely, if there's a rule that suppliers > cannot be unbound before any consumers linked to them, than the list of links > to suppliers for a consumer can only change at its registration/probe or > unbind/remove times (which simplifies things quite a bit). > > With that, the permanent links existing at the probe time for a consumer > device can be used to check whether or not to defer the probing of it > even before executing its probe callback. In turn, system suspend > synchronization should be a matter of calling device_pm_wait_for_dev() > for all consumers of a supplier device, in analogy with dpm_wait_for_children(), > and so on. > > Of course, the new lists have to be stable during those operations and ensuring > that is going to be somewhat tricky (AFAICS right now at least), but apart from > that the whole concept looks reasonably straightforward to me. > What follows is my prototype implementation of this. It took some time to develop (much more than I was hoping for), but here it goes at last. The first patch rearranges the code around __device_release_driver() a bit to prepare it for the next one. The second patch introduces the actual device links mechanics, but without system suspend/resume and runtime PM support which are added by the subsequent patches. This hasn't been really tested yet (apart from checking that it doesn't break things when device links are not in used, which would be rather embarrassing), but at this time I'd really like you to have a look and tell me what you think (especially if you see a reason why this is not going to work). Thanks, Rafael ^ permalink raw reply [flat|nested] 55+ messages in thread
* [RFC][PATCH 1/5] driver core: Add a wrapper around __device_release_driver() 2016-01-14 1:52 ` [RFC][PATCH 0/5] " Rafael J. Wysocki @ 2016-01-14 1:53 ` Rafael J. Wysocki 2016-01-14 1:54 ` [RFC][PATCH 2/5] driver core: Functional dependencies tracking support Rafael J. Wysocki ` (5 subsequent siblings) 6 siblings, 0 replies; 55+ messages in thread From: Rafael J. Wysocki @ 2016-01-14 1:53 UTC (permalink / raw) To: Linux PM list Cc: Greg Kroah-Hartman, Linux Kernel Mailing List, Alan Stern, Grant Likely, Mark Brown, Rob Herring, Tomeu Vizoso, Thierry Reding, Dmitry Torokhov, Geert Uytterhoeven, Michael Turquette From: Rafael J. Wysocki <rafael.j.wysocki@intel.com> Add an internal wrapper around __device_release_driver() that will acquire device locks and do the necessary checks before calling it. The next patch will make use of it. Signed-off-by: Rafael J. Wysocki <rafael.j.wysocki@intel.com> --- drivers/base/dd.c | 30 ++++++++++++++++++------------ 1 file changed, 18 insertions(+), 12 deletions(-) Index: linux-pm/drivers/base/dd.c =================================================================== --- linux-pm.orig/drivers/base/dd.c +++ linux-pm/drivers/base/dd.c @@ -776,6 +776,22 @@ static void __device_release_driver(stru } } +static void device_release_driver_internal(struct device *dev, + struct device_driver *drv, + struct device *parent) +{ + if (parent) + device_lock(parent); + + device_lock(dev); + if (!drv || drv == dev->driver) + __device_release_driver(dev); + + device_unlock(dev); + if (parent) + device_unlock(parent); +} + /** * device_release_driver - manually detach device from driver. * @dev: device. @@ -790,9 +806,7 @@ void device_release_driver(struct device * within their ->remove callback for the same device, they * will deadlock right here. */ - device_lock(dev); - __device_release_driver(dev); - device_unlock(dev); + device_release_driver_internal(dev, NULL, NULL); } EXPORT_SYMBOL_GPL(device_release_driver); @@ -817,15 +831,7 @@ void driver_detach(struct device_driver dev = dev_prv->device; get_device(dev); spin_unlock(&drv->p->klist_devices.k_lock); - - if (dev->parent) /* Needed for USB */ - device_lock(dev->parent); - device_lock(dev); - if (dev->driver == drv) - __device_release_driver(dev); - device_unlock(dev); - if (dev->parent) - device_unlock(dev->parent); + device_release_driver_internal(dev, drv, dev->parent); put_device(dev); } } ^ permalink raw reply [flat|nested] 55+ messages in thread
* [RFC][PATCH 2/5] driver core: Functional dependencies tracking support 2016-01-14 1:52 ` [RFC][PATCH 0/5] " Rafael J. Wysocki 2016-01-14 1:53 ` [RFC][PATCH 1/5] driver core: Add a wrapper around __device_release_driver() Rafael J. Wysocki @ 2016-01-14 1:54 ` Rafael J. Wysocki 2016-06-08 12:48 ` Mark Brown 2016-01-14 1:55 ` [RFC][PATCH 3/5] PM core: Make async suspend/resume of devices use device links Rafael J. Wysocki ` (4 subsequent siblings) 6 siblings, 1 reply; 55+ messages in thread From: Rafael J. Wysocki @ 2016-01-14 1:54 UTC (permalink / raw) To: Linux PM list Cc: Greg Kroah-Hartman, Linux Kernel Mailing List, Alan Stern, Grant Likely, Mark Brown, Rob Herring, Tomeu Vizoso, Thierry Reding, Dmitry Torokhov, Geert Uytterhoeven, Michael Turquette From: Rafael J. Wysocki <rafael.j.wysocki@intel.com> 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 <rafael.j.wysocki@intel.com> --- 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(-) Index: linux-pm/include/linux/device.h =================================================================== --- linux-pm.orig/include/linux/device.h +++ linux-pm/include/linux/device.h @@ -691,6 +691,34 @@ struct device_dma_parameters { unsigned long segment_boundary_mask; }; +enum devlink_status { + DEVICE_LINK_DORMANT = 0, /* Link not in use. */ + DEVICE_LINK_AVAILABLE, /* Supplier driver is present. */ + DEVICE_LINK_ACTIVE, /* Consumer driver is present too. */ + DEVICE_LINK_CONSUMER_PROBE, /* Consumer is probing. */ + DEVICE_LINK_SUPPLIER_UNBIND, /* Supplier is unbinding. */ +}; + +/* + * Device link flags. + * + * PERSISTENT: Do not delete the link on consumer device driver unbind. + * PROBE_TIME: Assume supplier device functional when creating the link. + */ +#define DEVICE_LINK_PERSISTENT (1 << 0) +#define DEVICE_LINK_PROBE_TIME (1 << 1) + +struct devlink { + struct device *supplier; + struct list_head s_node; + struct device *consumer; + struct list_head c_node; + enum devlink_status status; + u32 flags; + spinlock_t lock; + struct rcu_head rcu_head; +}; + /** * struct device - The basic device structure * @parent: The device's "parent" device, the device to which it is attached. @@ -716,6 +744,8 @@ struct device_dma_parameters { * on. This shrinks the "Board Support Packages" (BSPs) and * minimizes board-specific #ifdefs in drivers. * @driver_data: Private pointer for driver specific info. + * @supplier_links: Links to consumer devices. + * @consumer_links: Links to supplier devices. * @power: For device power management. * See Documentation/power/devices.txt for details. * @pm_domain: Provide callbacks that are executed during system suspend, @@ -782,6 +812,8 @@ struct device { core doesn't touch it */ void *driver_data; /* Driver data, set and get with dev_set/get_drvdata */ + struct list_head supplier_links; + struct list_head consumer_links; struct dev_pm_info power; struct dev_pm_domain *pm_domain; @@ -1098,6 +1130,10 @@ extern void device_shutdown(void); /* debugging and troubleshooting/diagnostic helpers. */ extern const char *dev_driver_string(const struct device *dev); +/* Device links interface. */ +struct devlink *device_link_add(struct device *consumer, + struct device *supplier, u32 flags); +void device_link_del(struct devlink *link); #ifdef CONFIG_PRINTK Index: linux-pm/drivers/base/core.c =================================================================== --- linux-pm.orig/drivers/base/core.c +++ linux-pm/drivers/base/core.c @@ -44,6 +44,367 @@ static int __init sysfs_deprecated_setup early_param("sysfs.deprecated", sysfs_deprecated_setup); #endif +/* Device links support. */ + +DEFINE_STATIC_SRCU(device_links_srcu); +static DEFINE_MUTEX(device_links_lock); + +static int device_reorder_to_tail(struct device *dev, void *not_used) +{ + struct devlink *link; + + devices_kset_move_last(dev); + device_pm_move_last(dev); + device_for_each_child(dev, NULL, device_reorder_to_tail); + list_for_each_entry(link, &dev->consumer_links, c_node) + device_reorder_to_tail(link->consumer, NULL); + + return 0; +} + +/** + * device_link_add - Create a link between two devices. + * @consumer: Consumer end of the link. + * @supplier: Supplier end of the link. + * @flags: Link flags. + * + * At least one of the flags must be set. If DEVICE_LINK_PROBE_TIME is set, the + * caller is expected to know that (a) the supplier device is present and active + * (ie. its driver is functional) and (b) the consumer device is probing at the + * moment and therefore the initial state of the link will be "consumer probe" + * in that case. If DEVICE_LINK_PROBE_TIME is not set, DEVICE_LINK_PERSISTENT + * must be set (meaning that the link will not go away when the consumer driver + * goes away). + * + * A side effect of the link creation is re-ordering of dpm_list and the + * devices_kset list by moving the consumer device and all devices depending + * on it to the ends of those lists. + */ +struct devlink *device_link_add(struct device *consumer, + struct device *supplier, u32 flags) +{ + struct devlink *link; + + if (!consumer || !supplier || !flags) + return NULL; + + mutex_lock(&device_links_lock); + + list_for_each_entry(link, &supplier->supplier_links, s_node) + if (link->consumer == consumer) + goto out; + + link = kmalloc(sizeof(*link), GFP_KERNEL); + if (!link) + goto out; + + get_device(supplier); + link->supplier = supplier; + INIT_LIST_HEAD(&link->s_node); + get_device(consumer); + link->consumer = consumer; + INIT_LIST_HEAD(&link->c_node); + link->flags = flags; + link->status = (flags & DEVICE_LINK_PROBE_TIME) ? + DEVICE_LINK_CONSUMER_PROBE : DEVICE_LINK_DORMANT; + spin_lock_init(&link->lock); + + /* + * Move the consumer and all of the devices depending on it to the end + * of dpm_list and the devices_kset list. + * + * We have to hold dpm_list locked throughout all that or else we may + * end up suspending with a wrong ordering of it. + */ + device_pm_lock(); + device_reorder_to_tail(consumer, NULL); + device_pm_unlock(); + + list_add_tail_rcu(&link->s_node, &supplier->supplier_links); + list_add_tail_rcu(&link->c_node, &consumer->consumer_links); + + dev_info(consumer, "Linked as a consumer to %s\n", dev_name(supplier)); + + out: + mutex_unlock(&device_links_lock); + return link; +} +EXPORT_SYMBOL_GPL(device_link_add); + +static void __devlink_free_srcu(struct rcu_head *rhead) +{ + struct devlink *link; + + link = container_of(rhead, struct devlink, rcu_head); + put_device(link->consumer); + put_device(link->supplier); + kfree(link); +} + +static void devlink_del(struct devlink *link) +{ + dev_info(link->consumer, "Dropping the link to %s\n", + dev_name(link->supplier)); + + list_del_rcu(&link->s_node); + list_del_rcu(&link->c_node); + call_srcu(&device_links_srcu, &link->rcu_head, __devlink_free_srcu); +} + +/** + * device_link_del - Delete a link between two devices. + * @link: Device link to delete. + */ +void device_link_del(struct devlink *link) +{ + mutex_lock(&device_links_lock); + devlink_del(link); + mutex_unlock(&device_links_lock); +} +EXPORT_SYMBOL_GPL(device_link_del); + +static int device_links_read_lock(void) +{ + return srcu_read_lock(&device_links_srcu); +} + +static void device_links_read_unlock(int idx) +{ + return srcu_read_unlock(&device_links_srcu, idx); +} + +static void device_links_missing_supplier(struct device *dev) +{ + struct devlink *link; + + list_for_each_entry_rcu(link, &dev->consumer_links, c_node) { + spin_lock(&link->lock); + + if (link->status == DEVICE_LINK_CONSUMER_PROBE) + link->status = DEVICE_LINK_AVAILABLE; + + spin_unlock(&link->lock); + } +} + +/** + * device_links_check_suppliers - Check supplier devices for this one. + * @dev: Consumer device. + * + * Check links from this device to any suppliers. Walk the list of the device's + * consumer links and see if all of the suppliers are available. If not, simply + * return -EPROBE_DEFER. + * + * Walk the list under SRCU and check each link's status field under its lock. + * + * We need to guarantee that the supplier will not go away after the check has + * been positive here. It only can go away in __device_release_driver() and + * that function checks the device's links to consumers. This means we need to + * mark the link as "consumer probe in progress" to make the supplier removal + * wait for us to complete (or bad things may happen). + */ +int device_links_check_suppliers(struct device *dev) +{ + struct devlink *link; + int idx, ret = 0; + + idx = device_links_read_lock(); + + list_for_each_entry_rcu(link, &dev->consumer_links, c_node) { + spin_lock(&link->lock); + if (link->status != DEVICE_LINK_AVAILABLE) { + spin_unlock(&link->lock); + device_links_missing_supplier(dev); + ret = -EPROBE_DEFER; + break; + } + link->status = DEVICE_LINK_CONSUMER_PROBE; + spin_unlock(&link->lock); + } + + device_links_read_unlock(idx); + return ret; +} + +/** + * device_links_driver_bound - Update device links after probing its driver. + * @dev: Device to update the links for. + * + * The probe has been successful, so update links from this device to any + * consumers by changing their status to "available". + * + * Also change the status of @dev's links to suppliers to "active". + */ +void device_links_driver_bound(struct device *dev) +{ + struct devlink *link; + int idx; + + idx = device_links_read_lock(); + + list_for_each_entry_rcu(link, &dev->supplier_links, s_node) { + spin_lock(&link->lock); + WARN_ON(link->status != DEVICE_LINK_DORMANT); + link->status = DEVICE_LINK_AVAILABLE; + spin_unlock(&link->lock); + } + + list_for_each_entry_rcu(link, &dev->consumer_links, c_node) { + spin_lock(&link->lock); + WARN_ON(link->status != DEVICE_LINK_CONSUMER_PROBE); + link->status = DEVICE_LINK_ACTIVE; + spin_unlock(&link->lock); + } + + device_links_read_unlock(idx); +} + +/** + * device_links_driver_gone - Update links after driver removal. + * @dev: Device whose driver has gone away. + * + * Update links to consumers for @dev by changing their status to "dormant". + */ +void device_links_driver_gone(struct device *dev) +{ + struct devlink *link; + int idx; + + idx = device_links_read_lock(); + + list_for_each_entry_rcu(link, &dev->supplier_links, s_node) { + WARN_ON(!(link->flags & DEVICE_LINK_PERSISTENT)); + spin_lock(&link->lock); + WARN_ON(link->status != DEVICE_LINK_SUPPLIER_UNBIND); + link->status = DEVICE_LINK_DORMANT; + spin_unlock(&link->lock); + } + + device_links_read_unlock(idx); +} + +/** + * device_links_no_driver - Update links of a device without a driver. + * @dev: Device without a drvier. + * + * Delete all non-persistent links from this device to any suppliers. + * Persistent links stay around, but their status is changed to "available", + * unless they already are in the "supplier unbind in progress" state in which + * case they need not be updated. + */ +void device_links_no_driver(struct device *dev) +{ + struct devlink *link, *ln; + + mutex_lock(&device_links_lock); + + list_for_each_entry_safe_reverse(link, ln, &dev->consumer_links, c_node) + if (link->flags & DEVICE_LINK_PERSISTENT) { + spin_lock(&link->lock); + + if (link->status != DEVICE_LINK_SUPPLIER_UNBIND) + link->status = DEVICE_LINK_AVAILABLE; + + spin_unlock(&link->lock); + } else { + devlink_del(link); + } + + mutex_unlock(&device_links_lock); +} + +/** + * device_links_busy - Check if there are any busy links to consumers. + * @dev: Device to check. + * + * Check each consumer of the device and return 'true' it if its link's status + * is one of "consumer probe" or "active" (meaning that the given consumer is + * probing right now or its driver is present). Otherwise, change the link + * state to "supplier unbind" to prevent the consumer from being probed + * successfully going forward. + * + * Return 'false' if there are no probing or active consumers. + */ +bool device_links_busy(struct device *dev) +{ + struct devlink *link; + int idx; + bool ret = false; + + idx = device_links_read_lock(); + + list_for_each_entry_rcu(link, &dev->supplier_links, s_node) { + spin_lock(&link->lock); + if (link->status == DEVICE_LINK_CONSUMER_PROBE + || link->status == DEVICE_LINK_ACTIVE) { + spin_unlock(&link->lock); + ret = true; + break; + } + link->status = DEVICE_LINK_SUPPLIER_UNBIND; + spin_unlock(&link->lock); + } + + device_links_read_unlock(idx); + return ret; +} + +/** + * device_links_unbind_consumers - Force unbind consumers of the given device. + * @dev: Device to unbind the consumers of. + * + * Walk the list of links to consumers for @dev and if any of them is in the + * "consumer probe" state, wait for all device probes in progress to complete + * and start over. + * + * If that's not the case, change the status of the link to "supplier unbind" + * and check if the link was in the "active" state. If so, force the consumer + * driver to unbind and start over (the consumer will not re-probe as we have + * changed the state of the link already). + */ +void device_links_unbind_consumers(struct device *dev) +{ + struct devlink *link; + int idx; + + start: + idx = device_links_read_lock(); + + list_for_each_entry_rcu(link, &dev->supplier_links, s_node) { + enum devlink_status status; + + spin_lock(&link->lock); + status = link->status; + if (status == DEVICE_LINK_CONSUMER_PROBE) { + spin_unlock(&link->lock); + + device_links_read_unlock(idx); + + wait_for_device_probe(); + goto start; + } + link->status = DEVICE_LINK_SUPPLIER_UNBIND; + if (status == DEVICE_LINK_ACTIVE) { + struct device *consumer = link->consumer; + + get_device(consumer); + spin_unlock(&link->lock); + + device_links_read_unlock(idx); + + device_release_driver_internal(consumer, NULL, + consumer->parent); + put_device(consumer); + goto start; + } + spin_unlock(&link->lock); + } + + device_links_read_unlock(idx); +} + +/* Device links support end. */ + int (*platform_notify)(struct device *dev) = NULL; int (*platform_notify_remove)(struct device *dev) = NULL; static struct kobject *dev_kobj; @@ -711,6 +1072,8 @@ void device_initialize(struct device *de #ifdef CONFIG_GENERIC_MSI_IRQ INIT_LIST_HEAD(&dev->msi_list); #endif + INIT_LIST_HEAD(&dev->supplier_links); + INIT_LIST_HEAD(&dev->consumer_links); } EXPORT_SYMBOL_GPL(device_initialize); @@ -1233,6 +1596,7 @@ void device_del(struct device *dev) { struct device *parent = dev->parent; struct class_interface *class_intf; + struct devlink *link, *ln; /* Notify clients of device removal. This call must come * before dpm_sysfs_remove(). @@ -1240,6 +1604,28 @@ void device_del(struct device *dev) if (dev->bus) blocking_notifier_call_chain(&dev->bus->p->bus_notifier, BUS_NOTIFY_DEL_DEVICE, dev); + + /* + * Delete all of the remaining links from this device to any other + * devices (either consumers or suppliers). + * + * This requires that all links be dormant, so warn if that's no the + * case. + */ + mutex_lock(&device_links_lock); + + list_for_each_entry_safe_reverse(link, ln, &dev->consumer_links, c_node) { + WARN_ON(link->status != DEVICE_LINK_DORMANT); + devlink_del(link); + } + + list_for_each_entry_safe_reverse(link, ln, &dev->supplier_links, s_node) { + WARN_ON(link->status != DEVICE_LINK_DORMANT); + devlink_del(link); + } + + mutex_unlock(&device_links_lock); + dpm_sysfs_remove(dev); if (parent) klist_del(&dev->p->knode_parent); Index: linux-pm/drivers/base/dd.c =================================================================== --- linux-pm.orig/drivers/base/dd.c +++ linux-pm/drivers/base/dd.c @@ -249,6 +249,7 @@ static void driver_bound(struct device * __func__, dev_name(dev)); klist_add_tail(&dev->p->knode_driver, &dev->driver->p->klist_devices); + device_links_driver_bound(dev); device_pm_check_callbacks(dev); @@ -399,6 +400,7 @@ probe_failed: blocking_notifier_call_chain(&dev->bus->p->bus_notifier, BUS_NOTIFY_DRIVER_NOT_BOUND, dev); pinctrl_bind_failed: + device_links_no_driver(dev); devres_release_all(dev); driver_sysfs_remove(dev); dev->driver = NULL; @@ -489,6 +491,10 @@ int driver_probe_device(struct device_dr if (!device_is_registered(dev)) return -ENODEV; + ret = device_links_check_suppliers(dev); + if (ret) + return ret; + pr_debug("bus: '%s': %s: matched device %s with driver %s\n", drv->bus->name, __func__, dev_name(dev), drv->name); @@ -736,7 +742,7 @@ EXPORT_SYMBOL_GPL(driver_attach); * __device_release_driver() must be called with @dev lock held. * When called for a USB interface, @dev->parent lock must be held as well. */ -static void __device_release_driver(struct device *dev) +static void __device_release_driver(struct device *dev, struct device *parent) { struct device_driver *drv; @@ -745,6 +751,25 @@ static void __device_release_driver(stru if (driver_allows_async_probing(drv)) async_synchronize_full(); + while (device_links_busy(dev)) { + device_unlock(dev); + if (parent) + device_unlock(parent); + + device_links_unbind_consumers(dev); + if (parent) + device_lock(parent); + + device_lock(dev); + /* + * A concurrent invocation of the same function might + * have released the driver successfully while this one + * was waiting, so check for that. + */ + if (dev->driver != drv) + return; + } + pm_runtime_get_sync(dev); driver_sysfs_remove(dev); @@ -760,6 +785,9 @@ static void __device_release_driver(stru dev->bus->remove(dev); else if (drv->remove) drv->remove(dev); + + device_links_driver_gone(dev); + device_links_no_driver(dev); devres_release_all(dev); dev->driver = NULL; dev_set_drvdata(dev, NULL); @@ -776,16 +804,16 @@ static void __device_release_driver(stru } } -static void device_release_driver_internal(struct device *dev, - struct device_driver *drv, - struct device *parent) +void device_release_driver_internal(struct device *dev, + struct device_driver *drv, + struct device *parent) { if (parent) device_lock(parent); device_lock(dev); if (!drv || drv == dev->driver) - __device_release_driver(dev); + __device_release_driver(dev, parent); device_unlock(dev); if (parent) @@ -798,6 +826,10 @@ static void device_release_driver_intern * * Manually detach device from driver. * When called for a USB interface, @dev->parent lock must be held. + * + * If this function is to be called with @dev->parent lock held, ensure that + * the device's consumers are unbound in advance or that their locks can be + * acquired under the @dev->parent lock. */ void device_release_driver(struct device *dev) { Index: linux-pm/drivers/base/base.h =================================================================== --- linux-pm.orig/drivers/base/base.h +++ linux-pm/drivers/base/base.h @@ -107,6 +107,9 @@ extern void bus_remove_device(struct 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); ^ permalink raw reply [flat|nested] 55+ messages in thread
* Re: [RFC][PATCH 2/5] driver core: Functional dependencies tracking support 2016-01-14 1:54 ` [RFC][PATCH 2/5] driver core: Functional dependencies tracking support Rafael J. Wysocki @ 2016-06-08 12:48 ` Mark Brown 2016-06-08 18:12 ` Rafael J. Wysocki 0 siblings, 1 reply; 55+ messages in thread From: Mark Brown @ 2016-06-08 12:48 UTC (permalink / raw) To: Rafael J. Wysocki Cc: Linux PM list, Greg Kroah-Hartman, Linux Kernel Mailing List, Alan Stern, Grant Likely, Mark Brown, Rob Herring, Tomeu Vizoso, Thierry Reding, Dmitry Torokhov, Geert Uytterhoeven, Michael Turquette [-- Attachment #1: Type: text/plain, Size: 515 bytes --] On Thu, Jan 14, 2016 at 02:54:17AM +0100, Rafael J. Wysocki wrote: > + * A side effect of the link creation is re-ordering of dpm_list and the > + * devices_kset list by moving the consumer device and all devices depending > + * on it to the ends of those lists. How does this work in the scenario where a device instantiates a child device then uses services that child provides to complete the initializiation? We do have that scenario currently for on chip regulators to allow external regulators to be used. [-- Attachment #2: signature.asc --] [-- Type: application/pgp-signature, Size: 473 bytes --] ^ permalink raw reply [flat|nested] 55+ messages in thread
* Re: [RFC][PATCH 2/5] driver core: Functional dependencies tracking support 2016-06-08 12:48 ` Mark Brown @ 2016-06-08 18:12 ` Rafael J. Wysocki 2016-06-08 18:35 ` Mark Brown 0 siblings, 1 reply; 55+ messages in thread From: Rafael J. Wysocki @ 2016-06-08 18:12 UTC (permalink / raw) To: Mark Brown Cc: Rafael J. Wysocki, Linux PM list, Greg Kroah-Hartman, Linux Kernel Mailing List, Alan Stern, Grant Likely, Mark Brown, Rob Herring, Tomeu Vizoso, Thierry Reding, Dmitry Torokhov, Geert Uytterhoeven, Michael Turquette On Wed, Jun 8, 2016 at 2:48 PM, Mark Brown <broonie@kernel.org> wrote: > On Thu, Jan 14, 2016 at 02:54:17AM +0100, Rafael J. Wysocki wrote: > >> + * A side effect of the link creation is re-ordering of dpm_list and the >> + * devices_kset list by moving the consumer device and all devices depending >> + * on it to the ends of those lists. > > How does this work in the scenario where a device instantiates a child > device then uses services that child provides to complete the > initializiation? We do have that scenario currently for on chip > regulators to allow external regulators to be used. I'm not sure I understand the question correctly, but it that is the parent and a child, we don't need an extra link entity to represent that dependency, as parent-child dependencies are taken by the current code into account already. This series was supposed to help with dependencies that aren't of the parent-child type. ^ permalink raw reply [flat|nested] 55+ messages in thread
* Re: [RFC][PATCH 2/5] driver core: Functional dependencies tracking support 2016-06-08 18:12 ` Rafael J. Wysocki @ 2016-06-08 18:35 ` Mark Brown 2016-06-08 20:48 ` Rafael J. Wysocki 0 siblings, 1 reply; 55+ messages in thread From: Mark Brown @ 2016-06-08 18:35 UTC (permalink / raw) To: Rafael J. Wysocki Cc: Rafael J. Wysocki, Linux PM list, Greg Kroah-Hartman, Linux Kernel Mailing List, Alan Stern, Grant Likely, Mark Brown, Rob Herring, Tomeu Vizoso, Thierry Reding, Dmitry Torokhov, Geert Uytterhoeven, Michael Turquette [-- Attachment #1: Type: text/plain, Size: 1418 bytes --] On Wed, Jun 08, 2016 at 08:12:34PM +0200, Rafael J. Wysocki wrote: > On Wed, Jun 8, 2016 at 2:48 PM, Mark Brown <broonie@kernel.org> wrote: > > On Thu, Jan 14, 2016 at 02:54:17AM +0100, Rafael J. Wysocki wrote: > >> + * A side effect of the link creation is re-ordering of dpm_list and the > >> + * devices_kset list by moving the consumer device and all devices depending > >> + * on it to the ends of those lists. > > How does this work in the scenario where a device instantiates a child > > device then uses services that child provides to complete the > > initializiation? We do have that scenario currently for on chip > > regulators to allow external regulators to be used. > I'm not sure I understand the question correctly, but it that is the > parent and a child, we don't need an extra link entity to represent > that dependency, as parent-child dependencies are taken by the current > code into account already. A parent (especially a MFD) may create a child to fulfil a role that could also be filled by a non-parent device, if that happens then depending on what ends up creating these links (it's not 100% clear from this series) it's likely that the link creation will also end up registering a dependency unless we do something special. Now I think about it this does depend on how we create the children though, it's less likely to be an issue if the dependencies are created by firmware code. [-- Attachment #2: signature.asc --] [-- Type: application/pgp-signature, Size: 473 bytes --] ^ permalink raw reply [flat|nested] 55+ messages in thread
* Re: [RFC][PATCH 2/5] driver core: Functional dependencies tracking support 2016-06-08 18:35 ` Mark Brown @ 2016-06-08 20:48 ` Rafael J. Wysocki 2016-06-08 22:24 ` Mark Brown 0 siblings, 1 reply; 55+ messages in thread From: Rafael J. Wysocki @ 2016-06-08 20:48 UTC (permalink / raw) To: Mark Brown Cc: Rafael J. Wysocki, Linux PM list, Greg Kroah-Hartman, Linux Kernel Mailing List, Alan Stern, Grant Likely, Rob Herring, Tomeu Vizoso, Thierry Reding, Dmitry Torokhov, Geert Uytterhoeven, Michael Turquette On Wed, Jun 8, 2016 at 8:35 PM, Mark Brown <broonie@kernel.org> wrote: > On Wed, Jun 08, 2016 at 08:12:34PM +0200, Rafael J. Wysocki wrote: >> On Wed, Jun 8, 2016 at 2:48 PM, Mark Brown <broonie@kernel.org> wrote: >> > On Thu, Jan 14, 2016 at 02:54:17AM +0100, Rafael J. Wysocki wrote: > >> >> + * A side effect of the link creation is re-ordering of dpm_list and the >> >> + * devices_kset list by moving the consumer device and all devices depending >> >> + * on it to the ends of those lists. > >> > How does this work in the scenario where a device instantiates a child >> > device then uses services that child provides to complete the >> > initializiation? We do have that scenario currently for on chip >> > regulators to allow external regulators to be used. > >> I'm not sure I understand the question correctly, but it that is the >> parent and a child, we don't need an extra link entity to represent >> that dependency, as parent-child dependencies are taken by the current >> code into account already. > > A parent (especially a MFD) may create a child to fulfil a role that > could also be filled by a non-parent device, if that happens then > depending on what ends up creating these links (it's not 100% clear from > this series) it's likely that the link creation will also end up > registering a dependency unless we do something special. Now I think > about it this does depend on how we create the children though, it's > less likely to be an issue if the dependencies are created by firmware > code. If that's a non-child device, I would expect it to exist beforehand and then the link creation would only enforce the specific ordering of certain operations (like runtime PM or async suspend/resume) between the two devices linked by it. That shouldn't be problematic in principle. Let me redo the series, though (that may take a couple of days realistically). ^ permalink raw reply [flat|nested] 55+ messages in thread
* Re: [RFC][PATCH 2/5] driver core: Functional dependencies tracking support 2016-06-08 20:48 ` Rafael J. Wysocki @ 2016-06-08 22:24 ` Mark Brown 0 siblings, 0 replies; 55+ messages in thread From: Mark Brown @ 2016-06-08 22:24 UTC (permalink / raw) To: Rafael J. Wysocki Cc: Rafael J. Wysocki, Linux PM list, Greg Kroah-Hartman, Linux Kernel Mailing List, Alan Stern, Grant Likely, Rob Herring, Tomeu Vizoso, Thierry Reding, Dmitry Torokhov, Geert Uytterhoeven, Michael Turquette [-- Attachment #1: Type: text/plain, Size: 760 bytes --] On Wed, Jun 08, 2016 at 10:48:23PM +0200, Rafael J. Wysocki wrote: > On Wed, Jun 8, 2016 at 8:35 PM, Mark Brown <broonie@kernel.org> wrote: > > A parent (especially a MFD) may create a child to fulfil a role that > > could also be filled by a non-parent device, if that happens then > > depending on what ends up creating these links (it's not 100% clear from > > this series) it's likely that the link creation will also end up > > registering a dependency unless we do something special. Now I think > > about it this does depend on how we create the children though, it's > > less likely to be an issue if the dependencies are created by firmware > > code. > If that's a non-child device, I would expect it to exist beforehand No, it's an actual child. [-- Attachment #2: signature.asc --] [-- Type: application/pgp-signature, Size: 473 bytes --] ^ permalink raw reply [flat|nested] 55+ messages in thread
* [RFC][PATCH 3/5] PM core: Make async suspend/resume of devices use device links 2016-01-14 1:52 ` [RFC][PATCH 0/5] " Rafael J. Wysocki 2016-01-14 1:53 ` [RFC][PATCH 1/5] driver core: Add a wrapper around __device_release_driver() Rafael J. Wysocki 2016-01-14 1:54 ` [RFC][PATCH 2/5] driver core: Functional dependencies tracking support Rafael J. Wysocki @ 2016-01-14 1:55 ` Rafael J. Wysocki 2016-06-08 12:59 ` Mark Brown 2016-01-14 1:56 ` [RFC][PATCH 4/5] PM core: Make runtime PM " Rafael J. Wysocki ` (3 subsequent siblings) 6 siblings, 1 reply; 55+ messages in thread From: Rafael J. Wysocki @ 2016-01-14 1:55 UTC (permalink / raw) To: Linux PM list Cc: Greg Kroah-Hartman, Linux Kernel Mailing List, Alan Stern, Grant Likely, Mark Brown, Rob Herring, Tomeu Vizoso, Thierry Reding, Dmitry Torokhov, Geert Uytterhoeven, Michael Turquette From: Rafael J. Wysocki <rafael.j.wysocki@intel.com> Make the device suspend/resume part of the core system suspend/resume code use device links to ensure that supplier and consumer devices will be suspended and resumed in the right order in case of async suspend/resume. The idea, roughly, is to use dpm_wait() to wait for all consumers before a supplier device suspend and to wait for all suppliers before a consumer device resume. Signed-off-by: Rafael J. Wysocki <rafael.j.wysocki@intel.com> --- drivers/base/base.h | 2 + drivers/base/core.c | 4 +- drivers/base/power/main.c | 68 +++++++++++++++++++++++++++++++++++++++++----- 3 files changed, 66 insertions(+), 8 deletions(-) Index: linux-pm/drivers/base/base.h =================================================================== --- linux-pm.orig/drivers/base/base.h +++ linux-pm/drivers/base/base.h @@ -163,3 +163,5 @@ extern void device_links_driver_gone(str 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); +extern int device_links_read_lock(void); +extern void device_links_read_unlock(int idx); Index: linux-pm/drivers/base/core.c =================================================================== --- linux-pm.orig/drivers/base/core.c +++ linux-pm/drivers/base/core.c @@ -163,12 +163,12 @@ void device_link_del(struct devlink *lin } EXPORT_SYMBOL_GPL(device_link_del); -static int device_links_read_lock(void) +int device_links_read_lock(void) { return srcu_read_lock(&device_links_srcu); } -static void device_links_read_unlock(int idx) +void device_links_read_unlock(int idx) { return srcu_read_unlock(&device_links_srcu, idx); } Index: linux-pm/drivers/base/power/main.c =================================================================== --- linux-pm.orig/drivers/base/power/main.c +++ linux-pm/drivers/base/power/main.c @@ -244,6 +244,62 @@ static void dpm_wait_for_children(struct device_for_each_child(dev, &async, dpm_wait_fn); } +static void dpm_wait_for_suppliers(struct device *dev, bool async) +{ + struct devlink *link; + int idx; + + idx = device_links_read_lock(); + + /* + * If the supplier goes away right after we've checked the link to it, + * we'll wait for its completion to change the state, but that's fine, + * because the only things that will block as a result are the SRCU + * callbacks freeing the link objects for the links in the list we're + * walking. + */ + list_for_each_entry_rcu(link, &dev->consumer_links, c_node) + if (link->status != DEVICE_LINK_DORMANT) + dpm_wait(link->supplier, async); + + device_links_read_unlock(idx); +} + +static void dpm_wait_for_superior(struct device *dev, bool async) +{ + dpm_wait(dev->parent, async); + dpm_wait_for_suppliers(dev, async); +} + +static void dpm_wait_for_consumers(struct device *dev, bool async) +{ + struct devlink *link; + int idx; + + idx = device_links_read_lock(); + + /* + * The status of a device link can only be changed from "dormant" by a + * probe, but that cannot happen during system suspend/resume. In + * theory it can change to "dormant" at that time, but then it is + * reasonable to wait for the target device anyway (eg. if it goes + * away, it's better to wait for it to go away completely and then + * continue instead of trying to continue in parallel with its + * unregistration). + */ + list_for_each_entry_rcu(link, &dev->supplier_links, s_node) + if (link->status != DEVICE_LINK_DORMANT) + dpm_wait(link->consumer, async); + + device_links_read_unlock(idx); +} + +static void dpm_wait_for_subordinate(struct device *dev, bool async) +{ + dpm_wait_for_children(dev, async); + dpm_wait_for_consumers(dev, async); +} + /** * pm_op - Return the PM operation appropriate for given PM event. * @ops: PM operations to choose from. @@ -488,7 +544,7 @@ static int device_resume_noirq(struct de if (!dev->power.is_noirq_suspended) goto Out; - dpm_wait(dev->parent, async); + dpm_wait_for_superior(dev, async); if (dev->pm_domain) { info = "noirq power domain "; @@ -618,7 +674,7 @@ static int device_resume_early(struct de if (!dev->power.is_late_suspended) goto Out; - dpm_wait(dev->parent, async); + dpm_wait_for_superior(dev, async); if (dev->pm_domain) { info = "early power domain "; @@ -750,7 +806,7 @@ static int device_resume(struct device * goto Complete; } - dpm_wait(dev->parent, async); + dpm_wait_for_superior(dev, async); dpm_watchdog_set(&wd, dev); device_lock(dev); @@ -1038,7 +1094,7 @@ static int __device_suspend_noirq(struct if (dev->power.syscore || dev->power.direct_complete) goto Complete; - dpm_wait_for_children(dev, async); + dpm_wait_for_subordinate(dev, async); if (dev->pm_domain) { info = "noirq power domain "; @@ -1185,7 +1241,7 @@ static int __device_suspend_late(struct if (dev->power.syscore || dev->power.direct_complete) goto Complete; - dpm_wait_for_children(dev, async); + dpm_wait_for_subordinate(dev, async); if (dev->pm_domain) { info = "late power domain "; @@ -1357,7 +1413,7 @@ static int __device_suspend(struct devic TRACE_DEVICE(dev); TRACE_SUSPEND(0); - dpm_wait_for_children(dev, async); + dpm_wait_for_subordinate(dev, async); if (async_error) goto Complete; ^ permalink raw reply [flat|nested] 55+ messages in thread
* Re: [RFC][PATCH 3/5] PM core: Make async suspend/resume of devices use device links 2016-01-14 1:55 ` [RFC][PATCH 3/5] PM core: Make async suspend/resume of devices use device links Rafael J. Wysocki @ 2016-06-08 12:59 ` Mark Brown 0 siblings, 0 replies; 55+ messages in thread From: Mark Brown @ 2016-06-08 12:59 UTC (permalink / raw) To: Rafael J. Wysocki Cc: Linux PM list, Greg Kroah-Hartman, Linux Kernel Mailing List, Alan Stern, Grant Likely, Mark Brown, Rob Herring, Tomeu Vizoso, Thierry Reding, Dmitry Torokhov, Geert Uytterhoeven, Michael Turquette [-- Attachment #1: Type: text/plain, Size: 723 bytes --] On Thu, Jan 14, 2016 at 02:55:25AM +0100, Rafael J. Wysocki wrote: > From: Rafael J. Wysocki <rafael.j.wysocki@intel.com> > > Make the device suspend/resume part of the core system > suspend/resume code use device links to ensure that supplier > and consumer devices will be suspended and resumed in the right > order in case of async suspend/resume. > > The idea, roughly, is to use dpm_wait() to wait for all consumers > before a supplier device suspend and to wait for all suppliers > before a consumer device resume. Same question with parents using their child - I'm thinking that perhaps we can arrange for the struct device of the parent to get used and ignore the child device outside of the MFD. [-- Attachment #2: signature.asc --] [-- Type: application/pgp-signature, Size: 473 bytes --] ^ permalink raw reply [flat|nested] 55+ messages in thread
* [RFC][PATCH 4/5] PM core: Make runtime PM of devices use device links 2016-01-14 1:52 ` [RFC][PATCH 0/5] " Rafael J. Wysocki ` (2 preceding siblings ...) 2016-01-14 1:55 ` [RFC][PATCH 3/5] PM core: Make async suspend/resume of devices use device links Rafael J. Wysocki @ 2016-01-14 1:56 ` Rafael J. Wysocki 2016-01-14 1:56 ` [RFC][PATCH 5/5] PM core: Optimize the use of device links for runtime PM Rafael J. Wysocki ` (2 subsequent siblings) 6 siblings, 0 replies; 55+ messages in thread From: Rafael J. Wysocki @ 2016-01-14 1:56 UTC (permalink / raw) To: Linux PM list Cc: Greg Kroah-Hartman, Linux Kernel Mailing List, Alan Stern, Grant Likely, Mark Brown, Rob Herring, Tomeu Vizoso, Thierry Reding, Dmitry Torokhov, Geert Uytterhoeven, Michael Turquette From: Rafael J. Wysocki <rafael.j.wysocki@intel.com> Modify the runtime PM framework to use device links to ensure that supplier devices will not be suspended if any of their consumer devices are active. The idea is to reference count suppliers on the consumer's resume and drop references to them on its suspend. The information on whether or not the supplier has been reference counted by the consumer's (runtime) resume is stored in a new field (rpm_active) in the link object for each link. It may be necessary to clean up those references when the supplier is unbinding and that's why the links whose status is DEVICE_LINK_SUPPLIER_UNBIND are skipped by the runtime suspend and resume code. The above means that if the consumer device is probed in the runtime-active state, the supplier has to be resumed and reference counted by device_link_add() so the code works as expected on its (runtime) suspend. There is a new flag, DEVICE_LINK_RPM_ACTIVE, to tell device_link_add() about that (in which case the caller is responsible for making sure that the consumer really will be runtime-active when runtime PM is enabled for it). The other new link flag, DEVICE_LINK_PM_RUNTIME, tells the core whether or not the link should be used for runtime PM at all. Signed-off-by: Rafael J. Wysocki <rafael.j.wysocki@intel.com> --- drivers/base/core.c | 15 ++++++ drivers/base/dd.c | 1 drivers/base/power/runtime.c | 93 ++++++++++++++++++++++++++++++++++++++++--- include/linux/device.h | 5 ++ include/linux/pm_runtime.h | 2 5 files changed, 111 insertions(+), 5 deletions(-) Index: linux-pm/drivers/base/power/runtime.c =================================================================== --- linux-pm.orig/drivers/base/power/runtime.c +++ linux-pm/drivers/base/power/runtime.c @@ -12,6 +12,8 @@ #include <linux/pm_runtime.h> #include <linux/pm_wakeirq.h> #include <trace/events/rpm.h> + +#include "../base.h" #include "power.h" typedef int (*pm_callback_t)(struct device *); @@ -266,19 +268,69 @@ static int rpm_check_suspend_allowed(str static int __rpm_callback(int (*cb)(struct device *), struct device *dev) __releases(&dev->power.lock) __acquires(&dev->power.lock) { - int retval; + struct devlink *link; + int retval, idx; - if (dev->power.irq_safe) + if (dev->power.irq_safe) { spin_unlock(&dev->power.lock); - else + } else { spin_unlock_irq(&dev->power.lock); + /* + * Resume suppliers if necessary. + * + * The device's runtime PM status cannot change until this + * routine returns, so it is safe to read the status outside of + * the lock. + */ + if (dev->power.runtime_status == RPM_RESUMING) { + idx = device_links_read_lock(); + + list_for_each_entry_rcu(link, &dev->consumer_links, c_node) + if ((link->flags & DEVICE_LINK_PM_RUNTIME) + && link->status != DEVICE_LINK_SUPPLIER_UNBIND + && !link->rpm_active) { + retval = pm_runtime_get_sync(link->supplier); + if (retval < 0) { + pm_runtime_put_noidle(link->supplier); + goto fail; + } + link->rpm_active = true; + } + + device_links_read_unlock(idx); + } + } + retval = cb(dev); - if (dev->power.irq_safe) + if (dev->power.irq_safe) { spin_lock(&dev->power.lock); - else + } else { + /* + * If the device is suspending and the callback has returned + * success, drop the usage counters of the suppliers that have + * been reference counted on its resume. + * + * Do that if resume fails too. + */ + if ((dev->power.runtime_status == RPM_SUSPENDING && !retval) + || (dev->power.runtime_status == RPM_RESUMING && retval)) { + idx = device_links_read_lock(); + + fail: + list_for_each_entry_rcu(link, &dev->consumer_links, c_node) + if (link->status != DEVICE_LINK_SUPPLIER_UNBIND + && link->rpm_active) { + pm_runtime_put(link->supplier); + link->rpm_active = false; + } + + device_links_read_unlock(idx); + } + spin_lock_irq(&dev->power.lock); + } return retval; } @@ -1443,6 +1495,37 @@ void pm_runtime_remove(struct device *de } /** + * pm_runtime_clean_up_links - Prepare links to consumers for driver removal. + * @dev: Device whose driver is going to be removed. + * + * Check links from this device to any consumers and if any of them have active + * runtime PM references to the device, drop the usage counter of the device + * (once per link). + * + * Since the device is guaranteed to be runtime-active at the point this is + * called, nothing else needs to be done here. + * + * Moreover, this is called after device_links_busy() has returned 'false', so + * the status of each link is guaranteed to be DEVICE_LINK_SUPPLIER_UNBIND and + * therefore rpm_active can't be manipulated concurrently. + */ +void pm_runtime_clean_up_links(struct device *dev) +{ + struct devlink *link; + int idx; + + idx = device_links_read_lock(); + + list_for_each_entry_rcu(link, &dev->supplier_links, s_node) + if (link->rpm_active) { + pm_runtime_put_noidle(dev); + link->rpm_active = false; + } + + device_links_read_unlock(idx); +} + +/** * pm_runtime_force_suspend - Force a device into suspend state if needed. * @dev: Device to suspend. * Index: linux-pm/include/linux/device.h =================================================================== --- linux-pm.orig/include/linux/device.h +++ linux-pm/include/linux/device.h @@ -704,9 +704,13 @@ enum devlink_status { * * PERSISTENT: Do not delete the link on consumer device driver unbind. * PROBE_TIME: Assume supplier device functional when creating the link. + * PM_RUNTIME: If set, the runtime PM framework will use this link. + * RPM_ACTIVE: Run pm_runtime_get_sync() on the supplier during link creation. */ #define DEVICE_LINK_PERSISTENT (1 << 0) #define DEVICE_LINK_PROBE_TIME (1 << 1) +#define DEVICE_LINK_PM_RUNTIME (1 << 2) +#define DEVICE_LINK_RPM_ACTIVE (1 << 3) struct devlink { struct device *supplier; @@ -715,6 +719,7 @@ struct devlink { struct list_head c_node; enum devlink_status status; u32 flags; + bool rpm_active; spinlock_t lock; struct rcu_head rcu_head; }; Index: linux-pm/drivers/base/core.c =================================================================== --- linux-pm.orig/drivers/base/core.c +++ linux-pm/drivers/base/core.c @@ -88,6 +88,11 @@ struct devlink *device_link_add(struct d if (!consumer || !supplier || !flags) return NULL; +#define RPM_ACTIVE_FLAGS (DEVICE_LINK_PM_RUNTIME | DEVICE_LINK_PROBE_TIME) + if ((flags & DEVICE_LINK_RPM_ACTIVE) + && (flags & RPM_ACTIVE_FLAGS) != RPM_ACTIVE_FLAGS) + return NULL; + mutex_lock(&device_links_lock); list_for_each_entry(link, &supplier->supplier_links, s_node) @@ -98,6 +103,16 @@ struct devlink *device_link_add(struct d if (!link) goto out; + if (flags & DEVICE_LINK_RPM_ACTIVE) { + if (pm_runtime_get_sync(supplier) < 0) { + pm_runtime_put_noidle(supplier); + kfree(link); + goto out; + } + link->rpm_active = true; + } else { + link->rpm_active = false; + } get_device(supplier); link->supplier = supplier; INIT_LIST_HEAD(&link->s_node); Index: linux-pm/drivers/base/dd.c =================================================================== --- linux-pm.orig/drivers/base/dd.c +++ linux-pm/drivers/base/dd.c @@ -771,6 +771,7 @@ static void __device_release_driver(stru } pm_runtime_get_sync(dev); + pm_runtime_clean_up_links(dev); driver_sysfs_remove(dev); Index: linux-pm/include/linux/pm_runtime.h =================================================================== --- linux-pm.orig/include/linux/pm_runtime.h +++ linux-pm/include/linux/pm_runtime.h @@ -55,6 +55,7 @@ extern unsigned long pm_runtime_autosusp extern void pm_runtime_update_max_time_suspended(struct device *dev, s64 delta_ns); extern void pm_runtime_set_memalloc_noio(struct device *dev, bool enable); +extern void pm_runtime_clean_up_links(struct device *dev); static inline bool pm_children_suspended(struct device *dev) { @@ -180,6 +181,7 @@ static inline unsigned long pm_runtime_a struct device *dev) { return 0; } static inline void pm_runtime_set_memalloc_noio(struct device *dev, bool enable){} +static inline void pm_runtime_clean_up_links(struct device *dev) {} #endif /* !CONFIG_PM */ ^ permalink raw reply [flat|nested] 55+ messages in thread
* [RFC][PATCH 5/5] PM core: Optimize the use of device links for runtime PM 2016-01-14 1:52 ` [RFC][PATCH 0/5] " Rafael J. Wysocki ` (3 preceding siblings ...) 2016-01-14 1:56 ` [RFC][PATCH 4/5] PM core: Make runtime PM " Rafael J. Wysocki @ 2016-01-14 1:56 ` Rafael J. Wysocki 2016-01-14 14:19 ` [RFC][PATCH 0/5] Functional dependencies between devices Tomeu Vizoso 2016-06-08 12:15 ` Mark Brown 6 siblings, 0 replies; 55+ messages in thread From: Rafael J. Wysocki @ 2016-01-14 1:56 UTC (permalink / raw) To: Linux PM list Cc: Greg Kroah-Hartman, Linux Kernel Mailing List, Alan Stern, Grant Likely, Mark Brown, Rob Herring, Tomeu Vizoso, Thierry Reding, Dmitry Torokhov, Geert Uytterhoeven, Michael Turquette From: Rafael J. Wysocki <rafael.j.wysocki@intel.com> If the device has no links to suppliers that should be used for runtime PM (links with DEVICE_LINK_PM_RUNTIME set), there is no reason to walk the list of suppliers for that device during runtime suspend and resume. Add a simple mechanism to detect that case and possibly avoid the extra unnecessary overhead. Signed-off-by: Rafael J. Wysocki <rafael.j.wysocki@intel.com> --- drivers/base/core.c | 6 ++++++ drivers/base/power/runtime.c | 23 ++++++++++++++++++++--- include/linux/pm.h | 1 + include/linux/pm_runtime.h | 4 ++++ 4 files changed, 31 insertions(+), 3 deletions(-) Index: linux-pm/include/linux/pm.h =================================================================== --- linux-pm.orig/include/linux/pm.h +++ linux-pm/include/linux/pm.h @@ -596,6 +596,7 @@ struct dev_pm_info { unsigned int use_autosuspend:1; unsigned int timer_autosuspends:1; unsigned int memalloc_noio:1; + unsigned int links_count; enum rpm_request request; enum rpm_status runtime_status; int runtime_error; Index: linux-pm/drivers/base/power/runtime.c =================================================================== --- linux-pm.orig/drivers/base/power/runtime.c +++ linux-pm/drivers/base/power/runtime.c @@ -270,6 +270,7 @@ static int __rpm_callback(int (*cb)(stru { struct devlink *link; int retval, idx; + bool use_links = dev->power.links_count > 0; if (dev->power.irq_safe) { spin_unlock(&dev->power.lock); @@ -283,7 +284,7 @@ static int __rpm_callback(int (*cb)(stru * routine returns, so it is safe to read the status outside of * the lock. */ - if (dev->power.runtime_status == RPM_RESUMING) { + if (use_links && dev->power.runtime_status == RPM_RESUMING) { idx = device_links_read_lock(); list_for_each_entry_rcu(link, &dev->consumer_links, c_node) @@ -314,8 +315,9 @@ static int __rpm_callback(int (*cb)(stru * * Do that if resume fails too. */ - if ((dev->power.runtime_status == RPM_SUSPENDING && !retval) - || (dev->power.runtime_status == RPM_RESUMING && retval)) { + if (use_links + && ((dev->power.runtime_status == RPM_SUSPENDING && !retval) + || (dev->power.runtime_status == RPM_RESUMING && retval))) { idx = device_links_read_lock(); fail: @@ -1525,6 +1527,21 @@ void pm_runtime_clean_up_links(struct de device_links_read_unlock(idx); } +void pm_runtime_new_link(struct device *dev) +{ + spin_lock_irq(&dev->power.lock); + dev->power.links_count++; + spin_unlock_irq(&dev->power.lock); +} + +void pm_runtime_drop_link(struct device *dev) +{ + spin_lock_irq(&dev->power.lock); + WARN_ON(dev->power.links_count == 0); + dev->power.links_count--; + spin_unlock_irq(&dev->power.lock); +} + /** * pm_runtime_force_suspend - Force a device into suspend state if needed. * @dev: Device to suspend. Index: linux-pm/include/linux/pm_runtime.h =================================================================== --- linux-pm.orig/include/linux/pm_runtime.h +++ linux-pm/include/linux/pm_runtime.h @@ -56,6 +56,8 @@ extern void pm_runtime_update_max_time_s s64 delta_ns); extern void pm_runtime_set_memalloc_noio(struct device *dev, bool enable); extern void pm_runtime_clean_up_links(struct device *dev); +extern void pm_runtime_new_link(struct device *dev); +extern void pm_runtime_drop_link(struct device *dev); static inline bool pm_children_suspended(struct device *dev) { @@ -182,6 +184,8 @@ static inline unsigned long pm_runtime_a static inline void pm_runtime_set_memalloc_noio(struct device *dev, bool enable){} static inline void pm_runtime_clean_up_links(struct device *dev) {} +static inline void pm_runtime_new_link(struct device *dev) {} +static inline void pm_runtime_drop_link(struct device *dev) {} #endif /* !CONFIG_PM */ Index: linux-pm/drivers/base/core.c =================================================================== --- linux-pm.orig/drivers/base/core.c +++ linux-pm/drivers/base/core.c @@ -119,6 +119,9 @@ struct devlink *device_link_add(struct d get_device(consumer); link->consumer = consumer; INIT_LIST_HEAD(&link->c_node); + if (flags & DEVICE_LINK_PM_RUNTIME) + pm_runtime_new_link(consumer); + link->flags = flags; link->status = (flags & DEVICE_LINK_PROBE_TIME) ? DEVICE_LINK_CONSUMER_PROBE : DEVICE_LINK_DORMANT; @@ -161,6 +164,9 @@ static void devlink_del(struct devlink * dev_info(link->consumer, "Dropping the link to %s\n", dev_name(link->supplier)); + if (link->flags & DEVICE_LINK_PM_RUNTIME) + pm_runtime_drop_link(link->consumer); + list_del_rcu(&link->s_node); list_del_rcu(&link->c_node); call_srcu(&device_links_srcu, &link->rcu_head, __devlink_free_srcu); ^ permalink raw reply [flat|nested] 55+ messages in thread
* Re: [RFC][PATCH 0/5] Functional dependencies between devices 2016-01-14 1:52 ` [RFC][PATCH 0/5] " Rafael J. Wysocki ` (4 preceding siblings ...) 2016-01-14 1:56 ` [RFC][PATCH 5/5] PM core: Optimize the use of device links for runtime PM Rafael J. Wysocki @ 2016-01-14 14:19 ` Tomeu Vizoso 2016-01-15 0:44 ` Rafael J. Wysocki 2016-06-08 12:15 ` Mark Brown 6 siblings, 1 reply; 55+ messages in thread From: Tomeu Vizoso @ 2016-01-14 14:19 UTC (permalink / raw) To: Rafael J. Wysocki Cc: Linux PM list, Greg Kroah-Hartman, Linux Kernel Mailing List, Alan Stern, Grant Likely, Mark Brown, Rob Herring, Thierry Reding, Dmitry Torokhov, Geert Uytterhoeven, Michael Turquette On 14 January 2016 at 02:52, Rafael J. Wysocki <rjw@rjwysocki.net> wrote: > On Tuesday, October 27, 2015 04:24:14 PM Rafael J. Wysocki wrote: >> Hi All, >> >> As discussed in the recent "On-demand device probing" thread and in a Kernel >> Summit session earlier today, 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). >> >> So I want to be able to represent those functional dependencies between devices >> and I'd like 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). >> >> My idea is to represent a supplier-consumer dependency between devices (or >> more precisely between device+driver combos) as a "link" object containing >> pointers to the devices in question, a list node for each of them and some >> additional information related to the management of those objects, ie. >> something like: >> >> struct device_link { >> struct device *supplier; >> struct list_head supplier_node; >> struct device *consumer; >> struct list_head consumer_node; >> <flags, status etc> >> }; >> >> In general, there will be two lists of those things per device, one list >> of links to consumers and one list of links to suppliers. >> >> In that picture, links will be created by calling, say: >> >> int device_add_link(struct device *me, struct device *my_supplier, unsigned int flags); >> >> and they will be deleted by the driver core when not needed any more. The >> creation of a link should also cause dpm_list and the list used during shutdown >> to be reordered if needed. >> >> In principle, it seems usefult to consider two types of links, one created >> at device registration time (when registering the second device from the linked >> pair, whichever it is) and one created at probe time (of the consumer device). >> I'll refer to them as "permanent" and "probe-time" links, respectively. >> >> The permanent links (created at device registration time) will stay around >> until one of the linked devices is unregistered (at which time the driver >> core will drop the link along with the device going away). The probe-time >> ones will be dropped (automatically) at the consumer device driver unbind time. >> >> There's a question about what if the supplier device is being unbound before >> the consumer one (for example, as a result of a hotplug event). My current >> view on that is that the consumer needs to be force-unbound in that case too, >> but I guess I may be persuaded otherwise given sufficiently convincing >> arguments. Anyway, there are reasons to do that, like for example it may >> help with the synchronization. Namely, if there's a rule that suppliers >> cannot be unbound before any consumers linked to them, than the list of links >> to suppliers for a consumer can only change at its registration/probe or >> unbind/remove times (which simplifies things quite a bit). >> >> With that, the permanent links existing at the probe time for a consumer >> device can be used to check whether or not to defer the probing of it >> even before executing its probe callback. In turn, system suspend >> synchronization should be a matter of calling device_pm_wait_for_dev() >> for all consumers of a supplier device, in analogy with dpm_wait_for_children(), >> and so on. >> >> Of course, the new lists have to be stable during those operations and ensuring >> that is going to be somewhat tricky (AFAICS right now at least), but apart from >> that the whole concept looks reasonably straightforward to me. >> > > What follows is my prototype implementation of this. It took some time > to develop (much more than I was hoping for), but here it goes at last. > > The first patch rearranges the code around __device_release_driver() a bit > to prepare it for the next one. > > The second patch introduces the actual device links mechanics, but without > system suspend/resume and runtime PM support which are added by the subsequent > patches. > > This hasn't been really tested yet (apart from checking that it doesn't break > things when device links are not in used, which would be rather embarrassing), > but at this time I'd really like you to have a look and tell me what you think > (especially if you see a reason why this is not going to work). Hi Rafael, have given a quick look and I have 2 questions for now: - Why deferring the probe if a supplier isn't ready? Seems like quite a bit of a waste to keep iterating that list until all suppliers have probed. If we know that a supplier is needed at a given time, why not probe it right away? - When were you thinking of calling device_link_add for permanent links? I also wonder if we could find clearer names for supplier_links and consumer_links, as it wasn't immediately clear to me what those lists contained. Maybe just "consumers" and "suppliers"? Thanks, Tomeu ^ permalink raw reply [flat|nested] 55+ messages in thread
* Re: [RFC][PATCH 0/5] Functional dependencies between devices 2016-01-14 14:19 ` [RFC][PATCH 0/5] Functional dependencies between devices Tomeu Vizoso @ 2016-01-15 0:44 ` Rafael J. Wysocki 0 siblings, 0 replies; 55+ messages in thread From: Rafael J. Wysocki @ 2016-01-15 0:44 UTC (permalink / raw) To: Tomeu Vizoso Cc: Linux PM list, Greg Kroah-Hartman, Linux Kernel Mailing List, Alan Stern, Grant Likely, Mark Brown, Rob Herring, Thierry Reding, Dmitry Torokhov, Geert Uytterhoeven, Michael Turquette On Thursday, January 14, 2016 03:19:03 PM Tomeu Vizoso wrote: > On 14 January 2016 at 02:52, Rafael J. Wysocki <rjw@rjwysocki.net> wrote: > > On Tuesday, October 27, 2015 04:24:14 PM Rafael J. Wysocki wrote: > >> Hi All, > >> > >> As discussed in the recent "On-demand device probing" thread and in a Kernel > >> Summit session earlier today, 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). > >> > >> So I want to be able to represent those functional dependencies between devices > >> and I'd like 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). > >> > >> My idea is to represent a supplier-consumer dependency between devices (or > >> more precisely between device+driver combos) as a "link" object containing > >> pointers to the devices in question, a list node for each of them and some > >> additional information related to the management of those objects, ie. > >> something like: > >> > >> struct device_link { > >> struct device *supplier; > >> struct list_head supplier_node; > >> struct device *consumer; > >> struct list_head consumer_node; > >> <flags, status etc> > >> }; > >> > >> In general, there will be two lists of those things per device, one list > >> of links to consumers and one list of links to suppliers. > >> > >> In that picture, links will be created by calling, say: > >> > >> int device_add_link(struct device *me, struct device *my_supplier, unsigned int flags); > >> > >> and they will be deleted by the driver core when not needed any more. The > >> creation of a link should also cause dpm_list and the list used during shutdown > >> to be reordered if needed. > >> > >> In principle, it seems usefult to consider two types of links, one created > >> at device registration time (when registering the second device from the linked > >> pair, whichever it is) and one created at probe time (of the consumer device). > >> I'll refer to them as "permanent" and "probe-time" links, respectively. > >> > >> The permanent links (created at device registration time) will stay around > >> until one of the linked devices is unregistered (at which time the driver > >> core will drop the link along with the device going away). The probe-time > >> ones will be dropped (automatically) at the consumer device driver unbind time. > >> > >> There's a question about what if the supplier device is being unbound before > >> the consumer one (for example, as a result of a hotplug event). My current > >> view on that is that the consumer needs to be force-unbound in that case too, > >> but I guess I may be persuaded otherwise given sufficiently convincing > >> arguments. Anyway, there are reasons to do that, like for example it may > >> help with the synchronization. Namely, if there's a rule that suppliers > >> cannot be unbound before any consumers linked to them, than the list of links > >> to suppliers for a consumer can only change at its registration/probe or > >> unbind/remove times (which simplifies things quite a bit). > >> > >> With that, the permanent links existing at the probe time for a consumer > >> device can be used to check whether or not to defer the probing of it > >> even before executing its probe callback. In turn, system suspend > >> synchronization should be a matter of calling device_pm_wait_for_dev() > >> for all consumers of a supplier device, in analogy with dpm_wait_for_children(), > >> and so on. > >> > >> Of course, the new lists have to be stable during those operations and ensuring > >> that is going to be somewhat tricky (AFAICS right now at least), but apart from > >> that the whole concept looks reasonably straightforward to me. > >> > > > > What follows is my prototype implementation of this. It took some time > > to develop (much more than I was hoping for), but here it goes at last. > > > > The first patch rearranges the code around __device_release_driver() a bit > > to prepare it for the next one. > > > > The second patch introduces the actual device links mechanics, but without > > system suspend/resume and runtime PM support which are added by the subsequent > > patches. > > > > This hasn't been really tested yet (apart from checking that it doesn't break > > things when device links are not in used, which would be rather embarrassing), > > but at this time I'd really like you to have a look and tell me what you think > > (especially if you see a reason why this is not going to work). > > Hi Rafael, > > have given a quick look and I have 2 questions for now: > > - Why deferring the probe if a supplier isn't ready? Seems like quite > a bit of a waste to keep iterating that list until all suppliers have > probed. If we know that a supplier is needed at a given time, why not > probe it right away? Because it is not guaranteed to succeed. The fact that the supplier is not ready may very well mean that its driver has not been loaded yet and we need to wait for that to happen before actually trying to bind the supplier. So what is done in the current patches is needed anyway and there might be some optimizations on top of that, but I'm not really sure how much of a difference they would make in practice. > > - When were you thinking of calling device_link_add for permanent links? For example, I'm envisioning calling it from the ACPI layer for devices with _DEP. > I also wonder if we could find clearer names for supplier_links and > consumer_links, as it wasn't immediately clear to me what those lists > contained. Maybe just "consumers" and "suppliers"? OK, I see why these names may be confusing. These are lists of links, not lists of devices, though, so I'd like the names to reflect that. What about links_to_suppliers and links_to_consumers? Somewhat longer, but should be less confusing IMO. Thanks, Rafael ^ permalink raw reply [flat|nested] 55+ messages in thread
* Re: [RFC][PATCH 0/5] Functional dependencies between devices 2016-01-14 1:52 ` [RFC][PATCH 0/5] " Rafael J. Wysocki ` (5 preceding siblings ...) 2016-01-14 14:19 ` [RFC][PATCH 0/5] Functional dependencies between devices Tomeu Vizoso @ 2016-06-08 12:15 ` Mark Brown 2016-06-08 17:24 ` Rafael J. Wysocki 6 siblings, 1 reply; 55+ messages in thread From: Mark Brown @ 2016-06-08 12:15 UTC (permalink / raw) To: Rafael J. Wysocki Cc: Linux PM list, Greg Kroah-Hartman, Linux Kernel Mailing List, Alan Stern, Grant Likely, Mark Brown, Rob Herring, Tomeu Vizoso, Thierry Reding, Dmitry Torokhov, Geert Uytterhoeven, Michael Turquette [-- Attachment #1: Type: text/plain, Size: 429 bytes --] On Thu, Jan 14, 2016 at 02:52:11AM +0100, Rafael J. Wysocki wrote: > What follows is my prototype implementation of this. It took some time > to develop (much more than I was hoping for), but here it goes at last. Might be worth posting this outside the thread with CCs to people working on system integration like SoC maintainers - I'd completely missed this as it was buried and looked like the middle of an old discussion. [-- Attachment #2: signature.asc --] [-- Type: application/pgp-signature, Size: 473 bytes --] ^ permalink raw reply [flat|nested] 55+ messages in thread
* Re: [RFC][PATCH 0/5] Functional dependencies between devices 2016-06-08 12:15 ` Mark Brown @ 2016-06-08 17:24 ` Rafael J. Wysocki 2016-06-08 17:33 ` Mark Brown 0 siblings, 1 reply; 55+ messages in thread From: Rafael J. Wysocki @ 2016-06-08 17:24 UTC (permalink / raw) To: Mark Brown Cc: Rafael J. Wysocki, Linux PM list, Greg Kroah-Hartman, Linux Kernel Mailing List, Alan Stern, Grant Likely, Mark Brown, Rob Herring, Tomeu Vizoso, Thierry Reding, Dmitry Torokhov, Geert Uytterhoeven, Michael Turquette On Wed, Jun 8, 2016 at 2:15 PM, Mark Brown <broonie@kernel.org> wrote: > On Thu, Jan 14, 2016 at 02:52:11AM +0100, Rafael J. Wysocki wrote: > >> What follows is my prototype implementation of this. It took some time >> to develop (much more than I was hoping for), but here it goes at last. > > Might be worth posting this outside the thread with CCs to people > working on system integration like SoC maintainers - I'd completely > missed this as it was buried and looked like the middle of an old > discussion. OK I can resend it afresh, but need to do some rework due to some changes that happened in the meantime. ^ permalink raw reply [flat|nested] 55+ messages in thread
* Re: [RFC][PATCH 0/5] Functional dependencies between devices 2016-06-08 17:24 ` Rafael J. Wysocki @ 2016-06-08 17:33 ` Mark Brown 0 siblings, 0 replies; 55+ messages in thread From: Mark Brown @ 2016-06-08 17:33 UTC (permalink / raw) To: Rafael J. Wysocki Cc: Rafael J. Wysocki, Linux PM list, Greg Kroah-Hartman, Linux Kernel Mailing List, Alan Stern, Grant Likely, Mark Brown, Rob Herring, Tomeu Vizoso, Thierry Reding, Dmitry Torokhov, Geert Uytterhoeven, Michael Turquette [-- Attachment #1: Type: text/plain, Size: 859 bytes --] On Wed, Jun 08, 2016 at 07:24:15PM +0200, Rafael J. Wysocki wrote: > On Wed, Jun 8, 2016 at 2:15 PM, Mark Brown <broonie@kernel.org> wrote: > > On Thu, Jan 14, 2016 at 02:52:11AM +0100, Rafael J. Wysocki wrote: > >> What follows is my prototype implementation of this. It took some time > >> to develop (much more than I was hoping for), but here it goes at last. > > Might be worth posting this outside the thread with CCs to people > > working on system integration like SoC maintainers - I'd completely > > missed this as it was buried and looked like the middle of an old > > discussion. > OK > I can resend it afresh, but need to do some rework due to some changes > that happened in the meantime. Yeah, it looks like the actual patches didn't get much comment but there's the constant grumbling about deferred probe which it'd be good to resolve. [-- Attachment #2: signature.asc --] [-- Type: application/pgp-signature, Size: 473 bytes --] ^ permalink raw reply [flat|nested] 55+ messages in thread
end of thread, other threads:[~2016-06-08 22:24 UTC | newest] Thread overview: 55+ messages (download: mbox.gz / follow: Atom feed) -- links below jump to the message on this page -- 2015-10-27 15:24 [RFD] Functional dependencies between devices Rafael J. Wysocki 2015-10-27 15:20 ` Tomeu Vizoso 2015-10-28 2:15 ` Rafael J. Wysocki 2015-10-28 14:26 ` Tomeu Vizoso 2015-10-28 15:54 ` Rafael J. Wysocki 2015-10-29 0:18 ` Mark Brown 2015-10-29 14:03 ` Tomeu Vizoso 2015-10-29 14:31 ` Alan Stern 2015-10-31 2:23 ` Rafael J. Wysocki 2015-10-31 15:22 ` Alan Stern 2015-10-29 0:15 ` Mark Brown 2015-10-31 2:13 ` Rafael J. Wysocki 2015-10-31 2:40 ` Mark Brown 2015-10-30 9:50 ` Linus Walleij 2015-10-30 22:52 ` Greg Kroah-Hartman 2016-01-07 14:55 ` Tomeu Vizoso 2016-01-07 21:29 ` Greg Kroah-Hartman 2016-01-08 7:28 ` Tomeu Vizoso 2016-01-08 15:15 ` Greg Kroah-Hartman 2015-11-09 12:32 ` Thierry Reding 2015-11-09 21:42 ` Rafael J. Wysocki 2015-11-17 12:44 ` Andrzej Hajda 2015-11-18 2:17 ` Rafael J. Wysocki 2015-11-19 9:08 ` Andrzej Hajda 2015-11-19 22:04 ` Rafael J. Wysocki 2015-11-20 1:11 ` Rafael J. Wysocki 2015-11-24 14:57 ` Andrzej Hajda 2015-11-24 16:28 ` Rafael J. Wysocki 2015-11-30 7:16 ` Andrzej Hajda 2015-11-17 12:49 ` Andrzej Hajda 2015-11-17 13:55 ` Mark Brown 2015-11-19 6:50 ` Andrzej Hajda 2015-11-21 14:04 ` Mark Brown 2015-11-24 13:56 ` Andrzej Hajda 2015-11-19 13:18 ` Thierry Reding 2015-11-21 13:26 ` Mark Brown 2015-11-17 20:31 ` Alan Stern 2015-11-17 22:47 ` Mark Brown 2016-01-14 1:52 ` [RFC][PATCH 0/5] " Rafael J. Wysocki 2016-01-14 1:53 ` [RFC][PATCH 1/5] driver core: Add a wrapper around __device_release_driver() Rafael J. Wysocki 2016-01-14 1:54 ` [RFC][PATCH 2/5] driver core: Functional dependencies tracking support Rafael J. Wysocki 2016-06-08 12:48 ` Mark Brown 2016-06-08 18:12 ` Rafael J. Wysocki 2016-06-08 18:35 ` Mark Brown 2016-06-08 20:48 ` Rafael J. Wysocki 2016-06-08 22:24 ` Mark Brown 2016-01-14 1:55 ` [RFC][PATCH 3/5] PM core: Make async suspend/resume of devices use device links Rafael J. Wysocki 2016-06-08 12:59 ` Mark Brown 2016-01-14 1:56 ` [RFC][PATCH 4/5] PM core: Make runtime PM " Rafael J. Wysocki 2016-01-14 1:56 ` [RFC][PATCH 5/5] PM core: Optimize the use of device links for runtime PM Rafael J. Wysocki 2016-01-14 14:19 ` [RFC][PATCH 0/5] Functional dependencies between devices Tomeu Vizoso 2016-01-15 0:44 ` Rafael J. Wysocki 2016-06-08 12:15 ` Mark Brown 2016-06-08 17:24 ` Rafael J. Wysocki 2016-06-08 17:33 ` Mark Brown
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox; as well as URLs for NNTP newsgroup(s).