[PATCH 0/2] Device links documentation

[PATCH 0/2] Device links documentation

[Lukas Wunner]

Here's my suggestion for initial documentation on the device links
feature that's queued for 4.10 on Greg KH's driver-core-next branch.
Please let me know if you have any additions or corrections.

To read this in rendered form:
http://wunner.de/kernel-doc/core-api/device_link.html

Patch [2/2] could go in via driver-core-next, but patch [1/2] would
need to go in via docs-next (because the core-api directory doesn't
exist yet in driver-core-next).

@Jonathan Corbet:  Is core-api the right place to put this? An
alternative would be Documentation/driver-api, but unlike core-api
it contains less prose text but mostly just bare API docs gleaned
from kernel-doc.


To render the documentation oneself, the device links patches need
to be merged from driver-core-next into docs-next.  Here's a branch
which contains all the necessary bits:
https://github.com/l1k/linux/commits/device_links_docs_v1

(The build is currently broken for docs-next, one needs to remove
80211.xml from Documentation/DocBook/Makefile and for some reason
$BUILDDIR/cec.h.rst is not found on the first build, but only on
the second build.)


About half of the documentation was distilled from the mailing list
discussion initiated by Luis Rodriguez after KS/LPC:
https://lkml.org/lkml/2016/11/7/790
https://lkml.org/lkml/2016/11/7/795
https://lkml.org/lkml/2016/11/8/899

The other half is based on the first draft I posted in September:
https://lkml.org/lkml/2016/9/27/215

Thanks,

Lukas


Lukas Wunner (2):
  Documentation/core-api/device_link: Add initial documentation
  driver core: Silence device links sphinx warning

 Documentation/core-api/device_link.rst | 279 +++++++++++++++++++++++++++++++++
 Documentation/core-api/index.rst       |   8 +
 drivers/base/core.c                    |   4 +-
 include/linux/device.h                 |   1 +
 4 files changed, 290 insertions(+), 2 deletions(-)
 create mode 100644 Documentation/core-api/device_link.rst

-- 
2.10.2

[PATCH 1/2] Documentation/core-api/device_link: Add initial documentation

[Lukas Wunner]

Document device links as introduced in v4.10 with commits:
    4bdb35506b89 ("driver core: Add a wrapper around
                   __device_release_driver()")
    9ed9895370ae ("driver core: Functional dependencies tracking
                   support")
    8c73b4288496 ("PM / sleep: Make async suspend/resume of devices use
                   device links")
    21d5c57b3726 ("PM / runtime: Use device links")
    baa8809f6097 ("PM / runtime: Optimize the use of device links")

Cc: Rafael J. Wysocki <rafael-DgEjT+Ai2ygdnm+yROfE0A@public.gmane.org>
Cc: Greg Kroah-Hartman <gregkh-hQyY1W1yCW8ekmWlsbkhG0B+6BGkLq7r@public.gmane.org>
Cc: Luis R. Rodriguez <mcgrof-DgEjT+Ai2ygdnm+yROfE0A@public.gmane.org>
Cc: Jonathan Corbet <corbet-T1hC0tSOHrs@public.gmane.org>
Cc: Silvio Fricke <silvio.fricke-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>
Cc: Marek Szyprowski <m.szyprowski-Sze3O3UU22JBDgjK7y7TUQ@public.gmane.org>
Cc: Bartlomiej Zolnierkiewicz <b.zolnierkie-Sze3O3UU22JBDgjK7y7TUQ@public.gmane.org>
Cc: Andrzej Hajda <a.hajda-Sze3O3UU22JBDgjK7y7TUQ@public.gmane.org>
Cc: Mauro Carvalho Chehab <mchehab-JPH+aEBZ4P+UEJcrhfAQsw@public.gmane.org>
Cc: Inki Dae <inki.dae-Sze3O3UU22JBDgjK7y7TUQ@public.gmane.org>
Cc: Joerg Roedel <joro-zLv9SwRftAIdnm+yROfE0A@public.gmane.org>
Cc: Kukjin Kim <kgene-DgEjT+Ai2ygdnm+yROfE0A@public.gmane.org>
Cc: Krzysztof Kozlowski <krzk-DgEjT+Ai2ygdnm+yROfE0A@public.gmane.org>
Cc: Mark Brown <broonie-DgEjT+Ai2ygdnm+yROfE0A@public.gmane.org>
Cc: Tomeu Vizoso <tomeu.vizoso-ZGY8ohtN/8qB+jHODAdFcQ@public.gmane.org>
Cc: Kevin Hilman <khilman-DgEjT+Ai2ygdnm+yROfE0A@public.gmane.org>
Cc: Ulf Hansson <ulf.hansson-QSEj5FYQhm4dnm+yROfE0A@public.gmane.org>
Cc: Geert Uytterhoeven <geert-Td1EMuHUCqxL1ZNQvxDV9g@public.gmane.org>
Cc: Tobias Jakobi <tjakobi-o02PS0xoJP9W0yFyLvAVXMxlOr/tl8fh@public.gmane.org>
Cc: Tomasz Figa <tomasz.figa-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>
Cc: Grant Likely <grant.likely-s3s/WqlpOiPyB63q8FvJNQ@public.gmane.org>
Cc: Laurent Pinchart <laurent.pinchart-ryLnwIuWjnjg/C1BVhZhaw@public.gmane.org>
Cc: Lars-Peter Clausen <lars-Qo5EllUWu/uELgA04lAiVw@public.gmane.org>
Cc: Dmitry Torokhov <dmitry.torokhov-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>
Cc: Christoph Hellwig <hch-wEGCiKHe2LqWVfeAwA7xHQ@public.gmane.org>
Cc: Arnd Bergmann <arnd-r2nGTMty4D4@public.gmane.org>
Cc: Alan Stern <stern-nwvwT67g6+6dFdvTe/nMLpVzexx5G7lz@public.gmane.org>
Cc: Hanjun Guo <guohanjun-hv44wF8Li93QT0dZR+AlfA@public.gmane.org>
Cc: linux-pm-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
Cc: linux-kernel-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
Cc: iommu-cunTk1MwBs9QetFLy7KEm3xJsTq8ys+cHZ5vskTnxNA@public.gmane.org
Cc: linux-samsung-soc-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
Signed-off-by: Lukas Wunner <lukas-JFq808J9C/izQB+pC5nmwQ@public.gmane.org>
---
 Documentation/core-api/device_link.rst | 279 +++++++++++++++++++++++++++++++++
 Documentation/core-api/index.rst       |   8 +
 2 files changed, 287 insertions(+)
 create mode 100644 Documentation/core-api/device_link.rst

diff --git a/Documentation/core-api/device_link.rst b/Documentation/core-api/device_link.rst
new file mode 100644
index 0000000..5f57134
--- /dev/null
+++ b/Documentation/core-api/device_link.rst
@@ -0,0 +1,279 @@
+============
+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 ``DL_FLAG_STATELESS`` flag.  In other words,
+enforcing driver presence on the supplier is optional.
+
+Another optional feature is runtime PM integration:  By setting the
+``DL_FLAG_PM_RUNTIME`` flag on addition of the device link, the PM core
+is instructed to runtime resume the supplier and keep it active
+whenever and for as long as the consumer is runtime resumed.
+
+Usage
+=====
+
+The earliest point in time when device links can be added is after
+:c:func:`device_add()` has been called for the supplier and
+:c:func:`device_initialize()` has been called for the consumer.
+
+It is legal to add them later, but care must be taken that the system
+remains in a consistent state:  E.g. a device link cannot be added in
+the midst of a suspend/resume transition, so either commencement of
+such a transition needs to be prevented with :c:func:`lock_system_sleep()`,
+or the device link needs to be added from a function which is guaranteed
+not to run in parallel to a suspend/resume transition, such as from a
+device ``->probe`` callback or a boot-time PCI quirk.
+
+Another example for an inconsistent state would be a device link that
+represents a driver presence dependency, yet is added from the consumer's
+``->probe`` callback while the supplier hasn't probed yet:  Had the driver
+core known about the device link earlier, it wouldn't have probed the
+consumer in the first place.  The onus is thus on the consumer to check
+presence of the supplier after adding the link, and defer probing on
+non-presence.
+
+If a device link is added in the ``->probe`` callback of the supplier or
+consumer driver, it is typically deleted in its ``->remove`` callback for
+symmetry.  That way, if the driver is compiled as a module, the device
+link is added on module load and orderly deleted on unload.  The same
+restrictions that apply to device link addition (e.g. exclusion of a
+parallel suspend/resume transition) apply equally to deletion.
+
+Several flags may be specified on device link addition, two of which
+have already been mentioned above:  ``DL_FLAG_STATELESS`` to express that no
+driver presence dependency is needed (but only correct suspend/resume and
+shutdown ordering) and ``DL_FLAG_PM_RUNTIME`` to express that runtime PM
+integration is desired.
+
+Two other flags are specifically targeted at use cases where the device
+link is added from the consumer's ``->probe`` callback:  ``DL_FLAG_RPM_ACTIVE``
+can be specified to runtime resume the supplier upon addition of the
+device link.  ``DL_FLAG_AUTOREMOVE`` causes the device link to be automatically
+purged when the consumer fails to probe or later unbinds.  This obviates
+the need to explicitly delete the link in the ``->remove`` callback or in
+the error path of the ``->probe`` callback.
+
+Limitations
+===========
+
+Driver authors should be aware that a driver presence dependency (i.e. when
+``DL_FLAG_STATELESS`` is not specified on link addition) may cause probing of
+the consumer to be deferred indefinitely.  This can become a problem if the
+consumer is required to probe before a certain initcall level is reached.
+Worse, if the supplier driver is blacklisted or missing, the consumer will
+never be probed.
+
+Sometimes drivers depend on optional resources.  They are able to operate
+in a degraded mode (reduced feature set or performance) when those resources
+are not present.  An example is an SPI controller that can use a DMA engine
+or work in PIO mode.  The controller can determine presence of the optional
+resources at probe time but on non-presence there is no way to know whether
+they will become available in the near future (due to a supplier driver
+probing) or never.  Consequently it cannot be determined whether to defer
+probing or not.  It would be possible to notify drivers when optional
+resources become available after probing, but it would come at a high cost
+for drivers as switching between modes of operation at runtime based on the
+availability of such resources would be much more complex than a mechanism
+based on probe deferral.  In any case optional resources are beyond the
+scope of device links.
+
+Examples
+========
+
+* An MMU device exists alongside a busmaster device, both are in the same
+  power domain.  The MMU implements DMA address translation for the busmaster
+  device and shall be runtime resumed and kept active whenever and as long
+  as the busmaster device is active.  The busmaster device's driver shall
+  not bind before the MMU is bound.  To achieve this, a device link with
+  runtime PM integration is added from the busmaster device (consumer)
+  to the MMU device (supplier).  The effect with regards to runtime PM
+  is the same as if the MMU was the parent of the master device.
+
+  The fact that both devices share the same power domain would normally
+  suggest usage of a :c:type:`struct dev_pm_domain` or :c:type:`struct
+  generic_pm_domain`, however these are not independent devices that
+  happen to share a power switch, but rather the MMU device serves the
+  busmaster device and is useless without it.  A device link creates a
+  synthetic hierarchical relationship between the devices and is thus
+  more apt.
+
+* A Thunderbolt host controller comprises a number of PCIe hotplug ports
+  and an NHI device to manage the PCIe switch.  On resume from system sleep,
+  the NHI device needs to re-establish PCI tunnels to attached devices
+  before the hotplug ports can resume.  If the hotplug ports were children
+  of the NHI, this resume order would automatically be enforced by the
+  PM core, but unfortunately they're aunts.  The solution is to add
+  device links from the hotplug ports (consumers) to the NHI device
+  (supplier).  A driver presence dependency is not necessary for this
+  use case.
+
+* Discrete GPUs in hybrid graphics laptops often feature an HDA controller
+  for HDMI/DP audio.  In the device hierarchy the HDA controller is a sibling
+  of the VGA device, yet both share the same power domain and the HDA
+  controller is only ever needed when an HDMI/DP display is attached to the
+  VGA device.  A device link from the HDA controller (consumer) to the
+  VGA device (supplier) aptly represents this relationship.
+
+* ACPI allows definition of a device start order by way of _DEP objects.
+  A classical example is when ACPI power management methods on one device
+  are implemented in terms of I\ :sup:`2`\ C accesses and require a specific
+  I\ :sup:`2`\ C controller to be present and functional for the power
+  management of the device in question to work.
+
+* In some SoCs a functional dependency exists from display, video codec and
+  video processing IP cores on transparent memory access IP cores that handle
+  burst access and compression/decompression.
+
+Alternatives
+============
+
+* A :c:type:`struct dev_pm_domain` can be used to override the bus,
+  class or device type callbacks.  It is intended for devices sharing
+  a single on/off switch, however it does not guarantee a specific
+  suspend/resume ordering, this needs to be implemented separately.
+  It also does not by itself track the runtime PM status of the involved
+  devices and turn off the power switch only when all of them are runtime
+  suspended.  Furthermore it cannot be used to enforce a specific shutdown
+  ordering or a driver presence dependency.
+
+* A :c:type:`struct generic_pm_domain` is a lot more heavyweight than a
+  device link and does not allow for shutdown ordering or driver presence
+  dependencies.  It also cannot be used on ACPI systems.
+
+Implementation
+==============
+
+The device hierarchy, which -- as the name implies -- is a tree,
+becomes a directed acyclic graph once device links are added.
+
+Ordering of these devices during suspend/resume is determined by the
+dpm_list.  During shutdown it is determined by the devices_kset.  With
+no device links present, the two lists are a flattened, one-dimensional
+representations of the device tree such that a device is placed behind
+all its ancestors.  That is achieved by traversing the ACPI namespace
+or OpenFirmware device tree top-down and appending devices to the lists
+as they are discovered.
+
+Once device links are added, the lists need to satisfy the additional
+constraint that a device is placed behind all its suppliers, recursively.
+To ensure this, upon addition of the device link the consumer and the
+entire sub-graph below it (all children and consumers of the consumer)
+are moved to the end of the list.  (Call to :c:func:`device_reorder_to_tail()`
+from :c:func:`device_link_add()`.)
+
+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 :c:func:`device_is_dependent()` from :c:func:`device_link_add()`.)
+If that constraint is violated, :c:func:`device_link_add()` will return
+``NULL`` and a ``WARNING`` will be logged.
+
+Notably this also prevents the 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 this case driver authors should weigh
+carefully if a device link is at all 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.
+
+State machine
+=============
+
+.. kernel-doc:: include/linux/device.h
+   :functions: device_link_state
+
+::
+
+                 .=============================.
+                 |                             |
+                 v                             |
+ DORMANT <=> AVAILABLE <=> CONSUMER_PROBE => ACTIVE
+    ^                                          |
+    |                                          |
+    '============ SUPPLIER_UNBIND <============'
+
+* The initial state of a device link is automatically determined by
+  :c:func:`device_link_add()` based on the driver presence on the supplier
+  and consumer.  If the link is created before any devices are probed, it
+  is set to ``DL_STATE_DORMANT``.
+
+* When a supplier device is bound to a driver, links to its consumers
+  progress to ``DL_STATE_AVAILABLE``.
+  (Call to :c:func:`device_links_driver_bound()` from
+  :c:func:`driver_bound()`.)
+
+* Before a consumer device is probed, presence of supplier drivers is
+  verified by checking that links to suppliers are in ``DL_STATE_AVAILABLE``
+  state.  The state of the links is updated to ``DL_STATE_CONSUMER_PROBE``.
+  (Call to :c:func:`device_links_check_suppliers()` from
+  :c:func:`really_probe()`.)
+  This prevents the supplier from unbinding.
+  (Call to :c:func:`wait_for_device_probe()` from
+  :c:func:`device_links_unbind_consumers()`.)
+
+* If the probe fails, links to suppliers revert back to ``DL_STATE_AVAILABLE``.
+  (Call to :c:func:`device_links_no_driver()` from :c:func:`really_probe()`.)
+
+* If the probe succeeds, links to suppliers progress to ``DL_STATE_ACTIVE``.
+  (Call to :c:func:`device_links_driver_bound()` from :c:func:`driver_bound()`.)
+
+* When the consumer's driver is later on removed, links to suppliers revert
+  back to ``DL_STATE_AVAILABLE``.
+  (Call to :c:func:`__device_links_no_driver()` from
+  :c:func:`device_links_driver_cleanup()`, which in turn is called from
+  :c:func:`__device_release_driver()`.)
+
+* Before a supplier's driver is removed, links to consumers that are not
+  bound to a driver are updated to ``DL_STATE_SUPPLIER_UNBIND``.
+  (Call to :c:func:`device_links_busy()` from
+  :c:func:`__device_release_driver()`.)
+  This prevents the consumers from binding.
+  (Call to :c:func:`device_links_check_suppliers()` from
+  :c:func:`really_probe()`.)
+  Consumers that are bound are freed from their driver; consumers that are
+  probing are waited for until they are done.
+  (Call to :c:func:`device_links_unbind_consumers()` from
+  :c:func:`__device_release_driver()`.)
+  Once all links to consumers are in ``DL_STATE_SUPPLIER_UNBIND`` state,
+  the supplier driver is released and the links revert to ``DL_STATE_DORMANT``.
+  (Call to :c:func:`device_links_driver_cleanup()` from
+  :c:func:`__device_release_driver()`.)
+
+API
+===
+
+.. kernel-doc:: drivers/base/core.c
+   :functions: device_link_add device_link_del
diff --git a/Documentation/core-api/index.rst b/Documentation/core-api/index.rst
index 2872ca1..2aed045 100644
--- a/Documentation/core-api/index.rst
+++ b/Documentation/core-api/index.rst
@@ -16,6 +16,14 @@ Core utilities
    local_ops
    workqueue
 
+Devices
+=======
+
+.. toctree::
+   :maxdepth: 1
+
+   device_link
+
 Interfaces for kernel debugging
 ===============================
 
-- 
2.10.2

Re: [PATCH 1/2] Documentation/core-api/device_link: Add initial documentation

[Mauro Carvalho Chehab]

Em Sun, 4 Dec 2016 13:10:04 +0100
Lukas Wunner <lukas@wunner.de> escreveu:

> Document device links as introduced in v4.10 with commits:
>     4bdb35506b89 ("driver core: Add a wrapper around
>                    __device_release_driver()")
>     9ed9895370ae ("driver core: Functional dependencies tracking
>                    support")
>     8c73b4288496 ("PM / sleep: Make async suspend/resume of devices use
>                    device links")
>     21d5c57b3726 ("PM / runtime: Use device links")
>     baa8809f6097 ("PM / runtime: Optimize the use of device links")
> 
> Cc: Rafael J. Wysocki <rafael@kernel.org>
> Cc: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
> Cc: Luis R. Rodriguez <mcgrof@kernel.org>
> Cc: Jonathan Corbet <corbet@lwn.net>
> Cc: Silvio Fricke <silvio.fricke@gmail.com>
> Cc: Marek Szyprowski <m.szyprowski@samsung.com>
> Cc: Bartlomiej Zolnierkiewicz <b.zolnierkie@samsung.com>
> Cc: Andrzej Hajda <a.hajda@samsung.com>
> Cc: Mauro Carvalho Chehab <mchehab@osg.samsung.com>
> Cc: Inki Dae <inki.dae@samsung.com>
> Cc: Joerg Roedel <joro@8bytes.org>
> Cc: Kukjin Kim <kgene@kernel.org>
> Cc: Krzysztof Kozlowski <krzk@kernel.org>
> Cc: Mark Brown <broonie@kernel.org>
> Cc: Tomeu Vizoso <tomeu.vizoso@collabora.com>
> Cc: Kevin Hilman <khilman@kernel.org>
> Cc: Ulf Hansson <ulf.hansson@linaro.org>
> Cc: Geert Uytterhoeven <geert@linux-m68k.org>
> Cc: Tobias Jakobi <tjakobi@math.uni-bielefeld.de>
> Cc: Tomasz Figa <tomasz.figa@gmail.com>
> Cc: Grant Likely <grant.likely@secretlab.ca>
> Cc: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
> Cc: Lars-Peter Clausen <lars@metafoo.de>
> Cc: Dmitry Torokhov <dmitry.torokhov@gmail.com>
> Cc: Christoph Hellwig <hch@infradead.org>
> Cc: Arnd Bergmann <arnd@arndb.de>
> Cc: Alan Stern <stern@rowland.harvard.edu>
> Cc: Hanjun Guo <guohanjun@huawei.com>
> Cc: linux-pm@vger.kernel.org
> Cc: linux-kernel@vger.kernel.org
> Cc: iommu@lists.linux-foundation.org
> Cc: linux-samsung-soc@vger.kernel.org
> Signed-off-by: Lukas Wunner <lukas@wunner.de>
> ---
>  Documentation/core-api/device_link.rst | 279 +++++++++++++++++++++++++++++++++
>  Documentation/core-api/index.rst       |   8 +
>  2 files changed, 287 insertions(+)
>  create mode 100644 Documentation/core-api/device_link.rst
> 
> diff --git a/Documentation/core-api/device_link.rst b/Documentation/core-api/device_link.rst
> new file mode 100644
> index 0000000..5f57134
> --- /dev/null
> +++ b/Documentation/core-api/device_link.rst
> @@ -0,0 +1,279 @@
> +============
> +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 ``DL_FLAG_STATELESS`` flag.  In other words,
> +enforcing driver presence on the supplier is optional.
> +
> +Another optional feature is runtime PM integration:  By setting the
> +``DL_FLAG_PM_RUNTIME`` flag on addition of the device link, the PM core
> +is instructed to runtime resume the supplier and keep it active
> +whenever and for as long as the consumer is runtime resumed.
> +
> +Usage
> +=====

You should be using, instead:

Usage
-----

(and the same '-' symbol for all sections of this chapter)

The way you did, in thesis, ReST should be putting all tags at the
same level as the first one, as they're all using '='. 

The other ReST markups look OK on my eyes.

Regards,
Mauro

> +
> +The earliest point in time when device links can be added is after
> +:c:func:`device_add()` has been called for the supplier and
> +:c:func:`device_initialize()` has been called for the consumer.
> +
> +It is legal to add them later, but care must be taken that the system
> +remains in a consistent state:  E.g. a device link cannot be added in
> +the midst of a suspend/resume transition, so either commencement of
> +such a transition needs to be prevented with :c:func:`lock_system_sleep()`,
> +or the device link needs to be added from a function which is guaranteed
> +not to run in parallel to a suspend/resume transition, such as from a
> +device ``->probe`` callback or a boot-time PCI quirk.
> +
> +Another example for an inconsistent state would be a device link that
> +represents a driver presence dependency, yet is added from the consumer's
> +``->probe`` callback while the supplier hasn't probed yet:  Had the driver
> +core known about the device link earlier, it wouldn't have probed the
> +consumer in the first place.  The onus is thus on the consumer to check
> +presence of the supplier after adding the link, and defer probing on
> +non-presence.
> +
> +If a device link is added in the ``->probe`` callback of the supplier or
> +consumer driver, it is typically deleted in its ``->remove`` callback for
> +symmetry.  That way, if the driver is compiled as a module, the device
> +link is added on module load and orderly deleted on unload.  The same
> +restrictions that apply to device link addition (e.g. exclusion of a
> +parallel suspend/resume transition) apply equally to deletion.
> +
> +Several flags may be specified on device link addition, two of which
> +have already been mentioned above:  ``DL_FLAG_STATELESS`` to express that no
> +driver presence dependency is needed (but only correct suspend/resume and
> +shutdown ordering) and ``DL_FLAG_PM_RUNTIME`` to express that runtime PM
> +integration is desired.
> +
> +Two other flags are specifically targeted at use cases where the device
> +link is added from the consumer's ``->probe`` callback:  ``DL_FLAG_RPM_ACTIVE``
> +can be specified to runtime resume the supplier upon addition of the
> +device link.  ``DL_FLAG_AUTOREMOVE`` causes the device link to be automatically
> +purged when the consumer fails to probe or later unbinds.  This obviates
> +the need to explicitly delete the link in the ``->remove`` callback or in
> +the error path of the ``->probe`` callback.
> +
> +Limitations
> +===========
> +
> +Driver authors should be aware that a driver presence dependency (i.e. when
> +``DL_FLAG_STATELESS`` is not specified on link addition) may cause probing of
> +the consumer to be deferred indefinitely.  This can become a problem if the
> +consumer is required to probe before a certain initcall level is reached.
> +Worse, if the supplier driver is blacklisted or missing, the consumer will
> +never be probed.
> +
> +Sometimes drivers depend on optional resources.  They are able to operate
> +in a degraded mode (reduced feature set or performance) when those resources
> +are not present.  An example is an SPI controller that can use a DMA engine
> +or work in PIO mode.  The controller can determine presence of the optional
> +resources at probe time but on non-presence there is no way to know whether
> +they will become available in the near future (due to a supplier driver
> +probing) or never.  Consequently it cannot be determined whether to defer
> +probing or not.  It would be possible to notify drivers when optional
> +resources become available after probing, but it would come at a high cost
> +for drivers as switching between modes of operation at runtime based on the
> +availability of such resources would be much more complex than a mechanism
> +based on probe deferral.  In any case optional resources are beyond the
> +scope of device links.
> +
> +Examples
> +========
> +
> +* An MMU device exists alongside a busmaster device, both are in the same
> +  power domain.  The MMU implements DMA address translation for the busmaster
> +  device and shall be runtime resumed and kept active whenever and as long
> +  as the busmaster device is active.  The busmaster device's driver shall
> +  not bind before the MMU is bound.  To achieve this, a device link with
> +  runtime PM integration is added from the busmaster device (consumer)
> +  to the MMU device (supplier).  The effect with regards to runtime PM
> +  is the same as if the MMU was the parent of the master device.
> +
> +  The fact that both devices share the same power domain would normally
> +  suggest usage of a :c:type:`struct dev_pm_domain` or :c:type:`struct
> +  generic_pm_domain`, however these are not independent devices that
> +  happen to share a power switch, but rather the MMU device serves the
> +  busmaster device and is useless without it.  A device link creates a
> +  synthetic hierarchical relationship between the devices and is thus
> +  more apt.
> +
> +* A Thunderbolt host controller comprises a number of PCIe hotplug ports
> +  and an NHI device to manage the PCIe switch.  On resume from system sleep,
> +  the NHI device needs to re-establish PCI tunnels to attached devices
> +  before the hotplug ports can resume.  If the hotplug ports were children
> +  of the NHI, this resume order would automatically be enforced by the
> +  PM core, but unfortunately they're aunts.  The solution is to add
> +  device links from the hotplug ports (consumers) to the NHI device
> +  (supplier).  A driver presence dependency is not necessary for this
> +  use case.
> +
> +* Discrete GPUs in hybrid graphics laptops often feature an HDA controller
> +  for HDMI/DP audio.  In the device hierarchy the HDA controller is a sibling
> +  of the VGA device, yet both share the same power domain and the HDA
> +  controller is only ever needed when an HDMI/DP display is attached to the
> +  VGA device.  A device link from the HDA controller (consumer) to the
> +  VGA device (supplier) aptly represents this relationship.
> +
> +* ACPI allows definition of a device start order by way of _DEP objects.
> +  A classical example is when ACPI power management methods on one device
> +  are implemented in terms of I\ :sup:`2`\ C accesses and require a specific
> +  I\ :sup:`2`\ C controller to be present and functional for the power
> +  management of the device in question to work.
> +
> +* In some SoCs a functional dependency exists from display, video codec and
> +  video processing IP cores on transparent memory access IP cores that handle
> +  burst access and compression/decompression.
> +
> +Alternatives
> +============
> +
> +* A :c:type:`struct dev_pm_domain` can be used to override the bus,
> +  class or device type callbacks.  It is intended for devices sharing
> +  a single on/off switch, however it does not guarantee a specific
> +  suspend/resume ordering, this needs to be implemented separately.
> +  It also does not by itself track the runtime PM status of the involved
> +  devices and turn off the power switch only when all of them are runtime
> +  suspended.  Furthermore it cannot be used to enforce a specific shutdown
> +  ordering or a driver presence dependency.
> +
> +* A :c:type:`struct generic_pm_domain` is a lot more heavyweight than a
> +  device link and does not allow for shutdown ordering or driver presence
> +  dependencies.  It also cannot be used on ACPI systems.
> +
> +Implementation
> +==============
> +
> +The device hierarchy, which -- as the name implies -- is a tree,
> +becomes a directed acyclic graph once device links are added.
> +
> +Ordering of these devices during suspend/resume is determined by the
> +dpm_list.  During shutdown it is determined by the devices_kset.  With
> +no device links present, the two lists are a flattened, one-dimensional
> +representations of the device tree such that a device is placed behind
> +all its ancestors.  That is achieved by traversing the ACPI namespace
> +or OpenFirmware device tree top-down and appending devices to the lists
> +as they are discovered.
> +
> +Once device links are added, the lists need to satisfy the additional
> +constraint that a device is placed behind all its suppliers, recursively.
> +To ensure this, upon addition of the device link the consumer and the
> +entire sub-graph below it (all children and consumers of the consumer)
> +are moved to the end of the list.  (Call to :c:func:`device_reorder_to_tail()`
> +from :c:func:`device_link_add()`.)
> +
> +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 :c:func:`device_is_dependent()` from :c:func:`device_link_add()`.)
> +If that constraint is violated, :c:func:`device_link_add()` will return
> +``NULL`` and a ``WARNING`` will be logged.
> +
> +Notably this also prevents the 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 this case driver authors should weigh
> +carefully if a device link is at all 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.
> +
> +State machine
> +=============
> +
> +.. kernel-doc:: include/linux/device.h
> +   :functions: device_link_state
> +
> +::
> +
> +                 .=============================.
> +                 |                             |
> +                 v                             |
> + DORMANT <=> AVAILABLE <=> CONSUMER_PROBE => ACTIVE
> +    ^                                          |
> +    |                                          |
> +    '============ SUPPLIER_UNBIND <============'
> +
> +* The initial state of a device link is automatically determined by
> +  :c:func:`device_link_add()` based on the driver presence on the supplier
> +  and consumer.  If the link is created before any devices are probed, it
> +  is set to ``DL_STATE_DORMANT``.
> +
> +* When a supplier device is bound to a driver, links to its consumers
> +  progress to ``DL_STATE_AVAILABLE``.
> +  (Call to :c:func:`device_links_driver_bound()` from
> +  :c:func:`driver_bound()`.)
> +
> +* Before a consumer device is probed, presence of supplier drivers is
> +  verified by checking that links to suppliers are in ``DL_STATE_AVAILABLE``
> +  state.  The state of the links is updated to ``DL_STATE_CONSUMER_PROBE``.
> +  (Call to :c:func:`device_links_check_suppliers()` from
> +  :c:func:`really_probe()`.)
> +  This prevents the supplier from unbinding.
> +  (Call to :c:func:`wait_for_device_probe()` from
> +  :c:func:`device_links_unbind_consumers()`.)
> +
> +* If the probe fails, links to suppliers revert back to ``DL_STATE_AVAILABLE``.
> +  (Call to :c:func:`device_links_no_driver()` from :c:func:`really_probe()`.)
> +
> +* If the probe succeeds, links to suppliers progress to ``DL_STATE_ACTIVE``.
> +  (Call to :c:func:`device_links_driver_bound()` from :c:func:`driver_bound()`.)
> +
> +* When the consumer's driver is later on removed, links to suppliers revert
> +  back to ``DL_STATE_AVAILABLE``.
> +  (Call to :c:func:`__device_links_no_driver()` from
> +  :c:func:`device_links_driver_cleanup()`, which in turn is called from
> +  :c:func:`__device_release_driver()`.)
> +
> +* Before a supplier's driver is removed, links to consumers that are not
> +  bound to a driver are updated to ``DL_STATE_SUPPLIER_UNBIND``.
> +  (Call to :c:func:`device_links_busy()` from
> +  :c:func:`__device_release_driver()`.)
> +  This prevents the consumers from binding.
> +  (Call to :c:func:`device_links_check_suppliers()` from
> +  :c:func:`really_probe()`.)
> +  Consumers that are bound are freed from their driver; consumers that are
> +  probing are waited for until they are done.
> +  (Call to :c:func:`device_links_unbind_consumers()` from
> +  :c:func:`__device_release_driver()`.)
> +  Once all links to consumers are in ``DL_STATE_SUPPLIER_UNBIND`` state,
> +  the supplier driver is released and the links revert to ``DL_STATE_DORMANT``.
> +  (Call to :c:func:`device_links_driver_cleanup()` from
> +  :c:func:`__device_release_driver()`.)
> +
> +API
> +===
> +
> +.. kernel-doc:: drivers/base/core.c
> +   :functions: device_link_add device_link_del
> diff --git a/Documentation/core-api/index.rst b/Documentation/core-api/index.rst
> index 2872ca1..2aed045 100644
> --- a/Documentation/core-api/index.rst
> +++ b/Documentation/core-api/index.rst
> @@ -16,6 +16,14 @@ Core utilities
>     local_ops
>     workqueue
>  
> +Devices
> +=======
> +
> +.. toctree::
> +   :maxdepth: 1
> +
> +   device_link
> +
>  Interfaces for kernel debugging
>  ===============================
>  


-- 
Thanks,
Mauro

Re: [PATCH 1/2] Documentation/core-api/device_link: Add initial documentation

[Jonathan Corbet]

On Mon, 5 Dec 2016 10:07:39 -0200
Mauro Carvalho Chehab <mchehab@osg.samsung.com> wrote:

> > +Usage
> > +=====  
> 
> You should be using, instead:
> 
> Usage
> -----
> 
> (and the same '-' symbol for all sections of this chapter)
> 
> The way you did, in thesis, ReST should be putting all tags at the
> same level as the first one, as they're all using '='. 

Actually, he did exactly what the documentation guide says to do.  Since
it only has the markers below the heading, it will still be at a lower
level than the top heading, which has markers both above and below.

jon

Re: [PATCH 1/2] Documentation/core-api/device_link: Add initial documentation

[Mauro Carvalho Chehab]

Em Mon, 5 Dec 2016 06:05:07 -0700
Jonathan Corbet <corbet-T1hC0tSOHrs@public.gmane.org> escreveu:

> On Mon, 5 Dec 2016 10:07:39 -0200
> Mauro Carvalho Chehab <mchehab-JPH+aEBZ4P+UEJcrhfAQsw@public.gmane.org> wrote:
> 
> > > +Usage
> > > +=====  
> > 
> > You should be using, instead:
> > 
> > Usage
> > -----
> > 
> > (and the same '-' symbol for all sections of this chapter)
> > 
> > The way you did, in thesis, ReST should be putting all tags at the
> > same level as the first one, as they're all using '='. 
> 
> Actually, he did exactly what the documentation guide says to do. 

Ah, OK! I guess I misread that section of the documentation.

> Since
> it only has the markers below the heading, it will still be at a lower
> level than the top heading, which has markers both above and below.

Yes, I noticed that it is on a lower level than the initial one with
markers above and below.


Lukas,

Documentation seems OK, from ReST PoV. Didn't read the documents
and the C code to be sure that they match the implementation.

Reviewed-by: Mauro Carvalho Chehab <mchehab-JsYNTwtnfakRB7SZvlqPiA@public.gmane.org>


-- 
Thanks,
Mauro

Re: [PATCH 1/2] Documentation/core-api/device_link: Add initial documentation

[Jonathan Corbet]

On Sun, 4 Dec 2016 13:10:04 +0100
Lukas Wunner <lukas@wunner.de> wrote:

> Document device links as introduced in v4.10 with commits:
>     4bdb35506b89 ("driver core: Add a wrapper around
>                    __device_release_driver()")
>     9ed9895370ae ("driver core: Functional dependencies tracking
>                    support")
>     8c73b4288496 ("PM / sleep: Make async suspend/resume of devices use
>                    device links")
>     21d5c57b3726 ("PM / runtime: Use device links")
>     baa8809f6097 ("PM / runtime: Optimize the use of device links")

I went ahead and applied this one to docs-next.  I did, however, take the
liberty of shifting it over to the driver-api manual.

Thanks,

jon

Re: [PATCH 1/2] Documentation/core-api/device_link: Add initial documentation

[Luis R. Rodriguez]

On Sun, Dec 04, 2016 at 01:10:04PM +0100, Lukas Wunner wrote:
> Document device links as introduced in v4.10 with commits:
>     4bdb35506b89 ("driver core: Add a wrapper around
>                    __device_release_driver()")
>     9ed9895370ae ("driver core: Functional dependencies tracking
>                    support")
>     8c73b4288496 ("PM / sleep: Make async suspend/resume of devices use
>                    device links")
>     21d5c57b3726 ("PM / runtime: Use device links")
>     baa8809f6097 ("PM / runtime: Optimize the use of device links")
> Signed-off-by: Lukas Wunner <lukas-JFq808J9C/izQB+pC5nmwQ@public.gmane.org>

First, thanks for doing this work.

> ---
>  Documentation/core-api/device_link.rst | 279 +++++++++++++++++++++++++++++++++
>  Documentation/core-api/index.rst       |   8 +
>  2 files changed, 287 insertions(+)
>  create mode 100644 Documentation/core-api/device_link.rst
> 
> diff --git a/Documentation/core-api/device_link.rst b/Documentation/core-api/device_link.rst
> new file mode 100644
> index 0000000..5f57134
> --- /dev/null
> +++ b/Documentation/core-api/device_link.rst
> @@ -0,0 +1,279 @@
> +============
> +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: 

This "device hierarchy" was rather confusing to grasp, I specially hard
a hard time understanding *why* the device links work did not do any
topological sorting at all. I'm also afraid this is not just "tribal
knowledge" -- not everyone was able to answer my questions about these
things, Rafafel however did do justice to some degree and I did provide notes
from this. I think it would be important then to jot down to help humans
grok:

  o where the implicit order we embrace comes from
  o what mechanisms are used to help provide some of this order
  o why this is *not* sufficient

The last one justifies the work. And more importantly, it may help
replace a whole bunch of other parallel work which had similar type of
solutions. To what extent it can do that well remains to be seen but
from what I can tell, that's future work worth looking into.

<-- snip -->

> +Limitations
> +===========
> +
> +Driver authors should be aware that a driver presence dependency (i.e. when
> +``DL_FLAG_STATELESS`` is not specified on link addition) may cause probing of
> +the consumer to be deferred indefinitely.  This can become a problem if the
> +consumer is required to probe before a certain initcall level is reached.
> +Worse, if the supplier driver is blacklisted or missing, the consumer will
> +never be probed.
> +
> +Sometimes drivers depend on optional resources.  They are able to operate
> +in a degraded mode (reduced feature set or performance) when those resources
> +are not present.  An example is an SPI controller that can use a DMA engine
> +or work in PIO mode.  The controller can determine presence of the optional
> +resources at probe time but on non-presence there is no way to know whether
> +they will become available in the near future (due to a supplier driver
> +probing) or never.  Consequently it cannot be determined whether to defer
> +probing or not.  It would be possible to notify drivers when optional
> +resources become available after probing, but it would come at a high cost
> +for drivers as switching between modes of operation at runtime based on the
> +availability of such resources would be much more complex than a mechanism
> +based on probe deferral.  In any case optional resources are beyond the
> +scope of device links.

Well you also forgot to mention the issues with deferred probe. It uses
deferred probe so if for whatever reason your subsystem or driver has issues
with it, this won't help.

<-- snip -->
> +* ACPI allows definition of a device start order by way of _DEP objects.
> +  A classical example is when ACPI power management methods on one device
> +  are implemented in terms of I\ :sup:`2`\ C accesses and require a specific
> +  I\ :sup:`2`\ C controller to be present and functional for the power
> +  management of the device in question to work.

Here is an example of current mechanisms used which to driver developers
provides implicit order --its all magical. This has can have limitations,
its important to annotate how this is limiting and also why this perhaps
was not considered as an enhancement in ACPI space. Ie, if semantics
existed in ACPI space for dependency info, why are we now left to our hims
for some cases on driver code?

> +
> +Alternatives
> +============
> +
> +* A :c:type:`struct dev_pm_domain` can be used to override the bus,
> +  class or device type callbacks.  It is intended for devices sharing
> +  a single on/off switch, however it does not guarantee a specific
> +  suspend/resume ordering, this needs to be implemented separately.
> +  It also does not by itself track the runtime PM status of the involved
> +  devices and turn off the power switch only when all of them are runtime
> +  suspended.  Furthermore it cannot be used to enforce a specific shutdown
> +  ordering or a driver presence dependency.
> +
> +* A :c:type:`struct generic_pm_domain` is a lot more heavyweight than a
> +  device link and does not allow for shutdown ordering or driver presence
> +  dependencies.  It also cannot be used on ACPI systems.

I provided a list of other frameworks in the kernel which have
their own solutions which are worth looking into, if anything to at
least determine if we have older frameworks to replace or to use
them to help complement device links framework to help with other
areas other "firmware tools" clearly are lacking.

> +
> +Implementation
> +==============
> +
> +The device hierarchy, which -- as the name implies -- is a tree,
> +becomes a directed acyclic graph once device links are added.

<insert sarcasm>

Oh look a DAG without any sort. How did that happen?

</end sarcasm>
> +
> +Ordering of these devices during suspend/resume is determined by the
> +dpm_list.

And where did order from that come from? My notes provided answers to
these questions.

  Luis

[PATCH 2/2] driver core: Silence device links sphinx warning

[Lukas Wunner]

Silence this warning emitted by sphinx:
include/linux/device.h:938: warning: No description found for parameter 'links'

While at it, fix typos in comments of device links code.

Cc: Rafael J. Wysocki <rafael-DgEjT+Ai2ygdnm+yROfE0A@public.gmane.org>
Cc: Greg Kroah-Hartman <gregkh-hQyY1W1yCW8ekmWlsbkhG0B+6BGkLq7r@public.gmane.org>
Cc: Jonathan Corbet <corbet-T1hC0tSOHrs@public.gmane.org>
Cc: Silvio Fricke <silvio.fricke-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>
Signed-off-by: Lukas Wunner <lukas-JFq808J9C/izQB+pC5nmwQ@public.gmane.org>
---
 drivers/base/core.c    | 4 ++--
 include/linux/device.h | 1 +
 2 files changed, 3 insertions(+), 2 deletions(-)

diff --git a/drivers/base/core.c b/drivers/base/core.c
index d0c9df5..8c25e68 100644
--- a/drivers/base/core.c
+++ b/drivers/base/core.c
@@ -172,7 +172,7 @@ static int device_reorder_to_tail(struct device *dev, void *not_used)
  *
  * The supplier device is required to be registered when this function is called
  * and NULL will be returned if that is not the case.  The consumer device need
- * not be registerd, however.
+ * not be registered, however.
  */
 struct device_link *device_link_add(struct device *consumer,
 				    struct device *supplier, u32 flags)
@@ -225,7 +225,7 @@ struct device_link *device_link_add(struct device *consumer,
 	INIT_LIST_HEAD(&link->c_node);
 	link->flags = flags;
 
-	/* Deterine the initial link state. */
+	/* Determine the initial link state. */
 	if (flags & DL_FLAG_STATELESS) {
 		link->status = DL_STATE_NONE;
 	} else {
diff --git a/include/linux/device.h b/include/linux/device.h
index 3896af4..87edfdf 100644
--- a/include/linux/device.h
+++ b/include/linux/device.h
@@ -813,6 +813,7 @@ struct dev_links_info {
  * 		on.  This shrinks the "Board Support Packages" (BSPs) and
  * 		minimizes board-specific #ifdefs in drivers.
  * @driver_data: Private pointer for driver specific info.
+ * @links:	Links to suppliers and consumers of this device.
  * @power:	For device power management.
  * 		See Documentation/power/admin-guide/devices.rst for details.
  * @pm_domain:	Provide callbacks that are executed during system suspend,
-- 
2.10.2

Re: [PATCH 2/2] driver core: Silence device links sphinx warning

[Mauro Carvalho Chehab]

Em Sun, 4 Dec 2016 13:10:04 +0100
Lukas Wunner <lukas@wunner.de> escreveu:

> Silence this warning emitted by sphinx:
> include/linux/device.h:938: warning: No description found for parameter 'links'
> 
> While at it, fix typos in comments of device links code.
> 
> Cc: Rafael J. Wysocki <rafael@kernel.org>
> Cc: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
> Cc: Jonathan Corbet <corbet@lwn.net>
> Cc: Silvio Fricke <silvio.fricke@gmail.com>
> Signed-off-by: Lukas Wunner <lukas@wunner.de>
> ---
>  drivers/base/core.c    | 4 ++--
>  include/linux/device.h | 1 +
>  2 files changed, 3 insertions(+), 2 deletions(-)
> 
> diff --git a/drivers/base/core.c b/drivers/base/core.c
> index d0c9df5..8c25e68 100644
> --- a/drivers/base/core.c
> +++ b/drivers/base/core.c
> @@ -172,7 +172,7 @@ static int device_reorder_to_tail(struct device *dev, void *not_used)
>   *
>   * The supplier device is required to be registered when this function is called
>   * and NULL will be returned if that is not the case.  The consumer device need
> - * not be registerd, however.
> + * not be registered, however.
>   */
>  struct device_link *device_link_add(struct device *consumer,
>  				    struct device *supplier, u32 flags)
> @@ -225,7 +225,7 @@ struct device_link *device_link_add(struct device *consumer,
>  	INIT_LIST_HEAD(&link->c_node);
>  	link->flags = flags;
>  
> -	/* Deterine the initial link state. */
> +	/* Determine the initial link state. */
>  	if (flags & DL_FLAG_STATELESS) {
>  		link->status = DL_STATE_NONE;
>  	} else {
> diff --git a/include/linux/device.h b/include/linux/device.h
> index 3896af4..87edfdf 100644
> --- a/include/linux/device.h
> +++ b/include/linux/device.h
> @@ -813,6 +813,7 @@ struct dev_links_info {
>   * 		on.  This shrinks the "Board Support Packages" (BSPs) and
>   * 		minimizes board-specific #ifdefs in drivers.
>   * @driver_data: Private pointer for driver specific info.
> + * @links:	Links to suppliers and consumers of this device.
>   * @power:	For device power management.
>   * 		See Documentation/power/admin-guide/devices.rst for details.
>   * @pm_domain:	Provide callbacks that are executed during system suspend,

Hmm... I'm not seeing "links" at driver-core-next:
	https://git.kernel.org/cgit/linux/kernel/git/gregkh/driver-core.git/tree/include/linux/device.h?h=driver-core-next

On what tree did you base this patch?

-- 
Thanks,
Mauro

Re: [PATCH 2/2] driver core: Silence device links sphinx warning

[Lukas Wunner]

On Mon, Dec 05, 2016 at 10:20:53AM -0200, Mauro Carvalho Chehab wrote:
> Em Sun, 4 Dec 2016 13:10:04 +0100 Lukas Wunner <lukas-JFq808J9C/izQB+pC5nmwQ@public.gmane.org> escreveu:
> > Silence this warning emitted by sphinx:
> > include/linux/device.h:938: warning: No description found for parameter 'links'
> > 
> > While at it, fix typos in comments of device links code.
> > 
> > Cc: Rafael J. Wysocki <rafael-DgEjT+Ai2ygdnm+yROfE0A@public.gmane.org>
> > Cc: Greg Kroah-Hartman <gregkh-hQyY1W1yCW8ekmWlsbkhG0B+6BGkLq7r@public.gmane.org>
> > Cc: Jonathan Corbet <corbet-T1hC0tSOHrs@public.gmane.org>
> > Cc: Silvio Fricke <silvio.fricke-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>
> > Signed-off-by: Lukas Wunner <lukas-JFq808J9C/izQB+pC5nmwQ@public.gmane.org>
> > ---
> >  drivers/base/core.c    | 4 ++--
> >  include/linux/device.h | 1 +
> >  2 files changed, 3 insertions(+), 2 deletions(-)
> > 
> > diff --git a/drivers/base/core.c b/drivers/base/core.c
> > index d0c9df5..8c25e68 100644
> > --- a/drivers/base/core.c
> > +++ b/drivers/base/core.c
> > @@ -172,7 +172,7 @@ static int device_reorder_to_tail(struct device *dev, void *not_used)
> >   *
> >   * The supplier device is required to be registered when this function is called
> >   * and NULL will be returned if that is not the case.  The consumer device need
> > - * not be registerd, however.
> > + * not be registered, however.
> >   */
> >  struct device_link *device_link_add(struct device *consumer,
> >  				    struct device *supplier, u32 flags)
> > @@ -225,7 +225,7 @@ struct device_link *device_link_add(struct device *consumer,
> >  	INIT_LIST_HEAD(&link->c_node);
> >  	link->flags = flags;
> >  
> > -	/* Deterine the initial link state. */
> > +	/* Determine the initial link state. */
> >  	if (flags & DL_FLAG_STATELESS) {
> >  		link->status = DL_STATE_NONE;
> >  	} else {
> > diff --git a/include/linux/device.h b/include/linux/device.h
> > index 3896af4..87edfdf 100644
> > --- a/include/linux/device.h
> > +++ b/include/linux/device.h
> > @@ -813,6 +813,7 @@ struct dev_links_info {
> >   * 		on.  This shrinks the "Board Support Packages" (BSPs) and
> >   * 		minimizes board-specific #ifdefs in drivers.
> >   * @driver_data: Private pointer for driver specific info.
> > + * @links:	Links to suppliers and consumers of this device.
> >   * @power:	For device power management.
> >   * 		See Documentation/power/admin-guide/devices.rst for details.
> >   * @pm_domain:	Provide callbacks that are executed during system suspend,
> 
> Hmm... I'm not seeing "links" at driver-core-next:
> 	https://git.kernel.org/cgit/linux/kernel/git/gregkh/driver-core.git/tree/include/linux/device.h?h=driver-core-next
> 
> On what tree did you base this patch?

You're looking at the right tree, just maybe not the right line. :-)
It's in line 887:

	https://git.kernel.org/cgit/linux/kernel/git/gregkh/driver-core.git/tree/include/linux/device.h?h=driver-core-next#n887

Thanks,

Lukas

Re: [PATCH 2/2] driver core: Silence device links sphinx warning

[Mauro Carvalho Chehab]

Em Mon, 5 Dec 2016 13:44:49 +0100
Lukas Wunner <lukas-JFq808J9C/izQB+pC5nmwQ@public.gmane.org> escreveu:

> On Mon, Dec 05, 2016 at 10:20:53AM -0200, Mauro Carvalho Chehab wrote:
> > Em Sun, 4 Dec 2016 13:10:04 +0100 Lukas Wunner <lukas-JFq808J9C/izQB+pC5nmwQ@public.gmane.org> escreveu:  
> > > Silence this warning emitted by sphinx:
> > > include/linux/device.h:938: warning: No description found for parameter 'links'
> > > 
> > > While at it, fix typos in comments of device links code.
> > > 
> > > Cc: Rafael J. Wysocki <rafael-DgEjT+Ai2ygdnm+yROfE0A@public.gmane.org>
> > > Cc: Greg Kroah-Hartman <gregkh-hQyY1W1yCW8ekmWlsbkhG0B+6BGkLq7r@public.gmane.org>
> > > Cc: Jonathan Corbet <corbet-T1hC0tSOHrs@public.gmane.org>
> > > Cc: Silvio Fricke <silvio.fricke-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>
> > > Signed-off-by: Lukas Wunner <lukas-JFq808J9C/izQB+pC5nmwQ@public.gmane.org>
> > > ---
> > >  drivers/base/core.c    | 4 ++--
> > >  include/linux/device.h | 1 +
> > >  2 files changed, 3 insertions(+), 2 deletions(-)
> > > 
> > > diff --git a/drivers/base/core.c b/drivers/base/core.c
> > > index d0c9df5..8c25e68 100644
> > > --- a/drivers/base/core.c
> > > +++ b/drivers/base/core.c
> > > @@ -172,7 +172,7 @@ static int device_reorder_to_tail(struct device *dev, void *not_used)
> > >   *
> > >   * The supplier device is required to be registered when this function is called
> > >   * and NULL will be returned if that is not the case.  The consumer device need
> > > - * not be registerd, however.
> > > + * not be registered, however.
> > >   */
> > >  struct device_link *device_link_add(struct device *consumer,
> > >  				    struct device *supplier, u32 flags)
> > > @@ -225,7 +225,7 @@ struct device_link *device_link_add(struct device *consumer,
> > >  	INIT_LIST_HEAD(&link->c_node);
> > >  	link->flags = flags;
> > >  
> > > -	/* Deterine the initial link state. */
> > > +	/* Determine the initial link state. */
> > >  	if (flags & DL_FLAG_STATELESS) {
> > >  		link->status = DL_STATE_NONE;
> > >  	} else {
> > > diff --git a/include/linux/device.h b/include/linux/device.h
> > > index 3896af4..87edfdf 100644
> > > --- a/include/linux/device.h
> > > +++ b/include/linux/device.h
> > > @@ -813,6 +813,7 @@ struct dev_links_info {
> > >   * 		on.  This shrinks the "Board Support Packages" (BSPs) and
> > >   * 		minimizes board-specific #ifdefs in drivers.
> > >   * @driver_data: Private pointer for driver specific info.
> > > + * @links:	Links to suppliers and consumers of this device.
> > >   * @power:	For device power management.
> > >   * 		See Documentation/power/admin-guide/devices.rst for details.
> > >   * @pm_domain:	Provide callbacks that are executed during system suspend,  
> > 
> > Hmm... I'm not seeing "links" at driver-core-next:
> > 	https://git.kernel.org/cgit/linux/kernel/git/gregkh/driver-core.git/tree/include/linux/device.h?h=driver-core-next
> > 
> > On what tree did you base this patch?  
> 
> You're looking at the right tree, just maybe not the right line. :-)

Ah, OK! yeah, I was tricked by this:
	@@ -813,6 +813,7 @@ struct dev_links_info {

Need more caffeine ;)

> It's in line 887:
> 
> 	https://git.kernel.org/cgit/linux/kernel/git/gregkh/driver-core.git/tree/include/linux/device.h?h=driver-core-next#n887

Patch looks good on my eyes.

Reviewed-by: Mauro Carvalho Chehab <mchehab-JsYNTwtnfakRB7SZvlqPiA@public.gmane.org>

-- 
Thanks,
Mauro

Re: [PATCH 2/2] driver core: Silence device links sphinx warning

[Greg Kroah-Hartman]

On Sun, Dec 04, 2016 at 01:10:04PM +0100, Lukas Wunner wrote:
> Silence this warning emitted by sphinx:
> include/linux/device.h:938: warning: No description found for parameter 'links'
> 
> While at it, fix typos in comments of device links code.
> 
> Cc: Rafael J. Wysocki <rafael-DgEjT+Ai2ygdnm+yROfE0A@public.gmane.org>
> Cc: Greg Kroah-Hartman <gregkh-hQyY1W1yCW8ekmWlsbkhG0B+6BGkLq7r@public.gmane.org>
> Cc: Jonathan Corbet <corbet-T1hC0tSOHrs@public.gmane.org>
> Cc: Silvio Fricke <silvio.fricke-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>
> Signed-off-by: Lukas Wunner <lukas-JFq808J9C/izQB+pC5nmwQ@public.gmane.org>
> ---
>  drivers/base/core.c    | 4 ++--
>  include/linux/device.h | 1 +
>  2 files changed, 3 insertions(+), 2 deletions(-)

I'll take this patch now to keep the warnings from showing up in
4.10-rc1.

thanks,

greg k-h

Re: [PATCH 2/2] driver core: Silence device links sphinx warning

[Rafael J. Wysocki]

On Sun, Dec 4, 2016 at 1:10 PM, Lukas Wunner <lukas@wunner.de> wrote:
> Silence this warning emitted by sphinx:
> include/linux/device.h:938: warning: No description found for parameter 'links'
>
> While at it, fix typos in comments of device links code.
>
> Cc: Rafael J. Wysocki <rafael@kernel.org>
> Cc: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
> Cc: Jonathan Corbet <corbet@lwn.net>
> Cc: Silvio Fricke <silvio.fricke@gmail.com>
> Signed-off-by: Lukas Wunner <lukas@wunner.de>

Acked-by: Rafael J. Wysocki <rafael.j.wysocki@intel.com>

and thanks for fixing these!

[Adds note to self to spellcheck patches next time.]

> ---
>  drivers/base/core.c    | 4 ++--
>  include/linux/device.h | 1 +
>  2 files changed, 3 insertions(+), 2 deletions(-)
>
> diff --git a/drivers/base/core.c b/drivers/base/core.c
> index d0c9df5..8c25e68 100644
> --- a/drivers/base/core.c
> +++ b/drivers/base/core.c
> @@ -172,7 +172,7 @@ static int device_reorder_to_tail(struct device *dev, void *not_used)
>   *
>   * The supplier device is required to be registered when this function is called
>   * and NULL will be returned if that is not the case.  The consumer device need
> - * not be registerd, however.
> + * not be registered, however.
>   */
>  struct device_link *device_link_add(struct device *consumer,
>                                     struct device *supplier, u32 flags)
> @@ -225,7 +225,7 @@ struct device_link *device_link_add(struct device *consumer,
>         INIT_LIST_HEAD(&link->c_node);
>         link->flags = flags;
>
> -       /* Deterine the initial link state. */
> +       /* Determine the initial link state. */
>         if (flags & DL_FLAG_STATELESS) {
>                 link->status = DL_STATE_NONE;
>         } else {
> diff --git a/include/linux/device.h b/include/linux/device.h
> index 3896af4..87edfdf 100644
> --- a/include/linux/device.h
> +++ b/include/linux/device.h
> @@ -813,6 +813,7 @@ struct dev_links_info {
>   *             on.  This shrinks the "Board Support Packages" (BSPs) and
>   *             minimizes board-specific #ifdefs in drivers.
>   * @driver_data: Private pointer for driver specific info.
> + * @links:     Links to suppliers and consumers of this device.
>   * @power:     For device power management.
>   *             See Documentation/power/admin-guide/devices.rst for details.
>   * @pm_domain: Provide callbacks that are executed during system suspend,
> --
> 2.10.2
>

Re: [PATCH 0/2] Device links documentation

[Mauro Carvalho Chehab]

Hi Lukas,

Em Sun, 4 Dec 2016 13:10:04 +0100
Lukas Wunner <lukas@wunner.de> escreveu:

> Here's my suggestion for initial documentation on the device links
> feature that's queued for 4.10 on Greg KH's driver-core-next branch.
> Please let me know if you have any additions or corrections.
> 
> To read this in rendered form:
> http://wunner.de/kernel-doc/core-api/device_link.html
> 
> Patch [2/2] could go in via driver-core-next, but patch [1/2] would
> need to go in via docs-next (because the core-api directory doesn't
> exist yet in driver-core-next).
>
> @Jonathan Corbet:  Is core-api the right place to put this? An
> alternative would be Documentation/driver-api, but unlike core-api
> it contains less prose text but mostly just bare API docs gleaned
> from kernel-doc.

Just my 2 cents here, but, IMHO, this belongs to driver-api.
Yeah, it contains less prose, with is, IMHO, something that should
be improved ;)

Regards,
Mauro

Re: [PATCH 0/2] Device links documentation

[Jonathan Corbet]

[Trimming the CC list a bit; claws-mail chokes on the full thing for some
reason]

On Sun, 4 Dec 2016 13:10:04 +0100
Lukas Wunner <lukas@wunner.de> wrote:

> @Jonathan Corbet:  Is core-api the right place to put this? An
> alternative would be Documentation/driver-api, but unlike core-api
> it contains less prose text but mostly just bare API docs gleaned
> from kernel-doc.

The amount of prose isn't really the determining factor here...  To me,
this is very much driver-oriented documentation, so I would rather see it
going into the driver-api sub-book.

Thanks for putting it together,

jon