Re: [RFD] Functional dependencies between devices

Re: [RFD] Functional dependencies between devices

[Tomeu Vizoso]

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/

[RFD] Functional dependencies between devices

[Rafael J. Wysocki]

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

Re: [RFD] Functional dependencies between devices

[Rafael J. Wysocki]

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

Re: [RFD] Functional dependencies between devices

[Tomeu Vizoso]

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/

Re: [RFD] Functional dependencies between devices

[Rafael J. Wysocki]

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

Re: [RFD] Functional dependencies between devices

[Mark Brown]

[-- 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 --]

Re: [RFD] Functional dependencies between devices

[Mark Brown]

[-- 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 --]

Re: [RFD] Functional dependencies between devices

[Tomeu Vizoso]

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/

Re: [RFD] Functional dependencies between devices

[Alan Stern]

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

Re: [RFD] Functional dependencies between devices

[Linus Walleij]

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

Re: [RFD] Functional dependencies between devices

[Greg Kroah-Hartman]

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

Re: [RFD] Functional dependencies between devices

[Rafael J. Wysocki]

[-- 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 --]

Re: [RFD] Functional dependencies between devices

[Rafael J. Wysocki]

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

Re: [RFD] Functional dependencies between devices

[Mark Brown]

[-- 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 --]

Re: [RFD] Functional dependencies between devices

[Alan Stern]

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

Re: [RFD] Functional dependencies between devices

[Thierry Reding]

[-- 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 --]

Re: [RFD] Functional dependencies between devices

[Rafael J. Wysocki]

[-- 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 --]

Re: [RFD] Functional dependencies between devices

[Andrzej Hajda]

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
> 

Re: [RFD] Functional dependencies between devices

[Andrzej Hajda]

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
> 

Re: [RFD] Functional dependencies between devices

[Mark Brown]

[-- 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 --]

Re: [RFD] Functional dependencies between devices

[Alan Stern]

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

Re: [RFD] Functional dependencies between devices

[Mark Brown]

[-- 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 --]

Re: [RFD] Functional dependencies between devices

[Rafael J. Wysocki]

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

Re: [RFD] Functional dependencies between devices

[Andrzej Hajda]

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

Re: [RFD] Functional dependencies between devices

[Andrzej Hajda]

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
>
>
>

Re: [RFD] Functional dependencies between devices

[Thierry Reding]

[-- 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 --]

Re: [RFD] Functional dependencies between devices

[Rafael J. Wysocki]

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

Re: [RFD] Functional dependencies between devices

[Rafael J. Wysocki]

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

Re: [RFD] Functional dependencies between devices

[Mark Brown]

[-- 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 --]

Re: [RFD] Functional dependencies between devices

[Mark Brown]

[-- 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 --]

Re: [RFD] Functional dependencies between devices

[Andrzej Hajda]

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

Re: [RFD] Functional dependencies between devices

[Andrzej Hajda]

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
>
>

Re: [RFD] Functional dependencies between devices

[Rafael J. Wysocki]

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

Re: [RFD] Functional dependencies between devices

[Andrzej Hajda]

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
>
>

Re: [RFD] Functional dependencies between devices

[Tomeu Vizoso]

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

Re: [RFD] Functional dependencies between devices

[Greg Kroah-Hartman]

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

Re: [RFD] Functional dependencies between devices

[Tomeu Vizoso]

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

Re: [RFD] Functional dependencies between devices

[Greg Kroah-Hartman]

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...

[RFC][PATCH 0/5] Functional dependencies between devices

[Rafael J. Wysocki]

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

[RFC][PATCH 1/5] driver core: Add a wrapper around __device_release_driver()

[Rafael J. Wysocki]

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);
 	}
 }

[RFC][PATCH 2/5] driver core: Functional dependencies tracking support

[Rafael J. Wysocki]

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);

[RFC][PATCH 3/5] PM core: Make async suspend/resume of devices use device links

[Rafael J. Wysocki]

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;

[RFC][PATCH 4/5] PM core: Make runtime PM of devices use device links

[Rafael J. Wysocki]

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 */
 

[RFC][PATCH 5/5] PM core: Optimize the use of device links for runtime PM

[Rafael J. Wysocki]

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);

Re: [RFC][PATCH 0/5] Functional dependencies between devices

[Tomeu Vizoso]

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

Re: [RFC][PATCH 0/5] Functional dependencies between devices

[Rafael J. Wysocki]

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

Re: [RFC][PATCH 0/5] Functional dependencies between devices

[Mark Brown]

[-- 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 --]

Re: [RFC][PATCH 2/5] driver core: Functional dependencies tracking support

[Mark Brown]

[-- 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 --]

Re: [RFC][PATCH 3/5] PM core: Make async suspend/resume of devices use device links

[Mark Brown]

[-- 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 --]

Re: [RFC][PATCH 0/5] Functional dependencies between devices

[Rafael J. Wysocki]

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.

Re: [RFC][PATCH 0/5] Functional dependencies between devices

[Mark Brown]

[-- 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 --]

Re: [RFC][PATCH 2/5] driver core: Functional dependencies tracking support

[Rafael J. Wysocki]

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.

Re: [RFC][PATCH 2/5] driver core: Functional dependencies tracking support

[Mark Brown]

[-- 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 --]

Re: [RFC][PATCH 2/5] driver core: Functional dependencies tracking support

[Rafael J. Wysocki]

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).

Re: [RFC][PATCH 2/5] driver core: Functional dependencies tracking support

[Mark Brown]

[-- 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 --]