| Message ID | 20160927123429.GA5828@wunner.de (mailing list archive) |
|---|---|
| State | RFC, archived |
| Headers | show |
On Tuesday, September 27, 2016 02:34:29 PM Lukas Wunner wrote: > [+cc corbet] > > To whom it may concern, > > I made some notes while reviewing the state machine in patch 2 of this > series and thought, why not rework it into something that could eventually > go into the Documentation/ tree? > > So here's an initial draft. There's some introductory text plus > a description of the state machine. Just putting this out there now > to ease reviewers' lives, despite the obvious WIP status. I'll try to > amend it as the series converges. > > This is already rst-formatted but I haven't actually run it through > sphinx yet. Thanks a lot for doing this! It looks good to me in general. I think it would be good to add it to the series at one point (if you don't mind). I'm only a bit reluctant about advertising the usage of links between children and parents, because that doesn't look like the right tool for the purpose (as I said before, I'd prefer to add a device flag causing the parent driver to be probed before the child one if needed). Thanks, Rafael > -- >8 -- > > Subject: [PATCH] Documentation: device links: Add initial documentation > > Signed-off-by: Lukas Wunner <lukas@wunner.de> > --- > Documentation/driver-model/device_link.rst | 95 ++++++++++++++++++++++++++++++ > 1 file changed, 95 insertions(+) > create mode 100644 Documentation/driver-model/device_link.rst > > diff --git a/Documentation/driver-model/device_link.rst b/Documentation/driver-model/device_link.rst > new file mode 100644 > index 0000000..ba911b4 > --- /dev/null > +++ b/Documentation/driver-model/device_link.rst > @@ -0,0 +1,95 @@ > +============ > +Device links > +============ > + > +By default, the driver core only enforces dependencies between devices > +that are borne out of a parent/child relationship within the device > +hierarchy: When suspending, resuming or shutting down the system, devices > +are ordered based on this relationship, i.e. children are always suspended > +before their parent, and the parent is always resumed before its children. > + > +Sometimes there is a need to represent device dependencies beyond the > +mere parent/child relationship, e.g. between siblings, and have the > +driver core automatically take care of them. > + > +Secondly, the driver core by default does not enforce any driver presence > +dependencies, i.e. that one device must be bound to a driver before > +another one can probe or function correctly. > + > +Often these two dependency types come together, so a device depends on > +another one both with regards to driver presence *and* with regards to > +suspend/resume and shutdown ordering. > + > +Device links allow representation of such dependencies in the driver core. > + > +In its standard form, a device link combines *both* dependency types: > +It guarantees correct suspend/resume and shutdown ordering between a > +"supplier" device and its "consumer" devices, and it guarantees driver > +presence on the supplier: The consumer devices are not probed before the > +supplier is bound to a driver, and they're unbound before the supplier > +is unbound. > + > +When driver presence on the supplier is irrelevant and only correct > +suspend/resume and shutdown ordering is needed, the device link may > +simply be set up with the DEVICE_LINK_STATELESS flag. > + > +A driver presence dependency between parent and child, i.e. within the > +regular device hierarchy, could in principle also be represented in the > +driver core using a device link. > + > +If a device link is set up with the DEVICE_LINK_AUTOREMOVE flag, it is > +automatically purged when the consumer fails to probe or later unbinds. > +This is handy when adding a device link from the consumer's ->probe hook, > +as it obviates the need to delete the link in the ->remove hook or in > +the error path of the ->probe hook. > + > + > +State machine > +============= > + > +""" > + .=============================. > + | | > + v | > +DORMANT <=> AVAILABLE <=> CONSUMER_PROBE => ACTIVE > + ^ | > + | | > + '============ SUPPLIER_UNBIND <============' > +""" > + > +* The initial state of a device link is passed in to device_link_add(). > + If the link is created before any devices are probed, it must be set to > + DEVICE_LINK_DORMANT. > + > +* When a supplier device is bound to a driver, links to its consumers > + progress to DEVICE_LINK_AVAILABLE. > + (Call to device_links_driver_bound() from driver_bound().) > + > +* Before a consumer device is probed, presence of supplier drivers is > + verified by checking that links to suppliers are in DEVICE_LINK_AVAILABLE > + state. The state of the links is updated to DEVICE_LINK_CONSUMER_PROBE. > + (Call to device_links_check_suppliers() from driver_probe_device().) > + This prevents the supplier from unbinding. > + (Call to wait_for_device_probe() in device_links_unbind_consumers().) > + > +* If the probe fails, links to suppliers revert back to DEVICE_LINK_AVAILABLE. > + (Call to device_links_no_driver() from really_probe().) > + > +* If the probe succeeds, links to suppliers progress to DEVICE_LINK_ACTIVE. > + (Call of device_links_driver_bound() from driver_bound().) > + > +* When the consumer's driver is later on removed, links to suppliers revert > + back to DEVICE_LINK_AVAILABLE. > + (Call to device_links_no_driver() from __device_release_driver().) > + > +* Before a supplier's driver is removed, links to consumers that are not > + bound to a driver are updated to DEVICE_LINK_SUPPLIER_UNBIND. > + (Call to device_links_busy() from __device_release_driver().) > + This prevents the consumers from binding. > + (Call to device_links_check_suppliers() from driver_probe_device().) > + Consumers that are bound are freed from their driver; consumers that are > + probing are waited for until they are done. > + (Call to device_links_unbind_consumers() from __device_release_driver().) > + Once all links to consumers are in DEVICE_LINK_SUPPLIER_UNBIND state, > + the supplier driver is released and the links revert to DEVICE_LINK_DORMANT. > + (Call to device_links_driver_gone() from __device_release_driver().) -- 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
On Wed, Sep 28, 2016 at 02:33:21AM +0200, Rafael J. Wysocki wrote: > On Tuesday, September 27, 2016 02:34:29 PM Lukas Wunner wrote: > > I made some notes while reviewing the state machine in patch 2 of this > > series and thought, why not rework it into something that could eventually > > go into the Documentation/ tree? > > > > So here's an initial draft. There's some introductory text plus > > a description of the state machine. Just putting this out there now > > to ease reviewers' lives, despite the obvious WIP status. I'll try to > > amend it as the series converges. > > > > This is already rst-formatted but I haven't actually run it through > > sphinx yet. > > Thanks a lot for doing this! > > It looks good to me in general. I think it would be good to add it to the > series at one point (if you don't mind). Sure thing, thanks. > I'm only a bit reluctant about advertising the usage of links between > children and parents, because that doesn't look like the right tool for > the purpose (as I said before, I'd prefer to add a device flag causing > the parent driver to be probed before the child one if needed). That wouldn't cover the unbinding of the child when the parent unbinds though, so it would only be a subset of the functionality offered by device links. I actually don't know of a use case where driver presence is needed between parent and child. But the patches look like they should work out of the box in such a scenario, so I was thinking, why forbid it? Someone might just try that because they think it should obviously work, and then they'll find out at runtime that it's forbidden. That gives us only a score of 5 in Rusty's API rating scheme. However for consistency, if you do want to forbid it, I think it should be forbidden for all ancestors of the device, not just the parent as v3 does it. (Suspend/resume + shutdown ordering is already handled for hierarchical dependencies, i.e. all ancestors.) Thanks, Lukas -- 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
On Wednesday, September 28, 2016 01:42:20 PM Lukas Wunner wrote: > On Wed, Sep 28, 2016 at 02:33:21AM +0200, Rafael J. Wysocki wrote: > > On Tuesday, September 27, 2016 02:34:29 PM Lukas Wunner wrote: > > > I made some notes while reviewing the state machine in patch 2 of this > > > series and thought, why not rework it into something that could eventually > > > go into the Documentation/ tree? > > > > > > So here's an initial draft. There's some introductory text plus > > > a description of the state machine. Just putting this out there now > > > to ease reviewers' lives, despite the obvious WIP status. I'll try to > > > amend it as the series converges. > > > > > > This is already rst-formatted but I haven't actually run it through > > > sphinx yet. > > > > Thanks a lot for doing this! > > > > It looks good to me in general. I think it would be good to add it to the > > series at one point (if you don't mind). > > Sure thing, thanks. > > > > I'm only a bit reluctant about advertising the usage of links between > > children and parents, because that doesn't look like the right tool for > > the purpose (as I said before, I'd prefer to add a device flag causing > > the parent driver to be probed before the child one if needed). > > That wouldn't cover the unbinding of the child when the parent unbinds > though, so it would only be a subset of the functionality offered by > device links. > > I actually don't know of a use case where driver presence is needed > between parent and child. But the patches look like they should work > out of the box in such a scenario, so I was thinking, why forbid it? > Someone might just try that because they think it should obviously work, > and then they'll find out at runtime that it's forbidden. That gives > us only a score of 5 in Rusty's API rating scheme. > > However for consistency, if you do want to forbid it, I think it should > be forbidden for all ancestors of the device, not just the parent as v3 > does it. (Suspend/resume + shutdown ordering is already handled for > hierarchical dependencies, i.e. all ancestors.) Well, there is a difference between allowing something to be done and documenting it as a good idea. :-) 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
On Thu, Sep 29, 2016 at 02:51:45AM +0200, Rafael J. Wysocki wrote: > On Wednesday, September 28, 2016 01:42:20 PM Lukas Wunner wrote: > > On Wed, Sep 28, 2016 at 02:33:21AM +0200, Rafael J. Wysocki wrote: > > > I'm only a bit reluctant about advertising the usage of links between > > > children and parents, because that doesn't look like the right tool for > > > the purpose (as I said before, I'd prefer to add a device flag causing > > > the parent driver to be probed before the child one if needed). > > > > That wouldn't cover the unbinding of the child when the parent unbinds > > though, so it would only be a subset of the functionality offered by > > device links. > > > > I actually don't know of a use case where driver presence is needed > > between parent and child. But the patches look like they should work > > out of the box in such a scenario, so I was thinking, why forbid it? > > Someone might just try that because they think it should obviously work, > > and then they'll find out at runtime that it's forbidden. That gives > > us only a score of 5 in Rusty's API rating scheme. > > > > However for consistency, if you do want to forbid it, I think it should > > be forbidden for all ancestors of the device, not just the parent as v3 > > does it. (Suspend/resume + shutdown ordering is already handled for > > hierarchical dependencies, i.e. all ancestors.) > > Well, there is a difference between allowing something to be done and > documenting it as a good idea. :-) I'm reworking the documentation and to address your concerns I have now reformulated this paragraph as follows: To prevent introduction of dependency loops into the graph, it is verified upon device link addition that the supplier is not dependent on the consumer or any children or consumers of the consumer. (Call to device_is_dependent() from device_link_add().) If that constraint is violated, device_link_add() will return %NULL and a WARNING will be logged. Notably this also prevents addition of a device link from a parent device to a child. However the converse is allowed, i.e. a device link from a child to a parent. Since the driver core already guarantees correct suspend/resume and shutdown ordering between parent and child, such a device link only makes sense if a driver presence dependency is needed on top of that. In that case driver authors should weigh carefully if a device link is the right tool for the purpose. A more suitable approach might be to simply use deferred probing or add a device flag causing the parent driver to be probed before the child one. If you'd prefer a different wording just shout. Thanks, Lukas -- 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
diff --git a/Documentation/driver-model/device_link.rst b/Documentation/driver-model/device_link.rst new file mode 100644 index 0000000..ba911b4 --- /dev/null +++ b/Documentation/driver-model/device_link.rst @@ -0,0 +1,95 @@ +============ +Device links +============ + +By default, the driver core only enforces dependencies between devices +that are borne out of a parent/child relationship within the device +hierarchy: When suspending, resuming or shutting down the system, devices +are ordered based on this relationship, i.e. children are always suspended +before their parent, and the parent is always resumed before its children. + +Sometimes there is a need to represent device dependencies beyond the +mere parent/child relationship, e.g. between siblings, and have the +driver core automatically take care of them. + +Secondly, the driver core by default does not enforce any driver presence +dependencies, i.e. that one device must be bound to a driver before +another one can probe or function correctly. + +Often these two dependency types come together, so a device depends on +another one both with regards to driver presence *and* with regards to +suspend/resume and shutdown ordering. + +Device links allow representation of such dependencies in the driver core. + +In its standard form, a device link combines *both* dependency types: +It guarantees correct suspend/resume and shutdown ordering between a +"supplier" device and its "consumer" devices, and it guarantees driver +presence on the supplier: The consumer devices are not probed before the +supplier is bound to a driver, and they're unbound before the supplier +is unbound. + +When driver presence on the supplier is irrelevant and only correct +suspend/resume and shutdown ordering is needed, the device link may +simply be set up with the DEVICE_LINK_STATELESS flag. + +A driver presence dependency between parent and child, i.e. within the +regular device hierarchy, could in principle also be represented in the +driver core using a device link. + +If a device link is set up with the DEVICE_LINK_AUTOREMOVE flag, it is +automatically purged when the consumer fails to probe or later unbinds. +This is handy when adding a device link from the consumer's ->probe hook, +as it obviates the need to delete the link in the ->remove hook or in +the error path of the ->probe hook. + + +State machine +============= + +""" + .=============================. + | | + v | +DORMANT <=> AVAILABLE <=> CONSUMER_PROBE => ACTIVE + ^ | + | | + '============ SUPPLIER_UNBIND <============' +""" + +* The initial state of a device link is passed in to device_link_add(). + If the link is created before any devices are probed, it must be set to + DEVICE_LINK_DORMANT. + +* When a supplier device is bound to a driver, links to its consumers + progress to DEVICE_LINK_AVAILABLE. + (Call to device_links_driver_bound() from driver_bound().) + +* Before a consumer device is probed, presence of supplier drivers is + verified by checking that links to suppliers are in DEVICE_LINK_AVAILABLE + state. The state of the links is updated to DEVICE_LINK_CONSUMER_PROBE. + (Call to device_links_check_suppliers() from driver_probe_device().) + This prevents the supplier from unbinding. + (Call to wait_for_device_probe() in device_links_unbind_consumers().) + +* If the probe fails, links to suppliers revert back to DEVICE_LINK_AVAILABLE. + (Call to device_links_no_driver() from really_probe().) + +* If the probe succeeds, links to suppliers progress to DEVICE_LINK_ACTIVE. + (Call of device_links_driver_bound() from driver_bound().) + +* When the consumer's driver is later on removed, links to suppliers revert + back to DEVICE_LINK_AVAILABLE. + (Call to device_links_no_driver() from __device_release_driver().) + +* Before a supplier's driver is removed, links to consumers that are not + bound to a driver are updated to DEVICE_LINK_SUPPLIER_UNBIND. + (Call to device_links_busy() from __device_release_driver().) + This prevents the consumers from binding. + (Call to device_links_check_suppliers() from driver_probe_device().) + Consumers that are bound are freed from their driver; consumers that are + probing are waited for until they are done. + (Call to device_links_unbind_consumers() from __device_release_driver().) + Once all links to consumers are in DEVICE_LINK_SUPPLIER_UNBIND state, + the supplier driver is released and the links revert to DEVICE_LINK_DORMANT. + (Call to device_links_driver_gone() from __device_release_driver().)
[+cc corbet] To whom it may concern, I made some notes while reviewing the state machine in patch 2 of this series and thought, why not rework it into something that could eventually go into the Documentation/ tree? So here's an initial draft. There's some introductory text plus a description of the state machine. Just putting this out there now to ease reviewers' lives, despite the obvious WIP status. I'll try to amend it as the series converges. This is already rst-formatted but I haven't actually run it through sphinx yet. Lukas -- >8 -- Subject: [PATCH] Documentation: device links: Add initial documentation Signed-off-by: Lukas Wunner <lukas@wunner.de> --- Documentation/driver-model/device_link.rst | 95 ++++++++++++++++++++++++++++++ 1 file changed, 95 insertions(+) create mode 100644 Documentation/driver-model/device_link.rst