All of lore.kernel.org
 help / color / mirror / Atom feed
From: Dmitry Torokhov <dmitry.torokhov@gmail.com>
To: Ulf Hansson <ulf.hansson@linaro.org>
Cc: "Rafael J. Wysocki" <rjw@rjwysocki.net>,
	Kevin Hilman <khilman@kernel.org>,
	Len Brown <len.brown@intel.com>, Pavel Machek <pavel@ucw.cz>,
	"linux-pm@vger.kernel.org" <linux-pm@vger.kernel.org>,
	"linux-arm-kernel@lists.infradead.org"
	<linux-arm-kernel@lists.infradead.org>,
	linux-samsung-soc <linux-samsung-soc@vger.kernel.org>,
	Geert Uytterhoeven <geert+renesas@glider.be>,
	Alan Stern <stern@rowland.harvard.edu>,
	Greg Kroah-Hartman <gregkh@linuxfoundation.org>,
	Tomasz Figa <tomasz.figa@gmail.com>,
	Simon Horman <horms@verge.net.au>,
	Magnus Damm <magnus.damm@gmail.com>,
	Ben Dooks <ben-linux@fluff.org>,
	Kukjin Kim <kgene.kim@samsung.com>,
	Philipp Zabel <philipp.zabel@gmail.com>,
	Mark Brown <broonie@kernel.org>, Wolfram Sang <wsa@the-dreams.de>,
	Russell King <linux@arm.linux.org.uk>,
	Jack Dai <jack.dai@rock-chips.com>,
	Jinkun Hong <jinkun.hong@rock-chips.com>,
	Aaron
Subject: Re: [PATCH v3 0/9] PM / Domains: Fix race conditions during boot
Date: Tue, 4 Nov 2014 01:05:21 -0800	[thread overview]
Message-ID: <20141104090521.GA13219@dtor-glaptop> (raw)
In-Reply-To: <CAPDyKFp=T8Luomeaoz5Y=eLAuFaOfdb+-aoU3QVnpjoDYVKPjg@mail.gmail.com>

On Tue, Nov 04, 2014 at 09:54:19AM +0100, Ulf Hansson wrote:
> [...]
> 
> > Generally, there are two or even three levels of runtime PM handling,
> > driver, (possibly) bus type and (possibly) PM domain (and multiple levels
> > of these are possible in principle).  All of them have to be initialized
> > at different times.
> >
> > Quite arguably, the PM domain and/or bus type runtime PM handling should
> > be initialized even before registerind the device or during device
> > registration.  Doing that later may be too late.  When the device has been
> > registered, runtime PM should work to an extent allowing the driver to access
> > the device and configure it further after calling pm_runtime_resume().
> >
> > Of course, if ->probe() is to call pm_runtime_resume() for this purpose,
> > it must take the fact that the driver's own ->runtime_resume() may be called
> > as a result of this into account.  That's why I'm asking whether or not the
> > core should call pm_runtime_resume() before calling really_probe() in a
> > followup branch of this thread.
> 
> I am reading the other thread, let's see.
> 
> >
> > The driver's own runtime PM handling must be initialized in the driver and
> > the only place suitable for that is ->probe().  However, it needs to be done
> > *before* the driver's own ->runtime_resume() or ->runtime_suspend() callback
> > is executed.  If that is done properly, it should be possible to cover
> > both the CONFIG_PM_RUNTIME set/unset cases in that code.
> >
> > And I wouldn't recommend anyone to do the runtime PM initialization in
> > ->runtime_resume() (when it is called for the first time), as that would be
> > error prone and fragile.
> 
> Great! That's means we are at least aligned on this topic. :-)
> 
> >
> >> The AMBA bus and some of its drivers a good example of how this has
> >> been implemented:
> >> driver/amba/bus.c
> >> drivers/mmc/host/mmci.c
> >> drivers/spi/spi-pl022.c
> >>
> >> This conclusion I have made from this is:
> >> - Using pm_runtime_get_sync() during the ->probe() path to explicitly
> >> power up a PM domain, is not suitable as the _common_ solution to
> >> solve the race condition. It certainly may work for some scenarios,
> >> but not for those I am looking at.
> >
> > I think, however, that it might work if the core calls pm_runtime_get_sync()
> > from driver_probe_device().
> 
> Currently this won't work.
> 
> That's because the buses' ->probe() are invoked in this path and they
> are doing the attachment of the device to its PM domain.
> 
> In other words, we can't power up the PM domain using
> pm_runtime_get_sync(), until the device has been attached to its PM
> domain. Right?


I think this is one of the issues that we have there. Why do we conflate
probing and placing the device into a power domain? The latter should
happen when we register the device. The fact that a device was probed
and has a driver bound or not bound to it should have no bearing on
whether the device is member of power domain or not.

Thanks.

-- 
Dmitry

WARNING: multiple messages have this Message-ID (diff)
From: dmitry.torokhov@gmail.com (Dmitry Torokhov)
To: linux-arm-kernel@lists.infradead.org
Subject: [PATCH v3 0/9] PM / Domains: Fix race conditions during boot
Date: Tue, 4 Nov 2014 01:05:21 -0800	[thread overview]
Message-ID: <20141104090521.GA13219@dtor-glaptop> (raw)
In-Reply-To: <CAPDyKFp=T8Luomeaoz5Y=eLAuFaOfdb+-aoU3QVnpjoDYVKPjg@mail.gmail.com>

On Tue, Nov 04, 2014 at 09:54:19AM +0100, Ulf Hansson wrote:
> [...]
> 
> > Generally, there are two or even three levels of runtime PM handling,
> > driver, (possibly) bus type and (possibly) PM domain (and multiple levels
> > of these are possible in principle).  All of them have to be initialized
> > at different times.
> >
> > Quite arguably, the PM domain and/or bus type runtime PM handling should
> > be initialized even before registerind the device or during device
> > registration.  Doing that later may be too late.  When the device has been
> > registered, runtime PM should work to an extent allowing the driver to access
> > the device and configure it further after calling pm_runtime_resume().
> >
> > Of course, if ->probe() is to call pm_runtime_resume() for this purpose,
> > it must take the fact that the driver's own ->runtime_resume() may be called
> > as a result of this into account.  That's why I'm asking whether or not the
> > core should call pm_runtime_resume() before calling really_probe() in a
> > followup branch of this thread.
> 
> I am reading the other thread, let's see.
> 
> >
> > The driver's own runtime PM handling must be initialized in the driver and
> > the only place suitable for that is ->probe().  However, it needs to be done
> > *before* the driver's own ->runtime_resume() or ->runtime_suspend() callback
> > is executed.  If that is done properly, it should be possible to cover
> > both the CONFIG_PM_RUNTIME set/unset cases in that code.
> >
> > And I wouldn't recommend anyone to do the runtime PM initialization in
> > ->runtime_resume() (when it is called for the first time), as that would be
> > error prone and fragile.
> 
> Great! That's means we are at least aligned on this topic. :-)
> 
> >
> >> The AMBA bus and some of its drivers a good example of how this has
> >> been implemented:
> >> driver/amba/bus.c
> >> drivers/mmc/host/mmci.c
> >> drivers/spi/spi-pl022.c
> >>
> >> This conclusion I have made from this is:
> >> - Using pm_runtime_get_sync() during the ->probe() path to explicitly
> >> power up a PM domain, is not suitable as the _common_ solution to
> >> solve the race condition. It certainly may work for some scenarios,
> >> but not for those I am looking at.
> >
> > I think, however, that it might work if the core calls pm_runtime_get_sync()
> > from driver_probe_device().
> 
> Currently this won't work.
> 
> That's because the buses' ->probe() are invoked in this path and they
> are doing the attachment of the device to its PM domain.
> 
> In other words, we can't power up the PM domain using
> pm_runtime_get_sync(), until the device has been attached to its PM
> domain. Right?


I think this is one of the issues that we have there. Why do we conflate
probing and placing the device into a power domain? The latter should
happen when we register the device. The fact that a device was probed
and has a driver bound or not bound to it should have no bearing on
whether the device is member of power domain or not.

Thanks.

-- 
Dmitry

  reply	other threads:[~2014-11-04  9:05 UTC|newest]

Thread overview: 98+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2014-10-13 14:02 [PATCH v3 0/9] PM / Domains: Fix race conditions during boot Ulf Hansson
2014-10-13 14:02 ` Ulf Hansson
2014-10-13 14:02 ` [PATCH v3 1/9] PM / Domains: Add dev_pm_domain_get|put() APIs Ulf Hansson
2014-10-13 14:02   ` Ulf Hansson
2014-10-13 14:02 ` [PATCH v3 2/9] PM / Domains: Enable genpd to support ->get|put() callbacks Ulf Hansson
2014-10-13 14:02   ` Ulf Hansson
2014-10-13 14:02 ` [PATCH v3 3/9] amba: Keep PM domain powered during ->probe() Ulf Hansson
2014-10-13 14:02   ` Ulf Hansson
2014-10-13 14:02 ` [PATCH v3 4/9] drivercore / platform: " Ulf Hansson
2014-10-13 14:02   ` Ulf Hansson
2014-10-30 20:47   ` Kevin Hilman
2014-10-30 20:47     ` Kevin Hilman
2014-10-31  0:07     ` Dmitry Torokhov
2014-10-31  0:07       ` Dmitry Torokhov
2014-10-31  9:23       ` Ulf Hansson
2014-10-31  9:23         ` Ulf Hansson
2014-11-01  0:21         ` Rafael J. Wysocki
2014-11-01  0:21           ` Rafael J. Wysocki
2014-10-31  9:19     ` Ulf Hansson
2014-10-31  9:19       ` Ulf Hansson
2014-10-13 14:02 ` [PATCH v3 5/9] i2c: core: " Ulf Hansson
2014-10-13 14:02   ` Ulf Hansson
2014-10-13 14:02 ` [PATCH v3 6/9] spi: " Ulf Hansson
2014-10-13 14:02   ` Ulf Hansson
2014-10-13 14:02 ` [PATCH v3 7/9] mmc: core: Attach PM domain prior probing of SDIO func driver Ulf Hansson
2014-10-13 14:02   ` Ulf Hansson
2014-10-13 14:02 ` [PATCH v3 8/9] mmmc: core: Keep PM domain powered during ->probe() " Ulf Hansson
2014-10-13 14:02   ` Ulf Hansson
2014-10-13 14:02 ` [PATCH v3 9/9] PM / Domains: Remove pm_genpd_dev_need_restore() API Ulf Hansson
2014-10-13 14:02   ` Ulf Hansson
2014-10-24 16:12 ` [PATCH v3 0/9] PM / Domains: Fix race conditions during boot Kevin Hilman
2014-10-24 16:12   ` Kevin Hilman
2014-10-24 16:18   ` Mark Brown
2014-10-24 16:18     ` Mark Brown
2014-10-30 20:46     ` Kevin Hilman
2014-10-30 20:46       ` Kevin Hilman
2014-10-30 23:56       ` Mark Brown
2014-10-30 23:56         ` Mark Brown
2014-10-31  9:16   ` Ulf Hansson
2014-10-31  9:16     ` Ulf Hansson
2014-11-01  0:20     ` Rafael J. Wysocki
2014-11-01  0:20       ` Rafael J. Wysocki
2014-11-01  1:08       ` pm_runtime_enable() in ->probe() (was: Re: [PATCH v3 0/9] PM / Domains: Fix race conditions during boot) Rafael J. Wysocki
2014-11-01  1:08         ` Rafael J. Wysocki
2014-11-01  1:14         ` Rafael J. Wysocki
2014-11-01  1:14           ` Rafael J. Wysocki
2014-11-03 17:00           ` pm_runtime_enable() in ->probe() Kevin Hilman
2014-11-03 17:00             ` Kevin Hilman
2014-11-03 20:06             ` Alan Stern
2014-11-04  0:00               ` Rafael J. Wysocki
2014-11-04 16:45                 ` Alan Stern
2014-11-03 23:55             ` Rafael J. Wysocki
2014-11-03 23:55               ` Rafael J. Wysocki
2014-11-01 15:15         ` pm_runtime_enable() in ->probe() (was: Re: [PATCH v3 0/9] PM / Domains: Fix race conditions during boot) Alan Stern
2014-11-04  0:09           ` Rafael J. Wysocki
2014-11-04  1:23             ` Rafael J. Wysocki
2014-11-03 14:03       ` [PATCH v3 0/9] PM / Domains: Fix race conditions during boot Ulf Hansson
2014-11-03 14:03         ` Ulf Hansson
2014-11-04  1:43         ` Rafael J. Wysocki
2014-11-04  1:43           ` Rafael J. Wysocki
2014-11-04  8:20           ` Geert Uytterhoeven
2014-11-04  8:20             ` Geert Uytterhoeven
2014-11-04 13:32             ` Rafael J. Wysocki
2014-11-04 13:32               ` Rafael J. Wysocki
2014-11-04  8:54           ` Ulf Hansson
2014-11-04  8:54             ` Ulf Hansson
2014-11-04  9:05             ` Dmitry Torokhov [this message]
2014-11-04  9:05               ` Dmitry Torokhov
2014-11-04  9:24               ` Ulf Hansson
2014-11-04  9:24                 ` Ulf Hansson
2014-11-04 13:56                 ` Rafael J. Wysocki
2014-11-04 13:56                   ` Rafael J. Wysocki
2014-11-04 17:01                   ` Ulf Hansson
2014-11-04 17:01                     ` Ulf Hansson
2014-11-04 18:29                     ` Dmitry Torokhov
2014-11-04 18:29                       ` Dmitry Torokhov
2014-11-04 21:38                       ` Rafael J. Wysocki
2014-11-04 21:38                         ` Rafael J. Wysocki
2014-11-05  8:17                         ` Ulf Hansson
2014-11-05  8:17                           ` Ulf Hansson
2014-11-04 13:52               ` Rafael J. Wysocki
2014-11-04 13:52                 ` Rafael J. Wysocki
2014-11-04 13:51             ` Rafael J. Wysocki
2014-11-04 13:51               ` Rafael J. Wysocki
2014-11-04 16:42               ` Ulf Hansson
2014-11-04 16:42                 ` Ulf Hansson
2014-11-07 17:25                 ` Grygorii Strashko
2014-11-07 17:25                   ` Grygorii Strashko
2014-11-11 11:05                   ` Ulf Hansson
2014-11-11 11:05                     ` Ulf Hansson
2014-11-12 18:01                     ` Grygorii Strashko
2014-11-12 18:01                       ` Grygorii Strashko
2014-11-13  2:07                   ` Rafael J. Wysocki
2014-11-13  2:07                     ` Rafael J. Wysocki
2014-11-13 20:13                     ` Grygorii Strashko
2014-11-13 20:13                       ` Grygorii Strashko
2014-11-13 14:05                   ` Pavel Machek
2014-11-13 14:05                     ` Pavel Machek

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=20141104090521.GA13219@dtor-glaptop \
    --to=dmitry.torokhov@gmail.com \
    --cc=ben-linux@fluff.org \
    --cc=broonie@kernel.org \
    --cc=geert+renesas@glider.be \
    --cc=gregkh@linuxfoundation.org \
    --cc=horms@verge.net.au \
    --cc=jack.dai@rock-chips.com \
    --cc=jinkun.hong@rock-chips.com \
    --cc=kgene.kim@samsung.com \
    --cc=khilman@kernel.org \
    --cc=len.brown@intel.com \
    --cc=linux-arm-kernel@lists.infradead.org \
    --cc=linux-pm@vger.kernel.org \
    --cc=linux-samsung-soc@vger.kernel.org \
    --cc=linux@arm.linux.org.uk \
    --cc=magnus.damm@gmail.com \
    --cc=pavel@ucw.cz \
    --cc=philipp.zabel@gmail.com \
    --cc=rjw@rjwysocki.net \
    --cc=stern@rowland.harvard.edu \
    --cc=tomasz.figa@gmail.com \
    --cc=ulf.hansson@linaro.org \
    --cc=wsa@the-dreams.de \
    /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 an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.