linux-pm.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
From: "Luis R. Rodriguez" <mcgrof-DgEjT+Ai2ygdnm+yROfE0A@public.gmane.org>
To: Lukas Wunner <lukas-JFq808J9C/izQB+pC5nmwQ@public.gmane.org>
Cc: Ulf Hansson <ulf.hansson-QSEj5FYQhm4dnm+yROfE0A@public.gmane.org>,
	Dmitry Torokhov
	<dmitry.torokhov-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>,
	"Rafael J. Wysocki"
	<rafael-DgEjT+Ai2ygdnm+yROfE0A@public.gmane.org>,
	Tomasz Figa <tomasz.figa-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>,
	Grant Likely
	<grant.likely-s3s/WqlpOiPyB63q8FvJNQ@public.gmane.org>,
	Andrzej Hajda <a.hajda-Sze3O3UU22JBDgjK7y7TUQ@public.gmane.org>,
	Laurent Pinchart
	<laurent.pinchart-ryLnwIuWjnjg/C1BVhZhaw@public.gmane.org>,
	Hanjun Guo <guohanjun-hv44wF8Li93QT0dZR+AlfA@public.gmane.org>,
	Lars-Peter Clausen <lars-Qo5EllUWu/uELgA04lAiVw@public.gmane.org>,
	Jonathan Corbet <corbet-T1hC0tSOHrs@public.gmane.org>,
	Mauro Carvalho Chehab
	<mchehab-JPH+aEBZ4P+UEJcrhfAQsw@public.gmane.org>,
	iommu-cunTk1MwBs9QetFLy7KEm3xJsTq8ys+cHZ5vskTnxNA@public.gmane.org,
	Krzysztof Kozlowski
	<krzk-DgEjT+Ai2ygdnm+yROfE0A@public.gmane.org>,
	Christoph Hellwig <hch-wEGCiKHe2LqWVfeAwA7xHQ@public.gmane.org>,
	linux-samsung-soc-u79uwXL29TY76Z2rM5mHXA@public.gmane.org,
	Kukjin Kim <kgene-DgEjT+Ai2ygdnm+yROfE0A@public.gmane.org>,
	Geert Uytterhoeven
	<geert-Td1EMuHUCqxL1ZNQvxDV9g@public.gmane.org>,
	Alan Stern
	<stern-nwvwT67g6+6dFdvTe/nMLpVzexx5G7lz@public.gmane.org>,
	Arnd Bergmann <arnd-r2nGTMty4D4@public.gmane.org>,
	Bartlomiej Zolnierkiewicz
	<b.zolnierkie-Sze3O3UU22JBDgjK7y7TUQ@public.gmane.org>,
	Inki Dae <inki.dae-Sze3O3UU22JBDgjK7y7TUQ@public.gmane.org>,
	Mark Brown
	<broonie-DgEjT+Ai2ygdnm+yROfE0A@public.gmane.org>Tomeu Vizoso
	<tom>
Subject: Re: [PATCH 1/2] Documentation/core-api/device_link: Add initial documentation
Date: Tue, 6 Dec 2016 02:41:50 +0100	[thread overview]
Message-ID: <20161206014150.GY1402@wotan.suse.de> (raw)
In-Reply-To: <f5b568e349aaeb31039a0c4d01fdb05d8b3bdd2a.1480849144.git.lukas-JFq808J9C/izQB+pC5nmwQ@public.gmane.org>

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

  parent reply	other threads:[~2016-12-06  1:41 UTC|newest]

Thread overview: 13+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
     [not found] <cover.1480849144.git.lukas@wunner.de>
2016-12-05 12:03 ` [PATCH 0/2] Device links documentation Mauro Carvalho Chehab
2016-12-05 13:09 ` Jonathan Corbet
     [not found] ` <f5b568e349aaeb31039a0c4d01fdb05d8b3bdd2a.1480849144.git.lukas@wunner.de>
2016-12-05 12:07   ` [PATCH 1/2] Documentation/core-api/device_link: Add initial documentation Mauro Carvalho Chehab
2016-12-05 13:05     ` Jonathan Corbet
     [not found]       ` <20161205060507.6bd6e944-T1hC0tSOHrs@public.gmane.org>
2016-12-05 13:36         ` Mauro Carvalho Chehab
2016-12-05 21:15   ` Jonathan Corbet
     [not found]   ` <f5b568e349aaeb31039a0c4d01fdb05d8b3bdd2a.1480849144.git.lukas-JFq808J9C/izQB+pC5nmwQ@public.gmane.org>
2016-12-06  1:41     ` Luis R. Rodriguez [this message]
2016-12-07 11:50       ` Lukas Wunner
     [not found] ` <a1c7a93ee8ef057fb70aa3abeed38daf989444fc.1480849144.git.lukas@wunner.de>
2016-12-05 12:20   ` [PATCH 2/2] driver core: Silence device links sphinx warning Mauro Carvalho Chehab
     [not found]     ` <20161205102030.2abae2b5-ch4gOOMV7nf/PtFMR13I2A@public.gmane.org>
2016-12-05 12:44       ` Lukas Wunner
     [not found]         ` <20161205124449.GA7539-JFq808J9C/izQB+pC5nmwQ@public.gmane.org>
2016-12-05 12:57           ` Mauro Carvalho Chehab
     [not found]   ` <a1c7a93ee8ef057fb70aa3abeed38daf989444fc.1480849144.git.lukas-JFq808J9C/izQB+pC5nmwQ@public.gmane.org>
2016-12-05 13:59     ` Greg Kroah-Hartman
2016-12-05 22:21   ` Rafael J. Wysocki

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20161206014150.GY1402@wotan.suse.de \
    --to=mcgrof-dgejt+ai2ygdnm+yrofe0a@public.gmane.org \
    --cc=a.hajda-Sze3O3UU22JBDgjK7y7TUQ@public.gmane.org \
    --cc=arnd-r2nGTMty4D4@public.gmane.org \
    --cc=b.zolnierkie-Sze3O3UU22JBDgjK7y7TUQ@public.gmane.org \
    --cc=broonie-DgEjT+Ai2ygdnm+yROfE0A@public.gmane.org \
    --cc=corbet-T1hC0tSOHrs@public.gmane.org \
    --cc=dmitry.torokhov-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org \
    --cc=geert-Td1EMuHUCqxL1ZNQvxDV9g@public.gmane.org \
    --cc=grant.likely-s3s/WqlpOiPyB63q8FvJNQ@public.gmane.org \
    --cc=guohanjun-hv44wF8Li93QT0dZR+AlfA@public.gmane.org \
    --cc=hch-wEGCiKHe2LqWVfeAwA7xHQ@public.gmane.org \
    --cc=inki.dae-Sze3O3UU22JBDgjK7y7TUQ@public.gmane.org \
    --cc=iommu-cunTk1MwBs9QetFLy7KEm3xJsTq8ys+cHZ5vskTnxNA@public.gmane.org \
    --cc=kgene-DgEjT+Ai2ygdnm+yROfE0A@public.gmane.org \
    --cc=krzk-DgEjT+Ai2ygdnm+yROfE0A@public.gmane.org \
    --cc=lars-Qo5EllUWu/uELgA04lAiVw@public.gmane.org \
    --cc=laurent.pinchart-ryLnwIuWjnjg/C1BVhZhaw@public.gmane.org \
    --cc=linux-samsung-soc-u79uwXL29TY76Z2rM5mHXA@public.gmane.org \
    --cc=lukas-JFq808J9C/izQB+pC5nmwQ@public.gmane.org \
    --cc=mchehab-JPH+aEBZ4P+UEJcrhfAQsw@public.gmane.org \
    --cc=rafael-DgEjT+Ai2ygdnm+yROfE0A@public.gmane.org \
    --cc=stern-nwvwT67g6+6dFdvTe/nMLpVzexx5G7lz@public.gmane.org \
    --cc=tomasz.figa-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org \
    --cc=ulf.hansson-QSEj5FYQhm4dnm+yROfE0A@public.gmane.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).